[libvirt] [PATCH 7/8] qemu: Implement the HTM pSeries feature

John Ferlan jferlan at redhat.com
Fri Mar 16 15:33:17 UTC 2018



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 at redhat.com>
> ---
>  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",
> 




More information about the libvir-list mailing list