Skip to content
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.

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

kvm-storage: provide isVMMigrate information to storage plugins #10093

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 @@
}

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

Check warning on line 82 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStorageAdaptor.java#L82

Added line #L82 was not covered by tests
// 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 @@

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

Check warning on line 109 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStoragePool.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/IscsiAdmStoragePool.java#L109

Added line #L109 was not covered by tests
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,10 @@
StorageAdaptor adaptor = getStorageAdaptor(type);
KVMStoragePool pool = adaptor.getStoragePool(poolUuid);

return adaptor.connectPhysicalDisk(volPath, pool, details);
return adaptor.connectPhysicalDisk(volPath, pool, details, false);

Check warning on line 136 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java#L136

Added line #L136 was not covered by tests
}

public boolean connectPhysicalDisksViaVmSpec(VirtualMachineTO vmSpec) {
public boolean connectPhysicalDisksViaVmSpec(VirtualMachineTO vmSpec, boolean isVMMigrate) {

Check warning on line 139 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java#L139

Added line #L139 was not covered by tests
boolean result = false;

final String vmName = vmSpec.getName();
Expand All @@ -159,7 +159,7 @@
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);

Check warning on line 162 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java#L162

Added line #L162 was not covered by tests

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

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

Check warning on line 1014 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java#L1014

Added line #L1014 was not covered by tests
// 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 @@
* 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) {

Check warning on line 96 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ManagedNfsStorageAdaptor.java#L96

Added line #L96 was not covered by tests

StoragePool sp = null;
Connect conn = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@
}

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

Check warning on line 184 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java#L184

Added line #L184 was not covered by tests
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 @@

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

Check warning on line 81 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIPool.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIPool.java#L81

Added line #L81 was not covered by tests
}

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

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

Check warning on line 92 in plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStoragePool.java

View check run for this annotation

Codecov / codecov/patch

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStoragePool.java#L92

Added line #L92 was not covered by tests
}

@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);
}
}
6 changes: 6 additions & 0 deletions plugins/storage/volume/linstor/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@
* @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");

Check warning on line 277 in plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java#L277

Added line #L277 was not covered by tests
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 @@ -289,7 +290,8 @@
}

@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 @@ -312,12 +314,13 @@
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);

Check warning on line 321 in plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java#L319-L321

Added lines #L319 - L321 were not covered by tests
// do not fail here as adding allow-two-primaries property is only a problem while live migrating
}

Check warning on line 323 in plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java#L323

Added line #L323 was not covered by tests
JoaoJandre marked this conversation as resolved.
Show resolved Hide resolved
}
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
@Override
public boolean connectPhysicalDisk(String volumeUuid, Map<String, String> details)
{
return _storageAdaptor.connectPhysicalDisk(volumeUuid, this, details);
return _storageAdaptor.connectPhysicalDisk(volumeUuid, this, details, false);

Check warning on line 77 in plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStoragePool.java

View check run for this annotation

Codecov / codecov/patch

plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStoragePool.java#L77

Added line #L77 was not covered by tests
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@
}

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

Check warning on line 247 in plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStorageAdaptor.java

View check run for this annotation

Codecov / codecov/patch

plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStorageAdaptor.java#L247

Added line #L247 was not covered by tests
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 @@

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

Check warning on line 134 in plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStoragePool.java

View check run for this annotation

Codecov / codecov/patch

plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/storage/StorPoolStoragePool.java#L134

Added line #L134 was not covered by tests
}

@Override
Expand Down
Loading