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@xxxxxxxxxx>
> ---
> 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>
