Introduce a few more new virsh commands for performing backup jobs, as
well as creating a checkpoint atomically with a snapshot.
At this time, I did not opt for a convenience command
'backup-begin-as' that cobbles together appropriate XML from the
user's command line arguments, but that may be a viable future
extension. Similarly, since backup is a potentially long-running
operation, it might be nice to add some sugar that automatically
handles waiting for the job to end, rather than making the user have
to poll or figure out virsh event to do the same.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tools/virsh-domain.c | 247 ++++++++++++++++++++++++++++++++++++++++-
tools/virsh-snapshot.c | 37 +++++-
tools/virsh.pod | 64 ++++++++++-
3 files changed, 337 insertions(+), 11 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 1970710c07..4ae456146b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1,7 +1,7 @@
/*
* virsh-domain.c: Commands to manage domain
*
- * Copyright (C) 2005, 2007-2016 Red Hat, Inc.
+ * Copyright (C) 2005-2019 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -14039,6 +14039,233 @@ cmdDomFSInfo(vshControl *ctl, const vshCmd *cmd)
return ret;
}
+
+/*
+ * "backup-begin" command
+ */
+static const vshCmdInfo info_backup_begin[] = {
+ {.name = "help",
+ .data = N_("Start a disk backup of a live domain")
+ },
+ {.name = "desc",
+ .data = N_("Use XML to start a full or incremental disk backup of a live "
+ "domain, optionally creating a checkpoint")
+ },
+ {.name = NULL}
+};
+
+static const vshCmdOptDef opts_backup_begin[] = {
+ VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+ {.name = "xmlfile",
+ .type = VSH_OT_STRING,
+ .help = N_("domain backup XML"),
+ },
+ {.name = "checkpointxml",
+ .type = VSH_OT_STRING,
+ .help = N_("domain checkpoint XML"),
+ },
+ {.name = "no-metadata",
+ .type = VSH_OT_BOOL,
+ .help = N_("create checkpoint but don't track metadata"),
+ },
+ {.name = "quiesce",
+ .type = VSH_OT_BOOL,
+ .help = N_("quiesce guest's file systems"),
+ },
+ /* TODO: --wait/--verbose/--timeout flags for push model backups? */
+ {.name = NULL}
+};
+
+static bool
+cmdBackupBegin(vshControl *ctl,
+ const vshCmd *cmd)
+{
+ virDomainPtr dom = NULL;
+ bool ret = false;
+ const char *backup_from = NULL;
+ char *backup_buffer = NULL;
+ const char *check_from = NULL;
+ char *check_buffer = NULL;
+ unsigned int flags = 0;
+ int id;
+
+ if (vshCommandOptBool(cmd, "no-metadata"))
+ flags |= VIR_DOMAIN_BACKUP_BEGIN_NO_METADATA;
+ if (vshCommandOptBool(cmd, "quiesce"))
+ flags |= VIR_DOMAIN_BACKUP_BEGIN_QUIESCE;
+
+ if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+ goto cleanup;
+
+ if (vshCommandOptStringReq(ctl, cmd, "xmlfile", &backup_from) < 0)
+ goto cleanup;
+ if (!backup_from) {
+ backup_buffer = vshStrdup(ctl, "<domainbackup/>");
+ } else {
+ if (virFileReadAll(backup_from, VSH_MAX_XML_FILE, &backup_buffer) < 0) {
+ vshSaveLibvirtError();
+ goto cleanup;
+ }
+ }
+
+ if (vshCommandOptStringReq(ctl, cmd, "checkpointxml", &check_from) < 0)
+ goto cleanup;
+ if (check_from) {
+ if (virFileReadAll(check_from, VSH_MAX_XML_FILE, &check_buffer) < 0) {
+ vshSaveLibvirtError();
+ goto cleanup;
+ }
+ }
+
+ id = virDomainBackupBegin(dom, backup_buffer, check_buffer, flags);
+
+ if (id < 0)
+ goto cleanup;
+
+ vshPrint(ctl, _("Backup id %d started\n"), id);
+ if (backup_from)
+ vshPrintExtra(ctl, _("backup used description from '%s'\n"),
+ backup_from);
+ if (check_buffer)
+ vshPrintExtra(ctl, _("checkpoint created from '%s'\n"), check_from);
+
+ ret = true;
+
+ cleanup:
+ VIR_FREE(backup_buffer);
+ VIR_FREE(check_buffer);
+ virshDomainFree(dom);
+
+ return ret;
+}
+
+/* TODO: backup-begin-as? */
+
+/*
+ * "backup-dumpxml" command
+ */
+static const vshCmdInfo info_backup_dumpxml[] = {
+ {.name = "help",
+ .data = N_("Dump XML for an ongoing domain block backup job")
+ },
+ {.name = "desc",
+ .data = N_("Backup Dump XML")
+ },
+ {.name = NULL}
+};
+
+static const vshCmdOptDef opts_backup_dumpxml[] = {
+ VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+ {.name = "id",
+ .type = VSH_OT_INT,
+ .help = N_("backup job id"),
+ /* TODO: Add API for listing active jobs, then adding a completer? */
+ },
+ /* TODO - worth adding this flag?
+ {.name = "checkpoint",
+ .type = VSH_OT_BOOL,
+ .help = N_("if the backup created a checkpoint, also dump that XML")
+ },
+ */
+ {.name = NULL}
+};
+
+static bool
+cmdBackupDumpXML(vshControl *ctl,
+ const vshCmd *cmd)
+{
+ virDomainPtr dom = NULL;
+ bool ret = false;
+ char *xml = NULL;
+ unsigned int flags = 0;
+ int id = 0;
+
+ if (vshCommandOptBool(cmd, "security-info"))
+ flags |= VIR_DOMAIN_XML_SECURE;
+
+ if (vshCommandOptInt(ctl, cmd, "id", &id) < 0)
+ return false;
+
+ if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+ return false;
+
+ if (!(xml = virDomainBackupGetXMLDesc(dom, id, flags)))
+ goto cleanup;
+
+ vshPrint(ctl, "%s", xml);
+ ret = true;
+
+ cleanup:
+ VIR_FREE(xml);
+ virshDomainFree(dom);
+
+ return ret;
+}
+
+
+/*
+ * "backup-end" command
+ */
+static const vshCmdInfo info_backup_end[] = {
+ {.name = "help",
+ .data = N_("Conclude a disk backup of a live domain")
+ },
+ {.name = "desc",
+ .data = N_("End a domain block backup job")
+ },
+ {.name = NULL}
+};
+
+static const vshCmdOptDef opts_backup_end[] = {
+ VIRSH_COMMON_OPT_DOMAIN_FULL(0),
+ {.name = "id",
+ .type = VSH_OT_INT,
+ .help = N_("backup job id"),
+ /* TODO: Add API for listing active jobs, then adding a completer? */
+ },
+ {.name = "abort",
+ .type = VSH_OT_BOOL,
+ .help = N_("abandon a push model backup that has not yet completed")
+ },
+ {.name = NULL}
+};
+
+static bool
+cmdBackupEnd(vshControl *ctl, const vshCmd *cmd)
+{
+ virDomainPtr dom = NULL;
+ bool ret = false;
+ unsigned int flags = 0;
+ int id = 0;
+ int rc;
+
+ if (vshCommandOptBool(cmd, "abort"))
+ flags |= VIR_DOMAIN_BACKUP_END_ABORT;
+
+ if (vshCommandOptInt(ctl, cmd, "id", &id) < 0)
+ return false;
+
+ if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+ goto cleanup;
+
+ rc = virDomainBackupEnd(dom, id, flags);
+
+ if (rc < 0)
+ goto cleanup;
+ if (rc == 0)
+ vshPrint(ctl, _("Backup id %d aborted"), id);
+ else
+ vshPrint(ctl, _("Backup id %d completed"), id);
+
+ ret = true;
+
+ cleanup:
+ virshDomainFree(dom);
+
+ return ret;
+}
+
+
const vshCmdDef domManagementCmds[] = {
{.name = "attach-device",
.handler = cmdAttachDevice,
@@ -14064,6 +14291,24 @@ const vshCmdDef domManagementCmds[] = {
.info = info_autostart,
.flags = 0
},
+ {.name = "backup-begin",
+ .handler = cmdBackupBegin,
+ .opts = opts_backup_begin,
+ .info = info_backup_begin,
+ .flags = 0
+ },
+ {.name = "backup-dumpxml",
+ .handler = cmdBackupDumpXML,
+ .opts = opts_backup_dumpxml,
+ .info = info_backup_dumpxml,
+ .flags = 0
+ },
+ {.name = "backup-end",
+ .handler = cmdBackupEnd,
+ .opts = opts_backup_end,
+ .info = info_backup_end,
+ .flags = 0
+ },
{.name = "blkdeviotune",
.handler = cmdBlkdeviotune,
.opts = opts_blkdeviotune,
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index f6bb38bc96..57ab80251b 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -40,18 +40,26 @@
/* Helper for snapshot-create and snapshot-create-as */
static bool
-virshSnapshotCreate(vshControl *ctl, virDomainPtr dom, const char *buffer,
- unsigned int flags, const char *from)
+virshSnapshotCreate(vshControl *ctl,
+ virDomainPtr dom,
+ const char *buffer,
+ const char *check_buffer,
+ unsigned int flags,
+ const char *from)
{
bool ret = false;
virDomainSnapshotPtr snapshot;
bool halt = false;
const char *name = NULL;
- snapshot = virDomainSnapshotCreateXML(dom, buffer, flags);
+ if (check_buffer)
+ snapshot = virDomainSnapshotCreateXML2(dom, buffer, check_buffer,
+ flags);
+ else
+ snapshot = virDomainSnapshotCreateXML(dom, buffer, flags);
/* Emulate --halt on older servers. */
- if (!snapshot && last_error->code == VIR_ERR_INVALID_ARG &&
+ if (!check_buffer && !snapshot && last_error->code == VIR_ERR_INVALID_ARG &&
(flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) {
int persistent;
@@ -117,6 +125,10 @@ static const vshCmdOptDef opts_snapshot_create[] = {
.type = VSH_OT_STRING,
.help = N_("domain snapshot XML")
},
+ {.name = "checkpointxml",
+ .type = VSH_OT_STRING,
+ .help = N_("domain checkpoint XML"),
+ },
{.name = "redefine",
.type = VSH_OT_BOOL,
.help = N_("redefine metadata for existing snapshot")
@@ -157,7 +169,9 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
bool ret = false;
const char *from = NULL;
char *buffer = NULL;
+ const char *check_from = NULL;
unsigned int flags = 0;
+ VIR_AUTOFREE(char *check_buffer) = NULL;
if (vshCommandOptBool(cmd, "redefine"))
flags |= VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE;
@@ -192,7 +206,18 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
}
}
- ret = virshSnapshotCreate(ctl, dom, buffer, flags, from);
+ if (vshCommandOptStringReq(ctl, cmd, "checkpointxml", &check_from) < 0)
+ goto cleanup;
+ if (check_from) {
+ if (virFileReadAll(check_from, VSH_MAX_XML_FILE, &check_buffer) < 0) {
+ vshSaveLibvirtError();
+ goto cleanup;
+ }
+ }
+
+ ret = virshSnapshotCreate(ctl, dom, buffer, check_buffer, flags, from);
+ if (ret && check_buffer)
+ vshPrintExtra(ctl, _("checkpoint created from '%s'\n"), check_from);
cleanup:
VIR_FREE(buffer);
@@ -434,7 +459,7 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
- ret = virshSnapshotCreate(ctl, dom, buffer, flags, NULL);
+ ret = virshSnapshotCreate(ctl, dom, buffer, NULL, flags, NULL);
cleanup:
virBufferFreeAndReset(&buf);
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 2f833ac184..e052976a4b 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -1348,6 +1348,55 @@ I<bandwidth> specifies copying bandwidth limit in MiB/s. For further information
on the I<bandwidth> argument see the corresponding section for the B<blockjob>
command.
+=item B<backup-begin> I<domain> [I<xmlfile>]
+[I<checkpointxml> [I<--no-metadata>]] [I<--quiesce>]
+
+Begin a new backup job, and output the resulting job id on success. If
+I<xmlfile> is omitted, this defaults to a full backup using a push
+model to filenames generated by libvirt; supplying XML allows
+fine-tuning such as requesting an incremental backup relative to an
+earlier checkpoint, controlling which disks participate or which
+filenames are involved, or requesting the use of a pull model backup.
+The B<backup-dumpxml> command shows any resulting values assigned by
+libvirt. For more information on backup XML, see:
+L<https://libvirt.org/formatbackup.html>.
+
+If I<checkpointxml> is specified, a second file with a top-level
+element of <domaincheckpoint> is used to create a simultaneous
+checkpoint, for doing a later incremental backup relative to the time
+the snapshot was created. If I<--no-metadata> is specified, then the
+checkpoint is created, but any metadata is immediately discarded. See
+B<checkpoint-create> for more details on checkpoints.
+
+If I<--quiesce> is specified, libvirt will try to use guest agent
+to freeze and unfreeze domain's mounted file systems. However,
+if domain has no guest agent, the backup job will fail.
+
+This command returns as soon as possible, and the backup job runs in
+the background; the progress of a push model backup can be checked
+with B<domjobinfo> or by waiting for an event with B<event> (the
+progress of a pull model backup is under the control of whatever third
+party connects to the NBD export). The job is ended with B<block-end>.
+
+=item B<backup-dumpxml> I<domain> [I<id>]
+
+Output XML describing the backup job I<id>. The default for I<id> is
+0, which works as long as there are no parallel jobs; it is also
+possible to use the positive id printed by B<backup-begin> on success.
+
+=item B<backup-end> I<domain> [I<id>] [I<--abort>]
+
+End the currentbackup job I<id>. The default for I<id> is 0, which
+works as long as there are no parallel jobs; it is also possible to
+use the positive id printed by B<backup-begin> on success.
+
+If the backup job is a push job, but the hypervisor is not yet
+complete, this command will fail unless I<--abort> is given; if
+aborted, the backup file is incomplete. If the backup job is a pull
+job, I<--abort> has no effect, because libvirt assumes the third-party
+client is done performing the backup.
+
+
=item B<blkdeviotune> I<domain> I<device>
[[I<--config>] [I<--live>] | [I<--current>]]
[[I<total-bytes-sec>] | [I<read-bytes-sec>] [I<write-bytes-sec>]]
@@ -4594,17 +4643,24 @@ used to represent properties of snapshots.
=over 4
=item B<snapshot-create> I<domain> [I<xmlfile>] {[I<--redefine> [I<--current>]]
-| [I<--no-metadata>] [I<--halt>] [I<--disk-only>] [I<--reuse-external>]
-[I<--quiesce>] [I<--atomic>] [I<--live>]}
+| [I<checkpointxml>] [I<--no-metadata>] [I<--halt>] [I<--disk-only>]
+[I<--reuse-external>] [I<--quiesce>] [I<--atomic>] [I<--live>]}
Create a snapshot for domain I<domain> with the properties specified in
-I<xmlfile>. Normally, the only properties settable for a domain snapshot
+I<xmlfile>, using a top-level element of <domainsnapshot>. Normally,
+the only properties settable for a domain snapshot
are the <name> and <description> elements, as well as <disks> if
I<--disk-only> is given; the rest of the fields are
ignored, and automatically filled in by libvirt. If I<xmlfile> is
completely omitted, then libvirt will choose a value for all fields.
The new snapshot will become current, as listed by B<snapshot-current>.
+If I<checkpointxml> is specified, a second file with a top-level
+element of <domaincheckpoint> is used to create a simultaneous
+checkpoint, for doing a later incremental backup relative to the time
+the snapshot was created. See B<checkpoint-create> for more details on
+checkpoints.
+
If I<--halt> is specified, the domain will be left in an inactive state
after the snapshot is created.
@@ -4630,7 +4686,7 @@ If I<--no-metadata> is specified, then the snapshot data is created,
but any metadata is immediately discarded (that is, libvirt does not
treat the snapshot as current, and cannot revert to the snapshot
unless I<--redefine> is later used to teach libvirt about the
-metadata again).
+metadata again). This flag also affects I<checkpointxml>.
If I<--reuse-external> is specified, and the snapshot XML requests an
external snapshot with a destination of an existing file, then the
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Mar 27, 2019 at 05:10:47 -0500, Eric Blake wrote:
> Introduce a few more new virsh commands for performing backup jobs, as
> well as creating a checkpoint atomically with a snapshot.
>
> At this time, I did not opt for a convenience command
> 'backup-begin-as' that cobbles together appropriate XML from the
> user's command line arguments, but that may be a viable future
> extension. Similarly, since backup is a potentially long-running
> operation, it might be nice to add some sugar that automatically
> handles waiting for the job to end, rather than making the user have
> to poll or figure out virsh event to do the same.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> tools/virsh-domain.c | 247 ++++++++++++++++++++++++++++++++++++++++-
> tools/virsh-snapshot.c | 37 +++++-
> tools/virsh.pod | 64 ++++++++++-
> 3 files changed, 337 insertions(+), 11 deletions(-)
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 1970710c07..4ae456146b 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -1,7 +1,7 @@
> /*
> * virsh-domain.c: Commands to manage domain
> *
> - * Copyright (C) 2005, 2007-2016 Red Hat, Inc.
> + * Copyright (C) 2005-2019 Red Hat, Inc.
This seems wrong. It's adding dates in the past. Ideally you disable
that script for libvirt altogether as it creates pointless churn.
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
[1] Using 'check' instead of 'checkpoint' in variable names may be
misleading below in multiple instances. It might evoke that it's
supposed to be checked rather than referring to a checkpoint.
> @@ -14039,6 +14039,233 @@ cmdDomFSInfo(vshControl *ctl, const vshCmd *cmd)
> return ret;
> }
>
> +
> +/*
> + * "backup-begin" command
> + */
> +static const vshCmdInfo info_backup_begin[] = {
> + {.name = "help",
> + .data = N_("Start a disk backup of a live domain")
> + },
> + {.name = "desc",
> + .data = N_("Use XML to start a full or incremental disk backup of a live "
> + "domain, optionally creating a checkpoint")
> + },
> + {.name = NULL}
> +};
> +
> +static const vshCmdOptDef opts_backup_begin[] = {
> + VIRSH_COMMON_OPT_DOMAIN_FULL(0),
> + {.name = "xmlfile",
> + .type = VSH_OT_STRING,
> + .help = N_("domain backup XML"),
> + },
> + {.name = "checkpointxml",
> + .type = VSH_OT_STRING,
> + .help = N_("domain checkpoint XML"),
> + },
> + {.name = "no-metadata",
> + .type = VSH_OT_BOOL,
> + .help = N_("create checkpoint but don't track metadata"),
> + },
> + {.name = "quiesce",
> + .type = VSH_OT_BOOL,
> + .help = N_("quiesce guest's file systems"),
> + },
> + /* TODO: --wait/--verbose/--timeout flags for push model backups? */
> + {.name = NULL}
> +};
> +
> +static bool
> +cmdBackupBegin(vshControl *ctl,
> + const vshCmd *cmd)
> +{
> + virDomainPtr dom = NULL;
> + bool ret = false;
> + const char *backup_from = NULL;
> + char *backup_buffer = NULL;
> + const char *check_from = NULL;
> + char *check_buffer = NULL;
[1]
> + unsigned int flags = 0;
> + int id;
> +
> + if (vshCommandOptBool(cmd, "no-metadata"))
> + flags |= VIR_DOMAIN_BACKUP_BEGIN_NO_METADATA;
> + if (vshCommandOptBool(cmd, "quiesce"))
> + flags |= VIR_DOMAIN_BACKUP_BEGIN_QUIESCE;
> +
> + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
> + goto cleanup;
> +
> + if (vshCommandOptStringReq(ctl, cmd, "xmlfile", &backup_from) < 0)
"backupxml"
> + goto cleanup;
> + if (!backup_from) {
> + backup_buffer = vshStrdup(ctl, "<domainbackup/>");
> + } else {
> + if (virFileReadAll(backup_from, VSH_MAX_XML_FILE, &backup_buffer) < 0) {
> + vshSaveLibvirtError();
> + goto cleanup;
> + }
> + }
> +
> + if (vshCommandOptStringReq(ctl, cmd, "checkpointxml", &check_from) < 0)
> + goto cleanup;
> + if (check_from) {
> + if (virFileReadAll(check_from, VSH_MAX_XML_FILE, &check_buffer) < 0) {
> + vshSaveLibvirtError();
> + goto cleanup;
> + }
> + }
> +
> + id = virDomainBackupBegin(dom, backup_buffer, check_buffer, flags);
> +
> + if (id < 0)
> + goto cleanup;
> +
> + vshPrint(ctl, _("Backup id %d started\n"), id);
> + if (backup_from)
> + vshPrintExtra(ctl, _("backup used description from '%s'\n"),
> + backup_from);
> + if (check_buffer)
> + vshPrintExtra(ctl, _("checkpoint created from '%s'\n"), check_from);
> +
> + ret = true;
> +
> + cleanup:
> + VIR_FREE(backup_buffer);
> + VIR_FREE(check_buffer);
> + virshDomainFree(dom);
> +
> + return ret;
> +}
> +
> +/* TODO: backup-begin-as? */
> +
> +/*
> + * "backup-dumpxml" command
> + */
> +static const vshCmdInfo info_backup_dumpxml[] = {
> + {.name = "help",
> + .data = N_("Dump XML for an ongoing domain block backup job")
> + },
> + {.name = "desc",
> + .data = N_("Backup Dump XML")
> + },
> + {.name = NULL}
> +};
> +
> +static const vshCmdOptDef opts_backup_dumpxml[] = {
> + VIRSH_COMMON_OPT_DOMAIN_FULL(0),
> + {.name = "id",
> + .type = VSH_OT_INT,
> + .help = N_("backup job id"),
> + /* TODO: Add API for listing active jobs, then adding a completer? */
> + },
> + /* TODO - worth adding this flag?
> + {.name = "checkpoint",
> + .type = VSH_OT_BOOL,
> + .help = N_("if the backup created a checkpoint, also dump that XML")
> + },
> + */
> + {.name = NULL}
> +};
> +
> +static bool
> +cmdBackupDumpXML(vshControl *ctl,
> + const vshCmd *cmd)
> +{
> + virDomainPtr dom = NULL;
> + bool ret = false;
> + char *xml = NULL;
> + unsigned int flags = 0;
> + int id = 0;
> +
> + if (vshCommandOptBool(cmd, "security-info"))
This option is not mentioned in opts_backup_dumpxml.
> + flags |= VIR_DOMAIN_XML_SECURE;
> +
> + if (vshCommandOptInt(ctl, cmd, "id", &id) < 0)
> + return false;
> +
> + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
> + return false;
> +
> + if (!(xml = virDomainBackupGetXMLDesc(dom, id, flags)))
> + goto cleanup;
> +
> + vshPrint(ctl, "%s", xml);
> + ret = true;
> +
> + cleanup:
> + VIR_FREE(xml);
> + virshDomainFree(dom);
> +
> + return ret;
> +}
[...]
> diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
> index f6bb38bc96..57ab80251b 100644
> --- a/tools/virsh-snapshot.c
> +++ b/tools/virsh-snapshot.c
> @@ -40,18 +40,26 @@
>
> /* Helper for snapshot-create and snapshot-create-as */
> static bool
> -virshSnapshotCreate(vshControl *ctl, virDomainPtr dom, const char *buffer,
> - unsigned int flags, const char *from)
> +virshSnapshotCreate(vshControl *ctl,
> + virDomainPtr dom,
> + const char *buffer,
> + const char *check_buffer,
> + unsigned int flags,
> + const char *from)
> {
> bool ret = false;
> virDomainSnapshotPtr snapshot;
> bool halt = false;
> const char *name = NULL;
>
> - snapshot = virDomainSnapshotCreateXML(dom, buffer, flags);
> + if (check_buffer)
[1]
> + snapshot = virDomainSnapshotCreateXML2(dom, buffer, check_buffer,
> + flags);
> + else
> + snapshot = virDomainSnapshotCreateXML(dom, buffer, flags);
>
> /* Emulate --halt on older servers. */
> - if (!snapshot && last_error->code == VIR_ERR_INVALID_ARG &&
> + if (!check_buffer && !snapshot && last_error->code == VIR_ERR_INVALID_ARG &&
> (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT)) {
> int persistent;
>
> @@ -117,6 +125,10 @@ static const vshCmdOptDef opts_snapshot_create[] = {
> .type = VSH_OT_STRING,
> .help = N_("domain snapshot XML")
> },
> + {.name = "checkpointxml",
> + .type = VSH_OT_STRING,
> + .help = N_("domain checkpoint XML"),
> + },
> {.name = "redefine",
> .type = VSH_OT_BOOL,
> .help = N_("redefine metadata for existing snapshot")
> @@ -157,7 +169,9 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
> bool ret = false;
> const char *from = NULL;
> char *buffer = NULL;
> + const char *check_from = NULL;
[...]
> unsigned int flags = 0;
> + VIR_AUTOFREE(char *check_buffer) = NULL;
VIR_AUTOFREE and legacy code is used inconsistently in this patch.
>
> if (vshCommandOptBool(cmd, "redefine"))
> flags |= VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 3/27/19 7:30 AM, Peter Krempa wrote:
> On Wed, Mar 27, 2019 at 05:10:47 -0500, Eric Blake wrote:
>> Introduce a few more new virsh commands for performing backup jobs, as
>> well as creating a checkpoint atomically with a snapshot.
>>
>> At this time, I did not opt for a convenience command
>> 'backup-begin-as' that cobbles together appropriate XML from the
>> user's command line arguments, but that may be a viable future
>> extension. Similarly, since backup is a potentially long-running
>> operation, it might be nice to add some sugar that automatically
>> handles waiting for the job to end, rather than making the user have
>> to poll or figure out virsh event to do the same.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>> tools/virsh-domain.c | 247 ++++++++++++++++++++++++++++++++++++++++-
>> tools/virsh-snapshot.c | 37 +++++-
>> tools/virsh.pod | 64 ++++++++++-
>> 3 files changed, 337 insertions(+), 11 deletions(-)
>>
>> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
>> index 1970710c07..4ae456146b 100644
>> --- a/tools/virsh-domain.c
>> +++ b/tools/virsh-domain.c
>> @@ -1,7 +1,7 @@
>> /*
>> * virsh-domain.c: Commands to manage domain
>> *
>> - * Copyright (C) 2005, 2007-2016 Red Hat, Inc.
>> + * Copyright (C) 2005-2019 Red Hat, Inc.
>
> This seems wrong. It's adding dates in the past. Ideally you disable
> that script for libvirt altogether as it creates pointless churn.
I'm always wary of not updating copyrights, but I also see the
complaints about possibly updating it incorrectly. Unless a lawyer gives
me a strong reason behind one way or the other, it's obvious enough that
many files have NOT been maintained after initial commit, so I can go
ahead and flip the kill switch in my .emacs file for auto-copyrights to
maintain status quo. I also don't mind scrubbing my series to remove the
churn I've already caused, if that makes you feel better (but for files
that my series DOES add, I will be listing dates starting from any file
I copied from, through 2019).
>
>> *
>> * This library is free software; you can redistribute it and/or
>> * modify it under the terms of the GNU Lesser General Public
>
> [1] Using 'check' instead of 'checkpoint' in variable names may be
> misleading below in multiple instances. It might evoke that it's
> supposed to be checked rather than referring to a checkpoint.
Fitting in 80 columns is harder with 'checkpoint', but I can do the
rename for legibility.
>> +
>> +static bool
>> +cmdBackupDumpXML(vshControl *ctl,
>> + const vshCmd *cmd)
>> +{
>> + virDomainPtr dom = NULL;
>> + bool ret = false;
>> + char *xml = NULL;
>> + unsigned int flags = 0;
>> + int id = 0;
>> +
>> + if (vshCommandOptBool(cmd, "security-info"))
>
> This option is not mentioned in opts_backup_dumpxml.
>
Rebase leftovers; as there is no VIR_DOMAIN_BACKUP_XML_SECURE, this flag
should be removed from the code.
>> + flags |= VIR_DOMAIN_XML_SECURE;
And this flags setting is bogus.
>
>> unsigned int flags = 0;
>> + VIR_AUTOFREE(char *check_buffer) = NULL;
>
> VIR_AUTOFREE and legacy code is used inconsistently in this patch.
I can try to use the new stuff in more places (new code introduction is
a good time; but this is also copy-and-paste from existing code; there's
also the issue of timing delays, where the longer my patches are
out-of-tree, the more the point they copied from has changed in the
meantime).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.