From 501093207eb1ed4845e0a65ee1ce7db7b9676e0b Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 23 Sep 2020 11:12:17 +0200 Subject: [PATCH 1/8] module: silence errors for module_load_qom_all(). Add mayfail bool parameter to module loading functions. Set it to true for module_load_qom_all() because device modules might not load into all system emulation variants. qemu-system-s390x for example will not load qxl because it lacks vga support. Makes "make check" less chatty. Drop module_loaded_qom_all check in module_load_qom_one to make sure we see errors for explicit load requests, i.e. module_load_qom_one("qxl") failing will log an error no matter whenever module_load_qom_all() was called before or not. Signed-off-by: Gerd Hoffmann Acked-by: Paolo Bonzini Message-id: 20200923091217.22662-1-kraxel@redhat.com --- include/qemu/module.h | 8 ++++---- softmmu/qtest.c | 2 +- util/module.c | 20 ++++++++++---------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/include/qemu/module.h b/include/qemu/module.h index 9121a475c1..944d403cbd 100644 --- a/include/qemu/module.h +++ b/include/qemu/module.h @@ -61,15 +61,15 @@ typedef enum { #define fuzz_target_init(function) module_init(function, \ MODULE_INIT_FUZZ_TARGET) #define migration_init(function) module_init(function, MODULE_INIT_MIGRATION) -#define block_module_load_one(lib) module_load_one("block-", lib) -#define ui_module_load_one(lib) module_load_one("ui-", lib) -#define audio_module_load_one(lib) module_load_one("audio-", lib) +#define block_module_load_one(lib) module_load_one("block-", lib, false) +#define ui_module_load_one(lib) module_load_one("ui-", lib, false) +#define audio_module_load_one(lib) module_load_one("audio-", lib, false) void register_module_init(void (*fn)(void), module_init_type type); void register_dso_module_init(void (*fn)(void), module_init_type type); void module_call_init(module_init_type type); -bool module_load_one(const char *prefix, const char *lib_name); +bool module_load_one(const char *prefix, const char *lib_name, bool mayfail); void module_load_qom_one(const char *type); void module_load_qom_all(void); diff --git a/softmmu/qtest.c b/softmmu/qtest.c index 2c6e8dc858..7965dc9a16 100644 --- a/softmmu/qtest.c +++ b/softmmu/qtest.c @@ -757,7 +757,7 @@ static void qtest_process_command(CharBackend *chr, gchar **words) g_assert(words[1] && words[2]); qtest_send_prefix(chr); - if (module_load_one(words[1], words[2])) { + if (module_load_one(words[1], words[2], false)) { qtest_sendf(chr, "OK\n"); } else { qtest_sendf(chr, "FAIL\n"); diff --git a/util/module.c b/util/module.c index a44ec38d93..836fd444e1 100644 --- a/util/module.c +++ b/util/module.c @@ -110,7 +110,7 @@ void module_call_init(module_init_type type) } #ifdef CONFIG_MODULES -static int module_load_file(const char *fname) +static int module_load_file(const char *fname, bool mayfail) { GModule *g_module; void (*sym)(void); @@ -134,8 +134,10 @@ static int module_load_file(const char *fname) g_module = g_module_open(fname, G_MODULE_BIND_LAZY | G_MODULE_BIND_LOCAL); if (!g_module) { - fprintf(stderr, "Failed to open module: %s\n", - g_module_error()); + if (!mayfail) { + fprintf(stderr, "Failed to open module: %s\n", + g_module_error()); + } ret = -EINVAL; goto out; } @@ -167,7 +169,7 @@ out: } #endif -bool module_load_one(const char *prefix, const char *lib_name) +bool module_load_one(const char *prefix, const char *lib_name, bool mayfail) { bool success = false; @@ -218,7 +220,7 @@ bool module_load_one(const char *prefix, const char *lib_name) for (i = 0; i < n_dirs; i++) { fname = g_strdup_printf("%s/%s%s", dirs[i], module_name, CONFIG_HOST_DSOSUF); - ret = module_load_file(fname); + ret = module_load_file(fname, mayfail); g_free(fname); fname = NULL; /* Try loading until loaded a module file */ @@ -275,13 +277,11 @@ void module_load_qom_one(const char *type) if (!type) { return; } - if (module_loaded_qom_all) { - return; - } for (i = 0; i < ARRAY_SIZE(qom_modules); i++) { if (strcmp(qom_modules[i].type, type) == 0) { module_load_one(qom_modules[i].prefix, - qom_modules[i].module); + qom_modules[i].module, + false); return; } } @@ -302,7 +302,7 @@ void module_load_qom_all(void) /* one module implementing multiple types -> load only once */ continue; } - module_load_one(qom_modules[i].prefix, qom_modules[i].module); + module_load_one(qom_modules[i].prefix, qom_modules[i].module, true); } module_loaded_qom_all = true; } From f88908cf3014bb028fc5ad33e32aa3065f0c2715 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 23 Sep 2020 12:37:28 +0200 Subject: [PATCH 2/8] modules: update qom object module comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gerd Hoffmann Reviewed-by: Philippe Mathieu-Daudé Message-id: 20200923103728.12026-1-kraxel@redhat.com --- util/module.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/util/module.c b/util/module.c index 836fd444e1..4349607ad1 100644 --- a/util/module.c +++ b/util/module.c @@ -250,8 +250,10 @@ bool module_load_one(const char *prefix, const char *lib_name, bool mayfail) * only a very few devices & objects. * * So with the expectation that this will be rather the exception than - * to rule and the list will not gain that many entries go with a + * the rule and the list will not gain that many entries, go with a * simple manually maintained list for now. + * + * The list must be sorted by module (module_load_qom_all() needs this). */ static struct { const char *type; From e220cf866267fbca3dae16f68ec01b67a4beb805 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 14 Oct 2020 14:11:15 +0200 Subject: [PATCH 3/8] ui/spice-app: don't use qemu_chr_open_spice_port directly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Save the parent object's open function pointer in the (new) VCChardevClass struct instead before overwriting it, so we can look it up when needed. Signed-off-by: Gerd Hoffmann Reviewed-by: Philippe Mathieu-Daudé Message-id: 20201014121120.13482-3-kraxel@redhat.com --- ui/spice-app.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/ui/spice-app.c b/ui/spice-app.c index 93e105c6ee..7e0550c79f 100644 --- a/ui/spice-app.c +++ b/ui/spice-app.c @@ -44,11 +44,15 @@ static char *sock_path; struct VCChardev { SpiceChardev parent; }; -typedef struct VCChardev VCChardev; + +struct VCChardevClass { + ChardevClass parent; + void (*parent_open)(Chardev *chr, ChardevBackend *backend, + bool *be_opened, Error **errp); +}; #define TYPE_CHARDEV_VC "chardev-vc" -DECLARE_INSTANCE_CHECKER(VCChardev, VC_CHARDEV, - TYPE_CHARDEV_VC) +OBJECT_DECLARE_TYPE(VCChardev, VCChardevClass, CHARDEV_VC) static ChardevBackend * chr_spice_backend_new(void) @@ -66,6 +70,7 @@ static void vc_chr_open(Chardev *chr, bool *be_opened, Error **errp) { + VCChardevClass *vc = CHARDEV_VC_GET_CLASS(chr); ChardevBackend *be; const char *fqdn = NULL; @@ -80,7 +85,7 @@ static void vc_chr_open(Chardev *chr, be = chr_spice_backend_new(); be->u.spiceport.data->fqdn = fqdn ? g_strdup(fqdn) : g_strdup_printf("org.qemu.console.%s", chr->label); - qemu_chr_open_spice_port(chr, be, be_opened, errp); + vc->parent_open(chr, be, be_opened, errp); qapi_free_ChardevBackend(be); } @@ -91,8 +96,11 @@ static void vc_chr_set_echo(Chardev *chr, bool echo) static void char_vc_class_init(ObjectClass *oc, void *data) { + VCChardevClass *vc = CHARDEV_VC_CLASS(oc); ChardevClass *cc = CHARDEV_CLASS(oc); + vc->parent_open = cc->open; + cc->parse = qemu_chr_parse_vc; cc->open = vc_chr_open; cc->chr_set_echo = vc_chr_set_echo; @@ -103,6 +111,7 @@ static const TypeInfo char_vc_type_info = { .parent = TYPE_CHARDEV_SPICEPORT, .instance_size = sizeof(VCChardev), .class_init = char_vc_class_init, + .class_size = sizeof(VCChardevClass), }; static void spice_app_atexit(void) From 70122d62302c97bcd205956a544b8e79f2a4a50f Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 14 Oct 2020 14:11:16 +0200 Subject: [PATCH 4/8] chardev/spice: make qemu_chr_open_spice_port static MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gerd Hoffmann Reviewed-by: Philippe Mathieu-Daudé Message-id: 20201014121120.13482-4-kraxel@redhat.com --- chardev/spice.c | 8 ++++---- include/chardev/spice.h | 3 --- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/chardev/spice.c b/chardev/spice.c index bf7ea1e294..051c23a84f 100644 --- a/chardev/spice.c +++ b/chardev/spice.c @@ -296,10 +296,10 @@ static void qemu_chr_open_spice_vmc(Chardev *chr, chr_open(chr, type); } -void qemu_chr_open_spice_port(Chardev *chr, - ChardevBackend *backend, - bool *be_opened, - Error **errp) +static void qemu_chr_open_spice_port(Chardev *chr, + ChardevBackend *backend, + bool *be_opened, + Error **errp) { ChardevSpicePort *spiceport = backend->u.spiceport.data; const char *name = spiceport->fqdn; diff --git a/include/chardev/spice.h b/include/chardev/spice.h index 99f26aedde..1115502cdf 100644 --- a/include/chardev/spice.h +++ b/include/chardev/spice.h @@ -24,7 +24,4 @@ typedef struct SpiceChardev SpiceChardev; DECLARE_INSTANCE_CHECKER(SpiceChardev, SPICE_CHARDEV, TYPE_CHARDEV_SPICE) -void qemu_chr_open_spice_port(Chardev *chr, ChardevBackend *backend, - bool *be_opened, Error **errp); - #endif From 93ab5844b2cd5367966d7b5bae154e0d3303b504 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 14 Oct 2020 14:11:17 +0200 Subject: [PATCH 5/8] chardev/spice: simplify chardev setup 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 Message-id: 20201014121120.13482-5-kraxel@redhat.com --- chardev/spice.c | 29 ++++++----------------------- include/chardev/spice.h | 1 - include/ui/qemu-spice.h | 1 - softmmu/vl.c | 9 +++++---- ui/spice-app.c | 17 +++++++++-------- ui/spice-core.c | 2 -- 6 files changed, 20 insertions(+), 39 deletions(-) diff --git a/chardev/spice.c b/chardev/spice.c index 051c23a84f..7d1fb17718 100644 --- a/chardev/spice.c +++ b/chardev/spice.c @@ -14,9 +14,6 @@ typedef struct SpiceCharSource { SpiceChardev *scd; } SpiceCharSource; -static QLIST_HEAD(, SpiceChardev) spice_chars = - QLIST_HEAD_INITIALIZER(spice_chars); - static int vmc_write(SpiceCharDeviceInstance *sin, const uint8_t *buf, int len) { SpiceChardev *scd = container_of(sin, SpiceChardev, sin); @@ -216,8 +213,6 @@ static void char_spice_finalize(Object *obj) vmc_unregister_interface(s); - QLIST_SAFE_REMOVE(s, next); - g_free((char *)s->sin.subtype); g_free((char *)s->sin.portname); } @@ -256,8 +251,6 @@ static void chr_open(Chardev *chr, const char *subtype) s->active = false; s->sin.subtype = g_strdup(subtype); - - QLIST_INSERT_HEAD(&spice_chars, s, next); } static void qemu_chr_open_spice_vmc(Chardev *chr, @@ -310,28 +303,18 @@ static void qemu_chr_open_spice_port(Chardev *chr, return; } + if (!using_spice) { + error_setg(errp, "spice not enabled"); + return; + } + chr_open(chr, "port"); *be_opened = false; s = SPICE_CHARDEV(chr); s->sin.portname = g_strdup(name); - if (using_spice) { - /* spice server already created */ - vmc_register_interface(s); - } -} - -void qemu_spice_register_ports(void) -{ - SpiceChardev *s; - - QLIST_FOREACH(s, &spice_chars, next) { - if (s->sin.portname == NULL) { - continue; - } - vmc_register_interface(s); - } + vmc_register_interface(s); } static void qemu_chr_parse_spice_vmc(QemuOpts *opts, ChardevBackend *backend, diff --git a/include/chardev/spice.h b/include/chardev/spice.h index 1115502cdf..58e5b727e9 100644 --- a/include/chardev/spice.h +++ b/include/chardev/spice.h @@ -13,7 +13,6 @@ struct SpiceChardev { bool blocked; const uint8_t *datapos; int datalen; - QLIST_ENTRY(SpiceChardev) next; }; typedef struct SpiceChardev SpiceChardev; diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h index 12474d88f4..0e8ec3f0d7 100644 --- a/include/ui/qemu-spice.h +++ b/include/ui/qemu-spice.h @@ -45,7 +45,6 @@ int qemu_spice_migrate_info(const char *hostname, int port, int tls_port, #else #define SPICE_NEEDS_SET_MM_TIME 0 #endif -void qemu_spice_register_ports(void); #else /* CONFIG_SPICE */ diff --git a/softmmu/vl.c b/softmmu/vl.c index 254ee5e525..cb476aa70b 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -4148,6 +4148,11 @@ void qemu_init(int argc, char **argv, char **envp) user_creatable_add_opts_foreach, object_create_initial, &error_fatal); + /* spice needs the timers to be initialized by this point */ + /* spice must initialize before audio as it changes the default auiodev */ + /* spice must initialize before chardevs (for spicevmc and spiceport) */ + qemu_spice_init(); + qemu_opts_foreach(qemu_find_opts("chardev"), chardev_init_func, NULL, &error_fatal); @@ -4156,10 +4161,6 @@ void qemu_init(int argc, char **argv, char **envp) fsdev_init_func, NULL, &error_fatal); #endif - /* spice needs the timers to be initialized by this point */ - /* spice must initialize before audio as it changes the default auiodev */ - qemu_spice_init(); - /* * Note: we need to create audio and block backends before * machine_set_property(), so machine properties can refer to diff --git a/ui/spice-app.c b/ui/spice-app.c index 7e0550c79f..026124ef56 100644 --- a/ui/spice-app.c +++ b/ui/spice-app.c @@ -129,7 +129,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) { @@ -174,6 +173,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); @@ -183,13 +191,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); diff --git a/ui/spice-core.c b/ui/spice-core.c index 10aa309f78..47700b2200 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -812,8 +812,6 @@ void qemu_spice_init(void) g_free(x509_cert_file); g_free(x509_cacert_file); - qemu_spice_register_ports(); - #ifdef HAVE_SPICE_GL if (qemu_opt_get_bool(opts, "gl", 0)) { if ((port != 0) || (tls_port != 0)) { From d72c34cccc5955eeeb393b174fd6a0794b6d823f Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 14 Oct 2020 14:11:18 +0200 Subject: [PATCH 6/8] meson: add spice_headers dependency. Used for files which (with CONFIG_SPICE=y) depend on spice header files to pick up some enum, but which do not depend on on the actual spice shared library. Signed-off-by: Gerd Hoffmann Message-id: 20201014121120.13482-6-kraxel@redhat.com --- audio/meson.build | 2 +- meson.build | 2 ++ monitor/meson.build | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/audio/meson.build b/audio/meson.build index 18a831129e..7d53b0f920 100644 --- a/audio/meson.build +++ b/audio/meson.build @@ -1,5 +1,5 @@ +softmmu_ss.add([spice_headers, files('audio.c')]) softmmu_ss.add(files( - 'audio.c', 'audio_legacy.c', 'mixeng.c', 'noaudio.c', diff --git a/meson.build b/meson.build index 1a4a482492..2c6169fab0 100644 --- a/meson.build +++ b/meson.build @@ -321,9 +321,11 @@ if 'CONFIG_LIBJACK' in config_host jack = declare_dependency(link_args: config_host['JACK_LIBS'].split()) endif spice = not_found +spice_headers = not_found if 'CONFIG_SPICE' in config_host spice = declare_dependency(compile_args: config_host['SPICE_CFLAGS'].split(), link_args: config_host['SPICE_LIBS'].split()) + spice_headers = declare_dependency(compile_args: config_host['SPICE_CFLAGS'].split()) endif rt = cc.find_library('rt', required: false) libdl = not_found diff --git a/monitor/meson.build b/monitor/meson.build index eb2a534fdc..6d00985ace 100644 --- a/monitor/meson.build +++ b/monitor/meson.build @@ -3,7 +3,7 @@ qmp_ss.add(files('monitor.c', 'qmp.c', 'qmp-cmds-control.c')) softmmu_ss.add(files( 'hmp-cmds.c', 'hmp.c', - 'qmp-cmds.c', )) +softmmu_ss.add([spice_headers, files('qmp-cmds.c')]) specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: [files('misc.c'), spice]) From fa264418acff6507b666b7dc987c4a731f84d710 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 14 Oct 2020 14:11:19 +0200 Subject: [PATCH 7/8] meson: add spice dependency to core spice source files. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Right now it happens to work by pure luck because the spice chardevs add the spice dependency to the softmmu source set. That'll change though once we start building spice chardevs as module, so lets fix it properly. Signed-off-by: Gerd Hoffmann Reviewed-by: Philippe Mathieu-Daudé Message-id: 20201014121120.13482-7-kraxel@redhat.com --- ui/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/meson.build b/ui/meson.build index 78ad792ffb..6ce8148678 100644 --- a/ui/meson.build +++ b/ui/meson.build @@ -14,7 +14,7 @@ softmmu_ss.add(files( )) softmmu_ss.add(when: 'CONFIG_LINUX', if_true: files('input-linux.c')) -softmmu_ss.add(when: 'CONFIG_SPICE', if_true: files('spice-core.c', 'spice-input.c', 'spice-display.c')) +softmmu_ss.add(when: [spice, 'CONFIG_SPICE'], if_true: files('spice-core.c', 'spice-input.c', 'spice-display.c')) softmmu_ss.add(when: cocoa, if_true: files('cocoa.m')) vnc_ss = ss.source_set() From 23ebeaae4eb09a0d92dc7f22b41e5dd08485c390 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 14 Oct 2020 14:11:20 +0200 Subject: [PATCH 8/8] chardev/spice: build spice chardevs as module Signed-off-by: Gerd Hoffmann Message-id: 20201014121120.13482-8-kraxel@redhat.com --- chardev/meson.build | 7 ++++++- util/module.c | 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/chardev/meson.build b/chardev/meson.build index dd2699a11b..859d8b04d4 100644 --- a/chardev/meson.build +++ b/chardev/meson.build @@ -26,7 +26,6 @@ chardev_ss.add(when: 'CONFIG_WIN32', if_true: files( chardev_ss = chardev_ss.apply(config_host, strict: false) softmmu_ss.add(files('chardev-sysemu.c', 'msmouse.c', 'wctablet.c', 'testdev.c')) -softmmu_ss.add(when: ['CONFIG_SPICE', spice], if_true: files('spice.c')) chardev_modules = {} @@ -36,4 +35,10 @@ if config_host.has_key('CONFIG_BRLAPI') chardev_modules += { 'baum': module_ss } endif +if config_host.has_key('CONFIG_SPICE') + module_ss = ss.source_set() + module_ss.add(when: [spice], if_true: files('spice.c')) + chardev_modules += { 'spice': module_ss } +endif + modules += { 'chardev': chardev_modules } diff --git a/util/module.c b/util/module.c index 4349607ad1..f0ed05fbd0 100644 --- a/util/module.c +++ b/util/module.c @@ -268,6 +268,8 @@ static struct { { "virtio-gpu-device", "hw-", "display-virtio-gpu" }, { "vhost-user-gpu", "hw-", "display-virtio-gpu" }, { "chardev-braille", "chardev-", "baum" }, + { "chardev-spicevmc", "chardev-", "spice" }, + { "chardev-spiceport", "chardev-", "spice" }, }; static bool module_loaded_qom_all;