[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v5 0/3] Dynamic module loading for
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v5 0/3] Dynamic module loading for block drivers |
Date: |
Mon, 25 Jul 2016 21:38:59 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 |
On 25.07.2016 15:56, Colin Lord wrote:
> On 07/23/2016 02:21 PM, Max Reitz wrote:
>> On 20.07.2016 16:30, Colin Lord wrote:
>>> Here's v5 of the modularization series. Since it seems the concensus is
>>> that modularizing the format drivers is unnecessary, this series no
>>> longer modularizes those and is thus much shorter than before.
>>>
>>> v5:
>>> - No format drivers are modularized, therefore the probe functions are
>>> all being left completely untouched. The bdrv_find_format function is
>>> also left untouched as a result.
>>
>> You also left the (host) device probing functions untouched in this
>> revision. However, those are actually only used by protocol drivers
>> (raw-posix and raw-win32, to be specific).
>>
>> Probably fine since I think it's impossible to build raw-posix or
>> raw-win32 as a module anyway (because bdrv_file is used as a "static"
>> reference in block.c).
>>
>>> - Remove dmg from block-obj-m since it is not a target of the
>>> modularization effort.
>>
>> Hm, I'm afraid I don't quite understand the reasoning behind this.
>> Intuitively, I'd say "Doesn't matter, it was already modular, so what
>> prevents it from staying that way?"
>>
>> Is it because the changes to util/module.c in patch 3 break how the dmg
>> module worked, e.g. that it was always implicitly fully loaded on qemu
>> startup if it was available, but now modules are only loaded on request?
>>
> Yes, that's pretty much it. Previously all the modules would get loaded
> during initialization, but since the third patch adds dynamic loading
> that no longer happens. As we aren't loading format drivers on demand,
> dmg.o should be added to block-obj-y instead of block-obj-m so that it
> doesn't get modularized.
OK, then, for the series:
Reviewed-by: Max Reitz <address@hidden>
> Also, I should mention that the third patch of this series is not quite
> right. I was looking at some stuff with John on Friday and he found a
> couple lines I somehow didn't delete from qemu/Makefile (around line 250):
>
> block-modules = $(foreach o,$(block-obj-m),"$(basename $(subst
> /,-,$o))",) NULL
> util/module.o-cflags = -D'CONFIG_BLOCK_MODULES=$(block-modules)'
>
> I was pretty sure I had taken care of these but I guess not. It doesn't
> actually affect anything as all it's doing is setting
> CONFIG_BLOCK_MODULES (which is no longer used anywhere once patch 3 gets
> applied), but it's obviously not good to have unused code sitting
> around. Should I submit another version with this fixed?
While these lines should eventually of course be removed, this can
simply be done in a follow-up patch just as well.
Max
>
> Colin
>
>> Max
>>
>>> - Modify module_block.py to only include the library name and protocol
>>> name fields in the generated struct. The other fields are no longer
>>> necessary for the drivers that are being modularized.
>>>
>>> v4:
>>> - Fix indentation of the generated header file module_block.h
>>> - Drivers and probe functions are now all located in the block/
>>> directory, rather than being split between block/ and block/probe/. In
>>> addition the header files for each probe/driver pair are in the block/
>>> directory, not the include/block/driver/ directory (which no longer
>>> exists).
>>> - Since the probe files are in block/ now, they follow the naming
>>> pattern of format-probe.c
>>> - Renamed crypto probe file to be crypto-probe.c, luks is no longer in
>>> the filename
>>> - Fixed formatting of parallels_probe() function header
>>> - Enforced consistent naming convention for the probe functions. They
>>> now follow the pattern bdrv_format_probe().
>>>
>>> Colin Lord (1):
>>> blockdev: prepare iSCSI block driver for dynamic loading
>>>
>>> Marc Mari (2):
>>> blockdev: Add dynamic generation of module_block.h
>>> blockdev: Add dynamic module loading for block drivers
>>>
>>> Makefile | 7 +++
>>> block.c | 37 ++++++++++++---
>>> block/Makefile.objs | 3 +-
>>> block/iscsi.c | 36 --------------
>>> include/qemu/module.h | 3 ++
>>> scripts/modules/module_block.py | 102
>>> ++++++++++++++++++++++++++++++++++++++++
>>> util/module.c | 38 +++++----------
>>> vl.c | 38 +++++++++++++++
>>> 8 files changed, 193 insertions(+), 71 deletions(-)
>>> create mode 100644 scripts/modules/module_block.py
>>>
>>
>>
>
signature.asc
Description: OpenPGP digital signature