Re: QEMU API cleanup initiative - Let's chat during the KVM call

[Paolo Bonzini]

On 05/10/20 16:52, John Snow wrote:
> - Markus considers the platonic ideal of a CLI one in which each flag is
> a configuration directive, and each directive that references another
> directive must appear after the directive in which it references.
> 
> - I tend to consider the ideal configuration to be a static object that
> has no inherent order from one key to the next, e.g. a JSON object or
> static flat file that can be used to configure the sysemu.
> 
> They're not compatible visions; and they have implications for error
> ordering and messages and so on.

I think they aren't incompatible.  Even your idea would probably forbid
cycles, so it only takes a topological sort to go from an unordered
configuration to an ordered one.  The only difference is whether it's
the user or the program that does it.

> For the meantime, Markus's vision is closer to what QEMU already does,
> so it's likely the winning answer for now and if a conversion to the
> other idea is required at a time later, we'll have to tackle it then. (I
> think.)
> 
> It's a good subject of discussion. (Also relevant: if parsing is to
> occur in more than the CLI context, the existing contextual CLI parser
> error system needs to be reworked to avoid monitor-unsafe error calls.
> It's not trivial, I think.)

I think parsing should occur in CLI context only, but errors can occur
elsewhere too.

I think the idea for this kind of refactoring is always to first make
the code behave the way you want, and only then change the
implementation to look the way you want.

Currently we have:

    switch (...) {
        case QEMU_OPT_...:
            /* something has side effects, something is just parsing */
    }

    init1();
    qemu_opts_foreach(something_opts, configure_something);
    init2();
    qemu_opts_foreach(some_more_opts, configure_some_more);
    init3();

    enter_preconfig();

We should first of all change it to

    parse_command_line() {
        apply_simple_options()l
        qemu_opts_foreach(something_opts, configure_something);
        qemu_opts_foreach(some_more_opts, configure_some_more);
    }

    switch (...) {
        case QEMU_OPT_...:
        /* no side effects on the initN() calls below */
    }

    init1();
    init2();
    init3();

    parse_command_line()

    enter_preconfig();

    more_init_that_needs_side_effects();

This way, the parse_command_line() and its qemu_opts_foreach callbacks
can become changed into a series of qmp_*() commands.  The commands can
be called within the appropriate loc_push() though.

Problem is, it's 1000 lines of initialization interspersed with
qemu_opts_foreach calls.  But it's doable.

Paolo