From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58412) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7RuA-0008Cd-08 for qemu-devel@nongnu.org; Tue, 13 Mar 2012 09:36:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S7Ru3-0003zD-Hj for qemu-devel@nongnu.org; Tue, 13 Mar 2012 09:36:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:30889) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7Ru3-0003ys-9V for qemu-devel@nongnu.org; Tue, 13 Mar 2012 09:36:39 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q2DDabKF005428 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 13 Mar 2012 09:36:37 -0400 Message-ID: <4F5F4D4D.1060600@redhat.com> Date: Tue, 13 Mar 2012 15:36:13 +0200 From: Orit Wasserman MIME-Version: 1.0 References: <1331143301-28408-1-git-send-email-pbonzini@redhat.com> <1331143301-28408-2-git-send-email-pbonzini@redhat.com> In-Reply-To: <1331143301-28408-2-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/3] virtio-blk: report non-zero status when failing SG_IO requests List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: amit.shah@redhat.com, qemu-devel@nongnu.org, mst@redhat.com 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. >=20 > Signed-off-by: Paolo Bonzini > --- > hw/virtio-blk.c | 48 +++++++++++++++++++++++------------------------= - > 1 files changed, 23 insertions(+), 25 deletions(-) >=20 > 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(Vir= tIOBlock *s) > return req; > } > =20 > -#ifdef __linux__ > static void virtio_blk_handle_scsi(VirtIOBlockReq *req) > { > - struct sg_io_hdr hdr; > - int ret; > + int ret =3D -1; > int status; > int i; > =20 > - if ((req->dev->vdev.guest_features & (1 << VIRTIO_BLK_F_SCSI)) =3D= =3D 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) > } > =20 > /* > - * No support for bidirection commands yet. > + * The scsi inhdr is placed in the second-to-last input segment, j= ust > + * 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 =3D (void *)req->elem.in_sg[req->elem.in_num - 2].iov_ba= se; > + > + if ((req->dev->vdev.guest_features & (1 << VIRTIO_BLK_F_SCSI)) =3D= =3D 0) { > + status =3D VIRTIO_BLK_S_UNSUPP; > + goto fail; > } > =20 > /* > - * The scsi inhdr is placed in the second-to-last input segment, j= ust > - * before the regular inhdr. > + * No support for bidirection commands yet. > */ > - req->scsi =3D (void *)req->elem.in_sg[req->elem.in_num - 2].iov_ba= se; > + if (req->elem.out_num > 2 && req->elem.in_num > 3) { > + status =3D VIRTIO_BLK_S_UNSUPP; > + goto fail; > + } > =20 > +#ifdef __linux__ > + struct sg_io_hdr hdr; > memset(&hdr, 0, sizeof(struct sg_io_hdr)); > hdr.interface_id =3D 'S'; > hdr.cmd_len =3D req->elem.out_sg[1].iov_len; > @@ -229,9 +227,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *= req) > =20 > ret =3D bdrv_ioctl(req->dev->bs, SG_IO, &hdr); > if (ret) { > - status =3D VIRTIO_BLK_S_UNSUPP; > - hdr.status =3D ret; > - hdr.resid =3D hdr.dxfer_len; > + goto fail; > } else if (hdr.status) { > status =3D VIRTIO_BLK_S_IOERR; > } else { > @@ -258,14 +254,16 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq= *req) > =20 > 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 =91virtio_blk_handle_request=92: virtio-blk.c:264:28: error: =91status=92 may be used uninitialized in thi= s function [-Werror=3Duninitialized] virtio-blk.c:151:9: note: =91status=92 was declared here cc1: all warnings being treated as errors Are you using -disable-werror ? Orit > g_free(req); > } > -#endif /* __linux__ */ > =20 > typedef struct MultiReqBuffer { > BlockRequest blkreq[32];