Re: [PATCH v2 03/18] modules: add qemu-modinfo utility

[Paolo Bonzini]

On 10/06/21 15:13, Daniel P. Berrangé wrote:
> On Thu, Jun 10, 2021 at 03:04:24PM +0200, Gerd Hoffmann wrote:
> >    Hi Paolo,
> >
> > > +if config_host.has_key('CONFIG_MODULES')
> > > +   qemu_modinfo = executable('qemu-modinfo', files('qemu-modinfo.c') + genh,
> > > +                             dependencies: [glib, qemuutil], install: 
> > > have_tools)
> > > +   custom_target('modinfo.json',
> > > +                 input: [ softmmu_mods, block_mods ],
> > > +                 output: 'modinfo.json',
> > > +                 install: true,
> > > +                 install_dir: qemu_moddir,
> > > +                 command: [ qemu_modinfo, '.' ])
> > > +endif
> >
> > I have trouble with this one.  Tried to declaring the modules as "input"
> > to make sure meson will only run qemu-modinfo when it is done building
> > the module.  But now and then I get build errors because qemu-modinfo
> > runs in parallel to a module build and qemu-modinfo throws an read error
> > because of that.
>
> softmmu_mods and block_mods are both lists already, so this sets a
> nested list and I wonder if it confuses meson  ? eg do you need
>
>   input: softmmu_mods + block_mods

No, that should be fine.  Perhaps it's because the inputs are not part 
of the command?  You can check what the build.ninja rule for 
modinfo.json looks like.

Also:

- it would be better to support both directories and file names, so that 
stale modules are not included in modinfo.json

- modinfo.json needs to be disabled on non-ELF platforms (x86, Windows). 
 One alternative is to use libbfd instead of including an ELF parser.

- If modinfo.json has to be installed, you need to build modinfo for the 
build machine in order to support cross compiling.  That however would 
require a cross libbfd, which is a pain.  Do you really need to install 
it?  Can the functionality instead be included in the main QEMU binary 
with a query-modules command or something like that.

Paolo

> Alternatively there is option to do:
>
>    'depends: softmmu_mods + block_mods
>
> though the meson docs claim that's not required if they're
> already listed against 'input:'