qemu-rust.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/17] maintainer updates for feb25 (qtest, gdbstub, plugins) pre-PR
@ 2025-02-07 15:30 Alex Bennée
  2025-02-07 15:30 ` [PATCH v2 01/17] tests/docker: replicate the check-rust-tools-nightly CI job Alex Bennée
                   ` (16 more replies)
  0 siblings, 17 replies; 28+ messages in thread
From: Alex Bennée @ 2025-02-07 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Xu, Alex Bennée, Thomas Huth, Weiwei Li, qemu-arm,
	Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu,
	Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv

Another month and another set of updates from across my maintainer
trees. They include:

  * qtest set/step clock cleanups

It seems the earlier patch to avoid deadlock when shutting down was
overly broad. So in this series after a little clean-up to the qtest
reply code we tighten up some checks for steps that make no sense and
eventually revert the original deadlock avoiding patch.

  * reducing migration test scope depending on accel

Fabino's continued work on the migration tests and avoiding them
taking too much of the overall test execution time. KVM guests still
get the full range and we smoke test TCG guests.

  * gdbstub late attachment

These are Ilya's patches to make it easier to debug heavily
multi-process linux-user invocations. Comes with documentation and
a test case ;-)

  * plugins

Fix a compiler false positive.

I intend to send the PR next week, the following patches still need review:

  tests/qtest: rename qtest_send_prefix and roll-up into qtest_send
  tests/qtest: don't attempt to clock_step while waiting for virtio ISR
  tests/docker: replicate the check-rust-tools-nightly CI job

Alex.

Alex Bennée (7):
  tests/docker: replicate the check-rust-tools-nightly CI job
  tests/qtest: don't attempt to clock_step while waiting for virtio ISR
  tests/qtest: don't step clock at start of npcm7xx periodic IRQ test
  tests/qtest: simplify qtest_process_inbuf
  tests/qtest: rename qtest_send_prefix and roll-up into qtest_send
  tests/qtest: tighten up the checks on clock_step
  Revert "util/timer: avoid deadlock when shutting down"

Fabiano Rosas (2):
  tests/qtest/migration: Add --full option
  tests/qtest/migration: Pick smoke tests

Ilya Leoshkevich (8):
  gdbstub: Allow the %d placeholder in the socket path
  gdbstub: Try unlinking the unix socket before binding
  user: Introduce user/signal.h
  user: Introduce host_interrupt_signal
  osdep: Introduce qemu_kill_thread()
  gdbstub: Allow late attachment
  docs/user: Document the %d placeholder and suspend=n QEMU_GDB features
  tests/tcg: Add late gdbstub attach test

 MAINTAINERS                                |   1 +
 docs/user/main.rst                         |  16 ++-
 bsd-user/signal-common.h                   |   1 -
 include/qemu/osdep.h                       |   9 ++
 include/system/qtest.h                     |   1 -
 include/user/signal.h                      |  25 ++++
 linux-user/signal-common.h                 |   1 -
 tests/qtest/migration/framework.h          |   1 +
 bsd-user/main.c                            |   1 -
 bsd-user/signal.c                          |  13 ++
 gdbstub/user.c                             | 150 ++++++++++++++++-----
 hw/ppc/spapr_rtas.c                        |   1 -
 hw/riscv/riscv_hart.c                      |   1 -
 linux-user/main.c                          |   1 -
 linux-user/signal.c                        |  26 +++-
 linux-user/syscall.c                       |   1 +
 stubs/monitor-fd.c                         |   9 ++
 system/qtest.c                             |  59 +++-----
 tests/qtest/libqos/virtio-pci-modern.c     |   6 +-
 tests/qtest/libqos/virtio-pci.c            |   6 +-
 tests/qtest/migration-test.c               |  25 ++++
 tests/qtest/migration/compression-tests.c  |  15 ++-
 tests/qtest/migration/cpr-tests.c          |   6 +
 tests/qtest/migration/file-tests.c         |  19 ++-
 tests/qtest/migration/misc-tests.c         |  16 ++-
 tests/qtest/migration/postcopy-tests.c     |  14 +-
 tests/qtest/migration/precopy-tests.c      |  23 +++-
 tests/qtest/migration/tls-tests.c          |  14 +-
 tests/qtest/npcm7xx_timer-test.c           |   1 -
 tests/tcg/multiarch/late-attach.c          |  41 ++++++
 util/oslib-posix.c                         |  15 +++
 util/qemu-timer.c                          |  16 +--
 stubs/meson.build                          |   2 +
 tests/docker/Makefile.include              |   3 +
 tests/docker/test-rust                     |  21 +++
 tests/guest-debug/run-test.py              |  15 ++-
 tests/qtest/meson.build                    |  11 +-
 tests/tcg/multiarch/Makefile.target        |   9 +-
 tests/tcg/multiarch/gdbstub/late-attach.py |  28 ++++
 util/meson.build                           |   2 +
 40 files changed, 490 insertions(+), 135 deletions(-)
 create mode 100644 include/user/signal.h
 create mode 100644 stubs/monitor-fd.c
 create mode 100644 tests/tcg/multiarch/late-attach.c
 create mode 100755 tests/docker/test-rust
 create mode 100644 tests/tcg/multiarch/gdbstub/late-attach.py

-- 
2.39.5



^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 01/17] tests/docker: replicate the check-rust-tools-nightly CI job
  2025-02-07 15:30 [PATCH v2 00/17] maintainer updates for feb25 (qtest, gdbstub, plugins) pre-PR Alex Bennée
@ 2025-02-07 15:30 ` Alex Bennée
  2025-02-07 15:30 ` [PATCH v2 02/17] tests/qtest: don't attempt to clock_step while waiting for virtio ISR Alex Bennée
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Alex Bennée @ 2025-02-07 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Xu, Alex Bennée, Thomas Huth, Weiwei Li, qemu-arm,
	Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu,
	Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv

This allows people to run the test locally:

  make docker-test-rust@fedora-rust-nightly

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v2
  - update MAINTAINERS
---
 MAINTAINERS                   |  1 +
 tests/docker/Makefile.include |  3 +++
 tests/docker/test-rust        | 21 +++++++++++++++++++++
 3 files changed, 25 insertions(+)
 create mode 100755 tests/docker/test-rust

diff --git a/MAINTAINERS b/MAINTAINERS
index 0cf37fce7b..efb7d09dea 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3386,6 +3386,7 @@ F: rust/rustfmt.toml
 
 Rust-related patches CC here
 L: qemu-rust@nongnu.org
+F: tests/docker/test-rust
 F: rust/
 
 SLIRP
diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index fead7d3abe..fa1cbb6726 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -236,3 +236,6 @@ docker-image: ${DOCKER_IMAGES:%=docker-image-%}
 
 docker-clean:
 	$(call quiet-command, $(DOCKER_SCRIPT) clean)
+
+# Overrides
+docker-test-rust%: NETWORK=1
diff --git a/tests/docker/test-rust b/tests/docker/test-rust
new file mode 100755
index 0000000000..e7e3e94a55
--- /dev/null
+++ b/tests/docker/test-rust
@@ -0,0 +1,21 @@
+#!/bin/bash -e
+#
+# Run the rust code checks (a.k.a. check-rust-tools-nightly)
+#
+# Copyright (c) 2025 Linaro Ltd
+#
+# Authors:
+#  Alex Bennée <alex.bennee@linaro.org>
+#
+# This work is licensed under the terms of the GNU GPL, version 2
+# or (at your option) any later version. See the COPYING file in
+# the top-level directory.
+
+. common.rc
+
+cd "$BUILD_DIR"
+
+configure_qemu --disable-user --disable-docs --enable-rust
+pyvenv/bin/meson devenv -w $QEMU_SRC/rust ${CARGO-cargo} fmt --check
+make clippy
+make rustdoc
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 02/17] tests/qtest: don't attempt to clock_step while waiting for virtio ISR
  2025-02-07 15:30 [PATCH v2 00/17] maintainer updates for feb25 (qtest, gdbstub, plugins) pre-PR Alex Bennée
  2025-02-07 15:30 ` [PATCH v2 01/17] tests/docker: replicate the check-rust-tools-nightly CI job Alex Bennée
@ 2025-02-07 15:30 ` Alex Bennée
  2025-02-07 16:02   ` Thomas Huth
  2025-02-07 15:30 ` [PATCH v2 03/17] tests/qtest: don't step clock at start of npcm7xx periodic IRQ test Alex Bennée
                   ` (14 subsequent siblings)
  16 siblings, 1 reply; 28+ messages in thread
From: Alex Bennée @ 2025-02-07 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Xu, Alex Bennée, Thomas Huth, Weiwei Li, qemu-arm,
	Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu,
	Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv

This replicates the changes from 92cb8f8bf6 (tests/qtest: remove
clock_steps from virtio tests) as there are no timers in the virtio
code. We still busy wait and timeout though.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v2
  - fix trailing space
---
 tests/qtest/libqos/virtio-pci-modern.c | 6 ++----
 tests/qtest/libqos/virtio-pci.c        | 6 ++----
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/tests/qtest/libqos/virtio-pci-modern.c b/tests/qtest/libqos/virtio-pci-modern.c
index 18d118866f..4e67fcbd5d 100644
--- a/tests/qtest/libqos/virtio-pci-modern.c
+++ b/tests/qtest/libqos/virtio-pci-modern.c
@@ -173,13 +173,11 @@ static bool get_config_isr_status(QVirtioDevice *d)
 
 static void wait_config_isr_status(QVirtioDevice *d, gint64 timeout_us)
 {
-    QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
     gint64 start_time = g_get_monotonic_time();
 
-    do {
+    while (!get_config_isr_status(d)) {
         g_assert(g_get_monotonic_time() - start_time <= timeout_us);
-        qtest_clock_step(dev->pdev->bus->qts, 100);
-    } while (!get_config_isr_status(d));
+    }
 }
 
 static void queue_select(QVirtioDevice *d, uint16_t index)
diff --git a/tests/qtest/libqos/virtio-pci.c b/tests/qtest/libqos/virtio-pci.c
index 485b8f6b7e..002bf8b8c2 100644
--- a/tests/qtest/libqos/virtio-pci.c
+++ b/tests/qtest/libqos/virtio-pci.c
@@ -171,13 +171,11 @@ static bool qvirtio_pci_get_config_isr_status(QVirtioDevice *d)
 static void qvirtio_pci_wait_config_isr_status(QVirtioDevice *d,
                                                gint64 timeout_us)
 {
-    QVirtioPCIDevice *dev = container_of(d, QVirtioPCIDevice, vdev);
     gint64 start_time = g_get_monotonic_time();
 
-    do {
+    while (!qvirtio_pci_get_config_isr_status(d)) {
         g_assert(g_get_monotonic_time() - start_time <= timeout_us);
-        qtest_clock_step(dev->pdev->bus->qts, 100);
-    } while (!qvirtio_pci_get_config_isr_status(d));
+    }
 }
 
 static void qvirtio_pci_queue_select(QVirtioDevice *d, uint16_t index)
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 03/17] tests/qtest: don't step clock at start of npcm7xx periodic IRQ test
  2025-02-07 15:30 [PATCH v2 00/17] maintainer updates for feb25 (qtest, gdbstub, plugins) pre-PR Alex Bennée
  2025-02-07 15:30 ` [PATCH v2 01/17] tests/docker: replicate the check-rust-tools-nightly CI job Alex Bennée
  2025-02-07 15:30 ` [PATCH v2 02/17] tests/qtest: don't attempt to clock_step while waiting for virtio ISR Alex Bennée
@ 2025-02-07 15:30 ` Alex Bennée
  2025-02-07 15:30 ` [PATCH v2 04/17] tests/qtest: simplify qtest_process_inbuf Alex Bennée
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Alex Bennée @ 2025-02-07 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Xu, Alex Bennée, Thomas Huth, Weiwei Li, qemu-arm,
	Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu,
	Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv

Until there are timers enabled the semantics of clock_step_next() will
fail. Since d524441a36 (system/qtest: properly feedback results of
clock_[step|set]) we will signal a FAIL if time doesn't advance.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Hao Wu <wuhaotsh@google.com>
---
 tests/qtest/npcm7xx_timer-test.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/qtest/npcm7xx_timer-test.c b/tests/qtest/npcm7xx_timer-test.c
index 58f58c2f71..43711049ca 100644
--- a/tests/qtest/npcm7xx_timer-test.c
+++ b/tests/qtest/npcm7xx_timer-test.c
@@ -465,7 +465,6 @@ static void test_periodic_interrupt(gconstpointer test_data)
     int i;
 
     tim_reset(td);
-    clock_step_next();
 
     tim_write_ticr(td, count);
     tim_write_tcsr(td, CEN | IE | MODE_PERIODIC | PRESCALE(ps));
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 04/17] tests/qtest: simplify qtest_process_inbuf
  2025-02-07 15:30 [PATCH v2 00/17] maintainer updates for feb25 (qtest, gdbstub, plugins) pre-PR Alex Bennée
                   ` (2 preceding siblings ...)
  2025-02-07 15:30 ` [PATCH v2 03/17] tests/qtest: don't step clock at start of npcm7xx periodic IRQ test Alex Bennée
@ 2025-02-07 15:30 ` Alex Bennée
  2025-02-07 15:31 ` [PATCH v2 05/17] tests/qtest: rename qtest_send_prefix and roll-up into qtest_send Alex Bennée
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Alex Bennée @ 2025-02-07 15:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Xu, Alex Bennée, Thomas Huth, Weiwei Li, qemu-arm,
	Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu,
	Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv, Peter Maydell

Don't both creating a GString to temporarily hold our qtest command.
Instead do a simpler g_strndup and use autofree to clean up
afterwards.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 system/qtest.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/system/qtest.c b/system/qtest.c
index e68ed0f2a8..bb1efba9fd 100644
--- a/system/qtest.c
+++ b/system/qtest.c
@@ -763,25 +763,21 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
     }
 }
 
+/*
+ * Process as much of @inbuf as we can in newline terminated chunks.
+ * Remove the processed commands from @inbuf as we go.
+ */
 static void qtest_process_inbuf(CharBackend *chr, GString *inbuf)
 {
     char *end;
 
     while ((end = strchr(inbuf->str, '\n')) != NULL) {
-        size_t offset;
-        GString *cmd;
-        gchar **words;
-
-        offset = end - inbuf->str;
+        size_t len = end - inbuf->str;
+        g_autofree char *cmd = g_strndup(inbuf->str, len);
+        g_auto(GStrv) words = g_strsplit(cmd, " ", 0);
 
-        cmd = g_string_new_len(inbuf->str, offset);
-        g_string_erase(inbuf, 0, offset + 1);
-
-        words = g_strsplit(cmd->str, " ", 0);
+        g_string_erase(inbuf, 0, len + 1);
         qtest_process_command(chr, words);
-        g_strfreev(words);
-
-        g_string_free(cmd, TRUE);
     }
 }
 
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 05/17] tests/qtest: rename qtest_send_prefix and roll-up into qtest_send
  2025-02-07 15:30 [PATCH v2 00/17] maintainer updates for feb25 (qtest, gdbstub, plugins) pre-PR Alex Bennée
                   ` (3 preceding siblings ...)
  2025-02-07 15:30 ` [PATCH v2 04/17] tests/qtest: simplify qtest_process_inbuf Alex Bennée
@ 2025-02-07 15:31 ` Alex Bennée
  2025-02-07 16:04   ` Thomas Huth
  2025-02-07 15:31 ` [PATCH v2 06/17] tests/qtest: tighten up the checks on clock_step Alex Bennée
                   ` (11 subsequent siblings)
  16 siblings, 1 reply; 28+ messages in thread
From: Alex Bennée @ 2025-02-07 15:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Xu, Alex Bennée, Thomas Huth, Weiwei Li, qemu-arm,
	Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu,
	Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv

qtest_send_prefix never actually sent something over the chardev, all
it does is print the timestamp to the QTEST_LOG when enabled. So
rename the function, make it static, remove the unused CharDev and
simplify all the call sites by handling that directly with
qtest_send (and qtest_log_send).

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/system/qtest.h |  1 -
 hw/ppc/spapr_rtas.c    |  1 -
 hw/riscv/riscv_hart.c  |  1 -
 system/qtest.c         | 26 +++-----------------------
 4 files changed, 3 insertions(+), 26 deletions(-)

diff --git a/include/system/qtest.h b/include/system/qtest.h
index c161d75165..6ddddc501b 100644
--- a/include/system/qtest.h
+++ b/include/system/qtest.h
@@ -24,7 +24,6 @@ static inline bool qtest_enabled(void)
 }
 
 #ifndef CONFIG_USER_ONLY
-void qtest_send_prefix(CharBackend *chr);
 void G_GNUC_PRINTF(2, 3) qtest_sendf(CharBackend *chr, const char *fmt, ...);
 void qtest_set_command_cb(bool (*pc_cb)(CharBackend *chr, gchar **words));
 bool qtest_driver(void);
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index df2e837632..503d441b48 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -565,7 +565,6 @@ static bool spapr_qtest_callback(CharBackend *chr, gchar **words)
         g_assert(rc == 0);
         res = qtest_rtas_call(words[1], nargs, args, nret, ret);
 
-        qtest_send_prefix(chr);
         qtest_sendf(chr, "OK %"PRIu64"\n", res);
 
         return true;
diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
index ad67cd7645..a55d156668 100644
--- a/hw/riscv/riscv_hart.c
+++ b/hw/riscv/riscv_hart.c
@@ -94,7 +94,6 @@ static bool csr_qtest_callback(CharBackend *chr, gchar **words)
         g_assert(rc == 0);
         csr_call(words[1], cpu, csr, &val);
 
-        qtest_send_prefix(chr);
         qtest_sendf(chr, "OK 0 "TARGET_FMT_lx"\n", (target_ulong)val);
 
         return true;
diff --git a/system/qtest.c b/system/qtest.c
index bb1efba9fd..28b6fac37c 100644
--- a/system/qtest.c
+++ b/system/qtest.c
@@ -265,7 +265,7 @@ static int hex2nib(char ch)
     }
 }
 
-void qtest_send_prefix(CharBackend *chr)
+static void qtest_log_timestamp(void)
 {
     if (!qtest_log_fp || !qtest_opened) {
         return;
@@ -282,7 +282,7 @@ static void G_GNUC_PRINTF(1, 2) qtest_log_send(const char *fmt, ...)
         return;
     }
 
-    qtest_send_prefix(NULL);
+    qtest_log_timestamp();
 
     va_start(ap, fmt);
     vfprintf(qtest_log_fp, fmt, ap);
@@ -301,6 +301,7 @@ static void qtest_server_char_be_send(void *opaque, const char *str)
 
 static void qtest_send(CharBackend *chr, const char *str)
 {
+    qtest_log_timestamp();
     qtest_server_send(qtest_server_send_opaque, str);
 }
 
@@ -324,7 +325,6 @@ static void qtest_irq_handler(void *opaque, int n, int level)
     if (irq_levels[n] != level) {
         CharBackend *chr = &qtest->qtest_chr;
         irq_levels[n] = level;
-        qtest_send_prefix(chr);
         qtest_sendf(chr, "IRQ %s %d\n",
                     level ? "raise" : "lower", n);
     }
@@ -380,19 +380,16 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         is_outbound = words[0][14] == 'o';
         dev = DEVICE(object_resolve_path(words[1], NULL));
         if (!dev) {
-            qtest_send_prefix(chr);
             qtest_send(chr, "FAIL Unknown device\n");
             return;
         }
 
         if (is_named && !is_outbound) {
-            qtest_send_prefix(chr);
             qtest_send(chr, "FAIL Interception of named in-GPIOs not yet supported\n");
             return;
         }
 
         if (irq_intercept_dev) {
-            qtest_send_prefix(chr);
             if (irq_intercept_dev != dev) {
                 qtest_send(chr, "FAIL IRQ intercept already enabled\n");
             } else {
@@ -419,7 +416,6 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
             }
         }
 
-        qtest_send_prefix(chr);
         if (interception_succeeded) {
             irq_intercept_dev = dev;
             qtest_send(chr, "OK\n");
@@ -438,7 +434,6 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
 
         dev = DEVICE(object_resolve_path(words[1], NULL));
         if (!dev) {
-            qtest_send_prefix(chr);
             qtest_send(chr, "FAIL Unknown device\n");
             return;
         }
@@ -457,7 +452,6 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         irq = qdev_get_gpio_in_named(dev, name, num);
 
         qemu_set_irq(irq, level);
-        qtest_send_prefix(chr);
         qtest_send(chr, "OK\n");
     } else if (strcmp(words[0], "outb") == 0 ||
                strcmp(words[0], "outw") == 0 ||
@@ -480,7 +474,6 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         } else if (words[0][3] == 'l') {
             cpu_outl(addr, value);
         }
-        qtest_send_prefix(chr);
         qtest_send(chr, "OK\n");
     } else if (strcmp(words[0], "inb") == 0 ||
         strcmp(words[0], "inw") == 0 ||
@@ -501,7 +494,6 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         } else if (words[0][2] == 'l') {
             value = cpu_inl(addr);
         }
-        qtest_send_prefix(chr);
         qtest_sendf(chr, "OK 0x%04x\n", value);
     } else if (strcmp(words[0], "writeb") == 0 ||
                strcmp(words[0], "writew") == 0 ||
@@ -537,7 +529,6 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
             address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
                                 &data, 8);
         }
-        qtest_send_prefix(chr);
         qtest_send(chr, "OK\n");
     } else if (strcmp(words[0], "readb") == 0 ||
                strcmp(words[0], "readw") == 0 ||
@@ -571,7 +562,6 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
                                &value, 8);
             tswap64s(&value);
         }
-        qtest_send_prefix(chr);
         qtest_sendf(chr, "OK 0x%016" PRIx64 "\n", value);
     } else if (strcmp(words[0], "read") == 0) {
         g_autoptr(GString) enc = NULL;
@@ -593,7 +583,6 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
 
         enc = qemu_hexdump_line(NULL, data, len, 0, 0);
 
-        qtest_send_prefix(chr);
         qtest_sendf(chr, "OK 0x%s\n", enc->str);
 
         g_free(data);
@@ -613,7 +602,6 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data,
                            len);
         b64_data = g_base64_encode(data, len);
-        qtest_send_prefix(chr);
         qtest_sendf(chr, "OK %s\n", b64_data);
 
         g_free(data);
@@ -649,7 +637,6 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
                             len);
         g_free(data);
 
-        qtest_send_prefix(chr);
         qtest_send(chr, "OK\n");
     } else if (strcmp(words[0], "memset") == 0) {
         uint64_t addr, len;
@@ -673,7 +660,6 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
             g_free(data);
         }
 
-        qtest_send_prefix(chr);
         qtest_send(chr, "OK\n");
     }  else if (strcmp(words[0], "b64write") == 0) {
         uint64_t addr, len;
@@ -705,10 +691,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data,
                             len);
 
-        qtest_send_prefix(chr);
         qtest_send(chr, "OK\n");
     } else if (strcmp(words[0], "endianness") == 0) {
-        qtest_send_prefix(chr);
         if (target_words_bigendian()) {
             qtest_sendf(chr, "OK big\n");
         } else {
@@ -726,7 +710,6 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
                                             QEMU_TIMER_ATTR_ALL);
         }
         new_ns = qemu_clock_advance_virtual_time(old_ns + ns);
-        qtest_send_prefix(chr);
         qtest_sendf(chr, "%s %"PRIi64"\n",
                     new_ns > old_ns ? "OK" : "FAIL", new_ns);
     } else if (strcmp(words[0], "module_load") == 0) {
@@ -734,7 +717,6 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         int rv;
         g_assert(words[1] && words[2]);
 
-        qtest_send_prefix(chr);
         rv = module_load(words[1], words[2], &local_err);
         if (rv > 0) {
             qtest_sendf(chr, "OK\n");
@@ -752,13 +734,11 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         ret = qemu_strtoi64(words[1], NULL, 0, &ns);
         g_assert(ret == 0);
         new_ns = qemu_clock_advance_virtual_time(ns);
-        qtest_send_prefix(chr);
         qtest_sendf(chr, "%s %"PRIi64"\n",
                     new_ns == ns ? "OK" : "FAIL", new_ns);
     } else if (process_command_cb && process_command_cb(chr, words)) {
         /* Command got consumed by the callback handler */
     } else {
-        qtest_send_prefix(chr);
         qtest_sendf(chr, "FAIL Unknown command '%s'\n", words[0]);
     }
 }
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 06/17] tests/qtest: tighten up the checks on clock_step
  2025-02-07 15:30 [PATCH v2 00/17] maintainer updates for feb25 (qtest, gdbstub, plugins) pre-PR Alex Bennée
                   ` (4 preceding siblings ...)
  2025-02-07 15:31 ` [PATCH v2 05/17] tests/qtest: rename qtest_send_prefix and roll-up into qtest_send Alex Bennée
@ 2025-02-07 15:31 ` Alex Bennée
  2025-02-07 15:31 ` [PATCH v2 07/17] Revert "util/timer: avoid deadlock when shutting down" Alex Bennée
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Alex Bennée @ 2025-02-07 15:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Xu, Alex Bennée, Thomas Huth, Weiwei Li, qemu-arm,
	Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu,
	Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv, Peter Maydell

It is invalid to call clock_step with an implied time to step forward
as if no timers are running we won't be able to advance.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

---
v3
  - used Peter's suggested wording
  - used plain old if over ternary operator
---
 system/qtest.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/system/qtest.c b/system/qtest.c
index 28b6fac37c..12152efbcd 100644
--- a/system/qtest.c
+++ b/system/qtest.c
@@ -708,10 +708,19 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         } else {
             ns = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL,
                                             QEMU_TIMER_ATTR_ALL);
+            if (ns < 0) {
+                qtest_send(chr, "FAIL "
+                           "cannot advance clock to the next deadline "
+                           "because there is no pending deadline\n");
+                return;
+            }
         }
         new_ns = qemu_clock_advance_virtual_time(old_ns + ns);
-        qtest_sendf(chr, "%s %"PRIi64"\n",
-                    new_ns > old_ns ? "OK" : "FAIL", new_ns);
+        if (new_ns > old_ns) {
+            qtest_sendf(chr, "OK %"PRIi64"\n", new_ns);
+        } else {
+            qtest_sendf(chr, "FAIL could not advance time\n");
+        }
     } else if (strcmp(words[0], "module_load") == 0) {
         Error *local_err = NULL;
         int rv;
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 07/17] Revert "util/timer: avoid deadlock when shutting down"
  2025-02-07 15:30 [PATCH v2 00/17] maintainer updates for feb25 (qtest, gdbstub, plugins) pre-PR Alex Bennée
                   ` (5 preceding siblings ...)
  2025-02-07 15:31 ` [PATCH v2 06/17] tests/qtest: tighten up the checks on clock_step Alex Bennée
@ 2025-02-07 15:31 ` Alex Bennée
  2025-02-07 15:31 ` [PATCH v2 08/17] tests/qtest/migration: Add --full option Alex Bennée
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Alex Bennée @ 2025-02-07 15:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Xu, Alex Bennée, Thomas Huth, Weiwei Li, qemu-arm,
	Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu,
	Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv, Peter Maydell

This reverts commit bc02be4508d8753d1f6071b77d10f4661587df6f.

Now we catch attempts to clock_step to the next timer when none are
enabled we can revert the previous attempt to prevent deadlock. As
long as a new target time is given we will move time forward even if
no timers will fire. This is desirable for tests which are checking
that nothing changes when things are disabled.

Previously most tests got away with it because --enable-slirp always
has a timer running while the test is active.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 util/qemu-timer.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index 0e8a453eaa..3243d2c515 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -675,17 +675,10 @@ int64_t qemu_clock_advance_virtual_time(int64_t dest)
 {
     int64_t clock = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
     AioContext *aio_context;
-    int64_t deadline;
-
     aio_context = qemu_get_aio_context();
-
-    deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL,
-                                          QEMU_TIMER_ATTR_ALL);
-    /*
-     * A deadline of < 0 indicates this timer is not enabled, so we
-     * won't get far trying to run it forward.
-     */
-    while (deadline >= 0 && clock < dest) {
+    while (clock < dest) {
+        int64_t deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL,
+                                                      QEMU_TIMER_ATTR_ALL);
         int64_t warp = qemu_soonest_timeout(dest - clock, deadline);
 
         qemu_virtual_clock_set_ns(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + warp);
@@ -693,9 +686,6 @@ int64_t qemu_clock_advance_virtual_time(int64_t dest)
         qemu_clock_run_timers(QEMU_CLOCK_VIRTUAL);
         timerlist_run_timers(aio_context->tlg.tl[QEMU_CLOCK_VIRTUAL]);
         clock = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-
-        deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL,
-                                              QEMU_TIMER_ATTR_ALL);
     }
     qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
 
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 08/17] tests/qtest/migration: Add --full option
  2025-02-07 15:30 [PATCH v2 00/17] maintainer updates for feb25 (qtest, gdbstub, plugins) pre-PR Alex Bennée
                   ` (6 preceding siblings ...)
  2025-02-07 15:31 ` [PATCH v2 07/17] Revert "util/timer: avoid deadlock when shutting down" Alex Bennée
@ 2025-02-07 15:31 ` Alex Bennée
  2025-02-07 15:31 ` [PATCH v2 09/17] tests/qtest/migration: Pick smoke tests Alex Bennée
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Alex Bennée @ 2025-02-07 15:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Xu, Alex Bennée, Thomas Huth, Weiwei Li, qemu-arm,
	Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu,
	Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv

From: Fabiano Rosas <farosas@suse.de>

Add a new command line option to allow selecting between running the
full set of tests or a smaller set of tests. The default will be to
run the small set (i.e. no comand line option provided) so we can
reduce the amount of tests run by default. Only hosts which support
KVM for the target architecture being tested will run the complete set
of tests.

Adjust the meson.build file to pass in the --full option when
appropriate.

(for now, set the option unconditionally until the next patch actually
creates the small set)

Use cases:

configure --target-list=aarch64-softmmu,ppc64-softmmu,s390x-softmmu,x86_64-softmmu

                        | before - 615s/244 tests  | after - 244s/100 tests
------------------------+--------------------------+-----------------------------
make check              | full set for all archs   | full set for the KVM arch,
make check-qtest        |                          | small set for the rest
                        |                          |
qemu-system-$ARCH       | full set for $ARCH       | small set for $ARCH, KVM or
./migration-test        |                          | TCG automatically chosen
                        |                          |
qemu-system-$ARCH       | N/A                      | full set for $ARCH, KVM or
./migration-test --full |                          | TCG automatically chosen
                        |                          |
migration-compat-x86_64 | full set for x86_64      | small set for x86_64
CI job                  |                          |
------------------------+--------------------------+-----------------------------

Signed-off-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Peter Xu <peterx@redhat.com>
Message-Id: <20250130184012.5711-2-farosas@suse.de>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/qtest/migration/framework.h         |  1 +
 tests/qtest/migration-test.c              | 26 +++++++++++++++++++++++
 tests/qtest/migration/compression-tests.c |  4 ++++
 tests/qtest/migration/cpr-tests.c         |  4 ++++
 tests/qtest/migration/file-tests.c        |  4 ++++
 tests/qtest/migration/misc-tests.c        |  4 ++++
 tests/qtest/migration/postcopy-tests.c    |  4 ++++
 tests/qtest/migration/precopy-tests.c     |  4 ++++
 tests/qtest/migration/tls-tests.c         |  4 ++++
 tests/qtest/meson.build                   | 11 +++++++++-
 10 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/migration/framework.h b/tests/qtest/migration/framework.h
index cb4a984700..e4a11870f6 100644
--- a/tests/qtest/migration/framework.h
+++ b/tests/qtest/migration/framework.h
@@ -24,6 +24,7 @@ typedef struct MigrationTestEnv {
     bool uffd_feature_thread_id;
     bool has_dirty_ring;
     bool is_x86;
+    bool full_set;
     const char *arch;
     const char *qemu_src;
     const char *qemu_dst;
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 5cad5060b3..61180fcbd5 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -14,13 +14,39 @@
 #include "migration/framework.h"
 #include "qemu/module.h"
 
+static void parse_args(int *argc_p, char ***argv_p, bool *full_set)
+{
+    int argc = *argc_p;
+    char **argv = *argv_p;
+    int i, j;
+
+    j = 1;
+    for (i = 1; i < argc; i++) {
+        if (g_str_equal(argv[i], "--full")) {
+            *full_set = true;
+            continue;
+        }
+        argv[j++] = argv[i];
+        if (i >= j) {
+            argv[i] = NULL;
+        }
+    }
+    *argc_p = j;
+}
+
 int main(int argc, char **argv)
 {
     MigrationTestEnv *env;
     int ret;
+    bool full_set = false;
+
+    /* strip the --full option if it's present */
+    parse_args(&argc, &argv, &full_set);
 
     g_test_init(&argc, &argv, NULL);
     env = migration_get_env();
+    env->full_set = full_set;
+    env->full_set = true; /* temporary */
     module_call_init(MODULE_INIT_QOM);
 
     migration_test_add_tls(env);
diff --git a/tests/qtest/migration/compression-tests.c b/tests/qtest/migration/compression-tests.c
index d78f1f11f1..9d0a258d51 100644
--- a/tests/qtest/migration/compression-tests.c
+++ b/tests/qtest/migration/compression-tests.c
@@ -155,6 +155,10 @@ void migration_test_add_compression(MigrationTestEnv *env)
 {
     tmpfs = env->tmpfs;
 
+    if (!env->full_set) {
+        return;
+    }
+
 #ifdef CONFIG_ZSTD
     migration_test_add("/migration/multifd/tcp/plain/zstd",
                        test_multifd_tcp_zstd);
diff --git a/tests/qtest/migration/cpr-tests.c b/tests/qtest/migration/cpr-tests.c
index 215b0df8c0..b1651fe48c 100644
--- a/tests/qtest/migration/cpr-tests.c
+++ b/tests/qtest/migration/cpr-tests.c
@@ -104,6 +104,10 @@ void migration_test_add_cpr(MigrationTestEnv *env)
 {
     tmpfs = env->tmpfs;
 
+    if (!env->full_set) {
+        return;
+    }
+
     /*
      * Our CI system has problems with shared memory.
      * Don't run this test until we find a workaround.
diff --git a/tests/qtest/migration/file-tests.c b/tests/qtest/migration/file-tests.c
index 84225c8c33..5b190853a5 100644
--- a/tests/qtest/migration/file-tests.c
+++ b/tests/qtest/migration/file-tests.c
@@ -304,6 +304,10 @@ void migration_test_add_file(MigrationTestEnv *env)
 {
     tmpfs = env->tmpfs;
 
+    if (!env->full_set) {
+        return;
+    }
+
     migration_test_add("/migration/precopy/file",
                        test_precopy_file);
 
diff --git a/tests/qtest/migration/misc-tests.c b/tests/qtest/migration/misc-tests.c
index dda3707cf3..996256ef87 100644
--- a/tests/qtest/migration/misc-tests.c
+++ b/tests/qtest/migration/misc-tests.c
@@ -262,6 +262,10 @@ void migration_test_add_misc(MigrationTestEnv *env)
 {
     tmpfs = env->tmpfs;
 
+    if (!env->full_set) {
+        return;
+    }
+
     migration_test_add("/migration/bad_dest", test_baddest);
 #ifndef _WIN32
     migration_test_add("/migration/analyze-script", test_analyze_script);
diff --git a/tests/qtest/migration/postcopy-tests.c b/tests/qtest/migration/postcopy-tests.c
index daf7449f2c..b08cde3270 100644
--- a/tests/qtest/migration/postcopy-tests.c
+++ b/tests/qtest/migration/postcopy-tests.c
@@ -81,6 +81,10 @@ static void test_postcopy_preempt_recovery(void)
 
 void migration_test_add_postcopy(MigrationTestEnv *env)
 {
+    if (!env->full_set) {
+        return;
+    }
+
     if (env->has_uffd) {
         migration_test_add("/migration/postcopy/plain", test_postcopy);
         migration_test_add("/migration/postcopy/recovery/plain",
diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c
index 436dbd98e8..c99a487bf1 100644
--- a/tests/qtest/migration/precopy-tests.c
+++ b/tests/qtest/migration/precopy-tests.c
@@ -955,6 +955,10 @@ void migration_test_add_precopy(MigrationTestEnv *env)
 {
     tmpfs = env->tmpfs;
 
+    if (!env->full_set) {
+        return;
+    }
+
     if (env->is_x86) {
         migration_test_add("/migration/precopy/unix/suspend/live",
                            test_precopy_unix_suspend_live);
diff --git a/tests/qtest/migration/tls-tests.c b/tests/qtest/migration/tls-tests.c
index 5704a1f992..aee56930ac 100644
--- a/tests/qtest/migration/tls-tests.c
+++ b/tests/qtest/migration/tls-tests.c
@@ -726,6 +726,10 @@ void migration_test_add_tls(MigrationTestEnv *env)
 {
     tmpfs = env->tmpfs;
 
+    if (!env->full_set) {
+        return;
+    }
+
     migration_test_add("/migration/precopy/unix/tls/psk",
                        test_precopy_unix_tls_psk);
 
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 5e062c752a..68316dbdc1 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -405,6 +405,8 @@ foreach dir : target_dirs
   target_base = dir.split('-')[0]
   qtest_emulator = emulators['qemu-system-' + target_base]
   target_qtests = get_variable('qtests_' + target_base, []) + qtests_generic
+  has_kvm = ('CONFIG_KVM' in config_all_accel and host_os == 'linux'
+             and cpu == target_base and fs.exists('/dev/kvm'))
 
   test_deps = roms
   qtest_env = environment()
@@ -438,11 +440,18 @@ foreach dir : target_dirs
         test: executable(test, src, dependencies: deps)
       }
     endif
+
+    test_args = ['--tap', '-k']
+
+    if test == 'migration-test' and has_kvm
+      test_args += ['--full']
+    endif
+
     test('qtest-@0@/@1@'.format(target_base, test),
          qtest_executables[test],
          depends: [test_deps, qtest_emulator, emulator_modules],
          env: qtest_env,
-         args: ['--tap', '-k'],
+         args: test_args,
          protocol: 'tap',
          timeout: slow_qtests.get(test, 60),
          priority: slow_qtests.get(test, 60),
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 09/17] tests/qtest/migration: Pick smoke tests
  2025-02-07 15:30 [PATCH v2 00/17] maintainer updates for feb25 (qtest, gdbstub, plugins) pre-PR Alex Bennée
                   ` (7 preceding siblings ...)
  2025-02-07 15:31 ` [PATCH v2 08/17] tests/qtest/migration: Add --full option Alex Bennée
@ 2025-02-07 15:31 ` Alex Bennée
  2025-02-07 15:31 ` [PATCH v2 10/17] gdbstub: Allow the %d placeholder in the socket path Alex Bennée
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Alex Bennée @ 2025-02-07 15:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Xu, Alex Bennée, Thomas Huth, Weiwei Li, qemu-arm,
	Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu,
	Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv

From: Fabiano Rosas <farosas@suse.de>

Choose a few tests per group and move them from the full set to the
smoke set.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Peter Xu <peterx@redhat.com>
Message-Id: <20250130184012.5711-3-farosas@suse.de>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/qtest/migration-test.c              |  1 -
 tests/qtest/migration/compression-tests.c | 11 ++++++---
 tests/qtest/migration/cpr-tests.c         |  2 ++
 tests/qtest/migration/file-tests.c        | 17 +++++++++-----
 tests/qtest/migration/misc-tests.c        | 12 +++++++---
 tests/qtest/migration/postcopy-tests.c    | 18 ++++++++++-----
 tests/qtest/migration/precopy-tests.c     | 27 ++++++++++++++---------
 tests/qtest/migration/tls-tests.c         | 10 +++++++--
 8 files changed, 67 insertions(+), 31 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 61180fcbd5..0893687174 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -46,7 +46,6 @@ int main(int argc, char **argv)
     g_test_init(&argc, &argv, NULL);
     env = migration_get_env();
     env->full_set = full_set;
-    env->full_set = true; /* temporary */
     module_call_init(MODULE_INIT_QOM);
 
     migration_test_add_tls(env);
diff --git a/tests/qtest/migration/compression-tests.c b/tests/qtest/migration/compression-tests.c
index 9d0a258d51..8b58401b84 100644
--- a/tests/qtest/migration/compression-tests.c
+++ b/tests/qtest/migration/compression-tests.c
@@ -151,10 +151,18 @@ static void test_multifd_tcp_zlib(void)
     test_precopy_common(&args);
 }
 
+static void migration_test_add_compression_smoke(MigrationTestEnv *env)
+{
+    migration_test_add("/migration/multifd/tcp/plain/zlib",
+                       test_multifd_tcp_zlib);
+}
+
 void migration_test_add_compression(MigrationTestEnv *env)
 {
     tmpfs = env->tmpfs;
 
+    migration_test_add_compression_smoke(env);
+
     if (!env->full_set) {
         return;
     }
@@ -183,7 +191,4 @@ void migration_test_add_compression(MigrationTestEnv *env)
         migration_test_add("/migration/precopy/unix/xbzrle",
                            test_precopy_unix_xbzrle);
     }
-
-    migration_test_add("/migration/multifd/tcp/plain/zlib",
-                       test_multifd_tcp_zlib);
 }
diff --git a/tests/qtest/migration/cpr-tests.c b/tests/qtest/migration/cpr-tests.c
index b1651fe48c..4758841824 100644
--- a/tests/qtest/migration/cpr-tests.c
+++ b/tests/qtest/migration/cpr-tests.c
@@ -104,6 +104,8 @@ void migration_test_add_cpr(MigrationTestEnv *env)
 {
     tmpfs = env->tmpfs;
 
+    /* no tests in the smoke set for now */
+
     if (!env->full_set) {
         return;
     }
diff --git a/tests/qtest/migration/file-tests.c b/tests/qtest/migration/file-tests.c
index 5b190853a5..f474b3590a 100644
--- a/tests/qtest/migration/file-tests.c
+++ b/tests/qtest/migration/file-tests.c
@@ -300,17 +300,25 @@ static void test_multifd_file_mapped_ram_fdset_dio(void)
 }
 #endif /* !_WIN32 */
 
+static void migration_test_add_file_smoke(MigrationTestEnv *env)
+{
+    migration_test_add("/migration/precopy/file",
+                       test_precopy_file);
+
+    migration_test_add("/migration/multifd/file/mapped-ram/dio",
+                       test_multifd_file_mapped_ram_dio);
+}
+
 void migration_test_add_file(MigrationTestEnv *env)
 {
     tmpfs = env->tmpfs;
 
+    migration_test_add_file_smoke(env);
+
     if (!env->full_set) {
         return;
     }
 
-    migration_test_add("/migration/precopy/file",
-                       test_precopy_file);
-
     migration_test_add("/migration/precopy/file/offset",
                        test_precopy_file_offset);
 #ifndef _WIN32
@@ -330,9 +338,6 @@ void migration_test_add_file(MigrationTestEnv *env)
     migration_test_add("/migration/multifd/file/mapped-ram/live",
                        test_multifd_file_mapped_ram_live);
 
-    migration_test_add("/migration/multifd/file/mapped-ram/dio",
-                       test_multifd_file_mapped_ram_dio);
-
 #ifndef _WIN32
     migration_test_add("/migration/multifd/file/mapped-ram/fdset",
                        test_multifd_file_mapped_ram_fdset);
diff --git a/tests/qtest/migration/misc-tests.c b/tests/qtest/migration/misc-tests.c
index 996256ef87..d03199f3c5 100644
--- a/tests/qtest/migration/misc-tests.c
+++ b/tests/qtest/migration/misc-tests.c
@@ -258,18 +258,24 @@ static void test_validate_uri_channels_none_set(void)
     do_test_validate_uri_channel(&args);
 }
 
+static void migration_test_add_misc_smoke(MigrationTestEnv *env)
+{
+#ifndef _WIN32
+    migration_test_add("/migration/analyze-script", test_analyze_script);
+#endif
+}
+
 void migration_test_add_misc(MigrationTestEnv *env)
 {
     tmpfs = env->tmpfs;
 
+    migration_test_add_misc_smoke(env);
+
     if (!env->full_set) {
         return;
     }
 
     migration_test_add("/migration/bad_dest", test_baddest);
-#ifndef _WIN32
-    migration_test_add("/migration/analyze-script", test_analyze_script);
-#endif
 
     /*
      * Our CI system has problems with shared memory.
diff --git a/tests/qtest/migration/postcopy-tests.c b/tests/qtest/migration/postcopy-tests.c
index b08cde3270..7e84e60fea 100644
--- a/tests/qtest/migration/postcopy-tests.c
+++ b/tests/qtest/migration/postcopy-tests.c
@@ -79,18 +79,26 @@ static void test_postcopy_preempt_recovery(void)
     test_postcopy_recovery_common(&args);
 }
 
-void migration_test_add_postcopy(MigrationTestEnv *env)
+static void migration_test_add_postcopy_smoke(MigrationTestEnv *env)
 {
-    if (!env->full_set) {
-        return;
-    }
-
     if (env->has_uffd) {
         migration_test_add("/migration/postcopy/plain", test_postcopy);
         migration_test_add("/migration/postcopy/recovery/plain",
                            test_postcopy_recovery);
         migration_test_add("/migration/postcopy/preempt/plain",
                            test_postcopy_preempt);
+    }
+}
+
+void migration_test_add_postcopy(MigrationTestEnv *env)
+{
+    migration_test_add_postcopy_smoke(env);
+
+    if (!env->full_set) {
+        return;
+    }
+
+    if (env->has_uffd) {
         migration_test_add("/migration/postcopy/preempt/recovery/plain",
                            test_postcopy_preempt_recovery);
 
diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c
index c99a487bf1..b16f10e908 100644
--- a/tests/qtest/migration/precopy-tests.c
+++ b/tests/qtest/migration/precopy-tests.c
@@ -951,14 +951,8 @@ static void test_dirty_limit(void)
     migrate_end(from, to, true);
 }
 
-void migration_test_add_precopy(MigrationTestEnv *env)
+static void migration_test_add_precopy_smoke(MigrationTestEnv *env)
 {
-    tmpfs = env->tmpfs;
-
-    if (!env->full_set) {
-        return;
-    }
-
     if (env->is_x86) {
         migration_test_add("/migration/precopy/unix/suspend/live",
                            test_precopy_unix_suspend_live);
@@ -970,6 +964,21 @@ void migration_test_add_precopy(MigrationTestEnv *env)
                        test_precopy_unix_plain);
 
     migration_test_add("/migration/precopy/tcp/plain", test_precopy_tcp_plain);
+    migration_test_add("/migration/multifd/tcp/uri/plain/none",
+                       test_multifd_tcp_uri_none);
+    migration_test_add("/migration/multifd/tcp/plain/cancel",
+                       test_multifd_tcp_cancel);
+}
+
+void migration_test_add_precopy(MigrationTestEnv *env)
+{
+    tmpfs = env->tmpfs;
+
+    migration_test_add_precopy_smoke(env);
+
+    if (!env->full_set) {
+        return;
+    }
 
     migration_test_add("/migration/precopy/tcp/plain/switchover-ack",
                        test_precopy_tcp_switchover_ack);
@@ -993,16 +1002,12 @@ void migration_test_add_precopy(MigrationTestEnv *env)
                                test_dirty_limit);
         }
     }
-    migration_test_add("/migration/multifd/tcp/uri/plain/none",
-                       test_multifd_tcp_uri_none);
     migration_test_add("/migration/multifd/tcp/channels/plain/none",
                        test_multifd_tcp_channels_none);
     migration_test_add("/migration/multifd/tcp/plain/zero-page/legacy",
                        test_multifd_tcp_zero_page_legacy);
     migration_test_add("/migration/multifd/tcp/plain/zero-page/none",
                        test_multifd_tcp_no_zero_page);
-    migration_test_add("/migration/multifd/tcp/plain/cancel",
-                       test_multifd_tcp_cancel);
     if (g_str_equal(env->arch, "x86_64")
         && env->has_kvm && env->has_dirty_ring) {
 
diff --git a/tests/qtest/migration/tls-tests.c b/tests/qtest/migration/tls-tests.c
index aee56930ac..2cb4a44bcd 100644
--- a/tests/qtest/migration/tls-tests.c
+++ b/tests/qtest/migration/tls-tests.c
@@ -722,10 +722,18 @@ static void test_multifd_tcp_tls_x509_reject_anon_client(void)
 }
 #endif /* CONFIG_TASN1 */
 
+static void migration_test_add_tls_smoke(MigrationTestEnv *env)
+{
+    migration_test_add("/migration/precopy/tcp/tls/psk/match",
+                       test_precopy_tcp_tls_psk_match);
+}
+
 void migration_test_add_tls(MigrationTestEnv *env)
 {
     tmpfs = env->tmpfs;
 
+    migration_test_add_tls_smoke(env);
+
     if (!env->full_set) {
         return;
     }
@@ -755,8 +763,6 @@ void migration_test_add_tls(MigrationTestEnv *env)
                        test_precopy_unix_tls_x509_override_host);
 #endif /* CONFIG_TASN1 */
 
-    migration_test_add("/migration/precopy/tcp/tls/psk/match",
-                       test_precopy_tcp_tls_psk_match);
     migration_test_add("/migration/precopy/tcp/tls/psk/mismatch",
                        test_precopy_tcp_tls_psk_mismatch);
 #ifdef CONFIG_TASN1
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 10/17] gdbstub: Allow the %d placeholder in the socket path
  2025-02-07 15:30 [PATCH v2 00/17] maintainer updates for feb25 (qtest, gdbstub, plugins) pre-PR Alex Bennée
                   ` (8 preceding siblings ...)
  2025-02-07 15:31 ` [PATCH v2 09/17] tests/qtest/migration: Pick smoke tests Alex Bennée
@ 2025-02-07 15:31 ` Alex Bennée
  2025-02-07 15:31 ` [PATCH v2 11/17] gdbstub: Try unlinking the unix socket before binding Alex Bennée
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Alex Bennée @ 2025-02-07 15:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Xu, Alex Bennée, Thomas Huth, Weiwei Li, qemu-arm,
	Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu,
	Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv, Ilya Leoshkevich,
	Richard Henderson

From: Ilya Leoshkevich <iii@linux.ibm.com>

Just like for QEMU_LOG_FILENAME, replace %d with PID in the GDB socket
path. This allows running multi-process applications with, e.g.,
export QEMU_GDB=/tmp/qemu-%d.sock. Currently this is not possible,
since the first process will cause the subsequent ones to fail due to
not being able to bind() the GDB socket.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Warner Losh <imp@bsdimp.com>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-Id: <20250117001542.8290-2-iii@linux.ibm.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub/user.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/gdbstub/user.c b/gdbstub/user.c
index c2bdfc3d49..fd29d595f4 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -317,9 +317,19 @@ static bool gdb_accept_socket(int gdb_fd)
 
 static int gdbserver_open_socket(const char *path)
 {
+    g_autoptr(GString) buf = g_string_new("");
     struct sockaddr_un sockaddr = {};
+    char *pid_placeholder;
     int fd, ret;
 
+    pid_placeholder = strstr(path, "%d");
+    if (pid_placeholder != NULL) {
+        g_string_append_len(buf, path, pid_placeholder - path);
+        g_string_append_printf(buf, "%d", qemu_get_thread_id());
+        g_string_append(buf, pid_placeholder + 2);
+        path = buf->str;
+    }
+
     fd = socket(AF_UNIX, SOCK_STREAM, 0);
     if (fd < 0) {
         perror("create socket");
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 11/17] gdbstub: Try unlinking the unix socket before binding
  2025-02-07 15:30 [PATCH v2 00/17] maintainer updates for feb25 (qtest, gdbstub, plugins) pre-PR Alex Bennée
                   ` (9 preceding siblings ...)
  2025-02-07 15:31 ` [PATCH v2 10/17] gdbstub: Allow the %d placeholder in the socket path Alex Bennée
@ 2025-02-07 15:31 ` Alex Bennée
  2025-05-20 14:50   ` Michael Tokarev
  2025-02-07 15:31 ` [PATCH v2 12/17] user: Introduce user/signal.h Alex Bennée
                   ` (5 subsequent siblings)
  16 siblings, 1 reply; 28+ messages in thread
From: Alex Bennée @ 2025-02-07 15:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Xu, Alex Bennée, Thomas Huth, Weiwei Li, qemu-arm,
	Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu,
	Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv, Ilya Leoshkevich

From: Ilya Leoshkevich <iii@linux.ibm.com>

In case an emulated process execve()s another emulated process, bind()
will fail, because the socket already exists. So try deleting it. Use
the existing unix_listen() function which does this. Link qemu-user
with qemu-sockets.c and add the monitor_get_fd() stub.

Note that it is not possible to handle this in do_execv(): deleting
gdbserver_user_state.socket_path before safe_execve() is not correct,
because the latter may fail, and afterwards we may lose control.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20250117001542.8290-3-iii@linux.ibm.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub/user.c     | 29 +++--------------------------
 stubs/monitor-fd.c |  9 +++++++++
 stubs/meson.build  |  2 ++
 util/meson.build   |  2 ++
 4 files changed, 16 insertions(+), 26 deletions(-)
 create mode 100644 stubs/monitor-fd.c

diff --git a/gdbstub/user.c b/gdbstub/user.c
index fd29d595f4..8225b70280 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -315,12 +315,10 @@ static bool gdb_accept_socket(int gdb_fd)
     return true;
 }
 
-static int gdbserver_open_socket(const char *path)
+static int gdbserver_open_socket(const char *path, Error **errp)
 {
     g_autoptr(GString) buf = g_string_new("");
-    struct sockaddr_un sockaddr = {};
     char *pid_placeholder;
-    int fd, ret;
 
     pid_placeholder = strstr(path, "%d");
     if (pid_placeholder != NULL) {
@@ -330,28 +328,7 @@ static int gdbserver_open_socket(const char *path)
         path = buf->str;
     }
 
-    fd = socket(AF_UNIX, SOCK_STREAM, 0);
-    if (fd < 0) {
-        perror("create socket");
-        return -1;
-    }
-
-    sockaddr.sun_family = AF_UNIX;
-    pstrcpy(sockaddr.sun_path, sizeof(sockaddr.sun_path) - 1, path);
-    ret = bind(fd, (struct sockaddr *)&sockaddr, sizeof(sockaddr));
-    if (ret < 0) {
-        perror("bind socket");
-        close(fd);
-        return -1;
-    }
-    ret = listen(fd, 1);
-    if (ret < 0) {
-        perror("listen socket");
-        close(fd);
-        return -1;
-    }
-
-    return fd;
+    return unix_listen(path, errp);
 }
 
 static bool gdb_accept_tcp(int gdb_fd)
@@ -424,7 +401,7 @@ bool gdbserver_start(const char *port_or_path, Error **errp)
     if (port > 0) {
         gdb_fd = gdbserver_open_port(port, errp);
     } else {
-        gdb_fd = gdbserver_open_socket(port_or_path);
+        gdb_fd = gdbserver_open_socket(port_or_path, errp);
     }
 
     if (gdb_fd < 0) {
diff --git a/stubs/monitor-fd.c b/stubs/monitor-fd.c
new file mode 100644
index 0000000000..9bb6749885
--- /dev/null
+++ b/stubs/monitor-fd.c
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include "qemu/osdep.h"
+#include "monitor/monitor.h"
+
+int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
+{
+    abort();
+}
diff --git a/stubs/meson.build b/stubs/meson.build
index a8b3aeb564..b0fee37e05 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -61,6 +61,8 @@ if have_user
   if not have_system
     stub_ss.add(files('qdev.c'))
   endif
+
+  stub_ss.add(files('monitor-fd.c'))
 endif
 
 if have_system
diff --git a/util/meson.build b/util/meson.build
index 5d8bef9891..780b5977a8 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -84,6 +84,8 @@ if have_block or have_ga
   util_ss.add(files('qemu-coroutine.c', 'qemu-coroutine-lock.c', 'qemu-coroutine-io.c'))
   util_ss.add(files(f'coroutine-@coroutine_backend@.c'))
   util_ss.add(files('thread-pool.c', 'qemu-timer.c'))
+endif
+if have_block or have_ga or have_user
   util_ss.add(files('qemu-sockets.c'))
 endif
 if have_block
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 12/17] user: Introduce user/signal.h
  2025-02-07 15:30 [PATCH v2 00/17] maintainer updates for feb25 (qtest, gdbstub, plugins) pre-PR Alex Bennée
                   ` (10 preceding siblings ...)
  2025-02-07 15:31 ` [PATCH v2 11/17] gdbstub: Try unlinking the unix socket before binding Alex Bennée
@ 2025-02-07 15:31 ` Alex Bennée
  2025-02-07 15:31 ` [PATCH v2 13/17] user: Introduce host_interrupt_signal Alex Bennée
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Alex Bennée @ 2025-02-07 15:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Xu, Alex Bennée, Thomas Huth, Weiwei Li, qemu-arm,
	Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu,
	Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv, Ilya Leoshkevich,
	Richard Henderson

From: Ilya Leoshkevich <iii@linux.ibm.com>

gdbstub needs target_to_host_signal(), so move its declaration to a
public header.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Warner Losh <imp@bsdimp.com>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-Id: <20250117001542.8290-4-iii@linux.ibm.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 bsd-user/signal-common.h   |  1 -
 include/user/signal.h      | 23 +++++++++++++++++++++++
 linux-user/signal-common.h |  1 -
 bsd-user/signal.c          |  1 +
 linux-user/signal.c        |  1 +
 linux-user/syscall.c       |  1 +
 6 files changed, 26 insertions(+), 2 deletions(-)
 create mode 100644 include/user/signal.h

diff --git a/bsd-user/signal-common.h b/bsd-user/signal-common.h
index 77d7c7a78b..4e634e04a3 100644
--- a/bsd-user/signal-common.h
+++ b/bsd-user/signal-common.h
@@ -42,7 +42,6 @@ void process_pending_signals(CPUArchState *env);
 void queue_signal(CPUArchState *env, int sig, int si_type,
                   target_siginfo_t *info);
 void signal_init(void);
-int target_to_host_signal(int sig);
 void target_to_host_sigset(sigset_t *d, const target_sigset_t *s);
 
 /*
diff --git a/include/user/signal.h b/include/user/signal.h
new file mode 100644
index 0000000000..19b6b9e5dd
--- /dev/null
+++ b/include/user/signal.h
@@ -0,0 +1,23 @@
+/*
+ * Signal-related declarations.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#ifndef USER_SIGNAL_H
+#define USER_SIGNAL_H
+
+#ifndef CONFIG_USER_ONLY
+#error Cannot include this header from system emulation
+#endif
+
+/**
+ * target_to_host_signal:
+ * @sig: target signal.
+ *
+ * On success, return the host signal between 0 (inclusive) and NSIG
+ * (exclusive) corresponding to the target signal @sig. Return any other value
+ * on failure.
+ */
+int target_to_host_signal(int sig);
+
+#endif
diff --git a/linux-user/signal-common.h b/linux-user/signal-common.h
index 8584d9ecc2..196d2406f8 100644
--- a/linux-user/signal-common.h
+++ b/linux-user/signal-common.h
@@ -61,7 +61,6 @@ void queue_signal(CPUArchState *env, int sig, int si_type,
                   target_siginfo_t *info);
 void host_to_target_siginfo(target_siginfo_t *tinfo, const siginfo_t *info);
 void target_to_host_siginfo(siginfo_t *info, const target_siginfo_t *tinfo);
-int target_to_host_signal(int sig);
 int host_to_target_signal(int sig);
 long do_sigreturn(CPUArchState *env);
 long do_rt_sigreturn(CPUArchState *env);
diff --git a/bsd-user/signal.c b/bsd-user/signal.c
index b4e1458237..8c51f6ce65 100644
--- a/bsd-user/signal.c
+++ b/bsd-user/signal.c
@@ -24,6 +24,7 @@
 #include "user/cpu_loop.h"
 #include "exec/page-protection.h"
 #include "user/page-protection.h"
+#include "user/signal.h"
 #include "user/tswap-target.h"
 #include "gdbstub/user.h"
 #include "signal-common.h"
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 087c4d270e..bffbef235c 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -36,6 +36,7 @@
 #include "user/cpu_loop.h"
 #include "user/page-protection.h"
 #include "user/safe-syscall.h"
+#include "user/signal.h"
 #include "tcg/tcg.h"
 
 /* target_siginfo_t must fit in gdbstub's siginfo save area. */
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 6ee02383da..90afaa4426 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -138,6 +138,7 @@
 #include "user-mmap.h"
 #include "user/page-protection.h"
 #include "user/safe-syscall.h"
+#include "user/signal.h"
 #include "qemu/guest-random.h"
 #include "qemu/selfmap.h"
 #include "user/syscall-trace.h"
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 13/17] user: Introduce host_interrupt_signal
  2025-02-07 15:30 [PATCH v2 00/17] maintainer updates for feb25 (qtest, gdbstub, plugins) pre-PR Alex Bennée
                   ` (11 preceding siblings ...)
  2025-02-07 15:31 ` [PATCH v2 12/17] user: Introduce user/signal.h Alex Bennée
@ 2025-02-07 15:31 ` Alex Bennée
  2025-02-07 15:31 ` [PATCH v2 14/17] osdep: Introduce qemu_kill_thread() Alex Bennée
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Alex Bennée @ 2025-02-07 15:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Xu, Alex Bennée, Thomas Huth, Weiwei Li, qemu-arm,
	Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu,
	Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv, Ilya Leoshkevich

From: Ilya Leoshkevich <iii@linux.ibm.com>

Attaching to the gdbstub of a running process requires stopping its
threads. For threads that run on a CPU, cpu_exit() is enough, but the
only way to grab attention of a thread that is stuck in a long-running
syscall is to interrupt it with a signal.

Reserve a host realtime signal for this, just like it's already done
for TARGET_SIGABRT on Linux. This may reduce the number of available
guest realtime signals by one, but this is acceptable, since there are
quite a lot of them, and it's unlikely that there are apps that need
them all.

Set signal_pending for the safe_sycall machinery to prevent invoking
the syscall. This is a lie, since we don't queue a guest signal, but
process_pending_signals() can handle the absence of pending signals.
The syscall returns with QEMU_ERESTARTSYS errno, which arranges for
the automatic restart. This is important, because it helps avoiding
disturbing poorly written guests.

Reviewed-by: Warner Losh <imp@bsdimp.com>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-Id: <20250117001542.8290-5-iii@linux.ibm.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/user/signal.h |  2 ++
 bsd-user/signal.c     | 12 ++++++++++++
 linux-user/signal.c   | 25 ++++++++++++++++++++-----
 3 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/include/user/signal.h b/include/user/signal.h
index 19b6b9e5dd..7fa33b05d9 100644
--- a/include/user/signal.h
+++ b/include/user/signal.h
@@ -20,4 +20,6 @@
  */
 int target_to_host_signal(int sig);
 
+extern int host_interrupt_signal;
+
 #endif
diff --git a/bsd-user/signal.c b/bsd-user/signal.c
index 8c51f6ce65..ff2ccbbf60 100644
--- a/bsd-user/signal.c
+++ b/bsd-user/signal.c
@@ -51,6 +51,8 @@ static inline int sas_ss_flags(TaskState *ts, unsigned long sp)
         on_sig_stack(ts, sp) ? SS_ONSTACK : 0;
 }
 
+int host_interrupt_signal = SIGRTMAX;
+
 /*
  * The BSD ABIs use the same signal numbers across all the CPU architectures, so
  * (unlike Linux) these functions are just the identity mapping. This might not
@@ -491,6 +493,12 @@ static void host_signal_handler(int host_sig, siginfo_t *info, void *puc)
     uintptr_t pc = 0;
     bool sync_sig = false;
 
+    if (host_sig == host_interrupt_signal) {
+        ts->signal_pending = 1;
+        cpu_exit(thread_cpu);
+        return;
+    }
+
     /*
      * Non-spoofed SIGSEGV and SIGBUS are synchronous, and need special
      * handling wrt signal blocking and unwinding.
@@ -854,6 +862,9 @@ void signal_init(void)
 
     for (i = 1; i <= TARGET_NSIG; i++) {
         host_sig = target_to_host_signal(i);
+        if (host_sig == host_interrupt_signal) {
+            continue;
+        }
         sigaction(host_sig, NULL, &oact);
         if (oact.sa_sigaction == (void *)SIG_IGN) {
             sigact_table[i - 1]._sa_handler = TARGET_SIG_IGN;
@@ -872,6 +883,7 @@ void signal_init(void)
             sigaction(host_sig, &act, NULL);
         }
     }
+    sigaction(host_interrupt_signal, &act, NULL);
 }
 
 static void handle_pending_signal(CPUArchState *env, int sig,
diff --git a/linux-user/signal.c b/linux-user/signal.c
index bffbef235c..81a98c6d02 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -517,6 +517,8 @@ static int core_dump_signal(int sig)
     }
 }
 
+int host_interrupt_signal;
+
 static void signal_table_init(const char *rtsig_map)
 {
     int hsig, tsig, count;
@@ -580,10 +582,10 @@ static void signal_table_init(const char *rtsig_map)
          * Attempts for configure "missing" signals via sigaction will be
          * silently ignored.
          *
-         * Reserve one signal for internal usage (see below).
+         * Reserve two signals for internal usage (see below).
          */
 
-        hsig = SIGRTMIN + 1;
+        hsig = SIGRTMIN + 2;
         for (tsig = TARGET_SIGRTMIN;
              hsig <= SIGRTMAX && tsig <= TARGET_NSIG;
              hsig++, tsig++) {
@@ -604,12 +606,17 @@ static void signal_table_init(const char *rtsig_map)
     host_to_target_signal_table[SIGABRT] = 0;
     for (hsig = SIGRTMIN; hsig <= SIGRTMAX; hsig++) {
         if (!host_to_target_signal_table[hsig]) {
-            host_to_target_signal_table[hsig] = TARGET_SIGABRT;
-            break;
+            if (host_interrupt_signal) {
+                host_to_target_signal_table[hsig] = TARGET_SIGABRT;
+                break;
+            } else {
+                host_interrupt_signal = hsig;
+            }
         }
     }
     if (hsig > SIGRTMAX) {
-        fprintf(stderr, "No rt signals left for SIGABRT mapping\n");
+        fprintf(stderr,
+                "No rt signals left for interrupt and SIGABRT mapping\n");
         exit(EXIT_FAILURE);
     }
 
@@ -689,6 +696,8 @@ void signal_init(const char *rtsig_map)
         }
         sigact_table[tsig - 1]._sa_handler = thand;
     }
+
+    sigaction(host_interrupt_signal, &act, NULL);
 }
 
 /* Force a synchronously taken signal. The kernel force_sig() function
@@ -1036,6 +1045,12 @@ static void host_signal_handler(int host_sig, siginfo_t *info, void *puc)
     bool sync_sig = false;
     void *sigmask;
 
+    if (host_sig == host_interrupt_signal) {
+        ts->signal_pending = 1;
+        cpu_exit(thread_cpu);
+        return;
+    }
+
     /*
      * Non-spoofed SIGSEGV and SIGBUS are synchronous, and need special
      * handling wrt signal blocking and unwinding.  Non-spoofed SIGILL,
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 14/17] osdep: Introduce qemu_kill_thread()
  2025-02-07 15:30 [PATCH v2 00/17] maintainer updates for feb25 (qtest, gdbstub, plugins) pre-PR Alex Bennée
                   ` (12 preceding siblings ...)
  2025-02-07 15:31 ` [PATCH v2 13/17] user: Introduce host_interrupt_signal Alex Bennée
@ 2025-02-07 15:31 ` Alex Bennée
  2025-02-07 15:31 ` [PATCH v2 15/17] gdbstub: Allow late attachment Alex Bennée
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 28+ messages in thread
From: Alex Bennée @ 2025-02-07 15:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Xu, Alex Bennée, Thomas Huth, Weiwei Li, qemu-arm,
	Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu,
	Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv, Ilya Leoshkevich,
	Richard Henderson

From: Ilya Leoshkevich <iii@linux.ibm.com>

Add a function for sending signals to individual threads. It does not make
sense on Windows, so do not provide an implementation, so that if someone
uses it by accident, they will get a linker error.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Warner Losh <imp@bsdimp.com>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-Id: <20250117001542.8290-6-iii@linux.ibm.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/qemu/osdep.h |  9 +++++++++
 util/oslib-posix.c   | 15 +++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 112ebdff21..4397a90680 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -631,6 +631,15 @@ bool qemu_write_pidfile(const char *pidfile, Error **errp);
 
 int qemu_get_thread_id(void);
 
+/**
+ * qemu_kill_thread:
+ * @tid: thread id.
+ * @sig: host signal.
+ *
+ * Send @sig to one of QEMU's own threads with identifier @tid.
+ */
+int qemu_kill_thread(int tid, int sig);
+
 #ifndef CONFIG_IOVEC
 struct iovec {
     void *iov_base;
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 2bb34dade3..a697c602c6 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -111,6 +111,21 @@ int qemu_get_thread_id(void)
 #endif
 }
 
+int qemu_kill_thread(int tid, int sig)
+{
+#if defined(__linux__)
+    return syscall(__NR_tgkill, getpid(), tid, sig);
+#elif defined(__FreeBSD__)
+    return thr_kill2(getpid(), tid, sig);
+#elif defined(__NetBSD__)
+    return _lwp_kill(tid, sig);
+#elif defined(__OpenBSD__)
+    return thrkill(tid, sig, NULL);
+#else
+    return kill(tid, sig);
+#endif
+}
+
 int qemu_daemon(int nochdir, int noclose)
 {
     return daemon(nochdir, noclose);
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 15/17] gdbstub: Allow late attachment
  2025-02-07 15:30 [PATCH v2 00/17] maintainer updates for feb25 (qtest, gdbstub, plugins) pre-PR Alex Bennée
                   ` (13 preceding siblings ...)
  2025-02-07 15:31 ` [PATCH v2 14/17] osdep: Introduce qemu_kill_thread() Alex Bennée
@ 2025-02-07 15:31 ` Alex Bennée
  2025-02-07 15:31 ` [PATCH v2 16/17] docs/user: Document the %d placeholder and suspend=n QEMU_GDB features Alex Bennée
  2025-02-07 15:31 ` [PATCH v2 17/17] tests/tcg: Add late gdbstub attach test Alex Bennée
  16 siblings, 0 replies; 28+ messages in thread
From: Alex Bennée @ 2025-02-07 15:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Xu, Alex Bennée, Thomas Huth, Weiwei Li, qemu-arm,
	Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu,
	Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv, Ilya Leoshkevich

From: Ilya Leoshkevich <iii@linux.ibm.com>

Allow debugging individual processes in multi-process applications by
starting them with export QEMU_GDB=/tmp/qemu-%d.sock,suspend=n.
Currently one would have to attach to every process to ensure the app
makes progress.

In case suspend=n is not specified, the flow remains unchanged. If it
is specified, then accepting the client connection is delegated to a
thread. In the future this machinery may be reused for handling
reconnections and interruptions.

On accepting a connection, the thread schedules gdb_handlesig() on the
first CPU and wakes it up with host_interrupt_signal. Note that the
result of this gdb_handlesig() invocation is handled, as opposed to
many other existing call sites. These other call sites probably need to
be fixed separately.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-Id: <20250117001542.8290-7-iii@linux.ibm.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 bsd-user/main.c   |   1 -
 gdbstub/user.c    | 115 +++++++++++++++++++++++++++++++++++++++++-----
 linux-user/main.c |   1 -
 3 files changed, 103 insertions(+), 14 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index b2f6a9be2f..fdb160bed0 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -629,7 +629,6 @@ int main(int argc, char **argv)
 
     if (gdbstub) {
         gdbserver_start(gdbstub, &error_fatal);
-        gdb_handlesig(cpu, 0, NULL, NULL, 0);
     }
     cpu_loop(env);
     /* never exits */
diff --git a/gdbstub/user.c b/gdbstub/user.c
index 8225b70280..3730f32c41 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -22,6 +22,7 @@
 #include "gdbstub/user.h"
 #include "gdbstub/enums.h"
 #include "hw/core/cpu.h"
+#include "user/signal.h"
 #include "trace.h"
 #include "internals.h"
 
@@ -393,32 +394,122 @@ static int gdbserver_open_port(int port, Error **errp)
     return fd;
 }
 
-bool gdbserver_start(const char *port_or_path, Error **errp)
+static bool gdbserver_accept(int port, int gdb_fd, const char *path)
 {
-    int port = g_ascii_strtoull(port_or_path, NULL, 10);
+    bool ret;
+
+    if (port > 0) {
+        ret = gdb_accept_tcp(gdb_fd);
+    } else {
+        ret = gdb_accept_socket(gdb_fd);
+        if (ret) {
+            gdbserver_user_state.socket_path = g_strdup(path);
+        }
+    }
+
+    if (!ret) {
+        close(gdb_fd);
+    }
+
+    return ret;
+}
+
+struct {
+    int port;
     int gdb_fd;
+    char *path;
+} gdbserver_args;
+
+static void do_gdb_handlesig(CPUState *cs, run_on_cpu_data arg)
+{
+    int sig;
+
+    sig = target_to_host_signal(gdb_handlesig(cs, 0, NULL, NULL, 0));
+    if (sig >= 1 && sig < NSIG) {
+        qemu_kill_thread(gdb_get_cpu_index(cs), sig);
+    }
+}
+
+static void *gdbserver_accept_thread(void *arg)
+{
+    if (gdbserver_accept(gdbserver_args.port, gdbserver_args.gdb_fd,
+                         gdbserver_args.path)) {
+        CPUState *cs = first_cpu;
+
+        async_safe_run_on_cpu(cs, do_gdb_handlesig, RUN_ON_CPU_NULL);
+        qemu_kill_thread(gdb_get_cpu_index(cs), host_interrupt_signal);
+    }
+
+    g_free(gdbserver_args.path);
+    gdbserver_args.path = NULL;
+
+    return NULL;
+}
 
+#define USAGE "\nUsage: -g {port|path}[,suspend={y|n}]"
+
+bool gdbserver_start(const char *args, Error **errp)
+{
+    g_auto(GStrv) argv = g_strsplit(args, ",", 0);
+    const char *port_or_path = NULL;
+    bool suspend = true;
+    int gdb_fd, port;
+    GStrv arg;
+
+    for (arg = argv; *arg; arg++) {
+        g_auto(GStrv) tokens = g_strsplit(*arg, "=", 2);
+
+        if (g_strcmp0(tokens[0], "suspend") == 0) {
+            if (tokens[1] == NULL) {
+                error_setg(errp,
+                           "gdbstub: missing \"suspend\" option value" USAGE);
+                return false;
+            } else if (!qapi_bool_parse(tokens[0], tokens[1],
+                                        &suspend, errp)) {
+                return false;
+            }
+        } else {
+            if (port_or_path) {
+                error_setg(errp, "gdbstub: unknown option \"%s\"" USAGE, *arg);
+                return false;
+            }
+            port_or_path = *arg;
+        }
+    }
+    if (!port_or_path) {
+        error_setg(errp, "gdbstub: port or path not specified" USAGE);
+        return false;
+    }
+
+    port = g_ascii_strtoull(port_or_path, NULL, 10);
     if (port > 0) {
         gdb_fd = gdbserver_open_port(port, errp);
     } else {
         gdb_fd = gdbserver_open_socket(port_or_path, errp);
     }
-
     if (gdb_fd < 0) {
         return false;
     }
 
-    if (port > 0 && gdb_accept_tcp(gdb_fd)) {
-        return true;
-    } else if (gdb_accept_socket(gdb_fd)) {
-        gdbserver_user_state.socket_path = g_strdup(port_or_path);
+    if (suspend) {
+        if (gdbserver_accept(port, gdb_fd, port_or_path)) {
+            gdb_handlesig(first_cpu, 0, NULL, NULL, 0);
+            return true;
+        } else {
+            error_setg(errp, "gdbstub: failed to accept connection");
+            return false;
+        }
+    } else {
+        QemuThread thread;
+
+        gdbserver_args.port = port;
+        gdbserver_args.gdb_fd = gdb_fd;
+        gdbserver_args.path = g_strdup(port_or_path);
+        qemu_thread_create(&thread, "gdb-accept",
+                           &gdbserver_accept_thread, NULL,
+                           QEMU_THREAD_DETACHED);
         return true;
     }
-
-    /* gone wrong */
-    close(gdb_fd);
-    error_setg(errp, "gdbstub: failed to accept connection");
-    return false;
 }
 
 void gdbserver_fork_start(void)
diff --git a/linux-user/main.c b/linux-user/main.c
index 7198fa0986..5c74c52cc5 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -1024,7 +1024,6 @@ int main(int argc, char **argv, char **envp)
 
     if (gdbstub) {
         gdbserver_start(gdbstub, &error_fatal);
-        gdb_handlesig(cpu, 0, NULL, NULL, 0);
     }
 
 #ifdef CONFIG_SEMIHOSTING
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 16/17] docs/user: Document the %d placeholder and suspend=n QEMU_GDB features
  2025-02-07 15:30 [PATCH v2 00/17] maintainer updates for feb25 (qtest, gdbstub, plugins) pre-PR Alex Bennée
                   ` (14 preceding siblings ...)
  2025-02-07 15:31 ` [PATCH v2 15/17] gdbstub: Allow late attachment Alex Bennée
@ 2025-02-07 15:31 ` Alex Bennée
  2025-02-07 15:31 ` [PATCH v2 17/17] tests/tcg: Add late gdbstub attach test Alex Bennée
  16 siblings, 0 replies; 28+ messages in thread
From: Alex Bennée @ 2025-02-07 15:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Xu, Alex Bennée, Thomas Huth, Weiwei Li, qemu-arm,
	Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu,
	Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv, Ilya Leoshkevich

From: Ilya Leoshkevich <iii@linux.ibm.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-Id: <20250117001542.8290-8-iii@linux.ibm.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 docs/user/main.rst | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/docs/user/main.rst b/docs/user/main.rst
index 80a77f0a0c..9a1c60448c 100644
--- a/docs/user/main.rst
+++ b/docs/user/main.rst
@@ -54,7 +54,7 @@ Command line options
 
 ::
 
-   qemu-i386 [-h] [-d] [-L path] [-s size] [-cpu model] [-g port] [-B offset] [-R size] program [arguments...]
+   qemu-i386 [-h] [-d] [-L path] [-s size] [-cpu model] [-g endpoint] [-B offset] [-R size] program [arguments...]
 
 ``-h``
    Print the help
@@ -91,8 +91,18 @@ Debug options:
    Activate logging of the specified items (use '-d help' for a list of
    log items)
 
-``-g port``
-   Wait gdb connection to port
+``-g endpoint``
+   Wait gdb connection to a port (e.g., ``1234``) or a unix socket (e.g.,
+   ``/tmp/qemu.sock``).
+
+   If a unix socket path contains single ``%d`` placeholder (e.g.,
+   ``/tmp/qemu-%d.sock``), it is replaced by the emulator PID, which is useful
+   when passing this option via the ``QEMU_GDB`` environment variable to a
+   multi-process application.
+
+   If the endpoint address is followed by ``,suspend=n`` (e.g.,
+   ``1234,suspend=n``), then the emulated program starts without waiting for a
+   connection, which can be established at any later point in time.
 
 ``-one-insn-per-tb``
    Run the emulation with one guest instruction per translation block.
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 17/17] tests/tcg: Add late gdbstub attach test
  2025-02-07 15:30 [PATCH v2 00/17] maintainer updates for feb25 (qtest, gdbstub, plugins) pre-PR Alex Bennée
                   ` (15 preceding siblings ...)
  2025-02-07 15:31 ` [PATCH v2 16/17] docs/user: Document the %d placeholder and suspend=n QEMU_GDB features Alex Bennée
@ 2025-02-07 15:31 ` Alex Bennée
  16 siblings, 0 replies; 28+ messages in thread
From: Alex Bennée @ 2025-02-07 15:31 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Xu, Alex Bennée, Thomas Huth, Weiwei Li, qemu-arm,
	Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu,
	Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv, Ilya Leoshkevich

From: Ilya Leoshkevich <iii@linux.ibm.com>

Add a small test to prevent regressions.
Make sure that host_interrupt_signal is not visible to the guest.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-Id: <20250117001542.8290-9-iii@linux.ibm.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/tcg/multiarch/late-attach.c          | 41 ++++++++++++++++++++++
 tests/guest-debug/run-test.py              | 15 ++++++--
 tests/tcg/multiarch/Makefile.target        |  9 ++++-
 tests/tcg/multiarch/gdbstub/late-attach.py | 28 +++++++++++++++
 4 files changed, 90 insertions(+), 3 deletions(-)
 create mode 100644 tests/tcg/multiarch/late-attach.c
 create mode 100644 tests/tcg/multiarch/gdbstub/late-attach.py

diff --git a/tests/tcg/multiarch/late-attach.c b/tests/tcg/multiarch/late-attach.c
new file mode 100644
index 0000000000..20a364034b
--- /dev/null
+++ b/tests/tcg/multiarch/late-attach.c
@@ -0,0 +1,41 @@
+/*
+ * Test attaching GDB to a running process.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include <assert.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+static const char *phase = "start";
+
+int main(void)
+{
+    sigset_t set;
+    int sig;
+
+    assert(sigfillset(&set) == 0);
+    assert(sigprocmask(SIG_BLOCK, &set, NULL) == 0);
+
+    /* Let GDB know it can send SIGUSR1. */
+    phase = "sigwait";
+    if (getenv("LATE_ATTACH_PY")) {
+        assert(sigwait(&set, &sig) == 0);
+        if (sig != SIGUSR1) {
+            fprintf(stderr, "Unexpected signal %d\n", sig);
+            return EXIT_FAILURE;
+        }
+    }
+
+    /* Check that the guest does not see host_interrupt_signal. */
+    assert(sigpending(&set) == 0);
+    for (sig = 1; sig < NSIG; sig++) {
+        if (sigismember(&set, sig)) {
+            fprintf(stderr, "Unexpected signal %d\n", sig);
+            return EXIT_FAILURE;
+        }
+    }
+
+    return EXIT_SUCCESS;
+}
diff --git a/tests/guest-debug/run-test.py b/tests/guest-debug/run-test.py
index 5a091db8be..75e9c92e03 100755
--- a/tests/guest-debug/run-test.py
+++ b/tests/guest-debug/run-test.py
@@ -36,6 +36,8 @@ def get_args():
     parser.add_argument("--gdb-args", help="Additional gdb arguments")
     parser.add_argument("--output", help="A file to redirect output to")
     parser.add_argument("--stderr", help="A file to redirect stderr to")
+    parser.add_argument("--no-suspend", action="store_true",
+                        help="Ask the binary to not wait for GDB connection")
 
     return parser.parse_args()
 
@@ -73,10 +75,19 @@ def log(output, msg):
 
     # Launch QEMU with binary
     if "system" in args.qemu:
+        if args.no_suspend:
+            suspend = ''
+        else:
+            suspend = ' -S'
         cmd = f'{args.qemu} {args.qargs} {args.binary}' \
-            f' -S -gdb unix:path={socket_name},server=on'
+            f'{suspend} -gdb unix:path={socket_name},server=on'
     else:
-        cmd = f'{args.qemu} {args.qargs} -g {socket_name} {args.binary}'
+        if args.no_suspend:
+            suspend = ',suspend=n'
+        else:
+            suspend = ''
+        cmd = f'{args.qemu} {args.qargs} -g {socket_name}{suspend}' \
+            f' {args.binary}'
 
     log(output, "QEMU CMD: %s" % (cmd))
     inferior = subprocess.Popen(shlex.split(cmd))
diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target
index 18d3cf4ae0..688a6be203 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -130,6 +130,13 @@ run-gdbstub-follow-fork-mode-parent: follow-fork-mode
 		--bin $< --test $(MULTIARCH_SRC)/gdbstub/follow-fork-mode-parent.py, \
 	following parents on fork)
 
+run-gdbstub-late-attach: late-attach
+	$(call run-test, $@, env LATE_ATTACH_PY=1 $(GDB_SCRIPT) \
+		--gdb $(GDB) \
+		--qemu $(QEMU) --qargs "$(QEMU_OPTS)" --no-suspend \
+		--bin $< --test $(MULTIARCH_SRC)/gdbstub/late-attach.py, \
+	attaching to a running process)
+
 else
 run-gdbstub-%:
 	$(call skip-test, "gdbstub test $*", "need working gdb with $(patsubst -%,,$(TARGET_NAME)) support")
@@ -139,7 +146,7 @@ EXTRA_RUNS += run-gdbstub-sha1 run-gdbstub-qxfer-auxv-read \
 	      run-gdbstub-registers run-gdbstub-prot-none \
 	      run-gdbstub-catch-syscalls run-gdbstub-follow-fork-mode-child \
 	      run-gdbstub-follow-fork-mode-parent \
-	      run-gdbstub-qxfer-siginfo-read
+	      run-gdbstub-qxfer-siginfo-read run-gdbstub-late-attach
 
 # ARM Compatible Semi Hosting Tests
 #
diff --git a/tests/tcg/multiarch/gdbstub/late-attach.py b/tests/tcg/multiarch/gdbstub/late-attach.py
new file mode 100644
index 0000000000..1d40efb5ec
--- /dev/null
+++ b/tests/tcg/multiarch/gdbstub/late-attach.py
@@ -0,0 +1,28 @@
+"""Test attaching GDB to a running process.
+
+SPDX-License-Identifier: GPL-2.0-or-later
+"""
+from test_gdbstub import main, report
+
+
+def run_test():
+    """Run through the tests one by one"""
+    try:
+        phase = gdb.parse_and_eval("phase").string()
+    except gdb.error:
+        # Assume the guest did not reach main().
+        phase = "start"
+
+    if phase == "start":
+        gdb.execute("break sigwait")
+        gdb.execute("continue")
+        phase = gdb.parse_and_eval("phase").string()
+    report(phase == "sigwait", "{} == \"sigwait\"".format(phase))
+
+    gdb.execute("signal SIGUSR1")
+
+    exitcode = int(gdb.parse_and_eval("$_exitcode"))
+    report(exitcode == 0, "{} == 0".format(exitcode))
+
+
+main(run_test)
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 02/17] tests/qtest: don't attempt to clock_step while waiting for virtio ISR
  2025-02-07 15:30 ` [PATCH v2 02/17] tests/qtest: don't attempt to clock_step while waiting for virtio ISR Alex Bennée
@ 2025-02-07 16:02   ` Thomas Huth
  0 siblings, 0 replies; 28+ messages in thread
From: Thomas Huth @ 2025-02-07 16:02 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Peter Xu, Weiwei Li, qemu-arm, Nicholas Piggin, Liu Zhiwei,
	Bin Meng, Tyrone Ting, Hao Wu, Kyle Evans, Alistair Francis,
	Philippe Mathieu-Daudé, Laurent Vivier, Riku Voipio,
	Harsh Prateek Bora, Fabiano Rosas, Alexandre Iooss,
	Laurent Vivier, Daniel Henrique Barboza, Warner Losh,
	Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour, qemu-ppc,
	Paolo Bonzini, qemu-rust, qemu-riscv

On 07/02/2025 16.30, Alex Bennée wrote:
> This replicates the changes from 92cb8f8bf6 (tests/qtest: remove
> clock_steps from virtio tests) as there are no timers in the virtio
> code. We still busy wait and timeout though.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
> v2
>    - fix trailing space
> ---
>   tests/qtest/libqos/virtio-pci-modern.c | 6 ++----
>   tests/qtest/libqos/virtio-pci.c        | 6 ++----
>   2 files changed, 4 insertions(+), 8 deletions(-)

I wonder whether a usleep(10000) in the loop might help to avoid burning too 
much CPU time in there?

Anyway,
Reviewed-by: Thomas Huth <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 05/17] tests/qtest: rename qtest_send_prefix and roll-up into qtest_send
  2025-02-07 15:31 ` [PATCH v2 05/17] tests/qtest: rename qtest_send_prefix and roll-up into qtest_send Alex Bennée
@ 2025-02-07 16:04   ` Thomas Huth
  0 siblings, 0 replies; 28+ messages in thread
From: Thomas Huth @ 2025-02-07 16:04 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Peter Xu, Weiwei Li, qemu-arm, Nicholas Piggin, Liu Zhiwei,
	Bin Meng, Tyrone Ting, Hao Wu, Kyle Evans, Alistair Francis,
	Philippe Mathieu-Daudé, Laurent Vivier, Riku Voipio,
	Harsh Prateek Bora, Fabiano Rosas, Alexandre Iooss,
	Laurent Vivier, Daniel Henrique Barboza, Warner Losh,
	Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour, qemu-ppc,
	Paolo Bonzini, qemu-rust, qemu-riscv

On 07/02/2025 16.31, Alex Bennée wrote:
> qtest_send_prefix never actually sent something over the chardev, all
> it does is print the timestamp to the QTEST_LOG when enabled. So
> rename the function, make it static, remove the unused CharDev and
> simplify all the call sites by handling that directly with
> qtest_send (and qtest_log_send).
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   include/system/qtest.h |  1 -
>   hw/ppc/spapr_rtas.c    |  1 -
>   hw/riscv/riscv_hart.c  |  1 -
>   system/qtest.c         | 26 +++-----------------------
>   4 files changed, 3 insertions(+), 26 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 11/17] gdbstub: Try unlinking the unix socket before binding
  2025-02-07 15:31 ` [PATCH v2 11/17] gdbstub: Try unlinking the unix socket before binding Alex Bennée
@ 2025-05-20 14:50   ` Michael Tokarev
  2025-05-20 15:52     ` Alex Bennée
  2025-05-20 22:21     ` Ilya Leoshkevich
  0 siblings, 2 replies; 28+ messages in thread
From: Michael Tokarev @ 2025-05-20 14:50 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Peter Xu, Thomas Huth, Weiwei Li, qemu-arm, Nicholas Piggin,
	Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu, Kyle Evans,
	Alistair Francis, Philippe Mathieu-Daudé, Laurent Vivier,
	Riku Voipio, Harsh Prateek Bora, Fabiano Rosas, Alexandre Iooss,
	Laurent Vivier, Daniel Henrique Barboza, Warner Losh,
	Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour, qemu-ppc,
	Paolo Bonzini, qemu-rust, qemu-riscv, Ilya Leoshkevich

07.02.2025 18:31, Alex Bennée wrote:
> From: Ilya Leoshkevich <iii@linux.ibm.com>
> 
> In case an emulated process execve()s another emulated process, bind()
> will fail, because the socket already exists. So try deleting it. Use
> the existing unix_listen() function which does this. Link qemu-user
> with qemu-sockets.c and add the monitor_get_fd() stub.
> 
> Note that it is not possible to handle this in do_execv(): deleting
> gdbserver_user_state.socket_path before safe_execve() is not correct,
> because the latter may fail, and afterwards we may lose control.

Please note: this is linux-user stuff, which is usually linked statically.
By linking it with qemu-sockets, we basically broke static linking, because
qemu-sockets uses getaddrinfo() &Co.  The previous code, I think, was there
for a reason, - to avoid this linkage.

How do you think about reverting this one and addressing the original
problem without using qemu-sockets?

Alternatively, it might be possible to split qemu-sockets.c into unix-related
stuff and generic stuff.

Thanks,

/mjt


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 11/17] gdbstub: Try unlinking the unix socket before binding
  2025-05-20 14:50   ` Michael Tokarev
@ 2025-05-20 15:52     ` Alex Bennée
  2025-05-20 16:01       ` Peter Maydell
  2025-05-20 22:21     ` Ilya Leoshkevich
  1 sibling, 1 reply; 28+ messages in thread
From: Alex Bennée @ 2025-05-20 15:52 UTC (permalink / raw)
  To: Michael Tokarev
  Cc: qemu-devel, Peter Xu, Thomas Huth, Weiwei Li, qemu-arm,
	Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu,
	Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv, Ilya Leoshkevich

Michael Tokarev <mjt@tls.msk.ru> writes:

> 07.02.2025 18:31, Alex Bennée wrote:
>> From: Ilya Leoshkevich <iii@linux.ibm.com>
>> In case an emulated process execve()s another emulated process,
>> bind()
>> will fail, because the socket already exists. So try deleting it. Use
>> the existing unix_listen() function which does this. Link qemu-user
>> with qemu-sockets.c and add the monitor_get_fd() stub.
>> Note that it is not possible to handle this in do_execv(): deleting
>> gdbserver_user_state.socket_path before safe_execve() is not correct,
>> because the latter may fail, and afterwards we may lose control.
>
> Please note: this is linux-user stuff, which is usually linked statically.
> By linking it with qemu-sockets, we basically broke static linking, because
> qemu-sockets uses getaddrinfo() &Co.  The previous code, I think, was there
> for a reason, - to avoid this linkage.

Oops, how come we didn't notice? We do have a bunch of --static targets
in the CI.

> How do you think about reverting this one and addressing the original
> problem without using qemu-sockets?
>
> Alternatively, it might be possible to split qemu-sockets.c into unix-related
> stuff and generic stuff.

You mean move all the unix_XXX() functions into a new unit that ensures
we don't need getaddrinfo()?

>
> Thanks,
>
> /mjt

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 11/17] gdbstub: Try unlinking the unix socket before binding
  2025-05-20 15:52     ` Alex Bennée
@ 2025-05-20 16:01       ` Peter Maydell
  0 siblings, 0 replies; 28+ messages in thread
From: Peter Maydell @ 2025-05-20 16:01 UTC (permalink / raw)
  To: Alex Bennée
  Cc: Michael Tokarev, qemu-devel, Peter Xu, Thomas Huth, Weiwei Li,
	qemu-arm, Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting,
	Hao Wu, Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv, Ilya Leoshkevich

On Tue, 20 May 2025 at 16:53, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Michael Tokarev <mjt@tls.msk.ru> writes:
>
> > 07.02.2025 18:31, Alex Bennée wrote:
> >> From: Ilya Leoshkevich <iii@linux.ibm.com>
> >> In case an emulated process execve()s another emulated process,
> >> bind()
> >> will fail, because the socket already exists. So try deleting it. Use
> >> the existing unix_listen() function which does this. Link qemu-user
> >> with qemu-sockets.c and add the monitor_get_fd() stub.
> >> Note that it is not possible to handle this in do_execv(): deleting
> >> gdbserver_user_state.socket_path before safe_execve() is not correct,
> >> because the latter may fail, and afterwards we may lose control.
> >
> > Please note: this is linux-user stuff, which is usually linked statically.
> > By linking it with qemu-sockets, we basically broke static linking, because
> > qemu-sockets uses getaddrinfo() &Co.  The previous code, I think, was there
> > for a reason, - to avoid this linkage.
>
> Oops, how come we didn't notice? We do have a bunch of --static targets
> in the CI.

We ignore these warnings because upstream glib means we
always have a bunch of false positives when we link a static
qemu-user executable. So this one (which is in our own code,
not in glib) slipped through.

I think it's in the same "annoying noise" category as the
glib ones, because the unix_listen() function won't actually
end up in a call to getaddrinfo(). But since this one's in our
own code we ought to fix it, either by splitting the source
file or by reverting the commit.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 11/17] gdbstub: Try unlinking the unix socket before binding
  2025-05-20 14:50   ` Michael Tokarev
  2025-05-20 15:52     ` Alex Bennée
@ 2025-05-20 22:21     ` Ilya Leoshkevich
  2025-05-21  6:22       ` Alex Bennée
  2025-05-21 15:34       ` Peter Maydell
  1 sibling, 2 replies; 28+ messages in thread
From: Ilya Leoshkevich @ 2025-05-20 22:21 UTC (permalink / raw)
  To: Michael Tokarev
  Cc: Alex Bennée, qemu-devel, Peter Xu, Thomas Huth, Weiwei Li,
	qemu-arm, Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting,
	Hao Wu, Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv

On 2025-05-20 15:50, Michael Tokarev wrote:
> 07.02.2025 18:31, Alex Bennée wrote:
>> From: Ilya Leoshkevich <iii@linux.ibm.com>
>> 
>> In case an emulated process execve()s another emulated process, bind()
>> will fail, because the socket already exists. So try deleting it. Use
>> the existing unix_listen() function which does this. Link qemu-user
>> with qemu-sockets.c and add the monitor_get_fd() stub.
>> 
>> Note that it is not possible to handle this in do_execv(): deleting
>> gdbserver_user_state.socket_path before safe_execve() is not correct,
>> because the latter may fail, and afterwards we may lose control.
> 
> Please note: this is linux-user stuff, which is usually linked 
> statically.
> By linking it with qemu-sockets, we basically broke static linking, 
> because
> qemu-sockets uses getaddrinfo() &Co.  The previous code, I think, was 
> there
> for a reason, - to avoid this linkage.
> 
> How do you think about reverting this one and addressing the original
> problem without using qemu-sockets?
> 
> Alternatively, it might be possible to split qemu-sockets.c into 
> unix-related
> stuff and generic stuff.
> 
> Thanks,
> 
> /mjt

I can split it.

However, wasn't it already broken in this regard?
With fccb744f41c69fec6fd92225fe907c6e69de5d44^ I get:

[2/2] Linking target qemu-s390x
/usr/bin/ld: /usr/lib64/libglib-2.0.a(gutils.c.o): in function 
`g_get_user_database_entry':
(.text+0xeb): warning: Using 'getpwnam_r' in statically linked 
applications requires at runtime the shared libraries from the glibc 
version used for linking
/usr/bin/ld: (.text+0x2be): warning: Using 'getpwuid' in statically 
linked applications requires at runtime the shared libraries from the 
glibc version used for linking
/usr/bin/ld: (.text+0x134): warning: Using 'getpwuid_r' in statically 
linked applications requires at runtime the shared libraries from the 
glibc version used for linking

This comes from glib, but the ultimate result is still the same.

Also, what are the symptoms of the breakage? IIUC as long as execution
does not reach getaddrinfo(), which it in this case should not, because
it is used only on inet paths, there should not be any issues, right?

In any case, the new warning is annoying and I better fix it for this
reason alone.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 11/17] gdbstub: Try unlinking the unix socket before binding
  2025-05-20 22:21     ` Ilya Leoshkevich
@ 2025-05-21  6:22       ` Alex Bennée
  2025-05-21 15:34       ` Peter Maydell
  1 sibling, 0 replies; 28+ messages in thread
From: Alex Bennée @ 2025-05-21  6:22 UTC (permalink / raw)
  To: Ilya Leoshkevich
  Cc: Michael Tokarev, qemu-devel, Peter Xu, Thomas Huth, Weiwei Li,
	qemu-arm, Nicholas Piggin, Liu Zhiwei, Bin Meng, Tyrone Ting,
	Hao Wu, Kyle Evans, Alistair Francis, Philippe Mathieu-Daudé,
	Laurent Vivier, Riku Voipio, Harsh Prateek Bora, Fabiano Rosas,
	Alexandre Iooss, Laurent Vivier, Daniel Henrique Barboza,
	Warner Losh, Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour,
	qemu-ppc, Paolo Bonzini, qemu-rust, qemu-riscv

Ilya Leoshkevich <iii@linux.ibm.com> writes:

> On 2025-05-20 15:50, Michael Tokarev wrote:
>> 07.02.2025 18:31, Alex Bennée wrote:
>>> From: Ilya Leoshkevich <iii@linux.ibm.com>
>>> In case an emulated process execve()s another emulated process,
>>> bind()
>>> will fail, because the socket already exists. So try deleting it. Use
>>> the existing unix_listen() function which does this. Link qemu-user
>>> with qemu-sockets.c and add the monitor_get_fd() stub.
>>> Note that it is not possible to handle this in do_execv(): deleting
>>> gdbserver_user_state.socket_path before safe_execve() is not correct,
>>> because the latter may fail, and afterwards we may lose control.
>> Please note: this is linux-user stuff, which is usually linked
>> statically.
>> By linking it with qemu-sockets, we basically broke static linking,
>> because
>> qemu-sockets uses getaddrinfo() &Co.  The previous code, I think,
>> was there
>> for a reason, - to avoid this linkage.
>> How do you think about reverting this one and addressing the
>> original
>> problem without using qemu-sockets?
>> Alternatively, it might be possible to split qemu-sockets.c into
>> unix-related
>> stuff and generic stuff.
>> Thanks,
>> /mjt
>
> I can split it.
>
> However, wasn't it already broken in this regard?
> With fccb744f41c69fec6fd92225fe907c6e69de5d44^ I get:
>
> [2/2] Linking target qemu-s390x
> /usr/bin/ld: /usr/lib64/libglib-2.0.a(gutils.c.o): in function
> `g_get_user_database_entry':
> (.text+0xeb): warning: Using 'getpwnam_r' in statically linked
> applications requires at runtime the shared libraries from the glibc
> version used for linking
> /usr/bin/ld: (.text+0x2be): warning: Using 'getpwuid' in statically
> linked applications requires at runtime the shared libraries from the
> glibc version used for linking
> /usr/bin/ld: (.text+0x134): warning: Using 'getpwuid_r' in statically
> linked applications requires at runtime the shared libraries from the
> glibc version used for linking
>
> This comes from glib, but the ultimate result is still the same.
>
> Also, what are the symptoms of the breakage? IIUC as long as execution
> does not reach getaddrinfo(), which it in this case should not, because
> it is used only on inet paths, there should not be any issues, right?
>
> In any case, the new warning is annoying and I better fix it for this
> reason alone.

I sent:

  Message-Id: <20250520165706.3976971-1-alex.bennee@linaro.org>
  Date: Tue, 20 May 2025 17:57:06 +0100
  Subject: [RFC PATCH] util: split unix socket functions out of qemu-sockets
  From: =?UTF-8?q?Alex=20Benn=C3=A9e?= <alex.bennee@linaro.org>

Although I'm still seeing some warnings on my build:

  # QEMU configure log Tue 20 May 18:14:18 BST 2025
  # Configured with: '../../configure' '--disable-docs' '--disable-tools' '--disable-system' '--disable-vhost-kernel' '--disable-vhost-user' '--disable-vhost-net' '--static'

Gives:

  [142/142] Linking target qemu-xtensaeb
  /usr/bin/ld: /usr/lib/x86_64-linux-gnu/libglib-2.0.a(gutils.c.o): in function `g_get_user_database_entry':
  (.text+0x287): warning: Using 'getpwuid' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
  /usr/bin/ld: (.text+0xe0): warning: Using 'getpwnam_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
  /usr/bin/ld: (.text+0x11e): warning: Using 'getpwuid_r' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 11/17] gdbstub: Try unlinking the unix socket before binding
  2025-05-20 22:21     ` Ilya Leoshkevich
  2025-05-21  6:22       ` Alex Bennée
@ 2025-05-21 15:34       ` Peter Maydell
  2025-05-21 15:37         ` Daniel P. Berrangé
  1 sibling, 1 reply; 28+ messages in thread
From: Peter Maydell @ 2025-05-21 15:34 UTC (permalink / raw)
  To: Ilya Leoshkevich
  Cc: Michael Tokarev, Alex Bennée, qemu-devel, Peter Xu,
	Thomas Huth, Weiwei Li, qemu-arm, Nicholas Piggin, Liu Zhiwei,
	Bin Meng, Tyrone Ting, Hao Wu, Kyle Evans, Alistair Francis,
	Philippe Mathieu-Daudé, Laurent Vivier, Riku Voipio,
	Harsh Prateek Bora, Fabiano Rosas, Alexandre Iooss,
	Laurent Vivier, Daniel Henrique Barboza, Warner Losh,
	Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour, qemu-ppc,
	Paolo Bonzini, qemu-rust, qemu-riscv

On Tue, 20 May 2025 at 23:22, Ilya Leoshkevich <iii@linux.ibm.com> wrote:
> However, wasn't it already broken in this regard?
> With fccb744f41c69fec6fd92225fe907c6e69de5d44^ I get:
>
> [2/2] Linking target qemu-s390x
> /usr/bin/ld: /usr/lib64/libglib-2.0.a(gutils.c.o): in function
> `g_get_user_database_entry':
> (.text+0xeb): warning: Using 'getpwnam_r' in statically linked
> applications requires at runtime the shared libraries from the glibc
> version used for linking
> /usr/bin/ld: (.text+0x2be): warning: Using 'getpwuid' in statically
> linked applications requires at runtime the shared libraries from the
> glibc version used for linking
> /usr/bin/ld: (.text+0x134): warning: Using 'getpwuid_r' in statically
> linked applications requires at runtime the shared libraries from the
> glibc version used for linking
>
> This comes from glib, but the ultimate result is still the same.

Those are in upstream glib, as you note. We can't fix those (unless we
have the enthusiasm to write patches for upstream glib: last time
we asked, they were not against the idea, but nobody on either side
had the time available to try to write the necessary patches).
But we can and should fix the cases in our own code.

> Also, what are the symptoms of the breakage? IIUC as long as execution
> does not reach getaddrinfo(), which it in this case should not, because
> it is used only on inet paths, there should not be any issues, right?

Correct -- if we don't call the function it's fine. But the easiest
way to be sure we don't call the function is to not link it in :-)
Otherwise future code changes could result in a call without our
realizing it.

Also, mjt's packaging for Debian puts in some stubs for the
offending getwpuid etc functions, which suppress the glib warnings
(this is why he noticed this whereas none of the rest of us did):

https://sources.debian.org/patches/qemu/1:10.0.0%2Bds-2/static-linux-user-stubs.diff/

thanks
-- PMM


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 11/17] gdbstub: Try unlinking the unix socket before binding
  2025-05-21 15:34       ` Peter Maydell
@ 2025-05-21 15:37         ` Daniel P. Berrangé
  2025-05-21 15:41           ` Peter Maydell
  0 siblings, 1 reply; 28+ messages in thread
From: Daniel P. Berrangé @ 2025-05-21 15:37 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Ilya Leoshkevich, Michael Tokarev, Alex Bennée, qemu-devel,
	Peter Xu, Thomas Huth, Weiwei Li, qemu-arm, Nicholas Piggin,
	Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu, Kyle Evans,
	Alistair Francis, Philippe Mathieu-Daudé, Laurent Vivier,
	Riku Voipio, Harsh Prateek Bora, Fabiano Rosas, Alexandre Iooss,
	Laurent Vivier, Daniel Henrique Barboza, Warner Losh,
	Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour, qemu-ppc,
	Paolo Bonzini, qemu-rust, qemu-riscv

On Wed, May 21, 2025 at 04:34:24PM +0100, Peter Maydell wrote:
> On Tue, 20 May 2025 at 23:22, Ilya Leoshkevich <iii@linux.ibm.com> wrote:
> > However, wasn't it already broken in this regard?
> > With fccb744f41c69fec6fd92225fe907c6e69de5d44^ I get:
> >
> > [2/2] Linking target qemu-s390x
> > /usr/bin/ld: /usr/lib64/libglib-2.0.a(gutils.c.o): in function
> > `g_get_user_database_entry':
> > (.text+0xeb): warning: Using 'getpwnam_r' in statically linked
> > applications requires at runtime the shared libraries from the glibc
> > version used for linking
> > /usr/bin/ld: (.text+0x2be): warning: Using 'getpwuid' in statically
> > linked applications requires at runtime the shared libraries from the
> > glibc version used for linking
> > /usr/bin/ld: (.text+0x134): warning: Using 'getpwuid_r' in statically
> > linked applications requires at runtime the shared libraries from the
> > glibc version used for linking
> >
> > This comes from glib, but the ultimate result is still the same.
> 
> Those are in upstream glib, as you note. We can't fix those (unless we
> have the enthusiasm to write patches for upstream glib: last time
> we asked, they were not against the idea, but nobody on either side
> had the time available to try to write the necessary patches).
> But we can and should fix the cases in our own code.
> 
> > Also, what are the symptoms of the breakage? IIUC as long as execution
> > does not reach getaddrinfo(), which it in this case should not, because
> > it is used only on inet paths, there should not be any issues, right?
> 
> Correct -- if we don't call the function it's fine. But the easiest
> way to be sure we don't call the function is to not link it in :-)
> Otherwise future code changes could result in a call without our
> realizing it.
> 
> Also, mjt's packaging for Debian puts in some stubs for the
> offending getwpuid etc functions, which suppress the glib warnings
> (this is why he noticed this whereas none of the rest of us did):
> 
> https://sources.debian.org/patches/qemu/1:10.0.0%2Bds-2/static-linux-user-stubs.diff/

Oh interesting, I was conincidentally wondering if adding dummy
stubs might suppress this.

How about we pull that change upstream, and expand it to getaddrinfo
too ?  I like the stubs much more than artificially splitting up
the source files


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 11/17] gdbstub: Try unlinking the unix socket before binding
  2025-05-21 15:37         ` Daniel P. Berrangé
@ 2025-05-21 15:41           ` Peter Maydell
  0 siblings, 0 replies; 28+ messages in thread
From: Peter Maydell @ 2025-05-21 15:41 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Ilya Leoshkevich, Michael Tokarev, Alex Bennée, qemu-devel,
	Peter Xu, Thomas Huth, Weiwei Li, qemu-arm, Nicholas Piggin,
	Liu Zhiwei, Bin Meng, Tyrone Ting, Hao Wu, Kyle Evans,
	Alistair Francis, Philippe Mathieu-Daudé, Laurent Vivier,
	Riku Voipio, Harsh Prateek Bora, Fabiano Rosas, Alexandre Iooss,
	Laurent Vivier, Daniel Henrique Barboza, Warner Losh,
	Pierrick Bouvier, Palmer Dabbelt, Mahmoud Mandour, qemu-ppc,
	Paolo Bonzini, qemu-rust, qemu-riscv

On Wed, 21 May 2025 at 16:38, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, May 21, 2025 at 04:34:24PM +0100, Peter Maydell wrote:
> > Also, mjt's packaging for Debian puts in some stubs for the
> > offending getwpuid etc functions, which suppress the glib warnings
> > (this is why he noticed this whereas none of the rest of us did):
> >
> > https://sources.debian.org/patches/qemu/1:10.0.0%2Bds-2/static-linux-user-stubs.diff/
>
> Oh interesting, I was conincidentally wondering if adding dummy
> stubs might suppress this.

If we do I think that g_assert_not_reached() is probably better than
returning -1 etc.

> How about we pull that change upstream, and expand it to getaddrinfo
> too ?  I like the stubs much more than artificially splitting up
> the source files

The stub is still effectively a runtime assert. Splitting the
source file gives us a compile time guarantee that we don't
call the function, so I prefer that.

thanks
-- PMM


^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2025-05-21 15:41 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-07 15:30 [PATCH v2 00/17] maintainer updates for feb25 (qtest, gdbstub, plugins) pre-PR Alex Bennée
2025-02-07 15:30 ` [PATCH v2 01/17] tests/docker: replicate the check-rust-tools-nightly CI job Alex Bennée
2025-02-07 15:30 ` [PATCH v2 02/17] tests/qtest: don't attempt to clock_step while waiting for virtio ISR Alex Bennée
2025-02-07 16:02   ` Thomas Huth
2025-02-07 15:30 ` [PATCH v2 03/17] tests/qtest: don't step clock at start of npcm7xx periodic IRQ test Alex Bennée
2025-02-07 15:30 ` [PATCH v2 04/17] tests/qtest: simplify qtest_process_inbuf Alex Bennée
2025-02-07 15:31 ` [PATCH v2 05/17] tests/qtest: rename qtest_send_prefix and roll-up into qtest_send Alex Bennée
2025-02-07 16:04   ` Thomas Huth
2025-02-07 15:31 ` [PATCH v2 06/17] tests/qtest: tighten up the checks on clock_step Alex Bennée
2025-02-07 15:31 ` [PATCH v2 07/17] Revert "util/timer: avoid deadlock when shutting down" Alex Bennée
2025-02-07 15:31 ` [PATCH v2 08/17] tests/qtest/migration: Add --full option Alex Bennée
2025-02-07 15:31 ` [PATCH v2 09/17] tests/qtest/migration: Pick smoke tests Alex Bennée
2025-02-07 15:31 ` [PATCH v2 10/17] gdbstub: Allow the %d placeholder in the socket path Alex Bennée
2025-02-07 15:31 ` [PATCH v2 11/17] gdbstub: Try unlinking the unix socket before binding Alex Bennée
2025-05-20 14:50   ` Michael Tokarev
2025-05-20 15:52     ` Alex Bennée
2025-05-20 16:01       ` Peter Maydell
2025-05-20 22:21     ` Ilya Leoshkevich
2025-05-21  6:22       ` Alex Bennée
2025-05-21 15:34       ` Peter Maydell
2025-05-21 15:37         ` Daniel P. Berrangé
2025-05-21 15:41           ` Peter Maydell
2025-02-07 15:31 ` [PATCH v2 12/17] user: Introduce user/signal.h Alex Bennée
2025-02-07 15:31 ` [PATCH v2 13/17] user: Introduce host_interrupt_signal Alex Bennée
2025-02-07 15:31 ` [PATCH v2 14/17] osdep: Introduce qemu_kill_thread() Alex Bennée
2025-02-07 15:31 ` [PATCH v2 15/17] gdbstub: Allow late attachment Alex Bennée
2025-02-07 15:31 ` [PATCH v2 16/17] docs/user: Document the %d placeholder and suspend=n QEMU_GDB features Alex Bennée
2025-02-07 15:31 ` [PATCH v2 17/17] tests/tcg: Add late gdbstub attach test Alex Bennée

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).