Michael Kublin has posted comments on this change.

Change subject: engine:VM with remaining disks will not be deleted.(#822051)
......................................................................


Patch Set 3: 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/RemoveVmCommand.java
Line 32:     private boolean hasImages;
In java, the boolean value by default false, no need to set explicitly to false

Line 228:                     // If the are no disks the VM can be safely 
removed from the DB.
Will never be true if vm has at least one lun disk, because of lun disk removed 
during RemoveVmFromDb().
Also the following code is called from two places , one from EndWithFailure and 
also from EndSuccessfully, maybe it can be written different

Line 232:                         // If the VM still has images related to it, 
change their status to Illegal, so the user can remove the VM again.
I think that if you are doing update it can be done at the same transaction?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic59918037a87a2c169c4410d297de81a03ab6848
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to