[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH v5 05/14] net: port socket to GSource
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [RFC PATCH v5 05/14] net: port socket to GSource |
Date: |
Fri, 26 Apr 2013 11:48:22 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Fri, Apr 26, 2013 at 10:47:26AM +0800, Liu Ping Fan wrote:
> @@ -141,6 +134,59 @@ static ssize_t net_socket_receive_dgram(NetClientState
> *nc, const uint8_t *buf,
> return ret;
> }
>
> +static gushort socket_connecting_readable(void *opaque)
> +{
> + return G_IO_IN;
> +}
> +
> +static gushort socket_listen_readable(void *opaque)
> +{
> + /* listen only handle in-req, no err */
> + return G_IO_IN;
>From the accept(2) man page:
"Linux accept() (and accept4()) passes already-pending network errors on
the new socket as an error code from accept()."
So we must handle errors from accept(2), please use G_IO_IN | G_IO_HUP |
G_IO_ERR.
> +static gushort socket_establish_readable(void *opaque)
> +{
> + NetSocketState *s = opaque;
> +
> + /* rely on net_socket_send to handle err */
> + if (s->read_poll && net_socket_can_send(s)) {
> + return G_IO_IN|G_IO_HUP|G_IO_ERR;
> + }
> + return G_IO_HUP|G_IO_ERR;
> +}
This new function always monitors G_IO_HUP | G_IO_ERR. The old code
only monitored it when read_poll == true and net_socket_can_send() ==
true.
Please preserve semantics.
> +static gushort socket_establish_writable(void *opaque)
> +{
> + NetSocketState *s = opaque;
> +
> + if (s->write_poll) {
> + return G_IO_OUT;
Errors/hang up?
> @@ -440,9 +529,20 @@ static NetSocketState
> *net_socket_fd_init_stream(NetClientState *peer,
> s->listen_fd = -1;
>
> if (is_connected) {
> - net_socket_connect(s);
> + assert(!s->nsrc);
> + s->nsrc = event_source_new(fd, net_socket_establish_handler, s);
> + s->nsrc->readable = socket_establish_readable;
> + s->nsrc->writable = socket_establish_writable;
> + nc->info->bind_ctx(nc, NULL);
> + net_socket_read_poll(s, true);
> + net_socket_write_poll(s, true);
> } else {
> - qemu_set_fd_handler(s->fd, NULL, net_socket_connect, s);
> + assert(!s->nsrc);
> + s->nsrc = event_source_new(fd, net_socket_connect_handler, s);
> + s->nsrc->readable = socket_connecting_readable;
The original code wants writeable, not readable.
> +static gboolean net_socket_listen_handler(gpointer data)
> +{
> + EventGSource *nsrc = data;
> + NetSocketState *s = nsrc->opaque;
> struct sockaddr_in saddr;
> socklen_t len;
> int fd;
>
> - for(;;) {
> - len = sizeof(saddr);
> - fd = qemu_accept(s->listen_fd, (struct sockaddr *)&saddr, &len);
> - if (fd < 0 && errno != EINTR) {
> - return;
> - } else if (fd >= 0) {
> - qemu_set_fd_handler(s->listen_fd, NULL, NULL, NULL);
> - break;
> - }
> + len = sizeof(saddr);
> + fd = qemu_accept(s->listen_fd, (struct sockaddr *)&saddr, &len);
> + if (fd < 0 && errno != EINTR) {
> + return false;
> }
This breaks the code when accept(2) is interrupted by a signal and we
get -1, errno == EINTR. Why did you remove the loop?
- [Qemu-devel] [RFC PATCH v5 00/14] port network layer onto glib, Liu Ping Fan, 2013/04/25
- [Qemu-devel] [RFC PATCH v5 02/14] net: introduce bind_ctx to NetClientInfo, Liu Ping Fan, 2013/04/25
- [Qemu-devel] [RFC PATCH v5 03/14] net: port tap onto GSource, Liu Ping Fan, 2013/04/25
- [Qemu-devel] [RFC PATCH v5 04/14] net: port vde onto GSource, Liu Ping Fan, 2013/04/25
- [Qemu-devel] [RFC PATCH v5 05/14] net: port socket to GSource, Liu Ping Fan, 2013/04/25
- Re: [Qemu-devel] [RFC PATCH v5 05/14] net: port socket to GSource,
Stefan Hajnoczi <=
[Qemu-devel] [RFC PATCH v5 06/14] net: port tap-win32 onto GSource, Liu Ping Fan, 2013/04/25
[Qemu-devel] [RFC PATCH v5 07/14] net: hub use lock to protect ports list, Liu Ping Fan, 2013/04/25
[Qemu-devel] [RFC PATCH v5 08/14] net: introduce lock to protect NetQueue, Liu Ping Fan, 2013/04/25
[Qemu-devel] [RFC PATCH v5 09/14] net: introduce lock to protect NetClientState's peer's access, Liu Ping Fan, 2013/04/25
[Qemu-devel] [RFC PATCH v5 10/14] net: make netclient re-entrant with refcnt, Liu Ping Fan, 2013/04/25
[Qemu-devel] [RFC PATCH v5 11/14] slirp: make timeout local, Liu Ping Fan, 2013/04/25
[Qemu-devel] [RFC PATCH v5 12/14] slirp: make slirp event dispatch based on slirp instance, not global, Liu Ping Fan, 2013/04/25
[Qemu-devel] [RFC PATCH v5 13/14] slirp: handle race condition, Liu Ping Fan, 2013/04/25