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

On Mon, 17 Apr 2017 14:22:20 -0600 Jim Fehlig <jfehlig(a)suse.com&gt; wrote: > On 03/24/2017 03:02 PM, Wim Ten Have wrote: >> From: Wim ten Have <wim.ten.have(a)oracle.com&gt; >> >> 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(a)oracle.com&gt; >> Signed-off-by: Wim ten Have <wim.ten.have(a)oracle.com&gt; >> --- >> 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.