[Engine-patches] Change in ovirt-engine[ovirt-engine-3.3]: engine : Updating host's cluster leads to cloning the host

[yzaslavs]

Hello Ravi Nori,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/18400

to review the following change.

Change subject: engine : Updating host's cluster leads to cloning the host
......................................................................

engine : Updating host's cluster leads to cloning the host

When trying to move host from one gluster cluster to
another, under certain conditions the operation fails
with an internal server error and the host appears in
both clusters.

This happens only when the host in the target or
source cluster was up during canDoAction and is down
during execution of glusterHostRemove or
glusterHostAdd restulting in NPE.

Change-Id: I1bda0efedd62bf70ae8670861dc3a2ee70ff77dc
Bug-Url: https://bugzilla.redhat.com/988033
Signed-off-by: Ravi Nori <rn...@redhat.com>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeVDSClusterCommand.java
1 file changed, 24 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/00/18400/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeVDSClusterCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeVDSClusterCommand.java
index 13cba26..7b88110 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeVDSClusterCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeVDSClusterCommand.java
@@ -158,6 +158,7 @@
 
         if (getSourceCluster().supportsGlusterService() && 
getClusterUtils().hasServers(getSourceCluster().getId())) {
             if (!glusterHostRemove(getSourceCluster().getId())) {
+                setSucceeded(false);
                 return;
             }
         }
@@ -165,6 +166,7 @@
         if (getTargetCluster().supportsGlusterService()
                 && 
getClusterUtils().hasMultipleServers(getTargetCluster().getId())) {
             if (!glusterHostAdd(getTargetCluster().getId())) {
+                setSucceeded(false);
                 return;
             }
         }
@@ -201,10 +203,17 @@
         String hostName =
                 (getVds().getHostName().isEmpty()) ? getVds().getManagementIp()
                         : getVds().getHostName();
+        VDS runningHostInSourceCluster = 
getClusterUtils().getUpServer(sourceClusterId);
+        if (runningHostInSourceCluster == null) {
+            log.error("Cannot remove host from source cluster, no host in Up 
status found in source cluster");
+            handleError(-1, "No host in Up status found in source cluster");
+            errorType = AuditLogType.GLUSTER_SERVER_REMOVE_FAILED;
+            return false;
+        }
         VDSReturnValue returnValue =
                 runVdsCommand(
                         VDSCommandType.RemoveGlusterServer,
-                        new 
RemoveGlusterServerVDSParameters((getClusterUtils().getUpServer(sourceClusterId)).getId(),
+                        new 
RemoveGlusterServerVDSParameters(runningHostInSourceCluster.getId(),
                                 hostName,
                                 false));
 
@@ -220,10 +229,17 @@
         String hostName =
                 (getVds().getHostName().isEmpty()) ? getVds().getManagementIp()
                         : getVds().getHostName();
+        VDS runningHostInTargetCluster = 
getClusterUtils().getUpServer(targetClusterId);
+        if (runningHostInTargetCluster == null) {
+            log.error("Cannot add host to target cluster, no host in Up status 
found in target cluster");
+            handleError(-1, "No host in Up status found in target cluster");
+            errorType = AuditLogType.GLUSTER_SERVER_ADD_FAILED;
+            return false;
+        }
         VDSReturnValue returnValue =
                 runVdsCommand(
                         VDSCommandType.AddGlusterServer,
-                        new 
AddGlusterServerVDSParameters(getClusterUtils().getUpServer(targetClusterId).getId(),
+                        new 
AddGlusterServerVDSParameters(runningHostInTargetCluster.getId(),
                                 hostName));
         if (!returnValue.getSucceeded()) {
             handleVdsError(returnValue);
@@ -233,6 +249,12 @@
         return true;
     }
 
+    private void handleError(int errorCode, String errorMsg) {
+        getReturnValue().getFault().setError(errorCode);
+        getReturnValue().getFault().setMessage(errorMsg);
+        getReturnValue().getExecuteFailedMessages().add(errorMsg);
+    }
+
     private ClusterUtils getClusterUtils() {
         return ClusterUtils.getInstance();
     }


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1bda0efedd62bf70ae8670861dc3a2ee70ff77dc
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.3
Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches