- Removed occurences of string.format usage

rajiv-jain-netapp · rajiv-jain-netapp · commit f2838798ec91 · 2026-05-11T18:04:55.000+05:30
- extracted the ONTAP plugin name to interface nane to use it from one place.
 - Corrected indentation gaps.
diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreProvider.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreProvider.java
@@ -29,6 +29,11 @@ public interface DataStoreProvider {
     String SAMPLE_IMAGE = "Sample";
     String SMB = "NFS";
     String DEFAULT_PRIMARY = "DefaultPrimary";
+    /**
+     * Primary storage provider name for NetApp ONTAP ({@code OntapPrimaryDatastoreProvider#getName}).
+     * Single canonical value; use this instead of duplicating the string across modules.
+     */
+    String ONTAP_PLUGIN_NAME = "NetApp ONTAP";
 
     enum DataStoreProviderType {
         PRIMARY, IMAGE, ImageCache, OBJECT
diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
@@ -51,6 +51,7 @@
 import com.cloud.vm.snapshot.VMSnapshotVO;
 import org.apache.cloudstack.backup.BackupOfferingVO;
 import org.apache.cloudstack.backup.dao.BackupOfferingDao;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider;
 import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
 import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
 import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
@@ -77,8 +78,6 @@ public class KvmFileBasedStorageVmSnapshotStrategy extends StorageVMSnapshotStra
 
     private static final List<Storage.StoragePoolType> supportedStoragePoolTypes = List.of(Storage.StoragePoolType.Filesystem, Storage.StoragePoolType.NetworkFilesystem, Storage.StoragePoolType.SharedMountPoint);
 
-    private static final String ONTAP_PROVIDER_NAME = "NetApp ONTAP";
-
     @Inject
     protected SnapshotDataStoreDao snapshotDataStoreDao;
 
@@ -327,8 +326,8 @@ public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean snapshotMe
         List<VolumeVO> volumes = volumeDao.findByInstance(vmId);
         for (VolumeVO volume : volumes) {
             StoragePoolVO storagePoolVO = storagePool.findById(volume.getPoolId());
-            if (storagePoolVO.isManaged() && ONTAP_PROVIDER_NAME.equals(storagePoolVO.getStorageProviderName())) {
-                logger.debug(" {} as the VM has a volume on ONTAP managed storage pool [{}]. " +
+            if (storagePoolVO.isManaged() && DataStoreProvider.ONTAP_PLUGIN_NAME.equals(storagePoolVO.getStorageProviderName())) {
+                logger.debug("{} as the VM has a volume on ONTAP managed storage pool [{}]. " +
                         "ONTAP managed storage has its own dedicated VM snapshot strategy.", cantHandleLog, storagePoolVO.getName());
                 return StrategyPriority.CANT_HANDLE;
             }
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java
@@ -22,6 +22,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.io.File;
 import java.nio.file.Files;
 import java.nio.file.Paths;
 
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/VolumeConcise.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/model/VolumeConcise.java
@@ -30,6 +30,7 @@ public class VolumeConcise {
     private String uuid;
 
     @JsonProperty("name")
+
     private String name;
 
     public String getUuid() {
@@ -45,6 +46,5 @@ public String getName() {
     }
 
     public void setName(String name) {
-        this.name = name;
-    }
+        this.name = name;  }
 }
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java
@@ -34,13 +34,13 @@
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.google.common.base.Preconditions;
 import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.engine.subsystem.api.storage.AbstractScope;
 import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
 import org.apache.cloudstack.engine.subsystem.api.storage.HostScope;
 import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreInfo;
 import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreLifeCycle;
 import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreParameters;
-import org.apache.cloudstack.engine.subsystem.api.storage.Scope;
 import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
@@ -232,7 +232,7 @@ private long validateInitializeInputs(Long capacityBytes, Long podId, Long clust
             throw new CloudRuntimeException("Storage pool name is null or empty, cannot create primary storage");
         }
 
-        if (StringUtils.isBlank(providerName)) {
+        if (StringUtils.isBlank(providerName )) {
             throw new CloudRuntimeException("Provider name is null or empty, cannot create primary storage");
         }
 
@@ -273,9 +273,9 @@ private long validateInitializeInputs(Long capacityBytes, Long podId, Long clust
         }
 
         // Detect missing required keys
-        HashSet<String> providedKeys = new HashSet<>(details.keySet());
+        Set<String> providedKeys = new HashSet<>(details.keySet());
         if (!providedKeys.containsAll(requiredKeys)) {
-            HashSet<String> missing = new HashSet<>(requiredKeys);
+            Set<String> missing = new HashSet<>(requiredKeys);
             missing.removeAll(providedKeys);
             throw new CloudRuntimeException("ONTAP primary storage creation failed, missing detail(s): " + missing);
         }
@@ -287,10 +287,10 @@ private long validateInitializeInputs(Long capacityBytes, Long podId, Long clust
     public boolean attachCluster(DataStore dataStore, ClusterScope scope) {
         logger.debug("In attachCluster for ONTAP primary storage");
         if (dataStore == null) {
-            throw new InvalidParameterValueException("dataStore should not be null");
+            throw new InvalidParameterValueException("DataStore should not be null");
         }
         if (scope == null) {
-            throw new InvalidParameterValueException("scope should not be null");
+            throw new InvalidParameterValueException("Scope should not be null");
         }
         List<String> hostsIdentifier = new ArrayList<>();
         StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId());
@@ -378,6 +378,7 @@ public boolean attachZone(DataStore dataStore, ZoneScope scope, Hypervisor.Hyper
         _dataStoreHelper.attachZone(dataStore);
         return true;
     }
+
     private boolean validateProtocolSupportAndFetchHostsIdentifier(List<HostVO> hosts, ProtocolType protocolType, List<String> hostIdentifiers) {
         switch (protocolType) {
             case ISCSI:
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProvider.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProvider.java
@@ -21,14 +21,14 @@
 
 
 import com.cloud.utils.component.ComponentContext;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreDriver;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreLifeCycle;
 import org.apache.cloudstack.engine.subsystem.api.storage.HypervisorHostListener;
 import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreProvider;
 import org.apache.cloudstack.storage.driver.OntapPrimaryDatastoreDriver;
 import org.apache.cloudstack.storage.lifecycle.OntapPrimaryDatastoreLifecycle;
 import org.apache.cloudstack.storage.listener.OntapHostListener;
-import org.apache.cloudstack.storage.utils.OntapStorageConstants;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.springframework.stereotype.Component;
@@ -66,7 +66,7 @@ public HypervisorHostListener getHostListener() {
     @Override
     public String getName() {
         logger.trace("OntapPrimaryDatastoreProvider: getName: Called");
-        return OntapStorageConstants.ONTAP_PLUGIN_NAME;
+        return DataStoreProvider.ONTAP_PLUGIN_NAME;
     }
 
     @Override
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/CloudStackVolume.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/model/CloudStackVolume.java
@@ -36,18 +36,23 @@ public class CloudStackVolume {
      *   a. snapshot workflows will get source LUN details from it.
      */
     private Lun lun;
+
     private String datastoreId;
+
     /**
      * FlexVolume UUID on which this cloudstack volume is created.
      *    a. Field is eligible for unified storage only.
      *    b. It will be null for the disaggregated storage.
      */
     private String flexVolumeUuid;
+
     /**
      * Field serves for snapshot workflows
      */
     private String destinationPath;
+
     private DataObject volumeInfo; // This is needed as we need DataObject to be passed to agent to create volume
+
     public FileInfo getFile() {
         return file;
     }
@@ -79,6 +84,7 @@ public DataObject getVolumeInfo() {
     public void setVolumeInfo(DataObject volumeInfo) {
         this.volumeInfo = volumeInfo;
     }
+
     public String getFlexVolumeUuid() {
         return flexVolumeUuid;
     }
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageConstants.java
@@ -19,10 +19,12 @@
 
 package org.apache.cloudstack.storage.utils;
 
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider;
 
 public class OntapStorageConstants {
 
-    public static final String ONTAP_PLUGIN_NAME = "NetApp ONTAP";
+    /** Same as {@link DataStoreProvider#ONTAP_PLUGIN_NAME}; retained for existing ONTAP plugin call sites. */
+    public static final String ONTAP_PLUGIN_NAME = DataStoreProvider.ONTAP_PLUGIN_NAME;
     public static final int NFS3_PORT = 2049;
     public static final int ISCSI_PORT = 3260;
 
diff --git a/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java b/plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategy.java
@@ -27,6 +27,7 @@
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider;
 import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
 import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotOptions;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
@@ -233,7 +234,7 @@ boolean allVolumesOnOntapManagedStorage(long vmId) {
                         volume.getId(), pool.getName());
                 return false;
             }
-            if (!OntapStorageConstants.ONTAP_PLUGIN_NAME.equals(pool.getStorageProviderName())) {
+            if (!DataStoreProvider.ONTAP_PLUGIN_NAME.equals(pool.getStorageProviderName())) {
                 logger.debug("allVolumesOnOntapManagedStorage: Volume [{}] is on managed pool [{}] with provider [{}], not ONTAP",
                         volume.getId(), pool.getName(), pool.getStorageProviderName());
                 return false;
diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProviderTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/provider/OntapPrimaryDatastoreProviderTest.java
@@ -19,14 +19,14 @@
 package org.apache.cloudstack.storage.provider;
 
 import com.cloud.utils.component.ComponentContext;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreDriver;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreLifeCycle;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider.DataStoreProviderType;
 import org.apache.cloudstack.engine.subsystem.api.storage.HypervisorHostListener;
 import org.apache.cloudstack.storage.driver.OntapPrimaryDatastoreDriver;
 import org.apache.cloudstack.storage.lifecycle.OntapPrimaryDatastoreLifecycle;
 import org.apache.cloudstack.storage.listener.OntapHostListener;
-import org.apache.cloudstack.storage.utils.OntapStorageConstants;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -60,7 +60,7 @@ void setUp() {
     @Test
     public void testGetName() {
         String name = provider.getName();
-        assertEquals(OntapStorageConstants.ONTAP_PLUGIN_NAME, name);
+        assertEquals(DataStoreProvider.ONTAP_PLUGIN_NAME, name);
     }
 
     @Test
diff --git a/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java b/plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/vmsnapshot/OntapVMSnapshotStrategyTest.java
@@ -45,7 +45,6 @@
 import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.apache.cloudstack.storage.to.VolumeObjectTO;
-import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.InjectMocks;
@@ -106,10 +105,6 @@ class OntapVMSnapshotStrategyTest {
     private static final String VM_INSTANCE_NAME = "i-2-100-VM";
     private static final String VM_UUID = "vm-uuid-123";
 
-    @Spy
-    @InjectMocks
-    private OntapVMSnapshotStrategy strategy;
-
     @Mock
     private UserVmDao userVmDao;
     @Mock
@@ -133,10 +128,9 @@ class OntapVMSnapshotStrategyTest {
     @Mock
     private VolumeDetailsDao volumeDetailsDao;
 
-    @BeforeEach
-    void setUp() {
-        // @InjectMocks handles injection into inherited fields
-    }
+    @Spy
+    @InjectMocks
+    private OntapVMSnapshotStrategy strategy;
 
     // ──────────────────────────────────────────────────────────────────────────
     // Helper: create common mocks
diff --git a/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java b/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
@@ -33,6 +33,7 @@
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.command.user.vmsnapshot.ListVMSnapshotCmd;
 import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider;
 import org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory;
 import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotOptions;
 import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotStrategy;
@@ -135,7 +136,6 @@
 public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase implements VMSnapshotManager, VMSnapshotService, VmWorkJobHandler, Configurable {
 
     public static final String VM_WORK_JOB_HANDLER = VMSnapshotManagerImpl.class.getSimpleName();
-    public static final String ONTAP_PLUGIN_NAME = "NetApp ONTAP";
 
     @Inject
     VMInstanceDao _vmInstanceDao;
@@ -393,7 +393,7 @@ public VMSnapshot allocVMSnapshot(Long vmId, String vsDisplayName, String vsDesc
             if (snapshotStrategy == null) {
                 // Check if this is ONTAP managed storage with memory snapshot request - provide specific error message
                 if (snapshotMemory && rootVolumePool.isManaged() &&
-                        ONTAP_PLUGIN_NAME.equals(rootVolumePool.getStorageProviderName())) {
+                        DataStoreProvider.ONTAP_PLUGIN_NAME.equals(rootVolumePool.getStorageProviderName())) {
                     String message = String.format("Memory snapshots (snapshotmemory=true) are not supported for VMs on ONTAP managed storage. " +
                             "Instance [%s] uses ONTAP storage which only supports disk-only (crash-consistent) snapshots. " +
                             "Please use snapshotmemory=false for disk-only snapshots.", userVmVo.getUuid());

Original file line number	Diff line number	Diff line change
`@@ -30,6 +30,7 @@ public class VolumeConcise {`
`30`	`30`	`private String uuid;`
`31`	`31`
`32`	`32`	`@JsonProperty("name")`
	`33`	`+`
`33`	`34`	`private String name;`
`34`	`35`
`35`	`36`	`public String getUuid() {`
`@@ -45,6 +46,5 @@ public String getName() {`
`45`	`46`	`}`
`46`	`47`
`47`	`48`	`public void setName(String name) {`
`48`		`- this.name = name;`
`49`		`- }`
	`49`	`+ this.name = name; }`
`50`	`50`	`}`