Omer Frenkel has posted comments on this change.
Change subject: engine: check and peer probe gluster servers
......................................................................
Patch Set 1: (7 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java
Line 185: return type;
Line 186: }
Line 187:
Line 188: private void initGlusterPeerProcess() {
Line 189: List<VDS> vdsList =
DbFacade.getInstance().getVdsDAO().getAllForVdsGroup(getVdsGroupId());
you can use:
getVdsDao().getAllForVdsGroupWithStatus(getVdsGroupId(), VDSStatus.Up);
to get only up servers in the cluster
Line 190: // If the cluster already having Gluster servers, get an up
server
Line 191: if (vdsList != null && vdsList.size() > 1) {
Line 192: VDS upServer = null;
Line 193: for (int i = 0; i < vdsList.size(); i++) {
Line 189: List<VDS> vdsList =
DbFacade.getInstance().getVdsDAO().getAllForVdsGroup(getVdsGroupId());
Line 190: // If the cluster already having Gluster servers, get an up
server
Line 191: if (vdsList != null && vdsList.size() > 1) {
Line 192: VDS upServer = null;
Line 193: for (int i = 0; i < vdsList.size(); i++) {
why not use for (VDS vds : vdsList) ?
Line 194: if (!Guid.OpEquality(getVdsId(),
vdsList.get(i).getId())
Line 195: && vdsList.get(i).getstatus() ==
VDSStatus.Up) {
Line 196: upServer = vdsList.get(i);
Line 197: break;
Line 198: }
Line 199: }
Line 200: // If new server is not part of the existing gluster
peers, add into peer group
Line 201: if (upServer != null) {
Line 202: if (!hostExists(getGlusterPeers(upServer.getId()),
upServer.getId())) {
are you sure second parameter shouldn't be getVdsId() ?
means:
if (!hostExists(getGlusterPeers(upServer.getId()), getVdsId())) {...
Line 203: String newServerName =
Line 204: getVds().gethost_name().isEmpty() ?
getVds().getManagmentIp() : getVds().gethost_name();
Line 205: glusterPeerProbe(upServer.getId(), newServerName);
Line 206: }
Line 222: VDSReturnValue returnValue = Backend.getInstance()
Line 223: .getResourceManager()
Line 224: .RunVdsCommand(VDSCommandType.GlusterHostsList,
Line 225: new
VdsIdVDSCommandParametersBase(upServerId));
Line 226: setSucceeded(returnValue.getSucceeded());
please dont set the commands succeeded here, it is set to true at the end of
the command.
Line 227: if (getSucceeded()) {
Line 228: glusterServers = (List<VDS>) returnValue.getReturnValue();
Line 229: } else {
Line 230:
getReturnValue().getFault().setError(returnValue.getVdsError().getCode());
Line 228: glusterServers = (List<VDS>) returnValue.getReturnValue();
Line 229: } else {
Line 230:
getReturnValue().getFault().setError(returnValue.getVdsError().getCode());
Line 231:
getReturnValue().getFault().setMessage(returnValue.getVdsError().getMessage());
Line 232: AuditLogDirector.log(new AuditLogableBase(upServerId),
AuditLogType.GLUSTER_HOST_LIST_FAILED);
audit log should be on the host which is initializing (getVdsId()), no?
Line 233: }
Line 234: return glusterServers;
Line 235: }
Line 236:
Line 238: VDSReturnValue returnValue =
Line 239: runVdsCommand(
Line 240: VDSCommandType.GlusterHostAdd,
Line 241: new GlusterHostAddVDSParameters(upServerId,
newServerName));
Line 242: setSucceeded(returnValue.getSucceeded());
please dont set the commands succeeded here, it is set to true at the end of
the command.
Line 243: if (!getSucceeded()) {
Line 244:
getReturnValue().getFault().setError(returnValue.getVdsError().getCode());
Line 245:
getReturnValue().getFault().setMessage(returnValue.getVdsError().getMessage());
Line 246: AuditLogDirector.log(new AuditLogableBase(upServerId),
AuditLogType.GLUSTER_HOST_ADD_FAILED);
Line 242: setSucceeded(returnValue.getSucceeded());
Line 243: if (!getSucceeded()) {
Line 244:
getReturnValue().getFault().setError(returnValue.getVdsError().getCode());
Line 245:
getReturnValue().getFault().setMessage(returnValue.getVdsError().getMessage());
Line 246: AuditLogDirector.log(new AuditLogableBase(upServerId),
AuditLogType.GLUSTER_HOST_ADD_FAILED);
audit log should be on the host which is initializing (getVdsId()), no?
Line 247: return;
Line 248: }
Line 249: }
Line 250:
--
To view, visit http://gerrit.ovirt.org/7243
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I13c8de35ac596d89d2a9c631c4a8ef26996ea860
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Selvasundaram <sesub...@redhat.com>
Gerrit-Reviewer: Dhandapani Gopal <dgo...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
