Checking pci_slot to avoid changing staus to BUILD forever
Currently when sriov agent is enabled and migrating a non-sriov
instance, non-sriov port status is frequently set to BUILD
instead of ACTIVE.
This is because the 'binding_activate' function in sriov-nic-agent sets it
BUILD with get_device_details_from_port_id(as it calls _get_new_status).
This patch checks network_ports in binding_activate and
skip binding port if it is not sriov port
Closes-Bug: #2072154
Change-Id: I2d7702e17c75c96ca2f29749dccab77cb2f4bcf4
(cherry picked from commit a311606fcd
)
This commit is contained in:
parent
ec5019574e
commit
27eee0b9e8
@ -112,17 +112,39 @@ class SriovNicSwitchRpcCallbacks(sg_rpc.SecurityGroupAgentRpcCallbackMixin):
|
|||||||
def binding_activate(self, context, **kwargs):
|
def binding_activate(self, context, **kwargs):
|
||||||
if kwargs.get('host') != self.agent.conf.host:
|
if kwargs.get('host') != self.agent.conf.host:
|
||||||
return
|
return
|
||||||
LOG.debug("binding activate for port %s", kwargs.get('port_id'))
|
|
||||||
device_details = self.agent.get_device_details_from_port_id(
|
port_id = kwargs.get('port_id')
|
||||||
kwargs.get('port_id'))
|
|
||||||
mac = device_details.get('mac_address')
|
def _is_port_id_in_network(network_port, port_id):
|
||||||
binding_profile = device_details.get('profile')
|
for network_id, ports in network_port.items():
|
||||||
if binding_profile:
|
for port in ports:
|
||||||
pci_slot = binding_profile.get('pci_slot')
|
if port['port_id'] == port_id:
|
||||||
self.agent.activated_bindings.add((mac, pci_slot))
|
return True
|
||||||
|
return False
|
||||||
|
|
||||||
|
is_port_id_sriov = _is_port_id_in_network(
|
||||||
|
self.agent.network_ports, port_id
|
||||||
|
)
|
||||||
|
|
||||||
|
if is_port_id_sriov:
|
||||||
|
LOG.debug("binding activate for port %s", port_id)
|
||||||
|
device_details = self.agent.get_device_details_from_port_id(
|
||||||
|
port_id)
|
||||||
|
mac = device_details.get('mac_address')
|
||||||
|
binding_profile = device_details.get('profile')
|
||||||
|
if binding_profile:
|
||||||
|
pci_slot = binding_profile.get('pci_slot')
|
||||||
|
self.agent.activated_bindings.add((mac, pci_slot))
|
||||||
|
else:
|
||||||
|
LOG.warning(
|
||||||
|
"binding_profile not found for port %s.",
|
||||||
|
port_id
|
||||||
|
)
|
||||||
else:
|
else:
|
||||||
LOG.warning("binding_profile not found for port %s.",
|
LOG.warning(
|
||||||
kwargs.get('port_id'))
|
"This port is not SRIOV, skip binding for port %s.",
|
||||||
|
port_id
|
||||||
|
)
|
||||||
|
|
||||||
def binding_deactivate(self, context, **kwargs):
|
def binding_deactivate(self, context, **kwargs):
|
||||||
if kwargs.get('host') != self.agent.conf.host:
|
if kwargs.get('host') != self.agent.conf.host:
|
||||||
|
@ -13,6 +13,7 @@
|
|||||||
# See the License for the specific language governing permissions and
|
# See the License for the specific language governing permissions and
|
||||||
# limitations under the License.
|
# limitations under the License.
|
||||||
|
|
||||||
|
import collections
|
||||||
import copy
|
import copy
|
||||||
from unittest import mock
|
from unittest import mock
|
||||||
|
|
||||||
@ -460,6 +461,7 @@ class FakeAgent(object):
|
|||||||
self.activated_bindings = set()
|
self.activated_bindings = set()
|
||||||
self.conf = mock.Mock()
|
self.conf = mock.Mock()
|
||||||
self.conf.host = 'host1'
|
self.conf.host = 'host1'
|
||||||
|
self.network_ports = collections.defaultdict(list)
|
||||||
|
|
||||||
|
|
||||||
class TestSriovNicSwitchRpcCallbacks(base.BaseTestCase):
|
class TestSriovNicSwitchRpcCallbacks(base.BaseTestCase):
|
||||||
@ -528,6 +530,12 @@ class TestSriovNicSwitchRpcCallbacks(base.BaseTestCase):
|
|||||||
}
|
}
|
||||||
kwargs = self._create_fake_bindings(fake_port, self.agent.conf.host)
|
kwargs = self._create_fake_bindings(fake_port, self.agent.conf.host)
|
||||||
kwargs['context'] = self.context
|
kwargs['context'] = self.context
|
||||||
|
|
||||||
|
self.agent.network_ports['network_id'].append({
|
||||||
|
'port_id': fake_port['id'],
|
||||||
|
'device': 'fake_device'
|
||||||
|
})
|
||||||
|
|
||||||
self.sriov_rpc_callback.binding_activate(**kwargs)
|
self.sriov_rpc_callback.binding_activate(**kwargs)
|
||||||
# Assert agent.activated_binding set contains the new binding
|
# Assert agent.activated_binding set contains the new binding
|
||||||
self.assertIn((fake_port['mac_address'],
|
self.assertIn((fake_port['mac_address'],
|
||||||
@ -538,10 +546,32 @@ class TestSriovNicSwitchRpcCallbacks(base.BaseTestCase):
|
|||||||
fake_port = self._create_fake_port()
|
fake_port = self._create_fake_port()
|
||||||
kwargs = self._create_fake_bindings(fake_port, 'other-host')
|
kwargs = self._create_fake_bindings(fake_port, 'other-host')
|
||||||
kwargs['context'] = self.context
|
kwargs['context'] = self.context
|
||||||
|
|
||||||
|
self.agent.network_ports[self.agent.conf.host].append({
|
||||||
|
'port_id': fake_port['id'],
|
||||||
|
'device': 'fake_device'
|
||||||
|
})
|
||||||
|
|
||||||
self.sriov_rpc_callback.binding_activate(**kwargs)
|
self.sriov_rpc_callback.binding_activate(**kwargs)
|
||||||
# Assert no bindings were added
|
# Assert no bindings were added
|
||||||
self.assertEqual(set(), self.agent.activated_bindings)
|
self.assertEqual(set(), self.agent.activated_bindings)
|
||||||
|
|
||||||
|
def test_binding_activate_port_not_in_network(self):
|
||||||
|
fake_port = self._create_fake_port()
|
||||||
|
kwargs = self._create_fake_bindings(fake_port, self.agent.conf.host)
|
||||||
|
kwargs['context'] = self.context
|
||||||
|
|
||||||
|
self.agent.network_ports['network_id'] = []
|
||||||
|
|
||||||
|
with mock.patch.object(sriov_nic_agent.LOG,
|
||||||
|
'warning') as mock_warning:
|
||||||
|
self.sriov_rpc_callback.binding_activate(**kwargs)
|
||||||
|
# Check that the warning message was logged
|
||||||
|
expected_msg = (
|
||||||
|
"This port is not SRIOV, skip binding for port %s."
|
||||||
|
)
|
||||||
|
mock_warning.assert_called_once_with(expected_msg, fake_port['id'])
|
||||||
|
|
||||||
def test_binding_deactivate(self):
|
def test_binding_deactivate(self):
|
||||||
# binding_deactivate() basically does nothing
|
# binding_deactivate() basically does nothing
|
||||||
# call it with both the agent's host and other host to cover
|
# call it with both the agent's host and other host to cover
|
||||||
|
Loading…
Reference in New Issue
Block a user