Skip to content

server: Fix NPE when migrating volume without template #7615

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gpordeus wants to merge 2 commits into from
Closed

server: Fix NPE when migrating volume without template #7615

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed992a9
PR
gpordeus Jul 4, 2023
38f47fb
Merge branch '4.18' into npe-migrate-host-and-pool
gpordeus Jul 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 54 additions & 21 deletions ...ain/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import com.cloud.storage.VMTemplateStorageResourceAssoc;
import com.cloud.storage.Volume;
import com.cloud.storage.VolumeVO;
import com.cloud.storage.dao.VMTemplateDao;
import com.cloud.storage.dao.VMTemplatePoolDao;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.vm.VirtualMachineManager;
Expand All @@ -68,6 +69,8 @@ public class KvmNonManagedStorageDataMotionStrategy extends StorageSystemDataMot
@Inject
private TemplateDataFactory templateDataFactory;
@Inject
private VMTemplateDao vmTemplateDao;
@Inject
private VMTemplatePoolDao vmTemplatePoolDao;
@Inject
private DataStoreManagerImpl dataStoreManagerImpl;
Expand Down Expand Up @@ -200,13 +203,45 @@ protected boolean shouldMigrateVolume(StoragePoolVO sourceStoragePool, Host dest
return supportStoragePoolType(sourceStoragePool.getPoolType());
}

protected boolean isTemplateCopyable(String skipLogMessage, VolumeInfo srcVolumeInfo, StoragePool destStoragePool) {
if (srcVolumeInfo.getTemplateId() == null) {
LOGGER.debug(String.format("%s volume [%s] does not have a template.", skipLogMessage, srcVolumeInfo.getId()));
return false;
}
if (srcVolumeInfo.getVolumeType() != Volume.Type.ROOT) {
LOGGER.debug(String.format("%s volume [%s] is not Root.", skipLogMessage, srcVolumeInfo.getId()));
return false;
}
if (vmTemplateDao.findById(srcVolumeInfo.getTemplateId()) == null) {
LOGGER.debug(String.format("%s template [%s] was removed.", skipLogMessage, srcVolumeInfo.getTemplateId()));
return false;
}
if (!isStoragePoolTypeInList(destStoragePool.getPoolType(), StoragePoolType.Filesystem, StoragePoolType.SharedMountPoint)) {
LOGGER.debug(String.format("%s storage pool type [%s] is not supported.", skipLogMessage, destStoragePool.getPoolType()));
return false;
}
return true;
}

protected boolean isCopyNeeded(String skipLogMessage, VolumeInfo srcVolumeInfo, VMTemplateStoragePoolVO sourceVolumeTemplateStoragePoolVO, DataStore sourceTemplateDataStore) {
if (sourceVolumeTemplateStoragePoolVO != null) {
LOGGER.debug(String.format("%s template [%s] already exists on target storage pool.", skipLogMessage, srcVolumeInfo.getTemplateId()));
return false;
}
if (sourceTemplateDataStore == null) {
LOGGER.debug(String.format("%s template [%s] data store not found.", skipLogMessage, srcVolumeInfo.getTemplateId()));
return false;
}
return true;
}
/**
* If the template is not on the target primary storage then it copies the template.
* If the template exists and is not on the target primary storage then it copies the template.
*/
@Override
protected void copyTemplateToTargetFilesystemStorageIfNeeded(VolumeInfo srcVolumeInfo, StoragePool srcStoragePool, DataStore destDataStore, StoragePool destStoragePool,
Host destHost) {
if (srcVolumeInfo.getVolumeType() != Volume.Type.ROOT || srcVolumeInfo.getTemplateId() == null) {
String skipLogMessage = String.format("Skipping copy template from source storage pool [%s] to target storage pool [%s] before migration as", srcStoragePool, destStoragePool);
if (!isTemplateCopyable(skipLogMessage, srcVolumeInfo, destStoragePool)) {
return;
}

Expand All @@ -217,28 +252,26 @@ protected void copyTemplateToTargetFilesystemStorageIfNeeded(VolumeInfo srcVolum
}

VMTemplateStoragePoolVO sourceVolumeTemplateStoragePoolVO = vmTemplatePoolDao.findByPoolTemplate(destStoragePool.getId(), srcVolumeInfo.getTemplateId(), null);
if (sourceVolumeTemplateStoragePoolVO == null && (isStoragePoolTypeInList(destStoragePool.getPoolType(), StoragePoolType.Filesystem, StoragePoolType.SharedMountPoint))) {
DataStore sourceTemplateDataStore = dataStoreManagerImpl.getRandomImageStore(srcVolumeInfo.getDataCenterId());
if (sourceTemplateDataStore != null) {
TemplateInfo sourceTemplateInfo = templateDataFactory.getTemplate(srcVolumeInfo.getTemplateId(), sourceTemplateDataStore);
TemplateObjectTO sourceTemplate = new TemplateObjectTO(sourceTemplateInfo);

LOGGER.debug(String.format("Could not find template [id=%s, name=%s] on the storage pool [id=%s]; copying the template to the target storage pool.",
srcVolumeInfo.getTemplateId(), sourceTemplateInfo.getName(), destDataStore.getId()));

TemplateInfo destTemplateInfo = templateDataFactory.getTemplate(srcVolumeInfo.getTemplateId(), destDataStore);
final TemplateObjectTO destTemplate = new TemplateObjectTO(destTemplateInfo);
Answer copyCommandAnswer = sendCopyCommand(destHost, sourceTemplate, destTemplate, destDataStore);

if (copyCommandAnswer != null && copyCommandAnswer.getResult()) {
updateTemplateReferenceIfSuccessfulCopy(srcVolumeInfo.getTemplateId(), destTemplateInfo.getUuid(), destDataStore.getId(), destTemplate.getSize());
}
return;
}
DataStore sourceTemplateDataStore = dataStoreManagerImpl.getRandomImageStore(srcVolumeInfo.getDataCenterId());
if (!isCopyNeeded(skipLogMessage, srcVolumeInfo, sourceVolumeTemplateStoragePoolVO, sourceTemplateDataStore)) {
return;
}

TemplateInfo sourceTemplateInfo = templateDataFactory.getTemplate(srcVolumeInfo.getTemplateId(), sourceTemplateDataStore);
TemplateObjectTO sourceTemplate = new TemplateObjectTO(sourceTemplateInfo);

LOGGER.debug(String.format("Copying template [%s] of volume [%s] from source storage pool [%s] to target storage pool [%s].", srcVolumeInfo.getTemplateId(), srcVolumeInfo.getId(), srcStoragePool.getId(), destStoragePool.getId()));

TemplateInfo destTemplateInfo = templateDataFactory.getTemplate(srcVolumeInfo.getTemplateId(), destDataStore);
final TemplateObjectTO destTemplate = new TemplateObjectTO(destTemplateInfo);
Answer copyCommandAnswer = sendCopyCommand(destHost, sourceTemplate, destTemplate, destDataStore);

if (copyCommandAnswer != null && copyCommandAnswer.getResult()) {
updateTemplateReferenceIfSuccessfulCopy(srcVolumeInfo.getTemplateId(), destTemplateInfo.getUuid(), destDataStore.getId(), destTemplate.getSize());
}
LOGGER.debug(String.format("Skipping 'copy template to target filesystem storage before migration' due to the template [%s] already exist on the storage pool [%s].", srcVolumeInfo.getTemplateId(), destStoragePool.getId()));
}


/**
* Update the template reference on table "template_spool_ref" (VMTemplateStoragePoolVO).
*/
Expand Down
51 changes: 35 additions & 16 deletions ...n/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1819,12 +1819,7 @@ public void copyAsync(Map<VolumeInfo, DataStore> volumeDataStoreMap, VirtualMach
continue;
}

if (srcVolumeInfo.getTemplateId() != null) {
LOGGER.debug(String.format("Copying template [%s] of volume [%s] from source storage pool [%s] to target storage pool [%s].", srcVolumeInfo.getTemplateId(), srcVolumeInfo.getId(), sourceStoragePool.getId(), destStoragePool.getId()));
copyTemplateToTargetFilesystemStorageIfNeeded(srcVolumeInfo, sourceStoragePool, destDataStore, destStoragePool, destHost);
} else {
LOGGER.debug(String.format("Skipping copy template from source storage pool [%s] to target storage pool [%s] before migration due to volume [%s] does not have a template.", sourceStoragePool.getId(), destStoragePool.getId(), srcVolumeInfo.getId()));
}
copyTemplateToTargetFilesystemStorageIfNeeded(srcVolumeInfo, sourceStoragePool, destDataStore, destStoragePool, destHost);

VolumeVO destVolume = duplicateVolumeOnAnotherStorage(srcVolume, destStoragePool);
VolumeInfo destVolumeInfo = _volumeDataFactory.getVolume(destVolume.getId(), destDataStore);
Expand Down Expand Up @@ -2028,16 +2023,40 @@ protected void copyTemplateToTargetFilesystemStorageIfNeeded(VolumeInfo srcVolum
/*
* Return backing file for volume (if any), only for KVM volumes
*/
String getVolumeBackingFile(VolumeInfo srcVolumeInfo) {
if (srcVolumeInfo.getHypervisorType() == HypervisorType.KVM &&
srcVolumeInfo.getTemplateId() != null && srcVolumeInfo.getPoolId() != null) {
VMTemplateVO template = _vmTemplateDao.findById(srcVolumeInfo.getTemplateId());
if (template.getFormat() != null && template.getFormat() != Storage.ImageFormat.ISO) {
VMTemplateStoragePoolVO ref = templatePoolDao.findByPoolTemplate(srcVolumeInfo.getPoolId(), srcVolumeInfo.getTemplateId(), null);
return ref != null ? ref.getInstallPath() : null;
}
protected String getVolumeBackingFile(VolumeInfo srcVolumeInfo) {
String logMessage = "Could not get volume backing file because";
if (srcVolumeInfo.getHypervisorType() != HypervisorType.KVM) {
LOGGER.debug(String.format("%s hypervisor is not KVM.", logMessage));
return null;
}
return null;
if (srcVolumeInfo.getTemplateId() == null) {
LOGGER.debug(String.format("%s volume does not have template.", logMessage));
return null;
}
if (srcVolumeInfo.getPoolId() == null) {
LOGGER.debug(String.format("%s volume is not on a storage pool.", logMessage));
return null;
}

LOGGER.debug(String.format("Searching for template [%s].", srcVolumeInfo.getTemplateId()));
VMTemplateVO template = _vmTemplateDao.findById(srcVolumeInfo.getTemplateId());

if (template == null) {
LOGGER.debug(String.format("Template [%s] was removed.", srcVolumeInfo.getTemplateId()));
return null;
}
if (template.getFormat() == null || template.getFormat() == Storage.ImageFormat.ISO) {
LOGGER.debug(String.format("Format [%s] of template [%s] is not valid for backing file.", template.getFormat(), srcVolumeInfo.getTemplateId()));
return null;
}

VMTemplateStoragePoolVO ref = templatePoolDao.findByPoolTemplate(srcVolumeInfo.getPoolId(), srcVolumeInfo.getTemplateId(), null);
if (ref == null) {
LOGGER.debug("Volume backing file not found in local storage pool.");
return null;
}
LOGGER.debug(String.format("Volume backing file install path [%s]", ref.getInstallPath()));
return ref.getInstallPath();
}

private void handlePostMigration(boolean success, Map<VolumeInfo, VolumeInfo> srcVolumeInfoToDestVolumeInfo, VirtualMachineTO vmTO, Host destHost) {
Expand Down Expand Up @@ -2937,4 +2956,4 @@ protected Boolean isStoragePoolTypeInList(StoragePoolType storagePoolTypeToValid

return supportedTypes.contains(storagePoolTypeToValidate);
};
}
}
Loading