1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] qemu: Introduce qemuDomainDelChardevTLSObjects

John Ferlan posted 1 patch 6 years, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20171220000137.22529-1-jferlan@redhat.com
src/qemu/qemu_hotplug.c | 107 +++++++++++++++++++++++-------------------------
1 file changed, 52 insertions(+), 55 deletions(-)
[libvirt] [PATCH] qemu: Introduce qemuDomainDelChardevTLSObjects
Posted by John Ferlan 6 years, 4 months ago 
Let's make a comment deletion helper similar to the Add helper
that can be called after the ExitMonitor.

The modify qemuDomainRemoveChrDevice and qemuDomainRemoveRNGDevice
to call the helper instead of inlining the copy and pasted code.

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

 An offshoot of the redirdev hot unplug patches from Chen Hanxiao.
 Let's create a helper that can remove the TLS chardev objects for
 the specific devices that may have used them. 

 src/qemu/qemu_hotplug.c | 107 +++++++++++++++++++++++-------------------------
 1 file changed, 52 insertions(+), 55 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 7de04c85a..85f47bee2 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1562,6 +1562,47 @@ qemuDomainAddChardevTLSObjects(virConnectPtr conn,
 }
 
 
+static int
+qemuDomainDelChardevTLSObjects(virQEMUDriverPtr driver,
+                               virDomainObjPtr vm,
+                               const char *inAlias)
+{
+    int ret = -1;
+    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    char *tlsAlias = NULL;
+    char *secAlias = NULL;
+
+    if (!(tlsAlias = qemuAliasTLSObjFromSrcAlias(inAlias)))
+        goto cleanup;
+
+    /* Best shot at this as the secinfo is destroyed after process launch
+     * and this path does not recreate it. Thus, if the config has the
+     * secret UUID and we have a serial TCP chardev, then formulate a
+     * secAlias which we'll attempt to destroy. */
+    if (cfg->chardevTLSx509secretUUID &&
+        !(secAlias = qemuDomainGetSecretAESAlias(inAlias, false)))
+        goto cleanup;
+
+    qemuDomainObjEnterMonitor(driver, vm);
+
+    ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
+    if (secAlias)
+        ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
+
+    if (qemuDomainObjExitMonitor(driver, vm) < 0)
+        goto cleanup;
+
+    ret = 0;
+
+ cleanup:
+    VIR_FREE(tlsAlias);
+    VIR_FREE(secAlias);
+    virObjectUnref(cfg);
+    return ret;
+}
+
+
 int qemuDomainAttachRedirdevDevice(virConnectPtr conn,
                                    virQEMUDriverPtr driver,
                                    virDomainObjPtr vm,
@@ -4120,10 +4161,7 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver,
                           virDomainChrDefPtr chr)
 {
     virObjectEventPtr event;
-    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
     char *charAlias = NULL;
-    char *tlsAlias = NULL;
-    char *secAlias = NULL;
     qemuDomainObjPrivatePtr priv = vm->privateData;
     int ret = -1;
     int rc;
@@ -4134,34 +4172,18 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver,
     if (!(charAlias = qemuAliasChardevFromDevAlias(chr->info.alias)))
         goto cleanup;
 
-    if (chr->source->type == VIR_DOMAIN_CHR_TYPE_TCP &&
-        chr->source->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES) {
-
-        if (!(tlsAlias = qemuAliasTLSObjFromSrcAlias(charAlias)))
-            goto cleanup;
-
-        /* Best shot at this as the secinfo is destroyed after process launch
-         * and this path does not recreate it. Thus, if the config has the
-         * secret UUID and we have a serial TCP chardev, then formulate a
-         * secAlias which we'll attempt to destroy. */
-        if (cfg->chardevTLSx509secretUUID &&
-            !(secAlias = qemuDomainGetSecretAESAlias(charAlias, false)))
-            goto cleanup;
-    }
-
     qemuDomainObjEnterMonitor(driver, vm);
     rc = qemuMonitorDetachCharDev(priv->mon, charAlias);
 
-    if (rc == 0) {
-        if (tlsAlias)
-            ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
-        if (secAlias)
-            ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
-    }
-
     if (qemuDomainObjExitMonitor(driver, vm) < 0)
         goto cleanup;
 
+    if (chr->source->type == VIR_DOMAIN_CHR_TYPE_TCP &&
+        chr->source->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES &&
+        rc == 0 &&
+        qemuDomainDelChardevTLSObjects(driver, vm, charAlias) < 0)
+        goto cleanup;
+
     virDomainAuditChardev(vm, chr, NULL, "detach", rc == 0);
 
     if (rc < 0)
@@ -4185,9 +4207,6 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver,
 
  cleanup:
     VIR_FREE(charAlias);
-    VIR_FREE(tlsAlias);
-    VIR_FREE(secAlias);
-    virObjectUnref(cfg);
     return ret;
 }
 
@@ -4198,11 +4217,8 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver,
                           virDomainRNGDefPtr rng)
 {
     virObjectEventPtr event;
-    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
     char *charAlias = NULL;
     char *objAlias = NULL;
-    char *tlsAlias = NULL;
-    char *secAlias = NULL;
     qemuDomainObjPrivatePtr priv = vm->privateData;
     ssize_t idx;
     int ret = -1;
@@ -4218,34 +4234,18 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver,
     if (!(charAlias = qemuAliasChardevFromDevAlias(rng->info.alias)))
         goto cleanup;
 
-    if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD) {
-        if (!(tlsAlias = qemuAliasTLSObjFromSrcAlias(charAlias)))
-            goto cleanup;
-
-        /* Best shot at this as the secinfo is destroyed after process launch
-         * and this path does not recreate it. Thus, if the config has the
-         * secret UUID and we have a serial TCP chardev, then formulate a
-         * secAlias which we'll attempt to destroy. */
-        if (cfg->chardevTLSx509secretUUID &&
-            !(secAlias = qemuDomainGetSecretAESAlias(charAlias, false)))
-            goto cleanup;
-    }
-
     qemuDomainObjEnterMonitor(driver, vm);
 
     rc = qemuMonitorDelObject(priv->mon, objAlias);
 
-    if (rc == 0 && rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD) {
-        ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias));
-        if (tlsAlias)
-            ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
-        if (secAlias)
-            ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
-    }
-
     if (qemuDomainObjExitMonitor(driver, vm) < 0)
         goto cleanup;
 
+    if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD &&
+        rc == 0 &&
+        qemuDomainDelChardevTLSObjects(driver, vm, charAlias) < 0)
+        goto cleanup;
+
     virDomainAuditRNG(vm, rng, NULL, "detach", rc == 0);
 
     if (rc < 0)
@@ -4269,9 +4269,6 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver,
  cleanup:
     VIR_FREE(charAlias);
     VIR_FREE(objAlias);
-    VIR_FREE(tlsAlias);
-    VIR_FREE(secAlias);
-    virObjectUnref(cfg);
     return ret;
 }
 
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Introduce qemuDomainDelChardevTLSObjects
Posted by Chen Hanxiao 6 years, 4 months ago 

At 2017-12-20 08:01:37, "John Ferlan" <jferlan@redhat.com> wrote:
>Let's make a comment deletion helper similar to the Add helper
>that can be called after the ExitMonitor.
>
>The modify qemuDomainRemoveChrDevice and qemuDomainRemoveRNGDevice
>to call the helper instead of inlining the copy and pasted code.
>
>Signed-off-by: John Ferlan <jferlan@redhat.com>
>---
>
> An offshoot of the redirdev hot unplug patches from Chen Hanxiao.
> Let's create a helper that can remove the TLS chardev objects for
> the specific devices that may have used them. 
>

Reviewed-by: Chen Hanxiao <chenhanxiao@gmail.com>

Regards,
- Chen

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2/1] qemu: Move TCP and haveTLS checks into qemuDomainDelChardevTLSObjects
Posted by John Ferlan 6 years, 4 months ago 
Similar to qemuDomainAddChardevTLSObjects let's move the chardev
source must be TCP and it has the @haveTLS flag set checks before
trying to delete the TLS objects.

For the Chr device this represents no change; however, for RNG device
this is an additionaly check that was missed in commit id '68808516'.
Before adding the objects, TCP and haveTLS are checked.

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

 Thanks for the review... Of course after re-reading my own patch "in
 the morning" I realized I could/should have taken the next step to check
 the chardev source for TCP and haveTLS being set similar to how the Add
 code does things. So that's what this patch does.


 src/qemu/qemu_hotplug.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 85f47bee2..b79807300 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1565,6 +1565,7 @@ qemuDomainAddChardevTLSObjects(virConnectPtr conn,
 static int
 qemuDomainDelChardevTLSObjects(virQEMUDriverPtr driver,
                                virDomainObjPtr vm,
+                               virDomainChrSourceDefPtr dev,
                                const char *inAlias)
 {
     int ret = -1;
@@ -1573,6 +1574,12 @@ qemuDomainDelChardevTLSObjects(virQEMUDriverPtr driver,
     char *tlsAlias = NULL;
     char *secAlias = NULL;
 
+    if (dev->type != VIR_DOMAIN_CHR_TYPE_TCP ||
+        dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_YES) {
+        ret = 0;
+        goto cleanup;
+    }
+
     if (!(tlsAlias = qemuAliasTLSObjFromSrcAlias(inAlias)))
         goto cleanup;
 
@@ -4178,10 +4185,8 @@ qemuDomainRemoveChrDevice(virQEMUDriverPtr driver,
     if (qemuDomainObjExitMonitor(driver, vm) < 0)
         goto cleanup;
 
-    if (chr->source->type == VIR_DOMAIN_CHR_TYPE_TCP &&
-        chr->source->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES &&
-        rc == 0 &&
-        qemuDomainDelChardevTLSObjects(driver, vm, charAlias) < 0)
+    if (rc == 0 &&
+        qemuDomainDelChardevTLSObjects(driver, vm, chr->source, charAlias) < 0)
         goto cleanup;
 
     virDomainAuditChardev(vm, chr, NULL, "detach", rc == 0);
@@ -4243,7 +4248,8 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver,
 
     if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD &&
         rc == 0 &&
-        qemuDomainDelChardevTLSObjects(driver, vm, charAlias) < 0)
+        qemuDomainDelChardevTLSObjects(driver, vm, rng->source.chardev,
+                                       charAlias) < 0)
         goto cleanup;
 
     virDomainAuditRNG(vm, rng, NULL, "detach", rc == 0);
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2/1] qemu: Move TCP and haveTLS checks into qemuDomainDelChardevTLSObjects
Posted by Chen Hanxiao 6 years, 3 months ago 

At 2017-12-20 19:49:41, "John Ferlan" <jferlan@redhat.com> wrote:
>Similar to qemuDomainAddChardevTLSObjects let's move the chardev
>source must be TCP and it has the @haveTLS flag set checks before
>trying to delete the TLS objects.
>
>For the Chr device this represents no change; however, for RNG device
>this is an additionaly check that was missed in commit id '68808516'.
>Before adding the objects, TCP and haveTLS are checked.
>
>Signed-off-by: John Ferlan <jferlan@redhat.com>
>---
>
> Thanks for the review... Of course after re-reading my own patch "in
> the morning" I realized I could/should have taken the next step to check
> the chardev source for TCP and haveTLS being set similar to how the Add
> code does things. So that's what this patch does.
>

Ahh, more helpful helper.

Reviewed-by: Chen Hanxiao <chenhanxiao@gmail.com>

Regards,
- Chen

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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.