Re: [PATCH v2 3/5] util: Add util methods required by ch networking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/16/24 22:25, Praveen K Paladugu wrote:
> virSocketSendMsgWithFDs method send fds along with payload using
> SCM_RIGHTS. virSocketRecv method polls, receives and sends the response
> to callers.
> 
> These methods are required to add network suppport in ch driver.
> 
> Signed-off-by: Praveen K Paladugu <prapal@xxxxxxxxxxxxxxxxxxx>
> ---
>  po/POTFILES              |   1 +
>  src/libvirt_private.syms |   2 +
>  src/util/virsocket.c     | 116 +++++++++++++++++++++++++++++++++++++++
>  src/util/virsocket.h     |   3 +
>  4 files changed, 122 insertions(+)
> 
> diff --git a/po/POTFILES b/po/POTFILES
> index 023c041f61..b594a8dd39 100644
> --- a/po/POTFILES
> +++ b/po/POTFILES
> @@ -326,6 +326,7 @@ src/util/virscsi.c
>  src/util/virscsihost.c
>  src/util/virscsivhost.c
>  src/util/virsecret.c
> +src/util/virsocket.c
>  src/util/virsocketaddr.c
>  src/util/virstoragefile.c
>  src/util/virstring.c
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 80d43ed15d..2c9b2635cd 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -3371,8 +3371,10 @@ virSecureEraseString;
>  
>  
>  # util/virsocket.h
> +virSocketRecv;
>  virSocketRecvFD;
>  virSocketSendFD;
> +virSocketSendMsgWithFDs;
>  
>  
>  # util/virsocketaddr.h
> diff --git a/src/util/virsocket.c b/src/util/virsocket.c
> index cd6f7ecd1b..8019fd1199 100644
> --- a/src/util/virsocket.c
> +++ b/src/util/virsocket.c
> @@ -19,11 +19,18 @@
>  
>  #include <config.h>
>  
> +#include "virerror.h"
>  #include "virsocket.h"
>  #include "virutil.h"
>  #include "virfile.h"
> +#include "virlog.h"
>  
>  #include <fcntl.h>
> +#include <poll.h>
> +
> +#define PKT_TIMEOUT_MS 500 /* ms */
> +
> +#define VIR_FROM_THIS VIR_FROM_NONE
>  
>  #ifdef WIN32
>  
> @@ -482,6 +489,108 @@ virSocketRecvFD(int sock, int fdflags)
>  
>      return fd;
>  }
> +
> +/**
> + * virSocketSendMsgWithFDs:
> + * @sock: socket to send payload and fds to
> + * @payload: payload to send
> + * @fds: array of fds to send
> + * @fds_len: len of fds array
> +
> + * Send @fds along with @payload to @sock using SCM_RIGHTS.
> + * Return number of bytes sent on success.
> + * On error, set errno and return -1.
> + */
> +int
> +virSocketSendMsgWithFDs(int sock, const char *payload, int *fds, size_t fds_len)
> +{
> +    struct msghdr msg;

This can be initialized to zero so that explicit memset() below can be
dropped.

> +    struct iovec iov[1]; /* Send a single payload, so set vector len to 1 */
> +    int ret;
> +    char control[CMSG_SPACE(sizeof(int)*fds_len)];

Yeah, we don't really do VLAs in libvirt. I wish we did and we could
even switch to C23 so that this could use an empty initializer and the
other memset below could be dropped, but that's fight for another day.

For now, this needs to be allocated on the heap. No biggie.

> +    struct cmsghdr *cmsg;
> +
> +    memset(&msg, 0, sizeof(msg));
> +    memset(control, 0, sizeof(control));
> +
> +    iov[0].iov_base = (void *) payload;
> +    iov[0].iov_len = strlen(payload);
> +
> +    msg.msg_iov = iov;
> +    msg.msg_iovlen = 1;
> +
> +    msg.msg_control = control;
> +    msg.msg_controllen = sizeof(control);
> +
> +    cmsg = CMSG_FIRSTHDR(&msg);
> +    /* check to eliminate "potential null pointer dereference" errors during build */
> +    if (!cmsg) {
> +        virReportSystemError(EFAULT, "%s", _("Couldn't fit control msg header in msg") );

Formatting.

> +        return -1;
> +    }
> +
> +    cmsg->cmsg_len = CMSG_LEN(sizeof(int) * fds_len);
> +    cmsg->cmsg_level = SOL_SOCKET;
> +    cmsg->cmsg_type = SCM_RIGHTS;
> +    memcpy(CMSG_DATA(cmsg), fds, sizeof(int) * fds_len);
> +
> +    do {
> +        ret = sendmsg(sock, &msg, 0);
> +    } while (ret < 0 && errno == EINTR);
> +
> +    if (ret < 0) {
> +        virReportSystemError(errno, "%s", _("sendmsg failed") );

Here too.

> +        return -1;
> +    }
> +
> +    return ret;
> +}
> +
> +/**
> + * virSocketRecv:
> + * @sock: socket to poll and receive response on
> + *
> + * This function polls @sock for response
> + * Returns received response or NULL on error.
> + */
> +char *
> +virSocketRecv(int sock)
> +{
> +    struct pollfd pfds[1];
> +    char *buf = NULL;
> +    size_t buf_len = 1024;
> +    int ret;
> +
> +    buf = g_new0(char, buf_len);
> +
> +    pfds[0].fd = sock;
> +    pfds[0].events = POLLIN;
> +
> +    do {
> +        ret = poll(pfds, G_N_ELEMENTS(pfds), PKT_TIMEOUT_MS);
> +    } while (ret < 0 && errno == EINTR);
> +
> +    if (ret <= 0) {
> +        if (ret < 0) {
> +            virReportSystemError(errno, _("Poll on sock %1$d failed"), sock);
> +        } else if (ret == 0) {
> +            virReportSystemError(errno, _("Poll on sock %1$d timed out"), sock);
> +        }
> +        return NULL;
> +    }
> +
> +    do {
> +        ret = recv(sock, buf, buf_len-1, 0);

I still think it's better to check whether there is any data to be read,
but hey - if there's any error waiting on the FD, then recv() will pick
it up and ...

> +    } while (ret < 0 && errno == EINTR);
> +
> +    if (ret < 0) {
> +        virReportSystemError(errno, _("recv on sock %1$d failed"), sock);
> +        return NULL;

... it's going to be propagated here.

> +    }
> +
> +    return g_steal_pointer(&buf);
> +}

Now, this function is put into !WIN32 block and also exposed in private
syms but lacks WIN32 stub. And if it weren't for poll() it could be just
moved out of this block. Now it needs a dummy implementation, otherwise
linking on WIN32 would fail.

> +
>  #else /* WIN32 */
>  int
>  virSocketSendFD(int sock G_GNUC_UNUSED, int fd G_GNUC_UNUSED)
> @@ -496,4 +605,11 @@ virSocketRecvFD(int sock G_GNUC_UNUSED, int fdflags G_GNUC_UNUSED)
>      errno = ENOSYS;
>      return -1;
>  }
> +
> +int
> +virSocketSendMsgWithFDs(int sock, const char *payload, int *fds, size_t fds_len)
> +{
> +    errno = ENOSYS;
> +    return -1;
> +}

Since the original virSocketSendMsgWithFDs() reports an error this stub
must too. Reason? Imagine a caller:

  if (virSocketSendMsgWithFDs(...) < 0) {
    /* Should an error be reported here? */
    return -1;
  }

The answer depends on which variant of the function was called. And
yeah, we could check whether there's an error set, but way easier is to
have both version behave the same in this respect.

>  #endif  /* WIN32 */
> diff --git a/src/util/virsocket.h b/src/util/virsocket.h
> index 419da8b3ae..00fbf52603 100644
> --- a/src/util/virsocket.h
> +++ b/src/util/virsocket.h
> @@ -22,6 +22,9 @@
>  
>  int virSocketSendFD(int sock, int fd);
>  int virSocketRecvFD(int sock, int fdflags);
> +int virSocketSendMsgWithFDs(int sock, const char *payload, int *fds,
> +                            size_t fd_len);
> +char * virSocketRecv(int sock);
>  
>  #ifdef WIN32
>  

Michal
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux