On Tue, Apr 20, 2010 at 10:03:19AM +0100, Guido Trotter wrote: > On Tue, Apr 20, 2010 at 9:31 AM, Iustin Pop <[email protected]> wrote: > > On Mon, Apr 19, 2010 at 04:54:37PM +0100, Guido Trotter wrote: > >> Currently some hypervisors (namely kvm) need to do some cleanup after > >> making sure an instance is stopped. With the moving of the retry cycle > >> in backend those cleanups were never done. In order to solve this we add > >> a new optional hypervisor function, CleanupInstance, which gets called > >> at the end of the shutdown procedure, and which interested hypervisors > >> can implement to be sure not to miss cleanup operations. > >> > >> Signed-off-by: Guido Trotter <[email protected]> > > > > NAK. > > > > See my patch from yesterday which fixes this by removing the retry cycle > > from backend. > > > > The rationale for not adding a separate cleanup step (which I thought > > about) is that: > > > > - you cannot know just from instance list whether to run or not the > > Cleanup > > - the existence of a new state (shutdown but not cleaned up) should be > > reflected all across to the master and command line tools, and we > > don't want that > > - it is cleaner to just make sure StopInstance does all the neeed > > cleanup, rather than introduce a new step > > > > Let's discuss! > > > > Ok, that solution works well for me! :)
Heh, I wanted to discuss this, as it's not quite trivial. I finally got to test my patch with burnin, and at least for Xen, the removal of the Retry from backend breaks things as the shutdown is not immediate. So: - we do need somewhere a retry logic for actually killing the VM (but not necessarily other cleanup steps) - we do need a cleanup step I think that both of these should be in the hypervisor code, since backend doesn't know about each hypervisor's needs. So we can either: - go with your patch as is, but we might need to add the Cleanup phase to the retry cycle - move the retry cycle inside hv_base, and abstract the actual "is instance running" and "terminate instance" in a generic way Comments? iustin -- Subscription settings: http://groups.google.com/group/ganeti-devel/subscribe?hl=en
