Re: [PATCH 6/6] migration: support picking vmstate disk in QMP snapshot commands

[Eric Blake <eblake@redhat.com>]

On 7/2/20 12:57 PM, Daniel P. Berrangé wrote:
> This wires up support for a new "vmstate" parameter to the QMP commands
> for snapshots (savevm, loadvm). This parameter accepts block driver
> state node name.
> 
> One use case for this would be a VM using OVMF firmware where the
> variables store is the first snapshottable disk image. The vmstate
> snapshot usually wants to be stored in the primary root disk of the
> VM, not the firmeware varstore. Thus there needs to be a mechanism

firmware

> to override the default choice of disk.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---

> +++ b/qapi/migration.json
> @@ -1630,6 +1630,7 @@
>   # @tag: name of the snapshot to create. If it already
>   # exists it will be replaced.
>   # @exclude: list of block device node names to exclude
> +# @vmstate: block device node name to save vmstate to
>   #
>   # Note that execution of the VM will be paused during the time
>   # it takes to save the snapshot
> @@ -1641,6 +1642,7 @@
>   # -> { "execute": "savevm",
>   #      "data": {
>   #         "tag": "my-snap",
> +#         "vmstate": "disk0",
>   #         "exclude": ["pflash0-vars"]
>   #      }
>   #    }
> @@ -1650,6 +1652,7 @@
>   ##
>   { 'command': 'savevm',
>     'data': { 'tag': 'str',
> +            '*vmstate': 'str',
>               '*exclude': ['str'] } }

During save, the list of block devices is obvious: everything that is 
not excluded.  But,

>   
>   ##
> @@ -1659,6 +1662,7 @@
>   #
>   # @tag: name of the snapshot to load.
>   # @exclude: list of block device node names to exclude
> +# @vmstate: block device node name to load vmstate from
>   #
>   # Returns: nothing
>   #
> @@ -1667,6 +1671,7 @@
>   # -> { "execute": "loadvm",
>   #      "data": {
>   #         "tag": "my-snap",
> +#         "vmstate": "disk0",
>   #         "exclude": ["pflash0-vars"]
>   #      }
>   #    }
> @@ -1676,6 +1681,7 @@
>   ##
>   { 'command': 'loadvm',
>     'data': { 'tag': 'str',
> +            '*vmstate': 'str',
>               '*exclude': ['str'] } }

...now that we support exclusion during saving, or even without 
exclusion but when the user has performed hotplug/unplug operations in 
the meantime from when the snapshot was created, isn't load better off 
listing all devices which SHOULD be restored, rather than excluding 
devices that should NOT be restored?  (After all, libvirt knows which 
disks existed at the time of the snapshot, which may be different than 
the set of disks that exist now even though we are throwing out the 
state now to go back to the state at the time of the snapshot)

Then there's the question of symmetry: if load needs an explicit list of 
blocks to load from (rather than the set of blocks that are currently 
associated with the machine), should save also take its list by positive 
inclusion rather than negative exclusion?

And why does delvm not need to specify which block is the vmstate? 
delvm is in the same boat as loadvm - the set of blocks involved at the 
time of the snapshot creation may be different than the set of blocks 
currently associated with the guest at the time you run load/delvm.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org