Allon Mureinik has posted comments on this change. Change subject: core: Consolidate vm down check when removing disk ......................................................................
Patch Set 1: (2 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java Line 79: Line 80: buildSharedLockMap(); Line 81: Line 82: return acquireLockInternal() && Line 83: validateAllVmsForDiskAreDown() && in psuedo-code, what happened here is the following: old code: if diskStorageType == image someValidations() if disk is a vm disk and not a template disk: isVmDown() -- done inside ImagesHandler someOtherValidations() else if disk is a vm disk and not a template disk: isVmDown() new code: if disk is a vm disk and not a template disk: isVmDown() if diskStorageType == image someValidations() someOtherValidations() there is a slight difference that the vm state is checked earlier, before all the other image validations, but IMHO it's a good change: 1. It's cheap to check, so it can save expensive database access if it fails fast 2. it's a use-action to fix Line 84: disk.getDiskStorageType() == DiskStorageType.IMAGE ? Line 85: canRemoveDiskBasedOnImageStorageCheck() : true; Line 86: } Line 87: Line 81: Line 82: return acquireLockInternal() && Line 83: validateAllVmsForDiskAreDown() && Line 84: disk.getDiskStorageType() == DiskStorageType.IMAGE ? Line 85: canRemoveDiskBasedOnImageStorageCheck() : true; yeah, I was getting a bit lazy, will fix. Line 86: } Line 87: Line 88: private boolean validateAllVmsForDiskAreDown() { Line 89: if (disk.getVmEntityType() == VmEntityType.VM) { -- To view, visit http://gerrit.ovirt.org/11013 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia1a23e1f8d129bf19c1d217c7b9201dc17a2d2c0 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches