OpenStack Compute (Nova)

db.instance_set_state reference in xenapi/vmops.py is wrong

Reported by Chris Behrens on 2011-09-25
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Undecided
Aaron Lee
Diablo
Undecided
Unassigned

Bug Description

There's all sorts of issues with xenapi/vmops.py's _create_vm() surrounding the code when 'VMhelper.ensure_free_mem' returns false.

1) LOG.exception() is wrong. There's no exception thrown
2) The call to db.instance_set_state should not be there. It's defined in db/api.py but is not in sqlalchemy/api.py
3) It returns None... but callers expect it to always succeed.

I think the solution is:

1) Change LOG.exception to a LOG.error
2) raise an exception instead of setting state and returning.
3) Remove instance_set_state from db/api.py

_create_vm() ends up being called from spawn() and finish_migration(). May want to look for callers of those in the compute manager and elsewhere to see that exceptions are caught properly and vm_state is set appropriately to vm_states.ERROR. There may be a separate bug filed for this, however, that aaron lee is working on.

Aaron Lee (aaron-lee) wrote :

This is very similar to 851374, and the fix is basically the same. However instead of separately logging the insufficient memory error I think it should be loged along with the rest of the errors in the api's error handler.

Changed in nova:
assignee: nobody → Aaron Lee (aaron-lee)
Aaron Lee (aaron-lee) wrote :
Changed in nova:
status: New → Fix Committed
Thierry Carrez (ttx) wrote :

Do not set to fix committed until it's merged in trunk, please

Changed in nova:
status: Fix Committed → In Progress

Reviewed: https://review.openstack.org/676
Committed: http://github.com/openstack/nova/commit/7dba1d9aa989760b190f1cf3bad2ed22bb2e2fc5
Submitter: Jenkins
Branch: master

 status fixcommitted
 done

commit 7dba1d9aa989760b190f1cf3bad2ed22bb2e2fc5
Author: Aaron Lee <email address hidden>
Date: Mon Sep 26 18:22:03 2011 -0500

    Raise InsufficientFreeMemory

    Kind of fixes bug 851374 & bug 858679

    Raises InsufficientFreeMemory if an instance can't
    start because of that. This will cause the normal
    instance failure recovery to catch this problem,
    set the state, and log the error. This also
    removes instance_set_state from db/api.py as that
    was causing these exceptions in the first place.

    Change-Id: I199aa6900890531b175e28c3b93d8dfb88e135d0

Changed in nova:
status: In Progress → Fix Committed

Reviewed: https://review.openstack.org/953
Committed: http://github.com/openstack/nova/commit/410a8df435fb424105872dc0a39f68eed902fbf2
Submitter: Jenkins
Branch: stable/diablo

 status fixcommitted
 done

commit 410a8df435fb424105872dc0a39f68eed902fbf2
Author: Aaron Lee <email address hidden>
Date: Mon Sep 26 18:22:03 2011 -0500

    Raise InsufficientFreeMemory

    Kind of fixes bug 851374 & bug 858679

    Raises InsufficientFreeMemory if an instance can't
    start because of that. This will cause the normal
    instance failure recovery to catch this problem,
    set the state, and log the error. This also
    removes instance_set_state from db/api.py as that
    was causing these exceptions in the first place.

    (cherry picked from commit 7dba1d9aa989760b190f1cf3bad2ed22bb2e2fc5)

    Change-Id: I93d4e3e2c264f520c0cd37c778a0db42eeb8345d

Thierry Carrez (ttx) on 2011-11-09
Changed in nova:
milestone: none → essex-1
Thierry Carrez (ttx) on 2011-11-17
Changed in nova:
status: Fix Committed → Fix Released

Hello Chris, or anyone else affected,

Accepted nova into oneiric-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

tags: added: verification-needed
Thierry Carrez (ttx) on 2012-04-05
Changed in nova:
milestone: essex-1 → 2012.1
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers