* [Qemu-devel] [PATCH 1/2] net/socket: convert non-blocking connect to use QIOChannel @ 2017-04-18 8:30 Mao Zhongyi 2017-04-18 8:30 ` [Qemu-devel] [PATCH 2/2] net/socket: Improve -net socket error reporting Mao Zhongyi 0 siblings, 1 reply; 3+ messages in thread From: Mao Zhongyi @ 2017-04-18 8:30 UTC (permalink / raw) To: qemu-devel; +Cc: jasowang The non-blocking connect mechanism doesn't work well in net connection, it still blocks on DNS lookups. So it is obsolete. Supersede it with QIOchannel. Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com> --- The test steps like this: $ qemu-system-x86_64 -net nic -net socket,listen=:1234 ~/img/test.img $ qemu-system-x86_64 -net nic -net socket,connect=127.0.0.1:1234 ~/img/test.img No exception. net/socket.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/net/socket.c b/net/socket.c index fe3547b..1886f98 100644 --- a/net/socket.c +++ b/net/socket.c @@ -33,6 +33,7 @@ #include "qemu/sockets.h" #include "qemu/iov.h" #include "qemu/main-loop.h" +#include "io/channel-socket.h" typedef struct NetSocketState { NetClientState nc; @@ -525,16 +526,21 @@ typedef struct { char *name; } socket_connect_data; -static void socket_connect_data_free(socket_connect_data *c) +static void socket_connect_data_free(void *opaque) { + socket_connect_data *c = opaque; + if (!c) { + return; + } qapi_free_SocketAddress(c->saddr); g_free(c->model); g_free(c->name); g_free(c); } -static void net_socket_connected(int fd, Error *err, void *opaque) +static void net_socket_connected(QIOTask *task, void *opaque) { + QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(qio_task_get_source(task)); socket_connect_data *c = opaque; NetSocketState *s; char *addr_str = NULL; @@ -543,13 +549,13 @@ static void net_socket_connected(int fd, Error *err, void *opaque) addr_str = socket_address_to_string(c->saddr, &local_error); if (addr_str == NULL) { error_report_err(local_error); - closesocket(fd); + closesocket(sioc->fd); goto end; } - s = net_socket_fd_init(c->peer, c->model, c->name, fd, true); + s = net_socket_fd_init(c->peer, c->model, c->name, sioc->fd, true); if (!s) { - closesocket(fd); + closesocket(sioc->fd); goto end; } @@ -567,7 +573,7 @@ static int net_socket_connect_init(NetClientState *peer, const char *host_str) { socket_connect_data *c = g_new0(socket_connect_data, 1); - int fd = -1; + QIOChannelSocket *sioc; Error *local_error = NULL; c->peer = peer; @@ -578,10 +584,12 @@ static int net_socket_connect_init(NetClientState *peer, goto err; } - fd = socket_connect(c->saddr, &local_error, net_socket_connected, c); - if (fd < 0) { - goto err; - } + sioc = qio_channel_socket_new(); + qio_channel_socket_connect_async(sioc, + c->saddr, + net_socket_connected, + c, + NULL); return 0; -- 2.9.3 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* [Qemu-devel] [PATCH 2/2] net/socket: Improve -net socket error reporting 2017-04-18 8:30 [Qemu-devel] [PATCH 1/2] net/socket: convert non-blocking connect to use QIOChannel Mao Zhongyi @ 2017-04-18 8:30 ` Mao Zhongyi 2017-04-18 16:02 ` Markus Armbruster 0 siblings, 1 reply; 3+ messages in thread From: Mao Zhongyi @ 2017-04-18 8:30 UTC (permalink / raw) To: qemu-devel; +Cc: jasowang When -net socket fails, it first reports a specific error, then a generic one, like this: $ qemu-system-x86_64 -net socket, qemu-system-x86_64: -net socket: exactly one of fd=, listen=, connect=, mcast= or udp= is required qemu-system-x86_64: -net socket: Device 'socket' could not be initialized Convert net_init_socket() to Error to get rid of the unwanted second error message. Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com> --- net/socket.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/net/socket.c b/net/socket.c index 1886f98..85ad9cd 100644 --- a/net/socket.c +++ b/net/socket.c @@ -691,7 +691,6 @@ static int net_socket_udp_init(NetClientState *peer, int net_init_socket(const Netdev *netdev, const char *name, NetClientState *peer, Error **errp) { - /* FIXME error_setg(errp, ...) on failure */ Error *err = NULL; const NetdevSocketOptions *sock; @@ -700,13 +699,13 @@ int net_init_socket(const Netdev *netdev, const char *name, if (sock->has_fd + sock->has_listen + sock->has_connect + sock->has_mcast + sock->has_udp != 1) { - error_report("exactly one of fd=, listen=, connect=, mcast= or udp=" + error_setg(errp, "exactly one of fd=, listen=, connect=, mcast= or udp=" " is required"); return -1; } if (sock->has_localaddr && !sock->has_mcast && !sock->has_udp) { - error_report("localaddr= is only valid with mcast= or udp="); + error_setg(errp, "localaddr= is only valid with mcast= or udp="); return -1; } @@ -752,7 +751,7 @@ int net_init_socket(const Netdev *netdev, const char *name, assert(sock->has_udp); if (!sock->has_localaddr) { - error_report("localaddr= is mandatory with udp="); + error_setg(errp, "localaddr= is mandatory with udp="); return -1; } if (net_socket_udp_init(peer, "socket", name, sock->udp, sock->localaddr) == -- 2.9.3 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] net/socket: Improve -net socket error reporting 2017-04-18 8:30 ` [Qemu-devel] [PATCH 2/2] net/socket: Improve -net socket error reporting Mao Zhongyi @ 2017-04-18 16:02 ` Markus Armbruster 0 siblings, 0 replies; 3+ messages in thread From: Markus Armbruster @ 2017-04-18 16:02 UTC (permalink / raw) To: Mao Zhongyi; +Cc: qemu-devel, jasowang Mao Zhongyi <maozy.fnst@cn.fujitsu.com> writes: > When -net socket fails, it first reports a specific error, then > a generic one, like this: > > $ qemu-system-x86_64 -net socket, > qemu-system-x86_64: -net socket: exactly one of fd=, listen=, connect=, mcast= or udp= is required > qemu-system-x86_64: -net socket: Device 'socket' could not be initialized > > Convert net_init_socket() to Error to get rid of the unwanted second > error message. > > Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com> Thanks for trying to tackle this. > --- > net/socket.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/net/socket.c b/net/socket.c > index 1886f98..85ad9cd 100644 > --- a/net/socket.c > +++ b/net/socket.c > @@ -691,7 +691,6 @@ static int net_socket_udp_init(NetClientState *peer, > int net_init_socket(const Netdev *netdev, const char *name, > NetClientState *peer, Error **errp) > { > - /* FIXME error_setg(errp, ...) on failure */ > Error *err = NULL; > const NetdevSocketOptions *sock; > > @@ -700,13 +699,13 @@ int net_init_socket(const Netdev *netdev, const char *name, > > if (sock->has_fd + sock->has_listen + sock->has_connect + sock->has_mcast + > sock->has_udp != 1) { > - error_report("exactly one of fd=, listen=, connect=, mcast= or udp=" > + error_setg(errp, "exactly one of fd=, listen=, connect=, mcast= or udp=" > " is required"); > return -1; > } > > if (sock->has_localaddr && !sock->has_mcast && !sock->has_udp) { > - error_report("localaddr= is only valid with mcast= or udp="); > + error_setg(errp, "localaddr= is only valid with mcast= or udp="); > return -1; > } > if (sock->has_fd) { int fd; fd = monitor_fd_param(cur_mon, sock->fd, &err); if (fd == -1) { error_report_err(err); You missed this one. return -1; } qemu_set_nonblock(fd); if (!net_socket_fd_init(peer, "socket", name, fd, 1)) { return -1; } return 0; } if (sock->has_listen) { if (net_socket_listen_init(peer, "socket", name, sock->listen) == -1) { net_socket_listen_init() calls error_report_err(). You need to convert it to Error. return -1; } return 0; } if (sock->has_connect) { if (net_socket_connect_init(peer, "socket", name, sock->connect) == -1) { Likewise. You also need to convert net_socket_connected(). You should review everything called by this function for use of error_report() or similar. return -1; } return 0; } if (sock->has_mcast) { /* if sock->localaddr is missing, it has been initialized to "all bits * zero" */ if (net_socket_mcast_init(peer, "socket", name, sock->mcast, sock->localaddr) == -1) { Different kind of problem here: this function can fail without reporting an error. You need to fix it to set an error when it fails. return -1; } return 0; } > @@ -752,7 +751,7 @@ int net_init_socket(const Netdev *netdev, const char *name, > > assert(sock->has_udp); > if (!sock->has_localaddr) { > - error_report("localaddr= is mandatory with udp="); > + error_setg(errp, "localaddr= is mandatory with udp="); > return -1; > } > if (net_socket_udp_init(peer, "socket", name, sock->udp, sock->localaddr) == ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-04-18 16:02 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-18 8:30 [Qemu-devel] [PATCH 1/2] net/socket: convert non-blocking connect to use QIOChannel Mao Zhongyi 2017-04-18 8:30 ` [Qemu-devel] [PATCH 2/2] net/socket: Improve -net socket error reporting Mao Zhongyi 2017-04-18 16:02 ` Markus Armbruster
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.