From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47494) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YRRDN-0007Z8-Hs for qemu-devel@nongnu.org; Fri, 27 Feb 2015 15:08:50 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YRRDI-00061Y-BK for qemu-devel@nongnu.org; Fri, 27 Feb 2015 15:08:49 -0500 Message-ID: <54F0CEC7.70000@redhat.com> Date: Fri, 27 Feb 2015 15:08:39 -0500 From: Max Reitz MIME-Version: 1.0 References: <1425055440-18038-1-git-send-email-mreitz@redhat.com> <1425055440-18038-6-git-send-email-mreitz@redhat.com> In-Reply-To: <1425055440-18038-6-git-send-email-mreitz@redhat.com> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 05/11] block: Move BDS close notifiers into BB List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-block@nongnu.org Cc: Kevin Wolf , Fam Zheng , qemu-devel@nongnu.org, Markus Armbruster , Stefan Hajnoczi , Paolo Bonzini On 2015-02-27 at 11:43, Max Reitz wrote: > The only remaining user of the BDS close notifiers is NBD which uses > them to determine when a BDS tree is being ejected. This patch removes > the BDS-level close notifiers and adds a notifier list to the > BlockBackend structure that is invoked whenever a BDS is removed. > > Symmetrically to that, another notifier list is added that is invoked > whenever a BDS is inserted. The dataplane implementations for virtio-blk > and virtio-scsi use both notifier types for setting up and removing op > blockers. This is not only important for setting up the op blockers on > insertion, but also for removing them on ejection since bdrv_delete() > asserts that there are no op blockers set up. > > Signed-off-by: Max Reitz > --- > block.c | 7 ---- > block/block-backend.c | 19 +++++++-- > blockdev-nbd.c | 36 +---------------- > hw/block/dataplane/virtio-blk.c | 77 +++++++++++++++++++++++++++--------- > hw/scsi/virtio-scsi.c | 87 ++++++++++++++++++++++++++++++++++++++--- > include/block/block.h | 1 - > include/block/block_int.h | 2 - > include/hw/virtio/virtio-scsi.h | 10 +++++ > include/sysemu/block-backend.h | 3 +- > nbd.c | 13 ++++++ > 10 files changed, 182 insertions(+), 73 deletions(-) [snip] > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c > index 5469bad..570f2fd 100644 > --- a/hw/scsi/virtio-scsi.c > +++ b/hw/scsi/virtio-scsi.c > @@ -755,6 +755,37 @@ static void virtio_scsi_change(SCSIBus *bus, SCSIDevice *dev, SCSISense sense) > } > } > > +static void virtio_scsi_set_up_op_blockers(VirtIOSCSI *s, SCSIDevice *sd) > +{ > + if (!s->blocker) { > + error_setg(&s->blocker, "block device is in use by data plane"); This will no longer be necessary with v2 of my virtio-scsi op blocker patch. > + } > + blk_op_block_all(sd->conf.blk, s->blocker); > +} > + > +static void virtio_scsi_remove_op_blockers(VirtIOSCSI *s, SCSIDevice *sd) > +{ > + if (s->blocker) { > + blk_op_unblock_all(sd->conf.blk, s->blocker); > + } > +} > + > +static void virtio_scsi_blk_insert_notifier(Notifier *n, void *data) > +{ > + VirtIOSCSIBlkChangeNotifier *cn = DO_UPCAST(VirtIOSCSIBlkChangeNotifier, > + n, n); > + assert(cn->sd->conf.blk == data); > + virtio_scsi_set_up_op_blockers(cn->s, cn->sd); > +} > + > +static void virtio_scsi_blk_remove_notifier(Notifier *n, void *data) > +{ > + VirtIOSCSIBlkChangeNotifier *cn = DO_UPCAST(VirtIOSCSIBlkChangeNotifier, > + n, n); > + assert(cn->sd->conf.blk == data); > + virtio_scsi_remove_op_blockers(cn->s, cn->sd); > +} > + > static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev, > Error **errp) > { > @@ -763,12 +794,26 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev, > SCSIDevice *sd = SCSI_DEVICE(dev); > > if (s->ctx && !s->dataplane_disabled) { > + VirtIOSCSIBlkChangeNotifier *insert_notifier, *remove_notifier; > + > + insert_notifier = g_new0(VirtIOSCSIBlkChangeNotifier, 1); > + insert_notifier->n.notify = virtio_scsi_blk_insert_notifier; > + insert_notifier->s = s; > + insert_notifier->sd = sd; > + blk_add_insert_bs_notifier(sd->conf.blk, &insert_notifier->n); > + QTAILQ_INSERT_TAIL(&s->insert_notifiers, insert_notifier, next); > + > + remove_notifier = g_new0(VirtIOSCSIBlkChangeNotifier, 1); > + remove_notifier->n.notify = virtio_scsi_blk_remove_notifier; > + remove_notifier->s = s; > + remove_notifier->sd = sd; > + blk_add_remove_bs_notifier(sd->conf.blk, &remove_notifier->n); > + QTAILQ_INSERT_TAIL(&s->remove_notifiers, remove_notifier, next); > + > if (blk_op_is_blocked(sd->conf.blk, BLOCK_OP_TYPE_DATAPLANE, errp)) { > return; > } > - assert(!s->blocker); > - error_setg(&s->blocker, "block device is in use by data plane"); Same for these two lines removed (they just don't exist any more). > - blk_op_block_all(sd->conf.blk, s->blocker); > + virtio_scsi_set_up_op_blockers(s, sd); > } > > if ((vdev->guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) { > @@ -784,6 +829,7 @@ static void virtio_scsi_hotunplug(HotplugHandler *hotplug_dev, DeviceState *dev, > VirtIODevice *vdev = VIRTIO_DEVICE(hotplug_dev); > VirtIOSCSI *s = VIRTIO_SCSI(vdev); > SCSIDevice *sd = SCSI_DEVICE(dev); > + VirtIOSCSIBlkChangeNotifier *insert_notifier, *remove_notifier; > > if ((vdev->guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) { > virtio_scsi_push_event(s, sd, > @@ -791,11 +837,39 @@ static void virtio_scsi_hotunplug(HotplugHandler *hotplug_dev, DeviceState *dev, > VIRTIO_SCSI_EVT_RESET_REMOVED); > } > > - if (s->ctx && s->blocker) { > - blk_op_unblock_all(sd->conf.blk, s->blocker); > + if (s->ctx) { > + virtio_scsi_remove_op_blockers(s, sd); > + } > + > + QTAILQ_FOREACH(insert_notifier, &s->insert_notifiers, next) { > + if (insert_notifier->sd == sd) { > + break; > + } > + } > + if (insert_notifier) { > + notifier_remove(&insert_notifier->n); > + QTAILQ_REMOVE(&s->insert_notifiers, insert_notifier, next); > + g_free(insert_notifier); > + } > + > + QTAILQ_FOREACH(remove_notifier, &s->remove_notifiers, next) { > + if (remove_notifier->sd == sd) { > + break; > + } > + } > + if (remove_notifier) { > + notifier_remove(&remove_notifier->n); > + QTAILQ_REMOVE(&s->remove_notifiers, remove_notifier, next); > + g_free(remove_notifier); > + } > + > + if (QTAILQ_EMPTY(&s->insert_notifiers) && > + QTAILQ_EMPTY(&s->remove_notifiers)) > + { > error_free(s->blocker); > s->blocker = NULL; > } > + And for this last block. Max