Copilot commented on code in PR #11351:
URL: https://github.com/apache/cloudstack/pull/11351#discussion_r2242636055
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -4192,15 +4192,7 @@ private UserVm createVirtualMachine(DataCenter zone,
ServiceOffering serviceOffe
}
DiskOfferingVO rootDiskOffering =
_diskOfferingDao.findById(rootDiskOfferingId);
- long volumesSize = 0;
- if (volume != null) {
- volumesSize = volume.getSize();
- } else if (snapshot != null) {
- VolumeVO volumeVO = _volsDao.findById(snapshot.getVolumeId());
- volumesSize = volumeVO != null ? volumeVO.getSize() : 0;
- } else {
- volumesSize = configureCustomRootDiskSize(customParameters,
template, hypervisorType, rootDiskOffering);
- }
+ long volumesSize = configureCustomRootDiskSize(customParameters,
template, hypervisorType, rootDiskOffering);
Review Comment:
The simplified logic now ignores the actual size of existing volumes and
snapshots, which could lead to data loss or corruption when the configured size
is smaller than the source volume/snapshot size. Consider adding validation to
ensure the configured size is at least as large as the source.
```suggestion
// Validate that the configured size is at least as large as the
source volume or snapshot size
if (snapshot != null) {
long sourceSize = snapshot.getSize(); // Assuming
snapshot.getSize() returns the size in bytes
if (volumesSize < sourceSize) {
throw new InvalidParameterValueException("The configured
root disk size (" + volumesSize +
" bytes) is smaller than the source snapshot size ("
+ sourceSize + " bytes).");
}
} else if (volume != null) {
long sourceSize = volume.getSize(); // Assuming volume.getSize()
returns the size in bytes
if (volumesSize < sourceSize) {
throw new InvalidParameterValueException("The configured
root disk size (" + volumesSize +
" bytes) is smaller than the source volume size (" +
sourceSize + " bytes).");
}
}
```
##########
plugins/storage/volume/storpool/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/StorPoolDownloadTemplateCommandWrapper.java:
##########
@@ -103,9 +103,10 @@ public CopyCmdAnswer execute(final
StorPoolDownloadTemplateCommand cmd, final Li
final QemuImgFile srcFile = new QemuImgFile(srcDisk.getPath(),
srcDisk.getFormat());
final QemuImg qemu = new QemuImg(cmd.getWaitInMillSeconds());
- StorPoolStorageAdaptor.resize(
Long.toString(srcDisk.getVirtualSize()), dst.getPath());
if (dst instanceof TemplateObjectTO) {
+ StorPoolStorageAdaptor.resize(
Long.toString(srcDisk.getVirtualSize()), dst.getPath());
+
Review Comment:
The resize operation is now only called for TemplateObjectTO instances, but
there's no validation that srcDisk.getVirtualSize() is appropriate for the
destination. Consider adding size validation before calling resize.
```suggestion
if (dst.getSize() != null && dst.getSize() <
srcDisk.getVirtualSize()) {
final String error = "Destination size " + dst.getSize()
+ " is smaller than source virtual size " + srcDisk.getVirtualSize();
SP_LOG(error);
return new CopyCmdAnswer(error);
}
StorPoolStorageAdaptor.resize(Long.toString(srcDisk.getVirtualSize()),
dst.getPath());
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
