All of lore.kernel.org
 help / color / mirror / Atom feed
* QEMU API cleanup initiative - Let's chat during the KVM call
@ 2020-10-04  0:14 John Snow
  2020-10-05 13:45 ` Stefan Hajnoczi
  2020-10-05 15:33 ` John Snow
  0 siblings, 2 replies; 8+ messages in thread
From: John Snow @ 2020-10-04  0:14 UTC (permalink / raw)
  To: QEMU Developers
  Cc: Kevin Wolf, Peter Maydell, Daniel P. Berrangé,
	Eduardo Habkost, Juan Quintela, Markus Armbruster,
	Andrea Bolognani, Gerd Hoffmann, Stefan Hajnoczi,
	Marc-André Lureau, Paolo Bonzini

Hi, everyone! I'd like to discuss some of this in the upstream KVM call.

TLDR: I would like to begin an organized effort to consolidate our CLI 
parsing, moving it onto QAPI. I'd like to talk about how we should 
proceed on the KVM call, prior to KVM Forum, where we should continue 
these discussions.


Background
----------

You may recall late last December that Stefan started a big discussion 
thread about Modernizing the QEMU API:
"Making QEMU easier for management tools and applications" [1]

There were lots of opinions and directions proposed for this, with many 
competing visions for where QEMU should go, or what it ought endeavor to be.

Though many of these visions conflict in terms of the implementations 
for their end goal, many shared a similar logical end-goal and shared 
some concrete intermediate steps. One of those concrete intermediate 
steps is the consolidation of our configuration and startup mechanisms 
into a unified API.


The QEMU API, Today
------------------

At the moment, QAPI is our most formal system for declaring types, 
structures and interfaces. I believe QAPI is not going anywhere, and so 
I am doubling down and committing to improving and expanding the QAPI 
subsystem.

I wanted to understand what QEMU's existing interface even *was*. We can 
understand QEMU's interface to be four components presently:

1. The QEMU Monitor Protocol (QMP)

QMP is based explicitly on top of QAPI, which we do indeed have formal 
specifications for in ./qapi. They are not standards-compliant, but they 
are at least unified.

2. The GTK UI

The GTK UI offers very minimal interface, and does not offer any feature 
that is not available through one of the other interfaces or standard 
operating system UI. Good!

3. The Human Monitor Protocol (HMP)

HMP is increasingly based on QMP, though the conversion is not complete 
and it is not clear if it will be "complete". This was a major sub-topic 
of the thread last December with no clear consensus. Some work continues 
to bring major HMP features over to QMP; notably Daniel Berrange has 
been trying to port savevm/loadvm over to QMP [2]. For now, it seems 
like HMP will be with us at least as a debugging and development 
interface. There is work to be done to audit and inventory any remaining 
features that must be ported to QMP, which are reundant with QMP, and 
which are uniquely useful as debugging interfaces.

4. The QEMU command-line

Last, we have the QEMU CLI. This interface was grown organically over 
time and features many different parser subsystems and loosely federated 
components with no unified specification document that explains the 
entire shape of the CLI.


Auditing the CLI interface
--------------------------

I wanted to know what our CLI really looked like. Not trusting any of 
our existing documentation to be complete/authoritative, I used the QEMU 
5.1 release as a basis and audited the entirety of that interface. [3]

Here's what I found:

- QEMU 5.1 has 131 command line flags
   - 93 of these take an argument
   - 38 of them do not.

If we want to unify the parsing into a single formal declaration, it 
would be helpful to know what we're dealing with. Of those flags that 
take arguments:

- 3 use QAPI to parse their argument directly
- 51 use QemuOpts in some way:
   - 36 use qemu_opts_parse[_noisily] directly
   - 10 desugar to `qemu_opts_parse_noisily` (-fdc, -hda, et al)
   - 5 add a single option using `qemu_opt_set`.
- 1 is parsed rather directly by QOM (-tb-size)
- 14 are stored directly as (const char *)
- 3 are parsed into numerical types with atoi/strtol/etc.
- 21 are parsed by custom parsing mechanisms.

For full, gory details, please see the document referenced at [3]. It's 
about 4000 lines of markdown detailing the QAPI/C structures that define 
each command line argument, as well as some fairly detailed analyses of 
the custom parsers and exactly which values they accept.


I'm not reading a 4,000+ line markdown document
-----------------------------------------------

Good news! I made a summary spreadsheet to summarize what I found. [4]

This spreadsheet summarizes the types of arguments we have and what 
parsers they utilize and their support status. The spreadsheet follows 
the order of flags as defined in qemu-options.hx, category-by-category.

I also tried to broadly assign "topics" to each flag for the purposes of 
laying out a better manual in the future, but I wasn't fully confident 
in many flags that affect things like boot, firmware, chipset, etc, so 
this is a work in progress.

https://docs.google.com/spreadsheets/d/1OJvzDwLpo2XsGytNp9Pr-vYIrb0pWrFKBNPtOyH80ew/edit?usp=sharing

If you don't have google, I have an ODS exported version of this 
spreadsheet too -- feel free to relay your feedback back to me here. [5]

Paolo Bonzini helped re-organize my initial draft and used it to 
identify flags perhaps most in need of attention to bring onto a new 
standard, annotated in yellow.

(Those items are: -k, -uuid, -no-hpet, -no-reboot, -no-shutdown, 
-incoming-, and -enable-fips.)

Of the remainder, quite a few are either already deprecated, are 
aliases, or are simple sugar for another command that could be expressed 
more compactly. Quite a few are already using *at least* QemuOpts such 
that porting them to QAPI should not be extremely mechanically difficult.

I would like to use the KVM call to discuss a roadmap for converting the 
remaining options to QAPI, what that would take, and who will take 
ownership for which subsystems/flags. I would like to bring these 
discussions to KVM Forum and lend serious, dedicated effort to finishing 
this task.


Related work and ongoing efforts
--------------------------------

Mentioned above, Daniel Berrange is porting HMP features to QMP [2].

I am adding python static typing to our QAPI generator in the belief 
that QAPI will continue to grow in importance for us, and inviting more 
developers to participate in writing QAPI generator backends by 
formalizing that interface. [6].

I am prototyping a new QAPI generator backend that produces Json-Schema, 
attempting to target various SDK generators that are compatible with 
e.g. OpenAPI (which uses a modified version Json-Schema as a 
sub-specification.)

Eduardo Habkost is working on making all QOM type definitions fully data 
driven, in the hopes that we might eventually be able to integrate these 
types with QAPI to eliminate stub types from the API. [7]

Marc-André is adding a Rust backend to the QAPI generator, along with a 
new API frontend that can communicate with dbus. [8]


--

[1] https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg04840.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg00587.html
[3] https://gitlab.com/jsnow/qemu/-/blob/cli_audit/docs/cli_audit.md
[4] 
https://docs.google.com/spreadsheets/d/1OJvzDwLpo2XsGytNp9Pr-vYIrb0pWrFKBNPtOyH80ew/edit?usp=sharing
[5] http://people.redhat.com/~jsnow/qemu-5_1-audit.ods
[6] https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg10723.html
[7] https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg08304.html 
(And more to come)
[8] https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg03971.html



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: QEMU API cleanup initiative - Let's chat during the KVM call
  2020-10-04  0:14 QEMU API cleanup initiative - Let's chat during the KVM call John Snow
@ 2020-10-05 13:45 ` Stefan Hajnoczi
  2020-10-05 14:52   ` John Snow
  2020-10-05 15:33 ` John Snow
  1 sibling, 1 reply; 8+ messages in thread
From: Stefan Hajnoczi @ 2020-10-05 13:45 UTC (permalink / raw)
  To: John Snow
  Cc: Kevin Wolf, Peter Maydell, Daniel P. Berrangé,
	Eduardo Habkost, Juan Quintela, Markus Armbruster,
	QEMU Developers, Gerd Hoffmann, Andrea Bolognani,
	Marc-André Lureau, Paolo Bonzini

[-- Attachment #1: Type: text/plain, Size: 972 bytes --]

On Sat, Oct 03, 2020 at 08:14:11PM -0400, John Snow wrote:
> I would like to use the KVM call to discuss a roadmap for converting the
> remaining options to QAPI, what that would take, and who will take ownership
> for which subsystems/flags. I would like to bring these discussions to KVM
> Forum and lend serious, dedicated effort to finishing this task.

Subsystem maintainers will need to review these patches. Hopefully many
of them are willing to do the conversion themselves. Code examples for
common conversions would help (e.g. QemuOpts to QAPI, strtol() to QAPI,
etc).

Do error messages need to be preserved? For example, maybe the input
validation order or the actual error message is different with QAPI and
that may affect programs that launch QEMU.

Is there any way to detect incompatible command-line changes besides
running make check? One idea is to run a fuzzer to check if certain
inputs are accepted by the new/old version but not the other.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: QEMU API cleanup initiative - Let's chat during the KVM call
  2020-10-05 13:45 ` Stefan Hajnoczi
@ 2020-10-05 14:52   ` John Snow
  2020-10-06  9:30     ` Paolo Bonzini
  2020-10-06  9:50     ` Daniel P. Berrangé
  0 siblings, 2 replies; 8+ messages in thread
From: John Snow @ 2020-10-05 14:52 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Kevin Wolf, Peter Maydell, Daniel P. Berrangé,
	Eduardo Habkost, Juan Quintela, Markus Armbruster,
	QEMU Developers, Gerd Hoffmann, Andrea Bolognani,
	Marc-André Lureau, Paolo Bonzini

On 10/5/20 9:45 AM, Stefan Hajnoczi wrote:
> On Sat, Oct 03, 2020 at 08:14:11PM -0400, John Snow wrote:
>> I would like to use the KVM call to discuss a roadmap for converting the
>> remaining options to QAPI, what that would take, and who will take ownership
>> for which subsystems/flags. I would like to bring these discussions to KVM
>> Forum and lend serious, dedicated effort to finishing this task.
> 
> Subsystem maintainers will need to review these patches. Hopefully many
> of them are willing to do the conversion themselves. Code examples for
> common conversions would help (e.g. QemuOpts to QAPI, strtol() to QAPI,
> etc).
> 

Yes, and this depends on how far exactly we want to go on our first 
conversion pass. The exact point we pick as our first intermediate step 
might determine these common conversions.

QemuOpts might be a reasonable first step, or maybe we want to go all 
the way straight to QAPI.

In a few cases (like -cpu), we probably want to start normalizing the 
way in which models are lookup up and defined; some architectures allow 
you to lowercase the models, or perform other text mapping conversions.

We could start deprecating / standardizing these transformations to try 
and unify the CPU parsing infrastructure which would give us a chance to 
describe it all with one set of rules.

Some of this depends on Markus's existing patches -- He has a series 
from 2018 (IIRC) that attempts a lot of the low-hanging fruit for this 
work, and that might serve as a reasonable basis.

Things to discuss.

> Do error messages need to be preserved? For example, maybe the input
> validation order or the actual error message is different with QAPI and
> that may affect programs that launch QEMU.
> 

It's a good point to talk about.

- 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.

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.)

> Is there any way to detect incompatible command-line changes besides
> running make check? One idea is to run a fuzzer to check if certain
> inputs are accepted by the new/old version but not the other.
> 

That might be interesting. I did some fairly thorough auditing to 
understand what each flag actually accepts at-present, but I don't have 
good experience with any fuzzer such to be able to model that exactly.

I'd be happy to hear about ways we could try to model this. Possibly 
intentionally deprecating things to reduce the size of some of our 
weirder parsers might help the modeling effort, too.

> Stefan
> 

--js



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: QEMU API cleanup initiative - Let's chat during the KVM call
  2020-10-04  0:14 QEMU API cleanup initiative - Let's chat during the KVM call John Snow
  2020-10-05 13:45 ` Stefan Hajnoczi
@ 2020-10-05 15:33 ` John Snow
  1 sibling, 0 replies; 8+ messages in thread
From: John Snow @ 2020-10-05 15:33 UTC (permalink / raw)
  To: QEMU Developers
  Cc: Kevin Wolf, Peter Maydell, Daniel P. Berrangé,
	Eduardo Habkost, Juan Quintela, Markus Armbruster,
	Andrea Bolognani, Gerd Hoffmann, Stefan Hajnoczi,
	Marc-André Lureau, Paolo Bonzini

On 10/3/20 8:14 PM, John Snow wrote:
> Hi, everyone! I'd like to discuss some of this in the upstream KVM call.
> 
> TLDR: I would like to begin an organized effort to consolidate our CLI 
> parsing, moving it onto QAPI. I'd like to talk about how we should 
> proceed on the KVM call, prior to KVM Forum, where we should continue 
> these discussions.
> 

Call details:

( https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg00997.html )

 > You can join the call from your web browser here:
 > https://bluejeans.com/497377100
 >
 > Meeting ID: 497377100
 > Phone numbers: https://www.redhat.com/en/conference-numbers

The call is at 13:00 UTC:

- 09:00 EDT
- 15:00 CEST


Intended area of focus:

- What work needs to be done to move the CLI onto QAPI
- How do we measure/document this conversion progress?
- What are the criteria for a complete conversion?
- How do we distribute this work?


(See quoted below for context/detail)


> 
> Background
> ----------
> 
> You may recall late last December that Stefan started a big discussion 
> thread about Modernizing the QEMU API:
> "Making QEMU easier for management tools and applications" [1]
> 
> There were lots of opinions and directions proposed for this, with many 
> competing visions for where QEMU should go, or what it ought endeavor to 
> be.
> 
> Though many of these visions conflict in terms of the implementations 
> for their end goal, many shared a similar logical end-goal and shared 
> some concrete intermediate steps. One of those concrete intermediate 
> steps is the consolidation of our configuration and startup mechanisms 
> into a unified API.
> 
> 
> The QEMU API, Today
> ------------------
> 
> At the moment, QAPI is our most formal system for declaring types, 
> structures and interfaces. I believe QAPI is not going anywhere, and so 
> I am doubling down and committing to improving and expanding the QAPI 
> subsystem.
> 
> I wanted to understand what QEMU's existing interface even *was*. We can 
> understand QEMU's interface to be four components presently:
> 
> 1. The QEMU Monitor Protocol (QMP)
> 
> QMP is based explicitly on top of QAPI, which we do indeed have formal 
> specifications for in ./qapi. They are not standards-compliant, but they 
> are at least unified.
> 
> 2. The GTK UI
> 
> The GTK UI offers very minimal interface, and does not offer any feature 
> that is not available through one of the other interfaces or standard 
> operating system UI. Good!
> 
> 3. The Human Monitor Protocol (HMP)
> 
> HMP is increasingly based on QMP, though the conversion is not complete 
> and it is not clear if it will be "complete". This was a major sub-topic 
> of the thread last December with no clear consensus. Some work continues 
> to bring major HMP features over to QMP; notably Daniel Berrange has 
> been trying to port savevm/loadvm over to QMP [2]. For now, it seems 
> like HMP will be with us at least as a debugging and development 
> interface. There is work to be done to audit and inventory any remaining 
> features that must be ported to QMP, which are reundant with QMP, and 
> which are uniquely useful as debugging interfaces.
> 
> 4. The QEMU command-line
> 
> Last, we have the QEMU CLI. This interface was grown organically over 
> time and features many different parser subsystems and loosely federated 
> components with no unified specification document that explains the 
> entire shape of the CLI.
> 
> 
> Auditing the CLI interface
> --------------------------
> 
> I wanted to know what our CLI really looked like. Not trusting any of 
> our existing documentation to be complete/authoritative, I used the QEMU 
> 5.1 release as a basis and audited the entirety of that interface. [3]
> 
> Here's what I found:
> 
> - QEMU 5.1 has 131 command line flags
>    - 93 of these take an argument
>    - 38 of them do not.
> 
> If we want to unify the parsing into a single formal declaration, it 
> would be helpful to know what we're dealing with. Of those flags that 
> take arguments:
> 
> - 3 use QAPI to parse their argument directly
> - 51 use QemuOpts in some way:
>    - 36 use qemu_opts_parse[_noisily] directly
>    - 10 desugar to `qemu_opts_parse_noisily` (-fdc, -hda, et al)
>    - 5 add a single option using `qemu_opt_set`.
> - 1 is parsed rather directly by QOM (-tb-size)
> - 14 are stored directly as (const char *)
> - 3 are parsed into numerical types with atoi/strtol/etc.
> - 21 are parsed by custom parsing mechanisms.
> 
> For full, gory details, please see the document referenced at [3]. It's 
> about 4000 lines of markdown detailing the QAPI/C structures that define 
> each command line argument, as well as some fairly detailed analyses of 
> the custom parsers and exactly which values they accept.
> 
> 
> I'm not reading a 4,000+ line markdown document
> -----------------------------------------------
> 
> Good news! I made a summary spreadsheet to summarize what I found. [4]
> 
> This spreadsheet summarizes the types of arguments we have and what 
> parsers they utilize and their support status. The spreadsheet follows 
> the order of flags as defined in qemu-options.hx, category-by-category.
> 
> I also tried to broadly assign "topics" to each flag for the purposes of 
> laying out a better manual in the future, but I wasn't fully confident 
> in many flags that affect things like boot, firmware, chipset, etc, so 
> this is a work in progress.
> 
> https://docs.google.com/spreadsheets/d/1OJvzDwLpo2XsGytNp9Pr-vYIrb0pWrFKBNPtOyH80ew/edit?usp=sharing 
> 
> 
> If you don't have google, I have an ODS exported version of this 
> spreadsheet too -- feel free to relay your feedback back to me here. [5]
> 
> Paolo Bonzini helped re-organize my initial draft and used it to 
> identify flags perhaps most in need of attention to bring onto a new 
> standard, annotated in yellow.
> 
> (Those items are: -k, -uuid, -no-hpet, -no-reboot, -no-shutdown, 
> -incoming-, and -enable-fips.)
> 
> Of the remainder, quite a few are either already deprecated, are 
> aliases, or are simple sugar for another command that could be expressed 
> more compactly. Quite a few are already using *at least* QemuOpts such 
> that porting them to QAPI should not be extremely mechanically difficult.
> 
> I would like to use the KVM call to discuss a roadmap for converting the 
> remaining options to QAPI, what that would take, and who will take 
> ownership for which subsystems/flags. I would like to bring these 
> discussions to KVM Forum and lend serious, dedicated effort to finishing 
> this task.
> 
> 
> Related work and ongoing efforts
> --------------------------------
> 
> Mentioned above, Daniel Berrange is porting HMP features to QMP [2].
> 
> I am adding python static typing to our QAPI generator in the belief 
> that QAPI will continue to grow in importance for us, and inviting more 
> developers to participate in writing QAPI generator backends by 
> formalizing that interface. [6].
> 
> I am prototyping a new QAPI generator backend that produces Json-Schema, 
> attempting to target various SDK generators that are compatible with 
> e.g. OpenAPI (which uses a modified version Json-Schema as a 
> sub-specification.)
> 
> Eduardo Habkost is working on making all QOM type definitions fully data 
> driven, in the hopes that we might eventually be able to integrate these 
> types with QAPI to eliminate stub types from the API. [7]
> 
> Marc-André is adding a Rust backend to the QAPI generator, along with a 
> new API frontend that can communicate with dbus. [8]
> 
> 
> -- 
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg04840.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg00587.html
> [3] https://gitlab.com/jsnow/qemu/-/blob/cli_audit/docs/cli_audit.md
> [4] 
> https://docs.google.com/spreadsheets/d/1OJvzDwLpo2XsGytNp9Pr-vYIrb0pWrFKBNPtOyH80ew/edit?usp=sharing 
> 
> [5] http://people.redhat.com/~jsnow/qemu-5_1-audit.ods
> [6] https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg10723.html
> [7] https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg08304.html 
> (And more to come)
> [8] https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg03971.html



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: QEMU API cleanup initiative - Let's chat during the KVM call
  2020-10-05 14:52   ` John Snow
@ 2020-10-06  9:30     ` Paolo Bonzini
  2020-10-06  9:40       ` Daniel P. Berrangé
  2020-10-06  9:50     ` Daniel P. Berrangé
  1 sibling, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2020-10-06  9:30 UTC (permalink / raw)
  To: John Snow, Stefan Hajnoczi
  Cc: Kevin Wolf, Peter Maydell, Daniel P. Berrangé,
	Eduardo Habkost, Juan Quintela, Markus Armbruster,
	QEMU Developers, Gerd Hoffmann, Andrea Bolognani,
	Marc-André Lureau

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



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: QEMU API cleanup initiative - Let's chat during the KVM call
  2020-10-06  9:30     ` Paolo Bonzini
@ 2020-10-06  9:40       ` Daniel P. Berrangé
  2020-10-06  9:53         ` Paolo Bonzini
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel P. Berrangé @ 2020-10-06  9:40 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Kevin Wolf, Peter Maydell, Eduardo Habkost, Juan Quintela,
	Markus Armbruster, QEMU Developers, Gerd Hoffmann,
	Stefan Hajnoczi, Marc-André Lureau, Andrea Bolognani,
	John Snow

On Tue, Oct 06, 2020 at 11:30:20AM +0200, Paolo Bonzini wrote:
> 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.

I feel that both of these approaches are equally broken, as they don't
honour the order in which arguments are passed by the caller when
creating resources.

This leads to the crazy hacks we have with -object where we have to
create certain objects at certain stages.

To make QEMU CLI parsing sane we need to be able to create objects as
we parse them.

   while (n++ < argc) {
        switch (argv[n]) {
           case QEMU_OPT_foo:
             ...parse argv[n]...
             ...create argv[n]...
        }
   }

IOW, all usage of 'qemu_opts_foreach' should be targetted for complete
elimination.

I'm not convinced that your proposed change takes us in direction, if
anything it is encoding the current split of parsing vs creation even
more strongly.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: QEMU API cleanup initiative - Let's chat during the KVM call
  2020-10-05 14:52   ` John Snow
  2020-10-06  9:30     ` Paolo Bonzini
@ 2020-10-06  9:50     ` Daniel P. Berrangé
  1 sibling, 0 replies; 8+ messages in thread
From: Daniel P. Berrangé @ 2020-10-06  9:50 UTC (permalink / raw)
  To: John Snow
  Cc: Kevin Wolf, Peter Maydell, Eduardo Habkost, Juan Quintela,
	Markus Armbruster, QEMU Developers, Gerd Hoffmann,
	Stefan Hajnoczi, Marc-André Lureau, Paolo Bonzini,
	Andrea Bolognani

On Mon, Oct 05, 2020 at 10:52:41AM -0400, 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.

In this view you would be creating resources in the order in which they
appear in the understanding that the mgmt app knows what dependancies
it placed between cli options. ie it knows that it wants "-object secret"
created before "-chardev foo", because the chardev depends on the secret
to exist.

Despite the fact that QEMU does not correctly honour the CLI arg order
today, libvirt will place arguments in the order in which they must be
created already. IOW, we know the order they appear in the CLI or
configuration will always work correctly, as long as QEMU honours it.

QEMU has had todo various hacks to deal with the fact that it doesn't
honour ordering, such as creating different -object types at different
stages in startup. This is a really horrible part of QEMU that constantly
causes us pain.

> - 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.

If you treat the configuration as un-ordered, then QEMU needs to be
intelligent enough to figure out the correct order to create all the
resources in. This requires some pieces of code to have a complete
view of all configuration, and know which attrs express dependencies.
It then has to be able to traverse the configuration in the correct
topological sorted order to create things.

Anything is possible, but from where QEMU is starting right now this
feels like a massive task to put in front of ourselves. We basically
have to solve the entire global configuration problem in order to get
this working as you can only do the topological sorting if you can see
the full picture.

The concern I have is that it also injects an element of non-determinism
into the startup procedure that can make things painful to debug. eg there
are theoretically many possible sort orders that are correct for creation,
but if things have unexpected side-effects during creation, these different
orders will not be strictly equivalent in reality.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: QEMU API cleanup initiative - Let's chat during the KVM call
  2020-10-06  9:40       ` Daniel P. Berrangé
@ 2020-10-06  9:53         ` Paolo Bonzini
  0 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2020-10-06  9:53 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Kevin Wolf, Peter Maydell, Eduardo Habkost, Juan Quintela,
	Markus Armbruster, QEMU Developers, Gerd Hoffmann,
	Stefan Hajnoczi, Marc-André Lureau, Andrea Bolognani,
	John Snow

On 06/10/20 11:40, Daniel P. Berrangé wrote:
>> 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.
> 
> I feel that both of these approaches are equally broken, as they don't
> honour the order in which arguments are passed by the caller when
> creating resources.

By design, in that I'm only looking at a backwards-compatible approach.

But once you have reached the second step, you can add QMP commands for
each command-line option (that matters), and configure the VM via QMP
commands.  That _will_ honor the order in which commands are executed by
the caller, obviously.

> I'm not convinced that your proposed change takes us in direction, if
> anything it is encoding the current split of parsing vs creation even
> more strongly.

Yes, but it enables the right way too.  Doing things in steps is the
only way to do them.

Paolo



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-10-06  9:54 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-04  0:14 QEMU API cleanup initiative - Let's chat during the KVM call John Snow
2020-10-05 13:45 ` Stefan Hajnoczi
2020-10-05 14:52   ` John Snow
2020-10-06  9:30     ` Paolo Bonzini
2020-10-06  9:40       ` Daniel P. Berrangé
2020-10-06  9:53         ` Paolo Bonzini
2020-10-06  9:50     ` Daniel P. Berrangé
2020-10-05 15:33 ` John Snow

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.