[libvirt] [PATCH v3 14/18] virsh: Expose bulk snapshot dumpxml/redefine

[John Ferlan]

On 3/4/19 10:34 PM, Eric Blake wrote:
> Add flags to the 'dumpxml' and 'snapshot-create' commands to pass
> the newly-added bulk snapshot flags through.
> 
> For command-line convenience, I intentionally made --redefine-list
> imply --redefine, even though the counterpart C flags are distinct
> (and you get an error if you pass _REDEFINE_LIST without _REDEFINE).
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  tools/virsh-domain.c   |  7 +++++++
>  tools/virsh-snapshot.c | 14 ++++++++++++++
>  tools/virsh.pod        | 18 ++++++++++++++----
>  3 files changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 5699018dcc..78854b1e0a 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -10055,6 +10055,10 @@ static const vshCmdOptDef opts_dumpxml[] = {
>       .type = VSH_OT_BOOL,
>       .help = N_("provide XML suitable for migrations")
>      },
> +    {.name = "snapshots",
> +     .type = VSH_OT_BOOL,
> +     .help = N_("include all domain snapshots in XML dump"),
> +    },
>      {.name = NULL}
>  };
> 
> @@ -10069,6 +10073,7 @@ cmdDumpXML(vshControl *ctl, const vshCmd *cmd)
>      bool secure = vshCommandOptBool(cmd, "security-info");
>      bool update = vshCommandOptBool(cmd, "update-cpu");
>      bool migratable = vshCommandOptBool(cmd, "migratable");
> +    bool snapshots = vshCommandOptBool(cmd, "snapshots");
> 
>      if (inactive)
>          flags |= VIR_DOMAIN_XML_INACTIVE;
> @@ -10078,6 +10083,8 @@ cmdDumpXML(vshControl *ctl, const vshCmd *cmd)
>          flags |= VIR_DOMAIN_XML_UPDATE_CPU;
>      if (migratable)
>          flags |= VIR_DOMAIN_XML_MIGRATABLE;
> +    if (snapshots)
> +        flags |= VIR_DOMAIN_XML_SNAPSHOTS;
> 
>      if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
>          return false;
> diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
> index e38ebb1f28..cbb7d744f9 100644
> --- a/tools/virsh-snapshot.c
> +++ b/tools/virsh-snapshot.c
> @@ -80,6 +80,13 @@ virshSnapshotCreate(vshControl *ctl, virDomainPtr dom, const char *buffer,
>          goto cleanup;
>      }
> 
> +    if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE_LIST) {
> +        vshPrintExtra(ctl, "%s",
> +                      _("Domain snapshot list imported successfully"));
> +        ret = true;
> +        goto cleanup;
> +    }
> +
>      name = virDomainSnapshotGetName(snapshot);
>      if (!name) {
>          vshError(ctl, "%s", _("Could not get snapshot name"));
> @@ -122,6 +129,10 @@ static const vshCmdOptDef opts_snapshot_create[] = {
>       .help = N_("redefine metadata for existing snapshot")
>      },
>      VIRSH_COMMON_OPT_CURRENT(N_("with redefine, set current snapshot")),
> +    {.name = "redefine-list",
> +     .type = VSH_OT_BOOL,
> +     .help = N_("bulk define a set of snapshots, implies --redefine"),
> +    },
>      {.name = "no-metadata",
>       .type = VSH_OT_BOOL,
>       .help = N_("take snapshot but create no metadata")
> @@ -177,6 +188,9 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
>          flags |= VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC;
>      if (vshCommandOptBool(cmd, "live"))
>          flags |= VIR_DOMAIN_SNAPSHOT_CREATE_LIVE;
> +    if (vshCommandOptBool(cmd, "redefine-list"))
> +        flags |= VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE |
> +            VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE_LIST;
> 
>      if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
>          goto cleanup;
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 8e18b30f29..ea51584d3e 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -1647,7 +1647,7 @@ is required in order to produce valid ELF file which can be later processed by
>  the crash utility.
> 
>  =item B<dumpxml> I<domain> [I<--inactive>] [I<--security-info>]
> -[I<--update-cpu>] [I<--migratable>]
> +[I<--update-cpu>] [I<--migratable>] [I<--snapshots>]
> 
>  Output the domain information as an XML dump to stdout, this format can be used
>  by the B<create> command. Additional options affecting the XML dump may be
> @@ -1660,6 +1660,9 @@ migrations, i.e., compatible with older libvirt releases and possibly amended
>  with internal run-time options. This option may automatically enable other
>  options (I<--update-cpu>, I<--security-info>, ...) as necessary.
> 
> +Using I<--snapshots> will expand the output to also include information on
> +all domain snapshots, for servers that understand the flag.

s/, for/ for/

s/understand/support/

(remove comma)...
> +
>  =item B<edit> I<domain>
> 
>  Edit the XML configuration file for a domain, which will affect the
> @@ -4544,8 +4547,9 @@ used to represent properties of snapshots.
> 
>  =over 4
> 
> -=item B<snapshot-create> I<domain> [I<xmlfile>] {[I<--redefine> [I<--current>]]
> -| [I<--no-metadata>] [I<--halt>] [I<--disk-only>] [I<--reuse-external>]
> +=item B<snapshot-create> I<domain> [I<xmlfile>] {[I<--redefine>
> +{[I<--current>] | [I<--redefine-list>]}] | [I<--no-metadata>]
> +[I<--halt>] [I<--disk-only>] [I<--reuse-external>]
>  [I<--quiesce>] [I<--atomic>] [I<--live>]}
> 
>  Create a snapshot for domain I<domain> with the properties specified in
> @@ -4575,7 +4579,12 @@ the same name and UUID, or to make slight alterations in the snapshot
>  metadata (such as host-specific aspects of the domain XML embedded in
>  the snapshot).  When this flag is supplied, the I<xmlfile> argument
>  is mandatory, and the domain's current snapshot will not be altered
> -unless the I<--current> flag is also given.
> +unless the I<--current> flag is also given.  If I<--redefine-list> is

New paragraph makes it easier to read...

> +specified, I<--redefine> is implied, I<--current> is rejected, and
> +the XML changes from being a single <domainsnapshot> to instead being
> +a <snapshots> element describing a list of snapshots. List form only

? s/list of snapshots/list of all snapshots/ ?

> +works if the domain has no currently-defined snapshot metadata, and

s/, and/ and/

> + can be obtained as a subset of I<dumpxml --snapshots> output.

s/ can/can

(extra space)

Reviewed-by: John Ferlan <jferlan at redhat.com>

John

> 
>  If I<--no-metadata> is specified, then the snapshot data is created,
>  but any metadata is immediately discarded (that is, libvirt does not
> @@ -4779,6 +4788,7 @@ files for disk images or memory state.
>  Output the snapshot XML for the domain's snapshot named I<snapshot>.
>  Using I<--security-info> will also include security sensitive information.
>  Use B<snapshot-current> to easily access the XML of the current snapshot.
> +To grab the XML for all snapshots at once, use B<dumpxml --snapshots>.
> 
>  =item B<snapshot-parent> I<domain> {I<snapshot> | I<--current>}
>