)]}'
{"nova/network/model.py":[{"author":{"_account_id":6598,"name":"Berezovsky Irena","email":"irenab.dev@gmail.com","username":"irenab"},"change_message_id":"91243ad6b3cc415f53feca8ee97b1136bf0e50de","unresolved":false,"context_lines":[{"line_number":82,"context_line":"VIF_MODEL_SPAPR_VLAN \u003d \u0027spapr-vlan\u0027"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"# The script to be run when plugging a VIF."},{"line_number":85,"context_line":"VIF_PLUGIN_SCRIPT \u003d \u0027vif_plugin_script\u0027"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"# Constant for max length of network interface names"},{"line_number":88,"context_line":"# eg \u0027bridge\u0027 in the Network class or \u0027devname\u0027 in"}],"source_content_type":"text/x-python","patch_set":9,"id":"7a016987_f36fda5d","line":85,"updated":"2015-05-31 08:06:44.000000000","message":"I would suggest to group it together with VIF_DETAILS defs in line 55-56","commit_id":"487acfb343cda855169d37be126598459b36ec84"}],"nova/objects/network_info.py":[{"author":{"_account_id":6598,"name":"Berezovsky Irena","email":"irenab.dev@gmail.com","username":"irenab"},"change_message_id":"91243ad6b3cc415f53feca8ee97b1136bf0e50de","unresolved":false,"context_lines":[{"line_number":469,"context_line":"        return [floating_ip for fixed_ip in self.fixed_ips()"},{"line_number":470,"context_line":"                for floating_ip in fixed_ip.floating_ips]"},{"line_number":471,"context_line":""},{"line_number":472,"context_line":"    def get_plugin_script(self):"},{"line_number":473,"context_line":"        if not self.details:"},{"line_number":474,"context_line":"            return None"},{"line_number":475,"context_line":"        return self.details.get(model.VIF_PLUGIN_SCRIPT)"}],"source_content_type":"text/x-python","patch_set":9,"id":"7a016987_d3df3e0b","line":472,"updated":"2015-05-31 08:06:44.000000000","message":"I think the following will do the job:\nif self.details:\n   return self.details.get(model.VIF_PLUGIN_SCRIPT)","commit_id":"487acfb343cda855169d37be126598459b36ec84"}],"nova/virt/libvirt/vif.py":[{"author":{"_account_id":13734,"name":"Nell Jerram","email":"nell@tigera.io","username":"neiljerram"},"change_message_id":"a71f1557fab6301f26a643ba9bd158b643f4535a","unresolved":false,"context_lines":[{"line_number":605,"context_line":"        plugin_script \u003d vif.get_plugin_script()"},{"line_number":606,"context_line":"        if plugin_script:"},{"line_number":607,"context_line":"            netutils.run_plug_script(instance, vif, plugin_script, \u0027plug\u0027)"},{"line_number":608,"context_line":"        else:"},{"line_number":609,"context_line":"            vif_slug \u003d self._normalize_vif_type(vif_type)"},{"line_number":610,"context_line":"            func \u003d getattr(self, \u0027plug_%s\u0027 % vif_slug, None)"},{"line_number":611,"context_line":"            if not func:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3ab451fb_587e70e5","line":608,"updated":"2015-04-17 12:16:01.000000000","message":"Interesting.  I hadn\u0027t previously appreciated that the plugin script will _replace_ the hardcoded plug_%s code.  I thought it would augment it.\n\nI had also thought that - in the libvirt case - we might use the \u003cscript\u003e XML element.  But on closer examination it seems like this might only be available for \u003cinterface type\u003d\u0027ethernet\u0027 ...\u003e, and also I\u0027m not sure if that script gets run when a vif is unplugged - so running the script from Nova, as you\u0027ve proposed, seems like the right thing.","commit_id":"42c359088f6c97972dd5ee8d897ac9dd00578246"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"a2f5e461a13b8b1946028a47f55bbb2fda39915b","unresolved":false,"context_lines":[{"line_number":605,"context_line":"        plugin_script \u003d vif.get_plugin_script()"},{"line_number":606,"context_line":"        if plugin_script:"},{"line_number":607,"context_line":"            netutils.run_plug_script(instance, vif, plugin_script, \u0027plug\u0027)"},{"line_number":608,"context_line":"        else:"},{"line_number":609,"context_line":"            vif_slug \u003d self._normalize_vif_type(vif_type)"},{"line_number":610,"context_line":"            func \u003d getattr(self, \u0027plug_%s\u0027 % vif_slug, None)"},{"line_number":611,"context_line":"            if not func:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3ab451fb_2e5a3e84","line":608,"in_reply_to":"3ab451fb_587e70e5","updated":"2015-04-17 12:54:06.000000000","message":"Actually, you were correct that it is meant to augment it. I just hadn\u0027t updated the patch yet. Coming soon.","commit_id":"42c359088f6c97972dd5ee8d897ac9dd00578246"},{"author":{"_account_id":13734,"name":"Nell Jerram","email":"nell@tigera.io","username":"neiljerram"},"change_message_id":"22b38179eece1ef58f68d51d03a75ef523a87410","unresolved":false,"context_lines":[{"line_number":612,"context_line":""},{"line_number":613,"context_line":"        plugin_script \u003d vif.get_plugin_script()"},{"line_number":614,"context_line":"        if plugin_script:"},{"line_number":615,"context_line":"            netutils.run_plug_script(instance, vif, plugin_script, \u0027plug\u0027)"},{"line_number":616,"context_line":""},{"line_number":617,"context_line":"    def unplug_bridge(self, instance, vif):"},{"line_number":618,"context_line":"        \"\"\"No manual unplugging required.\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"9a0c5dc1_57b4f8f0","line":615,"updated":"2015-05-12 17:22:43.000000000","message":"Since my previous review - in which I queried whether the plug script should augment or replace the built-in plugin code - you\u0027ve changed this so that it is an augmenting behaviour.\n\nI\u0027ve just reviewed the particular case for which my project might want to use vif_plug_script, and actually I think augmenting wouldn\u0027t work for that case - so after all it might be desirable to provide a replacing option as well.  On the other hand, it might be that my requirement could be met by other changes, such as your VIF type refactoring, or that it should be a new change that is independent of all your work.\n\nTo be precise, my requirement is for the TAP interface to be created with a fixed MAC address, i.e.\n\n    linux_net.create_tap_dev(dev, mac_address\u003d\u002700:61:fe:ed:ca:fe\u0027)\n\ninstead of\n\n    linux_net.create_tap_dev(dev)\n\nbecause with Calico (routed) networking this aids VM mobility.\n\nWhen a mac_address is passed to create_tap_dev, the key thing is that it installs that MAC address _before_ setting the interface up.  Whereas if I tried to do similar with an augmenting plug script, the closest I could get would be:\n\n- TAP interface is created and set up with its Linux-generated MAC address\n- plug script sets the interface down, installs the desired MAC address, and sets the interface up again.\n\nAnd I suspect that that flapping could cause problems for other components that are watching out for new TAP interfaces appearing...\n\nWhat do you think?\n\n1. Could this justify providing a replacing option for vif-plugin-script, instead of or as well as augmenting?\n\n2. Alternatively, will the ability for Neutron (i.e. our mechanism driver) to specify the MAC address fall naturally out of your VIF type refactoring work?\n\n3. Or, should I plan to meet my need with some entirely independent effort?\n\n(I\u0027m thinking probably (2).)","commit_id":"25c9ac0f7e11bf34550cf5f0c71895a7d0463f95"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1ead60d9e35f5126f49066a9af252069e96adb86","unresolved":false,"context_lines":[{"line_number":612,"context_line":""},{"line_number":613,"context_line":"        plugin_script \u003d vif.get_plugin_script()"},{"line_number":614,"context_line":"        if plugin_script:"},{"line_number":615,"context_line":"            netutils.run_plug_script(instance, vif, plugin_script, \u0027plug\u0027)"},{"line_number":616,"context_line":""},{"line_number":617,"context_line":"    def unplug_bridge(self, instance, vif):"},{"line_number":618,"context_line":"        \"\"\"No manual unplugging required.\"\"\""}],"source_content_type":"text/x-python","patch_set":6,"id":"7a016987_12b4a67d","line":615,"in_reply_to":"9a0c5dc1_57b4f8f0","updated":"2015-05-22 08:26:06.000000000","message":"a replacing behavior may be appropriate at the per vif level eg plug_\u003cvif\u003e but im not sure if a blanket replacement in plug is correct.\n\nif a replacing behavior was supported in plug and unplug then if the  vif-plugin-script is not present then the behavior should not be changed by this patch in any way.\n\nif the pre patch semantics are preserved when  vif-plugin-script is absent i think a replace behavior is fine in plug.","commit_id":"25c9ac0f7e11bf34550cf5f0c71895a7d0463f95"}],"nova/virt/netutils.py":[{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"8b26bffc5118aa4ab387bc0dcba70b713df8f55a","unresolved":false,"context_lines":[{"line_number":66,"context_line":"    # present - throws an exception. NOTE(beagles): I\u0027m currently undecided"},{"line_number":67,"context_line":"    # on how that should all fit - right now the decision of whether"},{"line_number":68,"context_line":"    # something is a required value is that I can\u0027t see there being any"},{"line_number":69,"context_line":"    # sensible script processing without it."},{"line_number":70,"context_line":"    env_mappings \u003d ["},{"line_number":71,"context_line":"        (\u0027VIF_ID\u0027, \u0027id\u0027, True),"},{"line_number":72,"context_line":"        (\u0027VIF_ADDRESS\u0027, \u0027address\u0027, False),"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_6b44f41a","line":69,"updated":"2015-03-09 13:23:52.000000000","message":"One of the things that troubles me if the nova.network.model.VIF class we have. The way we introduced VIF types to that resulted in one class which has the union of all attributes for all VIF types. This makes it rather unclear what attributes are relevant to what VIF type. If we were writing this again today, we\u0027d likely have the VIF class inherit from Nova Object base class and then have subclasses for each VIF type. \n\nNow considering how this relates to the scripts.. I think it is fine to pass VIF type specific attributes down into the script, *provided* we only pass them for the correct VIF type. ie if we have VIF Type\u003d\u003dbridge, then we should not pass the OVS interface id parameter - that should only be passed if the VIF type is \u0027ovs\u0027.\n\nNow, you could have a big dict here, listing the parameters that are required for each VIF type. Or the, bigger effort, approach is to actually turn VIF class into Nova object \u0026 introduce subclasses as I described.  Then, we could have a purely metadata driven approach here, where we just expose all the attributes on the subclass we get. This automatically ensure the script only gets parameters that are relevant to the VIF type.","commit_id":"9f6315c7ba5104693663c122b63f88d959a795d6"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"1f4ca86bf7e247beedf9e146ad8606ea3e57fc66","unresolved":false,"context_lines":[{"line_number":66,"context_line":"    # present - throws an exception. NOTE(beagles): I\u0027m currently undecided"},{"line_number":67,"context_line":"    # on how that should all fit - right now the decision of whether"},{"line_number":68,"context_line":"    # something is a required value is that I can\u0027t see there being any"},{"line_number":69,"context_line":"    # sensible script processing without it."},{"line_number":70,"context_line":"    env_mappings \u003d ["},{"line_number":71,"context_line":"        (\u0027VIF_ID\u0027, \u0027id\u0027, True),"},{"line_number":72,"context_line":"        (\u0027VIF_ADDRESS\u0027, \u0027address\u0027, False),"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_cd4890ce","line":69,"in_reply_to":"9a80dd14_52999d6c","updated":"2015-03-09 15:48:44.000000000","message":"Yup, there are some fields missing - I\u0027ll add shortly. Good idea regarding the OVS_INTERFACEID thing.","commit_id":"9f6315c7ba5104693663c122b63f88d959a795d6"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"6a39731f89cb7fa343d2e2bcf1d5157f66ea3698","unresolved":false,"context_lines":[{"line_number":66,"context_line":"    # present - throws an exception. NOTE(beagles): I\u0027m currently undecided"},{"line_number":67,"context_line":"    # on how that should all fit - right now the decision of whether"},{"line_number":68,"context_line":"    # something is a required value is that I can\u0027t see there being any"},{"line_number":69,"context_line":"    # sensible script processing without it."},{"line_number":70,"context_line":"    env_mappings \u003d ["},{"line_number":71,"context_line":"        (\u0027VIF_ID\u0027, \u0027id\u0027, True),"},{"line_number":72,"context_line":"        (\u0027VIF_ADDRESS\u0027, \u0027address\u0027, False),"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_52999d6c","line":69,"in_reply_to":"9a80dd14_6b44f41a","updated":"2015-03-09 15:33:00.000000000","message":"What about VIF Type: vhost-user or tap ?\n\nFor vhost-user case for dpdk ovs ml2, you need for example the ovs interface id parameter.\n\nFor any new ml2 switch using vhost-user, you could need for example vif bridge name.\n\nHow do you know which information is relevant for any new external ml2 switch using on vhost-user or tap vif type ?\n\nIn 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.","commit_id":"9f6315c7ba5104693663c122b63f88d959a795d6"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"bfda7ae5f0015b57d7ae4550e7eea53865fa7792","unresolved":false,"context_lines":[{"line_number":66,"context_line":"    # present - throws an exception. NOTE(beagles): I\u0027m currently undecided"},{"line_number":67,"context_line":"    # on how that should all fit - right now the decision of whether"},{"line_number":68,"context_line":"    # something is a required value is that I can\u0027t see there being any"},{"line_number":69,"context_line":"    # sensible script processing without it."},{"line_number":70,"context_line":"    env_mappings \u003d ["},{"line_number":71,"context_line":"        (\u0027VIF_ID\u0027, \u0027id\u0027, True),"},{"line_number":72,"context_line":"        (\u0027VIF_ADDRESS\u0027, \u0027address\u0027, False),"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_031f6b5c","line":69,"in_reply_to":"9a80dd14_cd4890ce","updated":"2015-03-17 18:07:52.000000000","message":"It also misses vif[\u0027network\u0027][\u0027bridge\u0027] for ovs, linux bridge (i.e. VIF_BRIDGE_NAME).","commit_id":"9f6315c7ba5104693663c122b63f88d959a795d6"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"bfda7ae5f0015b57d7ae4550e7eea53865fa7792","unresolved":false,"context_lines":[{"line_number":91,"context_line":"    # VIF_DETAIL. I\u0027m going to do the jsonutils thing for now - but maybe"},{"line_number":92,"context_line":"    # that isn\u0027t sufficient either."},{"line_number":93,"context_line":"    for name, value in vif.get(\u0027details\u0027, {}).iteritems():"},{"line_number":94,"context_line":"        result[\u0027%s%s\u0027 % (detail_prefix, name)] \u003d jsonutils.dumps(value)"},{"line_number":95,"context_line":"    return result"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_036dabcc","line":94,"updated":"2015-03-17 18:07:52.000000000","message":"Instead of using jsonutils.dump, using str(value) should be enough ?","commit_id":"9f6315c7ba5104693663c122b63f88d959a795d6"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"6a39731f89cb7fa343d2e2bcf1d5157f66ea3698","unresolved":false,"context_lines":[{"line_number":98,"context_line":"def run_plug_script(instance, vif, scriptpath, command):"},{"line_number":99,"context_line":"    environment_vars \u003d create_vif_plug_env(instance, vif)"},{"line_number":100,"context_line":"    try:"},{"line_number":101,"context_line":"        utils.execute(scriptpath, command, env_variables\u003denvironment_vars)"},{"line_number":102,"context_line":"    except processutils.ProcessExecutionError as e:"},{"line_number":103,"context_line":"        LOG.exception(e)"},{"line_number":104,"context_line":"        error_msg \u003d _(\u0027Failed to {command} VIF with {script} script, \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_12080591","line":101,"updated":"2015-03-09 15:33:00.000000000","message":"The script should be run in root ? (i.e. run_as_root\u003dTrue)\n\nWhat about rootwrap ?","commit_id":"9f6315c7ba5104693663c122b63f88d959a795d6"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"1f4ca86bf7e247beedf9e146ad8606ea3e57fc66","unresolved":false,"context_lines":[{"line_number":98,"context_line":"def run_plug_script(instance, vif, scriptpath, command):"},{"line_number":99,"context_line":"    environment_vars \u003d create_vif_plug_env(instance, vif)"},{"line_number":100,"context_line":"    try:"},{"line_number":101,"context_line":"        utils.execute(scriptpath, command, env_variables\u003denvironment_vars)"},{"line_number":102,"context_line":"    except processutils.ProcessExecutionError as e:"},{"line_number":103,"context_line":"        LOG.exception(e)"},{"line_number":104,"context_line":"        error_msg \u003d _(\u0027Failed to {command} VIF with {script} script, \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_eda714a5","line":101,"in_reply_to":"9a80dd14_12080591","updated":"2015-03-09 15:48:44.000000000","message":"Right. I brought this up in the mailing list thread. I had tried that out, but it raised some questions:\n\n- rootwrap filters would be required for each script type making deployment a bit more \"annoying\"\n- unless the list were restricted in some way, the rootwrap filters will fail of the provided environment variables change in some way, tightly coupling the deployments to an internal implementation detail (although Daniel\u0027s suggestion probably mitigates that)\n- if the user/deployer needs rootwrap, they can always use it from their script. If they don\u0027t then if we put it in the code, they are stuck with all of the headaches of the rootwrap filters regardless.\n\nI can easily change this (had it this way at one point in the git repo), but I\u0027d like everyone to think about alternatives and consequences - so I went with what I figured was the more contentious approach first.","commit_id":"9f6315c7ba5104693663c122b63f88d959a795d6"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"bfda7ae5f0015b57d7ae4550e7eea53865fa7792","unresolved":false,"context_lines":[{"line_number":98,"context_line":"def run_plug_script(instance, vif, scriptpath, command):"},{"line_number":99,"context_line":"    environment_vars \u003d create_vif_plug_env(instance, vif)"},{"line_number":100,"context_line":"    try:"},{"line_number":101,"context_line":"        utils.execute(scriptpath, command, env_variables\u003denvironment_vars)"},{"line_number":102,"context_line":"    except processutils.ProcessExecutionError as e:"},{"line_number":103,"context_line":"        LOG.exception(e)"},{"line_number":104,"context_line":"        error_msg \u003d _(\u0027Failed to {command} VIF with {script} script, \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_23a9cfdf","line":101,"in_reply_to":"9a80dd14_eda714a5","updated":"2015-03-17 18:07:52.000000000","message":"I have done an example of script using its own rootwrap.conf. Please see:\nhttps://github.com/6WIND/nova-vif-plug-example","commit_id":"9f6315c7ba5104693663c122b63f88d959a795d6"},{"author":{"_account_id":13734,"name":"Nell Jerram","email":"nell@tigera.io","username":"neiljerram"},"change_message_id":"a71f1557fab6301f26a643ba9bd158b643f4535a","unresolved":false,"context_lines":[{"line_number":47,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"def _flatten_nested_dict(target_dict, prefix, values):"},{"line_number":51,"context_line":"    for k, v in values.iteritems():"},{"line_number":52,"context_line":"        if isinstance(v, dict):"},{"line_number":53,"context_line":"            nested_prefix \u003d \u0027%s%s_\u0027 % (prefix, k.upper())"}],"source_content_type":"text/x-python","patch_set":4,"id":"3ab451fb_0baad4d4","line":50,"updated":"2015-04-17 12:16:01.000000000","message":"I\u0027d prefer to put this definition inside create_vif_plug_env.","commit_id":"42c359088f6c97972dd5ee8d897ac9dd00578246"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"a2f5e461a13b8b1946028a47f55bbb2fda39915b","unresolved":false,"context_lines":[{"line_number":47,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"def _flatten_nested_dict(target_dict, prefix, values):"},{"line_number":51,"context_line":"    for k, v in values.iteritems():"},{"line_number":52,"context_line":"        if isinstance(v, dict):"},{"line_number":53,"context_line":"            nested_prefix \u003d \u0027%s%s_\u0027 % (prefix, k.upper())"}],"source_content_type":"text/x-python","patch_set":4,"id":"3ab451fb_6ec72613","line":50,"in_reply_to":"3ab451fb_0baad4d4","updated":"2015-04-17 12:54:06.000000000","message":"No objection. Will do.","commit_id":"42c359088f6c97972dd5ee8d897ac9dd00578246"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"00d8d6655276b83ce2083b0c9a8ed748cadddc29","unresolved":false,"context_lines":[{"line_number":73,"context_line":"    return result"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"def run_plug_script(instance, vif, scriptpath, command):"},{"line_number":77,"context_line":"    environment_vars \u003d create_vif_plug_env(instance, vif)"},{"line_number":78,"context_line":"    try:"},{"line_number":79,"context_line":"        utils.execute(scriptpath, command, env_variables\u003denvironment_vars)"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a016987_a2ec552a","line":76,"updated":"2015-05-22 16:53:48.000000000","message":"See my note in your spec about how we should not allow a full script path to be passed form Neutron. We should agree on a fixed directory location to hold all scripts, and then just pass a unqualified filename. This avoids giving Neutron the ability to execute arbitrary  commands on nova hosts.","commit_id":"25c9ac0f7e11bf34550cf5f0c71895a7d0463f95"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"36355067c9dfb52466f60e972ea2a7988a0e1246","unresolved":false,"context_lines":[{"line_number":76,"context_line":"def run_plug_script(instance, vif, scriptpath, command):"},{"line_number":77,"context_line":"    environment_vars \u003d create_vif_plug_env(instance, vif)"},{"line_number":78,"context_line":"    try:"},{"line_number":79,"context_line":"        utils.execute(scriptpath, command, env_variables\u003denvironment_vars)"},{"line_number":80,"context_line":"    except processutils.ProcessExecutionError as e:"},{"line_number":81,"context_line":"        LOG.exception(e)"},{"line_number":82,"context_line":"        error_msg \u003d _(\u0027Failed to {command} VIF with {script} script, \u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"7ad32947_f5376be1","line":79,"updated":"2015-04-29 07:54:58.000000000","message":"With this code, I have the following issue:\n\n  2015-04-28 15:15:31.873 DEBUG oslo_concurrency.processutils ... vif_ovs_plugin plug from (pid\u003d44946) execute /usr/local/lib/python2.7/dist-packages/oslo_concurrency/processutils.py:199\n  2015-04-28 15:15:31.879 DEBUG oslo_concurrency.processutils [...] u\u0027vif_ovs_plugin plug\u0027 failed. Not Retrying. from (pid\u003d44946) execute /usr/local/lib/python2.7/dist-packages/oslo_concurrency/processutils.py:258\n  2015-04-28 15:15:31.880 TRACE nova.compute.manager [instance: 546421f7-636a-4554-8a84-e9403b49934b] OSError: [Errno 2] No such file or directory\n\nMy ovs_vif_plugin script is stored in /usr/local/bin and not in /usr/bin.\n\nWe have this problem because we set specific env_variables when we call utils.execute. By doing that, we unset the PATH environment variable.\n\nTo fix the issue we can add PATH to the environment_var:\n   environment_var[\u0027PATH\u0027] \u003d os.getenv(\u0027PATH\u0027)\n\nWhat do you think ?","commit_id":"25c9ac0f7e11bf34550cf5f0c71895a7d0463f95"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"862fc751c3681e4d46b0b0c9d872b9f146e95bbb","unresolved":false,"context_lines":[{"line_number":76,"context_line":"def run_plug_script(instance, vif, scriptpath, command):"},{"line_number":77,"context_line":"    environment_vars \u003d create_vif_plug_env(instance, vif)"},{"line_number":78,"context_line":"    try:"},{"line_number":79,"context_line":"        utils.execute(scriptpath, command, env_variables\u003denvironment_vars)"},{"line_number":80,"context_line":"    except processutils.ProcessExecutionError as e:"},{"line_number":81,"context_line":"        LOG.exception(e)"},{"line_number":82,"context_line":"        error_msg \u003d _(\u0027Failed to {command} VIF with {script} script, \u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aca2d91_63911247","line":79,"in_reply_to":"1aca2d91_2a4e706f","updated":"2015-05-09 12:21:20.000000000","message":"I don\u0027t see any reasons to only provide the PATH environment variable and not the other ones. So, yes, we should copy all the environment variables here.","commit_id":"25c9ac0f7e11bf34550cf5f0c71895a7d0463f95"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"00d8d6655276b83ce2083b0c9a8ed748cadddc29","unresolved":false,"context_lines":[{"line_number":76,"context_line":"def run_plug_script(instance, vif, scriptpath, command):"},{"line_number":77,"context_line":"    environment_vars \u003d create_vif_plug_env(instance, vif)"},{"line_number":78,"context_line":"    try:"},{"line_number":79,"context_line":"        utils.execute(scriptpath, command, env_variables\u003denvironment_vars)"},{"line_number":80,"context_line":"    except processutils.ProcessExecutionError as e:"},{"line_number":81,"context_line":"        LOG.exception(e)"},{"line_number":82,"context_line":"        error_msg \u003d _(\u0027Failed to {command} VIF with {script} script, \u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a016987_22c865a5","line":79,"in_reply_to":"7ad32947_f5376be1","updated":"2015-05-22 16:53:48.000000000","message":"In fact you need to take a deep copy of the current process environment, and just append to that, as there can be many other important env variables present.","commit_id":"25c9ac0f7e11bf34550cf5f0c71895a7d0463f95"},{"author":{"_account_id":6681,"name":"Brent Eagles","email":"beagles@redhat.com","username":"beagles"},"change_message_id":"d571bfd826c6087e1cfcad892e17d3ce1f93e30f","unresolved":false,"context_lines":[{"line_number":76,"context_line":"def run_plug_script(instance, vif, scriptpath, command):"},{"line_number":77,"context_line":"    environment_vars \u003d create_vif_plug_env(instance, vif)"},{"line_number":78,"context_line":"    try:"},{"line_number":79,"context_line":"        utils.execute(scriptpath, command, env_variables\u003denvironment_vars)"},{"line_number":80,"context_line":"    except processutils.ProcessExecutionError as e:"},{"line_number":81,"context_line":"        LOG.exception(e)"},{"line_number":82,"context_line":"        error_msg \u003d _(\u0027Failed to {command} VIF with {script} script, \u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"1aca2d91_2a4e706f","line":79,"in_reply_to":"7ad32947_f5376be1","updated":"2015-05-07 20:23:07.000000000","message":"Yes I think that would be the proper thing to do. I wonder if we might not want to create a copy of the entire environment, not just PATH. What do you think?","commit_id":"25c9ac0f7e11bf34550cf5f0c71895a7d0463f95"},{"author":{"_account_id":13869,"name":"Maxime Leroy","email":"maxime.leroy@6wind.com","username":"mleroy"},"change_message_id":"7d39ed460537970cde198068bab71243958b3853","unresolved":false,"context_lines":[{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    result \u003d {}"},{"line_number":71,"context_line":"    result[\u0027VIF_INSTANCE_ID\u0027] \u003d instance.uuid"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    # Create a flattened dictionary of the contents of the VIF data."},{"line_number":74,"context_line":"    # Nested dictionaries will be flattened out to a single level using"},{"line_number":75,"context_line":"    # key concatenation to provide a unique key at the root level. e.g."}],"source_content_type":"text/x-python","patch_set":9,"id":"5afe65bd_0f5f8d42","line":72,"updated":"2015-06-02 16:19:31.000000000","message":"The vif_plug_script needs to know about the mtu size. Else it\u0027s impossible to configure the bridge/interface to support jumbo frame.\n\nThere is an option in nova.conf  to set a specific mtu: network_device_mtu. This parameter should be added to the environment variables.\n\nWhat do you think ?","commit_id":"487acfb343cda855169d37be126598459b36ec84"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"48cb88ed8d5692869a3eb1468f0905e4cb3776c8","unresolved":false,"context_lines":[{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    result \u003d {}"},{"line_number":71,"context_line":"    result[\u0027VIF_INSTANCE_ID\u0027] \u003d instance.uuid"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    # Create a flattened dictionary of the contents of the VIF data."},{"line_number":74,"context_line":"    # Nested dictionaries will be flattened out to a single level using"},{"line_number":75,"context_line":"    # key concatenation to provide a unique key at the root level. e.g."}],"source_content_type":"text/x-python","patch_set":9,"id":"5afe65bd_82d4fdbc","line":72,"in_reply_to":"5afe65bd_0f5f8d42","updated":"2015-06-04 17:45:28.000000000","message":"i think it would be better for the neutron mechanism driver  to pass the mtu in the vif_binding details. failing that could your script not read the config file directly. there were discussions over the last few cycles over the desire to move away from having config options in the nova conf that change the behavior for networking.","commit_id":"487acfb343cda855169d37be126598459b36ec84"}]}