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 f42b7075ad7f87af9a1cb8c3f7dec8d43b6a6808..40c6914fa985e6ee7edf662d75f3c06a96593049 100644 (file)
--- 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 e7947741bd9d9f0a555d5cd59ff767eed34aee4a..c0fe0c6e753a6954efb24120a79a6df514c9415e 100644 (file)
--- 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 30a9f806f0a2b73ae9e8acf3969f63977e78527f..db8c29ec1de2a7e61678c30a6ffcccd8f7c59861 100644 (file)
--- 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 e06655605bc2df1dfdfbad7c08444e5ad83184d8..16cd11c21572b03161cba81b1613859c417ff198 100644 (file)
--- 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 c8371fac02ebfa267c8b358d9e0acec4889e89ad..96b8b0511c516ff2b5b59f09d4d4a54ffe9b095f 100644 (file)
--- 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 0ad73af335974a236341fa85d02c83f14af08934..6048c755fc0ca34fda7ebfc84571f900869555ee 100644 (file)
--- 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 0505f4e4a35b2eaaa46921bd0273666801a6c69c..b4d61bc576c7fb2d78c52567a7745e831faee5f2 100644 (file)
--- 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 cdc32d9b34a6bbc984528cd7ad063200bddf926d..b8659e33d62ebbd6ed6894ffb5bfee2e7c77fec9 100644 (file)
--- 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 c255b64f35dd29dc20826f5661f547d4f16fcb94..fad2c89304e6fabbee7e2fc26962babd9a9ac8be 100644 (file)
--- 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)