Changes related to Review Board comments

engine/api/src/org/apache/cloudstack/storage/command/DettachCommand.java

@@ -26,6 +26,8 @@ public class DettachCommand extends Command implements StorageSubSystemCommand {
     private String vmName;
     private boolean _managed;
     private String _iScsiName;
+    private String _storageHost;
+    private int _storagePort;
 
     public DettachCommand(DiskTO disk, String vmName) {
         super();
@@ -69,4 +71,20 @@ public class DettachCommand extends Command implements StorageSubSystemCommand {
     public String get_iScsiName() {
         return _iScsiName;
     }
+
+    public void setStorageHost(String storageHost) {
+        _storageHost = storageHost;
+    }
+
+    public String getStorageHost() {
+        return _storageHost;
+    }
+
+    public void setStoragePort(int storagePort) {
+        _storagePort = storagePort;
+    }
+
+    public int getStoragePort() {
+        return _storagePort;
+    }
 }

plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java

@@ -4069,13 +4069,44 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
         }
     }
 
+    public static String getDatastoreName(String str) {
+        return str.replace('/', '-');
+    }
+
+    // This methd can be used to determine if the datastore is active yet.
+    // When an iSCSI target is created on a host and that target already contains
+    // the metadata that represents a datastore, the datastore shows up within
+    // vCenter as existent, but not necessarily active.
+    // Call this method and pass in the datastore's name to wait, if necessary,
+    // for the datastore to become active.
+    private boolean datastoreFileExists(DatastoreMO dsMo, String volumeDatastorePath) {
+        for (int i = 0; i < 10; i++) {
+            try {
+                return dsMo.fileExists(volumeDatastorePath);
+            }
+            catch (Exception e) {
+                if (!e.getMessage().contains("is not accessible")) {
+                    break;
+                }
+            }
+
+            try {
+                Thread.sleep(5000);
+            }
+            catch (Exception e) {
+            }
+        }
+
+        return false;
+    }
+
     @Override
     public ManagedObjectReference handleDatastoreAndVmdkAttach(Command cmd, String iqn, String storageHost, int storagePort,
             String initiatorUsername, String initiatorPassword, String targetUsername, String targetPassword) throws Exception {
         VmwareContext context = getServiceContext();
         VmwareHypervisorHost hyperHost = getHyperHost(context);
 
-        ManagedObjectReference morDs = createVmfsDatastore(hyperHost, iqn,
+        ManagedObjectReference morDs = createVmfsDatastore(hyperHost, getDatastoreName(iqn),
                 storageHost, storagePort, iqn,
                 initiatorUsername, initiatorPassword,
                 targetUsername, targetPassword);
@@ -4084,7 +4115,7 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
 
         String volumeDatastorePath = String.format("[%s] %s.vmdk", dsMo.getName(), dsMo.getName());
 
-        if (!dsMo.fileExists(volumeDatastorePath)) {
+        if (!datastoreFileExists(dsMo, volumeDatastorePath)) {
             String dummyVmName = getWorkerName(context, cmd, 0);
 
             VirtualMachineMO vmMo = prepareVolumeHostDummyVm(hyperHost, dsMo, dummyVmName);
@@ -4096,6 +4127,7 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
             vmMo.createDisk(volumeDatastorePath, getMBsFromBytes(dsMo.getSummary().getFreeSpace()),
                     morDs, vmMo.getScsiDeviceControllerKey());
             vmMo.detachDisk(volumeDatastorePath, false);
+            vmMo.destroy();
         }
 
         return morDs;
@@ -4106,7 +4138,7 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
         VmwareContext context = getServiceContext();
         VmwareHypervisorHost hyperHost = getHyperHost(context);
 
-        deleteVmfsDatastore(hyperHost, iqn, storageHost, storagePort, iqn);
+        deleteVmfsDatastore(hyperHost, getDatastoreName(iqn), storageHost, storagePort, iqn);
     }
 
     protected Answer execute(AttachVolumeCommand cmd) {
@@ -4240,6 +4272,7 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
                                 hss.addInternetScsiStaticTargets(iScsiHbaDevice, lstTargets);
 
                                 hss.rescanHba(iScsiHbaDevice);
+                                hss.rescanVmfs();
                             }
                             catch (Exception ex) {
                                 synchronized (exceptions) {
@@ -4268,7 +4301,7 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
             throw new Exception(exceptions.get(0).getMessage());
         }
 
-        ManagedObjectReference morDs = hostDatastoreSystem.findDatastore(iqn);
+        ManagedObjectReference morDs = hostDatastoreSystem.findDatastoreByName(iqn);
 
         if (morDs != null) {
             return morDs;
@@ -4348,6 +4381,7 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa
                                 hostStorageSystem.removeInternetScsiStaticTargets(iScsiHbaDevice, lstTargets);
 
                                 hostStorageSystem.rescanHba(iScsiHbaDevice);
+                                hostStorageSystem.rescanVmfs();
                             }
                             catch (Exception ex) {
                                 exceptions.add(ex);

plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java

@@ -777,8 +777,8 @@ public class VmwareStorageProcessor implements StorageProcessor {
                 cmd.getChapTargetUsername(), cmd.getChapTargetPassword());
     }
 
-    private Answer attachVolume(Command cmd, DiskTO disk, boolean isAttach, boolean isManaged, String vmName) {
+    private Answer attachVolume(Command cmd, DiskTO disk, boolean isAttach, boolean isManaged, String vmName, String iScsiName, String storageHost, int storagePort) {
-        return attachVolume(cmd, disk, isAttach, isManaged, vmName, null, null, 0, null, null, null, null);
+        return attachVolume(cmd, disk, isAttach, isManaged, vmName, iScsiName, storageHost, storagePort, null, null, null, null);
     }
 
 	private Answer attachVolume(Command cmd, DiskTO disk, boolean isAttach, boolean isManaged, String vmName,
@@ -803,7 +803,7 @@ public class VmwareStorageProcessor implements StorageProcessor {
                             initiatorUsername, initiatorPassword, targetUsername, targetPassword);
             }
             else {
-                morDs = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(hyperHost, primaryStore.getUuid());
+                morDs = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(hyperHost, isManaged ? VmwareResource.getDatastoreName(iScsiName) : primaryStore.getUuid());
             }
 
             if (morDs == null) {
@@ -813,7 +813,7 @@ public class VmwareStorageProcessor implements StorageProcessor {
             }
 
 			DatastoreMO dsMo = new DatastoreMO(this.hostService.getServiceContext(null), morDs);
-			String datastoreVolumePath = String.format("[%s] %s.vmdk", dsMo.getName(), volumeTO.getPath());
+			String datastoreVolumePath = String.format("[%s] %s.vmdk", dsMo.getName(), isManaged ? dsMo.getName() : volumeTO.getPath());
 
             disk.setVdiUuid(datastoreVolumePath);
 
@@ -944,7 +944,7 @@ public class VmwareStorageProcessor implements StorageProcessor {
 
 	@Override
 	public Answer dettachVolume(DettachCommand cmd) {
-		return this.attachVolume(cmd, cmd.getDisk(), false, cmd.isManaged(), cmd.getVmName());
+		return this.attachVolume(cmd, cmd.getDisk(), false, cmd.isManaged(), cmd.getVmName(), cmd.get_iScsiName(), cmd.getStorageHost(), cmd.getStoragePort());
 	}
 
 	protected VirtualMachineMO prepareVolumeHostDummyVm(VmwareHypervisorHost hyperHost, DatastoreMO dsMo, String vmName) throws Exception {

server/src/com/cloud/storage/VolumeManagerImpl.java

@@ -1998,6 +1998,9 @@ public class VolumeManagerImpl extends ManagerBase implements VolumeManager {
 
             cmd.setManaged(volumePool.isManaged());
 
+            cmd.setStorageHost(volumePool.getHostAddress());
+            cmd.setStoragePort(volumePool.getPort());
+
             cmd.set_iScsiName(volume.get_iScsiName());
 
             try {

vmware-base/src/com/cloud/hypervisor/vmware/mo/HostDatastoreSystemMO.java

@@ -96,6 +96,25 @@ public class HostDatastoreSystemMO extends BaseMO {
 		return null;
 	}
 
+    public ManagedObjectReference findDatastoreByName(String datastoreName) throws Exception {
+        assert(datastoreName != null);
+
+        List<ManagedObjectReference> datastores = getDatastores();
+
+        if (datastores != null) {
+            for (ManagedObjectReference morDatastore : datastores) {
+                DatastoreInfo info = getDatastoreInfo(morDatastore);
+
+                if (info != null) {
+                    if (info.getName().equals(datastoreName))
+                        return morDatastore;
+                }
+            }
+        }
+
+        return null;
+    }
+
 	// TODO this is a hacking helper method, when we can pass down storage pool info along with volume
 	// we should be able to find the datastore by name
 	public ManagedObjectReference findDatastoreByExportPath(String exportPath) throws Exception {
@@ -130,7 +149,7 @@ public class HostDatastoreSystemMO extends BaseMO {
 
 	public ManagedObjectReference createVmfsDatastore(String datastoreName, HostScsiDisk hostScsiDisk) throws Exception {
 		// just grab the first instance of VmfsDatastoreOption
-		VmfsDatastoreOption vmfsDatastoreOption = _context.getService().queryVmfsDatastoreCreateOptions(_mor, hostScsiDisk.getDevicePath(), 4).get(0);
+		VmfsDatastoreOption vmfsDatastoreOption = _context.getService().queryVmfsDatastoreCreateOptions(_mor, hostScsiDisk.getDevicePath(), 5).get(0);
 
 		VmfsDatastoreCreateSpec vmfsDatastoreCreateSpec = (VmfsDatastoreCreateSpec)vmfsDatastoreOption.getSpec();
 

vmware-base/src/com/cloud/hypervisor/vmware/mo/HostStorageSystemMO.java

@@ -48,4 +48,8 @@ public class HostStorageSystemMO extends BaseMO {
 	public void rescanHba(String iScsiHbaDevice) throws Exception {
 		_context.getService().rescanHba(_mor, iScsiHbaDevice);
 	}
+
+    public void rescanVmfs() throws Exception {
+        _context.getService().rescanVmfs(_mor);
+    }
 }