Re: [PATCH v7 16/23] backup: Implement virsh support for backup

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

 



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@xxxxxxxxxx>
> ---
>  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;

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