[PATCH] vhost-user-test: no set non-blocking for cal fd less than 0.

[PATCH] vhost-user-test: no set non-blocking for cal fd less than 0.

[Yuxue Liu yuxue.liu@jaguarmicro.com]

From: Yuxue Liu <yuxue.liu@jaguarmicro.com>

In the scenario where vhost-user sets eventfd to -1,
qemu_chr_fe_get_msgfds retrieves fd as -1. When vhost_user_read
receives, it does not perform blocking operations on the descriptor
with fd=-1, so non-blocking operations should not be performed here
either.This is a normal use case. Calling g_unix_set_fd_nonblocking
at this point will cause the test to interrupt.

When vhost_user_write sets the call fd to -1, it sets the number of
fds to 0, so the fds obtained by qemu_chr_fe_get_msgfds will also
be 0.

Signed-off-by: Yuxue Liu <yuxue.liu@jaguarmicro.com>
---
 tests/qtest/vhost-user-test.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
index d4e437265f..7c8ef6268d 100644
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -458,7 +458,10 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
     case VHOST_USER_SET_VRING_KICK:
     case VHOST_USER_SET_VRING_CALL:
         /* consume the fd */
-        qemu_chr_fe_get_msgfds(chr, &fd, 1);
+        if (!qemu_chr_fe_get_msgfds(chr, &fd, 1) && fd < 0) {
+            qos_printf("call fd :%d, no set non-blocking\n", fd);
+            break;
+        }
         /*
          * This is a non-blocking eventfd.
          * The receive function forces it to be blocking,
-- 
2.43.0

Re: [PATCH] vhost-user-test: no set non-blocking for cal fd less than 0.

[Thomas Huth]

On 11/04/2024 09.35, Yuxue Liu yuxue.liu@jaguarmicro.com wrote:
> From: Yuxue Liu <yuxue.liu@jaguarmicro.com>
> 
> In the scenario where vhost-user sets eventfd to -1,
> qemu_chr_fe_get_msgfds retrieves fd as -1. When vhost_user_read
> receives, it does not perform blocking operations on the descriptor
> with fd=-1, so non-blocking operations should not be performed here
> either.This is a normal use case. Calling g_unix_set_fd_nonblocking
> at this point will cause the test to interrupt.
> 
> When vhost_user_write sets the call fd to -1, it sets the number of
> fds to 0, so the fds obtained by qemu_chr_fe_get_msgfds will also
> be 0.
> 
> Signed-off-by: Yuxue Liu <yuxue.liu@jaguarmicro.com>
> ---
>   tests/qtest/vhost-user-test.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
> index d4e437265f..7c8ef6268d 100644
> --- a/tests/qtest/vhost-user-test.c
> +++ b/tests/qtest/vhost-user-test.c
> @@ -458,7 +458,10 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
>       case VHOST_USER_SET_VRING_KICK:
>       case VHOST_USER_SET_VRING_CALL:
>           /* consume the fd */
> -        qemu_chr_fe_get_msgfds(chr, &fd, 1);
> +        if (!qemu_chr_fe_get_msgfds(chr, &fd, 1) && fd < 0) {
> +            qos_printf("call fd :%d, no set non-blocking\n", fd);
> +            break;
> +        }
>           /*
>            * This is a non-blocking eventfd.
>            * The receive function forces it to be blocking,

Could someone experienced with vhost-user please review this?

  Thanks,
   Thomas

Re: [PATCH] vhost-user-test: no set non-blocking for cal fd less than 0.

[Michael S. Tsirkin]

On Thu, Apr 11, 2024 at 03:35:55PM +0800, Yuxue Liu yuxue.liu@jaguarmicro.com wrote:
> From: Yuxue Liu <yuxue.liu@jaguarmicro.com>
> 
> In the scenario where vhost-user sets eventfd to -1,
> qemu_chr_fe_get_msgfds retrieves fd as -1. When vhost_user_read
> receives, it does not perform blocking operations on the descriptor
> with fd=-1, so non-blocking operations should not be performed here
> either.This is a normal use case. Calling g_unix_set_fd_nonblocking
> at this point will cause the test to interrupt.
> 
> When vhost_user_write sets the call fd to -1, it sets the number of
> fds to 0, so the fds obtained by qemu_chr_fe_get_msgfds will also
> be 0.
> 
> Signed-off-by: Yuxue Liu <yuxue.liu@jaguarmicro.com>

A bit more detail here please.
When does all this happen?

> ---
>  tests/qtest/vhost-user-test.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
> index d4e437265f..7c8ef6268d 100644
> --- a/tests/qtest/vhost-user-test.c
> +++ b/tests/qtest/vhost-user-test.c
> @@ -458,7 +458,10 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
>      case VHOST_USER_SET_VRING_KICK:
>      case VHOST_USER_SET_VRING_CALL:
>          /* consume the fd */
> -        qemu_chr_fe_get_msgfds(chr, &fd, 1);
> +        if (!qemu_chr_fe_get_msgfds(chr, &fd, 1) && fd < 0) {
> +            qos_printf("call fd :%d, no set non-blocking\n", fd);
> +            break;
> +        }
>          /*
>           * This is a non-blocking eventfd.
>           * The receive function forces it to be blocking,
> -- 
> 2.43.0

Re: [PATCH] vhost-user-test: no set non-blocking for cal fd less than 0.

[Gavin Liu]

Hi Michael, Thomas

The reason is I want to merge this patch: https://lore.kernel.org/all/SEYPR06MB67561ABD83633689B5037395EC062@SEYPR06MB6756.apcprd06.prod.outlook.com/
Compilation errors occur during CI :
	stderr:
		**
		ERROR:../tests/qtest/vhost-user-test.c:468:chr_read: assertion failed (err == NULL): Bad file descriptor (g-unix-error-quark, 0)
		**
		ERROR:../tests/qtest/qos-test.c:191:subprocess_run_one_test: child process (/aarch64/virt/generic-pcihost/pci-bus-generic/pci-bus/virtio-net-pci/virtio-net/virtio-net-tests/vhost-user/migrate/subprocess [22197]) failed unexpectedly (test program exited with status code -6) ―― 

The reason is that when the guest os uses the PMD mode, it does not enable interrupts in the device's MSIX.
In QEMU,  The function vhost_user_set_vring_call invokes vhost_set_vring_file to send the file descriptor information, where fd=-1, thus fd_num=0.
In CI , tests/qtest/vhost-user-test
   1：vhost-user-test invokes the chr_read function to handle the messages sent by the server.
   2：The chr_read function initializes the local variable fd to -1.
   3：
        case VHOST_USER_SET_VRING_CALL:
			qemu_chr_fe_get_msgfds(chr, &fd, 1); 
			
!!! Because s->read_msgfds_num = 0, after the function qemu_chr_fe_get_msgfds(chr, &fd, 1) is executed, fd=-1.!!!

			g_unix_set_fd_nonblocking(fd, true, &err); 
			
!!! An exception will occur when setting nonblocking when fd < 0. !!!	
			g_assert_no_error(err);

						
Offer a modification suggestion
   1：Refer to the tcp_chr_recv function:
						static ssize_t tcp_chr_recv(Chardev *chr, char *buf, size_t len)
						{
							SocketChardev *s = SOCKET_CHARDEV(chr);
							  -----
							  ----
							for (i = 0; i < s->read_msgfds_num; i++) {
								int fd = s->read_msgfds[i];
								if (fd < 0) {
				!!!!When fd < 0, the qemu_socket_set_block function will not be called !!!!			
									continue;
								}

								/* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
								qemu_socket_set_block(fd);

				
   2: modify chr_read function 

				case VHOST_USER_SET_VRING_CALL:
						qemu_chr_fe_get_msgfds(chr, &fd, 1); 
						if (fd <0)   /* Add a condition check. */
							break;
						g_unix_set_fd_nonblocking(fd, true, &err); 
						g_assert_no_error(err);

Best regards,
Yuxue Liu


-----Original Message-----
From: Michael S. Tsirkin mst@redhat.com
Sent: April 18, 2024 23:41
To: Gavin Liu gavin.liu@jaguarmicro.com
Cc: pbonzini@redhat.com; lvivier@redhat.com; thuth@redhat.com; qemu-devel@nongnu.org
Subject: Re: [PATCH] vhost-user-test: no set non-blocking for cal fd less than 0.


External Mail: This email originated from OUTSIDE of the organization!
Do not click links, open attachments or provide ANY information unless you recognize the sender and know the content is safe.


On Thu, Apr 11, 2024 at 03:35:55PM +0800, Yuxue Liu yuxue.liu@jaguarmicro.com wrote:
> From: Yuxue Liu <yuxue.liu@jaguarmicro.com>
>
> In the scenario where vhost-user sets eventfd to -1, 
> qemu_chr_fe_get_msgfds retrieves fd as -1. When vhost_user_read 
> receives, it does not perform blocking operations on the descriptor 
> with fd=-1, so non-blocking operations should not be performed here 
> either.This is a normal use case. Calling g_unix_set_fd_nonblocking at 
> this point will cause the test to interrupt.
>
> When vhost_user_write sets the call fd to -1, it sets the number of 
> fds to 0, so the fds obtained by qemu_chr_fe_get_msgfds will also be 
> 0.
>
> Signed-off-by: Yuxue Liu <yuxue.liu@jaguarmicro.com>

A bit more detail here please.
When does all this happen?

> ---
>  tests/qtest/vhost-user-test.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qtest/vhost-user-test.c 
> b/tests/qtest/vhost-user-test.c index d4e437265f..7c8ef6268d 100644
> --- a/tests/qtest/vhost-user-test.c
> +++ b/tests/qtest/vhost-user-test.c
> @@ -458,7 +458,10 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
>      case VHOST_USER_SET_VRING_KICK:
>      case VHOST_USER_SET_VRING_CALL:
>          /* consume the fd */
> -        qemu_chr_fe_get_msgfds(chr, &fd, 1);
> +        if (!qemu_chr_fe_get_msgfds(chr, &fd, 1) && fd < 0) {
> +            qos_printf("call fd :%d, no set non-blocking\n", fd);
> +            break;
> +        }
>          /*
>           * This is a non-blocking eventfd.
>           * The receive function forces it to be blocking,
> --
> 2.43.0