Re: [libvirt] [PATCH v5 3/9] libxl: error out on not supported CPU mode, instead of silently ignoring
On Fri, Mar 16, 2018 at 05:39:35PM +0000, Daniel P. Berrangé wrote:
On Wed, Mar 14, 2018 at 03:26:10AM +0100, Marek Marczykowski-Górecki
wrote:
> This change make libvirt XML with plain <cpu> element invalid for libxl,
> which affect not only upcoming CPUID support, but also NUMA. In fact,
> default mode 'custom' does not match what the driver actually does, so
> it was a bug. Adjust xenconfig driver accordingly.
> To not break existing configuration, add PostParse hook to update
> (previously ignored) default mode 'custom' to 'host-passthrough'.
Maybe I'm missing something, but by doing this silent conversion
from custom -> host-passthrough, don't you make it such that the
error about unsupported CPU mode is largely unreachable ? IOW seems
to end up with the same functional result as the original code,
except for a error when 'host-model' is used.
Mostly - yes.
I don't have a particular better idea though if we have alot of
pre-existing usage with mode=custom ?
Previously mode was mostly ignored (besides using it for enabling nested
HVM, which is now controlled additionally by global switch).
I'm not specifically attached to the PostParse, but I think it may be
nicer for users, to not throw an error on previously "valid"
configuration. If libvirt would have XML versioning, it could execute
such conversion only on upgrade...
Alternatively, instead of rejecting mode=custom, the whole cpuid
handling could be made conditional to mode=host-passthrough (and ignored
with mode=custom). I have an impression this would better fit into the
(libxl?) driver - I find it often that various settings are ignored
instead of throwing VIR_ERR_CONFIG_UNSUPPORTED... Admittedly, this
makes it easier to maintain a configuration compatible with wide range
of libvirt versions.
Has this been widely
done, or can we justify breaking it ?
> Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com>
> ---
> Changes since v4:
> - add PostParse hook to automatically set host-passthrough mode, if
> was the default one before (custom)
> Changes since v3:
> - fix vnuma tests (nested HVM implicitly enabled there)
> Changes since v2:
> - change separated from 'libxl: add support for CPUID features policy'
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
Attachments:
- signature.asc (application/pgp-signature — 488 bytes)