Code review comment for lp:~rackspace-titan/nova/instance_states

I like the fixes here and the new functionality!

A question for later, but have we ever swung back around to using an actual statemachine to enforce transitions? I think the code here all looks sound, but experience tells me that decisions based on state in the code can be fragile, at best. I'm asking because of things like the following:

519 + self.update(context,
520 + instance_id,
521 + vm_state=vm_states.SUSPENDED,
522 + task_state=task_states.RESUMING)

It doesn't really matter what state the instance was in before. It's suspend and about to resume, now. I know we didn't really enforce the state in all scenarios previously, so I guess I can't expect this patch to take care of all of those in one shot. I bring it up because it seems like in certain instances, you've chosen to try and enforce that the instance is in a good state,
but others aren't covered. Do we maybe we want to consider implementing a more statemachine-esque cleanup in a subsequent patch?

948 + # NOTE(blamar): None of the virt drivers use the 'callback' param

I wondered this myself. Good catch. Make a bug?

Great work on the tests!

I'll add my approve. Just want to touch base with you first and see what (if any) intent there is regarding my comments above.