The patch adds <cpu> element to domain capabilities XML: <cpu> <mode name='host-passthrough' supported='yes'/> <mode name='host-model' supported='yes'/> <mode name='custom' supported='yes'> <model>Broadwell</model> <model>Broadwell-noTSX</model> ... </mode> </cpu> Applications can use it to inspect what CPU configuration modes are supported for a specific combination of domain type, emulator binary, guest architecture and machine type. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- docs/formatdomaincaps.html.in | 44 ++++++ docs/schemas/domaincaps.rng | 43 ++++++ src/conf/domain_capabilities.c | 150 +++++++++++++++++++++ src/conf/domain_capabilities.h | 33 +++++ src/libvirt_private.syms | 4 + tests/domaincapsschemadata/basic.xml | 5 + tests/domaincapsschemadata/full.xml | 9 ++ tests/domaincapsschemadata/libxl-xenfv-usb.xml | 5 + tests/domaincapsschemadata/libxl-xenfv.xml | 5 + tests/domaincapsschemadata/libxl-xenpv-usb.xml | 5 + tests/domaincapsschemadata/libxl-xenpv.xml | 5 + tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml | 5 + .../qemu_2.6.0-gicv2-virt.aarch64.xml | 5 + .../qemu_2.6.0-gicv3-virt.aarch64.xml | 5 + tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml | 5 + tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml | 5 + tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml | 5 + tests/domaincapstest.c | 9 ++ 18 files changed, 347 insertions(+) diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in index d5a8414..15a13c1 100644 --- a/docs/formatdomaincaps.html.in +++ b/docs/formatdomaincaps.html.in @@ -142,6 +142,50 @@ &lt;loader/&gt; element.</dd> </dl> + <h3><a name="elementsCPU">CPU configuration</a></h3> + + <p> + The <code>cpu</code> element exposes options usable for configuring + guest CPU. + </p> + +<pre> +&lt;domainCapabilities&gt; + ... + &lt;cpu&gt; + &lt;mode name='host-passthrough' supported='yes'/&gt; + &lt;mode name='host-model' supported='yes'/&gt; + &lt;mode name='custom' supported='yes'&gt; + &lt;model&gt;Broadwell&lt;/model&gt; + &lt;model&gt;Broadwell-noTSX&lt;/model&gt; + &lt;model&gt;Haswell&lt;/model&gt; + ... + &lt;/mode&gt; + &lt;/cpu&gt; + ... +&lt;domainCapabilities&gt; +</pre> + + <p> + Each CPU mode understood by libvirt is described with a + <code>mode</code> element which tells whether the particular mode + is supported and provides (when applicable) more details about it: + </p> + + <dl> + <dt><code>host-passthrough</code></dt> + <dd>No mode specific details are provided.</dd> + + <dt><code>host-model</code></dt> + <dd>No mode specific details are provided yet.</dd> + + <dt><code>custom</code></dt> + <dd> + The <code>mode</code> element contains a list of supported CPU + models, each described by a dedicated <code>model</code> element. + </dd> + </dl> + <h3><a name="elementsDevices">Devices</a></h3> <p> diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng index 97da41f..9f3d225 100644 --- a/docs/schemas/domaincaps.rng +++ b/docs/schemas/domaincaps.rng @@ -29,6 +29,9 @@ <ref name='os'/> </optional> <optional> + <ref name='cpu'/> + </optional> + <optional> <ref name='devices'/> </optional> <optional> @@ -68,6 +71,46 @@ </element> </define> + <define name='cpu'> + <element name='cpu'> + <ref name='cpuHost'/> + <ref name='cpuHostModel'/> + <ref name='cpuCustom'/> + </element> + </define> + + <define name='cpuHost'> + <element name='mode'> + <attribute name='name'> + <value>host-passthrough</value> + </attribute> + <ref name='supported'/> + </element> + </define> + + <define name='cpuHostModel'> + <element name='mode'> + <attribute name='name'> + <value>host-model</value> + </attribute> + <ref name='supported'/> + </element> + </define> + + <define name='cpuCustom'> + <element name='mode'> + <attribute name='name'> + <value>custom</value> + </attribute> + <ref name='supported'/> + <zeroOrMore> + <element name='model'> + <text/> + </element> + </zeroOrMore> + </element> + </define> + <define name='devices'> <element name='devices'> <interleave> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index 1676f0e..c07f995 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -30,8 +30,10 @@ #define VIR_FROM_THIS VIR_FROM_CAPABILITIES static virClassPtr virDomainCapsClass; +static virClassPtr virDomainCapsCPUModelsClass; static void virDomainCapsDispose(void *obj); +static void virDomainCapsCPUModelsDispose(void *obj); static int virDomainCapsOnceInit(void) { @@ -40,6 +42,14 @@ static int virDomainCapsOnceInit(void) sizeof(virDomainCaps), virDomainCapsDispose))) return -1; + + virDomainCapsCPUModelsClass = virClassNew(virClassForObject(), + "virDomainCapsCPUModelsClass", + sizeof(virDomainCapsCPUModels), + virDomainCapsCPUModelsDispose); + if (!virDomainCapsCPUModelsClass) + return -1; + return 0; } @@ -68,11 +78,25 @@ virDomainCapsDispose(void *obj) VIR_FREE(caps->path); VIR_FREE(caps->machine); + virObjectUnref(caps->cpu.custom); virDomainCapsStringValuesFree(&caps->os.loader.values); } +static void +virDomainCapsCPUModelsDispose(void *obj) +{ + virDomainCapsCPUModelsPtr cpuModels = obj; + size_t i; + + for (i = 0; i < cpuModels->count; i++) + VIR_FREE(cpuModels->models[i].name); + + VIR_FREE(cpuModels->models); +} + + virDomainCapsPtr virDomainCapsNew(const char *path, const char *machine, @@ -100,6 +124,86 @@ virDomainCapsNew(const char *path, } +virDomainCapsCPUModelsPtr +virDomainCapsCPUModelsNew(size_t nmodels) +{ + virDomainCapsCPUModelsPtr cpuModels = NULL; + + if (virDomainCapsInitialize() < 0) + return NULL; + + if (!(cpuModels = virObjectNew(virDomainCapsCPUModelsClass))) + return NULL; + + if (VIR_ALLOC_N(cpuModels->models, nmodels) < 0) + goto error; + cpuModels->alloc = nmodels; + + return cpuModels; + + error: + virObjectUnref(cpuModels); + return NULL; +} + + +virDomainCapsCPUModelsPtr +virDomainCapsCPUModelsCopy(virDomainCapsCPUModelsPtr old) +{ + virDomainCapsCPUModelsPtr cpuModels; + size_t i; + + if (!(cpuModels = virDomainCapsCPUModelsNew(old->count))) + return NULL; + + for (i = 0; i < old->count; i++) { + if (virDomainCapsCPUModelsAdd(cpuModels, old->models[i].name, -1) < 0) + goto error; + } + + return cpuModels; + + error: + virObjectUnref(cpuModels); + return NULL; +} + + +int +virDomainCapsCPUModelsAddSteal(virDomainCapsCPUModelsPtr cpuModels, + char **name) +{ + if (VIR_RESIZE_N(cpuModels->models, cpuModels->alloc, + cpuModels->count, 1) < 0) + return -1; + + cpuModels->models[cpuModels->count++].name = *name; + *name = NULL; + return 0; +} + + +int +virDomainCapsCPUModelsAdd(virDomainCapsCPUModelsPtr cpuModels, + const char *name, + ssize_t nameLen) +{ + char *copy = NULL; + + if (VIR_STRNDUP(copy, name, nameLen) < 0) + goto error; + + if (virDomainCapsCPUModelsAddSteal(cpuModels, &copy) < 0) + goto error; + + return 0; + + error: + VIR_FREE(copy); + return -1; +} + + int virDomainCapsEnumSet(virDomainCapsEnumPtr capsEnum, const char *capsEnumName, @@ -234,6 +338,51 @@ virDomainCapsOSFormat(virBufferPtr buf, } static void +virDomainCapsCPUCustomFormat(virBufferPtr buf, + virDomainCapsCPUModelsPtr custom) +{ + size_t i; + + virBufferAdjustIndent(buf, 2); + + for (i = 0; i < custom->count; i++) { + virBufferAsprintf(buf, "<model>%s</model>\n", + custom->models[i].name); + } + + virBufferAdjustIndent(buf, -2); +} + +static void +virDomainCapsCPUFormat(virBufferPtr buf, + virDomainCapsCPUPtr cpu) +{ + virBufferAddLit(buf, "<cpu>\n"); + virBufferAdjustIndent(buf, 2); + + virBufferAsprintf(buf, "<mode name='%s' supported='%s'/>\n", + virCPUModeTypeToString(VIR_CPU_MODE_HOST_PASSTHROUGH), + cpu->hostPassthrough ? "yes" : "no"); + + virBufferAsprintf(buf, "<mode name='%s' supported='%s'/>\n", + virCPUModeTypeToString(VIR_CPU_MODE_HOST_MODEL), + cpu->hostModel ? "yes" : "no"); + + virBufferAsprintf(buf, "<mode name='%s' ", + virCPUModeTypeToString(VIR_CPU_MODE_CUSTOM)); + if (cpu->custom && cpu->custom->count) { + virBufferAddLit(buf, "supported='yes'>\n"); + virDomainCapsCPUCustomFormat(buf, cpu->custom); + virBufferAddLit(buf, "</mode>\n"); + } else { + virBufferAddLit(buf, "supported='no'/>\n"); + } + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</cpu>\n"); +} + +static void virDomainCapsDeviceDiskFormat(virBufferPtr buf, virDomainCapsDeviceDiskPtr const disk) { @@ -333,6 +482,7 @@ virDomainCapsFormatInternal(virBufferPtr buf, virBufferAsprintf(buf, "<vcpu max='%d'/>\n", caps->maxvcpus); virDomainCapsOSFormat(buf, &caps->os); + virDomainCapsCPUFormat(buf, &caps->cpu); virBufferAddLit(buf, "<devices>\n"); virBufferAdjustIndent(buf, 2); diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h index 492a9cf..509c306 100644 --- a/src/conf/domain_capabilities.h +++ b/src/conf/domain_capabilities.h @@ -102,6 +102,30 @@ struct _virDomainCapsFeatureGIC { virDomainCapsEnum version; /* Info about virGICVersion */ }; +typedef struct _virDomainCapsCPUModel virDomainCapsCPUModel; +typedef virDomainCapsCPUModel *virDomainCapsCPUModelPtr; +struct _virDomainCapsCPUModel { + char *name; +}; + +typedef struct _virDomainCapsCPUModels virDomainCapsCPUModels; +typedef virDomainCapsCPUModels *virDomainCapsCPUModelsPtr; +struct _virDomainCapsCPUModels { + virObject parent; + + size_t alloc; + size_t count; + virDomainCapsCPUModelPtr models; +}; + +typedef struct _virDomainCapsCPU virDomainCapsCPU; +typedef virDomainCapsCPU *virDomainCapsCPUPtr; +struct _virDomainCapsCPU { + bool hostPassthrough; + bool hostModel; + virDomainCapsCPUModelsPtr custom; +}; + struct _virDomainCaps { virObjectLockable parent; @@ -114,6 +138,7 @@ struct _virDomainCaps { int maxvcpus; virDomainCapsOS os; + virDomainCapsCPU cpu; virDomainCapsDeviceDisk disk; virDomainCapsDeviceGraphics graphics; virDomainCapsDeviceVideo video; @@ -129,6 +154,14 @@ virDomainCapsPtr virDomainCapsNew(const char *path, virArch arch, virDomainVirtType virttype); +virDomainCapsCPUModelsPtr virDomainCapsCPUModelsNew(size_t nmodels); +virDomainCapsCPUModelsPtr virDomainCapsCPUModelsCopy(virDomainCapsCPUModelsPtr old); +int virDomainCapsCPUModelsAddSteal(virDomainCapsCPUModelsPtr cpuModels, + char **name); +int virDomainCapsCPUModelsAdd(virDomainCapsCPUModelsPtr cpuModels, + const char *name, + ssize_t nameLen); + # define VIR_DOMAIN_CAPS_ENUM_SET(capsEnum, ...) \ do { \ unsigned int __values[] = {__VA_ARGS__}; \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 419c33d..f91ce17 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -152,6 +152,10 @@ virDomainAuditVcpu; # conf/domain_capabilities.h +virDomainCapsCPUModelsAdd; +virDomainCapsCPUModelsAddSteal; +virDomainCapsCPUModelsCopy; +virDomainCapsCPUModelsNew; virDomainCapsEnumClear; virDomainCapsEnumSet; virDomainCapsFormat; diff --git a/tests/domaincapsschemadata/basic.xml b/tests/domaincapsschemadata/basic.xml index 5513f99..6b788d9 100644 --- a/tests/domaincapsschemadata/basic.xml +++ b/tests/domaincapsschemadata/basic.xml @@ -4,6 +4,11 @@ <machine>my-machine-type</machine> <arch>x86_64</arch> <os supported='no'/> + <cpu> + <mode name='host-passthrough' supported='no'/> + <mode name='host-model' supported='no'/> + <mode name='custom' supported='no'/> + </cpu> <devices> <disk supported='no'/> <graphics supported='no'/> diff --git a/tests/domaincapsschemadata/full.xml b/tests/domaincapsschemadata/full.xml index 2f529ff..80fd1f0 100644 --- a/tests/domaincapsschemadata/full.xml +++ b/tests/domaincapsschemadata/full.xml @@ -19,6 +19,15 @@ </enum> </loader> </os> + <cpu> + <mode name='host-passthrough' supported='yes'/> + <mode name='host-model' supported='yes'/> + <mode name='custom' supported='yes'> + <model>Model1</model> + <model>Model2</model> + <model>Model3</model> + </mode> + </cpu> <devices> <disk supported='yes'> <enum name='diskDevice'> diff --git a/tests/domaincapsschemadata/libxl-xenfv-usb.xml b/tests/domaincapsschemadata/libxl-xenfv-usb.xml index c071d12..6a9e3d9 100644 --- a/tests/domaincapsschemadata/libxl-xenfv-usb.xml +++ b/tests/domaincapsschemadata/libxl-xenfv-usb.xml @@ -17,6 +17,11 @@ </enum> </loader> </os> + <cpu> + <mode name='host-passthrough' supported='no'/> + <mode name='host-model' supported='no'/> + <mode name='custom' supported='no'/> + </cpu> <devices> <disk supported='yes'> <enum name='diskDevice'> diff --git a/tests/domaincapsschemadata/libxl-xenfv.xml b/tests/domaincapsschemadata/libxl-xenfv.xml index 9436ef8..d48e699 100644 --- a/tests/domaincapsschemadata/libxl-xenfv.xml +++ b/tests/domaincapsschemadata/libxl-xenfv.xml @@ -17,6 +17,11 @@ </enum> </loader> </os> + <cpu> + <mode name='host-passthrough' supported='no'/> + <mode name='host-model' supported='no'/> + <mode name='custom' supported='no'/> + </cpu> <devices> <disk supported='yes'> <enum name='diskDevice'> diff --git a/tests/domaincapsschemadata/libxl-xenpv-usb.xml b/tests/domaincapsschemadata/libxl-xenpv-usb.xml index 4dd07bd..d1a3918 100644 --- a/tests/domaincapsschemadata/libxl-xenpv-usb.xml +++ b/tests/domaincapsschemadata/libxl-xenpv-usb.xml @@ -7,6 +7,11 @@ <os supported='yes'> <loader supported='no'/> </os> + <cpu> + <mode name='host-passthrough' supported='no'/> + <mode name='host-model' supported='no'/> + <mode name='custom' supported='no'/> + </cpu> <devices> <disk supported='yes'> <enum name='diskDevice'> diff --git a/tests/domaincapsschemadata/libxl-xenpv.xml b/tests/domaincapsschemadata/libxl-xenpv.xml index ab00a28..c0e3193 100644 --- a/tests/domaincapsschemadata/libxl-xenpv.xml +++ b/tests/domaincapsschemadata/libxl-xenpv.xml @@ -7,6 +7,11 @@ <os supported='yes'> <loader supported='no'/> </os> + <cpu> + <mode name='host-passthrough' supported='no'/> + <mode name='host-model' supported='no'/> + <mode name='custom' supported='no'/> + </cpu> <devices> <disk supported='yes'> <enum name='diskDevice'> diff --git a/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml b/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml index 161d0ab..6da28b0 100644 --- a/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml +++ b/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml @@ -18,6 +18,11 @@ </enum> </loader> </os> + <cpu> + <mode name='host-passthrough' supported='no'/> + <mode name='host-model' supported='no'/> + <mode name='custom' supported='no'/> + </cpu> <devices> <disk supported='yes'> <enum name='diskDevice'> diff --git a/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml index f5f0f1c..ee51684 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml @@ -18,6 +18,11 @@ </enum> </loader> </os> + <cpu> + <mode name='host-passthrough' supported='no'/> + <mode name='host-model' supported='no'/> + <mode name='custom' supported='no'/> + </cpu> <devices> <disk supported='yes'> <enum name='diskDevice'> diff --git a/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml index 1ae8172..88cced9 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml @@ -18,6 +18,11 @@ </enum> </loader> </os> + <cpu> + <mode name='host-passthrough' supported='no'/> + <mode name='host-model' supported='no'/> + <mode name='custom' supported='no'/> + </cpu> <devices> <disk supported='yes'> <enum name='diskDevice'> diff --git a/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml index 4e87cd2..09c0e1c 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml @@ -18,6 +18,11 @@ </enum> </loader> </os> + <cpu> + <mode name='host-passthrough' supported='no'/> + <mode name='host-model' supported='no'/> + <mode name='custom' supported='no'/> + </cpu> <devices> <disk supported='yes'> <enum name='diskDevice'> diff --git a/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml b/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml index 583fdf0..40b255e 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml @@ -18,6 +18,11 @@ </enum> </loader> </os> + <cpu> + <mode name='host-passthrough' supported='no'/> + <mode name='host-model' supported='no'/> + <mode name='custom' supported='no'/> + </cpu> <devices> <disk supported='yes'> <enum name='diskDevice'> diff --git a/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml b/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml index c453ce3..6706fec 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml @@ -18,6 +18,11 @@ </enum> </loader> </os> + <cpu> + <mode name='host-passthrough' supported='no'/> + <mode name='host-model' supported='no'/> + <mode name='custom' supported='no'/> + </cpu> <devices> <disk supported='yes'> <enum name='diskDevice'> diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index 5b7b7d0..907c0ad 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -60,6 +60,7 @@ fillAllCaps(virDomainCapsPtr domCaps) { virDomainCapsOSPtr os = &domCaps->os; virDomainCapsLoaderPtr loader = &os->loader; + virDomainCapsCPUPtr cpu = &domCaps->cpu; virDomainCapsDeviceDiskPtr disk = &domCaps->disk; virDomainCapsDeviceGraphicsPtr graphics = &domCaps->graphics; virDomainCapsDeviceVideoPtr video = &domCaps->video; @@ -77,6 +78,14 @@ fillAllCaps(virDomainCapsPtr domCaps) NULL) < 0) return -1; + cpu->hostPassthrough = true; + cpu->hostModel = true; + if (!(cpu->custom = virDomainCapsCPUModelsNew(3)) || + virDomainCapsCPUModelsAdd(cpu->custom, "Model1", -1) < 0 || + virDomainCapsCPUModelsAdd(cpu->custom, "Model2", -1) < 0 || + virDomainCapsCPUModelsAdd(cpu->custom, "Model3", -1) < 0) + return -1; + disk->supported = true; SET_ALL_BITS(disk->diskDevice); SET_ALL_BITS(disk->bus); -- 2.9.2

On Mon, Aug 29, 2016 at 12:31:44 -0400, John Ferlan wrote: Right, I changed it to use VIR_STEAL_PTR. ... Yeah, nmodels{,_max} is more common in our code base. Fixed. Jirka

On 08/12/2016 09:32 AM, Jiri Denemark wrote: [..] So if we don't have names, we don't get models and the following loop will only add to models if we've allocated it because we have names, right? Thus could there be an optimization to set/return ->count if !names prior to this check? e.g. if (!names) { *count = qemuCaps->cpuDefinitions->count; return 0; } This works, but it's tough to read. [...] I think "ret < 0" is superfluous... Coverity whines too [...]

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- tests/qemuxml2argvtest.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index a457c69..38f2025 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -267,7 +267,7 @@ typedef enum { static int testCompareXMLToArgvFiles(const char *xml, const char *cmdline, - virQEMUCapsPtr extraFlags, + virQEMUCapsPtr qemuCaps, const char *migrateURI, virQemuXML2ArgvTestFlags flags, unsigned int parseFlags) @@ -314,7 +314,7 @@ static int testCompareXMLToArgvFiles(const char *xml, if (qemuProcessPrepareMonitorChr(&monitor_chr, priv->libDir) < 0) goto out; - virQEMUCapsSetList(extraFlags, + virQEMUCapsSetList(qemuCaps, QEMU_CAPS_NO_ACPI, QEMU_CAPS_LAST); @@ -325,7 +325,7 @@ static int testCompareXMLToArgvFiles(const char *xml, goto out; } - virQEMUCapsFilterByMachineType(extraFlags, vm->def->os.machine); + virQEMUCapsFilterByMachineType(qemuCaps, vm->def->os.machine); log = virTestLogContentAndReset(); VIR_FREE(log); @@ -333,7 +333,7 @@ static int testCompareXMLToArgvFiles(const char *xml, if (vm->def->os.arch == VIR_ARCH_X86_64 || vm->def->os.arch == VIR_ARCH_I686) { - virQEMUCapsSet(extraFlags, QEMU_CAPS_PCI_MULTIBUS); + virQEMUCapsSet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS); } for (i = 0; i < vm->def->nhostdevs; i++) { @@ -390,7 +390,7 @@ static int testCompareXMLToArgvFiles(const char *xml, struct testInfo { const char *name; - virQEMUCapsPtr extraFlags; + virQEMUCapsPtr qemuCaps; const char *migrateFrom; int migrateFd; unsigned int flags; @@ -418,17 +418,17 @@ testCompareXMLToArgvHelper(const void *data) abs_srcdir, info->name) < 0) goto cleanup; - if (virQEMUCapsGet(info->extraFlags, QEMU_CAPS_MONITOR_JSON)) + if (virQEMUCapsGet(info->qemuCaps, QEMU_CAPS_MONITOR_JSON)) flags |= FLAG_JSON; - if (virQEMUCapsGet(info->extraFlags, QEMU_CAPS_ENABLE_FIPS)) + if (virQEMUCapsGet(info->qemuCaps, QEMU_CAPS_ENABLE_FIPS)) flags |= FLAG_FIPS; if (qemuTestCapsCacheInsert(driver.qemuCapsCache, info->name, - info->extraFlags) < 0) + info->qemuCaps) < 0) goto cleanup; - result = testCompareXMLToArgvFiles(xml, args, info->extraFlags, + result = testCompareXMLToArgvFiles(xml, args, info->qemuCaps, migrateURI, flags, info->parseFlags); cleanup: @@ -475,13 +475,13 @@ testPrepareExtraFlags(struct testInfo *info, { int ret = -1; - if (!(info->extraFlags = virQEMUCapsNew())) + if (!(info->qemuCaps = virQEMUCapsNew())) goto out; - if (testAddCPUModels(info->extraFlags, skipLegacyCPUs) < 0) + if (testAddCPUModels(info->qemuCaps, skipLegacyCPUs) < 0) goto out; - if (testQemuCapsSetGIC(info->extraFlags, gic) < 0) + if (testQemuCapsSetGIC(info->qemuCaps, gic) < 0) goto out; ret = 0; @@ -547,11 +547,11 @@ mymain(void) }; \ if (testPrepareExtraFlags(&info, skipLegacyCPUs, gic) < 0) \ return EXIT_FAILURE; \ - virQEMUCapsSetList(info.extraFlags, __VA_ARGS__, QEMU_CAPS_LAST);\ + virQEMUCapsSetList(info.qemuCaps, __VA_ARGS__, QEMU_CAPS_LAST); \ if (virTestRun("QEMU XML-2-ARGV " name, \ testCompareXMLToArgvHelper, &info) < 0) \ ret = -1; \ - virObjectUnref(info.extraFlags); \ + virObjectUnref(info.qemuCaps); \ } while (0) # define DO_TEST(name, ...) \ -- 2.9.2

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- tests/qemuxml2argvtest.c | 109 ++++++++++++++++++++--------------------------- 1 file changed, 47 insertions(+), 62 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 7ff9deb..4ec0990 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -265,14 +265,25 @@ typedef enum { FLAG_FIPS = 1 << 3, } virQemuXML2ArgvTestFlags; -static int testCompareXMLToArgvFiles(const char *xml, - const char *cmdline, - virQEMUCapsPtr qemuCaps, - const char *migrateURI, - virQemuXML2ArgvTestFlags flags, - unsigned int parseFlags) +struct testInfo { + const char *name; + virQEMUCapsPtr qemuCaps; + const char *migrateFrom; + int migrateFd; + unsigned int flags; + unsigned int parseFlags; +}; + +static int +testCompareXMLToArgv(const void *data) { + const struct testInfo *info = data; + char *xml = NULL; + char *args = NULL; + char *migrateURI = NULL; char *actualargv = NULL; + unsigned int flags = info->flags; + unsigned int parseFlags = info->parseFlags; int ret = -1; virDomainObjPtr vm = NULL; virDomainChrSourceDef monitor_chr; @@ -286,15 +297,37 @@ static int testCompareXMLToArgvFiles(const char *xml, if (!(conn = virGetConnect())) goto cleanup; + conn->secretDriver = &fakeSecretDriver; conn->storageDriver = &fakeStorageDriver; + if (virQEMUCapsGet(info->qemuCaps, QEMU_CAPS_MONITOR_JSON)) + flags |= FLAG_JSON; + + if (virQEMUCapsGet(info->qemuCaps, QEMU_CAPS_ENABLE_FIPS)) + flags |= FLAG_FIPS; + + if (qemuTestCapsCacheInsert(driver.qemuCapsCache, info->name, + info->qemuCaps) < 0) + goto cleanup; + + if (virAsprintf(&xml, "%s/qemuxml2argvdata/qemuxml2argv-%s.xml", + abs_srcdir, info->name) < 0 || + virAsprintf(&args, "%s/qemuxml2argvdata/qemuxml2argv-%s.args", + abs_srcdir, info->name) < 0) + goto cleanup; + + if (info->migrateFrom && + !(migrateURI = qemuMigrationIncomingURI(info->migrateFrom, + info->migrateFd))) + goto cleanup; + if (!(vm = virDomainObjNew(driver.xmlopt))) goto cleanup; + parseFlags |= VIR_DOMAIN_DEF_PARSE_INACTIVE; if (!(vm->def = virDomainDefParseFile(xml, driver.caps, driver.xmlopt, - (VIR_DOMAIN_DEF_PARSE_INACTIVE | - parseFlags)))) { + parseFlags))) { if (flags & FLAG_EXPECT_PARSE_ERROR) goto ok; goto cleanup; @@ -314,7 +347,7 @@ static int testCompareXMLToArgvFiles(const char *xml, if (qemuProcessPrepareMonitorChr(&monitor_chr, priv->libDir) < 0) goto cleanup; - virQEMUCapsSetList(qemuCaps, + virQEMUCapsSetList(info->qemuCaps, QEMU_CAPS_NO_ACPI, QEMU_CAPS_LAST); @@ -325,7 +358,7 @@ static int testCompareXMLToArgvFiles(const char *xml, goto cleanup; } - virQEMUCapsFilterByMachineType(qemuCaps, vm->def->os.machine); + virQEMUCapsFilterByMachineType(info->qemuCaps, vm->def->os.machine); log = virTestLogContentAndReset(); VIR_FREE(log); @@ -333,7 +366,7 @@ static int testCompareXMLToArgvFiles(const char *xml, if (vm->def->os.arch == VIR_ARCH_X86_64 || vm->def->os.arch == VIR_ARCH_I686) { - virQEMUCapsSet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS); + virQEMUCapsSet(info->qemuCaps, QEMU_CAPS_PCI_MULTIBUS); } for (i = 0; i < vm->def->nhostdevs; i++) { @@ -357,7 +390,7 @@ static int testCompareXMLToArgvFiles(const char *xml, if (!(actualargv = virCommandToString(cmd))) goto cleanup; - if (virTestCompareToFile(actualargv, cmdline) < 0) + if (virTestCompareToFile(actualargv, args) < 0) goto cleanup; ret = 0; @@ -384,58 +417,10 @@ static int testCompareXMLToArgvFiles(const char *xml, virCommandFree(cmd); virObjectUnref(vm); virObjectUnref(conn); - return ret; -} - - -struct testInfo { - const char *name; - virQEMUCapsPtr qemuCaps; - const char *migrateFrom; - int migrateFd; - unsigned int flags; - unsigned int parseFlags; -}; - -static int -testCompareXMLToArgvHelper(const void *data) -{ - int result = -1; - const struct testInfo *info = data; - char *xml = NULL; - char *args = NULL; - unsigned int flags = info->flags; - char *migrateURI = NULL; - - if (info->migrateFrom && - !(migrateURI = qemuMigrationIncomingURI(info->migrateFrom, - info->migrateFd))) - goto cleanup; - - if (virAsprintf(&xml, "%s/qemuxml2argvdata/qemuxml2argv-%s.xml", - abs_srcdir, info->name) < 0 || - virAsprintf(&args, "%s/qemuxml2argvdata/qemuxml2argv-%s.args", - abs_srcdir, info->name) < 0) - goto cleanup; - - if (virQEMUCapsGet(info->qemuCaps, QEMU_CAPS_MONITOR_JSON)) - flags |= FLAG_JSON; - - if (virQEMUCapsGet(info->qemuCaps, QEMU_CAPS_ENABLE_FIPS)) - flags |= FLAG_FIPS; - - if (qemuTestCapsCacheInsert(driver.qemuCapsCache, info->name, - info->qemuCaps) < 0) - goto cleanup; - - result = testCompareXMLToArgvFiles(xml, args, info->qemuCaps, - migrateURI, flags, info->parseFlags); - - cleanup: VIR_FREE(migrateURI); VIR_FREE(xml); VIR_FREE(args); - return result; + return ret; } @@ -549,7 +534,7 @@ mymain(void) return EXIT_FAILURE; \ virQEMUCapsSetList(info.qemuCaps, __VA_ARGS__, QEMU_CAPS_LAST); \ if (virTestRun("QEMU XML-2-ARGV " name, \ - testCompareXMLToArgvHelper, &info) < 0) \ + testCompareXMLToArgv, &info) < 0) \ ret = -1; \ virObjectUnref(info.qemuCaps); \ } while (0) -- 2.9.2

Some parts of qemuCaps depend on guest architecture, machine type, and possibly other things that we know only once the domain XML has been parsed. Let's move all these updates into a dedicated function. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- tests/qemuxml2argvtest.c | 48 +++++++++++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 82926db..9e9db6e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -272,6 +272,7 @@ struct testInfo { int migrateFd; unsigned int flags; unsigned int parseFlags; + bool skipLegacyCPUs; }; @@ -305,17 +306,15 @@ testAddCPUModels(virQEMUCapsPtr caps, bool skipLegacy) static int -testPrepareExtraFlags(struct testInfo *info, - bool skipLegacyCPUs, - int gic) +testInitQEMUCaps(struct testInfo *info, + int gic) { int ret = -1; if (!(info->qemuCaps = virQEMUCapsNew())) goto cleanup; - if (testAddCPUModels(info->qemuCaps, skipLegacyCPUs) < 0) - goto cleanup; + virQEMUCapsSet(info->qemuCaps, QEMU_CAPS_NO_ACPI); if (testQemuCapsSetGIC(info->qemuCaps, gic) < 0) goto cleanup; @@ -328,6 +327,27 @@ testPrepareExtraFlags(struct testInfo *info, static int +testUpdateQEMUCaps(const struct testInfo *info, + virDomainObjPtr vm) +{ + int ret = -1; + + if (testAddCPUModels(info->qemuCaps, info->skipLegacyCPUs) < 0) + goto cleanup; + + virQEMUCapsFilterByMachineType(info->qemuCaps, vm->def->os.machine); + + if (ARCH_IS_X86(vm->def->os.arch)) + virQEMUCapsSet(info->qemuCaps, QEMU_CAPS_PCI_MULTIBUS); + + ret = 0; + + cleanup: + return ret; +} + + +static int testCompareXMLToArgv(const void *data) { const struct testInfo *info = data; @@ -400,10 +420,6 @@ testCompareXMLToArgv(const void *data) if (qemuProcessPrepareMonitorChr(&monitor_chr, priv->libDir) < 0) goto cleanup; - virQEMUCapsSetList(info->qemuCaps, - QEMU_CAPS_NO_ACPI, - QEMU_CAPS_LAST); - if (STREQ(vm->def->os.machine, "pc") && STREQ(vm->def->emulator, "/usr/bin/qemu-system-x86_64")) { VIR_FREE(vm->def->os.machine); @@ -411,17 +427,13 @@ testCompareXMLToArgv(const void *data) goto cleanup; } - virQEMUCapsFilterByMachineType(info->qemuCaps, vm->def->os.machine); + if (testUpdateQEMUCaps(info, vm) < 0) + goto cleanup; log = virTestLogContentAndReset(); VIR_FREE(log); virResetLastError(); - if (vm->def->os.arch == VIR_ARCH_X86_64 || - vm->def->os.arch == VIR_ARCH_I686) { - virQEMUCapsSet(info->qemuCaps, QEMU_CAPS_PCI_MULTIBUS); - } - for (i = 0; i < vm->def->nhostdevs; i++) { virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i]; @@ -530,9 +542,11 @@ mymain(void) parseFlags, gic, ...) \ do { \ static struct testInfo info = { \ - name, NULL, migrateFrom, migrateFd, (flags), parseFlags \ + name, NULL, migrateFrom, migrateFd, (flags), parseFlags, \ + false \ }; \ - if (testPrepareExtraFlags(&info, skipLegacyCPUs, gic) < 0) \ + info.skipLegacyCPUs = skipLegacyCPUs; \ + if (testInitQEMUCaps(&info, gic) < 0) \ return EXIT_FAILURE; \ virQEMUCapsSetList(info.qemuCaps, __VA_ARGS__, QEMU_CAPS_LAST); \ if (virTestRun("QEMU XML-2-ARGV " name, \ -- 2.9.2

Changing a host architecture or a CPU is not as easy as assigning a new value to the appropriate element in virCaps since there is a relation between the CPU and host architecture (we don't really want to test anything on an AArch64 host with core2duo CPU). This patch introduces qemuTestSetHostArch and qemuTestSetHostCPU helpers which will make sure the host architecture matches the host CPU. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- tests/testutilsqemu.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++----- tests/testutilsqemu.h | 8 ++++++-- 2 files changed, 52 insertions(+), 7 deletions(-) diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 11dd20e..9b66101 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -16,6 +16,7 @@ virCPUDefPtr cpuDefault; virCPUDefPtr cpuHaswell; +virCPUDefPtr cpuPower8; static virCPUFeatureDef cpuDefaultFeatures[] = { { (char *) "lahf_lm", -1 }, @@ -92,6 +93,15 @@ static virCPUDef cpuHaswellData = { cpuHaswellFeatures, /* features */ }; +static virCPUDef cpuPower8Data = { + .type = VIR_CPU_TYPE_HOST, + .arch = VIR_ARCH_PPC64, + .model = (char *) "POWER8", + .sockets = 1, + .cores = 8, + .threads = 8, +}; + static virCapsGuestMachinePtr *testQemuAllocMachines(int *nmachines) { virCapsGuestMachinePtr *machines; @@ -331,7 +341,8 @@ virCapsPtr testQemuCapsInit(void) goto cleanup; if (!(cpuDefault = virCPUDefCopy(&cpuDefaultData)) || - !(cpuHaswell = virCPUDefCopy(&cpuHaswellData))) + !(cpuHaswell = virCPUDefCopy(&cpuHaswellData)) || + !(cpuPower8 = virCPUDefCopy(&cpuPower8Data))) goto cleanup; caps->host.cpu = cpuDefault; @@ -440,15 +451,45 @@ virCapsPtr testQemuCapsInit(void) cleanup: virCapabilitiesFreeMachines(machines, nmachines); - if (caps->host.cpu != cpuDefault) - virCPUDefFree(cpuDefault); - if (caps->host.cpu != cpuHaswell) - virCPUDefFree(cpuHaswell); + caps->host.cpu = NULL; + virCPUDefFree(cpuDefault); + virCPUDefFree(cpuHaswell); + virCPUDefFree(cpuPower8); virObjectUnref(caps); return NULL; } +void +qemuTestSetHostArch(virCapsPtr caps, + virArch arch) +{ + if (arch == VIR_ARCH_NONE) + arch = VIR_ARCH_X86_64; + caps->host.arch = arch; + qemuTestSetHostCPU(caps, NULL); +} + + +void +qemuTestSetHostCPU(virCapsPtr caps, + virCPUDefPtr cpu) +{ + virArch arch = caps->host.arch; + + if (!cpu) { + if (ARCH_IS_X86(arch)) + cpu = cpuDefault; + else if (ARCH_IS_PPC64(arch)) + cpu = cpuPower8; + } + + if (cpu) + caps->host.arch = cpu->arch; + caps->host.cpu = cpu; +} + + virQEMUCapsPtr qemuTestParseCapabilities(const char *capsFile) { diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h index f2b71e9..6d35116f 100644 --- a/tests/testutilsqemu.h +++ b/tests/testutilsqemu.h @@ -19,8 +19,12 @@ virQEMUCapsPtr qemuTestParseCapabilities(const char *capsFile); extern virCPUDefPtr cpuDefault; extern virCPUDefPtr cpuHaswell; -void testQemuCapsSetCPU(virCapsPtr caps, - virCPUDefPtr hostCPU); +extern virCPUDefPtr cpuPower8; + +void qemuTestSetHostArch(virCapsPtr caps, + virArch arch); +void qemuTestSetHostCPU(virCapsPtr caps, + virCPUDefPtr cpu); int qemuTestDriverInit(virQEMUDriver *driver); void qemuTestDriverFree(virQEMUDriver *driver); -- 2.9.2

On Mon, Aug 29, 2016 at 12:45:03 -0400, John Ferlan wrote: Yes, I separated it to a new patch. They are zero-initialized since they are static. It doesn't change much since caps is initialized a few lines earlier as if (!(caps = virCapabilitiesNew(VIR_ARCH_X86_64, false, false))) return NULL; But using qemuTestSetHostCPU() to initialize the CPU is cleaner. Fixed. ... You probably haven't noticed it, but this kind of sparse review with accumulated ACKs scattered in random places of random replies is quite hard to work with. Either an "ACK series" reply to a cover letter or separate and standalone replies to each patch with review comments and a conclusion (ack, nack, conditional ack) makes it a lot easier for the author to process the review. Jirka

On 08/12/2016 09:33 AM, Jiri Denemark wrote: [...] Adding this to the previous patch seems to work, but... Adding this one causes failure... Perhaps due to the: + if (cpu) + caps->host.arch = cpu->arch; in previous patch? or perhaps some shortly subsequent patch... It's not so much an objection to the patch, but by simply adding those API's in patch 10 I guess I would have expected them to just work, but trial and error proves otherwise. Of course reading future patches perhaps sheds even more light (or black magic, voodoo) ;-).

Adding x86 CPU models into a list of supported CPUs for non-x86 architectures is not a very good idea. Each architecture we test needs to maintain its own list of supported CPU models. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- .../qemuxml2argv-pseries-cpu-exact.args | 2 +- .../qemuxml2argv-pseries-cpu-exact.xml | 2 +- tests/qemuxml2argvtest.c | 37 +++++++++++++++------- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-exact.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-exact.args index 4d27f05..803c1aa 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-exact.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-exact.args @@ -8,7 +8,7 @@ QEMU_AUDIO_DRV=none \ -name QEMUGuest1 \ -S \ -M pseries \ --cpu POWER7_v2.3 \ +-cpu POWER7 \ -m 512 \ -smp 1,sockets=1,cores=1,threads=1 \ -uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-exact.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-exact.xml index b54dae2..830e781 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-exact.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-exact.xml @@ -7,7 +7,7 @@ <type arch='ppc64' machine='pseries'>hvm</type> </os> <cpu match='exact'> - <model>POWER7_v2.3</model> + <model>POWER7</model> <vendor>IBM</vendor> </cpu> <clock offset='utc'/> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 2b94ff0..ecdab3c 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -283,27 +283,42 @@ struct testInfo { static int testAddCPUModels(virQEMUCapsPtr caps, bool skipLegacy) { - const char *newModels[] = { + virArch arch = virQEMUCapsGetArch(caps); + const char *x86Models[] = { "Opteron_G3", "Opteron_G2", "Opteron_G1", "Nehalem", "Penryn", "Conroe", "Haswell-noTSX", "Haswell", }; - const char *legacyModels[] = { + const char *x86LegacyModels[] = { "n270", "athlon", "pentium3", "pentium2", "pentium", "486", "coreduo", "kvm32", "qemu32", "kvm64", "core2duo", "phenom", "qemu64", }; + const char *armModels[] = { + "cortex-a9", "cortex-a8", "cortex-a57", "cortex-a53", + }; + const char *ppc64Models[] = { + "POWER8", "POWER7", + }; - if (virQEMUCapsAddCPUDefinitions(caps, newModels, - ARRAY_CARDINALITY(newModels)) < 0) - return -1; + if (ARCH_IS_X86(arch)) { + if (virQEMUCapsAddCPUDefinitions(caps, x86Models, + ARRAY_CARDINALITY(x86Models)) < 0) + return -1; - if (skipLegacy) - return 0; - - if (virQEMUCapsAddCPUDefinitions(caps, legacyModels, - ARRAY_CARDINALITY(legacyModels)) < 0) - return -1; + if (!skipLegacy && + virQEMUCapsAddCPUDefinitions(caps, x86LegacyModels, + ARRAY_CARDINALITY(x86LegacyModels)) < 0) + return -1; + } else if (ARCH_IS_ARM(arch)) { + if (virQEMUCapsAddCPUDefinitions(caps, armModels, + ARRAY_CARDINALITY(armModels)) < 0) + return -1; + } else if (ARCH_IS_PPC64(arch)) { + if (virQEMUCapsAddCPUDefinitions(caps, ppc64Models, + ARRAY_CARDINALITY(ppc64Models)) < 0) + return -1; + } return 0; } -- 2.9.2

The x86 CPU driver translated each CPU definition from domain XML into CPUID data and then back to CPU definition. This effectively sorted the list of CPU features according to their CPUID values. Since this is going to change, we need to reorder CPU features in a few test files to make sure the generated QEMU command lines will not change. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- .../qemuxml2argvdata/qemuxml2argv-cpu-Haswell2.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml | 12 ++-- .../qemuxml2argv-cpu-exact2-nofallback.xml | 14 ++--- tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml | 14 ++--- .../qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml | 6 +- .../qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml | 20 +++---- .../qemuxml2argv-graphics-spice-timeout.xml | 24 ++++---- .../qemuxml2xmlout-graphics-spice-timeout.xml | 24 ++++---- tests/testutilsqemu.c | 70 +++++++++++----------- 9 files changed, 93 insertions(+), 93 deletions(-) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-Haswell2.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-Haswell2.xml index 0d4efa1..6a6722d 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-Haswell2.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-Haswell2.xml @@ -10,8 +10,8 @@ </os> <cpu mode='custom' match='exact'> <model fallback='forbid'>Haswell</model> - <feature policy='disable' name='rtm'/> <feature policy='disable' name='hle'/> + <feature policy='disable' name='rtm'/> </cpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml index 1d1e815..ebf5830 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact1.xml @@ -10,13 +10,13 @@ </os> <cpu match='exact'> <model fallback='allow'>qemu64</model> - <feature policy='disable' name='svm'/> - <feature policy='disable' name='lm'/> - <feature policy='disable' name='nx'/> - <feature policy='disable' name='syscall'/> - <feature policy='disable' name='clflush'/> - <feature policy='disable' name='pse36'/> <feature policy='disable' name='mca'/> + <feature policy='disable' name='pse36'/> + <feature policy='disable' name='clflush'/> + <feature policy='disable' name='syscall'/> + <feature policy='disable' name='nx'/> + <feature policy='disable' name='lm'/> + <feature policy='disable' name='svm'/> </cpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2-nofallback.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2-nofallback.xml index 6b9b7d4..f51f9ed 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2-nofallback.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2-nofallback.xml @@ -10,18 +10,18 @@ </os> <cpu match='exact'> <model fallback='forbid'>core2duo</model> - <feature name='lahf_lm' policy='require'/> + <feature name='ds' policy='require'/> + <feature name='ht' policy='require'/> + <feature name='tm' policy='optional'/> + <feature name='ds_cpl' policy='require'/> <feature name='xtpr' policy='require'/> + <feature name='3dnowext' policy='force'/> + <feature name='lahf_lm' policy='require'/> + <feature name='nx' policy='disable'/> <feature name='cx16' policy='disable'/> <feature name='tm2' policy='disable'/> - <feature name='ds_cpl' policy='require'/> <feature name='pbe' policy='disable'/> - <feature name='tm' policy='optional'/> - <feature name='ht' policy='require'/> <feature name='ss' policy='disable'/> - <feature name='ds' policy='require'/> - <feature name='nx' policy='disable'/> - <feature name='3dnowext' policy='force'/> <feature name='sse4a' policy='optional'/> <feature name='wdt' policy='forbid'/> </cpu> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml index eaea564..7fa77c5 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-exact2.xml @@ -10,18 +10,18 @@ </os> <cpu match='exact'> <model>core2duo</model> - <feature name='lahf_lm' policy='require'/> + <feature name='ds' policy='require'/> + <feature name='ht' policy='require'/> + <feature name='tm' policy='optional'/> + <feature name='ds_cpl' policy='require'/> <feature name='xtpr' policy='require'/> + <feature name='3dnowext' policy='force'/> + <feature name='lahf_lm' policy='require'/> + <feature name='nx' policy='disable'/> <feature name='cx16' policy='disable'/> <feature name='tm2' policy='disable'/> - <feature name='ds_cpl' policy='require'/> <feature name='pbe' policy='disable'/> - <feature name='tm' policy='optional'/> - <feature name='ht' policy='require'/> <feature name='ss' policy='disable'/> - <feature name='ds' policy='require'/> - <feature name='nx' policy='disable'/> - <feature name='3dnowext' policy='force'/> <feature name='sse4a' policy='optional'/> <feature name='wdt' policy='forbid'/> </cpu> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml index b8bbf25..fe6739c 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-minimum2.xml @@ -10,10 +10,10 @@ </os> <cpu match='minimum'> <model>qemu64</model> - <feature policy='disable' name='svm'/> - <feature policy='disable' name='lm'/> - <feature policy='disable' name='nx'/> <feature policy='disable' name='syscall'/> + <feature policy='disable' name='nx'/> + <feature policy='disable' name='lm'/> + <feature policy='disable' name='svm'/> </cpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml index a9fc9c5..84c77ea 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-strict1.xml @@ -10,21 +10,21 @@ </os> <cpu match='strict'> <model>core2duo</model> - <feature name='lahf_lm' policy='require'/> + <feature name='ds' policy='require'/> + <feature name='acpi' policy='optional'/> + <feature name='ht' policy='require'/> + <feature name='tm' policy='optional'/> + <feature name='ds_cpl' policy='require'/> + <feature name='vmx' policy='optional'/> + <feature name='est' policy='optional'/> <feature name='xtpr' policy='require'/> + <feature name='3dnowext' policy='force'/> + <feature name='lahf_lm' policy='require'/> + <feature name='nx' policy='disable'/> <feature name='cx16' policy='disable'/> <feature name='tm2' policy='disable'/> - <feature name='est' policy='optional'/> - <feature name='vmx' policy='optional'/> - <feature name='ds_cpl' policy='require'/> <feature name='pbe' policy='disable'/> - <feature name='tm' policy='optional'/> - <feature name='ht' policy='require'/> <feature name='ss' policy='disable'/> - <feature name='acpi' policy='optional'/> - <feature name='ds' policy='require'/> - <feature name='nx' policy='disable'/> - <feature name='3dnowext' policy='force'/> <feature name='sse4a' policy='optional'/> <feature name='wdt' policy='forbid'/> </cpu> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.xml index bfb189c..b00a8b7 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-timeout.xml @@ -19,19 +19,19 @@ <model>core2duo</model> <vendor>Intel</vendor> <topology sockets='1' cores='2' threads='1'/> - <feature policy='require' name='lahf_lm'/> - <feature policy='require' name='xtpr'/> - <feature policy='require' name='cx16'/> - <feature policy='require' name='tm2'/> - <feature policy='require' name='est'/> - <feature policy='require' name='vmx'/> - <feature policy='require' name='ds_cpl'/> - <feature policy='require' name='pbe'/> - <feature policy='require' name='tm'/> - <feature policy='require' name='ht'/> - <feature policy='require' name='ss'/> - <feature policy='require' name='acpi'/> <feature policy='require' name='ds'/> + <feature policy='require' name='acpi'/> + <feature policy='require' name='ss'/> + <feature policy='require' name='ht'/> + <feature policy='require' name='tm'/> + <feature policy='require' name='pbe'/> + <feature policy='require' name='ds_cpl'/> + <feature policy='require' name='vmx'/> + <feature policy='require' name='est'/> + <feature policy='require' name='tm2'/> + <feature policy='require' name='cx16'/> + <feature policy='require' name='xtpr'/> + <feature policy='require' name='lahf_lm'/> </cpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-timeout.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-timeout.xml index 912b542..5f881f1 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-timeout.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-graphics-spice-timeout.xml @@ -19,19 +19,19 @@ <model fallback='allow'>core2duo</model> <vendor>Intel</vendor> <topology sockets='1' cores='2' threads='1'/> - <feature policy='require' name='lahf_lm'/> - <feature policy='require' name='xtpr'/> - <feature policy='require' name='cx16'/> - <feature policy='require' name='tm2'/> - <feature policy='require' name='est'/> - <feature policy='require' name='vmx'/> - <feature policy='require' name='ds_cpl'/> - <feature policy='require' name='pbe'/> - <feature policy='require' name='tm'/> - <feature policy='require' name='ht'/> - <feature policy='require' name='ss'/> - <feature policy='require' name='acpi'/> <feature policy='require' name='ds'/> + <feature policy='require' name='acpi'/> + <feature policy='require' name='ss'/> + <feature policy='require' name='ht'/> + <feature policy='require' name='tm'/> + <feature policy='require' name='pbe'/> + <feature policy='require' name='ds_cpl'/> + <feature policy='require' name='vmx'/> + <feature policy='require' name='est'/> + <feature policy='require' name='tm2'/> + <feature policy='require' name='cx16'/> + <feature policy='require' name='xtpr'/> + <feature policy='require' name='lahf_lm'/> </cpu> <clock offset='utc'/> <on_poweroff>destroy</on_poweroff> diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 9b66101..1f0db16 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -19,19 +19,19 @@ virCPUDefPtr cpuHaswell; virCPUDefPtr cpuPower8; static virCPUFeatureDef cpuDefaultFeatures[] = { - { (char *) "lahf_lm", -1 }, - { (char *) "xtpr", -1 }, - { (char *) "cx16", -1 }, - { (char *) "tm2", -1 }, - { (char *) "est", -1 }, - { (char *) "vmx", -1 }, - { (char *) "ds_cpl", -1 }, - { (char *) "pbe", -1 }, - { (char *) "tm", -1 }, - { (char *) "ht", -1 }, - { (char *) "ss", -1 }, + { (char *) "ds", -1 }, { (char *) "acpi", -1 }, - { (char *) "ds", -1 } + { (char *) "ss", -1 }, + { (char *) "ht", -1 }, + { (char *) "tm", -1 }, + { (char *) "pbe", -1 }, + { (char *) "ds_cpl", -1 }, + { (char *) "vmx", -1 }, + { (char *) "est", -1 }, + { (char *) "tm2", -1 }, + { (char *) "cx16", -1 }, + { (char *) "xtpr", -1 }, + { (char *) "lahf_lm", -1 }, }; static virCPUDef cpuDefaultData = { VIR_CPU_TYPE_HOST, /* type */ @@ -51,30 +51,30 @@ static virCPUDef cpuDefaultData = { }; static virCPUFeatureDef cpuHaswellFeatures[] = { - { (char *) "lahf_lm", -1 }, - { (char *) "invtsc", -1 }, - { (char *) "abm", -1 }, - { (char *) "pdpe1gb", -1 }, - { (char *) "cmt", -1 }, - { (char *) "rdrand", -1 }, - { (char *) "f16c", -1 }, - { (char *) "osxsave", -1 }, - { (char *) "pdcm", -1 }, - { (char *) "xtpr", -1 }, - { (char *) "tm2", -1 }, - { (char *) "est", -1 }, - { (char *) "smx", -1 }, - { (char *) "vmx", -1 }, - { (char *) "ds_cpl", -1 }, - { (char *) "monitor", -1 }, - { (char *) "dtes64", -1 }, - { (char *) "pbe", -1 }, - { (char *) "tm", -1 }, - { (char *) "ht", -1 }, - { (char *) "ss", -1 }, - { (char *) "acpi", -1 }, - { (char *) "ds", -1 }, { (char *) "vme", -1 }, + { (char *) "ds", -1 }, + { (char *) "acpi", -1 }, + { (char *) "ss", -1 }, + { (char *) "ht", -1 }, + { (char *) "tm", -1 }, + { (char *) "pbe", -1 }, + { (char *) "dtes64", -1 }, + { (char *) "monitor", -1 }, + { (char *) "ds_cpl", -1 }, + { (char *) "vmx", -1 }, + { (char *) "smx", -1 }, + { (char *) "est", -1 }, + { (char *) "tm2", -1 }, + { (char *) "xtpr", -1 }, + { (char *) "pdcm", -1 }, + { (char *) "osxsave", -1 }, + { (char *) "f16c", -1 }, + { (char *) "rdrand", -1 }, + { (char *) "cmt", -1 }, + { (char *) "pdpe1gb", -1 }, + { (char *) "abm", -1 }, + { (char *) "invtsc", -1 }, + { (char *) "lahf_lm", -1 }, }; static virCPUDef cpuHaswellData = { VIR_CPU_TYPE_HOST, /* type */ -- 2.9.2

On 08/12/2016 09:33 AM, Jiri Denemark wrote: Noted by Coverity in this module - existing I think as well, it's just that the change piqued Coverity's interest in analyzing things... At the top of this function we have a: /* Ignore binary if extracting version info fails */ if (binary) { if (!(qemubinCaps = virQEMUCapsCacheLookup(cache, binary))) { virResetLastError(); ... Then there's the replace if condition w/ virQEMUCapsGuestIsNative followed by a: ret = virQEMUCapsInitGuestFromBinary(caps, binary, qemubinCaps, kvmbin, kvmbinCaps, guestarch); where it's noted that virQEMUCapsInitGuestFromBinary will dereference qemubinCaps in the call to virQEMUCapsGetMachineTypesCaps and it's possible that qemubinCaps is NULL if "binary" is set. The analysis doesn't go into the virQEMUCapsGuestIsNative condition. It's also notable that if !binary is checked in virQEMUCapsInitGuestFromBinary, so this is somewhat of an "edge" condition. Whether this is even possible - I'm not even sure!

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 24 +- src/qemu/qemu_capabilities.h | 3 +- src/qemu/qemu_driver.c | 7 +- tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml | 31 +- .../qemu_2.6.0-gicv2-virt.aarch64.xml | 37 +- .../qemu_2.6.0-gicv3-virt.aarch64.xml | 37 +- tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml | 37 +- tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml | 437 ++++++++++++++++++++- tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml | 35 +- tests/domaincapstest.c | 56 ++- 10 files changed, 682 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index aeea3a3..7a7ddb8 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4194,6 +4194,26 @@ virQEMUCapsFillDomainOSCaps(virDomainCapsOSPtr os, static int +virQEMUCapsFillDomainCPUCaps(virCapsPtr caps, + virQEMUCapsPtr qemuCaps, + virDomainCapsPtr domCaps) +{ + + if (domCaps->virttype == VIR_DOMAIN_VIRT_KVM && + virQEMUCapsGuestIsNative(caps->host.arch, qemuCaps->arch)) + domCaps->cpu.hostPassthrough = true; + + if (qemuCaps->cpuDefinitions && caps && caps->host.cpu) + domCaps->cpu.hostModel = virQEMUCapsGuestIsNative(caps->host.arch, + qemuCaps->arch); + + domCaps->cpu.custom = virObjectRef(qemuCaps->cpuDefinitions); + + return 0; +} + + +static int virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps, const char *machine, virDomainCapsDeviceDiskPtr disk) @@ -4400,7 +4420,8 @@ virQEMUCapsFillDomainFeatureGICCaps(virQEMUCapsPtr qemuCaps, int -virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, +virQEMUCapsFillDomainCaps(virCapsPtr caps, + virDomainCapsPtr domCaps, virQEMUCapsPtr qemuCaps, virFirmwarePtr *firmwares, size_t nfirmwares) @@ -4423,6 +4444,7 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, } if (virQEMUCapsFillDomainOSCaps(os, firmwares, nfirmwares) < 0 || + virQEMUCapsFillDomainCPUCaps(caps, qemuCaps, domCaps) < 0 || virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps, domCaps->machine, disk) < 0 || virQEMUCapsFillDomainDeviceGraphicsCaps(qemuCaps, graphics) < 0 || diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 0767a87..9fd38d9 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -494,7 +494,8 @@ int virQEMUCapsInitGuestFromBinary(virCapsPtr caps, virQEMUCapsPtr kvmbinCaps, virArch guestarch); -int virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps, +int virQEMUCapsFillDomainCaps(virCapsPtr caps, + virDomainCapsPtr domCaps, virQEMUCapsPtr qemuCaps, virFirmwarePtr *firmwares, size_t nfirmwares); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a424af0..7e4f4e7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18497,6 +18497,7 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn, virDomainCapsPtr domCaps = NULL; int arch = virArchFromHost(); /* virArch */ virQEMUDriverConfigPtr cfg = NULL; + virCapsPtr caps = NULL; virCheckFlags(0, ret); @@ -18505,6 +18506,9 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn, cfg = virQEMUDriverGetConfig(driver); + if (!(caps = virQEMUDriverGetCapabilities(driver, false))) + goto cleanup; + if (qemuHostdevHostSupportsPassthroughLegacy() || qemuHostdevHostSupportsPassthroughVFIO()) virttype = VIR_DOMAIN_VIRT_KVM; @@ -18576,13 +18580,14 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn, if (!(domCaps = virDomainCapsNew(emulatorbin, machine, arch, virttype))) goto cleanup; - if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps, + if (virQEMUCapsFillDomainCaps(caps, domCaps, qemuCaps, cfg->firmwares, cfg->nfirmwares) < 0) goto cleanup; ret = virDomainCapsFormat(domCaps); cleanup: virObjectUnref(cfg); + virObjectUnref(caps); virObjectUnref(domCaps); virObjectUnref(qemuCaps); return ret; diff --git a/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml b/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml index 6da28b0..4ee2f95 100644 --- a/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml +++ b/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml @@ -19,9 +19,34 @@ </loader> </os> <cpu> - <mode name='host-passthrough' supported='no'/> - <mode name='host-model' supported='no'/> - <mode name='custom' supported='no'/> + <mode name='host-passthrough' supported='yes'/> + <mode name='host-model' supported='yes'/> + <mode name='custom' supported='yes'> + <model>Opteron_G5</model> + <model>Opteron_G4</model> + <model>Opteron_G3</model> + <model>Opteron_G2</model> + <model>Opteron_G1</model> + <model>Haswell</model> + <model>SandyBridge</model> + <model>Westmere</model> + <model>Nehalem</model> + <model>Penryn</model> + <model>Conroe</model> + <model>n270</model> + <model>athlon</model> + <model>pentium3</model> + <model>pentium2</model> + <model>pentium</model> + <model>486</model> + <model>coreduo</model> + <model>kvm32</model> + <model>qemu32</model> + <model>kvm64</model> + <model>core2duo</model> + <model>phenom</model> + <model>qemu64</model> + </mode> </cpu> <devices> <disk supported='yes'> diff --git a/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml index ee51684..9e96f47 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml @@ -19,9 +19,40 @@ </loader> </os> <cpu> - <mode name='host-passthrough' supported='no'/> - <mode name='host-model' supported='no'/> - <mode name='custom' supported='no'/> + <mode name='host-passthrough' supported='yes'/> + <mode name='host-model' supported='yes'/> + <mode name='custom' supported='yes'> + <model>pxa262</model> + <model>pxa270-a0</model> + <model>arm1136</model> + <model>cortex-a15</model> + <model>pxa260</model> + <model>arm1136-r2</model> + <model>pxa261</model> + <model>pxa255</model> + <model>arm926</model> + <model>arm11mpcore</model> + <model>pxa250</model> + <model>ti925t</model> + <model>cortex-a57</model> + <model>sa1110</model> + <model>arm1176</model> + <model>cortex-a53</model> + <model>sa1100</model> + <model>pxa270-c5</model> + <model>cortex-a9</model> + <model>cortex-a8</model> + <model>pxa270-c0</model> + <model>arm1026</model> + <model>pxa270-b1</model> + <model>cortex-m3</model> + <model>cortex-m4</model> + <model>pxa270-b0</model> + <model>arm946</model> + <model>cortex-r5</model> + <model>pxa270-a1</model> + <model>pxa270</model> + </mode> </cpu> <devices> <disk supported='yes'> diff --git a/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml index 88cced9..c081b35 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml @@ -19,9 +19,40 @@ </loader> </os> <cpu> - <mode name='host-passthrough' supported='no'/> - <mode name='host-model' supported='no'/> - <mode name='custom' supported='no'/> + <mode name='host-passthrough' supported='yes'/> + <mode name='host-model' supported='yes'/> + <mode name='custom' supported='yes'> + <model>pxa262</model> + <model>pxa270-a0</model> + <model>arm1136</model> + <model>cortex-a15</model> + <model>pxa260</model> + <model>arm1136-r2</model> + <model>pxa261</model> + <model>pxa255</model> + <model>arm926</model> + <model>arm11mpcore</model> + <model>pxa250</model> + <model>ti925t</model> + <model>cortex-a57</model> + <model>sa1110</model> + <model>arm1176</model> + <model>cortex-a53</model> + <model>sa1100</model> + <model>pxa270-c5</model> + <model>cortex-a9</model> + <model>cortex-a8</model> + <model>pxa270-c0</model> + <model>arm1026</model> + <model>pxa270-b1</model> + <model>cortex-m3</model> + <model>cortex-m4</model> + <model>pxa270-b0</model> + <model>arm946</model> + <model>cortex-r5</model> + <model>pxa270-a1</model> + <model>pxa270</model> + </mode> </cpu> <devices> <disk supported='yes'> diff --git a/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml index 09c0e1c..811d2b7 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml @@ -19,9 +19,40 @@ </loader> </os> <cpu> - <mode name='host-passthrough' supported='no'/> - <mode name='host-model' supported='no'/> - <mode name='custom' supported='no'/> + <mode name='host-passthrough' supported='yes'/> + <mode name='host-model' supported='yes'/> + <mode name='custom' supported='yes'> + <model>pxa262</model> + <model>pxa270-a0</model> + <model>arm1136</model> + <model>cortex-a15</model> + <model>pxa260</model> + <model>arm1136-r2</model> + <model>pxa261</model> + <model>pxa255</model> + <model>arm926</model> + <model>arm11mpcore</model> + <model>pxa250</model> + <model>ti925t</model> + <model>cortex-a57</model> + <model>sa1110</model> + <model>arm1176</model> + <model>cortex-a53</model> + <model>sa1100</model> + <model>pxa270-c5</model> + <model>cortex-a9</model> + <model>cortex-a8</model> + <model>pxa270-c0</model> + <model>arm1026</model> + <model>pxa270-b1</model> + <model>cortex-m3</model> + <model>cortex-m4</model> + <model>pxa270-b0</model> + <model>arm946</model> + <model>cortex-r5</model> + <model>pxa270-a1</model> + <model>pxa270</model> + </mode> </cpu> <devices> <disk supported='yes'> diff --git a/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml b/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml index 40b255e..a0aa811 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml @@ -19,9 +19,440 @@ </loader> </os> <cpu> - <mode name='host-passthrough' supported='no'/> - <mode name='host-model' supported='no'/> - <mode name='custom' supported='no'/> + <mode name='host-passthrough' supported='yes'/> + <mode name='host-model' supported='yes'/> + <mode name='custom' supported='yes'> + <model>default</model> + <model>ppc</model> + <model>ppc32</model> + <model>ppc64</model> + <model>970mp</model> + <model>970fx</model> + <model>970</model> + <model>POWER8NVL</model> + <model>POWER8</model> + <model>POWER8E</model> + <model>POWER7+</model> + <model>POWER7</model> + <model>POWER5gs</model> + <model>POWER5+</model> + <model>Apollo7PM</model> + <model>7457A</model> + <model>7447A</model> + <model>Apollo7</model> + <model>7457</model> + <model>Apollo6</model> + <model>7455</model> + <model>7445</model> + <model>7451</model> + <model>7441</model> + <model>Vger</model> + <model>7450</model> + <model>7448</model> + <model>Nitro</model> + <model>7410</model> + <model>G4</model> + <model>Max</model> + <model>7400</model> + <model>Goldfinger</model> + <model>755</model> + <model>745</model> + <model>LoneStar</model> + <model>750l</model> + <model>750gx</model> + <model>750fx</model> + <model>750cxe</model> + <model>750cx</model> + <model>750cl</model> + <model>Conan/Doyle</model> + <model>G3</model> + <model>Typhoon</model> + <model>750</model> + <model>Arthur</model> + <model>740</model> + <model>Mach5</model> + <model>Sirocco</model> + <model>604e</model> + <model>Goldeneye</model> + <model>603r</model> + <model>Vaillant</model> + <model>Stretch</model> + <model>603e</model> + <model>Vanilla</model> + <model>601v</model> + <model>601</model> + <model>MPC8560</model> + <model>MPC8555E</model> + <model>MPC8555</model> + <model>MPC8548E</model> + <model>MPC8548</model> + <model>MPC8547E</model> + <model>MPC8545E</model> + <model>MPC8545</model> + <model>MPC8544E</model> + <model>MPC8544</model> + <model>MPC8543E</model> + <model>MPC8543</model> + <model>MPC8541E</model> + <model>MPC8541</model> + <model>MPC8540</model> + <model>MPC8533E</model> + <model>MPC8533</model> + <model>e500v2</model> + <model>e500v1</model> + <model>e500</model> + <model>MPC8347EA</model> + <model>MPC8347E</model> + <model>MPC8347A</model> + <model>MPC8347</model> + <model>e300</model> + <model>e200</model> + <model>MPC8280</model> + <model>MPC8275</model> + <model>MPC8272</model> + <model>MPC8271</model> + <model>MPC8270</model> + <model>MPC8266_HiP4</model> + <model>MPC8266_HiP3</model> + <model>MPC8266</model> + <model>MPC8265_HiP4</model> + <model>MPC8265_HiP3</model> + <model>MPC8265</model> + <model>MPC8264_HiP4</model> + <model>MPC8264_HiP3</model> + <model>MPC8264</model> + <model>MPC8260_HiP4</model> + <model>MPC8260_HiP3</model> + <model>MPC8260</model> + <model>MPC8255_HiP4</model> + <model>MPC8255_HiP3</model> + <model>MPC8255</model> + <model>MPC8250_HiP4</model> + <model>MPC8250_HiP3</model> + <model>MPC8250</model> + <model>MPC8248</model> + <model>MPC8247</model> + <model>MPC8245</model> + <model>MPC8241</model> + <model>PowerQUICC-II</model> + <model>MPC82xx</model> + <model>MPC5200B</model> + <model>MPC5200</model> + <model>MPC52xx</model> + <model>MPC8240</model> + <model>440EP</model> + <model>x2vp50</model> + <model>x2vp7</model> + <model>405GPe</model> + <model>405GP</model> + <model>405CR</model> + <model>405</model> + <model>403</model> + <model>750cl_v1.0</model> + <model>750cxe_v2.1</model> + <model>7457A_v1.2</model> + <model>755_v2.7</model> + <model>MPC8347ET</model> + <model>MPC8545E_v20</model> + <model>750_v2.1</model> + <model>755_v2.2</model> + <model>745_v2.0</model> + <model>7455_v1.0</model> + <model>MPC8547E_v21</model> + <model>STB25</model> + <model>POWER8_v2.0</model> + <model>401G2</model> + <model>MPC8349A</model> + <model>750e</model> + <model>MPC8545_v21</model> + <model>MPC8548_v11</model> + <model>970fx_v2.0</model> + <model>750fx_v2.0</model> + <model>POWER8E_v2.1</model> + <model>Cobra</model> + <model>603p</model> + <model>7400_v2.7</model> + <model>MPC8378E</model> + <model>e500v2_v10</model> + <model>e500mc</model> + <model>603e7v1</model> + <model>970mp_v1.0</model> + <model>MPC8555E_v11</model> + <model>440-Xilinx-w-dfpu</model> + <model>740e</model> + <model>405CRc</model> + <model>7447A_v1.1</model> + <model>MPC8543_v11</model> + <model>MPC8533E_v10</model> + <model>MPC8540_v21</model> + <model>e500v2_v21</model> + <model>e300c1</model> + <model>7400_v2.2</model> + <model>970fx_v1.0</model> + <model>750gx_v1.2</model> + <model>750fx_v1.0</model> + <model>MPC8641</model> + <model>MPC8544E_v10</model> + <model>405D4</model> + <model>7457_v1.1</model> + <model>970fx_v3.0</model> + <model>604e_v2.2</model> + <model>755_v2.6</model> + <model>7410_v1.4</model> + <model>Npe4GS3</model> + <model>745_v2.4</model> + <model>750_v2.0</model> + <model>7445_v3.2</model> + <model>750l_v3.2</model> + <model>MPC8540_v10</model> + <model>750_v1.0</model> + <model>MPC8343A</model> + <model>MPC8378</model> + <model>G2HiP3</model> + <model>750cxe_v3.0</model> + <model>e500_v10</model> + <model>STB03</model> + <model>MPC8567</model> + <model>MPC8545_v20</model> + <model>MPC8548_v10</model> + <model>755_v1.1</model> + <model>603e7v</model> + <model>Npe405H2</model> + <model>750_v3.0</model> + <model>Npe405H</model> + <model>7400_v2.6</model> + <model>405GPa</model> + <model>MPC8548_v21</model> + <model>MPC8541E_v10</model> + <model>750l_v2.2</model> + <model>MPC8555E_v10</model> + <model>7457A_v1.1</model> + <model>e200z6</model> + <model>MPC8379E</model> + <model>e500v2_v20</model> + <model>MPC8347AP</model> + <model>401B2</model> + <model>MPC8349E</model> + <model>755_v2.1</model> + <model>MPC8543_v21</model> + <model>MPC8547E_v20</model> + <model>745_v2.8</model> + <model>7455_v3.4</model> + <model>7448_v1.1</model> + <model>740_v2.2</model> + <model>7400_v1.1</model> + <model>MPC8567E</model> + <model>7441_v2.10</model> + <model>603e_v1.4</model> + <model>G2leGP1</model> + <model>7451_v2.10</model> + <model>MPC8343E</model> + <model>750cx_v2.2</model> + <model>Npe405L</model> + <model>603e7</model> + <model>401A1</model> + <model>MPC8377</model> + <model>7448_v2.1</model> + <model>7441_v2.3</model> + <model>7400_v2.1</model> + <model>405CRb</model> + <model>604</model> + <model>MPC8540_v20</model> + <model>MPC8543_v10</model> + <model>7447A_v1.0</model> + <model>7445_v2.1</model> + <model>e500_v20</model> + <model>750gx_v1.1</model> + <model>MPC8543E_v11</model> + <model>750cxe_v2.4</model> + <model>MPC8548_v20</model> + <model>MPC8347AT</model> + <model>POWER8NVL_v1.0</model> + <model>7457A_v1.0</model> + <model>7457_v1.0</model> + <model>7450_v1.2</model> + <model>MPC8572</model> + <model>755_v2.5</model> + <model>7410_v1.3</model> + <model>745_v2.3</model> + <model>750l_v2.1</model> + <model>405GPR</model> + <model>755_v2.0</model> + <model>MPC8541_v11</model> + <model>401C2</model> + <model>e500v2_v30</model> + <model>7455_v3.3</model> + <model>405EZ</model> + <model>MPC8568E</model> + <model>755_v1.0</model> + <model>603e_v1.3</model> + <model>MPC8560_v10</model> + <model>MPC8560_v21</model> + <model>G2H4</model> + <model>MPC8533_v11</model> + <model>740_v3.1</model> + <model>e200z5</model> + <model>MPC8349</model> + <model>7400_v2.0</model> + <model>405CRa</model> + <model>MPC8543E_v10</model> + <model>MPC8543E_v21</model> + <model>750cxe_v2.3</model> + <model>MPC8543_v20</model> + <model>745_v2.7</model> + <model>403GC</model> + <model>e5500</model> + <model>7448_v1.0</model> + <model>MPC5200_v12</model> + <model>740_v2.1</model> + <model>745_v2.2</model> + <model>7400_v1.0</model> + <model>7410_v1.2</model> + <model>x2vp4</model> + <model>MPC8555_v11</model> + <model>MPC8541_v10</model> + <model>405LP</model> + <model>750fx_v2.3</model> + <model>405EP</model> + <model>601_v2</model> + <model>MPC8544_v11</model> + <model>401D2</model> + <model>MPC8572E</model> + <model>604e_v1.0</model> + <model>750cx_v2.1</model> + <model>MPC5200B_v21</model> + <model>750l_v3.0</model> + <model>e300c4</model> + <model>7448_v2.0</model> + <model>7450_v2.1</model> + <model>G2leGP</model> + <model>750cxe_v3.1b</model> + <model>7400_v2.9</model> + <model>G2GP</model> + <model>603</model> + <model>405GPd</model> + <model>MPC8548E_v11</model> + <model>603e7t</model> + <model>IOP480</model> + <model>750gx_v1.0</model> + <model>MPC8560_v20</model> + <model>405D2</model> + <model>750fl</model> + <model>750l_v2.0</model> + <model>x2vp20</model> + <model>e600</model> + <model>7450_v1.1</model> + <model>755_v2.4</model> + <model>7445_v1.0</model> + <model>G2LS</model> + <model>MPC8543E_v20</model> + <model>7447_v1.1</model> + <model>603e_v2.2</model> + <model>603e_v4</model> + <model>7455_v3.2</model> + <model>740_v2.0</model> + <model>MPC8347P</model> + <model>440EPb</model> + <model>MPC603</model> + <model>603e_v1.2</model> + <model>750fx_v2.2</model> + <model>740_v1.0</model> + <model>MPC8544_v10</model> + <model>601_v1</model> + <model>745_v1.1</model> + <model>POWER7+_v2.1</model> + <model>750cx_v2.0</model> + <model>MPC8347EAP</model> + <model>401E2</model> + <model>MPC8641D</model> + <model>e300c3</model> + <model>MPC8533_v10</model> + <model>7441_v2.1</model> + <model>740_v3.0</model> + <model>POWER5+_v2.1</model> + <model>602</model> + <model>MPC8548E_v10</model> + <model>G2leLS</model> + <model>MPC8343</model> + <model>750cx_v1.0</model> + <model>750cxe_v2.2</model> + <model>604e_v2.4</model> + <model>755_v2.8</model> + <model>MPC8548E_v21</model> + <model>745_v2.6</model> + <model>G2le</model> + <model>403GB</model> + <model>MPC8545E_v21</model> + <model>POWER7_v2.3</model> + <model>750_v2.2</model> + <model>7450_v1.0</model> + <model>MPC5200_v11</model> + <model>755_v2.3</model> + <model>MPC8347EP</model> + <model>7410_v1.1</model> + <model>750gl</model> + <model>403GCX</model> + <model>750p</model> + <model>745_v2.1</model> + <model>7445_v3.4</model> + <model>G2</model> + <model>MPC8555_v10</model> + <model>MPC8347T</model> + <model>603e_v3</model> + <model>970_v2.2</model> + <model>440-Xilinx</model> + <model>740p</model> + <model>MPC5200B_v20</model> + <model>440EPX</model> + <model>603e_v1.1</model> + <model>7451_v2.3</model> + <model>7450_v2.0</model> + <model>7400_v2.8</model> + <model>7455_v2.1</model> + <model>405GPc</model> + <model>MPC8347EAT</model> + <model>970mp_v1.1</model> + <model>603e7v2</model> + <model>604r</model> + <model>7447A_v1.2</model> + <model>MPC8533E_v11</model> + <model>e500v2_v22</model> + <model>401F2</model> + <model>MPC8343EA</model> + <model>7457_v1.2</model> + <model>970fx_v3.1</model> + <model>7447_v1.0</model> + <model>745_v2.5</model> + <model>MPC8548E_v20</model> + <model>MPC8610</model> + <model>403GA</model> + <model>750cxe_v2.4b</model> + <model>MPC5200_v10</model> + <model>7410_v1.0</model> + <model>G2leGP3</model> + <model>603e_v4.1</model> + <model>7445_v3.3</model> + <model>440EPa</model> + <model>970fx_v2.1</model> + <model>MPC8377E</model> + <model>750fx_v2.1</model> + <model>601_v0</model> + <model>STB04</model> + <model>745_v1.0</model> + <model>G2HiP4</model> + <model>750cl_v2.0</model> + <model>750cxr</model> + <model>MPC8349EA</model> + <model>750cxe_v3.1</model> + <model>401</model> + <model>MPC8379</model> + <model>MPC8568</model> + <model>e300c2</model> + <model>750_v3.1</model> + <model>MPC8544E_v11</model> + <model>405GPb</model> + <model>MPC8541E_v11</model> + </mode> </cpu> <devices> <disk supported='yes'> diff --git a/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml b/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml index 6706fec..80101a4 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml @@ -19,9 +19,38 @@ </loader> </os> <cpu> - <mode name='host-passthrough' supported='no'/> - <mode name='host-model' supported='no'/> - <mode name='custom' supported='no'/> + <mode name='host-passthrough' supported='yes'/> + <mode name='host-model' supported='yes'/> + <mode name='custom' supported='yes'> + <model>Opteron_G5</model> + <model>Opteron_G4</model> + <model>Opteron_G3</model> + <model>Opteron_G2</model> + <model>Opteron_G1</model> + <model>Broadwell</model> + <model>Broadwell-noTSX</model> + <model>Haswell</model> + <model>Haswell-noTSX</model> + <model>IvyBridge</model> + <model>SandyBridge</model> + <model>Westmere</model> + <model>Nehalem</model> + <model>Penryn</model> + <model>Conroe</model> + <model>n270</model> + <model>athlon</model> + <model>pentium3</model> + <model>pentium2</model> + <model>pentium</model> + <model>486</model> + <model>coreduo</model> + <model>kvm32</model> + <model>qemu32</model> + <model>kvm64</model> + <model>core2duo</model> + <model>phenom</model> + <model>qemu64</model> + </mode> </cpu> <devices> <disk supported='yes'> diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index 907c0ad..10b7452 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -109,6 +109,54 @@ fillAllCaps(virDomainCapsPtr domCaps) #if WITH_QEMU # include "testutilsqemu.h" +static virCPUDef aarch64Cpu = { + 0, 0, 0, 0, NULL, NULL, 0, NULL, 1, 1, 1, 0, 0, NULL, +}; + +static virCPUDef ppc64leCpu = { + VIR_CPU_TYPE_HOST, 0, 0, + VIR_ARCH_PPC64LE, (char *) "POWER8", + NULL, 0, NULL, 1, 1, 1, 0, 0, NULL, +}; + +static virCPUDef x86Cpu = { + VIR_CPU_TYPE_HOST, 0, 0, + VIR_ARCH_X86_64, (char *) "Broadwell", + NULL, 0, NULL, 1, 1, 1, 0, 0, NULL, +}; + +static int +fakeHostCPU(virCapsPtr caps, + virArch arch) +{ + virCPUDefPtr cpu; + + switch (arch) { + case VIR_ARCH_AARCH64: + cpu = &aarch64Cpu; + break; + + case VIR_ARCH_PPC64LE: + cpu = &ppc64leCpu; + break; + + case VIR_ARCH_X86_64: + cpu = &x86Cpu; + break; + + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + "cannot fake host CPU for arch %s", + virArchToString(arch)); + return -1; + } + + if (!(caps->host.cpu = virCPUDefCopy(cpu))) + return -1; + + return 0; +} + static int fillQemuCaps(virDomainCapsPtr domCaps, const char *name, @@ -118,6 +166,7 @@ fillQemuCaps(virDomainCapsPtr domCaps, { int ret = -1; char *path = NULL; + virCapsPtr caps = NULL; virQEMUCapsPtr qemuCaps = NULL; virDomainCapsLoaderPtr loader = &domCaps->os.loader; @@ -136,7 +185,11 @@ fillQemuCaps(virDomainCapsPtr domCaps, virQEMUCapsGetDefaultMachine(qemuCaps)) < 0) goto cleanup; - if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps, + if (!(caps = virCapabilitiesNew(domCaps->arch, false, false)) || + fakeHostCPU(caps, domCaps->arch) < 0) + goto cleanup; + + if (virQEMUCapsFillDomainCaps(caps, domCaps, qemuCaps, cfg->firmwares, cfg->nfirmwares) < 0) goto cleanup; @@ -164,6 +217,7 @@ fillQemuCaps(virDomainCapsPtr domCaps, ret = 0; cleanup: + virObjectUnref(caps); virObjectUnref(qemuCaps); VIR_FREE(path); return ret; -- 2.9.2

On Mon, Aug 29, 2016 at 17:59:02 -0400, John Ferlan wrote: No, caps cannot be NULL and the code will go away completely in patch 28 in this series. I fixed it anyway. Jirka

On 08/12/2016 09:33 AM, Jiri Denemark wrote: [...] I seem to recall some recent work around the -q[uiet] switch - is that something that this code needs to be concerned with? (as silly as that seems while I'm typing it!)

cpuGetSubDriver already reports a useful error. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/cpu/cpu.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index d2b7ce3..fae3885 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -765,13 +765,8 @@ cpuGetModels(virArch arch, char ***models) VIR_DEBUG("arch=%s", virArchToString(arch)); - driver = cpuGetSubDriver(arch); - if (driver == NULL) { - virReportError(VIR_ERR_INVALID_ARG, - _("cannot find a driver for the architecture %s"), - virArchToString(arch)); + if (!(driver = cpuGetSubDriver(arch))) return -1; - } if (!driver->getModels) { if (models) -- 2.9.2

On 08/12/2016 09:33 AM, Jiri Denemark wrote: My Coverity environment had a build failure here: conf/domain_capabilities.c: In function 'virDomainCapsCPUModelsFilter': conf/domain_capabilities.c:183:48: error: passing argument 1 of 'virStringArrayHasString' from incompatible pointer type [-Werror=incompatible-pointer-types] if (models && !virStringArrayHasString(models, old->models[i].name)) ^~~~~~ In file included from conf/domain_capabilities.c:28:0: ./util/virstring.h:47:6: note: expected 'const char **' but argument is of type 'char **' bool virStringArrayHasString(const char **strings, const char *needle); ^~~~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors Makefile:8092: recipe for target 'conf/libvirt_conf_la-domain_capabilities.lo' failed Which a handily place (const char **) resolved. I seem to recall a discussion on something similar recently upstream... if cpuGetModels returns 0, then models is empty so is there any point in making the call? [...]

In case a hypervisor is able to tell us a list of supported CPU models and whether each CPU models can be used on the current host, we can propagate this to domain capabilities. This is a better alternative to calling virConnectCompareCPU for each supported CPU model. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- docs/formatdomaincaps.html.in | 10 ++-- docs/schemas/domaincaps.rng | 7 +++ src/conf/domain_capabilities.c | 50 +++++++++++++----- src/conf/domain_capabilities.h | 16 +++++- src/libvirt_private.syms | 2 + src/qemu/qemu_capabilities.c | 15 ++++-- tests/domaincapsschemadata/full.xml | 6 +-- tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml | 48 ++++++++--------- .../qemu_2.6.0-gicv2-virt.aarch64.xml | 60 +++++++++++----------- .../qemu_2.6.0-gicv3-virt.aarch64.xml | 60 +++++++++++----------- tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml | 60 +++++++++++----------- tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml | 4 +- tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml | 56 ++++++++++---------- tests/domaincapstest.c | 9 ++-- 14 files changed, 229 insertions(+), 174 deletions(-) diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in index 15a13c1..49ccbfc 100644 --- a/docs/formatdomaincaps.html.in +++ b/docs/formatdomaincaps.html.in @@ -156,9 +156,9 @@ &lt;mode name='host-passthrough' supported='yes'/&gt; &lt;mode name='host-model' supported='yes'/&gt; &lt;mode name='custom' supported='yes'&gt; - &lt;model&gt;Broadwell&lt;/model&gt; - &lt;model&gt;Broadwell-noTSX&lt;/model&gt; - &lt;model&gt;Haswell&lt;/model&gt; + &lt;model usable='no'&gt;Broadwell&lt;/model&gt; + &lt;model usable='yes'&gt;Broadwell-noTSX&lt;/model&gt; + &lt;model usable='no'&gt;Haswell&lt;/model&gt; ... &lt;/mode&gt; &lt;/cpu&gt; @@ -183,6 +183,10 @@ <dd> The <code>mode</code> element contains a list of supported CPU models, each described by a dedicated <code>model</code> element. + The <code>usable</code> attribute specifies whether the model can + be used on the host. A special value <code>unknown</code> says + libvirt does not have enough information to provide the usability + data. </dd> </dl> diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng index 9f3d225..5a605a7 100644 --- a/docs/schemas/domaincaps.rng +++ b/docs/schemas/domaincaps.rng @@ -105,6 +105,13 @@ <ref name='supported'/> <zeroOrMore> <element name='model'> + <attribute name='usable'> + <choice> + <value>yes</value> + <value>no</value> + <value>unknown</value> + </choice> + </attribute> <text/> </element> </zeroOrMore> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index 6f9f7e7..c9e3a28 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -29,6 +29,9 @@ #define VIR_FROM_THIS VIR_FROM_CAPABILITIES +VIR_ENUM_IMPL(virDomainCapsCPUUsable, VIR_DOMCAPS_CPU_USABLE_LAST, + "unknown", "yes", "no"); + static virClassPtr virDomainCapsClass; static virClassPtr virDomainCapsCPUModelsClass; @@ -157,7 +160,9 @@ virDomainCapsCPUModelsCopy(virDomainCapsCPUModelsPtr old) return NULL; for (i = 0; i < old->count; i++) { - if (virDomainCapsCPUModelsAdd(cpuModels, old->models[i].name, -1) < 0) + if (virDomainCapsCPUModelsAdd(cpuModels, + old->models[i].name, -1, + old->models[i].usable) < 0) goto error; } @@ -184,7 +189,8 @@ virDomainCapsCPUModelsFilter(virDomainCapsCPUModelsPtr old, continue; if (virDomainCapsCPUModelsAdd(cpuModels, - old->models[i].name, -1) < 0) + old->models[i].name, -1, + old->models[i].usable) < 0) goto error; } @@ -198,13 +204,16 @@ virDomainCapsCPUModelsFilter(virDomainCapsCPUModelsPtr old, int virDomainCapsCPUModelsAddSteal(virDomainCapsCPUModelsPtr cpuModels, - char **name) + char **name, + virDomainCapsCPUUsable usable) { if (VIR_RESIZE_N(cpuModels->models, cpuModels->alloc, cpuModels->count, 1) < 0) return -1; - cpuModels->models[cpuModels->count++].name = *name; + cpuModels->models[cpuModels->count].usable = usable; + cpuModels->models[cpuModels->count].name = *name; + cpuModels->count++; *name = NULL; return 0; } @@ -213,14 +222,15 @@ virDomainCapsCPUModelsAddSteal(virDomainCapsCPUModelsPtr cpuModels, int virDomainCapsCPUModelsAdd(virDomainCapsCPUModelsPtr cpuModels, const char *name, - ssize_t nameLen) + ssize_t nameLen, + virDomainCapsCPUUsable usable) { char *copy = NULL; if (VIR_STRNDUP(copy, name, nameLen) < 0) goto error; - if (virDomainCapsCPUModelsAddSteal(cpuModels, &copy) < 0) + if (virDomainCapsCPUModelsAddSteal(cpuModels, &copy, usable) < 0) goto error; return 0; @@ -366,18 +376,21 @@ virDomainCapsOSFormat(virBufferPtr buf, static void virDomainCapsCPUCustomFormat(virBufferPtr buf, - virDomainCapsCPUModelsPtr custom) + virDomainCapsCPUModelsPtr custom, + virDomainCapsCPUUsable usable) { size_t i; - - virBufferAdjustIndent(buf, 2); + const char *usableStr = virDomainCapsCPUUsableTypeToString(usable); for (i = 0; i < custom->count; i++) { - virBufferAsprintf(buf, "<model>%s</model>\n", - custom->models[i].name); - } + virDomainCapsCPUModelPtr model = custom->models + i; - virBufferAdjustIndent(buf, -2); + if (model->usable != usable) + continue; + + virBufferAsprintf(buf, "<model usable='%s'>%s</model>\n", + usableStr, model->name); + } } static void @@ -399,7 +412,16 @@ virDomainCapsCPUFormat(virBufferPtr buf, virCPUModeTypeToString(VIR_CPU_MODE_CUSTOM)); if (cpu->custom && cpu->custom->count) { virBufferAddLit(buf, "supported='yes'>\n"); - virDomainCapsCPUCustomFormat(buf, cpu->custom); + virBufferAdjustIndent(buf, 2); + + virDomainCapsCPUCustomFormat(buf, cpu->custom, + VIR_DOMCAPS_CPU_USABLE_YES); + virDomainCapsCPUCustomFormat(buf, cpu->custom, + VIR_DOMCAPS_CPU_USABLE_NO); + virDomainCapsCPUCustomFormat(buf, cpu->custom, + VIR_DOMCAPS_CPU_USABLE_UNKNOWN); + + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</mode>\n"); } else { virBufferAddLit(buf, "supported='no'/>\n"); diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h index 29b18bb..7498f89 100644 --- a/src/conf/domain_capabilities.h +++ b/src/conf/domain_capabilities.h @@ -102,10 +102,20 @@ struct _virDomainCapsFeatureGIC { virDomainCapsEnum version; /* Info about virGICVersion */ }; +typedef enum { + VIR_DOMCAPS_CPU_USABLE_UNKNOWN, + VIR_DOMCAPS_CPU_USABLE_YES, + VIR_DOMCAPS_CPU_USABLE_NO, + + VIR_DOMCAPS_CPU_USABLE_LAST +} virDomainCapsCPUUsable; +VIR_ENUM_DECL(virDomainCapsCPUUsable); + typedef struct _virDomainCapsCPUModel virDomainCapsCPUModel; typedef virDomainCapsCPUModel *virDomainCapsCPUModelPtr; struct _virDomainCapsCPUModel { char *name; + virDomainCapsCPUUsable usable; }; typedef struct _virDomainCapsCPUModels virDomainCapsCPUModels; @@ -159,10 +169,12 @@ virDomainCapsCPUModelsPtr virDomainCapsCPUModelsCopy(virDomainCapsCPUModelsPtr o virDomainCapsCPUModelsPtr virDomainCapsCPUModelsFilter(virDomainCapsCPUModelsPtr old, char **models); int virDomainCapsCPUModelsAddSteal(virDomainCapsCPUModelsPtr cpuModels, - char **name); + char **name, + virDomainCapsCPUUsable usable); int virDomainCapsCPUModelsAdd(virDomainCapsCPUModelsPtr cpuModels, const char *name, - ssize_t nameLen); + ssize_t nameLen, + virDomainCapsCPUUsable usable); # define VIR_DOMAIN_CAPS_ENUM_SET(capsEnum, ...) \ do { \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 80c112a..53d4e7f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -157,6 +157,8 @@ virDomainCapsCPUModelsAddSteal; virDomainCapsCPUModelsCopy; virDomainCapsCPUModelsFilter; virDomainCapsCPUModelsNew; +virDomainCapsCPUUsableTypeFromString; +virDomainCapsCPUUsableTypeToString; virDomainCapsEnumClear; virDomainCapsEnumSet; virDomainCapsFormat; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 123aae5..232ae1f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -665,7 +665,8 @@ virQEMUCapsParseX86Models(const char *output, len -= 2; } - if (virDomainCapsCPUModelsAdd(cpus, p, len) < 0) + if (virDomainCapsCPUModelsAdd(cpus, p, len, + VIR_DOMCAPS_CPU_USABLE_UNKNOWN) < 0) goto error; } while ((p = next)); @@ -713,7 +714,8 @@ virQEMUCapsParsePPCModels(const char *output, if (*p == '\n') continue; - if (virDomainCapsCPUModelsAdd(cpus, p, t - p - 1) < 0) + if (virDomainCapsCPUModelsAdd(cpus, p, t - p - 1, + VIR_DOMCAPS_CPU_USABLE_UNKNOWN) < 0) goto error; } while ((p = next)); @@ -2244,7 +2246,8 @@ virQEMUCapsAddCPUDefinitions(virQEMUCapsPtr qemuCaps, return -1; for (i = 0; i < count; i++) { - if (virDomainCapsCPUModelsAdd(qemuCaps->cpuDefinitions, name[i], -1) < 0) + if (virDomainCapsCPUModelsAdd(qemuCaps->cpuDefinitions, name[i], -1, + VIR_DOMCAPS_CPU_USABLE_UNKNOWN) < 0) return -1; } @@ -2593,7 +2596,8 @@ virQEMUCapsProbeQMPCPUDefinitions(virQEMUCapsPtr qemuCaps, for (i = 0; i < ncpus; i++) { if (virDomainCapsCPUModelsAddSteal(qemuCaps->cpuDefinitions, - &cpus[i]->name) < 0) + &cpus[i]->name, + VIR_DOMCAPS_CPU_USABLE_UNKNOWN) < 0) goto cleanup; } @@ -2960,7 +2964,8 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char *filename, } if (virDomainCapsCPUModelsAddSteal(qemuCaps->cpuDefinitions, - &str) < 0) + &str, + VIR_DOMCAPS_CPU_USABLE_UNKNOWN) < 0) goto cleanup; } } diff --git a/tests/domaincapsschemadata/full.xml b/tests/domaincapsschemadata/full.xml index 80fd1f0..1d58e57 100644 --- a/tests/domaincapsschemadata/full.xml +++ b/tests/domaincapsschemadata/full.xml @@ -23,9 +23,9 @@ <mode name='host-passthrough' supported='yes'/> <mode name='host-model' supported='yes'/> <mode name='custom' supported='yes'> - <model>Model1</model> - <model>Model2</model> - <model>Model3</model> + <model usable='yes'>Model3</model> + <model usable='no'>Model2</model> + <model usable='unknown'>Model1</model> </mode> </cpu> <devices> diff --git a/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml b/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml index 4ee2f95..2b17dd0 100644 --- a/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml +++ b/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml @@ -22,30 +22,30 @@ <mode name='host-passthrough' supported='yes'/> <mode name='host-model' supported='yes'/> <mode name='custom' supported='yes'> - <model>Opteron_G5</model> - <model>Opteron_G4</model> - <model>Opteron_G3</model> - <model>Opteron_G2</model> - <model>Opteron_G1</model> - <model>Haswell</model> - <model>SandyBridge</model> - <model>Westmere</model> - <model>Nehalem</model> - <model>Penryn</model> - <model>Conroe</model> - <model>n270</model> - <model>athlon</model> - <model>pentium3</model> - <model>pentium2</model> - <model>pentium</model> - <model>486</model> - <model>coreduo</model> - <model>kvm32</model> - <model>qemu32</model> - <model>kvm64</model> - <model>core2duo</model> - <model>phenom</model> - <model>qemu64</model> + <model usable='unknown'>Opteron_G5</model> + <model usable='unknown'>Opteron_G4</model> + <model usable='unknown'>Opteron_G3</model> + <model usable='unknown'>Opteron_G2</model> + <model usable='unknown'>Opteron_G1</model> + <model usable='unknown'>Haswell</model> + <model usable='unknown'>SandyBridge</model> + <model usable='unknown'>Westmere</model> + <model usable='unknown'>Nehalem</model> + <model usable='unknown'>Penryn</model> + <model usable='unknown'>Conroe</model> + <model usable='unknown'>n270</model> + <model usable='unknown'>athlon</model> + <model usable='unknown'>pentium3</model> + <model usable='unknown'>pentium2</model> + <model usable='unknown'>pentium</model> + <model usable='unknown'>486</model> + <model usable='unknown'>coreduo</model> + <model usable='unknown'>kvm32</model> + <model usable='unknown'>qemu32</model> + <model usable='unknown'>kvm64</model> + <model usable='unknown'>core2duo</model> + <model usable='unknown'>phenom</model> + <model usable='unknown'>qemu64</model> </mode> </cpu> <devices> diff --git a/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml index 9e96f47..8a54f9e 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml @@ -22,36 +22,36 @@ <mode name='host-passthrough' supported='yes'/> <mode name='host-model' supported='yes'/> <mode name='custom' supported='yes'> - <model>pxa262</model> - <model>pxa270-a0</model> - <model>arm1136</model> - <model>cortex-a15</model> - <model>pxa260</model> - <model>arm1136-r2</model> - <model>pxa261</model> - <model>pxa255</model> - <model>arm926</model> - <model>arm11mpcore</model> - <model>pxa250</model> - <model>ti925t</model> - <model>cortex-a57</model> - <model>sa1110</model> - <model>arm1176</model> - <model>cortex-a53</model> - <model>sa1100</model> - <model>pxa270-c5</model> - <model>cortex-a9</model> - <model>cortex-a8</model> - <model>pxa270-c0</model> - <model>arm1026</model> - <model>pxa270-b1</model> - <model>cortex-m3</model> - <model>cortex-m4</model> - <model>pxa270-b0</model> - <model>arm946</model> - <model>cortex-r5</model> - <model>pxa270-a1</model> - <model>pxa270</model> + <model usable='unknown'>pxa262</model> + <model usable='unknown'>pxa270-a0</model> + <model usable='unknown'>arm1136</model> + <model usable='unknown'>cortex-a15</model> + <model usable='unknown'>pxa260</model> + <model usable='unknown'>arm1136-r2</model> + <model usable='unknown'>pxa261</model> + <model usable='unknown'>pxa255</model> + <model usable='unknown'>arm926</model> + <model usable='unknown'>arm11mpcore</model> + <model usable='unknown'>pxa250</model> + <model usable='unknown'>ti925t</model> + <model usable='unknown'>cortex-a57</model> + <model usable='unknown'>sa1110</model> + <model usable='unknown'>arm1176</model> + <model usable='unknown'>cortex-a53</model> + <model usable='unknown'>sa1100</model> + <model usable='unknown'>pxa270-c5</model> + <model usable='unknown'>cortex-a9</model> + <model usable='unknown'>cortex-a8</model> + <model usable='unknown'>pxa270-c0</model> + <model usable='unknown'>arm1026</model> + <model usable='unknown'>pxa270-b1</model> + <model usable='unknown'>cortex-m3</model> + <model usable='unknown'>cortex-m4</model> + <model usable='unknown'>pxa270-b0</model> + <model usable='unknown'>arm946</model> + <model usable='unknown'>cortex-r5</model> + <model usable='unknown'>pxa270-a1</model> + <model usable='unknown'>pxa270</model> </mode> </cpu> <devices> diff --git a/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml index c081b35..8d8087f 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml @@ -22,36 +22,36 @@ <mode name='host-passthrough' supported='yes'/> <mode name='host-model' supported='yes'/> <mode name='custom' supported='yes'> - <model>pxa262</model> - <model>pxa270-a0</model> - <model>arm1136</model> - <model>cortex-a15</model> - <model>pxa260</model> - <model>arm1136-r2</model> - <model>pxa261</model> - <model>pxa255</model> - <model>arm926</model> - <model>arm11mpcore</model> - <model>pxa250</model> - <model>ti925t</model> - <model>cortex-a57</model> - <model>sa1110</model> - <model>arm1176</model> - <model>cortex-a53</model> - <model>sa1100</model> - <model>pxa270-c5</model> - <model>cortex-a9</model> - <model>cortex-a8</model> - <model>pxa270-c0</model> - <model>arm1026</model> - <model>pxa270-b1</model> - <model>cortex-m3</model> - <model>cortex-m4</model> - <model>pxa270-b0</model> - <model>arm946</model> - <model>cortex-r5</model> - <model>pxa270-a1</model> - <model>pxa270</model> + <model usable='unknown'>pxa262</model> + <model usable='unknown'>pxa270-a0</model> + <model usable='unknown'>arm1136</model> + <model usable='unknown'>cortex-a15</model> + <model usable='unknown'>pxa260</model> + <model usable='unknown'>arm1136-r2</model> + <model usable='unknown'>pxa261</model> + <model usable='unknown'>pxa255</model> + <model usable='unknown'>arm926</model> + <model usable='unknown'>arm11mpcore</model> + <model usable='unknown'>pxa250</model> + <model usable='unknown'>ti925t</model> + <model usable='unknown'>cortex-a57</model> + <model usable='unknown'>sa1110</model> + <model usable='unknown'>arm1176</model> + <model usable='unknown'>cortex-a53</model> + <model usable='unknown'>sa1100</model> + <model usable='unknown'>pxa270-c5</model> + <model usable='unknown'>cortex-a9</model> + <model usable='unknown'>cortex-a8</model> + <model usable='unknown'>pxa270-c0</model> + <model usable='unknown'>arm1026</model> + <model usable='unknown'>pxa270-b1</model> + <model usable='unknown'>cortex-m3</model> + <model usable='unknown'>cortex-m4</model> + <model usable='unknown'>pxa270-b0</model> + <model usable='unknown'>arm946</model> + <model usable='unknown'>cortex-r5</model> + <model usable='unknown'>pxa270-a1</model> + <model usable='unknown'>pxa270</model> </mode> </cpu> <devices> diff --git a/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml index 811d2b7..83c03db 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml @@ -22,36 +22,36 @@ <mode name='host-passthrough' supported='yes'/> <mode name='host-model' supported='yes'/> <mode name='custom' supported='yes'> - <model>pxa262</model> - <model>pxa270-a0</model> - <model>arm1136</model> - <model>cortex-a15</model> - <model>pxa260</model> - <model>arm1136-r2</model> - <model>pxa261</model> - <model>pxa255</model> - <model>arm926</model> - <model>arm11mpcore</model> - <model>pxa250</model> - <model>ti925t</model> - <model>cortex-a57</model> - <model>sa1110</model> - <model>arm1176</model> - <model>cortex-a53</model> - <model>sa1100</model> - <model>pxa270-c5</model> - <model>cortex-a9</model> - <model>cortex-a8</model> - <model>pxa270-c0</model> - <model>arm1026</model> - <model>pxa270-b1</model> - <model>cortex-m3</model> - <model>cortex-m4</model> - <model>pxa270-b0</model> - <model>arm946</model> - <model>cortex-r5</model> - <model>pxa270-a1</model> - <model>pxa270</model> + <model usable='unknown'>pxa262</model> + <model usable='unknown'>pxa270-a0</model> + <model usable='unknown'>arm1136</model> + <model usable='unknown'>cortex-a15</model> + <model usable='unknown'>pxa260</model> + <model usable='unknown'>arm1136-r2</model> + <model usable='unknown'>pxa261</model> + <model usable='unknown'>pxa255</model> + <model usable='unknown'>arm926</model> + <model usable='unknown'>arm11mpcore</model> + <model usable='unknown'>pxa250</model> + <model usable='unknown'>ti925t</model> + <model usable='unknown'>cortex-a57</model> + <model usable='unknown'>sa1110</model> + <model usable='unknown'>arm1176</model> + <model usable='unknown'>cortex-a53</model> + <model usable='unknown'>sa1100</model> + <model usable='unknown'>pxa270-c5</model> + <model usable='unknown'>cortex-a9</model> + <model usable='unknown'>cortex-a8</model> + <model usable='unknown'>pxa270-c0</model> + <model usable='unknown'>arm1026</model> + <model usable='unknown'>pxa270-b1</model> + <model usable='unknown'>cortex-m3</model> + <model usable='unknown'>cortex-m4</model> + <model usable='unknown'>pxa270-b0</model> + <model usable='unknown'>arm946</model> + <model usable='unknown'>cortex-r5</model> + <model usable='unknown'>pxa270-a1</model> + <model usable='unknown'>pxa270</model> </mode> </cpu> <devices> diff --git a/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml b/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml index d969274..14a087b 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml @@ -22,8 +22,8 @@ <mode name='host-passthrough' supported='yes'/> <mode name='host-model' supported='yes'/> <mode name='custom' supported='yes'> - <model>POWER8</model> - <model>POWER7</model> + <model usable='unknown'>POWER8</model> + <model usable='unknown'>POWER7</model> </mode> </cpu> <devices> diff --git a/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml b/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml index 80101a4..4294c64 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml @@ -22,34 +22,34 @@ <mode name='host-passthrough' supported='yes'/> <mode name='host-model' supported='yes'/> <mode name='custom' supported='yes'> - <model>Opteron_G5</model> - <model>Opteron_G4</model> - <model>Opteron_G3</model> - <model>Opteron_G2</model> - <model>Opteron_G1</model> - <model>Broadwell</model> - <model>Broadwell-noTSX</model> - <model>Haswell</model> - <model>Haswell-noTSX</model> - <model>IvyBridge</model> - <model>SandyBridge</model> - <model>Westmere</model> - <model>Nehalem</model> - <model>Penryn</model> - <model>Conroe</model> - <model>n270</model> - <model>athlon</model> - <model>pentium3</model> - <model>pentium2</model> - <model>pentium</model> - <model>486</model> - <model>coreduo</model> - <model>kvm32</model> - <model>qemu32</model> - <model>kvm64</model> - <model>core2duo</model> - <model>phenom</model> - <model>qemu64</model> + <model usable='unknown'>Opteron_G5</model> + <model usable='unknown'>Opteron_G4</model> + <model usable='unknown'>Opteron_G3</model> + <model usable='unknown'>Opteron_G2</model> + <model usable='unknown'>Opteron_G1</model> + <model usable='unknown'>Broadwell</model> + <model usable='unknown'>Broadwell-noTSX</model> + <model usable='unknown'>Haswell</model> + <model usable='unknown'>Haswell-noTSX</model> + <model usable='unknown'>IvyBridge</model> + <model usable='unknown'>SandyBridge</model> + <model usable='unknown'>Westmere</model> + <model usable='unknown'>Nehalem</model> + <model usable='unknown'>Penryn</model> + <model usable='unknown'>Conroe</model> + <model usable='unknown'>n270</model> + <model usable='unknown'>athlon</model> + <model usable='unknown'>pentium3</model> + <model usable='unknown'>pentium2</model> + <model usable='unknown'>pentium</model> + <model usable='unknown'>486</model> + <model usable='unknown'>coreduo</model> + <model usable='unknown'>kvm32</model> + <model usable='unknown'>qemu32</model> + <model usable='unknown'>kvm64</model> + <model usable='unknown'>core2duo</model> + <model usable='unknown'>phenom</model> + <model usable='unknown'>qemu64</model> </mode> </cpu> <devices> diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index 10b7452..511066d 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -81,9 +81,12 @@ fillAllCaps(virDomainCapsPtr domCaps) cpu->hostPassthrough = true; cpu->hostModel = true; if (!(cpu->custom = virDomainCapsCPUModelsNew(3)) || - virDomainCapsCPUModelsAdd(cpu->custom, "Model1", -1) < 0 || - virDomainCapsCPUModelsAdd(cpu->custom, "Model2", -1) < 0 || - virDomainCapsCPUModelsAdd(cpu->custom, "Model3", -1) < 0) + virDomainCapsCPUModelsAdd(cpu->custom, "Model1", -1, + VIR_DOMCAPS_CPU_USABLE_UNKNOWN) < 0 || + virDomainCapsCPUModelsAdd(cpu->custom, "Model2", -1, + VIR_DOMCAPS_CPU_USABLE_NO) < 0 || + virDomainCapsCPUModelsAdd(cpu->custom, "Model3", -1, + VIR_DOMCAPS_CPU_USABLE_YES) < 0) return -1; disk->supported = true; -- 2.9.2

On Mon, Aug 29, 2016 at 19:21:24 -0400, John Ferlan wrote: ... Fixed. Exactly, it's output only and we always format it so there's no reason for the attribute to be optional. ... Hmm, it really seems pretty stupid to group them by usable value, doesn't it? I just fixed the code so that the CPU models are printed in a single loop no matter whether they're usable or not. Hmm, you're right, I missed this in the cover letter. For any hypervisor that doesn't give us the usability data, all CPUs will have usable='unknown'. Or were you thinking about why don't we just skip this attribute if it's value is unknown? If so, I think it's better to be explicit. ... There's no code in QEMU driver which would set the value to anything but 'unknown' yet. There's no conversion involved. This is just a preparation for a later patch that would really ask QEMU for this stuff. Jirka

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- docs/schemas/cputypes.rng | 135 ++++++++++++++++++++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 129 +--------------------------------------- 2 files changed, 136 insertions(+), 128 deletions(-) create mode 100644 docs/schemas/cputypes.rng diff --git a/docs/schemas/cputypes.rng b/docs/schemas/cputypes.rng new file mode 100644 index 0000000..7cc9dd3 --- /dev/null +++ b/docs/schemas/cputypes.rng @@ -0,0 +1,135 @@ +<?xml version="1.0"?> +<!-- CPU-related definitions used in multiple grammars --> +<grammar xmlns="http://relaxng.org/ns/structure/1.0" + datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> + + <define name="cpuMode"> + <attribute name="mode"> + <choice> + <value>custom</value> + <value>host-model</value> + <value>host-passthrough</value> + </choice> + </attribute> + </define> + + <define name="cpuMatch"> + <attribute name="match"> + <choice> + <value>minimum</value> + <value>exact</value> + <value>strict</value> + </choice> + </attribute> + </define> + + <define name="cpuModel"> + <element name="model"> + <optional> + <attribute name="fallback"> + <choice> + <value>allow</value> + <value>forbid</value> + </choice> + </attribute> + </optional> + <optional> + <attribute name="vendor_id"> + <data type="string"> + <param name='pattern'>[^,]{12}</param> + </data> + </attribute> + </optional> + <choice> + <text/> + <empty/> + </choice> + </element> + </define> + + <define name="cpuVendor"> + <element name="vendor"> + <text/> + </element> + </define> + + <define name="cpuFeature"> + <element name="feature"> + <attribute name="policy"> + <choice> + <value>force</value> + <value>require</value> + <value>optional</value> + <value>disable</value> + <value>forbid</value> + </choice> + </attribute> + <attribute name="name"> + <ref name="featureName"/> + </attribute> + <empty/> + </element> + </define> + + <define name="cpuTopology"> + <element name="topology"> + <attribute name="sockets"> + <ref name="positiveInteger"/> + </attribute> + <attribute name="cores"> + <ref name="positiveInteger"/> + </attribute> + <attribute name="threads"> + <ref name="positiveInteger"/> + </attribute> + </element> + </define> + + <define name="cpuNuma"> + <element name="numa"> + <oneOrMore> + <ref name="numaCell"/> + </oneOrMore> + </element> + </define> + + <define name="numaCell"> + <element name="cell"> + <optional> + <attribute name="id"> + <ref name="unsignedInt"/> + </attribute> + </optional> + <attribute name="cpus"> + <ref name="cpuset"/> + </attribute> + <attribute name="memory"> + <ref name="memoryKB"/> + </attribute> + <optional> + <attribute name="unit"> + <ref name="unit"/> + </attribute> + </optional> + <optional> + <attribute name="memAccess"> + <choice> + <value>shared</value> + <value>private</value> + </choice> + </attribute> + </optional> + </element> + </define> + + <!-- Memory as an attribute is in KiB, no way to express a unit --> + <define name="memoryKB"> + <data type="unsignedLong"/> + </define> + <define name="featureName"> + <data type="string"> + <param name='pattern'>[a-zA-Z0-9\-_\.]+</param> + </data> + </define> + +</grammar> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 052f28c..c06f700 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4,6 +4,7 @@ <include href='basictypes.rng'/> <include href='storagecommon.rng'/> <include href='networkcommon.rng'/> + <include href='cputypes.rng'/> <!-- description and title element, may be placed anywhere under the root @@ -4377,125 +4378,6 @@ </element> </define> - <define name="cpuMode"> - <attribute name="mode"> - <choice> - <value>custom</value> - <value>host-model</value> - <value>host-passthrough</value> - </choice> - </attribute> - </define> - - <define name="cpuMatch"> - <attribute name="match"> - <choice> - <value>minimum</value> - <value>exact</value> - <value>strict</value> - </choice> - </attribute> - </define> - - <define name="cpuModel"> - <element name="model"> - <optional> - <attribute name="fallback"> - <choice> - <value>allow</value> - <value>forbid</value> - </choice> - </attribute> - </optional> - <optional> - <attribute name="vendor_id"> - <data type="string"> - <param name='pattern'>[^,]{12}</param> - </data> - </attribute> - </optional> - <choice> - <text/> - <empty/> - </choice> - </element> - </define> - - <define name="cpuVendor"> - <element name="vendor"> - <text/> - </element> - </define> - - <define name="cpuFeature"> - <element name="feature"> - <attribute name="policy"> - <choice> - <value>force</value> - <value>require</value> - <value>optional</value> - <value>disable</value> - <value>forbid</value> - </choice> - </attribute> - <attribute name="name"> - <ref name="featureName"/> - </attribute> - <empty/> - </element> - </define> - - <define name="cpuTopology"> - <element name="topology"> - <attribute name="sockets"> - <ref name="positiveInteger"/> - </attribute> - <attribute name="cores"> - <ref name="positiveInteger"/> - </attribute> - <attribute name="threads"> - <ref name="positiveInteger"/> - </attribute> - </element> - </define> - - <define name="cpuNuma"> - <element name="numa"> - <oneOrMore> - <ref name="numaCell"/> - </oneOrMore> - </element> - </define> - - <define name="numaCell"> - <element name="cell"> - <optional> - <attribute name="id"> - <ref name="unsignedInt"/> - </attribute> - </optional> - <attribute name="cpus"> - <ref name="cpuset"/> - </attribute> - <attribute name="memory"> - <ref name="memoryKB"/> - </attribute> - <optional> - <attribute name="unit"> - <ref name="unit"/> - </attribute> - </optional> - <optional> - <attribute name="memAccess"> - <choice> - <value>shared</value> - <value>private</value> - </choice> - </attribute> - </optional> - </element> - </define> - <!-- System information specification: Placeholder for system specific informations likes the ones @@ -5440,10 +5322,6 @@ <param name="maxInclusive">1000</param> </data> </define> - <!-- Memory as an attribute is in KiB, no way to express a unit --> - <define name="memoryKB"> - <data type="unsignedLong"/> - </define> <define name="domainName"> <data type="string"> <!-- Use literal newline instead of \n for bug in libxml2 2.7.6 --> @@ -5527,11 +5405,6 @@ <param name="pattern">[0-9]{1,20}</param> </data> </define> - <define name="featureName"> - <data type="string"> - <param name='pattern'>[a-zA-Z0-9\-_\.]+</param> - </data> - </define> <define name="timeDelta"> <data type="string"> <param name="pattern">(-|\+)?[0-9]+</param> -- 2.9.2

On 08/12/2016 09:33 AM, Jiri Denemark wrote: OK I lied slightly... 21 ACK ... build breaks here - probably because of new function qemuDomainDefValidate... luckily caps is passed as ATTRIBUTE_UNUSED, so it seems to be easily handled... John

Useful for copying a CPU definition without model related parts (i.e., without model name, feature list, vendor). Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/conf/cpu_conf.c | 16 +++++++++++++++- src/conf/cpu_conf.h | 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index b71528e..c6e847a 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -114,8 +114,9 @@ virCPUDefCopyModel(virCPUDefPtr dst, return 0; } + virCPUDefPtr -virCPUDefCopy(const virCPUDef *cpu) +virCPUDefCopyWithoutModel(const virCPUDef *cpu) { virCPUDefPtr copy; @@ -131,6 +132,18 @@ virCPUDefCopy(const virCPUDef *cpu) copy->threads = cpu->threads; copy->arch = cpu->arch; + return copy; +} + + +virCPUDefPtr +virCPUDefCopy(const virCPUDef *cpu) +{ + virCPUDefPtr copy; + + if (!(copy = virCPUDefCopyWithoutModel(cpu))) + return NULL; + if (virCPUDefCopyModel(copy, cpu, false) < 0) goto error; @@ -141,6 +154,7 @@ virCPUDefCopy(const virCPUDef *cpu) return NULL; } + virCPUDefPtr virCPUDefParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index 705ba6d..2bbab9e 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -127,6 +127,9 @@ virCPUDefPtr virCPUDefCopy(const virCPUDef *cpu); virCPUDefPtr +virCPUDefCopyWithoutModel(const virCPUDef *cpu); + +virCPUDefPtr virCPUDefParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt, virCPUType mode); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 53d4e7f..26f5bc8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -66,6 +66,7 @@ virCapabilitiesSetNetPrefix; virCPUDefAddFeature; virCPUDefCopy; virCPUDefCopyModel; +virCPUDefCopyWithoutModel; virCPUDefFormat; virCPUDefFormatBuf; virCPUDefFree; -- 2.9.2

On 08/12/2016 09:33 AM, Jiri Denemark wrote: Less "Move" and more "Take"... virCPUDefStealModel is a suggestion - of course it's more than ->model, but close enough I think.

The function filters all CPU features through a given callback while copying CPU model related parts of a CPU definition. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/conf/cpu_conf.c | 31 ++++++++++++++++++++++++------- src/conf/cpu_conf.h | 12 ++++++++++++ src/libvirt_private.syms | 1 + 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 13f3da3..b74c48b 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -86,28 +86,45 @@ virCPUDefCopyModel(virCPUDefPtr dst, const virCPUDef *src, bool resetPolicy) { + return virCPUDefCopyModelFilter(dst, src, resetPolicy, NULL, NULL); +} + + +int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) +virCPUDefCopyModelFilter(virCPUDefPtr dst, + const virCPUDef *src, + bool resetPolicy, + virCPUDefFeatureFilter filter, + void *opaque) +{ size_t i; + size_t n; if (VIR_STRDUP(dst->model, src->model) < 0 || VIR_STRDUP(dst->vendor, src->vendor) < 0 || VIR_STRDUP(dst->vendor_id, src->vendor_id) < 0 || VIR_ALLOC_N(dst->features, src->nfeatures) < 0) return -1; - dst->nfeatures_max = dst->nfeatures = src->nfeatures; + dst->nfeatures_max = src->nfeatures; + dst->nfeatures = 0; - for (i = 0; i < dst->nfeatures; i++) { + for (i = 0; i < src->nfeatures; i++) { + if (filter && !filter(src->features[i].name, opaque)) + continue; + + n = dst->nfeatures++; if (dst->type != src->type && resetPolicy) { if (dst->type == VIR_CPU_TYPE_HOST) - dst->features[i].policy = -1; + dst->features[n].policy = -1; else if (src->features[i].policy == -1) - dst->features[i].policy = VIR_CPU_FEATURE_REQUIRE; + dst->features[n].policy = VIR_CPU_FEATURE_REQUIRE; else - dst->features[i].policy = src->features[i].policy; + dst->features[n].policy = src->features[i].policy; } else { - dst->features[i].policy = src->features[i].policy; + dst->features[n].policy = src->features[i].policy; } - if (VIR_STRDUP(dst->features[i].name, src->features[i].name) < 0) + if (VIR_STRDUP(dst->features[n].name, src->features[i].name) < 0) return -1; } diff --git a/src/conf/cpu_conf.h b/src/conf/cpu_conf.h index d866a89..9d2d9b6 100644 --- a/src/conf/cpu_conf.h +++ b/src/conf/cpu_conf.h @@ -123,6 +123,18 @@ virCPUDefCopyModel(virCPUDefPtr dst, const virCPUDef *src, bool resetPolicy); +/* + * Returns true if feature @name should copied, false otherwise. + */ +typedef bool (*virCPUDefFeatureFilter)(const char *name, + void *opaque); + +int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) +virCPUDefCopyModelFilter(virCPUDefPtr dst, + const virCPUDef *src, + bool resetPolicy, + virCPUDefFeatureFilter filter, + void *opaque); void virCPUDefMoveModel(virCPUDefPtr dst, virCPUDefPtr src); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1cfebd5..3a33956 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -66,6 +66,7 @@ virCapabilitiesSetNetPrefix; virCPUDefAddFeature; virCPUDefCopy; virCPUDefCopyModel; +virCPUDefCopyModelFilter; virCPUDefCopyWithoutModel; virCPUDefFormat; virCPUDefFormatBuf; -- 2.9.2

On Tue, Aug 30, 2016 at 10:25:10 -0400, John Ferlan wrote: Copy & paste I guess. Fixed. Jirka

On 08/12/2016 09:33 AM, Jiri Denemark wrote: I personally think this should be closer to patch 22 where the caps gets introduced into virQEMUCapsNewForBinaryInternal, but it's not a review requirement... hostCpuModel ? IOW: Some way to make it more clear to a casual reader and perhaps easier to find via cscope Also I note that this isn't being written out to the cache (e.g. no change to virQEMUCapsFormatCache), so probably need to "note" that some how. Furthermore, perhaps this should be moved after the gic stuff and the note made that anything "after" this point isn't written out, rather it's refetched during Load This is leaked - eg. no virCPUDefFree in virQEMUCapsDispose Since this is a void function it is possible to have a failure (above) thus having qemuCaps->cpuModel be NULL... Store that knowledge away as it becomes important later in patch 40 as processing will call virQEMUCapsGetHostModel which returns this field. Should we virResetLastError() since we don't care about the failure in the two callers? [...]

The ARM CPU driver wrongly reported host CPU model as "host", which made host-model to be just an alias for host-passthrough. Let's drop this insanity. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/cpu/cpu_arm.c | 34 ++-------------------------------- 1 file changed, 2 insertions(+), 32 deletions(-) diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index 6090253..a3aed6b 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -37,36 +37,6 @@ static const virArch archs[] = { VIR_ARCH_AARCH64, }; -static virCPUDataPtr -armNodeData(virArch arch) -{ - virCPUDataPtr data; - - if (VIR_ALLOC(data) < 0) - return NULL; - - data->arch = arch; - - return data; -} - -static int -armDecode(virCPUDefPtr cpu, - const virCPUData *data ATTRIBUTE_UNUSED, - const char **models ATTRIBUTE_UNUSED, - unsigned int nmodels ATTRIBUTE_UNUSED, - const char *preferred ATTRIBUTE_UNUSED, - unsigned int flags) -{ - virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1); - - if (cpu->model == NULL && - VIR_STRDUP(cpu->model, "host") < 0) - return -1; - - return 0; -} - static void armDataFree(virCPUDataPtr data) { @@ -128,10 +98,10 @@ struct cpuArchDriver cpuDriverArm = { .arch = archs, .narch = ARRAY_CARDINALITY(archs), .compare = armCompare, - .decode = armDecode, + .decode = NULL, .encode = NULL, .free = armDataFree, - .nodeData = armNodeData, + .nodeData = NULL, .guestData = armGuestData, .baseline = armBaseline, .update = armUpdate, -- 2.9.2

The domain capabilities XML is capable of showing whether each guest CPU mode is supported or not with a possibility to provide additional details. This patch enhances host-model capability to advertise the exact CPU model which will be used as a host-model: <cpu> ... <mode name='host-model' supported='yes'> <model fallback='allow'>Broadwell</model> <vendor>Intel</vendor> <feature policy='disable' name='aes'/> <feature policy='require' name='vmx'/> </mode> ... </cpu> Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- docs/formatdomaincaps.html.in | 21 +++++++++++++++++++-- docs/schemas/domaincaps.rng | 10 ++++++++++ src/conf/domain_capabilities.c | 16 +++++++++++++--- src/conf/domain_capabilities.h | 2 +- src/qemu/qemu_capabilities.c | 4 +--- tests/domaincapsschemadata/full.xml | 5 ++++- tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml | 4 +++- .../qemu_2.6.0-gicv2-virt.aarch64.xml | 2 +- .../qemu_2.6.0-gicv3-virt.aarch64.xml | 2 +- tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml | 2 +- tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml | 4 +++- tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml | 4 +++- tests/domaincapstest.c | 10 ++++++++-- 13 files changed, 68 insertions(+), 18 deletions(-) diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in index 49ccbfc..34eb777 100644 --- a/docs/formatdomaincaps.html.in +++ b/docs/formatdomaincaps.html.in @@ -154,7 +154,12 @@ ... &lt;cpu&gt; &lt;mode name='host-passthrough' supported='yes'/&gt; - &lt;mode name='host-model' supported='yes'/&gt; + &lt;mode name='host-model' supported='yes'&gt; + &lt;model fallback='allow'&gt;Broadwell&lt;/model&gt; + &lt;vendor&gt;Intel&lt;/vendor&gt; + &lt;feature policy='disable' name='aes'/&gt; + &lt;feature policy='require' name='vmx'/&gt; + &lt;/mode&gt; &lt;mode name='custom' supported='yes'&gt; &lt;model usable='no'&gt;Broadwell&lt;/model&gt; &lt;model usable='yes'&gt;Broadwell-noTSX&lt;/model&gt; @@ -177,7 +182,19 @@ <dd>No mode specific details are provided.</dd> <dt><code>host-model</code></dt> - <dd>No mode specific details are provided yet.</dd> + <dd> + If <code>host-model</code> is supported by the hypervisor, the + <code>mode</code> describes the guest CPU which will be used when + starting a domain with <code>host-model</code> CPU. The hypervisor + specifics (such as unsupported CPU models or features, machine type, + etc.) may be accounted for in this guest CPU specification and thus + the CPU can be different from the one shown in host capabilities XML. + This is indicated by the <code>fallback</code> attribute of the + <code>model</code> sub element: <code>allow</code> means not all + specifics were accounted for and thus the CPU a guest will see may + be different; <code>forbid</code> says that the CPU a guest will see + should match this CPU definition. + </dd> <dt><code>custom</code></dt> <dd> diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng index 5a605a7..20cbc4e 100644 --- a/docs/schemas/domaincaps.rng +++ b/docs/schemas/domaincaps.rng @@ -2,6 +2,7 @@ <!-- A Relax NG schema for the libvirt domain capabilities XML format --> <grammar xmlns="http://relaxng.org/ns/structure/1.0" datatypeLibrary="http://www.w3.org/2001/XMLSchema-datatypes"> <include href='basictypes.rng'/> + <include href='cputypes.rng'/> <start> <ref name='domainCapabilities'/> </start> @@ -94,6 +95,15 @@ <value>host-model</value> </attribute> <ref name='supported'/> + <optional> + <ref name="cpuModel"/> + <optional> + <ref name="cpuVendor"/> + </optional> + <zeroOrMore> + <ref name="cpuFeature"/> + </zeroOrMore> + </optional> </element> </define> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c index c9e3a28..34379e9 100644 --- a/src/conf/domain_capabilities.c +++ b/src/conf/domain_capabilities.c @@ -404,9 +404,19 @@ virDomainCapsCPUFormat(virBufferPtr buf, virCPUModeTypeToString(VIR_CPU_MODE_HOST_PASSTHROUGH), cpu->hostPassthrough ? "yes" : "no"); - virBufferAsprintf(buf, "<mode name='%s' supported='%s'/>\n", - virCPUModeTypeToString(VIR_CPU_MODE_HOST_MODEL), - cpu->hostModel ? "yes" : "no"); + virBufferAsprintf(buf, "<mode name='%s' ", + virCPUModeTypeToString(VIR_CPU_MODE_HOST_MODEL)); + if (cpu->hostModel) { + virBufferAddLit(buf, "supported='yes'>\n"); + virBufferAdjustIndent(buf, 2); + + virCPUDefFormatBuf(buf, cpu->hostModel, false); + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</mode>\n"); + } else { + virBufferAddLit(buf, "supported='no'/>\n"); + } virBufferAsprintf(buf, "<mode name='%s' ", virCPUModeTypeToString(VIR_CPU_MODE_CUSTOM)); diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h index 7498f89..36693d7 100644 --- a/src/conf/domain_capabilities.h +++ b/src/conf/domain_capabilities.h @@ -132,7 +132,7 @@ typedef struct _virDomainCapsCPU virDomainCapsCPU; typedef virDomainCapsCPU *virDomainCapsCPUPtr; struct _virDomainCapsCPU { bool hostPassthrough; - bool hostModel; + virCPUDefPtr hostModel; virDomainCapsCPUModelsPtr custom; }; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 97dc877..5197180 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4260,9 +4260,7 @@ virQEMUCapsFillDomainCPUCaps(virCapsPtr caps, virQEMUCapsGuestIsNative(caps->host.arch, qemuCaps->arch)) domCaps->cpu.hostPassthrough = true; - if (qemuCaps->cpuDefinitions && caps && caps->host.cpu) - domCaps->cpu.hostModel = virQEMUCapsGuestIsNative(caps->host.arch, - qemuCaps->arch); + domCaps->cpu.hostModel = virCPUDefCopy(qemuCaps->cpuModel); if (qemuCaps->cpuDefinitions && cpuGetModels(domCaps->arch, &models) >= 0) { diff --git a/tests/domaincapsschemadata/full.xml b/tests/domaincapsschemadata/full.xml index 1d58e57..5853151 100644 --- a/tests/domaincapsschemadata/full.xml +++ b/tests/domaincapsschemadata/full.xml @@ -21,7 +21,10 @@ </os> <cpu> <mode name='host-passthrough' supported='yes'/> - <mode name='host-model' supported='yes'/> + <mode name='host-model' supported='yes'> + <model>host</model> + <vendor>CPU Vendorrr</vendor> + </mode> <mode name='custom' supported='yes'> <model usable='yes'>Model3</model> <model usable='no'>Model2</model> diff --git a/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml b/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml index 2b17dd0..4aa475c 100644 --- a/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml +++ b/tests/domaincapsschemadata/qemu_1.7.0.x86_64.xml @@ -20,7 +20,9 @@ </os> <cpu> <mode name='host-passthrough' supported='yes'/> - <mode name='host-model' supported='yes'/> + <mode name='host-model' supported='yes'> + <model fallback='allow'>Broadwell</model> + </mode> <mode name='custom' supported='yes'> <model usable='unknown'>Opteron_G5</model> <model usable='unknown'>Opteron_G4</model> diff --git a/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml index 8a54f9e..796c3af 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0-gicv2-virt.aarch64.xml @@ -20,7 +20,7 @@ </os> <cpu> <mode name='host-passthrough' supported='yes'/> - <mode name='host-model' supported='yes'/> + <mode name='host-model' supported='no'/> <mode name='custom' supported='yes'> <model usable='unknown'>pxa262</model> <model usable='unknown'>pxa270-a0</model> diff --git a/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml index 8d8087f..5a5f82c 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0-gicv3-virt.aarch64.xml @@ -20,7 +20,7 @@ </os> <cpu> <mode name='host-passthrough' supported='yes'/> - <mode name='host-model' supported='yes'/> + <mode name='host-model' supported='no'/> <mode name='custom' supported='yes'> <model usable='unknown'>pxa262</model> <model usable='unknown'>pxa270-a0</model> diff --git a/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml b/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml index 83c03db..90b57ff 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0.aarch64.xml @@ -20,7 +20,7 @@ </os> <cpu> <mode name='host-passthrough' supported='yes'/> - <mode name='host-model' supported='yes'/> + <mode name='host-model' supported='no'/> <mode name='custom' supported='yes'> <model usable='unknown'>pxa262</model> <model usable='unknown'>pxa270-a0</model> diff --git a/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml b/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml index 14a087b..962be6f 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0.ppc64le.xml @@ -20,7 +20,9 @@ </os> <cpu> <mode name='host-passthrough' supported='yes'/> - <mode name='host-model' supported='yes'/> + <mode name='host-model' supported='yes'> + <model fallback='allow'>POWER8</model> + </mode> <mode name='custom' supported='yes'> <model usable='unknown'>POWER8</model> <model usable='unknown'>POWER7</model> diff --git a/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml b/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml index 4294c64..a8975e8 100644 --- a/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml +++ b/tests/domaincapsschemadata/qemu_2.6.0.x86_64.xml @@ -20,7 +20,9 @@ </os> <cpu> <mode name='host-passthrough' supported='yes'/> - <mode name='host-model' supported='yes'/> + <mode name='host-model' supported='yes'> + <model fallback='allow'>Broadwell</model> + </mode> <mode name='custom' supported='yes'> <model usable='unknown'>Opteron_G5</model> <model usable='unknown'>Opteron_G4</model> diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c index 99971c2..e70fa05 100644 --- a/tests/domaincapstest.c +++ b/tests/domaincapstest.c @@ -65,8 +65,14 @@ fillAllCaps(virDomainCapsPtr domCaps) virDomainCapsDeviceGraphicsPtr graphics = &domCaps->graphics; virDomainCapsDeviceVideoPtr video = &domCaps->video; virDomainCapsDeviceHostdevPtr hostdev = &domCaps->hostdev; - domCaps->maxvcpus = 255; + virCPUDef host = { + VIR_CPU_TYPE_HOST, 0, 0, + VIR_ARCH_X86_64, (char *) "host", + NULL, 0, (char *) "CPU Vendorrr", + 0, 0, 0, 0, 0, NULL, + }; + domCaps->maxvcpus = 255; os->supported = true; loader->supported = true; @@ -79,7 +85,7 @@ fillAllCaps(virDomainCapsPtr domCaps) return -1; cpu->hostPassthrough = true; - cpu->hostModel = true; + cpu->hostModel = virCPUDefCopy(&host); if (!(cpu->custom = virDomainCapsCPUModelsNew(3)) || virDomainCapsCPUModelsAdd(cpu->custom, "Model1", -1, VIR_DOMCAPS_CPU_USABLE_UNKNOWN) < 0 || -- 2.9.2

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/qemu/qemu_capabilities.c | 55 ++++++++++++++++++++++++++++++++++---------- src/qemu/qemu_capabilities.h | 4 ++++ 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 670f944..f70a36c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2302,6 +2302,32 @@ virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps) } +bool +virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps, + virCapsPtr caps, + virDomainVirtType type, + virCPUMode mode) +{ + switch (mode) { + case VIR_CPU_MODE_HOST_PASSTHROUGH: + return type == VIR_DOMAIN_VIRT_KVM && + virQEMUCapsGuestIsNative(caps->host.arch, qemuCaps->arch); + + case VIR_CPU_MODE_HOST_MODEL: + return !!qemuCaps->cpuModel; + + case VIR_CPU_MODE_CUSTOM: + return qemuCaps->cpuDefinitions && + qemuCaps->cpuDefinitions->count > 0; + + case VIR_CPU_MODE_LAST: + break; + } + + return false; +} + + int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps, size_t *nmachines, virCapsGuestMachinePtr **machines) @@ -4260,22 +4286,27 @@ virQEMUCapsFillDomainCPUCaps(virCapsPtr caps, virQEMUCapsPtr qemuCaps, virDomainCapsPtr domCaps) { - virDomainCapsCPUModelsPtr filtered = NULL; - char **models = NULL; - - if (domCaps->virttype == VIR_DOMAIN_VIRT_KVM && - virQEMUCapsGuestIsNative(caps->host.arch, qemuCaps->arch)) + if (virQEMUCapsIsCPUModeSupported(qemuCaps, caps, domCaps->virttype, + VIR_CPU_MODE_HOST_PASSTHROUGH)) domCaps->cpu.hostPassthrough = true; - domCaps->cpu.hostModel = virCPUDefCopy(qemuCaps->cpuModel); + if (virQEMUCapsIsCPUModeSupported(qemuCaps, caps, domCaps->virttype, + VIR_CPU_MODE_HOST_MODEL)) + domCaps->cpu.hostModel = virCPUDefCopy(qemuCaps->cpuModel); - if (qemuCaps->cpuDefinitions && - cpuGetModels(domCaps->arch, &models) >= 0) { - filtered = virDomainCapsCPUModelsFilter(qemuCaps->cpuDefinitions, - models); - virStringFreeList(models); + if (virQEMUCapsIsCPUModeSupported(qemuCaps, caps, domCaps->virttype, + VIR_CPU_MODE_CUSTOM)) { + virDomainCapsCPUModelsPtr filtered = NULL; + char **models = NULL; + + if (qemuCaps->cpuDefinitions && + cpuGetModels(domCaps->arch, &models) >= 0) { + filtered = virDomainCapsCPUModelsFilter(qemuCaps->cpuDefinitions, + models); + virStringFreeList(models); + } + domCaps->cpu.custom = filtered; } - domCaps->cpu.custom = filtered; return 0; } diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 1203073..2ea5849 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -425,6 +425,10 @@ int virQEMUCapsGetCPUDefinitions(virQEMUCapsPtr qemuCaps, char ***names, size_t *count); virCPUDefPtr virQEMUCapsGetHostModel(virQEMUCapsPtr qemuCaps); +bool virQEMUCapsIsCPUModeSupported(virQEMUCapsPtr qemuCaps, + virCapsPtr caps, + virDomainVirtType type, + virCPUMode mode); const char *virQEMUCapsGetCanonicalMachine(virQEMUCapsPtr qemuCaps, const char *name); int virQEMUCapsGetMachineMaxCpus(virQEMUCapsPtr qemuCaps, -- 2.9.2

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/cpu/cpu_x86.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 670b02e..914352d 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1019,21 +1019,28 @@ x86ModelFromCPU(const virCPUDef *cpu, virCPUx86ModelPtr model = NULL; size_t i; + /* host CPU only contains required features; requesting other features + * just returns an empty model + */ + if (cpu->type == VIR_CPU_TYPE_HOST && + policy != VIR_CPU_FEATURE_REQUIRE) + return x86ModelNew(); + if (policy == VIR_CPU_FEATURE_REQUIRE) { if (!(model = x86ModelFind(map, cpu->model))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown CPU model %s"), cpu->model); - goto error; + return NULL; } - if (!(model = x86ModelCopy(model))) - goto error; - } else if (!(model = x86ModelNew())) { - goto error; - } else if (cpu->type == VIR_CPU_TYPE_HOST) { - return model; + model = x86ModelCopy(model); + } else { + model = x86ModelNew(); } + if (!model) + return NULL; + for (i = 0; i < cpu->nfeatures; i++) { virCPUx86FeaturePtr feature; -- 2.9.2

On 08/12/2016 09:33 AM, Jiri Denemark wrote: [1] [1] Coverity happlily points out that OPTIONAL is a DEAD_CODE path especially due to the prior check and continue; I see no other sane way around it other than a comment before the case: /* coverity[dead_error_condition] */ case VIR_CPU_FEATURE_OPTIONAL:

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/cpu/cpu_x86.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index dcf9c25..ed9b010 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -2652,8 +2652,11 @@ x86HasFeature(const virCPUData *data, return -1; if (!(feature = x86FeatureFind(map, name)) && - !(feature = x86FeatureFindInternal(name))) + !(feature = x86FeatureFindInternal(name))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown CPU feature %s"), name); goto cleanup; + } ret = x86DataIsSubset(&data->data.x86, &feature->data) ? 1 : 0; -- 2.9.2

The reworked API is now called virCPUUpdate and it should change the provided CPU definition into a one which can be consumed by QEMU command line builder: - host-passthrough remains unchanged - host-model is turned into custom CPU with a model and features copied from host - custom CPU with minimum match is converted similarly to host-model - optional features are updated according to host's CPU Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- po/POTFILES.in | 1 + src/cpu/cpu.c | 59 ++++-- src/cpu/cpu.h | 11 +- src/cpu/cpu_arm.c | 36 +++- src/cpu/cpu_ppc64.c | 32 ++-- src/cpu/cpu_x86.c | 212 ++++++++------------- src/libvirt_private.syms | 2 +- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_process.c | 2 +- tests/cputest.c | 14 +- .../cputestdata/x86-host+host-model-nofallback.xml | 2 +- tests/cputestdata/x86-host+host-model.xml | 2 +- .../x86-host+host-passthrough-features.xml | 4 + tests/cputestdata/x86-host+host-passthrough.xml | 19 +- tests/cputestdata/x86-host+min.xml | 27 +-- tests/cputestdata/x86-host+pentium3.xml | 39 ++-- tests/cputestdata/x86-host-invtsc+host-model.xml | 2 +- .../cputestdata/x86-host-passthrough-features.xml | 4 + 19 files changed, 227 insertions(+), 245 deletions(-) create mode 100644 tests/cputestdata/x86-host+host-passthrough-features.xml create mode 100644 tests/cputestdata/x86-host-passthrough-features.xml diff --git a/po/POTFILES.in b/po/POTFILES.in index 25dbc84..1469240 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -43,6 +43,7 @@ src/conf/virchrdev.c src/conf/virdomainobjlist.c src/conf/virsecretobj.c src/cpu/cpu.c +src/cpu/cpu_arm.c src/cpu/cpu_map.c src/cpu/cpu_ppc64.c src/cpu/cpu_x86.c diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index fae3885..f3eb211 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -579,38 +579,71 @@ cpuBaseline(virCPUDefPtr *cpus, /** - * cpuUpdate: + * virCPUUpdate: * - * @guest: guest CPU definition + * @arch: CPU architecture + * @guest: guest CPU definition to be updated * @host: host CPU definition * * Updates @guest CPU definition according to @host CPU. This is required to - * support guest CPU definition which are relative to host CPU, such as CPUs - * with VIR_CPU_MODE_CUSTOM and optional features or VIR_CPU_MATCH_MINIMUM, or - * CPUs with non-custom mode (VIR_CPU_MODE_HOST_MODEL, - * VIR_CPU_MODE_HOST_PASSTHROUGH). + * support guest CPU definitions specified relatively to host CPU, such as + * CPUs with VIR_CPU_MODE_CUSTOM and optional features or + * VIR_CPU_MATCH_MINIMUM, or CPUs with VIR_CPU_MODE_HOST_MODEL. + * When the guest CPU was not specified relatively, the function does nothing + * and returns success. * * Returns 0 on success, -1 on error. */ int -cpuUpdate(virCPUDefPtr guest, - const virCPUDef *host) +virCPUUpdate(virArch arch, + virCPUDefPtr guest, + const virCPUDef *host) { struct cpuArchDriver *driver; - VIR_DEBUG("guest=%p, host=%p", guest, host); + VIR_DEBUG("arch=%s, guest=%p mode=%s model=%s, host=%p model=%s", + virArchToString(arch), guest, virCPUModeTypeToString(guest->mode), + NULLSTR(guest->model), host, NULLSTR(host ? host->model : NULL)); - if ((driver = cpuGetSubDriver(host->arch)) == NULL) + if (!(driver = cpuGetSubDriver(arch))) return -1; - if (driver->update == NULL) { + if (guest->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) + return 0; + + if (guest->mode == VIR_CPU_MODE_CUSTOM && + guest->match != VIR_CPU_MATCH_MINIMUM) { + size_t i; + bool optional = false; + + for (i = 0; i < guest->nfeatures; i++) { + if (guest->features[i].policy == VIR_CPU_FEATURE_OPTIONAL) { + optional = true; + break; + } + } + + if (!optional) + return 0; + } + + /* We get here if guest CPU is either + * - host-model + * - custom with minimum match + * - custom with optional features + */ + if (!driver->update) { virReportError(VIR_ERR_NO_SUPPORT, - _("cannot update guest CPU data for %s architecture"), + _("cannot update guest CPU for %s architecture"), virArchToString(host->arch)); return -1; } - return driver->update(guest, host); + if (driver->update(guest, host) < 0) + return -1; + + VIR_DEBUG("model=%s", NULLSTR(guest->model)); + return 0; } diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 422818e..f4bb51d 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -87,8 +87,8 @@ typedef virCPUDefPtr unsigned int flags); typedef int -(*cpuArchUpdate) (virCPUDefPtr guest, - const virCPUDef *host); +(*virCPUArchUpdate)(virCPUDefPtr guest, + const virCPUDef *host); typedef int (*cpuArchHasFeature) (const virCPUData *data, @@ -114,7 +114,7 @@ struct cpuArchDriver { cpuArchNodeData nodeData; cpuArchGuestData guestData; cpuArchBaseline baseline; - cpuArchUpdate update; + virCPUArchUpdate update; cpuArchHasFeature hasFeature; cpuArchDataFormat dataFormat; cpuArchDataParse dataParse; @@ -182,9 +182,10 @@ cpuBaseline (virCPUDefPtr *cpus, ATTRIBUTE_NONNULL(1); int -cpuUpdate (virCPUDefPtr guest, +virCPUUpdate(virArch arch, + virCPUDefPtr guest, const virCPUDef *host) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); int cpuHasFeature(const virCPUData *data, diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index a3aed6b..49da36e 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -43,15 +43,41 @@ armDataFree(virCPUDataPtr data) VIR_FREE(data); } + static int -armUpdate(virCPUDefPtr guest, - const virCPUDef *host) +virCPUarmUpdate(virCPUDefPtr guest, + const virCPUDef *host) { + int ret = -1; + virCPUDefPtr updated = NULL; + + if (guest->mode != VIR_CPU_MODE_HOST_MODEL) + return 0; + + if (!host) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unknown host CPU model")); + goto cleanup; + } + + if (!(updated = virCPUDefCopyWithoutModel(guest))) + goto cleanup; + + updated->mode = VIR_CPU_MODE_CUSTOM; + if (virCPUDefCopyModel(updated, host, true) < 0) + goto cleanup; + + virCPUDefMoveModel(guest, updated); + guest->mode = VIR_CPU_MODE_CUSTOM; guest->match = VIR_CPU_MATCH_EXACT; - virCPUDefFreeModel(guest); - return virCPUDefCopyModel(guest, host, true); + ret = 0; + + cleanup: + virCPUDefFree(updated); + return ret; } + static virCPUCompareResult armGuestData(virCPUDefPtr host ATTRIBUTE_UNUSED, virCPUDefPtr guest ATTRIBUTE_UNUSED, @@ -104,6 +130,6 @@ struct cpuArchDriver cpuDriverArm = { .nodeData = NULL, .guestData = armGuestData, .baseline = armBaseline, - .update = armUpdate, + .update = virCPUarmUpdate, .hasFeature = NULL, }; diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index ad2d6a7..00faa22 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -751,27 +751,21 @@ ppc64DriverGuestData(virCPUDefPtr host, } static int -ppc64DriverUpdate(virCPUDefPtr guest, - const virCPUDef *host) +virCPUppc64Update(virCPUDefPtr guest, + const virCPUDef *host ATTRIBUTE_UNUSED) { - switch ((virCPUMode) guest->mode) { - case VIR_CPU_MODE_HOST_PASSTHROUGH: + /* + * - host-passthrough doesn't even get here + * - host-model is used for host CPU running in a compatibility mode and + * it needs to remain unchanged + * - custom doesn't support any optional features, there's nothing to + * update + */ + + if (guest->mode == VIR_CPU_MODE_CUSTOM) guest->match = VIR_CPU_MATCH_EXACT; - guest->fallback = VIR_CPU_FALLBACK_FORBID; - virCPUDefFreeModel(guest); - return virCPUDefCopyModel(guest, host, true); - case VIR_CPU_MODE_HOST_MODEL: - case VIR_CPU_MODE_CUSTOM: - return 0; - - case VIR_CPU_MODE_LAST: - break; - } - - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unexpected CPU mode: %d"), guest->mode); - return -1; + return 0; } static virCPUDefPtr @@ -915,7 +909,7 @@ struct cpuArchDriver cpuDriverPPC64 = { .nodeData = ppc64DriverNodeData, .guestData = ppc64DriverGuestData, .baseline = ppc64DriverBaseline, - .update = ppc64DriverUpdate, + .update = virCPUppc64Update, .hasFeature = NULL, .getModels = ppc64DriverGetModels, }; diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index e4bde79..746c804 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1121,40 +1121,6 @@ x86ModelFromCPU(const virCPUDef *cpu, } -static int -x86ModelSubtractCPU(virCPUx86ModelPtr model, - const virCPUDef *cpu, - virCPUx86MapPtr map) -{ - virCPUx86ModelPtr cpu_model; - size_t i; - - if (!(cpu_model = x86ModelFind(map, cpu->model))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unknown CPU model %s"), - cpu->model); - return -1; - } - - x86DataSubtract(&model->data, &cpu_model->data); - - for (i = 0; i < cpu->nfeatures; i++) { - virCPUx86FeaturePtr feature; - - if (!(feature = x86FeatureFind(map, cpu->features[i].name))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unknown CPU feature %s"), - cpu->features[i].name); - return -1; - } - - x86DataSubtract(&model->data, &feature->data); - } - - return 0; -} - - static virCPUx86CompareResult x86ModelCompare(virCPUx86ModelPtr model1, virCPUx86ModelPtr model2) @@ -2532,23 +2498,85 @@ x86Baseline(virCPUDefPtr *cpus, static int -x86UpdateCustom(virCPUDefPtr guest, +x86UpdateHostModel(virCPUDefPtr guest, + const virCPUDef *host, + virCPUx86MapPtr map) +{ + virCPUDefPtr updated = NULL; + size_t i; + int ret = -1; + + if (!(updated = virCPUDefCopyWithoutModel(host))) + goto cleanup; + + /* Remove non-migratable features by default */ + updated->type = VIR_CPU_TYPE_GUEST; + updated->mode = VIR_CPU_MODE_CUSTOM; + if (virCPUDefCopyModel(updated, host, true) < 0) + goto cleanup; + + i = 0; + while (i < updated->nfeatures) { + if (x86FeatureIsMigratable(updated->features[i].name, map) && + STRNEQ(updated->features[i].name, "cmt") && + STRNEQ(updated->features[i].name, "mbm_total") && + STRNEQ(updated->features[i].name, "mbm_local")) { + i++; + } else { + VIR_FREE(updated->features[i].name); + VIR_DELETE_ELEMENT_INPLACE(updated->features, i, updated->nfeatures); + } + } + + if (guest->vendor_id) { + VIR_FREE(updated->vendor_id); + if (VIR_STRDUP(updated->vendor_id, guest->vendor_id) < 0) + goto cleanup; + } + + for (i = 0; i < guest->nfeatures; i++) { + if (virCPUDefUpdateFeature(updated, + guest->features[i].name, + guest->features[i].policy) < 0) + goto cleanup; + } + + virCPUDefMoveModel(guest, updated); + guest->mode = VIR_CPU_MODE_CUSTOM; + guest->match = VIR_CPU_MATCH_EXACT; + ret = 0; + + cleanup: + virCPUDefFree(updated); + return ret; +} + + +static int +virCPUx86Update(virCPUDefPtr guest, const virCPUDef *host) { + virCPUx86ModelPtr model = NULL; + virCPUx86MapPtr map; int ret = -1; size_t i; - virCPUx86MapPtr map; - virCPUx86ModelPtr host_model = NULL; - if (!(map = virCPUx86GetMap()) || - !(host_model = x86ModelFromCPU(host, map, VIR_CPU_FEATURE_REQUIRE))) + if (!host) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("unknown host CPU model")); + return -1; + } + + if (!(map = virCPUx86GetMap())) + return -1; + + if (!(model = x86ModelFromCPU(host, map, -1))) goto cleanup; for (i = 0; i < guest->nfeatures; i++) { if (guest->features[i].policy == VIR_CPU_FEATURE_OPTIONAL) { int supported = x86FeatureInData(guest->features[i].name, - &host_model->data, map); - + &model->data, map); if (supported < 0) goto cleanup; else if (supported) @@ -2558,107 +2586,19 @@ x86UpdateCustom(virCPUDefPtr guest, } } - if (guest->match == VIR_CPU_MATCH_MINIMUM) { - guest->match = VIR_CPU_MATCH_EXACT; - if (x86ModelSubtractCPU(host_model, guest, map) || - x86DataToCPUFeatures(guest, VIR_CPU_FEATURE_REQUIRE, - &host_model->data, map)) - goto cleanup; - } - - ret = 0; + if (guest->mode == VIR_CPU_MODE_HOST_MODEL || + guest->match == VIR_CPU_MATCH_MINIMUM) + ret = x86UpdateHostModel(guest, host, map); + else + ret = 0; cleanup: - x86ModelFree(host_model); + x86ModelFree(model); return ret; } static int -x86UpdateHostModel(virCPUDefPtr guest, - const virCPUDef *host, - bool passthrough) -{ - virCPUDefPtr oldguest = NULL; - virCPUx86MapPtr map; - size_t i; - int ret = -1; - - if (!(map = virCPUx86GetMap())) - goto cleanup; - - /* update the host model according to the desired configuration */ - if (!(oldguest = virCPUDefCopy(guest))) - goto cleanup; - - virCPUDefFreeModel(guest); - if (virCPUDefCopyModel(guest, host, true) < 0) - goto cleanup; - - if (oldguest->vendor_id) { - VIR_FREE(guest->vendor_id); - if (VIR_STRDUP(guest->vendor_id, oldguest->vendor_id) < 0) - goto cleanup; - } - - /* Remove non-migratable features and CMT related features which QEMU - * knows nothing about. - * Note: this only works as long as no CPU model contains non-migratable - * features directly */ - i = 0; - while (i < guest->nfeatures) { - if (x86FeatureIsMigratable(guest->features[i].name, map) && - STRNEQ(guest->features[i].name, "cmt") && - STRNEQ(guest->features[i].name, "mbm_total") && - STRNEQ(guest->features[i].name, "mbm_local")) { - i++; - } else { - VIR_FREE(guest->features[i].name); - VIR_DELETE_ELEMENT_INPLACE(guest->features, i, guest->nfeatures); - } - } - for (i = 0; !passthrough && i < oldguest->nfeatures; i++) { - if (virCPUDefUpdateFeature(guest, - oldguest->features[i].name, - oldguest->features[i].policy) < 0) - goto cleanup; - } - - ret = 0; - - cleanup: - virCPUDefFree(oldguest); - return ret; -} - - -static int -x86Update(virCPUDefPtr guest, - const virCPUDef *host) -{ - switch ((virCPUMode) guest->mode) { - case VIR_CPU_MODE_CUSTOM: - return x86UpdateCustom(guest, host); - - case VIR_CPU_MODE_HOST_MODEL: - guest->match = VIR_CPU_MATCH_EXACT; - return x86UpdateHostModel(guest, host, false); - - case VIR_CPU_MODE_HOST_PASSTHROUGH: - guest->match = VIR_CPU_MATCH_MINIMUM; - return x86UpdateHostModel(guest, host, true); - - case VIR_CPU_MODE_LAST: - break; - } - - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unexpected CPU mode: %d"), guest->mode); - return -1; -} - - -static int x86HasFeature(const virCPUData *data, const char *name) { @@ -2715,7 +2655,7 @@ struct cpuArchDriver cpuDriverX86 = { #endif .guestData = x86GuestData, .baseline = x86Baseline, - .update = x86Update, + .update = virCPUx86Update, .hasFeature = x86HasFeature, .dataFormat = x86CPUDataFormat, .dataParse = x86CPUDataParse, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3a33956..ef04ce4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -971,7 +971,7 @@ cpuGetModels; cpuGuestData; cpuHasFeature; cpuNodeData; -cpuUpdate; +virCPUUpdate; # cpu/cpu_x86.h diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 823b58d..ce04c76 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6461,7 +6461,7 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr driver, if (cpu->mode == VIR_CPU_MODE_HOST_MODEL && !migrating && - cpuUpdate(cpu, host) < 0) + virCPUUpdate(def->os.arch, cpu, host) < 0) goto cleanup; if (compareAgainstHost && diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9ba7cbd..d8c589e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3275,7 +3275,7 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver, goto cleanup; } - if (cpuUpdate(def->cpu, caps->host.cpu) < 0) + if (virCPUUpdate(def->os.arch, def->cpu, caps->host.cpu) < 0) goto cleanup; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ae94c21..662a46a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4440,7 +4440,7 @@ qemuProcessStartValidateGuestCPU(virDomainObjPtr vm, if (cpu->mode == VIR_CPU_MODE_HOST_MODEL && flags & VIR_QEMU_PROCESS_START_NEW && - cpuUpdate(cpu, host) < 0) + virCPUUpdate(vm->def->os.arch, cpu, host) < 0) goto cleanup; cmp = cpuGuestData(host, cpu, &data, &compare_msg); diff --git a/tests/cputest.c b/tests/cputest.c index ee779fb..e6696e9 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -398,7 +398,7 @@ cpuTestUpdate(const void *arg) !(cpu = cpuTestLoadXML(data->arch, data->name))) goto cleanup; - if (cpuUpdate(cpu, host) < 0) + if (virCPUUpdate(host->arch, cpu, host) < 0) goto cleanup; if (virAsprintf(&result, "%s+%s", data->host, data->name) < 0) @@ -622,11 +622,14 @@ mymain(void) host "/" cpu " (" #result ")", \ host, cpu, NULL, 0, NULL, 0, result) +#define DO_TEST_UPDATE_ONLY(arch, host, cpu) \ + DO_TEST(arch, cpuTestUpdate, \ + cpu " on " host, \ + host, cpu, NULL, 0, NULL, 0, 0) \ + #define DO_TEST_UPDATE(arch, host, cpu, result) \ do { \ - DO_TEST(arch, cpuTestUpdate, \ - cpu " on " host, \ - host, cpu, NULL, 0, NULL, 0, 0); \ + DO_TEST_UPDATE_ONLY(arch, host, cpu); \ DO_TEST_COMPARE(arch, host, host "+" cpu, result); \ } while (0) @@ -737,8 +740,9 @@ mymain(void) DO_TEST_UPDATE("x86", "host", "guest", VIR_CPU_COMPARE_SUPERSET); DO_TEST_UPDATE("x86", "host", "host-model", VIR_CPU_COMPARE_IDENTICAL); DO_TEST_UPDATE("x86", "host", "host-model-nofallback", VIR_CPU_COMPARE_IDENTICAL); - DO_TEST_UPDATE("x86", "host", "host-passthrough", VIR_CPU_COMPARE_IDENTICAL); DO_TEST_UPDATE("x86", "host-invtsc", "host-model", VIR_CPU_COMPARE_SUPERSET); + DO_TEST_UPDATE_ONLY("x86", "host", "host-passthrough"); + DO_TEST_UPDATE_ONLY("x86", "host", "host-passthrough-features"); DO_TEST_UPDATE("ppc64", "host", "guest", VIR_CPU_COMPARE_IDENTICAL); DO_TEST_UPDATE("ppc64", "host", "guest-nofallback", VIR_CPU_COMPARE_INCOMPATIBLE); diff --git a/tests/cputestdata/x86-host+host-model-nofallback.xml b/tests/cputestdata/x86-host+host-model-nofallback.xml index bbb68fb..0c3ede0 100644 --- a/tests/cputestdata/x86-host+host-model-nofallback.xml +++ b/tests/cputestdata/x86-host+host-model-nofallback.xml @@ -1,4 +1,4 @@ -<cpu mode='host-model' match='exact'> +<cpu mode='custom' match='exact'> <model fallback='forbid'>Penryn</model> <vendor>Intel</vendor> <topology sockets='1' cores='2' threads='1'/> diff --git a/tests/cputestdata/x86-host+host-model.xml b/tests/cputestdata/x86-host+host-model.xml index c1014e2..a284767 100644 --- a/tests/cputestdata/x86-host+host-model.xml +++ b/tests/cputestdata/x86-host+host-model.xml @@ -1,4 +1,4 @@ -<cpu mode='host-model' match='exact'> +<cpu mode='custom' match='exact'> <model fallback='allow'>Penryn</model> <vendor>Intel</vendor> <feature policy='require' name='dca'/> diff --git a/tests/cputestdata/x86-host+host-passthrough-features.xml b/tests/cputestdata/x86-host+host-passthrough-features.xml new file mode 100644 index 0000000..dc2b775 --- /dev/null +++ b/tests/cputestdata/x86-host+host-passthrough-features.xml @@ -0,0 +1,4 @@ +<cpu mode='host-passthrough'> + <feature policy='disable' name='dca'/> + <feature policy='force' name='vmx'/> +</cpu> diff --git a/tests/cputestdata/x86-host+host-passthrough.xml b/tests/cputestdata/x86-host+host-passthrough.xml index 721fae5..655c7a7 100644 --- a/tests/cputestdata/x86-host+host-passthrough.xml +++ b/tests/cputestdata/x86-host+host-passthrough.xml @@ -1,18 +1 @@ -<cpu mode='host-passthrough' match='minimum'> - <model>Penryn</model> - <vendor>Intel</vendor> - <feature policy='require' name='dca'/> - <feature policy='require' name='xtpr'/> - <feature policy='require' name='tm2'/> - <feature policy='require' name='est'/> - <feature policy='require' name='vmx'/> - <feature policy='require' name='ds_cpl'/> - <feature policy='require' name='monitor'/> - <feature policy='require' name='pbe'/> - <feature policy='require' name='tm'/> - <feature policy='require' name='ht'/> - <feature policy='require' name='ss'/> - <feature policy='require' name='acpi'/> - <feature policy='require' name='ds'/> - <feature policy='require' name='vme'/> -</cpu> +<cpu mode='host-passthrough'/> diff --git a/tests/cputestdata/x86-host+min.xml b/tests/cputestdata/x86-host+min.xml index 6d2d5cd..a284767 100644 --- a/tests/cputestdata/x86-host+min.xml +++ b/tests/cputestdata/x86-host+min.xml @@ -1,17 +1,18 @@ <cpu mode='custom' match='exact'> <model fallback='allow'>Penryn</model> - <feature policy='require' name='vme'/> - <feature policy='require' name='ds'/> - <feature policy='require' name='acpi'/> - <feature policy='require' name='ss'/> - <feature policy='require' name='ht'/> - <feature policy='require' name='tm'/> - <feature policy='require' name='pbe'/> - <feature policy='require' name='monitor'/> - <feature policy='require' name='ds_cpl'/> - <feature policy='require' name='vmx'/> - <feature policy='require' name='est'/> - <feature policy='require' name='tm2'/> - <feature policy='require' name='xtpr'/> + <vendor>Intel</vendor> <feature policy='require' name='dca'/> + <feature policy='require' name='xtpr'/> + <feature policy='require' name='tm2'/> + <feature policy='require' name='est'/> + <feature policy='require' name='vmx'/> + <feature policy='require' name='ds_cpl'/> + <feature policy='require' name='monitor'/> + <feature policy='require' name='pbe'/> + <feature policy='require' name='tm'/> + <feature policy='require' name='ht'/> + <feature policy='require' name='ss'/> + <feature policy='require' name='acpi'/> + <feature policy='require' name='ds'/> + <feature policy='require' name='vme'/> </cpu> diff --git a/tests/cputestdata/x86-host+pentium3.xml b/tests/cputestdata/x86-host+pentium3.xml index a8c15f4..a284767 100644 --- a/tests/cputestdata/x86-host+pentium3.xml +++ b/tests/cputestdata/x86-host+pentium3.xml @@ -1,27 +1,18 @@ <cpu mode='custom' match='exact'> - <model fallback='allow'>pentium3</model> - <feature policy='require' name='apic'/> - <feature policy='require' name='clflush'/> - <feature policy='require' name='ds'/> - <feature policy='require' name='acpi'/> - <feature policy='require' name='sse2'/> - <feature policy='require' name='ss'/> - <feature policy='require' name='ht'/> - <feature policy='require' name='tm'/> - <feature policy='require' name='pbe'/> - <feature policy='require' name='pni'/> - <feature policy='require' name='monitor'/> - <feature policy='require' name='ds_cpl'/> - <feature policy='require' name='vmx'/> - <feature policy='require' name='est'/> - <feature policy='require' name='tm2'/> - <feature policy='require' name='ssse3'/> - <feature policy='require' name='cx16'/> - <feature policy='require' name='xtpr'/> + <model fallback='allow'>Penryn</model> + <vendor>Intel</vendor> <feature policy='require' name='dca'/> - <feature policy='require' name='sse4.1'/> - <feature policy='require' name='syscall'/> - <feature policy='require' name='nx'/> - <feature policy='require' name='lm'/> - <feature policy='require' name='lahf_lm'/> + <feature policy='require' name='xtpr'/> + <feature policy='require' name='tm2'/> + <feature policy='require' name='est'/> + <feature policy='require' name='vmx'/> + <feature policy='require' name='ds_cpl'/> + <feature policy='require' name='monitor'/> + <feature policy='require' name='pbe'/> + <feature policy='require' name='tm'/> + <feature policy='require' name='ht'/> + <feature policy='require' name='ss'/> + <feature policy='require' name='acpi'/> + <feature policy='require' name='ds'/> + <feature policy='require' name='vme'/> </cpu> diff --git a/tests/cputestdata/x86-host-invtsc+host-model.xml b/tests/cputestdata/x86-host-invtsc+host-model.xml index ad1bbf8..998ed23 100644 --- a/tests/cputestdata/x86-host-invtsc+host-model.xml +++ b/tests/cputestdata/x86-host-invtsc+host-model.xml @@ -1,4 +1,4 @@ -<cpu mode='host-model' match='exact'> +<cpu mode='custom' match='exact'> <model fallback='allow'>SandyBridge</model> <vendor>Intel</vendor> <feature policy='require' name='osxsave'/> diff --git a/tests/cputestdata/x86-host-passthrough-features.xml b/tests/cputestdata/x86-host-passthrough-features.xml new file mode 100644 index 0000000..dc2b775 --- /dev/null +++ b/tests/cputestdata/x86-host-passthrough-features.xml @@ -0,0 +1,4 @@ +<cpu mode='host-passthrough'> + <feature policy='disable' name='dca'/> + <feature policy='force' name='vmx'/> +</cpu> -- 2.9.2

On Tue, Aug 30, 2016 at 17:53:56 -0400, John Ferlan wrote: Fixed. ... Not really, I think the vir prefix is just used when someone refactors an old code. AFAIK the goal (not enforced in any way) was to eventually use the prefix for all functions. Anyway, see virQEMUCaps* is good example of such functions/types that are not in src/util. ... Yes, it is possible to enter virCPUUpdate with host == NULL. virQEMUCapsIsCPUModeSupported checks hostCPUModel only for VIR_CPU_MODE_HOST_MODEL, but virCPUUpdate is called for all CPU definitions. It may seem we do not call virCPUUpdate if hostCPUModel is NULL since the code is still not in its final shape after this patch (until patch 40 is applied). Yes, fixed. ... Oops, yes, I forgot to change this one when adding an explicit arch parameter. It depends on the guest CPU definition, sometimes host == NULL is OK, sometimes it is not. And the update functions detect whether they need host CPU model for updating the CPU and report an error. I think I mentioned it somewhere in the cover letter or maybe I just wanted to do that, but the goal is to completely rework all APIs provided by the cpu driver. Some of them will just change their names to virCPU*, some of them will be completely removed, and several new APIs will be added. So the new name also serves as a way to distinguish reworked/new APIs from those that still needs to be rewritten or removed. Done. Jirka

The API is supposed to make sure the provided CPU definition does not use a CPU model which is not supported by the hypervisor (if at all possible, of course). Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/cpu/cpu.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++ src/cpu/cpu.h | 14 ++++++++++++ src/cpu/cpu_x86.c | 51 +++++++++++++++++++++++++++++++++++++++++ src/libvirt_private.syms | 1 + 4 files changed, 125 insertions(+) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index f3eb211..e215304 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -809,3 +809,62 @@ cpuGetModels(virArch arch, char ***models) return driver->getModels(models); } + + +/** + * virCPUTranslate: + * + * @arch: CPU architecture + * @cpu: CPU definition to be translated + * @models: NULL-terminated list of allowed CPU models (NULL if all are allowed) + * @nmodels: number of CPU models in @models + * + * Translates @cpu model (if allowed by @cpu->fallback) to a closest CPU model + * from @models list. + * + * The function does nothing (and returns 0) if @cpu does not have to be + * translated. + * + * Returns -1 on error, 0 on success. + */ +int +virCPUTranslate(virArch arch, + virCPUDefPtr cpu, + char **models, + unsigned int nmodels) +{ + struct cpuArchDriver *driver; + + VIR_DEBUG("arch=%s, cpu=%p, model=%s, models=%p, nmodels=%u", + virArchToString(arch), cpu, NULLSTR(cpu->model), models, nmodels); + + if (!(driver = cpuGetSubDriver(arch))) + return -1; + + if (cpu->mode == VIR_CPU_MODE_HOST_MODEL || + cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) + return 0; + + if (cpuModelIsAllowed(cpu->model, (const char **) models, nmodels)) + return 0; + + if (cpu->fallback != VIR_CPU_FALLBACK_ALLOW) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("CPU model %s is not supported by hypervisor"), + cpu->model); + return -1; + } + + if (!driver->translate) { + virReportError(VIR_ERR_NO_SUPPORT, + _("cannot translate CPU model %s to a supported model"), + cpu->model); + return -1; + } + + if (driver->translate(cpu, (const char **) models, nmodels) < 0) + return -1; + + VIR_DEBUG("model=%s", NULLSTR(cpu->model)); + return 0; +} diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index f4bb51d..b8036b2 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -103,6 +103,11 @@ typedef virCPUDataPtr typedef int (*cpuArchGetModels) (char ***models); +typedef int +(*virCPUArchTranslate)(virCPUDefPtr cpu, + const char **models, + unsigned int nmodels); + struct cpuArchDriver { const char *name; const virArch *arch; @@ -119,6 +124,7 @@ struct cpuArchDriver { cpuArchDataFormat dataFormat; cpuArchDataParse dataParse; cpuArchGetModels getModels; + virCPUArchTranslate translate; }; @@ -202,6 +208,14 @@ cpuModelIsAllowed(const char *model, int cpuGetModels(virArch arch, char ***models); +int +virCPUTranslate(virArch arch, + virCPUDefPtr cpu, + char **models, + unsigned int nmodels) + ATTRIBUTE_NONNULL(2); + + /* cpuDataFormat and cpuDataParse are implemented for unit tests only and * have no real-life usage */ diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index c867f83..69b081d 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -2642,6 +2642,56 @@ x86GetModels(char ***models) } +static int +virCPUx86Translate(virCPUDefPtr cpu, + const char **models, + unsigned int nmodels) +{ + virCPUDefPtr translated = NULL; + virCPUx86MapPtr map; + virCPUx86ModelPtr model = NULL; + size_t i; + int ret = -1; + + if (!(map = virCPUx86GetMap())) + goto cleanup; + + if (!(model = x86ModelFromCPU(cpu, map, -1))) + goto cleanup; + + if (model->vendor && + virCPUx86DataAddCPUID(&model->data, &model->vendor->cpuid) < 0) + goto cleanup; + + if (x86DataAddSignature(&model->data, model->signature) < 0) + goto cleanup; + + if (!(translated = virCPUDefCopyWithoutModel(cpu))) + goto cleanup; + + if (VIR_STRDUP(translated->vendor, cpu->vendor) < 0 || + VIR_STRDUP(translated->vendor_id, cpu->vendor_id) < 0) + goto cleanup; + + if (x86Decode(translated, &model->data, models, nmodels, NULL, 0) < 0) + goto cleanup; + + for (i = 0; i < cpu->nfeatures; i++) { + virCPUFeatureDefPtr f = cpu->features + i; + if (virCPUDefUpdateFeature(translated, f->name, f->policy) < 0) + goto cleanup; + } + + virCPUDefMoveModel(cpu, translated); + ret = 0; + + cleanup: + virCPUDefFree(translated); + x86ModelFree(model); + return ret; +} + + struct cpuArchDriver cpuDriverX86 = { .name = "x86", .arch = archs, @@ -2662,4 +2712,5 @@ struct cpuArchDriver cpuDriverX86 = { .dataFormat = x86CPUDataFormat, .dataParse = x86CPUDataParse, .getModels = x86GetModels, + .translate = virCPUx86Translate, }; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ef04ce4..1f50499 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -971,6 +971,7 @@ cpuGetModels; cpuGuestData; cpuHasFeature; cpuNodeData; +virCPUTranslate; virCPUUpdate; -- 2.9.2

Both cpuCompare* APIs are renamed to virCPUCompare*. And they should now work for any guest CPU definition, i.e., even for host-passthrough (trivial) and host-model CPUs. The implementation in x86 driver is enhanced to provide a hint about -noTSX Broadwell and Haswell models when appropriate. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/cpu/cpu.c | 37 ++++++++++++--------- src/cpu/cpu.h | 21 ++++++------ src/cpu/cpu_arm.c | 8 ++--- src/cpu/cpu_ppc64.c | 15 +++++++-- src/cpu/cpu_x86.c | 84 ++++++++++++++++++++++++++++++++++++------------ src/libvirt_private.syms | 4 +-- src/qemu/qemu_driver.c | 14 ++------ tests/cputest.c | 4 +-- 8 files changed, 119 insertions(+), 68 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index d6b0372..4ea192c 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -91,8 +91,9 @@ cpuGetSubDriverByName(const char *name) /** - * cpuCompareXML: + * virCPUCompareXML: * + * @arch: CPU architecture * @host: host CPU definition * @xml: XML description of either guest or host CPU to be compared with @host * @@ -104,25 +105,26 @@ cpuGetSubDriverByName(const char *name) * the @host CPU. */ virCPUCompareResult -cpuCompareXML(virCPUDefPtr host, - const char *xml, - bool failIncompatible) +virCPUCompareXML(virArch arch, + virCPUDefPtr host, + const char *xml, + bool failIncompatible) { xmlDocPtr doc = NULL; xmlXPathContextPtr ctxt = NULL; virCPUDefPtr cpu = NULL; virCPUCompareResult ret = VIR_CPU_COMPARE_ERROR; - VIR_DEBUG("host=%p, xml=%s", host, NULLSTR(xml)); + VIR_DEBUG("arch=%s, host=%p, xml=%s", + virArchToString(arch), host, NULLSTR(xml)); if (!(doc = virXMLParseStringCtxt(xml, _("(CPU_definition)"), &ctxt))) goto cleanup; - cpu = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_AUTO); - if (cpu == NULL) + if (!(cpu = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_AUTO))) goto cleanup; - ret = cpuCompare(host, cpu, failIncompatible); + ret = virCPUCompare(arch, host, cpu, failIncompatible); cleanup: virCPUDefFree(cpu); @@ -134,8 +136,9 @@ cpuCompareXML(virCPUDefPtr host, /** - * cpuCompare: + * virCPUCompare: * + * @arch: CPU architecture * @host: host CPU definition * @cpu: either guest or host CPU to be compared with @host * @@ -147,21 +150,23 @@ cpuCompareXML(virCPUDefPtr host, * the @host CPU. */ virCPUCompareResult -cpuCompare(virCPUDefPtr host, - virCPUDefPtr cpu, - bool failIncompatible) +virCPUCompare(virArch arch, + virCPUDefPtr host, + virCPUDefPtr cpu, + bool failIncompatible) { struct cpuArchDriver *driver; - VIR_DEBUG("host=%p, cpu=%p", host, cpu); + VIR_DEBUG("arch=%s, host=%p, cpu=%p", + virArchToString(arch), host, cpu); - if ((driver = cpuGetSubDriver(host->arch)) == NULL) + if (!(driver = cpuGetSubDriver(arch))) return VIR_CPU_COMPARE_ERROR; - if (driver->compare == NULL) { + if (!driver->compare) { virReportError(VIR_ERR_NO_SUPPORT, _("cannot compare CPUs of %s architecture"), - virArchToString(host->arch)); + virArchToString(arch)); return VIR_CPU_COMPARE_ERROR; } diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index 77ccb38..0e81e91 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -45,7 +45,7 @@ struct _virCPUData { typedef virCPUCompareResult -(*cpuArchCompare) (virCPUDefPtr host, +(*virCPUArchCompare)(virCPUDefPtr host, virCPUDefPtr cpu, bool failIncompatible); @@ -116,7 +116,7 @@ struct cpuArchDriver { const char *name; const virArch *arch; unsigned int narch; - cpuArchCompare compare; + virCPUArchCompare compare; cpuArchDecode decode; cpuArchEncode encode; cpuArchDataFree free; @@ -134,16 +134,17 @@ struct cpuArchDriver { virCPUCompareResult -cpuCompareXML(virCPUDefPtr host, - const char *xml, - bool failIncompatible) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +virCPUCompareXML(virArch arch, + virCPUDefPtr host, + const char *xml, + bool failIncompatible); virCPUCompareResult -cpuCompare (virCPUDefPtr host, - virCPUDefPtr cpu, - bool failIncompatible) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +virCPUCompare(virArch arch, + virCPUDefPtr host, + virCPUDefPtr cpu, + bool failIncompatible) + ATTRIBUTE_NONNULL(3); int cpuDecode (virCPUDefPtr cpu, diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c index 3b68d83..2cef5bc 100644 --- a/src/cpu/cpu_arm.c +++ b/src/cpu/cpu_arm.c @@ -112,9 +112,9 @@ armBaseline(virCPUDefPtr *cpus, } static virCPUCompareResult -armCompare(virCPUDefPtr host ATTRIBUTE_UNUSED, - virCPUDefPtr cpu ATTRIBUTE_UNUSED, - bool failMessages ATTRIBUTE_UNUSED) +virCPUarmCompare(virCPUDefPtr host ATTRIBUTE_UNUSED, + virCPUDefPtr cpu ATTRIBUTE_UNUSED, + bool failMessages ATTRIBUTE_UNUSED) { return VIR_CPU_COMPARE_IDENTICAL; } @@ -123,7 +123,7 @@ struct cpuArchDriver cpuDriverArm = { .name = "arm", .arch = archs, .narch = ARRAY_CARDINALITY(archs), - .compare = armCompare, + .compare = virCPUarmCompare, .decode = NULL, .encode = NULL, .free = armDataFree, diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 6f005e5..21ea0e1 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -634,13 +634,24 @@ ppc64Compute(virCPUDefPtr host, } static virCPUCompareResult -ppc64DriverCompare(virCPUDefPtr host, +virCPUppc64Compare(virCPUDefPtr host, virCPUDefPtr cpu, bool failIncompatible) { virCPUCompareResult ret; char *message = NULL; + if (!host || !host->model) { + if (failIncompatible) { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", + _("unknown host CPU")); + } else { + VIR_WARN("unknown host CPU"); + ret = VIR_CPU_COMPARE_INCOMPATIBLE; + } + return -1; + } + ret = ppc64Compute(host, cpu, NULL, &message); if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) { @@ -902,7 +913,7 @@ struct cpuArchDriver cpuDriverPPC64 = { .name = "ppc64", .arch = archs, .narch = ARRAY_CARDINALITY(archs), - .compare = ppc64DriverCompare, + .compare = virCPUppc64Compare, .decode = ppc64DriverDecode, .encode = NULL, .free = ppc64DriverFree, diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 782c917..e0987c4 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1057,7 +1057,8 @@ x86ModelFromCPU(const virCPUDef *cpu, policy != -1) return x86ModelNew(); - if (policy == VIR_CPU_FEATURE_REQUIRE || policy == -1) { + if (cpu->model && + (policy == VIR_CPU_FEATURE_REQUIRE || policy == -1)) { if (!(model = x86ModelFind(map, cpu->model))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown CPU model %s"), cpu->model); @@ -1518,12 +1519,6 @@ x86Compute(virCPUDefPtr host, virArch arch; size_t i; - if (!cpu->model) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("no guest CPU model specified")); - return VIR_CPU_COMPARE_ERROR; - } - if (cpu->arch != VIR_ARCH_NONE) { bool found = false; @@ -1564,7 +1559,7 @@ x86Compute(virCPUDefPtr host, } if (!(map = virCPUx86GetMap()) || - !(host_model = x86ModelFromCPU(host, map, VIR_CPU_FEATURE_REQUIRE)) || + !(host_model = x86ModelFromCPU(host, map, -1)) || !(cpu_force = x86ModelFromCPU(cpu, map, VIR_CPU_FEATURE_FORCE)) || !(cpu_require = x86ModelFromCPU(cpu, map, VIR_CPU_FEATURE_REQUIRE)) || !(cpu_optional = x86ModelFromCPU(cpu, map, VIR_CPU_FEATURE_OPTIONAL)) || @@ -1663,25 +1658,68 @@ x86Compute(virCPUDefPtr host, static virCPUCompareResult -x86Compare(virCPUDefPtr host, - virCPUDefPtr cpu, - bool failIncompatible) +virCPUx86Compare(virCPUDefPtr host, + virCPUDefPtr cpu, + bool failIncompatible) { - virCPUCompareResult ret; + virCPUCompareResult ret = VIR_CPU_COMPARE_ERROR; + virCPUx86MapPtr map; + virCPUx86ModelPtr model = NULL; char *message = NULL; + if (!host || !host->model) { + if (failIncompatible) { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", + _("unknown host CPU")); + } else { + VIR_WARN("unknown host CPU"); + ret = VIR_CPU_COMPARE_INCOMPATIBLE; + } + goto cleanup; + } + ret = x86Compute(host, cpu, NULL, &message); - if (failIncompatible && ret == VIR_CPU_COMPARE_INCOMPATIBLE) { - ret = VIR_CPU_COMPARE_ERROR; - if (message) { - virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", message); - } else { - virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL); + if (ret == VIR_CPU_COMPARE_INCOMPATIBLE) { + bool noTSX = false; + + if (STREQ_NULLABLE(cpu->model, "Haswell") || + STREQ_NULLABLE(cpu->model, "Broadwell")) { + if (!(map = virCPUx86GetMap())) + goto cleanup; + + if (!(model = x86ModelFromCPU(cpu, map, -1))) + goto cleanup; + + noTSX = !x86FeatureInData("hle", &model->data, map) || + !x86FeatureInData("rtm", &model->data, map); + } + + if (failIncompatible) { + ret = VIR_CPU_COMPARE_ERROR; + if (message) { + if (noTSX) { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, + _("%s; try using '%s-noTSX' CPU model"), + message, cpu->model); + } else { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", message); + } + } else { + if (noTSX) { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, + _("try using '%s-noTSX' CPU model"), + cpu->model); + } else { + virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL); + } + } } } - VIR_FREE(message); + cleanup: + VIR_FREE(message); + x86ModelFree(model); return ret; } @@ -1692,6 +1730,12 @@ x86GuestData(virCPUDefPtr host, virCPUDataPtr *data, char **message) { + if (!guest->model) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("no guest CPU model specified")); + return VIR_CPU_COMPARE_ERROR; + } + return x86Compute(host, guest, data, message); } @@ -2719,7 +2763,7 @@ struct cpuArchDriver cpuDriverX86 = { .name = "x86", .arch = archs, .narch = ARRAY_CARDINALITY(archs), - .compare = x86Compare, + .compare = virCPUx86Compare, .decode = x86DecodeCPUData, .encode = x86Encode, .free = x86FreeCPUData, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 15aac81..5ed90d5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -960,8 +960,6 @@ virSecretObjSetValueSize; # cpu/cpu.h cpuBaseline; cpuBaselineXML; -cpuCompare; -cpuCompareXML; cpuDataFormat; cpuDataFree; cpuDataParse; @@ -971,6 +969,8 @@ cpuGetModels; cpuGuestData; cpuNodeData; virCPUCheckFeature; +virCPUCompare; +virCPUCompareXML; virCPUDataCheckFeature; virCPUTranslate; virCPUUpdate; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 695da14..f035ea1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12702,18 +12702,8 @@ qemuConnectCompareCPU(virConnectPtr conn, if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; - if (!caps->host.cpu || - !caps->host.cpu->model) { - if (failIncompatible) { - virReportError(VIR_ERR_CPU_INCOMPATIBLE, "%s", - _("cannot get host CPU capabilities")); - } else { - VIR_WARN("cannot get host CPU capabilities"); - ret = VIR_CPU_COMPARE_INCOMPATIBLE; - } - } else { - ret = cpuCompareXML(caps->host.cpu, xmlDesc, failIncompatible); - } + ret = virCPUCompareXML(caps->host.arch, caps->host.cpu, + xmlDesc, failIncompatible); cleanup: virObjectUnref(caps); diff --git a/tests/cputest.c b/tests/cputest.c index 72f00b6..6842d27 100644 --- a/tests/cputest.c +++ b/tests/cputest.c @@ -215,7 +215,7 @@ cpuTestCompare(const void *arg) !(cpu = cpuTestLoadXML(data->arch, data->name))) goto cleanup; - result = cpuCompare(host, cpu, false); + result = virCPUCompare(host->arch, host, cpu, false); if (data->result == VIR_CPU_COMPARE_ERROR) virResetLastError(); @@ -360,7 +360,7 @@ cpuTestBaseline(const void *arg) for (i = 0; i < ncpus; i++) { virCPUCompareResult cmp; - cmp = cpuCompare(cpus[i], baseline, false); + cmp = virCPUCompare(cpus[i]->arch, cpus[i], baseline, false); if (cmp != VIR_CPU_COMPARE_SUPERSET && cmp != VIR_CPU_COMPARE_IDENTICAL) { VIR_TEST_VERBOSE("\nbaseline CPU is incompatible with CPU %zu\n", -- 2.9.2

On Tue, Aug 30, 2016 at 17:57:01 -0400, John Ferlan wrote: ... I added a separate patch to fix this. host == NULL is definitely allowed by design, since we want to be able to successfully compare CPUs on ARM where we are not able to detect the host CPU model. I'm not exactly sure about the xml argument, but since NULLSTR(xml) was already here I figured this is a good place to return an error for xml == NULL rather than forcing all callers to do this check. However I apparently didn't implement the check :-) I added an explicit check for xml == NULL. Yes, although it will partially change in the future. ... Yup, it was done by design. ... Yes, it's related to this patch and required to do what commit message says: "Both cpuCompare* APIs are renamed to virCPUCompare*. And they should now work for any guest CPU definition, i.e., even for host-passthrough (trivial) and host-model CPUs." Thanks to this x86ModelFromCPU() may now be called for CPU definitions without a model, e.g., CPUs with host-model mode. Jirka

On 08/12/2016 09:33 AM, Jiri Denemark wrote: This is a nice reduction! So much more understandable. [...] It would seem this could be related to an earlier patch - that is the cpuUpdate patch... It can stay here, but it just feels like it could move. [...] So this will fail for mode == VIR_CPU_MODE_HOST_MODEL if ->cpuModel wasn't filled in, but it's just not clear enough to me (after 40 patches) whether the following checks could return NULL for the GetHostModel. IIRC none of the failures are cores, just reported errors, which is good and I supposed would be expected by this point in time! ^^^^^^^^^^^^^^^^^^^^^^^ This could legally be NULL and thus cause failures in the various compare function callbacks. Same here for update callbacks [...]

It really doesn't belong to the generic CPU driver. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/cpu/cpu_x86.c | 16 ++-------------- src/qemu/qemu_capabilities.c | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index e0987c4..237fa40 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -2558,22 +2558,10 @@ x86UpdateHostModel(virCPUDefPtr guest, /* Remove non-migratable features by default */ updated->type = VIR_CPU_TYPE_GUEST; updated->mode = VIR_CPU_MODE_CUSTOM; - if (virCPUDefCopyModel(updated, host, true) < 0) + if (virCPUDefCopyModelFilter(updated, host, true, + x86FeatureIsMigratable, map) < 0) goto cleanup; - i = 0; - while (i < updated->nfeatures) { - if (x86FeatureIsMigratable(updated->features[i].name, map) && - STRNEQ(updated->features[i].name, "cmt") && - STRNEQ(updated->features[i].name, "mbm_total") && - STRNEQ(updated->features[i].name, "mbm_local")) { - i++; - } else { - VIR_FREE(updated->features[i].name); - VIR_DELETE_ELEMENT_INPLACE(updated->features, i, updated->nfeatures); - } - } - if (guest->vendor_id) { VIR_FREE(updated->vendor_id); if (VIR_STRDUP(updated->vendor_id, guest->vendor_id) < 0) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f70a36c..187d4c1 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2859,6 +2859,19 @@ int virQEMUCapsProbeQMP(virQEMUCapsPtr qemuCaps, } +static bool +virQEMUCapsCPUFilterFeatures(const char *name, + void *opaque ATTRIBUTE_UNUSED) +{ + if (STREQ(name, "cmt") || + STREQ(name, "mbm_total") || + STREQ(name, "mbm_local")) + return false; + + return true; +} + + void virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, virCapsHostPtr host) @@ -2877,7 +2890,8 @@ virQEMUCapsInitHostCPUModel(virQEMUCapsPtr qemuCaps, cpu->mode = VIR_CPU_MODE_CUSTOM; cpu->match = VIR_CPU_MATCH_EXACT; - if (virCPUDefCopyModel(cpu, host->cpu, true) < 0) + if (virCPUDefCopyModelFilter(cpu, host->cpu, true, + virQEMUCapsCPUFilterFeatures, NULL) < 0) goto error; } -- 2.9.2