)]}'
{"nova/console/serial.py":[{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"cac41d71b2abce4cc7898cb4acb3393b0bd89305","unresolved":false,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"# TODO(sahid): Add a method to initialize ALOCATED_PORTS with the"},{"line_number":59,"context_line":"# already binded TPC port(s). (cf from danpb: list all running guests and"},{"line_number":60,"context_line":"# query the XML in libvirt driver to find out the TCP port(s) it uses)."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"@utils.synchronized(SERIAL_LOCK)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa1b9901_6832fdf3","line":60,"updated":"2015-08-20 12:29:13.000000000","message":"Is this comment then still valid after this patch set would be merged?","commit_id":"09d2e421966987f7338fffb72c9748af7b977314"},{"author":{"_account_id":14594,"name":"Alexander Schmidt","email":"alexs@linux.vnet.ibm.com","username":"alexs"},"change_message_id":"b72c9095bead47aa507d0706eaf79c1ebff5d86a","unresolved":false,"context_lines":[{"line_number":57,"context_line":""},{"line_number":58,"context_line":"# TODO(sahid): Add a method to initialize ALOCATED_PORTS with the"},{"line_number":59,"context_line":"# already binded TPC port(s). (cf from danpb: list all running guests and"},{"line_number":60,"context_line":"# query the XML in libvirt driver to find out the TCP port(s) it uses)."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"@utils.synchronized(SERIAL_LOCK)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa1b9901_a3435641","line":60,"in_reply_to":"fa1b9901_6832fdf3","updated":"2015-08-20 12:53:00.000000000","message":"Yes that\u0027s still valid, I did not add a method that runs during the startup of the service to query the existing config. I did not see a firm requirement as _get_serial_ports_from_instance() queries the information from libvirt when it\u0027s needed.","commit_id":"09d2e421966987f7338fffb72c9748af7b977314"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"cac41d71b2abce4cc7898cb4acb3393b0bd89305","unresolved":false,"context_lines":[{"line_number":81,"context_line":"    entry[\"host\"] \u003d host"},{"line_number":82,"context_line":"    entry[\"uuid\"] \u003d instance.uuid"},{"line_number":83,"context_line":"    entry[\"console_port\"] \u003d console_port"},{"line_number":84,"context_line":"    entry[\"tcp_port\"] \u003d tcp_port"},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    PORTS_BY_INSTANCE.append(entry)"},{"line_number":87,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa1b9901_fd32515d","line":84,"updated":"2015-08-20 12:29:13.000000000","message":"Maybe a small class would make sense for that dict.","commit_id":"09d2e421966987f7338fffb72c9748af7b977314"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"cac41d71b2abce4cc7898cb4acb3393b0bd89305","unresolved":false,"context_lines":[{"line_number":110,"context_line":"    if entry[\"uuid\"] !\u003d instance.uuid:"},{"line_number":111,"context_line":"        return False"},{"line_number":112,"context_line":"    if entry[\"console_port\"] !\u003d console_port:"},{"line_number":113,"context_line":"        return False"},{"line_number":114,"context_line":"    return True"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa1b9901_3d62093f","line":113,"updated":"2015-08-20 12:29:13.000000000","message":"This is another way to say that \"host\", \"uuid\" and \"console_port\" must all be equal, right? Could be simplified IMO.","commit_id":"09d2e421966987f7338fffb72c9748af7b977314"},{"author":{"_account_id":14594,"name":"Alexander Schmidt","email":"alexs@linux.vnet.ibm.com","username":"alexs"},"change_message_id":"b72c9095bead47aa507d0706eaf79c1ebff5d86a","unresolved":false,"context_lines":[{"line_number":110,"context_line":"    if entry[\"uuid\"] !\u003d instance.uuid:"},{"line_number":111,"context_line":"        return False"},{"line_number":112,"context_line":"    if entry[\"console_port\"] !\u003d console_port:"},{"line_number":113,"context_line":"        return False"},{"line_number":114,"context_line":"    return True"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa1b9901_a39836c1","line":113,"in_reply_to":"fa1b9901_3d62093f","updated":"2015-08-20 12:53:00.000000000","message":"Yeah sounds like a good idea to me, especially with this method as a class member.","commit_id":"09d2e421966987f7338fffb72c9748af7b977314"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"cac41d71b2abce4cc7898cb4acb3393b0bd89305","unresolved":false,"context_lines":[{"line_number":111,"context_line":"        return False"},{"line_number":112,"context_line":"    if entry[\"console_port\"] !\u003d console_port:"},{"line_number":113,"context_line":"        return False"},{"line_number":114,"context_line":"    return True"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"@utils.synchronized(SERIAL_LOCK)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa1b9901_fd09f184","line":114,"updated":"2015-08-20 12:29:13.000000000","message":"If the \"entry\" dict would be a class, we could move it there.","commit_id":"09d2e421966987f7338fffb72c9748af7b977314"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"cac41d71b2abce4cc7898cb4acb3393b0bd89305","unresolved":false,"context_lines":[{"line_number":119,"context_line":"    \"\"\"Release TCP port to be used next time.\"\"\""},{"line_number":120,"context_line":"    for entry in list(PORTS_BY_INSTANCE):"},{"line_number":121,"context_line":"        if _matches(entry, host, instance, console_port):"},{"line_number":122,"context_line":"            PORTS_BY_INSTANCE.remove(entry)"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    ALLOCATED_PORTS.discard((host, port))"},{"line_number":125,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa1b9901_080ca139","line":122,"updated":"2015-08-20 12:29:13.000000000","message":"nit: Could be a list comprehension","commit_id":"09d2e421966987f7338fffb72c9748af7b977314"},{"author":{"_account_id":14594,"name":"Alexander Schmidt","email":"alexs@linux.vnet.ibm.com","username":"alexs"},"change_message_id":"b72c9095bead47aa507d0706eaf79c1ebff5d86a","unresolved":false,"context_lines":[{"line_number":119,"context_line":"    \"\"\"Release TCP port to be used next time.\"\"\""},{"line_number":120,"context_line":"    for entry in list(PORTS_BY_INSTANCE):"},{"line_number":121,"context_line":"        if _matches(entry, host, instance, console_port):"},{"line_number":122,"context_line":"            PORTS_BY_INSTANCE.remove(entry)"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    ALLOCATED_PORTS.discard((host, port))"},{"line_number":125,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa1b9901_2360a6a4","line":122,"in_reply_to":"fa1b9901_080ca139","updated":"2015-08-20 12:53:00.000000000","message":"I thought this is simpler to read, but maybe it changes when entry becomes a class, I\u0027ll check again.","commit_id":"09d2e421966987f7338fffb72c9748af7b977314"},{"author":{"_account_id":14447,"name":"lyanchih","email":"lyan.h@inwinstack.com","username":"lyanchih"},"change_message_id":"3a2a7e6a13c7477f10618fa56b7737d9c4a3b455","unresolved":false,"context_lines":[{"line_number":60,"context_line":"# query the XML in libvirt driver to find out the TCP port(s) it uses)."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"class AllocatedPort(object):"},{"line_number":64,"context_line":"    def __init__(self, host, instance_uuid, console_port, tcp_port):"},{"line_number":65,"context_line":"        self.host \u003d host"},{"line_number":66,"context_line":"        self.instance_uuid \u003d instance_uuid"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_c17ad6bc","line":63,"updated":"2015-09-01 03:03:49.000000000","message":"You can try create one singleton class, such as SerialPortPool, which has following work:\n1. Store global variable ALLOCATED_PORTS_* \n2. Put acquire_port and release_ports for-loop match logic there\n3. Using tuple struct to store host, instance_uuid, and others\n4. move _acquire_port_on_host there\n5. rename matches to find\nThese are my suggest, I just though this will make code more clear.","commit_id":"30581375bef91ac1294bbce69f70c67f53c61cda"},{"author":{"_account_id":14594,"name":"Alexander Schmidt","email":"alexs@linux.vnet.ibm.com","username":"alexs"},"change_message_id":"6013fa1e033cfb7c792a17115ca558f40c0d8a5a","unresolved":false,"context_lines":[{"line_number":60,"context_line":"# query the XML in libvirt driver to find out the TCP port(s) it uses)."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"class AllocatedPort(object):"},{"line_number":64,"context_line":"    def __init__(self, host, instance_uuid, console_port, tcp_port):"},{"line_number":65,"context_line":"        self.host \u003d host"},{"line_number":66,"context_line":"        self.instance_uuid \u003d instance_uuid"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_83ba59bf","line":63,"in_reply_to":"da20952f_c17ad6bc","updated":"2015-09-02 08:56:43.000000000","message":"Hi, thank you for your suggestions! I think they are more related to a general cleanup of the serial console code, so I would like to address these in a later changeset after enabling live mgiration. Let me know if that\u0027s okay with you, thanks.","commit_id":"30581375bef91ac1294bbce69f70c67f53c61cda"},{"author":{"_account_id":14594,"name":"Alexander Schmidt","email":"alexs@linux.vnet.ibm.com","username":"alexs"},"change_message_id":"3db9cc3ce508d6c46ae96ea944ec1873ebffa00c","unresolved":false,"context_lines":[{"line_number":60,"context_line":"# query the XML in libvirt driver to find out the TCP port(s) it uses)."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"class AllocatedPort(object):"},{"line_number":64,"context_line":"    def __init__(self, host, instance_uuid, console_port, tcp_port):"},{"line_number":65,"context_line":"        self.host \u003d host"},{"line_number":66,"context_line":"        self.instance_uuid \u003d instance_uuid"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_d96f6fc2","line":63,"in_reply_to":"da20952f_c17ad6bc","updated":"2015-09-04 14:10:18.000000000","message":"In the latest patchset, I got rid of the changes in serial.py. Please review again, thank you!","commit_id":"30581375bef91ac1294bbce69f70c67f53c61cda"}],"nova/objects/migrate_data.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"fcd782fd56f3a390a655c8c43b7b26355b0270d0","unresolved":false,"context_lines":[{"line_number":137,"context_line":"            del primitive[\u0027target_connect_addr\u0027]"},{"line_number":138,"context_line":"        if target_version \u003c (1, 2):"},{"line_number":139,"context_line":"            if \u0027serial_listen_ports\u0027 in primitive:"},{"line_number":140,"context_line":"                del primitive[\u0027serial_listen_ports\u0027]"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def _bdms_to_legacy(self, legacy):"},{"line_number":143,"context_line":"        if not self.obj_attr_is_set(\u0027bdms\u0027):"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a57f1b5_89810b4d","line":140,"updated":"2016-02-10 14:55:49.000000000","message":"Normally we put these in reverse order so that we\u0027re backporting one step at a time until we get to the target version. It makes it a lot easier for later when some change affects something that was changed in the past.\n\nMight be worth changing here for consistency and the $future.","commit_id":"416eb6fa4a007f8429d03f28a078a2e5bccb0ceb"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"ff75d758e68e3bb1ca00d843f96feff3f19e4645","unresolved":false,"context_lines":[{"line_number":137,"context_line":"            del primitive[\u0027target_connect_addr\u0027]"},{"line_number":138,"context_line":"        if target_version \u003c (1, 2):"},{"line_number":139,"context_line":"            if \u0027serial_listen_ports\u0027 in primitive:"},{"line_number":140,"context_line":"                del primitive[\u0027serial_listen_ports\u0027]"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def _bdms_to_legacy(self, legacy):"},{"line_number":143,"context_line":"        if not self.obj_attr_is_set(\u0027bdms\u0027):"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a57f1b5_f7a62352","line":140,"in_reply_to":"3a57f1b5_89810b4d","updated":"2016-02-11 10:28:50.000000000","message":"Got it. The next patch set has the order:\n\n    if target_version \u003c (1,2)\n        ...\n    if target_version \u003c (1,1)\n        ...","commit_id":"416eb6fa4a007f8429d03f28a078a2e5bccb0ceb"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1ec77413842c17563a520b618b0478275a03be37","unresolved":false,"context_lines":[{"line_number":124,"context_line":"        \u0027graphics_listen_addr_vnc\u0027: fields.IPAddressField(nullable\u003dTrue),"},{"line_number":125,"context_line":"        \u0027graphics_listen_addr_spice\u0027: fields.IPAddressField(nullable\u003dTrue),"},{"line_number":126,"context_line":"        \u0027serial_listen_addr\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":127,"context_line":"        \u0027serial_listen_ports\u0027: fields.ListOfIntegersField(),"},{"line_number":128,"context_line":"        \u0027bdms\u0027: fields.ListOfObjectsField(\u0027LibvirtLiveMigrateBDMInfo\u0027),"},{"line_number":129,"context_line":"        \u0027target_connect_addr\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":130,"context_line":"    }"}],"source_content_type":"text/x-python","patch_set":22,"id":"1a122d0e_7ac80876","line":127,"updated":"2016-05-03 18:40:18.000000000","message":"In the next patch in this series, there is a block of code that has a comment above it that says \"we could do this in post_live_migrate... but we don\u0027t have information about the source guest\u0027s serial ports\". It occurred to me when reading that comment that if we instead changed this to store the old_serial_listen_ports and new_serial_listen_ports that we would have all the information we would need in the next patch, right?","commit_id":"a0e7b9ec308adb2ee54fbe047bf8249f657b0205"}],"nova/tests/unit/objects/test_migrate_data.py":[{"author":{"_account_id":12175,"name":"Eli Qiao","email":"qiaoliyong@gmail.com","username":"Eli"},"change_message_id":"31378cbd457ee32e01713e87cd306372b60f4ee9","unresolved":false,"context_lines":[{"line_number":73,"context_line":"    def test_obj_make_compatible(self):"},{"line_number":74,"context_line":"        props \u003d {"},{"line_number":75,"context_line":"            \u0027serial_listen_addr\u0027: \u0027127.0.0.1\u0027,"},{"line_number":76,"context_line":"            \u0027serial_listen_ports\u0027: [1000, 10001, 10002, 10003],"},{"line_number":77,"context_line":"        }"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"        obj \u003d migrate_data.LibvirtLiveMigrateData(**props)"}],"source_content_type":"text/x-python","patch_set":21,"id":"1a122d0e_bab0b7de","line":76,"range":{"start_line":76,"start_character":36,"end_line":76,"end_character":40},"updated":"2016-04-25 13:06:34.000000000","message":"s/1000/10000","commit_id":"f6eb752158a5abd60ffd881646842845bf2ed6a6"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":12814,"name":"Bartosz Fic","email":"bartosz.fic@intel.com","username":"bartosz-fic"},"change_message_id":"3b36c8e26a4dccdf1901a477d2d19d30fc2d78f7","unresolved":false,"context_lines":[{"line_number":2754,"context_line":"        self.flags(enabled\u003dTrue, group\u003d\u0027serial_console\u0027)"},{"line_number":2755,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)"},{"line_number":2756,"context_line":""},{"line_number":2757,"context_line":"        # mock_config \u003d mock.MagicMock()"},{"line_number":2758,"context_line":"        mock_guest \u003d mock.MagicMock()"},{"line_number":2759,"context_line":"        mock_get_guest.return_value \u003d mock_guest"},{"line_number":2760,"context_line":"        mock_get_serials.return_value \u003d [(\u0027127.0.0.1\u0027, \u002711000\u0027)]"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a29b11f_0b703f0a","line":2757,"updated":"2015-10-21 11:21:31.000000000","message":"\"# mock_config \u003d mock.MagicMock()\"\n\nCan it be removed ?","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"},{"author":{"_account_id":12814,"name":"Bartosz Fic","email":"bartosz.fic@intel.com","username":"bartosz-fic"},"change_message_id":"3b36c8e26a4dccdf1901a477d2d19d30fc2d78f7","unresolved":false,"context_lines":[{"line_number":7264,"context_line":"        drvr._create_images_and_backing(self.context, self.test_instance,"},{"line_number":7265,"context_line":"                                        \"/fake/instance/dir\", None)"},{"line_number":7266,"context_line":""},{"line_number":7267,"context_line":"    @mock.patch(\u0027nova.console.serial.acquire_port\u0027)"},{"line_number":7268,"context_line":"    def test_pre_live_migration_works_correctly_mocked(self, acquire_port):"},{"line_number":7269,"context_line":"        # Creating testdata"},{"line_number":7270,"context_line":"        vol \u003d {\u0027block_device_mapping\u0027: ["}],"source_content_type":"text/x-python","patch_set":12,"id":"3a29b11f_2bd17bfa","line":7267,"updated":"2015-10-21 11:21:31.000000000","message":"if we use return_value\u003d10000 in decorator. It will be able to remove \"acquire_port.return_value \u003d 10000\" from line 7280","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"},{"author":{"_account_id":12814,"name":"Bartosz Fic","email":"bartosz.fic@intel.com","username":"bartosz-fic"},"change_message_id":"3b36c8e26a4dccdf1901a477d2d19d30fc2d78f7","unresolved":false,"context_lines":[{"line_number":7357,"context_line":"                return_value\u003d())"},{"line_number":7358,"context_line":"    @mock.patch(\u0027nova.virt.configdrive.required_by\u0027,"},{"line_number":7359,"context_line":"                return_value\u003dTrue)"},{"line_number":7360,"context_line":"    @mock.patch(\u0027nova.console.serial.acquire_port\u0027)"},{"line_number":7361,"context_line":"    def test_pre_live_migration_block_with_config_drive_mocked_with_vfat("},{"line_number":7362,"context_line":"            self, acquire_port, mock_required_by,"},{"line_number":7363,"context_line":"            block_device_info_get_mapping):"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a29b11f_1db5e22f","line":7360,"updated":"2015-10-21 11:21:31.000000000","message":"The same as in line 7267","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"},{"author":{"_account_id":12814,"name":"Bartosz Fic","email":"bartosz.fic@intel.com","username":"bartosz-fic"},"change_message_id":"3b36c8e26a4dccdf1901a477d2d19d30fc2d78f7","unresolved":false,"context_lines":[{"line_number":7387,"context_line":"                                                 \u0027port\u0027: {\u00270\u0027: \u002710000\u0027}},"},{"line_number":7388,"context_line":"                          \u0027volume\u0027: {}}, res_data)"},{"line_number":7389,"context_line":""},{"line_number":7390,"context_line":"    @mock.patch(\u0027nova.console.serial.acquire_port\u0027)"},{"line_number":7391,"context_line":"    def test_pre_live_migration_vol_backed_works_correctly_mocked(self,"},{"line_number":7392,"context_line":"            acquire_port):"},{"line_number":7393,"context_line":"        # Creating testdata, using temp dir."}],"source_content_type":"text/x-python","patch_set":12,"id":"3a29b11f_ddd76a49","line":7390,"updated":"2015-10-21 11:21:31.000000000","message":"ditto","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"5cbfb3531409fa22bfbd2c23742dcdd62a5af043","unresolved":false,"context_lines":[{"line_number":996,"context_line":"        for source in tcp_devices:"},{"line_number":997,"context_line":"            device \u003d source.getparent()"},{"line_number":998,"context_line":"            target \u003d device.find(\"target\")"},{"line_number":999,"context_line":"            yield (source.get(\"host\"), int(source.get(\"service\")), int(target.get(\"port\")))"},{"line_number":1000,"context_line":""},{"line_number":1001,"context_line":"    @staticmethod"},{"line_number":1002,"context_line":"    def _get_rbd_driver():"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa1b9901_5b56e22a","line":999,"updated":"2015-08-20 14:01:07.000000000","message":"Maybe it\u0027s time to replace the tuple by an object. Why not directly use the LibvirtConfigGuestChar class?","commit_id":"09d2e421966987f7338fffb72c9748af7b977314"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"5cbfb3531409fa22bfbd2c23742dcdd62a5af043","unresolved":false,"context_lines":[{"line_number":5592,"context_line":"            if target is not None:"},{"line_number":5593,"context_line":"                port \u003d target.get(\u0027port\u0027)"},{"line_number":5594,"context_line":"                if source.get(\u0027service\u0027) is not None:"},{"line_number":5595,"context_line":"                    source.set(\u0027service\u0027, listen_addr[\u0027port\u0027][port])"},{"line_number":5596,"context_line":""},{"line_number":5597,"context_line":"        return xml_doc"},{"line_number":5598,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa1b9901_3bda0e4d","line":5595,"updated":"2015-08-20 14:01:07.000000000","message":"Maybe you could reduce the code duplication here. The only difference I see is the \"serial\" and \"console\" XML element which is used in \"findall\". For example, the for loop could be a inner function with \"device_type\" as parameter where the arguments could be \"serial\" or \"console\".","commit_id":"09d2e421966987f7338fffb72c9748af7b977314"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"a5d049d2c865b078c5f1c231ed0b5919ba54de15","unresolved":false,"context_lines":[{"line_number":956,"context_line":"            instance.save()"},{"line_number":957,"context_line":""},{"line_number":958,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":959,"context_line":"            serial_console.release_ports(instance)"},{"line_number":960,"context_line":""},{"line_number":961,"context_line":"        self._undefine_domain(instance)"},{"line_number":962,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_fbe76244","line":959,"updated":"2015-09-01 16:49:55.000000000","message":"This is not related to that fix.","commit_id":"30581375bef91ac1294bbce69f70c67f53c61cda"},{"author":{"_account_id":14594,"name":"Alexander Schmidt","email":"alexs@linux.vnet.ibm.com","username":"alexs"},"change_message_id":"3db9cc3ce508d6c46ae96ea944ec1873ebffa00c","unresolved":false,"context_lines":[{"line_number":956,"context_line":"            instance.save()"},{"line_number":957,"context_line":""},{"line_number":958,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":959,"context_line":"            serial_console.release_ports(instance)"},{"line_number":960,"context_line":""},{"line_number":961,"context_line":"        self._undefine_domain(instance)"},{"line_number":962,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_d97d2f6e","line":959,"in_reply_to":"da20952f_a3d51cba","updated":"2015-09-04 14:10:18.000000000","message":"I got rid of the changes in serial.py in the latest patchset.","commit_id":"30581375bef91ac1294bbce69f70c67f53c61cda"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"f701a120cc51486660f467412ca6a23e6b2f582c","unresolved":false,"context_lines":[{"line_number":956,"context_line":"            instance.save()"},{"line_number":957,"context_line":""},{"line_number":958,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":959,"context_line":"            serial_console.release_ports(instance)"},{"line_number":960,"context_line":""},{"line_number":961,"context_line":"        self._undefine_domain(instance)"},{"line_number":962,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_a3d51cba","line":959,"in_reply_to":"da20952f_e8d0d2f9","updated":"2015-09-02 09:30:08.000000000","message":"I think is not related because all of your changes in serial.py are not necessary.","commit_id":"30581375bef91ac1294bbce69f70c67f53c61cda"},{"author":{"_account_id":14594,"name":"Alexander Schmidt","email":"alexs@linux.vnet.ibm.com","username":"alexs"},"change_message_id":"baec33ad9b22ced0ef3787927ce2c91b2ab7650b","unresolved":false,"context_lines":[{"line_number":956,"context_line":"            instance.save()"},{"line_number":957,"context_line":""},{"line_number":958,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":959,"context_line":"            serial_console.release_ports(instance)"},{"line_number":960,"context_line":""},{"line_number":961,"context_line":"        self._undefine_domain(instance)"},{"line_number":962,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_e8d0d2f9","line":959,"in_reply_to":"da20952f_fbe76244","updated":"2015-09-02 08:43:43.000000000","message":"Why do you think it\u0027s not related? There is no need to obtain the serials via _get_serial_ports_from_instance() anymore as the serial.py code now knows which serial ports are allocated to an instance.","commit_id":"30581375bef91ac1294bbce69f70c67f53c61cda"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"a5d049d2c865b078c5f1c231ed0b5919ba54de15","unresolved":false,"context_lines":[{"line_number":3897,"context_line":"                console.listen_port \u003d ("},{"line_number":3898,"context_line":"                    serial_console.acquire_port("},{"line_number":3899,"context_line":"                        console.listen_host,"},{"line_number":3900,"context_line":"                        instance.uuid,"},{"line_number":3901,"context_line":"                        port))"},{"line_number":3902,"context_line":"                guest.add_device(console)"},{"line_number":3903,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_9bc0a67e","line":3900,"updated":"2015-09-01 16:49:55.000000000","message":"So we do not have to check if instance.uuid already have ports allocated. We should not pass here more that one time.","commit_id":"30581375bef91ac1294bbce69f70c67f53c61cda"},{"author":{"_account_id":14594,"name":"Alexander Schmidt","email":"alexs@linux.vnet.ibm.com","username":"alexs"},"change_message_id":"baec33ad9b22ced0ef3787927ce2c91b2ab7650b","unresolved":false,"context_lines":[{"line_number":3897,"context_line":"                console.listen_port \u003d ("},{"line_number":3898,"context_line":"                    serial_console.acquire_port("},{"line_number":3899,"context_line":"                        console.listen_host,"},{"line_number":3900,"context_line":"                        instance.uuid,"},{"line_number":3901,"context_line":"                        port))"},{"line_number":3902,"context_line":"                guest.add_device(console)"},{"line_number":3903,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_c8e14e9c","line":3900,"in_reply_to":"da20952f_9bc0a67e","updated":"2015-09-02 08:43:43.000000000","message":"This is exactly the problem, we do pass here more than once. This code is part of the _get_guest_xml() code chain. It is expected to return the existing XML for a guest, including the existing reserved serial console ports. Right now it acquires new ports on every pass which I need to fix for live migration.\n\nget_guest_xml() is invoked by spawn(), hard_reboot() (see bug https://bugs.launchpad.net/nova/+bug/1489853), _get_existing_domain_xml(), rescue() and multiple times during migration.","commit_id":"30581375bef91ac1294bbce69f70c67f53c61cda"},{"author":{"_account_id":14594,"name":"Alexander Schmidt","email":"alexs@linux.vnet.ibm.com","username":"alexs"},"change_message_id":"3db9cc3ce508d6c46ae96ea944ec1873ebffa00c","unresolved":false,"context_lines":[{"line_number":3897,"context_line":"                console.listen_port \u003d ("},{"line_number":3898,"context_line":"                    serial_console.acquire_port("},{"line_number":3899,"context_line":"                        console.listen_host,"},{"line_number":3900,"context_line":"                        instance.uuid,"},{"line_number":3901,"context_line":"                        port))"},{"line_number":3902,"context_line":"                guest.add_device(console)"},{"line_number":3903,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_9994978b","line":3900,"in_reply_to":"da20952f_c35ff073","updated":"2015-09-04 14:10:18.000000000","message":"Done","commit_id":"30581375bef91ac1294bbce69f70c67f53c61cda"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"f701a120cc51486660f467412ca6a23e6b2f582c","unresolved":false,"context_lines":[{"line_number":3897,"context_line":"                console.listen_port \u003d ("},{"line_number":3898,"context_line":"                    serial_console.acquire_port("},{"line_number":3899,"context_line":"                        console.listen_host,"},{"line_number":3900,"context_line":"                        instance.uuid,"},{"line_number":3901,"context_line":"                        port))"},{"line_number":3902,"context_line":"                guest.add_device(console)"},{"line_number":3903,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_c35ff073","line":3900,"in_reply_to":"da20952f_c8e14e9c","updated":"2015-09-02 09:30:08.000000000","message":"Yes so we need to fix that before or you should add a hack with a TODO note (example a condition to check if xml already have serial interfaces defined). We do not want all of that update is serial.py to workaround a bug in code.","commit_id":"30581375bef91ac1294bbce69f70c67f53c61cda"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"a5d049d2c865b078c5f1c231ed0b5919ba54de15","unresolved":false,"context_lines":[{"line_number":6288,"context_line":"                CONF.serial_console.proxyclient_address"},{"line_number":6289,"context_line":"        # TODO(alexschm): Remove when image_meta is always passed"},{"line_number":6290,"context_line":"        # as an objects.ImageMeta"},{"line_number":6291,"context_line":"        if not isinstance(image_meta, objects.ImageMeta):"},{"line_number":6292,"context_line":"            image_meta \u003d objects.ImageMeta.from_dict(image_meta)"},{"line_number":6293,"context_line":"        num_ports \u003d hardware.get_number_of_serial_ports("},{"line_number":6294,"context_line":"            instance.flavor, image_meta)"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_5868d487","line":6291,"updated":"2015-09-01 16:49:55.000000000","message":"That condition and todo note are not necessary. You should to use:\n\n  image_meta \u003d objects.ImageMeta.from_instance(instance)","commit_id":"30581375bef91ac1294bbce69f70c67f53c61cda"},{"author":{"_account_id":14594,"name":"Alexander Schmidt","email":"alexs@linux.vnet.ibm.com","username":"alexs"},"change_message_id":"b94d2c5ea08eabb307ddfdf7520a9256955eef53","unresolved":false,"context_lines":[{"line_number":6288,"context_line":"                CONF.serial_console.proxyclient_address"},{"line_number":6289,"context_line":"        # TODO(alexschm): Remove when image_meta is always passed"},{"line_number":6290,"context_line":"        # as an objects.ImageMeta"},{"line_number":6291,"context_line":"        if not isinstance(image_meta, objects.ImageMeta):"},{"line_number":6292,"context_line":"            image_meta \u003d objects.ImageMeta.from_dict(image_meta)"},{"line_number":6293,"context_line":"        num_ports \u003d hardware.get_number_of_serial_ports("},{"line_number":6294,"context_line":"            instance.flavor, image_meta)"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_e364744a","line":6291,"in_reply_to":"da20952f_08ff4635","updated":"2015-09-02 09:29:29.000000000","message":"I checked the code again. image_meta is already used in this method and it\u0027s obtained as a dict via image_meta \u003d utils.get_image_from_system_metadata(). The code I introduced translates it to an object which is required for the methods I call. If I instead use objects.ImageMeta.from_instance(), the dict is generated again and then translated to an object. So I think only translating the dict to an object is more efficient.","commit_id":"30581375bef91ac1294bbce69f70c67f53c61cda"},{"author":{"_account_id":14594,"name":"Alexander Schmidt","email":"alexs@linux.vnet.ibm.com","username":"alexs"},"change_message_id":"3db9cc3ce508d6c46ae96ea944ec1873ebffa00c","unresolved":false,"context_lines":[{"line_number":6288,"context_line":"                CONF.serial_console.proxyclient_address"},{"line_number":6289,"context_line":"        # TODO(alexschm): Remove when image_meta is always passed"},{"line_number":6290,"context_line":"        # as an objects.ImageMeta"},{"line_number":6291,"context_line":"        if not isinstance(image_meta, objects.ImageMeta):"},{"line_number":6292,"context_line":"            image_meta \u003d objects.ImageMeta.from_dict(image_meta)"},{"line_number":6293,"context_line":"        num_ports \u003d hardware.get_number_of_serial_ports("},{"line_number":6294,"context_line":"            instance.flavor, image_meta)"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_99db377c","line":6291,"in_reply_to":"da20952f_5868d487","updated":"2015-09-04 14:10:18.000000000","message":"Done","commit_id":"30581375bef91ac1294bbce69f70c67f53c61cda"},{"author":{"_account_id":14594,"name":"Alexander Schmidt","email":"alexs@linux.vnet.ibm.com","username":"alexs"},"change_message_id":"baec33ad9b22ced0ef3787927ce2c91b2ab7650b","unresolved":false,"context_lines":[{"line_number":6288,"context_line":"                CONF.serial_console.proxyclient_address"},{"line_number":6289,"context_line":"        # TODO(alexschm): Remove when image_meta is always passed"},{"line_number":6290,"context_line":"        # as an objects.ImageMeta"},{"line_number":6291,"context_line":"        if not isinstance(image_meta, objects.ImageMeta):"},{"line_number":6292,"context_line":"            image_meta \u003d objects.ImageMeta.from_dict(image_meta)"},{"line_number":6293,"context_line":"        num_ports \u003d hardware.get_number_of_serial_ports("},{"line_number":6294,"context_line":"            instance.flavor, image_meta)"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_08ff4635","line":6291,"in_reply_to":"da20952f_5868d487","updated":"2015-09-02 08:43:43.000000000","message":"ok I\u0027ll fix this with the next changeset","commit_id":"30581375bef91ac1294bbce69f70c67f53c61cda"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"a5d049d2c865b078c5f1c231ed0b5919ba54de15","unresolved":false,"context_lines":[{"line_number":6293,"context_line":"        num_ports \u003d hardware.get_number_of_serial_ports("},{"line_number":6294,"context_line":"            instance.flavor, image_meta)"},{"line_number":6295,"context_line":"        for port in six.moves.range(num_ports):"},{"line_number":6296,"context_line":"            res_data[\u0027serial_listen_addr\u0027][\u0027port\u0027][str(port)] \u003d str("},{"line_number":6297,"context_line":"                    serial_console.acquire_port("},{"line_number":6298,"context_line":"                        CONF.serial_console.proxyclient_address,"},{"line_number":6299,"context_line":"                        instance.uuid, port))"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_981c2c15","line":6296,"updated":"2015-09-01 16:49:55.000000000","message":"So no, you should to let method acquire_port returning next port available. We should not have to track if a port have already been allocated for an instance (that should never happen...)","commit_id":"30581375bef91ac1294bbce69f70c67f53c61cda"},{"author":{"_account_id":14594,"name":"Alexander Schmidt","email":"alexs@linux.vnet.ibm.com","username":"alexs"},"change_message_id":"baec33ad9b22ced0ef3787927ce2c91b2ab7650b","unresolved":false,"context_lines":[{"line_number":6293,"context_line":"        num_ports \u003d hardware.get_number_of_serial_ports("},{"line_number":6294,"context_line":"            instance.flavor, image_meta)"},{"line_number":6295,"context_line":"        for port in six.moves.range(num_ports):"},{"line_number":6296,"context_line":"            res_data[\u0027serial_listen_addr\u0027][\u0027port\u0027][str(port)] \u003d str("},{"line_number":6297,"context_line":"                    serial_console.acquire_port("},{"line_number":6298,"context_line":"                        CONF.serial_console.proxyclient_address,"},{"line_number":6299,"context_line":"                        instance.uuid, port))"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_e3123558","line":6296,"in_reply_to":"da20952f_981c2c15","updated":"2015-09-02 08:43:43.000000000","message":"This code runs on the target host during the live migration to acquire a port for the instance (which is not known to the target host yet). The acquired ports are passed back to the source host so the source host can modify the XML before triggering the migration.\n\nThanks for your review, let me know if I clarified your concerns.","commit_id":"30581375bef91ac1294bbce69f70c67f53c61cda"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"f701a120cc51486660f467412ca6a23e6b2f582c","unresolved":false,"context_lines":[{"line_number":6293,"context_line":"        num_ports \u003d hardware.get_number_of_serial_ports("},{"line_number":6294,"context_line":"            instance.flavor, image_meta)"},{"line_number":6295,"context_line":"        for port in six.moves.range(num_ports):"},{"line_number":6296,"context_line":"            res_data[\u0027serial_listen_addr\u0027][\u0027port\u0027][str(port)] \u003d str("},{"line_number":6297,"context_line":"                    serial_console.acquire_port("},{"line_number":6298,"context_line":"                        CONF.serial_console.proxyclient_address,"},{"line_number":6299,"context_line":"                        instance.uuid, port))"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_e3e1d4d1","line":6296,"in_reply_to":"da20952f_e3123558","updated":"2015-09-02 09:30:08.000000000","message":"My problem is you are modifying behavior of method acquire_port to track if port is already allocated for an instance which is not necessary. see comment 3900","commit_id":"30581375bef91ac1294bbce69f70c67f53c61cda"},{"author":{"_account_id":14594,"name":"Alexander Schmidt","email":"alexs@linux.vnet.ibm.com","username":"alexs"},"change_message_id":"3db9cc3ce508d6c46ae96ea944ec1873ebffa00c","unresolved":false,"context_lines":[{"line_number":6293,"context_line":"        num_ports \u003d hardware.get_number_of_serial_ports("},{"line_number":6294,"context_line":"            instance.flavor, image_meta)"},{"line_number":6295,"context_line":"        for port in six.moves.range(num_ports):"},{"line_number":6296,"context_line":"            res_data[\u0027serial_listen_addr\u0027][\u0027port\u0027][str(port)] \u003d str("},{"line_number":6297,"context_line":"                    serial_console.acquire_port("},{"line_number":6298,"context_line":"                        CONF.serial_console.proxyclient_address,"},{"line_number":6299,"context_line":"                        instance.uuid, port))"}],"source_content_type":"text/x-python","patch_set":5,"id":"da20952f_79de636e","line":6296,"in_reply_to":"da20952f_e3e1d4d1","updated":"2015-09-04 14:10:18.000000000","message":"Done","commit_id":"30581375bef91ac1294bbce69f70c67f53c61cda"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"880d75d9e5597c47b2430c8cb07090cc51aadc9b","unresolved":false,"context_lines":[{"line_number":5489,"context_line":""},{"line_number":5490,"context_line":"        return xml_doc"},{"line_number":5491,"context_line":""},{"line_number":5492,"context_line":"    def _update_serial_xml(self, xml_doc, listen_addr):"},{"line_number":5493,"context_line":"        for dev in xml_doc.findall(\"./devices/serial[@type\u003d\u0027tcp\u0027]/source\"):"},{"line_number":5494,"context_line":"            if dev.get(\u0027host\u0027) is not None:"},{"line_number":5495,"context_line":"                dev.set(\u0027host\u0027, listen_addr)"}],"source_content_type":"text/x-python","patch_set":12,"id":"ba15a1d1_cee10e9c","side":"PARENT","line":5492,"updated":"2015-09-16 08:25:58.000000000","message":"It will be good to have this method in libvirt.Guest:\n\n  guest.update_serial_ports(listen_addr)","commit_id":"d37f35a97f25ca5d9b4a100d52cd4935e322a922"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"ae6d0241552548a1f114beee6207245549799e75","unresolved":false,"context_lines":[{"line_number":5489,"context_line":""},{"line_number":5490,"context_line":"        return xml_doc"},{"line_number":5491,"context_line":""},{"line_number":5492,"context_line":"    def _update_serial_xml(self, xml_doc, listen_addr):"},{"line_number":5493,"context_line":"        for dev in xml_doc.findall(\"./devices/serial[@type\u003d\u0027tcp\u0027]/source\"):"},{"line_number":5494,"context_line":"            if dev.get(\u0027host\u0027) is not None:"},{"line_number":5495,"context_line":"                dev.set(\u0027host\u0027, listen_addr)"}],"source_content_type":"text/x-python","patch_set":12,"id":"ba15a1d1_6e96bb40","side":"PARENT","line":5492,"in_reply_to":"ba15a1d1_cee10e9c","updated":"2015-09-16 10:58:33.000000000","message":"@sahid: Given that it is in the same place like the other \"_update_*_xml\" methods and the short amount of time until the release candidate has its cut-off date, I would like to defer this refactoring to a separate patch set which takes the other \"_update_*_xml\" methods also into account.","commit_id":"d37f35a97f25ca5d9b4a100d52cd4935e322a922"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"d70b353caa520a233d2db11319088a824bdcbe58","unresolved":false,"context_lines":[{"line_number":3839,"context_line":""},{"line_number":3840,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":3841,"context_line":"            try:"},{"line_number":3842,"context_line":"                guest_obj \u003d self._host.get_guest(instance)"},{"line_number":3843,"context_line":"                serials \u003d self._get_serial_ports_from_guest(guest_obj)"},{"line_number":3844,"context_line":"            except exception.InstanceNotFound:"},{"line_number":3845,"context_line":"                num_ports \u003d hardware.get_number_of_serial_ports("}],"source_content_type":"text/x-python","patch_set":12,"id":"ba15a1d1_e0ad5d32","line":3842,"updated":"2015-09-14 12:27:24.000000000","message":"Why do you not reuse guest\u0027s object already defined so self._get_serial_ports_from_guest(guest_obj) will return at least an empty iterator.\n\nSo that try..except can be removed.","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"ea02ffe5f2750084e2c6727950c50bc1e49510b2","unresolved":false,"context_lines":[{"line_number":3839,"context_line":""},{"line_number":3840,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":3841,"context_line":"            try:"},{"line_number":3842,"context_line":"                guest_obj \u003d self._host.get_guest(instance)"},{"line_number":3843,"context_line":"                serials \u003d self._get_serial_ports_from_guest(guest_obj)"},{"line_number":3844,"context_line":"            except exception.InstanceNotFound:"},{"line_number":3845,"context_line":"                num_ports \u003d hardware.get_number_of_serial_ports("}],"source_content_type":"text/x-python","patch_set":12,"id":"ba15a1d1_b273fbe6","line":3842,"in_reply_to":"ba15a1d1_4f599c59","updated":"2015-09-15 15:22:46.000000000","message":"My point is we should just check XML to verify whether we already have or not serial ports defined.\n\n  serials \u003d self._get_serial_ports_from_guest(guest)\n  if not serials:\n    # create devices...\n\nOR do you mean, guest and guest_obj do not refer the same XML?","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"880d75d9e5597c47b2430c8cb07090cc51aadc9b","unresolved":false,"context_lines":[{"line_number":3839,"context_line":""},{"line_number":3840,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":3841,"context_line":"            try:"},{"line_number":3842,"context_line":"                guest_obj \u003d self._host.get_guest(instance)"},{"line_number":3843,"context_line":"                serials \u003d self._get_serial_ports_from_guest(guest_obj)"},{"line_number":3844,"context_line":"            except exception.InstanceNotFound:"},{"line_number":3845,"context_line":"                num_ports \u003d hardware.get_number_of_serial_ports("}],"source_content_type":"text/x-python","patch_set":12,"id":"ba15a1d1_6e056278","line":3842,"in_reply_to":"ba15a1d1_a36e4f94","updated":"2015-09-16 08:25:58.000000000","message":"Ok I understand now, \u0027guest\u0027 should always to refer a libvirt.Guest\u0027s object and \u0027conf\u0027 should to refer libvirt.config.Libvirt*,s objects.","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"6c29b034f2739060fa65085e8a3f5fe106ef5b20","unresolved":false,"context_lines":[{"line_number":3839,"context_line":""},{"line_number":3840,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":3841,"context_line":"            try:"},{"line_number":3842,"context_line":"                guest_obj \u003d self._host.get_guest(instance)"},{"line_number":3843,"context_line":"                serials \u003d self._get_serial_ports_from_guest(guest_obj)"},{"line_number":3844,"context_line":"            except exception.InstanceNotFound:"},{"line_number":3845,"context_line":"                num_ports \u003d hardware.get_number_of_serial_ports("}],"source_content_type":"text/x-python","patch_set":12,"id":"ba15a1d1_a36e4f94","line":3842,"in_reply_to":"ba15a1d1_b273fbe6","updated":"2015-09-15 16:08:14.000000000","message":"@sahid: \n\n    \"[...] do you mean, guest and guest_obj do not\n    refer the same XML?\"\n\nYes, exactly. The method ``self._host.get_guest(instance)`` returns a ``nova.virt.libvirt.Guest`` object which *will* have serial ports. This is the case when this method is called during a live-migration. \n\nThe parameter ``guest`` is a ``nova.virt.libvirt.config.LibvirtConfigGuest`` object which will *never* have serial ports at this point. This is the case when this method is called during the creation of an instance.","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"274864ded57da98f8fa9a30f25794316d5d442eb","unresolved":false,"context_lines":[{"line_number":3839,"context_line":""},{"line_number":3840,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":3841,"context_line":"            try:"},{"line_number":3842,"context_line":"                guest_obj \u003d self._host.get_guest(instance)"},{"line_number":3843,"context_line":"                serials \u003d self._get_serial_ports_from_guest(guest_obj)"},{"line_number":3844,"context_line":"            except exception.InstanceNotFound:"},{"line_number":3845,"context_line":"                num_ports \u003d hardware.get_number_of_serial_ports("}],"source_content_type":"text/x-python","patch_set":12,"id":"ba15a1d1_4f599c59","line":3842,"in_reply_to":"ba15a1d1_e0ad5d32","updated":"2015-09-15 15:08:00.000000000","message":"@sahid: I assume Alex uses ``self._host.get_guest(instance)`` to determine if the instance is already available on the host to re-use the already acquired ports. Otherwise the port range could be drained by one single instance which gets migrated multiple times from host A to host B and back. The last paragraph in his commit messages indicates that this is the cause of this implementation. Do you think so too?","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"},{"author":{"_account_id":12814,"name":"Bartosz Fic","email":"bartosz.fic@intel.com","username":"bartosz-fic"},"change_message_id":"3b36c8e26a4dccdf1901a477d2d19d30fc2d78f7","unresolved":false,"context_lines":[{"line_number":3846,"context_line":"                    flavor, image_meta)"},{"line_number":3847,"context_line":"                serials \u003d list()"},{"line_number":3848,"context_line":"                for port in six.moves.range(num_ports):"},{"line_number":3849,"context_line":"                    listen_host \u003d CONF.serial_console.proxyclient_address"},{"line_number":3850,"context_line":"                    listen_port \u003d serial_console.acquire_port(listen_host)"},{"line_number":3851,"context_line":"                    serials.append((listen_host, listen_port))"},{"line_number":3852,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"3a29b11f_dd80cac2","line":3849,"updated":"2015-10-21 11:21:31.000000000","message":"Will listen_host has the same value for each element in serials ?\n\nIf yes maybe it could be placed simply in different variable (out of loop), instead of each tuple in serials ?","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"},{"author":{"_account_id":14594,"name":"Alexander Schmidt","email":"alexs@linux.vnet.ibm.com","username":"alexs"},"change_message_id":"13986d88b26c0d6a54c59030c4ef7f27b3a9ec9c","unresolved":false,"context_lines":[{"line_number":3846,"context_line":"                    flavor, image_meta)"},{"line_number":3847,"context_line":"                serials \u003d list()"},{"line_number":3848,"context_line":"                for port in six.moves.range(num_ports):"},{"line_number":3849,"context_line":"                    listen_host \u003d CONF.serial_console.proxyclient_address"},{"line_number":3850,"context_line":"                    listen_port \u003d serial_console.acquire_port(listen_host)"},{"line_number":3851,"context_line":"                    serials.append((listen_host, listen_port))"},{"line_number":3852,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"3a29b11f_45fb71c1","line":3849,"in_reply_to":"3a29b11f_dd80cac2","updated":"2015-10-21 15:46:51.000000000","message":"I agree, but this will be a bit difficult to change as the alternative path (using _get_serial_ports_from_guest() above) also returns the serials in that tuple format. But I can look into this change once this bugfix gets merged.","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"},{"author":{"_account_id":12814,"name":"Bartosz Fic","email":"bartosz.fic@intel.com","username":"bartosz-fic"},"change_message_id":"3b36c8e26a4dccdf1901a477d2d19d30fc2d78f7","unresolved":false,"context_lines":[{"line_number":5498,"context_line":""},{"line_number":5499,"context_line":"    def _update_serial_xml(self, xml_doc, listen_addr):"},{"line_number":5500,"context_line":"        def set_listen_addr_and_port(source, listen_addr):"},{"line_number":5501,"context_line":"            if source.get(\u0027host\u0027) is not None:"},{"line_number":5502,"context_line":"                source.set(\u0027host\u0027, listen_addr[\u0027host\u0027])"},{"line_number":5503,"context_line":"            device \u003d source.getparent()"},{"line_number":5504,"context_line":"            target \u003d device.find(\"target\")"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a29b11f_fd2d86a8","line":5501,"updated":"2015-10-21 11:21:31.000000000","message":"if source.get(\u0027host\u0027):","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"},{"author":{"_account_id":12814,"name":"Bartosz Fic","email":"bartosz.fic@intel.com","username":"bartosz-fic"},"change_message_id":"3b36c8e26a4dccdf1901a477d2d19d30fc2d78f7","unresolved":false,"context_lines":[{"line_number":5502,"context_line":"                source.set(\u0027host\u0027, listen_addr[\u0027host\u0027])"},{"line_number":5503,"context_line":"            device \u003d source.getparent()"},{"line_number":5504,"context_line":"            target \u003d device.find(\"target\")"},{"line_number":5505,"context_line":"            if target is not None:"},{"line_number":5506,"context_line":"                port \u003d target.get(\u0027port\u0027)"},{"line_number":5507,"context_line":"                if source.get(\u0027service\u0027) is not None:"},{"line_number":5508,"context_line":"                    source.set(\u0027service\u0027, listen_addr[\u0027port\u0027][port])"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a29b11f_fda82622","line":5505,"updated":"2015-10-21 11:21:31.000000000","message":"if target:","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"},{"author":{"_account_id":12814,"name":"Bartosz Fic","email":"bartosz.fic@intel.com","username":"bartosz-fic"},"change_message_id":"3b36c8e26a4dccdf1901a477d2d19d30fc2d78f7","unresolved":false,"context_lines":[{"line_number":5504,"context_line":"            target \u003d device.find(\"target\")"},{"line_number":5505,"context_line":"            if target is not None:"},{"line_number":5506,"context_line":"                port \u003d target.get(\u0027port\u0027)"},{"line_number":5507,"context_line":"                if source.get(\u0027service\u0027) is not None:"},{"line_number":5508,"context_line":"                    source.set(\u0027service\u0027, listen_addr[\u0027port\u0027][port])"},{"line_number":5509,"context_line":""},{"line_number":5510,"context_line":"        for source in xml_doc.findall(\"./devices/serial[@type\u003d\u0027tcp\u0027]/source\"):"}],"source_content_type":"text/x-python","patch_set":12,"id":"3a29b11f_1d7de2b0","line":5507,"updated":"2015-10-21 11:21:31.000000000","message":"if source.get(\u0027service\u0027):","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"351b2ecb7a4b5b92f65e9bf4b06b0b7a4950ba35","unresolved":false,"context_lines":[{"line_number":6266,"context_line":"                    \u0027serial_listen_addr\u0027: {\u0027port\u0027: {}}}"},{"line_number":6267,"context_line":"        res_data[\u0027graphics_listen_addrs\u0027][\u0027vnc\u0027] \u003d CONF.vnc.vncserver_listen"},{"line_number":6268,"context_line":"        res_data[\u0027graphics_listen_addrs\u0027][\u0027spice\u0027] \u003d CONF.spice.server_listen"},{"line_number":6269,"context_line":"        res_data[\u0027serial_listen_addr\u0027][\u0027host\u0027] \u003d \\"},{"line_number":6270,"context_line":"                CONF.serial_console.proxyclient_address"},{"line_number":6271,"context_line":"        num_ports \u003d hardware.get_number_of_serial_ports("},{"line_number":6272,"context_line":"            instance.flavor, image_meta)"}],"source_content_type":"text/x-python","patch_set":12,"id":"ba15a1d1_d4ec29bd","line":6269,"updated":"2015-09-23 10:46:52.000000000","message":"So I think this causes a problem with live-upgrades.\n\nWe need to have old and new computes running together. This non-backwards compatible change in format will break that.","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"70d24d88b8537ef7f01175ad82281fcfb676c67b","unresolved":false,"context_lines":[{"line_number":6266,"context_line":"                    \u0027serial_listen_addr\u0027: {\u0027port\u0027: {}}}"},{"line_number":6267,"context_line":"        res_data[\u0027graphics_listen_addrs\u0027][\u0027vnc\u0027] \u003d CONF.vnc.vncserver_listen"},{"line_number":6268,"context_line":"        res_data[\u0027graphics_listen_addrs\u0027][\u0027spice\u0027] \u003d CONF.spice.server_listen"},{"line_number":6269,"context_line":"        res_data[\u0027serial_listen_addr\u0027][\u0027host\u0027] \u003d \\"},{"line_number":6270,"context_line":"                CONF.serial_console.proxyclient_address"},{"line_number":6271,"context_line":"        num_ports \u003d hardware.get_number_of_serial_ports("},{"line_number":6272,"context_line":"            instance.flavor, image_meta)"}],"source_content_type":"text/x-python","patch_set":12,"id":"da85f559_0343f113","line":6269,"in_reply_to":"9a1a9d01_75e25fd7","updated":"2015-11-12 14:42:35.000000000","message":"This changes this field from a single hostname string to a dictionary of host and port. I agree with john that this breaks upgrade. We need to stash the port somewhere peer to serial_listen_addr, unless I\u0027m missing something.","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"4dbbad7b6cdd76504a845af7b72abb006693d06e","unresolved":false,"context_lines":[{"line_number":6266,"context_line":"                    \u0027serial_listen_addr\u0027: {\u0027port\u0027: {}}}"},{"line_number":6267,"context_line":"        res_data[\u0027graphics_listen_addrs\u0027][\u0027vnc\u0027] \u003d CONF.vnc.vncserver_listen"},{"line_number":6268,"context_line":"        res_data[\u0027graphics_listen_addrs\u0027][\u0027spice\u0027] \u003d CONF.spice.server_listen"},{"line_number":6269,"context_line":"        res_data[\u0027serial_listen_addr\u0027][\u0027host\u0027] \u003d \\"},{"line_number":6270,"context_line":"                CONF.serial_console.proxyclient_address"},{"line_number":6271,"context_line":"        num_ports \u003d hardware.get_number_of_serial_ports("},{"line_number":6272,"context_line":"            instance.flavor, image_meta)"}],"source_content_type":"text/x-python","patch_set":12,"id":"ba15a1d1_e76e33ff","line":6269,"in_reply_to":"ba15a1d1_d4ec29bd","updated":"2015-09-24 08:42:08.000000000","message":"I do not think so:\n\n- For case where source is old and target is new; migrate_data will be enhanced with serial_listen_addr during pre-live_migration raised from old host to new host, so domain xml will be updated - bug fixed.\n- For case where source is new and target is old; migrate_data will not have  key \"serial_listen_addr\" so domain xml will be not updated and so that bug will continue but nothing incompatible.","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"4c0fe1350fa1ab5d821cab9b20d0ad948ff74e84","unresolved":false,"context_lines":[{"line_number":6266,"context_line":"                    \u0027serial_listen_addr\u0027: {\u0027port\u0027: {}}}"},{"line_number":6267,"context_line":"        res_data[\u0027graphics_listen_addrs\u0027][\u0027vnc\u0027] \u003d CONF.vnc.vncserver_listen"},{"line_number":6268,"context_line":"        res_data[\u0027graphics_listen_addrs\u0027][\u0027spice\u0027] \u003d CONF.spice.server_listen"},{"line_number":6269,"context_line":"        res_data[\u0027serial_listen_addr\u0027][\u0027host\u0027] \u003d \\"},{"line_number":6270,"context_line":"                CONF.serial_console.proxyclient_address"},{"line_number":6271,"context_line":"        num_ports \u003d hardware.get_number_of_serial_ports("},{"line_number":6272,"context_line":"            instance.flavor, image_meta)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9a1a9d01_75e25fd7","line":6269,"in_reply_to":"ba15a1d1_d4ec29bd","updated":"2015-09-25 12:41:44.000000000","message":"I have a new patch set ready to push which does this:\n\n    res_data[\u0027serial_listen_addr\u0027] \u003d \\\n            CONF.serial_console.proxyclient_address\n    res_data[\u0027serial_listen_ports\u0027] \u003d {}\n\nBut since we don\u0027t have a gate job which can check this (AFAIK) I still have to test it manually. Or is it possible to passthrough a config to a test job, for example to enable the serial console feature?","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"fe279c427917a347b950dcc3194afaf1daff035d","unresolved":false,"context_lines":[{"line_number":6266,"context_line":"                    \u0027serial_listen_addr\u0027: {\u0027port\u0027: {}}}"},{"line_number":6267,"context_line":"        res_data[\u0027graphics_listen_addrs\u0027][\u0027vnc\u0027] \u003d CONF.vnc.vncserver_listen"},{"line_number":6268,"context_line":"        res_data[\u0027graphics_listen_addrs\u0027][\u0027spice\u0027] \u003d CONF.spice.server_listen"},{"line_number":6269,"context_line":"        res_data[\u0027serial_listen_addr\u0027][\u0027host\u0027] \u003d \\"},{"line_number":6270,"context_line":"                CONF.serial_console.proxyclient_address"},{"line_number":6271,"context_line":"        num_ports \u003d hardware.get_number_of_serial_ports("},{"line_number":6272,"context_line":"            instance.flavor, image_meta)"}],"source_content_type":"text/x-python","patch_set":12,"id":"ba15a1d1_64977a3a","line":6269,"in_reply_to":"ba15a1d1_e76e33ff","updated":"2015-09-24 11:18:47.000000000","message":"My assumption here is, that we will still merge this patch in Liberty and talk about the upgrade behavior from Kilo -\u003e Liberty.\n\nKilo doesn\u0027t have the method \"_update_serial_xml\". Which means that the dict\n\n    serial_listen_addr \u003d  \n        {\u0027host\u0027: CONF.serial_console.proxyclient_address,\n        {\u0027port\u0027: \n            {\u00270\u0027:\u0027\u003cacquired_port_number\u003e}\n        }}\n\nwithin the migration data dict, which gets returned from the method \"pre_live_migration\" from a target Liberty compute node, will not be considered during the live-migration operation.\n\nWhen we live-migrate from a Liberty compute node to a Kilo compute node, the method \"pre_live_migration\" of the Kilo target does *not* return the \"serial_listen_addr\" dict which gets handled by the source Liberty node with\n\n    if serial_listen_addr:\n        xml_doc \u003d self._update_serial_xml(xml_doc, serial_listen_addr)\n    else:\n        self._verify_serial_console_is_disabled()\n\nin the method \"_update_xml()\". I\u0027m worried here about the else path. IIUC, we will raise a \"MigrationError\", because the Kilo target node doesn\u0027t return a \"serial_listen_addr\" in the migration data. Would you please double-check if my assumption is correct/wrong?","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"73adfd8ba51856c0a62b7539880b4e37ffb320e5","unresolved":false,"context_lines":[{"line_number":6266,"context_line":"                    \u0027serial_listen_addr\u0027: {\u0027port\u0027: {}}}"},{"line_number":6267,"context_line":"        res_data[\u0027graphics_listen_addrs\u0027][\u0027vnc\u0027] \u003d CONF.vnc.vncserver_listen"},{"line_number":6268,"context_line":"        res_data[\u0027graphics_listen_addrs\u0027][\u0027spice\u0027] \u003d CONF.spice.server_listen"},{"line_number":6269,"context_line":"        res_data[\u0027serial_listen_addr\u0027][\u0027host\u0027] \u003d \\"},{"line_number":6270,"context_line":"                CONF.serial_console.proxyclient_address"},{"line_number":6271,"context_line":"        num_ports \u003d hardware.get_number_of_serial_ports("},{"line_number":6272,"context_line":"            instance.flavor, image_meta)"}],"source_content_type":"text/x-python","patch_set":12,"id":"ba8a016a_f30fb834","line":6269,"in_reply_to":"da85f559_03077082","updated":"2015-11-19 15:44:44.000000000","message":"It still changes the format of the thing that we send over the wire and so you shouldn\u0027t do it, especially since there is no reason to break the format.\n\nIf this were an object, you wouldn\u0027t be able to make this change, and since it *should* be an object, that means we shouldn\u0027t intentionally break this, even if we think that it doesn\u0027t work properly as it is.","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"},{"author":{"_account_id":14594,"name":"Alexander Schmidt","email":"alexs@linux.vnet.ibm.com","username":"alexs"},"change_message_id":"b6a208806a60ec4785e0252e1ac600375684852d","unresolved":false,"context_lines":[{"line_number":6266,"context_line":"                    \u0027serial_listen_addr\u0027: {\u0027port\u0027: {}}}"},{"line_number":6267,"context_line":"        res_data[\u0027graphics_listen_addrs\u0027][\u0027vnc\u0027] \u003d CONF.vnc.vncserver_listen"},{"line_number":6268,"context_line":"        res_data[\u0027graphics_listen_addrs\u0027][\u0027spice\u0027] \u003d CONF.spice.server_listen"},{"line_number":6269,"context_line":"        res_data[\u0027serial_listen_addr\u0027][\u0027host\u0027] \u003d \\"},{"line_number":6270,"context_line":"                CONF.serial_console.proxyclient_address"},{"line_number":6271,"context_line":"        num_ports \u003d hardware.get_number_of_serial_ports("},{"line_number":6272,"context_line":"            instance.flavor, image_meta)"}],"source_content_type":"text/x-python","patch_set":12,"id":"da85f559_03077082","line":6269,"in_reply_to":"da85f559_0343f113","updated":"2015-11-12 15:01:20.000000000","message":"Hi Dan,\n\nyes I had the same discussion with John before. My argument is that live migration with serial console is broken without this change. So migration will fail for people using old code because the TCP ports are not migrated correctly. Using a mixture of old and new code will also fail due to the change being incompatible between mixed code levels. The fix is required on both the target and source host to do a successfull migration.\n\nFor this reason, I think the broken upgrade path is not an issue as it does not introduce a regression. Let me know if this makes sense to you, thanks for the review!","commit_id":"31b43e095ce85237d2fa050189156bbb86694d71"}]}