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. > > 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. > > In nearly all locations where virtio_scsi_bad_req() is called, the only > thing to do next is to return to the caller. > > The virtio_scsi_handle_cmd_req_prepare() function is an exception though. > > 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. > > We have now a new condition to take into account: the device is broken > and should stop all processing. > > 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(VirtIOSCSI *s, VirtIOSCSIReq *req)
> void virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq)
> {
> VirtIOSCSIReq *req, *next;
> + int ret;
> +
> QTAILQ_HEAD(, VirtIOSCSIReq) reqs = QTAILQ_HEAD_INITIALIZER(reqs);
>
> while ((req = virtio_scsi_pop_req(s, vq))) {
> - if (virtio_scsi_handle_cmd_req_prepare(s, req)) {
> + ret = virtio_scsi_handle_cmd_req_prepare(s, req);
> + if (!ret) {
> QTAILQ_INSERT_TAIL(&reqs, req, next);
> + } else if (ret == -EINVAL) {
> + /* The device is broken and shouldn't process any request */
> + while (!QTAILQ_EMPTY(&reqs)) {
> + req = 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.
signature.asc
Description: PGP signature
