[PATCH 14/24] backup: Allow configuring incremental backup per-disk individually

Peter Krempa pkrempa at redhat.com
Thu Jul 2 14:40:00 UTC 2020


The semantics of the backup operation don't strictly require that all
disks being backed up are part of the same incremental part (when a disk
was checkpointed/backed up separately or in a different VM), or even
they may not have an previous checkpoint at all (e.g. when the disk
was freshly hotplugged to the vm).

In such cases we can still create a common checkpoint for all of them
and backup differences according to configuration.

This patch adds a per-disk configuration of the checkpoint to do the
incremental backup from via the 'incremental' attribute and allows
perform full backups via the 'backupmode' attribute.

Note that no changes to the qemu driver are necessary to take advantage
of this as we already obey the per-disk 'incremental' field.

https://bugzilla.redhat.com/show_bug.cgi?id=1829829

Signed-off-by: Peter Krempa <pkrempa at redhat.com>
---
 docs/formatbackup.rst                        | 11 ++++
 docs/schemas/domainbackup.rng                | 16 ++++++
 src/conf/backup_conf.c                       | 57 +++++++++++++++++++-
 src/conf/backup_conf.h                       | 11 ++++
 tests/domainbackupxml2xmlin/backup-pull.xml  | 12 +++++
 tests/domainbackupxml2xmlout/backup-pull.xml | 12 +++++
 6 files changed, 118 insertions(+), 1 deletion(-)

diff --git a/docs/formatbackup.rst b/docs/formatbackup.rst
index 66583f562b..e5b6fc6eb0 100644
--- a/docs/formatbackup.rst
+++ b/docs/formatbackup.rst
@@ -65,6 +65,17 @@ were supplied). The following child elements and attributes are supported:
          should take part in the backup and using ``no`` excludes the disk from
          the backup.

+      ``backupmode``
+         This attribute overrides the implied backup mode inherited from the
+         definition of the backup itself. Value ``full`` forces a full backup
+         even if the backup calls for an incremental backup and ``incremental``
+         coupled with the attribute ``incremental='CHECKPOINTNAME`` for the disk
+         forces an incremental backup from ``CHECKPOINTNAME``.
+
+       ``incremental``
+         An optional attribute giving the name of an existing checkpoint of the
+         domain which overrides the one set by the ``<incremental>`` element.
+
       ``exportname``
          Allows modification of the NBD export name for the given disk. By
          default equal to disk target. Valid only for pull mode backups.
diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng
index 5165175152..650f5cd4c3 100644
--- a/docs/schemas/domainbackup.rng
+++ b/docs/schemas/domainbackup.rng
@@ -89,6 +89,20 @@
     </element>
   </define>

+  <define name='backupDiskMode'>
+    <optional>
+      <attribute name='backupmode'>
+        <choice>
+          <value>full</value>
+          <value>incremental</value>
+        </choice>
+      </attribute>
+    </optional>
+    <optional>
+      <attribute name='incremental'/>
+    </optional>
+  </define>
+
   <define name='backupPushDriver'>
     <optional>
       <element name='driver'>
@@ -127,6 +141,7 @@
             <attribute name='name'>
               <ref name='diskTarget'/>
             </attribute>
+            <ref name='backupDiskMode'/>
             <choice>
               <group>
                 <attribute name='backup'>
@@ -196,6 +211,7 @@
             <attribute name='name'>
               <ref name='diskTarget'/>
             </attribute>
+            <ref name='backupDiskMode'/>
             <optional>
               <attribute name='exportname'>
                 <text/>
diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c
index e9eea5af75..4f28073ab2 100644
--- a/src/conf/backup_conf.c
+++ b/src/conf/backup_conf.c
@@ -56,6 +56,13 @@ VIR_ENUM_IMPL(virDomainBackupDiskState,
               "cancelling",
               "cancelled");

+VIR_ENUM_DECL(virDomainBackupDiskBackupMode);
+VIR_ENUM_IMPL(virDomainBackupDiskBackupMode,
+              VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_LAST,
+              "",
+              "full",
+              "incremental");
+
 void
 virDomainBackupDefFree(virDomainBackupDefPtr def)
 {
@@ -96,6 +103,7 @@ virDomainBackupDiskDefParseXML(xmlNodePtr node,
     g_autofree char *driver = NULL;
     g_autofree char *backup = NULL;
     g_autofree char *state = NULL;
+    g_autofree char *backupmode = NULL;
     int tmp;
     xmlNodePtr srcNode;
     unsigned int storageSourceParseFlags = 0;
@@ -133,6 +141,19 @@ virDomainBackupDiskDefParseXML(xmlNodePtr node,
         def->exportbitmap = virXMLPropString(node, "exportbitmap");
     }

+    if ((backupmode = virXMLPropString(node, "backupmode"))) {
+        if ((tmp = virDomainBackupDiskBackupModeTypeFromString(backupmode)) < 0) {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("invalid backupmode '%s' of disk '%s'"),
+                           backupmode, def->name);
+            return -1;
+        }
+
+        def->backupmode = tmp;
+    }
+
+    def->incremental = virXMLPropString(node, "incremental");
+
     if (internal) {
         if (!(state = virXMLPropString(node, "state")) ||
             (tmp = virDomainBackupDiskStateTypeFromString(state)) < 0) {
@@ -342,6 +363,13 @@ virDomainBackupDiskDefFormat(virBufferPtr buf,
     if (disk->backup == VIR_TRISTATE_BOOL_YES) {
         virBufferAsprintf(&attrBuf, " type='%s'", virStorageTypeToString(disk->store->type));

+        if (disk->backupmode != VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_DEFAULT) {
+            virBufferAsprintf(&attrBuf, " backupmode='%s'",
+                              virDomainBackupDiskBackupModeTypeToString(disk->backupmode));
+        }
+
+        virBufferEscapeString(&attrBuf, " incremental='%s'", disk->incremental);
+
         virBufferEscapeString(&attrBuf, " exportname='%s'", disk->exportname);
         virBufferEscapeString(&attrBuf, " exportbitmap='%s'", disk->exportbitmap);

@@ -465,6 +493,24 @@ virDomainBackupAlignDisks(virDomainBackupDefPtr def,
             return -1;
         }

+        if (backupdisk->backupmode == VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_FULL &&
+            backupdisk->incremental) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("'full' backup mode incompatible with 'incremental' for disk '%s'"),
+                           backupdisk->name);
+            return -1;
+        }
+
+        if (backupdisk->backupmode == VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_INCREMENTAL &&
+            !backupdisk->incremental &&
+            !def->incremental) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("'incremental' backup mode of disk '%s' requires setting 'incremental' field for disk or backup"),
+                           backupdisk->name);
+            return -1;
+        }
+
+
         if (backupdisk->backup == VIR_TRISTATE_BOOL_YES &&
             virDomainBackupDefAssignStore(backupdisk, domdisk->src, suffix) < 0)
             return -1;
@@ -502,7 +548,16 @@ virDomainBackupAlignDisks(virDomainBackupDefPtr def,
     for (i = 0; i < def->ndisks; i++) {
         virDomainBackupDiskDefPtr backupdisk = &def->disks[i];

-        if (def->incremental && !backupdisk->incremental)
+        if (backupdisk->backupmode == VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_DEFAULT) {
+            if (def->incremental || backupdisk->incremental) {
+                backupdisk->backupmode = VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_INCREMENTAL;
+            } else {
+                backupdisk->backupmode = VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_FULL;
+            }
+        }
+
+        if (!backupdisk->incremental &&
+            backupdisk->backupmode == VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_INCREMENTAL)
             backupdisk->incremental = g_strdup(def->incremental);
     }

diff --git a/src/conf/backup_conf.h b/src/conf/backup_conf.h
index 172eb1cf1c..3f8b592b8d 100644
--- a/src/conf/backup_conf.h
+++ b/src/conf/backup_conf.h
@@ -45,12 +45,23 @@ typedef enum {
     VIR_DOMAIN_BACKUP_DISK_STATE_LAST
 } virDomainBackupDiskState;

+
+typedef enum {
+    VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_DEFAULT = 0,
+    VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_FULL,
+    VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_INCREMENTAL,
+
+    VIR_DOMAIN_BACKUP_DISK_BACKUP_MODE_LAST
+} virDomainBackupDiskBackupMode;
+
+
 /* Stores disk-backup information */
 typedef struct _virDomainBackupDiskDef virDomainBackupDiskDef;
 typedef virDomainBackupDiskDef *virDomainBackupDiskDefPtr;
 struct _virDomainBackupDiskDef {
     char *name;     /* name matching the <target dev='...' of the domain */
     virTristateBool backup; /* whether backup is requested */
+    virDomainBackupDiskBackupMode backupmode;
     char *incremental; /* name of the starting point checkpoint of an incremental backup */
     char *exportname; /* name of the NBD export for pull mode backup */
     char *exportbitmap; /* name of the bitmap exposed in NBD for pull mode backup */
diff --git a/tests/domainbackupxml2xmlin/backup-pull.xml b/tests/domainbackupxml2xmlin/backup-pull.xml
index c0bea4771d..c51f0995ac 100644
--- a/tests/domainbackupxml2xmlin/backup-pull.xml
+++ b/tests/domainbackupxml2xmlin/backup-pull.xml
@@ -6,5 +6,17 @@
       <scratch file='/path/to/file'/>
     </disk>
     <disk name='hda' backup='no'/>
+    <disk name='vdc' type='file' backupmode='full'>
+      <scratch file='/path/to/file'/>
+    </disk>
+    <disk name='vdd' type='file' backupmode='incremental'>
+      <scratch file='/path/to/file'/>
+    </disk>
+    <disk name='vde' type='file' backupmode='incremental' incremental='blah'>
+      <scratch file='/path/to/file'/>
+    </disk>
+    <disk name='vdf' type='file' incremental='bleh'>
+      <scratch file='/path/to/file'/>
+    </disk>
   </disks>
 </domainbackup>
diff --git a/tests/domainbackupxml2xmlout/backup-pull.xml b/tests/domainbackupxml2xmlout/backup-pull.xml
index 24fce9c0e7..d2f84cda7a 100644
--- a/tests/domainbackupxml2xmlout/backup-pull.xml
+++ b/tests/domainbackupxml2xmlout/backup-pull.xml
@@ -6,5 +6,17 @@
       <scratch file='/path/to/file'/>
     </disk>
     <disk name='hda' backup='no'/>
+    <disk name='vdc' backup='yes' type='file' backupmode='full'>
+      <scratch file='/path/to/file'/>
+    </disk>
+    <disk name='vdd' backup='yes' type='file' backupmode='incremental'>
+      <scratch file='/path/to/file'/>
+    </disk>
+    <disk name='vde' backup='yes' type='file' backupmode='incremental' incremental='blah'>
+      <scratch file='/path/to/file'/>
+    </disk>
+    <disk name='vdf' backup='yes' type='file' incremental='bleh'>
+      <scratch file='/path/to/file'/>
+    </disk>
   </disks>
 </domainbackup>
-- 
2.26.2




More information about the libvir-list mailing list