Skip to content

[VMware to KVM Migration] Fix for converted instance NPE issue when source VMware instance OVF is exported from management server #11003

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

Merged
merged 1 commit into from
Jul 4, 2025
Merged

[VMware to KVM Migration] Fix for converted instance NPE issue when source VMware instance OVF is exported from management server #11003

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions api/src/main/java/com/cloud/storage/VolumeApiService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,13 @@ Volume updateVolume(long volumeId, String path, String state, Long storageId,
* </table>
*/
boolean doesStoragePoolSupportDiskOffering(StoragePool destPool, DiskOffering diskOffering);

/**
* Checks if the storage pool supports the required disk offering tags
* destPool the storage pool to check the disk offering tags
* diskOfferingTags the tags that should be supported
* return whether the tags are supported in the storage pool
*/
boolean doesStoragePoolSupportDiskOfferingTags(StoragePool destPool, String diskOfferingTags);

Volume destroyVolume(long volumeId, Account caller, boolean expunge, boolean forceExpunge);
Expand Down
16 changes: 0 additions & 16 deletions core/src/main/java/com/cloud/agent/api/ConvertInstanceAnswer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,13 @@
// under the License.
package com.cloud.agent.api;

import org.apache.cloudstack.vm.UnmanagedInstanceTO;

public class ConvertInstanceAnswer extends Answer {

private String temporaryConvertUuid;

public ConvertInstanceAnswer() {
super();
}
private UnmanagedInstanceTO convertedInstance;

public ConvertInstanceAnswer(Command command, boolean success, String details) {
super(command, success, details);
}

public ConvertInstanceAnswer(Command command, UnmanagedInstanceTO convertedInstance) {
super(command, true, "");
this.convertedInstance = convertedInstance;
}

public ConvertInstanceAnswer(Command command, String temporaryConvertUuid) {
super(command, true, "");
Expand All @@ -44,8 +32,4 @@ public ConvertInstanceAnswer(Command command, String temporaryConvertUuid) {
public String getTemporaryConvertUuid() {
return temporaryConvertUuid;
}

public UnmanagedInstanceTO getConvertedInstance() {
return convertedInstance;
}
}
11 changes: 1 addition & 10 deletions core/src/main/java/com/cloud/agent/api/ConvertInstanceCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,10 @@
import com.cloud.agent.api.to.RemoteInstanceTO;
import com.cloud.hypervisor.Hypervisor;

import java.util.List;

public class ConvertInstanceCommand extends Command {

private RemoteInstanceTO sourceInstance;
private Hypervisor.HypervisorType destinationHypervisorType;
private List<String> destinationStoragePools;
private DataStoreTO conversionTemporaryLocation;
private String templateDirOnConversionLocation;
private boolean checkConversionSupport;
Expand All @@ -36,12 +33,10 @@ public class ConvertInstanceCommand extends Command {
public ConvertInstanceCommand() {
}

public ConvertInstanceCommand(RemoteInstanceTO sourceInstance, Hypervisor.HypervisorType destinationHypervisorType,
List<String> destinationStoragePools, DataStoreTO conversionTemporaryLocation,
public ConvertInstanceCommand(RemoteInstanceTO sourceInstance, Hypervisor.HypervisorType destinationHypervisorType, DataStoreTO conversionTemporaryLocation,
String templateDirOnConversionLocation, boolean checkConversionSupport, boolean exportOvfToConversionLocation) {
this.sourceInstance = sourceInstance;
this.destinationHypervisorType = destinationHypervisorType;
this.destinationStoragePools = destinationStoragePools;
this.conversionTemporaryLocation = conversionTemporaryLocation;
this.templateDirOnConversionLocation = templateDirOnConversionLocation;
this.checkConversionSupport = checkConversionSupport;
Expand All @@ -56,10 +51,6 @@ public Hypervisor.HypervisorType getDestinationHypervisorType() {
return destinationHypervisorType;
}

public List<String> getDestinationStoragePools() {
return destinationStoragePools;
}

public DataStoreTO getConversionTemporaryLocation() {
return conversionTemporaryLocation;
}
Expand Down
205 changes: 6 additions & 199 deletions .../java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,12 @@
//
package com.cloud.hypervisor.kvm.resource.wrapper;

import java.io.BufferedInputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.URLEncoder;
import java.nio.charset.Charset;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
import java.util.stream.Collectors;

import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
import org.apache.cloudstack.vm.UnmanagedInstanceTO;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;

import com.cloud.agent.api.Answer;
Expand All @@ -44,17 +34,12 @@
import com.cloud.agent.api.to.RemoteInstanceTO;
import com.cloud.hypervisor.Hypervisor;
import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
import com.cloud.hypervisor.kvm.resource.LibvirtDomainXMLParser;
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef;
import com.cloud.hypervisor.kvm.storage.KVMPhysicalDisk;
import com.cloud.hypervisor.kvm.storage.KVMStoragePool;
import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager;
import com.cloud.resource.CommandWrapper;
import com.cloud.resource.ResourceWrapper;
import com.cloud.storage.Storage;
import com.cloud.utils.FileUtil;
import com.cloud.utils.Pair;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.utils.script.OutputInterpreter;
import com.cloud.utils.script.Script;

Expand All @@ -77,15 +62,15 @@
String msg = String.format("Cannot convert the instance %s from VMware as the virt-v2v binary is not found. " +
"Please install virt-v2v%s on the host before attempting the instance conversion.", sourceInstanceName, serverResource.isUbuntuHost()? ", nbdkit" : "");
logger.info(msg);
return new ConvertInstanceAnswer(cmd, false, msg);
return new Answer(cmd, false, msg);
}

if (!areSourceAndDestinationHypervisorsSupported(sourceHypervisorType, destinationHypervisorType)) {
String err = destinationHypervisorType != Hypervisor.HypervisorType.KVM ?
String.format("The destination hypervisor type is %s, KVM was expected, cannot handle it", destinationHypervisorType) :
String.format("The source hypervisor type %s is not supported for KVM conversion", sourceHypervisorType);
logger.error(err);
return new ConvertInstanceAnswer(cmd, false, err);
return new Answer(cmd, false, err);
}

final KVMStoragePoolManager storagePoolMgr = serverResource.getStoragePoolMgr();
Expand All @@ -103,7 +88,7 @@
if (StringUtils.isBlank(exportInstanceOVAUrl)) {
String err = String.format("Couldn't export OVA for the VM %s, due to empty url", sourceInstanceName);
logger.error(err);
return new ConvertInstanceAnswer(cmd, false, err);
return new Answer(cmd, false, err);

Check warning on line 91 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java#L91 

Added line #L91 was not covered by tests
}

int noOfThreads = cmd.getThreadsCountToExportOvf();
Expand All @@ -117,7 +102,7 @@
if (!ovfExported) {
String err = String.format("Export OVA for the VM %s failed", sourceInstanceName);
logger.error(err);
return new ConvertInstanceAnswer(cmd, false, err);
return new Answer(cmd, false, err);

Check warning on line 105 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java#L105 

Added line #L105 was not covered by tests
}
sourceOVFDirPath = String.format("%s%s/", sourceOVFDirPath, sourceInstanceName);
} else {
Expand All @@ -140,15 +125,15 @@
"has a different virt-v2v version.",
ovfTemplateDirOnConversionLocation);
logger.error(err);
return new ConvertInstanceAnswer(cmd, false, err);
return new Answer(cmd, false, err);
}
return new ConvertInstanceAnswer(cmd, temporaryConvertUuid);
} catch (Exception e) {
String error = String.format("Error converting instance %s from %s, due to: %s",
sourceInstanceName, sourceHypervisorType, e.getMessage());
logger.error(error, e);
cleanupSecondaryStorage = true;
return new ConvertInstanceAnswer(cmd, false, error);
return new Answer(cmd, false, error);

Check warning on line 136 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtConvertInstanceCommandWrapper.java#L136 

Added line #L136 was not covered by tests
} finally {
if (ovfExported && StringUtils.isNotBlank(ovfTemplateDirOnConversionLocation)) {
String sourceOVFDir = String.format("%s/%s", temporaryConvertPath, ovfTemplateDirOnConversionLocation);
Expand Down Expand Up @@ -205,55 +190,6 @@
encodedUsername, encodedPassword, vcenter, datacenter, vm);
}

protected List<KVMPhysicalDisk> getTemporaryDisksFromParsedXml(KVMStoragePool pool, LibvirtDomainXMLParser xmlParser, String convertedBasePath) {
List<LibvirtVMDef.DiskDef> disksDefs = xmlParser.getDisks();
disksDefs = disksDefs.stream().filter(x -> x.getDiskType() == LibvirtVMDef.DiskDef.DiskType.FILE &&
x.getDeviceType() == LibvirtVMDef.DiskDef.DeviceType.DISK).collect(Collectors.toList());
if (CollectionUtils.isEmpty(disksDefs)) {
String err = String.format("Cannot find any disk defined on the converted XML domain %s.xml", convertedBasePath);
logger.error(err);
throw new CloudRuntimeException(err);
}
sanitizeDisksPath(disksDefs);
return getPhysicalDisksFromDefPaths(disksDefs, pool);
}

private List<KVMPhysicalDisk> getPhysicalDisksFromDefPaths(List<LibvirtVMDef.DiskDef> disksDefs, KVMStoragePool pool) {
List<KVMPhysicalDisk> disks = new ArrayList<>();
for (LibvirtVMDef.DiskDef diskDef : disksDefs) {
KVMPhysicalDisk physicalDisk = pool.getPhysicalDisk(diskDef.getDiskPath());
disks.add(physicalDisk);
}
return disks;
}

protected List<KVMPhysicalDisk> getTemporaryDisksWithPrefixFromTemporaryPool(KVMStoragePool pool, String path, String prefix) {
String msg = String.format("Could not parse correctly the converted XML domain, checking for disks on %s with prefix %s", path, prefix);
logger.info(msg);
pool.refresh();
List<KVMPhysicalDisk> disksWithPrefix = pool.listPhysicalDisks()
.stream()
.filter(x -> x.getName().startsWith(prefix) && !x.getName().endsWith(".xml"))
.collect(Collectors.toList());
if (CollectionUtils.isEmpty(disksWithPrefix)) {
msg = String.format("Could not find any converted disk with prefix %s on temporary location %s", prefix, path);
logger.error(msg);
throw new CloudRuntimeException(msg);
}
return disksWithPrefix;
}

private void cleanupDisksAndDomainFromTemporaryLocation(List<KVMPhysicalDisk> disks,
KVMStoragePool temporaryStoragePool,
String temporaryConvertUuid) {
for (KVMPhysicalDisk disk : disks) {
logger.info(String.format("Cleaning up temporary disk %s after conversion from temporary location", disk.getName()));
temporaryStoragePool.deletePhysicalDisk(disk.getName(), Storage.ImageFormat.QCOW2);
}
logger.info(String.format("Cleaning up temporary domain %s after conversion from temporary location", temporaryConvertUuid));
FileUtil.deleteFiles(temporaryStoragePool.getLocalPath(), temporaryConvertUuid, ".xml");
}

protected void sanitizeDisksPath(List<LibvirtVMDef.DiskDef> disks) {
for (LibvirtVMDef.DiskDef disk : disks) {
String[] diskPathParts = disk.getDiskPath().split("/");
Expand All @@ -262,114 +198,6 @@
}
}

protected List<KVMPhysicalDisk> moveTemporaryDisksToDestination(List<KVMPhysicalDisk> temporaryDisks,
List<String> destinationStoragePools,
KVMStoragePoolManager storagePoolMgr) {
List<KVMPhysicalDisk> targetDisks = new ArrayList<>();
if (temporaryDisks.size() != destinationStoragePools.size()) {
String warn = String.format("Discrepancy between the converted instance disks (%s) " +
"and the expected number of disks (%s)", temporaryDisks.size(), destinationStoragePools.size());
logger.warn(warn);
}
for (int i = 0; i < temporaryDisks.size(); i++) {
String poolPath = destinationStoragePools.get(i);
KVMStoragePool destinationPool = storagePoolMgr.getStoragePool(Storage.StoragePoolType.NetworkFilesystem, poolPath);
if (destinationPool == null) {
String err = String.format("Could not find a storage pool by URI: %s", poolPath);
logger.error(err);
continue;
}
if (destinationPool.getType() != Storage.StoragePoolType.NetworkFilesystem) {
String err = String.format("Storage pool by URI: %s is not an NFS storage", poolPath);
logger.error(err);
continue;
}
KVMPhysicalDisk sourceDisk = temporaryDisks.get(i);
if (logger.isDebugEnabled()) {
String msg = String.format("Trying to copy converted instance disk number %s from the temporary location %s" +
" to destination storage pool %s", i, sourceDisk.getPool().getLocalPath(), destinationPool.getUuid());
logger.debug(msg);
}

String destinationName = UUID.randomUUID().toString();

KVMPhysicalDisk destinationDisk = storagePoolMgr.copyPhysicalDisk(sourceDisk, destinationName, destinationPool, 7200 * 1000);
targetDisks.add(destinationDisk);
}
return targetDisks;
}

private UnmanagedInstanceTO getConvertedUnmanagedInstance(String baseName,
List<KVMPhysicalDisk> vmDisks,
LibvirtDomainXMLParser xmlParser) {
UnmanagedInstanceTO instanceTO = new UnmanagedInstanceTO();
instanceTO.setName(baseName);
instanceTO.setDisks(getUnmanagedInstanceDisks(vmDisks, xmlParser));
instanceTO.setNics(getUnmanagedInstanceNics(xmlParser));
return instanceTO;
}

private List<UnmanagedInstanceTO.Nic> getUnmanagedInstanceNics(LibvirtDomainXMLParser xmlParser) {
List<UnmanagedInstanceTO.Nic> nics = new ArrayList<>();
if (xmlParser != null) {
List<LibvirtVMDef.InterfaceDef> interfaces = xmlParser.getInterfaces();
for (LibvirtVMDef.InterfaceDef interfaceDef : interfaces) {
UnmanagedInstanceTO.Nic nic = new UnmanagedInstanceTO.Nic();
nic.setMacAddress(interfaceDef.getMacAddress());
nic.setNicId(interfaceDef.getBrName());
nic.setAdapterType(interfaceDef.getModel().toString());
nics.add(nic);
}
}
return nics;
}

protected List<UnmanagedInstanceTO.Disk> getUnmanagedInstanceDisks(List<KVMPhysicalDisk> vmDisks, LibvirtDomainXMLParser xmlParser) {
List<UnmanagedInstanceTO.Disk> instanceDisks = new ArrayList<>();
List<LibvirtVMDef.DiskDef> diskDefs = xmlParser != null ? xmlParser.getDisks() : null;
for (int i = 0; i< vmDisks.size(); i++) {
KVMPhysicalDisk physicalDisk = vmDisks.get(i);
KVMStoragePool storagePool = physicalDisk.getPool();
UnmanagedInstanceTO.Disk disk = new UnmanagedInstanceTO.Disk();
disk.setPosition(i);
Pair<String, String> storagePoolHostAndPath = getNfsStoragePoolHostAndPath(storagePool);
disk.setDatastoreHost(storagePoolHostAndPath.first());
disk.setDatastorePath(storagePoolHostAndPath.second());
disk.setDatastoreName(storagePool.getUuid());
disk.setDatastoreType(storagePool.getType().name());
disk.setCapacity(physicalDisk.getVirtualSize());
disk.setFileBaseName(physicalDisk.getName());
if (CollectionUtils.isNotEmpty(diskDefs)) {
LibvirtVMDef.DiskDef diskDef = diskDefs.get(i);
disk.setController(diskDef.getBusType() != null ? diskDef.getBusType().toString() : LibvirtVMDef.DiskDef.DiskBus.VIRTIO.toString());
} else {
// If the job is finished but we cannot parse the XML, the guest VM can use the virtio driver
disk.setController(LibvirtVMDef.DiskDef.DiskBus.VIRTIO.toString());
}
instanceDisks.add(disk);
}
return instanceDisks;
}

protected Pair<String, String> getNfsStoragePoolHostAndPath(KVMStoragePool storagePool) {
String sourceHostIp = null;
String sourcePath = null;
List<String[]> commands = new ArrayList<>();
commands.add(new String[]{Script.getExecutableAbsolutePath("mount")});
commands.add(new String[]{Script.getExecutableAbsolutePath("grep"), storagePool.getLocalPath()});
String storagePoolMountPoint = Script.executePipedCommands(commands, 0).second();
logger.debug(String.format("NFS Storage pool: %s - local path: %s, mount point: %s", storagePool.getUuid(), storagePool.getLocalPath(), storagePoolMountPoint));
if (StringUtils.isNotEmpty(storagePoolMountPoint)) {
String[] res = storagePoolMountPoint.strip().split(" ");
res = res[0].split(":");
if (res.length > 1) {
sourceHostIp = res[0].strip();
sourcePath = res[1].strip();
}
}
return new Pair<>(sourceHostIp, sourcePath);
}

private boolean exportOVAFromVMOnVcenter(String vmExportUrl,
String targetOvfDir,
int noOfThreads,
Expand Down Expand Up @@ -412,27 +240,6 @@
return exitValue == 0;
}

protected LibvirtDomainXMLParser parseMigratedVMXmlDomain(String installPath) throws IOException {
String xmlPath = String.format("%s.xml", installPath);
if (!new File(xmlPath).exists()) {
String err = String.format("Conversion failed. Unable to find the converted XML domain, expected %s", xmlPath);
logger.error(err);
throw new CloudRuntimeException(err);
}
InputStream is = new BufferedInputStream(new FileInputStream(xmlPath));
String xml = IOUtils.toString(is, Charset.defaultCharset());
final LibvirtDomainXMLParser parser = new LibvirtDomainXMLParser();
try {
parser.parseDomainXML(xml);
return parser;
} catch (RuntimeException e) {
String err = String.format("Error parsing the converted instance XML domain at %s: %s", xmlPath, e.getMessage());
logger.error(err, e);
logger.debug(xml);
return null;
}
}

protected String encodeUsername(String username) {
return URLEncoder.encode(username, Charset.defaultCharset());
}
Expand Down
Loading
Loading