On 08/02/2013 01:08 PM, Don Dugger 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
s/explictly/explicitly
list all features that are part of that model.
Signed-off-by: Don Dugger <donald.d.dugger@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.]
+++ 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))
The outer () aren't needed on this line.
+ return -1;
Indentation is off.
@@ -1406,6 +1436,9 @@ x86Decode(virCPUDefPtr cpu, goto out; }
+ if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES && + (x86AddFeatures(cpuModel, map) < 0))
Extra ()
+ goto out;
Indentation is off.
+++ 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",
@@ -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;
Mismatch in names. How did you even test this? My testing: I used just-compiled virsh with existing libvirtd, and was surprised to get no failure and no difference in output; after tweaking the virsh bug, I finally got expected failure: $ tools/virsh cpu-baseline ../foo.xml --features error: unsupported flags (0x1) in function qemuConnectBaselineCPU then after restarting to just-built libvirtd, I checked that adding --features changed the output. (hmm, maybe virsh can add some sanity checking code to make sure we aren't querying for a mismatched name; but that would be a followup patch...) Those are all minor, so I squashed this in and pushed. Thanks again for being patient, and for the pings along the way. diff --git i/src/cpu/cpu_x86.c w/src/cpu/cpu_x86.c index f16a3cb..41ce21f 100644 --- i/src/cpu/cpu_x86.c +++ w/src/cpu/cpu_x86.c @@ -1339,8 +1339,9 @@ x86AddFeatures(virCPUDefPtr cpu, } while (feature != NULL) { if (x86DataIsSubset(candidate->data, feature->data) && - (virCPUDefAddFeature(cpu, feature->name, VIR_CPU_FEATURE_REQUIRE) < 0)) - return -1; + virCPUDefAddFeature(cpu, feature->name, + VIR_CPU_FEATURE_REQUIRE) < 0) + return -1; feature = feature->next; } return 0; @@ -1437,8 +1438,8 @@ x86Decode(virCPUDefPtr cpu, } if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES && - (x86AddFeatures(cpuModel, map) < 0)) - goto out; + x86AddFeatures(cpuModel, map) < 0) + goto out; cpu->model = cpuModel->model; cpu->vendor = cpuModel->vendor; cpu->nfeatures = cpuModel->nfeatures; diff --git i/tools/virsh-domain.c w/tools/virsh-domain.c index 7d31622..13e3045 100644 --- i/tools/virsh-domain.c +++ w/tools/virsh-domain.c @@ -6182,7 +6182,7 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd) virBuffer buf = VIR_BUFFER_INITIALIZER; size_t i; - if (vshCommandOptBool(cmd, "model_features")) + if (vshCommandOptBool(cmd, "features")) flags |= VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES; if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org