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

[Jiri Denemark]

On Fri, Jan 10, 2020 at 17:57:22 +0100, Peter Krempa wrote:
> 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

The current code is pretty confusing as the {auth,encryption}Inherited
bools do not have the same semantics as the "inherited" word above
(otherwise the code would be doing the opposite of what is described
here), but the fix does what it's supposed to do. Hopefully someone will
clean up the existing code later...

>   * @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;
> 

Jirka