From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46875) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1cRX-0003TC-7Q for qemu-devel@nongnu.org; Wed, 25 Nov 2015 10:57:16 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a1cRW-0008TZ-54 for qemu-devel@nongnu.org; Wed, 25 Nov 2015 10:57:15 -0500 Date: Wed, 25 Nov 2015 16:57:06 +0100 From: Kevin Wolf Message-ID: <20151125155706.GE12581@noname.str.redhat.com> References: <1447108773-6836-1-git-send-email-mreitz@redhat.com> <1447108773-6836-13-git-send-email-mreitz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1447108773-6836-13-git-send-email-mreitz@redhat.com> Subject: Re: [Qemu-devel] [PATCH v7 12/24] virtio-blk: Functions for op blocker management List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: Alberto Garcia , qemu-block@nongnu.org, John Snow , qemu-devel@nongnu.org, Markus Armbruster , Stefan Hajnoczi , Paolo Bonzini Am 09.11.2015 um 23:39 hat Max Reitz geschrieben: > Put the code for setting up and removing op blockers into an own > function, respectively. Then, we can invoke those functions whenever a > BDS is removed from an virtio-blk BB or inserted into it. > > Signed-off-by: Max Reitz Do you know of a case where this is observable? I don't think you can change the medium of a virtio-blk device, and blk_set_bs() isn't converted to a wrapper around blk_remove/insert_bs() yet. If this patch is necessary to fix something observable, the latter is probably a bug. > diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c > index c42ddeb..4c95d5b 100644 > --- a/hw/block/dataplane/virtio-blk.c > +++ b/hw/block/dataplane/virtio-blk.c > @@ -39,6 +39,8 @@ struct VirtIOBlockDataPlane { > EventNotifier *guest_notifier; /* irq */ > QEMUBH *bh; /* bh for guest notification */ > > + Notifier insert_notifier, remove_notifier; > + > /* Note that these EventNotifiers are assigned by value. This is > * fine as long as you do not call event_notifier_cleanup on them > * (because you don't own the file descriptor or handle; you just > @@ -137,6 +139,54 @@ static void handle_notify(EventNotifier *e) > blk_io_unplug(s->conf->conf.blk); > } > > +static void data_plane_set_up_op_blockers(VirtIOBlockDataPlane *s) > +{ > + assert(!s->blocker); > + error_setg(&s->blocker, "block device is in use by data plane"); > + blk_op_block_all(s->conf->conf.blk, s->blocker); > + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_RESIZE, s->blocker); > + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_DRIVE_DEL, s->blocker); > + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_BACKUP_SOURCE, s->blocker); > + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_CHANGE, s->blocker); > + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_COMMIT_SOURCE, s->blocker); > + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_COMMIT_TARGET, s->blocker); > + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_EJECT, s->blocker); > + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT, > + s->blocker); > + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT, > + s->blocker); > + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_INTERNAL_SNAPSHOT_DELETE, > + s->blocker); > + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_MIRROR, s->blocker); > + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_STREAM, s->blocker); > + blk_op_unblock(s->conf->conf.blk, BLOCK_OP_TYPE_REPLACE, s->blocker); > +} This makes me wonder: What do we even block here any more? If I didn't miss anything, it's only BLOCK_OP_TYPE_BACKUP_TARGET, and I'm not sure why this needs to be blocked, or if we simply forgot to enable it. Kevin