Fix the inappropriate parameter name

The argument to the function should be the volume ID,
not the attachment ID.So the argument name of the
function here is easy to misunderstand.

Change-Id: Ie093b6acebe8b9e8dbdfed89856666db25bcf756

novaclient/tests/unit/v2/test_volumes.py

@@ -13,6 +13,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import mock
+
 from novaclient import api_versions
 from novaclient.tests.unit import utils
 from novaclient.tests.unit.v2 import fakes
@@ -55,6 +57,24 @@ class VolumesTest(utils.TestCase):
                               '/servers/1234/os-volume_attachments/Work')
         self.assertIsInstance(v, volumes.Volume)
 
+    def test_get_server_volume_with_exception(self):
+        self.assertRaises(TypeError,
+                          self.cs.volumes.get_server_volume,
+                          "1234")
+
+        self.assertRaises(TypeError,
+                          self.cs.volumes.get_server_volume,
+                          "1234",
+                          volume_id="Work",
+                          attachment_id="123")
+
+    @mock.patch('warnings.warn')
+    def test_get_server_volume_with_warn(self, mock_warn):
+        self.cs.volumes.get_server_volume(1234,
+                                          volume_id=None,
+                                          attachment_id="Work")
+        mock_warn.assert_called_once()
+
     def test_list_server_volumes(self):
         vl = self.cs.volumes.get_server_volumes(1234)
         self.assert_request_id(vl, fakes.FAKE_REQUEST_ID_LIST)
@@ -88,3 +108,21 @@ class VolumesV249Test(VolumesTest):
                 'device': '/dev/vdb',
                 'tag': 'test_tag'}})
         self.assertIsInstance(v, volumes.Volume)
+
+    def test_delete_server_volume_with_exception(self):
+        self.assertRaises(TypeError,
+                          self.cs.volumes.delete_server_volume,
+                          "1234")
+
+        self.assertRaises(TypeError,
+                          self.cs.volumes.delete_server_volume,
+                          "1234",
+                          volume_id="Work",
+                          attachment_id="123")
+
+    @mock.patch('warnings.warn')
+    def test_delete_server_volume_with_warn(self, mock_warn):
+        self.cs.volumes.delete_server_volume(1234,
+                                             volume_id=None,
+                                             attachment_id="Work")
+        mock_warn.assert_called_once()

novaclient/v2/volumes.py

@@ -16,6 +16,7 @@
 """
 Volume interface
 """
+import warnings
 
 from novaclient import api_versions
 from novaclient import base
@@ -95,17 +96,32 @@ class VolumeManager(base.Manager):
                             (server_id, src_volid,),
                             body, "volumeAttachment")
 
-    def get_server_volume(self, server_id, attachment_id):
+    def get_server_volume(self, server_id, volume_id=None, attachment_id=None):
         """
-        Get the volume identified by the attachment ID, that is attached to
+        Get the volume identified by the volume ID, that is attached to
         the given server ID
 
         :param server_id: The ID of the server
-        :param attachment_id: The ID of the attachment
+        :param volume_id: The ID of the volume to attach
         :rtype: :class:`Volume`
         """
+
+        if attachment_id is not None and volume_id is not None:
+            raise TypeError("You cannot specify both volume_id "
+                            "and attachment_id arguments.")
+
+        elif attachment_id is not None:
+            warnings.warn("attachment_id argument "
+                          "of volumes.get_server_volume "
+                          "method is deprecated in favor "
+                          "of volume_id.")
+            volume_id = attachment_id
+
+        if volume_id is None:
+            raise TypeError("volume_id is required argument.")
+
         return self._get("/servers/%s/os-volume_attachments/%s" % (server_id,
-                         attachment_id,), "volumeAttachment")
+                         volume_id,), "volumeAttachment")
 
     def get_server_volumes(self, server_id):
         """
@@ -117,13 +133,28 @@ class VolumeManager(base.Manager):
         return self._list("/servers/%s/os-volume_attachments" % server_id,
                           "volumeAttachments")
 
-    def delete_server_volume(self, server_id, attachment_id):
+    def delete_server_volume(self, server_id, volume_id=None,
+                             attachment_id=None):
         """
-        Detach a volume identified by the attachment ID from the given server
+        Detach a volume identified by the volume ID from the given server
 
         :param server_id: The ID of the server
-        :param attachment_id: The ID of the attachment
+        :param volume_id: The ID of the volume to attach
         :returns: An instance of novaclient.base.TupleWithMeta
         """
+        if attachment_id is not None and volume_id is not None:
+            raise TypeError("You cannot specify both volume_id "
+                            "and attachment_id arguments.")
+
+        elif attachment_id is not None:
+            warnings.warn("attachment_id argument "
+                          "of volumes.delete_server_volume "
+                          "method is deprecated in favor "
+                          "of volume_id.")
+            volume_id = attachment_id
+
+        if volume_id is None:
+            raise TypeError("volume_id is required argument.")
+
         return self._delete("/servers/%s/os-volume_attachments/%s" %
-                            (server_id, attachment_id,))
+                            (server_id, volume_id,))