Ayal Baron 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)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java
Line 64: if (!hasImages()) {
not getting a (memory) lock before even checking that the vm is down is racy
(in fact all the canDoAction checks are invalid if there is no lock to prevent
things from changing between then and now)
Line 186: hasImages(), hasImages(), true, true, true, true,
null);
why are you calling validateImages at all if there are no images?
Line 190: if (!hasImages()) {
you should check hasImages in canDoAction and if not skip all these validations
--
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 <amure...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@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: Roy Golan <rgo...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
