Re: [Qemu-devel] [PATCH 1/2] block/snapshot: Restrict set of snapshot nodes

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1554 bytes --]

On 9/17/19 6:04 AM, Kevin Wolf wrote:
> Nodes involved in internal snapshots were those that were returned by
> bdrv_next(), inserted and not read-only. bdrv_next() in turn returns all
> nodes that are either the root node of a BlockBackend or monitor-owned
> nodes.
> 
> With the typical -drive use, this worked well enough. However, in the
> typical -blockdev case, the user defines one node per option, making all
> nodes monitor-owned nodes. This includes protocol nodes etc. which often
> are not snapshottable, so "savevm" only returns an error.
> 
> Change the conditions so that internal snapshot still include all nodes
> that have a BlockBackend attached (we definitely want to snapshot
> anything attached to a guest device and probably also the built-in NBD
> server; snapshotting block job BlockBackends is more of an accident, but
> a preexisting one), but other monitor-owned nodes are only included if
> they have no parents.
> 
> This makes internal snapshots usable again with typical -blockdev
> configurations.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/snapshot.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)

Makes sense.  But you'll want Peter's review that it actually passes
testing with libvirt, rather than relying solely on my:

Reviewed-by: Eric Blake <eblake@redhat.com>

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


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