Re: [Qemu-devel] [PATCH 04/21] qmp: Dumb down how we run QMP command registration

[Eric Blake]

On 02/23/2017 03:45 PM, Markus Armbruster wrote:
> The way we get QMP commands registered is high tech:
> 
> * qapi-commands.py generates qmp_init_marshal() that does the actual work
> 
> * it also generates the magic to register it as a MODULE_INIT_QAPI
>   function, so it runs when someone calls
>   module_call_init(MODULE_INIT_QAPI)
> 
> * main() calls module_call_init()
> 
> QEMU needs to register a few non-qapified commands.  Same high tech
> works: monitor.c has its own qmp_init_marshal() along with the magic
> to make it run in module_call_init(MODULE_INIT_QAPI).

Eww. Two static functions with the same name, which really messes with
gdb debugging.

> 
> QEMU also needs to unregister commands that are not wanted in this
> build's configuration (commit 5032a16).  Simple enough:
> qmp_unregister_commands_hack().  The difficulty is to make it run
> after the generated qmp_init_marshal().  We can't simply run it in
> monitor.c's qmp_init_marshal(), because the order in which the
> registered functions run is indeterminate.  So qmp_init_marshal()
> registers qmp_init_marshal() separately.  Since registering *appends*

Another case of "A sets up A" when you meant "A sets up B", but this
time, I'm fairly certain you mean qmp_init_marshal() registers
qmp_unregister_commands_hack() separately.

> to the list of registered functions, this will make it run after all
> the functions that have been registered already.
> 
> I suspect it takes a long and expensive computer science education to
> not find this silly.

ROFL
(does that mean I didn't spend enough on my education? I'm sure you
could arrange to sell me an online certificate if I wanted more... :)

> 
> Dumb it down as follows:
> 
> * Drop MODULE_INIT_QAPI entirely
> 
> * Give the generated qmp_init_marshal() external linkage.
> 
> * Call it instead of module_call_init(MODULE_INIT_QAPI)
> 
> * Except in QEMU proper, call new monitor_init_qmp_commands() that in
>   turn calls the generated qmp_init_marshal(), registers the
>   additional commands and unregisters the unwanted ones.
> 
> Signed-off-by: Markus Armbruster <address@hidden>
> ---

> -static void qmp_init_marshal(void)
> +void monitor_init_qmp_commands(void)
>  {
> +    qmp_init_marshal();
> +
>      qmp_register_command("query-qmp-schema", qmp_query_qmp_schema,
>                           QCO_NO_OPTIONS);
>      qmp_register_command("device_add", qmp_device_add,
> @@ -1004,12 +1006,9 @@ static void qmp_init_marshal(void)
>      qmp_register_command("netdev_add", qmp_netdev_add,
>                           QCO_NO_OPTIONS);
>  
> -    /* call it after the rest of qapi_init() */
> -    register_module_init(qmp_unregister_commands_hack, MODULE_INIT_QAPI);
> +    qmp_unregister_commands_hack();

We could inline this function now, but keeping the _hack() name around
reminds us to consider conditional-compilation support in the generator
at a later date, so I'm fine with the approach you took.

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature