[libvirt] [PATCH 08/12] qemu: domain: Extract setup for disk source secrets

Peter Krempa posted 12 patches 7 years, 6 months ago
[libvirt] [PATCH 08/12] qemu: domain: Extract setup for disk source secrets
Posted by Peter Krempa 7 years, 6 months ago
Separate it so that it deals only with single virStorageSource, so that
it can later be reused for full backing chain support.

Two aliases are passed since authentication is more relevant to the
'storage backend' whereas encryption is more relevant to the protocol
layer. When using node names, the aliases will be different.
---
 src/qemu/qemu_domain.c | 49 +++++++++++++++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 18 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 24ed61bc2..4a2ba1761 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1368,27 +1368,19 @@ qemuDomainDiskHasEncryptionSecret(virStorageSourcePtr src)
 }


-/* qemuDomainSecretDiskPrepare:
- * @conn: Pointer to connection
- * @priv: pointer to domain private object
- * @disk: Pointer to a disk definition
- *
- * For the right disk, generate the qemuDomainSecretInfo structure.
- *
- * Returns 0 on success, -1 on failure
- */
-int
-qemuDomainSecretDiskPrepare(virConnectPtr conn,
-                            qemuDomainObjPrivatePtr priv,
-                            virDomainDiskDefPtr disk)
+static int
+qemuDomainSecretStorageSourcePrepare(virConnectPtr conn,
+                                     qemuDomainObjPrivatePtr priv,
+                                     virStorageSourcePtr src,
+                                     const char *authalias,
+                                     const char *encalias)
 {
-    virStorageSourcePtr src = disk->src;
     qemuDomainStorageSourcePrivatePtr srcPriv;

-    if (!(disk->src->privateData = qemuDomainStorageSourcePrivateNew()))
+    if (!(src->privateData = qemuDomainStorageSourcePrivateNew()))
         return -1;

-    srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
+    srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);

     if (qemuDomainSecretDiskCapable(src)) {
         virSecretUsageType usageType = VIR_SECRET_USAGE_TYPE_ISCSI;
@@ -1397,7 +1389,7 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
             usageType = VIR_SECRET_USAGE_TYPE_CEPH;

         if (!(srcPriv->secinfo =
-              qemuDomainSecretInfoNew(conn, priv, disk->info.alias,
+              qemuDomainSecretInfoNew(conn, priv, authalias,
                                       usageType, src->auth->username,
                                       &src->auth->seclookupdef, false)))
               return -1;
@@ -1405,7 +1397,7 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,

     if (qemuDomainDiskHasEncryptionSecret(src)) {
         if (!(srcPriv->encinfo =
-              qemuDomainSecretInfoNew(conn, priv, disk->info.alias,
+              qemuDomainSecretInfoNew(conn, priv, encalias,
                                       VIR_SECRET_USAGE_TYPE_VOLUME, NULL,
                                       &src->encryption->secrets[0]->seclookupdef,
                                       true)))
@@ -1416,6 +1408,27 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
 }


+/* qemuDomainSecretDiskPrepare:
+ * @conn: Pointer to connection
+ * @priv: pointer to domain private object
+ * @disk: Pointer to a disk definition
+ *
+ * For the right disk, generate the qemuDomainSecretInfo structure.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+
+int
+qemuDomainSecretDiskPrepare(virConnectPtr conn,
+                            qemuDomainObjPrivatePtr priv,
+                            virDomainDiskDefPtr disk)
+{
+    return qemuDomainSecretStorageSourcePrepare(conn, priv, disk->src,
+                                                disk->info.alias,
+                                                disk->info.alias);
+}
+
+
 /* qemuDomainSecretHostdevDestroy:
  * @disk: Pointer to a hostdev definition
  *
-- 
2.14.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/12] qemu: domain: Extract setup for disk source secrets
Posted by John Ferlan 7 years, 6 months ago

On 10/20/2017 09:47 AM, Peter Krempa wrote:
> Separate it so that it deals only with single virStorageSource, so that
> it can later be reused for full backing chain support.
> 
> Two aliases are passed since authentication is more relevant to the
> 'storage backend' whereas encryption is more relevant to the protocol
> layer. When using node names, the aliases will be different.
> ---
>  src/qemu/qemu_domain.c | 49 +++++++++++++++++++++++++++++++------------------
>  1 file changed, 31 insertions(+), 18 deletions(-)
> 

FWIW: The @authalias would be the secret for the source to access the
server (RBD only right now, but the iSCSI patches are on list) while the
@encalias would be the LUKS secret.

Does the backing chain allow different "auth" sources for different
chain members? Mind boggling, but I guess possible.

I suppose it would also be possible that some member of the chain
doesn't use the auth, but rather it's a local LUKS encrypted file - is
that the essential goal?

I think perhaps it'd help to add some comments for
qemuDomainSecretStorageSourcePrepare in order to describe the parameters
and the "expectations" for varying levels of the chain.  There's also
some "assumptions" built into the hotplug code for at least the top level.

In general as long as you add a bit more details as to the parameter
expectations,

Reviewed-by: John Ferlan <jferlan@redhat.com>

John

> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 24ed61bc2..4a2ba1761 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1368,27 +1368,19 @@ qemuDomainDiskHasEncryptionSecret(virStorageSourcePtr src)
>  }
> 
> 
> -/* qemuDomainSecretDiskPrepare:
> - * @conn: Pointer to connection
> - * @priv: pointer to domain private object
> - * @disk: Pointer to a disk definition
> - *
> - * For the right disk, generate the qemuDomainSecretInfo structure.
> - *
> - * Returns 0 on success, -1 on failure
> - */
> -int
> -qemuDomainSecretDiskPrepare(virConnectPtr conn,
> -                            qemuDomainObjPrivatePtr priv,
> -                            virDomainDiskDefPtr disk)
> +static int
> +qemuDomainSecretStorageSourcePrepare(virConnectPtr conn,
> +                                     qemuDomainObjPrivatePtr priv,
> +                                     virStorageSourcePtr src,
> +                                     const char *authalias,
> +                                     const char *encalias)
>  {
> -    virStorageSourcePtr src = disk->src;
>      qemuDomainStorageSourcePrivatePtr srcPriv;
> 
> -    if (!(disk->src->privateData = qemuDomainStorageSourcePrivateNew()))
> +    if (!(src->privateData = qemuDomainStorageSourcePrivateNew()))
>          return -1;
> 
> -    srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
> +    srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src);
> 
>      if (qemuDomainSecretDiskCapable(src)) {
>          virSecretUsageType usageType = VIR_SECRET_USAGE_TYPE_ISCSI;
> @@ -1397,7 +1389,7 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
>              usageType = VIR_SECRET_USAGE_TYPE_CEPH;
> 
>          if (!(srcPriv->secinfo =
> -              qemuDomainSecretInfoNew(conn, priv, disk->info.alias,
> +              qemuDomainSecretInfoNew(conn, priv, authalias,
>                                        usageType, src->auth->username,
>                                        &src->auth->seclookupdef, false)))
>                return -1;
> @@ -1405,7 +1397,7 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
> 
>      if (qemuDomainDiskHasEncryptionSecret(src)) {
>          if (!(srcPriv->encinfo =
> -              qemuDomainSecretInfoNew(conn, priv, disk->info.alias,
> +              qemuDomainSecretInfoNew(conn, priv, encalias,
>                                        VIR_SECRET_USAGE_TYPE_VOLUME, NULL,
>                                        &src->encryption->secrets[0]->seclookupdef,
>                                        true)))
> @@ -1416,6 +1408,27 @@ qemuDomainSecretDiskPrepare(virConnectPtr conn,
>  }
> 
> 
> +/* qemuDomainSecretDiskPrepare:
> + * @conn: Pointer to connection
> + * @priv: pointer to domain private object
> + * @disk: Pointer to a disk definition
> + *
> + * For the right disk, generate the qemuDomainSecretInfo structure.
> + *
> + * Returns 0 on success, -1 on failure
> + */
> +
> +int
> +qemuDomainSecretDiskPrepare(virConnectPtr conn,
> +                            qemuDomainObjPrivatePtr priv,
> +                            virDomainDiskDefPtr disk)
> +{
> +    return qemuDomainSecretStorageSourcePrepare(conn, priv, disk->src,
> +                                                disk->info.alias,
> +                                                disk->info.alias);
> +}
> +
> +
>  /* qemuDomainSecretHostdevDestroy:
>   * @disk: Pointer to a hostdev definition
>   *
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/12] qemu: domain: Extract setup for disk source secrets
Posted by Peter Krempa 7 years, 6 months ago
On Thu, Oct 26, 2017 at 11:12:08 -0400, John Ferlan wrote:
> 
> 
> On 10/20/2017 09:47 AM, Peter Krempa wrote:
> > Separate it so that it deals only with single virStorageSource, so that
> > it can later be reused for full backing chain support.
> > 
> > Two aliases are passed since authentication is more relevant to the
> > 'storage backend' whereas encryption is more relevant to the protocol
> > layer. When using node names, the aliases will be different.
> > ---
> >  src/qemu/qemu_domain.c | 49 +++++++++++++++++++++++++++++++------------------
> >  1 file changed, 31 insertions(+), 18 deletions(-)
> > 
> 
> FWIW: The @authalias would be the secret for the source to access the
> server (RBD only right now, but the iSCSI patches are on list) while the
> @encalias would be the LUKS secret.
> 
> Does the backing chain allow different "auth" sources for different
> chain members? Mind boggling, but I guess possible.
> 
> I suppose it would also be possible that some member of the chain
> doesn't use the auth, but rather it's a local LUKS encrypted file - is
> that the essential goal?
> 
> I think perhaps it'd help to add some comments for
> qemuDomainSecretStorageSourcePrepare in order to describe the parameters
> and the "expectations" for varying levels of the chain.  There's also
> some "assumptions" built into the hotplug code for at least the top level.

Yes every level can have different authentication data. (Since they can
reside on completely different storage technologies). This is possible
even now, but authentication will not work in that case.

The aliases are separate since in qemu the storage access layer and
format driver level have different node names. Authentication is
relevant to the storage access level, while encryption to the format
driver level.

I've added a comment trying to explain this.


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list