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

[Michal Privoznik]

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 <***@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);

[Michal Privoznik]

The way that the code is currently written makes my eyes hurt.

Signed-off-by: Michal Privoznik <***@redhat.com>
---
src/qemu/qemu_command.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b7a548c68c..32ed83f277 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5763,10 +5763,12 @@ qemuBuildRNGBackendChrdevStr(virLogManagerPtr logManager,
{
unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT |
QEMU_BUILD_CHARDEV_UNIX_FD_PASS;
- if (chardevStdioLogd)
- cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
+
*chr = NULL;

+ if (chardevStdioLogd)
+ cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
+
switch ((virDomainRNGBackend) rng->backend) {
case VIR_DOMAIN_RNG_BACKEND_RANDOM:
case VIR_DOMAIN_RNG_BACKEND_LAST:

[Ján Tomko]

Post by Michal Privoznik
The way that the code is currently written makes my eyes hurt.
---
src/qemu/qemu_command.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

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

Jano

[Ján Tomko]

Post by Michal Privoznik
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

Post by Michal Privoznik
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).

Post by Michal Privoznik
---
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.

Post by Michal Privoznik
+ rc = -1;
+
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ rc = -1;

goto cleanup;

Jano