[libvirt] [PATCH 2/2] qemuDomainRemoveRNGDevice: Remove associated chardev too

Michal Privoznik posted 2 patches 7 years, 2 months ago
There is a newer version of this series
[libvirt] [PATCH 2/2] qemuDomainRemoveRNGDevice: Remove associated chardev too
Posted by Michal Privoznik 7 years, 2 months ago
https://bugzilla.redhat.com/show_bug.cgi?id=1656014

An RNG device can consists of more devices than RND device
itself. For instance, in case of EGD there is a chardev that
connects to EGD daemon and feeds the qemu with random data. When
doing RNG device removal we have to remove the associated chardev
as well.

At the same time, we should not do any 'goto cleanup' until
virDomainAuditRNG() is called as it might result in us not
propagating the audit message.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_hotplug.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 5f756b7267..f1526bbd1b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4831,13 +4831,24 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver,
     rc = qemuMonitorDelObject(priv->mon, objAlias);
 
     if (qemuDomainObjExitMonitor(driver, vm) < 0)
-        goto cleanup;
+        rc = -1;
 
-    if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD &&
-        rc == 0 &&
-        qemuDomainDelChardevTLSObjects(driver, vm, rng->source.chardev,
-                                       charAlias) < 0)
-        goto cleanup;
+    if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD) {
+        if (rc == 0 &&
+            qemuDomainDelChardevTLSObjects(driver, vm, rng->source.chardev,
+                                           charAlias) < 0)
+            rc = -1;
+
+        if (rc == 0) {
+            qemuDomainObjEnterMonitor(driver, vm);
+
+            if (qemuMonitorDetachCharDev(priv->mon, charAlias) < 0)
+                rc = -1;
+
+            if (qemuDomainObjExitMonitor(driver, vm) < 0)
+                rc = -1;
+        }
+    }
 
     virDomainAuditRNG(vm, rng, NULL, "detach", rc == 0);
 
-- 
2.19.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemuDomainRemoveRNGDevice: Remove associated chardev too
Posted by Ján Tomko 7 years, 2 months ago
On Tue, Dec 04, 2018 at 02:39:30PM +0100, Michal Privoznik wrote:
>https://bugzilla.redhat.com/show_bug.cgi?id=1656014
>
>An RNG device can consists of more devices than RND device
>itself. For instance, in case of EGD there is a chardev that
>connects to EGD daemon and feeds the qemu with random data. When
>doing RNG device removal we have to remove the associated chardev
>as well.
>
>At the same time,

IOW: should have been a separate commit

>we should not do any 'goto cleanup' until
>virDomainAuditRNG() is called as it might result in us not
>propagating the audit message.

That is not the case for qemuDomainObjExitMonitor.
If it returns -1, the domain is dead already (and we audited that in
qemuProcessStop).

>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> src/qemu/qemu_hotplug.c | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
>diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>index 5f756b7267..f1526bbd1b 100644
>--- a/src/qemu/qemu_hotplug.c
>+++ b/src/qemu/qemu_hotplug.c
>@@ -4831,13 +4831,24 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver,
>     rc = qemuMonitorDelObject(priv->mon, objAlias);
>
>     if (qemuDomainObjExitMonitor(driver, vm) < 0)
>-        goto cleanup;
>+        rc = -1;
>
>-    if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD &&
>-        rc == 0 &&
>-        qemuDomainDelChardevTLSObjects(driver, vm, rng->source.chardev,
>-                                       charAlias) < 0)
>-        goto cleanup;
>+    if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD) {
>+        if (rc == 0 &&
>+            qemuDomainDelChardevTLSObjects(driver, vm, rng->source.chardev,
>+                                           charAlias) < 0)
>+            rc = -1;
>+
>+        if (rc == 0) {
>+            qemuDomainObjEnterMonitor(driver, vm);
>+
>+            if (qemuMonitorDetachCharDev(priv->mon, charAlias) < 0)

This should be done before detaching its TLS Objects.

>+                rc = -1;
>+
>+            if (qemuDomainObjExitMonitor(driver, vm) < 0)
>+                rc = -1;

goto cleanup;

Jano

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