[PATCH 0/9] Make HMP optional - follow-up

[PATCH 0/9] Make HMP optional - follow-up

[Marc-André Lureau]

Hi,

Fix the issues that Peter reported while merging "[GIT PULL 00/50] Make
HMP optional".

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
Marc-André Lureau (9):
      tests/ahci-test: replace HMP usage with QMP
      tests/functional: replace HMP with QMP
      tests/functional/s390x: use QMP for balloon
      tests/qtest: add qtest_qmp_job_wait()
      tests/ide-test: convert to QMP
      tests/drive_del-test: compile out HMP-dependent tests when !CONFIG_HMP
      qtest: compile out HMP helper when !CONFIG_HMP
      tests/functional: add skipTestIfNoHMP() helper
      tests/functional: fix some tests that require HMP

 tests/functional/arm/test_integratorcp.py |  7 ++---
 tests/functional/m68k/test_nextcube.py    |  6 ++--
 tests/functional/mips64el/test_malta.py   |  7 ++---
 tests/functional/ppc/test_ppe42.py        |  1 +
 tests/functional/qemu_test/testcase.py    |  6 ++++
 tests/functional/s390x/test_ccw_virtio.py |  4 +--
 tests/qtest/ahci-test.c                   |  5 ++--
 tests/qtest/drive_del-test.c              | 47 +++++++++++++++----------------
 tests/qtest/ide-test.c                    | 37 ++++++++++++++++++------
 tests/qtest/libqtest.c                    | 39 +++++++++++++++++++++++++
 tests/qtest/libqtest.h                    | 13 +++++++++
 11 files changed, 119 insertions(+), 53 deletions(-)
---
base-commit: ff1d2d19d7e24893e2012d879f8e73077e17b9bd
change-id: 20260906-nohmp-1d29f0c7494d

Best regards,
--  
Marc-André Lureau <marcandre.lureau@redhat.com>

[PATCH 1/9] tests/ahci-test: replace HMP usage with QMP

[Marc-André Lureau]

Use qemu-io command instead.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/qtest/ahci-test.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
index 3406ca884fb4..eb8d23999738 100644
--- a/tests/qtest/ahci-test.c
+++ b/tests/qtest/ahci-test.c
@@ -1900,8 +1900,7 @@ static void test_write_engine_stop_in_flight(void)
     ahci_io(ahci, port, CMD_WRITE_DMA, rx, AHCI_SECTOR_SIZE, 1);
 
     /* Suspend the backend write so the first sector stays in flight. */
-    g_free(qtest_hmp(ahci->parent->qts,
-                     "qemu-io drive0 \"break write_aio wr\""));
+    qtest_qemu_io(ahci->parent->qts, "drive0", "break write_aio wr");
 
     cmd = ahci_command_create(CMD_WRITE_PIO);
     ahci_command_adjust(cmd, 0, ptr, bufsize, 0);
@@ -1911,7 +1910,7 @@ static void test_write_engine_stop_in_flight(void)
     /* Drop the command list while the write is still outstanding. */
     ahci_px_clr(ahci, port, AHCI_PX_CMD, AHCI_PX_CMD_ST);
 
-    g_free(qtest_hmp(ahci->parent->qts, "qemu-io drive0 \"resume wr\""));
+    qtest_qemu_io(ahci->parent->qts, "drive0", "resume wr");
 
     /* Round-trip through the device to confirm qemu is still alive. */
     ahci_px_rreg(ahci, port, AHCI_PX_TFD);

-- 
2.55.0.543.g5ebe2ebe4ea8

[PATCH 2/9] tests/functional: replace HMP with QMP

[Marc-André Lureau]

Replace HMP with QMP equivalent.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/functional/arm/test_integratorcp.py | 7 ++-----
 tests/functional/m68k/test_nextcube.py    | 5 +----
 tests/functional/mips64el/test_malta.py   | 7 ++-----
 3 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/tests/functional/arm/test_integratorcp.py b/tests/functional/arm/test_integratorcp.py
index 23ae919359d5..fefed26266f4 100755
--- a/tests/functional/arm/test_integratorcp.py
+++ b/tests/functional/arm/test_integratorcp.py
@@ -72,11 +72,8 @@ def test_framebuffer_tux_logo(self):
         self.boot_integratorcp()
         framebuffer_ready = 'Console: switching to colour frame buffer device'
         wait_for_console_pattern(self, framebuffer_ready)
-        self.vm.cmd('human-monitor-command', command_line='stop')
-        res = self.vm.cmd('human-monitor-command',
-                          command_line='screendump %s' % screendump_path)
-        if 'unknown command' in res:
-            self.skipTest('screendump not available')
+        self.vm.cmd('stop')
+        self.vm.cmd('screendump', filename=screendump_path)
 
         cpu_count = 1
         match_threshold = 0.92
diff --git a/tests/functional/m68k/test_nextcube.py b/tests/functional/m68k/test_nextcube.py
index d917cf5424fe..dc99962eb75c 100755
--- a/tests/functional/m68k/test_nextcube.py
+++ b/tests/functional/m68k/test_nextcube.py
@@ -39,10 +39,7 @@ def check_bootrom_framebuffer(self, screenshot_path):
                 break
             time.sleep(0.1)
 
-        res = self.vm.cmd('human-monitor-command',
-                          command_line=f"screendump {screenshot_path}")
-        if 'unknown command' in res:
-            self.skipTest('screendump not available')
+        self.vm.cmd('screendump', filename=screenshot_path)
 
     @skipIfMissingImports("PIL")
     def test_bootrom_framebuffer_size(self):
diff --git a/tests/functional/mips64el/test_malta.py b/tests/functional/mips64el/test_malta.py
index 163bbaf5ca36..ba966d1bf9f4 100755
--- a/tests/functional/mips64el/test_malta.py
+++ b/tests/functional/mips64el/test_malta.py
@@ -154,11 +154,8 @@ def do_test_i6400_framebuffer_logo(self, cpu_cores_count):
         self.vm.launch()
         framebuffer_ready = 'Console: switching to colour frame buffer device'
         self.wait_for_console_pattern(framebuffer_ready)
-        self.vm.cmd('human-monitor-command', command_line='stop')
-        res = self.vm.cmd('human-monitor-command',
-                          command_line=f'screendump {screendump_path}')
-        if 'unknown command' in res:
-            self.skipTest('screendump not available')
+        self.vm.cmd('stop')
+        self.vm.cmd('screendump', filename=screendump_path)
 
         match_threshold = 0.95
         screendump_bgr = cv2.imread(screendump_path, cv2.IMREAD_COLOR)

-- 
2.55.0.543.g5ebe2ebe4ea8

[PATCH 3/9] tests/functional/s390x: use QMP for balloon

[Marc-André Lureau]

Unit is changed from megabytes to bytes.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/functional/s390x/test_ccw_virtio.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/functional/s390x/test_ccw_virtio.py b/tests/functional/s390x/test_ccw_virtio.py
index 1d4958bbe293..f816b1e201a1 100755
--- a/tests/functional/s390x/test_ccw_virtio.py
+++ b/tests/functional/s390x/test_ccw_virtio.py
@@ -162,10 +162,10 @@ def test_s390x_devices(self):
         # test the virtio-balloon device
         exec_command_and_wait_for_pattern(self, 'head -n 1 /proc/meminfo',
                                           'MemTotal:         115640 kB')
-        self.vm.cmd('human-monitor-command', command_line='balloon 96')
+        self.vm.cmd('balloon', value=96 * 1024 * 1024)
         exec_command_and_wait_for_pattern(self, 'head -n 1 /proc/meminfo',
                                           'MemTotal:          82872 kB')
-        self.vm.cmd('human-monitor-command', command_line='balloon 128')
+        self.vm.cmd('balloon', value=128 * 1024 * 1024)
         exec_command_and_wait_for_pattern(self, 'head -n 1 /proc/meminfo',
                                           'MemTotal:         115640 kB')
 

-- 
2.55.0.543.g5ebe2ebe4ea8

[PATCH 4/9] tests/qtest: add qtest_qmp_job_wait()

[Marc-André Lureau]

Add a helper to wait for job conclusion, used in next changes.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/qtest/libqtest.c | 37 +++++++++++++++++++++++++++++++++++++
 tests/qtest/libqtest.h | 11 +++++++++++
 2 files changed, 48 insertions(+)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 28bb72bfaecf..370f79a3b6fb 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -984,6 +984,43 @@ void qtest_qmp_eventwait(QTestState *s, const char *event)
     qobject_unref(response);
 }
 
+void qtest_qmp_job_wait(QTestState *s, const char *job_id)
+{
+    QDict *response, *data, *error;
+    QList *jobs;
+    const QListEntry *entry;
+
+    for (;;) {
+        response = qtest_qmp_eventwait_ref(s, "JOB_STATUS_CHANGE");
+        data = qdict_get_qdict(response, "data");
+        if (!strcmp(qdict_get_str(data, "id"), job_id) &&
+            !strcmp(qdict_get_str(data, "status"), "concluded")) {
+            qobject_unref(response);
+            break;
+        }
+        qobject_unref(response);
+    }
+
+    response = qtest_qmp(s, "{ 'execute': 'query-jobs' }");
+    g_assert(qdict_haskey(response, "return"));
+    jobs = qobject_to(QList, qdict_get(response, "return"));
+    g_assert(jobs);
+    QLIST_FOREACH_ENTRY(jobs, entry) {
+        QDict *job = qobject_to(QDict, qlist_entry_obj(entry));
+        if (!strcmp(qdict_get_str(job, "id"), job_id)) {
+            g_assert_null(qdict_get_try_str(job, "error"));
+            break;
+        }
+    }
+    qobject_unref(response);
+
+    response = qtest_qmp(s,
+        "{ 'execute': 'job-dismiss', 'arguments': { 'id': %s } }", job_id);
+    error = qdict_get_qdict(response, "error");
+    g_assert_null(error);
+    qobject_unref(response);
+}
+
 char *qtest_vhmp(QTestState *s, const char *fmt, va_list ap)
 {
     char *cmd;
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index 58491ca53ab7..f06e061d214b 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -398,6 +398,17 @@ QDict *qtest_qmp_eventwait_ref(QTestState *s, const char *event);
  */
 QDict *qtest_qmp_event_ref(QTestState *s, const char *event);
 
+/**
+ * qtest_qmp_job_wait:
+ * @s: #QTestState instance to operate on.
+ * @job_id: job identifier to wait for.
+ *
+ * Wait for a QMP job to reach "concluded" status by consuming
+ * JOB_STATUS_CHANGE events, then dismiss the job.
+ * Asserts that the job completed without error.
+ */
+void qtest_qmp_job_wait(QTestState *s, const char *job_id);
+
 /**
  * qtest_hmp:
  * @s: #QTestState instance to operate on.

-- 
2.55.0.543.g5ebe2ebe4ea8

[PATCH 5/9] tests/ide-test: convert to QMP

[Marc-André Lureau]

The QMP command is explicit, specify both the vmstate and devices.
We use the helper introduced previously to wait for job completion.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/qtest/ide-test.c | 37 ++++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c
index 895fec6d0806..cf109b079982 100644
--- a/tests/qtest/ide-test.c
+++ b/tests/qtest/ide-test.c
@@ -1345,11 +1345,34 @@ static void ide_prepare_markers(QTestState *qts, QPCIDevice *dev,
     ide_write_marker(qts, dev, ide_bar, 63, CHS_MARKER_DEFAULT);
 }
 
-static void ide_hmp_quiet(QTestState *qts, const char *command)
+static void ide_snapshot_save(QTestState *qts, const char *tag,
+                              const char *vmstate, const char *device)
 {
-    g_autofree char *out = qtest_hmp(qts, "%s", command);
+    qtest_qmp_assert_success(qts,
+        "{ 'execute': 'snapshot-save',"
+        "  'arguments': {"
+        "    'job-id': 'save0',"
+        "    'tag': %s,"
+        "    'vmstate': %s,"
+        "    'devices': [%s]"
+        "  }"
+        "}", tag, vmstate, device);
+    qtest_qmp_job_wait(qts, "save0");
+}
 
-    g_assert_cmpstr(out, ==, "");
+static void ide_snapshot_load(QTestState *qts, const char *tag,
+                              const char *vmstate, const char *device)
+{
+    qtest_qmp_assert_success(qts,
+        "{ 'execute': 'snapshot-load',"
+        "  'arguments': {"
+        "    'job-id': 'load0',"
+        "    'tag': %s,"
+        "    'vmstate': %s,"
+        "    'devices': [%s]"
+        "  }"
+        "}", tag, vmstate, device);
+    qtest_qmp_job_wait(qts, "load0");
 }
 
 static char *ide_migration_status(QTestState *qts)
@@ -1455,10 +1478,6 @@ static void test_migrate_chs_snapshot(void)
     char marker[9];
     int fd;
 
-#ifndef CONFIG_HMP
-    g_test_skip("HMP not enabled");
-    return;
-#endif
     if (!have_qemu_img()) {
         g_test_skip("QTEST_QEMU_IMG not set, snapshots need a qcow2 image");
         return;
@@ -1480,13 +1499,13 @@ static void test_migrate_chs_snapshot(void)
     /* Snapshot taken while the default translation is in effect */
     ide_read_chs_marker(qts, dev, ide_bar, 0, 1, 1, marker);
     g_assert_cmpstr(marker, ==, CHS_MARKER_DEFAULT);
-    ide_hmp_quiet(qts, "savevm s0");
+    ide_snapshot_save(qts, "s0", "hda", "hda");
 
     ide_set_translation(dev, ide_bar, 8, 32);
     ide_read_chs_marker(qts, dev, ide_bar, 0, 1, 1, marker);
     g_assert_cmpstr(marker, ==, CHS_MARKER_CUSTOM);
 
-    ide_hmp_quiet(qts, "loadvm s0");
+    ide_snapshot_load(qts, "s0", "hda", "hda");
 
     ide_read_chs_marker(qts, dev, ide_bar, 0, 1, 1, marker);
     g_assert_cmpstr(marker, ==, CHS_MARKER_DEFAULT);

-- 
2.55.0.543.g5ebe2ebe4ea8

[PATCH 6/9] tests/drive_del-test: compile out HMP-dependent tests when !CONFIG_HMP

[Marc-André Lureau]

The compiled-out functions are actually testing HMP-specific commands
and behaviour which operate on the old "drive" concept. The QMP
equivalent for block-driver nodes are already tested.

If HMP goes away, those tests should go away too. Compile them out.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/qtest/drive_del-test.c | 47 +++++++++++++++++++++-----------------------
 1 file changed, 22 insertions(+), 25 deletions(-)

diff --git a/tests/qtest/drive_del-test.c b/tests/qtest/drive_del-test.c
index cc52c2d87936..cbf94d5b9f0f 100644
--- a/tests/qtest/drive_del-test.c
+++ b/tests/qtest/drive_del-test.c
@@ -87,6 +87,7 @@ static void blockdev_add_with_media(QTestState *qts)
     g_assert(has_blockdev(qts));
 }
 
+#ifdef CONFIG_HMP
 static void drive_add(QTestState *qts)
 {
     char *resp = qtest_hmp(qts, "drive_add 0 if=none,id=drive0");
@@ -117,6 +118,7 @@ static void drive_del(QTestState *qts)
     g_assert(!has_drive(qts));
     g_free(resp);
 }
+#endif
 
 /*
  * qvirtio_get_dev_type:
@@ -165,13 +167,9 @@ static void device_del(QTestState *qts, bool and_reset)
 
 static void test_drive_without_dev(void)
 {
+#ifdef CONFIG_HMP
     QTestState *qts;
 
-#ifndef CONFIG_HMP
-    g_test_skip("HMP not enabled");
-    return;
-#endif
-
     /* Start with an empty drive */
     qts = qtest_init("-drive if=none,id=drive0 -M none");
 
@@ -184,19 +182,18 @@ static void test_drive_without_dev(void)
     drive_add(qts);
 
     qtest_quit(qts);
+#else
+    g_test_skip("HMP not enabled");
+#endif
 }
 
 static void test_after_failed_device_add(void)
 {
+#ifdef CONFIG_HMP
     char driver[32];
     QDict *response;
     QTestState *qts;
 
-#ifndef CONFIG_HMP
-    g_test_skip("HMP not enabled");
-    return;
-#endif
-
     if (!has_device_builtin("virtio-blk")) {
         g_test_skip("Device virtio-blk is not available");
         return;
@@ -227,17 +224,16 @@ static void test_after_failed_device_add(void)
     drive_add(qts);
 
     qtest_quit(qts);
+#else
+    g_test_skip("HMP not enabled");
+#endif
 }
 
 static void test_drive_del_device_del(void)
 {
+#ifdef CONFIG_HMP
     QTestState *qts;
 
-#ifndef CONFIG_HMP
-    g_test_skip("HMP not enabled");
-    return;
-#endif
-
     if (!has_device_builtin("virtio-scsi")) {
         g_test_skip("Device virtio-scsi is not available");
         return;
@@ -259,6 +255,9 @@ static void test_drive_del_device_del(void)
     g_assert(!has_drive(qts));
 
     qtest_quit(qts);
+#else
+    g_test_skip("HMP not enabled");
+#endif
 }
 
 static void test_cli_device_del(void)
@@ -416,15 +415,11 @@ static void test_device_add_and_del_q35(void)
 
 static void test_drive_add_device_add_and_del(void)
 {
+#ifdef CONFIG_HMP
     QTestState *qts;
     const char *arch = qtest_get_arch();
     const char *machine_addition = "";
 
-#ifndef CONFIG_HMP
-    g_test_skip("HMP not enabled");
-    return;
-#endif
-
     if (!has_device_builtin("virtio-blk")) {
         g_test_skip("Device virtio-blk is not available");
         return;
@@ -450,17 +445,16 @@ static void test_drive_add_device_add_and_del(void)
     g_assert(!has_drive(qts));
 
     qtest_quit(qts);
+#else
+    g_test_skip("HMP not enabled");
+#endif
 }
 
 static void test_drive_add_device_add_and_del_q35(void)
 {
+#ifdef CONFIG_HMP
     QTestState *qts;
 
-#ifndef CONFIG_HMP
-    g_test_skip("HMP not enabled");
-    return;
-#endif
-
     if (!has_device_builtin("virtio-blk")) {
         g_test_skip("Device virtio-blk is not available");
         return;
@@ -479,6 +473,9 @@ static void test_drive_add_device_add_and_del_q35(void)
     g_assert(!has_drive(qts));
 
     qtest_quit(qts);
+#else
+    g_test_skip("HMP not enabled");
+#endif
 }
 
 static void test_blockdev_add_device_add_and_del(void)

-- 
2.55.0.543.g5ebe2ebe4ea8

[PATCH 7/9] qtest: compile out HMP helper when !CONFIG_HMP

[Marc-André Lureau]

This should help prevent introducing HMP tests inadvertently.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/qtest/libqtest.c | 2 ++
 tests/qtest/libqtest.h | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 370f79a3b6fb..05d884761540 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1021,6 +1021,7 @@ void qtest_qmp_job_wait(QTestState *s, const char *job_id)
     qobject_unref(response);
 }
 
+#ifdef CONFIG_HMP
 char *qtest_vhmp(QTestState *s, const char *fmt, va_list ap)
 {
     char *cmd;
@@ -1048,6 +1049,7 @@ char *qtest_hmp(QTestState *s, const char *fmt, ...)
     va_end(ap);
     return ret;
 }
+#endif
 
 void qtest_qemu_io(QTestState *s, const char *device,
                    const char *fmt, ...)
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index f06e061d214b..20982d3fe8dd 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -409,6 +409,7 @@ QDict *qtest_qmp_event_ref(QTestState *s, const char *event);
  */
 void qtest_qmp_job_wait(QTestState *s, const char *job_id);
 
+#ifdef CONFIG_HMP
 /**
  * qtest_hmp:
  * @s: #QTestState instance to operate on.
@@ -434,6 +435,7 @@ char *qtest_hmp(QTestState *s, const char *fmt, ...) G_GNUC_PRINTF(2, 3);
  */
 char *qtest_vhmp(QTestState *s, const char *fmt, va_list ap)
     G_GNUC_PRINTF(2, 0);
+#endif
 
 /**
  * qtest_qemu_io:

-- 
2.55.0.543.g5ebe2ebe4ea8

[PATCH 8/9] tests/functional: add skipTestIfNoHMP() helper

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/functional/qemu_test/testcase.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
index 69d3d06cc0e3..bf2dc1dc101c 100644
--- a/tests/functional/qemu_test/testcase.py
+++ b/tests/functional/qemu_test/testcase.py
@@ -369,6 +369,12 @@ def require_accelerator(self, accelerator):
             self.skipTest("%s accelerator does not seem to be "
                           "available" % accelerator)
 
+    def skipTestIfNoHMP(self):
+        commands = self.vm.cmd('query-commands')
+        if not any(cmd['name'] == 'human-monitor-command'
+                   for cmd in commands):
+            self.skipTest('HMP support is not available')
+
     def require_netdev(self, netdevname):
         helptxt = run([self.qemu_bin, '-M', 'none', '-netdev', 'help'],
                       capture_output=True, check=True, encoding='utf8').stdout

-- 
2.55.0.543.g5ebe2ebe4ea8

[PATCH 9/9] tests/functional: fix some tests that require HMP

[Marc-André Lureau]

Those tests use "info registers" command and connot be converted
easily, for now skip them

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/functional/m68k/test_nextcube.py | 1 +
 tests/functional/ppc/test_ppe42.py     | 1 +
 2 files changed, 2 insertions(+)

diff --git a/tests/functional/m68k/test_nextcube.py b/tests/functional/m68k/test_nextcube.py
index dc99962eb75c..b0c0b653a7d0 100755
--- a/tests/functional/m68k/test_nextcube.py
+++ b/tests/functional/m68k/test_nextcube.py
@@ -27,6 +27,7 @@ def check_bootrom_framebuffer(self, screenshot_path):
 
         self.vm.add_args('-bios', rom_path)
         self.vm.launch()
+        self.skipTestIfNoHMP() # FIXME: QMP x-query-registers
 
         self.log.info('VM launched, waiting for display')
         # Wait for the FPU test to finish, then the display is available, too:
diff --git a/tests/functional/ppc/test_ppe42.py b/tests/functional/ppc/test_ppe42.py
index 53958a7938d1..a9c154f67328 100755
--- a/tests/functional/ppc/test_ppe42.py
+++ b/tests/functional/ppc/test_ppe42.py
@@ -72,6 +72,7 @@ def test_ppe42_instructions(self):
         self.vm.add_args('-device', 'loader,addr=0xfff80040,cpu-num=0')
         self.vm.add_args('-action', 'panic=pause')
         self.vm.launch()
+        self.skipTestIfNoHMP() # FIXME: QMP x-query-registers
         self._wait_pass_fail(self.timeout)
 
 if __name__ == '__main__':

-- 
2.55.0.543.g5ebe2ebe4ea8

Re: [PATCH 2/9] tests/functional: replace HMP with QMP

[Thomas Huth]

Am Sun, 06 Sep 2026 12:42:33 +0400
schrieb Marc-André Lureau <marcandre.lureau@redhat.com>:

> Replace HMP with QMP equivalent.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/functional/arm/test_integratorcp.py | 7 ++-----
>  tests/functional/m68k/test_nextcube.py    | 5 +----
>  tests/functional/mips64el/test_malta.py   | 7 ++-----
>  3 files changed, 5 insertions(+), 14 deletions(-)
> 
> diff --git a/tests/functional/arm/test_integratorcp.py b/tests/functional/arm/test_integratorcp.py
> index 23ae919359d5..fefed26266f4 100755
> --- a/tests/functional/arm/test_integratorcp.py
> +++ b/tests/functional/arm/test_integratorcp.py
> @@ -72,11 +72,8 @@ def test_framebuffer_tux_logo(self):
>          self.boot_integratorcp()
>          framebuffer_ready = 'Console: switching to colour frame buffer device'
>          wait_for_console_pattern(self, framebuffer_ready)
> -        self.vm.cmd('human-monitor-command', command_line='stop')
> -        res = self.vm.cmd('human-monitor-command',
> -                          command_line='screendump %s' % screendump_path)
> -        if 'unknown command' in res:
> -            self.skipTest('screendump not available')
> +        self.vm.cmd('stop')
> +        self.vm.cmd('screendump', filename=screendump_path)

screendump is an optional command - it is only available if CONFIG_PIXMAN
is enabled. That's why there was a check for "unknown command" here.
Does your new code still works if pixman is disabled?

 Thomas

Re: [PATCH 3/9] tests/functional/s390x: use QMP for balloon

[Thomas Huth]

Am Sun, 06 Sep 2026 12:42:34 +0400
schrieb Marc-André Lureau <marcandre.lureau@redhat.com>:

> Unit is changed from megabytes to bytes.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/functional/s390x/test_ccw_virtio.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/functional/s390x/test_ccw_virtio.py b/tests/functional/s390x/test_ccw_virtio.py
> index 1d4958bbe293..f816b1e201a1 100755
> --- a/tests/functional/s390x/test_ccw_virtio.py
> +++ b/tests/functional/s390x/test_ccw_virtio.py
> @@ -162,10 +162,10 @@ def test_s390x_devices(self):
>          # test the virtio-balloon device
>          exec_command_and_wait_for_pattern(self, 'head -n 1 /proc/meminfo',
>                                            'MemTotal:         115640 kB')
> -        self.vm.cmd('human-monitor-command', command_line='balloon 96')
> +        self.vm.cmd('balloon', value=96 * 1024 * 1024)
>          exec_command_and_wait_for_pattern(self, 'head -n 1 /proc/meminfo',
>                                            'MemTotal:          82872 kB')
> -        self.vm.cmd('human-monitor-command', command_line='balloon 128')
> +        self.vm.cmd('balloon', value=128 * 1024 * 1024)
>          exec_command_and_wait_for_pattern(self, 'head -n 1 /proc/meminfo',
>                                            'MemTotal:         115640 kB')
>  
> 

Reviewed-by: Thomas Huth <th.huth+qemu@posteo.eu>

Re: [PATCH 8/9] tests/functional: add skipTestIfNoHMP() helper

[Thomas Huth]

Am Sun, 06 Sep 2026 12:42:39 +0400
schrieb Marc-André Lureau <marcandre.lureau@redhat.com>:

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/functional/qemu_test/testcase.py | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
> index 69d3d06cc0e3..bf2dc1dc101c 100644
> --- a/tests/functional/qemu_test/testcase.py
> +++ b/tests/functional/qemu_test/testcase.py
> @@ -369,6 +369,12 @@ def require_accelerator(self, accelerator):
>              self.skipTest("%s accelerator does not seem to be "
>                            "available" % accelerator)
>  
> +    def skipTestIfNoHMP(self):
> +        commands = self.vm.cmd('query-commands')
> +        if not any(cmd['name'] == 'human-monitor-command'
> +                   for cmd in commands):
> +            self.skipTest('HMP support is not available')
> +
>      def require_netdev(self, netdevname):
>          helptxt = run([self.qemu_bin, '-M', 'none', '-netdev', 'help'],
>                        capture_output=True, check=True, encoding='utf8').stdout
> 

Reviewed-by: Thomas Huth <th.huth+qemu@posteo.eu>

Re: [PATCH 9/9] tests/functional: fix some tests that require HMP

[Thomas Huth]

Am Sun, 06 Sep 2026 12:42:40 +0400
schrieb Marc-André Lureau <marcandre.lureau@redhat.com>:

> Those tests use "info registers" command and connot be converted

s/connot/cannot/

> easily, for now skip them
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/functional/m68k/test_nextcube.py | 1 +
>  tests/functional/ppc/test_ppe42.py     | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/tests/functional/m68k/test_nextcube.py b/tests/functional/m68k/test_nextcube.py
> index dc99962eb75c..b0c0b653a7d0 100755
> --- a/tests/functional/m68k/test_nextcube.py
> +++ b/tests/functional/m68k/test_nextcube.py
> @@ -27,6 +27,7 @@ def check_bootrom_framebuffer(self, screenshot_path):
>  
>          self.vm.add_args('-bios', rom_path)
>          self.vm.launch()
> +        self.skipTestIfNoHMP() # FIXME: QMP x-query-registers
>  
>          self.log.info('VM launched, waiting for display')
>          # Wait for the FPU test to finish, then the display is available, too:
> diff --git a/tests/functional/ppc/test_ppe42.py b/tests/functional/ppc/test_ppe42.py
> index 53958a7938d1..a9c154f67328 100755
> --- a/tests/functional/ppc/test_ppe42.py
> +++ b/tests/functional/ppc/test_ppe42.py
> @@ -72,6 +72,7 @@ def test_ppe42_instructions(self):
>          self.vm.add_args('-device', 'loader,addr=0xfff80040,cpu-num=0')
>          self.vm.add_args('-action', 'panic=pause')
>          self.vm.launch()
> +        self.skipTestIfNoHMP() # FIXME: QMP x-query-registers
>          self._wait_pass_fail(self.timeout)

Why don't you use these as a decorator (with @) instead?

 Thomas

Re: [PATCH 1/9] tests/ahci-test: replace HMP usage with QMP

[Daniel P. Berrangé]

On Sun, Sep 06, 2026 at 12:42:32PM +0400, Marc-André Lureau wrote:
> Use qemu-io command instead.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/qtest/ahci-test.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Re: [PATCH 3/9] tests/functional/s390x: use QMP for balloon

[Daniel P. Berrangé]

On Sun, Sep 06, 2026 at 12:42:34PM +0400, Marc-André Lureau wrote:
> Unit is changed from megabytes to bytes.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/functional/s390x/test_ccw_virtio.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Re: [PATCH 5/9] tests/ide-test: convert to QMP

[Daniel P. Berrangé]

On Sun, Sep 06, 2026 at 12:42:36PM +0400, Marc-André Lureau wrote:
> The QMP command is explicit, specify both the vmstate and devices.
> We use the helper introduced previously to wait for job completion.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/qtest/ide-test.c | 37 ++++++++++++++++++++++++++++---------
>  1 file changed, 28 insertions(+), 9 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Re: [PATCH 6/9] tests/drive_del-test: compile out HMP-dependent tests when !CONFIG_HMP

[Daniel P. Berrangé]

On Sun, Sep 06, 2026 at 12:42:37PM +0400, Marc-André Lureau wrote:
> The compiled-out functions are actually testing HMP-specific commands
> and behaviour which operate on the old "drive" concept. The QMP
> equivalent for block-driver nodes are already tested.
> 
> If HMP goes away, those tests should go away too. Compile them out.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/qtest/drive_del-test.c | 47 +++++++++++++++++++++-----------------------
>  1 file changed, 22 insertions(+), 25 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Re: [PATCH 7/9] qtest: compile out HMP helper when !CONFIG_HMP

[Daniel P. Berrangé]

On Sun, Sep 06, 2026 at 12:42:38PM +0400, Marc-André Lureau wrote:
> This should help prevent introducing HMP tests inadvertently.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/qtest/libqtest.c | 2 ++
>  tests/qtest/libqtest.h | 2 ++
>  2 files changed, 4 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Re: [PATCH 9/9] tests/functional: fix some tests that require HMP

[Daniel P. Berrangé]

On Sun, Sep 06, 2026 at 12:42:40PM +0400, Marc-André Lureau wrote:
> Those tests use "info registers" command and connot be converted
> easily, for now skip them
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/functional/m68k/test_nextcube.py | 1 +
>  tests/functional/ppc/test_ppe42.py     | 1 +
>  2 files changed, 2 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Re: [PATCH 4/9] tests/qtest: add qtest_qmp_job_wait()

[Daniel P. Berrangé]

On Sun, Sep 06, 2026 at 12:42:35PM +0400, Marc-André Lureau wrote:
> Add a helper to wait for job conclusion, used in next changes.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/qtest/libqtest.c | 37 +++++++++++++++++++++++++++++++++++++
>  tests/qtest/libqtest.h | 11 +++++++++++
>  2 files changed, 48 insertions(+)
> 
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 28bb72bfaecf..370f79a3b6fb 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -984,6 +984,43 @@ void qtest_qmp_eventwait(QTestState *s, const char *event)
>      qobject_unref(response);
>  }
>  
> +void qtest_qmp_job_wait(QTestState *s, const char *job_id)
> +{
> +    QDict *response, *data, *error;
> +    QList *jobs;
> +    const QListEntry *entry;
> +
> +    for (;;) {
> +        response = qtest_qmp_eventwait_ref(s, "JOB_STATUS_CHANGE");
> +        data = qdict_get_qdict(response, "data");
> +        if (!strcmp(qdict_get_str(data, "id"), job_id) &&
> +            !strcmp(qdict_get_str(data, "status"), "concluded")) {



> +            qobject_unref(response);
> +            break;
> +        }
> +        qobject_unref(response);
> +    }
> +
> +    response = qtest_qmp(s, "{ 'execute': 'query-jobs' }");
> +    g_assert(qdict_haskey(response, "return"));
> +    jobs = qobject_to(QList, qdict_get(response, "return"));
> +    g_assert(jobs);
> +    QLIST_FOREACH_ENTRY(jobs, entry) {
> +        QDict *job = qobject_to(QDict, qlist_entry_obj(entry));
> +        if (!strcmp(qdict_get_str(job, "id"), job_id)) {
> +            g_assert_null(qdict_get_try_str(job, "error"));

If "job" did contain "error", then wouldn't we have hit
a SEGV in the qdict_get_str() call due to missing 'id'
field.

IOW, we should likely validate this is NOT an error
response first.


> +            break;
> +        }
> +    }
> +    qobject_unref(response);
> +
> +    response = qtest_qmp(s,
> +        "{ 'execute': 'job-dismiss', 'arguments': { 'id': %s } }", job_id);
> +    error = qdict_get_qdict(response, "error");
> +    g_assert_null(error);
> +    qobject_unref(response);
> +}
> +

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Re: [PATCH 4/9] tests/qtest: add qtest_qmp_job_wait()

[Marc-André Lureau]

Hi

On Mon, Sep 7, 2026 at 1:51 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Sun, Sep 06, 2026 at 12:42:35PM +0400, Marc-André Lureau wrote:
> > Add a helper to wait for job conclusion, used in next changes.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  tests/qtest/libqtest.c | 37 +++++++++++++++++++++++++++++++++++++
> >  tests/qtest/libqtest.h | 11 +++++++++++
> >  2 files changed, 48 insertions(+)
> >
> > diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> > index 28bb72bfaecf..370f79a3b6fb 100644
> > --- a/tests/qtest/libqtest.c
> > +++ b/tests/qtest/libqtest.c
> > @@ -984,6 +984,43 @@ void qtest_qmp_eventwait(QTestState *s, const char *event)
> >      qobject_unref(response);
> >  }
> >
> > +void qtest_qmp_job_wait(QTestState *s, const char *job_id)
> > +{
> > +    QDict *response, *data, *error;
> > +    QList *jobs;
> > +    const QListEntry *entry;
> > +
> > +    for (;;) {
> > +        response = qtest_qmp_eventwait_ref(s, "JOB_STATUS_CHANGE");
> > +        data = qdict_get_qdict(response, "data");
> > +        if (!strcmp(qdict_get_str(data, "id"), job_id) &&
> > +            !strcmp(qdict_get_str(data, "status"), "concluded")) {
>
>
>
> > +            qobject_unref(response);
> > +            break;
> > +        }
> > +        qobject_unref(response);
> > +    }
> > +
> > +    response = qtest_qmp(s, "{ 'execute': 'query-jobs' }");
> > +    g_assert(qdict_haskey(response, "return"));
> > +    jobs = qobject_to(QList, qdict_get(response, "return"));
> > +    g_assert(jobs);
> > +    QLIST_FOREACH_ENTRY(jobs, entry) {
> > +        QDict *job = qobject_to(QDict, qlist_entry_obj(entry));
> > +        if (!strcmp(qdict_get_str(job, "id"), job_id)) {
> > +            g_assert_null(qdict_get_try_str(job, "error"));
>
> If "job" did contain "error", then wouldn't we have hit
> a SEGV in the qdict_get_str() call due to missing 'id'
> field.

query-jobs returns [JobInfo], where "id" is mandary and "error" optional.

>
> IOW, we should likely validate this is NOT an error
> response first.

that's g_assert(qdict_haskey(response, "return"))

>
>
> > +            break;
> > +        }
> > +    }
> > +    qobject_unref(response);
> > +
> > +    response = qtest_qmp(s,
> > +        "{ 'execute': 'job-dismiss', 'arguments': { 'id': %s } }", job_id);
> > +    error = qdict_get_qdict(response, "error");
> > +    g_assert_null(error);
> > +    qobject_unref(response);
> > +}
> > +
>
> With regards,
> Daniel
> --
> |: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
> |: https://libvirt.org          ~~          https://entangle-photo.org :|
> |: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|
>

Re: [PATCH 2/9] tests/functional: replace HMP with QMP

[Marc-André Lureau]

Hi

On Sun, Sep 6, 2026 at 3:08 PM Thomas Huth <th.huth+qemu@posteo.eu> wrote:
>
> Am Sun, 06 Sep 2026 12:42:33 +0400
> schrieb Marc-André Lureau <marcandre.lureau@redhat.com>:
>
> > Replace HMP with QMP equivalent.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  tests/functional/arm/test_integratorcp.py | 7 ++-----
> >  tests/functional/m68k/test_nextcube.py    | 5 +----
> >  tests/functional/mips64el/test_malta.py   | 7 ++-----
> >  3 files changed, 5 insertions(+), 14 deletions(-)
> >
> > diff --git a/tests/functional/arm/test_integratorcp.py b/tests/functional/arm/test_integratorcp.py
> > index 23ae919359d5..fefed26266f4 100755
> > --- a/tests/functional/arm/test_integratorcp.py
> > +++ b/tests/functional/arm/test_integratorcp.py
> > @@ -72,11 +72,8 @@ def test_framebuffer_tux_logo(self):
> >          self.boot_integratorcp()
> >          framebuffer_ready = 'Console: switching to colour frame buffer device'
> >          wait_for_console_pattern(self, framebuffer_ready)
> > -        self.vm.cmd('human-monitor-command', command_line='stop')
> > -        res = self.vm.cmd('human-monitor-command',
> > -                          command_line='screendump %s' % screendump_path)
> > -        if 'unknown command' in res:
> > -            self.skipTest('screendump not available')
> > +        self.vm.cmd('stop')
> > +        self.vm.cmd('screendump', filename=screendump_path)
>
> screendump is an optional command - it is only available if CONFIG_PIXMAN
> is enabled. That's why there was a check for "unknown command" here.
> Does your new code still works if pixman is disabled?

My bad, let's skip the test when PIXMAN is not available. See upcoming v2.
thanks

Re: [PATCH 4/9] tests/qtest: add qtest_qmp_job_wait()

[Daniel P. Berrangé]

On Mon, Sep 07, 2026 at 02:13:18PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Sep 7, 2026 at 1:51 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Sun, Sep 06, 2026 at 12:42:35PM +0400, Marc-André Lureau wrote:
> > > Add a helper to wait for job conclusion, used in next changes.
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > >  tests/qtest/libqtest.c | 37 +++++++++++++++++++++++++++++++++++++
> > >  tests/qtest/libqtest.h | 11 +++++++++++
> > >  2 files changed, 48 insertions(+)
> > >
> > > diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> > > index 28bb72bfaecf..370f79a3b6fb 100644
> > > --- a/tests/qtest/libqtest.c
> > > +++ b/tests/qtest/libqtest.c
> > > @@ -984,6 +984,43 @@ void qtest_qmp_eventwait(QTestState *s, const char *event)
> > >      qobject_unref(response);
> > >  }
> > >
> > > +void qtest_qmp_job_wait(QTestState *s, const char *job_id)
> > > +{
> > > +    QDict *response, *data, *error;
> > > +    QList *jobs;
> > > +    const QListEntry *entry;
> > > +
> > > +    for (;;) {
> > > +        response = qtest_qmp_eventwait_ref(s, "JOB_STATUS_CHANGE");
> > > +        data = qdict_get_qdict(response, "data");
> > > +        if (!strcmp(qdict_get_str(data, "id"), job_id) &&
> > > +            !strcmp(qdict_get_str(data, "status"), "concluded")) {
> >
> >
> >
> > > +            qobject_unref(response);
> > > +            break;
> > > +        }
> > > +        qobject_unref(response);
> > > +    }
> > > +
> > > +    response = qtest_qmp(s, "{ 'execute': 'query-jobs' }");
> > > +    g_assert(qdict_haskey(response, "return"));
> > > +    jobs = qobject_to(QList, qdict_get(response, "return"));
> > > +    g_assert(jobs);
> > > +    QLIST_FOREACH_ENTRY(jobs, entry) {
> > > +        QDict *job = qobject_to(QDict, qlist_entry_obj(entry));
> > > +        if (!strcmp(qdict_get_str(job, "id"), job_id)) {
> > > +            g_assert_null(qdict_get_try_str(job, "error"));
> >
> > If "job" did contain "error", then wouldn't we have hit
> > a SEGV in the qdict_get_str() call due to missing 'id'
> > field.
> 
> query-jobs returns [JobInfo], where "id" is mandary and "error" optional.
> 
> >
> > IOW, we should likely validate this is NOT an error
> > response first.
> 
> that's g_assert(qdict_haskey(response, "return"))

Ah true, I'm mis-reading things.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|