On 2/6/19 2:18 PM, Eric Blake wrote:
> Introduce a few more new public APIs related to incremental backups.
> This builds on the previous notion of a checkpoint (without an
> existing checkpoint, the new API is a full backup, differing only
> from virDomainCopy in the point of time chosen); and also allows
> creation of a new checkpoint at the same time as starting the backup
> (after all, an incremental backup is only useful if it covers the
> state since the previous backup). It also enhances event reporting
> for signaling when a push model backup completes (where the
> hypervisor creates the backup); note that the pull model does not
> have an event (starting the backup lets a third party access the
> data, and only the third party knows when it is finished).
>
> The full list of new API:
> virDomainBackupBegin;
> virDomainBackupEnd;
> virDomainBackupGetXMLDesc;
>
> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
> ---
> include/libvirt/libvirt-domain-checkpoint.h | 21 ++
> include/libvirt/libvirt-domain.h | 17 +-
> src/driver-hypervisor.h | 14 ++
> src/qemu/qemu_blockjob.h | 1 +
> examples/object-events/event-test.c | 3 +
> src/conf/domain_conf.c | 2 +-
> src/libvirt-domain-checkpoint.c | 202 ++++++++++++++++++++
> src/libvirt-domain.c | 8 +-
> src/libvirt_public.syms | 3 +
> tools/virsh-domain.c | 8 +-
> 10 files changed, 273 insertions(+), 6 deletions(-)
>
> diff --git a/include/libvirt/libvirt-domain-checkpoint.h b/include/libvirt/libvirt-domain-checkpoint.h
> index 9006a46c6e..6c3a85a1bf 100644
> --- a/include/libvirt/libvirt-domain-checkpoint.h
> +++ b/include/libvirt/libvirt-domain-checkpoint.h
> @@ -152,4 +152,25 @@ int virDomainCheckpointDelete(virDomainCheckpointPtr checkpoint,
> int virDomainCheckpointRef(virDomainCheckpointPtr checkpoint);
> int virDomainCheckpointFree(virDomainCheckpointPtr checkpoint);
>
> +typedef enum {
> + VIR_DOMAIN_BACKUP_BEGIN_NO_METADATA = (1 << 0), /* Make checkpoint without
> + remembering it */
> + /* TODO: VIR_DOMAIN_BACKUP_BEGIN_QUIESCE */
Implement or drop TODO:
> +} virDomainBackupBeginFlags;
> +
> +/* Begin an incremental backup job, possibly creating a checkpoint. */
> +int virDomainBackupBegin(virDomainPtr domain, const char *diskXml,
> + const char *checkpointXml, unsigned int flags);
> +
> +/* Learn about an ongoing backup job. */
> +char *virDomainBackupGetXMLDesc(virDomainPtr domain, int id,
> + unsigned int flags);
> +
> +typedef enum {
> + VIR_DOMAIN_BACKUP_END_ABORT = (1 << 0), /* Abandon a push model backup */
> +} virDomainBackupEndFlags;
> +
> +/* Complete (or abort) an incremental backup job. */
> +int virDomainBackupEnd(virDomainPtr domain, int id, unsigned int flags);
> +
> #endif /* LIBVIRT_DOMAIN_CHECKPOINT_H */
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 0388911ded..cc23029c97 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -3,7 +3,7 @@
> * Summary: APIs for management of domains
> * Description: Provides APIs for the management of domains
> *
> - * Copyright (C) 2006-2015 Red Hat, Inc.
> + * Copyright (C) 2006-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
> @@ -2394,6 +2394,9 @@ typedef enum {
> * exists as long as sync is active */
> VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT = 4,
>
> + /* Backup (virDomainBackupBegin), job exists until virDomainBackupEnd */
> + VIR_DOMAIN_BLOCK_JOB_TYPE_BACKUP = 5,
> +
> # ifdef VIR_ENUM_SENTINELS
> VIR_DOMAIN_BLOCK_JOB_TYPE_LAST
> # endif
> @@ -3215,6 +3218,7 @@ typedef enum {
> VIR_DOMAIN_JOB_OPERATION_SNAPSHOT = 6,
> VIR_DOMAIN_JOB_OPERATION_SNAPSHOT_REVERT = 7,
> VIR_DOMAIN_JOB_OPERATION_DUMP = 8,
> + VIR_DOMAIN_JOB_OPERATION_BACKUP = 9,
>
> # ifdef VIR_ENUM_SENTINELS
> VIR_DOMAIN_JOB_OPERATION_LAST
> @@ -3230,6 +3234,14 @@ typedef enum {
> */
> # define VIR_DOMAIN_JOB_OPERATION "operation"
>
> +/**
> + * VIR_DOMAIN_JOB_ID:
> + *
> + * virDomainGetJobStats field: the id of the job (so far, only for jobs
> + * started by virDomainBackupBegin()), as VIR_TYPED_PARAM_INT.
> + */
> +# define VIR_DOMAIN_JOB_ID "id"
> +
> /**
> * VIR_DOMAIN_JOB_TIME_ELAPSED:
> *
> @@ -4054,7 +4066,8 @@ typedef void (*virConnectDomainEventMigrationIterationCallback)(virConnectPtr co
> * @nparams: size of the params array
> * @opaque: application specific data
> *
> - * This callback occurs when a job (such as migration) running on the domain
> + * This callback occurs when a job (such as migration or push-model
> + * virDomainBackupBegin()) running on the domain
> * is completed. The params array will contain statistics of the just completed
> * job as virDomainGetJobStats would return. The callback must not free @params
> * (the array will be freed once the callback finishes).
> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
> index 14db8e49f3..4e65b8ad36 100644
> --- a/src/driver-hypervisor.h
> +++ b/src/driver-hypervisor.h
> @@ -1376,6 +1376,17 @@ typedef int
> (*virDrvDomainCheckpointDelete)(virDomainCheckpointPtr checkpoint,
> unsigned int flags);
>
> +typedef int
> +(*virDrvDomainBackupBegin)(virDomainPtr domain, const char *diskXml,
> + const char *checkpointXml, unsigned int flags);
> +
> +typedef char *
> +(*virDrvDomainBackupGetXMLDesc)(virDomainPtr domain, int id,
> + unsigned int flags);
> +
> +typedef int
> +(*virDrvDomainBackupEnd)(virDomainPtr domain, int id, unsigned int flags);
> +
> typedef struct _virHypervisorDriver virHypervisorDriver;
> typedef virHypervisorDriver *virHypervisorDriverPtr;
>
> @@ -1638,6 +1649,9 @@ struct _virHypervisorDriver {
> virDrvDomainCheckpointIsCurrent domainCheckpointIsCurrent;
> virDrvDomainCheckpointHasMetadata domainCheckpointHasMetadata;
> virDrvDomainCheckpointDelete domainCheckpointDelete;
> + virDrvDomainBackupBegin domainBackupBegin;
> + virDrvDomainBackupGetXMLDesc domainBackupGetXMLDesc;
> + virDrvDomainBackupEnd domainBackupEnd;
> };
>
>
> diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h
> index c7325c6daf..95f53dde16 100644
> --- a/src/qemu/qemu_blockjob.h
> +++ b/src/qemu/qemu_blockjob.h
> @@ -55,6 +55,7 @@ typedef enum {
> QEMU_BLOCKJOB_TYPE_COPY = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY,
> QEMU_BLOCKJOB_TYPE_COMMIT = VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT,
> QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT = VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT,
> + QEMU_BLOCKJOB_TYPE_BACKUP = VIR_DOMAIN_BLOCK_JOB_TYPE_BACKUP,
> /* Additional enum values local to qemu */
> QEMU_BLOCKJOB_TYPE_INTERNAL,
> QEMU_BLOCKJOB_TYPE_LAST
> diff --git a/examples/object-events/event-test.c b/examples/object-events/event-test.c
> index fcf4492470..98337ad185 100644
> --- a/examples/object-events/event-test.c
> +++ b/examples/object-events/event-test.c
> @@ -891,6 +891,9 @@ blockJobTypeToStr(int type)
>
> case VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT:
> return "active layer block commit";
> +
> + case VIR_DOMAIN_BLOCK_JOB_TYPE_BACKUP:
> + return "backup";
> }
>
> return "unknown";
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 0221eb0634..fa3db9266f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1030,7 +1030,7 @@ VIR_ENUM_IMPL(virDomainHPTResizing,
> * <mirror> XML (remaining types are not two-phase). */
> VIR_ENUM_DECL(virDomainBlockJob);
> VIR_ENUM_IMPL(virDomainBlockJob, VIR_DOMAIN_BLOCK_JOB_TYPE_LAST,
> - "", "", "copy", "", "active-commit",
> + "", "", "copy", "", "active-commit", "",
> );
>
> VIR_ENUM_IMPL(virDomainMemoryModel,
> diff --git a/src/libvirt-domain-checkpoint.c b/src/libvirt-domain-checkpoint.c
> index 8a7b5b3c56..29cefe41f1 100644
> --- a/src/libvirt-domain-checkpoint.c
> +++ b/src/libvirt-domain-checkpoint.c
> @@ -721,3 +721,205 @@ virDomainCheckpointFree(virDomainCheckpointPtr checkpoint)
> virObjectUnref(checkpoint);
> return 0;
> }
Dirtying the namespace of libvirt-domain-checkpoint w/ Backup. Why not a
separate libvirt-domain-backup - beyond the obvious need to alter more
stuff of course. If kept here would seem to need to alter line #2 way
back up at the top to include virDomainBackupPtr API's too.
> +
> +
> +/**
> + * virDomainBackupBegin:
> + * @domain: a domain object
> + * @diskXml: description of storage to utilize and expose during
> + * the backup, or NULL
> + * @checkpointXml: description of a checkpoint to create, or NULL
> + * @flags: bitwise-OR of supported virDomainBackupBeginFlags
> + *
> + * Start a point-in-time backup job for the specified disks of a
> + * running domain.
> + *
> + * A backup job is mutually exclusive with domain migration
> + * (particularly when the job sets up an NBD export, since it is not
> + * possible to tell any NBD clients about a server migrating between
> + * hosts). For now, backup jobs are also mutually exclusive with any
> + * other block job on the same device, although this restriction may
> + * be lifted in a future release. Progress of the backup job can be
> + * tracked via virDomainGetJobStats(). The job remains active until a
> + * subsequent call to virDomainBackupEnd(), even if it no longer has
> + * anything to copy.
> + *
> + * This API differs from virDomainBlockCopy() in that it can grab the
> + * state of more than one disk in parallel, and the state is captured
s/, and/ and/
> + * as of the start of the job, rather than the end.
> + *
> + * There are two fundamental backup approaches. The first, called a
> + * push model, instructs the hypervisor to copy the state of the guest
> + * disk to the designated storage destination (which may be on the
> + * local file system or a network device); in this mode, the
s/; in/. In/
> + * hypervisor writes the content of the guest disk to the destination,
> + * then emits VIR_DOMAIN_EVENT_ID_JOB_COMPLETED when the backup is
> + * either complete or failed (the backup image is invalid if the job
> + * is ended prior to the event being emitted). The second, called a
s/is ended/ends/ (or fails ?)
> + * pull model, instructs the hypervisor to expose the state of the
> + * guest disk over an NBD export; a third-party client can then
s/; a/. A/
> + * connect to this export, and read whichever portions of the disk it
s/, and/ and/
> + * desires. In this mode, there is no event; libvirt has to be
> + * informed when the third-party NBD client is done and the backup
> + * resources can be released.
> + *
> + * The @diskXml parameter is optional but usually provided, and
s/, and/ and/
> + * contains details about the backup, including which backup mode to
> + * use, whether the backup is incremental from a previous checkpoint,
> + * which disks participate in the backup, the destination for a push
> + * model backup, and the temporary storage and NBD server details for
> + * a pull model backup. If omitted, the backup attempts to default to
> + * a push mode full backup of all disks, where libvirt generates a
> + * filename for each disk by appending a suffix of a timestamp in
> + * seconds since the Epoch. virDomainBackupGetXMLDesc() can be called
> + * to learn actual values selected. For more information, see
> + * formatcheckpoint.html#BackupAttributes.
> + *
> + * The @checkpointXml parameter is optional; if non-NULL, then libvirt
> + * behaves as if virDomainCheckpointCreateXML() were called with
> + * @checkpointXml and the flag VIR_DOMAIN_BACKUP_BEGIN_NO_METADATA
> + * forwarded appropriately, atomically covering the same guest state
s/, / and /
> + * that will be part of the backup. The creation of a new checkpoint
> + * allows for future incremental backups. Note that some hypervisors
> + * may require a particular disk format, such as qcow2, in order to
> + * take advantage of checkpoints, while allowing arbitrary formats
> + * if checkpoints are not involved.
> + *
> + * Returns a non-negative job id on success, or negative on failure.
s/, or/ or/
> + * This operation returns quickly, such that a user can choose to
> + * start a backup job between virDomainFSFreeze() and
> + * virDomainFSThaw() in order to create the backup while guest I/O is
> + * quiesced.
> + */
> +/* FIXME: Do we need a specific API for listing all current backup
> + * jobs (which, at the moment, is at most one job), or is it better to
> + * refactor other existing job APIs in libvirt-domain.c to have job-id
> + * counterparts along with a generic listing of all jobs (with flags
> + * for filtering to specific job types)?
> + */
Or do we wait until some consumer asks for this? Do you mean
GetBlockJobInfo? GetJobStats? GetJobInfo?
Is it required for initial implementation? Not sure I'm expert enough
to answer that questions!
> +int
> +virDomainBackupBegin(virDomainPtr domain, const char *diskXml,
> + const char *checkpointXml, unsigned int flags)
One line per argument
> +{
> + virConnectPtr conn;
> +
> + VIR_DOMAIN_DEBUG(domain, "diskXml=%s, checkpointXml=%s, flags=0x%x",
> + NULLSTR(diskXml), NULLSTR(checkpointXml), flags);
> +
> + virResetLastError();
> +
> + virCheckDomainReturn(domain, -1);
> + conn = domain->conn;
> +
> + virCheckReadOnlyGoto(conn->flags, error);
> + if (flags & VIR_DOMAIN_BACKUP_BEGIN_NO_METADATA)
> + virCheckNonNullArgGoto(checkpointXml, error);
> +
> + if (conn->driver->domainBackupBegin) {
> + int ret;
> + ret = conn->driver->domainBackupBegin(domain, diskXml, checkpointXml,
> + flags);
> + if (!ret)
> + goto error;
> + return ret;
> + }
> +
> + virReportUnsupportedError();
> + error:
> + virDispatchError(conn);
> + return -1;
> +}
> +
> +
> +/**
> + * virDomainBackupGetXMLDesc:
> + * @domain: a domain object
> + * @id: the id of an active backup job previously started with
> + * virDomainBackupBegin()
> + * @flags: extra flags; not used yet, so callers should always pass 0
> + *
> + * In some cases, a user can start a backup job without supplying all
> + * details, and rely on libvirt to fill in the rest (for example,
s/, and/ and/
> + * selecting the port used for an NBD export). This API can then be
> + * used to learn what default values were chosen.
> + *
> + * Returns a NUL-terminated UTF-8 encoded XML instance, or NULL in
s/, or/ or/
> + * case of error. The caller must free() the returned value.
> + */
Do we have the same security concerns as Checkpoint? IOW: One doesn't
necessarily have to use a conn that was used for virDomainBackupBegin or
do they? Should we just check for a non read-only connection (which I
assume has implications later in remote.x defs).
> +char *
> +virDomainBackupGetXMLDesc(virDomainPtr domain, int id, unsigned int flags)
One line per argument
> +{
> + virConnectPtr conn;
> +
> + VIR_DOMAIN_DEBUG(domain, "id=%d, flags=0x%x", id, flags);
> +
> + virResetLastError();
> +
> + virCheckDomainReturn(domain, NULL);
> + conn = domain->conn;
> +
> + virCheckNonNegativeArgGoto(id, error);
> +
> + if (conn->driver->domainBackupGetXMLDesc) {
> + char *ret;
> + ret = conn->driver->domainBackupGetXMLDesc(domain, id, flags);
> + if (!ret)
> + goto error;
> + return ret;
> + }
> +
> + virReportUnsupportedError();
> + error:
> + virDispatchError(conn);
> + return NULL;
> +}
> +
> +
> +/**
> + * virDomainBackupEnd:
> + * @domain: a domain object
> + * @id: the id of an active backup job previously started with
> + * virDomainBackupBegin()
> + * @flags: bitwise-OR of supported virDomainBackupEndFlags
> + *
> + * Conclude a point-in-time backup job @id on the given domain.
> + *
> + * If the backup job uses the push model, but the event marking that
> + * all data has been copied has not yet been emitted, then the command
> + * fails unless @flags includes VIR_DOMAIN_BACKUP_END_ABORT. If the
> + * event has been issued, or if the backup uses the pull model, the
> + * flag has no effect.
> + *
> + * Returns 1 if the backup job completed successfully (the backup
> + * destination file in a push model is consistent), 0 if the job was
> + * aborted successfully (only when VIR_DOMAIN_BACKUP_END_ABORT is
> + * passed; the destination file is unusable), and -1 on failure.
> + */
So 0 can only be returned if/when ABORT is used (just checking my
reading...)
> +int
> +virDomainBackupEnd(virDomainPtr domain, int id, unsigned int flags)
One line per argument
John
> +{
> + virConnectPtr conn;
> +
> + VIR_DOMAIN_DEBUG(domain, "id=%d, flags=0x%x", id, flags);
> +
> + virResetLastError();
> +
> + virCheckDomainReturn(domain, -1);
> + conn = domain->conn;
> +
> + virCheckReadOnlyGoto(conn->flags, error);
> + virCheckNonNegativeArgGoto(id, error);
> +
> + if (conn->driver->domainBackupEnd) {
> + int ret;
> + ret = conn->driver->domainBackupEnd(domain, id, flags);
> + if (!ret)
> + goto error;
> + return ret;
> + }
> +
> + virReportUnsupportedError();
> + error:
> + virDispatchError(conn);
> + return -1;
> +}
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 58b4863c8f..a159b9d78c 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -1,7 +1,7 @@
> /*
> * libvirt-domain.c: entry points for virDomainPtr APIs
> *
> - * Copyright (C) 2006-2015 Red Hat, Inc.
> + * Copyright (C) 2006-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
> @@ -10325,6 +10325,12 @@ virDomainBlockRebase(virDomainPtr dom, const char *disk,
> * over the destination format, the ability to copy to a destination that
> * is not a local file, and the possibility of additional tuning parameters.
> *
> + * The copy created by this API is not finalized until the job ends,
> + * and does not lend itself to incremental backups (beyond what
> + * VIR_DOMAIN_BLOCK_COPY_SHALLOW provides) nor to third-party control
> + * over the data being copied. For those features, use
> + * virDomainBackupBegin().
> + *
> * Returns 0 if the operation has started, -1 on failure.
> */
> int
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 1b18402e5e..96858388d1 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -816,6 +816,9 @@ LIBVIRT_4.10.0 {
>
> LIBVIRT_5.1.0 {
> global:
> + virDomainBackupBegin;
> + virDomainBackupEnd;
> + virDomainBackupGetXMLDesc;
> virDomainCheckpointCreateXML;
> virDomainCheckpointCurrent;
> virDomainCheckpointDelete;
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 275ac0c318..68d7dc6df7 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -2561,7 +2561,9 @@ VIR_ENUM_IMPL(virshDomainBlockJob,
> N_("Block Pull"),
> N_("Block Copy"),
> N_("Block Commit"),
> - N_("Active Block Commit"));
> + N_("Active Block Commit"),
> + N_("Backup"),
> +);
>
> static const char *
> virshDomainBlockJobToString(int type)
> @@ -6064,7 +6066,9 @@ VIR_ENUM_IMPL(virshDomainJobOperation,
> N_("Outgoing migration"),
> N_("Snapshot"),
> N_("Snapshot revert"),
> - N_("Dump"));
> + N_("Dump"),
> + N_("Backup"),
> +);
>
> static const char *
> virshDomainJobOperationToString(int op)
>
