[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 8/9] virtio-scsi: convert virtio_scsi_bad_req
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v3 8/9] virtio-scsi: convert virtio_scsi_bad_req() to use virtio_error() |
Date: |
Mon, 26 Sep 2016 17:35:38 +0100 |
User-agent: |
Mutt/1.7.0 (2016-08-17) |
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
- Re: [Qemu-devel] [PATCH v3 3/9] virtio-9p: handle handle_9p_output() error, (continued)
- [Qemu-devel] [PATCH v3 4/9] virtio-blk: handle virtio_blk_handle_request() errors, Greg Kurz, 2016/09/26
- [Qemu-devel] [PATCH v3 5/9] virtio-net: handle virtio_net_handle_ctrl() error, Greg Kurz, 2016/09/26
- [Qemu-devel] [PATCH v3 6/9] virtio-net: handle virtio_net_receive() errors, Greg Kurz, 2016/09/26
- [Qemu-devel] [PATCH v3 7/9] virtio-net: handle virtio_net_flush_tx() errors, Greg Kurz, 2016/09/26
- [Qemu-devel] [PATCH v3 8/9] virtio-scsi: convert virtio_scsi_bad_req() to use virtio_error(), Greg Kurz, 2016/09/26
- Re: [Qemu-devel] [PATCH v3 8/9] virtio-scsi: convert virtio_scsi_bad_req() to use virtio_error(),
Stefan Hajnoczi <=
- [Qemu-devel] [PATCH v3 9/9] virtio-scsi: handle virtio_scsi_set_config() error, Greg Kurz, 2016/09/26
- Re: [Qemu-devel] [PATCH v3 0/9] virtio: avoid inappropriate QEMU termination, no-reply, 2016/09/26