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

[Daniel Veillard]

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 at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/