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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/24/2017 03:02 PM, Wim Ten Have wrote:
From: Wim ten Have <wim.ten.have@xxxxxxxxxx>

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@xxxxxxxxxx>
Signed-off-by: Wim ten Have <wim.ten.have@xxxxxxxxxx>
---
 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?

+
+                cpu->nfeatures = cpu->nfeatures_max = 2;
+            }
+            cpu->mode = VIR_CPU_MODE_HOST_PASSTHROUGH;
+            cpu->type = VIR_CPU_TYPE_GUEST;
+            def->cpu = cpu;
+            cpu = NULL;
+ cleanup:
+            if (cpu) {
+                VIR_FREE(cpu->features);
+                VIR_FREE(cpu);
+                return -1;
+            }

I'm not fond of the cleanup label in the middle of this function. If we can disable only one of vmx or svm, then there will be one less strdup and one less cleanup spot. Cleanup can then occur at the point of error.

+        }
+
     } else {
         if (xenConfigCopyStringOpt(conf, "bootloader", &def->os.bootloader) < 0)
             return -1;
@@ -897,6 +939,31 @@ xenFormatXLOS(virConfPtr conf, virDomainDefPtr def)
         if (xenConfigSetString(conf, "boot", boot) < 0)
             return -1;

+        if (def->cpu &&
+            def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) {
+            bool hasHwVirt = true;
+
+            if (def->cpu->nfeatures) {
+                for (i = 0; i < def->cpu->nfeatures; i++) {
+
+                    switch (def->cpu->features[i].policy) {
+                        case VIR_CPU_FEATURE_DISABLE:
+                        case VIR_CPU_FEATURE_FORBID:
+                            if (STREQ(def->cpu->features[i].name, "vmx") ||
+                                STREQ(def->cpu->features[i].name, "svm"))
+                                hasHwVirt = false;
+                            break;
+
+                        default:
+                            break;

Similar to patch 1, replace 'default' with the other enum values.

Regards,
Jim

+                    }
+                }
+            }
+
+            if (xenConfigSetInt(conf, "nestedhvm", hasHwVirt) < 0)
+                return -1;
+        }
+
         /* XXX floppy disks */
     } else {
         if (def->os.bootloader &&


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux