On Sat, Jul 06, 2019 at 22:56:08 -0500, Eric Blake wrote:
> Introduce a bunch of new virsh commands for managing checkpoints in
> isolation. More commands are needed for performing incremental
> backups, but these commands were easy to implement by modeling heavily
> after virsh-snapshot.c. There is no need for checkpoint-revert or
> checkpoint-current since those snapshot APIs have no checkpoint
> counterpart. Similarly, it is not necessary to change which
> checkpoint is current when redefining from XML, since checkpoints
> expose whether they are current in the public XML (rather than the way
> snapshots did it behind the scenese). checkpoint-list is a bit
> simpler, in part because we don't have to cater to back-compat to
> older API. checkpoint-info is a bit trickier, because it requires
> parsing XML (maybe we'll want virDomainCheckpointIsCurrent() as an API
> after all).
>
> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
> ---
[...]
> diff --git a/tools/virsh-checkpoint.c b/tools/virsh-checkpoint.c
> new file mode 100644
> index 0000000000..8200687f8a
> --- /dev/null
> +++ b/tools/virsh-checkpoint.c
> @@ -0,0 +1,1226 @@
[...]
> +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")
> + },
> + {.name = "no-metadata",
> + .type = VSH_OT_BOOL,
> + .help = N_("create checkpoint but create no metadata")
> + },
This needs to be deleted. [1]
> + {.name = "quiesce",
> + .type = VSH_OT_BOOL,
> + .help = N_("quiesce guest's file systems")
> + },
> + {.name = NULL}
> +};
> +
> +static bool
> +cmdCheckpointCreate(vshControl *ctl,
> + const vshCmd *cmd)
> +{
> + virDomainPtr dom = NULL;
> + bool ret = false;
> + const char *from = NULL;
> + char *buffer = NULL;
> + unsigned int flags = 0;
> +
> + if (vshCommandOptBool(cmd, "redefine"))
> + flags |= VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE;
> + if (vshCommandOptBool(cmd, "no-metadata"))
> + flags |= VIR_DOMAIN_CHECKPOINT_CREATE_NO_METADATA;
[1]
[...]
> +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")
[1]
> +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"))
> + flags |= VIR_DOMAIN_CHECKPOINT_CREATE_NO_METADATA;
[1]
[...]
> +/* Helper for resolving {--current | --ARG name} into a checkpoint
> + * belonging to DOM. 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,
> + 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 (current && chkname) {
> + vshError(ctl, _("--%s and --current are mutually exclusive"), arg);
> + return -1;
> + }
> +
> + if (chkname) {
> + *chk = virDomainCheckpointLookupByName(dom, chkname, 0);
> + } else if (current) {
> + int count;
> + virDomainCheckpointPtr *checkpoints = NULL;
> +
> + count = virDomainListAllCheckpoints(dom, &checkpoints,
> + VIR_DOMAIN_CHECKPOINT_LIST_CURRENT);
> + if (count < 0)
> + return -1;
> + if (count == 0) {
> + vshError(ctl, _("domain has no current checkpoint"));
> + return -1;
> + }
> + if (count > 1) {
> + while (count-- > 0)
> + virDomainCheckpointFree(checkpoints[count]);
> + VIR_FREE(checkpoints);
> + vshError(ctl, _("domain has more than one current checkpoint"));
Ummm, so what's the point of all this? I'd rather not include any of
this if it's not done properly. [2]
> + return -1;
> + }
> + *chk = checkpoints[0];
> + VIR_FREE(checkpoints);
> + } else {
> + vshError(ctl, _("--%s or --current is required"), arg);
> + return -1;
> + }
> + if (!*chk) {
> + vshReportError(ctl);
> + return -1;
> + }
> +
> + *name = virDomainCheckpointGetName(*chk);
> + return 0;
> +}
[...]
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 4dffcafea0..8d69d349e9 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
[...]
> @@ -4655,6 +4669,10 @@ a persistent domain. However, for transient domains, snapshot
> metadata is silently lost when the domain quits running (whether
> by command such as B<destroy> or by internal guest action).
>
> +For now, it is not possible to create snapshots in a domain that has
> +checkpoints, although this restriction will be lifted in a future
> +release.
Next time please consider splitting out the tangentially related stuff
such as this or the changes to snapshot lists into separate commits to
improve review comfort.
> +
> =item B<snapshot-create-as> I<domain> {[I<--print-xml>]
> [I<--no-metadata>] [I<--halt>] [I<--reuse-external>]} [I<name>]
> [I<description>] [I<--disk-only> [I<--quiesce>]] [I<--atomic>]
[...]
> @@ -4892,6 +4914,191 @@ the data contents from that point in time.
>
> =back
>
> +=head1 CHECKPOINT COMMANDS
> +
> +The following commands manipulate domain checkpoints. Checkpoints serve as
> +a point in time to identify which portions of a guest's disks have changed
> +after that time, making it possible to perform incremental and differential
> +backups. Checkpoints are identified with a unique name. See
> +L<https://libvirt.org/formatcheckpoint.html> for documentation of the XML
> +format used to represent properties of checkpoints.
> +
> +=over 4
> +
> +=item B<checkpoint-create> I<domain> [I<xmlfile>] { I<--redefine>
> +| [I<--no-metadata>] [I<--quiesce>]}
> +
> +Create a checkpoint for domain I<domain> with the properties specified
> +in I<xmlfile> describing a <domaincheckpoint> top-level element. The
> +format of the input XML file will be validated against an internal RNG
> +schema (idential to using the L<virt-xml-validate(1)> tool). If
> +I<xmlfile> is completely omitted, then libvirt will create a
> +checkpoint with a name based on the current time.
> +
> +If I<--redefine> is specified, then all XML elements produced by
> +B<checkpoint-dumpxml> are valid; this can be used to migrate
> +checkpoint hierarchy from one machine to another, to recreate
> +hierarchy for the case of a transient domain that goes away and is
> +later recreated with the same name and UUID, or to make slight
> +alterations in the checkpoint metadata (such as host-specific aspects
> +of the domain XML embedded in the checkpoint). When this flag is
> +supplied, the I<xmlfile> argument is mandatory.
> +
> +If I<--no-metadata> is specified, then the checkpoint is only created
> +if the server does not require libvirt to track metadata for the
> +checkpoint (some hypervisors may always fail if this flag is
> +requested).
[1]
> +
> +If I<--quiesce> is specified, libvirt will try to use guest agent
> +to freeze and unfreeze domain's mounted file systems. However,
> +if domain has no guest agent, checkpoint creation will fail.
> +
> +Existence of checkpoint metadata will prevent attempts to B<undefine>
> +a persistent domain. However, for transient domains, checkpoint
> +metadata is silently lost when the domain quits running (whether
> +by command such as B<destroy> or by internal guest action).
> +
> +For now, it is not possible to create checkpoints in a domain that has
> +snapshots, although this restriction will be lifted in a future
> +release.
> +
> +=item B<checkpoint-create-as> I<domain> {[I<--print-xml>]
> +| [I<--no-metadata>]} [I<name>] [I<description>] [I<--quiesce>]
> +[I<--diskspec>] B<diskspec>]...
> +
> +Create a checkpoint for domain I<domain> with the given <name> and
> +<description>; if either value is omitted, libvirt will choose a
> +value. If I<--print-xml> is specified, then XML appropriate for
> +I<checkpoint-create> is output, rather than actually creating a
> +checkpoint.
> +
> +The I<--diskspec> option can be used to control which guest disks
> +participate in the checkpoint. This option can occur multiple times,
> +according to the number of <disk> elements in the domain xml. Each
> +<diskspec> is in the form B<disk[,checkpoint=type][,bitmap=name]>. A
> +literal I<--diskspec> must precede each B<diskspec> unless
> +all three of I<domain>, I<name>, and I<description> are also present.
> +For example, a diskspec of "vda,checkpoint=bitmap,bitmap=map1"
> +results in the following XML:
> + <disk name='vda' checkpoint='bitmap' bitmap='map1'/>
> +
> +If I<--quiesce> is specified, libvirt will try to use guest agent
> +to freeze and unfreeze domain's mounted file systems. However,
> +if domain has no guest agent, checkpoint creation will fail.
> +
> +If I<--no-metadata> is specified, then the checkpoint is only created
> +if the server does not require libvirt to track metadata for the
> +checkpoint (some hypervisors may always fail if this flag is
> +requested).
[1]
> +
> +For now, it is not possible to create checkpoints in a domain that has
> +snapshots, although this restriction will be lifted in a future
> +release.
> +
ACK if you get rid of [1], [2] and all uses of VIRSH_COMMON_OPT_CURRENT
in this patch.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
