Skip to content

Commit 8fb082c

Browse files
Pearl1594Pearl1594
committed
made necessary changes to allow migration of lock and deletion of detached volumes
1 parent 190b201 commit 8fb082c

File tree

10 files changed

+313
-19
lines changed

10 files changed

+313
-19
lines changed

‎core/src/main/java/com/cloud/agent/api/PreMigrationCommand.java‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
//
2+
// Licensed to the Apache Software Foundation (ASF) under one
3+
// or more contributor license agreements. See the NOTICE file
4+
// distributed with this work for additional information
5+
// regarding copyright ownership. The ASF licenses this file
6+
// to you under the Apache License, Version 2.0 (the
7+
// "License"); you may not use this file except in compliance
8+
// with the License. You may obtain a copy of the License at
9+
//
10+
// http://www.apache.org/licenses/LICENSE-2.0
11+
//
12+
// Unless required by applicable law or agreed to in writing,
13+
// software distributed under the License is distributed on an
14+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
// KIND, either express or implied. See the License for the
16+
// specific language governing permissions and limitations
17+
// under the License.
18+
//
19+
20+
package com.cloud.agent.api;
21+
22+
import com.cloud.agent.api.to.VirtualMachineTO;
23+
24+
/**
25+
* PreMigrationCommand is sent to the source host before VM migration starts.
26+
* It performs pre-migration tasks such as:
27+
* - Converting CLVM volume exclusive locks to shared mode so destination host can access them
28+
* - Other pre-migration preparation operations on the source host
29+
*
30+
* This command runs on the SOURCE host before PrepareForMigrationCommand runs on the DESTINATION host.
31+
*/
32+
public class PreMigrationCommand extends Command {
33+
private VirtualMachineTO vm;
34+
private String vmName;
35+
36+
protected PreMigrationCommand() {
37+
}
38+
39+
public PreMigrationCommand(VirtualMachineTO vm, String vmName) {
40+
this.vm = vm;
41+
this.vmName = vmName;
42+
}
43+
44+
public VirtualMachineTO getVirtualMachine() {
45+
return vm;
46+
}
47+
48+
public String getVmName() {
49+
return vmName;
50+
}
51+
52+
@Override
53+
public boolean executeInSequence() {
54+
return true;
55+
}
56+
}

‎engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeInfo.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,6 @@
3131

3232
public interface VolumeInfo extends DownloadableDataInfo, Volume {
3333

34-
/**
35-
* Constant for the volume detail key that stores the destination host ID for CLVM volume creation routing.
36-
* This helps ensure volumes are created on the correct host with exclusive locks.
37-
*/
38-
String DESTINATION_HOST_ID = "destinationHostId";
39-
4034
/**
4135
* Constant for the volume detail key that stores the host ID currently holding the CLVM exclusive lock.
4236
* This is used during lightweight lock migration to determine the source host for lock transfer.

‎engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@
136136
import com.cloud.agent.api.PrepareExternalProvisioningCommand;
137137
import com.cloud.agent.api.PrepareForMigrationAnswer;
138138
import com.cloud.agent.api.PrepareForMigrationCommand;
139+
import com.cloud.agent.api.PreMigrationCommand;
139140
import com.cloud.agent.api.RebootAnswer;
140141
import com.cloud.agent.api.RebootCommand;
141142
import com.cloud.agent.api.RecreateCheckpointsCommand;
@@ -3108,6 +3109,24 @@ protected void migrate(final VMInstanceVO vm, final long srcHostId, final Deploy
31083109
updateOverCommitRatioForVmProfile(profile, dest.getHost().getClusterId());
31093110

31103111
final VirtualMachineTO to = toVmTO(profile);
3112+
3113+
logger.info("Sending PreMigrationCommand to source host {} for VM {}", srcHostId, vm.getInstanceName());
3114+
final PreMigrationCommand preMigCmd = new PreMigrationCommand(to, vm.getInstanceName());
3115+
Answer preMigAnswer = null;
3116+
try {
3117+
preMigAnswer = _agentMgr.send(srcHostId, preMigCmd);
3118+
if (preMigAnswer == null || !preMigAnswer.getResult()) {
3119+
final String details = preMigAnswer != null ? preMigAnswer.getDetails() : "null answer returned";
3120+
final String msg = "Failed to prepare source host for migration: " + details;
3121+
logger.error("Failed to prepare source host {} for migration of VM {}: {}", srcHostId, vm.getInstanceName(), details);
3122+
throw new CloudRuntimeException(msg);
3123+
}
3124+
logger.info("Successfully prepared source host {} for migration of VM {}", srcHostId, vm.getInstanceName());
3125+
} catch (final AgentUnavailableException | OperationTimedoutException e) {
3126+
logger.error("Failed to send PreMigrationCommand to source host {}: {}", srcHostId, e.getMessage(), e);
3127+
throw new CloudRuntimeException("Failed to prepare source host for migration: " + e.getMessage(), e);
3128+
}
3129+
31113130
final PrepareForMigrationCommand pfmc = new PrepareForMigrationCommand(to);
31123131
setVmNetworkDetails(vm, to);
31133132

@@ -4914,6 +4933,27 @@ private void orchestrateMigrateForScale(final String vmUuid, final long srcHostI
49144933
volumeMgr.prepareForMigration(profile, dest);
49154934

49164935
final VirtualMachineTO to = toVmTO(profile);
4936+
4937+
// Step 1: Send PreMigrationCommand to source host to convert CLVM volumes to shared mode
4938+
// This must happen BEFORE PrepareForMigrationCommand on destination to avoid lock conflicts
4939+
logger.info("Sending PreMigrationCommand to source host {} for VM {}", srcHostId, vm.getInstanceName());
4940+
final PreMigrationCommand preMigCmd = new PreMigrationCommand(to, vm.getInstanceName());
4941+
Answer preMigAnswer = null;
4942+
try {
4943+
preMigAnswer = _agentMgr.send(srcHostId, preMigCmd);
4944+
if (preMigAnswer == null || !preMigAnswer.getResult()) {
4945+
final String details = preMigAnswer != null ? preMigAnswer.getDetails() : "null answer returned";
4946+
final String msg = "Failed to prepare source host for migration: " + details;
4947+
logger.error("Failed to prepare source host {} for migration of VM {}: {}", srcHostId, vm.getInstanceName(), details);
4948+
throw new CloudRuntimeException(msg);
4949+
}
4950+
logger.info("Successfully prepared source host {} for migration of VM {}", srcHostId, vm.getInstanceName());
4951+
} catch (final AgentUnavailableException | OperationTimedoutException e) {
4952+
logger.error("Failed to send PreMigrationCommand to source host {}: {}", srcHostId, e.getMessage(), e);
4953+
throw new CloudRuntimeException("Failed to prepare source host for migration: " + e.getMessage(), e);
4954+
}
4955+
4956+
// Step 2: Send PrepareForMigrationCommand to destination host
49174957
final PrepareForMigrationCommand pfmc = new PrepareForMigrationCommand(to);
49184958

49194959
ItWorkVO work = new ItWorkVO(UUID.randomUUID().toString(), _nodeId, State.Migrating, vm.getType(), vm.getId());

‎engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java‎

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -746,12 +746,16 @@ public VolumeInfo createVolume(VolumeInfo volumeInfo, VirtualMachine vm, Virtual
746746
volumeToString, poolToString);
747747
DataStore store = dataStoreMgr.getDataStore(pool.getId(), DataStoreRole.Primary);
748748

749-
// For CLVM pools, set the destination host hint so volume is created on the correct host
749+
// For CLVM pools, set the lock host hint so volume is created on the correct host
750750
// This avoids the need for shared mode activation and improves performance
751751
if (pool.getPoolType() == Storage.StoragePoolType.CLVM && hostId != null) {
752-
logger.info("CLVM pool detected. Setting destination host {} for volume {} to route creation to correct host",
752+
logger.info("CLVM pool detected. Setting lock host {} for volume {} to route creation to correct host",
753753
hostId, volumeInfo.getUuid());
754754
volumeInfo.setDestinationHostId(hostId);
755+
756+
// Persist CLVM lock host to database immediately so it survives across VolumeInfo object recreations
757+
// and serves as both the creation routing hint and the lock host tracker
758+
setClvmLockHostId(volumeInfo.getId(), hostId);
755759
}
756760

757761
for (int i = 0; i < 2; i++) {
@@ -795,6 +799,26 @@ private String getVolumeIdentificationInfos(Volume volume) {
795799
return String.format("uuid: %s, name: %s", volume.getUuid(), volume.getName());
796800
}
797801

802+
/**
803+
* Sets or updates the CLVM_LOCK_HOST_ID detail for a volume.
804+
* If the detail already exists, it will be updated. Otherwise, it will be created.
805+
*
806+
* @param volumeId The ID of the volume
807+
* @param hostId The host ID that holds/should hold the CLVM exclusive lock
808+
*/
809+
private void setClvmLockHostId(long volumeId, long hostId) {
810+
VolumeDetailVO existingDetail = _volDetailDao.findDetail(volumeId, VolumeInfo.CLVM_LOCK_HOST_ID);
811+
812+
if (existingDetail != null) {
813+
existingDetail.setValue(String.valueOf(hostId));
814+
_volDetailDao.update(existingDetail.getId(), existingDetail);
815+
logger.debug("Updated CLVM_LOCK_HOST_ID for volume {} to host {}", volumeId, hostId);
816+
} else {
817+
_volDetailDao.addDetail(volumeId, VolumeInfo.CLVM_LOCK_HOST_ID, String.valueOf(hostId), false);
818+
logger.debug("Created CLVM_LOCK_HOST_ID for volume {} with host {}", volumeId, hostId);
819+
}
820+
}
821+
798822
public String getRandomVolumeName() {
799823
return UUID.randomUUID().toString();
800824
}
@@ -1866,10 +1890,10 @@ private Pair<VolumeVO, DataStore> recreateVolume(VolumeVO vol, VirtualMachinePro
18661890
if (poolVO != null && poolVO.getPoolType() == Storage.StoragePoolType.CLVM) {
18671891
Long hostId = vm.getVirtualMachine().getHostId();
18681892
if (hostId != null) {
1869-
// Store in both memory and database so it persists across VolumeInfo recreations
1893+
// Using CLVM_LOCK_HOST_ID which serves dual purpose: creation routing and lock tracking
18701894
volume.setDestinationHostId(hostId);
1871-
_volDetailDao.addDetail(volume.getId(), VolumeInfo.DESTINATION_HOST_ID, String.valueOf(hostId), false);
1872-
logger.info("CLVM pool detected during volume creation from template. Setting destination host {} for volume {} (persisted to DB) to route creation to correct host",
1895+
setClvmLockHostId(volume.getId(), hostId);
1896+
logger.info("CLVM pool detected during volume creation from template. Setting lock host {} for volume {} (persisted to DB) to route creation to correct host",
18731897
hostId, volume.getUuid());
18741898
}
18751899
}

‎engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
import com.cloud.dc.DedicatedResourceVO;
3434
import com.cloud.dc.dao.DedicatedResourceDao;
3535
import com.cloud.storage.Storage;
36+
import com.cloud.storage.VolumeDetailVO;
37+
import com.cloud.storage.dao.VolumeDetailsDao;
3638
import com.cloud.user.Account;
3739
import com.cloud.utils.Pair;
3840
import com.cloud.utils.db.QueryBuilder;
@@ -41,6 +43,7 @@
4143
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
4244
import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
4345
import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector;
46+
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore;
4447
import org.apache.cloudstack.engine.subsystem.api.storage.Scope;
4548
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
4649
import org.apache.cloudstack.engine.subsystem.api.storage.StorageAction;
@@ -80,6 +83,8 @@ public class DefaultEndPointSelector implements EndPointSelector {
8083
private DedicatedResourceDao dedicatedResourceDao;
8184
@Inject
8285
private PrimaryDataStoreDao _storagePoolDao;
86+
@Inject
87+
private VolumeDetailsDao _volDetailsDao;
8388

8489
private static final String VOL_ENCRYPT_COLUMN_NAME = "volume_encryption_support";
8590
private final String findOneHostOnPrimaryStorage = "select t.id from "
@@ -449,7 +454,8 @@ public EndPoint select(DataObject object, boolean encryptionSupportRequired) {
449454
// For CLVM volumes with destination host hint, route to that specific host
450455
// This ensures volumes are created on the correct host with exclusive locks
451456
if (object instanceof VolumeInfo && store.getRole() == DataStoreRole.Primary) {
452-
EndPoint clvmEndpoint = selectClvmEndpointIfApplicable((VolumeInfo) object, "volume creation");
457+
VolumeInfo volInfo = (VolumeInfo) object;
458+
EndPoint clvmEndpoint = selectClvmEndpointIfApplicable(volInfo, "volume creation");
453459
if (clvmEndpoint != null) {
454460
return clvmEndpoint;
455461
}
@@ -558,6 +564,31 @@ public EndPoint select(DataObject object, StorageAction action, boolean encrypti
558564
}
559565
case DELETEVOLUME: {
560566
VolumeInfo volume = (VolumeInfo) object;
567+
568+
// For CLVM volumes, route to the host holding the exclusive lock
569+
if (volume.getHypervisorType() == Hypervisor.HypervisorType.KVM) {
570+
DataStore store = volume.getDataStore();
571+
if (store.getRole() == DataStoreRole.Primary) {
572+
StoragePoolVO pool = _storagePoolDao.findById(store.getId());
573+
if (pool != null && pool.getPoolType() == Storage.StoragePoolType.CLVM) {
574+
Long lockHostId = getClvmLockHostId(volume);
575+
if (lockHostId != null) {
576+
logger.info("Routing CLVM volume {} deletion to lock holder host {}",
577+
volume.getUuid(), lockHostId);
578+
EndPoint ep = getEndPointFromHostId(lockHostId);
579+
if (ep != null) {
580+
return ep;
581+
}
582+
logger.warn("Could not get endpoint for CLVM lock host {}, falling back to default selection",
583+
lockHostId);
584+
} else {
585+
logger.debug("No CLVM lock host tracked for volume {}, using default endpoint selection",
586+
volume.getUuid());
587+
}
588+
}
589+
}
590+
}
591+
561592
if (volume.getHypervisorType() == Hypervisor.HypervisorType.VMware) {
562593
VirtualMachine vm = volume.getAttachedVM();
563594
if (vm != null) {
@@ -654,4 +685,24 @@ public List<EndPoint> selectAll(DataStore store) {
654685
}
655686
return endPoints;
656687
}
688+
689+
/**
690+
* Retrieves the host ID that currently holds the exclusive lock on a CLVM volume.
691+
* This is tracked in volume_details table for proper routing of delete operations.
692+
*
693+
* @param volume The CLVM volume
694+
* @return Host ID holding the lock, or null if not tracked
695+
*/
696+
private Long getClvmLockHostId(VolumeInfo volume) {
697+
VolumeDetailVO detail = _volDetailsDao.findDetail(volume.getId(), VolumeInfo.CLVM_LOCK_HOST_ID);
698+
if (detail != null && detail.getValue() != null && !detail.getValue().isEmpty()) {
699+
try {
700+
return Long.parseLong(detail.getValue());
701+
} catch (NumberFormatException e) {
702+
logger.warn("Invalid CLVM lock host ID in volume_details for volume {}: {}",
703+
volume.getUuid(), detail.getValue());
704+
}
705+
}
706+
return null;
707+
}
657708
}

‎engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeObject.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -365,13 +365,16 @@ public void setDirectDownload(boolean directDownload) {
365365
@Override
366366
public Long getDestinationHostId() {
367367
// If not in memory, try to load from database (volume_details table)
368+
// For CLVM volumes, this uses the CLVM_LOCK_HOST_ID which serves dual purpose:
369+
// 1. During creation: hints where to create the volume
370+
// 2. After creation: tracks which host holds the exclusive lock
368371
if (destinationHostId == null && volumeVO != null) {
369-
VolumeDetailVO detail = volumeDetailsDao.findDetail(volumeVO.getId(), DESTINATION_HOST_ID);
372+
VolumeDetailVO detail = volumeDetailsDao.findDetail(volumeVO.getId(), CLVM_LOCK_HOST_ID);
370373
if (detail != null && detail.getValue() != null && !detail.getValue().isEmpty()) {
371374
try {
372375
destinationHostId = Long.parseLong(detail.getValue());
373376
} catch (NumberFormatException e) {
374-
logger.warn("Invalid destinationHostId value in volume_details for volume {}: {}", volumeVO.getUuid(), detail.getValue());
377+
logger.warn("Invalid CLVM lock host ID value in volume_details for volume {}: {}", volumeVO.getUuid(), detail.getValue());
375378
}
376379
}
377380
}

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,9 +238,6 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0.
238238
libvirtComputingResource.detachAndAttachConfigDriveISO(conn, vmName);
239239
}
240240

241-
// Activate CLVM volumes in shared mode before migration starts
242-
LibvirtComputingResource.modifyClvmVolumesStateForMigration(disks, libvirtComputingResource, to, LibvirtComputingResource.ClvmVolumeState.SHARED);
243-
244241
//run migration in thread so we can monitor it
245242
logger.info("Starting live migration of instance {} to destination host {} having the final XML configuration: {}.", vmName, dconn.getURI(), maskSensitiveInfoInXML(xmlDesc));
246243
final ExecutorService executor = Executors.newFixedThreadPool(1);

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPostMigrationCommandWrapper.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public Answer execute(final PostMigrationCommand command, final LibvirtComputing
7373
return new PostMigrationAnswer(command);
7474

7575
} catch (final LibvirtException e) {
76-
logger.error("LibVirt error during post-migration for VM {}: {}", vmName, e.getMessage(), e);
76+
logger.error("Libvirt error during post-migration for VM {}: {}", vmName, e.getMessage(), e);
7777
return new PostMigrationAnswer(command, e);
7878
} catch (final Exception e) {
7979
logger.error("Error during post-migration for VM {}: {}", vmName, e.getMessage(), e);

0 commit comments

Comments
 (0)