Copilot commented on code in PR #11264:
URL: https://github.com/apache/cloudstack/pull/11264#discussion_r2224305946
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMPhysicalDisk.java:
##########
@@ -71,6 +79,31 @@ private static String replaceHostAddress(String hostIp) {
return hostIp;
}
+ public static long getVirtualSizeFromFile(String path) {
+ try {
+ QemuImg qemu = new QemuImg(0);
+ QemuImgFile qemuFile = new QemuImgFile(path);
+ Map<String, String> info = qemu.info(qemuFile);
+ if (info.containsKey(QemuImg.VIRTUAL_SIZE)) {
+ return Long.parseLong(info.get(QemuImg.VIRTUAL_SIZE));
+ } else {
+ throw new CloudRuntimeException("Unable to determine virtual
size of volume at path " + path);
+ }
+ } catch (QemuImgException | LibvirtException ex) {
+ throw new CloudRuntimeException("Error when inspecting volume at
path " + path, ex);
+ }
+ }
+
+ public static void checkQcow2File(String path) {
+ if (ImageStoreUtil.isCorrectExtension(path, "qcow2")) {
+ try {
+ Qcow2Inspector.validateQcow2File(path);
+ } catch (RuntimeException e) {
+ throw new CloudRuntimeException("The volume file at path " +
path + " is not a valid QCOW2");
Review Comment:
Catching `RuntimeException` is too broad and may mask unexpected errors.
Consider catching the specific exception types that
`Qcow2Inspector.validateQcow2File` throws, or at minimum catch `Exception` and
handle specific validation failures separately from system errors.
```suggestion
} catch (IllegalArgumentException | IllegalStateException e) {
// Replace with specific exceptions thrown by validateQcow2File
throw new CloudRuntimeException("The volume file at path " +
path + " is not a valid QCOW2: " + e.getMessage(), e);
} catch (Exception e) { // Fallback for unexpected exceptions
throw new CloudRuntimeException("An unexpected error
occurred while validating the QCOW2 file at path " + path + ": " +
e.getMessage(), e);
```
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMPhysicalDisk.java:
##########
@@ -71,6 +79,31 @@ private static String replaceHostAddress(String hostIp) {
return hostIp;
}
+ public static long getVirtualSizeFromFile(String path) {
+ try {
+ QemuImg qemu = new QemuImg(0);
+ QemuImgFile qemuFile = new QemuImgFile(path);
+ Map<String, String> info = qemu.info(qemuFile);
+ if (info.containsKey(QemuImg.VIRTUAL_SIZE)) {
+ return Long.parseLong(info.get(QemuImg.VIRTUAL_SIZE));
+ } else {
+ throw new CloudRuntimeException("Unable to determine virtual
size of volume at path " + path);
+ }
+ } catch (QemuImgException | LibvirtException ex) {
+ throw new CloudRuntimeException("Error when inspecting volume at
path " + path, ex);
+ }
+ }
+
+ public static void checkQcow2File(String path) {
+ if (ImageStoreUtil.isCorrectExtension(path, "qcow2")) {
+ try {
+ Qcow2Inspector.validateQcow2File(path);
+ } catch (RuntimeException e) {
+ throw new CloudRuntimeException("The volume file at path " +
path + " is not a valid QCOW2");
Review Comment:
The error message should include the underlying validation failure details.
Consider including the original exception message: `"The volume file at path "
+ path + " is not a valid QCOW2: " + e.getMessage()`
```suggestion
throw new CloudRuntimeException("The volume file at path " +
path + " is not a valid QCOW2: " + e.getMessage(), e);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
