[libvirt] [PATCH RFC 09/11] conf: backup: Allow configuration of names exported via NBD
Nir Soffer
nsoffer at redhat.com
Sun Jan 5 23:49:34 UTC 2020
On Fri, Dec 20, 2019 at 3:27 PM Peter Krempa <pkrempa at 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 at 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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
Nir
More information about the libvir-list
mailing list