[Qemu-devel] [PATCH 2/9] char: return a tag when adding the fe handlers

[Marc-André Lureau]

The tag helps to keep track of the frontend, and will be used in a later
change for removing the associated callbacks. Once the conversion is
done, a warning will be added to not ignore the return value, since that
would likely lead to an incorrect behaviour.

Signed-off-by: Marc-André Lureau <address@hidden>
---
 qemu-char.c           | 149 ++++++++++++++++++++++++++++++++++----------------
 include/sysemu/char.h |  13 ++++-
 2 files changed, 111 insertions(+), 51 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index d83a896..168af69 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -89,6 +89,8 @@
 #define READ_RETRIES 10
 #define TCP_MAX_FDS 16
 
+typedef struct MuxDriver MuxDriver;
+
 /***********************************************************/
 /* Socket address helpers */
 
@@ -449,12 +451,14 @@ void qemu_chr_fe_printf(CharDriverState *s, const char 
*fmt, ...)
 
 static void remove_fd_in_watch(CharDriverState *chr);
 
-void qemu_chr_add_handlers_full(CharDriverState *s,
-                                IOCanReadHandler *fd_can_read,
-                                IOReadHandler *fd_read,
-                                IOEventHandler *fd_event,
-                                void *opaque,
-                                GMainContext *context)
+static void
+qemu_chr_set_handlers(CharDriverState *s,
+                      IOCanReadHandler *fd_can_read,
+                      IOReadHandler *fd_read,
+                      IOEventHandler *fd_event,
+                      void *opaque,
+                      GMainContext *context,
+                      int tag)
 {
     int fe_open;
 
@@ -469,7 +473,7 @@ void qemu_chr_add_handlers_full(CharDriverState *s,
     s->chr_event = fd_event;
     s->handler_opaque = opaque;
     if (s->chr_update_read_handler) {
-        s->chr_update_read_handler(s, context);
+        s->chr_update_read_handler(s, context, tag);
     }
 
     if (!s->explicit_fe_open) {
@@ -483,14 +487,43 @@ void qemu_chr_add_handlers_full(CharDriverState *s,
     }
 }
 
-void qemu_chr_add_handlers(CharDriverState *s,
-                           IOCanReadHandler *fd_can_read,
-                           IOReadHandler *fd_read,
-                           IOEventHandler *fd_event,
-                           void *opaque)
+static int mux_chr_new_handler_tag(CharDriverState *chr, GMainContext 
*context);
+static void mux_set_focus(MuxDriver *d, int focus);
+
+int qemu_chr_add_handlers_full(CharDriverState *s,
+                               IOCanReadHandler *fd_can_read,
+                               IOReadHandler *fd_read,
+                               IOEventHandler *fd_event,
+                               void *opaque,
+                               GMainContext *context)
+{
+    int tag = 0;
+
+    if (s->is_mux) {
+        tag = mux_chr_new_handler_tag(s, context);
+        if (tag < 0) {
+            return tag;
+        }
+    }
+
+    qemu_chr_set_handlers(s, fd_can_read, fd_read,
+                          fd_event, opaque, context, tag);
+
+    if (s->is_mux) {
+        mux_set_focus(s->opaque, tag);
+    }
+
+    return tag;
+}
+
+int qemu_chr_add_handlers(CharDriverState *s,
+                          IOCanReadHandler *fd_can_read,
+                          IOReadHandler *fd_read,
+                          IOEventHandler *fd_event,
+                          void *opaque)
 {
-    qemu_chr_add_handlers_full(s, fd_can_read, fd_read,
-                               fd_event, opaque, NULL);
+    return qemu_chr_add_handlers_full(s, fd_can_read, fd_read,
+                                      fd_event, opaque, NULL);
 }
 
 static int null_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
@@ -519,7 +552,7 @@ static CharDriverState *qemu_chr_open_null(const char *id,
 #define MAX_MUX 4
 #define MUX_BUFFER_SIZE 32     /* Must be a power of 2.  */
 #define MUX_BUFFER_MASK (MUX_BUFFER_SIZE - 1)
-typedef struct {
+struct MuxDriver {
     IOCanReadHandler *chr_can_read[MAX_MUX];
     IOReadHandler *chr_read[MAX_MUX];
     IOEventHandler *chr_event[MAX_MUX];
@@ -540,8 +573,7 @@ typedef struct {
     /* Protected by the CharDriverState chr_write_lock.  */
     int linestart;
     int64_t timestamps_start;
-} MuxDriver;
-
+};
 
 /* Called with chr_write_lock held.  */
 static int mux_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
@@ -655,12 +687,9 @@ static int mux_proc_byte(CharDriverState *chr, MuxDriver 
*d, int ch)
             qemu_chr_be_event(chr, CHR_EVENT_BREAK);
             break;
         case 'c':
+            assert(d->mux_cnt > 0); /* handler registered with first fe */
             /* Switch to the next registered device */
-            mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_OUT);
-            d->focus++;
-            if (d->focus >= d->mux_cnt)
-                d->focus = 0;
-            mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_IN);
+            mux_set_focus(d, (d->focus + 1) % d->mux_cnt);
             break;
         case 't':
             d->timestamps = !d->timestamps;
@@ -735,31 +764,18 @@ static void mux_chr_event(void *opaque, int event)
 }
 
 static void mux_chr_update_read_handler(CharDriverState *chr,
-                                        GMainContext *context)
+                                        GMainContext *context,
+                                        int tag)
 {
     MuxDriver *d = chr->opaque;
 
-    if (d->mux_cnt >= MAX_MUX) {
-        fprintf(stderr, "Cannot add I/O handlers, MUX array is full\n");
-        return;
-    }
-    d->ext_opaque[d->mux_cnt] = chr->handler_opaque;
-    d->chr_can_read[d->mux_cnt] = chr->chr_can_read;
-    d->chr_read[d->mux_cnt] = chr->chr_read;
-    d->chr_event[d->mux_cnt] = chr->chr_event;
-    /* Fix up the real driver with mux routines */
-    if (d->mux_cnt == 0) {
-        qemu_chr_add_handlers_full(d->drv, mux_chr_can_read,
-                                   mux_chr_read,
-                                   mux_chr_event,
-                                   chr, context);
-    }
-    if (d->focus != -1) {
-        mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_OUT);
-    }
-    d->focus = d->mux_cnt;
-    d->mux_cnt++;
-    mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_IN);
+    assert(tag >= 0);
+    assert(tag < d->mux_cnt);
+
+    d->ext_opaque[tag] = chr->handler_opaque;
+    d->chr_can_read[tag] = chr->chr_can_read;
+    d->chr_read[tag] = chr->chr_read;
+    d->chr_event[tag] = chr->chr_event;
 }
 
 static bool muxes_realized;
@@ -815,6 +831,39 @@ static void mux_chr_close(struct CharDriverState *chr)
     g_free(d);
 }
 
+static int mux_chr_new_handler_tag(CharDriverState *chr, GMainContext *context)
+{
+    MuxDriver *d = chr->opaque;
+
+    if (d->mux_cnt >= MAX_MUX) {
+        fprintf(stderr, "Cannot add I/O handlers, MUX array is full\n");
+        return -1;
+    }
+
+    /* Fix up the real driver with mux routines */
+    if (d->mux_cnt == 0) {
+        qemu_chr_add_handlers_full(d->drv, mux_chr_can_read,
+                                                mux_chr_read,
+                                                mux_chr_event,
+                                                chr, context);
+    }
+
+    return d->mux_cnt++;
+}
+
+static void mux_set_focus(MuxDriver *d, int focus)
+{
+    assert(focus >= 0);
+    assert(focus < d->mux_cnt);
+
+    if (d->focus != -1) {
+        mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_OUT);
+    }
+
+    d->focus = focus;
+    mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_IN);
+}
+
 static CharDriverState *qemu_chr_open_mux(const char *id,
                                           ChardevBackend *backend,
                                           ChardevReturn *ret, Error **errp)
@@ -1085,7 +1134,8 @@ static GSource *fd_chr_add_watch(CharDriverState *chr, 
GIOCondition cond)
 }
 
 static void fd_chr_update_read_handler(CharDriverState *chr,
-                                       GMainContext *context)
+                                       GMainContext *context,
+                                       int tag)
 {
     FDCharDriver *s = chr->opaque;
 
@@ -1342,7 +1392,8 @@ static void 
pty_chr_update_read_handler_locked(CharDriverState *chr)
 }
 
 static void pty_chr_update_read_handler(CharDriverState *chr,
-                                        GMainContext *context)
+                                        GMainContext *context,
+                                        int tag)
 {
     qemu_mutex_lock(&chr->chr_write_lock);
     pty_chr_update_read_handler_locked(chr);
@@ -2590,7 +2641,8 @@ static gboolean udp_chr_read(QIOChannel *chan, 
GIOCondition cond, void *opaque)
 }
 
 static void udp_chr_update_read_handler(CharDriverState *chr,
-                                        GMainContext *context)
+                                        GMainContext *context,
+                                        int tag)
 {
     NetCharDriver *s = chr->opaque;
 
@@ -3009,7 +3061,8 @@ static void tcp_chr_connect(void *opaque)
 }
 
 static void tcp_chr_update_read_handler(CharDriverState *chr,
-                                        GMainContext *context)
+                                        GMainContext *context,
+                                        int tag)
 {
     TCPCharDriver *s = chr->opaque;
 
diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 19dad3f..bdd61ea 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -81,7 +81,7 @@ struct CharDriverState {
                          const uint8_t *buf, int len);
     GSource *(*chr_add_watch)(struct CharDriverState *s, GIOCondition cond);
     void (*chr_update_read_handler)(struct CharDriverState *s,
-                                    GMainContext *context);
+                                    GMainContext *context, int tag);
     int (*chr_ioctl)(struct CharDriverState *s, int cmd, void *arg);
     int (*get_msgfds)(struct CharDriverState *s, int* fds, int num);
     int (*set_msgfds)(struct CharDriverState *s, int *fds, int num);
@@ -432,14 +432,21 @@ void qemu_chr_be_write_impl(CharDriverState *s, uint8_t 
*buf, int len);
  */
 void qemu_chr_be_event(CharDriverState *s, int event);
 
-void qemu_chr_add_handlers(CharDriverState *s,
+/**
+ * @qemu_chr_add_handlers:
+ *
+ * Register the frontend callbacks.
+ *
+ * Returns: a tag associated with the handlers, or -1 on error.
+ */
+int qemu_chr_add_handlers(CharDriverState *s,
                            IOCanReadHandler *fd_can_read,
                            IOReadHandler *fd_read,
                            IOEventHandler *fd_event,
                            void *opaque);
 
 /* This API can make handler run in the context what you pass to. */
-void qemu_chr_add_handlers_full(CharDriverState *s,
+int qemu_chr_add_handlers_full(CharDriverState *s,
                                 IOCanReadHandler *fd_can_read,
                                 IOReadHandler *fd_read,
                                 IOEventHandler *fd_event,
-- 
2.10.0