diff --git a/plugins/hypervisors/kvm/pom.xml b/plugins/hypervisors/kvm/pom.xml index 8ed960c6bc0c..bc7b6903aae7 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 e8924ecf5ebc..860c78f71663 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 @@ private KVMPhysicalDisk createPhysicalDiskByQemuImg(String name, KVMStoragePool 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 0a8ea27cd490..bf0002e7a71b 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.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 void convert(final QemuImgFile srcFile, final QemuImgFile destFile, 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 void convert(final QemuImgFile srcFile, final QemuImgFile destFile, 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 void convert(final QemuImgFile srcFile, final QemuImgFile destFile, } if (srcFile.getSize() < destFile.getSize()) { - this.resize(destFile, destFile.getSize()); + this.resize(destFile, destFile.getSize(), getResizeOptionsFromConvertOptions(options)); } } @@ -691,17 +703,18 @@ public void deleteSnapshot(final QemuImageOptions srcImageOpts, final String sna } } - 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); } /** @@ -747,19 +760,17 @@ public void rebase(final QemuImgFile file, final QemuImgFile backingFile, final /** * 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 +792,7 @@ public void resize(final QemuImgFile file, final long size, final boolean delta) final Script s = new Script(_qemuImgPath); s.add("resize"); + addScriptOptionsFromMap(options, s); s.add(file.getFileName()); s.add(newSize); s.execute(); @@ -789,7 +801,7 @@ public void resize(final QemuImgFile file, final long size, final boolean delta) /** * 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 +830,17 @@ public void resize(final QemuImageOptions imageOptions, final List q /** * 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 b0981dde26e7..f39628b4af49 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.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; + +import com.cloud.utils.script.Script; -@Ignore +@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 void testCreateWithSecretObject() throws QemuImgException, LibvirtExcepti 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 void testCreateSparseVolume() throws QemuImgException, LibvirtException { 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 void testCreateAndResize() throws QemuImgException, LibvirtException { 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 void testCreateAndResizeDeltaPositive() throws QemuImgException, LibvirtE 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 void testCreateAndResizeDeltaPositive() throws QemuImgException, LibvirtE 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 void testCreateAndResizeDeltaNegative() throws QemuImgException, LibvirtE 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 void testCreateAndResizeFail() throws QemuImgException, LibvirtException 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 void testCreateAndResizeZero() throws QemuImgException, LibvirtException 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 void testCheckAndRepair() throws LibvirtException { 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,109 @@ public void testCheckAndRepair() throws LibvirtException { 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)); + } }