On Wed, Mar 13, 2019 at 10:13:00PM -0500, Eric Blake wrote: > Based on recent list questions about the proposed addition of > virDomainCheckpointCreateXML(REDEFINE), it is worth adding some > clarification to the existing snapshot redefine documentation that is > serving as the basis for checkpoints. > > Normal snapshot creation requires very few elements from the user XML > (libvirt can pick sane defaults for items that are omitted, and many > fields, including <domain> are documented as readonly output fields > ignored on input). But during REDEFINE, the API wants the complete XML > produced by an earlier virDomainSnapshotGetXMLDesc, which includes a > <domain> sub-element capturing the state a the time the snapshot was > first created. As the domain state has likely changed in the meantime, > omitting <domain> during REDEFINE is extremely risky (to the point > that virDomainSnapshotRevert() requires the use of FORCE flag to use > such a snapshot) - we only support it because of > backwards-compatibility to snapshots created before 0.9.5 started > capturing <domain>. When checkpoints are introduced, the <domain> > element will be mandatory in REDEFINE. > > Note that because <domain> can be potentially huge, we also are unable > to include a bulk listing or redefine of all <domainsnapshot>s for a > single domain in one XML dump (for example, a 1M <domain> XML * 16 > snapshots explodes into 16M in a bulk form, which gets difficult to > send over RPC). Perhaps we could add a flag to request that the > <domain> sub-element be omitted on output, but such output is no > longer suitable for sane REDEFINE input. I'm not understanding why the "bulk listing" comments are relevant here ? We don't have any bulk list support in current code do we ? > Signed-off-by: Eric Blake <eblake@xxxxxxxxxx> > --- > > Question: 0.9.5 is so long ago, is it worth reworking the qemu > snapshot code to declare that REDEFINE will no longer import a > snapshot definition that lacks a <domain> subelement? (Note: the vbox > driver also supports snapshots and the REDEFINE flag, but does not > output <domain> and thus should still allow it to be omitted on > redefine). The current API wording DOES have the disclaimer regarding > REDEFINE that "the hypervisor may validate that reverting to the > snapshot appears to be possible", which we can use as our escape > clause whereby the modern qemu driver is merely adding a validation > that <domain> must be present for the redefine to be viable, even > though it is at odds with our usual efforts to always parse XML that > used to work in earlier versions. Would enforcing it actually make the code any simpler ? If not, then I wouldn't bother changing it, especially because the vbox driver is not enforcing it - I think its undesirable to have different rules for each driver in this respect. > diff --git a/src/libvirt-domain-snapshot.c b/src/libvirt-domain-snapshot.c > index 947547627a..be9bf71af9 100644 > --- a/src/libvirt-domain-snapshot.c > +++ b/src/libvirt-domain-snapshot.c > @@ -117,15 +117,21 @@ virDomainSnapshotGetConnect(virDomainSnapshotPtr snapshot) > * > * If @flags includes VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE, then this > * is a request to reinstate snapshot metadata that was previously > - * discarded, rather than creating a new snapshot. This can be used > - * to recreate a snapshot hierarchy on a destination, then remove it > - * on the source, in order to allow migration (since migration > - * normally fails if snapshot metadata still remains on the source > - * machine). When redefining snapshot metadata, the current snapshot > - * will not be altered unless the VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT > - * flag is also present. It is an error to request the > - * VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT flag without > - * VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE. On some hypervisors, > + * captured from virDomainSnapshotGetXMLDesc() before removing that > + * metadata, rather than creating a new snapshot. This can be used to > + * recreate a snapshot hierarchy on a destination, then remove it on > + * the source, in order to allow migration (since migration normally > + * fails if snapshot metadata still remains on the source machine). > + * Note that while original creation can omit a number of elements > + * from @xmlDesc (and libvirt will supply sane defaults based on the > + * domain state at that point in time), a redefinition must supply > + * more elements (as the domain may have changed in the meantime, so > + * that libvirt no longer has a way to resupply correct > + * defaults). When redefining snapshot metadata, the domain's current > + * snapshot will not be altered unless the > + * VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT flag is also present. It is an > + * error to request the VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT flag > + * without VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE. On some hypervisors, > * redefining an existing snapshot can be used to alter host-specific > * portions of the domain XML to be used during revert (such as > * backing filenames associated with disk devices), but must not alter Reviewed-by: Daniel P. Berrangé <berrange@xxxxxxxxxx> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
