Damans227 commented on code in PR #14388:
URL: https://github.com/apache/cloudstack/pull/14388#discussion_r4257835547
##########
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())) {
Review Comment:
should this also refuse a template copy on the pool? those are cs- resources
in the same resource group, and importvm doesnt check the template list like
volume import does
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetVolumesOnStorageCommandWrapper.java:
##########
@@ -97,6 +103,10 @@ private GetVolumesOnStorageAnswer
addVolumeByVolumePath(final GetVolumesOnStorag
return new GetVolumesOnStorageAnswer(command, false,
String.format("The file format is %s, but expected to be %s", fileFormat,
disk.getFormat()));
}
addDetailsToVolumeOnStorageTO(volumeOnStorageTO, info,
storagePool, disk);
+ // qemu's lock only reveals a guest on this host for pools such as
Linstor, so ask the pool
+ if
(Boolean.TRUE.equals(storagePool.isPhysicalDiskInUse(volumePath))) {
Review Comment:
should the full list ask the pool too? right now a volume in use on another
host shows as not locked there, only the lookup by path catches it
--
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]
