[GH-94]Add VNX QoS support by peter-wangxu · Pull Request #95 · emc-openstack/storops

peter-wangxu · 2017-02-20T11:13:11Z

Add VNXIOClass and related operations
Add VNXIOPolicy and related operations

jealous · 2017-03-03T02:19:42Z

+@cli_exception
+class VNXIOCLassRunningError(VNXIOClassError):
+    error_message = 'Can not modify or delete an I/O Class while it is '
+    'currently running'


currently is redundant and you missed a period.

I cannot modify this message, it's extracted from naviseccli output. I can add the period, since it's part of original output.

jealous · 2017-03-03T02:21:02Z

+@cli_exception
+class VNXIOPolicyRunningError(VNXIOPolicyError):
+    error_message = 'Can not modify a Policy, delete a Policy or update event '
+    'log settings while it is currently running'


Can not modify, delete a policy or update event log settings when the policy is running.

jealous · 2017-03-03T02:21:18Z

+
 @cli_exception
 class VNXTargetNotReadyError(VNXMigrationError):
    error_message = 'The destination LUN is not available for migration'


Missing period.

I am not sure whether there is a period for the error message, since it's harmless, how about just keep it?

jealous · 2017-03-03T02:26:08Z

+        cmd = ['nqm', '-ioclass', '-create', '-name', name, '-iotype', iotype]
+        cmd += int_var('-minsize', minsize)
+        cmd += int_var('-maxsize', maxsize)
+        if lun_ids:


line 888 to line 890 can be replaced with cmd += text_var('-luns', lun_ids)

lun_ids is a list of id, could not use text_var
As I found lun_ids must be a list of parameters, and cannot be joined as one single string parameter.

jealous · 2017-03-03T02:26:23Z

+        if lun_ids:
+            cmd += ['-luns']
+            cmd += lun_ids
+        if smp_names:


See above comment.

jealous · 2017-03-03T03:04:48Z

+
+    @staticmethod
+    def get(cli, name=None):
+        if name:


One return each function.

jealous · 2017-03-03T03:08:02Z

+    def create(cli, name, ioclasses=None, fail_action=None, time_limit=None,
+               eval_window=None):
+        ioclass_names = []
+        if ioclasses:


Duplicated with line 241 to line 243. Extract them to a function, please.

jealous · 2017-03-03T03:33:32Z

+        assert_that(ioclass.io_type, equal_to('ReadWrite'))
+        assert_that(ioclass.io_size_range, equal_to('Any'))
+        assert_that(ioclass.control_method, equal_to('No Control'))
+        assert_that(ioclass.luns[0], instance_of(VNXLun))


list(map(lambda l: assert_that(l, instance_of(VNXLun)), ioclass.luns))

jealous · 2017-03-03T03:34:01Z

+        assert_that(ioclass.control_method, equal_to('Limit'))
+        assert_that(ioclass.metric_type, equal_to('Bandwidth'))
+        assert_that(ioclass.goal_value, equal_to('1000.0 MB/s'))
+        assert_that(ioclass.luns[0], instance_of(VNXLun))


list(map(lambda l: assert_that(l, instance_of(VNXLun)), ioclass.luns))

jealous · 2017-03-03T03:36:35Z

+    @patch_cli
+    def test_delete_ioclasse(self):
+        ioclass = VNXIOClass(name='to_delete', cli=t_cli())
+        ioclass.delete()


jealous

Please add test for check_list, etc.

jealous · 2017-03-07T02:17:34Z

+@cli_exception
+class VNXIOCLassRunningError(VNXIOClassError):
+    error_message = 'Can not modify or delete an I/O Class while it is '
+    'currently running'


jealous · 2017-03-07T02:17:45Z

+@cli_exception
+class VNXIOPolicyRunningError(VNXIOPolicyError):
+    error_message = 'Can not modify a Policy, delete a Policy or update event '
+    'log settings while it is currently running'


jealous · 2017-03-07T02:17:54Z

+
 @cli_exception
 class VNXTargetNotReadyError(VNXMigrationError):
    error_message = 'The destination LUN is not available for migration'


jealous · 2017-03-07T02:19:12Z

+    else:
+        raise ValueError('"{}" must be list or None.'.format(value))
+    return ret
+


Please add test for this function.

jealous · 2017-03-07T02:20:23Z

+        self.tolerance = tolerance
+
+    def get_option(self):
+        if self.method == VNXCtrlMethod.NO_CTRL:


jealous · 2017-03-07T02:21:08Z

+        """Aggregator for ioclass_luns and ioclass_snapshots."""
+        lun_list, smp_list = [], []
+        if self.ioclass_luns:
+            lun_list = map(lambda l: VNXLun(lun_id=l.lun_id, name=l.name,


* Add VNXIOClass and related operations * Add VNXIOPolicy and related operations

peter-wangxu force-pushed the vnx_qos_support branch 21 times, most recently from 20e3d58 to 34bc809 Compare March 1, 2017 08:28

jealous suggested changes Mar 3, 2017

View reviewed changes

peter-wangxu force-pushed the vnx_qos_support branch from 34bc809 to 1c3a039 Compare March 3, 2017 09:29

jealous suggested changes Mar 7, 2017

View reviewed changes

jealous assigned peter-wangxu Mar 7, 2017

jealous added enhancement Unity labels Mar 7, 2017

peter-wangxu added VNX and removed Unity labels Mar 7, 2017

peter-wangxu force-pushed the vnx_qos_support branch from 1c3a039 to 4deea2f Compare March 7, 2017 02:52

peter-wangxu force-pushed the vnx_qos_support branch from 4deea2f to 2f69fb6 Compare March 7, 2017 03:20

[GH-94]Add VNX QoS support

53ce294

* Add VNXIOClass and related operations * Add VNXIOPolicy and related operations

peter-wangxu force-pushed the vnx_qos_support branch from 2f69fb6 to 53ce294 Compare March 7, 2017 04:04

jealous approved these changes Mar 7, 2017

View reviewed changes

jealous merged commit 60a8dd0 into develop Mar 7, 2017

jealous deleted the vnx_qos_support branch March 7, 2017 09:36

Conversation

peter-wangxu commented Feb 20, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peter-wangxu Mar 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jealous left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

peter-wangxu Mar 3, 2017 •

edited

Loading