)]}'
{"bin/neutron-dhcp-agent-dnsmasq-lease-init":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"96b184633dd71b3bda23617c57ce40c4131e785d","unresolved":false,"context_lines":[{"line_number":15,"context_line":"#    under the License."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from neutron.agent.linux import dhcp"},{"line_number":18,"context_line":"dhcp.Dnsmasq.lease_init()"}],"source_content_type":"application/octet-stream","patch_set":5,"id":"3a961159_70ffc8ab","line":18,"updated":"2015-01-05 21:32:15.000000000","message":"Can you make sure the execute permissions are set correctly on this script in your repository?  When I pulled this change down to take a look the resulting file showed:\n\n-rw-r--r--  1 haley haley  725 Jan  5 14:52 neutron-dhcp-agent-dnsmasq-lease-init","commit_id":"4230a09f0ad556d20a3db09e58113cb9276738f9"},{"author":{"_account_id":5115,"name":"Mike King","email":"mike.king@hp.com","username":"mike-king"},"change_message_id":"abc2c44b4fb3bb105e27f68f26cc3c88ebe7d49a","unresolved":false,"context_lines":[{"line_number":15,"context_line":"#    under the License."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from neutron.agent.linux import dhcp"},{"line_number":18,"context_line":"dhcp.Dnsmasq.lease_init()"}],"source_content_type":"application/octet-stream","patch_set":5,"id":"3a961159_096381b1","line":18,"updated":"2015-01-07 18:00:09.000000000","message":"I don\u0027t think this script is necessary at all if you use Brian\u0027s suggestion of moving it into console_scripts in setup.cfg","commit_id":"4230a09f0ad556d20a3db09e58113cb9276738f9"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"8de116ac9e6f2c607312c85a9837b8116d4a52b1","unresolved":false,"context_lines":[{"line_number":15,"context_line":"#    under the License."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"from neutron.agent.linux import dhcp"},{"line_number":18,"context_line":"dhcp.Dnsmasq.lease_init()"}],"source_content_type":"application/octet-stream","patch_set":5,"id":"3a961159_448039bf","line":18,"in_reply_to":"3a961159_70ffc8ab","updated":"2015-01-06 13:42:12.000000000","message":"Brian, you are right that I didn\u0027t set mode to x. But it should not matter because setup will change it to x mode automatically when doing pip install. But I will change it so as to be consistent with other files.","commit_id":"4230a09f0ad556d20a3db09e58113cb9276738f9"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"690e49abce39be6182a97331c3196fba46748020","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#!/bin/bash"},{"line_number":2,"context_line":"# Copyright (c) 2012 OpenStack Foundation."},{"line_number":3,"context_line":"# All Rights Reserved."},{"line_number":4,"context_line":"#"}],"source_content_type":"application/x-shellscript","patch_set":8,"id":"fa81d914_77ce3085","line":1,"updated":"2015-01-28 18:53:30.000000000","message":"Personally, I\u0027m okay with bash.  We have bash scripts in the repository already and it is everywhere on every unix I\u0027ve ever used.\n\nI think the problem would be if the script used bashisms and did not call out bash specifically as the interpreter here as you\u0027ve done.  So, I\u0027d say leave it as it is.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"3c0639f57f84266c2adf184e8d0cf8f4580bde66","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#!/bin/bash"},{"line_number":2,"context_line":"# Copyright (c) 2012 OpenStack Foundation."},{"line_number":3,"context_line":"# All Rights Reserved."},{"line_number":4,"context_line":"#"}],"source_content_type":"application/x-shellscript","patch_set":8,"id":"fa81d914_4abdda67","line":1,"in_reply_to":"fa81d914_77ce3085","updated":"2015-01-29 09:48:58.000000000","message":"Though I agree we have bash scripts in the tree, it\u0027s a bit different since those scripts are not part of production code but just tools, test hooks etc.\n\nTo be on completely safe side, I guess we will need to add a sanity check for that (I\u0027ll handle it in a separate patch though).","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"cd46da59994126408e1b04555a095d3d39020bd1","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#!/bin/bash"},{"line_number":2,"context_line":"# Copyright (c) 2012 OpenStack Foundation."},{"line_number":3,"context_line":"# All Rights Reserved."},{"line_number":4,"context_line":"#"},{"line_number":5,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"}],"source_content_type":"application/x-shellscript","patch_set":8,"id":"1a930d6b_b711cc5e","line":2,"updated":"2015-01-27 19:47:08.000000000","message":"2015","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"3b0d9052a477dbaca5754121d165031a61dd55a0","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#!/bin/bash"},{"line_number":2,"context_line":"# Copyright (c) 2012 OpenStack Foundation."},{"line_number":3,"context_line":"# All Rights Reserved."},{"line_number":4,"context_line":"#"},{"line_number":5,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"}],"source_content_type":"application/x-shellscript","patch_set":8,"id":"fa81d914_9925c70c","line":2,"in_reply_to":"1a930d6b_b711cc5e","updated":"2015-01-27 20:43:37.000000000","message":"Since I am the author og the file, I am pretty sure I don\u0027t work for foundation, so please remove the whole line.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"2091d71de0861ed78ff00d07be256fd51d6a031a","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#!/bin/bash"},{"line_number":2,"context_line":"# Copyright (c) 2012 OpenStack Foundation."},{"line_number":3,"context_line":"# All Rights Reserved."},{"line_number":4,"context_line":"#"},{"line_number":5,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"}],"source_content_type":"application/x-shellscript","patch_set":8,"id":"fa81d914_d53c4296","line":2,"in_reply_to":"fa81d914_9925c70c","updated":"2015-01-28 13:05:22.000000000","message":"Can we use this template without the Copyright line? \n\nI thought by default all people can contribute on be half of OpenStack Foundation.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"8b4abc1e256d35684a54180f75ab8efb39166ea3","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#!/bin/bash"},{"line_number":2,"context_line":"# Copyright (c) 2012 OpenStack Foundation."},{"line_number":3,"context_line":"# All Rights Reserved."},{"line_number":4,"context_line":"#"},{"line_number":5,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"}],"source_content_type":"application/x-shellscript","patch_set":8,"id":"fa81d914_0084d6d2","line":2,"in_reply_to":"fa81d914_d53c4296","updated":"2015-01-28 13:14:03.000000000","message":"\"It\u0027s likely the only valid reason for OpenStack Foundation copyright notices on new code these days is where the code was authored by an employee or contractor of the foundation.\"\n\nQuoting from https://wiki.openstack.org/wiki/LegalIssuesFAQ#Copyright_Headers","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"cd46da59994126408e1b04555a095d3d39020bd1","unresolved":false,"context_lines":[{"line_number":17,"context_line":"LEASE_EXP_MAX\u003d$(((2100 - 1970) * 365 * 24 * 3600))"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"function get_dhcp_host_id() {"},{"line_number":20,"context_line":"\tipaddr\u003d$1"},{"line_number":21,"context_line":"\tipaddr\u003d${ipaddr//./-}"},{"line_number":22,"context_line":"\tipaddr\u003d${ipaddr//:/-}"},{"line_number":23,"context_line":"\techo \"host-$ipaddr\""}],"source_content_type":"application/x-shellscript","patch_set":8,"id":"1a930d6b_5700a801","line":20,"updated":"2015-01-27 19:47:08.000000000","message":"Maybe use 4 spaces instead of tabs.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5aaf03578893d3745fa8bdd539e86eb44c33ace3","unresolved":false,"context_lines":[{"line_number":23,"context_line":"\techo \"host-$ipaddr\""},{"line_number":24,"context_line":"}"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"function strip_braces() {"},{"line_number":27,"context_line":"\tipaddr\u003d$1"},{"line_number":28,"context_line":"\tipaddr\u003d${ipaddr//[/}"},{"line_number":29,"context_line":"\tipaddr\u003d${ipaddr//]/}"}],"source_content_type":"application/x-shellscript","patch_set":8,"id":"fa81d914_79cf33b1","line":26,"updated":"2015-01-27 20:42:16.000000000","message":"Nit: function keyword is redundant (and bashism) -\u003e remove","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"cd46da59994126408e1b04555a095d3d39020bd1","unresolved":false,"context_lines":[{"line_number":30,"context_line":"\techo \"$ipaddr\""},{"line_number":31,"context_line":"}"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"if [ \"x$1\" \u003d \"xinit\" ]; then"},{"line_number":34,"context_line":"\tif [ -n \"$HOSTS_FILE\" ]; then"},{"line_number":35,"context_line":"\t\tif [ -e \"$HOSTS_FILE\" ]; then"},{"line_number":36,"context_line":"\t\t\tcat $HOSTS_FILE | while read -r line"}],"source_content_type":"application/x-shellscript","patch_set":8,"id":"fa81d914_90883cc8","line":33,"updated":"2015-01-27 19:47:08.000000000","message":"How about moving this to the top as the first statement to make it clear and reduce the indentation level of the rest of this stuff.\n\n  # top of the script.\n  if [ \"$1\" !\u003d \"init\" ]; then\n      exit\n  fi\n\nAlso, this business about adding an \u0027x\u0027 to the front of both values for comparison is completely unnecessary and antiquated in bash where you\u0027ve quoted the values.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5aaf03578893d3745fa8bdd539e86eb44c33ace3","unresolved":false,"context_lines":[{"line_number":30,"context_line":"\techo \"$ipaddr\""},{"line_number":31,"context_line":"}"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"if [ \"x$1\" \u003d \"xinit\" ]; then"},{"line_number":34,"context_line":"\tif [ -n \"$HOSTS_FILE\" ]; then"},{"line_number":35,"context_line":"\t\tif [ -e \"$HOSTS_FILE\" ]; then"},{"line_number":36,"context_line":"\t\t\tcat $HOSTS_FILE | while read -r line"}],"source_content_type":"application/x-shellscript","patch_set":8,"id":"fa81d914_d9549fcc","line":33,"in_reply_to":"fa81d914_90883cc8","updated":"2015-01-27 20:42:16.000000000","message":"Yes, x should be removed. It was relevant 30 years ago for some broken shells but not now.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"cd46da59994126408e1b04555a095d3d39020bd1","unresolved":false,"context_lines":[{"line_number":31,"context_line":"}"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"if [ \"x$1\" \u003d \"xinit\" ]; then"},{"line_number":34,"context_line":"\tif [ -n \"$HOSTS_FILE\" ]; then"},{"line_number":35,"context_line":"\t\tif [ -e \"$HOSTS_FILE\" ]; then"},{"line_number":36,"context_line":"\t\t\tcat $HOSTS_FILE | while read -r line"},{"line_number":37,"context_line":"\t\t\tdo"}],"source_content_type":"application/x-shellscript","patch_set":8,"id":"fa81d914_f0a6d852","line":34,"updated":"2015-01-27 19:47:08.000000000","message":"nit:  This is redundant.  The next statement will not succeed if this string is empty:\n\n  $ [ -e \"\" ]\n  $ echo $?\n  1","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"cd46da59994126408e1b04555a095d3d39020bd1","unresolved":false,"context_lines":[{"line_number":33,"context_line":"if [ \"x$1\" \u003d \"xinit\" ]; then"},{"line_number":34,"context_line":"\tif [ -n \"$HOSTS_FILE\" ]; then"},{"line_number":35,"context_line":"\t\tif [ -e \"$HOSTS_FILE\" ]; then"},{"line_number":36,"context_line":"\t\t\tcat $HOSTS_FILE | while read -r line"},{"line_number":37,"context_line":"\t\t\tdo"},{"line_number":38,"context_line":"\t\t\t\tIFS\u003d\u0027,\u0027 read -ra VALUES \u003c\u003c\u003c \"$line\""},{"line_number":39,"context_line":"\t\t\t\tmac\u003d${VALUES[0]}"}],"source_content_type":"application/x-shellscript","patch_set":8,"id":"fa81d914_937be628","line":36,"updated":"2015-01-27 19:47:08.000000000","message":"Would this work, I think it reads better:\n\n IFS\u003d,\n while read -r mac name ipaddr extra\n do\n     ipaddr\u003d$(strip_braces $ipaddr)\n     hostname\u003d$(get_dhcp_host_id $ipaddr)\n     echo \"$LEASE_EXP_MAX $mac $ipaddr $hostname *\"\n done \u003c $HOSTSFILE","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"690e49abce39be6182a97331c3196fba46748020","unresolved":false,"context_lines":[{"line_number":33,"context_line":"if [ \"x$1\" \u003d \"xinit\" ]; then"},{"line_number":34,"context_line":"\tif [ -n \"$HOSTS_FILE\" ]; then"},{"line_number":35,"context_line":"\t\tif [ -e \"$HOSTS_FILE\" ]; then"},{"line_number":36,"context_line":"\t\t\tcat $HOSTS_FILE | while read -r line"},{"line_number":37,"context_line":"\t\t\tdo"},{"line_number":38,"context_line":"\t\t\t\tIFS\u003d\u0027,\u0027 read -ra VALUES \u003c\u003c\u003c \"$line\""},{"line_number":39,"context_line":"\t\t\t\tmac\u003d${VALUES[0]}"}],"source_content_type":"application/x-shellscript","patch_set":8,"id":"fa81d914_74770ed5","line":36,"in_reply_to":"fa81d914_39d85b53","updated":"2015-01-28 18:53:30.000000000","message":"As long as the script should ignore the extra then this works.  The \"extra\" field is a catch all to the end of the line.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5aaf03578893d3745fa8bdd539e86eb44c33ace3","unresolved":false,"context_lines":[{"line_number":33,"context_line":"if [ \"x$1\" \u003d \"xinit\" ]; then"},{"line_number":34,"context_line":"\tif [ -n \"$HOSTS_FILE\" ]; then"},{"line_number":35,"context_line":"\t\tif [ -e \"$HOSTS_FILE\" ]; then"},{"line_number":36,"context_line":"\t\t\tcat $HOSTS_FILE | while read -r line"},{"line_number":37,"context_line":"\t\t\tdo"},{"line_number":38,"context_line":"\t\t\t\tIFS\u003d\u0027,\u0027 read -ra VALUES \u003c\u003c\u003c \"$line\""},{"line_number":39,"context_line":"\t\t\t\tmac\u003d${VALUES[0]}"}],"source_content_type":"application/x-shellscript","patch_set":8,"id":"fa81d914_39d85b53","line":36,"in_reply_to":"fa81d914_937be628","updated":"2015-01-27 20:42:16.000000000","message":"We will need to make sure we properly update the script once we get lines of variable entry number (as in a parallel patch for dhcp options for stateless subnets).","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"cd46da59994126408e1b04555a095d3d39020bd1","unresolved":false,"context_lines":[{"line_number":40,"context_line":"\t\t\t\tipaddr\u003d$(strip_braces ${VALUES[2]})"},{"line_number":41,"context_line":"\t\t\t\thostname\u003d$(get_dhcp_host_id $ipaddr)"},{"line_number":42,"context_line":"\t\t\t\techo \"$LEASE_EXP_MAX $mac $ipaddr $hostname *\""},{"line_number":43,"context_line":"\t\t\tdone"},{"line_number":44,"context_line":"\t\tfi"},{"line_number":45,"context_line":"\tfi"},{"line_number":46,"context_line":"fi"}],"source_content_type":"application/x-shellscript","patch_set":8,"id":"fa81d914_93e68615","line":43,"updated":"2015-01-27 19:47:08.000000000","message":"Since we\u0027re nit-picking the performance of this thing:\n\nInstead of \"cat $HOSTS_FILE |\" above, redirect from the file here to avoid a pipe:\n\nwhile read -r ...\ndo\n    ...\ndone \u003c $HOSTS_FILE","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"}],"etc/neutron/rootwrap.d/dhcp.filters":[{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"0887ef480ef45522831a108453025d54165a9efc","unresolved":false,"context_lines":[{"line_number":9,"context_line":"[Filters]"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"# dhcp-agent"},{"line_number":12,"context_line":"dnsmasq: EnvFilter, dnsmasq, root, HOSTS_FILE\u003d, NEUTRON_NETWORK_ID\u003d"},{"line_number":13,"context_line":"# dhcp-agent uses kill as well, that\u0027s handled by the generic KillFilter"},{"line_number":14,"context_line":"# it looks like these are the only signals needed, per"},{"line_number":15,"context_line":"# neutron/agent/linux/dhcp.py"}],"source_content_type":"application/octet-stream","patch_set":11,"id":"da86d52c_ee7252e4","line":12,"updated":"2015-02-02 10:15:20.000000000","message":"NEUTRON_NETWORK_ID may be removed,\n\nAfter this file is changed by an upgrade the dhcp-agent should be restarted quickly, so it shouldn\u0027t be an issue to running environments.\n\nI would remove NEUTRON_NETWORK_ID in this same patch as we are not passing it anymore.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"20d594f843d538667719672a1d9e54393b67d5cf","unresolved":false,"context_lines":[{"line_number":9,"context_line":"[Filters]"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"# dhcp-agent"},{"line_number":12,"context_line":"dnsmasq: EnvFilter, dnsmasq, root, HOSTS_FILE\u003d, NEUTRON_NETWORK_ID\u003d"},{"line_number":13,"context_line":"# dhcp-agent uses kill as well, that\u0027s handled by the generic KillFilter"},{"line_number":14,"context_line":"# it looks like these are the only signals needed, per"},{"line_number":15,"context_line":"# neutron/agent/linux/dhcp.py"}],"source_content_type":"application/octet-stream","patch_set":11,"id":"da86d52c_32ed9fc4","line":12,"in_reply_to":"da86d52c_5d81528e","updated":"2015-02-02 17:59:04.000000000","message":"Agree with @Ihar.  Let\u0027s remove it in a separate patch.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"6e077ac68f72de52e427e06bdb6320dd985c4c6a","unresolved":false,"context_lines":[{"line_number":9,"context_line":"[Filters]"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"# dhcp-agent"},{"line_number":12,"context_line":"dnsmasq: EnvFilter, dnsmasq, root, HOSTS_FILE\u003d, NEUTRON_NETWORK_ID\u003d"},{"line_number":13,"context_line":"# dhcp-agent uses kill as well, that\u0027s handled by the generic KillFilter"},{"line_number":14,"context_line":"# it looks like these are the only signals needed, per"},{"line_number":15,"context_line":"# neutron/agent/linux/dhcp.py"}],"source_content_type":"application/octet-stream","patch_set":11,"id":"da86d52c_fdcf66ae","line":12,"in_reply_to":"da86d52c_ee7252e4","updated":"2015-02-02 12:34:37.000000000","message":"I disagree. Cleanup belongs to a separate patch.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"15f0e8a699796be47c07fb6a2bc2c81efe95e053","unresolved":false,"context_lines":[{"line_number":9,"context_line":"[Filters]"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":"# dhcp-agent"},{"line_number":12,"context_line":"dnsmasq: EnvFilter, dnsmasq, root, HOSTS_FILE\u003d, NEUTRON_NETWORK_ID\u003d"},{"line_number":13,"context_line":"# dhcp-agent uses kill as well, that\u0027s handled by the generic KillFilter"},{"line_number":14,"context_line":"# it looks like these are the only signals needed, per"},{"line_number":15,"context_line":"# neutron/agent/linux/dhcp.py"}],"source_content_type":"application/octet-stream","patch_set":11,"id":"da86d52c_5d81528e","line":12,"in_reply_to":"da86d52c_fdcf66ae","updated":"2015-02-02 12:43:45.000000000","message":"On a later comment I actually said that, removing it could be done on a 2nd patch to limit the scope of this one.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"}],"neutron/agent/linux/dhcp.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"f585f281e63ffbc5bcef05cbc57f3b29f66b78cc","unresolved":false,"context_lines":[{"line_number":354,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":355,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":356,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":357,"context_line":"            \u0027--dhcp-leasefile\u003d%s\u0027 % self.get_conf_file_name(\u0027lease\u0027),"},{"line_number":358,"context_line":"        ]"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        possible_leases \u003d 0"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_4321fe9c","line":357,"updated":"2014-07-24 21:36:05.000000000","message":"What happens when the dhcp server is moved to another network node, then moved back?  Will it be using a lease file that is stale now?","commit_id":"89aa0a0d958a50e2d3c511e234bc40667cbe6697"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"8a0634ed43d997f2d020eb09aaa66c20c7f6c625","unresolved":false,"context_lines":[{"line_number":354,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":355,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":356,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":357,"context_line":"            \u0027--dhcp-leasefile\u003d%s\u0027 % self.get_conf_file_name(\u0027lease\u0027),"},{"line_number":358,"context_line":"        ]"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        possible_leases \u003d 0"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_661e4265","line":357,"in_reply_to":"5a890539_14889e1e","updated":"2014-11-13 14:37:41.000000000","message":"The traffic of db updating should not be high because lease time is relatively long and renew happens infrequently if successful. \n\nHowever, I agree with Carl that it is even better to fake it. The consequence is that all the renews will be successful, which doesn\u0027t hurt. \n\nThe point of renewing in neutron scenario is not to get IP reserved, but to update dhcp options in case there are changes. Client will still behave the same way: renew when half of lease time is reached, and the lease offered to client will not change because of this faked expire lease. In fact, I had a test and dnsmasq will restore the expire time to the correct one upon the first renew from client. So I agree with this solution: fake the lease expire upon dnsmasq start.\n\nI will be working on it.","commit_id":"89aa0a0d958a50e2d3c511e234bc40667cbe6697"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"6f55da7e73ead81bfdc4b625987a55765f14d3f4","unresolved":false,"context_lines":[{"line_number":354,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":355,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":356,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":357,"context_line":"            \u0027--dhcp-leasefile\u003d%s\u0027 % self.get_conf_file_name(\u0027lease\u0027),"},{"line_number":358,"context_line":"        ]"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        possible_leases \u003d 0"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_318b88c4","line":357,"in_reply_to":"5a890539_661e4265","updated":"2014-11-13 20:16:11.000000000","message":"+1  I agree with what you\u0027ve said.  I\u0027ll keep an eye on this review.  Thanks for your work here.","commit_id":"89aa0a0d958a50e2d3c511e234bc40667cbe6697"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e612c64fa69e11a844689cb96137e49a043dc12b","unresolved":false,"context_lines":[{"line_number":354,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":355,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":356,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":357,"context_line":"            \u0027--dhcp-leasefile\u003d%s\u0027 % self.get_conf_file_name(\u0027lease\u0027),"},{"line_number":358,"context_line":"        ]"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        possible_leases \u003d 0"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_14889e1e","line":357,"in_reply_to":"7a8c0949_1684d50a","updated":"2014-11-10 23:58:26.000000000","message":"I thought about this some more over the week-end.  I don\u0027t want to store the lease information in the database.  There is just too much traffic needed to update the lease in the database each time it changes.  Also, I don\u0027t think we need to tell dnsmasq the exact same lease expiration time that it had before restart.  I think we can fake it.\n\nWe could use the dhcp-script option like this.  Take the current time and add dnsmasq_lease_max to ensure that any valid lease that a client tries to renew is still found to be valid by dnsmasq.  I don\u0027t think it will make any difference, I don\u0027t think the client sends the actual lease expiration time with the DHCPRequest so there is no way that dnsmasq will know the difference is there?","commit_id":"89aa0a0d958a50e2d3c511e234bc40667cbe6697"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"642e29ced45b4a6afb45fde7497f721d2131efe3","unresolved":false,"context_lines":[{"line_number":354,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":355,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":356,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":357,"context_line":"            \u0027--dhcp-leasefile\u003d%s\u0027 % self.get_conf_file_name(\u0027lease\u0027),"},{"line_number":358,"context_line":"        ]"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        possible_leases \u003d 0"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_76227902","line":357,"in_reply_to":"7a8c0949_294521b3","updated":"2014-11-06 15:54:04.000000000","message":"Shared storage is not to fix the bug mentioned by this patch, it is to fix a new bug (which is not reported yet).\n\nThis fix itself is to solve the problem of lease lost upon dnsmasq restart. \n\nI admit that the problem of lease lost upon failover is not solved by this patch, but it is not introduced by this patch either.\n\nIt would be ideal if we could fix all the related issues in one patch, but this is not the one.","commit_id":"89aa0a0d958a50e2d3c511e234bc40667cbe6697"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"5e2aa2ecdff7bd0f803576278ea6d6d4c4c93e0c","unresolved":false,"context_lines":[{"line_number":354,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":355,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":356,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":357,"context_line":"            \u0027--dhcp-leasefile\u003d%s\u0027 % self.get_conf_file_name(\u0027lease\u0027),"},{"line_number":358,"context_line":"        ]"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        possible_leases \u003d 0"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_fe7d5f6f","line":357,"in_reply_to":"7a8c0949_294521b3","updated":"2014-11-06 16:41:36.000000000","message":"To solve the failover problem (or DHCP node migration problem), we should store the lease in database and update when clients renew it, but I would like to know in the first place why the DB stuff was completely removed when fixing bug 1202392?","commit_id":"89aa0a0d958a50e2d3c511e234bc40667cbe6697"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"0cbafb098a6db22ffbac0c5afbacf6a462465f1b","unresolved":false,"context_lines":[{"line_number":354,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":355,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":356,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":357,"context_line":"            \u0027--dhcp-leasefile\u003d%s\u0027 % self.get_conf_file_name(\u0027lease\u0027),"},{"line_number":358,"context_line":"        ]"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        possible_leases \u003d 0"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_497a6df2","line":357,"in_reply_to":"7a8c0949_294521b3","updated":"2014-11-03 14:40:09.000000000","message":"What is the penalty for a DHCP being moved to another node?  Maybe we can just absorb that cost in order to optimize the happy case where a VM is always served by the same network node?","commit_id":"89aa0a0d958a50e2d3c511e234bc40667cbe6697"},{"author":{"_account_id":12336,"name":"kazuhiro MIYASHITA","email":"miyakz@jp.fujitsu.com","username":"miyakz"},"change_message_id":"60eefbf6a37c5f2336783ce6020190b9bc2fc8e8","unresolved":false,"context_lines":[{"line_number":354,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":355,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":356,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":357,"context_line":"            \u0027--dhcp-leasefile\u003d%s\u0027 % self.get_conf_file_name(\u0027lease\u0027),"},{"line_number":358,"context_line":"        ]"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        possible_leases \u003d 0"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_d56d0e34","line":357,"in_reply_to":"7a8c0949_294521b3","updated":"2014-11-04 00:20:06.000000000","message":"dnsmasq returns DHCPNAK to VM even though IP address should be still leased. and result, VM\u0027s dhclient flushes its IP address and statinc routing information.","commit_id":"89aa0a0d958a50e2d3c511e234bc40667cbe6697"},{"author":{"_account_id":12336,"name":"kazuhiro MIYASHITA","email":"miyakz@jp.fujitsu.com","username":"miyakz"},"change_message_id":"60eefbf6a37c5f2336783ce6020190b9bc2fc8e8","unresolved":false,"context_lines":[{"line_number":354,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":355,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":356,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":357,"context_line":"            \u0027--dhcp-leasefile\u003d%s\u0027 % self.get_conf_file_name(\u0027lease\u0027),"},{"line_number":358,"context_line":"        ]"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        possible_leases \u003d 0"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_354ad2ac","line":357,"in_reply_to":"7a8c0949_294521b3","updated":"2014-11-04 00:20:06.000000000","message":"ports information is useful for this situation. when booting dhcp-agent, getting ports information from neutron-server and generates lease entry from each ports as if port\u0027s IP address was already leased.","commit_id":"89aa0a0d958a50e2d3c511e234bc40667cbe6697"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"642e29ced45b4a6afb45fde7497f721d2131efe3","unresolved":false,"context_lines":[{"line_number":354,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":355,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":356,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":357,"context_line":"            \u0027--dhcp-leasefile\u003d%s\u0027 % self.get_conf_file_name(\u0027lease\u0027),"},{"line_number":358,"context_line":"        ]"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        possible_leases \u003d 0"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_18cfa3b1","line":357,"in_reply_to":"7a8c0949_d56d0e34","updated":"2014-11-06 15:54:04.000000000","message":"Yes, and finally, dhcp clients will send DISCOVER with broadcast, and then dnsmasq will give offer, and everything would be fine.\n\nSo the major penalty is the frequent retries from all clients and the flooding warning logs.","commit_id":"89aa0a0d958a50e2d3c511e234bc40667cbe6697"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"27e8b1e31148c8e48553988b2ba68c813ce38805","unresolved":false,"context_lines":[{"line_number":354,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":355,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":356,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":357,"context_line":"            \u0027--dhcp-leasefile\u003d%s\u0027 % self.get_conf_file_name(\u0027lease\u0027),"},{"line_number":358,"context_line":"        ]"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        possible_leases \u003d 0"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_294521b3","line":357,"in_reply_to":"9aa7fdbe_55087c28","updated":"2014-11-03 14:39:21.000000000","message":"With distributed routing, this could mean shared storage between every compute node in the deployment.  Even if it is only the network nodes, shared storage seems like a pretty big cost to pay to fix this problem.\n\n(half-baked idea) Would it be possible to change the lease time so that it is something predictable or deterministic enough that we could generate the lease from other information known to the database?","commit_id":"89aa0a0d958a50e2d3c511e234bc40667cbe6697"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"642e29ced45b4a6afb45fde7497f721d2131efe3","unresolved":false,"context_lines":[{"line_number":354,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":355,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":356,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":357,"context_line":"            \u0027--dhcp-leasefile\u003d%s\u0027 % self.get_conf_file_name(\u0027lease\u0027),"},{"line_number":358,"context_line":"        ]"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        possible_leases \u003d 0"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a8c0949_1684d50a","line":357,"in_reply_to":"9aa7fdbe_55087c28","updated":"2014-11-06 15:54:04.000000000","message":"Yes you could do that, but it is to solve the problem of failover, so it is not mandatory for this fix.","commit_id":"89aa0a0d958a50e2d3c511e234bc40667cbe6697"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"44a344dcf7833fc27dd57625492915e18a830a47","unresolved":false,"context_lines":[{"line_number":354,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":355,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":356,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":357,"context_line":"            \u0027--dhcp-leasefile\u003d%s\u0027 % self.get_conf_file_name(\u0027lease\u0027),"},{"line_number":358,"context_line":"        ]"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        possible_leases \u003d 0"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_b6b02841","line":357,"in_reply_to":"baada198_3578a38e","updated":"2014-07-26 00:04:46.000000000","message":"Like mentioned in case 3) of my previous reply, if a new MAC is mapped to the old IP, then host file will have a new entry and the old entry in the lease file will have no impact. Dnsmasq will reply NAK to renews for the old MAC, because it doesn\u0027t exist in the host file. If DISCOVER comes from the new MAC, dnsmasq will generate a new lease in the lease file.\n\nIn short, in worst cases it just falls back to the behavior without this patch.","commit_id":"89aa0a0d958a50e2d3c511e234bc40667cbe6697"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"e923e597d8170229bfa4832c07f701eaf493e094","unresolved":false,"context_lines":[{"line_number":354,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":355,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":356,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":357,"context_line":"            \u0027--dhcp-leasefile\u003d%s\u0027 % self.get_conf_file_name(\u0027lease\u0027),"},{"line_number":358,"context_line":"        ]"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        possible_leases \u003d 0"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_77c3158a","line":357,"in_reply_to":"baada198_4321fe9c","updated":"2014-07-25 03:21:35.000000000","message":"In this case, yes. But it is better to consider moving/sync the lease file when moving the dhcp agent service (e.g. add the logic in pacemaker for active-standby failover).\n\nHowever, even if lease file is not taken care in the dhcp agent moving, the consequence is still better than no lease file at all:\n\n1) If the dhcp agent service is moved back before lease is expired for an IP, then there is no impact to this IP;\n\n2) If the time is too long, and the lease is already expired for an entry in the old lease file, then the impact is the same as if there is no lease file (the current state without this patch): the client will get NAK for renew until it send DISCOVER and then the lease will be restored;\n\n3) If the lease file is too old that some entries existing in the old lease file is already deleted in neutron, thus doesn\u0027t exist in dnsmasq host file, then the entry in lease file has no effect, because host checked before lease file. If there is a VM sending (hacking behavior) DHCPREQUEST to renew the IP in the lease file entry, it will get a NAK (address not available). So there is no security/consistency problem.","commit_id":"89aa0a0d958a50e2d3c511e234bc40667cbe6697"},{"author":{"_account_id":12336,"name":"kazuhiro MIYASHITA","email":"miyakz@jp.fujitsu.com","username":"miyakz"},"change_message_id":"fb17c855ea0c991adc4026053a55af778a24554b","unresolved":false,"context_lines":[{"line_number":354,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":355,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":356,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":357,"context_line":"            \u0027--dhcp-leasefile\u003d%s\u0027 % self.get_conf_file_name(\u0027lease\u0027),"},{"line_number":358,"context_line":"        ]"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        possible_leases \u003d 0"}],"source_content_type":"text/x-python","patch_set":3,"id":"9aa7fdbe_55087c28","line":357,"in_reply_to":"baada198_77c3158a","updated":"2014-10-28 23:26:28.000000000","message":"Brian Haley\t, Han Zhou.\n\nHow about using shared storage among Network Nodes.\nlease files are storad in that. \nEven if dhcp agents service is moved other node, and back, agents are able to read files.\n(ex: iSCSI storage servers)","commit_id":"89aa0a0d958a50e2d3c511e234bc40667cbe6697"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"2346a65c12193d3745228249f99c90ed25eba500","unresolved":false,"context_lines":[{"line_number":354,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self._output_hosts_file(),"},{"line_number":355,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":356,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":357,"context_line":"            \u0027--dhcp-leasefile\u003d%s\u0027 % self.get_conf_file_name(\u0027lease\u0027),"},{"line_number":358,"context_line":"        ]"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"        possible_leases \u003d 0"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_3578a38e","line":357,"in_reply_to":"baada198_77c3158a","updated":"2014-07-25 14:43:22.000000000","message":"I wasn\u0027t thinking of the Pacemaker case, we can\u0027t assume someone is running that and will fix the file up.\n\nI was thinking of when someone uses dhcp-agent-network-remove/dhcp-agent-network-add - for example, we use this to evacuate a network node for upgrade.  If we then move this dhcp server back the next day, what will happen if say, the lease file has an old entry, then we try and add an entry for the same IP with a new MAC.  Will it just work?\n\nPerhaps when we\u0027re initially spawning the process you should remove any old lease file, but not when doing a restart.  Would that cover this case?","commit_id":"89aa0a0d958a50e2d3c511e234bc40667cbe6697"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"5c26c2fbb20163749f743f9847d1232d1a52e91e","unresolved":false,"context_lines":[{"line_number":773,"context_line":"        return any(isolated_subnets[subnet.id] for subnet in network.subnets)"},{"line_number":774,"context_line":""},{"line_number":775,"context_line":"    @classmethod"},{"line_number":776,"context_line":"    def lease_update(cls):"},{"line_number":777,"context_line":"        network_id \u003d os.environ.get(cls.NEUTRON_NETWORK_ID_KEY)"},{"line_number":778,"context_line":"        dhcp_relay_socket \u003d os.environ.get(cls.NEUTRON_RELAY_SOCKET_PATH_KEY)"},{"line_number":779,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_3077073d","side":"PARENT","line":776,"updated":"2014-12-18 19:41:04.000000000","message":"Was this all just dead code?  I don\u0027t see it being useful for anything.","commit_id":"b4a0fa3c34cd193c953557cc0931681ff0582768"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"6fbb55b9b3e170a335fca153e6b2e20b74e96902","unresolved":false,"context_lines":[{"line_number":773,"context_line":"        return any(isolated_subnets[subnet.id] for subnet in network.subnets)"},{"line_number":774,"context_line":""},{"line_number":775,"context_line":"    @classmethod"},{"line_number":776,"context_line":"    def lease_update(cls):"},{"line_number":777,"context_line":"        network_id \u003d os.environ.get(cls.NEUTRON_NETWORK_ID_KEY)"},{"line_number":778,"context_line":"        dhcp_relay_socket \u003d os.environ.get(cls.NEUTRON_RELAY_SOCKET_PATH_KEY)"},{"line_number":779,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_351fdd6d","side":"PARENT","line":776,"in_reply_to":"3a961159_3077073d","updated":"2014-12-19 14:19:57.000000000","message":"Yes it is dead code, which was used before the old --dhcp-script was removed.","commit_id":"b4a0fa3c34cd193c953557cc0931681ff0582768"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"5c26c2fbb20163749f743f9847d1232d1a52e91e","unresolved":false,"context_lines":[{"line_number":777,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":778,"context_line":""},{"line_number":779,"context_line":"    @classmethod"},{"line_number":780,"context_line":"    def lease_init(cls):"},{"line_number":781,"context_line":"        action \u003d sys.argv[1]"},{"line_number":782,"context_line":"        if action !\u003d \u0027init\u0027:"},{"line_number":783,"context_line":"            sys.exit()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_70b98f14","line":780,"updated":"2014-12-18 19:41:04.000000000","message":"It seems very odd to me for this to live here in this class.  I think this should probably live as a stand-alone file with a main.  Sort of like rootwrap.","commit_id":"36e378873a53fdb7e3977f469726319c32b71a17"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"5c26c2fbb20163749f743f9847d1232d1a52e91e","unresolved":false,"context_lines":[{"line_number":783,"context_line":"            sys.exit()"},{"line_number":784,"context_line":""},{"line_number":785,"context_line":"        hostsfile \u003d None"},{"line_number":786,"context_line":"        network_id \u003d os.environ.get(cls.NEUTRON_NETWORK_ID_KEY)"},{"line_number":787,"context_line":"        ps_cmd \u003d \u0027ps ax | grep dnsmasq | grep \u0027 + network_id"},{"line_number":788,"context_line":"        proc \u003d subprocess.Popen(ps_cmd, shell\u003dTrue, stdout\u003dsubprocess.PIPE)"},{"line_number":789,"context_line":"        ps_out, ps_err \u003d proc.communicate()"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_eb0cb43d","line":786,"updated":"2014-12-18 19:41:04.000000000","message":"This could be a module constant rather than a class constant allowing my comment on 780 to be addressed.","commit_id":"36e378873a53fdb7e3977f469726319c32b71a17"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"1ac1a6cf340a9230edf95314b3db746285c9f46a","unresolved":false,"context_lines":[{"line_number":788,"context_line":"        proc \u003d subprocess.Popen(ps_cmd, shell\u003dTrue, stdout\u003dsubprocess.PIPE)"},{"line_number":789,"context_line":"        ps_out, ps_err \u003d proc.communicate()"},{"line_number":790,"context_line":"        if proc.returncode:"},{"line_number":791,"context_line":"            sys.exit()"},{"line_number":792,"context_line":""},{"line_number":793,"context_line":"        prefix \u003d \u0027--dhcp-hostsfile\u003d\u0027"},{"line_number":794,"context_line":"        for l in ps_out.split():"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_834ec2bf","line":791,"updated":"2014-12-18 14:58:37.000000000","message":"I don\u0027t think calling \u0027ps ax\u0027 is a good idea.  Can\u0027t you just do:\n\nhostfile \u003d self.get_conf_file_name(\u0027host\u0027)\n\nI would then move the code below that updates the file into its own function.","commit_id":"36e378873a53fdb7e3977f469726319c32b71a17"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"5c26c2fbb20163749f743f9847d1232d1a52e91e","unresolved":false,"context_lines":[{"line_number":788,"context_line":"        proc \u003d subprocess.Popen(ps_cmd, shell\u003dTrue, stdout\u003dsubprocess.PIPE)"},{"line_number":789,"context_line":"        ps_out, ps_err \u003d proc.communicate()"},{"line_number":790,"context_line":"        if proc.returncode:"},{"line_number":791,"context_line":"            sys.exit()"},{"line_number":792,"context_line":""},{"line_number":793,"context_line":"        prefix \u003d \u0027--dhcp-hostsfile\u003d\u0027"},{"line_number":794,"context_line":"        for l in ps_out.split():"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_30e2a701","line":791,"in_reply_to":"3a961159_834ec2bf","updated":"2014-12-18 19:41:04.000000000","message":"Remember that this is class method AND it is running in a totally different process which is spawned by dnsmasq itself.  So, calling self.anything will not work.  All context is lost.  We\u0027ve got to get the information from somewhere.  Ps was clever but is complex and maybe a little brittle.  So, I wonder if there is a better way.\n\nYou\u0027ve already shown that environment variables are inherited by the child process.  Why not pass the hostsfile directly via environment instead of passing the network id?  You could cut all of this code out and have the config file directly.","commit_id":"36e378873a53fdb7e3977f469726319c32b71a17"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"5c26c2fbb20163749f743f9847d1232d1a52e91e","unresolved":false,"context_lines":[{"line_number":805,"context_line":"                    host \u003d l.strip().split(\u0027,\u0027)"},{"line_number":806,"context_line":"                    mac \u003d host[0]"},{"line_number":807,"context_line":"                    ip_address \u003d host[2]"},{"line_number":808,"context_line":"                    hostname \u003d \u0027host-%s\u0027 % ip_address.replace("},{"line_number":809,"context_line":"                        \u0027.\u0027, \u0027-\u0027).replace(\u0027:\u0027, \u0027-\u0027)"},{"line_number":810,"context_line":"                    lease_exp \u003d \u00272000000000\u0027"},{"line_number":811,"context_line":"                    print(\u0027%s %s %s %s *\u0027 %"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_cb261868","line":808,"updated":"2014-12-18 19:41:04.000000000","message":"Could you extract this in to a module level utility since it is duplicated on L478 and should always match?","commit_id":"36e378873a53fdb7e3977f469726319c32b71a17"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"5c26c2fbb20163749f743f9847d1232d1a52e91e","unresolved":false,"context_lines":[{"line_number":807,"context_line":"                    ip_address \u003d host[2]"},{"line_number":808,"context_line":"                    hostname \u003d \u0027host-%s\u0027 % ip_address.replace("},{"line_number":809,"context_line":"                        \u0027.\u0027, \u0027-\u0027).replace(\u0027:\u0027, \u0027-\u0027)"},{"line_number":810,"context_line":"                    lease_exp \u003d \u00272000000000\u0027"},{"line_number":811,"context_line":"                    print(\u0027%s %s %s %s *\u0027 %"},{"line_number":812,"context_line":"                          (lease_exp, mac, ip_address, hostname))"},{"line_number":813,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_10c4cb90","line":810,"updated":"2014-12-18 19:41:04.000000000","message":"Don\u0027t think we\u0027ll be using this code 45 years from now?  ;)","commit_id":"36e378873a53fdb7e3977f469726319c32b71a17"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"5c26c2fbb20163749f743f9847d1232d1a52e91e","unresolved":false,"context_lines":[{"line_number":808,"context_line":"                    hostname \u003d \u0027host-%s\u0027 % ip_address.replace("},{"line_number":809,"context_line":"                        \u0027.\u0027, \u0027-\u0027).replace(\u0027:\u0027, \u0027-\u0027)"},{"line_number":810,"context_line":"                    lease_exp \u003d \u00272000000000\u0027"},{"line_number":811,"context_line":"                    print(\u0027%s %s %s %s *\u0027 %"},{"line_number":812,"context_line":"                          (lease_exp, mac, ip_address, hostname))"},{"line_number":813,"context_line":""},{"line_number":814,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"3a961159_6bd604f3","line":811,"updated":"2014-12-18 19:41:04.000000000","message":"I was going to say that there should be five fields [1] but I now see the asterisk in the fifth position.  This is probably fine for now.  I assume that this doesn\u0027t need to match anything in order for dnsmasq to honor the lease, right?  Has this worked in all of your testing.\n\nI wonder if you could make it a bit more explicit like this just so that we can come back to it if we ever support using a client identifier and not forgot about it:\n\n  client_id \u003d \u0027*\u0027\n  print(\u0027%s %s %s %s *\u0027 %\n      (lease_exp, mac, ip_address, hostname, client_id))\n\n [1] http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2006q2/000734.html","commit_id":"36e378873a53fdb7e3977f469726319c32b71a17"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"46d8832d6d996898bb846ab2bc5c0a832424a7b8","unresolved":false,"context_lines":[{"line_number":312,"context_line":"    _TAG_PREFIX \u003d \u0027tag%d\u0027"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    HOSTS_FILE_ID_KEY \u003d \u0027HOSTS_FILE\u0027"},{"line_number":315,"context_line":"    LEASE_EXP_MAX \u003d (2100 - 1970) * 365 * 24 * 3600"},{"line_number":316,"context_line":"    MINIMUM_VERSION \u003d 2.63"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_8a6e7c2d","line":315,"updated":"2015-01-07 21:33:40.000000000","message":"Just noticed this is quite the long \"fake\" lease time.  I know you can\u0027t read the dhcp_lease_duration tunable from here, but perhaps just using the 86400 default (or even a week) is a better choice than 130 years? :)","commit_id":"4230a09f0ad556d20a3db09e58113cb9276738f9"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"3d54df5bf2ad019474ca367e472c7544ab9a7bd4","unresolved":false,"context_lines":[{"line_number":312,"context_line":"    _TAG_PREFIX \u003d \u0027tag%d\u0027"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    HOSTS_FILE_ID_KEY \u003d \u0027HOSTS_FILE\u0027"},{"line_number":315,"context_line":"    LEASE_EXP_MAX \u003d (2100 - 1970) * 365 * 24 * 3600"},{"line_number":316,"context_line":"    MINIMUM_VERSION \u003d 2.63"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_b24e3494","line":315,"in_reply_to":"3a961159_4142ab3e","updated":"2015-01-08 16:13:32.000000000","message":"Han, oh, I thought this was the time from now until expiration, not and end-time, every place else lease time is a duration.","commit_id":"4230a09f0ad556d20a3db09e58113cb9276738f9"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"890101dbdcc043815e0af3cb9a219ec3295f1a38","unresolved":false,"context_lines":[{"line_number":312,"context_line":"    _TAG_PREFIX \u003d \u0027tag%d\u0027"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    HOSTS_FILE_ID_KEY \u003d \u0027HOSTS_FILE\u0027"},{"line_number":315,"context_line":"    LEASE_EXP_MAX \u003d (2100 - 1970) * 365 * 24 * 3600"},{"line_number":316,"context_line":"    MINIMUM_VERSION \u003d 2.63"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a961159_4142ab3e","line":315,"in_reply_to":"3a961159_8a6e7c2d","updated":"2015-01-08 09:19:51.000000000","message":"Brian, this is not the lease duration, but the expiration time. It means this code can work properly until year 2100.\n\nThis value is set just to make sure it will work after dhcp-agent and dnsmasq restart. It will not affect the real lease used by clients, because dhcp clients will still perform dhcp renew according to the real lease they were allocated previously. And as soon as the clients renew, the real lease will be stored in dnsmasq memory.","commit_id":"4230a09f0ad556d20a3db09e58113cb9276738f9"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"98bf7f538fe3a912d994fa6e224975c97f954c4a","unresolved":false,"context_lines":[{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    _TAG_PREFIX \u003d \u0027tag%d\u0027"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"    NEUTRON_NETWORK_ID_KEY \u003d \u0027NEUTRON_NETWORK_ID\u0027"},{"line_number":317,"context_line":"    NEUTRON_RELAY_SOCKET_PATH_KEY \u003d \u0027NEUTRON_RELAY_SOCKET_PATH\u0027"},{"line_number":318,"context_line":"    MINIMUM_VERSION \u003d 2.67"},{"line_number":319,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_aa527c65","side":"PARENT","line":316,"updated":"2015-01-15 11:53:53.000000000","message":"I\u0027m glad you clean up dead code here, though it would be better to see it as a separate patch.","commit_id":"1210d1c0595ca2888f147ee38257f7f3be96214f"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"fcd25406eed7ad5e9315ea96ec7806047d3520ee","unresolved":false,"context_lines":[{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    _TAG_PREFIX \u003d \u0027tag%d\u0027"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"    NEUTRON_NETWORK_ID_KEY \u003d \u0027NEUTRON_NETWORK_ID\u0027"},{"line_number":317,"context_line":"    NEUTRON_RELAY_SOCKET_PATH_KEY \u003d \u0027NEUTRON_RELAY_SOCKET_PATH\u0027"},{"line_number":318,"context_line":"    MINIMUM_VERSION \u003d 2.67"},{"line_number":319,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_1e896726","side":"PARENT","line":316,"in_reply_to":"3a961159_aa527c65","updated":"2015-01-22 19:30:09.000000000","message":"Let\u0027s split this patch in to a dead-code patch and new code patch.  We need to get past the current impasse.  Splitting them up will reduce the surface area of the patch.","commit_id":"1210d1c0595ca2888f147ee38257f7f3be96214f"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"d364b688a044402d6ee61e412387874f22596012","unresolved":false,"context_lines":[{"line_number":352,"context_line":"    def spawn_process(self):"},{"line_number":353,"context_line":"        \"\"\"Spawns a Dnsmasq process for the network.\"\"\""},{"line_number":354,"context_line":"        env \u003d {"},{"line_number":355,"context_line":"            self.NEUTRON_NETWORK_ID_KEY: self.network.id,"},{"line_number":356,"context_line":"        }"},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"        cmd \u003d ["}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_4a3598f5","side":"PARENT","line":355,"updated":"2015-01-15 12:03:41.000000000","message":"^ dead code too, ok. (see link @ line 775)","commit_id":"1210d1c0595ca2888f147ee38257f7f3be96214f"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"d364b688a044402d6ee61e412387874f22596012","unresolved":false,"context_lines":[{"line_number":772,"context_line":"        return any(isolated_subnets[subnet.id] for subnet in network.subnets)"},{"line_number":773,"context_line":""},{"line_number":774,"context_line":"    @classmethod"},{"line_number":775,"context_line":"    def lease_update(cls):"},{"line_number":776,"context_line":"        network_id \u003d os.environ.get(cls.NEUTRON_NETWORK_ID_KEY)"},{"line_number":777,"context_line":"        dhcp_relay_socket \u003d os.environ.get(cls.NEUTRON_RELAY_SOCKET_PATH_KEY)"},{"line_number":778,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_0aeab09f","side":"PARENT","line":775,"updated":"2015-01-15 12:03:41.000000000","message":"Ok, this was dead code from https://review.openstack.org/#/c/37580/","commit_id":"1210d1c0595ca2888f147ee38257f7f3be96214f"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"98bf7f538fe3a912d994fa6e224975c97f954c4a","unresolved":false,"context_lines":[{"line_number":312,"context_line":"    _TAG_PREFIX \u003d \u0027tag%d\u0027"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    HOSTS_FILE_ID_KEY \u003d \u0027HOSTS_FILE\u0027"},{"line_number":315,"context_line":"    LEASE_EXP_MAX \u003d (2100 - 1970) * 365 * 24 * 3600"},{"line_number":316,"context_line":"    MINIMUM_VERSION \u003d 2.67"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_dde1502c","line":315,"updated":"2015-01-15 11:53:53.000000000","message":"I just wonder why 2100 and not some other value.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"2592ba09adb529a0fa2680b18764c02fe73776ba","unresolved":false,"context_lines":[{"line_number":312,"context_line":"    _TAG_PREFIX \u003d \u0027tag%d\u0027"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    HOSTS_FILE_ID_KEY \u003d \u0027HOSTS_FILE\u0027"},{"line_number":315,"context_line":"    LEASE_EXP_MAX \u003d (2100 - 1970) * 365 * 24 * 3600"},{"line_number":316,"context_line":"    MINIMUM_VERSION \u003d 2.67"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_ed02b007","line":315,"in_reply_to":"3a961159_dde1502c","updated":"2015-01-16 14:24:46.000000000","message":"No special reasons, but 2100 keeps this value 4 bytes, yet large enough that none of us are able to see this code broken.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f3f5f75e10191e76f4ba1d944553f8a2def1ead8","unresolved":false,"context_lines":[{"line_number":312,"context_line":"    _TAG_PREFIX \u003d \u0027tag%d\u0027"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    HOSTS_FILE_ID_KEY \u003d \u0027HOSTS_FILE\u0027"},{"line_number":315,"context_line":"    LEASE_EXP_MAX \u003d (2100 - 1970) * 365 * 24 * 3600"},{"line_number":316,"context_line":"    MINIMUM_VERSION \u003d 2.67"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_0671c076","line":315,"in_reply_to":"3a961159_dde1502c","updated":"2015-01-21 22:54:15.000000000","message":"Y2.1K  ;)","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"9ecdb90d3bb1cdd6da4dd51595d5847e8a3409b2","unresolved":false,"context_lines":[{"line_number":312,"context_line":"    _TAG_PREFIX \u003d \u0027tag%d\u0027"},{"line_number":313,"context_line":""},{"line_number":314,"context_line":"    HOSTS_FILE_ID_KEY \u003d \u0027HOSTS_FILE\u0027"},{"line_number":315,"context_line":"    LEASE_EXP_MAX \u003d (2100 - 1970) * 365 * 24 * 3600"},{"line_number":316,"context_line":"    MINIMUM_VERSION \u003d 2.67"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_30ca4ba0","line":315,"in_reply_to":"3a961159_ed02b007","updated":"2015-01-16 14:48:49.000000000","message":":)\n(In parallel world) News headlines tomorrow: \"death remedy invented!\".","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"d364b688a044402d6ee61e412387874f22596012","unresolved":false,"context_lines":[{"line_number":454,"context_line":"        self.device_manager.update(self.network, self.interface_name)"},{"line_number":455,"context_line":""},{"line_number":456,"context_line":"    @staticmethod"},{"line_number":457,"context_line":"    def _ip2host(ip_address):"},{"line_number":458,"context_line":"        return \u0027host-%s\u0027 % ip_address.replace("},{"line_number":459,"context_line":"            \u0027.\u0027, \u0027-\u0027).replace(\u0027:\u0027, \u0027-\u0027)"},{"line_number":460,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_1dfe7805","line":457,"updated":"2015-01-15 12:03:41.000000000","message":"nit: following our general function naming convention: _ip_to_host ?","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f3f5f75e10191e76f4ba1d944553f8a2def1ead8","unresolved":false,"context_lines":[{"line_number":454,"context_line":"        self.device_manager.update(self.network, self.interface_name)"},{"line_number":455,"context_line":""},{"line_number":456,"context_line":"    @staticmethod"},{"line_number":457,"context_line":"    def _ip2host(ip_address):"},{"line_number":458,"context_line":"        return \u0027host-%s\u0027 % ip_address.replace("},{"line_number":459,"context_line":"            \u0027.\u0027, \u0027-\u0027).replace(\u0027:\u0027, \u0027-\u0027)"},{"line_number":460,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_a63b8c9a","line":457,"in_reply_to":"3a961159_1dfe7805","updated":"2015-01-21 22:54:15.000000000","message":"+1","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"d364b688a044402d6ee61e412387874f22596012","unresolved":false,"context_lines":[{"line_number":456,"context_line":"    @staticmethod"},{"line_number":457,"context_line":"    def _ip2host(ip_address):"},{"line_number":458,"context_line":"        return \u0027host-%s\u0027 % ip_address.replace("},{"line_number":459,"context_line":"            \u0027.\u0027, \u0027-\u0027).replace(\u0027:\u0027, \u0027-\u0027)"},{"line_number":460,"context_line":""},{"line_number":461,"context_line":"    def _iter_hosts(self):"},{"line_number":462,"context_line":"        \"\"\"Iterate over hosts."}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_dd179044","line":459,"updated":"2015-01-15 12:03:41.000000000","message":"+1 refactor","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"98bf7f538fe3a912d994fa6e224975c97f954c4a","unresolved":false,"context_lines":[{"line_number":776,"context_line":"        return any(isolated_subnets[subnet.id] for subnet in network.subnets)"},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"    def _lease_script_path(self):"},{"line_number":779,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":780,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_6a36d403","line":779,"updated":"2015-01-15 11:53:53.000000000","message":"Here, you assume sys.argv[0] is always absolete. It\u0027s not true, and won\u0027t work if agent was executed without full path specified.\n\nSo instead of doing that, I would recommend to generate a simple script inside network space that would invoke lease-init script we generate from setup.cfg without specifying full path, and allow shell to handle path traversion for you.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"79e90f40293f6120cfb6c959ea7a4effc256436a","unresolved":false,"context_lines":[{"line_number":776,"context_line":"        return any(isolated_subnets[subnet.id] for subnet in network.subnets)"},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"    def _lease_script_path(self):"},{"line_number":779,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":780,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_1b31ce0a","line":779,"in_reply_to":"3a961159_19e801de","updated":"2015-01-19 09:05:26.000000000","message":"Yes, it is a corner case. Though I still believe we should not rely on particular path for the script. In addition to your example, I may also mention that invokation of the agent via a symlink located in another place will also result in failure. Also, packagers may decide to move their executables in different place. There is too much moving and platform dependent stuff here to rely on that.\n\nThat just adds another argument for avoiding python interpreter invokation here. There is nothing that would need python here, shell is more than enough.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"34507e8974934980cdf44b17bb0d5cb0dbc33f09","unresolved":false,"context_lines":[{"line_number":776,"context_line":"        return any(isolated_subnets[subnet.id] for subnet in network.subnets)"},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"    def _lease_script_path(self):"},{"line_number":779,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":780,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_d038e0a1","line":779,"in_reply_to":"3a961159_1b31ce0a","updated":"2015-01-20 07:46:57.000000000","message":"For using symlink, there is no problem as long as the binaries of neutron-dhcp-agent and neutron-dhcp-agent-dnsmasq-lease-init scripts are in the same dir.\n\nIt is ok to cover corner cases if we have a clean solution, but at least I don\u0027t think we have yet.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"9ecdb90d3bb1cdd6da4dd51595d5847e8a3409b2","unresolved":false,"context_lines":[{"line_number":776,"context_line":"        return any(isolated_subnets[subnet.id] for subnet in network.subnets)"},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"    def _lease_script_path(self):"},{"line_number":779,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":780,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_f0bd23d8","line":779,"in_reply_to":"3a961159_4dc4a491","updated":"2015-01-16 14:48:49.000000000","message":"Yes, we may assume that the init script is located in the same directory as neutron-dhcp-agent. Though you won\u0027t be able to determine that dir from sys.argv[0] if agent is executed with no absolute path passed in CLI. In that case, dirname() will return \u0027\u0027, and join will give you just \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027, while dnsmasq expects absolute path.\n\nThe solution I propose is:\n1. generate a simple shell script in /var/lib/neutron/\u003cnetwork_id\u003e/... and pass it into --dhcp-script\u003d.\n2. in that shell script, you just call to init tool.\n\nSmth like:\n#!/bin/sh\nneutron-dhcp-agent-dnsmasq-lease-init\n\nIn that case, we may pass absolute path to dnsmasq (the path in /var/lib/neutron/... is known) while not relying on particular location of neutron-dhcp-agent-dnsmasq-lease-init (since it will be resolved by shell itself based on $PATH).","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"2592ba09adb529a0fa2680b18764c02fe73776ba","unresolved":false,"context_lines":[{"line_number":776,"context_line":"        return any(isolated_subnets[subnet.id] for subnet in network.subnets)"},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"    def _lease_script_path(self):"},{"line_number":779,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":780,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_4dc4a491","line":779,"in_reply_to":"3a961159_6a36d403","updated":"2015-01-16 14:24:46.000000000","message":"If neutron-dhcp-agent can be executed without absolute path, it is likely this init-script should be able to be invoked with the same relative path, isn\u0027t it?\n\nSorry that I don\u0027t understand the solution you proposed. Could you elaborate \"script inside network space\"?","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"fcd25406eed7ad5e9315ea96ec7806047d3520ee","unresolved":false,"context_lines":[{"line_number":776,"context_line":"        return any(isolated_subnets[subnet.id] for subnet in network.subnets)"},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"    def _lease_script_path(self):"},{"line_number":779,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":780,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_699d5759","line":779,"in_reply_to":"3a961159_814ad3aa","updated":"2015-01-22 19:30:09.000000000","message":"The only way there is a problem with argv[0] is if the agent is called with a relative path (e.g. ./path/to/script).  If it is called from CLI with no path, argv[0] will be absolute as Han explained.  I don\u0027t think any sane start-up script will use a relative path.  But, maybe we need to design for the insane.  I\u0027m not concerned about it enough to -1 the patch for this.\n\nWith that said, I\u0027m warming up to the idea of writing a shell script to a known location and passing that full path directly with --dhcp-script because it is more straight-forward and easier to see that it is correct.\n\nEither way, we should not rely on $PATH to find the script due to the security concern @Brian raised.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"15aef64394069ce8ce741a84cfb3e03d5b02d604","unresolved":false,"context_lines":[{"line_number":776,"context_line":"        return any(isolated_subnets[subnet.id] for subnet in network.subnets)"},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"    def _lease_script_path(self):"},{"line_number":779,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":780,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_e1663728","line":779,"in_reply_to":"3a961159_a34c3740","updated":"2015-01-16 16:20:05.000000000","message":"\u003e I don\u0027t think letting the shell determine which neutron-dhcp-agent-dnsmasq-lease-init to use is a good idea\nCan you elaborate on why so?\n\nYes, we can put the script in /var/lib/neutron. In that case though, we will need to generate it at runtime (meaning, writing out python file with the code of init tool during dhcp agent startup. This is pretty the same as Miguel would suggest with utilizing Lua, the only difference is it would be Python code and not Lua.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"51ad295f341af46c8f9ce30595ac48ae0b5ccc13","unresolved":false,"context_lines":[{"line_number":776,"context_line":"        return any(isolated_subnets[subnet.id] for subnet in network.subnets)"},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"    def _lease_script_path(self):"},{"line_number":779,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":780,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_814ad3aa","line":779,"in_reply_to":"3a961159_a34c3740","updated":"2015-01-22 13:05:04.000000000","message":"That sounds good to me, and aligned to written specifically crafted scripts in shell script. Although I believe lua would be more performant. (no need to fork at all), and more secure (no need to run as root).","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"84d56a9f9e5a14fd28573a8177b17ccbb26ac9d9","unresolved":false,"context_lines":[{"line_number":776,"context_line":"        return any(isolated_subnets[subnet.id] for subnet in network.subnets)"},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"    def _lease_script_path(self):"},{"line_number":779,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":780,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_f7a7d5b9","line":779,"in_reply_to":"3a961159_e1663728","updated":"2015-01-16 16:46:39.000000000","message":"\u003e I don\u0027t think letting the shell determine which neutron-dhcp-agent-dnsmasq-lease-init to use is a good idea Can you elaborate on why so?\n\n1. It could run the wrong one\n2. Security implications of running the wrong one, since we\u0027re doing this as root\n\nSo is running neutron-dhcp-agent by hand a common occurrence?  Seems like a corner-case to me.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"f7a0b449c7e0d5f07d41a9183c497629fd05bae2","unresolved":false,"context_lines":[{"line_number":776,"context_line":"        return any(isolated_subnets[subnet.id] for subnet in network.subnets)"},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"    def _lease_script_path(self):"},{"line_number":779,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":780,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_a34c3740","line":779,"in_reply_to":"3a961159_f0bd23d8","updated":"2015-01-16 15:36:20.000000000","message":"I don\u0027t think letting the shell determine which neutron-dhcp-agent-dnsmasq-lease-init to use is a good idea, can we put this in the same root directory where all the host and pid files live?  For example, /var/lib/neutron/dhcp/scripts ?","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"8667df7469111038f790a0ed675de0b55250e46c","unresolved":false,"context_lines":[{"line_number":776,"context_line":"        return any(isolated_subnets[subnet.id] for subnet in network.subnets)"},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"    def _lease_script_path(self):"},{"line_number":779,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":780,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_f81fc506","line":779,"in_reply_to":"3a961159_f7a7d5b9","updated":"2015-01-16 17:35:02.000000000","message":"Security concerns seem to be valid.\n\nI don\u0027t think we should rely on particular way users run our agents since we don\u0027t control their startup systems. We may implement \u0027init\u0027 tool in shell only, in that case we won\u0027t need to call to external tool and rely on $PATH.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"7ae4d45d20191d196034bcc4294b0d7ae6982569","unresolved":false,"context_lines":[{"line_number":776,"context_line":"        return any(isolated_subnets[subnet.id] for subnet in network.subnets)"},{"line_number":777,"context_line":""},{"line_number":778,"context_line":"    def _lease_script_path(self):"},{"line_number":779,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":780,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_19e801de","line":779,"in_reply_to":"3a961159_f81fc506","updated":"2015-01-18 15:49:14.000000000","message":"I agree with Brian this is a corner case. Even if \"neutron-dhcp-agent\" is passed to cli without any path (in this case, it would rely on $PATH), sys.argv[0] will still point to the absolute path, and the code will still work. The only case the code would be broken is when the cli specify relative path explicitly, such as: ./ or ../\n\nI would suggest to avoid complexity just for such corner case. And it is easy to identify the problem in those cases with the help of dnsmasq log: failed to execute ./neutron-dhcp-agent-dnsmasq-lease-init: No such file or directory","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"0f3b5f0e7724e3acadcd348e74baff6939a5397f","unresolved":false,"context_lines":[{"line_number":780,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @classmethod"},{"line_number":783,"context_line":"    def lease_init(cls):"},{"line_number":784,"context_line":"        action \u003d sys.argv[1]"},{"line_number":785,"context_line":"        if action !\u003d \u0027init\u0027:"},{"line_number":786,"context_line":"            sys.exit()"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_6d1871cd","line":783,"updated":"2015-01-08 20:06:58.000000000","message":"I would still like to see a test for this code if possible","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"98bf7f538fe3a912d994fa6e224975c97f954c4a","unresolved":false,"context_lines":[{"line_number":780,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @classmethod"},{"line_number":783,"context_line":"    def lease_init(cls):"},{"line_number":784,"context_line":"        action \u003d sys.argv[1]"},{"line_number":785,"context_line":"        if action !\u003d \u0027init\u0027:"},{"line_number":786,"context_line":"            sys.exit()"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_bdbf24fb","line":783,"in_reply_to":"3a961159_6d1871cd","updated":"2015-01-15 11:53:53.000000000","message":"Indeed, the script should be covered with tests, probably functional.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"d364b688a044402d6ee61e412387874f22596012","unresolved":false,"context_lines":[{"line_number":781,"context_line":""},{"line_number":782,"context_line":"    @classmethod"},{"line_number":783,"context_line":"    def lease_init(cls):"},{"line_number":784,"context_line":"        action \u003d sys.argv[1]"},{"line_number":785,"context_line":"        if action !\u003d \u0027init\u0027:"},{"line_number":786,"context_line":"            sys.exit()"},{"line_number":787,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_9da18897","line":784,"updated":"2015-01-15 12:03:41.000000000","message":"+ tests neccesary,","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"98bf7f538fe3a912d994fa6e224975c97f954c4a","unresolved":false,"context_lines":[{"line_number":785,"context_line":"        if action !\u003d \u0027init\u0027:"},{"line_number":786,"context_line":"            sys.exit()"},{"line_number":787,"context_line":""},{"line_number":788,"context_line":"        hostsfile \u003d os.environ.get(cls.HOSTS_FILE_ID_KEY)"},{"line_number":789,"context_line":""},{"line_number":790,"context_line":"        if hostsfile is None:"},{"line_number":791,"context_line":"            sys.exit()"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_9d118855","line":788,"updated":"2015-01-15 11:53:53.000000000","message":"How about moving the attributes you use and _ip2host into global scope and convert the method into a function? It does not really belong to the class, better leave it decoupled.\n\nI would even move further and move the code into a separate file. It\u0027s wasteful to run all those imports at the top of the file to do an easy line parse.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"d364b688a044402d6ee61e412387874f22596012","unresolved":false,"context_lines":[{"line_number":800,"context_line":"                    client_id \u003d \u0027*\u0027"},{"line_number":801,"context_line":"                    print(\u0027%u %s %s %s %s\u0027 %"},{"line_number":802,"context_line":"                          (cls.LEASE_EXP_MAX, mac, ip_address,"},{"line_number":803,"context_line":"                           hostname, client_id))"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_bd77243a","line":803,"updated":"2015-01-15 12:03:41.000000000","message":"I would suggest using --dhcp-luascript and dumping a tiny lua script from our code to the configuration dir, to avoid calling system scripts via sudo+rootwrap+etc, which is rather slow.\n\nrenewing 10k hosts could cost 1000-3000 seconds of CPU time (1 core) with default python spawn times via sudo/rootwrap.\n\nPlease not this is an additional requirement that seems to be covered in CentOS/RHEL, not sure about other distros.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"3478619d22f244519468699fb1d6548a3e4dfce1","unresolved":false,"context_lines":[{"line_number":800,"context_line":"                    client_id \u003d \u0027*\u0027"},{"line_number":801,"context_line":"                    print(\u0027%u %s %s %s %s\u0027 %"},{"line_number":802,"context_line":"                          (cls.LEASE_EXP_MAX, mac, ip_address,"},{"line_number":803,"context_line":"                           hostname, client_id))"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_9e39d243","line":803,"in_reply_to":"1a930d6b_a93c9fc0","updated":"2015-01-22 21:23:29.000000000","message":"Here is an external tools free version of shell script: http://paste.fedoraproject.org/173259/21961738/","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"fcd25406eed7ad5e9315ea96ec7806047d3520ee","unresolved":false,"context_lines":[{"line_number":800,"context_line":"                    client_id \u003d \u0027*\u0027"},{"line_number":801,"context_line":"                    print(\u0027%u %s %s %s %s\u0027 %"},{"line_number":802,"context_line":"                          (cls.LEASE_EXP_MAX, mac, ip_address,"},{"line_number":803,"context_line":"                           hostname, client_id))"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_a93c9fc0","line":803,"in_reply_to":"3a961159_05fb047c","updated":"2015-01-22 19:30:09.000000000","message":"@Ihar  What is trivial in shell to you might not be for everyone.  If it is trivial, would you mind contributing it to the patch?\n\nI agree that the startup time for this is a concern because of the extra imports at the top of the file.  However, I\u0027m not so sure that we need to be concerned about the startup time for a minimal python script.\n\nI would like to see this in a file of its own as I suggested in a previous patch set.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"79e90f40293f6120cfb6c959ea7a4effc256436a","unresolved":false,"context_lines":[{"line_number":800,"context_line":"                    client_id \u003d \u0027*\u0027"},{"line_number":801,"context_line":"                    print(\u0027%u %s %s %s %s\u0027 %"},{"line_number":802,"context_line":"                          (cls.LEASE_EXP_MAX, mac, ip_address,"},{"line_number":803,"context_line":"                           hostname, client_id))"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_5b0ef63d","line":803,"in_reply_to":"3a961159_19bf21cb","updated":"2015-01-19 09:05:26.000000000","message":"\u003e It is unlikely a large number of hosts from a SINGLE neutron network would renew their lease at the same time.\n\nHow is SINGLE part relevant here? The same dhcp agent may handle multiple networks, and invoking a new python interpreter for every new lease is not justified, especially while the job of init script is so trivial. It\u0027s even more unjustified if we consider that you invoke python interpreter AND import lots of irrelevant code at the top of the file, initializing oslo.config and doing other time consuming stuff.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"2592ba09adb529a0fa2680b18764c02fe73776ba","unresolved":false,"context_lines":[{"line_number":800,"context_line":"                    client_id \u003d \u0027*\u0027"},{"line_number":801,"context_line":"                    print(\u0027%u %s %s %s %s\u0027 %"},{"line_number":802,"context_line":"                          (cls.LEASE_EXP_MAX, mac, ip_address,"},{"line_number":803,"context_line":"                           hostname, client_id))"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_adb0a840","line":803,"in_reply_to":"3a961159_20b5cbcf","updated":"2015-01-16 14:24:46.000000000","message":"The code is not used for renew. It is called upon dnsmasq startup.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"bd039cac7a5f5cd169f1c4f60ee39da24550a7cc","unresolved":false,"context_lines":[{"line_number":800,"context_line":"                    client_id \u003d \u0027*\u0027"},{"line_number":801,"context_line":"                    print(\u0027%u %s %s %s %s\u0027 %"},{"line_number":802,"context_line":"                          (cls.LEASE_EXP_MAX, mac, ip_address,"},{"line_number":803,"context_line":"                           hostname, client_id))"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_76f95981","line":803,"in_reply_to":"3a961159_20b5cbcf","updated":"2015-01-15 13:17:55.000000000","message":"btw there is a third option - implement the script in shell. it does not look like a huge deal, and I think we may assume that /bin/sh is present in the platform (which is Linux only, as per file path).","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"69c7faa7c89e53b867ab1d46b9e8369e3b05ee59","unresolved":false,"context_lines":[{"line_number":800,"context_line":"                    client_id \u003d \u0027*\u0027"},{"line_number":801,"context_line":"                    print(\u0027%u %s %s %s %s\u0027 %"},{"line_number":802,"context_line":"                          (cls.LEASE_EXP_MAX, mac, ip_address,"},{"line_number":803,"context_line":"                           hostname, client_id))"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_05fb047c","line":803,"in_reply_to":"3a961159_531eba75","updated":"2015-01-20 10:43:33.000000000","message":"It does not matter who actually invokes the script, agent or dnsmasq. The point is that the way it is currently implemented is wasteful performance wise, and we have multiple solutions at hand to avoid that (afaik the best is to implement it in shell since it\u0027s so trivial, but if you insist on python, it should be at least isolated from other stuff in this file.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"34507e8974934980cdf44b17bb0d5cb0dbc33f09","unresolved":false,"context_lines":[{"line_number":800,"context_line":"                    client_id \u003d \u0027*\u0027"},{"line_number":801,"context_line":"                    print(\u0027%u %s %s %s %s\u0027 %"},{"line_number":802,"context_line":"                          (cls.LEASE_EXP_MAX, mac, ip_address,"},{"line_number":803,"context_line":"                           hostname, client_id))"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_531eba75","line":803,"in_reply_to":"3a961159_5b0ef63d","updated":"2015-01-20 07:46:57.000000000","message":"Ihar, the script is not invoked by dhcp-agent, but by dnsmasq, which is per-network.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"315fe2169c3984881b58ebf968ec65f89ce2702e","unresolved":false,"context_lines":[{"line_number":800,"context_line":"                    client_id \u003d \u0027*\u0027"},{"line_number":801,"context_line":"                    print(\u0027%u %s %s %s %s\u0027 %"},{"line_number":802,"context_line":"                          (cls.LEASE_EXP_MAX, mac, ip_address,"},{"line_number":803,"context_line":"                           hostname, client_id))"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_a0ad3bec","line":803,"in_reply_to":"3a961159_5d6800f4","updated":"2015-01-15 12:16:57.000000000","message":"I\u0027m ok with using lua scripts, but then we\u0027ll need to implement a sanity check to cover this new requirement, since lua support may be not present in dnsmasq.\n\nAlso, maybe we would need a fallback option to cover installations without lua support. But that depends on whether we know any known environments with lua support disabled (gentoo with USE\u003d\"-lua\" in make.conf?)","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"f3f5f75e10191e76f4ba1d944553f8a2def1ead8","unresolved":false,"context_lines":[{"line_number":800,"context_line":"                    client_id \u003d \u0027*\u0027"},{"line_number":801,"context_line":"                    print(\u0027%u %s %s %s %s\u0027 %"},{"line_number":802,"context_line":"                          (cls.LEASE_EXP_MAX, mac, ip_address,"},{"line_number":803,"context_line":"                           hostname, client_id))"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_e6a59403","line":803,"in_reply_to":"3a961159_76f95981","updated":"2015-01-21 22:54:15.000000000","message":"I don\u0027t think sudo + rootwrap is involved here.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"9d32c70896833282eda13b4dc77775f92236a110","unresolved":false,"context_lines":[{"line_number":800,"context_line":"                    client_id \u003d \u0027*\u0027"},{"line_number":801,"context_line":"                    print(\u0027%u %s %s %s %s\u0027 %"},{"line_number":802,"context_line":"                          (cls.LEASE_EXP_MAX, mac, ip_address,"},{"line_number":803,"context_line":"                           hostname, client_id))"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_20b5cbcf","line":803,"in_reply_to":"3a961159_a0ad3bec","updated":"2015-01-15 12:24:24.000000000","message":"I\u0027m unsure about how often the dhcp-script is called, if it\u0027s not for renewals it can be ok, also there is no rootwrap+sudo involved here.\n\nIn the other hand, I found this in the dnsmasq documentation for --dhcp-script:\n\n\"\"The process is run as root (assuming that dnsmasq was originally run as root) even if dnsmasq is configured to change UID to an unprivileged user.\"\"\"\n\nThe security implications for this, is that the lease script made by this patch will belong to the neutron user, if somebody from \"neutron\" could modify this script to gain root via dnsmasq.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"7ae4d45d20191d196034bcc4294b0d7ae6982569","unresolved":false,"context_lines":[{"line_number":800,"context_line":"                    client_id \u003d \u0027*\u0027"},{"line_number":801,"context_line":"                    print(\u0027%u %s %s %s %s\u0027 %"},{"line_number":802,"context_line":"                          (cls.LEASE_EXP_MAX, mac, ip_address,"},{"line_number":803,"context_line":"                           hostname, client_id))"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_19bf21cb","line":803,"in_reply_to":"3a961159_adb0a840","updated":"2015-01-18 15:49:14.000000000","message":"I forgot that this code actually will be invoked even for renew, although it will do nothing than checking the action is not \"init\" and then exit. However, I really don\u0027t think it is performance sensitive here. It is unlikely a large number of hosts from a SINGLE neutron network would renew their lease at the same time.\n\nI agree to move the _ip2host() to global space, if we assume other future DhcpLocalProcess implementation will use same format for hostname. But this method lease_init() is better to stay in class Dnsmasq, because it is specific to dnsmasq implementation.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"9ecdb90d3bb1cdd6da4dd51595d5847e8a3409b2","unresolved":false,"context_lines":[{"line_number":800,"context_line":"                    client_id \u003d \u0027*\u0027"},{"line_number":801,"context_line":"                    print(\u0027%u %s %s %s %s\u0027 %"},{"line_number":802,"context_line":"                          (cls.LEASE_EXP_MAX, mac, ip_address,"},{"line_number":803,"context_line":"                           hostname, client_id))"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_909bff71","line":803,"in_reply_to":"3a961159_adb0a840","updated":"2015-01-16 14:48:49.000000000","message":"We should strive to reduce time to start dnsmasq, so replacing the code with lua/shell script, or at least moving it into a separate file to avoid importing irrelevant stuff is highly recommended.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"be66cb6ff8b45113dec6180ba6373af514b15055","unresolved":false,"context_lines":[{"line_number":800,"context_line":"                    client_id \u003d \u0027*\u0027"},{"line_number":801,"context_line":"                    print(\u0027%u %s %s %s %s\u0027 %"},{"line_number":802,"context_line":"                          (cls.LEASE_EXP_MAX, mac, ip_address,"},{"line_number":803,"context_line":"                           hostname, client_id))"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a961159_5d6800f4","line":803,"in_reply_to":"3a961159_bd77243a","updated":"2015-01-15 12:11:15.000000000","message":"Here\u0027s an example of lua script doing init(): \nhttp://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2012q1/005425.html\n\nI can help you with that if you need. Lua is very simple, and the script would be rather tiny.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"51ad295f341af46c8f9ce30595ac48ae0b5ccc13","unresolved":false,"context_lines":[{"line_number":800,"context_line":"                    client_id \u003d \u0027*\u0027"},{"line_number":801,"context_line":"                    print(\u0027%u %s %s %s %s\u0027 %"},{"line_number":802,"context_line":"                          (cls.LEASE_EXP_MAX, mac, ip_address,"},{"line_number":803,"context_line":"                           hostname, client_id))"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":""},{"line_number":806,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a930d6b_17779c3b","line":803,"in_reply_to":"3a961159_e6a59403","updated":"2015-01-22 13:05:04.000000000","message":"@carl, it\u0027s not, I think I commented somewhere afterwards.\n\nBy the way, lua is not usable here, dnsmasq implementation is incomplete for the init() case.\n\nWe should only care about the python load time here at dnsmasq boot.","commit_id":"cfd7f9d0a6e0d7ace7582cbef5ef5925aa3c764d"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"0dcbc48a95112d6746ff81d3ba48a0dcfdedee2f","unresolved":false,"context_lines":[{"line_number":375,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % hosts_file,"},{"line_number":376,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self._output_addn_hosts_file(),"},{"line_number":377,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self._output_opts_file(),"},{"line_number":378,"context_line":"            \u0027--dhcp-script\u003d%s\u0027 % self._lease_script_path(),"},{"line_number":379,"context_line":"            \u0027--leasefile-ro\u0027,"},{"line_number":380,"context_line":"        ]"},{"line_number":381,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_8d70faa8","line":378,"updated":"2015-01-20 11:55:28.000000000","message":"for security reasons, if not specified, the script is going to be run as root, and that\u0027s not desirable:\n\nUse --dhcp-scriptuser\u003d current user ? (generally neutron)\n\n(Ihar pointed me out it was available on IRC)","commit_id":"f487b65952ff047b66ac04f11d7caba2468ce315"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"43a56d4ca05cd980cc0f3a60e6b8831cc2cd6931","unresolved":false,"context_lines":[{"line_number":802,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":803,"context_line":""},{"line_number":804,"context_line":"    @classmethod"},{"line_number":805,"context_line":"    def lease_init(cls, action\u003dNone, out\u003dsys.stdout):"},{"line_number":806,"context_line":"        if action:"},{"line_number":807,"context_line":"            _action \u003d action"},{"line_number":808,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a930d6b_242a967e","line":805,"updated":"2015-01-22 19:38:58.000000000","message":"Ok, I have measured the impact of this, by instrumentalizing lease_init, as the dnsmasq documentation was unclear, and don\u0027t want to talk about the 2000-line functions in it\u0027s code..., ok, it works well as expected.\n\nLook here for the result, I set the lease time to the minimum (120sec)\nhttp://paste.openstack.org/show/160413/\n\nIt seems that on every event, the script will be called (init, new, old, del).\n\ninit-\u003e start\nnew-\u003e new lease\nold -\u003e renew lease\ndel -\u003e cleared lease\n\n\nThat means that for every of those events from each instance, we will spawn a python script, which is not cheap.\n\n\nHereby I propose to write a single bash script per installation, along with the config files, and hardcode the init() results, to avoid needing to span any other bash tools (sed, awk, etc..)","commit_id":"f487b65952ff047b66ac04f11d7caba2468ce315"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"ce115cc086d392bf60897a13345e181d13b03567","unresolved":false,"context_lines":[{"line_number":802,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":803,"context_line":""},{"line_number":804,"context_line":"    @classmethod"},{"line_number":805,"context_line":"    def lease_init(cls, action\u003dNone, out\u003dsys.stdout):"},{"line_number":806,"context_line":"        if action:"},{"line_number":807,"context_line":"            _action \u003d action"},{"line_number":808,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a930d6b_b25ebf43","line":805,"in_reply_to":"1a930d6b_242a967e","updated":"2015-01-22 23:49:31.000000000","message":"See bash script in previous patch set.","commit_id":"f487b65952ff047b66ac04f11d7caba2468ce315"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"216b09c84d5a78b3a0b8395554a31c8f09b2c75b","unresolved":false,"context_lines":[{"line_number":802,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":803,"context_line":""},{"line_number":804,"context_line":"    @classmethod"},{"line_number":805,"context_line":"    def lease_init(cls, action\u003dNone, out\u003dsys.stdout):"},{"line_number":806,"context_line":"        if action:"},{"line_number":807,"context_line":"            _action \u003d action"},{"line_number":808,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a930d6b_e2c062e2","line":805,"in_reply_to":"1a930d6b_b25ebf43","updated":"2015-01-23 14:26:00.000000000","message":"I\u0027ve missed Miguel\u0027s suggestion at the very end of hist comment. I think that\u0027s actually the way to go. We will avoid maintaining a shell script, store all the code to generate lease-init event responses here, and avoid any potential forks inside shell script.\n\nAs a bonus point, the script will be posix compatible (using echo\u0027s only).","commit_id":"f487b65952ff047b66ac04f11d7caba2468ce315"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"b56ec6b4f8c0939714ada81f7bd2c313e7b920a0","unresolved":false,"context_lines":[{"line_number":802,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":803,"context_line":""},{"line_number":804,"context_line":"    @classmethod"},{"line_number":805,"context_line":"    def lease_init(cls, action\u003dNone, out\u003dsys.stdout):"},{"line_number":806,"context_line":"        if action:"},{"line_number":807,"context_line":"            _action \u003d action"},{"line_number":808,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a930d6b_045c8cec","line":805,"in_reply_to":"1a930d6b_e2c062e2","updated":"2015-01-23 20:28:43.000000000","message":"I think this is the best idea we have yet to improve this patch.","commit_id":"f487b65952ff047b66ac04f11d7caba2468ce315"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"81bb7f5b5036a9fb5a2fa4ca9ee7a10efd3c290e","unresolved":false,"context_lines":[{"line_number":825,"context_line":"                    client_id \u003d \u0027*\u0027"},{"line_number":826,"context_line":"                    out.write(\u0027%u %s %s %s %s\\n\u0027 %"},{"line_number":827,"context_line":"                          (cls.LEASE_EXP_MAX, mac, ip_address,"},{"line_number":828,"context_line":"                           hostname, client_id))"},{"line_number":829,"context_line":""},{"line_number":830,"context_line":""},{"line_number":831,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_7aff0203","line":828,"updated":"2015-01-20 11:51:28.000000000","message":"trying to guess how to write a lua script for this, I got into this dead end:\n\n\"\"\"  \n   At the moment there\u0027s no Lua equivalent of the shell-script\u0027s ability to\n   feed the lease database into dnsmasq at startup, so a Lua script can\u0027t \n   be used to replace the lease file in the way that a shell script can.\n   I\u0027m not sure of removing that limitation of easily possible.\n\"\"\"\n\nSo ignore it.\n\nYet, the shell script version, could be worth the reduced initialization time in a network node. Since network node startup times are bad enough already when handling lots of resources.\n\nCould you \n\n# HOSTS_FILE_ID_KEY\u003d.some_file_you_have. time neutron-dhcp-agent-dnsmasq-lease-init init\n\nseveral times in a console, and tell me how long does it take?","commit_id":"f487b65952ff047b66ac04f11d7caba2468ce315"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5aaf03578893d3745fa8bdd539e86eb44c33ace3","unresolved":false,"context_lines":[{"line_number":414,"context_line":"            cmd_callback\u003dself._build_cmdline_callback,"},{"line_number":415,"context_line":"            namespace\u003dself.network.namespace,"},{"line_number":416,"context_line":"            service\u003dDNSMASQ_SERVICE_NAME,"},{"line_number":417,"context_line":"            cmd_addl_env\u003d{self.NEUTRON_NETWORK_ID_KEY: self.network.id},"},{"line_number":418,"context_line":"            reload_cfg\u003dreload_with_HUP,"},{"line_number":419,"context_line":"            pid_file\u003dpid_filename)"},{"line_number":420,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"fa81d914_f99b2370","side":"PARENT","line":417,"updated":"2015-01-27 20:42:16.000000000","message":"This cleanup is irrelevant to the patch and is better split into a separate one.","commit_id":"f2f06faf893f2cbac282e324591312695cc23908"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"2091d71de0861ed78ff00d07be256fd51d6a031a","unresolved":false,"context_lines":[{"line_number":414,"context_line":"            cmd_callback\u003dself._build_cmdline_callback,"},{"line_number":415,"context_line":"            namespace\u003dself.network.namespace,"},{"line_number":416,"context_line":"            service\u003dDNSMASQ_SERVICE_NAME,"},{"line_number":417,"context_line":"            cmd_addl_env\u003d{self.NEUTRON_NETWORK_ID_KEY: self.network.id},"},{"line_number":418,"context_line":"            reload_cfg\u003dreload_with_HUP,"},{"line_number":419,"context_line":"            pid_file\u003dpid_filename)"},{"line_number":420,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"fa81d914_9a03d90e","side":"PARENT","line":417,"in_reply_to":"fa81d914_f99b2370","updated":"2015-01-28 13:05:22.000000000","message":"Ok, then I will also restore the test file and the root wrapper filter file.","commit_id":"f2f06faf893f2cbac282e324591312695cc23908"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"35705144704232f55249e64b90886dfd3165133c","unresolved":false,"context_lines":[{"line_number":336,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self.get_conf_file_name(\u0027host\u0027),"},{"line_number":337,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self.get_conf_file_name(\u0027addn_hosts\u0027),"},{"line_number":338,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self.get_conf_file_name(\u0027opts\u0027),"},{"line_number":339,"context_line":"            \u0027--dhcp-script\u003d%s\u0027 % self._lease_script_path(),"},{"line_number":340,"context_line":"            \u0027--leasefile-ro\u0027,"},{"line_number":341,"context_line":"        ]"},{"line_number":342,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"1a930d6b_169e6bb9","line":339,"updated":"2015-01-25 19:25:10.000000000","message":"add  --dhcp-scriptuser\u003d  with the agent running user please, otherwise the script will run as root, an attacker could manage to inject root operations.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"73cc8e6169ea9157db96c8b80bfd665540735f43","unresolved":false,"context_lines":[{"line_number":336,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self.get_conf_file_name(\u0027host\u0027),"},{"line_number":337,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self.get_conf_file_name(\u0027addn_hosts\u0027),"},{"line_number":338,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self.get_conf_file_name(\u0027opts\u0027),"},{"line_number":339,"context_line":"            \u0027--dhcp-script\u003d%s\u0027 % self._lease_script_path(),"},{"line_number":340,"context_line":"            \u0027--leasefile-ro\u0027,"},{"line_number":341,"context_line":"        ]"},{"line_number":342,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"1a930d6b_a55d5b49","line":339,"in_reply_to":"1a930d6b_169e6bb9","updated":"2015-01-26 07:06:22.000000000","message":"Miguel, thanks for the finding, but actually I tried the --dhcp-scriptuser option and it didn\u0027t work at all. The shadow dnsmasq process is created by the specified user, but when the lease-init script is invoked, it is still root, always.\n\nWe may need to find another way to solve this risk.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"cd46da59994126408e1b04555a095d3d39020bd1","unresolved":false,"context_lines":[{"line_number":336,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self.get_conf_file_name(\u0027host\u0027),"},{"line_number":337,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self.get_conf_file_name(\u0027addn_hosts\u0027),"},{"line_number":338,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self.get_conf_file_name(\u0027opts\u0027),"},{"line_number":339,"context_line":"            \u0027--dhcp-script\u003d%s\u0027 % self._lease_script_path(),"},{"line_number":340,"context_line":"            \u0027--leasefile-ro\u0027,"},{"line_number":341,"context_line":"        ]"},{"line_number":342,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"fa81d914_4a129363","line":339,"in_reply_to":"1a930d6b_7fb82511","updated":"2015-01-27 19:47:08.000000000","message":"Weird.  I agree with Miguel that we should keep this in here as long as it doesn\u0027t hurt.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"f93d0e12017d6e780711046ca117a4098afffaf0","unresolved":false,"context_lines":[{"line_number":336,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self.get_conf_file_name(\u0027host\u0027),"},{"line_number":337,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self.get_conf_file_name(\u0027addn_hosts\u0027),"},{"line_number":338,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self.get_conf_file_name(\u0027opts\u0027),"},{"line_number":339,"context_line":"            \u0027--dhcp-script\u003d%s\u0027 % self._lease_script_path(),"},{"line_number":340,"context_line":"            \u0027--leasefile-ro\u0027,"},{"line_number":341,"context_line":"        ]"},{"line_number":342,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"1a930d6b_7fb82511","line":339,"in_reply_to":"1a930d6b_8b459b44","updated":"2015-01-27 10:54:10.000000000","message":"I see in dnsmasq.c\n\nAs you said, in dnsmasq, they do the user change only for the renews/updates, not for the init (they use popen). That could be a bug, or intended (may be some init operations -for somebody- need root?) if that\u0027s the case, we could contribute dnsmasq with another option to allow dropping privileges for init too.\n\nI would set the --dhcp-scriptuser here (or at least a #TODO note), in the hope that we can change this into dnsmasq.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"50bc0dfc275dfee0c64cda366a2dde7c65e20217","unresolved":false,"context_lines":[{"line_number":336,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self.get_conf_file_name(\u0027host\u0027),"},{"line_number":337,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self.get_conf_file_name(\u0027addn_hosts\u0027),"},{"line_number":338,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self.get_conf_file_name(\u0027opts\u0027),"},{"line_number":339,"context_line":"            \u0027--dhcp-script\u003d%s\u0027 % self._lease_script_path(),"},{"line_number":340,"context_line":"            \u0027--leasefile-ro\u0027,"},{"line_number":341,"context_line":"        ]"},{"line_number":342,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"1a930d6b_8b459b44","line":339,"in_reply_to":"1a930d6b_a50f1687","updated":"2015-01-27 09:15:31.000000000","message":"Miguel, I think found the problem in dnsmasq code and reported a bug to launchpad (1414941). Since it is security related, it is by default private. And I subscribed you, Ihar and Carl to that bug.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"dc665134adee566f283bb1732daebc1abb0ae34b","unresolved":false,"context_lines":[{"line_number":336,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % self.get_conf_file_name(\u0027host\u0027),"},{"line_number":337,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self.get_conf_file_name(\u0027addn_hosts\u0027),"},{"line_number":338,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self.get_conf_file_name(\u0027opts\u0027),"},{"line_number":339,"context_line":"            \u0027--dhcp-script\u003d%s\u0027 % self._lease_script_path(),"},{"line_number":340,"context_line":"            \u0027--leasefile-ro\u0027,"},{"line_number":341,"context_line":"        ]"},{"line_number":342,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"1a930d6b_a50f1687","line":339,"in_reply_to":"1a930d6b_a55d5b49","updated":"2015-01-26 10:19:32.000000000","message":"I will check the dnsmasq sources to figure out why isn\u0027t it working, if that\u0027s the case may be we need to open a bug to dnsmasq.\n\nI will come back to remove the -1 once I\u0027ve checked.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"35705144704232f55249e64b90886dfd3165133c","unresolved":false,"context_lines":[{"line_number":808,"context_line":"    @staticmethod"},{"line_number":809,"context_line":"    def _lease_script_path():"},{"line_number":810,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":811,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":812,"context_line":""},{"line_number":813,"context_line":""},{"line_number":814,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a930d6b_76a96fde","line":811,"updated":"2015-01-25 19:25:10.000000000","message":"Instead of letting bash dynamically re-create the leases, the proposal in previous patchset was to create an script per network\n\nif \"init\" ...\n\n     echo \"....... entry 1\"\n     echo \"....... entry 2\"\n     echo \"....... entry 3\"\n\nfi\n\n\nThis way we avoid sub-process creation from the initial shell script and everything should be smoother.\n\nWe should not harm scalability of the dhcp-agent by having lots of small processes spawn for every lease, renewal or del.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"dc665134adee566f283bb1732daebc1abb0ae34b","unresolved":false,"context_lines":[{"line_number":808,"context_line":"    @staticmethod"},{"line_number":809,"context_line":"    def _lease_script_path():"},{"line_number":810,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":811,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":812,"context_line":""},{"line_number":813,"context_line":""},{"line_number":814,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a930d6b_3b11885d","line":811,"in_reply_to":"1a930d6b_05caaf1a","updated":"2015-01-26 10:19:32.000000000","message":"Hi Han,\n\nThank you for responding, I believe you\u0027re right:\n\n  $ wc --lines ~/host-test\n  1000 /home/centos/host-test\n\n  $ time HOSTS_FILE\u003d~/host-test ./neutron-dhcp-agent-dnsmasq-lease-init init \u003e/dev/null\n\n  real\t0m1.103s\n  user\t0m0.234s\n  sys\t0m0.941s\n\nFor just init, and 1 second for 1000 hosts seems rather acceptable.\n\nNormal invokation (non-init is ~2ms, to renew 1000 leases is 2s, quite reasonable too)\n\nI also suppose /bin/bash is going to be available in all systems, and you\u0027re explicitly invoking bash.\n\nIf nobody else has an issue with shipping/maintaining a bash script, I believe it\u0027s perfectly fine.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":161,"name":"Kevin Bringard","email":"kevinbri@cisco.com","username":"kbringard"},"change_message_id":"1a3ddde763dc46a047cdb4535ba7d075e66ef919","unresolved":false,"context_lines":[{"line_number":808,"context_line":"    @staticmethod"},{"line_number":809,"context_line":"    def _lease_script_path():"},{"line_number":810,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":811,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":812,"context_line":""},{"line_number":813,"context_line":""},{"line_number":814,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a930d6b_37195c22","line":811,"in_reply_to":"1a930d6b_3466c4ed","updated":"2015-01-27 14:39:38.000000000","message":"Just wanted to second this. As someone who\u0027s operated OpenStack at scale (tens of thousands of VMs) for a few years now, optimizing the control plane for minimal impact on the data plane is of huge importance to me. Having this operation happen as quickly, efficiently and securely as possible should be the focus.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"1b10822e9c04d33cf3d92ef0c42fed78edc28b42","unresolved":false,"context_lines":[{"line_number":808,"context_line":"    @staticmethod"},{"line_number":809,"context_line":"    def _lease_script_path():"},{"line_number":810,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":811,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":812,"context_line":""},{"line_number":813,"context_line":""},{"line_number":814,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a930d6b_f76fd428","line":811,"in_reply_to":"1a930d6b_37195c22","updated":"2015-01-27 14:45:04.000000000","message":"Sorry, I started by \"in any case doesn\u0027t seem reasonable to me\" but I forgot to correct that part. After doing the math, I found it acceptable within certain scalability constraints and lease time.\n\nBut yet, at high scale, with reduced lease time, it wouldn\u0027t be acceptable, not sure if that case is realistic or of any value. Yet the proposed bash implementation seems simple enough not to need any python spawn.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"0837f99798d79a45498edc3a45162a13b2e3f582","unresolved":false,"context_lines":[{"line_number":808,"context_line":"    @staticmethod"},{"line_number":809,"context_line":"    def _lease_script_path():"},{"line_number":810,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":811,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":812,"context_line":""},{"line_number":813,"context_line":""},{"line_number":814,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a930d6b_e8f6538e","line":811,"in_reply_to":"1a930d6b_3b11885d","updated":"2015-01-26 10:43:11.000000000","message":"Do we already ship anything implemented in Bash? That may require an explicit notion in some docs or even better, a sanity_check rule to check for bash existance.\n\nMiguel, Han, have you compared bash vs. bash, or bash vs. e.g. ash that is /bin/sh for Debian and Ubuntu?\n\nThere are several benefits in autogenerated script:\n- we avoid maintaining code in shell and use python for everything;\n- potential speedup due to ash used instead of bash.\n\nNot that I\u0027m totally against the current solution, I just don\u0027t see why we miss those two benefits while it\u0027s easy to achieve those.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"f93d0e12017d6e780711046ca117a4098afffaf0","unresolved":false,"context_lines":[{"line_number":808,"context_line":"    @staticmethod"},{"line_number":809,"context_line":"    def _lease_script_path():"},{"line_number":810,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":811,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":812,"context_line":""},{"line_number":813,"context_line":""},{"line_number":814,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a930d6b_3466c4ed","line":811,"in_reply_to":"1a930d6b_4e3f41b4","updated":"2015-01-27 10:54:10.000000000","message":"In any case, spawning a python VM, even on reduced imports doesn\u0027t seem reasonable to me.\n\n  $ cat \u003etest.py\n  import sys\n  print sys.argv\n  [centos@devstack neutron]$ time python test.py init\n  [\u0027test.py\u0027, \u0027init\u0027]\n\n  real\t0m0.022s\n\nWhich is 1 order of magnitude higher than bash itself:\n\n  $ cat \u003etest.sh\n  #!/bin/bash\n  echo $*\n\n  $ time bash test.sh init\n  init\n\n  real\t0m0.002s\n\n\n\nDoing some math (for 1000 networks * 10 instances, as we found that\u0027s more or less the limit a decent server could handle).\n\ninit time: (22ms+processing_time) * 1000 \u003d 22s + processing_time * 1000 \n\nminimum lease time (120s):\n22ms * (1/120) * 1000 * 10 \u003d 1833ms / s  \u003d183.3% of a core  (unacceptable)\n\ndefault lease time (86400s):\n\n22ms * (1/86400) * 1000 * 10 \u003d 2.4ms / s  (0.2% of a core) (nothing).\n\n\nfor bash:\n\nminimum lease time (120s):\n22ms * (1/120) * 1000 * 10 \u003d 183ms / s  (18% of a core) (acceptable)\n\ndefault lease time (86400s):\n\n22ms * (1/86400) * 1000 * 10 \u003d 0.24ms / s (0.02% of a core) (nothing).\n\n\nSo going with python would kill the extreme case of 10000 instances, and low lease time (not sure if there is a any good reason to go on such a low lease time)\n\n\nMy order of preference would be:\n\n1) generated script for the network, basically containing the host information, but in shell script format for dnsmasq consumption at init. \n\n2) importless python\n\n3) the current bash script (just weighed in Ihar concerns)\n\n1 vs 2 implementation differences seem minimal, and I believe 1 should be  preferred because the duplicating the information in two files (as only dnsmasq is the reader) should be a concern.\n\nBut, said that, 2 could perfectly be fine, I would like Carl Baldwin to come and give his opinion here.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"73cc8e6169ea9157db96c8b80bfd665540735f43","unresolved":false,"context_lines":[{"line_number":808,"context_line":"    @staticmethod"},{"line_number":809,"context_line":"    def _lease_script_path():"},{"line_number":810,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":811,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":812,"context_line":""},{"line_number":813,"context_line":""},{"line_number":814,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a930d6b_05caaf1a","line":811,"in_reply_to":"1a930d6b_76a96fde","updated":"2015-01-26 07:06:22.000000000","message":"Miguel, I think it would be kind of premature optimization if we go further. \n\nFor operations other than \"init\", the script exit immediately without creating any sub-processes. And on my test env (a VM with 2 cores) it is 5-8 ms. Besides, lease time is usually hours, and VMs belong to each neutron network is served by a separate dnsmasq process, so I doubt if there is scalability issue at all.\n\nFor \"init\", it is even less performance sensitive. On my test env it is around 50ms. How long it takes for the whole dhcp-agent startup? No matter how you improve this little script, you get only the tiny fraction of speed-up. Something else should be improved, if we are not satisfied with current startup time.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"50bc0dfc275dfee0c64cda366a2dde7c65e20217","unresolved":false,"context_lines":[{"line_number":808,"context_line":"    @staticmethod"},{"line_number":809,"context_line":"    def _lease_script_path():"},{"line_number":810,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":811,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":812,"context_line":""},{"line_number":813,"context_line":""},{"line_number":814,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a930d6b_4e3f41b4","line":811,"in_reply_to":"1a930d6b_e8f6538e","updated":"2015-01-27 09:15:31.000000000","message":"Ihar, I agree python is better for portability. But I am not quite comfortable with the idea of auto-generated shell script. It is weird that we maintain two copies of the same data for each network, and it is used for restart only.\n\nBash is still the available in most systems as fallback. But if it is really a concern, I would prefer an \"import-free\" version of python script. Performance is not the critical factor here. In real use cases, for a single network (handled by a dnsmasq process), renew rate would rarely above 1/s in average. Of course it depends on the size of the network and lease time, but tens of ms for a renew shouldn\u0027t be a problem.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"9d09c91d347843dc5c577ceb470ddd1bc70cfa02","unresolved":false,"context_lines":[{"line_number":808,"context_line":"    @staticmethod"},{"line_number":809,"context_line":"    def _lease_script_path():"},{"line_number":810,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":811,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":812,"context_line":""},{"line_number":813,"context_line":""},{"line_number":814,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"1a930d6b_c6dce848","line":811,"in_reply_to":"1a930d6b_f76fd428","updated":"2015-01-27 16:08:08.000000000","message":"Miguel,\n\nBased on your \u0027time\u0027 output above, I did a little test running both the python and bash two-line examples in a loop 1000 times, assuming there would be some improvement with a \u0027hot\u0027 cache.\n\nThe python version seemed to settle down to this after a little while:\n\nreal\t0m0.010s\nuser\t0m0.007s\nsys\t0m0.003s\n\nAnd the bash version:\n\nreal\t0m0.003s\nuser\t0m0.001s\nsys\t0m0.001s\n\nTotal clock time to run it 1000 times (python):\n\nreal\t0m12.136s\nuser\t0m8.921s\nsys\t0m3.218s\n\nAnd bash:\n\nreal\t0m2.300s\nuser\t0m1.132s\nsys\t0m1.231s\n\nSo if we\u0027re down at the minimum lease time of 120s (not likely I hope), and have thousands of VMs, then we shouldn\u0027t use python here like you said.\n\nGiven that, and looking at the shell scripts in tools/ we should just just /bin/sh and not use any bash-specific code in it since it seems that is discouraged, and /bin/sh will always be there, even if linked to another shell (like dash, which seemed even faster).\n\nIf we\u0027re only worried about 86400s and above then this really doesn\u0027t matter.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"cd46da59994126408e1b04555a095d3d39020bd1","unresolved":false,"context_lines":[{"line_number":808,"context_line":"    @staticmethod"},{"line_number":809,"context_line":"    def _lease_script_path():"},{"line_number":810,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":811,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":812,"context_line":""},{"line_number":813,"context_line":""},{"line_number":814,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa81d914_d0763406","line":811,"in_reply_to":"1a930d6b_f76fd428","updated":"2015-01-27 19:47:08.000000000","message":"Okay, we\u0027re still not in agreement.  Thanks everyone for your patience.  We\u0027ve got to come to an agreement soon so that we don\u0027t all get frustrated and miss out on this great fix which @Han has contributed and worked very hard on.\n\nReferring to @Miguel\u0027s third comment above (the long one):  I agree that #1 would be the best option.  I think it will be easier if we write two files (hosts file and the custom dhcp script) from the same source data rather than write the hosts file and then read and transform that data in to another form with another piece of code.  Basically, you\u0027d add another method which follows the same pattern as _output_hosts_file and call it from _output_config_files.  This is the most straight-forward option from a security perspective too, I think.\n\nWith these two options, I\u0027m no longer concerned about performance.  If we get a few milliseconds by using ash because we go with option #1 then that is great.  But, let\u0027s not base our decision on a few milliseconds.  @Kevin, I appreciate your sentiment, I also operate a cloud at scale.  But, we\u0027ve solved the scale issues that matter for this case.  Let\u0027s call it done from a performance perspective.\n\nI\u0027d rather not go for option #2 because I don\u0027t think it is worth sending @Han back to recode the script in python.\n\nWith all of this said, I don\u0027t think this current proposal is that bad.  I may -1 for a few small things but if it came down to it, I could probably be talked in to accepting this version if I had to.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"8b4abc1e256d35684a54180f75ab8efb39166ea3","unresolved":false,"context_lines":[{"line_number":808,"context_line":"    @staticmethod"},{"line_number":809,"context_line":"    def _lease_script_path():"},{"line_number":810,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":811,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":812,"context_line":""},{"line_number":813,"context_line":""},{"line_number":814,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa81d914_c0955ee5","line":811,"in_reply_to":"fa81d914_9a2cb9ad","updated":"2015-01-28 13:14:03.000000000","message":"I recommend not to waste time on ash/dash compatibility. Pure Bourne shell is very limited, and you probably won\u0027t be able to achieve POSIX compatibility for the script that needs to apply string replacements.\n\nThe script is mostly fine if we decide to go with the option #3. Just some nits.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"3c0639f57f84266c2adf184e8d0cf8f4580bde66","unresolved":false,"context_lines":[{"line_number":808,"context_line":"    @staticmethod"},{"line_number":809,"context_line":"    def _lease_script_path():"},{"line_number":810,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":811,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":812,"context_line":""},{"line_number":813,"context_line":""},{"line_number":814,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa81d914_8a6ef2ff","line":811,"in_reply_to":"fa81d914_b75508c8","updated":"2015-01-29 09:48:58.000000000","message":"I\u0027m for moving with option #3 for now.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"fd165103dbeabdff79b65eac5e07666860731b72","unresolved":false,"context_lines":[{"line_number":808,"context_line":"    @staticmethod"},{"line_number":809,"context_line":"    def _lease_script_path():"},{"line_number":810,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":811,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":812,"context_line":""},{"line_number":813,"context_line":""},{"line_number":814,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa81d914_c1047d17","line":811,"in_reply_to":"fa81d914_c0955ee5","updated":"2015-01-28 14:54:45.000000000","message":"I\u0027m ok with option #3 too, we can spend some time to test it at scale and see how it does before Kilo release, and then decide if change into #1 is necessary at all, or just an over-optimization.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"690e49abce39be6182a97331c3196fba46748020","unresolved":false,"context_lines":[{"line_number":808,"context_line":"    @staticmethod"},{"line_number":809,"context_line":"    def _lease_script_path():"},{"line_number":810,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":811,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":812,"context_line":""},{"line_number":813,"context_line":""},{"line_number":814,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa81d914_b75508c8","line":811,"in_reply_to":"fa81d914_c1047d17","updated":"2015-01-28 18:53:30.000000000","message":"So, are we agreeing to polish up option #3 and go with it?  I can support that.  I made notes in the script to clean it up a bit.\n\nA few other responses which may be moot if we are deciding to go with #3:\n\n@Han Redundant data in two files is not a problem.  They would both be generated entirely automatically from the same source data at the same time.\n\nYou\u0027re right that we pay a small price for writing the file.  But, we\u0027re writing 3 other files at the same time.  So, we\u0027re not changing the complexity of the solution, we\u0027re dealing with small constant factors here.  So, I wouldn\u0027t concern myself with that either.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"2091d71de0861ed78ff00d07be256fd51d6a031a","unresolved":false,"context_lines":[{"line_number":808,"context_line":"    @staticmethod"},{"line_number":809,"context_line":"    def _lease_script_path():"},{"line_number":810,"context_line":"        return os.path.join(os.path.dirname(sys.argv[0]),"},{"line_number":811,"context_line":"                            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027)"},{"line_number":812,"context_line":""},{"line_number":813,"context_line":""},{"line_number":814,"context_line":"class DeviceManager(object):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fa81d914_9a2cb9ad","line":811,"in_reply_to":"fa81d914_d0763406","updated":"2015-01-28 13:05:22.000000000","message":"If with go with option #1, an extra cost is updating the script file for every port creation/deletion. Besides, data will be redundant in both files, which I don\u0027t think is good. Finally, we still have to wrap the script with another script: to check the command line args, and if it is not \"init\" then exit. Really weird, isn\u0027t it?\n\nSo, I would like to go ahead with the current solution (option #3), and try to make it dash/ash compatible.","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"8eeacc0c99bdc68fde356860339527c388401321","unresolved":false,"context_lines":[{"line_number":319,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self.get_conf_file_name(\u0027addn_hosts\u0027),"},{"line_number":320,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self.get_conf_file_name(\u0027opts\u0027),"},{"line_number":321,"context_line":"            \u0027--dhcp-script\u003d%s\u0027 % self._lease_script_path(),"},{"line_number":322,"context_line":"            \u0027--dhcp-scriptuser\u003d%s\u0027 % os.environ.get(\u0027USER\u0027),"},{"line_number":323,"context_line":"            \u0027--leasefile-ro\u0027,"},{"line_number":324,"context_line":"        ]"},{"line_number":325,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"da86d52c_77bbb2c4","line":322,"updated":"2015-01-31 14:58:02.000000000","message":"getpass.getuser()","commit_id":"25ac6066f630e3a1118f38e396034790da5bd323"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"9e1d4cb96bae3a40781a763b3b2425b61fd7d689","unresolved":false,"context_lines":[{"line_number":319,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self.get_conf_file_name(\u0027addn_hosts\u0027),"},{"line_number":320,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self.get_conf_file_name(\u0027opts\u0027),"},{"line_number":321,"context_line":"            \u0027--dhcp-script\u003d%s\u0027 % self._lease_script_path(),"},{"line_number":322,"context_line":"            \u0027--dhcp-scriptuser\u003d%s\u0027 % os.environ.get(\u0027USER\u0027),"},{"line_number":323,"context_line":"            \u0027--leasefile-ro\u0027,"},{"line_number":324,"context_line":"        ]"},{"line_number":325,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"da86d52c_b7036a72","line":322,"in_reply_to":"da86d52c_77bbb2c4","updated":"2015-01-31 15:03:25.000000000","message":"On second thought, it\u0027s not ok either since we should not rely on environment to determine the user to run the script against. Instead, use getuid() + getpwuid(), like: pwd.getpwuid(os.getuid()).pw_name","commit_id":"25ac6066f630e3a1118f38e396034790da5bd323"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"6e077ac68f72de52e427e06bdb6320dd985c4c6a","unresolved":false,"context_lines":[{"line_number":320,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self.get_conf_file_name(\u0027addn_hosts\u0027),"},{"line_number":321,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self.get_conf_file_name(\u0027opts\u0027),"},{"line_number":322,"context_line":"            \u0027--dhcp-script\u003d%s\u0027 % self._lease_script_path(),"},{"line_number":323,"context_line":"            \u0027--dhcp-scriptuser\u003d%s\u0027 % pwd.getpwuid(os.getuid()).pw_name,"},{"line_number":324,"context_line":"            \u0027--leasefile-ro\u0027,"},{"line_number":325,"context_line":"        ]"},{"line_number":326,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_bd3c0e95","line":323,"updated":"2015-02-02 12:34:37.000000000","message":"(No action required)\nI suspect that would be a nice candidate for moving into a separate helper function, but of course you should not do it in this patch.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"0887ef480ef45522831a108453025d54165a9efc","unresolved":false,"context_lines":[{"line_number":320,"context_line":"            \u0027--addn-hosts\u003d%s\u0027 % self.get_conf_file_name(\u0027addn_hosts\u0027),"},{"line_number":321,"context_line":"            \u0027--dhcp-optsfile\u003d%s\u0027 % self.get_conf_file_name(\u0027opts\u0027),"},{"line_number":322,"context_line":"            \u0027--dhcp-script\u003d%s\u0027 % self._lease_script_path(),"},{"line_number":323,"context_line":"            \u0027--dhcp-scriptuser\u003d%s\u0027 % pwd.getpwuid(os.getuid()).pw_name,"},{"line_number":324,"context_line":"            \u0027--leasefile-ro\u0027,"},{"line_number":325,"context_line":"        ]"},{"line_number":326,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_6e55821c","line":323,"updated":"2015-02-02 10:15:20.000000000","message":"Thank you very much for looking at this. I\u0027ll keep you updated about conversation with the U/S dnsmasq.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"0887ef480ef45522831a108453025d54165a9efc","unresolved":false,"context_lines":[{"line_number":403,"context_line":"            namespace\u003dself.network.namespace,"},{"line_number":404,"context_line":"            service\u003dDNSMASQ_SERVICE_NAME,"},{"line_number":405,"context_line":"            cmd_addl_env\u003d{self.HOSTS_FILE_KEY: host_filename,"},{"line_number":406,"context_line":"                          self.NEUTRON_NETWORK_ID_KEY: self.network.id},"},{"line_number":407,"context_line":"            reload_cfg\u003dreload_with_HUP,"},{"line_number":408,"context_line":"            pid_file\u003dpid_filename)"},{"line_number":409,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_ee17b24f","line":406,"updated":"2015-02-02 10:15:20.000000000","message":"why do we pass yet the NEUTRON_NETWORK_ID  is it used somewhere?.\n\nIf we aren\u0027t using it I\u0027d remove it.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"15f0e8a699796be47c07fb6a2bc2c81efe95e053","unresolved":false,"context_lines":[{"line_number":403,"context_line":"            namespace\u003dself.network.namespace,"},{"line_number":404,"context_line":"            service\u003dDNSMASQ_SERVICE_NAME,"},{"line_number":405,"context_line":"            cmd_addl_env\u003d{self.HOSTS_FILE_KEY: host_filename,"},{"line_number":406,"context_line":"                          self.NEUTRON_NETWORK_ID_KEY: self.network.id},"},{"line_number":407,"context_line":"            reload_cfg\u003dreload_with_HUP,"},{"line_number":408,"context_line":"            pid_file\u003dpid_filename)"},{"line_number":409,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_1d5efa45","line":406,"in_reply_to":"da86d52c_1d4b5a2c","updated":"2015-02-02 12:43:45.000000000","message":"Ack, thanks for chipping in. Seems reasonable to me.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"6e077ac68f72de52e427e06bdb6320dd985c4c6a","unresolved":false,"context_lines":[{"line_number":403,"context_line":"            namespace\u003dself.network.namespace,"},{"line_number":404,"context_line":"            service\u003dDNSMASQ_SERVICE_NAME,"},{"line_number":405,"context_line":"            cmd_addl_env\u003d{self.HOSTS_FILE_KEY: host_filename,"},{"line_number":406,"context_line":"                          self.NEUTRON_NETWORK_ID_KEY: self.network.id},"},{"line_number":407,"context_line":"            reload_cfg\u003dreload_with_HUP,"},{"line_number":408,"context_line":"            pid_file\u003dpid_filename)"},{"line_number":409,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_1d4b5a2c","line":406,"in_reply_to":"da86d52c_ee17b24f","updated":"2015-02-02 12:34:37.000000000","message":"I\u0027ve asked the author before to avoid cleanup in this patch. It should be cleaned up later separately.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"0887ef480ef45522831a108453025d54165a9efc","unresolved":false,"context_lines":[{"line_number":764,"context_line":"        isolated_subnets \u003d cls.get_isolated_subnets(network)"},{"line_number":765,"context_line":"        return any(isolated_subnets[subnet.id] for subnet in network.subnets)"},{"line_number":766,"context_line":""},{"line_number":767,"context_line":"    @classmethod"},{"line_number":768,"context_line":"    def lease_update(cls):"},{"line_number":769,"context_line":"        network_id \u003d os.environ.get(cls.NEUTRON_NETWORK_ID_KEY)"},{"line_number":770,"context_line":"        dhcp_relay_socket \u003d os.environ.get(cls.NEUTRON_RELAY_SOCKET_PATH_KEY)"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_f10eb733","line":767,"updated":"2015-02-02 10:15:20.000000000","message":"Please:\nThis is not used anymore, and IMHO can be removed in this patch, as far as I understand, this was the old script that it was passed via --dhcp-script in Havana, right?.\n\nPlease let me know if I\u0027m wrong.\n\nWe could do it in a follow up patch if you want, no need to extend scope or hold the patch too much for this. But I would like to make sure we clear up any old code or unused environment variables.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"15f0e8a699796be47c07fb6a2bc2c81efe95e053","unresolved":false,"context_lines":[{"line_number":764,"context_line":"        isolated_subnets \u003d cls.get_isolated_subnets(network)"},{"line_number":765,"context_line":"        return any(isolated_subnets[subnet.id] for subnet in network.subnets)"},{"line_number":766,"context_line":""},{"line_number":767,"context_line":"    @classmethod"},{"line_number":768,"context_line":"    def lease_update(cls):"},{"line_number":769,"context_line":"        network_id \u003d os.environ.get(cls.NEUTRON_NETWORK_ID_KEY)"},{"line_number":770,"context_line":"        dhcp_relay_socket \u003d os.environ.get(cls.NEUTRON_RELAY_SOCKET_PATH_KEY)"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_7d2c56ac","line":767,"in_reply_to":"da86d52c_1d727a7c","updated":"2015-02-02 12:43:45.000000000","message":"agreement.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"6e077ac68f72de52e427e06bdb6320dd985c4c6a","unresolved":false,"context_lines":[{"line_number":764,"context_line":"        isolated_subnets \u003d cls.get_isolated_subnets(network)"},{"line_number":765,"context_line":"        return any(isolated_subnets[subnet.id] for subnet in network.subnets)"},{"line_number":766,"context_line":""},{"line_number":767,"context_line":"    @classmethod"},{"line_number":768,"context_line":"    def lease_update(cls):"},{"line_number":769,"context_line":"        network_id \u003d os.environ.get(cls.NEUTRON_NETWORK_ID_KEY)"},{"line_number":770,"context_line":"        dhcp_relay_socket \u003d os.environ.get(cls.NEUTRON_RELAY_SOCKET_PATH_KEY)"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_1d727a7c","line":767,"in_reply_to":"da86d52c_f10eb733","updated":"2015-02-02 12:34:37.000000000","message":"+1 for separate patch.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"}],"neutron/tests/functional/agent/linux/test_dnsmasq_lease_script.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"6e077ac68f72de52e427e06bdb6320dd985c4c6a","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright (c) 2015 OpenStack Foundation."},{"line_number":2,"context_line":"# All Rights Reserved."},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_7d69b68a","line":1,"updated":"2015-02-02 12:34:37.000000000","message":"Do you work for the foundation?","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"20d594f843d538667719672a1d9e54393b67d5cf","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright (c) 2015 OpenStack Foundation."},{"line_number":2,"context_line":"# All Rights Reserved."},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_b45b362d","line":1,"in_reply_to":"da86d52c_7d69b68a","updated":"2015-02-02 17:59:04.000000000","message":"I get people complaining about my copyright headers either way I do them.  I really don\u0027t care what tho copyright line has.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"6e077ac68f72de52e427e06bdb6320dd985c4c6a","unresolved":false,"context_lines":[{"line_number":17,"context_line":"import subprocess"},{"line_number":18,"context_line":"import tempfile"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"from neutron.agent.linux.dhcp import Dnsmasq"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"from neutron.tests.functional.agent.linux import base"},{"line_number":23,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_5d0312b8","line":20,"updated":"2015-02-02 12:34:37.000000000","message":"(No action required)\nSo we import an object, and pep8 does not complain? That is smth new.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"6e077ac68f72de52e427e06bdb6320dd985c4c6a","unresolved":false,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class TestDnsmasqLeaseScript(base.BaseLinuxTestCase):"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    SCRIPT \u003d \u0027bin/neutron-dhcp-agent-dnsmasq-lease-init\u0027"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    def test_init(self):"},{"line_number":30,"context_line":"        lines \u003d [\u002700:00:80:aa:bb:cc, inst-name, 1.2.3.4\\n\u0027,"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_1de7da13","line":27,"updated":"2015-02-02 12:34:37.000000000","message":"I believe the name can be shared with neutron/agent/linux/dhcp.py thru a common constant.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"20d594f843d538667719672a1d9e54393b67d5cf","unresolved":false,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class TestDnsmasqLeaseScript(base.BaseLinuxTestCase):"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    SCRIPT \u003d \u0027bin/neutron-dhcp-agent-dnsmasq-lease-init\u0027"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    def test_init(self):"},{"line_number":30,"context_line":"        lines \u003d [\u002700:00:80:aa:bb:cc, inst-name, 1.2.3.4\\n\u0027,"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_3447464d","line":27,"in_reply_to":"da86d52c_1de7da13","updated":"2015-02-02 17:59:04.000000000","message":"Done","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"6e077ac68f72de52e427e06bdb6320dd985c4c6a","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    SCRIPT \u003d \u0027bin/neutron-dhcp-agent-dnsmasq-lease-init\u0027"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    def test_init(self):"},{"line_number":30,"context_line":"        lines \u003d [\u002700:00:80:aa:bb:cc, inst-name, 1.2.3.4\\n\u0027,"},{"line_number":31,"context_line":"                 \u002700:00:80:aa:bb:cc, inst-name,[de:ad:be::ef]\\n\u0027]"},{"line_number":32,"context_line":"        fd, hostsfile_name \u003d tempfile.mkstemp(prefix\u003d\u0027test-hosts\u0027, text\u003dTrue)"},{"line_number":33,"context_line":"        with file(hostsfile_name, \u0027w\u0027) as f:"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_fd03e670","line":30,"updated":"2015-02-02 12:34:37.000000000","message":"I believe we don\u0027t have spaces after commas in the code.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"20d594f843d538667719672a1d9e54393b67d5cf","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    SCRIPT \u003d \u0027bin/neutron-dhcp-agent-dnsmasq-lease-init\u0027"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    def test_init(self):"},{"line_number":30,"context_line":"        lines \u003d [\u002700:00:80:aa:bb:cc, inst-name, 1.2.3.4\\n\u0027,"},{"line_number":31,"context_line":"                 \u002700:00:80:aa:bb:cc, inst-name,[de:ad:be::ef]\\n\u0027]"},{"line_number":32,"context_line":"        fd, hostsfile_name \u003d tempfile.mkstemp(prefix\u003d\u0027test-hosts\u0027, text\u003dTrue)"},{"line_number":33,"context_line":"        with file(hostsfile_name, \u0027w\u0027) as f:"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_d4afcae1","line":30,"in_reply_to":"da86d52c_5d06b221","updated":"2015-02-02 17:59:04.000000000","message":"I think I\u0027d rather the spaces not be here.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"15f0e8a699796be47c07fb6a2bc2c81efe95e053","unresolved":false,"context_lines":[{"line_number":27,"context_line":"    SCRIPT \u003d \u0027bin/neutron-dhcp-agent-dnsmasq-lease-init\u0027"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    def test_init(self):"},{"line_number":30,"context_line":"        lines \u003d [\u002700:00:80:aa:bb:cc, inst-name, 1.2.3.4\\n\u0027,"},{"line_number":31,"context_line":"                 \u002700:00:80:aa:bb:cc, inst-name,[de:ad:be::ef]\\n\u0027]"},{"line_number":32,"context_line":"        fd, hostsfile_name \u003d tempfile.mkstemp(prefix\u003d\u0027test-hosts\u0027, text\u003dTrue)"},{"line_number":33,"context_line":"        with file(hostsfile_name, \u0027w\u0027) as f:"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_5d06b221","line":30,"in_reply_to":"da86d52c_fd03e670","updated":"2015-02-02 12:43:45.000000000","message":"But it\u0027s not bad to check we can also handle such thing, as long as we also handle without spaces, that doesn\u0027t necessarily have to be the case btw...","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"6e077ac68f72de52e427e06bdb6320dd985c4c6a","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    def test_init(self):"},{"line_number":30,"context_line":"        lines \u003d [\u002700:00:80:aa:bb:cc, inst-name, 1.2.3.4\\n\u0027,"},{"line_number":31,"context_line":"                 \u002700:00:80:aa:bb:cc, inst-name,[de:ad:be::ef]\\n\u0027]"},{"line_number":32,"context_line":"        fd, hostsfile_name \u003d tempfile.mkstemp(prefix\u003d\u0027test-hosts\u0027, text\u003dTrue)"},{"line_number":33,"context_line":"        with file(hostsfile_name, \u0027w\u0027) as f:"},{"line_number":34,"context_line":"            f.writelines(lines)"},{"line_number":35,"context_line":"        os.close(fd)"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_7d9c566a","line":32,"updated":"2015-02-02 12:34:37.000000000","message":"We now have self.get_temp_file_path() from BaseTestCase.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"15f0e8a699796be47c07fb6a2bc2c81efe95e053","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    def test_init(self):"},{"line_number":30,"context_line":"        lines \u003d [\u002700:00:80:aa:bb:cc, inst-name, 1.2.3.4\\n\u0027,"},{"line_number":31,"context_line":"                 \u002700:00:80:aa:bb:cc, inst-name,[de:ad:be::ef]\\n\u0027]"},{"line_number":32,"context_line":"        fd, hostsfile_name \u003d tempfile.mkstemp(prefix\u003d\u0027test-hosts\u0027, text\u003dTrue)"},{"line_number":33,"context_line":"        with file(hostsfile_name, \u0027w\u0027) as f:"},{"line_number":34,"context_line":"            f.writelines(lines)"},{"line_number":35,"context_line":"        os.close(fd)"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_98ff5801","line":32,"in_reply_to":"da86d52c_7d9c566a","updated":"2015-02-02 12:43:45.000000000","message":"Ah, true!,","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"20d594f843d538667719672a1d9e54393b67d5cf","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    def test_init(self):"},{"line_number":30,"context_line":"        lines \u003d [\u002700:00:80:aa:bb:cc, inst-name, 1.2.3.4\\n\u0027,"},{"line_number":31,"context_line":"                 \u002700:00:80:aa:bb:cc, inst-name,[de:ad:be::ef]\\n\u0027]"},{"line_number":32,"context_line":"        fd, hostsfile_name \u003d tempfile.mkstemp(prefix\u003d\u0027test-hosts\u0027, text\u003dTrue)"},{"line_number":33,"context_line":"        with file(hostsfile_name, \u0027w\u0027) as f:"},{"line_number":34,"context_line":"            f.writelines(lines)"},{"line_number":35,"context_line":"        os.close(fd)"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_2660fd31","line":32,"in_reply_to":"da86d52c_98ff5801","updated":"2015-02-02 17:59:04.000000000","message":"Done","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"20d594f843d538667719672a1d9e54393b67d5cf","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        fd, hostsfile_name \u003d tempfile.mkstemp(prefix\u003d\u0027test-hosts\u0027, text\u003dTrue)"},{"line_number":33,"context_line":"        with file(hostsfile_name, \u0027w\u0027) as f:"},{"line_number":34,"context_line":"            f.writelines(lines)"},{"line_number":35,"context_line":"        os.close(fd)"},{"line_number":36,"context_line":"        os.environ[Dnsmasq.HOSTS_FILE_KEY] \u003d hostsfile_name"},{"line_number":37,"context_line":"        try:"},{"line_number":38,"context_line":"            output \u003d subprocess.check_output([self.SCRIPT, \u0027init\u0027])"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_6b51de61","line":35,"updated":"2015-02-02 17:59:04.000000000","message":"Create the file on L33 with os.fdopen() and you won\u0027t need this.\n\nupdate:  This comment is obsolete with L32 change.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"6e077ac68f72de52e427e06bdb6320dd985c4c6a","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        with file(hostsfile_name, \u0027w\u0027) as f:"},{"line_number":34,"context_line":"            f.writelines(lines)"},{"line_number":35,"context_line":"        os.close(fd)"},{"line_number":36,"context_line":"        os.environ[Dnsmasq.HOSTS_FILE_KEY] \u003d hostsfile_name"},{"line_number":37,"context_line":"        try:"},{"line_number":38,"context_line":"            output \u003d subprocess.check_output([self.SCRIPT, \u0027init\u0027])"},{"line_number":39,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_7daa9671","line":36,"updated":"2015-02-02 12:34:37.000000000","message":"You modify global environment. You should not do it.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"20d594f843d538667719672a1d9e54393b67d5cf","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        with file(hostsfile_name, \u0027w\u0027) as f:"},{"line_number":34,"context_line":"            f.writelines(lines)"},{"line_number":35,"context_line":"        os.close(fd)"},{"line_number":36,"context_line":"        os.environ[Dnsmasq.HOSTS_FILE_KEY] \u003d hostsfile_name"},{"line_number":37,"context_line":"        try:"},{"line_number":38,"context_line":"            output \u003d subprocess.check_output([self.SCRIPT, \u0027init\u0027])"},{"line_number":39,"context_line":"        except Exception:"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_665bd554","line":36,"in_reply_to":"da86d52c_7daa9671","updated":"2015-02-02 17:59:04.000000000","message":"Done (will use fixtures)","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"6e077ac68f72de52e427e06bdb6320dd985c4c6a","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        try:"},{"line_number":38,"context_line":"            output \u003d subprocess.check_output([self.SCRIPT, \u0027init\u0027])"},{"line_number":39,"context_line":"        except Exception:"},{"line_number":40,"context_line":"            raise"},{"line_number":41,"context_line":"        finally:"},{"line_number":42,"context_line":"            os.remove(hostsfile_name)"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_dd99c26f","line":40,"updated":"2015-02-02 12:34:37.000000000","message":"What does this except section mean? I think it\u0027s redundant.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"44adc00c7ff0d66a7c2818d4b1d427b219c027a9","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        try:"},{"line_number":38,"context_line":"            output \u003d subprocess.check_output([self.SCRIPT, \u0027init\u0027])"},{"line_number":39,"context_line":"        except Exception:"},{"line_number":40,"context_line":"            raise"},{"line_number":41,"context_line":"        finally:"},{"line_number":42,"context_line":"            os.remove(hostsfile_name)"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_584b2059","line":40,"in_reply_to":"da86d52c_1813685d","updated":"2015-02-02 12:46:47.000000000","message":"finally is called anyway.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"20d594f843d538667719672a1d9e54393b67d5cf","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        try:"},{"line_number":38,"context_line":"            output \u003d subprocess.check_output([self.SCRIPT, \u0027init\u0027])"},{"line_number":39,"context_line":"        except Exception:"},{"line_number":40,"context_line":"            raise"},{"line_number":41,"context_line":"        finally:"},{"line_number":42,"context_line":"            os.remove(hostsfile_name)"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_553afdcd","line":40,"in_reply_to":"da86d52c_584b2059","updated":"2015-02-02 17:59:04.000000000","message":"Right, these two lines could be removed but I it isn\u0027t a big deal.  If this is the only thing left then I\u0027ll fix it on my own.\n\nIn fact, with the use of get_temp_file_path on L32 the finally is not necessary.  The temp dir will get all cleaned up by the fixture.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"15f0e8a699796be47c07fb6a2bc2c81efe95e053","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        try:"},{"line_number":38,"context_line":"            output \u003d subprocess.check_output([self.SCRIPT, \u0027init\u0027])"},{"line_number":39,"context_line":"        except Exception:"},{"line_number":40,"context_line":"            raise"},{"line_number":41,"context_line":"        finally:"},{"line_number":42,"context_line":"            os.remove(hostsfile_name)"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_1813685d","line":40,"in_reply_to":"da86d52c_dd99c26f","updated":"2015-02-02 12:43:45.000000000","message":"I believe it\u0027s good to grant the removal of the hostsfile_name in case of an unexpected exception.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"0887ef480ef45522831a108453025d54165a9efc","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        except Exception:"},{"line_number":40,"context_line":"            raise"},{"line_number":41,"context_line":"        finally:"},{"line_number":42,"context_line":"            os.remove(hostsfile_name)"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"        lease \u003d (2100 - 1970) * 365 * 24 * 3600"},{"line_number":45,"context_line":"        self.assertEqual("}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_d1f37308","line":42,"updated":"2015-02-02 10:15:20.000000000","message":"in finally: os.environ[....HOSTS_FILE_KEY]\u003d\u0027\u0027  ?","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"20d594f843d538667719672a1d9e54393b67d5cf","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        except Exception:"},{"line_number":40,"context_line":"            raise"},{"line_number":41,"context_line":"        finally:"},{"line_number":42,"context_line":"            os.remove(hostsfile_name)"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"        lease \u003d (2100 - 1970) * 365 * 24 * 3600"},{"line_number":45,"context_line":"        self.assertEqual("}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_35ed89c4","line":42,"in_reply_to":"da86d52c_d1f37308","updated":"2015-02-02 17:59:04.000000000","message":"Use a fixture.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"0887ef480ef45522831a108453025d54165a9efc","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        finally:"},{"line_number":42,"context_line":"            os.remove(hostsfile_name)"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"        lease \u003d (2100 - 1970) * 365 * 24 * 3600"},{"line_number":45,"context_line":"        self.assertEqual("},{"line_number":46,"context_line":"            \u0027%s 00:00:80:aa:bb:cc 1.2.3.4 host-1-2-3-4 *\\n\u0027"},{"line_number":47,"context_line":"            \u0027%s 00:00:80:aa:bb:cc de:ad:be::ef host-de-ad-be--ef *\\n\u0027 %"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_d16e13c9","line":44,"updated":"2015-02-02 10:15:20.000000000","message":"please grab the lease constant from the neutron.agent.linux.dhcp module, so if there is an update in the main code we don\u0027t need to go here and fix the constant also.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"20d594f843d538667719672a1d9e54393b67d5cf","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        finally:"},{"line_number":42,"context_line":"            os.remove(hostsfile_name)"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"        lease \u003d (2100 - 1970) * 365 * 24 * 3600"},{"line_number":45,"context_line":"        self.assertEqual("},{"line_number":46,"context_line":"            \u0027%s 00:00:80:aa:bb:cc 1.2.3.4 host-1-2-3-4 *\\n\u0027"},{"line_number":47,"context_line":"            \u0027%s 00:00:80:aa:bb:cc de:ad:be::ef host-de-ad-be--ef *\\n\u0027 %"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_611d7f0f","line":44,"in_reply_to":"da86d52c_d16e13c9","updated":"2015-02-02 17:59:04.000000000","message":"The constant is no longer there.  It is in the bash code.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"6e077ac68f72de52e427e06bdb6320dd985c4c6a","unresolved":false,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"        lease \u003d (2100 - 1970) * 365 * 24 * 3600"},{"line_number":45,"context_line":"        self.assertEqual("},{"line_number":46,"context_line":"            \u0027%s 00:00:80:aa:bb:cc 1.2.3.4 host-1-2-3-4 *\\n\u0027"},{"line_number":47,"context_line":"            \u0027%s 00:00:80:aa:bb:cc de:ad:be::ef host-de-ad-be--ef *\\n\u0027 %"},{"line_number":48,"context_line":"            (lease, lease),"},{"line_number":49,"context_line":"            output)"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_dd0c6297","line":46,"updated":"2015-02-02 12:34:37.000000000","message":"I believe you should somehow reconstruct those expected lines from values commonly used for constructing \u0027lines\u0027 above.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"20d594f843d538667719672a1d9e54393b67d5cf","unresolved":false,"context_lines":[{"line_number":43,"context_line":""},{"line_number":44,"context_line":"        lease \u003d (2100 - 1970) * 365 * 24 * 3600"},{"line_number":45,"context_line":"        self.assertEqual("},{"line_number":46,"context_line":"            \u0027%s 00:00:80:aa:bb:cc 1.2.3.4 host-1-2-3-4 *\\n\u0027"},{"line_number":47,"context_line":"            \u0027%s 00:00:80:aa:bb:cc de:ad:be::ef host-de-ad-be--ef *\\n\u0027 %"},{"line_number":48,"context_line":"            (lease, lease),"},{"line_number":49,"context_line":"            output)"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_b1b428d2","line":46,"in_reply_to":"da86d52c_dd0c6297","updated":"2015-02-02 17:59:04.000000000","message":"I think if more tests are added that reuse the data then this makes sense but for this case it is just extra work.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"0887ef480ef45522831a108453025d54165a9efc","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def test_renew(self):"},{"line_number":52,"context_line":"        output \u003d subprocess.check_output([self.SCRIPT, \u0027old\u0027])"},{"line_number":53,"context_line":"        self.assertEqual(\u0027\u0027, output)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def test_hostsfile_not_exist(self):"},{"line_number":56,"context_line":"        os.environ[Dnsmasq.HOSTS_FILE_KEY] \u003d \u0027\u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_718d677b","line":53,"updated":"2015-02-02 10:15:20.000000000","message":"You may test here that even passing a HOSTS_FILE with data and $0 !\u003d init, still this output is empty.\n\nI would refactor out  the test file writing to a function for reuse in both tests.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"20d594f843d538667719672a1d9e54393b67d5cf","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        self.assertEqual(\u0027\u0027, output)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def test_hostsfile_not_exist(self):"},{"line_number":56,"context_line":"        os.environ[Dnsmasq.HOSTS_FILE_KEY] \u003d \u0027\u0027"},{"line_number":57,"context_line":"        output \u003d subprocess.check_output([self.SCRIPT, \u0027init\u0027])"},{"line_number":58,"context_line":"        self.assertEqual(\u0027\u0027, output)"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_71ed5025","line":56,"updated":"2015-02-02 17:59:04.000000000","message":"Should probably use a real file name for this test and maybe add another tests for not-specified.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"}],"neutron/tests/unit/test_linux_dhcp.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"a2308993d99876e00838017c0e84546f14ff6f82","unresolved":false,"context_lines":[{"line_number":1292,"context_line":"        mock_exists.assert_called_once_with(filename)"},{"line_number":1293,"context_line":"        mock_open.assert_called_once_with(filename)"},{"line_number":1294,"context_line":""},{"line_number":1295,"context_line":"    def test_lease_init(self):"},{"line_number":1296,"context_line":"        filename \u003d \u0027hostfile\u0027"},{"line_number":1297,"context_line":"        with mock.patch(\u0027os.environ.get\u0027) as mock_env_get:"},{"line_number":1298,"context_line":"            mock_env_get.return_value \u003d filename"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_963913da","line":1295,"updated":"2015-01-19 09:06:36.000000000","message":"I believe functional test that would avoid mocking is a lot better option to test the actual script, don\u0027t you think?","commit_id":"f487b65952ff047b66ac04f11d7caba2468ce315"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"95c69739c78eb500f7b0fac6f933f62d775020ad","unresolved":false,"context_lines":[{"line_number":1292,"context_line":"        mock_exists.assert_called_once_with(filename)"},{"line_number":1293,"context_line":"        mock_open.assert_called_once_with(filename)"},{"line_number":1294,"context_line":""},{"line_number":1295,"context_line":"    def test_lease_init(self):"},{"line_number":1296,"context_line":"        filename \u003d \u0027hostfile\u0027"},{"line_number":1297,"context_line":"        with mock.patch(\u0027os.environ.get\u0027) as mock_env_get:"},{"line_number":1298,"context_line":"            mock_env_get.return_value \u003d filename"}],"source_content_type":"text/x-python","patch_set":7,"id":"1a930d6b_728457d4","line":1295,"in_reply_to":"3a961159_0596a43e","updated":"2015-01-22 23:41:58.000000000","message":"I agree with Ihar, said that I believe we should not do python to handle this but build a static bash script per network, look at the other file comments, please.","commit_id":"f487b65952ff047b66ac04f11d7caba2468ce315"},{"author":{"_account_id":11762,"name":"Han Zhou","email":"zhouhan@gmail.com","username":"hanzhou"},"change_message_id":"25a37d9e74a572df84498a2727567be4ca65a5fb","unresolved":false,"context_lines":[{"line_number":1292,"context_line":"        mock_exists.assert_called_once_with(filename)"},{"line_number":1293,"context_line":"        mock_open.assert_called_once_with(filename)"},{"line_number":1294,"context_line":""},{"line_number":1295,"context_line":"    def test_lease_init(self):"},{"line_number":1296,"context_line":"        filename \u003d \u0027hostfile\u0027"},{"line_number":1297,"context_line":"        with mock.patch(\u0027os.environ.get\u0027) as mock_env_get:"},{"line_number":1298,"context_line":"            mock_env_get.return_value \u003d filename"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_b097d49b","line":1295,"in_reply_to":"3a961159_963913da","updated":"2015-01-20 07:48:49.000000000","message":"I think it is sufficient to use mock here if we just want to test the output format. Could you suggest what else is supposed to be covered in functional test?","commit_id":"f487b65952ff047b66ac04f11d7caba2468ce315"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"86580c83e7cc54bcb0c01b1e524a7f7bffc395be","unresolved":false,"context_lines":[{"line_number":1292,"context_line":"        mock_exists.assert_called_once_with(filename)"},{"line_number":1293,"context_line":"        mock_open.assert_called_once_with(filename)"},{"line_number":1294,"context_line":""},{"line_number":1295,"context_line":"    def test_lease_init(self):"},{"line_number":1296,"context_line":"        filename \u003d \u0027hostfile\u0027"},{"line_number":1297,"context_line":"        with mock.patch(\u0027os.environ.get\u0027) as mock_env_get:"},{"line_number":1298,"context_line":"            mock_env_get.return_value \u003d filename"}],"source_content_type":"text/x-python","patch_set":7,"id":"3a961159_0596a43e","line":1295,"in_reply_to":"3a961159_b097d49b","updated":"2015-01-20 10:43:15.000000000","message":"The reasoning is vice versa. We should not mock anything unless completely needed. The test is intended to check that script works, so the best thing is to test it as a whole and not mock out implementation.","commit_id":"f487b65952ff047b66ac04f11d7caba2468ce315"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"8eeacc0c99bdc68fde356860339527c388401321","unresolved":false,"context_lines":[{"line_number":727,"context_line":"            \u0027--interface\u003dtap0\u0027,"},{"line_number":728,"context_line":"            \u0027--except-interface\u003dlo\u0027,"},{"line_number":729,"context_line":"            \u0027--pid-file\u003d%s\u0027 % expected_pid_file,"},{"line_number":730,"context_line":"            \u0027--dhcp-hostsfile\u003d/dhcp/%s/host\u0027 % network.id,"},{"line_number":731,"context_line":"            \u0027--addn-hosts\u003d/dhcp/%s/addn_hosts\u0027 % network.id,"},{"line_number":732,"context_line":"            \u0027--dhcp-optsfile\u003d/dhcp/%s/opts\u0027 % network.id,"},{"line_number":733,"context_line":"            \u0027--dhcp-script\u003d/usr/local/bin/\u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"da86d52c_f7c6c248","line":730,"updated":"2015-01-31 14:58:02.000000000","message":"nit: I guess you should calculate the value once and reuse it here.","commit_id":"25ac6066f630e3a1118f38e396034790da5bd323"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"8eeacc0c99bdc68fde356860339527c388401321","unresolved":false,"context_lines":[{"line_number":732,"context_line":"            \u0027--dhcp-optsfile\u003d/dhcp/%s/opts\u0027 % network.id,"},{"line_number":733,"context_line":"            \u0027--dhcp-script\u003d/usr/local/bin/\u0027"},{"line_number":734,"context_line":"            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027,"},{"line_number":735,"context_line":"            \u0027--dhcp-scriptuser\u003d%s\u0027 % os.environ.get(\u0027USER\u0027),"},{"line_number":736,"context_line":"            \u0027--leasefile-ro\u0027]"},{"line_number":737,"context_line":""},{"line_number":738,"context_line":"        seconds \u003d \u0027\u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"da86d52c_97e746e4","line":735,"updated":"2015-01-31 14:58:02.000000000","message":"use getpass here too","commit_id":"25ac6066f630e3a1118f38e396034790da5bd323"},{"author":{"_account_id":8788,"name":"Miguel Angel Ajo","email":"mangelajo@redhat.com","username":"mangelajo"},"change_message_id":"0887ef480ef45522831a108453025d54165a9efc","unresolved":false,"context_lines":[{"line_number":732,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % hostsfile_name,"},{"line_number":733,"context_line":"            \u0027--addn-hosts\u003d/dhcp/%s/addn_hosts\u0027 % network.id,"},{"line_number":734,"context_line":"            \u0027--dhcp-optsfile\u003d/dhcp/%s/opts\u0027 % network.id,"},{"line_number":735,"context_line":"            \u0027--dhcp-script\u003d/usr/local/bin/\u0027"},{"line_number":736,"context_line":"            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027,"},{"line_number":737,"context_line":"            \u0027--dhcp-scriptuser\u003d%s\u0027 % pwd.getpwuid(os.getuid()).pw_name,"},{"line_number":738,"context_line":"            \u0027--leasefile-ro\u0027]"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_71bba70e","line":735,"updated":"2015-02-02 10:15:20.000000000","message":"not sure about this: \n\n   may we need to recalculate /usr/local/bin dynamically at\ntesting side too, to avoid differences among systems?.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"6e077ac68f72de52e427e06bdb6320dd985c4c6a","unresolved":false,"context_lines":[{"line_number":732,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % hostsfile_name,"},{"line_number":733,"context_line":"            \u0027--addn-hosts\u003d/dhcp/%s/addn_hosts\u0027 % network.id,"},{"line_number":734,"context_line":"            \u0027--dhcp-optsfile\u003d/dhcp/%s/opts\u0027 % network.id,"},{"line_number":735,"context_line":"            \u0027--dhcp-script\u003d/usr/local/bin/\u0027"},{"line_number":736,"context_line":"            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027,"},{"line_number":737,"context_line":"            \u0027--dhcp-scriptuser\u003d%s\u0027 % pwd.getpwuid(os.getuid()).pw_name,"},{"line_number":738,"context_line":"            \u0027--leasefile-ro\u0027]"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_7d1d36bd","line":735,"in_reply_to":"da86d52c_71bba70e","updated":"2015-02-02 12:34:37.000000000","message":"it\u0027s hardcoded in line 708.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"20d594f843d538667719672a1d9e54393b67d5cf","unresolved":false,"context_lines":[{"line_number":732,"context_line":"            \u0027--dhcp-hostsfile\u003d%s\u0027 % hostsfile_name,"},{"line_number":733,"context_line":"            \u0027--addn-hosts\u003d/dhcp/%s/addn_hosts\u0027 % network.id,"},{"line_number":734,"context_line":"            \u0027--dhcp-optsfile\u003d/dhcp/%s/opts\u0027 % network.id,"},{"line_number":735,"context_line":"            \u0027--dhcp-script\u003d/usr/local/bin/\u0027"},{"line_number":736,"context_line":"            \u0027neutron-dhcp-agent-dnsmasq-lease-init\u0027,"},{"line_number":737,"context_line":"            \u0027--dhcp-scriptuser\u003d%s\u0027 % pwd.getpwuid(os.getuid()).pw_name,"},{"line_number":738,"context_line":"            \u0027--leasefile-ro\u0027]"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_525e9346","line":735,"in_reply_to":"da86d52c_7d1d36bd","updated":"2015-02-02 17:59:04.000000000","message":"I think this is fine.","commit_id":"41c941780dcb1e62e119d59934a9d84d280f1339"}],"setup.cfg":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"96b184633dd71b3bda23617c57ce40c4131e785d","unresolved":false,"context_lines":[{"line_number":88,"context_line":"scripts \u003d"},{"line_number":89,"context_line":"    bin/neutron-rootwrap"},{"line_number":90,"context_line":"    bin/neutron-rootwrap-xen-dom0"},{"line_number":91,"context_line":"    bin/neutron-dhcp-agent-dnsmasq-lease-init"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"[global]"},{"line_number":94,"context_line":"setup-hooks \u003d"}],"source_content_type":"text/x-ttcn-cfg","patch_set":5,"id":"3a961159_ebef4d1b","line":91,"updated":"2015-01-05 21:32:15.000000000","message":"I\u0027m not sure this is quite correct, see below","commit_id":"4230a09f0ad556d20a3db09e58113cb9276738f9"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"96b184633dd71b3bda23617c57ce40c4131e785d","unresolved":false,"context_lines":[{"line_number":117,"context_line":"    neutron-openvswitch-agent \u003d neutron.plugins.openvswitch.agent.ovs_neutron_agent:main"},{"line_number":118,"context_line":"    neutron-ovs-cleanup \u003d neutron.agent.ovs_cleanup_util:main"},{"line_number":119,"context_line":"    neutron-restproxy-agent \u003d neutron.plugins.bigswitch.agent.restproxy_agent:main"},{"line_number":120,"context_line":"    neutron-server \u003d neutron.server:main"},{"line_number":121,"context_line":"    neutron-rootwrap \u003d oslo.rootwrap.cmd:main"},{"line_number":122,"context_line":"    neutron-usage-audit \u003d neutron.cmd.usage_audit:main"},{"line_number":123,"context_line":"    neutron-vpn-agent \u003d neutron.services.vpn.agent:main"}],"source_content_type":"text/x-ttcn-cfg","patch_set":5,"id":"3a961159_0b22d9a2","line":120,"updated":"2015-01-05 21:32:15.000000000","message":"Putting this line here:\n\nquantum-dhcp-agent-dnsmasq-lease-init \u003d neutron.agent.linux.dhcp:Dnsmasq.lease_init\n\nIs what I think you want to do, at least that\u0027s what was done with the old \"lease-update\" script.","commit_id":"4230a09f0ad556d20a3db09e58113cb9276738f9"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"9a9ca774e97ff2c982f9d1ca0524e92b227a7929","unresolved":false,"context_lines":[{"line_number":117,"context_line":"    neutron-openvswitch-agent \u003d neutron.plugins.openvswitch.agent.ovs_neutron_agent:main"},{"line_number":118,"context_line":"    neutron-ovs-cleanup \u003d neutron.agent.ovs_cleanup_util:main"},{"line_number":119,"context_line":"    neutron-restproxy-agent \u003d neutron.plugins.bigswitch.agent.restproxy_agent:main"},{"line_number":120,"context_line":"    neutron-server \u003d neutron.server:main"},{"line_number":121,"context_line":"    neutron-rootwrap \u003d oslo.rootwrap.cmd:main"},{"line_number":122,"context_line":"    neutron-usage-audit \u003d neutron.cmd.usage_audit:main"},{"line_number":123,"context_line":"    neutron-vpn-agent \u003d neutron.services.vpn.agent:main"}],"source_content_type":"text/x-ttcn-cfg","patch_set":5,"id":"3a961159_f6a730ba","line":120,"in_reply_to":"3a961159_0b22d9a2","updated":"2015-01-05 21:44:59.000000000","message":"Of course that should start with \"neutron\", I did a blind copy/paste from the git log for the other change","commit_id":"4230a09f0ad556d20a3db09e58113cb9276738f9"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"5aaf03578893d3745fa8bdd539e86eb44c33ace3","unresolved":false,"context_lines":[{"line_number":83,"context_line":"scripts \u003d"},{"line_number":84,"context_line":"    bin/neutron-rootwrap"},{"line_number":85,"context_line":"    bin/neutron-rootwrap-xen-dom0"},{"line_number":86,"context_line":"    bin/neutron-dhcp-agent-dnsmasq-lease-init"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"[global]"},{"line_number":89,"context_line":"setup-hooks \u003d"}],"source_content_type":"text/x-ttcn-cfg","patch_set":8,"id":"fa81d914_d9ac9f8f","line":86,"updated":"2015-01-27 20:42:16.000000000","message":"This will need rebase","commit_id":"b23d58999716a8f347a4d0fb4cfce77ee903ba4b"}]}