[libvirt] [PATCH 4/4] conf: Always format storage source auth and encryption under <source> for backing files
Peter Krempa
pkrempa at redhat.com
Fri Jan 10 16:57:22 UTC 2020
Historically there are two places where we format authentication and
encryption for a disk. The logich which formats it for backing files was
flawed though and didn't format it at all. This worked if the image
became a backing file through the means of a snapshot but not directly.
Force formatting of the source and encryption for any non-disk case to
fix the issue.
This caused problems in many places as we use the formatter to copy the
definition. Effectively any copy lost the secret definition.
https://bugzilla.redhat.com/show_bug.cgi?id=1789310
https://bugzilla.redhat.com/show_bug.cgi?id=1788898
Signed-off-by: Peter Krempa <pkrempa at redhat.com>
---
src/conf/backup_conf.c | 2 +-
src/conf/domain_conf.c | 13 ++++++++-----
src/conf/domain_conf.h | 1 +
src/conf/snapshot_conf.c | 2 +-
src/qemu/qemu_domain.c | 4 ++--
tests/qemublocktest.c | 2 +-
.../luks-disks-source-qcow2.x86_64-latest.xml | 6 +++++-
tests/virstoragetest.c | 2 +-
8 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c
index aa11967d2a..61dc8cd4b2 100644
--- a/src/conf/backup_conf.c
+++ b/src/conf/backup_conf.c
@@ -338,7 +338,7 @@ virDomainBackupDiskDefFormat(virBufferPtr buf,
virStorageFileFormatTypeToString(disk->store->format));
if (virDomainDiskSourceFormat(&childBuf, disk->store, sourcename,
- 0, false, storageSourceFormatFlags, NULL) < 0)
+ 0, false, storageSourceFormatFlags, true, NULL) < 0)
return -1;
}
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1290241923..ca5bbc3f35 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -24189,6 +24189,8 @@ virDomainDiskSourceFormatPrivateData(virBufferPtr buf,
* @policy: startup policy attribute value, if necessary
* @attrIndex: the 'index' attribute of <source> is formatted if true
* @flags: XML formatter flags
+ * @formatsecrets: Force formatting of <auth> and <encryption> even if they
+ * were inherited
* @xmlopt: XML formatter callbacks
*
* Formats @src into a <source> element. Note that this doesn't format the
@@ -24201,6 +24203,7 @@ virDomainDiskSourceFormat(virBufferPtr buf,
int policy,
bool attrIndex,
unsigned int flags,
+ bool formatsecrets,
virDomainXMLOptionPtr xmlopt)
{
g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER;
@@ -24257,13 +24260,13 @@ virDomainDiskSourceFormat(virBufferPtr buf,
* <auth> for a volume source type. The <auth> information is
* kept in the storage pool and would be overwritten anyway.
* So avoid formatting it for volumes. */
- if (src->auth && src->authInherited &&
+ if (src->auth && (src->authInherited || formatsecrets) &&
src->type != VIR_STORAGE_TYPE_VOLUME)
virStorageAuthDefFormat(&childBuf, src->auth);
/* If we found encryption as a child of <source>, then format it
* as we found it. */
- if (src->encryption && src->encryptionInherited &&
+ if (src->encryption && (src->encryptionInherited || formatsecrets) &&
virStorageEncryptionFormat(&childBuf, src->encryption) < 0)
return -1;
@@ -24324,7 +24327,7 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf,
virBufferAsprintf(&childBuf, "<format type='%s'/>\n",
virStorageFileFormatTypeToString(backingStore->format));
if (virDomainDiskSourceFormat(&childBuf, backingStore, "source", 0, false,
- flags, xmlopt) < 0)
+ flags, true, xmlopt) < 0)
return -1;
if (virDomainDiskBackingStoreFormat(&childBuf, backingStore, xmlopt, flags) < 0)
@@ -24486,7 +24489,7 @@ virDomainDiskDefFormatMirror(virBufferPtr buf,
virBufferEscapeString(&childBuf, "<format type='%s'/>\n", formatStr);
if (virDomainDiskSourceFormat(&childBuf, disk->mirror, "source", 0, true,
- flags, xmlopt) < 0)
+ flags, true, xmlopt) < 0)
return -1;
if (virDomainDiskBackingStoreFormat(&childBuf, disk->mirror, xmlopt, flags) < 0)
@@ -24585,7 +24588,7 @@ virDomainDiskDefFormat(virBufferPtr buf,
virStorageAuthDefFormat(buf, def->src->auth);
if (virDomainDiskSourceFormat(buf, def->src, "source", def->startupPolicy,
- true, flags, xmlopt) < 0)
+ true, flags, false, xmlopt) < 0)
return -1;
/* Don't format backingStore to inactive XMLs until the code for
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 76d6ef8e48..6ae89fa498 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3118,6 +3118,7 @@ int virDomainDiskSourceFormat(virBufferPtr buf,
int policy,
bool attrIndex,
unsigned int flags,
+ bool formatsecrets,
virDomainXMLOptionPtr xmlopt);
int
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 2bd4d6a276..37b5c2fdf7 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -818,7 +818,7 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
if (disk->src->format > 0)
virBufferEscapeString(buf, "<driver type='%s'/>\n",
virStorageFileFormatTypeToString(disk->src->format));
- if (virDomainDiskSourceFormat(buf, disk->src, "source", 0, false, 0,
+ if (virDomainDiskSourceFormat(buf, disk->src, "source", 0, false, 0, true,
xmlopt) < 0)
return -1;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1f358544ab..a6dde15bad 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2547,7 +2547,7 @@ qemuDomainObjPrivateXMLFormatBlockjobFormatSource(virBufferPtr buf,
virStorageTypeToString(src->type),
virStorageFileFormatTypeToString(src->format));
- if (virDomainDiskSourceFormat(&childBuf, src, "source", 0, true, xmlflags, xmlopt) < 0)
+ if (virDomainDiskSourceFormat(&childBuf, src, "source", 0, true, xmlflags, true, xmlopt) < 0)
return -1;
if (chain &&
@@ -2746,7 +2746,7 @@ qemuDomainObjPrivateXMLFormatNBDMigrationSource(virBufferPtr buf,
virStorageFileFormatTypeToString(src->format));
if (virDomainDiskSourceFormat(&childBuf, src, "source", 0, false,
- VIR_DOMAIN_DEF_FORMAT_STATUS, xmlopt) < 0)
+ VIR_DOMAIN_DEF_FORMAT_STATUS, true, xmlopt) < 0)
return -1;
virXMLFormatElement(buf, "migrationSource", &attrBuf, &childBuf);
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 3e9edb85f0..7ff6a6b17b 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -90,7 +90,7 @@ testBackingXMLjsonXML(const void *args)
return -1;
}
- if (virDomainDiskSourceFormat(&buf, jsonsrc, "source", 0, false, 0,
+ if (virDomainDiskSourceFormat(&buf, jsonsrc, "source", 0, false, 0, true,
NULL) < 0 ||
!(actualxml = virBufferContentAndReset(&buf))) {
fprintf(stderr, "failed to format disk source xml\n");
diff --git a/tests/qemuxml2xmloutdata/luks-disks-source-qcow2.x86_64-latest.xml b/tests/qemuxml2xmloutdata/luks-disks-source-qcow2.x86_64-latest.xml
index 33e6d02976..aa13fe5dfa 100644
--- a/tests/qemuxml2xmloutdata/luks-disks-source-qcow2.x86_64-latest.xml
+++ b/tests/qemuxml2xmloutdata/luks-disks-source-qcow2.x86_64-latest.xml
@@ -83,7 +83,11 @@
</source>
<backingStore type='file'>
<format type='qcow2'/>
- <source file='/storage/guest_disks/base.qcow2'/>
+ <source file='/storage/guest_disks/base.qcow2'>
+ <encryption format='luks'>
+ <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/>
+ </encryption>
+ </source>
<backingStore/>
</backingStore>
<target dev='vdf' bus='virtio'/>
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 0e274ad1b7..2862758752 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -612,7 +612,7 @@ testBackingParse(const void *args)
return -1;
}
- if (virDomainDiskSourceFormat(&buf, src, "source", 0, false, 0, NULL) < 0 ||
+ if (virDomainDiskSourceFormat(&buf, src, "source", 0, false, 0, true, NULL) < 0 ||
!(xml = virBufferContentAndReset(&buf))) {
fprintf(stderr, "failed to format disk source xml\n");
return -1;
--
2.24.1
More information about the libvir-list
mailing list