Updates to HPE-Primera and Pure FlashArray Drivers to use Host-based VLUN Assignments (apache#8889)

* Updates to change PUre and Primera to host-centric vlun assignments; various small bug fixes

* update to add timestamp when deleting pure volumes to avoid future conflicts

* update to migrate to properly check disk offering is valid for the target storage pool

* Updates to change PUre and Primera to host-centric vlun assignments; various small bug fixes

* update to add timestamp when deleting pure volumes to avoid future conflicts

* update to migrate to properly check disk offering is valid for the target storage pool

* improve error handling when copying volumes to add precision to which step failed

* rename pure volume before delete to avoid conflicts if the same name is used before its expunged on the array

* remove dead code in AdaptiveDataStoreLifeCycleImpl.java

* Fix issues found in PR checks

* fix session refresh TTL logic

* updates from PR comments

* logic to delete by path ONLY on supported OUI

* fix to StorageSystemDataMotionStrategy compile error

* change noisy debug message to trace message

* fix double callback call in handleVolumeMigrationFromNonManagedStorageToManagedStorage

* fix for flash array delete error

* fix typo in StorageSystemDataMotionStrategy

* change copyVolume to use writeback to speed up copy ops

* remove returning PrimaryStorageDownloadAnswer when connectPhysicalDisk returns false during KVMStorageProcessor template copy

* remove change to only set UUID on snapshot if it is a vmSnapshot

* reverting change to UserVmManagerImpl.configureCustomRootDiskSize

* add error checking/simplification per comments from @slavkap

* Update engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java

Co-authored-by: Suresh Kumar Anaparti <[email protected]>

* address PR comments from @sureshanaparti

---------

Co-authored-by: GLOVER RENE <[email protected]>
Co-authored-by: Suresh Kumar Anaparti <[email protected]>

Author: 3 people

Diff for: engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java

@@ -1504,18 +1504,17 @@ public void prepareForMigration(VirtualMachineProfile vm, DeployDestination dest
 
         for (VolumeVO vol : vols) {
             VolumeInfo volumeInfo = volFactory.getVolume(vol.getId());
-            DataTO volTO = volumeInfo.getTO();
-            DiskTO disk = storageMgr.getDiskWithThrottling(volTO, vol.getVolumeType(), vol.getDeviceId(), vol.getPath(), vm.getServiceOfferingId(), vol.getDiskOfferingId());
             DataStore dataStore = dataStoreMgr.getDataStore(vol.getPoolId(), DataStoreRole.Primary);
 
-            disk.setDetails(getDetails(volumeInfo, dataStore));
-
             PrimaryDataStore primaryDataStore = (PrimaryDataStore)dataStore;
             // This might impact other managed storages, enable requires access for migration in relevant datastore driver (currently enabled for PowerFlex storage pool only)
             if (primaryDataStore.isManaged() && volService.requiresAccessForMigration(volumeInfo, dataStore)) {
                 volService.grantAccess(volFactory.getVolume(vol.getId()), dest.getHost(), dataStore);
             }
-
+            // make sure this is done AFTER grantAccess, as grantAccess may change the volume's state
+            DataTO volTO = volumeInfo.getTO();
+            DiskTO disk = storageMgr.getDiskWithThrottling(volTO, vol.getVolumeType(), vol.getDeviceId(), vol.getPath(), vm.getServiceOfferingId(), vol.getDiskOfferingId());
+            disk.setDetails(getDetails(volumeInfo, dataStore));
             vm.addDisk(disk);
         }
 

Diff for: engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java

@@ -101,7 +101,9 @@ public void with(ModuleDefinition def, Stack<ModuleDefinition> parents) {
                     log.debug(String.format("Trying to obtain module [%s] context.", moduleDefinitionName));
                     ApplicationContext context = getApplicationContext(moduleDefinitionName);
                     try {
-                        if (context.containsBean("moduleStartup")) {
+                        if (context == null) {
+                            log.warn(String.format("Application context not found for module definition [%s]", moduleDefinitionName));
+                        } else if (context.containsBean("moduleStartup")) {
                             Runnable runnable = context.getBean("moduleStartup", Runnable.class);
                             log.info(String.format("Starting module [%s].", moduleDefinitionName));
                             runnable.run();

Diff for: framework/spring/module/src/main/java/org/apache/cloudstack/spring/module/model/impl/DefaultModuleDefinitionSet.java

@@ -122,7 +122,9 @@ public boolean checkAccess(Account account, String commandName) {
         }
 
         if (accountRole.getRoleType() == RoleType.Admin && accountRole.getId() == RoleType.Admin.getId()) {
-            LOGGER.info(String.format("Account [%s] is Root Admin or Domain Admin, all APIs are allowed.", account));
+            if (LOGGER.isTraceEnabled()) {
+                LOGGER.trace(String.format("Account [%s] is Root Admin or Domain Admin, all APIs are allowed.", account));
+            }
             return true;
         }
 

Diff for: plugins/acl/dynamic-role-based/src/main/java/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java

@@ -76,7 +76,9 @@ public List<String> getApisAllowedToUser(Role role, User user, List<String> apiN
 
         Project project = CallContext.current().getProject();
         if (project == null) {
-            LOGGER.warn(String.format("Project is null, ProjectRoleBasedApiAccessChecker only applies to projects, returning APIs [%s] for user [%s] as allowed.", apiNames, user));
+            if (LOGGER.isTraceEnabled()) {
+                LOGGER.trace(String.format("Project is null, ProjectRoleBasedApiAccessChecker only applies to projects, returning APIs [%s] for user [%s] as allowed.", apiNames, user));
+            }
             return apiNames;
         }
 
@@ -114,8 +116,10 @@ public boolean checkAccess(User user, String apiCommandName) throws PermissionDe
 
         Project project = CallContext.current().getProject();
         if (project == null) {
-            LOGGER.warn(String.format("Project is null, ProjectRoleBasedApiAccessChecker only applies to projects, returning API [%s] for user [%s] as allowed.", apiCommandName,
+            if (LOGGER.isTraceEnabled()) {
+                LOGGER.trace(String.format("Project is null, ProjectRoleBasedApiAccessChecker only applies to projects, returning API [%s] for user [%s] as allowed.", apiCommandName,
                 user));
+            }
             return true;
         }
 

Diff for: plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java

@@ -302,15 +302,27 @@ protected MigrateVolumeAnswer migrateRegularVolume(final MigrateVolumeCommand co
                 (destVolumeObjectTO.getPath() != null ? destVolumeObjectTO.getPath() : UUID.randomUUID().toString());
 
         try {
-            storagePoolManager.connectPhysicalDisk(srcPrimaryDataStore.getPoolType(), srcPrimaryDataStore.getUuid(), srcPath, srcDetails);
+            KVMStoragePool sourceStoragePool = storagePoolManager.getStoragePool(srcPrimaryDataStore.getPoolType(), srcPrimaryDataStore.getUuid());
+
+            if (!sourceStoragePool.connectPhysicalDisk(srcPath, srcDetails)) {
+                return new MigrateVolumeAnswer(command, false, "Unable to connect source volume on hypervisor", srcPath);
+            }
 
             KVMPhysicalDisk srcPhysicalDisk = storagePoolManager.getPhysicalDisk(srcPrimaryDataStore.getPoolType(), srcPrimaryDataStore.getUuid(), srcPath);
+            if (srcPhysicalDisk == null) {
+                return new MigrateVolumeAnswer(command, false, "Unable to get handle to source volume on hypervisor", srcPath);
+            }
 
             KVMStoragePool destPrimaryStorage = storagePoolManager.getStoragePool(destPrimaryDataStore.getPoolType(), destPrimaryDataStore.getUuid());
 
-            storagePoolManager.connectPhysicalDisk(destPrimaryDataStore.getPoolType(), destPrimaryDataStore.getUuid(), destPath, destDetails);
+            if (!destPrimaryStorage.connectPhysicalDisk(destPath, destDetails)) {
+                return new MigrateVolumeAnswer(command, false, "Unable to connect destination volume on hypervisor", srcPath);
+            }
 
-            storagePoolManager.copyPhysicalDisk(srcPhysicalDisk, destPath, destPrimaryStorage, command.getWaitInMillSeconds());
+            KVMPhysicalDisk newDiskCopy = storagePoolManager.copyPhysicalDisk(srcPhysicalDisk, destPath, destPrimaryStorage, command.getWaitInMillSeconds());
+            if (newDiskCopy == null) {
+                return new MigrateVolumeAnswer(command, false, "Copy command failed to return handle to copied physical disk", destPath);
+            }
         }
         catch (Exception ex) {
             return new MigrateVolumeAnswer(command, false, ex.getMessage(), null);

Diff for: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateVolumeCommandWrapper.java

@@ -16,13 +16,36 @@
 // under the License.
 package com.cloud.hypervisor.kvm.storage;
 
+import java.net.InetAddress;
+import java.net.UnknownHostException;
+
+import org.apache.log4j.Logger;
+
 import com.cloud.storage.Storage;
 import com.cloud.utils.exception.CloudRuntimeException;
 
 @StorageAdaptorInfo(storagePoolType=Storage.StoragePoolType.FiberChannel)
 public class FiberChannelAdapter extends MultipathSCSIAdapterBase {
+
+    private Logger LOGGER = Logger.getLogger(getClass());
+
+    private String hostname = null;
+    private String hostnameFq = null;
+
     public FiberChannelAdapter() {
         LOGGER.info("Loaded FiberChannelAdapter for StorageLayer");
+        // get the hostname - we need this to compare to connid values
+        try {
+            InetAddress inetAddress = InetAddress.getLocalHost();
+            hostname = inetAddress.getHostName(); // basic hostname
+            if (hostname.indexOf(".") > 0) {
+                hostname = hostname.substring(0, hostname.indexOf(".")); // strip off domain
+            }
+            hostnameFq = inetAddress.getCanonicalHostName(); // fully qualified hostname
+            LOGGER.info("Loaded FiberChannelAdapter for StorageLayer on host [" + hostname + "]");
+        } catch (UnknownHostException e) {
+            LOGGER.error("Error getting hostname", e);
+        }
     }
 
     @Override
@@ -72,6 +95,11 @@ public AddressInfo parseAndValidatePath(String inPath) {
                             address = value;
                         } else if (key.equals("connid")) {
                             connectionId = value;
+                        } else if (key.startsWith("connid.")) {
+                            String inHostname = key.substring(7);
+                            if (inHostname != null && (inHostname.equals(this.hostname) || inHostname.equals(this.hostnameFq))) {
+                                connectionId = value;
+                            }
                         }
                     }
                 }

Diff for: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/FiberChannelAdapter.java

@@ -268,7 +268,7 @@ public Answer copyTemplateToPrimaryStorage(final CopyCommand cmd) {
 
                 Map<String, String> details = primaryStore.getDetails();
 
-                String path = details != null ? details.get("managedStoreTarget") : null;
+                String path = derivePath(primaryStore, destData, details);
 
                 if (!storagePoolMgr.connectPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), path, details)) {
                     s_logger.warn("Failed to connect physical disk at path: " + path + ", in storage pool id: " + primaryStore.getUuid());
@@ -328,6 +328,16 @@ public Answer copyTemplateToPrimaryStorage(final CopyCommand cmd) {
         }
     }
 
+    private String derivePath(PrimaryDataStoreTO primaryStore, DataTO destData, Map<String, String> details) {
+        String path = null;
+        if (primaryStore.getPoolType() == StoragePoolType.FiberChannel) {
+            path = destData.getPath();
+        } else {
+            path = details != null ? details.get("managedStoreTarget") : null;
+        }
+        return path;
+    }
+
     // this is much like PrimaryStorageDownloadCommand, but keeping it separate. copies template direct to root disk
     private KVMPhysicalDisk templateToPrimaryDownload(final String templateUrl, final KVMStoragePool primaryPool, final String volUuid, final Long size, final int timeout) {
         final int index = templateUrl.lastIndexOf("/");
@@ -407,7 +417,7 @@ public Answer cloneVolumeFromBaseTemplate(final CopyCommand cmd) {
                 vol = templateToPrimaryDownload(templatePath, primaryPool, volume.getUuid(), volume.getSize(), cmd.getWaitInMillSeconds());
             } if (primaryPool.getType() == StoragePoolType.PowerFlex) {
                 Map<String, String> details = primaryStore.getDetails();
-                String path = details != null ? details.get("managedStoreTarget") : null;
+                String path = derivePath(primaryStore, destData, details);
 
                 if (!storagePoolMgr.connectPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), templatePath, details)) {
                     s_logger.warn("Failed to connect base template volume at path: " + templatePath + ", in storage pool id: " + primaryStore.getUuid());
@@ -1048,7 +1058,7 @@ public Answer backupSnapshot(final CopyCommand cmd) {
             srcVolume.clearPassphrase();
             if (isCreatedFromVmSnapshot) {
                 s_logger.debug("Ignoring removal of vm snapshot on primary as this snapshot is created from vm snapshot");
-            } else if (primaryPool.getType() != StoragePoolType.RBD) {
+            } else if (primaryPool != null && primaryPool.getType() != StoragePoolType.RBD) {
                 deleteSnapshotOnPrimary(cmd, snapshot, primaryPool);
             }
 
@@ -2482,8 +2492,7 @@ public Answer copyVolumeFromPrimaryToPrimary(CopyCommand cmd) {
                 if (!storagePoolMgr.connectPhysicalDisk(destPrimaryStore.getPoolType(), destPrimaryStore.getUuid(), destVolumePath, destPrimaryStore.getDetails())) {
                     s_logger.warn("Failed to connect dest volume at path: " + destVolumePath + ", in storage pool id: " + destPrimaryStore.getUuid());
                 }
-                String managedStoreTarget = destPrimaryStore.getDetails() != null ? destPrimaryStore.getDetails().get("managedStoreTarget") : null;
-                destVolumeName = managedStoreTarget != null ? managedStoreTarget : destVolumePath;
+                destVolumeName = derivePath(destPrimaryStore, destData, destPrimaryStore.getDetails());
             } else {
                 final String volumeName = UUID.randomUUID().toString();
                 destVolumeName = volumeName + "." + destFormat.getFileExtension();