On 2/18/25 19:12, Daniel P. Berrangé wrote:
> Currently we parse
>
> <os>
> <acpi>
> <table type="slic">...path...</table>
> </acpi>
> </os>
>
> into a flat 'char *slic_table' field which is rather an anti-pattern
> as it has special cased a single attribute type.
>
> This rewrites the internal design to permit multiple table types to
> be parsed, should we add more in future. Each type is permitted to
> only appear once.
>
> The Xen code is fairly dubious in its use of 'slic_table' to hold
> Xen's 'acpi_firmware' config option, as IIUC Xen's config is not
> limited to accepting a single table per file. It takes a concatenation
> of all data and ought to be represented as such. This is left for a
> future contributor to solve.
>
> Signed-off-by: Daniel P. Berrangé <[email protected]>
> ---
> src/conf/domain_conf.c | 87 +++++++++++++++++++++++++--------
> src/conf/domain_conf.h | 21 +++++++-
> src/libxl/libxl_conf.c | 8 ++-
> src/libxl/xen_xl.c | 22 +++++++--
> src/qemu/qemu_command.c | 13 +++--
> src/security/security_dac.c | 18 ++++---
> src/security/security_selinux.c | 16 +++---
> src/security/virt-aa-helper.c | 5 +-
> 8 files changed, 143 insertions(+), 47 deletions(-)
Please consider squashing in the following:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 04fb893587..69e9255494 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17888,29 +17888,30 @@ virDomainDefParseBootAcpiOptions(virDomainDef *def,
{
int n;
g_autofree xmlNodePtr *nodes = NULL;
- g_autofree char *tmp = NULL;
size_t ntables = 0;
virDomainOSACPITableDef **tables = NULL;
size_t i;
- size_t j;
if ((n = virXPathNodeSet("./os/acpi/table", ctxt, &nodes)) < 0)
return -1;
+ if (n == 0)
+ return 0;
+
+ tables = g_new0(virDomainOSACPITableDef *, n);
+
for (i = 0; i < n; i++) {
- int type;
- tmp = virXMLPropString(nodes[i], "type");
+ g_autofree char *path = virXMLNodeContentString(nodes[i]);
+ virDomainOsACPITable type;
+ size_t j;
- if (!tmp) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Missing acpi table type"));
+ if (!path)
goto error;
- }
- if ((type = virDomainOsACPITableTypeFromString(tmp)) < 0) {
- virReportError(VIR_ERR_XML_ERROR,
- _("Unknown acpi table type: %1$s"),
- tmp);
+ if (virXMLPropEnum(nodes[i], "type",
+ virDomainOsACPITableTypeFromString,
+ VIR_XML_PROP_REQUIRED,
+ &type) < 0) {
goto error;
}
@@ -17918,22 +17919,15 @@ virDomainDefParseBootAcpiOptions(virDomainDef *def,
if (tables[j]->type == type) {
virReportError(VIR_ERR_XML_ERROR,
_("ACPI table type '%1$s' may only appear
once"),
- tmp);
+ virDomainOsACPITableTypeToString(type));
goto error;
}
}
- VIR_FREE(tmp);
- if (!(tmp = virXMLNodeContentString(nodes[i])))
- goto error;
-
- tables = g_renew(virDomainOSACPITableDef *, tables, ntables + 1);
tables[ntables] = g_new0(virDomainOSACPITableDef, 1);
tables[ntables]->type = type;
- tables[ntables]->path = virFileSanitizePath(tmp);
+ tables[ntables]->path = virFileSanitizePath(path);
ntables++;
-
- VIR_FREE(tmp);
}
def->os.nacpiTables = ntables;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 7735cce325..e5b0c42171 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2483,7 +2483,7 @@ typedef enum {
VIR_ENUM_DECL(virDomainOsACPITable);
struct _virDomainOSACPITableDef {
- int type;
+ virDomainOsACPITable type;
char *path;
};
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 30a9f806f0..db8c29ec1d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -611,6 +611,8 @@ virDomainObjTaint;
virDomainObjUpdateModificationImpact;
virDomainObjWait;
virDomainObjWaitUntil;
+virDomainOsACPITableTypeFromString;
+virDomainOsACPITableTypeToString;
virDomainOsDefFirmwareTypeFromString;
virDomainOsDefFirmwareTypeToString;
virDomainOSTypeFromString;
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 939478a625..23de0be9db 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -974,7 +974,7 @@ get_files(vahControl * ctl)
if (vah_add_file(&buf, ctl->def->os.dtb, "r") != 0)
goto cleanup;
- for (i = 0; i < def->os.nacpiTables; i++) {
+ for (i = 0; i < ctl->def->os.nacpiTables; i++) {
if (vah_add_file(&buf, ctl->def->os.acpiTables[i]->path, "r") != 0)
goto cleanup;
}
Michal
