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

Reply via email to