Code review comment for lp:~nttdata/nova/kvm-pause-suspend

Revision history for this message
justinsb (justin-fathomdb) wrote :

This looks good, except for the exception handling logic in resume.

I think you should check for the exact error code when you've got the exception, rather than assuming that every exception is the one you expect (e.g. unexpected exceptions are thrown when the libvirt daemon isn't started, or when you don't have the right permissions).

Also, if there is a specific case you're trying to handle, can you please add a comment to the code so that everyone knows what it is. In general, I'm not sure we should be putting extra logic into the resume function - it just feels like it's the wrong layer. But I could be wrong - what is the situation you're handling here?

review: Needs Information

« Back to merge proposal