Has anybody had a chance to review this patch?
On Fri, Aug 02, 2013 at 01:08:19PM -0600, n0ano wrote:
Currently the virConnectBaselineCPU API does not expose the CPU features
that are part of the CPU's model. This patch adds a new flag,
VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, that causes the API to explictly
list all features that are part of that model.
Signed-off-by: Don Dugger <donald.d.dugger(a)intel.com>
---
[V2 - per review comments change the flag name to be more descriptive
and change errors from warnings to propogated errors.]
[V3 - Incorporate review suggestions to better handle flags and errors.
Also add documentation about the API change.]
include/libvirt/libvirt.h.in | 9 ++++++
src/cpu/cpu.c | 12 ++++---
src/cpu/cpu.h | 12 ++++---
src/cpu/cpu_arm.c | 6 +++-
src/cpu/cpu_generic.c | 5 ++-
src/cpu/cpu_powerpc.c | 10 ++++--
src/cpu/cpu_s390.c | 6 +++-
src/cpu/cpu_x86.c | 45 ++++++++++++++++++++++++---
src/libvirt.c | 7 ++++-
src/qemu/qemu_driver.c | 4 +--
tests/cputest.c | 30 +++++++++---------
tests/cputestdata/x86-baseline-3-result.xml | 35 +++++++++++++++++++++
tests/cputestdata/x86-baseline-3.xml | 7 +++++
tools/virsh-domain.c | 10 +++++-
tools/virsh.pod | 7 +++--
15 files changed, 166 insertions(+), 39 deletions(-)
create mode 100644 tests/cputestdata/x86-baseline-3-result.xml
create mode 100644 tests/cputestdata/x86-baseline-3.xml
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 7bd3559..b843355 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -4008,6 +4008,15 @@ int virConnectCompareCPU(virConnectPtr conn,
/**
+ * virConnectBaselineCPUFlags
+ *
+ * Flags when getting XML description of a computed CPU
+ */
+typedef enum {
+ VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES = (1 << 0), /* show all features
*/
+} virConnectBaselineCPUFlags;
+
+/**
* virConnectBaselineCPU:
*
* @conn: virConnect connection
diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
index 4124354..023ce26 100644
--- a/src/cpu/cpu.c
+++ b/src/cpu/cpu.c
@@ -167,7 +167,7 @@ cpuDecode(virCPUDefPtr cpu,
return -1;
}
- return driver->decode(cpu, data, models, nmodels, preferred);
+ return driver->decode(cpu, data, models, nmodels, preferred, 0);
}
@@ -276,7 +276,8 @@ char *
cpuBaselineXML(const char **xmlCPUs,
unsigned int ncpus,
const char **models,
- unsigned int nmodels)
+ unsigned int nmodels,
+ unsigned int flags)
{
xmlDocPtr doc = NULL;
xmlXPathContextPtr ctxt = NULL;
@@ -323,7 +324,7 @@ cpuBaselineXML(const char **xmlCPUs,
doc = NULL;
}
- if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels)))
+ if (!(cpu = cpuBaseline(cpus, ncpus, models, nmodels, flags)))
goto error;
cpustr = virCPUDefFormat(cpu, 0);
@@ -350,7 +351,8 @@ virCPUDefPtr
cpuBaseline(virCPUDefPtr *cpus,
unsigned int ncpus,
const char **models,
- unsigned int nmodels)
+ unsigned int nmodels,
+ unsigned int flags)
{
struct cpuArchDriver *driver;
size_t i;
@@ -392,7 +394,7 @@ cpuBaseline(virCPUDefPtr *cpus,
return NULL;
}
- return driver->baseline(cpus, ncpus, models, nmodels);
+ return driver->baseline(cpus, ncpus, models, nmodels, flags);
}
diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index 4003435..7f1d4bd 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -53,7 +53,8 @@ typedef int
const virCPUDataPtr data,
const char **models,
unsigned int nmodels,
- const char *preferred);
+ const char *preferred,
+ unsigned int flags);
typedef int
(*cpuArchEncode) (virArch arch,
@@ -81,7 +82,8 @@ typedef virCPUDefPtr
(*cpuArchBaseline) (virCPUDefPtr *cpus,
unsigned int ncpus,
const char **models,
- unsigned int nmodels);
+ unsigned int nmodels,
+ unsigned int flags);
typedef int
(*cpuArchUpdate) (virCPUDefPtr guest,
@@ -149,13 +151,15 @@ extern char *
cpuBaselineXML(const char **xmlCPUs,
unsigned int ncpus,
const char **models,
- unsigned int nmodels);
+ unsigned int nmodels,
+ unsigned int flags);
extern virCPUDefPtr
cpuBaseline (virCPUDefPtr *cpus,
unsigned int ncpus,
const char **models,
- unsigned int nmodels);
+ unsigned int nmodels,
+ unsigned int flags);
extern int
cpuUpdate (virCPUDefPtr guest,
diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
index 25e25ba..d1b2a99 100644
--- a/src/cpu/cpu_arm.c
+++ b/src/cpu/cpu_arm.c
@@ -44,8 +44,12 @@ ArmDecode(virCPUDefPtr cpu ATTRIBUTE_UNUSED,
const virCPUDataPtr data ATTRIBUTE_UNUSED,
const char **models ATTRIBUTE_UNUSED,
unsigned int nmodels ATTRIBUTE_UNUSED,
- const char *preferred ATTRIBUTE_UNUSED)
+ const char *preferred ATTRIBUTE_UNUSED,
+ unsigned int flags)
{
+
+ virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1);
+
return 0;
}
diff --git a/src/cpu/cpu_generic.c b/src/cpu/cpu_generic.c
index 2fe792f..1264da4 100644
--- a/src/cpu/cpu_generic.c
+++ b/src/cpu/cpu_generic.c
@@ -113,7 +113,8 @@ static virCPUDefPtr
genericBaseline(virCPUDefPtr *cpus,
unsigned int ncpus,
const char **models,
- unsigned int nmodels)
+ unsigned int nmodels,
+ unsigned int flags)
{
virCPUDefPtr cpu = NULL;
virCPUFeatureDefPtr features = NULL;
@@ -121,6 +122,8 @@ genericBaseline(virCPUDefPtr *cpus,
unsigned int count;
size_t i, j;
+ virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL);
+
if (!cpuModelIsAllowed(cpus[0]->model, models, nmodels)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("CPU model %s is not supported by hypervisor"),
diff --git a/src/cpu/cpu_powerpc.c b/src/cpu/cpu_powerpc.c
index 55a4153..647a8a1 100644
--- a/src/cpu/cpu_powerpc.c
+++ b/src/cpu/cpu_powerpc.c
@@ -304,12 +304,15 @@ ppcDecode(virCPUDefPtr cpu,
const virCPUDataPtr data,
const char **models,
unsigned int nmodels,
- const char *preferred ATTRIBUTE_UNUSED)
+ const char *preferred ATTRIBUTE_UNUSED,
+ unsigned int flags)
{
int ret = -1;
struct ppc_map *map;
const struct ppc_model *model;
+ virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1);
+
if (data == NULL || (map = ppcLoadMap()) == NULL)
return -1;
@@ -377,7 +380,8 @@ static virCPUDefPtr
ppcBaseline(virCPUDefPtr *cpus,
unsigned int ncpus,
const char **models,
- unsigned int nmodels)
+ unsigned int nmodels,
+ unsigned int flags)
{
struct ppc_map *map = NULL;
const struct ppc_model *model;
@@ -385,6 +389,8 @@ ppcBaseline(virCPUDefPtr *cpus,
virCPUDefPtr cpu = NULL;
size_t i;
+ virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL);
+
if (!(map = ppcLoadMap()))
goto error;
diff --git a/src/cpu/cpu_s390.c b/src/cpu/cpu_s390.c
index cbfae42..d997e06 100644
--- a/src/cpu/cpu_s390.c
+++ b/src/cpu/cpu_s390.c
@@ -48,8 +48,12 @@ s390Decode(virCPUDefPtr cpu ATTRIBUTE_UNUSED,
const virCPUDataPtr data ATTRIBUTE_UNUSED,
const char **models ATTRIBUTE_UNUSED,
unsigned int nmodels ATTRIBUTE_UNUSED,
- const char *preferred ATTRIBUTE_UNUSED)
+ const char *preferred ATTRIBUTE_UNUSED,
+ unsigned int flags)
{
+
+ virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1);
+
return 0;
}
diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index a388f0f..f16a3cb 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -1319,13 +1319,41 @@ x86GuestData(virCPUDefPtr host,
return x86Compute(host, guest, data, message);
}
+static int
+x86AddFeatures(virCPUDefPtr cpu,
+ struct x86_map *map)
+{
+ const struct x86_model *candidate;
+ const struct x86_feature *feature = map->features;
+
+ candidate = map->models;
+ while (candidate != NULL) {
+ if (STREQ(cpu->model, candidate->name))
+ break;
+ candidate = candidate->next;
+ }
+ if (!candidate) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("%s not a known CPU model"), cpu->model);
+ return -1;
+ }
+ while (feature != NULL) {
+ if (x86DataIsSubset(candidate->data, feature->data) &&
+ (virCPUDefAddFeature(cpu, feature->name, VIR_CPU_FEATURE_REQUIRE) <
0))
+ return -1;
+ feature = feature->next;
+ }
+ return 0;
+}
+
static int
x86Decode(virCPUDefPtr cpu,
const struct cpuX86Data *data,
const char **models,
unsigned int nmodels,
- const char *preferred)
+ const char *preferred,
+ unsigned int flags)
{
int ret = -1;
struct x86_map *map;
@@ -1334,6 +1362,8 @@ x86Decode(virCPUDefPtr cpu,
virCPUDefPtr cpuModel = NULL;
size_t i;
+ virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, -1);
+
if (data == NULL || (map = x86LoadMap()) == NULL)
return -1;
@@ -1406,6 +1436,9 @@ x86Decode(virCPUDefPtr cpu,
goto out;
}
+ if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES &&
+ (x86AddFeatures(cpuModel, map) < 0))
+ goto out;
cpu->model = cpuModel->model;
cpu->vendor = cpuModel->vendor;
cpu->nfeatures = cpuModel->nfeatures;
@@ -1426,9 +1459,10 @@ x86DecodeCPUData(virCPUDefPtr cpu,
const virCPUDataPtr data,
const char **models,
unsigned int nmodels,
- const char *preferred)
+ const char *preferred,
+ unsigned int flags)
{
- return x86Decode(cpu, data->data.x86, models, nmodels, preferred);
+ return x86Decode(cpu, data->data.x86, models, nmodels, preferred, flags);
}
@@ -1674,7 +1708,8 @@ static virCPUDefPtr
x86Baseline(virCPUDefPtr *cpus,
unsigned int ncpus,
const char **models,
- unsigned int nmodels)
+ unsigned int nmodels,
+ unsigned int flags)
{
struct x86_map *map = NULL;
struct x86_model *base_model = NULL;
@@ -1755,7 +1790,7 @@ x86Baseline(virCPUDefPtr *cpus,
if (vendor && x86DataAddCpuid(base_model->data, &vendor->cpuid)
< 0)
goto error;
- if (x86Decode(cpu, base_model->data, models, nmodels, NULL) < 0)
+ if (x86Decode(cpu, base_model->data, models, nmodels, NULL, flags) < 0)
goto error;
if (!outputVendor)
diff --git a/src/libvirt.c b/src/libvirt.c
index 8157488..6ce6c32 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -18522,11 +18522,16 @@ error:
* @conn: virConnect connection
* @xmlCPUs: array of XML descriptions of host CPUs
* @ncpus: number of CPUs in xmlCPUs
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of virConnectBaselineCPUFlags
*
* Computes the most feature-rich CPU which is compatible with all given
* host CPUs.
*
+ * If @flags includes VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES then libvirt
+ * will explicitly list all CPU features that are part of the host CPU,
+ * without this flag features that are part of the CPU model will not be
+ * listed.
+ *
* Returns XML description of the computed CPU or NULL on error.
*/
char *
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2daafa8..2ad236e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11056,12 +11056,12 @@ qemuConnectBaselineCPU(virConnectPtr conn ATTRIBUTE_UNUSED,
{
char *cpu = NULL;
- virCheckFlags(0, NULL);
+ virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, NULL);
if (virConnectBaselineCPUEnsureACL(conn) < 0)
goto cleanup;
- cpu = cpuBaselineXML(xmlCPUs, ncpus, NULL, 0);
+ cpu = cpuBaselineXML(xmlCPUs, ncpus, NULL, 0, flags);
cleanup:
return cpu;
diff --git a/tests/cputest.c b/tests/cputest.c
index 2e5f0cd..959cb9f 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -75,6 +75,7 @@ struct data {
const char *modelsName;
unsigned int nmodels;
const char *preferred;
+ unsigned int flags;
int result;
};
@@ -330,7 +331,7 @@ cpuTestBaseline(const void *arg)
if (!(cpus = cpuTestLoadMultiXML(data->arch, data->name, &ncpus)))
goto cleanup;
- baseline = cpuBaseline(cpus, ncpus, NULL, 0);
+ baseline = cpuBaseline(cpus, ncpus, NULL, 0, data->flags);
if (data->result < 0) {
virResetLastError();
if (!baseline)
@@ -510,12 +511,12 @@ mymain(void)
}
#define DO_TEST(arch, api, name, host, cpu, \
- models, nmodels, preferred, result) \
+ models, nmodels, preferred, flags, result) \
do { \
static struct data data = { \
arch, api, host, cpu, models, \
models == NULL ? NULL : #models, \
- nmodels, preferred, result \
+ nmodels, preferred, flags, result \
}; \
if (cpuTestRun(name, &data) < 0) \
ret = -1; \
@@ -524,31 +525,31 @@ mymain(void)
#define DO_TEST_COMPARE(arch, host, cpu, result) \
DO_TEST(arch, API_COMPARE, \
host "/" cpu " (" #result ")",
\
- host, cpu, NULL, 0, NULL, result)
+ host, cpu, NULL, 0, NULL, 0, result)
#define DO_TEST_UPDATE(arch, host, cpu, result) \
do { \
DO_TEST(arch, API_UPDATE, \
cpu " on " host, \
- host, cpu, NULL, 0, NULL, 0); \
+ host, cpu, NULL, 0, NULL, 0, 0); \
DO_TEST_COMPARE(arch, host, host "+" cpu, result); \
} while (0)
-#define DO_TEST_BASELINE(arch, name, result) \
+#define DO_TEST_BASELINE(arch, name, flags, result) \
DO_TEST(arch, API_BASELINE, name, NULL, "baseline-" name, \
- NULL, 0, NULL, result)
+ NULL, 0, NULL, flags, result)
#define DO_TEST_HASFEATURE(arch, host, feature, result) \
DO_TEST(arch, API_HAS_FEATURE, \
host "/" feature " (" #result ")",
\
- host, feature, NULL, 0, NULL, result)
+ host, feature, NULL, 0, NULL, 0, result)
#define DO_TEST_GUESTDATA(arch, host, cpu, models, preferred, result) \
DO_TEST(arch, API_GUEST_DATA, \
host "/" cpu " (" #models ", pref=" #preferred
")", \
host, cpu, models, \
models == NULL ? 0 : sizeof(models) / sizeof(char *), \
- preferred, result)
+ preferred, 0, result)
/* host to host comparison */
DO_TEST_COMPARE("x86", "host", "host",
VIR_CPU_COMPARE_IDENTICAL);
@@ -593,11 +594,12 @@ mymain(void)
DO_TEST_UPDATE("x86", "host", "host-passthrough",
VIR_CPU_COMPARE_IDENTICAL);
/* computing baseline CPUs */
- DO_TEST_BASELINE("x86", "incompatible-vendors", -1);
- DO_TEST_BASELINE("x86", "no-vendor", 0);
- DO_TEST_BASELINE("x86", "some-vendors", 0);
- DO_TEST_BASELINE("x86", "1", 0);
- DO_TEST_BASELINE("x86", "2", 0);
+ DO_TEST_BASELINE("x86", "incompatible-vendors", 0, -1);
+ DO_TEST_BASELINE("x86", "no-vendor", 0, 0);
+ DO_TEST_BASELINE("x86", "some-vendors", 0, 0);
+ DO_TEST_BASELINE("x86", "1", 0, 0);
+ DO_TEST_BASELINE("x86", "2", 0, 0);
+ DO_TEST_BASELINE("x86", "3",
VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES, 0);
/* CPU features */
DO_TEST_HASFEATURE("x86", "host", "vmx", YES);
diff --git a/tests/cputestdata/x86-baseline-3-result.xml
b/tests/cputestdata/x86-baseline-3-result.xml
new file mode 100644
index 0000000..d196112
--- /dev/null
+++ b/tests/cputestdata/x86-baseline-3-result.xml
@@ -0,0 +1,35 @@
+<cpu mode='custom' match='exact'>
+ <model fallback='allow'>Westmere</model>
+ <feature policy='require' name='lahf_lm'/>
+ <feature policy='require' name='lm'/>
+ <feature policy='require' name='nx'/>
+ <feature policy='require' name='syscall'/>
+ <feature policy='require' name='aes'/>
+ <feature policy='require' name='popcnt'/>
+ <feature policy='require' name='sse4.2'/>
+ <feature policy='require' name='sse4.1'/>
+ <feature policy='require' name='cx16'/>
+ <feature policy='require' name='ssse3'/>
+ <feature policy='require' name='pni'/>
+ <feature policy='require' name='sse2'/>
+ <feature policy='require' name='sse'/>
+ <feature policy='require' name='fxsr'/>
+ <feature policy='require' name='mmx'/>
+ <feature policy='require' name='clflush'/>
+ <feature policy='require' name='pse36'/>
+ <feature policy='require' name='pat'/>
+ <feature policy='require' name='cmov'/>
+ <feature policy='require' name='mca'/>
+ <feature policy='require' name='pge'/>
+ <feature policy='require' name='mtrr'/>
+ <feature policy='require' name='sep'/>
+ <feature policy='require' name='apic'/>
+ <feature policy='require' name='cx8'/>
+ <feature policy='require' name='mce'/>
+ <feature policy='require' name='pae'/>
+ <feature policy='require' name='msr'/>
+ <feature policy='require' name='tsc'/>
+ <feature policy='require' name='pse'/>
+ <feature policy='require' name='de'/>
+ <feature policy='require' name='fpu'/>
+</cpu>
diff --git a/tests/cputestdata/x86-baseline-3.xml b/tests/cputestdata/x86-baseline-3.xml
new file mode 100644
index 0000000..7654a1d
--- /dev/null
+++ b/tests/cputestdata/x86-baseline-3.xml
@@ -0,0 +1,7 @@
+<cpuTest>
+<cpu>
+ <arch>x86_64</arch>
+ <model>Westmere</model>
+ <topology sockets='1' cores='2' threads='1'/>
+</cpu>
+</cpuTest>
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 8cafce4..a1d5ab4 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6148,6 +6148,10 @@ static const vshCmdOptDef opts_cpu_baseline[] = {
.flags = VSH_OFLAG_REQ,
.help = N_("file containing XML CPU descriptions")
},
+ {.name = "features",
+ .type = VSH_OT_BOOL,
+ .help = N_("Show features that are part of the CPU model type")
+ },
{.name = NULL}
};
@@ -6159,6 +6163,7 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd)
char *buffer;
char *result = NULL;
const char **list = NULL;
+ unsigned int flags = 0;
int count = 0;
xmlDocPtr xml = NULL;
@@ -6168,6 +6173,9 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd)
virBuffer buf = VIR_BUFFER_INITIALIZER;
size_t i;
+ if (vshCommandOptBool(cmd, "model_features"))
+ flags |= VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES;
+
if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
return false;
@@ -6211,7 +6219,7 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd)
list[i] = vshStrdup(ctl, (const char *)xmlBufferContent(xml_buf));
}
- result = virConnectBaselineCPU(ctl->conn, list, count, 0);
+ result = virConnectBaselineCPU(ctl->conn, list, count, flags);
if (result) {
vshPrint(ctl, "%s", result);
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 3ff6da1..0ae5178 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -485,13 +485,16 @@ cell and the total free memory on the machine. Finally, with a
numeric argument or with --cellno plus a cell number it will display
the free memory for the specified cell only.
-=item B<cpu-baseline> I<FILE>
+=item B<cpu-baseline> I<FILE> [I<--features>]
Compute baseline CPU which will be supported by all host CPUs given in <file>.
The list of host CPUs is built by extracting all <cpu> elements from the
<file>. Thus, the <file> can contain either a set of <cpu> elements
separated
by new lines or even a set of complete <capabilities> elements printed by
-B<capabilities> command.
+B<capabilities> command. If I<--features> is specified then the
+resulting XML description will explicitly include all features that make
+up the CPU, without this option features that are part of the CPU model
+will not be listed in the XML description.
=item B<cpu-compare> I<FILE>
--
1.7.10.4
--
Don Dugger
"Censeo Toto nos in Kansa esse decisse." - D. Gale
n0ano(a)n0ano.com
Ph: 303/443-3786
--
Don Dugger
"Censeo Toto nos in Kansa esse decisse." - D. Gale
n0ano(a)n0ano.com
Ph: 303/443-3786