Merge remote-tracking branch 'origin/master'
diff --git a/core/store/dist/src/main/java/org/onlab/onos/store/device/impl/GossipDeviceStore.java b/core/store/dist/src/main/java/org/onlab/onos/store/device/impl/GossipDeviceStore.java
index ac9fc3e..21941b5 100644
--- a/core/store/dist/src/main/java/org/onlab/onos/store/device/impl/GossipDeviceStore.java
+++ b/core/store/dist/src/main/java/org/onlab/onos/store/device/impl/GossipDeviceStore.java
@@ -290,12 +290,17 @@
private DeviceEvent updateDevice(ProviderId providerId,
Device oldDevice,
Device newDevice, Timestamp newTimestamp) {
-
// We allow only certain attributes to trigger update
- if (!Objects.equals(oldDevice.hwVersion(), newDevice.hwVersion()) ||
- !Objects.equals(oldDevice.swVersion(), newDevice.swVersion()) ||
- !AnnotationsUtil.isEqual(oldDevice.annotations(), newDevice.annotations())) {
+ boolean propertiesChanged =
+ !Objects.equals(oldDevice.hwVersion(), newDevice.hwVersion()) ||
+ !Objects.equals(oldDevice.swVersion(), newDevice.swVersion());
+ boolean annotationsChanged =
+ !AnnotationsUtil.isEqual(oldDevice.annotations(), newDevice.annotations());
+ // Primary providers can respond to all changes, but ancillary ones
+ // should respond only to annotation changes.
+ if ((providerId.isAncillary() && annotationsChanged) ||
+ (!providerId.isAncillary() && (propertiesChanged || annotationsChanged))) {
boolean replaced = devices.replace(newDevice.id(), oldDevice, newDevice);
if (!replaced) {
verify(replaced,
diff --git a/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleDeviceStore.java b/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleDeviceStore.java
index fbfaf9d..c1be1f5 100644
--- a/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleDeviceStore.java
+++ b/core/store/trivial/src/main/java/org/onlab/onos/store/trivial/impl/SimpleDeviceStore.java
@@ -70,15 +70,16 @@
public static final String DEVICE_NOT_FOUND = "Device with ID %s not found";
- // collection of Description given from various providers
+ // Collection of Description given from various providers
private final ConcurrentMap<DeviceId, Map<ProviderId, DeviceDescriptions>>
- deviceDescs = Maps.newConcurrentMap();
+ deviceDescs = Maps.newConcurrentMap();
- // cache of Device and Ports generated by compositing descriptions from providers
+ // Cache of Device and Ports generated by compositing descriptions from providers
private final ConcurrentMap<DeviceId, Device> devices = Maps.newConcurrentMap();
- private final ConcurrentMap<DeviceId, ConcurrentMap<PortNumber, Port>> devicePorts = Maps.newConcurrentMap();
+ private final ConcurrentMap<DeviceId, ConcurrentMap<PortNumber, Port>>
+ devicePorts = Maps.newConcurrentMap();
- // available(=UP) devices
+ // Available (=UP) devices
private final Set<DeviceId> availableDevices = Sets.newConcurrentHashSet();
@@ -113,19 +114,17 @@
@Override
public DeviceEvent createOrUpdateDevice(ProviderId providerId,
- DeviceId deviceId,
- DeviceDescription deviceDescription) {
-
+ DeviceId deviceId,
+ DeviceDescription deviceDescription) {
Map<ProviderId, DeviceDescriptions> providerDescs
- = getOrCreateDeviceDescriptions(deviceId);
+ = getOrCreateDeviceDescriptions(deviceId);
synchronized (providerDescs) {
// locking per device
-
DeviceDescriptions descs
- = getOrCreateProviderDeviceDescriptions(providerDescs,
- providerId,
- deviceDescription);
+ = getOrCreateProviderDeviceDescriptions(providerDescs,
+ providerId,
+ deviceDescription);
Device oldDevice = devices.get(deviceId);
// update description
@@ -145,12 +144,11 @@
// Creates the device and returns the appropriate event if necessary.
// Guarded by deviceDescs value (=Device lock)
private DeviceEvent createDevice(ProviderId providerId, Device newDevice) {
-
// update composed device cache
Device oldDevice = devices.putIfAbsent(newDevice.id(), newDevice);
verify(oldDevice == null,
- "Unexpected Device in cache. PID:%s [old=%s, new=%s]",
- providerId, oldDevice, newDevice);
+ "Unexpected Device in cache. PID:%s [old=%s, new=%s]",
+ providerId, oldDevice, newDevice);
if (!providerId.isAncillary()) {
availableDevices.add(newDevice.id());
@@ -162,17 +160,24 @@
// Updates the device and returns the appropriate event if necessary.
// Guarded by deviceDescs value (=Device lock)
private DeviceEvent updateDevice(ProviderId providerId, Device oldDevice, Device newDevice) {
-
// We allow only certain attributes to trigger update
- if (!Objects.equals(oldDevice.hwVersion(), newDevice.hwVersion()) ||
- !Objects.equals(oldDevice.swVersion(), newDevice.swVersion()) ||
- !AnnotationsUtil.isEqual(oldDevice.annotations(), newDevice.annotations())) {
+ boolean propertiesChanged =
+ !Objects.equals(oldDevice.hwVersion(), newDevice.hwVersion()) ||
+ !Objects.equals(oldDevice.swVersion(), newDevice.swVersion());
+ boolean annotationsChanged =
+ !AnnotationsUtil.isEqual(oldDevice.annotations(), newDevice.annotations());
+
+ // Primary providers can respond to all changes, but ancillary ones
+ // should respond only to annotation changes.
+ if ((providerId.isAncillary() && annotationsChanged) ||
+ (!providerId.isAncillary() && (propertiesChanged || annotationsChanged))) {
boolean replaced = devices.replace(newDevice.id(), oldDevice, newDevice);
if (!replaced) {
+ // FIXME: Is the enclosing if required here?
verify(replaced,
- "Replacing devices cache failed. PID:%s [expected:%s, found:%s, new=%s]",
- providerId, oldDevice, devices.get(newDevice.id())
+ "Replacing devices cache failed. PID:%s [expected:%s, found:%s, new=%s]",
+ providerId, oldDevice, devices.get(newDevice.id())
, newDevice);
}
if (!providerId.isAncillary()) {
@@ -193,7 +198,7 @@
@Override
public DeviceEvent markOffline(DeviceId deviceId) {
Map<ProviderId, DeviceDescriptions> providerDescs
- = getOrCreateDeviceDescriptions(deviceId);
+ = getOrCreateDeviceDescriptions(deviceId);
// locking device
synchronized (providerDescs) {
@@ -212,9 +217,8 @@
@Override
public List<DeviceEvent> updatePorts(ProviderId providerId,
- DeviceId deviceId,
- List<PortDescription> portDescriptions) {
-
+ DeviceId deviceId,
+ List<PortDescription> portDescriptions) {
Device device = devices.get(deviceId);
checkArgument(device != null, DEVICE_NOT_FOUND, deviceId);
@@ -226,8 +230,8 @@
DeviceDescriptions descs = descsMap.get(providerId);
// every provider must provide DeviceDescription.
checkArgument(descs != null,
- "Device description for Device ID %s from Provider %s was not found",
- deviceId, providerId);
+ "Device description for Device ID %s from Provider %s was not found",
+ deviceId, providerId);
Map<PortNumber, Port> ports = getPortMap(deviceId);
@@ -247,8 +251,8 @@
newPort = composePort(device, number, descsMap);
events.add(oldPort == null ?
- createPort(device, newPort, ports) :
- updatePort(device, oldPort, newPort, ports));
+ createPort(device, newPort, ports) :
+ updatePort(device, oldPort, newPort, ports));
}
events.addAll(pruneOldPorts(device, ports, processed));
@@ -272,7 +276,7 @@
Port newPort,
Map<PortNumber, Port> ports) {
if (oldPort.isEnabled() != newPort.isEnabled() ||
- !AnnotationsUtil.isEqual(oldPort.annotations(), newPort.annotations())) {
+ !AnnotationsUtil.isEqual(oldPort.annotations(), newPort.annotations())) {
ports.put(oldPort.number(), newPort);
return new DeviceEvent(PORT_UPDATED, device, newPort);
@@ -303,7 +307,7 @@
// exist, it creates and registers a new one.
private ConcurrentMap<PortNumber, Port> getPortMap(DeviceId deviceId) {
return createIfAbsentUnchecked(devicePorts, deviceId,
- NewConcurrentHashMap.<PortNumber, Port>ifNeeded());
+ NewConcurrentHashMap.<PortNumber, Port>ifNeeded());
}
private Map<ProviderId, DeviceDescriptions> getOrCreateDeviceDescriptions(
@@ -325,9 +329,8 @@
// Guarded by deviceDescs value (=Device lock)
private DeviceDescriptions getOrCreateProviderDeviceDescriptions(
- Map<ProviderId, DeviceDescriptions> device,
- ProviderId providerId, DeviceDescription deltaDesc) {
-
+ Map<ProviderId, DeviceDescriptions> device,
+ ProviderId providerId, DeviceDescription deltaDesc) {
synchronized (device) {
DeviceDescriptions r = device.get(providerId);
if (r == null) {
@@ -340,7 +343,7 @@
@Override
public DeviceEvent updatePortStatus(ProviderId providerId, DeviceId deviceId,
- PortDescription portDescription) {
+ PortDescription portDescription) {
Device device = devices.get(deviceId);
checkArgument(device != null, DEVICE_NOT_FOUND, deviceId);
@@ -351,8 +354,8 @@
DeviceDescriptions descs = descsMap.get(providerId);
// assuming all providers must give DeviceDescription first
checkArgument(descs != null,
- "Device description for Device ID %s from Provider %s was not found",
- deviceId, providerId);
+ "Device description for Device ID %s from Provider %s was not found",
+ deviceId, providerId);
ConcurrentMap<PortNumber, Port> ports = getPortMap(deviceId);
final PortNumber number = portDescription.portNumber();
@@ -404,19 +407,19 @@
availableDevices.remove(deviceId);
descs.clear();
return device == null ? null :
- new DeviceEvent(DEVICE_REMOVED, device, null);
+ new DeviceEvent(DEVICE_REMOVED, device, null);
}
}
/**
* Returns a Device, merging description given from multiple Providers.
*
- * @param deviceId device identifier
+ * @param deviceId device identifier
* @param providerDescs Collection of Descriptions from multiple providers
* @return Device instance
*/
private Device composeDevice(DeviceId deviceId,
- Map<ProviderId, DeviceDescriptions> providerDescs) {
+ Map<ProviderId, DeviceDescriptions> providerDescs) {
checkArgument(!providerDescs.isEmpty(), "No Device descriptions supplied");
@@ -447,21 +450,21 @@
annotations = merge(annotations, e.getValue().getDeviceDesc().annotations());
}
- return new DefaultDevice(primary, deviceId , type, manufacturer,
- hwVersion, swVersion, serialNumber,
- chassisId, annotations);
+ return new DefaultDevice(primary, deviceId, type, manufacturer,
+ hwVersion, swVersion, serialNumber,
+ chassisId, annotations);
}
/**
* Returns a Port, merging description given from multiple Providers.
*
- * @param device device the port is on
- * @param number port number
+ * @param device device the port is on
+ * @param number port number
* @param descsMap Collection of Descriptions from multiple providers
* @return Port instance
*/
private Port composePort(Device device, PortNumber number,
- Map<ProviderId, DeviceDescriptions> descsMap) {
+ Map<ProviderId, DeviceDescriptions> descsMap) {
ProviderId primary = pickPrimaryPID(descsMap);
DeviceDescriptions primDescs = descsMap.get(primary);