Re: [libvirt] [PATCH 04/21] backup: Introduce virDomainBackup APIs

2019-12-03 Thread Peter Krempa
On Wed, Nov 27, 2019 at 11:30:30 -0600, Eric Blake wrote:
> On 11/26/19 3:39 PM, Peter Krempa wrote:

[...]

> > + * 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(). Completion of the job is also 
> > announced
> > + * asynchronously via VIR_DOMAIN_EVENT_ID_JOB_COMPLETED event.
> 
> Is that true for pull mode, or only for push mode?  With push mode, it's
> obvious when the event is needed - when qemu finishes pushing.  But in pull
> mode, the only time an event makes sense is when you finally abort the job
> and the NBD server goes away - but as that is always an explicit libvirt API
> call, does the event still make sense?

It's also emitted for the pull mode. The same way as we emit this event
for migration. A migration success is also reported by the migration API
return value, but the event is still emitted.

Given that the event also carries the statistics it might be of
interrest to the APP, or can be ignored.

> 
> > + *
> > + * 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


[...]

> > + *
> > + * 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.
> > + *
> > + * Returns a NUL-terminated UTF-8 encoded XML instance or NULL in
> > + * case of error.  The caller must free() the returned value.
> > + */
> 
> Do we need any further tweaks to the virDomainAbortJob() API to mention that
> it is used to end a backup?

virDomainAbortJob documents that it aborts any current background job at
the soonest opportunity. The rest of the documentation mentions only
caveats for certain kinds of jobs. Since we don't have any for backup I
don't feel that it's necessary.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list



Re: [libvirt] [PATCH 04/21] backup: Introduce virDomainBackup APIs

2019-11-27 Thread Eric Blake

On 11/26/19 3:39 PM, Peter Krempa wrote:

From: Eric Blake 

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).

The full list of new APIs:
 virDomainBackupBegin;
 virDomainBackupGetXMLDesc;

Signed-off-by: Eric Blake 
Signed-off-by: Peter Krempa 


The cover letter mentions obvious changes from when I wrote this: no job 
id parameter, and reuse existing abort job API instead of adding 
virDomainBackupEnd.  We'll still have to revisit how a decent job API 
addition down the road affects things, but I can live with this API for 
the short term use of only one backup job at a time.



+
+/**
+ * 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 a domain job and thus mutually exclusive with any other
+ * domain job such as migration.


A limitation we hope to lift later with a proper job API, but not a 
show-stopper for this interface.



+ *
+ * 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(). Completion of the job is also announced
+ * asynchronously via VIR_DOMAIN_EVENT_ID_JOB_COMPLETED event.


Is that true for pull mode, or only for push mode?  With push mode, it's 
obvious when the event is needed - when qemu finishes pushing.  But in 
pull mode, the only time an event makes sense is when you finally abort 
the job and the NBD server goes away - but as that is always an explicit 
libvirt API call, does the event still make sense?



+ *
+ * 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 virDomainAbortJob() is used prior to the event being
+ * emitted). This kind of the job finishes automatically. Users can
+ * determine success by using virDomainGetJobStats() with
+ * VIR_DOMAIN_JOB_STATS_COMPLETED flag.
+ *
+ * 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 libvir has to be informed via


libvirt


+ * virDomainAbortJob() 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  , including which backup mode to use, whether the


no space before comma


+ * 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 0 on success or -1 on failure.
+ */



+/**
+ * virDomainBackupGetXMLDesc:
+ * @domain: a domain object
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Queries the 

Re: [libvirt] [PATCH 04/21] backup: Introduce virDomainBackup APIs

2019-11-27 Thread Daniel P . Berrangé
On Tue, Nov 26, 2019 at 10:39:50PM +0100, Peter Krempa wrote:
> From: Eric Blake 
> 
> 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).
> 
> The full list of new APIs:
> virDomainBackupBegin;
> virDomainBackupGetXMLDesc;
> 
> Signed-off-by: Eric Blake 
> Signed-off-by: Peter Krempa 
> ---
>  include/libvirt/libvirt-domain.h |  14 +++-
>  src/driver-hypervisor.h  |  12 +++
>  src/libvirt-domain-checkpoint.c  |   7 +-
>  src/libvirt-domain.c | 138 +++
>  src/libvirt_public.syms  |   6 ++
>  5 files changed, 173 insertions(+), 4 deletions(-)

Reviewed-by: Daniel P. Berrangé 
 

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

[libvirt] [PATCH 04/21] backup: Introduce virDomainBackup APIs

2019-11-26 Thread Peter Krempa
From: Eric Blake 

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).

The full list of new APIs:
virDomainBackupBegin;
virDomainBackupGetXMLDesc;

Signed-off-by: Eric Blake 
Signed-off-by: Peter Krempa 
---
 include/libvirt/libvirt-domain.h |  14 +++-
 src/driver-hypervisor.h  |  12 +++
 src/libvirt-domain-checkpoint.c  |   7 +-
 src/libvirt-domain.c | 138 +++
 src/libvirt_public.syms  |   6 ++
 5 files changed, 173 insertions(+), 4 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index b52f58aa8f..e2ad4967e4 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -4129,8 +4129,10 @@ 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
- * is completed. The params array will contain statistics of the just completed
+ * This callback occurs when a job (such as migration or backup) 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).
  *
@@ -4949,4 +4951,12 @@ int virDomainAgentSetResponseTimeout(virDomainPtr domain,
  int timeout,
  unsigned int flags);

+int virDomainBackupBegin(virDomainPtr domain,
+ const char *backupXML,
+ const char *checkpointXML,
+ unsigned int flags);
+
+char *virDomainBackupGetXMLDesc(virDomainPtr domain,
+unsigned int flags);
+
 #endif /* LIBVIRT_DOMAIN_H */
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index 4afd8f6ec5..bce023017d 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -1377,6 +1377,16 @@ typedef int
int timeout,
unsigned int flags);

+typedef int
+(*virDrvDomainBackupBegin)(virDomainPtr domain,
+   const char *backupXML,
+   const char *checkpointXML,
+   unsigned int flags);
+
+typedef char *
+(*virDrvDomainBackupGetXMLDesc)(virDomainPtr domain,
+unsigned int flags);
+
 typedef struct _virHypervisorDriver virHypervisorDriver;
 typedef virHypervisorDriver *virHypervisorDriverPtr;

@@ -1638,4 +1648,6 @@ struct _virHypervisorDriver {
 virDrvDomainCheckpointDelete domainCheckpointDelete;
 virDrvDomainGetGuestInfo domainGetGuestInfo;
 virDrvDomainAgentSetResponseTimeout domainAgentSetResponseTimeout;
+virDrvDomainBackupBegin domainBackupBegin;
+virDrvDomainBackupGetXMLDesc domainBackupGetXMLDesc;
 };
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
- *  element, on a running @domain.  Note that @xmlDesc
- * must validate against the  XML schema.
+ *  element, on a running @domain.  Note that
+ * @xmlDesc must validate against the  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 Checkpoint XML
  * for more details on @xmlDesc. In particular, some hypervisors may require
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 77169ec4ca..ec402ba5c0 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -12541,3 +12541,141 @@ virDomainAgentSetResponseTimeout(virDomainPtr domain,
 virDispatchError(conn);
 return -1;
 }
+
+
+/**
+ *