On 03/09/2018 10:07 AM, Andrea Bolognani wrote:
> This is the first in a list of pSeries-specific optional features
> that have recently been introduced in QEMU. Along with the feature
> proper, some generic code that will make it easier to implement
> subsequent features is introduced as well.
>
> Signed-off-by: Andrea Bolognani <abologna@xxxxxxxxxx>
> ---
> docs/formatdomain.html.in | 7 +
> docs/schemas/domaincommon.rng | 5 +
> src/conf/domain_conf.c | 19 +++
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_command.c | 149 +++++++++++++++++++++
> src/qemu/qemu_domain.c | 13 ++
> .../pseries-features-invalid-machine.xml | 1 +
> tests/qemuxml2argvdata/pseries-features.args | 2 +-
> tests/qemuxml2argvdata/pseries-features.xml | 1 +
> tests/qemuxml2argvtest.c | 1 +
> tests/qemuxml2xmltest.c | 1 +
> 11 files changed, 199 insertions(+), 1 deletion(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 6fd2189cd2..51400afa49 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -2057,6 +2057,13 @@
> the attribute is not defined, the hypervisor default will be used.
> <span class="since">Since 3.10.0</span> (QEMU/KVM only)
> </dd>
> + <dt><code>htm</code></dt>
> + <dd>Configure HTM (Hardware Transational Memory) availability for
> + pSeries guests. Possible values for the <code>state</code> attribute
> + are <code>on</code> and <code>off</code>. If the attribute is not
> + defined, the hypervisor default will be used.
> + <span class="since">Since 4.2.0</span> (QEMU/KVM only)
> + </dd>
> <dt><code>vmcoreinfo</code></dt>
> <dd>Enable QEMU vmcoreinfo device to let the guest kernel save debug
> details. <span class="since">Since 3.10.0</span> (QEMU only)
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 8165e699d6..b4143f5bc3 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -4797,6 +4797,11 @@
> <optional>
> <ref name="hpt"/>
> </optional>
> + <optional>
> + <element name="htm">
> + <ref name="featurestate"/>
> + </element>
> + </optional>
> <optional>
> <ref name="vmcoreinfo"/>
> </optional>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 70b19311b4..98897d3a63 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",
> + "htm",
> );
>
> VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST,
> @@ -19297,6 +19298,22 @@ virDomainDefParseXML(xmlDocPtr xml,
> }
> break;
>
> + case VIR_DOMAIN_FEATURE_HTM:
> + if (!(tmp = virXMLPropString(nodes[i], "state"))) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("missing state attribute '%s' of feature '%s'"),
> + tmp, virDomainFeatureTypeToString(val));
> + goto error;
> + }
> + if ((def->features[val] = virTristateSwitchTypeFromString(tmp)) < 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("unknown state attribute '%s' of feature '%s'"),
> + tmp, virDomainFeatureTypeToString(val));
> + goto error;
> + }
> + VIR_FREE(tmp);
> + break;
> +
> /* coverity[dead_error_begin] */
> case VIR_DOMAIN_FEATURE_LAST:
> break;
> @@ -21384,6 +21401,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
> case VIR_DOMAIN_FEATURE_VMPORT:
> case VIR_DOMAIN_FEATURE_SMM:
> case VIR_DOMAIN_FEATURE_VMCOREINFO:
> + case VIR_DOMAIN_FEATURE_HTM:
> if (src->features[i] != dst->features[i]) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("State of feature '%s' differs: "
> @@ -26845,6 +26863,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
> case VIR_DOMAIN_FEATURE_PVSPINLOCK:
> case VIR_DOMAIN_FEATURE_VMPORT:
> case VIR_DOMAIN_FEATURE_SMM:
> + case VIR_DOMAIN_FEATURE_HTM:
> switch ((virTristateSwitch) def->features[i]) {
> case VIR_TRISTATE_SWITCH_LAST:
> case VIR_TRISTATE_SWITCH_ABSENT:
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 5859c8f4b1..b87a9d9de2 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1746,6 +1746,7 @@ typedef enum {
> VIR_DOMAIN_FEATURE_IOAPIC,
> VIR_DOMAIN_FEATURE_HPT,
> VIR_DOMAIN_FEATURE_VMCOREINFO,
> + VIR_DOMAIN_FEATURE_HTM,
>
> VIR_DOMAIN_FEATURE_LAST
> } virDomainFeature;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index fa0aa5d5c3..d58211c4c6 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7117,6 +7117,113 @@ qemuBuildNameCommandLine(virCommandPtr cmd,
> return 0;
> }
>
This looks particularly useful for other commands, but...
> +static int
> +virDomainFeatureToQEMUCaps(int feature)
> +{
> + switch ((virDomainFeature) feature) {
> + case VIR_DOMAIN_FEATURE_HTM:
> + return QEMU_CAPS_MACHINE_PSERIES_CAP_HTM;
> + case VIR_DOMAIN_FEATURE_ACPI:
> + case VIR_DOMAIN_FEATURE_APIC:
> + case VIR_DOMAIN_FEATURE_PAE:
> + case VIR_DOMAIN_FEATURE_HAP:
> + case VIR_DOMAIN_FEATURE_VIRIDIAN:
> + case VIR_DOMAIN_FEATURE_PRIVNET:
> + case VIR_DOMAIN_FEATURE_HYPERV:
> + case VIR_DOMAIN_FEATURE_KVM:
> + case VIR_DOMAIN_FEATURE_PVSPINLOCK:
> + case VIR_DOMAIN_FEATURE_CAPABILITIES:
> + case VIR_DOMAIN_FEATURE_PMU:
> + case VIR_DOMAIN_FEATURE_VMPORT:
> + case VIR_DOMAIN_FEATURE_GIC:
> + case VIR_DOMAIN_FEATURE_SMM:
> + case VIR_DOMAIN_FEATURE_IOAPIC:
> + case VIR_DOMAIN_FEATURE_HPT:
> + case VIR_DOMAIN_FEATURE_VMCOREINFO:
> + case VIR_DOMAIN_FEATURE_LAST:
> + default:
> + return -1;
> + }
> +
> + return -1;
> +}
> +
> +static const char*
> +virDomainFeatureToMachineOption(int feature)
> +{
> + switch ((virDomainFeature) feature) {
> + case VIR_DOMAIN_FEATURE_HTM:
> + return "cap-htm";
> + case VIR_DOMAIN_FEATURE_ACPI:
> + case VIR_DOMAIN_FEATURE_APIC:
> + case VIR_DOMAIN_FEATURE_PAE:
> + case VIR_DOMAIN_FEATURE_HAP:
> + case VIR_DOMAIN_FEATURE_VIRIDIAN:
> + case VIR_DOMAIN_FEATURE_PRIVNET:
> + case VIR_DOMAIN_FEATURE_HYPERV:
> + case VIR_DOMAIN_FEATURE_KVM:
> + case VIR_DOMAIN_FEATURE_PVSPINLOCK:
> + case VIR_DOMAIN_FEATURE_CAPABILITIES:
> + case VIR_DOMAIN_FEATURE_PMU:
> + case VIR_DOMAIN_FEATURE_VMPORT:
> + case VIR_DOMAIN_FEATURE_GIC:
> + case VIR_DOMAIN_FEATURE_SMM:
> + case VIR_DOMAIN_FEATURE_IOAPIC:
> + case VIR_DOMAIN_FEATURE_HPT:
> + case VIR_DOMAIN_FEATURE_VMCOREINFO:
> + case VIR_DOMAIN_FEATURE_LAST:
> + default:
> + return NULL;
> + }
> +
> + return NULL;
> +}
> +
> +static int
> +qemuBuildMachineCommandLineFeature(virBufferPtr buf,
> + virDomainFeature feature,
> + const char *value,
> + virQEMUCapsPtr qemuCaps)
> +{
> + const char *name;
> + const char *option;
> + int cap;
> + int ret = -1;
> +
> + if (!(name = virDomainFeatureTypeToString(feature))) {
> + virReportEnumRangeError(virDomainFeature, feature);
> + goto cleanup;
> + }
> +
> + if (!(option = virDomainFeatureToMachineOption(feature))) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Unknown QEMU option for '%s' feature"),
> + name);
> + goto cleanup;
> + }
> +
> + if ((cap = virDomainFeatureToQEMUCaps(feature)) < 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Unknown QEMU capability for '%s' feature"),
> + name);
> + goto cleanup;
> + }
> +
> + if (cap > 0 && !virQEMUCapsGet(qemuCaps, cap)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("'%s' 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,
> @@ -7343,6 +7450,48 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
> virBufferAsprintf(&buf, ",resize-hpt=%s", str);
> }
>
> + for (i = 0; i < VIR_DOMAIN_FEATURE_LAST; i++) {
> + const char *value;
> +
> + switch ((virDomainFeature) i) {
> + case VIR_DOMAIN_FEATURE_HTM:
> + if (def->features[i] == VIR_TRISTATE_SWITCH_ABSENT)
> + break;
> +
> + if (!(value = virTristateSwitchTypeToString(def->features[i]))) {
> + virReportEnumRangeError(virTristateSwitch, def->features[i]);
> + goto cleanup;
> + }
> +
> + if (qemuBuildMachineCommandLineFeature(&buf, i, value, qemuCaps) < 0)
> + goto cleanup;
> + break;
> +
> + case VIR_DOMAIN_FEATURE_ACPI:
> + case VIR_DOMAIN_FEATURE_APIC:
> + case VIR_DOMAIN_FEATURE_PAE:
> + case VIR_DOMAIN_FEATURE_HAP:
> + case VIR_DOMAIN_FEATURE_VIRIDIAN:
> + case VIR_DOMAIN_FEATURE_PRIVNET:
> + case VIR_DOMAIN_FEATURE_HYPERV:
> + case VIR_DOMAIN_FEATURE_KVM:
> + case VIR_DOMAIN_FEATURE_PVSPINLOCK:
> + case VIR_DOMAIN_FEATURE_CAPABILITIES:
> + case VIR_DOMAIN_FEATURE_PMU:
> + case VIR_DOMAIN_FEATURE_VMPORT:
> + case VIR_DOMAIN_FEATURE_GIC:
> + case VIR_DOMAIN_FEATURE_SMM:
> + case VIR_DOMAIN_FEATURE_IOAPIC:
> + case VIR_DOMAIN_FEATURE_HPT:
> + case VIR_DOMAIN_FEATURE_VMCOREINFO:
> + break;
> +
> + case VIR_DOMAIN_FEATURE_LAST:
> + default:
> + break;
> + }
> + }
> +
It's only generated/built for one - seems like a lot of work for little
gain unless of course there's a plan to add in all the others. Without
adding in others, one is left to wonder the "best" way to add things in
the future.
John
> 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 b55013de6a..35d84adae3 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3377,6 +3377,19 @@ qemuDomainDefValidateFeatures(const virDomainDef *def)
> }
> break;
>
> + case VIR_DOMAIN_FEATURE_HTM:
> + if (def->features[i] != VIR_TRISTATE_SWITCH_ABSENT &&
> + !qemuDomainIsPSeries(def)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("The '%s' feature is not supported for "
> + "architecture '%s' or machine type '%s'"),
> + featureName,
> + virArchToString(def->os.arch),
> + def->os.machine);
> + return -1;
> + }
> + break;
> +
> case VIR_DOMAIN_FEATURE_ACPI:
> case VIR_DOMAIN_FEATURE_APIC:
> case VIR_DOMAIN_FEATURE_PAE:
> diff --git a/tests/qemuxml2argvdata/pseries-features-invalid-machine.xml b/tests/qemuxml2argvdata/pseries-features-invalid-machine.xml
> index 5a6bb02d5b..76cbf0737f 100644
> --- a/tests/qemuxml2argvdata/pseries-features-invalid-machine.xml
> +++ b/tests/qemuxml2argvdata/pseries-features-invalid-machine.xml
> @@ -9,6 +9,7 @@
> <features>
> <!-- pSeries features can't be enabled for non-pSeries guests -->
> <hpt resizing='enabled'/>
> + <htm state='on'/>
> </features>
> <devices>
> <emulator>/usr/bin/qemu-system-x86_64</emulator>
> diff --git a/tests/qemuxml2argvdata/pseries-features.args b/tests/qemuxml2argvdata/pseries-features.args
> index 8cdb329651..0517ca8237 100644
> --- a/tests/qemuxml2argvdata/pseries-features.args
> +++ b/tests/qemuxml2argvdata/pseries-features.args
> @@ -7,7 +7,7 @@ QEMU_AUDIO_DRV=none \
> /usr/bin/qemu-system-ppc64 \
> -name guest \
> -S \
> --machine pseries,accel=tcg,resize-hpt=required \
> +-machine pseries,accel=tcg,resize-hpt=required,cap-htm=on \
> -m 512 \
> -smp 1,sockets=1,cores=1,threads=1 \
> -uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \
> diff --git a/tests/qemuxml2argvdata/pseries-features.xml b/tests/qemuxml2argvdata/pseries-features.xml
> index 5dd0dbd0be..a0e98db8b2 100644
> --- a/tests/qemuxml2argvdata/pseries-features.xml
> +++ b/tests/qemuxml2argvdata/pseries-features.xml
> @@ -10,6 +10,7 @@
> </os>
> <features>
> <hpt resizing='required'/>
> + <htm state='on'/>
> </features>
> <clock offset='utc'/>
> <on_poweroff>destroy</on_poweroff>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index ca53912ebe..03f8c429e0 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -1898,6 +1898,7 @@ mymain(void)
>
> DO_TEST("pseries-features",
> QEMU_CAPS_MACHINE_OPT,
> + QEMU_CAPS_MACHINE_PSERIES_CAP_HTM,
> QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT);
> DO_TEST_FAILURE("pseries-features",
> QEMU_CAPS_MACHINE_OPT);
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index a363b55446..b9a8bd6f14 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -765,6 +765,7 @@ mymain(void)
>
> DO_TEST("pseries-features",
> QEMU_CAPS_MACHINE_OPT,
> + QEMU_CAPS_MACHINE_PSERIES_CAP_HTM,
> QEMU_CAPS_MACHINE_PSERIES_RESIZE_HPT);
>
> DO_TEST("pseries-serial-native",
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
