)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"18245dcf20917c0b7601d041f990b0fd186f53b9","unresolved":false,"context_lines":[{"line_number":14,"context_line":"have to be integrated in this console creation flow."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"During the implementation I noticed that the host caps are *not*"},{"line_number":17,"context_line":"needed for creation. That was an unnecessary special case for s390x"},{"line_number":18,"context_line":"which didn\u0027t make any sense as the guest arch is the import piece."},{"line_number":19,"context_line":"That\u0027s the reason I dropped the \"caps\" parameter of the method"},{"line_number":20,"context_line":"\"_create_consoles\". That also made it necessary to adjust the unit"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3a71b18c_78bfce60","line":17,"range":{"start_line":17,"start_character":62,"end_line":17,"end_character":67},"updated":"2016-12-06 14:20:53.000000000","message":"Hmm, imagine that... :P","commit_id":"43500f0de69fb2e1e1359539712738e0894e0820"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"a0c151145501eba9333235135186b7fd68a1b2b0","unresolved":false,"context_lines":[{"line_number":14,"context_line":"have to be integrated in this console creation flow."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"During the implementation I noticed that the host caps are *not*"},{"line_number":17,"context_line":"needed for creation. That was an unnecessary special case for s390x"},{"line_number":18,"context_line":"which didn\u0027t make any sense as the guest arch is the import piece."},{"line_number":19,"context_line":"That\u0027s the reason I dropped the \"caps\" parameter of the method"},{"line_number":20,"context_line":"\"_create_consoles\". That also made it necessary to adjust the unit"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3a71b18c_41e3fed8","line":17,"range":{"start_line":17,"start_character":62,"end_line":17,"end_character":67},"in_reply_to":"3a71b18c_78bfce60","updated":"2016-12-06 14:30:15.000000000","message":"yeah, well, you know, what can I say, ¯\\_(ツ)_/¯","commit_id":"43500f0de69fb2e1e1359539712738e0894e0820"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"18245dcf20917c0b7601d041f990b0fd186f53b9","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"During the implementation I noticed that the host caps are *not*"},{"line_number":17,"context_line":"needed for creation. That was an unnecessary special case for s390x"},{"line_number":18,"context_line":"which didn\u0027t make any sense as the guest arch is the import piece."},{"line_number":19,"context_line":"That\u0027s the reason I dropped the \"caps\" parameter of the method"},{"line_number":20,"context_line":"\"_create_consoles\". That also made it necessary to adjust the unit"},{"line_number":21,"context_line":"tests."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3a71b18c_b8b5e67e","line":18,"updated":"2016-12-06 14:20:53.000000000","message":"s/import/important/ ?","commit_id":"43500f0de69fb2e1e1359539712738e0894e0820"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"a0c151145501eba9333235135186b7fd68a1b2b0","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"During the implementation I noticed that the host caps are *not*"},{"line_number":17,"context_line":"needed for creation. That was an unnecessary special case for s390x"},{"line_number":18,"context_line":"which didn\u0027t make any sense as the guest arch is the import piece."},{"line_number":19,"context_line":"That\u0027s the reason I dropped the \"caps\" parameter of the method"},{"line_number":20,"context_line":"\"_create_consoles\". That also made it necessary to adjust the unit"},{"line_number":21,"context_line":"tests."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3a71b18c_e1da4a16","line":18,"in_reply_to":"3a71b18c_b8b5e67e","updated":"2016-12-06 14:30:15.000000000","message":"yep, (finger) muscle memory was stronger, fixed in ps3.","commit_id":"43500f0de69fb2e1e1359539712738e0894e0820"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"18245dcf20917c0b7601d041f990b0fd186f53b9","unresolved":false,"context_lines":[{"line_number":21,"context_line":"tests."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"I also took the chance to rename the \"guest\" parameter, which represents"},{"line_number":24,"context_line":"the domain *configuration object*, to \"guest_cfg\". This is (almost) used"},{"line_number":25,"context_line":"in every other place in the libvirt driver."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"Related blueprint libvirt-virtlogd"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3a71b18c_38aab619","line":24,"updated":"2016-12-06 14:20:53.000000000","message":"++","commit_id":"43500f0de69fb2e1e1359539712738e0894e0820"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"ab9676490df2f8e38fc2dff73fee480cd829831c","unresolved":false,"context_lines":[{"line_number":4369,"context_line":"        elif virt_type not in (\"qemu\", \"kvm\"):"},{"line_number":4370,"context_line":"            self._create_pty_device(guest_cfg,"},{"line_number":4371,"context_line":"                                    vconfig.LibvirtConfigGuestConsole)"},{"line_number":4372,"context_line":"        elif (virt_type in (\"qemu\", \"kvm\") and"},{"line_number":4373,"context_line":"                  self._is_s390x_guest(image_meta)):"},{"line_number":4374,"context_line":"            self._create_consoles_s390x(guest_cfg, instance,"},{"line_number":4375,"context_line":"                                        flavor, image_meta)"},{"line_number":4376,"context_line":"        elif virt_type in (\"qemu\", \"kvm\"):"},{"line_number":4377,"context_line":"            self._create_consoles_qemu_kvm(guest_cfg, instance,"},{"line_number":4378,"context_line":"                                        flavor, image_meta)"},{"line_number":4379,"context_line":"        else:"},{"line_number":4380,"context_line":"            LOG.debug(\"Unknown combination of virt_type and CPU arch.\")"},{"line_number":4381,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a71b18c_251f1509","line":4378,"range":{"start_line":4372,"start_character":8,"end_line":4378,"end_character":59},"updated":"2016-12-05 19:54:33.000000000","message":"I expect this is written that way for clarity, otherwise could be rewritten to:\nelif (virt_type in (\"qemu\", \"kvm\"):\n    if self._is_s390x_guest(image_meta):\n        ...\n    else:\n        ...","commit_id":"57c292f04c34192291aa71a67737b4cb763810a6"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"59858961a03162f3de8fb46f632eb2af1b075c88","unresolved":false,"context_lines":[{"line_number":4369,"context_line":"        elif virt_type not in (\"qemu\", \"kvm\"):"},{"line_number":4370,"context_line":"            self._create_pty_device(guest_cfg,"},{"line_number":4371,"context_line":"                                    vconfig.LibvirtConfigGuestConsole)"},{"line_number":4372,"context_line":"        elif (virt_type in (\"qemu\", \"kvm\") and"},{"line_number":4373,"context_line":"                  self._is_s390x_guest(image_meta)):"},{"line_number":4374,"context_line":"            self._create_consoles_s390x(guest_cfg, instance,"},{"line_number":4375,"context_line":"                                        flavor, image_meta)"},{"line_number":4376,"context_line":"        elif virt_type in (\"qemu\", \"kvm\"):"},{"line_number":4377,"context_line":"            self._create_consoles_qemu_kvm(guest_cfg, instance,"},{"line_number":4378,"context_line":"                                        flavor, image_meta)"},{"line_number":4379,"context_line":"        else:"},{"line_number":4380,"context_line":"            LOG.debug(\"Unknown combination of virt_type and CPU arch.\")"},{"line_number":4381,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a71b18c_e631915b","line":4378,"range":{"start_line":4372,"start_character":8,"end_line":4378,"end_character":59},"in_reply_to":"3a71b18c_251f1509","updated":"2016-12-06 08:42:28.000000000","message":"@Timofey: Yes, simply for clarity by avoiding nested conditionals. The nested conditionals (to save a few chars?) were the root cause of the mess. Therefore I decided to have here a flat list of \"use cases\" here, at top the \"most special cases\" and to the bottom the \"most common cases\".","commit_id":"57c292f04c34192291aa71a67737b4cb763810a6"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"ab9676490df2f8e38fc2dff73fee480cd829831c","unresolved":false,"context_lines":[{"line_number":4432,"context_line":"                \"Instance does not exist yet on libvirt, we can \""},{"line_number":4433,"context_line":"                \"safely pass on looking for already defined serial \""},{"line_number":4434,"context_line":"                \"ports in its domain XML\", instance\u003dinstance)"},{"line_number":4435,"context_line":"            return False"},{"line_number":4436,"context_line":""},{"line_number":4437,"context_line":"    def _create_serial_consoles(self, guest_cfg, num_ports, char_dev_cls):"},{"line_number":4438,"context_line":"        for port in six.moves.range(num_ports):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a71b18c_e59b5d64","line":4435,"range":{"start_line":4435,"start_character":12,"end_line":4435,"end_character":24},"updated":"2016-12-05 19:54:33.000000000","message":"is it wrong spacing here?\nwhat if condition on line 4426 is evaluated to false, and no exception is happened too?","commit_id":"57c292f04c34192291aa71a67737b4cb763810a6"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"59858961a03162f3de8fb46f632eb2af1b075c88","unresolved":false,"context_lines":[{"line_number":4432,"context_line":"                \"Instance does not exist yet on libvirt, we can \""},{"line_number":4433,"context_line":"                \"safely pass on looking for already defined serial \""},{"line_number":4434,"context_line":"                \"ports in its domain XML\", instance\u003dinstance)"},{"line_number":4435,"context_line":"            return False"},{"line_number":4436,"context_line":""},{"line_number":4437,"context_line":"    def _create_serial_consoles(self, guest_cfg, num_ports, char_dev_cls):"},{"line_number":4438,"context_line":"        for port in six.moves.range(num_ports):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a71b18c_2ffefdad","line":4435,"range":{"start_line":4435,"start_character":12,"end_line":4435,"end_character":24},"in_reply_to":"3a71b18c_e59b5d64","updated":"2016-12-06 08:42:28.000000000","message":"@Timofey: If the condition you described happens, then None will be returned. This doesn\u0027t affect the usage of this method in conditionals which typically do a \n\n    if _serial_ports_already_defined(instance):\n        # do things\n\nas None gets there also evaluated as \"not True\". I agree that it could be more clearer to intend the \"return False\" to the left. I\u0027ll push a new ps for that, otherwise other people could be confused here too.","commit_id":"57c292f04c34192291aa71a67737b4cb763810a6"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"18245dcf20917c0b7601d041f990b0fd186f53b9","unresolved":false,"context_lines":[{"line_number":4377,"context_line":"            self._create_consoles_qemu_kvm(guest_cfg, instance,"},{"line_number":4378,"context_line":"                                        flavor, image_meta)"},{"line_number":4379,"context_line":"        else:"},{"line_number":4380,"context_line":"            LOG.debug(\"Unknown combination of virt_type and CPU arch.\")"},{"line_number":4381,"context_line":""},{"line_number":4382,"context_line":"    def _is_s390x_guest(self, image_meta):"},{"line_number":4383,"context_line":"        s390x_archs \u003d (fields.Architecture.S390, fields.Architecture.S390X)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_1e3403fa","line":4380,"updated":"2016-12-06 14:20:53.000000000","message":"Might be nice to add the virt_type and CPU arch to the debug message here...","commit_id":"43500f0de69fb2e1e1359539712738e0894e0820"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"a0c151145501eba9333235135186b7fd68a1b2b0","unresolved":false,"context_lines":[{"line_number":4377,"context_line":"            self._create_consoles_qemu_kvm(guest_cfg, instance,"},{"line_number":4378,"context_line":"                                        flavor, image_meta)"},{"line_number":4379,"context_line":"        else:"},{"line_number":4380,"context_line":"            LOG.debug(\"Unknown combination of virt_type and CPU arch.\")"},{"line_number":4381,"context_line":""},{"line_number":4382,"context_line":"    def _is_s390x_guest(self, image_meta):"},{"line_number":4383,"context_line":"        s390x_archs \u003d (fields.Architecture.S390, fields.Architecture.S390X)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_418e9e2e","line":4380,"in_reply_to":"3a71b18c_1e3403fa","updated":"2016-12-06 14:30:15.000000000","message":"As I have to respin for the issue you found in line 4401, I\u0027ll add that in ps3.","commit_id":"43500f0de69fb2e1e1359539712738e0894e0820"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"8b4fd160b2fb7eb70ae6f382a56a6fda31f14f39","unresolved":false,"context_lines":[{"line_number":4377,"context_line":"            self._create_consoles_qemu_kvm(guest_cfg, instance,"},{"line_number":4378,"context_line":"                                        flavor, image_meta)"},{"line_number":4379,"context_line":"        else:"},{"line_number":4380,"context_line":"            LOG.debug(\"Unknown combination of virt_type and CPU arch.\")"},{"line_number":4381,"context_line":""},{"line_number":4382,"context_line":"    def _is_s390x_guest(self, image_meta):"},{"line_number":4383,"context_line":"        s390x_archs \u003d (fields.Architecture.S390, fields.Architecture.S390X)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_24637043","line":4380,"in_reply_to":"3a71b18c_418e9e2e","updated":"2016-12-06 15:06:57.000000000","message":"Funny, I wanted to write a unit test for that and discovered that I will never run into this case because of lines 4369 + 4376. Assumed there is a virt_type \u003d\u003d \"faked-virt-type\", then line 4369 will be hit. Assumed there is virt_type \u003d\u003d \"qemu\" and cpu_arch \u003d\u003d \"markus-z-cpu\", then line 4376 will be hit. \nAt first, I added that out of my habit to always have a else path...\nLong story short, I\u0027ll remove this else path which is dead code.","commit_id":"43500f0de69fb2e1e1359539712738e0894e0820"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"18245dcf20917c0b7601d041f990b0fd186f53b9","unresolved":false,"context_lines":[{"line_number":4398,"context_line":"    def _create_consoles_s390x(self, guest_cfg, instance, flavor, image_meta):"},{"line_number":4399,"context_line":"        char_dev_cls \u003d vconfig.LibvirtConfigGuestConsole"},{"line_number":4400,"context_line":"        if (CONF.serial_console.enabled and"},{"line_number":4401,"context_line":"                not self._serial_ports_already_defined(instance)):"},{"line_number":4402,"context_line":"            num_ports \u003d hardware.get_number_of_serial_ports(flavor, image_meta)"},{"line_number":4403,"context_line":"            self._create_serial_consoles(guest_cfg, num_ports, char_dev_cls)"},{"line_number":4404,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_c138eed5","line":4401,"updated":"2016-12-06 14:20:53.000000000","message":"The way this if/else is structured, if the serial ports have already been defined, then _create_file_device() will be called. Is that the intention?","commit_id":"43500f0de69fb2e1e1359539712738e0894e0820"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"a0c151145501eba9333235135186b7fd68a1b2b0","unresolved":false,"context_lines":[{"line_number":4398,"context_line":"    def _create_consoles_s390x(self, guest_cfg, instance, flavor, image_meta):"},{"line_number":4399,"context_line":"        char_dev_cls \u003d vconfig.LibvirtConfigGuestConsole"},{"line_number":4400,"context_line":"        if (CONF.serial_console.enabled and"},{"line_number":4401,"context_line":"                not self._serial_ports_already_defined(instance)):"},{"line_number":4402,"context_line":"            num_ports \u003d hardware.get_number_of_serial_ports(flavor, image_meta)"},{"line_number":4403,"context_line":"            self._create_serial_consoles(guest_cfg, num_ports, char_dev_cls)"},{"line_number":4404,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_61aaba86","line":4401,"in_reply_to":"3a71b18c_c138eed5","updated":"2016-12-06 14:30:15.000000000","message":"No, that\u0027s not intentional, that\u0027s a big mistake and I\u0027m glad you found it. Give me a minute to fix this for ps3.","commit_id":"43500f0de69fb2e1e1359539712738e0894e0820"},{"author":{"_account_id":14819,"name":"Timofey Durakov","email":"timofei.nd@gmail.com","username":"tdurakov"},"change_message_id":"dd25999af5e4f0f7b596279cec38463a56d45bc2","unresolved":false,"context_lines":[{"line_number":4432,"context_line":"                \"Instance does not exist yet on libvirt, we can \""},{"line_number":4433,"context_line":"                \"safely pass on looking for already defined serial \""},{"line_number":4434,"context_line":"                \"ports in its domain XML\", instance\u003dinstance)"},{"line_number":4435,"context_line":"        return False"},{"line_number":4436,"context_line":""},{"line_number":4437,"context_line":"    def _create_serial_consoles(self, guest_cfg, num_ports, char_dev_cls):"},{"line_number":4438,"context_line":"        for port in six.moves.range(num_ports):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_e0617206","line":4435,"range":{"start_line":4435,"start_character":0,"end_line":4435,"end_character":20},"updated":"2016-12-06 12:51:35.000000000","message":"that\u0027s much clearer, thanks!","commit_id":"43500f0de69fb2e1e1359539712738e0894e0820"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"18245dcf20917c0b7601d041f990b0fd186f53b9","unresolved":false,"context_lines":[{"line_number":4440,"context_line":"            console.port \u003d port"},{"line_number":4441,"context_line":"            console.type \u003d \"tcp\""},{"line_number":4442,"context_line":"            console.listen_host \u003d (CONF.serial_console.proxyclient_address)"},{"line_number":4443,"context_line":"            listen_port \u003d (serial_console.acquire_port(console.listen_host))"},{"line_number":4444,"context_line":"            console.listen_port \u003d listen_port"},{"line_number":4445,"context_line":"            guest_cfg.add_device(console)"},{"line_number":4446,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_9e3d73c6","line":4443,"updated":"2016-12-06 14:20:53.000000000","message":"nit: the outer parens are not necessary on the above two lines.","commit_id":"43500f0de69fb2e1e1359539712738e0894e0820"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"a0c151145501eba9333235135186b7fd68a1b2b0","unresolved":false,"context_lines":[{"line_number":4440,"context_line":"            console.port \u003d port"},{"line_number":4441,"context_line":"            console.type \u003d \"tcp\""},{"line_number":4442,"context_line":"            console.listen_host \u003d (CONF.serial_console.proxyclient_address)"},{"line_number":4443,"context_line":"            listen_port \u003d (serial_console.acquire_port(console.listen_host))"},{"line_number":4444,"context_line":"            console.listen_port \u003d listen_port"},{"line_number":4445,"context_line":"            guest_cfg.add_device(console)"},{"line_number":4446,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_01a82676","line":4443,"in_reply_to":"3a71b18c_9e3d73c6","updated":"2016-12-06 14:30:15.000000000","message":"very true, the outer parenthesis will be removed in ps3.","commit_id":"43500f0de69fb2e1e1359539712738e0894e0820"}]}
