Re: Configuring onboard devices (was: Failing property setters + hardwired devices + -global = a bad day)

[Daniel P . Berrangé]

On Thu, Apr 30, 2020 at 12:03:12PM +0200, Markus Armbruster wrote:
> Peter Maydell <address@hidden> writes:
> 
> > On Thu, 30 Apr 2020 at 08:09, Markus Armbruster <address@hidden> 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

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


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

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

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|