Ayal Baron has posted comments on this change.

Change subject: core: change diskImagesNotIllegal in diskImagesAvailable
......................................................................


Patch Set 1: Code-Review+1

The name is not that good because the fundamental approach here is wrong.
Considering the current approach this patch is an improvement.

What the function actually does could be termed diskImagesValidForLocking

The patch also fixes a few places that only checked lock status but not 
legality.

2 things:
1. is there a way to make sure inside the method that the disks are locked 
in-memory by current task? because the 'if locked' part is only valid under 
that assumption.
2. after this is introduced, we can consider changing this from a validation to 
actually trying to lock the disks in db and avoid the funky semantics of 'if 
locked' and available and all that.
so method would be something like lockImagesInDb(enforceLegality)

In all cases but 1 [1] that I can think of we need to make sure that images are 
legal. In deletion however, it would be better to make it explicit 
(fenforceLegality = False) to make it clear that not checking legality was not 
an oversight vs. the current state where we do it implicitly by simply not 
checking legality and only lock status.

[1] when deleting an image we don't care if it's legal or not.

-- 
To view, visit http://gerrit.ovirt.org/20993
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2f04d33029bac5a5e179b2d9efa4d65d53768976
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: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to