From ba26d95ad7c8c2f71029da22c4f82fc0fcc7c968 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Tue, 16 Dec 2025 15:48:06 +0100 Subject: [PATCH 01/10] api: create/register/upload template with empty template tag (#12234) --- .../cloudstack/api/command/user/template/CreateTemplateCmd.java | 2 +- .../command/user/template/GetUploadParamsForTemplateCmd.java | 2 +- .../api/command/user/template/RegisterTemplateCmd.java | 2 +- .../java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/template/CreateTemplateCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/template/CreateTemplateCmd.java index 5f09ac6698d..ec5624715b6 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/template/CreateTemplateCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/template/CreateTemplateCmd.java @@ -211,7 +211,7 @@ public class CreateTemplateCmd extends BaseAsyncCreateCmd implements UserCmd { } public String getTemplateTag() { - return templateTag; + return StringUtils.isBlank(templateTag) ? null : templateTag; } public Map getDetails() { diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/template/GetUploadParamsForTemplateCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/template/GetUploadParamsForTemplateCmd.java index 8fa1a5d53eb..a4596c04a92 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/template/GetUploadParamsForTemplateCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/template/GetUploadParamsForTemplateCmd.java @@ -160,7 +160,7 @@ public class GetUploadParamsForTemplateCmd extends AbstractGetUploadParamsCmd { } public String getTemplateTag() { - return templateTag; + return StringUtils.isBlank(templateTag) ? null : templateTag; } public boolean isDeployAsIs() { diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmd.java index 1f968b869b9..8fe1a93a5a2 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmd.java @@ -265,7 +265,7 @@ public class RegisterTemplateCmd extends BaseCmd implements UserCmd { } public String getTemplateTag() { - return templateTag; + return StringUtils.isBlank(templateTag) ? null : templateTag; } public Map getDetails() { diff --git a/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java b/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java index 22cfe785edf..c11d8e1ae08 100644 --- a/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java +++ b/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java @@ -734,7 +734,7 @@ public class ConsoleProxyManagerImpl extends ManagerBase implements ConsoleProxy logger.debug("Unable to allocate proxy {} with {} in {} due to [{}]. Retrying with another template", proxy, template, dc, e.getMessage(), e); continue; } - throw new CloudRuntimeException("Failed to allocate proxy [%s] in zone [%s] with available templates", e); + throw new CloudRuntimeException(String.format("Failed to allocate proxy [%s] in zone [%s] with available templates", proxy, dc), e); } } From d5165183eae108a8dc9ab1a48d4fb702eae8d815 Mon Sep 17 00:00:00 2001 From: Brad House Date: Tue, 16 Dec 2025 10:58:32 -0500 Subject: [PATCH 02/10] KVM memballooning requires free page reporting and autodeflate (#11932) --- .../hypervisor/kvm/resource/LibvirtVMDef.java | 8 +++++++- .../kvm/resource/LibvirtDomainXMLParserTest.java | 4 ++-- .../hypervisor/kvm/resource/LibvirtVMDefTest.java | 15 +++++++++++++++ .../LibvirtMigrateVolumeCommandWrapperTest.java | 2 +- 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java index bf002b37f35..9b5eb2a6ee8 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java @@ -1340,7 +1340,13 @@ public class LibvirtVMDef { @Override public String toString() { StringBuilder memBalloonBuilder = new StringBuilder(); - memBalloonBuilder.append("\n"); + memBalloonBuilder.append("= 5001000 && s_libvirtVersion >= 6009000) { + memBalloonBuilder.append(" autodeflate='on' freePageReporting='on'"); + } + memBalloonBuilder.append(">\n"); if (StringUtils.isNotBlank(memBalloonStatsPeriod)) { memBalloonBuilder.append("\n"); } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParserTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParserTest.java index de50cf34202..e73b4079866 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParserTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtDomainXMLParserTest.java @@ -196,7 +196,7 @@ public class LibvirtDomainXMLParserTest extends TestCase { "" + "
" + "" + - "" + + "" + "" + "" + "
" + @@ -379,7 +379,7 @@ public class LibvirtDomainXMLParserTest extends TestCase { " \n" + "
\n" + " \n" + - " \n" + + " \n" + "
\n" + " \n" + " \n" + diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java index 856dc0be9dc..56ad267eac7 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java @@ -465,6 +465,21 @@ public class LibvirtVMDefTest extends TestCase { @Test public void memBalloonDefTestVirtio() { + LibvirtVMDef.setGlobalQemuVersion(5001000L); + LibvirtVMDef.setGlobalLibvirtVersion(6009000L); + String expectedXml = "\n\n"; + MemBalloonDef memBalloonDef = new MemBalloonDef(); + memBalloonDef.defVirtioMemBalloon("60"); + + String xmlDef = memBalloonDef.toString(); + + assertEquals(expectedXml, xmlDef); + } + + @Test + public void memBalloonDefTestVirtioOld() { + LibvirtVMDef.setGlobalQemuVersion(2006000L); + LibvirtVMDef.setGlobalLibvirtVersion(9008L); String expectedXml = "\n\n"; MemBalloonDef memBalloonDef = new MemBalloonDef(); memBalloonDef.defVirtioMemBalloon("60"); diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateVolumeCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateVolumeCommandWrapperTest.java index 4f1eba1a772..e3d35dfb4d1 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateVolumeCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateVolumeCommandWrapperTest.java @@ -208,7 +208,7 @@ public class LibvirtMigrateVolumeCommandWrapperTest { " \n" + "
\n" + " \n" + - " \n" + + " \n" + " \n" + "
\n" + " \n" + From de1b1d24c2e62eee155fa3aa53f722ecb01858a0 Mon Sep 17 00:00:00 2001 From: Brad House Date: Tue, 16 Dec 2025 14:07:22 -0500 Subject: [PATCH 03/10] Python exception processing static routes fixed (#11967) --- systemvm/debian/opt/cloud/bin/cs/CsAddress.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/systemvm/debian/opt/cloud/bin/cs/CsAddress.py b/systemvm/debian/opt/cloud/bin/cs/CsAddress.py index 3b4ad3d7472..c7dac4df47e 100755 --- a/systemvm/debian/opt/cloud/bin/cs/CsAddress.py +++ b/systemvm/debian/opt/cloud/bin/cs/CsAddress.py @@ -603,7 +603,7 @@ class CsIP: if item == "id": continue static_route = static_routes.get_bag()[item] - if static_route['ip_address'] == self.address['public_ip'] and not static_route['revoke']: + if 'ip_address' in static_route and static_route['ip_address'] == self.address['public_ip'] and not static_route['revoke']: self.fw.append(["mangle", "", "-A PREROUTING -m state --state NEW -i %s -s %s ! -d %s/32 -j ACL_OUTBOUND_%s" % (self.dev, static_route['network'], static_route['ip_address'], self.dev)]) From e08e66d66deb43ddd7fe6e0185fe7a0fbf94255a Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 17 Dec 2025 13:03:39 +0100 Subject: [PATCH 04/10] kvm: use preallocation option for fat disk resize (#11986) Signed-off-by: Abhishek Kumar --- plugins/hypervisors/kvm/pom.xml | 9 - .../kvm/storage/LibvirtStorageAdaptor.java | 2 +- .../apache/cloudstack/utils/qemu/QemuImg.java | 84 ++++--- .../cloudstack/utils/qemu/QemuImgTest.java | 207 ++++++++++++++++-- 4 files changed, 246 insertions(+), 56 deletions(-) diff --git a/plugins/hypervisors/kvm/pom.xml b/plugins/hypervisors/kvm/pom.xml index 8ed960c6bc0..bc7b6903aae 100644 --- a/plugins/hypervisors/kvm/pom.xml +++ b/plugins/hypervisors/kvm/pom.xml @@ -107,15 +107,6 @@ - - org.apache.maven.plugins - maven-surefire-plugin - - - **/QemuImg*.java - - - diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index 87544cfaa9d..a03daeb197b 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -1081,7 +1081,7 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { destFile.setSize(size); Map options = new HashMap(); if (List.of(StoragePoolType.NetworkFilesystem, StoragePoolType.Filesystem).contains(pool.getType())) { - options.put("preallocation", QemuImg.PreallocationType.getPreallocationType(provisioningType).toString()); + options.put(QemuImg.PREALLOCATION, QemuImg.PreallocationType.getPreallocationType(provisioningType).toString()); } try (KeyFile keyFile = new KeyFile(passphrase)) { diff --git a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java index 0a8ea27cd49..1fec561dc89 100644 --- a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java +++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java @@ -25,6 +25,7 @@ import java.util.Map; import java.util.regex.Pattern; import org.apache.cloudstack.storage.formatinspector.Qcow2Inspector; +import org.apache.commons.collections.MapUtils; import org.apache.commons.lang.NotImplementedException; import org.apache.commons.lang3.StringUtils; import org.libvirt.LibvirtException; @@ -51,6 +52,7 @@ public class QemuImg { public static final String ENCRYPT_FORMAT = "encrypt.format"; public static final String ENCRYPT_KEY_SECRET = "encrypt.key-secret"; public static final String TARGET_ZERO_FLAG = "--target-is-zero"; + public static final String PREALLOCATION = "preallocation"; public static final long QEMU_2_10 = 2010000; public static final long QEMU_5_10 = 5010000; @@ -393,6 +395,17 @@ public class QemuImg { convert(srcFile, destFile, options, qemuObjects, srcImageOpts, snapshotName, forceSourceFormat, false); } + protected Map getResizeOptionsFromConvertOptions(final Map options) { + if (MapUtils.isEmpty(options)) { + return null; + } + Map resizeOpts = new HashMap<>(); + if (options.containsKey(PREALLOCATION)) { + resizeOpts.put(PREALLOCATION, options.get(PREALLOCATION)); + } + return resizeOpts; + } + /** * Converts an image from source to destination. * @@ -420,7 +433,6 @@ public class QemuImg { public void convert(final QemuImgFile srcFile, final QemuImgFile destFile, final Map options, final List qemuObjects, final QemuImageOptions srcImageOpts, final String snapshotName, final boolean forceSourceFormat, boolean keepBitmaps) throws QemuImgException { - Script script = new Script(_qemuImgPath, timeout); if (StringUtils.isNotBlank(snapshotName)) { String qemuPath = Script.runSimpleBashScript(getQemuImgPathScript); @@ -484,7 +496,7 @@ public class QemuImg { } if (srcFile.getSize() < destFile.getSize()) { - this.resize(destFile, destFile.getSize()); + this.resize(destFile, destFile.getSize(), getResizeOptionsFromConvertOptions(options)); } } @@ -691,17 +703,29 @@ public class QemuImg { } } - private void addScriptOptionsFromMap(Map options, Script s) { - if (options != null && !options.isEmpty()) { - s.add("-o"); - final StringBuffer optionsBuffer = new StringBuffer(); - for (final Map.Entry option : options.entrySet()) { - optionsBuffer.append(option.getKey()).append('=').append(option.getValue()).append(','); - } - String optionsStr = optionsBuffer.toString(); - optionsStr = optionsStr.replaceAll(",$", ""); - s.add(optionsStr); + protected void addScriptOptionsFromMap(Map options, Script s) { + if (MapUtils.isEmpty(options)) { + return; } + s.add("-o"); + final StringBuffer optionsBuffer = new StringBuffer(); + for (final Map.Entry option : options.entrySet()) { + optionsBuffer.append(option.getKey()).append('=').append(option.getValue()).append(','); + } + String optionsStr = optionsBuffer.toString(); + optionsStr = optionsStr.replaceAll(",$", ""); + s.add(optionsStr); + } + + protected void addScriptResizeOptionsFromMap(Map options, Script s) { + if (MapUtils.isEmpty(options)) { + return; + } + if (options.containsKey(PREALLOCATION)) { + s.add(String.format("--%s=%s", PREALLOCATION, options.get(PREALLOCATION))); + options.remove(PREALLOCATION); + } + addScriptOptionsFromMap(options, s); } /** @@ -747,19 +771,17 @@ public class QemuImg { /** * Resizes an image. - * + *

* This method is a facade for 'qemu-img resize'. - * + *

* A negative size value will get prefixed with '-' and a positive with '+'. Sizes are in bytes and will be passed on that way. * - * @param file - * The file to be resized. - * @param size - * The new size. - * @param delta - * Flag to inform if the new size is a delta. + * @param file The file to be resized. + * @param size The new size. + * @param delta Flag to inform if the new size is a delta. + * @param options Script options for resizing. Takes a Map with key value */ - public void resize(final QemuImgFile file, final long size, final boolean delta) throws QemuImgException { + public void resize(final QemuImgFile file, final long size, final boolean delta, Map options) throws QemuImgException { String newSize = null; if (size == 0) { @@ -781,6 +803,7 @@ public class QemuImg { final Script s = new Script(_qemuImgPath); s.add("resize"); + addScriptResizeOptionsFromMap(options, s); s.add(file.getFileName()); s.add(newSize); s.execute(); @@ -789,7 +812,7 @@ public class QemuImg { /** * Resizes an image. * - * This method is a facade for {@link QemuImg#resize(QemuImgFile, long, boolean)}. + * This method is a facade for {@link QemuImg#resize(QemuImgFile, long, boolean, Map)}. * * A negative size value will get prefixed with - and a positive with +. Sizes are in bytes and will be passed on that way. * @@ -818,18 +841,17 @@ public class QemuImg { /** * Resizes an image. - * - * This method is a facade for {@link QemuImg#resize(QemuImgFile, long, boolean)}. - * + *

+ * This method is a facade for {@link QemuImg#resize(QemuImgFile, long, boolean, Map)}. + *

* A negative size value will get prefixed with - and a positive with +. Sizes are in bytes and will be passed on that way. * - * @param file - * The file to be resized. - * @param size - * The new size. + * @param file The file to be resized. + * @param size The new size. + * @param options Script options for resizing. Takes a Map with key value */ - public void resize(final QemuImgFile file, final long size) throws QemuImgException { - this.resize(file, size, false); + public void resize(final QemuImgFile file, final long size, Map options) throws QemuImgException { + this.resize(file, size, false, options); } /** diff --git a/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/qemu/QemuImgTest.java b/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/qemu/QemuImgTest.java index b0981dde26e..5a027425776 100644 --- a/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/qemu/QemuImgTest.java +++ b/plugins/hypervisors/kvm/src/test/java/org/apache/cloudstack/utils/qemu/QemuImgTest.java @@ -22,26 +22,45 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.io.File; -import com.cloud.utils.script.Script; - import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.UUID; +import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat; +import org.apache.commons.collections.MapUtils; import org.junit.Assert; +import org.junit.Assume; +import org.junit.BeforeClass; import org.junit.Ignore; import org.junit.Test; - -import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat; +import org.junit.runner.RunWith; +import org.libvirt.Connect; import org.libvirt.LibvirtException; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; -@Ignore +import com.cloud.utils.script.Script; + +@RunWith(MockitoJUnitRunner.class) public class QemuImgTest { + @BeforeClass + public static void setUp() { + Assume.assumeTrue("qemu-img not found", Script.runSimpleBashScript("command -v qemu-img") != null); + boolean libVirtAvailable = false; + try { + Connect conn = new Connect("qemu:///system", false); + conn.getVersion(); + libVirtAvailable = true; + } catch (LibvirtException ignored) {} + Assume.assumeTrue("libvirt not available", libVirtAvailable); + } + @Test public void testCreateAndInfo() throws QemuImgException, LibvirtException { String filename = "/tmp/" + UUID.randomUUID() + ".qcow2"; @@ -130,8 +149,7 @@ public class QemuImgTest { public void testCreateSparseVolume() throws QemuImgException, LibvirtException { String filename = "/tmp/" + UUID.randomUUID() + ".qcow2"; - /* 10TB virtual_size */ - long size = 10995116277760l; + long size = 10 * 1024 * 1024L; QemuImgFile file = new QemuImgFile(filename, size, PhysicalDiskFormat.QCOW2); String preallocation = "metadata"; Map options = new HashMap(); @@ -141,8 +159,8 @@ public class QemuImgTest { QemuImg qemu = new QemuImg(0); qemu.create(file, options); - String allocatedSize = Script.runSimpleBashScript(String.format("ls -alhs %s | awk '{print $1}'", file)); - String declaredSize = Script.runSimpleBashScript(String.format("ls -alhs %s | awk '{print $6}'", file)); + String allocatedSize = Script.runSimpleBashScript(String.format("ls -alhs %s | awk '{print $1}'", filename)); + String declaredSize = Script.runSimpleBashScript(String.format("ls -alhs %s | awk '{print $6}'", filename)); assertFalse(allocatedSize.equals(declaredSize)); @@ -162,7 +180,7 @@ public class QemuImgTest { try { QemuImg qemu = new QemuImg(0); qemu.create(file); - qemu.resize(file, endSize); + qemu.resize(file, endSize, null); Map info = qemu.info(file); if (info == null) { @@ -191,7 +209,7 @@ public class QemuImgTest { try { QemuImg qemu = new QemuImg(0); qemu.create(file); - qemu.resize(file, increment, true); + qemu.resize(file, increment, true, null); Map info = qemu.info(file); if (info == null) { @@ -208,6 +226,9 @@ public class QemuImgTest { f.delete(); } + // This test is failing and needs changes in QemuImg.resize to support shrinking images with delta sizes. + // Earlier whole test suite was ignored, now only this test is ignored to allow other tests to run. + @Ignore @Test public void testCreateAndResizeDeltaNegative() throws QemuImgException, LibvirtException { String filename = "/tmp/" + UUID.randomUUID() + ".qcow2"; @@ -219,7 +240,7 @@ public class QemuImgTest { try { QemuImg qemu = new QemuImg(0); qemu.create(file); - qemu.resize(file, increment, true); + qemu.resize(file, increment, true, null); Map info = qemu.info(file); if (info == null) { @@ -249,7 +270,7 @@ public class QemuImgTest { QemuImg qemu = new QemuImg(0); try { qemu.create(file); - qemu.resize(file, endSize); + qemu.resize(file, endSize, null); } finally { File f = new File(filename); f.delete(); @@ -265,7 +286,7 @@ public class QemuImgTest { QemuImg qemu = new QemuImg(0); qemu.create(file); - qemu.resize(file, 0); + qemu.resize(file, 0, null); File f = new File(filename); f.delete(); @@ -377,7 +398,7 @@ public class QemuImgTest { try { QemuImg qemu = new QemuImg(0); - qemu.checkAndRepair(file, null, null, null); + qemu.checkAndRepair(file, new QemuImageOptions(Collections.emptyMap()), Collections.emptyList(), null); } catch (QemuImgException e) { fail(e.getMessage()); } @@ -385,4 +406,160 @@ public class QemuImgTest { File f = new File(filename); f.delete(); } + + @Test + public void addScriptOptionsFromMapAddsValidOptions() throws LibvirtException, QemuImgException { + Script script = Mockito.mock(Script.class); + Map options = new HashMap<>(); + options.put("key1", "value1"); + options.put("key2", "value2"); + + QemuImg qemu = new QemuImg(0); + qemu.addScriptOptionsFromMap(options, script); + + Mockito.verify(script, Mockito.times(1)).add("-o"); + Mockito.verify(script, Mockito.times(1)).add("key1=value1,key2=value2"); + } + + @Test + public void addScriptOptionsFromMapHandlesEmptyOptions() throws LibvirtException, QemuImgException { + Script script = Mockito.mock(Script.class); + Map options = new HashMap<>(); + + QemuImg qemu = new QemuImg(0); + qemu.addScriptOptionsFromMap(options, script); + + Mockito.verify(script, Mockito.never()).add(Mockito.anyString()); + } + + @Test + public void addScriptOptionsFromMapHandlesNullOptions() throws LibvirtException, QemuImgException { + Script script = Mockito.mock(Script.class); + + QemuImg qemu = new QemuImg(0); + qemu.addScriptOptionsFromMap(null, script); + + Mockito.verify(script, Mockito.never()).add(Mockito.anyString()); + } + + @Test + public void addScriptOptionsFromMapHandlesTrailingComma() throws LibvirtException, QemuImgException { + Script script = Mockito.mock(Script.class); + Map options = new HashMap<>(); + options.put("key1", "value1"); + + QemuImg qemu = new QemuImg(0); + qemu.addScriptOptionsFromMap(options, script); + + Mockito.verify(script, Mockito.times(1)).add("-o"); + Mockito.verify(script, Mockito.times(1)).add("key1=value1"); + } + + @Test + public void getResizeOptionsFromConvertOptionsReturnsNullForEmptyOptions() throws LibvirtException, QemuImgException { + QemuImg qemuImg = new QemuImg(0); + Map options = new HashMap<>(); + + Map result = qemuImg.getResizeOptionsFromConvertOptions(options); + + Assert.assertNull(result); + } + + @Test + public void getResizeOptionsFromConvertOptionsReturnsNullForNullOptions() throws LibvirtException, QemuImgException { + QemuImg qemuImg = new QemuImg(0); + + Map result = qemuImg.getResizeOptionsFromConvertOptions(null); + + Assert.assertNull(result); + } + + @Test + public void getResizeOptionsFromConvertOptionsReturnsPreallocationOption() throws LibvirtException, QemuImgException { + QemuImg qemuImg = new QemuImg(0); + Map options = new HashMap<>(); + options.put(QemuImg.PREALLOCATION, "metadata"); + + Map result = qemuImg.getResizeOptionsFromConvertOptions(options); + + Assert.assertNotNull(result); + assertEquals(1, result.size()); + assertEquals("metadata", result.get(QemuImg.PREALLOCATION)); + } + + @Test + public void getResizeOptionsFromConvertOptionsIgnoresUnrelatedOptions() throws LibvirtException, QemuImgException { + QemuImg qemuImg = new QemuImg(0); + Map options = new HashMap<>(); + options.put("unrelatedKey", "unrelatedValue"); + + Map result = qemuImg.getResizeOptionsFromConvertOptions(options); + + Assert.assertTrue(MapUtils.isEmpty(result)); + } + + @Test + public void getResizeOptionsFromConvertOptionsHandlesMixedOptions() throws LibvirtException, QemuImgException { + QemuImg qemuImg = new QemuImg(0); + Map options = new HashMap<>(); + options.put(QemuImg.PREALLOCATION, "full"); + options.put("unrelatedKey", "unrelatedValue"); + + Map result = qemuImg.getResizeOptionsFromConvertOptions(options); + + Assert.assertNotNull(result); + assertEquals(1, result.size()); + assertEquals("full", result.get(QemuImg.PREALLOCATION)); + } + + @Test + public void addScriptResizeOptionsFromMapAddsPreallocationOption() throws LibvirtException, QemuImgException { + Script script = Mockito.mock(Script.class); + Map options = new HashMap<>(); + options.put(QemuImg.PREALLOCATION, "metadata"); + + QemuImg qemuImg = new QemuImg(0); + qemuImg.addScriptResizeOptionsFromMap(options, script); + + Mockito.verify(script, Mockito.times(1)).add("--preallocation=metadata"); + Mockito.verify(script, Mockito.never()).add("-o"); + assertTrue(options.isEmpty()); + } + + @Test + public void addScriptResizeOptionsFromMapHandlesEmptyOptions() throws LibvirtException, QemuImgException { + Script script = Mockito.mock(Script.class); + Map options = new HashMap<>(); + + QemuImg qemuImg = new QemuImg(0); + qemuImg.addScriptResizeOptionsFromMap(options, script); + + Mockito.verify(script, Mockito.never()).add(Mockito.anyString()); + } + + @Test + public void addScriptResizeOptionsFromMapHandlesNullOptions() throws LibvirtException, QemuImgException { + Script script = Mockito.mock(Script.class); + + QemuImg qemuImg = new QemuImg(0); + qemuImg.addScriptResizeOptionsFromMap(null, script); + + Mockito.verify(script, Mockito.never()).add(Mockito.anyString()); + } + + @Test + public void addScriptResizeOptionsFromMapHandlesMixedOptions() throws LibvirtException, QemuImgException { + Script script = Mockito.mock(Script.class); + Map options = new HashMap<>(); + options.put(QemuImg.PREALLOCATION, "full"); + options.put("key", "value"); + + QemuImg qemuImg = new QemuImg(0); + qemuImg.addScriptResizeOptionsFromMap(options, script); + + Mockito.verify(script, Mockito.times(1)).add("--preallocation=full"); + Mockito.verify(script, Mockito.times(1)).add("-o"); + Mockito.verify(script, Mockito.times(1)).add("key=value"); + assertFalse(options.containsKey(QemuImg.PREALLOCATION)); + } } From e9900aba23a089020722e89b438445e6d11128d7 Mon Sep 17 00:00:00 2001 From: Abhisar Sinha <63767682+abh1sar@users.noreply.github.com> Date: Wed, 17 Dec 2025 19:17:28 +0530 Subject: [PATCH 05/10] Show time correctly in the backup schedule UI (#12012) --- .../views/compute/backup/BackupSchedule.vue | 3 +- ui/src/views/compute/backup/FormSchedule.vue | 92 +++++++++++++++---- .../wizard/DeployInstanceBackupSelection.vue | 4 +- ui/src/views/storage/FormSchedule.vue | 79 ++++++++++++---- 4 files changed, 143 insertions(+), 35 deletions(-) diff --git a/ui/src/views/compute/backup/BackupSchedule.vue b/ui/src/views/compute/backup/BackupSchedule.vue index 627bcd8cfbf..b3e894ec982 100644 --- a/ui/src/views/compute/backup/BackupSchedule.vue +++ b/ui/src/views/compute/backup/BackupSchedule.vue @@ -99,7 +99,7 @@ export default { default: false }, dataSource: { - type: Object, + type: Array, required: true }, deleteFn: { @@ -128,6 +128,7 @@ export default { dataIndex: 'intervaltype' }, { + key: 'time', title: this.$t('label.time'), dataIndex: 'schedule' }, diff --git a/ui/src/views/compute/backup/FormSchedule.vue b/ui/src/views/compute/backup/FormSchedule.vue index 1833449c3cc..7d951deaf20 100644 --- a/ui/src/views/compute/backup/FormSchedule.vue +++ b/ui/src/views/compute/backup/FormSchedule.vue @@ -35,16 +35,16 @@ v-model:value="form.intervaltype" button-style="solid" @change="handleChangeIntervalType"> - + {{ $t('label.hourly') }} - + {{ $t('label.daily') }} - + {{ $t('label.weekly') }} - + {{ $t('label.monthly') }} @@ -54,6 +54,7 @@ @@ -79,6 +81,7 @@ @@ -90,7 +91,8 @@ export default { return { backupOffering: null, showAddBackupSchedule: false, - localBackupOfferingId: this.backupOfferingId + localBackupOfferingId: this.backupOfferingId, + dataSource: [] } }, provide () { diff --git a/ui/src/views/storage/FormSchedule.vue b/ui/src/views/storage/FormSchedule.vue index acc12e8158b..433e399d2a8 100644 --- a/ui/src/views/storage/FormSchedule.vue +++ b/ui/src/views/storage/FormSchedule.vue @@ -38,16 +38,16 @@ v-model:value="form.intervaltype" buttonStyle="solid" @change="handleChangeIntervalType"> - + {{ $t('label.hourly') }} - + {{ $t('label.daily') }} - + {{ $t('label.weekly') }} - + {{ $t('label.monthly') }} @@ -60,6 +60,7 @@ :title="$t('label.minute.past.hour')"> @@ -85,6 +87,7 @@ COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES = new ConfigKey<>(Boolean.class, "copy.public.templates.from.other.storages", + "Storage", "true", "Allow SSVMs to try copying public templates from one secondary storage to another instead of downloading them from the source.", + true, ConfigKey.Scope.Zone, null); + /** * should we execute in sequence not involving any storages? * @return tru if commands should execute in sequence diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java index 9609ba7751d..5a8dc3038aa 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java @@ -22,10 +22,12 @@ import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Set; import javax.inject.Inject; @@ -206,12 +208,22 @@ public class DataMigrationUtility { protected List getAllReadyTemplates(DataStore srcDataStore, Map, Long>> childTemplates, List templates) { List files = new LinkedList<>(); + Set idsForMigration = new HashSet<>(); + for (TemplateDataStoreVO template : templates) { - VMTemplateVO templateVO = templateDao.findById(template.getTemplateId()); - if (shouldMigrateTemplate(template, templateVO)) { - files.add(templateFactory.getTemplate(template.getTemplateId(), srcDataStore)); + long templateId = template.getTemplateId(); + if (idsForMigration.contains(templateId)) { + logger.warn("Template store reference [{}] is duplicated; not considering it for migration.", template); + continue; } + VMTemplateVO templateVO = templateDao.findById(templateId); + if (!shouldMigrateTemplate(template, templateVO)) { + continue; + } + files.add(templateFactory.getTemplate(template.getTemplateId(), srcDataStore)); + idsForMigration.add(templateId); } + for (TemplateInfo template: files) { List children = templateDao.listByParentTemplatetId(template.getId()); List temps = new ArrayList<>(); @@ -221,6 +233,7 @@ public class DataMigrationUtility { } childTemplates.put(template, new Pair<>(temps, getTotalChainSize(temps))); } + return (List) (List) files; } @@ -263,16 +276,37 @@ public class DataMigrationUtility { */ protected List getAllReadySnapshotsAndChains(DataStore srcDataStore, Map, Long>> snapshotChains, List snapshots) { List files = new LinkedList<>(); + Set idsForMigration = new HashSet<>(); + for (SnapshotDataStoreVO snapshot : snapshots) { - SnapshotVO snapshotVO = snapshotDao.findById(snapshot.getSnapshotId()); - if (snapshot.getState() == ObjectInDataStoreStateMachine.State.Ready && - snapshotVO != null && snapshotVO.getHypervisorType() != Hypervisor.HypervisorType.Simulator - && snapshot.getParentSnapshotId() == 0 ) { - SnapshotInfo snap = snapshotFactory.getSnapshot(snapshotVO.getSnapshotId(), snapshot.getDataStoreId(), snapshot.getRole()); - if (snap != null) { - files.add(snap); - } + long snapshotId = snapshot.getSnapshotId(); + if (idsForMigration.contains(snapshotId)) { + logger.warn("Snapshot store reference [{}] is duplicated; not considering it for migration.", snapshot); + continue; } + if (snapshot.getState() != ObjectInDataStoreStateMachine.State.Ready) { + logger.warn("Not migrating snapshot [{}] because its state is not ready.", snapshot); + continue; + } + SnapshotVO snapshotVO = snapshotDao.findById(snapshotId); + if (snapshotVO == null) { + logger.debug("Not migrating snapshot [{}] because we could not find its database entry.", snapshot); + continue; + } + if (snapshotVO.getHypervisorType() == Hypervisor.HypervisorType.Simulator) { + logger.debug("Not migrating snapshot [{}] because its hypervisor type is simulator.", snapshot); + continue; + } + if (snapshot.getParentSnapshotId() != 0) { + continue; // The child snapshot will be migrated in the for loop below. + } + SnapshotInfo snap = snapshotFactory.getSnapshot(snapshotVO.getSnapshotId(), snapshot.getDataStoreId(), snapshot.getRole()); + if (snap == null) { + logger.debug("Not migrating snapshot [{}] because we could not get its information.", snapshot); + continue; + } + files.add(snap); + idsForMigration.add(snapshotId); } for (SnapshotInfo parent : files) { @@ -285,7 +319,7 @@ public class DataMigrationUtility { chain.addAll(children); } } - snapshotChains.put(parent, new Pair, Long>(chain, getTotalChainSize(chain))); + snapshotChains.put(parent, new Pair<>(chain, getTotalChainSize(chain))); } return (List) (List) files; @@ -306,14 +340,31 @@ public class DataMigrationUtility { protected List getAllReadyVolumes(DataStore srcDataStore, List volumes) { List files = new LinkedList<>(); + Set idsForMigration = new HashSet<>(); + for (VolumeDataStoreVO volume : volumes) { - if (volume.getState() == ObjectInDataStoreStateMachine.State.Ready) { - VolumeInfo volumeInfo = volumeFactory.getVolume(volume.getVolumeId(), srcDataStore); - if (volumeInfo != null && volumeInfo.getHypervisorType() != Hypervisor.HypervisorType.Simulator) { - files.add(volumeInfo); - } + long volumeId = volume.getVolumeId(); + if (idsForMigration.contains(volumeId)) { + logger.warn("Volume store reference [{}] is duplicated; not considering it for migration.", volume); + continue; } + if (volume.getState() != ObjectInDataStoreStateMachine.State.Ready) { + logger.debug("Not migrating volume [{}] because its state is not ready.", volume); + continue; + } + VolumeInfo volumeInfo = volumeFactory.getVolume(volume.getVolumeId(), srcDataStore); + if (volumeInfo == null) { + logger.debug("Not migrating volume [{}] because we could not get its information.", volume); + continue; + } + if (volumeInfo.getHypervisorType() == Hypervisor.HypervisorType.Simulator) { + logger.debug("Not migrating volume [{}] because its hypervisor type is simulator.", volume); + continue; + } + files.add(volumeInfo); + idsForMigration.add(volumeId); } + return files; } @@ -325,10 +376,9 @@ public class DataMigrationUtility { /** Returns the count of active SSVMs - SSVM with agents in connected state, so as to dynamically increase the thread pool * size when SSVMs scale */ - protected int activeSSVMCount(DataStore dataStore) { - long datacenterId = dataStore.getScope().getScopeId(); + protected int activeSSVMCount(Long zoneId) { List ssvms = - secStorageVmDao.getSecStorageVmListInStates(null, datacenterId, VirtualMachine.State.Running, VirtualMachine.State.Migrating); + secStorageVmDao.getSecStorageVmListInStates(null, zoneId, VirtualMachine.State.Running, VirtualMachine.State.Migrating); int activeSSVMs = 0; for (SecondaryStorageVmVO vm : ssvms) { String name = "s-"+vm.getId()+"-VM"; diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java index 0773c20b6b9..37a1f8dc196 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java @@ -46,6 +46,8 @@ import org.apache.cloudstack.engine.subsystem.api.storage.SecondaryStorageServic import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService.TemplateApiResult; import org.apache.cloudstack.framework.async.AsyncCallFuture; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; @@ -71,9 +73,12 @@ import com.cloud.storage.dao.SnapshotDao; import com.cloud.utils.Pair; import com.cloud.utils.component.ManagerBase; import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.logging.log4j.ThreadContext; public class StorageOrchestrator extends ManagerBase implements StorageOrchestrationService, Configurable { + private static final String LOGCONTEXTID = "logcontextid"; + @Inject SnapshotDataStoreDao snapshotDataStoreDao; @Inject @@ -91,6 +96,8 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra @Inject private SecondaryStorageService secStgSrv; @Inject + TemplateService templateService; + @Inject TemplateDataStoreDao templateDataStoreDao; @Inject VolumeDataStoreDao volumeDataStoreDao; @@ -106,6 +113,9 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra Integer numConcurrentCopyTasksPerSSVM = 2; + private final Map zoneExecutorMap = new HashMap<>(); + private final Map zonePendingWorkCountMap = new HashMap<>(); + @Override public String getConfigComponentName() { return StorageOrchestrationService.class.getName(); @@ -167,8 +177,6 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra double meanstddev = getStandardDeviation(storageCapacities); double threshold = ImageStoreImbalanceThreshold.value(); MigrationResponse response = null; - ThreadPoolExecutor executor = new ThreadPoolExecutor(numConcurrentCopyTasksPerSSVM , numConcurrentCopyTasksPerSSVM, 30, - TimeUnit.MINUTES, new MigrateBlockingQueue<>(numConcurrentCopyTasksPerSSVM)); Date start = new Date(); if (meanstddev < threshold && migrationPolicy == MigrationPolicy.BALANCE) { logger.debug("mean std deviation of the image stores is below threshold, no migration required"); @@ -177,7 +185,7 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra } int skipped = 0; - List>> futures = new ArrayList<>(); + List> futures = new ArrayList<>(); while (true) { DataObject chosenFileForMigration = null; if (files.size() > 0) { @@ -206,7 +214,7 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra } if (shouldMigrate(chosenFileForMigration, srcDatastore.getId(), destDatastoreId, storageCapacities, snapshotChains, childTemplates, migrationPolicy)) { - storageCapacities = migrateAway(chosenFileForMigration, storageCapacities, snapshotChains, childTemplates, srcDatastore, destDatastoreId, executor, futures); + storageCapacities = migrateAway(chosenFileForMigration, storageCapacities, snapshotChains, childTemplates, srcDatastore, destDatastoreId, futures); } else { if (migrationPolicy == MigrationPolicy.BALANCE) { continue; @@ -217,7 +225,7 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra } } Date end = new Date(); - handleSnapshotMigration(srcDataStoreId, start, end, migrationPolicy, futures, storageCapacities, executor); + handleSnapshotMigration(srcDataStoreId, start, end, migrationPolicy, futures, storageCapacities); return handleResponse(futures, migrationPolicy, message, success); } @@ -250,9 +258,7 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra storageCapacities = getStorageCapacities(storageCapacities, srcImgStoreId); storageCapacities = getStorageCapacities(storageCapacities, destImgStoreId); - ThreadPoolExecutor executor = new ThreadPoolExecutor(numConcurrentCopyTasksPerSSVM, numConcurrentCopyTasksPerSSVM, 30, - TimeUnit.MINUTES, new MigrateBlockingQueue<>(numConcurrentCopyTasksPerSSVM)); - List>> futures = new ArrayList<>(); + List> futures = new ArrayList<>(); Date start = new Date(); while (true) { @@ -272,7 +278,7 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra } if (storageCapacityBelowThreshold(storageCapacities, destImgStoreId)) { - storageCapacities = migrateAway(chosenFileForMigration, storageCapacities, snapshotChains, childTemplates, srcDatastore, destImgStoreId, executor, futures); + storageCapacities = migrateAway(chosenFileForMigration, storageCapacities, snapshotChains, childTemplates, srcDatastore, destImgStoreId, futures); } else { message = "Migration failed. Destination store doesn't have enough capacity for migration"; success = false; @@ -289,7 +295,7 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra SnapshotInfo snapshotInfo = snapshotFactory.getSnapshot(snap.getSnapshotId(), snap.getDataStoreId(), DataStoreRole.Image); SnapshotInfo parentSnapshot = snapshotInfo.getParent(); if (snapshotInfo.getDataStore().getId() == srcImgStoreId && parentSnapshot != null && migratedSnapshotIdList.contains(parentSnapshot.getSnapshotId())) { - futures.add(executor.submit(new MigrateDataTask(snapshotInfo, srcDatastore, dataStoreManager.getDataStore(destImgStoreId, DataStoreRole.Image)))); + futures.add(submit(srcDatastore.getScope().getScopeId(), new MigrateDataTask(snapshotInfo, srcDatastore, dataStoreManager.getDataStore(destImgStoreId, DataStoreRole.Image)))); } }); } @@ -297,6 +303,11 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra return handleResponse(futures, null, message, success); } + @Override + public Future orchestrateTemplateCopyToImageStore(TemplateInfo source, DataStore destStore) { + return submit(destStore.getScope().getScopeId(), new CopyTemplateTask(source, destStore)); + } + protected Pair migrateCompleted(Long destDatastoreId, DataStore srcDatastore, List files, MigrationPolicy migrationPolicy, int skipped) { String message = ""; boolean success = true; @@ -332,19 +343,10 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra Map, Long>> templateChains, DataStore srcDatastore, Long destDatastoreId, - ThreadPoolExecutor executor, - List>> futures) { + List> futures) { Long fileSize = migrationHelper.getFileSize(chosenFileForMigration, snapshotChains, templateChains); - storageCapacities = assumeMigrate(storageCapacities, srcDatastore.getId(), destDatastoreId, fileSize); - long activeSsvms = migrationHelper.activeSSVMCount(srcDatastore); - long totalJobs = activeSsvms * numConcurrentCopyTasksPerSSVM; - // Increase thread pool size with increase in number of SSVMs - if ( totalJobs > executor.getCorePoolSize()) { - executor.setMaximumPoolSize((int) (totalJobs)); - executor.setCorePoolSize((int) (totalJobs)); - } MigrateDataTask task = new MigrateDataTask(chosenFileForMigration, srcDatastore, dataStoreManager.getDataStore(destDatastoreId, DataStoreRole.Image)); if (chosenFileForMigration instanceof SnapshotInfo ) { @@ -353,19 +355,64 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra if (chosenFileForMigration instanceof TemplateInfo) { task.setTemplateChain(templateChains); } - futures.add((executor.submit(task))); + futures.add(submit(srcDatastore.getScope().getScopeId(), task)); logger.debug("Migration of {}: {} is initiated.", chosenFileForMigration.getType().name(), chosenFileForMigration.getUuid()); return storageCapacities; } + protected Future submit(Long zoneId, Callable task) { + ThreadPoolExecutor executor; + synchronized (this) { + if (!zoneExecutorMap.containsKey(zoneId)) { + zoneExecutorMap.put(zoneId, new ThreadPoolExecutor(numConcurrentCopyTasksPerSSVM, numConcurrentCopyTasksPerSSVM, + 30, TimeUnit.MINUTES, new MigrateBlockingQueue<>(numConcurrentCopyTasksPerSSVM))); + zonePendingWorkCountMap.put(zoneId, 0); + } + zonePendingWorkCountMap.merge(zoneId, 1, Integer::sum); + scaleExecutorIfNecessary(zoneId); + executor = zoneExecutorMap.get(zoneId); + } + return executor.submit(task); + } - private MigrationResponse handleResponse(List>> futures, MigrationPolicy migrationPolicy, String message, boolean success) { + protected void scaleExecutorIfNecessary(Long zoneId) { + long activeSsvms = migrationHelper.activeSSVMCount(zoneId); + long totalJobs = activeSsvms * numConcurrentCopyTasksPerSSVM; + ThreadPoolExecutor executor = zoneExecutorMap.get(zoneId); + if (totalJobs > executor.getCorePoolSize()) { + logger.debug("Scaling up executor of zone [{}] from [{}] to [{}] threads.", zoneId, executor.getCorePoolSize(), + totalJobs); + executor.setMaximumPoolSize((int) (totalJobs)); + executor.setCorePoolSize((int) (totalJobs)); + } + } + + protected synchronized void tryCleaningUpExecutor(Long zoneId) { + if (!zoneExecutorMap.containsKey(zoneId)) { + logger.debug("No executor exists for zone [{}].", zoneId); + return; + } + + zonePendingWorkCountMap.merge(zoneId, -1, Integer::sum); + Integer pendingWorkCount = zonePendingWorkCountMap.get(zoneId); + if (pendingWorkCount > 0) { + logger.debug("Not cleaning executor of zone [{}] yet, as there is [{}] pending work.", zoneId, pendingWorkCount); + return; + } + + logger.debug("Cleaning executor of zone [{}].", zoneId); + ThreadPoolExecutor executor = zoneExecutorMap.get(zoneId); + zoneExecutorMap.remove(zoneId); + executor.shutdown(); + } + + private MigrationResponse handleResponse(List> futures, MigrationPolicy migrationPolicy, String message, boolean success) { int successCount = 0; - for (Future> future : futures) { + for (Future future : futures) { try { - AsyncCallFuture res = future.get(); - if (res.get().isSuccess()) { + DataObjectResult res = future.get(); + if (res.isSuccess()) { successCount++; } } catch ( InterruptedException | ExecutionException e) { @@ -379,7 +426,7 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra } private void handleSnapshotMigration(Long srcDataStoreId, Date start, Date end, MigrationPolicy policy, - List>> futures, Map> storageCapacities, ThreadPoolExecutor executor) { + List> futures, Map> storageCapacities) { DataStore srcDatastore = dataStoreManager.getDataStore(srcDataStoreId, DataStoreRole.Image); List snaps = snapshotDataStoreDao.findSnapshots(srcDataStoreId, start, end); if (!snaps.isEmpty()) { @@ -395,12 +442,12 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra storeId = dstores.get(1); } DataStore datastore = dataStoreManager.getDataStore(storeId, DataStoreRole.Image); - futures.add(executor.submit(new MigrateDataTask(snapshotInfo, srcDatastore, datastore))); + futures.add(submit(srcDatastore.getScope().getScopeId(), new MigrateDataTask(snapshotInfo, srcDatastore, datastore))); } if (parentSnapshot != null) { DataStore parentDS = dataStoreManager.getDataStore(parentSnapshot.getDataStore().getId(), DataStoreRole.Image); if (parentDS.getId() != snapshotInfo.getDataStore().getId()) { - futures.add(executor.submit(new MigrateDataTask(snapshotInfo, srcDatastore, parentDS))); + futures.add(submit(srcDatastore.getScope().getScopeId(), new MigrateDataTask(snapshotInfo, srcDatastore, parentDS))); } } } @@ -527,16 +574,19 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra return standardDeviation.evaluate(metricValues, mean); } - private class MigrateDataTask implements Callable> { + private class MigrateDataTask implements Callable { private DataObject file; private DataStore srcDataStore; private DataStore destDataStore; private Map, Long>> snapshotChain; private Map, Long>> templateChain; + private String logid; + public MigrateDataTask(DataObject file, DataStore srcDataStore, DataStore destDataStore) { this.file = file; this.srcDataStore = srcDataStore; this.destDataStore = destDataStore; + this.logid = ThreadContext.get(LOGCONTEXTID); } public void setSnapshotChains(Map, Long>> snapshotChain) { @@ -557,8 +607,50 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra } @Override - public AsyncCallFuture call() throws Exception { - return secStgSrv.migrateData(file, srcDataStore, destDataStore, snapshotChain, templateChain); + public DataObjectResult call() { + ThreadContext.put(LOGCONTEXTID, logid); + DataObjectResult result; + AsyncCallFuture future = secStgSrv.migrateData(file, srcDataStore, destDataStore, snapshotChain, templateChain); + try { + result = future.get(); + } catch (ExecutionException | InterruptedException e) { + logger.warn("Exception while migrating data to another secondary storage: {}", e.toString()); + result = new DataObjectResult(file); + result.setResult(e.toString()); + } + tryCleaningUpExecutor(srcDataStore.getScope().getScopeId()); + ThreadContext.clearAll(); + return result; + } + } + + private class CopyTemplateTask implements Callable { + private TemplateInfo sourceTmpl; + private DataStore destStore; + private String logid; + + public CopyTemplateTask(TemplateInfo sourceTmpl, DataStore destStore) { + this.sourceTmpl = sourceTmpl; + this.destStore = destStore; + this.logid = ThreadContext.get(LOGCONTEXTID); + } + + @Override + public TemplateApiResult call() { + ThreadContext.put(LOGCONTEXTID, logid); + TemplateApiResult result; + AsyncCallFuture future = templateService.copyTemplateToImageStore(sourceTmpl, destStore); + try { + result = future.get(); + } catch (ExecutionException | InterruptedException e) { + logger.warn("Exception while copying template [{}] from image store [{}] to image store [{}]: {}", + sourceTmpl.getUniqueName(), sourceTmpl.getDataStore().getName(), destStore.getName(), e.toString()); + result = new TemplateApiResult(sourceTmpl); + result.setResult(e.getMessage()); + } + tryCleaningUpExecutor(destStore.getScope().getScopeId()); + ThreadContext.clearAll(); + return result; } } } diff --git a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java index 38e0d0d081c..fd723b8bf34 100644 --- a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java +++ b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java @@ -31,6 +31,7 @@ import java.util.concurrent.ExecutionException; import javax.inject.Inject; +import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService; import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult; import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionService; @@ -42,7 +43,6 @@ import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.State; -import org.apache.cloudstack.engine.subsystem.api.storage.Scope; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.engine.subsystem.api.storage.StorageCacheManager; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory; @@ -58,6 +58,7 @@ import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.framework.messagebus.MessageBus; import org.apache.cloudstack.framework.messagebus.PublishScope; import org.apache.cloudstack.storage.command.CommandResult; +import org.apache.cloudstack.storage.command.CopyCmdAnswer; import org.apache.cloudstack.storage.command.DeleteCommand; import org.apache.cloudstack.storage.datastore.DataObjectManager; import org.apache.cloudstack.storage.datastore.ObjectInDataStoreManager; @@ -77,7 +78,6 @@ import com.cloud.agent.api.storage.ListTemplateCommand; import com.cloud.agent.api.to.DatadiskTO; import com.cloud.alert.AlertManager; import com.cloud.configuration.Config; -import com.cloud.configuration.Resource; import com.cloud.configuration.Resource.ResourceType; import com.cloud.dc.DataCenterVO; import com.cloud.dc.dao.ClusterDao; @@ -157,6 +157,8 @@ public class TemplateServiceImpl implements TemplateService { ImageStoreDetailsUtil imageStoreDetailsUtil; @Inject TemplateDataFactory imageFactory; + @Inject + StorageOrchestrationService storageOrchestrator; class TemplateOpContext extends AsyncRpcContext { final TemplateObject template; @@ -320,7 +322,6 @@ public class TemplateServiceImpl implements TemplateService { if (syncLock.lock(3)) { try { Long zoneId = store.getScope().getScopeId(); - Map templateInfos = listTemplate(store); if (templateInfos == null) { return; @@ -529,10 +530,6 @@ public class TemplateServiceImpl implements TemplateService { availHypers.add(HypervisorType.None); // bug 9809: resume ISO // download. for (VMTemplateVO tmplt : toBeDownloaded) { - if (tmplt.getUrl() == null) { // If url is null, skip downloading - logger.info("Skip downloading template {} since no url is specified.", tmplt); - continue; - } // if this is private template, skip sync to a new image store if (isSkipTemplateStoreDownload(tmplt, zoneId)) { logger.info("Skip sync downloading private template {} to a new image store", tmplt); @@ -551,14 +548,10 @@ public class TemplateServiceImpl implements TemplateService { } if (availHypers.contains(tmplt.getHypervisorType())) { - logger.info("Downloading template {} to image store {}", tmplt, store); - associateTemplateToZone(tmplt.getId(), zoneId); - TemplateInfo tmpl = _templateFactory.getTemplate(tmplt.getId(), store); - TemplateOpContext context = new TemplateOpContext<>(null,(TemplateObject)tmpl, null); - AsyncCallbackDispatcher caller = AsyncCallbackDispatcher.create(this); - caller.setCallback(caller.getTarget().createTemplateAsyncCallBack(null, null)); - caller.setContext(context); - createTemplateAsync(tmpl, store, caller); + boolean copied = isCopyFromOtherStoragesEnabled(zoneId) && tryCopyingTemplateToImageStore(tmplt, store); + if (!copied) { + tryDownloadingTemplateToImageStore(tmplt, store); + } } else { logger.info("Skip downloading template {} since current data center does not have hypervisor {}", tmplt, tmplt.getHypervisorType()); } @@ -605,6 +598,127 @@ public class TemplateServiceImpl implements TemplateService { } + protected void tryDownloadingTemplateToImageStore(VMTemplateVO tmplt, DataStore destStore) { + if (tmplt.getUrl() == null) { + logger.info("Not downloading template [{}] to image store [{}], as it has no URL.", tmplt.getUniqueName(), + destStore.getName()); + return; + } + logger.info("Downloading template [{}] to image store [{}].", tmplt.getUniqueName(), destStore.getName()); + associateTemplateToZone(tmplt.getId(), destStore.getScope().getScopeId()); + TemplateInfo tmpl = _templateFactory.getTemplate(tmplt.getId(), destStore); + TemplateOpContext context = new TemplateOpContext<>(null,(TemplateObject)tmpl, null); + AsyncCallbackDispatcher caller = AsyncCallbackDispatcher.create(this); + caller.setCallback(caller.getTarget().createTemplateAsyncCallBack(null, null)); + caller.setContext(context); + createTemplateAsync(tmpl, destStore, caller); + } + + protected boolean tryCopyingTemplateToImageStore(VMTemplateVO tmplt, DataStore destStore) { + Long zoneId = destStore.getScope().getScopeId(); + List storesInZone = _storeMgr.getImageStoresByZoneIds(zoneId); + for (DataStore sourceStore : storesInZone) { + Map existingTemplatesInSourceStore = listTemplate(sourceStore); + if (existingTemplatesInSourceStore == null || !existingTemplatesInSourceStore.containsKey(tmplt.getUniqueName())) { + logger.debug("Template [{}] does not exist on image store [{}]; searching on another one.", + tmplt.getUniqueName(), sourceStore.getName()); + continue; + } + TemplateObject sourceTmpl = (TemplateObject) _templateFactory.getTemplate(tmplt.getId(), sourceStore); + if (sourceTmpl.getInstallPath() == null) { + logger.warn("Can not copy template [{}] from image store [{}], as it returned a null install path.", tmplt.getUniqueName(), + sourceStore.getName()); + continue; + } + storageOrchestrator.orchestrateTemplateCopyToImageStore(sourceTmpl, destStore); + return true; + } + logger.debug("Can't copy template [{}] from another image store.", tmplt.getUniqueName()); + return false; + } + + @Override + public AsyncCallFuture copyTemplateToImageStore(DataObject source, DataStore destStore) { + TemplateObject sourceTmpl = (TemplateObject) source; + logger.debug("Copying template [{}] from image store [{}] to [{}].", sourceTmpl.getUniqueName(), sourceTmpl.getDataStore().getName(), + destStore.getName()); + TemplateObject destTmpl = (TemplateObject) destStore.create(sourceTmpl); + destTmpl.processEvent(Event.CreateOnlyRequested); + + AsyncCallFuture future = new AsyncCallFuture<>(); + TemplateOpContext context = new TemplateOpContext<>(null, destTmpl, future); + AsyncCallbackDispatcher caller = AsyncCallbackDispatcher.create(this); + caller.setCallback(caller.getTarget().copyTemplateToImageStoreCallback(null, null)).setContext(context); + + if (source.getDataStore().getId() == destStore.getId()) { + logger.debug("Destination image store [{}] is the same as the origin; returning success to normalize the metadata."); + CopyCmdAnswer answer = new CopyCmdAnswer(source.getTO()); + CopyCommandResult result = new CopyCommandResult("", answer); + caller.complete(result); + return future; + } + + _motionSrv.copyAsync(sourceTmpl, destTmpl, caller); + return future; + } + + protected Void copyTemplateToImageStoreCallback(AsyncCallbackDispatcher callback, TemplateOpContext context) { + TemplateInfo tmplt = context.getTemplate(); + CopyCommandResult result = callback.getResult(); + AsyncCallFuture future = context.getFuture(); + TemplateApiResult res = new TemplateApiResult(tmplt); + if (result.isSuccess()) { + logger.info("Copied template [{}] to image store [{}].", tmplt.getUniqueName(), tmplt.getDataStore().getName()); + tmplt.processEvent(Event.OperationSuccessed, result.getAnswer()); + publishTemplateCreation(tmplt); + } else { + logger.warn("Failed to copy template [{}] to image store [{}].", tmplt.getUniqueName(), tmplt.getDataStore().getName()); + res.setResult(result.getResult()); + tmplt.processEvent(Event.OperationFailed); + } + future.complete(res); + return null; + } + + protected boolean isCopyFromOtherStoragesEnabled(Long zoneId) { + return StorageManager.COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES.valueIn(zoneId); + } + + protected void publishTemplateCreation(TemplateInfo tmplt) { + VMTemplateVO tmpltVo = _templateDao.findById(tmplt.getId()); + + if (tmpltVo.isPublicTemplate()) { + _messageBus.publish(null, TemplateManager.MESSAGE_REGISTER_PUBLIC_TEMPLATE_EVENT, PublishScope.LOCAL, tmpltVo.getId()); + } + + Long size = tmplt.getSize(); + if (size == null) { + return; + } + + DataStore store = tmplt.getDataStore(); + TemplateDataStoreVO tmpltStore = _vmTemplateStoreDao.findByStoreTemplate(store.getId(), tmpltVo.getId()); + + long physicalSize = 0; + if (tmpltStore != null) { + physicalSize = tmpltStore.getPhysicalSize(); + } else { + logger.warn("No entry found in template_store_ref for template [{}] and image store [{}] at the end of registering template!", + tmpltVo.getUniqueName(), store.getName()); + } + + Long zoneId = store.getScope().getScopeId(); + if (zoneId != null) { + String usageEvent = tmplt.getFormat() == ImageFormat.ISO ? EventTypes.EVENT_ISO_CREATE : EventTypes.EVENT_TEMPLATE_CREATE; + UsageEventUtils.publishUsageEvent(usageEvent, tmpltVo.getAccountId(), zoneId, tmpltVo.getId(), tmpltVo.getName(), + null, null, physicalSize, size, VirtualMachineTemplate.class.getName(), tmpltVo.getUuid()); + } else { + logger.warn("Zone-wide image store [{}] has a null scope ID.", store); + } + + _resourceLimitMgr.incrementResourceCount(tmpltVo.getAccountId(), ResourceType.secondary_storage, size); + } + // persist entry in template_zone_ref table. zoneId can be empty for // region-wide image store, in that case, // we will associate the template to all the zones. @@ -650,45 +764,14 @@ public class TemplateServiceImpl implements TemplateService { protected Void createTemplateAsyncCallBack(AsyncCallbackDispatcher callback, TemplateOpContext context) { - TemplateInfo template = context.template; TemplateApiResult result = callback.getResult(); if (result.isSuccess()) { - VMTemplateVO tmplt = _templateDao.findById(template.getId()); - // need to grant permission for public templates - if (tmplt.isPublicTemplate()) { - _messageBus.publish(null, TemplateManager.MESSAGE_REGISTER_PUBLIC_TEMPLATE_EVENT, PublishScope.LOCAL, tmplt.getId()); - } - long accountId = tmplt.getAccountId(); - if (template.getSize() != null) { - // publish usage event - String etype = EventTypes.EVENT_TEMPLATE_CREATE; - if (tmplt.getFormat() == ImageFormat.ISO) { - etype = EventTypes.EVENT_ISO_CREATE; - } - // get physical size from template_store_ref table - long physicalSize = 0; - DataStore ds = template.getDataStore(); - TemplateDataStoreVO tmpltStore = _vmTemplateStoreDao.findByStoreTemplate(ds.getId(), template.getId()); - if (tmpltStore != null) { - physicalSize = tmpltStore.getPhysicalSize(); - } else { - logger.warn("No entry found in template_store_ref for template: {} and image store: {} at the end of registering template!", template, ds); - } - Scope dsScope = ds.getScope(); - if (dsScope.getScopeId() != null) { - UsageEventUtils.publishUsageEvent(etype, template.getAccountId(), dsScope.getScopeId(), template.getId(), template.getName(), null, null, - physicalSize, template.getSize(), VirtualMachineTemplate.class.getName(), template.getUuid()); - } else { - logger.warn("Zone scope image store {} has a null scope id", ds); - } - _resourceLimitMgr.incrementResourceCount(accountId, Resource.ResourceType.secondary_storage, template.getSize()); - } + publishTemplateCreation(context.template); } - return null; } - private Map listTemplate(DataStore ssStore) { + protected Map listTemplate(DataStore ssStore) { String nfsVersion = imageStoreDetailsUtil.getNfsVersion(ssStore.getId()); ListTemplateCommand cmd = new ListTemplateCommand(ssStore.getTO(), nfsVersion); EndPoint ep = _epSelector.select(ssStore); diff --git a/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java b/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java index bc6f37b201a..276581e2e48 100644 --- a/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java +++ b/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java @@ -18,9 +18,17 @@ */ package org.apache.cloudstack.storage.image; +import com.cloud.storage.template.TemplateProp; +import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; +import org.apache.cloudstack.engine.subsystem.api.storage.Scope; +import org.apache.cloudstack.framework.async.AsyncCallFuture; import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao; import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO; +import org.apache.cloudstack.storage.image.store.TemplateObject; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; @@ -33,6 +41,10 @@ import com.cloud.storage.DataStoreRole; import com.cloud.storage.Storage; import com.cloud.storage.VMTemplateVO; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + @RunWith(MockitoJUnitRunner.class) public class TemplateServiceImplTest { @@ -43,6 +55,49 @@ public class TemplateServiceImplTest { @Mock TemplateDataStoreDao templateDataStoreDao; + @Mock + TemplateDataFactoryImpl templateDataFactoryMock; + + @Mock + DataStoreManager dataStoreManagerMock; + + @Mock + VMTemplateVO tmpltMock; + + @Mock + TemplateProp tmpltPropMock; + + @Mock + TemplateObject templateInfoMock; + + @Mock + DataStore sourceStoreMock; + + @Mock + DataStore destStoreMock; + + @Mock + Scope zoneScopeMock; + + @Mock + StorageOrchestrationService storageOrchestrator; + + Map templatesInSourceStore = new HashMap<>(); + + @Before + public void setUp() { + Long zoneId = 1L; + Mockito.doReturn(2L).when(tmpltMock).getId(); + Mockito.doReturn("unique-name").when(tmpltMock).getUniqueName(); + Mockito.doReturn(zoneId).when(zoneScopeMock).getScopeId(); + Mockito.doReturn(zoneScopeMock).when(destStoreMock).getScope(); + Mockito.doReturn(List.of(sourceStoreMock, destStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(zoneId); + Mockito.doReturn(templatesInSourceStore).when(templateService).listTemplate(sourceStoreMock); + Mockito.doReturn(null).when(templateService).listTemplate(destStoreMock); + Mockito.doReturn("install-path").when(templateInfoMock).getInstallPath(); + Mockito.doReturn(templateInfoMock).when(templateDataFactoryMock).getTemplate(2L, sourceStoreMock); + } + @Test public void testIsSkipTemplateStoreDownloadPublicTemplate() { VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class); @@ -81,4 +136,51 @@ public class TemplateServiceImplTest { Mockito.when(templateDataStoreDao.findByTemplateZone(id, id, DataStoreRole.Image)).thenReturn(Mockito.mock(TemplateDataStoreVO.class)); Assert.assertTrue(templateService.isSkipTemplateStoreDownload(templateVO, id)); } + + @Test + public void tryDownloadingTemplateToImageStoreTestDownloadsTemplateWhenUrlIsNotNull() { + Mockito.doReturn("url").when(tmpltMock).getUrl(); + Mockito.doNothing().when(templateService).associateTemplateToZone(Mockito.anyLong(), Mockito.any(Long.class)); + + templateService.tryDownloadingTemplateToImageStore(tmpltMock, destStoreMock); + + Mockito.verify(templateService).createTemplateAsync(Mockito.any(), Mockito.any(), Mockito.any()); + } + + @Test + public void tryDownloadingTemplateToImageStoreTestDoesNothingWhenUrlIsNull() { + templateService.tryDownloadingTemplateToImageStore(tmpltMock, destStoreMock); + + Mockito.verify(templateService, Mockito.never()).createTemplateAsync(Mockito.any(), Mockito.any(), Mockito.any()); + } + + @Test + public void tryCopyingTemplateToImageStoreTestReturnsFalseWhenTemplateDoesNotExistOnAnotherImageStore() { + boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock); + + Assert.assertFalse(result); + Mockito.verify(storageOrchestrator, Mockito.never()).orchestrateTemplateCopyToImageStore(Mockito.any(), Mockito.any()); + } + + @Test + public void tryCopyingTemplateToImageStoreTestReturnsFalseWhenInstallPathIsNull() { + templatesInSourceStore.put(tmpltMock.getUniqueName(), tmpltPropMock); + Mockito.doReturn(null).when(templateInfoMock).getInstallPath(); + + boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock); + + Assert.assertFalse(result); + Mockito.verify(storageOrchestrator, Mockito.never()).orchestrateTemplateCopyToImageStore(Mockito.any(), Mockito.any()); + } + + @Test + public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenTemplateExistsInAnotherStorageAndTaskWasScheduled() { + templatesInSourceStore.put(tmpltMock.getUniqueName(), tmpltPropMock); + Mockito.doReturn(new AsyncCallFuture<>()).when(storageOrchestrator).orchestrateTemplateCopyToImageStore(Mockito.any(), Mockito.any()); + + boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock); + + Assert.assertTrue(result); + Mockito.verify(storageOrchestrator).orchestrateTemplateCopyToImageStore(Mockito.any(), Mockito.any()); + } } diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 5a66ad502f2..19da1425dc0 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -4194,7 +4194,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C VmwareAllowParallelExecution, DataStoreDownloadFollowRedirects, AllowVolumeReSizeBeyondAllocation, - StoragePoolHostConnectWorkers + StoragePoolHostConnectWorkers, + COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES }; } From 8936e4c53507b4cdb0c656a7689db863535ced00 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Thu, 18 Dec 2025 11:53:00 +0100 Subject: [PATCH 07/10] api:rename RegisterCmd.java => RegisterUserKeyCmd.java (#12259) --- api/src/main/java/com/cloud/user/AccountService.java | 4 ++-- .../cloudstack/api/command/admin/user/GetUserKeysCmd.java | 6 +++--- .../user/{RegisterCmd.java => RegisterUserKeyCmd.java} | 8 ++++---- ...RegisterResponse.java => RegisterUserKeyResponse.java} | 2 +- .../network/contrail/management/MockAccountManager.java | 4 ++-- .../main/java/com/cloud/server/ManagementServerImpl.java | 4 ++-- .../src/main/java/com/cloud/user/AccountManagerImpl.java | 4 ++-- 7 files changed, 16 insertions(+), 16 deletions(-) rename api/src/main/java/org/apache/cloudstack/api/command/admin/user/{RegisterCmd.java => RegisterUserKeyCmd.java} (93%) rename api/src/main/java/org/apache/cloudstack/api/response/{RegisterResponse.java => RegisterUserKeyResponse.java} (96%) diff --git a/api/src/main/java/com/cloud/user/AccountService.java b/api/src/main/java/com/cloud/user/AccountService.java index c0ebcf09f59..09fe5ffc059 100644 --- a/api/src/main/java/com/cloud/user/AccountService.java +++ b/api/src/main/java/com/cloud/user/AccountService.java @@ -25,7 +25,7 @@ import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.command.admin.account.CreateAccountCmd; import org.apache.cloudstack.api.command.admin.user.GetUserKeysCmd; -import org.apache.cloudstack.api.command.admin.user.RegisterCmd; +import org.apache.cloudstack.api.command.admin.user.RegisterUserKeyCmd; import org.apache.cloudstack.api.command.admin.user.UpdateUserCmd; import com.cloud.dc.DataCenter; @@ -95,7 +95,7 @@ public interface AccountService { void markUserRegistered(long userId); - public String[] createApiKeyAndSecretKey(RegisterCmd cmd); + public String[] createApiKeyAndSecretKey(RegisterUserKeyCmd cmd); public String[] createApiKeyAndSecretKey(final long userId); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/user/GetUserKeysCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/user/GetUserKeysCmd.java index cdd239f72b5..f04c5f6c478 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/user/GetUserKeysCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/user/GetUserKeysCmd.java @@ -26,14 +26,14 @@ import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseCmd; import org.apache.cloudstack.api.Parameter; -import org.apache.cloudstack.api.response.RegisterResponse; +import org.apache.cloudstack.api.response.RegisterUserKeyResponse; import org.apache.cloudstack.api.response.UserResponse; import java.util.Map; @APICommand(name = "getUserKeys", description = "This command allows the user to query the seceret and API keys for the account", - responseObject = RegisterResponse.class, + responseObject = RegisterUserKeyResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = true, authorized = {RoleType.User, RoleType.Admin, RoleType.DomainAdmin, RoleType.ResourceAdmin}, @@ -57,7 +57,7 @@ public class GetUserKeysCmd extends BaseCmd{ public void execute(){ Pair> keys = _accountService.getKeys(this); - RegisterResponse response = new RegisterResponse(); + RegisterUserKeyResponse response = new RegisterUserKeyResponse(); if(keys != null){ response.setApiKeyAccess(keys.first()); response.setApiKey(keys.second().get("apikey")); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/user/RegisterCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/user/RegisterUserKeyCmd.java similarity index 93% rename from api/src/main/java/org/apache/cloudstack/api/command/admin/user/RegisterCmd.java rename to api/src/main/java/org/apache/cloudstack/api/command/admin/user/RegisterUserKeyCmd.java index b3e7d2bec82..61f47e2799c 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/user/RegisterCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/user/RegisterUserKeyCmd.java @@ -22,17 +22,17 @@ import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseCmd; import org.apache.cloudstack.api.Parameter; -import org.apache.cloudstack.api.response.RegisterResponse; +import org.apache.cloudstack.api.response.RegisterUserKeyResponse; import org.apache.cloudstack.api.response.UserResponse; import com.cloud.user.Account; import com.cloud.user.User; @APICommand(name = "registerUserKeys", - responseObject = RegisterResponse.class, + responseObject = RegisterUserKeyResponse.class, description = "This command allows a user to register for the developer API, returning a secret key and an API key. This request is made through the integration API port, so it is a privileged command and must be made on behalf of a user. It is up to the implementer just how the username and password are entered, and then how that translates to an integration API request. Both secret key and API key should be returned to the user", requestHasSensitiveInfo = false, responseHasSensitiveInfo = true) -public class RegisterCmd extends BaseCmd { +public class RegisterUserKeyCmd extends BaseCmd { ///////////////////////////////////////////////////// @@ -81,7 +81,7 @@ public class RegisterCmd extends BaseCmd { @Override public void execute() { String[] keys = _accountService.createApiKeyAndSecretKey(this); - RegisterResponse response = new RegisterResponse(); + RegisterUserKeyResponse response = new RegisterUserKeyResponse(); if (keys != null) { response.setApiKey(keys[0]); response.setSecretKey(keys[1]); diff --git a/api/src/main/java/org/apache/cloudstack/api/response/RegisterResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/RegisterUserKeyResponse.java similarity index 96% rename from api/src/main/java/org/apache/cloudstack/api/response/RegisterResponse.java rename to api/src/main/java/org/apache/cloudstack/api/response/RegisterUserKeyResponse.java index dd17cc5cc8a..c767fb1e673 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/RegisterResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/RegisterUserKeyResponse.java @@ -23,7 +23,7 @@ import org.apache.cloudstack.api.BaseResponse; import com.cloud.serializer.Param; -public class RegisterResponse extends BaseResponse { +public class RegisterUserKeyResponse extends BaseResponse { @SerializedName(ApiConstants.API_KEY) @Param(description = "the api key of the registered user", isSensitive = true) private String apiKey; diff --git a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java index bc9dbfa7b43..2ff68b4836f 100644 --- a/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java +++ b/plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/MockAccountManager.java @@ -37,7 +37,7 @@ import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.command.admin.account.UpdateAccountCmd; import org.apache.cloudstack.api.command.admin.user.DeleteUserCmd; -import org.apache.cloudstack.api.command.admin.user.RegisterCmd; +import org.apache.cloudstack.api.command.admin.user.RegisterUserKeyCmd; import org.apache.cloudstack.api.command.admin.user.UpdateUserCmd; import org.apache.cloudstack.context.CallContext; @@ -118,7 +118,7 @@ public class MockAccountManager extends ManagerBase implements AccountManager { } @Override - public String[] createApiKeyAndSecretKey(RegisterCmd arg0) { + public String[] createApiKeyAndSecretKey(RegisterUserKeyCmd arg0) { // TODO Auto-generated method stub return null; } diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 1a3f18011be..8eb277c7b61 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -282,7 +282,7 @@ import org.apache.cloudstack.api.command.admin.user.GetUserKeysCmd; import org.apache.cloudstack.api.command.admin.user.ListUsersCmd; import org.apache.cloudstack.api.command.admin.user.LockUserCmd; import org.apache.cloudstack.api.command.admin.user.MoveUserCmd; -import org.apache.cloudstack.api.command.admin.user.RegisterCmd; +import org.apache.cloudstack.api.command.admin.user.RegisterUserKeyCmd; import org.apache.cloudstack.api.command.admin.user.UpdateUserCmd; import org.apache.cloudstack.api.command.admin.vlan.CreateVlanIpRangeCmd; import org.apache.cloudstack.api.command.admin.vlan.DedicatePublicIpRangeCmd; @@ -3931,7 +3931,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe cmdList.add(ListUsersCmd.class); cmdList.add(LockUserCmd.class); cmdList.add(MoveUserCmd.class); - cmdList.add(RegisterCmd.class); + cmdList.add(RegisterUserKeyCmd.class); cmdList.add(UpdateUserCmd.class); cmdList.add(CreateVlanIpRangeCmd.class); cmdList.add(UpdateVlanIpRangeCmd.class); diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index 80387c2fc12..b0ba97d6de4 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -61,7 +61,7 @@ import org.apache.cloudstack.api.command.admin.account.UpdateAccountCmd; import org.apache.cloudstack.api.command.admin.user.DeleteUserCmd; import org.apache.cloudstack.api.command.admin.user.GetUserKeysCmd; import org.apache.cloudstack.api.command.admin.user.MoveUserCmd; -import org.apache.cloudstack.api.command.admin.user.RegisterCmd; +import org.apache.cloudstack.api.command.admin.user.RegisterUserKeyCmd; import org.apache.cloudstack.api.command.admin.user.UpdateUserCmd; import org.apache.cloudstack.api.response.UserTwoFactorAuthenticationSetupResponse; import org.apache.cloudstack.auth.UserAuthenticator; @@ -3129,7 +3129,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M @Override @DB @ActionEvent(eventType = EventTypes.EVENT_REGISTER_FOR_SECRET_API_KEY, eventDescription = "register for the developer API keys") - public String[] createApiKeyAndSecretKey(RegisterCmd cmd) { + public String[] createApiKeyAndSecretKey(RegisterUserKeyCmd cmd) { Account caller = getCurrentCallingAccount(); final Long userId = cmd.getId(); From 79ab1566b110e66698dafbb01312a2bd7e359a71 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 19 Dec 2025 20:41:00 +0100 Subject: [PATCH 08/10] packaging: use latest cmk release link directly (#11429) Instead listing all GIthub releases first and finding the latest one use the direct link for the latest Github release. Signed-off-by: Abhishek Kumar --- debian/rules | 3 +-- packaging/el8/cloud.spec | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/debian/rules b/debian/rules index e7ff6759d44..da49ca55c1b 100755 --- a/debian/rules +++ b/debian/rules @@ -4,7 +4,6 @@ VERSION := $(shell grep '' pom.xml | head -2 | tail -1 | cut -d'>' -f2 PACKAGE = $(shell dh_listpackages|head -n 1|cut -d '-' -f 1) SYSCONFDIR = "/etc" DESTDIR = "debian/tmp" -CMK_REL := $(shell wget -O - "https://api.github.com/repos/apache/cloudstack-cloudmonkey/releases" 2>/dev/null | jq -r '.[0].tag_name') %: dh $@ --with systemd @@ -86,7 +85,7 @@ override_dh_auto_install: rm -rf $(DESTDIR)/usr/share/$(PACKAGE)-management/templates/systemvm/md5sum.txt # Bundle cmk in cloudstack-management - wget https://github.com/apache/cloudstack-cloudmonkey/releases/download/$(CMK_REL)/cmk.linux.x86-64 -O $(DESTDIR)/usr/bin/cmk + wget https://github.com/apache/cloudstack-cloudmonkey/releases/latest/download/cmk.linux.x86-64 -O $(DESTDIR)/usr/bin/cmk chmod +x $(DESTDIR)/usr/bin/cmk # nast hack for a couple of configuration files diff --git a/packaging/el8/cloud.spec b/packaging/el8/cloud.spec index 1e862ba0dc0..3d485112266 100644 --- a/packaging/el8/cloud.spec +++ b/packaging/el8/cloud.spec @@ -268,8 +268,7 @@ install -D client/target/utilities/bin/cloud-setup-baremetal ${RPM_BUILD_ROOT}%{ install -D client/target/utilities/bin/cloud-sysvmadm ${RPM_BUILD_ROOT}%{_bindir}/%{name}-sysvmadm install -D client/target/utilities/bin/cloud-update-xenserver-licenses ${RPM_BUILD_ROOT}%{_bindir}/%{name}-update-xenserver-licenses # Bundle cmk in cloudstack-management -CMK_REL=$(wget -O - "https://api.github.com/repos/apache/cloudstack-cloudmonkey/releases" 2>/dev/null | jq -r '.[0].tag_name') -wget https://github.com/apache/cloudstack-cloudmonkey/releases/download/$CMK_REL/cmk.linux.x86-64 -O ${RPM_BUILD_ROOT}%{_bindir}/cmk +wget https://github.com/apache/cloudstack-cloudmonkey/releases/latest/download/cmk.linux.x86-64 -O ${RPM_BUILD_ROOT}%{_bindir}/cmk chmod +x ${RPM_BUILD_ROOT}%{_bindir}/cmk cp -r client/target/utilities/scripts/db/* ${RPM_BUILD_ROOT}%{_datadir}/%{name}-management/setup From 061ce9b39bc1ec63f311b34e4246a6150ade9f22 Mon Sep 17 00:00:00 2001 From: Fabricio Duarte Date: Sat, 20 Dec 2025 06:40:26 -0300 Subject: [PATCH 09/10] Fix VM and volume metrics listing regressions (#12284) --- .../api/ListVolumesUsageHistoryCmd.java | 3 +- .../metrics/MetricsServiceImpl.java | 48 ++++++++++++++----- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVolumesUsageHistoryCmd.java b/plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVolumesUsageHistoryCmd.java index 4e9191a16f8..e5d6a2429d2 100644 --- a/plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVolumesUsageHistoryCmd.java +++ b/plugins/metrics/src/main/java/org/apache/cloudstack/api/ListVolumesUsageHistoryCmd.java @@ -21,7 +21,6 @@ import java.util.List; import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.api.response.ListResponse; -import org.apache.cloudstack.api.response.SystemVmResponse; import org.apache.cloudstack.api.response.VolumeResponse; import org.apache.cloudstack.response.VolumeMetricsStatsResponse; @@ -37,7 +36,7 @@ public class ListVolumesUsageHistoryCmd extends BaseResourceUsageHistoryCmd { @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = VolumeResponse.class, description = "the ID of the volume.") private Long id; - @Parameter(name=ApiConstants.IDS, type=CommandType.LIST, collectionType=CommandType.UUID, entityType= SystemVmResponse.class, description="the IDs of the volumes, mutually exclusive with id.") + @Parameter(name = ApiConstants.IDS, type = CommandType.LIST, collectionType = CommandType.UUID, entityType = VolumeResponse.class, description = "the IDs of the volumes, mutually exclusive with id.") private List ids; @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, description = "name of the volume (a substring match is made against the parameter value returning the data for all matching Volumes).") diff --git a/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java b/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java index c305d9312ba..77785133428 100644 --- a/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java +++ b/plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java @@ -244,6 +244,30 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements return createVolumeMetricsStatsResponse(volumeList, volumeStatsList); } + /** + * Outputs the parameters that should be used for access control in the query of a resource to + * {@code permittedAccounts} and {@code domainIdRecursiveListProject}. + * @param isIdProvided indicates whether any ID was provided to the command + */ + private void buildBaseACLSearchParametersForMetrics(boolean isIdProvided, List permittedAccounts, Ternary domainIdRecursiveListProject) { + Account caller = CallContext.current().getCallingAccount(); + Account.Type callerType = caller.getType(); + + boolean recursive = AccountTypesWithRecursiveUsageAccess.contains(callerType); + domainIdRecursiveListProject.second(recursive); + + // If no ID was provided, then the listing will skip project resources (null); otherwise, project resources should + // be listed as well (any long allows this) + Long id = isIdProvided ? 1L : null; + + // Allow users to also list metrics of resources owned by projects they belong to (-1L), and admins to list all + // metrics belonging to their domains recursively (null) + Long projectId = isIdProvided && callerType == Account.Type.NORMAL ? -1L : null; + + accountMgr.buildACLSearchParameters(caller, id, null, projectId, permittedAccounts, domainIdRecursiveListProject, true, false); + } + /** * Searches VMs based on {@code ListVMsUsageHistoryCmd} parameters. * @@ -251,18 +275,18 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements * @return the list of VMs. */ protected Pair, Integer> searchForUserVmsInternal(ListVMsUsageHistoryCmd cmd) { - final Long id = cmd.getId(); - Account caller = CallContext.current().getCallingAccount(); + List ids = getIdsListFromCmd(cmd.getId(), cmd.getIds()); + + boolean isIdProvided = CollectionUtils.isNotEmpty(ids); List permittedAccounts = new ArrayList<>(); - boolean recursive = AccountTypesWithRecursiveUsageAccess.contains(caller.getType()); - Ternary domainIdRecursiveListProject = new Ternary<>(null, recursive, null); - accountMgr.buildACLSearchParameters(caller, id, null, null, permittedAccounts, domainIdRecursiveListProject, true, false); + Ternary domainIdRecursiveListProject = new Ternary<>(null, null, null); + buildBaseACLSearchParametersForMetrics(isIdProvided, permittedAccounts, domainIdRecursiveListProject); + Long domainId = domainIdRecursiveListProject.first(); Boolean isRecursive = domainIdRecursiveListProject.second(); Project.ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third(); Filter searchFilter = new Filter(UserVmVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal()); - List ids = getIdsListFromCmd(cmd.getId(), cmd.getIds()); String name = cmd.getName(); String keyword = cmd.getKeyword(); @@ -357,18 +381,18 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements * @return the list of VMs. */ protected Pair, Integer> searchForVolumesInternal(ListVolumesUsageHistoryCmd cmd) { - final Long id = cmd.getId(); - Account caller = CallContext.current().getCallingAccount(); + List ids = getIdsListFromCmd(cmd.getId(), cmd.getIds()); + + boolean isIdProvided = CollectionUtils.isNotEmpty(ids); List permittedAccounts = new ArrayList<>(); - boolean recursive = AccountTypesWithRecursiveUsageAccess.contains(caller.getType()); - Ternary domainIdRecursiveListProject = new Ternary<>(null, recursive, null); - accountMgr.buildACLSearchParameters(caller, id, null, null, permittedAccounts, domainIdRecursiveListProject, true, false); + Ternary domainIdRecursiveListProject = new Ternary<>(null, null, null); + buildBaseACLSearchParametersForMetrics(isIdProvided, permittedAccounts, domainIdRecursiveListProject); + Long domainId = domainIdRecursiveListProject.first(); Boolean isRecursive = domainIdRecursiveListProject.second(); Project.ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third(); Filter searchFilter = new Filter(VolumeVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal()); - List ids = getIdsListFromCmd(cmd.getId(), cmd.getIds()); String name = cmd.getName(); String keyword = cmd.getKeyword(); From 47a6ac89ba5b3324c9dd91a52054e58ed19949c0 Mon Sep 17 00:00:00 2001 From: Vitor Hugo Homem Marzarotto <59698484+vits-hugs@users.noreply.github.com> Date: Sun, 21 Dec 2025 06:22:39 -0300 Subject: [PATCH 10/10] =?UTF-8?q?Update=20templateConfig.sh=20to=20not=20b?= =?UTF-8?q?reak=20with=20directorys=20with=20space=20on=20t=E2=80=A6=20(#1?= =?UTF-8?q?0898)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Vitor Hugo Homem Marzarotto Co-authored-by: Henrique Sato Co-authored-by: Wei Zhou --- engine/schema/templateConfig.sh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/engine/schema/templateConfig.sh b/engine/schema/templateConfig.sh index d10b8668b12..0d55eb37d04 100644 --- a/engine/schema/templateConfig.sh +++ b/engine/schema/templateConfig.sh @@ -62,8 +62,8 @@ function getChecksum() { } function createMetadataFile() { - local fileData=$(cat $SOURCEFILE) - echo -e "["default"]\nversion = $VERSION.${securityversion}\n" >> $METADATAFILE + local fileData=$(cat "$SOURCEFILE") + echo -e "["default"]\nversion = $VERSION.${securityversion}\n" >> "$METADATAFILE" for template in "${templates[@]}" do section="${template%%:*}" @@ -76,7 +76,7 @@ function createMetadataFile() { templatename="systemvm-${sectionHv%.*}-${VERSION}-${arch}" checksum=$(getChecksum "$fileData" "$VERSION-${arch}-$hvName") filename=$(echo ${downloadurl##*'/'}) - echo -e "["$section"]\ntemplatename = $templatename\nchecksum = $checksum\ndownloadurl = $downloadurl\nfilename = $filename\narch = $arch\nguestos = $guestos\n" >> $METADATAFILE + echo -e "["$section"]\ntemplatename = $templatename\nchecksum = $checksum\ndownloadurl = $downloadurl\nfilename = $filename\narch = $arch\nguestos = $guestos\n" >> "$METADATAFILE" done } @@ -91,8 +91,8 @@ templates=( "kvm-x86_64:https://download.cloudstack.org/systemvm/${CS_VERSION}/s "ovm3:https://download.cloudstack.org/systemvm/$CS_VERSION/systemvmtemplate-$VERSION-x86_64-ovm.raw.bz2" ) PARENTPATH="$( cd -- "$(dirname "$0")" >/dev/null 2>&1 ; pwd -P )/dist/systemvm-templates/" -mkdir -p $PARENTPATH -METADATAFILE=${PARENTPATH}"metadata.ini" -echo > $METADATAFILE -SOURCEFILE=${PARENTPATH}'md5sum.txt' +mkdir -p "$PARENTPATH" +METADATAFILE="${PARENTPATH}metadata.ini" +echo > "$METADATAFILE" +SOURCEFILE="${PARENTPATH}md5sum.txt" createMetadataFile