docs/manpages/virsh.rst | 20 ++++++++ include/libvirt/libvirt-host.h | 7 +++ src/driver-hypervisor.h | 10 ++++ src/libvirt-host.c | 65 +++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++ src/qemu/qemu_driver.c | 59 +++++++++++++++++++++++ src/remote/remote_daemon_dispatch.c | 62 ++++++++++++++++++++++++ src/remote/remote_driver.c | 52 ++++++++++++++++++++ src/remote/remote_protocol.x | 26 +++++++++- src/remote_protocol-structs | 16 +++++++ tools/virsh-host.c | 74 +++++++++++++++++++++++++++++ 11 files changed, 395 insertions(+), 1 deletion(-)
Allows for the query of hypervisor-known CPU models via the simple command: virsh hypervisor-cpu-models. For the QEMU driver, the models are queried via the capabilities file. Each model is printed to the terminal on its own line similar to the cpu-models command, and there is no order to the listing. There is a related libvirt-python merge request at https://gitlab.com/libvirt/libvirt-python/-/merge_requests/159 David Judkovics (4): libvirt: Introduce virConnectGetHypervisorCPUModelNames public API remote: Implement virConnectGetHypervisorCPUModelNames qemu_driver: Implement qemuConnectGetHypervisorCPUModelNames virsh: Introduce new hypervisor-cpu-models command docs/manpages/virsh.rst | 20 ++++++++ include/libvirt/libvirt-host.h | 7 +++ src/driver-hypervisor.h | 10 ++++ src/libvirt-host.c | 65 +++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++ src/qemu/qemu_driver.c | 59 +++++++++++++++++++++++ src/remote/remote_daemon_dispatch.c | 62 ++++++++++++++++++++++++ src/remote/remote_driver.c | 52 ++++++++++++++++++++ src/remote/remote_protocol.x | 26 +++++++++- src/remote_protocol-structs | 16 +++++++ tools/virsh-host.c | 74 +++++++++++++++++++++++++++++ 11 files changed, 395 insertions(+), 1 deletion(-) -- 2.47.0
On Wed, Jan 15, 2025 at 11:34:05AM +0100, Boris Fiuczynski wrote: > Allows for the query of hypervisor-known CPU models via the simple > command: virsh hypervisor-cpu-models. For the QEMU driver, the models > are queried via the capabilities file. Each model is printed to the > terminal on its own line similar to the cpu-models command, and there > is no order to the listing. This is just duplicating what we already expose to users in virConnectGetDomainCapabilities / virsh domcapabilities. eg # virsh domcapabilities --xpath '//cpu//model/text()' Snowridge qemu64 qemu32 phenom pentium3 pentium2 pentium n270 kvm64 kvm32 coreduo core2duo athlon Westmere-IBRS Westmere Snowridge Skylake-Server-noTSX-IBRS Skylake-Server-IBRS Skylake-Server Skylake-Client-noTSX-IBRS Skylake-Client-IBRS Skylake-Client SapphireRapids ...snip... Yes, we currently have a virConnectGetCPUModelNames(), but when we introduced the new QEMU-detectectable CPU support, we intended for the virConnectGetDomainCapabilities API to replace the former use case, so didn't add a new CPUModelNames API With 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 :|
On 15/01/2025 11.43, Daniel P. Berrangé wrote: > On Wed, Jan 15, 2025 at 11:34:05AM +0100, Boris Fiuczynski wrote: >> Allows for the query of hypervisor-known CPU models via the simple >> command: virsh hypervisor-cpu-models. For the QEMU driver, the models >> are queried via the capabilities file. Each model is printed to the >> terminal on its own line similar to the cpu-models command, and there >> is no order to the listing. > > This is just duplicating what we already expose to users in > virConnectGetDomainCapabilities / virsh domcapabilities. > > eg > > # virsh domcapabilities --xpath '//cpu//model/text()' Hi Daniel! From a plain end-users perspective (which I belong to here in this case), I think it would be very helpful to have a "virsh hypervisor-cpu-models" command! I already ran into this problem in the past: I wanted to get a list of CPU models that are supported by the hypervisor, I had to discover that "virsh cpu-models" is completely useless on s390x, I saw that there are already "hypervisor-cpu-compare" and "hypervisor-cpu-baseline" and then started wondering why there is nothing similar for easily getting the list of CPU models here. As an average user, it can be quite hard to figure out that you have to use "virsh domcapabilities" for this instead. Last time this has been discussed, you also mentioned that you wouldn't object such a command if it's based on GetDomainCapabilities (see https://marc.info/?i=ZHCZeXbywKCR5jzw@redhat.com), which seems now to be the case if I got patch 3/4 right? And considering that other people ran into this problem already, too (see https://web.archive.org/web/20230817214310/https://listman.redhat.com/archives/libvir-list/2022-June/232626.html ), I'd appreciate if we could give this a chance, please? Thanks, Thomas
On Wed, Jan 15, 2025 at 04:07:38PM +0100, Thomas Huth wrote: > On 15/01/2025 11.43, Daniel P. Berrangé wrote: > > On Wed, Jan 15, 2025 at 11:34:05AM +0100, Boris Fiuczynski wrote: > > > Allows for the query of hypervisor-known CPU models via the simple > > > command: virsh hypervisor-cpu-models. For the QEMU driver, the models > > > are queried via the capabilities file. Each model is printed to the > > > terminal on its own line similar to the cpu-models command, and there > > > is no order to the listing. > > > > This is just duplicating what we already expose to users in > > virConnectGetDomainCapabilities / virsh domcapabilities. > > > > eg > > > > # virsh domcapabilities --xpath '//cpu//model/text()' > > Hi Daniel! > > From a plain end-users perspective (which I belong to here in this case), I > think it would be very helpful to have a "virsh hypervisor-cpu-models" > command! I already ran into this problem in the past: I wanted to get a list > of CPU models that are supported by the hypervisor, I had to discover that > "virsh cpu-models" is completely useless on s390x, I saw that there are > already "hypervisor-cpu-compare" and "hypervisor-cpu-baseline" and then > started wondering why there is nothing similar for easily getting the list > of CPU models here. As an average user, it can be quite hard to figure out > that you have to use "virsh domcapabilities" for this instead. > > Last time this has been discussed, you also mentioned that you wouldn't > object such a command if it's based on GetDomainCapabilities (see > https://marc.info/?i=ZHCZeXbywKCR5jzw@redhat.com), which seems now to be the > case if I got patch 3/4 right? And considering that other people ran into > this problem already, too (see https://web.archive.org/web/20230817214310/https://listman.redhat.com/archives/libvir-list/2022-June/232626.html > ), I'd appreciate if we could give this a chance, please? Ah, I knew I had dejavu here. Indeed I did say that a virsh command based on GetDomainCapabilities was OK, but that is not what this series is doing. This is essentially the same as the old series, adding a new public virConnectGetHypervisorCPUModelNames API, which virsh then calls. virsh needs to call virConnectGetDomainCapabilities directly if we want a 'hypervisor-cpu-models' command. With 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 :|
> Indeed I did say that a virsh command based on GetDomainCapabilities
> was OK, but that is not what this series is doing.
>
> This is essentially the same as the old series, adding a new public
> virConnectGetHypervisorCPUModelNames API, which virsh then calls.
>
> virsh needs to call virConnectGetDomainCapabilities directly if we
> want a 'hypervisor-cpu-models' command.
Thanks for your feedback. It seems we have a disconnect with respect to
how this should be designed.
The first iteration I posted some time back pulled the CPU models
directly from the qemuCaps. This iteration instead pulls from the
domCaps, which I thought met the requirements of the feedback since both
the proposed API and existing virConnectGetDomainCapabilities interface
extract data from this structure. From my perspective, it makes sense
to extract the CPU models directly from the data structure and format
the output as desired versus adding an extra step to parse the XML.
This is my understanding of the operations of both approaches if the
steps were to be unfurled:
1. proposed API operations:
Retrieve hypervisor caps -> extract domain caps -> extract CPU
models -> print to stdout
2. virConnectGetDomainCapabilities operations:
Retrieve hypervisor caps -> extract domain caps -> format into XML
-> parse CPU models -> print to stdout
My goal is to work together to clear up any misunderstandings by laying
out the design as much as possible so we can agree on the approach for
the next iteration.
--
Regards,
Collin
On Fri, Jan 17, 2025 at 04:08:40PM -0000, walling@linux.ibm.com wrote: > > Indeed I did say that a virsh command based on GetDomainCapabilities > > was OK, but that is not what this series is doing. > > > > This is essentially the same as the old series, adding a new public > > virConnectGetHypervisorCPUModelNames API, which virsh then calls. > > > > virsh needs to call virConnectGetDomainCapabilities directly if we > > want a 'hypervisor-cpu-models' command. > > Thanks for your feedback. It seems we have a disconnect with respect to > how this should be designed. > > The first iteration I posted some time back pulled the CPU models > directly from the qemuCaps. This iteration instead pulls from the > domCaps, which I thought met the requirements of the feedback since both > the proposed API and existing virConnectGetDomainCapabilities interface > extract data from this structure. From my perspective, it makes sense > to extract the CPU models directly from the data structure and format > the output as desired versus adding an extra step to parse the XML. Adding new public APIs is an expensive task, especially as it ripples out through many language bindings and/or 3rd party protocol impls. We try to design APIs to be extensible to minimize the number of new APIs we must add in future. The virConnectGetDomainCapabilities API used XML format because we knew we have a huge amount of info to return that is growing all the time and we didn't want APIs for each different bit of information. Thus the idea of adding a virConnectGetHypervisorCPUModelNames API is contrary to our desired design approach and not acceptable. > > This is my understanding of the operations of both approaches if the > steps were to be unfurled: > > 1. proposed API operations: > Retrieve hypervisor caps -> extract domain caps -> extract CPU > models -> print to stdout > > 2. virConnectGetDomainCapabilities operations: > Retrieve hypervisor caps -> extract domain caps -> format into XML > -> parse CPU models -> print to stdout > > My goal is to work together to clear up any misunderstandings by laying > out the design as much as possible so we can agree on the approach for > the next iteration. To be clear if you want such a command in virsh, the expectation is that virsh calls virConnectGetDomainCapabilities, and parses the XML it gets back to extract the CPU model names. This shouldn't actually be that difficult, as you don't really need to parse the XML manually, just invoke an XPath expression to directly extract the CPU model names, letting the xpath library do the hard work. With 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 :|
On 1/17/25 11:15 AM, Daniel P. Berrangé wrote: > On Fri, Jan 17, 2025 at 04:08:40PM -0000, walling@linux.ibm.com wrote: >>> Indeed I did say that a virsh command based on GetDomainCapabilities >>> was OK, but that is not what this series is doing. >>> >>> This is essentially the same as the old series, adding a new public >>> virConnectGetHypervisorCPUModelNames API, which virsh then calls. >>> >>> virsh needs to call virConnectGetDomainCapabilities directly if we >>> want a 'hypervisor-cpu-models' command. >> >> Thanks for your feedback. It seems we have a disconnect with respect to >> how this should be designed. >> >> The first iteration I posted some time back pulled the CPU models >> directly from the qemuCaps. This iteration instead pulls from the >> domCaps, which I thought met the requirements of the feedback since both >> the proposed API and existing virConnectGetDomainCapabilities interface >> extract data from this structure. From my perspective, it makes sense >> to extract the CPU models directly from the data structure and format >> the output as desired versus adding an extra step to parse the XML. > > Adding new public APIs is an expensive task, especially as it ripples > out through many language bindings and/or 3rd party protocol impls. > > We try to design APIs to be extensible to minimize the number of new > APIs we must add in future. The virConnectGetDomainCapabilities API > used XML format because we knew we have a huge amount of info to > return that is growing all the time and we didn't want APIs for each > different bit of information. > > Thus the idea of adding a virConnectGetHypervisorCPUModelNames API is > contrary to our desired design approach and not acceptable. > Thanks for explaining this. It helps to gain insight into libvirt's approach to these things. I never considered the 3rd party implementations (even though the remote/* code eludes to this). >> >> This is my understanding of the operations of both approaches if the >> steps were to be unfurled: >> >> 1. proposed API operations: >> Retrieve hypervisor caps -> extract domain caps -> extract CPU >> models -> print to stdout >> >> 2. virConnectGetDomainCapabilities operations: >> Retrieve hypervisor caps -> extract domain caps -> format into XML >> -> parse CPU models -> print to stdout >> >> My goal is to work together to clear up any misunderstandings by laying >> out the design as much as possible so we can agree on the approach for >> the next iteration. > > To be clear if you want such a command in virsh, the expectation is that > virsh calls virConnectGetDomainCapabilities, and parses the XML it gets > back to extract the CPU model names. This shouldn't actually be that > difficult, as you don't really need to parse the XML manually, just invoke > an XPath expression to directly extract the CPU model names, letting the > xpath library do the hard work. > We'll do this for v2. > With regards, > Daniel -- Regards, Collin
© 2016 - 2025 Red Hat, Inc.