Re: [PATCH v2 3/7] src: introduce 'raw' and 'rawset' ACPI table types

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

 



CC Marek who did the original libxl integration of 'type=slic' mapping
to Xen's  'acpi_firmware' property.

Marek, this introduces a new 'rawset' ACPI type for direct passthrough
of ACPI table lists, which is intended to match Xen's semantics
exactly.

The later patch in this series changes the libxl driver to accept
both the old (slic) and new (rawset) types, but when emitting
XML libvirt will use the new type.

There's a possible risk this could upset apps if they're consuming
XML from libvirt and validating the type.

On Wed, Feb 26, 2025 at 08:16:15PM +0000, Daniel P. Berrangé wrote:
> The QEMU driver has only accepted type=slic even though QEMU is able to
> accept individual tables of any type, without needing to specify a
> signature. Introduce type=raw to address this usage scenario. Contrary
> to other types, this one may appear multiple times.
> 
> The Xen driver has mistakenly accepted type=slic and use it to set the
> Xen acpi_firmware setting, which performs a simple passthrough of
> multiple concatenated data table. Introduce type=rawset to address
> this usage scenario.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
>  docs/formatdomain.rst             | 19 ++++++++++++++++---
>  src/conf/domain_conf.c            |  5 ++++-
>  src/conf/domain_conf.h            |  2 ++
>  src/conf/schemas/domaincommon.rng |  6 +++++-
>  src/libxl/libxl_domain.c          |  7 +++++++
>  src/qemu/qemu_command.c           |  2 ++
>  src/qemu/qemu_validate.c          |  7 +++++++
>  7 files changed, 43 insertions(+), 5 deletions(-)
> 
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index fdb3c2459e..d3add663e2 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -490,9 +490,22 @@ These options apply to any form of booting of the guest OS.
>     ...
>  
>  ``acpi``
> -   The ``table`` element contains a fully-qualified path to the ACPI table. The
> -   ``type`` attribute contains the ACPI table type (currently only ``slic`` is
> -   supported) :since:`Since 1.3.5 (QEMU)` :since:`Since 5.9.0 (Xen)`
> +   The ``table`` element contains a fully-qualified path to the ACPI table,
> +   with the ``type`` attribute dictating what data must be present in the
> +   file:
> +
> +   * ``raw``: a single ACPI table with header and data, with ACPI
> +     signature auto-detected from header (:since:`Since 11.2.0`).
> +   * ``rawset``: concatenation of multiple ACPI tables with header
> +     and data, each with any ACPI signature, auto-detected from header
> +     (:since:`Since 11.2.0`).
> +   * ``slic``: a single ACPI table with header and data, providing
> +     software licensing information. The ACPI table signature in the
> +     header will be forced to ``SLIC`` (:since:`Since 1.3.5 (QEMU)`,
> +     mis-interpreted as ``rawset`` :since:`Since 5.9.0 (Xen)`).
> +
> +   Each type may be used only once, except for ``raw`` which can
> +   appear multiple times.
>  
>  
>  SMBIOS System Information
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index fc8ed9fc54..530473e200 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1459,6 +1459,8 @@ VIR_ENUM_IMPL(virDomainOsDefFirmwareFeature,
>  
>  VIR_ENUM_IMPL(virDomainOsACPITable,
>                VIR_DOMAIN_OS_ACPI_TABLE_TYPE_LAST,
> +              "raw",
> +              "rawset",
>                "slic",
>  );
>  
> @@ -17925,7 +17927,8 @@ virDomainDefParseBootAcpiOptions(virDomainDef *def,
>              goto error;
>  
>          for (j = 0; j < i; j++) {
> -            if (tables[j]->type == type) {
> +            if (tables[j]->type == type &&
> +                type != VIR_DOMAIN_OS_ACPI_TABLE_TYPE_RAW) {
>                  virReportError(VIR_ERR_XML_ERROR,
>                                 _("ACPI table type '%1$s' may only appear once"),
>                                 virDomainOsACPITableTypeToString(type));
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index cc9fd503fa..299e2cbd5b 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2475,6 +2475,8 @@ typedef enum {
>  VIR_ENUM_DECL(virDomainOsDefFirmwareFeature);
>  
>  typedef enum {
> +    VIR_DOMAIN_OS_ACPI_TABLE_TYPE_RAW,
> +    VIR_DOMAIN_OS_ACPI_TABLE_TYPE_RAWSET,
>      VIR_DOMAIN_OS_ACPI_TABLE_TYPE_SLIC,
>  
>      VIR_DOMAIN_OS_ACPI_TABLE_TYPE_LAST
> diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
> index 824da9d066..b5eaf7c233 100644
> --- a/src/conf/schemas/domaincommon.rng
> +++ b/src/conf/schemas/domaincommon.rng
> @@ -7180,7 +7180,11 @@
>        <zeroOrMore>
>          <element name="table">
>            <attribute name="type">
> -            <value>slic</value>
> +            <choice>
> +              <value>raw</value>
> +              <value>rawset</value>
> +              <value>slic</value>
> +            </choice>
>            </attribute>
>            <ref name="absFilePath"/>
>          </element>
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index efd01840de..e564d9e5fe 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -336,6 +336,13 @@ libxlDomainDefValidate(const virDomainDef *def,
>          case VIR_DOMAIN_OS_ACPI_TABLE_TYPE_SLIC:
>              break;
>  
> +        case VIR_DOMAIN_OS_ACPI_TABLE_TYPE_RAW:
> +        case VIR_DOMAIN_OS_ACPI_TABLE_TYPE_RAWSET:
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("ACPI table type '%1$s' is not supported"),
> +                           virDomainOsACPITableTypeToString(def->os.acpiTables[i]->type));
> +            return -1;
> +
>          default:
>          case VIR_DOMAIN_OS_ACPI_TABLE_TYPE_LAST:
>              virReportEnumRangeError(virDomainOsACPITable,
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 6048c755fc..bef06049ec 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -130,6 +130,8 @@ VIR_ENUM_IMPL(qemuNumaPolicy,
>  VIR_ENUM_DECL(qemuACPITableSIG);
>  VIR_ENUM_IMPL(qemuACPITableSIG,
>                VIR_DOMAIN_OS_ACPI_TABLE_TYPE_LAST,
> +              "", /* raw */
> +              "", /* rawset */
>                "SLIC");
>  
>  
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index 1759ab4e6e..de36bfc7ea 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -747,6 +747,13 @@ qemuValidateDomainDefBoot(const virDomainDef *def,
>          case VIR_DOMAIN_OS_ACPI_TABLE_TYPE_SLIC:
>              break;
>  
> +        case VIR_DOMAIN_OS_ACPI_TABLE_TYPE_RAW:
> +        case VIR_DOMAIN_OS_ACPI_TABLE_TYPE_RAWSET:
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("ACPI table type '%1$s' is not supported"),
> +                           virDomainOsACPITableTypeToString(def->os.acpiTables[i]->type));
> +            return -1;
> +
>          default:
>          case VIR_DOMAIN_OS_ACPI_TABLE_TYPE_LAST:
>              virReportEnumRangeError(virDomainOsACPITable,
> -- 
> 2.47.1
> 

With 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 :|




[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