* [PATCH] Revert "tests/qtest: use qos_printf instead of g_test_message"
@ 2025-07-28 14:57 Markus Armbruster
2025-07-28 15:20 ` Daniel P. Berrangé
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Markus Armbruster @ 2025-07-28 14:57 UTC (permalink / raw)
To: qemu-devel; +Cc: alex.bennee, pbonzini
This reverts commit 30ea13e9d97dcbd4ea541ddf9e8857fa1d5cb30f.
"make check" prints many lines like
stdout: 138: UNKNOWN: # # qos_test running single test in subprocess
stdout: 139: UNKNOWN: # # set_protocol_features: 0x42
stdout: 140: UNKNOWN: # # set_owner: start of session
stdout: 141: UNKNOWN: # # vhost-user: un-handled message: 14
stdout: 142: UNKNOWN: # # vhost-user: un-handled message: 14
stdout: 143: UNKNOWN: # # set_vring(0)=enabled
stdout: 144: UNKNOWN: # # set_vring(1)=enabled
stdout: 145: UNKNOWN: # # set_vring(0)=enabled
stdout: 146: UNKNOWN: # # set_vring(1)=enabled
stdout: 147: UNKNOWN: # # set_vring(0)=enabled
stdout: 148: UNKNOWN: # # set_vring(1)=enabled
stdout: 149: UNKNOWN: # # set_vring(0)=enabled
stdout: 150: UNKNOWN: # # set_vring(1)=enabled
stdout: 151: UNKNOWN: # # set_vring(0)=enabled
stdout: 152: UNKNOWN: # # set_vring(1)=enabled
stdout: 153: UNKNOWN: # # set_vring_num: 0/256
stdout: 154: UNKNOWN: # # set_vring_addr: 0x7f9060000000/0x7f905ffff000/0x7f9060001000
Turns out this is qos-test, and the culprit is a commit meant to ease
debugging. Revert it until a better solution is found.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/qtest/qos-test.c | 5 -----
tests/qtest/vhost-user-test.c | 27 +++++++++++++--------------
2 files changed, 13 insertions(+), 19 deletions(-)
diff --git a/tests/qtest/qos-test.c b/tests/qtest/qos-test.c
index abfd4b9512..00f39f33f6 100644
--- a/tests/qtest/qos-test.c
+++ b/tests/qtest/qos-test.c
@@ -328,11 +328,6 @@ static void walk_path(QOSGraphNode *orig_path, int len)
int main(int argc, char **argv, char** envp)
{
g_test_init(&argc, &argv, NULL);
-
- if (g_test_subprocess()) {
- qos_printf("qos_test running single test in subprocess\n");
- }
-
if (g_test_verbose()) {
qos_printf("ENVIRONMENT VARIABLES: {\n");
for (char **env = envp; *env != 0; env++) {
diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
index 75cb3e44b2..56472ca709 100644
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -26,7 +26,6 @@
#include "libqos/virtio-pci.h"
#include "libqos/malloc-pc.h"
-#include "libqos/qgraph_internal.h"
#include "hw/virtio/virtio-net.h"
#include "standard-headers/linux/vhost_types.h"
@@ -345,7 +344,7 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
}
if (size != VHOST_USER_HDR_SIZE) {
- qos_printf("%s: Wrong message size received %d\n", __func__, size);
+ g_test_message("Wrong message size received %d", size);
return;
}
@@ -356,8 +355,8 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
p += VHOST_USER_HDR_SIZE;
size = qemu_chr_fe_read_all(chr, p, msg.size);
if (size != msg.size) {
- qos_printf("%s: Wrong message size received %d != %d\n",
- __func__, size, msg.size);
+ g_test_message("Wrong message size received %d != %d",
+ size, msg.size);
goto out;
}
}
@@ -393,7 +392,7 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
* We don't need to do anything here, the remote is just
* letting us know it is in charge. Just log it.
*/
- qos_printf("set_owner: start of session\n");
+ g_test_message("set_owner: start of session\n");
break;
case VHOST_USER_GET_PROTOCOL_FEATURES:
@@ -419,7 +418,7 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
* the remote end to send this. There is no handshake reply so
* just log the details for debugging.
*/
- qos_printf("set_protocol_features: 0x%"PRIx64 "\n", msg.payload.u64);
+ g_test_message("set_protocol_features: 0x%"PRIx64 "\n", msg.payload.u64);
break;
/*
@@ -427,11 +426,11 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
* address of the vrings but we can simply report them.
*/
case VHOST_USER_SET_VRING_NUM:
- qos_printf("set_vring_num: %d/%d\n",
+ g_test_message("set_vring_num: %d/%d\n",
msg.payload.state.index, msg.payload.state.num);
break;
case VHOST_USER_SET_VRING_ADDR:
- qos_printf("set_vring_addr: 0x%"PRIx64"/0x%"PRIx64"/0x%"PRIx64"\n",
+ g_test_message("set_vring_addr: 0x%"PRIx64"/0x%"PRIx64"/0x%"PRIx64"\n",
msg.payload.addr.avail_user_addr,
msg.payload.addr.desc_user_addr,
msg.payload.addr.used_user_addr);
@@ -464,7 +463,7 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
case VHOST_USER_SET_VRING_CALL:
/* consume the fd */
if (!qemu_chr_fe_get_msgfds(chr, &fd, 1) && fd < 0) {
- qos_printf("call fd: %d, do not set non-blocking\n", fd);
+ g_test_message("call fd: %d, do not set non-blocking\n", fd);
break;
}
/*
@@ -510,12 +509,12 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
* fully functioning vhost-user we would enable/disable the
* vring monitoring.
*/
- qos_printf("set_vring(%d)=%s\n", msg.payload.state.index,
+ g_test_message("set_vring(%d)=%s\n", msg.payload.state.index,
msg.payload.state.num ? "enabled" : "disabled");
break;
default:
- qos_printf("vhost-user: un-handled message: %d\n", msg.request);
+ g_test_message("vhost-user: un-handled message: %d\n", msg.request);
break;
}
@@ -539,7 +538,7 @@ static const char *init_hugepagefs(void)
}
if (access(path, R_OK | W_OK | X_OK)) {
- qos_printf("access on path (%s): %s", path, strerror(errno));
+ g_test_message("access on path (%s): %s", path, strerror(errno));
g_test_fail();
return NULL;
}
@@ -549,13 +548,13 @@ static const char *init_hugepagefs(void)
} while (ret != 0 && errno == EINTR);
if (ret != 0) {
- qos_printf("statfs on path (%s): %s", path, strerror(errno));
+ g_test_message("statfs on path (%s): %s", path, strerror(errno));
g_test_fail();
return NULL;
}
if (fs.f_type != HUGETLBFS_MAGIC) {
- qos_printf("Warning: path not on HugeTLBFS: %s", path);
+ g_test_message("Warning: path not on HugeTLBFS: %s", path);
g_test_fail();
return NULL;
}
--
2.49.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] Revert "tests/qtest: use qos_printf instead of g_test_message"
2025-07-28 14:57 [PATCH] Revert "tests/qtest: use qos_printf instead of g_test_message" Markus Armbruster
@ 2025-07-28 15:20 ` Daniel P. Berrangé
2025-07-28 17:03 ` Markus Armbruster
2025-09-03 7:38 ` Michael Tokarev
2025-09-17 14:19 ` Michael Tokarev
2 siblings, 1 reply; 5+ messages in thread
From: Daniel P. Berrangé @ 2025-07-28 15:20 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, alex.bennee, pbonzini
On Mon, Jul 28, 2025 at 04:57:47PM +0200, Markus Armbruster wrote:
> This reverts commit 30ea13e9d97dcbd4ea541ddf9e8857fa1d5cb30f.
You had conflicts in this revert I presume ? That commit added
6 qos_printf calls, but this commit removes 13 qos_printf calls
>
> "make check" prints many lines like
>
> stdout: 138: UNKNOWN: # # qos_test running single test in subprocess
> stdout: 139: UNKNOWN: # # set_protocol_features: 0x42
> stdout: 140: UNKNOWN: # # set_owner: start of session
> stdout: 141: UNKNOWN: # # vhost-user: un-handled message: 14
> stdout: 142: UNKNOWN: # # vhost-user: un-handled message: 14
> stdout: 143: UNKNOWN: # # set_vring(0)=enabled
> stdout: 144: UNKNOWN: # # set_vring(1)=enabled
> stdout: 145: UNKNOWN: # # set_vring(0)=enabled
> stdout: 146: UNKNOWN: # # set_vring(1)=enabled
> stdout: 147: UNKNOWN: # # set_vring(0)=enabled
> stdout: 148: UNKNOWN: # # set_vring(1)=enabled
> stdout: 149: UNKNOWN: # # set_vring(0)=enabled
> stdout: 150: UNKNOWN: # # set_vring(1)=enabled
> stdout: 151: UNKNOWN: # # set_vring(0)=enabled
> stdout: 152: UNKNOWN: # # set_vring(1)=enabled
> stdout: 153: UNKNOWN: # # set_vring_num: 0/256
> stdout: 154: UNKNOWN: # # set_vring_addr: 0x7f9060000000/0x7f905ffff000/0x7f9060001000
>
> Turns out this is qos-test, and the culprit is a commit meant to ease
> debugging. Revert it until a better solution is found.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> tests/qtest/qos-test.c | 5 -----
> tests/qtest/vhost-user-test.c | 27 +++++++++++++--------------
> 2 files changed, 13 insertions(+), 19 deletions(-)
>
> diff --git a/tests/qtest/qos-test.c b/tests/qtest/qos-test.c
> index abfd4b9512..00f39f33f6 100644
> --- a/tests/qtest/qos-test.c
> +++ b/tests/qtest/qos-test.c
> @@ -328,11 +328,6 @@ static void walk_path(QOSGraphNode *orig_path, int len)
> int main(int argc, char **argv, char** envp)
> {
> g_test_init(&argc, &argv, NULL);
> -
> - if (g_test_subprocess()) {
> - qos_printf("qos_test running single test in subprocess\n");
> - }
> -
> if (g_test_verbose()) {
> qos_printf("ENVIRONMENT VARIABLES: {\n");
> for (char **env = envp; *env != 0; env++) {
> diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
> index 75cb3e44b2..56472ca709 100644
> --- a/tests/qtest/vhost-user-test.c
> +++ b/tests/qtest/vhost-user-test.c
> @@ -26,7 +26,6 @@
> #include "libqos/virtio-pci.h"
>
> #include "libqos/malloc-pc.h"
> -#include "libqos/qgraph_internal.h"
> #include "hw/virtio/virtio-net.h"
>
> #include "standard-headers/linux/vhost_types.h"
> @@ -345,7 +344,7 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
> }
>
> if (size != VHOST_USER_HDR_SIZE) {
> - qos_printf("%s: Wrong message size received %d\n", __func__, size);
> + g_test_message("Wrong message size received %d", size);
> return;
> }
>
> @@ -356,8 +355,8 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
> p += VHOST_USER_HDR_SIZE;
> size = qemu_chr_fe_read_all(chr, p, msg.size);
> if (size != msg.size) {
> - qos_printf("%s: Wrong message size received %d != %d\n",
> - __func__, size, msg.size);
> + g_test_message("Wrong message size received %d != %d",
> + size, msg.size);
> goto out;
> }
> }
> @@ -393,7 +392,7 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
> * We don't need to do anything here, the remote is just
> * letting us know it is in charge. Just log it.
> */
> - qos_printf("set_owner: start of session\n");
> + g_test_message("set_owner: start of session\n");
> break;
>
> case VHOST_USER_GET_PROTOCOL_FEATURES:
> @@ -419,7 +418,7 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
> * the remote end to send this. There is no handshake reply so
> * just log the details for debugging.
> */
> - qos_printf("set_protocol_features: 0x%"PRIx64 "\n", msg.payload.u64);
> + g_test_message("set_protocol_features: 0x%"PRIx64 "\n", msg.payload.u64);
> break;
>
> /*
> @@ -427,11 +426,11 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
> * address of the vrings but we can simply report them.
> */
> case VHOST_USER_SET_VRING_NUM:
> - qos_printf("set_vring_num: %d/%d\n",
> + g_test_message("set_vring_num: %d/%d\n",
> msg.payload.state.index, msg.payload.state.num);
> break;
> case VHOST_USER_SET_VRING_ADDR:
> - qos_printf("set_vring_addr: 0x%"PRIx64"/0x%"PRIx64"/0x%"PRIx64"\n",
> + g_test_message("set_vring_addr: 0x%"PRIx64"/0x%"PRIx64"/0x%"PRIx64"\n",
> msg.payload.addr.avail_user_addr,
> msg.payload.addr.desc_user_addr,
> msg.payload.addr.used_user_addr);
> @@ -464,7 +463,7 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
> case VHOST_USER_SET_VRING_CALL:
> /* consume the fd */
> if (!qemu_chr_fe_get_msgfds(chr, &fd, 1) && fd < 0) {
> - qos_printf("call fd: %d, do not set non-blocking\n", fd);
> + g_test_message("call fd: %d, do not set non-blocking\n", fd);
> break;
> }
> /*
> @@ -510,12 +509,12 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
> * fully functioning vhost-user we would enable/disable the
> * vring monitoring.
> */
> - qos_printf("set_vring(%d)=%s\n", msg.payload.state.index,
> + g_test_message("set_vring(%d)=%s\n", msg.payload.state.index,
> msg.payload.state.num ? "enabled" : "disabled");
> break;
>
> default:
> - qos_printf("vhost-user: un-handled message: %d\n", msg.request);
> + g_test_message("vhost-user: un-handled message: %d\n", msg.request);
> break;
> }
>
> @@ -539,7 +538,7 @@ static const char *init_hugepagefs(void)
> }
>
> if (access(path, R_OK | W_OK | X_OK)) {
> - qos_printf("access on path (%s): %s", path, strerror(errno));
> + g_test_message("access on path (%s): %s", path, strerror(errno));
> g_test_fail();
> return NULL;
> }
> @@ -549,13 +548,13 @@ static const char *init_hugepagefs(void)
> } while (ret != 0 && errno == EINTR);
>
> if (ret != 0) {
> - qos_printf("statfs on path (%s): %s", path, strerror(errno));
> + g_test_message("statfs on path (%s): %s", path, strerror(errno));
> g_test_fail();
> return NULL;
> }
>
> if (fs.f_type != HUGETLBFS_MAGIC) {
> - qos_printf("Warning: path not on HugeTLBFS: %s", path);
> + g_test_message("Warning: path not on HugeTLBFS: %s", path);
> g_test_fail();
> return NULL;
> }
> --
> 2.49.0
>
>
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] 5+ messages in thread* Re: [PATCH] Revert "tests/qtest: use qos_printf instead of g_test_message"
2025-07-28 15:20 ` Daniel P. Berrangé
@ 2025-07-28 17:03 ` Markus Armbruster
0 siblings, 0 replies; 5+ messages in thread
From: Markus Armbruster @ 2025-07-28 17:03 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, alex.bennee, pbonzini
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Mon, Jul 28, 2025 at 04:57:47PM +0200, Markus Armbruster wrote:
>> This reverts commit 30ea13e9d97dcbd4ea541ddf9e8857fa1d5cb30f.
>
> You had conflicts in this revert I presume ? That commit added
> 6 qos_printf calls, but this commit removes 13 qos_printf calls
Yes. Should've mentioned them in the commit message.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Revert "tests/qtest: use qos_printf instead of g_test_message"
2025-07-28 14:57 [PATCH] Revert "tests/qtest: use qos_printf instead of g_test_message" Markus Armbruster
2025-07-28 15:20 ` Daniel P. Berrangé
@ 2025-09-03 7:38 ` Michael Tokarev
2025-09-17 14:19 ` Michael Tokarev
2 siblings, 0 replies; 5+ messages in thread
From: Michael Tokarev @ 2025-09-03 7:38 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: alex.bennee, pbonzini, qemu-stable
On 28.07.2025 17:57, Markus Armbruster wrote:
> This reverts commit 30ea13e9d97dcbd4ea541ddf9e8857fa1d5cb30f.
>
> "make check" prints many lines like
>
> stdout: 138: UNKNOWN: # # qos_test running single test in subprocess
I'm picking this up for qemu-stable (10.0 & 10.1) too, since
these messages are annoying there too. It applies cleanly too.
Thanks,
/mjt
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Revert "tests/qtest: use qos_printf instead of g_test_message"
2025-07-28 14:57 [PATCH] Revert "tests/qtest: use qos_printf instead of g_test_message" Markus Armbruster
2025-07-28 15:20 ` Daniel P. Berrangé
2025-09-03 7:38 ` Michael Tokarev
@ 2025-09-17 14:19 ` Michael Tokarev
2 siblings, 0 replies; 5+ messages in thread
From: Michael Tokarev @ 2025-09-17 14:19 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: alex.bennee, pbonzini
On 28.07.2025 17:57, Markus Armbruster wrote:
> This reverts commit 30ea13e9d97dcbd4ea541ddf9e8857fa1d5cb30f.
>
> "make check" prints many lines like
>
> stdout: 138: UNKNOWN: # # qos_test running single test in subprocess
> stdout: 139: UNKNOWN: # # set_protocol_features: 0x42
> stdout: 140: UNKNOWN: # # set_owner: start of session
> stdout: 141: UNKNOWN: # # vhost-user: un-handled message: 14
> stdout: 142: UNKNOWN: # # vhost-user: un-handled message: 14
> stdout: 143: UNKNOWN: # # set_vring(0)=enabled
> stdout: 144: UNKNOWN: # # set_vring(1)=enabled
> stdout: 145: UNKNOWN: # # set_vring(0)=enabled
> stdout: 146: UNKNOWN: # # set_vring(1)=enabled
> stdout: 147: UNKNOWN: # # set_vring(0)=enabled
> stdout: 148: UNKNOWN: # # set_vring(1)=enabled
> stdout: 149: UNKNOWN: # # set_vring(0)=enabled
> stdout: 150: UNKNOWN: # # set_vring(1)=enabled
> stdout: 151: UNKNOWN: # # set_vring(0)=enabled
> stdout: 152: UNKNOWN: # # set_vring(1)=enabled
> stdout: 153: UNKNOWN: # # set_vring_num: 0/256
> stdout: 154: UNKNOWN: # # set_vring_addr: 0x7f9060000000/0x7f905ffff000/0x7f9060001000
>
> Turns out this is qos-test, and the culprit is a commit meant to ease
> debugging. Revert it until a better solution is found.
..
> @@ -393,7 +392,7 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
> * We don't need to do anything here, the remote is just
> * letting us know it is in charge. Just log it.
> */
> - qos_printf("set_owner: start of session\n");
> + g_test_message("set_owner: start of session\n");
Here, and elsewhere - checkpatch complains:
ERROR: Error messages should not contain newlines
#93: FILE: tests/qtest/vhost-user-test.c:395:
+ g_test_message("set_owner: start of session\n");
(while some g_test_message calls in the same patch does not contain
the newline). This is apparently the only place where g_test_message
is used with newlines.
I wonder if we should clean this up? Something like this:
diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
index 56472ca709..e8c3613560 100644
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -394,3 +394,3 @@ static void chr_read(void *opaque, const uint8_t
*buf, int size)
*/
- g_test_message("set_owner: start of session\n");
+ g_test_message("set_owner: start of session");
break;
@@ -420,3 +420,3 @@ static void chr_read(void *opaque, const uint8_t
*buf, int size)
*/
- g_test_message("set_protocol_features: 0x%"PRIx64 "\n",
msg.payload.u64);
+ g_test_message("set_protocol_features: 0x%"PRIx64,
msg.payload.u64);
break;
@@ -428,3 +428,3 @@ static void chr_read(void *opaque, const uint8_t
*buf, int size)
case VHOST_USER_SET_VRING_NUM:
- g_test_message("set_vring_num: %d/%d\n",
+ g_test_message("set_vring_num: %d/%d",
msg.payload.state.index, msg.payload.state.num);
@@ -432,3 +432,3 @@ static void chr_read(void *opaque, const uint8_t
*buf, int size)
case VHOST_USER_SET_VRING_ADDR:
- g_test_message("set_vring_addr:
0x%"PRIx64"/0x%"PRIx64"/0x%"PRIx64"\n",
+ g_test_message("set_vring_addr: 0x%"PRIx64"/0x%"PRIx64"/0x%"PRIx64,
msg.payload.addr.avail_user_addr,
@@ -465,3 +465,3 @@ static void chr_read(void *opaque, const uint8_t
*buf, int size)
if (!qemu_chr_fe_get_msgfds(chr, &fd, 1) && fd < 0) {
- g_test_message("call fd: %d, do not set non-blocking\n", fd);
+ g_test_message("call fd: %d, do not set non-blocking", fd);
break;
@@ -511,3 +511,3 @@ static void chr_read(void *opaque, const uint8_t
*buf, int size)
*/
- g_test_message("set_vring(%d)=%s\n", msg.payload.state.index,
+ g_test_message("set_vring(%d)=%s", msg.payload.state.index,
msg.payload.state.num ? "enabled" : "disabled");
@@ -516,3 +516,3 @@ static void chr_read(void *opaque, const uint8_t
*buf, int size)
default:
- g_test_message("vhost-user: un-handled message: %d\n",
msg.request);
+ g_test_message("vhost-user: un-handled message: %d", msg.request);
break;
Thanks,
/mjt
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-09-17 14:20 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-28 14:57 [PATCH] Revert "tests/qtest: use qos_printf instead of g_test_message" Markus Armbruster
2025-07-28 15:20 ` Daniel P. Berrangé
2025-07-28 17:03 ` Markus Armbruster
2025-09-03 7:38 ` Michael Tokarev
2025-09-17 14:19 ` Michael Tokarev
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.