Copilot commented on code in PR #13271:
URL: https://github.com/apache/cloudstack/pull/13271#discussion_r3316644272
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -2316,9 +2316,11 @@ private VolumeVO resizeVolumeInternal(VolumeVO volume,
DiskOfferingVO newDiskOff
HypervisorType hypervisorType =
_volsDao.getHypervisorType(volume.getId());
if (userVm != null) {
- if (volume.getVolumeType().equals(Volume.Type.ROOT) &&
userVm.getPowerState() != VirtualMachine.PowerState.PowerOff && hypervisorType
== HypervisorType.VMware) {
- logger.error(" For ROOT volume resize VM should be in Power
Off state.");
- throw new InvalidParameterValueException("VM current state is
: " + userVm.getPowerState() + ". But VM should be in " +
VirtualMachine.PowerState.PowerOff + " state.");
+ if (Volume.Type.ROOT.equals(volume.getVolumeType())
+ && ! State.Stopped.equals(userVm.getState())
+ && HypervisorType.VMware.equals(hypervisorType)) {
+ logger.error("For ROOT volume resize VM should be in Stopped
state.");
+ throw new InvalidParameterValueException("VM current state is
: " + userVm.getState() + ". But VM should be in " + State.Stopped + " state.");
}
Review Comment:
Add unit tests covering the VMware ROOT-volume offering-change/resize path
where VM power_state may lag behind: ensure the validation allows operation
when VM state is Stopped even if powerState is not PowerOff, and still rejects
when VM state is not Stopped.
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -2316,9 +2316,11 @@ private VolumeVO resizeVolumeInternal(VolumeVO volume,
DiskOfferingVO newDiskOff
HypervisorType hypervisorType =
_volsDao.getHypervisorType(volume.getId());
if (userVm != null) {
- if (volume.getVolumeType().equals(Volume.Type.ROOT) &&
userVm.getPowerState() != VirtualMachine.PowerState.PowerOff && hypervisorType
== HypervisorType.VMware) {
- logger.error(" For ROOT volume resize VM should be in Power
Off state.");
- throw new InvalidParameterValueException("VM current state is
: " + userVm.getPowerState() + ". But VM should be in " +
VirtualMachine.PowerState.PowerOff + " state.");
+ if (Volume.Type.ROOT.equals(volume.getVolumeType())
+ && ! State.Stopped.equals(userVm.getState())
Review Comment:
There is an extra whitespace after the unary '!' operator (`!
State.Stopped...`). This is inconsistent with the surrounding code style and
can trip Checkstyle/formatting rules; remove the space to keep
`!State.Stopped.equals(...)`.
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -2316,9 +2316,11 @@ private VolumeVO resizeVolumeInternal(VolumeVO volume,
DiskOfferingVO newDiskOff
HypervisorType hypervisorType =
_volsDao.getHypervisorType(volume.getId());
if (userVm != null) {
- if (volume.getVolumeType().equals(Volume.Type.ROOT) &&
userVm.getPowerState() != VirtualMachine.PowerState.PowerOff && hypervisorType
== HypervisorType.VMware) {
- logger.error(" For ROOT volume resize VM should be in Power
Off state.");
- throw new InvalidParameterValueException("VM current state is
: " + userVm.getPowerState() + ". But VM should be in " +
VirtualMachine.PowerState.PowerOff + " state.");
+ if (Volume.Type.ROOT.equals(volume.getVolumeType())
+ && ! State.Stopped.equals(userVm.getState())
+ && HypervisorType.VMware.equals(hypervisorType)) {
+ logger.error("For ROOT volume resize VM should be in Stopped
state.");
+ throw new InvalidParameterValueException("VM current state is
: " + userVm.getState() + ". But VM should be in " + State.Stopped + " state.");
Review Comment:
The exception message has awkward spacing around the colon ("state is :").
Consider rephrasing to a cleaner, user-facing message (e.g., "VM state is <x>;
must be Stopped") to improve readability.
##########
server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java:
##########
@@ -2396,9 +2398,11 @@ private void
validateVolumeReadyStateAndHypervisorChecks(VolumeVO volume, long c
UserVmVO userVm = _userVmDao.findById(volume.getInstanceId());
if (userVm != null) {
- if (volume.getVolumeType().equals(Volume.Type.ROOT) &&
userVm.getPowerState() != VirtualMachine.PowerState.PowerOff && hypervisorType
== HypervisorType.VMware) {
- logger.error(" For ROOT volume resize VM should be in Power
Off state.");
- throw new InvalidParameterValueException("VM current state is
: " + userVm.getPowerState() + ". But VM should be in " +
VirtualMachine.PowerState.PowerOff + " state.");
+ if (Volume.Type.ROOT.equals(volume.getVolumeType())
+ && !State.Stopped.equals(userVm.getState())
+ && hypervisorType == HypervisorType.VMware) {
+ logger.error("For ROOT volume resize VM should be in Stopped
state.");
+ throw new InvalidParameterValueException("VM current state is
: " + userVm.getState() + ". But VM should be in " + State.Stopped + " state.");
Review Comment:
Same as above: the InvalidParameterValueException message includes "state is
:" with an extra space before the colon. Consider tightening the wording for a
clearer user-facing error.
--
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]
