Re: [libvirt] [PATCH 03/16] snapshot: Create virDomainMoment base class
On 3/20/19 2:57 PM, John Ferlan wrote:
On 3/20/19 1:40 AM, Eric Blake wrote:
> Upcoming patches want to add virDomainCheckpoint that behaves very
> similarly to virDomainSnapshot; the easiest way to share common code
> is to give both classes a common base class. Thanks to the accessor
> functions in the previous patch, we have very few changes required
> outside of datatypes.[ch]. The subclass does have to use a dummy
> field, though, to satisfy virobject's insistence on size
> differentiation for type safety.
When I first wrote this patch, I was envisioning a common function in
virdomainmomentobjlist.c that would take a virClassPtr() argument and
spit back an appropriate list of subclassed objects but with a function
signature using virDomainMomentPtr **list (which I could then down-cast
to virDomainSnapshotPtr ** or virDomainCheckpointPtr ** as appropriate).
However, in re-reading what I actually did in patch 14/16, I ended up
keeping virDomainListSnapshots() as-is rather than moving it into
virdomainmomentobjlist.c; it turned out that
virDomainMomentObjListNames() with its char **list parameter was all the
more generic code I needed. As such, I'm not seeing any use in my
current code that needs the virDomainMoment type, so I'll probably drop
patches 2 and 3 and rework 16/16 to have virDomainCheckpoint subclass
virObject rather than virDomainMoment.
The subclassing of virDomainMomentDef and virDomainMomentObjList (the
rest of this series) still makes sense, though, and I'm also looking at
converting those types to use virObject rather than being bare C structs
(but it would be followup patches, not holding up the rest of this
series from going in according to reviews).
>
> Note that virClassNew() supports a NULL dispose method for a class
> that has nothing to clean up, but VIR_CLASS_NEW has no easy way to
> register such a class without a #define hack.
>
> I promised my teenage daughter Evelyn that I'd give her credit for her
> contribution to this commit. I asked her "What would be a good name
> for a base class for DomainSnapshot and DomainCheckpoint". After
> explaining what a base class was (using the classic OOB Square and
> Circle inherit from Shape), she came up with "DomainMoment", which is
> way better than my initial thought of "DomainPointInTime" or
> "DomainPIT".
Maybe it was her way to ensure some sort of "mom" 'ent'ry got included
into libvirt ;-}
Of course, if I drop this commit from my series, I'll have to move the
historical note into 10/16 (as the first use of that prefix after this
patch).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 488 bytes)