From: Markus Armbruster <armbru@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v4 11/23] monitor: Add completion support for
	option lists
References: <cover.1276641524.git.jan.kiszka@web.de>
	<cover.1276641524.git.jan.kiszka@web.de>
	<68962306385412f48130690d29a0349b322b9930.1276641524.git.jan.kiszka@web.de>
	<m37hlqaxqw.fsf@blackfin.pond.sub.org> <4C21E1CB.6040600@siemens.com>
Date: Wed, 23 Jun 2010 19:08:18 +0200
In-Reply-To: <4C21E1CB.6040600@siemens.com> (Jan Kiszka's message of "Wed, 23
	Jun 2010 12:28:27 +0200")
Message-ID: <m339wdr81p.fsf@blackfin.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>, Juan Quintela <quintela@redhat.com>, "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>, Luiz Capitulino <lcapitulino@redhat.com>, Blue Swirl <blauwirbel@gmail.com>, Jan Kiszka <jan.kiszka@web.de>, Avi Kivity <avi@redhat.com>

Jan Kiszka <jan.kiszka@siemens.com> writes:

> Markus Armbruster wrote:
>> Jan Kiszka <jan.kiszka@web.de> writes:
>> 
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> This enables command line completion inside option strings. A list of
>>> expected key names and their completion type can be appended to the 'O'
>>> inside parentheses ('O(key:type,...)'). The first use case is block
>>> device completion for the 'drive' option of 'device_add'.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>  monitor.c       |   68 ++++++++++++++++++++++++++++++++++++++++++++++---------
>>>  qemu-monitor.hx |    2 +-
>>>  2 files changed, 58 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/monitor.c b/monitor.c
>>> index c1006b4..3e0d862 100644
>>> --- a/monitor.c
>>> +++ b/monitor.c
>>> @@ -68,6 +68,9 @@
>>>   * 'O'          option string of the form NAME=VALUE,...
>>>   *              parsed according to QemuOptsList given by its name
>>>   *              Example: 'device:O' uses qemu_device_opts.
>>> + *              Command completion for specific keys can be requested via
>>> + *              appending '(NAME:TYPE,...)' with 'F', 'B' as type.
>>> + *              Example: 'device:O(bus:Q)' to expand 'bus=...' as qtree path.
>>>   *              Restriction: only lists with empty desc are supported
>>>   *              TODO lift the restriction
>>>   * 'i'          32 bit integer
>> 
>> Ugh.
>> 
>> Replacement of args_type by a proper data structure is long overdue.  We
>> keep piling features into that poor, hapless string.
>> 
>> Information on how to complete QemuOptsList options arguably belongs
>> into the option description, i.e. QemuOptDesc.
>
> For sure, that would be better. I just wonder how much of it should be
> stuffed into this series. I guess I will drop this part for now, just
> focusing on what device_show makes direct use of. Same for separate args
> for HMP and QMP.

Sensible.


Speaking of args_type.  We've accumulated too many ways to represent
dynamic data types, and too many ways to describe data types.

The most powerful dynamic data type is QObject.  It comes with a useful
plain-text representation (JSON).  We lack a data type for describing
QObjects.  We use args_type to describe special QObjects, namely monitor
command arguments.  We've clearly stretched that to the limit and then
some.  We could use a general solution.  JSON Schema as its plain-text
representation could be nice.

Then we have QemuOpts.  It's more limited (flat list instead of trees),
but it comes with a real data type to describe it (QemuOptsList).

There's also QEMUOptionParameter.  Serves a similar purpose.  I don't
know why we have both.

Over in qdev-land, we find Property, which can be viewed a data type to
describe (parts of) a struct.

Some unification would be nice.