From a9587bfd2e33c52151216c624aee8f09a4ca2f4c Mon Sep 17 00:00:00 2001 From: Rene Peinthor Date: Wed, 18 Dec 2024 09:13:41 +0100 Subject: [PATCH] kvm-storage: provide isVMMigrate information to storage plugins (#10093) Particular Linstor needs can use this information to only allow dual volume access for live migration and not enable it in general, which can and will lead to data corruption if for some reason 2 VMs get started on 2 different hosts. --- ...ibvirtPrepareForMigrationCommandWrapper.java | 2 +- .../wrapper/LibvirtStartCommandWrapper.java | 2 +- .../kvm/storage/IscsiAdmStorageAdaptor.java | 2 +- .../kvm/storage/IscsiAdmStoragePool.java | 2 +- .../kvm/storage/KVMStoragePoolManager.java | 6 +++--- .../kvm/storage/LibvirtStorageAdaptor.java | 2 +- .../kvm/storage/ManagedNfsStorageAdaptor.java | 2 +- .../kvm/storage/MultipathSCSIAdapterBase.java | 2 +- .../kvm/storage/MultipathSCSIPool.java | 2 +- .../kvm/storage/ScaleIOStorageAdaptor.java | 4 ++-- .../kvm/storage/ScaleIOStoragePool.java | 2 +- .../hypervisor/kvm/storage/StorageAdaptor.java | 11 +++++++++-- .../resource/LibvirtComputingResourceTest.java | 10 +++++----- .../kvm/storage/ScaleIOStoragePoolTest.java | 2 +- plugins/storage/volume/linstor/CHANGELOG.md | 6 ++++++ .../kvm/storage/LinstorStorageAdaptor.java | 17 ++++++++++------- .../kvm/storage/LinstorStoragePool.java | 2 +- .../kvm/storage/StorPoolStorageAdaptor.java | 2 +- .../kvm/storage/StorPoolStoragePool.java | 2 +- 19 files changed, 48 insertions(+), 32 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java index 77333f87140..ebb5a64263e 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java @@ -120,7 +120,7 @@ public final class LibvirtPrepareForMigrationCommandWrapper extends CommandWrapp skipDisconnect = true; - if (!storagePoolMgr.connectPhysicalDisksViaVmSpec(vm)) { + if (!storagePoolMgr.connectPhysicalDisksViaVmSpec(vm, true)) { return new PrepareForMigrationAnswer(command, "failed to connect physical disks to host"); } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStartCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStartCommandWrapper.java index 7b69993f2e5..92ebf39d396 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStartCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStartCommandWrapper.java @@ -77,7 +77,7 @@ public final class LibvirtStartCommandWrapper extends CommandWrapper details) { + public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map details, boolean isVMMigrate) { // ex. sudo iscsiadm -m node -T iqn.2012-03.com.test:volume1 -p 192.168.233.10:3260 -o new Script iScsiAdmCmd = new Script(true, "iscsiadm", 0, s_logger); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStoragePool.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStoragePool.java index a89650f6eb6..f5bfd898a4f 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStoragePool.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStoragePool.java @@ -106,7 +106,7 @@ public class IscsiAdmStoragePool implements KVMStoragePool { @Override public boolean connectPhysicalDisk(String name, Map details) { - return this._storageAdaptor.connectPhysicalDisk(name, this, details); + return this._storageAdaptor.connectPhysicalDisk(name, this, details, false); } @Override diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java index da6a192f129..2bd068b6c8f 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java @@ -133,10 +133,10 @@ public class KVMStoragePoolManager { StorageAdaptor adaptor = getStorageAdaptor(type); KVMStoragePool pool = adaptor.getStoragePool(poolUuid); - return adaptor.connectPhysicalDisk(volPath, pool, details); + return adaptor.connectPhysicalDisk(volPath, pool, details, false); } - public boolean connectPhysicalDisksViaVmSpec(VirtualMachineTO vmSpec) { + public boolean connectPhysicalDisksViaVmSpec(VirtualMachineTO vmSpec, boolean isVMMigrate) { boolean result = false; final String vmName = vmSpec.getName(); @@ -159,7 +159,7 @@ public class KVMStoragePoolManager { KVMStoragePool pool = getStoragePool(store.getPoolType(), store.getUuid()); StorageAdaptor adaptor = getStorageAdaptor(pool.getType()); - result = adaptor.connectPhysicalDisk(vol.getPath(), pool, disk.getDetails()); + result = adaptor.connectPhysicalDisk(vol.getPath(), pool, disk.getDetails(), isVMMigrate); if (!result) { s_logger.error("Failed to connect disks via vm spec for vm: " + vmName + " volume:" + vol.toString()); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index 11375969b6e..80a810c6313 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -1011,7 +1011,7 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { } @Override - public boolean connectPhysicalDisk(String name, KVMStoragePool pool, Map details) { + public boolean connectPhysicalDisk(String name, KVMStoragePool pool, Map details, boolean isVMMigrate) { // this is for managed storage that needs to prep disks prior to use return true; } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java index cb285383b9b..48e556fd08c 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java @@ -93,7 +93,7 @@ public class ManagedNfsStorageAdaptor implements StorageAdaptor { * creates a nfs storage pool using libvirt */ @Override - public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map details) { + public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map details, boolean isVMMigrate) { StoragePool sp = null; Connect conn = null; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java index 0d58bdaa177..558a5269ef4 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java @@ -181,7 +181,7 @@ public abstract class MultipathSCSIAdapterBase implements StorageAdaptor { } @Override - public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool, Map details) { + public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool, Map details, boolean isVMMigrate) { LOGGER.info("connectPhysicalDisk called for [" + volumePath + "]"); if (StringUtils.isEmpty(volumePath)) { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIPool.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIPool.java index bc2f072f719..229481b1f79 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIPool.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIPool.java @@ -78,7 +78,7 @@ public class MultipathSCSIPool implements KVMStoragePool { @Override public boolean connectPhysicalDisk(String volumeUuid, Map details) { - return storageAdaptor.connectPhysicalDisk(volumeUuid, this, details); + return storageAdaptor.connectPhysicalDisk(volumeUuid, this, details, false); } @Override diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java index bb5ba08a421..ab9533c075e 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java @@ -178,7 +178,7 @@ public class ScaleIOStorageAdaptor implements StorageAdaptor { return null; } - if(!connectPhysicalDisk(name, pool, null)) { + if(!connectPhysicalDisk(name, pool, null, false)) { throw new CloudRuntimeException(String.format("Failed to ensure disk %s was present", name)); } @@ -221,7 +221,7 @@ public class ScaleIOStorageAdaptor implements StorageAdaptor { } @Override - public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool, Map details) { + public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool, Map details, boolean isMigration) { if (StringUtils.isEmpty(volumePath) || pool == null) { LOGGER.error("Unable to connect physical disk due to insufficient data"); throw new CloudRuntimeException("Unable to connect physical disk due to insufficient data"); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStoragePool.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStoragePool.java index 77f21910da6..e8243c3f7cf 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStoragePool.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStoragePool.java @@ -89,7 +89,7 @@ public class ScaleIOStoragePool implements KVMStoragePool { @Override public boolean connectPhysicalDisk(String volumeUuid, Map details) { - return storageAdaptor.connectPhysicalDisk(volumeUuid, this, details); + return storageAdaptor.connectPhysicalDisk(volumeUuid, this, details, false); } @Override diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java index d1a1ed9a2ec..12c20c735c1 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java @@ -50,8 +50,15 @@ public interface StorageAdaptor { public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, PhysicalDiskFormat format, Storage.ProvisioningType provisioningType, long size, byte[] passphrase); - // given disk path (per database) and pool, prepare disk on host - public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool, Map details); + /** + * given disk path (per database) and pool, prepare disk on host + * @param volumePath volume path + * @param pool storage pool the disk is part of + * @param details disk details map + * @param isVMMigrate Indicates if request is while VM is migration + * @return true if connect was a success + */ + public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool, Map details, boolean isVMMigrate); // given disk path (per database) and pool, clean up disk on host public boolean disconnectPhysicalDisk(String volumePath, KVMStoragePool pool); diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java index 17bff8325ce..bbd1f8a73f2 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java @@ -1549,7 +1549,7 @@ public class LibvirtComputingResourceTest { when(libvirtComputingResourceMock.getVifDriver(nicTO.getType(), nicTO.getName())).thenReturn(vifDriver); when(libvirtComputingResourceMock.getStoragePoolMgr()).thenReturn(storagePoolManager); - when(storagePoolManager.connectPhysicalDisksViaVmSpec(vm)).thenReturn(true); + when(storagePoolManager.connectPhysicalDisksViaVmSpec(vm, true)).thenReturn(true); final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance(); assertNotNull(wrapper); @@ -5095,7 +5095,7 @@ public class LibvirtComputingResourceTest { fail(e.getMessage()); } - when(storagePoolMgr.connectPhysicalDisksViaVmSpec(vmSpec)).thenReturn(false); + when(storagePoolMgr.connectPhysicalDisksViaVmSpec(vmSpec, false)).thenReturn(false); final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance(); assertNotNull(wrapper); @@ -5296,7 +5296,7 @@ public class LibvirtComputingResourceTest { fail(e.getMessage()); } - when(storagePoolMgr.connectPhysicalDisksViaVmSpec(vmSpec)).thenReturn(true); + when(storagePoolMgr.connectPhysicalDisksViaVmSpec(vmSpec, false)).thenReturn(true); try { doNothing().when(libvirtComputingResourceMock).createVifs(vmSpec, vmDef); @@ -5375,7 +5375,7 @@ public class LibvirtComputingResourceTest { fail(e.getMessage()); } - when(storagePoolMgr.connectPhysicalDisksViaVmSpec(vmSpec)).thenReturn(true); + when(storagePoolMgr.connectPhysicalDisksViaVmSpec(vmSpec, false)).thenReturn(true); try { doNothing().when(libvirtComputingResourceMock).createVifs(vmSpec, vmDef); @@ -5452,7 +5452,7 @@ public class LibvirtComputingResourceTest { fail(e.getMessage()); } - when(storagePoolMgr.connectPhysicalDisksViaVmSpec(vmSpec)).thenReturn(true); + when(storagePoolMgr.connectPhysicalDisksViaVmSpec(vmSpec, false)).thenReturn(true); final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance(); assertNotNull(wrapper); diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/ScaleIOStoragePoolTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/ScaleIOStoragePoolTest.java index 492bc275a3f..bae8f3ac0ed 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/ScaleIOStoragePoolTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/ScaleIOStoragePoolTest.java @@ -195,7 +195,7 @@ public class ScaleIOStoragePoolTest { when(adapter.getPhysicalDisk(volumeId, pool)).thenReturn(disk); - final boolean result = adapter.connectPhysicalDisk(volumePath, pool, null); + final boolean result = adapter.connectPhysicalDisk(volumePath, pool, null, false); assertTrue(result); } } diff --git a/plugins/storage/volume/linstor/CHANGELOG.md b/plugins/storage/volume/linstor/CHANGELOG.md index e1ff9a5e269..957377e2978 100644 --- a/plugins/storage/volume/linstor/CHANGELOG.md +++ b/plugins/storage/volume/linstor/CHANGELOG.md @@ -11,6 +11,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Linstor heartbeat check now also ask linstor-controller if there is no connection between nodes +## [2024-12-11] + +### Fixed + +- Only set allow-two-primaries if a live migration is performed + ## [2024-10-28] ### Fixed diff --git a/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java b/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java index 90b1342c0a2..9b7a376e8f2 100644 --- a/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java +++ b/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java @@ -274,6 +274,7 @@ public class LinstorStorageAdaptor implements StorageAdaptor { * @throws ApiException if any problem connecting to the Linstor controller */ private void allow2PrimariesIfInUse(DevelopersApi api, String rscName) throws ApiException { + s_logger.debug("enabling allow-two-primaries"); String inUseNode = LinstorUtil.isResourceInUse(api, rscName); if (inUseNode != null && !inUseNode.equalsIgnoreCase(localNodeName)) { // allow 2 primaries for live migration, should be removed by disconnect on the other end @@ -289,7 +290,8 @@ public class LinstorStorageAdaptor implements StorageAdaptor { } @Override - public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool, Map details) + public boolean connectPhysicalDisk( + String volumePath, KVMStoragePool pool, Map details, boolean isVMMigration) { s_logger.debug(String.format("Linstor: connectPhysicalDisk %s:%s -> %s", pool.getUuid(), volumePath, details)); if (volumePath == null) { @@ -312,12 +314,13 @@ public class LinstorStorageAdaptor implements StorageAdaptor { throw new CloudRuntimeException(apiEx.getBestMessage(), apiEx); } - try - { - allow2PrimariesIfInUse(api, rscName); - } catch (ApiException apiEx) { - s_logger.error(apiEx); - // do not fail here as adding allow-two-primaries property is only a problem while live migrating + if (isVMMigration) { + try { + allow2PrimariesIfInUse(api, rscName); + } catch (ApiException apiEx) { + s_logger.error(apiEx); + // do not fail here as adding allow-two-primaries property is only a problem while live migrating + } } return true; } diff --git a/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStoragePool.java b/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStoragePool.java index 9a9c1a9717a..3907bae7f79 100644 --- a/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStoragePool.java +++ b/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStoragePool.java @@ -74,7 +74,7 @@ public class LinstorStoragePool implements KVMStoragePool { @Override public boolean connectPhysicalDisk(String volumeUuid, Map details) { - return _storageAdaptor.connectPhysicalDisk(volumeUuid, this, details); + return _storageAdaptor.connectPhysicalDisk(volumeUuid, this, details, false); } @Override diff --git a/plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStorageAdaptor.java b/plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStorageAdaptor.java index c7711c05ef5..4091be10470 100644 --- a/plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStorageAdaptor.java +++ b/plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStorageAdaptor.java @@ -244,7 +244,7 @@ public class StorPoolStorageAdaptor implements StorageAdaptor { } @Override - public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map details) { + public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map details, boolean isVMMigrate) { SP_LOG("StorPoolStorageAdaptor.connectPhysicalDisk: uuid=%s, pool=%s", volumeUuid, pool); log.debug(String.format("connectPhysicalDisk: uuid=%s, pool=%s", volumeUuid, pool)); diff --git a/plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStoragePool.java b/plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStoragePool.java index 02095503c3b..953b55b6e9a 100644 --- a/plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStoragePool.java +++ b/plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStoragePool.java @@ -131,7 +131,7 @@ public class StorPoolStoragePool implements KVMStoragePool { @Override public boolean connectPhysicalDisk(String name, Map details) { - return _storageAdaptor.connectPhysicalDisk(name, this, details); + return _storageAdaptor.connectPhysicalDisk(name, this, details, false); } @Override