* David Hildenbrand (da...@redhat.com) wrote: > On 03.06.20 14:26, David Hildenbrand wrote: > > On 03.06.20 14:24, Dr. David Alan Gilbert wrote: > >> * David Hildenbrand (da...@redhat.com) wrote: > >>> On 03.06.20 13:43, Dr. David Alan Gilbert wrote: > >>>> * David Hildenbrand (da...@redhat.com) wrote: > >>>>> On 03.06.20 12:43, Dr. David Alan Gilbert wrote: > >>>>>> * David Hildenbrand (da...@redhat.com) wrote: > >>>>>>> On 02.06.20 11:26, Dr. David Alan Gilbert wrote: > >>>>>>>> * Markus Armbruster (arm...@redhat.com) wrote: > >>>>>>>>> "Dr. David Alan Gilbert (git)" <dgilb...@redhat.com> writes: > >>>>>>>>> > >>>>>>>>>> From: "Dr. David Alan Gilbert" <dgilb...@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 <dgilb...@redhat.com> > >>>>>>>>>> Message-Id: <20200520151108.160598-3-dgilb...@redhat.com> > >>>>>>>>>> Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com> > >>>>>>>>>> Reviewed-by: Markus Armbruster <arm...@redhat.com> > >>>>>>>>>> Signed-off-by: Dr. David Alan Gilbert <dgilb...@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) > >>>> > >>>> I think the problem here is that the JSON parser is converting anything > >>>> with an 'e' as a float; JSON itself doesn't have the distinction > >>>> between int and float. > >>>> > >>> > >>> (and just to clarify - I assume you are aware - 300e6 != 300M. So the > >>> interface becomes way harder to use in case one wants to specify > >>> properly aligned sizes - 300M vs 314572800) > >> > >> Oops, yes, good point. > >> > >> I think on balance it's probably best that this keeps supporting JSON; > >> although tbh I'm not convinced there are any complex types that can be > >> set. > >> I'm not seeing a prettier answer. > > > > So, I have to use a calculator from now on to set a property that I can > > set on the QEMU cmdline just fine without it? :( > > > > This feels like a step backwards, @Markus any way to keep supporting sizes? > > Or what about adding qom-set-json instead for complex types instead of > changing the behavior if an existing interface?
Yes, this isn't nice - we could add a flag to qom-set to take nice numerics. Dave > -- > Thanks, > > David / dhildenb -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
