Changed argument from Device -> DeviceId
- roleChanged(), isReachable()
Eventually consistent nature of Device store was
interfering with mastership control.
Change-Id: I9c0dd846a4e30863f922f6706c6cb62fd7c83f29
diff --git a/core/api/src/main/java/org/onlab/onos/net/device/DeviceProvider.java b/core/api/src/main/java/org/onlab/onos/net/device/DeviceProvider.java
index 8411b09..8dd405a 100644
--- a/core/api/src/main/java/org/onlab/onos/net/device/DeviceProvider.java
+++ b/core/api/src/main/java/org/onlab/onos/net/device/DeviceProvider.java
@@ -16,6 +16,7 @@
package org.onlab.onos.net.device;
import org.onlab.onos.net.Device;
+import org.onlab.onos.net.DeviceId;
import org.onlab.onos.net.MastershipRole;
import org.onlab.onos.net.provider.Provider;
@@ -42,16 +43,16 @@
* Notifies the provider of a mastership role change for the specified
* device as decided by the core.
*
- * @param device affected device
+ * @param deviceId device identifier
* @param newRole newly determined mastership role
*/
- void roleChanged(Device device, MastershipRole newRole);
+ void roleChanged(DeviceId deviceId, MastershipRole newRole);
/**
* Checks the reachability (connectivity) of a device from this provider.
*
- * @param device device to check
+ * @param deviceId device identifier
* @return true if reachable, false otherwise
*/
- boolean isReachable(Device device);
+ boolean isReachable(DeviceId deviceId);
}
diff --git a/core/net/src/main/java/org/onlab/onos/net/device/impl/DeviceManager.java b/core/net/src/main/java/org/onlab/onos/net/device/impl/DeviceManager.java
index 258f57c..68665e4 100644
--- a/core/net/src/main/java/org/onlab/onos/net/device/impl/DeviceManager.java
+++ b/core/net/src/main/java/org/onlab/onos/net/device/impl/DeviceManager.java
@@ -159,13 +159,14 @@
}
// Check a device for control channel connectivity.
- private boolean isReachable(Device device) {
- // FIXME: Device might not be there yet. (eventual consistent)
- if (device == null) {
+ private boolean isReachable(DeviceId deviceId) {
+ DeviceProvider provider = getProvider(deviceId);
+ if (provider != null) {
+ return provider.isReachable(deviceId);
+ } else {
+ log.error("Provider not found for {}", deviceId);
return false;
}
- DeviceProvider provider = getProvider(device.providerId());
- return provider.isReachable(device);
}
@Override
@@ -226,15 +227,9 @@
log.warn("Provider for {} was not found. Cannot apply role {}", deviceId, newRole);
return false;
}
- // FIXME roleChanged should take DeviceId instead of Device
- Device device = store.getDevice(deviceId);
- if (device == null) {
- log.warn("{} was not there. Cannot apply role {}", deviceId, newRole);
- return false;
- }
- provider.roleChanged(device, newRole);
+ provider.roleChanged(deviceId, newRole);
+ // not triggering probe when triggered by provider service event
- // not triggering prove when triggered by provider service
return true;
}
@@ -442,6 +437,7 @@
Device device = store.getDevice(deviceId);
// FIXME: Device might not be there yet. (eventual consistent)
+ // FIXME relinquish role
if (device == null) {
log.warn("{} was not there. Cannot apply role {}", deviceId, newRole);
return false;
@@ -452,8 +448,7 @@
log.warn("Provider for {} was not found. Cannot apply role {}", deviceId, newRole);
return false;
}
- // FIXME roleChanged should take DeviceId instead of Device
- provider.roleChanged(device, newRole);
+ provider.roleChanged(deviceId, newRole);
if (newRole.equals(MastershipRole.MASTER)) {
// only trigger event when request was sent to provider
@@ -495,8 +490,7 @@
}
- final Device device = getDevice(did);
- final boolean isReachable = isReachable(device);
+ final boolean isReachable = isReachable(did);
if (!isReachable) {
// device is not connected to this node
if (myNextRole != NONE) {
@@ -505,6 +499,7 @@
+ "Relinquishing role. ",
myNextRole, did);
mastershipService.relinquishMastership(did);
+ // FIXME disconnect?
}
return;
}
@@ -523,6 +518,7 @@
switch (myNextRole) {
case MASTER:
+ final Device device = getDevice(did);
if ((device != null) && !isAvailable(did)) {
//flag the device as online. Is there a better way to do this?
DefaultDeviceDescription deviceDescription
diff --git a/core/net/src/test/java/org/onlab/onos/net/device/impl/DeviceManagerTest.java b/core/net/src/test/java/org/onlab/onos/net/device/impl/DeviceManagerTest.java
index 79e2f10..d278c24 100644
--- a/core/net/src/test/java/org/onlab/onos/net/device/impl/DeviceManagerTest.java
+++ b/core/net/src/test/java/org/onlab/onos/net/device/impl/DeviceManagerTest.java
@@ -251,7 +251,7 @@
private class TestProvider extends AbstractProvider implements DeviceProvider {
- private Device deviceReceived;
+ private DeviceId deviceReceived;
private MastershipRole roleReceived;
public TestProvider() {
@@ -263,13 +263,13 @@
}
@Override
- public void roleChanged(Device device, MastershipRole newRole) {
+ public void roleChanged(DeviceId device, MastershipRole newRole) {
deviceReceived = device;
roleReceived = newRole;
}
@Override
- public boolean isReachable(Device device) {
+ public boolean isReachable(DeviceId device) {
return false;
}
}