4:01 p.m.
On 09/21/2011 10:25 AM, Laine Stump wrote:
This patch was made in response to:
https://bugzilla.redhat.com/show_bug.cgi?id=738095
In short, qemu's default for the rombar setting (which makes the
firmware ROM of a PCI device visible/not on the guest) was previously
0 (not visible), but they recently changed the default to 1
(visible). Unfortunately, there are some PCI devices that fail in the
guest when rombar is 1, so the setting must be exposed in libvirt to
prevent a regression in behavior (it will still require explicitly
setting<rom bar='off'/> in the guest XML).
rombar is forced on/off by adding:
<rom bar='on|off'/>
inside a<hostdev> element that defines a PCI device. It is currently
ignored for all other types of devices.
I'm leaning towards deferring this until after a quick 0.9.6 release
that works around the qemu shutdown issue. That said, I'll still review
the patch.
At the moment there is no clean method to determine whether or not the
rombar option is supported by QEMU - this patch uses the advice of a
QEMU developer to assume support for qemu-0.12+. There is currently a
patch in the works to put this information in the output of "qemu-kvm
-device pci-assign,?", but of course if we switch to keying off that,
we would lose support for setting rombar on all the versions of qemu
between 0.12 and whatever version gets that patch.
Hopefully the assumption is close enough to correct, and qemu will give
us error messages if our request is inappropriate.
+<dt><code>rom</code></dt>
+<dd>The<code>rom</code> element is used to change how a PCI
+ device's ROM is presented to the guest. The<code>bar</code>
+ attribute can be set to "on" or "off", and determines
whether
+ or not the devices ROM will be visible in the guest's memory
s/devices/device's/
+ map. (in PCI documentation, this is described as being
s/in/In/
+ controlled by the "rombar" setting). If no rom bar
is
Maybe mention what the BAR acronym expands to (am I correct that it
means Base Address Register?), as in:
(In PCI documentation, the "rombar" setting controls the presence of the
Base Address Register for the ROM.)
+ specified, the qemu default will be used (older versions of
+ qemu used a default of "off", while newer qemus have a default
+ of "on").<span class="since">Since 0.9.6</span>
+</dd>
<dt><code>address</code></dt>
<dd>The<code>address</code> element for USB devices has a
<code>bus</code> and<code>device</code> attribute to
specify the
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index d0da41c..0ba35ef 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2059,6 +2059,16 @@
<optional>
<ref name="address"/>
</optional>
+<optional>
+<element name="rom">
+<attribute name="bar">
I think you need another layer of <optional> around the <attribute>;
that is, the user can do
<rom/>
which should be just as valid XML as:
<rom bar="on"/>
even if we never generate it ourselves.
+<choice>
+<value>on</value>
+<value>off</value>
+</choice>
+</attribute>
I think you need <empty/> here as well.
@@ -5485,6 +5491,20 @@ virDomainHostdevDefParseXML(const xmlNodePtr
node,
if (virDomainDeviceBootParseXML(cur,&def->bootIndex,
bootMap))
goto error;
+ } else if (xmlStrEqual(cur->name, BAD_CAST "rom")) {
+ char *rombar = virXMLPropString(cur, "bar");
+ if (!rombar) {
+ virDomainReportError(VIR_ERR_XML_ERROR,
+ "%s", _("missing rom bar
attribute"));
+ goto error;
+ }
Oh, I guess you're making it a hard error if bar=... is not present, in
which case my comment about another layer of <optional> can be disregarded.
@@ -1855,6 +1864,7 @@ VIR_ENUM_DECL(virDomainWatchdogAction)
VIR_ENUM_DECL(virDomainVideo)
VIR_ENUM_DECL(virDomainHostdevMode)
VIR_ENUM_DECL(virDomainHostdevSubsys)
+VIR_ENUM_DECL(virDomainPciRombarMode)
VIR_ENUM_DECL(virDomainHub)
VIR_ENUM_DECL(virDomainRedirdevBus)
VIR_ENUM_DECL(virDomainInput)
Export the two new symbols (virDomainPciRombarModeType{To,From}String)
in src/libvirt_private.syms.
@@ -1049,6 +1050,9 @@ qemuCapsComputeCmdFlags(const char *help,
if (version>= 13000)
qemuCapsSet(flags, QEMU_CAPS_PCI_MULTIFUNCTION);
+
+ if (version>= 12000)
+ qemuCapsSet(flags, QEMU_CAPS_PCI_ROMBAR);
Maybe add a comment why we use a version check rather than probing for
pci-assign.rombar=uint32 in 'qemu -device pci-assign,?'.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org