QEMU 2.11 for ppc64 changed all CPU model names to lower case. Since
libvirt can't change the model names for compatibility reasons, we need
to translate the matching lower case models to the names known by
libvirt.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
src/qemu/qemu_capabilities.c | 28 +++++++++++++++++--
src/qemu/qemu_capabilities.h | 3 +-
src/qemu/qemu_process.c | 2 +-
.../qemu_2.12.0.ppc64.xml | 6 +++-
.../caps_2.12.0.ppc64.xml | 12 ++++----
5 files changed, 40 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index a5cb24fec6..9fee5dcd8a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2217,16 +2217,39 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps,
virDomainCapsCPUModelsPtr
-virQEMUCapsFetchCPUDefinitions(qemuMonitorPtr mon)
+virQEMUCapsFetchCPUDefinitions(qemuMonitorPtr mon,
+ virArch arch)
{
virDomainCapsCPUModelsPtr models = NULL;
qemuMonitorCPUDefInfoPtr *cpus = NULL;
+ char **libvirtModels = NULL;
int ncpus = 0;
size_t i;
if ((ncpus = qemuMonitorGetCPUDefinitions(mon, &cpus)) < 0)
return NULL;
+ if (ARCH_IS_PPC(arch)) {
+ /* QEMU 2.11 for Power renamed all CPU models to lower case, we need
+ * to translate them back to libvirt's upper case model names. */
+ if (virCPUGetModels(arch, &libvirtModels) < 0)
+ goto error;
+
+ if (virStringListLength((const char **)libvirtModels) > 0) {
+ for (i = 0; i < ncpus; i++) {
+ const char *name;
+
+ if (!(name = virStringListSearch((const char **)libvirtModels,
+ cpus[i]->name)))
+ continue;
+
+ VIR_FREE(cpus[i]->name);
+ if (VIR_STRDUP(cpus[i]->name, name) < 0)
+ goto error;
+ }
+ }
+ }
+
if (!(models = virDomainCapsCPUModelsNew(ncpus)))
goto error;
@@ -2247,6 +2270,7 @@ virQEMUCapsFetchCPUDefinitions(qemuMonitorPtr mon)
for (i = 0; i < ncpus; i++)
qemuMonitorCPUDefInfoFree(cpus[i]);
VIR_FREE(cpus);
+ virStringListFree(libvirtModels);
return models;
error:
@@ -2266,7 +2290,7 @@ virQEMUCapsProbeQMPCPUDefinitions(virQEMUCapsPtr qemuCaps,
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QUERY_CPU_DEFINITIONS))
return 0;
- if (!(models = virQEMUCapsFetchCPUDefinitions(mon)))
+ if (!(models = virQEMUCapsFetchCPUDefinitions(mon, qemuCaps->arch)))
return -1;
if (tcg || !virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index d23c34c24d..3ab108a667 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -513,7 +513,8 @@ int virQEMUCapsAddCPUDefinitions(virQEMUCapsPtr qemuCaps,
virDomainCapsCPUUsable usable);
virDomainCapsCPUModelsPtr virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps,
virDomainVirtType type);
-virDomainCapsCPUModelsPtr virQEMUCapsFetchCPUDefinitions(qemuMonitorPtr mon);
+virDomainCapsCPUModelsPtr virQEMUCapsFetchCPUDefinitions(qemuMonitorPtr mon,
+ virArch arch);
typedef enum {
/* Host CPU definition reported in domain capabilities. */
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 5b73a61962..d09ff7fb87 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4127,7 +4127,7 @@ qemuProcessFetchCPUDefinitions(virQEMUDriverPtr driver,
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
goto error;
- models = virQEMUCapsFetchCPUDefinitions(priv->mon);
+ models = virQEMUCapsFetchCPUDefinitions(priv->mon, vm->def->os.arch);
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto error;
diff --git a/tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml b/tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml
index 5fac2ed772..1762417ee2 100644
--- a/tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml
+++ b/tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml
@@ -25,7 +25,11 @@
<mode name='host-model' supported='yes'>
<model fallback='allow'>POWER8</model>
</mode>
- <mode name='custom' supported='no'/>
+ <mode name='custom' supported='yes'>
+ <model usable='unknown'>POWER9</model>
+ <model usable='unknown'>POWER8</model>
+ <model usable='unknown'>POWER7</model>
+ </mode>
</cpu>
<devices>
<disk supported='yes'>
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
index bffe3b3b97..144b8883a2 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
@@ -171,12 +171,12 @@
<cpu type='kvm' name='970mp'/>
<cpu type='kvm' name='970fx'/>
<cpu type='kvm' name='970'/>
- <cpu type='kvm' name='power9'/>
+ <cpu type='kvm' name='POWER9'/>
<cpu type='kvm' name='power8nvl'/>
- <cpu type='kvm' name='power8'/>
+ <cpu type='kvm' name='POWER8'/>
<cpu type='kvm' name='power8e'/>
<cpu type='kvm' name='power7+'/>
- <cpu type='kvm' name='power7'/>
+ <cpu type='kvm' name='POWER7'/>
<cpu type='kvm' name='power5gs'/>
<cpu type='kvm' name='power5+'/>
<cpu type='kvm' name='apollo7pm'/>
@@ -609,12 +609,12 @@
<cpu type='tcg' name='970mp'/>
<cpu type='tcg' name='970fx'/>
<cpu type='tcg' name='970'/>
- <cpu type='tcg' name='power9'/>
+ <cpu type='tcg' name='POWER9'/>
<cpu type='tcg' name='power8nvl'/>
- <cpu type='tcg' name='power8'/>
+ <cpu type='tcg' name='POWER8'/>
<cpu type='tcg' name='power8e'/>
<cpu type='tcg' name='power7+'/>
- <cpu type='tcg' name='power7'/>
+ <cpu type='tcg' name='POWER7'/>
<cpu type='tcg' name='power5gs'/>
<cpu type='tcg' name='power5+'/>
<cpu type='tcg' name='apollo7pm'/>
--
2.17.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, 2018-05-17 at 17:33 +0200, Jiri Denemark wrote:
[...]
> --- a/tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml
> +++ b/tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml
> @@ -25,7 +25,11 @@
> <mode name='host-model' supported='yes'>
> <model fallback='allow'>POWER8</model>
> </mode>
This is quite suspicious - it looks like a proper CPU model, but
it's really a compatibility mode, so it should be lowercase rather
than uppercase. You certainly won't be able to use
<cpu mode='host-model>
<model>POWER8</model>
</cpu>
so why are we advertising the uppercase variant here? Am I missing
something?
> - <mode name='custom' supported='no'/>
> + <mode name='custom' supported='yes'>
> + <model usable='unknown'>POWER9</model>
> + <model usable='unknown'>POWER8</model>
> + <model usable='unknown'>POWER7</model>
> + </mode>
This is of course an improvement, but I'm not sure we want to keep
exposing uppercase model names to users.
I understand we need to keep accepting them for compatibility
reasons, but since QEMU has moved to lowercase CPU model names
wouldn't it make sense for libvirt to follow suit?
Doing so would have the interesting side effect of making the whole
mess with compat modes somewhat sane, at least when it comes to not
having two entirely separate set of names differing only in case.
Then again, I might just be missing some very obvious issues
preventing us from using lowercase names :)
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, May 22, 2018 at 11:02:17 +0200, Andrea Bolognani wrote: > On Thu, 2018-05-17 at 17:33 +0200, Jiri Denemark wrote: > [...] > > --- a/tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml > > +++ b/tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml > > @@ -25,7 +25,11 @@ > > <mode name='host-model' supported='yes'> > > <model fallback='allow'>POWER8</model> > > </mode> > > This is quite suspicious - it looks like a proper CPU model, but > it's really a compatibility mode, so it should be lowercase rather > than uppercase. You certainly won't be able to use > > <cpu mode='host-model> > <model>POWER8</model> > </cpu> > > so why are we advertising the uppercase variant here? Am I missing > something? Hmm, you're right. In general, this is advertising the host CPU (ideally as seen by QEMU), which doesn't really work for ppc since host-model was misused for compatibility modes. I think we'll have to add a special hack to produce <mode name='host-model' supported='yes'/> without showing any CPU model. Ideally, we would somehow list all supported compatibility modes, but this can be left for the future. > > - <mode name='custom' supported='no'/> > > + <mode name='custom' supported='yes'> > > + <model usable='unknown'>POWER9</model> > > + <model usable='unknown'>POWER8</model> > > + <model usable='unknown'>POWER7</model> > > + </mode> > > This is of course an improvement, but I'm not sure we want to keep > exposing uppercase model names to users. > > I understand we need to keep accepting them for compatibility > reasons, but since QEMU has moved to lowercase CPU model names > wouldn't it make sense for libvirt to follow suit? I don't think so. Introducing new aliases (i.e., lower case variants) for the existing models would IMHO cause more troubles than having a mixture of upper case and lower case names (once something like power10 is introduced). Users or apps would have to use a crystal ball to check which CPU model name would be compatible with older libvirt. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, 2018-05-22 at 15:46 +0200, Jiri Denemark wrote:
> On Tue, May 22, 2018 at 11:02:17 +0200, Andrea Bolognani wrote:
> > On Thu, 2018-05-17 at 17:33 +0200, Jiri Denemark wrote:
> > [...]
> > > --- a/tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml
> > > +++ b/tests/domaincapsschemadata/qemu_2.12.0.ppc64.xml
> > > @@ -25,7 +25,11 @@
> > > <mode name='host-model' supported='yes'>
> > > <model fallback='allow'>POWER8</model>
> > > </mode>
> >
> > This is quite suspicious - it looks like a proper CPU model, but
> > it's really a compatibility mode, so it should be lowercase rather
> > than uppercase. You certainly won't be able to use
> >
> > <cpu mode='host-model>
> > <model>POWER8</model>
> > </cpu>
> >
> > so why are we advertising the uppercase variant here? Am I missing
> > something?
>
> Hmm, you're right. In general, this is advertising the host CPU
> (ideally
> as seen by QEMU), which doesn't really work for ppc since host-model
> was
> misused for compatibility modes. I think we'll have to add a special
> hack to produce <mode name='host-model' supported='yes'/> without
> showing any CPU model. Ideally, we would somehow list all supported
> compatibility modes, but this can be left for the future.
Sounds good.
> > > - <mode name='custom' supported='no'/>
> > > + <mode name='custom' supported='yes'>
> > > + <model usable='unknown'>POWER9</model>
> > > + <model usable='unknown'>POWER8</model>
> > > + <model usable='unknown'>POWER7</model>
> > > + </mode>
> >
> > This is of course an improvement, but I'm not sure we want to keep
> > exposing uppercase model names to users.
> >
> > I understand we need to keep accepting them for compatibility
> > reasons, but since QEMU has moved to lowercase CPU model names
> > wouldn't it make sense for libvirt to follow suit?
>
> I don't think so. Introducing new aliases (i.e., lower case variants)
> for the existing models would IMHO cause more troubles than having a
> mixture of upper case and lower case names (once something like
> power10
> is introduced). Users or apps would have to use a crystal ball to
> check
> which CPU model name would be compatible with older libvirt.
You have a point. The current situation is a bit confusing, again
because of the misuse of host-model, but it's probably better to
stick with the confusing situation we've grown used to rather than
change things around for cosmetic reasons.
Plus, it's already strongly recommended to use
<cpu mode='host-model'>
<model>power8</model>
</cpu>
rather than
<cpu mode='custom'>
<model>POWER8</model>
</cpu>
because the resulting QEMU command line is more idiomatic, so
applications and users sticking with the best practices wouldn't
benefit from the change either way.
I disagree on having a mixture of uppercase and lowercase model,
though: that's just bad UI, and a clear violation of the principle
of least surprise; if and when a 'power10' CPU model will be added
to QEMU, we should introduce a suitable 'POWER10' alias along with
the existing ones.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.