[Qemu-devel] [PATCH 5/9] chardev: introduce chr_machine_done hook

Peter Xu posted 9 patches 7 years, 7 months ago
There is a newer version of this series
[Qemu-devel] [PATCH 5/9] chardev: introduce chr_machine_done hook
Posted by Peter Xu 7 years, 7 months ago
Introduce ChardevClass.chr_machine_done() hook so that chardevs can run
customized procedures after machine init.

There was an existing mux user already that did similar thing but used a
raw machine done notifier.  Generalize it into a framework, and let the
mux chardevs provide such a class-specific hook to achieve the same
thing.  Then we can move the mux related code to the char-mux.c file.

Since at it, replace the mux_realized variable with the global
machine_init_done varible.

This notifier framework will be further leverged by other type of
chardevs soon.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 chardev/char-mux.c     | 33 +++++++++++++++++++++++++++++----
 chardev/char.c         | 43 +++++++++++++++++--------------------------
 include/chardev/char.h |  2 ++
 3 files changed, 48 insertions(+), 30 deletions(-)

diff --git a/chardev/char-mux.c b/chardev/char-mux.c
index d48e78103a..1b925c8dec 100644
--- a/chardev/char-mux.c
+++ b/chardev/char-mux.c
@@ -27,6 +27,7 @@
 #include "qemu/option.h"
 #include "chardev/char.h"
 #include "sysemu/block-backend.h"
+#include "sysemu/sysemu.h"
 #include "chardev/char-mux.h"
 
 /* MUX driver for serial I/O splitting */
@@ -230,14 +231,12 @@ static void mux_chr_read(void *opaque, const uint8_t *buf, int size)
         }
 }
 
-bool muxes_realized;
-
 void mux_chr_send_all_event(Chardev *chr, int event)
 {
     MuxChardev *d = MUX_CHARDEV(chr);
     int i;
 
-    if (!muxes_realized) {
+    if (!machine_init_done) {
         return;
     }
 
@@ -327,7 +326,7 @@ static void qemu_chr_open_mux(Chardev *chr,
     /* only default to opened state if we've realized the initial
      * set of muxes
      */
-    *be_opened = muxes_realized;
+    *be_opened = machine_init_done;
     qemu_chr_fe_init(&d->chr, drv, errp);
 }
 
@@ -347,6 +346,31 @@ static void qemu_chr_parse_mux(QemuOpts *opts, ChardevBackend *backend,
     mux->chardev = g_strdup(chardev);
 }
 
+/**
+ * Called after processing of default and command-line-specified
+ * chardevs to deliver CHR_EVENT_OPENED events to any FEs attached
+ * to a mux chardev. This is done here to ensure that
+ * output/prompts/banners are only displayed for the FE that has
+ * focus when initial command-line processing/machine init is
+ * completed.
+ *
+ * After this point, any new FE attached to any new or existing
+ * mux will receive CHR_EVENT_OPENED notifications for the BE
+ * immediately.
+ */
+static int open_muxes(Chardev *chr)
+{
+    /* send OPENED to all already-attached FEs */
+    mux_chr_send_all_event(chr, CHR_EVENT_OPENED);
+    /*
+     * mark mux as OPENED so any new FEs will immediately receive
+     * OPENED event
+     */
+    qemu_chr_be_event(chr, CHR_EVENT_OPENED);
+
+    return 0;
+}
+
 static void char_mux_class_init(ObjectClass *oc, void *data)
 {
     ChardevClass *cc = CHARDEV_CLASS(oc);
@@ -357,6 +381,7 @@ static void char_mux_class_init(ObjectClass *oc, void *data)
     cc->chr_accept_input = mux_chr_accept_input;
     cc->chr_add_watch = mux_chr_add_watch;
     cc->chr_be_event = mux_chr_be_event;
+    cc->chr_machine_done = open_muxes;
 }
 
 static const TypeInfo char_mux_type_info = {
diff --git a/chardev/char.c b/chardev/char.c
index 01d979a1da..fda820863c 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -281,40 +281,31 @@ static const TypeInfo char_type_info = {
     .class_init = char_class_init,
 };
 
-/**
- * Called after processing of default and command-line-specified
- * chardevs to deliver CHR_EVENT_OPENED events to any FEs attached
- * to a mux chardev. This is done here to ensure that
- * output/prompts/banners are only displayed for the FE that has
- * focus when initial command-line processing/machine init is
- * completed.
- *
- * After this point, any new FE attached to any new or existing
- * mux will receive CHR_EVENT_OPENED notifications for the BE
- * immediately.
- */
-static int open_muxes(Object *child, void *opaque)
+static int chardev_machine_done_notify_one(Object *child, void *opaque)
 {
-    if (CHARDEV_IS_MUX(child)) {
-        /* send OPENED to all already-attached FEs */
-        mux_chr_send_all_event(CHARDEV(child), CHR_EVENT_OPENED);
-        /* mark mux as OPENED so any new FEs will immediately receive
-         * OPENED event
-         */
-        qemu_chr_be_event(CHARDEV(child), CHR_EVENT_OPENED);
+    Chardev *chr = (Chardev *)child;
+    ChardevClass *class = CHARDEV_GET_CLASS(chr);
+
+    if (class->chr_machine_done) {
+        return class->chr_machine_done(chr);
     }
 
     return 0;
 }
 
-static void muxes_realize_done(Notifier *notifier, void *unused)
+static void chardev_machine_done_hook(Notifier *notifier, void *unused)
 {
-    muxes_realized = true;
-    object_child_foreach(get_chardevs_root(), open_muxes, NULL);
+    int ret = object_child_foreach(get_chardevs_root(),
+                                   chardev_machine_done_notify_one, NULL);
+
+    if (ret) {
+        error_report("Failed to call chardev machine_done hooks");
+        exit(1);
+    }
 }
 
-static Notifier muxes_realize_notify = {
-    .notify = muxes_realize_done,
+static Notifier chardev_machine_done_notify = {
+    .notify = chardev_machine_done_hook,
 };
 
 static bool qemu_chr_is_busy(Chardev *s)
@@ -1118,7 +1109,7 @@ static void register_types(void)
      * as part of realize functions like serial_isa_realizefn when -nographic
      * is specified
      */
-    qemu_add_machine_init_done_notifier(&muxes_realize_notify);
+    qemu_add_machine_init_done_notifier(&chardev_machine_done_notify);
 }
 
 type_init(register_types);
diff --git a/include/chardev/char.h b/include/chardev/char.h
index a381dc3df8..1cb1f4763f 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -247,6 +247,8 @@ typedef struct ChardevClass {
     void (*chr_set_echo)(Chardev *chr, bool echo);
     void (*chr_set_fe_open)(Chardev *chr, int fe_open);
     void (*chr_be_event)(Chardev *s, int event);
+    /* Return 0 if succeeded, 1 if failed */
+    int (*chr_machine_done)(Chardev *chr);
 } ChardevClass;
 
 Chardev *qemu_chardev_new(const char *id, const char *typename,
-- 
2.14.3


Re: [Qemu-devel] [PATCH 5/9] chardev: introduce chr_machine_done hook
Posted by Marc-André Lureau 7 years, 7 months ago
Hi

On Mon, Mar 5, 2018 at 7:50 AM, Peter Xu <peterx@redhat.com> wrote:
> Introduce ChardevClass.chr_machine_done() hook so that chardevs can run
> customized procedures after machine init.
>
> There was an existing mux user already that did similar thing but used a
> raw machine done notifier.  Generalize it into a framework, and let the
> mux chardevs provide such a class-specific hook to achieve the same
> thing.  Then we can move the mux related code to the char-mux.c file.
>
> Since at it, replace the mux_realized variable with the global
> machine_init_done varible.
>
> This notifier framework will be further leverged by other type of
> chardevs soon.
>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>

patchew caught that:
tests/test-char.o: In function `char_mux_test':
/tmp/qemu-test/src/tests/test-char.c:169: undefined reference to
`muxes_realized'


> ---
>  chardev/char-mux.c     | 33 +++++++++++++++++++++++++++++----
>  chardev/char.c         | 43 +++++++++++++++++--------------------------
>  include/chardev/char.h |  2 ++
>  3 files changed, 48 insertions(+), 30 deletions(-)
>
> diff --git a/chardev/char-mux.c b/chardev/char-mux.c
> index d48e78103a..1b925c8dec 100644
> --- a/chardev/char-mux.c
> +++ b/chardev/char-mux.c
> @@ -27,6 +27,7 @@
>  #include "qemu/option.h"
>  #include "chardev/char.h"
>  #include "sysemu/block-backend.h"
> +#include "sysemu/sysemu.h"
>  #include "chardev/char-mux.h"
>
>  /* MUX driver for serial I/O splitting */
> @@ -230,14 +231,12 @@ static void mux_chr_read(void *opaque, const uint8_t *buf, int size)
>          }
>  }
>
> -bool muxes_realized;
> -
>  void mux_chr_send_all_event(Chardev *chr, int event)
>  {
>      MuxChardev *d = MUX_CHARDEV(chr);
>      int i;
>
> -    if (!muxes_realized) {
> +    if (!machine_init_done) {
>          return;
>      }
>
> @@ -327,7 +326,7 @@ static void qemu_chr_open_mux(Chardev *chr,
>      /* only default to opened state if we've realized the initial
>       * set of muxes
>       */
> -    *be_opened = muxes_realized;
> +    *be_opened = machine_init_done;
>      qemu_chr_fe_init(&d->chr, drv, errp);
>  }
>
> @@ -347,6 +346,31 @@ static void qemu_chr_parse_mux(QemuOpts *opts, ChardevBackend *backend,
>      mux->chardev = g_strdup(chardev);
>  }
>
> +/**
> + * Called after processing of default and command-line-specified
> + * chardevs to deliver CHR_EVENT_OPENED events to any FEs attached
> + * to a mux chardev. This is done here to ensure that
> + * output/prompts/banners are only displayed for the FE that has
> + * focus when initial command-line processing/machine init is
> + * completed.
> + *
> + * After this point, any new FE attached to any new or existing
> + * mux will receive CHR_EVENT_OPENED notifications for the BE
> + * immediately.
> + */
> +static int open_muxes(Chardev *chr)
> +{
> +    /* send OPENED to all already-attached FEs */
> +    mux_chr_send_all_event(chr, CHR_EVENT_OPENED);
> +    /*
> +     * mark mux as OPENED so any new FEs will immediately receive
> +     * OPENED event
> +     */
> +    qemu_chr_be_event(chr, CHR_EVENT_OPENED);
> +
> +    return 0;
> +}
> +
>  static void char_mux_class_init(ObjectClass *oc, void *data)
>  {
>      ChardevClass *cc = CHARDEV_CLASS(oc);
> @@ -357,6 +381,7 @@ static void char_mux_class_init(ObjectClass *oc, void *data)
>      cc->chr_accept_input = mux_chr_accept_input;
>      cc->chr_add_watch = mux_chr_add_watch;
>      cc->chr_be_event = mux_chr_be_event;
> +    cc->chr_machine_done = open_muxes;
>  }
>
>  static const TypeInfo char_mux_type_info = {
> diff --git a/chardev/char.c b/chardev/char.c
> index 01d979a1da..fda820863c 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -281,40 +281,31 @@ static const TypeInfo char_type_info = {
>      .class_init = char_class_init,
>  };
>
> -/**
> - * Called after processing of default and command-line-specified
> - * chardevs to deliver CHR_EVENT_OPENED events to any FEs attached
> - * to a mux chardev. This is done here to ensure that
> - * output/prompts/banners are only displayed for the FE that has
> - * focus when initial command-line processing/machine init is
> - * completed.
> - *
> - * After this point, any new FE attached to any new or existing
> - * mux will receive CHR_EVENT_OPENED notifications for the BE
> - * immediately.
> - */
> -static int open_muxes(Object *child, void *opaque)
> +static int chardev_machine_done_notify_one(Object *child, void *opaque)
>  {
> -    if (CHARDEV_IS_MUX(child)) {
> -        /* send OPENED to all already-attached FEs */
> -        mux_chr_send_all_event(CHARDEV(child), CHR_EVENT_OPENED);
> -        /* mark mux as OPENED so any new FEs will immediately receive
> -         * OPENED event
> -         */
> -        qemu_chr_be_event(CHARDEV(child), CHR_EVENT_OPENED);
> +    Chardev *chr = (Chardev *)child;
> +    ChardevClass *class = CHARDEV_GET_CLASS(chr);
> +
> +    if (class->chr_machine_done) {
> +        return class->chr_machine_done(chr);
>      }
>
>      return 0;
>  }
>
> -static void muxes_realize_done(Notifier *notifier, void *unused)
> +static void chardev_machine_done_hook(Notifier *notifier, void *unused)
>  {
> -    muxes_realized = true;
> -    object_child_foreach(get_chardevs_root(), open_muxes, NULL);
> +    int ret = object_child_foreach(get_chardevs_root(),
> +                                   chardev_machine_done_notify_one, NULL);
> +
> +    if (ret) {
> +        error_report("Failed to call chardev machine_done hooks");
> +        exit(1);
> +    }
>  }
>
> -static Notifier muxes_realize_notify = {
> -    .notify = muxes_realize_done,
> +static Notifier chardev_machine_done_notify = {
> +    .notify = chardev_machine_done_hook,
>  };
>
>  static bool qemu_chr_is_busy(Chardev *s)
> @@ -1118,7 +1109,7 @@ static void register_types(void)
>       * as part of realize functions like serial_isa_realizefn when -nographic
>       * is specified
>       */
> -    qemu_add_machine_init_done_notifier(&muxes_realize_notify);
> +    qemu_add_machine_init_done_notifier(&chardev_machine_done_notify);
>  }
>
>  type_init(register_types);
> diff --git a/include/chardev/char.h b/include/chardev/char.h
> index a381dc3df8..1cb1f4763f 100644
> --- a/include/chardev/char.h
> +++ b/include/chardev/char.h
> @@ -247,6 +247,8 @@ typedef struct ChardevClass {
>      void (*chr_set_echo)(Chardev *chr, bool echo);
>      void (*chr_set_fe_open)(Chardev *chr, int fe_open);
>      void (*chr_be_event)(Chardev *s, int event);
> +    /* Return 0 if succeeded, 1 if failed */
> +    int (*chr_machine_done)(Chardev *chr);
>  } ChardevClass;
>
>  Chardev *qemu_chardev_new(const char *id, const char *typename,
> --
> 2.14.3
>

Re: [Qemu-devel] [PATCH 5/9] chardev: introduce chr_machine_done hook
Posted by Peter Xu 7 years, 7 months ago
On Mon, Mar 05, 2018 at 11:54:22AM +0100, Marc-André Lureau wrote:
> Hi
> 
> On Mon, Mar 5, 2018 at 7:50 AM, Peter Xu <peterx@redhat.com> wrote:
> > Introduce ChardevClass.chr_machine_done() hook so that chardevs can run
> > customized procedures after machine init.
> >
> > There was an existing mux user already that did similar thing but used a
> > raw machine done notifier.  Generalize it into a framework, and let the
> > mux chardevs provide such a class-specific hook to achieve the same
> > thing.  Then we can move the mux related code to the char-mux.c file.
> >
> > Since at it, replace the mux_realized variable with the global
> > machine_init_done varible.
> >
> > This notifier framework will be further leverged by other type of
> > chardevs soon.
> >
> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> 
> patchew caught that:
> tests/test-char.o: In function `char_mux_test':
> /tmp/qemu-test/src/tests/test-char.c:169: undefined reference to
> `muxes_realized'

Yeh, I replied to patchew mail with a fix to be squashed.

I was planning to repost some days later, but let me repost now in
case that it may ease the reviewers a bit.  Thanks,

-- 
Peter Xu