On 9/28/22 07:55, David Gibson wrote:
+static int net_stream_server_init(NetClientState *peer,
+ const char *model,
+ const char *name,
+ SocketAddress *addr,
+ Error **errp)
+{
+ NetClientState *nc;
+ NetStreamState *s;
+ int fd, ret;
+
+ switch (addr->type) {
+ case SOCKET_ADDRESS_TYPE_INET: {
+ struct sockaddr_in saddr_in;
+
+ if (convert_host_port(&saddr_in, addr->u.inet.host, addr->u.inet.port,
+ errp) < 0) {
+ return -1;
+ }
+
+ fd = qemu_socket(PF_INET, SOCK_STREAM, 0);
+ if (fd < 0) {
+ error_setg_errno(errp, errno, "can't create stream socket");
+ return -1;
+ }
+ qemu_socket_set_nonblock(fd);
+
+ socket_set_fast_reuse(fd);
+
+ ret = bind(fd, (struct sockaddr *)&saddr_in, sizeof(saddr_in));
+ if (ret < 0) {
+ error_setg_errno(errp, errno, "can't bind ip=%s to socket",
+ inet_ntoa(saddr_in.sin_addr));
+ closesocket(fd);
+ return -1;
+ }
+ break;
+ }
+ case SOCKET_ADDRESS_TYPE_FD:
+ fd = monitor_fd_param(monitor_cur(), addr->u.fd.str, errp);
+ if (fd == -1) {
+ return -1;
+ }
+ ret = qemu_socket_try_set_nonblock(fd);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "%s: Can't use file descriptor %d",
+ name, fd);
+ return -1;
+ }
+ break;
+ default:
+ error_setg(errp, "only support inet or fd type");
+ return -1;
+ }
+
+ ret = listen(fd, 0);
Does this make sense for a passed in fd? If someone passes a "server"
fd, are they likely to be passing a socket on which bind() but not
listen() has been called? Or one on which both bind() and listen()
have been called?
Original code in net/socket.c doesn't manage server case with fd.
So I have checked what is done for QIO (all this code is overwritten by patch introducing
QIO anyway):
At the end of the series, we use qio_channel_socket_listen_async() in
net_stream_server_init(), that in the end calls socket_listen().
With SOCKET_ADDRESS_TYPE_FD we does the listen() (without bind()) with the
following comment:
case SOCKET_ADDRESS_TYPE_FD:
fd = socket_get_fd(addr->u.fd.str, errp);
if (fd < 0) {
return -1;
}
/*
* If the socket is not yet in the listen state, then transition it to
* the listen state now.
*
* If it's already listening then this updates the backlog value as
* requested.
*
* If this socket cannot listen because it's already in another state
* (e.g. unbound or connected) then we'll catch the error here.
*/
if (listen(fd, num) != 0) {
error_setg_errno(errp, errno, "Failed to listen on fd socket");
closesocket(fd);
return -1;
}
break;
So I think we should keep the listen() in our case too.
Thanks,
Laurent
