Committed by
Gerrit Code Review
bug fix: ResourceManager#unregisterResources only unregisters given resources an…
…d didn't unregister descendants (ONOS-3827). Change-Id: I48956234553053e2a54834129b5f10357ca116ea
Showing
2 changed files
with
42 additions
and
5 deletions
| ... | @@ -41,10 +41,13 @@ import org.onosproject.net.newresource.ResourceAdminService; | ... | @@ -41,10 +41,13 @@ import org.onosproject.net.newresource.ResourceAdminService; |
| 41 | import org.onosproject.net.newresource.BandwidthCapacity; | 41 | import org.onosproject.net.newresource.BandwidthCapacity; |
| 42 | import org.onosproject.net.newresource.Resource; | 42 | import org.onosproject.net.newresource.Resource; |
| 43 | import org.onosproject.net.newresource.Resources; | 43 | import org.onosproject.net.newresource.Resources; |
| 44 | +import org.onosproject.net.newresource.ResourceService; | ||
| 44 | import org.slf4j.Logger; | 45 | import org.slf4j.Logger; |
| 45 | import org.slf4j.LoggerFactory; | 46 | import org.slf4j.LoggerFactory; |
| 46 | 47 | ||
| 47 | import java.util.Collections; | 48 | import java.util.Collections; |
| 49 | +import java.util.LinkedList; | ||
| 50 | +import java.util.List; | ||
| 48 | import java.util.Optional; | 51 | import java.util.Optional; |
| 49 | import java.util.Set; | 52 | import java.util.Set; |
| 50 | import java.util.concurrent.ExecutorService; | 53 | import java.util.concurrent.ExecutorService; |
| ... | @@ -60,6 +63,7 @@ final class ResourceDeviceListener implements DeviceListener { | ... | @@ -60,6 +63,7 @@ final class ResourceDeviceListener implements DeviceListener { |
| 60 | private static final Logger log = LoggerFactory.getLogger(ResourceDeviceListener.class); | 63 | private static final Logger log = LoggerFactory.getLogger(ResourceDeviceListener.class); |
| 61 | 64 | ||
| 62 | private final ResourceAdminService adminService; | 65 | private final ResourceAdminService adminService; |
| 66 | + private final ResourceService resourceService; | ||
| 63 | private final DeviceService deviceService; | 67 | private final DeviceService deviceService; |
| 64 | private final DriverService driverService; | 68 | private final DriverService driverService; |
| 65 | private final NetworkConfigService netcfgService; | 69 | private final NetworkConfigService netcfgService; |
| ... | @@ -75,9 +79,11 @@ final class ResourceDeviceListener implements DeviceListener { | ... | @@ -75,9 +79,11 @@ final class ResourceDeviceListener implements DeviceListener { |
| 75 | * @param netcfgService {@link NetworkConfigService} to be used. | 79 | * @param netcfgService {@link NetworkConfigService} to be used. |
| 76 | * @param executor executor used for processing resource registration | 80 | * @param executor executor used for processing resource registration |
| 77 | */ | 81 | */ |
| 78 | - ResourceDeviceListener(ResourceAdminService adminService, DeviceService deviceService, DriverService driverService, | 82 | + ResourceDeviceListener(ResourceAdminService adminService, ResourceService resourceService, |
| 83 | + DeviceService deviceService, DriverService driverService, | ||
| 79 | NetworkConfigService netcfgService, ExecutorService executor) { | 84 | NetworkConfigService netcfgService, ExecutorService executor) { |
| 80 | this.adminService = checkNotNull(adminService); | 85 | this.adminService = checkNotNull(adminService); |
| 86 | + this.resourceService = checkNotNull(resourceService); | ||
| 81 | this.deviceService = checkNotNull(deviceService); | 87 | this.deviceService = checkNotNull(deviceService); |
| 82 | this.driverService = checkNotNull(driverService); | 88 | this.driverService = checkNotNull(driverService); |
| 83 | this.netcfgService = checkNotNull(netcfgService); | 89 | this.netcfgService = checkNotNull(netcfgService); |
| ... | @@ -123,7 +129,11 @@ final class ResourceDeviceListener implements DeviceListener { | ... | @@ -123,7 +129,11 @@ final class ResourceDeviceListener implements DeviceListener { |
| 123 | } | 129 | } |
| 124 | 130 | ||
| 125 | private void unregisterDeviceResource(Device device) { | 131 | private void unregisterDeviceResource(Device device) { |
| 126 | - executor.submit(() -> adminService.unregisterResources(Resources.discrete(device.id()).resource())); | 132 | + executor.submit(() -> { |
| 133 | + Resource devResource = Resources.discrete(device.id()).resource(); | ||
| 134 | + List<Resource> allResources = getDescendantResources(devResource); | ||
| 135 | + adminService.unregisterResources(allResources); | ||
| 136 | + }); | ||
| 127 | } | 137 | } |
| 128 | 138 | ||
| 129 | private void registerPortResource(Device device, Port port) { | 139 | private void registerPortResource(Device device, Port port) { |
| ... | @@ -175,8 +185,30 @@ final class ResourceDeviceListener implements DeviceListener { | ... | @@ -175,8 +185,30 @@ final class ResourceDeviceListener implements DeviceListener { |
| 175 | } | 185 | } |
| 176 | 186 | ||
| 177 | private void unregisterPortResource(Device device, Port port) { | 187 | private void unregisterPortResource(Device device, Port port) { |
| 178 | - Resource resource = Resources.discrete(device.id(), port.number()).resource(); | 188 | + executor.submit(() -> { |
| 179 | - executor.submit(() -> adminService.unregisterResources(resource)); | 189 | + Resource portResource = Resources.discrete(device.id(), port.number()).resource(); |
| 190 | + List<Resource> allResources = getDescendantResources(portResource); | ||
| 191 | + adminService.unregisterResources(allResources); | ||
| 192 | + }); | ||
| 193 | + } | ||
| 194 | + | ||
| 195 | + // Returns list of all descendant resources of given resource, including itself. | ||
| 196 | + private List<Resource> getDescendantResources(Resource parent) { | ||
| 197 | + LinkedList<Resource> allResources = new LinkedList<>(); | ||
| 198 | + allResources.add(parent); | ||
| 199 | + | ||
| 200 | + Set<Resource> nextResources = resourceService.getRegisteredResources(parent); | ||
| 201 | + while (!nextResources.isEmpty()) { | ||
| 202 | + Set<Resource> currentResources = nextResources; | ||
| 203 | + // resource list should be ordered from leaf to root | ||
| 204 | + allResources.addAll(0, currentResources); | ||
| 205 | + | ||
| 206 | + nextResources = currentResources.stream() | ||
| 207 | + .flatMap(r -> resourceService.getRegisteredResources(r).stream()) | ||
| 208 | + .collect(Collectors.toSet()); | ||
| 209 | + } | ||
| 210 | + | ||
| 211 | + return allResources; | ||
| 180 | } | 212 | } |
| 181 | 213 | ||
| 182 | /** | 214 | /** | ... | ... |
| ... | @@ -32,6 +32,7 @@ import org.onosproject.net.device.DeviceService; | ... | @@ -32,6 +32,7 @@ import org.onosproject.net.device.DeviceService; |
| 32 | import org.onosproject.net.driver.DriverService; | 32 | import org.onosproject.net.driver.DriverService; |
| 33 | import org.onosproject.net.newresource.BandwidthCapacity; | 33 | import org.onosproject.net.newresource.BandwidthCapacity; |
| 34 | import org.onosproject.net.newresource.ResourceAdminService; | 34 | import org.onosproject.net.newresource.ResourceAdminService; |
| 35 | +import org.onosproject.net.newresource.ResourceService; | ||
| 35 | import org.slf4j.Logger; | 36 | import org.slf4j.Logger; |
| 36 | 37 | ||
| 37 | import java.util.List; | 38 | import java.util.List; |
| ... | @@ -53,6 +54,9 @@ public final class ResourceRegistrar { | ... | @@ -53,6 +54,9 @@ public final class ResourceRegistrar { |
| 53 | protected ResourceAdminService adminService; | 54 | protected ResourceAdminService adminService; |
| 54 | 55 | ||
| 55 | @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) | 56 | @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) |
| 57 | + protected ResourceService resourceService; | ||
| 58 | + | ||
| 59 | + @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) | ||
| 56 | protected DriverService driverService; | 60 | protected DriverService driverService; |
| 57 | 61 | ||
| 58 | @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) | 62 | @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) |
| ... | @@ -87,7 +91,8 @@ public final class ResourceRegistrar { | ... | @@ -87,7 +91,8 @@ public final class ResourceRegistrar { |
| 87 | cfgListener = new ResourceNetworkConfigListener(adminService, cfgRegistry, executor); | 91 | cfgListener = new ResourceNetworkConfigListener(adminService, cfgRegistry, executor); |
| 88 | cfgRegistry.addListener(cfgListener); | 92 | cfgRegistry.addListener(cfgListener); |
| 89 | 93 | ||
| 90 | - deviceListener = new ResourceDeviceListener(adminService, deviceService, driverService, cfgRegistry, executor); | 94 | + deviceListener = new ResourceDeviceListener(adminService, resourceService, |
| 95 | + deviceService, driverService, cfgRegistry, executor); | ||
| 91 | deviceService.addListener(deviceListener); | 96 | deviceService.addListener(deviceListener); |
| 92 | 97 | ||
| 93 | // TODO Attempt initial registration of existing resources? | 98 | // TODO Attempt initial registration of existing resources? | ... | ... |
-
Please register or login to post a comment