[libvirt] [PATCH 5/6] qemu: (and conf) support rombar for network devices

[Laine Stump]

On 01/25/2012 10:18 PM, Eric Blake wrote:
> On 01/25/2012 08:08 PM, Eric Blake wrote:
>> On 01/25/2012 09:58 AM, Laine Stump wrote:
>>> When support for the rombar option was added, it was only added for
>>> PCI passthrough devices, configured with<hostdev>. The same option is
>>> available for any network device that is attached to the guest's PCI
>>> bus. This patch allows setting rombar for any PCI network device type.
>>>
>>> After adding cases to test this to qemuxml2argv-hostdev-pci-rombar.*,
>>> I decided to rename those files (to qemuxml2argv-pci-rom.*) to more
>>> accurately reflect the additional tests, and also noticed that up to
>>> now we've only been performing a domainschematest for that case, so I
>>> added the "pci-rom" test to both qemuxml2argv and qemuxml2xml (and in
>>> the process found some bugs whose fixes I squashed into previous
>>> commits of this series).
>>> ---
>>>   docs/formatdomain.html.in                          |   27 ++++++++++
>>>   docs/schemas/domaincommon.rng                      |   25 ++++++---
>>>   src/conf/domain_conf.c                             |    6 ++-
>>>   src/qemu/qemu_command.c                            |   54 +++++++++++++-------
>>>   .../qemuxml2argv-hostdev-pci-rombar.args           |    5 --
>>>   tests/qemuxml2argvdata/qemuxml2argv-pci-rom.args   |   12 ++++
>>>   ...dev-pci-rombar.xml =>  qemuxml2argv-pci-rom.xml} |   18 +++++++
>>>   tests/qemuxml2argvtest.c                           |    3 +
>>>   tests/qemuxml2xmltest.c                            |    1 +
>> docs, rng, and tests all in one go!  I'm impressed :)
>>
>> I didn't see anything fishy in here, so ACK.
> Actually,
>
>> +<define name="rom">
>> +<element name="rom">
>> +<attribute name="bar">
>> +<choice>
>> +<value>on</value>
>> +<value>off</value>
>> +</choice>
>> +</attribute>
>> +<empty/>
>> +</element>
>> +</define>
>> +
>> +-device pci-assign,host=06:12.6,id=hostdev1,bus=pci.0,addr=0x6,rombar=1,\
>> +romfile=/etc/fake/bootrom.bin \
>> +<interface type='user'>
>> +<mac address='52:54:00:24:a5:9e'/>
>> +<model type='virtio'/>
>> +<rom file='/etc/fake/bootrom.bin'/>
>> +</interface>
> You don't add rom file='' until patch 6/6, and the RNG schema doesn't
> look like it supports file yet.
>
> </me installs the series to see if this passes 'make check'...>
>
> 153) qemuxml2argvdata/qemuxml2argv-pci-rom.xml                    ... FAILED
> xmllint --relaxng
> /home/remote/eblake/libvirt/tests/../docs/schemas/domain.rng --noout
> /home/remote/eblake/libvirt/tests/qemuxml2argvdata/qemuxml2argv-pci-rom.xml
> Relax-NG validity error : Extra element devices in interleave
> /home/remote/eblake/libvirt/tests/qemuxml2argvdata/qemuxml2argv-pci-rom.xml:15:
> element devices: Relax-NG validity error : Element domain failed to
> validate content
> /home/remote/eblake/libvirt/tests/qemuxml2argvdata/qemuxml2argv-pci-rom.xml
> fails to validate
>
> It looks like you didn't quite rebase this for 'git bisect' purposes.
> Would you mind rebasing one more time to move the<rom file=>  stuff into
> 6/6 where the rng support was added?  Since it is just motion between
> the two patches, and the end result is the same, no need to post a v2.
>

Thanks for catching that! I forgot to mention it in the "pushed" message 
for 5/6, but I did make that change and verify make check passes at each 
stage of the patch series before pushing 5/6.

I've push 6/6 now too.