Re: [PULL 04/12] hmp: Simplify qom-set

[David Hildenbrand]

On 03.06.20 13:31, David Hildenbrand wrote:
> On 03.06.20 12:43, Dr. David Alan Gilbert wrote:
>> * David Hildenbrand (david@redhat.com) wrote:
>>> On 02.06.20 11:26, Dr. David Alan Gilbert wrote:
>>>> * Markus Armbruster (armbru@redhat.com) wrote:
>>>>> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> writes:
>>>>>
>>>>>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>>>>>>
>>>>>> Simplify qom_set by making it use qmp_qom_set and the JSON parser.
>>>>>>
>>>>>> (qemu) qom-get /machine smm
>>>>>> "auto"
>>>>>> (qemu) qom-set /machine smm "auto"
>>>>>>
>>>>>> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>>>>> Message-Id: <20200520151108.160598-3-dgilbert@redhat.com>
>>>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>>>>>> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>>>>>   With 's'->'S' type change suggested by Paolo and Markus
>>>>>
>>>>> This is actually more than just simplification, it's disarming a bear
>>>>> trap: the string visitor is restricted to a subset of the QAPI types,
>>>>> and when you qom-set a property with a type it can't handle, QEMU
>>>>> aborts.  I mentioned this in the discussion of possible ways out of the
>>>>> qom-get impasse, but missed reraising it in patch review.
>>>>>
>>>>> A suitably amended commit would be nice, but respinning the PR just for
>>>>> that may not be worthwhile.
>>>>
>>>> A bit late; still as long as we're removing bear traps not adding them.
>>>
>>> This breaks qom-set for my (virtio-mem) use case:
>>>
>>> echo "qom-set vm0 requested-size 300M" | sudo nc -U /var/tmp/mon_src
>>> QEMU 5.0.50 monitor - type 'help' for more information
>>> (qemu) qom-set vm0 requested-size 300M
>>> Error: Expecting at most one JSON value
>>
>> Does qom-set vm0 requested-size 300e6 do the same thing?
> 
> The property is defined to be of type "size".
> 
> (qemu) qom-set vm0 requested-size 300e6
> Error: Parameter 'requested-size' expects uint64
> 
> (not sure how "size" and "uint64" are mapped here)
> 

Oh, and before this commit, your example does work as well (well, fails
later because the number is not properly aligned, but passes the parsing
stage).

-- 
Thanks,

David / dhildenb