<div dir="ltr"><div>Hi,</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>virSocketRecvFD() { int fds[1]; virSocketRecvMultipleFDs(sock, fds, 1, fdflags); return fds[0]; }</div></blockquote><div>Yea, it's a good idea. </div></div> <div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Aug 20, 2021 at 3:57 PM Michal Prívozník <<a href="mailto:mprivozn@redhat.com">mprivozn@redhat.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 7/28/21 10:17 AM, Andrew Melnychenko wrote: > Similar to virSocketRecvFD() added virSocketRecvMultipleFDs(). > This function returns multiple fds through unix socket. > New function is required for "qemu-ebpf-rss-helper" program. > The helper may pass few file descriptors - eBPF program and maps. > > Signed-off-by: Andrew Melnychenko <<a href="mailto:andrew@daynix.com" target="_blank">andrew@daynix.com</a>> > --- > src/libvirt_private.syms | 1 + > src/util/virsocket.c | 83 ++++++++++++++++++++++++++++++++++++++++ > src/util/virsocket.h | 2 + > 3 files changed, 86 insertions(+) > > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index 43493ea76e..6987ff00c2 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -3226,6 +3226,7 @@ virSecureEraseString; > # util/virsocket.h > virSocketRecvFD; > virSocketSendFD; > +virSocketRecvMultipleFDs; This needs to be ordered. The correct order is: # util/virsocket.h virSocketRecvFD; virSocketRecvMultipleFDs; virSocketSendFD; > > > # util/virsocketaddr.h > diff --git a/src/util/virsocket.c b/src/util/virsocket.c > index b971da16e3..da8af42e72 100644 > --- a/src/util/virsocket.c > +++ b/src/util/virsocket.c > @@ -486,6 +486,82 @@ virSocketRecvFD(int sock, int fdflags) > > return fd; > } > + > + > +/* virSocketRecvMultipleFDs receives few file descriptors through the socket. > + The flags are a bitmask, possibly including O_CLOEXEC (defined in <fcntl.h>). > + > + Return the number of recived file descriptors on success, > + or -1 with errno set in case of error. > +*/ > +int > +virSocketRecvMultipleFDs(int sock, int *fds, size_t nfds, int fdflags) > +{ > + char byte = 0; > + struct iovec iov; > + struct msghdr msg; > + int ret = -1; > + ssize_t len; > + struct cmsghdr *cmsg; > + char buf[CMSG_SPACE(sizeof(int) * nfds)]; > + int fdflags_recvmsg = fdflags & O_CLOEXEC ? MSG_CMSG_CLOEXEC : 0; > + int fdSize = -1; > + int i = 0; > + int saved_errno = 0; > + > + if ((fdflags & ~O_CLOEXEC) != 0) { > + errno = EINVAL; > + return -1; > + } > + > + /* send at least one char */ > + memset(&msg, 0, sizeof(msg)); > + iov.iov_base = &byte; > + iov.iov_len = 1; > + msg.msg_iov = &iov; > + msg.msg_iovlen = 1; > + msg.msg_name = NULL; > + msg.msg_namelen = 0; > + > + msg.msg_control = buf; > + msg.msg_controllen = sizeof(buf); > + > + len = recvmsg(sock, &msg, fdflags_recvmsg); > + if (len < 0) { > + return -1; > + } > + > + cmsg = CMSG_FIRSTHDR(&msg); > + /* be paranoiac */ > + if (len == 0 || cmsg == NULL || cmsg->cmsg_len < CMSG_LEN(sizeof(int)) > + || cmsg->cmsg_level != SOL_SOCKET || cmsg->cmsg_type != SCM_RIGHTS) { > + /* fake errno: at end the file is not available */ > + errno = len ? EACCES : ENOTCONN; > + return -1; > + } > + > + fdSize = cmsg->cmsg_len - CMSG_LEN(0); > + memcpy(fds, CMSG_DATA(cmsg), fdSize); > + ret = fdSize/sizeof(int); Please put a space before and after '/'. Like this: ret = fdSize / sizeof(int); > + > + /* set close-on-exec flag */ > + if (!MSG_CMSG_CLOEXEC && (fdflags & O_CLOEXEC)) { > + for (i = 0; i < ret; ++i) { > + if (virSetCloseExec(fds[i]) < 0) { > + saved_errno = errno; This isn't needed really, because .. > + goto error; > + } > + } > + } > + > + return ret; > +error: > + for (i = 0; i < ret; ++i) { > + VIR_FORCE_CLOSE(fds[i]); .. VIR_FORCE_CLOSE() doesn't change errno. > + } > + errno = saved_errno; > + return -1; > +} But I wonder if this function is needed. I mean, we currently have virSocketRecvFD() and this new function looks very much like it. Would it be possible to turn virSocketRecvFD() into virSocketRecvMultipleFDs() and fix all (current) callers of virSocketRecvFD()? Alternatively, we can have virSocketRecvMultipleFDs() as you propose and then virSocketRecvFD() be just a thin wrapper over virSocketRecvMultipleFDs(), e.g. like this: virSocketRecvFD() { int fds[1]; virSocketRecvMultipleFDs(sock, fds, 1, fdflags); return fds[0]; } Or even better, have virSocketRecvFD() return FD via argument and its retval be 0/-1 (success/fail). My aim is to avoid having nearly the same code twice. Michal </blockquote></div>