Re: [PATCH v2] hmp: Make json format optional for qom-set

[David Hildenbrand]

On 15.06.20 08:17, Markus Armbruster wrote:
> David Hildenbrand <david@redhat.com> writes:
> 
>> Commit 7d2ef6dcc1cf ("hmp: Simplify qom-set") switched to the json
>> parser, making it possible to specify complex types. However, with this
>> change it is no longer possible to specify proper sizes (e.g., 2G, 128M),
>> turning the interface harder to use for properties that consume sizes.
>>
>> Let's switch back to the previous handling and allow to specify passing
>> json via the "-j" parameter.
> 
> Two issues:
> 
> 1. Makes qom-get and qom-set inconsistent
> 
>    qom-get formats as JSON, always.
> 
>    qom-set parses the string visitor's undocumented ad hoc language by
>    default.  You can make it parse JSON by passing -j.

This is the same language the QEMU cmdline uses, no?

> 
>    Not a show stopper, but sure ugly.  I feel documentation should point
>    it out.

Sure, we can fine-tune the documentation. For now we didn't have any
qom-get users, in contrast to qom-set. Not sure if it makes sense to
implement the same functionality for qom-get.

For now I can e.g.,

"echo "qom-set vm1 requested-size 256M" | sudo nc -U /var/tmp/mon_src"

then I can

echo "qom-get vm1 requested-size " | sudo nc -U /var/tmp/mon_src
-> 268435456

which is a value I can punch back into qom-set. At least for sizes this
works. Not perfect, not bad. Opinions?


> 
> 2. Rearms the string visitor death trap
> 
>    If you try to qom-set a property whose ->set() uses something the
>    string input visitor doesn't support, QEMU crashes.  I'm not aware of
>    such a ->set(), but this is a death trap all the same.  Mind, I
>    didn't actually *look* for such a ->set().  Details:

Thanks. Maybe I am missing something important, but this sounds like we
are missing a bunch of checks+errors. (wouldn't we be able to crash
using the QEMU cmdline as well when setting such properties?).

> 
>     Subject: Re: [RFC PATCH] qom: Implement qom-get HMP command
>     Date: Sat, 02 May 2020 08:02:43 +0200 (6 weeks, 2 days, 4 minutes ago)
>     Message-ID: <87a72q6fi4.fsf@dusky.pond.sub.org>
>     https://lists.nongnu.org/archive/html/qemu-devel/2020-05/msg00178.html
> 
>    Since we've had this death trap in the code for a number of years, I
>    can't call its restoration a show stopper.  It does feel like an
>    unadvisable risk, though.
> 

As long as there are no better alternatives to punch in data in the same
format the QEMU cmdline consumes, I think this is perfectly reasonable.
No good reason to make a HMP interface harder to use by humans IMHO.

Thanks!

-- 
Thanks,

David / dhildenb