From 6eba403ba020292806d0ec35edc7964eaa8e8eaf Mon Sep 17 00:00:00 2001
From: zhangyang <zhangyang1@chinatelecom.cn>
Date: Fri, 25 Jan 2019 18:11:17 +0800
Subject: [PATCH] Create volume attachments status check.

Currently the new cinder attachment_create method allows volumes in
all status to be performed for single attachment. This may cause
problems if volume-attach operation is taken on volumes in creating
status.
In order to avoid these problems, we should only allow volumes
in 'in-use', 'reserved' status to check whether this attachment
creation is a override operation for a same instance.

Related-bug: #1694530
Change-Id: Ie59f609d38290147a67a936c61e0164fe278da99
Closes-Bug: #1813267
---
 cinder/tests/unit/attachments/test_attachments_api.py | 11 +++++++++++
 cinder/volume/api.py                                  |  8 +++++---
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/cinder/tests/unit/attachments/test_attachments_api.py b/cinder/tests/unit/attachments/test_attachments_api.py
index f6c25913ac3..d2b59e51fd9 100644
--- a/cinder/tests/unit/attachments/test_attachments_api.py
+++ b/cinder/tests/unit/attachments/test_attachments_api.py
@@ -314,3 +314,14 @@ class AttachmentManagerTestCase(test.TestCase):
                           self.context,
                           aref,
                           connector)
+
+    def test_attachment_create_creating_volume(self):
+        """Test attachment_create on a creating volume."""
+        volume_params = {'status': 'creating'}
+
+        vref = tests_utils.create_volume(self.context, **volume_params)
+        self.assertRaises(exception.InvalidVolume,
+                          self.volume_api.attachment_create,
+                          self.context,
+                          vref,
+                          fake.UUID1)
diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index d0ccbc01ccd..22ef4cefb4f 100644
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -2075,7 +2075,7 @@ class API(base.Base):
 
         if not result:
             override = False
-            if instance_uuid:
+            if instance_uuid and vref.status in ('in-use', 'reserved'):
                 # Refresh the volume reference in case multiple instances were
                 # being concurrently attached to the same non-multiattach
                 # volume.
@@ -2090,9 +2090,11 @@ class API(base.Base):
                         break
 
             if not override:
-                msg = (_('Volume %(vol_id)s status must be %(statuses)s') %
+                msg = (_('Volume %(vol_id)s status must be %(statuses)s to '
+                         'reserve, but the current status is %(current)s.') %
                        {'vol_id': vref.id,
-                        'statuses': utils.build_or_str(expected['status'])})
+                        'statuses': utils.build_or_str(expected['status']),
+                        'current': vref.status})
                 raise exception.InvalidVolume(reason=msg)
 
         values = {'volume_id': vref.id,