If users wish to use different name for exported disks or bitmaps
the new fields allow to do so. Additionally they also document the
current settings.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
docs/formatbackup.html.in | 9 +++++++++
docs/schemas/domainbackup.rng | 8 ++++++++
src/conf/backup_conf.c | 11 +++++++++++
src/conf/backup_conf.h | 2 ++
tests/domainbackupxml2xmlin/backup-pull-seclabel.xml | 2 +-
tests/domainbackupxml2xmlout/backup-pull-seclabel.xml | 2 +-
6 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in
index 1c690901c7..7d2c6f1599 100644
--- a/docs/formatbackup.html.in
+++ b/docs/formatbackup.html.in
@@ -85,6 +85,15 @@
<dd>Setting this attribute to <code>yes</code>(default) specifies
that the disk should take part in the backup and using
<code>no</code> excludes the disk from the backup.</dd>
+ <dt><code>exportname</code></dt>
+ <dd>Allows to modify the NBD export name for the given disk.
+ By default equal to disk target.
+ Valid only for pull mode backups. </dd>
+ <dt><code>exportbitmap</code></dt>
+ <dd>Allows to modify the name of the bitmap describing dirty
+ blocks for an incremental backup exported via NBD export name
+ for the given disk.
+ Valid only for pull mode backups. </dd>
<dt><code>type</code></dt>
<dd>A mandatory attribute to describe the type of the
disk, except when <code>backup='no'</code> is
diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng
index c1e4d80302..395ea841f9 100644
--- a/docs/schemas/domainbackup.rng
+++ b/docs/schemas/domainbackup.rng
@@ -165,6 +165,14 @@
<attribute name='name'>
<ref name='diskTarget'/>
</attribute>
+ <optional>
+ <attribute name='exportname'>
+ <text/>
+ </attribute>
+ <attribute name='exportbitmap'>
+ <text/>
+ </attribute>
+ </optional>
<choice>
<group>
<attribute name='backup'>
diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c
index aa11967d2a..a4b87baa55 100644
--- a/src/conf/backup_conf.c
+++ b/src/conf/backup_conf.c
@@ -71,6 +71,8 @@ virDomainBackupDefFree(virDomainBackupDefPtr def)
virDomainBackupDiskDefPtr disk = def->disks + i;
g_free(disk->name);
+ g_free(disk->exportname);
+ g_free(disk->exportbitmap);
virObjectUnref(disk->store);
}
@@ -124,6 +126,11 @@ virDomainBackupDiskDefParseXML(xmlNodePtr node,
if (def->backup == VIR_TRISTATE_BOOL_NO)
return 0;
+ if (!push) {
+ def->exportname = virXMLPropString(node, "exportname");
+ def->exportbitmap = virXMLPropString(node, "exportbitmap");
+ }
+
if (internal) {
if (!(state = virXMLPropString(node, "state")) ||
(tmp = virDomainBackupDiskStateTypeFromString(state)) < 0) {
@@ -165,6 +172,7 @@ virDomainBackupDiskDefParseXML(xmlNodePtr node,
storageSourceParseFlags, xmlopt) < 0)
return -1;
+
if ((driver = virXPathString("string(./driver/@type)", ctxt))) {
def->store->format = virStorageFileFormatTypeFromString(driver);
if (def->store->format <= 0) {
@@ -333,6 +341,9 @@ virDomainBackupDiskDefFormat(virBufferPtr buf,
if (disk->backup == VIR_TRISTATE_BOOL_YES) {
virBufferAsprintf(&attrBuf, " type='%s'", virStorageTypeToString(disk->store->type));
+ virBufferEscapeString(&attrBuf, " exportname='%s'", disk->exportname);
+ virBufferEscapeString(&attrBuf, " exportbitmap='%s'", disk->exportbitmap);
+
if (disk->store->format > 0)
virBufferEscapeString(&childBuf, "<driver type='%s'/>\n",
virStorageFileFormatTypeToString(disk->store->format));
diff --git a/src/conf/backup_conf.h b/src/conf/backup_conf.h
index 7cf44245d4..672fd52ee7 100644
--- a/src/conf/backup_conf.h
+++ b/src/conf/backup_conf.h
@@ -51,6 +51,8 @@ typedef virDomainBackupDiskDef *virDomainBackupDiskDefPtr;
struct _virDomainBackupDiskDef {
char *name; /* name matching the <target dev='...' of the domain */
virTristateBool backup; /* whether backup is requested */
+ char *exportname; /* name of the NBD export for pull mode backup */
+ char *exportbitmap; /* name of the bitmap exposed in NBD for pull mode backup */
/* details of target for push-mode, or of the scratch file for pull-mode */
virStorageSourcePtr store;
diff --git a/tests/domainbackupxml2xmlin/backup-pull-seclabel.xml b/tests/domainbackupxml2xmlin/backup-pull-seclabel.xml
index a00d8758bb..4e6e602c19 100644
--- a/tests/domainbackupxml2xmlin/backup-pull-seclabel.xml
+++ b/tests/domainbackupxml2xmlin/backup-pull-seclabel.xml
@@ -2,7 +2,7 @@
<incremental>1525889631</incremental>
<server transport='tcp' name='localhost' port='10809'/>
<disks>
- <disk name='vda' type='file'>
+ <disk name='vda' type='file' exportname='test-vda' exportbitmap='blah'>
<driver type='qcow2'/>
<scratch file='/path/to/file'>
<seclabel model='dac' relabel='no'/>
diff --git a/tests/domainbackupxml2xmlout/backup-pull-seclabel.xml b/tests/domainbackupxml2xmlout/backup-pull-seclabel.xml
index c631c9b979..450f007d3a 100644
--- a/tests/domainbackupxml2xmlout/backup-pull-seclabel.xml
+++ b/tests/domainbackupxml2xmlout/backup-pull-seclabel.xml
@@ -2,7 +2,7 @@
<incremental>1525889631</incremental>
<server transport='tcp' name='localhost' port='10809'/>
<disks>
- <disk name='vda' backup='yes' type='file'>
+ <disk name='vda' backup='yes' type='file' exportname='test-vda' exportbitmap='blah'>
<driver type='qcow2'/>
<scratch file='/path/to/file'>
<seclabel model='dac' relabel='no'/>
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Dec 20, 2019 at 02:25:27PM +0100, Peter Krempa wrote: > If users wish to use different name for exported disks or bitmaps > the new fields allow to do so. Additionally they also document the > current settings. Is there a reason why users would want to change the names ? 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 12/20/19 8:00 AM, Daniel P. Berrangé wrote: > On Fri, Dec 20, 2019 at 02:25:27PM +0100, Peter Krempa wrote: >> If users wish to use different name for exported disks or bitmaps >> the new fields allow to do so. Additionally they also document the >> current settings. > > Is there a reason why users would want to change the names ? Right now, with only a single job possible at a time, libvirt picks a predictable name based on the guest disk being exported. But in the future, when we permit multiple jobs in parallel, it may become important to know which name is related to which job, or even to pick names based on what the client expects. I'm not sure we have an essential use case yet, but I'm also not rejecting this. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Dec 20, 2019 at 02:49:07PM -0600, Eric Blake wrote: > On 12/20/19 8:00 AM, Daniel P. Berrangé wrote: > > On Fri, Dec 20, 2019 at 02:25:27PM +0100, Peter Krempa wrote: > > > If users wish to use different name for exported disks or bitmaps > > > the new fields allow to do so. Additionally they also document the > > > current settings. > > > > Is there a reason why users would want to change the names ? > > Right now, with only a single job possible at a time, libvirt picks a > predictable name based on the guest disk being exported. But in the future, > when we permit multiple jobs in parallel, it may become important to know > which name is related to which job, or even to pick names based on what the > client expects. I'm not sure we have an essential use case yet, but I'm > also not rejecting this. If we have multiple parallel jobs, would we not also have a separate NBD server listening for each job ? 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 Fri, Dec 20, 2019 at 4:01 PM Daniel P. Berrangé <berrange@redhat.com> wrote: > > On Fri, Dec 20, 2019 at 02:25:27PM +0100, Peter Krempa wrote: > > If users wish to use different name for exported disks or bitmaps > > the new fields allow to do so. Additionally they also document the > > current settings. > > Is there a reason why users would want to change the names ? I don't see a reason for changing the exportname, but the default dirty bitmap name expose libvirt internal implementation details (e.g "libvirt-1-format"). We would choose use something like <backup-uuid>-<drive-name>. Displaying the names in the xml is important now, being able to modify it is nice to have for oVirt. Nir -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Dec 20, 2019 at 3:27 PM Peter Krempa <pkrempa@redhat.com> wrote:
>
> If users wish to use different name for exported disks or bitmaps
> the new fields allow to do so. Additionally they also document the
> current settings.
>
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> docs/formatbackup.html.in | 9 +++++++++
> docs/schemas/domainbackup.rng | 8 ++++++++
> src/conf/backup_conf.c | 11 +++++++++++
> src/conf/backup_conf.h | 2 ++
> tests/domainbackupxml2xmlin/backup-pull-seclabel.xml | 2 +-
> tests/domainbackupxml2xmlout/backup-pull-seclabel.xml | 2 +-
> 6 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in
> index 1c690901c7..7d2c6f1599 100644
> --- a/docs/formatbackup.html.in
> +++ b/docs/formatbackup.html.in
> @@ -85,6 +85,15 @@
> <dd>Setting this attribute to <code>yes</code>(default) specifies
> that the disk should take part in the backup and using
> <code>no</code> excludes the disk from the backup.</dd>
> + <dt><code>exportname</code></dt>
> + <dd>Allows to modify the NBD export name for the given disk.
> + By default equal to disk target.
> + Valid only for pull mode backups. </dd>
Makes sense. We assumed that the name is same as the drive name (e.g.
"sda"), but
being able to set this name or get it from the backup xml returned by
backupGetXMLDesc()
is more robust.
Currently oVirt generates the NBD URL after starting a backup,
assuming the drive name.
I think with this we will would call backupGetXMLDesc(), parse it and
extract the exportname.
> + <dt><code>exportbitmap</code></dt>
> + <dd>Allows to modify the name of the bitmap describing dirty
> + blocks for an incremental backup exported via NBD export name
> + for the given disk.
> + Valid only for pull mode backups. </dd>
Currently we discover this name using NBD_OPT_LIST_META_CONTEXT but being able
to set or get it from the xml is better.
I think the text should mention that this name must be unique, so you
cannot use the same name
in case you backup multiple disks.
We would probably use the drive name as well for this, so it will be
available as
"qemu:dirty-bitmap:sda". Or to ensure it can never clash with another bitmap,
<backup-uuid>-<drive-name> (7ce9b65d-b832-4ff2-b100-5170c9d02e2a-sda).
It would be nice to show these parameters in the examples at the
bottom of the page, in particualr
in an example output of backupGetXMLDesc(), where this value will always appear.
Are there any limits on the content and size of these strings?
> <dt><code>type</code></dt>
> <dd>A mandatory attribute to describe the type of the
> disk, except when <code>backup='no'</code> is
> diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng
> index c1e4d80302..395ea841f9 100644
> --- a/docs/schemas/domainbackup.rng
> +++ b/docs/schemas/domainbackup.rng
> @@ -165,6 +165,14 @@
> <attribute name='name'>
> <ref name='diskTarget'/>
> </attribute>
> + <optional>
> + <attribute name='exportname'>
> + <text/>
> + </attribute>
> + <attribute name='exportbitmap'>
> + <text/>
> + </attribute>
> + </optional>
> <choice>
> <group>
> <attribute name='backup'>
> diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c
> index aa11967d2a..a4b87baa55 100644
> --- a/src/conf/backup_conf.c
> +++ b/src/conf/backup_conf.c
> @@ -71,6 +71,8 @@ virDomainBackupDefFree(virDomainBackupDefPtr def)
> virDomainBackupDiskDefPtr disk = def->disks + i;
>
> g_free(disk->name);
> + g_free(disk->exportname);
> + g_free(disk->exportbitmap);
> virObjectUnref(disk->store);
> }
>
> @@ -124,6 +126,11 @@ virDomainBackupDiskDefParseXML(xmlNodePtr node,
> if (def->backup == VIR_TRISTATE_BOOL_NO)
> return 0;
>
> + if (!push) {
> + def->exportname = virXMLPropString(node, "exportname");
> + def->exportbitmap = virXMLPropString(node, "exportbitmap");
> + }
> +
> if (internal) {
> if (!(state = virXMLPropString(node, "state")) ||
> (tmp = virDomainBackupDiskStateTypeFromString(state)) < 0) {
> @@ -165,6 +172,7 @@ virDomainBackupDiskDefParseXML(xmlNodePtr node,
> storageSourceParseFlags, xmlopt) < 0)
> return -1;
>
> +
> if ((driver = virXPathString("string(./driver/@type)", ctxt))) {
> def->store->format = virStorageFileFormatTypeFromString(driver);
> if (def->store->format <= 0) {
> @@ -333,6 +341,9 @@ virDomainBackupDiskDefFormat(virBufferPtr buf,
> if (disk->backup == VIR_TRISTATE_BOOL_YES) {
> virBufferAsprintf(&attrBuf, " type='%s'", virStorageTypeToString(disk->store->type));
>
> + virBufferEscapeString(&attrBuf, " exportname='%s'", disk->exportname);
> + virBufferEscapeString(&attrBuf, " exportbitmap='%s'", disk->exportbitmap);
> +
> if (disk->store->format > 0)
> virBufferEscapeString(&childBuf, "<driver type='%s'/>\n",
> virStorageFileFormatTypeToString(disk->store->format));
> diff --git a/src/conf/backup_conf.h b/src/conf/backup_conf.h
> index 7cf44245d4..672fd52ee7 100644
> --- a/src/conf/backup_conf.h
> +++ b/src/conf/backup_conf.h
> @@ -51,6 +51,8 @@ typedef virDomainBackupDiskDef *virDomainBackupDiskDefPtr;
> struct _virDomainBackupDiskDef {
> char *name; /* name matching the <target dev='...' of the domain */
> virTristateBool backup; /* whether backup is requested */
> + char *exportname; /* name of the NBD export for pull mode backup */
> + char *exportbitmap; /* name of the bitmap exposed in NBD for pull mode backup */
>
> /* details of target for push-mode, or of the scratch file for pull-mode */
> virStorageSourcePtr store;
> diff --git a/tests/domainbackupxml2xmlin/backup-pull-seclabel.xml b/tests/domainbackupxml2xmlin/backup-pull-seclabel.xml
> index a00d8758bb..4e6e602c19 100644
> --- a/tests/domainbackupxml2xmlin/backup-pull-seclabel.xml
> +++ b/tests/domainbackupxml2xmlin/backup-pull-seclabel.xml
> @@ -2,7 +2,7 @@
> <incremental>1525889631</incremental>
> <server transport='tcp' name='localhost' port='10809'/>
> <disks>
> - <disk name='vda' type='file'>
> + <disk name='vda' type='file' exportname='test-vda' exportbitmap='blah'>
> <driver type='qcow2'/>
> <scratch file='/path/to/file'>
> <seclabel model='dac' relabel='no'/>
> diff --git a/tests/domainbackupxml2xmlout/backup-pull-seclabel.xml b/tests/domainbackupxml2xmlout/backup-pull-seclabel.xml
> index c631c9b979..450f007d3a 100644
> --- a/tests/domainbackupxml2xmlout/backup-pull-seclabel.xml
> +++ b/tests/domainbackupxml2xmlout/backup-pull-seclabel.xml
> @@ -2,7 +2,7 @@
> <incremental>1525889631</incremental>
> <server transport='tcp' name='localhost' port='10809'/>
> <disks>
> - <disk name='vda' backup='yes' type='file'>
> + <disk name='vda' backup='yes' type='file' exportname='test-vda' exportbitmap='blah'>
> <driver type='qcow2'/>
> <scratch file='/path/to/file'>
> <seclabel model='dac' relabel='no'/>
> --
> 2.23.0
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
Nir
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 12/20/19 7:25 AM, Peter Krempa wrote: > If users wish to use different name for exported disks or bitmaps different names > the new fields allow to do so. Additionally they also document the allow this possibility. > current settings. > > Signed-off-by: Peter Krempa <pkrempa@redhat.com> > --- > docs/formatbackup.html.in | 9 +++++++++ > docs/schemas/domainbackup.rng | 8 ++++++++ > src/conf/backup_conf.c | 11 +++++++++++ > src/conf/backup_conf.h | 2 ++ > tests/domainbackupxml2xmlin/backup-pull-seclabel.xml | 2 +- > tests/domainbackupxml2xmlout/backup-pull-seclabel.xml | 2 +- > 6 files changed, 32 insertions(+), 2 deletions(-) > > diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in > index 1c690901c7..7d2c6f1599 100644 > --- a/docs/formatbackup.html.in > +++ b/docs/formatbackup.html.in > @@ -85,6 +85,15 @@ > <dd>Setting this attribute to <code>yes</code>(default) specifies > that the disk should take part in the backup and using > <code>no</code> excludes the disk from the backup.</dd> > + <dt><code>exportname</code></dt> > + <dd>Allows to modify the NBD export name for the given disk. Allows modificatino of the NBD export name... > + By default equal to disk target. > + Valid only for pull mode backups. </dd> Why a space after '.'? > + <dt><code>exportbitmap</code></dt> > + <dd>Allows to modify the name of the bitmap describing dirty Allows modification of the name of the bitmap > + blocks for an incremental backup exported via NBD export name > + for the given disk. > + Valid only for pull mode backups. </dd> and another space > <dt><code>type</code></dt> > <dd>A mandatory attribute to describe the type of the > disk, except when <code>backup='no'</code> is > diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng > index c1e4d80302..395ea841f9 100644 > --- a/docs/schemas/domainbackup.rng > +++ b/docs/schemas/domainbackup.rng > @@ -165,6 +165,14 @@ > <attribute name='name'> > <ref name='diskTarget'/> > </attribute> > + <optional> > + <attribute name='exportname'> > + <text/> > + </attribute> > + <attribute name='exportbitmap'> > + <text/> > + </attribute> > + </optional> Do we need any limits (for example, qcow2 rejects bitmap names longer than 1k)? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.