Re: [Qemu-devel] [PATCH] Support for loading devices as dynamic libraries

[Anthony Liguori]

Dominik Żeromski <address@hidden> writes:

> Adding support for loading DSO with -device option.

Hi,

A few things:

1) Out of tree modules are boring and there's very little
   support/sympathy for supporting out of tree modules.  That said, if
   you implemented support for in tree modules and the build system
   happened to work with out of tree modules too (as Linux does), you
   would find much more support for that.

2) The GNU module guidelines should be followed.  Namely, we should
   expect modules to declare their licenses and programmatically enforce
   license compatibility.

3) You should use glib's module loading API, not libdl

4) An explicitly insmod command should be used to load modules.  Module
   dependency is very complicated.  It's easier to just load modules in
   a specific order based on a configuration file.

There are very useful reasons to have modules in QEMU.  I really think
Spice support would make sense as a module, for instance.  libspice has
a lot of dependencies and forcing distros to set those dependencies as
dependencies on QEMU really stinks.

So there's pretty good use-cases for in-tree modules.  It's definitely
worth doing.  It's also pretty useful when doing security certifications.

Regards,

Anthony Liguori

>
> Example Makefile for out of tree modules:
> #v+
> DEVICENAME=pcnet2
>
> hw-obj-y=pcnet-pci.o
> hw-obj-y+=pcnet.o
>
> include rules.mak
>
> .PHONY: all
>
> QEMU_CFLAGS=-I../qemu-kvm -I../qemu-kvm/hw 
> QEMU_CFLAGS+=-I../qemu-kvm/fpu -I../qemu-kvm/include
> QEMU_CFLAGS+=-I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include
>
> QEMU_CFLAGS+=-DTARGET_PHYS_ADDR_BITS=64 -fPIC
> LDFLAGS+=-shared
>
> LIBNAME=libqemu_$(DEVICENAME).so
>
> all: $(LIBNAME)
>
> $(LIBNAME): $(hw-obj-y)
>       $(call LINK,$^)
>
> clean:
>       rm -f *.o
>       rm -f *.d
>       rm -f $(LIBNAME)
>
> # Include automatically generated dependency files
> -include $(patsubst %.o, %.d, $(hw-obj-y))
>
> #v-
>
> Signed-off-by: Dominik Żeromski <address@hidden>
> ---
>  Makefile.target   |    4 +++-
>  hw/qdev-monitor.c |   11 +++++++++++
>  2 files changed, 14 insertions(+), 1 deletions(-)
>
> diff --git a/Makefile.target b/Makefile.target
> index 74f7a4a..7fd9245 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -130,7 +130,9 @@ obj-$(CONFIG_HAVE_GET_MEMORY_MAPPING) += memory_mapping.o
>  obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o
>  obj-$(CONFIG_NO_GET_MEMORY_MAPPING) += memory_mapping-stub.o
>  obj-$(CONFIG_NO_CORE_DUMP) += dump-stub.o
> -LIBS+=-lz
> +LIBS+=-lz -ldl
> +
> +LDFLAGS+=-rdynamic
>  
>  QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
>  QEMU_CFLAGS += $(VNC_SASL_CFLAGS)
> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> index 7915b45..3b5b0b0 100644
> --- a/hw/qdev-monitor.c
> +++ b/hw/qdev-monitor.c
> @@ -17,6 +17,8 @@
>   * License along with this library; if not, see 
> <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <dlfcn.h>
> +
>  #include "qdev.h"
>  #include "monitor.h"
>  #include "qmp-commands.h"
> @@ -402,6 +404,8 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>      const char *driver, *path, *id;
>      DeviceState *qdev;
>      BusState *bus;
> +    void *libhandle;
> +    char libname[NAME_MAX];
>  
>      driver = qemu_opt_get(opts, "driver");
>      if (!driver) {
> @@ -419,7 +423,14 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>              obj = object_class_by_name(driver);
>          }
>      }
> +    if (!obj) {
> +        snprintf(libname, sizeof(libname), "libqemu_%s.so", driver);
>  
> +        libhandle = dlopen(libname, RTLD_NOW);
> +        if (libhandle != NULL) {
> +            obj = object_class_by_name(driver);
> +        }
> +    }
>      if (!obj) {
>          qerror_report(QERR_INVALID_PARAMETER_VALUE, "driver", "device type");
>          return NULL;
> -- 
> 1.7.0.4