concurrent backups of the same volume are possible

[Tom Barron]

In the cinder backup API create method, there is a window between the check on volume state at

https://github.com/openstack/cinder/blob/master/cinder/backup/api.py#L131

and the change of volume state to 'backing-up' at

https://github.com/openstack/cinder/blob/master/cinder/backup/api.py#L199

in which multiple requests to backup the same available volume may enter. They will then all successfully set volume state
to 'backing-up' and proceed concurrently, despite the apparent intention of the original check at line 131 to not allow
a backup of a volume already being backed up.

We can see this if we insert a time.sleep(30) immediately before the
    self.db.volume_update(context, volume_id, {'status': 'backing-up'})
line, restart the api process, and issue a command like the
following:
     $ cinder backup-create 077e385f-0b08-480b-bfe4-eb31ff4de963 & cinder backup-create 077e385f-0b08-480b-bfe4-eb31ff4de963 & cinder backup-create 077e385f-0b08-480b-bfe4-eb31ff4de963 & cinder backup-create 077e385f-0b08-480b-bfe4-eb31ff4de963 & cinder backup-create 077e385f-0b08-480b-bfe4-eb31ff4de963 & cinder backup-create 077e385f-0b08-480b-bfe4-eb31ff4de963 & cinder backup-create 077e385f-0b08-480b-bfe4-eb31ff4de963 & cinder backup-create 077e385f-0b08-480b-bfe4-eb31ff4de963 &

If we then run 'cinder backup-list' we see these backups on the same volume running concurrently. We've just magnified a race window that is already there.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/218012
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=938cb0c5ab11638a07c22e9db2b0d241223d9d9f
Submitter: Jenkins
Branch: master

commit 938cb0c5ab11638a07c22e9db2b0d241223d9d9f
Author: Gorka Eguileor <email address hidden>
Date: Fri Aug 28 01:14:02 2015 +0200

    Move get_by_id to CinderObject

    Currently each Versioned Object needs to implement its own get_by_id,
    with this patch they don't need anymore, since it will be included in
    the base class CinderObject and it will work for all the objects.

    This will help for other things like having a refresh method or
    conditional updates in the objects.

    Related-Bug: #1490944
    Related-Bug: #1238093
    Related-Bug: #1490946
    Related-Bug: #1469659
    Change-Id: I355dc8eaefed93003533ee083f74acd1315f057e

[OpenStack Infra (hudson-openstack)]

Change abandoned by Gorka Eguileor (<email address hidden>) on branch: master
Review: https://review.openstack.org/218936

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/205834
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=1070c28774a74f3e482cb21dbd98f031010ec4a5
Submitter: Jenkins
Branch: master

commit 1070c28774a74f3e482cb21dbd98f031010ec4a5
Author: Gorka Eguileor <email address hidden>
Date: Sat Jul 25 12:34:28 2015 +0200

    Add atomic conditional updates to objects

    To allow atomic state changes across Cinder services we need to
    implement a way to easily do compare-and-swap.

    This patch adds methods to allow compare-and-swap on DB models and on
    Cinder Versioned Objects as well.

    Conditions for the compare part of the update can consist of:
    - Inclusion: status == 'available'
    - Exclusion: status != 'in-use'
    - Multi-inclusion: status in ('available', 'error')
    - Multi-exclusion: status not in ('attaching', 'in-use')
    - Sqlalchemy filters

    A complete example of usage would be the compare-and-swap used in volume
    delete requests that has to take in consideration not only the status
    but the attach and migration status as well as the volume not having
    snapshots:

     now = timeutils.utcnow()
     expected = {'attach_status': db.Not('attached'),
                 'migration_status': None,
                 'consistencygroup_id': None}
     good_status = ('available', 'error', 'error_restoring',
                    'error_extending')
     if not force:
         expected.update(status=good_status)

     # Volume cannot have snapshots if we want to delete it
     filters = [~sql.exists().where(models.Volume.id ==
                                    models.Snapshot.volume_id)]

     updated = vol_obj.conditional_update(
         {'status': 'deleting',
          'previous_status': vol_obj.model.status,
          'terminated_at': now},
         expected,
         filters)

    It can also be specified whether to save already dirtied fields from the
    objects or not and by default (if no expected_values argument is
    provided) it will make sure that the entry in the DB has the same values
    as the objects we are saving.

    We can select values based on conditions using Case objects in the
    'values' argument. For example:

     has_snapshot_filter = sql.exists().where(
         models.Snapshot.volume_id == models.Volume.id)
     case_values = volume.Case([(has_snapshot_filter, 'has-snapshot')],
                               else_='no-snapshot')
     volume.conditional_update({'status': case_values},
                               {'status': 'available'})

    Exclusion and multi-exclusion will handle, by default, NULL values like
    Python does instead of like SQL does, so NULL values will be considered
    different than any non NULL values. That way if we search for something
    not equal to 1 we will also get NULL values.

    WARNING: SQLAlchemy does not allow selecting order of SET clauses, so
    for now we cannot do things like
        {'previous_status': model.status, 'status': 'retyping'}
    because it will result in both previous_status and status being set to
    'retyping'. Issue has been reported [1] and a patch to fix it [...

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Change abandoned by Sean McGinnis (<email address hidden>) on branch: master
Review: https://review.openstack.org/202914
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Sean McGinnis (<email address hidden>) on branch: master
Review: https://review.openstack.org/255430
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.

[Jay Bryant (jsbryant)]

Moving this back to triaged given that the associated patch has been abandoned due to inactivity.

[Sean McGinnis (sean-mcginnis)]

Unassigning due to no activity for > 6 months.