)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"602a41809b5b77421755da1a87733ac0e88c0eca","unresolved":false,"context_lines":[{"line_number":10,"context_line":"the presence of PV drivers in the guest image back to dom0 though"},{"line_number":11,"context_line":"xenstore."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"It is possible for users to disable the xen agent that reports this"},{"line_number":14,"context_line":"info and therefore causing failures in live-migration. In cases where"},{"line_number":15,"context_line":"PV drivers are running it is safe to fake the presence of this information"},{"line_number":16,"context_line":"in xenstore. This plugin checks for such cases and writes a fake PV tools"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3afb71cf_80a3dcff","line":13,"updated":"2015-06-09 14:38:54.000000000","message":"@John/Bob - Dumb question, but how much of the driver actually requires this agent?  In other words, is it worth working around the need for an agent in this one particular case, or should the lack of an agent just cause fail fast behavior here?","commit_id":"5ceed906b36e9437e0cbdc1337400e5cab6a213f"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"0cb811b3a26df8c9ea8a841d39e2438b18cacb84","unresolved":false,"context_lines":[{"line_number":10,"context_line":"the presence of PV drivers in the guest image back to dom0 though"},{"line_number":11,"context_line":"xenstore."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"It is possible for users to disable the xen agent that reports this"},{"line_number":14,"context_line":"info and therefore causing failures in live-migration. In cases where"},{"line_number":15,"context_line":"PV drivers are running it is safe to fake the presence of this information"},{"line_number":16,"context_line":"in xenstore. This plugin checks for such cases and writes a fake PV tools"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3afb71cf_ad0a5077","line":13,"in_reply_to":"3afb71cf_80a3dcff","updated":"2015-06-09 15:17:07.000000000","message":"Answered in the comment. I will update the commit to make sure it gives more context as to why we should do this. thanks !","commit_id":"5ceed906b36e9437e0cbdc1337400e5cab6a213f"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"602a41809b5b77421755da1a87733ac0e88c0eca","unresolved":false,"context_lines":[{"line_number":13,"context_line":"It is possible for users to disable the xen agent that reports this"},{"line_number":14,"context_line":"info and therefore causing failures in live-migration. In cases where"},{"line_number":15,"context_line":"PV drivers are running it is safe to fake the presence of this information"},{"line_number":16,"context_line":"in xenstore. This plugin checks for such cases and writes a fake PV tools"},{"line_number":17,"context_line":"information prior to live-migration."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: Ib9624a6f325516b0124ebdecebb0951f29ba3ac4"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3afb71cf_a03ed889","line":16,"updated":"2015-06-09 14:38:54.000000000","message":"Where is the code that actually consumes this information from xenstore?  It seems a touch awkward to save a \"fake\" (not actually fake) version of this info there, when we could pass it explicitly to whatever code requires it.","commit_id":"5ceed906b36e9437e0cbdc1337400e5cab6a213f"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"0cb811b3a26df8c9ea8a841d39e2438b18cacb84","unresolved":false,"context_lines":[{"line_number":13,"context_line":"It is possible for users to disable the xen agent that reports this"},{"line_number":14,"context_line":"info and therefore causing failures in live-migration. In cases where"},{"line_number":15,"context_line":"PV drivers are running it is safe to fake the presence of this information"},{"line_number":16,"context_line":"in xenstore. This plugin checks for such cases and writes a fake PV tools"},{"line_number":17,"context_line":"information prior to live-migration."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Change-Id: Ib9624a6f325516b0124ebdecebb0951f29ba3ac4"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"3afb71cf_ed060885","line":16,"in_reply_to":"3afb71cf_a03ed889","updated":"2015-06-09 15:17:07.000000000","message":"i think John answered this also, but the version info is consumed by xapi outside of nova code, and when not present, even though the drivers are present things like live-migration fails. We are \"faking\" it in the sense that its  pushed from outside the vm.","commit_id":"5ceed906b36e9437e0cbdc1337400e5cab6a213f"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"cd6357fc5445c2cd058d3d8ff76fe561efdfe0c7","unresolved":false,"context_lines":[{"line_number":13,"context_line":"It is possible for users to disable the xen agent that reports this"},{"line_number":14,"context_line":"info and therefore causing failures in live-migration. In cases where"},{"line_number":15,"context_line":"PV drivers are running it is safe to fake the presence of this information"},{"line_number":16,"context_line":"in xenstore. This plugin checks for such cases and writes a fake PV tools"},{"line_number":17,"context_line":"information prior to live-migration."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"The information about the presence of pv driver or version is read"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"3a50d1a3_53bc8d05","line":16,"updated":"2015-07-22 20:53:11.000000000","message":"Does this mean that if the xen agent is not running then it\u0027s fine to do the live-migration despite xens objections?  If so why doesn\u0027t xen just proceed with the migration?  If Nova can determine that it\u0027s safe why doesn\u0027t xen think it\u0027s safe?","commit_id":"f4fea2f4abdabc2d0c1fefe2142ed5c2854e6158"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f9255cdf7ec04473e389ee81c864e76d46f0085e","unresolved":false,"context_lines":[{"line_number":13,"context_line":"It is possible for users to disable the xen agent that reports this"},{"line_number":14,"context_line":"info and therefore causing failures in live-migration. In cases where"},{"line_number":15,"context_line":"PV drivers are running it is safe to fake the presence of this information"},{"line_number":16,"context_line":"in xenstore. This plugin checks for such cases and writes a fake PV tools"},{"line_number":17,"context_line":"information prior to live-migration."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"The information about the presence of pv driver or version is read"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"1a4dcd0f_832759b5","line":16,"in_reply_to":"3a50d1a3_0ebede64","updated":"2015-08-11 16:31:42.000000000","message":"I might be good to add a little more context around that in the git commit message.","commit_id":"f4fea2f4abdabc2d0c1fefe2142ed5c2854e6158"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"e4a9f8ed8cf77eac149234e5be0acfa6edc65db8","unresolved":false,"context_lines":[{"line_number":13,"context_line":"It is possible for users to disable the xen agent that reports this"},{"line_number":14,"context_line":"info and therefore causing failures in live-migration. In cases where"},{"line_number":15,"context_line":"PV drivers are running it is safe to fake the presence of this information"},{"line_number":16,"context_line":"in xenstore. This plugin checks for such cases and writes a fake PV tools"},{"line_number":17,"context_line":"information prior to live-migration."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"The information about the presence of pv driver or version is read"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"3a50d1a3_0ebede64","line":16,"in_reply_to":"3a50d1a3_53bc8d05","updated":"2015-07-22 21:30:09.000000000","message":"The XenServer guest agent is only responsible for reporting guest information through xenstore to dom0.\n\nThe PV drivers (whether installed through the Windows tools or included in the Linux kernel) are responsible for all activities relating to live migration.\n\nThe historical check in XenServer for actions such as live migrate was to check the guest agent reported as up to date.  This was recently (XS 6.5?) relaxed to only check that the guest agent existed, and in future versions of XenServer we expect to only check for the PV tools.\n\nWe\u0027re happy that VMs with the tools installed but no guest agent can safely be live migrated","commit_id":"f4fea2f4abdabc2d0c1fefe2142ed5c2854e6158"}],"nova/tests/unit/virt/xenapi/test_vmops.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"cfd0dd43949f7a9701469e743ab753f0f6d86a43","unresolved":false,"context_lines":[{"line_number":1117,"context_line":"        mock_write_fake_pv_version,"},{"line_number":1118,"context_line":"        mock_pv_driver_version_reported,"},{"line_number":1119,"context_line":"        mock_pv_device_reported):"},{"line_number":1120,"context_line":"        "},{"line_number":1121,"context_line":"        mock_pv_device_reported.return_value \u003d True"},{"line_number":1122,"context_line":"        mock_pv_driver_version_reported.return_value \u003d False"},{"line_number":1123,"context_line":"        fake_instance \u003d {\"name\": \"fake_instance\"}"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_0dcf642a","line":1120,"updated":"2015-06-26 09:26:27.000000000","message":"nit","commit_id":"fac6b08f843e2910a4749f39105310cb5b50bd1e"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"b262601eb72b91ffbac9e30a5bac67d827edc0b6","unresolved":false,"context_lines":[{"line_number":1117,"context_line":"        mock_write_fake_pv_version,"},{"line_number":1118,"context_line":"        mock_pv_driver_version_reported,"},{"line_number":1119,"context_line":"        mock_pv_device_reported):"},{"line_number":1120,"context_line":"        "},{"line_number":1121,"context_line":"        mock_pv_device_reported.return_value \u003d True"},{"line_number":1122,"context_line":"        mock_pv_driver_version_reported.return_value \u003d False"},{"line_number":1123,"context_line":"        fake_instance \u003d {\"name\": \"fake_instance\"}"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_10e53bdb","line":1120,"in_reply_to":"fa32b979_0dcf642a","updated":"2015-06-26 09:38:19.000000000","message":"Will fix.","commit_id":"fac6b08f843e2910a4749f39105310cb5b50bd1e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"cfd0dd43949f7a9701469e743ab753f0f6d86a43","unresolved":false,"context_lines":[{"line_number":1123,"context_line":"        fake_instance \u003d {\"name\": \"fake_instance\"}"},{"line_number":1124,"context_line":"        self.vmops.ensure_pv_driver_info_for_live_migration(fake_instance, \"vm_rec\")"},{"line_number":1125,"context_line":""},{"line_number":1126,"context_line":"        mock_write_fake_pv_version.assert_called_once_with(fake_instance, \"vm_rec\")"},{"line_number":1127,"context_line":""},{"line_number":1128,"context_line":"    @mock.patch.object(vmops.VMOps, \u0027_read_from_xenstore\u0027)"},{"line_number":1129,"context_line":"    def test_pv_driver_version_reported_None(self, fake_read_from_xenstore):"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_0d6be4f2","line":1126,"updated":"2015-06-26 09:26:27.000000000","message":"what about testing when this is not called?","commit_id":"fac6b08f843e2910a4749f39105310cb5b50bd1e"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"b262601eb72b91ffbac9e30a5bac67d827edc0b6","unresolved":false,"context_lines":[{"line_number":1123,"context_line":"        fake_instance \u003d {\"name\": \"fake_instance\"}"},{"line_number":1124,"context_line":"        self.vmops.ensure_pv_driver_info_for_live_migration(fake_instance, \"vm_rec\")"},{"line_number":1125,"context_line":""},{"line_number":1126,"context_line":"        mock_write_fake_pv_version.assert_called_once_with(fake_instance, \"vm_rec\")"},{"line_number":1127,"context_line":""},{"line_number":1128,"context_line":"    @mock.patch.object(vmops.VMOps, \u0027_read_from_xenstore\u0027)"},{"line_number":1129,"context_line":"    def test_pv_driver_version_reported_None(self, fake_read_from_xenstore):"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_f0c5373d","line":1126,"in_reply_to":"fa32b979_0d6be4f2","updated":"2015-06-26 09:38:19.000000000","message":"Will fix.","commit_id":"fac6b08f843e2910a4749f39105310cb5b50bd1e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f9255cdf7ec04473e389ee81c864e76d46f0085e","unresolved":false,"context_lines":[{"line_number":1120,"context_line":"        path \u003d \"attr/PVAddons/MajorVersion\""},{"line_number":1121,"context_line":"        self.assertEqual(\"fake_xapi_return\","},{"line_number":1122,"context_line":"                         self.vmops._read_from_xenstore(fake_instance, path,"},{"line_number":1123,"context_line":"                                                        vm_ref\u003d\"vm_ref\"))"},{"line_number":1124,"context_line":""},{"line_number":1125,"context_line":"    @mock.patch.object(vmops.VMOps, \u0027_make_plugin_call\u0027)"},{"line_number":1126,"context_line":"    def test_read_from_xenstore_ignore_missing_path(self, fake_xapi_call):"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_c348015f","line":1123,"updated":"2015-08-11 16:31:42.000000000","message":"Nit: I think its good to add fake_xapi_call.assert_called_once_with in here, to save the duplication","commit_id":"f4fea2f4abdabc2d0c1fefe2142ed5c2854e6158"}],"nova/virt/xenapi/vmops.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"cfd0dd43949f7a9701469e743ab753f0f6d86a43","unresolved":false,"context_lines":[{"line_number":1956,"context_line":"                                      vm_ref\u003dvm_ref, path\u003dpath,"},{"line_number":1957,"context_line":"                                      value\u003djsonutils.dumps(value))"},{"line_number":1958,"context_line":""},{"line_number":1959,"context_line":"    def _read_from_xenstore(self, instance, path, ignore_missing_path\u003dTrue,"},{"line_number":1960,"context_line":"                            vm_ref\u003dNone):"},{"line_number":1961,"context_line":"        \"\"\"Reads the passed location from xenstore for the given vm."},{"line_number":1962,"context_line":"        Missing paths are ignored, unless excplicitely stated not to"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_2d858806","line":1959,"updated":"2015-06-26 09:26:27.000000000","message":"I though we had this already, clearly not.","commit_id":"fac6b08f843e2910a4749f39105310cb5b50bd1e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"cfd0dd43949f7a9701469e743ab753f0f6d86a43","unresolved":false,"context_lines":[{"line_number":1970,"context_line":"        else:"},{"line_number":1971,"context_line":"            ignore_missing \u003d \u0027False\u0027"},{"line_number":1972,"context_line":""},{"line_number":1973,"context_line":"        return self._make_plugin_call(\u0027xenstore.py\u0027, \u0027read_record\u0027, instance,"},{"line_number":1974,"context_line":"                                      vm_ref\u003dvm_ref, path\u003dpath,"},{"line_number":1975,"context_line":"                                      ignore_missing_path\u003dignore_missing)"},{"line_number":1976,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_adf75858","line":1973,"updated":"2015-06-26 09:26:27.000000000","message":"I don\u0027t see a unit test for this?","commit_id":"fac6b08f843e2910a4749f39105310cb5b50bd1e"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"b262601eb72b91ffbac9e30a5bac67d827edc0b6","unresolved":false,"context_lines":[{"line_number":1970,"context_line":"        else:"},{"line_number":1971,"context_line":"            ignore_missing \u003d \u0027False\u0027"},{"line_number":1972,"context_line":""},{"line_number":1973,"context_line":"        return self._make_plugin_call(\u0027xenstore.py\u0027, \u0027read_record\u0027, instance,"},{"line_number":1974,"context_line":"                                      vm_ref\u003dvm_ref, path\u003dpath,"},{"line_number":1975,"context_line":"                                      ignore_missing_path\u003dignore_missing)"},{"line_number":1976,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_d094b38d","line":1973,"in_reply_to":"fa32b979_adf75858","updated":"2015-06-26 09:38:19.000000000","message":"Will add one. thx.","commit_id":"fac6b08f843e2910a4749f39105310cb5b50bd1e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"cfd0dd43949f7a9701469e743ab753f0f6d86a43","unresolved":false,"context_lines":[{"line_number":2192,"context_line":"                raise exception.MigrationPreCheckError(reason\u003dmsg)"},{"line_number":2193,"context_line":"        return dest_check_data"},{"line_number":2194,"context_line":""},{"line_number":2195,"context_line":"    def ensure_pv_driver_info_for_live_migration(self, instance, vm_ref):"},{"line_number":2196,"context_line":"        \"\"\"Checks if pv drivers are present for this instance. If it is"},{"line_number":2197,"context_line":"        present but not reported, try to fake the info for live-migration\"\"\""},{"line_number":2198,"context_line":"        if self._pv_driver_version_reported(instance, vm_ref):"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_ed9ec053","line":2195,"updated":"2015-06-26 09:26:27.000000000","message":"Its tempting to move this into VM utils, but I get why we can\u0027t here.","commit_id":"fac6b08f843e2910a4749f39105310cb5b50bd1e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"10107869fa61188c7375a438aab0e963c80a254d","unresolved":false,"context_lines":[{"line_number":2200,"context_line":"            return"},{"line_number":2201,"context_line":"        else:"},{"line_number":2202,"context_line":"            if self._pv_device_reported(instance, vm_ref):"},{"line_number":2203,"context_line":"                # PV device is there, so we can fake the version info"},{"line_number":2204,"context_line":"                msg \u003d (\"PV device present for %s but missing pv driver info. \""},{"line_number":2205,"context_line":"                       \"Attempting to insert missing info in xenstore.\""},{"line_number":2206,"context_line":"                       % (instance[\u0027name\u0027]))"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_ad53d838","line":2203,"updated":"2015-06-26 09:30:28.000000000","message":"not sure you need this comment","commit_id":"fac6b08f843e2910a4749f39105310cb5b50bd1e"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"b262601eb72b91ffbac9e30a5bac67d827edc0b6","unresolved":false,"context_lines":[{"line_number":2200,"context_line":"            return"},{"line_number":2201,"context_line":"        else:"},{"line_number":2202,"context_line":"            if self._pv_device_reported(instance, vm_ref):"},{"line_number":2203,"context_line":"                # PV device is there, so we can fake the version info"},{"line_number":2204,"context_line":"                msg \u003d (\"PV device present for %s but missing pv driver info. \""},{"line_number":2205,"context_line":"                       \"Attempting to insert missing info in xenstore.\""},{"line_number":2206,"context_line":"                       % (instance[\u0027name\u0027]))"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_f0d8d781","line":2203,"in_reply_to":"fa32b979_ad53d838","updated":"2015-06-26 09:38:19.000000000","message":"Will remove.","commit_id":"fac6b08f843e2910a4749f39105310cb5b50bd1e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"10107869fa61188c7375a438aab0e963c80a254d","unresolved":false,"context_lines":[{"line_number":2217,"context_line":"        major_version \u003d self._read_from_xenstore(instance, xs_path, vm_ref\u003dvm_ref)"},{"line_number":2218,"context_line":"        LOG.debug(\"Major Version: %s reported for %s\" % (major_version, instance[\u0027name\u0027]))"},{"line_number":2219,"context_line":"        # xenstore reports by string"},{"line_number":2220,"context_line":"        if major_version \u003d\u003d \u0027\"None\"\u0027:"},{"line_number":2221,"context_line":"            return False"},{"line_number":2222,"context_line":"        else:"},{"line_number":2223,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_5072e3b1","line":2220,"updated":"2015-06-26 09:30:28.000000000","message":"Somehow this doesn\u0027t fee safe. What if the path is missing, do we get None?","commit_id":"fac6b08f843e2910a4749f39105310cb5b50bd1e"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"b262601eb72b91ffbac9e30a5bac67d827edc0b6","unresolved":false,"context_lines":[{"line_number":2217,"context_line":"        major_version \u003d self._read_from_xenstore(instance, xs_path, vm_ref\u003dvm_ref)"},{"line_number":2218,"context_line":"        LOG.debug(\"Major Version: %s reported for %s\" % (major_version, instance[\u0027name\u0027]))"},{"line_number":2219,"context_line":"        # xenstore reports by string"},{"line_number":2220,"context_line":"        if major_version \u003d\u003d \u0027\"None\"\u0027:"},{"line_number":2221,"context_line":"            return False"},{"line_number":2222,"context_line":"        else:"},{"line_number":2223,"context_line":"            return True"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_703a879a","line":2220,"in_reply_to":"fa32b979_5072e3b1","updated":"2015-06-26 09:38:19.000000000","message":"Yes, if there is no version, the path does not exists, and we ignore missing path in xenstore.py which return \u0027\"None\"\u0027","commit_id":"fac6b08f843e2910a4749f39105310cb5b50bd1e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"10107869fa61188c7375a438aab0e963c80a254d","unresolved":false,"context_lines":[{"line_number":2223,"context_line":"            return True"},{"line_number":2224,"context_line":""},{"line_number":2225,"context_line":"    def _pv_device_reported(self, instance, vm_ref):"},{"line_number":2226,"context_line":"        xs_path \u003d \"device/vif/0/state\""},{"line_number":2227,"context_line":"        ret \u003d self._read_from_xenstore(instance, xs_path, vm_ref\u003dvm_ref)"},{"line_number":2228,"context_line":"        LOG.debug(\"PV Device vif.0 reprting state %s\" % ret)"},{"line_number":2229,"context_line":"        # NOTE(sulo): We infer the presence of pv driver"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_50fd0307","line":2226,"updated":"2015-06-26 09:30:28.000000000","message":"so, I have a feeling we should check all vifs? not just the \"first\" vif?","commit_id":"fac6b08f843e2910a4749f39105310cb5b50bd1e"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"b262601eb72b91ffbac9e30a5bac67d827edc0b6","unresolved":false,"context_lines":[{"line_number":2223,"context_line":"            return True"},{"line_number":2224,"context_line":""},{"line_number":2225,"context_line":"    def _pv_device_reported(self, instance, vm_ref):"},{"line_number":2226,"context_line":"        xs_path \u003d \"device/vif/0/state\""},{"line_number":2227,"context_line":"        ret \u003d self._read_from_xenstore(instance, xs_path, vm_ref\u003dvm_ref)"},{"line_number":2228,"context_line":"        LOG.debug(\"PV Device vif.0 reprting state %s\" % ret)"},{"line_number":2229,"context_line":"        # NOTE(sulo): We infer the presence of pv driver"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_508d0329","line":2226,"in_reply_to":"fa32b979_50fd0307","updated":"2015-06-26 09:38:19.000000000","message":"Yeah, that was my first instinct also, but wanted to keep that general, ie. assume that at least 1 vif is present.\n\nI can try to get all the present vifs on the vm and check their status, I think that will work ?","commit_id":"fac6b08f843e2910a4749f39105310cb5b50bd1e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"10107869fa61188c7375a438aab0e963c80a254d","unresolved":false,"context_lines":[{"line_number":2231,"context_line":"        # device status as on of 1\u003dinitialising, 2\u003dinit wait, 3\u003dinitialised,"},{"line_number":2232,"context_line":"        # 4\u003dconnected, 5\u003dclosing, 6\u003dclosed we take that as the presence of"},{"line_number":2233,"context_line":"        # pv driver."},{"line_number":2234,"context_line":"        if int(ret) in (1,2,3,4,5,6):"},{"line_number":2235,"context_line":"            return True"},{"line_number":2236,"context_line":"        return False"},{"line_number":2237,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_f03537db","line":2234,"updated":"2015-06-26 09:30:28.000000000","message":"This can throw an exception, we have some oslo utils to convert ints from a string.","commit_id":"fac6b08f843e2910a4749f39105310cb5b50bd1e"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"b262601eb72b91ffbac9e30a5bac67d827edc0b6","unresolved":false,"context_lines":[{"line_number":2231,"context_line":"        # device status as on of 1\u003dinitialising, 2\u003dinit wait, 3\u003dinitialised,"},{"line_number":2232,"context_line":"        # 4\u003dconnected, 5\u003dclosing, 6\u003dclosed we take that as the presence of"},{"line_number":2233,"context_line":"        # pv driver."},{"line_number":2234,"context_line":"        if int(ret) in (1,2,3,4,5,6):"},{"line_number":2235,"context_line":"            return True"},{"line_number":2236,"context_line":"        return False"},{"line_number":2237,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_50b423d3","line":2234,"in_reply_to":"fa32b979_f03537db","updated":"2015-06-26 09:38:19.000000000","message":"Sorry wasnt aware of this. Will try to find out.","commit_id":"fac6b08f843e2910a4749f39105310cb5b50bd1e"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"cfd0dd43949f7a9701469e743ab753f0f6d86a43","unresolved":false,"context_lines":[{"line_number":2302,"context_line":"                self.ensure_pv_driver_info_for_live_migration(instance, vm_ref)"},{"line_number":2303,"context_line":"            except Exception, e:"},{"line_number":2304,"context_line":"                LOG.warning(str(e))"},{"line_number":2305,"context_line":" "},{"line_number":2306,"context_line":"            if migrate_data is not None:"},{"line_number":2307,"context_line":"                (kernel, ramdisk) \u003d vm_utils.lookup_kernel_ramdisk("},{"line_number":2308,"context_line":"                    self._session, vm_ref)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa32b979_adc3d856","line":2305,"updated":"2015-06-26 09:26:27.000000000","message":"I think this is the pep8 error.","commit_id":"fac6b08f843e2910a4749f39105310cb5b50bd1e"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"7d32b78e51e6092c1f26b447f14e885b2f5b17d8","unresolved":false,"context_lines":[{"line_number":2204,"context_line":"            if self._pv_device_reported(instance, vm_ref):"},{"line_number":2205,"context_line":"                msg \u003d (\"PV device present for %s but missing pv driver info. \""},{"line_number":2206,"context_line":"                       \"Attempting to insert missing info in xenstore.\""},{"line_number":2207,"context_line":"                       % (instance[\u0027name\u0027]))"},{"line_number":2208,"context_line":"                LOG.debug(msg)"},{"line_number":2209,"context_line":"                self._write_fake_pv_version(instance, vm_ref)"},{"line_number":2210,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa32b979_177e636a","line":2207,"updated":"2015-06-27 17:05:54.000000000","message":"please note that the instance name is not unique. Can you please use instance.uuid\n\nIn addition to this I suggest that you use the following:\nLOG.debug(\"...\", instance\u003dinstance)","commit_id":"271baa37b44310c15698c4e74b97522c0156a26c"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"7a7cfaeb496d8ec19425db64e87d9711fe9aa275","unresolved":false,"context_lines":[{"line_number":2204,"context_line":"            if self._pv_device_reported(instance, vm_ref):"},{"line_number":2205,"context_line":"                msg \u003d (\"PV device present for %s but missing pv driver info. \""},{"line_number":2206,"context_line":"                       \"Attempting to insert missing info in xenstore.\""},{"line_number":2207,"context_line":"                       % (instance[\u0027name\u0027]))"},{"line_number":2208,"context_line":"                LOG.debug(msg)"},{"line_number":2209,"context_line":"                self._write_fake_pv_version(instance, vm_ref)"},{"line_number":2210,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa32b979_1ab68af9","line":2207,"in_reply_to":"fa32b979_177e636a","updated":"2015-06-27 18:57:09.000000000","message":"thx, ill use the LOG.debug(\"...\", instance\u003dinstance) format.","commit_id":"271baa37b44310c15698c4e74b97522c0156a26c"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"7d32b78e51e6092c1f26b447f14e885b2f5b17d8","unresolved":false,"context_lines":[{"line_number":2205,"context_line":"                msg \u003d (\"PV device present for %s but missing pv driver info. \""},{"line_number":2206,"context_line":"                       \"Attempting to insert missing info in xenstore.\""},{"line_number":2207,"context_line":"                       % (instance[\u0027name\u0027]))"},{"line_number":2208,"context_line":"                LOG.debug(msg)"},{"line_number":2209,"context_line":"                self._write_fake_pv_version(instance, vm_ref)"},{"line_number":2210,"context_line":"            else:"},{"line_number":2211,"context_line":"                msg \u003d (\"Could not determine the presence of pv device for \""}],"source_content_type":"text/x-python","patch_set":4,"id":"fa32b979_f77adf76","line":2208,"updated":"2015-06-27 17:05:54.000000000","message":"why do we need the msg variable.\nYou can do this directly with LOG.debug(\"PV ...\")","commit_id":"271baa37b44310c15698c4e74b97522c0156a26c"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"7a7cfaeb496d8ec19425db64e87d9711fe9aa275","unresolved":false,"context_lines":[{"line_number":2205,"context_line":"                msg \u003d (\"PV device present for %s but missing pv driver info. \""},{"line_number":2206,"context_line":"                       \"Attempting to insert missing info in xenstore.\""},{"line_number":2207,"context_line":"                       % (instance[\u0027name\u0027]))"},{"line_number":2208,"context_line":"                LOG.debug(msg)"},{"line_number":2209,"context_line":"                self._write_fake_pv_version(instance, vm_ref)"},{"line_number":2210,"context_line":"            else:"},{"line_number":2211,"context_line":"                msg \u003d (\"Could not determine the presence of pv device for \""}],"source_content_type":"text/x-python","patch_set":4,"id":"fa32b979_3a8eee27","line":2208,"in_reply_to":"fa32b979_f77adf76","updated":"2015-06-27 18:57:09.000000000","message":"I do that mostly to get the longest possible line length really. I like doing that over doing LOG.exception(\"xx...\") and have to columnize the log.\n\nI guess you are suggesting we don\u0027t gain anything out of that anyway ... i change this for all, thanks for the suggestions.","commit_id":"271baa37b44310c15698c4e74b97522c0156a26c"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"7d32b78e51e6092c1f26b447f14e885b2f5b17d8","unresolved":false,"context_lines":[{"line_number":2208,"context_line":"                LOG.debug(msg)"},{"line_number":2209,"context_line":"                self._write_fake_pv_version(instance, vm_ref)"},{"line_number":2210,"context_line":"            else:"},{"line_number":2211,"context_line":"                msg \u003d (\"Could not determine the presence of pv device for \""},{"line_number":2212,"context_line":"                       \"%s. Skipping inserting pv driver info\""},{"line_number":2213,"context_line":"                       % (instance[\u0027name\u0027]))"},{"line_number":2214,"context_line":"                LOG.debug(msg)"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa32b979_d790fb98","line":2211,"updated":"2015-06-27 17:05:54.000000000","message":"same as above","commit_id":"271baa37b44310c15698c4e74b97522c0156a26c"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"7d32b78e51e6092c1f26b447f14e885b2f5b17d8","unresolved":false,"context_lines":[{"line_number":2218,"context_line":"        major_version \u003d self._read_from_xenstore(instance, xs_path,"},{"line_number":2219,"context_line":"                                                 vm_ref\u003dvm_ref)"},{"line_number":2220,"context_line":"        LOG.debug(\"Major Version: %s reported for %s\" % (major_version,"},{"line_number":2221,"context_line":"                                                         instance[\u0027name\u0027]))"},{"line_number":2222,"context_line":"        # xenstore reports back string only, if the path is missing we get"},{"line_number":2223,"context_line":"        # None as string, since missing paths are ignored."},{"line_number":2224,"context_line":"        if major_version \u003d\u003d \u0027\"None\"\u0027:"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa32b979_778faf75","line":2221,"updated":"2015-06-27 17:05:54.000000000","message":"same comments as above","commit_id":"271baa37b44310c15698c4e74b97522c0156a26c"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"7d32b78e51e6092c1f26b447f14e885b2f5b17d8","unresolved":false,"context_lines":[{"line_number":2239,"context_line":"        for device in net_devices:"},{"line_number":2240,"context_line":"            xs_path \u003d \"device/vif/%s/state\" % device"},{"line_number":2241,"context_line":"            ret \u003d self._read_from_xenstore(instance, xs_path, vm_ref\u003dvm_ref)"},{"line_number":2242,"context_line":"            LOG.debug(\"PV Device vif.%s reprting state %s\" % (device, ret))"},{"line_number":2243,"context_line":"            if strutils.is_int_like(ret) and int(ret) in (1, 2, 3, 4, 5, 6):"},{"line_number":2244,"context_line":"                return True"},{"line_number":2245,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"fa32b979_978a7363","line":2242,"updated":"2015-06-27 17:05:54.000000000","message":"reprting \u003d\u003e reporting","commit_id":"271baa37b44310c15698c4e74b97522c0156a26c"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"7a7cfaeb496d8ec19425db64e87d9711fe9aa275","unresolved":false,"context_lines":[{"line_number":2239,"context_line":"        for device in net_devices:"},{"line_number":2240,"context_line":"            xs_path \u003d \"device/vif/%s/state\" % device"},{"line_number":2241,"context_line":"            ret \u003d self._read_from_xenstore(instance, xs_path, vm_ref\u003dvm_ref)"},{"line_number":2242,"context_line":"            LOG.debug(\"PV Device vif.%s reprting state %s\" % (device, ret))"},{"line_number":2243,"context_line":"            if strutils.is_int_like(ret) and int(ret) in (1, 2, 3, 4, 5, 6):"},{"line_number":2244,"context_line":"                return True"},{"line_number":2245,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"fa32b979_5a917245","line":2242,"in_reply_to":"fa32b979_978a7363","updated":"2015-06-27 18:57:09.000000000","message":"oops .. will change.","commit_id":"271baa37b44310c15698c4e74b97522c0156a26c"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"7d32b78e51e6092c1f26b447f14e885b2f5b17d8","unresolved":false,"context_lines":[{"line_number":2249,"context_line":"        version \u003d self._session.product_version"},{"line_number":2250,"context_line":"        msg \u003d (\"Writing pvtools version major: %s minor: %s micro: %s for \""},{"line_number":2251,"context_line":"               \"instance %s\" % (version[0], version[1], version[2],"},{"line_number":2252,"context_line":"               instance[\u0027name\u0027]))"},{"line_number":2253,"context_line":"        LOG.debug(msg)"},{"line_number":2254,"context_line":"        # Write major version"},{"line_number":2255,"context_line":"        xs_path \u003d \"attr/PVAddons/MajorVersion\""}],"source_content_type":"text/x-python","patch_set":4,"id":"fa32b979_37e84716","line":2252,"updated":"2015-06-27 17:05:54.000000000","message":"same as above","commit_id":"271baa37b44310c15698c4e74b97522c0156a26c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"900f248c416066575fbaf661f762662ceaf7a61b","unresolved":false,"context_lines":[{"line_number":1959,"context_line":"    def _read_from_xenstore(self, instance, path, ignore_missing_path\u003dTrue,"},{"line_number":1960,"context_line":"                            vm_ref\u003dNone):"},{"line_number":1961,"context_line":"        \"\"\"Reads the passed location from xenstore for the given vm."},{"line_number":1962,"context_line":"        Missing paths are ignored, unless excplicitely stated not to"},{"line_number":1963,"context_line":"        which will cause and excpetion to be raised by xenstore."},{"line_number":1964,"context_line":"        A XenAPIPlugin.PluginError will be raised if any error is"},{"line_number":1965,"context_line":"        encountered in the read process."}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_e66d88cd","line":1962,"updated":"2015-06-29 12:22:33.000000000","message":"explicitely","commit_id":"1cb0013967850b878afaf5fd3af09d33a5d99f2c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"900f248c416066575fbaf661f762662ceaf7a61b","unresolved":false,"context_lines":[{"line_number":1960,"context_line":"                            vm_ref\u003dNone):"},{"line_number":1961,"context_line":"        \"\"\"Reads the passed location from xenstore for the given vm."},{"line_number":1962,"context_line":"        Missing paths are ignored, unless excplicitely stated not to"},{"line_number":1963,"context_line":"        which will cause and excpetion to be raised by xenstore."},{"line_number":1964,"context_line":"        A XenAPIPlugin.PluginError will be raised if any error is"},{"line_number":1965,"context_line":"        encountered in the read process."},{"line_number":1966,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_0673ccb2","line":1963,"updated":"2015-06-29 12:22:33.000000000","message":"exception","commit_id":"1cb0013967850b878afaf5fd3af09d33a5d99f2c"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"900f248c416066575fbaf661f762662ceaf7a61b","unresolved":false,"context_lines":[{"line_number":2239,"context_line":"            ret \u003d self._read_from_xenstore(instance, xs_path, vm_ref\u003dvm_ref)"},{"line_number":2240,"context_line":"            LOG.debug(\"PV Device vif.%(vif_ref)s reporting state %(ret)s\","},{"line_number":2241,"context_line":"                      {\u0027vif_ref\u0027: device, \u0027ret\u0027: ret}, instance\u003dinstance)"},{"line_number":2242,"context_line":"            if strutils.is_int_like(ret) and int(ret) in (1, 2, 3, 4, 5, 6):"},{"line_number":2243,"context_line":"                return True"},{"line_number":2244,"context_line":""},{"line_number":2245,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_66e73841","line":2242,"updated":"2015-06-29 12:22:33.000000000","message":"I believe we should not pass this test unless the state is 4.\nIn particular if the state is anything else then it\u0027s likely the live migration will fail, or that we\u0027re hitting a race condition and probably shouldn\u0027t attempt live migration until the PV drivers have been initialised and are connected.","commit_id":"1cb0013967850b878afaf5fd3af09d33a5d99f2c"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"7dd233d4103999db4b68ab2c586ff460dd69b25d","unresolved":false,"context_lines":[{"line_number":2239,"context_line":"            ret \u003d self._read_from_xenstore(instance, xs_path, vm_ref\u003dvm_ref)"},{"line_number":2240,"context_line":"            LOG.debug(\"PV Device vif.%(vif_ref)s reporting state %(ret)s\","},{"line_number":2241,"context_line":"                      {\u0027vif_ref\u0027: device, \u0027ret\u0027: ret}, instance\u003dinstance)"},{"line_number":2242,"context_line":"            if strutils.is_int_like(ret) and int(ret) in (1, 2, 3, 4, 5, 6):"},{"line_number":2243,"context_line":"                return True"},{"line_number":2244,"context_line":""},{"line_number":2245,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_2f46e023","line":2242,"in_reply_to":"ba3cc151_66e73841","updated":"2015-06-29 14:32:51.000000000","message":"Thanks Bob. I\u0027ll get this fixed up such that it only passes for 4.","commit_id":"1cb0013967850b878afaf5fd3af09d33a5d99f2c"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f9255cdf7ec04473e389ee81c864e76d46f0085e","unresolved":false,"context_lines":[{"line_number":2184,"context_line":"            vm_ref \u003d self._get_vm_opaque_ref(instance_ref)"},{"line_number":2185,"context_line":"            migrate_data \u003d dest_check_data[\u0027migrate_data\u0027]"},{"line_number":2186,"context_line":"            try:"},{"line_number":2187,"context_line":"                self._call_live_migrate_command("},{"line_number":2188,"context_line":"                    \"VM.assert_can_migrate\", vm_ref, migrate_data)"},{"line_number":2189,"context_line":"            except self._session.XenAPI.Failure as exc:"},{"line_number":2190,"context_line":"                reason \u003d exc.details[0]"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_48096e83","line":2187,"updated":"2015-08-11 16:31:42.000000000","message":"I think you actually need to call _ensure_pv_driver_info_for_live_migration here?\n\nI suspect this might fail with the PV error first, as its called first? Best to add it here instead I think.","commit_id":"f4fea2f4abdabc2d0c1fefe2142ed5c2854e6158"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"32d7047dccc8d8428c4d4b199f7d43620f2e9a1b","unresolved":false,"context_lines":[{"line_number":2184,"context_line":"            vm_ref \u003d self._get_vm_opaque_ref(instance_ref)"},{"line_number":2185,"context_line":"            migrate_data \u003d dest_check_data[\u0027migrate_data\u0027]"},{"line_number":2186,"context_line":"            try:"},{"line_number":2187,"context_line":"                self._call_live_migrate_command("},{"line_number":2188,"context_line":"                    \"VM.assert_can_migrate\", vm_ref, migrate_data)"},{"line_number":2189,"context_line":"            except self._session.XenAPI.Failure as exc:"},{"line_number":2190,"context_line":"                reason \u003d exc.details[0]"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_b98de27d","line":2187,"in_reply_to":"1a4dcd0f_48096e83","updated":"2015-08-12 08:40:17.000000000","message":"I think assert_can_migrate does not check pv version. its done later on once this test passes, during migrate_send call on the xapi side.\n\nThe source can migrate -\u003e assert_can_migrate is checking other things like vm power state, cpu features, sr, host state etc.\nhttps://github.com/xapi-project/xen-api/blob/master/ocaml/xapi/xapi_vm_migrate.ml#L761\n\nThe error happens later on during migrate_send call. The error on nova side happens during live_migrate call.","commit_id":"f4fea2f4abdabc2d0c1fefe2142ed5c2854e6158"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f9255cdf7ec04473e389ee81c864e76d46f0085e","unresolved":false,"context_lines":[{"line_number":2193,"context_line":"                raise exception.MigrationPreCheckError(reason\u003dmsg)"},{"line_number":2194,"context_line":"        return dest_check_data"},{"line_number":2195,"context_line":""},{"line_number":2196,"context_line":"    def ensure_pv_driver_info_for_live_migration(self, instance, vm_ref):"},{"line_number":2197,"context_line":"        \"\"\"Checks if pv drivers are present for this instance. If it is"},{"line_number":2198,"context_line":"        present but not reported, try to fake the info for live-migration."},{"line_number":2199,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_8805367b","line":2196,"updated":"2015-08-11 16:31:42.000000000","message":"so this is only used here, so I think we need to change it to:\n\n_ensure_pv_driver_info_for_live_migration","commit_id":"f4fea2f4abdabc2d0c1fefe2142ed5c2854e6158"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"32d7047dccc8d8428c4d4b199f7d43620f2e9a1b","unresolved":false,"context_lines":[{"line_number":2193,"context_line":"                raise exception.MigrationPreCheckError(reason\u003dmsg)"},{"line_number":2194,"context_line":"        return dest_check_data"},{"line_number":2195,"context_line":""},{"line_number":2196,"context_line":"    def ensure_pv_driver_info_for_live_migration(self, instance, vm_ref):"},{"line_number":2197,"context_line":"        \"\"\"Checks if pv drivers are present for this instance. If it is"},{"line_number":2198,"context_line":"        present but not reported, try to fake the info for live-migration."},{"line_number":2199,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_19dc1672","line":2196,"in_reply_to":"1a4dcd0f_8805367b","updated":"2015-08-12 08:40:17.000000000","message":"ah ok. yes. will change.","commit_id":"f4fea2f4abdabc2d0c1fefe2142ed5c2854e6158"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f9255cdf7ec04473e389ee81c864e76d46f0085e","unresolved":false,"context_lines":[{"line_number":2200,"context_line":"        if self._pv_driver_version_reported(instance, vm_ref):"},{"line_number":2201,"context_line":"            # Since driver version is reported we dont need to do anything"},{"line_number":2202,"context_line":"            return"},{"line_number":2203,"context_line":"        else:"},{"line_number":2204,"context_line":"            if self._pv_device_reported(instance, vm_ref):"},{"line_number":2205,"context_line":"                LOG.debug(\"PV device present but missing pv driver info. \""},{"line_number":2206,"context_line":"                          \"Attempting to insert missing info in xenstore.\","}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_08604600","line":2203,"updated":"2015-08-11 16:31:42.000000000","message":"Nit, I think you can drop the else here, which makes it easier to read.","commit_id":"f4fea2f4abdabc2d0c1fefe2142ed5c2854e6158"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"32d7047dccc8d8428c4d4b199f7d43620f2e9a1b","unresolved":false,"context_lines":[{"line_number":2200,"context_line":"        if self._pv_driver_version_reported(instance, vm_ref):"},{"line_number":2201,"context_line":"            # Since driver version is reported we dont need to do anything"},{"line_number":2202,"context_line":"            return"},{"line_number":2203,"context_line":"        else:"},{"line_number":2204,"context_line":"            if self._pv_device_reported(instance, vm_ref):"},{"line_number":2205,"context_line":"                LOG.debug(\"PV device present but missing pv driver info. \""},{"line_number":2206,"context_line":"                          \"Attempting to insert missing info in xenstore.\","}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_d9e1ce2b","line":2203,"in_reply_to":"1a4dcd0f_08604600","updated":"2015-08-12 08:40:17.000000000","message":"will change.","commit_id":"f4fea2f4abdabc2d0c1fefe2142ed5c2854e6158"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"cd6357fc5445c2cd058d3d8ff76fe561efdfe0c7","unresolved":false,"context_lines":[{"line_number":2232,"context_line":"        # NOTE(sulo): We infer the presence of pv driver"},{"line_number":2233,"context_line":"        # by the presence of a pv network device. If xenstore reports"},{"line_number":2234,"context_line":"        # device status as connected (status\u003d4) we take that as the presence"},{"line_number":2235,"context_line":"        # of pv driver. Any other status will likely cause migration to fail."},{"line_number":2236,"context_line":"        for device in net_devices:"},{"line_number":2237,"context_line":"            xs_path \u003d \"device/vif/%s/state\" % device"},{"line_number":2238,"context_line":"            ret \u003d self._read_from_xenstore(instance, xs_path, vm_ref\u003dvm_ref)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a50d1a3_b3f871c4","line":2235,"updated":"2015-07-22 20:53:11.000000000","message":"Is this documented in xen docs in a way that explains why it\u0027s safe to check this?  As someone who is not intimately familiar with the implementation of xen it concerns me that this relies on something that\u0027s completely opaque as to why it works.","commit_id":"f4fea2f4abdabc2d0c1fefe2142ed5c2854e6158"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f9255cdf7ec04473e389ee81c864e76d46f0085e","unresolved":false,"context_lines":[{"line_number":2232,"context_line":"        # NOTE(sulo): We infer the presence of pv driver"},{"line_number":2233,"context_line":"        # by the presence of a pv network device. If xenstore reports"},{"line_number":2234,"context_line":"        # device status as connected (status\u003d4) we take that as the presence"},{"line_number":2235,"context_line":"        # of pv driver. Any other status will likely cause migration to fail."},{"line_number":2236,"context_line":"        for device in net_devices:"},{"line_number":2237,"context_line":"            xs_path \u003d \"device/vif/%s/state\" % device"},{"line_number":2238,"context_line":"            ret \u003d self._read_from_xenstore(instance, xs_path, vm_ref\u003dvm_ref)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_a866f24e","line":2235,"in_reply_to":"3a50d1a3_24397ad7","updated":"2015-08-11 16:31:42.000000000","message":"this is very evil, but its \"correct\", and works around limitations of current XenServer versions.","commit_id":"f4fea2f4abdabc2d0c1fefe2142ed5c2854e6158"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"e4a9f8ed8cf77eac149234e5be0acfa6edc65db8","unresolved":false,"context_lines":[{"line_number":2232,"context_line":"        # NOTE(sulo): We infer the presence of pv driver"},{"line_number":2233,"context_line":"        # by the presence of a pv network device. If xenstore reports"},{"line_number":2234,"context_line":"        # device status as connected (status\u003d4) we take that as the presence"},{"line_number":2235,"context_line":"        # of pv driver. Any other status will likely cause migration to fail."},{"line_number":2236,"context_line":"        for device in net_devices:"},{"line_number":2237,"context_line":"            xs_path \u003d \"device/vif/%s/state\" % device"},{"line_number":2238,"context_line":"            ret \u003d self._read_from_xenstore(instance, xs_path, vm_ref\u003dvm_ref)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a50d1a3_24397ad7","line":2235,"in_reply_to":"3a50d1a3_b3f871c4","updated":"2015-07-22 21:30:09.000000000","message":"The status is just the indication that PV drivers exist.  Status\u003d4 means that the negotiation between dom0 and the guest is complete.  As such it\u0027s proof that the PV drivers exist in the domain and are functional.\n\nAs mentioned in the other comment, the minimum requirement for being able to live migrate is to have the PV drivers however XenServer has historically been checking more than the minimum requirement through the guest agent.","commit_id":"f4fea2f4abdabc2d0c1fefe2142ed5c2854e6158"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f9255cdf7ec04473e389ee81c864e76d46f0085e","unresolved":false,"context_lines":[{"line_number":2250,"context_line":"                                       \u0027minor\u0027: version[1],"},{"line_number":2251,"context_line":"                                       \u0027micro\u0027: version[2]},"},{"line_number":2252,"context_line":"                                       instance\u003dinstance)"},{"line_number":2253,"context_line":"        # Write major version"},{"line_number":2254,"context_line":"        xs_path \u003d \"attr/PVAddons/MajorVersion\""},{"line_number":2255,"context_line":"        self._write_to_xenstore(instance, xs_path, version[0], vm_ref\u003dvm_ref)"},{"line_number":2256,"context_line":"        # Write minor version"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_a8f4b25a","line":2253,"updated":"2015-08-11 16:31:42.000000000","message":"Nit: please drop these comments, instead just do:\n\nmajor_version \u003d version[0]\n\nAnd do that above the log message, and I think it will read better.","commit_id":"f4fea2f4abdabc2d0c1fefe2142ed5c2854e6158"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"32d7047dccc8d8428c4d4b199f7d43620f2e9a1b","unresolved":false,"context_lines":[{"line_number":2250,"context_line":"                                       \u0027minor\u0027: version[1],"},{"line_number":2251,"context_line":"                                       \u0027micro\u0027: version[2]},"},{"line_number":2252,"context_line":"                                       instance\u003dinstance)"},{"line_number":2253,"context_line":"        # Write major version"},{"line_number":2254,"context_line":"        xs_path \u003d \"attr/PVAddons/MajorVersion\""},{"line_number":2255,"context_line":"        self._write_to_xenstore(instance, xs_path, version[0], vm_ref\u003dvm_ref)"},{"line_number":2256,"context_line":"        # Write minor version"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_4c8ac265","line":2253,"in_reply_to":"1a4dcd0f_a8f4b25a","updated":"2015-08-12 08:40:17.000000000","message":"will change.","commit_id":"f4fea2f4abdabc2d0c1fefe2142ed5c2854e6158"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"f9255cdf7ec04473e389ee81c864e76d46f0085e","unresolved":false,"context_lines":[{"line_number":2308,"context_line":"            # Any xapi error due to PV version are caught and migration"},{"line_number":2309,"context_line":"            # will be safely reverted by the rollback process."},{"line_number":2310,"context_line":"            try:"},{"line_number":2311,"context_line":"                self.ensure_pv_driver_info_for_live_migration(instance, vm_ref)"},{"line_number":2312,"context_line":"            except Exception as e:"},{"line_number":2313,"context_line":"                LOG.warning(e)"},{"line_number":2314,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_080e063b","line":2311,"updated":"2015-08-11 16:31:42.000000000","message":"I think this might be the wrong place to add this, see above.","commit_id":"f4fea2f4abdabc2d0c1fefe2142ed5c2854e6158"},{"author":{"_account_id":4340,"name":"Sulochan Acharya","email":"sulochan@gmail.com","username":"sulochan-acharya"},"change_message_id":"32d7047dccc8d8428c4d4b199f7d43620f2e9a1b","unresolved":false,"context_lines":[{"line_number":2308,"context_line":"            # Any xapi error due to PV version are caught and migration"},{"line_number":2309,"context_line":"            # will be safely reverted by the rollback process."},{"line_number":2310,"context_line":"            try:"},{"line_number":2311,"context_line":"                self.ensure_pv_driver_info_for_live_migration(instance, vm_ref)"},{"line_number":2312,"context_line":"            except Exception as e:"},{"line_number":2313,"context_line":"                LOG.warning(e)"},{"line_number":2314,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a4dcd0f_8c49aa33","line":2311,"in_reply_to":"1a4dcd0f_080e063b","updated":"2015-08-12 08:40:17.000000000","message":"We can move it to source check but that does not gain us anything. See comment above. The way it is i think will ensure that in case when the instance is actually missing driver we actually dont insert fake version info, this error happens, and will get caught and rolled back properly.\n\nSo my feeling is that this is the right place to call it. Let me know if you feel differently.","commit_id":"f4fea2f4abdabc2d0c1fefe2142ed5c2854e6158"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"605b87faf90d69c502a5fabdaaae48a59b756f07","unresolved":false,"context_lines":[{"line_number":2308,"context_line":"            # Any xapi error due to PV version are caught and migration"},{"line_number":2309,"context_line":"            # will be safely reverted by the rollback process."},{"line_number":2310,"context_line":"            try:"},{"line_number":2311,"context_line":"                self.ensure_pv_driver_info_for_live_migration(instance, vm_ref)"},{"line_number":2312,"context_line":"            except Exception as e:"},{"line_number":2313,"context_line":"                LOG.warning(e)"},{"line_number":2314,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"fa1b9901_65000d67","line":2311,"in_reply_to":"1a4dcd0f_8c49aa33","updated":"2015-08-25 17:03:56.000000000","message":"I am unsure, it feels like a bug that this isn\u0027t required for check_can_live_migrate, but this all goes away in future versions, so I guess if this works, I am fine with it.","commit_id":"f4fea2f4abdabc2d0c1fefe2142ed5c2854e6158"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"352d85cfa3f1e5adcd4e9cca0af69a730771ee75","unresolved":false,"context_lines":[{"line_number":1967,"context_line":"        # NOTE(sulo): These need to be string for valid field type"},{"line_number":1968,"context_line":"        # for xapi."},{"line_number":1969,"context_line":"        if ignore_missing_path:"},{"line_number":1970,"context_line":"            ignore_missing \u003d \u0027True\u0027"},{"line_number":1971,"context_line":"        else:"},{"line_number":1972,"context_line":"            ignore_missing \u003d \u0027False\u0027"},{"line_number":1973,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"1a4dcd0f_91805ef9","line":1970,"updated":"2015-08-14 13:04:59.000000000","message":"Can\u0027t we just use str(ignore_missing_path)?","commit_id":"1c9ffa2221156bb6205bfe7856350072b7c3d2b6"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"e09a6d3b5f617342796fda8cf86e51413991c327","unresolved":false,"context_lines":[{"line_number":1967,"context_line":"        # NOTE(sulo): These need to be string for valid field type"},{"line_number":1968,"context_line":"        # for xapi."},{"line_number":1969,"context_line":"        if ignore_missing_path:"},{"line_number":1970,"context_line":"            ignore_missing \u003d \u0027True\u0027"},{"line_number":1971,"context_line":"        else:"},{"line_number":1972,"context_line":"            ignore_missing \u003d \u0027False\u0027"},{"line_number":1973,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"fa1b9901_45a8698c","line":1970,"in_reply_to":"1a4dcd0f_91805ef9","updated":"2015-08-25 17:04:16.000000000","message":"my preference:\n\nignore_missing \u003d \u0027True\u0027 if ignore_missing_path else \u0027False\u0027\n\nbut this is fine.","commit_id":"1c9ffa2221156bb6205bfe7856350072b7c3d2b6"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"8459582f41432ed852f09c722cd5a01958f7e77f","unresolved":false,"context_lines":[{"line_number":2243,"context_line":""},{"line_number":2244,"context_line":"        return False"},{"line_number":2245,"context_line":""},{"line_number":2246,"context_line":"    def _write_fake_pv_version(self, instance, vm_ref):"},{"line_number":2247,"context_line":"        version \u003d self._session.product_version"},{"line_number":2248,"context_line":"        LOG.debug(\"Writing pvtools version major: %(major)s minor: %(minor)s \""},{"line_number":2249,"context_line":"                  \"micro: %(micro)s\", {\u0027major\u0027: version[0],"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa1b9901_918cce80","line":2246,"updated":"2015-08-18 14:41:49.000000000","message":"So the \"fake\" version is derived from the current host version?\n\nIs this necessarily accurate, and if so, why not just always generate this and have a single code path instead of conditionally counting on the xapi agent in the guest to report it?","commit_id":"1c9ffa2221156bb6205bfe7856350072b7c3d2b6"},{"author":{"_account_id":2835,"name":"Brian Elliott","email":"bdelliott@gmail.com","username":"belliott"},"change_message_id":"edf41d5b9bf79167394324c113a9f8464bef8505","unresolved":false,"context_lines":[{"line_number":2243,"context_line":""},{"line_number":2244,"context_line":"        return False"},{"line_number":2245,"context_line":""},{"line_number":2246,"context_line":"    def _write_fake_pv_version(self, instance, vm_ref):"},{"line_number":2247,"context_line":"        version \u003d self._session.product_version"},{"line_number":2248,"context_line":"        LOG.debug(\"Writing pvtools version major: %(major)s minor: %(minor)s \""},{"line_number":2249,"context_line":"                  \"micro: %(micro)s\", {\u0027major\u0027: version[0],"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa1b9901_7071df43","line":2246,"in_reply_to":"fa1b9901_6b06e921","updated":"2015-08-24 21:04:50.000000000","message":"Okay, so if the tools version is reported properly by the agent it\u0027ll be more accurate.  If not reported, we use a fake version equal to the XS host.\n\nWhat\u0027s the downside if fake_version \u003e actual_tools_version?","commit_id":"1c9ffa2221156bb6205bfe7856350072b7c3d2b6"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"e09a6d3b5f617342796fda8cf86e51413991c327","unresolved":false,"context_lines":[{"line_number":2243,"context_line":""},{"line_number":2244,"context_line":"        return False"},{"line_number":2245,"context_line":""},{"line_number":2246,"context_line":"    def _write_fake_pv_version(self, instance, vm_ref):"},{"line_number":2247,"context_line":"        version \u003d self._session.product_version"},{"line_number":2248,"context_line":"        LOG.debug(\"Writing pvtools version major: %(major)s minor: %(minor)s \""},{"line_number":2249,"context_line":"                  \"micro: %(micro)s\", {\u0027major\u0027: version[0],"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa1b9901_457f0917","line":2246,"in_reply_to":"fa1b9901_7071df43","updated":"2015-08-25 17:04:16.000000000","message":"I like only over-riding the version when we have too, it just seems less invasive.","commit_id":"1c9ffa2221156bb6205bfe7856350072b7c3d2b6"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"7069a7b6eec030d244804fcc17115a6c199b7783","unresolved":false,"context_lines":[{"line_number":2243,"context_line":""},{"line_number":2244,"context_line":"        return False"},{"line_number":2245,"context_line":""},{"line_number":2246,"context_line":"    def _write_fake_pv_version(self, instance, vm_ref):"},{"line_number":2247,"context_line":"        version \u003d self._session.product_version"},{"line_number":2248,"context_line":"        LOG.debug(\"Writing pvtools version major: %(major)s minor: %(minor)s \""},{"line_number":2249,"context_line":"                  \"micro: %(micro)s\", {\u0027major\u0027: version[0],"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa1b9901_6b06e921","line":2246,"in_reply_to":"fa1b9901_918cce80","updated":"2015-08-24 13:14:52.000000000","message":"The version should normally be the version of the tools that are installed.  This version is not enforced to be the XS host version by XAPI, so it shouldn\u0027t matter \n\nIf we\u0027re faking the version, then it could be an arbitrary version (e.g. 6.2.0) - but we might as well claim that it matches the XenServer version.  On the other hand, I would be hesitant about including a static version number in here as it\u0027s unlikely to be updated so would eventually refer to a very old XS version.","commit_id":"1c9ffa2221156bb6205bfe7856350072b7c3d2b6"}]}
