[libvirt] [PATCH 13/17] conf: parse/format type='hostdev' network interfaces

[Laine Stump]

On 03/05/2012 03:45 PM, Michal Privoznik wrote:
> On 28.02.2012 21:14, Laine Stump wrote:
>> This is the new interface type that sets up a PCI/USB network device
>> to be assigned to the guest with PCI/USB passthrough after
>> initializing some network device-specific things from the config
>> (e.g. MAC address, virtualport profile parameters). Here is an example
>> of the syntax:
>>
>>   <interface type='hostdev' managed='yes'>
>>     <source>
>>       <address type='pci' domain='0' bus='0' slot='4' function='0'/>
>>     </source>
>>     <mac address='00:11:22:33:44:55'/>
>>     <address type='pci' domain='0' bus='0' slot='7' function='0'/>
>>   </interface>
>>
>> This would assign the PCI card from bus 0 slot 4 function 0 on the
>> host, to bus 0 slot 7 function 0 on the guest, but would first set the
>> MAC address of the card to 00:11:22:33:44:55.
>>
>> Although it's not expected to be used very much, usb network hostdevs
>> are also supported for completeness. <source> syntax is identical to
>> that for plain <hostdev> devices, except that the <address> element
>> should have "type='usb'" added if it's specified:
>>
>>   <interface type='hostdev'>
>>     <source>
>>       <address type='usb' bus='0' device='4'/>
>>     </source>
>>     <mac address='00:11:22:33:44:55'/>
>>   </interface>
>>
>> If the vendor/product form of usb specification is used, type='usb'
>> is implied:
>>
>>   <interface type='hostdev'>
>>     <source>
>>       <vendor id='0x0012'/>
>>       <product id='0x24dd'/>
>>     </source>
>>     <mac address='00:11:22:33:44:55'/>
>>   </interface>
>> ---
>> V2: address Eric's concerns from V1
>>  - check for OOM after strdup
>>  - put in a NOP virDomainHostdevDefClear() rather than just commenting
>>    "there is nothing in the HostdevDef that needs to be freed"
>>  - eliminate inconsistent {} usage.
>>
>>  docs/formatdomain.html.in                          |   41 +++++
>>  docs/schemas/domaincommon.rng                      |   50 +++++++
>>  src/conf/domain_conf.c                             |  154 ++++++++++++++++++--
>>  src/conf/domain_conf.h                             |   10 ++
>>  src/libvirt_private.syms                           |    1 +
>>  src/qemu/qemu_command.c                            |    1 +
>>  src/uml/uml_conf.c                                 |    5 +
>>  src/xenxs/xen_sxpr.c                               |    1 +
>>  .../qemuxml2argvdata/qemuxml2argv-net-hostdev.xml  |   48 ++++++
>>  tests/qemuxml2xmltest.c                            |    1 +
>>  10 files changed, 297 insertions(+), 15 deletions(-)
>>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml
>>
>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml
>> new file mode 100644
>> index 0000000..504e4f6
>> --- /dev/null
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev.xml
>> @@ -0,0 +1,48 @@
>> +<domain type='qemu'>
>> +  <name>QEMUGuest1</name>
>> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
>> +  <memory>219136</memory>
>> +  <currentMemory>219136</currentMemory>
>> +  <vcpu>1</vcpu>
>> +  <os>
>> +    <type arch='i686' machine='pc'>hvm</type>
>> +    <boot dev='hd'/>
>> +  </os>
>> +  <clock offset='utc'/>
>> +  <on_poweroff>destroy</on_poweroff>
>> +  <on_reboot>restart</on_reboot>
>> +  <on_crash>destroy</on_crash>
>> +  <devices>
>> +    <emulator>/usr/bin/qemu</emulator>
>> +    <disk type='block' device='disk'>
>> +      <source dev='/dev/HostVG/QEMUGuest1'/>
>> +      <target dev='hda' bus='ide'/>
>> +      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
>> +    </disk>
>> +    <controller type='usb' index='0'/>
>> +    <controller type='ide' index='0'/>
>> +    <interface type='hostdev' managed='yes'>
>> +      <mac address='00:11:22:33:44:55'/>
>> +      <source>
>> +        <address type='pci' domain='0x0002' bus='0x03' slot='0x07' function='0x1'/>
>> +      </source>
>> +      <virtualport type='802.1Qbg'>
>> +        <parameters managerid='11' typeid='1193047' typeidversion='2' instanceid='09b11c53-8b5c-4eeb-8f00-d84eaa0aaa4f'/>
>> +      </virtualport>
>> +    </interface>
>> +    <interface type='hostdev'>
>> +      <mac address='11:11:22:33:44:55'/>
>> +      <source>
>> +        <address type='usb' bus='0' device='2'/>
>> +      </source>
>> +    </interface>
>> +    <interface type='hostdev'>
>> +      <mac address='22:11:22:33:44:55'/>
>> +      <source>
>> +        <vendor id='0x0012'/>
>> +        <product id='0x24dd'/>
>> +      </source>
>> +    </interface>
> This looks odd to me; I mean why add this <interface> here but remove it
> in the very next patch? Maybe to prove/test that parsing and formatting
> works correctly even for this case.

I put that case in when the test case only tested the parser and
formatter, but then when I added the code to handle commandline
generation in the next patch (along with extending the test case to test
that as well) the new test began to fail. This was because it's
impossible to test the vendor/product id format of address specification
without having an actual device on the bus to compare against, and
qemusml2argvtest can't do that, so I had to remove it.

I actually thought that I'd squashed the removal back into the same test
where I'd added it, which is what I'll do before pushing. (It's
especially irrelevant since we've learned that allowing USB
type='hostdev' interfaces is pointless (with the current
hardware/drivers anyway) as the only reason to have it would be to set
the mac address, but the guest os' netdevice driver will end up
resetting the mac address we've set anyway :-()
>
> ACK
>
> Michal
>