MAAS

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

vm-pause-state-bug-1394382
Merge into trunk

Proposed by Julian Edwards on 2014-12-02

Status:

Merged

Approved by:

Julian Edwards on 2014-12-02

Approved revision:

no longer in the source branch.

Merged at revision:

3395

Proposed branch:

lp://staging/~julian-edwards/maas/vm-pause-state-bug-1394382

Merge into:

lp://staging/~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://staging/~julian-edwards/maas/vm-pause-state-bug-1394382

Critical

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Graham Binns (community)		2014-12-02	Approve on 2014-12-02
Review via email: mp+243361@code.staging.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.

Revision history for this message

Graham Binns (gmb) wrote on 2014-12-02:

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 2014-12-02:

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 2014-12-02:

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.