From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38798) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWkXw-0003IJ-9n for qemu-devel@nongnu.org; Wed, 24 Sep 2014 07:15:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XWkXp-0004mq-Mc for qemu-devel@nongnu.org; Wed, 24 Sep 2014 07:15:44 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39188) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWkXp-0004ls-4e for qemu-devel@nongnu.org; Wed, 24 Sep 2014 07:15:37 -0400 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s8OBFVRu007421 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Wed, 24 Sep 2014 07:15:31 -0400 Message-ID: <5422A7CF.4010400@redhat.com> Date: Wed, 24 Sep 2014 13:15:27 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1411547278-25915-1-git-send-email-famz@redhat.com> <1411547278-25915-7-git-send-email-famz@redhat.com> In-Reply-To: <1411547278-25915-7-git-send-email-famz@redhat.com> Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 6/7] scsi: Introduce scsi_req_cancel_async List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: Kevin Wolf , Stefan Hajnoczi Il 24/09/2014 10:27, 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_canceled we notify them. > > Signed-off-by: Fam Zheng There are some remnants of the old implementation. Apart from that it looks good. Paolo > --- > hw/scsi/scsi-bus.c | 31 ++++++++++++++++++++++++++++--- > include/hw/scsi/scsi.h | 3 +++ > 2 files changed, 31 insertions(+), 3 deletions(-) > > diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c > index fca4a9e..c1b05a1 100644 > --- a/hw/scsi/scsi-bus.c > +++ b/hw/scsi/scsi-bus.c > @@ -553,9 +553,10 @@ SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops, SCSIDevice *d, > BusState *qbus = BUS(bus); > const int memset_off = offsetof(SCSIRequest, sense) > + sizeof(req->sense); > + size_t reqops_size = reqops ? reqops->size : sizeof(SCSIRequest); > > - req = g_slice_alloc(reqops->size); > - memset((uint8_t *)req + memset_off, 0, reqops->size - memset_off); > + req = g_slice_alloc(reqops_size); > + memset((uint8_t *)req + memset_off, 0, reqops_size - memset_off); > req->refcount = 1; > req->bus = bus; > req->dev = d; This is not needed anymore. > @@ -566,6 +567,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; > } > @@ -1600,7 +1602,7 @@ void scsi_req_unref(SCSIRequest *req) > if (bus->info->free_request && req->hba_private) { > bus->info->free_request(bus, req->hba_private); > } > - if (req->ops->free_req) { > + if (req->ops && req->ops->free_req) { Same here. > req->ops->free_req(req); > } > object_unref(OBJECT(req->dev)); > @@ -1725,9 +1727,32 @@ void scsi_req_canceled(SCSIRequest *req) > if (req->bus->info->cancel) { > req->bus->info->cancel(req); > } > + notifier_list_notify(&req->cancel_notifiers, req); > scsi_req_unref(req); > } > > +/* Cancel @req asynchronously. > + * @tmf_req is added to @req's cancellation dependency list, the bus will be > + * notified with @tmf_req when all the requests it depends on are canceled. @tmf_req doesn't exist anymore... :) > + * @red: The request to cancel. > + * @tmf_req: The tmf request which cancels @req. > + * */ > +void scsi_req_cancel_async(SCSIRequest *req, Notifier *notifier) > +{ > + trace_scsi_req_cancel(req->dev->id, req->lun, req->tag); > + notifier_list_add(&req->cancel_notifiers, notifier); Perhaps wrap with "if (notifier)"? > + if (req->io_canceled) { > + return; > + } > + scsi_req_ref(req); > + scsi_req_dequeue(req); > + req->io_canceled = true; > + if (req->aiocb) { > + bdrv_aio_cancel_async(req->aiocb); > + } > +} > + > void scsi_req_cancel(SCSIRequest *req) > { > trace_scsi_req_cancel(req->dev->id, req->lun, req->tag); > diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h > index 0fa2a8a..6a051df 100644 > --- a/include/hw/scsi/scsi.h > +++ b/include/hw/scsi/scsi.h > @@ -5,6 +5,7 @@ > #include "block/block.h" > #include "hw/block/block.h" > #include "sysemu/sysemu.h" > +#include "qemu/notify.h" > > #define MAX_SCSI_DEVS 255 > > @@ -53,6 +54,7 @@ struct SCSIRequest { > void *hba_private; > size_t resid; > SCSICommand cmd; > + NotifierList cancel_notifiers; > > /* Note: > * - fields before sense are initialized by scsi_req_alloc; > @@ -267,6 +269,7 @@ int scsi_req_get_sense(SCSIRequest *req, uint8_t *buf, int len); > void scsi_req_abort(SCSIRequest *req, int status); > void scsi_req_canceled(SCSIRequest *req); > void scsi_req_cancel(SCSIRequest *req); > +void scsi_req_cancel_async(SCSIRequest *req, Notifier *notifier); > void scsi_req_retry(SCSIRequest *req); > void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense); > void scsi_device_set_ua(SCSIDevice *sdev, SCSISense sense); >