Code review comment for lp:~tamura-yoshiaki/nova/bug-805312

Revision history for this message
Yoshiaki Tamura (tamura-yoshiaki) wrote :

> This looks good but it looks like there is some logic is being duplicated:
>
> 35 + if vol['device_path'].startswith('/dev/'):
> 36 + vol['type'] = 'block'
> 37 + elif ':' in vol['device_path']:
> 38 + (protocol, name) = vol['device_path'].split(':')
> 39 + vol['type'] = 'network'
> 40 + vol['protocol'] = protocol
> 41 + vol['device_path'] = name
> 42 + else:
> 43 + raise
> exception.InvalidDevicePath(path=vol['device_path'])
>
> There is very similar logic in the attach_volume method. I recommend fixing
> this so that there is no repetition. Maybe something like:
>
> def _get_volume_device_type(self, device_path):
> if device_path.startswith('/dev/'):
> return 'block'
> elif ':' in device_path:
> return 'network'
> else:
> raise exception.InvalidDevicePath(path=device_path)
>
> And then re-use that.

Sorry for my slow response. Applied your comments.

« Back to merge proposal