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: 
SEYPR06MB67561ABD83633689B5037395EC062@SEYPR06MB6756.apcprd06.prod.outlook.com/">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