On 06/03/14 05:24, Eric Blake wrote:
> On 05/30/2014 02:37 AM, Peter Krempa wrote:
>> This patch adds option to specify that a json qemu command argument is
>> optional without the need to use if's or ternary operators to pass the
>> list. Additionally all the modifier characters are documented to avoid
>> user confusion.
>> ---
>> src/qemu/qemu_monitor_json.c | 122 +++++++++++++++++++++++++++++--------------
>> 1 file changed, 84 insertions(+), 38 deletions(-)
>>
>
> The diffstat is misleading - the bulk of the addition is documentation,
> and the bulk of the deletions are compression taking advantage of the
> new feature. Overall, I like the patch!
>
>> + *
>> + * i: signed integer value
>> + * z: signed integer value, omitted if zero
>> + *
>> + * I: signed long integer value
>> + * Z: integer value, signed, omitted if zero
>
> Looks more consistent as: "signed long integer value, omitted if zero"
>
>> + *
>> + * u: unsigned integer value
>> + * p: unsigned integer value, omitted if zero
>> + *
>> + * U: unsigned long integer value (see below for quirks)
>> + * P: unsigned long integer value, omitted if zero,
>
> drop the trailing comma
>
>> + *
>> + * d: double precision floating point number
>> + * b: boolean value
>
> Is it worth a B for a boolean value that is omitted if false?
I've added it as it's trivial. Hopefully someone will need it in the future.
>
>> + * n: json null value
>> + * a: json array
>> + */
>> type = key[0];
>> key += 2;
>>
>
>> @@ -3557,7 +3603,7 @@ int qemuMonitorJSONSendKey(qemuMonitorPtr mon,
>>
>> cmd = qemuMonitorJSONMakeCommand("send-key",
>> "a:keys", keys,
>> - holdtime ? "U:hold-time" : NULL, holdtime,
>> + "P:hold-time", holdtime,
>> NULL);
>
> Fix the indentation while you are at it.
>
> ACK with nits addressed.
>
I've fixed the docs and formatting and pushed.
Peter
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list