On Thu, 2018-10-04 at 09:26 +0200, 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. > > > 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. Last thing here: I went throught the patches and they look okay enough to go if Daniel agrees with the approach and we have an agreement on how to expand it in the future (if we do). > > > > > Cole Robinson (5): > > device: Add network e1000e > > device: Add sound ich9 > > device: Add chipset q35 > > os: Add q35/e1000e/ich9 for win2k8r2+ and win7+ > > os: linux: Add q35/ich9/e1000e alongside virtio1.0 <device> > > > > data/device/pcisig.com/pci-8086-10d3.d/class.xml.in | 8 > > ++++++++ > > data/device/pcisig.com/pci-8086-293e.d/class.xml.in | 8 > > ++++++++ > > data/device/qemu.org/chipset-x86_64-q35.d/class.xml.in | 8 > > ++++++++ > > data/os/debian.org/debian-9.xml.in | 3 +++ > > data/os/fedoraproject.org/fedora-23.xml.in | 5 ++++- > > data/os/microsoft.com/win-2k8r2.xml.in | 6 ++++++ > > data/os/microsoft.com/win-7.xml.in | 3 +++ > > data/os/opensuse.org/opensuse-42.2.xml.in | 3 +++ > > data/os/redhat.com/rhel-7.2.xml.in | 3 +++ > > data/os/suse.com/sled-12.2.xml.in | 3 +++ > > data/os/suse.com/sles-12.2.xml.in | 3 +++ > > data/os/ubuntu.com/ubuntu-17.04.xml.in | 3 +++ > > data/schema/osinfo.rng.in | 1 + > > 13 files changed, 56 insertions(+), 1 deletion(-) > > create mode 100644 data/device/pcisig.com/pci-8086- > > 10d3.d/class.xml.in > > create mode 100644 data/device/pcisig.com/pci-8086- > > 293e.d/class.xml.in > > create mode 100644 data/device/qemu.org/chipset-x86_64- > > q35.d/class.xml.in > > > > Best Regards, _______________________________________________ Libosinfo mailing list Libosinfo@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libosinfo
