)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1bca5bcc854efab34c3006c8ff8f9bfbd8201d03","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"handle logfiles for tcp-based consoles"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change the KVM driver to find and handle the logfile of tcp-based consoles"},{"line_number":10,"context_line":"just like regular (file only) consoles. The new code kicks in if no"},{"line_number":11,"context_line":"file-only console is found and also checks the file location of potential"},{"line_number":12,"context_line":"tcp-based console logs."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Closes-Bug: #1776481"},{"line_number":15,"context_line":"Change-Id: I3eac592f938a8d4ba813df535070e99b3130e863"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f79a3b5_c4d511e8","line":12,"range":{"start_line":9,"start_character":0,"end_line":12,"end_character":23},"updated":"2018-10-09 18:29:18.000000000","message":"in what case today can we create a guest with a tcp console today?","commit_id":"ce0cc70fb1bfc19614cdb2577d8ad197f42ebc94"},{"author":{"_account_id":28454,"name":"Georg Hoesch","email":"georg_hoesch@genua.de","username":"georghoesch"},"change_message_id":"7e4249ea733bb94fb0a76da4b48831ed9399d93c","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"handle logfiles for tcp-based consoles"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change the KVM driver to find and handle the logfile of tcp-based consoles"},{"line_number":10,"context_line":"just like regular (file only) consoles. The new code kicks in if no"},{"line_number":11,"context_line":"file-only console is found and also checks the file location of potential"},{"line_number":12,"context_line":"tcp-based console logs."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Closes-Bug: #1776481"},{"line_number":15,"context_line":"Change-Id: I3eac592f938a8d4ba813df535070e99b3130e863"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"3f79a3b5_3262f216","line":12,"range":{"start_line":9,"start_character":0,"end_line":12,"end_character":23},"in_reply_to":"3f79a3b5_c4d511e8","updated":"2018-10-10 13:42:32.000000000","message":"More information on the serial console feature is here:\nhttps://specs.openstack.org/openstack/nova-specs/specs/juno/implemented/serial-ports.html\nhttps://docs.openstack.org/nova/queens/admin/remote-console-access.html#serial-console\nhttps://docs.openstack.org/nova/queens/contributor/testing/serial-console.html\n\nIf you use kolla simply turn on \"enable_nova_serialconsole_proxy: yes\" in globals.yml","commit_id":"ce0cc70fb1bfc19614cdb2577d8ad197f42ebc94"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"a841ceacf429f08351f68fc2bbf393498b2971f3","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"handle logfiles for tcp-based consoles"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Rewrite libvirt get_console_output() and integrate tcp-based consoles."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"completely restructure get_console_output():"},{"line_number":12,"context_line":"- make common code flow visible,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3f79a3b5_6a23f858","line":9,"updated":"2018-11-29 14:51:59.000000000","message":"Just a note, you don\u0027t have to change anything, but this might have gone faster if this change were split in two, one for the refactor (which most any core should be able to review and approve if there are no functional changes) and then the actual functional change, which sounds like it would be pretty small. Anyway, that might have helped move this along faster since even though this isn\u0027t a very big patch, it\u0027s unfamiliar code to a lot of people so most are likely hesitant to even start looking at it if it\u0027s not very trivial.","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"634d78d2fd63055a5bd23edaf535e7cf0cc75f37","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"handle logfiles for tcp-based consoles"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Rewrite libvirt get_console_output() and integrate tcp-based consoles."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"completely restructure get_console_output():"},{"line_number":12,"context_line":"- make common code flow visible,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3f79a3b5_ab351aa8","line":9,"in_reply_to":"3f79a3b5_6a23f858","updated":"2018-12-05 15:24:19.000000000","message":"+1 - I\u0027d still like to see this done, to be honest, given the undocumented changes in behavior I\u0027m seeing","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4056162bf9e623df02757e0309c9eb2da74a4434","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"handle logfiles for tcp-based consoles"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Rewrite libvirt get_console_output() and integrate tcp-based consoles."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"completely restructure get_console_output():"},{"line_number":12,"context_line":"- make common code flow visible,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3f79a3b5_f739813c","line":9,"in_reply_to":"3f79a3b5_a1c0d980","updated":"2018-12-07 17:26:12.000000000","message":"\u003e The biggest part of this change is just refactoring. It doesn\u0027t\n \u003e change the functional aspects, but it has a possible side effect on\n \u003e a log message (see comment below).\n\nIt does. As noted, it won\u0027t handle a situation where there are multiple. you\u0027ve noted that that probably can\u0027t happen, but that doesn\u0027t mean it\u0027s not a functional change.\n\nLATER: as I\u0027ve now noted, \u0027find\u0027 has different behavior than I was expecting. Ignore the above.","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"},{"author":{"_account_id":28454,"name":"Georg Hoesch","email":"georg_hoesch@genua.de","username":"georghoesch"},"change_message_id":"d0535d2abb0b1cbb346ce745c1ed12152180ac92","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"handle logfiles for tcp-based consoles"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Rewrite libvirt get_console_output() and integrate tcp-based consoles."},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"completely restructure get_console_output():"},{"line_number":12,"context_line":"- make common code flow visible,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3f79a3b5_a1c0d980","line":9,"in_reply_to":"3f79a3b5_ab351aa8","updated":"2018-12-05 16:35:34.000000000","message":"The biggest part of this change is just refactoring. It doesn\u0027t change the functional aspects, but it has a possible side effect on a log message (see comment below).\n\nThe only new functionality is line 3121 with the xpath\n\"./devices/console[@type\u003d\u0027tcp\u0027]/log[@file]\". All other code was already there for the existing cases.","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"57915f532fe4a0f49decdfdb1f3ac5235d7ecec4","unresolved":false,"context_lines":[{"line_number":3106,"context_line":"                    continue"},{"line_number":3107,"context_line":""},{"line_number":3108,"context_line":"                if not os.path.exists(path):"},{"line_number":3109,"context_line":"                    LOG.info(\u0027Instance is configured with a tcp/file console, \u0027"},{"line_number":3110,"context_line":"                             \u0027but the backing file is not (yet?) present\u0027,"},{"line_number":3111,"context_line":"                             instance\u003dinstance)"},{"line_number":3112,"context_line":"                    return \"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_1426c968","line":3109,"range":{"start_line":3109,"start_character":24,"end_line":3109,"end_character":28},"updated":"2018-07-26 13:27:52.000000000","message":"warning?","commit_id":"147fe12d875d7f8061799b412b8b8d7edffeb449"},{"author":{"_account_id":28454,"name":"Georg Hoesch","email":"georg_hoesch@genua.de","username":"georghoesch"},"change_message_id":"dc0253e2938ee8b284ae9c1d772b4045b39280c9","unresolved":false,"context_lines":[{"line_number":3106,"context_line":"                    continue"},{"line_number":3107,"context_line":""},{"line_number":3108,"context_line":"                if not os.path.exists(path):"},{"line_number":3109,"context_line":"                    LOG.info(\u0027Instance is configured with a tcp/file console, \u0027"},{"line_number":3110,"context_line":"                             \u0027but the backing file is not (yet?) present\u0027,"},{"line_number":3111,"context_line":"                             instance\u003dinstance)"},{"line_number":3112,"context_line":"                    return \"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"5f7c97a3_6f1644f5","line":3109,"range":{"start_line":3109,"start_character":24,"end_line":3109,"end_character":28},"in_reply_to":"5f7c97a3_1426c968","updated":"2018-08-14 11:06:27.000000000","message":"I assume that your comment requests more information on the warning.\n\nI don\u0027t know exactly when this case can happen, but this block resembles the logfile block above and thus should have all its problems and error handlings (seen line 3090).","commit_id":"147fe12d875d7f8061799b412b8b8d7edffeb449"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1bca5bcc854efab34c3006c8ff8f9bfbd8201d03","unresolved":false,"context_lines":[{"line_number":3115,"context_line":"        xml \u003d guest.get_xml_desc()"},{"line_number":3116,"context_line":"        tree \u003d etree.fromstring(xml)"},{"line_number":3117,"context_line":""},{"line_number":3118,"context_line":"        # If the guest has a console logging to a file prefer to use that"},{"line_number":3119,"context_line":"        file_consoles \u003d tree.findall(\"./devices/console[@type\u003d\u0027file\u0027]\")"},{"line_number":3120,"context_line":"        if file_consoles:"},{"line_number":3121,"context_line":"            for file_console in file_consoles:"},{"line_number":3122,"context_line":"                source_node \u003d file_console.find(\u0027./source\u0027)"},{"line_number":3123,"context_line":"                if source_node is None:"},{"line_number":3124,"context_line":"                    continue"},{"line_number":3125,"context_line":"                path \u003d source_node.get(\"path\")"},{"line_number":3126,"context_line":"                if not path:"},{"line_number":3127,"context_line":"                    continue"},{"line_number":3128,"context_line":""},{"line_number":3129,"context_line":"                if not os.path.exists(path):"},{"line_number":3130,"context_line":"                    LOG.info(\u0027Instance is configured with a file console, \u0027"},{"line_number":3131,"context_line":"                             \u0027but the backing file is not (yet?) present\u0027,"},{"line_number":3132,"context_line":"                             instance\u003dinstance)"},{"line_number":3133,"context_line":"                    return \"\""},{"line_number":3134,"context_line":""},{"line_number":3135,"context_line":"                return self._get_console_output_file(instance, path)"},{"line_number":3136,"context_line":""},{"line_number":3137,"context_line":"        # Handle guests with tcp logging and an additional logfile"},{"line_number":3138,"context_line":"        tcp_consoles \u003d tree.findall(\"./devices/console[@type\u003d\u0027tcp\u0027]\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_64cf5d28","line":3135,"range":{"start_line":3118,"start_character":7,"end_line":3135,"end_character":68},"updated":"2018-10-09 18:29:18.000000000","message":"bar the log message all of the code is logically the same as the new code below so it could be refactor this out into a funtion that takes an error message and a xpath expression.\n\nbut i think an even better solution would be to use more of the power of xpath e.g.\n\n./devices/console[@type\u003d\u0027tcp\u0027]/log/@file\n\nwill select the path form the file attibute in the a log\n\nthe xpath expresstion that i think we want is the following\n\n(./devices/console[@type\u003d\u0027file\u0027]/source/@path | ./devices/console[@type\u003d\u0027tcp\u0027]/log/@file)[1]\n\nthis will select the path form ether a file back console or tcp console with a file log if either exits. if both exists it will sepect the first path from the two options.","commit_id":"ce0cc70fb1bfc19614cdb2577d8ad197f42ebc94"},{"author":{"_account_id":28454,"name":"Georg Hoesch","email":"georg_hoesch@genua.de","username":"georghoesch"},"change_message_id":"7e4249ea733bb94fb0a76da4b48831ed9399d93c","unresolved":false,"context_lines":[{"line_number":3115,"context_line":"        xml \u003d guest.get_xml_desc()"},{"line_number":3116,"context_line":"        tree \u003d etree.fromstring(xml)"},{"line_number":3117,"context_line":""},{"line_number":3118,"context_line":"        # If the guest has a console logging to a file prefer to use that"},{"line_number":3119,"context_line":"        file_consoles \u003d tree.findall(\"./devices/console[@type\u003d\u0027file\u0027]\")"},{"line_number":3120,"context_line":"        if file_consoles:"},{"line_number":3121,"context_line":"            for file_console in file_consoles:"},{"line_number":3122,"context_line":"                source_node \u003d file_console.find(\u0027./source\u0027)"},{"line_number":3123,"context_line":"                if source_node is None:"},{"line_number":3124,"context_line":"                    continue"},{"line_number":3125,"context_line":"                path \u003d source_node.get(\"path\")"},{"line_number":3126,"context_line":"                if not path:"},{"line_number":3127,"context_line":"                    continue"},{"line_number":3128,"context_line":""},{"line_number":3129,"context_line":"                if not os.path.exists(path):"},{"line_number":3130,"context_line":"                    LOG.info(\u0027Instance is configured with a file console, \u0027"},{"line_number":3131,"context_line":"                             \u0027but the backing file is not (yet?) present\u0027,"},{"line_number":3132,"context_line":"                             instance\u003dinstance)"},{"line_number":3133,"context_line":"                    return \"\""},{"line_number":3134,"context_line":""},{"line_number":3135,"context_line":"                return self._get_console_output_file(instance, path)"},{"line_number":3136,"context_line":""},{"line_number":3137,"context_line":"        # Handle guests with tcp logging and an additional logfile"},{"line_number":3138,"context_line":"        tcp_consoles \u003d tree.findall(\"./devices/console[@type\u003d\u0027tcp\u0027]\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_75ea94a0","line":3135,"range":{"start_line":3118,"start_character":7,"end_line":3135,"end_character":68},"in_reply_to":"3f79a3b5_64cf5d28","updated":"2018-10-10 13:42:32.000000000","message":"I already tried using one combined xpath statement but the xpath parser is crippled and does not accept complex expressions (like the one in your example).\n\nI know this leads to code duplication but I chose this to keep the code readable and understandable. I tried to refactor this in a function but ended up with a solution that was technically correct but hard to understand and document.\n\nBTW: instances usually have a tcp console or a file console. I think we can neglect the thought that both exist at once.","commit_id":"ce0cc70fb1bfc19614cdb2577d8ad197f42ebc94"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9cb5dc459c0c430971f26753db494ea667124e5a","unresolved":false,"context_lines":[{"line_number":3115,"context_line":"        xml \u003d guest.get_xml_desc()"},{"line_number":3116,"context_line":"        tree \u003d etree.fromstring(xml)"},{"line_number":3117,"context_line":""},{"line_number":3118,"context_line":"        # If the guest has a console logging to a file prefer to use that"},{"line_number":3119,"context_line":"        file_consoles \u003d tree.findall(\"./devices/console[@type\u003d\u0027file\u0027]\")"},{"line_number":3120,"context_line":"        if file_consoles:"},{"line_number":3121,"context_line":"            for file_console in file_consoles:"},{"line_number":3122,"context_line":"                source_node \u003d file_console.find(\u0027./source\u0027)"},{"line_number":3123,"context_line":"                if source_node is None:"},{"line_number":3124,"context_line":"                    continue"},{"line_number":3125,"context_line":"                path \u003d source_node.get(\"path\")"},{"line_number":3126,"context_line":"                if not path:"},{"line_number":3127,"context_line":"                    continue"},{"line_number":3128,"context_line":""},{"line_number":3129,"context_line":"                if not os.path.exists(path):"},{"line_number":3130,"context_line":"                    LOG.info(\u0027Instance is configured with a file console, \u0027"},{"line_number":3131,"context_line":"                             \u0027but the backing file is not (yet?) present\u0027,"},{"line_number":3132,"context_line":"                             instance\u003dinstance)"},{"line_number":3133,"context_line":"                    return \"\""},{"line_number":3134,"context_line":""},{"line_number":3135,"context_line":"                return self._get_console_output_file(instance, path)"},{"line_number":3136,"context_line":""},{"line_number":3137,"context_line":"        # Handle guests with tcp logging and an additional logfile"},{"line_number":3138,"context_line":"        tcp_consoles \u003d tree.findall(\"./devices/console[@type\u003d\u0027tcp\u0027]\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_4ca7d151","line":3135,"range":{"start_line":3118,"start_character":7,"end_line":3135,"end_character":68},"in_reply_to":"3f79a3b5_75ea94a0","updated":"2018-10-10 14:46:39.000000000","message":"in that case why not do \n\nfile \u003d tree.findall(\"./devices/console[@type\u003d\u0027file\u0027]/source/@path\")[0]\ntcp \u003d ree.findall(\"./devices/console[@type\u003d\u0027tcp\u0027]/log/@file\")[0]\npty \u003d ree.findall(\"./devices/console[@type\u003d\u0027pty\u0027]/source/@path\")[0]\n\npath \u003d file or tcp or pty\n\nif not os.path.exists(path):\n    LOG.info(\u0027Instance is configured with a console, \u0027\n             \u0027but the backing file is not (yet?) present\u0027, \n             instance\u003dinstance)\n    raise exception.ConsoleNotAvailable()\n\nif file or tcp:\n    return self._get_console_output_file(instance, path)\n\nconsole_log \u003d self._get_console_log_path(instance)\ndata \u003d nova.privsep.libvirt.readpty(pty)","commit_id":"ce0cc70fb1bfc19614cdb2577d8ad197f42ebc94"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1bca5bcc854efab34c3006c8ff8f9bfbd8201d03","unresolved":false,"context_lines":[{"line_number":3135,"context_line":"                return self._get_console_output_file(instance, path)"},{"line_number":3136,"context_line":""},{"line_number":3137,"context_line":"        # Handle guests with tcp logging and an additional logfile"},{"line_number":3138,"context_line":"        tcp_consoles \u003d tree.findall(\"./devices/console[@type\u003d\u0027tcp\u0027]\")"},{"line_number":3139,"context_line":"        if tcp_consoles:"},{"line_number":3140,"context_line":"            for tcp_console in tcp_consoles:"},{"line_number":3141,"context_line":"                log_node \u003d tcp_console.find(\u0027./log\u0027)"},{"line_number":3142,"context_line":"                if log_node is None:"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_64fe7d6b","line":3139,"range":{"start_line":3138,"start_character":8,"end_line":3139,"end_character":24},"updated":"2018-10-09 18:29:18.000000000","message":"so here we use an xpath expression to fine all concoels with type \u003d tcp","commit_id":"ce0cc70fb1bfc19614cdb2577d8ad197f42ebc94"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1bca5bcc854efab34c3006c8ff8f9bfbd8201d03","unresolved":false,"context_lines":[{"line_number":3138,"context_line":"        tcp_consoles \u003d tree.findall(\"./devices/console[@type\u003d\u0027tcp\u0027]\")"},{"line_number":3139,"context_line":"        if tcp_consoles:"},{"line_number":3140,"context_line":"            for tcp_console in tcp_consoles:"},{"line_number":3141,"context_line":"                log_node \u003d tcp_console.find(\u0027./log\u0027)"},{"line_number":3142,"context_line":"                if log_node is None:"},{"line_number":3143,"context_line":"                    continue"},{"line_number":3144,"context_line":"                path \u003d log_node.get(\"file\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_84263922","line":3141,"range":{"start_line":3141,"start_character":17,"end_line":3141,"end_character":52},"updated":"2018-10-09 18:29:18.000000000","message":"the for each console element this xpat expresstion fines any childeren form the current node of element type log","commit_id":"ce0cc70fb1bfc19614cdb2577d8ad197f42ebc94"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1bca5bcc854efab34c3006c8ff8f9bfbd8201d03","unresolved":false,"context_lines":[{"line_number":3141,"context_line":"                log_node \u003d tcp_console.find(\u0027./log\u0027)"},{"line_number":3142,"context_line":"                if log_node is None:"},{"line_number":3143,"context_line":"                    continue"},{"line_number":3144,"context_line":"                path \u003d log_node.get(\"file\")"},{"line_number":3145,"context_line":"                if not path:"},{"line_number":3146,"context_line":"                    continue"},{"line_number":3147,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_a429f5f1","line":3144,"range":{"start_line":3144,"start_character":17,"end_line":3144,"end_character":43},"updated":"2018-10-09 18:29:18.000000000","message":"if we find the log element we extract the file attribute or get NONE if it does not exist","commit_id":"ce0cc70fb1bfc19614cdb2577d8ad197f42ebc94"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1bca5bcc854efab34c3006c8ff8f9bfbd8201d03","unresolved":false,"context_lines":[{"line_number":3149,"context_line":"                    LOG.info(\u0027Instance is configured with a tcp/file console, \u0027"},{"line_number":3150,"context_line":"                             \u0027but the backing file is not (yet?) present\u0027,"},{"line_number":3151,"context_line":"                             instance\u003dinstance)"},{"line_number":3152,"context_line":"                    return \"\""},{"line_number":3153,"context_line":""},{"line_number":3154,"context_line":"                return self._get_console_output_file(instance, path)"},{"line_number":3155,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_c41c31c4","line":3152,"range":{"start_line":3152,"start_character":21,"end_line":3152,"end_character":29},"updated":"2018-10-09 18:29:18.000000000","message":"im not sure this is correct\nit is coppied form the file based example but perhaps a contiue would be better.\n\nthat said if since the existing file based console does this it likely is what the caller is expecting","commit_id":"ce0cc70fb1bfc19614cdb2577d8ad197f42ebc94"},{"author":{"_account_id":28454,"name":"Georg Hoesch","email":"georg_hoesch@genua.de","username":"georghoesch"},"change_message_id":"7e4249ea733bb94fb0a76da4b48831ed9399d93c","unresolved":false,"context_lines":[{"line_number":3149,"context_line":"                    LOG.info(\u0027Instance is configured with a tcp/file console, \u0027"},{"line_number":3150,"context_line":"                             \u0027but the backing file is not (yet?) present\u0027,"},{"line_number":3151,"context_line":"                             instance\u003dinstance)"},{"line_number":3152,"context_line":"                    return \"\""},{"line_number":3153,"context_line":""},{"line_number":3154,"context_line":"                return self._get_console_output_file(instance, path)"},{"line_number":3155,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_1b027173","line":3152,"range":{"start_line":3152,"start_character":21,"end_line":3152,"end_character":29},"in_reply_to":"3f79a3b5_c41c31c4","updated":"2018-10-10 13:42:32.000000000","message":"At this point we know that we have found an existing serial console node. As far as i can understand, the specified file might not yet be available (because the instance is still starting up ?). We have a console, just no output yet, and do not want to continue to \"ConsoleNotAvailable\".\n\nI copied this line from file-based consoles and cannot confirm that this case happens in reality. But the basic logic flow looks correct.","commit_id":"ce0cc70fb1bfc19614cdb2577d8ad197f42ebc94"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1bca5bcc854efab34c3006c8ff8f9bfbd8201d03","unresolved":false,"context_lines":[{"line_number":3154,"context_line":"                return self._get_console_output_file(instance, path)"},{"line_number":3155,"context_line":""},{"line_number":3156,"context_line":"        # Try \u0027pty\u0027 types"},{"line_number":3157,"context_line":"        pty_consoles \u003d tree.findall(\"./devices/console[@type\u003d\u0027pty\u0027]\")"},{"line_number":3158,"context_line":"        if pty_consoles:"},{"line_number":3159,"context_line":"            for pty_console in pty_consoles:"},{"line_number":3160,"context_line":"                source_node \u003d pty_console.find(\u0027./source\u0027)"},{"line_number":3161,"context_line":"                if source_node is None:"},{"line_number":3162,"context_line":"                    continue"},{"line_number":3163,"context_line":"                pty \u003d source_node.get(\"path\")"},{"line_number":3164,"context_line":"                if not pty:"},{"line_number":3165,"context_line":"                    continue"},{"line_number":3166,"context_line":"                break"},{"line_number":3167,"context_line":"            else:"},{"line_number":3168,"context_line":"                raise exception.ConsoleNotAvailable()"},{"line_number":3169,"context_line":"        else:"},{"line_number":3170,"context_line":"            raise exception.ConsoleNotAvailable()"},{"line_number":3171,"context_line":""},{"line_number":3172,"context_line":"        console_log \u003d self._get_console_log_path(instance)"},{"line_number":3173,"context_line":"        data \u003d nova.privsep.libvirt.readpty(pty)"},{"line_number":3174,"context_line":""},{"line_number":3175,"context_line":"        # NOTE(markus_z): The virt_types kvm and qemu are the only ones"},{"line_number":3176,"context_line":"        # which create a dedicated file device for the console logging."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_64399def","line":3173,"range":{"start_line":3157,"start_character":6,"end_line":3173,"end_character":48},"updated":"2018-10-09 18:29:18.000000000","message":"this could also be done with by extending the xpath expression to \n\n(./devices/console[@type\u003d\u0027file\u0027]/source/@path | ./devices/console[@type\u003d\u0027tcp\u0027]/log/@file | ./devices/console[@type\u003d\u0027pty\u0027]/source/@path)[1]\n\nalso the fact that we trow an exception in this case but return \"\" in the file and tcp cases is likely a bug and or should be made consitent.","commit_id":"ce0cc70fb1bfc19614cdb2577d8ad197f42ebc94"},{"author":{"_account_id":28454,"name":"Georg Hoesch","email":"georg_hoesch@genua.de","username":"georghoesch"},"change_message_id":"7e4249ea733bb94fb0a76da4b48831ed9399d93c","unresolved":false,"context_lines":[{"line_number":3154,"context_line":"                return self._get_console_output_file(instance, path)"},{"line_number":3155,"context_line":""},{"line_number":3156,"context_line":"        # Try \u0027pty\u0027 types"},{"line_number":3157,"context_line":"        pty_consoles \u003d tree.findall(\"./devices/console[@type\u003d\u0027pty\u0027]\")"},{"line_number":3158,"context_line":"        if pty_consoles:"},{"line_number":3159,"context_line":"            for pty_console in pty_consoles:"},{"line_number":3160,"context_line":"                source_node \u003d pty_console.find(\u0027./source\u0027)"},{"line_number":3161,"context_line":"                if source_node is None:"},{"line_number":3162,"context_line":"                    continue"},{"line_number":3163,"context_line":"                pty \u003d source_node.get(\"path\")"},{"line_number":3164,"context_line":"                if not pty:"},{"line_number":3165,"context_line":"                    continue"},{"line_number":3166,"context_line":"                break"},{"line_number":3167,"context_line":"            else:"},{"line_number":3168,"context_line":"                raise exception.ConsoleNotAvailable()"},{"line_number":3169,"context_line":"        else:"},{"line_number":3170,"context_line":"            raise exception.ConsoleNotAvailable()"},{"line_number":3171,"context_line":""},{"line_number":3172,"context_line":"        console_log \u003d self._get_console_log_path(instance)"},{"line_number":3173,"context_line":"        data \u003d nova.privsep.libvirt.readpty(pty)"},{"line_number":3174,"context_line":""},{"line_number":3175,"context_line":"        # NOTE(markus_z): The virt_types kvm and qemu are the only ones"},{"line_number":3176,"context_line":"        # which create a dedicated file device for the console logging."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_86c14884","line":3173,"range":{"start_line":3157,"start_character":6,"end_line":3173,"end_character":48},"in_reply_to":"3f79a3b5_64399def","updated":"2018-10-10 13:42:32.000000000","message":"For XPath: xpath parser can\u0027t handle this, see above.\n\nFor return \"\":\nThe control flow in the pty case is different. This is the last type of console and instead of finding a valid console and returning its output, the loops exits (with exception) if we do NOT find a console. If we have one, the code continues and returns _get_console_output_file().which returns an empty string if the file doesn\u0027t exist.","commit_id":"ce0cc70fb1bfc19614cdb2577d8ad197f42ebc94"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"91cb45391970f87182bc29dfd4bf0dc5a68453c9","unresolved":false,"context_lines":[{"line_number":3118,"context_line":"        # check for different types of consoles"},{"line_number":3119,"context_line":"        path_sources \u003d ["},{"line_number":3120,"context_line":"            (\u0027file\u0027, \"./devices/console[@type\u003d\u0027file\u0027]/source[@path]\", \u0027path\u0027),"},{"line_number":3121,"context_line":"            (\u0027tcp\u0027, \"./devices/console[@type\u003d\u0027tcp\u0027]/log[@file]\", \u0027file\u0027),"},{"line_number":3122,"context_line":"            (\u0027pty\u0027, \"./devices/console[@type\u003d\u0027pty\u0027]/source[@path]\", \u0027path\u0027)]"},{"line_number":3123,"context_line":"        console_type \u003d \"\""},{"line_number":3124,"context_line":"        console_path \u003d \"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_db8784b3","line":3121,"range":{"start_line":3121,"start_character":12,"end_line":3121,"end_character":72},"updated":"2018-10-30 16:56:53.000000000","message":"So, I was just double-checking this with a libvirt developer (thank: Michal Privoznik), and the above (which translates to XML snippet \u003clog file\u003d\u0027$FOO.log\u0027/\u003e) might not be present all the time.\n\n*Unless* we create the log file for TCP character device at time of defining the libvirt domain (instance) time.  Refer to: https://libvirt.org/formatdomain.html#elementsConsole\n\nAs a quick example (I just tested):\n\n(1) Define a guest with:\n\n    \u003cserial type\u003d\u0027tcp\u0027\u003e\n      \u003csource mode\u003d\u0027connect\u0027 host\u003d\u0027127.0.0.1\u0027 service\u003d\u00277777\u0027 tls\u003d\u0027no\u0027/\u003e\n      \u003cprotocol type\u003d\u0027raw\u0027/\u003e\n      \u003clog file\u003d\u0027/tmp/foo.log\u0027/\u003e\n      \u003ctarget type\u003d\u0027isa-serial\u0027 port\u003d\u00270\u0027\u003e\n        \u003cmodel name\u003d\u0027isa-serial\u0027/\u003e\n      \u003c/target\u003e\n      \u003calias name\u003d\u0027serial0\u0027/\u003e\n    \u003c/serial\u003e\n\n(2) Listen on port 7777: `nc -l -p 7777`\n\n(3) Start the guest, and observe the log file: `tail -f /tmp/foo.log`","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"},{"author":{"_account_id":28454,"name":"Georg Hoesch","email":"georg_hoesch@genua.de","username":"georghoesch"},"change_message_id":"65145761c7e45cd7f2d52b4ebe1acd9501696add","unresolved":false,"context_lines":[{"line_number":3118,"context_line":"        # check for different types of consoles"},{"line_number":3119,"context_line":"        path_sources \u003d ["},{"line_number":3120,"context_line":"            (\u0027file\u0027, \"./devices/console[@type\u003d\u0027file\u0027]/source[@path]\", \u0027path\u0027),"},{"line_number":3121,"context_line":"            (\u0027tcp\u0027, \"./devices/console[@type\u003d\u0027tcp\u0027]/log[@file]\", \u0027file\u0027),"},{"line_number":3122,"context_line":"            (\u0027pty\u0027, \"./devices/console[@type\u003d\u0027pty\u0027]/source[@path]\", \u0027path\u0027)]"},{"line_number":3123,"context_line":"        console_type \u003d \"\""},{"line_number":3124,"context_line":"        console_path \u003d \"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_6dbaf20c","line":3121,"range":{"start_line":3121,"start_character":12,"end_line":3121,"end_character":72},"in_reply_to":"3f79a3b5_3b331815","updated":"2018-11-02 14:13:58.000000000","message":"Thanks for the feedback. I\u0027ll try to answer all different aspects that you brought up:\n\n1) The goal of this change is to fix the \u0027default\u0027 behaviour of tcp consoles in nova/kvm, see bug description. I don\u0027t want to target all possible cases of serial consoles in different hypervisors.\n\n2) If the XML looks different (for whatever reason), the code won\u0027t break. The worst thing that could happen is that we do not find the console and raise a ConsoleNotAvailable() exception or return empty content if the backing logfile is not yet present. This is just the same behaviour as before, except that we identify an additional console type now.\n\n3) Concerning your example: As far as i can tell from the code, nova creates all tcp based ports as \u003cserial\u003e and \u003cconsole\u003e with mode \u0027bind\u0027, not \u0027connect\u0027.\n\nExample from my running openstack-instances (with [serial_console] enabled\u003dtrue):\n    \u003cconsole type\u003d\u0027tcp\u0027\u003e\n      \u003csource mode\u003d\u0027bind\u0027 host\u003d\u002710.1.0.1\u0027 service\u003d\u002710045\u0027 tls\u003d\u0027no\u0027/\u003e\n      \u003cprotocol type\u003d\u0027raw\u0027/\u003e\n      \u003clog file\u003d\u0027/tmp/123456/console.log\u0027 append\u003d\u0027off\u0027/\u003e\n      \u003ctarget type\u003d\u0027serial\u0027 port\u003d\u00270\u0027/\u003e\n      \u003calias name\u003d\u0027serial0\u0027/\u003e\n    \u003c/console\u003e\n\nAs far as i know this port is always and directly available. Your example doesn\u0027t behave like nova-created instances and is thus irrelevant. Even if it was, the code wouldn\u0027t break, see answer 2).\n\n4) Concerning your suggestion:\nThe creation of serial consoles at instance startup is already part of nova and it works fine. It is not part of this change. The same is true for documenting the getty service in the client. These are unrelated issues.","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"a1fbb78b20d71ef6d7fbcd01b6801757645a94fd","unresolved":false,"context_lines":[{"line_number":3118,"context_line":"        # check for different types of consoles"},{"line_number":3119,"context_line":"        path_sources \u003d ["},{"line_number":3120,"context_line":"            (\u0027file\u0027, \"./devices/console[@type\u003d\u0027file\u0027]/source[@path]\", \u0027path\u0027),"},{"line_number":3121,"context_line":"            (\u0027tcp\u0027, \"./devices/console[@type\u003d\u0027tcp\u0027]/log[@file]\", \u0027file\u0027),"},{"line_number":3122,"context_line":"            (\u0027pty\u0027, \"./devices/console[@type\u003d\u0027pty\u0027]/source[@path]\", \u0027path\u0027)]"},{"line_number":3123,"context_line":"        console_type \u003d \"\""},{"line_number":3124,"context_line":"        console_path \u003d \"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_b8d6d124","line":3121,"range":{"start_line":3121,"start_character":12,"end_line":3121,"end_character":72},"in_reply_to":"3f79a3b5_6dbaf20c","updated":"2018-12-05 12:39:16.000000000","message":"Hi Georg,\n\nSorry for the terrible delay in my response.\n\n \u003e Thanks for the feedback. I\u0027ll try to answer all different aspects\n \u003e that you brought up:\n \u003e \n \u003e 1) The goal of this change is to fix the \u0027default\u0027 behaviour of tcp\n \u003e consoles in nova/kvm, see bug description. I don\u0027t want to target\n \u003e all possible cases of serial consoles in different hypervisors.\n\nYes, that is reasonable.\n\n \u003e \n \u003e 2) If the XML looks different (for whatever reason), the code won\u0027t\n \u003e break. The worst thing that could happen is that we do not find the\n \u003e console and raise a ConsoleNotAvailable() exception or return empty\n \u003e content if the backing logfile is not yet present. This is just the\n \u003e same behaviour as before, except that we identify an additional\n \u003e console type now.\n\nYeah, the XML being different isn\u0027t concern, because point (3) below.\n\n \u003e \n \u003e 3) Concerning your example: As far as i can tell from the code,\n \u003e nova creates all tcp based ports as \u003cserial\u003e and \u003cconsole\u003e with\n \u003e mode \u0027bind\u0027, not \u0027connect\u0027.\n \u003e \n \u003e Example from my running openstack-instances (with [serial_console]\n \u003e enabled\u003dtrue):\n \u003e \u003cconsole type\u003d\u0027tcp\u0027\u003e\n \u003e \u003csource mode\u003d\u0027bind\u0027 host\u003d\u002710.1.0.1\u0027 service\u003d\u002710045\u0027 tls\u003d\u0027no\u0027/\u003e\n \u003e \u003cprotocol type\u003d\u0027raw\u0027/\u003e\n \u003e \u003clog file\u003d\u0027/tmp/123456/console.log\u0027 append\u003d\u0027off\u0027/\u003e\n \u003e \u003ctarget type\u003d\u0027serial\u0027 port\u003d\u00270\u0027/\u003e\n \u003e \u003calias name\u003d\u0027serial0\u0027/\u003e\n \u003e \u003c/console\u003e\n \u003e \n \u003e As far as i know this port is always and directly available. Your\n \u003e example doesn\u0027t behave like nova-created instances and is thus\n \u003e irrelevant. Even if it was, the code wouldn\u0027t break, see answer 2).\n\nI neglected to mention that my example was from a quick test done locally (*without* Nova), just trying to remember top off my head.  Sorry about that.\n\nNow I\u0027ve done a proper DevStack test, indeed my test matches yours, so that\u0027s my primary interactive console for a Nova guest (from Git master) started w/ [serial_console]/\u0027enabled_true\u0027:\n\n    \u003cconsole type\u003d\u0027tcp\u0027\u003e\n      \u003csource mode\u003d\u0027bind\u0027 host\u003d\u0027127.0.0.1\u0027 service\u003d\u002710000\u0027 tls\u003d\u0027no\u0027/\u003e\n      \u003cprotocol type\u003d\u0027raw\u0027/\u003e\n      \u003clog file\u003d\u0027/home/stack/src/cloud/data/nova/instances/16f76aaf-67e7-4bdb-b364-d0ee4f2a08bd/console.log\u0027 append\u003d\u0027off\u0027/\u003e\n      \u003ctarget type\u003d\u0027serial\u0027 port\u003d\u00270\u0027/\u003e\n      \u003calias name\u003d\u0027serial0\u0027/\u003e\n    \u003c/console\u003e\n\n\n \u003e 4) Concerning your suggestion:\n \u003e The creation of serial consoles at instance startup is already part\n \u003e of nova and it works fine. It is not part of this change. \n\u003e The same\n \u003e is true for documenting the getty service in the client. These are\n \u003e unrelated issues.\n\nYes, in light of point (3), we can disregard the above.\n\nThanks for the detailed response.","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"f1d97c949f0863717533674d1933fe60f5426e2a","unresolved":false,"context_lines":[{"line_number":3118,"context_line":"        # check for different types of consoles"},{"line_number":3119,"context_line":"        path_sources \u003d ["},{"line_number":3120,"context_line":"            (\u0027file\u0027, \"./devices/console[@type\u003d\u0027file\u0027]/source[@path]\", \u0027path\u0027),"},{"line_number":3121,"context_line":"            (\u0027tcp\u0027, \"./devices/console[@type\u003d\u0027tcp\u0027]/log[@file]\", \u0027file\u0027),"},{"line_number":3122,"context_line":"            (\u0027pty\u0027, \"./devices/console[@type\u003d\u0027pty\u0027]/source[@path]\", \u0027path\u0027)]"},{"line_number":3123,"context_line":"        console_type \u003d \"\""},{"line_number":3124,"context_line":"        console_path \u003d \"\""}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_3b331815","line":3121,"range":{"start_line":3121,"start_character":12,"end_line":3121,"end_character":72},"in_reply_to":"3f79a3b5_db8784b3","updated":"2018-10-30 17:00:01.000000000","message":"So, IIUC, to always have a reliable log file for TCP:\n\n(a) Ensure to create the log file for TCP character device at domain define time\n\n(b)  Then the existing check (\u0027tcp\u0027, \"./devices/console[@type\u003d\u0027tcp\u0027]/log[@file]\", \u0027file\u0027) will work\n\n(c) Start \"serial-getty@ttyS0.service\" in the guest, because systemd might not start it. We can document this, though.","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"634d78d2fd63055a5bd23edaf535e7cf0cc75f37","unresolved":false,"context_lines":[{"line_number":3123,"context_line":"        console_type \u003d \"\""},{"line_number":3124,"context_line":"        console_path \u003d \"\""},{"line_number":3125,"context_line":"        for c_type, epath, attrib in path_sources:"},{"line_number":3126,"context_line":"            node \u003d tree.find(epath)"},{"line_number":3127,"context_line":"            if (node is not None) and node.get(attrib):"},{"line_number":3128,"context_line":"                console_type \u003d c_type"},{"line_number":3129,"context_line":"                console_path \u003d node.get(attrib)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_392083ae","line":3126,"range":{"start_line":3126,"start_character":12,"end_line":3126,"end_character":35},"updated":"2018-12-05 15:24:19.000000000","message":"Previously we were using \u0027findall\u0027 yet now we\u0027re using \u0027find\u0027, which naturally would break the case where there are multiple matching consoles. I don\u0027t _think_ we support multiple consoles so this kind of makes sense, but I can\u0027t say for sure and I\u0027d like this to have been called out in the commit message so I\u0027d know it was intentional.","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"},{"author":{"_account_id":28454,"name":"Georg Hoesch","email":"georg_hoesch@genua.de","username":"georghoesch"},"change_message_id":"d0535d2abb0b1cbb346ce745c1ed12152180ac92","unresolved":false,"context_lines":[{"line_number":3123,"context_line":"        console_type \u003d \"\""},{"line_number":3124,"context_line":"        console_path \u003d \"\""},{"line_number":3125,"context_line":"        for c_type, epath, attrib in path_sources:"},{"line_number":3126,"context_line":"            node \u003d tree.find(epath)"},{"line_number":3127,"context_line":"            if (node is not None) and node.get(attrib):"},{"line_number":3128,"context_line":"                console_type \u003d c_type"},{"line_number":3129,"context_line":"                console_path \u003d node.get(attrib)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_8b34de48","line":3126,"range":{"start_line":3126,"start_character":12,"end_line":3126,"end_character":35},"in_reply_to":"3f79a3b5_392083ae","updated":"2018-12-05 16:35:34.000000000","message":"Yes, this is intended.\n\n- usually only one console exists\n- if multiple consoles exist (i don\u0027t know if this can happen at all), the code implicitly picks the first one.\n\nBefore, we were walking all consoles and checking for the existance of a subnode and attribute. Findall was only needed to proceed to the next node if the first node didn\u0027t have the right subnode/attribute.\n\nThe new code uses a better xpath and selects the first matching node directly, so no findall() is needed.","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4056162bf9e623df02757e0309c9eb2da74a4434","unresolved":false,"context_lines":[{"line_number":3123,"context_line":"        console_type \u003d \"\""},{"line_number":3124,"context_line":"        console_path \u003d \"\""},{"line_number":3125,"context_line":"        for c_type, epath, attrib in path_sources:"},{"line_number":3126,"context_line":"            node \u003d tree.find(epath)"},{"line_number":3127,"context_line":"            if (node is not None) and node.get(attrib):"},{"line_number":3128,"context_line":"                console_type \u003d c_type"},{"line_number":3129,"context_line":"                console_path \u003d node.get(attrib)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_b79229fa","line":3126,"range":{"start_line":3126,"start_character":12,"end_line":3126,"end_character":35},"in_reply_to":"3f79a3b5_8b34de48","updated":"2018-12-07 17:26:12.000000000","message":"Ah, so I\u0027d figured that \u0027find\u0027 would error out if multiple entries were found, hence my \"[this] would break the case where there are multiple matching consoles\" comment. This isn\u0027t the case, as seen below. That makes this less of an issue that I\u0027d imagined and the rest of what you\u0027ve said makes sense.\n\n    \u003e\u003e\u003e from lxml import etree\n    \u003e\u003e\u003e xml \u003d \"\"\"\\\n    ... \u003ctest\u003e\n    ...     \u003chello\u003eTest\u003c/hello\u003e\n    ...     \u003chello\u003eWorld\u003c/hello\u003e\n    ... \u003c/test\u003e\n    ... \"\"\"\n    \u003e\u003e\u003e tree \u003d etree.fromstring(xml)\n    \u003e\u003e\u003e tree.find(\u0027./hello\u0027)\n    \u003cElement hello at 0x7fcc409eeb48\u003e","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"634d78d2fd63055a5bd23edaf535e7cf0cc75f37","unresolved":false,"context_lines":[{"line_number":3124,"context_line":"        console_path \u003d \"\""},{"line_number":3125,"context_line":"        for c_type, epath, attrib in path_sources:"},{"line_number":3126,"context_line":"            node \u003d tree.find(epath)"},{"line_number":3127,"context_line":"            if (node is not None) and node.get(attrib):"},{"line_number":3128,"context_line":"                console_type \u003d c_type"},{"line_number":3129,"context_line":"                console_path \u003d node.get(attrib)"},{"line_number":3130,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_ebf79227","line":3127,"range":{"start_line":3127,"start_character":15,"end_line":3127,"end_character":33},"updated":"2018-12-05 15:24:19.000000000","message":"nit: these brackets aren\u0027t needed","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"},{"author":{"_account_id":28454,"name":"Georg Hoesch","email":"georg_hoesch@genua.de","username":"georghoesch"},"change_message_id":"d0535d2abb0b1cbb346ce745c1ed12152180ac92","unresolved":false,"context_lines":[{"line_number":3124,"context_line":"        console_path \u003d \"\""},{"line_number":3125,"context_line":"        for c_type, epath, attrib in path_sources:"},{"line_number":3126,"context_line":"            node \u003d tree.find(epath)"},{"line_number":3127,"context_line":"            if (node is not None) and node.get(attrib):"},{"line_number":3128,"context_line":"                console_type \u003d c_type"},{"line_number":3129,"context_line":"                console_path \u003d node.get(attrib)"},{"line_number":3130,"context_line":"                break"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_e126d16a","line":3127,"range":{"start_line":3127,"start_character":15,"end_line":3127,"end_character":33},"in_reply_to":"3f79a3b5_ebf79227","updated":"2018-12-05 16:35:34.000000000","message":"technically not needed but easier to understand (from my point of view). But I will fix the others if we have another changeset.","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e63936172cb432eec09558d418c34c612a0b1d69","unresolved":false,"context_lines":[{"line_number":3126,"context_line":"            node \u003d tree.find(epath)"},{"line_number":3127,"context_line":"            if (node is not None) and node.get(attrib):"},{"line_number":3128,"context_line":"                console_type \u003d c_type"},{"line_number":3129,"context_line":"                console_path \u003d node.get(attrib)"},{"line_number":3130,"context_line":"                break"},{"line_number":3131,"context_line":""},{"line_number":3132,"context_line":"        # instance has no console at all"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_b8acf5a8","line":3129,"range":{"start_line":3129,"start_character":16,"end_line":3129,"end_character":47},"updated":"2018-11-30 10:13:34.000000000","message":"this is safe now as the xpath only matches if the attrib also exists in the xml.","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f340bb5d160516ba99e7fe037c671bcc8965bf5f","unresolved":false,"context_lines":[{"line_number":3127,"context_line":"            if (node is not None) and node.get(attrib):"},{"line_number":3128,"context_line":"                console_type \u003d c_type"},{"line_number":3129,"context_line":"                console_path \u003d node.get(attrib)"},{"line_number":3130,"context_line":"                break"},{"line_number":3131,"context_line":""},{"line_number":3132,"context_line":"        # instance has no console at all"},{"line_number":3133,"context_line":"        if (console_path \u003d\u003d \"\"):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_d0e2729c","line":3130,"range":{"start_line":3130,"start_character":16,"end_line":3130,"end_character":21},"updated":"2018-10-11 23:20:22.000000000","message":"for other reviews this break is for the if above\ni was going to comment and say you should add one :)","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"634d78d2fd63055a5bd23edaf535e7cf0cc75f37","unresolved":false,"context_lines":[{"line_number":3130,"context_line":"                break"},{"line_number":3131,"context_line":""},{"line_number":3132,"context_line":"        # instance has no console at all"},{"line_number":3133,"context_line":"        if (console_path \u003d\u003d \"\"):"},{"line_number":3134,"context_line":"            raise exception.ConsoleNotAvailable()"},{"line_number":3135,"context_line":""},{"line_number":3136,"context_line":"        # instance has a console, but file doesn\u0027t exist (yet?)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_5958df56","line":3133,"range":{"start_line":3133,"start_character":8,"end_line":3133,"end_character":32},"updated":"2018-12-05 15:24:19.000000000","message":"nit:\n\n    if not console_path:\n\nwould be more succinct","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"634d78d2fd63055a5bd23edaf535e7cf0cc75f37","unresolved":false,"context_lines":[{"line_number":3133,"context_line":"        if (console_path \u003d\u003d \"\"):"},{"line_number":3134,"context_line":"            raise exception.ConsoleNotAvailable()"},{"line_number":3135,"context_line":""},{"line_number":3136,"context_line":"        # instance has a console, but file doesn\u0027t exist (yet?)"},{"line_number":3137,"context_line":"        if not os.path.exists(console_path):"},{"line_number":3138,"context_line":"            LOG.info(\u0027console logfile for instance does not exist\u0027,"},{"line_number":3139,"context_line":"                      instance\u003dinstance)"},{"line_number":3140,"context_line":"            return \"\""},{"line_number":3141,"context_line":""},{"line_number":3142,"context_line":"        # pty consoles need special handling"},{"line_number":3143,"context_line":"        if (console_type \u003d\u003d \u0027pty\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_796cbb04","line":3140,"range":{"start_line":3136,"start_character":0,"end_line":3140,"end_character":21},"updated":"2018-12-05 15:24:19.000000000","message":"Another change in behavior. We didn\u0027t previously do this for pty consoles, right? If so, is this a mistake or should we have been doing it all along?","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"},{"author":{"_account_id":28454,"name":"Georg Hoesch","email":"georg_hoesch@genua.de","username":"georghoesch"},"change_message_id":"d0535d2abb0b1cbb346ce745c1ed12152180ac92","unresolved":false,"context_lines":[{"line_number":3133,"context_line":"        if (console_path \u003d\u003d \"\"):"},{"line_number":3134,"context_line":"            raise exception.ConsoleNotAvailable()"},{"line_number":3135,"context_line":""},{"line_number":3136,"context_line":"        # instance has a console, but file doesn\u0027t exist (yet?)"},{"line_number":3137,"context_line":"        if not os.path.exists(console_path):"},{"line_number":3138,"context_line":"            LOG.info(\u0027console logfile for instance does not exist\u0027,"},{"line_number":3139,"context_line":"                      instance\u003dinstance)"},{"line_number":3140,"context_line":"            return \"\""},{"line_number":3141,"context_line":""},{"line_number":3142,"context_line":"        # pty consoles need special handling"},{"line_number":3143,"context_line":"        if (console_type \u003d\u003d \u0027pty\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_8e7e2cbe","line":3140,"range":{"start_line":3136,"start_character":0,"end_line":3140,"end_character":21},"in_reply_to":"3f79a3b5_796cbb04","updated":"2018-12-05 16:35:34.000000000","message":"Right, this was not part of the pty case before. I do not know whether this case cannot happen with pty consoles or if pty consoles trigger the crude exception handling in readpty() that returns an empty string. In the first case, the check doesn\u0027t hurt, in the latter it doesn\u0027t change the functional behaviour, just the log message.\n\nThe check might be skipped for pty consoles but I assumed that one simple and consistent check and log message for all cases is an advantage. Please tell me if this is not the case.","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4056162bf9e623df02757e0309c9eb2da74a4434","unresolved":false,"context_lines":[{"line_number":3133,"context_line":"        if (console_path \u003d\u003d \"\"):"},{"line_number":3134,"context_line":"            raise exception.ConsoleNotAvailable()"},{"line_number":3135,"context_line":""},{"line_number":3136,"context_line":"        # instance has a console, but file doesn\u0027t exist (yet?)"},{"line_number":3137,"context_line":"        if not os.path.exists(console_path):"},{"line_number":3138,"context_line":"            LOG.info(\u0027console logfile for instance does not exist\u0027,"},{"line_number":3139,"context_line":"                      instance\u003dinstance)"},{"line_number":3140,"context_line":"            return \"\""},{"line_number":3141,"context_line":""},{"line_number":3142,"context_line":"        # pty consoles need special handling"},{"line_number":3143,"context_line":"        if (console_type \u003d\u003d \u0027pty\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_97292da3","line":3140,"range":{"start_line":3136,"start_character":0,"end_line":3140,"end_character":21},"in_reply_to":"3f79a3b5_8e7e2cbe","updated":"2018-12-07 17:26:12.000000000","message":"++ thanks for the explanation","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"634d78d2fd63055a5bd23edaf535e7cf0cc75f37","unresolved":false,"context_lines":[{"line_number":3140,"context_line":"            return \"\""},{"line_number":3141,"context_line":""},{"line_number":3142,"context_line":"        # pty consoles need special handling"},{"line_number":3143,"context_line":"        if (console_type \u003d\u003d \u0027pty\u0027):"},{"line_number":3144,"context_line":"            console_log \u003d self._get_console_log_path(instance)"},{"line_number":3145,"context_line":"            data \u003d nova.privsep.libvirt.readpty(console_path)"},{"line_number":3146,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_ebca32a1","line":3143,"range":{"start_line":3143,"start_character":11,"end_line":3143,"end_character":35},"updated":"2018-12-05 15:24:19.000000000","message":"nit: brackets also not needed here","commit_id":"1a12829dfab5d1ffc5d5560c6e41533521ac4e44"}]}