LGTM, thanks On Wed, 18 Mar 2015 at 14:28 'Hrvoje Ribicic' via ganeti-devel < [email protected]> wrote:
> Xen's utilities used by Ganeti to report instance state can have > interesting quirks, such as reporting that an instance resides in > different and somewhat contradictory states. > > This patch improves the situation by ignoring the paused state, and > encoding some of the more exotics combinations that may appear. > > Signed-off-by: Hrvoje Ribicic <[email protected]> > --- > lib/hypervisor/hv_xen.py | 45 > ++++++++++++++++++++++++++-- > test/py/ganeti.hypervisor.hv_xen_unittest.py | 30 +++++++++++++++++++ > 2 files changed, 72 insertions(+), 3 deletions(-) > > diff --git a/lib/hypervisor/hv_xen.py b/lib/hypervisor/hv_xen.py > index caa5ac7..1da5acb 100644 > --- a/lib/hypervisor/hv_xen.py > +++ b/lib/hypervisor/hv_xen.py > @@ -182,7 +182,7 @@ def _IsInstanceRunning(instance_info): > """Determine whether an instance is running. > > An instance is running if it is in the following Xen states: > - running, blocked, or paused. > + running, blocked, paused, or dying (about to be destroyed / shutdown). > > For some strange reason, Xen once printed 'rb----' which does not make > any > sense because an instance cannot be both running and blocked. > Fortunately, > @@ -193,6 +193,9 @@ def _IsInstanceRunning(instance_info): > to be scheduled to run. > http://old-list-archives.xenproject.org/xen-users/2007-06/msg00849.html > > + A dying instance is about to be removed, but it is still consuming > resources, > + and counts as running. > + > @type instance_info: string > @param instance_info: Information about instance, as supplied by Xen. > @rtype: bool > @@ -202,15 +205,51 @@ def _IsInstanceRunning(instance_info): > return instance_info == "r-----" \ > or instance_info == "rb----" \ > or instance_info == "-b----" \ > - or instance_info == "--p---" \ > + or instance_info == "-----d" \ > or instance_info == "------" > > > def _IsInstanceShutdown(instance_info): > - return instance_info == "---s--" > + """Determine whether the instance is shutdown. > + > + An instance is shutdown when a user shuts it down from within, and we > do not > + remove domains to be able to detect that. > + > + The dying state has been added as a precaution, as Xen's status > reporting is > + weird. > + > + """ > + return instance_info == "---s--" \ > + or instance_info == "---s-d" > + > + > +def _IgnorePaused(instance_info): > + """Removes information about whether a Xen state is paused from the > state. > + > + As it turns out, an instance can be reported as paused in almost any > + condition. Paused instances can be paused, running instances can be > paused for > + scheduling, and any other condition can appear to be paused as a result > of > + races or improbable conditions in Xen's status reporting. > + As we do not use Xen's pause commands in any way at the time, we can > simply > + ignore the paused field and save ourselves a lot of trouble. > + > + Should we ever use the pause commands, several samples would be needed > before > + we could confirm the domain as paused. > + > + """ > + return instance_info.replace('p', '-') > > > def _XenToHypervisorInstanceState(instance_info): > + """Maps Xen states to hypervisor states. > + > + @type instance_info: string > + @param instance_info: Information about instance, as supplied by Xen. > + @rtype: L{hv_base.HvInstanceState} > + > + """ > + instance_info = _IgnorePaused(instance_info) > + > if _IsInstanceRunning(instance_info): > return hv_base.HvInstanceState.RUNNING > elif _IsInstanceShutdown(instance_info): > diff --git a/test/py/ganeti.hypervisor.hv_xen_unittest.py b/test/py/ > ganeti.hypervisor.hv_xen_unittest.py > index a2f14c7..7247e74 100755 > --- a/test/py/ganeti.hypervisor.hv_xen_unittest.py > +++ b/test/py/ganeti.hypervisor.hv_xen_unittest.py > @@ -169,6 +169,36 @@ class TestParseInstanceList( > testutils.GanetiTestCase): > self.fail("Exception was not raised") > > > +class TestInstanceStateParsing(unittest.TestCase): > + def testRunningStates(self): > + states = [ > + "r-----", > + "r-p---", > + "rb----", > + "rbp---", > + "-b----", > + "-bp---", > + "-----d", > + "--p--d", > + "------", > + "--p---", > + ] > + for state in states: > + self.assertEqual(hv_xen._XenToHypervisorInstanceState(state), > + hv_base.HvInstanceState.RUNNING) > + > + def testShutdownStates(self): > + states = [ > + "---s--", > + "--ps--", > + "---s-d", > + "--ps-d", > + ] > + for state in states: > + self.assertEqual(hv_xen._XenToHypervisorInstanceState(state), > + hv_base.HvInstanceState.SHUTDOWN) > + > + > class TestGetInstanceList(testutils.GanetiTestCase): > def _Fail(self): > return utils.RunResult(constants.EXIT_FAILURE, None, > -- > 2.2.0.rc0.207.ga3a616c > >
