)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"79940a092a1afd5dc1b29da30fe0ccfaefa4e5af","unresolved":false,"context_lines":[{"line_number":9,"context_line":"In commit 28d337b[1], we advertized that the NEXT_MIN_LIBVIRT and"},{"line_number":10,"context_line":"NEXT_MIN_QEMU_VERSION for \"Stein\" will be:"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"    libvirt 3.0.0 and QEMU 2.5.0"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Now that we are in the development cycle for \"Stein\", bump the"},{"line_number":15,"context_line":"MIN_{LIBVIRT,QEMU}_VERSION to the above agreed-upon versions."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"9fdfeff1_0f56815d","line":12,"range":{"start_line":12,"start_character":27,"end_line":12,"end_character":32},"updated":"2019-02-13 10:44:55.000000000","message":"2.8.0","commit_id":"634bc66c85fc5fa4b05f5ee0b23654c94157f37f"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"7ad93452a50a6ffababfa7aba7ce865cd38cbb21","unresolved":false,"context_lines":[{"line_number":17,"context_line":"As part of this clean-up:"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"  - Remove the _create_file_device() function and the calls to it; it is"},{"line_number":20,"context_line":"    a \"no-op\" when virtlogd is available.  As a result of this, in"},{"line_number":21,"context_line":"    _create_consoles_s390x(), this commit changes the console type \u0027pty\u0027"},{"line_number":22,"context_line":"    instead of \u0027file\u0027 for s390x\u0027s \"sclplm\" console device.  This is"},{"line_number":23,"context_line":"    valid, as the guest does not care about what is done in the backend."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"  - Remove the requirement for extra serial device (added in commit:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"9fdfeff1_47dead3e","line":22,"range":{"start_line":20,"start_character":43,"end_line":22,"end_character":58},"updated":"2019-02-13 14:21:02.000000000","message":"OCD nit: Bad grammar; I\u0027ll re-spin it","commit_id":"344e0fcb74f0e4057af0a72ae4924b4f2e2baed9"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f4c2a79930b0322b1dc89c7637323f62dd68bcc9","unresolved":false,"context_lines":[{"line_number":40,"context_line":"  - The unit test noise is largely mechanical: remove the superflous"},{"line_number":41,"context_line":"    serial device, and lower the index of the devices by 1."},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"The following version constants (and corresponding tests), that are now"},{"line_number":44,"context_line":"no longer required, will be removed in separate patches:"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    MIN_LIBVIRT_PARALLELS_SET_ADMIN_PASSWD,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"9fdfeff1_87d7b54b","line":43,"updated":"2019-02-13 14:16:27.000000000","message":"Looks like MIN_LIBVIRT_PERF_VERSION also.","commit_id":"344e0fcb74f0e4057af0a72ae4924b4f2e2baed9"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"7ad93452a50a6ffababfa7aba7ce865cd38cbb21","unresolved":false,"context_lines":[{"line_number":40,"context_line":"  - The unit test noise is largely mechanical: remove the superflous"},{"line_number":41,"context_line":"    serial device, and lower the index of the devices by 1."},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"The following version constants (and corresponding tests), that are now"},{"line_number":44,"context_line":"no longer required, will be removed in separate patches:"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    MIN_LIBVIRT_PARALLELS_SET_ADMIN_PASSWD,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"9fdfeff1_c7e93de3","line":43,"in_reply_to":"9fdfeff1_87d7b54b","updated":"2019-02-13 14:21:02.000000000","message":"Ah, yes.","commit_id":"344e0fcb74f0e4057af0a72ae4924b4f2e2baed9"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":19619,"name":"Michal Pryc","email":"mpryc@redhat.com","username":"migi"},"change_message_id":"1c113e48321bd07aeba0c37fb1eb778b53c0135e","unresolved":false,"context_lines":[{"line_number":239,"context_line":""},{"line_number":240,"context_line":"# Use the \"logd\" backend for handling stdout/stderr from QEMU processes."},{"line_number":241,"context_line":"MIN_LIBVIRT_VIRTLOGD \u003d (1, 3, 3)"},{"line_number":242,"context_line":"MIN_QEMU_VIRTLOGD \u003d (2, 7, 0)"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"# aarch64 architecture with KVM"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fdfeff1_96f0aa69","line":242,"range":{"start_line":242,"start_character":24,"end_line":242,"end_character":25},"updated":"2019-01-23 12:55:58.000000000","message":"Change to 9.","commit_id":"2133e4c0dbd5b3860d922c96f6998ee438f51f37"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"d80859f8a9d91a09e20ab334f4c5d44975602230","unresolved":false,"context_lines":[{"line_number":239,"context_line":""},{"line_number":240,"context_line":"# Use the \"logd\" backend for handling stdout/stderr from QEMU processes."},{"line_number":241,"context_line":"MIN_LIBVIRT_VIRTLOGD \u003d (1, 3, 3)"},{"line_number":242,"context_line":"MIN_QEMU_VIRTLOGD \u003d (2, 7, 0)"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"# aarch64 architecture with KVM"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fdfeff1_018d567a","line":242,"range":{"start_line":242,"start_character":24,"end_line":242,"end_character":25},"in_reply_to":"9fdfeff1_96f0aa69","updated":"2019-01-23 13:23:24.000000000","message":"Ha!  Indeed that works.  I was planning to remove the _VIRTLOGD version constants in a a separate patch, but obviously I need to remove it in _this_ main patch.\n\nThanks for the hint, Michal.","commit_id":"2133e4c0dbd5b3860d922c96f6998ee438f51f37"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"6824009f1b085b4ded62c7bfddcc7c55a1048a10","unresolved":false,"context_lines":[{"line_number":5036,"context_line":"    def _create_file_device(self, guest_cfg, instance, char_dev_cls,"},{"line_number":5037,"context_line":"                            target_type\u003dNone):"},{"line_number":5038,"context_line":"        if self._is_virtlogd_available():"},{"line_number":5039,"context_line":"            return"},{"line_number":5040,"context_line":""},{"line_number":5041,"context_line":"        consolelog \u003d char_dev_cls()"},{"line_number":5042,"context_line":"        consolelog.target_type \u003d target_type"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_e90edd8b","side":"PARENT","line":5039,"updated":"2019-01-24 17:43:01.000000000","message":"This removal is causing some of the failures.  I need to spend more time on the serial console code to know WTF I\u0027m doing here.","commit_id":"e3b517b6fd7c470d5cee420fce1456b98495d310"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"0b0406cf023203163f48a612194631eca4f50b24","unresolved":false,"context_lines":[{"line_number":5036,"context_line":"    def _create_file_device(self, guest_cfg, instance, char_dev_cls,"},{"line_number":5037,"context_line":"                            target_type\u003dNone):"},{"line_number":5038,"context_line":"        if self._is_virtlogd_available():"},{"line_number":5039,"context_line":"            return"},{"line_number":5040,"context_line":""},{"line_number":5041,"context_line":"        consolelog \u003d char_dev_cls()"},{"line_number":5042,"context_line":"        consolelog.target_type \u003d target_type"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_a8f66ad6","side":"PARENT","line":5039,"in_reply_to":"9fdfeff1_b7282ce2","updated":"2019-02-12 08:54:45.000000000","message":"Yeah, I\u0027ll rework the patch.\n\n(Sorry for the slow response; was occupied with something else.)","commit_id":"e3b517b6fd7c470d5cee420fce1456b98495d310"},{"author":{"_account_id":28931,"name":"Paul-Emile Element","email":"Paul-Emile.Element@windriver.com","username":"pelement"},"change_message_id":"c74eb1c7e29e0834a4a28043ec1fa0e281fcf529","unresolved":false,"context_lines":[{"line_number":5036,"context_line":"    def _create_file_device(self, guest_cfg, instance, char_dev_cls,"},{"line_number":5037,"context_line":"                            target_type\u003dNone):"},{"line_number":5038,"context_line":"        if self._is_virtlogd_available():"},{"line_number":5039,"context_line":"            return"},{"line_number":5040,"context_line":""},{"line_number":5041,"context_line":"        consolelog \u003d char_dev_cls()"},{"line_number":5042,"context_line":"        consolelog.target_type \u003d target_type"}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_b7282ce2","side":"PARENT","line":5039,"in_reply_to":"9fdfeff1_e90edd8b","updated":"2019-01-24 19:53:05.000000000","message":"Small mistake here. This function should simply return and do nothing. Your mod changed the logic.","commit_id":"e3b517b6fd7c470d5cee420fce1456b98495d310"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"79940a092a1afd5dc1b29da30fe0ccfaefa4e5af","unresolved":false,"context_lines":[{"line_number":690,"context_line":"    def _is_post_copy_available(self):"},{"line_number":691,"context_line":"        return self._host.has_min_version(lv_ver\u003dMIN_LIBVIRT_POSTCOPY_VERSION)"},{"line_number":692,"context_line":""},{"line_number":693,"context_line":"    def _is_virtlogd_available(self):"},{"line_number":694,"context_line":"        return self._host.has_min_version(MIN_LIBVIRT_VIRTLOGD,"},{"line_number":695,"context_line":"                                          MIN_QEMU_VIRTLOGD)"},{"line_number":696,"context_line":""},{"line_number":697,"context_line":"    def _is_native_luks_available(self):"},{"line_number":698,"context_line":"        return self._host.has_min_version(MIN_LIBVIRT_LUKS_VERSION,"}],"source_content_type":"text/x-python","patch_set":7,"id":"9fdfeff1_8f1a717a","side":"PARENT","line":695,"range":{"start_line":693,"start_character":0,"end_line":695,"end_character":60},"updated":"2019-02-13 10:44:55.000000000","message":"nit: could have reduced the diff somewhat by just returning True here and cleaning it up later. As is though, this is still mostly readable","commit_id":"95287619c9f59ea91537ac5789dda25aab41f5f6"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"79940a092a1afd5dc1b29da30fe0ccfaefa4e5af","unresolved":false,"context_lines":[{"line_number":5053,"context_line":"            else:"},{"line_number":5054,"context_line":"                guest_cfg.add_device(_create_base_dev())"},{"line_number":5055,"context_line":""},{"line_number":5056,"context_line":"    def _create_file_device(self, guest_cfg, instance, char_dev_cls,"},{"line_number":5057,"context_line":"                            target_type\u003dNone):"},{"line_number":5058,"context_line":"        if self._is_virtlogd_available():"},{"line_number":5059,"context_line":"            return"},{"line_number":5060,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"9fdfeff1_ca3147ef","side":"PARENT","line":5057,"range":{"start_line":5056,"start_character":0,"end_line":5057,"end_character":46},"updated":"2019-02-13 10:44:55.000000000","message":"Another nit: Could have kept this and just always returned. It\u0027s a no-op but the cleanup could conceivably be separate. However, it is a no-op so let\u0027s remove it here","commit_id":"95287619c9f59ea91537ac5789dda25aab41f5f6"},{"author":{"_account_id":28454,"name":"Georg Hoesch","email":"georg_hoesch@genua.de","username":"georghoesch"},"change_message_id":"8615d593ccfb88343fb4f2cb277d029f72979008","unresolved":false,"context_lines":[{"line_number":3170,"context_line":""},{"line_number":3171,"context_line":"        # check for different types of consoles"},{"line_number":3172,"context_line":"        path_sources \u003d ["},{"line_number":3173,"context_line":"            (\u0027file\u0027, \"./devices/console[@type\u003d\u0027file\u0027]/source[@path]\", \u0027path\u0027),"},{"line_number":3174,"context_line":"            (\u0027tcp\u0027, \"./devices/console[@type\u003d\u0027tcp\u0027]/log[@file]\", \u0027file\u0027),"},{"line_number":3175,"context_line":"            (\u0027pty\u0027, \"./devices/console[@type\u003d\u0027pty\u0027]/source[@path]\", \u0027path\u0027)]"},{"line_number":3176,"context_line":"        console_type \u003d \"\""}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_53298b34","line":3173,"updated":"2019-02-13 16:50:04.000000000","message":"Is this case obsolete after the change, are file-based sources still created?","commit_id":"344e0fcb74f0e4057af0a72ae4924b4f2e2baed9"},{"author":{"_account_id":28454,"name":"Georg Hoesch","email":"georg_hoesch@genua.de","username":"georghoesch"},"change_message_id":"8615d593ccfb88343fb4f2cb277d029f72979008","unresolved":false,"context_lines":[{"line_number":4945,"context_line":""},{"line_number":4946,"context_line":"    def _create_consoles(self, virt_type, guest_cfg, instance, flavor,"},{"line_number":4947,"context_line":"                         image_meta):"},{"line_number":4948,"context_line":"        # NOTE(markus_z): Beware! Below are so many conditionals that it is"},{"line_number":4949,"context_line":"        # easy to lose track. Use this chart to figure out your case:"},{"line_number":4950,"context_line":"        #"},{"line_number":4951,"context_line":"        # case | is serial | has       | is qemu | resulting"}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_78058cc1","line":4948,"updated":"2019-02-13 16:50:04.000000000","message":"This table came with the original change. I assume that it is not valid after the change and should be updated.\n\nAs far as I understand the current change it ensures that we always have exactly one log device(type) that should be listed here.","commit_id":"344e0fcb74f0e4057af0a72ae4924b4f2e2baed9"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"35bcf9d1c037e57004dc3785f5459f37ad4508bf","unresolved":false,"context_lines":[{"line_number":4945,"context_line":""},{"line_number":4946,"context_line":"    def _create_consoles(self, virt_type, guest_cfg, instance, flavor,"},{"line_number":4947,"context_line":"                         image_meta):"},{"line_number":4948,"context_line":"        # NOTE(markus_z): Beware! Below are so many conditionals that it is"},{"line_number":4949,"context_line":"        # easy to lose track. Use this chart to figure out your case:"},{"line_number":4950,"context_line":"        #"},{"line_number":4951,"context_line":"        # case | is serial | has       | is qemu | resulting"}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_66b63841","line":4948,"in_reply_to":"9fdfeff1_78058cc1","updated":"2019-02-14 14:31:08.000000000","message":"Good catch; yes, this table needs to be updated.  The serial/char device console stuff is head-spinning ... :-)","commit_id":"344e0fcb74f0e4057af0a72ae4924b4f2e2baed9"},{"author":{"_account_id":28454,"name":"Georg Hoesch","email":"georg_hoesch@genua.de","username":"georghoesch"},"change_message_id":"8615d593ccfb88343fb4f2cb277d029f72979008","unresolved":false,"context_lines":[{"line_number":5022,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":5023,"context_line":"            return"},{"line_number":5024,"context_line":"        else:"},{"line_number":5025,"context_line":"            guest_cfg.add_device(_create_logd_dev())"},{"line_number":5026,"context_line":""},{"line_number":5027,"context_line":"    def _serial_ports_already_defined(self, instance):"},{"line_number":5028,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_53cd0bb5","line":5025,"updated":"2019-02-13 16:50:04.000000000","message":"We only have one case left and maybe should inline the two functions above to make the code much shorter and easier to read and understand.","commit_id":"344e0fcb74f0e4057af0a72ae4924b4f2e2baed9"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"16cecff200df5797b352d33d49fc0e95c3d603c4","unresolved":false,"context_lines":[{"line_number":5022,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":5023,"context_line":"            return"},{"line_number":5024,"context_line":"        else:"},{"line_number":5025,"context_line":"            guest_cfg.add_device(_create_logd_dev())"},{"line_number":5026,"context_line":""},{"line_number":5027,"context_line":"    def _serial_ports_already_defined(self, instance):"},{"line_number":5028,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_1e33dff7","line":5025,"in_reply_to":"9fdfeff1_434194a5","updated":"2019-02-18 10:18:08.000000000","message":"The above is now done in this patch:\n\n    https://review.openstack.org/637152\n    libvirt: Rewrite _create_pty_device() to be clearer","commit_id":"344e0fcb74f0e4057af0a72ae4924b4f2e2baed9"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"35bcf9d1c037e57004dc3785f5459f37ad4508bf","unresolved":false,"context_lines":[{"line_number":5022,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":5023,"context_line":"            return"},{"line_number":5024,"context_line":"        else:"},{"line_number":5025,"context_line":"            guest_cfg.add_device(_create_logd_dev())"},{"line_number":5026,"context_line":""},{"line_number":5027,"context_line":"    def _serial_ports_already_defined(self, instance):"},{"line_number":5028,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_abcdc966","line":5025,"in_reply_to":"9fdfeff1_53cd0bb5","updated":"2019-02-14 14:31:08.000000000","message":"Noted ... however, it is probably cleaner to do that in a separate change.  I\u0027d like to keep the churn to minimum in this main version bump patch.","commit_id":"344e0fcb74f0e4057af0a72ae4924b4f2e2baed9"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"9dea5ac715e22ff7766012670fcf93bff374fa36","unresolved":false,"context_lines":[{"line_number":5022,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":5023,"context_line":"            return"},{"line_number":5024,"context_line":"        else:"},{"line_number":5025,"context_line":"            guest_cfg.add_device(_create_logd_dev())"},{"line_number":5026,"context_line":""},{"line_number":5027,"context_line":"    def _serial_ports_already_defined(self, instance):"},{"line_number":5028,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_434194a5","line":5025,"in_reply_to":"9fdfeff1_abcdc966","updated":"2019-02-15 09:03:52.000000000","message":"So, in a separate patch, I\u0027ll rework it this way:\n\n    def _create_pty_device(self, guest_cfg, char_dev_cls, target_type\u003dNone,\n                           log_path\u003dNone):\n\n        if CONF.serial_console.enabled:                                         \n            return                                                              \n                                                                                \n        consolepty \u003d char_dev_cls()                                             \n        consolepty.target_type \u003d target_type                                    \n        consolepty.type \u003d \"pty\"                                                 \n                                                                                \n        log \u003d vconfig.LibvirtConfigGuestCharDeviceLog()                         \n        log.file \u003d log_path                                                     \n        consolepty.log \u003d log                                                    \n                                                                                \n        guest_cfg.add_device(consolepty)","commit_id":"344e0fcb74f0e4057af0a72ae4924b4f2e2baed9"},{"author":{"_account_id":28454,"name":"Georg Hoesch","email":"georg_hoesch@genua.de","username":"georghoesch"},"change_message_id":"10ace99ac24a4c75293af6ee4b6d9902949fe7b8","unresolved":false,"context_lines":[{"line_number":4999,"context_line":"                self._check_number_of_serial_console(num_ports)"},{"line_number":5000,"context_line":"                self._create_serial_consoles(guest_cfg, num_ports,"},{"line_number":5001,"context_line":"                                             char_dev_cls, log_path)"},{"line_number":5002,"context_line":"        else:"},{"line_number":5003,"context_line":"            self._create_file_device(guest_cfg, instance, char_dev_cls)"},{"line_number":5004,"context_line":"        self._create_pty_device(guest_cfg, char_dev_cls, log_path\u003dlog_path)"},{"line_number":5005,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_e1d19bcd","side":"PARENT","line":5002,"updated":"2019-02-15 15:39:09.000000000","message":"move _create_pty_device() to the \u0027else\u0027 case instead of _create_file_device(). The function just NOOPs in the serial_console.enabled case.","commit_id":"95287619c9f59ea91537ac5789dda25aab41f5f6"},{"author":{"_account_id":28454,"name":"Georg Hoesch","email":"georg_hoesch@genua.de","username":"georghoesch"},"change_message_id":"10ace99ac24a4c75293af6ee4b6d9902949fe7b8","unresolved":false,"context_lines":[{"line_number":5015,"context_line":"        else:"},{"line_number":5016,"context_line":"            self._create_file_device(guest_cfg, instance, char_dev_cls,"},{"line_number":5017,"context_line":"                                     \"sclplm\")"},{"line_number":5018,"context_line":"        self._create_pty_device(guest_cfg, char_dev_cls, \"sclp\", log_path)"},{"line_number":5019,"context_line":""},{"line_number":5020,"context_line":"    def _create_pty_device(self, guest_cfg, char_dev_cls, target_type\u003dNone,"},{"line_number":5021,"context_line":"                           log_path\u003dNone):"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_81dc4fe4","side":"PARENT","line":5018,"updated":"2019-02-15 15:39:09.000000000","message":"move _create_pty_device() to the \u0027else\u0027 case instead of _create_file_device(). The function just NOOPs in the serial_console.enabled case.","commit_id":"95287619c9f59ea91537ac5789dda25aab41f5f6"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"19eb588e051546af66a7a5700d40fea811d8a3c2","unresolved":false,"context_lines":[{"line_number":4949,"context_line":"        # easy to lose track. Use this chart to figure out your case:"},{"line_number":4950,"context_line":"        #"},{"line_number":4951,"context_line":"        # case | is serial | has       | is qemu | resulting"},{"line_number":4952,"context_line":"        #      | enabled?  | virtlogd? | or kvm? | devices"},{"line_number":4953,"context_line":"        # --------------------------------------------------"},{"line_number":4954,"context_line":"        #    2 |        no |        no |     yes | pty"},{"line_number":4955,"context_line":"        #    3 |        no |       yes |      no | pty*"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_9e590f8f","line":4952,"range":{"start_line":4952,"start_character":29,"end_line":4952,"end_character":37},"updated":"2019-02-15 10:10:02.000000000","message":"Surely this entire column should go now since virtlogd is always present (or at least we don\u0027t have a conditional to check for it)\n\nLater: tbh, I\u0027d just remove this entire thing now. It should mostly be rewritten otherwise","commit_id":"900fe1c2e8e90be914f283563c0664439db54b5a"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"1f8a0105f603d2b681b8df70743ba4a79030f7b4","unresolved":false,"context_lines":[{"line_number":4949,"context_line":"        # easy to lose track. Use this chart to figure out your case:"},{"line_number":4950,"context_line":"        #"},{"line_number":4951,"context_line":"        # case | is serial | has       | is qemu | resulting"},{"line_number":4952,"context_line":"        #      | enabled?  | virtlogd? | or kvm? | devices"},{"line_number":4953,"context_line":"        # --------------------------------------------------"},{"line_number":4954,"context_line":"        #    2 |        no |        no |     yes | pty"},{"line_number":4955,"context_line":"        #    3 |        no |       yes |      no | pty*"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_043db1b9","line":4952,"range":{"start_line":4952,"start_character":29,"end_line":4952,"end_character":37},"in_reply_to":"9fdfeff1_9e590f8f","updated":"2019-02-18 11:21:31.000000000","message":"Right, I was reworking the table to reflect the new reality.  But probably removing it altogether is OK.","commit_id":"900fe1c2e8e90be914f283563c0664439db54b5a"},{"author":{"_account_id":28454,"name":"Georg Hoesch","email":"georg_hoesch@genua.de","username":"georghoesch"},"change_message_id":"10ace99ac24a4c75293af6ee4b6d9902949fe7b8","unresolved":false,"context_lines":[{"line_number":5000,"context_line":"                                                                image_meta)"},{"line_number":5001,"context_line":"                self._create_serial_consoles(guest_cfg, num_ports,"},{"line_number":5002,"context_line":"                                             char_dev_cls, log_path)"},{"line_number":5003,"context_line":"        self._create_pty_device(guest_cfg, char_dev_cls, \"sclplm\", log_path)"},{"line_number":5004,"context_line":"        self._create_pty_device(guest_cfg, char_dev_cls, \"sclp\", log_path)"},{"line_number":5005,"context_line":""},{"line_number":5006,"context_line":"    def _create_pty_device(self, guest_cfg, char_dev_cls, target_type\u003dNone,"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_a1007335","line":5003,"updated":"2019-02-15 15:39:09.000000000","message":"The else case is completely missing here. This is a functional change!","commit_id":"900fe1c2e8e90be914f283563c0664439db54b5a"},{"author":{"_account_id":28454,"name":"Georg Hoesch","email":"georg_hoesch@genua.de","username":"georghoesch"},"change_message_id":"10ace99ac24a4c75293af6ee4b6d9902949fe7b8","unresolved":false,"context_lines":[{"line_number":5018,"context_line":"            consolepty.log \u003d log"},{"line_number":5019,"context_line":"            return consolepty"},{"line_number":5020,"context_line":""},{"line_number":5021,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":5022,"context_line":"            return"},{"line_number":5023,"context_line":"        else:"},{"line_number":5024,"context_line":"            guest_cfg.add_device(_create_logd_dev())"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_dc09da2a","line":5021,"updated":"2019-02-15 15:39:09.000000000","message":"If all calls to _create_pty_device() check for serial_console.enabled themselves it should be removed here.\n\nThis also creates functionally symmetric code where\n_create_serial_consoles() and _create_pty_device() create the devices and _create_consoles_qemu_kvm() decides which type we need.","commit_id":"900fe1c2e8e90be914f283563c0664439db54b5a"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"241b2e5e31c5ce7265533982c21f45541fe25ebc","unresolved":false,"context_lines":[{"line_number":5018,"context_line":"            consolepty.log \u003d log"},{"line_number":5019,"context_line":"            return consolepty"},{"line_number":5020,"context_line":""},{"line_number":5021,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":5022,"context_line":"            return"},{"line_number":5023,"context_line":"        else:"},{"line_number":5024,"context_line":"            guest_cfg.add_device(_create_logd_dev())"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_5838769e","line":5021,"in_reply_to":"9fdfeff1_7d644cb9","updated":"2019-02-18 13:34:47.000000000","message":"The removal of this \u0027if\u0027 conditional, I\u0027ll do it in a follow-up change.\n\nBecause this is about to be merged:\n\n  https://review.openstack.org/#/c/637152/\n  libvirt: Rewrite _create_pty_device() to be clearer","commit_id":"900fe1c2e8e90be914f283563c0664439db54b5a"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"25a4a61fb71053a9f48987b6369f4876d83685b1","unresolved":false,"context_lines":[{"line_number":5018,"context_line":"            consolepty.log \u003d log"},{"line_number":5019,"context_line":"            return consolepty"},{"line_number":5020,"context_line":""},{"line_number":5021,"context_line":"        if CONF.serial_console.enabled:"},{"line_number":5022,"context_line":"            return"},{"line_number":5023,"context_line":"        else:"},{"line_number":5024,"context_line":"            guest_cfg.add_device(_create_logd_dev())"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_7d644cb9","line":5021,"in_reply_to":"9fdfeff1_dc09da2a","updated":"2019-02-18 12:59:52.000000000","message":"Yes, fully agreed.  Reworked it as such in the next iteration.  Thanks for the eagle eyes.","commit_id":"900fe1c2e8e90be914f283563c0664439db54b5a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1e3ada097bdf4ac482812c31b6009dbd501867cc","unresolved":false,"context_lines":[{"line_number":5015,"context_line":"        else:"},{"line_number":5016,"context_line":"            self._create_file_device(guest_cfg, instance, char_dev_cls,"},{"line_number":5017,"context_line":"                                     \"sclplm\")"},{"line_number":5018,"context_line":"        self._create_pty_device(guest_cfg, char_dev_cls, \"sclp\", log_path)"},{"line_number":5019,"context_line":""},{"line_number":5020,"context_line":"    def _create_pty_device(self, guest_cfg, char_dev_cls, target_type\u003dNone,"},{"line_number":5021,"context_line":"                           log_path\u003dNone):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_a72850ee","side":"PARENT","line":5018,"range":{"start_line":5018,"start_character":0,"end_line":5018,"end_character":74},"updated":"2019-02-19 10:01:49.000000000","message":"Asked this on IRC but I don\u0027t think I got a proper answer (or I didn\u0027t grasp it). We\u0027re removing this (or rather the \u0027sclplm\u0027 console above) because it doesn\u0027t actually work, right? If so, should this removal be done in a separate patch before this one since this is likely also broken on master under certain circumstances, no?","commit_id":"95287619c9f59ea91537ac5789dda25aab41f5f6"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"62794ffb5bbfd12ebd7df76e22281add249f8f10","unresolved":false,"context_lines":[{"line_number":5015,"context_line":"        else:"},{"line_number":5016,"context_line":"            self._create_file_device(guest_cfg, instance, char_dev_cls,"},{"line_number":5017,"context_line":"                                     \"sclplm\")"},{"line_number":5018,"context_line":"        self._create_pty_device(guest_cfg, char_dev_cls, \"sclp\", log_path)"},{"line_number":5019,"context_line":""},{"line_number":5020,"context_line":"    def _create_pty_device(self, guest_cfg, char_dev_cls, target_type\u003dNone,"},{"line_number":5021,"context_line":"                           log_path\u003dNone):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_6af0edfe","side":"PARENT","line":5018,"range":{"start_line":5018,"start_character":0,"end_line":5018,"end_character":74},"in_reply_to":"9fdfeff1_0a5e0112","updated":"2019-02-19 10:22:40.000000000","message":"Indeed, though I also pointed out that it is a very easy fix and is still wrong, so that\u0027s as good a reason as any to fix it now. Personally speaking, I would rather the bit of churn here so we have something simple to backport. It would also result in two smaller patches that have more focus on addressing one issue and one issue only. Seeing as there is an issue (I still wasn\u0027t sure above), any chance you could do this?","commit_id":"95287619c9f59ea91537ac5789dda25aab41f5f6"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"66a1c4146452bd982dcd945b26b789156aa12692","unresolved":false,"context_lines":[{"line_number":5015,"context_line":"        else:"},{"line_number":5016,"context_line":"            self._create_file_device(guest_cfg, instance, char_dev_cls,"},{"line_number":5017,"context_line":"                                     \"sclplm\")"},{"line_number":5018,"context_line":"        self._create_pty_device(guest_cfg, char_dev_cls, \"sclp\", log_path)"},{"line_number":5019,"context_line":""},{"line_number":5020,"context_line":"    def _create_pty_device(self, guest_cfg, char_dev_cls, target_type\u003dNone,"},{"line_number":5021,"context_line":"                           log_path\u003dNone):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_8ab8b1e5","side":"PARENT","line":5018,"range":{"start_line":5018,"start_character":0,"end_line":5018,"end_character":74},"in_reply_to":"9fdfeff1_6af0edfe","updated":"2019-02-19 10:29:35.000000000","message":"Alright, can do that. :-)","commit_id":"95287619c9f59ea91537ac5789dda25aab41f5f6"},{"author":{"_account_id":28454,"name":"Georg Hoesch","email":"georg_hoesch@genua.de","username":"georghoesch"},"change_message_id":"a5aa5ba19a90f1222eadf64b7d041c2f79e6f072","unresolved":false,"context_lines":[{"line_number":5015,"context_line":"        else:"},{"line_number":5016,"context_line":"            self._create_file_device(guest_cfg, instance, char_dev_cls,"},{"line_number":5017,"context_line":"                                     \"sclplm\")"},{"line_number":5018,"context_line":"        self._create_pty_device(guest_cfg, char_dev_cls, \"sclp\", log_path)"},{"line_number":5019,"context_line":""},{"line_number":5020,"context_line":"    def _create_pty_device(self, guest_cfg, char_dev_cls, target_type\u003dNone,"},{"line_number":5021,"context_line":"                           log_path\u003dNone):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_f64e0437","side":"PARENT","line":5018,"range":{"start_line":5018,"start_character":0,"end_line":5018,"end_character":74},"in_reply_to":"9fdfeff1_a72850ee","updated":"2019-02-19 13:54:00.000000000","message":"I assume that the reason why this didn\u0027t happen before is that previously only one of the nodes had a \u003clog file\u003d\"\"\u003e.\n\nReason (old codeflow, serial_console.enabled\u003dfalse)\n  create_file_device(\"sclplm\")\n    if virtlogd available:\n      noop\n    else\n      create file-based device\n  create_pty_device(\"sclp\")\n    if virtlogd available:\n      create pty device (logfile)\n    else\n      create pty device (base-device, no logfile)\n\nSo we with virtlogd we had one sclp device, without\nvirtlogd we had one sclplm device with logfile and one\nsclp device without logfile.\n\n\nThe change switches the second case to pty-based sclp.\nI don\u0027t know the difference between sclp and sclplm, but\none device in new code should be correct.","commit_id":"95287619c9f59ea91537ac5789dda25aab41f5f6"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"eb6f4ce6d0a27f106b8b1f426fb1a82053c59344","unresolved":false,"context_lines":[{"line_number":5015,"context_line":"        else:"},{"line_number":5016,"context_line":"            self._create_file_device(guest_cfg, instance, char_dev_cls,"},{"line_number":5017,"context_line":"                                     \"sclplm\")"},{"line_number":5018,"context_line":"        self._create_pty_device(guest_cfg, char_dev_cls, \"sclp\", log_path)"},{"line_number":5019,"context_line":""},{"line_number":5020,"context_line":"    def _create_pty_device(self, guest_cfg, char_dev_cls, target_type\u003dNone,"},{"line_number":5021,"context_line":"                           log_path\u003dNone):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_0a5e0112","side":"PARENT","line":5018,"range":{"start_line":5018,"start_character":0,"end_line":5018,"end_character":74},"in_reply_to":"9fdfeff1_a72850ee","updated":"2019-02-19 10:19:12.000000000","message":"The \"doesn\u0027t work\" here means: when \u0027virtlogd\u0027 is available, it results in a duplicate PTY device for s390x.  As you noted on IRC, no one has spotted this in ~2 years.  \n\nIf you insist, can do the work to split this off in a separate change.  However, I don\u0027t think it is worth the churn in this patch, IMHO; and should the need arise, the backport patch is simple to work out.","commit_id":"95287619c9f59ea91537ac5789dda25aab41f5f6"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"97fb42bb0330c0986afa196faa9f24ef03c13967","unresolved":false,"context_lines":[{"line_number":5015,"context_line":"        else:"},{"line_number":5016,"context_line":"            self._create_file_device(guest_cfg, instance, char_dev_cls,"},{"line_number":5017,"context_line":"                                     \"sclplm\")"},{"line_number":5018,"context_line":"        self._create_pty_device(guest_cfg, char_dev_cls, \"sclp\", log_path)"},{"line_number":5019,"context_line":""},{"line_number":5020,"context_line":"    def _create_pty_device(self, guest_cfg, char_dev_cls, target_type\u003dNone,"},{"line_number":5021,"context_line":"                           log_path\u003dNone):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_ea97f8ea","side":"PARENT","line":5018,"range":{"start_line":5018,"start_character":0,"end_line":5018,"end_character":74},"in_reply_to":"9fdfeff1_f64e0437","updated":"2019-02-19 15:00:46.000000000","message":"Ah, correct. I hadn\u0027t gone into the actual functions so I hadn\u0027t seen that they themselves had conditional. Man, this stuff is a maze.","commit_id":"95287619c9f59ea91537ac5789dda25aab41f5f6"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"4f6085ed5d19ca64019df3a82bf532c62ad2fa83","unresolved":false,"context_lines":[{"line_number":5015,"context_line":"        else:"},{"line_number":5016,"context_line":"            self._create_file_device(guest_cfg, instance, char_dev_cls,"},{"line_number":5017,"context_line":"                                     \"sclplm\")"},{"line_number":5018,"context_line":"        self._create_pty_device(guest_cfg, char_dev_cls, \"sclp\", log_path)"},{"line_number":5019,"context_line":""},{"line_number":5020,"context_line":"    def _create_pty_device(self, guest_cfg, char_dev_cls, target_type\u003dNone,"},{"line_number":5021,"context_line":"                           log_path\u003dNone):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_8a38acee","side":"PARENT","line":5018,"range":{"start_line":5018,"start_character":0,"end_line":5018,"end_character":74},"in_reply_to":"9fdfeff1_f64e0437","updated":"2019-02-19 15:06:52.000000000","message":"Yes, the above analysis is correct.  From my last two hours of investigation, we should not untangle the removal of \"slclpm\" into a separate patch.  \n\nI also discussed this with Stephen on the upstream IRC channel.  So we\u0027re on the same paragraph, same line.\n\nThanks, Georg for the through comments and double-checking.\n\n    * * *\n\nGeorg: On the difference between \"sclp\" and \"sclplm\", checking with a QEMU s390x maintainer:\n\n  - \"sclp\" is \"VT220\"-compatible;\n  - \"sclplm\" is line-mode","commit_id":"95287619c9f59ea91537ac5789dda25aab41f5f6"}]}