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

Cole Robinson crobinso at redhat.com
Mon Mar 7 18:45:56 UTC 2011


On 03/01/2011 08:19 AM, Gerhard Stenzel wrote:
> Hi, 
> "direct" virtual network interfaces can have additional
> information like 802.1Qbg VSI type information. This information
> is currently ignored in virt-manager and virt-inst.
> 
> This is RFC and comments are appreciated ....
> 
> A related patch for virt-manager will follow shortly.
> 
> 
> The following patch adds support for VSI type XML elements
> to virt-inst, so the respective value can be viewed and edited 
> via virt-manager
> 

Your mailer busted the patch. Easiest solution is to just attach the patch.

> Signed-off-by: Gerhard Stenzel <gerhard.stenzel at de.ibm.com>
> 
> diff -r 1f94ed6c4fc8 man/en/virt-install.1
> --- a/man/en/virt-install.1	Fri Feb 11 10:24:07 2011 -0500
> +++ b/man/en/virt-install.1	Tue Mar 01 10:19:42 2011 +0100
> @@ -124,7 +124,7 @@
>  .\"
> ========================================================================
>  .\"
>  .IX Title "VIRT-INSTALL 1"
> -.TH VIRT-INSTALL 1 "2011-01-14" "" "Virtual Machine Install Tools"
> +.TH VIRT-INSTALL 1 "2011-02-16" "" "Virtual Machine Install Tools"
>  .\" For nroff, turn off justification.  Always turn off hyphenation; it
> makes
>  .\" way too many mistakes in technical documents.
>  .if n .ad l

This part should be dropped.

> diff -r 1f94ed6c4fc8 tests/xmlparse-xml/change-nics-in.xml
> --- a/tests/xmlparse-xml/change-nics-in.xml	Fri Feb 11 10:24:07 2011
> -0500
> +++ b/tests/xmlparse-xml/change-nics-in.xml	Tue Mar 01 10:19:42 2011
> +0100
> @@ -36,6 +36,14 @@
>        <script path="/etc/qemu-ifup"/>
>        <target dev="nic02"/>
>      </interface>
> +    <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>
>      <input type="mouse" bus="ps2"/>
>      <graphics type="sdl" display=":3.4" xauth="/tmp/.Xauthority"/>
>      <console type="pty"/>
> diff -r 1f94ed6c4fc8 tests/xmlparse-xml/change-nics-out.xml
> --- a/tests/xmlparse-xml/change-nics-out.xml	Fri Feb 11 10:24:07 2011
> -0500
> +++ b/tests/xmlparse-xml/change-nics-out.xml	Tue Mar 01 10:19:42 2011
> +0100
> @@ -38,6 +38,14 @@
>        <source dev="eth1"/>
>        <script path="/etc/qemu-ifup"/>
>      </interface>
> +    <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>
>      <input type="mouse" bus="ps2"/>
>      <graphics type="sdl" display=":3.4" xauth="/tmp/.Xauthority"/>
>      <console type="pty"/>
> diff -r 1f94ed6c4fc8 tests/xmlparse.py
> --- a/tests/xmlparse.py	Fri Feb 11 10:24:07 2011 -0500
> +++ b/tests/xmlparse.py	Tue Mar 01 10:19:42 2011 +0100
> @@ -352,6 +352,7 @@
>          dev2 = guest.get_devices("interface")[1]
>          dev3 = guest.get_devices("interface")[2]
>          dev4 = guest.get_devices("interface")[3]
> +        dev5 = guest.get_devices("interface")[4]
>  
>          check = self._make_checker(dev1)
>          check("type", "user")
> @@ -385,6 +386,13 @@
>  
>          self._alter_compare(guest.get_config_xml(), outfile)
>  
> +        check = self._make_checker(dev5)
> +        check("type", "direct")
> +        check("source_dev", "eth0.1", "eth1.1")
> +        self.assertEquals(dev5.get_source(), "eth1.1")
> +
> +        self._alter_compare(guest.get_config_xml(), outfile)
> +

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

>      def testAlterInputs(self):
>          infile  = "tests/xmlparse-xml/change-inputs-in.xml"
>          outfile = "tests/xmlparse-xml/change-inputs-out.xml"
> diff -r 1f94ed6c4fc8 virtinst/VirtualNetworkInterface.py
> --- a/virtinst/VirtualNetworkInterface.py	Fri Feb 11 10:24:07 2011 -0500
> +++ b/virtinst/VirtualNetworkInterface.py	Tue Mar 01 10:19:42 2011 +0100
> @@ -83,6 +83,11 @@
>          self._model = None
>          self._target_dev = None
>          self._source_dev = None
> +        self._vsi_managerid = None
> +        self._vsi_typeid = None
> +        self._vsi_typeidversion = None
> +        self._vsi_instanceid = None
> +        self._virtualport_type = None
>  
>          # Generate _random_mac
>          self._random_mac = None
> @@ -209,6 +215,41 @@
>      source_dev = _xml_property(get_source_dev, set_source_dev,
>                                 xpath="./source/@dev")
>  
> +    def get_virtualport_type(self):
> +        return self._virtualport_type
> +    def set_virtualport_type(self, val):
> +        self._virtualport_type = val
> +    virtualport_type = _xml_property(get_virtualport_type,
> set_virtualport_type,
> +                               xpath="./virtualport/@type")
> +
> +    def get_vsi_managerid(self):
> +        return self._vsi_managerid
> +    def set_vsi_managerid(self, val):
> +        self._vsi_managerid = val
> +    vsi_managerid = _xml_property(get_vsi_managerid, set_vsi_managerid,
> +
> xpath="./virtualport/parameters/@managerid")
> +
> +    def get_vsi_typeid(self):
> +        return self._vsi_typeid
> +    def set_vsi_typeid(self, val):
> +        self._vsi_typeid = val
> +    vsi_typeid = _xml_property(get_vsi_typeid, set_vsi_typeid,
> +
> xpath="./virtualport/parameters/@typeid")
> +
> +    def get_vsi_typeidversion(self):
> +        return self._vsi_typeidversion
> +    def set_vsi_typeidversion(self, val):
> +        self._vsi_typeidversion = val
> +    vsi_typeidversion = _xml_property(get_vsi_typeidversion,
> set_vsi_typeidversion,
> +
> xpath="./virtualport/parameters/@typeidversion")
> +
> +    def get_vsi_instanceid(self):
> +        return self._vsi_instanceid
> +    def set_vsi_instanceid(self, val):
> +        self._vsi_instanceid = val
> +    vsi_instanceid = _xml_property(get_vsi_instanceid,
> set_vsi_instanceid,
> +
> xpath="./virtualport/parameters/@instanceid")
> +
>      def is_conflict_net(self, conn, mac=None):
>          """
>          is_conflict_net: determines if mac conflicts with others in
> system
> 
> 


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.

Thanks,
Cole




More information about the virt-tools-list mailing list