qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] qapi: What does "'gen': false" actually do, and when should


From: Markus Armbruster
Subject: [Qemu-devel] qapi: What does "'gen': false" actually do, and when should I use it? (was: Adding new migration-parameters - any easier way?)
Date: Tue, 09 Jun 2015 10:42:36 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux)

Two parts: 1. what does it do, and 2. when to use it.


Part 1: What does it do?

qapi-code-gen.txt explains it thus:

    In rare cases, QAPI cannot express a type-safe representation of a
    corresponding Client JSON Protocol command.  In these cases, if the
    command expression includes the key 'gen' with boolean value false,
    then the 'data' or 'returns' member that intends to bypass generated
    type-safety and do its own manual validation should use an inline
    dictionary definition, with a value of '**' rather than a valid type
    name for the keys that the generated code will not validate.  Please
    try to avoid adding new commands that rely on this, and instead use
    type-safe unions.  For an example of bypass usage:

     { 'command': 'netdev_add',
       'data': {'type': 'str', 'id': 'str', '*props': '**'},
       'gen': false }

I'm afraid that doesn't really answer the "what does it actually do?"
question, so let's examine an example.  netdev_add isn't the best choice
because it doesn't return anything, so let's use qom-get and qom-list.

{ 'command': 'qom-list',
  'data': { 'path': 'str' },
  'returns': [ 'ObjectPropertyInfo' ] }

{ 'command': 'qom-get',
  'data': { 'path': 'str', 'property': 'str' },
  'returns': '**',
  'gen': false }

qapi-commands.py generates qmp-marshal.c and qmp-commands.h.  For
qom-list, it generates qmp_marshal_input_qom_list().  For qom-get, it
generates nothing.

We still rely on qmp-commands.hx to bind the monitor to the schema.  For
qom-list, it binds to the generated function:

    {
        .name       = "qom-list",
        .args_type  = "path:s",
        .mhandler.cmd_new = qmp_marshal_input_qom_list,
    },

For qom-get, it binds to a hand-written one:

{
        .name       = "qom-get",
        .args_type  = "path:s,property:s",
        .mhandler.cmd_new = qmp_qom_get,
    },

Both are QMP command handlers, i.e their type is

    void (*cmd_new)(QDict *params, QObject **ret_data, Error **errp);

What does the generated handler do for us?  As the filename
qmp-marshal.c suggests, it's basically a wrapper around a command
handler with a "native C" interface that unmarshals from QDict to C
"native C" data types and back.  For qom-list, the native handler is

    ObjectPropertyInfoList *qmp_qom_list(const char *path, Error **errp)
    {
        // Do stuff with the arguments, create the result
        return props;
    }

This is the part you need to implement manually.

For qom-get, you implement this one instead:

    int qmp_qom_get(Monitor *mon, const QDict *qdict, QObject **ret)
    {
        // Unmarshall qdict into arguments
        const char *path = qdict_get_str(qdict, "path");
        const char *property = qdict_get_str(qdict, "property");
        Error *local_err = NULL;
        Object *obj;

        // Do stuff with the arguments, create the result

        // Marshal result into QObject
        // nothing to do in this case, because it happens to be one
        // already

    out:
        if (local_err) {
            // Go from Error API to QMP's QError
            // (QError will go away soon)
            qerror_report_err(local_err);
            error_free(local_err);
            return -1;
        }

        return 0;
    }

Note the boilerplate around 'do stuff'.

Notably absent from the boilerplate: rejecting unknown parameters,
checking mandatory parameters are present, checking types.  How come?

For historical reasons, part of the unmarshalling job is done by QMP
core, using args_type information from qmp-commands.hx.

For qom-list, .args_type = "path:s".  The core checks argument "path" is
present and is a JSON string, and no other argument is present.

For qom-get, .args_type = "path:s,property:s".  I trust you can guess
what the core checks there.

When we eliminate qmp-commands.hx, the core's checking goes away, and
needs to be replaced.

Generated handlers like qmp_marshal_input_qom_list() already do full
type checking of known parameters, so we just rely on that.  Handwritten
ones like qmp_qom_get() generally don't.  They'll need to be updated to
do it.


Part 2: When to use it?

We use 'gen': false when we can't (be bothered to) specify the exact
type of an argument or result.

Bad example: netdev_add

    We have arguments 'type': 'str' and '*props': '**'.

    We should have a union tagged by network backend type.  For each
    type, the union holds the type's properties (if any).

Better example: device_add (but that's not even QAPIfied, yet)

    If QAPIfied, we'd have arguments like 'driver': 'str' and '*props':
    '**'.

    Looks just like netdev_add.  The difference is that network backends
    and their properties are defined in one place, but device models and
    their properties aren't.  They get collected at run time.  As long
    as the schema is fixed at compile-time, it can't express the
    resulting tagged union.

Another good example: qom-get

    We have a return value '**'.

    The run time type is the type of the property identified by the
    arguments.  Therefore, the compile time type can only be the union
    of all property types, which effectively boils down to "anything".

    The only way to say "anything" right now is '**'.  Requires 'gen':
    false.  I figure we could extend the generators to support '**' in a
    few places, which may let us avoid 'gen': false here.

Drawback of '**': introspection knows nothing.

Introspection knowing nothing about netdev_add's and device_add's
acceptable properties is a big, painful gap.

Please don't invent new reasons for 'gen': false without a very
compelling use case.  If you think you have one, we need to talk to make
sure use of 'gen': false really beats the alternatives.  Alternatives
may include extending the generators.



reply via email to

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