You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/03/15 16:09:22 UTC
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4816: xenserver: retrieve correct name-label for presetup store
DaanHoogland commented on a change in pull request #4816:
URL: https://github.com/apache/cloudstack/pull/4816#discussion_r594469191
##########
File path: plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java
##########
@@ -96,6 +96,14 @@ public XenServerStorageProcessor(final CitrixResourceBase resource) {
hypervisorResource = resource;
}
+ protected String getSRNameLabel(final PrimaryDataStoreTO primaryStore) {
+ if (Storage.StoragePoolType.PreSetup.equals(primaryStore.getPoolType()) &&
+ !primaryStore.getPath().contains(primaryStore.getUuid())) {
+ return primaryStore.getPath().replace("/", "");
Review comment:
as above. Make sure it is only the last one being replaced.
##########
File path: plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixHelper.java
##########
@@ -44,4 +46,12 @@ public static String getPVbootloaderArgs(String guestOS) {
}
return "";
}
+
+ public static String getSRNameLabel(final StorageFilerTO primaryStore) {
+ if (Storage.StoragePoolType.PreSetup.equals(primaryStore.getType()) &&
+ !primaryStore.getPath().contains(primaryStore.getUuid())) {
+ return primaryStore.getPath().replace("/", "");
Review comment:
doesn't this replace all '/'? seems dangerous to me
##########
File path: plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java
##########
@@ -96,6 +96,14 @@ public XenServerStorageProcessor(final CitrixResourceBase resource) {
hypervisorResource = resource;
}
+ protected String getSRNameLabel(final PrimaryDataStoreTO primaryStore) {
+ if (Storage.StoragePoolType.PreSetup.equals(primaryStore.getPoolType()) &&
+ !primaryStore.getPath().contains(primaryStore.getUuid())) {
+ return primaryStore.getPath().replace("/", "");
Review comment:
also why is this method double implemented? here and in CitrixHelper? <quote style='ominous'>there can be only one</quote>
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org