conf: Always format storage source auth and encryption under <source> for backing...

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@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c
index aa11967d2aae0888241d7935febb56362ed1d789..61dc8cd4b2f9d05cd55e838be4ea638c749e5596 100644 (file)
--- 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 12902419235f37e9250b8e1eea144820d3f11a18..74b4a933ae8b5859d3325439dffdf77166da8bad 100644 (file)
--- 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> under <source>
+ *                 regardless of the original definition state
  * @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 76d6ef8e48a264363d52867508d81946e3ebda0a..6ae89fa498c5480dc0f9defafbe34a67f2fd08e1 100644 (file)
--- 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 2bd4d6a276c8c67b7b3a79b00aae6b42651ddfcb..37b5c2fdf7adb8425a9909d80d1b81c78f069c81 100644 (file)
--- 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 1f358544ab6c3c82159705f3ad0e23282b6b1167..a6dde15bad1929e854d9dc1ec8beeb2d8333f284 100644 (file)
--- 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 3e9edb85f01ad577b8b4f84ac927306833b65c74..7ff6a6b17bea240096886ca024677345e89e450f 100644 (file)
--- 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 33e6d029762484b8978b85e6090dd0c73bb55f77..aa13fe5dfad04b5a5f0b1d54bd985a7e938b1c22 100644 (file)
--- 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 0e274ad1b7fa9c6898f883800739cf5c19fdcb9f..2862758752f00842cd7fa4a67e79d74adcef2353 100644 (file)
--- 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;