Merge pull request #250 from Selvasundaram/master

Deleting CIFS config throws null pointer exception bug - fix 

7 files changed, 118 insertions, 80 deletions

diff --git a/src/com.gluster.storage.management.console/src/com/gluster/storage/management/console/GlusterDataModelManager.java b/src/com.gluster.storage.management.console/src/com/gluster/storage/management/console/GlusterDataModelManager.java
index 46f4393b..1e3dad3f 100644
--- a/src/com.gluster.storage.management.console/src/com/gluster/storage/management/console/GlusterDataModelManager.java
+++ b/src/com.gluster.storage.management.console/src/com/gluster/storage/management/console/GlusterDataModelManager.java
@@ -631,7 +631,7 @@ public class GlusterDataModelManager {
 	public List<Device> getReadyDevicesOfServer(String serverName, List<Device> excludeDevices) {
 		List<Device> devices = new ArrayList<Device>();
 		GlusterServer server = model.getCluster().getServer(serverName);
-		if (!server.isOnline()) {
+		if (server == null || !server.isOnline()) {
 			return devices;
 		}
 		for (Disk disk : server.getDisks()) {
diff --git a/src/com.gluster.storage.management.console/src/com/gluster/storage/management/console/views/VolumeSummaryView.java b/src/com.gluster.storage.management.console/src/com/gluster/storage/management/console/views/VolumeSummaryView.java
index dfd0c669..a12fcc0d 100644
--- a/src/com.gluster.storage.management.console/src/com/gluster/storage/management/console/views/VolumeSummaryView.java
+++ b/src/com.gluster.storage.management.console/src/com/gluster/storage/management/console/views/VolumeSummaryView.java
@@ -419,17 +419,21 @@ public class VolumeSummaryView extends ViewPart {
 					VolumesClient vc = new VolumesClient();
 					Volume newVolume = new Volume();
 					String cifsUsers = cifsUsersText.getText().trim();
-					try {
-						vc.setCifsConfig(volume.getName(), cifsCheckbox.getSelection(), cifsUsers);
-						enableCifsUsersControls(false);
-						newVolume = vc.getVolume(volume.getName());
-						modelManager.volumeChanged(volume, newVolume);
-					} catch (Exception e) {
-						MessageDialog.openError(Display.getDefault().getActiveShell(), "Cifs Configuration",
-								e.getMessage());
-						cifsCheckbox.setSelection(volume.isCifsEnable());
-						enableCifsUsersControls(cifsCheckbox.getSelection());
-						populateCifsUsersText();
+					// If no cifs users and removing cifs config, nothing to do
+					if (!(!cifsCheckbox.getSelection() && volume.getCifsUsers().toString() == "[]" && (cifsUsers
+							.isEmpty() || cifsUsers.equals("")))) {
+						try {
+							vc.setCifsConfig(volume.getName(), cifsCheckbox.getSelection(), cifsUsers);
+							enableCifsUsersControls(false);
+							newVolume = vc.getVolume(volume.getName());
+							modelManager.volumeChanged(volume, newVolume);
+						} catch (Exception e) {
+							MessageDialog.openError(Display.getDefault().getActiveShell(), "Cifs Configuration",
+									e.getMessage());
+							cifsCheckbox.setSelection(volume.isCifsEnable());
+							enableCifsUsersControls(cifsCheckbox.getSelection());
+							populateCifsUsersText();
+						}
 					}
 				}
 			});
@@ -573,16 +577,23 @@ public class VolumeSummaryView extends ViewPart {
 				} else {
 					// need to disable cifs
 					// TODO: hide the textbox and the link AFTER disabling cifs
-					Integer userAction = new MessageDialog(parent.getShell(), "CIFS Re-export", GUIHelper.getInstance()
-							.getImage(IImageKeys.VOLUME_16x16),
-							"Are you sure you want to stop the CIFS re-export for volume [" + volume.getName() + "]?",
-							MessageDialog.QUESTION, new String[] { "No", "Yes" }, -1).open();
-					if (userAction <= 0) { // user select cancel or pressed escape key
-						cifsCheckbox.setSelection(true); // back to previous state.
-					} else {
+					if ((volume.getCifsUsers() == null || volume.getCifsUsers().toString().equals("[]"))
+							&& cifsUsersText.getText().trim().equals("")) {
 						showCifsUsersControls(false);
 						enableCifsUsersControls(false);
-						saveCifsConfiguration();
+					} else {
+
+						Integer userAction = new MessageDialog(parent.getShell(), "CIFS Re-export", GUIHelper
+								.getInstance().getImage(IImageKeys.VOLUME_16x16),
+								"Are you sure you want to stop the CIFS re-export for volume [" + volume.getName()
+										+ "]?", MessageDialog.QUESTION, new String[] { "No", "Yes" }, -1).open();
+						if (userAction <= 0) { // user select cancel or pressed escape key
+							cifsCheckbox.setSelection(true); // back to previous state.
+						} else {
+							showCifsUsersControls(false);
+							enableCifsUsersControls(false);
+							saveCifsConfiguration();
+						}
 					}
 				}
 				populateCifsUsersText();
@@ -660,11 +671,6 @@ public class VolumeSummaryView extends ViewPart {
 
 	private void updateBrickChanges(Volume volume) {
 		numberOfBricks.setText("" + volume.getNumOfBricks());
-		Double replicaCount = 1d;
-		if (volume.getVolumeType() == VOLUME_TYPE.REPLICATE
-				|| volume.getVolumeType() == VOLUME_TYPE.DISTRIBUTED_REPLICATE) {
-			replicaCount = (double) volume.getReplicaCount();
-		}
 		totalDiskSpace.setText("" + NumberUtil.formatNumber(getTotalDiskSpace() / 1024));
 	}
 
@@ -729,12 +735,6 @@ public class VolumeSummaryView extends ViewPart {
 	private void createDiskSpaceField(Composite section) {
 		Label diskSpaceLabel = toolkit.createLabel(section, "Total Disk Space (GB): ", SWT.NONE);
 		diskSpaceLabel.setToolTipText("<b>bold</b>normal");
-		Double replicaCount = 1d;
-		if (volume.getVolumeType() == VOLUME_TYPE.REPLICATE
-				|| volume.getVolumeType() == VOLUME_TYPE.DISTRIBUTED_REPLICATE) {
-			// replicaCount = (double) volume.getReplicaCount();
-			replicaCount = (double) Volume.DEFAULT_REPLICA_COUNT;
-		}
 		totalDiskSpace = toolkit.createLabel(section,
 				"" + NumberUtil.formatNumber(getTotalDiskSpace() / 1024), SWT.NONE);
 		toolkit.createLabel(section, "", SWT.NONE); // dummy
diff --git a/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/resources/v1_0/GlusterServersResource.java b/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/resources/v1_0/GlusterServersResource.java
index d4a8bedf..89143129 100644
--- a/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/resources/v1_0/GlusterServersResource.java
+++ b/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/resources/v1_0/GlusterServersResource.java
@@ -217,12 +217,13 @@ public class GlusterServersResource extends AbstractResource {
 			if (e instanceof ConnectionException || serverUtil.isServerOnline(onlineServer) == false) {
 				// online server has gone offline! try with a different one.
 				onlineServer = clusterService.getNewOnlineServer(clusterName);
+				if (onlineServer == null) {
+					throw new GlusterRuntimeException("No online server found in cluster [" + clusterName + "]");
+				}
+				glusterUtil.addServer(onlineServer.getName(), serverName);
+			} else {
+				throw new GlusterRuntimeException(e.getMessage());
 			}
-			if (onlineServer == null) {
-				throw new GlusterRuntimeException("No online server found in cluster [" + clusterName + "]");
-			}
-
-			glusterUtil.addServer(onlineServer.getName(), serverName);
 		}
 	}
 
diff --git a/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/services/GlusterServerService.java b/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/services/GlusterServerService.java
index d4f484f4..72e414fc 100644
--- a/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/services/GlusterServerService.java
+++ b/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/services/GlusterServerService.java
@@ -81,12 +81,15 @@ public class GlusterServerService {
 			// check if online server has gone offline. If yes, try again one more time.
 			if (e instanceof ConnectionException || serverUtil.isServerOnline(onlineServer) == false) {
 				// online server has gone offline! try with a different one.
-				onlineServer = clusterService.getNewOnlineServer(clusterName);	
-			}
-			if (onlineServer == null) {
-				throw new GlusterRuntimeException("No online servers found in cluster [" + clusterName + "]");
+				onlineServer = clusterService.getNewOnlineServer(clusterName);
+				if (onlineServer == null) {
+					throw new GlusterRuntimeException("No online servers found in cluster [" + clusterName + "]");
+				}
+				glusterServers = getGlusterServers(clusterName, onlineServer, fetchDetails, maxCount, previousServerName);
+			} else {
+				throw new GlusterRuntimeException(e.getMessage());
 			}
-			glusterServers = getGlusterServers(clusterName, onlineServer, fetchDetails, maxCount, previousServerName);
+			
 		}
 		return glusterServers;
 	}
@@ -101,12 +104,13 @@ public class GlusterServerService {
 			if (e instanceof ConnectionException || serverUtil.isServerOnline(onlineServer) == false) {
 				// online server has gone offline! try with a different one.
 				onlineServer = clusterService.getNewOnlineServer(clusterName);
+				if (onlineServer == null) {
+					throw new GlusterRuntimeException("No online servers found in cluster [" + clusterName + "]");
+				}
+				glusterServers = glusterUtil.getGlusterServers(onlineServer);
+			} else {
+				throw new GlusterRuntimeException(e.getMessage());
 			}
-			if (onlineServer == null) {
-				throw new GlusterRuntimeException("No online servers found in cluster [" + clusterName + "]");
-			}
-
-			glusterServers = glusterUtil.getGlusterServers(onlineServer);
 		}
 		
 		// skip the servers by maxCount / previousServerName
@@ -166,11 +170,13 @@ public class GlusterServerService {
 			if (e instanceof ConnectionException || serverUtil.isServerOnline(onlineServer) == false) {
 				// online server has gone offline! try with a different one.
 				onlineServer = clusterService.getNewOnlineServer(clusterName);
+				if (onlineServer == null) {
+					throw new GlusterRuntimeException("No online servers found in cluster [" + clusterName + "]");
+				}
+				server = glusterUtil.getGlusterServer(onlineServer, serverName);
+			} else {
+				throw new GlusterRuntimeException(e.getMessage());
 			}
-			if (onlineServer == null) {
-				throw new GlusterRuntimeException("No online servers found in cluster [" + clusterName + "]");
-			}
-			server = glusterUtil.getGlusterServer(onlineServer, serverName);
 		}
 
 		if (fetchDetails && server.isOnline()) {
@@ -226,11 +232,13 @@ public class GlusterServerService {
 				if (e instanceof ConnectionException || serverUtil.isServerOnline(onlineServer) == false) {
 					// online server has gone offline! try with a different one.
 					onlineServer = clusterService.getNewOnlineServer(clusterName, serverName);
+					if (onlineServer == null) {
+						throw new GlusterRuntimeException("No online server found in cluster [" + clusterName + "]");
+					}
+					glusterUtil.removeServer(onlineServer.getName(), serverName);
+				} else {
+					throw new GlusterRuntimeException(e.getMessage());
 				}
-				if (onlineServer == null) {
-					throw new GlusterRuntimeException("No online server found in cluster [" + clusterName + "]");
-				}
-				glusterUtil.removeServer(onlineServer.getName(), serverName);
 			}
 			
 			try {
diff --git a/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/services/VolumeService.java b/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/services/VolumeService.java
index a0bee373..48a12435 100644
--- a/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/services/VolumeService.java
+++ b/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/services/VolumeService.java
@@ -43,6 +43,7 @@ import com.gluster.storage.management.core.exceptions.GlusterValidationException
 import com.gluster.storage.management.core.model.Brick;
 import com.gluster.storage.management.core.model.GlusterServer;
 import com.gluster.storage.management.core.model.Volume;
+import com.gluster.storage.management.core.model.Server.SERVER_STATUS;
 import com.gluster.storage.management.core.model.Volume.NAS_PROTOCOL;
 import com.gluster.storage.management.core.model.Volume.VOLUME_STATUS;
 import com.gluster.storage.management.core.model.Volume.VOLUME_TYPE;
@@ -126,8 +127,9 @@ public class VolumeService {
 				if (onlineServer == null) {
 					throw new GlusterRuntimeException("No online servers found in cluster [" + clusterName + "]");
 				}
-
 				glusterUtil.addBricks(volumeName, brickList, onlineServer.getName());
+			} else {
+				throw new GlusterRuntimeException(e.getMessage());
 			}
 		}
 	}
@@ -159,10 +161,12 @@ public class VolumeService {
 				if (onlineServer == null) {
 					throw new GlusterRuntimeException("No online servers found in cluster [" + clusterName + "]");
 				}
+				volume = glusterUtil.getVolume(volumeName, onlineServer.getName());
+				// Collect the CIFS users if CIFS Re-exported
+				fetchVolumeCifsUsers(clusterName, volume);
+			} else {
+				throw new GlusterRuntimeException(e.getMessage());
 			}
-			volume = glusterUtil.getVolume(volumeName, onlineServer.getName());
-			// Collect the CIFS users if CIFS Re-exported 
-			fetchVolumeCifsUsers(clusterName, volume);
 		}
 		return volume;
 	}
@@ -250,7 +254,9 @@ public class VolumeService {
 			File serversFile = new File(clusterServersListFile);
 			FileOutputStream fos = new FileOutputStream(serversFile);
 			for (GlusterServer server : glusterServers) {
-				fos.write((server.getName() + CoreConstants.NEWLINE).getBytes());
+				if (server.getStatus() == SERVER_STATUS.ONLINE) {
+					fos.write((server.getName() + CoreConstants.NEWLINE).getBytes());
+				}
 			}
 			fos.close();
 			return serversFile;
@@ -351,13 +357,12 @@ public class VolumeService {
 	
 	// To clear all the volume CIFS configurations from the server
 	public void clearCifsConfiguration(String clusterName, String onlineServerName, String serverName) {
-		VolumeService volumeService = new VolumeService();
 		File volumesFile = createOnlineVolumeList(clusterName, onlineServerName);
 		if (volumesFile == null) {
 			return;
 		}
 		try {
-			volumeService.removeServerVolumeCifsConfig(serverName, volumesFile.getAbsolutePath());
+			removeServerVolumeCifsConfig(serverName, volumesFile.getAbsolutePath());
 			volumesFile.delete();
 		} catch(Exception e) {
 			volumesFile.delete();
@@ -371,7 +376,7 @@ public class VolumeService {
 				+ ALL_ONLINE_VOLUMES_FILE_NAME + "_" + timestamp;
 		try {
 			List<Volume> volumes = getVolumes(clusterName);
-			if (volumes.size() == 0) {
+			if (volumes == null || volumes.size() == 0) {
 				return null;
 			}
 			File volumesFile = new File(volumeListFileName);
@@ -428,10 +433,11 @@ public class VolumeService {
 				if (onlineServer == null) {
 					throw new GlusterRuntimeException("No online servers found in cluster [" + clusterName + "]");
 				}
+				glusterUtil.createVolume(onlineServer.getName(), volumeName, volumeType, transportType, replicaCount,
+						stripeCount, bricks, accessProtocols, options);
+			} else {
+				throw new GlusterRuntimeException(e.getMessage());
 			}
-
-			glusterUtil.createVolume(onlineServer.getName(), volumeName, volumeType, transportType, replicaCount,
-					stripeCount, bricks, accessProtocols, options);
 		}
 
 		List<String> nasProtocols = Arrays.asList(accessProtocols.split(","));
@@ -690,6 +696,8 @@ public class VolumeService {
 				// online server has gone offline! try with a different one.
 				onlineServer = clusterService.getNewOnlineServer(clusterName);
 				performOperation(clusterName, volumeName, operation, onlineServer);
+			} else {
+				throw new GlusterRuntimeException(e.getMessage());
 			}
 		}
 	}
@@ -752,11 +760,13 @@ public class VolumeService {
 			if (e instanceof ConnectionException || serverUtil.isServerOnline(onlineServer) == false) {
 				// online server has gone offline! try with a different one.
 				onlineServer = clusterService.getNewOnlineServer(clusterName);
+				if (onlineServer == null) {
+					throw new GlusterRuntimeException("No online servers found in cluster [" + clusterName + "]");
+				}
+				glusterUtil.removeBricks(volumeName, brickList, onlineServer.getName());
+			} else {
+				throw new GlusterRuntimeException(e.getMessage());
 			}
-			if (onlineServer == null) {
-				throw new GlusterRuntimeException("No online servers found in cluster [" + clusterName + "]");
-			}
-			glusterUtil.removeBricks(volumeName, brickList, onlineServer.getName());
 		}
 	}
 
@@ -859,12 +869,15 @@ public class VolumeService {
 			if (e instanceof ConnectionException || serverUtil.isServerOnline(onlineServer) == false) {
 				// online server has gone offline! try with a different one.
 				onlineServer = clusterService.getNewOnlineServer(clusterName);
-			}
-			if (onlineServer == null) {
-				throw new GlusterRuntimeException("No online servers found in cluster [" + clusterName + "]");
-			}
+				if (onlineServer == null) {
+					throw new GlusterRuntimeException("No online servers found in cluster [" + clusterName + "]");
+				}
 
-			glusterUtil.resetOptions(volumeName, onlineServer.getName());
+				glusterUtil.resetOptions(volumeName, onlineServer.getName());
+			} else {
+				throw new GlusterRuntimeException(e.getMessage());
+			}
+						
 		}
 	}
 	
@@ -901,12 +914,15 @@ public class VolumeService {
 			if (e instanceof ConnectionException || serverUtil.isServerOnline(onlineServer) == false) {
 				// online server has gone offline! try with a different one.
 				onlineServer = clusterService.getNewOnlineServer(clusterName);
+				if (onlineServer == null) {
+					throw new GlusterRuntimeException("No online servers found in cluster [" + clusterName + "]");
+				}
+				glusterUtil.setOption(volumeName, key, value, onlineServer.getName());
+			} else {
+				throw new GlusterRuntimeException(e.getMessage());
 			}
-			if (onlineServer == null) {
-				throw new GlusterRuntimeException("No online servers found in cluster [" + clusterName + "]");
-			}
-
-			glusterUtil.setOption(volumeName, key, value, onlineServer.getName());
+			
+			
 		}
 	}
 }
diff --git a/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/tasks/MigrateBrickTask.java b/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/tasks/MigrateBrickTask.java
index a1fce1eb..e7dd9c02 100644
--- a/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/tasks/MigrateBrickTask.java
+++ b/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/tasks/MigrateBrickTask.java
@@ -25,6 +25,7 @@ import org.springframework.context.ApplicationContext;
 import org.springframework.web.context.ContextLoader;
 
 import com.gluster.storage.management.core.exceptions.ConnectionException;
+import com.gluster.storage.management.core.exceptions.GlusterRuntimeException;
 import com.gluster.storage.management.core.model.Status;
 import com.gluster.storage.management.core.model.TaskInfo.TASK_TYPE;
 import com.gluster.storage.management.core.model.TaskStatus;
@@ -99,6 +100,8 @@ public class MigrateBrickTask extends Task {
 			if (e instanceof ConnectionException || serverUtil.isServerOnline(getOnlineServer()) == false) {
 				// online server might have gone Offline. try with a new one.
 				startMigration(getNewOnlineServer().getName());
+			} else {
+				throw new GlusterRuntimeException(e.getMessage());
 			}
 		}
 	}
@@ -123,6 +126,8 @@ public class MigrateBrickTask extends Task {
 			if (e instanceof ConnectionException || serverUtil.isServerOnline(getOnlineServer()) == false) {
 				// online server might have gone offline. try with a new one.
 				pauseMigration(getNewOnlineServer().getName());
+			} else {
+				throw new GlusterRuntimeException(e.getMessage());
 			}
 		}
 	}
@@ -154,6 +159,8 @@ public class MigrateBrickTask extends Task {
 			if (e instanceof ConnectionException || serverUtil.isServerOnline(getOnlineServer()) == false) {
 				// online server might have gone offline. try with a new one.
 				commitMigration(getNewOnlineServer().getName());
+			} else {
+				throw new GlusterRuntimeException(e.getMessage());
 			}
 		}
 	}
@@ -181,6 +188,8 @@ public class MigrateBrickTask extends Task {
 			if (e instanceof ConnectionException || serverUtil.isServerOnline(getOnlineServer()) == false) {
 				// online server might have gone offline. try with a new one.
 				stopMigration(getNewOnlineServer().getName());
+			} else {
+				throw new GlusterRuntimeException(e.getMessage());
 			}
 		}
 	}
@@ -206,7 +215,7 @@ public class MigrateBrickTask extends Task {
 			if (e instanceof ConnectionException || serverUtil.isServerOnline(getOnlineServer()) == false) {
 				// online server might have gone offline. try with a new one.
 				return checkMigrationStatus(getNewOnlineServer().getName());
-			}
+			} 
 		}
 		return null;
 	}
diff --git a/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/tasks/RebalanceVolumeTask.java b/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/tasks/RebalanceVolumeTask.java
index 8c844f5b..89636fdf 100644
--- a/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/tasks/RebalanceVolumeTask.java
+++ b/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/tasks/RebalanceVolumeTask.java
@@ -72,6 +72,8 @@ public class RebalanceVolumeTask extends Task {
 				// online server might have gone offline. try with a new one
 				serverName = getNewOnlineServer().getName();
 				startRebalance(serverName);
+			} else {
+				throw new GlusterRuntimeException(e.getMessage());
 			}
 		}
 	}
@@ -98,6 +100,8 @@ public class RebalanceVolumeTask extends Task {
 			if (e instanceof ConnectionException || serverUtil.isServerOnline(getOnlineServer()) == false) {
 				// online server might have gone offline. update the failure status
 				getTaskInfo().setStatus(new TaskStatus(new Status(Status.STATUS_CODE_FAILURE, e.getMessage())));
+			} else {
+				throw new GlusterRuntimeException(e.getMessage());
 			}
 		}
 	}