Re: [PATCH 07/10] libxl: add support for PVH

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

 



On 09/10/2018 04:02 PM, Marek Marczykowski-Górecki wrote:
On Mon, Sep 10, 2018 at 03:44:33PM -0600, Jim Fehlig wrote:
On 08/05/2018 03:48 PM, Marek Marczykowski-Górecki wrote:
Since this is something between PV and HVM, it makes sense to put the
setting in place where domain type is specified.
To enable it, use <os><type machine="xenpvh">...</type></os>. It is
also included in capabilities.xml, for every supported HVM guest type - it
doesn't seems to be any other requirement (besides new enough Xen).

Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
---
   docs/formatcaps.html.in        |  4 +--
   docs/schemas/domaincommon.rng  |  1 +-
   src/libxl/libxl_capabilities.c | 23 ++++++++++++++++++--
   src/libxl/libxl_conf.c         | 41 ++++++++++++++++++++++++++++++-----
   src/libxl/libxl_driver.c       |  6 +++--
   5 files changed, 64 insertions(+), 11 deletions(-)

diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in
index 34a74a9..1f17aa9 100644
--- a/docs/formatcaps.html.in
+++ b/docs/formatcaps.html.in
@@ -104,8 +104,8 @@
               <dt><code>machine</code></dt><dd>Machine type, for use in
                 <a href="formatdomain.html#attributeOSTypeMachine">machine</a>
                 attribute of os/type element in domain XML. For example Xen
-              supports <code>xenfv</code> for HVM or <code>xenpv</code> for
-              PV.</dd>
+              supports <code>xenfv</code> for HVM, <code>xenpv</code> for
+              PV, or <code>xenpvh</code> for PVHv2.</dd>
               <dt><code>domain</code></dt><dd>Supported domain type, named by the
                 <code>type</code> attribute.</dd>
             </dl>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index eded1ca..d32b0cb 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -341,6 +341,7 @@
             <choice>
               <value>xenpv</value>
               <value>xenfv</value>
+            <value>xenpvh</value>
             </choice>
           </attribute>
         </optional>
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index 18596c7..e7b26f1 100644
--- a/src/libxl/libxl_capabilities.c
+++ b/src/libxl/libxl_capabilities.c
@@ -57,6 +57,7 @@ struct guest_arch {
       virArch arch;
       int bits;
       int hvm;
+    int pvh;
       int pae;
       int nonpae;
       int ia64_be;
@@ -491,13 +492,29 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
                   guest_archs[i].nonpae = nonpae;
               if (ia64_be)
                   guest_archs[i].ia64_be = ia64_be;
+
+            /* On Xen >= 4.9 add PVH for each HVM guest, and do it only once */

I'm having problems understanding this. Do you mean add a PVH for each
supported HVM arch, but exclude PAE? E.g. standard xen_caps on x86_64
contains

xen-3.0-x86_64 xen-3.0-x86_32p hvm-3.0-x86_32 hvm-3.0-x86_32p hvm-3.0-x86_64

Given these caps, should a PVH be added that corresponds to the
hvm-3.0-x86_32 cap and another for the hvm-3.0-x86_64 cap, but the
hvm-3.0-x86_32p cap excluded?

Yes, exactly. Setting PAE (or not) is possible only for HVM, but not
PVH.

It would be much better if Xen would report support for PVH
explicitly...

+            if ((ver_info->xen_version_major > 4 ||
+                    (ver_info->xen_version_major == 4 &&
+                     ver_info->xen_version_minor >= 9)) &&
+                    hvm && i == nr_guest_archs-1) {

How about checking for hvm && !pae instead of i == nr_guest_archs-1?

+                i = nr_guest_archs;
+                /* Too many arch flavours - highly unlikely ! */
+                if (i >= ARRAY_CARDINALITY(guest_archs))
+                    continue;
+                nr_guest_archs++;
+                guest_archs[i].arch = arch;
+                guest_archs[i].pvh = 1;
+            }

Without answers to the above questions, I can't really comment on this code.
Regardless, since PVH is not advertised in xen_caps shouldn't it be added to
guest_archs outside of the loop parsing xen_caps?

This works on assumption that if you have HVM and new enough Xen, then
you have PVH. Just having new Xen isn't enough - for example the
hardware may lack VT-x.

           }
       }
       regfree(&regex);
       for (i = 0; i < nr_guest_archs; ++i) {
           virCapsGuestPtr guest;
-        char const *const xen_machines[] = {guest_archs[i].hvm ? "xenfv" : "xenpv"};
+        char const *const xen_machines[] = {
+            guest_archs[i].hvm ? "xenfv" :
+                (guest_archs[i].pvh ? "xenpvh" : "xenpv")};
           virCapsGuestMachinePtr *machines;
           if ((machines = virCapabilitiesAllocMachines(xen_machines, 1)) == NULL)
@@ -557,7 +574,9 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
                                                  1,
                                                  0) == NULL)
                   return -1;
+        }
+        if (guest_archs[i].hvm || guest_archs[i].pvh) {
               if (virCapabilitiesAddGuestFeature(guest,
                                                  "hap",
                                                  1,
@@ -580,7 +599,7 @@ libxlMakeDomainOSCaps(const char *machine,
       os->supported = true;
-    if (STREQ(machine, "xenpv"))
+    if (STREQ(machine, "xenpv") || STREQ(machine, "xenpvh"))
           return 0;
       capsLoader->supported = true;
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index f3da0ed..2df40ec 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -133,8 +133,11 @@ libxlMakeDomCreateInfo(libxl_ctx *ctx,
       libxl_domain_create_info_init(c_info);
-    if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
-        c_info->type = LIBXL_DOMAIN_TYPE_HVM;
+    if (def->os.type == VIR_DOMAIN_OSTYPE_HVM ||
+            (def->os.type == VIR_DOMAIN_OSTYPE_XEN &&
+             STREQ(def->os.machine, "xenpvh"))) {
+        c_info->type = def->os.type == VIR_DOMAIN_OSTYPE_HVM ?
+            LIBXL_DOMAIN_TYPE_HVM : LIBXL_DOMAIN_TYPE_PVH;
           switch ((virTristateSwitch) def->features[VIR_DOMAIN_FEATURE_HAP]) {
           case VIR_TRISTATE_SWITCH_OFF:
               libxl_defbool_set(&c_info->hap, false);
@@ -276,7 +279,8 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
       virDomainClockDef clock = def->clock;
       libxl_ctx *ctx = cfg->ctx;
       libxl_domain_build_info *b_info = &d_config->b_info;
-    int hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM;
+    bool hvm = def->os.type == VIR_DOMAIN_OSTYPE_HVM;
+    bool pvh = STREQ(def->os.machine, "xenpvh");
       size_t i;
       size_t nusbdevice = 0;
@@ -284,6 +288,8 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
       if (hvm)
           libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_HVM);
+    else if (pvh)
+        libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH);
       else
           libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PV);
@@ -375,7 +381,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
       def->mem.cur_balloon = VIR_ROUND_UP(def->mem.cur_balloon, 1024);
       b_info->max_memkb = virDomainDefGetMemoryInitial(def);
       b_info->target_memkb = def->mem.cur_balloon;
-    if (hvm) {
+    if (hvm || pvh) {
           if (caps &&
               def->cpu && def->cpu->mode == (VIR_CPU_MODE_HOST_PASSTHROUGH)) {
               bool hasHwVirt = false;
@@ -647,6 +653,31 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
               return -1;
           }
   #endif
+    } else if (pvh) {
+#ifdef LIBXL_HAVE_BUILDINFO_KERNEL
+        if (VIR_STRDUP(b_info->cmdline, def->os.cmdline) < 0)
+            return -1;
+        if (VIR_STRDUP(b_info->kernel, def->os.kernel) < 0)
+            return -1;
+        if (VIR_STRDUP(b_info->ramdisk, def->os.initrd) < 0)
+            return -1;
+#else
+        /*
+         * Shouldn't happen as LIBXL_HAVE_BUILDINFO_KERNEL is there since Xen
+         * 4.5, but PVHv2 since 4.9.
+         */
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("PVH guest type not supported"));
+#endif

I guess this is needed else the build will fail on Xen < 4.5?

Yes, exactly.

Maybe it is
time to bump the minimum supported Xen version to 4.6 :-). I say that a bit
jokingly, but I did propose it a few months back.

IMO good idea, since Xen < 4.6 is not supported anymore.

BTW, here is a link to my earlier series to drop support for 4.4 and 4.5

https://www.redhat.com/archives/libvir-list/2018-March/msg01704.html

I'll rebase and resend that soon.

Regards,
Jim

--
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