| Message ID | 20250225104526.2924175-1-haoqian.he@smartx.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | chardev: use remoteAddr if the chardev is client | expand |
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
> 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
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 = "]";
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(-)