Re: [libvirt] [PATCH 4/4] conf: Always format storage source auth and encryption under <source> for backing files

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(a)redhat.com&gt; --- 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;