9:57 a.m.
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é <berrange(a)redhat.com>
---
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