On Thu, 06/27 12:57, Paolo Bonzini wrote:
> Il 27/06/2013 11:41, Fam Zheng ha scritto:
> > On Thu, 06/27 10:15, Stefan Hajnoczi wrote:
> >> On Wed, Jun 26, 2013 at 11:59:19AM +0800, Fam Zheng wrote:
> >>> Add target-id (optional) to drive-backup command, to make the target bs
> >>> a named drive so that we can operate on it (e.g. export with NBD).
> >>>
> >>> Signed-off-by: Fam Zheng <f...@redhat.com>
> >>> ---
> >>> blockdev.c | 4 +++-
> >>> qapi-schema.json | 7 +++++--
> >>> qmp-commands.hx | 3 ++-
> >>> 3 files changed, 10 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/blockdev.c b/blockdev.c
> >>> index b3a57e0..5e694f3 100644
> >>> --- a/blockdev.c
> >>> +++ b/blockdev.c
> >>> @@ -935,6 +935,7 @@ static void drive_backup_prepare(BlkTransactionState
> >>> *common, Error **errp)
> >>> backup = common->action->drive_backup;
> >>>
> >>> qmp_drive_backup(backup->device, backup->target,
> >>> + backup->has_target_id, backup->target_id,
> >>> backup->has_format, backup->format,
> >>> backup->has_mode, backup->mode,
> >>> backup->has_speed, backup->speed,
> >>> @@ -1420,6 +1421,7 @@ void qmp_block_commit(const char *device,
> >>> }
> >>>
> >>> void qmp_drive_backup(const char *device, const char *target,
> >>> + bool has_target_id, const char *target_id,
> >>> bool has_format, const char *format,
> >>> bool has_mode, enum NewImageMode mode,
> >>> bool has_speed, int64_t speed,
> >>> @@ -1494,7 +1496,7 @@ void qmp_drive_backup(const char *device, const
> >>> char *target,
> >>> return;
> >>> }
> >>>
> >>> - target_bs = bdrv_new("");
> >>> + target_bs = bdrv_new(has_target_id ? target_id : "");
> >>
> >> This raises a new issue:
> >>
> >> Now that the target can be named, what happens when the user issues a
> >> monitor command, e.g. drive-del, block-resize, or drive-backup :)?
> >>
> >> We have a clumsy form of protection with bdrv_set_in_use(). It makes
> >> several monitor commands refuse with -EBUSY.
> >>
> >> Perhaps we should have a command permission set so it's possible to
> >> allow/deny specific commands.
> >>
> >
> > Yes, this makes me realize that ref count it not a solution to retire
> > bs->in_use, because we can't tell if drive-del or block-resize is safe
> > with only reference number. But I can't think of two situations to deny
> > different subsets of commands, shouldn't a general blocker, like in_use
> > does, be good enough?
>
> For example, right now nbd-server-add does not check bdrv_in_use. But
> shrinking a device that is exposed via NBD could be surprising to the
> NBD clients.
>
So it seems to me that both block job and nbd server have the same
restriction on device: don't resize, and notify on close. So my question
is if we implement bdrv_add_command_blocker(), do the callers still need to
distinguish what actions to block, or it's generally to block all the actions
those change the device parameter?
--
Fam
