From 7a5a66336209138e4c510ce9e4f26e6416ff7975 Mon Sep 17 00:00:00 2001
From: Nicolas Vazquez <nicovazquez90@gmail.com>
Date: Fri, 27 Jan 2023 07:53:32 -0300
Subject: [PATCH] Fix for #220: memory leak on volume allocation (#224)

* Remove unnecessary logging

* Add marvin test

* Fix marvin test error
---
 .../com/cloud/deploy/DeployDestination.java   |  8 +--
 test/integration/smoke/test_vm_life_cycle.py  | 64 +++++++++++++++++++
 2 files changed, 65 insertions(+), 7 deletions(-)

diff --git a/api/src/main/java/com/cloud/deploy/DeployDestination.java b/api/src/main/java/com/cloud/deploy/DeployDestination.java
index 91b2068e94b..a7ceddadba8 100644
--- a/api/src/main/java/com/cloud/deploy/DeployDestination.java
+++ b/api/src/main/java/com/cloud/deploy/DeployDestination.java
@@ -147,13 +147,7 @@ public class DeployDestination implements Serializable {
         destination.append("Storage(");
         if (displayStorage && _storage != null) {
             StringBuffer storageBuf = new StringBuffer();
-            //String storageStr = "";
             for (Volume vol : _storage.keySet()) {
-                if (!storageBuf.toString().equals("")) {
-                    storageBuf.append(storageBuf.toString());
-                    storageBuf.append(", ");
-                }
-                storageBuf.append(storageBuf);
                 storageBuf.append("Volume(");
                 storageBuf.append(vol.getId());
                 storageBuf.append("|");
@@ -162,7 +156,7 @@ public class DeployDestination implements Serializable {
                 storageBuf.append(_storage.get(vol).getId());
                 storageBuf.append(")");
             }
-            destination.append(storageBuf.toString());
+            destination.append(storageBuf);
         }
         return destination.append(")]").toString();
     }
diff --git a/test/integration/smoke/test_vm_life_cycle.py b/test/integration/smoke/test_vm_life_cycle.py
index 9ae89137d3a..b56eab276bb 100644
--- a/test/integration/smoke/test_vm_life_cycle.py
+++ b/test/integration/smoke/test_vm_life_cycle.py
@@ -876,6 +876,70 @@ class TestVMLifeCycle(cloudstackTestCase):
 
         self.assertEqual(Volume.list(self.apiclient, id=vol1.id), None, "List response contains records when it should not")
 
+    @attr(tags = ["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="false")
+    def test_12_start_vm_multiple_volumes_allocated(self):
+        """Test attaching 14 datadisks and start VM
+        """
+
+        # Validate the following
+        # 1. Deploys a VM without starting it and attaches 14 datadisks to it
+        # 2. Start VM successfully
+        # 3. Destroys the VM with DataDisks option
+
+        custom_disk_offering = DiskOffering.list(self.apiclient, name='Custom')[0]
+
+        # Create VM without starting it
+        vm = VirtualMachine.create(
+            self.apiclient,
+            self.services["small"],
+            accountid=self.account.name,
+            domainid=self.account.domainid,
+            serviceofferingid=self.small_offering.id,
+            startvm=False
+        )
+
+        # Create and attach volumes
+        volume_ids = []
+        self.services["custom_volume"]["customdisksize"] = 1
+        self.services["custom_volume"]["zoneid"] = self.zone.id
+        for i in range(14):
+            volume = Volume.create_custom_disk(
+                self.apiclient,
+                self.services["custom_volume"],
+                account=self.account.name,
+                domainid=self.account.domainid,
+                diskofferingid=custom_disk_offering.id
+            )
+            volume_ids.append(volume.id)
+            VirtualMachine.attach_volume(vm, self.apiclient, volume)
+
+        # Start the VM
+        self.debug("Starting VM - ID: %s" % vm.id)
+        vm.start(self.apiclient)
+        list_vm_response = VirtualMachine.list(
+            self.apiclient,
+            id=vm.id
+        )
+        self.assertEqual(
+            isinstance(list_vm_response, list),
+            True,
+            "Check list response returns a valid list"
+        )
+        self.assertNotEqual(
+            len(list_vm_response),
+            0,
+            "Check VM available in List Virtual Machines"
+        )
+        self.assertEqual(
+            list_vm_response[0].state,
+            "Running",
+            "Check virtual machine is in running state"
+        )
+
+        # Destroy VM and volumes
+        self.debug("Destroy VM - ID: %s" % vm.id)
+        vm.delete(self.apiclient, volumeIds=volume_ids)
+
 
 class TestSecuredVmMigration(cloudstackTestCase):