Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Well done! #2

Open
cholcombe973 opened this issue May 22, 2018 · 11 comments
Open

Well done! #2

cholcombe973 opened this issue May 22, 2018 · 11 comments

Comments

@cholcombe973
Copy link

This is fantastic! I've been struggling with how to get this working on and off for a long time now: https://github.com/cholcombe973/parse_qapi and https://github.com/cholcombe973/qemu-rust. I started tinkering with it again yesterday but realized today you'd already done it. Is there anywhere I can help on this?

@arcnmx
Copy link
Owner

arcnmx commented May 23, 2018

Thanks! It's fairly functional right now, and the best thing you could do it try to use it and see if there are any pain points with the way the API currently works, and as a sanity check to identify any possible bugs. A few things in particular that stand out to me:

  • The mid level API (qapi/tokio-qapi crates) design is not something I'm super happy with right now. It works but feels a little messy in places? I would have liked to make tokio-qapi build upon qapi as a dependency but that didn't really work out...
  • Supporting OOB commands (Support OOB Commands #1) combined with the tokio API would be pretty awesome.
  • Documentation is lacking on pretty much everything. The examples are the only way to figure out how to even use it properly right now. Per-crate READMEs may also help here.
  • Pulling in documentation from the json schemas would be neat too, but might require a custom parser more along the lines of your nom approach rather than trying to get it to work with my hacked up serde json decoder.
  • A higher level API is entirely missing, this is just a raw command dispatch library at the moment. Proper native rust objects for everything and QOM would be neat, but probably also a large undertaking. The QEMU API covers a vast amount of functionality, and it's hard to say which areas would be most useful to focus on even when pared down.
  • There's probably a little jankiness in some of the codegen/conversion results/types that I can't recall at the moment.
  • I imported the 2.12.0 schemas recently but haven't really audited the changes or checked whether the new types ended up 100% correct.
  • There are no unit tests. A few approaches could be taken here (a mock server vs real integration tests vs qemu etc)...

@cholcombe973
Copy link
Author

Ok cool so there's a lot of places I can lend a hand. I also thought about creating some code to allow this to invoke qemu with various parameters.

@arcnmx
Copy link
Owner

arcnmx commented May 23, 2018

Huh, now that would be an interesting project if we could parse QEMU's qemu-options.hx file into Rust types that could generate a qemu command-line. That interface is just as complex as qapi!

@cholcombe973
Copy link
Author

Wow what in the heck file format is this qemu-options.hx file in? I've never seen something like this before. If I could parse this crazy file I'd have something way better than the manual thing I was working on

@arcnmx
Copy link
Owner

arcnmx commented May 23, 2018

I have no idea to be honest. It certainly looks structured enough to be usable for us to parse, but I'm not really sure how that file turns into an option parser in qemu itself.

@cholcombe973
Copy link
Author

I wonder if something like peg could be useful here.

@arcnmx
Copy link
Owner

arcnmx commented May 23, 2018

Hm, see qemuopts. Maybe the .hx file is only for generating texinfo documentation? Though it's probably better than trying to parse the C structs strewn throughout the codebase.

As for peg, maybe, it's probably just up to personal preference/comfort whether to use something like it or nom.

@cholcombe973
Copy link
Author

The blocks seem to be defined by STEXI and ETEXI. I'll play around a little and see what i can do with this

@arcnmx
Copy link
Owner

arcnmx commented May 23, 2018

Hm yeah, it does look mostly freeform texinfo, so it's hard to say whether it's strict/structured enough to be parsed without fuzzing in some places... The real structured data is indeed scattered in the C source files ><

@jnsnow
Copy link

jnsnow commented Sep 14, 2019

For what it's worth, QEMU itself has wanted to refactor its option parsing and documentation very badly for a very long time, but the subsystems in QEMU are so diverse as to be almost entirely fractured; so any sweeping changes to the options management wind up requiring a lot of specific knowledge of arcane modules to bring about fully.

Not to mention that we did not have a real deprecation policy until quite recently; and we have many existing legacy users who rely on some commands that are actually quite difficult to support.

It's not something I get to spend much of my time on, but better documentation for QEMU and as a result a more mechanical, data-driven schema for the QEMU command-line that ties into the QAPI schema is on my mind, but it's a huge project. KVM Forum 2019 is coming up and it will for sure be on my list of things to talk about to see if we can't come up with some plans to modernize our documentation and interface specifications.

@jnsnow
Copy link

jnsnow commented Jun 11, 2021

Two years later: I spend a LOT of time on this now. We are actively refactoring our command line parsing and marching towards a version of QEMU that can be configured entirely through QMP, at which point the desire for parsing command line options in this library will be moot.

I happen to know that elmarco has also been experimenting with generating QAPI types directly into rust using our in-tree QAPI generator (Python). Perhaps there is some synchronicity possible there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants