Re: [libvirt] [PATCH v2] Add flag to BaselineCPU API to return detailed CPU features
Sigh. Rebase, I've heard of that, maybe I should remember to do it :-(
The failures are related to the rebasing, I'm working on that and
I'll incorporate your other fixes (with a couple of comments in line).
Only open issue is the flag name, I have no problems with
_EXPAND_FEATURES, unless there's an objection I'll do that.
On Tue, Jul 30, 2013 at 03:59:38PM -0600, Eric Blake wrote:
...
> +++ b/src/cpu/cpu.h
> @@ -49,7 +49,8 @@ typedef int
> const union cpuData *data,
> const char **models,
> unsigned int nmodels,
> - const char *preferred);
> + const char *preferred,
> + unsigned int flags);
>
> typedef int
> (*cpuArchEncode) (const virCPUDefPtr cpu,
> @@ -76,7 +77,8 @@ typedef virCPUDefPtr
> (*cpuArchBaseline) (virCPUDefPtr *cpus,
> unsigned int ncpus,
> const char **models,
> - unsigned int nmodels);
> + unsigned int nmodels,
> + unsigned int /* flags */);
No need to comment out the parameter name (multiple instances */.
This was to avoid compile failures with unused parameters but
this becomes moot when I put in the code to explicitly check
that flag.
...
> @@ -1383,6 +1416,9 @@ x86Decode(virCPUDefPtr cpu,
> goto out;
> }
>
> + if (flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE)
> + if (x86AddFeatures(cpuModel, map) < 0)
> + goto out;
You could write this with fewer nested if:
if ((flags & VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURE) &&
x86AddFeatures(cpuModel, map) < 0)
goto out;
I actually prefer the nested ifs over the logical expression (to
me that's more obvious) but I don't mind changing if that's your
preferred coding style.
...
> @@ -6049,7 +6057,8 @@ 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);
> +vshPrint(ctl, "result - %p\n", result);
Leftover debugging?
Yep, shoot me, I should know better. Sorry about that.
--
Don Dugger
"Censeo Toto nos in Kansa esse decisse." - D. Gale
n0ano(a)n0ano.com
Ph: 303/443-3786