)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0ce7e614437fc37aafe31173e9e4fa82057dd731","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Libvirt: Move graphics checking and console verify early when LM"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch moves the follows migration data seting to"},{"line_number":10,"context_line":"check_can_live_migrate_destination:"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"    graphics_listen_addr_vnc \u003d CONF.vnc.vncserver_listen"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"da6ed579_d84ac654","line":9,"range":{"start_line":9,"start_character":21,"end_line":9,"end_character":28},"updated":"2016-01-15 07:58:17.000000000","message":"nit: s/follows/following/","commit_id":"cbbd9c85954bdb46dcaabddcceeb3ee52872626f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0ce7e614437fc37aafe31173e9e4fa82057dd731","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Libvirt: Move graphics checking and console verify early when LM"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch moves the follows migration data seting to"},{"line_number":10,"context_line":"check_can_live_migrate_destination:"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"    graphics_listen_addr_vnc \u003d CONF.vnc.vncserver_listen"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"da6ed579_b87a7286","line":9,"range":{"start_line":9,"start_character":44,"end_line":9,"end_character":50},"updated":"2016-01-15 07:58:17.000000000","message":"nit: s/seting/settings/","commit_id":"cbbd9c85954bdb46dcaabddcceeb3ee52872626f"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"5afdddf145d8f61814463cd02029dbfc47ddc835","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Libvirt: Move graphics checking and console verify early when LM"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch moves the follows migration data seting to"},{"line_number":10,"context_line":"check_can_live_migrate_destination:"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"    graphics_listen_addr_vnc \u003d CONF.vnc.vncserver_listen"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"da6ed579_5b59f08c","line":9,"range":{"start_line":9,"start_character":44,"end_line":9,"end_character":50},"in_reply_to":"da6ed579_b87a7286","updated":"2016-01-19 07:38:58.000000000","message":"Done","commit_id":"cbbd9c85954bdb46dcaabddcceeb3ee52872626f"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"5afdddf145d8f61814463cd02029dbfc47ddc835","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Libvirt: Move graphics checking and console verify early when LM"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch moves the follows migration data seting to"},{"line_number":10,"context_line":"check_can_live_migrate_destination:"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"    graphics_listen_addr_vnc \u003d CONF.vnc.vncserver_listen"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"da6ed579_fb71c40c","line":9,"range":{"start_line":9,"start_character":21,"end_line":9,"end_character":28},"in_reply_to":"da6ed579_d84ac654","updated":"2016-01-19 07:38:58.000000000","message":"Done","commit_id":"cbbd9c85954bdb46dcaabddcceeb3ee52872626f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0ce7e614437fc37aafe31173e9e4fa82057dd731","unresolved":false,"context_lines":[{"line_number":13,"context_line":"    graphics_listen_addr_spice \u003d CONF.spice.server_listen"},{"line_number":14,"context_line":"    serial_listen_addr \u003d CONF.serial_console.proxyclient_address"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"By doing this we can move the following checking early to before we really"},{"line_number":17,"context_line":"doing live_migration operations, it will give admin a useful/meanful messages:"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"    _check_graphics_addresses_can_live_migrate()"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"da6ed579_186cdecc","line":16,"range":{"start_line":16,"start_character":40,"end_line":16,"end_character":48},"updated":"2016-01-15 07:58:17.000000000","message":"nit: s/checking/checks/","commit_id":"cbbd9c85954bdb46dcaabddcceeb3ee52872626f"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"5afdddf145d8f61814463cd02029dbfc47ddc835","unresolved":false,"context_lines":[{"line_number":13,"context_line":"    graphics_listen_addr_spice \u003d CONF.spice.server_listen"},{"line_number":14,"context_line":"    serial_listen_addr \u003d CONF.serial_console.proxyclient_address"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"By doing this we can move the following checking early to before we really"},{"line_number":17,"context_line":"doing live_migration operations, it will give admin a useful/meanful messages:"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"    _check_graphics_addresses_can_live_migrate()"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"da6ed579_3b53dc6c","line":16,"range":{"start_line":16,"start_character":40,"end_line":16,"end_character":48},"in_reply_to":"da6ed579_186cdecc","updated":"2016-01-19 07:38:58.000000000","message":"Done","commit_id":"cbbd9c85954bdb46dcaabddcceeb3ee52872626f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0ce7e614437fc37aafe31173e9e4fa82057dd731","unresolved":false,"context_lines":[{"line_number":14,"context_line":"    serial_listen_addr \u003d CONF.serial_console.proxyclient_address"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"By doing this we can move the following checking early to before we really"},{"line_number":17,"context_line":"doing live_migration operations, it will give admin a useful/meanful messages:"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"    _check_graphics_addresses_can_live_migrate()"},{"line_number":20,"context_line":"    _verify_serial_console_is_disabled()"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"da6ed579_98852e5b","line":17,"range":{"start_line":17,"start_character":61,"end_line":17,"end_character":68},"updated":"2016-01-15 07:58:17.000000000","message":"nit: s/meanful/meaningful/","commit_id":"cbbd9c85954bdb46dcaabddcceeb3ee52872626f"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"5afdddf145d8f61814463cd02029dbfc47ddc835","unresolved":false,"context_lines":[{"line_number":14,"context_line":"    serial_listen_addr \u003d CONF.serial_console.proxyclient_address"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"By doing this we can move the following checking early to before we really"},{"line_number":17,"context_line":"doing live_migration operations, it will give admin a useful/meanful messages:"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"    _check_graphics_addresses_can_live_migrate()"},{"line_number":20,"context_line":"    _verify_serial_console_is_disabled()"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"da6ed579_5bd010c9","line":17,"range":{"start_line":17,"start_character":61,"end_line":17,"end_character":68},"in_reply_to":"da6ed579_98852e5b","updated":"2016-01-19 07:38:58.000000000","message":"Done","commit_id":"cbbd9c85954bdb46dcaabddcceeb3ee52872626f"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"0ce7e614437fc37aafe31173e9e4fa82057dd731","unresolved":false,"context_lines":[{"line_number":6300,"context_line":"                          self.context, instance, dest_check_data,"},{"line_number":6301,"context_line":"                          block_device_info\u003dbdi)"},{"line_number":6302,"context_line":""},{"line_number":6303,"context_line":"    @mock.patch.object(fakelibvirt, \u0027VIR_DOMAIN_XML_MIGRATABLE\u0027, None,"},{"line_number":6304,"context_line":"                       create\u003dTrue)"},{"line_number":6305,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.LibvirtDriver.\u0027"},{"line_number":6306,"context_line":"                \u0027_check_graphics_addresses_can_live_migrate\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"da6ed579_18fdbeb3","line":6303,"updated":"2016-01-15 07:58:17.000000000","message":"Mocking parts of a fake feels odd. Can we simply adapt our fake to be useful for testing?","commit_id":"cbbd9c85954bdb46dcaabddcceeb3ee52872626f"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"3e3f42b8b06122d4be50ae83f56c22f998916e29","unresolved":false,"context_lines":[{"line_number":6300,"context_line":"                          self.context, instance, dest_check_data,"},{"line_number":6301,"context_line":"                          block_device_info\u003dbdi)"},{"line_number":6302,"context_line":""},{"line_number":6303,"context_line":"    @mock.patch.object(fakelibvirt, \u0027VIR_DOMAIN_XML_MIGRATABLE\u0027, None,"},{"line_number":6304,"context_line":"                       create\u003dTrue)"},{"line_number":6305,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.LibvirtDriver.\u0027"},{"line_number":6306,"context_line":"                \u0027_check_graphics_addresses_can_live_migrate\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba63e191_2444d672","line":6303,"in_reply_to":"da6ed579_18fdbeb3","updated":"2016-01-19 07:28:58.000000000","message":"Other tests do this already. See e.g. test_live_migration_fails_with_serial_console_without_migratable()\n\nI don\u0027t immediately see a better way of triggering these code paths that are there to deal with older versions of libvirt.","commit_id":"cbbd9c85954bdb46dcaabddcceeb3ee52872626f"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"5afdddf145d8f61814463cd02029dbfc47ddc835","unresolved":false,"context_lines":[{"line_number":6300,"context_line":"                          self.context, instance, dest_check_data,"},{"line_number":6301,"context_line":"                          block_device_info\u003dbdi)"},{"line_number":6302,"context_line":""},{"line_number":6303,"context_line":"    @mock.patch.object(fakelibvirt, \u0027VIR_DOMAIN_XML_MIGRATABLE\u0027, None,"},{"line_number":6304,"context_line":"                       create\u003dTrue)"},{"line_number":6305,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.LibvirtDriver.\u0027"},{"line_number":6306,"context_line":"                \u0027_check_graphics_addresses_can_live_migrate\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"da6ed579_7b0e348d","line":6303,"in_reply_to":"da6ed579_18fdbeb3","updated":"2016-01-19 07:38:58.000000000","message":"hi Balazs, I want to mock VIR_DOMAIN_XML_MIGRATABLE\u003dNone\nthis is an example.\nhttps://github.com/openstack/nova/blob/master/nova/tests/unit/virt/libvirt/test_driver.py#L6761\n\nif you feels odd, can you please give some suggestion on how to adapt our fakelibvirt driver ?","commit_id":"cbbd9c85954bdb46dcaabddcceeb3ee52872626f"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c44d0433aed939b64b525980210308873701ef78","unresolved":false,"context_lines":[{"line_number":6300,"context_line":"                          self.context, instance, dest_check_data,"},{"line_number":6301,"context_line":"                          block_device_info\u003dbdi)"},{"line_number":6302,"context_line":""},{"line_number":6303,"context_line":"    @mock.patch.object(fakelibvirt, \u0027VIR_DOMAIN_XML_MIGRATABLE\u0027, None,"},{"line_number":6304,"context_line":"                       create\u003dTrue)"},{"line_number":6305,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.LibvirtDriver.\u0027"},{"line_number":6306,"context_line":"                \u0027_check_graphics_addresses_can_live_migrate\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba63e191_8ead988a","line":6303,"in_reply_to":"da6ed579_7b0e348d","updated":"2016-01-19 10:13:19.000000000","message":"Let me be more specific. Your tests need a fake. That is fine, you create a fake (fakelibvirt) and use it to simulate the behavior of libvirt. Then this specific test needs that this fakelibvirt behave differently. As the fakelibvirt is already a code created for the test it would make sense to me to make that fake adaptable for your test need. There is such an option already by create_fake_libvirt_mock() call, and this is used by some test already in this file.\n\nI feel mock shall only be used to mock out parts of the code under test, fakelibvirt is not the code under test but part of the test framework. \n\nAlso mentally a bit hard to follow that your test create a LibvirtDriver object and then mock the fakelibvirt to modify that LibvirtDriver. So it is already a bit nicer to replace this line:\n\n@mock.patch.object(fakelibvirt,\u0027VIR_DOMAIN_XML_MIGRATABLE\u0027, None,create\u003dTrue)\n\nwith this:\n\n@mock.patch(\u0027nova.virt.libvirt.driver.libvirt.VIR_DOMAIN_XML_MIGRATABLE\u0027, None)\n\nas it makes the connection more explicit between your mocking and your test code. \n\nLooking at this test file I see that this problem is not specific to your new test case so I guess you just followed the existing pattern here. Hence I removed my -1.","commit_id":"cbbd9c85954bdb46dcaabddcceeb3ee52872626f"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"5447e6461866f748200d6acd5d55d3c065a643ac","unresolved":false,"context_lines":[{"line_number":6354,"context_line":"                \u0027_check_graphics_addresses_can_live_migrate\u0027)"},{"line_number":6355,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.LibvirtDriver.\u0027"},{"line_number":6356,"context_line":"                \u0027_verify_serial_console_is_disabled\u0027,"},{"line_number":6357,"context_line":"                side_effect\u003dexception.MigrationError(reason\u003d\u0027\u0027))"},{"line_number":6358,"context_line":"    def test_check_can_live_migrate_source_block_migration_console_disable("},{"line_number":6359,"context_line":"            self, mock_console, mock_check_graphics):"},{"line_number":6360,"context_line":"        instance \u003d objects.Instance(**self.test_instance)"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba63e191_a4776601","line":6357,"updated":"2016-01-19 07:26:44.000000000","message":"Hmm, why do this? Why not just set serial_console.enabled to trigger the exception?","commit_id":"ec939728d93c464ce5dc956d3426588419cad2f9"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"614e6de4236336704f66f08d98fada53fdbb93ee","unresolved":false,"context_lines":[{"line_number":6354,"context_line":"                \u0027_check_graphics_addresses_can_live_migrate\u0027)"},{"line_number":6355,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.LibvirtDriver.\u0027"},{"line_number":6356,"context_line":"                \u0027_verify_serial_console_is_disabled\u0027,"},{"line_number":6357,"context_line":"                side_effect\u003dexception.MigrationError(reason\u003d\u0027\u0027))"},{"line_number":6358,"context_line":"    def test_check_can_live_migrate_source_block_migration_console_disable("},{"line_number":6359,"context_line":"            self, mock_console, mock_check_graphics):"},{"line_number":6360,"context_line":"        instance \u003d objects.Instance(**self.test_instance)"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba63e191_2d6ef9c9","line":6357,"in_reply_to":"ba63e191_a4776601","updated":"2016-01-19 08:21:25.000000000","message":"Done","commit_id":"ec939728d93c464ce5dc956d3426588419cad2f9"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"5447e6461866f748200d6acd5d55d3c065a643ac","unresolved":false,"context_lines":[{"line_number":6358,"context_line":"    def test_check_can_live_migrate_source_block_migration_console_disable("},{"line_number":6359,"context_line":"            self, mock_console, mock_check_graphics):"},{"line_number":6360,"context_line":"        instance \u003d objects.Instance(**self.test_instance)"},{"line_number":6361,"context_line":"        fakelibvirt.VIR_DOMAIN_XML_MIGRATABLE \u003d None"},{"line_number":6362,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":6363,"context_line":"        dest_check_data \u003d objects.LibvirtLiveMigrateData("},{"line_number":6364,"context_line":"            filename\u003d\u0027file\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba63e191_445f8a72","line":6361,"updated":"2016-01-19 07:26:44.000000000","message":"This needs to be cleaned up after the test. Other tests do:\n\n    @mock.patch.object(fakelibvirt, \u0027VIR_DOMAIN_XML_MIGRATABLE\u0027, None,\n                       create\u003dTrue)","commit_id":"ec939728d93c464ce5dc956d3426588419cad2f9"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"614e6de4236336704f66f08d98fada53fdbb93ee","unresolved":false,"context_lines":[{"line_number":6358,"context_line":"    def test_check_can_live_migrate_source_block_migration_console_disable("},{"line_number":6359,"context_line":"            self, mock_console, mock_check_graphics):"},{"line_number":6360,"context_line":"        instance \u003d objects.Instance(**self.test_instance)"},{"line_number":6361,"context_line":"        fakelibvirt.VIR_DOMAIN_XML_MIGRATABLE \u003d None"},{"line_number":6362,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":6363,"context_line":"        dest_check_data \u003d objects.LibvirtLiveMigrateData("},{"line_number":6364,"context_line":"            filename\u003d\u0027file\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba63e191_27e7f8e5","line":6361,"in_reply_to":"ba63e191_445f8a72","updated":"2016-01-19 08:21:25.000000000","message":"yeah, I was before, but Balazs -1 for using mock on a fake obj.","commit_id":"ec939728d93c464ce5dc956d3426588419cad2f9"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"5447e6461866f748200d6acd5d55d3c065a643ac","unresolved":false,"context_lines":[{"line_number":6379,"context_line":"            self):"},{"line_number":6380,"context_line":"        self.flags(vncserver_listen\u003d\u0027bad\u0027, enabled\u003dTrue, group\u003d\u0027vnc\u0027)"},{"line_number":6381,"context_line":"        instance \u003d objects.Instance(**self.test_instance)"},{"line_number":6382,"context_line":"        fakelibvirt.VIR_DOMAIN_XML_MIGRATABLE \u003d None"},{"line_number":6383,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":6384,"context_line":"        dest_check_data \u003d objects.LibvirtLiveMigrateData("},{"line_number":6385,"context_line":"            filename\u003d\u0027file\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba63e191_4488eae7","line":6382,"updated":"2016-01-19 07:26:44.000000000","message":"Same.","commit_id":"ec939728d93c464ce5dc956d3426588419cad2f9"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"2527eeb79d93c0bed4201a8d9f71832487209d3b","unresolved":false,"context_lines":[{"line_number":6355,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.LibvirtDriver.\u0027"},{"line_number":6356,"context_line":"                \u0027_check_graphics_addresses_can_live_migrate\u0027)"},{"line_number":6357,"context_line":"    def test_check_can_live_migrate_source_block_migration_console_disable("},{"line_number":6358,"context_line":"            self, mock_console):"},{"line_number":6359,"context_line":"        CONF.set_override(\"enabled\", True, \"serial_console\")"},{"line_number":6360,"context_line":"        instance \u003d objects.Instance(**self.test_instance)"},{"line_number":6361,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba63e191_a1ef4daf","line":6358,"updated":"2016-01-19 10:36:53.000000000","message":"You probably meant to name this mock_check_graphics, but it\u0027s unused ... hardly worth fixing","commit_id":"573458a278c7ee40b5eacb6cc0a8163d99204dc4"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"6667425423b04f49f13525ac5738551c3a7d0d5f","unresolved":false,"context_lines":[{"line_number":6355,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.driver.LibvirtDriver.\u0027"},{"line_number":6356,"context_line":"                \u0027_check_graphics_addresses_can_live_migrate\u0027)"},{"line_number":6357,"context_line":"    def test_check_can_live_migrate_source_block_migration_console_disable("},{"line_number":6358,"context_line":"            self, mock_console):"},{"line_number":6359,"context_line":"        CONF.set_override(\"enabled\", True, \"serial_console\")"},{"line_number":6360,"context_line":"        instance \u003d objects.Instance(**self.test_instance)"},{"line_number":6361,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba63e191_e451d363","line":6358,"in_reply_to":"ba63e191_a1ef4daf","updated":"2016-01-19 11:41:15.000000000","message":"ya, thanks Mark the mock sequence is reverted. I will try to fix it if another version is needed.\n\nThanks.\nEli.","commit_id":"573458a278c7ee40b5eacb6cc0a8163d99204dc4"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"6f0ba54fe22cf20feb967f0d766fb2b0af16ada5","unresolved":false,"context_lines":[{"line_number":5650,"context_line":"                                      None)"},{"line_number":5651,"context_line":""},{"line_number":5652,"context_line":"            if (migratable_flag is None or"},{"line_number":5653,"context_line":"                    (listen_addrs is None and not volume)):"},{"line_number":5654,"context_line":"                # TODO(alexs-h): These checks could be moved to the"},{"line_number":5655,"context_line":"                # check_can_live_migrate_destination/source phase"},{"line_number":5656,"context_line":"                self._check_graphics_addresses_can_live_migrate(listen_addrs)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa69d971_a847f375","side":"PARENT","line":5653,"updated":"2016-01-08 15:08:45.000000000","message":"Note the backwards compat case that handling listen_addrs \u003d\u003d None is support to deal with ...\n\nIf the destination is an older compute which doesn\u0027t have the code for collecting the destination host\u0027s listen addresses, the effect is the same as if we don\u0027t have VIR_DOMAIN_XML_MIGRATABLE support","commit_id":"c960334421fc7817ba54da817540ca3bade5546f"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"6f0ba54fe22cf20feb967f0d766fb2b0af16ada5","unresolved":false,"context_lines":[{"line_number":5654,"context_line":"                # TODO(alexs-h): These checks could be moved to the"},{"line_number":5655,"context_line":"                # check_can_live_migrate_destination/source phase"},{"line_number":5656,"context_line":"                self._check_graphics_addresses_can_live_migrate(listen_addrs)"},{"line_number":5657,"context_line":"                self._verify_serial_console_is_disabled()"},{"line_number":5658,"context_line":"                dom.migrateToURI(CONF.libvirt.live_migration_uri % dest,"},{"line_number":5659,"context_line":"                                 logical_sum,"},{"line_number":5660,"context_line":"                                 None,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa69d971_0ef88f48","side":"PARENT","line":5657,"updated":"2016-01-08 15:08:45.000000000","message":"Note from I6c9500283027193179c11d18ad4242e165a9ad6e that the intent here is that these calls are only for when VIR_DOMAIN_XML_MIGRATABLE is not available\n\nYou\u0027re calling it in all cases now?","commit_id":"c960334421fc7817ba54da817540ca3bade5546f"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"6f0ba54fe22cf20feb967f0d766fb2b0af16ada5","unresolved":false,"context_lines":[{"line_number":6312,"context_line":"        migrate_data.graphics_listen_addr_spice \u003d CONF.spice.server_listen"},{"line_number":6313,"context_line":"        migrate_data.serial_listen_addr \u003d \\"},{"line_number":6314,"context_line":"            CONF.serial_console.proxyclient_address"},{"line_number":6315,"context_line":""},{"line_number":6316,"context_line":"        for vol in block_device_mapping:"},{"line_number":6317,"context_line":"            connection_info \u003d vol[\u0027connection_info\u0027]"},{"line_number":6318,"context_line":"            if connection_info.get(\u0027serial\u0027):"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa69d971_b181a9fd","side":"PARENT","line":6315,"updated":"2016-01-08 15:08:45.000000000","message":"This is in pre_live_migration() which is executed on the destination host. And the _check_graphics_addresses_can_live_migrate() (which runs on the source) is expecting values from the destination.","commit_id":"c960334421fc7817ba54da817540ca3bade5546f"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"6bd06d2a919bc204b2bb80befa1304beca80bdf8","unresolved":false,"context_lines":[{"line_number":6312,"context_line":"        migrate_data.graphics_listen_addr_spice \u003d CONF.spice.server_listen"},{"line_number":6313,"context_line":"        migrate_data.serial_listen_addr \u003d \\"},{"line_number":6314,"context_line":"            CONF.serial_console.proxyclient_address"},{"line_number":6315,"context_line":""},{"line_number":6316,"context_line":"        for vol in block_device_mapping:"},{"line_number":6317,"context_line":"            connection_info \u003d vol[\u0027connection_info\u0027]"},{"line_number":6318,"context_line":"            if connection_info.get(\u0027serial\u0027):"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa69d971_f1528cdd","side":"PARENT","line":6315,"in_reply_to":"fa69d971_b181a9fd","updated":"2016-01-11 03:47:54.000000000","message":"these data settings are move to check_can_live_migrate_destination() which is running on destination.","commit_id":"c960334421fc7817ba54da817540ca3bade5546f"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"6f0ba54fe22cf20feb967f0d766fb2b0af16ada5","unresolved":false,"context_lines":[{"line_number":5166,"context_line":"        data.disk_available_mb \u003d disk_available_mb"},{"line_number":5167,"context_line":"        data.graphics_listen_addr_vnc \u003d CONF.vnc.vncserver_listen"},{"line_number":5168,"context_line":"        data.graphics_listen_addr_spice \u003d CONF.spice.server_listen"},{"line_number":5169,"context_line":"        data.serial_listen_addr \u003d CONF.serial_console.proxyclient_address"},{"line_number":5170,"context_line":"        return data"},{"line_number":5171,"context_line":""},{"line_number":5172,"context_line":"    def check_can_live_migrate_destination_cleanup(self, context,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa69d971_b1af697c","line":5169,"updated":"2016-01-08 15:08:45.000000000","message":"This is running on the destination host, and _check_graphics_addresses_can_live_migrate() expects values from the destination host. Good.","commit_id":"b5c888a8b7605eedf3a499194f3ebbcedddb5b8a"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"6f0ba54fe22cf20feb967f0d766fb2b0af16ada5","unresolved":false,"context_lines":[{"line_number":5197,"context_line":"        source \u003d CONF.host"},{"line_number":5198,"context_line":""},{"line_number":5199,"context_line":"        if not isinstance(dest_check_data, migrate_data_obj.LiveMigrateData):"},{"line_number":5200,"context_line":"            listen_addrs \u003d dest_check_data.get(\u0027graphics_listen_addrs\u0027)"},{"line_number":5201,"context_line":"            md_obj \u003d migrate_data_obj.LibvirtLiveMigrateData()"},{"line_number":5202,"context_line":"            md_obj.from_legacy_dict(dest_check_data)"},{"line_number":5203,"context_line":"            dest_check_data \u003d md_obj"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa69d971_ac562e04","line":5200,"updated":"2016-01-08 15:08:45.000000000","message":"Hmm, isn\u0027t graphics_listen_addrs under pre_live_migration_result?\n\nI think I\u0027d feel more confident about this legacy code path if we deleted this line and ...","commit_id":"b5c888a8b7605eedf3a499194f3ebbcedddb5b8a"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"6bd06d2a919bc204b2bb80befa1304beca80bdf8","unresolved":false,"context_lines":[{"line_number":5197,"context_line":"        source \u003d CONF.host"},{"line_number":5198,"context_line":""},{"line_number":5199,"context_line":"        if not isinstance(dest_check_data, migrate_data_obj.LiveMigrateData):"},{"line_number":5200,"context_line":"            listen_addrs \u003d dest_check_data.get(\u0027graphics_listen_addrs\u0027)"},{"line_number":5201,"context_line":"            md_obj \u003d migrate_data_obj.LibvirtLiveMigrateData()"},{"line_number":5202,"context_line":"            md_obj.from_legacy_dict(dest_check_data)"},{"line_number":5203,"context_line":"            dest_check_data \u003d md_obj"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa69d971_d1950833","line":5200,"in_reply_to":"fa69d971_ac562e04","updated":"2016-01-11 03:47:54.000000000","message":"hi Mark, thanks, you are right,  if dest_check_data is an old type (dict), we don\u0027t have pre_live_migration_result in older version compute node.\n\nwhat should we do? Seems we can not remove all checking in  _live_migration_operation for backwards compatible.","commit_id":"b5c888a8b7605eedf3a499194f3ebbcedddb5b8a"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"6f0ba54fe22cf20feb967f0d766fb2b0af16ada5","unresolved":false,"context_lines":[{"line_number":5201,"context_line":"            md_obj \u003d migrate_data_obj.LibvirtLiveMigrateData()"},{"line_number":5202,"context_line":"            md_obj.from_legacy_dict(dest_check_data)"},{"line_number":5203,"context_line":"            dest_check_data \u003d md_obj"},{"line_number":5204,"context_line":"        else:"},{"line_number":5205,"context_line":"            listen_addrs \u003d {"},{"line_number":5206,"context_line":"                \u0027vnc\u0027: dest_check_data.graphics_listen_addr_vnc,"},{"line_number":5207,"context_line":"                \u0027spice\u0027: dest_check_data.graphics_listen_addr_spice}"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa69d971_aca4ee07","line":5204,"updated":"2016-01-08 15:08:45.000000000","message":"... remove the else\n\nThat way the from_legacy_dict() is the only code that\u0027s dealing with the details of the dict format","commit_id":"b5c888a8b7605eedf3a499194f3ebbcedddb5b8a"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"6bd06d2a919bc204b2bb80befa1304beca80bdf8","unresolved":false,"context_lines":[{"line_number":5201,"context_line":"            md_obj \u003d migrate_data_obj.LibvirtLiveMigrateData()"},{"line_number":5202,"context_line":"            md_obj.from_legacy_dict(dest_check_data)"},{"line_number":5203,"context_line":"            dest_check_data \u003d md_obj"},{"line_number":5204,"context_line":"        else:"},{"line_number":5205,"context_line":"            listen_addrs \u003d {"},{"line_number":5206,"context_line":"                \u0027vnc\u0027: dest_check_data.graphics_listen_addr_vnc,"},{"line_number":5207,"context_line":"                \u0027spice\u0027: dest_check_data.graphics_listen_addr_spice}"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa69d971_d10ae889","line":5204,"in_reply_to":"fa69d971_aca4ee07","updated":"2016-01-11 03:47:54.000000000","message":"yeah, will think it more.","commit_id":"b5c888a8b7605eedf3a499194f3ebbcedddb5b8a"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"6f0ba54fe22cf20feb967f0d766fb2b0af16ada5","unresolved":false,"context_lines":[{"line_number":5205,"context_line":"            listen_addrs \u003d {"},{"line_number":5206,"context_line":"                \u0027vnc\u0027: dest_check_data.graphics_listen_addr_vnc,"},{"line_number":5207,"context_line":"                \u0027spice\u0027: dest_check_data.graphics_listen_addr_spice}"},{"line_number":5208,"context_line":""},{"line_number":5209,"context_line":"        try:"},{"line_number":5210,"context_line":"            self._check_graphics_addresses_can_live_migrate(listen_addrs)"},{"line_number":5211,"context_line":"            self._verify_serial_console_is_disabled()"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa69d971_23ffa4ab","line":5208,"updated":"2016-01-08 15:08:45.000000000","message":"Note also we have a new backwards compat issue - if the destination host is older, these values won\u0027t be set so the checks can\u0027t test them\n\nUgh, that implies we should ...\n\nDo the checks in can_live_migrate_source() if there\u0027s no VIR_DOMAIN_XML_MIGRATABLE and if the listen addresses are set\n\nDo the checks in live_migration_operation() if we haven\u0027t already run the checks, and VIR_DOMAIN_XML_MIGRATABLE isn\u0027t available, and the listen addresses are set (can be removed when we no longer need to support the old nova-compute version)\n\nDo the checks in the failure case of live_migration_operations() if the listen addresses are set (can be removed when we don\u0027t need to support that buggy libvirt version)","commit_id":"b5c888a8b7605eedf3a499194f3ebbcedddb5b8a"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"6bd06d2a919bc204b2bb80befa1304beca80bdf8","unresolved":false,"context_lines":[{"line_number":5205,"context_line":"            listen_addrs \u003d {"},{"line_number":5206,"context_line":"                \u0027vnc\u0027: dest_check_data.graphics_listen_addr_vnc,"},{"line_number":5207,"context_line":"                \u0027spice\u0027: dest_check_data.graphics_listen_addr_spice}"},{"line_number":5208,"context_line":""},{"line_number":5209,"context_line":"        try:"},{"line_number":5210,"context_line":"            self._check_graphics_addresses_can_live_migrate(listen_addrs)"},{"line_number":5211,"context_line":"            self._verify_serial_console_is_disabled()"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa69d971_7cf513eb","line":5208,"in_reply_to":"fa69d971_23ffa4ab","updated":"2016-01-11 03:47:54.000000000","message":"okay, I will rethink more.","commit_id":"b5c888a8b7605eedf3a499194f3ebbcedddb5b8a"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"6f0ba54fe22cf20feb967f0d766fb2b0af16ada5","unresolved":false,"context_lines":[{"line_number":5208,"context_line":""},{"line_number":5209,"context_line":"        try:"},{"line_number":5210,"context_line":"            self._check_graphics_addresses_can_live_migrate(listen_addrs)"},{"line_number":5211,"context_line":"            self._verify_serial_console_is_disabled()"},{"line_number":5212,"context_line":"        except exception.MigrationError as e:"},{"line_number":5213,"context_line":"            raise exception.MigrationPreCheckError(reason\u003de.format_message())"},{"line_number":5214,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa69d971_c8ba67e1","line":5211,"updated":"2016-01-08 15:08:45.000000000","message":"You should only be doing these checks if libvirt doesn\u0027t have VIR_DOMAIN_XML_MIGRATABLE","commit_id":"b5c888a8b7605eedf3a499194f3ebbcedddb5b8a"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"6f0ba54fe22cf20feb967f0d766fb2b0af16ada5","unresolved":false,"context_lines":[{"line_number":5210,"context_line":"            self._check_graphics_addresses_can_live_migrate(listen_addrs)"},{"line_number":5211,"context_line":"            self._verify_serial_console_is_disabled()"},{"line_number":5212,"context_line":"        except exception.MigrationError as e:"},{"line_number":5213,"context_line":"            raise exception.MigrationPreCheckError(reason\u003de.format_message())"},{"line_number":5214,"context_line":""},{"line_number":5215,"context_line":"        dest_check_data.is_shared_instance_path \u003d ("},{"line_number":5216,"context_line":"            self._check_shared_storage_test_file("}],"source_content_type":"text/x-python","patch_set":2,"id":"fa69d971_28ff83ab","line":5213,"updated":"2016-01-08 15:08:45.000000000","message":"Probably best to change these checks to raise MigrationPreCheckError rather than MigrationError","commit_id":"b5c888a8b7605eedf3a499194f3ebbcedddb5b8a"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"6bd06d2a919bc204b2bb80befa1304beca80bdf8","unresolved":false,"context_lines":[{"line_number":5210,"context_line":"            self._check_graphics_addresses_can_live_migrate(listen_addrs)"},{"line_number":5211,"context_line":"            self._verify_serial_console_is_disabled()"},{"line_number":5212,"context_line":"        except exception.MigrationError as e:"},{"line_number":5213,"context_line":"            raise exception.MigrationPreCheckError(reason\u003de.format_message())"},{"line_number":5214,"context_line":""},{"line_number":5215,"context_line":"        dest_check_data.is_shared_instance_path \u003d ("},{"line_number":5216,"context_line":"            self._check_shared_storage_test_file("}],"source_content_type":"text/x-python","patch_set":2,"id":"fa69d971_316fc42c","line":5213,"in_reply_to":"fa69d971_28ff83ab","updated":"2016-01-11 03:47:54.000000000","message":"Probably we can not do it now, these checkes are all raising MigrationError since they are called in _live_migration_operation , we still need to reuse them when we do \u0027live migration\u0027, so it is not accurate\n to raising MigrationPreCheckError.\n\nBut we can do a cleanup litter if all checking are not needed in _live_migration_operation.","commit_id":"b5c888a8b7605eedf3a499194f3ebbcedddb5b8a"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"6f0ba54fe22cf20feb967f0d766fb2b0af16ada5","unresolved":false,"context_lines":[{"line_number":5702,"context_line":"                                 instance\u003dinstance)"},{"line_number":5703,"context_line":"                        self._check_graphics_addresses_can_live_migrate("},{"line_number":5704,"context_line":"                            listen_addrs)"},{"line_number":5705,"context_line":"                        self._verify_serial_console_is_disabled()"},{"line_number":5706,"context_line":"                        dom.migrateToURI("},{"line_number":5707,"context_line":"                            CONF.libvirt.live_migration_uri % dest,"},{"line_number":5708,"context_line":"                            logical_sum,"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa69d971_28d783ee","line":5705,"updated":"2016-01-08 15:08:45.000000000","message":"Keeping the checks here sort of defeats the purpose of moving them earlier, but it\u0027s going to be hard to handle this case as a pre-check\n\nWe should probably make a concerted effort to figure out whether we still need this fallback at all. Is that buggy version of libvirt something we still need to worry about? Should be the subject of a different patch, though.","commit_id":"b5c888a8b7605eedf3a499194f3ebbcedddb5b8a"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"5447e6461866f748200d6acd5d55d3c065a643ac","unresolved":false,"context_lines":[{"line_number":5321,"context_line":"            dest_check_data \u003d md_obj"},{"line_number":5322,"context_line":""},{"line_number":5323,"context_line":"        if (\u0027graphics_listen_addr_vnc\u0027 in dest_check_data or"},{"line_number":5324,"context_line":"            \u0027graphics_listen_addr_spice\u0027 in dest_check_data):"},{"line_number":5325,"context_line":"            listen_addrs \u003d {"},{"line_number":5326,"context_line":"                \u0027vnc\u0027: dest_check_data.graphics_listen_addr_vnc,"},{"line_number":5327,"context_line":"                \u0027spice\u0027: dest_check_data.graphics_listen_addr_spice}"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba63e191_a6776dd3","line":5324,"updated":"2016-01-19 07:26:44.000000000","message":"I wasn\u0027t sure this was good practice - treating a LibvirtLiveMigrateData object as a collection ... but I see that VersionedObject does implement __contains__() so I guess it makes sense.\n\nI might have added a comment that these checks are only here to handle older versions on the destination host - they can be removed later.","commit_id":"ec939728d93c464ce5dc956d3426588419cad2f9"},{"author":{"_account_id":1247,"name":"Mark McLoughlin","email":"markmc@redhat.com","username":"markmc"},"change_message_id":"5447e6461866f748200d6acd5d55d3c065a643ac","unresolved":false,"context_lines":[{"line_number":6452,"context_line":"        migrate_data.graphics_listen_addr_vnc \u003d CONF.vnc.vncserver_listen"},{"line_number":6453,"context_line":"        migrate_data.graphics_listen_addr_spice \u003d CONF.spice.server_listen"},{"line_number":6454,"context_line":"        migrate_data.serial_listen_addr \u003d \\"},{"line_number":6455,"context_line":"            CONF.serial_console.proxyclient_address"},{"line_number":6456,"context_line":""},{"line_number":6457,"context_line":"        for vol in block_device_mapping:"},{"line_number":6458,"context_line":"            connection_info \u003d vol[\u0027connection_info\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba63e191_04dbd27d","line":6455,"updated":"2016-01-19 07:26:44.000000000","message":"You originally had removed these from here. Why did you add them back? I\u0027m pretty sure they\u0027re guaranteed to be set already at this point?","commit_id":"ec939728d93c464ce5dc956d3426588419cad2f9"},{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"614e6de4236336704f66f08d98fada53fdbb93ee","unresolved":false,"context_lines":[{"line_number":6452,"context_line":"        migrate_data.graphics_listen_addr_vnc \u003d CONF.vnc.vncserver_listen"},{"line_number":6453,"context_line":"        migrate_data.graphics_listen_addr_spice \u003d CONF.spice.server_listen"},{"line_number":6454,"context_line":"        migrate_data.serial_listen_addr \u003d \\"},{"line_number":6455,"context_line":"            CONF.serial_console.proxyclient_address"},{"line_number":6456,"context_line":""},{"line_number":6457,"context_line":"        for vol in block_device_mapping:"},{"line_number":6458,"context_line":"            connection_info \u003d vol[\u0027connection_info\u0027]"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba63e191_c7ee1cec","line":6455,"in_reply_to":"ba63e191_04dbd27d","updated":"2016-01-19 08:21:25.000000000","message":"hi Mark, I am wrong here.\nGuess it\u0027s rebase stuff, or made by mistake, I will removed them","commit_id":"ec939728d93c464ce5dc956d3426588419cad2f9"}]}