[GitHub] [cloudstack] wido commented on a change in pull request #3280: Remove code that generated /var/lib/libvirt/images/null on target host

[GitBox <gi...@apache.org>]

wido commented on a change in pull request #3280: Remove code that generated /var/lib/libvirt/images/null on target host
URL: https://github.com/apache/cloudstack/pull/3280#discussion_r277066463
 
 

 ##########
 File path: engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java
 ##########
 @@ -111,28 +106,8 @@ protected StrategyPriority internalCanHandle(Map<VolumeInfo, DataStore> volumeMa
      * Example: /var/lib/libvirt/images/f3d49ecc-870c-475a-89fa-fd0124420a9b
      */
     @Override
-    protected String generateDestPath(VirtualMachineTO vmTO, VolumeVO srcVolume, Host destHost, StoragePoolVO destStoragePool, VolumeInfo destVolumeInfo) {
-        DiskOfferingVO diskOffering = _diskOfferingDao.findById(srcVolume.getDiskOfferingId());
-        DiskProfile diskProfile = new DiskProfile(destVolumeInfo, diskOffering, HypervisorType.KVM);
-        String templateUuid = getTemplateUuid(destVolumeInfo.getTemplateId());
-        CreateCommand rootImageProvisioningCommand = new CreateCommand(diskProfile, templateUuid, destStoragePool, true);
-
-        Answer rootImageProvisioningAnswer = agentManager.easySend(destHost.getId(), rootImageProvisioningCommand);
-
-        if (rootImageProvisioningAnswer == null) {
-            throw new CloudRuntimeException(String.format("Migration with storage of vm [%s] failed while provisioning root image", vmTO.getName()));
-        }
-
-        if (!rootImageProvisioningAnswer.getResult()) {
-            throw new CloudRuntimeException(String.format("Unable to modify target volume on the host [host id:%s, name:%s]", destHost.getId(), destHost.getName()));
-        }
-
-        String libvirtDestImgsPath = null;
-        if (rootImageProvisioningAnswer instanceof CreateAnswer) {
-            libvirtDestImgsPath = ((CreateAnswer)rootImageProvisioningAnswer).getVolume().getName();
-        }
-        // File.getAbsolutePath is used to keep the file separator as it should be and eliminate a verification to check if exists a file separator in the last character of libvirtDestImgsPath.
-        return new File(libvirtDestImgsPath, destVolumeInfo.getUuid()).getAbsolutePath();
+    protected String generateDestPath(Host destHost, StoragePoolVO destStoragePool, VolumeInfo destVolumeInfo) {
+        return new File(destStoragePool.getPath(), destVolumeInfo.getUuid()).getAbsolutePath();
 
 Review comment:
   You assume that all KVM storage is always a file. Although that is true for NFS and iSCSI, in Ceph's case this isn't.
   
   I haven't tested it, but this might become a problem with Ceph storage. You might want to look at this and maybe implement a getPath() function per type of storage.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services