Re: [PATCH v9 08/13] backup: Implement virsh support for checkpoints

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux