Re: [Qemu-devel] [PATCH v3 6/7] scsi: Introduce scsi_req_cancel_async

[Fam Zheng]

On Thu, 09/25 10:51, Paolo Bonzini wrote:
> Il 25/09/2014 04:20, Fam Zheng ha scritto:
> > Devices will call this function to start an asynchronous cancellation. The
> > bus->info->cancel will be called after the request is canceled.
> > 
> > Devices will probably need to track a separate TMF request that triggers 
> > this
> > cancellation, and wait until the cancellation is done before completing it. 
> > So
> > we store a notifier list in SCSIRequest and in scsi_req_cancel_complete we
> > notify them.
> > 
> > Signed-off-by: Fam Zheng <address@hidden>
> > ---
> >  hw/scsi/scsi-bus.c     | 23 +++++++++++++++++++++++
> >  include/hw/scsi/scsi.h |  3 +++
> >  2 files changed, 26 insertions(+)
> > 
> > diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> > index c91db63..df7585a 100644
> > --- a/hw/scsi/scsi-bus.c
> > +++ b/hw/scsi/scsi-bus.c
> > @@ -566,6 +566,7 @@ SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops, 
> > SCSIDevice *d,
> >      req->ops = reqops;
> >      object_ref(OBJECT(d));
> >      object_ref(OBJECT(qbus->parent));
> > +    notifier_list_init(&req->cancel_notifiers);
> >      trace_scsi_req_alloc(req->dev->id, req->lun, req->tag);
> >      return req;
> >  }
> > @@ -1725,9 +1726,31 @@ void scsi_req_cancel_complete(SCSIRequest *req)
> >      if (req->bus->info->cancel) {
> >          req->bus->info->cancel(req);
> >      }
> > +    notifier_list_notify(&req->cancel_notifiers, req);
> 
> I think you also have to call notifier_list_notify from
> scsi_req_complete, because a cancelled request might end up being
> completed instead of cancelled.

Yes, will update the series.

> 
> In fact, the next obvious step (enabled by your bdrv_aio_cancel cleanup)
> would be to _not_ call scsi_req_cancel_complete if we can report
> completion or if there was an I/O error.  This can happen for
> scsi-generic, scsi_dma_complete_noio, etc.  Basically, moving the
> io_canceled check from the beginning of the completion routine to just
> before bdrv_aio_* or dma_aio_* are called.

Okay.

Thanks,
Fam