[Top][All Lists]

[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: Colin Lord
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH v5 0/3] Dynamic module loading for block drivers
Date: Mon, 25 Jul 2016 09:56:26 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1

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.

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?


> 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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]