[libvirt] [PATCH 0/4] qemu: support for SLIC ACPI tables
Windows uses this BLOB for activation purposes. https://bugzilla.redhat.com/show_bug.cgi?id=1327537 Ján Tomko (4): Separate virDomainDefParseBootOptions conf: add <acpi table> to <os> qemu: format SLIC ACPI table command line security: label the slic_table docs/formatdomain.html.in | 6 + docs/schemas/domaincommon.rng | 10 ++ src/conf/domain_conf.c | 159 +++++++++++++-------- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 4 + 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 | 28 ++++ tests/qemuxml2argvtest.c | 2 + .../qemuxml2xmlout-acpi-table.xml | 32 +++++ tests/qemuxml2xmltest.c | 3 + 13 files changed, 215 insertions(+), 63 deletions(-) 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
Split out parsing of most of the <os> subelements into a separate function. --- src/conf/domain_conf.c | 146 ++++++++++++++++++++++++++++--------------------- 1 file changed, 83 insertions(+), 63 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ed0c471..331ff06 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15095,6 +15095,87 @@ virDomainVcpuParse(virDomainDefPtr def, return ret; } + +static int +virDomainDefParseBootOptions(virDomainDefPtr def, + xmlXPathContextPtr ctxt, + virHashTablePtr *bootHash) +{ + xmlNodePtr *nodes = NULL; + int ret = -1; + size_t i; + int n; + + /* + * Booting options for different OS types.... + * + * - A bootloader (and optional kernel+initrd) (xen) + * - A kernel + initrd (xen) + * - A boot device (and optional kernel+initrd) (hvm) + * - An init script (exe) + */ + + if (def->os.type == VIR_DOMAIN_OSTYPE_EXE) { + def->os.init = virXPathString("string(./os/init[1])", ctxt); + def->os.cmdline = virXPathString("string(./os/cmdline[1])", ctxt); + + if ((n = virXPathNodeSet("./os/initarg", ctxt, &nodes)) < 0) + goto error; + + if (VIR_ALLOC_N(def->os.initargv, n+1) < 0) + goto error; + for (i = 0; i < n; i++) { + if (!nodes[i]->children || + !nodes[i]->children->content) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("No data supplied for <initarg> element")); + goto error; + } + if (VIR_STRDUP(def->os.initargv[i], + (const char*) nodes[i]->children->content) < 0) + goto error; + } + def->os.initargv[n] = NULL; + VIR_FREE(nodes); + } + + if (def->os.type == VIR_DOMAIN_OSTYPE_XEN || + def->os.type == VIR_DOMAIN_OSTYPE_HVM || + def->os.type == VIR_DOMAIN_OSTYPE_UML) { + xmlNodePtr loader_node; + + def->os.kernel = virXPathString("string(./os/kernel[1])", ctxt); + def->os.initrd = virXPathString("string(./os/initrd[1])", ctxt); + def->os.cmdline = virXPathString("string(./os/cmdline[1])", ctxt); + def->os.dtb = virXPathString("string(./os/dtb[1])", ctxt); + def->os.root = virXPathString("string(./os/root[1])", ctxt); + if ((loader_node = virXPathNode("./os/loader[1]", ctxt))) { + if (VIR_ALLOC(def->os.loader) < 0) + goto error; + + if (virDomainLoaderDefParseXML(loader_node, def->os.loader) < 0) + goto error; + + def->os.loader->nvram = virXPathString("string(./os/nvram[1])", ctxt); + def->os.loader->templt = virXPathString("string(./os/nvram[1]/@template)", ctxt); + } + } + + if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { + if (virDomainDefParseBootXML(ctxt, def) < 0) + goto error; + if (!(*bootHash = virHashCreate(5, NULL))) + goto error; + } + + ret = 0; + + error: + VIR_FREE(nodes); + return ret; +} + + static virDomainDefPtr virDomainDefParseXML(xmlDocPtr xml, xmlNodePtr root, @@ -16070,69 +16151,8 @@ virDomainDefParseXML(xmlDocPtr xml, } VIR_FREE(nodes); - - /* - * Booting options for different OS types.... - * - * - A bootloader (and optional kernel+initrd) (xen) - * - A kernel + initrd (xen) - * - A boot device (and optional kernel+initrd) (hvm) - * - An init script (exe) - */ - - if (def->os.type == VIR_DOMAIN_OSTYPE_EXE) { - def->os.init = virXPathString("string(./os/init[1])", ctxt); - def->os.cmdline = virXPathString("string(./os/cmdline[1])", ctxt); - - if ((n = virXPathNodeSet("./os/initarg", ctxt, &nodes)) < 0) - goto error; - - if (VIR_ALLOC_N(def->os.initargv, n+1) < 0) - goto error; - for (i = 0; i < n; i++) { - if (!nodes[i]->children || - !nodes[i]->children->content) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("No data supplied for <initarg> element")); - goto error; - } - if (VIR_STRDUP(def->os.initargv[i], - (const char*) nodes[i]->children->content) < 0) - goto error; - } - def->os.initargv[n] = NULL; - VIR_FREE(nodes); - } - - if (def->os.type == VIR_DOMAIN_OSTYPE_XEN || - def->os.type == VIR_DOMAIN_OSTYPE_HVM || - def->os.type == VIR_DOMAIN_OSTYPE_UML) { - xmlNodePtr loader_node; - - def->os.kernel = virXPathString("string(./os/kernel[1])", ctxt); - def->os.initrd = virXPathString("string(./os/initrd[1])", ctxt); - def->os.cmdline = virXPathString("string(./os/cmdline[1])", ctxt); - def->os.dtb = virXPathString("string(./os/dtb[1])", ctxt); - def->os.root = virXPathString("string(./os/root[1])", ctxt); - if ((loader_node = virXPathNode("./os/loader[1]", ctxt))) { - if (VIR_ALLOC(def->os.loader) < 0) - goto error; - - if (virDomainLoaderDefParseXML(loader_node, def->os.loader) < 0) - goto error; - - def->os.loader->nvram = virXPathString("string(./os/nvram[1])", ctxt); - def->os.loader->templt = virXPathString("string(./os/nvram[1]/@template)", ctxt); - } - } - - if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { - if (virDomainDefParseBootXML(ctxt, def) < 0) - goto error; - if (!(bootHash = virHashCreate(5, NULL))) - goto error; - } - + if (virDomainDefParseBootOptions(def, ctxt, &bootHash) < 0) + goto error; /* analysis of the disk devices */ if ((n = virXPathNodeSet("./devices/disk", ctxt, &nodes)) < 0) -- 2.7.3
On 05/13/2016 10:52 AM, Ján Tomko wrote:
Split out parsing of most of the <os> subelements into a separate function. --- src/conf/domain_conf.c | 146 ++++++++++++++++++++++++++++--------------------- 1 file changed, 83 insertions(+), 63 deletions(-)
ACK John
Add a new element to <domain> XML: <os> <acpi table="slic">/path/to/acpi/table/file</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 | 6 ++++ docs/schemas/domaincommon.rng | 10 +++++++ src/conf/domain_conf.c | 13 +++++++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-acpi-table.xml | 28 +++++++++++++++++++ .../qemuxml2xmlout-acpi-table.xml | 32 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 3 ++ 7 files changed, 93 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 58b8cb6..a294978 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -277,6 +277,7 @@ <initrd>/root/f8-i386-initrd</initrd> <cmdline>console=ttyS0 ks=http://example.com/f8-i386/os/</cmdline>; <dtb>/root/ppc.dtb</dtb> + <acpi table="slic">/path/to/slic.dat</> </os> ...</pre> @@ -302,6 +303,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 contents of the <code>table</code> attribute specifies the + ACPI table type (Currently only <code>slic</code> is supported) + and the element contains a fully-qualified path to the ACPI table. + <span class="since">Since 1.3.5</span></dd> </dl> <h4><a name="elementsOSContainer">Container boot</a></h4> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 8798001..8c4be11 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -306,6 +306,16 @@ <optional> <ref name="bios"/> </optional> + <optional> + <element name="acpi"> + <attribute name="table"> + <choice> + <value>slic</value> + </choice> + </attribute> + <ref name="absFilePath"/> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 331ff06..6f86e46 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); @@ -15102,6 +15103,7 @@ virDomainDefParseBootOptions(virDomainDefPtr def, virHashTablePtr *bootHash) { xmlNodePtr *nodes = NULL; + char *tmp = NULL; int ret = -1; size_t i; int n; @@ -15162,6 +15164,14 @@ virDomainDefParseBootOptions(virDomainDefPtr def, } if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { + tmp = virXPathString("string(./os/acpi[1]/@table)", ctxt); + if (STREQ_NULLABLE(tmp, "slic")) { + VIR_FREE(tmp); + tmp = virXPathString("string(./os/acpi[1])", ctxt); + def->os.slic_table = virFileSanitizePath(tmp); + VIR_FREE(tmp); + } + if (virDomainDefParseBootXML(ctxt, def) < 0) goto error; if (!(*bootHash = virHashCreate(5, NULL))) @@ -15172,6 +15182,7 @@ virDomainDefParseBootOptions(virDomainDefPtr def, error: VIR_FREE(nodes); + VIR_FREE(tmp); return ret; } @@ -22494,6 +22505,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->os.dtb); virBufferEscapeString(buf, "<root>%s</root>\n", def->os.root); + virBufferEscapeString(buf, "<acpi table='slic'>%s</acpi>\n", + def->os.slic_table); 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 b9e696d..0b3f1a2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1875,6 +1875,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..f807791 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-acpi-table.xml @@ -0,0 +1,28 @@ +<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='slic'>/var/lib/libvirt/acpi/slic.dat</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..54a4d90 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-acpi-table.xml @@ -0,0 +1,32 @@ +<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='slic'>/var/lib/libvirt/acpi/slic.dat</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 5a43fa9..0c9fd7e 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -794,6 +794,9 @@ mymain(void) cfg->vncAutoUnixSocket = false; virObjectUnref(cfg); + + DO_TEST("acpi-table"); + qemuTestDriverFree(&driver); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; -- 2.7.3
On 05/13/2016 10:52 AM, Ján Tomko wrote:
Add a new element to <domain> XML: <os> <acpi table="slic">/path/to/acpi/table/file</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 | 6 ++++ docs/schemas/domaincommon.rng | 10 +++++++ src/conf/domain_conf.c | 13 +++++++++ src/conf/domain_conf.h | 1 + tests/qemuxml2argvdata/qemuxml2argv-acpi-table.xml | 28 +++++++++++++++++++ .../qemuxml2xmlout-acpi-table.xml | 32 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 3 ++ 7 files changed, 93 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-acpi-table.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-acpi-table.xml
Yet another example of a path that "could" need comma escaping: http://wiki.libvirt.org/page/BiteSizedTasks#qemu:_Use_comma_escaping_for_mor...
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 58b8cb6..a294978 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -277,6 +277,7 @@ <initrd>/root/f8-i386-initrd</initrd> <cmdline>console=ttyS0 ks=http://example.com/f8-i386/os/</cmdline>; <dtb>/root/ppc.dtb</dtb> + <acpi table="slic">/path/to/slic.dat</>
Since you format as table='slic', this should use same format e.g. s/"slic"/'slic'/
</os> ...</pre>
@@ -302,6 +303,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 contents of the <code>table</code> attribute specifies the + ACPI table type (Currently only <code>slic</code> is supported)
s/Currently/currently/
+ and the element contains a fully-qualified path to the ACPI table. + <span class="since">Since 1.3.5</span></dd>
[1] Since this is only supported for 'hvm', that should be noted as well
</dl>
<h4><a name="elementsOSContainer">Container boot</a></h4> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 8798001..8c4be11 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -306,6 +306,16 @@ <optional> <ref name="bios"/> </optional> + <optional> + <element name="acpi"> + <attribute name="table"> + <choice> + <value>slic</value> + </choice> + </attribute> + <ref name="absFilePath"/> + </element> + </optional>
Could be it's own define like 'bios'... 'smbios'... Not a requirement though.
</interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 331ff06..6f86e46 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); @@ -15102,6 +15103,7 @@ virDomainDefParseBootOptions(virDomainDefPtr def, virHashTablePtr *bootHash) { xmlNodePtr *nodes = NULL; + char *tmp = NULL; int ret = -1; size_t i; int n; @@ -15162,6 +15164,14 @@ virDomainDefParseBootOptions(virDomainDefPtr def, }
if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) {
[1] ^^^
+ tmp = virXPathString("string(./os/acpi[1]/@table)", ctxt); + if (STREQ_NULLABLE(tmp, "slic")) { + VIR_FREE(tmp); + tmp = virXPathString("string(./os/acpi[1])", ctxt); + def->os.slic_table = virFileSanitizePath(tmp); + VIR_FREE(tmp); + } + if (virDomainDefParseBootXML(ctxt, def) < 0) goto error; if (!(*bootHash = virHashCreate(5, NULL))) @@ -15172,6 +15182,7 @@ virDomainDefParseBootOptions(virDomainDefPtr def,
error: VIR_FREE(nodes); + VIR_FREE(tmp); return ret; }
@@ -22494,6 +22505,8 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->os.dtb); virBufferEscapeString(buf, "<root>%s</root>\n", def->os.root); + virBufferEscapeString(buf, "<acpi table='slic'>%s</acpi>\n", + def->os.slic_table);
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 b9e696d..0b3f1a2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1875,6 +1875,7 @@ struct _virDomainOSDef { char *cmdline; char *dtb; char *root; + char *slic_table;
This is a path right? Why not just slic_path to make it obvious or could just be slic. IDC whichever way it goes. Just wouldn't want someone to expect to find a parsed table in there only to realize it's a path... Your call on this one. ACK with doc adjustments... I also trust that you can make the code adjustments properly... John
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..f807791 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-acpi-table.xml @@ -0,0 +1,28 @@ +<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='slic'>/var/lib/libvirt/acpi/slic.dat</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..54a4d90 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-acpi-table.xml @@ -0,0 +1,32 @@ +<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='slic'>/var/lib/libvirt/acpi/slic.dat</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 5a43fa9..0c9fd7e 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -794,6 +794,9 @@ mymain(void) cfg->vncAutoUnixSocket = false;
virObjectUnref(cfg); + + DO_TEST("acpi-table"); + qemuTestDriverFree(&driver);
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
On Fri, May 13, 2016 at 16:52:14 +0200, Ján Tomko wrote:
Add a new element to <domain> XML: <os> <acpi table="slic">/path/to/acpi/table/file</acpi>
It looks like the table could compromise of multiple sub-tables which would add a rather unpleasantly looking XML: <acpi table="slic">/path/to/acpi/table/file</acpi> <acpi table="blurb">/path/to/acpi/table/blurb</acpi> <acpi table="foo"> ... Rather than this it would be better if 'table' were a subelement of <acpi> with a attribute 'type' or something along that to overcome that. Peter
<os> <acpi table="slic">/path/to/acpi/table/file</acpi> </os> will result in: -acpitable sig=SLIC,file=/path/to/acpi/table/file https://bugzilla.redhat.com/show_bug.cgi?id=1327537 --- src/qemu/qemu_command.c | 4 ++++ tests/qemuxml2argvdata/qemuxml2argv-acpi-table.args | 19 +++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 25 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 0d6d5f8..574f37c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6289,6 +6289,10 @@ qemuBuildBootCommandLine(virCommandPtr cmd, goto error; } } + if (def->os.slic_table) { + virCommandAddArg(cmd, "-acpitable"); + virCommandAddArgFormat(cmd, "sig=SLIC,file=%s", def->os.slic_table); + } 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 d1cfbec..a7e3e8e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1906,6 +1906,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 05/13/2016 10:52 AM, Ján Tomko wrote:
<os> <acpi table="slic">/path/to/acpi/table/file</acpi> </os>
will result in:
-acpitable sig=SLIC,file=/path/to/acpi/table/file
https://bugzilla.redhat.com/show_bug.cgi?id=1327537 --- src/qemu/qemu_command.c | 4 ++++ tests/qemuxml2argvdata/qemuxml2argv-acpi-table.args | 19 +++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 25 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-acpi-table.args
Is there a need for a capability check? When was this added to QEMU? I see there is one added for "-dtb" (commit id '0b3509e24') ACK for what's here, but can we get the answer for whether there's a need for a caps bit check before push. John
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 0d6d5f8..574f37c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6289,6 +6289,10 @@ qemuBuildBootCommandLine(virCommandPtr cmd, goto error; } } + if (def->os.slic_table) { + virCommandAddArg(cmd, "-acpitable"); + virCommandAddArgFormat(cmd, "sig=SLIC,file=%s", def->os.slic_table); + }
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 d1cfbec..a7e3e8e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1906,6 +1906,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;
On 05/17/2016 11:24 AM, John Ferlan wrote:
On 05/13/2016 10:52 AM, Ján Tomko wrote:
<os> <acpi table="slic">/path/to/acpi/table/file</acpi> </os>
will result in:
-acpitable sig=SLIC,file=/path/to/acpi/table/file
https://bugzilla.redhat.com/show_bug.cgi?id=1327537 --- src/qemu/qemu_command.c | 4 ++++ tests/qemuxml2argvdata/qemuxml2argv-acpi-table.args | 19 +++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 25 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-acpi-table.args
Is there a need for a capability check? When was this added to QEMU?
I see there is one added for "-dtb" (commit id '0b3509e24')
ACK for what's here, but can we get the answer for whether there's a need for a caps bit check before push.
IMO just for giving a mildly nicer error on the command line for an uncommon config it's not worth adding a feature check (i've made this mistake too). One day it may be worth adding feature checks for basically everything, but only to expose the feature list to apps through domcapabilities - Cole
Add support for the slic_able 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 7eeb4ef..e5f25bc 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1051,6 +1051,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 05/13/2016 10:52 AM, Ján Tomko wrote:
Add support for the slic_able to the security drivers. ^ Extra space
--- src/security/security_dac.c | 5 +++++ src/security/security_selinux.c | 5 +++++ src/security/virt-aa-helper.c | 4 ++++ 3 files changed, 14 insertions(+)
Should this go before we add the qemu command line? Although I see this is done in the same order as the dtb addition, so no difference to that I guess. ACK John
participants (4)
-
Cole Robinson -
John Ferlan -
Ján Tomko -
Peter Krempa