Martin Mucha has posted comments on this change. Change subject: core: Removal of labelled network from DC inconsistent with removal from cluster ......................................................................
Patch Set 1: (6 comments) http://gerrit.ovirt.org/#/c/30795/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/cluster/NetworkHelper.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/cluster/NetworkHelper.java: Line 119: public static boolean setupNetworkSupported(Version version) { Line 120: return VersionSupport.isActionSupported(VdcActionType.SetupNetworks, version); Line 121: } Line 122: Line 123: public static void removeNetworkFromHostsInCluster(Network network, Guid clusterId, CommandContext context, InterfaceDao interfaceDao) { > You can get the singleton instance of interfaace dao calling- 'DbFacade.get Done Line 124: List<VdsNetworkInterface> nics = interfaceDao.getAllInterfacesByLabelForCluster(clusterId, network.getLabel()); Line 125: removeNetworkFromHosts(network, context, nics); Line 126: } Line 127: Line 124: List<VdsNetworkInterface> nics = interfaceDao.getAllInterfacesByLabelForCluster(clusterId, network.getLabel()); Line 125: removeNetworkFromHosts(network, context, nics); Line 126: } Line 127: Line 128: public static void removeNetworkFromHostsInDataCenter(Network network, Guid dataCenterId, CommandContext context, InterfaceDao interfaceDao) { > Same here, no need to pass 'InterfaceDao' as a parameter. Done Line 129: List<VdsNetworkInterface> nics = interfaceDao.getAllInterfacesByLabelForDataCenter(dataCenterId, network.getLabel()); Line 130: removeNetworkFromHosts(network, context, nics); Line 131: } Line 132: Line 129: List<VdsNetworkInterface> nics = interfaceDao.getAllInterfacesByLabelForDataCenter(dataCenterId, network.getLabel()); Line 130: removeNetworkFromHosts(network, context, nics); Line 131: } Line 132: Line 133: protected static void removeNetworkFromHosts(Network network, CommandContext context, List<VdsNetworkInterface> nics) { > Why protected and not private? Done Line 134: RemoveNetworkParametersBuilder builder = new RemoveNetworkParametersBuilder(network, context); Line 135: ArrayList<VdcActionParametersBase> parameters = builder.buildParameters(nics); Line 136: Line 137: if (!parameters.isEmpty()) { Line 135: ArrayList<VdcActionParametersBase> parameters = builder.buildParameters(nics); Line 136: Line 137: if (!parameters.isEmpty()) { Line 138: NetworkParametersBuilder.updateParametersSequencing(parameters); Line 139: Backend.getInstance().runInternalMultipleActions(VdcActionType.PersistentSetupNetworks, parameters, context); > After rethinking it, it is even better to pass 'cloneContextAndDetachFromPa Done Line 140: } Line 141: } http://gerrit.ovirt.org/#/c/30795/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/RemoveNetworkCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/RemoveNetworkCommand.java: Line 65: if (getParameters().isRemoveFromNetworkProvider()) { Line 66: removeExternalNetwork(); Line 67: } Line 68: } else { Line 69: if (NetworkUtils.isLabeled(getNetwork()) > The code here duplicates the following code- Done Line 70: && NetworkHelper.setupNetworkSupported(getStoragePool().getcompatibility_version())) { Line 71: removeNetworkFromHosts(); Line 72: } Line 73: } http://gerrit.ovirt.org/#/c/30795/1/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/InterfaceDaoDbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/InterfaceDaoDbFacadeImpl.java: Line 244: getCustomMapSqlParameterSource().addValue("cluster_id", clusterId)); Line 245: } Line 246: Line 247: @Override Line 248: public List<VdsNetworkInterface> getAllInterfacesByLabelForDataCenter(Guid dataCenterId, String label) { > Duplicates the code in 'getAllInterfacesByLabelForCluster(..)'. Please refa Done Line 249: List<VdsNetworkInterface> result = new ArrayList<>(); Line 250: Line 251: for (VdsNetworkInterface vdsNetworkInterface : getAllInterfacesByDataCenterId(dataCenterId)) { Line 252: final Set<String> labels = vdsNetworkInterface.getLabels(); -- To view, visit http://gerrit.ovirt.org/30795 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9ebc162324ff2ed98374d6a9a3bd9d87bb69db3f Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Martin Mucha <[email protected]> Gerrit-Reviewer: Moti Asayag <[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
