Liron Aravot has posted comments on this change.

Change subject: core: fix removal of lun disk with no vm (#841265)
......................................................................


Patch Set 1: (2 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
the client already knows what type of disk he wants to remove, he has the 
entity and it's type.
if we don't want to allow explicit invocation of the client by the disk type, 
we can have the client execute a command which will it's execute would initiate 
the correct command for the disk. anyway - i don't think that it's right to 
have one class that handles few cases and has IF statement to differ between 
them in a large number of methods.

Line 148:     // in DB to the vms list and may perform the operation on an 
outdated list
the TODOs will be removed to another patch - if there is any chance for race i 
think that we need to eliminate it..you could say that for all of race 
conditions.

--
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

Reply via email to