From: Daniel P. Berrangé <berrange@redhat.com>
Date: Mon, 17 Feb 2025 16:30:07 +0000 (+0000)
Subject: conf: introduce support for multiple ACPI tables
X-Git-Tag: v11.2.0-rc1~258
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=55f48d38522a4657815668dae9ed9184c8870766;p=thirdparty%2Flibvirt.git

conf: introduce support for multiple ACPI tables

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 currently
permitted to only appear once.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f42b7075ad..40c6914fa9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1457,6 +1457,11 @@ VIR_ENUM_IMPL(virDomainOsDefFirmwareFeature,
               "secure-boot",
 );
 
+VIR_ENUM_IMPL(virDomainOsACPITable,
+              VIR_DOMAIN_OS_ACPI_TABLE_TYPE_LAST,
+              "slic",
+);
+
 VIR_ENUM_IMPL(virDomainCFPC,
               VIR_DOMAIN_CFPC_LAST,
               "none",
@@ -3899,6 +3904,15 @@ virDomainSecDefFree(virDomainSecDef *def)
     g_free(def);
 }
 
+void virDomainOSACPITableDefFree(virDomainOSACPITableDef *def)
+{
+    if (!def)
+        return;
+    g_free(def->path);
+    g_free(def);
+}
+
+
 static void
 virDomainOSDefClear(virDomainOSDef *os)
 {
@@ -3924,7 +3938,9 @@ virDomainOSDefClear(virDomainOSDef *os)
     g_free(os->cmdline);
     g_free(os->dtb);
     g_free(os->root);
-    g_free(os->slic_table);
+    for (i = 0; i < os->nacpiTables; i++)
+        virDomainOSACPITableDefFree(os->acpiTables[i]);
+    g_free(os->acpiTables);
     virDomainLoaderDefFree(os->loader);
     g_free(os->bootloader);
     g_free(os->bootloaderArgs);
@@ -17858,40 +17874,57 @@ 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;
 
     if ((n = virXPathNodeSet("./os/acpi/table", ctxt, &nodes)) < 0)
         return -1;
 
-    if (n > 1) {
-        virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("Only one acpi table is supported"));
-        return -1;
-    }
+    if (n == 0)
+        return 0;
 
-    if (n == 1) {
-        tmp = virXMLPropString(nodes[0], "type");
+    tables = g_new0(virDomainOSACPITableDef *, n);
+    for (i = 0; i < n; i++) {
+        g_autofree char *path = virXMLNodeContentString(nodes[i]);
+        virDomainOsACPITable type;
+        size_t j;
 
-        if (!tmp) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("Missing acpi table type"));
-            return -1;
-        }
+        if (!path)
+            goto error;
 
-        if (STREQ_NULLABLE(tmp, "slic")) {
-            VIR_FREE(tmp);
-            if (!(tmp = virXMLNodeContentString(nodes[0])))
-                return -1;
+        if (virXMLPropEnum(nodes[i], "type",
+                           virDomainOsACPITableTypeFromString,
+                           VIR_XML_PROP_REQUIRED,
+                           &type) < 0)
+            goto error;
 
-            def->os.slic_table = virFileSanitizePath(tmp);
-        } else {
-            virReportError(VIR_ERR_XML_ERROR,
-                           _("Unknown acpi table type: %1$s"),
-                           tmp);
-            return -1;
+        for (j = 0; j < i; j++) {
+            if (tables[j]->type == type) {
+                virReportError(VIR_ERR_XML_ERROR,
+                               _("ACPI table type '%1$s' may only appear once"),
+                               virDomainOsACPITableTypeToString(type));
+                goto error;
+            }
         }
+
+        tables[ntables] = g_new0(virDomainOSACPITableDef, 1);
+        tables[ntables]->type = type;
+        tables[ntables]->path = virFileSanitizePath(path);
+        ntables++;
     }
 
+    def->os.nacpiTables = ntables;
+    def->os.acpiTables = tables;
+
     return 0;
+
+ error:
+    for (i = 0; i < ntables; i++) {
+        virDomainOSACPITableDefFree(tables[i]);
+    }
+    g_free(tables);
+    return -1;
 }
 
 
@@ -28418,11 +28451,16 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def,
                           def->os.dtb);
     virBufferEscapeString(buf, "<root>%s</root>\n",
                           def->os.root);
-    if (def->os.slic_table) {
+
+    if (def->os.nacpiTables) {
         virBufferAddLit(buf, "<acpi>\n");
         virBufferAdjustIndent(buf, 2);
-        virBufferEscapeString(buf, "<table type='slic'>%s</table>\n",
-                              def->os.slic_table);
+        for (i = 0; i < def->os.nacpiTables; i++) {
+            virBufferAsprintf(buf, "<table type='%s'>",
+                              virDomainOsACPITableTypeToString(def->os.acpiTables[i]->type));
+            virBufferEscapeString(buf, "%s</table>\n",
+                                  def->os.acpiTables[i]->path);
+        }
         virBufferAdjustIndent(buf, -2);
         virBufferAddLit(buf, "</acpi>\n");
     }
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e7947741bd..c0fe0c6e75 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2481,6 +2481,24 @@ typedef enum {
 
 VIR_ENUM_DECL(virDomainOsDefFirmwareFeature);
 
+typedef enum {
+    VIR_DOMAIN_OS_ACPI_TABLE_TYPE_SLIC,
+
+    VIR_DOMAIN_OS_ACPI_TABLE_TYPE_LAST
+} virDomainOsACPITable;
+
+VIR_ENUM_DECL(virDomainOsACPITable);
+
+struct _virDomainOSACPITableDef {
+    virDomainOsACPITable type;
+    char *path;
+};
+
+typedef struct _virDomainOSACPITableDef virDomainOSACPITableDef;
+void virDomainOSACPITableDefFree(virDomainOSACPITableDef *def);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainOSACPITableDef, virDomainOSACPITableDefFree);
+
+
 struct _virDomainOSDef {
     int type;
     virDomainOsDefFirmware firmware;
@@ -2503,7 +2521,8 @@ struct _virDomainOSDef {
     char *cmdline;
     char *dtb;
     char *root;
-    char *slic_table;
+    size_t nacpiTables;
+    virDomainOSACPITableDef **acpiTables;
     virDomainLoaderDef *loader;
     char *bootloader;
     char *bootloaderArgs;
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/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index e06655605b..16cd11c215 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -582,8 +582,9 @@ libxlMakeDomBuildInfo(virDomainDef *def,
                           VIR_TRISTATE_SWITCH_ON);
 #endif
 
-        /* copy SLIC table path to acpi_firmware */
-        b_info->u.hvm.acpi_firmware = g_strdup(def->os.slic_table);
+        /* copy the table path to acpi_firmware */
+        if (def->os.nacpiTables)
+            b_info->u.hvm.acpi_firmware = g_strdup(def->os.acpiTables[0]->path);
 
         if (def->nsounds > 0) {
             /*
diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c
index c8371fac02..96b8b0511c 100644
--- a/src/libxl/xen_xl.c
+++ b/src/libxl/xen_xl.c
@@ -106,6 +106,7 @@ xenParseXLOS(virConf *conf, virDomainDef *def, virCaps *caps)
         g_autofree char *bios = NULL;
         g_autofree char *bios_path = NULL;
         g_autofree char *boot = NULL;
+        g_autofree char *slic = NULL;
         int val = 0;
 
         if (xenConfigGetString(conf, "bios", &bios, NULL) < 0)
@@ -133,8 +134,15 @@ xenParseXLOS(virConf *conf, virDomainDef *def, virCaps *caps)
             }
         }
 
-        if (xenConfigCopyStringOpt(conf, "acpi_firmware", &def->os.slic_table) < 0)
+        if (xenConfigCopyStringOpt(conf, "acpi_firmware", &slic) < 0)
             return -1;
+        if (slic != NULL) {
+            def->os.nacpiTables = 1;
+            def->os.acpiTables = g_new0(virDomainOSACPITableDef *, 1);
+            def->os.acpiTables[0] = g_new0(virDomainOSACPITableDef, 1);
+            def->os.acpiTables[0]->type = VIR_DOMAIN_OS_ACPI_TABLE_TYPE_SLIC;
+            def->os.acpiTables[0]->path = g_steal_pointer(&slic);
+        }
 
         if (xenConfigCopyStringOpt(conf, "kernel", &def->os.kernel) < 0)
             return -1;
@@ -1130,8 +1138,9 @@ xenFormatXLOS(virConf *conf, virDomainDef *def)
                 return -1;
         }
 
-        if (def->os.slic_table &&
-            xenConfigSetString(conf, "acpi_firmware", def->os.slic_table) < 0)
+        if (def->os.nacpiTables &&
+            xenConfigSetString(conf, "acpi_firmware",
+                               def->os.acpiTables[0]->path) < 0)
             return -1;
 
         if (def->os.kernel &&
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0ad73af335..6048c755fc 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -127,6 +127,11 @@ VIR_ENUM_IMPL(qemuNumaPolicy,
               "restrictive",
 );
 
+VIR_ENUM_DECL(qemuACPITableSIG);
+VIR_ENUM_IMPL(qemuACPITableSIG,
+              VIR_DOMAIN_OS_ACPI_TABLE_TYPE_LAST,
+              "SLIC");
+
 
 const char *
 qemuAudioDriverTypeToString(virDomainAudioType type)
@@ -5995,6 +6000,7 @@ qemuBuildBootCommandLine(virCommand *cmd,
 {
     g_auto(virBuffer) boot_buf = VIR_BUFFER_INITIALIZER;
     g_autofree char *boot_opts_str = NULL;
+    size_t i;
 
     if (def->os.bootmenu) {
         if (def->os.bootmenu == VIR_TRISTATE_BOOL_YES)
@@ -6028,11 +6034,12 @@ qemuBuildBootCommandLine(virCommand *cmd,
         virCommandAddArgList(cmd, "-append", def->os.cmdline, NULL);
     if (def->os.dtb)
         virCommandAddArgList(cmd, "-dtb", def->os.dtb, NULL);
-    if (def->os.slic_table) {
+    for (i = 0; i < def->os.nacpiTables; i++) {
         g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
         virCommandAddArg(cmd, "-acpitable");
-        virBufferAddLit(&buf, "sig=SLIC,file=");
-        virQEMUBuildBufferEscapeComma(&buf, def->os.slic_table);
+        virBufferAsprintf(&buf, "sig=%s,file=",
+                          qemuACPITableSIGTypeToString(def->os.acpiTables[i]->type));
+        virQEMUBuildBufferEscapeComma(&buf, def->os.acpiTables[i]->path);
         virCommandAddArgBuffer(cmd, &buf);
     }
 
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 0505f4e4a3..b4d61bc576 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -2050,9 +2050,10 @@ virSecurityDACRestoreAllLabel(virSecurityManager *mgr,
         virSecurityDACRestoreFileLabel(mgr, def->os.dtb) < 0)
         rc = -1;
 
-    if (def->os.slic_table &&
-        virSecurityDACRestoreFileLabel(mgr, def->os.slic_table) < 0)
-        rc = -1;
+    for (i = 0; i < def->os.nacpiTables; i++) {
+        if (virSecurityDACRestoreFileLabel(mgr, def->os.acpiTables[i]->path) < 0)
+            rc = -1;
+    }
 
     if (def->pstore &&
         virSecurityDACRestoreFileLabel(mgr, def->pstore->path) < 0)
@@ -2300,11 +2301,12 @@ virSecurityDACSetAllLabel(virSecurityManager *mgr,
                                    user, group, true) < 0)
         return -1;
 
-    if (def->os.slic_table &&
-        virSecurityDACSetOwnership(mgr, NULL,
-                                   def->os.slic_table,
-                                   user, group, true) < 0)
-        return -1;
+    for (i = 0; i < def->os.nacpiTables; i++) {
+        if (virSecurityDACSetOwnership(mgr, NULL,
+                                       def->os.acpiTables[i]->path,
+                                       user, group, true) < 0)
+            return -1;
+    }
 
     if (def->pstore &&
         virSecurityDACSetOwnership(mgr, NULL,
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index cdc32d9b34..b8659e33d6 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -3013,9 +3013,10 @@ virSecuritySELinuxRestoreAllLabel(virSecurityManager *mgr,
         virSecuritySELinuxRestoreFileLabel(mgr, def->os.dtb, true) < 0)
         rc = -1;
 
-    if (def->os.slic_table &&
-        virSecuritySELinuxRestoreFileLabel(mgr, def->os.slic_table, true) < 0)
-        rc = -1;
+    for (i = 0; i < def->os.nacpiTables; i++) {
+        if (virSecuritySELinuxRestoreFileLabel(mgr, def->os.acpiTables[i]->path, true) < 0)
+            rc = -1;
+    }
 
     if (def->pstore &&
         virSecuritySELinuxRestoreFileLabel(mgr, def->pstore->path, true) < 0)
@@ -3443,10 +3444,11 @@ virSecuritySELinuxSetAllLabel(virSecurityManager *mgr,
                                      data->content_context, true) < 0)
         return -1;
 
-    if (def->os.slic_table &&
-        virSecuritySELinuxSetFilecon(mgr, def->os.slic_table,
-                                     data->content_context, true) < 0)
-        return -1;
+    for (i = 0; i < def->os.nacpiTables; i++) {
+        if (virSecuritySELinuxSetFilecon(mgr, def->os.acpiTables[i]->path,
+                                         data->content_context, true) < 0)
+            return -1;
+    }
 
     if (def->pstore &&
         virSecuritySELinuxSetFilecon(mgr, def->pstore->path,
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index c255b64f35..fad2c89304 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -974,9 +974,10 @@ get_files(vahControl * ctl)
         if (vah_add_file(&buf, ctl->def->os.dtb, "r") != 0)
             goto cleanup;
 
-    if (ctl->def->os.slic_table)
-        if (vah_add_file(&buf, ctl->def->os.slic_table, "r") != 0)
+    for (i = 0; i < ctl->def->os.nacpiTables; i++) {
+        if (vah_add_file(&buf, ctl->def->os.acpiTables[i]->path, "r") != 0)
             goto cleanup;
+    }
 
     if (ctl->def->pstore)
         if (vah_add_file(&buf, ctl->def->pstore->path, "rw") != 0)