virt: fix partial QoS removal

Since commit 50cd770, vdsm uses the old
bandwidth value when updating (the QoS) on the NIC.

Now lets say we have the following active on a VM NIC:
    <bandwidth>
        <inbound average="128000" peak="384000" burst="1024000" />
        <outbound average="128000" burst="1024000" peak="384000" />
    </bandwidth>

And we pass the following update from the engine:
      <bandwidth>
        <outbound average="128000" burst="1024000" peak="384000"/>
      </bandwidth>

Then it will preserve the inbound QoS, which is not something we want.

This is a leftover from pre 'libvirt XML' oVirt era.
Cause before it needed an empty 'inbound' tag to remove the inbound QoS.
See the notes in the above commit:
'It is deleted if it is in specParams but empty ({'inbound':{}}).'

But as the complete XML is now passed to vdsm, we can just rely on the
data coming from the engine, without the need to parse the old bandwidth
data.

Signed-off-by: Jean-Louis Dupond <jean-louis@dupond.be>

Author: dupondje

lib/vdsm/virt/vmdevices/network.py

@@ -302,18 +302,12 @@ def _filter_parameter_map(self):
                 yield parameter['name'], parameter['value']
 
     @staticmethod
-    def get_bandwidth_xml(specParams, oldBandwidth=None):
+    def get_bandwidth_xml(specParams):
         """Returns a valid libvirt xml dom element object."""
         bandwidth = vmxml.Element('bandwidth')
-        old = {} if oldBandwidth is None else dict(
-            (vmxml.tag(elem), elem)
-            for elem in vmxml.children(oldBandwidth))
         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(etree_element=old[key])
-            elif elem:
+            if 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)
@@ -487,7 +481,7 @@ def update_bandwidth_xml(iface, vnicXML, specParams=None):
         vmxml.remove_child(vnicXML, oldBandwidth)
     if (specParams and
             ('inbound' in specParams or 'outbound' in specParams)):
-        newBandwidth = iface.get_bandwidth_xml(specParams, oldBandwidth)
+        newBandwidth = iface.get_bandwidth_xml(specParams)
         vmxml.append_child(vnicXML, newBandwidth)
 
 

tests/virt/device_test.py

@@ -105,13 +105,14 @@ def testInterfaceXMLBandwidthUpdate(self):
                     <inbound average="1000" burst="1024" peak="5000"/>
                     <outbound average="128" burst="256"/>
                 </bandwidth>"""
-        NEW_OUT = {'outbound': {'average': 1042, 'burst': 128, 'peak': 500}}
+        NEW_BW = {'inbound': {'average': 1000, 'burst': 1024, 'peak': 5000},
+                  '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']
+                </bandwidth>""" % NEW_BW['outbound']
 
         dev = {'nicModel': 'virtio', 'macAddr': '52:54:00:59:F5:3F',
                'network': 'ovirtmgmt', 'address': self.PCI_ADDR_DICT,
@@ -126,7 +127,7 @@ def testInterfaceXMLBandwidthUpdate(self):
         iface = vmdevices.network.Interface(self.log, **dev)
         orig_bandwidth = iface.getXML().findall('bandwidth')[0]
         self.assert_dom_xml_equal(orig_bandwidth, originalBwidthXML)
-        bandwith = iface.get_bandwidth_xml(NEW_OUT, orig_bandwidth)
+        bandwith = iface.get_bandwidth_xml(NEW_BW)
         self.assert_dom_xml_equal(bandwith, updatedBwidthXML)
 
     def testInterfaceFilterUpdate(self):

tests/virt/devicexml_test.py

@@ -162,6 +162,22 @@ def test_update_bandwidth_xml(self, base_spec_params):
         vmdevices.network.update_bandwidth_xml(dev, vnic_xml, specParams)
         self.assertXMLEqual(xmlutils.tostring(vnic_xml), XML)
 
+        specParams = {}
+        XML = u"""
+        <interface type='network'>
+          <mac address="fake" />
+          <source bridge='default'/>
+          <link state="up"/>
+          <bandwidth>
+            <outbound average='128' peak='256' burst='256'/>
+          </bandwidth>
+        </interface>
+        """
+        dev = vmdevices.network.Interface(self.log, **conf)
+        vnic_xml = dev.getXML()
+        vmdevices.network.update_bandwidth_xml(dev, vnic_xml, specParams)
+        self.assertXMLEqual(xmlutils.tostring(vnic_xml), XML)
+
         specParams = {}
         XML = u"""
         <interface type='network'>
@@ -170,6 +186,8 @@ def test_update_bandwidth_xml(self, base_spec_params):
           <link state="up"/>
         </interface>
         """
+        dev = vmdevices.network.Interface(self.log, **conf)
+        vnic_xml = dev.getXML()
         vmdevices.network.update_bandwidth_xml(dev, vnic_xml, specParams)
         self.assertXMLEqual(xmlutils.tostring(vnic_xml), XML)