| Message ID | 8b5f13cc11701ed8a61273aef88eeee0f0aa44c6.1608263018.git.elena.ufimtseva@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Initial support for multi-process Qemu | expand |
Hi On Fri, Dec 18, 2020 at 7:59 AM <elena.ufimtseva@oracle.com> wrote: > From: Jagannathan Raman <jag.raman@oracle.com> > > Export socket_get_fd() helper function. The function protorype is > changed to be more generic. Use monitor_fd_param() instead of > monitor_fd_get() in order to support named fds as well. > > Signed-off-by: Jagannathan Raman <jag.raman@oracle.com> > Signed-off-by: John G Johnson <john.g.johnson@oracle.com> > Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com> > --- > include/qemu/sockets.h | 1 + > stubs/monitor.c | 2 +- > tests/test-util-sockets.c | 2 +- > util/qemu-sockets.c | 18 ++++++++++-------- > 4 files changed, 13 insertions(+), 10 deletions(-) > > diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h > index 7d1f813576..d87cf9f1c5 100644 > --- a/include/qemu/sockets.h > +++ b/include/qemu/sockets.h > @@ -41,6 +41,7 @@ int unix_listen(const char *path, Error **errp); > int unix_connect(const char *path, Error **errp); > > SocketAddress *socket_parse(const char *str, Error **errp); > +int socket_get_fd(const char *fdstr, Error **errp); > int socket_connect(SocketAddress *addr, Error **errp); > int socket_listen(SocketAddress *addr, int num, Error **errp); > void socket_listen_cleanup(int fd, Error **errp); > diff --git a/stubs/monitor.c b/stubs/monitor.c > index 20786ac4ff..6e6850cd3a 100644 > --- a/stubs/monitor.c > +++ b/stubs/monitor.c > @@ -3,7 +3,7 @@ > #include "monitor/monitor.h" > #include "../monitor/monitor-internal.h" > > -int monitor_get_fd(Monitor *mon, const char *name, Error **errp) > +int monitor_fd_param(Monitor *mon, const char *fdname, Error **errp) > { > error_setg(errp, "only QEMU supports file descriptor passing"); > return -1; > diff --git a/tests/test-util-sockets.c b/tests/test-util-sockets.c > index 67486055ed..e790a0af25 100644 > --- a/tests/test-util-sockets.c > +++ b/tests/test-util-sockets.c > @@ -54,7 +54,7 @@ static int mon_fd = -1; > static const char *mon_fdname; > __thread Monitor *cur_mon; > > -int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp) > +int monitor_fd_param(Monitor *mon, const char *fdname, Error **errp) > { > g_assert(cur_mon); > g_assert(mon == cur_mon); > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c > index 8af0278f15..694552cb7d 100644 > --- a/util/qemu-sockets.c > +++ b/util/qemu-sockets.c > @@ -1116,16 +1116,12 @@ fail: > return NULL; > } > > -static int socket_get_fd(const char *fdstr, int num, Error **errp) > +int socket_get_fd(const char *fdstr, Error **errp) > { > Monitor *cur_mon = monitor_cur(); > int fd; > - if (num != 1) { > - error_setg_errno(errp, EINVAL, "socket_get_fd: too many > connections"); > - return -1; > - } > if (cur_mon) { > - fd = monitor_get_fd(cur_mon, fdstr, errp); > + fd = monitor_fd_param(cur_mon, fdstr, errp); > It's not a good idea to change 2 things in the same patch. Although here the change is fairly small, it may have consequences that are unrelated, and we may want to revert just that. Here, if the fd name is numeric, it will break current users. So I am afraid we can't change it like that. Perhaps add a flag like "named_fd_only" and call one or the other? On a related note, I find it quite confusing that we have -add-fd using fdset, but qmp 'getfd' for named fd (and no command line version?). Some commands can use fdset (all with file path, since /dev/fdset/ makes it accessible), while others rely on monitor_get_fd(). It's an area that could benefit cleanups and better API/docs. That's not for you to do though, just me mumbling. > if (fd < 0) { > return -1; > } > @@ -1159,7 +1155,7 @@ int socket_connect(SocketAddress *addr, Error **errp) > break; > > case SOCKET_ADDRESS_TYPE_FD: > - fd = socket_get_fd(addr->u.fd.str, 1, errp); > + fd = socket_get_fd(addr->u.fd.str, errp); > break; > > case SOCKET_ADDRESS_TYPE_VSOCK: > @@ -1177,6 +1173,12 @@ int socket_listen(SocketAddress *addr, int num, > Error **errp) > int fd; > > trace_socket_listen(num); > + > + if (num != 1) { > + error_setg_errno(errp, EINVAL, "socket_get_fd: too many > connections"); > You should also change the error message to locate socket_listen instead. + return -1; > + } > + > switch (addr->type) { > case SOCKET_ADDRESS_TYPE_INET: > fd = inet_listen_saddr(&addr->u.inet, 0, num, errp); > @@ -1187,7 +1189,7 @@ int socket_listen(SocketAddress *addr, int num, > Error **errp) > break; > > case SOCKET_ADDRESS_TYPE_FD: > - fd = socket_get_fd(addr->u.fd.str, num, errp); > + fd = socket_get_fd(addr->u.fd.str, errp); > break; > > case SOCKET_ADDRESS_TYPE_VSOCK: > -- > 2.25.GIT > >
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h index 7d1f813576..d87cf9f1c5 100644 --- a/include/qemu/sockets.h +++ b/include/qemu/sockets.h @@ -41,6 +41,7 @@ int unix_listen(const char *path, Error **errp); int unix_connect(const char *path, Error **errp); SocketAddress *socket_parse(const char *str, Error **errp); +int socket_get_fd(const char *fdstr, Error **errp); int socket_connect(SocketAddress *addr, Error **errp); int socket_listen(SocketAddress *addr, int num, Error **errp); void socket_listen_cleanup(int fd, Error **errp); diff --git a/stubs/monitor.c b/stubs/monitor.c index 20786ac4ff..6e6850cd3a 100644 --- a/stubs/monitor.c +++ b/stubs/monitor.c @@ -3,7 +3,7 @@ #include "monitor/monitor.h" #include "../monitor/monitor-internal.h" -int monitor_get_fd(Monitor *mon, const char *name, Error **errp) +int monitor_fd_param(Monitor *mon, const char *fdname, Error **errp) { error_setg(errp, "only QEMU supports file descriptor passing"); return -1; diff --git a/tests/test-util-sockets.c b/tests/test-util-sockets.c index 67486055ed..e790a0af25 100644 --- a/tests/test-util-sockets.c +++ b/tests/test-util-sockets.c @@ -54,7 +54,7 @@ static int mon_fd = -1; static const char *mon_fdname; __thread Monitor *cur_mon; -int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp) +int monitor_fd_param(Monitor *mon, const char *fdname, Error **errp) { g_assert(cur_mon); g_assert(mon == cur_mon); diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c index 8af0278f15..694552cb7d 100644 --- a/util/qemu-sockets.c +++ b/util/qemu-sockets.c @@ -1116,16 +1116,12 @@ fail: return NULL; } -static int socket_get_fd(const char *fdstr, int num, Error **errp) +int socket_get_fd(const char *fdstr, Error **errp) { Monitor *cur_mon = monitor_cur(); int fd; - if (num != 1) { - error_setg_errno(errp, EINVAL, "socket_get_fd: too many connections"); - return -1; - } if (cur_mon) { - fd = monitor_get_fd(cur_mon, fdstr, errp); + fd = monitor_fd_param(cur_mon, fdstr, errp); if (fd < 0) { return -1; } @@ -1159,7 +1155,7 @@ int socket_connect(SocketAddress *addr, Error **errp) break; case SOCKET_ADDRESS_TYPE_FD: - fd = socket_get_fd(addr->u.fd.str, 1, errp); + fd = socket_get_fd(addr->u.fd.str, errp); break; case SOCKET_ADDRESS_TYPE_VSOCK: @@ -1177,6 +1173,12 @@ int socket_listen(SocketAddress *addr, int num, Error **errp) int fd; trace_socket_listen(num); + + if (num != 1) { + error_setg_errno(errp, EINVAL, "socket_get_fd: too many connections"); + return -1; + } + switch (addr->type) { case SOCKET_ADDRESS_TYPE_INET: fd = inet_listen_saddr(&addr->u.inet, 0, num, errp); @@ -1187,7 +1189,7 @@ int socket_listen(SocketAddress *addr, int num, Error **errp) break; case SOCKET_ADDRESS_TYPE_FD: - fd = socket_get_fd(addr->u.fd.str, num, errp); + fd = socket_get_fd(addr->u.fd.str, errp); break; case SOCKET_ADDRESS_TYPE_VSOCK: