On 6/20/24 11:21, Rayhan Faizel wrote:
Under the test environment, driver->domainEventState is uninitialized. If a disk gets dropped, it will attempt to queue an event which will cause a segmentation fault. This crash does not occur during normal use.
This patch adds a quick check to ensure driver->domainEventState is not NULL along with a testcase exercising the dropping of disks with startupPolicy set as 'optional'.
Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com> --- src/qemu/qemu_domain.c | 3 +- ...tuppolicy-optional-drop.x86_64-latest.args | 33 ++++++++++++++++ ...rtuppolicy-optional-drop.x86_64-latest.xml | 38 +++++++++++++++++++ .../disk-startuppolicy-optional-drop.xml | 23 +++++++++++ tests/qemuxmlconftest.c | 2 + 5 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.xml
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7ba2ea4a5e..109c5bbd52 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7592,7 +7592,8 @@ qemuDomainCheckRemoveOptionalDisk(virQEMUDriver *driver, virDomainDiskDefFree(disk); }
- virObjectEventStateQueue(driver->domainEventState, event); + if (driver->domainEventState) + virObjectEventStateQueue(driver->domainEventState, event); }
I'd rather see our tests creating this queue. E.g. like this: diff --git i/tests/testutilsqemu.c w/tests/testutilsqemu.c index d70850cb5d..b870657063 100644 --- i/tests/testutilsqemu.c +++ w/tests/testutilsqemu.c @@ -231,6 +231,7 @@ void qemuTestDriverFree(virQEMUDriver *driver) virFileDeleteTree(driver->config->stateDir); virFileDeleteTree(driver->config->configDir); } + virObjectUnref(driver->domainEventState); virObjectUnref(driver->qemuCapsCache); virObjectUnref(driver->xmlopt); virObjectUnref(driver->caps); @@ -343,6 +344,9 @@ int qemuTestDriverInit(virQEMUDriver *driver) cfg->configDir = g_strdup(configdir); + if (!(driver->domainEventState = virObjectEventStateNew())) + goto error; + driver->caps = testQemuCapsInit(); if (!driver->caps) goto error; BTW: this is also what qemuhotplugtest does: diff --git i/tests/qemuhotplugtest.c w/tests/qemuhotplugtest.c index d935ad58ed..f707121c47 100644 --- i/tests/qemuhotplugtest.c +++ w/tests/qemuhotplugtest.c @@ -501,9 +501,6 @@ mymain(void) virEventRegisterDefaultImpl(); - if (!(driver.domainEventState = virObjectEventStateNew())) - return EXIT_FAILURE; - driver.lockManager = virLockManagerPluginNew("nop", "qemu", driver.config->configBaseDir, 0); Michal