)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1e47245ab27c7a8aa536ac0f5faeaa6a89747a56","unresolved":false,"context_lines":[{"line_number":14,"context_line":"conductor, which makes a problem for attached volumes."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This early in the process the host has just barely been chosen and no SR"},{"line_number":17,"context_line":"information has been configured yet for these volumes or their initators."},{"line_number":18,"context_line":"Additionally we cannot prepare this SR any earlier as BDM information is"},{"line_number":19,"context_line":"not set up until the pre_live_migration method. With the options to either"},{"line_number":20,"context_line":"skip this assertion completely or swallow the exception, I have chosen to"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1f9dbf25_2b07201f","line":17,"range":{"start_line":17,"start_character":63,"end_line":17,"end_character":72},"updated":"2018-03-01 10:46:26.000000000","message":"initiators","commit_id":"8e23a0ffcf68b682268700952ad95724cee155a9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1e47245ab27c7a8aa536ac0f5faeaa6a89747a56","unresolved":false,"context_lines":[{"line_number":20,"context_line":"skip this assertion completely or swallow the exception, I have chosen to"},{"line_number":21,"context_line":"swallow the exception.  My reasons for this are two-fold:"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"1. --Block-migration can be called without regard for whether an iSCSI"},{"line_number":24,"context_line":"volume is attached, and we still want to ensure that VIF, CPU and other"},{"line_number":25,"context_line":"factors are checked, and not just skip all checks entirely."},{"line_number":26,"context_line":"2. Currently the Assert only exists within the --block-migration code"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1f9dbf25_3e199c78","line":23,"range":{"start_line":23,"start_character":5,"end_line":23,"end_character":6},"updated":"2018-03-01 10:46:26.000000000","message":"b","commit_id":"8e23a0ffcf68b682268700952ad95724cee155a9"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"2c6e29d48498cbbe6053ea371365aec141e66dcf","unresolved":false,"context_lines":[{"line_number":20,"context_line":"skip this assertion completely or swallow the exception, I have chosen to"},{"line_number":21,"context_line":"swallow the exception.  My reasons for this are two-fold:"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"1. --Block-migration can be called without regard for whether an iSCSI"},{"line_number":24,"context_line":"volume is attached, and we still want to ensure that VIF, CPU and other"},{"line_number":25,"context_line":"factors are checked, and not just skip all checks entirely."},{"line_number":26,"context_line":"2. Currently the Assert only exists within the --block-migration code"},{"line_number":27,"context_line":"base but this needs to change. A future commit will remove this logic"},{"line_number":28,"context_line":"to ensure that the commit runs without this flag. Once that is done we"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"5f93b717_04a15346","line":25,"range":{"start_line":23,"start_character":3,"end_line":25,"end_character":59},"updated":"2018-02-01 09:07:32.000000000","message":"A good reason:-)","commit_id":"8e23a0ffcf68b682268700952ad95724cee155a9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1e47245ab27c7a8aa536ac0f5faeaa6a89747a56","unresolved":false,"context_lines":[{"line_number":27,"context_line":"base but this needs to change. A future commit will remove this logic"},{"line_number":28,"context_line":"to ensure that the commit runs without this flag. Once that is done we"},{"line_number":29,"context_line":"want to be able to continue to use this Exception swallow logic rather"},{"line_number":30,"context_line":"than continuing to skip  the assert for all XCP2.1.0+ even without volumes."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"This decision should help us handle less work in a future commit and does not"},{"line_number":33,"context_line":"seem to align with the goals of that commit, where it does align properly here."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1f9dbf25_1e4aa054","line":30,"range":{"start_line":30,"start_character":24,"end_line":30,"end_character":25},"updated":"2018-03-01 10:46:26.000000000","message":"s/  / /","commit_id":"8e23a0ffcf68b682268700952ad95724cee155a9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1e47245ab27c7a8aa536ac0f5faeaa6a89747a56","unresolved":false,"context_lines":[{"line_number":89,"context_line":""},{"line_number":90,"context_line":"Finally, an adjustment to the Fake XenAPI driver had to be made as it currently"},{"line_number":91,"context_line":"does not include the host details beyond hostname environment in the"},{"line_number":92,"context_line":"create_host check.  The change ammends the stub dictionary to include this data."},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"Change-Id: Ie5295564a68f877fc061376c00f3fa706370a251"},{"line_number":95,"context_line":"Closes-Bug: #1704071"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1f9dbf25_7ed96481","line":92,"range":{"start_line":92,"start_character":31,"end_line":92,"end_character":38},"updated":"2018-03-01 10:46:26.000000000","message":"amends","commit_id":"8e23a0ffcf68b682268700952ad95724cee155a9"}],"nova/tests/unit/virt/xenapi/test_vmops.py":[{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"0a08fda0141060b4dae7339fc7db2711cc715749","unresolved":false,"context_lines":[{"line_number":1795,"context_line":"                migrate_data)"},{"line_number":1796,"context_line":""},{"line_number":1797,"context_line":"    @mock.patch.object(vmops.VMOps, \u0027_call_live_migrate_command\u0027)"},{"line_number":1798,"context_line":"    def test_check_can_live_migrate_source_with_xcp2(self, mock_call_migrate):"},{"line_number":1799,"context_line":"        ctxt \u003d \u0027ctxt\u0027"},{"line_number":1800,"context_line":"        fake_instance \u003d {\"name\": \"fake_instance\"}"},{"line_number":1801,"context_line":"        fake_dest_check_data \u003d objects.XenapiLiveMigrateData()"}],"source_content_type":"text/x-python","patch_set":5,"id":"df7087c5_1bd97682","line":1798,"range":{"start_line":1798,"start_character":8,"end_line":1798,"end_character":52},"updated":"2018-03-07 10:11:19.000000000","message":"Seems we don\u0027t have any UT to cover the exception of MigrationPreCheckError for check_can_live_migrate_source. Worthy to add an unit test to check that the exception will raise if the failure is not \u0027VDI_NOT_IN_MAP\u0027.","commit_id":"86e6892fe822dd8ea755b24d418e10216e5b8446"},{"author":{"_account_id":18020,"name":"Brooks Kaminski","email":"bhkaminski@gmail.com","username":"bhkaminski1"},"change_message_id":"b78cc8fe16696c34b0a5941159e7742b4fdb8069","unresolved":false,"context_lines":[{"line_number":1795,"context_line":"                migrate_data)"},{"line_number":1796,"context_line":""},{"line_number":1797,"context_line":"    @mock.patch.object(vmops.VMOps, \u0027_call_live_migrate_command\u0027)"},{"line_number":1798,"context_line":"    def test_check_can_live_migrate_source_with_xcp2(self, mock_call_migrate):"},{"line_number":1799,"context_line":"        ctxt \u003d \u0027ctxt\u0027"},{"line_number":1800,"context_line":"        fake_instance \u003d {\"name\": \"fake_instance\"}"},{"line_number":1801,"context_line":"        fake_dest_check_data \u003d objects.XenapiLiveMigrateData()"}],"source_content_type":"text/x-python","patch_set":5,"id":"df7087c5_678250cb","line":1798,"range":{"start_line":1798,"start_character":8,"end_line":1798,"end_character":52},"in_reply_to":"df7087c5_1bd97682","updated":"2018-03-07 14:30:31.000000000","message":"Done","commit_id":"86e6892fe822dd8ea755b24d418e10216e5b8446"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"0a08fda0141060b4dae7339fc7db2711cc715749","unresolved":false,"context_lines":[{"line_number":1813,"context_line":"            mock_vm.return_value \u003d \u0027vm_ref\u0027"},{"line_number":1814,"context_line":"            mock_host_sw.return_value \u003d {\u0027platform_name\u0027: \u0027XCP\u0027,"},{"line_number":1815,"context_line":"                                         \u0027platform_version\u0027: \u00272.1.0\u0027}"},{"line_number":1816,"context_line":"            fake_dest_check_data \u003d self.vmops.check_can_live_migrate_source("},{"line_number":1817,"context_line":"                ctxt, fake_instance, fake_dest_check_data)"},{"line_number":1818,"context_line":""},{"line_number":1819,"context_line":"        self.assertTrue(fake_dest_check_data)"}],"source_content_type":"text/x-python","patch_set":5,"id":"df7087c5_98502c35","line":1816,"range":{"start_line":1816,"start_character":12,"end_line":1816,"end_character":32},"updated":"2018-03-07 10:11:19.000000000","message":"I\u0027d suggest use another variable name instead of overriding the fake data.","commit_id":"86e6892fe822dd8ea755b24d418e10216e5b8446"},{"author":{"_account_id":18020,"name":"Brooks Kaminski","email":"bhkaminski@gmail.com","username":"bhkaminski1"},"change_message_id":"b78cc8fe16696c34b0a5941159e7742b4fdb8069","unresolved":false,"context_lines":[{"line_number":1813,"context_line":"            mock_vm.return_value \u003d \u0027vm_ref\u0027"},{"line_number":1814,"context_line":"            mock_host_sw.return_value \u003d {\u0027platform_name\u0027: \u0027XCP\u0027,"},{"line_number":1815,"context_line":"                                         \u0027platform_version\u0027: \u00272.1.0\u0027}"},{"line_number":1816,"context_line":"            fake_dest_check_data \u003d self.vmops.check_can_live_migrate_source("},{"line_number":1817,"context_line":"                ctxt, fake_instance, fake_dest_check_data)"},{"line_number":1818,"context_line":""},{"line_number":1819,"context_line":"        self.assertTrue(fake_dest_check_data)"}],"source_content_type":"text/x-python","patch_set":5,"id":"df7087c5_aa27cdb6","line":1816,"range":{"start_line":1816,"start_character":12,"end_line":1816,"end_character":32},"in_reply_to":"df7087c5_98502c35","updated":"2018-03-07 14:30:31.000000000","message":"Done","commit_id":"86e6892fe822dd8ea755b24d418e10216e5b8446"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"0a08fda0141060b4dae7339fc7db2711cc715749","unresolved":false,"context_lines":[{"line_number":1816,"context_line":"            fake_dest_check_data \u003d self.vmops.check_can_live_migrate_source("},{"line_number":1817,"context_line":"                ctxt, fake_instance, fake_dest_check_data)"},{"line_number":1818,"context_line":""},{"line_number":1819,"context_line":"        self.assertTrue(fake_dest_check_data)"},{"line_number":1820,"context_line":""},{"line_number":1821,"context_line":"    def test_generate_vif_network_map(self):"},{"line_number":1822,"context_line":"        with mock.patch.object(self._session.VIF,"}],"source_content_type":"text/x-python","patch_set":5,"id":"df7087c5_db5f7e0c","line":1819,"range":{"start_line":1819,"start_character":13,"end_line":1819,"end_character":23},"updated":"2018-03-07 10:11:19.000000000","message":"fake_dest_check_data is not a bool value. You need check if fake_dest_check_data is returned. Also worthy to check if the function of mock_host_sw is really invoked.","commit_id":"86e6892fe822dd8ea755b24d418e10216e5b8446"},{"author":{"_account_id":18020,"name":"Brooks Kaminski","email":"bhkaminski@gmail.com","username":"bhkaminski1"},"change_message_id":"b78cc8fe16696c34b0a5941159e7742b4fdb8069","unresolved":false,"context_lines":[{"line_number":1816,"context_line":"            fake_dest_check_data \u003d self.vmops.check_can_live_migrate_source("},{"line_number":1817,"context_line":"                ctxt, fake_instance, fake_dest_check_data)"},{"line_number":1818,"context_line":""},{"line_number":1819,"context_line":"        self.assertTrue(fake_dest_check_data)"},{"line_number":1820,"context_line":""},{"line_number":1821,"context_line":"    def test_generate_vif_network_map(self):"},{"line_number":1822,"context_line":"        with mock.patch.object(self._session.VIF,"}],"source_content_type":"text/x-python","patch_set":5,"id":"df7087c5_0a191978","line":1819,"range":{"start_line":1819,"start_character":13,"end_line":1819,"end_character":23},"in_reply_to":"df7087c5_db5f7e0c","updated":"2018-03-07 14:30:31.000000000","message":"Done","commit_id":"86e6892fe822dd8ea755b24d418e10216e5b8446"}],"nova/virt/xenapi/vmops.py":[{"author":{"_account_id":17922,"name":"Huan Xie","email":"huan.xie@citrix.com","username":"huan"},"change_message_id":"9a2ce7d811a2c1fcf88ea2b0a111c24a59c98e8a","unresolved":false,"context_lines":[{"line_number":2372,"context_line":"                # Since product versions here can have microversions we cannot"},{"line_number":2373,"context_line":"                # use a normal float comparison.  We just pull the major"},{"line_number":2374,"context_line":"                # version to compare."},{"line_number":2375,"context_line":"                if (host_sw[\u0027product_brand\u0027] \u003d\u003d \"XenServer\" and"},{"line_number":2376,"context_line":"                        int(host_sw[\u0027product_version\u0027][0]) \u003e\u003d 7):"},{"line_number":2377,"context_line":"                    pass"},{"line_number":2378,"context_line":"                else:"},{"line_number":2379,"context_line":"                    self._call_live_migrate_command("},{"line_number":2380,"context_line":"                        \"VM.assert_can_migrate\", vm_ref, dest_check_data)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f93b717_79f4eb86","line":2377,"range":{"start_line":2375,"start_character":0,"end_line":2377,"end_character":24},"updated":"2018-01-29 02:46:26.000000000","message":"Hi, I have some concern:\n(1) If we skip the check (I mean VM.assert_can_migrate), we will hide other real checking, e.g. The CPU type, the vif:network mapping. And in later step, we may fail again as the live migration cannot satisfy the required condition.\n(2) If we do want to skip the whole checking, then I think using XCP version is better than product version, i.e. platform_version","commit_id":"4ab3eb24565ba4dc931f4d0a5677a12cbf96b3ef"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"7ffd47718e255a190067e6c499d35971b041138b","unresolved":false,"context_lines":[{"line_number":2372,"context_line":"                # Since product versions here can have microversions we cannot"},{"line_number":2373,"context_line":"                # use a normal float comparison.  We just pull the major"},{"line_number":2374,"context_line":"                # version to compare."},{"line_number":2375,"context_line":"                if (host_sw[\u0027product_brand\u0027] \u003d\u003d \"XenServer\" and"},{"line_number":2376,"context_line":"                        int(host_sw[\u0027product_version\u0027][0]) \u003e\u003d 7):"},{"line_number":2377,"context_line":"                    pass"},{"line_number":2378,"context_line":"                else:"},{"line_number":2379,"context_line":"                    self._call_live_migrate_command("},{"line_number":2380,"context_line":"                        \"VM.assert_can_migrate\", vm_ref, dest_check_data)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f93b717_6750b4d5","line":2377,"range":{"start_line":2375,"start_character":0,"end_line":2377,"end_character":24},"in_reply_to":"5f93b717_13eb4079","updated":"2018-01-29 12:01:21.000000000","message":"1) I\u0027m a bit confused; why is the check for being able to migrate to this destiation only performed if we have a block device?  assert_can_migrate should surely be called in *all* situations to perform the CPU / VIF check?  This can be treated as a second bug rather than trying to fix it here.  The original code was only calling assert_can_migrate in the case of block devices, therefore I think we only need to worry about block devices and not CPU/VIF/etc.\nEven with block devices, there could be reasons other than the new SR check that fail the assert - so only looking at VDI_MAP is best indeed.\n\n2) For product_version we should actually be using platform_version (note that \"platform_name\" is XCP).  The reasoning is that XenServer is just the Citrix branding.  There are other hypervisor products based on Xen+XAPI which are not branded XenServer","commit_id":"4ab3eb24565ba4dc931f4d0a5677a12cbf96b3ef"},{"author":{"_account_id":18020,"name":"Brooks Kaminski","email":"bhkaminski@gmail.com","username":"bhkaminski1"},"change_message_id":"95d2b8a776967222d4625dc1d9bad489f35a4edc","unresolved":false,"context_lines":[{"line_number":2372,"context_line":"                # Since product versions here can have microversions we cannot"},{"line_number":2373,"context_line":"                # use a normal float comparison.  We just pull the major"},{"line_number":2374,"context_line":"                # version to compare."},{"line_number":2375,"context_line":"                if (host_sw[\u0027product_brand\u0027] \u003d\u003d \"XenServer\" and"},{"line_number":2376,"context_line":"                        int(host_sw[\u0027product_version\u0027][0]) \u003e\u003d 7):"},{"line_number":2377,"context_line":"                    pass"},{"line_number":2378,"context_line":"                else:"},{"line_number":2379,"context_line":"                    self._call_live_migrate_command("},{"line_number":2380,"context_line":"                        \"VM.assert_can_migrate\", vm_ref, dest_check_data)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f93b717_ca9f4d01","line":2377,"range":{"start_line":2375,"start_character":0,"end_line":2377,"end_character":24},"in_reply_to":"5f93b717_6750b4d5","updated":"2018-01-29 14:11:49.000000000","message":"1)  I agree.. it was fairly shocking to me to see this code only within this code block.. and I agree it\u0027s something that we need to address in the future.  Your final statement seems to indicate you wish to accept and swallow the assert instead of skipping as well, is this accurate?  Just getting a feel of what you guys would like to see more before I make any adjustments.","commit_id":"4ab3eb24565ba4dc931f4d0a5677a12cbf96b3ef"},{"author":{"_account_id":18020,"name":"Brooks Kaminski","email":"bhkaminski@gmail.com","username":"bhkaminski1"},"change_message_id":"58d99ed4dcfda17c51e404276f68e826de84256e","unresolved":false,"context_lines":[{"line_number":2372,"context_line":"                # Since product versions here can have microversions we cannot"},{"line_number":2373,"context_line":"                # use a normal float comparison.  We just pull the major"},{"line_number":2374,"context_line":"                # version to compare."},{"line_number":2375,"context_line":"                if (host_sw[\u0027product_brand\u0027] \u003d\u003d \"XenServer\" and"},{"line_number":2376,"context_line":"                        int(host_sw[\u0027product_version\u0027][0]) \u003e\u003d 7):"},{"line_number":2377,"context_line":"                    pass"},{"line_number":2378,"context_line":"                else:"},{"line_number":2379,"context_line":"                    self._call_live_migrate_command("},{"line_number":2380,"context_line":"                        \"VM.assert_can_migrate\", vm_ref, dest_check_data)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f93b717_13eb4079","line":2377,"range":{"start_line":2375,"start_character":0,"end_line":2377,"end_character":24},"in_reply_to":"5f93b717_79f4eb86","updated":"2018-01-29 03:16:16.000000000","message":"I think we have a couple of options here left to us without major revamp of the code now that we know that we do not need to keep this mapping yet.  We can use your similar code to swallow the exception ONLY if the VDI_MAP fails but I do not believe we need to then rebuild it or change the way the existing codebase handles this situation, to avoid overhauling xenapis live migration codebase to work around a feature. This will allow us to continue to assert and swallow exceptions in a specific case and move on, since all we need to return is the dest_data.   Alternatively if we do continue to skip, what would be the benefit of utilizing XCP Ver over Xenserver Product Ver?","commit_id":"4ab3eb24565ba4dc931f4d0a5677a12cbf96b3ef"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"c21c105587b9a4b0ea93b595ef4de03b4925ddc8","unresolved":false,"context_lines":[{"line_number":2372,"context_line":"                # Since product versions here can have microversions we cannot"},{"line_number":2373,"context_line":"                # use a normal float comparison.  We just pull the major"},{"line_number":2374,"context_line":"                # version to compare."},{"line_number":2375,"context_line":"                if (host_sw[\u0027product_brand\u0027] \u003d\u003d \"XenServer\" and"},{"line_number":2376,"context_line":"                        int(host_sw[\u0027product_version\u0027][0]) \u003e\u003d 7):"},{"line_number":2377,"context_line":"                    pass"},{"line_number":2378,"context_line":"                else:"},{"line_number":2379,"context_line":"                    self._call_live_migrate_command("},{"line_number":2380,"context_line":"                        \"VM.assert_can_migrate\", vm_ref, dest_check_data)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5f93b717_798c87d6","line":2377,"range":{"start_line":2375,"start_character":0,"end_line":2377,"end_character":24},"in_reply_to":"5f93b717_ca9f4d01","updated":"2018-02-01 03:21:59.000000000","message":"All,\n\nI have a check on the history. \"VM.assert_can_migrate\" was not only for block device before this commit:\nhttps://review.openstack.org/#/c/247853/23/nova/virt/xenapi/vmops.py@2194\n\nI don\u0027t know the reason why it went in that way.\nBut anyway I think it\u0027s accepted to skip VM.assert_can_migrate for \u003eXS7.0 (platform version is 2.1) basing on the situation. And I\u0027d suggest move the platform version check to line 2364. And add comments on \u003eXS7(plt\u003e\u003d2.1) has problem on block migration checking.\n\nBob\u0027s suggestion on \"only looking at VDI_MAP\u0027 is good if we can do it. But unfortunately seems we can\u0027t. The reason is that since XS7.0, assert_cam_migrate requires vif_map, vdi_map existing in the input parameters. Actually it ever was to empty vdi_map and vif_map before invoking \"assert_cam_migrate\". \nhttps://review.openstack.org/#/c/9879/10/nova/virt/xenapi/vmops.py@1550\n\nAlso see the errors of \"VIF_NOT_IN_MAP\" in https://bugs.launchpad.net/nova/+bug/1658877\n\nRegards,\nJianghua","commit_id":"4ab3eb24565ba4dc931f4d0a5677a12cbf96b3ef"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1e47245ab27c7a8aa536ac0f5faeaa6a89747a56","unresolved":false,"context_lines":[{"line_number":2242,"context_line":"        # Works around aggregate checking as not all places use aggregates."},{"line_number":2243,"context_line":"        host_ref \u003d self._get_host_ref_no_aggr()"},{"line_number":2244,"context_line":"        host_rec \u003d self._session.call_xenapi(\"host.get_record\", host_ref)"},{"line_number":2245,"context_line":"        return host_rec[\u0027software_version\u0027]"},{"line_number":2246,"context_line":""},{"line_number":2247,"context_line":"    def _get_network_ref(self):"},{"line_number":2248,"context_line":"        # Get the network to for migrate."}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_de0438c9","line":2245,"range":{"start_line":2245,"start_character":23,"end_line":2245,"end_character":43},"updated":"2018-03-01 10:46:26.000000000","message":"is the \u0027software_version\u0027 key guaranteed to always be in the host_rec dict?","commit_id":"8e23a0ffcf68b682268700952ad95724cee155a9"},{"author":{"_account_id":18020,"name":"Brooks Kaminski","email":"bhkaminski@gmail.com","username":"bhkaminski1"},"change_message_id":"0c9ab3a7c9fec356c47bd0f02f2003162a1b2959","unresolved":false,"context_lines":[{"line_number":2242,"context_line":"        # Works around aggregate checking as not all places use aggregates."},{"line_number":2243,"context_line":"        host_ref \u003d self._get_host_ref_no_aggr()"},{"line_number":2244,"context_line":"        host_rec \u003d self._session.call_xenapi(\"host.get_record\", host_ref)"},{"line_number":2245,"context_line":"        return host_rec[\u0027software_version\u0027]"},{"line_number":2246,"context_line":""},{"line_number":2247,"context_line":"    def _get_network_ref(self):"},{"line_number":2248,"context_line":"        # Get the network to for migrate."}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_810f352d","line":2245,"range":{"start_line":2245,"start_character":23,"end_line":2245,"end_character":43},"in_reply_to":"1f9dbf25_de0438c9","updated":"2018-03-01 11:15:46.000000000","message":"Thanks for the review Jay, I will answer both fields here.  These fields are the Xen Toolstack Platform driver in use by the system so they will indeed exist within dict for a XenAPI driven system. Most generally this will be XCP but for older releases it may be XenAPI / XL / XEND or possibly even Libvirsh. While I don\u0027t suspect anything other than XCP would be running openstack, it is still best to grab the toolstack to verify before we run these swallows. The same here for the versions, it should always report.  In regards to the location, it will always report on Dom0, the controller domain for the particular hypervisor.  Pulling data from here rather than the aggregates allows us to avoid situations where aggregates for Xen HA are not in use.  In concept we should be able to pull this resident_on field from any of the VMs on the hypervisor but I HAVE seen cases were non-dom0 entities did indeed have a blank field due to some syncing shenanigans in XS, thus the reliance on domain 0 here.","commit_id":"8e23a0ffcf68b682268700952ad95724cee155a9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1e47245ab27c7a8aa536ac0f5faeaa6a89747a56","unresolved":false,"context_lines":[{"line_number":2368,"context_line":"                dest_check_data.block_migration):"},{"line_number":2369,"context_line":"            vm_ref \u003d self._get_vm_opaque_ref(instance_ref)"},{"line_number":2370,"context_line":"            host_sw \u003d self._get_host_software_versions()"},{"line_number":2371,"context_line":"            host_pfv \u003d host_sw[\u0027platform_version\u0027]"},{"line_number":2372,"context_line":"            try:"},{"line_number":2373,"context_line":"                self._call_live_migrate_command("},{"line_number":2374,"context_line":"                    \"VM.assert_can_migrate\", vm_ref, dest_check_data)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1f9dbf25_3e18bc1b","line":2371,"range":{"start_line":2371,"start_character":23,"end_line":2371,"end_character":50},"updated":"2018-03-01 10:46:26.000000000","message":"similar question... is the \u0027platform_version\u0027 key guaranteed to always be in the host_sw dict?","commit_id":"8e23a0ffcf68b682268700952ad95724cee155a9"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"324c1ed0f3dbc0e39cf36de10bdc9d3850b3104f","unresolved":false,"context_lines":[{"line_number":2378,"context_line":"                # as the SR has not been configured on the hypervisor at this"},{"line_number":2379,"context_line":"                # point in the migration.  We swallow this exception until a"},{"line_number":2380,"context_line":"                # more intensive refactor can be done to correct this."},{"line_number":2381,"context_line":"                if (\"VDI_NOT_IN_MAP\" in reason and"},{"line_number":2382,"context_line":"                        host_sw[\u0027platform_name\u0027] \u003d\u003d \"XCP\" and"},{"line_number":2383,"context_line":"                        versionutils.is_compatible(\"2.1.0\", host_pfv)):"},{"line_number":2384,"context_line":"                    LOG.debug(\"Skipping exception for XCP\u003e\u003d2.1.0, %s\", reason)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f93b717_b929a137","line":2381,"range":{"start_line":2381,"start_character":21,"end_line":2381,"end_character":46},"updated":"2018-02-01 13:37:09.000000000","message":"So now that we\u0027re running the check, and swalling this exception, do we actually have to check for the platform version?\nEarlier versions cannot throw this exception here (as I understand it)","commit_id":"8e23a0ffcf68b682268700952ad95724cee155a9"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"20c3b27b4302fa8a5ff4183dd78bb2518b67d19c","unresolved":false,"context_lines":[{"line_number":2378,"context_line":"                # as the SR has not been configured on the hypervisor at this"},{"line_number":2379,"context_line":"                # point in the migration.  We swallow this exception until a"},{"line_number":2380,"context_line":"                # more intensive refactor can be done to correct this."},{"line_number":2381,"context_line":"                if (\"VDI_NOT_IN_MAP\" in reason and"},{"line_number":2382,"context_line":"                        host_sw[\u0027platform_name\u0027] \u003d\u003d \"XCP\" and"},{"line_number":2383,"context_line":"                        versionutils.is_compatible(\"2.1.0\", host_pfv)):"},{"line_number":2384,"context_line":"                    LOG.debug(\"Skipping exception for XCP\u003e\u003d2.1.0, %s\", reason)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f93b717_353c9f2d","line":2381,"range":{"start_line":2381,"start_character":21,"end_line":2381,"end_character":46},"in_reply_to":"5f93b717_a3b39ae8","updated":"2018-02-02 08:47:41.000000000","message":"OK - that\u0027s fine.","commit_id":"8e23a0ffcf68b682268700952ad95724cee155a9"},{"author":{"_account_id":18020,"name":"Brooks Kaminski","email":"bhkaminski@gmail.com","username":"bhkaminski1"},"change_message_id":"cd9e8d25479c359210b0d9154b8264de0cc6b5a0","unresolved":false,"context_lines":[{"line_number":2378,"context_line":"                # as the SR has not been configured on the hypervisor at this"},{"line_number":2379,"context_line":"                # point in the migration.  We swallow this exception until a"},{"line_number":2380,"context_line":"                # more intensive refactor can be done to correct this."},{"line_number":2381,"context_line":"                if (\"VDI_NOT_IN_MAP\" in reason and"},{"line_number":2382,"context_line":"                        host_sw[\u0027platform_name\u0027] \u003d\u003d \"XCP\" and"},{"line_number":2383,"context_line":"                        versionutils.is_compatible(\"2.1.0\", host_pfv)):"},{"line_number":2384,"context_line":"                    LOG.debug(\"Skipping exception for XCP\u003e\u003d2.1.0, %s\", reason)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f93b717_59973d49","line":2381,"range":{"start_line":2381,"start_character":21,"end_line":2381,"end_character":46},"in_reply_to":"5f93b717_b929a137","updated":"2018-02-01 13:43:02.000000000","message":"Hi Bob, I believe that this error can still be thrown in some cases such as an empty list being sent, but let me get some confirmation on that.","commit_id":"8e23a0ffcf68b682268700952ad95724cee155a9"},{"author":{"_account_id":18020,"name":"Brooks Kaminski","email":"bhkaminski@gmail.com","username":"bhkaminski1"},"change_message_id":"d01b7c806f1256194f6b2895920c702164fb9691","unresolved":false,"context_lines":[{"line_number":2378,"context_line":"                # as the SR has not been configured on the hypervisor at this"},{"line_number":2379,"context_line":"                # point in the migration.  We swallow this exception until a"},{"line_number":2380,"context_line":"                # more intensive refactor can be done to correct this."},{"line_number":2381,"context_line":"                if (\"VDI_NOT_IN_MAP\" in reason and"},{"line_number":2382,"context_line":"                        host_sw[\u0027platform_name\u0027] \u003d\u003d \"XCP\" and"},{"line_number":2383,"context_line":"                        versionutils.is_compatible(\"2.1.0\", host_pfv)):"},{"line_number":2384,"context_line":"                    LOG.debug(\"Skipping exception for XCP\u003e\u003d2.1.0, %s\", reason)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5f93b717_a3b39ae8","line":2381,"range":{"start_line":2381,"start_character":21,"end_line":2381,"end_character":46},"in_reply_to":"5f93b717_b929a137","updated":"2018-02-02 05:37:33.000000000","message":"I looked into this some more with Jianghua and it looks like this exception was added quite a long time ago: \n\nhttps://github.com/xapi-project/xen-api/commit/6eaad3b6745c0392ac3b3ef91c33451b1f110aed\n\nWhile I cannot say why we haven\u0027t run into this in the past, the error possibility does seem to at least exist pre XCP2.1 even though we can not seem to recognize completely how to trigger it in the older environment.  \n\nDue to the pretty light handed nature of the extra logic here I would personally rather keep it to be safe.  Do you agree?","commit_id":"8e23a0ffcf68b682268700952ad95724cee155a9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"246976646037c38104e40e4795d3268e519c0700","unresolved":false,"context_lines":[{"line_number":2372,"context_line":"            try:"},{"line_number":2373,"context_line":"                self._call_live_migrate_command("},{"line_number":2374,"context_line":"                    \"VM.assert_can_migrate\", vm_ref, dest_check_data)"},{"line_number":2375,"context_line":"            except self._session.XenAPI.Failure as exc:"},{"line_number":2376,"context_line":"                reason \u003d exc.details[0]"},{"line_number":2377,"context_line":"                # XCP\u003e\u003d2.1 Will error on this assert call if iSCSI are attached"},{"line_number":2378,"context_line":"                # as the SR has not been configured on the hypervisor at this"},{"line_number":2379,"context_line":"                # point in the migration.  We swallow this exception until a"},{"line_number":2380,"context_line":"                # more intensive refactor can be done to correct this."},{"line_number":2381,"context_line":"                if (\"VDI_NOT_IN_MAP\" in reason and"},{"line_number":2382,"context_line":"                        host_sw[\u0027platform_name\u0027] \u003d\u003d \"XCP\" and"},{"line_number":2383,"context_line":"                        versionutils.is_compatible(\"2.1.0\", host_pfv)):"},{"line_number":2384,"context_line":"                    LOG.debug(\"Skipping exception for XCP\u003e\u003d2.1.0, %s\", reason)"},{"line_number":2385,"context_line":"                    return dest_check_data"},{"line_number":2386,"context_line":"                msg \u003d _(\u0027assert_can_migrate failed because: %s\u0027) % reason"},{"line_number":2387,"context_line":"                LOG.debug(msg, exc_info\u003dTrue)"},{"line_number":2388,"context_line":"                raise exception.MigrationPreCheckError(reason\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff6b8bd7_7621944d","line":2385,"range":{"start_line":2375,"start_character":12,"end_line":2385,"end_character":42},"updated":"2018-03-06 15:39:25.000000000","message":"We should be able to have a simple mocked unit test for this scenario where _call_live_migrate_command raises self._session.XenAPI.Failure and the condition evaluates to True such that we return dest_check_data and don\u0027t raise MigrationPreCheckError. Then if I remove the code fix here, the test would fail because MigrationPreCheckError would get raised.","commit_id":"35a92f0e2483856ed0f6cf75c7a2a199d23bc5de"},{"author":{"_account_id":18020,"name":"Brooks Kaminski","email":"bhkaminski@gmail.com","username":"bhkaminski1"},"change_message_id":"4277d057c5de19a88c9486ea2d3553ca0d41d68b","unresolved":false,"context_lines":[{"line_number":2372,"context_line":"            try:"},{"line_number":2373,"context_line":"                self._call_live_migrate_command("},{"line_number":2374,"context_line":"                    \"VM.assert_can_migrate\", vm_ref, dest_check_data)"},{"line_number":2375,"context_line":"            except self._session.XenAPI.Failure as exc:"},{"line_number":2376,"context_line":"                reason \u003d exc.details[0]"},{"line_number":2377,"context_line":"                # XCP\u003e\u003d2.1 Will error on this assert call if iSCSI are attached"},{"line_number":2378,"context_line":"                # as the SR has not been configured on the hypervisor at this"},{"line_number":2379,"context_line":"                # point in the migration.  We swallow this exception until a"},{"line_number":2380,"context_line":"                # more intensive refactor can be done to correct this."},{"line_number":2381,"context_line":"                if (\"VDI_NOT_IN_MAP\" in reason and"},{"line_number":2382,"context_line":"                        host_sw[\u0027platform_name\u0027] \u003d\u003d \"XCP\" and"},{"line_number":2383,"context_line":"                        versionutils.is_compatible(\"2.1.0\", host_pfv)):"},{"line_number":2384,"context_line":"                    LOG.debug(\"Skipping exception for XCP\u003e\u003d2.1.0, %s\", reason)"},{"line_number":2385,"context_line":"                    return dest_check_data"},{"line_number":2386,"context_line":"                msg \u003d _(\u0027assert_can_migrate failed because: %s\u0027) % reason"},{"line_number":2387,"context_line":"                LOG.debug(msg, exc_info\u003dTrue)"},{"line_number":2388,"context_line":"                raise exception.MigrationPreCheckError(reason\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":4,"id":"df7087c5_c88d777c","line":2385,"range":{"start_line":2375,"start_character":12,"end_line":2385,"end_character":42},"in_reply_to":"ff6b8bd7_7621944d","updated":"2018-03-07 04:48:16.000000000","message":"Done - Thanks so much for the assistance with this.  It helped me learn a lot about these tests.","commit_id":"35a92f0e2483856ed0f6cf75c7a2a199d23bc5de"},{"author":{"_account_id":17920,"name":"Jianghua Wang","email":"jianghua.wang@citrix.com","username":"wjhfresh"},"change_message_id":"0a08fda0141060b4dae7339fc7db2711cc715749","unresolved":false,"context_lines":[{"line_number":2376,"context_line":"                reason \u003d exc.details[0]"},{"line_number":2377,"context_line":"                # XCP\u003e\u003d2.1 Will error on this assert call if iSCSI are attached"},{"line_number":2378,"context_line":"                # as the SR has not been configured on the hypervisor at this"},{"line_number":2379,"context_line":"                # point in the migration.  We swallow this exception until a"},{"line_number":2380,"context_line":"                # more intensive refactor can be done to correct this."},{"line_number":2381,"context_line":"                if (\"VDI_NOT_IN_MAP\" in reason and"},{"line_number":2382,"context_line":"                        host_sw[\u0027platform_name\u0027] \u003d\u003d \"XCP\" and"}],"source_content_type":"text/x-python","patch_set":5,"id":"df7087c5_d89a246d","line":2379,"range":{"start_line":2379,"start_character":41,"end_line":2379,"end_character":43},"updated":"2018-03-07 10:11:19.000000000","message":"nit: Remove the redundant white space.","commit_id":"86e6892fe822dd8ea755b24d418e10216e5b8446"},{"author":{"_account_id":18020,"name":"Brooks Kaminski","email":"bhkaminski@gmail.com","username":"bhkaminski1"},"change_message_id":"b78cc8fe16696c34b0a5941159e7742b4fdb8069","unresolved":false,"context_lines":[{"line_number":2376,"context_line":"                reason \u003d exc.details[0]"},{"line_number":2377,"context_line":"                # XCP\u003e\u003d2.1 Will error on this assert call if iSCSI are attached"},{"line_number":2378,"context_line":"                # as the SR has not been configured on the hypervisor at this"},{"line_number":2379,"context_line":"                # point in the migration.  We swallow this exception until a"},{"line_number":2380,"context_line":"                # more intensive refactor can be done to correct this."},{"line_number":2381,"context_line":"                if (\"VDI_NOT_IN_MAP\" in reason and"},{"line_number":2382,"context_line":"                        host_sw[\u0027platform_name\u0027] \u003d\u003d \"XCP\" and"}],"source_content_type":"text/x-python","patch_set":5,"id":"df7087c5_074cb4c5","line":2379,"range":{"start_line":2379,"start_character":41,"end_line":2379,"end_character":43},"in_reply_to":"df7087c5_d89a246d","updated":"2018-03-07 14:30:31.000000000","message":"Done","commit_id":"86e6892fe822dd8ea755b24d418e10216e5b8446"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d01253eaa38e5abccf6c737a21c6bbf2435a8b2d","unresolved":false,"context_lines":[{"line_number":2242,"context_line":"        # Works around aggregate checking as not all places use aggregates."},{"line_number":2243,"context_line":"        host_ref \u003d self._get_host_ref_no_aggr()"},{"line_number":2244,"context_line":"        host_rec \u003d self._session.call_xenapi(\"host.get_record\", host_ref)"},{"line_number":2245,"context_line":"        return host_rec[\u0027software_version\u0027]"},{"line_number":2246,"context_line":""},{"line_number":2247,"context_line":"    def _get_network_ref(self):"},{"line_number":2248,"context_line":"        # Get the network to for migrate."}],"source_content_type":"text/x-python","patch_set":6,"id":"df7087c5_4b3b389b","line":2245,"updated":"2018-03-12 12:31:41.000000000","message":"This was added in XenServer 4.0, so that seems OK:\nhttp://xapi-project.github.io/xen-api/classes/host.html","commit_id":"0e9cd6c4d66ca4afb95bb60edb412af9e96c546e"}]}