Fixed NPE on volume creation from snapshot (apache#6839)

JoaoJandre · João Jandre · web-flow · commit 14937e1adbcb · 2022-10-26T08:44:01.000+02:00
Co-authored-by: João Jandre &lt;joao@scclouds.com.br&gt;
diff --git a/api/src/main/java/com/cloud/deploy/DataCenterDeployment.java b/api/src/main/java/com/cloud/deploy/DataCenterDeployment.java
@@ -18,6 +18,7 @@
 
 import com.cloud.deploy.DeploymentPlanner.ExcludeList;
 import com.cloud.vm.ReservationContext;
+import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -117,4 +118,10 @@ public boolean isMigrationPlan() {
         return migrationPlan;
     }
 
+    @Override
+    public String toString() {
+        return ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, "_dcId", "_podId", "_clusterId", "_poolId", "_hostid", "_physicalNetworkId",
+                "migrationPlan");
+    }
+
 }
diff --git a/engine/components-api/src/main/java/com/cloud/vm/VirtualMachineProfileImpl.java b/engine/components-api/src/main/java/com/cloud/vm/VirtualMachineProfileImpl.java
@@ -86,7 +86,10 @@ public VirtualMachineProfileImpl(VirtualMachine.Type type) {
 
     @Override
     public String toString() {
-        return _vm.toString();
+        if (_vm != null) {
+            return _vm.toString();
+        }
+        return "";
     }
 
     @Override
diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java
@@ -374,15 +374,15 @@ public StoragePool findStoragePool(DiskProfile dskCh, DataCenter dc, Pod pod, Lo
                 if (storagePool.isPresent()) {
                     storage = (StoragePool)this.dataStoreMgr.getDataStore(storagePool.get().getId(), DataStoreRole.Primary);
                     s_logger.debug(String.format("VM [%s] has a preferred storage pool [%s]. Volume Orchestrator found this storage using Storage Pool Allocator [%s] and will"
-                            + " use it.", vm, storage, allocator));
+                            + " use it.", vm, storage, allocator.getClass().getSimpleName()));
                 } else {
                     storage = (StoragePool)dataStoreMgr.getDataStore(poolList.get(0).getId(), DataStoreRole.Primary);
                     s_logger.debug(String.format("VM [%s] does not have a preferred storage pool or it cannot be used. Volume Orchestrator will use the available Storage Pool"
-                            + " [%s], which was discovered using Storage Pool Allocator [%s].", vm, storage, allocator));
+                            + " [%s], which was discovered using Storage Pool Allocator [%s].", vm, storage, allocator.getClass().getSimpleName()));
                 }
                 return storage;
             }
-            s_logger.debug(String.format("Could not find any available Storage Pool using Storage Pool Allocator [%s].", allocator));
+            s_logger.debug(String.format("Could not find any available Storage Pool using Storage Pool Allocator [%s].", allocator.getClass().getSimpleName()));
         }
         s_logger.info("Volume Orchestrator could not find any available Storage Pool.");
         return null;
diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
@@ -19,6 +19,7 @@
 import java.math.BigDecimal;
 import java.security.SecureRandom;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
@@ -389,4 +390,9 @@ protected void logStartOfSearch(DiskProfile dskCh, VirtualMachineProfile vmProfi
                 + "deployment plan [%s]. Returning up to [%d] and bypassStorageTypeCheck [%s].", this.getClass().getSimpleName(), dskCh, vmProfile, plan, returnUpTo, bypassStorageTypeCheck));
     }
 
+    protected void logEndOfSearch(List<StoragePool> storagePoolList) {
+        s_logger.debug(String.format("%s is returning [%s] suitable storage pools [%s].", this.getClass().getSimpleName(), storagePoolList.size(),
+                Arrays.toString(storagePoolList.toArray())));
+    }
+
 }
diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java
@@ -107,7 +107,7 @@ protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmPr
             }
         }
 
-        s_logger.debug(String.format("ClusterScopeStoragePoolAllocator is returning [%s] suitable storage pools [%s].", suitablePools.size(), suitablePools));
+        logEndOfSearch(suitablePools);
 
         return suitablePools;
     }
diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/LocalStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/LocalStoragePoolAllocator.java
@@ -120,7 +120,7 @@ protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmPr
                 avoid.addPool(pool.getId());
             }
         }
-        s_logger.debug(String.format("LocalStoragePoolAllocator returning [%s] suitable storage pools [%s].", suitablePools.size(), suitablePools));
+        logEndOfSearch(suitablePools);
 
         return suitablePools;
     }
diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java
@@ -102,7 +102,7 @@ protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmPr
                 }
             }
         }
-        LOGGER.debug(String.format("ZoneWideStoragePoolAllocator is returning [%s] suitable storage pools [%s].", suitablePools.size(), suitablePools));
+        logEndOfSearch(suitablePools);
 
         return suitablePools;
     }
diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/PrimaryDataStoreImpl.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/PrimaryDataStoreImpl.java
@@ -46,6 +46,7 @@
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
 import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
 import org.apache.cloudstack.storage.volume.VolumeObject;
+import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
 import org.apache.log4j.Logger;
 
 import com.cloud.agent.api.to.DataObjectType;
@@ -464,4 +465,9 @@ public StoragePoolType getParentPoolType() {
         }
         return null;
     }
+
+    @Override
+    public String toString() {
+        return ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, "name", "uuid");
+    }
 }
diff --git a/plugins/storage-allocators/random/src/main/java/org/apache/cloudstack/storage/allocator/RandomStoragePoolAllocator.java b/plugins/storage-allocators/random/src/main/java/org/apache/cloudstack/storage/allocator/RandomStoragePoolAllocator.java
@@ -73,7 +73,7 @@ public List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmProfi
             }
         }
 
-        s_logger.debug(String.format("RandomStoragePoolAllocator is returning [%s] suitable storage pools [%s].", suitablePools.size(), suitablePools));
+        logEndOfSearch(suitablePools);
 
         return suitablePools;
     }

Original file line number	Diff line number	Diff line change
`@@ -374,15 +374,15 @@ public StoragePool findStoragePool(DiskProfile dskCh, DataCenter dc, Pod pod, Lo`
`374`	`374`	`if (storagePool.isPresent()) {`
`375`	`375`	`storage = (StoragePool)this.dataStoreMgr.getDataStore(storagePool.get().getId(), DataStoreRole.Primary);`
`376`	`376`	`s_logger.debug(String.format("VM [%s] has a preferred storage pool [%s]. Volume Orchestrator found this storage using Storage Pool Allocator [%s] and will"`
`377`		`- + " use it.", vm, storage, allocator));`
	`377`	`+ + " use it.", vm, storage, allocator.getClass().getSimpleName()));`
`378`	`378`	`} else {`
`379`	`379`	`storage = (StoragePool)dataStoreMgr.getDataStore(poolList.get(0).getId(), DataStoreRole.Primary);`
`380`	`380`	`s_logger.debug(String.format("VM [%s] does not have a preferred storage pool or it cannot be used. Volume Orchestrator will use the available Storage Pool"`
`381`		`- + " [%s], which was discovered using Storage Pool Allocator [%s].", vm, storage, allocator));`
	`381`	`+ + " [%s], which was discovered using Storage Pool Allocator [%s].", vm, storage, allocator.getClass().getSimpleName()));`
`382`	`382`	`}`
`383`	`383`	`return storage;`
`384`	`384`	`}`
`385`		`- s_logger.debug(String.format("Could not find any available Storage Pool using Storage Pool Allocator [%s].", allocator));`
	`385`	`+ s_logger.debug(String.format("Could not find any available Storage Pool using Storage Pool Allocator [%s].", allocator.getClass().getSimpleName()));`
`386`	`386`	`}`
`387`	`387`	`s_logger.info("Volume Orchestrator could not find any available Storage Pool.");`
`388`	`388`	`return null;`
Original file line number	Diff line number	Diff line change
`@@ -107,7 +107,7 @@ protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmPr`
`107`	`107`	`}`
`108`	`108`	`}`
`109`	`109`
`110`		`- s_logger.debug(String.format("ClusterScopeStoragePoolAllocator is returning [%s] suitable storage pools [%s].", suitablePools.size(), suitablePools));`
	`110`	`+ logEndOfSearch(suitablePools);`
`111`	`111`
`112`	`112`	`return suitablePools;`
`113`	`113`	`}`
Original file line number	Diff line number	Diff line change
`@@ -120,7 +120,7 @@ protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmPr`
`120`	`120`	`avoid.addPool(pool.getId());`
`121`	`121`	`}`
`122`	`122`	`}`
`123`		`- s_logger.debug(String.format("LocalStoragePoolAllocator returning [%s] suitable storage pools [%s].", suitablePools.size(), suitablePools));`
	`123`	`+ logEndOfSearch(suitablePools);`
`124`	`124`
`125`	`125`	`return suitablePools;`
`126`	`126`	`}`
Original file line number	Diff line number	Diff line change
`@@ -102,7 +102,7 @@ protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmPr`
`102`	`102`	`}`
`103`	`103`	`}`
`104`	`104`	`}`
`105`		`- LOGGER.debug(String.format("ZoneWideStoragePoolAllocator is returning [%s] suitable storage pools [%s].", suitablePools.size(), suitablePools));`
	`105`	`+ logEndOfSearch(suitablePools);`
`106`	`106`
`107`	`107`	`return suitablePools;`
`108`	`108`	`}`
Original file line number	Diff line number	Diff line change
`@@ -46,6 +46,7 @@`
`46`	`46`	`import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;`
`47`	`47`	`import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;`
`48`	`48`	`import org.apache.cloudstack.storage.volume.VolumeObject;`
	`49`	`+import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;`
`49`	`50`	`import org.apache.log4j.Logger;`
`50`	`51`
`51`	`52`	`import com.cloud.agent.api.to.DataObjectType;`
`@@ -464,4 +465,9 @@ public StoragePoolType getParentPoolType() {`
`464`	`465`	`}`
`465`	`466`	`return null;`
`466`	`467`	`}`
	`468`	`+`
	`469`	`+ @Override`
	`470`	`+ public String toString() {`
	`471`	`+ return ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, "name", "uuid");`
	`472`	`+ }`
`467`	`473`	`}`
Original file line number	Diff line number	Diff line change
`@@ -73,7 +73,7 @@ public List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmProfi`
`73`	`73`	`}`
`74`	`74`	`}`
`75`	`75`
`76`		`- s_logger.debug(String.format("RandomStoragePoolAllocator is returning [%s] suitable storage pools [%s].", suitablePools.size(), suitablePools));`
	`76`	`+ logEndOfSearch(suitablePools);`
`77`	`77`
`78`	`78`	`return suitablePools;`
`79`	`79`	`}`