[libvirt] [PATCH v4 13/20] backup: Implement virsh support for checkpoints
John Ferlan
jferlan at redhat.com
Tue Feb 12 19:15:43 UTC 2019
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 at 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 | 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 at gmail.com>
> ##
> ## 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 at redhat.com>
> + * Karel Zak <kzak at redhat.com>
> + * Daniel P. Berrange <berrange at 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"));
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},
>
More information about the libvir-list
mailing list