Michael Kublin has posted comments on this change. Change subject: core: fix removal of lun disk with no vm (#841265) ......................................................................
Patch Set 1: 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/RemoveDiskCommand.java Line 53: // it should be separated to different classes that will handle different scenarios Why? , so client will need to know which type of disk it wants remove and will call for different commands? Why client will send to us information that we know? Line 148: // in DB to the vms list and may perform the operation on an outdated list Don't add TODO to code or fix it or don't write it, also how these related to bug? By the way we are also acquiring an exclusive lock on disk, so if race - it is a race with very small probability Line 305: // that was processed on the CanDoAction phase. If needed verify, don't write TODO, also why, disk should be moved to ImageLocked if it is image, if it is LUN disk endVmCommand() is not called at all. Also how TODO related to problem that u are trying to solve -- To view, visit http://gerrit.ovirt.org/6566 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iad5a09962da87d9e3f8d06685daf2f64ed751fd3 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Aravot <[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: Michael Kublin <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
