Kanagaraj M has uploaded a new change for review.

Change subject: engine: Fix brick validation during create volume
......................................................................

engine: Fix brick validation during create volume

while creating a volume, the bricks will be validated
against the entries in gluster_volume_bricks table.

The above check is already present during add bricks(after volume creation),
now this is included in create volume as well.

Change-Id: Ie5c9afac29c2c982544b636c9d731ac39803cc7c
Bug-Url: https://bugzilla.redhat.com/856109
Signed-off-by: Kanagaraj M <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/AddBricksToGlusterVolumeCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/CreateGlusterVolumeCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterCommandBase.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterVolumeCommandBase.java
4 files changed, 33 insertions(+), 34 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/39/13239/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/AddBricksToGlusterVolumeCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/AddBricksToGlusterVolumeCommand.java
index 89ef846..ea3b599 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/AddBricksToGlusterVolumeCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/AddBricksToGlusterVolumeCommand.java
@@ -1,9 +1,7 @@
 package org.ovirt.engine.core.bll.gluster;
 
 import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
 
 import org.ovirt.engine.core.bll.Backend;
 import org.ovirt.engine.core.bll.LockIdNameAttribute;
@@ -62,8 +60,8 @@
             }
         }
 
-        return (updateBrickServerNames(getParameters().getBricks(), true)
-                && validateBricks(getParameters().getBricks()));
+        return updateBrickServerNames(getParameters().getBricks(), true)
+                && validateDuplicateBricks(getParameters().getBricks());
     }
 
     @Override
@@ -184,29 +182,6 @@
 
     private GlusterStatus getBrickStatus() {
         return getGlusterVolume().getStatus();
-    }
-
-    private boolean validateBricks(List<GlusterBrickEntity> newBricks) {
-        Set<String> bricks = new HashSet<String>();
-        for (GlusterBrickEntity brick : newBricks) {
-            if (bricks.contains(brick.getQualifiedName())) {
-                
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_DUPLICATE_BRICKS);
-                addCanDoActionMessage(String.format("$brick %1$s", 
brick.getQualifiedName()));
-                return false;
-            }
-            bricks.add(brick.getQualifiedName());
-
-            GlusterBrickEntity existingBrick =
-                    
getGlusterBrickDao().getBrickByServerIdAndDirectory(brick.getServerId(), 
brick.getBrickDirectory());
-            if (existingBrick != null) {
-                
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_BRICK_ALREADY_EXISTS_IN_VOLUME);
-                addCanDoActionMessage(String.format("$brick %1$s", 
brick.getQualifiedName()));
-                addCanDoActionMessage(String.format("$volumeName %1$s",
-                        
getGlusterVolumeDao().getById(existingBrick.getVolumeId()).getName()));
-                return false;
-            }
-        }
-        return true;
     }
 
     @Override
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/CreateGlusterVolumeCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/CreateGlusterVolumeCommand.java
index 6208f40..6c73668 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/CreateGlusterVolumeCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/CreateGlusterVolumeCommand.java
@@ -302,7 +302,7 @@
             break;
         }
 
-        return updateBrickServerNames(bricks, true);
+        return updateBrickServerNames(bricks, true) && 
validateDuplicateBricks(bricks);
     }
 
     private void setBrickOrder(List<GlusterBrickEntity> bricks) {
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterCommandBase.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterCommandBase.java
index d957349..8a91b0a 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterCommandBase.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterCommandBase.java
@@ -1,8 +1,10 @@
 package org.ovirt.engine.core.bll.gluster;
 
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.SystemUtils;
@@ -19,6 +21,7 @@
 import org.ovirt.engine.core.common.utils.Pair;
 import org.ovirt.engine.core.dal.VdcBllMessages;
 import org.ovirt.engine.core.dao.VdsStaticDAO;
+import org.ovirt.engine.core.dao.gluster.GlusterBrickDao;
 
 /**
  * Base class for all Gluster commands
@@ -115,7 +118,34 @@
         return true;
     }
 
+    protected boolean validateDuplicateBricks(List<GlusterBrickEntity> 
newBricks) {
+        Set<String> bricks = new HashSet<String>();
+        for (GlusterBrickEntity brick : newBricks) {
+            if (bricks.contains(brick.getQualifiedName())) {
+                
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_DUPLICATE_BRICKS);
+                addCanDoActionMessage(String.format("$brick %1$s", 
brick.getQualifiedName()));
+                return false;
+            }
+            bricks.add(brick.getQualifiedName());
+
+            GlusterBrickEntity existingBrick =
+                    
getGlusterBrickDao().getBrickByServerIdAndDirectory(brick.getServerId(), 
brick.getBrickDirectory());
+            if (existingBrick != null) {
+                
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_BRICK_ALREADY_EXISTS_IN_VOLUME);
+                addCanDoActionMessage(String.format("$brick %1$s", 
brick.getQualifiedName()));
+                addCanDoActionMessage(String.format("$volumeName %1$s",
+                        
getGlusterVolumeDao().getById(existingBrick.getVolumeId()).getName()));
+                return false;
+            }
+        }
+        return true;
+    }
+
     public VdsStaticDAO getVdsStaticDao() {
         return getDbFacade().getVdsStaticDao();
     }
+
+    protected GlusterBrickDao getGlusterBrickDao() {
+        return getDbFacade().getGlusterBrickDao();
+    }
 }
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterVolumeCommandBase.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterVolumeCommandBase.java
index d8d4ea6..aae9ad4 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterVolumeCommandBase.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterVolumeCommandBase.java
@@ -8,7 +8,6 @@
 import org.ovirt.engine.core.common.action.gluster.GlusterVolumeParameters;
 import org.ovirt.engine.core.common.businessentities.VDSGroup;
 import org.ovirt.engine.core.dal.VdcBllMessages;
-import org.ovirt.engine.core.dao.gluster.GlusterBrickDao;
 import org.ovirt.engine.core.dao.gluster.GlusterOptionDao;
 
 /**
@@ -20,10 +19,6 @@
     public GlusterVolumeCommandBase(T params) {
         super(params);
         setGlusterVolumeId(getParameters().getVolumeId());
-    }
-
-    protected GlusterBrickDao getGlusterBrickDao() {
-        return getDbFacade().getGlusterBrickDao();
     }
 
     protected GlusterOptionDao getGlusterOptionDao() {
@@ -51,7 +46,6 @@
 
         return true;
     }
-
 
     @Override
     public List<PermissionSubject> getPermissionCheckSubjects() {


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie5c9afac29c2c982544b636c9d731ac39803cc7c
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Kanagaraj M <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to