We're going to introduce a number of optional, pSeries-specific
features, so we want to have a dedicated sub-element to avoid
cluttering the <domain><features> element.
Along with the generic framework, we also introduce the first
actual feature: HPT (Hash Page Table) tuning. This will replace
the existing <hpt> feature later on, but right now they simply
co-exist without interfering with each other.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
docs/formatdomain.html.in | 20 +++++++++
docs/schemas/domaincommon.rng | 16 +++++++
src/conf/domain_conf.c | 102 ++++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 11 +++++
src/libvirt_private.syms | 2 +
src/qemu/qemu_command.c | 93 ++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.c | 7 +++
7 files changed, 251 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index d272cc1ba..b2584fbb0 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1768,6 +1768,9 @@
<kvm>
<hidden state='on'/>
</kvm>
+ <pseries>
+ <hpt resizing='required'/>
+ </pseries>
<pvspinlock state='on'/>
<gic version='2'/>
<ioapic driver='qemu'/>
@@ -1904,6 +1907,23 @@
</tr>
</table>
</dd>
+ <dt><code>pseries</code></dt>
+ <dd>Various features to change the behavior of pSeries guests.
+ <table class="top_table">
+ <tr>
+ <th>Feature</th>
+ <th>Description</th>
+ <th>Value</th>
+ <th>Since</th>
+ </tr>
+ <tr>
+ <td>hpt</td>
+ <td>Configure HPT (Hash Page Table)</td>
+ <td>resizing: enabled, disabled, required</td>
+ <td><span class="since">4.1.0 (QEMU 2.10)</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
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 75838b581..fead6e7cc 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4703,6 +4703,9 @@
<optional>
<ref name="kvm"/>
</optional>
+ <optional>
+ <ref name="pseries"/>
+ </optional>
<optional>
<element name="privnet">
<empty/>
@@ -5526,6 +5529,19 @@
</element>
</define>
+ <!-- Optional pSeries features -->
+ <define name="pseries">
+ <element name="pseries">
+ <interleave>
+ <optional>
+ <element name="hpt">
+ <ref name="resizing"/>
+ </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 a1c25060f..577a804df 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -152,6 +152,7 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST,
"ioapic",
"hpt",
"vmcoreinfo",
+ "pseries",
);
VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST,
@@ -173,6 +174,11 @@ VIR_ENUM_IMPL(virDomainHyperv, VIR_DOMAIN_HYPERV_LAST,
VIR_ENUM_IMPL(virDomainKVM, VIR_DOMAIN_KVM_LAST,
"hidden")
+VIR_ENUM_IMPL(virDomainPSeries,
+ VIR_DOMAIN_PSERIES_LAST,
+ "hpt",
+);
+
VIR_ENUM_IMPL(virDomainCapsFeature, VIR_DOMAIN_CAPS_FEATURE_LAST,
"audit_control",
"audit_write",
@@ -18879,6 +18885,7 @@ virDomainDefParseXML(xmlDocPtr xml,
case VIR_DOMAIN_FEATURE_HYPERV:
case VIR_DOMAIN_FEATURE_VMCOREINFO:
case VIR_DOMAIN_FEATURE_KVM:
+ case VIR_DOMAIN_FEATURE_PSERIES:
def->features[val] = VIR_TRISTATE_SWITCH_ON;
break;
@@ -19114,6 +19121,54 @@ virDomainDefParseXML(xmlDocPtr xml,
VIR_FREE(nodes);
}
+ if (def->features[VIR_DOMAIN_FEATURE_PSERIES] == VIR_TRISTATE_SWITCH_ON) {
+ int feature;
+ int value;
+
+ if ((n = virXPathNodeSet("./features/pseries/*", ctxt, &nodes)) < 0)
+ goto error;
+
+ for (i = 0; i < n; i++) {
+ feature = virDomainPSeriesTypeFromString((const char *) nodes[i]->name);
+
+ if (feature < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unknown '%s' pSeries feature"),
+ nodes[i]->name);
+ goto error;
+ }
+
+ switch ((virDomainPSeries) feature) {
+ case VIR_DOMAIN_PSERIES_HPT:
+ if (!(tmp = virXMLPropString(nodes[i], "resizing"))) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Missing '%s' attribute for "
+ "'%s' pSeries feature"),
+ "resizing", nodes[i]->name);
+ goto error;
+ }
+
+ if ((value = virDomainHPTResizingTypeFromString(tmp)) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Invalid value '%s' for '%s' "
+ "attribute of '%s' pSeries feature"),
+ tmp, "resizing", nodes[i]->name);
+ goto error;
+ }
+
+ def->pseries_features[feature] = VIR_TRISTATE_SWITCH_ON;
+ def->pseries_hpt_resizing = value;
+
+ VIR_FREE(tmp);
+ break;
+
+ case VIR_DOMAIN_PSERIES_LAST:
+ break;
+ }
+ }
+ VIR_FREE(nodes);
+ }
+
if ((n = virXPathNodeSet("./features/capabilities/*", ctxt, &nodes)) < 0)
goto error;
@@ -21140,6 +21195,29 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
}
}
+ /* pSeries features */
+ if (src->features[VIR_DOMAIN_FEATURE_PSERIES] == VIR_TRISTATE_SWITCH_ON) {
+ for (i = 0; i < VIR_DOMAIN_PSERIES_LAST; i++) {
+ switch ((virDomainPSeries) i) {
+ case VIR_DOMAIN_PSERIES_HPT:
+ if (src->pseries_features[i] != dst->pseries_features[i] ||
+ src->pseries_hpt_resizing != dst->pseries_hpt_resizing) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("State of '%s' pSeries feature differs: "
+ "source: '%s', destination: '%s'"),
+ virDomainPSeriesTypeToString(i),
+ virDomainHPTResizingTypeToString(src->pseries_hpt_resizing),
+ virDomainHPTResizingTypeToString(dst->pseries_hpt_resizing));
+ return false;
+ }
+ break;
+
+ case VIR_DOMAIN_PSERIES_LAST:
+ break;
+ }
+ }
+ }
+
/* ioapic */
if (src->ioapic != dst->ioapic) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -26487,6 +26565,30 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virBufferAddLit(buf, "</kvm>\n");
break;
+ case VIR_DOMAIN_FEATURE_PSERIES:
+ if (def->features[i] != VIR_TRISTATE_SWITCH_ON)
+ break;
+
+ virBufferAddLit(buf, "<pseries>\n");
+ virBufferAdjustIndent(buf, 2);
+ for (j = 0; j < VIR_DOMAIN_PSERIES_LAST; j++) {
+ switch ((virDomainPSeries) j) {
+ case VIR_DOMAIN_PSERIES_HPT:
+ if (def->pseries_features[j] != VIR_TRISTATE_SWITCH_ON)
+ break;
+
+ virBufferAsprintf(buf, "<hpt resizing='%s'/>\n",
+ virDomainHPTResizingTypeToString(def->pseries_hpt_resizing));
+ break;
+
+ case VIR_DOMAIN_PSERIES_LAST:
+ break;
+ }
+ }
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</pseries>\n");
+ break;
+
case VIR_DOMAIN_FEATURE_CAPABILITIES:
if (def->features[i] == VIR_DOMAIN_CAPABILITIES_POLICY_DEFAULT &&
!virDomainDefHasCapabilitiesFeatures(def))
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 6f7f96b3d..e4ae2a26c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1740,6 +1740,7 @@ typedef enum {
VIR_DOMAIN_FEATURE_IOAPIC,
VIR_DOMAIN_FEATURE_HPT,
VIR_DOMAIN_FEATURE_VMCOREINFO,
+ VIR_DOMAIN_FEATURE_PSERIES,
VIR_DOMAIN_FEATURE_LAST
} virDomainFeature;
@@ -1766,6 +1767,14 @@ typedef enum {
VIR_DOMAIN_KVM_LAST
} virDomainKVM;
+typedef enum {
+ VIR_DOMAIN_PSERIES_HPT = 0,
+
+ VIR_DOMAIN_PSERIES_LAST
+} virDomainPSeries;
+
+VIR_ENUM_DECL(virDomainPSeries);
+
typedef enum {
VIR_DOMAIN_CAPABILITIES_POLICY_DEFAULT = 0,
VIR_DOMAIN_CAPABILITIES_POLICY_ALLOW,
@@ -2346,11 +2355,13 @@ struct _virDomainDef {
int apic_eoi;
int hyperv_features[VIR_DOMAIN_HYPERV_LAST];
int kvm_features[VIR_DOMAIN_KVM_LAST];
+ int pseries_features[VIR_DOMAIN_PSERIES_LAST];
unsigned int hyperv_spinlocks;
virGICVersion gic_version;
char *hyperv_vendor_id;
virDomainIOAPIC ioapic;
virDomainHPTResizing hpt_resizing;
+ virDomainHPTResizing pseries_hpt_resizing;
/* These options are of type virTristateSwitch: ON = keep, OFF = drop */
int caps_features[VIR_DOMAIN_CAPS_FEATURE_LAST];
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bc8cc1fba..ae9eecb5d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -484,6 +484,8 @@ virDomainPausedReasonTypeFromString;
virDomainPausedReasonTypeToString;
virDomainPMSuspendedReasonTypeFromString;
virDomainPMSuspendedReasonTypeToString;
+virDomainPSeriesTypeFromString;
+virDomainPSeriesTypeToString;
virDomainRedirdevBusTypeFromString;
virDomainRedirdevBusTypeToString;
virDomainRedirdevDefFind;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b8aede32d..a053b597c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7360,6 +7360,79 @@ qemuBuildNameCommandLine(virCommandPtr cmd,
return 0;
}
+static int
+virDomainPSeriesToQEMUCaps(int feature)
+{
+ switch ((virDomainPSeries) feature) {
+ case VIR_DOMAIN_PSERIES_HPT:
+ return QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT;
+ case VIR_DOMAIN_PSERIES_LAST:
+ break;
+ }
+
+ return -1;
+}
+
+static const char*
+virDomainPSeriesToMachineOption(int feature)
+{
+ switch ((virDomainPSeries) feature) {
+ case VIR_DOMAIN_PSERIES_HPT:
+ return "resize-hpt";
+ case VIR_DOMAIN_PSERIES_LAST:
+ break;
+ }
+
+ return NULL;
+}
+
+static int
+qemuBuildMachineCommandLinePSeriesFeature(virBufferPtr buf,
+ virDomainPSeries feature,
+ const char *value,
+ virQEMUCapsPtr qemuCaps)
+{
+ const char *name = virDomainPSeriesTypeToString(feature);
+ const char *option = virDomainPSeriesToMachineOption(feature);
+ int cap;
+ int ret = -1;
+
+ if (!option) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unknown QEMU option for '%s' pSeries feature"),
+ name);
+ goto cleanup;
+ }
+
+ if (!value) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Invalid value for '%s' pSeries feature"),
+ name);
+ goto cleanup;
+ }
+
+ if ((cap = virDomainPSeriesToQEMUCaps(feature)) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unknown QEMU capability for '%s' pSeries feature"),
+ name);
+ goto cleanup;
+ }
+
+ if (!virQEMUCapsGet(qemuCaps, cap)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("'%s' pSeries feature not supported by this QEMU binary"),
+ name);
+ goto cleanup;
+ }
+
+ virBufferAsprintf(buf, ",%s=%s", option, value);
+
+ ret = 0;
+
+ cleanup:
+ return ret;
+}
+
static int
qemuBuildMachineCommandLine(virCommandPtr cmd,
virQEMUDriverConfigPtr cfg,
@@ -7592,6 +7665,26 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
virBufferAsprintf(&buf, ",resize-hpt=%s", str);
}
+ if (def->features[VIR_DOMAIN_FEATURE_PSERIES] == VIR_TRISTATE_SWITCH_ON) {
+ const char *value;
+
+ for (i = 0; i < VIR_DOMAIN_PSERIES_LAST; i++) {
+ switch ((virDomainPSeries) i) {
+ case VIR_DOMAIN_PSERIES_HPT:
+ if (def->pseries_features[i] != VIR_TRISTATE_SWITCH_ON)
+ break;
+
+ value = virDomainHPTResizingTypeToString(def->pseries_hpt_resizing);
+ if (qemuBuildMachineCommandLinePSeriesFeature(&buf, i, value, qemuCaps) < 0)
+ goto cleanup;
+ break;
+
+ case VIR_DOMAIN_PSERIES_LAST:
+ goto cleanup;
+ }
+ }
+ }
+
if (cpu && cpu->model &&
cpu->mode == VIR_CPU_MODE_HOST_MODEL &&
qemuDomainIsPSeries(def) &&
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6b4bd3cca..edef3838e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3125,6 +3125,13 @@ qemuDomainDefVerifyFeatures(const virDomainDef *def)
return -1;
}
+ if (def->features[VIR_DOMAIN_FEATURE_PSERIES] == VIR_TRISTATE_SWITCH_ON &&
+ !qemuDomainIsPSeries(def)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("pSeries features are only supported for pSeries guests"));
+ return -1;
+ }
+
if (def->features[VIR_DOMAIN_FEATURE_HPT] == VIR_TRISTATE_SWITCH_ON &&
!qemuDomainIsPSeries(def)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
--
2.14.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Jan 23, 2018 at 04:45:01PM +0100, Andrea Bolognani wrote: > We're going to introduce a number of optional, pSeries-specific > features, so we want to have a dedicated sub-element to avoid > cluttering the <domain><features> element. I don't think we want todo this as all. Pretty much *every* single existing <feature> element is architecture specific. Adding a nested level just for pseries is making it different from existing practice and I don't see any benefit to that. 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
On Thu, 2018-01-25 at 15:03 +0000, Daniel P. Berrangé wrote: > On Tue, Jan 23, 2018 at 04:45:01PM +0100, Andrea Bolognani wrote: > > We're going to introduce a number of optional, pSeries-specific > > features, so we want to have a dedicated sub-element to avoid > > cluttering the <domain><features> element. > > I don't think we want todo this as all. Pretty much *every* single > existing <feature> element is architecture specific. Adding a nested > level just for pseries is making it different from existing practice > and I don't see any benefit to that. Well, there's going to be a bunch of them added pretty soon (six or so) plus probably more down the line, so that's already quite a bit of clutter. My reasoning was to try and organize them the way we already do with <kvm> and <hyperv> capabilities - even though pSeries is not a hypervisor per se, you can kinda see sPAPR as a specification implemented by various hypervisors, like PowerVM and in our case QEMU/KVM. But if you think this effort is misguided and they belong to the top-level <features> element, then I'm okay with that too. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Jan 25, 2018 at 05:12:49PM +0100, Andrea Bolognani wrote: > On Thu, 2018-01-25 at 15:03 +0000, Daniel P. Berrangé wrote: > > On Tue, Jan 23, 2018 at 04:45:01PM +0100, Andrea Bolognani wrote: > > > We're going to introduce a number of optional, pSeries-specific > > > features, so we want to have a dedicated sub-element to avoid > > > cluttering the <domain><features> element. > > > > I don't think we want todo this as all. Pretty much *every* single > > existing <feature> element is architecture specific. Adding a nested > > level just for pseries is making it different from existing practice > > and I don't see any benefit to that. > > Well, there's going to be a bunch of them added pretty soon (six > or so) plus probably more down the line, so that's already quite > a bit of clutter. My reasoning was to try and organize them the > way we already do with <kvm> and <hyperv> capabilities - even > though pSeries is not a hypervisor per se, you can kinda see > sPAPR as a specification implemented by various hypervisors, like > PowerVM and in our case QEMU/KVM. But if you think this effort is > misguided and they belong to the top-level <features> element, > then I'm okay with that too. I guess where the nesting makes sense is if there's a chance of having namespace collision between features. eg if both kvm and hyperv had a feature called "pvspinlocks", you might want to enable them separately, so the nesting is important there. If you do think the pSeries nesting is important in this respect, can you still leave the existing "hpt" feature un-nested for sake of full back-compat - just nest new features. 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
On Thu, 2018-01-25 at 16:16 +0000, Daniel P. Berrangé wrote: > > My reasoning was to try and organize them the > > way we already do with <kvm> and <hyperv> capabilities - even > > though pSeries is not a hypervisor per se, you can kinda see > > sPAPR as a specification implemented by various hypervisors, like > > PowerVM and in our case QEMU/KVM. But if you think this effort is > > misguided and they belong to the top-level <features> element, > > then I'm okay with that too. > > I guess where the nesting makes sense is if there's a chance of having > namespace collision between features. eg if both kvm and hyperv > had a feature called "pvspinlocks", you might want to enable them > separately, so the nesting is important there. Mh, I don't foresee that kind of collision happening. We should be safe; and if it ever turns out not to be the case, then we can just nest the new features instead. I'll respin a simpler version of this. Thanks for the feedback :) -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.