Alon Bar-Lev has posted comments on this change.

Change subject: engine:changed AddVdsCommand to use LockManager properly
......................................................................


Patch Set 2:

(1 comment)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
Line 573:     protected Map<String, Pair<String, String>> getExclusiveLocks() {
Line 574:         VDSGroup cluster = getVdsGroup();
Line 575:         if (cluster != null && cluster.supportsGlusterService() && 
!isInternalExecution()) {
Line 576:             return 
Collections.singletonMap(cluster.getId().toString(),
Line 577:                     
LockMessagesMatchUtil.makeLockingPair(LockingGroup.GLUSTER, 
VdcBllMessages.ACTION_TYPE_FAILED_GLUSTER_OPERATION_INPROGRESS));
reading the code and trying to make any sense out of it...

I think that the gluster functionality should be split off this command 
entirely.

We should have AddVds command, that triggers either InstallVds or 
InstallGluster based on ConfigValues.InstallVds value, in InstallGluster do all 
gluster specific magic.

I do not see any reason for the generic simple AddVds command to handle gluster 
specific issues, maybe apart from isValidGlusterPeer, not sure.
Line 578:         }
Line 579:         return null;
Line 580:     }


-- 
To view, visit http://gerrit.ovirt.org/18350
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6189d7f0046a943a58db9d6734c92fe462c9db52
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Sahina Bose <sab...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Sahina Bose <sab...@redhat.com>
Gerrit-Reviewer: Shubhendu Tripathi <shtri...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to