* [PATCH 1/7] spice: simplify chardev setup
2020-07-23 17:46 [PATCH 0/7] Make SPICE a load module Christophe de Dinechin
@ 2020-07-23 17:46 ` Christophe de Dinechin
2020-07-23 17:46 ` [PATCH 2/7] build: fix device module builds Christophe de Dinechin
` (5 subsequent siblings)
6 siblings, 0 replies; 17+ messages in thread
From: Christophe de Dinechin @ 2020-07-23 17:46 UTC (permalink / raw)
To: qemu-devel, kraxel
Cc: Eduardo Habkost, Michael S. Tsirkin, Dr. David Alan Gilbert,
Markus Armbruster, Paolo Bonzini, Cleber Rosa,
Marc-André Lureau, Richard Henderson
From: Gerd Hoffmann <kraxel@redhat.com>
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>
Signed-off-by: Christophe de Dinechin <dinechin@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 bf7ea1e294..9733f06716 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 @@ 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 1f7339b649..2013255f34 100644
--- a/include/chardev/spice.h
+++ b/include/chardev/spice.h
@@ -12,7 +12,6 @@ typedef struct SpiceChardev {
bool blocked;
const uint8_t *datapos;
int datalen;
- QLIST_ENTRY(SpiceChardev) next;
} SpiceChardev;
#define TYPE_CHARDEV_SPICE "chardev-spice"
diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index 8c23dfe717..d34cea2e0f 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -46,7 +46,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 f476ef89ed..bc0dcc4f58 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -4125,6 +4125,11 @@ void qemu_init(int argc, char **argv, char **envp)
page_size_init();
socket_init();
+ /* 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("object"),
user_creatable_add_opts_foreach,
object_create_initial, &error_fatal);
@@ -4139,10 +4144,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 40fb2ef573..03d971b15f 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);
diff --git a/ui/spice-core.c b/ui/spice-core.c
index ecc2ec2c55..37dd68f2ab 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -813,8 +813,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)) {
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 2/7] build: fix device module builds
2020-07-23 17:46 [PATCH 0/7] Make SPICE a load module Christophe de Dinechin
2020-07-23 17:46 ` [PATCH 1/7] spice: simplify chardev setup Christophe de Dinechin
@ 2020-07-23 17:46 ` Christophe de Dinechin
2020-07-28 16:37 ` Philippe Mathieu-Daudé
2020-07-23 17:46 ` [PATCH 3/7] minikconf: Pass variables for modules Christophe de Dinechin
` (4 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Christophe de Dinechin @ 2020-07-23 17:46 UTC (permalink / raw)
To: qemu-devel, kraxel
Cc: Eduardo Habkost, Michael S. Tsirkin, Dr. David Alan Gilbert,
Markus Armbruster, Paolo Bonzini, Cleber Rosa,
Marc-André Lureau, Richard Henderson
From: Gerd Hoffmann <kraxel@redhat.com>
See comment. Feels quite hackish. Better ideas anyone?
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
---
dtc | 2 +-
roms/SLOF | 2 +-
roms/openbios | 2 +-
roms/opensbi | 2 +-
roms/seabios | 2 +-
slirp | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/dtc b/dtc
index 85e5d83984..88f18909db 160000
--- a/dtc
+++ b/dtc
@@ -1 +1 @@
-Subproject commit 85e5d839847af54efab170f2b1331b2a6421e647
+Subproject commit 88f18909db731a627456f26d779445f84e449536
diff --git a/roms/SLOF b/roms/SLOF
index e18ddad851..9546892a80 160000
--- a/roms/SLOF
+++ b/roms/SLOF
@@ -1 +1 @@
-Subproject commit e18ddad8516ff2cfe36ec130200318f7251aa78c
+Subproject commit 9546892a80d5a4c73deea6719de46372f007f4a6
diff --git a/roms/openbios b/roms/openbios
index 75fbb41d28..7e5b89e429 160000
--- a/roms/openbios
+++ b/roms/openbios
@@ -1 +1 @@
-Subproject commit 75fbb41d2857d93208c74a8e0228b29fd7bf04c0
+Subproject commit 7e5b89e4295063d8eba55b9c8ce8bc681c2d129a
diff --git a/roms/opensbi b/roms/opensbi
index 9f1b72ce66..be92da280d 160000
--- a/roms/opensbi
+++ b/roms/opensbi
@@ -1 +1 @@
-Subproject commit 9f1b72ce66d659e91013b358939e832fb27223f5
+Subproject commit be92da280d87c38a2e0adc5d3f43bab7b5468f09
diff --git a/roms/seabios b/roms/seabios
index 88ab0c1552..f21b5a4aeb 160000
--- a/roms/seabios
+++ b/roms/seabios
@@ -1 +1 @@
-Subproject commit 88ab0c15525ced2eefe39220742efe4769089ad8
+Subproject commit f21b5a4aeb020f2a5e2c6503f906a9349dd2f069
diff --git a/slirp b/slirp
index 2faae0f778..126c04acba 160000
--- a/slirp
+++ b/slirp
@@ -1 +1 @@
-Subproject commit 2faae0f778f818fadc873308f983289df697eb93
+Subproject commit 126c04acbabd7ad32c2b018fe10dfac2a3bc1210
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 2/7] build: fix device module builds
2020-07-23 17:46 ` [PATCH 2/7] build: fix device module builds Christophe de Dinechin
@ 2020-07-28 16:37 ` Philippe Mathieu-Daudé
2020-07-29 16:01 ` Christophe de Dinechin
0 siblings, 1 reply; 17+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-28 16:37 UTC (permalink / raw)
To: Christophe de Dinechin, qemu-devel, kraxel
Cc: Eduardo Habkost, Michael S. Tsirkin, Markus Armbruster,
Dr. David Alan Gilbert, Marc-André Lureau, Cleber Rosa,
Paolo Bonzini, Richard Henderson
On 7/23/20 7:46 PM, Christophe de Dinechin wrote:
> From: Gerd Hoffmann <kraxel@redhat.com>
>
> See comment. Feels quite hackish. Better ideas anyone?
I don't understand this patch, how is it related to the rest of
your series?
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
> ---
> dtc | 2 +-
> roms/SLOF | 2 +-
> roms/openbios | 2 +-
> roms/opensbi | 2 +-
> roms/seabios | 2 +-
> slirp | 2 +-
> 6 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/dtc b/dtc
> index 85e5d83984..88f18909db 160000
> --- a/dtc
> +++ b/dtc
> @@ -1 +1 @@
> -Subproject commit 85e5d839847af54efab170f2b1331b2a6421e647
> +Subproject commit 88f18909db731a627456f26d779445f84e449536
> diff --git a/roms/SLOF b/roms/SLOF
> index e18ddad851..9546892a80 160000
> --- a/roms/SLOF
> +++ b/roms/SLOF
> @@ -1 +1 @@
> -Subproject commit e18ddad8516ff2cfe36ec130200318f7251aa78c
> +Subproject commit 9546892a80d5a4c73deea6719de46372f007f4a6
> diff --git a/roms/openbios b/roms/openbios
> index 75fbb41d28..7e5b89e429 160000
> --- a/roms/openbios
> +++ b/roms/openbios
> @@ -1 +1 @@
> -Subproject commit 75fbb41d2857d93208c74a8e0228b29fd7bf04c0
> +Subproject commit 7e5b89e4295063d8eba55b9c8ce8bc681c2d129a
> diff --git a/roms/opensbi b/roms/opensbi
> index 9f1b72ce66..be92da280d 160000
> --- a/roms/opensbi
> +++ b/roms/opensbi
> @@ -1 +1 @@
> -Subproject commit 9f1b72ce66d659e91013b358939e832fb27223f5
> +Subproject commit be92da280d87c38a2e0adc5d3f43bab7b5468f09
> diff --git a/roms/seabios b/roms/seabios
> index 88ab0c1552..f21b5a4aeb 160000
> --- a/roms/seabios
> +++ b/roms/seabios
> @@ -1 +1 @@
> -Subproject commit 88ab0c15525ced2eefe39220742efe4769089ad8
> +Subproject commit f21b5a4aeb020f2a5e2c6503f906a9349dd2f069
> diff --git a/slirp b/slirp
> index 2faae0f778..126c04acba 160000
> --- a/slirp
> +++ b/slirp
> @@ -1 +1 @@
> -Subproject commit 2faae0f778f818fadc873308f983289df697eb93
> +Subproject commit 126c04acbabd7ad32c2b018fe10dfac2a3bc1210
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/7] build: fix device module builds
2020-07-28 16:37 ` Philippe Mathieu-Daudé
@ 2020-07-29 16:01 ` Christophe de Dinechin
0 siblings, 0 replies; 17+ messages in thread
From: Christophe de Dinechin @ 2020-07-29 16:01 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel,
Markus Armbruster, Dr. David Alan Gilbert,
Marc-André Lureau, kraxel, Cleber Rosa, Paolo Bonzini,
Richard Henderson
On 2020-07-28 at 18:37 CEST, Philippe Mathieu-Daudé wrote...
> On 7/23/20 7:46 PM, Christophe de Dinechin wrote:
>> From: Gerd Hoffmann <kraxel@redhat.com>
>>
>> See comment. Feels quite hackish. Better ideas anyone?
>
> I don't understand this patch, how is it related to the rest of
> your series?
It's a leftover from an earlier workaround. For some reason, only some
spurious submodule changes remained over time. That patch can now be
dropped, I believe.
>
>>
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
>> ---
>> dtc | 2 +-
>> roms/SLOF | 2 +-
>> roms/openbios | 2 +-
>> roms/opensbi | 2 +-
>> roms/seabios | 2 +-
>> slirp | 2 +-
>> 6 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/dtc b/dtc
>> index 85e5d83984..88f18909db 160000
>> --- a/dtc
>> +++ b/dtc
>> @@ -1 +1 @@
>> -Subproject commit 85e5d839847af54efab170f2b1331b2a6421e647
>> +Subproject commit 88f18909db731a627456f26d779445f84e449536
>> diff --git a/roms/SLOF b/roms/SLOF
>> index e18ddad851..9546892a80 160000
>> --- a/roms/SLOF
>> +++ b/roms/SLOF
>> @@ -1 +1 @@
>> -Subproject commit e18ddad8516ff2cfe36ec130200318f7251aa78c
>> +Subproject commit 9546892a80d5a4c73deea6719de46372f007f4a6
>> diff --git a/roms/openbios b/roms/openbios
>> index 75fbb41d28..7e5b89e429 160000
>> --- a/roms/openbios
>> +++ b/roms/openbios
>> @@ -1 +1 @@
>> -Subproject commit 75fbb41d2857d93208c74a8e0228b29fd7bf04c0
>> +Subproject commit 7e5b89e4295063d8eba55b9c8ce8bc681c2d129a
>> diff --git a/roms/opensbi b/roms/opensbi
>> index 9f1b72ce66..be92da280d 160000
>> --- a/roms/opensbi
>> +++ b/roms/opensbi
>> @@ -1 +1 @@
>> -Subproject commit 9f1b72ce66d659e91013b358939e832fb27223f5
>> +Subproject commit be92da280d87c38a2e0adc5d3f43bab7b5468f09
>> diff --git a/roms/seabios b/roms/seabios
>> index 88ab0c1552..f21b5a4aeb 160000
>> --- a/roms/seabios
>> +++ b/roms/seabios
>> @@ -1 +1 @@
>> -Subproject commit 88ab0c15525ced2eefe39220742efe4769089ad8
>> +Subproject commit f21b5a4aeb020f2a5e2c6503f906a9349dd2f069
>> diff --git a/slirp b/slirp
>> index 2faae0f778..126c04acba 160000
>> --- a/slirp
>> +++ b/slirp
>> @@ -1 +1 @@
>> -Subproject commit 2faae0f778f818fadc873308f983289df697eb93
>> +Subproject commit 126c04acbabd7ad32c2b018fe10dfac2a3bc1210
>>
--
Cheers,
Christophe de Dinechin (IRC c3d)
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/7] minikconf: Pass variables for modules
2020-07-23 17:46 [PATCH 0/7] Make SPICE a load module Christophe de Dinechin
2020-07-23 17:46 ` [PATCH 1/7] spice: simplify chardev setup Christophe de Dinechin
2020-07-23 17:46 ` [PATCH 2/7] build: fix device module builds Christophe de Dinechin
@ 2020-07-23 17:46 ` Christophe de Dinechin
2020-08-04 6:19 ` Gerd Hoffmann
2020-07-23 17:46 ` [PATCH 4/7] spice: Make spice a module configuration Christophe de Dinechin
` (3 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Christophe de Dinechin @ 2020-07-23 17:46 UTC (permalink / raw)
To: qemu-devel, kraxel
Cc: Eduardo Habkost, Michael S. Tsirkin, Dr. David Alan Gilbert,
Markus Armbruster, Paolo Bonzini, Cleber Rosa,
Marc-André Lureau, Richard Henderson
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
---
scripts/minikconf.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/minikconf.py b/scripts/minikconf.py
index bcd91015d3..d60add97f6 100755
--- a/scripts/minikconf.py
+++ b/scripts/minikconf.py
@@ -690,10 +690,10 @@ if __name__ == '__main__':
parser = KconfigParser(data)
external_vars = set()
for arg in argv[3:]:
- m = re.match(r'^(CONFIG_[A-Z0-9_]+)=([yn]?)$', arg)
+ m = re.match(r'^(CONFIG_[A-Z0-9_]+)=([ymn]?)$', arg)
if m is not None:
name, value = m.groups()
- parser.do_assignment(name, value == 'y')
+ parser.do_assignment(name, value == 'y' or value == 'm')
external_vars.add(name[7:])
else:
fp = open(arg, 'rt', encoding='utf-8')
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 3/7] minikconf: Pass variables for modules
2020-07-23 17:46 ` [PATCH 3/7] minikconf: Pass variables for modules Christophe de Dinechin
@ 2020-08-04 6:19 ` Gerd Hoffmann
0 siblings, 0 replies; 17+ messages in thread
From: Gerd Hoffmann @ 2020-08-04 6:19 UTC (permalink / raw)
To: Christophe de Dinechin
Cc: Eduardo Habkost, Michael S. Tsirkin, Markus Armbruster,
Dr. David Alan Gilbert, qemu-devel, Paolo Bonzini, Cleber Rosa,
Marc-André Lureau, Richard Henderson
On Thu, Jul 23, 2020 at 07:46:11PM +0200, Christophe de Dinechin wrote:
> Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
> ---
> scripts/minikconf.py | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/minikconf.py b/scripts/minikconf.py
> index bcd91015d3..d60add97f6 100755
> --- a/scripts/minikconf.py
> +++ b/scripts/minikconf.py
> @@ -690,10 +690,10 @@ if __name__ == '__main__':
> parser = KconfigParser(data)
> external_vars = set()
> for arg in argv[3:]:
> - m = re.match(r'^(CONFIG_[A-Z0-9_]+)=([yn]?)$', arg)
> + m = re.match(r'^(CONFIG_[A-Z0-9_]+)=([ymn]?)$', arg)
> if m is not None:
> name, value = m.groups()
> - parser.do_assignment(name, value == 'y')
> + parser.do_assignment(name, value == 'y' or value == 'm')
> external_vars.add(name[7:])
> else:
> fp = open(arg, 'rt', encoding='utf-8')
Hmm, I somehow doubt it is _that_ simple, miniconf supports only bool
(y+n) not tristate (y+n+m) ...
take care,
Gerd
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/7] spice: Make spice a module configuration
2020-07-23 17:46 [PATCH 0/7] Make SPICE a load module Christophe de Dinechin
` (2 preceding siblings ...)
2020-07-23 17:46 ` [PATCH 3/7] minikconf: Pass variables for modules Christophe de Dinechin
@ 2020-07-23 17:46 ` Christophe de Dinechin
2020-08-04 6:26 ` Gerd Hoffmann
2020-07-23 17:46 ` [PATCH 5/7] spice: Move all the spice-related code in spice-app.so Christophe de Dinechin
` (2 subsequent siblings)
6 siblings, 1 reply; 17+ messages in thread
From: Christophe de Dinechin @ 2020-07-23 17:46 UTC (permalink / raw)
To: qemu-devel, kraxel
Cc: Eduardo Habkost, Michael S. Tsirkin, Dr. David Alan Gilbert,
Markus Armbruster, Paolo Bonzini, Cleber Rosa,
Marc-André Lureau, Richard Henderson
This commit changes the spice configuration 'm' by default, and moves
the spice components to obj-m variables. It is sufficient to build
without modules enable, but does not link correctly yet, since no
shims have been created for the missing functions yet.
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
---
chardev/Makefile.objs | 3 ++-
configure | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/chardev/Makefile.objs b/chardev/Makefile.objs
index 3783dadc4c..7cf05c9541 100644
--- a/chardev/Makefile.objs
+++ b/chardev/Makefile.objs
@@ -26,4 +26,5 @@ baum.o-cflags := $(SDL_CFLAGS)
baum.o-libs := $(BRLAPI_LIBS)
endif
-common-obj-$(CONFIG_SPICE) += spice.o
+common-obj-$(CONFIG_SPICE) += spice.mo
+spice.mo-objs := spice.o
diff --git a/configure b/configure
index 4bd80ed507..054aab31be 100755
--- a/configure
+++ b/configure
@@ -7534,7 +7534,7 @@ if test "$posix_memalign" = "yes" ; then
fi
if test "$spice" = "yes" ; then
- echo "CONFIG_SPICE=y" >> $config_host_mak
+ echo "CONFIG_SPICE=m" >> $config_host_mak
fi
if test "$smartcard" = "yes" ; then
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 4/7] spice: Make spice a module configuration
2020-07-23 17:46 ` [PATCH 4/7] spice: Make spice a module configuration Christophe de Dinechin
@ 2020-08-04 6:26 ` Gerd Hoffmann
0 siblings, 0 replies; 17+ messages in thread
From: Gerd Hoffmann @ 2020-08-04 6:26 UTC (permalink / raw)
To: Christophe de Dinechin
Cc: Eduardo Habkost, Michael S. Tsirkin, Markus Armbruster,
Dr. David Alan Gilbert, qemu-devel, Paolo Bonzini, Cleber Rosa,
Marc-André Lureau, Richard Henderson
On Thu, Jul 23, 2020 at 07:46:12PM +0200, Christophe de Dinechin wrote:
> This commit changes the spice configuration 'm' by default, and moves
> the spice components to obj-m variables. It is sufficient to build
> without modules enable, but does not link correctly yet, since no
> shims have been created for the missing functions yet.
So it breaks the modular build? Don't do that, it is bad for bisecting.
I guess this patch should simply come later in the patch series.
I can see that it is useful to have it here for development, so you
easily find all the places which need a fixup, but for merging the
patches should be reordered.
take care,
Gerd
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/7] spice: Move all the spice-related code in spice-app.so
2020-07-23 17:46 [PATCH 0/7] Make SPICE a load module Christophe de Dinechin
` (3 preceding siblings ...)
2020-07-23 17:46 ` [PATCH 4/7] spice: Make spice a module configuration Christophe de Dinechin
@ 2020-07-23 17:46 ` Christophe de Dinechin
2020-08-04 10:05 ` Gerd Hoffmann
2020-07-23 17:46 ` [PATCH 6/7] build: Add SPICE_CFLAGS and SPICE_LIBS to relevant files Christophe de Dinechin
2020-07-23 17:46 ` [PATCH 7/7] spice: Call qemu spice functions indirectly Christophe de Dinechin
6 siblings, 1 reply; 17+ messages in thread
From: Christophe de Dinechin @ 2020-07-23 17:46 UTC (permalink / raw)
To: qemu-devel, kraxel
Cc: Eduardo Habkost, Michael S. Tsirkin, Dr. David Alan Gilbert,
Markus Armbruster, Paolo Bonzini, Cleber Rosa,
Marc-André Lureau, Richard Henderson
If we want to build spice as a separately loadable module, we need to
put all the spice code in one loadable module, because the build
system does not know how to deal with dependencies yet.
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
---
audio/Makefile.objs | 2 +-
chardev/Makefile.objs | 3 +--
ui/Makefile.objs | 8 ++++----
3 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/audio/Makefile.objs b/audio/Makefile.objs
index b4a4c11f31..298c895ff5 100644
--- a/audio/Makefile.objs
+++ b/audio/Makefile.objs
@@ -1,5 +1,5 @@
common-obj-y = audio.o audio_legacy.o noaudio.o wavaudio.o mixeng.o
-common-obj-$(CONFIG_SPICE) += spiceaudio.o
+spice-app.mo-objs += ../audio/spiceaudio.o
common-obj-$(CONFIG_AUDIO_COREAUDIO) += coreaudio.o
common-obj-$(CONFIG_AUDIO_DSOUND) += dsoundaudio.o
common-obj-$(CONFIG_AUDIO_WIN_INT) += audio_win_int.o
diff --git a/chardev/Makefile.objs b/chardev/Makefile.objs
index 7cf05c9541..2add4319a9 100644
--- a/chardev/Makefile.objs
+++ b/chardev/Makefile.objs
@@ -26,5 +26,4 @@ baum.o-cflags := $(SDL_CFLAGS)
baum.o-libs := $(BRLAPI_LIBS)
endif
-common-obj-$(CONFIG_SPICE) += spice.mo
-spice.mo-objs := spice.o
+spice-app.mo-objs += ../chardev/spice.o
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index 504b196479..1ab515e23d 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -11,7 +11,6 @@ common-obj-y += keymaps.o console.o cursor.o qemu-pixman.o
common-obj-y += input.o input-keymap.o input-legacy.o kbd-state.o
common-obj-y += input-barrier.o
common-obj-$(CONFIG_LINUX) += input-linux.o
-common-obj-$(CONFIG_SPICE) += spice-core.o spice-input.o spice-display.o
common-obj-$(CONFIG_COCOA) += cocoa.o
common-obj-$(CONFIG_VNC) += $(vnc-obj-y)
common-obj-$(call lnot,$(CONFIG_VNC)) += vnc-stubs.o
@@ -53,10 +52,11 @@ curses.mo-objs := curses.o
curses.mo-cflags := $(CURSES_CFLAGS) $(ICONV_CFLAGS)
curses.mo-libs := $(CURSES_LIBS) $(ICONV_LIBS)
-ifeq ($(CONFIG_GIO)$(CONFIG_SPICE),yy)
-common-obj-$(if $(CONFIG_MODULES),m,y) += spice-app.mo
+common-obj-$(CONFIG_SPICE) += spice-app.mo
+spice-app.mo-objs += spice-core.o spice-input.o spice-display.o
+ifeq ($(CONFIG_GIO)$(CONFIG_SPICE),ym)
+spice-app.mo-objs += spice-app.o
endif
-spice-app.mo-objs := spice-app.o
spice-app.mo-cflags := $(GIO_CFLAGS)
spice-app.mo-libs := $(GIO_LIBS)
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 5/7] spice: Move all the spice-related code in spice-app.so
2020-07-23 17:46 ` [PATCH 5/7] spice: Move all the spice-related code in spice-app.so Christophe de Dinechin
@ 2020-08-04 10:05 ` Gerd Hoffmann
0 siblings, 0 replies; 17+ messages in thread
From: Gerd Hoffmann @ 2020-08-04 10:05 UTC (permalink / raw)
To: Christophe de Dinechin
Cc: Eduardo Habkost, Michael S. Tsirkin, Markus Armbruster,
Dr. David Alan Gilbert, qemu-devel, Paolo Bonzini, Cleber Rosa,
Marc-André Lureau, Richard Henderson
On Thu, Jul 23, 2020 at 07:46:13PM +0200, Christophe de Dinechin wrote:
> If we want to build spice as a separately loadable module, we need to
> put all the spice code in one loadable module, because the build
> system does not know how to deal with dependencies yet.
I'd tend to think teaching qemu about module dependencies will work
better.
I've digged around in g_module_open() docs a bit. There seems to be no
way to set a shared library search path, which makes it tricky to simply
depend on the dynamic linker because features like loading modules from
the build directory when qemu is started from the build directory will
not work as expected then.
So qemu has do deal with it, in the simplest case just a static
dependency list compiled in which is checked on every module load.
Symbols from other modules are available if they are loaded without
G_MODULE_BIND_LOCAL.
That will make module autoloading work better. Right now it works fine
with -display spice-app. It doesn't work with a plain -spice. And
"-audiodev spice,id=test" will not load the spice module either.
I think we should have:
(1) A audio-spice.so module (autoload will work, by naming
convention for audio modules).
(2) A chardev-spice.so module (autoload needs two lines in qom_modules
in util/module.c, one for each chardev).
(3) A ui/spice-app.so module (autoload works by naming convention).
(4) A ui/spice-core.so module which all other spice modules depend on.
qemu must explicitly load that one in case it finds -spice on the
command line.
take care,
Gerd
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 6/7] build: Add SPICE_CFLAGS and SPICE_LIBS to relevant files
2020-07-23 17:46 [PATCH 0/7] Make SPICE a load module Christophe de Dinechin
` (4 preceding siblings ...)
2020-07-23 17:46 ` [PATCH 5/7] spice: Move all the spice-related code in spice-app.so Christophe de Dinechin
@ 2020-07-23 17:46 ` Christophe de Dinechin
2020-08-04 10:06 ` Gerd Hoffmann
2020-07-23 17:46 ` [PATCH 7/7] spice: Call qemu spice functions indirectly Christophe de Dinechin
6 siblings, 1 reply; 17+ messages in thread
From: Christophe de Dinechin @ 2020-07-23 17:46 UTC (permalink / raw)
To: qemu-devel, kraxel
Cc: Eduardo Habkost, Michael S. Tsirkin, Dr. David Alan Gilbert,
Markus Armbruster, Paolo Bonzini, Cleber Rosa,
Marc-André Lureau, Richard Henderson
Instead of adding the spice build flags to the top-level build
options, add them where they are necessary. This is a step to move the
burden of linking with spice libraries away from the top-level qemu.
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
---
configure | 4 ++--
hw/display/Makefile.objs | 1 +
hw/i386/pc.c | 1 -
monitor/Makefile.objs | 3 +++
softmmu/Makefile.objs | 2 +-
ui/Makefile.objs | 4 ++--
6 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/configure b/configure
index 054aab31be..86fc03699a 100755
--- a/configure
+++ b/configure
@@ -5223,8 +5223,6 @@ EOF
$pkg_config --atleast-version=0.12.3 spice-protocol && \
compile_prog "$spice_cflags" "$spice_libs" ; then
spice="yes"
- libs_softmmu="$libs_softmmu $spice_libs"
- QEMU_CFLAGS="$QEMU_CFLAGS $spice_cflags"
spice_protocol_version=$($pkg_config --modversion spice-protocol)
spice_server_version=$($pkg_config --modversion spice-server)
else
@@ -7535,6 +7533,8 @@ fi
if test "$spice" = "yes" ; then
echo "CONFIG_SPICE=m" >> $config_host_mak
+ echo "SPICE_CFLAGS=$spice_cflags" >> $config_host_mak
+ echo "SPICE_LIBS=$spice_libs" >> $config_host_mak
fi
if test "$smartcard" = "yes" ; then
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index d619594ad4..3963fd1dcd 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -47,6 +47,7 @@ obj-$(CONFIG_VGA) += vga.o
ifeq ($(CONFIG_QXL),y)
common-obj-m += qxl.mo
qxl.mo-objs = qxl.o qxl-logger.o qxl-render.o
+qxl.mo-cflags += $(SPICE_CFLAGS)
endif
common-obj-$(CONFIG_VIRTIO_GPU) += virtio-gpu-base.o virtio-gpu.o virtio-gpu-3d.o
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 3d419d5991..9f28a91df9 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -64,7 +64,6 @@
#include "kvm_i386.h"
#include "hw/xen/xen.h"
#include "hw/xen/start_info.h"
-#include "ui/qemu-spice.h"
#include "exec/memory.h"
#include "exec/address-spaces.h"
#include "sysemu/arch_init.h"
diff --git a/monitor/Makefile.objs b/monitor/Makefile.objs
index a8533c9dd7..fd58d80195 100644
--- a/monitor/Makefile.objs
+++ b/monitor/Makefile.objs
@@ -2,5 +2,8 @@ obj-y += misc.o
common-obj-y += monitor.o qmp.o hmp.o
common-obj-y += qmp-cmds.o qmp-cmds-control.o
common-obj-y += hmp-cmds.o
+qmp-cmds.o-cflags += $(SPICE_CFLAGS)
+hmp-cmds.o-cflags += $(SPICE_CFLAGS)
+misc.o-cflags += $(SPICE_CFLAGS)
storage-daemon-obj-y += monitor.o qmp.o qmp-cmds-control.o
diff --git a/softmmu/Makefile.objs b/softmmu/Makefile.objs
index a414a74c50..4e36ff47a2 100644
--- a/softmmu/Makefile.objs
+++ b/softmmu/Makefile.objs
@@ -11,4 +11,4 @@ obj-y += memory_mapping.o
obj-y += qtest.o
obj-y += vl.o
-vl.o-cflags := $(GPROF_CFLAGS) $(SDL_CFLAGS)
+vl.o-cflags := $(GPROF_CFLAGS) $(SDL_CFLAGS) $(SPICE_CFLAGS)
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index 1ab515e23d..6a6fda2f06 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -57,8 +57,8 @@ spice-app.mo-objs += spice-core.o spice-input.o spice-display.o
ifeq ($(CONFIG_GIO)$(CONFIG_SPICE),ym)
spice-app.mo-objs += spice-app.o
endif
-spice-app.mo-cflags := $(GIO_CFLAGS)
-spice-app.mo-libs := $(GIO_LIBS)
+spice-app.mo-cflags := $(GIO_CFLAGS) $(SPICE_CFLAGS)
+spice-app.mo-libs := $(GIO_LIBS) $(SPICE_LIBS)
common-obj-$(CONFIG_OPENGL) += shader.o
common-obj-$(CONFIG_OPENGL) += console-gl.o
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 6/7] build: Add SPICE_CFLAGS and SPICE_LIBS to relevant files
2020-07-23 17:46 ` [PATCH 6/7] build: Add SPICE_CFLAGS and SPICE_LIBS to relevant files Christophe de Dinechin
@ 2020-08-04 10:06 ` Gerd Hoffmann
0 siblings, 0 replies; 17+ messages in thread
From: Gerd Hoffmann @ 2020-08-04 10:06 UTC (permalink / raw)
To: Christophe de Dinechin
Cc: Eduardo Habkost, Michael S. Tsirkin, Markus Armbruster,
Dr. David Alan Gilbert, qemu-devel, Paolo Bonzini, Cleber Rosa,
Marc-André Lureau, Richard Henderson
On Thu, Jul 23, 2020 at 07:46:14PM +0200, Christophe de Dinechin wrote:
> Instead of adding the spice build flags to the top-level build
> options, add them where they are necessary. This is a step to move the
> burden of linking with spice libraries away from the top-level qemu.
>
> Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
This is a good idea anyway, independent from the modularization effort.
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
take care,
Gerd
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 7/7] spice: Call qemu spice functions indirectly
2020-07-23 17:46 [PATCH 0/7] Make SPICE a load module Christophe de Dinechin
` (5 preceding siblings ...)
2020-07-23 17:46 ` [PATCH 6/7] build: Add SPICE_CFLAGS and SPICE_LIBS to relevant files Christophe de Dinechin
@ 2020-07-23 17:46 ` Christophe de Dinechin
2020-07-27 17:38 ` Dr. David Alan Gilbert
2020-08-04 10:18 ` Gerd Hoffmann
6 siblings, 2 replies; 17+ messages in thread
From: Christophe de Dinechin @ 2020-07-23 17:46 UTC (permalink / raw)
To: qemu-devel, kraxel
Cc: Eduardo Habkost, Michael S. Tsirkin, Dr. David Alan Gilbert,
Markus Armbruster, Paolo Bonzini, Cleber Rosa,
Marc-André Lureau, Richard Henderson
In order to be able to move all of spice in a shared library that is
loaded dynamically only on demand, this commit puts the functions
called by the main binary in a QemuSpiceOps structure, and use static
inline stubs to preserve the call sites.
With these changes, the following shared libraries are no longer
necessary in the top-level qemu binary when building with CONFIG_MDULES:
libspice-server.so.1 => /lib64/libspice-server.so.1 (HEX)
libopus.so.0 => /lib64/libopus.so.0 (HEX)
liblz4.so.1 => /lib64/liblz4.so.1 (HEX)
libgstapp-1.0.so.0 => /lib64/libgstapp-1.0.so.0 (HEX)
libgstvideo-1.0.so.0 => /lib64/libgstvideo-1.0.so.0 (HEX)
libgstbase-1.0.so.0 => /lib64/libgstbase-1.0.so.0 (HEX)
libgstreamer-1.0.so.0 => /lib64/libgstreamer-1.0.so.0 (HEX)
libssl.so.1.1 => /lib64/libssl.so.1.1 (HEX)
liborc-0.4.so.0 => /lib64/liborc-0.4.so.0 (HEX)
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
---
include/ui/qemu-spice.h | 74 +++++++++++++++++++----------------------
monitor/hmp-cmds.c | 13 ++++++++
monitor/misc.c | 2 +-
monitor/qmp-cmds.c | 6 ++--
softmmu/vl.c | 28 ++++++++++++++--
ui/spice-core.c | 33 ++++++++++++++++--
6 files changed, 107 insertions(+), 49 deletions(-)
diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index d34cea2e0f..9721d8817e 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -25,21 +25,28 @@
#include <spice.h>
#include "qemu/config-file.h"
-extern int using_spice;
+#define using_spice (qemu_spice.in_use())
-void qemu_spice_init(void);
void qemu_spice_input_init(void);
void qemu_spice_audio_init(void);
-void qemu_spice_display_init(void);
-int qemu_spice_display_add_client(int csock, int skipauth, int tls);
int qemu_spice_add_interface(SpiceBaseInstance *sin);
bool qemu_spice_have_display_interface(QemuConsole *con);
int qemu_spice_add_display_interface(QXLInstance *qxlin, QemuConsole *con);
+
+#if !defined(CONFIG_MODULES) || defined(BUILD_DSO)
+bool qemu_is_using_spice(void);
+void qemu_start_using_spice(void);
+void qemu_spice_init(void);
+void qemu_spice_display_init(void);
+int qemu_spice_display_add_client(int csock, int skipauth, int tls);
int qemu_spice_set_passwd(const char *passwd,
bool fail_if_connected, bool disconnect_if_connected);
int qemu_spice_set_pw_expire(time_t expires);
int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
const char *subject);
+struct SpiceInfo *qemu_spice_query(Error **errp);
+#endif /* !CONFIG_MODULES || BUILD_DSO */
+
#if !defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06)
#define SPICE_NEEDS_SET_MM_TIME 1
@@ -47,46 +54,33 @@ int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
#define SPICE_NEEDS_SET_MM_TIME 0
#endif
-#else /* CONFIG_SPICE */
-
-#include "qemu/error-report.h"
-
-#define using_spice 0
-#define spice_displays 0
-static inline int qemu_spice_set_passwd(const char *passwd,
- bool fail_if_connected,
- bool disconnect_if_connected)
-{
- return -1;
-}
-static inline int qemu_spice_set_pw_expire(time_t expires)
-{
- return -1;
-}
-static inline int qemu_spice_migrate_info(const char *h, int p, int t,
- const char *s)
-{
- return -1;
-}
+#else /* !CONFIG_SPICE */
-static inline int qemu_spice_display_add_client(int csock, int skipauth,
- int tls)
-{
- return -1;
-}
+#define using_spice 0
-static inline void qemu_spice_display_init(void)
-{
- /* This must never be called if CONFIG_SPICE is disabled */
- error_report("spice support is disabled");
- abort();
-}
+#endif /* CONFIG_SPICE */
-static inline void qemu_spice_init(void)
+/* Module high-level inteface for SPICE */
+struct QemuSpiceOps
{
-}
-
-#endif /* CONFIG_SPICE */
+ bool (*in_use)(void);
+ void (*init)(void);
+ void (*display_init)(void);
+ int (*display_add_client)(int csock, int skipauth, int tls);
+
+ int (*set_passwd)(const char *passwd,
+ bool fail_if_connected,
+ bool disconnect_if_connected);
+ int (*set_pw_expire)(time_t expires);
+ int (*migrate_info)(const char *hostname,
+ int port, int tls_port,
+ const char *subject);
+ struct SpiceInfo * (*query)(Error **errp);
+};
+typedef struct QemuSpiceOps QemuSpiceOps;
+void qemu_spice_ops_register(QemuSpiceOps *ops);
+
+extern struct QemuSpiceOps qemu_spice;
static inline bool qemu_using_spice(Error **errp)
{
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index ae4b6a4246..8e10af188f 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -57,6 +57,7 @@
#ifdef CONFIG_SPICE
#include <spice/enums.h>
+#include "ui/qemu-spice.h"
#endif
void hmp_handle_error(Monitor *mon, Error *err)
@@ -573,6 +574,14 @@ void hmp_info_vnc(Monitor *mon, const QDict *qdict)
#endif
#ifdef CONFIG_SPICE
+SpiceInfo *qmp_query_spice(Error **errp)
+{
+ if (!using_spice) {
+ return NULL;
+ }
+ return qemu_spice.query(errp);
+}
+
void hmp_info_spice(Monitor *mon, const QDict *qdict)
{
SpiceChannelList *chan;
@@ -599,6 +608,10 @@ void hmp_info_spice(Monitor *mon, const QDict *qdict)
info = qmp_query_spice(NULL);
+ if (!info) {
+ monitor_printf(mon, "Server: not started\n");
+ goto out;
+ }
if (!info->enabled) {
monitor_printf(mon, "Server: disabled\n");
goto out;
diff --git a/monitor/misc.c b/monitor/misc.c
index e847b58a8c..88f8915734 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -443,7 +443,7 @@ void qmp_client_migrate_info(const char *protocol, const char *hostname,
return;
}
- if (qemu_spice_migrate_info(hostname,
+ if (qemu_spice.migrate_info(hostname,
has_port ? port : -1,
has_tls_port ? tls_port : -1,
cert_subject)) {
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 864cbfa32e..9f9822b3f3 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -196,7 +196,7 @@ void qmp_set_password(const char *protocol, const char *password,
if (!qemu_using_spice(errp)) {
return;
}
- rc = qemu_spice_set_passwd(password, fail_if_connected,
+ rc = qemu_spice.set_passwd(password, fail_if_connected,
disconnect_if_connected);
if (rc != 0) {
error_setg(errp, QERR_SET_PASSWD_FAILED);
@@ -242,7 +242,7 @@ void qmp_expire_password(const char *protocol, const char *whenstr,
if (!qemu_using_spice(errp)) {
return;
}
- rc = qemu_spice_set_pw_expire(when);
+ rc = qemu_spice.set_pw_expire(when);
if (rc != 0) {
error_setg(errp, QERR_SET_PASSWD_FAILED);
}
@@ -339,7 +339,7 @@ void qmp_add_client(const char *protocol, const char *fdname,
}
skipauth = has_skipauth ? skipauth : false;
tls = has_tls ? tls : false;
- if (qemu_spice_display_add_client(fd, skipauth, tls) < 0) {
+ if (qemu_spice.display_add_client(fd, skipauth, tls) < 0) {
error_setg(errp, "spice failed to add client");
close(fd);
}
diff --git a/softmmu/vl.c b/softmmu/vl.c
index bc0dcc4f58..8defda56c9 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -173,6 +173,30 @@ int icount_align_option;
QemuUUID qemu_uuid;
bool qemu_uuid_set;
+static bool qemu_is_not_using_spice(void)
+{
+ return 0;
+}
+
+static void qemu_spice_init_no_op(void)
+{
+}
+
+/*
+ * Only two fields are initialized, that can be used even when SPICE
+ * is not configured or not loaded. Other functions are protected by
+ * checking if using_spice.
+ */
+QemuSpiceOps qemu_spice = {
+ .in_use = qemu_is_not_using_spice,
+ .init = qemu_spice_init_no_op,
+};
+
+void qemu_spice_ops_register(QemuSpiceOps *ops)
+{
+ memcpy(&qemu_spice, ops, sizeof(qemu_spice));
+}
+
static NotifierList exit_notifiers =
NOTIFIER_LIST_INITIALIZER(exit_notifiers);
@@ -4128,7 +4152,7 @@ void qemu_init(int argc, char **argv, char **envp)
/* 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_spice.init();
qemu_opts_foreach(qemu_find_opts("object"),
user_creatable_add_opts_foreach,
@@ -4424,7 +4448,7 @@ void qemu_init(int argc, char **argv, char **envp)
#endif
if (using_spice) {
- qemu_spice_display_init();
+ qemu_spice.display_init();
}
if (foreach_device_config(DEV_GDB, gdbserver_start) < 0) {
diff --git a/ui/spice-core.c b/ui/spice-core.c
index 37dd68f2ab..9d7ac70d92 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -48,7 +48,7 @@ static time_t auth_expires = TIME_MAX;
static int spice_migration_completed;
static int spice_display_is_running;
static int spice_have_target_host;
-int using_spice = 0;
+static int is_using_spice;
static QemuThread me;
@@ -503,7 +503,7 @@ static QemuOptsList qemu_spice_opts = {
},
};
-SpiceInfo *qmp_query_spice(Error **errp)
+SpiceInfo *qemu_spice_query(Error **errp)
{
QemuOpts *opts = QTAILQ_FIRST(&qemu_spice_opts.head);
int port, tls_port;
@@ -634,6 +634,16 @@ static void vm_change_state_handler(void *opaque, int running,
}
}
+bool qemu_is_using_spice(void)
+{
+ return is_using_spice;
+}
+
+void qemu_start_using_spice(void)
+{
+ is_using_spice = 1;
+}
+
void qemu_spice_init(void)
{
QemuOpts *opts = QTAILQ_FIRST(&qemu_spice_opts.head);
@@ -796,7 +806,7 @@ void qemu_spice_init(void)
error_report("failed to initialize spice server");
exit(1);
};
- using_spice = 1;
+ qemu_start_using_spice();
migration_state.notify = migration_state_notifier;
add_migration_state_change_notifier(&migration_state);
@@ -1000,3 +1010,20 @@ static void spice_register_config(void)
qemu_add_opts(&qemu_spice_opts);
}
opts_init(spice_register_config);
+
+static QemuSpiceOps qemu_spice_ops = {
+ .in_use = qemu_is_using_spice,
+ .init = qemu_spice_init,
+ .display_init = qemu_spice_display_init,
+ .display_add_client = qemu_spice_display_add_client,
+ .set_passwd = qemu_spice_set_passwd,
+ .set_pw_expire = qemu_spice_set_pw_expire,
+ .migrate_info = qemu_spice_migrate_info,
+ .query = qemu_spice_query,
+};
+
+static void spice_register_ops(void)
+{
+ qemu_spice_ops_register(&qemu_spice_ops);
+}
+type_init(spice_register_ops);
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 7/7] spice: Call qemu spice functions indirectly
2020-07-23 17:46 ` [PATCH 7/7] spice: Call qemu spice functions indirectly Christophe de Dinechin
@ 2020-07-27 17:38 ` Dr. David Alan Gilbert
2020-08-04 10:25 ` Gerd Hoffmann
2020-08-04 10:18 ` Gerd Hoffmann
1 sibling, 1 reply; 17+ messages in thread
From: Dr. David Alan Gilbert @ 2020-07-27 17:38 UTC (permalink / raw)
To: Christophe de Dinechin
Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel,
Markus Armbruster, Paolo Bonzini, kraxel, Cleber Rosa,
Marc-André Lureau, Richard Henderson
* Christophe de Dinechin (dinechin@redhat.com) wrote:
> In order to be able to move all of spice in a shared library that is
> loaded dynamically only on demand, this commit puts the functions
> called by the main binary in a QemuSpiceOps structure, and use static
> inline stubs to preserve the call sites.
>
> With these changes, the following shared libraries are no longer
> necessary in the top-level qemu binary when building with CONFIG_MDULES:
>
> libspice-server.so.1 => /lib64/libspice-server.so.1 (HEX)
> libopus.so.0 => /lib64/libopus.so.0 (HEX)
> liblz4.so.1 => /lib64/liblz4.so.1 (HEX)
> libgstapp-1.0.so.0 => /lib64/libgstapp-1.0.so.0 (HEX)
> libgstvideo-1.0.so.0 => /lib64/libgstvideo-1.0.so.0 (HEX)
> libgstbase-1.0.so.0 => /lib64/libgstbase-1.0.so.0 (HEX)
> libgstreamer-1.0.so.0 => /lib64/libgstreamer-1.0.so.0 (HEX)
> libssl.so.1.1 => /lib64/libssl.so.1.1 (HEX)
> liborc-0.4.so.0 => /lib64/liborc-0.4.so.0 (HEX)
>
> Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
> ---
> include/ui/qemu-spice.h | 74 +++++++++++++++++++----------------------
> monitor/hmp-cmds.c | 13 ++++++++
> monitor/misc.c | 2 +-
> monitor/qmp-cmds.c | 6 ++--
> softmmu/vl.c | 28 ++++++++++++++--
> ui/spice-core.c | 33 ++++++++++++++++--
> 6 files changed, 107 insertions(+), 49 deletions(-)
>
> diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
> index d34cea2e0f..9721d8817e 100644
> --- a/include/ui/qemu-spice.h
> +++ b/include/ui/qemu-spice.h
> @@ -25,21 +25,28 @@
> #include <spice.h>
> #include "qemu/config-file.h"
>
> -extern int using_spice;
> +#define using_spice (qemu_spice.in_use())
>
> -void qemu_spice_init(void);
> void qemu_spice_input_init(void);
> void qemu_spice_audio_init(void);
> -void qemu_spice_display_init(void);
> -int qemu_spice_display_add_client(int csock, int skipauth, int tls);
> int qemu_spice_add_interface(SpiceBaseInstance *sin);
> bool qemu_spice_have_display_interface(QemuConsole *con);
> int qemu_spice_add_display_interface(QXLInstance *qxlin, QemuConsole *con);
> +
> +#if !defined(CONFIG_MODULES) || defined(BUILD_DSO)
> +bool qemu_is_using_spice(void);
> +void qemu_start_using_spice(void);
> +void qemu_spice_init(void);
> +void qemu_spice_display_init(void);
> +int qemu_spice_display_add_client(int csock, int skipauth, int tls);
> int qemu_spice_set_passwd(const char *passwd,
> bool fail_if_connected, bool disconnect_if_connected);
> int qemu_spice_set_pw_expire(time_t expires);
> int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
> const char *subject);
> +struct SpiceInfo *qemu_spice_query(Error **errp);
> +#endif /* !CONFIG_MODULES || BUILD_DSO */
> +
>
> #if !defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06)
> #define SPICE_NEEDS_SET_MM_TIME 1
> @@ -47,46 +54,33 @@ int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
> #define SPICE_NEEDS_SET_MM_TIME 0
> #endif
>
> -#else /* CONFIG_SPICE */
> -
> -#include "qemu/error-report.h"
> -
> -#define using_spice 0
> -#define spice_displays 0
> -static inline int qemu_spice_set_passwd(const char *passwd,
> - bool fail_if_connected,
> - bool disconnect_if_connected)
> -{
> - return -1;
> -}
> -static inline int qemu_spice_set_pw_expire(time_t expires)
> -{
> - return -1;
> -}
> -static inline int qemu_spice_migrate_info(const char *h, int p, int t,
> - const char *s)
> -{
> - return -1;
> -}
> +#else /* !CONFIG_SPICE */
>
> -static inline int qemu_spice_display_add_client(int csock, int skipauth,
> - int tls)
> -{
> - return -1;
> -}
> +#define using_spice 0
>
> -static inline void qemu_spice_display_init(void)
> -{
> - /* This must never be called if CONFIG_SPICE is disabled */
> - error_report("spice support is disabled");
> - abort();
> -}
> +#endif /* CONFIG_SPICE */
>
> -static inline void qemu_spice_init(void)
> +/* Module high-level inteface for SPICE */
> +struct QemuSpiceOps
> {
> -}
> -
> -#endif /* CONFIG_SPICE */
> + bool (*in_use)(void);
> + void (*init)(void);
> + void (*display_init)(void);
> + int (*display_add_client)(int csock, int skipauth, int tls);
> +
> + int (*set_passwd)(const char *passwd,
> + bool fail_if_connected,
> + bool disconnect_if_connected);
> + int (*set_pw_expire)(time_t expires);
> + int (*migrate_info)(const char *hostname,
> + int port, int tls_port,
> + const char *subject);
> + struct SpiceInfo * (*query)(Error **errp);
> +};
> +typedef struct QemuSpiceOps QemuSpiceOps;
> +void qemu_spice_ops_register(QemuSpiceOps *ops);
> +
> +extern struct QemuSpiceOps qemu_spice;
>
> static inline bool qemu_using_spice(Error **errp)
> {
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index ae4b6a4246..8e10af188f 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -57,6 +57,7 @@
>
> #ifdef CONFIG_SPICE
> #include <spice/enums.h>
> +#include "ui/qemu-spice.h"
> #endif
>
> void hmp_handle_error(Monitor *mon, Error *err)
> @@ -573,6 +574,14 @@ void hmp_info_vnc(Monitor *mon, const QDict *qdict)
> #endif
>
> #ifdef CONFIG_SPICE
> +SpiceInfo *qmp_query_spice(Error **errp)
> +{
> + if (!using_spice) {
> + return NULL;
> + }
> + return qemu_spice.query(errp);
> +}
> +
It's a bit weird to put the qmp function in the hmp file.
> void hmp_info_spice(Monitor *mon, const QDict *qdict)
> {
> SpiceChannelList *chan;
> @@ -599,6 +608,10 @@ void hmp_info_spice(Monitor *mon, const QDict *qdict)
>
> info = qmp_query_spice(NULL);
>
> + if (!info) {
> + monitor_printf(mon, "Server: not started\n");
> + goto out;
> + }
> if (!info->enabled) {
> monitor_printf(mon, "Server: disabled\n");
> goto out;
Dave
> diff --git a/monitor/misc.c b/monitor/misc.c
> index e847b58a8c..88f8915734 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -443,7 +443,7 @@ void qmp_client_migrate_info(const char *protocol, const char *hostname,
> return;
> }
>
> - if (qemu_spice_migrate_info(hostname,
> + if (qemu_spice.migrate_info(hostname,
> has_port ? port : -1,
> has_tls_port ? tls_port : -1,
> cert_subject)) {
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index 864cbfa32e..9f9822b3f3 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -196,7 +196,7 @@ void qmp_set_password(const char *protocol, const char *password,
> if (!qemu_using_spice(errp)) {
> return;
> }
> - rc = qemu_spice_set_passwd(password, fail_if_connected,
> + rc = qemu_spice.set_passwd(password, fail_if_connected,
> disconnect_if_connected);
> if (rc != 0) {
> error_setg(errp, QERR_SET_PASSWD_FAILED);
> @@ -242,7 +242,7 @@ void qmp_expire_password(const char *protocol, const char *whenstr,
> if (!qemu_using_spice(errp)) {
> return;
> }
> - rc = qemu_spice_set_pw_expire(when);
> + rc = qemu_spice.set_pw_expire(when);
> if (rc != 0) {
> error_setg(errp, QERR_SET_PASSWD_FAILED);
> }
> @@ -339,7 +339,7 @@ void qmp_add_client(const char *protocol, const char *fdname,
> }
> skipauth = has_skipauth ? skipauth : false;
> tls = has_tls ? tls : false;
> - if (qemu_spice_display_add_client(fd, skipauth, tls) < 0) {
> + if (qemu_spice.display_add_client(fd, skipauth, tls) < 0) {
> error_setg(errp, "spice failed to add client");
> close(fd);
> }
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index bc0dcc4f58..8defda56c9 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -173,6 +173,30 @@ int icount_align_option;
> QemuUUID qemu_uuid;
> bool qemu_uuid_set;
>
> +static bool qemu_is_not_using_spice(void)
> +{
> + return 0;
> +}
> +
> +static void qemu_spice_init_no_op(void)
> +{
> +}
> +
> +/*
> + * Only two fields are initialized, that can be used even when SPICE
> + * is not configured or not loaded. Other functions are protected by
> + * checking if using_spice.
> + */
> +QemuSpiceOps qemu_spice = {
> + .in_use = qemu_is_not_using_spice,
> + .init = qemu_spice_init_no_op,
> +};
> +
> +void qemu_spice_ops_register(QemuSpiceOps *ops)
> +{
> + memcpy(&qemu_spice, ops, sizeof(qemu_spice));
> +}
> +
> static NotifierList exit_notifiers =
> NOTIFIER_LIST_INITIALIZER(exit_notifiers);
>
> @@ -4128,7 +4152,7 @@ void qemu_init(int argc, char **argv, char **envp)
> /* 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_spice.init();
>
> qemu_opts_foreach(qemu_find_opts("object"),
> user_creatable_add_opts_foreach,
> @@ -4424,7 +4448,7 @@ void qemu_init(int argc, char **argv, char **envp)
> #endif
>
> if (using_spice) {
> - qemu_spice_display_init();
> + qemu_spice.display_init();
> }
>
> if (foreach_device_config(DEV_GDB, gdbserver_start) < 0) {
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index 37dd68f2ab..9d7ac70d92 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -48,7 +48,7 @@ static time_t auth_expires = TIME_MAX;
> static int spice_migration_completed;
> static int spice_display_is_running;
> static int spice_have_target_host;
> -int using_spice = 0;
> +static int is_using_spice;
>
> static QemuThread me;
>
> @@ -503,7 +503,7 @@ static QemuOptsList qemu_spice_opts = {
> },
> };
>
> -SpiceInfo *qmp_query_spice(Error **errp)
> +SpiceInfo *qemu_spice_query(Error **errp)
> {
> QemuOpts *opts = QTAILQ_FIRST(&qemu_spice_opts.head);
> int port, tls_port;
> @@ -634,6 +634,16 @@ static void vm_change_state_handler(void *opaque, int running,
> }
> }
>
> +bool qemu_is_using_spice(void)
> +{
> + return is_using_spice;
> +}
> +
> +void qemu_start_using_spice(void)
> +{
> + is_using_spice = 1;
> +}
> +
> void qemu_spice_init(void)
> {
> QemuOpts *opts = QTAILQ_FIRST(&qemu_spice_opts.head);
> @@ -796,7 +806,7 @@ void qemu_spice_init(void)
> error_report("failed to initialize spice server");
> exit(1);
> };
> - using_spice = 1;
> + qemu_start_using_spice();
>
> migration_state.notify = migration_state_notifier;
> add_migration_state_change_notifier(&migration_state);
> @@ -1000,3 +1010,20 @@ static void spice_register_config(void)
> qemu_add_opts(&qemu_spice_opts);
> }
> opts_init(spice_register_config);
> +
> +static QemuSpiceOps qemu_spice_ops = {
> + .in_use = qemu_is_using_spice,
> + .init = qemu_spice_init,
> + .display_init = qemu_spice_display_init,
> + .display_add_client = qemu_spice_display_add_client,
> + .set_passwd = qemu_spice_set_passwd,
> + .set_pw_expire = qemu_spice_set_pw_expire,
> + .migrate_info = qemu_spice_migrate_info,
> + .query = qemu_spice_query,
> +};
> +
> +static void spice_register_ops(void)
> +{
> + qemu_spice_ops_register(&qemu_spice_ops);
> +}
> +type_init(spice_register_ops);
> --
> 2.26.2
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 7/7] spice: Call qemu spice functions indirectly
2020-07-27 17:38 ` Dr. David Alan Gilbert
@ 2020-08-04 10:25 ` Gerd Hoffmann
0 siblings, 0 replies; 17+ messages in thread
From: Gerd Hoffmann @ 2020-08-04 10:25 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel,
Markus Armbruster, Paolo Bonzini, Cleber Rosa,
Christophe de Dinechin, Marc-André Lureau,
Richard Henderson
> > #ifdef CONFIG_SPICE
> > +SpiceInfo *qmp_query_spice(Error **errp)
> > +{
> > + if (!using_spice) {
> > + return NULL;
> > + }
> > + return qemu_spice.query(errp);
> > +}
> > +
>
> It's a bit weird to put the qmp function in the hmp file.
Yes, it should go to qmp-cmds.c instead.
The #ifdef CONFIG_SPICE should not be needed. Also the !using_spice
case should throw an error. Or, even better, just have a
qemu_spice_query_stub() function throwing an error, then the
!using_spice check can be dropped.
take care,
Gerd
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 7/7] spice: Call qemu spice functions indirectly
2020-07-23 17:46 ` [PATCH 7/7] spice: Call qemu spice functions indirectly Christophe de Dinechin
2020-07-27 17:38 ` Dr. David Alan Gilbert
@ 2020-08-04 10:18 ` Gerd Hoffmann
1 sibling, 0 replies; 17+ messages in thread
From: Gerd Hoffmann @ 2020-08-04 10:18 UTC (permalink / raw)
To: Christophe de Dinechin
Cc: Eduardo Habkost, Michael S. Tsirkin, Markus Armbruster,
Dr. David Alan Gilbert, qemu-devel, Paolo Bonzini, Cleber Rosa,
Marc-André Lureau, Richard Henderson
Hi,
These stubs (picked two examples) ...
> -static inline int qemu_spice_set_passwd(const char *passwd,
> - bool fail_if_connected,
> - bool disconnect_if_connected)
> -{
> - return -1;
> -}
> -static inline void qemu_spice_display_init(void)
> -{
> - /* This must never be called if CONFIG_SPICE is disabled */
> - error_report("spice support is disabled");
> - abort();
> -}
... can simply be moved to this place ...
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -173,6 +173,30 @@ int icount_align_option;
> QemuUUID qemu_uuid;
> bool qemu_uuid_set;
>
> +static bool qemu_is_not_using_spice(void)
> +{
> + return 0;
> +}
> +
> +static void qemu_spice_init_no_op(void)
> +{
> +}
(I'd suggest to name this qemu_spice_display_init_stub)
> +
> +/*
> + * Only two fields are initialized, that can be used even when SPICE
> + * is not configured or not loaded. Other functions are protected by
> + * checking if using_spice.
> + */
> +QemuSpiceOps qemu_spice = {
> + .in_use = qemu_is_not_using_spice,
> + .init = qemu_spice_init_no_op,
... and hooked up here, i.e.
.init = qemu_spice_display_init_stub,
.set_passwd = qemu_spice_set_passwd_stub,
[ ... ]
Also I'd suggest to place the code in a new file (maybe named
ui/spice-ops.c, or ui/spice-stubs.c) which is compiled into qemu
unconditionally.
take care,
Gerd
^ permalink raw reply [flat|nested] 17+ messages in thread