[libvirt] [PATCH v3 13/18] snapshot: Add VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE_LIST flag

[Eric Blake]

On 3/7/19 10:13 AM, Ján Tomko wrote:
> On Mon, Mar 04, 2019 at 09:34:40PM -0600, Eric Blake wrote:
>> Continue the work of the previous patch in making it possible
>> to copy the state of a transient domain with snapshots from one
>> host to another, by allowing the destination to perform bulk
>> redefines.  Note that the destination still has to do separate
>> calls for creating/defining the domain first, and then redefining
>> the snapshots (that is, there is intentional asymmetry between
>> dumping the list in virDomainGetXMLDesc() but redefining it via
>> virDomainSnapshotCreateXML()), but this is better than the
>> previous state of having to make multiple REDEFINE calls.
> 
> What is the intention behind the assymetry?

virDomainSnapshotGetXMLDesc won't work - you can't pass in NULL because
that function requires a snapshot (in order to get at the virDomain and
virConnection) to even make the call.

On the flip side, I did NOT want virDomainDefine/virDomainCreate to take
the <snapshots> argument, even with the presence of a flag, because
there are scenarios where you want the domain defined before you add in
the snapshots; virDomainSnapshotCreateXML with new flag fit that purpose
well.

I could, however, add a new API instead of a new flag overloading to the
existing API.  Naming is hard, maybe:

virDomainGetSnapshotsXMLDesc

since it would be a new virDomain function, but returns the new
<snapshots> XML element.

> 
> It feels odd to request the list by a flag to virDomainGetXMLDesc
> (because we're not going to reuse the whole <domain>, just the
> <snapshots> sub-element here). Having a counterpart to the API doing the
> redefine would only mean two calls (GetXMLDesc, GetSnapshots) instead of
> getting every snapshot separately.

One call (virDomainGetXMLDesc with flag) is even better than two
(virDomainGetXMLDesc, virDomainGetSnapshotsXMLDesc), but a new API has
the benefit of not suffering from the recently-fixed bug about unknown
new flags being rejected by buggy old servers.

> 
> Also, virDomainSnapshotCreateXML is designed for a single snapshot,
> using a flag to turn it into a different API
> ('virDomainSnapshotsCreateXML'?
> 'virDomainSnapshotsRedefine'?) leads to strangeness like returning
> a single snapshot while making no guarantees on which one it is
> or a repetition of this pattern:
> if (flags & REDEFINE_LIST) {
>    /* ... */
>    goto cleanup; /* <- no fallthrough here */
> }

If I add a new API for getting the XML, then it is not a stretch to
require a new API for redefining all snapshots at once. And now that
I've typed this up, the suggestion for a separate API is starting to be
more appealing.

Looks like I'll be posting a v4 shortly.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190307/392d0c2c/attachment-0001.sig>