Re: [Qemu-devel] [PATCH] add reopen to blockdev-transaction

[Paolo Bonzini <pbonzini@redhat.com>]

Il 01/03/2012 16:13, Federico Simoncelli ha scritto:
> Signed-off-by: Federico Simoncelli <fsimonce@redhat.com>
> ---
>  blockdev.c       |    8 ++++++++
>  qapi-schema.json |   12 ++++++++++++
>  qmp-commands.hx  |    6 +++++-
>  3 files changed, 25 insertions(+), 1 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 56da5c9..36fe07c 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -798,6 +798,14 @@ void qmp_blockdev_transaction(BlockdevActionList *dev_list,
>                                           dev_info->mirror->target);
>              break;
>  
> +        case BLOCKDEV_ACTION_KIND_REOPEN:
> +            device = dev_info->reopen->device;
> +            if (dev_info->format->has_format) {
> +                format = dev_info->reopen->format;
> +            }
> +            new_source = g_strdup(dev_info->reopen->target);
> +            break;
> +
>          default:
>              abort();
>          }
> diff --git a/qapi-schema.json b/qapi-schema.json
> index b33875d..17f7548 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1145,6 +1145,17 @@
>  { 'type': 'BlockdevMirror',
>    'data': { 'device': 'str', 'target': 'str', '*format': 'str',
>              '*reuse': 'bool' } }
> +##
> +# @BlockdevReopen
> +#
> +# @device: the name of the device to reopen.
> +#
> +# @target: the target of the new image.
> +#
> +# @format: #optional the format of the new image, default is 'qcow2'.
> +##
> +{ 'type': 'BlockdevReopen',
> +  'data': { 'device': 'str', 'target': 'str', '*format': 'str' } }
>  
>  ##
>  # @BlockdevAction
> @@ -1156,6 +1167,7 @@
>    'data': {
>         'snapshot': 'BlockdevSnapshot',
>         'mirror': 'BlockdevMirror',
> +       'reopen': 'BlockdevReopen',
>     } }
>  
>  ##
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 50ac5a0..317c448 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -720,7 +720,7 @@ Arguments:
>  
>  actions array:
>      - "type": the operation to perform.  The only supported
> -      values are "snapshot" and "mirror". (json-string)
> +      values are "snapshot", "mirror" and "reopen". (json-string)
>      - "data": a dictionary.  The contents depend on the value
>        of "type".  When "type" is "snapshot":
>        - "device": device name to snapshot (json-string)
> @@ -734,6 +734,10 @@ actions array:
>        - "format": format of new image (json-string, optional)
>        - "reuse": whether QEMU should look for an existing image file
>          (json-bool, optional, default false)
> +      When "type" is "reopen":
> +      - "device": device name to reopen (json-string)
> +      - "target": name of destination image file (json-string)
> +      - "format": format of new image (json-string, optional)
>  
>  Example:
>  

This keeps the whole "tower" of backing_hds from the old block device,
right? (including the blkmirror!)  I think that instead you need to
modify bdrv_append, probably by saving the action kind in
BlkTransactionStates.

But you can even keep from your first patch the drive-reopen command and
not make it atomic, that shouldn't be a problem.

Paolo