Support detach interface with same MAC from instance
When detach_interface nova uses <interface> XML generated
from scatch, which is missing the PCI device information that
libvirt would use to uniquely identify devices.
In case instance has mutiple interfaces with same MAC address.
Libvirt will failed with below error message:
libvirtError: operation failed: multiple devices matching
mac address fa:16:3e:60:46:1f found
This patch fixes this problem by provide a new function
get_interface_by_cfg, this function uses cfg generated by
nova.virt.libvirt.vif.get_config as parameter,
return a LibvirtConfigGuestInterface object.
Also added function format_dom for
LibvirtConfigGuestDeviceAddressPCI, which will be used to
generate pci address for LibvirtConfigGuestInterface.
Change-Id: I8acae90c9d2111ed35f58f374f321d64f01ba563
Closes-Bug: #1621076
This commit is contained in:
Leehom Li (feli5)
Matt Riedemann
parent
8b9f9cc8e2
commit
a5c38cc861
|
|
@ -16424,7 +16424,19 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
|
|||
expected_flags):
|
||||
instance = self._create_instance()
|
||||
network_info = _fake_network_info(self, 1)
|
||||
domain = FakeVirtDomain()
|
||||
domain = FakeVirtDomain(fake_xml="""
|
||||
<domain type='kvm'>
|
||||
<devices>
|
||||
<interface type='bridge'>
|
||||
<mac address='52:54:00:f6:35:8f'/>
|
||||
<model type='virtio'/>
|
||||
<source bridge='br0'/>
|
||||
<target dev='tap12345678'/>
|
||||
<address type='pci' domain='0x0000' bus='0x00'
|
||||
slot='0x03' function='0x0'/>
|
||||
</interface>
|
||||
</devices>
|
||||
</domain>""")
|
||||
self.mox.StubOutWithMock(host.Host, 'get_domain')
|
||||
self.mox.StubOutWithMock(self.drvr.firewall_driver,
|
||||
'setup_basic_filtering')
|
||||
|
|
@ -16432,30 +16444,51 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
|
|||
self.mox.StubOutWithMock(domain, 'info')
|
||||
|
||||
host.Host.get_domain(instance).AndReturn(domain)
|
||||
domain.info().AndReturn([power_state, 1, 2, 3, 4])
|
||||
|
||||
if method == 'attach_interface':
|
||||
self.drvr.firewall_driver.setup_basic_filtering(
|
||||
instance, [network_info[0]])
|
||||
|
||||
fake_image_meta = objects.ImageMeta.from_dict(
|
||||
{'id': instance.image_ref})
|
||||
|
||||
expected = self.drvr.vif_driver.get_config(
|
||||
instance, network_info[0], fake_image_meta, instance.flavor,
|
||||
CONF.libvirt.virt_type, self.drvr._host)
|
||||
|
||||
self.mox.StubOutWithMock(self.drvr.vif_driver,
|
||||
'get_config')
|
||||
self.drvr.vif_driver.get_config(
|
||||
instance, network_info[0],
|
||||
mox.IsA(objects.ImageMeta),
|
||||
mox.IsA(objects.Flavor),
|
||||
CONF.libvirt.virt_type,
|
||||
self.drvr._host).AndReturn(expected)
|
||||
domain.info().AndReturn([power_state, 1, 2, 3, 4])
|
||||
if method == 'attach_interface':
|
||||
fake_image_meta = objects.ImageMeta.from_dict(
|
||||
{'id': instance.image_ref})
|
||||
expected = self.drvr.vif_driver.get_config(
|
||||
instance, network_info[0], fake_image_meta, instance.flavor,
|
||||
CONF.libvirt.virt_type, self.drvr._host)
|
||||
self.mox.StubOutWithMock(self.drvr.vif_driver,
|
||||
'get_config')
|
||||
self.drvr.vif_driver.get_config(
|
||||
instance, network_info[0],
|
||||
mox.IsA(objects.ImageMeta),
|
||||
mox.IsA(objects.Flavor),
|
||||
CONF.libvirt.virt_type,
|
||||
self.drvr._host).AndReturn(expected)
|
||||
domain.attachDeviceFlags(expected.to_xml(), flags=expected_flags)
|
||||
elif method == 'detach_interface':
|
||||
domain.detachDeviceFlags(expected.to_xml(), expected_flags)
|
||||
expected = vconfig.LibvirtConfigGuestInterface()
|
||||
expected.parse_str("""
|
||||
<interface type='bridge'>
|
||||
<mac address='52:54:00:f6:35:8f'/>
|
||||
<model type='virtio'/>
|
||||
<source bridge='br0'/>
|
||||
<target dev='tap12345678'/>
|
||||
</interface>""")
|
||||
self.mox.StubOutWithMock(self.drvr.vif_driver,
|
||||
'get_config')
|
||||
self.drvr.vif_driver.get_config(
|
||||
instance, network_info[0],
|
||||
mox.IsA(objects.ImageMeta),
|
||||
mox.IsA(objects.Flavor),
|
||||
CONF.libvirt.virt_type,
|
||||
self.drvr._host).AndReturn(expected)
|
||||
domain.detachDeviceFlags("""
|
||||
<interface type='bridge'>
|
||||
<mac address='52:54:00:f6:35:8f'/>
|
||||
<model type='virtio'/>
|
||||
<source bridge='br0'/>
|
||||
<target dev='tap12345678'/>
|
||||
<address type='pci' domain='0x0000' bus='0x00'
|
||||
slot='0x03' function='0x0'/>
|
||||
</interface>""", expected_flags)
|
||||
|
||||
self.mox.ReplayAll()
|
||||
if method == 'attach_interface':
|
||||
|
|
@ -16509,21 +16542,81 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
|
|||
guest = mock.Mock(spec='nova.virt.libvirt.guest.Guest')
|
||||
guest.get_power_state = mock.Mock()
|
||||
self.drvr._host.get_guest = mock.Mock(return_value=guest)
|
||||
self.drvr.vif_driver = mock.Mock()
|
||||
error = fakelibvirt.libvirtError(
|
||||
'no matching network device was found')
|
||||
error.err = (fakelibvirt.VIR_ERR_OPERATION_FAILED,)
|
||||
guest.detach_device = mock.Mock(side_effect=error)
|
||||
# mock out that get_interface_by_mac doesn't find the interface
|
||||
guest.get_interface_by_mac = mock.Mock(return_value=None)
|
||||
# mock out that get_interface_by_cfg doesn't find the interface
|
||||
guest.get_interface_by_cfg = mock.Mock(return_value=None)
|
||||
self.drvr.detach_interface(self.context, instance, vif)
|
||||
guest.get_interface_by_mac.assert_called_once_with(vif['address'])
|
||||
# an error shouldn't be logged, but a warning should be logged
|
||||
self.assertFalse(mock_log.error.called)
|
||||
self.assertEqual(1, mock_log.warning.call_count)
|
||||
self.assertIn('the device is no longer found on the guest',
|
||||
six.text_type(mock_log.warning.call_args[0]))
|
||||
|
||||
def test_detach_interface_device_with_same_mac_address(self):
|
||||
instance = self._create_instance()
|
||||
network_info = _fake_network_info(self, 1)
|
||||
domain = FakeVirtDomain(fake_xml="""
|
||||
<domain type='kvm'>
|
||||
<devices>
|
||||
<interface type='bridge'>
|
||||
<mac address='52:54:00:f6:35:8f'/>
|
||||
<model type='virtio'/>
|
||||
<source bridge='br0'/>
|
||||
<target dev='tap12345678'/>
|
||||
<address type='pci' domain='0x0000' bus='0x00'
|
||||
slot='0x03' function='0x0'/>
|
||||
</interface>
|
||||
<interface type='bridge'>
|
||||
<mac address='52:54:00:f6:35:8f'/>
|
||||
<model type='virtio'/>
|
||||
<source bridge='br1'/>
|
||||
<target dev='tap87654321'/>
|
||||
<address type='pci' domain='0x0000' bus='0x00'
|
||||
slot='0x03' function='0x1'/>
|
||||
</interface>
|
||||
</devices>
|
||||
</domain>""")
|
||||
self.mox.StubOutWithMock(host.Host, 'get_domain')
|
||||
self.mox.StubOutWithMock(self.drvr.firewall_driver,
|
||||
'setup_basic_filtering')
|
||||
self.mox.StubOutWithMock(domain, 'attachDeviceFlags')
|
||||
self.mox.StubOutWithMock(domain, 'info')
|
||||
|
||||
host.Host.get_domain(instance).AndReturn(domain)
|
||||
domain.info().AndReturn([power_state.RUNNING, 1, 2, 3, 4])
|
||||
expected = vconfig.LibvirtConfigGuestInterface()
|
||||
expected.parse_str("""
|
||||
<interface type='bridge'>
|
||||
<mac address='52:54:00:f6:35:8f'/>
|
||||
<model type='virtio'/>
|
||||
<source bridge='br0'/>
|
||||
<target dev='tap12345678'/>
|
||||
</interface>""")
|
||||
self.mox.StubOutWithMock(self.drvr.vif_driver, 'get_config')
|
||||
self.drvr.vif_driver.get_config(
|
||||
instance, network_info[0],
|
||||
mox.IsA(objects.ImageMeta),
|
||||
mox.IsA(objects.Flavor),
|
||||
CONF.libvirt.virt_type,
|
||||
self.drvr._host).AndReturn(expected)
|
||||
expected_flags = (fakelibvirt.VIR_DOMAIN_AFFECT_CONFIG |
|
||||
fakelibvirt.VIR_DOMAIN_AFFECT_LIVE)
|
||||
domain.detachDeviceFlags("""
|
||||
<interface type='bridge'>
|
||||
<mac address='52:54:00:f6:35:8f'/>
|
||||
<model type='virtio'/>
|
||||
<source bridge='br0'/>
|
||||
<target dev='tap12345678'/>
|
||||
<address type='pci' domain='0x0000' bus='0x00'
|
||||
slot='0x03' function='0x0'/>
|
||||
</interface>""", expected_flags)
|
||||
self.mox.ReplayAll()
|
||||
self.drvr.detach_interface(self.context, instance, network_info[0])
|
||||
self.mox.VerifyAll()
|
||||
|
||||
@mock.patch('nova.virt.libvirt.utils.write_to_file')
|
||||
# NOTE(mdbooth): The following 4 mocks are required to execute
|
||||
# get_guest_xml().
|
||||
|
|
|
|||
|
|
@ -433,9 +433,18 @@ class GuestTestCase(test.NoDBTestCase):
|
|||
self.assertEqual(1, len(devs))
|
||||
self.assertIsInstance(devs[0], vconfig.LibvirtConfigGuestInterface)
|
||||
|
||||
cfg = vconfig.LibvirtConfigGuestInterface()
|
||||
cfg.parse_str("""
|
||||
<interface type="bridge">
|
||||
<mac address="fa:16:3e:f9:af:ae"/>
|
||||
<model type="virtio"/>
|
||||
<driver name="qemu"/>
|
||||
<source bridge="qbr84008d03-11"/>
|
||||
<target dev="tap84008d03-11"/>
|
||||
</interface>""")
|
||||
self.assertIsNotNone(
|
||||
self.guest.get_interface_by_mac('fa:16:3e:f9:af:ae'))
|
||||
self.assertIsNone(self.guest.get_interface_by_mac(None))
|
||||
self.guest.get_interface_by_cfg(cfg))
|
||||
self.assertIsNone(self.guest.get_interface_by_cfg(None))
|
||||
|
||||
def test_get_info(self):
|
||||
self.domain.info.return_value = (1, 2, 3, 4, 5)
|
||||
|
|
|
|||
|
|
@ -1370,11 +1370,25 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
instance.image_meta,
|
||||
instance.flavor,
|
||||
CONF.libvirt.virt_type, self._host)
|
||||
interface = guest.get_interface_by_cfg(cfg)
|
||||
try:
|
||||
self.vif_driver.unplug(instance, vif)
|
||||
# NOTE(mriedem): When deleting an instance and using Neutron,
|
||||
# we can be racing against Neutron deleting the port and
|
||||
# sending the vif-deleted event which then triggers a call to
|
||||
# detach the interface, so if the interface is not found then
|
||||
# we can just log it as a warning.
|
||||
if not interface:
|
||||
mac = vif.get('address')
|
||||
# The interface is gone so just log it as a warning.
|
||||
LOG.warning(_LW('Detaching interface %(mac)s failed because '
|
||||
'the device is no longer found on the guest.'),
|
||||
{'mac': mac}, instance=instance)
|
||||
return
|
||||
|
||||
state = guest.get_power_state(self._host)
|
||||
live = state in (power_state.RUNNING, power_state.PAUSED)
|
||||
guest.detach_device(cfg, persistent=True, live=live)
|
||||
guest.detach_device(interface, persistent=True, live=live)
|
||||
except libvirt.libvirtError as ex:
|
||||
error_code = ex.get_error_code()
|
||||
if error_code == libvirt.VIR_ERR_NO_DOMAIN:
|
||||
|
|
@ -1389,11 +1403,11 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
# network device no longer exists. Libvirt will fail with
|
||||
# "operation failed: no matching network device was found"
|
||||
# which unfortunately does not have a unique error code so we
|
||||
# need to look up the interface by MAC and if it's not found
|
||||
# need to look up the interface by config and if it's not found
|
||||
# then we can just log it as a warning rather than tracing an
|
||||
# error.
|
||||
mac = vif.get('address')
|
||||
interface = guest.get_interface_by_mac(mac)
|
||||
interface = guest.get_interface_by_cfg(cfg)
|
||||
if interface:
|
||||
LOG.error(_LE('detaching network adapter failed.'),
|
||||
instance=instance, exc_info=True)
|
||||
|
|
|
|||
|
|
@ -226,20 +226,29 @@ class Guest(object):
|
|||
|
||||
return interfaces
|
||||
|
||||
def get_interface_by_mac(self, mac):
|
||||
"""Lookup a LibvirtConfigGuestInterface by the MAC address.
|
||||
def get_interface_by_cfg(self, cfg):
|
||||
"""Lookup a full LibvirtConfigGuestInterface with
|
||||
LibvirtConfigGuestInterface generated
|
||||
by nova.virt.libvirt.vif.get_config.
|
||||
|
||||
:param mac: MAC address of the guest interface.
|
||||
:type mac: str
|
||||
:param cfg: config object that represents the guest interface.
|
||||
:type cfg: LibvirtConfigGuestInterface object
|
||||
:returns: nova.virt.libvirt.config.LibvirtConfigGuestInterface instance
|
||||
if found, else None
|
||||
"""
|
||||
|
||||
if mac:
|
||||
if cfg:
|
||||
interfaces = self.get_all_devices(
|
||||
vconfig.LibvirtConfigGuestInterface)
|
||||
for interface in interfaces:
|
||||
if interface.mac_addr == mac:
|
||||
# NOTE(leehom) LibvirtConfigGuestInterface get from domain and
|
||||
# LibvirtConfigGuestInterface generated by
|
||||
# nova.virt.libvirt.vif.get_config must be identical.
|
||||
if (interface.mac_addr == cfg.mac_addr and
|
||||
interface.net_type == cfg.net_type and
|
||||
interface.source_dev == cfg.source_dev and
|
||||
interface.target_dev == cfg.target_dev and
|
||||
interface.vhostuser_path == cfg.vhostuser_path):
|
||||
return interface
|
||||
|
||||
def get_vcpus_info(self):
|
||||
|
|
|
|||
Loading…
Reference in New Issue