1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH v4 0/8] bulk snapshot list/redefine (incremental backup saga)
  4. View patch

[libvirt] [PATCH v4 2/8] snapshot: Support topological virDomainSnapshotForEach()

Eric Blake posted 8 patches 6 years, 11 months ago
[libvirt] [PATCH v4 2/8] snapshot: Support topological virDomainSnapshotForEach()
Posted by Eric Blake 6 years, 11 months ago 
Previous patches added topological sorting only for existing public
API functions, but it turns out that it will also useful for an
upcoming API addition that wants to visit all snapshots.  Add a
parameter, and update all existing callers (none of which care
about ordering).

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 src/conf/snapshot_conf.h | 1 +
 src/conf/snapshot_conf.c | 6 +++++-
 src/qemu/qemu_domain.c   | 2 +-
 src/vz/vz_driver.c       | 3 ++-
 4 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 6d79dbb0da..ba9362c744 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -170,6 +170,7 @@ virDomainSnapshotObjPtr virDomainSnapshotFindByName(virDomainSnapshotObjListPtr
 void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,
                                     virDomainSnapshotObjPtr snapshot);
 int virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots,
+                             bool topological,
                              virHashIterator iter,
                              void *data);
 int virDomainSnapshotForEachChild(virDomainSnapshotObjPtr snapshot,
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index e2c91a5072..8235d7c526 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -1050,7 +1050,7 @@ virDomainSnapshotObjListFormat(virBufferPtr buf,
                               current_snapshot->def->name);
     virBufferAddLit(buf, ">\n");
     virBufferAdjustIndent(buf, 2);
-    if (virDomainSnapshotForEach(snapshots, virDomainSnapshotFormatOne,
+    if (virDomainSnapshotForEach(snapshots, false, virDomainSnapshotFormatOne,
                                  &data) < 0) {
         virBufferFreeAndReset(buf);
         return -1;
@@ -1293,9 +1293,13 @@ void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,

 int
 virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots,
+                         bool topological,
                          virHashIterator iter,
                          void *data)
 {
+    if (topological)
+        return virDomainSnapshotForEachDescendant(&snapshots->metaroot,
+                                                  iter, data);
     return virHashForEach(snapshots->objs, iter, data);
 }

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1659e88478..34f3669967 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8675,7 +8675,7 @@ qemuDomainSnapshotDiscardAllMetadata(virQEMUDriverPtr driver,
     rem.vm = vm;
     rem.metadata_only = true;
     rem.err = 0;
-    virDomainSnapshotForEach(vm->snapshots, qemuDomainSnapshotDiscardAll,
+    virDomainSnapshotForEach(vm->snapshots, false, qemuDomainSnapshotDiscardAll,
                              &rem);
     if (rem.current)
         vm->current_snapshot = NULL;
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 066d617524..2a0fc98f72 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -2170,7 +2170,8 @@ vzFindCurrentSnapshot(virDomainSnapshotObjListPtr snapshots)
 {
     virDomainSnapshotObjPtr current = NULL;

-    virDomainSnapshotForEach(snapshots, vzCurrentSnapshotIterator, &current);
+    virDomainSnapshotForEach(snapshots, false, vzCurrentSnapshotIterator,
+                             &current);
     return current;
 }

-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 2/8] snapshot: Support topological virDomainSnapshotForEach()
Posted by Ján Tomko 6 years, 11 months ago 
On Mon, Mar 11, 2019 at 09:38:33PM -0500, Eric Blake wrote:
>Previous patches added topological sorting only for existing public
>API functions, but it turns out that it will also useful for an
>upcoming API addition that wants to visit all snapshots.  Add a
>parameter, and update all existing callers (none of which care
>about ordering).
>
>Signed-off-by: Eric Blake <eblake@redhat.com>
>---
> src/conf/snapshot_conf.h | 1 +
> src/conf/snapshot_conf.c | 6 +++++-
> src/qemu/qemu_domain.c   | 2 +-
> src/vz/vz_driver.c       | 3 ++-
> 4 files changed, 9 insertions(+), 3 deletions(-)
>
>diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
>index 6d79dbb0da..ba9362c744 100644
>--- a/src/conf/snapshot_conf.h
>+++ b/src/conf/snapshot_conf.h
>@@ -170,6 +170,7 @@ virDomainSnapshotObjPtr virDomainSnapshotFindByName(virDomainSnapshotObjListPtr
> void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,
>                                     virDomainSnapshotObjPtr snapshot);
> int virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots,
>+                             bool topological,
>                              virHashIterator iter,
>                              void *data);
> int virDomainSnapshotForEachChild(virDomainSnapshotObjPtr snapshot,
>diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
>index e2c91a5072..8235d7c526 100644
>--- a/src/conf/snapshot_conf.c
>+++ b/src/conf/snapshot_conf.c
>@@ -1050,7 +1050,7 @@ virDomainSnapshotObjListFormat(virBufferPtr buf,
>                               current_snapshot->def->name);
>     virBufferAddLit(buf, ">\n");
>     virBufferAdjustIndent(buf, 2);
>-    if (virDomainSnapshotForEach(snapshots, virDomainSnapshotFormatOne,
>+    if (virDomainSnapshotForEach(snapshots, false, virDomainSnapshotFormatOne,
>                                  &data) < 0) {
>         virBufferFreeAndReset(buf);
>         return -1;
>@@ -1293,9 +1293,13 @@ void virDomainSnapshotObjListRemove(virDomainSnapshotObjListPtr snapshots,
>
> int
> virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots,
>+                         bool topological,

Rather than introducing a bool parameter (which shows up as
non-descriptive 'false' in the caller), I'd put it into the name of the
function:

virDomainSnapshotForEachTopological

which would be a thin wrapper for virDomainSnapshotForEachDescendant.
Alternatively, the one caller can call virDomainSnapshotForEachDescendant
with &snapshots->metaroot directly.

Jano

>                          virHashIterator iter,
>                          void *data)
> {
>+    if (topological)
>+        return virDomainSnapshotForEachDescendant(&snapshots->metaroot,
>+                                                  iter, data);
>     return virHashForEach(snapshots->objs, iter, data);
> }
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 2/8] snapshot: Support topological virDomainSnapshotForEach()
Posted by Eric Blake 6 years, 11 months ago 
On 3/12/19 8:00 AM, Ján Tomko wrote:
> On Mon, Mar 11, 2019 at 09:38:33PM -0500, Eric Blake wrote:
>> Previous patches added topological sorting only for existing public
>> API functions, but it turns out that it will also useful for an
>> upcoming API addition that wants to visit all snapshots.  Add a
>> parameter, and update all existing callers (none of which care
>> about ordering).
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---

>> int
>> virDomainSnapshotForEach(virDomainSnapshotObjListPtr snapshots,
>> +                         bool topological,
> 
> Rather than introducing a bool parameter (which shows up as
> non-descriptive 'false' in the caller), I'd put it into the name of the
> function:
> 
> virDomainSnapshotForEachTopological
> 
> which would be a thin wrapper for virDomainSnapshotForEachDescendant.
> Alternatively, the one caller can call virDomainSnapshotForEachDescendant
> with &snapshots->metaroot directly.
> 

I was debating whether any caller outside of snapshot_conf.c would need
a topological visit (knowing about the metaroot outside of
snapshot_conf.c is a leaky abstraction that I want to avoid). But you're
right that for THIS series, the only caller resides in snapshot_conf.c,
and therefore doing it directly is just as nice. I think that means I
can completely drop this patch and go with that approach instead.

By the way, did you have any comments on my other series adding
topological sorting to the various virDomainSnapshotList API calls? (I
may also end up squashing some of these changes into those, as that
series has not been pushed yet).

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.