This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch 4.22
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.22 by this push:
new e08e66d66de kvm: use preallocation option for fat disk resize (#11986)
e08e66d66de is described below
commit e08e66d66deb43ddd7fe6e0185fe7a0fbf94255a
Author: Abhishek Kumar <[email protected]>
AuthorDate: Wed Dec 17 13:03:39 2025 +0100
kvm: use preallocation option for fat disk resize (#11986)
Signed-off-by: Abhishek Kumar <[email protected]>
---
plugins/hypervisors/kvm/pom.xml | 9 -
.../kvm/storage/LibvirtStorageAdaptor.java | 2 +-
.../org/apache/cloudstack/utils/qemu/QemuImg.java | 84 ++++++---
.../apache/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 @@
</execution>
</executions>
</plugin>
- <plugin>
- <groupId>org.apache.maven.plugins</groupId>
- <artifactId>maven-surefire-plugin</artifactId>
- <configuration>
- <excludes>
- <exclude>**/QemuImg*.java</exclude>
- </excludes>
- </configuration>
- </plugin>
</plugins>
</build>
<profiles>
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<String, String> options = new HashMap<String, String>();
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<String, String> getResizeOptionsFromConvertOptions(final
Map<String, String> options) {
+ if (MapUtils.isEmpty(options)) {
+ return null;
+ }
+ Map<String, String> 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<String, String> options, final
List<QemuObject> 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<String, String> options, Script
s) {
- if (options != null && !options.isEmpty()) {
- s.add("-o");
- final StringBuffer optionsBuffer = new StringBuffer();
- for (final Map.Entry<String, String> 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<String, String> options, Script
s) {
+ if (MapUtils.isEmpty(options)) {
+ return;
}
+ s.add("-o");
+ final StringBuffer optionsBuffer = new StringBuffer();
+ for (final Map.Entry<String, String> 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<String, String> 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.
- *
+ * <p>
* This method is a facade for 'qemu-img resize'.
- *
+ * <p>
* 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<String,
String> 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<String, String> 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)}.
- *
+ * <p>
+ * This method is a facade for {@link QemuImg#resize(QemuImgFile, long,
boolean, Map)}.
+ * <p>
* 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<String,
String> 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<String,
String> 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<String, String> options = new HashMap<String, String>();
@@ -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<String, String> 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<String, String> 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<String, String> 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<String, String> 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<String, String> 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<String, String> 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<String, String> options = new HashMap<>();
+
+ Map<String, String> result =
qemuImg.getResizeOptionsFromConvertOptions(options);
+
+ Assert.assertNull(result);
+ }
+
+ @Test
+ public void getResizeOptionsFromConvertOptionsReturnsNullForNullOptions()
throws LibvirtException, QemuImgException {
+ QemuImg qemuImg = new QemuImg(0);
+
+ Map<String, String> result =
qemuImg.getResizeOptionsFromConvertOptions(null);
+
+ Assert.assertNull(result);
+ }
+
+ @Test
+ public void getResizeOptionsFromConvertOptionsReturnsPreallocationOption()
throws LibvirtException, QemuImgException {
+ QemuImg qemuImg = new QemuImg(0);
+ Map<String, String> options = new HashMap<>();
+ options.put(QemuImg.PREALLOCATION, "metadata");
+
+ Map<String, String> 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<String, String> options = new HashMap<>();
+ options.put("unrelatedKey", "unrelatedValue");
+
+ Map<String, String> result =
qemuImg.getResizeOptionsFromConvertOptions(options);
+
+ Assert.assertTrue(MapUtils.isEmpty(result));
+ }
+
+ @Test
+ public void getResizeOptionsFromConvertOptionsHandlesMixedOptions() throws
LibvirtException, QemuImgException {
+ QemuImg qemuImg = new QemuImg(0);
+ Map<String, String> options = new HashMap<>();
+ options.put(QemuImg.PREALLOCATION, "full");
+ options.put("unrelatedKey", "unrelatedValue");
+
+ Map<String, String> 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<String, String> 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<String, String> 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<String, String> 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));
+ }
}
