8:43 a.m.
*** BLURB HERE ***
Michal Prívozník (2):
remote_daemon: Set shutdown callbacks only after init is done
Revert "qemu: Avoid crash in qemuStateShutdownPrepare() and
qemuStateShutdownWait()"
src/qemu/qemu_driver.c | 6 ------
src/remote/remote_daemon.c | 7 ++++---
2 files changed, 4 insertions(+), 9 deletions(-)
--
2.32.0
8:43 a.m.
New subject: [PATCH 1/2] remote_daemon: Set shutdown callbacks only after init is done
The initialization of drivers happens in a separate thread.
However, the main thread continues initialization and sets
shutdown callbacks (virStateShutdownPrepare() and
virStateShutdownWait()) even though the driver init thread is
still running. This is dangerous because if the daemon decides to
quit early (e.g. because SIGINT was delivered) the
shutdownPrepare and shutdownWait callback are called over
partially init drivers.
Set callbacks only after all drivers were initialized.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/218
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2027400
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/remote/remote_daemon.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index de43a54c2e..4e10f3ad23 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -626,6 +626,10 @@ static void daemonRunStateInit(void *opaque)
driversInitialized = true;
+ virNetDaemonSetShutdownCallbacks(dmn,
+ virStateShutdownPrepare,
+ virStateShutdownWait);
+
/* Tie the non-privileged daemons to the session/shutdown lifecycle */
if (!virNetDaemonIsPrivileged(dmn)) {
@@ -1214,9 +1218,6 @@ int main(int argc, char **argv) {
#endif
/* Run event loop. */
- virNetDaemonSetShutdownCallbacks(dmn,
- virStateShutdownPrepare,
- virStateShutdownWait);
virNetDaemonRun(dmn);
ret = 0;
--
2.32.0
6:39 a.m.
New subject: [PATCH 1/2] remote_daemon: Set shutdown callbacks only after init is done
On Thu, Dec 09, 2021 at 15:43:55 +0100, Michal Privoznik wrote:
The initialization of drivers happens in a separate thread.
However, the main thread continues initialization and sets
shutdown callbacks (virStateShutdownPrepare() and
virStateShutdownWait()) even though the driver init thread is
still running. This is dangerous because if the daemon decides to
quit early (e.g. because SIGINT was delivered) the
shutdownPrepare and shutdownWait callback are called over
partially init drivers.
Set callbacks only after all drivers were initialized.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/218
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2027400
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/remote/remote_daemon.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index de43a54c2e..4e10f3ad23 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -626,6 +626,10 @@ static void daemonRunStateInit(void *opaque)
driversInitialized = true;
+ virNetDaemonSetShutdownCallbacks(dmn,
+ virStateShutdownPrepare,
+ virStateShutdownWait);
+
Okay so this placement ensures that the state shutdown code will only
ever be executed if the state was already initialized ...
/* Tie the non-privileged daemons to the session/shutdown
lifecycle */
if (!virNetDaemonIsPrivileged(dmn)) {
@@ -1214,9 +1218,6 @@ int main(int argc, char **argv) {
#endif
/* Run event loop. */
- virNetDaemonSetShutdownCallbacks(dmn,
- virStateShutdownPrepare,
- virStateShutdownWait);
... which wasn't always true here.
virNetDaemonRun(dmn);
ret = 0;
So at this point it's still possible that the daemon will quit without
the callbacks being called, but tat was possible even before.
Based on the above and the fact that I wasn't able to reproduce the
crash:
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
8:43 a.m.
New subject: [PATCH 2/2] Revert "qemu: Avoid crash in qemuStateShutdownPrepare() and qemuStateShutdownWait()"
This reverts commit 69977ff10560a80bcf5bf93f1a3f819a2d1623ca.
After previous commit it's no longer possible that QEMU driver is
not initialized in qemuStateShutdownPrepare() nor
qemuStateShutdownWait().
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8093b8f69b..2b1a34fafb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1034,9 +1034,6 @@ qemuStateStop(void)
static int
qemuStateShutdownPrepare(void)
{
- if (!qemu_driver)
- return 0;
-
virThreadPoolStop(qemu_driver->workerPool);
return 0;
}
@@ -1056,9 +1053,6 @@ qemuDomainObjStopWorkerIter(virDomainObj *vm,
static int
qemuStateShutdownWait(void)
{
- if (!qemu_driver)
- return 0;
-
virDomainObjListForEach(qemu_driver->domains, false,
qemuDomainObjStopWorkerIter, NULL);
virThreadPoolDrain(qemu_driver->workerPool);
--
2.32.0
6:41 a.m.
New subject: [PATCH 2/2] Revert "qemu: Avoid crash in qemuStateShutdownPrepare() and qemuStateShutdownWait()"
On Thu, Dec 09, 2021 at 15:43:56 +0100, Michal Privoznik wrote:
This reverts commit 69977ff10560a80bcf5bf93f1a3f819a2d1623ca.
After previous commit it's no longer possible that QEMU driver is
not initialized in qemuStateShutdownPrepare() nor
qemuStateShutdownWait().
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 6 ------
1 file changed, 6 deletions(-)
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
1123
days inactive
1124
days old
4 comments
2 participants
participants (2)
-
Michal Privoznik -
Peter Krempa