[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 4/5] snapshot: add virsh snapshot-list --tree



On Sat, Sep 24, 2011 at 06:30:05PM -0600, Eric Blake wrote:
> Reuse the tree listing of nodedev-list, coupled with the new helper
> function to efficiently grab snapshot parent names, to produce
> tree output for a snapshot hierarchy.  For example:
> 
> $ virsh snapshot-list dom --tree
> root1
>  |
>   +- sibling1
>   +- sibling2
>   |   |
>   |   +- grandchild
>   |
>   +- sibling3
> 
> root2
>  |
>   +- child
> 
> * tools/virsh.c (cmdSnapshotList): Add --tree.
> * tools/virsh.pod (snapshot-list): Document it.
> ---
>  tools/virsh.c   |   83 +++++++++++++++++++++++++++++++++++++++++++++---------
>  tools/virsh.pod |   14 +++++----
>  2 files changed, 77 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index 035b209..7e7b65d 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -12987,7 +12987,7 @@ vshGetSnapshotParent(vshControl *ctl, virDomainSnapshotPtr snapshot)
>          parent = virDomainSnapshotGetParent(snapshot, 0);
>          if (parent) {
>              /* API works, and virDomainSnapshotGetName will succeed */
> -            parent_name = vshStrdup(ctl, virDomainSnapshotGetName(snapshot));
> +            parent_name = vshStrdup(ctl, virDomainSnapshotGetName(parent));
>              goto cleanup;
>          }
>          if (last_error->code == VIR_ERR_NO_DOMAIN_SNAPSHOT) {

  That looks like a bug fix squashed in here instead of previous patch,
what am I missing ?

> @@ -13032,6 +13032,7 @@ static const vshCmdOptDef opts_snapshot_list[] = {
>      {"roots", VSH_OT_BOOL, 0, N_("list only snapshots without parents")},
>      {"metadata", VSH_OT_BOOL, 0,
>       N_("list only snapshots that have metadata that would prevent undefine")},
> +    {"tree", VSH_OT_BOOL, 0, N_("list snapshots in a tree")},
>      {NULL, 0, 0, NULL}
>  };
> 
> @@ -13057,6 +13058,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
>      time_t creation_time_t;
>      char timestr[100];
>      struct tm time_info;
> +    bool tree = vshCommandOptBool(cmd, "tree");
> 
>      if (vshCommandOptBool(cmd, "parent")) {
>          if (vshCommandOptBool(cmd, "roots")) {
> @@ -13064,8 +13066,18 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
>                       _("--parent and --roots are mutually exlusive"));
>              return false;
>          }
> +        if (tree) {
> +            vshError(ctl, "%s",
> +                     _("--parent and --tree are mutually exlusive"));
> +            return false;
> +        }
>          parent_filter = 1;
>      } else if (vshCommandOptBool(cmd, "roots")) {
> +        if (tree) {
> +            vshError(ctl, "%s",
> +                     _("--roots and --tree are mutually exlusive"));
> +            return false;
> +        }
>          flags |= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
>      }
> 
> @@ -13095,23 +13107,66 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
>      if (numsnaps < 0)
>          goto cleanup;
> 
> -    if (parent_filter > 0)
> -        vshPrintExtra(ctl, " %-20s %-25s %-15s %s",
> -                      _("Name"), _("Creation Time"), _("State"), _("Parent"));
> -    else
> -        vshPrintExtra(ctl, " %-20s %-25s %s",
> -                      _("Name"), _("Creation Time"), _("State"));
> -    vshPrintExtra(ctl, "\n\
> +    if (!tree) {
> +        if (parent_filter > 0)
> +            vshPrintExtra(ctl, " %-20s %-25s %-15s %s",
> +                          _("Name"), _("Creation Time"), _("State"),
> +                          _("Parent"));
> +        else
> +            vshPrintExtra(ctl, " %-20s %-25s %s",
> +                          _("Name"), _("Creation Time"), _("State"));
> +        vshPrintExtra(ctl, "\n\
>  ------------------------------------------------------------\n");
> +    }
> 
> -    if (numsnaps) {
> -        if (VIR_ALLOC_N(names, numsnaps) < 0)
> -            goto cleanup;
> +    if (!numsnaps) {
> +        ret = true;
> +        goto cleanup;
> +    }
> 
> -        actual = virDomainSnapshotListNames(dom, names, numsnaps, flags);
> -        if (actual < 0)
> -            goto cleanup;
> +    if (VIR_ALLOC_N(names, numsnaps) < 0)
> +        goto cleanup;
> +
> +    actual = virDomainSnapshotListNames(dom, names, numsnaps, flags);
> +    if (actual < 0)
> +        goto cleanup;
> 
> +    if (tree) {
> +        char indentBuf[INDENT_BUFLEN];
> +        char **parents = vshMalloc(ctl, sizeof(char *) * actual);
> +        for (i = 0; i < actual; i++) {
> +            /* free up memory from previous iterations of the loop */
> +            if (snapshot)
> +                virDomainSnapshotFree(snapshot);
> +            snapshot = virDomainSnapshotLookupByName(dom, names[i], 0);
> +            if (!snapshot) {
> +                while (--i >= 0)
> +                    VIR_FREE(parents[i]);
> +                VIR_FREE(parents);
> +                goto cleanup;
> +            }
> +            parents[i] = vshGetSnapshotParent(ctl, snapshot);
> +        }
> +        for (i = 0 ; i < actual ; i++) {
> +            memset(indentBuf, '\0', sizeof indentBuf);
> +            if (parents[i] == NULL)
> +                cmdNodeListDevicesPrint(ctl,
> +                                        names,
> +                                        parents,
> +                                        actual,
> +                                        i,
> +                                        i,
> +                                        0,
> +                                        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);
> 
>          for (i = 0; i < actual; i++) {
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index ddf7578..49aa42a 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -1934,15 +1934,17 @@ except that it does some error checking.
>  The editor used can be supplied by the C<$VISUAL> or C<$EDITOR> environment
>  variables, and defaults to C<vi>.
> 
> -=item B<snapshot-list> I<domain> [{I<--parent> | I<--roots>}] [I<--metadata>]
> +=item B<snapshot-list> I<domain> [{I<--parent> | I<--roots> | I<--tree>}]
> +[I<--metadata>]
> 
> -List all of the available snapshots for the given domain.
> +List all of the available snapshots for the given domain, defaulting
> +to show columns for the snapshot name, creation time, and domain state.
> 
>  If I<--parent> is specified, add a column to the output table giving
> -the name of the parent of each snapshot.
> -
> -If I<--roots> is specified, the list will be filtered to just snapshots
> -that have no parents; this option is not compatible with I<--parent>.
> +the name of the parent of each snapshot.  If I<--roots> is specified,
> +the list will be filtered to just snapshots that have no parents.
> +If I<--tree> is specified, the output will be in a tree format, listing
> +just snapshot names.  These three options are mutually exclusive.
> 
>  If I<--metadata> is specified, the list will be filtered to just
>  snapshots that involve libvirt metadata, and thus would prevent

  ACK, great improvement,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]