Allon Mureinik has posted comments on this change.
Change subject: core: Allow remove snapshot w/o disks (#825809)
......................................................................
Patch Set 14: I would prefer that you didn't submit this
(3 inline comments)
see answers to review inline - I will submit a new patchset with agreed fixed.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java
Line 64: if (!hasImages()) {
Look at the top of the class - it has @LockIdNameAttribute defined on it.
This means a memory lock will be taken before the canDoAction is even started.
Line 186: hasImages(), hasImages(), true, true, true, true,
null);
Because this is a bad. bad name for an overly coupled method in ImagesHandler,
which also performs several checks on the VM itself. Once it's decoupled, this
can (read: should) be refactored.
However, you are right - there are several checks here that currently have
"true" and should have "hasImages()".
Line 190: if (!hasImages()) {
Done
--
To view, visit http://gerrit.ovirt.org/6119
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie65a93e75419b8b63e6c7e46fd9137f3db7684c4
Gerrit-PatchSet: 14
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
