Fix suspend for non hostdev sriov ports
change I3a45b1fb41e8e446d1f25d7a1d77991c8bf2a1ed tried to fix bug #1563874 by using _detach_pci_device to remove hostdev pci devices however that breaks other usecase so we attempt to fix that by only calling _detach_pci_device for devices it can handle and use detach_interface for the rest. Related-bug: #1563874 Related-bug: #1970467 Change-Id: I351d58d6922ca169b641500c12ffd6f91829df90
This commit is contained in:
Sean Mooney
parent
6f1c7ab2e7
commit
51a970af37
@ -16764,15 +16764,17 @@ class LibvirtConnTestCase(test.NoDBTestCase,
|
||||
instance.system_metadata)
|
||||
self.assertTrue(mock_attachDevice.called)
|
||||
|
||||
@mock.patch.object(host.Host,
|
||||
'has_min_version', return_value=True)
|
||||
def _test_detach_direct_passthrough_ports(self,
|
||||
mock_has_min_version, vif_type):
|
||||
@mock.patch.object(
|
||||
host.Host, 'has_min_version', new=mock.Mock(return_value=True)
|
||||
)
|
||||
def _test_detach_direct_passthrough_ports(
|
||||
self, vif_type, detach_device=True,
|
||||
vnic_type=network_model.VNIC_TYPE_DIRECT):
|
||||
instance = objects.Instance(**self.test_instance)
|
||||
|
||||
expeted_pci_slot = "0000:00:00.0"
|
||||
network_info = _fake_network_info(self)
|
||||
network_info[0]['vnic_type'] = network_model.VNIC_TYPE_DIRECT
|
||||
network_info[0]['vnic_type'] = vnic_type
|
||||
# some more adjustments for the fake network_info so that
|
||||
# the correct get_config function will be executed (vif's
|
||||
# get_config_hw_veb - which is according to the real SRIOV vif)
|
||||
@ -16788,29 +16790,52 @@ class LibvirtConnTestCase(test.NoDBTestCase,
|
||||
# pci_manager.get_instance_pci_devs will not return an empty list
|
||||
# which will eventually fail the assertion for detachDeviceFlags
|
||||
expected_pci_device_obj = (
|
||||
objects.PciDevice(address=expeted_pci_slot, request_id=None))
|
||||
objects.PciDevice(
|
||||
address=expeted_pci_slot, request_id=None, compute_node_id=42
|
||||
)
|
||||
)
|
||||
instance.pci_devices = objects.PciDeviceList()
|
||||
instance.pci_devices.objects = [expected_pci_device_obj]
|
||||
|
||||
domain = FakeVirtDomain()
|
||||
domain = FakeVirtDomain(id=24601, name='Jean Valjean')
|
||||
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
|
||||
guest = libvirt_guest.Guest(domain)
|
||||
|
||||
with mock.patch.object(drvr, '_detach_pci_devices') as mock_detach_pci:
|
||||
with mock.patch.object(
|
||||
drvr, '_detach_pci_devices'
|
||||
) as mock_detach_pci, mock.patch.object(
|
||||
drvr, 'detach_interface'
|
||||
) as mock_detach_interface:
|
||||
drvr._detach_direct_passthrough_ports(
|
||||
self.context, instance, guest)
|
||||
mock_detach_pci.assert_called_once_with(
|
||||
guest, [expected_pci_device_obj])
|
||||
if detach_device:
|
||||
mock_detach_pci.assert_called_once_with(
|
||||
guest, [expected_pci_device_obj])
|
||||
else:
|
||||
mock_detach_interface.assert_called_once()
|
||||
|
||||
def test_detach_direct_passthrough_ports_interface_interface_hostdev(self):
|
||||
def test_detach_direct_passthrough_ports_ovs_hw_offload(self):
|
||||
# Note: test detach_direct_passthrough_ports method for vif with config
|
||||
# LibvirtConfigGuestInterface
|
||||
self._test_detach_direct_passthrough_ports(vif_type="hw_veb")
|
||||
self._test_detach_direct_passthrough_ports("ovs", detach_device=False)
|
||||
|
||||
def test_detach_direct_passthrough_ports_interface_pci_hostdev(self):
|
||||
def test_detach_direct_passthrough_ports_sriov_nic_agent(self):
|
||||
# Note: test detach_direct_passthrough_ports method for vif with config
|
||||
# LibvirtConfigGuestInterface
|
||||
self._test_detach_direct_passthrough_ports(
|
||||
"hw_veb", detach_device=False
|
||||
)
|
||||
|
||||
def test_detach_direct_physical_passthrough_ports_sriov_nic_agent(self):
|
||||
self._test_detach_direct_passthrough_ports(
|
||||
"hostdev_physical",
|
||||
vnic_type=network_model.VNIC_TYPE_DIRECT_PHYSICAL
|
||||
)
|
||||
|
||||
def test_detach_direct_passthrough_ports_infiniband(self):
|
||||
# Note: test detach_direct_passthrough_ports method for vif with config
|
||||
# LibvirtConfigGuestHostdevPCI
|
||||
self._test_detach_direct_passthrough_ports(vif_type="ib_hostdev")
|
||||
self._test_detach_direct_passthrough_ports("ib_hostdev")
|
||||
|
||||
@mock.patch.object(host.Host, 'has_min_version', return_value=True)
|
||||
@mock.patch.object(FakeVirtDomain, 'detachDeviceFlags')
|
||||
@ -16820,9 +16845,10 @@ class LibvirtConnTestCase(test.NoDBTestCase,
|
||||
|
||||
network_info = _fake_network_info(self, 2)
|
||||
|
||||
direct_physical = network_model.VNIC_TYPE_DIRECT_PHYSICAL
|
||||
for network_info_inst in network_info:
|
||||
network_info_inst['vnic_type'] = network_model.VNIC_TYPE_DIRECT
|
||||
network_info_inst['type'] = "hw_veb"
|
||||
network_info_inst['vnic_type'] = direct_physical
|
||||
network_info_inst['type'] = "hostdev_physical"
|
||||
network_info_inst['details'] = dict(vlan="2145")
|
||||
network_info_inst['address'] = "fa:16:3e:96:2a:48"
|
||||
|
||||
|
||||
@ -4982,16 +4982,18 @@ class LibvirtDriver(driver.ComputeDriver):
|
||||
guest_config = vconfig.LibvirtConfigGuest()
|
||||
guest_config.parse_dom(xml_doc)
|
||||
|
||||
for hdev in [d for d in guest_config.devices
|
||||
if isinstance(d, vconfig.LibvirtConfigGuestHostdevPCI)]:
|
||||
for hdev in [
|
||||
d for d in guest_config.devices
|
||||
if isinstance(d, vconfig.LibvirtConfigGuestHostdevPCI)
|
||||
]:
|
||||
hdbsf = [hdev.domain, hdev.bus, hdev.slot, hdev.function]
|
||||
dbsf = pci_utils.parse_address(dev.address)
|
||||
if [int(x, 16) for x in hdbsf] ==\
|
||||
[int(x, 16) for x in dbsf]:
|
||||
raise exception.PciDeviceDetachFailed(reason=
|
||||
"timeout",
|
||||
dev=dev)
|
||||
|
||||
if (
|
||||
[int(x, 16) for x in hdbsf] ==
|
||||
[int(x, 16) for x in dbsf]
|
||||
):
|
||||
raise exception.PciDeviceDetachFailed(
|
||||
reason="timeout", dev=dev)
|
||||
except libvirt.libvirtError as ex:
|
||||
error_code = ex.get_error_code()
|
||||
if error_code == libvirt.VIR_ERR_NO_DOMAIN:
|
||||
@ -5039,33 +5041,76 @@ class LibvirtDriver(driver.ComputeDriver):
|
||||
instance=instance)
|
||||
guest.attach_device(cfg)
|
||||
|
||||
# TODO(sean-k-mooney): we should try and converge this fuction with
|
||||
# _detach_direct_passthrough_vifs which does the same operation correctly
|
||||
# for live migration
|
||||
def _detach_direct_passthrough_ports(self, context, instance, guest):
|
||||
network_info = instance.info_cache.network_info
|
||||
if network_info is None:
|
||||
return
|
||||
|
||||
if self._has_direct_passthrough_port(network_info):
|
||||
# In case of VNIC_TYPES_DIRECT_PASSTHROUGH ports we create
|
||||
# pci request per direct passthrough port. Therefore we can trust
|
||||
# that pci_slot value in the vif is correct.
|
||||
direct_passthrough_pci_addresses = [
|
||||
|
||||
attached_via_hostdev_element = []
|
||||
attached_via_interface_element = []
|
||||
|
||||
for vif in network_info:
|
||||
if vif['profile'].get('pci_slot') is None:
|
||||
# this is not an sriov interface so skip it
|
||||
continue
|
||||
|
||||
if (vif['vnic_type'] not in
|
||||
network_model.VNIC_TYPES_DIRECT_PASSTHROUGH):
|
||||
continue
|
||||
|
||||
cfg = self.vif_driver.get_config(
|
||||
instance, vif, instance.image_meta, instance.flavor,
|
||||
CONF.libvirt.virt_type)
|
||||
LOG.debug(f'Detaching type: {type(cfg)}, data: {cfg}')
|
||||
if isinstance(cfg, vconfig.LibvirtConfigGuestHostdevPCI):
|
||||
attached_via_hostdev_element.append(vif)
|
||||
else:
|
||||
attached_via_interface_element.append(vif)
|
||||
|
||||
pci_devs = pci_manager.get_instance_pci_devs(instance, 'all')
|
||||
hostdev_pci_addresses = {
|
||||
vif['profile']['pci_slot']
|
||||
for vif in network_info
|
||||
if (vif['vnic_type'] in
|
||||
network_model.VNIC_TYPES_DIRECT_PASSTHROUGH and
|
||||
vif['profile'].get('pci_slot') is not None)
|
||||
for vif in attached_via_hostdev_element
|
||||
}
|
||||
direct_passthrough_pci_addresses = [
|
||||
pci_dev for pci_dev in pci_devs
|
||||
if pci_dev.address in hostdev_pci_addresses
|
||||
]
|
||||
|
||||
# use detach_pci_devices to avoid failure in case of
|
||||
# multiple guest direct passthrough ports with the same MAC
|
||||
# (protection use-case, ports are on different physical
|
||||
# interfaces)
|
||||
pci_devs = pci_manager.get_instance_pci_devs(instance, 'all')
|
||||
direct_passthrough_pci_addresses = (
|
||||
[pci_dev for pci_dev in pci_devs
|
||||
if pci_dev.address in direct_passthrough_pci_addresses])
|
||||
# FIXME(sean-k-mooney): i am using _detach_pci_devices because
|
||||
# of the previous comment introduced by change-id:
|
||||
# I3a45b1fb41e8e446d1f25d7a1d77991c8bf2a1ed
|
||||
# in relation to bug 1563874 however i'm not convinced that
|
||||
# patch was correct so we should reevaluate if we should do this.
|
||||
# The intent of using _detach_pci_devices is
|
||||
# to somehow cater for the use case where multiple ports have
|
||||
# the same MAC address however _detach_pci_device can only remove
|
||||
# device that are attached as hostdev elements, not via the
|
||||
# interface element.
|
||||
# So using it for all devices would break vnic-type direct when
|
||||
# using the sriov_nic_agent ml2 driver or vif of vnic_type vdpa.
|
||||
# Since PF ports cant have the same MAC that means that this
|
||||
# use case was for hardware offloaded OVS? many NICs do not allow
|
||||
# two VFs to have the same MAC on different VLANs due to the
|
||||
# ordering of the VLAN and MAC filters in there static packet
|
||||
# processing pipeline as such its unclear if this will work in any
|
||||
# non ovs offload case. We should look into this more closely
|
||||
# as from my testing in this patch we appear to use the interface
|
||||
# element for hardware offloaded ovs too. Infiniband and vnic_type
|
||||
# direct-physical port type do need this code path, both those cant
|
||||
# have duplicate MACs...
|
||||
self._detach_pci_devices(guest, direct_passthrough_pci_addresses)
|
||||
|
||||
# for ports that are attached with interface elements we cannot use
|
||||
# _detach_pci_devices so we use detach_interface
|
||||
for vif in attached_via_interface_element:
|
||||
self.detach_interface(context, instance, vif)
|
||||
|
||||
def _update_compute_provider_status(self, context, service):
|
||||
"""Calls the ComputeVirtAPI.update_compute_provider_status method
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user