From 49ba5a763f9d2c7c496f391ca8149a18541bfec7 Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Thu, 22 Jul 2021 09:05:51 +0100 Subject: [PATCH] libvirt: Handle silent failures to extend volume within os-brick As seen in bug #1849425 os-brick can fail to extend an underlying volume device on the compute silently, returning a new_size of None to n-cpu. While this should ultimatley be addressed in os-brick n-cpu can also handle this before we eventually run into a type error when attemting floor division later in the volume extend flow. Change-Id: Ic8091537274a5ad27fb5af8939f81ed154b7ad7c Closes-Bug: #1849425 --- nova/exception.py | 5 + nova/tests/unit/virt/libvirt/test_driver.py | 120 +++++++++++++++----- nova/virt/libvirt/driver.py | 15 ++- 3 files changed, 108 insertions(+), 32 deletions(-) diff --git a/nova/exception.py b/nova/exception.py index f5e393e5a6d5..ef232af69a43 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -272,6 +272,11 @@ class VolumeDetachFailed(Invalid): "Reason: %(reason)s") +class VolumeExtendFailed(Invalid): + msg_fmt = _("Volume %(volume_id)s could not be extended. " + "Reason: %(reason)s") + + class MultiattachNotSupportedByVirtDriver(NovaException): # This exception indicates the compute hosting the instance does not # support multiattach volumes. This should generally be considered a diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 4062d96604a4..03eea0555fd3 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -9532,8 +9532,11 @@ class LibvirtConnTestCase(test.NoDBTestCase, instance = objects.Instance(**self.test_instance) connection_info = { 'driver_volume_type': 'fake', - 'data': {'device_path': '/fake', - 'access_mode': 'rw'} + 'data': { + 'volume_id': uuids.volume_id, + 'device_path': '/fake', + 'access_mode': 'rw' + } } new_size = 20 * units.Gi @@ -9561,21 +9564,55 @@ class LibvirtConnTestCase(test.NoDBTestCase, def test_extend_volume_with_volume_driver_without_support(self): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) instance = objects.Instance(**self.test_instance) + connection_info = { + 'driver_volume_type': 'fake', + 'serial': uuids.volume_id, + 'data': { + 'volume_id': uuids.volume_id, + 'device_path': '/fake', + 'access_mode': 'rw' + } + } with mock.patch.object(drvr, '_extend_volume', side_effect=NotImplementedError()): - connection_info = {'driver_volume_type': 'fake'} self.assertRaises(exception.ExtendVolumeNotSupported, drvr.extend_volume, self.context, connection_info, instance, 0) + def test_extend_volume_with_new_size_none(self): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + instance = objects.Instance(**self.test_instance) + connection_info = { + 'driver_volume_type': 'fake', + 'serial': uuids.volume_id, + 'data': { + 'volume_id': uuids.volume_id, + 'device_path': '/fake', + 'access_mode': 'rw' + } + } + with mock.patch.object(drvr, '_extend_volume', return_value=None): + self.assertRaises( + exception.VolumeExtendFailed, + drvr.extend_volume, + self.context, + connection_info, + instance, + 0 + ) + def test_extend_volume_disk_not_found(self): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) instance = objects.Instance(**self.test_instance) connection_info = { 'driver_volume_type': 'fake', - 'data': {'device_path': '/fake', - 'access_mode': 'rw'} + 'serial': uuids.volume_id, + 'data': { + 'volume_id': uuids.volume_id, + 'device_path': '/fake', + 'access_mode': 'rw' + } } new_size = 20 * units.Gi @@ -9590,6 +9627,15 @@ class LibvirtConnTestCase(test.NoDBTestCase, def test_extend_volume_with_instance_not_found(self): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) instance = objects.Instance(**self.test_instance) + connection_info = { + 'driver_volume_type': 'fake', + 'serial': uuids.volume_id, + 'data': { + 'volume_id': uuids.volume_id, + 'device_path': '/fake', + 'access_mode': 'rw' + } + } with test.nested( mock.patch.object(host.Host, '_get_domain', @@ -9597,7 +9643,6 @@ class LibvirtConnTestCase(test.NoDBTestCase, instance_id=instance.uuid)), mock.patch.object(drvr, '_extend_volume') ) as (_get_domain, _extend_volume): - connection_info = {'driver_volume_type': 'fake'} self.assertRaises(exception.InstanceNotFound, drvr.extend_volume, self.context, connection_info, instance, 0) @@ -9607,9 +9652,14 @@ class LibvirtConnTestCase(test.NoDBTestCase, instance = objects.Instance(**self.test_instance) connection_info = { 'driver_volume_type': 'fake', - 'data': {'device_path': '/fake', - 'access_mode': 'rw'} + 'serial': uuids.volume_id, + 'data': { + 'volume_id': uuids.volume_id, + 'device_path': '/fake', + 'access_mode': 'rw' + } } + new_size = 20 * units.Gi guest = mock.Mock(spec=libvirt_guest.Guest) @@ -9631,12 +9681,14 @@ class LibvirtConnTestCase(test.NoDBTestCase, drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) instance = objects.Instance(**self.test_instance) connection_info = { - 'serial': '58a84f6d-3f0c-4e19-a0af-eb657b790657', + 'serial': uuids.volume_id, 'driver_volume_type': 'fake', - 'data': {'cluster_name': 'fake', - 'auth_enabled': False, - 'volume_id': '58a84f6d-3f0c-4e19-a0af-eb657b790657', - 'access_mode': 'rw'} + 'data': { + 'cluster_name': 'fake', + 'auth_enabled': False, + 'volume_id': uuids.volume_id, + 'access_mode': 'rw' + } } new_size = 20 * units.Gi @@ -9648,7 +9700,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, block_device.resize = mock.Mock() disk = mock.Mock( spec=vconfig.LibvirtConfigGuestDisk, - serial='58a84f6d-3f0c-4e19-a0af-eb657b790657', + serial=uuids.volume_id, target_dev='vdb') guest.get_block_device = mock.Mock(return_value=block_device) guest.get_all_disks = mock.Mock(return_value=[disk]) @@ -9668,12 +9720,14 @@ class LibvirtConnTestCase(test.NoDBTestCase, drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) instance = objects.Instance(**self.test_instance) connection_info = { - 'serial': '58a84f6d-3f0c-4e19-a0af-eb657b790657', + 'serial': uuids.volume_id, 'driver_volume_type': 'fake', - 'data': {'cluster_name': 'fake', - 'auth_enabled': False, - 'volume_id': '58a84f6d-3f0c-4e19-a0af-eb657b790657', - 'access_mode': 'rw'} + 'data': { + 'cluster_name': 'fake', + 'auth_enabled': False, + 'volume_id': uuids.volume_id, + 'access_mode': 'rw' + } } new_size = 20 * units.Gi @@ -9684,7 +9738,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, block_device.resize = mock.Mock() disk = mock.Mock( spec=vconfig.LibvirtConfigGuestDisk, - serial='12345678-abcd-abcd-abcd-0123456789012', + serial=uuids.different_volume_id, target_dev='vdb') guest.get_block_device = mock.Mock(return_value=block_device) guest.get_all_disks = mock.Mock(return_value=[disk]) @@ -9709,7 +9763,9 @@ class LibvirtConnTestCase(test.NoDBTestCase, connection_info = { 'serial': uuids.volume_id, 'driver_volume_type': 'fake', - 'data': {'access_mode': 'rw'} + 'data': { + 'access_mode': 'rw' + } } disk_1 = mock.Mock(spec=vconfig.LibvirtConfigGuestDisk, @@ -9748,8 +9804,10 @@ class LibvirtConnTestCase(test.NoDBTestCase, connection_info = { 'serial': uuids.volume_id, 'driver_volume_type': 'fake', - 'data': {'device_path': mock.sentinel.device_path, - 'access_mode': 'rw'} + 'data': { + 'device_path': mock.sentinel.device_path, + 'access_mode': 'rw' + } } block_device = mock.Mock(spec=libvirt_guest.BlockDevice, @@ -9786,8 +9844,10 @@ class LibvirtConnTestCase(test.NoDBTestCase, connection_info = { 'serial': uuids.volume_id, 'driver_volume_type': 'fake', - 'data': {'device_path': mock.sentinel.device_path, - 'access_mode': 'rw'} + 'data': { + 'device_path': mock.sentinel.device_path, + 'access_mode': 'rw' + } } block_device = mock.Mock(spec=libvirt_guest.BlockDevice, @@ -9837,10 +9897,12 @@ class LibvirtConnTestCase(test.NoDBTestCase, connection_info = { 'serial': uuids.volume_id, 'driver_volume_type': 'rbd', - 'data': {'name': 'pool/volume', - 'auth_enabled': 'true', - 'auth_username': 'username', - 'access_mode': 'rw'} + 'data': { + 'name': 'pool/volume', + 'auth_enabled': 'true', + 'auth_username': 'username', + 'access_mode': 'rw' + } } disk_1 = mock.Mock(spec=vconfig.LibvirtConfigGuestDisk, serial=uuids.volume_id, diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 65f4e44f2731..6b31747b7c1e 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -2666,9 +2666,19 @@ class LibvirtDriver(driver.ComputeDriver): def extend_volume(self, context, connection_info, instance, requested_size): + volume_id = driver_block_device.get_volume_id(connection_info) try: - new_size = self._extend_volume(connection_info, instance, - requested_size) + new_size = self._extend_volume( + connection_info, instance, requested_size) + + # NOTE(lyarwood): Handle cases where os-brick has ignored failures + # and returned an invalid new_size of None through the vol drivers + if new_size is None: + raise exception.VolumeExtendFailed( + volume_id=volume_id, + reason="Failure to resize underlying volume on compute." + ) + except NotImplementedError: raise exception.ExtendVolumeNotSupported() @@ -2677,7 +2687,6 @@ class LibvirtDriver(driver.ComputeDriver): try: guest = self._host.get_guest(instance) state = guest.get_power_state(self._host) - volume_id = driver_block_device.get_volume_id(connection_info) active_state = state in (power_state.RUNNING, power_state.PAUSED) if active_state: if 'device_path' in connection_info['data']: