[PATCH 0/4] qemuhotplugtest: Fix timeouts
The test takes too long for no good reason. One was caused by the compiler inlining too much and second one was a deliberate decision. Peter Krempa (4): qemuDomainGetUnplugTimeout: Add G_GNUC_NO_INLINE mock:qemuDomainGetUnplugTimeout: Remove unused attribute for '@vm' mock: qemuDomainGetUnplugTimeout: Decrease timeout qemuhotplugtest: detach: Remove commands which are not issued src/qemu/qemu_hotplug.c | 2 +- tests/qemuhotplugmock.c | 6 +++--- tests/qemuhotplugtest.c | 9 +++------ 3 files changed, 7 insertions(+), 10 deletions(-) -- 2.26.0
The function is mocked in qemuhotplugmock.so. Recent clang versions decided to inline it so the mock stopped working resulting in qemuhotplugtest wasting 15 seconds waiting for timeouts. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 14654a17d7..60d0729f1e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -5122,7 +5122,7 @@ qemuDomainResetDeviceRemoval(virDomainObjPtr vm) } -unsigned long long +unsigned long long G_GNUC_NO_INLINE qemuDomainGetUnplugTimeout(virDomainObjPtr vm) { if (qemuDomainIsPSeries(vm->def)) -- 2.26.0
'@vm' is used to use a different timeout for ppc64 guests. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuhotplugmock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemuhotplugmock.c b/tests/qemuhotplugmock.c index 8e5b07788d..8fe7fe7448 100644 --- a/tests/qemuhotplugmock.c +++ b/tests/qemuhotplugmock.c @@ -39,7 +39,7 @@ init_syms(void) } unsigned long long -qemuDomainGetUnplugTimeout(virDomainObjPtr vm G_GNUC_UNUSED) +qemuDomainGetUnplugTimeout(virDomainObjPtr vm) { /* Wait only 100ms for DEVICE_DELETED event. Give a greater * timeout in case of PSeries guest to be consistent with the -- 2.26.0
We always queue the DEVICE_DELETED events before successful return from the command so that tests are reliable. This means we can decrease the unplug timeout as it's guaranteed to be executed in correct order. According to my testing it shaves off ~450ms of test run: real 0m0.721s vs. real 0m0.259s Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuhotplugmock.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qemuhotplugmock.c b/tests/qemuhotplugmock.c index 8fe7fe7448..d2324913cf 100644 --- a/tests/qemuhotplugmock.c +++ b/tests/qemuhotplugmock.c @@ -45,8 +45,8 @@ qemuDomainGetUnplugTimeout(virDomainObjPtr vm) * timeout in case of PSeries guest to be consistent with the * original logic. */ if (qemuDomainIsPSeries(vm->def)) - return 200; - return 100; + return 20; + return 10; } -- 2.26.0
The 'human-monitor-command' equates to the 'drive-del' command issued by the hotplug code on successful detach of a device. This means that it's not issued for failed attempts and thus should not be added to the expected list. Unfortunately our test monitor doesn't ensure that all expected commands were consumed. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuhotplugtest.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index d9244dca44..9a787f9d11 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -715,8 +715,7 @@ mymain(void) "human-monitor-command", HMP("OK\\r\\n"), "device_add", QMP_OK); DO_TEST_DETACH("base-live", "disk-virtio", true, true, - "device_del", QMP_OK, - "human-monitor-command", HMP("")); + "device_del", QMP_OK); DO_TEST_DETACH("base-live", "disk-virtio", false, false, "device_del", QMP_DEVICE_DELETED("virtio-disk4") QMP_OK, "human-monitor-command", HMP("")); @@ -725,8 +724,7 @@ mymain(void) "human-monitor-command", HMP("OK\\r\\n"), "device_add", QMP_OK); DO_TEST_DETACH("base-live", "disk-usb", true, true, - "device_del", QMP_OK, - "human-monitor-command", HMP("")); + "device_del", QMP_OK); DO_TEST_DETACH("base-live", "disk-usb", false, false, "device_del", QMP_DEVICE_DELETED("usb-disk16") QMP_OK, "human-monitor-command", HMP("")); @@ -735,8 +733,7 @@ mymain(void) "human-monitor-command", HMP("OK\\r\\n"), "device_add", QMP_OK); DO_TEST_DETACH("base-live", "disk-scsi", true, true, - "device_del", QMP_OK, - "human-monitor-command", HMP("")); + "device_del", QMP_OK); DO_TEST_DETACH("base-live", "disk-scsi", false, false, "device_del", QMP_DEVICE_DELETED("scsi0-0-0-5") QMP_OK, "human-monitor-command", HMP("")); -- 2.26.0
On 4/23/20 11:40 AM, Peter Krempa wrote:
The test takes too long for no good reason. One was caused by the compiler inlining too much and second one was a deliberate decision.
Peter Krempa (4): qemuDomainGetUnplugTimeout: Add G_GNUC_NO_INLINE mock:qemuDomainGetUnplugTimeout: Remove unused attribute for '@vm' mock: qemuDomainGetUnplugTimeout: Decrease timeout qemuhotplugtest: detach: Remove commands which are not issued
src/qemu/qemu_hotplug.c | 2 +- tests/qemuhotplugmock.c | 6 +++--- tests/qemuhotplugtest.c | 9 +++------ 3 files changed, 7 insertions(+), 10 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Privoznik -
Peter Krempa