yahoo-eng-team team mailing list archive

[OpenStack Infra <1533834@xxxxxxxxxxxxxxxxxx>]

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

commit c1a61fcc37a06f7d25b86c5ace47e9d68008ddff
Author: Dan Smith <dansmith@xxxxxxxxxx>
Date:   Wed Jan 13 13:12:20 2016 -0800

    Fix undetected races when getting BDMs by volume id
    
    This deprecates the BDM.get_by_volume_id() method and introduces two new ones.
    The existing method will fail to detect that multiple instances can have BDMs
    pointing at the same volume. This could happen via race, and will be normal
    behavior when we get volume multi-attach support.
    
    Instead of this method which takes instance as an optional parameter, a new
    method is introduced which requires the property when searching for a single
    BDM for a volume. Another method is added that allows us to fetch BDMs for
    volumes where there should be only one instance attached. This method checks
    that the single-owner consistency is correct, or fails otherwise.
    
    Closes-Bug: #1533834
    Co-Authored-By: Ildiko Vancsa <ildiko.vancsa@xxxxxxxxxxxx>
    Co-Authored-By: Krisztian Gacsal <krisztian.gacsal@xxxxxxxxxxxx>
    Change-Id: I06cdeef38aeb3f3e65dfc0ced8a75960b5d57f49


** Changed in: nova
       Status: In Progress => Fix Released

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to OpenStack Compute (nova).
https://bugs.launchpad.net/bugs/1533834

Title:
  nova.objects.BlockDeviceMapping.get_by_volume_id can be racy and
  should be deprecated

Status in OpenStack Compute (nova):
  Fix Released

Bug description:
  The method here:

  https://github.com/openstack/nova/blob/9f2e0ea1ce2792505cdae6e8f3cce9e32845ea64/nova/objects/block_device.py#L200

  Allows getting a BDM with or without an instance_uuid.

  There is no unique constraint on the block_device_mappings table, so
  we could potentially have a race here where some code calls this
  without an instance uuid (which there are several in the compute API
  and manager) and we could have multiple BDMs mapped to the same
  volume_id in the database. We get one back and the .first() call in
  the DB API hides that we're corrupt:

  https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L4035

  So what we need to do is be explicit about calling this kind of
  method. This is also related to the multiattach series here:

  https://review.openstack.org/#/q/topic:bp/volume-multi-attach

  Because after that series we'll be able to associate the same
  volume_id to multiple instances (by design).

  So we need to:

  1. deprecate the existing bdm.get_by_volume_id method

  2. add a new method which takes volume_id and instance_uuid (not
  optional) and returns a single BDM for those.

  3. add a new method which takes only volume_id (no instance uuid) and
  if we get multiple BDMs back from the database, it raises an error
  because we should only get one, else the DB is corrupt.

  4. change all of code that's calling (1) to all the new methods in (2)
  or (3).

  This should help us move forward on the multiattach blueprint in the
  Newton release.

To manage notifications about this bug go to:
https://bugs.launchpad.net/nova/+bug/1533834/+subscriptions