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

Revision history for this message
Devin Carlen (devcamcar) 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.

review: Needs Fixing

« Back to merge proposal