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
