Re: [PATCH] Revert "tests/qtest: use qos_printf instead of g_test_message"

2025-09-17 Thread Michael Tokarev 

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: 
0x7f906000/0x7f905000/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

Re: [PATCH] Revert "tests/qtest: use qos_printf instead of g_test_message"

2025-09-03 Thread Michael Tokarev 

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

Re: [PATCH] Revert "tests/qtest: use qos_printf instead of g_test_message"

2025-07-28 Thread Markus Armbruster 
Daniel P. Berrangé  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.

Re: [PATCH] Revert "tests/qtest: use qos_printf instead of g_test_message"

2025-07-28 Thread Daniel P . Berrangé 
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: 
> 0x7f906000/0x7f905000/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 
> ---
>  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: 
>

[PATCH] Revert "tests/qtest: use qos_printf instead of g_test_message"

2025-07-28 Thread Markus Armbruster 
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: 
0x7f906000/0x7f905000/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 
---
 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) {
-