From 29ee8011b4e1cf5371b1aa9c6c0f930eb49fe795 Mon Sep 17 00:00:00 2001
From: Mohammed Naser <mnaser@vexxhost.com>
Date: Mon, 29 Oct 2018 19:18:26 +0100
Subject: [PATCH] Add tests for bug #1800511

In Rocky, we started including the MTU for networks when vif_type
is bridge or tap and libvirt >= 3.3.0, however this also meant
that we started specifying the MTU when doing live migrations.

It seems that the guest ABI changes when setting the MTU which
means that live migrations will fail.  This broke the live migration
of any instance that was launched prior to upgrading to Rocky, as
it was not loaded with the ABI having host_mtu specified.

This is a passing (negative) test-case which will include a follow
up patch that contains a fix and correcting tests.

Related-Bug: #1800511
Change-Id: Ia2fe50d727b1f83e808cb9dda3a55f853f048a3e
---
 .../tests/unit/virt/libvirt/test_migration.py | 130 +++++++++++++++---
 1 file changed, 113 insertions(+), 17 deletions(-)

diff --git a/nova/tests/unit/virt/libvirt/test_migration.py b/nova/tests/unit/virt/libvirt/test_migration.py
index 9c4d594c4b6a..64b0739f413b 100644
--- a/nova/tests/unit/virt/libvirt/test_migration.py
+++ b/nova/tests/unit/virt/libvirt/test_migration.py
@@ -671,7 +671,7 @@ class UtilityMigrationTestCase(test.NoDBTestCase):
   </memoryBacking>
 </domain>"""))
 
-    def test_update_vif_xml(self):
+    def _test_update_vif_xml(self, conf, original_xml, expected_xml):
         """Simulates updating the guest xml for live migrating from a host
         using OVS to a host using vhostuser as the networking backend.
         """
@@ -695,6 +695,22 @@ class UtilityMigrationTestCase(test.NoDBTestCase):
         ]
         data = objects.LibvirtLiveMigrateData(vifs=migrate_vifs)
 
+        get_vif_config = mock.MagicMock(return_value=conf)
+        doc = etree.fromstring(original_xml)
+        updated_xml = etree.tostring(
+            migration._update_vif_xml(doc, data, get_vif_config),
+            encoding='unicode')
+        self.assertThat(updated_xml, matchers.XMLMatches(expected_xml))
+
+    def test_update_vif_xml_to_vhostuser(self):
+        conf = vconfig.LibvirtConfigGuestInterface()
+        conf.net_type = "vhostuser"
+        conf.vhostuser_type = "unix"
+        conf.vhostuser_mode = "server"
+        conf.mac_addr = "DE:AD:BE:EF:CA:FE"
+        conf.vhostuser_path = "/vhost-user/test.sock"
+        conf.model = "virtio"
+
         original_xml = """<domain>
  <uuid>3de6550a-8596-4937-8046-9d862036bca5</uuid>
  <devices>
@@ -711,21 +727,6 @@ class UtilityMigrationTestCase(test.NoDBTestCase):
     </interface>
  </devices>
 </domain>""" % uuids.ovs
-
-        conf = vconfig.LibvirtConfigGuestInterface()
-        conf.net_type = "vhostuser"
-        conf.vhostuser_type = "unix"
-        conf.vhostuser_mode = "server"
-        conf.mac_addr = "DE:AD:BE:EF:CA:FE"
-        conf.vhostuser_path = "/vhost-user/test.sock"
-        conf.model = "virtio"
-
-        get_vif_config = mock.MagicMock(return_value=conf)
-        doc = etree.fromstring(original_xml)
-        updated_xml = etree.tostring(
-            migration._update_vif_xml(doc, data, get_vif_config),
-            encoding='unicode')
-
         # Note that <target> and <virtualport> are dropped from the ovs source
         # interface xml since they aren't applicable to the vhostuser
         # destination interface xml. The type attribute value changes and the
@@ -742,7 +743,102 @@ class UtilityMigrationTestCase(test.NoDBTestCase):
     </interface>
  </devices>
 </domain>"""
-        self.assertThat(updated_xml, matchers.XMLMatches(expected_xml))
+        self._test_update_vif_xml(conf, original_xml, expected_xml)
+
+    def test_update_vif_xml_to_bridge_without_mtu(self):
+        """This test validates _update_vif_xml to make sure it does not add
+        MTU to the interface if it does not exist in the source XML.
+        """
+        conf = vconfig.LibvirtConfigGuestInterface()
+        conf.net_type = "bridge"
+        conf.source_dev = "qbra188171c-ea"
+        conf.target_dev = "tapa188171c-ea"
+        conf.mac_addr = "DE:AD:BE:EF:CA:FE"
+        conf.vhostuser_path = "/vhost-user/test.sock"
+        conf.model = "virtio"
+        conf.mtu = 9000
+
+        original_xml = """<domain>
+ <uuid>3de6550a-8596-4937-8046-9d862036bca5</uuid>
+ <devices>
+    <interface type="bridge">
+        <mac address="DE:AD:BE:EF:CA:FE"/>
+        <model type="virtio"/>
+        <source bridge="qbra188171c-ea"/>
+        <target dev="tapa188171c-ea"/>
+        <virtualport type="openvswitch">
+            <parameters interfaceid="%s"/>
+        </virtualport>
+        <address type='pci' domain='0x0000' bus='0x00' slot='0x04'
+                 function='0x0'/>
+    </interface>
+ </devices>
+</domain>""" % uuids.ovs
+        expected_xml = """<domain>
+ <uuid>3de6550a-8596-4937-8046-9d862036bca5</uuid>
+ <devices>
+    <interface type="bridge">
+        <mac address="DE:AD:BE:EF:CA:FE"/>
+        <model type="virtio"/>
+        <source bridge="qbra188171c-ea"/>
+        <!--
+            NOTE(mnaser): This should not be here, remove when fixed
+                          https://bugs.launchpad.net/nova/+bug/1800511
+        -->
+        <mtu size="9000"/>
+        <target dev="tapa188171c-ea"/>
+        <address type='pci' domain='0x0000' bus='0x00' slot='0x04'
+                 function='0x0'/>
+    </interface>
+ </devices>
+</domain>"""
+        self._test_update_vif_xml(conf, original_xml, expected_xml)
+
+    def test_update_vif_xml_to_bridge_with_mtu(self):
+        """This test validates _update_vif_xml to make sure it maintains the
+        interface MTU if it exists in the source XML
+        """
+        conf = vconfig.LibvirtConfigGuestInterface()
+        conf.net_type = "bridge"
+        conf.source_dev = "qbra188171c-ea"
+        conf.target_dev = "tapa188171c-ea"
+        conf.mac_addr = "DE:AD:BE:EF:CA:FE"
+        conf.vhostuser_path = "/vhost-user/test.sock"
+        conf.model = "virtio"
+        conf.mtu = 9000
+
+        original_xml = """<domain>
+ <uuid>3de6550a-8596-4937-8046-9d862036bca5</uuid>
+ <devices>
+    <interface type="bridge">
+        <mac address="DE:AD:BE:EF:CA:FE"/>
+        <model type="virtio"/>
+        <source bridge="qbra188171c-ea"/>
+        <target dev="tapa188171c-ea"/>
+        <mtu size="9000"/>
+        <virtualport type="openvswitch">
+            <parameters interfaceid="%s"/>
+        </virtualport>
+        <address type='pci' domain='0x0000' bus='0x00' slot='0x04'
+                 function='0x0'/>
+    </interface>
+ </devices>
+</domain>""" % uuids.ovs
+        expected_xml = """<domain>
+ <uuid>3de6550a-8596-4937-8046-9d862036bca5</uuid>
+ <devices>
+    <interface type="bridge">
+        <mac address="DE:AD:BE:EF:CA:FE"/>
+        <model type="virtio"/>
+        <source bridge="qbra188171c-ea"/>
+        <mtu size="9000"/>
+        <target dev="tapa188171c-ea"/>
+        <address type='pci' domain='0x0000' bus='0x00' slot='0x04'
+                 function='0x0'/>
+    </interface>
+ </devices>
+</domain>"""
+        self._test_update_vif_xml(conf, original_xml, expected_xml)
 
     def test_update_vif_xml_no_mac_address_in_xml(self):
         """Tests that the <mac address> is not found in the <interface> XML