Skip to content

Add support to update secondary and primary storage URLs #13193

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.

Sign up for GitHub

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

Draft
Pearl1594 wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Add support to update secondary and primary storage URLs #13193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3bae392
Add support to update secondary and primary storage URLs
May 19, 2026
eaffcd7
Add test, and propagate the modification of URL of primary store to t…
May 20, 2026
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
11 changes: 11 additions & 0 deletions api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateImageStoreCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ///////////////////////
/////////////////////////////////////////////////////
Expand All @@ -70,6 +77,10 @@ public Long getCapacityBytes() {
return capacityBytes;
}

public String getUrl() {
return url;
}

/////////////////////////////////////////////////////
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////
Expand Down
7 changes: 6 additions & 1 deletion api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStoragePoolCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Comment on lines +76 to +81
since = "4.19.0")
private String url;

Expand Down
2 changes: 1 addition & 1 deletion engine/components-api/src/main/java/com/cloud/storage/StorageManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ void connectHostsToPool(DataStore primaryStore, List<Long> 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();

Expand Down
33 changes: 33 additions & 0 deletions ...hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,39 @@
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 {

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

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Extract this nested try block into a separate method. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ5HUpehilR_hphlMp5c&open=AZ5HUpehilR_hphlMp5c&pullRequest=13193
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.
Expand Down Expand Up @@ -1250,7 +1283,7 @@
* If it has been created on Primary Storage, it will be copied on the Primary Storage
*/
@Override
public KVMPhysicalDisk createDiskFromTemplate(KVMPhysicalDisk template,

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

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

A "Brain Method" was detected. Refactor it to reduce at least one of the following metrics: LOC from 76 to 64, Complexity from 16 to 14, Nesting Level from 5 to 2, Number of Variables from 23 to 6. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ5HUpehilR_hphlMp5d&open=AZ5HUpehilR_hphlMp5d&pullRequest=13193
String name, PhysicalDiskFormat format, Storage.ProvisioningType provisioningType, long size, KVMStoragePool destPool, int timeout, byte[] passphrase) {

logger.info("Creating volume " + name + " from template " + template.getName() + " in pool " + destPool.getUuid() +
Expand Down
175 changes: 175 additions & 0 deletions ...rvisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -56,6 +60,9 @@
@Mock
LibvirtStoragePool mockPool;

@Mock
StorageLayer storageLayer;

MockedStatic<Script> mockScript;

@Spy
Expand All @@ -67,6 +74,7 @@
libvirtConnectionMockedStatic = Mockito.mockStatic(LibvirtConnection.class);
Mockito.reset(mockPool);
mockScript = Mockito.mockStatic(Script.class);
ReflectionTestUtils.setField(libvirtStorageAdaptor, "_storageLayer", storageLayer);
}

@After
Expand Down Expand Up @@ -176,4 +184,171 @@

Mockito.verify(mockPool, never()).setUsedIops(anyLong());
}

/**
* Creates a {@link LibvirtException} via reflection since its only constructor is package-private.
*/
private static LibvirtException createLibvirtException(String message) throws Exception {
org.libvirt.Error virError = Mockito.mock(org.libvirt.Error.class);
Mockito.when(virError.getMessage()).thenReturn(message);
java.lang.reflect.Constructor<LibvirtException> ctor =
LibvirtException.class.getDeclaredConstructor(org.libvirt.Error.class);
ctor.setAccessible(true);
return ctor.newInstance(virError);
}

private String buildNfsPoolXml(String uuid, String host, String dir, String targetPath) {
return "<pool type='netfs'>\n" +
"<name>" + uuid + "</name>\n<uuid>" + uuid + "</uuid>\n" +
"<source>\n<host name='" + host + "'/>\n<dir path='" + dir + "'/>\n</source>\n" +
"<target>\n<path>" + targetPath + "</path>\n</target>\n</pool>\n";
}

/**
* NFS pool exists with same host and path: should reuse it without destroying.
*/
@Test
public void testCreateStoragePool_NFS_SameHostAndPath_ReusesPool() throws Exception {
String uuid = UUID.randomUUID().toString();
String host = "10.0.0.1";
String path = "/export/primary";
String targetPath = "/mnt/" + uuid;
String poolXml = buildNfsPoolXml(uuid, host, path, targetPath);

Connect conn = Mockito.mock(Connect.class);
StoragePool sp = Mockito.mock(StoragePool.class);
Mockito.when(LibvirtConnection.getConnection()).thenReturn(conn);
Mockito.when(conn.storagePoolLookupByUUIDString(uuid)).thenReturn(sp);
Mockito.when(sp.isActive()).thenReturn(1);
Mockito.when(sp.getXMLDesc(0)).thenReturn(poolXml);
Mockito.when(Script.runSimpleBashScriptForExitValue(anyString())).thenReturn(0);
Mockito.doReturn(Mockito.mock(KVMStoragePool.class)).when(libvirtStorageAdaptor).getStoragePool(uuid);

libvirtStorageAdaptor.createStoragePool(uuid, host, 0, path, null,
Storage.StoragePoolType.NetworkFilesystem, null, true);

// pool was not destroyed since source didn't change
Mockito.verify(sp, Mockito.never()).destroy();
Mockito.verify(sp, Mockito.never()).undefine();
}

/**
* NFS pool exists but path has changed: should destroy and recreate.
*/
@Test
public void testCreateStoragePool_NFS_PathChanged_DestroysAndRecreates() throws Exception {
String uuid = UUID.randomUUID().toString();
String host = "10.0.0.1";
String oldPath = "/export/old";
String newPath = "/export/new";
String targetPath = "/mnt/" + uuid;
String poolXml = buildNfsPoolXml(uuid, host, oldPath, targetPath);

Connect conn = Mockito.mock(Connect.class);
StoragePool sp = Mockito.mock(StoragePool.class);
Mockito.when(LibvirtConnection.getConnection()).thenReturn(conn);
Mockito.when(conn.storagePoolLookupByUUIDString(uuid)).thenReturn(sp);
Mockito.when(sp.isActive()).thenReturn(1);
Mockito.when(sp.getXMLDesc(0)).thenReturn(poolXml);
Mockito.when(sp.isPersistent()).thenReturn(1);
Mockito.when(conn.listStoragePools()).thenReturn(new String[]{});
StoragePool newSp = Mockito.mock(StoragePool.class);
Mockito.when(conn.storagePoolCreateXML(anyString(), Mockito.eq(0))).thenReturn(newSp);
Mockito.when(Script.runSimpleBashScriptForExitValue(anyString())).thenReturn(0);
Mockito.doReturn(Mockito.mock(KVMStoragePool.class)).when(libvirtStorageAdaptor).getStoragePool(uuid);

libvirtStorageAdaptor.createStoragePool(uuid, host, 0, newPath, null,
Storage.StoragePoolType.NetworkFilesystem, null, true);

Mockito.verify(sp).destroy();
Mockito.verify(sp).undefine();
Mockito.verify(sp).free();
}

/**
* NFS pool exists but host has changed: should destroy and recreate.
*/
@Test
public void testCreateStoragePool_NFS_HostChanged_DestroysAndRecreates() throws Exception {
String uuid = UUID.randomUUID().toString();
String oldHost = "10.0.0.1";
String newHost = "10.0.0.2";
String path = "/export/primary";
String targetPath = "/mnt/" + uuid;
String poolXml = buildNfsPoolXml(uuid, oldHost, path, targetPath);

Connect conn = Mockito.mock(Connect.class);
StoragePool sp = Mockito.mock(StoragePool.class);
Mockito.when(LibvirtConnection.getConnection()).thenReturn(conn);
Mockito.when(conn.storagePoolLookupByUUIDString(uuid)).thenReturn(sp);
Mockito.when(sp.isActive()).thenReturn(1);
Mockito.when(sp.getXMLDesc(0)).thenReturn(poolXml);
Mockito.when(sp.isPersistent()).thenReturn(1);
Mockito.when(conn.listStoragePools()).thenReturn(new String[]{});
StoragePool newSp = Mockito.mock(StoragePool.class);
Mockito.when(conn.storagePoolCreateXML(anyString(), Mockito.eq(0))).thenReturn(newSp);
Mockito.when(Script.runSimpleBashScriptForExitValue(anyString())).thenReturn(0);
Mockito.doReturn(Mockito.mock(KVMStoragePool.class)).when(libvirtStorageAdaptor).getStoragePool(uuid);

libvirtStorageAdaptor.createStoragePool(uuid, newHost, 0, path, null,
Storage.StoragePoolType.NetworkFilesystem, null, true);

Mockito.verify(sp).destroy();
Mockito.verify(sp).undefine();
Mockito.verify(sp).free();
}

/**
* RBD pool exists with different source — should NOT destroy (RBD is excluded from this logic).
*/
@Test
public void testCreateStoragePool_RBD_SourceChanged_DoesNotDestroy() throws Exception {
String uuid = UUID.randomUUID().toString();
String rbdPoolXml = "<pool type='rbd'>\n<name>" + uuid + "</name>\n<uuid>" + uuid + "</uuid>\n" +

Check warning on line 307 in plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptorTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this unused "rbdPoolXml" local variable. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ5HUpiIilR_hphlMp5f&open=AZ5HUpiIilR_hphlMp5f&pullRequest=13193
"<source>\n<host name='10.0.0.1'/>\n<name>oldpool</name>\n</source>\n</pool>\n";

Check warning on line 308 in plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptorTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this useless assignment to local variable "rbdPoolXml". 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ5HUpiIilR_hphlMp5e&open=AZ5HUpiIilR_hphlMp5e&pullRequest=13193

Connect conn = Mockito.mock(Connect.class);
StoragePool sp = Mockito.mock(StoragePool.class);
Mockito.when(LibvirtConnection.getConnection()).thenReturn(conn);
Mockito.when(conn.storagePoolLookupByUUIDString(uuid)).thenReturn(sp);
Mockito.when(sp.isActive()).thenReturn(1);
Mockito.doReturn(Mockito.mock(KVMStoragePool.class)).when(libvirtStorageAdaptor).getStoragePool(uuid);

libvirtStorageAdaptor.createStoragePool(uuid, "10.0.0.2", 6789, "newpool", "user:secret",
Storage.StoragePoolType.RBD, null, true);

Mockito.verify(sp, Mockito.never()).destroy();
Mockito.verify(sp, Mockito.never()).undefine();
}

/**
* NFS pool exists, destroy fails — pool should be reused (not crash), sp.free() not called.
*/
@Test
public void testCreateStoragePool_NFS_DestroyFails_ReusesExistingPool() throws Exception {
String uuid = UUID.randomUUID().toString();
String host = "10.0.0.1";
String oldPath = "/export/old";
String newPath = "/export/new";
String targetPath = "/mnt/" + uuid;
String poolXml = buildNfsPoolXml(uuid, host, oldPath, targetPath);

Connect conn = Mockito.mock(Connect.class);
StoragePool sp = Mockito.mock(StoragePool.class);
Mockito.when(LibvirtConnection.getConnection()).thenReturn(conn);
Mockito.when(conn.storagePoolLookupByUUIDString(uuid)).thenReturn(sp);
Mockito.when(sp.isActive()).thenReturn(1);
Mockito.when(sp.getXMLDesc(0)).thenReturn(poolXml);
Mockito.when(sp.isPersistent()).thenReturn(1);
LibvirtException libvirtException = createLibvirtException("pool is busy");
Mockito.doThrow(libvirtException).when(sp).destroy();
Mockito.when(Script.runSimpleBashScriptForExitValue(anyString())).thenReturn(0);
Mockito.doReturn(Mockito.mock(KVMStoragePool.class)).when(libvirtStorageAdaptor).getStoragePool(uuid);

libvirtStorageAdaptor.createStoragePool(uuid, host, 0, newPath, null,
Storage.StoragePoolType.NetworkFilesystem, null, true);

Mockito.verify(sp).destroy();
Mockito.verify(sp, Mockito.never()).free();
}
}
2 changes: 1 addition & 1 deletion server/src/main/java/com/cloud/server/StatsCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@
}
}

protected void init(Map<String, String> configs) {

Check warning on line 433 in server/src/main/java/com/cloud/server/StatsCollector.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

A "Brain Method" was detected. Refactor it to reduce at least one of the following metrics: LOC from 127 to 64, Complexity from 19 to 14, Nesting Level from 3 to 2, Number of Variables from 12 to 6. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ5BdKQjc-PA8pjUL6Z0&open=AZ5BdKQjc-PA8pjUL6Z0&pullRequest=13193
_executor = Executors.newScheduledThreadPool(6, new NamedThreadFactory("StatsCollector"));

hostStatsInterval = NumbersUtil.parseLong(configs.get("host.stats.interval"), ONE_MINUTE_IN_MILLISCONDS);
Expand Down Expand Up @@ -1448,7 +1448,7 @@
try {
Transaction.execute(new TransactionCallbackNoReturn() {
@Override
public void doInTransactionWithoutResult(TransactionStatus status) {

Check warning on line 1451 in server/src/main/java/com/cloud/server/StatsCollector.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

A "Brain Method" was detected. Refactor it to reduce at least one of the following metrics: LOC from 70 to 64, Complexity from 15 to 14, Nesting Level from 3 to 2, Number of Variables from 15 to 6. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ5BdKQjc-PA8pjUL6Z1&open=AZ5BdKQjc-PA8pjUL6Z1&pullRequest=13193
Pair<Map<Long, VMInstanceVO>, Map<String, Long>> vmsAndMap = getVmMapForStatsForHost(host);
Map<Long, VMInstanceVO> vmMap = vmsAndMap.first();
HashMap<Long, List<? extends VmDiskStats>> vmDiskStatsById =
Expand Down Expand Up @@ -1557,7 +1557,7 @@
try {
Transaction.execute(new TransactionCallbackNoReturn() {
@Override
public void doInTransactionWithoutResult(TransactionStatus status) {

Check warning on line 1560 in server/src/main/java/com/cloud/server/StatsCollector.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

A "Brain Method" was detected. Refactor it to reduce at least one of the following metrics: LOC from 68 to 64, Complexity from 19 to 14, Nesting Level from 3 to 2, Number of Variables from 15 to 6. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ5BdKQjc-PA8pjUL6Z2&open=AZ5BdKQjc-PA8pjUL6Z2&pullRequest=13193
Pair<Map<Long, VMInstanceVO>, Map<String, Long>> vmsAndMap = getVmMapForStatsForHost(host);
Map<Long, VMInstanceVO> vmMap = vmsAndMap.first();
HashMap<Long, List<? extends VmNetworkStats>> vmNetworkStatsById =
Expand Down Expand Up @@ -1698,7 +1698,7 @@

class StorageCollector extends ManagedContextRunnable {
@Override
protected void runInContext() {

Check warning on line 1701 in server/src/main/java/com/cloud/server/StatsCollector.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

A "Brain Method" was detected. Refactor it to reduce at least one of the following metrics: LOC from 76 to 64, Complexity from 23 to 14, Nesting Level from 6 to 2, Number of Variables from 24 to 6. 


See more on https://sonarcloud.io/project/issues?id=apache_cloudstack&issues=AZ5BdKQjc-PA8pjUL6Zz&open=AZ5BdKQjc-PA8pjUL6Zz&pullRequest=13193
try {
if (logger.isDebugEnabled()) {
logger.debug("StorageCollector is running...");
Expand Down Expand Up @@ -1787,7 +1787,7 @@
&& (_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
Expand Down
Loading
Loading