Re: [Qemu-devel] [PATCH v2 03/24] keyval: New keyval_parse()

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 02/28/2017 03:26 PM, Markus Armbruster wrote:
>> keyval_parse() parses KEY=VALUE,... into a QDict.  Works like
>> qemu_opts_parse(), except:
>> 
>> * Returns a QDict instead of a QemuOpts (d'oh).
>> 
>> * Supports nesting, unlike QemuOpts: a KEY is split into key
>>   fragments at '.' (dotted key convention; the block layer does
>>   something similar on top of QemuOpts).  The key fragments are QDict
>>   keys, and the last one's value is updated to VALUE.
>> 
>> * Each key fragment may be up to 127 bytes long.  qemu_opts_parse()
>>   limits the entire key to 127 bytes.
>> 
>> * Overlong key fragments are rejected.  qemu_opts_parse() silently
>>   truncates them.
>> 
>> * Empty key fragments are rejected.  qemu_opts_parse() happily
>>   accepts empty keys.
>> 
>> * It does not store the returned value.  qemu_opts_parse() stores it
>>   in the QemuOptsList.
>> 
>> * It does not treat parameter "id" specially.  qemu_opts_parse()
>>   ignores all but the first "id", and fails when its value isn't
>>   id_wellformed(), or duplicate (a QemuOpts with the same ID is
>>   already stored).  It also screws up when a value contains ",id=".
>> 
>> * Implied value is not supported.  qemu_opts_parse() desugars "foo" to
>>   "foo=on", and "nofoo" to "foo=off".
>> 
>> * An implied key's value can't be empty, and can't contain ','.
>
> or '=' (but the presence of '=' means no implied key, while the presence
> of ',' marks end of the implied key's value).  Not sure it's worth
> tweaking this commit message any further.

Both fail the assertion they're meant to fail (I tested).  I'll fix the
commit message.

>> 
>> I intend to grow this into a saner replacement for QemuOpts.  It'll
>> take time, though.
>> 
>> Note: keyval_parse() provides no way to do lists, and its key syntax
>> is incompatible with the __RFQDN_ prefix convention for downstream
>> extensions, because it blindly splits at '.', even in __RFQDN_.  Both
>> issues will be addressed later in the series.
>> 
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>
> Looks like you addressed all the comments.
> Reviewed-by: Eric Blake <address@hidden>

Thanks!