On Fri, Aug 9, 2013 at 12:29 AM, Michael Roth <mdr...@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 <pingf...@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
