[virt-tools-list] [RFC 1 of2] adding 802.1Qbg VSI type support to virtinst and virtmanager

[Cole Robinson]

On 03/08/2011 04:56 AM, Gerhard Stenzel wrote:
> On Mon, 2011-03-07 at 13:45 -0500, Cole Robinson wrote:
> ...
>> Your mailer busted the patch. Easiest solution is to just attach the patch.
> 
> ok, attached this time
> 
> ...
>> This part should be dropped.
> done
> 
> ...
>> _alter_compare should only be called once at the end of the function, it
>> compares the guest instance that we altered against the expected output file.
>> In fact I'm surprised calling it twice like this actually works: is python
>> setup.py test actually passing?
>>
>> You should probably also be checking all the new interface properties you are
>> adding, like
>>
>> check("vsi_managerid", "12", "15")
>>
>> or similar.
>>
>> Additionally, take a look at the _make_checker definition, that last
>> assertEquals is redundant.
> 
> partially done. I have to admit I don't fully understand yet how the
> tests work ... i just copied and modified.
> 

An example:

        check = self._make_checker(dev3)
        check("type", "bridge")

This call is checking that dev3.type == "bridge"

        check("bridge", "foobr0", "newfoo0")

This call is checking that dev3.bridge == "foobr0". It is then doing

  dev3.bridge = "newfoo0"
  assert(dev3.bridge, "newfoo0")

Since we are parsing XML in place, this is going to alter the final XML
document that is produced (we will now have <source bridge='newfoo0'/>)
so you will needed to edit the test file in tests/xmlparse-xml/change-nics-out.xml

Since you added a new network device to parse:

    <interface type="direct">
      <mac address="00:11:22:33:44:55"/>
      <source dev="eth0.1" mode="vepa"/>
      <virtualport type="802.1Qbg">
        <parameters managerid="12" typeid="1193046" typeidversion="1"
instanceid="09b11c53-8b5c-4eeb-8f00-d84eaa0aaa3b"/>
      </virtualport>
      <address type="pci" domain="0x0000" bus="0x00" slot="0x07" function="0x0"/>
    </interface>

You will want to do to make sure that virtinst is correctly parsing and
setting every new property, ex:

        check = self._make_checker(dev5)
        ...
        check("vsi_managerid", "12", "15")
	...

which should generate new XML like

      <virtualport type="802.1Qbg">
        <parameters managerid="15" .../>
      </virtualport>


> ...
>>
>> I think I asked this before, but is it possible that an interface can have
>> more than 1 virtualport? If not now then in the future? If so, it might be
>> better to make a class InterfaceVirtualPort or something, and have the
>> VirtualInterface carry a list of those.
> 
> the virtual interface of a VM can have one virtual port. But I am happy
> to make such a class, if you point me to an example (I am not really a
> python expert).
> 

You can start with this:

diff -r 7faf95085c8c virtinst/VirtualNetworkInterface.py
--- a/virtinst/VirtualNetworkInterface.py	Wed Mar 09 11:45:54 2011 -0500
+++ b/virtinst/VirtualNetworkInterface.py	Wed Mar 09 14:27:44 2011 -0500
@@ -22,6 +22,7 @@

 import _util
 import VirtualDevice
+import XMLBuilderDomain
 from XMLBuilderDomain import _xml_property
 from virtinst import _virtinst as _

@@ -44,6 +45,12 @@
         count += _util.get_xml_path(xml, func=count_cb)
     return count

+class VirtualPort(XMLBuilderDomain.XMLBuilderDomain):
+
+    def __init__(self, conn, parsexml=None, parsexmlnode=None, caps=None):
+        XMLBuilderDomain.XMLBuilderDomain.__init__(self, conn, parsexml,
+                                                   parsexmlnode, caps=caps)
+
 class VirtualNetworkInterface(VirtualDevice.VirtualDevice):

     _virtual_device_type = VirtualDevice.VirtualDevice.VIRTUAL_DEV_NET
@@ -83,6 +90,7 @@
         self._model = None
         self._target_dev = None
         self._source_dev = None
+        self._virtualport = VirtualPort(conn, parsexml, parsexmlnode, caps)

         # Generate _random_mac
         self._random_mac = None
@@ -136,6 +144,10 @@
             return None
         return self.network or self.bridge or self.source_dev

+    def _get_virtualport(self):
+        return self._virtualport
+    virtualport = property(_get_virtualport)
+
     def get_type(self):
         return self._type
     def set_type(self, val):


Then move your current changes to VirtualPort and rename as appropriate.
dev.vsi_instanceid would then be dev.virtualport.instanceid

Also, might as well add a property for /virtualport/@mode as well, even if
changing it isn't a priority

- Cole