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:
[email protected]
With regards,
Apache Git Services
