GeezerVison is showing github.com — importVM improvements: RBD support, volume format, and cluster selection by abh1sar · Pull Request #14162 · apache/cloudstack
importVM improvements: RBD support, volume format, and cluster selection
#14162
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.
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
Uh oh!
There was an error while loading. Please reload this page.
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter
Filter by extension
.java
(10)
All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Hide whitespace
Diff view
Hide whitespace
There are no files selected for viewing
Original file line number
Diff line number
Diff line change
Expand Up
@@ -92,7 +92,7 @@ public class ImportVmCmd extends ImportUnmanagedInstanceCmd {
@Parameter(name = ApiConstants.DISK_PATH,
type = CommandType.STRING,
description = "path of the disk image")
description = "path of the disk image. It is the file name on file based storage pools (NFS, Local, SharedMountPoint), and the image name on RBD storage pools ")
private String diskPath;
@Parameter(name = ApiConstants.IMPORT_SOURCE,
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -63,7 +63,7 @@ public class ImportVolumeCmd extends BaseAsyncCmd {
@Parameter(name = ApiConstants.PATH,
type = BaseCmd.CommandType.STRING,
required = true,
description = "the path of the volume")
description = "the path of the volume. It is the file name on file based storage pools (NFS, Local, SharedMountPoint), and the image name on RBD storage pools ")
private String path;
@Parameter(name = ApiConstants.NAME,
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -183,10 +183,12 @@ List<DiskProfile> allocateTemplatedVolumes(Type type, String name, DiskOffering
*/
DiskProfile importVolume(Type type, String name, DiskOffering offering, Long sizeInBytes, Long minIops, Long maxIops,
Long zoneId, HypervisorType hypervisorType, VirtualMachine vm, VirtualMachineTemplate template,
Account owner, Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo);
Account owner, Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo,
Storage.ImageFormat format);
DiskProfile updateImportedVolume(Type type, DiskOffering offering, VirtualMachine vm, VirtualMachineTemplate template,
Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo, DiskProfile diskProfile);
Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo, DiskProfile diskProfile,
Storage.ImageFormat format);
/**
* Unmanage VM volumes
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -2328,7 +2328,8 @@ public void updateVolumeDiskChain(long volumeId, String path, String chainInfo,
@Override
public DiskProfile importVolume(Type type, String name, DiskOffering offering, Long sizeInBytes, Long minIops, Long maxIops,
Long zoneId, HypervisorType hypervisorType, VirtualMachine vm, VirtualMachineTemplate template, Account owner,
Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo) {
Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo,
ImageFormat format) {
if (sizeInBytes == null) {
sizeInBytes = offering.getDiskSize();
}
Expand Down
Expand Up
@@ -2367,7 +2368,9 @@ public DiskProfile importVolume(Type type, String name, DiskOffering offering, L
vol.setDisplayVolume(userVm.isDisplayVm());
}
vol.setFormat(getSupportedImageFormatForCluster(hypervisorType));
// The format the hypervisor actually reported for the existing image wins; pools such as RBD
// hold raw images even though QCOW2 is the cluster default for KVM.
vol.setFormat(format != null ? format : getSupportedImageFormatForCluster(hypervisorType));
vol.setPoolId(poolId);
vol.setPoolType(poolType);
vol.setPath(path);
Expand All
@@ -2379,7 +2382,8 @@ public DiskProfile importVolume(Type type, String name, DiskOffering offering, L
@Override
public DiskProfile updateImportedVolume(Type type, DiskOffering offering, VirtualMachine vm, VirtualMachineTemplate template,
Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo, DiskProfile diskProfile) {
Long deviceId, Long poolId, Storage.StoragePoolType poolType, String path, String chainInfo, DiskProfile diskProfile,
ImageFormat format) {
VolumeVO vol = _volsDao.findById(diskProfile.getVolumeId());
if (vm != null) {
Expand Down
Expand Up
@@ -2411,7 +2415,9 @@ public DiskProfile updateImportedVolume(Type type, DiskOffering offering, Virtua
vol.setDisplayVolume(userVm.isDisplayVm());
}
vol.setFormat(getSupportedImageFormatForCluster(vm.getHypervisorType()));
// The format the hypervisor actually reported for the existing image wins; pools such as RBD
// hold raw images even though QCOW2 is the cluster default for KVM.
vol.setFormat(format != null ? format : getSupportedImageFormatForCluster(vm.getHypervisorType()));
vol.setPoolId(poolId);
vol.setPoolType(poolType);
vol.setPath(path);
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -241,7 +241,7 @@ public void testImportVolume() {
volumeOrchestrator.importVolume(volumeType, name, diskOffering, sizeInBytes, null, null,
zoneId, hypervisorType, null, null, owner,
deviceId, poolId, Storage.StoragePoolType.NetworkFilesystem, path, chainInfo);
deviceId, poolId, Storage.StoragePoolType.NetworkFilesystem, path, chainInfo, null );
VolumeVO volume = volumeVOMockedConstructionConstruction.constructed().get(0);
Mockito.verify(volume, Mockito.never()).setInstanceId(Mockito.anyLong());
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -50,7 +50,8 @@ public final class LibvirtCheckVolumeCommandWrapper extends CommandWrapper<Check
private static final List<Storage.StoragePoolType> STORAGE_POOL_TYPES_SUPPORTED = Arrays.asList(
Storage.StoragePoolType.Filesystem,
Storage.StoragePoolType.NetworkFilesystem,
Storage.StoragePoolType.SharedMountPoint);
Storage.StoragePoolType.SharedMountPoint,
Storage.StoragePoolType.RBD);
@Override
public Answer execute(final CheckVolumeCommand command, final LibvirtComputingResource libvirtComputingResource) {
Expand All
@@ -64,13 +65,24 @@ public Answer execute(final CheckVolumeCommand command, final LibvirtComputingRe
if (STORAGE_POOL_TYPES_SUPPORTED.contains(storageFilerTO.getType())) {
final KVMPhysicalDisk vol = pool.getPhysicalDisk(srcFile);
final String path = vol.getPath();
try {
KVMPhysicalDisk.checkQcow2File(path);
} catch (final CloudRuntimeException e) {
return new CheckVolumeAnswer(command, false, "", 0, getVolumeDetails(pool, vol));
final boolean isRbd = Storage.StoragePoolType.RBD.equals(storageFilerTO.getType());
Map<VolumeOnStorageTO.Detail, String> volumeDetails = getVolumeDetails(pool, vol);
if (MapUtils.isEmpty(volumeDetails)) {
return new Answer(command, false, "Unable to read the volume on the storage pool");
}
if (!isRbd) {
Comment thread
weizhouapache marked this conversation as resolved.
Resolved
Uh oh!
There was an error while loading. Please reload this page.
try {
KVMPhysicalDisk.checkQcow2File(path);
} catch (final CloudRuntimeException e) {
return new CheckVolumeAnswer(command, false, "", 0, volumeDetails);
}
}
long size = KVMPhysicalDisk.getVirtualSizeFromFile(path);
// Images on RBD are raw and the path is an image name that qemu-img cannot open
// without the rbd: URI, so take the size libvirt already reported for the volume.
long size = isRbd ? vol.getVirtualSize() : KVMPhysicalDisk.getVirtualSizeFromFile(path);
return new CheckVolumeAnswer(command, true, "", size, getVolumeDetails(pool, vol));
Comment thread
weizhouapache marked this conversation as resolved.
Resolved
Uh oh!
There was an error while loading. Please reload this page.
} else {
return new Answer(command, false, "Unsupported Storage Pool");
Expand Down
Expand Up
@@ -122,6 +134,9 @@ private Map<String, String> getDiskFileInfo(KVMStoragePool pool, KVMPhysicalDisk
try {
QemuImg qemu = new QemuImg(0);
QemuImgFile qemuFile = new QemuImgFile(disk.getPath(), disk.getFormat());
if (Storage.StoragePoolType.RBD.equals(pool.getType())) {
qemuFile = new QemuImgFile(KVMPhysicalDisk.RBDStringBuilder(pool, disk.getPath()), disk.getFormat());
}
return qemu.info(qemuFile, secure);
} catch (QemuImgException | LibvirtException ex) {
logger.error("Failed to get info of disk file: " + ex.getMessage());
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -461,10 +461,27 @@ private VolumeVO importVolumeInternal(VolumeOnStorageTO volume, DiskOfferingVO d
Account owner, StoragePoolVO pool, String volumeName) {
DiskProfile diskProfile = volumeManager.importVolume(Volume.Type.DATADISK, volumeName, diskOffering,
volume.getVirtualSize(), null, null, pool.getDataCenterId(), volume.getHypervisorType(), null, null,
owner, null, pool.getId(), pool.getPoolType(), volume.getPath(), null);
owner, null, pool.getId(), pool.getPoolType(), volume.getPath(), null, getImageFormat(volume.getFormat()) );
return volumeDao.findById(diskProfile.getVolumeId());
}
/**
* Maps the format the hypervisor reported for the volume on the pool onto an image format, so that
* the imported volume records what is actually on the pool (raw on RBD, qcow2 on file based pools)
* instead of the cluster default for the hypervisor. Returns null when the format is not recognised.
*/
protected Storage.ImageFormat getImageFormat(String format) {
if (StringUtils.isBlank(format)) {
return null;
}
try {
return Storage.ImageFormat.valueOf(format.toUpperCase());
} catch (IllegalArgumentException e) {
logger.warn("Unrecognised image format {} reported for the volume being imported, falling back to the hypervisor default", format);
return null;
}
}
protected void checkResourceLimitForImportVolume(Account owner, VolumeOnStorageTO volume, DiskOfferingVO diskOffering, List<Reserver> reservations) {
Long volumeSize = volume.getVirtualSize();
try {
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -796,14 +796,16 @@ private Pair<DiskProfile, StoragePool> importExternalDisk(UnmanagedInstanceTO.Di
}
diskProfile.setSize(copyRemoteVolumeAnswer.getSize());
DiskProfile profile = volumeManager.updateImportedVolume(type, diskOffering, vm, template, deviceId,
storagePool.getId(), storagePool.getPoolType(), copyRemoteVolumeAnswer.getFilename(), chainInfo, diskProfile);
storagePool.getId(), storagePool.getPoolType(), copyRemoteVolumeAnswer.getFilename(), chainInfo, diskProfile,
getImageFormatFromVolumeDetails(copyRemoteVolumeAnswer.getVolumeDetails()));
return new Pair<>(profile, storagePool);
}
private Pair<DiskProfile, StoragePool> importKVMLocalDisk(VirtualMachine vm, DiskOffering diskOffering,
Volume.Type type, VirtualMachineTemplate template,
Long deviceId, Long hostId, String diskPath, DiskProfile diskProfile) {
Long deviceId, Long hostId, String diskPath, DiskProfile diskProfile,
Storage.ImageFormat format) {
List<StoragePoolVO> storagePools = primaryDataStoreDao.findLocalStoragePoolsByHostAndTags(hostId, null);
if(storagePools.size() < 1) {
throw new CloudRuntimeException("Local Storage not found for host");
Expand All
@@ -812,22 +814,44 @@ private Pair<DiskProfile, StoragePool> importKVMLocalDisk(VirtualMachine vm, Dis
StoragePool storagePool = storagePools.get(0);
DiskProfile profile = volumeManager.updateImportedVolume(type, diskOffering, vm, template, deviceId,
storagePool.getId(), storagePool.getPoolType(), diskPath, null, diskProfile);
storagePool.getId(), storagePool.getPoolType(), diskPath, null, diskProfile, format );
return new Pair<>(profile, storagePool);
}
private Pair<DiskProfile, StoragePool> importKVMSharedDisk(VirtualMachine vm, DiskOffering diskOffering,
Volume.Type type, VirtualMachineTemplate template,
Long deviceId, Long poolId, String diskPath, DiskProfile diskProfile) {
Long deviceId, Long poolId, String diskPath, DiskProfile diskProfile,
Storage.ImageFormat format) {
StoragePool storagePool = primaryDataStoreDao.findById(poolId);
DiskProfile profile = volumeManager.updateImportedVolume(type, diskOffering, vm, template, deviceId,
poolId, storagePool.getPoolType(), diskPath, null, diskProfile);
poolId, storagePool.getPoolType(), diskPath, null, diskProfile, format );
return new Pair<>(profile, storagePool);
}
/**
* Reads the image format the hypervisor reported for an existing volume, so that the imported
* volume records what is actually on the pool (raw on RBD, qcow2 on file based pools) instead of
* the cluster default for the hypervisor. Returns null when the agent did not report a format.
*/
protected Storage.ImageFormat getImageFormatFromVolumeDetails(Map<VolumeOnStorageTO.Detail, String> volumeDetails) {
if (MapUtils.isEmpty(volumeDetails)) {
return null;
}
String fileFormat = volumeDetails.get(VolumeOnStorageTO.Detail.FILE_FORMAT);
if (StringUtils.isBlank(fileFormat)) {
return null;
}
try {
return Storage.ImageFormat.valueOf(fileFormat.toUpperCase());
} catch (IllegalArgumentException e) {
logger.warn("Unrecognised image format {} reported for the volume being imported, falling back to the hypervisor default", fileFormat);
return null;
}
}
private Pair<DiskProfile, StoragePool> importDisk(UnmanagedInstanceTO.Disk disk, VirtualMachine vm, Cluster cluster, DiskOffering diskOffering,
Volume.Type type, String name, Long diskSize, Long minIops, Long maxIops, VirtualMachineTemplate template,
Account owner, Long deviceId) {
Expand All
@@ -842,7 +866,7 @@ private Pair<DiskProfile, StoragePool> importDisk(UnmanagedInstanceTO.Disk disk,
}
StoragePool storagePool = getStoragePool(disk, zone, cluster, diskOffering);
DiskProfile profile = volumeManager.importVolume(type, name, diskOffering, diskSize,
minIops, maxIops, vm.getDataCenterId(), vm.getHypervisorType(), vm, template, owner, deviceId, storagePool.getId(), storagePool.getPoolType(), path, chainInfo);
minIops, maxIops, vm.getDataCenterId(), vm.getHypervisorType(), vm, template, owner, deviceId, storagePool.getId(), storagePool.getPoolType(), path, chainInfo, null );
return new Pair<DiskProfile, StoragePool>(profile, storagePool);
}
Expand Down
Expand Up
@@ -2647,6 +2671,13 @@ private UserVmResponse importKvmInstance(ImportVmCmd cmd) {
throw new InvalidParameterValueException("Disk image is already in use");
}
// A host the planner is pinned to must be able to see the pool, otherwise the volume check
// runs on a host that cannot reach the image and reports it as missing.
if (ImportSource.SHARED == importSource && hostId != null && storagePoolHostDao.findByPoolHost(poolId, hostId) == null) {
throw new InvalidParameterValueException(String.format(
"Specified host does not have access to the storage pool: %s", storagePool.getUuid()));
}
DiskOffering diskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId());
if (diskOffering != null && !storagePoolSupportsDiskOffering(storagePool, diskOffering)) {
Expand Down
Expand Up
@@ -2923,7 +2954,13 @@ private UserVm importKvmVirtualMachineFromDisk(final ImportSource importSource,
ServiceOfferingVO dummyOffering = serviceOfferingDao.findById(userVm.getId(), serviceOffering.getId());
profile.setServiceOffering(dummyOffering);
DeploymentPlanner.ExcludeList excludeList = new DeploymentPlanner.ExcludeList();
final DataCenterDeployment plan = new DataCenterDeployment(zone.getId(), null, null, hostId, poolId, null);
// Confine the plan to the pod and cluster of the pool the caller asked for. Otherwise the
// planner is free to pick a host in another cluster that cannot see the pool, and the volume
// check then runs against whichever pool that cluster does have. Both are null for a zone
// wide pool, which every host can see.
StoragePoolVO importStoragePool = primaryDataStoreDao.findById(poolId);
final DataCenterDeployment plan = new DataCenterDeployment(zone.getId(), importStoragePool.getPodId(),
importStoragePool.getClusterId(), hostId, poolId, null);
DeployDestination dest = null;
try {
dest = deploymentPlanningManager.planDeployment(profile, plan, excludeList, null);
Expand Down
Expand Up
@@ -2968,12 +3005,13 @@ private UserVm importKvmVirtualMachineFromDisk(final ImportSource importSource,
List<Pair<DiskProfile, StoragePool>> diskProfileStoragePoolList = new ArrayList<>();
try {
long deviceId = 1L;
Storage.ImageFormat diskFormat = getImageFormatFromVolumeDetails(checkVolumeAnswer.getVolumeDetails());
if(ImportSource.SHARED == importSource) {
diskProfileStoragePoolList.add(importKVMSharedDisk(userVm, diskOffering, Volume.Type.ROOT,
template, deviceId, poolId, diskPath, diskProfile));
template, deviceId, poolId, diskPath, diskProfile, diskFormat ));
} else if(ImportSource.LOCAL == importSource) {
diskProfileStoragePoolList.add(importKVMLocalDisk(userVm, diskOffering, Volume.Type.ROOT,
template, deviceId, hostId, diskPath, diskProfile));
template, deviceId, hostId, diskPath, diskProfile, diskFormat ));
}
} catch (Exception e) {
logger.error(String.format("Failed to import volumes while importing vm: %s", instanceName), e);
Expand Down
Loading
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Loading
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.