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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
