aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaolo Bonzini <pbonzini@redhat.com>2012-05-16 12:54:03 +0200
committerAnthony Liguori <aliguori@us.ibm.com>2012-05-21 15:40:50 -0500
commitf34e73cd69bdbdb9b1d56b288c5e14d6fff58165 (patch)
tree1214cf7f27ee9b25cfb49e1f0ae531a8c79385e9
parent80a2ba3d3cf33b777d6a45776b0625b9a4283951 (diff)
downloadqemu-f34e73cd69bdbdb9b1d56b288c5e14d6fff58165.zip
qemu-f34e73cd69bdbdb9b1d56b288c5e14d6fff58165.tar.gz
qemu-f34e73cd69bdbdb9b1d56b288c5e14d6fff58165.tar.bz2
virtio-blk: report non-zero status when failing SG_IO requests
Linux really looks only at scsi->errors for SG_IO requests; it does not look at the virtio request status at all. Because of this, when a SG_IO request is failed early with virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP), without writing hdr.status, it will look like a success to the guest. This is their bug, but we can make it safe for older guests now by forcing scsi->errors to have a non-zero value whenever a request has to be failed. But if we fix the bug in the guest driver, we will have another problem because QEMU returns VIRTIO_BLK_S_IOERR if the status is non-zero, and Linux translates that to -EIO. Rather, the guest should succeed the request and pass the non-zero status via the userspace-provided SG_IO structure. So, remove the case where virtio_blk_handle_scsi can return VIRTIO_BLK_S_IOERR. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
-rw-r--r--hw/virtio-blk.c51
1 files changed, 23 insertions, 28 deletions
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index 49990f8..d11bb80 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -145,20 +145,12 @@ static VirtIOBlockReq *virtio_blk_get_request(VirtIOBlock *s)
return req;
}
-#ifdef __linux__
static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
{
- struct sg_io_hdr hdr;
int ret;
- int status;
+ int status = VIRTIO_BLK_S_OK;
int i;
- if ((req->dev->vdev.guest_features & (1 << VIRTIO_BLK_F_SCSI)) == 0) {
- virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);
- g_free(req);
- return;
- }
-
/*
* We require at least one output segment each for the virtio_blk_outhdr
* and the SCSI command block.
@@ -173,20 +165,26 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
}
/*
- * No support for bidirection commands yet.
+ * The scsi inhdr is placed in the second-to-last input segment, just
+ * before the regular inhdr.
*/
- if (req->elem.out_num > 2 && req->elem.in_num > 3) {
- virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);
- g_free(req);
- return;
+ req->scsi = (void *)req->elem.in_sg[req->elem.in_num - 2].iov_base;
+
+ if ((req->dev->vdev.guest_features & (1 << VIRTIO_BLK_F_SCSI)) == 0) {
+ status = VIRTIO_BLK_S_UNSUPP;
+ goto fail;
}
/*
- * The scsi inhdr is placed in the second-to-last input segment, just
- * before the regular inhdr.
+ * No support for bidirection commands yet.
*/
- req->scsi = (void *)req->elem.in_sg[req->elem.in_num - 2].iov_base;
+ if (req->elem.out_num > 2 && req->elem.in_num > 3) {
+ status = VIRTIO_BLK_S_UNSUPP;
+ goto fail;
+ }
+#ifdef __linux__
+ struct sg_io_hdr hdr;
memset(&hdr, 0, sizeof(struct sg_io_hdr));
hdr.interface_id = 'S';
hdr.cmd_len = req->elem.out_sg[1].iov_len;
@@ -230,12 +228,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
ret = bdrv_ioctl(req->dev->bs, SG_IO, &hdr);
if (ret) {
status = VIRTIO_BLK_S_UNSUPP;
- hdr.status = ret;
- hdr.resid = hdr.dxfer_len;
- } else if (hdr.status) {
- status = VIRTIO_BLK_S_IOERR;
- } else {
- status = VIRTIO_BLK_S_OK;
+ goto fail;
}
/*
@@ -258,14 +251,16 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
virtio_blk_req_complete(req, status);
g_free(req);
-}
#else
-static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
-{
- virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);
+ abort();
+#endif
+
+fail:
+ /* Just put anything nonzero so that the ioctl fails in the guest. */
+ stl_p(&req->scsi->errors, 255);
+ virtio_blk_req_complete(req, status);
g_free(req);
}
-#endif /* __linux__ */
typedef struct MultiReqBuffer {
BlockRequest blkreq[32];