From 3bae3926a150448a91f7600ade1c774d6010e085 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Tue, 19 May 2026 13:15:00 -0400 Subject: [PATCH 1/3] Add support to update secondary and primary storage URLs --- .../admin/storage/UpdateImageStoreCmd.java | 11 +++++++ .../com/cloud/storage/StorageManager.java | 2 +- .../java/com/cloud/server/StatsCollector.java | 2 +- .../com/cloud/storage/StorageManagerImpl.java | 32 +++++++++++++++++-- 4 files changed, 42 insertions(+), 5 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateImageStoreCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateImageStoreCmd.java index 0e1631a46ba2..ee83b78d07fe 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateImageStoreCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateImageStoreCmd.java @@ -50,6 +50,13 @@ public class UpdateImageStoreCmd extends BaseCmd { description = "The number of bytes CloudStack can use on this image storage.\n\tNOTE: this will be overwritten by the StatsCollector as soon as there is a SSVM to query the storage.") private Long capacityBytes; + @Parameter(name = ApiConstants.URL, type = CommandType.STRING, required = false, + description = "The new URL for the image store (e.g. nfs://new-host/export). " + + "The image store must be in read-only state before its URL can be changed. " + + "After updating, destroy and recreate any Secondary Storage VMs so they remount the new path.", + since = "4.23.0") + private String url; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -70,6 +77,10 @@ public Long getCapacityBytes() { return capacityBytes; } + public String getUrl() { + return url; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java index 3c62738f9ed5..455ac8c33389 100644 --- a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java +++ b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java @@ -411,7 +411,7 @@ void connectHostsToPool(DataStore primaryStore, List hostIds, Scope scope, Long getDiskIopsWriteRate(ServiceOffering offering, DiskOffering diskOffering); - ImageStore updateImageStoreStatus(Long id, String name, Boolean readonly, Long capacityBytes); + ImageStore updateImageStoreStatus(Long id, String name, Boolean readonly, Long capacityBytes, String url); void cleanupDownloadUrls(); diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index 049ed4543617..5a98feac51a2 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -1787,7 +1787,7 @@ private void updateStorageStats(ConcurrentHashMap storageSta && (_storageStats.get(storeId).getCapacityBytes() == 0l || _storageStats.get(storeId).getCapacityBytes() != storageStats.get(storeId).getCapacityBytes())) { // get add to DB rigorously - _storageManager.updateImageStoreStatus(storeId, null, null, storageStats.get(storeId).getCapacityBytes()); + _storageManager.updateImageStoreStatus(storeId, null, null, storageStats.get(storeId).getCapacityBytes(), null); } } // if in _storageStats and not in storageStats it gets discarded diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 7c501c78beeb..f62df0585ae1 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -1285,6 +1285,10 @@ public PrimaryDataStoreInfo updateStoragePool(UpdateStoragePoolCmd cmd) throws I changes = true; } + if (cmd.getUrl() != null) { + changes = true; + } + if (changes) { StoragePoolVO storagePool = _storagePoolDao.findById(id); DataStoreProvider dataStoreProvider = _dataStoreProviderMgr.getDataStoreProvider(storagePool.getStorageProviderName()); @@ -1300,6 +1304,21 @@ public PrimaryDataStoreInfo updateStoragePool(UpdateStoragePoolCmd cmd) throws I _storagePoolDao.updateCapacityIops(id, updatedCapacityIops); } if (cmd.getUrl() != null) { + if (!storagePool.isInMaintenance()) { + throw new InvalidParameterValueException("Storage pool must be in Maintenance state before its URL can be changed. " + + "Please put the pool into maintenance first."); + } + URI newUri; + try { + newUri = new URI(cmd.getUrl()); + } catch (URISyntaxException e) { + throw new InvalidParameterValueException("Invalid URL format: " + cmd.getUrl()); + } + storagePool.setHostAddress(newUri.getHost()); + storagePool.setPath(newUri.getPath()); + if (newUri.getPort() != -1) { + storagePool.setPort(newUri.getPort()); + } details.put("url", cmd.getUrl()); } _storagePoolDao.update(id, storagePool); @@ -4129,18 +4148,25 @@ public ImageStore migrateToObjectStore(String name, String url, String providerN @Override public ImageStore updateImageStore(UpdateImageStoreCmd cmd) { - return updateImageStoreStatus(cmd.getId(), cmd.getName(), cmd.getReadonly(), cmd.getCapacityBytes()); + return updateImageStoreStatus(cmd.getId(), cmd.getName(), cmd.getReadonly(), cmd.getCapacityBytes(), cmd.getUrl()); } @Override @ActionEvent(eventType = EventTypes.EVENT_UPDATE_IMAGE_STORE_ACCESS_STATE, eventDescription = "image store access updated") - public ImageStore updateImageStoreStatus(Long id, String name, Boolean readonly, Long capacityBytes) { + public ImageStore updateImageStoreStatus(Long id, String name, Boolean readonly, Long capacityBytes, String url) { // Input validation ImageStoreVO imageStoreVO = _imageStoreDao.findById(id); if (imageStoreVO == null) { throw new IllegalArgumentException("Unable to find image store with ID: " + id); } + if (url != null) { + if (!imageStoreVO.isReadonly()) { + throw new InvalidParameterValueException("Image store must be set to read-only (maintenance) state before its URL can be changed. " + + "Please set readOnly=true on the image store first."); + } + imageStoreVO.setUrl(url); + } if (com.cloud.utils.StringUtils.isNotBlank(name)) { imageStoreVO.setName(name); } @@ -4156,7 +4182,7 @@ public ImageStore updateImageStoreStatus(Long id, String name, Boolean readonly, @Override public ImageStore updateImageStoreStatus(Long id, Boolean readonly) { - return updateImageStoreStatus(id, null, readonly, null); + return updateImageStoreStatus(id, null, readonly, null, null); } /** From eaffcd7be8a5d36e6f2d4042ef9b08d7cb8bdb0e Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Wed, 20 May 2026 16:34:37 -0400 Subject: [PATCH 2/3] Add test, and propagate the modification of URL of primary store to the hosts only for nfs and gluster --- .../admin/storage/UpdateStoragePoolCmd.java | 7 +- .../kvm/storage/LibvirtStorageAdaptor.java | 33 ++++ .../storage/LibvirtStorageAdaptorTest.java | 175 ++++++++++++++++++ .../com/cloud/storage/StorageManagerImpl.java | 89 +++++++-- 4 files changed, 288 insertions(+), 16 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStoragePoolCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStoragePoolCmd.java index 4b0a6ba00b28..9529da84df76 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStoragePoolCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStoragePoolCmd.java @@ -73,7 +73,12 @@ public class UpdateStoragePoolCmd extends BaseCmd { @Parameter(name = ApiConstants.URL, type = CommandType.STRING, required = false, - description = "the URL of the storage pool", + description = "the URL of the storage pool. Supported only for NFS and Gluster pool type." + + "The pool must be in Maintenance mode before changing the URL. WARNING: use this parameter" + + "with caution. It is intended for failover scenarios where the storage content is already " + + "fully mirrored at the new location. Pointing to a new location without ensuring complete " + + "data parity will result in data loss or corruption. After the URL is updated, the new mount" + + "is applied to all connected hosts or restart the Management server", since = "4.19.0") private String url; 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 a03daeb197bf..5e00c24e45c8 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 @@ -766,6 +766,39 @@ public KVMStoragePool createStoragePool(String name, String host, int port, Stri path = path.substring(0, path.length() - 1); } + // For NFS and Gluster pools, if the existing pool's source host or path has changed + // (e.g. after a storage URL update), destroy and undefine it so it gets recreated + // with the new source below. Restricted to NFS/Gluster only. + if (sp != null && (type == StoragePoolType.NetworkFilesystem || type == StoragePoolType.Gluster)) { + try { + LibvirtStoragePoolDef existingDef = getStoragePoolDef(conn, sp); + if (existingDef != null) { + String existingSourceHost = existingDef.getSourceHost(); + String existingSourceDir = existingDef.getSourceDir(); + boolean hostChanged = host != null && existingSourceHost != null && !existingSourceHost.equals(host); + boolean pathChanged = path != null && existingSourceDir != null && !existingSourceDir.equals(path); + if (hostChanged || pathChanged) { + logger.info("Storage pool {} source has changed (host: {} -> {}, path: {} -> {}); destroying and redefining.", + name, existingSourceHost, host, existingSourceDir, path); + try { + if (sp.isPersistent() == 1) { + sp.destroy(); + sp.undefine(); + } else { + sp.destroy(); + } + sp.free(); + sp = null; + } catch (LibvirtException destroyEx) { + logger.error("Failed to destroy storage pool {} with changed source; will attempt to reuse existing pool: {}", name, destroyEx.getMessage()); + } + } + } + } catch (LibvirtException e) { + logger.warn("Failed to check existing storage pool {} source path, will attempt to reuse it: {}", name, e.getMessage()); + } + } + if (sp == null) { // see if any existing pool by another name is using our storage path. // if anyone is, undefine the pool so we can define it as requested. diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptorTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptorTest.java index 88346abd0176..259cb603aeb8 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptorTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptorTest.java @@ -42,9 +42,13 @@ import com.cloud.hypervisor.kvm.resource.LibvirtConnection; import com.cloud.hypervisor.kvm.resource.LibvirtStoragePoolDef; import com.cloud.storage.Storage; +import com.cloud.storage.StorageLayer; import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.Script; +import org.libvirt.LibvirtException; + +import org.springframework.test.util.ReflectionTestUtils; @RunWith(MockitoJUnitRunner.class) public class LibvirtStorageAdaptorTest { @@ -56,6 +60,9 @@ public class LibvirtStorageAdaptorTest { @Mock LibvirtStoragePool mockPool; + @Mock + StorageLayer storageLayer; + MockedStatic