[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]