[libvirt] [PATCH 12/38] qemu: domain: Regenerate auth/enc secret aliases when restoring status XML

Peter Krempa posted 38 patches 6 years, 11 months ago
[libvirt] [PATCH 12/38] qemu: domain: Regenerate auth/enc secret aliases when restoring status XML
Posted by Peter Krempa 6 years, 11 months ago
Previously we did not store the aliases but rather re-generated them
when unplug was necessary. This is very cumbersome since the knowledge
when and which alias to use needs to be stored in the hotplug code as
well.

While this patch will not strictly improve this situation since there
still will be two places containing this code it at least will allow to
remove the mess from the disk-unplug code and will prevent introducing
more mess when adding blockdev support.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_domain.c                             | 90 +++++++++++++++++++++-
 .../disk-secinfo-upgrade-out.xml                   | 16 ++++
 2 files changed, 105 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a6494ff5fc..d070c013a1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5838,8 +5838,91 @@ qemuDomainChrDefPostParse(virDomainChrDefPtr chr,
 }


+/**
+ * qemuDomainDeviceDiskDefPostParseRestoreSecAlias:
+ *
+ * Re-generate aliases for objects related to the storage source if they
+ * were not stored in the status XML by an older libvirt.
+ *
+ * Note that qemuCaps should be always present for a status XML.
+ */
+static int
+qemuDomainDeviceDiskDefPostParseRestoreSecAlias(virDomainDiskDefPtr disk,
+                                                virQEMUCapsPtr qemuCaps,
+                                                unsigned int parseFlags)
+{
+    qemuDomainStorageSourcePrivatePtr priv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
+    bool restoreAuthSecret = false;
+    bool restoreEncSecret = false;
+    char *authalias = NULL;
+    char *encalias = NULL;
+    int ret = -1;
+
+    if (!(parseFlags & VIR_DOMAIN_DEF_PARSE_STATUS) ||
+        !qemuCaps ||
+        virStorageSourceIsEmpty(disk->src) ||
+        !virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_SECRET))
+        return 0;
+
+    /* network storage authentication secret */
+    if (disk->src->auth &&
+        (!priv || !priv->secinfo)) {
+
+        /* only RBD and iSCSI (with capability) were supporting authentication
+         * using secret object at the time we did not format the alias into the
+         * status XML */
+        if (virStorageSourceGetActualType(disk->src) == VIR_STORAGE_TYPE_NETWORK &&
+            (disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD ||
+             (disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI &&
+              virQEMUCapsGet(qemuCaps, QEMU_CAPS_ISCSI_PASSWORD_SECRET))))
+            restoreAuthSecret = true;
+    }
+
+    /* disk encryption secret */
+    if (disk->src->encryption &&
+        disk->src->encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS &&
+        (!priv || !priv->encinfo))
+        restoreEncSecret = true;
+
+    if (!restoreAuthSecret && !restoreEncSecret)
+        return 0;
+
+    if (!priv) {
+        if (!(disk->src->privateData = qemuDomainStorageSourcePrivateNew()))
+            return -1;
+
+        priv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src);
+    }
+
+    if (restoreAuthSecret) {
+        if (!(authalias = qemuDomainGetSecretAESAlias(disk->info.alias, false)))
+            goto cleanup;
+
+        if (qemuStorageSourcePrivateDataAssignSecinfo(&priv->secinfo, &authalias) < 0)
+            goto cleanup;
+    }
+
+    if (restoreEncSecret) {
+        if (!(encalias = qemuDomainGetSecretAESAlias(disk->info.alias, true)))
+            goto cleanup;
+
+        if (qemuStorageSourcePrivateDataAssignSecinfo(&priv->encinfo, &encalias) < 0)
+            goto cleanup;
+    }
+
+    ret = 0;
+
+ cleanup:
+    VIR_FREE(authalias);
+    VIR_FREE(encalias);
+    return ret;
+}
+
+
 static int
 qemuDomainDeviceDiskDefPostParse(virDomainDiskDefPtr disk,
+                                 virQEMUCapsPtr qemuCaps,
+                                 unsigned int parseFlags,
                                  virQEMUDriverConfigPtr cfg)
 {
     /* set default disk types and drivers */
@@ -5873,6 +5956,10 @@ qemuDomainDeviceDiskDefPostParse(virDomainDiskDefPtr disk,
             disk->mirror->format = VIR_STORAGE_FILE_RAW;
     }

+    if (qemuDomainDeviceDiskDefPostParseRestoreSecAlias(disk, qemuCaps,
+                                                        parseFlags) < 0)
+        return -1;
+
     return 0;
 }

@@ -5964,7 +6051,8 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
         break;

     case VIR_DOMAIN_DEVICE_DISK:
-        ret = qemuDomainDeviceDiskDefPostParse(dev->data.disk, cfg);
+        ret = qemuDomainDeviceDiskDefPostParse(dev->data.disk, qemuCaps,
+                                               parseFlags, cfg);
         break;

     case VIR_DOMAIN_DEVICE_VIDEO:
diff --git a/tests/qemustatusxml2xmldata/disk-secinfo-upgrade-out.xml b/tests/qemustatusxml2xmldata/disk-secinfo-upgrade-out.xml
index d364fc7644..a554bca99c 100644
--- a/tests/qemustatusxml2xmldata/disk-secinfo-upgrade-out.xml
+++ b/tests/qemustatusxml2xmldata/disk-secinfo-upgrade-out.xml
@@ -317,6 +317,11 @@
           <encryption format='luks'>
             <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/>
           </encryption>
+          <privateData>
+            <objects>
+              <secret type='encryption' alias='virtio-disk1-luks-secret0'/>
+            </objects>
+          </privateData>
         </source>
         <backingStore/>
         <target dev='vdb' bus='virtio'/>
@@ -350,6 +355,12 @@
           <encryption format='luks'>
             <secret type='passphrase' uuid='0a81f5b2-8403-7b23-c8d6-21ccc2f80d6f'/>
           </encryption>
+          <privateData>
+            <objects>
+              <secret type='auth' alias='virtio-disk3-secret0'/>
+              <secret type='encryption' alias='virtio-disk3-luks-secret0'/>
+            </objects>
+          </privateData>
         </source>
         <backingStore/>
         <target dev='vdd' bus='virtio'/>
@@ -381,6 +392,11 @@
           <auth username='testuser-rbd'>
             <secret type='ceph' usage='testuser-rbd-secret'/>
           </auth>
+          <privateData>
+            <objects>
+              <secret type='auth' alias='virtio-disk5-secret0'/>
+            </objects>
+          </privateData>
         </source>
         <backingStore/>
         <target dev='vdf' bus='virtio'/>
-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 12/38] qemu: domain: Regenerate auth/enc secret aliases when restoring status XML
Posted by Ján Tomko 6 years, 11 months ago
On Wed, May 30, 2018 at 02:41:08PM +0200, Peter Krempa wrote:
>Previously we did not store the aliases but rather re-generated them
>when unplug was necessary. This is very cumbersome since the knowledge
>when and which alias to use needs to be stored in the hotplug code as
>well.
>
>While this patch will not strictly improve this situation since there
>still will be two places containing this code it at least will allow to
>remove the mess from the disk-unplug code and will prevent introducing
>more mess when adding blockdev support.
>
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/qemu/qemu_domain.c                             | 90 +++++++++++++++++++++-
> .../disk-secinfo-upgrade-out.xml                   | 16 ++++
> 2 files changed, 105 insertions(+), 1 deletion(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 12/38] qemu: domain: Regenerate auth/enc secret aliases when restoring status XML
Posted by John Ferlan 6 years, 11 months ago

On 05/30/2018 08:41 AM, Peter Krempa wrote:
> Previously we did not store the aliases but rather re-generated them
> when unplug was necessary. This is very cumbersome since the knowledge
> when and which alias to use needs to be stored in the hotplug code as
> well.
> 
> While this patch will not strictly improve this situation since there
> still will be two places containing this code it at least will allow to
> remove the mess from the disk-unplug code and will prevent introducing
> more mess when adding blockdev support.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/qemu/qemu_domain.c                             | 90 +++++++++++++++++++++-
>  .../disk-secinfo-upgrade-out.xml                   | 16 ++++
>  2 files changed, 105 insertions(+), 1 deletion(-)
> 

Just dawned on me - what about hostdev/iscsi?  for alias saving et. al.
Patch 2 uses common API's to set up secrets for hostdev, but what would
store the alias for them?  Or is that in some future patch I haven't
read yet?

John

[...]

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 12/38] qemu: domain: Regenerate auth/enc secret aliases when restoring status XML
Posted by Peter Krempa 6 years, 11 months ago
On Wed, May 30, 2018 at 17:45:28 -0400, John Ferlan wrote:
> 
> 
> On 05/30/2018 08:41 AM, Peter Krempa wrote:
> > Previously we did not store the aliases but rather re-generated them
> > when unplug was necessary. This is very cumbersome since the knowledge
> > when and which alias to use needs to be stored in the hotplug code as
> > well.
> > 
> > While this patch will not strictly improve this situation since there
> > still will be two places containing this code it at least will allow to
> > remove the mess from the disk-unplug code and will prevent introducing
> > more mess when adding blockdev support.
> > 
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> >  src/qemu/qemu_domain.c                             | 90 +++++++++++++++++++++-
> >  .../disk-secinfo-upgrade-out.xml                   | 16 ++++
> >  2 files changed, 105 insertions(+), 1 deletion(-)
> > 
> 
> Just dawned on me - what about hostdev/iscsi?  for alias saving et. al.
> Patch 2 uses common API's to set up secrets for hostdev, but what would
> store the alias for them?  Or is that in some future patch I haven't
> read yet?

Hostdevs don't make sense with full backing chains so fixing that will
not be critical for adding -blockdev support there. I will see whether
that is required when trying to use blockdev for the hostdev.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list