)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6598,"name":"Berezovsky Irena","email":"irenab.dev@gmail.com","username":"irenab"},"change_message_id":"6607e15407e22c20b49386a90806a03b94904d5a","unresolved":false,"context_lines":[{"line_number":10,"context_line":"plug/unplug operations to permit integration with networking"},{"line_number":11,"context_line":"implementations without non-virtualization driver specific changes to"},{"line_number":12,"context_line":"Nova."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I929218e2420edf04b8c77c0cbd5d5be1c2ad79dd"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"9a80dd14_fd9aaf60","line":13,"updated":"2015-04-18 20:19:03.000000000","message":"Please link this patch to the launchpad bp","commit_id":"712a643aa2d2a7b810b27b899c9203cd4a0dcd66"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"850945da5c17ecd114a2dccc055aff12471356bc","unresolved":false,"context_lines":[{"line_number":10,"context_line":"plug/unplug operations to permit integration with networking"},{"line_number":11,"context_line":"implementations without non-virtualization driver specific changes to"},{"line_number":12,"context_line":"Nova."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: I929218e2420edf04b8c77c0cbd5d5be1c2ad79dd"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3acd31a7_de40b536","line":13,"updated":"2015-05-04 22:56:15.000000000","message":"please add the blueprint for the spec.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"}],"specs/liberty/approved/vif-plug-script.rst":[{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"055a359e8c6603c3a17c899aa8bf71e76a928d7f","unresolved":false,"context_lines":[{"line_number":59,"context_line":"to the script by populating environment variables."},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"The script is called by nova in the following way:"},{"line_number":62,"context_line":"ENVIRONNEMT_VARIABLES  plug_script.[sh|py] [plug|unplug]"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"+------------------------------+--------------------------------+"},{"line_number":65,"context_line":"| VIF data member              | Environment variable           |"}],"source_content_type":"text/x-rst","patch_set":1,"id":"9a80dd14_5467e4b6","line":62,"updated":"2015-03-08 16:56:28.000000000","message":"is using ENVIRONMENT_VARIABLES the best option?\n \nmaybe we can pass a string which is a json syntax something like \n\u0027{\"vif_id\": \"xxx\",  \"vif_network_bridge\": \"xxx\" ...  }\u0027\nand the plug script will do the following \ndata\u003djson.loads(sys.argv[1])\nto convert it to dict","commit_id":"470ddc9fd4d4600c350da29faa4a1b7962723c9b"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"b6e190600b0dfd85b9e68ff2f1de46cf62b2cf9a","unresolved":false,"context_lines":[{"line_number":59,"context_line":"to the script by populating environment variables."},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"The script is called by nova in the following way:"},{"line_number":62,"context_line":"ENVIRONNEMT_VARIABLES  plug_script.[sh|py] [plug|unplug]"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"+------------------------------+--------------------------------+"},{"line_number":65,"context_line":"| VIF data member              | Environment variable           |"}],"source_content_type":"text/x-rst","patch_set":1,"id":"9a80dd14_df476d75","line":62,"in_reply_to":"9a80dd14_5467e4b6","updated":"2015-03-08 21:47:13.000000000","message":"Yes, we could do that. It does mean making the decision that simple shell scripts are not going to be an option. I think ENVIRONMENT_VARIABLES were chosen because they were the simplest to employ from the script side of things.","commit_id":"470ddc9fd4d4600c350da29faa4a1b7962723c9b"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"055a359e8c6603c3a17c899aa8bf71e76a928d7f","unresolved":false,"context_lines":[{"line_number":60,"context_line":""},{"line_number":61,"context_line":"The script is called by nova in the following way:"},{"line_number":62,"context_line":"ENVIRONNEMT_VARIABLES  plug_script.[sh|py] [plug|unplug]"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"+------------------------------+--------------------------------+"},{"line_number":65,"context_line":"| VIF data member              | Environment variable           |"},{"line_number":66,"context_line":"+\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d+\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d+"}],"source_content_type":"text/x-rst","patch_set":1,"id":"9a80dd14_5450c4d3","line":63,"updated":"2015-03-08 16:56:28.000000000","message":"ENVIRONNEMT  should be ENVIRONMENT","commit_id":"470ddc9fd4d4600c350da29faa4a1b7962723c9b"},{"author":{"_account_id":13734,"name":"Nell Jerram","email":"nell@tigera.io","username":"neiljerram"},"change_message_id":"e58202e634357012ae3a1f2b5399f9b6bf902eb2","unresolved":false,"context_lines":[{"line_number":40,"context_line":"This change provides a mechanism to allow support for new VIF types to be"},{"line_number":41,"context_line":"supported through deployment configuration and not changes to Nova. This"},{"line_number":42,"context_line":"includes out-of-tree third party implementations."},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"Project Priority"},{"line_number":45,"context_line":"-----------------"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"3ab451fb_c93e34b8","line":43,"updated":"2015-04-17 13:03:42.000000000","message":"I absolutely agree that this is the objective.  But unfortunately this spec as it stands doesn\u0027t do enough to make that a reality.\n\nSpecifically - for my project (Calico) and for any others that build on \u003cinterface type\u003d\u0027ethernet\u0027 ...\u003e - we still need to add a VIF type that tells Nova to do a plain \u003cinterface type\u003d\u0027ethernet\u0027 ...\u003e.  _Then_ we can also specify vif_plugin_script so as to say how to plug and connect up that Ethernet interface.\n\nMy spec at https://review.openstack.org/#/c/166831/ provides VIF_TYPE_TAP for that basic Ethernet interface.  With that spec and this one, my project at least will have what it needs to be able to work with vanilla Nova.\n\nBut I guess that there are other cases than \u003cinterface type\u003d\u0027ethernet\u0027 ...\u003e, and I\u0027d like to see an overall plan that caters to those too.  Perhaps this could be a set of basic VIF types that essentially just specify the XXX in \u003cinterface type\u003dXXX ...\u003e, with anything more solution-specific being added by the vif_plugin_script?  Or perhaps we should allow Neutron to directly specify the entire \u003cinterface\u003e XML - maybe in some generic form so as not to be so libvirt-centric?\n\nWhat do you think?","commit_id":"712a643aa2d2a7b810b27b899c9203cd4a0dcd66"},{"author":{"_account_id":6598,"name":"Berezovsky Irena","email":"irenab.dev@gmail.com","username":"irenab"},"change_message_id":"6607e15407e22c20b49386a90806a03b94904d5a","unresolved":false,"context_lines":[{"line_number":40,"context_line":"This change provides a mechanism to allow support for new VIF types to be"},{"line_number":41,"context_line":"supported through deployment configuration and not changes to Nova. This"},{"line_number":42,"context_line":"includes out-of-tree third party implementations."},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"Project Priority"},{"line_number":45,"context_line":"-----------------"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-rst","patch_set":2,"id":"dac4157f_7f96c795","line":43,"in_reply_to":"3ab451fb_c93e34b8","updated":"2015-04-18 20:19:03.000000000","message":"Agree. At least for basic Ethernet interface support, mentioned vif_type_tap and this patch proposal looks complimentary.","commit_id":"712a643aa2d2a7b810b27b899c9203cd4a0dcd66"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"7236e9f2441d91540c7563509a490b71571e96b2","unresolved":false,"context_lines":[{"line_number":78,"context_line":"+------------------------------+--------------------------------+"},{"line_number":79,"context_line":"| project id/tenant            | VIF_PROJECT_ID                 |"},{"line_number":80,"context_line":"+------------------------------+--------------------------------+"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":""},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":2,"id":"9a80dd14_4babb088","line":81,"updated":"2015-03-09 13:26:52.000000000","message":"As mentioned in my review of the code, we have a number of VIF types and each VIF type has a set of attributes that are expected by Nova. These are partially overlapping sets.  I think the script should only be given attributes that are explicitly related to the VIF type of the NIC in question.\n\nThe problem is that we don\u0027t have a good record of which parameters are associated with which VIF type in Nova right now. The nova.network.model.VIF class just accepts the union of all possible parameters. So I think we probably want to clean that up by introducing subclasses for each VIF type, and probably also making it inherit from the Nova Object base class. Then you can do a purely metadata driven approach to exposing env variables where we just directly map the object attributes to env variables.","commit_id":"712a643aa2d2a7b810b27b899c9203cd4a0dcd66"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"3a9af65cad39e6eb8fe3789215c79a6db31302f5","unresolved":false,"context_lines":[{"line_number":78,"context_line":"+------------------------------+--------------------------------+"},{"line_number":79,"context_line":"| project id/tenant            | VIF_PROJECT_ID                 |"},{"line_number":80,"context_line":"+------------------------------+--------------------------------+"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":""},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":2,"id":"9a80dd14_f2453143","line":81,"in_reply_to":"9a80dd14_4babb088","updated":"2015-03-09 15:14:31.000000000","message":"Interesting idea. The set-union nature of nova.network.model.VIF has been a pet-peeve of mine for some time, but \"not liking it\" didn\u0027t seem  a sufficient reason to change it. This seems as good time a time as any to look at what that kind of change would entail.","commit_id":"712a643aa2d2a7b810b27b899c9203cd4a0dcd66"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"5d2be6a03e7c790b4bf42ffe3d763a7bddfd0b8f","unresolved":false,"context_lines":[{"line_number":78,"context_line":"+------------------------------+--------------------------------+"},{"line_number":79,"context_line":"| project id/tenant            | VIF_PROJECT_ID                 |"},{"line_number":80,"context_line":"+------------------------------+--------------------------------+"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":""},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":2,"id":"9a80dd14_0d646848","line":81,"in_reply_to":"9a80dd14_f2453143","updated":"2015-03-09 15:50:20.000000000","message":"As mentioned in my review of the code: How do you know which VIF Types attributes is relevant for external ml2 plugin using vhost-user or tap vif type for example ?\n\nI think the VIF_OVS_INTERFACEID attribute is not a real problem. Actually, in nova/network/neutronv2/api.py, the \u0027ovs_interfaceid\u0027 is set to port[\u0027id\u0027]. Instead of providing VIF_OVS_INTERFACEID, we should provide VIF_PORT_ID information to the plug script. VIF_PORT_ID is enough generic term to be useful for any ml2 bridge.\n\nAny VIF type specific attribute should be provided in VIF_DETAILS.\n\nWhat do you think ?","commit_id":"712a643aa2d2a7b810b27b899c9203cd4a0dcd66"},{"author":{"_account_id":11647,"name":"Przemyslaw Czesnowicz","email":"przemyslaw.czesnowicz@intel.com","username":"pczesno"},"change_message_id":"bafed762848e4940c212c0400fbf0e54761a6acb","unresolved":false,"context_lines":[{"line_number":5,"context_line":" http://creativecommons.org/licenses/by/3.0/legalcode"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":8,"context_line":"VIF plug script support for Nova"},{"line_number":9,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_cb18e777","line":8,"updated":"2015-05-05 12:21:32.000000000","message":"Is this spec libvirt driver specific or are you targeting all the drivers with this?","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"f5ea2bef6c5751293a6adc0f6ea134c0f64bf9a5","unresolved":false,"context_lines":[{"line_number":5,"context_line":" http://creativecommons.org/licenses/by/3.0/legalcode"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":8,"context_line":"VIF plug script support for Nova"},{"line_number":9,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":10,"context_line":""},{"line_number":11,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"1aca2d91_a7476076","line":8,"in_reply_to":"3acd31a7_cb18e777","updated":"2015-05-07 18:41:17.000000000","message":"While I\u0027ve added support to libvirt for this, there is nothing intentionally libvirt specific about it. Hopefully other drivers will find it useful.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"832ccc0f266675f0fd17cf66c30a99a9359053e3","unresolved":false,"context_lines":[{"line_number":40,"context_line":"By providing a mechanism for execution of external operations upon VIF \"plug\""},{"line_number":41,"context_line":"and \"unplug\" operations, this change enables supporting network implementation"},{"line_number":42,"context_line":"and configurations that are not directly supported through the hypervisor\u0027s VIF"},{"line_number":43,"context_line":"types."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"Project Priority"},{"line_number":46,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"bab921fb_096f202c","line":43,"updated":"2015-04-25 10:59:38.000000000","message":"I think we should explain here that vif_plugin_script mechanism can be used with basic VIF_TYPE like\n- VIF_TYPE_TAP (https://review.openstack.org/#/c/166831/)\n- VIT_TYPE_VHOSTUSER (https://review.openstack.org/#/c/138736/)\n\nBy the way, for vhostuser, the vif_plug_script could remove the needs of \u0027vhost_user_ovs_plug\u0027 in the vif details.\n\nCould you please update this section in consequence ?","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"f5ea2bef6c5751293a6adc0f6ea134c0f64bf9a5","unresolved":false,"context_lines":[{"line_number":40,"context_line":"By providing a mechanism for execution of external operations upon VIF \"plug\""},{"line_number":41,"context_line":"and \"unplug\" operations, this change enables supporting network implementation"},{"line_number":42,"context_line":"and configurations that are not directly supported through the hypervisor\u0027s VIF"},{"line_number":43,"context_line":"types."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"Project Priority"},{"line_number":46,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"1aca2d91_e71b187f","line":43,"in_reply_to":"3acd31a7_008dd27a","updated":"2015-05-07 18:41:17.000000000","message":"Not to my knowledge.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"f5ea2bef6c5751293a6adc0f6ea134c0f64bf9a5","unresolved":false,"context_lines":[{"line_number":40,"context_line":"By providing a mechanism for execution of external operations upon VIF \"plug\""},{"line_number":41,"context_line":"and \"unplug\" operations, this change enables supporting network implementation"},{"line_number":42,"context_line":"and configurations that are not directly supported through the hypervisor\u0027s VIF"},{"line_number":43,"context_line":"types."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"Project Priority"},{"line_number":46,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"1aca2d91_875fc440","line":43,"in_reply_to":"bab921fb_096f202c","updated":"2015-05-07 18:41:17.000000000","message":"Done","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1e9cd314006ef37d80e1fdc2fa1c8e9e0d0dba42","unresolved":false,"context_lines":[{"line_number":40,"context_line":"By providing a mechanism for execution of external operations upon VIF \"plug\""},{"line_number":41,"context_line":"and \"unplug\" operations, this change enables supporting network implementation"},{"line_number":42,"context_line":"and configurations that are not directly supported through the hypervisor\u0027s VIF"},{"line_number":43,"context_line":"types."},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"Project Priority"},{"line_number":46,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_008dd27a","line":43,"in_reply_to":"bab921fb_096f202c","updated":"2015-05-02 23:29:49.000000000","message":"when this was initially discussed on the mailing list we pushed back against implementing the \u0027vhost_user_ovs_plug\u0027 mechanism as  vif_plug_script\nbecause the interface was undefined and we believed this introduces a potential security hole into nova if the script is not confined by the root wrapper.\n\ni would love to see a standard VIF_TYPE_TAP and a extensible plug mechanism added to nova provide the mechanism is well defined.\n\nis this topic proposed for any of the design summit sessions?","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1e9cd314006ef37d80e1fdc2fa1c8e9e0d0dba42","unresolved":false,"context_lines":[{"line_number":50,"context_line":"Proposed change"},{"line_number":51,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"This change introduces a new VIF field, VIF_PLUGIN_SCRIPT. This field will be"},{"line_number":54,"context_line":"populated by Neutron and refer to a back-end specific script that will be"},{"line_number":55,"context_line":"called from the virtualization driver when the system is ready to connect the"},{"line_number":56,"context_line":"guest to a L2 network."}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_005272dd","line":53,"updated":"2015-05-02 23:29:49.000000000","message":"what is the format of the field?\na sting contianing the script?\na url? file system url? http?\n\nthe vif binding details are populated by the mechanism driver running on the network node or controller depending on your configuration. in a mixed host os environment different ways of packaging the backed that uses this mechanism could result in the scripts been in different locations.\n\nif scripts are restricted to python only and the  VIF_PLUGIN_SCRIPT vif_binding field is a fully qualified module name this solve the issue of the neutron mechanism driver not knowing where the plugin script is located on each nova compute node.\n\nthis would also allow importlib to be used to invoke plug and unplug functions directly see comment on line 81.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"12f3278e13549c8e0d45371c23529ef1398569b8","unresolved":false,"context_lines":[{"line_number":50,"context_line":"Proposed change"},{"line_number":51,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"This change introduces a new VIF field, VIF_PLUGIN_SCRIPT. This field will be"},{"line_number":54,"context_line":"populated by Neutron and refer to a back-end specific script that will be"},{"line_number":55,"context_line":"called from the virtualization driver when the system is ready to connect the"},{"line_number":56,"context_line":"guest to a L2 network."}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_e6c8ae24","line":53,"in_reply_to":"3acd31a7_005272dd","updated":"2015-05-03 21:22:46.000000000","message":"You should specify here the name of your script. The script can be shell scripts, or python scripts or whatever you wants.\n\nYou script should be installed in one of the directory of $PATH environment variable. Thus it can be installed in different place on the different hosts, there are no issue here.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"f5ea2bef6c5751293a6adc0f6ea134c0f64bf9a5","unresolved":false,"context_lines":[{"line_number":50,"context_line":"Proposed change"},{"line_number":51,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"This change introduces a new VIF field, VIF_PLUGIN_SCRIPT. This field will be"},{"line_number":54,"context_line":"populated by Neutron and refer to a back-end specific script that will be"},{"line_number":55,"context_line":"called from the virtualization driver when the system is ready to connect the"},{"line_number":56,"context_line":"guest to a L2 network."}],"source_content_type":"text/x-rst","patch_set":3,"id":"1aca2d91_82b0d244","line":53,"in_reply_to":"3acd31a7_139ce65b","updated":"2015-05-07 18:41:17.000000000","message":"I\u0027m not convinced that PATH should be used. If it is up to neutron deployment to place the script where it can be called, shouldn\u0027t it decide the path problem?","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"850945da5c17ecd114a2dccc055aff12471356bc","unresolved":false,"context_lines":[{"line_number":50,"context_line":"Proposed change"},{"line_number":51,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"This change introduces a new VIF field, VIF_PLUGIN_SCRIPT. This field will be"},{"line_number":54,"context_line":"populated by Neutron and refer to a back-end specific script that will be"},{"line_number":55,"context_line":"called from the virtualization driver when the system is ready to connect the"},{"line_number":56,"context_line":"guest to a L2 network."}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_139ce65b","line":53,"in_reply_to":"3acd31a7_e6c8ae24","updated":"2015-05-04 22:56:15.000000000","message":"perhaps it would be better to introduce a nova config variable for where the scripts would be stored instead of searching all directories on the $path. e.g VIF_PLUG_DIR\n\ni still think there are other issues with using a script name of path. for example adding a new ml2 mechanism driver that supports a particular in tree VIF_TYPE requires no changes nova or neutron following the vendor decomposition spec.\n\nthere would be nothing preventing two mech drivers providing a plug script with the same. for example a compute node may have vm using both ovs ports and sriov ports. and the sriov mech driver could have the same plug_script name as the ovs mech driver.\n\nif we rely on the filesystem namespacing  we would have to require the VIF_PLUGIN_SCRIPT to contain a value or \"\u003cmech driver name\u003e/\u003cplug scrip tname\u003e\"\nso that each mech driver/plug-in would have its own directory.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1e9cd314006ef37d80e1fdc2fa1c8e9e0d0dba42","unresolved":false,"context_lines":[{"line_number":53,"context_line":"This change introduces a new VIF field, VIF_PLUGIN_SCRIPT. This field will be"},{"line_number":54,"context_line":"populated by Neutron and refer to a back-end specific script that will be"},{"line_number":55,"context_line":"called from the virtualization driver when the system is ready to connect the"},{"line_number":56,"context_line":"guest to a L2 network."},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"Nova shall be modified to call the script and interpret success or failure"},{"line_number":59,"context_line":"based on standard error level codes. Nova shall provide additional information"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_a0cbbea4","line":56,"updated":"2015-05-02 23:29:49.000000000","message":"does this spec have a neutron counterpart.\nas this change is stadeling the interface between both project it would be good to have input form the neutron core team also.\n\nwould the plugin script resided in nova, neuron or external repo.\nthe latter is probably most appropriate for flexibility.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"f5ea2bef6c5751293a6adc0f6ea134c0f64bf9a5","unresolved":false,"context_lines":[{"line_number":53,"context_line":"This change introduces a new VIF field, VIF_PLUGIN_SCRIPT. This field will be"},{"line_number":54,"context_line":"populated by Neutron and refer to a back-end specific script that will be"},{"line_number":55,"context_line":"called from the virtualization driver when the system is ready to connect the"},{"line_number":56,"context_line":"guest to a L2 network."},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"Nova shall be modified to call the script and interpret success or failure"},{"line_number":59,"context_line":"based on standard error level codes. Nova shall provide additional information"}],"source_content_type":"text/x-rst","patch_set":3,"id":"1aca2d91_e7d3185e","line":56,"in_reply_to":"3acd31a7_0a3eefd9","updated":"2015-05-07 18:41:17.000000000","message":"That patch should probably be revised to by a simple DO NOT MERGE without a specific blueprint. I will do so shortly. It is up to whomever is implementing the neutron side in their mechanism driver or plugin to decide how it should be configured, derived implemented, etc.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"850945da5c17ecd114a2dccc055aff12471356bc","unresolved":false,"context_lines":[{"line_number":53,"context_line":"This change introduces a new VIF field, VIF_PLUGIN_SCRIPT. This field will be"},{"line_number":54,"context_line":"populated by Neutron and refer to a back-end specific script that will be"},{"line_number":55,"context_line":"called from the virtualization driver when the system is ready to connect the"},{"line_number":56,"context_line":"guest to a L2 network."},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"Nova shall be modified to call the script and interpret success or failure"},{"line_number":59,"context_line":"based on standard error level codes. Nova shall provide additional information"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_0a3eefd9","line":56,"in_reply_to":"3acd31a7_66ddbedf","updated":"2015-05-04 22:56:15.000000000","message":"the neutron review below apears to be related to this spec\nas the gerrit topic refered to the blueprint for this feature.\n\nhttps://review.openstack.org/#/c/162471/2\n\nif this is related you will need a neutron counterpart for the neuton changes.\n\nthe vhostuser changes did have coresponding neuton bluprints \nhttps://blueprints.launchpad.net/neutron/+spec/ml2-dpdk-ovs-mechanism-driver\nhttps://blueprints.launchpad.net/neutron/+spec/ml2-use-dpdkvhost\n\ndue to the Neutron core and vendor code decomposition\nspec see https://review.openstack.org/#/c/134680/\nit was not required to modify neutron to implement the mechanism driver.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"12f3278e13549c8e0d45371c23529ef1398569b8","unresolved":false,"context_lines":[{"line_number":53,"context_line":"This change introduces a new VIF field, VIF_PLUGIN_SCRIPT. This field will be"},{"line_number":54,"context_line":"populated by Neutron and refer to a back-end specific script that will be"},{"line_number":55,"context_line":"called from the virtualization driver when the system is ready to connect the"},{"line_number":56,"context_line":"guest to a L2 network."},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"Nova shall be modified to call the script and interpret success or failure"},{"line_number":59,"context_line":"based on standard error level codes. Nova shall provide additional information"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_66ddbedf","line":56,"in_reply_to":"3acd31a7_a0cbbea4","updated":"2015-05-03 21:22:46.000000000","message":"There is no neutron counterpart like the vhostuser specs.\n\nThe plugin script should be stored in your ml2 neutron agent/mechanism driver repo.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":11647,"name":"Przemyslaw Czesnowicz","email":"przemyslaw.czesnowicz@intel.com","username":"pczesno"},"change_message_id":"bafed762848e4940c212c0400fbf0e54761a6acb","unresolved":false,"context_lines":[{"line_number":63,"context_line":"ENVIRONMEMT_VARIABLES  plug_script.[sh|py] [plug|unplug]"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"The ENVIRONMENT_VARIABLES are generated from the data members of the relevant"},{"line_number":66,"context_line":"VIF object and follow the general format:"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"\"VIF_%s_%s\" % (attribute_name.upper(), str(attribute.value))"},{"line_number":69,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_9051fa38","line":66,"updated":"2015-05-05 12:21:32.000000000","message":"Currently both vif and instance objects are passed to plug/unplug methods.\nWill the instance object be passed same way as the vif object?","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"f5ea2bef6c5751293a6adc0f6ea134c0f64bf9a5","unresolved":false,"context_lines":[{"line_number":63,"context_line":"ENVIRONMEMT_VARIABLES  plug_script.[sh|py] [plug|unplug]"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"The ENVIRONMENT_VARIABLES are generated from the data members of the relevant"},{"line_number":66,"context_line":"VIF object and follow the general format:"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"\"VIF_%s_%s\" % (attribute_name.upper(), str(attribute.value))"},{"line_number":69,"context_line":""}],"source_content_type":"text/x-rst","patch_set":3,"id":"1aca2d91_47564cd3","line":66,"in_reply_to":"3acd31a7_9051fa38","updated":"2015-05-07 18:41:17.000000000","message":"We do not intend to do so. Do you feel there is a requirement for that?","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1e9cd314006ef37d80e1fdc2fa1c8e9e0d0dba42","unresolved":false,"context_lines":[{"line_number":78,"context_line":"VIF_DETAILS_OVS_HYBRID_PLUG\u003d\u0027True\u0027"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"This formatting allows for arbitrary nesting with each level following the same"},{"line_number":81,"context_line":"general pattern."},{"line_number":82,"context_line":""},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_e02d3647","line":81,"updated":"2015-05-02 23:29:49.000000000","message":"if we restricted VIF_PLUGIN_SCRIPT to python only we could do the following\nwe could modify nova to import the module and invoke plug and unplug functions directly.\nimport importlib\n... \nplugin\u003dimportlib.import_module(VIF_PLUGIN_SCRIPT)\n\nwhere VIF_PLUGIN_SCRIPT is something like \"my_backend.nova.plug_script\"\ne.g. networking_ovs_dpdk.driver.plug_handler\n\nthis would allow the plug and unplug functions to be invoked as follows\n\nplugin.plug(vif) \nplugin.unplug(vif)\n\nthis will elimination the need for arbitrary serialisation and deserialisation of object/dictionaries to environmental variables.\n\nfor instance why  use teh serialisation method above verses invoking the script with the final argument being a dictonary serialsed to a json string?\n\nimporting the functions and directly invoking them would seam much cleaner and easier to follow.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"f5ea2bef6c5751293a6adc0f6ea134c0f64bf9a5","unresolved":false,"context_lines":[{"line_number":78,"context_line":"VIF_DETAILS_OVS_HYBRID_PLUG\u003d\u0027True\u0027"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"This formatting allows for arbitrary nesting with each level following the same"},{"line_number":81,"context_line":"general pattern."},{"line_number":82,"context_line":""},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":3,"id":"1aca2d91_c2f36a09","line":81,"in_reply_to":"3acd31a7_9867f5b6","updated":"2015-05-07 18:41:17.000000000","message":"I can\u0027t speak for Daniel, but it was my understanding that this mechanism was in direct response to the objections in that dialogue that, as far as I can tell, were not successfully refuted.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"850945da5c17ecd114a2dccc055aff12471356bc","unresolved":false,"context_lines":[{"line_number":78,"context_line":"VIF_DETAILS_OVS_HYBRID_PLUG\u003d\u0027True\u0027"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"This formatting allows for arbitrary nesting with each level following the same"},{"line_number":81,"context_line":"general pattern."},{"line_number":82,"context_line":""},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_9867f5b6","line":81,"in_reply_to":"3acd31a7_a66e36ff","updated":"2015-05-04 22:56:15.000000000","message":"i do not see how this proposal escapes nova  maintain a stable Python API for VIF pluging.\nthis proposal is still introducing a stable api by specifying the format of how nova will populate environmental variables. \n\nthe key differences with my suggestion above is that it is limited to just the plug and unplug functions. \nit does not allow for arbitrary external VIF_TYPES but does address the stated use case of this proposal \n\n\"providing a mechanism for execution of external operations upon VIF \"plug\"\tand \"unplug\" operations\"\n\ni believe the main objection to your original proposal was that it violated novas out of tree policy\nhttp://docs.openstack.org/developer/nova/devref/policies.html#out-of-tree-support\nand that it allowed new external vif types to be added without nova review which would have api/data model implications.\n\nDaniel Berrange said this better then i can on line 69 \nhttps://review.openstack.org/#/c/136827/2/specs/kilo/approved/libvirt-vif-driver-plugin.rst\n\n\"This is not a problem we need / want to optimize away. Review of VIF drivers is a critically important part of their development. New VIF types almost always imply new data passed between Neutron \u0026 Nova REST APIs in the VIF binding metadata. Anything that involves data in the public API must be formally reviewed because it is has significant implications for long term support \u0026 upgrades.\"\n\ni don\u0027t want to take Daniel Berrange\u0027s words out of context but if i have please correct me.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"12f3278e13549c8e0d45371c23529ef1398569b8","unresolved":false,"context_lines":[{"line_number":78,"context_line":"VIF_DETAILS_OVS_HYBRID_PLUG\u003d\u0027True\u0027"},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"This formatting allows for arbitrary nesting with each level following the same"},{"line_number":81,"context_line":"general pattern."},{"line_number":82,"context_line":""},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_a66e36ff","line":81,"in_reply_to":"3acd31a7_e02d3647","updated":"2015-05-03 21:22:46.000000000","message":"Your proposal is similar to my initial blueprint:\nhttps://blueprints.launchpad.net/nova/+spec/libvirt-vif-driver-plugin.\n\nBut the nova core community is against to maintain a stable Python API for VIF plugging.\n\nThat\u0027s why a consensus came up on the mailing list:\nhttp://lists.openstack.org/pipermail/openstack-dev/2014-December/052744.html, about a VIF plug script mechanism.\n\nI don\u0027t see the point to reopen this discussion.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1e9cd314006ef37d80e1fdc2fa1c8e9e0d0dba42","unresolved":false,"context_lines":[{"line_number":97,"context_line":"implement VIF plugging. This is inappropriately \"heavy\" for what amounts to a"},{"line_number":98,"context_line":"few calls with some variables. It introduces a class of errors that need to be"},{"line_number":99,"context_line":"handled such as import errors for the implementation modules, unknown"},{"line_number":100,"context_line":"exceptions percolating up to Nova, etc."},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"As the ultimate result in both approaches is typically calling an external"},{"line_number":103,"context_line":"command, the script approach is more direct and sensible."}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_c086ba64","line":100,"updated":"2015-05-02 23:29:49.000000000","message":"If the plugging scripts amounts to only a few calls then the api would be small.\nhaving a lightweight api (a single abstact class with a vif_plug and vif_unplug methods ) would help standardise the plug-in scripts.\n\ni think this is a much cleaner option then the one proposed.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"f5ea2bef6c5751293a6adc0f6ea134c0f64bf9a5","unresolved":false,"context_lines":[{"line_number":97,"context_line":"implement VIF plugging. This is inappropriately \"heavy\" for what amounts to a"},{"line_number":98,"context_line":"few calls with some variables. It introduces a class of errors that need to be"},{"line_number":99,"context_line":"handled such as import errors for the implementation modules, unknown"},{"line_number":100,"context_line":"exceptions percolating up to Nova, etc."},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"As the ultimate result in both approaches is typically calling an external"},{"line_number":103,"context_line":"command, the script approach is more direct and sensible."}],"source_content_type":"text/x-rst","patch_set":3,"id":"1aca2d91_42563ad3","line":100,"in_reply_to":"3acd31a7_c086ba64","updated":"2015-05-07 18:41:17.000000000","message":"I think this has already been discussed and discarded, hasn\u0027t it?","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1e9cd314006ef37d80e1fdc2fa1c8e9e0d0dba42","unresolved":false,"context_lines":[{"line_number":108,"context_line":"There is no data model impact. The VIF_PLUGIN_SCRIPT is included in the"},{"line_number":109,"context_line":"\u0027details\u0027 field of nova.network.model.VIF and is stored as part of the BLOB"},{"line_number":110,"context_line":"string. Migration does not present a problem as only new VIF types and"},{"line_number":111,"context_line":"instances will be supported in this iteration."},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"REST API impact"},{"line_number":114,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_001712ee","line":111,"updated":"2015-05-02 23:29:49.000000000","message":"are you suggesting storing the script inline?\n\nthis would greatly increase the size of any message sent over the amqp bus that contained the vif binding details.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"12f3278e13549c8e0d45371c23529ef1398569b8","unresolved":false,"context_lines":[{"line_number":108,"context_line":"There is no data model impact. The VIF_PLUGIN_SCRIPT is included in the"},{"line_number":109,"context_line":"\u0027details\u0027 field of nova.network.model.VIF and is stored as part of the BLOB"},{"line_number":110,"context_line":"string. Migration does not present a problem as only new VIF types and"},{"line_number":111,"context_line":"instances will be supported in this iteration."},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"REST API impact"},{"line_number":114,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_26fea637","line":111,"in_reply_to":"3acd31a7_001712ee","updated":"2015-05-03 21:22:46.000000000","message":"No the script will be not stored inline. The script needs to be installed on each compute node.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1e9cd314006ef37d80e1fdc2fa1c8e9e0d0dba42","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"The Neutron client and Neutron may allow administrators to specify the script"},{"line_number":120,"context_line":"as part of the port creation, which would require changes within Neutron."},{"line_number":121,"context_line":"However there is no API impact that would affect Nova."},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"Security impact"},{"line_number":124,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_80e1e200","line":121,"updated":"2015-05-02 23:29:49.000000000","message":"allowing arbitrary  administrators to replace the script on port create sounds like a manageability nightmare. what is the use case for allowing this?","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"f5ea2bef6c5751293a6adc0f6ea134c0f64bf9a5","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"The Neutron client and Neutron may allow administrators to specify the script"},{"line_number":120,"context_line":"as part of the port creation, which would require changes within Neutron."},{"line_number":121,"context_line":"However there is no API impact that would affect Nova."},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"Security impact"},{"line_number":124,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"1aca2d91_87cfc47e","line":121,"in_reply_to":"3acd31a7_80e1e200","updated":"2015-05-07 18:41:17.000000000","message":"We don\u0027t have a use case and I do not disagree. The point of mentioning it here is to make it clear that it is particular to a port - not anything else, i.e. no changes to the nova or neutron APIs. Perhaps it would be better to simply remove the paragraph.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"832ccc0f266675f0fd17cf66c30a99a9359053e3","unresolved":false,"context_lines":[{"line_number":125,"context_line":""},{"line_number":126,"context_line":"There is a potential security impact as the script specified in"},{"line_number":127,"context_line":"VIF_PLUGIN_SCRIPT must be run with elevated privileges (e.g. through rootwrap)"},{"line_number":128,"context_line":"and consequently could potentially execute arbitrary commands. As always,"},{"line_number":129,"context_line":"restricted access to networking and hypervisor hosts must be ensured and, if"},{"line_number":130,"context_line":"possible, the constellation of privileges provided to the VIF_PLUGIN_SCRIPT be"},{"line_number":131,"context_line":"restricted to only those needed for proper operation."}],"source_content_type":"text/x-rst","patch_set":3,"id":"bab921fb_0943a043","line":128,"updated":"2015-04-25 10:59:38.000000000","message":"As discussed here: https://review.openstack.org/#/c/162470/2/nova/virt/netutils.py, line 101, there are no needs to execute the script with rootwrap.\n\nThe script can still call commands with \u0027sudo nova-rootwrap /etc/nova/rootwrap.conf\u0027 to get root privilege.\n\nCould you please update this section in consequence ?","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"12f3278e13549c8e0d45371c23529ef1398569b8","unresolved":false,"context_lines":[{"line_number":125,"context_line":""},{"line_number":126,"context_line":"There is a potential security impact as the script specified in"},{"line_number":127,"context_line":"VIF_PLUGIN_SCRIPT must be run with elevated privileges (e.g. through rootwrap)"},{"line_number":128,"context_line":"and consequently could potentially execute arbitrary commands. As always,"},{"line_number":129,"context_line":"restricted access to networking and hypervisor hosts must be ensured and, if"},{"line_number":130,"context_line":"possible, the constellation of privileges provided to the VIF_PLUGIN_SCRIPT be"},{"line_number":131,"context_line":"restricted to only those needed for proper operation."}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_c61c9279","line":128,"in_reply_to":"3acd31a7_6097e648","updated":"2015-05-03 21:22:46.000000000","message":"1. The script will not be running with root privileges or with nova root wrapper. The specs needs to be updated to reflect that.\n\n2. I thinks there is no needs to let the administrators to run arbitrary scripts for port creation.  We should remove the mention line 119 about this point.\n\nWith these two points, there is no security issue.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"850945da5c17ecd114a2dccc055aff12471356bc","unresolved":false,"context_lines":[{"line_number":125,"context_line":""},{"line_number":126,"context_line":"There is a potential security impact as the script specified in"},{"line_number":127,"context_line":"VIF_PLUGIN_SCRIPT must be run with elevated privileges (e.g. through rootwrap)"},{"line_number":128,"context_line":"and consequently could potentially execute arbitrary commands. As always,"},{"line_number":129,"context_line":"restricted access to networking and hypervisor hosts must be ensured and, if"},{"line_number":130,"context_line":"possible, the constellation of privileges provided to the VIF_PLUGIN_SCRIPT be"},{"line_number":131,"context_line":"restricted to only those needed for proper operation."}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_fe3c91c9","line":128,"in_reply_to":"3acd31a7_c61c9279","updated":"2015-05-04 22:56:15.000000000","message":"some scripts will need root privileges to run. \nfor example to call ovs-vsctl you most have root privileges as the ovs socket used to communicate with the ovs db is owned\nby root. your example  https://github.com/6WIND/nova-vif-plug-example/ does show how a root wrapper could be provided or how novas could be used so that should not be an issue provided the user the script executes as has pass wordless sudo rights.\n\nremoving line 119 will close the security hole though as the script must be installed by the sys admin/ deplorer  and they should validate the software they are deploying into there cloud.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1e9cd314006ef37d80e1fdc2fa1c8e9e0d0dba42","unresolved":false,"context_lines":[{"line_number":125,"context_line":""},{"line_number":126,"context_line":"There is a potential security impact as the script specified in"},{"line_number":127,"context_line":"VIF_PLUGIN_SCRIPT must be run with elevated privileges (e.g. through rootwrap)"},{"line_number":128,"context_line":"and consequently could potentially execute arbitrary commands. As always,"},{"line_number":129,"context_line":"restricted access to networking and hypervisor hosts must be ensured and, if"},{"line_number":130,"context_line":"possible, the constellation of privileges provided to the VIF_PLUGIN_SCRIPT be"},{"line_number":131,"context_line":"restricted to only those needed for proper operation."}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_6097e648","line":128,"in_reply_to":"bab921fb_0943a043","updated":"2015-05-02 23:29:49.000000000","message":"while a script could use their own rootwaper there is also nothing in this proposal that requires it to so this is still a security hole.\n\nif you run the script without the nova root wrapper and combine that with the ability to specify arbitrary scripts at port create then this seams like throwing the baby out with the bathwater. \n\ngranted if an attacker obtains credential with cloud admin rights you have bigger issues but i would like to think even with those rights to the cloud it should take more then:\n    1 query list of hypervisor hosts \n    2 create neuton port with attack script with root access\n    3 boot vm with that port\n    4 migrate vm to each hypervisor.\nto get root privilege to all the underlying compute nodes.\n\nthat might be a little mellow dramatic but am i wrong?","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1e9cd314006ef37d80e1fdc2fa1c8e9e0d0dba42","unresolved":false,"context_lines":[{"line_number":147,"context_line":"Performance Impact"},{"line_number":148,"context_line":"------------------"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"None."},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"Other deployer impact"},{"line_number":153,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_0073924e","line":150,"updated":"2015-05-02 23:29:49.000000000","message":"none?\n\nif importlib is not used a new shell and potentially python interpreter instance will be created with every invocation of plug or unplug while the overhead should be small there will likely be some increase in the port binding time.\n\nif the script is embedded in the vif_bindings directly this will increase packet size of all messages including the vif_bindings dictionary.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"12f3278e13549c8e0d45371c23529ef1398569b8","unresolved":false,"context_lines":[{"line_number":147,"context_line":"Performance Impact"},{"line_number":148,"context_line":"------------------"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"None."},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"Other deployer impact"},{"line_number":153,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_a613d665","line":150,"in_reply_to":"3acd31a7_0073924e","updated":"2015-05-03 21:22:46.000000000","message":"Yes, none or insignificant if you prefer.\n\nImportlib or not, you need to call shell commands to plug your interface like ovs-vsctl in a shell.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"850945da5c17ecd114a2dccc055aff12471356bc","unresolved":false,"context_lines":[{"line_number":147,"context_line":"Performance Impact"},{"line_number":148,"context_line":"------------------"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"None."},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"Other deployer impact"},{"line_number":153,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_b81e31b9","line":150,"in_reply_to":"3acd31a7_a613d665","updated":"2015-05-04 22:56:15.000000000","message":"well for ovs you could use ovsdb directly as odl and ovs_lib now do i just wanted to highlight that there might be a small performance hit vs the current implementation.\n\nfor example you would now need to serialise the vif object to environmental variable and then parse them in the script  addition to call ovs-vsctl at a minimum which would be slower.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"f5ea2bef6c5751293a6adc0f6ea134c0f64bf9a5","unresolved":false,"context_lines":[{"line_number":147,"context_line":"Performance Impact"},{"line_number":148,"context_line":"------------------"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"None."},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"Other deployer impact"},{"line_number":153,"context_line":"---------------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"1aca2d91_07ef3488","line":150,"in_reply_to":"3acd31a7_b81e31b9","updated":"2015-05-07 18:41:17.000000000","message":"In comparison we often have to make multiple calls to external commands to \"plug\" a VIF. I\u0027ve reworded to be include neglible.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"850945da5c17ecd114a2dccc055aff12471356bc","unresolved":false,"context_lines":[{"line_number":154,"context_line":""},{"line_number":155,"context_line":"Deployment of Nova is not impacted. Deployment of Neutron however will need to"},{"line_number":156,"context_line":"install plugin scripts into an appropriate accessible location so it can be"},{"line_number":157,"context_line":"called by Nova."},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"Developer impact"},{"line_number":160,"context_line":"----------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3acd31a7_982a558f","line":157,"updated":"2015-05-04 22:56:15.000000000","message":"this impacts the deployment of neutron plugins or ml2 mech drivers more so then neutron itself .\n\nas these are moving out of the neutron tree it will still impact the neutron documentation but will impact these out of tree components more.","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"f5ea2bef6c5751293a6adc0f6ea134c0f64bf9a5","unresolved":false,"context_lines":[{"line_number":154,"context_line":""},{"line_number":155,"context_line":"Deployment of Nova is not impacted. Deployment of Neutron however will need to"},{"line_number":156,"context_line":"install plugin scripts into an appropriate accessible location so it can be"},{"line_number":157,"context_line":"called by Nova."},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"Developer impact"},{"line_number":160,"context_line":"----------------"}],"source_content_type":"text/x-rst","patch_set":3,"id":"1aca2d91_62f5b6c9","line":157,"in_reply_to":"3acd31a7_982a558f","updated":"2015-05-07 18:41:17.000000000","message":"Done","commit_id":"36ee12a1a432bcd2e0440327c1b96b7ef0af929b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ec767b06bade39129a3496ab0ff5092d605c702b","unresolved":false,"context_lines":[{"line_number":67,"context_line":"to the script by populating environment variables."},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"The script is called by nova in the following way:"},{"line_number":70,"context_line":"ENVIRONMEMT_VARIABLES  plug_script.[sh|py] [plug|unplug]"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"The ENVIRONMENT_VARIABLES are generated from the data members of the relevant"},{"line_number":73,"context_line":"VIF object and follow the general format:"}],"source_content_type":"text/x-rst","patch_set":4,"id":"1aca2d91_2b8bfbc4","line":70,"updated":"2015-05-08 14:23:44.000000000","message":"if you are supporting more then just python why restrict to sh.\n\nsome might perfer to have a plug script in perl,ruby,powershell\n\nprovided they can parse the environmental variables, command line arguments   and return exit codes then they should work with this mechanism too.","commit_id":"8a6246f131d0ff697d9eee58e8f976fb18d3694e"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"10819737223c8f288d5fdc0d2aa197a1a39c6927","unresolved":false,"context_lines":[{"line_number":67,"context_line":"to the script by populating environment variables."},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"The script is called by nova in the following way:"},{"line_number":70,"context_line":"ENVIRONMEMT_VARIABLES  plug_script.[sh|py] [plug|unplug]"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"The ENVIRONMENT_VARIABLES are generated from the data members of the relevant"},{"line_number":73,"context_line":"VIF object and follow the general format:"}],"source_content_type":"text/x-rst","patch_set":4,"id":"1aca2d91_0665d6df","line":70,"in_reply_to":"1aca2d91_2b8bfbc4","updated":"2015-05-08 14:33:34.000000000","message":"Yes, of course you are right. That is an oversight.","commit_id":"8a6246f131d0ff697d9eee58e8f976fb18d3694e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ec767b06bade39129a3496ab0ff5092d605c702b","unresolved":false,"context_lines":[{"line_number":70,"context_line":"ENVIRONMEMT_VARIABLES  plug_script.[sh|py] [plug|unplug]"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"The ENVIRONMENT_VARIABLES are generated from the data members of the relevant"},{"line_number":73,"context_line":"VIF object and follow the general format:"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"\"VIF_%s_%s\" % (attribute_name.upper(), str(attribute.value))"},{"line_number":76,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"1aca2d91_00513e62","line":73,"updated":"2015-05-08 14:23:44.000000000","message":"the instance object will also need to be serialized for example see\n\n def plug_vhostuser(self, instance, vif):\n        ovs_plug \u003d vif[\u0027details\u0027].get(\n                                network_model.VIF_DETAILS_VHOSTUSER_OVS_PLUG,\n                                False)\n        if ovs_plug:\n            iface_id \u003d self.get_ovs_interfaceid(vif)\n            port_name \u003d os.path.basename(\n                    vif[\u0027details\u0027][network_model.VIF_DETAILS_VHOSTUSER_SOCKET])\n            linux_net.create_ovs_vif_port(self.get_bridge_name(vif),\n                                          port_name, iface_id, vif[\u0027address\u0027],\n                                          instance.uuid)\n            linux_net.ovs_set_vhostuser_port_type(port_name)\n\nin this case the instance uuid is used int he port creation\n\nit is also used in most other plug methods such as\n\nplug_iovisor\n\nplug_vrouter\n\nplug_mlnx_direct\n\nto support the same plug mechanisms as currently used the instance object should also be serialized when invoking a plug script.","commit_id":"8a6246f131d0ff697d9eee58e8f976fb18d3694e"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"10819737223c8f288d5fdc0d2aa197a1a39c6927","unresolved":false,"context_lines":[{"line_number":70,"context_line":"ENVIRONMEMT_VARIABLES  plug_script.[sh|py] [plug|unplug]"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"The ENVIRONMENT_VARIABLES are generated from the data members of the relevant"},{"line_number":73,"context_line":"VIF object and follow the general format:"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"\"VIF_%s_%s\" % (attribute_name.upper(), str(attribute.value))"},{"line_number":76,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"1aca2d91_c68c7e81","line":73,"in_reply_to":"1aca2d91_00513e62","updated":"2015-05-08 14:33:34.000000000","message":"The instance ID will be included (the current POC includes that). Thanks for pointing this out. When \"instance object will be serialized\" I think of the object itself, not just the id - hence my response to the previous related comment.","commit_id":"8a6246f131d0ff697d9eee58e8f976fb18d3694e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ec767b06bade39129a3496ab0ff5092d605c702b","unresolved":false,"context_lines":[{"line_number":115,"context_line":"There is no data model impact. The VIF_PLUGIN_SCRIPT is included in the"},{"line_number":116,"context_line":"\u0027details\u0027 field of nova.network.model.VIF and is stored as part of the BLOB"},{"line_number":117,"context_line":"string. Migration does not present a problem as only new VIF types and"},{"line_number":118,"context_line":"instances will be supported in this iteration."},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"REST API impact"},{"line_number":121,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"1aca2d91_405bb62e","line":118,"updated":"2015-05-08 14:23:44.000000000","message":"this statement means at this will not be supported for current in tree vif types including VIF_TYPE_VHOSTUSER unless addressed by a separate spec.\n\nVIF_TYPE_TAP would be supported as it is a new vif type in this cycle","commit_id":"8a6246f131d0ff697d9eee58e8f976fb18d3694e"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"b55da896483f3717319cd40b18510bdcbaddea80","unresolved":false,"context_lines":[{"line_number":115,"context_line":"There is no data model impact. The VIF_PLUGIN_SCRIPT is included in the"},{"line_number":116,"context_line":"\u0027details\u0027 field of nova.network.model.VIF and is stored as part of the BLOB"},{"line_number":117,"context_line":"string. Migration does not present a problem as only new VIF types and"},{"line_number":118,"context_line":"instances will be supported in this iteration."},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"REST API impact"},{"line_number":121,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"1aca2d91_a6de220f","line":118,"in_reply_to":"1aca2d91_405bb62e","updated":"2015-05-08 15:04:45.000000000","message":"Poor choice of words. Clarified.","commit_id":"8a6246f131d0ff697d9eee58e8f976fb18d3694e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ec767b06bade39129a3496ab0ff5092d605c702b","unresolved":false,"context_lines":[{"line_number":123,"context_line":"The VIF_PLUGIN_SCRIPT is encapsulated in the Neutron port details and does not"},{"line_number":124,"context_line":"require a change to the Neutron API from that perspective."},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"The Neutron client and Neutron may allow administrators to specify the script"},{"line_number":127,"context_line":"as part of the port creation, which would require changes within Neutron."},{"line_number":128,"context_line":"However there is no API impact that would affect Nova."},{"line_number":129,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"1aca2d91_206afab3","line":126,"updated":"2015-05-08 14:23:44.000000000","message":"i taught this section was going to be removed?","commit_id":"8a6246f131d0ff697d9eee58e8f976fb18d3694e"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"b55da896483f3717319cd40b18510bdcbaddea80","unresolved":false,"context_lines":[{"line_number":123,"context_line":"The VIF_PLUGIN_SCRIPT is encapsulated in the Neutron port details and does not"},{"line_number":124,"context_line":"require a change to the Neutron API from that perspective."},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"The Neutron client and Neutron may allow administrators to specify the script"},{"line_number":127,"context_line":"as part of the port creation, which would require changes within Neutron."},{"line_number":128,"context_line":"However there is no API impact that would affect Nova."},{"line_number":129,"context_line":""}],"source_content_type":"text/x-rst","patch_set":4,"id":"1aca2d91_e6f3fa9c","line":126,"in_reply_to":"1aca2d91_206afab3","updated":"2015-05-08 15:04:45.000000000","message":"Done","commit_id":"8a6246f131d0ff697d9eee58e8f976fb18d3694e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ec767b06bade39129a3496ab0ff5092d605c702b","unresolved":false,"context_lines":[{"line_number":160,"context_line":""},{"line_number":161,"context_line":"Deployment of Nova is not impacted. However, deployment of Neutron components"},{"line_number":162,"context_line":"that use this mechanism will need to install plugin scripts into an appropriate"},{"line_number":163,"context_line":"accessible location so it can be called by Nova."},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"Developer impact"},{"line_number":166,"context_line":"----------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"1aca2d91_6b6fb3c5","line":163,"updated":"2015-05-08 14:23:44.000000000","message":"while this statment is more general  perhaps it is better if the the location and naming scheme was defined by nova.\n\n/etc/nova is probably not correct but for example\n/etc/nova/plug_handler/\u003cmech driver/plugin name\u003e/\u003cplug script\u003e\n\nthis will simplfy deployment/documentation as plug scripts will always be stored in one location and standarise the VIF_PLUG_SCRIPT vairable to be \"\u003cmech driver/plugin name\u003e/\u003cplug script\u003e\"","commit_id":"8a6246f131d0ff697d9eee58e8f976fb18d3694e"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"b55da896483f3717319cd40b18510bdcbaddea80","unresolved":false,"context_lines":[{"line_number":160,"context_line":""},{"line_number":161,"context_line":"Deployment of Nova is not impacted. However, deployment of Neutron components"},{"line_number":162,"context_line":"that use this mechanism will need to install plugin scripts into an appropriate"},{"line_number":163,"context_line":"accessible location so it can be called by Nova."},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"Developer impact"},{"line_number":166,"context_line":"----------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"1aca2d91_7ad41a46","line":163,"in_reply_to":"1aca2d91_6b6fb3c5","updated":"2015-05-08 15:04:45.000000000","message":"Maybe I\u0027m misunderstanding you, but a few points come to mind:\n- The script itself is a neutron detail, not a nova one. I\u0027d expect locations to be more neutron-centric than nova.\n- The location of the script is entirely a deployment choice for the operators, isn\u0027t it? The notion of standardized location doesn\u0027t make much sense to me. It might be on a shared drive for example.\n\nIs there a particular reason that you feel a nova based location is required?","commit_id":"8a6246f131d0ff697d9eee58e8f976fb18d3694e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4a917094134aa643634b0a019bb15d73bb354b0a","unresolved":false,"context_lines":[{"line_number":160,"context_line":""},{"line_number":161,"context_line":"Deployment of Nova is not impacted. However, deployment of Neutron components"},{"line_number":162,"context_line":"that use this mechanism will need to install plugin scripts into an appropriate"},{"line_number":163,"context_line":"accessible location so it can be called by Nova."},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"Developer impact"},{"line_number":166,"context_line":"----------------"}],"source_content_type":"text/x-rst","patch_set":4,"id":"9a0c5dc1_42cc3335","line":163,"in_reply_to":"1aca2d91_7ad41a46","updated":"2015-05-12 20:24:07.000000000","message":"i guess i view the script differently. the plug actions currently is a nova responsibility. the script will be invoked by the nova compute agent hence my suggestion that this should be a nova based location.\n\nthere are two reason i would prefer a standardized location. \n\nfirst is documentation. \n\nif there is a standardized location then it makes documenting the responsibility and invocation of the script easier. \n\nsecondly having a standard location means that the entire shell path does not have to be searched to find the scrips and run it.\n\nneither are blocking issues but that was my motivation.","commit_id":"8a6246f131d0ff697d9eee58e8f976fb18d3694e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ec767b06bade39129a3496ab0ff5092d605c702b","unresolved":false,"context_lines":[{"line_number":190,"context_line":"  * validation of file location,"},{"line_number":191,"context_line":"  * population of environment variables,"},{"line_number":192,"context_line":"  * execution of script (logging of output when verbose and debug are True)"},{"line_number":193,"context_line":"  * interpretation of error codes"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"Dependencies"},{"line_number":196,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":4,"id":"1aca2d91_ab5f0be4","line":193,"updated":"2015-05-08 14:23:44.000000000","message":"should a standard set of error codes be defined or just 0 for success and anything else for failure.\n\ne.g. \n\n-1 vif binding failed\n\n-2 invalid argument\n\n-3 permission denied\n\n... \n\nthis could result in better errors when someting goes wrong.\nthe error stream from the script could then be uses as the body of a standard exception. that might be overkill but it is worth discussing.\n\nsilent failures are always the hardest to fix.","commit_id":"8a6246f131d0ff697d9eee58e8f976fb18d3694e"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"b55da896483f3717319cd40b18510bdcbaddea80","unresolved":false,"context_lines":[{"line_number":190,"context_line":"  * validation of file location,"},{"line_number":191,"context_line":"  * population of environment variables,"},{"line_number":192,"context_line":"  * execution of script (logging of output when verbose and debug are True)"},{"line_number":193,"context_line":"  * interpretation of error codes"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"Dependencies"},{"line_number":196,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":4,"id":"1aca2d91_ba1632ef","line":193,"in_reply_to":"1aca2d91_ab5f0be4","updated":"2015-05-08 15:04:45.000000000","message":"Yes. I think this will need to be worked out somewhat. I think it will be more clear what a basic list will need to be as we get people actually using the facillity. Silent failure is not going to happen anyway as a non zero exit code will result in an exception with the error code include. The \"interpretation of error codes\" was meant to convey the possibility of taking specific error codes and converting to an exception.\n\nDo you feel it is necessary to outline a preliminary standard list in this spec?","commit_id":"8a6246f131d0ff697d9eee58e8f976fb18d3694e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"ec767b06bade39129a3496ab0ff5092d605c702b","unresolved":false,"context_lines":[{"line_number":201,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"This mechanism is primarily for supporting 3rd party networking implementations"},{"line_number":204,"context_line":"and will depend on 3rd party CI jobs and reporting for validation."},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"Documentation Impact"},{"line_number":207,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":4,"id":"1aca2d91_8b932f4f","line":204,"updated":"2015-05-08 14:23:44.000000000","message":"i think it would be good to include some unit tests at least with mocked scripts\nto validate that the mechanium works in nova.\n\nideally functional tests would also be added as i dont think relying solely on 3rd party ci jobs is enough.","commit_id":"8a6246f131d0ff697d9eee58e8f976fb18d3694e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4a917094134aa643634b0a019bb15d73bb354b0a","unresolved":false,"context_lines":[{"line_number":201,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"This mechanism is primarily for supporting 3rd party networking implementations"},{"line_number":204,"context_line":"and will depend on 3rd party CI jobs and reporting for validation."},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"Documentation Impact"},{"line_number":207,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":4,"id":"9a0c5dc1_e22567c2","line":204,"in_reply_to":"1aca2d91_7a4e5a97","updated":"2015-05-12 20:24:07.000000000","message":"no provided there is basic test coverage in tree for the feature, unit test might be sufficient  i think that is fine but i would be uncomfortable relying solely on 3rd party ci jobs\n\nmy main concern would be the serialization to environmental variables. provided that is functional tested i do not think other functional test are required.","commit_id":"8a6246f131d0ff697d9eee58e8f976fb18d3694e"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"b55da896483f3717319cd40b18510bdcbaddea80","unresolved":false,"context_lines":[{"line_number":201,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"This mechanism is primarily for supporting 3rd party networking implementations"},{"line_number":204,"context_line":"and will depend on 3rd party CI jobs and reporting for validation."},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"Documentation Impact"},{"line_number":207,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":4,"id":"1aca2d91_7a4e5a97","line":204,"in_reply_to":"1aca2d91_8b932f4f","updated":"2015-05-08 15:04:45.000000000","message":"Hah. I assumed unit tests were a given and that no patches that didn\u0027t include working unit tests would be merged.\n\nWhile functional tests *might* be beneficial, the synthesized code path would be pretty significant - I don\u0027t want to end up synthesizing a long expected path because we would end up effectively just stringing a bunch of unit tests together. Do you see this as a -1 issue?","commit_id":"8a6246f131d0ff697d9eee58e8f976fb18d3694e"},{"author":{"_account_id":13734,"name":"Nell Jerram","email":"nell@tigera.io","username":"neiljerram"},"change_message_id":"ef4a5262eefe0c77f494c5af4cb2c650fe2b322c","unresolved":false,"context_lines":[{"line_number":72,"context_line":"The ENVIRONMENT_VARIABLES are generated from the data members of the relevant"},{"line_number":73,"context_line":"VIF object and follow the general format:"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"\"VIF_%s_%s\" % (attribute_name.upper(), str(attribute.value))"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"If an attribute value is a nested dictionary, it is flattened with the prefix"},{"line_number":78,"context_line":"formed of a concatenation of the initial prefix, \"VIF\\_\", and the attribute"}],"source_content_type":"text/x-rst","patch_set":7,"id":"9a0c5dc1_49a58102","line":75,"updated":"2015-05-12 16:31:44.000000000","message":"Did you mean to write \t\"VIF_%s\u003d%s\" here?  I can\u0027t see why an attribute _value_ would become part of an environment variable _name_.","commit_id":"fee85d1f6c7919be3f7f4e5c3a28217f437d7d2f"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"0c4b9ffbc33a9924fce3d0b2b4560a54aac09f72","unresolved":false,"context_lines":[{"line_number":89,"context_line":""},{"line_number":90,"context_line":"The relevant instance ID is also included in the environment as"},{"line_number":91,"context_line":"VIF_INSTANCE_ID."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"Alternatives"},{"line_number":95,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":7,"id":"9af37de9_529312a4","line":92,"updated":"2015-05-09 20:58:52.000000000","message":"I think you should define what are the expected return code for the script.\nThis may be obvious as 0 - sucess 1- failure,  but  maybe we should consider other return code like 2- for warning. also you should define to what standard I/O streams the script should write for each return code. I assume you plan to log error on failure and log debug/info on success.","commit_id":"fee85d1f6c7919be3f7f4e5c3a28217f437d7d2f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"784a36a1e48f08cd60a383835771438d436565cb","unresolved":false,"context_lines":[{"line_number":89,"context_line":""},{"line_number":90,"context_line":"The relevant instance ID is also included in the environment as"},{"line_number":91,"context_line":"VIF_INSTANCE_ID."},{"line_number":92,"context_line":""},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"Alternatives"},{"line_number":95,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":7,"id":"9a0c5dc1_4020ab9b","line":92,"in_reply_to":"9af37de9_529312a4","updated":"2015-05-12 20:23:48.000000000","message":"yes i think this would be valuable.\n\ni personally prefer negative numbers for failure, 0 for success and positive non 0 numbers when there was an error that i handled internally or at least that my convention of function return codes.\n\nthe exact semantic/values are not important but it would be nice to have a small set of preferred return codes that can be used.","commit_id":"fee85d1f6c7919be3f7f4e5c3a28217f437d7d2f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"784a36a1e48f08cd60a383835771438d436565cb","unresolved":false,"context_lines":[{"line_number":202,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":203,"context_line":""},{"line_number":204,"context_line":"This mechanism is primarily for supporting 3rd party networking implementations"},{"line_number":205,"context_line":"and will depend on 3rd party CI jobs and reporting for validation."},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"Documentation Impact"},{"line_number":208,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":7,"id":"9a0c5dc1_25b029dd","line":205,"updated":"2015-05-12 20:23:48.000000000","message":"will a test to validate the environment variable mapping be added to nova?\n\nideally a mock vif would be serialized and a dumpy test script would be invoked.\n\nthe test script would read the environmentally variables and exit successfully if the expected values were found else exit with an error. \n\nthis is the only functional test that i think would be required to validate this feature as the operation of the the VIF_PLUGIN_SCRIPT is out of this spec.\n\nall other testing i think can be covered in unit tests and 3rd party ci jobs.","commit_id":"fee85d1f6c7919be3f7f4e5c3a28217f437d7d2f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d895d725114708cad6d951906bf70259cd836f1c","unresolved":false,"context_lines":[{"line_number":60,"context_line":"This change introduces a new VIF field, VIF_PLUGIN_SCRIPT. This field will be"},{"line_number":61,"context_line":"populated by Neutron and refer to a back-end specific script that will be"},{"line_number":62,"context_line":"called from the virtualization driver when the system is ready to connect the"},{"line_number":63,"context_line":"guest to a L2 network."},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"Nova shall be modified to call the script and interpret success or failure"},{"line_number":66,"context_line":"based on standard error level codes. Nova shall provide additional information"}],"source_content_type":"text/x-rst","patch_set":9,"id":"7a016987_d56b9883","line":63,"updated":"2015-05-22 08:52:05.000000000","message":"i would add that if the  VIF_PLUGIN_SCRIPT is not specified by neuton, the current plug behavior of all existing plug methods will not be changed.","commit_id":"f8f910522c680f156548a29a40e86a1156177bb7"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"b60db449fae929ddaf889e0aa6f2cdb66f935850","unresolved":false,"context_lines":[{"line_number":60,"context_line":"This change introduces a new VIF field, VIF_PLUGIN_SCRIPT. This field will be"},{"line_number":61,"context_line":"populated by Neutron and refer to a back-end specific script that will be"},{"line_number":62,"context_line":"called from the virtualization driver when the system is ready to connect the"},{"line_number":63,"context_line":"guest to a L2 network."},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"Nova shall be modified to call the script and interpret success or failure"},{"line_number":66,"context_line":"based on standard error level codes. Nova shall provide additional information"}],"source_content_type":"text/x-rst","patch_set":9,"id":"7a016987_b11c5b6b","line":63,"in_reply_to":"7a016987_d56b9883","updated":"2015-05-26 19:09:30.000000000","message":"Done","commit_id":"f8f910522c680f156548a29a40e86a1156177bb7"},{"author":{"_account_id":6802,"name":"Joel Coffman","email":"jmc7tp@gmail.com","username":"joel-coffman"},"change_message_id":"69ab1bb0fbfe3c30955a58834a51fc67cff704b5","unresolved":false,"context_lines":[{"line_number":76,"context_line":""},{"line_number":77,"context_line":"If an attribute value is a nested dictionary, it is flattened with the prefix"},{"line_number":78,"context_line":"formed of a concatenation of the initial prefix, \"VIF\\_\", and the attribute"},{"line_number":79,"context_line":"name. For example:"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"vif[\"details\"][\"ovs_hybrid_plug\"]\u003dTrue"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-rst","patch_set":9,"id":"7a016987_a2bd75d9","line":79,"updated":"2015-05-22 17:14:09.000000000","message":"There is many-to-one relationship between the attributes and environment variables. For example,\n\n  vif[\"details\"][\"ovs_hybrid_plug\"]\n\nand\n\n  vif[\"details_ovs\"][\"hybrid_plug\"]\n\nboth produce\n\n  VIF_DETAILS_OVS_HYBRID_PLUG\n\nThis collision could produce undefined behavior if the values are different -- i.e., the second environment variable will overwrite the first.","commit_id":"f8f910522c680f156548a29a40e86a1156177bb7"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"3f32ace96bf639b077d89bab1a063ff7ba8d1929","unresolved":false,"context_lines":[{"line_number":76,"context_line":""},{"line_number":77,"context_line":"If an attribute value is a nested dictionary, it is flattened with the prefix"},{"line_number":78,"context_line":"formed of a concatenation of the initial prefix, \"VIF\\_\", and the attribute"},{"line_number":79,"context_line":"name. For example:"},{"line_number":80,"context_line":""},{"line_number":81,"context_line":"vif[\"details\"][\"ovs_hybrid_plug\"]\u003dTrue"},{"line_number":82,"context_line":""}],"source_content_type":"text/x-rst","patch_set":9,"id":"7a016987_fdc2f283","line":79,"in_reply_to":"7a016987_a2bd75d9","updated":"2015-05-22 17:22:34.000000000","message":"While you are of course correct, it doesn\u0027t seem that we\u0027re likely to hit this case in practice. If we ever did, then I\u0027d suggest we just not use clashing names in the change that introduces the problem.","commit_id":"f8f910522c680f156548a29a40e86a1156177bb7"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"45b2cb9a7865ff36019baa8eaf0b97163fd11db2","unresolved":false,"context_lines":[{"line_number":95,"context_line":""},{"line_number":96,"context_line":"0 - Success"},{"line_number":97,"context_line":"-1 - Plug/unplug failure"},{"line_number":98,"context_line":"-2 - Warning"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"In all cases, scripts should write informational, warning or error messages to"},{"line_number":101,"context_line":"stderr."}],"source_content_type":"text/x-rst","patch_set":9,"id":"7a016987_273e37c1","line":98,"updated":"2015-05-22 16:32:24.000000000","message":"The WIFEXITED macro only extracts the 8 least significant bits of the value passed to exit(). Thus any negative values end up getting turned into positive values. So you should really define positive error values in the first place, if you want to have special interpretation of results. \n\nIn reality though, most shell scripts are so unbelievably badly written when it comes to error handling, that error codes from programs they invoke will inevitably leak back out to the caller, so any attempt to assign meaningful codes to specific exit status is really doomed to failure.\n\nIf a script has a non-fatal error and wants to issue a warning, it should just print the warning on stderr and return 0 to indicate success. Nova can capture stderr from the program and log it, so the admin will get a record of any warnings.  Then Nova can just treat any non-zero exit status as a fatal error which will be much more reliable.","commit_id":"f8f910522c680f156548a29a40e86a1156177bb7"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bc359c5302c24ac6424d99953362ee77fd0238dc","unresolved":false,"context_lines":[{"line_number":95,"context_line":""},{"line_number":96,"context_line":"0 - Success"},{"line_number":97,"context_line":"-1 - Plug/unplug failure"},{"line_number":98,"context_line":"-2 - Warning"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"In all cases, scripts should write informational, warning or error messages to"},{"line_number":101,"context_line":"stderr."}],"source_content_type":"text/x-rst","patch_set":9,"id":"7a016987_a2da1215","line":98,"in_reply_to":"7a016987_273e37c1","updated":"2015-05-22 18:34:17.000000000","message":"+1 \ni think that approach would satisfy the behavior we  wanted of being able to log non fatal errors and signal fatal error with a non 0 return code.","commit_id":"f8f910522c680f156548a29a40e86a1156177bb7"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"b60db449fae929ddaf889e0aa6f2cdb66f935850","unresolved":false,"context_lines":[{"line_number":95,"context_line":""},{"line_number":96,"context_line":"0 - Success"},{"line_number":97,"context_line":"-1 - Plug/unplug failure"},{"line_number":98,"context_line":"-2 - Warning"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"In all cases, scripts should write informational, warning or error messages to"},{"line_number":101,"context_line":"stderr."}],"source_content_type":"text/x-rst","patch_set":9,"id":"7a016987_b12e9ba3","line":98,"in_reply_to":"7a016987_a2da1215","updated":"2015-05-26 19:09:30.000000000","message":"Ok, I think I\u0027ve got this addressed. Please re-review.","commit_id":"f8f910522c680f156548a29a40e86a1156177bb7"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"45b2cb9a7865ff36019baa8eaf0b97163fd11db2","unresolved":false,"context_lines":[{"line_number":142,"context_line":"There is no security impact inherent to this mechanism. Implementations of the"},{"line_number":143,"context_line":"script may require extra permissions but that is within the scope of the"},{"line_number":144,"context_line":"configured commands or the implementation of the script, not the plug script"},{"line_number":145,"context_line":"mechanism."},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"Notifications impact"},{"line_number":148,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":9,"id":"7a016987_c7b88313","line":145,"updated":"2015-05-22 16:32:24.000000000","message":"There is a security impact in that any Neutron host now has the ability to get arbitrary code run on any Nova host, since Nova will blindly execute any pathname passed over.\n\neg, Neutron could set   VIF_PLUGIN_SCRIPT\u003d/bin/reboot\n\nObviously by design the plugin scripts can do pretty much anything, but I think we should set down some restrictions so that Neutron is only able to ask for execution of things that are explicitly considered to be plugin scripts.\n\neg we should require that all plugin scripts that are permitted to be executed are installed in a directory such as /usr/libexec/nova/vif-plugins.  Then do not allow Neutron to pass the fully qualified pathname - just a filename that is relative to that standard directory.  nova should validate that filename doesn\u0027t contain any \u0027/\u0027 too. That way Nova is at least somewhat protected wrt what can invoked.","commit_id":"f8f910522c680f156548a29a40e86a1156177bb7"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"b60db449fae929ddaf889e0aa6f2cdb66f935850","unresolved":false,"context_lines":[{"line_number":142,"context_line":"There is no security impact inherent to this mechanism. Implementations of the"},{"line_number":143,"context_line":"script may require extra permissions but that is within the scope of the"},{"line_number":144,"context_line":"configured commands or the implementation of the script, not the plug script"},{"line_number":145,"context_line":"mechanism."},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"Notifications impact"},{"line_number":148,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":9,"id":"7a016987_d108f72f","line":145,"in_reply_to":"7a016987_4266be51","updated":"2015-05-26 19:09:30.000000000","message":"That\u0027s a good reason :) I\u0027m sold. I\u0027ve made it configurable and added some description of behavior and validation.","commit_id":"f8f910522c680f156548a29a40e86a1156177bb7"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bc359c5302c24ac6424d99953362ee77fd0238dc","unresolved":false,"context_lines":[{"line_number":142,"context_line":"There is no security impact inherent to this mechanism. Implementations of the"},{"line_number":143,"context_line":"script may require extra permissions but that is within the scope of the"},{"line_number":144,"context_line":"configured commands or the implementation of the script, not the plug script"},{"line_number":145,"context_line":"mechanism."},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"Notifications impact"},{"line_number":148,"context_line":"--------------------"}],"source_content_type":"text/x-rst","patch_set":9,"id":"7a016987_4266be51","line":145,"in_reply_to":"7a016987_c7b88313","updated":"2015-05-22 18:34:17.000000000","message":"+1 this also allows execution fo the script directly without having to search the path and provides a common directory for documentation and installation of the scripts.","commit_id":"f8f910522c680f156548a29a40e86a1156177bb7"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"45b2cb9a7865ff36019baa8eaf0b97163fd11db2","unresolved":false,"context_lines":[{"line_number":156,"context_line":"feature?"},{"line_number":157,"context_line":""},{"line_number":158,"context_line":"* Does this change have an impact on python-novaclient? What does the user"},{"line_number":159,"context_line":"  interface there look like?"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":"Performance Impact"},{"line_number":162,"context_line":"------------------"}],"source_content_type":"text/x-rst","patch_set":9,"id":"7a016987_676d4fcb","line":159,"updated":"2015-05-22 16:32:24.000000000","message":"Boilerplate text to be deleted.","commit_id":"f8f910522c680f156548a29a40e86a1156177bb7"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"001e140188afc99555a324ae8b5ed4efc5488cf3","unresolved":false,"context_lines":[{"line_number":185,"context_line":"-----------"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"Primary assignee:"},{"line_number":188,"context_line":"  maxime-leroy \u003cmaxime.leroy@6wind.com\u003e irc:mleroy"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"Other assignee:"},{"line_number":191,"context_line":"  Brent Eagles \u003cbeagles@redhat.com\u003e irc: beagles"}],"source_content_type":"text/x-rst","patch_set":9,"id":"7a016987_1e9f1a60","line":188,"updated":"2015-05-21 22:49:53.000000000","message":"Thanks. I think you should be/remain the primary assignee, as you have done most of the jobs on the p.o.c patches.\n\nKeep me, in other assignee. I will continue help you by reviewing and testing it with ovs-fp ML2 mechanism driver (see https://git.openstack.org/cgit/stackforge/networking-6wind).","commit_id":"f8f910522c680f156548a29a40e86a1156177bb7"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"b60db449fae929ddaf889e0aa6f2cdb66f935850","unresolved":false,"context_lines":[{"line_number":185,"context_line":"-----------"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"Primary assignee:"},{"line_number":188,"context_line":"  maxime-leroy \u003cmaxime.leroy@6wind.com\u003e irc:mleroy"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"Other assignee:"},{"line_number":191,"context_line":"  Brent Eagles \u003cbeagles@redhat.com\u003e irc: beagles"}],"source_content_type":"text/x-rst","patch_set":9,"id":"7a016987_34085928","line":188,"in_reply_to":"7a016987_1e9f1a60","updated":"2015-05-26 19:09:30.000000000","message":"Done","commit_id":"f8f910522c680f156548a29a40e86a1156177bb7"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"df9cfa5b0f9f134e2b027ec2b7aeba3bd9a42d46","unresolved":false,"context_lines":[{"line_number":103,"context_line":"stderr. If the error code is 1, the contents of stderr will be logged. If the"},{"line_number":104,"context_line":"error code is 0, the contents of stderr will be logged if debug logging is"},{"line_number":105,"context_line":"enabled."},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"This change adds a configuration option, VIF_SCRIPT_DIR, to nova to specify a"},{"line_number":108,"context_line":"root filesystem location for the plug scripts. VIF_PLUGIN_SCRIPT field values"},{"line_number":109,"context_line":"specifying a prefix of \u0027/\u0027 or containing relative path modifiers such as \u0027..\u0027"}],"source_content_type":"text/x-rst","patch_set":10,"id":"7a016987_c9e39832","line":106,"updated":"2015-05-27 09:12:08.000000000","message":"I don\u0027t think we should explicitly check for exit code of 1 - we should allow for any non-zero exit code to indicate error.","commit_id":"8cddac83fe1992eb9ffe2a5ea67b119b67929209"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"1009639ce79f169e33dc8a57407b780878f5fb65","unresolved":false,"context_lines":[{"line_number":103,"context_line":"stderr. If the error code is 1, the contents of stderr will be logged. If the"},{"line_number":104,"context_line":"error code is 0, the contents of stderr will be logged if debug logging is"},{"line_number":105,"context_line":"enabled."},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"This change adds a configuration option, VIF_SCRIPT_DIR, to nova to specify a"},{"line_number":108,"context_line":"root filesystem location for the plug scripts. VIF_PLUGIN_SCRIPT field values"},{"line_number":109,"context_line":"specifying a prefix of \u0027/\u0027 or containing relative path modifiers such as \u0027..\u0027"}],"source_content_type":"text/x-rst","patch_set":10,"id":"7a016987_68c37758","line":106,"in_reply_to":"7a016987_c9e39832","updated":"2015-05-27 12:47:38.000000000","message":"Right. Will revise.","commit_id":"8cddac83fe1992eb9ffe2a5ea67b119b67929209"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"df9cfa5b0f9f134e2b027ec2b7aeba3bd9a42d46","unresolved":false,"context_lines":[{"line_number":104,"context_line":"error code is 0, the contents of stderr will be logged if debug logging is"},{"line_number":105,"context_line":"enabled."},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"This change adds a configuration option, VIF_SCRIPT_DIR, to nova to specify a"},{"line_number":108,"context_line":"root filesystem location for the plug scripts. VIF_PLUGIN_SCRIPT field values"},{"line_number":109,"context_line":"specifying a prefix of \u0027/\u0027 or containing relative path modifiers such as \u0027..\u0027"},{"line_number":110,"context_line":"are treated as invalid and will cause an error."}],"source_content_type":"text/x-rst","patch_set":10,"id":"7a016987_c968388a","line":107,"updated":"2015-05-27 09:12:08.000000000","message":"We should probably document a standard value for VIF_SCRIPT_DIR as vendors packaging their work will benefit from a standard location across all deployments.","commit_id":"8cddac83fe1992eb9ffe2a5ea67b119b67929209"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"1009639ce79f169e33dc8a57407b780878f5fb65","unresolved":false,"context_lines":[{"line_number":104,"context_line":"error code is 0, the contents of stderr will be logged if debug logging is"},{"line_number":105,"context_line":"enabled."},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"This change adds a configuration option, VIF_SCRIPT_DIR, to nova to specify a"},{"line_number":108,"context_line":"root filesystem location for the plug scripts. VIF_PLUGIN_SCRIPT field values"},{"line_number":109,"context_line":"specifying a prefix of \u0027/\u0027 or containing relative path modifiers such as \u0027..\u0027"},{"line_number":110,"context_line":"are treated as invalid and will cause an error."}],"source_content_type":"text/x-rst","patch_set":10,"id":"7a016987_e8ad279c","line":107,"in_reply_to":"7a016987_c968388a","updated":"2015-05-27 12:47:38.000000000","message":"I agree. Any suggestions? How about the location specified in you previous related comment?","commit_id":"8cddac83fe1992eb9ffe2a5ea67b119b67929209"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"df9cfa5b0f9f134e2b027ec2b7aeba3bd9a42d46","unresolved":false,"context_lines":[{"line_number":107,"context_line":"This change adds a configuration option, VIF_SCRIPT_DIR, to nova to specify a"},{"line_number":108,"context_line":"root filesystem location for the plug scripts. VIF_PLUGIN_SCRIPT field values"},{"line_number":109,"context_line":"specifying a prefix of \u0027/\u0027 or containing relative path modifiers such as \u0027..\u0027"},{"line_number":110,"context_line":"are treated as invalid and will cause an error."},{"line_number":111,"context_line":""},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":10,"id":"7a016987_e95414cb","line":110,"updated":"2015-05-27 09:12:08.000000000","message":"I think we should just disallow \u0027/\u0027 completely, rather than trying to allow relative sub-directories. I think its pretty reasonable to expect all the scripts to be in the same directory - sub-directories could be just for any extra assets the script might need","commit_id":"8cddac83fe1992eb9ffe2a5ea67b119b67929209"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"1009639ce79f169e33dc8a57407b780878f5fb65","unresolved":false,"context_lines":[{"line_number":107,"context_line":"This change adds a configuration option, VIF_SCRIPT_DIR, to nova to specify a"},{"line_number":108,"context_line":"root filesystem location for the plug scripts. VIF_PLUGIN_SCRIPT field values"},{"line_number":109,"context_line":"specifying a prefix of \u0027/\u0027 or containing relative path modifiers such as \u0027..\u0027"},{"line_number":110,"context_line":"are treated as invalid and will cause an error."},{"line_number":111,"context_line":""},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":10,"id":"7a016987_c87cab22","line":110,"in_reply_to":"7a016987_e95414cb","updated":"2015-05-27 12:47:38.000000000","message":"Ok, I\u0027m cool with that. Certainly makes it easier to validate.","commit_id":"8cddac83fe1992eb9ffe2a5ea67b119b67929209"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"726485d78876dcfaae1d04928616ba6f7816ad80","unresolved":false,"context_lines":[{"line_number":104,"context_line":"values containing path elements such as \u0027/\u0027 are treated as invalid and cause"},{"line_number":105,"context_line":"the a plug operation failure. If vif_script_dir is not specified, a default"},{"line_number":106,"context_line":"value of \u0027/usr/libexec/nova/vif-plugins\u0027 is used."},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"Alternatives"},{"line_number":110,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":13,"id":"5afe65bd_683f3b26","line":107,"updated":"2015-06-02 10:52:14.000000000","message":"I have several problems regarding  the vif_script_dir option:\n\n1. When your mechanism driver is installed, the vif_plug_script is copied into /usr/libexec/nova/vif_plugins. Later, if the user sets a specific value for vif_script_dir like /mypath/vif_plugins into nova.conf, nova will fail to find the script for the mechanism driver.  I think vif_script_dir should not be configurable. \n\n2. Having an absolute path for vif_script_dir breaks virtual environment support and standard python installations. Python files (like a python vif_plug script) should be installed under sys.prefix (i.e. /usr/local or ~/myproject/.venv). The vif_script_dir should be a path relative to sys.prefix.\n\n3. I didn\u0027t find how to install the script into vif_script_dir with pbr. It is not possible to get the vif_script_dir value dynamically into setup.cfg for the \u0027console_scripts\u0027 entry. To properly install the script, I need to specify console_scripts into setup.py.\n\nExample:\n  \n  CONF \u003d cfg.CONF\n  CONF([\u0027--config-file\u0027, \u0027/etc/nova/nova.conf\u0027])\n  CONF.import_opt(\u0027vif_script_dir\u0027, \u0027nova.virt.netutils\u0027)\n\n  setuptools.setup(\n      setup_requires\u003d[\u0027pbr\u0027],\n      pbr\u003dTrue,\n      entry_points\u003d{\n         ...\n        \u0027console_scripts\u0027: [\n            CONF.vif_script_dir + \u0027/vif-ovs-fp-plug \u003d networking_6wind.plug_scripts.vif_ovs_fp_plug:main\u0027,\n      ]\n  ),\n\nThis seems tedious and error prone. Maybe we need a new setuptools entry_point for vif_scripts ?","commit_id":"e2b5be183c4573b1e256c1b6b8a879831f53b546"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d53044244015241fe0d7e0a046bf6a6061cb6341","unresolved":false,"context_lines":[{"line_number":104,"context_line":"values containing path elements such as \u0027/\u0027 are treated as invalid and cause"},{"line_number":105,"context_line":"the a plug operation failure. If vif_script_dir is not specified, a default"},{"line_number":106,"context_line":"value of \u0027/usr/libexec/nova/vif-plugins\u0027 is used."},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"Alternatives"},{"line_number":110,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3afb71cf_bf3507f8","line":107,"in_reply_to":"3afb71cf_77eed623","updated":"2015-06-08 17:29:02.000000000","message":"i personally prefer having a  vif_script_dir under the nova dir but the lack of a way to specify this cleanly in setup.cfg dynamically is an issue. \n\none approach to supporting the vif_script_dir would be for the deployment tool to copy or symlink the VIF_PLUGIN_SCRIPT int the vif_script_dir during deployment of openstack. the benefit of this is that we do not pollute the system path with scripts that should only be called by nova.\n\nhaving said that if we enforce that no path separators \"\\\" or \"/\" are present and that VIF_PLUGIN_SCRIPT starts with \"vif-plugin-\" then yes i agree that Maxime\u0027s solution would be workable. i dislike polluting the system path but it may be the simplest solution from a deployment point of view.","commit_id":"e2b5be183c4573b1e256c1b6b8a879831f53b546"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"314040fa9d1faa1f216fb842f0dd628faa9c8a1f","unresolved":false,"context_lines":[{"line_number":104,"context_line":"values containing path elements such as \u0027/\u0027 are treated as invalid and cause"},{"line_number":105,"context_line":"the a plug operation failure. If vif_script_dir is not specified, a default"},{"line_number":106,"context_line":"value of \u0027/usr/libexec/nova/vif-plugins\u0027 is used."},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"Alternatives"},{"line_number":110,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3afb71cf_5bd4b5bb","line":107,"in_reply_to":"3afb71cf_bf3507f8","updated":"2015-06-09 15:01:13.000000000","message":"We already pollute the system path with the various openstack commands, so this is not really significantly worse, as long as we have a clear prefix - we should perhaps use \u0027nova-vif-\u003cblah\u003e\u0027 as the name, so its clear to admins it is a command that will be used by nova.","commit_id":"e2b5be183c4573b1e256c1b6b8a879831f53b546"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"df19c67d10bf7afd70a9477a938146568ee40429","unresolved":false,"context_lines":[{"line_number":104,"context_line":"values containing path elements such as \u0027/\u0027 are treated as invalid and cause"},{"line_number":105,"context_line":"the a plug operation failure. If vif_script_dir is not specified, a default"},{"line_number":106,"context_line":"value of \u0027/usr/libexec/nova/vif-plugins\u0027 is used."},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"Alternatives"},{"line_number":110,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":13,"id":"5afe65bd_8f313d0e","line":107,"in_reply_to":"5afe65bd_683f3b26","updated":"2015-06-02 11:13:20.000000000","message":"Really the expectation is that VIF plugins should be installed in to the same prefix as Nova is installed into. eg if Nova is in /opt/openstack the vif_script_dir should point to the dir under that location, or if nova is in /usr, the vif_script_dir will point there. This isn\u0027t really a configure parameters that admins should expect to change - rather something set when openstack is installed, typically by the package vendors","commit_id":"e2b5be183c4573b1e256c1b6b8a879831f53b546"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"986ad788125f79548baf5fd58a0ca57940d7ed71","unresolved":false,"context_lines":[{"line_number":104,"context_line":"values containing path elements such as \u0027/\u0027 are treated as invalid and cause"},{"line_number":105,"context_line":"the a plug operation failure. If vif_script_dir is not specified, a default"},{"line_number":106,"context_line":"value of \u0027/usr/libexec/nova/vif-plugins\u0027 is used."},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"Alternatives"},{"line_number":110,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":13,"id":"5afe65bd_eb198189","line":107,"in_reply_to":"5afe65bd_8f313d0e","updated":"2015-06-04 04:07:49.000000000","message":"Why vif-plugins dir should be in the same prefix as Nova ?\n\nMy concern is how I can configure the setup.cfg of my vendor plugins to install the script under vif_script_dir and not /usr/bin. The PBR library copies scripts under $base/bin or $base/local/bin. I didn\u0027t find any proper ways to change that.\n\nPlease see: https://review.openstack.org/#/c/188043/","commit_id":"e2b5be183c4573b1e256c1b6b8a879831f53b546"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"e617ab9fccf7fef13fb7f9a6d921b574db1baf60","unresolved":false,"context_lines":[{"line_number":104,"context_line":"values containing path elements such as \u0027/\u0027 are treated as invalid and cause"},{"line_number":105,"context_line":"the a plug operation failure. If vif_script_dir is not specified, a default"},{"line_number":106,"context_line":"value of \u0027/usr/libexec/nova/vif-plugins\u0027 is used."},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"Alternatives"},{"line_number":110,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":13,"id":"3afb71cf_77eed623","line":107,"in_reply_to":"5afe65bd_a91b75c9","updated":"2015-06-05 15:44:18.000000000","message":"I think that sounds like a workable idea, that still ensures we don\u0027t allow arbitrary commend execution.","commit_id":"e2b5be183c4573b1e256c1b6b8a879831f53b546"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"29ae4f6d361b4e1ca3db16c975db5635a6e6715f","unresolved":false,"context_lines":[{"line_number":104,"context_line":"values containing path elements such as \u0027/\u0027 are treated as invalid and cause"},{"line_number":105,"context_line":"the a plug operation failure. If vif_script_dir is not specified, a default"},{"line_number":106,"context_line":"value of \u0027/usr/libexec/nova/vif-plugins\u0027 is used."},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"Alternatives"},{"line_number":110,"context_line":"------------"}],"source_content_type":"text/x-rst","patch_set":13,"id":"5afe65bd_a91b75c9","line":107,"in_reply_to":"5afe65bd_eb198189","updated":"2015-06-04 09:37:27.000000000","message":"Instead of vif_script_dir, I propose:\n\n1. the vif script should be installed in $PATH (i.e. /usr/bin, /usr/local/bin, ~/.venv/bin). scripts or console_scripts in setup.cfg can be used to install the script in $PATH.\n\n2. VIF_PLUGIN_SCRIPT should be the name of the script, not a path (i.e. don\u0027t contain any \u0027/\u0027)\n\n3. VIF_PLUGIN_SCRIPT should start with the following prefix: vif-plugin-, example: vif-plugin-ovs-fp. It will prevent to run any arbitrary scripts on the host.\n\nWhat do you think ?","commit_id":"e2b5be183c4573b1e256c1b6b8a879831f53b546"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ba3817c37d03a1f922e0ccc7b72789bbcd99db02","unresolved":false,"context_lines":[{"line_number":29,"context_line":"  and review overhead as well as code that cannot be fully tested outside of"},{"line_number":30,"context_line":"  third party test suites."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"- Third party out-of-tree implemenations require Nova to publish and support a"},{"line_number":33,"context_line":"  stable extension API so the operations could be integrated with the core"},{"line_number":34,"context_line":"  components."},{"line_number":35,"context_line":""}],"source_content_type":"text/x-rst","patch_set":14,"id":"3afb71cf_53cec02e","line":32,"updated":"2015-06-10 03:18:10.000000000","message":"s/implemenations/implementations/","commit_id":"865091e2f665f9542bede8ab0a100978ae349d35"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"66b4db7e844e76e70c05d2811b84756b94943752","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"Proposed change"},{"line_number":58,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"This change introduces a new VIF field, VIF_PLUGIN_SCRIPT. This field will be"},{"line_number":61,"context_line":"populated by Neutron and refer to a back-end specific script that will be"},{"line_number":62,"context_line":"called from the virtualization driver when the system is ready to connect the"}],"source_content_type":"text/x-rst","patch_set":14,"id":"3afb71cf_4f4a5056","line":59,"updated":"2015-06-11 11:24:34.000000000","message":"Can we please include Dan\u0027s summary in here, I really like how he put things, to quote Dan:\n\nlibvirt has a pretty small finite number of different XML configurations it supports for network interfaces. Across the various VIF drivers that exist, we pretty much already cover all the different interesting XML configurations - in fact some of them are covered multiple times and only differ in the action of the plug/unplug methods. So the goal of this work is that we get to a place where we have 100% coverage of the different libvirt XML configurations, and thus will never have to change the libvirt VIF driver class again. All the vendor specific stuff that curently requires addition of new VIF driver code will live in the external script.","commit_id":"865091e2f665f9542bede8ab0a100978ae349d35"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ba3817c37d03a1f922e0ccc7b72789bbcd99db02","unresolved":false,"context_lines":[{"line_number":100,"context_line":"contents of stderr will be logged if debug logging is enabled."},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"Plug script filenames must begin with the prefix \u0027vif-plugin-\u0027 and contain no"},{"line_number":103,"context_line":"relative or absolute path elements. VIF_PLUG_SCRIPT values that do not begin"},{"line_number":104,"context_line":"with the prefix will result in an error. The script must be installed in a"},{"line_number":105,"context_line":"directory included in the PATH environment variable and standard path search"},{"line_number":106,"context_line":"rules apply."}],"source_content_type":"text/x-rst","patch_set":14,"id":"3afb71cf_be1ffb06","line":103,"updated":"2015-06-10 03:18:10.000000000","message":"s/PLUG/PLUGIN/","commit_id":"865091e2f665f9542bede8ab0a100978ae349d35"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ba3817c37d03a1f922e0ccc7b72789bbcd99db02","unresolved":false,"context_lines":[{"line_number":103,"context_line":"relative or absolute path elements. VIF_PLUG_SCRIPT values that do not begin"},{"line_number":104,"context_line":"with the prefix will result in an error. The script must be installed in a"},{"line_number":105,"context_line":"directory included in the PATH environment variable and standard path search"},{"line_number":106,"context_line":"rules apply."},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":14,"id":"3afb71cf_becdbb86","line":106,"updated":"2015-06-10 03:18:10.000000000","message":"Instead of needing the script to be installed in a directory included in PATH, why not have a configuration option like \"neutron_vif_plugin_script_dir\" or similar that Nova would then prepend to its utils.execute() call that runs this script file? That would ensure a singular place to put all these scripts and make the admins job a little easier (as well as the configuration management folks who will need to figure out how to get the Neutron vif plugin scripts into a particular filesystem location...","commit_id":"865091e2f665f9542bede8ab0a100978ae349d35"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"f0c28bad363f1d00ba0ca93a55da91fc73781906","unresolved":false,"context_lines":[{"line_number":103,"context_line":"relative or absolute path elements. VIF_PLUG_SCRIPT values that do not begin"},{"line_number":104,"context_line":"with the prefix will result in an error. The script must be installed in a"},{"line_number":105,"context_line":"directory included in the PATH environment variable and standard path search"},{"line_number":106,"context_line":"rules apply."},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":14,"id":"3afb71cf_7c82a1c9","line":106,"in_reply_to":"3afb71cf_369d67bc","updated":"2015-06-10 09:37:32.000000000","message":"If we want to set an absolute path like  /usr/libexec/nova/vif-plugins to install vif script, the setup.cfg of your neutron mechanism driver project should contains:\n\n   [entry_points]\nconsole_scripts \u003d\n  /usr/libexec/nova/vif-plugins/vif-mymechanismdriver-script \u003d mymechanismdriver.plugin_script:main\n\nBy doing that, we are breaking compatibility with virtual environment:\n\n   # cd ~/networking_mymechanismdriver\n   # virtualenv venv\n   # source venv/bin/activate\n   # python setup.py install\n      ....\n      Installing /usr/libexec/nova/vif-plugins/vif-mymechanismdriver-script into /path_to_my_project/networking_mymechanismdriver/venv/bin\n      error: [Errno 13] Permission denied \u0027/usr/libexec/nova/vif-plugins/vif-mymechanismdriver-script\n\nTox uses virtual environment to install your mechanism driver package under .tox/(name_of_the_test). So, for the unity tests is mandatory to support virtual environment.\n\nOne more thing, setting absolute path in setup.cfg is going against the standard python installation logic. The files including in a python package should be install under sys.prefix/ and not into an absolute path. For more information, please see:\nhttps://docs.python.org/2/install/#how-installation-works","commit_id":"865091e2f665f9542bede8ab0a100978ae349d35"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"66b4db7e844e76e70c05d2811b84756b94943752","unresolved":false,"context_lines":[{"line_number":103,"context_line":"relative or absolute path elements. VIF_PLUG_SCRIPT values that do not begin"},{"line_number":104,"context_line":"with the prefix will result in an error. The script must be installed in a"},{"line_number":105,"context_line":"directory included in the PATH environment variable and standard path search"},{"line_number":106,"context_line":"rules apply."},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":14,"id":"3afb71cf_cf1e405e","line":106,"in_reply_to":"3afb71cf_7c82a1c9","updated":"2015-06-11 11:24:34.000000000","message":"Maybe we have a config that defaults to using $PATH? I kinda like not just relaying on patch, but I also hate myself for suggesting we add another config variable.","commit_id":"865091e2f665f9542bede8ab0a100978ae349d35"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"0bf67b263dd1f9a084d9d42410fe89cb32269d89","unresolved":false,"context_lines":[{"line_number":103,"context_line":"relative or absolute path elements. VIF_PLUG_SCRIPT values that do not begin"},{"line_number":104,"context_line":"with the prefix will result in an error. The script must be installed in a"},{"line_number":105,"context_line":"directory included in the PATH environment variable and standard path search"},{"line_number":106,"context_line":"rules apply."},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"Alternatives"}],"source_content_type":"text/x-rst","patch_set":14,"id":"3afb71cf_369d67bc","line":106,"in_reply_to":"3afb71cf_becdbb86","updated":"2015-06-10 08:55:33.000000000","message":"That was done previously, but the objection was that it assumes everything is installed in the same prefix. If though Neutron \u0026 Nova are installed in /usr, but the Neutron mechanism is installed in some alternate prefix eg /opt/blah, then a single script dir doesn\u0027t work. By using $PATH, it is possible to dissassociate the install locations for the base software vs the vendor mechanism.\n\nPersonally I\u0027m not 100% convinced that is a scenario we need to really care about, but I\u0027m not too fussed really. My only real criteria is that there are strict rules on which scripts can be invoked, so Neutron doesn\u0027t get the ability to run totally arbitrary host OS binaries via Nova.","commit_id":"865091e2f665f9542bede8ab0a100978ae349d35"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"66b4db7e844e76e70c05d2811b84756b94943752","unresolved":false,"context_lines":[{"line_number":105,"context_line":"directory included in the PATH environment variable and standard path search"},{"line_number":106,"context_line":"rules apply."},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"Alternatives"},{"line_number":110,"context_line":"------------"},{"line_number":111,"context_line":""}],"source_content_type":"text/x-rst","patch_set":14,"id":"3afb71cf_afa6640c","line":108,"updated":"2015-06-11 11:24:34.000000000","message":"Should we want to say where we expect this script to live and who installs it?\n\nI think we expect the script to live inside the neutron plugin tree, rather than in the nova tree. Nova just uses the script, as directed by neutron.","commit_id":"865091e2f665f9542bede8ab0a100978ae349d35"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"df28c0f9994a51ebec5a7574db77af669b288329","unresolved":false,"context_lines":[{"line_number":105,"context_line":"directory included in the PATH environment variable and standard path search"},{"line_number":106,"context_line":"rules apply."},{"line_number":107,"context_line":""},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"Alternatives"},{"line_number":110,"context_line":"------------"},{"line_number":111,"context_line":""}],"source_content_type":"text/x-rst","patch_set":14,"id":"3afb71cf_32177746","line":108,"in_reply_to":"3afb71cf_afa6640c","updated":"2015-06-11 11:49:47.000000000","message":"The text here already says that the script must live in $PATH, which in most cases will be /usr/bin - basically whereever python setuptools installs executes.","commit_id":"865091e2f665f9542bede8ab0a100978ae349d35"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"66b4db7e844e76e70c05d2811b84756b94943752","unresolved":false,"context_lines":[{"line_number":122,"context_line":"implement VIF plugging. This is inappropriately \"heavy\" for what amounts to a"},{"line_number":123,"context_line":"few calls with some variables. It introduces a class of errors that need to be"},{"line_number":124,"context_line":"handled such as import errors for the implementation modules, unknown"},{"line_number":125,"context_line":"exceptions percolating up to Nova, etc."},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"As the ultimate result in both approaches is typically calling an external"},{"line_number":128,"context_line":"command, the script approach is more direct and sensible."}],"source_content_type":"text/x-rst","patch_set":14,"id":"3afb71cf_5c4d6cc2","line":125,"updated":"2015-06-11 11:24:34.000000000","message":"But is that not better than re-inventing another import mechanism? I duno, this seems quite attractive, if we keen that API very simple.","commit_id":"865091e2f665f9542bede8ab0a100978ae349d35"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"ba3817c37d03a1f922e0ccc7b72789bbcd99db02","unresolved":false,"context_lines":[{"line_number":145,"context_line":"Security impact"},{"line_number":146,"context_line":"---------------"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"The VIF_PLUG_SCRIPT validation rules prevent arbitrary commands on the system"},{"line_number":149,"context_line":"from being executed. It is the deployer\u0027s responsibility to prevent scripts"},{"line_number":150,"context_line":"accessing dangerous programs or resources."},{"line_number":151,"context_line":""}],"source_content_type":"text/x-rst","patch_set":14,"id":"3afb71cf_1e24cf5d","line":148,"updated":"2015-06-10 03:18:10.000000000","message":"s/PLUG/PLUGIN/","commit_id":"865091e2f665f9542bede8ab0a100978ae349d35"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"66b4db7e844e76e70c05d2811b84756b94943752","unresolved":false,"context_lines":[{"line_number":147,"context_line":""},{"line_number":148,"context_line":"The VIF_PLUG_SCRIPT validation rules prevent arbitrary commands on the system"},{"line_number":149,"context_line":"from being executed. It is the deployer\u0027s responsibility to prevent scripts"},{"line_number":150,"context_line":"accessing dangerous programs or resources."},{"line_number":151,"context_line":""},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"Notifications impact"}],"source_content_type":"text/x-rst","patch_set":14,"id":"3afb71cf_cf7ac076","line":150,"updated":"2015-06-11 11:24:34.000000000","message":"I wonder if we want Nova to have a whitelist of allowed plug-in scripts, to limit the impact of a compromised neutron system?\n\nAlthough the more I think about that, the more stupid that sounds.","commit_id":"865091e2f665f9542bede8ab0a100978ae349d35"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"df28c0f9994a51ebec5a7574db77af669b288329","unresolved":false,"context_lines":[{"line_number":147,"context_line":""},{"line_number":148,"context_line":"The VIF_PLUG_SCRIPT validation rules prevent arbitrary commands on the system"},{"line_number":149,"context_line":"from being executed. It is the deployer\u0027s responsibility to prevent scripts"},{"line_number":150,"context_line":"accessing dangerous programs or resources."},{"line_number":151,"context_line":""},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"Notifications impact"}],"source_content_type":"text/x-rst","patch_set":14,"id":"3afb71cf_0f3458f9","line":150,"in_reply_to":"3afb71cf_cf7ac076","updated":"2015-06-11 11:49:47.000000000","message":"A whitelist would defeat one of the main purposes of this proposal, which is to allow Neutron plugins to innovate without having to constantly lock-step update Nova.\n\nWe have sufficient security protection with the proposal that requires the plugin scripts to have a well defined name prefix - it prevents Neutron requesting execution of arbitrary OS programs","commit_id":"865091e2f665f9542bede8ab0a100978ae349d35"},{"author":{"_account_id":13734,"name":"Nell Jerram","email":"nell@tigera.io","username":"neiljerram"},"change_message_id":"683d82252740cce912feae7a75694d6c3da459f5","unresolved":false,"context_lines":[{"line_number":147,"context_line":""},{"line_number":148,"context_line":"The VIF_PLUG_SCRIPT validation rules prevent arbitrary commands on the system"},{"line_number":149,"context_line":"from being executed. It is the deployer\u0027s responsibility to prevent scripts"},{"line_number":150,"context_line":"accessing dangerous programs or resources."},{"line_number":151,"context_line":""},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"Notifications impact"}],"source_content_type":"text/x-rst","patch_set":14,"id":"3afb71cf_6fd98cf2","line":150,"in_reply_to":"3afb71cf_cf7ac076","updated":"2015-06-11 11:35:39.000000000","message":"Aargh, please no!  If we had that, then any Neutron-based networking experimentation work would _still_ need to push through a Nova change as well - even after all this work that we\u0027re trying to do here to make that unnecessary in the future.","commit_id":"865091e2f665f9542bede8ab0a100978ae349d35"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"18c3d2c70ffefba66b20520f04217b989075681c","unresolved":false,"context_lines":[{"line_number":58,"context_line":"Project Priority"},{"line_number":59,"context_line":"-----------------"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"TBD"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"Proposed change"},{"line_number":64,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_4dba36e3","line":61,"updated":"2015-06-12 15:47:46.000000000","message":"So I am tempted to make this a priority, along with the brick work, and glance v2 work, so lets leave TBD for now.","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0acf9bfcd818c443bf9b0ea5e0480dbd9954dbb5","unresolved":false,"context_lines":[{"line_number":71,"context_line":"have 100% coverage of the different libvirt XML configurations, and thus will"},{"line_number":72,"context_line":"not have to change the libvirt VIF driver class except to add future relevant"},{"line_number":73,"context_line":"XML configurations. This change places all the vendor specific operations that"},{"line_number":74,"context_line":"currently requires addition of new VIF driver code in an external script."},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"Moving functionality intrinsically external to Nova into an external script, we"},{"line_number":77,"context_line":"prevent incurring additional technical debt in supporting additional network"}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_9cd05555","line":74,"updated":"2015-06-12 18:09:06.000000000","message":"Okay, so, neutron says \"this is a foobar vif\" and nova runs /path/to/vif-plugin-foobar with the details of the neutron reply. That script barfs up some XML and we jam it into our guest xml before we build, is that right?\n\nI really don\u0027t see why we need to do that as an external script. We have to go to a lot of work to generate the environment, which will be annoying for consumers to re-consolidate into something coherent, and of course has security and performance implications.\n\nIMHO, this should be a library with a strong interface contract that we consume. I think it should probably be owned by neutron, although Nova people will have to participate in a non-trivial way I expect. That way, Nova is not providing a straight-up plugin architecture, but defining what VIF plugging looks like. If the library needs to have plugin support for out-of-tree stuff, then so be it, but that way nova is somewhat isolated from this behavior.","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"bb1d351c7e79122cebfa8e9bf796f34a93f922db","unresolved":false,"context_lines":[{"line_number":71,"context_line":"have 100% coverage of the different libvirt XML configurations, and thus will"},{"line_number":72,"context_line":"not have to change the libvirt VIF driver class except to add future relevant"},{"line_number":73,"context_line":"XML configurations. This change places all the vendor specific operations that"},{"line_number":74,"context_line":"currently requires addition of new VIF driver code in an external script."},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"Moving functionality intrinsically external to Nova into an external script, we"},{"line_number":77,"context_line":"prevent incurring additional technical debt in supporting additional network"}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_f03f6d23","line":74,"in_reply_to":"1af86dd1_9cd05555","updated":"2015-06-12 18:32:40.000000000","message":"No, the script will have *nothing* todo with the XML whatsoever - see my comment later.","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"18c3d2c70ffefba66b20520f04217b989075681c","unresolved":false,"context_lines":[{"line_number":72,"context_line":"not have to change the libvirt VIF driver class except to add future relevant"},{"line_number":73,"context_line":"XML configurations. This change places all the vendor specific operations that"},{"line_number":74,"context_line":"currently requires addition of new VIF driver code in an external script."},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"Moving functionality intrinsically external to Nova into an external script, we"},{"line_number":77,"context_line":"prevent incurring additional technical debt in supporting additional network"},{"line_number":78,"context_line":"implementations. Externalizing these operations also allows quality assurance"}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_cd54e655","line":75,"updated":"2015-06-12 15:47:46.000000000","message":"I like this intent.\n\nAfter giving it more thought, I really want to see if we can go a step further.\n\nWhat about deprecating any drivers that have code in that do things other than manipulate the libvirt XML?\n\nThey are mostly untested upstream, so it would be a great way to start pushing on this technical debt you describe in the next section.\n\nI would signal possible deletion in the N release, or something like that, to give folks time to replace them.\n\nBut I think we need to start that timer, so we make it very clear where new things should go.","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"580ccc54731bfb9b4605ad5e043b62d74d482b9c","unresolved":false,"context_lines":[{"line_number":72,"context_line":"not have to change the libvirt VIF driver class except to add future relevant"},{"line_number":73,"context_line":"XML configurations. This change places all the vendor specific operations that"},{"line_number":74,"context_line":"currently requires addition of new VIF driver code in an external script."},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"Moving functionality intrinsically external to Nova into an external script, we"},{"line_number":77,"context_line":"prevent incurring additional technical debt in supporting additional network"},{"line_number":78,"context_line":"implementations. Externalizing these operations also allows quality assurance"}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_41e2fa04","line":75,"in_reply_to":"1af86dd1_4df3510b","updated":"2015-06-12 17:54:33.000000000","message":"Yeah, I am tempted to state that here, but we could mod the spec later.","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":13734,"name":"Nell Jerram","email":"nell@tigera.io","username":"neiljerram"},"change_message_id":"177111a907e561d4f196476c1d475388f39bb4ce","unresolved":false,"context_lines":[{"line_number":72,"context_line":"not have to change the libvirt VIF driver class except to add future relevant"},{"line_number":73,"context_line":"XML configurations. This change places all the vendor specific operations that"},{"line_number":74,"context_line":"currently requires addition of new VIF driver code in an external script."},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"Moving functionality intrinsically external to Nova into an external script, we"},{"line_number":77,"context_line":"prevent incurring additional technical debt in supporting additional network"},{"line_number":78,"context_line":"implementations. Externalizing these operations also allows quality assurance"}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_aa462bbd","line":75,"in_reply_to":"1af86dd1_cd54e655","updated":"2015-06-12 16:04:40.000000000","message":"+1 to deprecating all plug/unplug code.  I\u0027ve been inching towards that in my comments, and I think you\u0027re right that it\u0027s the way forward.  It makes for a clear position about where plug/unplug logic should be - in the script - and explodes any concerns about whether the script should be able to override the built-in plug/unplug code.","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"5abfc012aa95982691cb99c9c09538c61951a081","unresolved":false,"context_lines":[{"line_number":72,"context_line":"not have to change the libvirt VIF driver class except to add future relevant"},{"line_number":73,"context_line":"XML configurations. This change places all the vendor specific operations that"},{"line_number":74,"context_line":"currently requires addition of new VIF driver code in an external script."},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"Moving functionality intrinsically external to Nova into an external script, we"},{"line_number":77,"context_line":"prevent incurring additional technical debt in supporting additional network"},{"line_number":78,"context_line":"implementations. Externalizing these operations also allows quality assurance"}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_4df3510b","line":75,"in_reply_to":"1af86dd1_cd54e655","updated":"2015-06-12 16:13:55.000000000","message":"I agree we need to deprecate existing drivers, but I feel like we probably want to get this new work done before we consider that. IOW, my feeling is we ship one release with existing stuff unchanged and fully supported, alongside the new plugin approach. The release after that we deprecate the non-plugin ones. And the release after that we remove it. So we span this over 3 cycles in total, counting Lemming as the first. In Muppet we\u0027d deprecate and in Naughty we\u0027d delete.","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"580ccc54731bfb9b4605ad5e043b62d74d482b9c","unresolved":false,"context_lines":[{"line_number":99,"context_line":"The ENVIRONMENT_VARIABLES are generated from the data members of the relevant"},{"line_number":100,"context_line":"VIF object and follow the general format:"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"\"VIF_%s\u003d%s\" % (attribute_name.upper(), str(attribute.value))"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"If an attribute value is a nested dictionary, it is flattened with the prefix"},{"line_number":105,"context_line":"formed of a concatenation of the initial prefix, \"VIF\\_\", and the attribute"}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_814fd291","line":102,"updated":"2015-06-12 17:54:33.000000000","message":"so I think this bit still worries me...","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0acf9bfcd818c443bf9b0ea5e0480dbd9954dbb5","unresolved":false,"context_lines":[{"line_number":146,"context_line":"implement VIF plugging. This is inappropriately \"heavy\" for what amounts to a"},{"line_number":147,"context_line":"few calls with some variables. It introduces a class of errors that need to be"},{"line_number":148,"context_line":"handled such as import errors for the implementation modules, unknown"},{"line_number":149,"context_line":"exceptions percolating up to Nova, etc."},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"As the ultimate result in both approaches is typically calling an external"},{"line_number":152,"context_line":"command, the script approach is more direct and sensible."}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_7cbfd9d0","line":149,"updated":"2015-06-12 18:09:06.000000000","message":"I don\u0027t agree with this. I think it\u0027s far more unnatural to make this an exec-based interface. I agree that the exec interface avoids us having to define a real API, and have something that manages the interface, the code, etc. I agree that this makes it far easier to just plop a script into a directory and be done with it. I don\u0027t agree that those are good reasons to do it this way.","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"18c3d2c70ffefba66b20520f04217b989075681c","unresolved":false,"context_lines":[{"line_number":154,"context_line":"These approaches also imply \"loading\" unknown code into the Nova process"},{"line_number":155,"context_line":"itself. This introduces a prohibitive risk as there are no enforceable"},{"line_number":156,"context_line":"protection mechanisms to prevent unvetted code from performing arbitrary"},{"line_number":157,"context_line":"operations within Nova."},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"Data model impact"},{"line_number":160,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_ad92ca55","line":157,"updated":"2015-06-12 15:47:46.000000000","message":"a very good point actually, I missed that point last time.","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"707e6ae819691bc0862f87ea17f395f0a9822524","unresolved":false,"context_lines":[{"line_number":154,"context_line":"These approaches also imply \"loading\" unknown code into the Nova process"},{"line_number":155,"context_line":"itself. This introduces a prohibitive risk as there are no enforceable"},{"line_number":156,"context_line":"protection mechanisms to prevent unvetted code from performing arbitrary"},{"line_number":157,"context_line":"operations within Nova."},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"Data model impact"},{"line_number":160,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":16,"id":"fa32b979_6db065f5","line":157,"in_reply_to":"1af86dd1_53876b61","updated":"2015-06-15 08:40:07.000000000","message":"As mentioned earlier in this spec, there is a pretty limited set of possible XML configs supported by libvirt, and the data that goes into those is controlled by the data passed back from Neutron in the port binding dict. That is a simple solved problem, which is why these scripts do not need to get involved in that aspect of it. On top of the finite set of XML configs, there is the ever growing list of vendor specific commands that need to be executed, which is what this mechanism allows for.  There is no need for a library to do two way mappings, and we certainly don\u0027t want to outsource the generation of the libvirt XML config to an external component, which is why we keep that part directly under libvirt driver control.","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":13734,"name":"Nell Jerram","email":"nell@tigera.io","username":"neiljerram"},"change_message_id":"59608ed33b48e3864a688303561d25f0acd6d587","unresolved":false,"context_lines":[{"line_number":154,"context_line":"These approaches also imply \"loading\" unknown code into the Nova process"},{"line_number":155,"context_line":"itself. This introduces a prohibitive risk as there are no enforceable"},{"line_number":156,"context_line":"protection mechanisms to prevent unvetted code from performing arbitrary"},{"line_number":157,"context_line":"operations within Nova."},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"Data model impact"},{"line_number":160,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_731267b6","line":157,"in_reply_to":"1af86dd1_53876b61","updated":"2015-06-12 18:55:00.000000000","message":"Sean - I\u0027m not clear what you mean here with \"this library\".  Do you mean the script?  And are you imagining that the Nova-generated XML would be an input into the script?\n\nPerhaps you meant that the set of all plugin scripts would be managed together and released coherently as a \"library\".  If so, I\u0027m afraid I don\u0027t believe that is the intention; rather, each Neutron backend would likely manage and release its own plugin script in the same way as it currently does with (e.g.) its ML2 mechanism driver code.","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"25977dc71c6bff865a4884945cfbe39f2433a481","unresolved":false,"context_lines":[{"line_number":154,"context_line":"These approaches also imply \"loading\" unknown code into the Nova process"},{"line_number":155,"context_line":"itself. This introduces a prohibitive risk as there are no enforceable"},{"line_number":156,"context_line":"protection mechanisms to prevent unvetted code from performing arbitrary"},{"line_number":157,"context_line":"operations within Nova."},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"Data model impact"},{"line_number":160,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_53876b61","line":157,"in_reply_to":"1af86dd1_90d8391f","updated":"2015-06-12 18:43:04.000000000","message":"So, the fact that the communication is only 1 way seems of limited value, because it will still require logic in nova for all the hypervisor specific config. It feels like it would be a lot nicer if this library had the two way mapping, which included exposing up hypervisor config. That would also let us release this library independently of Nova releases, and support new devices on older supported Nova releases without backports.","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"bb1d351c7e79122cebfa8e9bf796f34a93f922db","unresolved":false,"context_lines":[{"line_number":154,"context_line":"These approaches also imply \"loading\" unknown code into the Nova process"},{"line_number":155,"context_line":"itself. This introduces a prohibitive risk as there are no enforceable"},{"line_number":156,"context_line":"protection mechanisms to prevent unvetted code from performing arbitrary"},{"line_number":157,"context_line":"operations within Nova."},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"Data model impact"},{"line_number":160,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_90d8391f","line":157,"in_reply_to":"1af86dd1_9c8f3523","updated":"2015-06-12 18:32:40.000000000","message":"No, these scripts will have *NO* control over the XML whatsoever - that remains 100% under Nova\u0027s direct control. The XML contents will be decided entirely based on information provided in the VIF object metadata exclusively.\n\nThe script is only used for the plug/unplug actions. which are the doing the host OS setup for the port. As mentioned earlier, really the best architecture would be for Neutron to provide an agent on every compute node, and have neutron talk to that agent directly todo the VIF setup in the host OS. Neutron is strongly against having their own agent though.  These VIF scripts should be viewed as serving the same functional role as an agent would. So essentially instead of neutron having a direct message bus connection to an agent of its own, it is invoking its agent work indirectly by telling nova to run the script on its behalf. The end results is ultimately the same from a functional POV.","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"0acf9bfcd818c443bf9b0ea5e0480dbd9954dbb5","unresolved":false,"context_lines":[{"line_number":154,"context_line":"These approaches also imply \"loading\" unknown code into the Nova process"},{"line_number":155,"context_line":"itself. This introduces a prohibitive risk as there are no enforceable"},{"line_number":156,"context_line":"protection mechanisms to prevent unvetted code from performing arbitrary"},{"line_number":157,"context_line":"operations within Nova."},{"line_number":158,"context_line":""},{"line_number":159,"context_line":"Data model impact"},{"line_number":160,"context_line":"-----------------"}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_9c8f3523","line":157,"in_reply_to":"1af86dd1_ad92ca55","updated":"2015-06-12 18:09:06.000000000","message":"Seriously, we\u0027re letting this script barf up XML we\u0027re going to put into our guest\u0027s definition. I think we\u0027re pretty much trusting this script at the highest level. We load libraries all the time. They can misbehave. This is not a good reason to make this an exec-based interface, IMHO :)","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"18c3d2c70ffefba66b20520f04217b989075681c","unresolved":false,"context_lines":[{"line_number":176,"context_line":""},{"line_number":177,"context_line":"The VIF_PLUGIN_SCRIPT validation rules prevent arbitrary commands on the system"},{"line_number":178,"context_line":"from being executed. It is the deployer\u0027s responsibility to prevent scripts"},{"line_number":179,"context_line":"accessing dangerous programs or resources."},{"line_number":180,"context_line":""},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"Notifications impact"}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_4db6b6dd","line":179,"updated":"2015-06-12 15:47:46.000000000","message":"So this worries me, and I have an idea here...\n\nLets have a whitelist config value, so the admin is able to restrict what nova will script nova will call.\n\nNow I was thinking we can use that, so we ship the default setting to include the VIF scripts we have tested in some third party CI. That might mean the list is empty at first.\n\nWhile Neutron is not showing any level of restriction to VIF plugins, I think Nova should tell its users what drivers have been tested. This seems like a nice easy way to do that.\n\nIts just an idea at this point, there could be a horrid part of this I am not seeing.","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"580ccc54731bfb9b4605ad5e043b62d74d482b9c","unresolved":false,"context_lines":[{"line_number":176,"context_line":""},{"line_number":177,"context_line":"The VIF_PLUGIN_SCRIPT validation rules prevent arbitrary commands on the system"},{"line_number":178,"context_line":"from being executed. It is the deployer\u0027s responsibility to prevent scripts"},{"line_number":179,"context_line":"accessing dangerous programs or resources."},{"line_number":180,"context_line":""},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"Notifications impact"}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_412b1a31","line":179,"in_reply_to":"1af86dd1_2d1495f3","updated":"2015-06-12 17:54:33.000000000","message":"So I still think tell folks what we knows work is important.\n\nIt also limits the arbitrary code execution thing, somewhat.\n\nBut its something we could add later.","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"5abfc012aa95982691cb99c9c09538c61951a081","unresolved":false,"context_lines":[{"line_number":176,"context_line":""},{"line_number":177,"context_line":"The VIF_PLUGIN_SCRIPT validation rules prevent arbitrary commands on the system"},{"line_number":178,"context_line":"from being executed. It is the deployer\u0027s responsibility to prevent scripts"},{"line_number":179,"context_line":"accessing dangerous programs or resources."},{"line_number":180,"context_line":""},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"Notifications impact"}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_2d1495f3","line":179,"in_reply_to":"1af86dd1_4db6b6dd","updated":"2015-06-12 16:13:55.000000000","message":"I\u0027m not convinced about this need \u0026  benefit of a config option here. It feels like we\u0027d just adding a needless hurdle for people in dev, and something that will achieve nothing in production. ie any vendor\u0027s production provisioning tool would just update that config option, so no admin deploying openstack will ever get this support  message you\u0027re atempting to give to them.\n\nAlso, this is all about moving the burden of support out of Nova and into Neutron, so to me it feels pretty odd for Nova to be making a statement about the quality of support of something that belongs to Neutron.","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":13734,"name":"Nell Jerram","email":"nell@tigera.io","username":"neiljerram"},"change_message_id":"177111a907e561d4f196476c1d475388f39bb4ce","unresolved":false,"context_lines":[{"line_number":176,"context_line":""},{"line_number":177,"context_line":"The VIF_PLUGIN_SCRIPT validation rules prevent arbitrary commands on the system"},{"line_number":178,"context_line":"from being executed. It is the deployer\u0027s responsibility to prevent scripts"},{"line_number":179,"context_line":"accessing dangerous programs or resources."},{"line_number":180,"context_line":""},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"Notifications impact"}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_aae2ab9d","line":179,"in_reply_to":"1af86dd1_4db6b6dd","updated":"2015-06-12 16:04:40.000000000","message":"Sounds good to me.  It would be easy enough for an out of tree project such as mine to advise installers to extend that value.","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":13734,"name":"Nell Jerram","email":"nell@tigera.io","username":"neiljerram"},"change_message_id":"177111a907e561d4f196476c1d475388f39bb4ce","unresolved":false,"context_lines":[{"line_number":206,"context_line":"Developer impact"},{"line_number":207,"context_line":"----------------"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"Developers implementing connections with L2 networks not directly supported by"},{"line_number":210,"context_line":"virtualization drivers will use this facility instead of adding integration"},{"line_number":211,"context_line":"code to nova."},{"line_number":212,"context_line":""}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_aa906bed","line":209,"updated":"2015-06-12 16:04:40.000000000","message":"Nit: do you need to say \"L2\" here?  I\u0027m interested (and beginning to be involved) in supporting L3-routed Neutron networks, so am keen not to entrench the L2 meme more than it already is!","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"18c3d2c70ffefba66b20520f04217b989075681c","unresolved":false,"context_lines":[{"line_number":208,"context_line":""},{"line_number":209,"context_line":"Developers implementing connections with L2 networks not directly supported by"},{"line_number":210,"context_line":"virtualization drivers will use this facility instead of adding integration"},{"line_number":211,"context_line":"code to nova."},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"Implementation"},{"line_number":214,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":16,"id":"1af86dd1_ad3c0a30","line":211,"updated":"2015-06-12 15:47:46.000000000","message":"As above I think we want to be stronger than that, if we can.","commit_id":"963d405eea8fe6c2490a08305635b3ace0e9bdbd"},{"author":{"_account_id":3217,"name":"Ian Wells","username":"ijw-ubuntu"},"change_message_id":"db216d16686de30196fc3370e25934094a7bbc39","unresolved":false,"context_lines":[{"line_number":27,"context_line":"- New VIF types require additional code to be added to the hypervisor driver"},{"line_number":28,"context_line":"  to be called when a VIF is created or destroyed. This incurs technical debt"},{"line_number":29,"context_line":"  and review overhead as well as code that cannot be fully tested outside of"},{"line_number":30,"context_line":"  third party test suites."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"- Implementation of VIF types that are not supported by Nova Network must be"},{"line_number":33,"context_line":"  reviewed and approved by Nova reviewers. Typically, the Nova reviewer does"}],"source_content_type":"text/x-rst","patch_set":17,"id":"1af86dd1_ff54d654","line":30,"updated":"2015-06-12 20:56:11.000000000","message":"They still do.  You just hope that there will be fewer of them.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":3217,"name":"Ian Wells","username":"ijw-ubuntu"},"change_message_id":"db216d16686de30196fc3370e25934094a7bbc39","unresolved":false,"context_lines":[{"line_number":32,"context_line":"- Implementation of VIF types that are not supported by Nova Network must be"},{"line_number":33,"context_line":"  reviewed and approved by Nova reviewers. Typically, the Nova reviewer does"},{"line_number":34,"context_line":"  not have access to resources to allow them to test, debug or otherwise"},{"line_number":35,"context_line":"  evaluate changes, requiring them to approve some changes on \"faith\"."},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"- Third party out-of-tree implementations require Nova to publish and support a"},{"line_number":38,"context_line":"  stable extension API so the operations could be integrated with the core"}],"source_content_type":"text/x-rst","patch_set":17,"id":"1af86dd1_bf4e5ebf","line":35,"updated":"2015-06-12 20:56:11.000000000","message":"They still do.  You just hope that there will be fewer of them.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"7b396e81fd9777ec26f7e70ae33340ed02283a7d","unresolved":false,"context_lines":[{"line_number":32,"context_line":"- Implementation of VIF types that are not supported by Nova Network must be"},{"line_number":33,"context_line":"  reviewed and approved by Nova reviewers. Typically, the Nova reviewer does"},{"line_number":34,"context_line":"  not have access to resources to allow them to test, debug or otherwise"},{"line_number":35,"context_line":"  evaluate changes, requiring them to approve some changes on \"faith\"."},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"- Third party out-of-tree implementations require Nova to publish and support a"},{"line_number":38,"context_line":"  stable extension API so the operations could be integrated with the core"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_4d356144","line":35,"in_reply_to":"1af86dd1_bf4e5ebf","updated":"2015-06-15 08:57:55.000000000","message":"This isn\u0027t just a matter of hope - looking at existing / previous VIF type additions demonstrates this to be the case. There is lots of overlap in the libvirt VIF driver side of things that would be eliminated","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":3217,"name":"Ian Wells","username":"ijw-ubuntu"},"change_message_id":"db216d16686de30196fc3370e25934094a7bbc39","unresolved":false,"context_lines":[{"line_number":42,"context_line":"  lifecycle. For example, Nova would not accept code to support a new VIF type"},{"line_number":43,"context_line":"  that is not yet supported by Neutron. This introduces a risk that it may take"},{"line_number":44,"context_line":"  multiple cycles for a new type to be supported, even if the implementation is"},{"line_number":45,"context_line":"  complete and in-tree in Neutron."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"Use Cases"}],"source_content_type":"text/x-rst","patch_set":17,"id":"1af86dd1_bfe53ea9","line":45,"updated":"2015-06-12 20:56:11.000000000","message":"vhostuser would be the perfect example of this, and this spec can\u0027t build the next vhostuser.  Both problems do arise, granted, but I think we\u0027ve had more cases of \u0027I need new XML\u0027 than \u0027the XML suits me fine, but I need Nova-side weirdness\u0027.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"7b396e81fd9777ec26f7e70ae33340ed02283a7d","unresolved":false,"context_lines":[{"line_number":42,"context_line":"  lifecycle. For example, Nova would not accept code to support a new VIF type"},{"line_number":43,"context_line":"  that is not yet supported by Neutron. This introduces a risk that it may take"},{"line_number":44,"context_line":"  multiple cycles for a new type to be supported, even if the implementation is"},{"line_number":45,"context_line":"  complete and in-tree in Neutron."},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"Use Cases"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_0dd2f900","line":45,"in_reply_to":"1af86dd1_bfe53ea9","updated":"2015-06-15 08:57:55.000000000","message":"The vhostuser case was really the exception, as it was a case where libvirt had recently gained support for new VIF XML config. This is a very rare occurance - the first time this has actually happened in Nova - every other time we were targetting something that had existed in libvirt for years.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":8190,"name":"Drew Thorstensen (thorst)","email":"thorst@us.ibm.com","username":"thorst"},"change_message_id":"4b7956baff26942e9888e241ccae2e2026ea5084","unresolved":false,"context_lines":[{"line_number":61,"context_line":"Proposed change"},{"line_number":62,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"Libvirt has a small finite number of different XML configurations it supports"},{"line_number":65,"context_line":"for network interfaces. Across the various VIF drivers that exist, we pretty"},{"line_number":66,"context_line":"much already cover all the different interesting XML configurations - in fact"},{"line_number":67,"context_line":"some of them are covered multiple times and only differ in the action of the"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_0a99e37b","line":64,"updated":"2015-06-18 00:49:59.000000000","message":"Is this predominately specific to libvirt or do you envision that it would also be useful for other hypervisor drivers?\n\nIt seems like it would be the solution for all hypervisors...but just want to verify.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"4ddebb3344f19d264d15908f287f10049b032a26","unresolved":false,"context_lines":[{"line_number":61,"context_line":"Proposed change"},{"line_number":62,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"Libvirt has a small finite number of different XML configurations it supports"},{"line_number":65,"context_line":"for network interfaces. Across the various VIF drivers that exist, we pretty"},{"line_number":66,"context_line":"much already cover all the different interesting XML configurations - in fact"},{"line_number":67,"context_line":"some of them are covered multiple times and only differ in the action of the"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_5b53346b","line":64,"in_reply_to":"fa32b979_0a99e37b","updated":"2015-06-18 10:00:46.000000000","message":"The spec is focus on libvirt. But the vif_script feature should be extends to other hypervisor drivers. (with other blueprints).","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"ca83ded5a34bda57d02974592ac41a9e7d096064","unresolved":false,"context_lines":[{"line_number":61,"context_line":"Proposed change"},{"line_number":62,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"Libvirt has a small finite number of different XML configurations it supports"},{"line_number":65,"context_line":"for network interfaces. Across the various VIF drivers that exist, we pretty"},{"line_number":66,"context_line":"much already cover all the different interesting XML configurations - in fact"},{"line_number":67,"context_line":"some of them are covered multiple times and only differ in the action of the"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_f058e28c","line":64,"in_reply_to":"fa32b979_1f78fc8e","updated":"2015-06-19 16:19:03.000000000","message":"@Drew - yes, that\u0027s the general idea.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":8190,"name":"Drew Thorstensen (thorst)","email":"thorst@us.ibm.com","username":"thorst"},"change_message_id":"e35cb6a000fac7a7f458b0a36a847269c1b6e43b","unresolved":false,"context_lines":[{"line_number":61,"context_line":"Proposed change"},{"line_number":62,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"Libvirt has a small finite number of different XML configurations it supports"},{"line_number":65,"context_line":"for network interfaces. Across the various VIF drivers that exist, we pretty"},{"line_number":66,"context_line":"much already cover all the different interesting XML configurations - in fact"},{"line_number":67,"context_line":"some of them are covered multiple times and only differ in the action of the"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_1f78fc8e","line":64,"in_reply_to":"fa32b979_5b53346b","updated":"2015-06-19 12:02:15.000000000","message":"From the nova perspective, what parts actually require a new blue print?  Simply making sure the \u0027plug_vif\u0027 methods in the other drivers invoke the script?\n\nThe agents (and their scripts) within Neutron would obviously need to add support for the other hypervisors.  But outside the nova scope.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":11307,"name":"Andreas Scheuring","email":"andreas.scheuring@de.ibm.com","username":"uniqueName"},"change_message_id":"eb9d182cf1beead9bfd39e90d2b6c79d4816298e","unresolved":false,"context_lines":[{"line_number":65,"context_line":"for network interfaces. Across the various VIF drivers that exist, we pretty"},{"line_number":66,"context_line":"much already cover all the different interesting XML configurations - in fact"},{"line_number":67,"context_line":"some of them are covered multiple times and only differ in the action of the"},{"line_number":68,"context_line":"plug/unplug methods. A goal of this work is that to get to a place where we"},{"line_number":69,"context_line":"have 100% coverage of the different libvirt XML configurations in Nova, and"},{"line_number":70,"context_line":"thus will not have to change the libvirt VIF driver class except to add future"},{"line_number":71,"context_line":"relevant XML configurations. This change places all the vendor specific"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_072f9188","line":68,"updated":"2015-06-18 08:19:49.000000000","message":"s/is that to/is to","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":11307,"name":"Andreas Scheuring","email":"andreas.scheuring@de.ibm.com","username":"uniqueName"},"change_message_id":"eb9d182cf1beead9bfd39e90d2b6c79d4816298e","unresolved":false,"context_lines":[{"line_number":72,"context_line":"operations that currently requires addition of new VIF driver code in an"},{"line_number":73,"context_line":"external script."},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"Moving functionality intrinsically external to Nova into an external script, we"},{"line_number":76,"context_line":"prevent incurring additional technical debt in supporting additional network"},{"line_number":77,"context_line":"implementations. Externalizing these operations also allows quality assurance"},{"line_number":78,"context_line":"activities such as reviews and testing to be the responsibility of the most"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_91a1a348","line":75,"updated":"2015-06-18 08:19:49.000000000","message":"What does \"moving them external to nova\" mean? Into the vendors neutron driver/plugin repository? Or into a separate repo only containing vif-plug scripts?\n\nAnd what is meant by external script? From another comment I got that it could by any type of script (shell, python,..). Is that true? Could you make that clear somewhere in this spec, that it\u0027s not restricted to python?","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":11307,"name":"Andreas Scheuring","email":"andreas.scheuring@de.ibm.com","username":"uniqueName"},"change_message_id":"b5cbc5165fbf6bf658f4d98363173cb14bdedc92","unresolved":false,"context_lines":[{"line_number":72,"context_line":"operations that currently requires addition of new VIF driver code in an"},{"line_number":73,"context_line":"external script."},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"Moving functionality intrinsically external to Nova into an external script, we"},{"line_number":76,"context_line":"prevent incurring additional technical debt in supporting additional network"},{"line_number":77,"context_line":"implementations. Externalizing these operations also allows quality assurance"},{"line_number":78,"context_line":"activities such as reviews and testing to be the responsibility of the most"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_a00d3990","line":75,"in_reply_to":"fa32b979_5b5d7445","updated":"2015-06-19 07:40:15.000000000","message":"makes sense. Could you please update the spec to mention both points explicitly? Thanks!","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"4ddebb3344f19d264d15908f287f10049b032a26","unresolved":false,"context_lines":[{"line_number":72,"context_line":"operations that currently requires addition of new VIF driver code in an"},{"line_number":73,"context_line":"external script."},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"Moving functionality intrinsically external to Nova into an external script, we"},{"line_number":76,"context_line":"prevent incurring additional technical debt in supporting additional network"},{"line_number":77,"context_line":"implementations. Externalizing these operations also allows quality assurance"},{"line_number":78,"context_line":"activities such as reviews and testing to be the responsibility of the most"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_5b5d7445","line":75,"in_reply_to":"fa32b979_91a1a348","updated":"2015-06-18 10:00:46.000000000","message":"The script should be contain into the vendors neutron driver/plugin repository.\nExample: https://git.openstack.org/cgit/stackforge/networking-6wind/tree/networking_6wind/plug_scripts/vif_ovs_fp_plug.py\n\nAny type of scripts can be supported. Beagle has tested the vif script against a shell script. And me with a python one.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"c9ea3143c98fdde256dc468b3311146d471894d9","unresolved":false,"context_lines":[{"line_number":83,"context_line":"This change introduces a new VIF field, VIF_PLUGIN_SCRIPT. This field will be"},{"line_number":84,"context_line":"populated by Neutron and refer to a back-end specific script that will be"},{"line_number":85,"context_line":"called from the virtualization driver when the system is ready to connect the"},{"line_number":86,"context_line":"guest to a network."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Note: the script is *not* a mechanism for altering the guest XML or the"},{"line_number":89,"context_line":"behavior already implemented in Nova. It may only result in the execution of"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_86f52a58","line":86,"updated":"2015-06-16 23:12:26.000000000","message":"I understand what you are suggesting here, but it just seems relatively weird that an API call returns the path to an executable in its response.\n\nIs it possible to consider an option where neutron returns a string uniquely identifying the driver, and then nova picks the actual executable associated with it?","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"2ed2b8edb81dc0308fdadb5e293ef6f88602d878","unresolved":false,"context_lines":[{"line_number":83,"context_line":"This change introduces a new VIF field, VIF_PLUGIN_SCRIPT. This field will be"},{"line_number":84,"context_line":"populated by Neutron and refer to a back-end specific script that will be"},{"line_number":85,"context_line":"called from the virtualization driver when the system is ready to connect the"},{"line_number":86,"context_line":"guest to a network."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Note: the script is *not* a mechanism for altering the guest XML or the"},{"line_number":89,"context_line":"behavior already implemented in Nova. It may only result in the execution of"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_ef7cfbb6","line":86,"in_reply_to":"fa32b979_86f52a58","updated":"2015-06-17 09:55:30.000000000","message":"That\u0027s certainly workable too - would merely require us to declare a naming convention for the executable.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e626ef3957d62fcd5fc494a4a90078d1fb2f9ef8","unresolved":false,"context_lines":[{"line_number":83,"context_line":"This change introduces a new VIF field, VIF_PLUGIN_SCRIPT. This field will be"},{"line_number":84,"context_line":"populated by Neutron and refer to a back-end specific script that will be"},{"line_number":85,"context_line":"called from the virtualization driver when the system is ready to connect the"},{"line_number":86,"context_line":"guest to a network."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Note: the script is *not* a mechanism for altering the guest XML or the"},{"line_number":89,"context_line":"behavior already implemented in Nova. It may only result in the execution of"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_7f6ac182","line":86,"in_reply_to":"fa32b979_ef7cfbb6","updated":"2015-06-17 18:19:00.000000000","message":"if we were going down that route we are moving closer to one of the previous proposals which was to return the fully qualified python class name.\n\nin this case i think you would have to do more then have a fixed naming convention by also introduces a config option in nova to define the mapping from the unique id to the executable.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"c9ea3143c98fdde256dc468b3311146d471894d9","unresolved":false,"context_lines":[{"line_number":86,"context_line":"guest to a network."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Note: the script is *not* a mechanism for altering the guest XML or the"},{"line_number":89,"context_line":"behavior already implemented in Nova. It may only result in the execution of"},{"line_number":90,"context_line":"additional external operations needed to support a virtual networking"},{"line_number":91,"context_line":"solution. With the exception of an error code result, scripts do not interfere"},{"line_number":92,"context_line":"with or alter the flow of existing VIF plug operations."}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_869c4a10","line":89,"updated":"2015-06-16 23:12:26.000000000","message":"Question. when this script runs, does the libvirt domain already exist? If yes, how do you ensure this script does not fiddle with the domain","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"266ddd93e9d687d34e60fbdb5998d8d53697b113","unresolved":false,"context_lines":[{"line_number":86,"context_line":"guest to a network."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Note: the script is *not* a mechanism for altering the guest XML or the"},{"line_number":89,"context_line":"behavior already implemented in Nova. It may only result in the execution of"},{"line_number":90,"context_line":"additional external operations needed to support a virtual networking"},{"line_number":91,"context_line":"solution. With the exception of an error code result, scripts do not interfere"},{"line_number":92,"context_line":"with or alter the flow of existing VIF plug operations."}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_2dc16454","line":89,"in_reply_to":"fa32b979_6f478b77","updated":"2015-06-17 23:11:17.000000000","message":"thanks this makes sense. I was a bit rusty on the nova boot process.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"2ed2b8edb81dc0308fdadb5e293ef6f88602d878","unresolved":false,"context_lines":[{"line_number":86,"context_line":"guest to a network."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"Note: the script is *not* a mechanism for altering the guest XML or the"},{"line_number":89,"context_line":"behavior already implemented in Nova. It may only result in the execution of"},{"line_number":90,"context_line":"additional external operations needed to support a virtual networking"},{"line_number":91,"context_line":"solution. With the exception of an error code result, scripts do not interfere"},{"line_number":92,"context_line":"with or alter the flow of existing VIF plug operations."}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_6f478b77","line":89,"in_reply_to":"fa32b979_869c4a10","updated":"2015-06-17 09:55:30.000000000","message":"It depends. When VIF plugging during initial guest startup, no libvirt domain will exist. When VIF plugging during VIF hotplug, the domain will obviously exist.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":11307,"name":"Andreas Scheuring","email":"andreas.scheuring@de.ibm.com","username":"uniqueName"},"change_message_id":"eb9d182cf1beead9bfd39e90d2b6c79d4816298e","unresolved":false,"context_lines":[{"line_number":91,"context_line":"solution. With the exception of an error code result, scripts do not interfere"},{"line_number":92,"context_line":"with or alter the flow of existing VIF plug operations."},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"The VIF_PLUGIN_SCRIPT adds operations to existing plug and unplug operations."},{"line_number":95,"context_line":"If it is not specified, the existing plug and unplug behavior is unaffected."},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"Nova shall be modified to call the script and interpret success or failure"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_74408d07","line":94,"updated":"2015-06-18 08:19:49.000000000","message":"due to the discussion below: \n\ns/adds operations to/overrides\n\n(or something similar)","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"c9ea3143c98fdde256dc468b3311146d471894d9","unresolved":false,"context_lines":[{"line_number":92,"context_line":"with or alter the flow of existing VIF plug operations."},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"The VIF_PLUGIN_SCRIPT adds operations to existing plug and unplug operations."},{"line_number":95,"context_line":"If it is not specified, the existing plug and unplug behavior is unaffected."},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"Nova shall be modified to call the script and interpret success or failure"},{"line_number":98,"context_line":"based on standard error level codes. Nova shall provide additional information"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_c68ed2cf","line":95,"updated":"2015-06-16 23:12:26.000000000","message":"Is the script executed before or after the actual plug?\nMy apologies if you have stated this somewhere else and I missed it.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":11307,"name":"Andreas Scheuring","email":"andreas.scheuring@de.ibm.com","username":"uniqueName"},"change_message_id":"eb9d182cf1beead9bfd39e90d2b6c79d4816298e","unresolved":false,"context_lines":[{"line_number":92,"context_line":"with or alter the flow of existing VIF plug operations."},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"The VIF_PLUGIN_SCRIPT adds operations to existing plug and unplug operations."},{"line_number":95,"context_line":"If it is not specified, the existing plug and unplug behavior is unaffected."},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"Nova shall be modified to call the script and interpret success or failure"},{"line_number":98,"context_line":"based on standard error level codes. Nova shall provide additional information"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_74e72dac","line":95,"in_reply_to":"fa32b979_8d975047","updated":"2015-06-18 08:19:49.000000000","message":"Salvatore, I don\u0027t get this. At the moment I only see one vif driver for libvirt, which is the LibvirtGenericVIFDriver. Are you proposing to have vendor specific vif drivers besides the Generic vif driver instead of the plug-script feature for the generic vif driver described in this spec?","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"2ed2b8edb81dc0308fdadb5e293ef6f88602d878","unresolved":false,"context_lines":[{"line_number":92,"context_line":"with or alter the flow of existing VIF plug operations."},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"The VIF_PLUGIN_SCRIPT adds operations to existing plug and unplug operations."},{"line_number":95,"context_line":"If it is not specified, the existing plug and unplug behavior is unaffected."},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"Nova shall be modified to call the script and interpret success or failure"},{"line_number":98,"context_line":"based on standard error level codes. Nova shall provide additional information"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_ef2abb8f","line":95,"in_reply_to":"fa32b979_c68ed2cf","updated":"2015-06-17 09:55:30.000000000","message":"I consider this to be instead of the existing plug/unplug impl. If the script is requested, it is run, if it is not requested, then the current plug/unplug implis run. The goal is back compat with existing Neutron/Nova behaviour. Eventually the existing plug/unplug impls will be able to be removed.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"266ddd93e9d687d34e60fbdb5998d8d53697b113","unresolved":false,"context_lines":[{"line_number":92,"context_line":"with or alter the flow of existing VIF plug operations."},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"The VIF_PLUGIN_SCRIPT adds operations to existing plug and unplug operations."},{"line_number":95,"context_line":"If it is not specified, the existing plug and unplug behavior is unaffected."},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"Nova shall be modified to call the script and interpret success or failure"},{"line_number":98,"context_line":"based on standard error level codes. Nova shall provide additional information"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_8d975047","line":95,"in_reply_to":"fa32b979_ef2abb8f","updated":"2015-06-17 23:11:17.000000000","message":"cool. So it is an alternative to the generic plug/unplug driver. It does not complement, but replace such operations.\n\nHowever this kind of reinforces however my thought that this spec is actually claiming that we should consider having again multiple VIF drivers for libvirt, with the generic driver being obviously the one used in the majority of cases.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":11307,"name":"Andreas Scheuring","email":"andreas.scheuring@de.ibm.com","username":"uniqueName"},"change_message_id":"eb9d182cf1beead9bfd39e90d2b6c79d4816298e","unresolved":false,"context_lines":[{"line_number":96,"context_line":""},{"line_number":97,"context_line":"Nova shall be modified to call the script and interpret success or failure"},{"line_number":98,"context_line":"based on standard error level codes. Nova shall provide additional information"},{"line_number":99,"context_line":"to the script by populating environment variables."},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"The script is called by nova in the following way:"},{"line_number":102,"context_line":"ENVIRONMEMT_VARIABLES  plug_script [plug|unplug]"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_47dcb972","line":99,"updated":"2015-06-18 08:19:49.000000000","message":"This works as executing the plug-script is done in a child-process? I\u0027m not an expert when it comes to the scope of environment variables, just wanted to make sure it has been considered!","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":11307,"name":"Andreas Scheuring","email":"andreas.scheuring@de.ibm.com","username":"uniqueName"},"change_message_id":"eb9d182cf1beead9bfd39e90d2b6c79d4816298e","unresolved":false,"context_lines":[{"line_number":104,"context_line":"The ENVIRONMENT_VARIABLES are generated from the data members of the relevant"},{"line_number":105,"context_line":"VIF object and follow the general format:"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"\"VIF_%s\u003d%s\" % (attribute_name.upper(), str(attribute.value))"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"If an attribute value is a nested dictionary, it is flattened with the prefix"},{"line_number":110,"context_line":"formed of a concatenation of the initial prefix, \"VIF\\_\", and the attribute"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_a7bd85d9","line":107,"updated":"2015-06-18 08:19:49.000000000","message":"This name is very generic...Can this variable be overiden by some other parallel vif plug action? Or is this being prohibited, as parallel plug operation would run in different threads which own its own variables?","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"4ddebb3344f19d264d15908f287f10049b032a26","unresolved":false,"context_lines":[{"line_number":104,"context_line":"The ENVIRONMENT_VARIABLES are generated from the data members of the relevant"},{"line_number":105,"context_line":"VIF object and follow the general format:"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"\"VIF_%s\u003d%s\" % (attribute_name.upper(), str(attribute.value))"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"If an attribute value is a nested dictionary, it is flattened with the prefix"},{"line_number":110,"context_line":"formed of a concatenation of the initial prefix, \"VIF\\_\", and the attribute"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_bba4c0b9","line":107,"in_reply_to":"fa32b979_a7bd85d9","updated":"2015-06-18 10:00:46.000000000","message":"It will run in a new process. The processutils.execute from oslo_concurrency use the subprocess module to spawn a new processes to execute the script.\n\nSee: https://docs.python.org/2/library/subprocess.html","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"c9ea3143c98fdde256dc468b3311146d471894d9","unresolved":false,"context_lines":[{"line_number":114,"context_line":""},{"line_number":115,"context_line":"becomes"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"VIF_DETAILS_OVS_HYBRID_PLUG\u003d\u0027True\u0027"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"This formatting allows for arbitrary nesting with each level following the same"},{"line_number":120,"context_line":"general pattern."}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_0677dad3","line":117,"updated":"2015-06-16 23:12:26.000000000","message":"and then it will be recompose back into..\n\nvif[\u0027details\u0027][\u0027ovs\u0027][\u0027hybrid\u0027][\u0027plug\u0027] ;)\n\nAlso, shouldn\u0027t you be case-sensitive in this process, which at first glance sounds super fiddly and error-prone? I\u0027d just pump json to the script through a pipe, but I\u0027m thinkin aloud","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"d1307ba4a3676527dbd98fc6c3faad3b4ca46167","unresolved":false,"context_lines":[{"line_number":114,"context_line":""},{"line_number":115,"context_line":"becomes"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"VIF_DETAILS_OVS_HYBRID_PLUG\u003d\u0027True\u0027"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"This formatting allows for arbitrary nesting with each level following the same"},{"line_number":120,"context_line":"general pattern."}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_49e06e96","line":117,"in_reply_to":"fa32b979_0677dad3","updated":"2015-06-17 17:01:39.000000000","message":"I was trying to avoid assumptions about the implementation of the script. e.g. maybe not python, maybe parsing json is a pain. I don\u0027t consider the environment mapping to be a true serialization protocol so bidirectional equivalence wasn\u0027t a requirement either. Do we feel this is necessary?","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"266ddd93e9d687d34e60fbdb5998d8d53697b113","unresolved":false,"context_lines":[{"line_number":114,"context_line":""},{"line_number":115,"context_line":"becomes"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"VIF_DETAILS_OVS_HYBRID_PLUG\u003d\u0027True\u0027"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"This formatting allows for arbitrary nesting with each level following the same"},{"line_number":120,"context_line":"general pattern."}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_ade6d412","line":117,"in_reply_to":"fa32b979_49e06e96","updated":"2015-06-17 23:11:17.000000000","message":"Your point makes sense as well, and the general guideline here should be to not get into too much complication. So, if we assume the script can be anything, let\u0027s use environment variables, but plain variables, without thinking about strategies for converting dicts into env variables.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"4ddebb3344f19d264d15908f287f10049b032a26","unresolved":false,"context_lines":[{"line_number":114,"context_line":""},{"line_number":115,"context_line":"becomes"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"VIF_DETAILS_OVS_HYBRID_PLUG\u003d\u0027True\u0027"},{"line_number":118,"context_line":""},{"line_number":119,"context_line":"This formatting allows for arbitrary nesting with each level following the same"},{"line_number":120,"context_line":"general pattern."}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_bb2e00f9","line":117,"in_reply_to":"fa32b979_ade6d412","updated":"2015-06-18 10:00:46.000000000","message":"In fact, the nova patch already converts dict into env variable. See create_vif_plug_env(instance, vif) function in https://review.openstack.org/#/c/162470/9/nova/virt/netutils.py line 55.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":11307,"name":"Andreas Scheuring","email":"andreas.scheuring@de.ibm.com","username":"uniqueName"},"change_message_id":"eb9d182cf1beead9bfd39e90d2b6c79d4816298e","unresolved":false,"context_lines":[{"line_number":132,"context_line":"contents of stderr will be logged if debug logging is enabled."},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"Plug script filenames must begin with the prefix \u0027vif-plugin-\u0027 and contain no"},{"line_number":135,"context_line":"relative or absolute path elements. VIF_PLUGIN_SCRIPT values that do not begin"},{"line_number":136,"context_line":"with the prefix will result in an error. The script must be installed in a"},{"line_number":137,"context_line":"directory included in the PATH environment variable and standard path search"},{"line_number":138,"context_line":"rules apply."}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_949bd165","line":135,"updated":"2015-06-18 08:19:49.000000000","message":"what does that mean? The filename is not allowed to contain a \"/\" or a \".\"? Or does that mean that the file is not allowed to be softlink? Or something else?","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"4ddebb3344f19d264d15908f287f10049b032a26","unresolved":false,"context_lines":[{"line_number":132,"context_line":"contents of stderr will be logged if debug logging is enabled."},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"Plug script filenames must begin with the prefix \u0027vif-plugin-\u0027 and contain no"},{"line_number":135,"context_line":"relative or absolute path elements. VIF_PLUGIN_SCRIPT values that do not begin"},{"line_number":136,"context_line":"with the prefix will result in an error. The script must be installed in a"},{"line_number":137,"context_line":"directory included in the PATH environment variable and standard path search"},{"line_number":138,"context_line":"rules apply."}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_a559fd36","line":135,"in_reply_to":"fa32b979_949bd165","updated":"2015-06-18 10:00:46.000000000","message":"It means you should provide the name of the scripts (i.e. vif-plugin-mymechanism driver) and not a (relative or absolute) path (i.e. ../path/vif-plugin-mymechanism_driver or /usr/bin/vif-plugin-mymechanism_driver).","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":11307,"name":"Andreas Scheuring","email":"andreas.scheuring@de.ibm.com","username":"uniqueName"},"change_message_id":"b5cbc5165fbf6bf658f4d98363173cb14bdedc92","unresolved":false,"context_lines":[{"line_number":132,"context_line":"contents of stderr will be logged if debug logging is enabled."},{"line_number":133,"context_line":""},{"line_number":134,"context_line":"Plug script filenames must begin with the prefix \u0027vif-plugin-\u0027 and contain no"},{"line_number":135,"context_line":"relative or absolute path elements. VIF_PLUGIN_SCRIPT values that do not begin"},{"line_number":136,"context_line":"with the prefix will result in an error. The script must be installed in a"},{"line_number":137,"context_line":"directory included in the PATH environment variable and standard path search"},{"line_number":138,"context_line":"rules apply."}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_e04fc18f","line":135,"in_reply_to":"fa32b979_a559fd36","updated":"2015-06-19 07:40:15.000000000","message":"OK, so the second part of the first sentence applies to the VIF_PLUGIN_SCRIPT value and not to the filename. Could you please restructure the sentence a bit to make that clear?","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"c9ea3143c98fdde256dc468b3311146d471894d9","unresolved":false,"context_lines":[{"line_number":145,"context_line":"party vendors could extend the current VIF type support through subclassing or"},{"line_number":146,"context_line":"similar programmatic approaches. In practice, this is fragile as small changes"},{"line_number":147,"context_line":"to the base classes within Nova present a significant regression risk to the"},{"line_number":148,"context_line":"derived types."},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"An example of this is the vif_plug mechanism:"},{"line_number":151,"context_line":"See: https://blueprints.launchpad.net/nova/+spec/libvirt-vif-driver-plugin"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_2610de58","line":148,"updated":"2015-06-16 23:12:26.000000000","message":"Isn\u0027t the vif plugin script also dependent on the data produced by the vif plugging process - thus causing the same kind of issue? Or does the \"strongly typed\" interface you are proposing here address this problem?","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"change_message_id":"266ddd93e9d687d34e60fbdb5998d8d53697b113","unresolved":false,"context_lines":[{"line_number":145,"context_line":"party vendors could extend the current VIF type support through subclassing or"},{"line_number":146,"context_line":"similar programmatic approaches. In practice, this is fragile as small changes"},{"line_number":147,"context_line":"to the base classes within Nova present a significant regression risk to the"},{"line_number":148,"context_line":"derived types."},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"An example of this is the vif_plug mechanism:"},{"line_number":151,"context_line":"See: https://blueprints.launchpad.net/nova/+spec/libvirt-vif-driver-plugin"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_ad7b3427","line":148,"in_reply_to":"fa32b979_04c1eb53","updated":"2015-06-17 23:11:17.000000000","message":"\"may\" as in \"it could\" or as \"it should\"?\nI am asking because in a previous comment from Daniel I got the feeling the script would be usually executed before libvirt instructs the hypervisor to start the instance.\n\nBut on the other hand the latter part of your comment confirms the other comment from Daniel about the script being a replacement and not a complement to the current plug operation","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"d1307ba4a3676527dbd98fc6c3faad3b4ca46167","unresolved":false,"context_lines":[{"line_number":145,"context_line":"party vendors could extend the current VIF type support through subclassing or"},{"line_number":146,"context_line":"similar programmatic approaches. In practice, this is fragile as small changes"},{"line_number":147,"context_line":"to the base classes within Nova present a significant regression risk to the"},{"line_number":148,"context_line":"derived types."},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"An example of this is the vif_plug mechanism:"},{"line_number":151,"context_line":"See: https://blueprints.launchpad.net/nova/+spec/libvirt-vif-driver-plugin"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_04c1eb53","line":148,"in_reply_to":"fa32b979_2610de58","updated":"2015-06-17 17:01:39.000000000","message":"The VIF plugin script may be dependent on results of the hypervisor starting the instance. Considering the types of operations that would appear in the script would be similar to the ones that are currently called when \"plug\" is called (e.g. plug_mlnx_direct or plug_ovs_hybrid_bridge), I would expect this to be workable.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":8190,"name":"Drew Thorstensen (thorst)","email":"thorst@us.ibm.com","username":"thorst"},"change_message_id":"4b7956baff26942e9888e241ccae2e2026ea5084","unresolved":false,"context_lines":[{"line_number":177,"context_line":"---------------"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"The VIF_PLUGIN_SCRIPT is encapsulated in the Neutron port details and does not"},{"line_number":180,"context_line":"require a change to the Neutron API from that perspective."},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"Security impact"},{"line_number":183,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_aae317d8","line":180,"updated":"2015-06-18 00:49:59.000000000","message":"Could you elaborate which field from the Neutron Port API would be used to derive the script?\n\nIts not clear to me which it would be from this specification: https://wiki.openstack.org/wiki/Neutron/APIv2-specification#Port","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":8190,"name":"Drew Thorstensen (thorst)","email":"thorst@us.ibm.com","username":"thorst"},"change_message_id":"e35cb6a000fac7a7f458b0a36a847269c1b6e43b","unresolved":false,"context_lines":[{"line_number":177,"context_line":"---------------"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"The VIF_PLUGIN_SCRIPT is encapsulated in the Neutron port details and does not"},{"line_number":180,"context_line":"require a change to the Neutron API from that perspective."},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"Security impact"},{"line_number":183,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_ec47b475","line":180,"in_reply_to":"fa32b979_2553ad3a","updated":"2015-06-19 12:02:15.000000000","message":"See comment below.  My concern, since this clearly crosses projects to orchestrate, is where a plugin author within Neutron would understand this capability when writing their own plugin.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"4ddebb3344f19d264d15908f287f10049b032a26","unresolved":false,"context_lines":[{"line_number":177,"context_line":"---------------"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"The VIF_PLUGIN_SCRIPT is encapsulated in the Neutron port details and does not"},{"line_number":180,"context_line":"require a change to the Neutron API from that perspective."},{"line_number":181,"context_line":""},{"line_number":182,"context_line":"Security impact"},{"line_number":183,"context_line":"---------------"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_2553ad3a","line":180,"in_reply_to":"fa32b979_aae317d8","updated":"2015-06-18 10:00:46.000000000","message":"The field used in the Neutron Port API is \u0027binding:vif_details\u0027.\n\nSee: http://developer.openstack.org/api-ref-networking-v2-ext.html#listPorts\n\nThe mechanism driver needs to add the script name into this vif_details.\nExample: https://git.openstack.org/cgit/stackforge/networking-6wind/tree/networking_6wind/ml2_drivers/mech_ovs_fp.py#n45","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":8190,"name":"Drew Thorstensen (thorst)","email":"thorst@us.ibm.com","username":"thorst"},"change_message_id":"4b7956baff26942e9888e241ccae2e2026ea5084","unresolved":false,"context_lines":[{"line_number":184,"context_line":""},{"line_number":185,"context_line":"The VIF_PLUGIN_SCRIPT validation rules prevent arbitrary commands on the system"},{"line_number":186,"context_line":"from being executed. It is the deployer\u0027s responsibility to prevent scripts"},{"line_number":187,"context_line":"accessing dangerous programs or resources."},{"line_number":188,"context_line":""},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"Notifications impact"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_4aafeb7c","line":187,"updated":"2015-06-18 00:49:59.000000000","message":"Is the script invoked from the nova process itself?  If so, I\u0027m wondering if changes in that script could impact the rootwrap configuration for the nova-compute process?","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"4ddebb3344f19d264d15908f287f10049b032a26","unresolved":false,"context_lines":[{"line_number":184,"context_line":""},{"line_number":185,"context_line":"The VIF_PLUGIN_SCRIPT validation rules prevent arbitrary commands on the system"},{"line_number":186,"context_line":"from being executed. It is the deployer\u0027s responsibility to prevent scripts"},{"line_number":187,"context_line":"accessing dangerous programs or resources."},{"line_number":188,"context_line":""},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"Notifications impact"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_c5076180","line":187,"in_reply_to":"fa32b979_4aafeb7c","updated":"2015-06-18 10:00:46.000000000","message":"The script is executed by nova process. But it\u0027s not executed with root right, so without nova rootwrap.\n\nThe script can still call some commands with nova rootwrap or its own rootwrap.\n\nFor more information,  please see the discussion I have with Beagle on the nova patch: https://review.openstack.org/#/c/162470/2/nova/virt/netutils.py line 101","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":8190,"name":"Drew Thorstensen (thorst)","email":"thorst@us.ibm.com","username":"thorst"},"change_message_id":"e35cb6a000fac7a7f458b0a36a847269c1b6e43b","unresolved":false,"context_lines":[{"line_number":184,"context_line":""},{"line_number":185,"context_line":"The VIF_PLUGIN_SCRIPT validation rules prevent arbitrary commands on the system"},{"line_number":186,"context_line":"from being executed. It is the deployer\u0027s responsibility to prevent scripts"},{"line_number":187,"context_line":"accessing dangerous programs or resources."},{"line_number":188,"context_line":""},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"Notifications impact"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_bf079017","line":187,"in_reply_to":"fa32b979_c5076180","updated":"2015-06-19 12:02:15.000000000","message":"Could you add some information about that in this section?  I\u0027m not sure what the validation rules are noted above?\n\nBut nonetheless, that\u0027s good to hear.  If a new rootwrap is needed though, then the compute process will need to be booted with another rootwrap file...so that could cause cross project concerns with Chef/Puppet deployment...but outside the scope of this bp.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":11307,"name":"Andreas Scheuring","email":"andreas.scheuring@de.ibm.com","username":"uniqueName"},"change_message_id":"eb9d182cf1beead9bfd39e90d2b6c79d4816298e","unresolved":false,"context_lines":[{"line_number":241,"context_line":"  * execution of script (logging of output when verbose and debug are True)"},{"line_number":242,"context_line":"  * interpretation of error codes, potentially converting a base set to Nova"},{"line_number":243,"context_line":"  exceptions"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"Dependencies"},{"line_number":246,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":247,"context_line":""}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_473919d6","line":244,"updated":"2015-06-18 08:19:49.000000000","message":"In \"Proposed Change\" you said: \"A goal of this work is that to get to a place where we \thave 100% coverage of the different libvirt XML configurations in Nova.\"\n\nDoes this spec also cover a consolidation of the vif-xml (get_config_) methods or not? At least the text indicates something like that. Or should this be a separate effort?","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"4ddebb3344f19d264d15908f287f10049b032a26","unresolved":false,"context_lines":[{"line_number":241,"context_line":"  * execution of script (logging of output when verbose and debug are True)"},{"line_number":242,"context_line":"  * interpretation of error codes, potentially converting a base set to Nova"},{"line_number":243,"context_line":"  exceptions"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"Dependencies"},{"line_number":246,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":247,"context_line":""}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_20539b3a","line":244,"in_reply_to":"fa32b979_473919d6","updated":"2015-06-18 10:00:46.000000000","message":"First, the vif_script feature will be used with new bare \"VIF_TYPE\" like VIF_VHOSTUSER and VIF_TAP.\n\nThen, we want to generalize this usage for all mechanism drivers. For that,\nthe different VIF_TYPE need to be progressively consolidate.\n\nThus, consolidate the VIF_TYPE in Nova is a separate effort. I think the final goal is to remove all the plug/unplug code from Nova. There are already some discussion regarding deprecated the plug/unplug code in Nova. See: https://review.openstack.org/#/c/162468/16/specs/liberty/approved/vif-plug-script.rst line 75","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":8190,"name":"Drew Thorstensen (thorst)","email":"thorst@us.ibm.com","username":"thorst"},"change_message_id":"4b7956baff26942e9888e241ccae2e2026ea5084","unresolved":false,"context_lines":[{"line_number":245,"context_line":"Dependencies"},{"line_number":246,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"Neutron requires changes to supply the values for the SCRIPT."},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"Testing"},{"line_number":251,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_4a812bfa","line":248,"updated":"2015-06-18 00:49:59.000000000","message":"Is there a corresponding Neutron blueprint for this that could be referenced?  I\u0027d be interested to see what specific components within Neutron could fill in that field (I\u0027m thinking it might be the ML2 Mechanism Driver) but that\u0027s probably a detail for a corresponding Neutron blueprint.\n\nIt would also be interesting to see the dynamics of this with the ongoing decomposition in Neutron.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"4ddebb3344f19d264d15908f287f10049b032a26","unresolved":false,"context_lines":[{"line_number":245,"context_line":"Dependencies"},{"line_number":246,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"Neutron requires changes to supply the values for the SCRIPT."},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"Testing"},{"line_number":251,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_db7ae487","line":248,"in_reply_to":"fa32b979_4a812bfa","updated":"2015-06-18 10:00:46.000000000","message":"As we don\u0027t need to patch Neutron to support this feature, there are no Neutron blueprint.\n\nBut you can have a look to the ovs-fp mechanism driver (under development) using this feature: https://git.openstack.org/cgit/stackforge/networking-6wind/tree/networking_6wind/ml2_drivers/mech_ovs_fp.py#n45","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":8190,"name":"Drew Thorstensen (thorst)","email":"thorst@us.ibm.com","username":"thorst"},"change_message_id":"e35cb6a000fac7a7f458b0a36a847269c1b6e43b","unresolved":false,"context_lines":[{"line_number":245,"context_line":"Dependencies"},{"line_number":246,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"Neutron requires changes to supply the values for the SCRIPT."},{"line_number":249,"context_line":""},{"line_number":250,"context_line":"Testing"},{"line_number":251,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_cc9d100c","line":248,"in_reply_to":"fa32b979_db7ae487","updated":"2015-06-19 12:02:15.000000000","message":"It seems a bit odd that there isn\u0027t something corresponding in the neutron side (except in a decomposed plugin, not part of the core) that utilizes the feature.  Will there be documentation for plugin authors, or mechanism driver/agent authors to know about this feature?\n\nAgree this may not be a dependency, but probably a reference?","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":8190,"name":"Drew Thorstensen (thorst)","email":"thorst@us.ibm.com","username":"thorst"},"change_message_id":"e35cb6a000fac7a7f458b0a36a847269c1b6e43b","unresolved":false,"context_lines":[{"line_number":262,"context_line":"Documentation Impact"},{"line_number":263,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"The VIF_PLUG_SCRIPT feature and the environment variable mapping requires"},{"line_number":266,"context_line":"documenting as well as a description of how the script is meant to be used. The"},{"line_number":267,"context_line":"documentation should outline the naming rules and suggest how scripts should be"},{"line_number":268,"context_line":"deployed."}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_6ca64455","line":265,"updated":"2015-06-19 12:02:15.000000000","message":"Will this be in the nova wiki or the neutron wiki?","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"ca83ded5a34bda57d02974592ac41a9e7d096064","unresolved":false,"context_lines":[{"line_number":262,"context_line":"Documentation Impact"},{"line_number":263,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":264,"context_line":""},{"line_number":265,"context_line":"The VIF_PLUG_SCRIPT feature and the environment variable mapping requires"},{"line_number":266,"context_line":"documenting as well as a description of how the script is meant to be used. The"},{"line_number":267,"context_line":"documentation should outline the naming rules and suggest how scripts should be"},{"line_number":268,"context_line":"deployed."}],"source_content_type":"text/x-rst","patch_set":17,"id":"fa32b979_135b1030","line":265,"in_reply_to":"fa32b979_6ca64455","updated":"2015-06-19 16:19:03.000000000","message":"It would be a nova wiki as it is essentially a hook style API in Nova.","commit_id":"d2a1b75647219ce9d77a47c309f7f560ee845354"}]}