[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 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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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