On 1/17/25 11:15 AM, Daniel P. Berrangé wrote: > On Fri, Jan 17, 2025 at 04:08:40PM -0000, walling@xxxxxxxxxxxxx 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