Re: [Qemu-devel] [PATCH v2 0/2] Dynamic module support for block drivers

["Denis V. Lunev" <den-lists@parallels.com>]

On 09/21/2015 05:44 PM, Marc Marí wrote:
> On Mon, 21 Sep 2015 16:43:27 +0300
> "Denis V. Lunev" <den-lists@parallels.com> wrote:
>
>> On 09/08/2015 04:53 PM, Marc Marí wrote:
>>> The current module infrastructure has been improved to enable
>>> dynamic module loading.
>>>
>>> This reduces the load time for very simple guests. For the following
>>> configuration (very loaded)
>>>
>>> ./configure --enable-sdl --enable-gtk --enable-vte --enable-curses \
>>>       --enable-vnc --enable-vnc-{jpeg,tls,sasl,png} --enable-virtfs \
>>>       --enable-brlapi --enable-curl --enable-fdt --enable-bluez \
>>>       --enable-kvm --enable-rdma --enable-uuid --enable-vde \
>>>       --enable-linux-aio --enable-cap-ng --enable-attr
>>> --enable-vhost-net \ --enable-vhost-scsi --enable-spice
>>> --enable-rbd --enable-libiscsi \ --enable-smartcard-nss
>>> --enable-guest-agent --enable-libusb \ --enable-usb-redir
>>> --enable-lzo --enable-snappy --enable-bzip2 \ --enable-seccomp
>>> --enable-coroutine-pool --enable-glusterfs \ --enable-tpm
>>> --enable-libssh2 --enable-vhdx --enable-numa \ --enable-tcmalloc
>>> --target-list=x86_64-softmmu
>>>
>>> With modules disabled, there are 142 libraries loaded at startup.
>>> Time is the following:
>>>    LD time: 0.065 seconds
>>>    QEMU time: 0.02 seconds
>>>    Total time: 0.085 seconds
>>>
>>> With this patch series and modules enabled, there are 128 libraries
>>> loaded at startup. Time is the following:
>>>    LD time: 0.02 seconds
>>>    QEMU time: 0.02 seconds
>>>    Total time: 0.04 seconds
>>>
>>> Where LD time is the time between the program startup and the jump
>>> to main, and QEMU time is the time between the start of main and
>>> the first kvm_entry.
>>>
>>> These results are just with a few block drivers, that were already
>>> a module. Adding more modules (block or not block) should be easy,
>>> and will reduce the load time even more.
>>>
>>> Marc Marí (2):
>>>     Add dynamic module loading for block drivers
>>>     Add dynamic generation of module_block.h
>>>
>>>    .gitignore                      |   1 +
>>>    Makefile                        |  10 ++-
>>>    block.c                         |  70 +++++++++++++++++++++
>>>    configure                       |   2 +-
>>>    include/qemu/module.h           |   3 +
>>>    scripts/modules/module_block.py | 134
>>> ++++++++++++++++++++++++++++++++++++++++
>>> util/module.c                   |  38 ++++-------- 7 files changed,
>>> 227 insertions(+), 31 deletions(-) create mode 100755
>>> scripts/modules/module_block.py
>>>
>>   From my point of view the design looks a bit complex.
>> The approach should be quite similar to one used
>> in Linux kernel.
>>
>> If the block driver is configured as a module, block_init
>> should register proper hooks and create generic lists.
>> C code parsing does not look like a good approach
>> to me.
>>
>> If block_init is a bad name, we could use something like
>> module_init.
>>
> I think applying the kind of modules of the Linux kernel here would
> mean reworking an important part of the drivers that want to be
> converted into modules.
>
> If that's the case, I think it's better to have a less good solution
> that is still good enough, and doesn't mean reworking half of the QEMU
> codebase. Correct me if I'm wrong.
>
> But of course, this is not the only valid approach.
>
> Thanks
> Marc
Frankly speaking this does not look to me like a very big deal
and a huge rework. Build system should create 2 object files
for a modularized driver:
- one with driver stub which has required properties aka has_probe, name etc
   and a bit meaning that the driver is not present
- one with driver code
and from my point of view changes will not be big with this
approach.

Anyway, this is my opinion about this stuff. That is all, I not
against your change, I would like to note that there is a better
way which will allow further improvements.

Den