Re: [libvirt] [PATCHv2 4/7] snapshot: virsh fallback for snapshot-list --from children

Iterating over one level of children requires parsing all snapshots and their parents; a bit of code shuffling makes it pretty easy to do this as well. * tools/virsh.c (cmdSnapshotList): Add another fallback. --- v2: fix compilation error, rebase around ctl flag tools/virsh.c | 48 ++++++++++++++++++++++++++++++++---------------- 1 files changed, 32 insertions(+), 16 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index e3bc364..b2523d3 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -13117,6 +13117,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) information needed, 1 for parent column */ int numsnaps; char **names = NULL; + char **parents = NULL; int actual = 0; int i; xmlDocPtr xml = NULL; @@ -13132,6 +13133,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) bool tree = vshCommandOptBool(cmd, "tree"); const char *from = NULL; virDomainSnapshotPtr start = NULL; + bool descendants = false; if (vshCommandOptString(cmd, "from", &from) < 0) { vshError(ctl, _("invalid from argument '%s'"), from); @@ -13175,27 +13177,29 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) goto cleanup; if (from) { + descendants = vshCommandOptBool(cmd, "descendants"); start = virDomainSnapshotLookupByName(dom, from, 0); if (!start) goto cleanup; - if (vshCommandOptBool(cmd, "descendants") || tree) { + if (descendants || tree) { flags |= VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS; } numsnaps = ctl->useSnapshotOld ? -1 : virDomainSnapshotNumChildren(start, flags); - /* XXX Is it worth emulating --from without --tree on older servers? */ - if (tree) { - if (numsnaps >= 0) { - numsnaps++; - } else if (ctl->useSnapshotOld || - last_error->code == VIR_ERR_NO_SUPPORT) { - /* We can emulate --tree --from. */ + if (numsnaps < 0) { + /* XXX also want to emulate --descendants without --tree */ + if ((!descendants || tree) && + (ctl->useSnapshotOld || + last_error->code == VIR_ERR_NO_SUPPORT)) { + /* We can emulate --from. */ virFreeError(last_error); last_error = NULL; ctl->useSnapshotOld = true; flags &= ~VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS; numsnaps = virDomainSnapshotNum(dom, flags); } + } else if (tree) { + numsnaps++; } } else { numsnaps = virDomainSnapshotNum(dom, flags); @@ -13259,9 +13263,11 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) if (actual < 0) goto cleanup; - if (tree) { - char indentBuf[INDENT_BUFLEN]; - char **parents = vshCalloc(ctl, sizeof(char *), actual); + if (!tree) + qsort(&names[0], actual, sizeof(char*), namesorter); + + if (tree || ctl->useSnapshotOld) { + parents = vshCalloc(ctl, sizeof(char *), actual); for (i = (from && !ctl->useSnapshotOld); i < actual; i++) { /* free up memory from previous iterations of the loop */ if (snapshot) @@ -13275,6 +13281,9 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) goto cleanup; } } + } + if (tree) { + char indentBuf[INDENT_BUFLEN]; for (i = 0 ; i < actual ; i++) { memset(indentBuf, '\0', sizeof indentBuf); if (ctl->useSnapshotOld ? STREQ(names[i], from) : !parents[i]) @@ -13288,16 +13297,19 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) 0, indentBuf); } - for (i = 0 ; i < actual ; i++) - VIR_FREE(parents[i]); - VIR_FREE(parents); ret = true; goto cleanup; } else { - qsort(&names[0], actual, sizeof(char*), namesorter); + if (ctl->useSnapshotOld && descendants) { + /* XXX emulate --descendants as well */ + goto cleanup; + } for (i = 0; i < actual; i++) { + if (ctl->useSnapshotOld && STRNEQ_NULLABLE(parents[i], from)) + continue; + /* free up memory from previous iterations of the loop */ VIR_FREE(parent); VIR_FREE(state); @@ -13362,9 +13374,13 @@ cleanup: xmlXPathFreeContext(ctxt); xmlFreeDoc(xml); VIR_FREE(doc); - for (i = 0; i < actual; i++) + for (i = 0; i < actual; i++) { VIR_FREE(names[i]); + if (parents) + VIR_FREE(parents[i]); + } VIR_FREE(names); + VIR_FREE(parents); if (dom) virDomainFree(dom);