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@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
On 7/2/20 9:40 AM, Peter Krempa wrote:
> 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@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``
s/backup and/backup, and/
> + 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>
As written, you validate:
backupmode="full" incremental="blah"
Better might be:
<define name='backupDiskMode'>
<optional>
<choice>
<attribute name='backupmode'>
<value>full</value>
</attribute>
<group>
<optional>
<attribute name='backupmode'>
<value>incremental</value>
</attribute>
</optional>
<optional>
<attribute name='incremental'/>
</optional>
</broup>
</choice>
</optional>
</define>
which also has the advantage of allowing the user to omit
backupmode='incremental' when supplying incremental='name' (since then
that mode is implied).
Do we need to restrict the set of values that can be supplied for a
incremental name? (That's a bigger issue than just this patch: for
example, do we want to refuse a checkpoint named "../foo"? As long as
checkpoint names don't match directly to file names, we aren't at risk
of a filesystem escape, but starting strict and relaxing later is better
than starting relaxed and wishing we had limited certain patterns after all)
> @@ -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;
> + }
You had to check this manually, instead of letting the .rng file enforce
it for you by the construct I listed above as an alternative.
> +
> + 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;
> + }
Do we really need to require that the user provides
backupmode='incremental', or if they omit it, can we just imply it based
on the presence of incremental='name'?
> +++ 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>
So this is a demo of overriding an overall incremental request with a
full for this disk.
> + <disk name='vdd' type='file' backupmode='incremental'>
> + <scratch file='/path/to/file'/>
> + </disk>
What incremental bitmap name do we default to if the overall backupjob
requested full? Or is that an error?
> + <disk name='vde' type='file' backupmode='incremental' incremental='blah'>
> + <scratch file='/path/to/file'/>
> + </disk>
This is a demo of using a different checkpoint for this disk than for
the overall job.
> + <disk name='vdf' type='file' incremental='bleh'>
> + <scratch file='/path/to/file'/>
> + </disk>
And this is a demo of allowing backupmode='incremental' to be skipped
when it makes sense.
> </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'/>
Why is backupmode='incremental' not present in output? Even when it can
be omitted in input, it makes sense for output to include the resulting
value of anything that was defaulted.
> + </disk>
> </disks>
> </domainbackup>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
On Thu, Jul 02, 2020 at 14:31:19 -0500, Eric Blake wrote:
> On 7/2/20 9:40 AM, Peter Krempa wrote:
> > 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@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``
>
> s/backup and/backup, and/
>
> > + 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>
>
> As written, you validate:
>
> backupmode="full" incremental="blah"
>
> Better might be:
>
> <define name='backupDiskMode'>
> <optional>
> <choice>
> <attribute name='backupmode'>
> <value>full</value>
> </attribute>
> <group>
> <optional>
> <attribute name='backupmode'>
> <value>incremental</value>
> </attribute>
> </optional>
> <optional>
> <attribute name='incremental'/>
> </optional>
> </broup>
> </choice>
> </optional>
> </define>
>
> which also has the advantage of allowing the user to omit
> backupmode='incremental' when supplying incremental='name' (since then that
> mode is implied).
Nice, I'll use this one. My brain stopped working when doing the schema
and I couldn't figure this one out.
>
> Do we need to restrict the set of values that can be supplied for a
> incremental name? (That's a bigger issue than just this patch: for example,
> do we want to refuse a checkpoint named "../foo"? As long as checkpoint
> names don't match directly to file names, we aren't at risk of a filesystem
> escape, but starting strict and relaxing later is better than starting
> relaxed and wishing we had limited certain patterns after all)
I'll think about this and possbily post a separate patch. The same also
applies to the <incremental> element which also doesn't do validation.
Luckily it's not officially supported yet so we can still make it more
strict.
> > @@ -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;
> > + }
>
> You had to check this manually, instead of letting the .rng file enforce it
> for you by the construct I listed above as an alternative.
Sure thing! I actually prefer the RNG solution.
>
> > +
> > + 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;
> > + }
>
> Do we really need to require that the user provides
> backupmode='incremental', or if they omit it, can we just imply it based on
> the presence of incremental='name'?
No, this check validates that if an explicit incremental mode is
requested and neither the per-disk nor per-backup 'incremental' setting
is provided we reject such a config because we wouldn't be able to infer
which is the point where to backup from.
> > +++ 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>
>
> So this is a demo of overriding an overall incremental request with a full
> for this disk.
>
> > + <disk name='vdd' type='file' backupmode='incremental'>
> > + <scratch file='/path/to/file'/>
> > + </disk>
>
> What incremental bitmap name do we default to if the overall backupjob
> requested full? Or is that an error?
It's an error as noted above.
> > + <disk name='vde' type='file' backupmode='incremental' incremental='blah'>
> > + <scratch file='/path/to/file'/>
> > + </disk>
>
> This is a demo of using a different checkpoint for this disk than for the
> overall job.
>
> > + <disk name='vdf' type='file' incremental='bleh'>
> > + <scratch file='/path/to/file'/>
> > + </disk>
>
> And this is a demo of allowing backupmode='incremental' to be skipped when
> it makes sense.
>
> > </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'/>
>
> Why is backupmode='incremental' not present in output? Even when it can be
> omitted in input, it makes sense for output to include the resulting value
> of anything that was defaulted.
Well the code fills it in in 'virDomainBackupAlignDisks', but that
function is not called from the test suite.
'virDomainBackupAlignDisks' requires a domain definition to do some
matching around. While I agree that it should be tested, it's not really
in scope of this patch.
>
> > + </disk>
> > </disks>
> > </domainbackup>
> >
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3226
> Virtualization: qemu.org | libvirt.org
>
© 2016 - 2026 Red Hat, Inc.