[PATCH v1 3/4] virsh: Introduce new hypervisor-cpu-models command

Collin Walling posted 4 patches 2 years, 10 months ago
[PATCH v1 3/4] virsh: Introduce new hypervisor-cpu-models command
Posted by Collin Walling 2 years, 10 months ago
This command is a virsh wrapper for virConnectGetHypervisorCPUNames.

Signed-off-by: Collin Walling <walling@linux.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
---
 docs/manpages/virsh.rst | 20 ++++++++++++
 tools/virsh-host.c      | 70 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 26c328d390..c156e60d6e 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -1004,6 +1004,26 @@ listed in the XML description. If *--migratable* is specified, features that
 block migration will not be included in the resulting CPU.
 
 
+hypervisor-cpu-models
+---------------------
+
+**Syntax:**
+
+::
+
+   hypervisor-cpu-models [virttype] [emulator] [arch] [machine]
+
+Print the list of CPU models known by the hypervisor for the specified architecture.
+It is not guaranteed that a listed CPU will run on the host. To determine CPU
+model compatibility with the host, see ``virsh hypervisor-cpu-baseline`` and
+``virsh hypervisor-cpu-compare``.
+
+The *virttype* option specifies the virtualization type (usable in the 'type'
+attribute of the <domain> top level element from the domain XML). *emulator*
+specifies the path to the emulator, *arch* specifies the CPU architecture, and
+*machine* specifies the machine type.
+
+
 DOMAIN COMMANDS
 ===============
 
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 21d479fd01..2eedc1517b 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -1779,6 +1779,70 @@ cmdHypervisorCPUBaseline(vshControl *ctl,
     return ret;
 }
 
+/*
+ * "hypervisor-cpu-models" command
+ */
+static const vshCmdInfo info_hypervisor_cpu_models[] = {
+    {.name = "help",
+     .data = N_("Hypervisor reported CPU models")
+    },
+    {.name = "desc",
+     .data = N_("Get the CPU models reported by the hypervisor.")
+    },
+    {.name = NULL}
+};
+
+static const vshCmdOptDef opts_hypervisor_cpu_models[] = {
+    {.name = "virttype",
+     .type = VSH_OT_STRING,
+     .completer = virshDomainVirtTypeCompleter,
+     .help = N_("virtualization type (/domain/@type)"),
+    },
+    {.name = "emulator",
+     .type = VSH_OT_STRING,
+     .help = N_("path to emulator binary (/domain/devices/emulator)"),
+    },
+    {.name = "arch",
+     .type = VSH_OT_STRING,
+     .completer = virshArchCompleter,
+     .help = N_("CPU architecture (/domain/os/type/@arch)"),
+    },
+    {.name = "machine",
+     .type = VSH_OT_STRING,
+     .help = N_("machine type (/domain/os/type/@machine)"),
+    },
+    {.name = NULL}
+};
+
+static bool
+cmdHypervisorCPUModelNames(vshControl *ctl,
+                           const vshCmd *cmd)
+{
+    const char *virttype = NULL;
+    const char *emulator = NULL;
+    const char *arch = NULL;
+    const char *machine = NULL;
+    g_autofree char *result = NULL;
+    virshControl *priv = ctl->privData;
+
+    if (vshCommandOptStringReq(ctl, cmd, "virttype", &virttype) < 0 ||
+        vshCommandOptStringReq(ctl, cmd, "emulator", &emulator) < 0 ||
+        vshCommandOptStringReq(ctl, cmd, "arch", &arch) < 0 ||
+        vshCommandOptStringReq(ctl, cmd, "machine", &machine) < 0)
+        return false;
+
+    result = virConnectGetHypervisorCPUModelNames(priv->conn, emulator, arch,
+                                                  machine, virttype, 0);
+
+    if (!result) {
+        vshError(ctl, "%s", _("failed to get CPU model names"));
+        return false;
+    }
+
+    vshPrint(ctl, "%s\n", result);
+    return true;
+}
+
 
 const vshCmdDef hostAndHypervisorCmds[] = {
     {.name = "allocpages",
@@ -1847,6 +1911,12 @@ const vshCmdDef hostAndHypervisorCmds[] = {
      .info = info_hypervisor_cpu_compare,
      .flags = 0
     },
+    {.name = "hypervisor-cpu-models",
+     .handler = cmdHypervisorCPUModelNames,
+     .opts = opts_hypervisor_cpu_models,
+     .info = info_hypervisor_cpu_models,
+     .flags = 0
+    },
     {.name = "maxvcpus",
      .handler = cmdMaxvcpus,
      .opts = opts_maxvcpus,
-- 
2.39.0
Re: [PATCH v1 3/4] virsh: Introduce new hypervisor-cpu-models command
Posted by Daniel P. Berrangé 2 years, 10 months ago
On Wed, Mar 22, 2023 at 11:39:17AM -0400, Collin Walling wrote:
> This command is a virsh wrapper for virConnectGetHypervisorCPUNames.
> 
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
> ---
>  docs/manpages/virsh.rst | 20 ++++++++++++
>  tools/virsh-host.c      | 70 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 90 insertions(+)
> 
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index 26c328d390..c156e60d6e 100644
> --- a/docs/manpages/virsh.rst
> +++ b/docs/manpages/virsh.rst
> @@ -1004,6 +1004,26 @@ listed in the XML description. If *--migratable* is specified, features that
>  block migration will not be included in the resulting CPU.
>  
>  
> +hypervisor-cpu-models
> +---------------------
> +
> +**Syntax:**
> +
> +::
> +
> +   hypervisor-cpu-models [virttype] [emulator] [arch] [machine]
> +
> +Print the list of CPU models known by the hypervisor for the specified architecture.
> +It is not guaranteed that a listed CPU will run on the host. To determine CPU
> +model compatibility with the host, see ``virsh hypervisor-cpu-baseline`` and
> +``virsh hypervisor-cpu-compare``.
> +
> +The *virttype* option specifies the virtualization type (usable in the 'type'
> +attribute of the <domain> top level element from the domain XML). *emulator*
> +specifies the path to the emulator, *arch* specifies the CPU architecture, and
> +*machine* specifies the machine type.

This is redundant IMHO, as domcapabilities already reports, along with
info about the usability.

$ virsh domcapabilities  | xmllint -xpath "//cpu/mode[@name='custom']/model[@usable='yes']/text()" - | sort
486
Broadwell-noTSX
Broadwell-noTSX-IBRS
Conroe
core2duo
coreduo
Haswell-noTSX
Haswell-noTSX-IBRS
IvyBridge
IvyBridge-IBRS
kvm32
kvm64
n270
Nehalem
Nehalem-IBRS
Opteron_G1
Opteron_G2
Penryn
pentium
pentium2
pentium3
qemu32
qemu64
SandyBridge
SandyBridge-IBRS
Skylake-Client-noTSX-IBRS
Westmere
Westmere-IBRS


Admittedly xpath is hairy, but the same info could be got with grep+awk
if desired.

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 :|
Re: [PATCH v1 3/4] virsh: Introduce new hypervisor-cpu-models command
Posted by Boris Fiuczynski 2 years, 10 months ago
On 3/22/23 4:53 PM, Daniel P. Berrangé wrote:
> On Wed, Mar 22, 2023 at 11:39:17AM -0400, Collin Walling wrote:
>> This command is a virsh wrapper for virConnectGetHypervisorCPUNames.
>>
>> Signed-off-by: Collin Walling <walling@linux.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
>> ---
>>   docs/manpages/virsh.rst | 20 ++++++++++++
>>   tools/virsh-host.c      | 70 +++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 90 insertions(+)
>>
>> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
>> index 26c328d390..c156e60d6e 100644
>> --- a/docs/manpages/virsh.rst
>> +++ b/docs/manpages/virsh.rst
>> @@ -1004,6 +1004,26 @@ listed in the XML description. If *--migratable* is specified, features that
>>   block migration will not be included in the resulting CPU.
>>   
>>   
>> +hypervisor-cpu-models
>> +---------------------
>> +
>> +**Syntax:**
>> +
>> +::
>> +
>> +   hypervisor-cpu-models [virttype] [emulator] [arch] [machine]
>> +
>> +Print the list of CPU models known by the hypervisor for the specified architecture.
>> +It is not guaranteed that a listed CPU will run on the host. To determine CPU
>> +model compatibility with the host, see ``virsh hypervisor-cpu-baseline`` and
>> +``virsh hypervisor-cpu-compare``.
>> +
>> +The *virttype* option specifies the virtualization type (usable in the 'type'
>> +attribute of the <domain> top level element from the domain XML). *emulator*
>> +specifies the path to the emulator, *arch* specifies the CPU architecture, and
>> +*machine* specifies the machine type.
> 
> This is redundant IMHO, as domcapabilities already reports, along with
> info about the usability.
> 
> $ virsh domcapabilities  | xmllint -xpath "//cpu/mode[@name='custom']/model[@usable='yes']/text()" - | sort
> 486
> Broadwell-noTSX
> Broadwell-noTSX-IBRS
> Conroe
> core2duo
> coreduo
> Haswell-noTSX
> Haswell-noTSX-IBRS
> IvyBridge
> IvyBridge-IBRS
> kvm32
> kvm64
> n270
> Nehalem
> Nehalem-IBRS
> Opteron_G1
> Opteron_G2
> Penryn
> pentium
> pentium2
> pentium3
> qemu32
> qemu64
> SandyBridge
> SandyBridge-IBRS
> Skylake-Client-noTSX-IBRS
> Westmere
> Westmere-IBRS
> 
> 
> Admittedly xpath is hairy, but the same info could be got with grep+awk
> if desired.
> 
> With regards,
> Daniel
> 

Isn't the redundancy already existing with "virsh cpu-models"?
If users have to use the hypervisor-cpu-* calls and they have used the 
cpu-* calls before shouldn't the usability be about the same?

-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
Re: [PATCH v1 3/4] virsh: Introduce new hypervisor-cpu-models command
Posted by Daniel P. Berrangé 2 years, 10 months ago
On Wed, Mar 22, 2023 at 05:13:06PM +0100, Boris Fiuczynski wrote:
> On 3/22/23 4:53 PM, Daniel P. Berrangé wrote:
> > On Wed, Mar 22, 2023 at 11:39:17AM -0400, Collin Walling wrote:
> > > This command is a virsh wrapper for virConnectGetHypervisorCPUNames.
> > > 
> > > Signed-off-by: Collin Walling <walling@linux.ibm.com>
> > > Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
> > > ---
> > >   docs/manpages/virsh.rst | 20 ++++++++++++
> > >   tools/virsh-host.c      | 70 +++++++++++++++++++++++++++++++++++++++++
> > >   2 files changed, 90 insertions(+)
> > > 
> > > diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> > > index 26c328d390..c156e60d6e 100644
> > > --- a/docs/manpages/virsh.rst
> > > +++ b/docs/manpages/virsh.rst
> > > @@ -1004,6 +1004,26 @@ listed in the XML description. If *--migratable* is specified, features that
> > >   block migration will not be included in the resulting CPU.
> > > +hypervisor-cpu-models
> > > +---------------------
> > > +
> > > +**Syntax:**
> > > +
> > > +::
> > > +
> > > +   hypervisor-cpu-models [virttype] [emulator] [arch] [machine]
> > > +
> > > +Print the list of CPU models known by the hypervisor for the specified architecture.
> > > +It is not guaranteed that a listed CPU will run on the host. To determine CPU
> > > +model compatibility with the host, see ``virsh hypervisor-cpu-baseline`` and
> > > +``virsh hypervisor-cpu-compare``.
> > > +
> > > +The *virttype* option specifies the virtualization type (usable in the 'type'
> > > +attribute of the <domain> top level element from the domain XML). *emulator*
> > > +specifies the path to the emulator, *arch* specifies the CPU architecture, and
> > > +*machine* specifies the machine type.
> > 
> > This is redundant IMHO, as domcapabilities already reports, along with
> > info about the usability.
> > 
> > $ virsh domcapabilities  | xmllint -xpath "//cpu/mode[@name='custom']/model[@usable='yes']/text()" - | sort
> > 486
> > Broadwell-noTSX
> > Broadwell-noTSX-IBRS
> > Conroe
> > core2duo
> > coreduo
> > Haswell-noTSX
> > Haswell-noTSX-IBRS
> > IvyBridge
> > IvyBridge-IBRS
> > kvm32
> > kvm64
> > n270
> > Nehalem
> > Nehalem-IBRS
> > Opteron_G1
> > Opteron_G2
> > Penryn
> > pentium
> > pentium2
> > pentium3
> > qemu32
> > qemu64
> > SandyBridge
> > SandyBridge-IBRS
> > Skylake-Client-noTSX-IBRS
> > Westmere
> > Westmere-IBRS
> > 
> > 
> > Admittedly xpath is hairy, but the same info could be got with grep+awk
> > if desired.
> 
> Isn't the redundancy already existing with "virsh cpu-models"?
> If users have to use the hypervisor-cpu-* calls and they have used the cpu-*
> calls before shouldn't the usability be about the same?

The 'cpu-models' commands came first, but its design was rather flawed.
It just reported CPU models alone and could not reflect whether they
were actually supportable by the hypervisor, and was not extensible to
let us fix it. We had to abandon it and switch to the new approach based
on domain capabilities which is extensible via the XML.

Potentially we could have a virsh hypervisor-cpu-models command that
queries the domain capabilities API, rather than adding a new 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 :|