Sahina Bose has posted comments on this change. Change subject: engine: Enable gluster hook command ......................................................................
Patch Set 3: (6 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/EnableGlusterHookCommand.java Line 49: if ( Guid.isNullOrEmpty(getParameters().getHookId())) { Line 50: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_GLUSTER_HOOK_ID_IS_REQUIRED); Line 51: return false; Line 52: } Line 53: Up server check is already done in superclass. Can remove from here. Line 54: List<Guid> upServerIds = getUpServerIds(); Line 55: if (upServerIds == null || upServerIds.isEmpty()) { Line 56: addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_NO_UP_SERVER_FOUND); Line 57: return false; Line 57: return false; Line 58: } Line 59: return true; Line 60: } Line 61: Is this public method required here? Can getAllUpServers be used instead? Line 62: public List<Guid> getUpServerIds() { Line 63: List<Guid> upServerIds = new ArrayList<Guid>(); Line 64: for (VDS server : getAllUpServers(getParameters().getClusterId())) { Line 65: upServerIds.add(server.getId()); Line 86: Line 87: if (!errors.isEmpty()) { Line 88: handleVdsErrors(AuditLogType.GLUSTER_HOOK_ENABLE_FAILED, errors); Line 89: } Line 90: Shouldn't status be set to enabled in DB only if hook enable succeeded on all servers in the cluster? Line 91: if (getUpServerIds().size() > errors.size()) { Line 92: UpdateHookStatusInDb(getParameters().getClusterId(), Line 93: entity.getName(), GlusterHookStatus.ENABLED); Line 94: setSucceeded(true); .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllErrors.java Line 346: GlusterVolumeProfileStopFailed(4159), Line 347: GlusterVolumeProfileInfoFailed(4160), Line 348: GlusterAddHostFailed(4404), Line 349: RemoveGlusterServerFailed(4406), Line 350: GlusterPeerListFailed(4407), GlusterHookFailed - when would this be returned? Line 351: GlusterHookFailed(4500), Line 352: GlusterHookEnableFailed(4501), Line 353: GlusterHookAlreadyEnabled(4503), Line 354: GlusterHookNotFound(4505), .................................................... File backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties Line 576: GLUSTER_VOLUME_OPTION_RESET_FROM_CLI=Option ${option} was reset on Volume ${glusterVolumeName} from gluster CLI. Now reset in engine DB as well. Line 577: GLUSTER_VOLUME_PROPERTIES_CHANGED_FROM_CLI=Gluster Volume ${glusterVolumeName} properties were changed from CLI. Now updated in engine DB as well. Line 578: GLUSTER_VOLUME_BRICK_ADDED_FROM_CLI=Brick ${brick} was added to Volume ${glusterVolumeName} from gluster CLI. Now added in engine DB as well. Line 579: GLUSTER_VOLUME_BRICK_REMOVED_FROM_CLI=Brick ${brick} was removed from Volume ${glusterVolumeName} from gluster CLI. Now removed in engine DB as well. Line 580: GLUSTER_SERVER_REMOVED_FROM_CLI=Server ${VdsName} was removed from Cluster ${VdsGroupName} from gluster CLI. Now removed in engine DB as well. Gluster_hook messages should also log the hook names Line 581: GLUSTER_HOOK_ENABLE=Gluster Hook enabled on ${VdsName}. .................................................... File backend/manager/modules/dal/src/main/resources/bundles/ExecutionMessages.properties Line 81: job.GlusterVolumeRemoveBricks=Removing Bricks from Gluster Volume ${GlusterVolume} Line 82: job.StartRebalanceGlusterVolume=Starting Rebalance on Gluster Volume ${GlusterVolume} Line 83: job.ReplaceGlusterVolumeBrick=Replacing Brick in Gluster Volume ${GlusterVolume} Line 84: job.AddBricksToGlusterVolume=Adding Bricks to Gluster Volume ${GlusterVolume} Line 85: job.RemoveGlusterServer=Removing Gluster Server ${VDS} Gluster hookname should be a parameter in the message. Line 86: job.EnableGlusterHook=Enabling Gluster Hook on ${VDS} Line 87: Line 88: # Step types Line 89: step.VALIDATING=Validating -- To view, visit http://gerrit.ovirt.org/10906 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icc6f9c77393ebed2803ec2a1b295a09f61642c31 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Selvasundaram <sesub...@redhat.com> Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Sahina Bose <sab...@redhat.com> Gerrit-Reviewer: Selvasundaram <sesub...@redhat.com> Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches