[PATCH v3 4/5] conf: add encryption engine property

Peter Krempa pkrempa at redhat.com
Thu Oct 7 07:43:37 UTC 2021


On Wed, Oct 06, 2021 at 05:18:45 -0500, Or Ozeri wrote:
> This commit extends libvirt XML configuration to support a custom encryption engine.
> This means that <encryption format="luks" engine="qemu">  becomes valid.
> The only engine for now is qemu. However, a new engine (librbd) will be added in an upcoming commit.
> If no engine is specified, qemu will be used (assuming qemu driver is used).
> 
> Signed-off-by: Or Ozeri <oro at il.ibm.com>
> ---
>  docs/formatstorageencryption.html.in          |   6 +
>  docs/schemas/domainbackup.rng                 |   7 +
>  docs/schemas/storagecommon.rng                |   7 +
>  src/conf/storage_encryption_conf.c            |  31 +++-
>  src/conf/storage_encryption_conf.h            |   9 +
>  src/qemu/qemu_block.c                         |   2 +
>  src/qemu/qemu_domain.c                        |   8 +
>  tests/qemustatusxml2xmldata/upgrade-out.xml   |   6 +-
>  tests/qemuxml2xmloutdata/disk-nvme.xml        |  65 ++++++-
>  .../disk-slices.x86_64-latest.xml             |   4 +-
>  .../encrypted-disk-usage.xml                  |  38 ++++-
>  tests/qemuxml2xmloutdata/encrypted-disk.xml   |   2 +-
>  .../luks-disks-source-qcow2.x86_64-latest.xml |  14 +-
>  .../qemuxml2xmloutdata/luks-disks-source.xml  |  10 +-
>  tests/qemuxml2xmloutdata/luks-disks.xml       |  47 +++++-
>  tests/qemuxml2xmloutdata/user-aliases.xml     | 159 +++++++++++++++++-
>  16 files changed, 392 insertions(+), 23 deletions(-)
>  mode change 120000 => 100644 tests/qemuxml2xmloutdata/disk-nvme.xml
>  mode change 120000 => 100644 tests/qemuxml2xmloutdata/encrypted-disk-usage.xml
>  mode change 120000 => 100644 tests/qemuxml2xmloutdata/luks-disks.xml
>  mode change 120000 => 100644 tests/qemuxml2xmloutdata/user-aliases.xml

[...]

> diff --git a/src/conf/storage_encryption_conf.c b/src/conf/storage_encryption_conf.c
> index 2df4ec96af..e8da02b605 100644
> --- a/src/conf/storage_encryption_conf.c
> +++ b/src/conf/storage_encryption_conf.c
> @@ -47,6 +47,11 @@ VIR_ENUM_IMPL(virStorageEncryptionFormat,
>                "default", "qcow", "luks", "luks2",
>  );
>  
> +VIR_ENUM_IMPL(virStorageEncryptionEngine,
> +              VIR_STORAGE_ENCRYPTION_ENGINE_LAST,
> +              "default", "qemu",
> +);
> +
>  static void
>  virStorageEncryptionInfoDefClear(virStorageEncryptionInfoDef *def)
>  {
> @@ -120,6 +125,7 @@ virStorageEncryptionCopy(const virStorageEncryption *src)
>      ret->secrets = g_new0(virStorageEncryptionSecret *, src->nsecrets);
>      ret->nsecrets = src->nsecrets;
>      ret->format = src->format;
> +    ret->engine = src->engine;
>  
>      for (i = 0; i < src->nsecrets; i++) {
>          if (!(ret->secrets[i] = virStorageEncryptionSecretCopy(src->secrets[i])))
> @@ -217,6 +223,7 @@ virStorageEncryptionParseNode(xmlNodePtr node,
>      xmlNodePtr *nodes = NULL;
>      virStorageEncryption *encdef = NULL;
>      virStorageEncryption *ret = NULL;
> +    g_autofree char *engine_str = NULL;
>      g_autofree char *format_str = NULL;
>      int n;
>      size_t i;
> @@ -239,6 +246,16 @@ virStorageEncryptionParseNode(xmlNodePtr node,
>          goto cleanup;
>      }
>  
> +    if ((engine_str = virXPathString("string(./@engine)", ctxt))) {
> +        if ((encdef->engine =
> +             virStorageEncryptionEngineTypeFromString(engine_str)) < 0) {

'default' must not be an allowed value, thus <=

> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("unknown volume encryption engine type %s"),
> +                           engine_str);
> +            goto cleanup;
> +        }
> +    }

But you can replace all of above and parse this as:

    if (virXMLPropEnum(node, "engine",
                       virStorageEncryptionEngineTypeFromString,
                       VIR_XML_PROP_NONZERO,
                       &encdef->engine) < 0)
      goto cleanup;

This does the proper type check and everything internally and doesn't
require temp variables. Additionally it allows you to declare 'engine'
as the correct enum type [1].


> +
>      if ((n = virXPathNodeSet("./secret", ctxt, &nodes)) < 0)
>          goto cleanup;
>  

[...]

> diff --git a/src/conf/storage_encryption_conf.h b/src/conf/storage_encryption_conf.h
> index 32e3a1243a..c722f832f5 100644
> --- a/src/conf/storage_encryption_conf.h
> +++ b/src/conf/storage_encryption_conf.h

[...]

> @@ -64,6 +72,7 @@ VIR_ENUM_DECL(virStorageEncryptionFormat);
>  
>  typedef struct _virStorageEncryption virStorageEncryption;
>  struct _virStorageEncryption {
> +    int engine; /* virStorageEncryptionEngineType */

[1]

Declare this as:

    virStorageEncryptionEngine engine;

rather than using integer.

>      int format; /* virStorageEncryptionFormatType */
>      int payload_offset;
>

[...]

> diff --git a/tests/qemuxml2xmloutdata/disk-nvme.xml b/tests/qemuxml2xmloutdata/disk-nvme.xml
> deleted file mode 120000
> index ea9eb267ac..0000000000
> --- a/tests/qemuxml2xmloutdata/disk-nvme.xml
> +++ /dev/null
> @@ -1 +0,0 @@
> -../qemuxml2argvdata/disk-nvme.xml

Files that were originally symlinks ...

> \ No newline at end of file
> diff --git a/tests/qemuxml2xmloutdata/disk-nvme.xml b/tests/qemuxml2xmloutdata/disk-nvme.xml
> new file mode 100644
> index 0000000000..9a5fafce7d
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/disk-nvme.xml

... must not be expanded to full files. That is a drawback of using sed
-i to do the conversion.

> @@ -0,0 +1,64 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> index be5cd25084..a058cbad61 100644

[..]

> diff --git a/tests/qemuxml2xmloutdata/encrypted-disk-usage.xml b/tests/qemuxml2xmloutdata/encrypted-disk-usage.xml
> deleted file mode 120000
> index a1a4f841e9..0000000000
> --- a/tests/qemuxml2xmloutdata/encrypted-disk-usage.xml
> +++ /dev/null
> @@ -1 +0,0 @@
> -../qemuxml2argvdata/encrypted-disk-usage.xml

symlink ...

> \ No newline at end of file
> diff --git a/tests/qemuxml2xmloutdata/encrypted-disk-usage.xml b/tests/qemuxml2xmloutdata/encrypted-disk-usage.xml
> new file mode 100644
> index 0000000000..d2b87b94b6
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/encrypted-disk-usage.xml
> @@ -0,0 +1,37 @@
> +<domain type='qemu'>
> +  <name>encryptdisk</name>

[...]

> diff --git a/tests/qemuxml2xmloutdata/luks-disks.xml b/tests/qemuxml2xmloutdata/luks-disks.xml
> deleted file mode 120000
> index d65e470c32..0000000000
> --- a/tests/qemuxml2xmloutdata/luks-disks.xml
> +++ /dev/null
> @@ -1 +0,0 @@
> -../qemuxml2argvdata/luks-disks.xml
> \ No newline at end of file

symlink

> diff --git a/tests/qemuxml2xmloutdata/luks-disks.xml b/tests/qemuxml2xmloutdata/luks-disks.xml
> new file mode 100644
> index 0000000000..1c76f0dc26
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/luks-disks.xml
> @@ -0,0 +1,46 @@
> +<domain type='qemu'>
> +  <name>encryptdisk</name>
> +  <uuid>496898a6-e6ff-f7c8-5dc2-3cf410945ee9</uuid>

[...]

> diff --git a/tests/qemuxml2xmloutdata/user-aliases.xml b/tests/qemuxml2xmloutdata/user-aliases.xml
> deleted file mode 120000
> index b5a27f08cd..0000000000
> --- a/tests/qemuxml2xmloutdata/user-aliases.xml
> +++ /dev/null
> @@ -1 +0,0 @@
> -../qemuxml2argvdata/user-aliases.xml

symlink

> \ No newline at end of file
> diff --git a/tests/qemuxml2xmloutdata/user-aliases.xml b/tests/qemuxml2xmloutdata/user-aliases.xml
> new file mode 100644
> index 0000000000..10b7749521
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/user-aliases.xml
> @@ -0,0 +1,158 @@
> +<domain type='kvm'>
> +  <name>gentoo</name>
> +  <uuid>a75aca4b-a02f-2bcb-4a91-c93cd848c34b</uuid>
> +  <memory unit='KiB'>4194304</memory>




More information about the libvir-list mailing list