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 2022/03/21 13:21:14 UTC
[GitHub] [cloudstack] nvazquez commented on a change in pull request #6147: kvm: support multiple local storage pools
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org