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 <mre...@redhat.com>
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