On 03/07/2012 08:01 PM, Paolo Bonzini wrote:
> Linux really looks only at scsi->errors. Arguably it is their bug,
> but we can make it safe for older guests now.
>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> hw/virtio-blk.c | 48 +++++++++++++++++++++++-------------------------
> 1 files changed, 23 insertions(+), 25 deletions(-)
>
> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> index 49990f8..b7e510d 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 ret = -1;
> int status;
> 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;
> @@ -229,9 +227,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;
> + goto fail;
> } else if (hdr.status) {
> status = VIRTIO_BLK_S_IOERR;
> } else {
> @@ -258,14 +254,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);
I get to following compile error:
In function ‘virtio_blk_handle_request’:
virtio-blk.c:264:28: error: ‘status’ may be used uninitialized in this function
[-Werror=uninitialized]
virtio-blk.c:151:9: note: ‘status’ was declared here
cc1: all warnings being treated as errors
Are you using -disable-werror ?
Orit
> g_free(req);
> }
> -#endif /* __linux__ */
>
> typedef struct MultiReqBuffer {
> BlockRequest blkreq[32];
