3:50 a.m.
2011/9/29 Eric Blake <eblake(a)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.
src/vbox/vbox_tmpl.c | 71
++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index b483d0f..3d5f4ae 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -6046,6 +6046,76 @@ cleanup:
}
static virDomainSnapshotPtr
+vboxDomainSnapshotGetParent(virDomainSnapshotPtr snapshot,
+ unsigned int flags)
+{
+ virDomainPtr dom = snapshot->domain;
+ VBOX_OBJECT_CHECK(dom->conn, virDomainSnapshotPtr, NULL);
+ vboxIID iid = VBOX_IID_INITIALIZER;
+ IMachine *machine = NULL;
+ ISnapshot *snap = NULL;
+ ISnapshot *parent = NULL;
+ PRUnichar *nameUtf16 = NULL;
+ char *name = NULL;
+ nsresult rc;
+
+ virCheckFlags(0, NULL);
+
+ vboxIIDFromUUID(&iid, dom->uuid);
+ rc = VBOX_OBJECT_GET_MACHINE(iid.value, &machine);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_NO_DOMAIN, "%s",
+ _("no domain with matching UUID"));
+ goto cleanup;
+ }
+
+ 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.
+ rc = snap->vtbl->GetParent(snap, &parent);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("could not get parent of snapshot %s"),
+ snapshot->name);
+ goto cleanup;
+ }
+ if (!parent) {
+ vboxError(VIR_ERR_NO_DOMAIN_SNAPSHOT,
+ _("snapshot '%s' does not have a parent"),
+ snapshot->name);
+ goto cleanup;
+ }
+
+ rc = parent->vtbl->GetName(parent, &nameUtf16);
+ if (NS_FAILED(rc) || !nameUtf16) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ _("could not get name of parent of snapshot %s"),
+ snapshot->name);
+ goto cleanup;
+ }
+ VBOX_UTF16_TO_UTF8(nameUtf16, &name);
+ VBOX_UTF16_FREE(nameUtf16);
Because VBOX_UTF16_FREE doesn't set the pointer to NULL, calling it
twice on the same pointer isn't safe.
+
+ ret = virGetDomainSnapshot(dom, name);
+
+cleanup:
+ VBOX_UTF8_FREE(name);
+ VBOX_UTF16_FREE(nameUtf16);
And this second call segfaults in the success path.
+ VBOX_RELEASE(snap);
+ VBOX_RELEASE(parent);
+ VBOX_RELEASE(machine);
+ vboxIIDUnalloc(&iid);
+ return ret;
+}
ACK with this diff applied:
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 3d5f4ae..2eb23fb 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -6072,13 +6072,6 @@
vboxDomainSnapshotGetParent(virDomainSnapshotPtr snapshot,
if (!(snap = vboxDomainSnapshotGet(data, dom, machine, snapshot->name)))
goto cleanup;
- rc = machine->vtbl->GetCurrentSnapshot(machine, &snap);
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("could not get current snapshot"));
- goto cleanup;
- }
-
rc = snap->vtbl->GetParent(snap, &parent);
if (NS_FAILED(rc)) {
vboxError(VIR_ERR_INTERNAL_ERROR,
@@ -6101,7 +6094,10 @@
vboxDomainSnapshotGetParent(virDomainSnapshotPtr snapshot,
goto cleanup;
}
VBOX_UTF16_TO_UTF8(nameUtf16, &name);
- VBOX_UTF16_FREE(nameUtf16);
+ if (!name) {
+ virReportOOMError();
+ goto cleanup;
+ }
ret = virGetDomainSnapshot(dom, name);
--
Matthias Bolte
http://photron.blogspot.com