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

Jim Fehlig jfehlig at suse.com
Mon Apr 13 21:22:31 UTC 2020


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 at invisiblethingslab.com>
> ---
>   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 @@
      <hidden state='on'/>
      <hint-dedicated state='on'/>
    </kvm>
+  <xen>
+    <host-e820 state='on'/>
+  </xen>
    <pvspinlock state='on'/>
    <gic version='2'/>
    <ioapic driver='qemu'/>
@@ -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:
> 





More information about the libvir-list mailing list