Sahina Bose has posted comments on this change. Change subject: engine : Bll Command for Create Geo Rep session ......................................................................
Patch Set 18: (8 comments) http://gerrit.ovirt.org/#/c/29834/18/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterVolumeGeoRepSessionCreateCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterVolumeGeoRepSessionCreateCommand.java: Line 14: import org.ovirt.engine.core.common.vdscommands.gluster.GlusterVolumeGeoRepSessionVDSParameters; Line 15: Line 16: public class GlusterVolumeGeoRepSessionCreateCommand extends GeoRepSessionCommandBase<GlusterVolumeGeoRepCreateParameters>{ Line 17: Line 18: GlusterVolumeEntity masterVolume; private Line 19: Line 20: public GlusterVolumeGeoRepSessionCreateCommand(GlusterVolumeGeoRepCreateParameters params) { Line 21: super(params); Line 22: } Line 73: if (passwordlessSSHCreateReturnValue.getSucceeded()) { Line 74: sessionCreateReturnValue = runVdsCommand(VDSCommandType.GlusterVolumeGeoRepSessionCreate, new GlusterVolumeGeoRepSessionVDSParameters(upServer.getId(), masterVolume.getName(), slaveHost, parameter.getSlaveVolumeName(), parameter.isForce())); Line 75: if (!sessionCreateReturnValue.getSucceeded()) { Line 76: handleVdsError(AuditLogType.GLUSTER_VOLUME_GEO_REP_CREATE_FAILED, sessionCreateReturnValue.getVdsError().getMessage()); Line 77: return; setSucceeded value to be set before returning? Line 78: } Line 79: setSucceeded(sessionCreateReturnValue.getSucceeded()); Line 80: GlusterGeoRepSession newSession = new GlusterGeoRepSession(); Line 81: newSession.setMasterVolumeId(masterVolume.getId()); Line 81: newSession.setMasterVolumeId(masterVolume.getId()); Line 82: newSession.setMasterVolumeName(masterVolume.getName()); Line 83: newSession.setSlaveHostName(slaveHost); Line 84: newSession.setSlaveVolumeName(parameter.getSlaveVolumeName()); Line 85: newSession.setSlaveVolumeId(getGlusterVolumeDao().getByName(getVdsDAO().getByName(slaveHost).getVdsGroupId(), parameter.getSlaveVolumeName()).getId()); possible NPE. Line 86: newSession.setSlaveNodeUuid(getVdsDAO().getByName(slaveHost).getId()); Line 87: getGlusterGeoRepDao().save(newSession); Line 88: } else { Line 89: handleVdsError(AuditLogType.GLUSTER_VOLUME_PASSWORD_LESS_SSH_FAILED, passwordlessSSHCreateReturnValue.getVdsError().getMessage()); Line 82: newSession.setMasterVolumeName(masterVolume.getName()); Line 83: newSession.setSlaveHostName(slaveHost); Line 84: newSession.setSlaveVolumeName(parameter.getSlaveVolumeName()); Line 85: newSession.setSlaveVolumeId(getGlusterVolumeDao().getByName(getVdsDAO().getByName(slaveHost).getVdsGroupId(), parameter.getSlaveVolumeName()).getId()); Line 86: newSession.setSlaveNodeUuid(getVdsDAO().getByName(slaveHost).getId()); same here Line 87: getGlusterGeoRepDao().save(newSession); Line 88: } else { Line 89: handleVdsError(AuditLogType.GLUSTER_VOLUME_PASSWORD_LESS_SSH_FAILED, passwordlessSSHCreateReturnValue.getVdsError().getMessage()); Line 90: setSucceeded(false); Line 88: } else { Line 89: handleVdsError(AuditLogType.GLUSTER_VOLUME_PASSWORD_LESS_SSH_FAILED, passwordlessSSHCreateReturnValue.getVdsError().getMessage()); Line 90: setSucceeded(false); Line 91: } Line 92: getReturnValue().setActionReturnValue(sessionCreateReturnValue); Should you be setting the bll return value to a VDSReturnValue? Or did you mean to set the return value to the created georep session id? Line 93: } http://gerrit.ovirt.org/#/c/29834/18/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java: Line 300: StopRemoveGlusterVolumeBricks(1423, ActionGroup.MANIPULATE_GLUSTER_VOLUME, false, QuotaDependency.NONE), Line 301: CommitRemoveGlusterVolumeBricks(1424, ActionGroup.MANIPULATE_GLUSTER_VOLUME, false, QuotaDependency.NONE), Line 302: RefreshGlusterVolumeDetails(1425, ActionGroup.MANIPULATE_GLUSTER_VOLUME, QuotaDependency.NONE), Line 303: GlusterVolumeGeoRepStart(1426, ActionGroup.MANIPULATE_GLUSTER_VOLUME, QuotaDependency.NONE), Line 304: GlusterVolumeGeoRepSessionCreate(1427, ActionGroup.MANIPULATE_GLUSTER_VOLUME, QuotaDependency.NONE), Can you change to follow convention - verb first? Line 305: Line 306: // Scheduling Policy Line 307: AddClusterPolicy(1450, ActionGroup.EDIT_STORAGE_POOL_CONFIGURATION, false, QuotaDependency.NONE), Line 308: EditClusterPolicy(1451, ActionGroup.EDIT_STORAGE_POOL_CONFIGURATION, false, QuotaDependency.NONE), http://gerrit.ovirt.org/#/c/29834/18/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties: Line 1122: ACTION_TYPE_FAILED_GLUSTER_VOLUME_CANNOT_STOP_REMOVE_BRICK_IN_PROGRESS= Cannot ${action} ${type}. Remove brick operation is running on the volume ${volumeName} in cluster ${vdsGroup}. Line 1123: ACTION_TYPE_FAILED_GLUSTER_OPERATION_INPROGRESS=Cannot ${action} ${type}. Gluster operation is in progress in cluster. Please try again. Line 1124: ACTION_TYPE_FAILED_GEOREP_SESSION_INVALID=Cannot ${action} ${type}. Geo-replication session not found. Line 1125: ACTION_TYPE_FAILED_GEOREP_SESSION_LOCKED=Cannot ${action} ${type}. Another action is in progress on this geo-replication session. Please try again. Line 1126: ACTION_TYPE_FAILED_GEOREP_SESSION_CREATED=Cannot ${action} ${type}. Geo-replication session is created. already created? Line 1127: ACTION_TYPE_FAILED_TAG_ID_REQUIRED=Cannot ${action} ${type}. Tag ID is required. Line 1128: Line 1129: ACTION_TYPE_FAILED_QOS_OUT_OF_RANGE_VALUES=Cannot ${action} ${type}. Values are out of range. Line 1130: ACTION_TYPE_FAILED_QOS_STORAGE_POOL_NOT_EXIST=Cannot ${action} ${type}. Invalid data center. http://gerrit.ovirt.org/#/c/29834/18/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java File frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java: Line 3047: Line 3048: @DefaultStringValue("Cannot ${action} ${type}. Another action is in progress on this geo-replication session. Please try again.") Line 3049: String ACTION_TYPE_FAILED_GEOREP_SESSION_LOCKED(); Line 3050: Line 3051: @DefaultStringValue("Cannot ${action} ${type}. Geo-replication session is created.") already created? Line 3052: String ACTION_TYPE_FAILED_GEOREP_SESSION_CREATED(); Line 3053: Line 3054: @DefaultStringValue("Cannot ${action} ${type}. All three values are needed in order to define QoS on each network directions.") Line 3055: String ACTION_TYPE_FAILED_NETWORK_QOS_MISSING_VALUES(); -- To view, visit http://gerrit.ovirt.org/29834 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iccc92235deea42d7818336b2402476193cbe789c Gerrit-PatchSet: 18 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: anmolbabu <[email protected]> Gerrit-Reviewer: Kanagaraj M <[email protected]> Gerrit-Reviewer: Ramesh N <[email protected]> Gerrit-Reviewer: Sahina Bose <[email protected]> Gerrit-Reviewer: Shubhendu Tripathi <[email protected]> Gerrit-Reviewer: anmolbabu <[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
