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

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;
     }
 

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

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

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;
 

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);

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");

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'/>

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;