Shireesh Anjal has uploaded a new change for review.

Change subject: gluster: Update volumes by id instead of name
......................................................................

gluster: Update volumes by id instead of name

Modifying the logic to compare fetched volumes with existing volumes
to use volume ids instead of name. This handles the rare scenario
where someone deletes a volume and creates a new one with exactly
same name from the gluster CLI.

Change-Id: If9053da4294b7c5aa762645737a2effccd5a9c4d
Bug-Url: https://bugzilla.redhat.com/888144
Signed-off-by: Shireesh Anjal <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterManager.java
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/gluster/GlusterVolumesListReturnForXmlRpc.java
2 files changed, 14 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/29/10429/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterManager.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterManager.java
index 3835829..ea973a9 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterManager.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterManager.java
@@ -408,7 +408,7 @@
         acquireLock(cluster.getId());
         try {
             // Pass a copy of the existing servers as the fetchVolumes method 
can potentially remove elements from it
-            Map<String, GlusterVolumeEntity> volumesMap = 
fetchVolumes(upServer, new ArrayList<VDS>(existingServers));
+            Map<Guid, GlusterVolumeEntity> volumesMap = fetchVolumes(upServer, 
new ArrayList<VDS>(existingServers));
             if (volumesMap == null) {
                 log.errorFormat("gluster volume info command failed on all 
servers of the cluster {0}."
                         + "Can't refresh it's data at this point.", 
cluster.getname());
@@ -431,8 +431,8 @@
      * @param existingServers
      * @return
      */
-    private Map<String, GlusterVolumeEntity> fetchVolumes(VDS upServer, 
List<VDS> existingServers) {
-        Map<String, GlusterVolumeEntity> fetchedVolumes = null;
+    private Map<Guid, GlusterVolumeEntity> fetchVolumes(VDS upServer, 
List<VDS> existingServers) {
+        Map<Guid, GlusterVolumeEntity> fetchedVolumes = null;
         while (fetchedVolumes == null && existingServers.size() > 0) {
             fetchedVolumes = fetchVolumes(upServer);
             if (fetchedVolumes == null) {
@@ -447,18 +447,18 @@
     }
 
     @SuppressWarnings("unchecked")
-    protected Map<String, GlusterVolumeEntity> fetchVolumes(VDS upServer) {
+    protected Map<Guid, GlusterVolumeEntity> fetchVolumes(VDS upServer) {
         VDSReturnValue result =
                 runVdsCommand(VDSCommandType.GlusterVolumesList, new 
GlusterVolumesListVDSParameters(upServer.getId(),
                         upServer.getvds_group_id()));
 
-        return result.getSucceeded() ? (Map<String, GlusterVolumeEntity>) 
result.getReturnValue() : null;
+        return result.getSucceeded() ? (Map<Guid, GlusterVolumeEntity>) 
result.getReturnValue() : null;
     }
 
-    private void removeDeletedVolumes(Guid clusterId, Map<String, 
GlusterVolumeEntity> volumesMap) {
+    private void removeDeletedVolumes(Guid clusterId, Map<Guid, 
GlusterVolumeEntity> volumesMap) {
         List<Guid> idsToRemove = new ArrayList<Guid>();
         for (GlusterVolumeEntity volume : 
getVolumeDao().getByClusterId(clusterId)) {
-            if (!volumesMap.containsKey(volume.getName())) {
+            if (!volumesMap.containsKey(volume.getId())) {
                 idsToRemove.add(volume.getId());
                 log.debugFormat("Volume {0} has been removed directly using 
the gluster CLI. Removing it from engine as well.",
                         volume.getName());
@@ -475,12 +475,12 @@
         }
     }
 
-    private void updateExistingAndNewVolumes(Guid clusterId, Map<String, 
GlusterVolumeEntity> volumesMap) {
-        for (Entry<String, GlusterVolumeEntity> entry : volumesMap.entrySet()) 
{
+    private void updateExistingAndNewVolumes(Guid clusterId, Map<Guid, 
GlusterVolumeEntity> volumesMap) {
+        for (Entry<Guid, GlusterVolumeEntity> entry : volumesMap.entrySet()) {
             GlusterVolumeEntity volume = entry.getValue();
             log.debugFormat("Analyzing volume {0}", volume.getName());
 
-            GlusterVolumeEntity existingVolume = 
getVolumeDao().getByName(clusterId, volume.getName());
+            GlusterVolumeEntity existingVolume = 
getVolumeDao().getById(entry.getKey());
             if (existingVolume == null) {
                 try {
                     createVolume(volume);
diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/gluster/GlusterVolumesListReturnForXmlRpc.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/gluster/GlusterVolumesListReturnForXmlRpc.java
index c04e0e5..322e935 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/gluster/GlusterVolumesListReturnForXmlRpc.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/gluster/GlusterVolumesListReturnForXmlRpc.java
@@ -38,7 +38,7 @@
     private static final String STRIPE_COUNT = "stripeCount";
 
     private Guid clusterId;
-    private Map<String, GlusterVolumeEntity> volumes = new HashMap<String, 
GlusterVolumeEntity>();
+    private Map<Guid, GlusterVolumeEntity> volumes = new HashMap<Guid, 
GlusterVolumeEntity>();
     private static Log log = 
LogFactory.getLog(GlusterVolumesListReturnForXmlRpc.class);
 
     @SuppressWarnings("unchecked")
@@ -55,7 +55,8 @@
         for (Entry<String, Object> entry : volumesMap.entrySet()) {
             log.debugFormat("received volume {0}", entry.getKey());
 
-            volumes.put(entry.getKey(), getVolume((Map<String, 
Object>)entry.getValue()));
+            GlusterVolumeEntity volume = getVolume((Map<String, 
Object>)entry.getValue());
+            volumes.put(volume.getId(), volume);
         }
     }
 
@@ -219,7 +220,7 @@
         return null;
     }
 
-    public Map<String, GlusterVolumeEntity> getVolumes() {
+    public Map<Guid, GlusterVolumeEntity> getVolumes() {
         return volumes;
     }
 }


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

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

Reply via email to