From 425d2574e0a97622cc0278e6e02ba42f911dbceb Mon Sep 17 00:00:00 2001 From: Rene Peinthor Date: Fri, 7 Nov 2025 08:30:51 +0100 Subject: [PATCH 1/2] linstor: improve aborting copyAsync Seems instead of returning a unsuccessful answer it is better to throw a CloudRuntimeException for error handling. --- .../driver/LinstorPrimaryDataStoreDriverImpl.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) 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 306e92599366..4edfb876c7d9 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 @@ -1063,9 +1063,7 @@ public void copyAsync(DataObject srcData, DataObject dstData, AsyncCompletionCal Answer answer = copyVolume(srcData, dstData); res = new CopyCommandResult(null, answer); } else { - Answer answer = new Answer(null, false, "noimpl"); - res = new CopyCommandResult(null, answer); - res.setResult("Not implemented yet"); + throw new CloudRuntimeException("Not implemented for Linstor primary storage."); } callback.complete(res); } @@ -1203,8 +1201,8 @@ private Answer copyTemplate(DataObject srcData, DataObject dstData) { if (optEP.isPresent()) { answer = optEP.get().sendMessage(cmd); } else { - answer = new Answer(cmd, false, "Unable to get matching Linstor endpoint."); deleteResourceDefinition(pool, rscName); + throw new CloudRuntimeException("Unable to get matching Linstor endpoint."); } } catch (ApiException exc) { logger.error("copy template failed: ", exc); @@ -1246,7 +1244,7 @@ private Answer copyVolume(DataObject srcData, DataObject dstData) { answer = optEP.get().sendMessage(cmd); } else { - answer = new Answer(cmd, false, "Unable to get matching Linstor endpoint."); + throw new CloudRuntimeException("Unable to get matching Linstor endpoint."); } } catch (ApiException exc) { logger.error("copy volume failed: ", exc); @@ -1285,8 +1283,8 @@ private Answer copyFromTemporaryResource( snapshotObject.setPath(devName); origCmd.setSrcTO(snapshotObject.getTO()); answer = optEPAny.get().sendMessage(origCmd); - } else{ - answer = new Answer(origCmd, false, "Unable to get matching Linstor endpoint."); + } else { + throw new CloudRuntimeException("Unable to get matching Linstor endpoint."); } } finally { // delete the temporary resource, noop if already gone From 1e9040f8b2dbccec40c25bcdbd448296e5ac51ee Mon Sep 17 00:00:00 2001 From: Rene Peinthor Date: Fri, 7 Nov 2025 08:32:47 +0100 Subject: [PATCH 2/2] linstor: Pick correct host for agent operations Before it was possible that a Host was picked for the primary storage that was not even part of cluster, now we only consider hosts from either the same cluster or same zone depending on primary storage settings. --- .../LinstorPrimaryDataStoreDriverImpl.java | 86 ++++++++++--------- 1 file changed, 47 insertions(+), 39 deletions(-) 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 4edfb876c7d9..6a0e36e2dd4b 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 @@ -63,6 +63,7 @@ import com.cloud.api.storage.LinstorRevertBackupSnapshotCommand; import com.cloud.configuration.Config; import com.cloud.host.Host; +import com.cloud.host.HostVO; import com.cloud.host.dao.HostDao; import com.cloud.resource.ResourceState; import com.cloud.storage.DataStoreRole; @@ -921,9 +922,10 @@ private String revertSnapshotFromImageStore( _backupsnapshotwait, VirtualMachineManager.ExecuteInSequence.value()); - Optional optEP = getDiskfullEP(linstorApi, rscName); + final StoragePool pool = (StoragePool) volumeInfo.getDataStore(); + Optional optEP = getDiskfullEP(linstorApi, pool, rscName); if (optEP.isEmpty()) { - optEP = getLinstorEP(linstorApi, rscName); + optEP = getLinstorEP(linstorApi, pool, rscName); } if (optEP.isPresent()) { @@ -1068,6 +1070,22 @@ public void copyAsync(DataObject srcData, DataObject dstData, AsyncCompletionCal callback.complete(res); } + private Host getEnabledClusterHost(StoragePool storagePool, List linstorNodeNames) { + List csHosts; + if (storagePool.getClusterId() != null) { + csHosts = _hostDao.findByClusterId(storagePool.getClusterId()); + } else { + csHosts = _hostDao.findByDataCenterId(storagePool.getDataCenterId()); + } + Collections.shuffle(csHosts); // so we do not always pick the same host for operations + for (HostVO host : csHosts) { + if (host.getResourceState() == ResourceState.Enabled && linstorNodeNames.contains(host.getName())) { + return host; + } + } + return null; + } + /** * Tries to get a Linstor cloudstack end point, that is at least diskless. * @@ -1076,47 +1094,37 @@ public void copyAsync(DataObject srcData, DataObject dstData, AsyncCompletionCal * @return Optional RemoteHostEndPoint if one could get found. * @throws ApiException */ - private Optional getLinstorEP(DevelopersApi api, String rscName) throws ApiException { + private Optional getLinstorEP(DevelopersApi api, StoragePool storagePool, String rscName) + throws ApiException { List linstorNodeNames = LinstorUtil.getLinstorNodeNames(api); - Collections.shuffle(linstorNodeNames); // do not always pick the first linstor node - - Host host = null; - for (String nodeName : linstorNodeNames) { - host = _hostDao.findByName(nodeName); - if (host != null && host.getResourceState() == ResourceState.Enabled) { - logger.info(String.format("Linstor: Make resource %s available on node %s ...", rscName, nodeName)); - ApiCallRcList answers = api.resourceMakeAvailableOnNode(rscName, nodeName, new ResourceMakeAvailable()); - if (!answers.hasError()) { - break; // found working host - } else { - logger.error( - String.format("Linstor: Unable to make resource %s on node %s available: %s", - rscName, - nodeName, - LinstorUtil.getBestErrorMessage(answers))); - } + Host host = getEnabledClusterHost(storagePool, linstorNodeNames); + if (host != null) { + logger.info("Linstor: Make resource {} available on node {} ...", rscName, host.getName()); + ApiCallRcList answers = api.resourceMakeAvailableOnNode( + rscName, host.getName(), new ResourceMakeAvailable()); + if (answers.hasError()) { + logger.error("Linstor: Unable to make resource {} on node {} available: {}", + rscName, host.getName(), LinstorUtil.getBestErrorMessage(answers)); + return Optional.empty(); + } else { + return Optional.of(RemoteHostEndPoint.getHypervisorHostEndPoint(host)); } } - if (host == null) - { - logger.error("Linstor: Couldn't create a resource on any cloudstack host."); - return Optional.empty(); - } - else - { - return Optional.of(RemoteHostEndPoint.getHypervisorHostEndPoint(host)); - } + logger.error("Linstor: Couldn't create a resource on any cloudstack host."); + return Optional.empty(); } - private Optional getDiskfullEP(DevelopersApi api, String rscName) throws ApiException { + private Optional getDiskfullEP(DevelopersApi api, StoragePool storagePool, String rscName) + throws ApiException { List linSPs = LinstorUtil.getDiskfulStoragePools(api, rscName); if (linSPs != null) { - for (com.linbit.linstor.api.model.StoragePool sp : linSPs) { - Host host = _hostDao.findByName(sp.getNodeName()); - if (host != null && host.getResourceState() == ResourceState.Enabled) { - return Optional.of(RemoteHostEndPoint.getHypervisorHostEndPoint(host)); - } + List linstorNodeNames = linSPs.stream() + .map(com.linbit.linstor.api.model.StoragePool::getNodeName) + .collect(Collectors.toList()); + Host host = getEnabledClusterHost(storagePool, linstorNodeNames); + if (host != null) { + return Optional.of(RemoteHostEndPoint.getHypervisorHostEndPoint(host)); } } logger.error("Linstor: No diskfull host found."); @@ -1197,7 +1205,7 @@ private Answer copyTemplate(DataObject srcData, DataObject dstData) { VirtualMachineManager.ExecuteInSequence.value()); try { - Optional optEP = getLinstorEP(api, rscName); + Optional optEP = getLinstorEP(api, pool, rscName); if (optEP.isPresent()) { answer = optEP.get().sendMessage(cmd); } else { @@ -1239,7 +1247,7 @@ private Answer copyVolume(DataObject srcData, DataObject dstData) { Answer answer; try { - Optional optEP = getLinstorEP(api, rscName); + Optional optEP = getLinstorEP(api, pool, rscName); if (optEP.isPresent()) { answer = optEP.get().sendMessage(cmd); } @@ -1277,7 +1285,7 @@ private Answer copyFromTemporaryResource( try { String devName = restoreResourceFromSnapshot(api, pool, rscName, snapshotName, restoreName); - Optional optEPAny = getLinstorEP(api, restoreName); + Optional optEPAny = getLinstorEP(api, pool, restoreName); if (optEPAny.isPresent()) { // patch the src device path to the temporary linstor resource snapshotObject.setPath(devName); @@ -1346,7 +1354,7 @@ protected Answer copySnapshot(DataObject srcData, DataObject destData) { VirtualMachineManager.ExecuteInSequence.value()); cmd.setOptions(options); - Optional optEP = getDiskfullEP(api, rscName); + Optional optEP = getDiskfullEP(api, pool, rscName); Answer answer; if (optEP.isPresent()) { answer = optEP.get().sendMessage(cmd);