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.
This commit is contained in:
Rene Peinthor 2024-12-18 09:13:41 +01:00 committed by GitHub
parent b4ad04badf
commit a9587bfd2e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
19 changed files with 48 additions and 32 deletions

View File

@ -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");
}

View File

@ -77,7 +77,7 @@ public final class LibvirtStartCommandWrapper extends CommandWrapper<StartComman
libvirtComputingResource.createVbd(conn, vmSpec, vmName, vm);
if (!storagePoolMgr.connectPhysicalDisksViaVmSpec(vmSpec)) {
if (!storagePoolMgr.connectPhysicalDisksViaVmSpec(vmSpec, false)) {
return new StartAnswer(command, "Failed to connect physical disks to host");
}

View File

@ -79,7 +79,7 @@ public class IscsiAdmStorageAdaptor implements StorageAdaptor {
}
@Override
public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map<String, String> details) {
public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map<String, String> 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);

View File

@ -106,7 +106,7 @@ public class IscsiAdmStoragePool implements KVMStoragePool {
@Override
public boolean connectPhysicalDisk(String name, Map<String, String> details) {
return this._storageAdaptor.connectPhysicalDisk(name, this, details);
return this._storageAdaptor.connectPhysicalDisk(name, this, details, false);
}
@Override

View File

@ -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());

View File

@ -1011,7 +1011,7 @@ public class LibvirtStorageAdaptor implements StorageAdaptor {
}
@Override
public boolean connectPhysicalDisk(String name, KVMStoragePool pool, Map<String, String> details) {
public boolean connectPhysicalDisk(String name, KVMStoragePool pool, Map<String, String> details, boolean isVMMigrate) {
// this is for managed storage that needs to prep disks prior to use
return true;
}

View File

@ -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<String, String> details) {
public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map<String, String> details, boolean isVMMigrate) {
StoragePool sp = null;
Connect conn = null;

View File

@ -181,7 +181,7 @@ public abstract class MultipathSCSIAdapterBase implements StorageAdaptor {
}
@Override
public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool, Map<String, String> details) {
public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool, Map<String, String> details, boolean isVMMigrate) {
LOGGER.info("connectPhysicalDisk called for [" + volumePath + "]");
if (StringUtils.isEmpty(volumePath)) {

View File

@ -78,7 +78,7 @@ public class MultipathSCSIPool implements KVMStoragePool {
@Override
public boolean connectPhysicalDisk(String volumeUuid, Map<String, String> details) {
return storageAdaptor.connectPhysicalDisk(volumeUuid, this, details);
return storageAdaptor.connectPhysicalDisk(volumeUuid, this, details, false);
}
@Override

View File

@ -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<String, String> details) {
public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool, Map<String, String> 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");

View File

@ -89,7 +89,7 @@ public class ScaleIOStoragePool implements KVMStoragePool {
@Override
public boolean connectPhysicalDisk(String volumeUuid, Map<String, String> details) {
return storageAdaptor.connectPhysicalDisk(volumeUuid, this, details);
return storageAdaptor.connectPhysicalDisk(volumeUuid, this, details, false);
}
@Override

View File

@ -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<String, String> 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<String, String> details, boolean isVMMigrate);
// given disk path (per database) and pool, clean up disk on host
public boolean disconnectPhysicalDisk(String volumePath, KVMStoragePool pool);

View File

@ -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);

View File

@ -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);
}
}

View File

@ -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

View File

@ -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<String, String> details)
public boolean connectPhysicalDisk(
String volumePath, KVMStoragePool pool, Map<String, String> details, boolean isVMMigration)
{
s_logger.debug(String.format("Linstor: connectPhysicalDisk %s:%s -> %s", pool.getUuid(), volumePath, details));
if (volumePath == null) {
@ -312,13 +314,14 @@ public class LinstorStorageAdaptor implements StorageAdaptor {
throw new CloudRuntimeException(apiEx.getBestMessage(), apiEx);
}
try
{
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;
}

View File

@ -74,7 +74,7 @@ public class LinstorStoragePool implements KVMStoragePool {
@Override
public boolean connectPhysicalDisk(String volumeUuid, Map<String, String> details)
{
return _storageAdaptor.connectPhysicalDisk(volumeUuid, this, details);
return _storageAdaptor.connectPhysicalDisk(volumeUuid, this, details, false);
}
@Override

View File

@ -244,7 +244,7 @@ public class StorPoolStorageAdaptor implements StorageAdaptor {
}
@Override
public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map<String, String> details) {
public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool pool, Map<String, String> details, boolean isVMMigrate) {
SP_LOG("StorPoolStorageAdaptor.connectPhysicalDisk: uuid=%s, pool=%s", volumeUuid, pool);
log.debug(String.format("connectPhysicalDisk: uuid=%s, pool=%s", volumeUuid, pool));

View File

@ -131,7 +131,7 @@ public class StorPoolStoragePool implements KVMStoragePool {
@Override
public boolean connectPhysicalDisk(String name, Map<String, String> details) {
return _storageAdaptor.connectPhysicalDisk(name, this, details);
return _storageAdaptor.connectPhysicalDisk(name, this, details, false);
}
@Override