On 2/6/19 2:18 PM, 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@xxxxxxxxxx> > --- > 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 | 1329 ++++++++++++++++++++++++++++++++++++++ > tools/virsh-completer.c | 52 +- > tools/virsh-util.c | 11 + > tools/virsh.c | 2 + > 10 files changed, 1433 insertions(+), 2 deletions(-) > create mode 100644 tools/virsh-checkpoint.h > create mode 100644 tools/virsh-checkpoint.c virsh.pod would be useful in order to figure out what everything means. This is just too much being added here to really give this a proper review. This really is a case where too much has been built up. Start with the basics and build up from there. I have similar things as previously w/r/t 2 blank lines between functions and one argument per line per method - all newer norms than when -snapshot was created. Also may as well use the VIR_AUTOFREE type functions for (char *) VIR_FREE's. And VIR_AUTOPTR(virString) as approriate > > 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 4563fd76ac..59a8582f4a 100644 > --- a/tools/virsh-completer.h > +++ b/tools/virsh-completer.h > @@ -71,6 +71,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 613c9a77f0..1153385d2a 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@xxxxxxxxx> > ## > ## This library is free software; you can redistribute it and/or > @@ -220,6 +220,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..cd08569813 > --- /dev/null > +++ b/tools/virsh-checkpoint.c > @@ -0,0 +1,1329 @@ > +/* > + * virsh-checkpoint.c: 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/>. > + * > + * Daniel Veillard <veillard@xxxxxxxxxx> > + * Karel Zak <kzak@xxxxxxxxxx> > + * Daniel P. Berrange <berrange@xxxxxxxxxx> > + * > + */ > + > +#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")); Could not find domain checkpoint '%s' would be better > + 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") > + }, > + /* TODO - worth adding this flag? > + {.name = "quiesce", > + .type = VSH_OT_BOOL, > + .help = N_("quiesce guest's file systems") > + }, > + */ More to do > + {.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; > + /* TODO > + if (vshCommandOptBool(cmd, "quiesce")) > + flags |= VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE; > + */ Need to address the TODO > + > + 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") > + }, > + /* TODO > + {.name = "quiesce", > + .type = VSH_OT_BOOL, > + .help = N_("quiesce guest's file systems") > + }, > + */ More todo > + {.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; > + } > + /* TODO > + if (vshCommandOptBool(cmd, "quiesce")) > + flags |= VIR_DOMAIN_CHECKPOINT_CREATE_QUIESCE; > + */ More things to do > + > + 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 > + */ Oh and this is *really* a scary thing to allow! Especially since IIRC the Assign function would just create a new one if a name didn't exist... There seems to be a few things that could happen without locks that could really mess things up. > +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; I need a better map. When creating a checkpoint a couple patches ago, the AssignDef would fail if the name already existed. This code seems to take existing XML with a name that would already exist. > + > + 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); True, so if I don't have one, but want to create one, then I cannot use this? > + 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)); Meaning we could have partial printing if the subequent fails > + > + /* 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 = virDomainCheckpointListChildren(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 = virDomainCheckpointListChildren(checkpoint, NULL, flags); > + if (count < 0) > + goto cleanup; > + vshPrint(ctl, "%-15s %d\n", _("Descendants:"), count); Is delineating children and desendants necessary? > + > + /* Metadata. */ > + metadata = virDomainCheckpointHasMetadata(checkpoint, 0); > + if (metadata >= 0) > + vshPrint(ctl, "%-15s %s\n", _("Metadata:"), > + metadata ? _("yes") : _("no")); Does the lack of children or failures therein mean we shouldn't print this. Ordering I suppose. > + > + 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 = virDomainCheckpointListChildren(from, &chks, flags); > + else > + count = virDomainListCheckpoints(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. */ Still mumbling about too many options. > + 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 = NULL} > +}; > + That's a lot of different options... I'm confused over multiple checkpoints without a parent. I would figure a checkpoint without a parent is the root or first checkpoint and children are essentially linear. But this tree concept just complicates things so much. > +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 (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); Similar thoughts here w/r/t parent being root or first checkpoint. > + 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} > +}; Like I noted above - my eyes and brain are in absolute overloaded mode. Hard to perform a solid/good review with just so much data. John > diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c > index e62226fc13..6918f215b4 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-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 > @@ -596,6 +596,56 @@ 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 = virDomainListCheckpoints(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-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}, >
