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@redhat.com>
---
 tools/virsh-domain.c   |  7 +++++++
 tools/virsh-snapshot.c | 14 ++++++++++++++
 2 files changed, 21 insertions(+)

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 6cadb2b0d6..a58731c46e 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;
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 2/23/19 4:24 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@redhat.com>
> ---
>  tools/virsh-domain.c   |  7 +++++++
>  tools/virsh-snapshot.c | 14 ++++++++++++++
>  2 files changed, 21 insertions(+)
> 

Need to update virsh.pod as well to describe new arguments. Do you think
that a separation of the dumpxml and the parse/create into order
pertinent patches would be better? That is the --snapshots for dumpxml
after patch7 (which probably could swap places w/ patch6) so that the
dumpxml and parse/create functionality is all in logical order. It's not
that important.

John

[...]

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 2/27/19 10:54 AM, John Ferlan wrote:
> 
> 
> On 2/23/19 4:24 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@redhat.com>
>> ---
>>  tools/virsh-domain.c   |  7 +++++++
>>  tools/virsh-snapshot.c | 14 ++++++++++++++
>>  2 files changed, 21 insertions(+)
>>
> 
> Need to update virsh.pod as well to describe new arguments. Do you think
> that a separation of the dumpxml and the parse/create into order
> pertinent patches would be better? That is the --snapshots for dumpxml
> after patch7 (which probably could swap places w/ patch6) so that the
> dumpxml and parse/create functionality is all in logical order. It's not
> that important.

Yeah, I waffled on that. v1 was definitely more of a "do all dumpxml
changes, then do all redefine changes" breakup. But I intentionally
refactored this way (do all API changes, then all virsh changes),
especially since the virsh changes are so small that it's easy to review
both at once.  Depending on how you feel about the qemu changes, I'm
still open to the idea of doing dumpxml separate from redefine in each
of the drivers, if it makes it easier to backport one but not the other.
Furthermore, your comments against v4 of the incremental backup sparked
my idea of possibly using VIR_DOMAIN_XML_SNAPSHOTS internally (so that
domains have just ONE xml file on disk, rather than the main domain xml
+ one xml per snapshot), and that may have fallout on making the dumpxml
portion reasonable to backport in isolation.

I'll have to see how things play out in the rest of the series before
deciding if splitting this one is worth it.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list