nvazquez commented on a change in pull request #6147:
URL: https://github.com/apache/cloudstack/pull/6147#discussion_r831095329



##########
File path: 
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -1008,6 +1013,36 @@ public boolean configure(final String name, final 
Map<String, Object> params) th
         if (_localStoragePath == null) {
             _localStoragePath = "/var/lib/libvirt/images/";
         }
+        _localStorageUUID = (String)params.get("local.storage.uuid");
+        if (_localStorageUUID == null) {
+            _localStorageUUID = UUID.randomUUID().toString();
+        }
+
+        String[] localStorageRelativePaths = 
_localStoragePath.split(CONFIG_VALUES_SEPARATOR);
+        String[] localStorageUUIDs = 
_localStorageUUID.split(CONFIG_VALUES_SEPARATOR, -1);
+        if (localStorageRelativePaths.length != localStorageUUIDs.length) {
+            throw new ConfigurationException("The path and UUID of local 
storage pools have different length");

Review comment:
       I think that instead of killing the agent we can log the error and 
continue with at least the default local storage and UUID

##########
File path: 
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -1008,6 +1013,36 @@ public boolean configure(final String name, final 
Map<String, Object> params) th
         if (_localStoragePath == null) {
             _localStoragePath = "/var/lib/libvirt/images/";
         }
+        _localStorageUUID = (String)params.get("local.storage.uuid");
+        if (_localStorageUUID == null) {
+            _localStorageUUID = UUID.randomUUID().toString();
+        }
+
+        String[] localStorageRelativePaths = 
_localStoragePath.split(CONFIG_VALUES_SEPARATOR);
+        String[] localStorageUUIDs = 
_localStorageUUID.split(CONFIG_VALUES_SEPARATOR, -1);
+        if (localStorageRelativePaths.length != localStorageUUIDs.length) {
+            throw new ConfigurationException("The path and UUID of local 
storage pools have different length");
+        }
+        for (String localStorageRelativePath : localStorageRelativePaths) {
+            final File storagePath = new File(localStorageRelativePath);
+            _localStoragePaths.add(storagePath.getAbsolutePath());
+        }
+        _localStoragePath = StringUtils.join(_localStoragePaths, 
CONFIG_VALUES_SEPARATOR);
+        params.put("local.storage.path", _localStoragePath);
+
+        for (String localStorageUUID : localStorageUUIDs) {
+            if (StringUtils.isBlank(localStorageUUID)) {

Review comment:
       This could be a new method for UUID validation

##########
File path: 
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -1008,6 +1013,36 @@ public boolean configure(final String name, final 
Map<String, Object> params) th
         if (_localStoragePath == null) {
             _localStoragePath = "/var/lib/libvirt/images/";
         }
+        _localStorageUUID = (String)params.get("local.storage.uuid");
+        if (_localStorageUUID == null) {
+            _localStorageUUID = UUID.randomUUID().toString();
+        }
+
+        String[] localStorageRelativePaths = 
_localStoragePath.split(CONFIG_VALUES_SEPARATOR);
+        String[] localStorageUUIDs = 
_localStorageUUID.split(CONFIG_VALUES_SEPARATOR, -1);
+        if (localStorageRelativePaths.length != localStorageUUIDs.length) {
+            throw new ConfigurationException("The path and UUID of local 
storage pools have different length");
+        }
+        for (String localStorageRelativePath : localStorageRelativePaths) {
+            final File storagePath = new File(localStorageRelativePath);
+            _localStoragePaths.add(storagePath.getAbsolutePath());
+        }
+        _localStoragePath = StringUtils.join(_localStoragePaths, 
CONFIG_VALUES_SEPARATOR);
+        params.put("local.storage.path", _localStoragePath);
+
+        for (String localStorageUUID : localStorageUUIDs) {

Review comment:
       If for example: localStoragePath contains 3 paths and localStorageUUIDs 
contains 2 UUIDS, should we always fail or try to map the maximum pairs? In 
this example: mapping the first 2 paths with the 2 UUIDs?




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