[PATCH] chardev: use remoteAddr if the chardev is client

[PATCH] chardev: use remoteAddr if the chardev is client

[Haoqian He]

If the chardev is client, the socket file path in localAddr may be NULL.
This is because the socket path comes from getsockname(), according
to man page, getsockname() returns the current address bound by the
socket sockfd. If the chardev is client, it's socket is unbound sockfd.

Therefore, when computing the client chardev socket file path, using
remoteAddr is more appropriate.

Signed-off-by: Haoqian He <haoqian.he@smartx.com>
---
 chardev/char-socket.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 91496ceda9..2f842f9f88 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -571,9 +571,13 @@ static char *qemu_chr_compute_filename(SocketChardev *s)
 
     switch (ss->ss_family) {
     case AF_UNIX:
-        return g_strdup_printf("unix:%s%s",
-                               ((struct sockaddr_un *)(ss))->sun_path,
-                               s->is_listen ? ",server=on" : "");
+        if (s->is_listen) {
+            return g_strdup_printf("unix:%s,server=on",
+                                   ((struct sockaddr_un *)(ss))->sun_path);
+        } else {
+            return g_strdup_printf("unix:%s",
+                                   ((struct sockaddr_un *)(ps))->sun_path);
+        }
     case AF_INET6:
         left  = "[";
         right = "]";
-- 
2.48.1

Re: [PATCH] chardev: use remoteAddr if the chardev is client

[Marc-André Lureau]

Hi

On Tue, Feb 25, 2025 at 2:47 PM Haoqian He <haoqian.he@smartx.com> wrote:
>
> If the chardev is client, the socket file path in localAddr may be NULL.
> This is because the socket path comes from getsockname(), according
> to man page, getsockname() returns the current address bound by the
> socket sockfd. If the chardev is client, it's socket is unbound sockfd.
>
> Therefore, when computing the client chardev socket file path, using
> remoteAddr is more appropriate.
>
> Signed-off-by: Haoqian He <haoqian.he@smartx.com>
> ---
>  chardev/char-socket.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 91496ceda9..2f842f9f88 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -571,9 +571,13 @@ static char *qemu_chr_compute_filename(SocketChardev *s)
>
>      switch (ss->ss_family) {
>      case AF_UNIX:
> -        return g_strdup_printf("unix:%s%s",
> -                               ((struct sockaddr_un *)(ss))->sun_path,
> -                               s->is_listen ? ",server=on" : "");
> +        if (s->is_listen) {
> +            return g_strdup_printf("unix:%s,server=on",
> +                                   ((struct sockaddr_un *)(ss))->sun_path);
> +        } else {
> +            return g_strdup_printf("unix:%s",
> +                                   ((struct sockaddr_un *)(ps))->sun_path);
> +        }
>      case AF_INET6:
>          left  = "[";
>          right = "]";
> --
> 2.48.1
>
>

This patch doesn't change anything, and I don't understand the problem
you are trying to fix.

Can you provide more details or a test scenario?

thanks

-- 
Marc-André Lureau

Re: [PATCH] chardev: use remoteAddr if the chardev is client

[Haoqian He]

> 2025年2月25日 21:29，Marc-André Lureau <marcandre.lureau@gmail.com> 写道：
> 
> Hi Haoqian
> 
> On Tue, Feb 25, 2025 at 5:19 PM Haoqian He <haoqian.he@smartx.com> wrote:
>> 
>> I use chardev to connect with a vhost-user target, the chardev backend type is
>> socket, part of QEMU boot parameter:
>>  -device vhost-user-blk-pci,chardev=my-vhost-blk-0,id=my-vhost-blk-0,\
>>  bus=pci.1,addr=0x1,bootindex=1,num-queues=4 \
>>  -chardev socket,id=my-vhost-blk-0,path=/tmp/vhost-blk.1
>> 
>> I want to log the chardev’s socket path when socket successfully connected
>> in tcp_chr_connect (chr->filename), but i got "unix:" instead of the expected
>> "unix:/tmp/vhost-blk.1".
>> 
>> The chr->filename was computed from the function qemu_chr_compute_filename,
>> which always return the unix path store by local QIOChannelSocket localAddr.
>> 
>> Unfortunately, the localAddr is obtained via getsockname (see the function
>> qio_channel_socket_set_fd), and according to the man page:
>> "getsockname() returns the current address bound by the socket sockfd".
>> 
>> In this scenario, the socket client's sockfd is unbonded, so the socket
>> filename in localAddr(ss) is NULL.
>> As a solution, we can use remoteAddr(ps) obtained by getpeername (see the
>> function qio_channel_socket_set_fd).
>> 
> 
> thanks a lot, I didn't notice ps != ss in the patch. That makes sense.
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> 
> 
> -- 
> Marc-André Lureau

Thanks, Lureau.

--
Haoqian