| Message ID | 1526560782-18732-1-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | ui: add x_keymap.o to modules | expand |
On Thu, May 17, 2018 at 02:39:42PM +0200, Paolo Bonzini wrote: > x_keymap.o is common to the SDL and GTK+ modules, and it causes the > QEMU binary to link to the X11 libraries. Add it separately to the > modules to keep the main QEMU binary smaller. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > ui/Makefile.objs | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/ui/Makefile.objs b/ui/Makefile.objs > index cc78434..00f6976 100644 > --- a/ui/Makefile.objs > +++ b/ui/Makefile.objs > @@ -15,10 +15,6 @@ common-obj-$(CONFIG_COCOA) += cocoa.o > common-obj-$(CONFIG_VNC) += $(vnc-obj-y) > common-obj-$(call lnot,$(CONFIG_VNC)) += vnc-stubs.o > > -common-obj-$(CONFIG_X11) += x_keymap.o > -x_keymap.o-cflags := $(X11_CFLAGS) > -x_keymap.o-libs := $(X11_LIBS) > - > # ui-sdl module > common-obj-$(CONFIG_SDL) += sdl.mo > ifeq ($(CONFIG_SDLABI),1.2) > @@ -46,6 +42,13 @@ gtk.mo-objs += gtk-gl-area.o > endif > endif > > +ifeq ($(CONFIG_X11),y) > +sdl.mo-objs += x_keymap.o > +gtk.mo-objs += x_keymap.o Would this cause symbol clash if both sdl & gtk modules are loaded at the same time, or have we used linker scripts to limit what symbols each module exposes ? > +x_keymap.o-cflags := $(X11_CFLAGS) > +x_keymap.o-libs := $(X11_LIBS) > +endif > + > common-obj-$(CONFIG_CURSES) += curses.mo > curses.mo-objs := curses.o > curses.mo-cflags := $(CURSES_CFLAGS) > -- > 1.8.3.1 > > Regards, Daniel
On 17/05/2018 14:44, Daniel P. Berrangé wrote: >> +ifeq ($(CONFIG_X11),y) >> +sdl.mo-objs += x_keymap.o >> +gtk.mo-objs += x_keymap.o > Would this cause symbol clash if both sdl & gtk modules are loaded > at the same time, or have we used linker scripts to limit what symbols > each module exposes ? > We don't, but: 1) the file has only functions and no data; 2) in any case the symbols are the same, so it is not a real clash. Adding linker scripts would be a nice improvement, but it is not necessary for this patch. Another possibility would be to include x_keymap.c in the files that use it and make qemu_xkeymap_mapping_table static, but I think it would be the worst. Paolo
Hi, > > +ifeq ($(CONFIG_X11),y) > > +sdl.mo-objs += x_keymap.o > > +gtk.mo-objs += x_keymap.o > > Would this cause symbol clash if both sdl & gtk modules are loaded > at the same time, or have we used linker scripts to limit what symbols > each module exposes ? Related: can modules depend on modules, so we could make x_keymap a module of its own and have both gtk and sdl depend on it? That would also be useful when trying to modularize spice. cheers, Gerd
On Thu, May 17, 2018 at 02:51:10PM +0200, Gerd Hoffmann wrote: > Hi, > > > > +ifeq ($(CONFIG_X11),y) > > > +sdl.mo-objs += x_keymap.o > > > +gtk.mo-objs += x_keymap.o > > > > Would this cause symbol clash if both sdl & gtk modules are loaded > > at the same time, or have we used linker scripts to limit what symbols > > each module exposes ? > > Related: can modules depend on modules, so we could make x_keymap a > module of its own and have both gtk and sdl depend on it? > > That would also be useful when trying to modularize spice. Yes, you could create a xkeymap.so, and link to that from both sdl.so and gtk.so, so you'll only get one copy of xkeymap.so if both are loaded. I don't think it is worth it for this particular case though, since we'll be deleting the SDL1 code when 2.14 dev cycle opens, leaving GTK as the only user of it. SDL2 already has abstracted keycodes. Regards, Daniel
On Thu, May 17, 2018 at 02:50:04PM +0200, Paolo Bonzini wrote: > On 17/05/2018 14:44, Daniel P. Berrangé wrote: > >> +ifeq ($(CONFIG_X11),y) > >> +sdl.mo-objs += x_keymap.o > >> +gtk.mo-objs += x_keymap.o > > Would this cause symbol clash if both sdl & gtk modules are loaded > > at the same time, or have we used linker scripts to limit what symbols > > each module exposes ? > > > > We don't, but: 1) the file has only functions and no data; 2) in any > case the symbols are the same, so it is not a real clash. Ok, as long as dlopen() doesn't whine its fine with me. > Adding linker scripts would be a nice improvement, but it is not > necessary for this patch. Agreed. > Another possibility would be to include x_keymap.c in the files that use > it and make qemu_xkeymap_mapping_table static, but I think it would be > the worst. It isn't worth trying to be too clever since we're deleting SDL1 in the 2.14 dev cycle. Regards, Daniel
On 17/05/2018 14:51, Gerd Hoffmann wrote: > Hi, > >>> +ifeq ($(CONFIG_X11),y) >>> +sdl.mo-objs += x_keymap.o >>> +gtk.mo-objs += x_keymap.o >> >> Would this cause symbol clash if both sdl & gtk modules are loaded >> at the same time, or have we used linker scripts to limit what symbols >> each module exposes ? > > Related: can modules depend on modules, so we could make x_keymap a > module of its own and have both gtk and sdl depend on it? > > That would also be useful when trying to modularize spice. How hard would it be to modularize the libspice-server side? The part of the library that is used by QXL rendering should have much fewer dependencies than the part that is used for keyboard, mouse, audio, vmchannel/agent, etc. Then you could link libspice-server-core into QEMU and libspice-server into the modules. Unless both have been linked together, functions such as spice_server_add_client would fail, and so would adding most of the SPICE_INTERFACE_* interface kinds. Thanks, Paolo
Hi, > > Related: can modules depend on modules, so we could make x_keymap a > > module of its own and have both gtk and sdl depend on it? > > > > That would also be useful when trying to modularize spice. > > How hard would it be to modularize the libspice-server side? The part > of the library that is used by QXL rendering should have much fewer > dependencies than the part that is used for keyboard, mouse, audio, > vmchannel/agent, etc. kbd, mouse, audio is needed on the client side and is not part of libspice-server anyway. So spice is much less of a burden compared to sdl/gtk which bring alot of ui toolkit deps. cheers, Gerd
On 17/05/2018 15:45, Gerd Hoffmann wrote: > Hi, > >>> Related: can modules depend on modules, so we could make x_keymap a >>> module of its own and have both gtk and sdl depend on it? >>> >>> That would also be useful when trying to modularize spice. >> >> How hard would it be to modularize the libspice-server side? The part >> of the library that is used by QXL rendering should have much fewer >> dependencies than the part that is used for keyboard, mouse, audio, >> vmchannel/agent, etc. > > kbd, mouse, audio is needed on the client side and is not part of > libspice-server anyway. Yes, I'm talking about separating the client side from the QXL rendering part. > So spice is much less of a burden compared > to sdl/gtk which bring alot of ui toolkit deps. But SPICE does bring in 16 libraries, including both of NSS and OpenSSL... Paolo
On Thu, May 17, 2018 at 02:39:42PM +0200, Paolo Bonzini wrote: > x_keymap.o is common to the SDL and GTK+ modules, and it causes the > QEMU binary to link to the X11 libraries. Add it separately to the > modules to keep the main QEMU binary smaller. Added to ui patch queue. thanks, Gerd
diff --git a/ui/Makefile.objs b/ui/Makefile.objs index cc78434..00f6976 100644 --- a/ui/Makefile.objs +++ b/ui/Makefile.objs @@ -15,10 +15,6 @@ common-obj-$(CONFIG_COCOA) += cocoa.o common-obj-$(CONFIG_VNC) += $(vnc-obj-y) common-obj-$(call lnot,$(CONFIG_VNC)) += vnc-stubs.o -common-obj-$(CONFIG_X11) += x_keymap.o -x_keymap.o-cflags := $(X11_CFLAGS) -x_keymap.o-libs := $(X11_LIBS) - # ui-sdl module common-obj-$(CONFIG_SDL) += sdl.mo ifeq ($(CONFIG_SDLABI),1.2) @@ -46,6 +42,13 @@ gtk.mo-objs += gtk-gl-area.o endif endif +ifeq ($(CONFIG_X11),y) +sdl.mo-objs += x_keymap.o +gtk.mo-objs += x_keymap.o +x_keymap.o-cflags := $(X11_CFLAGS) +x_keymap.o-libs := $(X11_LIBS) +endif + common-obj-$(CONFIG_CURSES) += curses.mo curses.mo-objs := curses.o curses.mo-cflags := $(CURSES_CFLAGS)
x_keymap.o is common to the SDL and GTK+ modules, and it causes the QEMU binary to link to the X11 libraries. Add it separately to the modules to keep the main QEMU binary smaller. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- ui/Makefile.objs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)