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

[Alberto Garcia]

On Fri 11 Sep 2015 07:58:11 PM CEST, Eric Blake wrote:

>> +void qmp_blockdev_snapshot(const char *device, const char *snapshot,
>> +                           Error **errp)
>> +{
>> +    BlockdevSnapshot snapshot_data = {
>> +        .device = (char *) device,
>> +        .snapshot = (char *) snapshot
>> +    };
>
> Hmm. Sounds like you'd love to use my pending 'box':true qapi glue to
> make this function have the saner signature of
>
> void qmp_blockdev_snapshot(BlockdevSnapshot *arg, Error **errp);
>
> rather than having to rebuild the struct yourself (with an annoying
> cast to lose const) :)
> https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02599.html

That's cool! We can simplify this later as you suggest.

>> +Create a snapshot, by installing 'device' as the backing image of
>> +'snapshot'. Additionally, if 'device' is associated with a block
>> +device, the block device changes to using 'snapshot' as its new
>> +active image.
>
> Still didn't answer the question from the earlier review of whether
> the blockdev-snapshot-sync behavior of specifying the node name of an
> active layer in order to not pivot the block device to the snapshot
> still makes sense to support in the blockdev-snapshot case.

Sorry if I overlooked the explanation, but I still didn't get the use
case for this.

And particularly for this command, that doesn't create any image but
simply uses a previously-opened one.

What would 'pivot': false do ? As far as I can see, specifying the node
name of an active layer in blockdev-snapshot-sync seems to pivot the
block device to the snapshot.

{ "execute": "blockdev-snapshot-sync",
             "arguments": { "node-name": "hd0",
                            "snapshot-file": "hd1.qcow2",
                            "snapshot-node-name": "hd1" } }

(qemu) info block
virtio0 (hd1): hd1.qcow2 (qcow2)
    Cache mode:       writeback
    Backing file:     hd0.qcow2 (chain depth: 1)

Berto