Arik Hadas has posted comments on this change.

Change subject: core: managed removal of memory volumes in negative flows
......................................................................


Patch Set 2:

"I don't really agree on that part, we should have the handling on one place, 
remove image command actually already have an option to not perform any db 
operations - RemoveImageParameters.getDbOperationScope() so only minor change 
might be required there instead of repeating it."

I don't mind calling RemoveImage instead of DeleteImageGroup if RemoveImage 
already supports this flow and you guys in the storage think it is better, but 
not as part of this patch. It is done this way because this code evolved from 
the code that handled hibernation volumes which used DeleteImageGroup directly 
(and RemoveImage uses as well..).

"Why is seems to you like a hack? that's the provided API, it's legitimate 
use..this is the reason for those methods to be exposed."

IIUC it is a hack to support creating tasks in the end-action phase that wasn't 
planned in the beginning. You know what, leave the history aside, I prefer not 
to use it and invoke separate command that poll the task as part of the 
infrastructure. You suggest another way that will also work but I don't see why 
it is better, so I prefer to leave it like that.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4b270ec0e1ab41cae34459dde9f9cf47b1b5bdf
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to