Gerd,

libvirt of course can't include this option on the commandline if the version of qemu being run doesn't support it. But the help output doesn't list whether or not it's there.


Did rombar show up in the same release as -device? Or was it later? Can you tell me the particular release? Maybe there was another feature that *is* reported in the help output that came up at the same time?

IIRC this is qemu 0.12 (same version which switches from bochs bios to seabios).

One concern I have is that with "<rom bar='on|off'/>", we're adding another element that can't really be re-used when we discover other PCI driver items that need to be configurable. For other drivers we put these in "<driver>" (eg event_idx), but in this case that doesn't really seem appropriate (correct me if I'm wrong) because this is a user-visible item that is at least semi-standard, while the things in <driver> have been implementation details of the driver.

Another question - what should the default be? If no rombar is specified, do we leave it out (which could lead to regressions like the current one if no extra action is taken)? Or do we always add the option, setting it to the "old" default of 0? I guess the question is really "how much regression is acceptable?"

(In reply to comment #4)
> One concern I have is that with "<rom bar='on|off'/>", we're adding another
> element that can't really be re-used when we discover other PCI driver items
> that need to be configurable. For other drivers we put these in "<driver>" (eg
> event_idx), but in this case that doesn't really seem appropriate (correct me
> if I'm wrong) because this is a user-visible item that is at least
> semi-standard, while the things in <driver> have been implementation details of
> the driver.
> 
> Another question - what should the default be? If no rombar is specified, do we
> leave it out (which could lead to regressions like the current one if no extra
> action is taken)? Or do we always add the option, setting it to the "old"
> default of 0? I guess the question is really "how much regression is
> acceptable?"

First, you have 'needinfo' set but didn't identify which person you wanted the info from.... so here's my shot at it.

rombar is device-specific, in this case, it's a PCI bus property.

So, it shouldn't be associated with 'driver' (as in pci-assign, usb-host, etc.),
but with pci-assign only.

I also recommend not adding it at all to the command line if not requested to be set; let qemu defaults drive it.

(In reply to comment #5)

> First, you have 'needinfo' set but didn't identify which person you wanted the
> info from.... so here's my shot at it.

Right, because I thought it was something that more than one person might want to comment on, particularly the default setting, which is sometimes a contentious issue.

I've posted a patch upstream which implements this as discussed above (except that the xml is "<rom bar='0'/> (note the extra space):

https://www.redhat.com/archives/libvir-list/2011-September/msg00855.html

Currently the patch assumes presence of rombar if the qemu version is >= 0.12. If the patch for Bug 678731 were pushed, we could instead look in the output of "qemu-kvm -device pci-assign,?". I'm not sure if that would be a net gain or net loss.

Once the above patch is accepted/pushed upstream I will post it for inclusion in a RHEL build.

patch for 678731 was posted & included upstream qeme.
Gerd posted it to rhvirt-patches list, and last time I checked, 
Markus & I ack'd it;
so we need another ack to get into 6.2-qemu-kvm; ok sent a request;
alex williamson will do that.

So, it's in upstream qemu; don't know how/when avi pulls it into qemu-kvm upstream; in queue for rhel6.2.

is that good enough for libvirt to change it based on -device pci-assign,? probe?

That would be good for a controlled environment like RHEL, but we decided that it would be more accurate for the upstream patch to use the qemu version, and that it's best for the RHEL patch to match the upstream patch. So in the end it just checks version anyway.

The upstream patch has been pushed, and was rebased and sent to rhvirt-patches for inclusion in RHEL:

http://post-office.corp.redhat.com/archives/rhvirt-patches/2011-September/msg00882.html

commit dc79852af88a6ee8b03f8fd7e48e039bae056ed3
Author: Laine Stump <laine>
Date:   Tue Sep 20 13:31:52 2011 -0400

    qemu: add ability to set PCI device "rombar" on or off
    
    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.

This issue has been resolved on rhel6.2 Beta(2.6.32-202.el6.x86_64) with libvirt-0.9.4-14.el6.x86_64 and qemu-kvm-0.12.1.2-2.183.el6.x86_64, so move the bug to VERIFIED status, the following is test report for rom bar='on|off':

# virsh dumpxml vr-rhel5u4-x86_64-kvm
......
    <hostdev mode='subsystem' type='pci' managed='yes'>
      <source>
        <address domain='0x0000' bus='0x00' slot='0x19' function='0x0'/>
      </source>
      <alias name='hostdev0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
      <rom bar='off'/>
    </hostdev>
......

# ps -ef|grep qemu-kvm|grep -v grep
qemu      3950     1 58 16:25 ?        00:00:09 /usr/libexec/qemu-kvm -S -M rhel6.2.0 -enable-kvm -m 1024 -smp 1,sockets=1,cores=1,threads=1 -name vr-rhel5u4-x86_64-kvm -uuid 894d5b0f-42c3-c13d-ee81-f55a5313cecc -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/vr-rhel5u4-x86_64-kvm.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -drive file=/var/lib/libvirt/images/vr-rhel5u4-x86_64-kvm,if=none,id=drive-ide0-0-0,format=qcow2 -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 -netdev tap,fd=23,id=hostnet0 -device rtl8139,netdev=hostnet0,id=net0,mac=52:54:00:7b:91:a4,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -usb -vnc 127.0.0.1:0 -k en-us -vga cirrus -device AC97,id=sound0,bus=pci.0,addr=0x4 -device pci-assign,host=00:19.0,id=hostdev0,configfd=24,bus=pci.0,addr=0x6,rombar=0 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5

# virsh dumpxml vr-rhel5u4-x86_64-kvm
......
    <hostdev mode='subsystem' type='pci' managed='yes'>
      <source>
        <address domain='0x0000' bus='0x00' slot='0x19' function='0x0'/>
      </source>
      <alias name='hostdev0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
      <rom bar='on'/>
    </hostdev>
......


# ps -ef|grep qemu-kvm|grep -v grep
qemu      4258     1 21 16:32 ?        00:00:01 /usr/libexec/qemu-kvm -S -M rhel6.2.0 -enable-kvm -m 1024 -smp 1,sockets=1,cores=1,threads=1 -name vr-rhel5u4-x86_64-kvm -uuid 894d5b0f-42c3-c13d-ee81-f55a5313cecc -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/vr-rhel5u4-x86_64-kvm.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown -drive file=/var/lib/libvirt/images/vr-rhel5u4-x86_64-kvm,if=none,id=drive-ide0-0-0,format=qcow2 -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 -netdev tap,fd=23,id=hostnet0 -device rtl8139,netdev=hostnet0,id=net0,mac=52:54:00:7b:91:a4,bus=pci.0,addr=0x3 -chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 -usb -vnc 127.0.0.1:0 -k en-us -vga cirrus -device AC97,id=sound0,bus=pci.0,addr=0x4 -device pci-assign,host=00:19.0,id=hostdev0,configfd=24,bus=pci.0,addr=0x6,rombar=1 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2011-1513.html