[libvirt] [PATCHv2 0/3] qemu: support for SLIC ACPI tables

Windows uses this BLOB for activation purposes. https://bugzilla.redhat.com/show_bug.cgi?id=1327537 In v2: * make <table> a subelement of <acpi> * minor documentation fixes * comma-escape the value on QEMU command line * mention that this option was added in 2009 Ján Tomko (3): conf: add <acpi><table> to <os> qemu: format SLIC ACPI table command line security: label the slic_table docs/formatdomain.html.in | 8 ++++ docs/schemas/domaincommon.rng | 18 +++++++++ src/conf/domain_conf.c | 46 ++++++++++++++++++++++ src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 7 ++++ src/security/security_dac.c | 5 +++ src/security/security_selinux.c | 5 +++ src/security/virt-aa-helper.c | 4 ++ .../qemuxml2argvdata/qemuxml2argv-acpi-table.args | 19 +++++++++ tests/qemuxml2argvdata/qemuxml2argv-acpi-table.xml | 30 ++++++++++++++ tests/qemuxml2argvtest.c | 2 + .../qemuxml2xmlout-acpi-table.xml | 34 ++++++++++++++++ tests/qemuxml2xmltest.c | 3 ++ 13 files changed, 182 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-acpi-table.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-acpi-table.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-acpi-table.xml -- 2.7.3

Add a new element to <domain> XML: <os> <acpi> <table type="slic">/path/to/acpi/table/file</table> </acpi> </os> To supply a path to a SLIC (Software Licensing) ACPI table blob. https://bugzilla.redhat.com/show_bug.cgi?id=1327537 --- docs/formatdomain.html.in | 8 ++++ docs/schemas/domaincommon.rng | 18 +++++++++ src/conf/domain_conf.c | 46 ++++++++++++++++++++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-acpi-table.xml | 30 ++++++++++++++ .../qemuxml2xmlout-acpi-table.xml | 34 ++++++++++++++++ tests/qemuxml2xmltest.c | 3 ++ 7 files changed, 140 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-acpi-table.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-acpi-table.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 72bfa35..9b3f9ee 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -277,6 +277,9 @@ <initrd>/root/f8-i386-initrd</initrd> <cmdline>console=ttyS0 ks=http://example.com/f8-i386/os/</cmdline> <dtb>/root/ppc.dtb</dtb> + <acpi> + <table type='slic'>/path/to/slic.dat</table> + </acpi> </os> ...</pre> @@ -302,6 +305,11 @@ <dd>The contents of this element specify the fully-qualified path to the (optional) device tree binary (dtb) image in the host OS. <span class="since">Since 1.0.4</span></dd> + <dt><code>acpi</code></dt> + <dd>The <code>table</code> element contains a fully-qualified path + to the ACPI table. The <code>type</code> attribute contains the + ACPI table type (currently only <code>slic</code> is supported) + <span class="since">Since 1.3.5 (QEMU only)</span></dd> </dl> <h4><a name="elementsOSContainer">Container boot</a></h4> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 903fd7e..bbe6761 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -306,6 +306,9 @@ <optional> <ref name="bios"/> </optional> + <optional> + <ref name="acpiTable"/> + </optional> </interleave> </element> </define> @@ -4505,6 +4508,21 @@ </data> </define> + <define name="acpiTable"> + <element name="acpi"> + <zeroOrMore> + <element name="table"> + <attribute name="type"> + <choice> + <value>slic</value> + </choice> + </attribute> + <ref name="absFilePath"/> + </element> + </zeroOrMore> + </element> + </define> + <define name="smbios"> <element name="smbios"> <attribute name="mode"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fb05bf7..d85b4b8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2619,6 +2619,7 @@ void virDomainDefFree(virDomainDefPtr def) VIR_FREE(def->os.cmdline); VIR_FREE(def->os.dtb); VIR_FREE(def->os.root); + VIR_FREE(def->os.slic_table); virDomainLoaderDefFree(def->os.loader); VIR_FREE(def->os.bootloader); VIR_FREE(def->os.bootloaderArgs); @@ -15115,6 +15116,8 @@ virDomainDefParseBootOptions(virDomainDefPtr def, virHashTablePtr *bootHash) { xmlNodePtr *nodes = NULL; + xmlNodePtr oldnode; + char *tmp = NULL; int ret = -1; size_t i; int n; @@ -15175,6 +15178,40 @@ virDomainDefParseBootOptions(virDomainDefPtr def, } if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { + if ((n = virXPathNodeSet("./os/acpi/table", ctxt, &nodes)) < 0) + goto error; + + if (n > 1) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Only one acpi table is supported")); + goto error; + } + + if (n == 1) { + oldnode = ctxt->node; + ctxt->node = nodes[0]; + tmp = virXPathString("string(./@type)", ctxt); + + if (!tmp) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing acpi table type")); + goto error; + } + + if (STREQ_NULLABLE(tmp, "slic")) { + VIR_FREE(tmp); + tmp = virXPathString("string(.)", ctxt); + def->os.slic_table = virFileSanitizePath(tmp); + VIR_FREE(tmp); + } else { + virReportError(VIR_ERR_XML_ERROR, + _("Unknown acpi table type: %s"), + tmp); + goto error; + } + ctxt->node = oldnode; + } + if (virDomainDefParseBootXML(ctxt, def) < 0) goto error; if (!(*bootHash = virHashCreate(5, NULL))) @@ -15185,6 +15222,7 @@ virDomainDefParseBootOptions(virDomainDefPtr def, error: VIR_FREE(nodes); + VIR_FREE(tmp); return ret; } @@ -22516,6 +22554,14 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->os.dtb); virBufferEscapeString(buf, "<root>%s</root>\n", def->os.root); + if (def->os.slic_table) { + virBufferAddLit(buf, "<acpi>\n"); + virBufferAdjustIndent(buf, 2); + virBufferEscapeString(buf, "<table type='slic'>%s</table>\n", + def->os.slic_table); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</acpi>\n"); + } if (!def->os.bootloader) { for (n = 0; n < def->os.nBootDevs; n++) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 82e581b..d25a61d 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1746,6 +1746,7 @@ struct _virDomainOSDef { char *cmdline; char *dtb; char *root; + char *slic_table; virDomainLoaderDefPtr loader; char *bootloader; char *bootloaderArgs; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-acpi-table.xml b/tests/qemuxml2argvdata/qemuxml2argv-acpi-table.xml new file mode 100644 index 0000000..9b8f590 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-acpi-table.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + <acpi> + <table type='slic'>/var/lib/libvirt/acpi/slic.dat</table> + </acpi> + </os> + <features> + <acpi/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-acpi-table.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-acpi-table.xml new file mode 100644 index 0000000..2eb25a3 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-acpi-table.xml @@ -0,0 +1,34 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <acpi> + <table type='slic'>/var/lib/libvirt/acpi/slic.dat</table> + </acpi> + <boot dev='hd'/> + </os> + <features> + <acpi/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index c85cd60..e23831b 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -806,6 +806,9 @@ mymain(void) DO_TEST("virtio-input-passthrough"); virObjectUnref(cfg); + + DO_TEST("acpi-table"); + qemuTestDriverFree(&driver); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; -- 2.7.3

On Mon, May 23, 2016 at 20:01:16 +0200, Ján Tomko wrote:
Add a new element to <domain> XML: <os> <acpi> <table type="slic">/path/to/acpi/table/file</table> </acpi> </os>
To supply a path to a SLIC (Software Licensing) ACPI table blob.
https://bugzilla.redhat.com/show_bug.cgi?id=1327537 --- docs/formatdomain.html.in | 8 ++++ docs/schemas/domaincommon.rng | 18 +++++++++ src/conf/domain_conf.c | 46 ++++++++++++++++++++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-acpi-table.xml | 30 ++++++++++++++ .../qemuxml2xmlout-acpi-table.xml | 34 ++++++++++++++++ tests/qemuxml2xmltest.c | 3 ++ 7 files changed, 140 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-acpi-table.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-acpi-table.xml
ACK

<os> <acpi> <table type="slic">/path/to/acpi/table/file</table> </acpi> </os> will result in: -acpitable sig=SLIC,file=/path/to/acpi/table/file This option was introduced by QEMU commit 8a92ea2 in 2009. https://bugzilla.redhat.com/show_bug.cgi?id=1327537 --- src/qemu/qemu_command.c | 7 +++++++ tests/qemuxml2argvdata/qemuxml2argv-acpi-table.args | 19 +++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 28 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-acpi-table.args diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 102837b..368bd87 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6222,6 +6222,13 @@ qemuBuildBootCommandLine(virCommandPtr cmd, goto error; } } + if (def->os.slic_table) { + virBuffer buf = VIR_BUFFER_INITIALIZER; + virCommandAddArg(cmd, "-acpitable"); + virBufferAddLit(&buf, "sig=SLIC,file="); + qemuBufferEscapeComma(&buf, def->os.slic_table); + virCommandAddArgBuffer(cmd, &buf); + } return 0; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-acpi-table.args b/tests/qemuxml2argvdata/qemuxml2argv-acpi-table.args new file mode 100644 index 0000000..31902ba --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-acpi-table.args @@ -0,0 +1,19 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 214 \ +-smp 1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-boot c \ +-acpitable sig=SLIC,file=/var/lib/libvirt/acpi/slic.dat \ +-usb diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index bce11c3..db0a7f7 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1972,6 +1972,8 @@ mymain(void) DO_TEST("master-key", QEMU_CAPS_OBJECT_SECRET); + DO_TEST("acpi-table", NONE); + qemuTestDriverFree(&driver); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; -- 2.7.3

On Mon, May 23, 2016 at 20:01:17 +0200, Ján Tomko wrote:
<os> <acpi> <table type="slic">/path/to/acpi/table/file</table> </acpi> </os>
will result in:
-acpitable sig=SLIC,file=/path/to/acpi/table/file
This option was introduced by QEMU commit 8a92ea2 in 2009.
https://bugzilla.redhat.com/show_bug.cgi?id=1327537 --- src/qemu/qemu_command.c | 7 +++++++ tests/qemuxml2argvdata/qemuxml2argv-acpi-table.args | 19 +++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 28 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-acpi-table.args
ACK

Add support for the slic_table to the security drivers. --- src/security/security_dac.c | 5 +++++ src/security/security_selinux.c | 5 +++++ src/security/virt-aa-helper.c | 4 ++++ 3 files changed, 14 insertions(+) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index df3ed47..442ce70 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1218,6 +1218,11 @@ virSecurityDACSetAllLabel(virSecurityManagerPtr mgr, def->os.dtb, user, group) < 0) return -1; + if (def->os.slic_table && + virSecurityDACSetOwnership(priv, NULL, + def->os.slic_table, user, group) < 0) + return -1; + return 0; } diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index b33d54a..aa61767 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -2444,6 +2444,11 @@ virSecuritySELinuxSetAllLabel(virSecurityManagerPtr mgr, data->content_context) < 0) return -1; + if (def->os.slic_table && + virSecuritySELinuxSetFilecon(mgr, def->os.slic_table, + data->content_context) < 0) + return -1; + if (stdin_path && virSecuritySELinuxSetFilecon(mgr, stdin_path, data->content_context) < 0) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 537e89d..691bbdf 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -993,6 +993,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) + goto cleanup; + if (ctl->def->os.loader && ctl->def->os.loader->path) if (vah_add_file(&buf, ctl->def->os.loader->path, "r") != 0) goto cleanup; -- 2.7.3

On Mon, May 23, 2016 at 20:01:18 +0200, Ján Tomko wrote:
Add support for the slic_table to the security drivers.
In this case, you should add a note to the documentation that the file is not treated as shared and should be copied for every VM to avoid problems as with shared kernel files.
--- src/security/security_dac.c | 5 +++++ src/security/security_selinux.c | 5 +++++ src/security/virt-aa-helper.c | 4 ++++ 3 files changed, 14 insertions(+)a
diff --git a/src/security/security_dac.c b/src/security/security_dac.c index df3ed47..442ce70 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1218,6 +1218,11 @@ virSecurityDACSetAllLabel(virSecurityManagerPtr mgr, def->os.dtb, user, group) < 0) return -1;
+ if (def->os.slic_table && + virSecurityDACSetOwnership(priv, NULL, + def->os.slic_table, user, group) < 0) + return -1; + return 0; }
All 3 security driver IMPLs are missing addition to virSecurity.*RestoreAllLabel. ACK with that added. Peter

On Wed, May 25, 2016 at 16:53:25 +0200, Peter Krempa wrote:
On Mon, May 23, 2016 at 20:01:18 +0200, Ján Tomko wrote:
Add support for the slic_table to the security drivers.
In this case, you should add a note to the documentation that the file is not treated as shared and should be copied for every VM to avoid problems as with shared kernel files.
--- src/security/security_dac.c | 5 +++++ src/security/security_selinux.c | 5 +++++ src/security/virt-aa-helper.c | 4 ++++ 3 files changed, 14 insertions(+)a
diff --git a/src/security/security_dac.c b/src/security/security_dac.c index df3ed47..442ce70 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1218,6 +1218,11 @@ virSecurityDACSetAllLabel(virSecurityManagerPtr mgr, def->os.dtb, user, group) < 0) return -1;
+ if (def->os.slic_table && + virSecurityDACSetOwnership(priv, NULL, + def->os.slic_table, user, group) < 0) + return -1; + return 0; }
All 3 security driver IMPLs are missing addition to virSecurity.*RestoreAllLabel.
I've noticed that they are actually considered shared, thus ACK without any change.

On Wed, May 25, 2016 at 05:06:27PM +0200, Peter Krempa wrote:
On Wed, May 25, 2016 at 16:53:25 +0200, Peter Krempa wrote:
On Mon, May 23, 2016 at 20:01:18 +0200, Ján Tomko wrote:
Add support for the slic_table to the security drivers.
In this case, you should add a note to the documentation that the file is not treated as shared and should be copied for every VM to avoid problems as with shared kernel files.
--- src/security/security_dac.c | 5 +++++ src/security/security_selinux.c | 5 +++++ src/security/virt-aa-helper.c | 4 ++++ 3 files changed, 14 insertions(+)a
diff --git a/src/security/security_dac.c b/src/security/security_dac.c index df3ed47..442ce70 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1218,6 +1218,11 @@ virSecurityDACSetAllLabel(virSecurityManagerPtr mgr, def->os.dtb, user, group) < 0) return -1;
+ if (def->os.slic_table && + virSecurityDACSetOwnership(priv, NULL, + def->os.slic_table, user, group) < 0) + return -1; + return 0; }
All 3 security driver IMPLs are missing addition to virSecurity.*RestoreAllLabel.
I've noticed that they are actually considered shared, thus ACK without any change.
Thanks, I have pushed the series now. Jan
participants (2)
-
Ján Tomko
-
Peter Krempa