Re: [PATCH] spice: simplify chardev setup

[Gerd Hoffmann]

On Wed, Jul 22, 2020 at 12:19:43PM +0200, Christophe de Dinechin wrote:
> 
> On 2020-07-22 at 11:20 CEST, Christophe de Dinechin wrote...
> > On 2020-07-22 at 10:49 CEST, Gerd Hoffmann wrote...
> >> Initialize spice before chardevs.  That allows to register the spice
> >> chardevs directly in the init function and removes the need to maintain
> >> a linked list of chardevs just for registration.
> >>
> >> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> >
> > Looks good to me, but I still need to test how this integrates with my work
> > on putting SPICE in a module.
> 
> That part does not seem to work so well. It looks like with this move, the
> init happens before the module is loaded:
> 
>    qemu-system-x86_64 -display spice-app
>    Unexpected error in qemu_chr_open_spice_port() at 
> ui/../chardev/spice.c:307:
>    qemu-system-x86_64: spice not enabled
> 
> I'll try to find the correct fix. Any idea how to address this?

move chardev init from early to normal:

commit 77297a71e6be60997caf2c55c09ce01a8c492bc2
Author: Gerd Hoffmann <kraxel@redhat.com>
Date:   Wed Jul 22 13:17:28 2020 +0200

    [fixup] spice app

diff --git a/ui/spice-app.c b/ui/spice-app.c
index 40fb2ef57399..03d971b15f0c 100644
--- a/ui/spice-app.c
+++ b/ui/spice-app.c
@@ -117,7 +117,6 @@ static void spice_app_atexit(void)
 static void spice_app_display_early_init(DisplayOptions *opts)
 {
     QemuOpts *qopts;
-    ChardevBackend *be = chr_spice_backend_new();
     GError *err = NULL;
 
     if (opts->has_full_screen) {
@@ -162,6 +161,15 @@ static void spice_app_display_early_init(DisplayOptions 
*opts)
     qemu_opt_set(qopts, "gl", opts->has_gl ? "on" : "off", &error_abort);
     display_opengl = opts->has_gl;
 #endif
+}
+
+static void spice_app_display_init(DisplayState *ds, DisplayOptions *opts)
+{
+    ChardevBackend *be = chr_spice_backend_new();
+    QemuOpts *qopts;
+    GError *err = NULL;
+    gchar *uri;
+
     be->u.spiceport.data->fqdn = g_strdup("org.qemu.monitor.qmp.0");
     qemu_chardev_new("org.qemu.monitor.qmp", TYPE_CHARDEV_SPICEPORT,
                      be, NULL, &error_abort);
@@ -171,13 +179,6 @@ static void spice_app_display_early_init(DisplayOptions 
*opts)
     qemu_opt_set(qopts, "mode", "control", &error_abort);
 
     qapi_free_ChardevBackend(be);
-}
-
-static void spice_app_display_init(DisplayState *ds, DisplayOptions *opts)
-{
-    GError *err = NULL;
-    gchar *uri;
-
     uri = g_strjoin("", "spice+unix://", app_dir, "/", "spice.sock", NULL);
     info_report("Launching display with URI: %s", uri);
     g_app_info_launch_default_for_uri(uri, NULL, &err);