winterhazel commented on code in PR #6589: URL: https://github.com/apache/cloudstack/pull/6589#discussion_r1731331495
########## plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java: ########## @@ -1057,7 +1111,7 @@ private Map<VirtualDisk, VolumeVO> getDisksMapping(Backup backup, List<VirtualDi throw new CloudRuntimeException("Failed to find the volumes details from the VM backup"); } - List<Backup.VolumeInfo> backedUpVolumes = vm.getBackupVolumeList(); + List<Backup.VolumeInfo> backedUpVolumes = backup.getBackupVolumeList(); Review Comment: Please delete lines 1119-1131 (marking the "extra volumes" as removed). They are not necessary anymore with this change, and can result in inconsistent removed volumes in the `Ready` state if the restoration process fails after marking the volumes as removed and before synchronizing them (such as by detaching the root volume before restoring the backup). Currently, everything that is done in these lines will be undone in `com.cloud.hypervisor.guru.VMwareGuru#updateVolume`. By deleting these lines, the workflow will remain the same for all volumes, with the exception that we will not need to undo the volume removal. These lines do not affect the workflow for extra volumes either; they will be detached independently. -- 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