Shireesh Anjal has posted comments on this change.
Change subject: engine: Get Volume Advanced Details Query
......................................................................
Patch Set 7: (6 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GetGlusterVolumeAdvancedDetailsQuery.java
Line 30: getParameters().getVolumeName(),
Line 31: getParameters().getBrickName(),
Line 32:
getParameters().detailsRequired()));
Line 33:
getQueryReturnValue().setReturnValue(returnValue.getReturnValue());
Line 34: } catch (Exception e) {
What's the point of catching the exception and then throwing it again, without
doing anything in the catch block? There is no need to even wrap it inside a
RuntimeException, since what is being thrown inside getUpServerId() is already
a RuntimeException.
Line 35: throw new RuntimeException(e);
Line 36: }
Line 37: }
Line 38:
Line 35: throw new RuntimeException(e);
Line 36: }
Line 37: }
Line 38:
Line 39: private Guid getUpServerId() {
I think it is better to pass the cluster id as an argument to this method.
Line 40: VDS vds =
getClusterUtils().getUpServer(getParameters().getClusterId());
Line 41: if (vds == null) {
Line 42: throw new RuntimeException("No up server found");
Line 43: }
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllErrors.java
Line 340: GlusterVolumeReplaceBrickStartFailed(4142),
Line 341: GlusterHostRemoveFailed(4406),
Line 342: GlusterAddHostFailed(4404),
Line 343: GlusterPeerListFailed(4407),
Line 344: GlusterVolumeStatusFailed(4157),
Please arrange the gluster related error codes in the correct sequence
(ascending order of error numbers)
Line 345:
Line 346:
Line 347: UnicodeArgumentException(4900),
Line 348:
....................................................
File
backend/manager/modules/dal/src/main/resources/bundles/VdsmErrors.properties
Line 321: GlusterVolumeReplaceBrickStartFailed=Gluster Volume Replace Brick
Start Failed
Line 322: GlusterHostRemoveFailed=Gluster Server Remove Failed
Line 323: GlusterAddHostFailed=Gluster Server Add Failed
Line 324: GlusterPeerListFailed=Gluster Peer List Failed
Line 325: GlusterVolumeStatusFailed=Gluster Volume Status Failed
Change description to 'Could not fetch advanced volume details'
Line 326:
Line 327: CANT_RECONSTRUCT_WHEN_A_DOMAIN_IN_POOL_IS_LOCKED=Can't reconstruct
the Master Domain when the Data Center contains Domains in Locked
state.\nPlease wait until the operation for these Domains ends before trying to
reconstruct the Master Domain again.
Line 328: NO_IMPLEMENTATION=Not implemented
Line 329: FailedToPlugDisk=Failed to hot-plug disk
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/GlusterVolumeStatusReturnForXmlRpc.java
Line 85: volumeAdvancedDetails.setVolumeId(volume.getId());
Line 86: List<BrickDetails> brickDetails =
prepareBrickDetails(volume, (Object[]) statusInfo.get(BRICKS));
Line 87: volumeAdvancedDetails.setBrickDetails(brickDetails);
Line 88:
Line 89: if (statusInfo.containsKey(NFS_KEY) ||
statusInfo.containsKey(SHD_KEY)) {
The containsKey() check is happening anyway in getServiceInfo(). So I think it
is not required here.
Line 90:
volumeAdvancedDetails.setServiceInfo(prepareServiceInfo(statusInfo));
Line 91: }
Line 92: }
Line 93: }
Line 102: getServiceInfo(statusInfo, serviceInfoList, ServiceType.SHD);
Line 103: return serviceInfoList;
Line 104: }
Line 105:
Line 106: private void getServiceInfo(Map<String, Object> statusInfo,
List<ServiceInfo> serviceInfoList, ServiceType serviceType) {
I think the last argument can be the KEY itself instead of taking the
ServiceType and then arriving at the key.
Line 107: String service = (serviceType == ServiceType.NFS) ? NFS_KEY :
SHD_KEY;
Line 108: if (statusInfo.containsKey(service)) {
Line 109: Object[] serviceInfo = (Object[]) statusInfo.get(service);
Line 110: for (Object serviceObj : serviceInfo) {
--
To view, visit http://gerrit.ovirt.org/7807
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If6b590eaebeb1d06b7278300d5e12b2dab9eb093
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dhandapani Gopal <dgo...@redhat.com>
Gerrit-Reviewer: Dhandapani Gopal <dgo...@redhat.com>
Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com>
Gerrit-Reviewer: Selvasundaram <sesub...@redhat.com>
Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
