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 <[email protected]>
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
