On Fri, Apr 20, 2018 at 17:44:30 +0200, Andrea Bolognani wrote:
The attribute can be used to disable ROM loading completely
for a device.
You could mention that this is necessary because even if you turn the
ROM BAR off, some firmware might still load it.
Resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=1425058
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_command.c | 13 ++++++++++--
tests/qemuxml2argvdata/pci-rom-disabled.args | 26 ++++++++++++++++++++++++
tests/qemuxml2argvdata/pci-rom-disabled.xml | 20 ++++++++++++++++++
tests/qemuxml2argvtest.c | 1 +
tests/qemuxml2xmloutdata/pci-rom-disabled.xml | 29 +++++++++++++++++++++++++++
tests/qemuxml2xmltest.c | 1 +
6 files changed, 88 insertions(+), 2 deletions(-)
create mode 100644 tests/qemuxml2argvdata/pci-rom-disabled.args
create mode 100644 tests/qemuxml2argvdata/pci-rom-disabled.xml
create mode 100644 tests/qemuxml2xmloutdata/pci-rom-disabled.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b666f3715f..84c4e1e350 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -442,13 +442,20 @@ static int
qemuBuildRomStr(virBufferPtr buf,
virDomainDeviceInfoPtr info)
{
- if (info->rombar || info->romfile) {
+ if (info->romenabled || info->rombar || info->romfile) {
if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- "%s", _("rombar and romfile are supported only
for PCI devices"));
+ "%s", _("ROM tuning is only supported for PCI
devices"));
return -1;
}
+ /* Passing an empty romfile= tells QEMU to disable ROM entirely for
+ * this device, and makes other settings irrelevant */
+ if (info->romenabled == VIR_TRISTATE_BOOL_NO) {
+ virBufferAddLit(buf, ",romfile=");
+ goto done;
You can return early rather than having to jump. This function needs to
be refactored anyways, so no need to be nice to others comming after
you.
+ }
+
switch (info->rombar) {
case VIR_TRISTATE_SWITCH_OFF:
virBufferAddLit(buf, ",rombar=0");
@@ -464,6 +471,8 @@ qemuBuildRomStr(virBufferPtr buf,
virQEMUBuildBufferEscapeComma(buf, info->romfile);
}
}
+
+ done:
return 0;
}
[...]
diff --git a/tests/qemuxml2argvdata/pci-rom-disabled.xml
b/tests/qemuxml2argvdata/pci-rom-disabled.xml
new file mode 100644
index 0000000000..1c12052382
--- /dev/null
+++ b/tests/qemuxml2argvdata/pci-rom-disabled.xml
@@ -0,0 +1,20 @@
+<domain type='qemu'>
+ <name>guest</name>
+ <uuid>c7a5fdbd-edaf-9466-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219100</memory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='x86_64' machine='pc'>hvm</type>
+ </os>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ <controller type='pci' model='pci-root'/>
+ <controller type='usb' model='none'/>
+ <interface type='user'>
+ <mac address='52:54:00:24:a5:9f'/>
+ <model type='virtio'/>
+ <rom enabled='no'/>
If we are going to explicitly keep around the quirk with the empty file
I think we should add a test case for it. It will not be fun though
since such XML is invalid.
+ </interface>
+ <memballoon model='none'/>
+ </devices>
+</domain>
ACK if you get rid of that jump.
The test case will need to be added separately anyways.