libvirt: prepare domain XML update for serial ports

The serial console feature uses "ports" on a compute node to enable
a websocket connection. On a host, one port can only be used by one
single instance. If another instance gets live-migrated to that host,
an update of the ports in the domain XML is needed. Otherwise we will
get an exception from libvirt/qemu which states:

    Failed to bind socket: Cannot assign requested address

This change here *prepares* the update of the domain XML. Another change
is necessary which actually provides the ports to use for this update
function within the LibvirtLiveMigrateData object.

This change is a preparation for the fix of bug 1455252.

Co-Authored-By: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com>
Change-Id: I531ec76bc82beaea89f3a94a72e8a75627a9aa23

diff --git a/nova/tests/unit/virt/libvirt/test_migration.py b/nova/tests/unit/virt/libvirt/test_migration.py
index fb644a1301d8277a214fbb3f27c2ee2cf04187c1..b9c79046592058c98f25e898bf64158b2ab117c3 100644 (file)
--- a/nova/tests/unit/virt/libvirt/test_migration.py
+++ b/nova/tests/unit/virt/libvirt/test_migration.py
@@ -79,6 +79,17 @@ class UtilityMigrationTestCase(test.NoDBTestCase):
         addr = migration.serial_listen_addr(data)
         self.assertIsNone(addr)
 
+    def test_serial_listen_ports(self):
+        data = objects.LibvirtLiveMigrateData(
+            serial_listen_ports=[1, 2, 3])
+        ports = migration.serial_listen_ports(data)
+        self.assertEqual([1, 2, 3], ports)
+
+    def test_serial_listen_ports_emtpy(self):
+        data = objects.LibvirtLiveMigrateData()
+        ports = migration.serial_listen_ports(data)
+        self.assertEqual([], ports)
+
     @mock.patch('lxml.etree.tostring')
     @mock.patch.object(migration, '_update_perf_events_xml')
     @mock.patch.object(migration, '_update_graphics_xml')
@@ -101,26 +112,60 @@ class UtilityMigrationTestCase(test.NoDBTestCase):
 
     def test_update_serial_xml_serial(self):
         data = objects.LibvirtLiveMigrateData(
-            serial_listen_addr='127.0.0.100')
+            serial_listen_addr='127.0.0.100',
+            serial_listen_ports=[2001])
         xml = """<domain>
   <devices>
     <serial type="tcp">
-      <source host="127.0.0.1"/>
+      <source host="127.0.0.1" service="2000"/>
+      <target type="serial" port="0"/>
     </serial>
   </devices>
 </domain>"""
         doc = etree.fromstring(xml)
         res = etree.tostring(migration._update_serial_xml(doc, data))
-        new_xml = xml.replace("127.0.0.1", "127.0.0.100")
+        new_xml = xml.replace("127.0.0.1", "127.0.0.100").replace(
+            "2000", "2001")
         self.assertThat(res, matchers.XMLMatches(new_xml))
 
     def test_update_serial_xml_console(self):
         data = objects.LibvirtLiveMigrateData(
-            serial_listen_addr='127.0.0.100')
+            serial_listen_addr='127.0.0.100',
+            serial_listen_ports=[299, 300])
         xml = """<domain>
   <devices>
     <console type="tcp">
-      <source host="127.0.0.1"/>
+      <source host="127.0.0.1" service="2001"/>
+      <target type="serial" port="0"/>
+    </console>
+    <console type="tcp">
+      <source host="127.0.0.1" service="2002"/>
+      <target type="serial" port="1"/>
+    </console>
+  </devices>
+</domain>"""
+        doc = etree.fromstring(xml)
+        res = etree.tostring(migration._update_serial_xml(doc, data))
+        new_xml = xml.replace("127.0.0.1", "127.0.0.100").replace(
+            "2001", "299").replace("2002", "300")
+
+        self.assertThat(res, matchers.XMLMatches(new_xml))
+
+    def test_update_serial_xml_without_ports(self):
+        # This test is for backwards compatibility when we don't
+        # get the serial ports from the target node.
+        data = objects.LibvirtLiveMigrateData(
+            serial_listen_addr='127.0.0.100',
+            serial_listen_ports=[])
+        xml = """<domain>
+  <devices>
+    <console type="tcp">
+      <source host="127.0.0.1" service="2001"/>
+      <target type="serial" port="0"/>
+    </console>
+    <console type="tcp">
+      <source host="127.0.0.1" service="2002"/>
+      <target type="serial" port="1"/>
     </console>
   </devices>
 </domain>"""

diff --git a/nova/virt/libvirt/migration.py b/nova/virt/libvirt/migration.py
index 7a6e42f4b4a734af509315fd61f5caf5fa02cca2..38178a57055846151f6576eeca0cb2628ce4f05a 100644 (file)
--- a/nova/virt/libvirt/migration.py
+++ b/nova/virt/libvirt/migration.py
@@ -65,6 +65,15 @@ def serial_listen_addr(migrate_data):
     return listen_addr
 
 
+# TODO(sahid): remove me for Q*
+def serial_listen_ports(migrate_data):
+    """Returns ports serial from a LibvirtLiveMigrateData"""
+    ports = []
+    if migrate_data.obj_attr_is_set('serial_listen_ports'):
+        ports = migrate_data.serial_listen_ports
+    return ports
+
+
 def get_updated_guest_xml(guest, migrate_data, get_volume_config):
     xml_doc = etree.fromstring(guest.get_xml_desc(dump_migratable=True))
     xml_doc = _update_graphics_xml(xml_doc, migrate_data)
@@ -91,12 +100,31 @@ def _update_graphics_xml(xml_doc, migrate_data):
 
 def _update_serial_xml(xml_doc, migrate_data):
     listen_addr = serial_listen_addr(migrate_data)
-    for dev in xml_doc.findall("./devices/serial[@type='tcp']/source"):
-        if dev.get('host') is not None:
-            dev.set('host', listen_addr)
-    for dev in xml_doc.findall("./devices/console[@type='tcp']/source"):
-        if dev.get('host') is not None:
-            dev.set('host', listen_addr)
+    listen_ports = serial_listen_ports(migrate_data)
+
+    def set_listen_addr_and_port(source, listen_addr, serial_listen_ports):
+        # The XML nodes can be empty, which would make checks like
+        # "if source.get('host'):" different to an explicit check for
+        # None. That's why we have to check for None in this method.
+        if source.get('host') is not None:
+            source.set('host', listen_addr)
+        device = source.getparent()
+        target = device.find("target")
+        if target is not None and source.get('service') is not None:
+            port_index = int(target.get('port'))
+            # NOTE (markus_z): Previous releases might not give us the
+            # ports yet, that's why we have this check here.
+            if len(serial_listen_ports) > port_index:
+                source.set('service', str(serial_listen_ports[port_index]))
+
+    # This updates all "LibvirtConfigGuestSerial" devices
+    for source in xml_doc.findall("./devices/serial[@type='tcp']/source"):
+        set_listen_addr_and_port(source, listen_addr, listen_ports)
+
+    # This updates all "LibvirtConfigGuestConsole" devices
+    for source in xml_doc.findall("./devices/console[@type='tcp']/source"):
+        set_listen_addr_and_port(source, listen_addr, listen_ports)
+
     return xml_doc