Re: [libvirt] [PATCH 1/2] snapshot: Permit redefine of offline external snapshot

Due to historical back-compat, 'virsh snapshot-create-as' favors internal snapshots (but can't be used on domains with raw storage), while 'virsh snapshot-create-as --disk-only) favors external

snapshots. What's more, snapshots created with --disk-only while the domain was running are marked as snapshot state 'disk-snapshot', while snapshots created while the domain was offline are marked as snapshot state 'shutdown' (a 'disk-snapshot' image might not be quiescent, while a 'shutdown' snapshot always is). But this leads to some interesting problems: if we create a --disk-only snapshot of an offline guest, and then immediately try to 'virsh snapshot-create --redefine' using the resulting XML to overwrite the existing snapashot in place, things silently succeed, but 'virsh snapshot-create --redefine --disk-only' fails because the snapshot state is not 'disk-only'. Worse, if we delete the snapshot metadata first and then try to recreate things, omitting --disk-only fails because the verification code wants to force the default of an internal snapshot (which doesn't work with raw disks), and using --disk-only fails because the verification code doesn't see the 'disk-only' state in the snapshot xml - making it impossible to recreate the snapshot metadata. Ideally, the presence or absence of the --disk-only flag, and the presence or absence of an existing snapshot being overwritten, shouldn't matter; if the XML is valid for one situation, it should always be valid to redefine the metadata for that snapshot. Fix things by uniformly declaring that a redefined snapshot in the 'shutdown' state can permit a disk-only external snapshot (we got it right in only one out of three places in the function). See also https://bugzilla.redhat.com/1680304; this fixes the domain-agnostic problems mentioned there, but another patch is needed to fix further oddities with the qemu driver. I did not check for sure, but this has probably been broken even prior to when commit 670e86bf (1.1.4) factored redefine prep out of qemu code into the common snapshot_conf code.

Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/conf/snapshot_conf.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)