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


##########
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:
   Thanks @Damans227, for the review and good catch - passing null here left 
uuid/qc/cvm untouched instead of clearing them, since addStorPoolTags only 
sends a key when its value isn't null. Fixed to pass "" for all of them 
(cs/uuid/cvm/qc/vc-policy), so unmanage now clears every tag it set



-- 
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