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 <arm...@redhat.com>
> ---
> -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 <ebl...@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
