Ayal Baron has posted comments on this change.
Change subject: backend: check image status before ExportRepoImage
......................................................................
Patch Set 1: Code-Review-1
(2 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportRepoImageCommand.java
Line 236: }
Line 237: }
Line 238:
Line 239: DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(Arrays.asList(getDiskImage()));
Line 240: if (!validate(diskImagesValidator.diskImagesNotIllegal())
! notIllegal? i.e not not not legal?
seriously?
diskImagesNotIllegal needs to be renamed asap.
then this would become:
!validate(diskImagesValidator.diskImagesLegal)
Shorter and much much much more legible
Line 241: ||
!validate(diskImagesValidator.diskImagesNotLocked())) {
Line 242: return false;
Line 243: }
Line 244:
Line 237: }
Line 238:
Line 239: DiskImagesValidator diskImagesValidator = new
DiskImagesValidator(Arrays.asList(getDiskImage()));
Line 240: if (!validate(diskImagesValidator.diskImagesNotIllegal())
Line 241: ||
!validate(diskImagesValidator.diskImagesNotLocked())) {
! notLocked? 1. same rename
2. here I cannot understand if you return false when they're locked or when
they're not locked.
Either way, checking lock using 'if' is wrong as you know. But since you
obviously know this I must be missing something here?
If you need the lock to perform the op then take it (nonblocking if you want to
fail when locked).
Line 242: return false;
Line 243: }
Line 244:
Line 245: return true;
--
To view, visit http://gerrit.ovirt.org/20871
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0957a05db4a0fdcddf56a63b0a3da0b4d27db6d
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
