Damans227 commented on code in PR #14389:
URL: https://github.com/apache/cloudstack/pull/14389#discussion_r4269917586


##########
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/driver/StorPoolPrimaryDataStoreDriver.java:
##########
@@ -1367,6 +1377,120 @@ public Pair<Long, Long> getVolumeStats(StoragePool 
storagePool, String volumeId)
         return null;
     }
 
+    /**
+     * Lists StorPool volumes available for import (globalId name, no "cs" tag 
unless
+     * {@code forUnmanage}). When {@code volumePath} is given, looks up that 
one volume directly
+     * instead of browsing the whole pool; when {@code forUnmanage} is also 
true, an already
+     * "cs"-tagged volume is still returned, since the caller is 
unmanageVolume() looking for the
+     * very volume it is about to drop from CloudStack.
+     */
+    @Override
+    public List<VolumeOnStorageTO> listVolumesForImport(StoragePool pool, 
String volumePath, String keyword, boolean forUnmanage) {
+        
StorPoolUtil.spLog("StorPoolPrimaryDataStoreDriver.listVolumesForImport: 
pool=%s, volumePath=%s, keyword=%s, forUnmanage=%s", pool.getName(), 
volumePath, keyword, forUnmanage);
+        SpConnectionDesc conn = StorPoolUtil.getSpConnection(pool.getUuid(), 
pool.getId(), storagePoolDetailsDao, primaryStoreDao);
+        List<VolumeOnStorageTO> volumes = new ArrayList<>();
+
+        if (StringUtils.isNotBlank(volumePath)) {
+            return getVolumeOnStorageByPath(pool, volumePath, conn, volumes, 
forUnmanage);
+        }
+
+        SpApiResponse resp = StorPoolUtil.volumesListResponse(conn);
+        if (resp.getError() != null) {
+            
StorPoolUtil.spLog("StorPoolPrimaryDataStoreDriver.listVolumesForImport: could 
not list volumes for pool=%s. Error: %s", pool.getName(), resp.getError());
+            return volumes;
+        }
+        return getVolumeOnStorage(pool, keyword, resp, volumes);
+    }
+
+    @NotNull
+    private List<VolumeOnStorageTO> getVolumeOnStorage(StoragePool pool, 
String keyword, SpApiResponse resp, List<VolumeOnStorageTO> volumes) {
+        JsonArray volumesList = 
resp.fullJson.getAsJsonObject().getAsJsonArray("data");
+
+        for (int i = 0; i < volumesList.size(); i++) {
+            JsonObject volume = volumesList.get(i).getAsJsonObject();
+            String rawName = volume.get("name").getAsString();
+            if (isVolumeDeleted(volume) || !isEligibleForBulkImport(volume, 
rawName)) {
+                continue;
+            }
+
+            String name = rawName.substring(1);
+
+            if (StringUtils.isNotBlank(keyword) && !name.contains(keyword)) {
+                continue;
+            }
+            volumes.add(toVolumeOnStorageTO(volume, name));
+        }
+        
StorPoolUtil.spLog("StorPoolPrimaryDataStoreDriver.listVolumesForImport: found 
%s volume(s) for pool=%s", volumes.size(), pool.getName());
+        return volumes;
+    }
+
+    @NotNull
+    private List<VolumeOnStorageTO> getVolumeOnStorageByPath(StoragePool pool, 
String volumePath, SpConnectionDesc conn, List<VolumeOnStorageTO> volumes, 
boolean forUnmanage) {
+        String name = StorPoolStorageAdaptor.getVolumeNameFromPath(volumePath, 
true);
+        SpApiResponse resp = StorPoolUtil.volumeDescribeResponse(name, conn);
+        if (resp.getError() != null) {
+            
StorPoolUtil.spLog("StorPoolPrimaryDataStoreDriver.listVolumesForImport: could 
not describe volume=%s on pool=%s. Error: %s", name, pool.getName(), 
resp.getError());
+            return volumes;
+        }
+        JsonArray data = 
resp.fullJson.getAsJsonObject().getAsJsonArray("data");
+        if (data == null || data.isEmpty()) {
+            return volumes;
+        }
+        JsonObject volume = data.get(0).getAsJsonObject();
+        String rawName = volume.get("name").getAsString();
+        if (isVolumeDeleted(volume) || !isGlobalIdVolumeName(rawName) || 
(!forUnmanage && isTaggedCs(volume))) {
+            return volumes;
+        }
+        volumes.add(toVolumeOnStorageTO(volume, rawName.substring(1)));
+        return volumes;
+    }
+
+    private static boolean isVolumeDeleted(JsonObject volume) {
+        return volume.has("deleted") && volume.get("deleted").getAsBoolean();
+    }
+
+    private static boolean isGlobalIdVolumeName(String rawName) {
+        return !rawName.startsWith("*") && !rawName.contains("@") && 
rawName.startsWith("~");
+    }
+
+    private static boolean isTaggedCs(JsonObject volume) {
+        JsonObject tags = volume.has("tags") && 
!volume.get("tags").isJsonNull() ? volume.getAsJsonObject("tags") : null;
+        return tags != null && tags.has("cs");
+    }
+
+    private static boolean isEligibleForBulkImport(JsonObject volume, String 
rawName) {
+        return isGlobalIdVolumeName(rawName) && !isTaggedCs(volume);
+    }
+
+    private VolumeOnStorageTO toVolumeOnStorageTO(JsonObject volume, String 
name) {
+        String path = StorPoolUtil.devPath(name);
+        long size = volume.get("size").getAsLong();
+        long spaceUsed = volume.has("spaceUsed") ? 
volume.get("spaceUsed").getAsLong() : size;
+        StorPoolUtil.spLog("listVolumesForImport: found volume=%s, path=%s, 
size=%s, spaceUsed=%s", name, path, size, spaceUsed);
+        return new VolumeOnStorageTO(Hypervisor.HypervisorType.KVM, path, 
name, path,
+                Storage.ImageFormat.QCOW2.toString(), spaceUsed, size);
+    }
+
+    @Override
+    public void volumeManagementChanged(StoragePool pool, Volume volume, 
boolean imported) {
+        
StorPoolUtil.spLog("StorPoolPrimaryDataStoreDriver.volumeManagementChanged: 
pool=%s, volume=%s, imported=%s", pool.getName(), volume.getPath(), imported);
+        SpConnectionDesc conn = StorPoolUtil.getSpConnection(pool.getUuid(), 
pool.getId(), storagePoolDetailsDao, primaryStoreDao);
+        String name = 
StorPoolStorageAdaptor.getVolumeNameFromPath(volume.getPath(), true);
+        Map<String, String> tags;
+        if (imported) {
+            String tier = volume.getDiskOfferingId() != null ? 
getTierFromOfferingDetail(volume.getDiskOfferingId()) : null;
+            Long instanceId = volume.getInstanceId();
+            tags = StorPoolHelper.addStorPoolTags(volume.getUuid(), 
getVMInstanceUUID(instanceId), "volume", getVcPolicyTag(instanceId), tier);
+        } else {
+            tags = StorPoolHelper.addStorPoolTags(null, null, "", null, null);

Review Comment:
   that covers it



##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckVolumeCommandWrapper.java:
##########
@@ -63,27 +71,37 @@ public Answer execute(final CheckVolumeCommand command, 
final LibvirtComputingRe
 
         try {
             if 
(STORAGE_POOL_TYPES_SUPPORTED.contains(storageFilerTO.getType())) {
-                final KVMPhysicalDisk vol = pool.getPhysicalDisk(srcFile);
-                final String path = vol.getPath();
-                final boolean isRbd = 
Storage.StoragePoolType.RBD.equals(storageFilerTO.getType());
-
-                Map<VolumeOnStorageTO.Detail, String> volumeDetails = 
getVolumeDetails(pool, vol);
-                if (MapUtils.isEmpty(volumeDetails)) {
-                    return new Answer(command, false, "Unable to read the 
volume on the storage pool");
+                final boolean shouldAttachTheVolume = 
POOL_NEEDS_TO_EXPOSE_THE_VOLUME.contains(storageFilerTO.getType());
+                if (shouldAttachTheVolume && 
!pool.connectPhysicalDisk(srcFile, new HashMap<>())) {
+                    return new Answer(command, false, "Unable to attach the 
volume on the storage pool");
                 }
+                try {
+                    final KVMPhysicalDisk vol = pool.getPhysicalDisk(srcFile);
+                    final String path = vol.getPath();
+                    final boolean isRawBlockPool = 
RAW_BLOCK_STORAGE_POOL_TYPES.contains(storageFilerTO.getType());
+
+                    Map<VolumeOnStorageTO.Detail, String> volumeDetails = 
getVolumeDetails(pool, vol);
+                    if (MapUtils.isEmpty(volumeDetails)) {
+                        return new Answer(command, false, "Unable to read the 
volume on the storage pool");
+                    }
 
-                if (!isRbd) {
-                    try {
-                        KVMPhysicalDisk.checkQcow2File(path);
-                    } catch (final CloudRuntimeException e) {
-                        return new CheckVolumeAnswer(command, false, "", 0, 
volumeDetails);
+                    if (!isRawBlockPool) {
+                        try {
+                            KVMPhysicalDisk.checkQcow2File(path);
+                        } catch (final CloudRuntimeException e) {
+                            return new CheckVolumeAnswer(command, false, "", 
0, volumeDetails);
+                        }
                     }
-                }
 
-                // Images on RBD are raw and the path is an image name that 
qemu-img cannot open
-                // without the rbd: URI, so take the size libvirt already 
reported for the volume.
-                long size = isRbd ? vol.getVirtualSize() : 
KVMPhysicalDisk.getVirtualSizeFromFile(path);
-                return new CheckVolumeAnswer(command, true, "", size, 
volumeDetails);
+                    // Images on RBD are raw and the path is an image name 
that qemu-img cannot open
+                    // without the rbd: URI, so take the size libvirt already 
reported for the volume
+                    long size = isRawBlockPool ? vol.getVirtualSize() : 
KVMPhysicalDisk.getVirtualSizeFromFile(path);
+                    return new CheckVolumeAnswer(command, true, "", size, 
volumeDetails);
+                } finally {
+                    if (shouldAttachTheVolume) {
+                        pool.disconnectPhysicalDisk(srcFile);

Review Comment:
   that covers 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]

Reply via email to