From: Eric Blake <eblake@redhat.com>
Introduce a few 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 from
virDomainBlockCopy in the point of time chosen and in operation on
multiple disks at once); 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).
A backup job also affects filtering a listing of domains, as well as
adding 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).
Since multiple backup jobs can be run in parallel in the future (well,
qemu doesn't support it yet, but we don't want to preclude the idea),
virDomainBackupBegin() returns a positive job id, and the id is also
visible in the backup XML. But until a future libvirt release adds a
bunch of APIs related to parallel job management where job ids will
actually matter, the documentation is also clear that job id 0 means
the 'currently running backup job' (provided one exists), for use in
virDomainBackupGetXMLDesc() and virDomainBackupEnd().
The full list of new APIs:
virDomainBackupBegin;
virDomainBackupEnd;
virDomainBackupGetXMLDesc;
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
include/libvirt/libvirt-domain.h | 26 ++++-
src/driver-hypervisor.h | 20 ++++
src/libvirt-domain-checkpoint.c | 7 +-
src/libvirt-domain.c | 191 +++++++++++++++++++++++++++++++
src/libvirt_public.syms | 8 ++
tools/virsh-domain.c | 4 +-
6 files changed, 252 insertions(+), 4 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index 22277b0a84..2d9f69f7d4 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -3267,6 +3267,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
@@ -3282,6 +3283,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:
*
@@ -4106,7 +4115,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).
@@ -4916,4 +4926,18 @@ int virDomainGetGuestInfo(virDomainPtr domain,
int *nparams,
unsigned int flags);
+
+int virDomainBackupBegin(virDomainPtr domain,
+ const char *backupXML,
+ const char *checkpointXML,
+ unsigned int flags);
+
+char *virDomainBackupGetXMLDesc(virDomainPtr domain,
+ int id,
+ unsigned int flags);
+
+int virDomainBackupEnd(virDomainPtr domain,
+ int id,
+ unsigned int flags);
+
#endif /* LIBVIRT_DOMAIN_H */
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index 015b2cd01c..cf69cf528d 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -1372,6 +1372,23 @@ typedef int
int *nparams,
unsigned int flags);
+
+typedef int
+(*virDrvDomainBackupBegin)(virDomainPtr domain,
+ const char *backupXML,
+ 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;
@@ -1632,4 +1649,7 @@ struct _virHypervisorDriver {
virDrvDomainCheckpointGetParent domainCheckpointGetParent;
virDrvDomainCheckpointDelete domainCheckpointDelete;
virDrvDomainGetGuestInfo domainGetGuestInfo;
+ virDrvDomainBackupBegin domainBackupBegin;
+ virDrvDomainBackupGetXMLDesc domainBackupGetXMLDesc;
+ virDrvDomainBackupEnd domainBackupEnd;
};
diff --git a/src/libvirt-domain-checkpoint.c b/src/libvirt-domain-checkpoint.c
index fa391f8a06..432c2d5a52 100644
--- a/src/libvirt-domain-checkpoint.c
+++ b/src/libvirt-domain-checkpoint.c
@@ -102,8 +102,11 @@ virDomainCheckpointGetConnect(virDomainCheckpointPtr checkpoint)
* @flags: bitwise-OR of supported virDomainCheckpointCreateFlags
*
* Create a new checkpoint using @xmlDesc, with a top-level
- * <domaincheckpoint> element, on a running @domain. Note that @xmlDesc
- * must validate against the <domaincheckpoint> XML schema.
+ * <domaincheckpoint> element, on a running @domain. Note that
+ * @xmlDesc must validate against the <domaincheckpoint> XML schema.
+ * Typically, it is more common to create a new checkpoint as part of
+ * kicking off a backup job with virDomainBackupBegin(); however, it
+ * is also possible to start a checkpoint without a backup.
*
* See <a href="formatcheckpoint.html#CheckpointAttributes">Checkpoint XML</a>
* for more details on @xmlDesc. In particular, some hypervisors may require
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index dcab179e6e..7564c9eee1 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -12497,3 +12497,194 @@ int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
virDispatchError(domain->conn);
return -1;
}
+
+
+/**
+ * virDomainBackupBegin:
+ * @domain: a domain object
+ * @backupXML: description of the requested backup
+ * @checkpointXML: description of a checkpoint to create or NULL
+ * @flags: unused; callers must pass 0
+ *
+ * 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.
+ *
+ * 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
+ * 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
+ * fails or virDomainBackupEnd() is used prior to the event being
+ * emitted).
+ *
+ * The second, called a pull model, instructs the hypervisor to expose
+ * the state of the guest disk over an NBD export. A third-party
+ * client can then connect to this export and read whichever portions
+ * of the disk it desires. In this mode, there is no event; libvirt
+ * has to be informed via virDomainBackupEnd() when the third-party
+ * NBD client is done and the backup resources can be released.
+ *
+ * The @backupXML parameter contains details about the backup in the top-level
+ * element <domainbackup> , 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.
+ *
+ * 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 to create
+ * a checkpoint atomically covering the same point in time as 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.
+ * This id is then passed to virDomainBackupGetXMLDesc() and
+ * virDomainBackupEnd().
+ */
+int
+virDomainBackupBegin(virDomainPtr domain,
+ const char *backupXML,
+ const char *checkpointXML,
+ unsigned int flags)
+{
+ virConnectPtr conn;
+
+ VIR_DOMAIN_DEBUG(domain, "backupXML=%s, checkpointXML=%s, flags=0x%x",
+ NULLSTR(backupXML), NULLSTR(checkpointXML), flags);
+
+ virResetLastError();
+
+ virCheckDomainReturn(domain, -1);
+ conn = domain->conn;
+
+ virCheckNonNullArgGoto(backupXML, error);
+ virCheckReadOnlyGoto(conn->flags, error);
+
+ if (conn->driver->domainBackupBegin) {
+ int ret;
+ ret = conn->driver->domainBackupBegin(domain, backupXML, checkpointXML,
+ flags);
+ if (ret < 0)
+ goto error;
+ return ret;
+ }
+
+ virReportUnsupportedError();
+ error:
+ virDispatchError(conn);
+ return -1;
+}
+
+
+/**
+ * virDomainBackupGetXMLDesc:
+ * @domain: a domain object
+ * @id: the id of an active backup job
+ * @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,
+ * selecting the port used for an NBD export). This API can then be
+ * used to learn what default values were chosen.
+ *
+ * @id can either be the return value of a previous virDomainBackupBegin().
+ *
+ * Returns a NUL-terminated UTF-8 encoded XML instance or NULL in
+ * case of error. The caller must free() the returned value.
+ */
+char *
+virDomainBackupGetXMLDesc(virDomainPtr domain,
+ int id,
+ unsigned int flags)
+{
+ 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
+ * @flags: unused; callers must pass 0
+ *
+ * Conclude a point-in-time backup job of the given domain.
+ *
+ * In case of a push model backup the backup is aborted and will be incomplete.
+ *
+ * This API is asynchronous thus a successful return does not indicate that the
+ * job was concluded yet.
+ *
+ * Returns 0 if the backup job termination was requested successfully, or -1 on
+ * failure.
+ */
+int
+virDomainBackupEnd(virDomainPtr domain,
+ int id,
+ unsigned int flags)
+{
+ 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 < 0)
+ goto error;
+ return ret;
+ }
+
+ virReportUnsupportedError();
+ error:
+ virDispatchError(conn);
+ return -1;
+}
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 40655fbbf5..bf7ef80741 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -858,7 +858,15 @@ LIBVIRT_5.7.0 {
} LIBVIRT_5.6.0;
LIBVIRT_5.8.0 {
+ global:
virConnectSetIdentity;
} LIBVIRT_5.7.0;
+LIBVIRT_5.9.0 {
+ global:
+ virDomainBackupBegin;
+ virDomainBackupEnd;
+ virDomainBackupGetXMLDesc;
+} LIBVIRT_5.8.0;
+
# .... define new API here using predicted next version number ....
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 3ba8451470..5ecc8a229c 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6080,7 +6080,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)
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-listOn Fri, Oct 18, 2019 at 06:11:15PM +0200, Peter Krempa wrote:
> From: Eric Blake <eblake@redhat.com>
>
> Introduce a few 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 from
> virDomainBlockCopy in the point of time chosen and in operation on
> multiple disks at once); 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).
>
> A backup job also affects filtering a listing of domains, as well as
> adding 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).
>
> Since multiple backup jobs can be run in parallel in the future (well,
> qemu doesn't support it yet, but we don't want to preclude the idea),
> virDomainBackupBegin() returns a positive job id, and the id is also
> visible in the backup XML. But until a future libvirt release adds a
> bunch of APIs related to parallel job management where job ids will
> actually matter, the documentation is also clear that job id 0 means
> the 'currently running backup job' (provided one exists), for use in
> virDomainBackupGetXMLDesc() and virDomainBackupEnd().
>
> The full list of new APIs:
> virDomainBackupBegin;
> virDomainBackupEnd;
> virDomainBackupGetXMLDesc;
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> include/libvirt/libvirt-domain.h | 26 ++++-
> src/driver-hypervisor.h | 20 ++++
> src/libvirt-domain-checkpoint.c | 7 +-
> src/libvirt-domain.c | 191 +++++++++++++++++++++++++++++++
> src/libvirt_public.syms | 8 ++
> tools/virsh-domain.c | 4 +-
> 6 files changed, 252 insertions(+), 4 deletions(-)
>
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 22277b0a84..2d9f69f7d4 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -3267,6 +3267,7 @@ typedef enum {
>
> +/**
> + * 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:
> *
> @@ -4106,7 +4115,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).
> @@ -4916,4 +4926,18 @@ int virDomainGetGuestInfo(virDomainPtr domain,
> int *nparams,
> unsigned int flags);
>
> +
> +int virDomainBackupBegin(virDomainPtr domain,
> + const char *backupXML,
> + const char *checkpointXML,
> + unsigned int flags);
> +
> +char *virDomainBackupGetXMLDesc(virDomainPtr domain,
> + int id,
> + unsigned int flags);
> +
> +int virDomainBackupEnd(virDomainPtr domain,
> + int id,
> + unsigned int flags);
So this is still using a plain integer job ID, which is a concern
wrt future extensibility.
Earlier in the year I queried whether we should turn the "job" into a
fully fledged object, using either a string or a UUID to identify it
uniquely.
https://www.redhat.com/archives/libvir-list/2019-March/msg01695.html
IOW having something like this:
typedef struct _virDomainJob virDomainJob;
typedef virDomainJob *virDomainJobPtr;
void virDomainJobFree(virDomainJobPtr job);
virDomainJobLookupByUUID(virDomainPtr job,
unsigned char *uuid);
int virDomainJobGetType(virDomainJobPtr job);
int virDomainJobGetUUID(virDomainJobPtr job,
unsigned char *uuid);
int virDomainJobGetUUIDString(virDomainJobPtr job,
char *uuidstr);
virDomainJobPtr virDomainBackupBegin(virDomainPtr domain,
const char *backupXML,
const char *checkpointXML,
unsigned int flags);
char *virDomainBackupGetXMLDesc(virDomainPtr domain,
virDomainJobPtr job,
unsigned int flags);
int virDomainBackupEnd(virDomainPtr domain,
virDomainJobPtr job,
unsigned int flags);
Privately we'd define
struct _virDomainJob {
unsigned char uuid[VIR_UUID_BUFLEN];
int type;
};
So we don't do a RPC call for virDomainJobGet{Type,UUID,UUIDString},
and we'd just serialize the uuid over the wire for the virDomainBackup
APIs I presume.
Currently we have a single domain block job that can be active at any time
and it would be desirable to fold that into the new API in some way.
We can either create new v2 APIs for existing methods making them return
a virDomainJobPtr, or we could reserve a well-known UUID exclusively
to refer to the single default domain block job, which the user can grab
via virDomainJobLookupByUUID().
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Nov 21, 2019 at 18:54:12 +0000, Daniel Berrange wrote:
> On Fri, Oct 18, 2019 at 06:11:15PM +0200, Peter Krempa wrote:
> > From: Eric Blake <eblake@redhat.com>
> >
> > Introduce a few 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 from
> > virDomainBlockCopy in the point of time chosen and in operation on
> > multiple disks at once); 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).
> >
> > A backup job also affects filtering a listing of domains, as well as
> > adding 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).
> >
> > Since multiple backup jobs can be run in parallel in the future (well,
> > qemu doesn't support it yet, but we don't want to preclude the idea),
> > virDomainBackupBegin() returns a positive job id, and the id is also
> > visible in the backup XML. But until a future libvirt release adds a
> > bunch of APIs related to parallel job management where job ids will
> > actually matter, the documentation is also clear that job id 0 means
> > the 'currently running backup job' (provided one exists), for use in
> > virDomainBackupGetXMLDesc() and virDomainBackupEnd().
> >
> > The full list of new APIs:
> > virDomainBackupBegin;
> > virDomainBackupEnd;
> > virDomainBackupGetXMLDesc;
> >
> > Signed-off-by: Eric Blake <eblake@redhat.com>
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > include/libvirt/libvirt-domain.h | 26 ++++-
> > src/driver-hypervisor.h | 20 ++++
> > src/libvirt-domain-checkpoint.c | 7 +-
> > src/libvirt-domain.c | 191 +++++++++++++++++++++++++++++++
> > src/libvirt_public.syms | 8 ++
> > tools/virsh-domain.c | 4 +-
> > 6 files changed, 252 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> > index 22277b0a84..2d9f69f7d4 100644
> > --- a/include/libvirt/libvirt-domain.h
> > +++ b/include/libvirt/libvirt-domain.h
> > @@ -3267,6 +3267,7 @@ typedef enum {
>
>
> >
> > +/**
> > + * 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:
> > *
> > @@ -4106,7 +4115,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).
> > @@ -4916,4 +4926,18 @@ int virDomainGetGuestInfo(virDomainPtr domain,
> > int *nparams,
> > unsigned int flags);
> >
> > +
> > +int virDomainBackupBegin(virDomainPtr domain,
> > + const char *backupXML,
> > + const char *checkpointXML,
> > + unsigned int flags);
> > +
> > +char *virDomainBackupGetXMLDesc(virDomainPtr domain,
> > + int id,
> > + unsigned int flags);
> > +
> > +int virDomainBackupEnd(virDomainPtr domain,
> > + int id,
> > + unsigned int flags);
>
> So this is still using a plain integer job ID, which is a concern
> wrt future extensibility.
My current plan is to go ahead with API based on this old one but with
no support for any parallel jobs. Basically the same thing but 'id'
argument removed.
This actually fits in with the original documentation which was already
ACKed for the virDomainBackupBegin API which said that the backup job
uses the domain async job infrastructure. This means that the
virDomainAbortJob and virDomainGetJobStats can be used to monitor the
blockjob. It also gives us the possibility to query the state of a
finised job by passing VIR_DOMAIN_JOB_STATS_COMPLETED to
virDomainGetJobStats. We also have an async event for reporting the job
state. I'll also consider removing virDomainBackupEnd for this
implementation as virDomainAbortJob should be enough.
I spoke with oVirt devs who are very keen on getting this API finished
and their requirements currently don't require any parallel jobs. In
fact they were abusing Eric's design which only ever returned the same
job ID so that they would not have to persist it.
Given that we'll need to deal with the domain job anyways for the better
job infra outlined below I don't think adding these APIs will be too
much of a burden in the interim so that we can appease oVirt's desire
for this feature and we'll have more time to design the new job
interface properly.
> Earlier in the year I queried whether we should turn the "job" into a
> fully fledged object, using either a string or a UUID to identify it
> uniquely.
>
> https://www.redhat.com/archives/libvir-list/2019-March/msg01695.html
>
> IOW having something like this:
Going forward I want this not only for the backup job but basically for
any long running operation. We needed this for a long time but of the
two long running job impls we have both are not flexible enough.
There is only one 'domain job' (migration/save/etc) and blockjobs are
bound to disks.
> typedef struct _virDomainJob virDomainJob;
> typedef virDomainJob *virDomainJobPtr;
I actually started some work on this but didn't get far yet. I used the
same name in my case, but I'm partially afraid that virDomainAbortJob
which would not be related to these objects will be mistaken for
actually working in this case.
Unfortunately I don't have any better idea.
> void virDomainJobFree(virDomainJobPtr job);
>
> virDomainJobLookupByUUID(virDomainPtr job,
> unsigned char *uuid);
>
> int virDomainJobGetType(virDomainJobPtr job);
> int virDomainJobGetUUID(virDomainJobPtr job,
> unsigned char *uuid);
> int virDomainJobGetUUIDString(virDomainJobPtr job,
> char *uuidstr);
>
>
> virDomainJobPtr virDomainBackupBegin(virDomainPtr domain,
> const char *backupXML,
> const char *checkpointXML,
> unsigned int flags);
I was thinking about a super-universal API so that we don't have to redo
all APIs for blockjobs. Something along
virDomainJobPtr virDomainJobBegin(virDomainPtr domain,
const char *jobxml,
unsigned int flags);
It would give us the flexibility to add new jobs and arguments for them
via XML which is more flexible (e.g. we could easily add a
virDomainBlockPull with the 'top' argument which is currently missing
but qemu started to support it some time ago). On the other hand that
seems a too prone to abuse.
As of the above I'm unsure about the tradeofs between flexibility and
too much flexibility in this case. But that's probably for a future
discussion.
>
> char *virDomainBackupGetXMLDesc(virDomainPtr domain,
> virDomainJobPtr job,
> unsigned int flags);
>
> int virDomainBackupEnd(virDomainPtr domain,
> virDomainJobPtr job,
> unsigned int flags);
>
> Privately we'd define
>
> struct _virDomainJob {
> unsigned char uuid[VIR_UUID_BUFLEN];
> int type;
> };
>
> So we don't do a RPC call for virDomainJobGet{Type,UUID,UUIDString},
> and we'd just serialize the uuid over the wire for the virDomainBackup
> APIs I presume.
>
> Currently we have a single domain block job that can be active at any time
> and it would be desirable to fold that into the new API in some way.
Yes exactly.
>
> We can either create new v2 APIs for existing methods making them return
> a virDomainJobPtr, or we could reserve a well-known UUID exclusively
> to refer to the single default domain block job, which the user can grab
> via virDomainJobLookupByUUID().
Yes I was thinking along the same lines. Given that we'll need to deal
with this anyways I don't feel as bad adding the backup job as a domain
job for now with all the quirks and also all the infrastructure a domain
job already provides and then dealing wit the domain jobs properly
later.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Nov 22, 2019 at 09:06:31AM +0100, Peter Krempa wrote:
> On Thu, Nov 21, 2019 at 18:54:12 +0000, Daniel Berrange wrote:
> > On Fri, Oct 18, 2019 at 06:11:15PM +0200, Peter Krempa wrote:
> > > From: Eric Blake <eblake@redhat.com>
> > >
> > > Introduce a few 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 from
> > > virDomainBlockCopy in the point of time chosen and in operation on
> > > multiple disks at once); 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).
> > >
> > > A backup job also affects filtering a listing of domains, as well as
> > > adding 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).
> > >
> > > Since multiple backup jobs can be run in parallel in the future (well,
> > > qemu doesn't support it yet, but we don't want to preclude the idea),
> > > virDomainBackupBegin() returns a positive job id, and the id is also
> > > visible in the backup XML. But until a future libvirt release adds a
> > > bunch of APIs related to parallel job management where job ids will
> > > actually matter, the documentation is also clear that job id 0 means
> > > the 'currently running backup job' (provided one exists), for use in
> > > virDomainBackupGetXMLDesc() and virDomainBackupEnd().
> > >
> > > The full list of new APIs:
> > > virDomainBackupBegin;
> > > virDomainBackupEnd;
> > > virDomainBackupGetXMLDesc;
> > >
> > > Signed-off-by: Eric Blake <eblake@redhat.com>
> > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > > ---
> > > include/libvirt/libvirt-domain.h | 26 ++++-
> > > src/driver-hypervisor.h | 20 ++++
> > > src/libvirt-domain-checkpoint.c | 7 +-
> > > src/libvirt-domain.c | 191 +++++++++++++++++++++++++++++++
> > > src/libvirt_public.syms | 8 ++
> > > tools/virsh-domain.c | 4 +-
> > > 6 files changed, 252 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> > > index 22277b0a84..2d9f69f7d4 100644
> > > --- a/include/libvirt/libvirt-domain.h
> > > +++ b/include/libvirt/libvirt-domain.h
> > > @@ -3267,6 +3267,7 @@ typedef enum {
> >
> >
> > >
> > > +/**
> > > + * 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:
> > > *
> > > @@ -4106,7 +4115,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).
> > > @@ -4916,4 +4926,18 @@ int virDomainGetGuestInfo(virDomainPtr domain,
> > > int *nparams,
> > > unsigned int flags);
> > >
> > > +
> > > +int virDomainBackupBegin(virDomainPtr domain,
> > > + const char *backupXML,
> > > + const char *checkpointXML,
> > > + unsigned int flags);
> > > +
> > > +char *virDomainBackupGetXMLDesc(virDomainPtr domain,
> > > + int id,
> > > + unsigned int flags);
> > > +
> > > +int virDomainBackupEnd(virDomainPtr domain,
> > > + int id,
> > > + unsigned int flags);
> >
> > So this is still using a plain integer job ID, which is a concern
> > wrt future extensibility.
>
> My current plan is to go ahead with API based on this old one but with
> no support for any parallel jobs. Basically the same thing but 'id'
> argument removed.
>
> This actually fits in with the original documentation which was already
> ACKed for the virDomainBackupBegin API which said that the backup job
> uses the domain async job infrastructure. This means that the
> virDomainAbortJob and virDomainGetJobStats can be used to monitor the
> blockjob. It also gives us the possibility to query the state of a
> finised job by passing VIR_DOMAIN_JOB_STATS_COMPLETED to
> virDomainGetJobStats. We also have an async event for reporting the job
> state. I'll also consider removing virDomainBackupEnd for this
> implementation as virDomainAbortJob should be enough.
Ok, that makes sense. So we'll just have the Begin + GetXMLDesc APIs
for now.
> I spoke with oVirt devs who are very keen on getting this API finished
> and their requirements currently don't require any parallel jobs. In
> fact they were abusing Eric's design which only ever returned the same
> job ID so that they would not have to persist it.
Hah, that's gross :-)
> Given that we'll need to deal with the domain job anyways for the better
> job infra outlined below I don't think adding these APIs will be too
> much of a burden in the interim so that we can appease oVirt's desire
> for this feature and we'll have more time to design the new job
> interface properly.
Ok, that's fine with me. As you say, adding a 2nd variant of the API
later if we need many jobs is not the end of the world, since we'll
need todo that anyway for many other APIs we already have.,
My preference is indeed to get something finally merged so that oVirt
can use it, since we've been debating the design on list here for waaaay
too long now.
> > Earlier in the year I queried whether we should turn the "job" into a
> > fully fledged object, using either a string or a UUID to identify it
> > uniquely.
> >
> > https://www.redhat.com/archives/libvir-list/2019-March/msg01695.html
> >
> > IOW having something like this:
>
> Going forward I want this not only for the backup job but basically for
> any long running operation. We needed this for a long time but of the
> two long running job impls we have both are not flexible enough.
>
> There is only one 'domain job' (migration/save/etc) and blockjobs are
> bound to disks.
Yeah, we kinda messed up in these two designs.
> > typedef struct _virDomainJob virDomainJob;
> > typedef virDomainJob *virDomainJobPtr;
>
> I actually started some work on this but didn't get far yet. I used the
> same name in my case, but I'm partially afraid that virDomainAbortJob
> which would not be related to these objects will be mistaken for
> actually working in this case.
>
> Unfortunately I don't have any better idea.
We could just pick a completely different term. eg "virDomainOperation"
> > void virDomainJobFree(virDomainJobPtr job);
> >
> > virDomainJobLookupByUUID(virDomainPtr job,
> > unsigned char *uuid);
> >
> > int virDomainJobGetType(virDomainJobPtr job);
> > int virDomainJobGetUUID(virDomainJobPtr job,
> > unsigned char *uuid);
> > int virDomainJobGetUUIDString(virDomainJobPtr job,
> > char *uuidstr);
> >
> >
> > virDomainJobPtr virDomainBackupBegin(virDomainPtr domain,
> > const char *backupXML,
> > const char *checkpointXML,
> > unsigned int flags);
>
> I was thinking about a super-universal API so that we don't have to redo
> all APIs for blockjobs. Something along
>
> virDomainJobPtr virDomainJobBegin(virDomainPtr domain,
> const char *jobxml,
> unsigned int flags);
>
> It would give us the flexibility to add new jobs and arguments for them
> via XML which is more flexible (e.g. we could easily add a
> virDomainBlockPull with the 'top' argument which is currently missing
> but qemu started to support it some time ago). On the other hand that
> seems a too prone to abuse.
virTypedParameters is always an option if we wish to make the
specific job creation API more flexible without going all the
way to XML
>
> As of the above I'm unsure about the tradeofs between flexibility and
> too much flexibility in this case. But that's probably for a future
> discussion.
Yeah, its a tricky balance between making it super flexible, and
making it more simple at an API caller POV.
Basically we have two choices
- consider the "job" to be just a way to monitor and manage
completion of jobs that something else spawned (what we
do now)
- consider the "job" to be the way to control the entire
lifecycle of the job, including spawning it.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.