Moti Asayag has posted comments on this change. Change subject: engine: Cleanup the vfs when the vm is down ......................................................................
Patch Set 6: Code-Review-1 (5 comments) https://gerrit.ovirt.org/#/c/38624/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ProcessDownVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ProcessDownVmCommand.java: Line 49: @Inject Line 50: private ResourceManager resourceManager; Line 51: Line 52: @Inject Line 53: HostNicVfsConfigHelper hostNicVfsConfigHelper; package modifier for testing ? Line 54: Line 55: protected ProcessDownVmCommand(Guid commandId) { Line 56: super(commandId); Line 57: } Line 95: Guid hostId = hostNicVfsConfigHelper.removeVmIdFromVfs(getVmId()); Line 96: Line 97: // refresh host network interfaces to get the VFs that were detached from the VM and re-attached to the host Line 98: if (hostId != null) { Line 99: VDS host = DbFacade.getInstance().getVdsDao().get(hostId); please replace DbFacade.getInstance().getVdsDao() with @Inject private VdsDao hostDao; Line 100: Line 101: resourceManager.runVdsCommand(VDSCommandType.CollectVdsNetworkData, Line 102: new CollectHostNetworkDataVdsCommandParameters(host, Line 103: Collections.<VdsNetworkInterface> emptyList())); https://gerrit.ovirt.org/#/c/38624/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/HostNicVfsConfigHelperImpl.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/HostNicVfsConfigHelperImpl.java: Line 266: return hostId; Line 267: } Line 268: Line 269: private boolean isVf(HostDevice device) { Line 270: return !StringUtils.isBlank(device.getParentPhysicalFunction()); use StringUtils.isNotBlank() Line 271: } https://gerrit.ovirt.org/#/c/38624/6/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/HostNicVfsConfigHelperImplTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/HostNicVfsConfigHelperImplTest.java: Line 440: List<HostDevice> vfs = mockVfsOnNetDevice(numOfVfWithVmId, vmId); Line 441: allDevices.addAll(vfs); Line 442: Line 443: for (int i = 0; i <= numOfOtherDeviceWithVmId; ++i) { Line 444: HostDevice hostDevice = new HostDevice(); you can extract this into a method createHostDevice() Line 445: hostDevice.setHostId(HOST_ID); Line 446: hostDevice.setVmId(vmId); Line 447: otherDeviceWithVmId.add(hostDevice); Line 448: } Line 454: } Line 455: Line 456: hostNicVfsConfigHelper.removeVmIdFromVfs(vmId); Line 457: Line 458: for (HostDevice vf : vfs) { this looks wrong to me - at this time the vmId should already be null and should be verified here to be null. Line 459: vf.setVmId(null); Line 460: } Line 461: Line 462: if (numOfVfWithVmId == 0) { -- To view, visit https://gerrit.ovirt.org/38624 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia1f861a451f4e0961ec122e4c2594081ea6dc71b Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alona Kaplan <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
