From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35580) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1boYsd-0007yr-J3 for qemu-devel@nongnu.org; Mon, 26 Sep 2016 12:35:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1boYsY-0008At-93 for qemu-devel@nongnu.org; Mon, 26 Sep 2016 12:35:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39666) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1boYsX-0008Aa-VU for qemu-devel@nongnu.org; Mon, 26 Sep 2016 12:35:42 -0400 Date: Mon, 26 Sep 2016 17:35:38 +0100 From: Stefan Hajnoczi Message-ID: <20160926163538.GG25705@stefanha-x1.localdomain> References: <147487882735.6679.8076815106195077844.stgit@bahia> <147487889679.6679.15125059315439240946.stgit@bahia> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="HB4mHL4PVvkpZAgW" Content-Disposition: inline In-Reply-To: <147487889679.6679.15125059315439240946.stgit@bahia> Subject: Re: [Qemu-devel] [PATCH v3 8/9] virtio-scsi: convert virtio_scsi_bad_req() to use virtio_error() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Kurz Cc: qemu-devel@nongnu.org, Kevin Wolf , "Michael S. Tsirkin" , Jason Wang , Max Reitz , "Aneesh Kumar K.V" , Cornelia Huck , Paolo Bonzini --HB4mHL4PVvkpZAgW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Sep 26, 2016 at 10:34:56AM +0200, Greg Kurz wrote: > The virtio_scsi_bad_req() function is called when a guest sends a > request with missing or ill-sized headers. This generally happens > when the virtio_scsi_parse_req() function returns an error. >=20 > With this patch, virtio_scsi_bad_req() will mark the device as broken, > detach the request from the virtqueue and free it, instead of forcing > QEMU to exit. >=20 > In nearly all locations where virtio_scsi_bad_req() is called, the only > thing to do next is to return to the caller. >=20 > The virtio_scsi_handle_cmd_req_prepare() function is an exception though. >=20 > It is called in a loop by virtio_scsi_handle_cmd_vq() and passed requests > freshly popped from a cmd virtqueue; virtio_scsi_handle_cmd_req_prepare() > does some sanity checks on the request and returns a boolean flag to > indicate whether the request should be queued or not. In the latter case, > virtio_scsi_handle_cmd_req_prepare() has detected a non-fatal error and > sent a response back to the guest. >=20 > We have now a new condition to take into account: the device is broken > and should stop all processing. >=20 > The return value of virtio_scsi_handle_cmd_req_prepare() is hence changed > to an int. A return value of zero means that the request should be queued. > Other non-fatal error cases where the reqyest shoudn't be queued return s/reqyest/request/ > @@ -574,11 +578,24 @@ static void virtio_scsi_handle_cmd_req_submit(VirtI= OSCSI *s, VirtIOSCSIReq *req) > void virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq) > { > VirtIOSCSIReq *req, *next; > + int ret; > + > QTAILQ_HEAD(, VirtIOSCSIReq) reqs =3D QTAILQ_HEAD_INITIALIZER(reqs); > =20 > while ((req =3D virtio_scsi_pop_req(s, vq))) { > - if (virtio_scsi_handle_cmd_req_prepare(s, req)) { > + ret =3D virtio_scsi_handle_cmd_req_prepare(s, req); > + if (!ret) { > QTAILQ_INSERT_TAIL(&reqs, req, next); > + } else if (ret =3D=3D -EINVAL) { > + /* The device is broken and shouldn't process any request */ > + while (!QTAILQ_EMPTY(&reqs)) { > + req =3D QTAILQ_FIRST(&reqs); > + QTAILQ_REMOVE(&reqs, req, next); > + blk_io_unplug(req->sreq->dev->conf.blk); Are you sure blk_io_plug() was called for this request? If we returned early in virtio_scsi_handle_cmd_req_prepare() then it wasn't called. > + scsi_req_unref(req->sreq); Which scsi_req_ref() is this paired with? If it's the call in scsi_req_enqueue() then that function was never called and we shouldn't unref. --HB4mHL4PVvkpZAgW Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEcBAEBAgAGBQJX6U5aAAoJEJykq7OBq3PI8nQH/jAH2QiNwoOmhaMQxbx9oI9K q3wlwHQx/0jLTuds7E3BulHE5l57d1SeBabzaqKo6XYWcIpfPR2hDb4ZT3ukc0fc prEWBDOIwcItnPs0mW7C0/g4GYFSQterqzdT2QVZRiXBXVK1rAxeqlr81AErxW10 gFWureQ43x5kItHrLnirn/ZQo/t9N2lViI7Z5gdmDPQV3T0wN8HC4fZ3d6Yw0Ui0 Jvapfc89arflC9Pilf+2N/tPRVt/jsMWfFUJZ+kTkdFe8DyGXV6EezY+kAKf0Nm5 RdKkFMo2uoeRFEL+zr5kwB3E74mL32M82LnSW/n6PfY2jUll5590AXdb7dbjVF8= =yZsc -----END PGP SIGNATURE----- --HB4mHL4PVvkpZAgW--