Re: [PATCH 3/7] vhost: improve socket layer logs

[David Marchand <david.marchand@redhat.com>]

On Thu, Dec 23, 2021 at 9:37 AM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> This patch adds the Vhost socket path whenever possible in
> order to make debugging possible when multiple Vhost
> devices are in use.

This patch could go after patch 5 so that we have a dev->ifname
available instead of fd.


>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  lib/vhost/socket.c | 129 ++++++++++++++++++---------------------------
>  1 file changed, 51 insertions(+), 78 deletions(-)
>
> diff --git a/lib/vhost/socket.c b/lib/vhost/socket.c
> index 82963c1e6d..fd6b3a3ee4 100644
> --- a/lib/vhost/socket.c
> +++ b/lib/vhost/socket.c
> @@ -124,12 +124,12 @@ read_fd_message(int sockfd, char *buf, int buflen, int *fds, int max_fds,
>         ret = recvmsg(sockfd, &msgh, 0);
>         if (ret <= 0) {
>                 if (ret)
> -                       VHOST_LOG_CONFIG(ERR, "recvmsg failed\n");
> +                       VHOST_LOG_CONFIG(ERR, "recvmsg failed (fd %d)\n", sockfd);

strerror(errno) ?


>                 return ret;
>         }
>
>         if (msgh.msg_flags & (MSG_TRUNC | MSG_CTRUNC)) {
> -               VHOST_LOG_CONFIG(ERR, "truncated msg\n");
> +               VHOST_LOG_CONFIG(ERR, "truncated msg (fd %d)\n", sockfd);
>                 return -1;
>         }
>
> @@ -192,7 +192,7 @@ send_fd_message(int sockfd, char *buf, int buflen, int *fds, int fd_num)
>         } while (ret < 0 && errno == EINTR);
>
>         if (ret < 0) {
> -               VHOST_LOG_CONFIG(ERR,  "sendmsg error\n");
> +               VHOST_LOG_CONFIG(ERR,  "sendmsg error (fd %d)\n", sockfd);

strerror(errno) ?

There is an extra space before "sendmsg error...


In this same function, we have a log with no context / fd logged:
                        VHOST_LOG_CONFIG(ERR, "cmsg == NULL\n");
I'd add sockfd (and/or dev->ifname if available) in this log.

>                 return ret;
>         }
>


The rest lgtm.


-- 
David Marchand