Re: Configuring onboard devices

[Markus Armbruster <armbru@redhat.com>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Thu, Apr 30, 2020 at 12:03:12PM +0200, Markus Armbruster wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>> 
>> > On Thu, 30 Apr 2020 at 08:09, Markus Armbruster <armbru@redhat.com> wrote:
>> >> Our means to configure onboard devices are weak.  We sidestepped this
>> >> for isa-fdc by taking it off the board, and thus make -device work.
>> >
>> > This seems to be a general dynamic: the x86 pc machine works
>> > via -device options (or is changed so it can work that way);
>> > and then people propose dropping/deprecating/etc the config
>> > options that work with onboard devices, without providing
>> > clear solutions/instructions on how the command line needs
>> > to change/etc for the mass of boards which are not the x86
>> > pc machine and which do have a lot of onboard devices which
>> > can't be handled via -device.
>> >
>> > So my gut reaction to the "we should deprecate -global"
>> > suggestions in this thread was a bit "here we go again"...
>> > What works for x86 or even "what is sufficient for libvirt"
>> > doesn't necessarily cover all the cases.
>> 
>> Such shortsighted proposals have been made, but don't think it's what
>> we're doing here.
>> 
>> You're 100% right in that we do need to configure onboard devices.
>> -global is a terrible way to do it, though: it applies to *all* devices
>> of a kind.  What if the board has more than one?  What if the can add
>> more?
>
>
>> Any better ideas for letting users configure onboard devices?
>
> All the devices in QEMU form a tree, as reported by "info qtree".
> So, IIUC, the challenge is to provide a way to uniquely identify
> any node in the tree.
>
> Devices configured by the user/mgmt app will have an "id" property
> but most built-in devices will not have any "id". In addition even
> user configured devices may create multiple sub-nodes in the tree
> without "id" parameters.
>
> Uniquely referencing nodes in a tee is a solved problem though,
> even without "id" parameters. The XPath query languages shows
> this for XML.
>
> -global defines a query language based on the object type, and
> property name which is insufficiently flexible

Yes.

> -set defines a query language based on the object type and ID
> value and property name(s) which is again insufficiently flexible.

-set is even more obscure than -global.  It modifies existing QemuOpts.
You could use it to monkey-patch a -device to the left, or a [device]
from a -readconfig file to the left, which is a bit more useful.  You
can't use it to configure onboard devices, because there is no QemuOpts
to monkey-patch for them.

> We "merely" need a new query language targetted to QEMU's qtree
> structure, which we can expose in the CLI that gives unique access
> to every possible property.
>
> Here is the truncated 'info qtree' for a running guest of mine:
>
> bus: main-system-bus
>   type System
>   dev: kvm-ioapic, id ""
>     gpio-in "" 24
>     gsi_base = 0 (0x0)
>     mmio 00000000fec00000/0000000000001000
>   dev: i440FX-pcihost, id ""
>     pci-hole64-size = 2147483648 (2 GiB)
>     short_root_bus = 0 (0x0)
>     x-pci-hole64-fix = false
>     bus: pci.0
>       type PCI
>       dev: virtio-balloon-pci, id "balloon0"
>         disable-legacy = "off"
>         disable-modern = true
>         class = 255 (0xff)
>         virtio-pci-bus-master-bug-migration = false
>         migrate-extra = false
>         modern-pio-notify = false
>         x-disable-pcie = true
>         page-per-vq = true
>         x-ignore-backend-features = true
>         ats = false
>         x-pcie-deverr-init = false
>         x-pcie-lnkctl-init = false
>         x-pcie-pm-init = false
>         addr = 08.0
>         romfile = ""
>         rombar = 1 (0x1)
>         multifunction = false
>         command_serr_enable = true
>         x-pcie-lnksta-dllla = true
>         x-pcie-extcap-init = false
>         class Class 00ff, addr 00:08.0, pci id 1af4:1002 (sub 1af4:0005)
>         bar 0: i/o at 0xc100 [0xc11f]
>         bus: virtio-bus
>           type virtio-pci-bus
>           dev: virtio-balloon-device, id ""
>             deflate-on-oom = false
>             free-page-hint = false
>             qemu-4-0-config-size = false
>             iothread = ""
>             indirect_desc = true
>             event_idx = true
>             notify_on_empty = true
>             any_layout = true
>             iommu_platform = false
>             use-started = false
>
>
> Consider the problem is to set the "deflate-on-oom" property on
> the balloon device.
>
> To uniquely identify this we can have a string:
>
>  /dev[1]/bus[pci/0]/dev[id=balloon0]/bus[virtio-bus]/dev[0]/deflate-on-oom=true

qdev already supports identifying a node in this tree by its path, but
these qdev paths are FUBAR.

Moreover, the qtree misses "busless" devices.

The QOM tree should be as complete as possible, i.e. lack only non-qdev
devices[*].  It also has sane paths.  Have a look at "info qom-tree",
please.

> If we consider that "id" values are unique, we can allow a simplication
> by omitting everything before that part of the match - "//" could indicate
> an omitted part like XPath allows, so we'd get
>
>  //dev[id=balloon0]/bus[virtio-bus]/dev[0]/deflate-on-oom=true
>
> There's only one bus, and one dev on that bus, so knowing this we can
> simplify a bit more and still be a unique query, to get this:
>
>  //dev[id=balloon0]/bus/dev/deflate-on-oom=true
>
> Or even allow use of "//" in the middle too:
>
>  //dev[id=balloon0]//deflate-on-oom=true
>
> Which conceptually says
>
>    "find the device with id balloon0 and set the property 'deflate-on-oom'
>    on the first child node in the qtree that hsa such a property name"
>
> I didn't say this would be pretty, and of course no one would seriously
> use this syntax for the virtio-balloon device, as you'd just set the
> property with -device. It should work for the many built-in devices
> though.
>
> Now just provide a new CLI arg
>
>  $QEMU -qtree //dev[id=balloon0]//deflate-on-oom=true



[*] It's been eleven years.  Device models that still ignore qdev need
to be taken out and shot, along with the machine types using them.  Any
machine types we actually miss then need to be revived and fixed up.