the pci deivce assigned to instance is inconsistent with DB record when restarting nova-compute

Bug #1415768 reported by Rui Chen
20
This bug affects 3 people
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Fix Released
High
Paul Murray
Juno
Fix Released
High
Nikola Đipanov

Bug Description

After restarting nova-compute process, I found that the pci device assigned to instance in libvirt.xml was different with the record in 'pci_devices' DB table.

Every time nova-compute was restarted, pci_tracker.allocations was reset to empty dict, it didn't contain the pci devices had been allocated to instances, so some pci devices would be reallocated to the instances, and record these pci into DB, maybe they was inconsistent with the libvirt.xml.

IOW, nova-compute would reallocated the pci device for the instance with pci request when restarting.

See details:
http://git.openstack.org/cgit/openstack/nova/tree/nova/compute/resource_tracker.py#n347

This is a probabilistic problem, not always can be reproduced. If the instance have a lot of pci devices, it happen more.

Face this bug in kilo master.

Rui Chen (kiwik-chenrui)
Changed in nova:
assignee: nobody → Rui Chen (kiwik-chenrui)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (master)

Fix proposed to branch: master
Review: https://review.openstack.org/151528

Changed in nova:
status: New → In Progress
tags: added: pci-passthrough
Changed in nova:
importance: Undecided → Low
Changed in nova:
assignee: Rui Chen (kiwik-chenrui) → Ed Leafe (ed-leafe)
Changed in nova:
assignee: Ed Leafe (ed-leafe) → Rui Chen (kiwik-chenrui)
Changed in nova:
assignee: Rui Chen (kiwik-chenrui) → Paul Murray (pmurray)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (master)

Reviewed: https://review.openstack.org/148904
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=c3ffcd18d9fb1d999d6fa360a811b4c7fdcaba13
Submitter: Jenkins
Branch: master

commit c3ffcd18d9fb1d999d6fa360a811b4c7fdcaba13
Author: Paul Murray <email address hidden>
Date: Tue Aug 26 18:31:30 2014 +0200

    Move ComputeNode creation at init stage in ResourceTracker

    This is the first step in a series refactoring code that uses
    ResourceTracker.compute_node so that it can later be changed
    to be a ComputeNode object. Note that in this patch compute_node
    is still a dict.

    The main refactor in this patch is to move initialization of the
    compute_node property to a new method called _init_compute_node()
    That is called near the beginning of update_available_resource().

    At the moment the update methods use a method parameter called
    resources that is either the resources data structure obtained from
    the virt driver or the compute_node property, depending on how it is
    called. The result is always copied into the compute_node property.
    This change initialises the compute_node property and creates the
    compute node record as an initialisation step.

    Moving the initialization of the compute_node property to the start
    of update_available_resources() paves the way for the next patch in
    this series to use it consistently in the update methods. The
    code will then be ready to introduce the ComputeNode object.

    This patch also fixes bug 1415768

    Co-Authored-By: Sylvain Bauza <email address hidden>
    Co-Authored-By: Ed Leafe <email address hidden>

    Closes-bug #1415768

    Change-Id: Ic04af76c3835a5bf63a42163d0335d6c7e26d68a

Changed in nova:
status: In Progress → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on nova (master)

Change abandoned by Rui Chen (<email address hidden>) on branch: master
Review: https://review.openstack.org/151528
Reason: Replace by https://review.openstack.org/#/c/148904/

Thierry Carrez (ttx)
Changed in nova:
milestone: none → kilo-rc1
status: Fix Committed → Fix Released
tags: added: juno-backport-potential
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (stable/juno)

Fix proposed to branch: stable/juno
Review: https://review.openstack.org/173226

Revision history for this message
Ihar Hrachyshka (ihar-hrachyshka) wrote :

Full disclosure: I was guilty to discuss the bug intent with the backport proposer thru private channels since it was not clear what's the reason to nominate the bug for Juno while it's a refactor and of Low importance. Here is the summary of the discussion for other's reference.

The failure scenario is as follows: nova-compute is started with no pci devices, then a device is assigned to it, and db is populated with proper records; then the service is restarted; _update_available_resource is called for the node, and it goes directly to pci_manager.PciDevTracker initialization, passing node_id argument as None, since self.compute_node is not yet initialized (it will be initialized later in the method, by calling to _sync_compute_node, but it will be too late). So manager.PciDevTracker is initialized with empty node_id, so pci_devs are not initialized at all, meaning all pci devices that were created before the service restart are lost.

The fix moves the code that initializes self.compute_node to before manager.PciDevTracker initialization, so that node_id is always passed properly, meaning that self.pci_devs are actually loaded from db and reused.

The end result of the bug is that nova-compute forgets about any pci devices that were created before restart, and also fails to start the service at all. So pci passthrough users are busted, meaning they cannot rely on the service to come up after any restart, or crash. Which is a really major issue for those users.

With that in mind, I now believe the backport should be merged.

Changed in nova:
importance: Low → High
Alan Pevec (apevec)
tags: removed: juno-backport-potential
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to nova (stable/juno)

Reviewed: https://review.openstack.org/173226
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=6c4f6fb466cce6b02087418055ff56c41679db79
Submitter: Jenkins
Branch: stable/juno

commit 6c4f6fb466cce6b02087418055ff56c41679db79
Author: Paul Murray <email address hidden>
Date: Tue Aug 26 18:31:30 2014 +0200

    Move ComputeNode creation at init stage in ResourceTracker

    This is the first step in a series refactoring code that uses
    ResourceTracker.compute_node so that it can later be changed
    to be a ComputeNode object. Note that in this patch compute_node
    is still a dict.

    The main refactor in this patch is to move initialization of the
    compute_node property to a new method called _init_compute_node()
    That is called near the beginning of update_available_resource().

    At the moment the update methods use a method parameter called
    resources that is either the resources data structure obtained from
    the virt driver or the compute_node property, depending on how it is
    called. The result is always copied into the compute_node property.
    This change initialises the compute_node property and creates the
    compute node record as an initialisation step.

    Moving the initialization of the compute_node property to the start
    of update_available_resources() paves the way for the next patch in
    this series to use it consistently in the update methods. The
    code will then be ready to introduce the ComputeNode object.

    This patch also fixes bug 1415768

    Co-Authored-By: Sylvain Bauza <email address hidden>
    Co-Authored-By: Ed Leafe <email address hidden>

    (cherry picked from commit c3ffcd18d9fb1d999d6fa360a811b4c7fdcaba13)

    Conflicts:
     nova/compute/resource_tracker.py
     nova/tests/pci/test_pci_manager.py
     nova/tests/unit/compute/test_tracker.py

    Closes-bug #1415768
    Closes-bug #1383465

    Change-Id: Ic04af76c3835a5bf63a42163d0335d6c7e26d68a

Thierry Carrez (ttx)
Changed in nova:
milestone: kilo-rc1 → 2015.1.0
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.