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

[Fam Zheng]

On Mon, 09/16 11:24, Alex Bligh wrote:
> 
> 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?
> 

Sounds good to me, I agree that "whitelist" is not necessary for user. We still
need a known_modules in the new module_load() code and it'll look very similar
to current whitelist. But I'll change the name.

Fam