Introduce a bunch of new virsh commands for managing checkpoints
in isolation. More commands are needed for performing incremental
backups, but these commands were easy to implement by modeling
heavily after virsh-snapshot.c (no need for checkpoint-revert,
and checkpoint-list was a lot easier since we don't have to cater
to older libvirt API).

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 tools/virsh-checkpoint.h     |   29 +
 tools/virsh-completer.h      |    4 +
 tools/virsh-util.h           |    3 +
 tools/virsh.h                |    1 +
 po/POTFILES                  |    1 +
 tools/Makefile.am            |    3 +-
 tools/virsh-checkpoint.c     | 1370 ++++++++++++++++++++++++++++++++++
 tools/virsh-completer.c      |   53 +-
 tools/virsh-domain-monitor.c |   25 +-
 tools/virsh-domain.c         |   15 +
 tools/virsh-util.c           |   11 +
 tools/virsh.c                |    2 +
 tools/virsh.pod              |  238 +++++-
 13 files changed, 1745 insertions(+), 10 deletions(-)
 create mode 100644 tools/virsh-checkpoint.h
 create mode 100644 tools/virsh-checkpoint.c

diff --git a/tools/virsh-checkpoint.h b/tools/virsh-checkpoint.h
new file mode 100644
index 0000000000..707defa1c5
--- /dev/null
+++ b/tools/virsh-checkpoint.h
@@ -0,0 +1,29 @@
+/*
+ * virsh-checkpoint.h: Commands to manage domain checkpoints
+ *
+ * Copyright (C) 2005-2018 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef LIBVIRT_VIRSH_CHECKPOINT_H
+# define LIBVIRT_VIRSH_CHECKPOINT_H
+
+# include "virsh.h"
+
+extern const vshCmdDef checkpointCmds[];
+
+#endif /* LIBVIRT_VIRSH_CHECKPOINT_H */
diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h
index 2e2e1edafb..429e695a55 100644
--- a/tools/virsh-completer.h
+++ b/tools/virsh-completer.h
@@ -75,6 +75,10 @@ char ** virshSecretUUIDCompleter(vshControl *ctl,
                                  const vshCmd *cmd,
                                  unsigned int flags);

+char ** virshCheckpointNameCompleter(vshControl *ctl,
+                                     const vshCmd *cmd,
+                                     unsigned int flags);
+
 char ** virshSnapshotNameCompleter(vshControl *ctl,
                                    const vshCmd *cmd,
                                    unsigned int flags);
diff --git a/tools/virsh-util.h b/tools/virsh-util.h
index fb2ed277af..f814558144 100644
--- a/tools/virsh-util.h
+++ b/tools/virsh-util.h
@@ -43,6 +43,9 @@ virshCommandOptDomain(vshControl *ctl,
 void
 virshDomainFree(virDomainPtr dom);

+void
+virshDomainCheckpointFree(virDomainCheckpointPtr chk);
+
 void
 virshDomainSnapshotFree(virDomainSnapshotPtr snap);

diff --git a/tools/virsh.h b/tools/virsh.h
index 254ce3289e..da157d6caa 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -41,6 +41,7 @@
 /*
  * Command group types
  */
+# define VIRSH_CMD_GRP_CHECKPOINT       "Checkpoint"
 # define VIRSH_CMD_GRP_DOM_MANAGEMENT   "Domain Management"
 # define VIRSH_CMD_GRP_DOM_MONITORING   "Domain Monitoring"
 # define VIRSH_CMD_GRP_STORAGE_POOL     "Storage Pool"
diff --git a/po/POTFILES b/po/POTFILES
index 57c55fb35f..70417cd01b 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -300,6 +300,7 @@ src/xenconfig/xen_xl.c
 src/xenconfig/xen_xm.c
 tests/virpolkittest.c
 tools/libvirt-guests.sh.in
+tools/virsh-checkpoint.c
 tools/virsh-console.c
 tools/virsh-domain-monitor.c
 tools/virsh-domain.c
diff --git a/tools/Makefile.am b/tools/Makefile.am
index c6064dee08..72cfde5ae6 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -1,4 +1,4 @@
-## Copyright (C) 2005-2016 Red Hat, Inc.
+## Copyright (C) 2005-2018 Red Hat, Inc.
 ## Copyright (C) 2013 Yuto KAWAMURA(kawamuray) <kawamuray.dadada@gmail.com>
 ##
 ## This library is free software; you can redistribute it and/or
@@ -216,6 +216,7 @@ virt_login_shell_CFLAGS = \

 virsh_SOURCES = \
 		virsh.c virsh.h \
+		virsh-checkpoint.c virsh-checkpoint.h \
 		virsh-completer.c virsh-completer.h \
 		virsh-console.c virsh-console.h \
 		virsh-domain.c virsh-domain.h \
diff --git a/tools/virsh-checkpoint.c b/tools/virsh-checkpoint.c
new file mode 100644
index 0000000000..3089383dc5
--- /dev/null
+++ b/tools/virsh-checkpoint.c
@@ -0,0 +1,1370 @@
+/*
+ * virsh-checkpoint.c: Commands to manage domain checkpoints
+ *
+ * Copyright (C) 2005-2019 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ *  Daniel Veillard <veillard@redhat.com>
+ *  Karel Zak <kzak@redhat.com>
+ *  Daniel P. Berrange <berrange@redhat.com>
+ *
+ */
+
+#include <config.h>
+#include "virsh-checkpoint.h"
+
+#include <assert.h>
+
+#include <libxml/parser.h>
+#include <libxml/tree.h>
+#include <libxml/xpath.h>
+#include <libxml/xmlsave.h>
+
+#include "internal.h"
+#include "virbuffer.h"
+#include "viralloc.h"
+#include "virfile.h"
+#include "virsh-util.h"
+#include "virstring.h"
+#include "virxml.h"
+#include "conf/checkpoint_conf.h"
+
+/* Helper for checkpoint-create and checkpoint-create-as */
+static bool
+virshCheckpointCreate(vshControl *ctl,
+                      virDomainPtr dom,
+                      const char *buffer,
+                      unsigned int flags,
+                      const char *from)
+{
+    bool ret = false;
+    virDomainCheckpointPtr checkpoint;
+    const char *name = NULL;
+
+    checkpoint = virDomainCheckpointCreateXML(dom, buffer, flags);
+
+    if (checkpoint == NULL)
+        goto cleanup;
+
+    name = virDomainCheckpointGetName(checkpoint);
+    if (!name) {
+        vshError(ctl, "%s", _("Could not get snapshot name"));
+        goto cleanup;
+    }
+
+    if (from)
+        vshPrintExtra(ctl, _("Domain checkpoint %s created from '%s'"),
+                      name, from);
+    else
+        vshPrintExtra(ctl, _("Domain checkpoint %s created"), name);
+
+    ret = true;
+
+ cleanup:
+    virshDomainCheckpointFree(checkpoint);
+    return ret;
+}
+
+
+/*
+ * "checkpoint-create" command
+ */
+static const vshCmdInfo info_checkpoint_create[] = {
+    {.name = "help",
+     .data = N_("Create a checkpoint from XML")
+    },
+    {.name = "desc",
+     .data = N_("Create a checkpoint from XML for use in "
+                "future incremental backups")
+    },
+    {.name = NULL}
+};
+
+static const vshCmdOptDef opts_checkpoint_create[] = {
+    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+    {.name = "xmlfile",
+     .type = VSH_OT_STRING,
+     .help = N_("domain checkpoint XML")
+    },
+    {.name = "redefine",
+     .type = VSH_OT_BOOL,
+     .help = N_("redefine metadata for existing checkpoint")
+    },
+    VIRSH_COMMON_OPT_CURRENT(N_("with redefine, set current checkpoint")),
+    {.name = "no-metadata",
+     .type = VSH_OT_BOOL,
+     .help = N_("create checkpoint but create no metadata")
+    },
+    {.name = "quiesce",
+     .type = VSH_OT_BOOL,
+     .help = N_("quiesce guest's file systems")
+    },
+    {.name = NULL}
+};
+
+static bool
+cmdCheckpointCreate(vshControl *ctl,
+                    const vshCmd *cmd)
+{
+    virDomainPtr dom = NULL;
+    bool ret = false;
+    const char *from = NULL;
+    char *buffer = NULL;
+    unsigned int flags = 0;
+
+    if (vshCommandOptBool(cmd, "redefine"))
+        flags |= VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE;
+    if (vshCommandOptBool(cmd, "current"))
+        flags |= VIR_DOMAIN_CHECKPOINT_CREATE_CURRENT;
+    if (vshCommandOptBool(cmd, "no-metadata"))
+        flags |= VIR_DOMAIN_CHECKPOINT_CREATE_NO_METADATA;
+    if (vshCommandOptBool(cmd, "quiesce"))
+        flags |= VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE;
+
+    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+        goto cleanup;
+
+    if (vshCommandOptStringReq(ctl, cmd, "xmlfile", &from) < 0)
+        goto cleanup;
+    if (!from) {
+        buffer = vshStrdup(ctl, "<domaincheckpoint/>");
+    } else {
+        if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) {
+            vshSaveLibvirtError();
+            goto cleanup;
+        }
+    }
+
+    ret = virshCheckpointCreate(ctl, dom, buffer, flags, from);
+
+ cleanup:
+    VIR_FREE(buffer);
+    virshDomainFree(dom);
+
+    return ret;
+}
+
+
+/*
+ * "checkpoint-create-as" command
+ */
+static int
+virshParseCheckpointDiskspec(vshControl *ctl,
+                             virBufferPtr buf,
+                             const char *str)
+{
+    int ret = -1;
+    const char *name = NULL;
+    const char *checkpoint = NULL;
+    const char *bitmap = NULL;
+    char **array = NULL;
+    int narray;
+    size_t i;
+
+    narray = vshStringToArray(str, &array);
+    if (narray <= 0)
+        goto cleanup;
+
+    name = array[0];
+    for (i = 1; i < narray; i++) {
+        if (!checkpoint && STRPREFIX(array[i], "checkpoint="))
+            checkpoint = array[i] + strlen("checkpoint=");
+        else if (!bitmap && STRPREFIX(array[i], "bitmap="))
+            bitmap = array[i] + strlen("bitmap=");
+        else
+            goto cleanup;
+    }
+
+    virBufferEscapeString(buf, "<disk name='%s'", name);
+    if (checkpoint)
+        virBufferAsprintf(buf, " checkpoint='%s'", checkpoint);
+    if (bitmap)
+        virBufferAsprintf(buf, " bitmap='%s'", bitmap);
+    virBufferAddLit(buf, "/>\n");
+    ret = 0;
+ cleanup:
+    if (ret < 0)
+        vshError(ctl, _("unable to parse diskspec: %s"), str);
+    virStringListFree(array);
+    return ret;
+}
+
+static const vshCmdInfo info_checkpoint_create_as[] = {
+    {.name = "help",
+     .data = N_("Create a checkpoint from a set of args")
+    },
+    {.name = "desc",
+     .data = N_("Create a checkpoint from arguments for use in "
+                "future incremental backups")
+    },
+    {.name = NULL}
+};
+
+static const vshCmdOptDef opts_checkpoint_create_as[] = {
+    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+    {.name = "name",
+     .type = VSH_OT_STRING,
+     .help = N_("name of checkpoint")
+    },
+    {.name = "description",
+     .type = VSH_OT_STRING,
+     .help = N_("description of checkpoint")
+    },
+    {.name = "print-xml",
+     .type = VSH_OT_BOOL,
+     .help = N_("print XML document rather than create")
+    },
+    {.name = "no-metadata",
+     .type = VSH_OT_BOOL,
+     .help = N_("take checkpoint but create no metadata")
+    },
+    {.name = "quiesce",
+     .type = VSH_OT_BOOL,
+     .help = N_("quiesce guest's file systems")
+    },
+    {.name = "diskspec",
+     .type = VSH_OT_ARGV,
+     .help = N_("disk attributes: disk[,checkpoint=type][,bitmap=name]")
+    },
+    {.name = NULL}
+};
+
+
+static bool
+cmdCheckpointCreateAs(vshControl *ctl,
+                      const vshCmd *cmd)
+{
+    virDomainPtr dom = NULL;
+    bool ret = false;
+    char *buffer = NULL;
+    const char *name = NULL;
+    const char *desc = NULL;
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+    unsigned int flags = 0;
+    const vshCmdOpt *opt = NULL;
+
+    if (vshCommandOptBool(cmd, "no-metadata")) {
+        if (vshCommandOptBool(cmd, "print-xml")) {
+            vshError(ctl, "%s",
+                     _("--print-xml is incompatible with --no-metadata"));
+            return false;
+        }
+        flags |= VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA;
+    }
+    if (vshCommandOptBool(cmd, "quiesce"))
+        flags |= VIR_DOMAIN_CHECKPOINT_CREATE_QUIESCE;
+
+    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+        return false;
+
+    if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0 ||
+        vshCommandOptStringReq(ctl, cmd, "description", &desc) < 0)
+        goto cleanup;
+
+    virBufferAddLit(&buf, "<domaincheckpoint>\n");
+    virBufferAdjustIndent(&buf, 2);
+    virBufferEscapeString(&buf, "<name>%s</name>\n", name);
+    virBufferEscapeString(&buf, "<description>%s</description>\n", desc);
+
+    if (vshCommandOptBool(cmd, "diskspec")) {
+        virBufferAddLit(&buf, "<disks>\n");
+        virBufferAdjustIndent(&buf, 2);
+        while ((opt = vshCommandOptArgv(ctl, cmd, opt))) {
+            if (virshParseCheckpointDiskspec(ctl, &buf, opt->data) < 0)
+                goto cleanup;
+        }
+        virBufferAdjustIndent(&buf, -2);
+        virBufferAddLit(&buf, "</disks>\n");
+    }
+    virBufferAdjustIndent(&buf, -2);
+    virBufferAddLit(&buf, "</domaincheckpoint>\n");
+
+    if (virBufferError(&buf)) {
+        vshError(ctl, "%s", _("Out of memory"));
+        goto cleanup;
+    }
+
+    buffer = virBufferContentAndReset(&buf);
+
+    if (vshCommandOptBool(cmd, "print-xml")) {
+        vshPrint(ctl, "%s\n",  buffer);
+        ret = true;
+        goto cleanup;
+    }
+
+    ret = virshCheckpointCreate(ctl, dom, buffer, flags, NULL);
+
+ cleanup:
+    virBufferFreeAndReset(&buf);
+    VIR_FREE(buffer);
+    virshDomainFree(dom);
+
+    return ret;
+}
+
+
+/* Helper for resolving {--current | --ARG name} into a checkpoint
+ * belonging to DOM.  If EXCLUSIVE, fail if both --current and arg are
+ * present.  On success, populate *CHK and *NAME, before returning 0.
+ * On failure, return -1 after issuing an error message.  */
+static int
+virshLookupCheckpoint(vshControl *ctl,
+                      const vshCmd *cmd,
+                      const char *arg,
+                      bool exclusive,
+                      virDomainPtr dom,
+                      virDomainCheckpointPtr *chk,
+                      const char **name)
+{
+    bool current = vshCommandOptBool(cmd, "current");
+    const char *chkname = NULL;
+
+    if (vshCommandOptStringReq(ctl, cmd, arg, &chkname) < 0)
+        return -1;
+
+    if (exclusive && current && chkname) {
+        vshError(ctl, _("--%s and --current are mutually exclusive"), arg);
+        return -1;
+    }
+
+    if (chkname) {
+        *chk = virDomainCheckpointLookupByName(dom, chkname, 0);
+    } else if (current) {
+        *chk = virDomainCheckpointCurrent(dom, 0);
+    } else {
+        vshError(ctl, _("--%s or --current is required"), arg);
+        return -1;
+    }
+    if (!*chk) {
+        vshReportError(ctl);
+        return -1;
+    }
+
+    *name = virDomainCheckpointGetName(*chk);
+    return 0;
+}
+
+
+/*
+ * "checkpoint-edit" command
+ */
+static const vshCmdInfo info_checkpoint_edit[] = {
+    {.name = "help",
+     .data = N_("edit XML for a checkpoint")
+    },
+    {.name = "desc",
+     .data = N_("Edit the domain checkpoint XML for a named checkpoint")
+    },
+    {.name = NULL}
+};
+
+static const vshCmdOptDef opts_checkpoint_edit[] = {
+    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+    {.name = "checkpointname",
+     .type = VSH_OT_STRING,
+     .help = N_("checkpoint name"),
+     .completer = virshCheckpointNameCompleter,
+    },
+    VIRSH_COMMON_OPT_CURRENT(N_("also set edited checkpoint as current")),
+    {.name = "rename",
+     .type = VSH_OT_BOOL,
+     .help = N_("allow renaming an existing checkpoint")
+    },
+    {.name = "clone",
+     .type = VSH_OT_BOOL,
+     .help = N_("allow cloning to new name")
+    },
+    {.name = NULL}
+};
+
+static bool
+cmdCheckpointEdit(vshControl *ctl,
+                  const vshCmd *cmd)
+{
+    virDomainPtr dom = NULL;
+    virDomainCheckpointPtr checkpoint = NULL;
+    virDomainCheckpointPtr edited = NULL;
+    const char *name = NULL;
+    const char *edited_name;
+    bool ret = false;
+    unsigned int getxml_flags = VIR_DOMAIN_CHECKPOINT_XML_SECURE;
+    unsigned int define_flags = VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE;
+    bool rename_okay = vshCommandOptBool(cmd, "rename");
+    bool clone_okay = vshCommandOptBool(cmd, "clone");
+
+    VSH_EXCLUSIVE_OPTIONS_EXPR("rename", rename_okay, "clone", clone_okay)
+
+    if (vshCommandOptBool(cmd, "current") &&
+        vshCommandOptBool(cmd, "checkpointname"))
+        define_flags |= VIR_DOMAIN_CHECKPOINT_CREATE_CURRENT;
+
+    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+        return false;
+
+    if (virshLookupCheckpoint(ctl, cmd, "checkpointname", false, dom,
+                              &checkpoint, &name) < 0)
+        goto cleanup;
+
+#define EDIT_GET_XML \
+    virDomainCheckpointGetXMLDesc(checkpoint, getxml_flags)
+#define EDIT_NOT_CHANGED \
+    do { \
+        /* Depending on flags, we re-edit even if XML is unchanged.  */ \
+        if (!(define_flags & VIR_DOMAIN_CHECKPOINT_CREATE_CURRENT)) { \
+            vshPrintExtra(ctl, \
+                          _("Checkpoint %s XML configuration not changed.\n"), \
+                          name); \
+            ret = true; \
+            goto edit_cleanup; \
+        } \
+    } while (0)
+#define EDIT_DEFINE \
+    edited = virDomainCheckpointCreateXML(dom, doc_edited, define_flags)
+#include "virsh-edit.c"
+
+    edited_name = virDomainCheckpointGetName(edited);
+    if (STREQ(name, edited_name)) {
+        vshPrintExtra(ctl, _("Checkpoint %s edited.\n"), name);
+    } else if (clone_okay) {
+        vshPrintExtra(ctl, _("Checkpoint %s cloned to %s.\n"), name,
+                      edited_name);
+    } else {
+        unsigned int delete_flags;
+
+        delete_flags = VIR_DOMAIN_CHECKPOINT_DELETE_METADATA_ONLY;
+        if (virDomainCheckpointDelete(rename_okay ? checkpoint : edited,
+                                      delete_flags) < 0) {
+            vshReportError(ctl);
+            vshError(ctl, _("Failed to clean up %s"),
+                     rename_okay ? name : edited_name);
+            goto cleanup;
+        }
+        if (!rename_okay) {
+            vshError(ctl, _("Must use --rename or --clone to change %s to %s"),
+                     name, edited_name);
+            goto cleanup;
+        }
+    }
+
+    ret = true;
+
+ cleanup:
+    if (!ret && name)
+        vshError(ctl, _("Failed to update %s"), name);
+    virshDomainCheckpointFree(edited);
+    virshDomainCheckpointFree(checkpoint);
+    virshDomainFree(dom);
+    return ret;
+}
+
+
+/*
+ * "checkpoint-current" command
+ */
+static const vshCmdInfo info_checkpoint_current[] = {
+    {.name = "help",
+     .data = N_("Get or set the current checkpoint")
+    },
+    {.name = "desc",
+     .data = N_("Get or set the current checkpoint")
+    },
+    {.name = NULL}
+};
+
+static const vshCmdOptDef opts_checkpoint_current[] = {
+    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+    {.name = "name",
+     .type = VSH_OT_BOOL,
+     .help = N_("list the name, rather than the full xml")
+    },
+    {.name = "security-info",
+     .type = VSH_OT_BOOL,
+     .help = N_("include security sensitive information in XML dump")
+    },
+    {.name = "no-domain",
+     .type = VSH_OT_BOOL,
+     .help = N_("exclude <domain> from XML")
+    },
+    {.name = "size",
+     .type = VSH_OT_BOOL,
+     .help = N_("include backup size estimate in XML dump")
+    },
+    {.name = "checkpointname",
+     .type = VSH_OT_STRING,
+     .help = N_("name of existing checkpoint to make current"),
+     .completer = virshCheckpointNameCompleter,
+    },
+    {.name = NULL}
+};
+
+static bool
+cmdCheckpointCurrent(vshControl *ctl,
+                     const vshCmd *cmd)
+{
+    virDomainPtr dom = NULL;
+    bool ret = false;
+    int current;
+    virDomainCheckpointPtr checkpoint = NULL;
+    char *xml = NULL;
+    const char *checkpointname = NULL;
+    unsigned int flags = 0;
+    const char *domname;
+
+    if (vshCommandOptBool(cmd, "security-info"))
+        flags |= VIR_DOMAIN_CHECKPOINT_XML_SECURE;
+    if (vshCommandOptBool(cmd, "no-domain"))
+        flags |= VIR_DOMAIN_CHECKPOINT_XML_NO_DOMAIN;
+    if (vshCommandOptBool(cmd, "size"))
+        flags |= VIR_DOMAIN_CHECKPOINT_XML_SIZE;
+
+    VSH_EXCLUSIVE_OPTIONS("name", "checkpointname");
+
+    if (!(dom = virshCommandOptDomain(ctl, cmd, &domname)))
+        return false;
+
+    if (vshCommandOptStringReq(ctl, cmd, "checkpointname", &checkpointname) < 0)
+        goto cleanup;
+
+    if (checkpointname) {
+        virDomainCheckpointPtr checkpoint2 = NULL;
+        flags = (VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE |
+                 VIR_DOMAIN_CHECKPOINT_CREATE_CURRENT);
+
+        if (!(checkpoint = virDomainCheckpointLookupByName(dom,
+                                                           checkpointname, 0)))
+            goto cleanup;
+
+        xml = virDomainCheckpointGetXMLDesc(checkpoint,
+                                            VIR_DOMAIN_CHECKPOINT_XML_SECURE);
+        if (!xml)
+            goto cleanup;
+
+        if (!(checkpoint2 = virDomainCheckpointCreateXML(dom, xml, flags)))
+            goto cleanup;
+
+        virshDomainCheckpointFree(checkpoint2);
+        vshPrintExtra(ctl, _("Checkpoint %s set as current"), checkpointname);
+        ret = true;
+        goto cleanup;
+    }
+
+    if ((current = virDomainHasCurrentCheckpoint(dom, 0)) < 0)
+        goto cleanup;
+
+    if (!current) {
+        vshError(ctl, _("domain '%s' has no current checkpoint"), domname);
+        goto cleanup;
+    } else {
+        if (!(checkpoint = virDomainCheckpointCurrent(dom, 0)))
+            goto cleanup;
+
+        if (vshCommandOptBool(cmd, "name")) {
+            const char *name;
+            if (!(name = virDomainCheckpointGetName(checkpoint)))
+                goto cleanup;
+
+            vshPrint(ctl, "%s", name);
+        } else {
+            if (!(xml = virDomainCheckpointGetXMLDesc(checkpoint, flags)))
+                goto cleanup;
+
+            vshPrint(ctl, "%s", xml);
+        }
+    }
+
+    ret = true;
+
+ cleanup:
+    if (!ret)
+        vshReportError(ctl);
+    VIR_FREE(xml);
+    virshDomainCheckpointFree(checkpoint);
+    virshDomainFree(dom);
+
+    return ret;
+}
+
+
+/* Helper function to get the name of a checkpoint's parent.  Caller
+ * must free the result.  Returns 0 on success (including when it was
+ * proven no parent exists), and -1 on failure with error reported
+ * (such as no checkpoint support or domain deleted in meantime).  */
+static int
+virshGetCheckpointParent(vshControl *ctl,
+                         virDomainCheckpointPtr checkpoint,
+                         char **parent_name)
+{
+    virDomainCheckpointPtr parent = NULL;
+    int ret = -1;
+
+    *parent_name = NULL;
+
+    parent = virDomainCheckpointGetParent(checkpoint, 0);
+    if (parent) {
+        /* API works, and virDomainCheckpointGetName will succeed */
+        *parent_name = vshStrdup(ctl, virDomainCheckpointGetName(parent));
+        ret = 0;
+    } else if (last_error->code == VIR_ERR_NO_DOMAIN_CHECKPOINT) {
+        /* API works, and we found a root with no parent */
+        ret = 0;
+    }
+
+    if (ret < 0) {
+        vshReportError(ctl);
+        vshError(ctl, "%s", _("unable to determine if checkpoint has parent"));
+    } else {
+        vshResetLibvirtError();
+    }
+    virshDomainCheckpointFree(parent);
+    return ret;
+}
+
+
+/*
+ * "checkpoint-info" command
+ */
+static const vshCmdInfo info_checkpoint_info[] = {
+    {.name = "help",
+     .data = N_("checkpoint information")
+    },
+    {.name = "desc",
+     .data = N_("Returns basic information about a checkpoint.")
+    },
+    {.name = NULL}
+};
+
+static const vshCmdOptDef opts_checkpoint_info[] = {
+    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+    {.name = "checkpointname",
+     .type = VSH_OT_STRING,
+     .help = N_("checkpoint name"),
+     .completer = virshCheckpointNameCompleter,
+    },
+    VIRSH_COMMON_OPT_CURRENT(N_("info on current checkpoint")),
+    {.name = NULL}
+};
+
+
+static bool
+cmdCheckpointInfo(vshControl *ctl,
+                  const vshCmd *cmd)
+{
+    virDomainPtr dom;
+    virDomainCheckpointPtr checkpoint = NULL;
+    const char *name;
+    char *parent = NULL;
+    bool ret = false;
+    int count;
+    unsigned int flags;
+    int current;
+    int metadata;
+
+    dom = virshCommandOptDomain(ctl, cmd, NULL);
+    if (dom == NULL)
+        return false;
+
+    if (virshLookupCheckpoint(ctl, cmd, "checkpointname", true, dom,
+                              &checkpoint, &name) < 0)
+        goto cleanup;
+
+    vshPrint(ctl, "%-15s %s\n", _("Name:"), name);
+    vshPrint(ctl, "%-15s %s\n", _("Domain:"), virDomainGetName(dom));
+
+    /* Determine if checkpoint is current.  */
+    current = virDomainCheckpointIsCurrent(checkpoint, 0);
+    if (current < 0) {
+        vshError(ctl, "%s",
+                 _("unexpected problem querying checkpoint state"));
+        goto cleanup;
+    }
+    vshPrint(ctl, "%-15s %s\n", _("Current:"),
+             current > 0 ? _("yes") : _("no"));
+
+    if (virshGetCheckpointParent(ctl, checkpoint, &parent) < 0) {
+        vshError(ctl, "%s",
+                 _("unexpected problem querying checkpoint state"));
+        goto cleanup;
+    }
+    vshPrint(ctl, "%-15s %s\n", _("Parent:"), parent ? parent : "-");
+
+    /* Children, Descendants.  */
+    flags = 0;
+    count = virDomainCheckpointListAllChildren(checkpoint, NULL, flags);
+    if (count < 0) {
+        if (last_error->code == VIR_ERR_NO_SUPPORT) {
+            vshResetLibvirtError();
+            ret = true;
+        }
+        goto cleanup;
+    }
+    vshPrint(ctl, "%-15s %d\n", _("Children:"), count);
+    flags = VIR_DOMAIN_CHECKPOINT_LIST_DESCENDANTS;
+    count = virDomainCheckpointListAllChildren(checkpoint, NULL, flags);
+    if (count < 0)
+        goto cleanup;
+    vshPrint(ctl, "%-15s %d\n", _("Descendants:"), count);
+
+    /* Metadata.  */
+    metadata = virDomainCheckpointHasMetadata(checkpoint, 0);
+    if (metadata >= 0)
+        vshPrint(ctl, "%-15s %s\n", _("Metadata:"),
+                 metadata ? _("yes") : _("no"));
+
+    ret = true;
+
+ cleanup:
+    VIR_FREE(parent);
+    virshDomainCheckpointFree(checkpoint);
+    virshDomainFree(dom);
+    return ret;
+}
+
+
+/* Helpers for collecting a list of checkpoints.  */
+struct virshChk {
+    virDomainCheckpointPtr chk;
+    char *parent;
+};
+struct virshCheckpointList {
+    struct virshChk *chks;
+    int nchks;
+};
+typedef struct virshCheckpointList *virshCheckpointListPtr;
+
+static void
+virshCheckpointListFree(virshCheckpointListPtr chklist)
+{
+    size_t i;
+
+    if (!chklist)
+        return;
+    if (chklist->chks) {
+        for (i = 0; i < chklist->nchks; i++) {
+            virshDomainCheckpointFree(chklist->chks[i].chk);
+            VIR_FREE(chklist->chks[i].parent);
+        }
+        VIR_FREE(chklist->chks);
+    }
+    VIR_FREE(chklist);
+}
+
+
+static int
+virshChkSorter(const void *a,
+               const void *b)
+{
+    const struct virshChk *sa = a;
+    const struct virshChk *sb = b;
+
+    if (sa->chk && !sb->chk)
+        return -1;
+    if (!sa->chk)
+        return sb->chk != NULL;
+
+    return vshStrcasecmp(virDomainCheckpointGetName(sa->chk),
+                         virDomainCheckpointGetName(sb->chk));
+}
+
+
+/* Compute a list of checkpoints from DOM.  If FROM is provided, the
+ * list is limited to descendants of the given checkpoint.  If FLAGS is
+ * given, the list is filtered.  If TREE is specified, then all but
+ * FROM or the roots will also have parent information.  */
+static virshCheckpointListPtr
+virshCheckpointListCollect(vshControl *ctl,
+                           virDomainPtr dom,
+                           virDomainCheckpointPtr from,
+                           unsigned int orig_flags,
+                           bool tree)
+{
+    size_t i;
+    char **names = NULL;
+    int count = -1;
+    virDomainCheckpointPtr *chks;
+    virshCheckpointListPtr chklist = vshMalloc(ctl, sizeof(*chklist));
+    virshCheckpointListPtr ret = NULL;
+    unsigned int flags = orig_flags;
+
+    if (from)
+        count = virDomainCheckpointListAllChildren(from, &chks, flags);
+    else
+        count = virDomainListAllCheckpoints(dom, &chks, flags);
+    if (count < 0) {
+        vshError(ctl, "%s",
+                 _("unexpected problem querying checkpoints"));
+        goto cleanup;
+    }
+
+    /* When mixing --from and --tree, we also want a copy of from
+     * in the list, but with no parent for that one entry.  */
+    chklist->chks = vshCalloc(ctl, count + (tree && from),
+                              sizeof(*chklist->chks));
+    chklist->nchks = count;
+    for (i = 0; i < count; i++)
+        chklist->chks[i].chk = chks[i];
+    VIR_FREE(chks);
+    if (tree) {
+        for (i = 0; i < count; i++) {
+            if (virshGetCheckpointParent(ctl, chklist->chks[i].chk,
+                                         &chklist->chks[i].parent) < 0)
+                goto cleanup;
+        }
+        if (from) {
+            chklist->chks[chklist->nchks++].chk = from;
+            virDomainCheckpointRef(from);
+        }
+    }
+
+    qsort(chklist->chks, chklist->nchks, sizeof(*chklist->chks),
+          virshChkSorter);
+
+    ret = chklist;
+    chklist = NULL;
+
+ cleanup:
+    virshCheckpointListFree(chklist);
+    if (names && count > 0)
+        for (i = 0; i < count; i++)
+            VIR_FREE(names[i]);
+    VIR_FREE(names);
+    return ret;
+}
+
+
+static const char *
+virshCheckpointListLookup(int id,
+                          bool parent,
+                          void *opaque)
+{
+    virshCheckpointListPtr chklist = opaque;
+    if (parent)
+        return chklist->chks[id].parent;
+    return virDomainCheckpointGetName(chklist->chks[id].chk);
+}
+
+
+/*
+ * "checkpoint-list" command
+ */
+static const vshCmdInfo info_checkpoint_list[] = {
+    {.name = "help",
+     .data = N_("List checkpoints for a domain")
+    },
+    {.name = "desc",
+     .data = N_("Checkpoint List")
+    },
+    {.name = NULL}
+};
+
+static const vshCmdOptDef opts_checkpoint_list[] = {
+    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+    {.name = "parent",
+     .type = VSH_OT_BOOL,
+     .help = N_("add a column showing parent checkpoint")
+    },
+    {.name = "roots",
+     .type = VSH_OT_BOOL,
+     .help = N_("list only checkpoints without parents")
+    },
+    {.name = "leaves",
+     .type = VSH_OT_BOOL,
+     .help = N_("list only checkpoints without children")
+    },
+    {.name = "no-leaves",
+     .type = VSH_OT_BOOL,
+     .help = N_("list only checkpoints that are not leaves (with children)")
+    },
+    {.name = "metadata",
+     .type = VSH_OT_BOOL,
+     .help = N_("list only checkpoints that have metadata that would prevent undefine")
+    },
+    {.name = "no-metadata",
+     .type = VSH_OT_BOOL,
+     .help = N_("list only checkpoints that have no metadata managed by libvirt")
+    },
+    {.name = "tree",
+     .type = VSH_OT_BOOL,
+     .help = N_("list checkpoints in a tree")
+    },
+    {.name = "from",
+     .type = VSH_OT_STRING,
+     .help = N_("limit list to children of given checkpoint"),
+     .completer = virshCheckpointNameCompleter,
+    },
+    VIRSH_COMMON_OPT_CURRENT(N_("limit list to children of current checkpoint")),
+    {.name = "descendants",
+     .type = VSH_OT_BOOL,
+     .help = N_("with --from, list all descendants")
+    },
+    {.name = "name",
+     .type = VSH_OT_BOOL,
+     .help = N_("list checkpoint names only")
+    },
+    {.name = "topological",
+     .type = VSH_OT_BOOL,
+     .help = N_("sort list topologically rather than by name"),
+    },
+
+    {.name = NULL}
+};
+
+static bool
+cmdCheckpointList(vshControl *ctl,
+                  const vshCmd *cmd)
+{
+    virDomainPtr dom = NULL;
+    bool ret = false;
+    unsigned int flags = 0;
+    size_t i;
+    xmlDocPtr xml = NULL;
+    xmlXPathContextPtr ctxt = NULL;
+    char *doc = NULL;
+    virDomainCheckpointPtr checkpoint = NULL;
+    long long creation_longlong;
+    time_t creation_time_t;
+    char timestr[100];
+    struct tm time_info;
+    bool tree = vshCommandOptBool(cmd, "tree");
+    bool name = vshCommandOptBool(cmd, "name");
+    bool from = vshCommandOptBool(cmd, "from");
+    bool parent = vshCommandOptBool(cmd, "parent");
+    bool roots = vshCommandOptBool(cmd, "roots");
+    bool current = vshCommandOptBool(cmd, "current");
+    const char *from_chk = NULL;
+    char *parent_chk = NULL;
+    virDomainCheckpointPtr start = NULL;
+    virshCheckpointListPtr chklist = NULL;
+
+    VSH_EXCLUSIVE_OPTIONS_VAR(tree, name);
+    VSH_EXCLUSIVE_OPTIONS_VAR(parent, roots);
+    VSH_EXCLUSIVE_OPTIONS_VAR(parent, tree);
+    VSH_EXCLUSIVE_OPTIONS_VAR(roots, tree);
+    VSH_EXCLUSIVE_OPTIONS_VAR(roots, from);
+    VSH_EXCLUSIVE_OPTIONS_VAR(roots, current);
+
+#define FILTER(option, flag) \
+    do { \
+        if (vshCommandOptBool(cmd, option)) { \
+            if (tree) { \
+                vshError(ctl, \
+                         _("--%s and --tree are mutually exclusive"), \
+                         option); \
+                return false; \
+            } \
+            flags |= VIR_DOMAIN_CHECKPOINT_LIST_ ## flag; \
+        } \
+    } while (0)
+
+    FILTER("leaves", LEAVES);
+    FILTER("no-leaves", NO_LEAVES);
+#undef FILTER
+
+    if (vshCommandOptBool(cmd, "topological"))
+        flags |= VIR_DOMAIN_CHECKPOINT_LIST_TOPOLOGICAL;
+
+    if (roots)
+        flags |= VIR_DOMAIN_CHECKPOINT_LIST_ROOTS;
+
+    if (vshCommandOptBool(cmd, "metadata"))
+        flags |= VIR_DOMAIN_CHECKPOINT_LIST_METADATA;
+
+    if (vshCommandOptBool(cmd, "no-metadata"))
+        flags |= VIR_DOMAIN_CHECKPOINT_LIST_NO_METADATA;
+
+    if (vshCommandOptBool(cmd, "descendants")) {
+        if (!from && !current) {
+            vshError(ctl, "%s",
+                     _("--descendants requires either --from or --current"));
+            return false;
+        }
+        flags |= VIR_DOMAIN_CHECKPOINT_LIST_DESCENDANTS;
+    }
+
+    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+        return false;
+
+    if ((from || current) &&
+        virshLookupCheckpoint(ctl, cmd, "from", true, dom, &start, &from_chk) < 0)
+        goto cleanup;
+
+    if (!(chklist = virshCheckpointListCollect(ctl, dom, start, flags, tree)))
+        goto cleanup;
+
+    if (!tree && !name) {
+        if (parent)
+            vshPrintExtra(ctl, " %-20s %-25s %s",
+                          _("Name"), _("Creation Time"), _("Parent"));
+        else
+            vshPrintExtra(ctl, " %-20s %-25s",
+                          _("Name"), _("Creation Time"));
+        vshPrintExtra(ctl, "\n"
+                           "------------------------------"
+                           "--------------\n");
+    }
+
+    if (tree) {
+        for (i = 0; i < chklist->nchks; i++) {
+            if (!chklist->chks[i].parent &&
+                vshTreePrint(ctl, virshCheckpointListLookup, chklist,
+                             chklist->nchks, i) < 0)
+                goto cleanup;
+        }
+        ret = true;
+        goto cleanup;
+    }
+
+    for (i = 0; i < chklist->nchks; i++) {
+        const char *chk_name;
+
+        /* free up memory from previous iterations of the loop */
+        VIR_FREE(parent_chk);
+        xmlXPathFreeContext(ctxt);
+        xmlFreeDoc(xml);
+        VIR_FREE(doc);
+
+        checkpoint = chklist->chks[i].chk;
+        chk_name = virDomainCheckpointGetName(checkpoint);
+        assert(chk_name);
+
+        if (name) {
+            /* just print the checkpoint name */
+            vshPrint(ctl, "%s\n", chk_name);
+            continue;
+        }
+
+        if (!(doc = virDomainCheckpointGetXMLDesc(checkpoint, 0)))
+            continue;
+
+        if (!(xml = virXMLParseStringCtxt(doc, _("(domain_checkpoint)"), &ctxt)))
+            continue;
+
+        if (parent)
+            parent_chk = virXPathString("string(/domaincheckpoint/parent/name)",
+                                        ctxt);
+
+        if (virXPathLongLong("string(/domaincheckpoint/creationTime)", ctxt,
+                             &creation_longlong) < 0)
+            continue;
+        creation_time_t = creation_longlong;
+        if (creation_time_t != creation_longlong) {
+            vshError(ctl, "%s", _("time_t overflow"));
+            continue;
+        }
+        localtime_r(&creation_time_t, &time_info);
+        strftime(timestr, sizeof(timestr), "%Y-%m-%d %H:%M:%S %z",
+                 &time_info);
+
+        if (parent)
+            vshPrint(ctl, " %-20s %-25s %s\n",
+                     chk_name, timestr, parent_chk ?: "-");
+        else
+            vshPrint(ctl, " %-20s %-25s\n", chk_name, timestr);
+    }
+
+    ret = true;
+
+ cleanup:
+    /* this frees up memory from the last iteration of the loop */
+    virshCheckpointListFree(chklist);
+    VIR_FREE(parent_chk);
+    virshDomainCheckpointFree(start);
+    xmlXPathFreeContext(ctxt);
+    xmlFreeDoc(xml);
+    VIR_FREE(doc);
+    virshDomainFree(dom);
+
+    return ret;
+}
+
+
+/*
+ * "checkpoint-dumpxml" command
+ */
+static const vshCmdInfo info_checkpoint_dumpxml[] = {
+    {.name = "help",
+     .data = N_("Dump XML for a domain checkpoint")
+    },
+    {.name = "desc",
+     .data = N_("Checkpoint Dump XML")
+    },
+    {.name = NULL}
+};
+
+static const vshCmdOptDef opts_checkpoint_dumpxml[] = {
+    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+    {.name = "checkpointname",
+     .type = VSH_OT_DATA,
+     .flags = VSH_OFLAG_REQ,
+     .help = N_("checkpoint name"),
+     .completer = virshCheckpointNameCompleter,
+    },
+    {.name = "security-info",
+     .type = VSH_OT_BOOL,
+     .help = N_("include security sensitive information in XML dump")
+    },
+    {.name = "no-domain",
+     .type = VSH_OT_BOOL,
+     .help = N_("exclude <domain> from XML")
+    },
+    {.name = "size",
+     .type = VSH_OT_BOOL,
+     .help = N_("include backup size estimate in XML dump")
+    },
+    {.name = NULL}
+};
+
+static bool
+cmdCheckpointDumpXML(vshControl *ctl,
+                     const vshCmd *cmd)
+{
+    virDomainPtr dom = NULL;
+    bool ret = false;
+    const char *name = NULL;
+    virDomainCheckpointPtr checkpoint = NULL;
+    char *xml = NULL;
+    unsigned int flags = 0;
+
+    if (vshCommandOptBool(cmd, "security-info"))
+        flags |= VIR_DOMAIN_CHECKPOINT_XML_SECURE;
+    if (vshCommandOptBool(cmd, "no-domain"))
+        flags |= VIR_DOMAIN_CHECKPOINT_XML_NO_DOMAIN;
+    if (vshCommandOptBool(cmd, "size"))
+        flags |= VIR_DOMAIN_CHECKPOINT_XML_SIZE;
+
+    if (vshCommandOptStringReq(ctl, cmd, "checkpointname", &name) < 0)
+        return false;
+
+    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+        return false;
+
+    if (!(checkpoint = virDomainCheckpointLookupByName(dom, name, 0)))
+        goto cleanup;
+
+    if (!(xml = virDomainCheckpointGetXMLDesc(checkpoint, flags)))
+        goto cleanup;
+
+    vshPrint(ctl, "%s", xml);
+    ret = true;
+
+ cleanup:
+    VIR_FREE(xml);
+    virshDomainCheckpointFree(checkpoint);
+    virshDomainFree(dom);
+
+    return ret;
+}
+
+
+/*
+ * "checkpoint-parent" command
+ */
+static const vshCmdInfo info_checkpoint_parent[] = {
+    {.name = "help",
+     .data = N_("Get the name of the parent of a checkpoint")
+    },
+    {.name = "desc",
+     .data = N_("Extract the checkpoint's parent, if any")
+    },
+    {.name = NULL}
+};
+
+static const vshCmdOptDef opts_checkpoint_parent[] = {
+    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+    {.name = "checkpointname",
+     .type = VSH_OT_STRING,
+     .help = N_("find parent of checkpoint name"),
+     .completer = virshCheckpointNameCompleter,
+    },
+    VIRSH_COMMON_OPT_CURRENT(N_("find parent of current checkpoint")),
+    {.name = NULL}
+};
+
+static bool
+cmdCheckpointParent(vshControl *ctl,
+                    const vshCmd *cmd)
+{
+    virDomainPtr dom = NULL;
+    bool ret = false;
+    const char *name = NULL;
+    virDomainCheckpointPtr checkpoint = NULL;
+    char *parent = NULL;
+
+    dom = virshCommandOptDomain(ctl, cmd, NULL);
+    if (dom == NULL)
+        goto cleanup;
+
+    if (virshLookupCheckpoint(ctl, cmd, "checkpointname", true, dom,
+                              &checkpoint, &name) < 0)
+        goto cleanup;
+
+    if (virshGetCheckpointParent(ctl, checkpoint, &parent) < 0)
+        goto cleanup;
+    if (!parent) {
+        vshError(ctl, _("checkpoint '%s' has no parent"), name);
+        goto cleanup;
+    }
+
+    vshPrint(ctl, "%s", parent);
+
+    ret = true;
+
+ cleanup:
+    VIR_FREE(parent);
+    virshDomainCheckpointFree(checkpoint);
+    virshDomainFree(dom);
+
+    return ret;
+}
+
+
+/*
+ * "checkpoint-delete" command
+ */
+static const vshCmdInfo info_checkpoint_delete[] = {
+    {.name = "help",
+     .data = N_("Delete a domain checkpoint")
+    },
+    {.name = "desc",
+     .data = N_("Checkpoint Delete")
+    },
+    {.name = NULL}
+};
+
+static const vshCmdOptDef opts_checkpoint_delete[] = {
+    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+    {.name = "checkpointname",
+     .type = VSH_OT_STRING,
+     .help = N_("checkpoint name"),
+     .completer = virshCheckpointNameCompleter,
+    },
+    VIRSH_COMMON_OPT_CURRENT(N_("delete current checkpoint")),
+    {.name = "children",
+     .type = VSH_OT_BOOL,
+     .help = N_("delete checkpoint and all children")
+    },
+    {.name = "children-only",
+     .type = VSH_OT_BOOL,
+     .help = N_("delete children but not checkpoint")
+    },
+    {.name = "metadata",
+     .type = VSH_OT_BOOL,
+     .help = N_("delete only libvirt metadata, leaving checkpoint contents behind")
+    },
+    {.name = NULL}
+};
+
+static bool
+cmdCheckpointDelete(vshControl *ctl,
+                    const vshCmd *cmd)
+{
+    virDomainPtr dom = NULL;
+    bool ret = false;
+    const char *name = NULL;
+    virDomainCheckpointPtr checkpoint = NULL;
+    unsigned int flags = 0;
+
+    dom = virshCommandOptDomain(ctl, cmd, NULL);
+    if (dom == NULL)
+        goto cleanup;
+
+    if (virshLookupCheckpoint(ctl, cmd, "checkpointname", true, dom,
+                              &checkpoint, &name) < 0)
+        goto cleanup;
+
+    if (vshCommandOptBool(cmd, "children"))
+        flags |= VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN;
+    if (vshCommandOptBool(cmd, "children-only"))
+        flags |= VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY;
+    if (vshCommandOptBool(cmd, "metadata"))
+        flags |= VIR_DOMAIN_CHECKPOINT_DELETE_METADATA_ONLY;
+
+    if (virDomainCheckpointDelete(checkpoint, flags) == 0) {
+        if (flags & VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY)
+            vshPrintExtra(ctl, _("Domain checkpoint %s children deleted\n"), name);
+        else
+            vshPrintExtra(ctl, _("Domain checkpoint %s deleted\n"), name);
+    } else {
+        vshError(ctl, _("Failed to delete checkpoint %s"), name);
+        goto cleanup;
+    }
+
+    ret = true;
+
+ cleanup:
+    virshDomainCheckpointFree(checkpoint);
+    virshDomainFree(dom);
+
+    return ret;
+}
+
+
+const vshCmdDef checkpointCmds[] = {
+    {.name = "checkpoint-create",
+     .handler = cmdCheckpointCreate,
+     .opts = opts_checkpoint_create,
+     .info = info_checkpoint_create,
+     .flags = 0
+    },
+    {.name = "checkpoint-create-as",
+     .handler = cmdCheckpointCreateAs,
+     .opts = opts_checkpoint_create_as,
+     .info = info_checkpoint_create_as,
+     .flags = 0
+    },
+    {.name = "checkpoint-current",
+     .handler = cmdCheckpointCurrent,
+     .opts = opts_checkpoint_current,
+     .info = info_checkpoint_current,
+     .flags = 0
+    },
+    {.name = "checkpoint-delete",
+     .handler = cmdCheckpointDelete,
+     .opts = opts_checkpoint_delete,
+     .info = info_checkpoint_delete,
+     .flags = 0
+    },
+    {.name = "checkpoint-dumpxml",
+     .handler = cmdCheckpointDumpXML,
+     .opts = opts_checkpoint_dumpxml,
+     .info = info_checkpoint_dumpxml,
+     .flags = 0
+    },
+    {.name = "checkpoint-edit",
+     .handler = cmdCheckpointEdit,
+     .opts = opts_checkpoint_edit,
+     .info = info_checkpoint_edit,
+     .flags = 0
+    },
+    {.name = "checkpoint-info",
+     .handler = cmdCheckpointInfo,
+     .opts = opts_checkpoint_info,
+     .info = info_checkpoint_info,
+     .flags = 0
+    },
+    {.name = "checkpoint-list",
+     .handler = cmdCheckpointList,
+     .opts = opts_checkpoint_list,
+     .info = info_checkpoint_list,
+     .flags = 0
+    },
+    {.name = "checkpoint-parent",
+     .handler = cmdCheckpointParent,
+     .opts = opts_checkpoint_parent,
+     .info = info_checkpoint_parent,
+     .flags = 0
+    },
+    {.name = NULL}
+};
diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index c4adbb70d0..ed5e1015c9 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -1,7 +1,7 @@
 /*
  * virsh-completer.c: virsh completer callbacks
  *
- * Copyright (C) 2017 Red Hat, Inc.
+ * Copyright (C) 2017-2019 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -623,6 +623,57 @@ virshSecretUUIDCompleter(vshControl *ctl,
 }


+char **
+virshCheckpointNameCompleter(vshControl *ctl,
+                             const vshCmd *cmd,
+                             unsigned int flags)
+{
+    virshControlPtr priv = ctl->privData;
+    virDomainPtr dom = NULL;
+    virDomainCheckpointPtr *checkpoints = NULL;
+    int ncheckpoints = 0;
+    size_t i = 0;
+    char **ret = NULL;
+
+    virCheckFlags(0, NULL);
+
+    if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
+        return NULL;
+
+    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+        return NULL;
+
+    if ((ncheckpoints = virDomainListAllCheckpoints(dom, &checkpoints,
+                                                    flags)) < 0)
+        goto error;
+
+    if (VIR_ALLOC_N(ret, ncheckpoints + 1) < 0)
+        goto error;
+
+    for (i = 0; i < ncheckpoints; i++) {
+        const char *name = virDomainCheckpointGetName(checkpoints[i]);
+
+        if (VIR_STRDUP(ret[i], name) < 0)
+            goto error;
+
+        virshDomainCheckpointFree(checkpoints[i]);
+    }
+    VIR_FREE(checkpoints);
+    virshDomainFree(dom);
+
+    return ret;
+
+ error:
+    for (; i < ncheckpoints; i++)
+        virshDomainCheckpointFree(checkpoints[i]);
+    VIR_FREE(checkpoints);
+    for (i = 0; i < ncheckpoints; i++)
+        VIR_FREE(ret[i]);
+    VIR_FREE(ret);
+    virshDomainFree(dom);
+    return NULL;
+}
+
 char **
 virshSnapshotNameCompleter(vshControl *ctl,
                            const vshCmd *cmd,
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index ad739a9df5..a52e3e7e32 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1,7 +1,7 @@
 /*
  * virsh-domain-monitor.c: Commands to monitor domain status
  *
- * Copyright (C) 2005, 2007-2016 Red Hat, Inc.
+ * Copyright (C) 2005-2019 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -1614,6 +1614,7 @@ virshDomainListCollect(vshControl *ctl, unsigned int flags)
     int autostart;
     int state;
     int nsnap;
+    int nchk;
     int mansave;
     virshControlPtr priv = ctl->privData;

@@ -1781,6 +1782,17 @@ virshDomainListCollect(vshControl *ctl, unsigned int flags)
                 goto remove_entry;
         }

+        /* checkpoint filter */
+        if (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_CHECKPOINT)) {
+            if ((nchk = virDomainListAllCheckpoints(dom, NULL, 0)) < 0) {
+                vshError(ctl, "%s", _("Failed to get checkpoint count"));
+                goto cleanup;
+            }
+            if (!((VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_HAS_CHECKPOINT) && nchk > 0) ||
+                  (VSH_MATCH(VIR_CONNECT_LIST_DOMAINS_NO_CHECKPOINT) && nchk == 0)))
+                goto remove_entry;
+        }
+
         /* the domain matched all filters, it may stay */
         continue;

@@ -1842,6 +1854,14 @@ static const vshCmdOptDef opts_list[] = {
      .type = VSH_OT_BOOL,
      .help = N_("list domains without a snapshot")
     },
+    {.name = "with-checkpoint",
+     .type = VSH_OT_BOOL,
+     .help = N_("list domains with existing checkpoint")
+    },
+    {.name = "without-checkpoint",
+     .type = VSH_OT_BOOL,
+     .help = N_("list domains without a checkpoint")
+    },
     {.name = "state-running",
      .type = VSH_OT_BOOL,
      .help = N_("list domains in running state")
@@ -1941,6 +1961,9 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
     FILTER("with-snapshot",    VIR_CONNECT_LIST_DOMAINS_HAS_SNAPSHOT);
     FILTER("without-snapshot", VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT);

+    FILTER("with-checkpoint",    VIR_CONNECT_LIST_DOMAINS_HAS_CHECKPOINT);
+    FILTER("without-checkpoint", VIR_CONNECT_LIST_DOMAINS_NO_CHECKPOINT);
+
     FILTER("state-running", VIR_CONNECT_LIST_DOMAINS_RUNNING);
     FILTER("state-paused",  VIR_CONNECT_LIST_DOMAINS_PAUSED);
     FILTER("state-shutoff", VIR_CONNECT_LIST_DOMAINS_SHUTOFF);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 8402f8fce6..1970710c07 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -3630,6 +3630,10 @@ static const vshCmdOptDef opts_undefine[] = {
      .type = VSH_OT_BOOL,
      .help = N_("remove all domain snapshot metadata, if inactive")
     },
+    {.name = "checkpoints-metadata",
+     .type = VSH_OT_BOOL,
+     .help = N_("remove all domain checkpoint metadata, if inactive")
+    },
     {.name = "nvram",
      .type = VSH_OT_BOOL,
      .help = N_("remove nvram file, if inactive")
@@ -3659,6 +3663,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
     /* User-requested actions.  */
     bool managed_save = vshCommandOptBool(cmd, "managed-save");
     bool snapshots_metadata = vshCommandOptBool(cmd, "snapshots-metadata");
+    bool checkpoints_metadata = vshCommandOptBool(cmd, "checkpoints-metadata");
     bool wipe_storage = vshCommandOptBool(cmd, "wipe-storage");
     bool remove_all_storage = vshCommandOptBool(cmd, "remove-all-storage");
     bool delete_snapshots = vshCommandOptBool(cmd, "delete-snapshots");
@@ -3667,6 +3672,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
     /* Positive if these items exist.  */
     int has_managed_save = 0;
     int has_snapshots_metadata = 0;
+    int has_checkpoints_metadata = 0;
     int has_snapshots = 0;
     /* True if undefine will not strand data, even on older servers.  */
     bool managed_save_safe = false;
@@ -3713,6 +3719,8 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
         flags |= VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA;
         snapshots_safe = true;
     }
+    if (checkpoints_metadata)
+        flags |= VIR_DOMAIN_UNDEFINE_CHECKPOINTS_METADATA;
     if (nvram)
         flags |= VIR_DOMAIN_UNDEFINE_NVRAM;
     if (keep_nvram)
@@ -3761,7 +3769,12 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
                 managed_save_safe = snapshots_safe = true;
             }
         }
+        has_checkpoints_metadata
+            = virDomainListAllCheckpoints(dom, NULL,
+                                          VIR_DOMAIN_CHECKPOINT_LIST_METADATA);
     }
+    /* Clear flags which older servers might reject, if they would
+     * otherwise have no effect. */
     if (!has_managed_save) {
         flags &= ~VIR_DOMAIN_UNDEFINE_MANAGED_SAVE;
         managed_save_safe = true;
@@ -3772,6 +3785,8 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd)
         flags &= ~VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA;
         snapshots_safe = true;
     }
+    if (has_checkpoints_metadata == 0)
+        flags &= ~VIR_DOMAIN_UNDEFINE_CHECKPOINTS_METADATA;

     /* Stash domain description for later use */
     if (vol_string || remove_all_storage) {
diff --git a/tools/virsh-util.c b/tools/virsh-util.c
index aa88397d61..933d1c825d 100644
--- a/tools/virsh-util.c
+++ b/tools/virsh-util.c
@@ -228,6 +228,17 @@ virshDomainFree(virDomainPtr dom)
 }


+void
+virshDomainCheckpointFree(virDomainCheckpointPtr chk)
+{
+    if (!chk)
+        return;
+
+    vshSaveLibvirtHelperError();
+    virDomainCheckpointFree(chk); /* sc_prohibit_obj_free_apis_in_virsh */
+}
+
+
 void
 virshDomainSnapshotFree(virDomainSnapshotPtr snap)
 {
diff --git a/tools/virsh.c b/tools/virsh.c
index b41304a888..0de41e33b8 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -50,6 +50,7 @@
 #include "virstring.h"
 #include "virgettext.h"

+#include "virsh-checkpoint.h"
 #include "virsh-console.h"
 #include "virsh-domain.h"
 #include "virsh-domain-monitor.h"
@@ -832,6 +833,7 @@ static const vshCmdGrp cmdGroups[] = {
     {VIRSH_CMD_GRP_DOM_MANAGEMENT, "domain", domManagementCmds},
     {VIRSH_CMD_GRP_DOM_MONITORING, "monitor", domMonitoringCmds},
     {VIRSH_CMD_GRP_HOST_AND_HV, "host", hostAndHypervisorCmds},
+    {VIRSH_CMD_GRP_CHECKPOINT, "checkpoint", checkpointCmds},
     {VIRSH_CMD_GRP_IFACE, "interface", ifaceCmds},
     {VIRSH_CMD_GRP_NWFILTER, "filter", nwfilterCmds},
     {VIRSH_CMD_GRP_NETWORK, "network", networkCmds},
diff --git a/tools/virsh.pod b/tools/virsh.pod
index a0fe949c55..2f833ac184 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -409,6 +409,7 @@ Inject NMI to the guest.
               [I<--with-managed-save>] [I<--without-managed-save>]
               [I<--autostart>] [I<--no-autostart>]
               [I<--with-snapshot>] [I<--without-snapshot>]
+              [I<--with-checkpoint>] [I<--without-checkpoint>]
               [I<--state-running>] [I<--state-paused>]
               [I<--state-shutoff>] [I<--state-other>]

@@ -514,6 +515,11 @@ this feature disabled use I<--no-autostart>.
 Domains that have snapshot images can be listed using flag I<--with-snapshot>,
 domains without a snapshot I<--without-snapshot>.

+=item B<Checkpoint existence>
+
+Domains that have checkpoints can be listed using flag I<--with-checkpoint>,
+domains without a checkpoint I<--without-checkpoint>.
+
 =back

 When talking to older servers, this command is forced to use a series of API
@@ -809,7 +815,8 @@ can be restarted later.
 If I<domain> is transient, then the metadata of any snapshots will
 be lost once the guest stops running, but the snapshot contents still
 exist, and a new domain with the same name and UUID can restore the
-snapshot metadata with B<snapshot-create>.
+snapshot metadata with B<snapshot-create>.  Similarly, the metadata of
+any checkpoints will be lost, but can be restored with B<checkpoint-create>.

 If I<--graceful> is specified, don't resort to extreme measures
 (e.g. SIGKILL) when the guest doesn't stop after a reasonable timeout;
@@ -1570,7 +1577,7 @@ Convert a domain Id (or UUID) to domain name
 Rename a domain. This command changes current domain name to the new name
 specified in the second argument.

-B<Note>: Domain must be inactive and without snapshots.
+B<Note>: Domain must be inactive and without snapshots or checkpoints.

 =item B<domstate> I<domain> [I<--reason>]

@@ -2812,10 +2819,11 @@ services must be shutdown in the domain.
 The exact behavior of a domain when it shuts down is set by the
 I<on_poweroff> parameter in the domain's XML definition.

-If I<domain> is transient, then the metadata of any snapshots will
-be lost once the guest stops running, but the snapshot contents still
-exist, and a new domain with the same name and UUID can restore the
-snapshot metadata with B<snapshot-create>.
+If I<domain> is transient, then the metadata of any snapshots and
+checkpoints will be lost once the guest stops running, but the underlying
+contents still exist, and a new domain with the same name and UUID can
+restore the snapshot metadata with B<snapshot-create>, and the checkpoint
+metadata with B<checkpoint-create>.

 By default the hypervisor will try to pick a suitable shutdown
 method. To specify an alternative method, the I<--mode> parameter
@@ -2892,7 +2900,7 @@ Output the device used for the TTY console of the domain. If the information
 is not available the processes will provide an exit code of 1.

 =item B<undefine> I<domain> [I<--managed-save>] [I<--snapshots-metadata>]
-[I<--nvram>] [I<--keep-nvram>]
+[I<--checkpoints-metadata>] [I<--nvram>] [I<--keep-nvram>]
 [ {I<--storage> B<volumes> | I<--remove-all-storage> [I<--delete-snapshots>]}
 I<--wipe-storage>]

@@ -2910,6 +2918,12 @@ domain.  Without the flag, attempts to undefine an inactive domain with
 snapshot metadata will fail.  If the domain is active, this flag is
 ignored.

+The I<--checkpoints-metadata> flag guarantees that any checkpoints (see the
+B<checkpoint-list> command) are also cleaned up when undefining an inactive
+domain.  Without the flag, attempts to undefine an inactive domain with
+checkpoint metadata will fail.  If the domain is active, this flag is
+ignored.
+
 I<--nvram> and I<--keep-nvram> specify accordingly to delete or keep nvram
 (/domain/os/nvram/) file. If the domain has an nvram file and the flags are
 omitted, the undefine will fail.
@@ -4877,6 +4891,216 @@ the data contents from that point in time.

 =back

+=head1 CHECKPOINT COMMANDS
+
+The following commands manipulate domain checkpoints.  Checkpoints serve as
+a point in time to identify which portions of a guest's disks have changed
+after that time, making it possible to perform incremental and differential
+backups.  Checkpoints are identified with a unique name.  See
+L<https://libvirt.org/formatcheckpoint.html> for documentation of the XML
+format used to represent properties of checkpoints.
+
+=over 4
+
+=item B<checkpoint-create> I<domain> [I<xmlfile>] {[I<--redefine>
+{[I<--current>] | [I<--redefine-list>]}] | [I<--no-metadata>] [I<--quiesce>]}
+
+Create a checkpoint for domain I<domain> with the properties specified
+in I<xmlfile> describing a <domaincheckpoint> top-level element. If
+I<xmlfile> is completely omitted, then libvirt will create a
+checkpoint with a name based on the current time. The new checkpoint
+will become current, as listed by B<checkpoint-current>.
+
+If I<--redefine> is specified, then all XML elements produced by
+B<checkpoint-dumpxml> are valid; this can be used to migrate
+checkpoint hierarchy from one machine to another, to recreate
+hierarchy for the case of a transient domain that goes away and is
+later recreated with the same name and UUID, or to make slight
+alterations in the checkpoint metadata (such as host-specific aspects
+of the domain XML embedded in the checkpoint).  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.  If I<--redefine-list> is specified, I<--redefine> is
+implied, I<--current> is rejected, and the XML changes from being a
+single <domaincheckpoint> to instead being a <checkpoints> element
+describing a list of checkpoints. List form only works if the domain
+has no currently-defined checkpoint metadata, and can be obtained as a
+subset of I<dumpxml --checkpoints> output.
+
+If I<--no-metadata> is specified, then the checkpoint data is created,
+but any metadata is immediately discarded (that is, libvirt does not
+treat the checkpoint as current, and cannot use the checkpoint for an
+incremental backup unless I<--redefine> is later used to teach libvirt
+about the metadata again).
+
+If I<--quiesce> is specified, libvirt will try to use guest agent
+to freeze and unfreeze domain's mounted file systems. However,
+if domain has no guest agent, checkpoint creation will fail.
+
+Existence of checkpoint metadata will prevent attempts to B<undefine>
+a persistent domain.  However, for transient domains, checkpoint
+metadata is silently lost when the domain quits running (whether
+by command such as B<destroy> or by internal guest action).
+
+=item B<checkpoint-create-as> I<domain> {[I<--print-xml>]
+| [I<--no-metadata>]} [I<name>] [I<description>] [I<--quiesce>]
+[I<--diskspec>] B<diskspec>]...
+
+Create a checkpoint for domain I<domain> with the given <name> and
+<description>; if either value is omitted, libvirt will choose a
+value.  If I<--print-xml> is specified, then XML appropriate for
+I<checkpoint-create> is output, rather than actually creating a
+checkpoint.
+
+The I<--diskspec> option can be used to control which guest disks participate in the checkpoint. This option can occur
+multiple times, according to the number of <disk> elements in the domain
+xml.  Each <diskspec> is in the
+form B<disk[,checkpoint=type][,bitmap=name]>. A literal I<--diskspec> must precede each B<diskspec> unless
+all three of I<domain>, I<name>, and I<description> are also present.
+For example, a diskspec of "vda,checkpoint=bitmap,bitmap=map1"
+results in the following XML:
+  <disk name='vda' checkpoint='bitmap' bitmap='map1'/>
+
+If I<--quiesce> is specified, libvirt will try to use guest agent
+to freeze and unfreeze domain's mounted file systems. However,
+if domain has no guest agent, checkpoint creation will fail.
+
+If I<--no-metadata> is specified, then the checkpoint data is created,
+but any metadata is immediately discarded (that is, libvirt does not
+treat the checkpoint as current, and cannot use the checkpoint for an
+incremental backup unless I<--redefine> is later used to teach libvirt
+about the metadata again).
+
+=item B<checkpoint-current> I<domain> {[I<--name>] | [I<--security-info>]
+[I<--no-domain>] [I<--size>] | [I<checkpointname>]}
+
+Without I<checkpointname>, this will output the checkpoint XML for the
+domain's current checkpoint (if any).  If I<--name> is specified,
+output just the current checkpoint name instead of the full xml.
+Otherwise, using I<--security-info> will also include security
+sensitive information in the XML, using I<--size> will add XML
+indicating roughly how much guest data has changed since the
+checkpoint was created, and using I<--no-domain> will omit the
+<domain> element from the output for a more compact view.
+
+With I<checkpointname>, this is a request to make the existing named
+checkpoint become the current checkpoint.
+
+=item B<checkpoint-edit> I<domain> [I<checkpointname>] [I<--current>]
+{[I<--rename>] | [I<--clone>]}
+
+Edit the XML configuration file for I<checkpointname> of a domain.  If
+both I<checkpointname> and I<--current> are specified, also force the
+edited checkpoint to become the current snapshot.  If
+I<checkpointname> is omitted, then I<--current> must be supplied, to
+edit the current checkpoint.
+
+This is equivalent to:
+
+ virsh checkpoint-dumpxml dom name > checkpoint.xml
+ vi checkpoint.xml (or make changes with your other text editor)
+ virsh checkpoint-create dom checkpoint.xml --redefine [--current]
+
+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>.
+
+If I<--rename> is specified, then the edits can change the checkpoint
+name.  If I<--clone> is specified, then changing the snapshot name
+will create a clone of the checkpoint metadata.  If neither is
+specified, then the edits must not change the checkpoint name.  Note
+that changing a checkpoint name must be done with care, since some
+drivers may require the original checkpoint name for actually
+accessing changes since a point in time.
+
+=item B<checkpoint-info> I<domain> {I<checkpoint> | I<--current>}
+
+Output basic information about a named <checkpoint>, or the current
+checkpoint with I<--current>.
+
+=item B<checkpoint-list> I<domain> [I<--metadata>] [I<--no-metadata>]
+[{I<--parent> | I<--roots> | [{I<--tree> | I<--name>}]}] [I<--topological>]
+[{[I<--from>] B<checkpoint> | I<--current>} [I<--descendants>]]
+[I<--leaves>] [I<--no-leaves>]
+
+List all of the available checkpoints for the given domain, defaulting
+to show columns for the checkpoint name and creation time.
+
+Normally, table form output is sorted by checkpoint name; using
+I<--topological> instead sorts so that no child is listed before its
+ancestors (although there may be more than one possible ordering with
+this property).
+
+If I<--parent> is specified, add a column to the output table giving
+the name of the parent of each checkpoint.  If I<--roots> is
+specified, the list will be filtered to just checkpoints that have no
+parents.  If I<--tree> is specified, the output will be in a tree
+format, listing just checkpoint names.  These three options are
+mutually exclusive. If I<--name> is specified only the checkpoint name
+is printed. This option is mutually exclusive with I<--tree>.
+
+If I<--from> is provided, filter the list to checkpoints which are
+children of the given B<checkpoint>; or if I<--current> is provided,
+start at the current checkpoint.  When used in isolation or with
+I<--parent>, the list is limited to direct children unless
+I<--descendants> is also present.  When used with I<--tree>, the use
+of I<--descendants> is implied.  This option is not compatible with
+I<--roots>.  Note that the starting point of I<--from> or I<--current>
+is not included in the list unless the I<--tree> option is also
+present.
+
+If I<--leaves> is specified, the list will be filtered to just
+checkpoints that have no children.  Likewise, if I<--no-leaves> is
+specified, the list will be filtered to just checkpoints with
+children.  (Note that omitting both options does no filtering, while
+providing both options will either produce the same list or error out
+depending on whether the server recognizes the flags).  Filtering
+options are not compatible with I<--tree>.
+
+If I<--metadata> is specified, the list will be filtered to just
+checkpoints that involve libvirt metadata, and thus would prevent
+B<undefine> of a persistent domain, or be lost on B<destroy> of a
+transient domain.  Likewise, if I<--no-metadata> is specified, the
+list will be filtered to just checkpoints that exist without the need
+for libvirt metadata.
+
+=item B<checkpoint-dumpxml> I<domain> I<snapshot> [I<--security-info>]
+[I<--no-domain>] [I<--size>]
+
+Output the snapshot XML for the domain's checkpoint named
+I<checkpoint>.  Using I<--security-info> will also include security
+sensitive information, using I<--size> will add XML indicating roughly
+how much guest data has changed since the checkpoint was created, and
+using I<--no-domain> will omit the <domain> element from the output
+for a more compact view.  Use B<checkpoint-current> to easily access
+the XML of the current snapshot.  To grab the XML for all checkpoints
+at once, use B<dumpxml --checkpoints>.
+
+=item B<checkpoint-parent> I<domain> {I<checkpoint> | I<--current>}
+
+Output the name of the parent checkpoint, if any, for the given
+I<checkpoint>, or for the current checkpoint with I<--current>.
+
+=item B<checkpoint-delete> I<domain> {I<checkpoint> | I<--current>}
+[I<--metadata>] [{I<--children> | I<--children-only>}]
+
+Delete the checkpoint for the domain named I<checkpoint>, or the
+current checkpoint with I<--current>.  The record of which portions of
+the disk changed since the checkpoint are merged into the parent
+checkpoint (if any). If I<--children> is passed, then delete this
+checkpoint and any children of this checkpoint.  If I<--children-only>
+is passed, then delete any children of this checkpoint, but leave this
+checkpoint intact. These two flags are mutually exclusive.
+
+If I<--metadata> is specified, then only delete the checkpoint
+metadata maintained by libvirt, while leaving the checkpoint contents
+intact for access by external tools; otherwise deleting a checkpoint
+also removes the ability to perform an incremental backup from that
+point in time.
+
+=back
+
 =head1 NWFILTER COMMANDS

 The following commands manipulate network filters. Network filters allow
-- 
2.20.1

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

On Wed, Mar 27, 2019 at 05:10:46AM -0500, Eric Blake wrote:
>Introduce a bunch of new virsh commands for managing checkpoints
>in isolation. More commands are needed for performing incremental
>backups, but these commands were easy to implement by modeling
>heavily after virsh-snapshot.c (no need for checkpoint-revert,
>and checkpoint-list was a lot easier since we don't have to cater
>to older libvirt API).
>
>Signed-off-by: Eric Blake <eblake@redhat.com>
>---
> tools/virsh-checkpoint.h     |   29 +
> tools/virsh-completer.h      |    4 +
> tools/virsh-util.h           |    3 +
> tools/virsh.h                |    1 +
> po/POTFILES                  |    1 +
> tools/Makefile.am            |    3 +-
> tools/virsh-checkpoint.c     | 1370 ++++++++++++++++++++++++++++++++++
> tools/virsh-completer.c      |   53 +-
> tools/virsh-domain-monitor.c |   25 +-
> tools/virsh-domain.c         |   15 +
> tools/virsh-util.c           |   11 +
> tools/virsh.c                |    2 +
> tools/virsh.pod              |  238 +++++-
> 13 files changed, 1745 insertions(+), 10 deletions(-)
> create mode 100644 tools/virsh-checkpoint.h
> create mode 100644 tools/virsh-checkpoint.c
>
>+static bool
>+cmdCheckpointInfo(vshControl *ctl,
>+                  const vshCmd *cmd)
>+{
>+    virDomainPtr dom;
>+    virDomainCheckpointPtr checkpoint = NULL;
>+    const char *name;
>+    char *parent = NULL;
>+    bool ret = false;
>+    int count;
>+    unsigned int flags;
>+    int current;
>+    int metadata;
>+
>+    dom = virshCommandOptDomain(ctl, cmd, NULL);
>+    if (dom == NULL)
>+        return false;
>+
>+    if (virshLookupCheckpoint(ctl, cmd, "checkpointname", true, dom,
>+                              &checkpoint, &name) < 0)
>+        goto cleanup;
>+
>+    vshPrint(ctl, "%-15s %s\n", _("Name:"), name);
>+    vshPrint(ctl, "%-15s %s\n", _("Domain:"), virDomainGetName(dom));

We have vshTableNew and vshTableRowAppend that can compute the
indentation at run-time regardless of the locale.

>+
>+    /* Determine if checkpoint is current.  */
>+    current = virDomainCheckpointIsCurrent(checkpoint, 0);
>+    if (current < 0) {
>+        vshError(ctl, "%s",
>+                 _("unexpected problem querying checkpoint state"));
>+        goto cleanup;
>+    }
>+    vshPrint(ctl, "%-15s %s\n", _("Current:"),
>+             current > 0 ? _("yes") : _("no"));
>+
>+    if (virshGetCheckpointParent(ctl, checkpoint, &parent) < 0) {
>+        vshError(ctl, "%s",
>+                 _("unexpected problem querying checkpoint state"));
>+        goto cleanup;
>+    }
>+    vshPrint(ctl, "%-15s %s\n", _("Parent:"), parent ? parent : "-");
>+
>+    /* Children, Descendants.  */
>+    flags = 0;
>+    count = virDomainCheckpointListAllChildren(checkpoint, NULL, flags);
>+    if (count < 0) {
>+        if (last_error->code == VIR_ERR_NO_SUPPORT) {
>+            vshResetLibvirtError();
>+            ret = true;
>+        }
>+        goto cleanup;
>+    }
>+    vshPrint(ctl, "%-15s %d\n", _("Children:"), count);
>+    flags = VIR_DOMAIN_CHECKPOINT_LIST_DESCENDANTS;
>+    count = virDomainCheckpointListAllChildren(checkpoint, NULL, flags);
>+    if (count < 0)
>+        goto cleanup;
>+    vshPrint(ctl, "%-15s %d\n", _("Descendants:"), count);
>+
>+    /* Metadata.  */
>+    metadata = virDomainCheckpointHasMetadata(checkpoint, 0);
>+    if (metadata >= 0)
>+        vshPrint(ctl, "%-15s %s\n", _("Metadata:"),
>+                 metadata ? _("yes") : _("no"));
>+
>+    ret = true;
>+
>+ cleanup:
>+    VIR_FREE(parent);
>+    virshDomainCheckpointFree(checkpoint);
>+    virshDomainFree(dom);
>+    return ret;
>+}

[...]

>+
>+static bool
>+cmdCheckpointList(vshControl *ctl,
>+                  const vshCmd *cmd)
>+{
>+    virDomainPtr dom = NULL;
>+    bool ret = false;
>+    unsigned int flags = 0;
>+    size_t i;
>+    xmlDocPtr xml = NULL;
>+    xmlXPathContextPtr ctxt = NULL;
>+    char *doc = NULL;
>+    virDomainCheckpointPtr checkpoint = NULL;
>+    long long creation_longlong;
>+    time_t creation_time_t;
>+    char timestr[100];
>+    struct tm time_info;
>+    bool tree = vshCommandOptBool(cmd, "tree");
>+    bool name = vshCommandOptBool(cmd, "name");
>+    bool from = vshCommandOptBool(cmd, "from");
>+    bool parent = vshCommandOptBool(cmd, "parent");
>+    bool roots = vshCommandOptBool(cmd, "roots");
>+    bool current = vshCommandOptBool(cmd, "current");
>+    const char *from_chk = NULL;
>+    char *parent_chk = NULL;
>+    virDomainCheckpointPtr start = NULL;
>+    virshCheckpointListPtr chklist = NULL;
>+
>+    VSH_EXCLUSIVE_OPTIONS_VAR(tree, name);
>+    VSH_EXCLUSIVE_OPTIONS_VAR(parent, roots);
>+    VSH_EXCLUSIVE_OPTIONS_VAR(parent, tree);
>+    VSH_EXCLUSIVE_OPTIONS_VAR(roots, tree);
>+    VSH_EXCLUSIVE_OPTIONS_VAR(roots, from);
>+    VSH_EXCLUSIVE_OPTIONS_VAR(roots, current);
>+
>+#define FILTER(option, flag) \
>+    do { \
>+        if (vshCommandOptBool(cmd, option)) { \
>+            if (tree) { \
>+                vshError(ctl, \
>+                         _("--%s and --tree are mutually exclusive"), \
>+                         option); \
>+                return false; \
>+            } \
>+            flags |= VIR_DOMAIN_CHECKPOINT_LIST_ ## flag; \
>+        } \
>+    } while (0)
>+
>+    FILTER("leaves", LEAVES);
>+    FILTER("no-leaves", NO_LEAVES);
>+#undef FILTER
>+
>+    if (vshCommandOptBool(cmd, "topological"))
>+        flags |= VIR_DOMAIN_CHECKPOINT_LIST_TOPOLOGICAL;
>+
>+    if (roots)
>+        flags |= VIR_DOMAIN_CHECKPOINT_LIST_ROOTS;
>+
>+    if (vshCommandOptBool(cmd, "metadata"))
>+        flags |= VIR_DOMAIN_CHECKPOINT_LIST_METADATA;
>+
>+    if (vshCommandOptBool(cmd, "no-metadata"))
>+        flags |= VIR_DOMAIN_CHECKPOINT_LIST_NO_METADATA;
>+
>+    if (vshCommandOptBool(cmd, "descendants")) {
>+        if (!from && !current) {
>+            vshError(ctl, "%s",
>+                     _("--descendants requires either --from or --current"));
>+            return false;
>+        }
>+        flags |= VIR_DOMAIN_CHECKPOINT_LIST_DESCENDANTS;
>+    }
>+
>+    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
>+        return false;
>+
>+    if ((from || current) &&
>+        virshLookupCheckpoint(ctl, cmd, "from", true, dom, &start, &from_chk) < 0)
>+        goto cleanup;
>+
>+    if (!(chklist = virshCheckpointListCollect(ctl, dom, start, flags, tree)))
>+        goto cleanup;
>+
>+    if (!tree && !name) {
>+        if (parent)
>+            vshPrintExtra(ctl, " %-20s %-25s %s",
>+                          _("Name"), _("Creation Time"), _("Parent"));
>+        else
>+            vshPrintExtra(ctl, " %-20s %-25s",
>+                          _("Name"), _("Creation Time"));

vshTableNew

>+        vshPrintExtra(ctl, "\n"
>+                           "------------------------------"
>+                           "--------------\n");
>+    }
>+
>+    if (tree) {
>+        for (i = 0; i < chklist->nchks; i++) {
>+            if (!chklist->chks[i].parent &&
>+                vshTreePrint(ctl, virshCheckpointListLookup, chklist,
>+                             chklist->nchks, i) < 0)
>+                goto cleanup;
>+        }
>+        ret = true;
>+        goto cleanup;
>+    }
>+
>+    for (i = 0; i < chklist->nchks; i++) {

Putting this whole loop in a separate function would make this function
more readable - both tree and non-tree would share the same 'ret = true'
assignment and, more importantly, all the XML parsing stuff is only
needed for non-tree printing.

>+        const char *chk_name;
>+
>+        /* free up memory from previous iterations of the loop */

Sounds like a job for VIR_AUTOFREE

>+        VIR_FREE(parent_chk);
>+        xmlXPathFreeContext(ctxt);
>+        xmlFreeDoc(xml);
>+        VIR_FREE(doc);
>+
>+        checkpoint = chklist->chks[i].chk;
>+        chk_name = virDomainCheckpointGetName(checkpoint);
>+        assert(chk_name);
>+
>+        if (name) {
>+            /* just print the checkpoint name */
>+            vshPrint(ctl, "%s\n", chk_name);
>+            continue;
>+        }
>+
>+        if (!(doc = virDomainCheckpointGetXMLDesc(checkpoint, 0)))
>+            continue;
>+
>+        if (!(xml = virXMLParseStringCtxt(doc, _("(domain_checkpoint)"), &ctxt)))
>+            continue;
>+
>+        if (parent)
>+            parent_chk = virXPathString("string(/domaincheckpoint/parent/name)",
>+                                        ctxt);
>+
>+        if (virXPathLongLong("string(/domaincheckpoint/creationTime)", ctxt,
>+                             &creation_longlong) < 0)
>+            continue;
>+        creation_time_t = creation_longlong;
>+        if (creation_time_t != creation_longlong) {
>+            vshError(ctl, "%s", _("time_t overflow"));
>+            continue;
>+        }
>+        localtime_r(&creation_time_t, &time_info);
>+        strftime(timestr, sizeof(timestr), "%Y-%m-%d %H:%M:%S %z",
>+                 &time_info);
>+
>+        if (parent)
>+            vshPrint(ctl, " %-20s %-25s %s\n",
>+                     chk_name, timestr, parent_chk ?: "-");

vshTableRowApend

>+        else
>+            vshPrint(ctl, " %-20s %-25s\n", chk_name, timestr);
>+    }
>+
>+    ret = true;
>+
>+ cleanup:
>+    /* this frees up memory from the last iteration of the loop */
>+    virshCheckpointListFree(chklist);
>+    VIR_FREE(parent_chk);
>+    virshDomainCheckpointFree(start);
>+    xmlXPathFreeContext(ctxt);
>+    xmlFreeDoc(xml);
>+    VIR_FREE(doc);
>+    virshDomainFree(dom);
>+
>+    return ret;
>+}
>+
>+
>+/*
>+ * "checkpoint-dumpxml" command
>+ */
>+static const vshCmdInfo info_checkpoint_dumpxml[] = {
>+    {.name = "help",
>+     .data = N_("Dump XML for a domain checkpoint")
>+    },
>+    {.name = "desc",
>+     .data = N_("Checkpoint Dump XML")
>+    },
>+    {.name = NULL}
>+};
>+
>+static const vshCmdOptDef opts_checkpoint_dumpxml[] = {
>+    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
>+    {.name = "checkpointname",
>+     .type = VSH_OT_DATA,
>+     .flags = VSH_OFLAG_REQ,
>+     .help = N_("checkpoint name"),
>+     .completer = virshCheckpointNameCompleter,
>+    },
>+    {.name = "security-info",
>+     .type = VSH_OT_BOOL,
>+     .help = N_("include security sensitive information in XML dump")
>+    },
>+    {.name = "no-domain",
>+     .type = VSH_OT_BOOL,
>+     .help = N_("exclude <domain> from XML")
>+    },
>+    {.name = "size",
>+     .type = VSH_OT_BOOL,
>+     .help = N_("include backup size estimate in XML dump")
>+    },
>+    {.name = NULL}
>+};
>+
>+static bool
>+cmdCheckpointDumpXML(vshControl *ctl,
>+                     const vshCmd *cmd)
>+{
>+    virDomainPtr dom = NULL;
>+    bool ret = false;
>+    const char *name = NULL;
>+    virDomainCheckpointPtr checkpoint = NULL;
>+    char *xml = NULL;
>+    unsigned int flags = 0;
>+
>+    if (vshCommandOptBool(cmd, "security-info"))
>+        flags |= VIR_DOMAIN_CHECKPOINT_XML_SECURE;
>+    if (vshCommandOptBool(cmd, "no-domain"))
>+        flags |= VIR_DOMAIN_CHECKPOINT_XML_NO_DOMAIN;
>+    if (vshCommandOptBool(cmd, "size"))
>+        flags |= VIR_DOMAIN_CHECKPOINT_XML_SIZE;
>+
>+    if (vshCommandOptStringReq(ctl, cmd, "checkpointname", &name) < 0)
>+        return false;
>+
>+    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
>+        return false;
>+
>+    if (!(checkpoint = virDomainCheckpointLookupByName(dom, name, 0)))
>+        goto cleanup;
>+
>+    if (!(xml = virDomainCheckpointGetXMLDesc(checkpoint, flags)))
>+        goto cleanup;
>+
>+    vshPrint(ctl, "%s", xml);
>+    ret = true;
>+
>+ cleanup:
>+    VIR_FREE(xml);
>+    virshDomainCheckpointFree(checkpoint);
>+    virshDomainFree(dom);
>+
>+    return ret;
>+}
>+
>+
>+/*
>+ * "checkpoint-parent" command
>+ */
>+static const vshCmdInfo info_checkpoint_parent[] = {
>+    {.name = "help",
>+     .data = N_("Get the name of the parent of a checkpoint")
>+    },
>+    {.name = "desc",
>+     .data = N_("Extract the checkpoint's parent, if any")
>+    },
>+    {.name = NULL}
>+};
>+
>+static const vshCmdOptDef opts_checkpoint_parent[] = {
>+    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
>+    {.name = "checkpointname",
>+     .type = VSH_OT_STRING,
>+     .help = N_("find parent of checkpoint name"),
>+     .completer = virshCheckpointNameCompleter,
>+    },
>+    VIRSH_COMMON_OPT_CURRENT(N_("find parent of current checkpoint")),
>+    {.name = NULL}
>+};
>+
>+static bool
>+cmdCheckpointParent(vshControl *ctl,
>+                    const vshCmd *cmd)
>+{
>+    virDomainPtr dom = NULL;
>+    bool ret = false;
>+    const char *name = NULL;
>+    virDomainCheckpointPtr checkpoint = NULL;
>+    char *parent = NULL;
>+
>+    dom = virshCommandOptDomain(ctl, cmd, NULL);
>+    if (dom == NULL)
>+        goto cleanup;
>+
>+    if (virshLookupCheckpoint(ctl, cmd, "checkpointname", true, dom,
>+                              &checkpoint, &name) < 0)
>+        goto cleanup;
>+
>+    if (virshGetCheckpointParent(ctl, checkpoint, &parent) < 0)
>+        goto cleanup;
>+    if (!parent) {
>+        vshError(ctl, _("checkpoint '%s' has no parent"), name);
>+        goto cleanup;
>+    }
>+
>+    vshPrint(ctl, "%s", parent);
>+
>+    ret = true;
>+
>+ cleanup:
>+    VIR_FREE(parent);
>+    virshDomainCheckpointFree(checkpoint);
>+    virshDomainFree(dom);
>+
>+    return ret;
>+}
>+
>+
>+/*
>+ * "checkpoint-delete" command
>+ */
>+static const vshCmdInfo info_checkpoint_delete[] = {
>+    {.name = "help",
>+     .data = N_("Delete a domain checkpoint")
>+    },
>+    {.name = "desc",
>+     .data = N_("Checkpoint Delete")
>+    },
>+    {.name = NULL}
>+};
>+
>+static const vshCmdOptDef opts_checkpoint_delete[] = {
>+    VIRSH_COMMON_OPT_DOMAIN_FULL(0),
>+    {.name = "checkpointname",
>+     .type = VSH_OT_STRING,
>+     .help = N_("checkpoint name"),
>+     .completer = virshCheckpointNameCompleter,
>+    },
>+    VIRSH_COMMON_OPT_CURRENT(N_("delete current checkpoint")),
>+    {.name = "children",
>+     .type = VSH_OT_BOOL,
>+     .help = N_("delete checkpoint and all children")
>+    },
>+    {.name = "children-only",
>+     .type = VSH_OT_BOOL,
>+     .help = N_("delete children but not checkpoint")
>+    },
>+    {.name = "metadata",
>+     .type = VSH_OT_BOOL,
>+     .help = N_("delete only libvirt metadata, leaving checkpoint contents behind")
>+    },
>+    {.name = NULL}
>+};
>+
>+static bool
>+cmdCheckpointDelete(vshControl *ctl,
>+                    const vshCmd *cmd)
>+{
>+    virDomainPtr dom = NULL;
>+    bool ret = false;
>+    const char *name = NULL;
>+    virDomainCheckpointPtr checkpoint = NULL;
>+    unsigned int flags = 0;
>+
>+    dom = virshCommandOptDomain(ctl, cmd, NULL);
>+    if (dom == NULL)
>+        goto cleanup;
>+
>+    if (virshLookupCheckpoint(ctl, cmd, "checkpointname", true, dom,
>+                              &checkpoint, &name) < 0)
>+        goto cleanup;
>+
>+    if (vshCommandOptBool(cmd, "children"))
>+        flags |= VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN;
>+    if (vshCommandOptBool(cmd, "children-only"))
>+        flags |= VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY;
>+    if (vshCommandOptBool(cmd, "metadata"))
>+        flags |= VIR_DOMAIN_CHECKPOINT_DELETE_METADATA_ONLY;
>+
>+    if (virDomainCheckpointDelete(checkpoint, flags) == 0) {
>+        if (flags & VIR_DOMAIN_CHECKPOINT_DELETE_CHILDREN_ONLY)
>+            vshPrintExtra(ctl, _("Domain checkpoint %s children deleted\n"), name);
>+        else
>+            vshPrintExtra(ctl, _("Domain checkpoint %s deleted\n"), name);
>+    } else {
>+        vshError(ctl, _("Failed to delete checkpoint %s"), name);
>+        goto cleanup;
>+    }
>+
>+    ret = true;
>+
>+ cleanup:
>+    virshDomainCheckpointFree(checkpoint);
>+    virshDomainFree(dom);
>+
>+    return ret;
>+}
>+
>+
>+const vshCmdDef checkpointCmds[] = {
>+    {.name = "checkpoint-create",
>+     .handler = cmdCheckpointCreate,
>+     .opts = opts_checkpoint_create,
>+     .info = info_checkpoint_create,
>+     .flags = 0
>+    },
>+    {.name = "checkpoint-create-as",
>+     .handler = cmdCheckpointCreateAs,
>+     .opts = opts_checkpoint_create_as,
>+     .info = info_checkpoint_create_as,
>+     .flags = 0
>+    },
>+    {.name = "checkpoint-current",
>+     .handler = cmdCheckpointCurrent,
>+     .opts = opts_checkpoint_current,
>+     .info = info_checkpoint_current,
>+     .flags = 0
>+    },
>+    {.name = "checkpoint-delete",
>+     .handler = cmdCheckpointDelete,
>+     .opts = opts_checkpoint_delete,
>+     .info = info_checkpoint_delete,
>+     .flags = 0
>+    },
>+    {.name = "checkpoint-dumpxml",
>+     .handler = cmdCheckpointDumpXML,
>+     .opts = opts_checkpoint_dumpxml,
>+     .info = info_checkpoint_dumpxml,
>+     .flags = 0
>+    },
>+    {.name = "checkpoint-edit",
>+     .handler = cmdCheckpointEdit,
>+     .opts = opts_checkpoint_edit,
>+     .info = info_checkpoint_edit,
>+     .flags = 0
>+    },
>+    {.name = "checkpoint-info",
>+     .handler = cmdCheckpointInfo,
>+     .opts = opts_checkpoint_info,
>+     .info = info_checkpoint_info,
>+     .flags = 0
>+    },
>+    {.name = "checkpoint-list",
>+     .handler = cmdCheckpointList,
>+     .opts = opts_checkpoint_list,
>+     .info = info_checkpoint_list,
>+     .flags = 0
>+    },
>+    {.name = "checkpoint-parent",
>+     .handler = cmdCheckpointParent,
>+     .opts = opts_checkpoint_parent,
>+     .info = info_checkpoint_parent,
>+     .flags = 0
>+    },
>+    {.name = NULL}
>+};
>diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
>index c4adbb70d0..ed5e1015c9 100644
>--- a/tools/virsh-completer.c
>+++ b/tools/virsh-completer.c
>@@ -1,7 +1,7 @@
> /*
>  * virsh-completer.c: virsh completer callbacks
>  *
>- * Copyright (C) 2017 Red Hat, Inc.
>+ * Copyright (C) 2017-2019 Red Hat, Inc.
>  *
>  * This library is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU Lesser General Public
>@@ -623,6 +623,57 @@ virshSecretUUIDCompleter(vshControl *ctl,
> }
>
>
>+char **
>+virshCheckpointNameCompleter(vshControl *ctl,
>+                             const vshCmd *cmd,
>+                             unsigned int flags)
>+{
>+    virshControlPtr priv = ctl->privData;
>+    virDomainPtr dom = NULL;
>+    virDomainCheckpointPtr *checkpoints = NULL;
>+    int ncheckpoints = 0;
>+    size_t i = 0;
>+    char **ret = NULL;
>+
>+    virCheckFlags(0, NULL);
>+
>+    if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
>+        return NULL;
>+
>+    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
>+        return NULL;
>+
>+    if ((ncheckpoints = virDomainListAllCheckpoints(dom, &checkpoints,
>+                                                    flags)) < 0)

I recommend adding an 'int rc' variable and only assigning the result to
ncheckpoints if it's non-negative,

>+        goto error;
>+
>+    if (VIR_ALLOC_N(ret, ncheckpoints + 1) < 0)
>+        goto error;
>+
>+    for (i = 0; i < ncheckpoints; i++) {
>+        const char *name = virDomainCheckpointGetName(checkpoints[i]);
>+
>+        if (VIR_STRDUP(ret[i], name) < 0)
>+            goto error;
>+
>+        virshDomainCheckpointFree(checkpoints[i]);
>+    }
>+    VIR_FREE(checkpoints);
>+    virshDomainFree(dom);
>+
>+    return ret;
>+
>+ error:
>+    for (; i < ncheckpoints; i++)
>+        virshDomainCheckpointFree(checkpoints[i]);

otherwise this loop will try to run for a long time.
Also, given that we free these on success too, having a shared 'cleanup'
path would be nicer.


>+    VIR_FREE(checkpoints);
>+    for (i = 0; i < ncheckpoints; i++)
>+        VIR_FREE(ret[i]);
>+    VIR_FREE(ret);
>+    virshDomainFree(dom);
>+    return NULL;
>+}
>+
> char **
> virshSnapshotNameCompleter(vshControl *ctl,
>                            const vshCmd *cmd,

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

On 3/27/19 9:29 AM, Ján Tomko wrote:
> On Wed, Mar 27, 2019 at 05:10:46AM -0500, Eric Blake wrote:
>> Introduce a bunch of new virsh commands for managing checkpoints
>> in isolation. More commands are needed for performing incremental
>> backups, but these commands were easy to implement by modeling
>> heavily after virsh-snapshot.c (no need for checkpoint-revert,
>> and checkpoint-list was a lot easier since we don't have to cater
>> to older libvirt API).
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>

>> +    if (virshLookupCheckpoint(ctl, cmd, "checkpointname", true, dom,
>> +                              &checkpoint, &name) < 0)
>> +        goto cleanup;
>> +
>> +    vshPrint(ctl, "%-15s %s\n", _("Name:"), name);
>> +    vshPrint(ctl, "%-15s %s\n", _("Domain:"), virDomainGetName(dom));
> 
> We have vshTableNew and vshTableRowAppend that can compute the
> indentation at run-time regardless of the locale.

Nice! Those were added after I forked from snapshot code 6 months ago. I
wonder if I can get some code sharing between the two in virsh the way I
did with code sharing in src/conf/.


>> +
>> +    if (tree) {
>> +        for (i = 0; i < chklist->nchks; i++) {
>> +            if (!chklist->chks[i].parent &&
>> +                vshTreePrint(ctl, virshCheckpointListLookup, chklist,
>> +                             chklist->nchks, i) < 0)
>> +                goto cleanup;
>> +        }
>> +        ret = true;
>> +        goto cleanup;
>> +    }
>> +
>> +    for (i = 0; i < chklist->nchks; i++) {
> 
> Putting this whole loop in a separate function would make this function
> more readable - both tree and non-tree would share the same 'ret = true'
> assignment and, more importantly, all the XML parsing stuff is only
> needed for non-tree printing.

Technical debt. I should fix snapshots to be smarter first, and then
copy the smarter solution first-try into checkpoints.

> 
>> +        const char *chk_name;
>> +
>> +        /* free up memory from previous iterations of the loop */
> 
> Sounds like a job for VIR_AUTOFREE

Yes, quite a few places. Point them out, and I'll gladly use it.


>> +char **
>> +virshCheckpointNameCompleter(vshControl *ctl,
>> +                             const vshCmd *cmd,
>> +                             unsigned int flags)
>> +{
>> +    virshControlPtr priv = ctl->privData;
>> +    virDomainPtr dom = NULL;
>> +    virDomainCheckpointPtr *checkpoints = NULL;
>> +    int ncheckpoints = 0;
>> +    size_t i = 0;
>> +    char **ret = NULL;
>> +
>> +    virCheckFlags(0, NULL);
>> +
>> +    if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
>> +        return NULL;
>> +
>> +    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
>> +        return NULL;
>> +
>> +    if ((ncheckpoints = virDomainListAllCheckpoints(dom, &checkpoints,
>> +                                                    flags)) < 0)
> 
> I recommend adding an 'int rc' variable and only assigning the result to
> ncheckpoints if it's non-negative,

Makes sense.

> 
>> +        goto error;
>> +
>> +    if (VIR_ALLOC_N(ret, ncheckpoints + 1) < 0)
>> +        goto error;
>> +
>> +    for (i = 0; i < ncheckpoints; i++) {
>> +        const char *name = virDomainCheckpointGetName(checkpoints[i]);
>> +
>> +        if (VIR_STRDUP(ret[i], name) < 0)
>> +            goto error;
>> +
>> +        virshDomainCheckpointFree(checkpoints[i]);
>> +    }
>> +    VIR_FREE(checkpoints);
>> +    virshDomainFree(dom);
>> +
>> +    return ret;
>> +
>> + error:
>> +    for (; i < ncheckpoints; i++)
>> +        virshDomainCheckpointFree(checkpoints[i]);
> 
> otherwise this loop will try to run for a long time.
> Also, given that we free these on success too, having a shared 'cleanup'
> path would be nicer.

Snapshots has the same bug, looks like fixing that is obvious 5.2
material, regardless of the decision on the rest of this series.

(I had no idea I'd be encountering so much yak's hair when I used
snapshots as my starting point...)

-- 
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

On 3/27/19 10:43 AM, Eric Blake wrote:
> On 3/27/19 9:29 AM, Ján Tomko wrote:
>> On Wed, Mar 27, 2019 at 05:10:46AM -0500, Eric Blake wrote:
>>> Introduce a bunch of new virsh commands for managing checkpoints
>>> in isolation. More commands are needed for performing incremental
>>> backups, but these commands were easy to implement by modeling
>>> heavily after virsh-snapshot.c (no need for checkpoint-revert,
>>> and checkpoint-list was a lot easier since we don't have to cater
>>> to older libvirt API).
>>>
>>> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
>>> +    if (virshLookupCheckpoint(ctl, cmd, "checkpointname", true, dom,
>>> +                              &checkpoint, &name) < 0)
>>> +        goto cleanup;
>>> +
>>> +    vshPrint(ctl, "%-15s %s\n", _("Name:"), name);
>>> +    vshPrint(ctl, "%-15s %s\n", _("Domain:"), virDomainGetName(dom));
>>
>> We have vshTableNew and vshTableRowAppend that can compute the
>> indentation at run-time regardless of the locale.
> 
> Nice! Those were added after I forked from snapshot code 6 months ago. I
> wonder if I can get some code sharing between the two in virsh the way I
> did with code sharing in src/conf/.

I also seem to recall seeing recent code changes to output '-' instead
of a blank column, such as for a 'Parent' column vs. roots with no
parents. Snapshot code doesn't appear to be doing that yet; if it does
start doing that, checkpoints should do likewise.

-- 
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