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

[Laszlo Ersek]

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.

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.

Sorry for ranting...

Laszlo