From e4ed3a3a741625df045c0589db85b5262064ef48 Mon Sep 17 00:00:00 2001 From: Sahid Orentino Ferdjaoui Date: Mon, 14 Nov 2016 11:10:58 -0500 Subject: [PATCH] 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 Change-Id: I01dc0424ea3cf5576ed9d2b49a7a3e458324fbe4 --- nova/tests/unit/virt/libvirt/test_driver.py | 19 +++++++++++++++++++ .../tests/unit/virt/libvirt/test_migration.py | 6 ++++++ nova/virt/libvirt/driver.py | 5 ++++- nova/virt/libvirt/migration.py | 12 +++++++++++- 4 files changed, 40 insertions(+), 2 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 0e60818913..092c652a63 100644 --- 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 78906814db..fb644a1301 100644 --- 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 d731859cd1..2bbc4732cd 100644 --- 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 dcfc679b2c..7a6e42f4b4 100644 --- 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 -- 2.39.5