Re: [Qemu-devel] [PATCH v6 3/4] block: add a 'blockdev-snapshot' QMP command

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3191 bytes --]

On 22.09.2015 15:28, Alberto Garcia wrote:
> One of the limitations of the 'blockdev-snapshot-sync' command is that
> it does not allow passing BlockdevOptions to the newly created
> snapshots, so they are always opened using the default values.
> 
> Extending the command to allow passing options is not a practical
> solution because there is overlap between those options and some of
> the existing parameters of the command.
> 
> This patch introduces a new 'blockdev-snapshot' command with a simpler
> interface: it just takes two references to existing block devices that
> will be used as the source and target for the snapshot.
> 
> Since the main difference between the two commands is that one of them
> creates and opens the target image, while the other uses an already
> opened one, the bulk of the implementation is shared.
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Max Reitz <mreitz@redhat.com>
> ---
>  blockdev.c           | 163 ++++++++++++++++++++++++++++++++-------------------
>  qapi-schema.json     |   2 +
>  qapi/block-core.json |  28 +++++++++
>  qmp-commands.hx      |  38 ++++++++++++
>  4 files changed, 171 insertions(+), 60 deletions(-)
> 

[snip]

> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 495670b..e5bd0e0 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -1454,6 +1454,44 @@ Example:
>  EQMP
>  
>      {
> +        .name       = "blockdev-snapshot",
> +        .args_type  = "node:s,overlay:s",
> +        .mhandler.cmd_new = qmp_marshal_input_blockdev_snapshot,

As of 7fad30f06eb6aa57aaa8f3d264288f24ae7646f0, this needs to be
qmp_marshal_blockdev_snapshot.

> +    },
> +
> +SQMP
> +blockdev-snapshot
> +-----------------
> +Since 2.5
> +
> +Create a snapshot, by installing 'node' as the backing image of
> +'overlay'. Additionally, if 'node' is associated with a block
> +device, the block device changes to using 'overlay' as its new active
> +image.
> +
> +Arguments:
> +
> +- "node": device that will have a snapshot created (json-string)
> +- "overlay": device that will have 'node' as its backing image (json-string)
> +
> +Example:
> +
> +-> { "execute": "blockdev-add",
> +                "arguments": { "options": { "driver": "qcow2",
> +                                            "node-name": "node1534",
> +                                            "file": { "driver": "file",
> +                                                      "filename": "hd1.qcow2" },
> +                                            "backing": "" } } }
> +
> +<- { "return": {} }
> +
> +-> { "execute": "blockdev-snapshot", "arguments": { "node": "ide-hd0",
> +                                                    "overlay": "node1534" } }
> +<- { "return": {} }
> +
> +EQMP
> +
> +    {
>          .name       = "blockdev-snapshot-internal-sync",
>          .args_type  = "device:B,name:s",
>          .mhandler.cmd_new = qmp_marshal_input_blockdev_snapshot_internal_sync,

Consequently, this context needs to be fixed up, too.

With that changed:

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]