]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Update list_replication_targets
authorJohn Griffith <john.griffith8@gmail.com>
Tue, 24 Nov 2015 21:46:19 +0000 (14:46 -0700)
committerJohn Griffith <john.griffith8@gmail.com>
Tue, 24 Nov 2015 21:46:52 +0000 (14:46 -0700)
We were planning to let the list targets call be a
free for all, and just be a generic dict of info.

This probably isn't such a great idea, inparticular since
we have a required identifier field for each replication
device it makes more sense to just use that in the response
and leave the rest of the info internal.

In the future if there's a need we can always implement a
show command that displays all the dirty details.

This patch changes the identifier name to 'target_device_id'
to be more self describing, and updates the docstrings and
devref docs to reflect the changes and agreed upon changes.

Change-Id: If14130f5d5bb2e6df4478bd0e14a1f33d706bf78

cinder/volume/driver.py
cinder/volume/manager.py
doc/source/devref/replication.rst

index de3a0d183df11199111f83e96bb9c5dd6eb856ae..97817155a8b062062e088fd122d3d273e49834ed 100644 (file)
@@ -215,7 +215,7 @@ volume_opts = [
                       "times in a single config section to specify multiple "
                       "replication target devices.  Each entry takes the "
                       "standard dict config form: replication_device = "
-                      "device_target_id:<required>,"
+                      "target_device_id:<required>,"
                       "managed_backend_name:<host@backend_name>,"
                       "key1:value1,key2:value2..."),
     cfg.BoolOpt('image_upload_use_cinder_backend',
index 2a637cfd88f465d369003da99cb0ace600c6a5d6..efe42f362e057b0f97217b1a8d1e7931e4fac705 100644 (file)
@@ -3323,28 +3323,22 @@ class VolumeManager(manager.SchedulerDependentManager):
 
         This method is used to query a backend to get the current
         replication config info for the specified volume.
-
         In the case of a volume that isn't being replicated,
         the driver should return an empty list.
 
+        There is one required field for configuration of
+        replication, (target_device_id).  As such we
+        use that for the response in list_replication_targets.
 
-        Example response for replicating to a managed backend:
-            {'volume_id': volume['id'],
-             'targets':[{'remote_device_id': 'vendor-id-for-target-device',
-                         'managed_host': 'backend_name'}...]
-
-        Example response for replicating to an unmanaged backend:
-            {'volume_id': volume['id'], 'targets':[
-                {'remote_device_id': 'vendor-id-for-target-device',
-                                      'san_ip': '1.1.1.1',
-                                      'san_login': 'admin'},
-                                      ....]}
+        Internal methods can be added to extract additional
+        details if needed, but the only detail that should be
+        exposed to an end user is the identifier.  In the case
+        of an Admin, (s)he can always cross check the cinder.conf
+        file that they have configured for additional info.
 
-        NOTE: It's the responsibility of the driver to mask out any
-        passwords or sensitive information.
-
-        `remote_device_id` is required and is used for drivers to identify
-        the devices they have in use.
+        Example response:
+            {'volume_id': volume['id'],
+             'targets':[<target-device-id>,...]'
 
         """
         # NOTE(hemna) We intentionally don't enforce the driver being
index 46a7595995a2d073011903338f61f247e6c85dc3..bd1371e675b5263704a27452cc1ae2375233a72a 100644 (file)
@@ -31,9 +31,12 @@ like to configure.
 There are two standardized keys in the config
 entry, all others are vendor-unique:
 
-* device_target_id:<vendor-identifier-for-rep-target>
+* target_device_id:<vendor-identifier-for-rep-target>
 * managed_backend_name:<cinder-backend-host-entry>,"
 
+target_device_id is REQUIRED in all configurations
+
+
 
 An example config entry for a managed replication device
 would look like this::
@@ -209,17 +212,14 @@ act as a toggle, allowing to switch back and forth betweeen primary and secondar
 
 **list_replication_targets**
 
-Used by the admin to query a volume for a list of configured replication targets
-The expected return for this call is expected to mimic the form used in the config file.
-
-For a volume replicating to managed replication targets::
-
-    {'volume_id': volume['id'], 'targets':[{'type': 'managed',
-                                            'backend_name': 'backend_name'}...]
-
-For a volume replicating to external/unmanaged targets::
+Used by the admin to query a volume for a list of configured replication targets.
 
-    {'volume_id': volume['id'], 'targets':[{'type': 'unmanaged',
-                                            'san_ip': '127.0.0.1',
-                                            'san_login': 'admin'...}...]
+The expected response is simply the single required field in replication-device
+configuration.  It's possible that in the future we may want to add a show
+command that provides all the various details about a target replication
+device.  This would be of the form:
+`show_replication_target <target_device_id>`
 
+Example response:
+    {'volume_id': volume['id'],
+     'targets':[<target-device-id>,...]'