mirror of https://github.com/xemu-project/xemu.git
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>
This commit is contained in:
parent
80a2ba3d3c
commit
f34e73cd69
|
@ -145,20 +145,12 @@ static VirtIOBlockReq *virtio_blk_get_request(VirtIOBlock *s)
|
||||||
return req;
|
return req;
|
||||||
}
|
}
|
||||||
|
|
||||||
#ifdef __linux__
|
|
||||||
static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
|
static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
|
||||||
{
|
{
|
||||||
struct sg_io_hdr hdr;
|
|
||||||
int ret;
|
int ret;
|
||||||
int status;
|
int status = VIRTIO_BLK_S_OK;
|
||||||
int i;
|
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
|
* We require at least one output segment each for the virtio_blk_outhdr
|
||||||
* and the SCSI command block.
|
* and the SCSI command block.
|
||||||
|
@ -172,21 +164,27 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
|
||||||
* No support for bidirection commands yet.
|
|
||||||
*/
|
|
||||||
if (req->elem.out_num > 2 && req->elem.in_num > 3) {
|
|
||||||
virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);
|
|
||||||
g_free(req);
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* The scsi inhdr is placed in the second-to-last input segment, just
|
* The scsi inhdr is placed in the second-to-last input segment, just
|
||||||
* before the regular inhdr.
|
* before the regular inhdr.
|
||||||
*/
|
*/
|
||||||
req->scsi = (void *)req->elem.in_sg[req->elem.in_num - 2].iov_base;
|
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;
|
||||||
|
}
|
||||||
|
|
||||||
|
/*
|
||||||
|
* No support for bidirection commands yet.
|
||||||
|
*/
|
||||||
|
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));
|
memset(&hdr, 0, sizeof(struct sg_io_hdr));
|
||||||
hdr.interface_id = 'S';
|
hdr.interface_id = 'S';
|
||||||
hdr.cmd_len = req->elem.out_sg[1].iov_len;
|
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);
|
ret = bdrv_ioctl(req->dev->bs, SG_IO, &hdr);
|
||||||
if (ret) {
|
if (ret) {
|
||||||
status = VIRTIO_BLK_S_UNSUPP;
|
status = VIRTIO_BLK_S_UNSUPP;
|
||||||
hdr.status = ret;
|
goto fail;
|
||||||
hdr.resid = hdr.dxfer_len;
|
|
||||||
} else if (hdr.status) {
|
|
||||||
status = VIRTIO_BLK_S_IOERR;
|
|
||||||
} else {
|
|
||||||
status = VIRTIO_BLK_S_OK;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
@ -258,14 +251,16 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
|
||||||
|
|
||||||
virtio_blk_req_complete(req, status);
|
virtio_blk_req_complete(req, status);
|
||||||
g_free(req);
|
g_free(req);
|
||||||
}
|
|
||||||
#else
|
#else
|
||||||
static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
|
abort();
|
||||||
{
|
#endif
|
||||||
virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);
|
|
||||||
|
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);
|
g_free(req);
|
||||||
}
|
}
|
||||||
#endif /* __linux__ */
|
|
||||||
|
|
||||||
typedef struct MultiReqBuffer {
|
typedef struct MultiReqBuffer {
|
||||||
BlockRequest blkreq[32];
|
BlockRequest blkreq[32];
|
||||||
|
|
Loading…
Reference in New Issue