[libvirt] [PATCH v5 3/9] libxl: error out on not supported CPU mode, instead of silently ignoring

[Marek Marczykowski-Górecki]

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 at 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?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180316/6d049b79/attachment-0001.sig>