)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"3a81c273d6a3fa8160d4bbeb55ef2f64bfd11500","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Aditi Raveesh \u003caditi.raveesh@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2013-11-04 17:33:22 +0530"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Allow windows builds with xentools 6.1 and 6.2"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Building a windows VM with pre-intalled xentools"},{"line_number":10,"context_line":"6.1 or 6.2 requires device_id to be set on XEN VM record."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"AAAATn%2F%2FfhU%3D","line":7,"updated":"2013-11-04 16:36:16.000000000","message":"This should be prefixed with [Xen] or Xenapi:","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"d22640e2d96fd5d58e28558e6c5d31e6e481b0da","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Aditi Raveesh \u003caditi.raveesh@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2013-11-04 17:33:22 +0530"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Allow windows builds with xentools 6.1 and 6.2"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Building a windows VM with pre-intalled xentools"},{"line_number":10,"context_line":"6.1 or 6.2 requires device_id to be set on XEN VM record."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"AAAATn%2F%2FeMs%3D","line":7,"in_reply_to":"AAAATn%2F%2FeV8%3D","updated":"2013-11-05 09:19:32.000000000","message":"Done","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"d22640e2d96fd5d58e28558e6c5d31e6e481b0da","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Aditi Raveesh \u003caditi.raveesh@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2013-11-04 17:33:22 +0530"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Allow windows builds with xentools 6.1 and 6.2"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Building a windows VM with pre-intalled xentools"},{"line_number":10,"context_line":"6.1 or 6.2 requires device_id to be set on XEN VM record."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"AAAATn%2F%2FeMw%3D","line":7,"in_reply_to":"AAAATn%2F%2FfhU%3D","updated":"2013-11-05 09:19:32.000000000","message":"Done","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"12e16d5fca7160fc4d60d9570f34e351c305e0ca","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Aditi Raveesh \u003caditi.raveesh@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2013-11-04 17:33:22 +0530"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Allow windows builds with xentools 6.1 and 6.2"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Building a windows VM with pre-intalled xentools"},{"line_number":10,"context_line":"6.1 or 6.2 requires device_id to be set on XEN VM record."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"AAAATn%2F%2FeV8%3D","line":7,"in_reply_to":"AAAATn%2F%2FfhU%3D","updated":"2013-11-05 08:34:24.000000000","message":"XenAPI: would be better than [xen] as it does not apply to xen under libvirt.","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"3a81c273d6a3fa8160d4bbeb55ef2f64bfd11500","unresolved":false,"context_lines":[{"line_number":10,"context_line":"6.1 or 6.2 requires device_id to be set on XEN VM record."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Id3cd9d569519f2df1afa046ddabf33f00d63c615"},{"line_number":13,"context_line":"Implements: allow-windows-builds-with-xentools6.1-and-6.2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"AAAATn%2F%2FfhQ%3D","line":13,"updated":"2013-11-04 16:36:16.000000000","message":"this should be \u0027bp\u0027 or \u0027blueprint\u0027.","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"d22640e2d96fd5d58e28558e6c5d31e6e481b0da","unresolved":false,"context_lines":[{"line_number":10,"context_line":"6.1 or 6.2 requires device_id to be set on XEN VM record."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Id3cd9d569519f2df1afa046ddabf33f00d63c615"},{"line_number":13,"context_line":"Implements: allow-windows-builds-with-xentools6.1-and-6.2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"AAAATn%2F%2FeM0%3D","line":13,"in_reply_to":"AAAATn%2F%2FfhQ%3D","updated":"2013-11-05 09:19:32.000000000","message":"Done","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":8430,"name":"Christopher Lefelhocz","email":"christopher.lefelhoc@rackspace.com","username":"leif"},"change_message_id":"4952644d202111cc686a951446c39b5cc43a97e9","unresolved":false,"context_lines":[{"line_number":10,"context_line":"6.1 or 6.2 requires device_id to be set on XEN VM record."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Id3cd9d569519f2df1afa046ddabf33f00d63c615"},{"line_number":13,"context_line":"Implements: blueprint allow-windows-builds-with-xentools6.1-and-6.2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"AAAATn%2F%2Fd0w%3D","line":13,"updated":"2013-11-05 11:57:07.000000000","message":"hmmmm... \n\nlooks like string is too long and consequently we can\u0027t use link to go to blueprint directly.  We also need to get blueprint approved and would be a good place to resolve Bob\u0027s comments on approach.","commit_id":"ac00b45bb760b9255bc2955abcc07af217cd0c1f"},{"author":{"_account_id":6950,"name":"Kashi Reddy","email":"kashi.reddy@rackspace.com","username":"kashivreddy"},"change_message_id":"81fa89e19719a2b6c310f89891a84a2f576f727d","unresolved":false,"context_lines":[{"line_number":10,"context_line":"6.1 or 6.2 requires device_id to be set on XEN VM record."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Id3cd9d569519f2df1afa046ddabf33f00d63c615"},{"line_number":13,"context_line":"Implements: blueprint allow-windows-builds-with-xentools6.1-and-6.2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"AAAATn%2F%2Fdaw%3D","line":13,"in_reply_to":"AAAATn%2F%2Fd0w%3D","updated":"2013-11-05 16:31:47.000000000","message":"I think link isn\u0027t working because it has a \".\" in the name.","commit_id":"ac00b45bb760b9255bc2955abcc07af217cd0c1f"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"118510ab2e32a992934c220b47a40807b8e8f7fd","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Xenapi: Allow windows builds with xentools 6.1 and 6.2"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Building a windows VM with pre-intalled xentools"},{"line_number":10,"context_line":"6.1 or 6.2 requires device_id to be set on XEN VM record."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Id3cd9d569519f2df1afa046ddabf33f00d63c615"},{"line_number":13,"context_line":"Implements: blueprint allow-windows-builds-with-xentools6.1-and-6.2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"AAAATn%2F%2FaLU%3D","line":10,"updated":"2013-11-07 01:24:25.000000000","message":"DocImpact - needs to describe where to set things on the image and when","commit_id":"04d7b771b039ae7aff52be8bd92de2a42fe922a0"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"6c0b8c5757e443e47d91af5f5d29de68ef188316","unresolved":false,"context_lines":[{"line_number":10,"context_line":"6.1 or 6.2 requires device_id to be set on XEN VM record."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Id3cd9d569519f2df1afa046ddabf33f00d63c615"},{"line_number":13,"context_line":"Implements: blueprint allow-windows-builds-with-xentools6.1-and-6.2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"AAAATn%2F%2FcFk%3D","line":13,"updated":"2013-11-06 07:09:47.000000000","message":"I believe the problem with the link is that blueprints shouldn\u0027t include a period - the regular expression used to identify the blueprint treats them as an end of line marker.","commit_id":"04d7b771b039ae7aff52be8bd92de2a42fe922a0"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"6d103e6487523403c9817885e2f817538ccfcbbc","unresolved":false,"context_lines":[{"line_number":10,"context_line":"6.1 or 6.2 requires device_id to be set on XEN VM record."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Id3cd9d569519f2df1afa046ddabf33f00d63c615"},{"line_number":13,"context_line":"Implements: blueprint allow-windows-builds-with-xentools6.1-and-6.2"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"AAAATn%2F%2Fb5Q%3D","line":13,"in_reply_to":"AAAATn%2F%2FcFk%3D","updated":"2013-11-07 04:57:13.000000000","message":"Yes, will make that change.","commit_id":"04d7b771b039ae7aff52be8bd92de2a42fe922a0"}],"nova/tests/virt/xenapi/test_vm_utils.py":[{"author":{"_account_id":6992,"name":"Kaushik Chandrashekar","email":"kaushik.chandrashekar@rackspace.com","username":"kaushikc"},"change_message_id":"96c122e74269190aec4eb7ed193df8570c7744a0","unresolved":false,"context_lines":[{"line_number":1818,"context_line":""},{"line_number":1819,"context_line":"    def test_device_id_for_tools_6_2_and_windows_vm(self):"},{"line_number":1820,"context_line":"        image_meta \u003d {\u0027os_type\u0027: \u0027windows\u0027, \u0027com.openstack__1__xentools\u0027: 6.2}"},{"line_number":1821,"context_line":"        self.assertEqual(2, vm_utils.get_device_id(image_meta\u003dimage_meta))"},{"line_number":1822,"context_line":""},{"line_number":1823,"context_line":"    def test_device_id_for_tools_6_1_and_linux_vm(self):"},{"line_number":1824,"context_line":"        image_meta \u003d {\u0027os_type\u0027: \u0027linux\u0027, \u0027com.openstack__1__xentools\u0027: 6.1}"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2Ff%2BY%3D","line":1821,"updated":"2013-11-04 10:38:44.000000000","message":"Add a test case with windows os_type but with com.openstack__1__xentools version not present and something which is other than 6.1 and 6.2","commit_id":"e34bdf9096af887937b203739231068d1a6d2bb5"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"15e9d91bbb5c95ea38ef2e50f2b283cbee3e97a9","unresolved":false,"context_lines":[{"line_number":1818,"context_line":""},{"line_number":1819,"context_line":"    def test_device_id_for_tools_6_2_and_windows_vm(self):"},{"line_number":1820,"context_line":"        image_meta \u003d {\u0027os_type\u0027: \u0027windows\u0027, \u0027com.openstack__1__xentools\u0027: 6.2}"},{"line_number":1821,"context_line":"        self.assertEqual(2, vm_utils.get_device_id(image_meta\u003dimage_meta))"},{"line_number":1822,"context_line":""},{"line_number":1823,"context_line":"    def test_device_id_for_tools_6_1_and_linux_vm(self):"},{"line_number":1824,"context_line":"        image_meta \u003d {\u0027os_type\u0027: \u0027linux\u0027, \u0027com.openstack__1__xentools\u0027: 6.1}"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2Ff40%3D","line":1821,"in_reply_to":"AAAATn%2F%2Ff%2BY%3D","updated":"2013-11-04 12:18:38.000000000","message":"Done","commit_id":"e34bdf9096af887937b203739231068d1a6d2bb5"},{"author":{"_account_id":8430,"name":"Christopher Lefelhocz","email":"christopher.lefelhoc@rackspace.com","username":"leif"},"change_message_id":"3f7257451d743cb230ae9b448ea6c9cd8f1aa061","unresolved":false,"context_lines":[{"line_number":1811,"context_line":"        self.assertEqual(expected, mock_strip.call_args_list)"},{"line_number":1812,"context_line":""},{"line_number":1813,"context_line":""},{"line_number":1814,"context_line":"class DeviceIdTestCase(VMUtilsTestBase):"},{"line_number":1815,"context_line":"    def test_device_id_for_tools_6_1_and_windows_vm(self):"},{"line_number":1816,"context_line":"        image_meta \u003d {\u0027os_type\u0027: \u0027windows\u0027, \u0027com.openstack__1__xentools\u0027: 6.1}"},{"line_number":1817,"context_line":"        self.assertEqual(2, vm_utils.get_device_id(image_meta\u003dimage_meta))"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2FfcI%3D","line":1814,"updated":"2013-11-04 17:33:35.000000000","message":"After looking over code some negative testing may be appropriate.  Having something for a default case return or exception thrown if that makes sense.","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"d22640e2d96fd5d58e28558e6c5d31e6e481b0da","unresolved":false,"context_lines":[{"line_number":1811,"context_line":"        self.assertEqual(expected, mock_strip.call_args_list)"},{"line_number":1812,"context_line":""},{"line_number":1813,"context_line":""},{"line_number":1814,"context_line":"class DeviceIdTestCase(VMUtilsTestBase):"},{"line_number":1815,"context_line":"    def test_device_id_for_tools_6_1_and_windows_vm(self):"},{"line_number":1816,"context_line":"        image_meta \u003d {\u0027os_type\u0027: \u0027windows\u0027, \u0027com.openstack__1__xentools\u0027: 6.1}"},{"line_number":1817,"context_line":"        self.assertEqual(2, vm_utils.get_device_id(image_meta\u003dimage_meta))"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2Fegs%3D","line":1814,"in_reply_to":"AAAATn%2F%2FfcI%3D","updated":"2013-11-05 09:19:32.000000000","message":"The default value is set to 1. We don\u0027t want an exception thrown.\n\ntest_device_id_if_tools_version_is_not_set is the negative scenario where the property xentools version is not set.","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"0c3ce61ff172c66a1ddff4b47ed9856d2a561770","unresolved":false,"context_lines":[{"line_number":1837,"context_line":"    def test_device_id_if_vm_type_is_not_set(self):"},{"line_number":1838,"context_line":"        self.assertEqual(\"0001\", vm_utils.get_device_id(image_meta\u003d{}))"},{"line_number":1839,"context_line":""},{"line_number":1840,"context_line":"    def test_device_id_if_tools_higher_than_6_1(self):"},{"line_number":1841,"context_line":"        image_meta \u003d {\u0027os_type\u0027: \u0027windows\u0027,"},{"line_number":1842,"context_line":"                      \u0027com.openstack__1__xentools\u0027: \u00276.1.4\u0027}"},{"line_number":1843,"context_line":"        self.assertEqual(\"0002\", vm_utils.get_device_id(image_meta\u003dimage_meta))"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2Fc6U%3D","line":1840,"updated":"2013-11-05 22:12:00.000000000","message":"Did you cover the case where the os_type is windows and xentools version \u003c 6.1?  Is that possible?","commit_id":"ac00b45bb760b9255bc2955abcc07af217cd0c1f"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"d508594c09ce240b590b5ffdddb233394df18808","unresolved":false,"context_lines":[{"line_number":1837,"context_line":"    def test_device_id_if_vm_type_is_not_set(self):"},{"line_number":1838,"context_line":"        self.assertEqual(\"0001\", vm_utils.get_device_id(image_meta\u003d{}))"},{"line_number":1839,"context_line":""},{"line_number":1840,"context_line":"    def test_device_id_if_tools_higher_than_6_1(self):"},{"line_number":1841,"context_line":"        image_meta \u003d {\u0027os_type\u0027: \u0027windows\u0027,"},{"line_number":1842,"context_line":"                      \u0027com.openstack__1__xentools\u0027: \u00276.1.4\u0027}"},{"line_number":1843,"context_line":"        self.assertEqual(\"0002\", vm_utils.get_device_id(image_meta\u003dimage_meta))"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2FcRE%3D","line":1840,"in_reply_to":"AAAATn%2F%2Fc6U%3D","updated":"2013-11-06 05:58:15.000000000","message":"Done","commit_id":"ac00b45bb760b9255bc2955abcc07af217cd0c1f"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"5690166f00e3ccd82a9758550bbed1a967b515a8","unresolved":false,"context_lines":[{"line_number":1874,"context_line":"                         \u0027timeoffset\u0027: \u00270\u0027, \u0027viridian\u0027: \u0027true\u0027, \u0027acpi\u0027: \u0027true\u0027,"},{"line_number":1875,"context_line":"                         \u0027device_id\u0027: \u00270002\u0027},"},{"line_number":1876,"context_line":"            \u0027PV_legacy_args\u0027: \u0027\u0027, \u0027PV_kernel\u0027: \u0027\u0027, \u0027affinity\u0027: \u0027\u0027,"},{"line_number":1877,"context_line":"            \u0027recommendations\u0027: \u0027\u0027, \u0027ha_always_run\u0027: False}"},{"line_number":1878,"context_line":""},{"line_number":1879,"context_line":"        session.call_xenapi.assert_called_with(\u0027VM.create\u0027, expected_vm_rec)"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F%2FLek%3D","line":1877,"updated":"2013-11-13 10:57:10.000000000","message":"This blob is really hard to read.","commit_id":"09a63bd4fb4e3eeb7a3e8d8f891fb49a07aaa2d4"}],"nova/utils.py":[{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"0c3ce61ff172c66a1ddff4b47ed9856d2a561770","unresolved":false,"context_lines":[{"line_number":1042,"context_line":"            version_as_float \u003d float(\".\".join(major_minor_versions))"},{"line_number":1043,"context_line":"            return version_as_float"},{"line_number":1044,"context_line":"    except Exception:"},{"line_number":1045,"context_line":"        raise exception.NovaException(message\u003d\"Version invalid\")"},{"line_number":1046,"context_line":""},{"line_number":1047,"context_line":""},{"line_number":1048,"context_line":"def is_neutron():"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2Fc7E%3D","line":1045,"updated":"2013-11-05 22:12:00.000000000","message":"Include the invalid version so the message will be meaningful for debugging?","commit_id":"ac00b45bb760b9255bc2955abcc07af217cd0c1f"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"d508594c09ce240b590b5ffdddb233394df18808","unresolved":false,"context_lines":[{"line_number":1042,"context_line":"            version_as_float \u003d float(\".\".join(major_minor_versions))"},{"line_number":1043,"context_line":"            return version_as_float"},{"line_number":1044,"context_line":"    except Exception:"},{"line_number":1045,"context_line":"        raise exception.NovaException(message\u003d\"Version invalid\")"},{"line_number":1046,"context_line":""},{"line_number":1047,"context_line":""},{"line_number":1048,"context_line":"def is_neutron():"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2FcRA%3D","line":1045,"in_reply_to":"AAAATn%2F%2Fc7E%3D","updated":"2013-11-06 05:58:15.000000000","message":"Done","commit_id":"ac00b45bb760b9255bc2955abcc07af217cd0c1f"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"unresolved":false,"context_lines":[{"line_number":1042,"context_line":"            version_as_float \u003d float(\".\".join(major_minor_versions))"},{"line_number":1043,"context_line":"            return version_as_float"},{"line_number":1044,"context_line":"    except Exception:"},{"line_number":1045,"context_line":"        raise exception.NovaException(message\u003d\"Version %s invalid\" % version)"},{"line_number":1046,"context_line":""},{"line_number":1047,"context_line":""},{"line_number":1048,"context_line":"def is_neutron():"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATn%2F%2FVlI%3D","line":1045,"updated":"2013-11-09 09:31:09.000000000","message":"please use the i18n format.","commit_id":"03987784ef94ed09ca48d3ab993d442d9388c429"}],"nova/virt/xenapi/vm_utils.py":[{"author":{"_account_id":6992,"name":"Kaushik Chandrashekar","email":"kaushik.chandrashekar@rackspace.com","username":"kaushikc"},"change_message_id":"96c122e74269190aec4eb7ed193df8570c7744a0","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        os_type \u003d image_meta[\u0027os_type\u0027]"},{"line_number":199,"context_line":"        xentools \u003d image_meta[\u0027com.openstack__1__xentools\u0027]"},{"line_number":200,"context_line":"    except KeyError:"},{"line_number":201,"context_line":"        return device_id"},{"line_number":202,"context_line":"    if(os_type \u003d\u003d \"windows\" and float(xentools) \u003e\u003d 6.1):"},{"line_number":203,"context_line":"        device_id \u003d 2"},{"line_number":204,"context_line":"    return device_id"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2Ff%2BQ%3D","line":201,"updated":"2013-11-04 10:38:44.000000000","message":"try to have an if condition rather than catching the exception to see if the metadata is set","commit_id":"e34bdf9096af887937b203739231068d1a6d2bb5"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"15e9d91bbb5c95ea38ef2e50f2b283cbee3e97a9","unresolved":false,"context_lines":[{"line_number":198,"context_line":"        os_type \u003d image_meta[\u0027os_type\u0027]"},{"line_number":199,"context_line":"        xentools \u003d image_meta[\u0027com.openstack__1__xentools\u0027]"},{"line_number":200,"context_line":"    except KeyError:"},{"line_number":201,"context_line":"        return device_id"},{"line_number":202,"context_line":"    if(os_type \u003d\u003d \"windows\" and float(xentools) \u003e\u003d 6.1):"},{"line_number":203,"context_line":"        device_id \u003d 2"},{"line_number":204,"context_line":"    return device_id"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2Ff4s%3D","line":201,"in_reply_to":"AAAATn%2F%2Ff%2BQ%3D","updated":"2013-11-04 12:18:38.000000000","message":"Done","commit_id":"e34bdf9096af887937b203739231068d1a6d2bb5"},{"author":{"_account_id":6992,"name":"Kaushik Chandrashekar","email":"kaushik.chandrashekar@rackspace.com","username":"kaushikc"},"change_message_id":"96c122e74269190aec4eb7ed193df8570c7744a0","unresolved":false,"context_lines":[{"line_number":284,"context_line":""},{"line_number":285,"context_line":"    hypervisor_version \u003d session.product_version"},{"line_number":286,"context_line":"    if(utils.convert_version_to_int(hypervisor_version) \u003e"},{"line_number":287,"context_line":"           utils.convert_version_to_int(\u00276.0\u0027)):"},{"line_number":288,"context_line":"        rec[\u0027platform\u0027][\u0027device_id\u0027] \u003d get_device_id(image_meta\u003dimage_meta)"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    vm_ref \u003d session.call_xenapi(\u0027VM.create\u0027, rec)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2Ff%2BI%3D","line":287,"updated":"2013-11-04 10:38:44.000000000","message":"Will this take care of cases where the hypervisor is something like 5.6.1 or just 7?  Would it be safer to get the major number and compare that instead?","commit_id":"e34bdf9096af887937b203739231068d1a6d2bb5"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"15e9d91bbb5c95ea38ef2e50f2b283cbee3e97a9","unresolved":false,"context_lines":[{"line_number":284,"context_line":""},{"line_number":285,"context_line":"    hypervisor_version \u003d session.product_version"},{"line_number":286,"context_line":"    if(utils.convert_version_to_int(hypervisor_version) \u003e"},{"line_number":287,"context_line":"           utils.convert_version_to_int(\u00276.0\u0027)):"},{"line_number":288,"context_line":"        rec[\u0027platform\u0027][\u0027device_id\u0027] \u003d get_device_id(image_meta\u003dimage_meta)"},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"    vm_ref \u003d session.call_xenapi(\u0027VM.create\u0027, rec)"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAATn%2F%2Ff4w%3D","line":287,"in_reply_to":"AAAATn%2F%2Ff%2BI%3D","updated":"2013-11-04 12:18:38.000000000","message":"Done","commit_id":"e34bdf9096af887937b203739231068d1a6d2bb5"},{"author":{"_account_id":8430,"name":"Christopher Lefelhocz","email":"christopher.lefelhoc@rackspace.com","username":"leif"},"change_message_id":"3f7257451d743cb230ae9b448ea6c9cd8f1aa061","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        }.get(image_type_id)"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"def get_device_id(image_meta\u003d{}):"},{"line_number":196,"context_line":"    os_type \u003d image_meta.get(\u0027os_type\u0027, None)"},{"line_number":197,"context_line":"    xentools \u003d image_meta.get(\u0027com.openstack__1__xentools\u0027, None)"},{"line_number":198,"context_line":"    device_id \u003d 1"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2Ffb8%3D","line":195,"updated":"2013-11-04 17:33:35.000000000","message":"Based on the comments below, I think a table mapping meta-data strings to device ids may be more appropriate.  That way if we need to update it in the future we just update the code rather than some twisted logic.\n\nAdding a default if it doesn\u0027t match in this case would be straightforward as well.","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"3a81c273d6a3fa8160d4bbeb55ef2f64bfd11500","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        }.get(image_type_id)"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"def get_device_id(image_meta\u003d{}):"},{"line_number":196,"context_line":"    os_type \u003d image_meta.get(\u0027os_type\u0027, None)"},{"line_number":197,"context_line":"    xentools \u003d image_meta.get(\u0027com.openstack__1__xentools\u0027, None)"},{"line_number":198,"context_line":"    device_id \u003d 1"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2Ffhg%3D","line":195,"updated":"2013-11-04 16:36:16.000000000","message":"Here, and in all other places you set the argument, you should use None rather than {}.  And then in this method add \u0027if image_meta is None; image_meta \u003d {}\u0027.","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"d22640e2d96fd5d58e28558e6c5d31e6e481b0da","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        }.get(image_type_id)"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"def get_device_id(image_meta\u003d{}):"},{"line_number":196,"context_line":"    os_type \u003d image_meta.get(\u0027os_type\u0027, None)"},{"line_number":197,"context_line":"    xentools \u003d image_meta.get(\u0027com.openstack__1__xentools\u0027, None)"},{"line_number":198,"context_line":"    device_id \u003d 1"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2FeO4%3D","line":195,"in_reply_to":"AAAATn%2F%2Ffb8%3D","updated":"2013-11-05 09:19:32.000000000","message":"We may not be able to maintain a table for this because we\u0027ll have to keep one key value pair for each possible xentools version. For future versions added, we\u0027ll have to remember to add a new mapping.","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"d22640e2d96fd5d58e28558e6c5d31e6e481b0da","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        }.get(image_type_id)"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"def get_device_id(image_meta\u003d{}):"},{"line_number":196,"context_line":"    os_type \u003d image_meta.get(\u0027os_type\u0027, None)"},{"line_number":197,"context_line":"    xentools \u003d image_meta.get(\u0027com.openstack__1__xentools\u0027, None)"},{"line_number":198,"context_line":"    device_id \u003d 1"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2Feq8%3D","line":195,"in_reply_to":"AAAATn%2F%2Ffhg%3D","updated":"2013-11-05 09:19:32.000000000","message":"Done","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"e58615002b3a8918a5f386d1c5be3fdaf5af87e6","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        }.get(image_type_id)"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"def get_device_id(image_meta\u003d{}):"},{"line_number":196,"context_line":"    os_type \u003d image_meta.get(\u0027os_type\u0027, None)"},{"line_number":197,"context_line":"    xentools \u003d image_meta.get(\u0027com.openstack__1__xentools\u0027, None)"},{"line_number":198,"context_line":"    device_id \u003d 1"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2FfgY%3D","line":195,"in_reply_to":"AAAATn%2F%2Ffhg%3D","updated":"2013-11-04 16:49:46.000000000","message":"Yup.  Default values of empty dicts are dangerous because the same mutable dictionary instance gets re-used for each invocation of the function.\n\nAlso a docstring here would be cool to explain what exactly the function is for.","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"e58615002b3a8918a5f386d1c5be3fdaf5af87e6","unresolved":false,"context_lines":[{"line_number":195,"context_line":"def get_device_id(image_meta\u003d{}):"},{"line_number":196,"context_line":"    os_type \u003d image_meta.get(\u0027os_type\u0027, None)"},{"line_number":197,"context_line":"    xentools \u003d image_meta.get(\u0027com.openstack__1__xentools\u0027, None)"},{"line_number":198,"context_line":"    device_id \u003d 1"},{"line_number":199,"context_line":"    if not xentools:"},{"line_number":200,"context_line":"        return device_id"},{"line_number":201,"context_line":"    xentools_higher_than_or_equal_to_6_1 \u003d\\"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2FfgQ%3D","line":198,"updated":"2013-11-04 16:49:46.000000000","message":"What does a value of 1 represent?","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"d22640e2d96fd5d58e28558e6c5d31e6e481b0da","unresolved":false,"context_lines":[{"line_number":195,"context_line":"def get_device_id(image_meta\u003d{}):"},{"line_number":196,"context_line":"    os_type \u003d image_meta.get(\u0027os_type\u0027, None)"},{"line_number":197,"context_line":"    xentools \u003d image_meta.get(\u0027com.openstack__1__xentools\u0027, None)"},{"line_number":198,"context_line":"    device_id \u003d 1"},{"line_number":199,"context_line":"    if not xentools:"},{"line_number":200,"context_line":"        return device_id"},{"line_number":201,"context_line":"    xentools_higher_than_or_equal_to_6_1 \u003d\\"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2FeOk%3D","line":198,"in_reply_to":"AAAATn%2F%2FeVc%3D","updated":"2013-11-05 09:19:32.000000000","message":"Yes, we\u0027ll change this to be a string.","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"12e16d5fca7160fc4d60d9570f34e351c305e0ca","unresolved":false,"context_lines":[{"line_number":195,"context_line":"def get_device_id(image_meta\u003d{}):"},{"line_number":196,"context_line":"    os_type \u003d image_meta.get(\u0027os_type\u0027, None)"},{"line_number":197,"context_line":"    xentools \u003d image_meta.get(\u0027com.openstack__1__xentools\u0027, None)"},{"line_number":198,"context_line":"    device_id \u003d 1"},{"line_number":199,"context_line":"    if not xentools:"},{"line_number":200,"context_line":"        return device_id"},{"line_number":201,"context_line":"    xentools_higher_than_or_equal_to_6_1 \u003d\\"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2FeVc%3D","line":198,"in_reply_to":"AAAATn%2F%2FfgQ%3D","updated":"2013-11-05 08:34:24.000000000","message":"I think we should use \"0001\" and \"0002\" as strings rather than ints.  The device_id field is typically set to the full string, rather than the short int.\n\nFor Brian, 0001 and 0002 represent different hardware profiles to be exposed by qemu to the guests.","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"3a81c273d6a3fa8160d4bbeb55ef2f64bfd11500","unresolved":false,"context_lines":[{"line_number":201,"context_line":"    xentools_higher_than_or_equal_to_6_1 \u003d\\"},{"line_number":202,"context_line":"        (utils.convert_version_to_int(str(xentools)) \u003e\u003d"},{"line_number":203,"context_line":"         utils.convert_version_to_int(\u00276.1\u0027))"},{"line_number":204,"context_line":"    if(os_type \u003d\u003d \"windows\" and xentools_higher_than_or_equal_to_6_1):"},{"line_number":205,"context_line":"        device_id \u003d 2"},{"line_number":206,"context_line":"    return device_id"},{"line_number":207,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2Ffg0%3D","line":204,"updated":"2013-11-04 16:36:16.000000000","message":"what happens if xentools is not \u003e\u003d 6.1 and device_id \u003d\u003d 1?  Why only set it to 2 in this case?","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"d22640e2d96fd5d58e28558e6c5d31e6e481b0da","unresolved":false,"context_lines":[{"line_number":201,"context_line":"    xentools_higher_than_or_equal_to_6_1 \u003d\\"},{"line_number":202,"context_line":"        (utils.convert_version_to_int(str(xentools)) \u003e\u003d"},{"line_number":203,"context_line":"         utils.convert_version_to_int(\u00276.1\u0027))"},{"line_number":204,"context_line":"    if(os_type \u003d\u003d \"windows\" and xentools_higher_than_or_equal_to_6_1):"},{"line_number":205,"context_line":"        device_id \u003d 2"},{"line_number":206,"context_line":"    return device_id"},{"line_number":207,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2FeOM%3D","line":204,"in_reply_to":"AAAATn%2F%2FeU8%3D","updated":"2013-11-05 09:19:32.000000000","message":"xentools version will tell us the value of the xentools version on the image that it is being built from. \n\nThats the reason we are setting the device_id to 2 only if its windows and has new style of xen tools (6.1 and above)","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"12e16d5fca7160fc4d60d9570f34e351c305e0ca","unresolved":false,"context_lines":[{"line_number":201,"context_line":"    xentools_higher_than_or_equal_to_6_1 \u003d\\"},{"line_number":202,"context_line":"        (utils.convert_version_to_int(str(xentools)) \u003e\u003d"},{"line_number":203,"context_line":"         utils.convert_version_to_int(\u00276.1\u0027))"},{"line_number":204,"context_line":"    if(os_type \u003d\u003d \"windows\" and xentools_higher_than_or_equal_to_6_1):"},{"line_number":205,"context_line":"        device_id \u003d 2"},{"line_number":206,"context_line":"    return device_id"},{"line_number":207,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2FeU8%3D","line":204,"in_reply_to":"AAAATn%2F%2Ffg0%3D","updated":"2013-11-05 08:34:24.000000000","message":"If xen tools is \u003c 6.1 then device id should always be 1, not 2.\nIf xen tools is \u003e 6.1 then device_id should be 2 only if the new style tools are installed - but I guess one question is what is this xentools value set to (I\u0027ll probably find out later); it seems to be related to the XenServer version - but even in XenServer 6.1 some guests (Linux, but also XP) get installed with the \"old style\" tools requiring 0001 as the device_id.\n\nI think that we shouldn\u0027t be confusing the tools version with the hardware ID being exposed and the image property should be about the hardware ID required (equivalent to it being on the XenServer VM template).","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"e58615002b3a8918a5f386d1c5be3fdaf5af87e6","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        (utils.convert_version_to_int(str(xentools)) \u003e\u003d"},{"line_number":203,"context_line":"         utils.convert_version_to_int(\u00276.1\u0027))"},{"line_number":204,"context_line":"    if(os_type \u003d\u003d \"windows\" and xentools_higher_than_or_equal_to_6_1):"},{"line_number":205,"context_line":"        device_id \u003d 2"},{"line_number":206,"context_line":"    return device_id"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2FfgM%3D","line":205,"updated":"2013-11-04 16:49:46.000000000","message":"Again, what is a 2?","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"d22640e2d96fd5d58e28558e6c5d31e6e481b0da","unresolved":false,"context_lines":[{"line_number":202,"context_line":"        (utils.convert_version_to_int(str(xentools)) \u003e\u003d"},{"line_number":203,"context_line":"         utils.convert_version_to_int(\u00276.1\u0027))"},{"line_number":204,"context_line":"    if(os_type \u003d\u003d \"windows\" and xentools_higher_than_or_equal_to_6_1):"},{"line_number":205,"context_line":"        device_id \u003d 2"},{"line_number":206,"context_line":"    return device_id"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2FeOI%3D","line":205,"in_reply_to":"AAAATn%2F%2FfgM%3D","updated":"2013-11-05 09:19:32.000000000","message":"Done","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"3a81c273d6a3fa8160d4bbeb55ef2f64bfd11500","unresolved":false,"context_lines":[{"line_number":285,"context_line":"        rec[\u0027HVM_boot_policy\u0027] \u003d \u0027BIOS order\u0027"},{"line_number":286,"context_line":""},{"line_number":287,"context_line":"    hypervisor_version \u003d session.product_version"},{"line_number":288,"context_line":"    if(utils.convert_version_to_int(hypervisor_version) \u003e"},{"line_number":289,"context_line":"           utils.convert_version_to_int(\u00276.0\u0027)):"},{"line_number":290,"context_line":"        rec[\u0027platform\u0027][\u0027device_id\u0027] \u003d get_device_id(image_meta\u003dimage_meta)"},{"line_number":291,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2Ffgo%3D","line":288,"updated":"2013-11-04 16:36:16.000000000","message":"What happens if you set this for xen \u003c\u003d 6.0?","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"d22640e2d96fd5d58e28558e6c5d31e6e481b0da","unresolved":false,"context_lines":[{"line_number":285,"context_line":"        rec[\u0027HVM_boot_policy\u0027] \u003d \u0027BIOS order\u0027"},{"line_number":286,"context_line":""},{"line_number":287,"context_line":"    hypervisor_version \u003d session.product_version"},{"line_number":288,"context_line":"    if(utils.convert_version_to_int(hypervisor_version) \u003e"},{"line_number":289,"context_line":"           utils.convert_version_to_int(\u00276.0\u0027)):"},{"line_number":290,"context_line":"        rec[\u0027platform\u0027][\u0027device_id\u0027] \u003d get_device_id(image_meta\u003dimage_meta)"},{"line_number":291,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2FeOA%3D","line":288,"in_reply_to":"AAAATn%2F%2Ffgo%3D","updated":"2013-11-05 09:19:32.000000000","message":"As Bob Ball mentioned, this VM might not even boot up.\nSo, we\u0027ll have to set the device_id only for hypervisor_version \u003e 6.0","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"e58615002b3a8918a5f386d1c5be3fdaf5af87e6","unresolved":false,"context_lines":[{"line_number":286,"context_line":""},{"line_number":287,"context_line":"    hypervisor_version \u003d session.product_version"},{"line_number":288,"context_line":"    if(utils.convert_version_to_int(hypervisor_version) \u003e"},{"line_number":289,"context_line":"           utils.convert_version_to_int(\u00276.0\u0027)):"},{"line_number":290,"context_line":"        rec[\u0027platform\u0027][\u0027device_id\u0027] \u003d get_device_id(image_meta\u003dimage_meta)"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"    vm_ref \u003d session.call_xenapi(\u0027VM.create\u0027, rec)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2Fffs%3D","line":289,"updated":"2013-11-04 16:49:46.000000000","message":"The \u0027convert_version_to_int\u0027 function appears to expect a tuple.\n\nUnit test this please - I don\u0027t think it works.","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"d22640e2d96fd5d58e28558e6c5d31e6e481b0da","unresolved":false,"context_lines":[{"line_number":286,"context_line":""},{"line_number":287,"context_line":"    hypervisor_version \u003d session.product_version"},{"line_number":288,"context_line":"    if(utils.convert_version_to_int(hypervisor_version) \u003e"},{"line_number":289,"context_line":"           utils.convert_version_to_int(\u00276.0\u0027)):"},{"line_number":290,"context_line":"        rec[\u0027platform\u0027][\u0027device_id\u0027] \u003d get_device_id(image_meta\u003dimage_meta)"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"    vm_ref \u003d session.call_xenapi(\u0027VM.create\u0027, rec)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2FeN8%3D","line":289,"in_reply_to":"AAAATn%2F%2Fffs%3D","updated":"2013-11-05 09:19:32.000000000","message":"The method can handle string inputs as well.","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"e58615002b3a8918a5f386d1c5be3fdaf5af87e6","unresolved":false,"context_lines":[{"line_number":287,"context_line":"    hypervisor_version \u003d session.product_version"},{"line_number":288,"context_line":"    if(utils.convert_version_to_int(hypervisor_version) \u003e"},{"line_number":289,"context_line":"           utils.convert_version_to_int(\u00276.0\u0027)):"},{"line_number":290,"context_line":"        rec[\u0027platform\u0027][\u0027device_id\u0027] \u003d get_device_id(image_meta\u003dimage_meta)"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"    vm_ref \u003d session.call_xenapi(\u0027VM.create\u0027, rec)"},{"line_number":293,"context_line":"    LOG.debug(_(\u0027Created VM\u0027), instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2Fffk%3D","line":290,"updated":"2013-11-04 16:49:46.000000000","message":"Add a comment explaining the purpose","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"d22640e2d96fd5d58e28558e6c5d31e6e481b0da","unresolved":false,"context_lines":[{"line_number":287,"context_line":"    hypervisor_version \u003d session.product_version"},{"line_number":288,"context_line":"    if(utils.convert_version_to_int(hypervisor_version) \u003e"},{"line_number":289,"context_line":"           utils.convert_version_to_int(\u00276.0\u0027)):"},{"line_number":290,"context_line":"        rec[\u0027platform\u0027][\u0027device_id\u0027] \u003d get_device_id(image_meta\u003dimage_meta)"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"    vm_ref \u003d session.call_xenapi(\u0027VM.create\u0027, rec)"},{"line_number":293,"context_line":"    LOG.debug(_(\u0027Created VM\u0027), instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2FeM4%3D","line":290,"in_reply_to":"AAAATn%2F%2FeUY%3D","updated":"2013-11-05 09:19:32.000000000","message":"We set the device_id only for hypervisor versions 6.1 and above.","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"d22640e2d96fd5d58e28558e6c5d31e6e481b0da","unresolved":false,"context_lines":[{"line_number":287,"context_line":"    hypervisor_version \u003d session.product_version"},{"line_number":288,"context_line":"    if(utils.convert_version_to_int(hypervisor_version) \u003e"},{"line_number":289,"context_line":"           utils.convert_version_to_int(\u00276.0\u0027)):"},{"line_number":290,"context_line":"        rec[\u0027platform\u0027][\u0027device_id\u0027] \u003d get_device_id(image_meta\u003dimage_meta)"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"    vm_ref \u003d session.call_xenapi(\u0027VM.create\u0027, rec)"},{"line_number":293,"context_line":"    LOG.debug(_(\u0027Created VM\u0027), instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2FeNI%3D","line":290,"in_reply_to":"AAAATn%2F%2Fffk%3D","updated":"2013-11-05 09:19:32.000000000","message":"Done","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"12e16d5fca7160fc4d60d9570f34e351c305e0ca","unresolved":false,"context_lines":[{"line_number":287,"context_line":"    hypervisor_version \u003d session.product_version"},{"line_number":288,"context_line":"    if(utils.convert_version_to_int(hypervisor_version) \u003e"},{"line_number":289,"context_line":"           utils.convert_version_to_int(\u00276.0\u0027)):"},{"line_number":290,"context_line":"        rec[\u0027platform\u0027][\u0027device_id\u0027] \u003d get_device_id(image_meta\u003dimage_meta)"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"    vm_ref \u003d session.call_xenapi(\u0027VM.create\u0027, rec)"},{"line_number":293,"context_line":"    LOG.debug(_(\u0027Created VM\u0027), instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2FeUY%3D","line":290,"in_reply_to":"AAAATn%2F%2Fffk%3D","updated":"2013-11-05 08:34:24.000000000","message":"I think this is broken; if you boot a VM which requires device_id\u003d0002 on a host that does not support it, or for which it is not set, then I can\u0027t guarantee the VM will boot.\n\nThe check for device_id should therefore be implemented as a filter for the scheduler so it knows which hypervisors it can successfully boot on.","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"0c3ce61ff172c66a1ddff4b47ed9856d2a561770","unresolved":false,"context_lines":[{"line_number":192,"context_line":"        }.get(image_type_id)"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"def get_device_id(image_meta\u003dNone):"},{"line_number":196,"context_line":"    \"\"\"device_id should be 2 for windows VMs which run new xentools (\u003e\u003d6.1)"},{"line_number":197,"context_line":"    refer to http://support.citrix.com/article/CTX135099 for more info"},{"line_number":198,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAATn%2F%2Fc68%3D","line":195,"updated":"2013-11-05 22:12:00.000000000","message":"Kudos.  This is a whole lot easier to read broken out into a separate function.\n\nAlso, great docstring!","commit_id":"ac00b45bb760b9255bc2955abcc07af217cd0c1f"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"6c0b8c5757e443e47d91af5f5d29de68ef188316","unresolved":false,"context_lines":[{"line_number":210,"context_line":"    if not xentools or os_type !\u003d \"windows\":"},{"line_number":211,"context_line":"        return device_id"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    if utils.get_major_minor_version(xentools) \u003e\u003d 6.1:"},{"line_number":214,"context_line":"        device_id \u003d \"0002\""},{"line_number":215,"context_line":"    return device_id"},{"line_number":216,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2FcE4%3D","line":213,"updated":"2013-11-06 07:09:47.000000000","message":"I\u0027m still concerned by this check; it depends on com.openstack__1__xentools is set - which is not clear.\n\nWhen you are using XP or 2k3 with XenServer 6.1, the device_id must be 0001 - this is because the new style tools are only installed with newer versions of windows, even on XenServer 6.1.\n\nIf you are using Windows 2008 which has 6.0 tools installed, the device_id must be 0001.\n\nIf you are using Windows 2008 which has the 6.1 tools re-installed then it must be 0002.  This means that if a customer has upgraded their PV tools while the image was running then we  should be providing the updated hardware ID.  There is logic in XAPI to make this change to the VM record while it is running - perhaps we need logic to transfer it to the image so that when it is backed up to glance the details are transferred.\n\nThe code above seems to get the logic wrong in some cases - e.g. 2K3 running on XS 6.1.","commit_id":"04d7b771b039ae7aff52be8bd92de2a42fe922a0"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"6d103e6487523403c9817885e2f817538ccfcbbc","unresolved":false,"context_lines":[{"line_number":210,"context_line":"    if not xentools or os_type !\u003d \"windows\":"},{"line_number":211,"context_line":"        return device_id"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    if utils.get_major_minor_version(xentools) \u003e\u003d 6.1:"},{"line_number":214,"context_line":"        device_id \u003d \"0002\""},{"line_number":215,"context_line":"    return device_id"},{"line_number":216,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2FZ40%3D","line":213,"in_reply_to":"AAAATn%2F%2FaMQ%3D","updated":"2013-11-07 04:57:13.000000000","message":"How about setting the device_id itself in the image_metadata? Then we wouldn\u0027t need to derive this value using the xentools version and os type.\n\nWe can provide documentation around what this value should be set to.","commit_id":"04d7b771b039ae7aff52be8bd92de2a42fe922a0"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"118510ab2e32a992934c220b47a40807b8e8f7fd","unresolved":false,"context_lines":[{"line_number":210,"context_line":"    if not xentools or os_type !\u003d \"windows\":"},{"line_number":211,"context_line":"        return device_id"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    if utils.get_major_minor_version(xentools) \u003e\u003d 6.1:"},{"line_number":214,"context_line":"        device_id \u003d \"0002\""},{"line_number":215,"context_line":"    return device_id"},{"line_number":216,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2FaMQ%3D","line":213,"in_reply_to":"AAAATn%2F%2FbHo%3D","updated":"2013-11-07 01:24:25.000000000","message":"My issue with how this is set is around the logic for when it is set really.  I guess it\u0027s a manual thing, but if you do not have it set and then the user upgrades the version of the PV tools in-guest the device_id should be updated to make use of the PV devices.  Perhaps this is OK and just needs documenting as users upgrading their xen tools in this way is unusual.\n\nWe certainly need a docimpact flag or corresponding change detailing how to set up images.\n\nIf we really are just relying on this value to determine what hardware profile we should present to the guest then we shouldn\u0027t check the os_type being windows; if we want to use the new hardware profile then we use it.","commit_id":"04d7b771b039ae7aff52be8bd92de2a42fe922a0"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"03f7a33130cbb10fca75a9b4d73159d89a78121b","unresolved":false,"context_lines":[{"line_number":210,"context_line":"    if not xentools or os_type !\u003d \"windows\":"},{"line_number":211,"context_line":"        return device_id"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    if utils.get_major_minor_version(xentools) \u003e\u003d 6.1:"},{"line_number":214,"context_line":"        device_id \u003d \"0002\""},{"line_number":215,"context_line":"    return device_id"},{"line_number":216,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2FbHo%3D","line":213,"in_reply_to":"AAAATn%2F%2FcE4%3D","updated":"2013-11-06 15:04:35.000000000","message":"Bob, if you\u0027re concerned that the image metadata property may not be set, then how *should* the code detect if the newer tools are installed?","commit_id":"04d7b771b039ae7aff52be8bd92de2a42fe922a0"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"6c0b8c5757e443e47d91af5f5d29de68ef188316","unresolved":false,"context_lines":[{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    if utils.get_major_minor_version(xentools) \u003e\u003d 6.1:"},{"line_number":214,"context_line":"        device_id \u003d \"0002\""},{"line_number":215,"context_line":"    return device_id"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"def create_vm(session, instance, name_label, kernel, ramdisk,"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2FcFE%3D","line":215,"updated":"2013-11-06 07:09:47.000000000","message":"Can we have a test in here that\u0027ll error if the hypervisor version is \u003c 6.1 and we are trying to set device_id to be 0002?","commit_id":"04d7b771b039ae7aff52be8bd92de2a42fe922a0"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"118510ab2e32a992934c220b47a40807b8e8f7fd","unresolved":false,"context_lines":[{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    if utils.get_major_minor_version(xentools) \u003e\u003d 6.1:"},{"line_number":214,"context_line":"        device_id \u003d \"0002\""},{"line_number":215,"context_line":"    return device_id"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"def create_vm(session, instance, name_label, kernel, ramdisk,"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2FaLw%3D","line":215,"in_reply_to":"AAAATn%2F%2FbHc%3D","updated":"2013-11-07 01:24:25.000000000","message":"Absolutely - just a sanity check.  But I think it\u0027s worth having as you can easily forget to set up the scheduler filter and I\u0027d rather have an error which we can see obviously here rather than an obscure error with the VM not starting up properly or just not using the PV devices.","commit_id":"04d7b771b039ae7aff52be8bd92de2a42fe922a0"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"03f7a33130cbb10fca75a9b4d73159d89a78121b","unresolved":false,"context_lines":[{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    if utils.get_major_minor_version(xentools) \u003e\u003d 6.1:"},{"line_number":214,"context_line":"        device_id \u003d \"0002\""},{"line_number":215,"context_line":"    return device_id"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"def create_vm(session, instance, name_label, kernel, ramdisk,"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2FbHc%3D","line":215,"in_reply_to":"AAAATn%2F%2FcFE%3D","updated":"2013-11-06 15:04:35.000000000","message":"This would just be a sanity check of last resort, right?  In theory, we shouldn\u0027t be scheduling instances with newer tools to oldest hypervisors (\u003c 6.1) that cannot start the VM?","commit_id":"04d7b771b039ae7aff52be8bd92de2a42fe922a0"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"6c0b8c5757e443e47d91af5f5d29de68ef188316","unresolved":false,"context_lines":[{"line_number":297,"context_line":""},{"line_number":298,"context_line":"    # The device_id is required to be set for hypervisor version 6.1 and above"},{"line_number":299,"context_line":"    hypervisor_version \u003d session.product_version"},{"line_number":300,"context_line":"    if utils.get_major_minor_version(hypervisor_version) \u003e\u003d 6.1:"},{"line_number":301,"context_line":"        rec[\u0027platform\u0027][\u0027device_id\u0027] \u003d get_device_id(image_meta\u003dimage_meta)"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"    vm_ref \u003d session.call_xenapi(\u0027VM.create\u0027, rec)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2FcFM%3D","line":300,"updated":"2013-11-06 07:09:47.000000000","message":"Could this use versionutils.is_compatible?  Slight change in definition, however might make more sense.\nversionutils is in oslo - so perhaps get_major_minor_version should be there rather than nova.","commit_id":"04d7b771b039ae7aff52be8bd92de2a42fe922a0"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"118510ab2e32a992934c220b47a40807b8e8f7fd","unresolved":false,"context_lines":[{"line_number":297,"context_line":""},{"line_number":298,"context_line":"    # The device_id is required to be set for hypervisor version 6.1 and above"},{"line_number":299,"context_line":"    hypervisor_version \u003d session.product_version"},{"line_number":300,"context_line":"    if utils.get_major_minor_version(hypervisor_version) \u003e\u003d 6.1:"},{"line_number":301,"context_line":"        rec[\u0027platform\u0027][\u0027device_id\u0027] \u003d get_device_id(image_meta\u003dimage_meta)"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"    vm_ref \u003d session.call_xenapi(\u0027VM.create\u0027, rec)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2FaLk%3D","line":300,"in_reply_to":"AAAATn%2F%2FbG4%3D","updated":"2013-11-07 01:24:25.000000000","message":"Agreed.  It defaults to 0001 if it\u0027s not set, and is ignored on \u003c6.1","commit_id":"04d7b771b039ae7aff52be8bd92de2a42fe922a0"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"03f7a33130cbb10fca75a9b4d73159d89a78121b","unresolved":false,"context_lines":[{"line_number":297,"context_line":""},{"line_number":298,"context_line":"    # The device_id is required to be set for hypervisor version 6.1 and above"},{"line_number":299,"context_line":"    hypervisor_version \u003d session.product_version"},{"line_number":300,"context_line":"    if utils.get_major_minor_version(hypervisor_version) \u003e\u003d 6.1:"},{"line_number":301,"context_line":"        rec[\u0027platform\u0027][\u0027device_id\u0027] \u003d get_device_id(image_meta\u003dimage_meta)"},{"line_number":302,"context_line":""},{"line_number":303,"context_line":"    vm_ref \u003d session.call_xenapi(\u0027VM.create\u0027, rec)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAATn%2F%2FbG4%3D","line":300,"in_reply_to":"AAAATn%2F%2FcFM%3D","updated":"2013-11-06 15:04:35.000000000","message":"Wondering if we can get rid of the hypervisor version check here based on Bob\u0027s comment on this patch set that the device id setting  should be independent of the hypervisor version.\n\nCouldn\u0027t we just always set rec[\u0027platform\u0027][\u0027device_id\u0027] then?","commit_id":"04d7b771b039ae7aff52be8bd92de2a42fe922a0"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"8a7fc3539d83cfb124d2eb24b5a015d5e1e5fb01","unresolved":false,"context_lines":[{"line_number":201,"context_line":"    # The device_id is required to be set for hypervisor version 6.1 and above"},{"line_number":202,"context_line":"    hypervisor_version \u003d session.product_version"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"    if utils.get_major_minor_version(hypervisor_version) \u003e\u003d 6.1:"},{"line_number":205,"context_line":"        device_id \u003d image_meta.get(\u0027com.openstack__1__xs_device_id\u0027, \u00270001\u0027)"},{"line_number":206,"context_line":"        return device_id"},{"line_number":207,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2FX1c%3D","line":204,"updated":"2013-11-08 01:18:02.000000000","message":"I think this logic should be the other way round; we should check xs_device_id always - if it\u0027s set on the image and we try to boot on a \u003c6.1 hypervisor then there\u0027s a problem.\nOtherwise a VM might boot on a hypervisor expecting to set device_id but it doesn\u0027t get set and that could cause errors / confusion.\n\nWhether we error or not we error if device_id is 0002 and the hypervisor is \u003c 6.1 is a different question.  I think with the logic the other way round it\u0027s easy enough therefore I think we should add it as a check that the scheduler filter has worked and we\u0027re not doing something we know will fail.","commit_id":"03c89cfc96db784a3d4cc976248d0247eb1b1dd9"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"74ce92e3474652a3ed7a12ee5654190205fbe41c","unresolved":false,"context_lines":[{"line_number":201,"context_line":"    # The device_id is required to be set for hypervisor version 6.1 and above"},{"line_number":202,"context_line":"    hypervisor_version \u003d session.product_version"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"    if utils.get_major_minor_version(hypervisor_version) \u003e\u003d 6.1:"},{"line_number":205,"context_line":"        device_id \u003d image_meta.get(\u0027com.openstack__1__xs_device_id\u0027, \u00270001\u0027)"},{"line_number":206,"context_line":"        return device_id"},{"line_number":207,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAATn%2F%2FXaY%3D","line":204,"in_reply_to":"AAAATn%2F%2FX1c%3D","updated":"2013-11-08 05:32:31.000000000","message":"Done","commit_id":"03c89cfc96db784a3d4cc976248d0247eb1b1dd9"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"eea79714885438eeab5d6f4a779037963140d4e5","unresolved":false,"context_lines":[{"line_number":203,"context_line":""},{"line_number":204,"context_line":"    # The device_id is required to be set for hypervisor version 6.1 and above"},{"line_number":205,"context_line":"    if device_id and utils.get_major_minor_version(hypervisor_version) \u003e\u003d 6.1:"},{"line_number":206,"context_line":"        vm_record[\u0027platform\u0027][\u0027device_id\u0027] \u003d device_id"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"def create_vm(session, instance, name_label, kernel, ramdisk,"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAATn%2F%2FXZ8%3D","line":206,"updated":"2013-11-08 05:42:34.000000000","message":"A definite improvement - but what I meant was something like the following (need a better exception string)\n\ndevice_id \u003d image_meta.get(...)\nif device_id:\n  if utils.get_maj... \u003e\u003d6.1:\n    vm_record[... \u003d device_id\n  else:\n    raise \"Device ID %s required, but not supported by hypervisor version %s\"","commit_id":"e27efb6ba9e2f1aad892da6f655cb64bf5c5912e"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"unresolved":false,"context_lines":[{"line_number":203,"context_line":""},{"line_number":204,"context_line":"    # The device_id is required to be set for hypervisor version 6.1 and above"},{"line_number":205,"context_line":"    if device_id and utils.get_major_minor_version(hypervisor_version) \u003e\u003d 6.1:"},{"line_number":206,"context_line":"        vm_record[\u0027platform\u0027][\u0027device_id\u0027] \u003d device_id"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"def create_vm(session, instance, name_label, kernel, ramdisk,"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAATn%2F%2FWJ4%3D","line":206,"in_reply_to":"AAAATn%2F%2FWLE%3D","updated":"2013-11-08 18:18:24.000000000","message":"Makes sense. Done.","commit_id":"e27efb6ba9e2f1aad892da6f655cb64bf5c5912e"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"b485b984a7e2837bbff6c24497fe426b91e11b67","unresolved":false,"context_lines":[{"line_number":203,"context_line":""},{"line_number":204,"context_line":"    # The device_id is required to be set for hypervisor version 6.1 and above"},{"line_number":205,"context_line":"    if device_id and utils.get_major_minor_version(hypervisor_version) \u003e\u003d 6.1:"},{"line_number":206,"context_line":"        vm_record[\u0027platform\u0027][\u0027device_id\u0027] \u003d device_id"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"def create_vm(session, instance, name_label, kernel, ramdisk,"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAATn%2F%2FWLE%3D","line":206,"in_reply_to":"AAAATn%2F%2FWPA%3D","updated":"2013-11-08 17:19:34.000000000","message":"I mean raise an exception *only* if the device id is present and the hypervisor \u003c 6.1.  Like in Bob\u0027s pseudocode above.","commit_id":"e27efb6ba9e2f1aad892da6f655cb64bf5c5912e"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"unresolved":false,"context_lines":[{"line_number":203,"context_line":""},{"line_number":204,"context_line":"    # The device_id is required to be set for hypervisor version 6.1 and above"},{"line_number":205,"context_line":"    if device_id and utils.get_major_minor_version(hypervisor_version) \u003e\u003d 6.1:"},{"line_number":206,"context_line":"        vm_record[\u0027platform\u0027][\u0027device_id\u0027] \u003d device_id"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"def create_vm(session, instance, name_label, kernel, ramdisk,"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAATn%2F%2FWPA%3D","line":206,"in_reply_to":"AAAATn%2F%2FWPg%3D","updated":"2013-11-08 16:22:28.000000000","message":"Agreed, the exception handling would be a cleaner approach.\n\nBut we would have to set the device_id on the metadata of each and every image that will use hypervisor_version \u003e\u003d 6.1\n\nDo we want to set such a restriction?","commit_id":"e27efb6ba9e2f1aad892da6f655cb64bf5c5912e"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"unresolved":false,"context_lines":[{"line_number":203,"context_line":""},{"line_number":204,"context_line":"    # The device_id is required to be set for hypervisor version 6.1 and above"},{"line_number":205,"context_line":"    if device_id and utils.get_major_minor_version(hypervisor_version) \u003e\u003d 6.1:"},{"line_number":206,"context_line":"        vm_record[\u0027platform\u0027][\u0027device_id\u0027] \u003d device_id"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"def create_vm(session, instance, name_label, kernel, ramdisk,"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAATn%2F%2FWOw%3D","line":206,"in_reply_to":"AAAATn%2F%2FWPg%3D","updated":"2013-11-08 16:23:27.000000000","message":"The VM might fail to boot up, when the hypervisor_version \u003e\u003d 6.1, and the vm is a windows one, and the device_id is not set.","commit_id":"e27efb6ba9e2f1aad892da6f655cb64bf5c5912e"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"b485b984a7e2837bbff6c24497fe426b91e11b67","unresolved":false,"context_lines":[{"line_number":203,"context_line":""},{"line_number":204,"context_line":"    # The device_id is required to be set for hypervisor version 6.1 and above"},{"line_number":205,"context_line":"    if device_id and utils.get_major_minor_version(hypervisor_version) \u003e\u003d 6.1:"},{"line_number":206,"context_line":"        vm_record[\u0027platform\u0027][\u0027device_id\u0027] \u003d device_id"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"def create_vm(session, instance, name_label, kernel, ramdisk,"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAATn%2F%2FWPg%3D","line":206,"in_reply_to":"AAAATn%2F%2FWsU%3D","updated":"2013-11-08 16:13:13.000000000","message":"\"In the case where the device_id is set but the hypervisor_version \u003c 6.1, the build will still fail at a later stage. \"\n\nWhat is that later stage you refer to?  I like the idea of a fail-fast sanity check with a clear error message in this case.","commit_id":"e27efb6ba9e2f1aad892da6f655cb64bf5c5912e"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"f8f20a394131e8d35cb7940928e0ab71f3ae41c4","unresolved":false,"context_lines":[{"line_number":203,"context_line":""},{"line_number":204,"context_line":"    # The device_id is required to be set for hypervisor version 6.1 and above"},{"line_number":205,"context_line":"    if device_id and utils.get_major_minor_version(hypervisor_version) \u003e\u003d 6.1:"},{"line_number":206,"context_line":"        vm_record[\u0027platform\u0027][\u0027device_id\u0027] \u003d device_id"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"def create_vm(session, instance, name_label, kernel, ramdisk,"}],"source_content_type":"text/x-python","patch_set":10,"id":"AAAATn%2F%2FWsU%3D","line":206,"in_reply_to":"AAAATn%2F%2FXZ8%3D","updated":"2013-11-08 12:07:40.000000000","message":"In the case where the device_id is set but the hypervisor_version \u003c 6.1, the build will still fail at a later stage.\nRaising an exception if device_id is not set when the hypervisor_version \u003c 6.1, would mean that we have to ensure that it is set for all images. (Linux and Windows).\n\nInstead, if we don\u0027t raise the exception, we can just set the device_id only for images that require it to be set.","commit_id":"e27efb6ba9e2f1aad892da6f655cb64bf5c5912e"},{"author":{"_account_id":1030,"name":"Chris Behrens","email":"cbehrens@codestud.com","username":"cbehrens"},"unresolved":false,"context_lines":[{"line_number":198,"context_line":"    \"\"\""},{"line_number":199,"context_line":"    if image_meta is None:"},{"line_number":200,"context_line":"        image_meta \u003d {}"},{"line_number":201,"context_line":"    device_id \u003d image_meta.get(\u0027com.openstack__1__xs_device_id\u0027)"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    # The device_id is required to be set for hypervisor version 6.1 and above"},{"line_number":204,"context_line":"    if device_id:"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAATn%2F%2FWDw%3D","line":201,"updated":"2013-11-08 18:41:19.000000000","message":"This should be org.openstack__*  right?","commit_id":"7368c869ada447c930430091723357abf98e5ef0"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"unresolved":false,"context_lines":[{"line_number":208,"context_line":"        if hypervisor_major_minor_version \u003e\u003d 6.1:"},{"line_number":209,"context_line":"            vm_record[\u0027platform\u0027][\u0027device_id\u0027] \u003d device_id"},{"line_number":210,"context_line":"        else:"},{"line_number":211,"context_line":"            raise exception.NovaException(\"Device id %s required, but \""},{"line_number":212,"context_line":"                \"not supported by hypervisor version %s\""},{"line_number":213,"context_line":"                % (device_id, hypervisor_version))"},{"line_number":214,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAATn%2F%2FVlE%3D","line":211,"updated":"2013-11-09 09:31:09.000000000","message":"please use the i18n format","commit_id":"03987784ef94ed09ca48d3ab993d442d9388c429"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"unresolved":false,"context_lines":[{"line_number":205,"context_line":"        hypervisor_version \u003d session.product_version"},{"line_number":206,"context_line":"        hypervisor_major_minor_version \u003d utils.get_major_minor_version("},{"line_number":207,"context_line":"            session.product_version)"},{"line_number":208,"context_line":"        if hypervisor_major_minor_version \u003e\u003d 6.1:"},{"line_number":209,"context_line":"            vm_record[\u0027platform\u0027][\u0027device_id\u0027] \u003d device_id"},{"line_number":210,"context_line":"        else:"},{"line_number":211,"context_line":"            msg \u003d _(\"Device id %(id)s required, but not supported by \""}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F%2FTk8%3D","line":208,"updated":"2013-11-11 10:23:09.000000000","message":"I am not sure this is that easy. We need to check XCP versions as well, but they are going away, so lets ignore that for now.\n\nI would rather see a separate \"is_support_device_id\" function that does this test.","commit_id":"2582cea74af438ec25260608a1fe499c35f00a18"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"unresolved":false,"context_lines":[{"line_number":205,"context_line":"        hypervisor_version \u003d session.product_version"},{"line_number":206,"context_line":"        hypervisor_major_minor_version \u003d utils.get_major_minor_version("},{"line_number":207,"context_line":"            session.product_version)"},{"line_number":208,"context_line":"        if hypervisor_major_minor_version \u003e\u003d 6.1:"},{"line_number":209,"context_line":"            vm_record[\u0027platform\u0027][\u0027device_id\u0027] \u003d device_id"},{"line_number":210,"context_line":"        else:"},{"line_number":211,"context_line":"            msg \u003d _(\"Device id %(id)s required, but not supported by \""}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F%2FTRQ%3D","line":208,"in_reply_to":"AAAATn%2F%2FTk8%3D","updated":"2013-11-11 12:40:01.000000000","message":"Isn\u0027t using org.openstack... the convention that should be followed?","commit_id":"2582cea74af438ec25260608a1fe499c35f00a18"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"unresolved":false,"context_lines":[{"line_number":206,"context_line":"        hypervisor_major_minor_version \u003d utils.get_major_minor_version("},{"line_number":207,"context_line":"            session.product_version)"},{"line_number":208,"context_line":"        if hypervisor_major_minor_version \u003e\u003d 6.1:"},{"line_number":209,"context_line":"            vm_record[\u0027platform\u0027][\u0027device_id\u0027] \u003d device_id"},{"line_number":210,"context_line":"        else:"},{"line_number":211,"context_line":"            msg \u003d _(\"Device id %(id)s required, but not supported by \""},{"line_number":212,"context_line":"                    \"hypervisor version %(version)s\") % {\u0027id\u0027: device_id,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F%2FTk4%3D","line":209,"updated":"2013-11-11 10:23:09.000000000","message":"This function should just return the device_id to be set, or None if nothing should be set.","commit_id":"2582cea74af438ec25260608a1fe499c35f00a18"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"unresolved":false,"context_lines":[{"line_number":206,"context_line":"        hypervisor_major_minor_version \u003d utils.get_major_minor_version("},{"line_number":207,"context_line":"            session.product_version)"},{"line_number":208,"context_line":"        if hypervisor_major_minor_version \u003e\u003d 6.1:"},{"line_number":209,"context_line":"            vm_record[\u0027platform\u0027][\u0027device_id\u0027] \u003d device_id"},{"line_number":210,"context_line":"        else:"},{"line_number":211,"context_line":"            msg \u003d _(\"Device id %(id)s required, but not supported by \""},{"line_number":212,"context_line":"                    \"hypervisor version %(version)s\") % {\u0027id\u0027: device_id,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F%2FQIw%3D","line":209,"in_reply_to":"AAAATn%2F%2FTk4%3D","updated":"2013-11-12 08:50:29.000000000","message":"Done","commit_id":"2582cea74af438ec25260608a1fe499c35f00a18"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"unresolved":false,"context_lines":[{"line_number":211,"context_line":"            msg \u003d _(\"Device id %(id)s required, but not supported by \""},{"line_number":212,"context_line":"                    \"hypervisor version %(version)s\") % {\u0027id\u0027: device_id,"},{"line_number":213,"context_line":"                     \u0027version\u0027: hypervisor_version}"},{"line_number":214,"context_line":"            raise exception.NovaException(msg)"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"def create_vm(session, instance, name_label, kernel, ramdisk,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F%2FTko%3D","line":214,"updated":"2013-11-11 10:23:09.000000000","message":"Not sure if we have a better exception, but this should be OK. Maybe a needs newer hypervisor kind of thing?","commit_id":"2582cea74af438ec25260608a1fe499c35f00a18"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"unresolved":false,"context_lines":[{"line_number":211,"context_line":"            msg \u003d _(\"Device id %(id)s required, but not supported by \""},{"line_number":212,"context_line":"                    \"hypervisor version %(version)s\") % {\u0027id\u0027: device_id,"},{"line_number":213,"context_line":"                     \u0027version\u0027: hypervisor_version}"},{"line_number":214,"context_line":"            raise exception.NovaException(msg)"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"def create_vm(session, instance, name_label, kernel, ramdisk,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F%2FQHM%3D","line":214,"in_reply_to":"AAAATn%2F%2FTko%3D","updated":"2013-11-12 08:50:29.000000000","message":"Done","commit_id":"2582cea74af438ec25260608a1fe499c35f00a18"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"unresolved":false,"context_lines":[{"line_number":292,"context_line":"        rec[\u0027HVM_boot_params\u0027] \u003d {\u0027order\u0027: \u0027dc\u0027}"},{"line_number":293,"context_line":"        rec[\u0027HVM_boot_policy\u0027] \u003d \u0027BIOS order\u0027"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"    set_device_id(session, image_meta, rec)"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    vm_ref \u003d session.call_xenapi(\u0027VM.create\u0027, rec)"},{"line_number":298,"context_line":"    LOG.debug(_(\u0027Created VM\u0027), instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F%2FTlw%3D","line":295,"updated":"2013-11-11 10:23:09.000000000","message":"I would rather see get_device_id here, and really, I would rather see it passed into create_vm.","commit_id":"2582cea74af438ec25260608a1fe499c35f00a18"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"unresolved":false,"context_lines":[{"line_number":292,"context_line":"        rec[\u0027HVM_boot_params\u0027] \u003d {\u0027order\u0027: \u0027dc\u0027}"},{"line_number":293,"context_line":"        rec[\u0027HVM_boot_policy\u0027] \u003d \u0027BIOS order\u0027"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"    set_device_id(session, image_meta, rec)"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    vm_ref \u003d session.call_xenapi(\u0027VM.create\u0027, rec)"},{"line_number":298,"context_line":"    LOG.debug(_(\u0027Created VM\u0027), instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F%2FQHA%3D","line":295,"in_reply_to":"AAAATn%2F%2FTlw%3D","updated":"2013-11-12 08:50:29.000000000","message":"Done","commit_id":"2582cea74af438ec25260608a1fe499c35f00a18"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"unresolved":false,"context_lines":[{"line_number":192,"context_line":"        }.get(image_type_id)"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"def get_device_id(session, image_meta):"},{"line_number":196,"context_line":"    \"\"\"device_id should be 2 for windows VMs which run new xentools (\u003e\u003d6.1)"},{"line_number":197,"context_line":"    refer to http://support.citrix.com/article/CTX135099 for more info"},{"line_number":198,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAATn%2F%2FOHg%3D","line":195,"updated":"2013-11-12 17:40:17.000000000","message":"can we call it get_vm_device_id or something like that?\n\nI just worry about thinking its a disk or vif device id.","commit_id":"1ed51866287bfe6ebfcd93d2b78face5bd9d7669"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"unresolved":false,"context_lines":[{"line_number":192,"context_line":"        }.get(image_type_id)"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"def get_device_id(session, image_meta):"},{"line_number":196,"context_line":"    \"\"\"device_id should be 2 for windows VMs which run new xentools (\u003e\u003d6.1)"},{"line_number":197,"context_line":"    refer to http://support.citrix.com/article/CTX135099 for more info"},{"line_number":198,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAATn%2F%2FMfM%3D","line":195,"in_reply_to":"AAAATn%2F%2FOHg%3D","updated":"2013-11-13 08:24:38.000000000","message":"Done","commit_id":"1ed51866287bfe6ebfcd93d2b78face5bd9d7669"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"unresolved":false,"context_lines":[{"line_number":212,"context_line":"            raise exception.NovaException(msg)"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"def hypervisor_supports_device_id(version):"},{"line_number":216,"context_line":"    hypervisor_major_minor_version \u003d utils.get_major_minor_version(version)"},{"line_number":217,"context_line":"    return(hypervisor_major_minor_version \u003e\u003d 6.1)"},{"line_number":218,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAATn%2F%2FOXE%3D","line":215,"updated":"2013-11-12 17:40:17.000000000","message":"This should probably start with an underscore, as its private.","commit_id":"1ed51866287bfe6ebfcd93d2b78face5bd9d7669"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"unresolved":false,"context_lines":[{"line_number":212,"context_line":"            raise exception.NovaException(msg)"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":""},{"line_number":215,"context_line":"def hypervisor_supports_device_id(version):"},{"line_number":216,"context_line":"    hypervisor_major_minor_version \u003d utils.get_major_minor_version(version)"},{"line_number":217,"context_line":"    return(hypervisor_major_minor_version \u003e\u003d 6.1)"},{"line_number":218,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAATn%2F%2FMfA%3D","line":215,"in_reply_to":"AAAATn%2F%2FOXE%3D","updated":"2013-11-13 08:24:38.000000000","message":"Done","commit_id":"1ed51866287bfe6ebfcd93d2b78face5bd9d7669"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"5690166f00e3ccd82a9758550bbed1a967b515a8","unresolved":false,"context_lines":[{"line_number":219,"context_line":"def get_vm_device_id(session, image_meta):"},{"line_number":220,"context_line":"    \"\"\"device_id should be 2 for windows VMs which run new xentools (\u003e\u003d6.1)"},{"line_number":221,"context_line":"    refer to http://support.citrix.com/article/CTX135099 for more info"},{"line_number":222,"context_line":"    \"\"\""},{"line_number":223,"context_line":"    if image_meta is None:"},{"line_number":224,"context_line":"        image_meta \u003d {}"},{"line_number":225,"context_line":"    device_id \u003d image_meta.get(\u0027xenapi_device_id\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"AAAATn%2F%2FLd4%3D","line":222,"updated":"2013-11-13 10:57:10.000000000","message":"This should be a NOTE() instead of a doc string I think. This isn\u0027t describing how the method works.","commit_id":"09a63bd4fb4e3eeb7a3e8d8f891fb49a07aaa2d4"}],"nova/virt/xenapi/vmops.py":[{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"e58615002b3a8918a5f386d1c5be3fdaf5af87e6","unresolved":false,"context_lines":[{"line_number":536,"context_line":"        use_pv_kernel \u003d (mode \u003d\u003d vm_mode.XEN)"},{"line_number":537,"context_line":"        LOG.debug(_(\"Using PV kernel: %s\") % use_pv_kernel, instance\u003dinstance)"},{"line_number":538,"context_line":"        vm_ref \u003d vm_utils.create_vm(self._session, instance, name_label,"},{"line_number":539,"context_line":"                                    kernel_file, ramdisk_file,"},{"line_number":540,"context_line":"                                    use_pv_kernel,"},{"line_number":541,"context_line":"                                    image_meta)"},{"line_number":542,"context_line":"        return vm_ref"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2Fffc%3D","line":539,"updated":"2013-11-04 16:49:46.000000000","message":"Nit picky, but you can collapse this to fewer lines. :)","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"change_message_id":"d22640e2d96fd5d58e28558e6c5d31e6e481b0da","unresolved":false,"context_lines":[{"line_number":536,"context_line":"        use_pv_kernel \u003d (mode \u003d\u003d vm_mode.XEN)"},{"line_number":537,"context_line":"        LOG.debug(_(\"Using PV kernel: %s\") % use_pv_kernel, instance\u003dinstance)"},{"line_number":538,"context_line":"        vm_ref \u003d vm_utils.create_vm(self._session, instance, name_label,"},{"line_number":539,"context_line":"                                    kernel_file, ramdisk_file,"},{"line_number":540,"context_line":"                                    use_pv_kernel,"},{"line_number":541,"context_line":"                                    image_meta)"},{"line_number":542,"context_line":"        return vm_ref"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAATn%2F%2FesA%3D","line":539,"in_reply_to":"AAAATn%2F%2Fffc%3D","updated":"2013-11-05 09:19:32.000000000","message":"Done","commit_id":"794f5e4c4a273942fb0eb96f5f4486d24690d23f"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"unresolved":false,"context_lines":[{"line_number":533,"context_line":"            self._virtapi.instance_update(context,"},{"line_number":534,"context_line":"                                          instance[\u0027uuid\u0027], {\u0027vm_mode\u0027: mode})"},{"line_number":535,"context_line":""},{"line_number":536,"context_line":"        use_pv_kernel \u003d (mode \u003d\u003d vm_mode.XEN)"},{"line_number":537,"context_line":"        LOG.debug(_(\"Using PV kernel: %s\") % use_pv_kernel, instance\u003dinstance)"},{"line_number":538,"context_line":"        vm_ref \u003d vm_utils.create_vm(self._session, instance, name_label,"},{"line_number":539,"context_line":"                                    kernel_file, ramdisk_file,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F%2FTlk%3D","line":536,"updated":"2013-11-11 10:23:09.000000000","message":"I would like to see the device_id extracted here, then passed into create_vm, the image_meta is the wrong abstraction level compared to the other arguments.","commit_id":"2582cea74af438ec25260608a1fe499c35f00a18"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"unresolved":false,"context_lines":[{"line_number":533,"context_line":"            self._virtapi.instance_update(context,"},{"line_number":534,"context_line":"                                          instance[\u0027uuid\u0027], {\u0027vm_mode\u0027: mode})"},{"line_number":535,"context_line":""},{"line_number":536,"context_line":"        use_pv_kernel \u003d (mode \u003d\u003d vm_mode.XEN)"},{"line_number":537,"context_line":"        LOG.debug(_(\"Using PV kernel: %s\") % use_pv_kernel, instance\u003dinstance)"},{"line_number":538,"context_line":"        vm_ref \u003d vm_utils.create_vm(self._session, instance, name_label,"},{"line_number":539,"context_line":"                                    kernel_file, ramdisk_file,"}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAATn%2F%2FQI8%3D","line":536,"in_reply_to":"AAAATn%2F%2FTlk%3D","updated":"2013-11-12 08:50:29.000000000","message":"Done","commit_id":"2582cea74af438ec25260608a1fe499c35f00a18"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"unresolved":false,"context_lines":[{"line_number":533,"context_line":"            self._virtapi.instance_update(context,"},{"line_number":534,"context_line":"                                          instance[\u0027uuid\u0027], {\u0027vm_mode\u0027: mode})"},{"line_number":535,"context_line":""},{"line_number":536,"context_line":"        device_id \u003d vm_utils.get_device_id(self._session, image_meta)"},{"line_number":537,"context_line":"        use_pv_kernel \u003d (mode \u003d\u003d vm_mode.XEN)"},{"line_number":538,"context_line":"        LOG.debug(_(\"Using PV kernel: %s\") % use_pv_kernel, instance\u003dinstance)"},{"line_number":539,"context_line":"        vm_ref \u003d vm_utils.create_vm(self._session, instance, name_label,"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAATn%2F%2FOHM%3D","line":536,"updated":"2013-11-12 17:40:17.000000000","message":"I don\u0027t see any tests checking this call, it would be nice to add a test for _create_vm_record to check we pass the device_id correctly, etc.","commit_id":"1ed51866287bfe6ebfcd93d2b78face5bd9d7669"},{"author":{"_account_id":7532,"name":"Aditi Raveesh","email":"aditi.raveesh@gmail.com","username":"aditiraveesh"},"unresolved":false,"context_lines":[{"line_number":533,"context_line":"            self._virtapi.instance_update(context,"},{"line_number":534,"context_line":"                                          instance[\u0027uuid\u0027], {\u0027vm_mode\u0027: mode})"},{"line_number":535,"context_line":""},{"line_number":536,"context_line":"        device_id \u003d vm_utils.get_device_id(self._session, image_meta)"},{"line_number":537,"context_line":"        use_pv_kernel \u003d (mode \u003d\u003d vm_mode.XEN)"},{"line_number":538,"context_line":"        LOG.debug(_(\"Using PV kernel: %s\") % use_pv_kernel, instance\u003dinstance)"},{"line_number":539,"context_line":"        vm_ref \u003d vm_utils.create_vm(self._session, instance, name_label,"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAATn%2F%2FL1k%3D","line":536,"in_reply_to":"AAAATn%2F%2FOHM%3D","updated":"2013-11-13 08:24:38.000000000","message":"Done","commit_id":"1ed51866287bfe6ebfcd93d2b78face5bd9d7669"}]}