2011/10/7 Eric Blake <ebl...@redhat.com>:
> Adding this for VBox was a bit harder than for ESX, but the same
> principles apply for starting the traversal at a known point
> rather than covering the entire hierarchy.
>
> * src/vbox/vbox_tmpl.c (vboxCountDescendants)
> (vboxDomainSnapshotNumChildren)
> (vboxDomainSnapshotListChildrenNames): New functions.
> ---
>
> Changes in v2: avoid leaking snapshot, and fix recursive children
> names to get through loop properly by transferring initial snapshot
> into loop then skipping that element while grabbing names.
>
> Still untested from my end, but hopefully does better.
>
> src/vbox/vbox_tmpl.c | 213
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 213 insertions(+), 0 deletions(-)
>
> diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
> index c74d2cf..84a4fca 100644
> --- a/src/vbox/vbox_tmpl.c
> +++ b/src/vbox/vbox_tmpl.c
> +static int
> +vboxDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot,
> + char **names,
> + int nameslen,
> + unsigned int flags)
> +{
> + virDomainPtr dom = snapshot->domain;
> + VBOX_OBJECT_CHECK(dom->conn, int, -1);
> + vboxIID iid = VBOX_IID_INITIALIZER;
> + IMachine *machine = NULL;
> + ISnapshot *snap = NULL;
> + nsresult rc;
> + ISnapshot **snapshots = NULL;
> + PRUint32 count = 0;
> + int i;
> + vboxArray children = VBOX_ARRAY_INITIALIZER;
> +
> + virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS |
> + VIR_DOMAIN_SNAPSHOT_LIST_METADATA, -1);
> +
> + 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;
> +
> + if (!nameslen || (flags & VIR_DOMAIN_SNAPSHOT_LIST_METADATA)) {
> + ret = 0;
> + goto cleanup;
> + }
> +
> + /* Over-allocates, but this is the easiest way to do things */
> + rc = machine->vtbl->GetSnapshotCount(machine, &count);
> + if (NS_FAILED(rc)) {
> + vboxError(VIR_ERR_INTERNAL_ERROR,
> + _("could not get snapshot count for domain %s"),
> + dom->name);
> + goto cleanup;
> + }
> +
> + if (VIR_ALLOC_N(snapshots, count) < 0) {
> + virReportOOMError();
> + goto cleanup;
> + }
> +
> + rc = vboxArrayGet(&children, snap, snap->vtbl->GetChildren);
> + if (NS_FAILED(rc)) {
> + vboxError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("could not get children snapshots"));
> + goto cleanup;
> + }
> +
> + if (flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS) {
> + int top = children.count;
> + int next;
> +
> + snapshots[0] = snap;
> + snap = NULL;
> + for (next = 0; next < count; next++) {
> + if (!snapshots[next])
> + break;
> + rc = vboxArrayGet(&children, snapshots[next],
> + snapshots[next]->vtbl->GetChildren);
> + if (NS_FAILED(rc)) {
> + vboxError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("could not get children snapshots"));
> + goto cleanup;
> + }
> + for (i = 0; i < children.count; i++) {
> + ISnapshot *child = children.items[i];
> + if (!child)
> + continue;
> + if (top == count) {
> + vboxError(VIR_ERR_INTERNAL_ERROR,
> + _("unexpected number of snapshots > %u"),
> count);
> + vboxArrayRelease(&children);
> + goto cleanup;
> + }
> + VBOX_ADDREF(child);
> + snapshots[top++] = child;
> + }
> + vboxArrayRelease(&children);
> + }
> + count = top - 1;
> + } else {
> + count = children.count;
> + }
> +
> + for (i = 0; i < nameslen; i++) {
> + PRUnichar *nameUtf16;
> + char *name;
> + int j = i + !!(flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS);
> +
> + if (i >= count)
> + break;
> +
> + rc = snapshots[j]->vtbl->GetName(snapshots[j], &nameUtf16);
Still a segfault here because snapshots[0] is NULL. Also I think this
logic is too complicated here. Are we really afraid of a stack
overflow because of a domain with many snapshots that we need to avoid
a recursive solution?
You already implemented vboxCountDescendants recursively, I'd suggest
to do the same here too.
I attached a patch to be applied on top of this one that does this and
works for me.
ACK with the attached patch applied.
--
Matthias Bolte
http://photron.blogspot.com
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index fd585c4..4169b02 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -6108,6 +6108,79 @@ cleanup:
}
static int
+vboxGetDescendantNames(vboxGlobalData *data, ISnapshot *snapshot, char **names,
+ int nameslen, bool recurse)
+{
+ bool success = false;
+ vboxArray children = VBOX_ARRAY_INITIALIZER;
+ nsresult rc;
+ int count = 0;
+ int result;
+ int i;
+
+ rc = vboxArrayGet(&children, snapshot, snapshot->vtbl->GetChildren);
+ if (NS_FAILED(rc)) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("could not get children snapshots"));
+ goto cleanup;
+ }
+
+ for (i = 0; i < children.count && count < nameslen; i++) {
+ PRUnichar *nameUtf16;
+ char *name;
+ ISnapshot *child = children.items[i];
+
+ rc = child->vtbl->GetName(child, &nameUtf16);
+ if (NS_FAILED(rc) || !nameUtf16) {
+ vboxError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("could not get snapshot name"));
+ goto cleanup;
+ }
+
+ VBOX_UTF16_TO_UTF8(nameUtf16, &name);
+ VBOX_UTF16_FREE(nameUtf16);
+ names[count] = strdup(name);
+ VBOX_UTF8_FREE(name);
+
+ if (names[count] == NULL) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ count++;
+
+ if (count >= nameslen) {
+ break;
+ }
+
+ if (recurse) {
+ result = vboxGetDescendantNames(data, child, names + count,
+ nameslen - count, true);
+
+ if (result < 0) {
+ goto cleanup;
+ }
+
+ count += result;
+ }
+ }
+
+ success = true;
+
+cleanup:
+ if (!success) {
+ for (i = 0; i < count; ++i) {
+ VIR_FREE(names[i]);
+ }
+
+ count = -1;
+ }
+
+ vboxArrayRelease(&children);
+ return count;
+}
+
+static int
vboxDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot,
char **names,
int nameslen,
@@ -6119,14 +6192,13 @@ vboxDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot,
IMachine *machine = NULL;
ISnapshot *snap = NULL;
nsresult rc;
- ISnapshot **snapshots = NULL;
- PRUint32 count = 0;
- int i;
- vboxArray children = VBOX_ARRAY_INITIALIZER;
+ bool recurse;
virCheckFlags(VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS |
VIR_DOMAIN_SNAPSHOT_LIST_METADATA, -1);
+ recurse = (flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS) != 0;
+
vboxIIDFromUUID(&iid, dom->uuid);
rc = VBOX_OBJECT_GET_MACHINE(iid.value, &machine);
if (NS_FAILED(rc)) {
@@ -6143,98 +6215,9 @@ vboxDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot,
goto cleanup;
}
- /* Over-allocates, but this is the easiest way to do things */
- rc = machine->vtbl->GetSnapshotCount(machine, &count);
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- _("could not get snapshot count for domain %s"),
- dom->name);
- goto cleanup;
- }
-
- if (VIR_ALLOC_N(snapshots, count) < 0) {
- virReportOOMError();
- goto cleanup;
- }
-
- rc = vboxArrayGet(&children, snap, snap->vtbl->GetChildren);
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("could not get children snapshots"));
- goto cleanup;
- }
-
- if (flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS) {
- int top = children.count;
- int next;
-
- snapshots[0] = snap;
- snap = NULL;
- for (next = 0; next < count; next++) {
- if (!snapshots[next])
- break;
- rc = vboxArrayGet(&children, snapshots[next],
- snapshots[next]->vtbl->GetChildren);
- if (NS_FAILED(rc)) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("could not get children snapshots"));
- goto cleanup;
- }
- for (i = 0; i < children.count; i++) {
- ISnapshot *child = children.items[i];
- if (!child)
- continue;
- if (top == count) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected number of snapshots > %u"), count);
- vboxArrayRelease(&children);
- goto cleanup;
- }
- VBOX_ADDREF(child);
- snapshots[top++] = child;
- }
- vboxArrayRelease(&children);
- }
- count = top - 1;
- } else {
- count = children.count;
- }
-
- for (i = 0; i < nameslen; i++) {
- PRUnichar *nameUtf16;
- char *name;
- int j = i + !!(flags & VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS);
-
- if (i >= count)
- break;
-
- rc = snapshots[j]->vtbl->GetName(snapshots[j], &nameUtf16);
- if (NS_FAILED(rc) || !nameUtf16) {
- vboxError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("could not get snapshot name"));
- goto cleanup;
- }
- VBOX_UTF16_TO_UTF8(nameUtf16, &name);
- VBOX_UTF16_FREE(nameUtf16);
- names[i] = strdup(name);
- VBOX_UTF8_FREE(name);
- if (!names[i]) {
- virReportOOMError();
- goto cleanup;
- }
- }
-
- if (count <= nameslen)
- ret = count;
- else
- ret = nameslen;
+ ret = vboxGetDescendantNames(data, snap, names, nameslen, recurse);
cleanup:
- if (count > 0) {
- for (i = 0; i < count; i++)
- VBOX_RELEASE(snapshots[i]);
- }
- VIR_FREE(snapshots);
VBOX_RELEASE(machine);
VBOX_RELEASE(snap);
vboxIIDUnalloc(&iid);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
