RE: [PATCH v2 0/6] Introduce active RBD disk snapshot support

[Or Ozeri]

Hi Peter,

I'm missing some clarifications here:
https://listman.redhat.com/archives/libvir-list/2023-March/238546.html
https://listman.redhat.com/archives/libvir-list/2023-March/238548.html

Could you please have a look?

Thanks,
Or

> -Original Message-
> From: Peter Krempa 
> Sent: Monday, 20 February 2023 16:24
> To: Or Ozeri 
> Cc: libvir-list@redhat.com; Danny Harnik ; Pavel
> Hrdina 
> Subject: [EXTERNAL] Re: [PATCH v2 0/6] Introduce active RBD disk snapshot
> support
> 
> On Wed, Feb 15, 2023 at 05:28:16 -0600, Or Ozeri wrote:
> > Internal disk snapshots are currently only supported on non-active VMs.
> > For RBD disks only, this patch series extends this support for active VMs
> running with qemu.
> > We also add the option to set a name for each RBD snapshot, and allow
> taking them alongside other external disk snapshots (mixing).
> > Deletion and reverting to snapshots containing RBD snapshots is not
> allowed, and is validated.
> > Note that taking RBD snapshots on a non-active VM is still unsupported.
> >
> > Changes in v2:
> > - reduce patch to RBD use-case only (e.g. not including qcow2 internal
> snapshots)
> > - add validation to disallow RBD snapshots deletion / reverting
> >
> > Or Ozeri (6):
> >   conf: Add snapshotName attribute for internal disk snapshot
> >   qemu: Block deletion and reverting on non-full internal snapshots
> >   qemu: Add internal support for active disk internal snapshots
> >   qemu: Allow active disk snapshots for RBD disks
> >   qemu: Allow setting per-disk snapshot name for RBD disks
> >   qemu: Allow mixing active internal and external active disk
> > snapshots
> 
> Note that also Pavel (CC'd) might want to add more comments as he's
> currently working on the snapshot code.

[PATCH v2 7/7] qemu: add luks-any encryption support for RBD images

[Or Ozeri]

The newly added luks-any rbd encryption format in qemu
allows for opening both LUKS and LUKS2 encryption formats.
This commit enables libvirt uses to use this wildcard format.

Signed-off-by: Or Ozeri 
---
 docs/formatstorageencryption.rst  |  9 
 src/conf/schemas/storagecommon.rng|  1 +
 src/conf/storage_encryption_conf.c|  2 +-
 src/conf/storage_encryption_conf.h|  1 +
 src/libvirt_private.syms  |  1 +
 src/qemu/qemu_block.c | 10 +++--
 src/qemu/qemu_domain.c| 29 +++-
 ...k-rbd-encryption-luks-any.x86_64-7.2.0.err |  1 +
 ...rbd-encryption-luks-any.x86_64-latest.args | 37 
 .../disk-network-rbd-encryption-luks-any.xml  | 39 
 tests/qemuxml2argvtest.c  |  2 +
 ...-rbd-encryption-luks-any.x86_64-latest.xml | 44 +++
 tests/qemuxml2xmltest.c   |  1 +
 13 files changed, 170 insertions(+), 7 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-luks-any.x86_64-7.2.0.err
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-luks-any.x86_64-latest.args
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-luks-any.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-network-rbd-encryption-luks-any.x86_64-latest.xml

diff --git a/docs/formatstorageencryption.rst b/docs/formatstorageencryption.rst
index 3b3e9ea379..071ea8f4d1 100644
--- a/docs/formatstorageencryption.rst
+++ b/docs/formatstorageencryption.rst
@@ -109,6 +109,15 @@ to a qemu VM using the qemu VM driver. A single
  element is expected (except for the case of
 RBD layered encryption mentioned above).
 
+``luks-any`` format
+~~~
+
+The ``luks-any`` format is currently supported only by the ``librbd`` engine,
+and can only be applied to RBD network disks (RBD images). This format will try
+to parse the disk as either LUKS or LUKS2, depending on the actual on-disk
+format. A single  element is expected (except
+for the case of RBD layered encryption mentioned above) :since:`Since 9.3.0` .
+
 Examples
 
 
diff --git a/src/conf/schemas/storagecommon.rng 
b/src/conf/schemas/storagecommon.rng
index 225456f03c..14704c737e 100644
--- a/src/conf/schemas/storagecommon.rng
+++ b/src/conf/schemas/storagecommon.rng
@@ -14,6 +14,7 @@
   qcow
   luks
   luks2
+  luks-any
 
   
   
diff --git a/src/conf/storage_encryption_conf.c 
b/src/conf/storage_encryption_conf.c
index 3651ff8cfd..639cbf2e58 100644
--- a/src/conf/storage_encryption_conf.c
+++ b/src/conf/storage_encryption_conf.c
@@ -41,7 +41,7 @@ VIR_ENUM_IMPL(virStorageEncryptionSecret,
 
 VIR_ENUM_IMPL(virStorageEncryptionFormat,
   VIR_STORAGE_ENCRYPTION_FORMAT_LAST,
-  "default", "qcow", "luks", "luks2",
+  "default", "qcow", "luks", "luks2", "luks-any",
 );
 
 VIR_ENUM_IMPL(virStorageEncryptionEngine,
diff --git a/src/conf/storage_encryption_conf.h 
b/src/conf/storage_encryption_conf.h
index 312599ad44..03f0e60feb 100644
--- a/src/conf/storage_encryption_conf.h
+++ b/src/conf/storage_encryption_conf.h
@@ -66,6 +66,7 @@ typedef enum {
 VIR_STORAGE_ENCRYPTION_FORMAT_QCOW, /* Both qcow and qcow2 */
 VIR_STORAGE_ENCRYPTION_FORMAT_LUKS,
 VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2,
+VIR_STORAGE_ENCRYPTION_FORMAT_LUKS_ANY,
 
 VIR_STORAGE_ENCRYPTION_FORMAT_LAST,
 } virStorageEncryptionFormatType;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6f44788233..3187112ca3 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1095,6 +1095,7 @@ virStorageVolTypeToString;
 
 # conf/storage_encryption_conf.h
 virStorageEncryptionFormat;
+virStorageEncryptionFormatTypeToString;
 virStorageEncryptionFree;
 virStorageEncryptionParseNode;
 
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index d50cfa20c5..3fafc31084 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -575,6 +575,10 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 encformat = "luks2";
 break;
 
+case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS_ANY:
+encformat = "luks-any";
+break;
+
 case VIR_STORAGE_ENCRYPTION_FORMAT_QCOW:
 case VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT:
 case VIR_STORAGE_ENCRYPTION_FORMAT_LAST:
@@ -1052,10 +1056,8 @@ qemuBlockStorageSourceGetCryptoProps(virStorageSource 
*src,
 break;
 
 case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2:
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("luks2 is currently not supported by the qemu 
encryption engine"));
-return -1;
-

[PATCH v2 6/7] qemu: capabilities: Introduce QEMU_CAPS_RBD_ENCRYPTION_LUKS_ANY capability

[Or Ozeri]

This capability represents that qemu supports the "luks-any" encryption
format for RBD images.
Both LUKS and LUKS2 formats can be parsed using this wildcard format.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml | 1 +
 3 files changed, 4 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c03ad714fa..22b0d2ca86 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -692,6 +692,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "netdev.stream.reconnect", /* QEMU_CAPS_NETDEV_STREAM_RECONNECT 
*/
   "virtio-gpu.blob", /* QEMU_CAPS_VIRTIO_GPU_BLOB */
   "rbd-encryption-layering", /* QEMU_CAPS_RBD_ENCRYPTION_LAYERING 
*/
+  "rbd-encryption-luks-any", /* QEMU_CAPS_RBD_ENCRYPTION_LUKS_ANY 
*/
 );
 
 
@@ -1558,6 +1559,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsQMPSchemaQueries[] = {
 { "blockdev-add/arg-type/+file/aio/^io_uring", QEMU_CAPS_AIO_IO_URING },
 { "blockdev-add/arg-type/+rbd/encrypt", QEMU_CAPS_RBD_ENCRYPTION },
 { "blockdev-add/arg-type/+rbd/encrypt/parent", 
QEMU_CAPS_RBD_ENCRYPTION_LAYERING },
+{ "blockdev-add/arg-type/+rbd/encrypt/format/^luks-any", 
QEMU_CAPS_RBD_ENCRYPTION_LUKS_ANY },
 { "blockdev-add/arg-type/+nbd/tls-hostname", 
QEMU_CAPS_BLOCKDEV_NBD_TLS_HOSTNAME },
 { "blockdev-snapshot/$allow-write-only-overlay", 
QEMU_CAPS_BLOCKDEV_SNAPSHOT_ALLOW_WRITE_ONLY },
 { "chardev-add/arg-type/backend/+socket/data/reconnect", 
QEMU_CAPS_CHARDEV_RECONNECT },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 2d2c5f8eaf..c4f1708639 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -671,6 +671,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_NETDEV_STREAM_RECONNECT, /* -netdev stream supports reconnect */
 QEMU_CAPS_VIRTIO_GPU_BLOB, /* -device virtio-gpu-*.blob= */
 QEMU_CAPS_RBD_ENCRYPTION_LAYERING, /* layered encryption support for Ceph 
RBD */
+QEMU_CAPS_RBD_ENCRYPTION_LUKS_ANY, /* luks-any (LUKS and LUKS2) encryption 
format for Ceph RBD */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml
index d120f5dc3c..74128be904 100644
--- a/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml
@@ -205,6 +205,7 @@
   
   
   
+  
   7002050
   0
   43100244
-- 
2.25.1

[PATCH v2 5/7] qemu: add support for librbd layered encryption

[Or Ozeri]

This commit enables libvirt users to use layered encryption
of RBD images, using the librbd encryption engine.
This allows opening of an encrypted cloned image
whose parent is encrypted with a possibly different encryption key.
To open such images, multiple encryption secrets are expected
to be defined under the encryption XML tag.

Signed-off-by: Or Ozeri 
---
 docs/formatstorageencryption.rst  | 11 +++--
 src/conf/schemas/storagecommon.rng|  4 +-
 src/qemu/qemu_block.c | 20 ++--
 src/qemu/qemu_domain.c| 14 ++
 src/qemu/qemu_validate.c  |  8 
 ...k-rbd-encryption-layering.x86_64-7.2.0.err |  1 +
 ...rbd-encryption-layering.x86_64-latest.args | 39 
 .../disk-network-rbd-encryption-layering.xml  | 41 +
 tests/qemuxml2argvtest.c  |  2 +
 ...-rbd-encryption-layering.x86_64-latest.xml | 46 +++
 tests/qemuxml2xmltest.c   |  1 +
 11 files changed, 178 insertions(+), 9 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-layering.x86_64-7.2.0.err
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-layering.x86_64-latest.args
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-layering.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-network-rbd-encryption-layering.x86_64-latest.xml

diff --git a/docs/formatstorageencryption.rst b/docs/formatstorageencryption.rst
index 2c19473d6b..3b3e9ea379 100644
--- a/docs/formatstorageencryption.rst
+++ b/docs/formatstorageencryption.rst
@@ -28,7 +28,10 @@ network disks. If the engine tag is not specified, the 
``qemu`` engine will be
 used by default (assuming the qemu driver is used). Note that ``librbd`` engine
 is currently only supported by the qemu VM driver, and is not supported by the
 storage driver. Furthermore, the storage driver currently ignores the 
``engine``
-tag.
+tag. :since:`since 9.3.0` RBD layered encryption is supported. Layered
+encryption requires a secret per each encrypted layer. The first secret
+corresponds to the (child) image itself, the second secret to the parent image,
+and so forth.
 
 The ``encryption`` tag can currently contain a sequence of ``secret`` tags, 
each
 with mandatory attributes ``type`` and either ``uuid`` or ``usage`` (
@@ -55,7 +58,8 @@ added to libvirt.
 The ``luks`` format is specific to a luks encrypted volume and the secret is
 used in order to either encrypt during volume creation or decrypt the volume 
for
 usage by the domain. A single  element is
-expected. :since:`Since 2.1.0` .
+expected (except for the case of RBD layered encryption mentioned above).
+:since:`Since 2.1.0` .
 
 For volume creation, it is possible to specify the encryption algorithm used to
 encrypt the luks volume. The following two optional elements may be provided 
for
@@ -102,7 +106,8 @@ can only be applied to RBD network disks (RBD images). 
Since the ``librbd``
 engine is currently not supported by the libvirt storage driver, you cannot use
 it to control such disks. However, pre-formatted RBD luks2 disks can be loaded
 to a qemu VM using the qemu VM driver. A single
- element is expected.
+ element is expected (except for the case of
+RBD layered encryption mentioned above).
 
 Examples
 
diff --git a/src/conf/schemas/storagecommon.rng 
b/src/conf/schemas/storagecommon.rng
index 23eff9ecb1..225456f03c 100644
--- a/src/conf/schemas/storagecommon.rng
+++ b/src/conf/schemas/storagecommon.rng
@@ -26,7 +26,9 @@
   
   
 
-  
+  
+
+  
   
 
   
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 0cc3b82cca..d50cfa20c5 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -564,6 +564,8 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 
 if (src->encryption &&
 src->encryption->engine == VIR_STORAGE_ENCRYPTION_ENGINE_LIBRBD) {
+size_t i;
+
 switch ((virStorageEncryptionFormatType) src->encryption->format) {
 case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS:
 encformat = "luks";
@@ -580,11 +582,19 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 break;
 }
 
-if (virJSONValueObjectAdd(&encrypt,
-  "s:format", encformat,
-  "s:key-secret", srcPriv->encinfo[0]->alias,
-  NULL) < 0)
-return NULL;
+for (i = src->encryption->nsecrets; i > 0; --i) {
+g_autoptr(virJSONValue) new = NULL;
+
+/* we consume the lower layer 'encrypt' into a new object */
+if (virJSONValueObjectAdd(&new,
+  "s:fo

[PATCH v2 2/7] qemu: add support for multiple secret aliases

[Or Ozeri]

Change secret aliases from %s-%s-secret0 to %s-%s-secret%lu,
which will later be used for storage encryption requiring more
than a single secret.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_alias.c|  8 +---
 src/qemu/qemu_alias.h|  3 ++-
 src/qemu/qemu_domain.c   | 14 --
 src/qemu/qemu_hotplug.c  |  2 +-
 src/qemu/qemu_migration_params.c |  2 +-
 5 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index a9809797d5..2e0a50b68b 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -801,17 +801,19 @@ qemuDomainGetMasterKeyAlias(void)
 /* qemuAliasForSecret:
  * @parentalias: alias of the parent object
  * @obj: optional sub-object of the parent device the secret is for
+ * @secret_idx: secret index number (0 in the case of a single secret)
  *
  * Generate alias for a secret object used by @parentalias device or one of
  * the dependencies of the device described by @obj.
  */
 char *
 qemuAliasForSecret(const char *parentalias,
-   const char *obj)
+   const char *obj,
+   size_t secret_idx)
 {
 if (obj)
-return g_strdup_printf("%s-%s-secret0", parentalias, obj);
-return g_strdup_printf("%s-secret0", parentalias);
+return g_strdup_printf("%s-%s-secret%lu", parentalias, obj, 
secret_idx);
+return g_strdup_printf("%s-secret%lu", parentalias, secret_idx);
 }
 
 /* qemuAliasTLSObjFromSrcAlias
diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
index f13f4cc5f8..eae08020dc 100644
--- a/src/qemu/qemu_alias.h
+++ b/src/qemu/qemu_alias.h
@@ -86,7 +86,8 @@ char *qemuAliasFromHostdev(const virDomainHostdevDef 
*hostdev);
 char *qemuDomainGetMasterKeyAlias(void);
 
 char *qemuAliasForSecret(const char *parentalias,
- const char *obj);
+ const char *obj,
+ size_t secret_idx);
 
 char *qemuAliasTLSObjFromSrcAlias(const char *srcAlias)
 ATTRIBUTE_NONNULL(1);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0feab09bee..f62fb453a9 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1317,6 +1317,7 @@ qemuDomainSecretInfoSetup(qemuDomainObjPrivate *priv,
  * @priv: pointer to domain private object
  * @srcalias: Alias of the disk/hostdev used to generate the secret alias
  * @secretuse: specific usage for the secret (may be NULL if main object is 
using it)
+ * @secret_idx: secret index number (0 in the case of a single secret)
  * @usageType: The virSecretUsageType
  * @username: username to use for authentication (may be NULL)
  * @seclookupdef: Pointer to seclookupdef data
@@ -1329,12 +1330,13 @@ static qemuDomainSecretInfo *
 qemuDomainSecretInfoSetupFromSecret(qemuDomainObjPrivate *priv,
 const char *srcalias,
 const char *secretuse,
+size_t secret_idx,
 virSecretUsageType usageType,
 const char *username,
 virSecretLookupTypeDef *seclookupdef)
 {
 qemuDomainSecretInfo *secinfo;
-g_autofree char *alias = qemuAliasForSecret(srcalias, secretuse);
+g_autofree char *alias = qemuAliasForSecret(srcalias, secretuse, 
secret_idx);
 g_autofree uint8_t *secret = NULL;
 size_t secretlen = 0;
 VIR_IDENTITY_AUTORESTORE virIdentity *oldident = 
virIdentityElevateCurrent();
@@ -1384,7 +1386,7 @@ qemuDomainSecretInfoTLSNew(qemuDomainObjPrivate *priv,
 }
 seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_UUID;
 
-return qemuDomainSecretInfoSetupFromSecret(priv, srcAlias, NULL,
+return qemuDomainSecretInfoSetupFromSecret(priv, srcAlias, NULL, 0,
VIR_SECRET_USAGE_TYPE_TLS,
NULL, &seclookupdef);
 }
@@ -1411,7 +1413,7 @@ 
qemuDomainSecretStorageSourcePrepareCookies(qemuDomainObjPrivate *priv,
 virStorageSource *src,
 const char *aliasprotocol)
 {
-g_autofree char *secretalias = qemuAliasForSecret(aliasprotocol, 
"httpcookie");
+g_autofree char *secretalias = qemuAliasForSecret(aliasprotocol, 
"httpcookie", 0);
 g_autofree char *cookies = qemuBlockStorageSourceGetCookieString(src);
 
 return qemuDomainSecretInfoSetup(priv, secretalias, NULL,
@@ -1460,7 +1462,7 @@ qemuDomainSecretStorageSourcePrepare(qemuDomainObjPrivate 
*priv,
 usageType = VIR_SECRET_USAGE_TYPE_CEPH;
 
 if (!(srcPriv->secinfo = qemuDomainSecretInfoSetupFromSecret(priv, 
aliasprotocol,
- "auth",
+ 

[PATCH v2 4/7] qemu: add multi-secret support in _qemuDomainStorageSourcePrivate

[Or Ozeri]

This commit changes the _qemuDomainStorageSourcePrivate struct
to support multiple secrets (instead of a single one before this commit).
This will useful for storage encryption requiring more than a single secret.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_block.c | 25 +---
 src/qemu/qemu_command.c   | 20 +++---
 src/qemu/qemu_domain.c| 75 ++-
 src/qemu/qemu_domain.h|  3 +-
 tests/qemublocktest.c |  7 ++-
 tests/qemustatusxml2xmldata/modern-in.xml | 14 +
 6 files changed, 108 insertions(+), 36 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 9e1ecf68f9..0cc3b82cca 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -582,7 +582,7 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 
 if (virJSONValueObjectAdd(&encrypt,
   "s:format", encformat,
-  "s:key-secret", srcPriv->encinfo->alias,
+  "s:key-secret", srcPriv->encinfo[0]->alias,
   NULL) < 0)
 return NULL;
 }
@@ -979,7 +979,8 @@ qemuBlockStorageSourceGetFormatLUKSProps(virStorageSource 
*src,
 {
 qemuDomainStorageSourcePrivate *srcPriv = 
QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
 
-if (!srcPriv || !srcPriv->encinfo || !srcPriv->encinfo->alias) {
+/* validation ensures that the qemu encryption engine accepts only a 
single secret */
+if (!srcPriv || !srcPriv->encinfo || !srcPriv->encinfo[0]->alias) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("missing secret info for 'luks' driver"));
 return -1;
@@ -987,7 +988,7 @@ qemuBlockStorageSourceGetFormatLUKSProps(virStorageSource 
*src,
 
 if (virJSONValueObjectAdd(&props,
   "s:driver", "luks",
-  "s:key-secret", srcPriv->encinfo->alias,
+  "s:key-secret", srcPriv->encinfo[0]->alias,
   NULL) < 0)
 return -1;
 
@@ -1053,9 +1054,10 @@ qemuBlockStorageSourceGetCryptoProps(virStorageSource 
*src,
 return -1;
 }
 
+/* validation ensures that the qemu encryption engine accepts only a 
single secret */
 return virJSONValueObjectAdd(encprops,
  "s:format", encformat,
- "s:key-secret", srcpriv->encinfo->alias,
+ "s:key-secret", srcpriv->encinfo[0]->alias,
  NULL);
 }
 
@@ -1617,10 +1619,15 @@ qemuBlockStorageSourceDetachPrepare(virStorageSource 
*src)
 data->authsecretAlias = g_strdup(srcpriv->secinfo->alias);
 
 if (srcpriv->encinfo) {
-data->encryptsecretCount = 1;
-data->encryptsecretProps = g_new0(virJSONValue *, 1);
-data->encryptsecretAlias = g_new0(char *, 1);
-data->encryptsecretAlias[0] = g_strdup(srcpriv->encinfo->alias);
+size_t i;
+
+data->encryptsecretCount = srcpriv->enccount;
+data->encryptsecretProps = g_new0(virJSONValue *, 
srcpriv->enccount);
+data->encryptsecretAlias = g_new0(char *, srcpriv->enccount);
+
+for (i = 0; i < srcpriv->enccount; ++i) {
+data->encryptsecretAlias[i] = 
g_strdup(srcpriv->encinfo[i]->alias);
+}
 }
 
 if (srcpriv->httpcookie)
@@ -1986,7 +1993,7 @@ 
qemuBlockStorageSourceCreateGetEncryptionLUKS(virStorageSource *src,
 
 if (srcpriv &&
 srcpriv->encinfo)
-keysecret = srcpriv->encinfo->alias;
+keysecret = srcpriv->encinfo[0]->alias;
 
 if (virJSONValueObjectAdd(&props,
   "s:key-secret", keysecret,
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f166e1c891..7c577ae6ca 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1603,7 +1603,7 @@ qemuBuildDriveSourceStr(virDomainDiskDef *disk,
 {
 virStorageType actualType = virStorageSourceGetActualType(disk->src);
 qemuDomainStorageSourcePrivate *srcpriv = 
QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
-qemuDomainSecretInfo *encinfo = NULL;
+qemuDomainSecretInfo **encinfo = NULL;
 g_autoptr(virJSONValue) srcprops = NULL;
 bool rawluks = false;
 
@@ -1647,12 +1647,12 @@ qemuBuildDriveSourceStr(virDomainDiskDef *disk,
 
 if (encinfo) {
 if (disk->src->format == VIR_STORAGE_FILE_RAW) {
-virBufferAsprintf(buf, "key-secret=%s,", encinfo-&g

[PATCH v2 3/7] qemu: add multi-secret support in qemuBlockStorageSourceAttachData

[Or Ozeri]

This commit changes the qemuBlockStorageSourceAttachData struct
to support multiple secrets (instead of a single one before this commit).
This will useful for storage encryption requiring more than a single secret.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_block.c| 32 +++-
 src/qemu/qemu_block.h|  5 +++--
 src/qemu/qemu_blockjob.c |  6 ++
 src/qemu/qemu_command.c  | 19 +++
 4 files changed, 47 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 8fcebd8992..9e1ecf68f9 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1311,6 +1311,7 @@ 
qemuBlockStorageSourceGetBlockdevStorageSliceProps(virStorageSource *src)
 void
 qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachData *data)
 {
+size_t i;
 if (!data)
 return;
 
@@ -1320,12 +1321,16 @@ 
qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachData *data)
 virJSONValueFree(data->prmgrProps);
 virJSONValueFree(data->authsecretProps);
 virJSONValueFree(data->httpcookiesecretProps);
-virJSONValueFree(data->encryptsecretProps);
+for (i = 0; i < data->encryptsecretCount; ++i) {
+virJSONValueFree(data->encryptsecretProps[i]);
+g_free(data->encryptsecretAlias[i]);
+}
 virJSONValueFree(data->tlsProps);
 virJSONValueFree(data->tlsKeySecretProps);
 g_free(data->tlsAlias);
 g_free(data->tlsKeySecretAlias);
 g_free(data->authsecretAlias);
+g_free(data->encryptsecretProps);
 g_free(data->encryptsecretAlias);
 g_free(data->httpcookiesecretAlias);
 g_free(data->driveCmd);
@@ -1436,10 +1441,12 @@ static int
 qemuBlockStorageSourceAttachApplyFormatDeps(qemuMonitor *mon,
 qemuBlockStorageSourceAttachData 
*data)
 {
-if (data->encryptsecretProps &&
-qemuMonitorAddObject(mon, &data->encryptsecretProps,
- &data->encryptsecretAlias) < 0)
-return -1;
+size_t i;
+for (i = 0; i < data->encryptsecretCount; ++i) {
+if (qemuMonitorAddObject(mon, &data->encryptsecretProps[i],
+ &data->encryptsecretAlias[i]) < 0)
+return -1;
+}
 
 return 0;
 }
@@ -1525,6 +1532,7 @@ qemuBlockStorageSourceAttachRollback(qemuMonitor *mon,
  qemuBlockStorageSourceAttachData *data)
 {
 virErrorPtr orig_err;
+size_t i;
 
 virErrorPreserveLast(&orig_err);
 
@@ -1550,8 +1558,10 @@ qemuBlockStorageSourceAttachRollback(qemuMonitor *mon,
 if (data->authsecretAlias)
 ignore_value(qemuMonitorDelObject(mon, data->authsecretAlias, false));
 
-if (data->encryptsecretAlias)
-ignore_value(qemuMonitorDelObject(mon, data->encryptsecretAlias, 
false));
+for (i = 0; i < data->encryptsecretCount; ++i) {
+if (data->encryptsecretAlias[i])
+ignore_value(qemuMonitorDelObject(mon, 
data->encryptsecretAlias[i], false));
+}
 
 if (data->httpcookiesecretAlias)
 ignore_value(qemuMonitorDelObject(mon, data->httpcookiesecretAlias, 
false));
@@ -1606,8 +1616,12 @@ qemuBlockStorageSourceDetachPrepare(virStorageSource 
*src)
 if (srcpriv->secinfo)
 data->authsecretAlias = g_strdup(srcpriv->secinfo->alias);
 
-if (srcpriv->encinfo)
-data->encryptsecretAlias = g_strdup(srcpriv->encinfo->alias);
+if (srcpriv->encinfo) {
+data->encryptsecretCount = 1;
+data->encryptsecretProps = g_new0(virJSONValue *, 1);
+data->encryptsecretAlias = g_new0(char *, 1);
+data->encryptsecretAlias[0] = g_strdup(srcpriv->encinfo->alias);
+}
 
 if (srcpriv->httpcookie)
 data->httpcookiesecretAlias = g_strdup(srcpriv->httpcookie->alias);
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index 5a61a19da2..530d88d28e 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -89,8 +89,9 @@ struct qemuBlockStorageSourceAttachData {
 virJSONValue *authsecretProps;
 char *authsecretAlias;
 
-virJSONValue *encryptsecretProps;
-char *encryptsecretAlias;
+size_t encryptsecretCount;
+virJSONValue **encryptsecretProps;
+char **encryptsecretAlias;
 
 virJSONValue *httpcookiesecretProps;
 char *httpcookiesecretAlias;
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index a20cf1db62..818e90022c 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1336,9 +1336,15 @@ qemuBlockJobProcessEventConcludedCreate(virQEMUDriver 
*driver,
 /* the format node part was not attached yet, so we don't need to detach 
it */
 backend->formatAttached = false;
 

[PATCH v2 1/7] qemu: capabilities: Introduce QEMU_CAPS_RBD_ENCRYPTION_LAYERING capability

[Or Ozeri]

This capability represents that qemu supports the layered encryption
of RBD images, where a cloned image is encrypted with a possible
different encryption than its parent image.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml | 1 +
 3 files changed, 4 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 218e6c09a4..c03ad714fa 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -691,6 +691,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
   /* 445 */
   "netdev.stream.reconnect", /* QEMU_CAPS_NETDEV_STREAM_RECONNECT 
*/
   "virtio-gpu.blob", /* QEMU_CAPS_VIRTIO_GPU_BLOB */
+  "rbd-encryption-layering", /* QEMU_CAPS_RBD_ENCRYPTION_LAYERING 
*/
 );
 
 
@@ -1556,6 +1557,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsQMPSchemaQueries[] = {
 { "blockdev-add/arg-type/+nvme", QEMU_CAPS_DRIVE_NVME },
 { "blockdev-add/arg-type/+file/aio/^io_uring", QEMU_CAPS_AIO_IO_URING },
 { "blockdev-add/arg-type/+rbd/encrypt", QEMU_CAPS_RBD_ENCRYPTION },
+{ "blockdev-add/arg-type/+rbd/encrypt/parent", 
QEMU_CAPS_RBD_ENCRYPTION_LAYERING },
 { "blockdev-add/arg-type/+nbd/tls-hostname", 
QEMU_CAPS_BLOCKDEV_NBD_TLS_HOSTNAME },
 { "blockdev-snapshot/$allow-write-only-overlay", 
QEMU_CAPS_BLOCKDEV_SNAPSHOT_ALLOW_WRITE_ONLY },
 { "chardev-add/arg-type/backend/+socket/data/reconnect", 
QEMU_CAPS_CHARDEV_RECONNECT },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 21f23cff96..2d2c5f8eaf 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -670,6 +670,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 /* 445 */
 QEMU_CAPS_NETDEV_STREAM_RECONNECT, /* -netdev stream supports reconnect */
 QEMU_CAPS_VIRTIO_GPU_BLOB, /* -device virtio-gpu-*.blob= */
+QEMU_CAPS_RBD_ENCRYPTION_LAYERING, /* layered encryption support for Ceph 
RBD */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml
index ee35ed2258..d120f5dc3c 100644
--- a/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml
@@ -204,6 +204,7 @@
   
   
   
+  
   7002050
   0
   43100244
-- 
2.25.1

[PATCH v2 0/7] qemu: add support for librbd layered encryption

[Or Ozeri]

v2:
 - add luks-any commits (including nit fixes)
 - removed qemu 8.0.0 replies commit
 - remove tautological if condition in qemuBlockStorageSourceAttachData 
initialization
 - add comments on validation of a single secret in qemu encryption engine
 - fix leak of qemuDomainStorageSourcePrivate->encinfo
 - remove ctxt->node modification in privatedata xml parsing
 - add test to modern-in.xml
 - squash commit #6
 - add validation for a single secret in sd card disk

Or Ozeri (7):
  qemu: capabilities: Introduce QEMU_CAPS_RBD_ENCRYPTION_LAYERING
capability
  qemu: add support for multiple secret aliases
  qemu: add multi-secret support in qemuBlockStorageSourceAttachData
  qemu: add multi-secret support in _qemuDomainStorageSourcePrivate
  qemu: add support for librbd layered encryption
  qemu: capabilities: Introduce QEMU_CAPS_RBD_ENCRYPTION_LUKS_ANY
capability
  qemu: add luks-any encryption support for RBD images

 docs/formatstorageencryption.rst  |  20 ++-
 src/conf/schemas/storagecommon.rng|   5 +-
 src/conf/storage_encryption_conf.c|   2 +-
 src/conf/storage_encryption_conf.h|   1 +
 src/libvirt_private.syms  |   1 +
 src/qemu/qemu_alias.c |   8 +-
 src/qemu/qemu_alias.h |   3 +-
 src/qemu/qemu_block.c |  77 ---
 src/qemu/qemu_block.h |   5 +-
 src/qemu/qemu_blockjob.c  |   6 +
 src/qemu/qemu_capabilities.c  |   4 +
 src/qemu/qemu_capabilities.h  |   2 +
 src/qemu/qemu_command.c   |  29 +++-
 src/qemu/qemu_domain.c| 130 +++---
 src/qemu/qemu_domain.h|   3 +-
 src/qemu/qemu_hotplug.c   |   2 +-
 src/qemu/qemu_migration_params.c  |   2 +-
 src/qemu/qemu_validate.c  |   8 ++
 tests/qemublocktest.c |   7 +-
 .../caps_8.0.0.x86_64.xml |   2 +
 tests/qemustatusxml2xmldata/modern-in.xml |  14 ++
 ...k-rbd-encryption-layering.x86_64-7.2.0.err |   1 +
 ...rbd-encryption-layering.x86_64-latest.args |  39 ++
 .../disk-network-rbd-encryption-layering.xml  |  41 ++
 ...k-rbd-encryption-luks-any.x86_64-7.2.0.err |   1 +
 ...rbd-encryption-luks-any.x86_64-latest.args |  37 +
 .../disk-network-rbd-encryption-luks-any.xml  |  39 ++
 tests/qemuxml2argvtest.c  |   4 +
 ...-rbd-encryption-layering.x86_64-latest.xml |  46 +++
 ...-rbd-encryption-luks-any.x86_64-latest.xml |  44 ++
 tests/qemuxml2xmltest.c   |   2 +
 31 files changed, 517 insertions(+), 68 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-layering.x86_64-7.2.0.err
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-layering.x86_64-latest.args
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-layering.xml
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-luks-any.x86_64-7.2.0.err
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-luks-any.x86_64-latest.args
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-luks-any.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-network-rbd-encryption-layering.x86_64-latest.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-network-rbd-encryption-luks-any.x86_64-latest.xml

-- 
2.25.1

RE: [PATCH v1 2/3] qemu: capabilities: Introduce QEMU_CAPS_RBD_ENCRYPTION_LUKS_ANY capability

[Or Ozeri]

> -Original Message-
> From: Peter Krempa 
> Sent: Friday, 10 March 2023 12:06
> To: Or Ozeri 
> Cc: libvir-list@redhat.com; idryo...@gmail.com; Danny Harnik
> 
> Subject: [EXTERNAL] Re: [PATCH v1 2/3] qemu: capabilities: Introduce
> QEMU_CAPS_RBD_ENCRYPTION_LUKS_ANY capability
> 
> Next time if you have two series that depend on each other it's better to just
> send them as one. I had to rebase this to fit on top of you previous posting.
> 

This patch series is actually independent of the previous one.
I did group them together to one patch in qemu for non-technical reasons.
Anyways, I don't mind grouping them here as well.

> Since a v2 of the secret layering series is needed please make sure to post
> this and rebase it on top as part of that series.
> 
> Reviewed-by: Peter Krempa 

RE: [PATCH v1 7/7] qemu: add support for librbd layered encryption

[Or Ozeri]

> -Original Message-
> From: Peter Krempa 
> Sent: Friday, 10 March 2023 11:47
> To: Or Ozeri 
> Cc: libvir-list@redhat.com; idryo...@gmail.com; Danny Harnik
> 
> Subject: [EXTERNAL] Re: [PATCH v1 7/7] qemu: add support for librbd layered
> encryption
> 
> > @@ -5210,6 +5216,14 @@
> qemuDomainValidateStorageSource(virStorageSource *src,
> > _("librbd encryption is supported only 
> > with RBD backed
> disks"));
> >  return -1;
> >  }
> > +
> > +if (src->encryption->nsecrets > 1) {
> > +if (!virQEMUCapsGet(qemuCaps,
> QEMU_CAPS_RBD_ENCRYPTION_LAYERING)) {
> > +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > +   _("librbd encryption layering is 
> > not supported by this
> QEMU binary"));
> > +return -1;
> > +}
> 
> As noted in previous patch you must here validate that also the disk is not an
> SD card.
> 

I tried searching the code to understand these questions:
1. How to tell that a disk is an SD card?
2. Why should using multiple secrets be prevented on an SD card disk? And why 
is a single secret OK?

I could not find an answer to question 2. But I count on your expertise so we 
can ignore this question.
The first question however must be answered in order to implement the check you 
talked about.
My guess is the answer is (disk->bus == VIR_DOMAIN_DISK_BUS_SD). Is this 
correct?
But then, you said the check is to be placed inside 
qemuDomainValidateStorageSource, which has the virStorageSource, but not the 
parent virDomainDiskDef.
Do you suggest to extend the signature of qemuDomainValidateStorageSource with 
an additional "bool isSdDisk"?

RE: [PATCH v2 5/6] qemu: Allow setting per-disk snapshot name for RBD disks

[Or Ozeri]

> -Original Message-
> From: Peter Krempa 
> Sent: Monday, 20 February 2023 16:13
> To: Or Ozeri 
> Cc: libvir-list@redhat.com; Danny Harnik 
> Subject: [EXTERNAL] Re: [PATCH v2 5/6] qemu: Allow setting per-disk
> snapshot name for RBD disks
> 
> > diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> index
> > a10bdf7bf2..c72bdb4723 100644
> > --- a/src/qemu/qemu_snapshot.c
> > +++ b/src/qemu/qemu_snapshot.c
> > @@ -664,7 +664,7 @@ qemuSnapshotPrepare(virDomainObj *vm,
> >  return -1;
> >  }
> >
> > -if (disk->snapshot_name) {
> > +if (disk->snapshot_name && !is_raw_rbd) {
> >  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > _("snapshot name setting for disk %s 
> > unsupported "
> >   "for storage type %s"),
> 
> I don't see anything that updates the equivalent of dom_disk->src-
> >snapshot after the snapshot:
> 
> The 'snapshot' property is filled as the equivalent property when formatting
> the backend definition for the 'rbd' disk.
> 
> In case when the 'snapshot' field is meant to actually mean label the 'old'
> state. You then must actually tweak the snapshot metadata to point to it.
> That will allow proper reversion of the image.

We actually block reverting to this type of snapshot, see
" revert to a non-full internal snapshot not supported yet" in 
qemuSnapshotRevertValidate.
Given this, is this change still required?
Actually I'm not sure I understand. Is setting dom_disk->src->snapshot enough?
Or can you please point me to another place in the code where a change is 
required?

RE: [PATCH v2 3/6] qemu: Add internal support for active disk internal snapshots

[Or Ozeri]

> -Original Message-
> From: Peter Krempa 
> Sent: Monday, 20 February 2023 16:00
> To: Or Ozeri 
> Cc: libvir-list@redhat.com; Danny Harnik 
> Subject: [EXTERNAL] Re: [PATCH v2 3/6] qemu: Add internal support for
> active disk internal snapshots
> 
> This modification is done to a function named
> `qemuSnapshotCreateActiveExternal` but clearly enabled creation of internal
> snapshots. You'll need to address that one too.

By address, you mean rename?
It treats both external and disks-only (which can be internal / external / 
mixed)
So maybe qemuSnapshotCreateActiveExternalOrDisksOnly?
I can't think of a neat name for it :\

> 
> Since your patches add intermingling (at least) partial of snapshots the code
> will need to be split up separately.
> 
> 1) check that the old-style full internal snapshot is requested and deal with
> that as a special case

This check already exists, so I guess no change is required?

> 2) separate qemuSnapshotCreateActiveExternal into:
> - 2.1) - the bit that creates the external memory snapshot
> - 2.2) - the bit that creates the external disk snapshot

You mean separate each of the bits mentioned above to a new function?
The bit that creates the memory snapshot also pauses the VM, and unpause
after all is done (including the disks snapshot). I'm guessing you do not want
to include this as part of the 2.1 mentioned above?

> - 2.3) - modify the bit that creates external disk snapshot to also
>  do the internal disk snapshot for rbd
> - 2.4) - call them in proper sequence. I suspect you also want to do
>  a external memory snapshot including the internal RBD
>  snapshot as s transaction.

Actually no, there is no current use-case for that (that I know of).
> 
> Good bit is that the oldschool internal snapshot is a completely separate case
> and can't be intermingled here.
> 
> But the existing code must be modified to honour the change of semantics
> you are introducing.

[PATCH v1 3/3] qemu: add luks-any encryption support for RBD images

[Or Ozeri]

The newly added luks-any rbd encryption format in qemu
allows for opening both LUKS and LUKS2 encryption formats.
This commit enables libvirt uses to use this wildcard format.

Signed-off-by: Or Ozeri 
---
 docs/formatstorageencryption.rst  |  9 
 src/conf/schemas/storagecommon.rng|  1 +
 src/conf/storage_encryption_conf.c|  2 +-
 src/conf/storage_encryption_conf.h|  1 +
 src/libvirt_private.syms  |  1 +
 src/qemu/qemu_block.c | 10 -
 src/qemu/qemu_domain.c| 32 +-
 ...k-rbd-encryption-luks-any.x86_64-7.2.0.err |  1 +
 ...rbd-encryption-luks-any.x86_64-latest.args | 38 
 .../disk-network-rbd-encryption-luks-any.xml  | 39 
 tests/qemuxml2argvtest.c  |  2 +
 ...-rbd-encryption-luks-any.x86_64-latest.xml | 44 +++
 tests/qemuxml2xmltest.c   |  1 +
 13 files changed, 176 insertions(+), 5 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-luks-any.x86_64-7.2.0.err
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-luks-any.x86_64-latest.args
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-luks-any.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-network-rbd-encryption-luks-any.x86_64-latest.xml

diff --git a/docs/formatstorageencryption.rst b/docs/formatstorageencryption.rst
index 2c19473d6b..c58d088403 100644
--- a/docs/formatstorageencryption.rst
+++ b/docs/formatstorageencryption.rst
@@ -104,6 +104,15 @@ it to control such disks. However, pre-formatted RBD luks2 
disks can be loaded
 to a qemu VM using the qemu VM driver. A single
  element is expected.
 
+``luks-any`` format
+~~~
+
+The ``luks-any`` format is currently supported only by the ``librbd`` engine,
+and can only be applied to RBD network disks (RBD images). This format will try
+to parse the disk as either LUKS or LUKS2, depending on the actual on-disk
+format. A single  element is expected (except
+for the case of RBD layered encryption mentioned above) :since:`Since 9.3.0` .
+
 Examples
 
 
diff --git a/src/conf/schemas/storagecommon.rng 
b/src/conf/schemas/storagecommon.rng
index 4d6e646c9a..aef1bd969c 100644
--- a/src/conf/schemas/storagecommon.rng
+++ b/src/conf/schemas/storagecommon.rng
@@ -14,6 +14,7 @@
   qcow
   luks
   luks2
+  luks-any
 
   
   
diff --git a/src/conf/storage_encryption_conf.c 
b/src/conf/storage_encryption_conf.c
index 3651ff8cfd..639cbf2e58 100644
--- a/src/conf/storage_encryption_conf.c
+++ b/src/conf/storage_encryption_conf.c
@@ -41,7 +41,7 @@ VIR_ENUM_IMPL(virStorageEncryptionSecret,
 
 VIR_ENUM_IMPL(virStorageEncryptionFormat,
   VIR_STORAGE_ENCRYPTION_FORMAT_LAST,
-  "default", "qcow", "luks", "luks2",
+  "default", "qcow", "luks", "luks2", "luks-any",
 );
 
 VIR_ENUM_IMPL(virStorageEncryptionEngine,
diff --git a/src/conf/storage_encryption_conf.h 
b/src/conf/storage_encryption_conf.h
index 312599ad44..03f0e60feb 100644
--- a/src/conf/storage_encryption_conf.h
+++ b/src/conf/storage_encryption_conf.h
@@ -66,6 +66,7 @@ typedef enum {
 VIR_STORAGE_ENCRYPTION_FORMAT_QCOW, /* Both qcow and qcow2 */
 VIR_STORAGE_ENCRYPTION_FORMAT_LUKS,
 VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2,
+VIR_STORAGE_ENCRYPTION_FORMAT_LUKS_ANY,
 
 VIR_STORAGE_ENCRYPTION_FORMAT_LAST,
 } virStorageEncryptionFormatType;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b249dcc85c..eb3ff37b81 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1095,6 +1095,7 @@ virStorageVolTypeToString;
 
 # conf/storage_encryption_conf.h
 virStorageEncryptionFormat;
+virStorageEncryptionFormatTypeToString;
 virStorageEncryptionFree;
 virStorageEncryptionParseNode;
 
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 5e700eff99..254d82df41 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -572,6 +572,10 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 encformat = "luks2";
 break;
 
+case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS_ANY:
+encformat = "luks-any";
+break;
+
 case VIR_STORAGE_ENCRYPTION_FORMAT_QCOW:
 case VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT:
 case VIR_STORAGE_ENCRYPTION_FORMAT_LAST:
@@ -1040,8 +1044,10 @@ qemuBlockStorageSourceGetCryptoProps(virStorageSource 
*src,
 break;
 
 case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2:
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("luks2 is currently not supported by the qemu 
encryption engine"));
+case VIR_STORAGE_ENCRYP

[PATCH v1 2/3] qemu: capabilities: Introduce QEMU_CAPS_RBD_ENCRYPTION_LUKS_ANY capability

[Or Ozeri]

This capability represents that qemu supports the "luks-any" encryption
format for RBD images.
Both LUKS and LUKS2 formats can be parsed using this wildcard format.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml | 1 +
 3 files changed, 4 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 3cb5785baa..94df1fd8ed 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -690,6 +690,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
 
   /* 445 */
   "netdev.stream.reconnect", /* QEMU_CAPS_NETDEV_STREAM_RECONNECT 
*/
+  "rbd-encryption-luks-any", /* QEMU_CAPS_RBD_ENCRYPTION_LUKS_ANY 
*/
 );
 
 
@@ -1554,6 +1555,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsQMPSchemaQueries[] = {
 { "blockdev-add/arg-type/+nvme", QEMU_CAPS_DRIVE_NVME },
 { "blockdev-add/arg-type/+file/aio/^io_uring", QEMU_CAPS_AIO_IO_URING },
 { "blockdev-add/arg-type/+rbd/encrypt", QEMU_CAPS_RBD_ENCRYPTION },
+{ "blockdev-add/arg-type/+rbd/encrypt/format/^luks-any", 
QEMU_CAPS_RBD_ENCRYPTION_LUKS_ANY },
 { "blockdev-add/arg-type/+nbd/tls-hostname", 
QEMU_CAPS_BLOCKDEV_NBD_TLS_HOSTNAME },
 { "blockdev-snapshot/$allow-write-only-overlay", 
QEMU_CAPS_BLOCKDEV_SNAPSHOT_ALLOW_WRITE_ONLY },
 { "chardev-add/arg-type/backend/+socket/data/reconnect", 
QEMU_CAPS_CHARDEV_RECONNECT },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index d049f79dd9..9bfa9addbc 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -669,6 +669,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 
 /* 445 */
 QEMU_CAPS_NETDEV_STREAM_RECONNECT, /* -netdev stream supports reconnect */
+QEMU_CAPS_RBD_ENCRYPTION_LUKS_ANY, /* luks-any (LUKS and LUKS2) encryption 
format for Ceph RBD */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml
index ce051d3f1c..3957cb0805 100644
--- a/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml
@@ -206,6 +206,7 @@
   
   
   
+  
   7002050
   0
   43100244
-- 
2.25.1

[PATCH v1 1/3] tests: qemucapabilitiesdata: Add luks-any encryption format

[Or Ozeri]

luks-any encryption format for RBD images was added in b8f218ef.

Signed-off-by: Or Ozeri
---
 tests/qemucapabilitiesdata/caps_8.0.0.x86_64.replies | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.replies 
b/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.replies
index a41b3e1825..ecf8852e03 100644
--- a/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.replies
+++ b/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.replies
@@ -17035,6 +17035,10 @@
 {
   "case": "luks2",
   "type": "668"
+},
+{
+  "case": "luks-any",
+  "type": "668"
 }
   ],
   "members": [
@@ -20003,6 +20007,9 @@
 },
 {
   "name": "luks2"
+},
+{
+  "name": "luks-any"
 }
   ],
   "meta-type": "enum",
-- 
2.25.1

[PATCH v1 0/3] qemu: add luks-any encryption support for RBD images

[Or Ozeri]

Starting from Ceph 0f93f745 (unreleased 18.0.0) and qemu b8f218ef (unreleased 
8.0.0),
qemu and librbd users can use a wildcard format ("luks-any" in qemu, "luks" in 
librbd).
This format can be used to parse the image as either LUKS or LUKS2, 
auto-detecting
the actual format from the on-disk header.

This patch series enables libvirt users to use this wildcard format as well
(for RBD images only, of course).

I manually patched the qemu 8.0.0 replies file to reflect relevant qemu support,
to allow my tests to run.
Note that any build qemu will not support this feature, unless compiled
while having a librbd that has this feature bundled.

Or Ozeri (3):
  tests: qemucapabilitiesdata: Add luks-any encryption format
  qemu: capabilities: Introduce QEMU_CAPS_RBD_ENCRYPTION_LUKS_ANY
capability
  qemu: add luks-any encryption support for RBD images

 docs/formatstorageencryption.rst  |  9 
 src/conf/schemas/storagecommon.rng|  1 +
 src/conf/storage_encryption_conf.c|  2 +-
 src/conf/storage_encryption_conf.h|  1 +
 src/libvirt_private.syms  |  1 +
 src/qemu/qemu_block.c | 10 -
 src/qemu/qemu_capabilities.c  |  2 +
 src/qemu/qemu_capabilities.h  |  1 +
 src/qemu/qemu_domain.c| 32 +-
 .../caps_8.0.0.x86_64.replies |  7 +++
 .../caps_8.0.0.x86_64.xml |  1 +
 ...k-rbd-encryption-luks-any.x86_64-7.2.0.err |  1 +
 ...rbd-encryption-luks-any.x86_64-latest.args | 38 
 .../disk-network-rbd-encryption-luks-any.xml  | 39 
 tests/qemuxml2argvtest.c  |  2 +
 ...-rbd-encryption-luks-any.x86_64-latest.xml | 44 +++
 tests/qemuxml2xmltest.c   |  1 +
 17 files changed, 187 insertions(+), 5 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-luks-any.x86_64-7.2.0.err
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-luks-any.x86_64-latest.args
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-luks-any.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-network-rbd-encryption-luks-any.x86_64-latest.xml

-- 
2.25.1

[PATCH v1 7/7] qemu: add support for librbd layered encryption

[Or Ozeri]

This commit enables libvirt users to use layered encryption
of RBD images, using the librbd encryption engine.
This allows opening of an encrypted cloned image
whose parent is encrypted with a possibly different encryption key.
To open such images, multiple encryption secrets are expected
to be defined under the encryption XML tag.

Signed-off-by: Or Ozeri 
---
 docs/formatstorageencryption.rst  | 11 +++--
 src/conf/schemas/storagecommon.rng|  4 +-
 src/qemu/qemu_block.c | 23 +++---
 src/qemu/qemu_domain.c| 14 ++
 ...k-rbd-encryption-layering.x86_64-7.2.0.err |  1 +
 ...rbd-encryption-layering.x86_64-latest.args | 39 
 .../disk-network-rbd-encryption-layering.xml  | 40 +
 tests/qemuxml2argvtest.c  |  2 +
 ...-rbd-encryption-layering.x86_64-latest.xml | 45 +++
 tests/qemuxml2xmltest.c   |  1 +
 10 files changed, 169 insertions(+), 11 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-layering.x86_64-7.2.0.err
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-layering.x86_64-latest.args
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-layering.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-network-rbd-encryption-layering.x86_64-latest.xml

diff --git a/docs/formatstorageencryption.rst b/docs/formatstorageencryption.rst
index 2c19473d6b..3b3e9ea379 100644
--- a/docs/formatstorageencryption.rst
+++ b/docs/formatstorageencryption.rst
@@ -28,7 +28,10 @@ network disks. If the engine tag is not specified, the 
``qemu`` engine will be
 used by default (assuming the qemu driver is used). Note that ``librbd`` engine
 is currently only supported by the qemu VM driver, and is not supported by the
 storage driver. Furthermore, the storage driver currently ignores the 
``engine``
-tag.
+tag. :since:`since 9.3.0` RBD layered encryption is supported. Layered
+encryption requires a secret per each encrypted layer. The first secret
+corresponds to the (child) image itself, the second secret to the parent image,
+and so forth.
 
 The ``encryption`` tag can currently contain a sequence of ``secret`` tags, 
each
 with mandatory attributes ``type`` and either ``uuid`` or ``usage`` (
@@ -55,7 +58,8 @@ added to libvirt.
 The ``luks`` format is specific to a luks encrypted volume and the secret is
 used in order to either encrypt during volume creation or decrypt the volume 
for
 usage by the domain. A single  element is
-expected. :since:`Since 2.1.0` .
+expected (except for the case of RBD layered encryption mentioned above).
+:since:`Since 2.1.0` .
 
 For volume creation, it is possible to specify the encryption algorithm used to
 encrypt the luks volume. The following two optional elements may be provided 
for
@@ -102,7 +106,8 @@ can only be applied to RBD network disks (RBD images). 
Since the ``librbd``
 engine is currently not supported by the libvirt storage driver, you cannot use
 it to control such disks. However, pre-formatted RBD luks2 disks can be loaded
 to a qemu VM using the qemu VM driver. A single
- element is expected.
+ element is expected (except for the case of
+RBD layered encryption mentioned above).
 
 Examples
 
diff --git a/src/conf/schemas/storagecommon.rng 
b/src/conf/schemas/storagecommon.rng
index 4d6e646c9a..ff24ae9548 100644
--- a/src/conf/schemas/storagecommon.rng
+++ b/src/conf/schemas/storagecommon.rng
@@ -26,7 +26,9 @@
   
   
 
-  
+  
+
+  
   
 
   
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index f6d21d2040..e0f355874f 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -543,7 +543,8 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 qemuDomainStorageSourcePrivate *srcPriv = 
QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
 g_autoptr(virJSONValue) servers = NULL;
 virJSONValue *ret = NULL;
-g_autoptr(virJSONValue) encrypt = NULL;
+g_autolist(virJSONValue) encrypts = NULL;
+virJSONValue *null_encrypt = NULL;
 const char *encformat = NULL;
 const char *username = NULL;
 g_autoptr(virJSONValue) authmodes = NULL;
@@ -563,6 +564,8 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 
 if (src->encryption &&
 src->encryption->engine == VIR_STORAGE_ENCRYPTION_ENGINE_LIBRBD) {
+size_t i;
+
 switch ((virStorageEncryptionFormatType) src->encryption->format) {
 case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS:
 encformat = "luks";
@@ -579,11 +582,17 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 break;
 }
 
-if (virJSONValueObjectAdd(&encrypt,
-  "s:format", encformat,
- 

[PATCH v1 6/7] qemu: support pass-on of multiple secrets to _qemuDomainStorageSourcePrivate

[Or Ozeri]

This commit extends qemuDomainSecretStorageSourcePrepare
to setup multiple qemu secrets as defined by virStorageSource->encryption.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_domain.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a3b9b57cfa..ffe29dc832 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1482,14 +1482,19 @@ 
qemuDomainSecretStorageSourcePrepare(qemuDomainObjPrivate *priv,
 }
 
 if (hasEnc) {
-srcPriv->enccount = 1;
-srcPriv->encinfo = g_new0(qemuDomainSecretInfo *, 1);
-if (!(srcPriv->encinfo[0] = qemuDomainSecretInfoSetupFromSecret(priv, 
aliasformat,
-
"encryption", 0,
-
VIR_SECRET_USAGE_TYPE_VOLUME,
-NULL,
-
&src->encryption->secrets[0]->seclookupdef)))
-return -1;
+size_t nsecrets = src->encryption->nsecrets;
+size_t i;
+
+srcPriv->enccount = nsecrets;
+srcPriv->encinfo = g_new0(qemuDomainSecretInfo *, nsecrets);
+for (i = 0; i < nsecrets; ++i) {
+if (!(srcPriv->encinfo[i] = 
qemuDomainSecretInfoSetupFromSecret(priv, aliasformat,
+
"encryption", i,
+
VIR_SECRET_USAGE_TYPE_VOLUME,
+
NULL,
+
&src->encryption->secrets[i]->seclookupdef)))
+return -1;
+}
 }
 
 if (src->ncookies &&
-- 
2.25.1

[PATCH v1 5/7] qemu: add multi-secret support in _qemuDomainStorageSourcePrivate

[Or Ozeri]

This commit changes the _qemuDomainStorageSourcePrivate struct
to support multiple secrets (instead of a single one before this commit).
This will useful for storage encryption requiring more than a single secret.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_block.c   | 22 +++-
 src/qemu/qemu_command.c | 20 ++-
 src/qemu/qemu_domain.c  | 75 -
 src/qemu/qemu_domain.h  |  3 +-
 tests/qemublocktest.c   |  7 ++--
 5 files changed, 91 insertions(+), 36 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 2e3e0f6572..f6d21d2040 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -581,7 +581,7 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 
 if (virJSONValueObjectAdd(&encrypt,
   "s:format", encformat,
-  "s:key-secret", srcPriv->encinfo->alias,
+  "s:key-secret", srcPriv->encinfo[0]->alias,
   NULL) < 0)
 return NULL;
 }
@@ -978,7 +978,7 @@ qemuBlockStorageSourceGetFormatLUKSProps(virStorageSource 
*src,
 {
 qemuDomainStorageSourcePrivate *srcPriv = 
QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
 
-if (!srcPriv || !srcPriv->encinfo || !srcPriv->encinfo->alias) {
+if (!srcPriv || !srcPriv->encinfo || !srcPriv->encinfo[0]->alias) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("missing secret info for 'luks' driver"));
 return -1;
@@ -986,7 +986,7 @@ qemuBlockStorageSourceGetFormatLUKSProps(virStorageSource 
*src,
 
 if (virJSONValueObjectAdd(&props,
   "s:driver", "luks",
-  "s:key-secret", srcPriv->encinfo->alias,
+  "s:key-secret", srcPriv->encinfo[0]->alias,
   NULL) < 0)
 return -1;
 
@@ -1054,7 +1054,7 @@ qemuBlockStorageSourceGetCryptoProps(virStorageSource 
*src,
 
 return virJSONValueObjectAdd(encprops,
  "s:format", encformat,
- "s:key-secret", srcpriv->encinfo->alias,
+ "s:key-secret", srcpriv->encinfo[0]->alias,
  NULL);
 }
 
@@ -1616,13 +1616,17 @@ qemuBlockStorageSourceDetachPrepare(virStorageSource 
*src)
 data->authsecretAlias = g_strdup(srcpriv->secinfo->alias);
 
 if (srcpriv->encinfo) {
+size_t i;
+
 if (!data->encryptsecretAlias) {
-data->encryptsecretCount = 1;
-data->encryptsecretProps = g_new0(virJSONValue *, 1);
-data->encryptsecretAlias = g_new0(char *, 1);
+data->encryptsecretCount = srcpriv->enccount;
+data->encryptsecretProps = g_new0(virJSONValue *, 
srcpriv->enccount);
+data->encryptsecretAlias = g_new0(char *, srcpriv->enccount);
 }
 
-data->encryptsecretAlias[0] = g_strdup(srcpriv->encinfo->alias);
+for (i = 0; i < srcpriv->enccount; ++i) {
+data->encryptsecretAlias[i] = 
g_strdup(srcpriv->encinfo[i]->alias);
+}
 }
 
 if (srcpriv->httpcookie)
@@ -1987,7 +1991,7 @@ 
qemuBlockStorageSourceCreateGetEncryptionLUKS(virStorageSource *src,
 
 if (srcpriv &&
 srcpriv->encinfo)
-keysecret = srcpriv->encinfo->alias;
+keysecret = srcpriv->encinfo[0]->alias;
 
 if (virJSONValueObjectAdd(&props,
   "s:key-secret", keysecret,
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f5dcb46e42..69f0d74b92 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1603,7 +1603,7 @@ qemuBuildDriveSourceStr(virDomainDiskDef *disk,
 {
 virStorageType actualType = virStorageSourceGetActualType(disk->src);
 qemuDomainStorageSourcePrivate *srcpriv = 
QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
-qemuDomainSecretInfo *encinfo = NULL;
+qemuDomainSecretInfo **encinfo = NULL;
 g_autoptr(virJSONValue) srcprops = NULL;
 bool rawluks = false;
 
@@ -1647,12 +1647,12 @@ qemuBuildDriveSourceStr(virDomainDiskDef *disk,
 
 if (encinfo) {
 if (disk->src->format == VIR_STORAGE_FILE_RAW) {
-virBufferAsprintf(buf, "key-secret=%s,", encinfo->alias);
+virBufferAsprintf(buf, "key-secret=%s,", encinfo[0]->alias);
 rawluks = true;
 } else if (disk->src->format == VIR_STORAGE_FILE_QCOW2 &&
disk->src->encrypti

[PATCH v1 3/7] qemu: add support for multiple secret aliases

[Or Ozeri]

Change secret aliases from %s-%s-secret0 to %s-%s-secret%lu,
which will later be used for storage encryption requiring more
than a single secret.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_alias.c|  8 +---
 src/qemu/qemu_alias.h|  3 ++-
 src/qemu/qemu_domain.c   | 14 --
 src/qemu/qemu_hotplug.c  |  2 +-
 src/qemu/qemu_migration_params.c |  2 +-
 5 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index a9809797d5..2e0a50b68b 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -801,17 +801,19 @@ qemuDomainGetMasterKeyAlias(void)
 /* qemuAliasForSecret:
  * @parentalias: alias of the parent object
  * @obj: optional sub-object of the parent device the secret is for
+ * @secret_idx: secret index number (0 in the case of a single secret)
  *
  * Generate alias for a secret object used by @parentalias device or one of
  * the dependencies of the device described by @obj.
  */
 char *
 qemuAliasForSecret(const char *parentalias,
-   const char *obj)
+   const char *obj,
+   size_t secret_idx)
 {
 if (obj)
-return g_strdup_printf("%s-%s-secret0", parentalias, obj);
-return g_strdup_printf("%s-secret0", parentalias);
+return g_strdup_printf("%s-%s-secret%lu", parentalias, obj, 
secret_idx);
+return g_strdup_printf("%s-secret%lu", parentalias, secret_idx);
 }
 
 /* qemuAliasTLSObjFromSrcAlias
diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
index f13f4cc5f8..eae08020dc 100644
--- a/src/qemu/qemu_alias.h
+++ b/src/qemu/qemu_alias.h
@@ -86,7 +86,8 @@ char *qemuAliasFromHostdev(const virDomainHostdevDef 
*hostdev);
 char *qemuDomainGetMasterKeyAlias(void);
 
 char *qemuAliasForSecret(const char *parentalias,
- const char *obj);
+ const char *obj,
+ size_t secret_idx);
 
 char *qemuAliasTLSObjFromSrcAlias(const char *srcAlias)
 ATTRIBUTE_NONNULL(1);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f5fd140c85..80c9852dae 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1317,6 +1317,7 @@ qemuDomainSecretInfoSetup(qemuDomainObjPrivate *priv,
  * @priv: pointer to domain private object
  * @srcalias: Alias of the disk/hostdev used to generate the secret alias
  * @secretuse: specific usage for the secret (may be NULL if main object is 
using it)
+ * @secret_idx: secret index number (0 in the case of a single secret)
  * @usageType: The virSecretUsageType
  * @username: username to use for authentication (may be NULL)
  * @seclookupdef: Pointer to seclookupdef data
@@ -1329,12 +1330,13 @@ static qemuDomainSecretInfo *
 qemuDomainSecretInfoSetupFromSecret(qemuDomainObjPrivate *priv,
 const char *srcalias,
 const char *secretuse,
+size_t secret_idx,
 virSecretUsageType usageType,
 const char *username,
 virSecretLookupTypeDef *seclookupdef)
 {
 qemuDomainSecretInfo *secinfo;
-g_autofree char *alias = qemuAliasForSecret(srcalias, secretuse);
+g_autofree char *alias = qemuAliasForSecret(srcalias, secretuse, 
secret_idx);
 g_autofree uint8_t *secret = NULL;
 size_t secretlen = 0;
 VIR_IDENTITY_AUTORESTORE virIdentity *oldident = 
virIdentityElevateCurrent();
@@ -1384,7 +1386,7 @@ qemuDomainSecretInfoTLSNew(qemuDomainObjPrivate *priv,
 }
 seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_UUID;
 
-return qemuDomainSecretInfoSetupFromSecret(priv, srcAlias, NULL,
+return qemuDomainSecretInfoSetupFromSecret(priv, srcAlias, NULL, 0,
VIR_SECRET_USAGE_TYPE_TLS,
NULL, &seclookupdef);
 }
@@ -1411,7 +1413,7 @@ 
qemuDomainSecretStorageSourcePrepareCookies(qemuDomainObjPrivate *priv,
 virStorageSource *src,
 const char *aliasprotocol)
 {
-g_autofree char *secretalias = qemuAliasForSecret(aliasprotocol, 
"httpcookie");
+g_autofree char *secretalias = qemuAliasForSecret(aliasprotocol, 
"httpcookie", 0);
 g_autofree char *cookies = qemuBlockStorageSourceGetCookieString(src);
 
 return qemuDomainSecretInfoSetup(priv, secretalias, NULL,
@@ -1460,7 +1462,7 @@ qemuDomainSecretStorageSourcePrepare(qemuDomainObjPrivate 
*priv,
 usageType = VIR_SECRET_USAGE_TYPE_CEPH;
 
 if (!(srcPriv->secinfo = qemuDomainSecretInfoSetupFromSecret(priv, 
aliasprotocol,
- "auth",
+ 

[PATCH v1 4/7] qemu: add multi-secret support in qemuBlockStorageSourceAttachData

[Or Ozeri]

This commit changes the qemuBlockStorageSourceAttachData struct
to support multiple secrets (instead of a single one before this commit).
This will useful for storage encryption requiring more than a single secret.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_block.c| 35 ++-
 src/qemu/qemu_block.h|  5 +++--
 src/qemu/qemu_blockjob.c |  6 ++
 src/qemu/qemu_command.c  | 21 +
 4 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 5e700eff99..2e3e0f6572 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1310,6 +1310,7 @@ 
qemuBlockStorageSourceGetBlockdevStorageSliceProps(virStorageSource *src)
 void
 qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachData *data)
 {
+size_t i;
 if (!data)
 return;
 
@@ -1319,12 +1320,16 @@ 
qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachData *data)
 virJSONValueFree(data->prmgrProps);
 virJSONValueFree(data->authsecretProps);
 virJSONValueFree(data->httpcookiesecretProps);
-virJSONValueFree(data->encryptsecretProps);
+for (i = 0; i < data->encryptsecretCount; ++i) {
+virJSONValueFree(data->encryptsecretProps[i]);
+g_free(data->encryptsecretAlias[i]);
+}
 virJSONValueFree(data->tlsProps);
 virJSONValueFree(data->tlsKeySecretProps);
 g_free(data->tlsAlias);
 g_free(data->tlsKeySecretAlias);
 g_free(data->authsecretAlias);
+g_free(data->encryptsecretProps);
 g_free(data->encryptsecretAlias);
 g_free(data->httpcookiesecretAlias);
 g_free(data->driveCmd);
@@ -1435,10 +1440,12 @@ static int
 qemuBlockStorageSourceAttachApplyFormatDeps(qemuMonitor *mon,
 qemuBlockStorageSourceAttachData 
*data)
 {
-if (data->encryptsecretProps &&
-qemuMonitorAddObject(mon, &data->encryptsecretProps,
- &data->encryptsecretAlias) < 0)
-return -1;
+size_t i;
+for (i = 0; i < data->encryptsecretCount; ++i) {
+if (qemuMonitorAddObject(mon, &data->encryptsecretProps[i],
+ &data->encryptsecretAlias[i]) < 0)
+return -1;
+}
 
 return 0;
 }
@@ -1524,6 +1531,7 @@ qemuBlockStorageSourceAttachRollback(qemuMonitor *mon,
  qemuBlockStorageSourceAttachData *data)
 {
 virErrorPtr orig_err;
+size_t i;
 
 virErrorPreserveLast(&orig_err);
 
@@ -1549,8 +1557,10 @@ qemuBlockStorageSourceAttachRollback(qemuMonitor *mon,
 if (data->authsecretAlias)
 ignore_value(qemuMonitorDelObject(mon, data->authsecretAlias, false));
 
-if (data->encryptsecretAlias)
-ignore_value(qemuMonitorDelObject(mon, data->encryptsecretAlias, 
false));
+for (i = 0; i < data->encryptsecretCount; ++i) {
+if (data->encryptsecretAlias[i])
+ignore_value(qemuMonitorDelObject(mon, 
data->encryptsecretAlias[i], false));
+}
 
 if (data->httpcookiesecretAlias)
 ignore_value(qemuMonitorDelObject(mon, data->httpcookiesecretAlias, 
false));
@@ -1605,8 +1615,15 @@ qemuBlockStorageSourceDetachPrepare(virStorageSource 
*src)
 if (srcpriv->secinfo)
 data->authsecretAlias = g_strdup(srcpriv->secinfo->alias);
 
-if (srcpriv->encinfo)
-data->encryptsecretAlias = g_strdup(srcpriv->encinfo->alias);
+if (srcpriv->encinfo) {
+if (!data->encryptsecretAlias) {
+data->encryptsecretCount = 1;
+data->encryptsecretProps = g_new0(virJSONValue *, 1);
+data->encryptsecretAlias = g_new0(char *, 1);
+}
+
+data->encryptsecretAlias[0] = g_strdup(srcpriv->encinfo->alias);
+}
 
 if (srcpriv->httpcookie)
 data->httpcookiesecretAlias = g_strdup(srcpriv->httpcookie->alias);
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index 5a61a19da2..530d88d28e 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -89,8 +89,9 @@ struct qemuBlockStorageSourceAttachData {
 virJSONValue *authsecretProps;
 char *authsecretAlias;
 
-virJSONValue *encryptsecretProps;
-char *encryptsecretAlias;
+size_t encryptsecretCount;
+virJSONValue **encryptsecretProps;
+char **encryptsecretAlias;
 
 virJSONValue *httpcookiesecretProps;
 char *httpcookiesecretAlias;
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index a20cf1db62..818e90022c 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1336,9 +1336,15 @@ qemuBlockJobProcessEventConcludedCreate(virQEMUDriver 
*driver,
 /* the format node part was not attached yet, so we don't need t

[PATCH v1 2/7] qemu: capabilities: Introduce QEMU_CAPS_RBD_ENCRYPTION_LAYERING capability

[Or Ozeri]

This capability represents that qemu supports the layered encryption
of RBD images, where a cloned image is encrypted with a possible
different encryption than its parent image.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml | 1 +
 3 files changed, 4 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 3cb5785baa..fe69a752ee 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -690,6 +690,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
 
   /* 445 */
   "netdev.stream.reconnect", /* QEMU_CAPS_NETDEV_STREAM_RECONNECT 
*/
+  "rbd-encryption-layering", /* QEMU_CAPS_RBD_ENCRYPTION_LAYERING 
*/
 );
 
 
@@ -1554,6 +1555,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsQMPSchemaQueries[] = {
 { "blockdev-add/arg-type/+nvme", QEMU_CAPS_DRIVE_NVME },
 { "blockdev-add/arg-type/+file/aio/^io_uring", QEMU_CAPS_AIO_IO_URING },
 { "blockdev-add/arg-type/+rbd/encrypt", QEMU_CAPS_RBD_ENCRYPTION },
+{ "blockdev-add/arg-type/+rbd/encrypt/parent", 
QEMU_CAPS_RBD_ENCRYPTION_LAYERING },
 { "blockdev-add/arg-type/+nbd/tls-hostname", 
QEMU_CAPS_BLOCKDEV_NBD_TLS_HOSTNAME },
 { "blockdev-snapshot/$allow-write-only-overlay", 
QEMU_CAPS_BLOCKDEV_SNAPSHOT_ALLOW_WRITE_ONLY },
 { "chardev-add/arg-type/backend/+socket/data/reconnect", 
QEMU_CAPS_CHARDEV_RECONNECT },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index d049f79dd9..1f29c691e7 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -669,6 +669,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 
 /* 445 */
 QEMU_CAPS_NETDEV_STREAM_RECONNECT, /* -netdev stream supports reconnect */
+QEMU_CAPS_RBD_ENCRYPTION_LAYERING, /* layered encryption support for Ceph 
RBD */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml
index ce051d3f1c..b90ad6d831 100644
--- a/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.xml
@@ -206,6 +206,7 @@
   
   
   
+  
   7002050
   0
   43100244
-- 
2.25.1

[PATCH v1 1/7] tests: qemucapabilitiesdata: Add rbd encryption layering

[Or Ozeri]

RBD encryption layering support was added to qemu in 0f385a24.

Signed-off-by: Or Ozeri
---
 tests/qemucapabilitiesdata/caps_8.0.0.x86_64.replies | 5 +
 1 file changed, 5 insertions(+)

diff --git a/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.replies 
b/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.replies
index a41b3e1825..21df6c5e22 100644
--- a/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.replies
+++ b/tests/qemucapabilitiesdata/caps_8.0.0.x86_64.replies
@@ -17041,6 +17041,11 @@
 {
   "name": "format",
   "type": "666"
+},
+{
+  "name": "parent",
+  "default": null,
+  "type": "545"
 }
   ],
   "meta-type": "object"
-- 
2.25.1

[PATCH v1 0/7] qemu: add support for librbd layered encryption

[Or Ozeri]

Starting from Ceph 0f93f745 (unreleased 18.0.0) and qemu 0f385a24 (unreleased 
8.0.0),
qemu and librbd users can use encrypted RBD cloned images, where the
parent image is encrypted using a different scheme (e.g. different passphrase).
Opening such image require supplying of multiple secrets.
This patch series allows libvirt users to supply multiple secrets necessary
for using such RBD images.
For example:

  
  


Note that unlike the qemu and libvirt API, we don't allow the user
to specify the format of the parent image, but just the passphrase.
We do so to minimize the changes made in libvirt.
To still be able to support RBD images where the parent is encrypted
using a different format (e.g. LUKS2 cloned image of a LUKS parent),
an additional patch series allowing for LUKS* (luks-any) format
will be submitted.

In high-level, this patch series does the following:
- change the qemuBlockStorageSourceAttachData struct to support multiple secrets
- change the qemuDomainStorageSourcePrivate struct to support multiple secrets
- translate multiple secrets from virStorageEncryption to qemu private data

I manually patched the qemu 8.0.0 replies file to reflect relevant qemu support,
to allow my tests to run.
Note that any build qemu will not support this feature, unless compiled
while having a librbd that has this feature bundled.

Or Ozeri (7):
  tests: qemucapabilitiesdata: Add rbd encryption layering
  qemu: capabilities: Introduce QEMU_CAPS_RBD_ENCRYPTION_LAYERING
capability
  qemu: add support for multiple secret aliases
  qemu: add multi-secret support in qemuBlockStorageSourceAttachData
  qemu: add multi-secret support in _qemuDomainStorageSourcePrivate
  qemu: support pass-on of multiple secrets to
_qemuDomainStorageSourcePrivate
  qemu: add support for librbd layered encryption

 docs/formatstorageencryption.rst  |  11 +-
 src/conf/schemas/storagecommon.rng|   4 +-
 src/qemu/qemu_alias.c |   8 +-
 src/qemu/qemu_alias.h |   3 +-
 src/qemu/qemu_block.c |  70 
 src/qemu/qemu_block.h |   5 +-
 src/qemu/qemu_blockjob.c  |   6 +
 src/qemu/qemu_capabilities.c  |   2 +
 src/qemu/qemu_capabilities.h  |   1 +
 src/qemu/qemu_command.c   |  31 +++--
 src/qemu/qemu_domain.c| 106 ++
 src/qemu/qemu_domain.h|   3 +-
 src/qemu/qemu_hotplug.c   |   2 +-
 src/qemu/qemu_migration_params.c  |   2 +-
 tests/qemublocktest.c |   7 +-
 .../caps_8.0.0.x86_64.replies |   5 +
 .../caps_8.0.0.x86_64.xml |   1 +
 ...k-rbd-encryption-layering.x86_64-7.2.0.err |   1 +
 ...rbd-encryption-layering.x86_64-latest.args |  39 +++
 .../disk-network-rbd-encryption-layering.xml  |  40 +++
 tests/qemuxml2argvtest.c  |   2 +
 ...-rbd-encryption-layering.x86_64-latest.xml |  45 
 tests/qemuxml2xmltest.c   |   1 +
 23 files changed, 332 insertions(+), 63 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-layering.x86_64-7.2.0.err
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-layering.x86_64-latest.args
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption-layering.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-network-rbd-encryption-layering.x86_64-latest.xml

-- 
2.25.1

[PATCH v2 1/6] conf: Add snapshotName attribute for internal disk snapshot

[Or Ozeri]

Add a new attribute to snapshot disks XML element,
which allows specifying the name that will be used to refer
to internal snapshots.
If unspecified, the parent snapshot name will be used.
For now, make this attribute unsupported for qemu domains.

Signed-off-by: Or Ozeri 
---
 docs/formatsnapshot.rst |  5 +
 src/conf/schemas/domainsnapshot.rng |  4 
 src/conf/snapshot_conf.c| 27 +++
 src/conf/snapshot_conf.h|  2 ++
 src/qemu/qemu_snapshot.c|  9 +
 5 files changed, 47 insertions(+)

diff --git a/docs/formatsnapshot.rst b/docs/formatsnapshot.rst
index 085c712053..d98066dd8c 100644
--- a/docs/formatsnapshot.rst
+++ b/docs/formatsnapshot.rst
@@ -145,6 +145,11 @@ The top-level ``domainsnapshot`` element may contain the 
following elements:
   driver and supported values are ``file``, ``block`` and ``network``
   :since:`(since 1.2.2)`.
 
+  :since:`Since 9.1.0` the ``disk`` element supports an optional attribute
+``snapshotName`` if the ``snapshot`` attribute is set to 
``internal``. This
+attribute specifies the name that will be used to refer to the
+internal disk snapshot.
+
   ``source``
 
  If the snapshot mode is external (whether specified or inherited),
diff --git a/src/conf/schemas/domainsnapshot.rng 
b/src/conf/schemas/domainsnapshot.rng
index 4048266f1d..19f097d2b3 100644
--- a/src/conf/schemas/domainsnapshot.rng
+++ b/src/conf/schemas/domainsnapshot.rng
@@ -209,6 +209,10 @@
   manual
 
   
+  
+
+
+  
 
   
 
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 9bf3c78353..58a6afa26d 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -150,6 +150,8 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
   VIR_STORAGE_TYPE_FILE) < 0)
 return -1;
 
+def->snapshot_name = virXMLPropString(node, "snapshotName");
+
 if (src->type == VIR_STORAGE_TYPE_VOLUME ||
 src->type == VIR_STORAGE_TYPE_DIR) {
 virReportError(VIR_ERR_XML_ERROR,
@@ -192,6 +194,10 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
 (src->path || src->format))
 def->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
 
+if (def->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT &&
+def->snapshot_name)
+def->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
+
 def->name = g_steal_pointer(&name);
 def->src = g_steal_pointer(&src);
 
@@ -670,6 +676,15 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
 return -1;
 }
 
+if (snapdisk->snapshot_name &&
+snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("snapshotName for disk '%s' requires "
+ "use of internal snapshot mode"),
+   snapdisk->name);
+return -1;
+}
+
 if (snapdisk->src->path &&
 snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -714,6 +729,16 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
 if (virDomainSnapshotDefAssignExternalNames(snapdef) < 0)
 return -1;
 
+/* set default snapshot name for internal snapshots */
+for (i = 0; i < snapdef->ndisks; i++) {
+virDomainSnapshotDiskDef * disk = &snapdef->disks[i];
+
+if (disk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL &&
+!disk->snapshot_name) {
+disk->snapshot_name = g_strdup(snapdef->parent.name);
+}
+}
+
 return 0;
 }
 
@@ -748,6 +773,8 @@ virDomainSnapshotDiskDefFormat(virBuffer *buf,
 if (disk->snapshot > 0)
 virBufferAsprintf(&attrBuf, " snapshot='%s'",
   
virDomainSnapshotLocationTypeToString(disk->snapshot));
+if (disk->snapshot_name)
+virBufferAsprintf(&attrBuf, " snapshotName='%s'", disk->snapshot_name);
 
 if (disk->snapshotDeleteInProgress)
 virBufferAddLit(&childBuf, "\n");
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 96c77ef42b..c133c105c7 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -57,6 +57,8 @@ struct _virDomainSnapshotDiskDef {
 /* details of wrapper external file. src is always non-NULL.
  * XXX optimize this to allow NULL for internal snapshots? */
 virStorageSource *src;
+
+char *snapshot_name; /* snapshot name for internal snapshots */
 };
 
 void
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
in

[PATCH v2 6/6] qemu: Allow mixing active internal and external active disk snapshots

[Or Ozeri]

Previous commit added support for active RBD disk snapshots,
using the same qmp_transaction used also for external snapshots.
The same transaction can be used to mix internal and external snapshots.
To allow this, this commit simply removes the check that disallows mixing
for active snapshots.
Note that previous commits added validation disallowing deletion
and reverting of RBD disks. The same validation applies to the mixed
type of snapshots introduced by this commit.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_snapshot.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index c72bdb4723..81b3e94988 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -746,10 +746,9 @@ qemuSnapshotPrepare(virDomainObj *vm,
 return -1;
 }
 
-/* For now, we don't allow mixing internal and external disks.
- * XXX technically, we could mix internal and external disks for
+/* For now, we don't allow mixing internal and external disks for
  * offline snapshots */
-if ((found_internal && external) ||
+if ((found_internal && external && !active) ||
  (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL && external) ||
  (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL && 
found_internal)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-- 
2.25.1

[PATCH v2 0/6] Introduce active RBD disk snapshot support

[Or Ozeri]

Internal disk snapshots are currently only supported on non-active VMs.
For RBD disks only, this patch series extends this support for active VMs 
running with qemu.
We also add the option to set a name for each RBD snapshot, and allow taking 
them alongside other external disk snapshots (mixing).
Deletion and reverting to snapshots containing RBD snapshots is not allowed, 
and is validated.
Note that taking RBD snapshots on a non-active VM is still unsupported.

Changes in v2:
- reduce patch to RBD use-case only (e.g. not including qcow2 internal 
snapshots)
- add validation to disallow RBD snapshots deletion / reverting

Or Ozeri (6):
  conf: Add snapshotName attribute for internal disk snapshot
  qemu: Block deletion and reverting on non-full internal snapshots
  qemu: Add internal support for active disk internal snapshots
  qemu: Allow active disk snapshots for RBD disks
  qemu: Allow setting per-disk snapshot name for RBD disks
  qemu: Allow mixing active internal and external active disk snapshots

 docs/formatsnapshot.rst   |   5 +
 src/conf/schemas/domainsnapshot.rng   |   4 +
 src/conf/snapshot_conf.c  |  56 ++
 src/conf/snapshot_conf.h  |   5 +
 src/libvirt_private.syms  |   1 +
 src/qemu/qemu_monitor.c   |   9 ++
 src/qemu/qemu_monitor.h   |   5 +
 src/qemu/qemu_monitor_json.c  |  14 +++
 src/qemu/qemu_monitor_json.h  |   5 +
 src/qemu/qemu_snapshot.c  | 105 --
 .../disk_snapshot.xml |   2 +-
 .../disk_snapshot.xml |   2 +-
 .../disk_snapshot_redefine.xml|   2 +-
 tests/qemumonitorjsontest.c   |   1 +
 14 files changed, 181 insertions(+), 35 deletions(-)

-- 
2.25.1

[PATCH v2 5/6] qemu: Allow setting per-disk snapshot name for RBD disks

[Or Ozeri]

This commit adds the option for setting per-disk snapshot name for
RBD disks. All other disk types are still disallowed to use the
snapshotName attribute.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_snapshot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index a10bdf7bf2..c72bdb4723 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -664,7 +664,7 @@ qemuSnapshotPrepare(virDomainObj *vm,
 return -1;
 }
 
-if (disk->snapshot_name) {
+if (disk->snapshot_name && !is_raw_rbd) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("snapshot name setting for disk %s 
unsupported "
  "for storage type %s"),
-- 
2.25.1

[PATCH v2 3/6] qemu: Add internal support for active disk internal snapshots

[Or Ozeri]

libvirt supports taking external disk snapshots on a running VM,
using qemu's "blockdev-snapshot" command.
qemu also supports "blockdev-snapshot-internal-sync" to do the
same for internal snapshots.
This commit wraps this (old) qemu capability to allow future libvirt
users to take internal disk snapshots on a running VM.
This will only work for disk types which support internal snapshots,
and thus we require the disk type to be part of a white list of known
types. For this commit, the list of supported formats is empty.
An upcoming commit will allow RBD disks to use this new capability.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_monitor.c   |  9 +++
 src/qemu/qemu_monitor.h   |  5 ++
 src/qemu/qemu_monitor_json.c  | 14 
 src/qemu/qemu_monitor_json.h  |  5 ++
 src/qemu/qemu_snapshot.c  | 72 ---
 .../disk_snapshot.xml |  2 +-
 .../disk_snapshot.xml |  2 +-
 .../disk_snapshot_redefine.xml|  2 +-
 tests/qemumonitorjsontest.c   |  1 +
 9 files changed, 82 insertions(+), 30 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 38f89167e0..f6dab34243 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4225,6 +4225,15 @@ qemuMonitorTransactionSnapshotBlockdev(virJSONValue 
*actions,
 }
 
 
+int
+qemuMonitorTransactionInternalSnapshotBlockdev(virJSONValue *actions,
+   const char *device,
+   const char *name)
+{
+return qemuMonitorJSONTransactionInternalSnapshotBlockdev(actions, device, 
name);
+}
+
+
 int
 qemuMonitorTransactionBackup(virJSONValue *actions,
  const char *device,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 2d16214ba2..1bfd1ccbc2 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1411,6 +1411,11 @@ qemuMonitorTransactionSnapshotBlockdev(virJSONValue 
*actions,
const char *node,
const char *overlay);
 
+int
+qemuMonitorTransactionInternalSnapshotBlockdev(virJSONValue *actions,
+   const char *device,
+   const char *name);
+
 typedef enum {
 QEMU_MONITOR_TRANSACTION_BACKUP_SYNC_MODE_NONE = 0,
 QEMU_MONITOR_TRANSACTION_BACKUP_SYNC_MODE_INCREMENTAL,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index db99017555..002a6caa52 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -8307,6 +8307,20 @@ qemuMonitorJSONTransactionSnapshotBlockdev(virJSONValue 
*actions,
  NULL);
 }
 
+
+int
+qemuMonitorJSONTransactionInternalSnapshotBlockdev(virJSONValue *actions,
+   const char *device,
+   const char *name)
+{
+return qemuMonitorJSONTransactionAdd(actions,
+ "blockdev-snapshot-internal-sync",
+ "s:device", device,
+ "s:name", name,
+ NULL);
+}
+
+
 VIR_ENUM_DECL(qemuMonitorTransactionBackupSyncMode);
 VIR_ENUM_IMPL(qemuMonitorTransactionBackupSyncMode,
   QEMU_MONITOR_TRANSACTION_BACKUP_SYNC_MODE_LAST,
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 6f376cf9b7..313004f327 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -779,6 +779,11 @@ qemuMonitorJSONTransactionSnapshotBlockdev(virJSONValue 
*actions,
const char *node,
const char *overlay);
 
+int
+qemuMonitorJSONTransactionInternalSnapshotBlockdev(virJSONValue *actions,
+   const char *device,
+   const char *name);
+
 int
 qemuMonitorJSONTransactionBackup(virJSONValue *actions,
  const char *device,
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index c1855b3028..e82352ba7d 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -736,7 +736,7 @@ qemuSnapshotPrepare(virDomainObj *vm,
 }
 
 /* disk snapshot requires at least one disk */
-if (def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT && !external) {
+if (def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT && !external && 
!found_internal) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("disk-only snapshots req

[PATCH v2 4/6] qemu: Allow active disk snapshots for RBD disks

[Or Ozeri]

This commit removes the check disallowing users to take
active disk-only snapshots of RBD disks.
The actual support for this functionality was added in a previous commit.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_snapshot.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index e82352ba7d..a10bdf7bf2 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -630,6 +630,8 @@ qemuSnapshotPrepare(virDomainObj *vm,
 for (i = 0; i < def->ndisks; i++) {
 virDomainSnapshotDiskDef *disk = &def->disks[i];
 virDomainDiskDef *dom_disk = vm->def->disks[i];
+bool is_raw_rbd = (dom_disk->src->format == VIR_STORAGE_FILE_RAW &&
+   dom_disk->src->protocol == 
VIR_STORAGE_NET_PROTOCOL_RBD);
 
 if (disk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_NO &&
 qemuDomainDiskBlockJobIsActive(dom_disk))
@@ -639,7 +641,7 @@ qemuSnapshotPrepare(virDomainObj *vm,
 case VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL:
 found_internal = true;
 
-if (def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT && active) {
+if (def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT && active && 
!is_raw_rbd) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("active qemu domains require external disk "
  "snapshots; disk %s requested internal"),
@@ -652,7 +654,8 @@ qemuSnapshotPrepare(virDomainObj *vm,
 return -1;
 
 if (dom_disk->src->format > 0 &&
-dom_disk->src->format != VIR_STORAGE_FILE_QCOW2) {
+dom_disk->src->format != VIR_STORAGE_FILE_QCOW2 &&
+!is_raw_rbd) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("internal snapshot for disk %s unsupported "
  "for storage type %s"),
-- 
2.25.1

[PATCH v2 2/6] qemu: Block deletion and reverting on non-full internal snapshots

[Or Ozeri]

An upcoming commit will add support for creating a disks-only snapshot
using internal disk snapshots. Deleting or reverting to these will not be
supported, at least not for now.
This commit adds a validation for this.

Signed-off-by: Or Ozeri 
---
 src/conf/snapshot_conf.c | 29 +
 src/conf/snapshot_conf.h |  3 +++
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_snapshot.c | 12 
 4 files changed, 45 insertions(+)

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 58a6afa26d..879fe4c7a1 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -945,6 +945,35 @@ virDomainSnapshotIsExternal(virDomainMomentObj *snap)
 return virDomainSnapshotDefIsExternal(def);
 }
 
+bool
+virDomainSnapshotDefIsNonFullInternal(virDomainSnapshotDef *def)
+{
+bool has_internal = false;
+bool is_full = true;
+size_t i;
+
+for (i = 0; i < def->ndisks; i++) {
+if (def->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL) {
+has_internal = true;
+} else {
+is_full = false;
+}
+}
+
+if (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL)
+return !is_full;
+
+return has_internal;
+}
+
+bool
+virDomainSnapshotIsNonFullInternal(virDomainMomentObj *snap)
+{
+virDomainSnapshotDef *def = virDomainSnapshotObjGetDef(snap);
+
+return virDomainSnapshotDefIsNonFullInternal(def);
+}
+
 int
 virDomainSnapshotRedefinePrep(virDomainObj *vm,
   virDomainSnapshotDef *snapdef,
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index c133c105c7..8f40748602 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -125,6 +125,9 @@ int virDomainSnapshotAlignDisks(virDomainSnapshotDef 
*snapshot,
 bool virDomainSnapshotDefIsExternal(virDomainSnapshotDef *def);
 bool virDomainSnapshotIsExternal(virDomainMomentObj *snap);
 
+bool virDomainSnapshotDefIsNonFullInternal(virDomainSnapshotDef *def);
+bool virDomainSnapshotIsNonFullInternal(virDomainMomentObj *snap);
+
 int virDomainSnapshotRedefinePrep(virDomainObj *vm,
   virDomainSnapshotDef *snapdef,
   virDomainMomentObj **snap,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 576ec8f95f..0d38e86936 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1027,6 +1027,7 @@ virDomainSnapshotDiskDefFree;
 virDomainSnapshotDiskDefParseXML;
 virDomainSnapshotFormatConvertXMLFlags;
 virDomainSnapshotIsExternal;
+virDomainSnapshotIsNonFullInternal;
 virDomainSnapshotRedefinePrep;
 virDomainSnapshotStateTypeFromString;
 virDomainSnapshotStateTypeToString;
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 1aa2f05300..c1855b3028 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1828,6 +1828,12 @@ qemuSnapshotRevertValidate(virDomainObj *vm,
 return -1;
 }
 
+if (virDomainSnapshotIsNonFullInternal(snap)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("revert to a non-full internal snapshot not supported 
yet"));
+return -1;
+}
+
 if (!snap->def->dom) {
 virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY,
_("snapshot '%s' lacks domain '%s' rollback info"),
@@ -3061,6 +3067,12 @@ qemuSnapshotDeleteValidate(virDomainObj *vm,
virDomainMomentObj *snap,
unsigned int flags)
 {
+if (virDomainSnapshotIsNonFullInternal(snap)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("deletion of a non-full internal snapshot not 
supported yet"));
+return -1;
+}
+
 if (!virDomainSnapshotIsExternal(snap) &&
 virDomainObjIsActive(vm)) {
 ssize_t i;
-- 
2.25.1

RE: [PATCH v1 0/4] Introduce active disk internal snapshot support

[Or Ozeri]

> -Original Message-
> From: Or Ozeri
> Sent: Sunday, 15 January 2023 13:41
> To: Peter Krempa 
> Cc: libvir-list@redhat.com; Danny Harnik 
> Subject: RE: [EXTERNAL] Re: [PATCH v1 0/4] Introduce active disk internal
> snapshot support
> 
> > Note that a few days ago Pavel pushed his series adding support for
> > deletion of external snapshots. That code heavily depends on the fact
> > that mixing internal/external snapshots was forbidden.
> >
> > Thus the patch in this series will not be acceptable until you fix the
> > code that Pavel added.
> >
> 
> My code touches only snapshot creation.
> I see that for deletion, Pavel added a check which yields:
> "deletion of external and internal children disk snapshots not supported "
> So I don't think any change is required, unless you want to support a new
> feature of deletion of mixed snapshots.

I missed that this check is for snapshot children.
But I guess a similar validation can be added to qemuSnapshotDeleteValidate,
to ensure that user is not trying to delete a mixed snapshot.

RE: [PATCH v1 0/4] Introduce active disk internal snapshot support

[Or Ozeri]

> -Original Message-
> From: Peter Krempa 
> Sent: Friday, 13 January 2023 17:21
> To: Or Ozeri 
> Cc: libvir-list@redhat.com
> Subject: [EXTERNAL] Re: [PATCH v1 0/4] Introduce active disk internal
> snapshot support
> 
> Could you elaborate how this will be useful? Generally disk only snapshots
> are of very limited use as when reverting you get to a state equating of a
> power loss of a real box.
> 
> This means potentially corrupted files, state and other drawbacks.
> 

This will be useful for backup of the disks for disaster recovery.
i.e. once the internal snapshots are taken, they will be backed up (using diffs 
of course) 
to a different geographic location.

> With external disk only snapshots this was at least somewhat useful as it
> made the original image read-only and thus accessible with a different
> process.
> 
> With internal snapshots you still can't even read that image as it's still 
> locked
> in write mode, so the utility value is even less.
> 

The use-case we're considering is using RBD disks with raw format.
These internal RBD snapshots are read-accessible even while the original disk 
is still being write-accessed.

> > This implementation for the qemu driver works even when there are
> > other disks which ask for external snapshot.
> > Thus we remove the restriction disallowing mixing of internal and
> > external disk snapshots for active VMs.
> > Note that mixing is still disallowed for non-active VMs, as it require
> > a bit more work in a different area of the code.
> 
> Note that a few days ago Pavel pushed his series adding support for deletion
> of external snapshots. That code heavily depends on the fact that mixing
> internal/external snapshots was forbidden.
> 
> Thus the patch in this series will not be acceptable until you fix the code 
> that
> Pavel added.
> 

My code touches only snapshot creation.
I see that for deletion, Pavel added a check which yields:
"deletion of external and internal children disk snapshots not supported "
So I don't think any change is required, unless you want to support a new 
feature of deletion of mixed snapshots.
I don't think we should put a lot of effort into coding a full-matrix of 
support between any two features
(i.e. snapshot deletion, and active internal snapshots) if there's no client 
use-case justifying this work.

> Also note that Pavel is working on reversion of external snapshots so that
> work might also colide.
> 

Like snapshot deletion, this sounds like something that can work alongside by 
simply adding a check which
will disallow reverting in a mixed case.

> 
> This bit is potentially dangerous as based on your code you don't seem to be
> checking that the name is unique across snapshots, which can cause potential
> problems when names between snapshots collide.
> 

I actually think you can specify a name for an internal snapshot today, if you 
set  under , so my modification only extends this to 
allow per-disk name, instead of a single name for all disks.
So in theory I think snapshot name can collide even in today's code.
Second, IMHO this patch already handles collision to the best extent.
If there's a name collision on one of the disks, the 
blockdev-snapshot-internal-sync for that disk will fail,
and the encapsulating qmp_transaction will revert all its actions.

> This is also something that I feel requires elaboration of how it will be 
> useful.

Just like external snapshots allowing you to specify a custom path for the new 
file, some clients wish to use their own
naming-convention for internal snapshots (instead of using the ctime names 
generated by libvirt)

[PATCH v1] qemu: block: fix error when blockcopy target is librbd encrypted

[Or Ozeri]

Encryption secrets are considered a format dependency, even
when being used by the storage node itself, as in the case of
using encryption engine=librbd.
Currently, the storage node is created (blockdev-add) before
creating the format dependencies (including encryption secrets).
As a result, when trying to perform a blockcopy when the target
disk uses librbd encryption, an error of this form is returned:
"error: internal error: unable to execute QEMU command 'blockdev-add': No 
secret with id 'libvirt-5-format-encryption-secret0'"
To overcome this error, we change the order of commands so that
format dependencies are created BEFORE creating the storage node.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index e865aa17f9..2aafb2fac7 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1490,9 +1490,9 @@ qemuBlockStorageSourceAttachApply(qemuMonitor *mon,
   qemuBlockStorageSourceAttachData *data)
 {
 if (qemuBlockStorageSourceAttachApplyStorageDeps(mon, data) < 0 ||
+qemuBlockStorageSourceAttachApplyFormatDeps(mon, data) < 0 ||
 qemuBlockStorageSourceAttachApplyStorage(mon, data) < 0 ||
 qemuBlockStorageSourceAttachApplyStorageSlice(mon, data) < 0 ||
-qemuBlockStorageSourceAttachApplyFormatDeps(mon, data) < 0 ||
 qemuBlockStorageSourceAttachApplyFormat(mon, data) < 0)
 return -1;
 
-- 
2.25.1

[PATCH v1 2/4] qemu: Support active disk internal snapshots

[Or Ozeri]

libvirt supports taking external disk snapshots on a running VM,
using qemu's "blockdev-snapshot" command.
qemu also supports "blockdev-snapshot-internal-sync" to do the
same for internal snapshots.
This commit wraps this (old) qemu capability to allow libvirt users
to take internal disk snapshots on a running VM.
This will only work for disk types which support internal snapshots,
and thus we require the disk type to be part of a white list of known
types (only RBD disks for start).

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_monitor.c   |  9 ++
 src/qemu/qemu_monitor.h   |  5 ++
 src/qemu/qemu_monitor_json.c  | 14 
 src/qemu/qemu_monitor_json.h  |  5 ++
 src/qemu/qemu_snapshot.c  | 83 +++
 .../disk_snapshot.xml |  2 +-
 .../disk_snapshot.xml |  2 +-
 .../disk_snapshot_redefine.xml|  2 +-
 tests/qemumonitorjsontest.c   |  1 +
 9 files changed, 84 insertions(+), 39 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 38f89167e0..f6dab34243 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4225,6 +4225,15 @@ qemuMonitorTransactionSnapshotBlockdev(virJSONValue 
*actions,
 }
 
 
+int
+qemuMonitorTransactionInternalSnapshotBlockdev(virJSONValue *actions,
+   const char *device,
+   const char *name)
+{
+return qemuMonitorJSONTransactionInternalSnapshotBlockdev(actions, device, 
name);
+}
+
+
 int
 qemuMonitorTransactionBackup(virJSONValue *actions,
  const char *device,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 2d16214ba2..1bfd1ccbc2 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1411,6 +1411,11 @@ qemuMonitorTransactionSnapshotBlockdev(virJSONValue 
*actions,
const char *node,
const char *overlay);
 
+int
+qemuMonitorTransactionInternalSnapshotBlockdev(virJSONValue *actions,
+   const char *device,
+   const char *name);
+
 typedef enum {
 QEMU_MONITOR_TRANSACTION_BACKUP_SYNC_MODE_NONE = 0,
 QEMU_MONITOR_TRANSACTION_BACKUP_SYNC_MODE_INCREMENTAL,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index db99017555..002a6caa52 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -8307,6 +8307,20 @@ qemuMonitorJSONTransactionSnapshotBlockdev(virJSONValue 
*actions,
  NULL);
 }
 
+
+int
+qemuMonitorJSONTransactionInternalSnapshotBlockdev(virJSONValue *actions,
+   const char *device,
+   const char *name)
+{
+return qemuMonitorJSONTransactionAdd(actions,
+ "blockdev-snapshot-internal-sync",
+ "s:device", device,
+ "s:name", name,
+ NULL);
+}
+
+
 VIR_ENUM_DECL(qemuMonitorTransactionBackupSyncMode);
 VIR_ENUM_IMPL(qemuMonitorTransactionBackupSyncMode,
   QEMU_MONITOR_TRANSACTION_BACKUP_SYNC_MODE_LAST,
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 6f376cf9b7..313004f327 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -779,6 +779,11 @@ qemuMonitorJSONTransactionSnapshotBlockdev(virJSONValue 
*actions,
const char *node,
const char *overlay);
 
+int
+qemuMonitorJSONTransactionInternalSnapshotBlockdev(virJSONValue *actions,
+   const char *device,
+   const char *name);
+
 int
 qemuMonitorJSONTransactionBackup(virJSONValue *actions,
  const char *device,
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index b8416808b3..9146ecae2f 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -639,20 +639,13 @@ qemuSnapshotPrepare(virDomainObj *vm,
 case VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL:
 found_internal = true;
 
-if (def->state == VIR_DOMAIN_SNAPSHOT_DISK_SNAPSHOT && active) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("active qemu domains require external disk "
- "snapshots; disk %s requested internal"),
-   disk->name);
-

[PATCH v1 0/4] Introduce active disk internal snapshot support

[Or Ozeri]

Internal disk snapshots are currently only supported on non-active VMs.
This patch series extends this support for active VMs running with qemu.

This implementation for the qemu driver works even when there are other
disks which ask for external snapshot.
Thus we remove the restriction disallowing mixing of internal and external
disk snapshots for active VMs.
Note that mixing is still disallowed for non-active VMs, as it require
a bit more work in a different area of the code.

Additionally we add a new attribute to allow the user specifying a
unique snapshot name for each internal disk snapshot.
In case this optional attribute is not specified, snapshot name
will be taken from the domain snapshot name, as it is currently done today.

Or Ozeri (4):
  conf: add snapshotName attribute for internal disk snapshot
  qemu: Support active disk internal snapshots
  qemu: allow mixing active internal and external active disk snapshots
  qemu: switch offline internal disk snapshots to use snapshotName

 docs/formatsnapshot.rst   |  5 ++
 src/conf/schemas/domainsnapshot.rng   |  4 +
 src/conf/snapshot_conf.c  | 27 ++
 src/conf/snapshot_conf.h  |  2 +
 src/qemu/qemu_domain.c|  2 +-
 src/qemu/qemu_monitor.c   |  9 ++
 src/qemu/qemu_monitor.h   |  5 ++
 src/qemu/qemu_monitor_json.c  | 14 +++
 src/qemu/qemu_monitor_json.h  |  5 ++
 src/qemu/qemu_snapshot.c  | 88 +++
 .../disk_snapshot.xml |  2 +-
 .../disk_snapshot.xml |  2 +-
 .../disk_snapshot_redefine.xml|  2 +-
 tests/qemumonitorjsontest.c   |  1 +
 14 files changed, 125 insertions(+), 43 deletions(-)

-- 
2.25.1

[PATCH v1 3/4] qemu: allow mixing active internal and external active disk snapshots

[Or Ozeri]

Previous commit added support for active internal disk snapshots,
using the same qmp_transaction used also for external snapshots.
The same transaction can be used to mix internal and external snapshots.
To allow this, this commit simply removes the check that disallows mixing
for active snapshots.
Offline snapshots still do not support mixing.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_snapshot.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 9146ecae2f..da29fa2853 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -727,10 +727,9 @@ qemuSnapshotPrepare(virDomainObj *vm,
 return -1;
 }
 
-/* For now, we don't allow mixing internal and external disks.
- * XXX technically, we could mix internal and external disks for
+/* For now, we don't allow mixing internal and external disks for
  * offline snapshots */
-if ((found_internal && external) ||
+if ((found_internal && external && !active) ||
  (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL && external) ||
  (def->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL && 
found_internal)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-- 
2.25.1

[PATCH v1 1/4] conf: add snapshotName attribute for internal disk snapshot

[Or Ozeri]

Add a new attribute to snapshot disks XML element,
which allows specifying the name that will be used to refer
to internal snapshots.

Signed-off-by: Or Ozeri 
---
 docs/formatsnapshot.rst |  5 +
 src/conf/schemas/domainsnapshot.rng |  4 
 src/conf/snapshot_conf.c| 27 +++
 src/conf/snapshot_conf.h|  2 ++
 4 files changed, 38 insertions(+)

diff --git a/docs/formatsnapshot.rst b/docs/formatsnapshot.rst
index 085c712053..d98066dd8c 100644
--- a/docs/formatsnapshot.rst
+++ b/docs/formatsnapshot.rst
@@ -145,6 +145,11 @@ The top-level ``domainsnapshot`` element may contain the 
following elements:
   driver and supported values are ``file``, ``block`` and ``network``
   :since:`(since 1.2.2)`.
 
+  :since:`Since 9.1.0` the ``disk`` element supports an optional attribute
+``snapshotName`` if the ``snapshot`` attribute is set to 
``internal``. This
+attribute specifies the name that will be used to refer to the
+internal disk snapshot.
+
   ``source``
 
  If the snapshot mode is external (whether specified or inherited),
diff --git a/src/conf/schemas/domainsnapshot.rng 
b/src/conf/schemas/domainsnapshot.rng
index 4048266f1d..19f097d2b3 100644
--- a/src/conf/schemas/domainsnapshot.rng
+++ b/src/conf/schemas/domainsnapshot.rng
@@ -209,6 +209,10 @@
   manual
 
   
+  
+
+
+  
 
   
 
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 9bf3c78353..58a6afa26d 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -150,6 +150,8 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
   VIR_STORAGE_TYPE_FILE) < 0)
 return -1;
 
+def->snapshot_name = virXMLPropString(node, "snapshotName");
+
 if (src->type == VIR_STORAGE_TYPE_VOLUME ||
 src->type == VIR_STORAGE_TYPE_DIR) {
 virReportError(VIR_ERR_XML_ERROR,
@@ -192,6 +194,10 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
 (src->path || src->format))
 def->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
 
+if (def->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT &&
+def->snapshot_name)
+def->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL;
+
 def->name = g_steal_pointer(&name);
 def->src = g_steal_pointer(&src);
 
@@ -670,6 +676,15 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
 return -1;
 }
 
+if (snapdisk->snapshot_name &&
+snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("snapshotName for disk '%s' requires "
+ "use of internal snapshot mode"),
+   snapdisk->name);
+return -1;
+}
+
 if (snapdisk->src->path &&
 snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -714,6 +729,16 @@ virDomainSnapshotAlignDisks(virDomainSnapshotDef *snapdef,
 if (virDomainSnapshotDefAssignExternalNames(snapdef) < 0)
 return -1;
 
+/* set default snapshot name for internal snapshots */
+for (i = 0; i < snapdef->ndisks; i++) {
+virDomainSnapshotDiskDef * disk = &snapdef->disks[i];
+
+if (disk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL &&
+!disk->snapshot_name) {
+disk->snapshot_name = g_strdup(snapdef->parent.name);
+}
+}
+
 return 0;
 }
 
@@ -748,6 +773,8 @@ virDomainSnapshotDiskDefFormat(virBuffer *buf,
 if (disk->snapshot > 0)
 virBufferAsprintf(&attrBuf, " snapshot='%s'",
   
virDomainSnapshotLocationTypeToString(disk->snapshot));
+if (disk->snapshot_name)
+virBufferAsprintf(&attrBuf, " snapshotName='%s'", disk->snapshot_name);
 
 if (disk->snapshotDeleteInProgress)
 virBufferAddLit(&childBuf, "\n");
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 96c77ef42b..c133c105c7 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -57,6 +57,8 @@ struct _virDomainSnapshotDiskDef {
 /* details of wrapper external file. src is always non-NULL.
  * XXX optimize this to allow NULL for internal snapshots? */
 virStorageSource *src;
+
+char *snapshot_name; /* snapshot name for internal snapshots */
 };
 
 void
-- 
2.25.1

[PATCH v1 4/4] qemu: switch offline internal disk snapshots to use snapshotName

[Or Ozeri]

Previous commit added snapshotName attribute to internal disk snapshots.
This commit changes the qemu domain to use this new attribute for offline
internal disk snapshots.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 2eb5653254..9ab1b5b9a3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7099,7 +7099,7 @@ qemuDomainSnapshotForEachQcow2Raw(virQEMUDriver *driver,
 
 for (i = 0; i < ndisks; i++) {
 g_autoptr(virCommand) cmd = virCommandNewArgList(qemuimgbin, 
"snapshot",
- op, snap->def->name, 
NULL);
+ op, 
snapdef->disks[i].snapshot_name, NULL);
 int format = virDomainDiskGetFormat(def->disks[i]);
 
 /* FIXME: we also need to handle LVM here */
-- 
2.25.1

Internal disk-only snapshots in qemu domain

[Or Ozeri]

Hi,

We are considering the following use-case of libvirt's domain snapshot (and 
revert) feature:

  1.  Domain is qemu
  2.  Snapshot type is disk-only (VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY)
  3.  Snapshot location is internal (VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL)
  4.  All of the VM disks are:
 *   Network type (VIR_STORAGE_TYPE_NETWORK)
 *   RBD protocol (VIR_STORAGE_NET_PROTOCOL_RBD)
 *   Raw format (VIR_STORAGE_FILE_RAW)
  5.  VM can be either active or inactive

We want to extend libvirt to support this use-case.

Consider the following changes:

  1.  For the inactive VM case, libvirt currently calls "qemu-img snapshot" for 
taking a snapshot, as well as reverting (adding "-a" to qemu-img).
The same would also work for our-use case, as basically the same qemu-img 
command would work for RBD disks as well (using rbd:poolname/imagename as the 
filename).
So basically we need to reuse code from qemuDomainSnapshotForEachQcow2Raw.
  2.  For the active VM case, we can re-use code from 
qemuDomainSnapshotCreateActiveExternal, but call qemu's  
"snapshot_blkdev_internal" instead of the current "snapshot_blkdev".
This would work for RBD disks, but more generally for any qemu block driver 
which implements "bdrv_snapshot_create".

BTW it seems to me that if you try to take an internal snapshot of just the 
disks, you will actually get an external snapshot? Or is there some place in 
the code which validates and fails this case?

Anyhow, I would really appreciate any feedback or thoughts.

Thanks,
Or

[PATCH v1] docs: add minor clarifications for librbd encryption

[Or Ozeri]

This should make the documentation less confusing mainly for
Ceph people.

Signed-off-by: Or Ozeri 
---
 docs/formatstorageencryption.html.in | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/docs/formatstorageencryption.html.in 
b/docs/formatstorageencryption.html.in
index 66bf95ab0b..c8c9f08d1e 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -129,9 +129,10 @@
 "luks2" format
 
   The luks2 format is currently supported only by the
-  librbd engine, and can only be applied to RBD network disks.
+  librbd engine, and can only be applied to RBD network disks
+  (RBD images).
   Since the librbd engine is currently not supported by the
-  storage driver, you cannot use it to control such disks. However,
+  libvirt storage driver, you cannot use it to control such disks. However,
   pre-formatted RBD luks2 disks can be loaded to a qemu VM using the qemu
   VM driver.
   A single
-- 
2.25.1

RE: [PATCH v5 0/5] Add support for librbd encryption

[Or Ozeri]

The first Ceph version to support RBD encryption is 16.1.0.Smaller versions will cause qemu (>=6.1.0) to return -ENOTSUP, "RBD library does not support image encryption".Also, this only works on linux machines (e.g. will not work on BSD/windows).-"Han Han" <h...@redhat.com> wrote: ->To: "Or Ozeri" <o...@il.ibm.com>>From: "Han Han" <h...@redhat.com>>Date: 10/28/2021 05:58AM>Cc: libvir-list@redhat.com, idryo...@gmail.com,>to.my.troc...@gmail.com, dan...@il.ibm.com>Subject: [EXTERNAL] Re: [PATCH v5 0/5] Add support for librbd>encryption>>Hi Or, I have a question about this feature. For>rbd encryption in ceph, is it introduced from ceph-v16.2.0? Does it>require the ceph cluster side >= this version? On Sun, Oct 24, 2021>at 5:54 PM Or Ozeri <o...@il.ibm.com> wrote: ‍ ‍>> >Hi Or,>I have a question about this feature. For rbd encryption in ceph, is>it introduced from ceph-v16.2.0?>Does it require the ceph cluster side >= this version?>>On Sun, Oct 24, 2021 at 5:54 PM Or Ozeri <o...@il.ibm.com> wrote:>v5: rebased + nit fixes suggested by Peter> v4:>  - added disk post parse to image creation flow in qemublocktest>(since more tests failed after adding engine validation)>  - removed symlink changes>  - added luks2 and engine documentation>  - switched to using enum engine instead of int>  - added validation for encryption engine and formats> v3: rebased on master> v2: addressed (hopefully) all of Peter's v1 comments (thanks Peter!)> > Feel free to make any other changes before pushing. Thanks!> > Or Ozeri (5):>   qemu: add disk post parse to qemublocktest>   qemu: capablities: Detect presence of 'rbd-encryption' as>     QEMU_CAPS_RBD_ENCRYPTION>   conf: add encryption engine property>   qemu: add librbd encryption engine>   conf: add luks2 encryption format> >  docs/formatstorageencryption.html.in          | 29 ++->  docs/schemas/domainbackup.rng                 |  7 ++>  docs/schemas/storagecommon.rng                |  9 ++>  src/conf/storage_encryption_conf.c            | 28 ++->  src/conf/storage_encryption_conf.h            | 11 +++>  src/qemu/qemu_block.c                         | 41 +>  src/qemu/qemu_capabilities.c                  |  2 +>  src/qemu/qemu_capabilities.h                  |  1 +>  src/qemu/qemu_domain.c                        | 69 ++->  src/qemu/qemu_domain.h                        |  3 +>  tests/qemublocktest.c                         | 29 +++>  .../caps_6.1.0.x86_64.xml                     |  1 +>  .../caps_6.2.0.x86_64.xml                     |  1 +>  tests/qemustatusxml2xmldata/upgrade-out.xml   |  6 +->  ...sk-network-rbd-encryption.x86_64-6.0.0.err |  1 +>  ...-network-rbd-encryption.x86_64-latest.args | 49 +++>  .../disk-network-rbd-encryption.xml           | 75>+>  tests/qemuxml2argvdata/disk-nvme.xml          |  2 +->  .../qemuxml2argvdata/encrypted-disk-usage.xml |  2 +->  tests/qemuxml2argvdata/luks-disks.xml         |  4 +->  tests/qemuxml2argvdata/user-aliases.xml       |  2 +->  tests/qemuxml2argvtest.c                      |  2 +>  ...k-network-rbd-encryption.x86_64-latest.xml | 83>+++>  .../disk-slices.x86_64-latest.xml             |  4 +->  tests/qemuxml2xmloutdata/encrypted-disk.xml   |  2 +->  .../luks-disks-source-qcow2.x86_64-latest.xml | 14 ++-->  .../qemuxml2xmloutdata/luks-disks-source.xml  | 10 +-->  tests/qemuxml2xmltest.c                       |  1 +>  28 files changed, 443 insertions(+), 45 deletions(-)>  create mode 100644>tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-6.0.0.err>  create mode 100644>tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-latest.args>  create mode 100644>tests/qemuxml2argvdata/disk-network-rbd-encryption.xml>  create mode 100644>tests/qemuxml2xmloutdata/disk-network-rbd-encryption.x86_64-latest.xm>l> > -- > 2.25.1> >   

[PATCH v5 3/5] conf: add encryption engine property

[Or Ozeri]

This commit extends libvirt XML configuration to support a custom encryption 
engine.
This means that   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 
---
 docs/formatstorageencryption.html.in  |  6 +
 docs/schemas/domainbackup.rng |  7 +
 docs/schemas/storagecommon.rng|  7 +
 src/conf/storage_encryption_conf.c| 26 ++-
 src/conf/storage_encryption_conf.h|  9 +++
 src/qemu/qemu_block.c |  2 ++
 src/qemu/qemu_domain.c| 20 ++
 tests/qemustatusxml2xmldata/upgrade-out.xml   |  6 ++---
 tests/qemuxml2argvdata/disk-nvme.xml  |  2 +-
 .../qemuxml2argvdata/encrypted-disk-usage.xml |  2 +-
 tests/qemuxml2argvdata/luks-disks.xml |  4 +--
 tests/qemuxml2argvdata/user-aliases.xml   |  2 +-
 .../disk-slices.x86_64-latest.xml |  4 +--
 tests/qemuxml2xmloutdata/encrypted-disk.xml   |  2 +-
 .../luks-disks-source-qcow2.x86_64-latest.xml | 14 +-
 .../qemuxml2xmloutdata/luks-disks-source.xml  | 10 +++
 16 files changed, 99 insertions(+), 24 deletions(-)

diff --git a/docs/formatstorageencryption.html.in 
b/docs/formatstorageencryption.html.in
index 7215c307d7..178fcd0d7c 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -23,6 +23,12 @@
   content of the encryption tag.  Other format values may be
   defined in the future.
 
+
+  The encryption tag supports an optional engine
+  tag, which allows selecting which component actually handles
+  the encryption. Currently defined values of engine are
+  qemu.
+
 
   The encryption tag can currently contain a sequence of
   secret tags, each with mandatory attributes 
type
diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng
index c03455a5a7..05cc28ab00 100644
--- a/docs/schemas/domainbackup.rng
+++ b/docs/schemas/domainbackup.rng
@@ -14,6 +14,13 @@
   luks
 
   
+  
+
+  
+qemu
+  
+
+  
   
 
 
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 9ebb27700d..60dcfac06c 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -15,6 +15,13 @@
   luks
 
   
+  
+
+  
+qemu
+  
+
+  
   
 
 
diff --git a/src/conf/storage_encryption_conf.c 
b/src/conf/storage_encryption_conf.c
index 9112b96cc7..7fd601e4a2 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",
 );
 
+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])))
@@ -239,6 +245,12 @@ virStorageEncryptionParseNode(xmlNodePtr node,
 goto cleanup;
 }
 
+if (virXMLPropEnum(node, "engine",
+   virStorageEncryptionEngineTypeFromString,
+   VIR_XML_PROP_NONZERO,
+   &encdef->engine) < 0)
+  goto cleanup;
+
 if ((n = virXPathNodeSet("./secret", ctxt, &nodes)) < 0)
 goto cleanup;
 
@@ -327,6 +339,7 @@ int
 virStorageEncryptionFormat(virBuffer *buf,
virStorageEncryption *enc)
 {
+const char *engine;
 const char *format;
 size_t i;
 
@@ -335,7 +348,18 @@ virStorageEncryptionFormat(virBuffer *buf,
"%s", _("unexpected encryption format"));
 return -1;
 }
-virBufferAsprintf(buf, "\n", format);
+if (enc->engine == VIR_STORAGE_ENCRYPTION_ENGINE_DEFAULT) {
+virBufferAsprintf(buf, "\n", format);
+} else {
+if (!(engine = virStorageEncryptionEngineTypeToString(enc->engine))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   "%s", _("unexpected encryption engine"));
+return -1;
+}
+virBufferAsprintf(buf, "\n&q

[PATCH v5 2/5] qemu: capablities: Detect presence of 'rbd-encryption' as QEMU_CAPS_RBD_ENCRYPTION

[Or Ozeri]

rbd encryption is new in qemu 6.1.0.
This commit adds capability probing for it.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 1 +
 tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml | 1 +
 4 files changed, 5 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index cddd39924d..6e72a18455 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -651,6 +651,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "chardev.json", /* QEMU_CAPS_CHARDEV_JSON */
   "device.json", /* QEMU_CAPS_DEVICE_JSON */
   "query-dirty-rate", /* QEMU_CAPS_QUERY_DIRTY_RATE */
+  "rbd-encryption", /* QEMU_CAPS_RBD_ENCRYPTION */
 );
 
 
@@ -1561,6 +1562,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsQMPSchemaQueries[] = {
 { "blockdev-add/arg-type/+file/$dynamic-auto-read-only", 
QEMU_CAPS_BLOCK_FILE_AUTO_READONLY_DYNAMIC },
 { "blockdev-add/arg-type/+nvme", QEMU_CAPS_DRIVE_NVME },
 { "blockdev-add/arg-type/+file/aio/^io_uring", QEMU_CAPS_AIO_IO_URING },
+{ "blockdev-add/arg-type/+rbd/encrypt", QEMU_CAPS_RBD_ENCRYPTION },
 { "blockdev-add/arg-type/discard", QEMU_CAPS_DRIVE_DISCARD },
 { "blockdev-add/arg-type/detect-zeroes", QEMU_CAPS_DRIVE_DETECT_ZEROES },
 { "blockdev-backup", QEMU_CAPS_BLOCKDEV_BACKUP },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index bb53d9ae46..338470ac5d 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -631,6 +631,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_CHARDEV_JSON, /* -chardev accepts JSON */
 QEMU_CAPS_DEVICE_JSON, /* -device accepts JSON */
 QEMU_CAPS_QUERY_DIRTY_RATE, /* accepts query-dirty-rate */
+QEMU_CAPS_RBD_ENCRYPTION, /* Ceph RBD encryption support */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
index 98c2fcedce..e60ed4705b 100644
--- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
@@ -240,6 +240,7 @@
   
   
   
+  
   6001000
   0
   43100243
diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml
index 5a46da0a6a..5622745347 100644
--- a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml
@@ -241,6 +241,7 @@
   
   
   
+  
   6001050
   0
   43100244
-- 
2.25.1

[PATCH v5 1/5] qemu: add disk post parse to qemublocktest

[Or Ozeri]

The post parse callback is part of the real (non-test) processing flow.
This commit adds it (for disks) to the qemublocktest flow as well.
Specifically, this will be needed for tests that use luks encryption,
so that the default encryption engine (which is added in an upcoming commit)
will be overridden by qemu.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_domain.c |  2 +-
 src/qemu/qemu_domain.h |  3 +++
 tests/qemublocktest.c  | 29 -
 3 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1bd3730281..5ff602e3af 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5218,7 +5218,7 @@ 
qemuDomainDeviceDiskDefPostParseRestoreSecAlias(virDomainDiskDef *disk,
 }
 
 
-static int
+int
 qemuDomainDeviceDiskDefPostParse(virDomainDiskDef *disk,
  unsigned int parseFlags)
 {
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 9cf5d5479e..6728ab047e 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -857,6 +857,9 @@ int qemuDomainSecretPrepare(virQEMUDriver *driver,
 int qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk,
 virQEMUCaps *qemuCaps);
 
+int qemuDomainDeviceDiskDefPostParse(virDomainDiskDef *disk,
+ unsigned int parseFlags);
+
 int qemuDomainPrepareChannel(virDomainChrDef *chr,
  const char *domainChannelTargetDir)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 3e61e923a9..0176fbd3f4 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -276,6 +276,9 @@ testQemuDiskXMLToProps(const void *opaque)
VIR_DOMAIN_DEF_PARSE_STATUS)))
 return -1;
 
+if (qemuDomainDeviceDiskDefPostParse(disk, 0) < 0)
+return -1;
+
 if (!(vmdef = virDomainDefNew(data->driver->xmlopt)))
 return -1;
 
@@ -470,32 +473,24 @@ testQemuImageCreateLoadDiskXML(const char *name,
virDomainXMLOption *xmlopt)
 
 {
-virDomainSnapshotDiskDef *diskdef = NULL;
-g_autoptr(xmlDoc) doc = NULL;
-g_autoptr(xmlXPathContext) ctxt = NULL;
-xmlNodePtr node;
+virDomainDiskDef *disk = NULL;
 g_autofree char *xmlpath = NULL;
-virStorageSource *ret = NULL;
+g_autofree char *xmlstr = NULL;
 
 xmlpath = g_strdup_printf("%s%s.xml", testQemuImageCreatePath, name);
 
-if (!(doc = virXMLParseFileCtxt(xmlpath, &ctxt)))
+if (virTestLoadFile(xmlpath, &xmlstr) < 0)
 return NULL;
 
-if (!(node = virXPathNode("//disk", ctxt))) {
-VIR_TEST_VERBOSE("failed to find  element\n");
+/* qemu stores node names in the status XML portion */
+if (!(disk = virDomainDiskDefParse(xmlstr, xmlopt,
+   VIR_DOMAIN_DEF_PARSE_STATUS)))
 return NULL;
-}
 
-diskdef = g_new0(virDomainSnapshotDiskDef, 1);
-
-if (virDomainSnapshotDiskDefParseXML(node, ctxt, diskdef,
- VIR_DOMAIN_DEF_PARSE_STATUS,
- xmlopt) == 0)
-ret = g_steal_pointer(&diskdef->src);
+if (qemuDomainDeviceDiskDefPostParse(disk, 0) < 0)
+return NULL;
 
-virDomainSnapshotDiskDefFree(diskdef);
-return ret;
+return disk->src;
 }
 
 
-- 
2.25.1

[PATCH v5 0/5] Add support for librbd encryption

[Or Ozeri]

v5: rebased + nit fixes suggested by Peter
v4:
 - added disk post parse to image creation flow in qemublocktest (since more 
tests failed after adding engine validation)
 - removed symlink changes
 - added luks2 and engine documentation
 - switched to using enum engine instead of int
 - added validation for encryption engine and formats
v3: rebased on master
v2: addressed (hopefully) all of Peter's v1 comments (thanks Peter!)

Feel free to make any other changes before pushing. Thanks!

Or Ozeri (5):
  qemu: add disk post parse to qemublocktest
  qemu: capablities: Detect presence of 'rbd-encryption' as
QEMU_CAPS_RBD_ENCRYPTION
  conf: add encryption engine property
  qemu: add librbd encryption engine
  conf: add luks2 encryption format

 docs/formatstorageencryption.html.in  | 29 ++-
 docs/schemas/domainbackup.rng |  7 ++
 docs/schemas/storagecommon.rng|  9 ++
 src/conf/storage_encryption_conf.c| 28 ++-
 src/conf/storage_encryption_conf.h| 11 +++
 src/qemu/qemu_block.c | 41 +
 src/qemu/qemu_capabilities.c  |  2 +
 src/qemu/qemu_capabilities.h  |  1 +
 src/qemu/qemu_domain.c| 69 ++-
 src/qemu/qemu_domain.h|  3 +
 tests/qemublocktest.c | 29 +++
 .../caps_6.1.0.x86_64.xml |  1 +
 .../caps_6.2.0.x86_64.xml |  1 +
 tests/qemustatusxml2xmldata/upgrade-out.xml   |  6 +-
 ...sk-network-rbd-encryption.x86_64-6.0.0.err |  1 +
 ...-network-rbd-encryption.x86_64-latest.args | 49 +++
 .../disk-network-rbd-encryption.xml   | 75 +
 tests/qemuxml2argvdata/disk-nvme.xml  |  2 +-
 .../qemuxml2argvdata/encrypted-disk-usage.xml |  2 +-
 tests/qemuxml2argvdata/luks-disks.xml |  4 +-
 tests/qemuxml2argvdata/user-aliases.xml   |  2 +-
 tests/qemuxml2argvtest.c  |  2 +
 ...k-network-rbd-encryption.x86_64-latest.xml | 83 +++
 .../disk-slices.x86_64-latest.xml |  4 +-
 tests/qemuxml2xmloutdata/encrypted-disk.xml   |  2 +-
 .../luks-disks-source-qcow2.x86_64-latest.xml | 14 ++--
 .../qemuxml2xmloutdata/luks-disks-source.xml  | 10 +--
 tests/qemuxml2xmltest.c   |  1 +
 28 files changed, 443 insertions(+), 45 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-6.0.0.err
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-encryption.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-network-rbd-encryption.x86_64-latest.xml

-- 
2.25.1

[PATCH v5 5/5] conf: add luks2 encryption format

[Or Ozeri]

This commit extends libvirt XML configuration to support luks2 encryption 
format.
This means that  becomes valid.
Currently librbd is the only engine that supports this new format.

Signed-off-by: Or Ozeri 
---
 docs/formatstorageencryption.html.in | 14 +-
 docs/schemas/storagecommon.rng   |  1 +
 src/conf/storage_encryption_conf.c   |  2 +-
 src/conf/storage_encryption_conf.h   |  1 +
 src/qemu/qemu_block.c|  9 +
 src/qemu/qemu_domain.c   |  9 -
 ...isk-network-rbd-encryption.x86_64-latest.args | 16 ++--
 .../disk-network-rbd-encryption.xml  | 12 
 ...disk-network-rbd-encryption.x86_64-latest.xml | 13 +
 9 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/docs/formatstorageencryption.html.in 
b/docs/formatstorageencryption.html.in
index fb04a6a0ad..86d884f93d 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -18,7 +18,7 @@
   is encryption, with a mandatory
   attribute format.  Currently defined values
   of format are default, qcow,
-  and luks.
+  luks, and luks2.
   Each value of format implies some expectations about the
   content of the encryption tag.  Other format values may be
   defined in the future.
@@ -125,6 +125,18 @@
   
 
 
+"luks2" format
+
+  The luks2 format is currently supported only by the
+  librbd engine, and can only be applied to RBD network disks.
+  Since the librbd engine is currently not supported by the
+  storage driver, you cannot use it to control such disks. However,
+  pre-formatted RBD luks2 disks can be loaded to a qemu VM using the qemu
+  VM driver.
+  A single
+  <secret type='passphrase'...> element is expected.
+
+
 
 Examples
 
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 3ddff02e43..591a158209 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -13,6 +13,7 @@
   default
   qcow
   luks
+  luks2
 
   
   
diff --git a/src/conf/storage_encryption_conf.c 
b/src/conf/storage_encryption_conf.c
index d45ad717a0..a65ef1f8a2 100644
--- a/src/conf/storage_encryption_conf.c
+++ b/src/conf/storage_encryption_conf.c
@@ -44,7 +44,7 @@ VIR_ENUM_IMPL(virStorageEncryptionSecret,
 
 VIR_ENUM_IMPL(virStorageEncryptionFormat,
   VIR_STORAGE_ENCRYPTION_FORMAT_LAST,
-  "default", "qcow", "luks",
+  "default", "qcow", "luks", "luks2",
 );
 
 VIR_ENUM_IMPL(virStorageEncryptionEngine,
diff --git a/src/conf/storage_encryption_conf.h 
b/src/conf/storage_encryption_conf.h
index 0931618608..312599ad44 100644
--- a/src/conf/storage_encryption_conf.h
+++ b/src/conf/storage_encryption_conf.h
@@ -65,6 +65,7 @@ typedef enum {
 VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT = 0,
 VIR_STORAGE_ENCRYPTION_FORMAT_QCOW, /* Both qcow and qcow2 */
 VIR_STORAGE_ENCRYPTION_FORMAT_LUKS,
+VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2,
 
 VIR_STORAGE_ENCRYPTION_FORMAT_LAST,
 } virStorageEncryptionFormatType;
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 4af06aea1b..34fdec2c4b 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -908,6 +908,10 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 encformat = "luks";
 break;
 
+case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2:
+encformat = "luks2";
+break;
+
 case VIR_STORAGE_ENCRYPTION_FORMAT_QCOW:
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("librbd encryption engine only supports 
luks/luks2 formats"));
@@ -1358,6 +1362,11 @@ qemuBlockStorageSourceGetCryptoProps(virStorageSource 
*src,
 encformat = "luks";
 break;
 
+case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("luks2 is currently not supported by the qemu 
encryption engine"));
+return -1;
+
 case VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT:
 case VIR_STORAGE_ENCRYPTION_FORMAT_LAST:
 default:
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 71cebec4e8..4080671dd8 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1188,7 +1188,8 @@ static bool
 qemuDomainDiskHasEncryptionSecret(virStorageSource *src)
 {
 if (!virStorageSourceIsEmpty(src) && src->encryption &&
-src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS &&
+(src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMA

[PATCH v5 4/5] qemu: add librbd encryption engine

[Or Ozeri]

rbd encryption is new in qemu 6.1.0.
This commit adds a new encryption engine property which
allows the user to use this new encryption engine.

Signed-off-by: Or Ozeri 
---
 docs/formatstorageencryption.html.in  | 11 ++-
 docs/schemas/storagecommon.rng|  1 +
 src/conf/storage_encryption_conf.c|  2 +-
 src/conf/storage_encryption_conf.h|  1 +
 src/qemu/qemu_block.c | 30 
 src/qemu/qemu_domain.c| 38 ++
 ...sk-network-rbd-encryption.x86_64-6.0.0.err |  1 +
 ...-network-rbd-encryption.x86_64-latest.args | 45 
 .../disk-network-rbd-encryption.xml   | 63 +
 tests/qemuxml2argvtest.c  |  2 +
 ...k-network-rbd-encryption.x86_64-latest.xml | 70 +++
 tests/qemuxml2xmltest.c   |  1 +
 12 files changed, 263 insertions(+), 2 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-6.0.0.err
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-encryption.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-network-rbd-encryption.x86_64-latest.xml

diff --git a/docs/formatstorageencryption.html.in 
b/docs/formatstorageencryption.html.in
index 178fcd0d7c..fb04a6a0ad 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -27,7 +27,16 @@
   The encryption tag supports an optional engine
   tag, which allows selecting which component actually handles
   the encryption. Currently defined values of engine are
-  qemu.
+  qemu and librbd.
+  Both qemu and librbd require using the qemu
+  driver.
+  The librbd engine requires qemu version >= 6.1.0,
+  and is only applicable for RBD network disks.
+  If the engine tag is not specified, the qemu engine will be
+  used by default (assuming the qemu driver is used).
+  Note that librbd engine is currently only supported by the
+  qemu VM driver, and is not supported by the storage driver. Furthermore,
+  the storage driver currently ignores the engine tag.
 
 
   The encryption tag can currently contain a sequence of
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 60dcfac06c..3ddff02e43 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -19,6 +19,7 @@
 
   
 qemu
+librbd
   
 
   
diff --git a/src/conf/storage_encryption_conf.c 
b/src/conf/storage_encryption_conf.c
index 7fd601e4a2..d45ad717a0 100644
--- a/src/conf/storage_encryption_conf.c
+++ b/src/conf/storage_encryption_conf.c
@@ -49,7 +49,7 @@ VIR_ENUM_IMPL(virStorageEncryptionFormat,
 
 VIR_ENUM_IMPL(virStorageEncryptionEngine,
   VIR_STORAGE_ENCRYPTION_ENGINE_LAST,
-  "default", "qemu",
+  "default", "qemu", "librbd",
 );
 
 static void
diff --git a/src/conf/storage_encryption_conf.h 
b/src/conf/storage_encryption_conf.h
index e0ac0fe4bf..0931618608 100644
--- a/src/conf/storage_encryption_conf.h
+++ b/src/conf/storage_encryption_conf.h
@@ -54,6 +54,7 @@ struct _virStorageEncryptionInfoDef {
 typedef enum {
 VIR_STORAGE_ENCRYPTION_ENGINE_DEFAULT = 0,
 VIR_STORAGE_ENCRYPTION_ENGINE_QEMU,
+VIR_STORAGE_ENCRYPTION_ENGINE_LIBRBD,
 
 VIR_STORAGE_ENCRYPTION_ENGINE_LAST,
 } virStorageEncryptionEngine;
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 0e2395278a..4af06aea1b 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -875,6 +875,8 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 qemuDomainStorageSourcePrivate *srcPriv = 
QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
 g_autoptr(virJSONValue) servers = NULL;
 virJSONValue *ret = NULL;
+g_autoptr(virJSONValue) encrypt = NULL;
+const char *encformat;
 const char *username = NULL;
 g_autoptr(virJSONValue) authmodes = NULL;
 g_autoptr(virJSONValue) mode = NULL;
@@ -899,12 +901,40 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 return NULL;
 }
 
+if (src->encryption &&
+src->encryption->engine == VIR_STORAGE_ENCRYPTION_ENGINE_LIBRBD) {
+switch ((virStorageEncryptionFormatType) src->encryption->format) {
+case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS:
+encformat = "luks";
+break;
+
+case VIR_STORAGE_ENCRYPTION_FORMAT_QCOW:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("librbd encryption engine only supports 
luks/luks2 formats"));
+return NULL;
+
+case VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT:
+ 

RE: [PATCH v4 4/5] qemu: add librbd encryption engine

[Or Ozeri]

Thanks for reviewing all of my patches!I'm fine with you making any of the changes you suggested.So the only change I need to make is "specify what's happening in the storage driver"?Can you elaborate what do you mean by that?I can add something like:For librbd engine, the encryption happens inside the librbd storage driver, so block read/write requests coming in from the hypervisor (qemu) are plaintext,but encrypted by the storage driver before being persisted.Is this the kind of thing you were thinking about?-"Peter Krempa" <pkre...@redhat.com> wrote: ->To: "Or Ozeri" <o...@il.ibm.com>>From: "Peter Krempa" <pkre...@redhat.com>>Date: 10/21/2021 02:16PM>Cc: libvir-list@redhat.com, idryo...@gmail.com,>to.my.troc...@gmail.com, dan...@il.ibm.com>Subject: [EXTERNAL] Re: [PATCH v4 4/5] qemu: add librbd encryption>engine>>On Thu, Oct 07, 2021 at 14:21:20 -0500, Or Ozeri wrote:>> rbd encryption is new in qemu 6.1.0.>> This commit adds a new encryption engine property which>> allows the user to use this new encryption engine.>> >> Signed-off-by: Or Ozeri <o...@il.ibm.com>>> --->>  docs/formatstorageencryption.html.in  |  7 +->>  docs/schemas/storagecommon.rng|  1 +>>  src/conf/storage_encryption_conf.c|  2 +->>  src/conf/storage_encryption_conf.h|  1 +>>  src/qemu/qemu_block.c | 26 +++>>  src/qemu/qemu_domain.c| 34 +>>  ...sk-network-rbd-encryption.x86_64-6.0.0.err |  1 +>>  ...-network-rbd-encryption.x86_64-latest.args | 45 >>  .../disk-network-rbd-encryption.xml   | 63>+>>  tests/qemuxml2argvtest.c  |  2 +>>  ...k-network-rbd-encryption.x86_64-latest.xml | 70>+++>>  tests/qemuxml2xmltest.c   |  1 +>>  12 files changed, 251 insertions(+), 2 deletions(-)>>  create mode 100644>tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-6.0.0.err>>  create mode 100644>tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-latest.args>>  create mode 100644>tests/qemuxml2argvdata/disk-network-rbd-encryption.xml>>  create mode 100644>tests/qemuxml2xmloutdata/disk-network-rbd-encryption.x86_64-latest.xm>l>> >> diff --git a/docs/formatstorageencryption.html.in>b/docs/formatstorageencryption.html.in>> index 178fcd0d7c..02ee8f8ca3 100644>> --- a/docs/formatstorageencryption.html.in>> +++ b/docs/formatstorageencryption.html.in>> @@ -27,7 +27,12 @@>>The encryption tag supports an optional>engine>>tag, which allows selecting which component actually handles>>the encryption. Currently defined values of>engine are>> -  qemu.>> +  qemu and librbd.>> +  Both qemu and librbd require using>the qemu driver.>> +  The librbd engine requires qemu version >=>6.1.0,>> +  and is only applicable for RBD network disks.>> +  If the engine tag is not specified, the qemu>engine will be>> +  used by default (assuming the qemu driver is used).>>Okay, this is slightly better but it doesn't specify what's happening>in>the storage driver.>>>  >>  >>The encryption tag can currently contain a>sequence of>>[...]>>>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c>> index 354f65c6d5..13869dd79b 100644>> --- a/src/qemu/qemu_domain.c>> +++ b/src/qemu/qemu_domain.c>> @@ -4814,6 +4814,40 @@>qemuDomainValidateStorageSource(virStorageSource *src,>>  if (src->encryption) {>>  switch (src->encryption->engine) {>>  case VIR_STORAGE_ENCRYPTION_ENGINE_QEMU:>> +switch ((virStorageEncryptionFormatType)>src->encryption->format) {>> +case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS:>> +case VIR_STORAGE_ENCRYPTION_FORMAT_QCOW:>> +break;>> +>> +case VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT:>> +case VIR_STORAGE_ENCRYPTION_FORMAT_LAST:>> +default:>> +>virReportEnumRangeError(virStorageEncryptionFormatType,>> +>src->encryption->format);>> +return -1;>>This here is okay, because both are basically impossible.>>> +}>> +>> +break;>> +case VIR_STORAGE_ENCRYPTION_ENGINE_LIBRBD:>> +if (!virQEMUCapsGet(qemuCaps,>QEMU_CAPS_RBD_ENCR

Re: [PATCH v4 0/5] Add support for librbd encryption

[Or Ozeri]

Any comments? :)-"Or Ozeri" <o...@il.ibm.com> wrote: -To: libvir-list@redhat.comFrom: "Or Ozeri" <o...@il.ibm.com>Date: 10/07/2021 10:21PMCc: to.my.troc...@gmail.com, dan...@il.ibm.com, idryo...@gmail.com, "Or Ozeri" <o...@il.ibm.com>Subject: [PATCH v4 0/5] Add support for librbd encryptionv4: - added disk post parse to image creation flow in qemublocktest (since more tests failed after adding engine validation) - removed symlink changes - added luks2 and engine documentation - switched to using enum engine instead of int - added validation for encryption engine and formatsv3: rebased on masterv2: addressed (hopefully) all of Peter's v1 comments (thanks Peter!)Or Ozeri (5):  qemu: add disk post parse to qemublocktest  qemu: capablities: Detect presence of 'rbd-encryption' as    QEMU_CAPS_RBD_ENCRYPTION  conf: add encryption engine property  qemu: add librbd encryption engine  conf: add luks2 encryption format docs/formatstorageencryption.html.in          | 23 - docs/schemas/domainbackup.rng                 |  7 ++ docs/schemas/storagecommon.rng                |  9 ++ src/conf/storage_encryption_conf.c            | 29 ++- src/conf/storage_encryption_conf.h            | 11 +++ src/qemu/qemu_block.c                         | 33  src/qemu/qemu_capabilities.c                  |  2 + src/qemu/qemu_capabilities.h                  |  1 + src/qemu/qemu_domain.c                        | 61 +- src/qemu/qemu_domain.h                        |  3 + tests/qemublocktest.c                         | 29 +++ .../caps_6.1.0.x86_64.xml                     |  1 + tests/qemustatusxml2xmldata/upgrade-out.xml   |  6 +- ...sk-network-rbd-encryption.x86_64-6.0.0.err |  1 + ...-network-rbd-encryption.x86_64-latest.args | 49 +++ .../disk-network-rbd-encryption.xml           | 75 + tests/qemuxml2argvdata/disk-nvme.xml          |  2 +- .../qemuxml2argvdata/encrypted-disk-usage.xml |  2 +- tests/qemuxml2argvdata/luks-disks.xml         |  4 +- tests/qemuxml2argvdata/user-aliases.xml       |  2 +- tests/qemuxml2argvtest.c                      |  2 + ...k-network-rbd-encryption.x86_64-latest.xml | 83 +++ .../disk-slices.x86_64-latest.xml             |  4 +- tests/qemuxml2xmloutdata/encrypted-disk.xml   |  2 +- .../luks-disks-source-qcow2.x86_64-latest.xml | 14 ++-- .../qemuxml2xmloutdata/luks-disks-source.xml  | 10 +-- tests/qemuxml2xmltest.c                       |  1 + 27 files changed, 421 insertions(+), 45 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-6.0.0.err create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-encryption.xml create mode 100644 tests/qemuxml2xmloutdata/disk-network-rbd-encryption.x86_64-latest.xml-- 2.25.1

[PATCH v4 2/5] qemu: capablities: Detect presence of 'rbd-encryption' as QEMU_CAPS_RBD_ENCRYPTION

[Or Ozeri]

rbd encryption is new in qemu 6.1.0.
This commit adds capability probing for it.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 1 +
 3 files changed, 4 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 82687dbf39..ea0734db15 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -644,6 +644,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "virtio-mem-pci", /* QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI */
   "memory-backend-file.reserve", /* 
QEMU_CAPS_MEMORY_BACKEND_RESERVE */
   "piix4.acpi-root-pci-hotplug", /* 
QEMU_CAPS_PIIX4_ACPI_ROOT_PCI_HOTPLUG */
+  "rbd-encryption", /* QEMU_CAPS_RBD_ENCRYPTION */
 );
 
 
@@ -1565,6 +1566,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsQMPSchemaQueries[] = {
 { "blockdev-add/arg-type/+file/$dynamic-auto-read-only", 
QEMU_CAPS_BLOCK_FILE_AUTO_READONLY_DYNAMIC },
 { "blockdev-add/arg-type/+nvme", QEMU_CAPS_DRIVE_NVME },
 { "blockdev-add/arg-type/+file/aio/^io_uring", QEMU_CAPS_AIO_IO_URING },
+{ "blockdev-add/arg-type/+rbd/encrypt", QEMU_CAPS_RBD_ENCRYPTION },
 { "blockdev-add/arg-type/discard", QEMU_CAPS_DRIVE_DISCARD },
 { "blockdev-add/arg-type/detect-zeroes", QEMU_CAPS_DRIVE_DETECT_ZEROES },
 { "blockdev-backup", QEMU_CAPS_BLOCKDEV_BACKUP },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 2bbfc15dc4..674da98539 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -624,6 +624,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI, /* -device virtio-mem-pci */
 QEMU_CAPS_MEMORY_BACKEND_RESERVE, /* -object memory-backend-*.reserve= */
 QEMU_CAPS_PIIX4_ACPI_ROOT_PCI_HOTPLUG, /* -M pc 
PIIX4_PM.acpi-root-pci-hotplug */
+QEMU_CAPS_RBD_ENCRYPTION, /* Ceph RBD encryption support */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
index 87b37a2b7c..8180cfd6c2 100644
--- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
@@ -243,6 +243,7 @@
   
   
   
+  
   6001000
   0
   43100243
-- 
2.25.1

[PATCH v4 0/5] Add support for librbd encryption

[Or Ozeri]

v4:
 - added disk post parse to image creation flow in qemublocktest (since more 
tests failed after adding engine validation)
 - removed symlink changes
 - added luks2 and engine documentation
 - switched to using enum engine instead of int
 - added validation for encryption engine and formats
v3: rebased on master
v2: addressed (hopefully) all of Peter's v1 comments (thanks Peter!)

Or Ozeri (5):
  qemu: add disk post parse to qemublocktest
  qemu: capablities: Detect presence of 'rbd-encryption' as
QEMU_CAPS_RBD_ENCRYPTION
  conf: add encryption engine property
  qemu: add librbd encryption engine
  conf: add luks2 encryption format

 docs/formatstorageencryption.html.in  | 23 -
 docs/schemas/domainbackup.rng |  7 ++
 docs/schemas/storagecommon.rng|  9 ++
 src/conf/storage_encryption_conf.c| 29 ++-
 src/conf/storage_encryption_conf.h| 11 +++
 src/qemu/qemu_block.c | 33 
 src/qemu/qemu_capabilities.c  |  2 +
 src/qemu/qemu_capabilities.h  |  1 +
 src/qemu/qemu_domain.c| 61 +-
 src/qemu/qemu_domain.h|  3 +
 tests/qemublocktest.c | 29 +++
 .../caps_6.1.0.x86_64.xml |  1 +
 tests/qemustatusxml2xmldata/upgrade-out.xml   |  6 +-
 ...sk-network-rbd-encryption.x86_64-6.0.0.err |  1 +
 ...-network-rbd-encryption.x86_64-latest.args | 49 +++
 .../disk-network-rbd-encryption.xml   | 75 +
 tests/qemuxml2argvdata/disk-nvme.xml  |  2 +-
 .../qemuxml2argvdata/encrypted-disk-usage.xml |  2 +-
 tests/qemuxml2argvdata/luks-disks.xml |  4 +-
 tests/qemuxml2argvdata/user-aliases.xml   |  2 +-
 tests/qemuxml2argvtest.c  |  2 +
 ...k-network-rbd-encryption.x86_64-latest.xml | 83 +++
 .../disk-slices.x86_64-latest.xml |  4 +-
 tests/qemuxml2xmloutdata/encrypted-disk.xml   |  2 +-
 .../luks-disks-source-qcow2.x86_64-latest.xml | 14 ++--
 .../qemuxml2xmloutdata/luks-disks-source.xml  | 10 +--
 tests/qemuxml2xmltest.c   |  1 +
 27 files changed, 421 insertions(+), 45 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-6.0.0.err
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-encryption.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-network-rbd-encryption.x86_64-latest.xml

-- 
2.25.1

[PATCH v4 5/5] conf: add luks2 encryption format

[Or Ozeri]

This commit extends libvirt XML configuration to support luks2 encryption 
format.
This means that  becomes valid.
Currently librbd is the only engine that supports this new format.

Signed-off-by: Or Ozeri 
---
 docs/formatstorageencryption.html.in | 12 +++-
 docs/schemas/storagecommon.rng   |  1 +
 src/conf/storage_encryption_conf.c   |  2 +-
 src/conf/storage_encryption_conf.h   |  1 +
 src/qemu/qemu_block.c|  5 +
 src/qemu/qemu_domain.c   |  5 -
 ...isk-network-rbd-encryption.x86_64-latest.args | 16 ++--
 .../disk-network-rbd-encryption.xml  | 12 
 ...disk-network-rbd-encryption.x86_64-latest.xml | 13 +
 9 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/docs/formatstorageencryption.html.in 
b/docs/formatstorageencryption.html.in
index 02ee8f8ca3..6cf1f94a9f 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -18,7 +18,7 @@
   is encryption, with a mandatory
   attribute format.  Currently defined values
   of format are default, qcow,
-  and luks.
+  luks, and luks2.
   Each value of format implies some expectations about the
   content of the encryption tag.  Other format values may be
   defined in the future.
@@ -121,6 +121,16 @@
   
 
 
+"luks2" format
+
+  The luks2 format is currently supported only by the
+  librbd engine, and can only be applied to RBD network disks.
+  luks2 encrypted RBD disks can be decrypted by the domain,
+  but creation of such disks is currently not supported through libvirt.
+  A single
+  <secret type='passphrase'...> element is expected.
+
+
 
 Examples
 
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 3ddff02e43..591a158209 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -13,6 +13,7 @@
   default
   qcow
   luks
+  luks2
 
   
   
diff --git a/src/conf/storage_encryption_conf.c 
b/src/conf/storage_encryption_conf.c
index 3c1267ed40..c312236d4c 100644
--- a/src/conf/storage_encryption_conf.c
+++ b/src/conf/storage_encryption_conf.c
@@ -44,7 +44,7 @@ VIR_ENUM_IMPL(virStorageEncryptionSecret,
 
 VIR_ENUM_IMPL(virStorageEncryptionFormat,
   VIR_STORAGE_ENCRYPTION_FORMAT_LAST,
-  "default", "qcow", "luks",
+  "default", "qcow", "luks", "luks2",
 );
 
 VIR_ENUM_IMPL(virStorageEncryptionEngine,
diff --git a/src/conf/storage_encryption_conf.h 
b/src/conf/storage_encryption_conf.h
index 0931618608..312599ad44 100644
--- a/src/conf/storage_encryption_conf.h
+++ b/src/conf/storage_encryption_conf.h
@@ -65,6 +65,7 @@ typedef enum {
 VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT = 0,
 VIR_STORAGE_ENCRYPTION_FORMAT_QCOW, /* Both qcow and qcow2 */
 VIR_STORAGE_ENCRYPTION_FORMAT_LUKS,
+VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2,
 
 VIR_STORAGE_ENCRYPTION_FORMAT_LAST,
 } virStorageEncryptionFormatType;
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 5b1b5bea2e..62c40d39d1 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -908,6 +908,10 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 encformat = "luks";
 break;
 
+case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2:
+encformat = "luks2";
+break;
+
 case VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT:
 case VIR_STORAGE_ENCRYPTION_FORMAT_QCOW:
 case VIR_STORAGE_ENCRYPTION_FORMAT_LAST:
@@ -1355,6 +1359,7 @@ qemuBlockStorageSourceGetCryptoProps(virStorageSource 
*src,
 break;
 
 case VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT:
+case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2:
 case VIR_STORAGE_ENCRYPTION_FORMAT_LAST:
 default:
 virReportEnumRangeError(virStorageEncryptionFormatType,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 13869dd79b..8c2a5408da 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1228,7 +1228,8 @@ static bool
 qemuDomainDiskHasEncryptionSecret(virStorageSource *src)
 {
 if (!virStorageSourceIsEmpty(src) && src->encryption &&
-src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS &&
+(src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS ||
+ src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2) &&
 src->encryption->nsecrets > 0)
 return true;
 
@@ -4820,6 +4821,7 @@ qemuDomainValidateStorageSource(virStorageSource *src,
 break;
 
 case VIR_STORAGE_EN

[PATCH v4 4/5] qemu: add librbd encryption engine

[Or Ozeri]

rbd encryption is new in qemu 6.1.0.
This commit adds a new encryption engine property which
allows the user to use this new encryption engine.

Signed-off-by: Or Ozeri 
---
 docs/formatstorageencryption.html.in  |  7 +-
 docs/schemas/storagecommon.rng|  1 +
 src/conf/storage_encryption_conf.c|  2 +-
 src/conf/storage_encryption_conf.h|  1 +
 src/qemu/qemu_block.c | 26 +++
 src/qemu/qemu_domain.c| 34 +
 ...sk-network-rbd-encryption.x86_64-6.0.0.err |  1 +
 ...-network-rbd-encryption.x86_64-latest.args | 45 
 .../disk-network-rbd-encryption.xml   | 63 +
 tests/qemuxml2argvtest.c  |  2 +
 ...k-network-rbd-encryption.x86_64-latest.xml | 70 +++
 tests/qemuxml2xmltest.c   |  1 +
 12 files changed, 251 insertions(+), 2 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-6.0.0.err
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-encryption.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-network-rbd-encryption.x86_64-latest.xml

diff --git a/docs/formatstorageencryption.html.in 
b/docs/formatstorageencryption.html.in
index 178fcd0d7c..02ee8f8ca3 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -27,7 +27,12 @@
   The encryption tag supports an optional engine
   tag, which allows selecting which component actually handles
   the encryption. Currently defined values of engine are
-  qemu.
+  qemu and librbd.
+  Both qemu and librbd require using the qemu 
driver.
+  The librbd engine requires qemu version >= 6.1.0,
+  and is only applicable for RBD network disks.
+  If the engine tag is not specified, the qemu engine will be
+  used by default (assuming the qemu driver is used).
 
 
   The encryption tag can currently contain a sequence of
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 60dcfac06c..3ddff02e43 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -19,6 +19,7 @@
 
   
 qemu
+librbd
   
 
   
diff --git a/src/conf/storage_encryption_conf.c 
b/src/conf/storage_encryption_conf.c
index 59178b41ef..3c1267ed40 100644
--- a/src/conf/storage_encryption_conf.c
+++ b/src/conf/storage_encryption_conf.c
@@ -49,7 +49,7 @@ VIR_ENUM_IMPL(virStorageEncryptionFormat,
 
 VIR_ENUM_IMPL(virStorageEncryptionEngine,
   VIR_STORAGE_ENCRYPTION_ENGINE_LAST,
-  "default", "qemu",
+  "default", "qemu", "librbd",
 );
 
 static void
diff --git a/src/conf/storage_encryption_conf.h 
b/src/conf/storage_encryption_conf.h
index e0ac0fe4bf..0931618608 100644
--- a/src/conf/storage_encryption_conf.h
+++ b/src/conf/storage_encryption_conf.h
@@ -54,6 +54,7 @@ struct _virStorageEncryptionInfoDef {
 typedef enum {
 VIR_STORAGE_ENCRYPTION_ENGINE_DEFAULT = 0,
 VIR_STORAGE_ENCRYPTION_ENGINE_QEMU,
+VIR_STORAGE_ENCRYPTION_ENGINE_LIBRBD,
 
 VIR_STORAGE_ENCRYPTION_ENGINE_LAST,
 } virStorageEncryptionEngine;
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 18c5852d2e..5b1b5bea2e 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -875,6 +875,8 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 qemuDomainStorageSourcePrivate *srcPriv = 
QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
 g_autoptr(virJSONValue) servers = NULL;
 virJSONValue *ret = NULL;
+g_autoptr(virJSONValue) encrypt = NULL;
+const char *encformat;
 const char *username = NULL;
 g_autoptr(virJSONValue) authmodes = NULL;
 g_autoptr(virJSONValue) mode = NULL;
@@ -899,12 +901,36 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 return NULL;
 }
 
+if (src->encryption &&
+src->encryption->engine == VIR_STORAGE_ENCRYPTION_ENGINE_LIBRBD) {
+switch ((virStorageEncryptionFormatType) src->encryption->format) {
+case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS:
+encformat = "luks";
+break;
+
+case VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT:
+case VIR_STORAGE_ENCRYPTION_FORMAT_QCOW:
+case VIR_STORAGE_ENCRYPTION_FORMAT_LAST:
+default:
+virReportEnumRangeError(virStorageEncryptionFormatType,
+src->encryption->format);
+return NULL;
+}
+
+if (virJSONValueObjectCreate(&encrypt,
+ "s:format", encformat,
+ "s:key-secret", 

[PATCH v4 3/5] conf: add encryption engine property

[Or Ozeri]

This commit extends libvirt XML configuration to support a custom encryption 
engine.
This means that   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 
---
 docs/formatstorageencryption.html.in  |  6 +
 docs/schemas/domainbackup.rng |  7 +
 docs/schemas/storagecommon.rng|  7 +
 src/conf/storage_encryption_conf.c| 27 ++-
 src/conf/storage_encryption_conf.h|  9 +++
 src/qemu/qemu_block.c |  2 ++
 src/qemu/qemu_domain.c| 20 ++
 tests/qemustatusxml2xmldata/upgrade-out.xml   |  6 ++---
 tests/qemuxml2argvdata/disk-nvme.xml  |  2 +-
 .../qemuxml2argvdata/encrypted-disk-usage.xml |  2 +-
 tests/qemuxml2argvdata/luks-disks.xml |  4 +--
 tests/qemuxml2argvdata/user-aliases.xml   |  2 +-
 .../disk-slices.x86_64-latest.xml |  4 +--
 tests/qemuxml2xmloutdata/encrypted-disk.xml   |  2 +-
 .../luks-disks-source-qcow2.x86_64-latest.xml | 14 +-
 .../qemuxml2xmloutdata/luks-disks-source.xml  | 10 +++
 16 files changed, 100 insertions(+), 24 deletions(-)

diff --git a/docs/formatstorageencryption.html.in 
b/docs/formatstorageencryption.html.in
index 7215c307d7..178fcd0d7c 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -23,6 +23,12 @@
   content of the encryption tag.  Other format values may be
   defined in the future.
 
+
+  The encryption tag supports an optional engine
+  tag, which allows selecting which component actually handles
+  the encryption. Currently defined values of engine are
+  qemu.
+
 
   The encryption tag can currently contain a sequence of
   secret tags, each with mandatory attributes 
type
diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng
index c03455a5a7..05cc28ab00 100644
--- a/docs/schemas/domainbackup.rng
+++ b/docs/schemas/domainbackup.rng
@@ -14,6 +14,13 @@
   luks
 
   
+  
+
+  
+qemu
+  
+
+  
   
 
 
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 9ebb27700d..60dcfac06c 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -15,6 +15,13 @@
   luks
 
   
+  
+
+  
+qemu
+  
+
+  
   
 
 
diff --git a/src/conf/storage_encryption_conf.c 
b/src/conf/storage_encryption_conf.c
index 9112b96cc7..59178b41ef 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",
 );
 
+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,12 @@ virStorageEncryptionParseNode(xmlNodePtr node,
 goto cleanup;
 }
 
+if (virXMLPropEnum(node, "engine",
+   virStorageEncryptionEngineTypeFromString,
+   VIR_XML_PROP_NONZERO,
+   &encdef->engine) < 0)
+  goto cleanup;
+
 if ((n = virXPathNodeSet("./secret", ctxt, &nodes)) < 0)
 goto cleanup;
 
@@ -327,6 +340,7 @@ int
 virStorageEncryptionFormat(virBuffer *buf,
virStorageEncryption *enc)
 {
+const char *engine;
 const char *format;
 size_t i;
 
@@ -335,7 +349,18 @@ virStorageEncryptionFormat(virBuffer *buf,
"%s", _("unexpected encryption format"));
 return -1;
 }
-virBufferAsprintf(buf, "\n", format);
+if (enc->engine == VIR_STORAGE_ENCRYPTION_ENGINE_DEFAULT) {
+virBufferAsprintf(buf, "\n", format);
+} else {
+if (!(engine = 

[PATCH v4 1/5] qemu: add disk post parse to qemublocktest

[Or Ozeri]

The post parse callback is part of the real (non-test) processing flow.
This commit adds it (for disks) to the qemublocktest flow as well.
Specifically, this will be needed for tests that use luks encryption,
so that the default encryption engine (which is added in an upcoming commit)
will be overridden by qemu.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_domain.c |  2 +-
 src/qemu/qemu_domain.h |  3 +++
 tests/qemublocktest.c  | 29 -
 3 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a755f8678e..288a40bca6 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5259,7 +5259,7 @@ 
qemuDomainDeviceDiskDefPostParseRestoreSecAlias(virDomainDiskDef *disk,
 }
 
 
-static int
+int
 qemuDomainDeviceDiskDefPostParse(virDomainDiskDef *disk,
  unsigned int parseFlags)
 {
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 64f92988b7..0642e44fbc 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -872,6 +872,9 @@ int qemuDomainSecretPrepare(virQEMUDriver *driver,
 int qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk,
 virQEMUCaps *qemuCaps);
 
+int qemuDomainDeviceDiskDefPostParse(virDomainDiskDef *disk,
+ unsigned int parseFlags);
+
 int qemuDomainPrepareChannel(virDomainChrDef *chr,
  const char *domainChannelTargetDir)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 3e61e923a9..0176fbd3f4 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -276,6 +276,9 @@ testQemuDiskXMLToProps(const void *opaque)
VIR_DOMAIN_DEF_PARSE_STATUS)))
 return -1;
 
+if (qemuDomainDeviceDiskDefPostParse(disk, 0) < 0)
+return -1;
+
 if (!(vmdef = virDomainDefNew(data->driver->xmlopt)))
 return -1;
 
@@ -470,32 +473,24 @@ testQemuImageCreateLoadDiskXML(const char *name,
virDomainXMLOption *xmlopt)
 
 {
-virDomainSnapshotDiskDef *diskdef = NULL;
-g_autoptr(xmlDoc) doc = NULL;
-g_autoptr(xmlXPathContext) ctxt = NULL;
-xmlNodePtr node;
+virDomainDiskDef *disk = NULL;
 g_autofree char *xmlpath = NULL;
-virStorageSource *ret = NULL;
+g_autofree char *xmlstr = NULL;
 
 xmlpath = g_strdup_printf("%s%s.xml", testQemuImageCreatePath, name);
 
-if (!(doc = virXMLParseFileCtxt(xmlpath, &ctxt)))
+if (virTestLoadFile(xmlpath, &xmlstr) < 0)
 return NULL;
 
-if (!(node = virXPathNode("//disk", ctxt))) {
-VIR_TEST_VERBOSE("failed to find  element\n");
+/* qemu stores node names in the status XML portion */
+if (!(disk = virDomainDiskDefParse(xmlstr, xmlopt,
+   VIR_DOMAIN_DEF_PARSE_STATUS)))
 return NULL;
-}
 
-diskdef = g_new0(virDomainSnapshotDiskDef, 1);
-
-if (virDomainSnapshotDiskDefParseXML(node, ctxt, diskdef,
- VIR_DOMAIN_DEF_PARSE_STATUS,
- xmlopt) == 0)
-ret = g_steal_pointer(&diskdef->src);
+if (qemuDomainDeviceDiskDefPostParse(disk, 0) < 0)
+return NULL;
 
-virDomainSnapshotDiskDefFree(diskdef);
-return ret;
+return disk->src;
 }
 
 
-- 
2.25.1

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

[Or Ozeri]

But that means changing the input file from using the default engine, to using "qemu" explicitly.I was thinking that keeping the input xml unchanged gets a stronger test, since it also checks that default uses qemu.Also since most users will still not use the new engine property, so keeping the input xml unchanged also tests the more common workload.-"Peter Krempa" <pkre...@redhat.com> wrote: ->To: "Or Ozeri" <o...@il.ibm.com>>From: "Peter Krempa" <pkre...@redhat.com>>Date: 10/07/2021 03:26PM>Cc: libvir-list@redhat.com, idryo...@gmail.com,>to.my.troc...@gmail.com, "Danny Harnik" <dan...@il.ibm.com>>Subject: [EXTERNAL] Re: [PATCH v3 4/5] conf: add encryption engine>property>>On Thu, Oct 07, 2021 at 11:31:42 +, Or Ozeri wrote:>>-"Peter Krempa" <[1]pkre...@redhat.com> wrote: ->>>> diff --git a/tests/qemuxml2xmloutdata/disk-nvme.xml>>>b/tests/qemuxml2xmloutdata/disk-nvme.xml>>>> deleted file mode 12>>>> index ea9eb267ac..00>>>> --- 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 00..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.>>Should I create a new file>>e.g. tests/qemuxml2xmloutdata/disk-nvme-content.xml>>and point the disk-nvme.xml symlink to it?>>No just keep symlinks as symlinks. You are not changing anything that>would warant a change to full output file. Namely if the output file>is>a symlink to the input file, then the change to the input file should>be>sufficient as the output file is the same one.>>

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

[Or Ozeri]

-"Peter Krempa"  wrote: ->> diff --git a/tests/qemuxml2xmloutdata/disk-nvme.xml>b/tests/qemuxml2xmloutdata/disk-nvme.xml>> deleted file mode 12>> index ea9eb267ac..00>> --- 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 00..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.Should I create a new file e.g. tests/qemuxml2xmloutdata/disk-nvme-content.xmland point the disk-nvme.xml symlink to it?

[PATCH v3 5/5] qemu: add librbd encryption engine

[Or Ozeri]

rbd encryption is new in qemu 6.1.0.
This commit adds a new encryption engine property which
allows the user to use this new encryption engine.

Signed-off-by: Or Ozeri 
---
 docs/formatstorageencryption.html.in  |  2 +-
 docs/schemas/storagecommon.rng|  1 +
 src/conf/storage_encryption_conf.c|  2 +-
 src/conf/storage_encryption_conf.h|  1 +
 src/qemu/qemu_block.c | 30 +++
 src/qemu/qemu_domain.c| 24 ++
 ...sk-network-rbd-encryption.x86_64-6.0.0.err |  1 +
 ...-network-rbd-encryption.x86_64-latest.args | 49 +++
 .../disk-network-rbd-encryption.xml   | 75 +
 tests/qemuxml2argvtest.c  |  2 +
 ...k-network-rbd-encryption.x86_64-latest.xml | 83 +++
 tests/qemuxml2xmltest.c   |  1 +
 12 files changed, 269 insertions(+), 2 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-6.0.0.err
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-encryption.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-network-rbd-encryption.x86_64-latest.xml

diff --git a/docs/formatstorageencryption.html.in 
b/docs/formatstorageencryption.html.in
index 5783381a4a..31ec2698a1 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -27,7 +27,7 @@
   The encryption tag supports an optional engine
   tag, which allows selecting which component actually handles
   the encryption. Currently defined values of engine are
-  qemu.
+  qemu and librbd.
 
 
   The encryption tag can currently contain a sequence of
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index b34577c582..591a158209 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -20,6 +20,7 @@
 
   
 qemu
+librbd
   
 
   
diff --git a/src/conf/storage_encryption_conf.c 
b/src/conf/storage_encryption_conf.c
index e8da02b605..2639b43119 100644
--- a/src/conf/storage_encryption_conf.c
+++ b/src/conf/storage_encryption_conf.c
@@ -49,7 +49,7 @@ VIR_ENUM_IMPL(virStorageEncryptionFormat,
 
 VIR_ENUM_IMPL(virStorageEncryptionEngine,
   VIR_STORAGE_ENCRYPTION_ENGINE_LAST,
-  "default", "qemu",
+  "default", "qemu", "librbd",
 );
 
 static void
diff --git a/src/conf/storage_encryption_conf.h 
b/src/conf/storage_encryption_conf.h
index c722f832f5..2e6f9193bd 100644
--- a/src/conf/storage_encryption_conf.h
+++ b/src/conf/storage_encryption_conf.h
@@ -54,6 +54,7 @@ struct _virStorageEncryptionInfoDef {
 typedef enum {
 VIR_STORAGE_ENCRYPTION_ENGINE_DEFAULT = 0,
 VIR_STORAGE_ENCRYPTION_ENGINE_QEMU,
+VIR_STORAGE_ENCRYPTION_ENGINE_LIBRBD,
 
 VIR_STORAGE_ENCRYPTION_ENGINE_LAST,
 } virStorageEncryptionEngineType;
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index a43831ce18..62c40d39d1 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -875,6 +875,8 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 qemuDomainStorageSourcePrivate *srcPriv = 
QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
 g_autoptr(virJSONValue) servers = NULL;
 virJSONValue *ret = NULL;
+g_autoptr(virJSONValue) encrypt = NULL;
+const char *encformat;
 const char *username = NULL;
 g_autoptr(virJSONValue) authmodes = NULL;
 g_autoptr(virJSONValue) mode = NULL;
@@ -899,12 +901,40 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 return NULL;
 }
 
+if (src->encryption &&
+src->encryption->engine == VIR_STORAGE_ENCRYPTION_ENGINE_LIBRBD) {
+switch ((virStorageEncryptionFormatType) src->encryption->format) {
+case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS:
+encformat = "luks";
+break;
+
+case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2:
+encformat = "luks2";
+break;
+
+case VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT:
+case VIR_STORAGE_ENCRYPTION_FORMAT_QCOW:
+case VIR_STORAGE_ENCRYPTION_FORMAT_LAST:
+default:
+virReportEnumRangeError(virStorageEncryptionFormatType,
+src->encryption->format);
+return NULL;
+}
+
+if (virJSONValueObjectCreate(&encrypt,
+ "s:format", encformat,
+ "s:key-secret", srcPriv->encinfo->alias,
+ NULL) < 0)
+return NULL;
+}
+
 if (virJSONVal

[PATCH v3 3/5] conf: add luks2 encryption format

[Or Ozeri]

This commit extends libvirt XML configuration to support luks2 encryption 
format.
This means that  becomes valid.
Actual handler (other than returning "not supported") for this new format will 
be added in an upcoming commit.

Signed-off-by: Or Ozeri 
---
 docs/formatstorageencryption.html.in | 2 +-
 docs/schemas/storagecommon.rng   | 1 +
 src/conf/storage_encryption_conf.c   | 2 +-
 src/conf/storage_encryption_conf.h   | 1 +
 src/qemu/qemu_block.c| 1 +
 src/qemu/qemu_domain.c   | 3 ++-
 6 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/docs/formatstorageencryption.html.in 
b/docs/formatstorageencryption.html.in
index 7215c307d7..b2631ab25d 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -18,7 +18,7 @@
   is encryption, with a mandatory
   attribute format.  Currently defined values
   of format are default, qcow,
-  and luks.
+  luks, and luks2.
   Each value of format implies some expectations about the
   content of the encryption tag.  Other format values may be
   defined in the future.
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 9ebb27700d..7d1d066289 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -13,6 +13,7 @@
   default
   qcow
   luks
+  luks2
 
   
   
diff --git a/src/conf/storage_encryption_conf.c 
b/src/conf/storage_encryption_conf.c
index 9112b96cc7..2df4ec96af 100644
--- a/src/conf/storage_encryption_conf.c
+++ b/src/conf/storage_encryption_conf.c
@@ -44,7 +44,7 @@ VIR_ENUM_IMPL(virStorageEncryptionSecret,
 
 VIR_ENUM_IMPL(virStorageEncryptionFormat,
   VIR_STORAGE_ENCRYPTION_FORMAT_LAST,
-  "default", "qcow", "luks",
+  "default", "qcow", "luks", "luks2",
 );
 
 static void
diff --git a/src/conf/storage_encryption_conf.h 
b/src/conf/storage_encryption_conf.h
index 34adbd5f7b..32e3a1243a 100644
--- a/src/conf/storage_encryption_conf.h
+++ b/src/conf/storage_encryption_conf.h
@@ -56,6 +56,7 @@ typedef enum {
 VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT = 0,
 VIR_STORAGE_ENCRYPTION_FORMAT_QCOW, /* Both qcow and qcow2 */
 VIR_STORAGE_ENCRYPTION_FORMAT_LUKS,
+VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2,
 
 VIR_STORAGE_ENCRYPTION_FORMAT_LAST,
 } virStorageEncryptionFormatType;
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 393d3f44d7..31b6b3566b 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1328,6 +1328,7 @@ qemuBlockStorageSourceGetCryptoProps(virStorageSource 
*src,
 break;
 
 case VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT:
+case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2:
 case VIR_STORAGE_ENCRYPTION_FORMAT_LAST:
 default:
 virReportEnumRangeError(virStorageEncryptionFormatType,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 288a40bca6..cd65e8b365 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1228,7 +1228,8 @@ static bool
 qemuDomainDiskHasEncryptionSecret(virStorageSource *src)
 {
 if (!virStorageSourceIsEmpty(src) && src->encryption &&
-src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS &&
+(src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS ||
+ src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2) &&
 src->encryption->nsecrets > 0)
 return true;
 
-- 
2.25.1

[PATCH v3 0/5] Add support for librbd encryption

[Or Ozeri]

v3: rebased on master
v2: addressed (hopefully) all of Peter's v1 comments (thanks Peter!)

Or Ozeri (5):
  qemu: add disk post parse to qemublocktest
  qemu: add rbd encryption capability probing
  conf: add luks2 encryption format
  conf: add encryption engine property
  qemu: add librbd encryption engine

 docs/formatstorageencryption.html.in  |   8 +-
 docs/schemas/domainbackup.rng |   7 +
 docs/schemas/storagecommon.rng|   9 +
 src/conf/storage_encryption_conf.c|  33 +++-
 src/conf/storage_encryption_conf.h|  11 ++
 src/qemu/qemu_block.c |  33 
 src/qemu/qemu_capabilities.c  |   2 +
 src/qemu/qemu_capabilities.h  |   1 +
 src/qemu/qemu_domain.c|  37 +++-
 src/qemu/qemu_domain.h|   3 +
 tests/qemublocktest.c |   3 +
 .../caps_6.1.0.x86_64.xml |   1 +
 tests/qemustatusxml2xmldata/upgrade-out.xml   |   6 +-
 ...sk-network-rbd-encryption.x86_64-6.0.0.err |   1 +
 ...-network-rbd-encryption.x86_64-latest.args |  49 ++
 .../disk-network-rbd-encryption.xml   |  75 +
 tests/qemuxml2argvtest.c  |   2 +
 ...k-network-rbd-encryption.x86_64-latest.xml |  83 +
 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 +-
 tests/qemuxml2xmltest.c   |   1 +
 27 files changed, 677 insertions(+), 27 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-6.0.0.err
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-encryption.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-network-rbd-encryption.x86_64-latest.xml
 mode change 12 => 100644 tests/qemuxml2xmloutdata/disk-nvme.xml
 mode change 12 => 100644 tests/qemuxml2xmloutdata/encrypted-disk-usage.xml
 mode change 12 => 100644 tests/qemuxml2xmloutdata/luks-disks.xml
 mode change 12 => 100644 tests/qemuxml2xmloutdata/user-aliases.xml

-- 
2.25.1

[PATCH v3 1/5] qemu: add disk post parse to qemublocktest

[Or Ozeri]

The post parse callback is part of the real (non-test) processing flow.
This commit adds it (for disks) to the qemublocktest flow as well.
Specifically, this will be needed for tests that use luks encryption,
so that the default encryption engine (which is added in an upcoming commit)
will be overridden by qemu.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_domain.c | 2 +-
 src/qemu/qemu_domain.h | 3 +++
 tests/qemublocktest.c  | 3 +++
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a755f8678e..288a40bca6 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5259,7 +5259,7 @@ 
qemuDomainDeviceDiskDefPostParseRestoreSecAlias(virDomainDiskDef *disk,
 }
 
 
-static int
+int
 qemuDomainDeviceDiskDefPostParse(virDomainDiskDef *disk,
  unsigned int parseFlags)
 {
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 64f92988b7..0642e44fbc 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -872,6 +872,9 @@ int qemuDomainSecretPrepare(virQEMUDriver *driver,
 int qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk,
 virQEMUCaps *qemuCaps);
 
+int qemuDomainDeviceDiskDefPostParse(virDomainDiskDef *disk,
+ unsigned int parseFlags);
+
 int qemuDomainPrepareChannel(virDomainChrDef *chr,
  const char *domainChannelTargetDir)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 3e61e923a9..0e4bb146c9 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -276,6 +276,9 @@ testQemuDiskXMLToProps(const void *opaque)
VIR_DOMAIN_DEF_PARSE_STATUS)))
 return -1;
 
+if (qemuDomainDeviceDiskDefPostParse(disk, 0) < 0)
+return -1;
+
 if (!(vmdef = virDomainDefNew(data->driver->xmlopt)))
 return -1;
 
-- 
2.25.1

[PATCH v3 2/5] qemu: add rbd encryption capability probing

[Or Ozeri]

rbd encryption is new in qemu 6.1.0.
This commit adds capability probing for it.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 1 +
 3 files changed, 4 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 82687dbf39..ea0734db15 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -644,6 +644,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "virtio-mem-pci", /* QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI */
   "memory-backend-file.reserve", /* 
QEMU_CAPS_MEMORY_BACKEND_RESERVE */
   "piix4.acpi-root-pci-hotplug", /* 
QEMU_CAPS_PIIX4_ACPI_ROOT_PCI_HOTPLUG */
+  "rbd-encryption", /* QEMU_CAPS_RBD_ENCRYPTION */
 );
 
 
@@ -1565,6 +1566,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsQMPSchemaQueries[] = {
 { "blockdev-add/arg-type/+file/$dynamic-auto-read-only", 
QEMU_CAPS_BLOCK_FILE_AUTO_READONLY_DYNAMIC },
 { "blockdev-add/arg-type/+nvme", QEMU_CAPS_DRIVE_NVME },
 { "blockdev-add/arg-type/+file/aio/^io_uring", QEMU_CAPS_AIO_IO_URING },
+{ "blockdev-add/arg-type/+rbd/encrypt", QEMU_CAPS_RBD_ENCRYPTION },
 { "blockdev-add/arg-type/discard", QEMU_CAPS_DRIVE_DISCARD },
 { "blockdev-add/arg-type/detect-zeroes", QEMU_CAPS_DRIVE_DETECT_ZEROES },
 { "blockdev-backup", QEMU_CAPS_BLOCKDEV_BACKUP },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 2bbfc15dc4..674da98539 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -624,6 +624,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_DEVICE_VIRTIO_MEM_PCI, /* -device virtio-mem-pci */
 QEMU_CAPS_MEMORY_BACKEND_RESERVE, /* -object memory-backend-*.reserve= */
 QEMU_CAPS_PIIX4_ACPI_ROOT_PCI_HOTPLUG, /* -M pc 
PIIX4_PM.acpi-root-pci-hotplug */
+QEMU_CAPS_RBD_ENCRYPTION, /* Ceph RBD encryption support */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
index 87b37a2b7c..8180cfd6c2 100644
--- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
@@ -243,6 +243,7 @@
   
   
   
+  
   6001000
   0
   43100243
-- 
2.25.1

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

[Or Ozeri]

This commit extends libvirt XML configuration to support a custom encryption 
engine.
This means that   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 
---
 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 12 => 100644 tests/qemuxml2xmloutdata/disk-nvme.xml
 mode change 12 => 100644 tests/qemuxml2xmloutdata/encrypted-disk-usage.xml
 mode change 12 => 100644 tests/qemuxml2xmloutdata/luks-disks.xml
 mode change 12 => 100644 tests/qemuxml2xmloutdata/user-aliases.xml

diff --git a/docs/formatstorageencryption.html.in 
b/docs/formatstorageencryption.html.in
index b2631ab25d..5783381a4a 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -23,6 +23,12 @@
   content of the encryption tag.  Other format values may be
   defined in the future.
 
+
+  The encryption tag supports an optional engine
+  tag, which allows selecting which component actually handles
+  the encryption. Currently defined values of engine are
+  qemu.
+
 
   The encryption tag can currently contain a sequence of
   secret tags, each with mandatory attributes 
type
diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng
index c03455a5a7..05cc28ab00 100644
--- a/docs/schemas/domainbackup.rng
+++ b/docs/schemas/domainbackup.rng
@@ -14,6 +14,13 @@
   luks
 
   
+  
+
+  
+qemu
+  
+
+  
   
 
 
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 7d1d066289..b34577c582 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -16,6 +16,13 @@
   luks2
 
   
+  
+
+  
+qemu
+  
+
+  
   
 
 
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) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("unknown volume encryption engine type %s"),
+   engine_str);
+goto cleanup;
+}
+}
+
 if ((n = virXPathNodeSet("./secret", ctxt, &nodes)) < 0)
 goto cleanup;
 
@@ -327,6 +344,7 @@ int
 virStorageEncryptionFormat(virBuffer *buf,
virStora

RE: [PATCH v2 1/5] qemu: add disk post parse to qemublocktest

[Or Ozeri]

My commit which adds the encryption engine configuration is default to "default".Only in the post-parse callback in the qemu driver, I switch it from "default" to "qemu".Thus, if you don't call the post-parse on this test, then tests which use luks format fail.Specifically:network-qcow2-backing-chain-encryption_authfile-qcow2-backing-chain-encryptionfile-raw-luks-"Peter Krempa" <pkre...@redhat.com> wrote: -To: "Or Ozeri" <o...@il.ibm.com>From: "Peter Krempa" <pkre...@redhat.com>Date: 10/06/2021 09:37AMCc: libvir-list@redhat.com, idryo...@gmail.com, to.my.troc...@gmail.com, dan...@il.ibm.comSubject: [EXTERNAL] Re: [PATCH v2 1/5] qemu: add disk post parse to qemublocktestOn Tue, Oct 05, 2021 at 09:41:12 -0500, Or Ozeri wrote:> The post parse callback is part of the real (non-test) processing flow.> This commit adds it (for disks) to the qemublocktest flow as well.Could you please elaborate why this is needed? Specificallyqemublocktest takes a few liberties from the "real" code flow since weneed to fake a lot of stuff. E.g. see testQemuDiskXMLToJSONFakeSecrets.Specifically I didn't see anything in your patches [1] which would addanything to the post parse callback.[1] Well after my crude rebase of the series. What you've posted didn'tapply neither on master nor on the last release. I had to go one versionback and it had conflicts which I didn't spend much time thinking about.

[PATCH v2 0/5] Add support for librbd encryption

[Or Ozeri]

Changes from v1: addressed (hopefully) all of Peter's v1 comments (thanks 
Peter!)

Or Ozeri (5):
  qemu: add disk post parse to qemublocktest
  qemu: add rbd encryption capability probing
  conf: add luks2 encryption format
  conf: add encryption engine property
  qemu: add librbd encryption engine

 docs/formatstorageencryption.html.in  |   8 +-
 docs/schemas/domainbackup.rng |   7 +
 docs/schemas/storagecommon.rng|   9 +
 src/conf/storage_encryption_conf.c|  33 +++-
 src/conf/storage_encryption_conf.h|  11 ++
 src/qemu/qemu_block.c |  33 
 src/qemu/qemu_capabilities.c  |   2 +
 src/qemu/qemu_capabilities.h  |   1 +
 src/qemu/qemu_domain.c|  37 +++-
 src/qemu/qemu_domain.h|   4 +
 tests/qemublocktest.c |   3 +
 .../caps_6.1.0.x86_64.xml |   1 +
 tests/qemustatusxml2xmldata/upgrade-out.xml   |   6 +-
 ...sk-network-rbd-encryption.x86_64-6.0.0.err |   1 +
 ...-network-rbd-encryption.x86_64-latest.args |  49 ++
 .../disk-network-rbd-encryption.xml   |  75 +
 tests/qemuxml2argvtest.c  |   2 +
 ...k-network-rbd-encryption.x86_64-latest.xml |  83 +
 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 +-
 tests/qemuxml2xmltest.c   |   1 +
 27 files changed, 678 insertions(+), 27 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-6.0.0.err
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-encryption.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-network-rbd-encryption.x86_64-latest.xml
 mode change 12 => 100644 tests/qemuxml2xmloutdata/disk-nvme.xml
 mode change 12 => 100644 tests/qemuxml2xmloutdata/encrypted-disk-usage.xml
 mode change 12 => 100644 tests/qemuxml2xmloutdata/luks-disks.xml
 mode change 12 => 100644 tests/qemuxml2xmloutdata/user-aliases.xml

-- 
2.25.1

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

[Or Ozeri]

This commit extends libvirt XML configuration to support a custom encryption 
engine.
This means that   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 
---
 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 12 => 100644 tests/qemuxml2xmloutdata/disk-nvme.xml
 mode change 12 => 100644 tests/qemuxml2xmloutdata/encrypted-disk-usage.xml
 mode change 12 => 100644 tests/qemuxml2xmloutdata/luks-disks.xml
 mode change 12 => 100644 tests/qemuxml2xmloutdata/user-aliases.xml

diff --git a/docs/formatstorageencryption.html.in 
b/docs/formatstorageencryption.html.in
index b2631ab25d..5783381a4a 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -23,6 +23,12 @@
   content of the encryption tag.  Other format values may be
   defined in the future.
 
+
+  The encryption tag supports an optional engine
+  tag, which allows selecting which component actually handles
+  the encryption. Currently defined values of engine are
+  qemu.
+
 
   The encryption tag can currently contain a sequence of
   secret tags, each with mandatory attributes 
type
diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng
index c03455a5a7..05cc28ab00 100644
--- a/docs/schemas/domainbackup.rng
+++ b/docs/schemas/domainbackup.rng
@@ -14,6 +14,13 @@
   luks
 
   
+  
+
+  
+qemu
+  
+
+  
   
 
 
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 7d1d066289..b34577c582 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -16,6 +16,13 @@
   luks2
 
   
+  
+
+  
+qemu
+  
+
+  
   
 
 
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) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("unknown volume encryption engine type %s"),
+   engine_str);
+goto cleanup;
+}
+}
+
 if ((n = virXPathNodeSet("./secret", ctxt, &nodes)) < 0)
 goto cleanup;
 
@@ -327,6 +344,7 @@ int
 virStorageEncryptionFormat(virBuffer *buf,
virStora

[PATCH v2 1/5] qemu: add disk post parse to qemublocktest

[Or Ozeri]

The post parse callback is part of the real (non-test) processing flow.
This commit adds it (for disks) to the qemublocktest flow as well.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_domain.c | 2 +-
 src/qemu/qemu_domain.h | 4 
 tests/qemublocktest.c  | 3 +++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 25b7f03204..472ff670b1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5415,7 +5415,7 @@ 
qemuDomainDeviceDiskDefPostParseRestoreSecAlias(virDomainDiskDef *disk,
 }
 
 
-static int
+int
 qemuDomainDeviceDiskDefPostParse(virDomainDiskDef *disk,
  virQEMUCaps *qemuCaps,
  unsigned int parseFlags)
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index cb1cd968d5..9a784501a0 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -899,6 +899,10 @@ int qemuDomainDefValidateDiskLunSource(const 
virStorageSource *src)
 int qemuDomainDeviceDefValidateDisk(const virDomainDiskDef *disk,
 virQEMUCaps *qemuCaps);
 
+int qemuDomainDeviceDiskDefPostParse(virDomainDiskDef *disk,
+ virQEMUCaps *qemuCaps,
+ unsigned int parseFlags);
+
 int qemuDomainPrepareChannel(virDomainChrDef *chr,
  const char *domainChannelTargetDir)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 4af8862c5b..617e1b8ae1 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -279,6 +279,9 @@ testQemuDiskXMLToProps(const void *opaque)
VIR_DOMAIN_DEF_PARSE_STATUS)))
 return -1;
 
+if (qemuDomainDeviceDiskDefPostParse(disk, data->qemuCaps, 0) < 0)
+return -1;
+
 if (!(vmdef = virDomainDefNew(data->driver->xmlopt)))
 return -1;
 
-- 
2.25.1

[PATCH v2 3/5] conf: add luks2 encryption format

[Or Ozeri]

This commit extends libvirt XML configuration to support luks2 encryption 
format.
This means that  becomes valid.
Actual handler (other than returning "not supported") for this new format will 
be added in an upcoming commit.

Signed-off-by: Or Ozeri 
---
 docs/formatstorageencryption.html.in | 2 +-
 docs/schemas/storagecommon.rng   | 1 +
 src/conf/storage_encryption_conf.c   | 2 +-
 src/conf/storage_encryption_conf.h   | 1 +
 src/qemu/qemu_block.c| 1 +
 src/qemu/qemu_domain.c   | 3 ++-
 6 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/docs/formatstorageencryption.html.in 
b/docs/formatstorageencryption.html.in
index 7215c307d7..b2631ab25d 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -18,7 +18,7 @@
   is encryption, with a mandatory
   attribute format.  Currently defined values
   of format are default, qcow,
-  and luks.
+  luks, and luks2.
   Each value of format implies some expectations about the
   content of the encryption tag.  Other format values may be
   defined in the future.
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 9ebb27700d..7d1d066289 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -13,6 +13,7 @@
   default
   qcow
   luks
+  luks2
 
   
   
diff --git a/src/conf/storage_encryption_conf.c 
b/src/conf/storage_encryption_conf.c
index 9112b96cc7..2df4ec96af 100644
--- a/src/conf/storage_encryption_conf.c
+++ b/src/conf/storage_encryption_conf.c
@@ -44,7 +44,7 @@ VIR_ENUM_IMPL(virStorageEncryptionSecret,
 
 VIR_ENUM_IMPL(virStorageEncryptionFormat,
   VIR_STORAGE_ENCRYPTION_FORMAT_LAST,
-  "default", "qcow", "luks",
+  "default", "qcow", "luks", "luks2",
 );
 
 static void
diff --git a/src/conf/storage_encryption_conf.h 
b/src/conf/storage_encryption_conf.h
index 34adbd5f7b..32e3a1243a 100644
--- a/src/conf/storage_encryption_conf.h
+++ b/src/conf/storage_encryption_conf.h
@@ -56,6 +56,7 @@ typedef enum {
 VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT = 0,
 VIR_STORAGE_ENCRYPTION_FORMAT_QCOW, /* Both qcow and qcow2 */
 VIR_STORAGE_ENCRYPTION_FORMAT_LUKS,
+VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2,
 
 VIR_STORAGE_ENCRYPTION_FORMAT_LAST,
 } virStorageEncryptionFormatType;
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 0bc92f6a23..f7aa052822 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1333,6 +1333,7 @@ qemuBlockStorageSourceGetCryptoProps(virStorageSource 
*src,
 break;
 
 case VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT:
+case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2:
 case VIR_STORAGE_ENCRYPTION_FORMAT_LAST:
 default:
 virReportEnumRangeError(virStorageEncryptionFormatType,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 472ff670b1..2d35106c2f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1354,7 +1354,8 @@ static bool
 qemuDomainDiskHasEncryptionSecret(virStorageSource *src)
 {
 if (!virStorageSourceIsEmpty(src) && src->encryption &&
-src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS &&
+(src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS ||
+ src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2) &&
 src->encryption->nsecrets > 0)
 return true;
 
-- 
2.25.1

[PATCH v2 5/5] qemu: add librbd encryption engine

[Or Ozeri]

rbd encryption is new in qemu 6.1.0.
This commit adds a new encryption engine property which
allows the user to use this new encryption engine.

Signed-off-by: Or Ozeri 
---
 docs/formatstorageencryption.html.in  |  2 +-
 docs/schemas/storagecommon.rng|  1 +
 src/conf/storage_encryption_conf.c|  2 +-
 src/conf/storage_encryption_conf.h|  1 +
 src/qemu/qemu_block.c | 30 +++
 src/qemu/qemu_domain.c| 24 ++
 ...sk-network-rbd-encryption.x86_64-6.0.0.err |  1 +
 ...-network-rbd-encryption.x86_64-latest.args | 49 +++
 .../disk-network-rbd-encryption.xml   | 75 +
 tests/qemuxml2argvtest.c  |  2 +
 ...k-network-rbd-encryption.x86_64-latest.xml | 83 +++
 tests/qemuxml2xmltest.c   |  1 +
 12 files changed, 269 insertions(+), 2 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-6.0.0.err
 create mode 100644 
tests/qemuxml2argvdata/disk-network-rbd-encryption.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-network-rbd-encryption.xml
 create mode 100644 
tests/qemuxml2xmloutdata/disk-network-rbd-encryption.x86_64-latest.xml

diff --git a/docs/formatstorageencryption.html.in 
b/docs/formatstorageencryption.html.in
index 5783381a4a..31ec2698a1 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -27,7 +27,7 @@
   The encryption tag supports an optional engine
   tag, which allows selecting which component actually handles
   the encryption. Currently defined values of engine are
-  qemu.
+  qemu and librbd.
 
 
   The encryption tag can currently contain a sequence of
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index b34577c582..591a158209 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -20,6 +20,7 @@
 
   
 qemu
+librbd
   
 
   
diff --git a/src/conf/storage_encryption_conf.c 
b/src/conf/storage_encryption_conf.c
index e8da02b605..2639b43119 100644
--- a/src/conf/storage_encryption_conf.c
+++ b/src/conf/storage_encryption_conf.c
@@ -49,7 +49,7 @@ VIR_ENUM_IMPL(virStorageEncryptionFormat,
 
 VIR_ENUM_IMPL(virStorageEncryptionEngine,
   VIR_STORAGE_ENCRYPTION_ENGINE_LAST,
-  "default", "qemu",
+  "default", "qemu", "librbd",
 );
 
 static void
diff --git a/src/conf/storage_encryption_conf.h 
b/src/conf/storage_encryption_conf.h
index c722f832f5..2e6f9193bd 100644
--- a/src/conf/storage_encryption_conf.h
+++ b/src/conf/storage_encryption_conf.h
@@ -54,6 +54,7 @@ struct _virStorageEncryptionInfoDef {
 typedef enum {
 VIR_STORAGE_ENCRYPTION_ENGINE_DEFAULT = 0,
 VIR_STORAGE_ENCRYPTION_ENGINE_QEMU,
+VIR_STORAGE_ENCRYPTION_ENGINE_LIBRBD,
 
 VIR_STORAGE_ENCRYPTION_ENGINE_LAST,
 } virStorageEncryptionEngineType;
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 693c43dfcc..164365b820 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -875,6 +875,8 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 qemuDomainStorageSourcePrivate *srcPriv = 
QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
 g_autoptr(virJSONValue) servers = NULL;
 virJSONValue *ret = NULL;
+g_autoptr(virJSONValue) encrypt = NULL;
+const char *encformat;
 const char *username = NULL;
 g_autoptr(virJSONValue) authmodes = NULL;
 g_autoptr(virJSONValue) mode = NULL;
@@ -899,12 +901,40 @@ qemuBlockStorageSourceGetRBDProps(virStorageSource *src,
 return NULL;
 }
 
+if (src->encryption &&
+src->encryption->engine == VIR_STORAGE_ENCRYPTION_ENGINE_LIBRBD) {
+switch ((virStorageEncryptionFormatType) src->encryption->format) {
+case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS:
+encformat = "luks";
+break;
+
+case VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2:
+encformat = "luks2";
+break;
+
+case VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT:
+case VIR_STORAGE_ENCRYPTION_FORMAT_QCOW:
+case VIR_STORAGE_ENCRYPTION_FORMAT_LAST:
+default:
+virReportEnumRangeError(virStorageEncryptionFormatType,
+src->encryption->format);
+return NULL;
+}
+
+if (virJSONValueObjectCreate(&encrypt,
+ "s:format", encformat,
+ "s:key-secret", 
srcPriv->encinfo->s.aes.alias,
+ NULL) < 0)
+return NULL;
+}
+
 if (virJSONVal

[PATCH v2 2/5] qemu: add rbd encryption capability probing

[Or Ozeri]

rbd encryption is new in qemu 6.1.0.
This commit adds capability probing for it.

Signed-off-by: Or Ozeri 
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 1 +
 3 files changed, 4 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 70c3ec2f0c..85da5725cf 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -638,6 +638,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
   "query-display-options", /* QEMU_CAPS_QUERY_DISPLAY_OPTIONS */
   "s390-pv-guest", /* QEMU_CAPS_S390_PV_GUEST */
   "set-action", /* QEMU_CAPS_SET_ACTION */
+  "rbd-encryption", /* QEMU_CAPS_RBD_ENCRYPTION */
 );
 
 
@@ -1560,6 +1561,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsQMPSchemaQueries[] = {
 { "blockdev-add/arg-type/+file/$dynamic-auto-read-only", 
QEMU_CAPS_BLOCK_FILE_AUTO_READONLY_DYNAMIC },
 { "blockdev-add/arg-type/+nvme", QEMU_CAPS_DRIVE_NVME },
 { "blockdev-add/arg-type/+file/aio/^io_uring", QEMU_CAPS_AIO_IO_URING },
+{ "blockdev-add/arg-type/+rbd/encrypt", QEMU_CAPS_RBD_ENCRYPTION },
 { "blockdev-add/arg-type/discard", QEMU_CAPS_DRIVE_DISCARD },
 { "blockdev-add/arg-type/detect-zeroes", QEMU_CAPS_DRIVE_DETECT_ZEROES },
 { "blockdev-backup", QEMU_CAPS_BLOCKDEV_BACKUP },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index bc762d1916..576ed9d1ba 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -618,6 +618,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 QEMU_CAPS_QUERY_DISPLAY_OPTIONS, /* 'query-display-options' qmp command 
present */
 QEMU_CAPS_S390_PV_GUEST, /* -object s390-pv-guest,... */
 QEMU_CAPS_SET_ACTION, /* 'set-action' QMP command */
+QEMU_CAPS_RBD_ENCRYPTION, /* Ceph RBD encryption support */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
index eca9facf80..efd37e8ee1 100644
--- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml
@@ -257,6 +257,7 @@
   
   
   
+  
   6001000
   0
   43100243
-- 
2.25.1

[PATCH v1] qemu: Add support for librbd encryption

[Or Ozeri]

Starting from ceph Pacific, RBD has built-in support for image-level encryption.
qemu 6.1 added support for this encryption using a new "encrypt" property
to the RBD qdict.
This commit extends the libvirt XML API to allow the user to choose between
the existing qemu encryption engine, and the new librbd encryption engine.

Signed-off-by: Or Ozeri 
---
 docs/formatstorageencryption.html.in  |  8 +++-
 docs/schemas/domainbackup.rng |  7 
 docs/schemas/storagecommon.rng|  8 
 src/conf/storage_encryption_conf.c| 30 +-
 src/conf/storage_encryption_conf.h| 11 +
 src/qemu/qemu_block.c | 40 +++
 src/qemu/qemu_domain.c|  3 +-
 .../backup-pull-encrypted.xml |  6 +--
 .../backup-pull-internal-invalid.xml  |  6 +--
 .../backup-push-encrypted.xml |  6 +--
 tests/qemustatusxml2xmldata/upgrade-out.xml   |  6 +--
 tests/qemuxml2argvdata/disk-nvme.xml  |  2 +-
 tests/qemuxml2argvdata/disk-slices.xml|  4 +-
 .../qemuxml2argvdata/encrypted-disk-usage.xml |  2 +-
 tests/qemuxml2argvdata/encrypted-disk.xml |  2 +-
 .../luks-disks-source-qcow2.xml   | 14 +++
 tests/qemuxml2argvdata/luks-disks-source.xml  | 10 ++---
 tests/qemuxml2argvdata/luks-disks.xml |  4 +-
 tests/qemuxml2argvdata/user-aliases.xml   |  2 +-
 .../disk-slices.x86_64-latest.xml |  4 +-
 tests/qemuxml2xmloutdata/encrypted-disk.xml   |  2 +-
 .../luks-disks-source-qcow2.x86_64-latest.xml | 14 +++
 .../qemuxml2xmloutdata/luks-disks-source.xml  | 10 ++---
 .../storagevolxml2xmlout/vol-luks-cipher.xml  |  2 +-
 tests/storagevolxml2xmlout/vol-luks.xml   |  2 +-
 .../vol-qcow2-encryption.xml  |  2 +-
 tests/storagevolxml2xmlout/vol-qcow2-luks.xml |  2 +-
 27 files changed, 154 insertions(+), 55 deletions(-)

diff --git a/docs/formatstorageencryption.html.in 
b/docs/formatstorageencryption.html.in
index 7215c307d7..e0eb8697aa 100644
--- a/docs/formatstorageencryption.html.in
+++ b/docs/formatstorageencryption.html.in
@@ -18,11 +18,17 @@
   is encryption, with a mandatory
   attribute format.  Currently defined values
   of format are default, qcow,
-  and luks.
+  luks, and luks2.
   Each value of format implies some expectations about the
   content of the encryption tag.  Other format values may be
   defined in the future.
 
+
+  The encryption tag supports an optional engine
+  tag, which allows selecting which component actually handles
+  the encryption. Currently defined values of engine are
+  qemu (default) and librbd.
+
 
   The encryption tag can currently contain a sequence of
   secret tags, each with mandatory attributes 
type
diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng
index c03455a5a7..05cc28ab00 100644
--- a/docs/schemas/domainbackup.rng
+++ b/docs/schemas/domainbackup.rng
@@ -14,6 +14,13 @@
   luks
 
   
+  
+
+  
+qemu
+  
+
+  
   
 
 
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 9ebb27700d..3ddff02e43 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -15,6 +15,14 @@
   luks
 
   
+  
+
+  
+qemu
+librbd
+  
+
+  
   
 
 
diff --git a/src/conf/storage_encryption_conf.c 
b/src/conf/storage_encryption_conf.c
index 9112b96cc7..64044057bf 100644
--- a/src/conf/storage_encryption_conf.c
+++ b/src/conf/storage_encryption_conf.c
@@ -44,7 +44,12 @@ VIR_ENUM_IMPL(virStorageEncryptionSecret,
 
 VIR_ENUM_IMPL(virStorageEncryptionFormat,
   VIR_STORAGE_ENCRYPTION_FORMAT_LAST,
-  "default", "qcow", "luks",
+  "default", "qcow", "luks", "luks2",
+);
+
+VIR_ENUM_IMPL(virStorageEncryptionEngine,
+  VIR_STORAGE_ENCRYPTION_ENGINE_LAST,
+  "qemu", "librbd",
 );
 
 static void
@@ -217,6 +222,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 +245,18 @@ virStorageEncryptionParseNode(xmlNodePtr node,
 goto cleanup;
 }
 
+if (!(engine_str = virXPathString("string(./@engine)", ctxt))) {
+encdef->engine = VIR_STORAGE_ENCRYPTION_ENGINE_QEMU;
+} else {
+if ((encdef->engine =
+ virStorageEncryptionEngineTypeFromString(engine_str)) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPO

RBD encryption support in libvirt

[Or Ozeri]

Hi,


I wanted to get your advice on a patch I'm preparing for libvirt.
It touches the code-path that allows using LUKS encryption on top of an RBD 
image.


We recently added LUKS and LUKS2 encryption support in Ceph's librbd.
We exposed this in qemu in a recent patch by adding new optional "encrypt" 
member to BlockdevOptionsRbd.
This patch was included in the recent release of qemu 6.1.
To enable libvirt users to use librbd encryption, we need libvirt to use this 
new "encrypt" when it builds the blockdev options for RBD.


The interesting question is how to define the libvirt XML syntax that will 
trigger the use of librbd encryption.
My thought was to use the already existing  tag.
In that case, we just need to add a new format 
VIR_STORAGE_ENCRYPTION_FORMAT_LUKS2 to the enum virStorageEncryptionFormatType.
This type will be checked in qemuBlockStorageSourceGetRBDProps.


The problem with this approach is that it only works for LUKS2.
librbd encryption also supports LUKS1.
We want to allow the user to choose between the qemu LUKS implementation and 
the librbd one.
One reason to keep support both is that on the one hand librbd only supports 
XTS mode.
On the other hand, qemu implementation will not support a chain of uniquely 
encrypted RBD images (each serving as a backing store for the previous one).


So we need a way in the XML API to support both implementations.
Our current thought is to add a new "engine" attribute to the encryption tag.
By default, encryption will use the QEMU LUKS implementation, unless 
 is specified.
To make this more general, we can have engine='backend' instead of engine='rbd' 
to denote that the encryption is to be delegated to the backend storage 
properties (instead of the format properties).
This way, the encryption tag will only be parsed in the flow of 
qemuBlockStorageSourceGetBackendProps, instead of in 
qemuBlockStorageSourceGetBlockdevProps


We'll appreciate any feedback you have on this.


Thanks,
Or