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

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



On 10/02/2011 02:50 AM, Matthias Bolte wrote:
2011/9/29 Eric Blake<eblake redhat com>:
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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


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