Re: [Qemu-devel] [PATCH v4 8/9] virtio-scsi: convert virtio_scsi_bad_req() to use virtio_error()

[Stefan Hajnoczi]

On Fri, Sep 30, 2016 at 05:13:40PM +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 request shoudn't be queued  return
> a negative errno (values are vaguely inspired by the error condition, but
> the only goal here is to discriminate the case we're interested in).
> 
> And finally, if virtio_scsi_bad_req() was called, -EINVAL is returned. In
> this case, virtio_scsi_handle_cmd_vq() detaches and frees already queued
> requests, instead of submitting them.
> 
> Signed-off-by: Greg Kurz <address@hidden>
> ---
> v4: - fixed success return value in virtio_scsi_handle_cmd_req_prepare()
>     - fixed typo in changelog
> ---
>  hw/scsi/virtio-scsi.c |   46 ++++++++++++++++++++++++++++++++--------------
>  1 file changed, 32 insertions(+), 14 deletions(-)

Reviewed-by: Stefan Hajnoczi <address@hidden>

signature.asc
Description: PGP signature