On Fri, Sep 30, 2011 at 05:09:26PM -0600, Eric Blake wrote:
> 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);
>
Okay, getting parents global to the function as well as associated
cleanup looks a bit simpler actually.
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list