Code review comment for lp://qastaging/~justin-fathomdb/nova/restart-instance

Thanks termie ... agree with your style notes. (In my defense, some of them are on docstring moved over from the fake)

The commented out init_host in xen_api is because it now inherits the functionality from the base class, so the function (which was a stub function) should no longer be there. However, there were some implementor's notes that weren't my place to delete, so I didn't.

The big issue is around PersistenceMode. While at the moment there are only two values (so it should be a boolean), I can well imagine that we might have more. For example:

On guest crash, should we restart?
On host crash, should we auto-migrate or shutdown?

etc

So can we focus first on what the behaviour should be?

The idea was that PersistenceMode was going to have functions that would define each 'decision point' (like auto-restart on host start?), so that it was clear what the code was doing, and we weren't linking together decisions that weren't actually required to be linked simply so that we could use a simpler flag. Right now, there's only one such function (autostart_on_host_boot), but that's because our desired behaviours still aren't very clear to me.

What I might do is extend PersistenceMode so that it does define all these decision points as functions, and then we can clearly see the decisions we have to make, and whether we want a boolean or an enum or multiple booleans. Sound good?