qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: Markus Armbruster <armbru@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Chris Wright <chrisw@redhat.com>,
	qemu-devel <qemu-devel@nongnu.org>,
	Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>,
	Adam Litke <agl@us.ibm.com>
Subject: Re: [Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling
Date: Thu, 17 Mar 2011 13:28:29 -0500	[thread overview]
Message-ID: <4D8252CD.4060607@codemonkey.ws> (raw)
In-Reply-To: <m3d3lp4vqo.fsf@blackfin.pond.sub.org>

On 03/17/2011 10:22 AM, Markus Armbruster wrote:
>
>> void qcfg_handle_vnc(VncConfig *option, Error **errp)
>> {
>> }
>>
>> And that's it.  You can squirrel away the option such that they all
>> can be processed later, you can perform additional validation and
>> return an error, or you can implement the appropriate logic.
>>
>> The VncConfig structure is a proper C data structure.  The advantages
>> of this approach compared to QemuOpts are similar to QAPI:
>>
>> 1) Strong typing means less bugs with lack of command line validation.
>> In many cases, a bad command line results in a SEGV today.
> Static typing, you mean.

Both.  We don't consistently check error returns when dealing with 
QemuOpts so while it does implement dynamic typing, the dynamic typing 
is not consistently enforced.

>> 2) Every option is formally specified and documented in a way that is
>> both rigorous and machine readable.  This means we can generate high
>> quality documentation in a variety of formats.
> You make it sound like QemuOpts wouldn't specify options.  It does.
> Where your proposal differs from QemuOpts, in my opinion, is that it
> uses C types rather than QemuOpts' very own ad hoc type system, and is
> more expressive, see your 6) below.

My point in that bullet is that the documentation is machine readable 
and highly structured.

>> 3) The command line parameters support full introspection.  This
>> should provide the same functionality as Dan's earlier introspection
>> patches.
> Again, this isn't something QemuOpts could not do.  We just neglected to
> make its option specification available outside QEMU, mostly because for
> lack of consensus on what we want to expose there.

It's all just a matter of bits, right :-)

But QemuOpts is just about parsing a string, it doesn't really 
explicitly tie to a command line.  Another layer would be needed to 
clearly map that option foo is associated with this QemuOpts.

So yeah, it could grow in that direction but there's a lot more to it 
then just returning a list of names and ties given a qemu opts definition.


>> 5) Very complex data types can be implemented.  We had some discussion
>> of supporting nested structures with -blockdev.  This wouldn't work
>> with QemuOpts but I've already implemented it with QCFG (blockdev
>> syntax is my test case right now).  The syntax I'm currently using is
>> -blockdev
>> cache=none,id=foo,format.qcow.protocol.nbd.hostname=localhost where
>> .' is used to reference sub structures.
> We might want some syntactic sugar so that users don't have to repeat
> format.qcow.protocol.nbd.FOO for every FOO they want to configure.

-set will still function in this model as key/value parsing and 
translation to a C structure are two separate steps.

But yeah, I think we ought to brain storm ways to simplify blockdev 
because the structure is very complex.  But that's a separate discussion.

>> Initial code is in my QAPI tree.
>>
>> I'm not going to start converting things until we get closer to the
>> end of 0.15 and QAPI is fully merged.  My plan is to focus on this for
>> 0.16 and do a full conversion for the 0.16 time frame using the same
>> approach as QAPI.  That means that for 0.16, we would be able to set
>> all command line options via QMP in a programmatic fashion with full
>> support for introspection.
>>
>> I'm haven't yet closed on how to bridge this to qdev.  qdev is a big
>> consumer of QemuOpts today.  I have some general ideas about what I'd
>> like to do but so far, I haven't written anything down.
> Glad you mention qdev.
>
> qdev properties describe the configurable parts of qdev state objects.
> A state object is a C struct.  The description is C data.  Poor man's
> reflection.

Right.  The problem is it's very hard to reflect C even if you parse it 
without additional annotations.  For instance:

typedef struct Foo {
     Bar *bar;
} Foo;

What the type of bar is is ambigious.  It could be a pointer to a list 
of Bar's (if bar has an embedded pointer), it could be an array of Bars 
that is terminated using a field within Bar, it could be a pointer to a 
fixed size array of Bars, or it could be just a pointer to a single Bar 
object.

So you end up needing additional annotations like:

typedef struct Foo {
    size_t n_bar;
    Bar *bar sizeis(n_bar);
} Foo;

This is what most IDLs that use C style syntax do.

> qdev needs to parse and print a device's properties.  It uses QemuOpts
> to parse NAME=VALUE,... option strings into a list of (NAME, VALUE), and
> callbacks to parse the VALUEs.  Awkward, especially when you go QMP ->
> option string ->  qdev struct.

Indeed.  You can do very weird things like pass a float through qdev and 
it appears as a (valid) string type with QemuOpts.

But qdev properties are a little odder semantically.  qdev properties 
are construct-only and read-only.   They are construct-only because they 
are set implicitly in the object before its created as a sort of default 
value.

It's easy to bridge QCFG/QMP to this but I've been thinking of going a 
step further.  I've been thinking of taking the current qdev properties 
and making them proper construction properties and removing the magic 
involved in setting their default value.  This would require the code 
generator to create a construction function that is called such as:

DeviceState *isa_serial_init(int iobase, int irq, CharDriverState *chr)
{
      ISADeviceState *dev = qdev_alloc(&isa_serial_desc);

      dev->iobase = iobase;
      dev->irq = irq;
      dev->chr = chr;
      // ...
}

There would be a separate interface for getting/setting properties.  It 
might even be something as simple as, you have to implement:

int isa_serial_get_iobase(ISADeviceState *dev);
int isa_serial_get_irq(ISADeviceState *dev);
...

This ends up being a powerful interface because you can easily get these 
properties within QEMU, but you also (transparently) map these to the 
wire.  It also extends really nicely for setting properties which 
becomes an interesting way to implement dynamic device logic (like 
setting the link status of a network card).

> Yet another one: vmstate, which describes migratable parts of qdev state
> objects.

Yes, for now, I'm ignoring vmstate.  To really solve vmstate, I think 
you need to describe the complete object instead of just it's properties.

> Unlike these two, QCFG doesn't describe C structs, it generates them
> from JSON specs.  If I understand your proposal correctly.  Hmm.

Correct.

> Can we avoid defining our data types in JSON rather than C?

I didn't start with describing them in JSON.  I started by describing 
them in Python with the idea that I'd write a IDL parser using a C style 
syntax that would then generate the Python structures.  Using the fixed 
Python data structures, I could play with code generation without all 
the trouble of writing a full blown IDL parser.

But I never got around to writing that parser and have actually become 
fond of the Python/JSON style syntax.

> Can we adopt *one* reflection mechanism?

Yes, I think we should start with that being the JSON syntax I'm using 
for QAPI/QCFG and then down the road, we can introduce a totally new 
syntax if need be.  But I think we need a central schema that describes 
all externally visible interfaces.  I think this is really the key idea 
here.

>> I wanted to share these plans early hoping to get some feedback and
>> also to maybe interest some folks in helping out.
> There's also QEMUOptionParameter, thankfully limited to block code.  Any
> plans to cover that as well?

QCFG will replace this because the BlockdevConfig structure can be 
passed throughout the block layer to enable block format specific 
options to be implemented.

QEMUOptionParameter is just used for bdrv_create today right?  Do we 
want create options to be different than run time options or would a 
superset of the two be appropriate?

Regards,

Anthony Liguori

  reply	other threads:[~2011-03-17 18:28 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-14 17:48 [Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling Anthony Liguori
2011-03-14 19:52 ` Lluís
2011-03-14 20:04   ` Anthony Liguori
2011-03-15 10:09 ` Kevin Wolf
2011-03-15 13:27   ` Anthony Liguori
2011-03-15 13:45     ` Kevin Wolf
2011-03-15 13:56       ` Anthony Liguori
2011-03-18 18:12     ` Stefan Hajnoczi
2011-03-15 11:21 ` [Qemu-devel] " Kevin Wolf
2011-03-15 13:37   ` Anthony Liguori
2011-03-15 13:51     ` Kevin Wolf
2011-03-17 15:26       ` Markus Armbruster
2011-03-18  4:12         ` Anthony Liguori
2011-03-18 13:07           ` Markus Armbruster
2011-03-17 15:22 ` [Qemu-devel] " Markus Armbruster
2011-03-17 18:28   ` Anthony Liguori [this message]
2011-03-18  9:44     ` Kevin Wolf
2011-03-18 14:04     ` Markus Armbruster
2011-03-18 22:39       ` Anthony Liguori
2011-03-22 13:01         ` Markus Armbruster
2011-03-22 15:49           ` Anthony Liguori
2011-03-24  8:32             ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D8252CD.4060607@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=agl@us.ibm.com \
    --cc=armbru@redhat.com \
    --cc=chrisw@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).