On Fri, 2015-08-21 at 14:27 -0700, Jiri Denemark wrote:
> +static virCPUCompareResult
> +ppc64CheckCompatibilityMode(const char *host_model,
> + const char *compat_mode)
> +{
> + int host;
> + int compat;
> + char *tmp;
> + virCPUCompareResult ret = VIR_CPU_COMPARE_IDENTICAL;
Shouldn't ret be initialized to VIR_CPU_COMPARE_ERROR so that we
don't
report everything is OK on errors?
Initializing it to VIR_CPU_COMPARE_IDENTICAL allows us to
just jump to the exit point if a compatibility mode is not
used (see check right below).
That could be replaced with an explicit return if you
think that would make the code easier to understand.
> +
> + if (!compat_mode)
> + goto out;
[...]
> + ret = VIR_CPU_COMPARE_INCOMPATIBLE;
> +
> + /* Version check */
> + if (compat > host)
> + goto out;
> +
> + ret = VIR_CPU_COMPARE_IDENTICAL;
if (compat > host)
ret = VIR_CPU_COMPARE_INCOMPATIBLE;
else
ret = VIR_CPU_COMPARE_IDENTICAL;
would be a bit more obvious I think.
Both work for me, so why not :)
In the long term, I think we should store compatibility modes within
cpu_map.xml, but ACK to this with the small issues addressed.
Sure, that will come later on.
Cheers.
--
Andrea Bolognani
Software Engineer - Virtualization Team