Re: [PATCH 2/2] snapshot: implement getparent for vbox

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/02/2011 02:50 AM, Matthias Bolte wrote:
2011/9/29 Eric Blake<eblake@xxxxxxxxxx>:
Again, built by copying from existing functions.

* src/vbox/vbox_tmpl.c (vboxDomainSnapshotGetParent): New function.
---

I can only compile-test this; I'm relying on someone with an actual
vbox setup to actually test it.

This patch needs some fixes, see below.

Also, I didn't see anything in
existing code that would efficiently implement
virDomainSnapshotNumChildren; there may an API that I'm not aware of,
but someone else will have to implement that API (Matthias?)

Is virDomainSnapshotNumChildren supposed to only return the number of
direct children, or all children (direct children, grandchildren, etc)
of a snapshot? In the later case they'll have to be counted
recursively, unfortunately.

Both, depending on the value of flags.

+
+    if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot->name)))
+        goto cleanup;

At this point you already have the snapshot you want.

+    rc = machine->vtbl->GetCurrentSnapshot(machine,&snap);
+    if (NS_FAILED(rc)) {
+        vboxError(VIR_ERR_INTERNAL_ERROR, "%s",
+                  _("could not get current snapshot"));
+        goto cleanup;
+    }

This block here gives you the current snapshot, that's not what you want.

That's why we do reviews :)


ACK with this diff applied:

Done.

--
Eric Blake   eblake@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]