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@xxxxxxxxxx> > --- > 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.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
