From ebabd029aca0d1f4694762c7ac2671cb7d9c062d Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Wed, 24 Oct 2012 16:14:36 -0700 Subject: [PATCH] libvirt: persist volume attachments into config When you attach a volume to a running instance, only the running xml is updated in libvirt. This is due to the usage of the VIRT_DOMAIN_AFFECT_CURRENT flag passed to attach device. What we really want is to affect both the config and the running xml. There is no combination of flags that works in all states, so we explicitly set the right combination of flags depending on the running state of the vm. Includes a test to test_virt_drivers to ensure the flags are passed correctly. Fixes bug 1071069 Change-Id: I1992748b08daf99cf03dfeb0937ad42457fff1a3 (cherry picked from commit 814e7f8c827aa21a87a62594d5b6413bb49e31e1) --- nova/tests/fakelibvirt.py | 7 ++++++- nova/tests/test_virt_drivers.py | 12 ++++++++++++ nova/virt/libvirt/driver.py | 16 ++++++++++++++-- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/nova/tests/fakelibvirt.py b/nova/tests/fakelibvirt.py index b933b004ab1..7c9d5b23808 100644 --- a/nova/tests/fakelibvirt.py +++ b/nova/tests/fakelibvirt.py @@ -73,6 +73,8 @@ def _reset(): VIR_DOMAIN_UNDEFINE_MANAGED_SAVE = 1 VIR_DOMAIN_AFFECT_CURRENT = 0 +VIR_DOMAIN_AFFECT_LIVE = 1 +VIR_DOMAIN_AFFECT_CONFIG = 2 VIR_CPU_COMPARE_ERROR = -1 VIR_CPU_COMPARE_INCOMPATIBLE = 0 @@ -337,7 +339,10 @@ def attachDevice(self, xml): self._def['devices']['disks'] += [disk_info] return True - def attachDeviceFlags(self, xml, _flags): + def attachDeviceFlags(self, xml, flags): + if (flags & VIR_DOMAIN_AFFECT_LIVE and + self._state != VIR_DOMAIN_RUNNING): + raise libvirtError("AFFECT_LIVE only allowed for running domains!") self.attachDevice(xml) def detachDevice(self, xml): diff --git a/nova/tests/test_virt_drivers.py b/nova/tests/test_virt_drivers.py index 18b6bdc7dd9..e79ab44441c 100644 --- a/nova/tests/test_virt_drivers.py +++ b/nova/tests/test_virt_drivers.py @@ -389,6 +389,18 @@ def test_attach_detach_volume(self): instance_ref['name'], '/mnt/nova/something') + @catch_notimplementederror + def test_attach_detach_different_power_states(self): + instance_ref, network_info = self._get_running_instance() + self.connection.power_off(instance_ref) + self.connection.attach_volume({'driver_volume_type': 'fake'}, + instance_ref['name'], + '/mnt/nova/something') + self.connection.power_on(instance_ref) + self.connection.detach_volume({'driver_volume_type': 'fake'}, + instance_ref['name'], + '/mnt/nova/something') + @catch_notimplementederror def test_get_info(self): instance_ref, network_info = self._get_running_instance() diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index dc02a413901..0329acca143 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -648,7 +648,13 @@ def attach_volume(self, connection_info, instance_name, mountpoint): self._conn.defineXML(domxml) else: try: - flags = (libvirt.VIR_DOMAIN_AFFECT_CURRENT) + # NOTE(vish): We can always affect config because our + # domains are persistent, but we should only + # affect live if the domain is running. + flags = libvirt.VIR_DOMAIN_AFFECT_CONFIG + state = LIBVIRT_POWER_STATE[virt_dom.info()[0]] + if state == power_state.RUNNING: + flags |= libvirt.VIR_DOMAIN_AFFECT_LIVE virt_dom.attachDeviceFlags(conf.to_xml(), flags) except Exception, ex: if isinstance(ex, libvirt.libvirtError): @@ -709,7 +715,13 @@ def detach_volume(self, connection_info, instance_name, mountpoint): domxml = virt_dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE) self._conn.defineXML(domxml) else: - flags = (libvirt.VIR_DOMAIN_AFFECT_CURRENT) + # NOTE(vish): We can always affect config because our + # domains are persistent, but we should only + # affect live if the domain is running. + flags = libvirt.VIR_DOMAIN_AFFECT_CONFIG + state = LIBVIRT_POWER_STATE[virt_dom.info()[0]] + if state == power_state.RUNNING: + flags |= libvirt.VIR_DOMAIN_AFFECT_LIVE virt_dom.detachDeviceFlags(xml, flags) finally: self.volume_driver_method('disconnect_volume',