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

*** BLURB HERE *** Michal Prívozník (2): qemuBuildRNGBackendChrdevStr: Fix formatting qemuDomainRemoveRNGDevice: Remove associated chardev too src/qemu/qemu_command.c | 6 ++++-- src/qemu/qemu_hotplug.c | 23 +++++++++++++++++------ 2 files changed, 21 insertions(+), 8 deletions(-) -- 2.19.2

The way that the code is currently written makes my eyes hurt. Signed-off-by: Michal Privoznik <mprivozn@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: -- 2.19.2

On Tue, Dec 04, 2018 at 02:39:29PM +0100, Michal Privoznik wrote:
The way that the code is currently written makes my eyes hurt.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

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

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
participants (2)
-
Ján Tomko
-
Michal Privoznik