Re: [Qemu-devel] [PATCH v2 REPOST 1/2] Add dynamic module loading for block drivers

[Stefan Hajnoczi]

On Tue, Apr 12, 2016 at 02:41:13PM +0100, Richard W.M. Jones wrote:
> From: Marc Marí <address@hidden>
> 
> Extend the current module interface to allow for block drivers to be loaded
> dynamically on request.

I like this approach to run-time loading QEMU modules because it's not a
plugin system that would inevitably be abused by license violators.

> The only block drivers that can be converted into modules are the drivers
> that don't perform any init operation except for registering themselves. This
> is why libiscsi has been disabled as a module.

This libiscsi issue is easy to solve.  Compile QemuOptsList
qemu_iscsi_opts into the main binary.  Please do it in a separate
commit.

> @@ -2811,6 +2865,22 @@ void bdrv_iterate_format(void (*it)(void *opaque, 
> const char *name),
>          }
>      }
>  
> +    for (n = 0; n < ARRAY_SIZE(block_driver_modules); ++n) {
> +        if (block_driver_modules[n].format_name) {
> +            bool found = false;
> +            int i = count;
> +            while (formats && i && !found) {
> +                found = !strcmp(formats[--i],
> +                                block_driver_modules[n].format_name);
> +            }
> +
> +            if (!found) {
> +                formats = g_renew(const char *, formats, count + 1);
> +                formats[count++] = block_driver_modules[n].format_name;
> +            }
> +        }
> +    }

There is code duplication in other hunks but this is where I'd draw the
line.  I can be done without copy-paste:

static const char **add_format(const char **formats, int *count,
                               const char *format_name)
{
    int i;

    while (i = 0; i < *count; i++) {
        if (!strcmp(formats[i], format_name) {
            return;
        }
    }

    *count += 1;
    formats = g_renew(const char *, formats, *count);
    formats[*count] = format_name;
    return formats;
}

...

QLIST_FOREACH(drv, &bdrv_drivers, list) {
    if (drv->format_name) {
        formats = add_format(formats, &count, drv->format_name);
    }
}

for (n = 0; n < ARRAY_SIZE(block_driver_modules); ++n) {
    if (block_driver_modules[n].format_name) {
        formats = add_format(formats, &count,
                             block_driver_modules[n].format_name);
    }
}

signature.asc
Description: PGP signature