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

["Daniel P. Berrangé" <berrange@redhat.com>]

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 :|