[libvirt] [PATCH v2 2/3] xenconfig: add conversions for xen-xl

[Jim Fehlig]

Wim ten Have wrote:
> On Mon, 17 Apr 2017 14:22:20 -0600
> Jim Fehlig <jfehlig at suse.com> wrote:
> 
>> On 03/24/2017 03:02 PM, Wim Ten Have wrote:
>>> From: Wim ten Have <wim.ten.have at oracle.com>
>>>
>>> Per xen-xl conversions from and to native under host-passthrough
>>> mode we take care for Xen (nestedhvm = mode) applied and inherited
>>> settings generating or processing correct feature policy:
>>>
>>> [On Intel (VT-x) architectures]
>>> <feature policy='disable' name='vmx'/>
>>>
>>> or
>>>
>>> [On AMD (AMD-V) architectures]
>>> <feature policy='disable' name='svm'/>
>>>
>>> It will then generate (or parse) for nestedhvm=1 in/from xl format.
>>>
>>> Signed-off-by: Joao Martins <joao.m.martins at oracle.com>
>>> Signed-off-by: Wim ten Have <wim.ten.have at oracle.com>
>>> ---
>>>  src/xenconfig/xen_xl.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 67 insertions(+)
>>>
>>> diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
>>> index 74f68b3..d3797b8 100644
>>> --- a/src/xenconfig/xen_xl.c
>>> +++ b/src/xenconfig/xen_xl.c
>>> @@ -106,6 +106,7 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
>>>      if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
>>>          const char *bios;
>>>          const char *boot;
>>> +        int val = 0;
>>>
>>>          if (xenConfigGetString(conf, "bios", &bios, NULL) < 0)
>>>              return -1;
>>> @@ -164,6 +165,47 @@ xenParseXLOS(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
>>>              }
>>>              def->os.nBootDevs++;
>>>          }
>>> +
>>> +        if (xenConfigGetBool(conf, "nestedhvm", &val, -1) < 0)
>>> +            return -1;
>>> +
>>> +        if (val != -1) {
>>> +            virCPUDefPtr cpu = NULL;
>>> +
>>> +            if (VIR_ALLOC(cpu) < 0)
>>> +                return -1;
>>> +
>>> +            if (val == 0) {
>>> +                if (VIR_ALLOC_N(cpu->features, 2) < 0)
>>> +                    goto cleanup;
>>> +
>>> +                /*
>>> +                 * Below is pointless in real world but for purpose
>>> +                 * of testing let's check features depth holding at
>>> +                 * least multiple elements and also check if both
>>> +                 * vmx|svm are understood.
>>> +                 */
>>> +                cpu->features[0].policy = VIR_CPU_FEATURE_DISABLE;
>>> +                if (VIR_STRDUP(cpu->features[0].name, "vmx") < 0)
>>> +                    goto cleanup;
>>> +                cpu->features[1].policy = VIR_CPU_FEATURE_DISABLE;
>>> +                if (VIR_STRDUP(cpu->features[1].name, "svm") < 0)
>>> +                    goto cleanup;  
>> Since caps is passed to this function, can it be used to determine whether to 
>> disable vmx or svm?
> 
>   Yes :-) ... thanks for enlightening me here as that is actually the correct
>   approach if non-test domain (conversion) actions are made under libvirtd.
> 
>   There's one minor question here per me.  To process caps for vmx|svm I'll
>   bring in virCPUCheckFeature() which at its turn requires me to include "cpu/cpu.h"
>   for its prototype.  Unfortunate cfg.mk does not anticipate and raises a
>   syntax-check caveat.
> 
> 	prohibit_cross_inclusion
> 	src/xenconfig/xen_xl.c:51:#include "cpu/cpu.h"
> 	maint.mk: unsafe cross-directory include
> 	cfg.mk:773: recipe for target 'sc_prohibit_cross_inclusion' failed
> 	make: *** [sc_prohibit_cross_inclusion] Error 1
> 
>   That is ... unless I apply below change to cfg.mk.
> 
> -           xenapi/ | xenconfig/ ) safe="($$dir|util|conf|xen)";;       \
> +           xenapi/ | xenconfig/ ) safe="($$dir|util|conf|xen|cpu)";;   \
> 
>   Is that lethal to do?  I tried to reason and fail to see why not, ie. i
>   am a bit clueless for its specific reason ... but also new to whole arena.

Adding 'cpu' as a safe dir should be fine. I'm not really sure why xenconfig is
included in the check. Seems it should be treated like the hypervisor driver
directories (qemu, libxl, lxc, etc.).

Regards,
Jim