diff mbox series

[v2,06/15] qio: store gsources for net listeners

Message ID 20180301084438.13594-7-peterx@redhat.com
State New
Headers show
Series qio: general non-default GMainContext support | expand

Commit Message

Peter Xu March 1, 2018, 8:44 a.m. UTC 
Originally we were storing the GSources tag IDs.  That'll be not enough
if we are going to support non-default gcontext for QIO code.  Switch to
GSources without changing anything real.  Now we still always pass in
NULL, which means the default gcontext.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 include/io/net-listener.h | 21 ++++++++++++++--
 io/net-listener.c         | 62 +++++++++++++++++++++++++++++------------------
 2 files changed, 58 insertions(+), 25 deletions(-)

Comments

Daniel P. Berrangé March 1, 2018, 3:40 p.m. UTC | #1 
On Thu, Mar 01, 2018 at 04:44:29PM +0800, Peter Xu wrote:
> Originally we were storing the GSources tag IDs.  That'll be not enough
> if we are going to support non-default gcontext for QIO code.  Switch to
> GSources without changing anything real.  Now we still always pass in
> NULL, which means the default gcontext.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  include/io/net-listener.h | 21 ++++++++++++++--
>  io/net-listener.c         | 62 +++++++++++++++++++++++++++++------------------
>  2 files changed, 58 insertions(+), 25 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
Paolo Bonzini March 1, 2018, 5:12 p.m. UTC | #2 
On 01/03/2018 09:44, Peter Xu wrote:
> Originally we were storing the GSources tag IDs.  That'll be not enough
> if we are going to support non-default gcontext for QIO code.  Switch to
> GSources without changing anything real.  Now we still always pass in
> NULL, which means the default gcontext.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  include/io/net-listener.h | 21 ++++++++++++++--
>  io/net-listener.c         | 62 +++++++++++++++++++++++++++++------------------
>  2 files changed, 58 insertions(+), 25 deletions(-)
> 
> diff --git a/include/io/net-listener.h b/include/io/net-listener.h
> index 56d6da7a76..566be283b3 100644
> --- a/include/io/net-listener.h
> +++ b/include/io/net-listener.h
> @@ -53,7 +53,7 @@ struct QIONetListener {
>  
>      char *name;
>      QIOChannelSocket **sioc;
> -    gulong *io_tag;
> +    GSource **io_source;
>      size_t nsioc;
>  
>      bool connected;
> @@ -120,17 +120,34 @@ void qio_net_listener_add(QIONetListener *listener,
>                            QIOChannelSocket *sioc);
>  
>  /**
> - * qio_net_listener_set_client_func:
> + * qio_net_listener_set_client_func_full:
>   * @listener: the network listener object
>   * @func: the callback function
>   * @data: opaque data to pass to @func
>   * @notify: callback to free @data
> + * @context: the context that the sources will be bound to

Please add a note like "if %NULL, the default context will be used".

Paolo
Peter Xu March 2, 2018, 4:10 a.m. UTC | #3 
On Thu, Mar 01, 2018 at 06:12:57PM +0100, Paolo Bonzini wrote:
> On 01/03/2018 09:44, Peter Xu wrote:
> > Originally we were storing the GSources tag IDs.  That'll be not enough
> > if we are going to support non-default gcontext for QIO code.  Switch to
> > GSources without changing anything real.  Now we still always pass in
> > NULL, which means the default gcontext.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  include/io/net-listener.h | 21 ++++++++++++++--
> >  io/net-listener.c         | 62 +++++++++++++++++++++++++++++------------------
> >  2 files changed, 58 insertions(+), 25 deletions(-)
> > 
> > diff --git a/include/io/net-listener.h b/include/io/net-listener.h
> > index 56d6da7a76..566be283b3 100644
> > --- a/include/io/net-listener.h
> > +++ b/include/io/net-listener.h
> > @@ -53,7 +53,7 @@ struct QIONetListener {
> >  
> >      char *name;
> >      QIOChannelSocket **sioc;
> > -    gulong *io_tag;
> > +    GSource **io_source;
> >      size_t nsioc;
> >  
> >      bool connected;
> > @@ -120,17 +120,34 @@ void qio_net_listener_add(QIONetListener *listener,
> >                            QIOChannelSocket *sioc);
> >  
> >  /**
> > - * qio_net_listener_set_client_func:
> > + * qio_net_listener_set_client_func_full:
> >   * @listener: the network listener object
> >   * @func: the callback function
> >   * @data: opaque data to pass to @func
> >   * @notify: callback to free @data
> > + * @context: the context that the sources will be bound to
> 
> Please add a note like "if %NULL, the default context will be used".

Will do. Thanks,
Peter Xu March 2, 2018, 4:59 a.m. UTC | #4 
On Thu, Mar 01, 2018 at 06:12:57PM +0100, Paolo Bonzini wrote:
> On 01/03/2018 09:44, Peter Xu wrote:
> > Originally we were storing the GSources tag IDs.  That'll be not enough
> > if we are going to support non-default gcontext for QIO code.  Switch to
> > GSources without changing anything real.  Now we still always pass in
> > NULL, which means the default gcontext.
> > 
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  include/io/net-listener.h | 21 ++++++++++++++--
> >  io/net-listener.c         | 62 +++++++++++++++++++++++++++++------------------
> >  2 files changed, 58 insertions(+), 25 deletions(-)
> > 
> > diff --git a/include/io/net-listener.h b/include/io/net-listener.h
> > index 56d6da7a76..566be283b3 100644
> > --- a/include/io/net-listener.h
> > +++ b/include/io/net-listener.h
> > @@ -53,7 +53,7 @@ struct QIONetListener {
> >  
> >      char *name;
> >      QIOChannelSocket **sioc;
> > -    gulong *io_tag;
> > +    GSource **io_source;
> >      size_t nsioc;
> >  
> >      bool connected;
> > @@ -120,17 +120,34 @@ void qio_net_listener_add(QIONetListener *listener,
> >                            QIOChannelSocket *sioc);
> >  
> >  /**
> > - * qio_net_listener_set_client_func:
> > + * qio_net_listener_set_client_func_full:
> >   * @listener: the network listener object
> >   * @func: the callback function
> >   * @data: opaque data to pass to @func
> >   * @notify: callback to free @data
> > + * @context: the context that the sources will be bound to
> 
> Please add a note like "if %NULL, the default context will be used".

Fixed this one too (and for both patches, I kept Dan's r-b).  Thanks,
diff mbox series

Patch

diff --git a/include/io/net-listener.h b/include/io/net-listener.h
index 56d6da7a76..566be283b3 100644
--- a/include/io/net-listener.h
+++ b/include/io/net-listener.h
@@ -53,7 +53,7 @@  struct QIONetListener {
 
     char *name;
     QIOChannelSocket **sioc;
-    gulong *io_tag;
+    GSource **io_source;
     size_t nsioc;
 
     bool connected;
@@ -120,17 +120,34 @@  void qio_net_listener_add(QIONetListener *listener,
                           QIOChannelSocket *sioc);
 
 /**
- * qio_net_listener_set_client_func:
+ * qio_net_listener_set_client_func_full:
  * @listener: the network listener object
  * @func: the callback function
  * @data: opaque data to pass to @func
  * @notify: callback to free @data
+ * @context: the context that the sources will be bound to
  *
  * Register @func to be invoked whenever a new client
  * connects to the listener. @func will be invoked
  * passing in the QIOChannelSocket instance for the
  * client.
  */
+void qio_net_listener_set_client_func_full(QIONetListener *listener,
+                                           QIONetListenerClientFunc func,
+                                           gpointer data,
+                                           GDestroyNotify notify,
+                                           GMainContext *context);
+
+/**
+ * qio_net_listener_set_client_func:
+ * @listener: the network listener object
+ * @func: the callback function
+ * @data: opaque data to pass to @func
+ * @notify: callback to free @data
+ *
+ * Wrapper of qio_net_listener_set_client_func_full(), only that the
+ * sources will always be bound to default main context.
+ */
 void qio_net_listener_set_client_func(QIONetListener *listener,
                                       QIONetListenerClientFunc func,
                                       gpointer data,
diff --git a/io/net-listener.c b/io/net-listener.c
index 3e9ac51b0e..14fd47db6c 100644
--- a/io/net-listener.c
+++ b/io/net-listener.c
@@ -106,12 +106,15 @@  int qio_net_listener_open_sync(QIONetListener *listener,
     }
 }
 
-static guint qio_net_listener_source_add(QIONetListener *listener,
-                                         QIOChannelSocket *sioc)
+static GSource *qio_net_listener_source_add(QIONetListener *listener,
+                                            QIOChannelSocket *sioc,
+                                            GMainContext *context)
 {
-    return qio_channel_add_watch(QIO_CHANNEL(sioc), G_IO_IN,
-                                 qio_net_listener_channel_func,
-                                 listener, (GDestroyNotify)object_unref);
+    return qio_channel_add_watch_source(QIO_CHANNEL(sioc), G_IO_IN,
+                                        qio_net_listener_channel_func,
+                                        listener,
+                                        (GDestroyNotify)object_unref,
+                                        context);
 }
 
 static void qio_net_listener_sources_clear(QIONetListener *listener)
@@ -119,23 +122,25 @@  static void qio_net_listener_sources_clear(QIONetListener *listener)
     size_t i;
 
     for (i = 0; i < listener->nsioc; i++) {
-        if (listener->io_tag[i]) {
-            g_source_remove(listener->io_tag[i]);
-            listener->io_tag[i] = 0;
+        if (listener->io_source[i]) {
+            g_source_destroy(listener->io_source[i]);
+            g_source_unref(listener->io_source[i]);
+            listener->io_source[i] = NULL;
         }
     }
 }
 
-static void qio_net_listener_sources_update(QIONetListener *listener)
+static void qio_net_listener_sources_update(QIONetListener *listener,
+                                            GMainContext *context)
 {
     size_t i;
 
     if (listener->io_func != NULL) {
         for (i = 0; i < listener->nsioc; i++) {
-            assert(listener->io_tag[i] == 0);
+            assert(listener->io_source[i] == NULL);
             object_ref(OBJECT(listener));
-            listener->io_tag[i] = qio_net_listener_source_add(
-                listener, listener->sioc[i]);
+            listener->io_source[i] = qio_net_listener_source_add(
+                listener, listener->sioc[i], context);
         }
     }
 }
@@ -151,27 +156,30 @@  void qio_net_listener_add(QIONetListener *listener,
 
     listener->sioc = g_renew(QIOChannelSocket *, listener->sioc,
                              listener->nsioc + 1);
-    listener->io_tag = g_renew(gulong, listener->io_tag, listener->nsioc + 1);
+    listener->io_source = g_renew(typeof(listener->io_source[0]),
+                                  listener->io_source,
+                                  listener->nsioc + 1);
     listener->sioc[listener->nsioc] = sioc;
-    listener->io_tag[listener->nsioc] = 0;
+    listener->io_source[listener->nsioc] = NULL;
 
     object_ref(OBJECT(sioc));
     listener->connected = true;
 
     if (listener->io_func != NULL) {
         object_ref(OBJECT(listener));
-        listener->io_tag[listener->nsioc] = qio_net_listener_source_add(
-            listener, listener->sioc[listener->nsioc]);
+        listener->io_source[listener->nsioc] = qio_net_listener_source_add(
+            listener, listener->sioc[listener->nsioc], NULL);
     }
 
     listener->nsioc++;
 }
 
 
-void qio_net_listener_set_client_func(QIONetListener *listener,
-                                      QIONetListenerClientFunc func,
-                                      gpointer data,
-                                      GDestroyNotify notify)
+void qio_net_listener_set_client_func_full(QIONetListener *listener,
+                                           QIONetListenerClientFunc func,
+                                           gpointer data,
+                                           GDestroyNotify notify,
+                                           GMainContext *context)
 {
     if (listener->io_notify) {
         listener->io_notify(listener->io_data);
@@ -181,9 +189,17 @@  void qio_net_listener_set_client_func(QIONetListener *listener,
     listener->io_notify = notify;
 
     qio_net_listener_sources_clear(listener);
-    qio_net_listener_sources_update(listener);
+    qio_net_listener_sources_update(listener, context);
 }
 
+void qio_net_listener_set_client_func(QIONetListener *listener,
+                                      QIONetListenerClientFunc func,
+                                      gpointer data,
+                                      GDestroyNotify notify)
+{
+    qio_net_listener_set_client_func_full(listener, func, data,
+                                          notify, NULL);
+}
 
 struct QIONetListenerClientWaitData {
     QIOChannelSocket *sioc;
@@ -248,7 +264,7 @@  QIOChannelSocket *qio_net_listener_wait_client(QIONetListener *listener)
     g_main_loop_unref(loop);
     g_main_context_unref(ctxt);
 
-    qio_net_listener_sources_update(listener);
+    qio_net_listener_sources_update(listener, NULL);
 
     return data.sioc;
 }
@@ -285,7 +301,7 @@  static void qio_net_listener_finalize(Object *obj)
     for (i = 0; i < listener->nsioc; i++) {
         object_unref(OBJECT(listener->sioc[i]));
     }
-    g_free(listener->io_tag);
+    g_free(listener->io_source);
     g_free(listener->sioc);
     g_free(listener->name);
 }