On Sat, Sep 14, 2024 at 10:42:36 +0200, Markus Armbruster wrote: > Peter Krempa <pkrempa@xxxxxxxxxx> writes: > > > This is a little off-topic: > > > > So I wanted to make libvirt use the new parameter to stay ahead > > deprecation. I've applied this patch to qemu, dumped capabilities and > > pretty much expected a bunch of test cases in libvirt fail as they'd be > > using a deprecated field as libvirt is supposed to validate everything. > > > > And the test suite passed unexpectedly. I've dug further and noticed > > that for some reason libvirt doesn't still use JSON parameters for > > -chardev (which is the pre-requisite for validation). > > > > I've also noticed that at some point I attempted to convert it over > > witnessed by having an (unused) capability named QEMU_CAPS_CHARDEV_JSON > > that I've introduced. > > > > My questions are: > > 1) Does '-chardev' accept JSON identical to 'chardev-add' QMP command? > > Sadly, no. Yeah, in the meanwhile I had a look and also remembered that we spoke about why this was the case. (All the 'wrapper' objects making the schema of 'chardev-add' extremely unpleasant) > How badly do you want it? So the main motivation is to have just one instance of the code generating the config for qemu. As JSON has type information inside libvirt always generates JSON internally first. This is the case for -object, -device, -netdev, -blockdev, ... For '-chardev' we currently (in tree) have two separate formatters for '-chardev' and QMP 'chardev-add'. I do have a reasonalby looking rework in the works which unifies them (with a few quirky "if (commandline)" blocks). In cases when we need to support the old syntax for any reason we do have a converter that takes JSON and outputs the qemuopts syntax. This is possible for now as we have mostly flat structures and the only difference for now is how the arrays are processed in -device vs -netdev (by callbacks to the converter). For -chardev this will require a bit more logic as we need to avoid the extra wrappers for commandline output and few fields have different names in QMP than on the commandline. My main goal here is to achieve validation against the QMP schema, and it's much easier for us to add test cases via XML than hardcoding them in C. So honestly I don't think we want it too badly. Definitely not so bad that having it with bad design. I reckon that doing the refactor will also simplify things for the future if the QMP design will be changed to e.g. drop the wrappers. > > If yes: > > If we implemented it: > > > 2) Since when can that be used? (What can I use as a witness) > > I figure we'd provide a witness the same way we did when we added JSON > support to -device: add a feature @json-cli to chardev-add. Yup, that'll always work :) > > 3) Are there any gotchas? > > Not aware of any. Can't be 100% sure until we try. > > > I wonder this as I'd love to finish that out, but I really don't fancy > > digging into qemu to find a gotcha 3/4 of the way there. > > Understandable :) > > > Anyways, as I've already stated, this patch is okay for libvirt, but I > > didn't review the implementation, thus, on behalf of libvirt: > > > > ACKed-by: Peter Krempa <pkrempa@xxxxxxxxxx> > > Thanks! >
