On 9/30/18 8:15 PM, Marek Marczykowski-Górecki wrote:
Since this is something between PV and HVM, it makes sense to put the
setting in place where domain type is specified.
To enable it, use <os><type machine="xenpvh">...</type></os>. It is
also included in capabilities.xml, for every supported HVM guest type - it
doesn't seems to be any other requirement (besides new enough Xen).

Signed-off-by: Marek Marczykowski-Górecki <marma...@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

No video, graphics or hostdev passthrough - bummer. Begs the question: what to do with PVH XML config containing these devices? Reject it? Silently ignore? I'll also need to remember to enable these as PVH gains support for the devices.

  - use #ifdef LIBXL_DOMAIN_TYPE_PVH instead of hypervisor version to
  check for PVH support

This is a much better approach than the version check. I should have thought of that earlier, sorry.

  - compile fix for Xen < 4.9
---
  docs/formatcaps.html.in        |  4 +--
  docs/schemas/domaincommon.rng  |  1 +-
  src/conf/domain_conf.c         |  6 ++--
  src/libxl/libxl_capabilities.c | 47 ++++++++++++++++++++++++++++-----
  src/libxl/libxl_conf.c         | 50 ++++++++++++++++++++++++++++++-----
  src/libxl/libxl_driver.c       |  6 ++--
  6 files changed, 95 insertions(+), 19 deletions(-)

diff --git a/docs/formatcaps.html.in b/docs/formatcaps.html.in
index 0d9c53d..b8102fd 100644
--- a/docs/formatcaps.html.in
+++ b/docs/formatcaps.html.in
@@ -104,8 +104,8 @@
              <dt><code>machine</code></dt><dd>Machine type, for use in
                <a href="formatdomain.html#attributeOSTypeMachine">machine</a>
                attribute of os/type element in domain XML. For example Xen
-              supports <code>xenfv</code> for HVM or <code>xenpv</code> for
-              PV.</dd>
+              supports <code>xenfv</code> for HVM, <code>xenpv</code> for
+              PV, or <code>xenpvh</code> for PVHv2.</dd>

s/PVHv2/PVH/

              <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/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..be51a4d 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;
@@ -439,6 +440,9 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
              int hvm = STRPREFIX(&token[subs[1].rm_so], "hvm");
              virArch arch;
              int pae = 0, nonpae = 0, ia64_be = 0;
+#ifdef LIBXL_DOMAIN_TYPE_PVH
+            bool new_arch_added = false;
+#endif
if (STRPREFIX(&token[subs[2].rm_so], "x86_32")) {
                  arch = VIR_ARCH_I686;
@@ -475,8 +479,12 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
              if (i >= ARRAY_CARDINALITY(guest_archs))
                  continue;
              /* Didn't find a match, so create a new one */
-            if (i == nr_guest_archs)
+            if (i == nr_guest_archs) {
                  nr_guest_archs++;
+#ifdef LIBXL_DOMAIN_TYPE_PVH
+                new_arch_added = true;
+#endif
+            }

My idea to add this variable for clarity now seems diluted by the need for the extra ifdef's :-/. Your original approach would avoid them.

The rest of the patch looks good, but before going further let's get a resolution to the VIR_DOMAIN_OSTYPE_XENPVH vs os.machine approach to modeling PVH. I can do some poking on IRC if there are no responses to this thread.

Regards,
Jim

guest_archs[i].arch = arch;
              guest_archs[i].hvm = hvm;
@@ -491,13 +499,33 @@ libxlCapsInitGuests(libxl_ctx *ctx, virCapsPtr caps)
                  guest_archs[i].nonpae = nonpae;
              if (ia64_be)
                  guest_archs[i].ia64_be = ia64_be;
+
+            /*
+             * Xen 4.9 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 LIBXL_DOMAIN_TYPE_PVH
+            if (hvm && new_arch_added) {
+                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 +585,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 +610,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 +762,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..0007906 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 LIBXL_DOMAIN_TYPE_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 LIBXL_DOMAIN_TYPE_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 {


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

Reply via email to