From 7a9657ef5307e769f084bd9f7e20ebe1ee6ac044 Mon Sep 17 00:00:00 2001 From: Artem Pisarenko Date: Tue, 6 Nov 2018 18:40:51 +0600 Subject: chardev: fix mess in OPENED/CLOSED events when muxed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When chardev is multiplexed (mux=on) there are a lot of cases where CHR_EVENT_OPENED/CHR_EVENT_CLOSED events pairing (expected from frontend side) is broken. There are either generation of multiple repeated or extra CHR_EVENT_OPENED events, or CHR_EVENT_CLOSED just isn't generated at all. This is mostly because 'qemu_chr_fe_set_handlers()' function makes its own (and often wrong) implicit decision on updated frontend state and invokes 'fd_event' callback with 'CHR_EVENT_OPENED'. And even worse, it doesn't do symmetric action in opposite direction, as someone may expect (i.e. it doesn't invoke previously set 'fd_event' with 'CHR_EVENT_CLOSED'). Muxed chardev uses trick by calling this function again to replace callback handlers with its own ones, but it doesn't account for such side effect. Fix that using extended version of this function with added argument for disabling side effect and keep original function for compatibility with lots of frontends already using this interface and being "tolerant" to its side effects. One more source of event duplication is just line of code in char-mux.c, which does far more than comment above says (obvious fix). Signed-off-by: Artem Pisarenko Reviewed-by: Marc-André Lureau Message-Id: <7dde6abbd21682857f8294644013173c0b9949b3.1541507990.git.artem.k.pisarenko@gmail.com> Signed-off-by: Marc-André Lureau --- include/chardev/char-fe.h | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) (limited to 'include') diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h index 46c997d..c1b7fd9 100644 --- a/include/chardev/char-fe.h +++ b/include/chardev/char-fe.h @@ -67,7 +67,7 @@ bool qemu_chr_fe_backend_connected(CharBackend *be); bool qemu_chr_fe_backend_open(CharBackend *be); /** - * qemu_chr_fe_set_handlers: + * qemu_chr_fe_set_handlers_full: * @b: a CharBackend * @fd_can_read: callback to get the amount of data the frontend may * receive @@ -79,12 +79,28 @@ bool qemu_chr_fe_backend_open(CharBackend *be); * @context: a main loop context or NULL for the default * @set_open: whether to call qemu_chr_fe_set_open() implicitely when * any of the handler is non-NULL + * @sync_state: whether to issue event callback with updated state * * Set the front end char handlers. The front end takes the focus if * any of the handler is non-NULL. * * Without associated Chardev, nothing is changed. */ +void qemu_chr_fe_set_handlers_full(CharBackend *b, + IOCanReadHandler *fd_can_read, + IOReadHandler *fd_read, + IOEventHandler *fd_event, + BackendChangeHandler *be_change, + void *opaque, + GMainContext *context, + bool set_open, + bool sync_state); + +/** + * qemu_chr_fe_set_handlers: + * + * Version of qemu_chr_fe_set_handlers_full() with sync_state = true. + */ void qemu_chr_fe_set_handlers(CharBackend *b, IOCanReadHandler *fd_can_read, IOReadHandler *fd_read, -- cgit v1.1 From dbb44504c28683a4428308f72b9f61733fcda007 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Mon, 11 Feb 2019 18:24:28 +0000 Subject: io: add qio_task_wait_thread to join with a background thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the ability for a caller to wait for completion of the background thread to synchronously dispatch its result, without needing to wait for the main loop to run the idle callback. This method needs very careful usage to avoid a dangerous race condition with the free'ing of the task. The completion callback is normally invoked from an idle callback registered with the main loop context. The qio_task_wait_thread method must only be called if the completion callback has not yet run. The only safe way to achieve this is to run the qio_task_wait_thread method from the thread that executes the main loop. It is generally a bad idea to use this method since it will block execution of the main loop, however, the design of the character devices and its usage from vhostuser already requires blocking execution. Signed-off-by: Daniel P. Berrangé Message-Id: <20190211182442.8542-3-berrange@redhat.com> Signed-off-by: Marc-André Lureau --- include/io/task.h | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) (limited to 'include') diff --git a/include/io/task.h b/include/io/task.h index 9e09b95..57d8ba8 100644 --- a/include/io/task.h +++ b/include/io/task.h @@ -232,7 +232,8 @@ QIOTask *qio_task_new(Object *source, * * Run a task in a background thread. When @worker * returns it will call qio_task_complete() in - * the event thread context that provided. + * the thread that is running the main loop associated + * with @context. */ void qio_task_run_in_thread(QIOTask *task, QIOTaskWorker worker, @@ -240,6 +241,32 @@ void qio_task_run_in_thread(QIOTask *task, GDestroyNotify destroy, GMainContext *context); + +/** + * qio_task_wait_thread: + * @task: the task struct + * + * Wait for completion of a task that was previously + * invoked using qio_task_run_in_thread. This MUST + * ONLY be invoked if the task has not already + * completed, since after the completion callback + * is invoked, @task will have been freed. + * + * To avoid racing with execution of the completion + * callback provided with qio_task_new, this method + * MUST ONLY be invoked from the thread that is + * running the main loop associated with @context + * parameter to qio_task_run_in_thread. + * + * When the thread has completed, the completion + * callback provided to qio_task_new will be invoked. + * When that callback returns @task will be freed, + * so @task must not be referenced after this + * method completes. + */ +void qio_task_wait_thread(QIOTask *task); + + /** * qio_task_complete: * @task: the task struct -- cgit v1.1 From 4ad6f6cb149b03ab9399a63208918cde09248294 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 13 Feb 2019 14:18:13 +0100 Subject: char: allow specifying a GMainContext at opening time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will be needed by vhost-user-test, when each test switches to its own GMainLoop and GMainContext. Otherwise, for a reconnecting socket the initial connection will happen on the default GMainContext, and no one will be listening on it. Signed-off-by: Paolo Bonzini Reviewed-by: Daniel P. Berrangé Message-Id: <20190202110834.24880-1-pbonzini@redhat.com> Signed-off-by: Marc-André Lureau --- include/chardev/char.h | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'include') diff --git a/include/chardev/char.h b/include/chardev/char.h index 014566c..c0b57f7 100644 --- a/include/chardev/char.h +++ b/include/chardev/char.h @@ -73,6 +73,7 @@ struct Chardev { /** * qemu_chr_new_from_opts: * @opts: see qemu-config.c for a list of valid options + * @context: the #GMainContext to be used at initialization time * * Create a new character backend from a QemuOpts list. * @@ -81,6 +82,7 @@ struct Chardev { * or left untouched in case of help option */ Chardev *qemu_chr_new_from_opts(QemuOpts *opts, + GMainContext *context, Error **errp); /** @@ -106,25 +108,29 @@ ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts, * qemu_chr_new: * @label: the name of the backend * @filename: the URI + * @context: the #GMainContext to be used at initialization time * * Create a new character backend from a URI. * Do not implicitly initialize a monitor if the chardev is muxed. * * Returns: a new character backend */ -Chardev *qemu_chr_new(const char *label, const char *filename); +Chardev *qemu_chr_new(const char *label, const char *filename, + GMainContext *context); /** * qemu_chr_new_mux_mon: * @label: the name of the backend * @filename: the URI + * @context: the #GMainContext to be used at initialization time * * Create a new character backend from a URI. * Implicitly initialize a monitor if the chardev is muxed. * * Returns: a new character backend */ -Chardev *qemu_chr_new_mux_mon(const char *label, const char *filename); +Chardev *qemu_chr_new_mux_mon(const char *label, const char *filename, + GMainContext *context); /** * qemu_chr_change: @@ -146,6 +152,7 @@ void qemu_chr_cleanup(void); * @label: the name of the backend * @filename: the URI * @permit_mux_mon: if chardev is muxed, initialize a monitor + * @context: the #GMainContext to be used at initialization time * * Create a new character backend from a URI. * Character device communications are not written @@ -154,7 +161,7 @@ void qemu_chr_cleanup(void); * Returns: a new character backend */ Chardev *qemu_chr_new_noreplay(const char *label, const char *filename, - bool permit_mux_mon); + bool permit_mux_mon, GMainContext *context); /** * qemu_chr_be_can_write: @@ -272,7 +279,8 @@ typedef struct ChardevClass { } ChardevClass; Chardev *qemu_chardev_new(const char *id, const char *typename, - ChardevBackend *backend, Error **errp); + ChardevBackend *backend, GMainContext *context, + Error **errp); extern int term_escape_char; -- cgit v1.1 From 3d9e232240f9d029d7255b5b11d3a2f61c53d0d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 6 Feb 2019 18:43:23 +0100 Subject: char: update the mux handlers in class callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of handling mux chardev in a special way in qemu_chr_fe_set_handlers(), we may use the chr_update_read_handler class callback instead. Signed-off-by: Marc-André Lureau Message-Id: <20190206174328.9736-2-marcandre.lureau@redhat.com> Reviewed-by: Paolo Bonzini --- include/chardev/char-mux.h | 1 - 1 file changed, 1 deletion(-) (limited to 'include') diff --git a/include/chardev/char-mux.h b/include/chardev/char-mux.h index 1e13187..572cefd 100644 --- a/include/chardev/char-mux.h +++ b/include/chardev/char-mux.h @@ -55,7 +55,6 @@ typedef struct MuxChardev { #define CHARDEV_IS_MUX(chr) \ object_dynamic_cast(OBJECT(chr), TYPE_CHARDEV_MUX) -void mux_chr_set_handlers(Chardev *chr, GMainContext *context); void mux_set_focus(Chardev *chr, int focus); void mux_chr_send_all_event(Chardev *chr, int event); -- cgit v1.1 From 64c3f266dd14d62886b4a51f6bfce49ac14620fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 6 Feb 2019 18:43:26 +0100 Subject: chardev: add a note about frontend sources and context switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Marc-André Lureau Message-Id: <20190206174328.9736-5-marcandre.lureau@redhat.com> Reviewed-by: Paolo Bonzini --- include/chardev/char-fe.h | 3 +++ 1 file changed, 3 insertions(+) (limited to 'include') diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h index c1b7fd9..aa1b864 100644 --- a/include/chardev/char-fe.h +++ b/include/chardev/char-fe.h @@ -184,6 +184,9 @@ void qemu_chr_fe_printf(CharBackend *be, const char *fmt, ...) * is active; return the #GSource's tag. If it is disconnected, * or without associated Chardev, return 0. * + * Note that you are responsible to update the front-end sources if + * you are switching the main context with qemu_chr_fe_set_handlers(). + * * Returns: the source tag */ guint qemu_chr_fe_add_watch(CharBackend *be, GIOCondition cond, -- cgit v1.1