[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] hmp: Make json format optional for qom-set
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [PATCH v2] hmp: Make json format optional for qom-set |
Date: |
Mon, 15 Jun 2020 10:40:16 +0100 |
User-agent: |
Mutt/1.14.0 (2020-05-02) |
* Markus Armbruster (armbru@redhat.com) 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.
>
> Not a show stopper, but sure ugly. I feel documentation should point
> it out.
I can imagine one way around this owuld be to remove the flag and make
it happen in the failure case; i.e.:
obj = qobject_from_json(value, &err);
if (err == NULL) {
qmp_qom_set(path, property, obj, &err);
} else {
somehow check if it parses with the integer parser and if it
does use object_property_parse
}
unfortunately that else path is a bit messy, because you need to pick a
parser in this case and then if that fails probably present the json
error message not it's error.
> 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:
>
> 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.
That just needs fixing in qom somewhere; it shouldn't assert - people
are too free with asserts.
Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK