11:35 a.m.
On 09/22/2011 10:05 AM, Laine Stump wrote:
(This addresses Eric's comments in his review of V1. BTW, I'm
a bit
surprised nobody has commented about the choice of name/placement of
the new attribute - speak now or forever hold your peace :-))
I was okay with the xml naming - but I agree that if anyone else has a
better suggestion, now is the time to speak up :)
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.
+++ b/docs/formatdomain.html.in
@@ -1371,6 +1371,7 @@
<address bus='0x06' slot='0x02'
function='0x0'/>
</source>
<boot order='1'/>
+<rom bar='0'/>
s/0/off/
</hostdev>
</devices>
...</pre>
@@ -1402,6 +1403,18 @@
used together with general boot elements in
<a href="#elementsOSBIOS">BIOS bootloader</a> section.
<span class="since">Since 0.8.8</span></dd>
+<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 device's ROM will be visible in the guest's memory
+ map. (In PCI documentation, the "rombar" setting controls the
+ presence of the Base Address Register for the ROM). If no rom
+ bar is 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>
looks much nicer, now that my v1 comments are incorporated, but still
one nit:
s/0.9.6/0.9.7/
@@ -10387,6 +10407,18 @@ virDomainHostdevDefFormat(virBufferPtr buf,
if (virDomainDeviceInfoFormat(buf,&def->info, flags)< 0)
return -1;
+ if (def->rombar) {
+ const char *rombar
+ = virDomainPciRombarModeTypeToString(def->rombar);
+ if (!rombar) {
+ virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unexpected rom bar value %d"),
+ def->rombar);
+ return -1;
+ }
+ virBufferAsprintf(buf, "<rom bar='%s'/>\n", rombar);
Aargh - Thunderbird strikes me again. When will they EVER fix their
horrible whitespace munging bug?
This will conflict with my <snapshotdomain> reindentation patch series -
so whoever applies first gets the easier side of the bargain :)
+ }
+
virBufferAddLit(buf, "</hostdev>\n");
return 0;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 371f270..262c970 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -936,6 +936,14 @@ enum virDomainHostdevSubsysType {
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST
};
+enum virDomainPciRombarMode {
+ VIR_DOMAIN_PCI_ROMBAR_DEFAULT = 0,
+ VIR_DOMAIN_PCI_ROMBAR_ON,
+ VIR_DOMAIN_PCI_ROMBAR_OFF,
+
+ VIR_DOMAIN_PCI_ROMBAR_LAST
+};
+
typedef struct _virDomainHostdevDef virDomainHostdevDef;
typedef virDomainHostdevDef *virDomainHostdevDefPtr;
struct _virDomainHostdevDef {
@@ -964,6 +972,7 @@ struct _virDomainHostdevDef {
} source;
int bootIndex;
virDomainDeviceInfo info; /* Guest address */
+ int rombar; /* rombar on/off/unspecified */
Your comment could go out of date if the enum grows. Lately, I've been
listing fields like this as:
int rombar; /* enum virDomainPciRombarMode */
which stays correct even if we later add new values to the enum.
+++ b/src/qemu/qemu_capabilities.h
@@ -110,6 +110,7 @@ enum qemuCapsFlags {
QEMU_CAPS_PCI_OHCI = 71, /* -device pci-ohci */
QEMU_CAPS_USB_REDIR = 72, /* -device usb-redir */
QEMU_CAPS_USB_HUB = 73, /* -device usb-hub */
+ QEMU_CAPS_PCI_ROMBAR = 74, /* -device rombar=0|1 */
Needs an obvious rebase for NO_SHUTDOWN and DRIVE_CACHE_UNSAFE.
ACK to the idea. I think my findings are small enough that I'm okay if
you push with nits fixed rather than posting a v3 patch, although you
may still want to wait for any other comments on the proposed xml spelling.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org