abh1sar commented on code in PR #14388:
URL: https://github.com/apache/cloudstack/pull/14388#discussion_r4247563605
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java:
##########
@@ -357,6 +357,51 @@ public boolean connectPhysicalDisk(
return true;
}
+ /**
+ * Checks that a resource which is not managed by CloudStack yet can be
imported into this pool. It must
+ * belong to the resource group of the pool, otherwise a resource owned by
a volume of another pool on the
+ * same controller could be imported a second time.
+ */
+ public void checkPhysicalDiskForImport(String name, LinstorStoragePool
pool) {
+ final DevelopersApi api = getLinstorAPI(pool);
+ final String rscName = getLinstorRscName(name);
+ try {
+ List<ResourceDefinition> rscDfns = api.resourceDefinitionList(
+ Collections.singletonList(rscName), null, null, null,
null);
+ if (rscDfns == null || rscDfns.isEmpty()) {
+ throw new CloudRuntimeException(String.format("Linstor:
resource %s not found", rscName));
+ }
+ final String rscGrpName = rscDfns.get(0).getResourceGroupName();
+ if (rscGrpName == null ||
!rscGrpName.equalsIgnoreCase(pool.getResourceGroup())) {
+ throw new CloudRuntimeException(String.format(
+ "Linstor: resource %s belongs to resource group %s,
not to %s of this storage pool",
+ rscName, rscGrpName, pool.getResourceGroup()));
+ }
+ } catch (ApiException apiEx) {
+ logger.error(apiEx);
+ throw new CloudRuntimeException(apiEx.getBestMessage(), apiEx);
+ }
Review Comment:
Matches the existing logging in this class (e.g. `getPhysicalDisk`), and the
exception is rethrown with the cause attached.
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java:
##########
@@ -572,7 +617,30 @@ public KVMPhysicalDisk createDiskFromTemplate(
@Override
public List<KVMPhysicalDisk> listPhysicalDisks(String storagePoolUuid,
KVMStoragePool pool)
{
- throw new UnsupportedOperationException("Listing disks is not
supported for this configuration.");
+ // lists the resources of the pool's resource group by their volume
path, for volume import
+ final String rscGrpName = ((LinstorStoragePool)
pool).getResourceGroup();
+ final DevelopersApi api = getLinstorAPI(pool);
+ try {
+ List<KVMPhysicalDisk> disks = new ArrayList<>();
+ for (ResourceDefinition rd : api.resourceDefinitionList(null,
true, null, null, null)) {
Review Comment:
The Linstor API has no server-side filter by resource group or name prefix;
the plugin already lists all definitions the same way
(`LinstorUtil.getRDListStartingWith`, `findResourceDefinition`, the volume
stats cache). This is only used by the admin `listVolumesForImport` call.
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java:
##########
@@ -572,7 +617,30 @@ public KVMPhysicalDisk createDiskFromTemplate(
@Override
public List<KVMPhysicalDisk> listPhysicalDisks(String storagePoolUuid,
KVMStoragePool pool)
{
- throw new UnsupportedOperationException("Listing disks is not
supported for this configuration.");
+ // lists the resources of the pool's resource group by their volume
path, for volume import
+ final String rscGrpName = ((LinstorStoragePool)
pool).getResourceGroup();
+ final DevelopersApi api = getLinstorAPI(pool);
+ try {
+ List<KVMPhysicalDisk> disks = new ArrayList<>();
+ for (ResourceDefinition rd : api.resourceDefinitionList(null,
true, null, null, null)) {
+ if
(!rd.getName().toLowerCase().startsWith(LinstorUtil.RSC_PREFIX) ||
rd.getResourceGroupName() == null
+ ||
!rd.getResourceGroupName().equalsIgnoreCase(rscGrpName)
+ || rd.getVolumeDefinitions() == null ||
rd.getVolumeDefinitions().isEmpty()) {
+ continue;
+ }
+ final KVMPhysicalDisk disk = new
KVMPhysicalDisk(LinstorUtil.formatDrbdByResDevicePath(rd.getName()),
+
rd.getName().substring(LinstorUtil.RSC_PREFIX.length()), pool);
Review Comment:
A Linstor resource definition always has a name (it is its key); the
existing helpers rely on that as well.
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStoragePool.java:
##########
@@ -92,6 +92,18 @@ public boolean disconnectPhysicalDisk(String volumeUuid)
return _storageAdaptor.disconnectPhysicalDisk(volumeUuid, this);
}
+ @Override
+ public void checkPhysicalDiskForImport(String volumeUuid)
+ {
+
((LinstorStorageAdaptor)_storageAdaptor).checkPhysicalDiskForImport(volumeUuid,
this);
+ }
+
+ @Override
+ public Boolean isPhysicalDiskInUse(String volumeUuid)
+ {
+ return
((LinstorStorageAdaptor)_storageAdaptor).isPhysicalDiskInUse(volumeUuid, this);
+ }
Review Comment:
Follows the existing pattern in this class (`getCapacity`, `getUsed`,
`getAvailable`, `isNodeOnline`); the pool is only ever created by
`LinstorStorageAdaptor`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
