Sergey Gotliv has posted comments on this change. Change subject: engine: Commands to add, edit and remove iscsi bonds ......................................................................
Patch Set 7: (13 comments) http://gerrit.ovirt.org/#/c/22952/7/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddIscsiBondCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddIscsiBondCommand.java: Line 1: package org.ovirt.engine.core.bll; > shouldn't this be in bll.storage? Done Line 2: Line 3: import java.util.Collections; Line 4: import java.util.List; Line 5: Line 12: import org.ovirt.engine.core.compat.Guid; Line 13: import org.ovirt.engine.core.utils.transaction.TransactionMethod; Line 14: import org.ovirt.engine.core.utils.transaction.TransactionSupport; Line 15: Line 16: @LockIdNameAttribute(isReleaseAtEndOfExecute = false) > why false? you don't have an async task here. Done Line 17: @NonTransactiveCommandAttribute Line 18: public class AddIscsiBondCommand<T extends AddIscsiBondParameters> extends CommandBase<T> { Line 19: Line 20: public AddIscsiBondCommand(T parameters) { Line 24: Line 25: public AddIscsiBondCommand(Guid commandId) { Line 26: super(commandId); Line 27: } Line 28: > How about having a canDoAction() to check that this entity doesn't exist ye Done Line 29: @Override Line 30: protected void executeCommand() { Line 31: final List<Guid> networkIds = getParameters().getIscsiBond().getNetworkIds(); Line 32: final List<String> connectionsIds = getParameters().getIscsiBond().getStorageConnectionIds(); Line 40: public Void runInTransaction() { Line 41: getDbFacade().getIscsiBondDao().save(iscsiBond); Line 42: Line 43: for (Guid networkId : networkIds) { Line 44: getDbFacade().getIscsiBondDao().addNetworkToIscsiBond(iscsiBond.getId(), networkId); > This should be batched - make the dao extend MassOperationsGenericDaoDbFaca I don't see how MassOperationsGenericDaoDbFacade can help me. I am trying to populate the reference table here. Line 45: } Line 46: Line 47: for (String connectionId : connectionsIds) { Line 48: getDbFacade().getIscsiBondDao().addStorageConnectionToIscsiBond(iscsiBond.getId(), connectionId); http://gerrit.ovirt.org/#/c/22952/7/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EditIscsiBondCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/EditIscsiBondCommand.java: Line 1: package org.ovirt.engine.core.bll; > shouldn't this be in bll.storage? Done Line 2: Line 3: import java.util.Collections; Line 4: import java.util.HashSet; Line 5: import java.util.List; http://gerrit.ovirt.org/#/c/22952/7/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveIscsiBondCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveIscsiBondCommand.java: Line 1: package org.ovirt.engine.core.bll; > shouldn't this be in bll.storage? Done Line 2: Line 3: import java.util.Collections; Line 4: import java.util.List; Line 5: Line 30: } Line 31: Line 32: @Override Line 33: protected void executeCommand() { Line 34: TransactionSupport.executeInNewTransaction(new TransactionMethod<Object>() { > Just make the command transactive Done Line 35: @Override Line 36: public Void runInTransaction() { Line 37: getDbFacade().getIscsiBondDao().remove(getParameters().getIscsiBondId()); Line 38: return null; http://gerrit.ovirt.org/#/c/22952/7/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java: Line 837: //mom policies Line 838: USER_UPDATED_MOM_POLICIES(10200), Line 839: USER_FAILED_TO_UPDATE_MOM_POLICIES(10201), Line 840: Line 841: // Iscsi bond > /s/Iscsi/iSCSI Done Line 842: ISCSI_BOND_ADD_SUCCESS(10300), Line 843: ISCSI_BOND_ADD_FAILED(10301), Line 844: ISCSI_BOND_EDIT_SUCCESS(10302), Line 845: ISCSI_BOND_EDIT_FAILED(10303), Line 838: USER_UPDATED_MOM_POLICIES(10200), Line 839: USER_FAILED_TO_UPDATE_MOM_POLICIES(10201), Line 840: Line 841: // Iscsi bond Line 842: ISCSI_BOND_ADD_SUCCESS(10300), > Please remember to change this code when rebasing since HA Reservation alre Done Line 843: ISCSI_BOND_ADD_FAILED(10301), Line 844: ISCSI_BOND_EDIT_SUCCESS(10302), Line 845: ISCSI_BOND_EDIT_FAILED(10303), Line 846: ISCSI_BOND_REMOVE_SUCCESS(10304), http://gerrit.ovirt.org/#/c/22952/7/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java: Line 32: VAR__TYPE__HOST_CAPABILITIES, Line 33: VAR__TYPE__NETWORK_QOS, Line 34: VAR__TYPE__SPM, Line 35: VAR__TYPE__CLUSTER_POLICY, Line 36: VAR__TYPE__POLICY_UNIT, > Please add VAR_TYPE_ISCSI_MULTIPATH Done Line 37: VAR__TYPE__SUBNET, Line 38: Line 39: // Gluster types Line 40: VAR__TYPE__GLUSTER_VOLUME, http://gerrit.ovirt.org/#/c/22952/7/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties: Line 1111: VALIDATION.ISCSI_BOND.DESCRIPTION.MAX=Iscsi bond description must not exceed 4000 characters Line 1112: VALIDATION.ISCSI_BOND.NAME.INVALID.CHARACTER=Iscsi Bond name must be formed from alpha-numeric characters or "-_." Line 1113: VALIDATION.ISCSI_BOND.DESCRIPTION.INVALID=Iscsi Bond description must be formed only from alpha-numeric characters and special characters that conform to the standard ASCII character set. Line 1114: Line 1115: ISCSI_BOND_NOT_EXISTS=The specified iscsi bond doesn't exist. > Please add here Cannot ${action} ${type}... Done http://gerrit.ovirt.org/#/c/22952/7/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties File frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties: Line 955: POWER_MANAGEMENT_ACTION_ON_ENTITY_ALREADY_IN_PROGRESS=Cannot perform ${action}. Another power management action is already in progress. Line 956: LABELED_NETWORK_INTERFACE_NOT_FOUND=A labeled network interface could not be found. Line 957: NETWORK_LABEL_CONFLICT=The networks represented by label cannot be configured on the same network interface. Line 958: Line 959: ISCSI_BOND_NOT_EXISTS=The specified iscsi bond doesn't exist. > Please add here Cannot ${action} ${type}. Done http://gerrit.ovirt.org/#/c/22952/7/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties File frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties: Line 1079: POWER_MANAGEMENT_ACTION_ON_ENTITY_ALREADY_IN_PROGRESS=Another power management action, ${action}, is already in progress. Line 1080: LABELED_NETWORK_INTERFACE_NOT_FOUND=A labeled network interface could not be found. Line 1081: NETWORK_LABEL_CONFLICT=The networks represented by label cannot be configured on the same network interface. Line 1082: Line 1083: ISCSI_BOND_NOT_EXISTS=The specified iscsi bond doesn't exist. > Please add here Cannot ${action} ${type}. Done -- To view, visit http://gerrit.ovirt.org/22952 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06890ea40d22b86d9421cb1b0c68ea28bc430b0a Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv <sgot...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
