[PATCH v2] tests: Move domainEventState initialization to qemuTestDriverInit

Rayhan Faizel posted 1 patch 3 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20240703214013.7230-1-rayhan.faizel@gmail.com
tests/qemuhotplugtest.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                       |  1 +
tests/testutilsqemu.c                         |  4 ++
6 files changed, 99 insertions(+), 3 deletions(-)
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
[PATCH v2] tests: Move domainEventState initialization to qemuTestDriverInit
Posted by Rayhan Faizel 3 months, 2 weeks ago
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 moves driver->domainEventState initialization from qemuhotplugtest
to qemuTestDriverInit in testutilsqemu (Credit goes to Michal Privoznik as he
had already provided the diff).

An additional test case is added to test dropping of disks with startupPolicy
set as optional.

Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com>
---

As the patch title has completely changed,
v1: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/4476AKGV4MWNSTUR4DAJVZSVOQEINVJK/#4476AKGV4MWNSTUR4DAJVZSVOQEINVJK

[Changes in v2]
- Move domainEventState initialization instead of adding a NULL check

 tests/qemuhotplugtest.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                       |  1 +
 tests/testutilsqemu.c                         |  4 ++
 6 files changed, 99 insertions(+), 3 deletions(-)
 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/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index d935ad58ed..f707121c47 100644
--- a/tests/qemuhotplugtest.c
+++ b/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);
diff --git a/tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.x86_64-latest.args b/tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.x86_64-latest.args
new file mode 100644
index 0000000000..13ddbc1a5d
--- /dev/null
+++ b/tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.x86_64-latest.args
@@ -0,0 +1,33 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' \
+-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=off \
+-accel tcg \
+-cpu qemu64 \
+-m size=219136k \
+-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-boot strict=on \
+-device '{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
+-device '{"driver":"lsi","id":"scsi0","bus":"pci.0","addr":"0x2"}' \
+-audiodev '{"id":"audio1","driver":"none"}' \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.x86_64-latest.xml b/tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.x86_64-latest.xml
new file mode 100644
index 0000000000..27d0639109
--- /dev/null
+++ b/tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.x86_64-latest.xml
@@ -0,0 +1,38 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219100</memory>
+  <currentMemory unit='KiB'>219100</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <cpu mode='custom' match='exact' check='none'>
+    <model fallback='forbid'>qemu64</model>
+  </cpu>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <disk type='volume' device='disk'>
+      <driver name='qemu'/>
+      <source pool='inactive' volume='inactive' startupPolicy='optional'/>
+      <target dev='sda' bus='scsi'/>
+      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+    </disk>
+    <controller type='usb' index='0' model='piix3-uhci'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <controller type='scsi' index='0' model='lsilogic'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+    </controller>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <audio id='1' type='none'/>
+    <memballoon model='none'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.xml b/tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.xml
new file mode 100644
index 0000000000..c6c59978c6
--- /dev/null
+++ b/tests/qemuxmlconfdata/disk-startuppolicy-optional-drop.xml
@@ -0,0 +1,23 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219100</memory>
+  <currentMemory unit='KiB'>219100</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <disk type='volume' device='disk'>
+        <source pool='inactive' volume='inactive' startupPolicy='optional'/>
+        <target dev='sda'/>
+    </disk>
+    <memballoon model='none'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c
index 4a711fceeb..2a495cc892 100644
--- a/tests/qemuxmlconftest.c
+++ b/tests/qemuxmlconftest.c
@@ -2947,6 +2947,7 @@ mymain(void)
     DO_TEST_CAPS_LATEST_FAILURE("disk-network-iscsi-zero-hosts-invalid")
     DO_TEST_CAPS_LATEST_PARSE_ERROR("hostdev-scsi-vhost-rawio-invalid")
     DO_TEST_CAPS_LATEST_PARSE_ERROR("hostdev-scsi-vhost-sgio-invalid")
+    DO_TEST_CAPS_LATEST("disk-startuppolicy-optional-drop")
 
     /* check that all input files were actually used here */
     if (testConfXMLCheck(existingTestCases) < 0)
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index d70850cb5d..ee6cae218a 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -236,6 +236,7 @@ void qemuTestDriverFree(virQEMUDriver *driver)
     virObjectUnref(driver->caps);
     virObjectUnref(driver->config);
     virObjectUnref(driver->securityManager);
+    virObjectUnref(driver->domainEventState);
     g_clear_object(&driver->nbdkitCapsCache);
 
     virCPUDefFree(cpuDefault);
@@ -369,6 +370,9 @@ int qemuTestDriverInit(virQEMUDriver *driver)
     if (!(driver->securityManager = virSecurityManagerNewStack(mgr)))
         goto error;
 
+    if (!(driver->domainEventState = virObjectEventStateNew()))
+        goto error;
+
     qemuTestSetHostCPU(driver, driver->hostarch, NULL);
 
     VIR_FREE(cfg->vncTLSx509certdir);
-- 
2.34.1
Re: [PATCH v2] tests: Move domainEventState initialization to qemuTestDriverInit
Posted by Michal Prívozník 3 months, 2 weeks ago
On 7/3/24 23:40, 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 moves driver->domainEventState initialization from qemuhotplugtest
> to qemuTestDriverInit in testutilsqemu (Credit goes to Michal Privoznik as he
> had already provided the diff).
> 
> An additional test case is added to test dropping of disks with startupPolicy
> set as optional.
> 
> Signed-off-by: Rayhan Faizel <rayhan.faizel@gmail.com>
> ---
> 
> As the patch title has completely changed,
> v1: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/4476AKGV4MWNSTUR4DAJVZSVOQEINVJK/#4476AKGV4MWNSTUR4DAJVZSVOQEINVJK
> 
> [Changes in v2]
> - Move domainEventState initialization instead of adding a NULL check
> 
>  tests/qemuhotplugtest.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                       |  1 +
>  tests/testutilsqemu.c                         |  4 ++
>  6 files changed, 99 insertions(+), 3 deletions(-)
>  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

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and merged.

Michal