From c4cdd72b9e36b645288043c3406a825e38bdfd11 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Mon, 21 Mar 2022 13:45:55 +0100 Subject: [PATCH 1/5] kvm: support multiple local storage pools --- .../resource/LibvirtComputingResource.java | 75 ++++++++++++++----- 1 file changed, 58 insertions(+), 17 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 164fabce17ae..ff3aaa03edb7 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -215,6 +215,8 @@ public class LibvirtComputingResource extends ServerResourceBase implements ServerResource, VirtualRouterDeployer { private static final Logger s_logger = Logger.getLogger(LibvirtComputingResource.class); + private static final String CONFIG_VALUES_SEPARATOR = ","; + private static final String LEGACY = "legacy"; private static final String SECURE = "secure"; @@ -382,6 +384,9 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv protected String _agentHooksVmOnStopScript = "libvirt-vm-state-change.groovy"; protected String _agentHooksVmOnStopMethod = "onStop"; + protected List _localStoragePaths = new ArrayList<>(); + protected List _localStorageUUIDs = new ArrayList<>(); + private static final String CONFIG_DRIVE_ISO_DISK_LABEL = "hdd"; private static final int CONFIG_DRIVE_ISO_DEVICE_ID = 4; @@ -1008,6 +1013,36 @@ public boolean configure(final String name, final Map 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)) { + throw new ConfigurationException("The UUID of local storage pools must be non-blank"); + } + try { + UUID.fromString(localStorageUUID); + } catch (IllegalArgumentException ex) { + throw new ConfigurationException("The UUID of local storage pool is invalid : " + localStorageUUID); + } + _localStorageUUIDs.add(localStorageUUID); + } + _localStorageUUID = StringUtils.join(_localStorageUUIDs, CONFIG_VALUES_SEPARATOR); + params.put("local.storage.uuid", _localStorageUUID); /* Directory to use for Qemu sockets like for the Qemu Guest Agent */ _qemuSocketsPath = new File("/var/lib/libvirt/qemu"); @@ -1016,14 +1051,6 @@ public boolean configure(final String name, final Map params) th _qemuSocketsPath = new File(_qemuSocketsPathVar); } - final File storagePath = new File(_localStoragePath); - _localStoragePath = storagePath.getAbsolutePath(); - - _localStorageUUID = (String)params.get("local.storage.uuid"); - if (_localStorageUUID == null) { - _localStorageUUID = UUID.randomUUID().toString(); - } - value = (String)params.get("scripts.timeout"); _timeout = Duration.standardSeconds(NumbersUtil.parseInt(value, 30 * 60)); @@ -3285,12 +3312,31 @@ public StartupCommand[] initialize() { _hostDistro = cmd.getHostDetails().get("Host.OS"); } + List startupCommands = new ArrayList<>(); + startupCommands.add(cmd); + for (int i = 0; i < _localStoragePaths.size(); i++) { + String localStoragePath = _localStoragePaths.get(i); + String localStorageUUID = _localStorageUUIDs.get(i); + StartupStorageCommand sscmd = createLocalStoragePool(localStoragePath, localStorageUUID, cmd); + if (sscmd != null) { + startupCommands.add(sscmd); + } + } + StartupCommand[] startupCommandsArray = new StartupCommand[startupCommands.size()]; + int i = 0; + for (StartupCommand startupCommand : startupCommands) { + startupCommandsArray[i] = startupCommand; + i++; + } + return startupCommandsArray; + } + + private StartupStorageCommand createLocalStoragePool(String localStoragePath, String localStorageUUID, StartupRoutingCommand cmd) { StartupStorageCommand sscmd = null; try { - - final KVMStoragePool localStoragePool = _storagePoolMgr.createStoragePool(_localStorageUUID, "localhost", -1, _localStoragePath, "", StoragePoolType.Filesystem); + final KVMStoragePool localStoragePool = _storagePoolMgr.createStoragePool(localStorageUUID, "localhost", -1, localStoragePath, "", StoragePoolType.Filesystem); final com.cloud.agent.api.StoragePoolInfo pi = - new com.cloud.agent.api.StoragePoolInfo(localStoragePool.getUuid(), cmd.getPrivateIpAddress(), _localStoragePath, _localStoragePath, + new com.cloud.agent.api.StoragePoolInfo(localStoragePool.getUuid(), cmd.getPrivateIpAddress(), localStoragePath, localStoragePath, StoragePoolType.Filesystem, localStoragePool.getCapacity(), localStoragePool.getAvailable()); sscmd = new StartupStorageCommand(); @@ -3301,12 +3347,7 @@ public StartupCommand[] initialize() { } catch (final CloudRuntimeException e) { s_logger.debug("Unable to initialize local storage pool: " + e); } - - if (sscmd != null) { - return new StartupCommand[] {cmd, sscmd}; - } else { - return new StartupCommand[] {cmd}; - } + return sscmd; } public String diskUuidToSerial(String uuid) { From 8178d33cb68d044f43b812efc9db1240a71c8262 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Mon, 21 Mar 2022 15:08:23 +0100 Subject: [PATCH 2/5] Update #6147: add method configureLocalStorage --- .../resource/LibvirtComputingResource.java | 70 ++++++++++--------- 1 file changed, 36 insertions(+), 34 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index ff3aaa03edb7..26f0cdceb3ec 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -1009,40 +1009,7 @@ public boolean configure(final String name, final Map params) th } } - _localStoragePath = (String)params.get("local.storage.path"); - 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)) { - throw new ConfigurationException("The UUID of local storage pools must be non-blank"); - } - try { - UUID.fromString(localStorageUUID); - } catch (IllegalArgumentException ex) { - throw new ConfigurationException("The UUID of local storage pool is invalid : " + localStorageUUID); - } - _localStorageUUIDs.add(localStorageUUID); - } - _localStorageUUID = StringUtils.join(_localStorageUUIDs, CONFIG_VALUES_SEPARATOR); - params.put("local.storage.uuid", _localStorageUUID); + configureLocalStorage(params); /* Directory to use for Qemu sockets like for the Qemu Guest Agent */ _qemuSocketsPath = new File("/var/lib/libvirt/qemu"); @@ -1315,6 +1282,41 @@ public boolean configure(final String name, final Map params) th return true; } + private void configureLocalStorage(final Map params) throws ConfigurationException { + _localStoragePath = (String)params.get("local.storage.path"); + 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); + 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); + + for (String localStorageUUID : localStorageUUIDs) { + if (StringUtils.isBlank(localStorageUUID)) { + throw new ConfigurationException("The UUID of local storage pools must be non-blank"); + } + try { + UUID.fromString(localStorageUUID); + } catch (IllegalArgumentException ex) { + throw new ConfigurationException("The UUID of local storage pool is invalid : " + localStorageUUID); + } + _localStorageUUIDs.add(localStorageUUID); + } + _localStorageUUID = StringUtils.join(_localStorageUUIDs, CONFIG_VALUES_SEPARATOR); + } + public boolean configureHostParams(final Map params) { final File file = PropertiesUtil.findConfigFile("agent.properties"); if (file == null) { From e58b673445cbf07eeffca3c1f8670c51ee5439be Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Mon, 21 Mar 2022 15:12:46 +0100 Subject: [PATCH 3/5] Update #6147: add method validateLocalStorageUUID --- .../resource/LibvirtComputingResource.java | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 26f0cdceb3ec..3ba426545174 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -1304,19 +1304,23 @@ private void configureLocalStorage(final Map params) throws Conf _localStoragePath = StringUtils.join(_localStoragePaths, CONFIG_VALUES_SEPARATOR); for (String localStorageUUID : localStorageUUIDs) { - if (StringUtils.isBlank(localStorageUUID)) { - throw new ConfigurationException("The UUID of local storage pools must be non-blank"); - } - try { - UUID.fromString(localStorageUUID); - } catch (IllegalArgumentException ex) { - throw new ConfigurationException("The UUID of local storage pool is invalid : " + localStorageUUID); - } + validateLocalStorageUUID(localStorageUUID); _localStorageUUIDs.add(localStorageUUID); } _localStorageUUID = StringUtils.join(_localStorageUUIDs, CONFIG_VALUES_SEPARATOR); } + private void validateLocalStorageUUID(String localStorageUUID) throws ConfigurationException { + if (StringUtils.isBlank(localStorageUUID)) { + throw new ConfigurationException("The UUID of local storage pools must be non-blank"); + } + try { + UUID.fromString(localStorageUUID); + } catch (IllegalArgumentException ex) { + throw new ConfigurationException("The UUID of local storage pool is invalid : " + localStorageUUID); + } + } + public boolean configureHostParams(final Map params) { final File file = PropertiesUtil.findConfigFile("agent.properties"); if (file == null) { From c9b6286d895e385658ade641d012d5b7cf2baa14 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 23 Mar 2022 09:23:58 +0100 Subject: [PATCH 4/5] Update #6147: remove underscore --- .../resource/LibvirtComputingResource.java | 38 +++++++++---------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 3ba426545174..794cba58d294 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -349,8 +349,6 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv protected String _pool; protected String _localGateway; private boolean _canBridgeFirewall; - protected String _localStoragePath; - protected String _localStorageUUID; protected boolean _noMemBalloon = false; protected String _guestCpuArch; protected String _guestCpuMode; @@ -384,8 +382,8 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv protected String _agentHooksVmOnStopScript = "libvirt-vm-state-change.groovy"; protected String _agentHooksVmOnStopMethod = "onStop"; - protected List _localStoragePaths = new ArrayList<>(); - protected List _localStorageUUIDs = new ArrayList<>(); + protected List localStoragePaths = new ArrayList<>(); + protected List localStorageUUIDs = new ArrayList<>(); private static final String CONFIG_DRIVE_ISO_DISK_LABEL = "hdd"; private static final int CONFIG_DRIVE_ISO_DEVICE_ID = 4; @@ -1283,31 +1281,29 @@ public boolean configure(final String name, final Map params) th } private void configureLocalStorage(final Map params) throws ConfigurationException { - _localStoragePath = (String)params.get("local.storage.path"); - if (_localStoragePath == null) { - _localStoragePath = "/var/lib/libvirt/images/"; + String localStoragePath = (String)params.get("local.storage.path"); + if (localStoragePath == null) { + localStoragePath = "/var/lib/libvirt/images/"; } - _localStorageUUID = (String)params.get("local.storage.uuid"); - if (_localStorageUUID == null) { - _localStorageUUID = UUID.randomUUID().toString(); + String localStorageUUIDString = (String)params.get("local.storage.uuid"); + if (localStorageUUIDString == null) { + localStorageUUIDString = UUID.randomUUID().toString(); } - String[] localStorageRelativePaths = _localStoragePath.split(CONFIG_VALUES_SEPARATOR); - String[] localStorageUUIDs = _localStorageUUID.split(CONFIG_VALUES_SEPARATOR); - if (localStorageRelativePaths.length != localStorageUUIDs.length) { + String[] localStorageRelativePaths = localStoragePath.split(CONFIG_VALUES_SEPARATOR); + String[] localStorageUUIDStrings = localStorageUUIDString.split(CONFIG_VALUES_SEPARATOR); + if (localStorageRelativePaths.length != localStorageUUIDStrings.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()); + localStoragePaths.add(storagePath.getAbsolutePath()); } - _localStoragePath = StringUtils.join(_localStoragePaths, CONFIG_VALUES_SEPARATOR); - for (String localStorageUUID : localStorageUUIDs) { + for (String localStorageUUID : localStorageUUIDStrings) { validateLocalStorageUUID(localStorageUUID); - _localStorageUUIDs.add(localStorageUUID); + localStorageUUIDs.add(localStorageUUID); } - _localStorageUUID = StringUtils.join(_localStorageUUIDs, CONFIG_VALUES_SEPARATOR); } private void validateLocalStorageUUID(String localStorageUUID) throws ConfigurationException { @@ -3320,9 +3316,9 @@ public StartupCommand[] initialize() { List startupCommands = new ArrayList<>(); startupCommands.add(cmd); - for (int i = 0; i < _localStoragePaths.size(); i++) { - String localStoragePath = _localStoragePaths.get(i); - String localStorageUUID = _localStorageUUIDs.get(i); + for (int i = 0; i < localStoragePaths.size(); i++) { + String localStoragePath = localStoragePaths.get(i); + String localStorageUUID = localStorageUUIDs.get(i); StartupStorageCommand sscmd = createLocalStoragePool(localStoragePath, localStorageUUID, cmd); if (sscmd != null) { startupCommands.add(sscmd); From a4e6c270414bfb64c893b5c6fc976cbd70285b5e Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Thu, 24 Mar 2022 11:34:06 +0100 Subject: [PATCH 5/5] Update #6147: add unit tests --- .../resource/LibvirtComputingResource.java | 12 ++++--- .../LibvirtComputingResourceTest.java | 36 +++++++++++++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 794cba58d294..c1e1eedffb15 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -382,6 +382,10 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv protected String _agentHooksVmOnStopScript = "libvirt-vm-state-change.groovy"; protected String _agentHooksVmOnStopMethod = "onStop"; + protected static final String LOCAL_STORAGE_PATH = "local.storage.path"; + protected static final String LOCAL_STORAGE_UUID = "local.storage.uuid"; + protected static final String DEFAULT_LOCAL_STORAGE_PATH = "/var/lib/libvirt/images/"; + protected List localStoragePaths = new ArrayList<>(); protected List localStorageUUIDs = new ArrayList<>(); @@ -1280,12 +1284,12 @@ public boolean configure(final String name, final Map params) th return true; } - private void configureLocalStorage(final Map params) throws ConfigurationException { - String localStoragePath = (String)params.get("local.storage.path"); + protected void configureLocalStorage(final Map params) throws ConfigurationException { + String localStoragePath = (String)params.get(LOCAL_STORAGE_PATH); if (localStoragePath == null) { - localStoragePath = "/var/lib/libvirt/images/"; + localStoragePath = DEFAULT_LOCAL_STORAGE_PATH; } - String localStorageUUIDString = (String)params.get("local.storage.uuid"); + String localStorageUUIDString = (String)params.get(LOCAL_STORAGE_UUID); if (localStorageUUIDString == null) { localStorageUUIDString = UUID.randomUUID().toString(); } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java index 43d12efbc4f0..03c450b4b959 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java @@ -5841,4 +5841,40 @@ public void setCpuSharesTestSuccessfullySetCpuShares() throws LibvirtException { return true; })); } + + private void configLocalStorageTests(Map params) throws ConfigurationException { + LibvirtComputingResource libvirtComputingResourceSpy = Mockito.spy(new LibvirtComputingResource()); + libvirtComputingResourceSpy.configureLocalStorage(params); + } + + @Test + public void testConfigureLocalStorageWithEmptyParams() throws ConfigurationException { + Map params = new HashMap<>(); + configLocalStorageTests(params); + } + + @Test + public void testConfigureLocalStorageWithMultiplePaths() throws ConfigurationException { + Map params = new HashMap<>(); + params.put(LibvirtComputingResource.LOCAL_STORAGE_PATH, "/var/lib/libvirt/images/,/var/lib/libvirt/images2/"); + params.put(LibvirtComputingResource.LOCAL_STORAGE_UUID, UUID.randomUUID().toString() + "," + UUID.randomUUID().toString()); + configLocalStorageTests(params); + } + + @Test(expected = ConfigurationException.class) + public void testConfigureLocalStorageWithDifferentLength() throws ConfigurationException { + Map params = new HashMap<>(); + params.put(LibvirtComputingResource.LOCAL_STORAGE_PATH, "/var/lib/libvirt/images/,/var/lib/libvirt/images2/"); + params.put(LibvirtComputingResource.LOCAL_STORAGE_UUID, UUID.randomUUID().toString()); + configLocalStorageTests(params); + } + + @Test(expected = ConfigurationException.class) + public void testConfigureLocalStorageWithInvalidUUID() throws ConfigurationException { + Map params = new HashMap<>(); + params.put(LibvirtComputingResource.LOCAL_STORAGE_PATH, "/var/lib/libvirt/images/"); + params.put(LibvirtComputingResource.LOCAL_STORAGE_UUID, "111111"); + configLocalStorageTests(params); + } + }