Skip to content

Commit a4a3099

Browse files
committed
linstor: Fix using multiple primary storage with same linstor-controller
It should have been always possible to use multiple primary storages, with the same linstor-controller, by just using different resource-groups with different settings. But if the same template was used on 2 different primary storages, there would be a name collision on the linstor-controller, as 2 of them would get allocated to the same name. This commit fixes this, by intelligently reusing the same template, as long as possible (if select filter properties match enough).
1 parent b186272 commit a4a3099

File tree

4 files changed

+367
-50
lines changed

4 files changed

+367
-50
lines changed

plugins/storage/volume/linstor/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ All notable changes to Linstor CloudStack plugin will be documented in this file
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8+
## [2025-01-27]
9+
10+
### Fixed
11+
12+
- Use of multiple primary storages on the same linstor controller
13+
814
## [2025-01-20]
915

1016
### Fixed

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

Lines changed: 85 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,11 @@
6060
import com.linbit.linstor.api.model.VolumeDefinition;
6161

6262
import java.io.File;
63+
import java.io.FileInputStream;
64+
import java.io.IOException;
65+
import java.nio.file.Files;
66+
import java.nio.file.Path;
67+
import java.nio.file.Paths;
6368

6469
@StorageAdaptorInfo(storagePoolType=Storage.StoragePoolType.Linstor)
6570
public class LinstorStorageAdaptor implements StorageAdaptor {
@@ -198,10 +203,10 @@ public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, Qemu
198203
final DevelopersApi api = getLinstorAPI(pool);
199204

200205
try {
201-
List<ResourceDefinition> definitionList = api.resourceDefinitionList(
202-
Collections.singletonList(rscName), null, null, null);
206+
ResourceDefinition resourceDefinition = LinstorUtil.findResourceDefinition(
207+
api, rscName, lpool.getResourceGroup());
203208

204-
if (definitionList.isEmpty()) {
209+
if (resourceDefinition == null) {
205210
ResourceGroupSpawn rgSpawn = new ResourceGroupSpawn();
206211
rgSpawn.setResourceDefinitionName(rscName);
207212
rgSpawn.addVolumeSizesItem(size / 1024); // linstor uses KiB
@@ -211,22 +216,28 @@ public KVMPhysicalDisk createPhysicalDisk(String name, KVMStoragePool pool, Qemu
211216
handleLinstorApiAnswers(answers, "Linstor: Unable to spawn resource.");
212217
}
213218

219+
String foundRscName = resourceDefinition != null ? resourceDefinition.getName() : rscName;
220+
214221
// query linstor for the device path
215222
List<ResourceWithVolumes> resources = api.viewResources(
216223
Collections.emptyList(),
217-
Collections.singletonList(rscName),
224+
Collections.singletonList(foundRscName),
218225
Collections.emptyList(),
219226
null,
220227
null,
221228
null);
222229

223-
makeResourceAvailable(api, rscName, false);
230+
makeResourceAvailable(api, foundRscName, false);
224231

225232
if (!resources.isEmpty() && !resources.get(0).getVolumes().isEmpty()) {
226233
final String devPath = resources.get(0).getVolumes().get(0).getDevicePath();
227234
s_logger.info("Linstor: Created drbd device: " + devPath);
228235
final KVMPhysicalDisk kvmDisk = new KVMPhysicalDisk(devPath, name, pool);
229236
kvmDisk.setFormat(QemuImg.PhysicalDiskFormat.RAW);
237+
long allocatedKib = resources.get(0).getVolumes().get(0).getAllocatedSizeKib() != null ?
238+
resources.get(0).getVolumes().get(0).getAllocatedSizeKib() : 0;
239+
kvmDisk.setSize(allocatedKib >= 0 ? allocatedKib * 1024 : 0);
240+
kvmDisk.setVirtualSize(size);
230241
return kvmDisk;
231242
} else {
232243
s_logger.error("Linstor: viewResources didn't return resources or volumes.");
@@ -475,16 +486,39 @@ public boolean deletePhysicalDisk(String name, KVMStoragePool pool, Storage.Imag
475486
{
476487
s_logger.debug("Linstor: deletePhysicalDisk " + name);
477488
final DevelopersApi api = getLinstorAPI(pool);
478-
489+
final String rscName = getLinstorRscName(name);
490+
final LinstorStoragePool linstorPool = (LinstorStoragePool) pool;
491+
String rscGrpName = linstorPool.getResourceGroup();
492+
boolean deleted = false;
479493
try {
480-
final String rscName = getLinstorRscName(name);
481-
s_logger.debug("Linstor: delete resource definition " + rscName);
482-
ApiCallRcList answers = api.resourceDefinitionDelete(rscName);
483-
handleLinstorApiAnswers(answers, "Linstor: Unable to delete resource definition " + rscName);
494+
List<ResourceDefinition> existingRDs = LinstorUtil.getRDListStartingWith(api, rscName);
495+
496+
for (ResourceDefinition rd : existingRDs) {
497+
int expectedProps = 0; // if it is a non template resource, we don't expect any _cs-template-for- prop
498+
String propKey = LinstorUtil.getTemplateForAuxPropKey(rscGrpName);
499+
if (rd.getProps().containsKey(propKey)) {
500+
ResourceDefinitionModify rdm = new ResourceDefinitionModify();
501+
rdm.deleteProps(Collections.singletonList(propKey));
502+
api.resourceDefinitionModify(rd.getName(), rdm);
503+
expectedProps = 1;
504+
}
505+
506+
// if there is only one template-for property left for templates, the template isn't needed anymore
507+
// or if it isn't a template anyway, it will not have this Aux property
508+
// _cs-template-for- poperties work like a ref-count.
509+
if (rd.getProps().keySet().stream()
510+
.filter(key -> key.startsWith("Aux/" + LinstorUtil.CS_TEMPLATE_FOR_PREFIX))
511+
.count() == expectedProps) {
512+
ApiCallRcList answers = api.resourceDefinitionDelete(rd.getName());
513+
checkLinstorAnswersThrow(answers);
514+
deleted = true;
515+
}
516+
}
484517
} catch (ApiException apiEx) {
518+
s_logger.error("Linstor: ApiEx - " + apiEx.getMessage());
485519
throw new CloudRuntimeException(apiEx.getBestMessage(), apiEx);
486520
}
487-
return true;
521+
return deleted;
488522
}
489523

490524
@Override
@@ -558,6 +592,31 @@ private boolean resourceSupportZeroBlocks(KVMStoragePool destPool, String resNam
558592
return false;
559593
}
560594

595+
/**
596+
* Checks if the given disk is the SystemVM template, by checking its properties file in the same directory.
597+
* The initial systemvm template resource isn't created on the management server, but
598+
* we now need to know if the systemvm template is used, while copying.
599+
* @param disk
600+
* @return True if it is the systemvm template disk, else false.
601+
*/
602+
private static boolean isSystemTemplate(KVMPhysicalDisk disk) {
603+
Path diskPath = Paths.get(disk.getPath());
604+
Path propFile = diskPath.getParent().resolve("template.properties");
605+
if (Files.exists(propFile)) {
606+
java.util.Properties templateProps = new java.util.Properties();
607+
try {
608+
templateProps.load(new FileInputStream(propFile.toFile()));
609+
String desc = templateProps.getProperty("description");
610+
if (desc.startsWith("SystemVM Template")) {
611+
return true;
612+
}
613+
} catch (IOException e) {
614+
return false;
615+
}
616+
}
617+
return false;
618+
}
619+
561620
@Override
562621
public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk disk, String name, KVMStoragePool destPools, int timeout, byte[] srcPassphrase, byte[] destPassphrase, Storage.ProvisioningType provisioningType)
563622
{
@@ -569,17 +628,29 @@ public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk disk, String name, KVMSt
569628

570629
final KVMPhysicalDisk dstDisk = destPools.createPhysicalDisk(
571630
name, QemuImg.PhysicalDiskFormat.RAW, provisioningType, disk.getVirtualSize(), null);
631+
final String rscName = getLinstorRscName(name);
572632

573633
final DevelopersApi api = getLinstorAPI(destPools);
574-
applyAuxProps(api, name, disk.getDispName(), disk.getVmName());
634+
// if it is the initial systemvm disk copy, we need to apply the _cs-template-for property.
635+
if (isSystemTemplate(disk)) {
636+
applyAuxProps(api, name, "SystemVM Template", null);
637+
LinstorStoragePool linPool = (LinstorStoragePool) destPools;
638+
try {
639+
LinstorUtil.setAuxTemplateForProperty(api, rscName, linPool.getResourceGroup());
640+
} catch (ApiException apiExc) {
641+
s_logger.error(String.format("Error setting aux template for property for %s", rscName));
642+
logLinstorAnswers(apiExc.getApiCallRcList());
643+
}
644+
} else {
645+
applyAuxProps(api, name, disk.getDispName(), disk.getVmName());
646+
}
575647

576648
s_logger.debug(String.format("Linstor.copyPhysicalDisk: dstPath: %s", dstDisk.getPath()));
577649
final QemuImgFile destFile = new QemuImgFile(dstDisk.getPath());
578650
destFile.setFormat(dstDisk.getFormat());
579651
destFile.setSize(disk.getVirtualSize());
580652

581-
boolean zeroedDevice = resourceSupportZeroBlocks(destPools, LinstorUtil.RSC_PREFIX + name);
582-
653+
boolean zeroedDevice = resourceSupportZeroBlocks(destPools, rscName);
583654
try {
584655
final QemuImg qemu = new QemuImg(timeout, zeroedDevice, true);
585656
qemu.convert(srcFile, destFile);

0 commit comments

Comments
 (0)