Skip to content

Commit

Permalink
kvm-storage: provide isVMMigrate information to storage plugins
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rp- committed Dec 11, 2024
1 parent f9b1767 commit c7bd99e
Show file tree
Hide file tree
Showing 18 changed files with 42 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public Answer execute(final PrepareForMigrationCommand command, final LibvirtCom

skipDisconnect = true;

if (!storagePoolMgr.connectPhysicalDisksViaVmSpec(vm)) {
if (!storagePoolMgr.connectPhysicalDisksViaVmSpec(vm, true)) {
return new PrepareForMigrationAnswer(command, "failed to connect physical disks to host");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public Answer execute(final StartCommand command, final LibvirtComputingResource

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public KVMPhysicalDisk createPhysicalDisk(String volumeUuid, KVMStoragePool pool
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public KVMPhysicalDisk createPhysicalDisk(String name, Storage.ProvisioningType

@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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,10 @@ public boolean connectPhysicalDisk(StoragePoolType type, String poolUuid, String
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();
Expand All @@ -159,7 +159,7 @@ public boolean connectPhysicalDisksViaVmSpec(VirtualMachineTO vmSpec) {
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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1011,7 +1011,7 @@ private KVMPhysicalDisk createPhysicalDiskByQemuImg(String name, KVMStoragePool
}

@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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public KVMPhysicalDisk createPhysicalDisk(String volumeUuid, KVMStoragePool pool
* 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ public boolean deleteStoragePool(String uuid) {
}

@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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public KVMPhysicalDisk createPhysicalDisk(String arg0, PhysicalDiskFormat arg1,

@Override
public boolean connectPhysicalDisk(String volumeUuid, Map<String, String> details) {
return storageAdaptor.connectPhysicalDisk(volumeUuid, this, details);
return storageAdaptor.connectPhysicalDisk(volumeUuid, this, details, false);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, Qemu
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));
}

Expand Down Expand Up @@ -221,7 +221,7 @@ public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, Qemu
}

@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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public KVMPhysicalDisk createPhysicalDisk(String volumeUuid, Storage.Provisionin

@Override
public boolean connectPhysicalDisk(String volumeUuid, Map<String, String> details) {
return storageAdaptor.connectPhysicalDisk(volumeUuid, this, details);
return storageAdaptor.connectPhysicalDisk(volumeUuid, this, details, false);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,15 @@ public default KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool po
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1549,7 +1549,7 @@ public void testPrepareForMigrationCommandMigration() {

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);
Expand Down Expand Up @@ -5095,7 +5095,7 @@ public void testStartCommandFailedConnect() {
fail(e.getMessage());
}

when(storagePoolMgr.connectPhysicalDisksViaVmSpec(vmSpec)).thenReturn(false);
when(storagePoolMgr.connectPhysicalDisksViaVmSpec(vmSpec, false)).thenReturn(false);

final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance();
assertNotNull(wrapper);
Expand Down Expand Up @@ -5296,7 +5296,7 @@ public void testStartCommand() throws Exception {
fail(e.getMessage());
}

when(storagePoolMgr.connectPhysicalDisksViaVmSpec(vmSpec)).thenReturn(true);
when(storagePoolMgr.connectPhysicalDisksViaVmSpec(vmSpec, false)).thenReturn(true);
try {
doNothing().when(libvirtComputingResourceMock).createVifs(vmSpec, vmDef);

Expand Down Expand Up @@ -5375,7 +5375,7 @@ public void testStartCommandIsolationEc2() throws Exception {
fail(e.getMessage());
}

when(storagePoolMgr.connectPhysicalDisksViaVmSpec(vmSpec)).thenReturn(true);
when(storagePoolMgr.connectPhysicalDisksViaVmSpec(vmSpec, false)).thenReturn(true);
try {
doNothing().when(libvirtComputingResourceMock).createVifs(vmSpec, vmDef);

Expand Down Expand Up @@ -5452,7 +5452,7 @@ public void testStartCommandHostMemory() {
fail(e.getMessage());
}

when(storagePoolMgr.connectPhysicalDisksViaVmSpec(vmSpec)).thenReturn(true);
when(storagePoolMgr.connectPhysicalDisksViaVmSpec(vmSpec, false)).thenReturn(true);

final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance();
assertNotNull(wrapper);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ public void testConnectPhysicalDisk() {

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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ private void setAllowTwoPrimariesOnRc(DevelopersApi api, String rscName, String
* @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
Expand All @@ -288,7 +289,8 @@ private void allow2PrimariesIfInUse(DevelopersApi api, String rscName) throws Ap
}

@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) {
Expand All @@ -311,12 +313,13 @@ public boolean connectPhysicalDisk(String volumePath, KVMStoragePool pool, Map<S
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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public KVMPhysicalDisk createPhysicalDisk(String volumeUuid, Storage.Provisionin
@Override
public boolean connectPhysicalDisk(String volumeUuid, Map<String, String> details)
{
return _storageAdaptor.connectPhysicalDisk(volumeUuid, this, details);
return _storageAdaptor.connectPhysicalDisk(volumeUuid, this, details, false);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ public KVMPhysicalDisk getPhysicalDisk(String volumeUuid, KVMStoragePool pool) {
}

@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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public KVMPhysicalDisk createPhysicalDisk(String name, Storage.ProvisioningType

@Override
public boolean connectPhysicalDisk(String name, Map<String, String> details) {
return _storageAdaptor.connectPhysicalDisk(name, this, details);
return _storageAdaptor.connectPhysicalDisk(name, this, details, false);
}

@Override
Expand Down

0 comments on commit c7bd99e

Please sign in to comment.