Re: [Qemu-devel] [PATCH 0/2] Dynamic module loading for block drivers
On Tue, Jun 21, 2016 at 06:59:58PM +0200, Paolo Bonzini wrote: > On 21/06/2016 17:42, Colin Lord wrote: > > It could easily return the BlockDriver then > > using the name field. It seems to me that this would work, and would be > > a fairly minor change from how things are now (in particular I think > > that symlinks wouldn't be necessary with this). > > Yes, I agree. I like that more than symlinks too. Regarding where names come from, in general: 1. From the format=, driver=, etc options during opening or creating images. Also from backing filenames inside image files. 2. From probing, see bdrv_find_protocol() and bdrv_find_format(). The simple approach to #2 is to load all modules, but this defeats the purpose of on-demand loading. There are solutions for #2 without loading all modules. You could move probing functions into the QEMU binary, for example, so that they don't require loading modules. Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 0/2] Dynamic module loading for block drivers
On 21/06/2016 17:42, Colin Lord wrote: > It could easily return the BlockDriver then > using the name field. It seems to me that this would work, and would be > a fairly minor change from how things are now (in particular I think > that symlinks wouldn't be necessary with this). Yes, I agree. paolo
Re: [Qemu-devel] [PATCH 0/2] Dynamic module loading for block drivers
On 21/06/2016 11:32, Stefan Hajnoczi wrote:
> I think the issue comes from the fact that you are considering something
> like load_block_module(const char *filename) as the API instead of
> request_block_driver(const char *driver_name). In the latter case it's
> possible to return a BlockDriver pointer. In the former it's not.
>
> The request_block_driver() approach requires a mapping from block driver
> names to modules. This can be achieved using a directory layout with
> symlinks (hmm...Windows portability?):
>
> /usr/lib/qemu/block/
> +--- sheepdog.so
> +--- by-protocol/
> +--- sheepdog+unix -> ../sheepdog.so
>
> request_block_driver() would look at
> /usr/lib/qemu/block/by-protocol/ to find the module file.
Another possibility is to add a ".loaded" element to the
block_driver_modules[] array and break the recursion (or infinite loop):
retry:
QLIST_FOREACH(drv1, &bdrv_drivers, list) {
if (!strcmp(drv1->format_name, format_name)) {
return drv1;
}
}
for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
if (!block_driver_modules[i].loaded &&
!strcmp(block_driver_modules[i].format_name, format_name)) {
block_driver_modules[i].loaded = true;
block_module_load_one(block_driver_modules[i].library_name);
goto retry;
}
}
BTW, please give a name to block_driver_modules[x]'s type, so that you
can assign &block_driver_modules[i] to a pointer and use that as a
shortcut.
Thanks,
Paolo
[Qemu-devel] [PATCH 0/2] Dynamic module loading for block drivers
This is a repost of some previous patches written by Marc Marí which were also reposted by Richard Jones a few months ago. The original series and reposted series are here: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg01995.html https://lists.nongnu.org/archive/html/qemu-devel/2016-04/msg01994.html I've tried to take some of the previous feedback and integrate it into this series. There are also a few things I haven't touched that were previously mentioned though: 1) Denis Lunev suggested having block_module_load_one return the loaded driver to reduce duplicated for loops in many of the functions in block.c. I'd be happy to do this but wasn't completely sure how error handling would happen in that case since currently the return value is an integer error code. Would I switch to using the error handling mechanisms provided in util/error.c? 2) In the past some debate was had about the design of the modules. I haven't made any changes in this regard because it didn't seem a conclusion had been reached. Some links for background: Fam Zheng suggested a simpler parser: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02201.html Denis Lunev suggested a design more similar to Linux kernel modules: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg05331.html Richard Jones suggested reasons to keep things as is: https://lists.nongnu.org/archive/html/qemu-devel/2016-04/msg01994.html Marc Mari (2): blockdev: Add dynamic generation of module_block.h blockdev: Add dynamic module loading for block drivers .gitignore | 1 + Makefile| 11 +++- block.c | 86 +++--- include/qemu/module.h | 3 + scripts/modules/module_block.py | 134 util/module.c | 37 +++ 6 files changed, 233 insertions(+), 39 deletions(-) create mode 100644 scripts/modules/module_block.py -- 2.5.5
