Re: [Qemu-devel] [PATCH for-1.4 v2 4/6] qemu-option: Disable two helpful messages that got broken recently

[Markus Armbruster]

Laszlo Ersek <address@hidden> writes:

> On 02/15/13 01:20, Laszlo Ersek wrote:
>> On 02/14/13 17:36, Luiz Capitulino wrote:
>>> On Thu, 14 Feb 2013 14:31:50 +0100
>>> Markus Armbruster <address@hidden> wrote:
>
>>>> chardev-add: the schema defines an object type for each backend
>>>> (ChardevFile, ChardevSocket, ...), and collects them together in
>>>> discriminated union ChardevBackend.  chardev_add takes that union.
>>>> Thus, the schema accurately describes chardev-add's arguments, and the
>>>> generated marshaller takes care of parsing chardev-add arguments into
>>>> the appropriate objects.
>>>
>>> Yes, it's a nice solution. I don't know why we didn't have that idea
>>> when discussing netdev_add. Maybe we were biased by the old
>>> implementation.
>>>
>>>> netdev_add: the schema defines an object type for each backend
>>>> (NetdevNoneOptions, NetdevUserOptions, ...).  netdev_add doesn't use
>>>> them, but takes arbitrary parameters instead.  The connection is made in
>>>> code, which stuffs the parameters in a QemuOpts (losing all JSON type
>>>> information), then takes them out again to stuff them into the
>>>> appropriate object type.  A job the marshaller should be able to do for
>>>> us.
>>>>
>>>> For me, the way chardev-add works makes a whole lot more sense, and I
>>>> think we should clean up netdev_add to work the same way.
>
> So, regarding netdev_add again,
>
> --[schema dict]--> qmp_netdev_add()                  ---\
> --[QemuOpts]-->    net_client_init() == opts-visitor ---\
> --[C struct]-->    specific logic
>
> (a) I agree that the intermediary QemuOpts representation is
> superfluous, and that we could directly expose the schema over QMP, ie.
> go from schema dict right to C struct. However,
>
> (b) opts-visitor's primary responsibility remains mangling one QemuOpts
> instance into a C struct. This effectively hamstrings any affected
> schema. You *can* choose to expose/reuse that schema over the wire, but
> you won't have any freedom massaging the schema later on just for the
> QMP caller's sake.
>
> --[schema dict]--> qmp_netdev_add() --[C struct]--> specific logic
> --[QemuOpts]-->    opts-visitor     --[C struct]--> specific logic
>
> Obviously, you want to share the [C struct] and the "specific logic"
> between the two cases. However the C struct (the schema) is hamstrung by
> QemuOpts, thus ultimately the QMP wire format is dictated by the
> QemuOpts format that you accept on the command line.
>
> At first sight this might come through as a "semantic match" (same stuff
> available on the command line as over QMP), but:
> - you can't compose the underlying schema just any way you like,
> - what you can't express as a single QemuOpts object (think dictionaries
> in dictionaries), you can't allow over QMP.

Correct in general, but need not be an issue in a specific case.

When it is, I'd suggest to try something like:

* Create a schema appropriate for QMP.  This results in a C data
  structure (generated) and code accepting it.  Let's call the latter
  "the interface".

* Create a schema for an opts-visitor, use it to take apart the option
  string.  This results in another C data structure.

* Write glue code to convert the opts-visitor result into the data
  structure accepted by the interface.

Alternatively, if QemuOpts are sufficiently simple, take them apart the
old-fashioned way, without visitors, then call a suitable interface
shared with the QMP case.  Calling the QMP handler requires building a
QDict, so you might be better off aiming lower.  The obvious option is
filling in the QMP schema's C data structure so you can call "the
interface".

> With chardev_add, IIUC, first you create a logical schema, and expose it
> over QMP (all dandy), then hand-craft qemu_opt_get_XXXX() code, with
> properties encoded as C string literals, in order to shoehorn the
> logical schema into the command line (QemuOpts). I couldn't call this
> approach "bad" with a straight face (it clearly works in practice!), but
> as I perceived it, opts-visitor had been invented precisely to eliminate
> this.

If I understand you correctly, this is exactly what I just described
under "alternatively, if QemuOpts are sufficiently simple".  Except the
options are not really simple.  So maybe we'd be better off doing it the
other way.

> Sorry for ranting...

You call this a rant?  ;)