Re: [PATCH 1/4] conf: add xen specific feature: e820_host

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

 



On 4/13/20 2:10 PM, Marek Marczykowski-Górecki wrote:
This is Xen specific option to provide domain e820 map based on host
one. Useful when using PCI passthrough, see Xen documentation for more
details.

I would reword this a bit to mention it is PV-only and documented in the xl.cfg man page. E.g.

e820_host is a Xen-specific option only available for PV domains that provides the domain a virtual e820 memory map based on the host one. It is required when using PCI passthrough and is generally considered safe for any PV kernel. e820_host is silently ignored if set in HVM domain configuration. See xl.cfg(5) man page in the Xen documentation for more details.


Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
---
  docs/schemas/domaincommon.rng |  16 +++++-

You also need to provide documentation. Something like

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 6f43976815..aea2267a7f 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2061,6 +2061,9 @@
     &lt;hidden state='on'/&gt;
     &lt;hint-dedicated state='on'/&gt;
   &lt;/kvm&gt;
+  &lt;xen&gt;
+    &lt;host-e820 state='on'/&gt;
+  &lt;/xen&gt;
   &lt;pvspinlock state='on'/&gt;
   &lt;gic version='2'/&gt;
   &lt;ioapic driver='qemu'/&gt;
@@ -2242,6 +2245,23 @@
         </tr>
       </table>
       </dd>
+      <dt><code>xen</code></dt>
+      <dd>Various features to change the behavior of the Xen hypervisor.
+      <table class="top_table">
+        <tr>
+          <th>Feature</th>
+          <th>Description</th>
+          <th>Value</th>
+          <th>Since</th>
+        </tr>
+        <tr>
+          <td>host-e820</td>
+          <td>Expose the host e820 to the guest (PV only)</td>
+          <td>on, off</td>
+          <td><span class="since">6.3.8</span></td>
+        </tr>
+      </table>
+      </dd>
       <dt><code>pmu</code></dt>
       <dd>Depending on the <code>state</code> attribute (values <code>on</code>,
         <code>off</code>, default <code>on</code>) enable or disable the


  src/conf/domain_conf.c        | 106 +++++++++++++++++++++++++++++++++++-
  src/conf/domain_conf.h        |   9 +++-
  src/qemu/qemu_validate.c      |   1 +-
  4 files changed, 132 insertions(+)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 65d6580..2b5f844 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5349,6 +5349,9 @@
              <ref name="kvm"/>
            </optional>
            <optional>
+            <ref name="xen"/>
+          </optional>
+          <optional>
              <element name="privnet">
                <empty/>
              </element>
@@ -6337,6 +6340,19 @@
      </element>
    </define>
+ <!-- Optional Xen features -->
+  <define name="xen">
+    <element name="xen">
+      <interleave>
+        <optional>
+          <element name="e820_host">

On occasion there seems to be debate about underscore vs dash, but the schema has a lot of both and using the same name as xl.cfg is user friendly.

+            <ref name="featurestate"/>
+          </element>
+        </optional>
+      </interleave>
+    </element>
+  </define>
+
    <!-- Optional capabilities features -->
    <define name="capabilities">
      <element name="capabilities">
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8e81463..13ff168 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -160,6 +160,7 @@ VIR_ENUM_IMPL(virDomainFeature,
                "privnet",
                "hyperv",
                "kvm",
+              "xen",

This should be added at the end of the enum.

                "pvspinlock",
                "capabilities",
                "pmu",
@@ -206,6 +207,11 @@ VIR_ENUM_IMPL(virDomainKVM,
                "hint-dedicated",
  );
+VIR_ENUM_IMPL(virDomainXen,
+              VIR_DOMAIN_XEN_LAST,
+              "e820_host"
+);
+
  VIR_ENUM_IMPL(virDomainMsrsUnknown,
                VIR_DOMAIN_MSRS_UNKNOWN_LAST,
                "ignore",
@@ -20862,6 +20868,7 @@ virDomainDefParseXML(xmlDocPtr xml,
          case VIR_DOMAIN_FEATURE_HYPERV:
          case VIR_DOMAIN_FEATURE_KVM:
          case VIR_DOMAIN_FEATURE_MSRS:
+        case VIR_DOMAIN_FEATURE_XEN:
              def->features[val] = VIR_TRISTATE_SWITCH_ON;
              break;
@@ -21172,6 +21179,55 @@ virDomainDefParseXML(xmlDocPtr xml,
          VIR_FREE(nodes);
      }
+ if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) {
+        int feature;
+        int value;
+        node = ctxt->node;
+        if ((n = virXPathNodeSet("./features/xen/*", ctxt, &nodes)) < 0)
+            goto error;
+
+        for (i = 0; i < n; i++) {
+            feature = virDomainXenTypeFromString((const char *)nodes[i]->name);
+            if (feature < 0) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               _("unsupported Xen feature: %s"),
+                               nodes[i]->name);
+                goto error;
+            }
+
+            ctxt->node = nodes[i];
+
+            switch ((virDomainXen) feature) {
+                case VIR_DOMAIN_XEN_E820_HOST:
+                    if (!(tmp = virXPathString("string(./@state)", ctxt))) {

I think it is better to use virXMLPropString here, similar to the VIR_DOMAIN_FEATURE_KVM conditional. You can then avoid saving, setting, and restoring ctxt->node.

+                        virReportError(VIR_ERR_XML_ERROR,
+                                       _("missing 'state' attribute for "
+                                         "Xen feature '%s'"),
+                                       nodes[i]->name);
+                        goto error;
+                    }
+
+                    if ((value = virTristateSwitchTypeFromString(tmp)) < 0) {
+                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                       _("invalid value of state argument "
+                                         "for Xen feature '%s'"),
+                                       nodes[i]->name);
+                        goto error;
+                    }
+
+                    VIR_FREE(tmp);
+                    def->xen_features[feature] = value;
+                    break;
+
+                /* coverity[dead_error_begin] */
+                case VIR_DOMAIN_XEN_LAST:
+                    break;
+            }
+        }
+        VIR_FREE(nodes);
+        ctxt->node = node;
+    }
+
      if (def->features[VIR_DOMAIN_FEATURE_SMM] == VIR_TRISTATE_SWITCH_ON) {
          int rv = virDomainParseScaledValue("string(./features/smm/tseg)",
                                             "string(./features/smm/tseg/@unit)",
@@ -23173,6 +23229,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
          case VIR_DOMAIN_FEATURE_PRIVNET:
          case VIR_DOMAIN_FEATURE_HYPERV:
          case VIR_DOMAIN_FEATURE_KVM:
+        case VIR_DOMAIN_FEATURE_XEN:
          case VIR_DOMAIN_FEATURE_PVSPINLOCK:
          case VIR_DOMAIN_FEATURE_PMU:
          case VIR_DOMAIN_FEATURE_VMPORT:
@@ -23344,6 +23401,30 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
          }
      }
+ /* xen */
+    if (src->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) {
+        for (i = 0; i < VIR_DOMAIN_XEN_LAST; i++) {
+            switch ((virDomainXen) i) {
+            case VIR_DOMAIN_XEN_E820_HOST:
+                if (src->xen_features[i] != dst->xen_features[i]) {
+                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                   _("State of Xen feature '%s' differs: "
+                                     "source: '%s', destination: '%s'"),
+                                   virDomainXenTypeToString(i),
+                                   virTristateSwitchTypeToString(src->xen_features[i]),
+                                   virTristateSwitchTypeToString(dst->xen_features[i]));
+                    return false;
+                }
+
+                break;
+
+            /* coverity[dead_error_begin] */
+            case VIR_DOMAIN_XEN_LAST:
+                break;
+            }
+        }
+    }
+
      /* kvm */
      if (src->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) {
          for (i = 0; i < VIR_DOMAIN_KVM_LAST; i++) {
@@ -28959,6 +29040,31 @@ virDomainDefFormatFeatures(virBufferPtr buf,
              virBufferAddLit(&childBuf, "</kvm>\n");
              break;
+ case VIR_DOMAIN_FEATURE_XEN:
+            if (def->features[i] != VIR_TRISTATE_SWITCH_ON)
+                break;
+
+            virBufferAddLit(&childBuf, "<xen>\n");
+            virBufferAdjustIndent(&childBuf, 2);
+            for (j = 0; j < VIR_DOMAIN_XEN_LAST; j++) {
+                switch ((virDomainXen) j) {
+                case VIR_DOMAIN_XEN_E820_HOST:
+                    if (def->xen_features[j])
+                        virBufferAsprintf(&childBuf, "<%s state='%s'/>\n",
+                                          virDomainXenTypeToString(j),
+                                          virTristateSwitchTypeToString(
+                                              def->xen_features[j]));
+                    break;
+
+                /* coverity[dead_error_begin] */
+                case VIR_DOMAIN_XEN_LAST:
+                    break;
+                }
+            }
+            virBufferAdjustIndent(&childBuf, -2);
+            virBufferAddLit(&childBuf, "</xen>\n");
+            break;
+
          case VIR_DOMAIN_FEATURE_CAPABILITIES:
              for (j = 0; j < VIR_DOMAIN_PROCES_CAPS_FEATURE_LAST; j++) {
                  if (def->caps_features[j] != VIR_TRISTATE_SWITCH_ABSENT)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index aad3f82..9849189 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1801,6 +1801,7 @@ typedef enum {
      VIR_DOMAIN_FEATURE_PRIVNET,
      VIR_DOMAIN_FEATURE_HYPERV,
      VIR_DOMAIN_FEATURE_KVM,
+    VIR_DOMAIN_FEATURE_XEN,

This should be added to the end of the enum.

Regards,
Jim

      VIR_DOMAIN_FEATURE_PVSPINLOCK,
      VIR_DOMAIN_FEATURE_CAPABILITIES,
      VIR_DOMAIN_FEATURE_PMU,
@@ -1860,6 +1861,12 @@ typedef enum {
  } virDomainMsrsUnknown;
typedef enum {
+    VIR_DOMAIN_XEN_E820_HOST = 0,
+
+    VIR_DOMAIN_XEN_LAST
+} virDomainXen;
+
+typedef enum {
      VIR_DOMAIN_CAPABILITIES_POLICY_DEFAULT = 0,
      VIR_DOMAIN_CAPABILITIES_POLICY_ALLOW,
      VIR_DOMAIN_CAPABILITIES_POLICY_DENY,
@@ -2484,6 +2491,7 @@ struct _virDomainDef {
      int hyperv_features[VIR_DOMAIN_HYPERV_LAST];
      int kvm_features[VIR_DOMAIN_KVM_LAST];
      int msrs_features[VIR_DOMAIN_MSRS_LAST];
+    int xen_features[VIR_DOMAIN_XEN_LAST];
      unsigned int hyperv_spinlocks;
      int hyperv_stimer_direct;
      virGICVersion gic_version;
@@ -3529,6 +3537,7 @@ VIR_ENUM_DECL(virDomainGraphicsSpiceMouseMode);
  VIR_ENUM_DECL(virDomainGraphicsVNCSharePolicy);
  VIR_ENUM_DECL(virDomainHyperv);
  VIR_ENUM_DECL(virDomainKVM);
+VIR_ENUM_DECL(virDomainXen);
  VIR_ENUM_DECL(virDomainMsrsUnknown);
  VIR_ENUM_DECL(virDomainRNGModel);
  VIR_ENUM_DECL(virDomainRNGBackend);
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 971e4a9..cb0ff8d 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -302,6 +302,7 @@ qemuValidateDomainDefFeatures(const virDomainDef *def,
              }
              break;
+ case VIR_DOMAIN_FEATURE_XEN:
          case VIR_DOMAIN_FEATURE_ACPI:
          case VIR_DOMAIN_FEATURE_PAE:
          case VIR_DOMAIN_FEATURE_HAP:







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

  Powered by Linux