Mike Kolesnik has posted comments on this change.
Change subject: engine: Edit Network and apply to hosts at once
......................................................................
Patch Set 5: Code-Review-1
(8 comments)
You have a bug in the canDoAcion logic
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/UpdateNetworkCommand.java
Line 38:
Line 39: @NonTransactiveCommandAttribute
Line 40: public class UpdateNetworkCommand<T extends
AddNetworkStoragePoolParameters> extends NetworkCommon<T> implements
RenamedEntityInfoProvider{
Line 41: private Network oldNetwork;
Line 42: private SetupNetworksParametersBuilder builder;
Why save the builder and not just the parameters list?
Line 43:
Line 44: public UpdateNetworkCommand(T parameters) {
Line 45: super(parameters);
Line 46: }
Line 182: }
Line 183:
Line 184: public ValidationResult nonVmNetworkNotUsedByVms(Network
updatedNetwork) {
Line 185: if (networkChangedToNonVmNetwork(updatedNetwork)) {
Line 186: networkNotUsed(getVms(),
VdcBllMessages.VAR__ENTITIES__VMS);
Shouldn't you return the return value from this call?
Line 187: }
Line 188:
Line 189: return ValidationResult.VALID;
Line 190: }
Line 196: public ValidationResult networkNotUsedByRunningVms() {
Line 197: List<VM> runningVms = new ArrayList<>();
Line 198:
Line 199: for (VM vm : getVms()) {
Line 200: if (vm.getStatus() != VMStatus.Down) {
You should use vm.isRunningOrPaused()
Line 201: runningVms.add(vm);
Line 202: }
Line 203: }
Line 204:
Line 206: }
Line 207:
Line 208: public ValidationResult
nonVmNetworkNotUsedByTemplates(Network updatedNetwork) {
Line 209: if (networkChangedToNonVmNetwork(updatedNetwork)) {
Line 210: networkNotUsed(getTemplates(),
VdcBllMessages.VAR__ENTITIES__VMS);
Shouldn't you return the return value from this call?
Line 211: }
Line 212:
Line 213: return ValidationResult.VALID;
Line 214: }
Line 261:
Line 262: }
Line 263:
Line 264: private abstract class SetupNetworksParametersBuilder {
Line 265: private ArrayList<VdcActionParametersBase> parameters = new
ArrayList<>();
Please use List as the reference type
Line 266:
Line 267: protected abstract void buildParameters(Network network);
Line 268:
Line 269: protected boolean setupNetworkSupported(VDS host) {
Line 281: protected List<VdsNetworkInterface> getHostNics(VDS host) {
Line 282: return
getDbFacade().getInterfaceDao().getAllInterfacesForVds(host.getId());
Line 283: }
Line 284:
Line 285: public ArrayList<VdcActionParametersBase> getParameters() {
Same here
Line 286: return parameters;
Line 287: }
Line 288: }
Line 289:
Line 326: SetupNetworksParameters setupNetworkParams =
createSetupNetworksParameters(host);
Line 327: List<VdsNetworkInterface> nics =
setupNetworkParams.getInterfaces();
Line 328: for (VdsNetworkInterface nic : nics) {
Line 329: if (StringUtils.equals(nic.getNetworkName(),
getOldNetwork().getName())) {
Line 330: nic.setNetworkName(network.getName());
You can call getNetworkName()
Line 331: break;
Line 332: }
Line 333: }
Line 334:
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/AddNetworkStoragePoolParameters.java
Line 13: @NotNull
Line 14: private Network network;
Line 15:
Line 16: private boolean vnicProfileRequired;
Line 17: private boolean configureHosts;
You can call it "applyToHosts"..
Line 18:
Line 19: public AddNetworkStoragePoolParameters() {
Line 20: vnicProfileRequired = true;
Line 21: }
--
To view, visit http://gerrit.ovirt.org/22053
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iec04c7fb0c29ba6f61b7d788a9c3917f5d46554e
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Moti Asayag <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[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
