Merge lp:~julian-edwards/maas/vm-pause-state-bug-1394382 into lp:~maas-committers/maas/trunk

Proposed by Julian Edwards
Status: Merged
Approved by: Julian Edwards
Approved revision: no longer in the source branch.
Merged at revision: 3395
Proposed branch: lp:~julian-edwards/maas/vm-pause-state-bug-1394382
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 41 lines (+22/-5)
1 file modified
src/provisioningserver/drivers/hardware/virsh.py (+22/-5)
To merge this branch: bzr merge lp:~julian-edwards/maas/vm-pause-state-bug-1394382
Reviewer Review Type Date Requested Status
Graham Binns (community) Approve
Review via email: mp+243361@code.launchpad.net

Commit message

Handle more of the known states in which a virsh machine can be, instead of crashing.

Description of the change

The existing tests cover the functionality, nothing has changed other than adding the lookup table with extra states.

To post a comment you must log in.
Revision history for this message
Graham Binns (gmb) wrote :

It's worth adding a tests that shows that for each VM state in VirshVMState, power_state_virsh() returns a value that makes sense. Other than that, LGTM.

review: Approve
Revision history for this message
Julian Edwards (julian-edwards) wrote :

On Tuesday 02 Dec 2014 07:21:07 you wrote:
> Review: Approve
>
> It's worth adding a tests that shows that for each VM state in VirshVMState,
> power_state_virsh() returns a value that makes sense. Other than that,
> LGTM.

Thanks for the review.

I'm not sure what that would be really testing. There are already tests for
the happy path and the unhappy path. The rest of it is just mappings that
I've added.

Revision history for this message
Graham Binns (gmb) wrote :

On 2 December 2014 at 08:03, Julian Edwards <email address hidden>
wrote:

>
> I'm not sure what that would be really testing. There are already tests
> for
> the happy path and the unhappy path. The rest of it is just mappings that
> I've added.
>

Ah, so there are. In my morning-brain state I didn't see them. Fair enough.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/provisioningserver/drivers/hardware/virsh.py'
2--- src/provisioningserver/drivers/hardware/virsh.py 2014-09-15 14:28:28 +0000
3+++ src/provisioningserver/drivers/hardware/virsh.py 2014-12-02 05:59:23 +0000
4@@ -33,6 +33,24 @@
5 class VirshVMState:
6 OFF = "shut off"
7 ON = "running"
8+ NO_STATE = "no state"
9+ IDLE = "idle"
10+ PAUSED = "paused"
11+ IN_SHUTDOWN = "in shutdown"
12+ CRASHED = "crashed"
13+ PM_SUSPENDED = "pmsuspended"
14+
15+
16+VM_STATE_TO_POWER_STATE = {
17+ VirshVMState.OFF: "off",
18+ VirshVMState.ON: "on",
19+ VirshVMState.NO_STATE: "off",
20+ VirshVMState.IDLE: "off",
21+ VirshVMState.PAUSED: "off",
22+ VirshVMState.IN_SHUTDOWN: "on",
23+ VirshVMState.CRASHED: "off",
24+ VirshVMState.PM_SUSPENDED: "off",
25+ }
26
27
28 class VirshError(Exception):
29@@ -238,8 +256,7 @@
30 if state is None:
31 raise VirshError('Failed to get domain: %s' % machine)
32
33- if state == VirshVMState.OFF:
34- return 'off'
35- elif state == VirshVMState.ON:
36- return 'on'
37- raise VirshError('Unknown state: %s' % state)
38+ try:
39+ return VM_STATE_TO_POWER_STATE[state]
40+ except KeyError:
41+ raise VirshError('Unknown state: %s' % state)