[v4] libxl: PVHv2 support

[libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Marek Marczykowski-Górecki 6 years ago

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@invisiblethingslab.com>
---
Changes in v2 proposed by Jim:
 - use new_arch_added var instead of i == nr_guest_archs for clarity
 - improve comment
 - adjust for now required Xen >= 4.6 (remove part for Xen < 4.5)

Changes in v3:
 - limit VIR_DOMAIN_OSTYPE_XEN -> VIR_DOMAIN_OSTYPE_LINUX conversion to
 Xen PV only
 - do not accept VIR_DOMAIN_OSTYPE_LINUX for PVH
 - fix reported capabilities for PVH - remove hostdev passthrough and
 video/graphics
 - use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to
 check for PVH support
 - compile fix for Xen < 4.9

Changes in v4:
 - revert to "i == nr_guest_archs-1" check
 - let configure check for LIBXL_DOMAIN_TYPE_PVH and use #ifdef
 HAVE_XEN_PVH then
 - fix comment about Xen version
---
 docs/formatcaps.html.in        |  4 +--
 docs/schemas/domaincommon.rng  |  1 +-
 m4/virt-driver-libxl.m4        |  3 ++-
 src/conf/domain_conf.c         |  6 ++--
 src/libxl/libxl_capabilities.c | 38 ++++++++++++++++++++++-----
 src/libxl/libxl_conf.c         | 50 ++++++++++++++++++++++++++++++-----
 src/libxl/libxl_driver.c       |  6 ++--
 7 files changed, 90 insertions(+), 18 deletions(-)

diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in
index 0d9c53d..fe113bc 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 PVH.</dd>
             <dt><code>domain</code></dt><dd>The <code>type</code> attribute of
               this element specifies the type of hypervisor required to run the
               domain. Use in <a href="formatdomain.html#attributeDomainType">type</a>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 099a949..820a7c6 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/m4/virt-driver-libxl.m4 b/m4/virt-driver-libxl.m4
index 479d911..2cd97cc 100644
--- a/m4/virt-driver-libxl.m4
+++ b/m4/virt-driver-libxl.m4
@@ -75,6 +75,9 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_LIBXL], [
     ])
   fi
 
+  dnl Check if Xen has support for PVH
+  AC_CHECK_DECL(LIBXL_DOMAIN_TYPE_PVH, [AC_DEFINE([HAVE_XEN_PVH], [1], [Define to 1 if Xen has PVH support.])], [], [#include <libxl.h>])
+
   AC_SUBST([LIBXL_CFLAGS])
   AC_SUBST([LIBXL_LIBS])
 ])
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9911d56..a945ad4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19094,7 +19094,8 @@ virDomainDefParseCaps(virDomainDefPtr def,
      * be 'xen'. So we accept the former and convert
      */
     if (def->os.type == VIR_DOMAIN_OSTYPE_LINUX &&
-        def->virtType == VIR_DOMAIN_VIRT_XEN) {
+        def->virtType == VIR_DOMAIN_VIRT_XEN &&
+        (!def->os.machine || STREQ(def->os.machine, "xenpv"))) {
         def->os.type = VIR_DOMAIN_OSTYPE_XEN;
     }
 
@@ -27705,7 +27706,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
      * be 'xen'. So we convert to the former for backcompat
      */
     if (def->virtType == VIR_DOMAIN_VIRT_XEN &&
-        def->os.type == VIR_DOMAIN_OSTYPE_XEN)
+        def->os.type == VIR_DOMAIN_OSTYPE_XEN &&
+        (!def->os.machine || STREQ(def->os.machine, "xenpv")))
         virBufferAsprintf(buf, ">%s</type>\n",
                           virDomainOSTypeToString(VIR_DOMAIN_OSTYPE_LINUX));
     else
diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
index 18596c7..eecd5e9 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,33 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
                 guest_archs[i].nonpae = nonpae;
             if (ia64_be)
                 guest_archs[i].ia64_be = ia64_be;
+
+            /*
+             * Xen 4.10 introduced support for the PVH guest type, which
+             * requires hardware virtualization support similar to the
+             * HVM guest type. Add a PVH guest type for each new HVM
+             * guest type.
+             */
+#ifdef HAVE_XEN_PVH
+            if (hvm && i == nr_guest_archs-1) {
+                i = nr_guest_archs;
+                /* Ensure we have not exhausted the guest_archs array */
+                if (nr_guest_archs >= ARRAY_CARDINALITY(guest_archs))
+                    continue;
+                guest_archs[nr_guest_archs].arch = arch;
+                guest_archs[nr_guest_archs].pvh = 1;
+                nr_guest_archs++;
+            }
+#endif
         }
     }
     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 +578,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 +603,7 @@ libxlMakeDomainOSCaps(const char *machine,
 
     os->supported = true;
 
-    if (STREQ(machine, "xenpv"))
+    if (STREQ(machine, "xenpv") || STREQ(machine, "xenpvh"))
         return 0;
 
     capsLoader->supported = true;
@@ -732,11 +755,14 @@ libxlMakeDomainCapabilities(virDomainCapsPtr domCaps,
         domCaps->maxvcpus = PV_MAX_VCPUS;
 
     if (libxlMakeDomainOSCaps(domCaps->machine, os, firmwares, nfirmwares) < 0 ||
-        libxlMakeDomainDeviceDiskCaps(disk) < 0 ||
-        libxlMakeDomainDeviceGraphicsCaps(graphics) < 0 ||
-        libxlMakeDomainDeviceVideoCaps(video) < 0 ||
-        libxlMakeDomainDeviceHostdevCaps(hostdev) < 0)
+        libxlMakeDomainDeviceDiskCaps(disk) < 0)
+        return -1;
+    if (STRNEQ(domCaps->machine, "xenpvh") &&
+        (libxlMakeDomainDeviceGraphicsCaps(graphics) < 0 ||
+         libxlMakeDomainDeviceVideoCaps(video) < 0 ||
+         libxlMakeDomainDeviceHostdevCaps(hostdev) < 0))
         return -1;
+
     return 0;
 }
 
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index f3da0ed..db50a73 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -133,8 +133,20 @@ libxlMakeDomCreateInfo(libxl_ctx *ctx,
 
     libxl_domain_create_info_init(c_info);
 
-    if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
+    if (def->os.type == VIR_DOMAIN_OSTYPE_HVM ||
+            (def->os.type == VIR_DOMAIN_OSTYPE_XEN &&
+             STREQ(def->os.machine, "xenpvh"))) {
+#ifdef HAVE_XEN_PVH
+        c_info->type = def->os.type == VIR_DOMAIN_OSTYPE_HVM ?
+            LIBXL_DOMAIN_TYPE_HVM : LIBXL_DOMAIN_TYPE_PVH;
+#else
+        if (STREQ(def->os.machine, "xenpvh")) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                    _("PVH guest type not supported"));
+            return -1;
+        }
         c_info->type = LIBXL_DOMAIN_TYPE_HVM;
+#endif
         switch ((virTristateSwitch) def->features[VIR_DOMAIN_FEATURE_HAP]) {
         case VIR_TRISTATE_SWITCH_OFF:
             libxl_defbool_set(&c_info->hap, false);
@@ -276,16 +288,26 @@ 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;
 
     libxl_domain_build_info_init(b_info);
 
-    if (hvm)
+    if (hvm) {
         libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_HVM);
-    else
+    } else if (pvh) {
+#ifdef HAVE_XEN_PVH
+        libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH);
+#else
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                _("PVH guest type not supported"));
+        return -1;
+#endif
+    } else {
         libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PV);
+    }
 
     b_info->max_vcpus = virDomainDefGetVcpusMax(def);
     if (libxl_cpu_bitmap_alloc(ctx, &b_info->avail_vcpus, b_info->max_vcpus))
@@ -375,7 +397,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 +669,22 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
             return -1;
         }
 #endif
+    } else if (pvh) {
+        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;
+#ifdef LIBXL_HAVE_BUILDINFO_BOOTLOADER
+        if (VIR_STRDUP(b_info->bootloader, def->os.bootloader) < 0)
+            return -1;
+        if (def->os.bootloaderArgs) {
+            if (!(b_info->bootloader_args =
+                  virStringSplit(def->os.bootloaderArgs, " \t\n", 0)))
+                return -1;
+        }
+#endif
     } else {
         /*
          * For compatibility with the legacy xen toolstack, default to pygrub
@@ -1230,7 +1268,7 @@ libxlMakeNic(virDomainDefPtr def,
             STRNEQ(l_nic->model, "netfront")) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("only model 'netfront' is supported for "
-                             "Xen PV domains"));
+                             "Xen PV(H) domains"));
             return -1;
         }
         if (VIR_STRDUP(x_nic->model, l_nic->model) < 0)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index efd47a3..6287cef 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -6398,9 +6398,11 @@ libxlConnectGetDomainCapabilities(virConnectPtr conn,
         emulatorbin = "/usr/bin/qemu-system-x86_64";
 
     if (machine) {
-        if (STRNEQ(machine, "xenpv") && STRNEQ(machine, "xenfv")) {
+        if (STRNEQ(machine, "xenpv") &&
+                STRNEQ(machine, "xenpvh") &&
+                STRNEQ(machine, "xenfv")) {
             virReportError(VIR_ERR_INVALID_ARG, "%s",
-                           _("Xen only supports 'xenpv' and 'xenfv' machines"));
+                           _("Xen only supports 'xenpv', 'xenpvh' and 'xenfv' machines"));
             goto cleanup;
         }
     } else {
-- 
git-series 0.9.1

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

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Jim Fehlig 6 years ago

On 10/7/18 9:39 AM, 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@invisiblethingslab.com>
> ---
> Changes in v2 proposed by Jim:
>   - use new_arch_added var instead of i == nr_guest_archs for clarity
>   - improve comment
>   - adjust for now required Xen >= 4.6 (remove part for Xen < 4.5)
> 
> Changes in v3:
>   - limit VIR_DOMAIN_OSTYPE_XEN -> VIR_DOMAIN_OSTYPE_LINUX conversion to
>   Xen PV only
>   - do not accept VIR_DOMAIN_OSTYPE_LINUX for PVH
>   - fix reported capabilities for PVH - remove hostdev passthrough and
>   video/graphics
>   - use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to
>   check for PVH support
>   - compile fix for Xen < 4.9
> 
> Changes in v4:
>   - revert to "i == nr_guest_archs-1" check
>   - let configure check for LIBXL_DOMAIN_TYPE_PVH and use #ifdef
>   HAVE_XEN_PVH then
>   - fix comment about Xen version
> ---
>   docs/formatcaps.html.in        |  4 +--
>   docs/schemas/domaincommon.rng  |  1 +-
>   m4/virt-driver-libxl.m4        |  3 ++-
>   src/conf/domain_conf.c         |  6 ++--
>   src/libxl/libxl_capabilities.c | 38 ++++++++++++++++++++++-----
>   src/libxl/libxl_conf.c         | 50 ++++++++++++++++++++++++++++++-----
>   src/libxl/libxl_driver.c       |  6 ++--
>   7 files changed, 90 insertions(+), 18 deletions(-)
> 
> diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in
> index 0d9c53d..fe113bc 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 PVH.</dd>
>               <dt><code>domain</code></dt><dd>The <code>type</code> attribute of
>                 this element specifies the type of hypervisor required to run the
>                 domain. Use in <a href="formatdomain.html#attributeDomainType">type</a>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 099a949..820a7c6 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/m4/virt-driver-libxl.m4 b/m4/virt-driver-libxl.m4
> index 479d911..2cd97cc 100644
> --- a/m4/virt-driver-libxl.m4
> +++ b/m4/virt-driver-libxl.m4
> @@ -75,6 +75,9 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_LIBXL], [
>       ])
>     fi
>   
> +  dnl Check if Xen has support for PVH
> +  AC_CHECK_DECL(LIBXL_DOMAIN_TYPE_PVH, [AC_DEFINE([HAVE_XEN_PVH], [1], [Define to 1 if Xen has PVH support.])], [], [#include <libxl.h>])
> +
>     AC_SUBST([LIBXL_CFLAGS])
>     AC_SUBST([LIBXL_LIBS])
>   ])
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 9911d56..a945ad4 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -19094,7 +19094,8 @@ virDomainDefParseCaps(virDomainDefPtr def,
>        * be 'xen'. So we accept the former and convert
>        */
>       if (def->os.type == VIR_DOMAIN_OSTYPE_LINUX &&
> -        def->virtType == VIR_DOMAIN_VIRT_XEN) {
> +        def->virtType == VIR_DOMAIN_VIRT_XEN &&
> +        (!def->os.machine || STREQ(def->os.machine, "xenpv"))) {
>           def->os.type = VIR_DOMAIN_OSTYPE_XEN;
>       }
>   
> @@ -27705,7 +27706,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>        * be 'xen'. So we convert to the former for backcompat
>        */
>       if (def->virtType == VIR_DOMAIN_VIRT_XEN &&
> -        def->os.type == VIR_DOMAIN_OSTYPE_XEN)
> +        def->os.type == VIR_DOMAIN_OSTYPE_XEN &&
> +        (!def->os.machine || STREQ(def->os.machine, "xenpv")))
>           virBufferAsprintf(buf, ">%s</type>\n",
>                             virDomainOSTypeToString(VIR_DOMAIN_OSTYPE_LINUX));
>       else

I'd still like to hear what others think of extending the hack. mprivozn? You 
often have an interesting opinion :-).

> diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
> index 18596c7..eecd5e9 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,33 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
>                   guest_archs[i].nonpae = nonpae;
>               if (ia64_be)
>                   guest_archs[i].ia64_be = ia64_be;
> +
> +            /*
> +             * Xen 4.10 introduced support for the PVH guest type, which
> +             * requires hardware virtualization support similar to the
> +             * HVM guest type. Add a PVH guest type for each new HVM
> +             * guest type.
> +             */
> +#ifdef HAVE_XEN_PVH
> +            if (hvm && i == nr_guest_archs-1) {
> +                i = nr_guest_archs;
> +                /* Ensure we have not exhausted the guest_archs array */
> +                if (nr_guest_archs >= ARRAY_CARDINALITY(guest_archs))
> +                    continue;
> +                guest_archs[nr_guest_archs].arch = arch;
> +                guest_archs[nr_guest_archs].pvh = 1;
> +                nr_guest_archs++;
> +            }
> +#endif
>           }
>       }
>       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 +578,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 +603,7 @@ libxlMakeDomainOSCaps(const char *machine,
>   
>       os->supported = true;
>   
> -    if (STREQ(machine, "xenpv"))
> +    if (STREQ(machine, "xenpv") || STREQ(machine, "xenpvh"))
>           return 0;
>   
>       capsLoader->supported = true;
> @@ -732,11 +755,14 @@ libxlMakeDomainCapabilities(virDomainCapsPtr domCaps,
>           domCaps->maxvcpus = PV_MAX_VCPUS;
>   
>       if (libxlMakeDomainOSCaps(domCaps->machine, os, firmwares, nfirmwares) < 0 ||
> -        libxlMakeDomainDeviceDiskCaps(disk) < 0 ||
> -        libxlMakeDomainDeviceGraphicsCaps(graphics) < 0 ||
> -        libxlMakeDomainDeviceVideoCaps(video) < 0 ||
> -        libxlMakeDomainDeviceHostdevCaps(hostdev) < 0)
> +        libxlMakeDomainDeviceDiskCaps(disk) < 0)
> +        return -1;
> +    if (STRNEQ(domCaps->machine, "xenpvh") &&
> +        (libxlMakeDomainDeviceGraphicsCaps(graphics) < 0 ||
> +         libxlMakeDomainDeviceVideoCaps(video) < 0 ||
> +         libxlMakeDomainDeviceHostdevCaps(hostdev) < 0))
>           return -1;
> +
>       return 0;
>   }
>   
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index f3da0ed..db50a73 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -133,8 +133,20 @@ libxlMakeDomCreateInfo(libxl_ctx *ctx,
>   
>       libxl_domain_create_info_init(c_info);
>   
> -    if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
> +    if (def->os.type == VIR_DOMAIN_OSTYPE_HVM ||
> +            (def->os.type == VIR_DOMAIN_OSTYPE_XEN &&
> +             STREQ(def->os.machine, "xenpvh"))) {
> +#ifdef HAVE_XEN_PVH
> +        c_info->type = def->os.type == VIR_DOMAIN_OSTYPE_HVM ?
> +            LIBXL_DOMAIN_TYPE_HVM : LIBXL_DOMAIN_TYPE_PVH;
> +#else
> +        if (STREQ(def->os.machine, "xenpvh")) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                    _("PVH guest type not supported"));
> +            return -1;
> +        }
>           c_info->type = LIBXL_DOMAIN_TYPE_HVM;
> +#endif
>           switch ((virTristateSwitch) def->features[VIR_DOMAIN_FEATURE_HAP]) {
>           case VIR_TRISTATE_SWITCH_OFF:
>               libxl_defbool_set(&c_info->hap, false);
> @@ -276,16 +288,26 @@ 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;
>   
>       libxl_domain_build_info_init(b_info);
>   
> -    if (hvm)
> +    if (hvm) {
>           libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_HVM);
> -    else
> +    } else if (pvh) {
> +#ifdef HAVE_XEN_PVH
> +        libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PVH);
> +#else
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                _("PVH guest type not supported"));
> +        return -1;
> +#endif
> +    } else {
>           libxl_domain_build_info_init_type(b_info, LIBXL_DOMAIN_TYPE_PV);
> +    }
>   
>       b_info->max_vcpus = virDomainDefGetVcpusMax(def);
>       if (libxl_cpu_bitmap_alloc(ctx, &b_info->avail_vcpus, b_info->max_vcpus))
> @@ -375,7 +397,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 +669,22 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
>               return -1;
>           }
>   #endif
> +    } else if (pvh) {
> +        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;
> +#ifdef LIBXL_HAVE_BUILDINFO_BOOTLOADER
> +        if (VIR_STRDUP(b_info->bootloader, def->os.bootloader) < 0)
> +            return -1;
> +        if (def->os.bootloaderArgs) {
> +            if (!(b_info->bootloader_args =
> +                  virStringSplit(def->os.bootloaderArgs, " \t\n", 0)))
> +                return -1;
> +        }
> +#endif

This is probably fine, but AFAIK no bootloaders currently support pvh. Juergen 
is working on support in grub2 but that seems to be a little ways off.

Regards,
Jim

>       } else {
>           /*
>            * For compatibility with the legacy xen toolstack, default to pygrub
> @@ -1230,7 +1268,7 @@ libxlMakeNic(virDomainDefPtr def,
>               STRNEQ(l_nic->model, "netfront")) {
>               virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                              _("only model 'netfront' is supported for "
> -                             "Xen PV domains"));
> +                             "Xen PV(H) domains"));
>               return -1;
>           }
>           if (VIR_STRDUP(x_nic->model, l_nic->model) < 0)
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index efd47a3..6287cef 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -6398,9 +6398,11 @@ libxlConnectGetDomainCapabilities(virConnectPtr conn,
>           emulatorbin = "/usr/bin/qemu-system-x86_64";
>   
>       if (machine) {
> -        if (STRNEQ(machine, "xenpv") && STRNEQ(machine, "xenfv")) {
> +        if (STRNEQ(machine, "xenpv") &&
> +                STRNEQ(machine, "xenpvh") &&
> +                STRNEQ(machine, "xenfv")) {
>               virReportError(VIR_ERR_INVALID_ARG, "%s",
> -                           _("Xen only supports 'xenpv' and 'xenfv' machines"));
> +                           _("Xen only supports 'xenpv', 'xenpvh' and 'xenfv' machines"));
>               goto cleanup;
>           }
>       } else {
> 

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

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Marek Marczykowski-Górecki 6 years ago

On Tue, Oct 09, 2018 at 04:45:01PM -0600, Jim Fehlig wrote:
> On 10/7/18 9:39 AM, Marek Marczykowski-Górecki wrote:
> > @@ -647,6 +669,22 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
> >               return -1;
> >           }
> >   #endif
> > +    } else if (pvh) {
> > +        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;
> > +#ifdef LIBXL_HAVE_BUILDINFO_BOOTLOADER
> > +        if (VIR_STRDUP(b_info->bootloader, def->os.bootloader) < 0)
> > +            return -1;
> > +        if (def->os.bootloaderArgs) {
> > +            if (!(b_info->bootloader_args =
> > +                  virStringSplit(def->os.bootloaderArgs, " \t\n", 0)))
> > +                return -1;
> > +        }
> > +#endif
> 
> This is probably fine, but AFAIK no bootloaders currently support pvh.
> Juergen is working on support in grub2 but that seems to be a little ways
> off.

This is independent of grub2 (which could be set as "kernel"), the
"bootloader" option here is about a program run _in dom0_ to find the
kernel to load, instead of providing a path directly (see xl.cfg(5)).
Like pygrub. And while I haven't tested it with PVH, I see no reason why
it shouldn't work.

-- 
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?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Jim Fehlig 6 years ago

On 10/9/18 5:04 PM, Marek Marczykowski-Górecki wrote:
> On Tue, Oct 09, 2018 at 04:45:01PM -0600, Jim Fehlig wrote:
>> On 10/7/18 9:39 AM, Marek Marczykowski-Górecki wrote:
>>> @@ -647,6 +669,22 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
>>>                return -1;
>>>            }
>>>    #endif
>>> +    } else if (pvh) {
>>> +        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;
>>> +#ifdef LIBXL_HAVE_BUILDINFO_BOOTLOADER
>>> +        if (VIR_STRDUP(b_info->bootloader, def->os.bootloader) < 0)
>>> +            return -1;
>>> +        if (def->os.bootloaderArgs) {
>>> +            if (!(b_info->bootloader_args =
>>> +                  virStringSplit(def->os.bootloaderArgs, " \t\n", 0)))
>>> +                return -1;
>>> +        }
>>> +#endif
>>
>> This is probably fine, but AFAIK no bootloaders currently support pvh.
>> Juergen is working on support in grub2 but that seems to be a little ways
>> off.
> 
> This is independent of grub2 (which could be set as "kernel"), the
> "bootloader" option here is about a program run _in dom0_ to find the
> kernel to load, instead of providing a path directly (see xl.cfg(5)).

Yes of course. I had grub2 on the mind since I had just talked to Juergen about 
the status.

> Like pygrub. And while I haven't tested it with PVH, I see no reason why
> it shouldn't work.

Yep, agreed. As long as you don't mind poking at VM images in dom0 is should 
just work.

Regards,
Jim

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

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Daniel P. Berrangé 6 years ago

On Tue, Oct 09, 2018 at 04:45:01PM -0600, Jim Fehlig wrote:
> On 10/7/18 9:39 AM, 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@invisiblethingslab.com>
> > ---
> > Changes in v2 proposed by Jim:
> >   - use new_arch_added var instead of i == nr_guest_archs for clarity
> >   - improve comment
> >   - adjust for now required Xen >= 4.6 (remove part for Xen < 4.5)
> > 
> > Changes in v3:
> >   - limit VIR_DOMAIN_OSTYPE_XEN -> VIR_DOMAIN_OSTYPE_LINUX conversion to
> >   Xen PV only
> >   - do not accept VIR_DOMAIN_OSTYPE_LINUX for PVH
> >   - fix reported capabilities for PVH - remove hostdev passthrough and
> >   video/graphics
> >   - use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to
> >   check for PVH support
> >   - compile fix for Xen < 4.9
> > 
> > Changes in v4:
> >   - revert to "i == nr_guest_archs-1" check
> >   - let configure check for LIBXL_DOMAIN_TYPE_PVH and use #ifdef
> >   HAVE_XEN_PVH then
> >   - fix comment about Xen version
> > ---
> >   docs/formatcaps.html.in        |  4 +--
> >   docs/schemas/domaincommon.rng  |  1 +-
> >   m4/virt-driver-libxl.m4        |  3 ++-
> >   src/conf/domain_conf.c         |  6 ++--
> >   src/libxl/libxl_capabilities.c | 38 ++++++++++++++++++++++-----
> >   src/libxl/libxl_conf.c         | 50 ++++++++++++++++++++++++++++++-----
> >   src/libxl/libxl_driver.c       |  6 ++--
> >   7 files changed, 90 insertions(+), 18 deletions(-)
> > 
> > diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in
> > index 0d9c53d..fe113bc 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 PVH.</dd>
> >               <dt><code>domain</code></dt><dd>The <code>type</code> attribute of
> >                 this element specifies the type of hypervisor required to run the
> >                 domain. Use in <a href="formatdomain.html#attributeDomainType">type</a>
> > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> > index 099a949..820a7c6 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/m4/virt-driver-libxl.m4 b/m4/virt-driver-libxl.m4
> > index 479d911..2cd97cc 100644
> > --- a/m4/virt-driver-libxl.m4
> > +++ b/m4/virt-driver-libxl.m4
> > @@ -75,6 +75,9 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_LIBXL], [
> >       ])
> >     fi
> > +  dnl Check if Xen has support for PVH
> > +  AC_CHECK_DECL(LIBXL_DOMAIN_TYPE_PVH, [AC_DEFINE([HAVE_XEN_PVH], [1], [Define to 1 if Xen has PVH support.])], [], [#include <libxl.h>])
> > +
> >     AC_SUBST([LIBXL_CFLAGS])
> >     AC_SUBST([LIBXL_LIBS])
> >   ])
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 9911d56..a945ad4 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -19094,7 +19094,8 @@ virDomainDefParseCaps(virDomainDefPtr def,
> >        * be 'xen'. So we accept the former and convert
> >        */
> >       if (def->os.type == VIR_DOMAIN_OSTYPE_LINUX &&
> > -        def->virtType == VIR_DOMAIN_VIRT_XEN) {
> > +        def->virtType == VIR_DOMAIN_VIRT_XEN &&
> > +        (!def->os.machine || STREQ(def->os.machine, "xenpv"))) {
> >           def->os.type = VIR_DOMAIN_OSTYPE_XEN;
> >       }
> > @@ -27705,7 +27706,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
> >        * be 'xen'. So we convert to the former for backcompat
> >        */
> >       if (def->virtType == VIR_DOMAIN_VIRT_XEN &&
> > -        def->os.type == VIR_DOMAIN_OSTYPE_XEN)
> > +        def->os.type == VIR_DOMAIN_OSTYPE_XEN &&
> > +        (!def->os.machine || STREQ(def->os.machine, "xenpv")))
> >           virBufferAsprintf(buf, ">%s</type>\n",
> >                             virDomainOSTypeToString(VIR_DOMAIN_OSTYPE_LINUX));
> >       else
> 
> I'd still like to hear what others think of extending the hack. mprivozn?
> You often have an interesting opinion :-).

IIUC, this means that we get this behaviour:

    Input                            Output

 type=linux, machine=NULL        type=linux, machine=NULL
 type=linux, machine=xenpv       type=linux, machine=xenpvh
 type=xen, machine=NULL          type=linux, machine=NULL
 type=xen, machine=xenpv         type=linux, machine=xenpv
 type=xen, machine=xenpvh        type=xen, machine=xenpvh

And one combo not allowed

 type=linux, machine=xenpvh

I can understand the motivation behind this change, but I wonder if it is
worth it or not to have the difference in behaviour.  I'm quite torn.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Michal Privoznik 6 years ago

On 10/10/2018 10:45 AM, Daniel P. Berrangé wrote:
> On Tue, Oct 09, 2018 at 04:45:01PM -0600, Jim Fehlig wrote:
>> On 10/7/18 9:39 AM, 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@invisiblethingslab.com>
>>> ---
>>> Changes in v2 proposed by Jim:
>>>   - use new_arch_added var instead of i == nr_guest_archs for clarity
>>>   - improve comment
>>>   - adjust for now required Xen >= 4.6 (remove part for Xen < 4.5)
>>>
>>> Changes in v3:
>>>   - limit VIR_DOMAIN_OSTYPE_XEN -> VIR_DOMAIN_OSTYPE_LINUX conversion to
>>>   Xen PV only
>>>   - do not accept VIR_DOMAIN_OSTYPE_LINUX for PVH
>>>   - fix reported capabilities for PVH - remove hostdev passthrough and
>>>   video/graphics
>>>   - use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to
>>>   check for PVH support
>>>   - compile fix for Xen < 4.9
>>>
>>> Changes in v4:
>>>   - revert to "i == nr_guest_archs-1" check
>>>   - let configure check for LIBXL_DOMAIN_TYPE_PVH and use #ifdef
>>>   HAVE_XEN_PVH then
>>>   - fix comment about Xen version
>>> ---
>>>   docs/formatcaps.html.in        |  4 +--
>>>   docs/schemas/domaincommon.rng  |  1 +-
>>>   m4/virt-driver-libxl.m4        |  3 ++-
>>>   src/conf/domain_conf.c         |  6 ++--
>>>   src/libxl/libxl_capabilities.c | 38 ++++++++++++++++++++++-----
>>>   src/libxl/libxl_conf.c         | 50 ++++++++++++++++++++++++++++++-----
>>>   src/libxl/libxl_driver.c       |  6 ++--
>>>   7 files changed, 90 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in
>>> index 0d9c53d..fe113bc 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 PVH.</dd>
>>>               <dt><code>domain</code></dt><dd>The <code>type</code> attribute of
>>>                 this element specifies the type of hypervisor required to run the
>>>                 domain. Use in <a href="formatdomain.html#attributeDomainType">type</a>
>>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>>> index 099a949..820a7c6 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/m4/virt-driver-libxl.m4 b/m4/virt-driver-libxl.m4
>>> index 479d911..2cd97cc 100644
>>> --- a/m4/virt-driver-libxl.m4
>>> +++ b/m4/virt-driver-libxl.m4
>>> @@ -75,6 +75,9 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_LIBXL], [
>>>       ])
>>>     fi
>>> +  dnl Check if Xen has support for PVH
>>> +  AC_CHECK_DECL(LIBXL_DOMAIN_TYPE_PVH, [AC_DEFINE([HAVE_XEN_PVH], [1], [Define to 1 if Xen has PVH support.])], [], [#include <libxl.h>])
>>> +
>>>     AC_SUBST([LIBXL_CFLAGS])
>>>     AC_SUBST([LIBXL_LIBS])
>>>   ])
>>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>>> index 9911d56..a945ad4 100644
>>> --- a/src/conf/domain_conf.c
>>> +++ b/src/conf/domain_conf.c
>>> @@ -19094,7 +19094,8 @@ virDomainDefParseCaps(virDomainDefPtr def,
>>>        * be 'xen'. So we accept the former and convert
>>>        */
>>>       if (def->os.type == VIR_DOMAIN_OSTYPE_LINUX &&
>>> -        def->virtType == VIR_DOMAIN_VIRT_XEN) {
>>> +        def->virtType == VIR_DOMAIN_VIRT_XEN &&
>>> +        (!def->os.machine || STREQ(def->os.machine, "xenpv"))) {
>>>           def->os.type = VIR_DOMAIN_OSTYPE_XEN;
>>>       }
>>> @@ -27705,7 +27706,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>>>        * be 'xen'. So we convert to the former for backcompat
>>>        */
>>>       if (def->virtType == VIR_DOMAIN_VIRT_XEN &&
>>> -        def->os.type == VIR_DOMAIN_OSTYPE_XEN)
>>> +        def->os.type == VIR_DOMAIN_OSTYPE_XEN &&
>>> +        (!def->os.machine || STREQ(def->os.machine, "xenpv")))
>>>           virBufferAsprintf(buf, ">%s</type>\n",
>>>                             virDomainOSTypeToString(VIR_DOMAIN_OSTYPE_LINUX));
>>>       else
>>
>> I'd still like to hear what others think of extending the hack. mprivozn?
>> You often have an interesting opinion :-).
> 
> IIUC, this means that we get this behaviour:
> 
>     Input                            Output
> 
>  type=linux, machine=NULL        type=linux, machine=NULL
>  type=linux, machine=xenpv       type=linux, machine=xenpvh

I think we would get type=xen, machine=xenpv, wouldn't we?

>  type=xen, machine=NULL          type=linux, machine=NULL
>  type=xen, machine=xenpv         type=linux, machine=xenpv
>  type=xen, machine=xenpvh        type=xen, machine=xenpvh
> 
> And one combo not allowed
> 
>  type=linux, machine=xenpvh
> 
> I can understand the motivation behind this change, but I wonder if it is
> worth it or not to have the difference in behaviour.  I'm quite torn.

I think we can do this. xenpvh falls into xen world and not Linux. IIRC
the question in previous versions was if it is safe to make decisions
based on machine type in post parse callbacks. Short answer is yes. The
longer answer is yes, but in driver specific callbacks.

I view code under src/conf/ to be hypervisor agnostic (at least it
should be) and thus any hypervisor specific decisions/changes to user
XML should be made from src/hypervisor/.

Anyway, maybe that was a bit off topic.

Michal

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

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Daniel P. Berrangé 6 years ago

On Wed, Oct 10, 2018 at 11:06:38AM +0200, Michal Privoznik wrote:
> On 10/10/2018 10:45 AM, Daniel P. Berrangé wrote:
> > On Tue, Oct 09, 2018 at 04:45:01PM -0600, Jim Fehlig wrote:
> >> On 10/7/18 9:39 AM, 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@invisiblethingslab.com>
> >>> ---
> >>> Changes in v2 proposed by Jim:
> >>>   - use new_arch_added var instead of i == nr_guest_archs for clarity
> >>>   - improve comment
> >>>   - adjust for now required Xen >= 4.6 (remove part for Xen < 4.5)
> >>>
> >>> Changes in v3:
> >>>   - limit VIR_DOMAIN_OSTYPE_XEN -> VIR_DOMAIN_OSTYPE_LINUX conversion to
> >>>   Xen PV only
> >>>   - do not accept VIR_DOMAIN_OSTYPE_LINUX for PVH
> >>>   - fix reported capabilities for PVH - remove hostdev passthrough and
> >>>   video/graphics
> >>>   - use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to
> >>>   check for PVH support
> >>>   - compile fix for Xen < 4.9
> >>>
> >>> Changes in v4:
> >>>   - revert to "i == nr_guest_archs-1" check
> >>>   - let configure check for LIBXL_DOMAIN_TYPE_PVH and use #ifdef
> >>>   HAVE_XEN_PVH then
> >>>   - fix comment about Xen version
> >>> ---
> >>>   docs/formatcaps.html.in        |  4 +--
> >>>   docs/schemas/domaincommon.rng  |  1 +-
> >>>   m4/virt-driver-libxl.m4        |  3 ++-
> >>>   src/conf/domain_conf.c         |  6 ++--
> >>>   src/libxl/libxl_capabilities.c | 38 ++++++++++++++++++++++-----
> >>>   src/libxl/libxl_conf.c         | 50 ++++++++++++++++++++++++++++++-----
> >>>   src/libxl/libxl_driver.c       |  6 ++--
> >>>   7 files changed, 90 insertions(+), 18 deletions(-)
> >>>
> >>> diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in
> >>> index 0d9c53d..fe113bc 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 PVH.</dd>
> >>>               <dt><code>domain</code></dt><dd>The <code>type</code> attribute of
> >>>                 this element specifies the type of hypervisor required to run the
> >>>                 domain. Use in <a href="formatdomain.html#attributeDomainType">type</a>
> >>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> >>> index 099a949..820a7c6 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/m4/virt-driver-libxl.m4 b/m4/virt-driver-libxl.m4
> >>> index 479d911..2cd97cc 100644
> >>> --- a/m4/virt-driver-libxl.m4
> >>> +++ b/m4/virt-driver-libxl.m4
> >>> @@ -75,6 +75,9 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_LIBXL], [
> >>>       ])
> >>>     fi
> >>> +  dnl Check if Xen has support for PVH
> >>> +  AC_CHECK_DECL(LIBXL_DOMAIN_TYPE_PVH, [AC_DEFINE([HAVE_XEN_PVH], [1], [Define to 1 if Xen has PVH support.])], [], [#include <libxl.h>])
> >>> +
> >>>     AC_SUBST([LIBXL_CFLAGS])
> >>>     AC_SUBST([LIBXL_LIBS])
> >>>   ])
> >>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> >>> index 9911d56..a945ad4 100644
> >>> --- a/src/conf/domain_conf.c
> >>> +++ b/src/conf/domain_conf.c
> >>> @@ -19094,7 +19094,8 @@ virDomainDefParseCaps(virDomainDefPtr def,
> >>>        * be 'xen'. So we accept the former and convert
> >>>        */
> >>>       if (def->os.type == VIR_DOMAIN_OSTYPE_LINUX &&
> >>> -        def->virtType == VIR_DOMAIN_VIRT_XEN) {
> >>> +        def->virtType == VIR_DOMAIN_VIRT_XEN &&
> >>> +        (!def->os.machine || STREQ(def->os.machine, "xenpv"))) {
> >>>           def->os.type = VIR_DOMAIN_OSTYPE_XEN;
> >>>       }
> >>> @@ -27705,7 +27706,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
> >>>        * be 'xen'. So we convert to the former for backcompat
> >>>        */
> >>>       if (def->virtType == VIR_DOMAIN_VIRT_XEN &&
> >>> -        def->os.type == VIR_DOMAIN_OSTYPE_XEN)
> >>> +        def->os.type == VIR_DOMAIN_OSTYPE_XEN &&
> >>> +        (!def->os.machine || STREQ(def->os.machine, "xenpv")))
> >>>           virBufferAsprintf(buf, ">%s</type>\n",
> >>>                             virDomainOSTypeToString(VIR_DOMAIN_OSTYPE_LINUX));
> >>>       else
> >>
> >> I'd still like to hear what others think of extending the hack. mprivozn?
> >> You often have an interesting opinion :-).
> > 
> > IIUC, this means that we get this behaviour:
> > 
> >     Input                            Output
> > 
> >  type=linux, machine=NULL        type=linux, machine=NULL
> >  type=linux, machine=xenpv       type=linux, machine=xenpvh
> 
> I think we would get type=xen, machine=xenpv, wouldn't we?

Sigh, yes, of course.

> 
> >  type=xen, machine=NULL          type=linux, machine=NULL
> >  type=xen, machine=xenpv         type=linux, machine=xenpv
> >  type=xen, machine=xenpvh        type=xen, machine=xenpvh
> > 
> > And one combo not allowed
> > 
> >  type=linux, machine=xenpvh
> > 
> > I can understand the motivation behind this change, but I wonder if it is
> > worth it or not to have the difference in behaviour.  I'm quite torn.
> 
> I think we can do this. xenpvh falls into xen world and not Linux. IIRC
> the question in previous versions was if it is safe to make decisions
> based on machine type in post parse callbacks. Short answer is yes. The
> longer answer is yes, but in driver specific callbacks.
> 
> I view code under src/conf/ to be hypervisor agnostic (at least it
> should be) and thus any hypervisor specific decisions/changes to user
> XML should be made from src/hypervisor/.

So we should move the hack to the post-parse callbacks in the xen
driver - but we don't have an equivalent pre-format callback for
the reverse hack, do we ?


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Peter Krempa 6 years ago

On Wed, Oct 10, 2018 at 10:14:44 +0100, Daniel Berrange wrote:
> On Wed, Oct 10, 2018 at 11:06:38AM +0200, Michal Privoznik wrote:
> > On 10/10/2018 10:45 AM, Daniel P. Berrangé wrote:

[...]

> > I view code under src/conf/ to be hypervisor agnostic (at least it
> > should be) and thus any hypervisor specific decisions/changes to user
> > XML should be made from src/hypervisor/.
> 
> So we should move the hack to the post-parse callbacks in the xen
> driver - but we don't have an equivalent pre-format callback for
> the reverse hack, do we ?

No we don't. Any hacks necessary are built into the formatter so that
they operate on temporary copies of the data.

Having anything that would modify the def on formatting would seem like
a bad idea since that can happen at random points. Doing a copy of the
def to do that would be expensive and also in current situation
pointless since copy is done via format->parse, so a post-parse gets
applied anyways there.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Jim Fehlig 5 years, 12 months ago

On 10/7/18 9:39 AM, 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@invisiblethingslab.com>
> ---
> Changes in v2 proposed by Jim:
>   - use new_arch_added var instead of i == nr_guest_archs for clarity
>   - improve comment
>   - adjust for now required Xen >= 4.6 (remove part for Xen < 4.5)
> 
> Changes in v3:
>   - limit VIR_DOMAIN_OSTYPE_XEN -> VIR_DOMAIN_OSTYPE_LINUX conversion to
>   Xen PV only
>   - do not accept VIR_DOMAIN_OSTYPE_LINUX for PVH
>   - fix reported capabilities for PVH - remove hostdev passthrough and
>   video/graphics
>   - use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to
>   check for PVH support
>   - compile fix for Xen < 4.9
> 
> Changes in v4:
>   - revert to "i == nr_guest_archs-1" check
>   - let configure check for LIBXL_DOMAIN_TYPE_PVH and use #ifdef
>   HAVE_XEN_PVH then
>   - fix comment about Xen version
> ---
>   docs/formatcaps.html.in        |  4 +--
>   docs/schemas/domaincommon.rng  |  1 +-
>   m4/virt-driver-libxl.m4        |  3 ++-
>   src/conf/domain_conf.c         |  6 ++--
>   src/libxl/libxl_capabilities.c | 38 ++++++++++++++++++++++-----
>   src/libxl/libxl_conf.c         | 50 ++++++++++++++++++++++++++++++-----
>   src/libxl/libxl_driver.c       |  6 ++--
>   7 files changed, 90 insertions(+), 18 deletions(-)
> 
> diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in
> index 0d9c53d..fe113bc 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 PVH.</dd>
>               <dt><code>domain</code></dt><dd>The <code>type</code> attribute of
>                 this element specifies the type of hypervisor required to run the
>                 domain. Use in <a href="formatdomain.html#attributeDomainType">type</a>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 099a949..820a7c6 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/m4/virt-driver-libxl.m4 b/m4/virt-driver-libxl.m4
> index 479d911..2cd97cc 100644
> --- a/m4/virt-driver-libxl.m4
> +++ b/m4/virt-driver-libxl.m4
> @@ -75,6 +75,9 @@ AC_DEFUN([LIBVIRT_DRIVER_CHECK_LIBXL], [
>       ])
>     fi
>   
> +  dnl Check if Xen has support for PVH
> +  AC_CHECK_DECL(LIBXL_DOMAIN_TYPE_PVH, [AC_DEFINE([HAVE_XEN_PVH], [1], [Define to 1 if Xen has PVH support.])], [], [#include <libxl.h>])

Nice! I was unable to get this to work, probably due to some botched quoting.

> +
>     AC_SUBST([LIBXL_CFLAGS])
>     AC_SUBST([LIBXL_LIBS])
>   ])
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 9911d56..a945ad4 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -19094,7 +19094,8 @@ virDomainDefParseCaps(virDomainDefPtr def,
>        * be 'xen'. So we accept the former and convert
>        */
>       if (def->os.type == VIR_DOMAIN_OSTYPE_LINUX &&
> -        def->virtType == VIR_DOMAIN_VIRT_XEN) {
> +        def->virtType == VIR_DOMAIN_VIRT_XEN &&
> +        (!def->os.machine || STREQ(def->os.machine, "xenpv"))) {
>           def->os.type = VIR_DOMAIN_OSTYPE_XEN;
>       }
>   
> @@ -27705,7 +27706,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>        * be 'xen'. So we convert to the former for backcompat
>        */
>       if (def->virtType == VIR_DOMAIN_VIRT_XEN &&
> -        def->os.type == VIR_DOMAIN_OSTYPE_XEN)
> +        def->os.type == VIR_DOMAIN_OSTYPE_XEN &&
> +        (!def->os.machine || STREQ(def->os.machine, "xenpv")))
>           virBufferAsprintf(buf, ">%s</type>\n",
>                             virDomainOSTypeToString(VIR_DOMAIN_OSTYPE_LINUX));
>       else
> diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c
> index 18596c7..eecd5e9 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,33 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
>                   guest_archs[i].nonpae = nonpae;
>               if (ia64_be)
>                   guest_archs[i].ia64_be = ia64_be;
> +
> +            /*
> +             * Xen 4.10 introduced support for the PVH guest type, which
> +             * requires hardware virtualization support similar to the
> +             * HVM guest type. Add a PVH guest type for each new HVM
> +             * guest type.
> +             */
> +#ifdef HAVE_XEN_PVH
> +            if (hvm && i == nr_guest_archs-1) {
> +                i = nr_guest_archs;
> +                /* Ensure we have not exhausted the guest_archs array */
> +                if (nr_guest_archs >= ARRAY_CARDINALITY(guest_archs))
> +                    continue;
> +                guest_archs[nr_guest_archs].arch = arch;
> +                guest_archs[nr_guest_archs].pvh = 1;
> +                nr_guest_archs++;
> +            }
> +#endif
>           }
>       }
>       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")};

I had some couch time at the start of the weekend and was finally able to try 
using this series with virt-install. As it turns out, reporting duplicate 
<guest> blocks for <os_type> 'xen' is not quite right. Instead we will want to 
report the additional <machine> under the existing 'xen' <guest> blocks. E.g. 
instead of adding a duplicate

   <guest>
     <os_type>xen</os_type>
     <arch name='x86_64'>
       <wordsize>64</wordsize>
       <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator>
       <machine>xenpvh</machine>
       <domain type='xen'/>
     </arch>
     ...
   </guest>

we'll want to include the xenpvh machine in the existing <guest> config for xen

   <guest>
     <os_type>xen</os_type>
     <arch name='x86_64'>
       <wordsize>64</wordsize>
       <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator>
       <machine>xenpvh</machine>
       <machine>xenpv</machine>
       <domain type='xen'/>
     </arch>
   </guest>

With this change to the capabilities reporting, virt-install works without 
modification using

virt-install --paravirt --machine xenpv ...

I didn't think too hard about the best way to handle this, but the attached diff 
is a POC hack that works with unmodified virt-install.

Regards,
Jim

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

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Marek Marczykowski-Górecki 5 years, 11 months ago

On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
> I had some couch time at the start of the weekend and was finally able to
> try using this series with virt-install. As it turns out, reporting
> duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we
> will want to report the additional <machine> under the existing 'xen'
> <guest> blocks. E.g. instead of adding a duplicate
> 
>   <guest>
>     <os_type>xen</os_type>
>     <arch name='x86_64'>
>       <wordsize>64</wordsize>
>       <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator>
>       <machine>xenpvh</machine>
>       <domain type='xen'/>
>     </arch>
>     ...
>   </guest>
> 
> we'll want to include the xenpvh machine in the existing <guest> config for xen
> 
>   <guest>
>     <os_type>xen</os_type>
>     <arch name='x86_64'>
>       <wordsize>64</wordsize>
>       <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator>
>       <machine>xenpvh</machine>
>       <machine>xenpv</machine>
>       <domain type='xen'/>
>     </arch>
>   </guest>
> 
> With this change to the capabilities reporting, virt-install works without
> modification using
> 
> virt-install --paravirt --machine xenpv ...
> 
> I didn't think too hard about the best way to handle this, but the attached
> diff is a POC hack that works with unmodified virt-install.

I can get it reported this way, but it will be wrong, because <features>
will be incorrectly reported. For example hap should be reported for
xenpvh, but not for xenpv, so bundling them together makes it
impossible. Similar for acpi and probably others too.

If this really needs to be reported together, I'd go with reporting
superset of features, so os_type xen entry will have all features of PV
and PVH. But then, it should probably reject PV features for PVH
machine somewhere - in post parse hook? Or maybe it should forcibly
remove them - for example I see PAE is forcibly enabled for 64bit HVM
guests.

-- 
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?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Jim Fehlig 5 years, 11 months ago

On 10/16/18 7:23 PM, Marek Marczykowski-Górecki wrote:
> On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
>> I had some couch time at the start of the weekend and was finally able to
>> try using this series with virt-install. As it turns out, reporting
>> duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we
>> will want to report the additional <machine> under the existing 'xen'
>> <guest> blocks. E.g. instead of adding a duplicate
>>
>>    <guest>
>>      <os_type>xen</os_type>
>>      <arch name='x86_64'>
>>        <wordsize>64</wordsize>
>>        <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator>
>>        <machine>xenpvh</machine>
>>        <domain type='xen'/>
>>      </arch>
>>      ...
>>    </guest>
>>
>> we'll want to include the xenpvh machine in the existing <guest> config for xen
>>
>>    <guest>
>>      <os_type>xen</os_type>
>>      <arch name='x86_64'>
>>        <wordsize>64</wordsize>
>>        <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator>
>>        <machine>xenpvh</machine>
>>        <machine>xenpv</machine>
>>        <domain type='xen'/>
>>      </arch>
>>    </guest>
>>
>> With this change to the capabilities reporting, virt-install works without
>> modification using
>>
>> virt-install --paravirt --machine xenpv ...
>>
>> I didn't think too hard about the best way to handle this, but the attached
>> diff is a POC hack that works with unmodified virt-install.
> 
> I can get it reported this way, but it will be wrong, because <features>
> will be incorrectly reported. For example hap should be reported for
> xenpvh, but not for xenpv, so bundling them together makes it
> impossible. Similar for acpi and probably others too.

Yes, you are correct :-(. Modeling PVH has been more of a PITA than I expected.

> 
> If this really needs to be reported together, I'd go with reporting
> superset of features, so os_type xen entry will have all features of PV
> and PVH.

Reporting features that cannot possibly be supported doesn't see right. Let's 
summarize what we've learned thus far and see if that helps with modeling PVH.

PVH is a new xen machine type that is a hybrid of PV and HVM. Like HVM, PVH 
requires hardware virtualization support. Like PV, PVH requires a modified guest 
kernel, and one that is modified differently than PV (e.g. CONFIG_XEN_PVH vs
CONFIG_XEN_PV in the linux kernel). PV and PVH have different feature sets. E.g. 
PVH supports HAP, ACPI, etc., but PV does not. (Perhaps another useful data 
point: the long term goal of the xen community is to remove CONFIG_XEN_PV, 
essentially making PVH the new PV from xen perspective, but that is obviously a 
long ways out.)

Currently in libvirt, PV is modeled as VIR_DOMAIN_OSTYPE_XEN and machine 
"xenpv". HVM is modeled as VIR_DOMAIN_OSTYPE_HVM and machine "xenfv". For PVH 
we've considered VIR_DOMAIN_OSTYPE_XENPVH with machine "xenpvh", or simply 
adding PVH as machine "xenpvh" under existing VIR_DOMAIN_OSTYPE_XEN.

I've pushed for adding a new machine "xenpvh" under existing 
VIR_DOMAIN_OSTYPE_XEN with primary reason that both are OS types modified to run 
on xen. Also existing tools like virt-{install,manager} know how to handle 
OSTYPE_{HVM,XEN} and machines, allowing them to support PVH without (or with 
minimal) modification.

Have I been narrow-minded with my "both are OS types modified to run on xen" 
reasoning for using VIR_DOMAIN_OSTYPE_XEN? Should we really consider PVH a new 
OSTYPE? Your reminder that PV and PVH have a different feature set hints to 
modeling PVH as VIR_DOMAIN_OSTYPE_XENPVH. It is unfortunate that tools above 
libvirt would have to be taught about this new OSTYPE, but that shouldn't 
prevent using VIR_DOMAIN_OSTYPE_XENPVH if in fact it is the best way to model PVH.

Sadly I haven't strongly convinced myself one way or the other. I still like the 
idea of reusing VIR_DOMAIN_OSTYPE_XEN and adding machine "xenpvh" since it seems 
easier from an app perspective, but maybe I just need slapped and admit that PVH 
is a new OSTYPE. (I'm sure you'd like to do the slapping at this point...)

Regards,
Jim

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

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Marek Marczykowski-Górecki 5 years, 11 months ago

On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
> I had some couch time at the start of the weekend and was finally able to
> try using this series with virt-install. As it turns out, reporting
> duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we
> will want to report the additional <machine> under the existing 'xen'
> <guest> blocks. 

Is that virt-install limitation? In that case, IMO virt-install should
be fixed, instead of changing capabilities xml to match its limitations.

-- 
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?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Jim Fehlig 5 years, 11 months ago

On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote:
> On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
>> I had some couch time at the start of the weekend and was finally able to
>> try using this series with virt-install. As it turns out, reporting
>> duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we
>> will want to report the additional <machine> under the existing 'xen'
>> <guest> blocks.
> 
> Is that virt-install limitation? In that case, IMO virt-install should
> be fixed, instead of changing capabilities xml to match its limitations.

Perhaps it is a virt-install limitation, but my suggestion was based more on how 
the qemu driver reports the different machines

<guest>
   <os_type>hvm</os_type>
   <arch name='x86_64'>
     <wordsize>64</wordsize>
     <emulator>/usr/bin/qemu-system-x86_64</emulator>
     <machine maxCpus='255'>pc-i440fx-3.0</machine>
     <machine maxCpus='288'>pc-q35-3.0</machine>
     ...
   </arch>
</guest>

Compare that with reporting PV and PVH in different <guest> blocks, where the 
<os_type> and <arch> are the same. It seems confusing from a consumers POV

<guest>
   <os_type>xen</os_type>
   <arch name='x86_64'>
     <wordsize>64</wordsize>
     <emulator>/usr/bin/qemu-system-x86_64</emulator>
     <machine>xenpv</machine>
   </arch>
</guest>

<guest>
   <os_type>xen</os_type>
   <arch name='x86_64'>
     <wordsize>64</wordsize>
     <emulator>/usr/bin/qemu-system-x86_64</emulator>
     <machine>xenpvh</machine>
   </arch>
</guest>

How should a consumer interpret that? Is the machine for os_type=xen, 
arch=x86_64 a xenpv or a xenpvh?

Regards,
Jim

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

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Marek Marczykowski-Górecki 5 years, 11 months ago

On Thu, Oct 18, 2018 at 11:08:34AM -0600, Jim Fehlig wrote:
> On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote:
> > On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
> > > I had some couch time at the start of the weekend and was finally able to
> > > try using this series with virt-install. As it turns out, reporting
> > > duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we
> > > will want to report the additional <machine> under the existing 'xen'
> > > <guest> blocks.
> > 
> > Is that virt-install limitation? In that case, IMO virt-install should
> > be fixed, instead of changing capabilities xml to match its limitations.
> 
> Perhaps it is a virt-install limitation, but my suggestion was based more on
> how the qemu driver reports the different machines
> 
> <guest>
>   <os_type>hvm</os_type>
>   <arch name='x86_64'>
>     <wordsize>64</wordsize>
>     <emulator>/usr/bin/qemu-system-x86_64</emulator>
>     <machine maxCpus='255'>pc-i440fx-3.0</machine>
>     <machine maxCpus='288'>pc-q35-3.0</machine>
>     ...
>   </arch>
> </guest>
> 
> Compare that with reporting PV and PVH in different <guest> blocks, where
> the <os_type> and <arch> are the same. It seems confusing from a consumers
> POV
> 
> <guest>
>   <os_type>xen</os_type>
>   <arch name='x86_64'>
>     <wordsize>64</wordsize>
>     <emulator>/usr/bin/qemu-system-x86_64</emulator>
>     <machine>xenpv</machine>
>   </arch>
> </guest>
> 
> <guest>
>   <os_type>xen</os_type>
>   <arch name='x86_64'>
>     <wordsize>64</wordsize>
>     <emulator>/usr/bin/qemu-system-x86_64</emulator>
>     <machine>xenpvh</machine>
>   </arch>
> </guest>
> 
> How should a consumer interpret that? Is the machine for os_type=xen,
> arch=x86_64 a xenpv or a xenpvh?

I don't see a problem - each guest block represent set of possible
configurations. Given the current structure, you could also ask "is
the os_type for arch=x86_64 a xen or a hvm?". Both are valid, with
possibly different set of features available. And the same goes for
xenpv and xenpvh machines.
Actually, I see qemu had similar problem as we have now with some features
being specific to some machine value - maxCpus. And as solution, it was
put in machine's attributes. But I think this approach is short-sighted.

-- 
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?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Jim Fehlig 5 years, 11 months ago

On 10/18/18 11:35 AM, Marek Marczykowski-Górecki wrote:
> On Thu, Oct 18, 2018 at 11:08:34AM -0600, Jim Fehlig wrote:
>> On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote:
>>> On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
>>>> I had some couch time at the start of the weekend and was finally able to
>>>> try using this series with virt-install. As it turns out, reporting
>>>> duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we
>>>> will want to report the additional <machine> under the existing 'xen'
>>>> <guest> blocks.
>>>
>>> Is that virt-install limitation? In that case, IMO virt-install should
>>> be fixed, instead of changing capabilities xml to match its limitations.
>>
>> Perhaps it is a virt-install limitation, but my suggestion was based more on
>> how the qemu driver reports the different machines
>>
>> <guest>
>>    <os_type>hvm</os_type>
>>    <arch name='x86_64'>
>>      <wordsize>64</wordsize>
>>      <emulator>/usr/bin/qemu-system-x86_64</emulator>
>>      <machine maxCpus='255'>pc-i440fx-3.0</machine>
>>      <machine maxCpus='288'>pc-q35-3.0</machine>
>>      ...
>>    </arch>
>> </guest>
>>
>> Compare that with reporting PV and PVH in different <guest> blocks, where
>> the <os_type> and <arch> are the same. It seems confusing from a consumers
>> POV
>>
>> <guest>
>>    <os_type>xen</os_type>
>>    <arch name='x86_64'>
>>      <wordsize>64</wordsize>
>>      <emulator>/usr/bin/qemu-system-x86_64</emulator>
>>      <machine>xenpv</machine>
>>    </arch>
>> </guest>
>>
>> <guest>
>>    <os_type>xen</os_type>
>>    <arch name='x86_64'>
>>      <wordsize>64</wordsize>
>>      <emulator>/usr/bin/qemu-system-x86_64</emulator>
>>      <machine>xenpvh</machine>
>>    </arch>
>> </guest>
>>
>> How should a consumer interpret that? Is the machine for os_type=xen,
>> arch=x86_64 a xenpv or a xenpvh?
> 
> I don't see a problem - each guest block represent set of possible
> configurations. Given the current structure, you could also ask "is
> the os_type for arch=x86_64 a xen or a hvm?". Both are valid, with
> possibly different set of features available. And the same goes for
> xenpv and xenpvh machines.

Right, it is not a problem. I've not been super confident in our modeling choice 
and keep coming up with lame reasons while VIR_DOMAIN_OSTYPE_XENPVH might be a 
better approach. But it is time for me to stop talking in circles and commit 
this series. VIR_DOMAIN_OSTYPE_XENPV and machine xenpvh still feels like the 
best approach and no one has flat out objected to that. We can always adjust the 
capabilities reporting later if we feel there is a better way to do it.

> Actually, I see qemu had similar problem as we have now with some features
> being specific to some machine value - maxCpus. And as solution, it was
> put in machine's attributes. But I think this approach is short-sighted.

Agreed, we can't just keep adding attributes. Seems a better approach would be 
<features> for each <machine>, but that is beyond the scope of this series.

Regards,
Jim

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

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Daniel P. Berrangé 5 years, 11 months ago

On Thu, Oct 18, 2018 at 11:08:34AM -0600, Jim Fehlig wrote:
> On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote:
> > On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
> > > I had some couch time at the start of the weekend and was finally able to
> > > try using this series with virt-install. As it turns out, reporting
> > > duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we
> > > will want to report the additional <machine> under the existing 'xen'
> > > <guest> blocks.
> > 
> > Is that virt-install limitation? In that case, IMO virt-install should
> > be fixed, instead of changing capabilities xml to match its limitations.
> 
> Perhaps it is a virt-install limitation, but my suggestion was based more on
> how the qemu driver reports the different machines
> 
> <guest>
>   <os_type>hvm</os_type>
>   <arch name='x86_64'>
>     <wordsize>64</wordsize>
>     <emulator>/usr/bin/qemu-system-x86_64</emulator>
>     <machine maxCpus='255'>pc-i440fx-3.0</machine>
>     <machine maxCpus='288'>pc-q35-3.0</machine>
>     ...
>   </arch>
> </guest>
> 
> Compare that with reporting PV and PVH in different <guest> blocks, where
> the <os_type> and <arch> are the same. It seems confusing from a consumers
> POV
> 
> <guest>
>   <os_type>xen</os_type>
>   <arch name='x86_64'>
>     <wordsize>64</wordsize>
>     <emulator>/usr/bin/qemu-system-x86_64</emulator>
>     <machine>xenpv</machine>
>   </arch>
> </guest>
> 
> <guest>
>   <os_type>xen</os_type>
>   <arch name='x86_64'>
>     <wordsize>64</wordsize>
>     <emulator>/usr/bin/qemu-system-x86_64</emulator>
>     <machine>xenpvh</machine>
>   </arch>
> </guest>
> 
> How should a consumer interpret that? Is the machine for os_type=xen,
> arch=x86_64 a xenpv or a xenpvh?

Yes, you are right - any pair of (os_type, arch) should be unique
in the capabilities XML. So all machines should be reported in the
same block.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Jim Fehlig 5 years, 11 months ago

On 10/19/18 3:11 AM, Daniel P. Berrangé wrote:
> On Thu, Oct 18, 2018 at 11:08:34AM -0600, Jim Fehlig wrote:
>> On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote:
>>> On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
>>>> I had some couch time at the start of the weekend and was finally able to
>>>> try using this series with virt-install. As it turns out, reporting
>>>> duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we
>>>> will want to report the additional <machine> under the existing 'xen'
>>>> <guest> blocks.
>>>
>>> Is that virt-install limitation? In that case, IMO virt-install should
>>> be fixed, instead of changing capabilities xml to match its limitations.
>>
>> Perhaps it is a virt-install limitation, but my suggestion was based more on
>> how the qemu driver reports the different machines
>>
>> <guest>
>>    <os_type>hvm</os_type>
>>    <arch name='x86_64'>
>>      <wordsize>64</wordsize>
>>      <emulator>/usr/bin/qemu-system-x86_64</emulator>
>>      <machine maxCpus='255'>pc-i440fx-3.0</machine>
>>      <machine maxCpus='288'>pc-q35-3.0</machine>
>>      ...
>>    </arch>
>> </guest>
>>
>> Compare that with reporting PV and PVH in different <guest> blocks, where
>> the <os_type> and <arch> are the same. It seems confusing from a consumers
>> POV
>>
>> <guest>
>>    <os_type>xen</os_type>
>>    <arch name='x86_64'>
>>      <wordsize>64</wordsize>
>>      <emulator>/usr/bin/qemu-system-x86_64</emulator>
>>      <machine>xenpv</machine>
>>    </arch>
>> </guest>
>>
>> <guest>
>>    <os_type>xen</os_type>
>>    <arch name='x86_64'>
>>      <wordsize>64</wordsize>
>>      <emulator>/usr/bin/qemu-system-x86_64</emulator>
>>      <machine>xenpvh</machine>
>>    </arch>
>> </guest>
>>
>> How should a consumer interpret that? Is the machine for os_type=xen,
>> arch=x86_64 a xenpv or a xenpvh?
> 
> Yes, you are right - any pair of (os_type, arch) should be unique
> in the capabilities XML. So all machines should be reported in the
> same block.

Our difficulty with that is xenpv and xenpvh machines have different features. 
Marek pointed out that the qemu driver reports the "feature" maxCpus as an 
attribute on the machine element, but we're hesitant to keep adding attributes 
for each feature that is unique to a machine.

Another option we discussed was reporting a superset of all features so that 
e.g. (xen, x86_64) block would contain features supported by both PV and PVH and 
then rejecting unsupported features when parsing domXML or starting the VM. This 
option is rather distasteful.

And we also have the option of adding VIR_DOMAIN_OSTYPE_XENPVH, which I've shied 
away from but may be a better way to go in the end. Do you have any suggestions 
we may have overlooked?

Regards,
Jim

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

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Daniel P. Berrangé 5 years, 11 months ago

On Fri, Oct 19, 2018 at 08:06:18AM -0600, Jim Fehlig wrote:
> On 10/19/18 3:11 AM, Daniel P. Berrangé wrote:
> > On Thu, Oct 18, 2018 at 11:08:34AM -0600, Jim Fehlig wrote:
> > > On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote:
> > > > On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
> > > > > I had some couch time at the start of the weekend and was finally able to
> > > > > try using this series with virt-install. As it turns out, reporting
> > > > > duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we
> > > > > will want to report the additional <machine> under the existing 'xen'
> > > > > <guest> blocks.
> > > > 
> > > > Is that virt-install limitation? In that case, IMO virt-install should
> > > > be fixed, instead of changing capabilities xml to match its limitations.
> > > 
> > > Perhaps it is a virt-install limitation, but my suggestion was based more on
> > > how the qemu driver reports the different machines
> > > 
> > > <guest>
> > >    <os_type>hvm</os_type>
> > >    <arch name='x86_64'>
> > >      <wordsize>64</wordsize>
> > >      <emulator>/usr/bin/qemu-system-x86_64</emulator>
> > >      <machine maxCpus='255'>pc-i440fx-3.0</machine>
> > >      <machine maxCpus='288'>pc-q35-3.0</machine>
> > >      ...
> > >    </arch>
> > > </guest>
> > > 
> > > Compare that with reporting PV and PVH in different <guest> blocks, where
> > > the <os_type> and <arch> are the same. It seems confusing from a consumers
> > > POV
> > > 
> > > <guest>
> > >    <os_type>xen</os_type>
> > >    <arch name='x86_64'>
> > >      <wordsize>64</wordsize>
> > >      <emulator>/usr/bin/qemu-system-x86_64</emulator>
> > >      <machine>xenpv</machine>
> > >    </arch>
> > > </guest>
> > > 
> > > <guest>
> > >    <os_type>xen</os_type>
> > >    <arch name='x86_64'>
> > >      <wordsize>64</wordsize>
> > >      <emulator>/usr/bin/qemu-system-x86_64</emulator>
> > >      <machine>xenpvh</machine>
> > >    </arch>
> > > </guest>
> > > 
> > > How should a consumer interpret that? Is the machine for os_type=xen,
> > > arch=x86_64 a xenpv or a xenpvh?
> > 
> > Yes, you are right - any pair of (os_type, arch) should be unique
> > in the capabilities XML. So all machines should be reported in the
> > same block.
> 
> Our difficulty with that is xenpv and xenpvh machines have different
> features. Marek pointed out that the qemu driver reports the "feature"
> maxCpus as an attribute on the machine element, but we're hesitant to keep
> adding attributes for each feature that is unique to a machine.
> 
> Another option we discussed was reporting a superset of all features so that
> e.g. (xen, x86_64) block would contain features supported by both PV and PVH
> and then rejecting unsupported features when parsing domXML or starting the
> VM. This option is rather distasteful.
> 
> And we also have the option of adding VIR_DOMAIN_OSTYPE_XENPVH, which I've
> shied away from but may be a better way to go in the end. Do you have any
> suggestions we may have overlooked?

Oooh, it looks like i've been mis-understanding PVH in all my previous
reviews.

I thought it was simply a "normal" Xen paravirtualized guest kernel. ie
any 'pv' guest is also a valid 'pvh' guest. Looking at the docs

  https://wiki.xen.org/wiki/Xen_Project_Software_Overview#Guest_Types

It appears I was wrong. It says a pvh guest kernel relies on hardware
virt extensions for part of its work and paravirt for other parts. So
really is a hybrid between pv and hvm.

With that in mind, we should indeed have a distinct OS type constant
to express this.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Jim Fehlig 5 years, 11 months ago

On 10/19/18 8:14 AM, Daniel P. Berrangé wrote:
> On Fri, Oct 19, 2018 at 08:06:18AM -0600, Jim Fehlig wrote:
>> On 10/19/18 3:11 AM, Daniel P. Berrangé wrote:
>>> On Thu, Oct 18, 2018 at 11:08:34AM -0600, Jim Fehlig wrote:
>>>> On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote:
>>>>> On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
>>>>>> I had some couch time at the start of the weekend and was finally able to
>>>>>> try using this series with virt-install. As it turns out, reporting
>>>>>> duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we
>>>>>> will want to report the additional <machine> under the existing 'xen'
>>>>>> <guest> blocks.
>>>>>
>>>>> Is that virt-install limitation? In that case, IMO virt-install should
>>>>> be fixed, instead of changing capabilities xml to match its limitations.
>>>>
>>>> Perhaps it is a virt-install limitation, but my suggestion was based more on
>>>> how the qemu driver reports the different machines
>>>>
>>>> <guest>
>>>>     <os_type>hvm</os_type>
>>>>     <arch name='x86_64'>
>>>>       <wordsize>64</wordsize>
>>>>       <emulator>/usr/bin/qemu-system-x86_64</emulator>
>>>>       <machine maxCpus='255'>pc-i440fx-3.0</machine>
>>>>       <machine maxCpus='288'>pc-q35-3.0</machine>
>>>>       ...
>>>>     </arch>
>>>> </guest>
>>>>
>>>> Compare that with reporting PV and PVH in different <guest> blocks, where
>>>> the <os_type> and <arch> are the same. It seems confusing from a consumers
>>>> POV
>>>>
>>>> <guest>
>>>>     <os_type>xen</os_type>
>>>>     <arch name='x86_64'>
>>>>       <wordsize>64</wordsize>
>>>>       <emulator>/usr/bin/qemu-system-x86_64</emulator>
>>>>       <machine>xenpv</machine>
>>>>     </arch>
>>>> </guest>
>>>>
>>>> <guest>
>>>>     <os_type>xen</os_type>
>>>>     <arch name='x86_64'>
>>>>       <wordsize>64</wordsize>
>>>>       <emulator>/usr/bin/qemu-system-x86_64</emulator>
>>>>       <machine>xenpvh</machine>
>>>>     </arch>
>>>> </guest>
>>>>
>>>> How should a consumer interpret that? Is the machine for os_type=xen,
>>>> arch=x86_64 a xenpv or a xenpvh?
>>>
>>> Yes, you are right - any pair of (os_type, arch) should be unique
>>> in the capabilities XML. So all machines should be reported in the
>>> same block.
>>
>> Our difficulty with that is xenpv and xenpvh machines have different
>> features. Marek pointed out that the qemu driver reports the "feature"
>> maxCpus as an attribute on the machine element, but we're hesitant to keep
>> adding attributes for each feature that is unique to a machine.
>>
>> Another option we discussed was reporting a superset of all features so that
>> e.g. (xen, x86_64) block would contain features supported by both PV and PVH
>> and then rejecting unsupported features when parsing domXML or starting the
>> VM. This option is rather distasteful.
>>
>> And we also have the option of adding VIR_DOMAIN_OSTYPE_XENPVH, which I've
>> shied away from but may be a better way to go in the end. Do you have any
>> suggestions we may have overlooked?
> 
> Oooh, it looks like i've been mis-understanding PVH in all my previous
> reviews.
> 
> I thought it was simply a "normal" Xen paravirtualized guest kernel. ie
> any 'pv' guest is also a valid 'pvh' guest. Looking at the docs
> 
>    https://wiki.xen.org/wiki/Xen_Project_Software_Overview#Guest_Types
> 
> It appears I was wrong. It says a pvh guest kernel relies on hardware
> virt extensions for part of its work and paravirt for other parts. So
> really is a hybrid between pv and hvm.

Right. The Xen wiki also has a good writeup about the various guest types

https://wiki.xenproject.org/wiki/Understanding_the_Virtualization_Spectrum

> With that in mind, we should indeed have a distinct OS type constant
> to express this.

There have been some long threads in the various versions of this series with a 
lot of waffling :-). I made a few attempts at summarizing what we learned about 
PV vs PVH but could never build a strong case (at least in my own head) for 
either of the two modeling approaches

https://www.redhat.com/archives/libvir-list/2018-October/msg00214.html
https://www.redhat.com/archives/libvir-list/2018-October/msg00891.html

Regards,
Jim

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

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Daniel P. Berrangé 5 years, 11 months ago

On Fri, Oct 19, 2018 at 08:53:15AM -0600, Jim Fehlig wrote:
> On 10/19/18 8:14 AM, Daniel P. Berrangé wrote:
> > On Fri, Oct 19, 2018 at 08:06:18AM -0600, Jim Fehlig wrote:
> > > On 10/19/18 3:11 AM, Daniel P. Berrangé wrote:
> > > > On Thu, Oct 18, 2018 at 11:08:34AM -0600, Jim Fehlig wrote:
> > > > > On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote:
> > > > > > On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
> > > > > > > I had some couch time at the start of the weekend and was finally able to
> > > > > > > try using this series with virt-install. As it turns out, reporting
> > > > > > > duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we
> > > > > > > will want to report the additional <machine> under the existing 'xen'
> > > > > > > <guest> blocks.
> > > > > > 
> > > > > > Is that virt-install limitation? In that case, IMO virt-install should
> > > > > > be fixed, instead of changing capabilities xml to match its limitations.
> > > > > 
> > > > > Perhaps it is a virt-install limitation, but my suggestion was based more on
> > > > > how the qemu driver reports the different machines
> > > > > 
> > > > > <guest>
> > > > >     <os_type>hvm</os_type>
> > > > >     <arch name='x86_64'>
> > > > >       <wordsize>64</wordsize>
> > > > >       <emulator>/usr/bin/qemu-system-x86_64</emulator>
> > > > >       <machine maxCpus='255'>pc-i440fx-3.0</machine>
> > > > >       <machine maxCpus='288'>pc-q35-3.0</machine>
> > > > >       ...
> > > > >     </arch>
> > > > > </guest>
> > > > > 
> > > > > Compare that with reporting PV and PVH in different <guest> blocks, where
> > > > > the <os_type> and <arch> are the same. It seems confusing from a consumers
> > > > > POV
> > > > > 
> > > > > <guest>
> > > > >     <os_type>xen</os_type>
> > > > >     <arch name='x86_64'>
> > > > >       <wordsize>64</wordsize>
> > > > >       <emulator>/usr/bin/qemu-system-x86_64</emulator>
> > > > >       <machine>xenpv</machine>
> > > > >     </arch>
> > > > > </guest>
> > > > > 
> > > > > <guest>
> > > > >     <os_type>xen</os_type>
> > > > >     <arch name='x86_64'>
> > > > >       <wordsize>64</wordsize>
> > > > >       <emulator>/usr/bin/qemu-system-x86_64</emulator>
> > > > >       <machine>xenpvh</machine>
> > > > >     </arch>
> > > > > </guest>
> > > > > 
> > > > > How should a consumer interpret that? Is the machine for os_type=xen,
> > > > > arch=x86_64 a xenpv or a xenpvh?
> > > > 
> > > > Yes, you are right - any pair of (os_type, arch) should be unique
> > > > in the capabilities XML. So all machines should be reported in the
> > > > same block.
> > > 
> > > Our difficulty with that is xenpv and xenpvh machines have different
> > > features. Marek pointed out that the qemu driver reports the "feature"
> > > maxCpus as an attribute on the machine element, but we're hesitant to keep
> > > adding attributes for each feature that is unique to a machine.
> > > 
> > > Another option we discussed was reporting a superset of all features so that
> > > e.g. (xen, x86_64) block would contain features supported by both PV and PVH
> > > and then rejecting unsupported features when parsing domXML or starting the
> > > VM. This option is rather distasteful.
> > > 
> > > And we also have the option of adding VIR_DOMAIN_OSTYPE_XENPVH, which I've
> > > shied away from but may be a better way to go in the end. Do you have any
> > > suggestions we may have overlooked?
> > 
> > Oooh, it looks like i've been mis-understanding PVH in all my previous
> > reviews.
> > 
> > I thought it was simply a "normal" Xen paravirtualized guest kernel. ie
> > any 'pv' guest is also a valid 'pvh' guest. Looking at the docs
> > 
> >    https://wiki.xen.org/wiki/Xen_Project_Software_Overview#Guest_Types
> > 
> > It appears I was wrong. It says a pvh guest kernel relies on hardware
> > virt extensions for part of its work and paravirt for other parts. So
> > really is a hybrid between pv and hvm.
> 
> Right. The Xen wiki also has a good writeup about the various guest types
> 
> https://wiki.xenproject.org/wiki/Understanding_the_Virtualization_Spectrum
> 
> > With that in mind, we should indeed have a distinct OS type constant
> > to express this.
> 
> There have been some long threads in the various versions of this series
> with a lot of waffling :-). I made a few attempts at summarizing what we
> learned about PV vs PVH but could never build a strong case (at least in my
> own head) for either of the two modeling approaches
> 
> https://www.redhat.com/archives/libvir-list/2018-October/msg00214.html
> https://www.redhat.com/archives/libvir-list/2018-October/msg00891.html

It has a bad name, but essentially you should consider "ostype" to
refer to the   Hypervisor <-> Guest hardware ABI.

Based on what I read, and your 2 links here, PV is clearly a different
hardware ABI from PVH. Guest kernels needs different modifications for
PV vs PVH.

Sorry I didn't spot this sooner, and let this go off down the blind
alley of switching based on machine type, when we should have used
the ostype :-(

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Jim Fehlig 5 years, 11 months ago

On 10/19/18 8:59 AM, Daniel P. Berrangé wrote:
> On Fri, Oct 19, 2018 at 08:53:15AM -0600, Jim Fehlig wrote:
>> On 10/19/18 8:14 AM, Daniel P. Berrangé wrote:
>>> On Fri, Oct 19, 2018 at 08:06:18AM -0600, Jim Fehlig wrote:
>>>> On 10/19/18 3:11 AM, Daniel P. Berrangé wrote:
>>>>> On Thu, Oct 18, 2018 at 11:08:34AM -0600, Jim Fehlig wrote:
>>>>>> On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote:
>>>>>>> On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
>>>>>>>> I had some couch time at the start of the weekend and was finally able to
>>>>>>>> try using this series with virt-install. As it turns out, reporting
>>>>>>>> duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we
>>>>>>>> will want to report the additional <machine> under the existing 'xen'
>>>>>>>> <guest> blocks.
>>>>>>>
>>>>>>> Is that virt-install limitation? In that case, IMO virt-install should
>>>>>>> be fixed, instead of changing capabilities xml to match its limitations.
>>>>>>
>>>>>> Perhaps it is a virt-install limitation, but my suggestion was based more on
>>>>>> how the qemu driver reports the different machines
>>>>>>
>>>>>> <guest>
>>>>>>      <os_type>hvm</os_type>
>>>>>>      <arch name='x86_64'>
>>>>>>        <wordsize>64</wordsize>
>>>>>>        <emulator>/usr/bin/qemu-system-x86_64</emulator>
>>>>>>        <machine maxCpus='255'>pc-i440fx-3.0</machine>
>>>>>>        <machine maxCpus='288'>pc-q35-3.0</machine>
>>>>>>        ...
>>>>>>      </arch>
>>>>>> </guest>
>>>>>>
>>>>>> Compare that with reporting PV and PVH in different <guest> blocks, where
>>>>>> the <os_type> and <arch> are the same. It seems confusing from a consumers
>>>>>> POV
>>>>>>
>>>>>> <guest>
>>>>>>      <os_type>xen</os_type>
>>>>>>      <arch name='x86_64'>
>>>>>>        <wordsize>64</wordsize>
>>>>>>        <emulator>/usr/bin/qemu-system-x86_64</emulator>
>>>>>>        <machine>xenpv</machine>
>>>>>>      </arch>
>>>>>> </guest>
>>>>>>
>>>>>> <guest>
>>>>>>      <os_type>xen</os_type>
>>>>>>      <arch name='x86_64'>
>>>>>>        <wordsize>64</wordsize>
>>>>>>        <emulator>/usr/bin/qemu-system-x86_64</emulator>
>>>>>>        <machine>xenpvh</machine>
>>>>>>      </arch>
>>>>>> </guest>
>>>>>>
>>>>>> How should a consumer interpret that? Is the machine for os_type=xen,
>>>>>> arch=x86_64 a xenpv or a xenpvh?
>>>>>
>>>>> Yes, you are right - any pair of (os_type, arch) should be unique
>>>>> in the capabilities XML. So all machines should be reported in the
>>>>> same block.
>>>>
>>>> Our difficulty with that is xenpv and xenpvh machines have different
>>>> features. Marek pointed out that the qemu driver reports the "feature"
>>>> maxCpus as an attribute on the machine element, but we're hesitant to keep
>>>> adding attributes for each feature that is unique to a machine.
>>>>
>>>> Another option we discussed was reporting a superset of all features so that
>>>> e.g. (xen, x86_64) block would contain features supported by both PV and PVH
>>>> and then rejecting unsupported features when parsing domXML or starting the
>>>> VM. This option is rather distasteful.
>>>>
>>>> And we also have the option of adding VIR_DOMAIN_OSTYPE_XENPVH, which I've
>>>> shied away from but may be a better way to go in the end. Do you have any
>>>> suggestions we may have overlooked?
>>>
>>> Oooh, it looks like i've been mis-understanding PVH in all my previous
>>> reviews.
>>>
>>> I thought it was simply a "normal" Xen paravirtualized guest kernel. ie
>>> any 'pv' guest is also a valid 'pvh' guest. Looking at the docs
>>>
>>>     https://wiki.xen.org/wiki/Xen_Project_Software_Overview#Guest_Types
>>>
>>> It appears I was wrong. It says a pvh guest kernel relies on hardware
>>> virt extensions for part of its work and paravirt for other parts. So
>>> really is a hybrid between pv and hvm.
>>
>> Right. The Xen wiki also has a good writeup about the various guest types
>>
>> https://wiki.xenproject.org/wiki/Understanding_the_Virtualization_Spectrum
>>
>>> With that in mind, we should indeed have a distinct OS type constant
>>> to express this.
>>
>> There have been some long threads in the various versions of this series
>> with a lot of waffling :-). I made a few attempts at summarizing what we
>> learned about PV vs PVH but could never build a strong case (at least in my
>> own head) for either of the two modeling approaches
>>
>> https://www.redhat.com/archives/libvir-list/2018-October/msg00214.html
>> https://www.redhat.com/archives/libvir-list/2018-October/msg00891.html
> 
> It has a bad name, but essentially you should consider "ostype" to
> refer to the   Hypervisor <-> Guest hardware ABI.

This is the key point I didn't consider :-(.

> Based on what I read, and your 2 links here, PV is clearly a different
> hardware ABI from PVH. Guest kernels needs different modifications for
> PV vs PVH.

Right.

> Sorry I didn't spot this sooner, and let this go off down the blind
> alley of switching based on machine type, when we should have used
> the ostype :-(

I've been around here long enough that I should have realized your key point 
above. Marek, I don't know what else to say but I'm sorry and will owe you many 
drinks of your choice should our paths cross...

Regards,
Jim

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

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Marek Marczykowski-Górecki 5 years, 11 months ago

On Fri, Oct 19, 2018 at 03:59:03PM +0100, Daniel P. Berrangé wrote:
> On Fri, Oct 19, 2018 at 08:53:15AM -0600, Jim Fehlig wrote:
> > On 10/19/18 8:14 AM, Daniel P. Berrangé wrote:
> > > On Fri, Oct 19, 2018 at 08:06:18AM -0600, Jim Fehlig wrote:
> > > > On 10/19/18 3:11 AM, Daniel P. Berrangé wrote:
> > > > > On Thu, Oct 18, 2018 at 11:08:34AM -0600, Jim Fehlig wrote:
> > > > > > On 10/17/18 12:59 PM, Marek Marczykowski-Górecki wrote:
> > > > > > > On Sat, Oct 13, 2018 at 08:46:19AM -0600, Jim Fehlig wrote:
> > > > > > > > I had some couch time at the start of the weekend and was finally able to
> > > > > > > > try using this series with virt-install. As it turns out, reporting
> > > > > > > > duplicate <guest> blocks for <os_type> 'xen' is not quite right. Instead we
> > > > > > > > will want to report the additional <machine> under the existing 'xen'
> > > > > > > > <guest> blocks.
> > > > > > > 
> > > > > > > Is that virt-install limitation? In that case, IMO virt-install should
> > > > > > > be fixed, instead of changing capabilities xml to match its limitations.
> > > > > > 
> > > > > > Perhaps it is a virt-install limitation, but my suggestion was based more on
> > > > > > how the qemu driver reports the different machines
> > > > > > 
> > > > > > <guest>
> > > > > >     <os_type>hvm</os_type>
> > > > > >     <arch name='x86_64'>
> > > > > >       <wordsize>64</wordsize>
> > > > > >       <emulator>/usr/bin/qemu-system-x86_64</emulator>
> > > > > >       <machine maxCpus='255'>pc-i440fx-3.0</machine>
> > > > > >       <machine maxCpus='288'>pc-q35-3.0</machine>
> > > > > >       ...
> > > > > >     </arch>
> > > > > > </guest>
> > > > > > 
> > > > > > Compare that with reporting PV and PVH in different <guest> blocks, where
> > > > > > the <os_type> and <arch> are the same. It seems confusing from a consumers
> > > > > > POV
> > > > > > 
> > > > > > <guest>
> > > > > >     <os_type>xen</os_type>
> > > > > >     <arch name='x86_64'>
> > > > > >       <wordsize>64</wordsize>
> > > > > >       <emulator>/usr/bin/qemu-system-x86_64</emulator>
> > > > > >       <machine>xenpv</machine>
> > > > > >     </arch>
> > > > > > </guest>
> > > > > > 
> > > > > > <guest>
> > > > > >     <os_type>xen</os_type>
> > > > > >     <arch name='x86_64'>
> > > > > >       <wordsize>64</wordsize>
> > > > > >       <emulator>/usr/bin/qemu-system-x86_64</emulator>
> > > > > >       <machine>xenpvh</machine>
> > > > > >     </arch>
> > > > > > </guest>
> > > > > > 
> > > > > > How should a consumer interpret that? Is the machine for os_type=xen,
> > > > > > arch=x86_64 a xenpv or a xenpvh?
> > > > > 
> > > > > Yes, you are right - any pair of (os_type, arch) should be unique
> > > > > in the capabilities XML. So all machines should be reported in the
> > > > > same block.
> > > > 
> > > > Our difficulty with that is xenpv and xenpvh machines have different
> > > > features. Marek pointed out that the qemu driver reports the "feature"
> > > > maxCpus as an attribute on the machine element, but we're hesitant to keep
> > > > adding attributes for each feature that is unique to a machine.
> > > > 
> > > > Another option we discussed was reporting a superset of all features so that
> > > > e.g. (xen, x86_64) block would contain features supported by both PV and PVH
> > > > and then rejecting unsupported features when parsing domXML or starting the
> > > > VM. This option is rather distasteful.
> > > > 
> > > > And we also have the option of adding VIR_DOMAIN_OSTYPE_XENPVH, which I've
> > > > shied away from but may be a better way to go in the end. Do you have any
> > > > suggestions we may have overlooked?
> > > 
> > > Oooh, it looks like i've been mis-understanding PVH in all my previous
> > > reviews.
> > > 
> > > I thought it was simply a "normal" Xen paravirtualized guest kernel. ie
> > > any 'pv' guest is also a valid 'pvh' guest. Looking at the docs
> > > 
> > >    https://wiki.xen.org/wiki/Xen_Project_Software_Overview#Guest_Types
> > > 
> > > It appears I was wrong. It says a pvh guest kernel relies on hardware
> > > virt extensions for part of its work and paravirt for other parts. So
> > > really is a hybrid between pv and hvm.
> > 
> > Right. The Xen wiki also has a good writeup about the various guest types
> > 
> > https://wiki.xenproject.org/wiki/Understanding_the_Virtualization_Spectrum
> > 
> > > With that in mind, we should indeed have a distinct OS type constant
> > > to express this.
> > 
> > There have been some long threads in the various versions of this series
> > with a lot of waffling :-). I made a few attempts at summarizing what we
> > learned about PV vs PVH but could never build a strong case (at least in my
> > own head) for either of the two modeling approaches
> > 
> > https://www.redhat.com/archives/libvir-list/2018-October/msg00214.html
> > https://www.redhat.com/archives/libvir-list/2018-October/msg00891.html
> 
> It has a bad name, but essentially you should consider "ostype" to
> refer to the   Hypervisor <-> Guest hardware ABI.

Oh, if that's the case, then indeed separate ostype makes sense. Maybe
it worth expanding ostype description somewhere in documentation?

> Based on what I read, and your 2 links here, PV is clearly a different
> hardware ABI from PVH. Guest kernels needs different modifications for
> PV vs PVH.
> 
> Sorry I didn't spot this sooner, and let this go off down the blind
> alley of switching based on machine type, when we should have used
> the ostype :-(

What machine type should it use then? Still xenpvh, but make all the
decisions based on ostype?

-- 
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?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Daniel P. Berrangé 5 years, 11 months ago

On Fri, Oct 19, 2018 at 05:10:30PM +0200, Marek Marczykowski-Górecki wrote:
> On Fri, Oct 19, 2018 at 03:59:03PM +0100, Daniel P. Berrangé wrote:
> > On Fri, Oct 19, 2018 at 08:53:15AM -0600, Jim Fehlig wrote:
> > > own head) for either of the two modeling approaches
> > > 
> > > https://www.redhat.com/archives/libvir-list/2018-October/msg00214.html
> > > https://www.redhat.com/archives/libvir-list/2018-October/msg00891.html
> > 
> > It has a bad name, but essentially you should consider "ostype" to
> > refer to the   Hypervisor <-> Guest hardware ABI.
> 
> Oh, if that's the case, then indeed separate ostype makes sense. Maybe
> it worth expanding ostype description somewhere in documentation?
> 
> > Based on what I read, and your 2 links here, PV is clearly a different
> > hardware ABI from PVH. Guest kernels needs different modifications for
> > PV vs PVH.
> > 
> > Sorry I didn't spot this sooner, and let this go off down the blind
> > alley of switching based on machine type, when we should have used
> > the ostype :-(
> 
> What machine type should it use then? Still xenpvh, but make all the
> decisions based on ostype?

If Xen/QEMU calls the machine type 'xenpvh' then yeah we can still
use it. By having a distinct ostype, when the XML says "xenpvh" for
OS type, the XML parser can automatically find the correct machine
type name from the capabilities data. So mgmt apps using libvirt
won't need to set the machine type themselves, can just rely on the
default, after they've set the ostype.



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Marek Marczykowski-Górecki 5 years, 11 months ago

On Fri, Oct 19, 2018 at 04:19:21PM +0100, Daniel P. Berrangé wrote:
> On Fri, Oct 19, 2018 at 05:10:30PM +0200, Marek Marczykowski-Górecki wrote:
> > On Fri, Oct 19, 2018 at 03:59:03PM +0100, Daniel P. Berrangé wrote:
> > > On Fri, Oct 19, 2018 at 08:53:15AM -0600, Jim Fehlig wrote:
> > > > own head) for either of the two modeling approaches
> > > > 
> > > > https://www.redhat.com/archives/libvir-list/2018-October/msg00214.html
> > > > https://www.redhat.com/archives/libvir-list/2018-October/msg00891.html
> > > 
> > > It has a bad name, but essentially you should consider "ostype" to
> > > refer to the   Hypervisor <-> Guest hardware ABI.
> > 
> > Oh, if that's the case, then indeed separate ostype makes sense. Maybe
> > it worth expanding ostype description somewhere in documentation?

Also, such definition of os type, make "linux" os type for Xen PV even weirder...

> > > Based on what I read, and your 2 links here, PV is clearly a different
> > > hardware ABI from PVH. Guest kernels needs different modifications for
> > > PV vs PVH.
> > > 
> > > Sorry I didn't spot this sooner, and let this go off down the blind
> > > alley of switching based on machine type, when we should have used
> > > the ostype :-(
> > 
> > What machine type should it use then? Still xenpvh, but make all the
> > decisions based on ostype?
> 
> If Xen/QEMU calls the machine type 'xenpvh' then yeah we can still
> use it. 

There is no qemu in the picture, and Xen (libxl) have just one thing:
type, not separate os type and machine type. So, it's only libvirt
specific bit here and we can choose it arbitrarily. As you wrote
below, libvirt can fill it based on os type, so I'll make it "xenpvh".

> By having a distinct ostype, when the XML says "xenpvh" for
> OS type, the XML parser can automatically find the correct machine
> type name from the capabilities data. So mgmt apps using libvirt
> won't need to set the machine type themselves, can just rely on the
> default, after they've set the ostype.

-- 
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?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v4 2/5] libxl: add support for PVH

Posted by Jim Fehlig 5 years, 11 months ago

On 10/19/18 9:25 AM, Marek Marczykowski-Górecki wrote:
> On Fri, Oct 19, 2018 at 04:19:21PM +0100, Daniel P. Berrangé wrote:
>> On Fri, Oct 19, 2018 at 05:10:30PM +0200, Marek Marczykowski-Górecki wrote:
>>> On Fri, Oct 19, 2018 at 03:59:03PM +0100, Daniel P. Berrangé wrote:
>>>> On Fri, Oct 19, 2018 at 08:53:15AM -0600, Jim Fehlig wrote:
>>>>> own head) for either of the two modeling approaches
>>>>>
>>>>> https://www.redhat.com/archives/libvir-list/2018-October/msg00214.html
>>>>> https://www.redhat.com/archives/libvir-list/2018-October/msg00891.html
>>>>
>>>> It has a bad name, but essentially you should consider "ostype" to
>>>> refer to the   Hypervisor <-> Guest hardware ABI.
>>>
>>> Oh, if that's the case, then indeed separate ostype makes sense. Maybe
>>> it worth expanding ostype description somewhere in documentation?
> 
> Also, such definition of os type, make "linux" os type for Xen PV even weirder...

Yeah, I wish we could ditch it.

BTW, please include a patch for docs/news.xml in V5. Thanks!

Regards,
Jim

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

[libvirt] [PATCH v4 1/5] libxl: reorder libxlMakeDomBuildInfo for upcoming PVH support
[libvirt] [PATCH v4 2/5] libxl: add support for PVH
[libvirt] [PATCH v4 3/5] tests: add basic Xen PVH test
[libvirt] [PATCH v4 4/5] xenconfig: add support for parsing type= xl config entry
[libvirt] [PATCH v4 5/5] xenconfig: add support for type="pvh"