libvirt: do not return serial address if disabled on destination

During live migration, the check on the destination host is returning
the address of serial proxy even if disabled. However, the absence of
this serial address is the signal the source node needs to know to
determine if the target node supports the serial console feature.
The absence of the serial_listen_addr is specified by the value None.
This makes it also necessary to *not* do a string conversion of None,
as this (maybe surprisingly) results in the string 'None'. The string
'None' however is *not* a valid serial_listen_addr value, as this
must be a valid IP address. The reason for this is, that this value
is used to update the domain XML of the libvirt guest.

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

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

diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index 0e60818913293f5caeade3c885372fa864eb36df..092c652a6394a2a84947bf07cf473b35f72462a8 100644 (file)
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -6865,6 +6865,7 @@ class LibvirtConnTestCase(test.NoDBTestCase):
         self.flags(vncserver_listen='192.0.2.12', group='vnc')
         self.flags(server_listen='198.51.100.34', group='spice')
         self.flags(proxyclient_address='203.0.113.56', group='serial_console')
+        self.flags(enabled=True, group='serial_console')
         mock_cpu.return_value = 1
 
         instance_ref = objects.Instance(**self.test_instance)
@@ -6881,6 +6882,24 @@ class LibvirtConnTestCase(test.NoDBTestCase):
         self.assertEqual('203.0.113.56',
                          str(result.serial_listen_addr))
 
+    @mock.patch.object(objects.Service, 'get_by_compute_host')
+    @mock.patch.object(libvirt_driver.LibvirtDriver,
+                       '_create_shared_storage_test_file',
+                       return_value='fake')
+    @mock.patch.object(fakelibvirt.Connection, 'compareCPU',
+                       return_value=1)
+    def test_check_can_live_migrate_dest_ensure_serial_adds_not_set(
+            self, mock_cpu, mock_test_file, mock_svc):
+        self.flags(proxyclient_address='127.0.0.1', group='serial_console')
+        self.flags(enabled=False, group='serial_console')
+        instance_ref = objects.Instance(**self.test_instance)
+        instance_ref.vcpu_model = test_vcpu_model.fake_vcpumodel
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        compute_info = {'cpu_info': 'asdf', 'disk_available_least': 1}
+        result = drvr.check_can_live_migrate_destination(
+            self.context, instance_ref, compute_info, compute_info)
+        self.assertIsNone(result.serial_listen_addr)
+
     @mock.patch.object(libvirt_driver.LibvirtDriver,
                        '_create_shared_storage_test_file',
                        return_value='fake')

diff --git a/nova/tests/unit/virt/libvirt/test_migration.py b/nova/tests/unit/virt/libvirt/test_migration.py
index 78906814db8ce30654cc600a8860bf13c19cf74f..fb644a1301d8277a214fbb3f27c2ee2cf04187c1 100644 (file)
--- a/nova/tests/unit/virt/libvirt/test_migration.py
+++ b/nova/tests/unit/virt/libvirt/test_migration.py
@@ -73,6 +73,12 @@ class UtilityMigrationTestCase(test.NoDBTestCase):
         addr = migration.serial_listen_addr(data)
         self.assertIsNone(addr)
 
+    def test_serial_listen_addr_None(self):
+        data = objects.LibvirtLiveMigrateData()
+        data.serial_listen_addr = None
+        addr = migration.serial_listen_addr(data)
+        self.assertIsNone(addr)
+
     @mock.patch('lxml.etree.tostring')
     @mock.patch.object(migration, '_update_perf_events_xml')
     @mock.patch.object(migration, '_update_graphics_xml')

diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index d731859cd1979104f936eb1e1b1979baa59a4e04..2bbc4732cdf55ec54cd371cc39535634050cd5b6 100644 (file)
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -5461,7 +5461,10 @@ class LibvirtDriver(driver.ComputeDriver):
         data.image_type = CONF.libvirt.images_type
         data.graphics_listen_addr_vnc = CONF.vnc.vncserver_listen
         data.graphics_listen_addr_spice = CONF.spice.server_listen
-        data.serial_listen_addr = CONF.serial_console.proxyclient_address
+        if CONF.serial_console.enabled:
+            data.serial_listen_addr = CONF.serial_console.proxyclient_address
+        else:
+            data.serial_listen_addr = None
         # Notes(eliqiao): block_migration and disk_over_commit are not
         # nullable, so just don't set them if they are None
         if block_migration is not None:

diff --git a/nova/virt/libvirt/migration.py b/nova/virt/libvirt/migration.py
index dcfc679b2c24612ef7f3c923344fff1e344c37be..7a6e42f4b4a734af509315fd61f5caf5fa02cca2 100644 (file)
--- a/nova/virt/libvirt/migration.py
+++ b/nova/virt/libvirt/migration.py
@@ -50,8 +50,18 @@ def graphics_listen_addrs(migrate_data):
 def serial_listen_addr(migrate_data):
     """Returns listen address serial from a LibvirtLiveMigrateData"""
     listen_addr = None
+    # NOTE (markus_z/dansmith): Our own from_legacy_dict() code can return
+    # an object with nothing set here. That can happen based on the
+    # compute RPC version pin. Until we can bump that major (which we
+    # can do just before Ocata releases), we may still get a legacy
+    # dict over the wire, converted to an object, and thus is may be unset
+    # here.
     if migrate_data.obj_attr_is_set('serial_listen_addr'):
-        listen_addr = str(migrate_data.serial_listen_addr)
+        # NOTE (markus_z): The value of 'serial_listen_addr' is either
+        # an IP address (as string type) or None. There's no need of a
+        # conversion, in fact, doing a string conversion of None leads to
+        # 'None', which is an invalid (string type) value here.
+        listen_addr = migrate_data.serial_listen_addr
     return listen_addr