Re: [Qemu-devel] [PATCH v10 5/8] module: implement module loading

[Alex Bligh]

On 16 Sep 2013, at 10:51, Fam Zheng wrote:

> On Mon, 09/16 11:44, Paolo Bonzini wrote:
>> Il 16/09/2013 10:59, Daniel P. Berrange ha scritto:
>>>> The init function of dynamic module is no longer with
>>>> __attribute__((constructor)) as static linked version, and need to be
>>>> explicitly called once loaded. The function name is mangled with per
>>>> configure fingerprint as:
>>>> 
>>>>    init_$(date +%s$$$RANDOM)
>> 
>> Does this work for a module that calls module_init multiple times?
>> 
> Why should a module calls module_init, instead of the main function?
> 
> This name is generated per "./configure", not per object or per make, so it's
> essentially the same with any fixed function name, except for two objects 
> built
> from two different "./configure" (which is the purpose for the mangling here).

I think I must be missing something here.

We do not have a stable API/ABI and it seems generally acknowledged at this
stage that we don't need one. Therefore, to avoid API/ABI mismatch between
the executables and the modules, in ./configure you are generating a random
cookie (effectively) that you are calling the fingerprint.

The executable will then not load the module unless the module has the
right cookie. As far as I can tell, that means the module needs to be
built within the same build harness as the executables, or it won't
know what to call its init function.

And that's perfectly compatible with the stated objective:
> The main idea behind modules is to isolate dependencies on third party
> libraries from qemu executables, such as libglusterfs or librbd, so that the
> end users can install core qemu package with fewer dependencies.  And only for
> those who want to use particular modules, need they install qemu-foo
> sub-package, which in turn requires libbar and libbiz packages.

... this being to isolate dependencies, and not to enable third party
modules built outside the tree.

That's all well and good, but if the modules are all built within the
same build harness, why do we need a whitelist or a readdir() at all? We
know what the modules are, because they were the ones that were built
at the same time. Why not just process the list of modules it was
built with, and if you get EEXIST, move on?

The other issue I have with this approach is that it doesn't address
the situation where the distro builds the qemu version against its
distributed version of the development library for the plug in, but
on deployment, everyone wants to use a new development library. To
give a concrete example, qemu in Ubuntu Precise is build against
librbd 0.42 (from memory - don't shoot me if wrong), and that supports
fewer calls than librbd 0.62 which is what anyone sane will want to
run.

At risk of heresy, can I suggest a rather simpler scheme that requires
a total of zero infrastructure changes?

Here's a patch against qemu 1.0 (sorry) Ubuntu dist (sorry) that
uses weak binding to load and compile against any version of
librbd:
  
https://github.com/flexiant/qemu/commit/6fa2e9c95bdaca7c814881e27f04424fb6cc2960

This requires librbd-dev headers of some sort in order to build. But
librbd does not need to be there on a deployment, and indeed multiple
versions work (and the patch does different things depending on
librbd version).

This would seem to achieve the stated objective (qemu package can
be installed without librbd dependencies) without any reliance
on modules at all. So we could scrap the whole modules infrastructure
and just tell people developing third party libraries to use weak
binding. As you can see, for librbd anyway, the changes required
to support weak binding are pretty minimal.

-- 
Alex Bligh