Change in vdsm[ovirt-3.3]: vnic QoS: Fix specParams to XML definition conversion

asegurap at redhat.com asegurap at redhat.com
Mon Mar 31 14:10:59 UTC 2014


Hello Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/26253

to review the following change.

Change subject: vnic QoS: Fix specParams to XML definition conversion
......................................................................

vnic QoS: Fix specParams to XML definition conversion

VDSM API specifies specParams input and output data to have values
specified as integers. In order to generate valid XML, those values
should have been converted to strings (and the test should have been
passing integers as per VDSM schema).

This patch fixes the conversion code and the tests for it (fix one,
add a new one for the update flow).

Change-Id: I25041b8f853c870388ecd9c9f9176bd384242234
Bug-Url: https://bugzilla.redhat.com/1067064
Signed-off-by: Antoni S. Puimedon <asegurap at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/25788
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
Signed-off-by: Antoni S. Puimedon <asegurap at redhat.com>
---
M tests/vmTests.py
M vdsm/vm.py
2 files changed, 46 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/53/26253/1

diff --git a/tests/vmTests.py b/tests/vmTests.py
index 6215041..e061d42 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -394,14 +394,42 @@
                'network': 'ovirtmgmt', 'address': self.PCI_ADDR_DICT,
                'device': 'bridge', 'type': 'interface',
                'bootOrder': '1', 'filter': 'no-mac-spoofing',
-               'specParams': {'inbound': {'average': '1000', 'peak': '5000',
-                                          'burst': '1024'},
-                              'outbound': {'average': '128', 'burst': '256'}}}
+               'specParams': {'inbound': {'average': 1000, 'peak': 5000,
+                                          'burst': 1024},
+                              'outbound': {'average': 128, 'burst': 256}}}
 
         self.conf['custom'] = {'vhost': 'ovirtmgmt:true', 'sndbuf': '0'}
         iface = vm.NetworkInterfaceDevice(self.conf, self.log, **dev)
         self.assertXML(iface.getXML(), interfaceXML)
 
+    def testInterfaceXMLBandwidthUpdate(self):
+        originalBwidthXML = """
+                <bandwidth>
+                    <inbound average="1000" burst="1024" peak="5000"/>
+                    <outbound average="128" burst="256"/>
+                </bandwidth>"""
+        NEW_OUT = {'outbound': {'average': 1042, 'burst': 128, 'peak': 500}}
+        updatedBwidthXML = """
+                <bandwidth>
+                    <inbound average="1000" burst="1024" peak="5000"/>
+                    <outbound average="%(average)s" burst="%(burst)s"
+                    peak="%(peak)s"/>
+                </bandwidth>""" % NEW_OUT['outbound']
+
+        dev = {'nicModel': 'virtio', 'macAddr': '52:54:00:59:F5:3F',
+               'network': 'ovirtmgmt', 'address': self.PCI_ADDR_DICT,
+               'device': 'bridge', 'type': 'interface',
+               'bootOrder': '1', 'filter': 'no-mac-spoofing',
+               'specParams': {'inbound': {'average': 1000, 'peak': 5000,
+                                          'burst': 1024},
+                              'outbound': {'average': 128, 'burst': 256}}}
+        self.conf['custom'] = {'vhost': 'ovirtmgmt:true', 'sndbuf': '0'}
+        iface = vm.NetworkInterfaceDevice(self.conf, self.log, **dev)
+        originalBandwidth = iface.getXML().getElementsByTagName('bandwidth')[0]
+        self.assertXML(originalBandwidth, originalBwidthXML)
+        self.assertXML(iface.paramsToBandwidthXML(NEW_OUT, originalBandwidth),
+                       updatedBwidthXML)
+
     def testControllerXML(self):
         devConfs = [
             {'device': 'ide', 'index': '0', 'address': self.PCI_ADDR_DICT},
diff --git a/vdsm/vm.py b/vdsm/vm.py
index 832bca8..09e6031 100644
--- a/vdsm/vm.py
+++ b/vdsm/vm.py
@@ -1413,27 +1413,23 @@
 
         if hasattr(self, 'specParams'):
             if 'inbound' in self.specParams or 'outbound' in self.specParams:
-                iface.appendChild(self.getXMLBandwidth(self.specParams))
+                iface.appendChild(self.paramsToBandwidthXML(self.specParams))
         return iface
 
-    def getXMLBandwidth(self, specParams, oldBandwidth=None):
+    def paramsToBandwidthXML(self, specParams, oldBandwidth=None):
+        """Returns a valid libvirt xml dom element object."""
         bandwidth = self.createXmlElem('bandwidth', None)
-        # Inbound and Outbound traffic can be independently shaped.
-        for attr in ('inbound', 'outbound'):
-            newSetting = specParams.get(attr)
-            # if newSetting is specified, replace current settings
-            if newSetting:
-                bandwidth.appendChildWithArgs(attr, **newSetting)
-            # if newSetting is not specified, keep current settings
-            elif newSetting is None:
-                if oldBandwidth is not None:
-                    attrXMLs = oldBandwidth.getElementsByTagName
-                    attrXML = attrXMLs[0] if len(attrXMLs) else None
-                else:
-                    attrXML = None
-                if attrXML is not None:
-                    bandwidth.appendChild(attrXML)
-            # if newSetting is {} do nothing = remove current settings
+        old = {} if oldBandwidth is None else dict(
+            (elem.nodeName, elem) for elem in oldBandwidth.childNodes)
+        for key in ('inbound', 'outbound'):
+            elem = specParams.get(key)
+            if elem is None:  # Use the old setting if present
+                if key in old:
+                    bandwidth.appendChild(old[key])
+            elif elem:
+                # Convert the values to string for adding them to the XML def
+                attrs = dict((key, str(value)) for key, value in elem.items())
+                bandwidth.appendChildWithArgs(key, **attrs)
         return bandwidth
 
 
@@ -3344,7 +3340,7 @@
                 ('inbound' in specParams or 'outbound' in specParams)):
             oldBandwidths = vnicXML.getElementsByTagName('bandwidth')
             oldBandwidth = oldBandwidths[0] if len(oldBandwidths) else None
-            newBandwidth = dev.getXMLBandwidth(specParams, oldBandwidth)
+            newBandwidth = dev.paramsToBandwidthXML(specParams, oldBandwidth)
             if oldBandwidth is None:
                 vnicXML.appendChild(newBandwidth)
             else:


-- 
To view, visit http://gerrit.ovirt.org/26253
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I25041b8f853c870388ecd9c9f9176bd384242234
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.3
Gerrit-Owner: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>


More information about the vdsm-patches mailing list