qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v4 3/4] block: add a 'blockdev-snapshot' QMP command
Date: Fri, 18 Sep 2015 08:49:06 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0

On 09/14/2015 10:01 AM, 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 <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> Cc: Max Reitz <address@hidden>
> ---

> +++ b/qapi/block-core.json
> @@ -705,6 +705,21 @@
>              '*format': 'str', '*mode': 'NewImageMode' } }
>  
>  ##
> +# @BlockdevSnapshot
> +#
> +# @device: device or node name to generate the snapshot from.

I'm still wondering if 'node' is a better name than 'device' here.

> +#
> +# @snapshot: reference to the existing block device that will be used
> +#            for the snapshot. It must not have a current backing file
> +#            (this can be achieved by passing "backing": "" to
> +#            blockdev-add).

Possibly confusing terminology.

Let's consider: the act of creating a snapshot says to go from:

image1 [read-write]

to

image1 [read-only] <- image2 [read-write]

that is, image1 is now the snapshot of the state in time we executed the
command, and image2 is the delta from what happened since the snapshot.
 Therefore, image2 is NOT the snapshot.  Naming the command
'blockdev-snapshot' is fine, but I think we can have better names for
the arguments.

Better wording might be:

@device: device or node that will have a snapshot created

@overlay: reference to existing block device that will become the
overlay of device, as part of creating the snapshot. It must not have a
current backing file...

> +Example:
> +
> +-> { "execute": "blockdev-add",
> +                "arguments": { "options": { "driver": "qcow2",
> +                                            "node-name": "node1534",
> +                                            "file": { "driver": "file",
> +                                                      "filename": 
> "hd1.qcow2" },
> +                                            "backing": "" } } }
> +
> +<- { "return": {} }
> +
> +-> { "execute": "blockdev-snapshot", "arguments": { "device": "ide-hd0",
> +                                                    "snapshot": "node1534" } 
> }
> +<- { "return": {} }

The example would need tweaking if we rename members of the command, but
it definitely helps.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]