* [Qemu-devel] [PATCH v1 1/5] util: introduce gsource event abstraction
2013-08-08 6:26 [Qemu-devel] [PATCH v1 0/5] make slirp subsystem self-contained Liu Ping Fan
@ 2013-08-08 6:26 ` Liu Ping Fan
2013-08-08 16:29 ` Michael Roth
2013-08-08 21:03 ` Michael Roth
2013-08-08 6:26 ` [Qemu-devel] [PATCH v1 2/5] slirp: make timeout local Liu Ping Fan
` (3 subsequent siblings)
4 siblings, 2 replies; 14+ messages in thread
From: Liu Ping Fan @ 2013-08-08 6:26 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, mdroth, Stefan Hajnoczi
Introduce struct EventsGSource. It will ease the usage of GSource
associated with a group of files, which are dynamically allocated
and release, ex, slirp.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
util/Makefile.objs | 1 +
util/event_gsource.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++
util/event_gsource.h | 37 +++++++++++++++++++++
3 files changed, 132 insertions(+)
create mode 100644 util/event_gsource.c
create mode 100644 util/event_gsource.h
diff --git a/util/Makefile.objs b/util/Makefile.objs
index dc72ab0..eec55bd 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -11,3 +11,4 @@ util-obj-y += iov.o aes.o qemu-config.o qemu-sockets.o uri.o notify.o
util-obj-y += qemu-option.o qemu-progress.o
util-obj-y += hexdump.o
util-obj-y += crc32c.o
+util-obj-y += event_gsource.o
diff --git a/util/event_gsource.c b/util/event_gsource.c
new file mode 100644
index 0000000..4b9fa89
--- /dev/null
+++ b/util/event_gsource.c
@@ -0,0 +1,94 @@
+/*
+ * Copyright (C) 2013 IBM
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; under version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "event_gsource.h"
+#include "qemu/bitops.h"
+
+GPollFD *events_source_add_pollfd(EventsGSource *src, int fd)
+{
+ GPollFD *retfd;
+
+ retfd = g_slice_alloc(sizeof(GPollFD));
+ retfd->events = 0;
+ retfd->fd = fd;
+ src->pollfds_list = g_list_append(src->pollfds_list, retfd);
+ if (fd >= 0) {
+ g_source_add_poll(&src->source, retfd);
+ }
+
+ return retfd;
+}
+
+void events_source_remove_pollfd(EventsGSource *src, GPollFD *pollfd)
+{
+ g_source_remove_poll(&src->source, pollfd);
+ src->pollfds_list = g_list_remove(src->pollfds_list, pollfd);
+ g_slice_free(GPollFD, pollfd);
+}
+
+static gboolean events_source_check(GSource *src)
+{
+ EventsGSource *nsrc = (EventsGSource *)src;
+ GList *cur;
+ GPollFD *gfd;
+
+ cur = nsrc->pollfds_list;
+ while (cur) {
+ gfd = cur->data;
+ if (gfd->fd >= 0 && (gfd->revents & gfd->events)) {
+ return true;
+ }
+ cur = g_list_next(cur);
+ }
+
+ return false;
+}
+
+static gboolean events_source_dispatch(GSource *src, GSourceFunc cb,
+ gpointer data)
+{
+ gboolean ret = false;
+
+ if (cb) {
+ ret = cb(data);
+ }
+ return ret;
+}
+
+EventsGSource *events_source_new(GPrepare prepare, GSourceFunc dispatch_cb,
+ void *opaque)
+{
+ EventsGSource *src;
+ GSourceFuncs *gfuncs = g_new0(GSourceFuncs, 1);
+ gfuncs->prepare = prepare;
+ gfuncs->check = events_source_check,
+ gfuncs->dispatch = events_source_dispatch,
+
+ src = (EventsGSource *)g_source_new(gfuncs, sizeof(EventsGSource));
+ src->gfuncs = gfuncs;
+ src->pollfds_list = NULL;
+ src->opaque = opaque;
+ g_source_set_callback(&src->source, dispatch_cb, src, NULL);
+
+ return src;
+}
+
+void events_source_release(EventsGSource *src)
+{
+ assert(!src->pollfds_list);
+ g_free(src->gfuncs);
+ g_source_destroy(&src->source);
+}
diff --git a/util/event_gsource.h b/util/event_gsource.h
new file mode 100644
index 0000000..8755952
--- /dev/null
+++ b/util/event_gsource.h
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2013 IBM
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; under version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef EVENT_GSOURCE_H
+#define EVENT_GSOURCE_H
+#include "qemu-common.h"
+
+typedef gboolean (*GPrepare)(GSource *source, gint *timeout_);
+
+/* multi fd drive GSource*/
+typedef struct EventsGSource {
+ GSource source;
+ /* a group of GPollFD which dynamically join or leave the GSource */
+ GList *pollfds_list;
+ GSourceFuncs *gfuncs;
+ void *opaque;
+} EventsGSource;
+
+EventsGSource *events_source_new(GPrepare prepare, GSourceFunc dispatch_cb,
+ void *opaque);
+void events_source_release(EventsGSource *src);
+GPollFD *events_source_add_pollfd(EventsGSource *src, int fd);
+void events_source_remove_pollfd(EventsGSource *src, GPollFD *pollfd);
+#endif
--
1.8.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH v1 1/5] util: introduce gsource event abstraction
2013-08-08 6:26 ` [Qemu-devel] [PATCH v1 1/5] util: introduce gsource event abstraction Liu Ping Fan
@ 2013-08-08 16:29 ` Michael Roth
2013-08-09 7:10 ` liu ping fan
2013-08-08 21:03 ` Michael Roth
1 sibling, 1 reply; 14+ messages in thread
From: Michael Roth @ 2013-08-08 16:29 UTC (permalink / raw)
To: Liu Ping Fan, qemu-devel; +Cc: Paolo Bonzini, Stefan Hajnoczi
Quoting Liu Ping Fan (2013-08-08 01:26:07)
> Introduce struct EventsGSource. It will ease the usage of GSource
> associated with a group of files, which are dynamically allocated
> and release, ex, slirp.
>
> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
> ---
> util/Makefile.objs | 1 +
> util/event_gsource.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> util/event_gsource.h | 37 +++++++++++++++++++++
> 3 files changed, 132 insertions(+)
> create mode 100644 util/event_gsource.c
> create mode 100644 util/event_gsource.h
>
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index dc72ab0..eec55bd 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -11,3 +11,4 @@ util-obj-y += iov.o aes.o qemu-config.o qemu-sockets.o uri.o notify.o
> util-obj-y += qemu-option.o qemu-progress.o
> util-obj-y += hexdump.o
> util-obj-y += crc32c.o
> +util-obj-y += event_gsource.o
> diff --git a/util/event_gsource.c b/util/event_gsource.c
> new file mode 100644
> index 0000000..4b9fa89
> --- /dev/null
> +++ b/util/event_gsource.c
> @@ -0,0 +1,94 @@
> +/*
> + * Copyright (C) 2013 IBM
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; under version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "event_gsource.h"
> +#include "qemu/bitops.h"
> +
> +GPollFD *events_source_add_pollfd(EventsGSource *src, int fd)
Small nit, but if the class is EventsGSource, the methods should
use the events_gsource_* prefix. Or we can just call it EventsSource.
> +{
> + GPollFD *retfd;
> +
> + retfd = g_slice_alloc(sizeof(GPollFD));
> + retfd->events = 0;
> + retfd->fd = fd;
> + src->pollfds_list = g_list_append(src->pollfds_list, retfd);
> + if (fd >= 0) {
> + g_source_add_poll(&src->source, retfd);
> + }
> +
> + return retfd;
> +}
> +
> +void events_source_remove_pollfd(EventsGSource *src, GPollFD *pollfd)
> +{
> + g_source_remove_poll(&src->source, pollfd);
> + src->pollfds_list = g_list_remove(src->pollfds_list, pollfd);
> + g_slice_free(GPollFD, pollfd);
> +}
> +
> +static gboolean events_source_check(GSource *src)
> +{
> + EventsGSource *nsrc = (EventsGSource *)src;
> + GList *cur;
> + GPollFD *gfd;
> +
> + cur = nsrc->pollfds_list;
> + while (cur) {
> + gfd = cur->data;
> + if (gfd->fd >= 0 && (gfd->revents & gfd->events)) {
> + return true;
> + }
> + cur = g_list_next(cur);
> + }
> +
> + return false;
> +}
> +
> +static gboolean events_source_dispatch(GSource *src, GSourceFunc cb,
> + gpointer data)
> +{
> + gboolean ret = false;
> +
> + if (cb) {
> + ret = cb(data);
> + }
> + return ret;
> +}
> +
> +EventsGSource *events_source_new(GPrepare prepare, GSourceFunc dispatch_cb,
> + void *opaque)
> +{
> + EventsGSource *src;
> + GSourceFuncs *gfuncs = g_new0(GSourceFuncs, 1);
> + gfuncs->prepare = prepare;
I'm not sure how useful this EventsGSource abstraction is if it requires users
to implement a custom prepare() function that accesses EventsGSource fields
directly.
Either we should just make this SlirpGSource until another user comes
along where we can look at pulling out common bits, or we should pass in a
prepare() function that operates on the opaque data instead of the
underlying EventsGSource.
If you take the latter approach, you might consider having
events_source_add_pollfd take a GPollFD* arg instead of an fd, then storing
a pointer to the same GPollFD* in your opaque/Slirp object so you can do things
like set the event masks for all the GPollFDs in the prepare cb prior to
completing the GSource's prepare function (which could then do something generic
like return true if any GPollFDs have a non-zero event mask)
> + gfuncs->check = events_source_check,
> + gfuncs->dispatch = events_source_dispatch,
> +
> + src = (EventsGSource *)g_source_new(gfuncs, sizeof(EventsGSource));
> + src->gfuncs = gfuncs;
> + src->pollfds_list = NULL;
> + src->opaque = opaque;
> + g_source_set_callback(&src->source, dispatch_cb, src, NULL);
> +
> + return src;
> +}
> +
> +void events_source_release(EventsGSource *src)
> +{
> + assert(!src->pollfds_list);
> + g_free(src->gfuncs);
> + g_source_destroy(&src->source);
> +}
> diff --git a/util/event_gsource.h b/util/event_gsource.h
> new file mode 100644
> index 0000000..8755952
> --- /dev/null
> +++ b/util/event_gsource.h
> @@ -0,0 +1,37 @@
> +/*
> + * Copyright (C) 2013 IBM
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; under version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef EVENT_GSOURCE_H
> +#define EVENT_GSOURCE_H
> +#include "qemu-common.h"
> +
> +typedef gboolean (*GPrepare)(GSource *source, gint *timeout_);
> +
> +/* multi fd drive GSource*/
> +typedef struct EventsGSource {
> + GSource source;
> + /* a group of GPollFD which dynamically join or leave the GSource */
> + GList *pollfds_list;
> + GSourceFuncs *gfuncs;
> + void *opaque;
> +} EventsGSource;
> +
> +EventsGSource *events_source_new(GPrepare prepare, GSourceFunc dispatch_cb,
> + void *opaque);
> +void events_source_release(EventsGSource *src);
> +GPollFD *events_source_add_pollfd(EventsGSource *src, int fd);
> +void events_source_remove_pollfd(EventsGSource *src, GPollFD *pollfd);
> +#endif
> --
> 1.8.1.4
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH v1 1/5] util: introduce gsource event abstraction
2013-08-08 16:29 ` Michael Roth
@ 2013-08-09 7:10 ` liu ping fan
0 siblings, 0 replies; 14+ messages in thread
From: liu ping fan @ 2013-08-09 7:10 UTC (permalink / raw)
To: Michael Roth; +Cc: Paolo Bonzini, qemu-devel, Stefan Hajnoczi
On Fri, Aug 9, 2013 at 12:29 AM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> Quoting Liu Ping Fan (2013-08-08 01:26:07)
>> Introduce struct EventsGSource. It will ease the usage of GSource
>> associated with a group of files, which are dynamically allocated
>> and release, ex, slirp.
>>
>> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
>> ---
>> util/Makefile.objs | 1 +
>> util/event_gsource.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> util/event_gsource.h | 37 +++++++++++++++++++++
>> 3 files changed, 132 insertions(+)
>> create mode 100644 util/event_gsource.c
>> create mode 100644 util/event_gsource.h
>>
>> diff --git a/util/Makefile.objs b/util/Makefile.objs
>> index dc72ab0..eec55bd 100644
>> --- a/util/Makefile.objs
>> +++ b/util/Makefile.objs
>> @@ -11,3 +11,4 @@ util-obj-y += iov.o aes.o qemu-config.o qemu-sockets.o uri.o notify.o
>> util-obj-y += qemu-option.o qemu-progress.o
>> util-obj-y += hexdump.o
>> util-obj-y += crc32c.o
>> +util-obj-y += event_gsource.o
>> diff --git a/util/event_gsource.c b/util/event_gsource.c
>> new file mode 100644
>> index 0000000..4b9fa89
>> --- /dev/null
>> +++ b/util/event_gsource.c
>> @@ -0,0 +1,94 @@
>> +/*
>> + * Copyright (C) 2013 IBM
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; under version 2 of the License.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include "event_gsource.h"
>> +#include "qemu/bitops.h"
>> +
>> +GPollFD *events_source_add_pollfd(EventsGSource *src, int fd)
>
> Small nit, but if the class is EventsGSource, the methods should
> use the events_gsource_* prefix. Or we can just call it EventsSource.
>
Will fix.
[...]
>> +
>> +EventsGSource *events_source_new(GPrepare prepare, GSourceFunc dispatch_cb,
>> + void *opaque)
>> +{
>> + EventsGSource *src;
>> + GSourceFuncs *gfuncs = g_new0(GSourceFuncs, 1);
>> + gfuncs->prepare = prepare;
>
> I'm not sure how useful this EventsGSource abstraction is if it requires users
> to implement a custom prepare() function that accesses EventsGSource fields
> directly.
>
> Either we should just make this SlirpGSource until another user comes
> along where we can look at pulling out common bits, or we should pass in a
> prepare() function that operates on the opaque data instead of the
> underlying EventsGSource.
>
Maybe SlirpGSource, since the prepare of slirp is too complicated.
> If you take the latter approach, you might consider having
> events_source_add_pollfd take a GPollFD* arg instead of an fd, then storing
> a pointer to the same GPollFD* in your opaque/Slirp object so you can do things
The GPollFD* is stored in slirp's socket (each slirp-socket has a sock-fd ).
> like set the event masks for all the GPollFDs in the prepare cb prior to
> completing the GSource's prepare function (which could then do something generic
> like return true if any GPollFDs have a non-zero event mask)
>
What is the aim of "masks for all the GPollFDs"? We just poll the
GPollFD when so->state ask us to do that. Otherwise they are kept
clean.
Thanks and regards,
Pingfan
>> + gfuncs->check = events_source_check,
>> + gfuncs->dispatch = events_source_dispatch,
>> +
>> + src = (EventsGSource *)g_source_new(gfuncs, sizeof(EventsGSource));
>> + src->gfuncs = gfuncs;
>> + src->pollfds_list = NULL;
>> + src->opaque = opaque;
>> + g_source_set_callback(&src->source, dispatch_cb, src, NULL);
>> +
>> + return src;
>> +}
>> +
>> +void events_source_release(EventsGSource *src)
>> +{
>> + assert(!src->pollfds_list);
>> + g_free(src->gfuncs);
>> + g_source_destroy(&src->source);
>> +}
>> diff --git a/util/event_gsource.h b/util/event_gsource.h
>> new file mode 100644
>> index 0000000..8755952
>> --- /dev/null
>> +++ b/util/event_gsource.h
>> @@ -0,0 +1,37 @@
>> +/*
>> + * Copyright (C) 2013 IBM
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; under version 2 of the License.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#ifndef EVENT_GSOURCE_H
>> +#define EVENT_GSOURCE_H
>> +#include "qemu-common.h"
>> +
>> +typedef gboolean (*GPrepare)(GSource *source, gint *timeout_);
>> +
>> +/* multi fd drive GSource*/
>> +typedef struct EventsGSource {
>> + GSource source;
>> + /* a group of GPollFD which dynamically join or leave the GSource */
>> + GList *pollfds_list;
>> + GSourceFuncs *gfuncs;
>> + void *opaque;
>> +} EventsGSource;
>> +
>> +EventsGSource *events_source_new(GPrepare prepare, GSourceFunc dispatch_cb,
>> + void *opaque);
>> +void events_source_release(EventsGSource *src);
>> +GPollFD *events_source_add_pollfd(EventsGSource *src, int fd);
>> +void events_source_remove_pollfd(EventsGSource *src, GPollFD *pollfd);
>> +#endif
>> --
>> 1.8.1.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/5] util: introduce gsource event abstraction
2013-08-08 6:26 ` [Qemu-devel] [PATCH v1 1/5] util: introduce gsource event abstraction Liu Ping Fan
2013-08-08 16:29 ` Michael Roth
@ 2013-08-08 21:03 ` Michael Roth
2013-08-08 21:12 ` Michael Roth
2013-08-14 19:26 ` Michael Roth
1 sibling, 2 replies; 14+ messages in thread
From: Michael Roth @ 2013-08-08 21:03 UTC (permalink / raw)
To: Liu Ping Fan, qemu-devel; +Cc: Paolo Bonzini, Stefan Hajnoczi
Quoting Liu Ping Fan (2013-08-08 01:26:07)
> Introduce struct EventsGSource. It will ease the usage of GSource
> associated with a group of files, which are dynamically allocated
> and release, ex, slirp.
>
> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
> ---
> util/Makefile.objs | 1 +
> util/event_gsource.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> util/event_gsource.h | 37 +++++++++++++++++++++
> 3 files changed, 132 insertions(+)
> create mode 100644 util/event_gsource.c
> create mode 100644 util/event_gsource.h
>
> diff --git a/util/Makefile.objs b/util/Makefile.objs
> index dc72ab0..eec55bd 100644
> --- a/util/Makefile.objs
> +++ b/util/Makefile.objs
> @@ -11,3 +11,4 @@ util-obj-y += iov.o aes.o qemu-config.o qemu-sockets.o uri.o notify.o
> util-obj-y += qemu-option.o qemu-progress.o
> util-obj-y += hexdump.o
> util-obj-y += crc32c.o
> +util-obj-y += event_gsource.o
> diff --git a/util/event_gsource.c b/util/event_gsource.c
> new file mode 100644
> index 0000000..4b9fa89
> --- /dev/null
> +++ b/util/event_gsource.c
> @@ -0,0 +1,94 @@
> +/*
> + * Copyright (C) 2013 IBM
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; under version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "event_gsource.h"
> +#include "qemu/bitops.h"
> +
> +GPollFD *events_source_add_pollfd(EventsGSource *src, int fd)
> +{
> + GPollFD *retfd;
> +
> + retfd = g_slice_alloc(sizeof(GPollFD));
> + retfd->events = 0;
> + retfd->fd = fd;
> + src->pollfds_list = g_list_append(src->pollfds_list, retfd);
I think moving to a GSource to simplify our mainloop implementation is
worthwhile even if we still rely on the global mutex and don't initially
support running those GSources outside the main iothread. But since being
able to eventually offload NetClient backends to seperate events loops to
support things like virtio-net dataplane is (I assume) still one of the
eventual goals, we should consider how to deal with concurrent
access to EventsGSource members.
For instance, In the case of slirp your dispatch callback may modify
src->pollfds_lists via
slirp_handler->tcp_input->socreate->events_source_add_pollfd(), while
another thread attempts to call socreate() via something like
net_slirp_hostfwd_add from the monitor (that's being driven by a separate
main loop).
So events_source_add_pollfd() and the various prepare/check/dispatch
functions should take a lock on pollfds_lists.
Dispatch is tricky though, since dispatch() invoke callbacks that may in
turn try to call events_source_add_pollfd(), as is the case above, in which
case you can deadlock.
I've been looking at the situation with regard to moving
qemu_set_fd_handler* to a GSource.
GLib has to deal with the same issue with regard to calls to
g_source_attach() while it's iterating through it's list of GSources. It
uses the GMainContext lock protect that list, and actually doesn't disallow
use of functions like g_source_attach() within a dispatch function. In
g_main_context_dispatch(), to work around the potential deadlock issue, it
actually builds up a separate list of dispatch cb functions and callback data,
then drops the GMainContext lock before iterating through that list and
calling the dispatch cb functions for all the GSources that fired.
This new list it builds up is safe from concurrent modification since
only the main loop thread can access it.
AFAIK there's 3 ways to deal with this type of concurrency:
a) Move to a 1-to-1 mapping between GPollFDs and EventGSources: we can then
let GLib handle managing our list of GPollFDs for us. We may still need a
mutex for other members of EventsGSource, but that lock can be taken from
within our prepare/check/dispatch functions and held for the duration of
the calls without any strange deadlock issues.
The major downside here is potentially performance. Currently we do an
O(n) lookup for stuff like qemu_set_fd_handler, where n is the number of
IOHandlers. If we move to 1-to-1 fd-to-GSource mapping, it's O(m), where
m is all GSources attached to the GMainContext. I'm not sure what the
performance penalty would be, but it will get worse as the number of
GSources increases. Not sure if this penalty is applicable for slirp,
as it doesn't seem like we need to do any sort of per-socket/fd lookup,
since we have a direct pointer to the GPollFD (if you take the approach
I mentioned above where you pass a GPollFD* to event_source_add_pollfd())
b) Stick with this many-to-1 mapping between GPollFDs and EventGSources: we
can then introduce variants of events_source_{add,remove}_pollfd
that don't take the EventGSource mutex so you can call them inside the
dispatch function, which is nasty, because in the case of slirp you'll then
end up with similar variants for things like socreate(), or:
c) Stick with the many-to-1 mapping, but do what glib does and build a list
of dispatch callbacks, then drop the EventGSource lock before calling them.
(I know for EventGSource we currently have 1 cb for all the FDs, but the
requirements are the same, you're just pushing synchronization concerns
higher up the stack to
slirp_event_source_add_pollfd/slirp_prepare/slirp_handler, and will run
into the same recursive locking issue in slirp_handler as a result. I think
it's better to handle it all in EventGSource so non-slirp users don't need
to implement the same trick, but the approach should be applicable either
way)
One concern here is that we might remove an event via
sofree()->slirp_event_source_remove_pollfd() just after
EventGSource->dispatch() drops EventGSource->mutex, so it might still end up
dispatching cb for that pfd even though we've deleted it. I think we can
have EventGSource set a flag in this case indicating it's in the middle of
dispatch, so that event_source_{add,remove}_pfd can wait on a condition
variable like EventGSource->cond_event_dispatch_complete before completing.
I'll be looking at c) WRT to the qemu_set_fd_handler stuff. Perhaps we can
re-use the same GSourceFuncs here as well, but don't let that bottleneck you,
just wanted to bring it up for discussion.
> + if (fd >= 0) {
> + g_source_add_poll(&src->source, retfd);
> + }
> +
> + return retfd;
> +}
> +
> +void events_source_remove_pollfd(EventsGSource *src, GPollFD *pollfd)
> +{
> + g_source_remove_poll(&src->source, pollfd);
> + src->pollfds_list = g_list_remove(src->pollfds_list, pollfd);
> + g_slice_free(GPollFD, pollfd);
> +}
> +
> +static gboolean events_source_check(GSource *src)
> +{
> + EventsGSource *nsrc = (EventsGSource *)src;
> + GList *cur;
> + GPollFD *gfd;
> +
> + cur = nsrc->pollfds_list;
> + while (cur) {
> + gfd = cur->data;
> + if (gfd->fd >= 0 && (gfd->revents & gfd->events)) {
> + return true;
> + }
> + cur = g_list_next(cur);
> + }
> +
> + return false;
> +}
> +
> +static gboolean events_source_dispatch(GSource *src, GSourceFunc cb,
> + gpointer data)
> +{
> + gboolean ret = false;
> +
> + if (cb) {
> + ret = cb(data);
> + }
> + return ret;
> +}
> +
> +EventsGSource *events_source_new(GPrepare prepare, GSourceFunc dispatch_cb,
> + void *opaque)
> +{
> + EventsGSource *src;
> + GSourceFuncs *gfuncs = g_new0(GSourceFuncs, 1);
> + gfuncs->prepare = prepare;
> + gfuncs->check = events_source_check,
> + gfuncs->dispatch = events_source_dispatch,
> +
> + src = (EventsGSource *)g_source_new(gfuncs, sizeof(EventsGSource));
> + src->gfuncs = gfuncs;
> + src->pollfds_list = NULL;
> + src->opaque = opaque;
> + g_source_set_callback(&src->source, dispatch_cb, src, NULL);
> +
> + return src;
> +}
> +
> +void events_source_release(EventsGSource *src)
> +{
> + assert(!src->pollfds_list);
> + g_free(src->gfuncs);
> + g_source_destroy(&src->source);
> +}
> diff --git a/util/event_gsource.h b/util/event_gsource.h
> new file mode 100644
> index 0000000..8755952
> --- /dev/null
> +++ b/util/event_gsource.h
> @@ -0,0 +1,37 @@
> +/*
> + * Copyright (C) 2013 IBM
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; under version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef EVENT_GSOURCE_H
> +#define EVENT_GSOURCE_H
> +#include "qemu-common.h"
> +
> +typedef gboolean (*GPrepare)(GSource *source, gint *timeout_);
> +
> +/* multi fd drive GSource*/
> +typedef struct EventsGSource {
> + GSource source;
> + /* a group of GPollFD which dynamically join or leave the GSource */
> + GList *pollfds_list;
> + GSourceFuncs *gfuncs;
> + void *opaque;
> +} EventsGSource;
> +
> +EventsGSource *events_source_new(GPrepare prepare, GSourceFunc dispatch_cb,
> + void *opaque);
> +void events_source_release(EventsGSource *src);
> +GPollFD *events_source_add_pollfd(EventsGSource *src, int fd);
> +void events_source_remove_pollfd(EventsGSource *src, GPollFD *pollfd);
> +#endif
> --
> 1.8.1.4
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH v1 1/5] util: introduce gsource event abstraction
2013-08-08 21:03 ` Michael Roth
@ 2013-08-08 21:12 ` Michael Roth
2013-08-14 19:26 ` Michael Roth
1 sibling, 0 replies; 14+ messages in thread
From: Michael Roth @ 2013-08-08 21:12 UTC (permalink / raw)
To: Liu Ping Fan, qemu-devel; +Cc: Paolo Bonzini, Stefan Hajnoczi
Quoting Michael Roth (2013-08-08 16:03:30)
> Quoting Liu Ping Fan (2013-08-08 01:26:07)
> > Introduce struct EventsGSource. It will ease the usage of GSource
> > associated with a group of files, which are dynamically allocated
> > and release, ex, slirp.
> >
> > Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
> > ---
> > util/Makefile.objs | 1 +
> > util/event_gsource.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > util/event_gsource.h | 37 +++++++++++++++++++++
> > 3 files changed, 132 insertions(+)
> > create mode 100644 util/event_gsource.c
> > create mode 100644 util/event_gsource.h
> >
> > diff --git a/util/Makefile.objs b/util/Makefile.objs
> > index dc72ab0..eec55bd 100644
> > --- a/util/Makefile.objs
> > +++ b/util/Makefile.objs
> > @@ -11,3 +11,4 @@ util-obj-y += iov.o aes.o qemu-config.o qemu-sockets.o uri.o notify.o
> > util-obj-y += qemu-option.o qemu-progress.o
> > util-obj-y += hexdump.o
> > util-obj-y += crc32c.o
> > +util-obj-y += event_gsource.o
> > diff --git a/util/event_gsource.c b/util/event_gsource.c
> > new file mode 100644
> > index 0000000..4b9fa89
> > --- /dev/null
> > +++ b/util/event_gsource.c
> > @@ -0,0 +1,94 @@
> > +/*
> > + * Copyright (C) 2013 IBM
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; under version 2 of the License.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include "event_gsource.h"
> > +#include "qemu/bitops.h"
> > +
> > +GPollFD *events_source_add_pollfd(EventsGSource *src, int fd)
> > +{
> > + GPollFD *retfd;
> > +
> > + retfd = g_slice_alloc(sizeof(GPollFD));
> > + retfd->events = 0;
> > + retfd->fd = fd;
> > + src->pollfds_list = g_list_append(src->pollfds_list, retfd);
>
> I think moving to a GSource to simplify our mainloop implementation is
> worthwhile even if we still rely on the global mutex and don't initially
> support running those GSources outside the main iothread. But since being
> able to eventually offload NetClient backends to seperate events loops to
> support things like virtio-net dataplane is (I assume) still one of the
> eventual goals, we should consider how to deal with concurrent
> access to EventsGSource members.
>
> For instance, In the case of slirp your dispatch callback may modify
> src->pollfds_lists via
> slirp_handler->tcp_input->socreate->events_source_add_pollfd(), while
> another thread attempts to call socreate() via something like
> net_slirp_hostfwd_add from the monitor (that's being driven by a separate
> main loop).
>
> So events_source_add_pollfd() and the various prepare/check/dispatch
> functions should take a lock on pollfds_lists.
>
> Dispatch is tricky though, since dispatch() invoke callbacks that may in
> turn try to call events_source_add_pollfd(), as is the case above, in which
> case you can deadlock.
>
> I've been looking at the situation with regard to moving
> qemu_set_fd_handler* to a GSource.
>
> GLib has to deal with the same issue with regard to calls to
> g_source_attach() while it's iterating through it's list of GSources. It
> uses the GMainContext lock protect that list, and actually doesn't disallow
> use of functions like g_source_attach() within a dispatch function. In
> g_main_context_dispatch(), to work around the potential deadlock issue, it
> actually builds up a separate list of dispatch cb functions and callback data,
> then drops the GMainContext lock before iterating through that list and
> calling the dispatch cb functions for all the GSources that fired.
> This new list it builds up is safe from concurrent modification since
> only the main loop thread can access it.
>
> AFAIK there's 3 ways to deal with this type of concurrency:
>
> a) Move to a 1-to-1 mapping between GPollFDs and EventGSources: we can then
> let GLib handle managing our list of GPollFDs for us. We may still need a
> mutex for other members of EventsGSource, but that lock can be taken from
> within our prepare/check/dispatch functions and held for the duration of
> the calls without any strange deadlock issues.
I take that back, would still need to drop EventsGSource mutex, in
EventsGSource->dispatch prior calling the dispatch cb, but this is trivial
unless the dispatch cb or associated user_data can be modified, which isn't
the case with this interface.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH v1 1/5] util: introduce gsource event abstraction
2013-08-08 21:03 ` Michael Roth
2013-08-08 21:12 ` Michael Roth
@ 2013-08-14 19:26 ` Michael Roth
1 sibling, 0 replies; 14+ messages in thread
From: Michael Roth @ 2013-08-14 19:26 UTC (permalink / raw)
To: Liu Ping Fan, qemu-devel; +Cc: Paolo Bonzini, Stefan Hajnoczi
Quoting Michael Roth (2013-08-08 16:03:30)
> Quoting Liu Ping Fan (2013-08-08 01:26:07)
> > Introduce struct EventsGSource. It will ease the usage of GSource
> > associated with a group of files, which are dynamically allocated
> > and release, ex, slirp.
> >
> > Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
> > ---
> > util/Makefile.objs | 1 +
> > util/event_gsource.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > util/event_gsource.h | 37 +++++++++++++++++++++
> > 3 files changed, 132 insertions(+)
> > create mode 100644 util/event_gsource.c
> > create mode 100644 util/event_gsource.h
> >
> > diff --git a/util/Makefile.objs b/util/Makefile.objs
> > index dc72ab0..eec55bd 100644
> > --- a/util/Makefile.objs
> > +++ b/util/Makefile.objs
> > @@ -11,3 +11,4 @@ util-obj-y += iov.o aes.o qemu-config.o qemu-sockets.o uri.o notify.o
> > util-obj-y += qemu-option.o qemu-progress.o
> > util-obj-y += hexdump.o
> > util-obj-y += crc32c.o
> > +util-obj-y += event_gsource.o
> > diff --git a/util/event_gsource.c b/util/event_gsource.c
> > new file mode 100644
> > index 0000000..4b9fa89
> > --- /dev/null
> > +++ b/util/event_gsource.c
> > @@ -0,0 +1,94 @@
> > +/*
> > + * Copyright (C) 2013 IBM
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; under version 2 of the License.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include "event_gsource.h"
> > +#include "qemu/bitops.h"
> > +
> > +GPollFD *events_source_add_pollfd(EventsGSource *src, int fd)
> > +{
> > + GPollFD *retfd;
> > +
> > + retfd = g_slice_alloc(sizeof(GPollFD));
> > + retfd->events = 0;
> > + retfd->fd = fd;
> > + src->pollfds_list = g_list_append(src->pollfds_list, retfd);
>
> I think moving to a GSource to simplify our mainloop implementation is
> worthwhile even if we still rely on the global mutex and don't initially
> support running those GSources outside the main iothread. But since being
> able to eventually offload NetClient backends to seperate events loops to
> support things like virtio-net dataplane is (I assume) still one of the
> eventual goals, we should consider how to deal with concurrent
> access to EventsGSource members.
>
> For instance, In the case of slirp your dispatch callback may modify
> src->pollfds_lists via
> slirp_handler->tcp_input->socreate->events_source_add_pollfd(), while
> another thread attempts to call socreate() via something like
> net_slirp_hostfwd_add from the monitor (that's being driven by a separate
> main loop).
>
> So events_source_add_pollfd() and the various prepare/check/dispatch
> functions should take a lock on pollfds_lists.
>
> Dispatch is tricky though, since dispatch() invoke callbacks that may in
> turn try to call events_source_add_pollfd(), as is the case above, in which
> case you can deadlock.
>
> I've been looking at the situation with regard to moving
> qemu_set_fd_handler* to a GSource.
>
> GLib has to deal with the same issue with regard to calls to
> g_source_attach() while it's iterating through it's list of GSources. It
> uses the GMainContext lock protect that list, and actually doesn't disallow
> use of functions like g_source_attach() within a dispatch function. In
> g_main_context_dispatch(), to work around the potential deadlock issue, it
> actually builds up a separate list of dispatch cb functions and callback data,
> then drops the GMainContext lock before iterating through that list and
> calling the dispatch cb functions for all the GSources that fired.
> This new list it builds up is safe from concurrent modification since
> only the main loop thread can access it.
>
> AFAIK there's 3 ways to deal with this type of concurrency:
>
> a) Move to a 1-to-1 mapping between GPollFDs and EventGSources: we can then
> let GLib handle managing our list of GPollFDs for us. We may still need a
> mutex for other members of EventsGSource, but that lock can be taken from
> within our prepare/check/dispatch functions and held for the duration of
> the calls without any strange deadlock issues.
>
> The major downside here is potentially performance. Currently we do an
> O(n) lookup for stuff like qemu_set_fd_handler, where n is the number of
> IOHandlers. If we move to 1-to-1 fd-to-GSource mapping, it's O(m), where
> m is all GSources attached to the GMainContext. I'm not sure what the
> performance penalty would be, but it will get worse as the number of
> GSources increases. Not sure if this penalty is applicable for slirp,
> as it doesn't seem like we need to do any sort of per-socket/fd lookup,
> since we have a direct pointer to the GPollFD (if you take the approach
> I mentioned above where you pass a GPollFD* to event_source_add_pollfd())
>
> b) Stick with this many-to-1 mapping between GPollFDs and EventGSources: we
> can then introduce variants of events_source_{add,remove}_pollfd
> that don't take the EventGSource mutex so you can call them inside the
> dispatch function, which is nasty, because in the case of slirp you'll then
> end up with similar variants for things like socreate(), or:
>
> c) Stick with the many-to-1 mapping, but do what glib does and build a list
> of dispatch callbacks, then drop the EventGSource lock before calling them.
>
> (I know for EventGSource we currently have 1 cb for all the FDs, but the
> requirements are the same, you're just pushing synchronization concerns
> higher up the stack to
> slirp_event_source_add_pollfd/slirp_prepare/slirp_handler, and will run
> into the same recursive locking issue in slirp_handler as a result. I think
> it's better to handle it all in EventGSource so non-slirp users don't need
> to implement the same trick, but the approach should be applicable either
> way)
>
> One concern here is that we might remove an event via
> sofree()->slirp_event_source_remove_pollfd() just after
> EventGSource->dispatch() drops EventGSource->mutex, so it might still end up
> dispatching cb for that pfd even though we've deleted it. I think we can
> have EventGSource set a flag in this case indicating it's in the middle of
> dispatch, so that event_source_{add,remove}_pfd can wait on a condition
> variable like EventGSource->cond_event_dispatch_complete before completing.
>
> I'll be looking at c) WRT to the qemu_set_fd_handler stuff. Perhaps we can
> re-use the same GSourceFuncs here as well, but don't let that bottleneck you,
> just wanted to bring it up for discussion.
Here's the c) approach I was referring to:
https://github.com/mdroth/qemu/blob/9a749a2a1ae93ac1b7d6a1216edaf0ca96ff1edb/iohandler.c#L110
It was actually quite a bit more straightforward: we set a flag during dispatch
that tells registration/de-registration that they cannot modify the event list
until the dispatch_complete condition is issued by GSource's dispatch function
unless that thread owns the GMainContext (which we can easily check via
g_main_context_is_owner() due to the requirement that callers of
g_main_context_dispatch must call g_main_context_acquire)
As a result, we can drop the GSource's mutex prior to walking the list of event
callbacks, and don't even need to build up a second list. The special
consideration is use the Q*_FOREACH__SAFE macros while walking, since callbacks
might modify it while we do so.
Anthony wasn't too enthusiastic about it and after talking with him a bit I
decided to look into a lockless approach for fd-based events, but hopefully it
at least provides a reference for a possible approach to the issue if
lockless isn't a viable option for the GSource, or we don't care about
lock contention due to rapid de/re-registration of events/pfds/fds for a
particular GSource.
>
> > + if (fd >= 0) {
> > + g_source_add_poll(&src->source, retfd);
> > + }
> > +
> > + return retfd;
> > +}
> > +
> > +void events_source_remove_pollfd(EventsGSource *src, GPollFD *pollfd)
> > +{
> > + g_source_remove_poll(&src->source, pollfd);
> > + src->pollfds_list = g_list_remove(src->pollfds_list, pollfd);
> > + g_slice_free(GPollFD, pollfd);
> > +}
> > +
> > +static gboolean events_source_check(GSource *src)
> > +{
> > + EventsGSource *nsrc = (EventsGSource *)src;
> > + GList *cur;
> > + GPollFD *gfd;
> > +
> > + cur = nsrc->pollfds_list;
> > + while (cur) {
> > + gfd = cur->data;
> > + if (gfd->fd >= 0 && (gfd->revents & gfd->events)) {
> > + return true;
> > + }
> > + cur = g_list_next(cur);
> > + }
> > +
> > + return false;
> > +}
> > +
> > +static gboolean events_source_dispatch(GSource *src, GSourceFunc cb,
> > + gpointer data)
> > +{
> > + gboolean ret = false;
> > +
> > + if (cb) {
> > + ret = cb(data);
> > + }
> > + return ret;
> > +}
> > +
> > +EventsGSource *events_source_new(GPrepare prepare, GSourceFunc dispatch_cb,
> > + void *opaque)
> > +{
> > + EventsGSource *src;
> > + GSourceFuncs *gfuncs = g_new0(GSourceFuncs, 1);
> > + gfuncs->prepare = prepare;
> > + gfuncs->check = events_source_check,
> > + gfuncs->dispatch = events_source_dispatch,
> > +
> > + src = (EventsGSource *)g_source_new(gfuncs, sizeof(EventsGSource));
> > + src->gfuncs = gfuncs;
> > + src->pollfds_list = NULL;
> > + src->opaque = opaque;
> > + g_source_set_callback(&src->source, dispatch_cb, src, NULL);
> > +
> > + return src;
> > +}
> > +
> > +void events_source_release(EventsGSource *src)
> > +{
> > + assert(!src->pollfds_list);
> > + g_free(src->gfuncs);
> > + g_source_destroy(&src->source);
> > +}
> > diff --git a/util/event_gsource.h b/util/event_gsource.h
> > new file mode 100644
> > index 0000000..8755952
> > --- /dev/null
> > +++ b/util/event_gsource.h
> > @@ -0,0 +1,37 @@
> > +/*
> > + * Copyright (C) 2013 IBM
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; under version 2 of the License.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#ifndef EVENT_GSOURCE_H
> > +#define EVENT_GSOURCE_H
> > +#include "qemu-common.h"
> > +
> > +typedef gboolean (*GPrepare)(GSource *source, gint *timeout_);
> > +
> > +/* multi fd drive GSource*/
> > +typedef struct EventsGSource {
> > + GSource source;
> > + /* a group of GPollFD which dynamically join or leave the GSource */
> > + GList *pollfds_list;
> > + GSourceFuncs *gfuncs;
> > + void *opaque;
> > +} EventsGSource;
> > +
> > +EventsGSource *events_source_new(GPrepare prepare, GSourceFunc dispatch_cb,
> > + void *opaque);
> > +void events_source_release(EventsGSource *src);
> > +GPollFD *events_source_add_pollfd(EventsGSource *src, int fd);
> > +void events_source_remove_pollfd(EventsGSource *src, GPollFD *pollfd);
> > +#endif
> > --
> > 1.8.1.4
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH v1 2/5] slirp: make timeout local
2013-08-08 6:26 [Qemu-devel] [PATCH v1 0/5] make slirp subsystem self-contained Liu Ping Fan
2013-08-08 6:26 ` [Qemu-devel] [PATCH v1 1/5] util: introduce gsource event abstraction Liu Ping Fan
@ 2013-08-08 6:26 ` Liu Ping Fan
2013-08-09 8:06 ` Paolo Bonzini
2013-08-08 6:26 ` [Qemu-devel] [PATCH v1 3/5] slirp: make slirp event dispatch based on slirp instance Liu Ping Fan
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Liu Ping Fan @ 2013-08-08 6:26 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, mdroth, Stefan Hajnoczi
Each slirp has its own time to caculate timeout.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
slirp/slirp.c | 22 ++++++++++------------
slirp/slirp.h | 3 +++
2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 80b28ea..55654d5 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -40,8 +40,6 @@ static const uint8_t special_ethaddr[ETH_ALEN] = {
static const uint8_t zero_ethaddr[ETH_ALEN] = { 0, 0, 0, 0, 0, 0 };
u_int curtime;
-static u_int time_fasttimo, last_slowtimo;
-static int do_slowtimo;
static QTAILQ_HEAD(slirp_instances, Slirp) slirp_instances =
QTAILQ_HEAD_INITIALIZER(slirp_instances);
@@ -278,14 +276,13 @@ void slirp_pollfds_fill(GArray *pollfds)
/*
* First, TCP sockets
*/
- do_slowtimo = 0;
QTAILQ_FOREACH(slirp, &slirp_instances, entry) {
/*
* *_slowtimo needs calling if there are IP fragments
* in the fragment queue, or there are TCP connections active
*/
- do_slowtimo |= ((slirp->tcb.so_next != &slirp->tcb) ||
+ slirp->do_slowtimo = ((slirp->tcb.so_next != &slirp->tcb) ||
(&slirp->ipq.ip_link != slirp->ipq.ip_link.next));
for (so = slirp->tcb.so_next; so != &slirp->tcb;
@@ -299,8 +296,9 @@ void slirp_pollfds_fill(GArray *pollfds)
/*
* See if we need a tcp_fasttimo
*/
- if (time_fasttimo == 0 && so->so_tcpcb->t_flags & TF_DELACK) {
- time_fasttimo = curtime; /* Flag when we want a fasttimo */
+ if (slirp->time_fasttimo == 0 &&
+ so->so_tcpcb->t_flags & TF_DELACK) {
+ slirp->time_fasttimo = curtime; /* Flag when want a fasttimo */
}
/*
@@ -381,7 +379,7 @@ void slirp_pollfds_fill(GArray *pollfds)
udp_detach(so);
continue;
} else {
- do_slowtimo = 1; /* Let socket expire */
+ slirp->do_slowtimo = 1; /* Let socket expire */
}
}
@@ -422,7 +420,7 @@ void slirp_pollfds_fill(GArray *pollfds)
icmp_detach(so);
continue;
} else {
- do_slowtimo = 1; /* Let socket expire */
+ slirp->do_slowtimo = 1; /* Let socket expire */
}
}
@@ -454,14 +452,14 @@ void slirp_pollfds_poll(GArray *pollfds, int select_error)
/*
* See if anything has timed out
*/
- if (time_fasttimo && ((curtime - time_fasttimo) >= 2)) {
+ if (slirp->time_fasttimo && ((curtime - slirp->time_fasttimo) >= 2)) {
tcp_fasttimo(slirp);
- time_fasttimo = 0;
+ slirp->time_fasttimo = 0;
}
- if (do_slowtimo && ((curtime - last_slowtimo) >= 499)) {
+ if (slirp->do_slowtimo && ((curtime - slirp->last_slowtimo) >= 499)) {
ip_slowtimo(slirp);
tcp_slowtimo(slirp);
- last_slowtimo = curtime;
+ slirp->last_slowtimo = curtime;
}
/*
diff --git a/slirp/slirp.h b/slirp/slirp.h
index fe0e65d..008360e 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -203,6 +203,9 @@ bool arp_table_search(Slirp *slirp, uint32_t ip_addr,
struct Slirp {
QTAILQ_ENTRY(Slirp) entry;
+ u_int time_fasttimo;
+ u_int last_slowtimo;
+ int do_slowtimo;
/* virtual network configuration */
struct in_addr vnetwork_addr;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH v1 2/5] slirp: make timeout local
2013-08-08 6:26 ` [Qemu-devel] [PATCH v1 2/5] slirp: make timeout local Liu Ping Fan
@ 2013-08-09 8:06 ` Paolo Bonzini
2013-08-09 8:48 ` liu ping fan
0 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2013-08-09 8:06 UTC (permalink / raw)
To: Liu Ping Fan; +Cc: qemu-devel, Stefan Hajnoczi, mdroth
Il 08/08/2013 08:26, Liu Ping Fan ha scritto:
> Each slirp has its own time to caculate timeout.
>
> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
> ---
> slirp/slirp.c | 22 ++++++++++------------
> slirp/slirp.h | 3 +++
> 2 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index 80b28ea..55654d5 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -40,8 +40,6 @@ static const uint8_t special_ethaddr[ETH_ALEN] = {
> static const uint8_t zero_ethaddr[ETH_ALEN] = { 0, 0, 0, 0, 0, 0 };
>
> u_int curtime;
> -static u_int time_fasttimo, last_slowtimo;
> -static int do_slowtimo;
>
> static QTAILQ_HEAD(slirp_instances, Slirp) slirp_instances =
> QTAILQ_HEAD_INITIALIZER(slirp_instances);
> @@ -278,14 +276,13 @@ void slirp_pollfds_fill(GArray *pollfds)
> /*
> * First, TCP sockets
> */
> - do_slowtimo = 0;
>
> QTAILQ_FOREACH(slirp, &slirp_instances, entry) {
> /*
> * *_slowtimo needs calling if there are IP fragments
> * in the fragment queue, or there are TCP connections active
> */
> - do_slowtimo |= ((slirp->tcb.so_next != &slirp->tcb) ||
> + slirp->do_slowtimo = ((slirp->tcb.so_next != &slirp->tcb) ||
> (&slirp->ipq.ip_link != slirp->ipq.ip_link.next));
>
> for (so = slirp->tcb.so_next; so != &slirp->tcb;
> @@ -299,8 +296,9 @@ void slirp_pollfds_fill(GArray *pollfds)
> /*
> * See if we need a tcp_fasttimo
> */
> - if (time_fasttimo == 0 && so->so_tcpcb->t_flags & TF_DELACK) {
> - time_fasttimo = curtime; /* Flag when we want a fasttimo */
> + if (slirp->time_fasttimo == 0 &&
> + so->so_tcpcb->t_flags & TF_DELACK) {
> + slirp->time_fasttimo = curtime; /* Flag when want a fasttimo */
> }
>
> /*
> @@ -381,7 +379,7 @@ void slirp_pollfds_fill(GArray *pollfds)
> udp_detach(so);
> continue;
> } else {
> - do_slowtimo = 1; /* Let socket expire */
> + slirp->do_slowtimo = 1; /* Let socket expire */
> }
> }
>
> @@ -422,7 +420,7 @@ void slirp_pollfds_fill(GArray *pollfds)
> icmp_detach(so);
> continue;
> } else {
> - do_slowtimo = 1; /* Let socket expire */
> + slirp->do_slowtimo = 1; /* Let socket expire */
> }
> }
>
> @@ -454,14 +452,14 @@ void slirp_pollfds_poll(GArray *pollfds, int select_error)
> /*
> * See if anything has timed out
> */
> - if (time_fasttimo && ((curtime - time_fasttimo) >= 2)) {
> + if (slirp->time_fasttimo && ((curtime - slirp->time_fasttimo) >= 2)) {
> tcp_fasttimo(slirp);
> - time_fasttimo = 0;
> + slirp->time_fasttimo = 0;
> }
> - if (do_slowtimo && ((curtime - last_slowtimo) >= 499)) {
> + if (slirp->do_slowtimo && ((curtime - slirp->last_slowtimo) >= 499)) {
> ip_slowtimo(slirp);
> tcp_slowtimo(slirp);
> - last_slowtimo = curtime;
> + slirp->last_slowtimo = curtime;
> }
>
> /*
> diff --git a/slirp/slirp.h b/slirp/slirp.h
> index fe0e65d..008360e 100644
> --- a/slirp/slirp.h
> +++ b/slirp/slirp.h
> @@ -203,6 +203,9 @@ bool arp_table_search(Slirp *slirp, uint32_t ip_addr,
>
> struct Slirp {
> QTAILQ_ENTRY(Slirp) entry;
> + u_int time_fasttimo;
> + u_int last_slowtimo;
> + int do_slowtimo;
>
> /* virtual network configuration */
> struct in_addr vnetwork_addr;
>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
In addition, you could modify slirp_update_timeout to run after
slirp_pollfds_fill, and actually look at the timeout fields to set the
right timeout (2 if time_fasttimo is nonzero, 499 if do_slowtimo is true).
Paolo
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH v1 2/5] slirp: make timeout local
2013-08-09 8:06 ` Paolo Bonzini
@ 2013-08-09 8:48 ` liu ping fan
2013-08-09 9:32 ` Paolo Bonzini
0 siblings, 1 reply; 14+ messages in thread
From: liu ping fan @ 2013-08-09 8:48 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Stefan Hajnoczi, mdroth
On Fri, Aug 9, 2013 at 4:06 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 08/08/2013 08:26, Liu Ping Fan ha scritto:
>> Each slirp has its own time to caculate timeout.
>>
>> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
>> ---
>> slirp/slirp.c | 22 ++++++++++------------
>> slirp/slirp.h | 3 +++
>> 2 files changed, 13 insertions(+), 12 deletions(-)
>>
>> diff --git a/slirp/slirp.c b/slirp/slirp.c
>> index 80b28ea..55654d5 100644
>> --- a/slirp/slirp.c
>> +++ b/slirp/slirp.c
>> @@ -40,8 +40,6 @@ static const uint8_t special_ethaddr[ETH_ALEN] = {
>> static const uint8_t zero_ethaddr[ETH_ALEN] = { 0, 0, 0, 0, 0, 0 };
>>
>> u_int curtime;
>> -static u_int time_fasttimo, last_slowtimo;
>> -static int do_slowtimo;
>>
>> static QTAILQ_HEAD(slirp_instances, Slirp) slirp_instances =
>> QTAILQ_HEAD_INITIALIZER(slirp_instances);
>> @@ -278,14 +276,13 @@ void slirp_pollfds_fill(GArray *pollfds)
>> /*
>> * First, TCP sockets
>> */
>> - do_slowtimo = 0;
>>
>> QTAILQ_FOREACH(slirp, &slirp_instances, entry) {
>> /*
>> * *_slowtimo needs calling if there are IP fragments
>> * in the fragment queue, or there are TCP connections active
>> */
>> - do_slowtimo |= ((slirp->tcb.so_next != &slirp->tcb) ||
>> + slirp->do_slowtimo = ((slirp->tcb.so_next != &slirp->tcb) ||
>> (&slirp->ipq.ip_link != slirp->ipq.ip_link.next));
>>
>> for (so = slirp->tcb.so_next; so != &slirp->tcb;
>> @@ -299,8 +296,9 @@ void slirp_pollfds_fill(GArray *pollfds)
>> /*
>> * See if we need a tcp_fasttimo
>> */
>> - if (time_fasttimo == 0 && so->so_tcpcb->t_flags & TF_DELACK) {
>> - time_fasttimo = curtime; /* Flag when we want a fasttimo */
>> + if (slirp->time_fasttimo == 0 &&
>> + so->so_tcpcb->t_flags & TF_DELACK) {
>> + slirp->time_fasttimo = curtime; /* Flag when want a fasttimo */
>> }
>>
>> /*
>> @@ -381,7 +379,7 @@ void slirp_pollfds_fill(GArray *pollfds)
>> udp_detach(so);
>> continue;
>> } else {
>> - do_slowtimo = 1; /* Let socket expire */
>> + slirp->do_slowtimo = 1; /* Let socket expire */
>> }
>> }
>>
>> @@ -422,7 +420,7 @@ void slirp_pollfds_fill(GArray *pollfds)
>> icmp_detach(so);
>> continue;
>> } else {
>> - do_slowtimo = 1; /* Let socket expire */
>> + slirp->do_slowtimo = 1; /* Let socket expire */
>> }
>> }
>>
>> @@ -454,14 +452,14 @@ void slirp_pollfds_poll(GArray *pollfds, int select_error)
>> /*
>> * See if anything has timed out
>> */
>> - if (time_fasttimo && ((curtime - time_fasttimo) >= 2)) {
>> + if (slirp->time_fasttimo && ((curtime - slirp->time_fasttimo) >= 2)) {
>> tcp_fasttimo(slirp);
>> - time_fasttimo = 0;
>> + slirp->time_fasttimo = 0;
>> }
>> - if (do_slowtimo && ((curtime - last_slowtimo) >= 499)) {
>> + if (slirp->do_slowtimo && ((curtime - slirp->last_slowtimo) >= 499)) {
>> ip_slowtimo(slirp);
>> tcp_slowtimo(slirp);
>> - last_slowtimo = curtime;
>> + slirp->last_slowtimo = curtime;
>> }
>>
>> /*
>> diff --git a/slirp/slirp.h b/slirp/slirp.h
>> index fe0e65d..008360e 100644
>> --- a/slirp/slirp.h
>> +++ b/slirp/slirp.h
>> @@ -203,6 +203,9 @@ bool arp_table_search(Slirp *slirp, uint32_t ip_addr,
>>
>> struct Slirp {
>> QTAILQ_ENTRY(Slirp) entry;
>> + u_int time_fasttimo;
>> + u_int last_slowtimo;
>> + int do_slowtimo;
>>
>> /* virtual network configuration */
>> struct in_addr vnetwork_addr;
>>
>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>
> In addition, you could modify slirp_update_timeout to run after
> slirp_pollfds_fill, and actually look at the timeout fields to set the
> right timeout (2 if time_fasttimo is nonzero, 499 if do_slowtimo is true).
>
After patch 3,4, the timeout will be an param for GSource's prepare.
So I think I will set the right timeout value in patch4?
Thanks,
Pingfan
> Paolo
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PATCH v1 2/5] slirp: make timeout local
2013-08-09 8:48 ` liu ping fan
@ 2013-08-09 9:32 ` Paolo Bonzini
0 siblings, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2013-08-09 9:32 UTC (permalink / raw)
To: liu ping fan; +Cc: qemu-devel, Stefan Hajnoczi, mdroth
Il 09/08/2013 10:48, liu ping fan ha scritto:
> After patch 3,4, the timeout will be an param for GSource's prepare.
> So I think I will set the right timeout value in patch4?
No, please do it before (in fact you can extract this patch and the
timeout change to a seprate series). Then patch 4 can simply move the
timeout logic to the GSource's prepare function.
Paolo
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH v1 3/5] slirp: make slirp event dispatch based on slirp instance
2013-08-08 6:26 [Qemu-devel] [PATCH v1 0/5] make slirp subsystem self-contained Liu Ping Fan
2013-08-08 6:26 ` [Qemu-devel] [PATCH v1 1/5] util: introduce gsource event abstraction Liu Ping Fan
2013-08-08 6:26 ` [Qemu-devel] [PATCH v1 2/5] slirp: make timeout local Liu Ping Fan
@ 2013-08-08 6:26 ` Liu Ping Fan
2013-08-08 6:26 ` [Qemu-devel] [PATCH v1 4/5] slirp: decouple timeout for gpoll Liu Ping Fan
2013-08-08 6:26 ` [Qemu-devel] [PATCH v1 5/5] slirp: fold curtime into slirp instance Liu Ping Fan
4 siblings, 0 replies; 14+ messages in thread
From: Liu Ping Fan @ 2013-08-08 6:26 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, mdroth, Stefan Hajnoczi
Executing slirp_pollfds_fill/_poll actions based on each slirp instance,
so we can decouple the slirp_pollfds_fill/_poll from main-loop.
Inside each slirp instance, it contains a GSource. The GSource holds a
group of GPollFD, which is the foundation of slirp-sockets. So the state-change
of GPollFD will drive the state of slirp-sockets.
Note, the logic in slirp_pollfds_fill/_poll is not changed, but due to drop of
the functions, rearrange the code to obey the coding style. For other minor
changes, they accord to the nearby style.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
main-loop.c | 4 -
net/slirp.c | 24 +++
slirp/libslirp.h | 7 +-
slirp/slirp.c | 567 +++++++++++++++++++++++++------------------------------
slirp/socket.c | 2 +
slirp/socket.h | 1 +
stubs/slirp.c | 8 -
7 files changed, 291 insertions(+), 322 deletions(-)
diff --git a/main-loop.c b/main-loop.c
index a44fff6..c0da803 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -459,14 +459,10 @@ int main_loop_wait(int nonblocking)
/* XXX: separate device handlers from system ones */
#ifdef CONFIG_SLIRP
slirp_update_timeout(&timeout);
- slirp_pollfds_fill(gpollfds);
#endif
qemu_iohandler_fill(gpollfds);
ret = os_host_main_loop_wait(timeout);
qemu_iohandler_poll(gpollfds, ret);
-#ifdef CONFIG_SLIRP
- slirp_pollfds_poll(gpollfds, (ret < 0));
-#endif
qemu_run_all_timers();
diff --git a/net/slirp.c b/net/slirp.c
index 124e953..8592a3c 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -36,6 +36,7 @@
#include "qemu/sockets.h"
#include "slirp/libslirp.h"
#include "sysemu/char.h"
+#include "util/event_gsource.h"
static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
{
@@ -76,6 +77,7 @@ typedef struct SlirpState {
#ifndef _WIN32
char smb_dir[128];
#endif
+ EventsGSource *slirp_src;
} SlirpState;
static struct slirp_config_str *slirp_configs;
@@ -120,6 +122,7 @@ static void net_slirp_cleanup(NetClientState *nc)
SlirpState *s = DO_UPCAST(SlirpState, nc, nc);
slirp_cleanup(s->slirp);
+ events_source_release(s->slirp_src);
slirp_smb_cleanup(s);
QTAILQ_REMOVE(&slirp_stacks, s, entry);
}
@@ -131,6 +134,24 @@ static NetClientInfo net_slirp_info = {
.cleanup = net_slirp_cleanup,
};
+GPollFD *slirp_gsource_add_pollfd(void *opaque, int fd)
+{
+ GPollFD *retfd;
+ SlirpState *s = opaque;
+ EventsGSource *src = s->slirp_src;
+ retfd = events_source_add_pollfd(src, fd);
+
+ return retfd;
+}
+
+void slirp_gsource_remove_pollfd(void *opaque, GPollFD *pollfd)
+{
+ SlirpState *s = opaque;
+ EventsGSource *src = s->slirp_src;
+
+ events_source_remove_pollfd(src, pollfd);
+}
+
static int net_slirp_init(NetClientState *peer, const char *model,
const char *name, int restricted,
const char *vnetwork, const char *vhost,
@@ -244,6 +265,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
s->slirp = slirp_init(restricted, net, mask, host, vhostname,
tftp_export, bootfile, dhcp, dns, dnssearch, s);
+ s->slirp_src = events_source_new(slirp_prepare, slirp_handler, s->slirp);
+
QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
for (config = slirp_configs; config; config = config->next) {
@@ -266,6 +289,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
goto error;
}
#endif
+ g_source_attach(&s->slirp_src->source, NULL);
return 0;
diff --git a/slirp/libslirp.h b/slirp/libslirp.h
index ceabff8..2dc131f 100644
--- a/slirp/libslirp.h
+++ b/slirp/libslirp.h
@@ -17,11 +17,10 @@ Slirp *slirp_init(int restricted, struct in_addr vnetwork,
void slirp_cleanup(Slirp *slirp);
void slirp_update_timeout(uint32_t *timeout);
-void slirp_pollfds_fill(GArray *pollfds);
-
-void slirp_pollfds_poll(GArray *pollfds, int select_error);
void slirp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len);
+gboolean slirp_prepare(GSource *source, gint *time);
+gboolean slirp_handler(gpointer data);
/* you must provide the following functions: */
void slirp_output(void *opaque, const uint8_t *pkt, int pkt_len);
@@ -40,5 +39,7 @@ void slirp_socket_recv(Slirp *slirp, struct in_addr guest_addr,
int guest_port, const uint8_t *buf, int size);
size_t slirp_socket_can_recv(Slirp *slirp, struct in_addr guest_addr,
int guest_port);
+GPollFD *slirp_gsource_add_pollfd(void *opaque, int fd);
+void slirp_gsource_remove_pollfd(void *opaque, GPollFD *pollfd);
#endif
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 55654d5..37244be 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -26,6 +26,7 @@
#include "sysemu/char.h"
#include "slirp.h"
#include "hw/hw.h"
+#include "util/event_gsource.h"
/* host loopback address */
struct in_addr loopback_addr;
@@ -262,386 +263,338 @@ void slirp_update_timeout(uint32_t *timeout)
if (!QTAILQ_EMPTY(&slirp_instances)) {
*timeout = MIN(1000, *timeout);
}
+ curtime = qemu_get_clock_ms(rt_clock);
}
-void slirp_pollfds_fill(GArray *pollfds)
+gboolean slirp_prepare(GSource *source, gint *time)
{
- Slirp *slirp;
+ EventsGSource *slirp_src = (EventsGSource *)source;
+ Slirp *slirp = slirp_src->opaque;
struct socket *so, *so_next;
-
- if (QTAILQ_EMPTY(&slirp_instances)) {
- return;
- }
+ int events = 0;
/*
- * First, TCP sockets
+ * *_slowtimo needs calling if there are IP fragments
+ * in the fragment queue, or there are TCP connections active
*/
+ slirp->do_slowtimo = ((slirp->tcb.so_next != &slirp->tcb) ||
+ (&slirp->ipq.ip_link != slirp->ipq.ip_link.next));
+
+ for (so = slirp->tcb.so_next; so != &slirp->tcb;
+ so = so_next) {
- QTAILQ_FOREACH(slirp, &slirp_instances, entry) {
+ so_next = so->so_next;
+ if (so->pollfd->fd == -1 && so->s != -1) {
+ so->pollfd->fd = so->s;
+ g_source_add_poll(source, so->pollfd);
+ }
/*
- * *_slowtimo needs calling if there are IP fragments
- * in the fragment queue, or there are TCP connections active
+ * See if we need a tcp_fasttimo
*/
- slirp->do_slowtimo = ((slirp->tcb.so_next != &slirp->tcb) ||
- (&slirp->ipq.ip_link != slirp->ipq.ip_link.next));
-
- for (so = slirp->tcb.so_next; so != &slirp->tcb;
- so = so_next) {
- int events = 0;
-
- so_next = so->so_next;
-
- so->pollfds_idx = -1;
-
- /*
- * See if we need a tcp_fasttimo
- */
- if (slirp->time_fasttimo == 0 &&
- so->so_tcpcb->t_flags & TF_DELACK) {
- slirp->time_fasttimo = curtime; /* Flag when want a fasttimo */
- }
-
- /*
- * NOFDREF can include still connecting to local-host,
- * newly socreated() sockets etc. Don't want to select these.
- */
- if (so->so_state & SS_NOFDREF || so->s == -1) {
- continue;
- }
-
- /*
- * Set for reading sockets which are accepting
- */
- if (so->so_state & SS_FACCEPTCONN) {
- GPollFD pfd = {
- .fd = so->s,
- .events = G_IO_IN | G_IO_HUP | G_IO_ERR,
- };
- so->pollfds_idx = pollfds->len;
- g_array_append_val(pollfds, pfd);
- continue;
- }
+ if (slirp->time_fasttimo == 0 &&
+ so->so_tcpcb->t_flags & TF_DELACK) {
+ slirp->time_fasttimo = curtime; /* Flag when want a fasttimo */
+ }
- /*
- * Set for writing sockets which are connecting
- */
- if (so->so_state & SS_ISFCONNECTING) {
- GPollFD pfd = {
- .fd = so->s,
- .events = G_IO_OUT | G_IO_ERR,
- };
- so->pollfds_idx = pollfds->len;
- g_array_append_val(pollfds, pfd);
- continue;
- }
+ /*
+ * NOFDREF can include still connecting to local-host,
+ * newly socreated() sockets etc. Don't want to select these.
+ */
+ if (so->so_state & SS_NOFDREF || so->s == -1) {
+ continue;
+ }
- /*
- * Set for writing if we are connected, can send more, and
- * we have something to send
- */
- if (CONN_CANFSEND(so) && so->so_rcv.sb_cc) {
- events |= G_IO_OUT | G_IO_ERR;
- }
+ /*
+ * Set for reading sockets which are accepting
+ */
+ if (so->so_state & SS_FACCEPTCONN) {
+ so->pollfd->events = G_IO_IN | G_IO_HUP | G_IO_ERR;
+ continue;
+ }
- /*
- * Set for reading (and urgent data) if we are connected, can
- * receive more, and we have room for it XXX /2 ?
- */
- if (CONN_CANFRCV(so) &&
- (so->so_snd.sb_cc < (so->so_snd.sb_datalen/2))) {
- events |= G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_PRI;
- }
+ /*
+ * Set for writing sockets which are connecting
+ */
+ if (so->so_state & SS_ISFCONNECTING) {
+ so->pollfd->events = G_IO_OUT | G_IO_ERR;
+ continue;
+ }
- if (events) {
- GPollFD pfd = {
- .fd = so->s,
- .events = events,
- };
- so->pollfds_idx = pollfds->len;
- g_array_append_val(pollfds, pfd);
- }
+ /*
+ * Set for writing if we are connected, can send more, and
+ * we have something to send
+ */
+ if (CONN_CANFSEND(so) && so->so_rcv.sb_cc) {
+ events |= G_IO_OUT | G_IO_ERR;
}
/*
- * UDP sockets
+ * Set for reading (and urgent data) if we are connected, can
+ * receive more, and we have room for it XXX /2 ?
*/
- for (so = slirp->udb.so_next; so != &slirp->udb;
- so = so_next) {
- so_next = so->so_next;
+ if (CONN_CANFRCV(so) &&
+ (so->so_snd.sb_cc < (so->so_snd.sb_datalen/2))) {
+ events |= G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_PRI;
+ }
- so->pollfds_idx = -1;
+ if (events) {
+ so->pollfd->events = events;
+ }
+ }
- /*
- * See if it's timed out
- */
- if (so->so_expire) {
- if (so->so_expire <= curtime) {
- udp_detach(so);
- continue;
- } else {
- slirp->do_slowtimo = 1; /* Let socket expire */
- }
- }
+ /*
+ * UDP sockets
+ */
+ for (so = slirp->udb.so_next; so != &slirp->udb;
+ so = so_next) {
+ so_next = so->so_next;
- /*
- * When UDP packets are received from over the
- * link, they're sendto()'d straight away, so
- * no need for setting for writing
- * Limit the number of packets queued by this session
- * to 4. Note that even though we try and limit this
- * to 4 packets, the session could have more queued
- * if the packets needed to be fragmented
- * (XXX <= 4 ?)
- */
- if ((so->so_state & SS_ISFCONNECTED) && so->so_queued <= 4) {
- GPollFD pfd = {
- .fd = so->s,
- .events = G_IO_IN | G_IO_HUP | G_IO_ERR,
- };
- so->pollfds_idx = pollfds->len;
- g_array_append_val(pollfds, pfd);
+ /*
+ * See if it's timed out
+ */
+ if (so->so_expire) {
+ if (so->so_expire <= curtime) {
+ udp_detach(so);
+ continue;
+ } else {
+ slirp->do_slowtimo = 1; /* Let socket expire */
}
}
/*
- * ICMP sockets
+ * When UDP packets are received from over the
+ * link, they're sendto()'d straight away, so
+ * no need for setting for writing
+ * Limit the number of packets queued by this session
+ * to 4. Note that even though we try and limit this
+ * to 4 packets, the session could have more queued
+ * if the packets needed to be fragmented
+ * (XXX <= 4 ?)
*/
- for (so = slirp->icmp.so_next; so != &slirp->icmp;
- so = so_next) {
- so_next = so->so_next;
+ if ((so->so_state & SS_ISFCONNECTED) && so->so_queued <= 4) {
+ so->pollfd->events = G_IO_IN | G_IO_HUP | G_IO_ERR;
+ }
+ }
- so->pollfds_idx = -1;
+ /*
+ * ICMP sockets
+ */
+ for (so = slirp->icmp.so_next; so != &slirp->icmp;
+ so = so_next) {
+ so_next = so->so_next;
- /*
- * See if it's timed out
- */
- if (so->so_expire) {
- if (so->so_expire <= curtime) {
- icmp_detach(so);
- continue;
- } else {
- slirp->do_slowtimo = 1; /* Let socket expire */
- }
+ /*
+ * See if it's timed out
+ */
+ if (so->so_expire) {
+ if (so->so_expire <= curtime) {
+ icmp_detach(so);
+ continue;
+ } else {
+ slirp->do_slowtimo = 1; /* Let socket expire */
}
+ }
- if (so->so_state & SS_ISFCONNECTED) {
- GPollFD pfd = {
- .fd = so->s,
- .events = G_IO_IN | G_IO_HUP | G_IO_ERR,
- };
- so->pollfds_idx = pollfds->len;
- g_array_append_val(pollfds, pfd);
- }
+ if (so->so_state & SS_ISFCONNECTED) {
+ so->pollfd->events = G_IO_IN | G_IO_HUP | G_IO_ERR;
}
}
+
+ return false;
}
-void slirp_pollfds_poll(GArray *pollfds, int select_error)
+gboolean slirp_handler(gpointer data)
{
- Slirp *slirp;
+ EventsGSource *src = data;
+ Slirp *slirp = src->opaque;
struct socket *so, *so_next;
int ret;
- if (QTAILQ_EMPTY(&slirp_instances)) {
- return;
+ /*
+ * See if anything has timed out
+ */
+ if (slirp->time_fasttimo && ((curtime - slirp->time_fasttimo) >= 2)) {
+ tcp_fasttimo(slirp);
+ slirp->time_fasttimo = 0;
+ }
+ if (slirp->do_slowtimo && ((curtime - slirp->last_slowtimo) >= 499)) {
+ ip_slowtimo(slirp);
+ tcp_slowtimo(slirp);
+ slirp->last_slowtimo = curtime;
}
- curtime = qemu_get_clock_ms(rt_clock);
+ /*
+ * Check TCP sockets
+ */
+ for (so = slirp->tcb.so_next; so != &slirp->tcb;
+ so = so_next) {
+ int revents;
- QTAILQ_FOREACH(slirp, &slirp_instances, entry) {
- /*
- * See if anything has timed out
- */
- if (slirp->time_fasttimo && ((curtime - slirp->time_fasttimo) >= 2)) {
- tcp_fasttimo(slirp);
- slirp->time_fasttimo = 0;
+ so_next = so->so_next;
+
+ revents = 0;
+ if (so->pollfd) {
+ revents = so->pollfd->revents;
}
- if (slirp->do_slowtimo && ((curtime - slirp->last_slowtimo) >= 499)) {
- ip_slowtimo(slirp);
- tcp_slowtimo(slirp);
- slirp->last_slowtimo = curtime;
+ if (so->so_state & SS_NOFDREF || so->s == -1) {
+ continue;
}
/*
- * Check sockets
+ * Check for URG data
+ * This will soread as well, so no need to
+ * test for G_IO_IN below if this succeeds
*/
- if (!select_error) {
+ if (revents & G_IO_PRI) {
+ sorecvoob(so);
+ }
+ /*
+ * Check sockets for reading
+ */
+ else if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) {
/*
- * Check TCP sockets
+ * Check for incoming connections
*/
- for (so = slirp->tcb.so_next; so != &slirp->tcb;
- so = so_next) {
- int revents;
-
- so_next = so->so_next;
-
- revents = 0;
- if (so->pollfds_idx != -1) {
- revents = g_array_index(pollfds, GPollFD,
- so->pollfds_idx).revents;
- }
+ if (so->so_state & SS_FACCEPTCONN) {
+ tcp_connect(so);
+ continue;
+ } /* else */
+ ret = soread(so);
- if (so->so_state & SS_NOFDREF || so->s == -1) {
- continue;
- }
+ /* Output it if we read something */
+ if (ret > 0) {
+ tcp_output(sototcpcb(so));
+ }
+ }
- /*
- * Check for URG data
- * This will soread as well, so no need to
- * test for G_IO_IN below if this succeeds
- */
- if (revents & G_IO_PRI) {
- sorecvoob(so);
- }
- /*
- * Check sockets for reading
- */
- else if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) {
- /*
- * Check for incoming connections
- */
- if (so->so_state & SS_FACCEPTCONN) {
- tcp_connect(so);
+ /*
+ * Check sockets for writing
+ */
+ if (!(so->so_state & SS_NOFDREF) &&
+ (revents & (G_IO_OUT | G_IO_ERR))) {
+ /*
+ * Check for non-blocking, still-connecting sockets
+ */
+ if (so->so_state & SS_ISFCONNECTING) {
+ /* Connected */
+ so->so_state &= ~SS_ISFCONNECTING;
+
+ ret = send(so->s, (const void *) &ret, 0, 0);
+ if (ret < 0) {
+ /* XXXXX Must fix, zero bytes is a NOP */
+ if (errno == EAGAIN || errno == EWOULDBLOCK ||
+ errno == EINPROGRESS || errno == ENOTCONN) {
continue;
- } /* else */
- ret = soread(so);
-
- /* Output it if we read something */
- if (ret > 0) {
- tcp_output(sototcpcb(so));
}
- }
- /*
- * Check sockets for writing
- */
- if (!(so->so_state & SS_NOFDREF) &&
- (revents & (G_IO_OUT | G_IO_ERR))) {
- /*
- * Check for non-blocking, still-connecting sockets
- */
- if (so->so_state & SS_ISFCONNECTING) {
- /* Connected */
- so->so_state &= ~SS_ISFCONNECTING;
-
- ret = send(so->s, (const void *) &ret, 0, 0);
- if (ret < 0) {
- /* XXXXX Must fix, zero bytes is a NOP */
- if (errno == EAGAIN || errno == EWOULDBLOCK ||
- errno == EINPROGRESS || errno == ENOTCONN) {
- continue;
- }
-
- /* else failed */
- so->so_state &= SS_PERSISTENT_MASK;
- so->so_state |= SS_NOFDREF;
- }
- /* else so->so_state &= ~SS_ISFCONNECTING; */
-
- /*
- * Continue tcp_input
- */
- tcp_input((struct mbuf *)NULL, sizeof(struct ip), so);
- /* continue; */
- } else {
- ret = sowrite(so);
- }
- /*
- * XXXXX If we wrote something (a lot), there
- * could be a need for a window update.
- * In the worst case, the remote will send
- * a window probe to get things going again
- */
+ /* else failed */
+ so->so_state &= SS_PERSISTENT_MASK;
+ so->so_state |= SS_NOFDREF;
}
+ /* else so->so_state &= ~SS_ISFCONNECTING; */
/*
- * Probe a still-connecting, non-blocking socket
- * to check if it's still alive
+ * Continue tcp_input
*/
-#ifdef PROBE_CONN
- if (so->so_state & SS_ISFCONNECTING) {
- ret = qemu_recv(so->s, &ret, 0, 0);
-
- if (ret < 0) {
- /* XXX */
- if (errno == EAGAIN || errno == EWOULDBLOCK ||
- errno == EINPROGRESS || errno == ENOTCONN) {
- continue; /* Still connecting, continue */
- }
-
- /* else failed */
- so->so_state &= SS_PERSISTENT_MASK;
- so->so_state |= SS_NOFDREF;
-
- /* tcp_input will take care of it */
- } else {
- ret = send(so->s, &ret, 0, 0);
- if (ret < 0) {
- /* XXX */
- if (errno == EAGAIN || errno == EWOULDBLOCK ||
- errno == EINPROGRESS || errno == ENOTCONN) {
- continue;
- }
- /* else failed */
- so->so_state &= SS_PERSISTENT_MASK;
- so->so_state |= SS_NOFDREF;
- } else {
- so->so_state &= ~SS_ISFCONNECTING;
- }
-
- }
- tcp_input((struct mbuf *)NULL, sizeof(struct ip), so);
- } /* SS_ISFCONNECTING */
-#endif
+ tcp_input((struct mbuf *)NULL, sizeof(struct ip), so);
+ /* continue; */
+ } else {
+ ret = sowrite(so);
}
/*
- * Now UDP sockets.
- * Incoming packets are sent straight away, they're not buffered.
- * Incoming UDP data isn't buffered either.
+ * XXXXX If we wrote something (a lot), there
+ * could be a need for a window update.
+ * In the worst case, the remote will send
+ * a window probe to get things going again
*/
- for (so = slirp->udb.so_next; so != &slirp->udb;
- so = so_next) {
- int revents;
-
- so_next = so->so_next;
+ }
- revents = 0;
- if (so->pollfds_idx != -1) {
- revents = g_array_index(pollfds, GPollFD,
- so->pollfds_idx).revents;
+ /*
+ * Probe a still-connecting, non-blocking socket
+ * to check if it's still alive
+ */
+#ifdef PROBE_CONN
+ if (so->so_state & SS_ISFCONNECTING) {
+ ret = qemu_recv(so->s, &ret, 0, 0);
+
+ if (ret < 0) {
+ /* XXX */
+ if (errno == EAGAIN || errno == EWOULDBLOCK ||
+ errno == EINPROGRESS || errno == ENOTCONN) {
+ continue; /* Still connecting, continue */
}
- if (so->s != -1 &&
- (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR))) {
- sorecvfrom(so);
+ /* else failed */
+ so->so_state &= SS_PERSISTENT_MASK;
+ so->so_state |= SS_NOFDREF;
+
+ /* tcp_input will take care of it */
+ } else {
+ ret = send(so->s, &ret, 0, 0);
+ if (ret < 0) {
+ /* XXX */
+ if (errno == EAGAIN || errno == EWOULDBLOCK ||
+ errno == EINPROGRESS || errno == ENOTCONN) {
+ continue;
+ }
+ /* else failed */
+ so->so_state &= SS_PERSISTENT_MASK;
+ so->so_state |= SS_NOFDREF;
+ } else {
+ so->so_state &= ~SS_ISFCONNECTING;
}
+
}
+ tcp_input((struct mbuf *)NULL, sizeof(struct ip), so);
+ } /* SS_ISFCONNECTING */
+#endif
+ }
- /*
- * Check incoming ICMP relies.
- */
- for (so = slirp->icmp.so_next; so != &slirp->icmp;
- so = so_next) {
- int revents;
+ /*
+ * Now UDP sockets.
+ * Incoming packets are sent straight away, they're not buffered.
+ * Incoming UDP data isn't buffered either.
+ */
+ for (so = slirp->udb.so_next; so != &slirp->udb;
+ so = so_next) {
+ int revents;
- so_next = so->so_next;
+ so_next = so->so_next;
- revents = 0;
- if (so->pollfds_idx != -1) {
- revents = g_array_index(pollfds, GPollFD,
- so->pollfds_idx).revents;
- }
+ revents = 0;
+ if (so->pollfd) {
+ revents = so->pollfd->revents;
+ }
- if (so->s != -1 &&
- (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR))) {
- icmp_receive(so);
- }
- }
+ if (so->s != -1 &&
+ (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR))) {
+ sorecvfrom(so);
}
+ }
+
+ /*
+ * Check incoming ICMP relies.
+ */
+ for (so = slirp->icmp.so_next; so != &slirp->icmp;
+ so = so_next) {
+ int revents;
+
+ so_next = so->so_next;
- if_start(slirp);
+ revents = 0;
+ if (so->pollfd) {
+ revents = so->pollfd->revents;
+ }
+
+ if (so->s != -1 &&
+ (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR))) {
+ icmp_receive(so);
+ }
}
+
+ if_start(slirp);
+ return true;
}
static void arp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len)
diff --git a/slirp/socket.c b/slirp/socket.c
index 8e8819c..4da7dfa 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -52,6 +52,7 @@ socreate(Slirp *slirp)
so->s = -1;
so->slirp = slirp;
so->pollfds_idx = -1;
+ so->pollfd = slirp_gsource_add_pollfd(slirp->opaque, so->s);
}
return(so);
}
@@ -64,6 +65,7 @@ sofree(struct socket *so)
{
Slirp *slirp = so->slirp;
+ slirp_gsource_remove_pollfd(slirp->opaque, so->pollfd);
if (so->so_emu==EMU_RSH && so->extra) {
sofree(so->extra);
so->extra=NULL;
diff --git a/slirp/socket.h b/slirp/socket.h
index 57e0407..522c5f0 100644
--- a/slirp/socket.h
+++ b/slirp/socket.h
@@ -21,6 +21,7 @@ struct socket {
int s; /* The actual socket */
int pollfds_idx; /* GPollFD GArray index */
+ GPollFD *pollfd;
Slirp *slirp; /* managing slirp instance */
diff --git a/stubs/slirp.c b/stubs/slirp.c
index f1fc833..c343364 100644
--- a/stubs/slirp.c
+++ b/stubs/slirp.c
@@ -5,11 +5,3 @@ void slirp_update_timeout(uint32_t *timeout)
{
}
-void slirp_pollfds_fill(GArray *pollfds)
-{
-}
-
-void slirp_pollfds_poll(GArray *pollfds, int select_error)
-{
-}
-
--
1.8.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v1 4/5] slirp: decouple timeout for gpoll
2013-08-08 6:26 [Qemu-devel] [PATCH v1 0/5] make slirp subsystem self-contained Liu Ping Fan
` (2 preceding siblings ...)
2013-08-08 6:26 ` [Qemu-devel] [PATCH v1 3/5] slirp: make slirp event dispatch based on slirp instance Liu Ping Fan
@ 2013-08-08 6:26 ` Liu Ping Fan
2013-08-08 6:26 ` [Qemu-devel] [PATCH v1 5/5] slirp: fold curtime into slirp instance Liu Ping Fan
4 siblings, 0 replies; 14+ messages in thread
From: Liu Ping Fan @ 2013-08-08 6:26 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, mdroth, Stefan Hajnoczi
In order to decouple with main loop, we fill timeout for gpoll
through slirp's GSource prepare. (Later, after curtime is decoupled,
slirp system will be self-contained, and isolated from main-loop)
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
slirp/slirp.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 37244be..ecc4d88 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -260,9 +260,6 @@ void slirp_cleanup(Slirp *slirp)
void slirp_update_timeout(uint32_t *timeout)
{
- if (!QTAILQ_EMPTY(&slirp_instances)) {
- *timeout = MIN(1000, *timeout);
- }
curtime = qemu_get_clock_ms(rt_clock);
}
@@ -273,6 +270,7 @@ gboolean slirp_prepare(GSource *source, gint *time)
struct socket *so, *so_next;
int events = 0;
+ *time = MIN(1000, *time);
/*
* *_slowtimo needs calling if there are IP fragments
* in the fragment queue, or there are TCP connections active
--
1.8.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PATCH v1 5/5] slirp: fold curtime into slirp instance
2013-08-08 6:26 [Qemu-devel] [PATCH v1 0/5] make slirp subsystem self-contained Liu Ping Fan
` (3 preceding siblings ...)
2013-08-08 6:26 ` [Qemu-devel] [PATCH v1 4/5] slirp: decouple timeout for gpoll Liu Ping Fan
@ 2013-08-08 6:26 ` Liu Ping Fan
4 siblings, 0 replies; 14+ messages in thread
From: Liu Ping Fan @ 2013-08-08 6:26 UTC (permalink / raw)
To: qemu-devel; +Cc: Paolo Bonzini, mdroth, Stefan Hajnoczi
Each slirp instance will work on its own curtime updated when _prepare.
This patch help to fold the remaining main-loop's related stuff into GSource,
and isolated from main-loop.
Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
main-loop.c | 3 ---
slirp/ip_icmp.c | 4 ++--
slirp/libslirp.h | 2 +-
slirp/main.h | 1 -
slirp/misc.c | 4 ++--
slirp/slirp.c | 17 +++++++----------
slirp/slirp.h | 1 +
slirp/socket.c | 8 ++++----
slirp/tcp_subr.c | 2 +-
slirp/tftp.c | 4 ++--
slirp/udp.c | 4 ++--
stubs/Makefile.objs | 1 -
stubs/slirp.c | 7 -------
13 files changed, 22 insertions(+), 36 deletions(-)
delete mode 100644 stubs/slirp.c
diff --git a/main-loop.c b/main-loop.c
index c0da803..ee655ad 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -457,9 +457,6 @@ int main_loop_wait(int nonblocking)
/* poll any events */
g_array_set_size(gpollfds, 0); /* reset for new iteration */
/* XXX: separate device handlers from system ones */
-#ifdef CONFIG_SLIRP
- slirp_update_timeout(&timeout);
-#endif
qemu_iohandler_fill(gpollfds);
ret = os_host_main_loop_wait(timeout);
qemu_iohandler_poll(gpollfds, ret);
diff --git a/slirp/ip_icmp.c b/slirp/ip_icmp.c
index 9f1cb08..da9efc4 100644
--- a/slirp/ip_icmp.c
+++ b/slirp/ip_icmp.c
@@ -89,7 +89,7 @@ static int icmp_send(struct socket *so, struct mbuf *m, int hlen)
so->so_iptos = ip->ip_tos;
so->so_type = IPPROTO_ICMP;
so->so_state = SS_ISFCONNECTED;
- so->so_expire = curtime + SO_EXPIRE;
+ so->so_expire = so->slirp->curtime + SO_EXPIRE;
addr.sin_family = AF_INET;
addr.sin_addr = so->so_faddr;
@@ -184,7 +184,7 @@ icmp_input(struct mbuf *m, int hlen)
slirp->vnetwork_addr.s_addr) {
/* It's an alias */
if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
- if (get_dns_addr(&addr.sin_addr) < 0)
+ if (get_dns_addr(&addr.sin_addr, slirp->curtime) < 0)
addr.sin_addr = loopback_addr;
} else {
addr.sin_addr = loopback_addr;
diff --git a/slirp/libslirp.h b/slirp/libslirp.h
index 2dc131f..3e10a68 100644
--- a/slirp/libslirp.h
+++ b/slirp/libslirp.h
@@ -6,7 +6,7 @@
struct Slirp;
typedef struct Slirp Slirp;
-int get_dns_addr(struct in_addr *pdns_addr);
+int get_dns_addr(struct in_addr *pdns_addr, u_int curtime);
Slirp *slirp_init(int restricted, struct in_addr vnetwork,
struct in_addr vnetmask, struct in_addr vhost,
diff --git a/slirp/main.h b/slirp/main.h
index f2e58cf..d7e9726 100644
--- a/slirp/main.h
+++ b/slirp/main.h
@@ -30,7 +30,6 @@ extern int ctty_closed;
extern char *slirp_tty;
extern char *exec_shell;
-extern u_int curtime;
extern struct in_addr loopback_addr;
extern unsigned long loopback_mask;
extern char *username;
diff --git a/slirp/misc.c b/slirp/misc.c
index 0bcc481..b62de16 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -314,7 +314,7 @@ void slirp_connection_info(Slirp *slirp, Monitor *mon)
dst_port = so->so_lport;
} else {
snprintf(buf, sizeof(buf), " UDP[%d sec]",
- (so->so_expire - curtime) / 1000);
+ (so->so_expire - so->slirp->curtime) / 1000);
src.sin_addr = so->so_laddr;
src.sin_port = so->so_lport;
dst_addr = so->so_faddr;
@@ -330,7 +330,7 @@ void slirp_connection_info(Slirp *slirp, Monitor *mon)
for (so = slirp->icmp.so_next; so != &slirp->icmp; so = so->so_next) {
snprintf(buf, sizeof(buf), " ICMP[%d sec]",
- (so->so_expire - curtime) / 1000);
+ (so->so_expire - so->slirp->curtime) / 1000);
src.sin_addr = so->so_laddr;
dst_addr = so->so_faddr;
monitor_printf(mon, "%-19s %3d %15s - ", buf, so->s,
diff --git a/slirp/slirp.c b/slirp/slirp.c
index ecc4d88..bbe7161 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -40,8 +40,6 @@ static const uint8_t special_ethaddr[ETH_ALEN] = {
static const uint8_t zero_ethaddr[ETH_ALEN] = { 0, 0, 0, 0, 0, 0 };
-u_int curtime;
-
static QTAILQ_HEAD(slirp_instances, Slirp) slirp_instances =
QTAILQ_HEAD_INITIALIZER(slirp_instances);
@@ -50,7 +48,7 @@ static u_int dns_addr_time;
#ifdef _WIN32
-int get_dns_addr(struct in_addr *pdns_addr)
+int get_dns_addr(struct in_addr *pdns_addr, u_int curtime)
{
FIXED_INFO *FixedInfo=NULL;
ULONG BufLen;
@@ -104,7 +102,7 @@ static void winsock_cleanup(void)
static struct stat dns_addr_stat;
-int get_dns_addr(struct in_addr *pdns_addr)
+int get_dns_addr(struct in_addr *pdns_addr, u_int curtime)
{
char buff[512];
char buff2[257];
@@ -258,19 +256,17 @@ void slirp_cleanup(Slirp *slirp)
#define CONN_CANFSEND(so) (((so)->so_state & (SS_FCANTSENDMORE|SS_ISFCONNECTED)) == SS_ISFCONNECTED)
#define CONN_CANFRCV(so) (((so)->so_state & (SS_FCANTRCVMORE|SS_ISFCONNECTED)) == SS_ISFCONNECTED)
-void slirp_update_timeout(uint32_t *timeout)
-{
- curtime = qemu_get_clock_ms(rt_clock);
-}
-
gboolean slirp_prepare(GSource *source, gint *time)
{
EventsGSource *slirp_src = (EventsGSource *)source;
Slirp *slirp = slirp_src->opaque;
struct socket *so, *so_next;
int events = 0;
+ u_int curtime;
- *time = MIN(1000, *time);
+ *time = 1000;
+ curtime = qemu_get_clock_ms(rt_clock);
+ slirp->curtime = curtime;
/*
* *_slowtimo needs calling if there are IP fragments
* in the fragment queue, or there are TCP connections active
@@ -407,6 +403,7 @@ gboolean slirp_handler(gpointer data)
Slirp *slirp = src->opaque;
struct socket *so, *so_next;
int ret;
+ u_int curtime = slirp->curtime;
/*
* See if anything has timed out
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 008360e..018d4a9 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -206,6 +206,7 @@ struct Slirp {
u_int time_fasttimo;
u_int last_slowtimo;
int do_slowtimo;
+ u_int curtime;
/* virtual network configuration */
struct in_addr vnetwork_addr;
diff --git a/slirp/socket.c b/slirp/socket.c
index 4da7dfa..8820b30 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -520,9 +520,9 @@ sorecvfrom(struct socket *so)
*/
if (so->so_expire) {
if (so->so_fport == htons(53))
- so->so_expire = curtime + SO_EXPIREFAST;
+ so->so_expire = so->slirp->curtime + SO_EXPIREFAST;
else
- so->so_expire = curtime + SO_EXPIRE;
+ so->so_expire = so->slirp->curtime + SO_EXPIRE;
}
/*
@@ -553,7 +553,7 @@ sosendto(struct socket *so, struct mbuf *m)
slirp->vnetwork_addr.s_addr) {
/* It's an alias */
if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
- if (get_dns_addr(&addr.sin_addr) < 0)
+ if (get_dns_addr(&addr.sin_addr, slirp->curtime) < 0)
addr.sin_addr = loopback_addr;
} else {
addr.sin_addr = loopback_addr;
@@ -575,7 +575,7 @@ sosendto(struct socket *so, struct mbuf *m)
* but only if it's an expirable socket
*/
if (so->so_expire)
- so->so_expire = curtime + SO_EXPIRE;
+ so->so_expire = slirp->curtime + SO_EXPIRE;
so->so_state &= SS_PERSISTENT_MASK;
so->so_state |= SS_ISFCONNECTED; /* So that it gets select()ed */
return 0;
diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
index 043f28f..69dbeec 100644
--- a/slirp/tcp_subr.c
+++ b/slirp/tcp_subr.c
@@ -347,7 +347,7 @@ int tcp_fconnect(struct socket *so)
slirp->vnetwork_addr.s_addr) {
/* It's an alias */
if (so->so_faddr.s_addr == slirp->vnameserver_addr.s_addr) {
- if (get_dns_addr(&addr.sin_addr) < 0)
+ if (get_dns_addr(&addr.sin_addr, slirp->curtime) < 0)
addr.sin_addr = loopback_addr;
} else {
addr.sin_addr = loopback_addr;
diff --git a/slirp/tftp.c b/slirp/tftp.c
index 1a79c45..5ef772e 100644
--- a/slirp/tftp.c
+++ b/slirp/tftp.c
@@ -32,7 +32,7 @@ static inline int tftp_session_in_use(struct tftp_session *spt)
static inline void tftp_session_update(struct tftp_session *spt)
{
- spt->timestamp = curtime;
+ spt->timestamp = spt->slirp->curtime;
}
static void tftp_session_terminate(struct tftp_session *spt)
@@ -57,7 +57,7 @@ static int tftp_session_allocate(Slirp *slirp, struct tftp_t *tp)
goto found;
/* sessions time out after 5 inactive seconds */
- if ((int)(curtime - spt->timestamp) > 5000) {
+ if ((int)(slirp->curtime - spt->timestamp) > 5000) {
tftp_session_terminate(spt);
goto found;
}
diff --git a/slirp/udp.c b/slirp/udp.c
index b105f87..e0c90df 100644
--- a/slirp/udp.c
+++ b/slirp/udp.c
@@ -313,7 +313,7 @@ int
udp_attach(struct socket *so)
{
if((so->s = qemu_socket(AF_INET,SOCK_DGRAM,0)) != -1) {
- so->so_expire = curtime + SO_EXPIRE;
+ so->so_expire = so->slirp->curtime + SO_EXPIRE;
insque(so, &so->slirp->udb);
}
return(so->s);
@@ -361,7 +361,7 @@ udp_listen(Slirp *slirp, uint32_t haddr, u_int hport, uint32_t laddr,
return NULL;
}
so->s = qemu_socket(AF_INET,SOCK_DGRAM,0);
- so->so_expire = curtime + SO_EXPIRE;
+ so->so_expire = slirp->curtime + SO_EXPIRE;
insque(so, &slirp->udb);
addr.sin_family = AF_INET;
diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index 9b701b4..dafc452 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -19,7 +19,6 @@ stub-obj-y += mon-set-error.o
stub-obj-y += pci-drive-hot-add.o
stub-obj-y += reset.o
stub-obj-y += set-fd-handler.o
-stub-obj-y += slirp.o
stub-obj-y += sysbus.o
stub-obj-y += vm-stop.o
stub-obj-y += vmstate.o
diff --git a/stubs/slirp.c b/stubs/slirp.c
deleted file mode 100644
index c343364..0000000
--- a/stubs/slirp.c
+++ /dev/null
@@ -1,7 +0,0 @@
-#include "qemu-common.h"
-#include "slirp/slirp.h"
-
-void slirp_update_timeout(uint32_t *timeout)
-{
-}
-
--
1.8.1.4
^ permalink raw reply related [flat|nested] 14+ messages in thread