Implement list_volumes() API for VMAX driver #70
Conversation
| free_cap = total_cap - used_cap | ||
|
|
||
| # TODO: Update constants.VolumeStatus to make mapping more precise | ||
| switcher = { |
There was a problem hiding this comment.
just optimization suggestion:
switcher can be out of foor loop or even top of try block.
|
|
||
| try: | ||
| # List all volumes | ||
| volumes = self.conn.provisioning.get_volume_list(filters={'data_volume': 'false'}) |
There was a problem hiding this comment.
make sure, no. of chars in a line not >79
| if vol['num_of_storage_groups'] == 1: | ||
| sg = vol['storageGroupId'][0] | ||
| sg_info = self.conn.provisioning.get_storage_group(sg) | ||
| v['original_pool_id'] = sg_info['srp'] |
There was a problem hiding this comment.
'original_pool_id' or 'original_id' ?? . Plz cross check models.
one Q:
incase 'num_of_storage_groups' !=1, there will be no 'original_id' of a pool??
There was a problem hiding this comment.
It is 'original_pool_id' (in class Volume(BASE, DolphinBase)). Currently we meed SG of the volume to get these info. Please see the TODO comment in the code.
| def list_volumes(self, storage_id): | ||
|
|
||
| try: | ||
| # List all volumes |
There was a problem hiding this comment.
Can u update comment on what are type of volumes from backend will be listed for more clarity
| return volume_list | ||
|
|
||
| except Exception as err: | ||
| LOG.error("Failed to get list volumes from vmax: {}".format(err)) |
There was a problem hiding this comment.
Keep as VMAX as maintained in other places
|
LGTM |
| used_cap = (total_cap * vol['allocated_percent']) / 100.0 | ||
| free_cap = total_cap - used_cap | ||
|
|
||
| status = switcher.get(vol['status'], |
There was a problem hiding this comment.
I suggest , default to 'Unknown', what do you think?
Codecov Report
@@ Coverage Diff @@
## master #70 +/- ##
==========================================
- Coverage 29.37% 29.20% -0.18%
==========================================
Files 59 59
Lines 3819 3845 +26
Branches 428 431 +3
==========================================
+ Hits 1122 1123 +1
- Misses 2680 2705 +25
Partials 17 17
|
joseph-v
force-pushed
the
vollist
branch
3 times, most recently
from
a6702ef
to
b8e2222
Compare
|
LGTM |
This PR implements list_volumes() API for the Dell EMC VMAX driver.
Functionality is verified using VMAX250F backed.
What this PR does / why we need it:
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #Special notes for your reviewer:
Release note: