From 7ebc392b0c11a13bd73bd6aed18742d0bd5e0df4 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Wed, 14 Feb 2024 14:44:44 +0530 Subject: [PATCH 1/2] Check if volume on datastore requires access for migration, and grant/revoke volume access if requires --- .../api/storage/PrimaryDataStoreDriver.java | 2 ++ .../engine/subsystem/api/storage/VolumeService.java | 2 ++ .../engine/orchestration/VolumeOrchestrator.java | 8 ++++---- .../storage/volume/VolumeServiceImpl.java | 13 +++++++++++++ .../driver/DateraPrimaryDataStoreDriver.java | 5 +++++ .../CloudStackPrimaryDataStoreDriverImpl.java | 5 +++++ .../driver/LinstorPrimaryDataStoreDriverImpl.java | 5 +++++ .../driver/NexentaPrimaryDataStoreDriver.java | 5 +++++ .../driver/SamplePrimaryDataStoreDriverImpl.java | 5 +++++ .../driver/ScaleIOPrimaryDataStoreDriver.java | 5 +++++ .../driver/SolidFirePrimaryDataStoreDriver.java | 5 +++++ .../driver/StorPoolPrimaryDataStoreDriver.java | 5 +++++ 12 files changed, 61 insertions(+), 4 deletions(-) diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java index 540d4f6673aa..b833ef6ed583 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java @@ -42,6 +42,8 @@ enum QualityOfServiceState { MIGRATION, NO_MIGRATION } void revokeAccess(DataObject dataObject, Host host, DataStore dataStore); + boolean requiresAccessForMigration(DataObject dataObject); + /** * intended for managed storage (cloud.storage_pool.managed = true) * if not managed, return volume.getSize() diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeService.java index 2c12b70e9eb7..a6ebf95a99ca 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/VolumeService.java @@ -54,6 +54,8 @@ public VolumeInfo getVolume() { void revokeAccess(DataObject dataObject, Host host, DataStore dataStore); + boolean requiresAccessForMigration(DataObject dataObject, DataStore dataStore); + /** * Creates the volume based on the given criteria * diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java index c3908795f7cb..ec10c344157e 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java @@ -1228,8 +1228,8 @@ public void release(long vmId, long hostId) { DataStore dataStore = dataStoreMgr.getDataStore(volumeForVm.getPoolId(), DataStoreRole.Primary); PrimaryDataStore primaryDataStore = (PrimaryDataStore)dataStore; - // This might impact other managed storages, grant access for PowerFlex storage pool only - if (primaryDataStore.isManaged() && primaryDataStore.getPoolType() == Storage.StoragePoolType.PowerFlex) { + // This might impact other managed storages, enable requires access for migration in relevant datastore driver (currently enabled for PowerFlex storage pool only) + if (primaryDataStore.isManaged() && volService.requiresAccessForMigration(volumeInfo, dataStore)) { volService.revokeAccess(volumeInfo, host, dataStore); } } @@ -1507,8 +1507,8 @@ public void prepareForMigration(VirtualMachineProfile vm, DeployDestination dest disk.setDetails(getDetails(volumeInfo, dataStore)); PrimaryDataStore primaryDataStore = (PrimaryDataStore)dataStore; - // This might impact other managed storages, grant access for PowerFlex storage pool only - if (primaryDataStore.isManaged() && primaryDataStore.getPoolType() == Storage.StoragePoolType.PowerFlex) { + // This might impact other managed storages, enable requires access for migration in relevant datastore driver (currently enabled for PowerFlex storage pool only) + if (primaryDataStore.isManaged() && volService.requiresAccessForMigration(volumeInfo, dataStore)) { volService.grantAccess(volFactory.getVolume(vol.getId()), dest.getHost(), dataStore); } diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java index 47577cc52b2c..03460d0d1e5e 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java @@ -266,6 +266,19 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) } } + @Override + public boolean requiresAccessForMigration(DataObject dataObject, DataStore dataStore) { + DataStoreDriver dataStoreDriver = dataStore != null ? dataStore.getDriver() : null; + if (dataStoreDriver == null) { + return false; + } + + if (dataStoreDriver instanceof PrimaryDataStoreDriver) { + return ((PrimaryDataStoreDriver)dataStoreDriver).requiresAccessForMigration(dataObject); + } + return false; + } + @Override public AsyncCallFuture createVolumeAsync(VolumeInfo volume, DataStore dataStore) { AsyncCallFuture future = new AsyncCallFuture(); diff --git a/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/driver/DateraPrimaryDataStoreDriver.java b/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/driver/DateraPrimaryDataStoreDriver.java index 79b621671175..456a43fb49a3 100644 --- a/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/driver/DateraPrimaryDataStoreDriver.java +++ b/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/driver/DateraPrimaryDataStoreDriver.java @@ -394,6 +394,11 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) } } + @Override + public boolean requiresAccessForMigration(DataObject dataObject) { + return false; + } + /** * Returns the size of template on this primary storage. If we already have a * template on this storage, we return 0 diff --git a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java index 4453906d2aa5..c81f13921c1c 100644 --- a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java +++ b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java @@ -170,6 +170,11 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) { } + @Override + public boolean requiresAccessForMigration(DataObject dataObject) { + return false; + } + @Override public long getUsedBytes(StoragePool storagePool) { return 0; diff --git a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java index d2d13eafc483..76e2f0b79f45 100644 --- a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java +++ b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java @@ -148,6 +148,11 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) { } + @Override + public boolean requiresAccessForMigration(DataObject dataObject) { + return false; + } + @Override public long getUsedBytes(StoragePool storagePool) { diff --git a/plugins/storage/volume/nexenta/src/main/java/org/apache/cloudstack/storage/datastore/driver/NexentaPrimaryDataStoreDriver.java b/plugins/storage/volume/nexenta/src/main/java/org/apache/cloudstack/storage/datastore/driver/NexentaPrimaryDataStoreDriver.java index 84051888c5e6..2ed5cf9b850b 100644 --- a/plugins/storage/volume/nexenta/src/main/java/org/apache/cloudstack/storage/datastore/driver/NexentaPrimaryDataStoreDriver.java +++ b/plugins/storage/volume/nexenta/src/main/java/org/apache/cloudstack/storage/datastore/driver/NexentaPrimaryDataStoreDriver.java @@ -68,6 +68,11 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) //To change body of implemented methods use File | Settings | File Templates. } + @Override + public boolean requiresAccessForMigration(DataObject dataObject) { + return false; + } + @Override public long getUsedBytes(StoragePool storagePool) { return 0; diff --git a/plugins/storage/volume/sample/src/main/java/org/apache/cloudstack/storage/datastore/driver/SamplePrimaryDataStoreDriverImpl.java b/plugins/storage/volume/sample/src/main/java/org/apache/cloudstack/storage/datastore/driver/SamplePrimaryDataStoreDriverImpl.java index 732786047c43..489040430759 100644 --- a/plugins/storage/volume/sample/src/main/java/org/apache/cloudstack/storage/datastore/driver/SamplePrimaryDataStoreDriverImpl.java +++ b/plugins/storage/volume/sample/src/main/java/org/apache/cloudstack/storage/datastore/driver/SamplePrimaryDataStoreDriverImpl.java @@ -88,6 +88,11 @@ public ChapInfo getChapInfo(DataObject dataObject) { @Override public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) {} + @Override + public boolean requiresAccessForMigration(DataObject dataObject) { + return false; + } + @Override public long getUsedBytes(StoragePool storagePool) { return 0; diff --git a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java index cad88dcdd15e..431fddb566ff 100644 --- a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java +++ b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java @@ -302,6 +302,11 @@ public String getConnectedSdc(long poolId, long hostId) { return null; } + @Override + public boolean requiresAccessForMigration(DataObject dataObject) { + return true; + } + @Override public long getUsedBytes(StoragePool storagePool) { long usedSpaceBytes = 0; diff --git a/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java b/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java index 4478dc98ca45..a985bff5a985 100644 --- a/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java +++ b/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java @@ -384,6 +384,11 @@ private Iops getIops(Long minIops, Long maxIops, long storagePoolId) { return new Iops(minIops, maxIops, getDefaultBurstIops(storagePoolId, maxIops)); } + @Override + public boolean requiresAccessForMigration(DataObject dataObject) { + return false; + } + @Override public long getUsedBytes(StoragePool storagePool) { return getUsedBytes(storagePool, Long.MIN_VALUE); diff --git a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java index 22ad73a118a3..56caf69a86e2 100644 --- a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java +++ b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java @@ -177,6 +177,11 @@ public boolean grantAccess(DataObject data, Host host, DataStore dataStore) { public void revokeAccess(DataObject data, Host host, DataStore dataStore) { } + @Override + public boolean requiresAccessForMigration(DataObject dataObject) { + return false; + } + private void updateStoragePool(final long poolId, final long deltaUsedBytes) { StoragePoolVO storagePool = primaryStoreDao.findById(poolId); final long capacity = storagePool.getCapacityBytes(); From 3b17c587738ff5824dfd06eda18a66e119114832 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Fri, 16 Feb 2024 11:59:17 +0530 Subject: [PATCH 2/2] Updated default implementation for requiresAccessForMigration method in PrimaryDataStoreDriver --- .../engine/subsystem/api/storage/PrimaryDataStoreDriver.java | 4 +++- .../datastore/driver/DateraPrimaryDataStoreDriver.java | 5 ----- .../driver/CloudStackPrimaryDataStoreDriverImpl.java | 5 ----- .../datastore/driver/LinstorPrimaryDataStoreDriverImpl.java | 5 ----- .../datastore/driver/NexentaPrimaryDataStoreDriver.java | 5 ----- .../datastore/driver/SamplePrimaryDataStoreDriverImpl.java | 5 ----- .../datastore/driver/SolidFirePrimaryDataStoreDriver.java | 5 ----- .../datastore/driver/StorPoolPrimaryDataStoreDriver.java | 5 ----- 8 files changed, 3 insertions(+), 36 deletions(-) diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java index b833ef6ed583..4ab88c19316c 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java @@ -42,7 +42,9 @@ enum QualityOfServiceState { MIGRATION, NO_MIGRATION } void revokeAccess(DataObject dataObject, Host host, DataStore dataStore); - boolean requiresAccessForMigration(DataObject dataObject); + default boolean requiresAccessForMigration(DataObject dataObject) { + return false; + } /** * intended for managed storage (cloud.storage_pool.managed = true) diff --git a/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/driver/DateraPrimaryDataStoreDriver.java b/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/driver/DateraPrimaryDataStoreDriver.java index 456a43fb49a3..79b621671175 100644 --- a/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/driver/DateraPrimaryDataStoreDriver.java +++ b/plugins/storage/volume/datera/src/main/java/org/apache/cloudstack/storage/datastore/driver/DateraPrimaryDataStoreDriver.java @@ -394,11 +394,6 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) } } - @Override - public boolean requiresAccessForMigration(DataObject dataObject) { - return false; - } - /** * Returns the size of template on this primary storage. If we already have a * template on this storage, we return 0 diff --git a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java index c81f13921c1c..4453906d2aa5 100644 --- a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java +++ b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java @@ -170,11 +170,6 @@ public boolean grantAccess(DataObject dataObject, Host host, DataStore dataStore public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) { } - @Override - public boolean requiresAccessForMigration(DataObject dataObject) { - return false; - } - @Override public long getUsedBytes(StoragePool storagePool) { return 0; diff --git a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java index 76e2f0b79f45..d2d13eafc483 100644 --- a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java +++ b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java @@ -148,11 +148,6 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) { } - @Override - public boolean requiresAccessForMigration(DataObject dataObject) { - return false; - } - @Override public long getUsedBytes(StoragePool storagePool) { diff --git a/plugins/storage/volume/nexenta/src/main/java/org/apache/cloudstack/storage/datastore/driver/NexentaPrimaryDataStoreDriver.java b/plugins/storage/volume/nexenta/src/main/java/org/apache/cloudstack/storage/datastore/driver/NexentaPrimaryDataStoreDriver.java index 2ed5cf9b850b..84051888c5e6 100644 --- a/plugins/storage/volume/nexenta/src/main/java/org/apache/cloudstack/storage/datastore/driver/NexentaPrimaryDataStoreDriver.java +++ b/plugins/storage/volume/nexenta/src/main/java/org/apache/cloudstack/storage/datastore/driver/NexentaPrimaryDataStoreDriver.java @@ -68,11 +68,6 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) //To change body of implemented methods use File | Settings | File Templates. } - @Override - public boolean requiresAccessForMigration(DataObject dataObject) { - return false; - } - @Override public long getUsedBytes(StoragePool storagePool) { return 0; diff --git a/plugins/storage/volume/sample/src/main/java/org/apache/cloudstack/storage/datastore/driver/SamplePrimaryDataStoreDriverImpl.java b/plugins/storage/volume/sample/src/main/java/org/apache/cloudstack/storage/datastore/driver/SamplePrimaryDataStoreDriverImpl.java index 489040430759..732786047c43 100644 --- a/plugins/storage/volume/sample/src/main/java/org/apache/cloudstack/storage/datastore/driver/SamplePrimaryDataStoreDriverImpl.java +++ b/plugins/storage/volume/sample/src/main/java/org/apache/cloudstack/storage/datastore/driver/SamplePrimaryDataStoreDriverImpl.java @@ -88,11 +88,6 @@ public ChapInfo getChapInfo(DataObject dataObject) { @Override public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore) {} - @Override - public boolean requiresAccessForMigration(DataObject dataObject) { - return false; - } - @Override public long getUsedBytes(StoragePool storagePool) { return 0; diff --git a/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java b/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java index a985bff5a985..4478dc98ca45 100644 --- a/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java +++ b/plugins/storage/volume/solidfire/src/main/java/org/apache/cloudstack/storage/datastore/driver/SolidFirePrimaryDataStoreDriver.java @@ -384,11 +384,6 @@ private Iops getIops(Long minIops, Long maxIops, long storagePoolId) { return new Iops(minIops, maxIops, getDefaultBurstIops(storagePoolId, maxIops)); } - @Override - public boolean requiresAccessForMigration(DataObject dataObject) { - return false; - } - @Override public long getUsedBytes(StoragePool storagePool) { return getUsedBytes(storagePool, Long.MIN_VALUE); diff --git a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java index 56caf69a86e2..22ad73a118a3 100644 --- a/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java +++ b/plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java @@ -177,11 +177,6 @@ public boolean grantAccess(DataObject data, Host host, DataStore dataStore) { public void revokeAccess(DataObject data, Host host, DataStore dataStore) { } - @Override - public boolean requiresAccessForMigration(DataObject dataObject) { - return false; - } - private void updateStoragePool(final long poolId, final long deltaUsedBytes) { StoragePoolVO storagePool = primaryStoreDao.findById(poolId); final long capacity = storagePool.getCapacityBytes();