Code review comment for lp:~ed-leafe/nova/xenstore-plugin

I've tested this and everything seems to work as advertised. Some feedback:

1) pep8:

$ pep8 -r xenstore.py
xenstore.py:39:66: W291 trailing whitespace
xenstore.py:40:70: W291 trailing whitespace
xenstore.py:49:80: E501 line too long (89 characters)
xenstore.py:54:30: W291 trailing whitespace
xenstore.py:59:1: E302 expected 2 blank lines, found 1
xenstore.py:66:80: E501 line too long (83 characters)
xenstore.py:70:1: E302 expected 2 blank lines, found 1
xenstore.py:101:1: E302 expected 2 blank lines, found 1
xenstore.py:108:1: E302 expected 2 blank lines, found 1
xenstore.py:140:1: E302 expected 2 blank lines, found 1
xenstore.py:146:80: E501 line too long (100 characters)

2) Exception handling:

There are a few places in nova/virt/xenapi/vmops.py where we try to parse the JSON, fail, and then return the original response. For example, in list_from_xenstore:

    ret = self._make_xenstore_call('list_records', vm, path)
    try:
        return json.loads(ret)
    except ValueError:
        # Not a valid JSON value
        return ret

Shouldn't this re-raise the exception if we have received an invalid JSON response? It seems that when this succeeds we get a Python data structure back, and when it fails we instead get some garbage from the XenAPI.

3) There is a multi-line comment about several methods that were created to interact with the xenstore parameter record. You mention that they aren't used, but might be useful. Should we avoid adding this code until its needed? Otherwise, it's code bloat.