libvirt: disconnect volume from host during detach
Under certain failure scenarios it may be that although the libvirt definition for the volume has been removed for the instance that the associated storage lun on the compute server may not have been fully cleaned up yet. In case users try an other attempt to detach volume we should not stop the process whether the device is not found in domain definition but try to disconnect the logical device from host. This commit makes the process to attempt a disconnect volume even if the device is not attached to the guest. Closes-Bug: #1727260 Signed-off-by: Sahid Orentino Ferdjaoui <sahid.ferdjaoui@redhat.com> (cherry picked from commitce531dd1b7) (cherry picked from commitd6a072b5c5) Conflicts: nova/tests/unit/virt/libvirt/test_driver.py NOTE: The conflicts were due to the newer testcase mocking 'nova.virt.libvirt.host.Host._get_domain' where the older code calls 'nova.virt.libvirt.host.Host.get_domain', and also dealing with the fact that the older code doesn't pass 'encryption' to self._disconnect_volume(). This latter issue means that we need to move the call to encryptor.detach_volume() to ensure it gets called if we hit exception.DeviceNotFound when detaching the device from the guest. This is similar to the original code proposed in https://review.openstack.org/#/c/515008/9/nova/virt/libvirt/driver.py but it requires special handling for the scenario where cryptsetup tries to destroy a dm-crypt device that has already been destroyed. Signed-off-by: Chris Friesen <chris.friesen@windriver.com> Change-Id: I4182642aab3fd2ffb1c97d2de9bdca58982289d8
This commit is contained in:
Sahid Orentino Ferdjaoui
Matt Riedemann
parent
56350b977e
commit
92bd7ea118
|
|
@ -6819,8 +6819,10 @@ class LibvirtConnTestCase(test.NoDBTestCase,
|
|||
mock_disconnect_volume.assert_called_with(
|
||||
connection_info, 'vdc', instance)
|
||||
|
||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
|
||||
@mock.patch('nova.virt.libvirt.host.Host.get_domain')
|
||||
def test_detach_volume_disk_not_found(self, mock_get_domain):
|
||||
def test_detach_volume_disk_not_found(self, mock_get_domain,
|
||||
mock_disconnect_volume):
|
||||
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
|
||||
instance = objects.Instance(**self.test_instance)
|
||||
mock_xml_without_disk = """<domain>
|
||||
|
|
@ -6836,10 +6838,115 @@ class LibvirtConnTestCase(test.NoDBTestCase,
|
|||
mock_dom.info.return_value = [power_state.RUNNING, 512, 512, 2, 1234,
|
||||
5678]
|
||||
mock_get_domain.return_value = mock_dom
|
||||
self.assertRaises(exception.DiskNotFound, drvr.detach_volume,
|
||||
connection_info, instance, '/dev/vdc')
|
||||
|
||||
drvr.detach_volume(connection_info, instance, '/dev/vdc')
|
||||
|
||||
mock_get_domain.assert_called_once_with(instance)
|
||||
mock_disconnect_volume.assert_called_once_with(
|
||||
connection_info, 'vdc', instance)
|
||||
|
||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor')
|
||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
|
||||
@mock.patch('nova.virt.libvirt.host.Host.get_domain')
|
||||
def test_detach_volume_disk_not_found_encryption(self, mock_get_domain,
|
||||
mock_disconnect_volume,
|
||||
mock_get_encryptor):
|
||||
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
|
||||
instance = objects.Instance(**self.test_instance)
|
||||
mock_xml_without_disk = """<domain>
|
||||
<devices>
|
||||
</devices>
|
||||
</domain>"""
|
||||
mock_dom = mock.MagicMock(return_value=mock_xml_without_disk)
|
||||
encryption = {"provider": "NoOpEncryptor"}
|
||||
mock_encryptor = mock.MagicMock(spec=encryptors.nop.NoOpEncryptor)
|
||||
mock_get_encryptor.return_value = mock_encryptor
|
||||
|
||||
connection_info = {"driver_volume_type": "fake",
|
||||
"data": {"device_path": "/fake",
|
||||
"access_mode": "rw"}}
|
||||
|
||||
mock_dom.info.return_value = [power_state.RUNNING, 512, 512, 2, 1234,
|
||||
5678]
|
||||
mock_get_domain.return_value = mock_dom
|
||||
|
||||
drvr.detach_volume(connection_info, instance, '/dev/vdc',
|
||||
encryption)
|
||||
mock_get_encryptor.assert_called_once_with(connection_info, encryption)
|
||||
mock_encryptor.detach_volume.assert_called_once_with(**encryption)
|
||||
mock_disconnect_volume.assert_called_once_with(
|
||||
connection_info, 'vdc', instance)
|
||||
|
||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor')
|
||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
|
||||
@mock.patch('nova.virt.libvirt.host.Host.get_domain')
|
||||
def test_detach_volume_disk_not_found_encryption_err(self, mock_get_domain,
|
||||
mock_disconnect_volume,
|
||||
mock_get_encryptor):
|
||||
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
|
||||
instance = objects.Instance(**self.test_instance)
|
||||
mock_xml_without_disk = """<domain>
|
||||
<devices>
|
||||
</devices>
|
||||
</domain>"""
|
||||
mock_dom = mock.MagicMock(return_value=mock_xml_without_disk)
|
||||
encryption = {"provider": "NoOpEncryptor"}
|
||||
mock_encryptor = mock.MagicMock(spec=encryptors.nop.NoOpEncryptor)
|
||||
mock_encryptor.detach_volume = mock.MagicMock(
|
||||
side_effect=processutils.ProcessExecutionError(exit_code=4)
|
||||
)
|
||||
mock_get_encryptor.return_value = mock_encryptor
|
||||
|
||||
connection_info = {"driver_volume_type": "fake",
|
||||
"data": {"device_path": "/fake",
|
||||
"access_mode": "rw"}}
|
||||
|
||||
mock_dom.info.return_value = [power_state.RUNNING, 512, 512, 2, 1234,
|
||||
5678]
|
||||
mock_get_domain.return_value = mock_dom
|
||||
|
||||
drvr.detach_volume(connection_info, instance, '/dev/vdc',
|
||||
encryption)
|
||||
mock_get_encryptor.assert_called_once_with(connection_info, encryption)
|
||||
mock_encryptor.detach_volume.assert_called_once_with(**encryption)
|
||||
mock_disconnect_volume.assert_called_once_with(
|
||||
connection_info, 'vdc', instance)
|
||||
|
||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor')
|
||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
|
||||
@mock.patch('nova.virt.libvirt.host.Host.get_domain')
|
||||
def test_detach_volume_disk_not_found_encryption_err_reraise(
|
||||
self, mock_get_domain, mock_disconnect_volume,
|
||||
mock_get_encryptor):
|
||||
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
|
||||
instance = objects.Instance(**self.test_instance)
|
||||
mock_xml_without_disk = """<domain>
|
||||
<devices>
|
||||
</devices>
|
||||
</domain>"""
|
||||
mock_dom = mock.MagicMock(return_value=mock_xml_without_disk)
|
||||
encryption = {"provider": "NoOpEncryptor"}
|
||||
mock_encryptor = mock.MagicMock(spec=encryptors.nop.NoOpEncryptor)
|
||||
# Any nonzero exit code other than 4 would work here.
|
||||
mock_encryptor.detach_volume = mock.MagicMock(
|
||||
side_effect=processutils.ProcessExecutionError(exit_code=1)
|
||||
)
|
||||
mock_get_encryptor.return_value = mock_encryptor
|
||||
|
||||
connection_info = {"driver_volume_type": "fake",
|
||||
"data": {"device_path": "/fake",
|
||||
"access_mode": "rw"}}
|
||||
|
||||
mock_dom.info.return_value = [power_state.RUNNING, 512, 512, 2, 1234,
|
||||
5678]
|
||||
mock_get_domain.return_value = mock_dom
|
||||
|
||||
self.assertRaises(processutils.ProcessExecutionError,
|
||||
drvr.detach_volume, connection_info, instance,
|
||||
'/dev/vdc', encryption)
|
||||
mock_get_encryptor.assert_called_once_with(connection_info, encryption)
|
||||
mock_encryptor.detach_volume.assert_called_once_with(**encryption)
|
||||
mock_disconnect_volume.assert_not_called()
|
||||
|
||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
|
||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor')
|
||||
|
|
|
|||
|
|
@ -1422,11 +1422,6 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
live=live)
|
||||
wait_for_detach()
|
||||
|
||||
if encryption:
|
||||
encryptor = self._get_volume_encryptor(connection_info,
|
||||
encryption)
|
||||
encryptor.detach_volume(**encryption)
|
||||
|
||||
except exception.InstanceNotFound:
|
||||
# NOTE(zhaoqin): If the instance does not exist, _lookup_by_name()
|
||||
# will throw InstanceNotFound exception. Need to
|
||||
|
|
@ -1434,7 +1429,11 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
LOG.warning("During detach_volume, instance disappeared.",
|
||||
instance=instance)
|
||||
except exception.DeviceNotFound:
|
||||
raise exception.DiskNotFound(location=disk_dev)
|
||||
# We should still try to disconnect logical device from
|
||||
# host, an error might have happened during a previous
|
||||
# call.
|
||||
LOG.info("Device %s not found in instance.",
|
||||
disk_dev, instance=instance)
|
||||
except libvirt.libvirtError as ex:
|
||||
# NOTE(vish): This is called to cleanup volumes after live
|
||||
# migration, so we should still disconnect even if
|
||||
|
|
@ -1447,6 +1446,26 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
else:
|
||||
raise
|
||||
|
||||
try:
|
||||
if encryption:
|
||||
encryptor = self._get_volume_encryptor(connection_info,
|
||||
encryption)
|
||||
encryptor.detach_volume(**encryption)
|
||||
except processutils.ProcessExecutionError as e:
|
||||
# cryptsetup returns 4 when attempting to destroy a non-existent
|
||||
# dm-crypt device. We assume here that the caller hasn't specified
|
||||
# the wrong device, and that it doesn't exist because it has
|
||||
# already been destroyed.
|
||||
if e.exit_code == 4:
|
||||
LOG.debug("Ignoring exit code 4, volume already destroyed")
|
||||
else:
|
||||
with excutils.save_and_reraise_exception():
|
||||
LOG.warning("Could not disconnect encrypted volume "
|
||||
"%(volume)s. If dm-crypt device is still "
|
||||
"active it will have to be destroyed manually "
|
||||
"for cleanup to succeed.",
|
||||
{'volume': disk_dev})
|
||||
|
||||
self._disconnect_volume(connection_info, disk_dev, instance)
|
||||
|
||||
def extend_volume(self, connection_info, instance):
|
||||
|
|
|
|||
Loading…
Reference in New Issue