Re: [PATCH osinfo-db 0/5 v2] Add q35 devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



CCing danpb as well

On 10/04/2018 03:26 AM, Fabiano Fidêncio wrote:
On Fri, 2018-09-28 at 18:55 -0400, Cole Robinson wrote:
The main goal of this series is to advertise q35 chipset and device
support for suitable OS. Some details about the individual decisions

* We group q35+ich9+e1000e together. This likely isn't completely
   accurate but in the virt case I think it only makes sense to
   specify the latter two devices when pcie (q35) is present, so
   I went with it.

* We only advertise q35 when virtio1.0 devices are available, on
linux.
   This definitely isn't accurate, q35 predates virtio1.0 by 8 years
or
   so, but there's issues combining q35 with legacy virtio, and that's
   the only combo I'll be exploring for virt-manager so other
scenarios
   my go untested.

* The actual OS+q35 combos are largely untested by me.

v1 posting:
https://www.redhat.com/archives/libosinfo/2018-August/msg00006.html

v2 changes:
     * Rebased, drop committed patches
     * Rework the chipset device ID and dir structure


In the original posting danpb suggested introducing a new <chipset>
concept to cover this case, roughly outlined here:

https://www.redhat.com/archives/libosinfo/2018-September/msg00014.html

This posting doesn't add that and instead uses <device> like before,
but with some changes:

$ cat data/device/qemu.org/chipset-x86_64-q35.d/class.xml.in
<libosinfo version="0.0.1">
<!-- Licensed under the GNU General Public License version 2 or
later.
      See http://www.gnu.org/licenses/ for a copy of the license text
-->
   <device id="http://qemu.org/chipset/x86_64/q35";>
     <name>q35</name>
     <class>chipset.virtual</class>
   </device>
</libosinfo>


My justifications for not going the full <chipset> route:

* It's much more work to add new APIs, test them, document them, etc.
   I'm lazy, but it's also a lot more code to maintain forever.

* Will be harder in the short term for clients as well. virt-manager
   will need to use the new APIs and correctly check that they exist,
   vs just using the existing device APIs.

* I think we should bite the bullet and make the <device> concept
   more fuzzy, since we may want that anyways to model things like
   hyperv features which aren't all strictly devices.

* Eventually we will need to extend the XML schema with some way to
   mark <devices> unsupported if a new version of an OS drops support.
   This is already the case with newer windows which doesn't support
   ac97 out of the box, though libosinfo still reports support. If
   we have a separate <chipset> concept, we will need to duplicate
   that schema and likely libosinfo support to handle that case as
   well. So more work going forward.


Now some questions: A quick one: is class=chipset.virtual okay? I
used .virtual as some day we may want to track actual chipset PCI
devices for example, like the PCI ID for q35.

Considering we're going for the approach you suggested, yes, it does
make sense.


Bigger question: What is the expected way that API users should
determine if libosinfo supports a particular device? Take USB tablet
support for example. The qemu usb tablet has this device xml:

   <device id="http://usb.org/usb/80ee/0021";>
     <name>tablet</name>
     <class>input</class>
   </device>

Ways you can presently search for that device with the API:

- Search for the device ID which is expected to be unique
- Search for name=tablet which presently is unique. However that's a
   super generic name, and nothing seems to enforce <name> uniqueness
   for devices, so I don't know if we can/should count on that.
- Search for class=input name=tablet which narrows it a bit

Boxes takes the second apprach.
It basically gets the list of all supported devices and then does, for
instance:
`find_device_by_prop(supported_devices, DEVICE_PROP_NAME, "virtio-
net")`


virt-manager for example does the last step. I don't know if that's
a good idea though, given how generic name is. Do we aim to give
any guarantees about <name> uniqueness, or that it will never change?
Is <class> never expected to change?

With the current codebase, I'd say that although we don' enforce, apps
are already assuming the <name> uniqueness. So, in case it fits your
design choices, we could go for that.


With that context, let's consider querying chipset. I want to make
sure this syntax isn't going to make future virt chipset additions
difficult. Dan's list in the email lays out some other examples:

      qemu.org/chipset/i686-q35
      qemu.org/chipset/x86_64-q35
      qemu.org/chipset/aarch64/virt
      qemu.org/chipset/riscv/virt

If <name> is expected to be globally unique, we probably don't want
to use name=q35 incase it clashes with a future pcisig.com <device>,
another virt stack's chipset, or even qemu q35 for i686. So that
could
mean name=x86_64-q35, or even name=qemu-x86_64-q35. That gets ugly
and
redundant fast though.

And here I see that it may not fit your design choices :-)

Anyways, can't we enforce the arch/platform for this?

So, for instance ...
<device id="foo">
   <name>q35</name>
   <class>chipset.virtual</class>
   <arch>x86_64</arch>
   <platform id="http://qemu.org/qemu/1.5.3"/>
</device>

Btw, I don't have a proper understanding on how we use the platform so
far and if someone is in the mood to give a nice explanation, that
would be really welcome! :-)

In any case, we'd have to expand the "device" schema and have new APIs
for that in order to guarantee we're not matching the wrong device.


Re: platform. I don't think it was expected to be used like that. My understanding is that the platform concept was so an app can ask: 'I have qemu version 5.6.7 on my system, what devices does it support?', and then you could cross reference that with what devices the OS supports, and choose a hardware combo that will work.

Problem is, 'what devices does qemu support' is not something that can effectively be tracked in static data. qemu could have support for a feature but it was compiled out. qemu could support a feature on some archs but not others. qemu might support a feature depending on some specific host state (/dev/kvm being available). A qemu feature might be modularized so is only available when a specific sub package is installed.

Really libvirt+qemu is the only thing that can tell us what hardware a particular combo supports, and for that we have domcapabilities, although it needs to grow a lot. So if my understanding of the platform concept is correct, then I think it's dead these days.


So IMO the sensible thing would be to enforce uniqueness in two ways:
1) the device ID, 2) the set of all device properties. So that would
lead
to these 4 configs

   <device id="http://qemu.org/chipset/x86_64/q35";>
     <name>q35</name>
     <arch>x86_64</arch>
     <class>chipset.virtual</class>
   </device>
   <device id="http://qemu.org/chipset/i686/q35";>
     <name>q35</name>
     <arch>i686</arch>
     <class>chipset.virtual</class>
   </device>
   <device id="http://qemu.org/chipset/aarch64/virt";>
     <name>virt</name>
     <arch>aarch64</arch>
     <class>chipset.virtual</class>
   </device>
   <device id="http://qemu.org/chipset/riscv/virt";>
     <name>virt</name>
     <arch>riscv</arch>
     <class>chipset.virtual</class>
   </device>

Aha. I should have read the whole e-mail before start answering it. :-/



The downsides of that: 1) if an app naively searchs for name=q35 they
may get the wrong result, because it ignored <arch>. Easy to misuse.


That's something that should be documented how the apps are expected to
use and that's it, IMHO.

2) There aren't any other examples yet of non-unique <name> so maybe
it causes other issues.

True.


All of that makes me think that the only safe recommendation for apps
is to search by device id. <name> should be treated largely as
metadata.

We don't necessarily need to implement all this now, since the q35
case
is fairly straightforward, but we should at least make sure we aren't
going down the wrong path.

Please, take my answers with a  grain of salt as I do believe that
Daniel should/will want to have his opinion on this series as well.


Yes I would like Dan's feedback as well, on q35 as a class=chipset.virtual <device>, and this <name> uniqueness bit

The summary of my thoughts on <name> uniqueness.

- If <name> is intended to be unique, we should have an idea of how to format <name> fields to avoid collisions and start moving towards it. Maybe make them something closer to what lsusb/lspci prints, full human readable strings. For x86 q35 for example this could be like 'QEMU Chipset q35 x86_64'. This uniqueness should also be enforced somewhere

- If <name> is not intended to be unique, we should document that matching on device ID is the safest way to avoid accidental matches, and all other fields are just for filtering and cannot be relied upon for uniqueness. And probably file bugs against all libosinfo using apps to switch to this method when looking for specific device support. FWIW I switched to this method in virt-manager upstream because it's safe regardless, example: https://github.com/virt-manager/virt-manager/blob/a4097b7691bce9367ef3ffeca2ed3124399df918/virtinst/osdict.py#L382

Thanks,
Cole

_______________________________________________
Libosinfo mailing list
Libosinfo@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libosinfo




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Fedora Users]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux