disk info in details

fix start ssvm traditionally comments cleaned
apache · nvazquez · May 28, 2020 · May 29, 2020 · May 29, 2020 · Jun 3, 2020
commit 91c784b2175c65d6705f321c3e7a8973607cf4e7
diff --git a/api/src/main/java/com/cloud/storage/ImageStore.java b/api/src/main/java/com/cloud/storage/ImageStore.java
@@ -23,6 +23,7 @@ public interface ImageStore extends Identity, InternalIdentity {
 
     String OVF_PROPERTY_PREFIX = "ACS-property-";
     String REQUIRED_NETWORK_PREFIX = "ACS-network-";
+    String DISK_DEFINITION_PREFIX = "ACS-disk-";
 
     /**
      * @return name of the object store.

diff --git a/core/src/main/java/com/cloud/agent/api/storage/DownloadAnswer.java b/core/src/main/java/com/cloud/agent/api/storage/DownloadAnswer.java
@@ -25,6 +25,7 @@
 import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.Command;
 import com.cloud.agent.api.LogLevel;
+import com.cloud.agent.api.to.DatadiskTO;
 import com.cloud.storage.VMTemplateStorageResourceAssoc;
 import com.cloud.storage.VMTemplateStorageResourceAssoc.Status;
 import org.apache.cloudstack.api.net.NetworkPrerequisiteTO;
@@ -44,6 +45,8 @@ public class DownloadAnswer extends Answer {
     private List<OVFPropertyTO> ovfProperties;
     @LogLevel(LogLevel.Log4jLevel.Off)
     private List<NetworkPrerequisiteTO> networkRequirements;
+    @LogLevel(LogLevel.Log4jLevel.Off)
+    private List<DatadiskTO> disks;
 
     public String getCheckSum() {
         return checkSum;
@@ -169,4 +172,12 @@ public List<NetworkPrerequisiteTO> getNetworkRequirements() {
     public void setNetworkRequirements(List<NetworkPrerequisiteTO> networkRequirements) {
         this.networkRequirements = networkRequirements;
     }
+
+    public List<DatadiskTO> getDisks() {
+        return disks;
+    }
+
+    public void setDisks(List<DatadiskTO> disks) {
+        this.disks = disks;
+    }
 }
diff --git a/core/src/main/java/com/cloud/storage/resource/StorageSubsystemCommandHandlerBase.java b/core/src/main/java/com/cloud/storage/resource/StorageSubsystemCommandHandlerBase.java
@@ -81,7 +81,7 @@ protected Answer execute(CopyCommand cmd) {
         DataTO destData = cmd.getDestTO();
         DataStoreTO srcDataStore = srcData.getDataStore();
         DataStoreTO destDataStore = destData.getDataStore();
-        // FR37 I should get here!
+
         if (srcData.getObjectType() == DataObjectType.TEMPLATE &&
             (srcData.getDataStore().getRole() == DataStoreRole.Image || srcData.getDataStore().getRole() == DataStoreRole.ImageCache) &&
             destData.getDataStore().getRole() == DataStoreRole.Primary) {

diff --git a/core/src/main/java/com/cloud/storage/template/OVAProcessor.java b/core/src/main/java/com/cloud/storage/template/OVAProcessor.java
@@ -78,7 +78,8 @@ public FormatInfo process(String templatePath, ImageFormat format, String templa
         setFileSystemAccessRights(templatePath);
 
         FormatInfo info = createFormatInfo(templatePath, templateName, templateFilePath, templateFileFullPath);
-        // The intention is to use the ova file as is for deployment and use done processing only for
+
+        // The intention is to use the ova file as is for deployment and use processing result only for
         // - property assessment and
         // - reconsiliation of
         // - - disks,
@@ -112,6 +113,9 @@ private void validateOva(String templateFileFullPath, FormatInfo info) throws In
         if (LOGGER.isTraceEnabled()) {
             LOGGER.trace(String.format("Found %d disks in template %s", CollectionUtils.isNotEmpty(disks) ? disks.size() : 0, ovfFilePath));
         }
+        if (CollectionUtils.isNotEmpty(disks)) {
+            info.disks = disks;
+        }
         List<NetworkPrerequisiteTO> nets = ovfHelper.getNetPrerequisitesFromDocument(doc);
         if (CollectionUtils.isNotEmpty(nets)) {
             LOGGER.info("Found " + nets.size() + " prerequisite networks");
@@ -126,9 +130,6 @@ private void validateOva(String templateFileFullPath, FormatInfo info) throws In
         } else if (LOGGER.isTraceEnabled()) {
             LOGGER.trace(String.format("no ovf properties found in template %s", ovfFilePath));
         }
-            // FR37 TODO if something is bad something will have been thrown above
-//            LOGGER.info("The ovf file " + ovfFilePath + " is invalid ", e);
-//            throw new InternalErrorException("OVA package has bad ovf file " + e.getMessage(), e);
     }
 
     private void setFileSystemAccessRights(String templatePath) {

diff --git a/core/src/main/java/com/cloud/storage/template/Processor.java b/core/src/main/java/com/cloud/storage/template/Processor.java
@@ -24,6 +24,7 @@
 import java.util.List;
 
 import com.cloud.agent.api.storage.OVFPropertyTO;
+import com.cloud.agent.api.to.DatadiskTO;
 import com.cloud.exception.InternalErrorException;
 import com.cloud.storage.Storage.ImageFormat;
 import com.cloud.utils.component.Adapter;
@@ -57,6 +58,7 @@ class FormatInfo {
         public boolean isCorrupted;
         public List<OVFPropertyTO> ovfProperties;
         public List<NetworkPrerequisiteTO> networks;
+        public List<DatadiskTO> disks;
     }
 
     long getVirtualSize(File file) throws IOException;

diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
@@ -1155,7 +1155,7 @@ public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfil
                     handlePath(vmTO.getDisks(), vm.getHypervisorType());
 
                     cmds = new Commands(Command.OnError.Stop);
-                    // FR37 TODO decide on extra info to put in start command for deployAsIs
+                    // FR37 TODO vmTO must contain definitions of disks to be expected for deployAsIs
                     cmds.addCommand(new StartCommand(vmTO, dest.getHost(), getExecuteInSequence(vm.getHypervisorType())));
 
                     vmGuru.finalizeDeployment(cmds, vmProfile, dest, ctx);

diff --git a/...stration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/...stration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java
@@ -140,7 +140,6 @@
 import com.cloud.vm.dao.UserVmCloneSettingDao;
 import com.cloud.vm.dao.UserVmDao;
 
-import static com.cloud.storage.resource.StorageProcessor.COPY_NOT_NEEDED_FOR_DEPLOY_AS_IS;
 import static com.cloud.storage.resource.StorageProcessor.REQUEST_TEMPLATE_RELOAD;
 
 public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrationService, Configurable {
@@ -1337,9 +1336,6 @@ private Pair<VolumeVO, DataStore> recreateVolume(VolumeVO vol, VirtualMachinePro
                     if (result.getResult().contains(REQUEST_TEMPLATE_RELOAD) && (i == 0)) {
                         s_logger.debug("Retry template re-deploy for vmware");
                         continue;
-                    } else if (result.getResult().contains(COPY_NOT_NEEDED_FOR_DEPLOY_AS_IS)) { // FR37 can be removed again ?
-                        s_logger.debug("template should be used from content library");
-                        break;
                     } else {
                         s_logger.debug("Unable to create " + newVol + ":" + result.getResult());
                         throw new StorageUnavailableException("Unable to create " + newVol + ":" + result.getResult(), destPool.getId());

diff --git a/...e/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java b/...e/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java
@@ -212,11 +212,23 @@ private void persistNetworkRequirements(List<NetworkPrerequisiteTO> networkRequi
             if (LOGGER.isTraceEnabled()) {
                 LOGGER.trace(String.format("saving property %s for template %d as detail", network.getName(), templateId));
             }
-            persistRequiredNetworkAsSetOfTemplateDetails(templateId, network);
+            persistRequiredNetworkAsASingleTemplateDetail(templateId, network);
         }
     }
 
-    private void persistRequiredNetworkAsSetOfTemplateDetails(long templateId, NetworkPrerequisiteTO network) {
+    private void persistDiskDefinitions(List<DatadiskTO> disks, long templateId) {
+        if (LOGGER.isTraceEnabled()) {
+            LOGGER.trace(String.format("saving disk definitionsn for template %d as details", templateId));
+        }
+        for (DatadiskTO disk : disks) {
+            if (LOGGER.isTraceEnabled()) {
+                LOGGER.trace(String.format("saving property %s for template %d as detail", disk.getDiskId(), templateId));
+            }
+            persistDiskDefinitionAsASingleTemplateDetail(templateId, disk);
+        }
+    }
+
+    private void persistRequiredNetworkAsASingleTemplateDetail(long templateId, NetworkPrerequisiteTO network) {
         String key = network.getName();
         String propKey = ImageStore.REQUIRED_NETWORK_PREFIX + key;
         try {
@@ -227,6 +239,17 @@ private void persistRequiredNetworkAsSetOfTemplateDetails(long templateId, Netwo
         }
     }
 
+    private void persistDiskDefinitionAsASingleTemplateDetail(long templateId, DatadiskTO disk) {
+        String key = disk.getDiskId();
+        String propKey = ImageStore.DISK_DEFINITION_PREFIX + key;
+        try {
+            String propValue = gson.toJson(disk);
+            savePropertyAttribute(templateId, propKey, propValue);
+        } catch (RuntimeException re) {
+            LOGGER.warn("gson marshalling of disk definition object fails: " + propKey,re);
+        }
+    }
+
     private void savePropertyAttribute(long templateId, String key, String value) {
         if ( templateDetailsDao.findDetail(templateId,key) != null) {
             LOGGER.debug(String.format("detail '%s' existed for template %d, deleting.", key, templateId));
@@ -250,11 +273,12 @@ protected Void createTemplateAsyncCallback(AsyncCallbackDispatcher<? extends Bas
         DataStore store = obj.getDataStore();
         List<OVFPropertyTO> ovfProperties = answer.getOvfProperties();
         List<NetworkPrerequisiteTO> networkRequirements = answer.getNetworkRequirements();
+        List<DatadiskTO> disks = answer.getDisks();
 
         TemplateDataStoreVO tmpltStoreVO = _templateStoreDao.findByStoreTemplate(store.getId(), obj.getId());
         if (tmpltStoreVO != null) {
             if (tmpltStoreVO.getDownloadState() == VMTemplateStorageResourceAssoc.Status.DOWNLOADED) {
-                persistExtraDetails(obj, ovfProperties, networkRequirements);
+                persistExtraDetails(obj, ovfProperties, networkRequirements, disks);
                 if (LOGGER.isDebugEnabled()) {
                     LOGGER.debug("Template is already in DOWNLOADED state, ignore further incoming DownloadAnswer");
                 }
@@ -294,15 +318,15 @@ protected Void createTemplateAsyncCallback(AsyncCallbackDispatcher<? extends Bas
                 templateDaoBuilder.setChecksum(answer.getCheckSum());
                 _templateDao.update(obj.getId(), templateDaoBuilder);
             }
-            persistExtraDetails(obj, ovfProperties, networkRequirements);
+            persistExtraDetails(obj, ovfProperties, networkRequirements, disks);
 
             CreateCmdResult result = new CreateCmdResult(null, null);
             caller.complete(result);
         }
         return null;
     }
 
-    private void persistExtraDetails(DataObject obj, List<OVFPropertyTO> ovfProperties, List<NetworkPrerequisiteTO> networkRequirements) {
+    private void persistExtraDetails(DataObject obj, List<OVFPropertyTO> ovfProperties, List<NetworkPrerequisiteTO> networkRequirements, List<DatadiskTO> disks) {
         if (LOGGER.isTraceEnabled()) {
             LOGGER.trace(String.format("saving %d ovf properties for template '%s' as details", ovfProperties != null ? ovfProperties.size() : 0, obj.getUuid()));
         }
@@ -315,6 +339,12 @@ private void persistExtraDetails(DataObject obj, List<OVFPropertyTO> ovfProperti
         if (CollectionUtils.isNotEmpty(networkRequirements)) {
             persistNetworkRequirements(networkRequirements, obj.getId());
         }
+        if (LOGGER.isTraceEnabled()) {
+            LOGGER.trace(String.format("saving %d disks definitions for template '%s' as details", disks != null ? disks.size() : 0, obj.getUuid()));
+        }
+        if (CollectionUtils.isNotEmpty(disks)) {
+            persistDiskDefinitions(disks, obj.getId());
+        }
     }
 
     protected Void

diff --git a/...isors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/StartCommandExecutor.java b/...isors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/StartCommandExecutor.java
@@ -26,6 +26,7 @@
 import java.util.List;
 import java.util.Map;
 
+import com.cloud.hypervisor.vmware.VmwareResourceException;
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.storage.configdrive.ConfigDrive;
 import org.apache.cloudstack.storage.to.TemplateObjectTO;
@@ -169,15 +170,17 @@ protected StartAnswer execute(StartCommand cmd) {
 
             VirtualMachineMO vmMo = hyperHost.findVmOnHyperHost(vmInternalCSName);
             // FR37 - this may need checking, if the first datastore is the right one - ideally it should be datastore where first disk is hosted
-            DatastoreMO dsRootVolumeIsOn = null; //getDatastoreThatRootDiskIsOn(dataStoresDetails, disks);
-            /*
-            if (dsRootVolumeIsOn == null) {
-                String msg = "Unable to locate datastore details of root volume";
-                LOGGER.error(msg);
-                // throw a more specific Exception
-                throw new Exception(msg);
+            DatastoreMO dsRootVolumeIsOn = null; //
+            if (! installAsIs) {
+                dsRootVolumeIsOn = getDatastoreThatRootDiskIsOn(dataStoresDetails, disks);
+
+                if (dsRootVolumeIsOn == null) {
+                    String msg = "Unable to locate datastore details of root volume";
+                    LOGGER.error(msg);
+                    // throw a more specific Exception
+                    throw new VmwareResourceException(msg);
+                }
             }
-            */
 
             VirtualMachineDiskInfoBuilder diskInfoBuilder = null;
             DiskControllerType systemVmScsiControllerType = DiskControllerType.lsilogic;

diff --git a/...rage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java b/...rage/server/src/main/java/org/apache/cloudstack/storage/template/DownloadManagerImpl.java
@@ -38,6 +38,7 @@
 import javax.naming.ConfigurationException;
 
 import com.cloud.agent.api.storage.OVFPropertyTO;
+import com.cloud.agent.api.to.DatadiskTO;
 import com.cloud.storage.template.Processor;
 import com.cloud.storage.template.S3TemplateDownloader;
 import com.cloud.storage.template.TemplateDownloader;
@@ -128,6 +129,7 @@ private static class DownloadJob {
         private final ResourceType resourceType;
         private List<OVFPropertyTO> ovfProperties;
         private List<NetworkPrerequisiteTO> networks;
+        private List<DatadiskTO> disks;
 
         public DownloadJob(TemplateDownloader td, String jobId, long id, String tmpltName, ImageFormat format, boolean hvm, Long accountId, String descr, String cksum,
                 String installPathPrefix, ResourceType resourceType) {
@@ -238,6 +240,14 @@ public List<NetworkPrerequisiteTO> getNetworks() {
         public void setNetworks(List<NetworkPrerequisiteTO> networks) {
             this.networks = networks;
         }
+
+        public List<DatadiskTO> getDisks() {
+            return disks;
+        }
+
+        public void setDisks(List<DatadiskTO> disks) {
+            this.disks = disks;
+        }
     }
 
     public static final Logger LOGGER = Logger.getLogger(DownloadManagerImpl.class);
@@ -537,6 +547,9 @@ private String postProcessAfterDownloadComplete(DownloadJob dnld, String resourc
                 if (CollectionUtils.isNotEmpty(info.networks)) {
                     dnld.setNetworks(info.networks);
                 }
+                if (CollectionUtils.isNotEmpty(info.disks)) {
+                    dnld.setDisks(info.disks);
+                }
                 break;
             }
         }
@@ -843,6 +856,9 @@ private DownloadAnswer handleDownloadProgressCmd(SecondaryStorageResource resour
             if (CollectionUtils.isNotEmpty(dj.getNetworks())) {
                 answer.setNetworkRequirements(dj.getNetworks());
             }
+            if (CollectionUtils.isNotEmpty(dj.getDisks())) {
+                answer.setDisks(dj.getDisks());
+            }
             jobs.remove(jobId);
             return answer;
         default: