Skip to content

vmware: fix stopped VM volume migration #4758

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
rohityadavcloud merged 16 commits into from
Apr 24, 2021
Merged

vmware: fix stopped VM volume migration #4758

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
80f9b4c
prevent other vm disks getting deleted
shwstppr Mar 5, 2021
54ef6e7
vmware: fix inter-cluster stopped vm migration
shwstppr Mar 31, 2021
7cc6b6e
fix detached volume inter-cluster migration
shwstppr Apr 6, 2021
ee6aacd
cleanup unused method
shwstppr Apr 6, 2021
db2363d
review changes
shwstppr Apr 7, 2021
e172623
changes
shwstppr Apr 7, 2021
691ca70
vmware: allow attached volume migration using VmwareStorageMotionStra…
shwstppr Apr 5, 2021
9973e57
find vm clusterid with multiple ROOT volumes
shwstppr Apr 8, 2021
2356894
fix successive storage migration
shwstppr Apr 8, 2021
80fdcbd
Merge branch '4.15' of github.com:apache/cloudstack into fix-vm-volmi…
shwstppr Apr 8, 2021
28ae509
Merge branch 'fix-intercluster-migration-4838' into fix-vm-volmigrate…
shwstppr Apr 8, 2021
79bb82c
fix intercluster check
shwstppr Apr 11, 2021
b5bbe9f
Merge branch '4.15' into fix-vm-volmigrate-bug
shwstppr Apr 11, 2021
5d685a9
refactor vm cluster, host method
shwstppr Apr 12, 2021
1fdc825
remove inter-pod check
shwstppr Apr 13, 2021
5e89cb0
address review comment
shwstppr Apr 15, 2021
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
4 changes: 2 additions & 2 deletions core/src/main/java/com/cloud/agent/api/storage/MigrateVolumeCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ public MigrateVolumeCommand(long volumeId, String volumePath, StoragePool pool,
this.setWait(timeout);
}

public MigrateVolumeCommand(long volumeId, String volumePath, StoragePool sourcePool, StoragePool targetPool, String hostGuidInTargetCluster) {
this(volumeId,volumePath,targetPool, null, Volume.Type.UNKNOWN, -1);
public MigrateVolumeCommand(long volumeId, String volumePath, String attachedVmName, StoragePool sourcePool, StoragePool targetPool, String hostGuidInTargetCluster) {
this(volumeId,volumePath,targetPool, attachedVmName, Volume.Type.UNKNOWN, -1);
this.sourcePool = new StorageFilerTO(sourcePool);
this.hostGuidInTargetCluster = hostGuidInTargetCluster;
}
Expand Down
3 changes: 3 additions & 0 deletions engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import com.cloud.template.VirtualMachineTemplate;
import com.cloud.user.Account;
import com.cloud.uservm.UserVm;
import com.cloud.utils.Pair;
import com.cloud.utils.component.Manager;
import com.cloud.utils.fsm.NoTransitionException;

Expand Down Expand Up @@ -255,4 +256,6 @@ static String getHypervisorHostname(String name) {
boolean unmanage(String vmUuid);

UserVm restoreVirtualMachine(long vmId, Long newTemplateId) throws ResourceUnavailableException, InsufficientCapacityException;

Pair<Long, Long> findClusterAndHostIdForVm(long vmId);
}
86 changes: 49 additions & 37 deletions engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2294,43 +2294,6 @@ private void markVolumesInPool(VMInstanceVO vm, StoragePool destPool, Answer[] h
}
}

private Pair<Long, Long> findClusterAndHostIdForVm(VMInstanceVO vm) {
Long hostId = vm.getHostId();
Long clusterId = null;
// OfflineVmwareMigration: probably this is null when vm is stopped
if(hostId == null) {
hostId = vm.getLastHostId();
if (s_logger.isDebugEnabled()) {
s_logger.debug(String.format("host id is null, using last host id %d", hostId) );
}
}
if (hostId == null) {
List<VolumeVO> volumes = _volsDao.findByInstanceAndType(vm.getId(), Type.ROOT);
if (CollectionUtils.isNotEmpty(volumes)) {
for (VolumeVO rootVolume : volumes) {
if (rootVolume.getPoolId() != null) {
StoragePoolVO pool = _storagePoolDao.findById(rootVolume.getPoolId());
if (pool != null && pool.getClusterId() != null) {
clusterId = pool.getClusterId();
List<HostVO> hosts = _hostDao.findHypervisorHostInCluster(pool.getClusterId());
if (CollectionUtils.isNotEmpty(hosts)) {
hostId = hosts.get(0).getId();
break;
}
}
}
}
}
}
if (clusterId == null && hostId != null) {
HostVO host = _hostDao.findById(hostId);
if (host != null) {
clusterId = host.getClusterId();
}
}
return new Pair<>(clusterId, hostId);
}

private void migrateThroughHypervisorOrStorage(StoragePool destPool, VMInstanceVO vm) throws StorageUnavailableException, InsufficientCapacityException {
final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm);
Pair<Long, Long> vmClusterAndHost = findClusterAndHostIdForVm(vm);
Expand Down Expand Up @@ -5913,4 +5876,53 @@ private Pair<JobInfo.Status, String> orchestrateUpdateDefaultNic(final VmWorkUpd
_jobMgr.marshallResultObject(result));
}

private Pair<Long, Long> findClusterAndHostIdForVmFromVolumes(long vmId) {
Long clusterId = null;
Long hostId = null;
List<VolumeVO> volumes = _volsDao.findByInstance(vmId);
for (VolumeVO volume : volumes) {
if (Volume.State.Ready.equals(volume.getState()) &&
volume.getPoolId() != null) {
StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId());
if (pool != null && pool.getClusterId() != null) {
clusterId = pool.getClusterId();
// hostId to be used only for sending commands, capacity check skipped
List<HostVO> hosts = _hostDao.findHypervisorHostInCluster(pool.getClusterId());
if (CollectionUtils.isNotEmpty(hosts)) {
hostId = hosts.get(0).getId();
break;
}
}
}
}
return new Pair<>(clusterId, hostId);
}

private Pair<Long, Long> findClusterAndHostIdForVm(VirtualMachine vm) {
Long hostId = vm.getHostId();
Long clusterId = null;
if(hostId == null) {
hostId = vm.getLastHostId();
if (s_logger.isDebugEnabled()) {
s_logger.debug(String.format("host id is null, using last host id %d", hostId) );
}
}
if (hostId == null) {
return findClusterAndHostIdForVmFromVolumes(vm.getId());
}
HostVO host = _hostDao.findById(hostId);
if (host != null) {
clusterId = host.getClusterId();
}
return new Pair<>(clusterId, hostId);
}

@Override
public Pair<Long, Long> findClusterAndHostIdForVm(long vmId) {
VMInstanceVO vm = _vmDao.findById(vmId);
if (vm == null) {
return new Pair<>(null, null);
}
return findClusterAndHostIdForVm(vm);
}
}
47 changes: 3 additions & 44 deletions plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,48 +206,7 @@ protected VMwareGuru() {
@Override public VirtualMachineTO implement(VirtualMachineProfile vm) {
vmwareVmImplementer.setGlobalNestedVirtualisationEnabled(VmwareEnableNestedVirtualization.value());
vmwareVmImplementer.setGlobalNestedVPerVMEnabled(VmwareEnableNestedVirtualizationPerVM.value());
return vmwareVmImplementer.implement(vm, toVirtualMachineTO(vm), getClusterId(vm.getId()));
}

private Long getClusterIdFromVmVolume(long vmId) {
Long clusterId = null;
List<VolumeVO> volumes = _volumeDao.findByInstanceAndType(vmId, Volume.Type.ROOT);
if (CollectionUtils.isNotEmpty(volumes)) {
for (VolumeVO rootVolume : volumes) {
if (rootVolume.getPoolId() != null) {
StoragePoolVO pool = _storagePoolDao.findById(rootVolume.getPoolId());
if (pool != null && pool.getClusterId() != null) {
clusterId = pool.getClusterId();
break;
}
}
}
}
return clusterId;
}

private Long getClusterId(long vmId) {
Long clusterId = null;
Long hostId = null;
VMInstanceVO vm = _vmDao.findById(vmId);
if (vm != null) {
hostId = _vmDao.findById(vmId).getHostId();
}
if (vm != null && hostId == null) {
// If VM is in stopped state then hostId would be undefined. Hence read last host's Id instead.
hostId = _vmDao.findById(vmId).getLastHostId();
}
HostVO host = null;
if (hostId != null) {
host = _hostDao.findById(hostId);
}
if (host != null) {
clusterId = host.getClusterId();
} else {
clusterId = getClusterIdFromVmVolume(vmId);
}

return clusterId;
return vmwareVmImplementer.implement(vm, toVirtualMachineTO(vm), vmManager.findClusterAndHostIdForVm(vm.getId()).first());
}

@Override @DB public Pair<Boolean, Long> getCommandHostDelegation(long hostId, Command cmd) {
Expand Down Expand Up @@ -445,7 +404,7 @@ private static String resolveNameInGuid(String guid) {

@Override public Map<String, String> getClusterSettings(long vmId) {
Map<String, String> details = new HashMap<String, String>();
Long clusterId = getClusterId(vmId);
Long clusterId = vmManager.findClusterAndHostIdForVm(vmId).first();
if (clusterId != null) {
details.put(VmwareReserveCpu.key(), VmwareReserveCpu.valueIn(clusterId).toString());
details.put(VmwareReserveMemory.key(), VmwareReserveMemory.valueIn(clusterId).toString());
Expand Down Expand Up @@ -1120,7 +1079,7 @@ private String getHostGuidInTargetCluster(boolean isInterClusterMigration, Long
}

final Long destClusterId = destination.getClusterId();
final Long srcClusterId = getClusterId(vm.getId());
final Long srcClusterId = vmManager.findClusterAndHostIdForVm(vm.getId()).first();
final boolean isInterClusterMigration = isInterClusterMigration(destClusterId, srcClusterId);
MigrateVmToPoolCommand migrateVmToPoolCommand = new MigrateVmToPoolCommand(vm.getInstanceName(),
vols, destination.getUuid(), getHostGuidInTargetCluster(isInterClusterMigration, destClusterId), true);
Expand Down
19 changes: 9 additions & 10 deletions ...s/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;

import com.cloud.hypervisor.vmware.mo.VirtualStorageObjectManagerMO;
import com.cloud.hypervisor.vmware.mo.VirtualMachineDiskInfoBuilder;
import com.vmware.vim25.BaseConfigInfoDiskFileBackingInfo;
import com.vmware.vim25.VStorageObject;
import com.vmware.vim25.VirtualDiskType;
import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand;
import org.apache.cloudstack.storage.command.AttachAnswer;
import org.apache.cloudstack.storage.command.AttachCommand;
Expand Down Expand Up @@ -84,7 +79,9 @@
import com.cloud.hypervisor.vmware.mo.HostStorageSystemMO;
import com.cloud.hypervisor.vmware.mo.HypervisorHostHelper;
import com.cloud.hypervisor.vmware.mo.NetworkDetails;
import com.cloud.hypervisor.vmware.mo.VirtualMachineDiskInfoBuilder;
import com.cloud.hypervisor.vmware.mo.VirtualMachineMO;
import com.cloud.hypervisor.vmware.mo.VirtualStorageObjectManagerMO;
import com.cloud.hypervisor.vmware.mo.VmwareHypervisorHost;
import com.cloud.hypervisor.vmware.resource.VmwareResource;
import com.cloud.hypervisor.vmware.util.VmwareContext;
Expand All @@ -104,6 +101,7 @@
import com.cloud.vm.VmDetailConstants;
import com.google.common.base.Strings;
import com.google.gson.Gson;
import com.vmware.vim25.BaseConfigInfoDiskFileBackingInfo;
import com.vmware.vim25.DatastoreHostMount;
import com.vmware.vim25.HostHostBusAdapter;
import com.vmware.vim25.HostInternetScsiHba;
Expand All @@ -122,11 +120,13 @@
import com.vmware.vim25.HostUnresolvedVmfsVolume;
import com.vmware.vim25.InvalidStateFaultMsg;
import com.vmware.vim25.ManagedObjectReference;
import com.vmware.vim25.VStorageObject;
import com.vmware.vim25.VirtualDeviceBackingInfo;
import com.vmware.vim25.VirtualDeviceConfigSpec;
import com.vmware.vim25.VirtualDeviceConfigSpecOperation;
import com.vmware.vim25.VirtualDisk;
import com.vmware.vim25.VirtualDiskFlatVer2BackingInfo;
import com.vmware.vim25.VirtualDiskType;
import com.vmware.vim25.VirtualMachineConfigSpec;
import com.vmware.vim25.VmConfigInfo;
import com.vmware.vim25.VmfsDatastoreExpandSpec;
Expand Down Expand Up @@ -2683,15 +2683,14 @@ public Answer deleteVolume(DeleteCommand cmd) {
List<VirtualDisk> virtualDisks = vmMo.getVirtualDisks();
List<String> managedDatastoreNames = getManagedDatastoreNamesFromVirtualDisks(virtualDisks);

// Preserve other disks of the VM
Copy link
Contributor

@harikrishna-patnala harikrishna-patnala Mar 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these changes @shwstppr.
Can you please test destroy VM and expunge VM cases with VM having disks. Because this particular block is removed to address disk detachments in case of destroy VM. If those operations are not effected, this looks good to me.

Copy link
Contributor Author

@shwstppr shwstppr Mar 9, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@harikrishna-patnala I've verified the destroy case and have seen any issue.
Will try to share API responses if needed

List<String> detachedDisks = vmMo.detachAllDisksExcept(vol.getPath(), diskInfo != null ? diskInfo.getDiskDeviceBusName() : null);
VmwareStorageLayoutHelper.moveVolumeToRootFolder(new DatacenterMO(context, morDc), detachedDisks);
// let vmMo.destroy to delete volume for us
// vmMo.tearDownDevices(new Class<?>[] { VirtualDisk.class, VirtualEthernetCard.class });

if (isManaged) {
List<String> detachedDisks = vmMo.detachAllDisksExcept(vol.getPath(), diskInfo != null ? diskInfo.getDiskDeviceBusName() : null);
VmwareStorageLayoutHelper.moveVolumeToRootFolder(new DatacenterMO(context, morDc), detachedDisks);
vmMo.unregisterVm();
}
else {
} else {
vmMo.destroy();
}

Expand Down
Loading