This command is a virsh wrapper for virConnectCompareHypervisorCPU.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
tools/virsh-host.c | 113 +++++++++++++++++++++++++++++++++++++++++++++
tools/virsh.pod | 29 +++++++++++-
2 files changed, 141 insertions(+), 1 deletion(-)
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index ea2c411c02..1e7cfcbd5e 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -1595,6 +1595,113 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
+
+/*
+ * "hypervisor-cpu-compare" command
+ */
+static const vshCmdInfo info_hypervisor_cpu_compare[] = {
+ {.name = "help",
+ .data = N_("compare a CPU with the CPU created by a hypervisor on the host")
+ },
+ {.name = "desc",
+ .data = N_("compare CPU with hypervisor CPU")
+ },
+ {.name = NULL}
+};
+
+static const vshCmdOptDef opts_hypervisor_cpu_compare[] = {
+ VIRSH_COMMON_OPT_FILE(N_("file containing an XML CPU description")),
+ {.name = "virttype",
+ .type = VSH_OT_STRING,
+ .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,
+ .help = N_("domain architecture (/domain/os/type/@arch)"),
+ },
+ {.name = "machine",
+ .type = VSH_OT_STRING,
+ .help = N_("machine type (/domain/os/type/@machine)"),
+ },
+ {.name = "error",
+ .type = VSH_OT_BOOL,
+ .help = N_("report error if CPUs are incompatible")
+ },
+ {.name = NULL}
+};
+
+static bool
+cmdHypervisorCPUCompare(vshControl *ctl,
+ const vshCmd *cmd)
+{
+ const char *from = NULL;
+ const char *virttype = NULL;
+ const char *emulator = NULL;
+ const char *arch = NULL;
+ const char *machine = NULL;
+ bool ret = false;
+ int result;
+ char **cpus = NULL;
+ unsigned int flags = 0;
+ virshControlPtr priv = ctl->privData;
+
+ if (vshCommandOptBool(cmd, "error"))
+ flags |= VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE;
+
+ if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0 ||
+ 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;
+
+ if (!(cpus = vshExtractCPUDefXMLs(ctl, from)))
+ return false;
+
+ result = virConnectCompareHypervisorCPU(priv->conn, emulator, arch,
+ machine, virttype, cpus[0], flags);
+
+ switch (result) {
+ case VIR_CPU_COMPARE_INCOMPATIBLE:
+ vshPrint(ctl,
+ _("CPU described in %s is incompatible with the CPU provided "
+ "by hypervisor on the host\n"),
+ from);
+ goto cleanup;
+ break;
+
+ case VIR_CPU_COMPARE_IDENTICAL:
+ vshPrint(ctl,
+ _("CPU described in %s is identical to the CPU provided by "
+ "hypervisor on the host\n"),
+ from);
+ break;
+
+ case VIR_CPU_COMPARE_SUPERSET:
+ vshPrint(ctl,
+ _("The CPU provided by hypervisor on the host is a superset "
+ "of CPU described in %s\n"),
+ from);
+ break;
+
+ case VIR_CPU_COMPARE_ERROR:
+ default:
+ vshError(ctl, _("Failed to compare hypervisor CPU with %s"), from);
+ goto cleanup;
+ }
+
+ ret = true;
+
+ cleanup:
+ virStringListFree(cpus);
+ return ret;
+}
+
+
const vshCmdDef hostAndHypervisorCmds[] = {
{.name = "allocpages",
.handler = cmdAllocpages,
@@ -1650,6 +1757,12 @@ const vshCmdDef hostAndHypervisorCmds[] = {
.info = info_hostname,
.flags = 0
},
+ {.name = "hypervisor-cpu-compare",
+ .handler = cmdHypervisorCPUCompare,
+ .opts = opts_hypervisor_cpu_compare,
+ .info = info_hypervisor_cpu_compare,
+ .flags = 0
+ },
{.name = "maxvcpus",
.handler = cmdMaxvcpus,
.opts = opts_maxvcpus,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index ea10e1ad43..1a55092efd 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -585,7 +585,9 @@ features that block migration will not be included in the resulting CPU.
=item B<cpu-compare> I<FILE> [I<--error>]
-Compare CPU definition from XML <file> with host CPU. The XML <file> may
+Compare CPU definition from XML <file> with host CPU. (See
+B<hypervisor-cpu-compare> command for comparing the CPU definition with the CPU
+which a specific hypervisor is able to provide on the host.) The XML <file> may
contain either host or guest CPU definition. The host CPU definition is the
<cpu> element and its contents as printed by B<capabilities> command. The
guest CPU definition is the <cpu> element and its contents from domain XML
@@ -616,6 +618,31 @@ specified, then the output will be single-quoted where needed, so that
it is suitable for reuse in a shell context. If I<--xml> is
specified, then the output will be escaped for use in XML.
+=item B<hypervisor-cpu-compare> I<FILE> [I<virttype>] [I<emulator>] [I<arch>]
+[I<machine>] [I<--error>]
+
+Compare CPU definition from XML <file> with the CPU the specified hypervisor
+is able to provide on the host. (This is different from B<cpu-compare> which
+compares the CPU definition with the host CPU without considering any specific
+hypervisor and its abilities.)
+
+The XML I<FILE> may contain either host or guest CPU definition. The host CPU
+definition is the <cpu> element and its contents as printed by B<capabilities>
+command. The guest CPU definition is the <cpu> element and its contents from
+domain XML definition or the CPU definition created from the host CPU model
+found in domain capabilities XML (printed by B<domcapabilities> command). In
+addition to the <cpu> element itself, this command accepts full domain XML,
+capabilities XML, or domain capabilities XML containing the CPU definition. For
+more information on guest CPU definition see:
+L<https://libvirt.org/formatdomain.html#elementsCPU>.
+
+The I<virttype> option specifies the virtualization type (usable in the 'type'
+attribute of the <domain> top level element from the domain XML). I<emulator>
+specifies the path to the emulator, I<arch> specifies the CPU architecture, and
+I<machine> specifies the machine type. If I<--error> is specified, the command
+will return an error when the given CPU is incompatible with host CPU and a
+message providing more details about the incompatibility will be printed out.
+
=back
=head1 DOMAIN COMMANDS
--
2.17.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
I've applied and looked at the patches up to this point. Things are looking good thus far. Will give them another once-over tomorrow and continue with the rest of the patches. -- Respectfully, - Collin Walling -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, May 16, 2018 at 10:39:29AM +0200, Jiri Denemark wrote: >This command is a virsh wrapper for virConnectCompareHypervisorCPU. > >Signed-off-by: Jiri Denemark <jdenemar@redhat.com> >--- > tools/virsh-host.c | 113 +++++++++++++++++++++++++++++++++++++++++++++ > tools/virsh.pod | 29 +++++++++++- > 2 files changed, 141 insertions(+), 1 deletion(-) > Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 05/16/2018 04:39 AM, Jiri Denemark wrote:
> This command is a virsh wrapper for virConnectCompareHypervisorCPU.
>
> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> ---
> tools/virsh-host.c | 113 +++++++++++++++++++++++++++++++++++++++++++++
> tools/virsh.pod | 29 +++++++++++-
> 2 files changed, 141 insertions(+), 1 deletion(-)
>
> diff --git a/tools/virsh-host.c b/tools/virsh-host.c
> index ea2c411c02..1e7cfcbd5e 100644
> --- a/tools/virsh-host.c
> +++ b/tools/virsh-host.c
> @@ -1595,6 +1595,113 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd)
> goto cleanup;
> }
>
> +
> +/*
> + * "hypervisor-cpu-compare" command
> + */
Really just a nit:
I'm somewhat torn by the verbose command name. "hypervisor-" is a bit cumbersome,
but hy<tab> will auto-complete it for you at this point. Maybe shorten it to hv-cpu-compare?
> +static const vshCmdInfo info_hypervisor_cpu_compare[] = {
> + {.name = "help",
> + .data = N_("compare a CPU with the CPU created by a hypervisor on the host")
> + },
> + {.name = "desc",
> + .data = N_("compare CPU with hypervisor CPU")
> + },
> + {.name = NULL}
> +};
> +
> +static const vshCmdOptDef opts_hypervisor_cpu_compare[] = {
> + VIRSH_COMMON_OPT_FILE(N_("file containing an XML CPU description")),
> + {.name = "virttype",
> + .type = VSH_OT_STRING,
> + .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,
> + .help = N_("domain architecture (/domain/os/type/@arch)"),
> + },
Your documentation states that this option is the "CPU architecture", which
I think I like more than "domain architecture".
> + {.name = "machine",
> + .type = VSH_OT_STRING,
> + .help = N_("machine type (/domain/os/type/@machine)"),
> + },
> + {.name = "error",
> + .type = VSH_OT_BOOL,
> + .help = N_("report error if CPUs are incompatible")
> + },
> + {.name = NULL}
> +};
> +
> +static bool
> +cmdHypervisorCPUCompare(vshControl *ctl,
> + const vshCmd *cmd)
> +{
> + const char *from = NULL;
> + const char *virttype = NULL;
> + const char *emulator = NULL;
> + const char *arch = NULL;
> + const char *machine = NULL;
> + bool ret = false;
> + int result;
> + char **cpus = NULL;
> + unsigned int flags = 0;
> + virshControlPtr priv = ctl->privData;
> +
> + if (vshCommandOptBool(cmd, "error"))
> + flags |= VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE;
> +
> + if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0 ||
> + 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;
> +
> + if (!(cpus = vshExtractCPUDefXMLs(ctl, from)))
> + return false;
> +
> + result = virConnectCompareHypervisorCPU(priv->conn, emulator, arch,
> + machine, virttype, cpus[0], flags);
> +
> + switch (result) {
> + case VIR_CPU_COMPARE_INCOMPATIBLE:
> + vshPrint(ctl,
> + _("CPU described in %s is incompatible with the CPU provided "
> + "by hypervisor on the host\n"),
> + from);
How much information regarding a CPU definition does libvirt consider when comparing CPU's
for x86 (and for other archs, if you happen to know)? On s390, we only take the cpu model
and features into consideration.
If the archs other than s390 will only use the cpu model and features as well -- or if this
API should explicitly work only with CPU models -- then perhaps it is more accurate for these
messages to state "CPU model" instead of "CPU"? This change would also have to be propagated
in to the documentation, replacing "CPU definition" with "CPU model".
> + goto cleanup;
> + break;
> +
> + case VIR_CPU_COMPARE_IDENTICAL:
> + vshPrint(ctl,
> + _("CPU described in %s is identical to the CPU provided by "
> + "hypervisor on the host\n"),
> + from);
> + break;
> +
> + case VIR_CPU_COMPARE_SUPERSET:
> + vshPrint(ctl,
> + _("The CPU provided by hypervisor on the host is a superset "
> + "of CPU described in %s\n"),
> + from);
> + break;
> +
> + case VIR_CPU_COMPARE_ERROR:
> + default:
> + vshError(ctl, _("Failed to compare hypervisor CPU with %s"), from);
> + goto cleanup;
> + }
> +
> + ret = true;
> +
> + cleanup:
> + virStringListFree(cpus);
> + return ret;
> +}
> +
> +
> const vshCmdDef hostAndHypervisorCmds[] = {
> {.name = "allocpages",
> .handler = cmdAllocpages,
> @@ -1650,6 +1757,12 @@ const vshCmdDef hostAndHypervisorCmds[] = {
> .info = info_hostname,
> .flags = 0
> },
> + {.name = "hypervisor-cpu-compare",
> + .handler = cmdHypervisorCPUCompare,
> + .opts = opts_hypervisor_cpu_compare,
> + .info = info_hypervisor_cpu_compare,
> + .flags = 0
> + },
> {.name = "maxvcpus",
> .handler = cmdMaxvcpus,
> .opts = opts_maxvcpus,
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index ea10e1ad43..1a55092efd 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -585,7 +585,9 @@ features that block migration will not be included in the resulting CPU.
>
> =item B<cpu-compare> I<FILE> [I<--error>]
>
> -Compare CPU definition from XML <file> with host CPU. The XML <file> may
> +Compare CPU definition from XML <file> with host CPU. (See
> +B<hypervisor-cpu-compare> command for comparing the CPU definition with the CPU
> +which a specific hypervisor is able to provide on the host.) The XML <file> may
> contain either host or guest CPU definition. The host CPU definition is the
> <cpu> element and its contents as printed by B<capabilities> command. The
> guest CPU definition is the <cpu> element and its contents from domain XML
> @@ -616,6 +618,31 @@ specified, then the output will be single-quoted where needed, so that
> it is suitable for reuse in a shell context. If I<--xml> is
> specified, then the output will be escaped for use in XML.
>
> +=item B<hypervisor-cpu-compare> I<FILE> [I<virttype>] [I<emulator>] [I<arch>]
> +[I<machine>] [I<--error>]
> +
> +Compare CPU definition from XML <file> with the CPU the specified hypervisor
What is "the specified hypervisor"? To me, this implies that the user would have
to provide the other options to specify a hypervisor for the command, but you can
simply provide the XML <file> and be done.
I think it reads better as:
"Compares the CPU definition from an XML <file> with the CPU the hypervisor is able
to provide on the host."
And then leave it up to your last paragraph (which defines the additional options)
to explain how to "specify" a hypervisor.
> +is able to provide on the host. (This is different from B<cpu-compare> which
> +compares the CPU definition with the host CPU without considering any specific
> +hypervisor and its abilities.)
> +
> +The XML I<FILE> may contain either host or guest CPU definition. The host CPU
"either _a_ host or guest CPU definition"
> +definition is the <cpu> element and its contents as printed by B<capabilities>
"by _the_ B<capabilities> command"
> +command. The guest CPU definition is the <cpu> element and its contents from
"from _the_ domain XML definition"
> +domain XML definition or the CPU definition created from the host CPU model
> +found in domain capabilities XML (printed by B<domcapabilities> command). In
"found in _the_ domain capabilities XML (printed by _the_ B<domcapabilities command)."
> +addition to the <cpu> element itself, this command accepts full domain XML,
> +capabilities XML, or domain capabilities XML containing the CPU definition. For
> +more information on guest CPU definition see:
> +L<https://libvirt.org/formatdomain.html#elementsCPU>.
> +
> +The I<virttype> option specifies the virtualization type (usable in the 'type'
> +attribute of the <domain> top level element from the domain XML). I<emulator>
> +specifies the path to the emulator, I<arch> specifies the CPU architecture, and
> +I<machine> specifies the machine type. If I<--error> is specified, the command
> +will return an error when the given CPU is incompatible with host CPU and a
"is incompatible with _the_ host CPU"
> +message providing more details about the incompatibility will be printed out.
> +
> =back
>
> =head1 DOMAIN COMMANDS
>
Functionally, everything looks good.
--
Respectfully,
- Collin Walling
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, May 24, 2018 at 13:24:25 -0400, Collin Walling wrote:
> On 05/16/2018 04:39 AM, Jiri Denemark wrote:
> > This command is a virsh wrapper for virConnectCompareHypervisorCPU.
> >
> > Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> > ---
> > tools/virsh-host.c | 113 +++++++++++++++++++++++++++++++++++++++++++++
> > tools/virsh.pod | 29 +++++++++++-
> > 2 files changed, 141 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/virsh-host.c b/tools/virsh-host.c
> > index ea2c411c02..1e7cfcbd5e 100644
> > --- a/tools/virsh-host.c
> > +++ b/tools/virsh-host.c
> > @@ -1595,6 +1595,113 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd)
> > goto cleanup;
> > }
> >
> > +
> > +/*
> > + * "hypervisor-cpu-compare" command
> > + */
>
> Really just a nit:
>
> I'm somewhat torn by the verbose command name. "hypervisor-" is a bit cumbersome,
> but hy<tab> will auto-complete it for you at this point. Maybe shorten it to hv-cpu-compare?
Yeah, hv-* is definitely shorter, but I don't know if it's better. What
do others think?
> > +static const vshCmdInfo info_hypervisor_cpu_compare[] = {
> > + {.name = "help",
> > + .data = N_("compare a CPU with the CPU created by a hypervisor on the host")
> > + },
> > + {.name = "desc",
> > + .data = N_("compare CPU with hypervisor CPU")
> > + },
> > + {.name = NULL}
> > +};
> > +
> > +static const vshCmdOptDef opts_hypervisor_cpu_compare[] = {
> > + VIRSH_COMMON_OPT_FILE(N_("file containing an XML CPU description")),
> > + {.name = "virttype",
> > + .type = VSH_OT_STRING,
> > + .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,
> > + .help = N_("domain architecture (/domain/os/type/@arch)"),
> > + },
>
> Your documentation states that this option is the "CPU architecture", which
> I think I like more than "domain architecture".
Definitely, I forgot to fix it here.
>
> > + {.name = "machine",
> > + .type = VSH_OT_STRING,
> > + .help = N_("machine type (/domain/os/type/@machine)"),
> > + },
> > + {.name = "error",
> > + .type = VSH_OT_BOOL,
> > + .help = N_("report error if CPUs are incompatible")
> > + },
> > + {.name = NULL}
> > +};
> > +
> > +static bool
> > +cmdHypervisorCPUCompare(vshControl *ctl,
> > + const vshCmd *cmd)
> > +{
> > + const char *from = NULL;
> > + const char *virttype = NULL;
> > + const char *emulator = NULL;
> > + const char *arch = NULL;
> > + const char *machine = NULL;
> > + bool ret = false;
> > + int result;
> > + char **cpus = NULL;
> > + unsigned int flags = 0;
> > + virshControlPtr priv = ctl->privData;
> > +
> > + if (vshCommandOptBool(cmd, "error"))
> > + flags |= VIR_CONNECT_COMPARE_CPU_FAIL_INCOMPATIBLE;
> > +
> > + if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0 ||
> > + 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;
> > +
> > + if (!(cpus = vshExtractCPUDefXMLs(ctl, from)))
> > + return false;
> > +
> > + result = virConnectCompareHypervisorCPU(priv->conn, emulator, arch,
> > + machine, virttype, cpus[0], flags);
> > +
> > + switch (result) {
> > + case VIR_CPU_COMPARE_INCOMPATIBLE:
> > + vshPrint(ctl,
> > + _("CPU described in %s is incompatible with the CPU provided "
> > + "by hypervisor on the host\n"),
> > + from);
>
> How much information regarding a CPU definition does libvirt consider when comparing CPU's
> for x86 (and for other archs, if you happen to know)? On s390, we only take the cpu model
> and features into consideration.
>
> If the archs other than s390 will only use the cpu model and features as well -- or if this
> API should explicitly work only with CPU models -- then perhaps it is more accurate for these
> messages to state "CPU model" instead of "CPU"? This change would also have to be propagated
> in to the documentation, replacing "CPU definition" with "CPU model".
It doesn't really matter what libvirt currently checks for which
architecture. The API takes a CPU definition XML and libvirt will use
anything it needs from that.
...
> > @@ -616,6 +618,31 @@ specified, then the output will be single-quoted where needed, so that
> > it is suitable for reuse in a shell context. If I<--xml> is
> > specified, then the output will be escaped for use in XML.
> >
> > +=item B<hypervisor-cpu-compare> I<FILE> [I<virttype>] [I<emulator>] [I<arch>]
> > +[I<machine>] [I<--error>]
> > +
> > +Compare CPU definition from XML <file> with the CPU the specified hypervisor
>
> What is "the specified hypervisor"? To me, this implies that the user would have
> to provide the other options to specify a hypervisor for the command, but you can
> simply provide the XML <file> and be done.
>
> I think it reads better as:
>
> "Compares the CPU definition from an XML <file> with the CPU the hypervisor is able
> to provide on the host."
Yeah, your version looks better.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, May 25, 2018 at 12:35:09PM +0200, Jiri Denemark wrote: > On Thu, May 24, 2018 at 13:24:25 -0400, Collin Walling wrote: > > On 05/16/2018 04:39 AM, Jiri Denemark wrote: > > > This command is a virsh wrapper for virConnectCompareHypervisorCPU. > > > > > > Signed-off-by: Jiri Denemark <jdenemar@redhat.com> > > > --- > > > tools/virsh-host.c | 113 +++++++++++++++++++++++++++++++++++++++++++++ > > > tools/virsh.pod | 29 +++++++++++- > > > 2 files changed, 141 insertions(+), 1 deletion(-) > > > > > > diff --git a/tools/virsh-host.c b/tools/virsh-host.c > > > index ea2c411c02..1e7cfcbd5e 100644 > > > --- a/tools/virsh-host.c > > > +++ b/tools/virsh-host.c > > > @@ -1595,6 +1595,113 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd) > > > goto cleanup; > > > } > > > > > > + > > > +/* > > > + * "hypervisor-cpu-compare" command > > > + */ > > > > Really just a nit: > > > > I'm somewhat torn by the verbose command name. "hypervisor-" is a bit cumbersome, > > but hy<tab> will auto-complete it for you at this point. Maybe shorten it to hv-cpu-compare? > > Yeah, hv-* is definitely shorter, but I don't know if it's better. What > do others think? Since you asked (and as a heavy `virsh` user) ... although I like shorter commands, I go by "explicit is better than implicit" (within reason) with written text. FWIW, I lean towards the full spelling 'hypervisor'; one less acronym to auto-expand in your head. [...] -- /kashyap -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
>>> + switch (result) {
>>> + case VIR_CPU_COMPARE_INCOMPATIBLE:
>>> + vshPrint(ctl,
>>> + _("CPU described in %s is incompatible with the CPU provided "
>>> + "by hypervisor on the host\n"),
>>> + from);
>>
>> How much information regarding a CPU definition does libvirt consider when comparing CPU's
>> for x86 (and for other archs, if you happen to know)? On s390, we only take the cpu model
>> and features into consideration.
>>
>> If the archs other than s390 will only use the cpu model and features as well -- or if this
>> API should explicitly work only with CPU models -- then perhaps it is more accurate for these
>> messages to state "CPU model" instead of "CPU"? This change would also have to be propagated
>> in to the documentation, replacing "CPU definition" with "CPU model".
>
> It doesn't really matter what libvirt currently checks for which
> architecture. The API takes a CPU definition XML and libvirt will use
> anything it needs from that.
>
I had to bat this around in my head a bit. Truthfully, I think trying to expand on
why we got the result might be a little much. Perhaps I should have more faith in the
user to understand what is taken into consideration when CPUs are compared :)
Reviewed-by: Collin Walling <walling@linux.ibm.com>
--
Respectfully,
- Collin Walling
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, May 25, 2018 at 15:22:43 -0400, Collin Walling wrote:
> >>> + switch (result) {
> >>> + case VIR_CPU_COMPARE_INCOMPATIBLE:
> >>> + vshPrint(ctl,
> >>> + _("CPU described in %s is incompatible with the CPU provided "
> >>> + "by hypervisor on the host\n"),
> >>> + from);
> >>
> >> How much information regarding a CPU definition does libvirt consider when comparing CPU's
> >> for x86 (and for other archs, if you happen to know)? On s390, we only take the cpu model
> >> and features into consideration.
> >>
> >> If the archs other than s390 will only use the cpu model and features as well -- or if this
> >> API should explicitly work only with CPU models -- then perhaps it is more accurate for these
> >> messages to state "CPU model" instead of "CPU"? This change would also have to be propagated
> >> in to the documentation, replacing "CPU definition" with "CPU model".
> >
> > It doesn't really matter what libvirt currently checks for which
> > architecture. The API takes a CPU definition XML and libvirt will use
> > anything it needs from that.
> >
>
> I had to bat this around in my head a bit. Truthfully, I think trying to expand on
> why we got the result might be a little much.
That's (partially) the reason behind --error option. It will cause the
API to return an error (rather than VIR_CPU_COMPARE_INCOMPATIBLE) and
you can check the error messages for details about the incompatibility.
For example, the x86 CPU driver will list all missing features.
> Perhaps I should have more faith in the user to understand what is
> taken into consideration when CPUs are compared :)
The user should not really need to know what is used for comparison.
They would just have a CPU definition they want to use for a guest and
this API can be used to check whether such CPU can be run on a given
host.
For example, oVirt has a set of CPU definitions from which a user can
select and calls the compare API to check which hosts can run which CPU
so that they can properly schedule where to run individual VMs.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.