DaanHoogland commented on a change in pull request #3828: [WIP DO NOT MERGE] 
[KVM] Direct download agnostic of the storage provider
URL: https://github.com/apache/cloudstack/pull/3828#discussion_r369455650
 
 

 ##########
 File path: 
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
 ##########
 @@ -1693,38 +1696,77 @@ private DirectTemplateDownloader 
getDirectTemplateDownloaderFromCommand(DirectDo
     @Override
     public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand 
cmd) {
         final PrimaryDataStoreTO pool = cmd.getDestPool();
-        if (!pool.getPoolType().equals(StoragePoolType.NetworkFilesystem)) {
-            return new DirectDownloadAnswer(false, "Unsupported pool type " + 
pool.getPoolType().toString(), true);
-        }
-        KVMStoragePool destPool = 
storagePoolMgr.getStoragePool(pool.getPoolType(), pool.getUuid());
         DirectTemplateDownloader downloader;
+        KVMPhysicalDisk template;
 
         try {
-            downloader = getDirectTemplateDownloaderFromCommand(cmd, destPool);
-        } catch (IllegalArgumentException e) {
-            return new DirectDownloadAnswer(false, "Unable to create direct 
downloader: " + e.getMessage(), true);
-        }
+            s_logger.info("Verifying temporary location for downloading the 
template exists on the host");
+            String temporaryDownloadPath = 
resource.getDirectDownloadTemporaryDownloadPath();
+            if (!isLocationAccessible(temporaryDownloadPath)) {
+                String msg = "The temporary location path for downloading 
templates does not exist: " +
+                        temporaryDownloadPath + " on this host";
+                s_logger.error(msg);
+                return new DirectDownloadAnswer(false, msg, true);
+            }
 
-        try {
+            s_logger.info("Checking for free space on the host for downloading 
the template");
+            if 
(!isEnoughSpaceForDownloadTemplateOnTemporaryLocation(cmd.getTemplateSize())) {
+                String msg = "Not enough space on the defined temporary 
location to download the template " + cmd.getTemplateId();
+                s_logger.error(msg);
+                return new DirectDownloadAnswer(false, msg, true);
+            }
+
+            KVMStoragePool destPool = 
storagePoolMgr.getStoragePool(pool.getPoolType(), pool.getUuid());
+            downloader = getDirectTemplateDownloaderFromCommand(cmd, destPool, 
temporaryDownloadPath);
             s_logger.info("Trying to download template");
-            if (!downloader.downloadTemplate()) {
+            Pair<Boolean, String> result = downloader.downloadTemplate();
+            if (!result.first()) {
                 s_logger.warn("Couldn't download template");
                 return new DirectDownloadAnswer(false, "Unable to download 
template", true);
             }
+            String tempFilePath = result.second();
             if (!downloader.validateChecksum()) {
                 s_logger.warn("Couldn't validate template checksum");
                 return new DirectDownloadAnswer(false, "Checksum validation 
failed", false);
             }
-            if (!downloader.extractAndInstallDownloadedTemplate()) {
-                s_logger.warn("Couldn't extract and install template");
-                return new DirectDownloadAnswer(false, "Extraction and 
installation failed", false);
-            }
+            template = 
storagePoolMgr.createPhysicalDiskFromDirectDownloadTemplate(tempFilePath, 
destPool, 100);
         } catch (CloudRuntimeException e) {
             s_logger.warn("Error downloading template " + cmd.getTemplateId() 
+ " due to: " + e.getMessage());
             return new DirectDownloadAnswer(false, "Unable to download 
template: " + e.getMessage(), true);
+        } catch (IllegalArgumentException e) {
+            return new DirectDownloadAnswer(false, "Unable to create direct 
downloader: " + e.getMessage(), true);
         }
 
-        DirectTemplateInformation info = downloader.getTemplateInformation();
-        return new DirectDownloadAnswer(true, info.getSize(), 
info.getInstallPath());
+        return new DirectDownloadAnswer(true, template.getSize(), 
template.getName());
+    }
+
+    /**
+     * True if location exists
+     */
+    private boolean isLocationAccessible(String temporaryDownloadPath) {
+        File dir = new File(temporaryDownloadPath);
+        return dir.exists();
+    }
+
+    /**
+     * Perform a free space check on the host for downloading the direct 
download templates
+     * @param templateSize template size obtained from remote server when 
registering the template (in bytes)
+     */
+    protected boolean isEnoughSpaceForDownloadTemplateOnTemporaryLocation(Long 
templateSize) {
+        if (templateSize == null || templateSize == 0L) {
+            s_logger.info("The server did not provide the template size, 
assuming there is enough space to download it");
 
 Review comment:
   👍 this one I agree is good for the operator to have.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to