[libvirt] [PATCH 6/8] backup: Add support for filtering based on current moment

[Eric Blake]

On 7/8/19 3:27 AM, Peter Krempa wrote:
> On Fri, Jul 05, 2019 at 23:37:33 -0500, Eric Blake wrote:
>> Right now, the snapshot API permits at most one current snapshot, and
>> includes specific API for getting at that snapshot
>> (virDomainHasCurrentSnapshot, virDomainSnapshotCurrent,
>> virDomainSnapshotIsCurrent).  However, with upcoming checkpoints, it
>> is conceivable that a hypervisor could mark multiple checkpoints as
> 
> The same happens to snapshots too. Since you can create an "incomplete"
> snapshot by unselecting some disks and then do a complement snapshot of
> all the remaining disks then you get to the same situation.
> 
> For more fun you can create a snapshot which partially overlaps and then
> neither of the "current" snapshots makes sense any more.

For now, I'm deferring this patch to later (or maybe even dropping it),
based on the v9 review comments about using the one-and-only leaf node
snapshot as the default parent at least for the short term where we
interlock against having snapshots. If we find that changing a
checkpoint current node makes sense with snapshots, we can always add
additional filtering at that time, but right now is premature.  While it
is more churn for me, I agree with the goal of stripping down the
initial implementation to the bare minimum needed, and this does not
qualify in that regards.

>> @@ -78,8 +78,10 @@ typedef enum {
>>      VIR_DOMAIN_MOMENT_LIST_TOPOLOGICAL = (1 << 1),
>>      VIR_DOMAIN_MOMENT_LIST_LEAVES      = (1 << 2),
>>      VIR_DOMAIN_MOMENT_LIST_NO_LEAVES   = (1 << 3),
>> -    VIR_DOMAIN_MOMENT_LIST_METADATA    = (1 << 4),
>> -    VIR_DOMAIN_MOMENT_LIST_NO_METADATA = (1 << 5),
>> +    VIR_DOMAIN_MOMENT_LIST_CURRENT     = (1 << 4),
>> +    VIR_DOMAIN_MOMENT_LIST_NO_CURRENT  = (1 << 5),
>> +    VIR_DOMAIN_MOMENT_LIST_METADATA    = (1 << 6),
>> +    VIR_DOMAIN_MOMENT_LIST_NO_METADATA = (1 << 7),
> 
> Why the reorder?

Because if we like the idea of filtering on a current node better than
the idea of no_metadata, then patches that add current filtering in v9
use bit 6 right away, while bits for metadata filtering of checkpoints
sit unused, and it looks odd to have a gap in the numbering. The
snapshot code already has to do mapping between internal bits from the
public API flags, but my initial goal was to have the internal bits
match the checkpoint public bits.  That said, since I'm now working on
stripping the notion of current filtering out of checkpoints before the
initial implementation, the question is moot until (or if) this patch
ever gets revived.

> 
> 
>>  } virDomainMomentFilters;
>>
>>  #define VIR_DOMAIN_MOMENT_FILTERS_METADATA \
> 
> ACK

Thanks for the review, even if later patches changed our minds.

-- 
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/20190710/1853781a/attachment-0001.sig>