yahoo-eng-team team mailing list archive

[Matt Riedemann <mriedem@xxxxxxxxxx>]

Public bug reported:

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.

** Affects: nova
     Importance: Medium
     Assignee: Dan Smith (danms)
         Status: Triaged


** Tags: db unified-objects volumes

** Changed in: nova
       Status: New => Triaged

** Changed in: nova
   Importance: Undecided => Medium

** Changed in: nova
     Assignee: (unassigned) => Dan Smith (danms)

-- 
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):
  Triaged

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