)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c78a362f300760bc880ae950d3bd0b45f234e6d8","unresolved":false,"context_lines":[{"line_number":24,"context_line":"deployments of Neutron which will always be providing the"},{"line_number":25,"context_line":"VIF information in the legacy format. It avoids the need"},{"line_number":26,"context_line":"for Nova to install any custom python packages beyond the"},{"line_number":27,"context_line":"core os_vif library"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"When Neutron is later converted to use os_vif, it will"},{"line_number":30,"context_line":"send a serialized os_vif.objects.VIFBase subclass over"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":25,"id":"bacf61ea_3d43796e","line":27,"range":{"start_line":27,"start_character":5,"end_line":27,"end_character":11},"updated":"2016-08-02 16:06:52.000000000","message":"nit: os-vif","commit_id":"6ec7b5e3091853508f2d9201d723dd5e89c43a15"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c0331d0e80109f2942282f3b26c900515715a0aa","unresolved":false,"context_lines":[{"line_number":38,"context_line":"eg \u0027vif_plug_ovs\u0027."},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"The plugins are required to use privsep for running"},{"line_number":41,"context_line":"privileged task, but this commit doesn\u0027t delete any"},{"line_number":42,"context_line":"rootwrap rules yet, as its difficult to untangle"},{"line_number":43,"context_line":"which rules are still needed by nova-network or other"},{"line_number":44,"context_line":"vif types. privsep is configured per-plugin via a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":25,"id":"dada55a8_c5582935","line":41,"updated":"2016-07-19 18:08:26.000000000","message":"s/task/tasks/","commit_id":"6ec7b5e3091853508f2d9201d723dd5e89c43a15"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c0331d0e80109f2942282f3b26c900515715a0aa","unresolved":false,"context_lines":[{"line_number":39,"context_line":""},{"line_number":40,"context_line":"The plugins are required to use privsep for running"},{"line_number":41,"context_line":"privileged task, but this commit doesn\u0027t delete any"},{"line_number":42,"context_line":"rootwrap rules yet, as its difficult to untangle"},{"line_number":43,"context_line":"which rules are still needed by nova-network or other"},{"line_number":44,"context_line":"vif types. privsep is configured per-plugin via a"},{"line_number":45,"context_line":"nova.conf entry named \u0027vif_plug_\u003cplugin namee\u003e_privileged\u0027"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":25,"id":"dada55a8_a55b752d","line":42,"updated":"2016-07-19 18:08:26.000000000","message":"s/its/it\u0027s/","commit_id":"6ec7b5e3091853508f2d9201d723dd5e89c43a15"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c0331d0e80109f2942282f3b26c900515715a0aa","unresolved":false,"context_lines":[{"line_number":42,"context_line":"rootwrap rules yet, as its difficult to untangle"},{"line_number":43,"context_line":"which rules are still needed by nova-network or other"},{"line_number":44,"context_line":"vif types. privsep is configured per-plugin via a"},{"line_number":45,"context_line":"nova.conf entry named \u0027vif_plug_\u003cplugin namee\u003e_privileged\u0027"},{"line_number":46,"context_line":"group."},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"Blueprint: os-vif-library"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":25,"id":"dada55a8_05436143","line":45,"updated":"2016-07-19 18:08:26.000000000","message":"s/namee/name/","commit_id":"6ec7b5e3091853508f2d9201d723dd5e89c43a15"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"940319b74aa635afff6b34707eff4b687055cf64","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This refactors the libvirt vif.py module so that it uses the"},{"line_number":10,"context_line":"os-vif library."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"All the various get_config methods are turned into static"},{"line_number":13,"context_line":"methods in the vif.py module. All the various plug/unplug"},{"line_number":14,"context_line":"methods are put in subclasses of the os-vif Plugin class."},{"line_number":15,"context_line":"These plugins are all registered as stevedore entrypoints"},{"line_number":16,"context_line":"with a \u0027compat_\u0027 prefix."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"The existing nova.network.model.VIF python objects are"},{"line_number":19,"context_line":"translated into subclasses of os_vif.objects.VIFBase,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":26,"id":"bacf61ea_f5928d4a","line":16,"range":{"start_line":12,"start_character":0,"end_line":16,"end_character":24},"updated":"2016-08-02 19:41:53.000000000","message":"Where is this done? Like, we have the os-vif job, but I don\u0027t see any configuration to use the plugins (not to mention the os-vif job is using neutron).","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"692ab47f58a0db877247881042a35f2790e30cb5","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This refactors the libvirt vif.py module so that it uses the"},{"line_number":10,"context_line":"os-vif library."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"All the various get_config methods are turned into static"},{"line_number":13,"context_line":"methods in the vif.py module. All the various plug/unplug"},{"line_number":14,"context_line":"methods are put in subclasses of the os-vif Plugin class."},{"line_number":15,"context_line":"These plugins are all registered as stevedore entrypoints"},{"line_number":16,"context_line":"with a \u0027compat_\u0027 prefix."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"The existing nova.network.model.VIF python objects are"},{"line_number":19,"context_line":"translated into subclasses of os_vif.objects.VIFBase,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":26,"id":"bacf61ea_56c0a3e0","line":16,"range":{"start_line":12,"start_character":0,"end_line":16,"end_character":24},"in_reply_to":"bacf61ea_f5928d4a","updated":"2016-08-03 13:00:53.000000000","message":"Oh this is outdated text from an earlier version","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"940319b74aa635afff6b34707eff4b687055cf64","unresolved":false,"context_lines":[{"line_number":26,"context_line":"for Nova to install any custom python packages beyond the"},{"line_number":27,"context_line":"core os-vif library"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"When Neutron is later converted to use os-vif, it will"},{"line_number":30,"context_line":"send a serialized os_vif.objects.VIFBase subclass over"},{"line_number":31,"context_line":"the wire to Nova, refering to an external plugin. One"},{"line_number":32,"context_line":"release after Neutron has shipped with full os-vif"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":26,"id":"bacf61ea_35b425d0","line":29,"range":{"start_line":29,"start_character":0,"end_line":29,"end_character":45},"updated":"2016-08-02 19:41:53.000000000","message":"Is this why the os-vif job doesn\u0027t use neutron?","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"692ab47f58a0db877247881042a35f2790e30cb5","unresolved":false,"context_lines":[{"line_number":26,"context_line":"for Nova to install any custom python packages beyond the"},{"line_number":27,"context_line":"core os-vif library"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"When Neutron is later converted to use os-vif, it will"},{"line_number":30,"context_line":"send a serialized os_vif.objects.VIFBase subclass over"},{"line_number":31,"context_line":"the wire to Nova, refering to an external plugin. One"},{"line_number":32,"context_line":"release after Neutron has shipped with full os-vif"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":26,"id":"bacf61ea_b6e75fab","line":29,"range":{"start_line":29,"start_character":0,"end_line":29,"end_character":45},"in_reply_to":"bacf61ea_35b425d0","updated":"2016-08-03 13:00:53.000000000","message":"No, the os-vif job is mostly irrelevant. That is merely testing the os-vif git master against nova, which was useful before we had any formal release of os-vif. Now we have os-vif in requirements.txt, *all* existing Nova CI jobs will be using os-vif to test this commit. This gives us coverage of nova-network linux bridge and neutron openvswitch.\n\nThis comment is referring to a future when Neutron sends us serialized os-vif obeccts instead of a port binding dict, which is completely separate","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dc17efa0b953d0a194ae2f353129c09155f191f3","unresolved":false,"context_lines":[{"line_number":26,"context_line":"for Nova to install any custom python packages beyond the"},{"line_number":27,"context_line":"core os-vif library"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"When Neutron is later converted to use os-vif, it will"},{"line_number":30,"context_line":"send a serialized os_vif.objects.VIFBase subclass over"},{"line_number":31,"context_line":"the wire to Nova, refering to an external plugin. One"},{"line_number":32,"context_line":"release after Neutron has shipped with full os-vif"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":26,"id":"bacf61ea_2c6dc604","line":29,"range":{"start_line":29,"start_character":0,"end_line":29,"end_character":45},"in_reply_to":"bacf61ea_b6e75fab","updated":"2016-08-03 20:08:11.000000000","message":"I don\u0027t think so, unless I\u0027m missing something. For example, the code logs at debug level \"Converting VIF\" when doing the nova.network.model.VIF to os_vif.objects.VIF, but that doesn\u0027t show up in the logs for jobs running on this change:\n\nhttp://logs.openstack.org/72/269672/27/check/gate-tempest-dsvm-neutron-full-ubuntu-xenial/410e7a4/logs/screen-n-cpu.txt.gz","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"940319b74aa635afff6b34707eff4b687055cf64","unresolved":false,"context_lines":[{"line_number":33,"context_line":"support, the various \u0027compat_\u0027 plugins can be deleted"},{"line_number":34,"context_line":"from Nova."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"When using os-vif each plugin gets its own section in"},{"line_number":37,"context_line":"the nova.conf file named \u0027vif_plug_\u003cplugin name\u003e\u0027"},{"line_number":38,"context_line":"eg \u0027vif_plug_ovs\u0027."},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"The plugins are required to use privsep for running"},{"line_number":41,"context_line":"privileged tasks, but this commit doesn\u0027t delete any"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":26,"id":"bacf61ea_15dc8117","line":38,"range":{"start_line":36,"start_character":0,"end_line":38,"end_character":18},"updated":"2016-08-02 19:41:53.000000000","message":"We should work these config options into nova.conf.sample like we do for other external libraries, like the oslo libraries.\n\nAlso, seems we should have a release note about using/configuration this, or some kind of docs for any deployment changes for using this, i.e. configuring the entry point for the plugin you want to use?","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"692ab47f58a0db877247881042a35f2790e30cb5","unresolved":false,"context_lines":[{"line_number":33,"context_line":"support, the various \u0027compat_\u0027 plugins can be deleted"},{"line_number":34,"context_line":"from Nova."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"When using os-vif each plugin gets its own section in"},{"line_number":37,"context_line":"the nova.conf file named \u0027vif_plug_\u003cplugin name\u003e\u0027"},{"line_number":38,"context_line":"eg \u0027vif_plug_ovs\u0027."},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"The plugins are required to use privsep for running"},{"line_number":41,"context_line":"privileged tasks, but this commit doesn\u0027t delete any"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":26,"id":"bacf61ea_31909995","line":38,"range":{"start_line":36,"start_character":0,"end_line":38,"end_character":18},"in_reply_to":"bacf61ea_15dc8117","updated":"2016-08-03 13:00:53.000000000","message":"With other libraries they\u0027re hard dependencies of Nova at build time. The VIF plugins only get associated with nova at deployment time - nova has no formal dependency on existence of any plugin impl. At the very most we would be able to add sample for the 2 built-in plugins linuxbridge + ovs as a special case.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dc17efa0b953d0a194ae2f353129c09155f191f3","unresolved":false,"context_lines":[{"line_number":33,"context_line":"support, the various \u0027compat_\u0027 plugins can be deleted"},{"line_number":34,"context_line":"from Nova."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"When using os-vif each plugin gets its own section in"},{"line_number":37,"context_line":"the nova.conf file named \u0027vif_plug_\u003cplugin name\u003e\u0027"},{"line_number":38,"context_line":"eg \u0027vif_plug_ovs\u0027."},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"The plugins are required to use privsep for running"},{"line_number":41,"context_line":"privileged tasks, but this commit doesn\u0027t delete any"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":26,"id":"bacf61ea_bf01d606","line":38,"range":{"start_line":36,"start_character":0,"end_line":38,"end_character":18},"in_reply_to":"bacf61ea_31909995","updated":"2016-08-03 20:08:11.000000000","message":"I\u0027m still confused as to how this would be working in the gate jobs because I\u0027m not seeing that happening w/o some configuration in those jobs to use this.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7579d465dba415a2a951c7185006aade70fd4f75","unresolved":false,"context_lines":[{"line_number":24,"context_line":"directly deserialize the os-vif objects, skipping all"},{"line_number":25,"context_line":"use of the nova.network.model objects."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"As part of using os-vif, each registered plugins gets the"},{"line_number":28,"context_line":"ability to read config parameters from nova.conf in a"},{"line_number":29,"context_line":"private section named \u0027vif_plug_\u003cplugin name\u003e\u0027. For"},{"line_number":30,"context_line":"example the OpenVSwitch plugin will be in \u0027vif_plug_ovs\u0027."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":27,"id":"bacf61ea_bfdf160b","line":27,"range":{"start_line":27,"start_character":41,"end_line":27,"end_character":48},"updated":"2016-08-03 20:23:20.000000000","message":"nit: plugin","commit_id":"6e0f2833912d4357a767bd1771ff0e300273fb59"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7579d465dba415a2a951c7185006aade70fd4f75","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"As part of using os-vif, each registered plugins gets the"},{"line_number":28,"context_line":"ability to read config parameters from nova.conf in a"},{"line_number":29,"context_line":"private section named \u0027vif_plug_\u003cplugin name\u003e\u0027. For"},{"line_number":30,"context_line":"example the OpenVSwitch plugin will be in \u0027vif_plug_ovs\u0027."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Plugins which make use of oslo.privsep for running some"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":27,"id":"bacf61ea_bf06f682","line":29,"updated":"2016-08-03 20:23:20.000000000","message":"Again, I don\u0027t think our gate jobs are actually testing any of the os-vif conversion code because the debug logging statements for doing the conversion are showing up. I\u0027d like to sort that out in case we have a false sense of things working here.","commit_id":"6e0f2833912d4357a767bd1771ff0e300273fb59"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"15a8ce56f6bf109c2075edb3180cdde8ac2c2b38","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":"As part of using os-vif, each registered plugins gets the"},{"line_number":28,"context_line":"ability to read config parameters from nova.conf in a"},{"line_number":29,"context_line":"private section named \u0027vif_plug_\u003cplugin name\u003e\u0027. For"},{"line_number":30,"context_line":"example the OpenVSwitch plugin will be in \u0027vif_plug_ovs\u0027."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Plugins which make use of oslo.privsep for running some"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":27,"id":"bacf61ea_02ea49ac","line":29,"in_reply_to":"bacf61ea_bf06f682","updated":"2016-08-03 20:40:32.000000000","message":"Ah here it is in the subsequent change:\n\nhttp://logs.openstack.org/95/350595/1/check/gate-tempest-dsvm-full-ubuntu-xenial/28f7ffe/logs/screen-n-cpu.txt.gz#_2016-08-03_17_53_45_291","commit_id":"6e0f2833912d4357a767bd1771ff0e300273fb59"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"c0dee0e6ae8dd94b5c7344d8b0d55701fb2b00dd","unresolved":false,"context_lines":[{"line_number":27,"context_line":"As part of using os-vif, each registered plugin gets the"},{"line_number":28,"context_line":"ability to read config parameters from nova.conf in a"},{"line_number":29,"context_line":"private section named \u0027vif_plug_\u003cplugin name\u003e\u0027. For"},{"line_number":30,"context_line":"example the OpenVSwitch plugin will be in \u0027vif_plug_ovs\u0027."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Plugins which make use of oslo.privsep for running some"},{"line_number":33,"context_line":"privileged code will have a further private section in"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":29,"id":"9ad45d7e_d722d1b8","line":30,"updated":"2016-08-09 08:09:53.000000000","message":"I don\u0027t see that being registered yet, and appearing in the samples, I guess that comes up later.","commit_id":"a8a0d89fdd7d8b85d64368747618bdf38199b9c3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c538a241d1cce7a00fe22bad9302ee3b50a31767","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"Plugins which make use of oslo.privsep for running some"},{"line_number":33,"context_line":"privileged code will have a further private section in"},{"line_number":34,"context_line":"nova.conf named \u0027vif_plug_\u003cplugin namee\u003e_privileged\u0027"},{"line_number":35,"context_line":"from which the privsep library will pull its configuration."},{"line_number":36,"context_line":"It is unlikely that operators will need to set any params"},{"line_number":37,"context_line":"in this section, since the defaults should work optimally"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":29,"id":"bacf61ea_dd5544fa","line":34,"range":{"start_line":34,"start_character":34,"end_line":34,"end_character":39},"updated":"2016-08-05 16:27:26.000000000","message":"name","commit_id":"a8a0d89fdd7d8b85d64368747618bdf38199b9c3"}],"etc/nova/rootwrap.d/compute.filters":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1753b5038d536858982c6ebe0d8e8add408a3cbb","unresolved":false,"context_lines":[{"line_number":218,"context_line":"# including os-brick and os-vif plugins."},{"line_number":219,"context_line":"# This line ties the superuser privs with the config files, context name,"},{"line_number":220,"context_line":"# and (implicitly) the actual python code invoked."},{"line_number":221,"context_line":"privsep-rootwrap: RegExpFilter, privsep-helper, root, privsep-helper, --config-file, /etc/(?!\\.\\.).*, --privsep_context, .*, --privsep_sock_path, /tmp/.*"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"# nova/storage/linuxscsi.py: sg_scan device"},{"line_number":224,"context_line":"sg_scan: CommandFilter, sg_scan, root"}],"source_content_type":"application/octet-stream","patch_set":25,"id":"1aa78d24_570e1233","line":221,"range":{"start_line":221,"start_character":121,"end_line":221,"end_character":124},"updated":"2016-07-14 02:19:50.000000000","message":"I haven\u0027t reviewed the rest of this change yet (I know, I need to, and plan on it), but there is the whole grenade/privsep issue that we\u0027re already dealing with os-brick, danpb is aware of that thread in the dev list.\n\nI plan on talking to sdague about the upgrade issue at the nova midcycle next week to figure out the path forward on that, but we need to sort that out before knowing what we\u0027re doing with os-brick and os-vif.","commit_id":"6ec7b5e3091853508f2d9201d723dd5e89c43a15"},{"author":{"_account_id":2750,"name":"Sean Dague","email":"sean@dague.net","username":"sdague"},"change_message_id":"785f0afbbc4991dbb2c0ad891b7e8da9ac6d235b","unresolved":false,"context_lines":[{"line_number":218,"context_line":"# including os-brick and os-vif plugins."},{"line_number":219,"context_line":"# This line ties the superuser privs with the config files, context name,"},{"line_number":220,"context_line":"# and (implicitly) the actual python code invoked."},{"line_number":221,"context_line":"privsep-rootwrap: RegExpFilter, privsep-helper, root, privsep-helper, --config-file, /etc/(?!\\.\\.).*, --privsep_context, .*, --privsep_sock_path, /tmp/.*"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"# nova/storage/linuxscsi.py: sg_scan device"},{"line_number":224,"context_line":"sg_scan: CommandFilter, sg_scan, root"}],"source_content_type":"application/octet-stream","patch_set":25,"id":"bacf61ea_7d717177","line":221,"range":{"start_line":221,"start_character":121,"end_line":221,"end_character":124},"in_reply_to":"1aa78d24_570e1233","updated":"2016-08-02 15:58:44.000000000","message":"This line no longer really does anything because of the rootwrap privsep white list. It might be worth changing up the comments here, and commenting the whole thing out.","commit_id":"6ec7b5e3091853508f2d9201d723dd5e89c43a15"}],"nova/network/os_vif.py":[{"author":{"_account_id":1561,"name":"Russell Bryant","email":"rbryant@redhat.com","username":"russellb"},"change_message_id":"6d106f6bf64003dcc4e670a8030c5e4b5b04c0d1","unresolved":false,"context_lines":[{"line_number":156,"context_line":"            port_profile \u003d profile,"},{"line_number":157,"context_line":"            plugin \u003d \"compat_ovs\","},{"line_number":158,"context_line":"            vif_name \u003d _get_vif_name(vif),"},{"line_number":159,"context_line":"            bridge_name \u003d _get_hybrid_bridge_name(vif))"},{"line_number":160,"context_line":""},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"# VIF_TYPE_IVS \u003d \u0027ivs\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a68dd71_bbe6a814","line":159,"updated":"2016-01-26 15:12:26.000000000","message":"should this be vif[\u0027network\u0027][\u0027bridge\u0027] ?  This is the non-hybrid case, right?","commit_id":"70b8bf411bc475eb9056e8e14e621cf99b5c0538"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1c7b943f118acbf22d962a86fcd6db9302685b82","unresolved":false,"context_lines":[{"line_number":161,"context_line":""},{"line_number":162,"context_line":"# VIF_TYPE_IVS \u003d \u0027ivs\u0027"},{"line_number":163,"context_line":"def _convert_vif_ivs(vif, instance):"},{"line_number":164,"context_line":"    profile \u003d objects.vif.VIFPortProfileOVS("},{"line_number":165,"context_line":"        instance_id \u003d vif.get(\u0027ovs_interfaceid\u0027) or vif[\u0027id\u0027])"},{"line_number":166,"context_line":"    if _get_firewall_required(vif) or vif.is_hybrid_plug_enabled():"},{"line_number":167,"context_line":"        return _get_vif_instance("},{"line_number":168,"context_line":"            vif, instance,"},{"line_number":169,"context_line":"            objects.vif.VIFBridge,"},{"line_number":170,"context_line":"            port_profile \u003d profile,"},{"line_number":171,"context_line":"            plugin \u003d \"compat_ivs_hybrid\","},{"line_number":172,"context_line":"            vif_name \u003d _get_vif_name(vif),"},{"line_number":173,"context_line":"            bridge_name \u003d _get_hybrid_bridge_name(vif))"},{"line_number":174,"context_line":"    else:"},{"line_number":175,"context_line":"        return _get_vif_instance("},{"line_number":176,"context_line":"            vif, instance,"},{"line_number":177,"context_line":"            objects.vif.VIFGeneric,"},{"line_number":178,"context_line":"            port_profile \u003d profile,"},{"line_number":179,"context_line":"            plugin \u003d \"compat_ivs\","},{"line_number":180,"context_line":"            vif_name \u003d _get_vif_name(vif))"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"# VIF_TYPE_DVS \u003d \u0027dvs\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a68dd71_8ae60be7","line":180,"range":{"start_line":164,"start_character":3,"end_line":180,"end_character":42},"updated":"2016-01-20 12:29:12.000000000","message":"this seams to be almost identical to the ovs case\n\ncan we combine into a function and reuse in both of the above\n\ndef _convert_vif_ovs(vif, instance):\n   _convert_hybrid_plug(vif,instance,\"compat_ovs_hybrid\",\"compat_ovs\")\n\n\ndef _convert_vif_ivs(vif, instance):\n   _convert_hybrid_plug(vif,instance,\"compat_ivs_hybrid\",\"compat_ivs\")\n\n\ndef _convert_hybrid_plug(vif,instance,plugin_hybrid,plugin_non_hybrid):\n    profile \u003d objects.vif.VIFPortProfileOVS(\n        instance_id \u003d vif.get(\u0027ovs_interfaceid\u0027) or vif[\u0027id\u0027])\n    if _get_firewall_required(vif) or vif.is_hybrid_plug_enabled():\n        return _get_vif_instance(\n            vif, instance,\n            objects.vif.VIFBridge,\n            port_profile \u003d profile,\n            plugin \u003d plugin_hybrid,\n            vif_name \u003d _get_vif_name(vif),\n            bridge_name \u003d _get_hybrid_bridge_name(vif))\n    else:\n        return _get_vif_instance(\n            vif, instance,\n            objects.vif.VIFGeneric,\n            port_profile \u003d profile,\n            plugin \u003d plugin_non_hybrid,\n            vif_name \u003d _get_vif_name(vif))","commit_id":"70b8bf411bc475eb9056e8e14e621cf99b5c0538"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1c7b943f118acbf22d962a86fcd6db9302685b82","unresolved":false,"context_lines":[{"line_number":284,"context_line":"        path \u003d vif[\"details\"][model.VIF_DETAILS_VHOSTUSER_SOCKET],"},{"line_number":285,"context_line":"        mode \u003d vif[\"details\"].get("},{"line_number":286,"context_line":"            model.VIF_DETAILS_VHOSTUSER_MODE,"},{"line_number":287,"context_line":"            objects.fields.VIFVHostUserMode.SERVER))"},{"line_number":288,"context_line":""},{"line_number":289,"context_line":""},{"line_number":290,"context_line":"# VIF_TYPE_VROUTER \u003d \u0027vrouter\u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a68dd71_4a26d317","line":287,"range":{"start_line":287,"start_character":44,"end_line":287,"end_character":50},"updated":"2016-01-20 12:29:12.000000000","message":"i think it would be better to default to CLIENT instead as dpdk cant support server mode at present. hopefully that will get fixed with 2.3 ...\n\nim not sure if we should actully default here though.\nif neutron does not spcify it is probably better to fail with an exception here.","commit_id":"70b8bf411bc475eb9056e8e14e621cf99b5c0538"}],"nova/network/os_vif_util.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7bb4fcb5cb2875da85718087c4a24337e867663b","unresolved":false,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"def _get_firewall_required(vif):"},{"line_number":51,"context_line":"    if vif.is_neutron_filtering_enabled():"},{"line_number":52,"context_line":"        return False"},{"line_number":53,"context_line":"    if CONF.firewall_driver !\u003d \"nova.virt.firewall.NoopFirewallDriver\":"},{"line_number":54,"context_line":"        return True"},{"line_number":55,"context_line":"    return False"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"def convert_instance(instance):"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a5ae5dd_f3003c68","line":55,"range":{"start_line":51,"start_character":4,"end_line":55,"end_character":16},"updated":"2016-02-05 18:22:07.000000000","message":"NIT: should this be\n\nif vif.is_neutron_filtering_enabled():\n        return False\nelif CONF.firewall_driver !\u003d \"nova.virt.firewall.NoopFirewallDriver\":\n        return True\nelse: return False\n\ni guess the last else could be dropped \nand just return false but its never a case that\nwe will take both ifs so they should not be separate.","commit_id":"a11b0616eff11b99681a5b61daeb16bd2b9fd4c9"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7bb4fcb5cb2875da85718087c4a24337e867663b","unresolved":false,"context_lines":[{"line_number":165,"context_line":"    return _get_vif_instance("},{"line_number":166,"context_line":"        vif,"},{"line_number":167,"context_line":"        objects.vif.VIFBridge,"},{"line_number":168,"context_line":"        plugin \u003d \"linux_bridge\","},{"line_number":169,"context_line":"        vif_name \u003d _get_vif_name(vif),"},{"line_number":170,"context_line":"        bridge_name \u003d vif[\"network\"][\"bridge\"])"},{"line_number":171,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5a5ae5dd_ee85afce","line":168,"range":{"start_line":168,"start_character":8,"end_line":168,"end_character":32},"updated":"2016-02-05 18:22:07.000000000","message":"should this not be vif.get(\u0027plugin\u0027,\"linux_bridge\") or something similar to read the plugin name for the neutron vif binding details if present and fall back to the default,\"linux_bridge\" in this case, when the the value is not set by neutron.\n\nwithout that vendors cannot simple supply a ml2 plugin for neutron and a os-vif plugin for there network backend which plugs the interface in there specific way without a change to nova to update this hardcoded mapping of vif_type to plugins.\n\nvendors would not be able to introduce new vif_types or modify the libvirt config via that mechanism but with out the flexibility to supply there own plugin without modifying nova it defeats the point of OS-VIF no?.","commit_id":"a11b0616eff11b99681a5b61daeb16bd2b9fd4c9"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7bb4fcb5cb2875da85718087c4a24337e867663b","unresolved":false,"context_lines":[{"line_number":174,"context_line":"def _convert_vif_ovs(vif):"},{"line_number":175,"context_line":"    profile \u003d objects.vif.VIFPortProfileOpenVSwitch("},{"line_number":176,"context_line":"        interface_id \u003d vif.get(\u0027ovs_interfaceid\u0027) or vif[\u0027id\u0027])"},{"line_number":177,"context_line":"    if _get_firewall_required(vif) or vif.is_hybrid_plug_enabled():"},{"line_number":178,"context_line":"        return _get_vif_instance("},{"line_number":179,"context_line":"            vif,"},{"line_number":180,"context_line":"            objects.vif.VIFBridge,"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a5ae5dd_aeaed7e3","line":177,"range":{"start_line":177,"start_character":7,"end_line":177,"end_character":34},"updated":"2016-02-05 18:22:07.000000000","message":"NIT: the naming of this function is a little ambiguous.\n\nthis should probaly be _get_nova_firewall_required\n\nthe impentation of _get_firewall_required does correctly check if nova needs to create a firewall and report false if neutron will configure it so the logic is correct.\n\nmy issue with the name is that in the case of a neutron firewall is enabled and requested for the port _get_firewall_required retruns false which suggest that the port is not firewalled when in fact it is just not by nova.","commit_id":"a11b0616eff11b99681a5b61daeb16bd2b9fd4c9"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7bb4fcb5cb2875da85718087c4a24337e867663b","unresolved":false,"context_lines":[{"line_number":201,"context_line":"            vif,"},{"line_number":202,"context_line":"            objects.vif.VIFBridge,"},{"line_number":203,"context_line":"            port_profile \u003d profile,"},{"line_number":204,"context_line":"            plugin \u003d \"XXXXivs_hybrid\","},{"line_number":205,"context_line":"            vif_name \u003d _get_vif_name(vif),"},{"line_number":206,"context_line":"            bridge_name \u003d _get_hybrid_bridge_name(vif))"},{"line_number":207,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a5ae5dd_ce5553c2","line":204,"range":{"start_line":204,"start_character":22,"end_line":204,"end_character":26},"updated":"2016-02-05 18:22:07.000000000","message":"what is the significance of the XXXX prefix?","commit_id":"a11b0616eff11b99681a5b61daeb16bd2b9fd4c9"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a0338c1e350e17146a24044d506e52ff0070110d","unresolved":false,"context_lines":[{"line_number":214,"context_line":""},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"# VIF_TYPE_DVS \u003d \u0027dvs\u0027"},{"line_number":217,"context_line":"def _convert_vif_dvs(vif):"},{"line_number":218,"context_line":"    return"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"dae33548_2711b55d","line":217,"updated":"2016-02-18 15:51:10.000000000","message":"Why is this not returning an object?","commit_id":"a11b0616eff11b99681a5b61daeb16bd2b9fd4c9"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a0338c1e350e17146a24044d506e52ff0070110d","unresolved":false,"context_lines":[{"line_number":393,"context_line":""},{"line_number":394,"context_line":"    if not func:"},{"line_number":395,"context_line":"        raise exception.NovaException("},{"line_number":396,"context_line":"            _LE(\"Unsupported VIF type %(type)s convert \u0027%(func)s\u0027\") %"},{"line_number":397,"context_line":"            { \u0027type\u0027: vif[\u0027type\u0027], \u0027func\u0027: funcname })"},{"line_number":398,"context_line":""},{"line_number":399,"context_line":"    vifobj \u003d func(vif)"}],"source_content_type":"text/x-python","patch_set":5,"id":"dae33548_e7334d12","line":396,"updated":"2016-02-18 15:51:10.000000000","message":"a exception does not need a log hint translation. this should be _(\"Unsupported ..\")","commit_id":"a11b0616eff11b99681a5b61daeb16bd2b9fd4c9"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8f335ee0fb52a25da53788905937a5aad428ccaf","unresolved":false,"context_lines":[{"line_number":40,"context_line":"def _get_vif_name(vif):"},{"line_number":41,"context_line":"    if \u0027devname\u0027 in vif and vif[\u0027devname\u0027] is not None:"},{"line_number":42,"context_line":"        return vif[\u0027devname\u0027]"},{"line_number":43,"context_line":"    return (\"nic\" + vif[\u0027id\u0027])[:model.NIC_NAME_LEN]"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"def _get_hybrid_bridge_name(vif):"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_d340f9a7","line":43,"updated":"2016-04-19 09:38:50.000000000","message":"can we change \"nic\" to \u0027nic\u0027 - just seems to be more consistent. sorry for the nit picking","commit_id":"4e0a47394b7304ad8b2cdeaecc5abb42b080da66"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8f335ee0fb52a25da53788905937a5aad428ccaf","unresolved":false,"context_lines":[{"line_number":44,"context_line":""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"def _get_hybrid_bridge_name(vif):"},{"line_number":47,"context_line":"    return (\"qbr\" + vif[\u0027id\u0027])[:model.NIC_NAME_LEN]"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"def _get_firewall_required(vif):"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_b3229dff","line":47,"updated":"2016-04-19 09:38:50.000000000","message":"ditto","commit_id":"4e0a47394b7304ad8b2cdeaecc5abb42b080da66"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8f335ee0fb52a25da53788905937a5aad428ccaf","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        uuid\u003dinstance.uuid,"},{"line_number":61,"context_line":"        name\u003dinstance.name)"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    if hasattr(instance, \"project_id\") and instance.project_id is not None:"},{"line_number":64,"context_line":"        info.project_id \u003d instance.project_id"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    return info"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_b3d73dbe","line":63,"updated":"2016-04-19 09:38:50.000000000","message":"can you use getattr here?","commit_id":"4e0a47394b7304ad8b2cdeaecc5abb42b080da66"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7e02d7c4931402fec4f9340272f3a10b6e2c5550","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        uuid\u003dinstance.uuid,"},{"line_number":61,"context_line":"        name\u003dinstance.name)"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    if hasattr(instance, \"project_id\") and instance.project_id is not None:"},{"line_number":64,"context_line":"        info.project_id \u003d instance.project_id"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    return info"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_66e680b1","line":63,"in_reply_to":"1a122d0e_b3d73dbe","updated":"2016-04-19 11:41:22.000000000","message":"is the intention to no set the info.project_id un less \ninstance project_id is valid? if so what default should be used with getattr? the is not None check would seam to imply that None is not desirable in this case but i could be wrong.","commit_id":"4e0a47394b7304ad8b2cdeaecc5abb42b080da66"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8f335ee0fb52a25da53788905937a5aad428ccaf","unresolved":false,"context_lines":[{"line_number":399,"context_line":""},{"line_number":400,"context_line":"    if not func:"},{"line_number":401,"context_line":"        raise exception.NovaException("},{"line_number":402,"context_line":"            _LE(\"Unsupported VIF type %(type)s convert \u0027%(func)s\u0027\") %"},{"line_number":403,"context_line":"            {\u0027type\u0027: vif[\u0027type\u0027], \u0027func\u0027: funcname})"},{"line_number":404,"context_line":""},{"line_number":405,"context_line":"    vifobj \u003d func(vif)"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_b3ac5d36","line":402,"updated":"2016-04-19 09:38:50.000000000","message":"a log hint should not be used for a Exception. Please remove the _LE\nthis should just be _()","commit_id":"4e0a47394b7304ad8b2cdeaecc5abb42b080da66"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7e02d7c4931402fec4f9340272f3a10b6e2c5550","unresolved":false,"context_lines":[{"line_number":399,"context_line":""},{"line_number":400,"context_line":"    if not func:"},{"line_number":401,"context_line":"        raise exception.NovaException("},{"line_number":402,"context_line":"            _LE(\"Unsupported VIF type %(type)s convert \u0027%(func)s\u0027\") %"},{"line_number":403,"context_line":"            {\u0027type\u0027: vif[\u0027type\u0027], \u0027func\u0027: funcname})"},{"line_number":404,"context_line":""},{"line_number":405,"context_line":"    vifobj \u003d func(vif)"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_a6d09820","line":402,"in_reply_to":"1a122d0e_b3ac5d36","updated":"2016-04-19 11:41:22.000000000","message":"actully exceptions should not be mark for translation at all\n\n\"Exception text should not be marked for translation, because if an exception occurs there is no guarantee that the translation machinery will be functional.\" see: https://wiki.openstack.org/wiki/Translations#What_To_Translate","commit_id":"4e0a47394b7304ad8b2cdeaecc5abb42b080da66"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1119b71e9d5b4e070032e74a6e14702a670814b2","unresolved":false,"context_lines":[{"line_number":186,"context_line":"            vif,"},{"line_number":187,"context_line":"            objects.vif.VIFBridge,"},{"line_number":188,"context_line":"            port_profile \u003d profile,"},{"line_number":189,"context_line":"            plugin \u003d \"ovs_hybrid\","},{"line_number":190,"context_line":"            vif_name \u003d _get_vif_name(vif),"},{"line_number":191,"context_line":"            bridge_name \u003d _get_hybrid_bridge_name(vif))"},{"line_number":192,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_f8cbac1c","line":189,"updated":"2016-05-19 13:35:30.000000000","message":"this should be \nplugin \u003d \"ovs\"\nonce https://review.openstack.org/#/c/285613/2 is merged.\ni will submit a followup patch to change this is this change merges first.","commit_id":"642b03e4e22c6204499aabbb7fa68f1e14551657"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"050ae8bab5b99c2504fd5422476d7f72908429ff","unresolved":false,"context_lines":[{"line_number":186,"context_line":"            vif,"},{"line_number":187,"context_line":"            objects.vif.VIFBridge,"},{"line_number":188,"context_line":"            port_profile \u003d profile,"},{"line_number":189,"context_line":"            plugin \u003d \"ovs_hybrid\","},{"line_number":190,"context_line":"            vif_name \u003d _get_vif_name(vif),"},{"line_number":191,"context_line":"            bridge_name \u003d _get_hybrid_bridge_name(vif))"},{"line_number":192,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_f43202f1","line":189,"in_reply_to":"bab6814e_b3161e53","updated":"2016-05-20 19:10:28.000000000","message":"it should be but its not \nhttps://github.com/openstack/os-vif/blob/1.0.0/vif_plug_ovs/ovs.py#L63\n\nill split that out of my vhost-user change and submit separately.","commit_id":"642b03e4e22c6204499aabbb7fa68f1e14551657"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"f485faf3506162bd2f209e366a9610bbcd2c7454","unresolved":false,"context_lines":[{"line_number":186,"context_line":"            vif,"},{"line_number":187,"context_line":"            objects.vif.VIFBridge,"},{"line_number":188,"context_line":"            port_profile \u003d profile,"},{"line_number":189,"context_line":"            plugin \u003d \"ovs_hybrid\","},{"line_number":190,"context_line":"            vif_name \u003d _get_vif_name(vif),"},{"line_number":191,"context_line":"            bridge_name \u003d _get_hybrid_bridge_name(vif))"},{"line_number":192,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_b3161e53","line":189,"in_reply_to":"bab6814e_f8cbac1c","updated":"2016-05-19 14:31:25.000000000","message":"In fact it should already be just \"ovs\" - we already killed the hybrid plugin for os-vif 1.0.0","commit_id":"642b03e4e22c6204499aabbb7fa68f1e14551657"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1119b71e9d5b4e070032e74a6e14702a670814b2","unresolved":false,"context_lines":[{"line_number":324,"context_line":"    elif vif[\u0027details\u0027].get("},{"line_number":325,"context_line":"            model.VIF_DETAILS_VHOSTUSER_OVS_PLUG,"},{"line_number":326,"context_line":"            False):"},{"line_number":327,"context_line":"        plugin \u003d \"XXXXvhostuser_ovs\""},{"line_number":328,"context_line":"    else:"},{"line_number":329,"context_line":"        plugin \u003d \"XXXXvhostuser\""},{"line_number":330,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"bab6814e_388894e7","line":327,"updated":"2016-05-19 13:35:30.000000000","message":"this should be \nplugin \u003d \"ovs\"\nonce https://review.openstack.org/#/c/285613/2 is merged.\ni will submit a followup patch to change this is this change merges first.","commit_id":"642b03e4e22c6204499aabbb7fa68f1e14551657"},{"author":{"_account_id":17671,"name":"Francesco Santoro","email":"fraroot@gmail.com","username":"francesco.santoro"},"change_message_id":"6209fde8eea320695d6088eb87e940b8b8df74be","unresolved":false,"context_lines":[{"line_number":179,"context_line":"def _convert_vif_ovs(vif):"},{"line_number":180,"context_line":"    profile \u003d objects.vif.VIFPortProfileOpenVSwitch("},{"line_number":181,"context_line":"        interface_id \u003d vif.get(\u0027ovs_interfaceid\u0027) or vif[\u0027id\u0027])"},{"line_number":182,"context_line":"    if _get_firewall_required(vif) or vif.is_hybrid_plug_enabled():"},{"line_number":183,"context_line":"        return _get_vif_instance("},{"line_number":184,"context_line":"            vif,"},{"line_number":185,"context_line":"            objects.vif.VIFBridge,"}],"source_content_type":"text/x-python","patch_set":15,"id":"bab6814e_fd3117a3","line":182,"updated":"2016-05-23 12:15:58.000000000","message":"Here we could think to add this hybrid_plug information to the port_profile (with an os-vif patch) and to move the if/else (and the needed methods) to the ovs plugin.\nWith this approach even other plugins (like vhostuser) that need to access this hybrid_plug information could do the same via the port_profile. Does it make sense for you?","commit_id":"3bee42a91374649eef817820e0cbcc1e3d3631a3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1cfd50f43a9a10306e221a1769e03990d5b0e224","unresolved":false,"context_lines":[{"line_number":179,"context_line":"def _convert_vif_ovs(vif):"},{"line_number":180,"context_line":"    profile \u003d objects.vif.VIFPortProfileOpenVSwitch("},{"line_number":181,"context_line":"        interface_id \u003d vif.get(\u0027ovs_interfaceid\u0027) or vif[\u0027id\u0027])"},{"line_number":182,"context_line":"    if _get_firewall_required(vif) or vif.is_hybrid_plug_enabled():"},{"line_number":183,"context_line":"        return _get_vif_instance("},{"line_number":184,"context_line":"            vif,"},{"line_number":185,"context_line":"            objects.vif.VIFBridge,"}],"source_content_type":"text/x-python","patch_set":15,"id":"bab6814e_cc87395f","line":182,"in_reply_to":"bab6814e_fd3117a3","updated":"2016-05-23 15:40:42.000000000","message":"hybrid plug and vhost-user are mutually exclucive with upstream ovs.\n\nwith vhost-user there is no tap device to add to a linux bridge. vhost-user ports are not kernel netdevs and cannot be used with the kenel networking stack directly without a backend such as ovs as an intermediary.","commit_id":"3bee42a91374649eef817820e0cbcc1e3d3631a3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1cfd50f43a9a10306e221a1769e03990d5b0e224","unresolved":false,"context_lines":[{"line_number":313,"context_line":"        vif_name \u003d _get_vif_name(vif))"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"# VIF_TYPE_VHOSTUSER \u003d \u0027vhostuser\u0027"},{"line_number":317,"context_line":"def _convert_vif_vhostuser(vif):"},{"line_number":318,"context_line":"    if vif[\u0027details\u0027].get("},{"line_number":319,"context_line":"            model.VIF_DETAILS_VHOSTUSER_FP_PLUG,"},{"line_number":320,"context_line":"            False):"}],"source_content_type":"text/x-python","patch_set":15,"id":"bab6814e_ef4caf9f","line":317,"range":{"start_line":316,"start_character":0,"end_line":317,"end_character":32},"updated":"2016-05-23 15:40:42.000000000","message":"as an fyi in the future it may be necessary to pass\nthe image properties and flavor info to os-vif also\n\nthe special case i am thinking of is for multiqueue with vhost-user.\ncurrently ovs has a global and a per port setting to control the  number of queues for the interface. the global max value is deprecated. if it is removed the ov-vif plug driver will need to set the queues when plugin the interface into ovs.\n\nfrom a dpdk point of view when ovs is changed to use the vhost pmd it should be able to detect the queues without requireing it to be programed per interface. if the global value is removed we will ether need to have os-vif set the value or will need this auto detection in place first.\n\nthe ovs brdige name should also be passed from the vif details\nto the plug driver.\n\nthese can be adressed in a follow up change though if and when they are needed.","commit_id":"3bee42a91374649eef817820e0cbcc1e3d3631a3"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"413b550f9bf34fe715e6d5bb047135fd9f626bb7","unresolved":false,"context_lines":[{"line_number":313,"context_line":"        vif_name \u003d _get_vif_name(vif))"},{"line_number":314,"context_line":""},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"# VIF_TYPE_VHOSTUSER \u003d \u0027vhostuser\u0027"},{"line_number":317,"context_line":"def _convert_vif_vhostuser(vif):"},{"line_number":318,"context_line":"    if vif[\u0027details\u0027].get("},{"line_number":319,"context_line":"            model.VIF_DETAILS_VHOSTUSER_FP_PLUG,"},{"line_number":320,"context_line":"            False):"}],"source_content_type":"text/x-python","patch_set":15,"id":"9abb7d3a_84120a57","line":317,"range":{"start_line":316,"start_character":0,"end_line":317,"end_character":32},"in_reply_to":"bab6814e_ef4caf9f","updated":"2016-06-02 12:34:35.000000000","message":"Yep, the code in this method is not even run yet - we explicitly skip anything other than plain ovs or linux bridge. This is just a placeholder for future patch which enables os-vif for vhostuser, so its fine if we update it later","commit_id":"3bee42a91374649eef817820e0cbcc1e3d3631a3"},{"author":{"_account_id":17671,"name":"Francesco Santoro","email":"fraroot@gmail.com","username":"francesco.santoro"},"change_message_id":"6209fde8eea320695d6088eb87e940b8b8df74be","unresolved":false,"context_lines":[{"line_number":318,"context_line":"    if vif[\u0027details\u0027].get("},{"line_number":319,"context_line":"            model.VIF_DETAILS_VHOSTUSER_FP_PLUG,"},{"line_number":320,"context_line":"            False):"},{"line_number":321,"context_line":"        plugin \u003d \"XXXXvhostuser_fp\""},{"line_number":322,"context_line":"    elif vif[\u0027details\u0027].get("},{"line_number":323,"context_line":"            model.VIF_DETAILS_VHOSTUSER_OVS_PLUG,"},{"line_number":324,"context_line":"            False):"}],"source_content_type":"text/x-python","patch_set":15,"id":"bab6814e_97b26a4a","line":321,"updated":"2016-05-23 12:15:58.000000000","message":"The current nova implementation of plug_vhostuser_fp needs to check both the model.VIF_DETAILS_VHOSTUSER_OVS_PLUG flag and the vif.is_hybrid_plug_enabled() return value.\nIn particular the VIF_DETAILS_VHOSTUSER_OVS_PLUG is set by the mechanism driver and used to distinguish the linuxbridge plug from the ovs one.\nThe is_hybrid_plug_enabled() is used to support ovs plug with security groups.\nIf we want to guarantee a proper transition of plug_vhostuser_fp to an os-vif plugin those variables should be someway exposed to the plugin (it is not the case with the current os-vif implementation). I just want to make you aware of this point and eventually open a discussion for possible solutions.","commit_id":"3bee42a91374649eef817820e0cbcc1e3d3631a3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1cfd50f43a9a10306e221a1769e03990d5b0e224","unresolved":false,"context_lines":[{"line_number":318,"context_line":"    if vif[\u0027details\u0027].get("},{"line_number":319,"context_line":"            model.VIF_DETAILS_VHOSTUSER_FP_PLUG,"},{"line_number":320,"context_line":"            False):"},{"line_number":321,"context_line":"        plugin \u003d \"XXXXvhostuser_fp\""},{"line_number":322,"context_line":"    elif vif[\u0027details\u0027].get("},{"line_number":323,"context_line":"            model.VIF_DETAILS_VHOSTUSER_OVS_PLUG,"},{"line_number":324,"context_line":"            False):"}],"source_content_type":"text/x-python","patch_set":15,"id":"bab6814e_af190724","line":321,"in_reply_to":"bab6814e_97b26a4a","updated":"2016-05-23 15:40:42.000000000","message":"VIF_DETAILS_VHOSTUSER_OVS_PLUG is not used to distinguish \n the linuxbridge plug from the ovs one.\n\nVIF_DETAILS_VHOSTUSER_OVS_PLUG is used to control if the port should be added to ovs with type\u003ddpdkvhostuser or not.\n\nVIF_DETAILS_OVS_HYBRID_PLUG  and \nportbindings.OVS_HYBRID_PLUG: hybrid_plug_required are used for that see:\nhttps://github.com/openstack/nova/blob/master/nova/network/model.py#L53\nhttps://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py#L53\n\nfor the special case of vhost-user with \nhttps://github.com/openstack/networking-6wind and the fastpath datapath then this info is indeed need.\n\nas you said in your other comment perhaps a port porifle should be used to carry this infomation however i dont think the standard ovs code should be changed.\n\ni think we should add a new port profile for the fastpath dataplane that would carry this extra information in this case.","commit_id":"3bee42a91374649eef817820e0cbcc1e3d3631a3"},{"author":{"_account_id":17671,"name":"Francesco Santoro","email":"fraroot@gmail.com","username":"francesco.santoro"},"change_message_id":"a6f1f1d93793f6dc4037ae8baa1556001eb4ecf0","unresolved":false,"context_lines":[{"line_number":318,"context_line":"    if vif[\u0027details\u0027].get("},{"line_number":319,"context_line":"            model.VIF_DETAILS_VHOSTUSER_FP_PLUG,"},{"line_number":320,"context_line":"            False):"},{"line_number":321,"context_line":"        plugin \u003d \"XXXXvhostuser_fp\""},{"line_number":322,"context_line":"    elif vif[\u0027details\u0027].get("},{"line_number":323,"context_line":"            model.VIF_DETAILS_VHOSTUSER_OVS_PLUG,"},{"line_number":324,"context_line":"            False):"}],"source_content_type":"text/x-python","patch_set":15,"id":"bab6814e_e66786e8","line":321,"in_reply_to":"bab6814e_af190724","updated":"2016-05-24 12:25:53.000000000","message":"I am aware of that but what I meant with my comment is that the networking-6wind mechanism driver uses VIF_DETAILS_VHOSTUSER_OVS_PLUG for a different purpose.\n\nThis was needed to let fastpath technology support plugging vhostuser interfaces in linuxbridge. And here I mean pure linuxbridge (https://github.com/openstack/networking-6wind/blob/master/networking_6wind/ml2_drivers/linuxbridge/mech_driver/mech_lb_fp.py) and not ovs hybrid plug.\n\nVia the VIF_DETAILS_VHOSTUSER_OVS_PLUG it is possible, for the networking-6wind driver, to distinguish the fastapath ovs plug (https://github.com/openstack/nova/blob/master/nova/tests/unit/virt/libvirt/test_vif.py#L319) from the pure linuxbridge (https://github.com/openstack/nova/blob/master/nova/tests/unit/virt/libvirt/test_vif.py#L298).\n\nI agree that exporting the VIF_DETAILS_OVS_HYBRID_PLUG in a dedicated port_profile is a good approach for hybrid plug detection but it will solve just a part of the problem.\n\nSo the point is that this patch, as it is, will break networking-6wind. Surely the design I described before should be reworked in a different patch but this one should not break existing nova code.","commit_id":"3bee42a91374649eef817820e0cbcc1e3d3631a3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e721c1c7c19d13b7b243608ba0edc7b034e53f1e","unresolved":false,"context_lines":[{"line_number":184,"context_line":"            vif,"},{"line_number":185,"context_line":"            objects.vif.VIFBridge,"},{"line_number":186,"context_line":"            port_profile \u003d profile,"},{"line_number":187,"context_line":"            plugin \u003d \"ovs_hybrid\","},{"line_number":188,"context_line":"            vif_name \u003d _get_vif_name(vif),"},{"line_number":189,"context_line":"            bridge_name \u003d _get_hybrid_bridge_name(vif))"},{"line_number":190,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":16,"id":"9abb7d3a_aa88b0c7","line":187,"range":{"start_line":187,"start_character":11,"end_line":187,"end_character":34},"updated":"2016-06-02 14:33:04.000000000","message":"the neutron test are failing with the following error\n\nNoMatchingPlugin: No VIF plugin was found with the name ovs_hybrid","commit_id":"7018f29e9671581a4e26763e063c34b55efb5672"},{"author":{"_account_id":17671,"name":"Francesco Santoro","email":"fraroot@gmail.com","username":"francesco.santoro"},"change_message_id":"e84153e7407f21fd5a66715cecd370cba4930933","unresolved":false,"context_lines":[{"line_number":318,"context_line":"    if vif[\u0027details\u0027].get("},{"line_number":319,"context_line":"            model.VIF_DETAILS_VHOSTUSER_FP_PLUG,"},{"line_number":320,"context_line":"            False):"},{"line_number":321,"context_line":"        plugin \u003d \"XXXXvhostuser_fp\""},{"line_number":322,"context_line":"    elif vif[\u0027details\u0027].get("},{"line_number":323,"context_line":"            model.VIF_DETAILS_VHOSTUSER_OVS_PLUG,"},{"line_number":324,"context_line":"            False):"}],"source_content_type":"text/x-python","patch_set":17,"id":"7aa08908_a7caed18","line":321,"updated":"2016-06-08 14:42:47.000000000","message":"Sorry to keep posting it but concerning the problems this patch will cause for vhostuser_fp plugin (VIF_DETAILS_VHOSTUSER_OVS_PLUG no more visible) how do you think we are going to manage it? Will this specific patch take into account a solution for this scenario or not?","commit_id":"f7d5084c2be9ae0ea917e561c416715236e6ee5f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f818980089cd8b662a5f0d5d90211709b585a187","unresolved":false,"context_lines":[{"line_number":318,"context_line":"    if vif[\u0027details\u0027].get("},{"line_number":319,"context_line":"            model.VIF_DETAILS_VHOSTUSER_FP_PLUG,"},{"line_number":320,"context_line":"            False):"},{"line_number":321,"context_line":"        plugin \u003d \"XXXXvhostuser_fp\""},{"line_number":322,"context_line":"    elif vif[\u0027details\u0027].get("},{"line_number":323,"context_line":"            model.VIF_DETAILS_VHOSTUSER_OVS_PLUG,"},{"line_number":324,"context_line":"            False):"}],"source_content_type":"text/x-python","patch_set":17,"id":"7aa08908_4466a42a","line":321,"in_reply_to":"7aa08908_2488b870","updated":"2016-06-08 15:06:46.000000000","message":"yes as daniel mentioned vhost-user is out of scope of this patch.\n\ni will submit one for vhost-user once the base functionality works. we can discuss how to account for the requirements of the fp datapath in that patch.\n\nif you notice on line 390 convert_vif function checks if the vif_type is ovs or linuxbridge. if its not it will return none.\n\nthe convert_vif function is called on line 530 of https://review.openstack.org/#/c/269672/17/nova/virt/libvirt/vif.py\n\nif the convert_vif retruns none it falls back to the previous vif plug logic. the new workflow of delegating to os-vif is only taken in the convert_vif function retruns an os-vif vif object.","commit_id":"f7d5084c2be9ae0ea917e561c416715236e6ee5f"},{"author":{"_account_id":17671,"name":"Francesco Santoro","email":"fraroot@gmail.com","username":"francesco.santoro"},"change_message_id":"92028282ae2d613c3764f2d5d71b0731f1e28a04","unresolved":false,"context_lines":[{"line_number":318,"context_line":"    if vif[\u0027details\u0027].get("},{"line_number":319,"context_line":"            model.VIF_DETAILS_VHOSTUSER_FP_PLUG,"},{"line_number":320,"context_line":"            False):"},{"line_number":321,"context_line":"        plugin \u003d \"XXXXvhostuser_fp\""},{"line_number":322,"context_line":"    elif vif[\u0027details\u0027].get("},{"line_number":323,"context_line":"            model.VIF_DETAILS_VHOSTUSER_OVS_PLUG,"},{"line_number":324,"context_line":"            False):"}],"source_content_type":"text/x-python","patch_set":17,"id":"7aa08908_4dbfe760","line":321,"in_reply_to":"7aa08908_4466a42a","updated":"2016-06-08 15:45:45.000000000","message":"Ok perfect. Thank you for your clarification, it was just to make sure that the check at line 390 will always be part of this patch.","commit_id":"f7d5084c2be9ae0ea917e561c416715236e6ee5f"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"2559da6517960d9071cf0f1a66c407f5f240c81c","unresolved":false,"context_lines":[{"line_number":318,"context_line":"    if vif[\u0027details\u0027].get("},{"line_number":319,"context_line":"            model.VIF_DETAILS_VHOSTUSER_FP_PLUG,"},{"line_number":320,"context_line":"            False):"},{"line_number":321,"context_line":"        plugin \u003d \"XXXXvhostuser_fp\""},{"line_number":322,"context_line":"    elif vif[\u0027details\u0027].get("},{"line_number":323,"context_line":"            model.VIF_DETAILS_VHOSTUSER_OVS_PLUG,"},{"line_number":324,"context_line":"            False):"}],"source_content_type":"text/x-python","patch_set":17,"id":"7aa08908_2488b870","line":321,"in_reply_to":"7aa08908_a7caed18","updated":"2016-06-08 14:54:19.000000000","message":"As mentioned before, this patch is only concerning itself with getting ovs + linux bridge working.\n\nThe remainder of these _convert_vif_* methods are just a placeholder for future work.\n\nAs such, I\u0027m not really wanting to get into a discussion about it here. We need to actually focus on geting ovs + linux bridge working before we explore any other types of VIF.","commit_id":"f7d5084c2be9ae0ea917e561c416715236e6ee5f"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"bda9f2a2ef4e66ab74c98a69649786b362a1993f","unresolved":false,"context_lines":[{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"def _get_vif_name(vif):"},{"line_number":40,"context_line":"    if \u0027devname\u0027 in vif and vif[\u0027devname\u0027] is not None:"},{"line_number":41,"context_line":"        return vif[\u0027devname\u0027]"},{"line_number":42,"context_line":"    return (\u0027nic\u0027 + vif[\u0027id\u0027])[:model.NIC_NAME_LEN]"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_b25c384c","line":40,"updated":"2016-06-12 22:51:28.000000000","message":"how about:\n\n    if vif.get(\u0027devname\u0027) is not None:\n\nalso, is it ok if devname is empty string?","commit_id":"cb872cd716e9132f62747af644a8cc42ed5b0b6c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"21295f9497a85f6b6b1b95923ea66f943992609c","unresolved":false,"context_lines":[{"line_number":37,"context_line":""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"def _get_vif_name(vif):"},{"line_number":40,"context_line":"    if \u0027devname\u0027 in vif and vif[\u0027devname\u0027] is not None:"},{"line_number":41,"context_line":"        return vif[\u0027devname\u0027]"},{"line_number":42,"context_line":"    return (\u0027nic\u0027 + vif[\u0027id\u0027])[:model.NIC_NAME_LEN]"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_27e90fe7","line":40,"in_reply_to":"7aa08908_b25c384c","updated":"2016-06-13 13:33:05.000000000","message":"We shouldn\u0027t be getting an empty string, so don\u0027t think we need explicitly check that here. If this was a problem, then I think the code which decodes the  RPC reply from Neutron should deal with it rather than waiting until here.\n\nI\u0027ll switch to vif.get(\"devname\", None) though","commit_id":"cb872cd716e9132f62747af644a8cc42ed5b0b6c"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"bda9f2a2ef4e66ab74c98a69649786b362a1993f","unresolved":false,"context_lines":[{"line_number":148,"context_line":""},{"line_number":149,"context_line":"    netobj \u003d objects.network.Network("},{"line_number":150,"context_line":"        id \u003d network[\u0027id\u0027],"},{"line_number":151,"context_line":"        bridge \u003d network[\u0027bridge\u0027],"},{"line_number":152,"context_line":"        subnets \u003d _convert_subnets(network[\u0027subnets\u0027]))"},{"line_number":153,"context_line":"    if network[\u0027label\u0027] is not None:"},{"line_number":154,"context_line":"        netobj.label \u003d network[\u0027label\u0027]"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_524b5487","line":151,"updated":"2016-06-12 22:51:28.000000000","message":"just FYI, bridge can be None (this is the case for Hyper-V VIFs). Setting a Bridge to None raises an exception in os-vif Network object.","commit_id":"cb872cd716e9132f62747af644a8cc42ed5b0b6c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"21295f9497a85f6b6b1b95923ea66f943992609c","unresolved":false,"context_lines":[{"line_number":148,"context_line":""},{"line_number":149,"context_line":"    netobj \u003d objects.network.Network("},{"line_number":150,"context_line":"        id \u003d network[\u0027id\u0027],"},{"line_number":151,"context_line":"        bridge \u003d network[\u0027bridge\u0027],"},{"line_number":152,"context_line":"        subnets \u003d _convert_subnets(network[\u0027subnets\u0027]))"},{"line_number":153,"context_line":"    if network[\u0027label\u0027] is not None:"},{"line_number":154,"context_line":"        netobj.label \u003d network[\u0027label\u0027]"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_a7bb3fe4","line":151,"in_reply_to":"7aa08908_524b5487","updated":"2016-06-13 13:33:05.000000000","message":"Ok, will fix that.","commit_id":"cb872cd716e9132f62747af644a8cc42ed5b0b6c"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"bda9f2a2ef4e66ab74c98a69649786b362a1993f","unresolved":false,"context_lines":[{"line_number":170,"context_line":"    return _get_vif_instance("},{"line_number":171,"context_line":"        vif,"},{"line_number":172,"context_line":"        objects.vif.VIFBridge,"},{"line_number":173,"context_line":"        plugin \u003d \"linux_bridge\","},{"line_number":174,"context_line":"        vif_name \u003d _get_vif_name(vif),"},{"line_number":175,"context_line":"        bridge_name \u003d vif[\"network\"][\"bridge\"])"},{"line_number":176,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_32504832","line":173,"updated":"2016-06-12 22:51:28.000000000","message":"can\u0027t we use the names in nova.network.models? the names like:\n\nVIF_TYPE_BRIDGE, VIF_TYPE_OVS, etc.?\n\nI think it would be better.\n\nalso, I\u0027m wondering how pep8 doesn\u0027t complain, as there are whitespaces sorounding the \u003d.","commit_id":"cb872cd716e9132f62747af644a8cc42ed5b0b6c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"21295f9497a85f6b6b1b95923ea66f943992609c","unresolved":false,"context_lines":[{"line_number":170,"context_line":"    return _get_vif_instance("},{"line_number":171,"context_line":"        vif,"},{"line_number":172,"context_line":"        objects.vif.VIFBridge,"},{"line_number":173,"context_line":"        plugin \u003d \"linux_bridge\","},{"line_number":174,"context_line":"        vif_name \u003d _get_vif_name(vif),"},{"line_number":175,"context_line":"        bridge_name \u003d vif[\"network\"][\"bridge\"])"},{"line_number":176,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_47c37b57","line":173,"in_reply_to":"7aa08908_32504832","updated":"2016-06-13 13:33:05.000000000","message":"The plugin names are not required to match the existing VIF_TYPE_xxx constants - any matching is just co-incidence. So using those constants would be misleading IMHO.","commit_id":"cb872cd716e9132f62747af644a8cc42ed5b0b6c"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"36550ed1b098802eeeb975d1ea096072f3d8ddaa","unresolved":false,"context_lines":[{"line_number":190,"context_line":"    else:"},{"line_number":191,"context_line":"        return _get_vif_instance("},{"line_number":192,"context_line":"            vif,"},{"line_number":193,"context_line":"            objects.vif.VIFOpenVSwitch,"},{"line_number":194,"context_line":"            port_profile \u003d profile,"},{"line_number":195,"context_line":"            plugin \u003d \"ovs\","},{"line_number":196,"context_line":"            vif_name \u003d _get_vif_name(vif),"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_02283e32","line":193,"updated":"2016-06-13 14:44:28.000000000","message":"hm, I don\u0027t see this being treated in the os_vif\u0027s OVSPlugin. OVSPlugin\u0027s plug only treats VIFBridge and VIFHostUser.","commit_id":"cb872cd716e9132f62747af644a8cc42ed5b0b6c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"8d1b0aac85dbae421704b190ef1739e467f8b3a7","unresolved":false,"context_lines":[{"line_number":190,"context_line":"    else:"},{"line_number":191,"context_line":"        return _get_vif_instance("},{"line_number":192,"context_line":"            vif,"},{"line_number":193,"context_line":"            objects.vif.VIFOpenVSwitch,"},{"line_number":194,"context_line":"            port_profile \u003d profile,"},{"line_number":195,"context_line":"            plugin \u003d \"ovs\","},{"line_number":196,"context_line":"            vif_name \u003d _get_vif_name(vif),"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_65402ca6","line":193,"in_reply_to":"7aa08908_02283e32","updated":"2016-06-13 15:08:14.000000000","message":"The ovs plugin currently supports VIFOpenVSwitch, VIFBridge and VIFVHostUser - see the \u0027describe\u0027 method impl. It happens that the plug/unplug code for VIFOpenVSwitch is a no-op, as libvirt automatically handles that logic, which is why you only see code related to VIFBridge/VIFVHostUser in the plug/unplug methods.","commit_id":"cb872cd716e9132f62747af644a8cc42ed5b0b6c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"4a9e21abc76d45e6ddcd65e558aa0e7cc0204eee","unresolved":false,"context_lines":[{"line_number":190,"context_line":"    else:"},{"line_number":191,"context_line":"        return _get_vif_instance("},{"line_number":192,"context_line":"            vif,"},{"line_number":193,"context_line":"            objects.vif.VIFOpenVSwitch,"},{"line_number":194,"context_line":"            port_profile \u003d profile,"},{"line_number":195,"context_line":"            plugin \u003d \"ovs\","},{"line_number":196,"context_line":"            vif_name \u003d _get_vif_name(vif),"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_11ea4a74","line":193,"in_reply_to":"7aa08908_36adbc84","updated":"2016-06-13 16:26:25.000000000","message":"VIFVHostUser support was only added to the ovs plugin in git master last week. This code is targetting os-vif 1.0.0 release","commit_id":"cb872cd716e9132f62747af644a8cc42ed5b0b6c"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"85c1f3aef43369f0eb1997bd71d4b455ec23efd0","unresolved":false,"context_lines":[{"line_number":190,"context_line":"    else:"},{"line_number":191,"context_line":"        return _get_vif_instance("},{"line_number":192,"context_line":"            vif,"},{"line_number":193,"context_line":"            objects.vif.VIFOpenVSwitch,"},{"line_number":194,"context_line":"            port_profile \u003d profile,"},{"line_number":195,"context_line":"            plugin \u003d \"ovs\","},{"line_number":196,"context_line":"            vif_name \u003d _get_vif_name(vif),"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_36adbc84","line":193,"in_reply_to":"7aa08908_65402ca6","updated":"2016-06-13 16:23:22.000000000","message":"I see. Still, you\u0027re not creating any vif of the VIFHostUser type. Why not?","commit_id":"cb872cd716e9132f62747af644a8cc42ed5b0b6c"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"bda9f2a2ef4e66ab74c98a69649786b362a1993f","unresolved":false,"context_lines":[{"line_number":206,"context_line":"            vif,"},{"line_number":207,"context_line":"            objects.vif.VIFBridge,"},{"line_number":208,"context_line":"            port_profile \u003d profile,"},{"line_number":209,"context_line":"            plugin \u003d \"XXXXivs_hybrid\","},{"line_number":210,"context_line":"            vif_name \u003d _get_vif_name(vif),"},{"line_number":211,"context_line":"            bridge_name \u003d _get_hybrid_bridge_name(vif))"},{"line_number":212,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_7246507c","line":209,"updated":"2016-06-12 22:51:28.000000000","message":"why the XXXX?","commit_id":"cb872cd716e9132f62747af644a8cc42ed5b0b6c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"21295f9497a85f6b6b1b95923ea66f943992609c","unresolved":false,"context_lines":[{"line_number":206,"context_line":"            vif,"},{"line_number":207,"context_line":"            objects.vif.VIFBridge,"},{"line_number":208,"context_line":"            port_profile \u003d profile,"},{"line_number":209,"context_line":"            plugin \u003d \"XXXXivs_hybrid\","},{"line_number":210,"context_line":"            vif_name \u003d _get_vif_name(vif),"},{"line_number":211,"context_line":"            bridge_name \u003d _get_hybrid_bridge_name(vif))"},{"line_number":212,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_0794d340","line":209,"in_reply_to":"7aa08908_7246507c","updated":"2016-06-13 13:33:05.000000000","message":"Because the plugin doesn\u0027t exist, so I\u0027ve actually no idea what the name will be. I\u0027m just going to delete this code as it seems to be constantly confusing reviewers.","commit_id":"cb872cd716e9132f62747af644a8cc42ed5b0b6c"},{"author":{"_account_id":8213,"name":"Claudiu Belu","email":"cbelu@cloudbasesolutions.com","username":"claudiub"},"change_message_id":"bda9f2a2ef4e66ab74c98a69649786b362a1993f","unresolved":false,"context_lines":[{"line_number":388,"context_line":"    LOG.debug(\"Convert VIF %s\", str(vif))"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"    if not (vif[\u0027type\u0027] \u003d\u003d model.VIF_TYPE_BRIDGE or"},{"line_number":391,"context_line":"            vif[\u0027type\u0027] \u003d\u003d model.VIF_TYPE_OVS):"},{"line_number":392,"context_line":"        LOG.debug(\"Not converting VIF type %s yet\","},{"line_number":393,"context_line":"                  vif[\u0027type\u0027])"},{"line_number":394,"context_line":"        return None"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_92413c66","line":391,"updated":"2016-06-12 22:51:28.000000000","message":"I suggest:\n\n    if vif[\u0027type\u0027] not in _SUPPORTED_VIF_TYPES:\n\nor something similar. There are going to be a lot more vif types.","commit_id":"cb872cd716e9132f62747af644a8cc42ed5b0b6c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"21295f9497a85f6b6b1b95923ea66f943992609c","unresolved":false,"context_lines":[{"line_number":388,"context_line":"    LOG.debug(\"Convert VIF %s\", str(vif))"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"    if not (vif[\u0027type\u0027] \u003d\u003d model.VIF_TYPE_BRIDGE or"},{"line_number":391,"context_line":"            vif[\u0027type\u0027] \u003d\u003d model.VIF_TYPE_OVS):"},{"line_number":392,"context_line":"        LOG.debug(\"Not converting VIF type %s yet\","},{"line_number":393,"context_line":"                  vif[\u0027type\u0027])"},{"line_number":394,"context_line":"        return None"}],"source_content_type":"text/x-python","patch_set":20,"id":"7aa08908_e77f0715","line":391,"in_reply_to":"7aa08908_92413c66","updated":"2016-06-13 13:33:05.000000000","message":"I\u0027m going to change this entirely, so that the convert_vif_xxx methods just raise NotImplementedError instead","commit_id":"cb872cd716e9132f62747af644a8cc42ed5b0b6c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c78a362f300760bc880ae950d3bd0b45f234e6d8","unresolved":false,"context_lines":[{"line_number":32,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":33,"context_line":"CONF \u003d cfg.CONF"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"os_vif.initialize()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"def _get_vif_name(vif):"}],"source_content_type":"text/x-python","patch_set":25,"id":"bacf61ea_bd2e49a3","line":35,"updated":"2016-08-02 16:06:52.000000000","message":"Probably worth commenting what this does.","commit_id":"6ec7b5e3091853508f2d9201d723dd5e89c43a15"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c0331d0e80109f2942282f3b26c900515715a0aa","unresolved":false,"context_lines":[{"line_number":35,"context_line":"os_vif.initialize()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"def _get_vif_name(vif):"},{"line_number":39,"context_line":"    if vif.get(\u0027devname\u0027, None) is not None:"},{"line_number":40,"context_line":"        return vif[\u0027devname\u0027]"},{"line_number":41,"context_line":"    return (\u0027nic\u0027 + vif[\u0027id\u0027])[:model.NIC_NAME_LEN]"}],"source_content_type":"text/x-python","patch_set":25,"id":"dada55a8_454d596b","line":38,"updated":"2016-07-19 18:08:26.000000000","message":"I know a number of these methods are small and \"private\", but it would be great to have some brief docstrings that at least indicate what the type of various parameters is. Especially considering we have a dict-like nova/network/models:VIF object thing and a similarly-named os_vif.objects.VIF object...","commit_id":"6ec7b5e3091853508f2d9201d723dd5e89c43a15"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c0331d0e80109f2942282f3b26c900515715a0aa","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    return (\u0027qbr\u0027 + vif[\u0027id\u0027])[:model.NIC_NAME_LEN]"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"def _get_firewall_required(vif):"},{"line_number":49,"context_line":"    if vif.is_neutron_filtering_enabled():"},{"line_number":50,"context_line":"        return False"},{"line_number":51,"context_line":"    if CONF.firewall_driver !\u003d \"nova.virt.firewall.NoopFirewallDriver\":"}],"source_content_type":"text/x-python","patch_set":25,"id":"dada55a8_2548255b","line":48,"updated":"2016-07-19 18:08:26.000000000","message":"femtonit: s/_get/_is/","commit_id":"6ec7b5e3091853508f2d9201d723dd5e89c43a15"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c0331d0e80109f2942282f3b26c900515715a0aa","unresolved":false,"context_lines":[{"line_number":53,"context_line":"    return False"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"def convert_instance(instance):"},{"line_number":57,"context_line":"    info \u003d objects.instance_info.InstanceInfo("},{"line_number":58,"context_line":"        uuid\u003dinstance.uuid,"},{"line_number":59,"context_line":"        name\u003dinstance.name)"}],"source_content_type":"text/x-python","patch_set":25,"id":"dada55a8_85759199","line":56,"updated":"2016-07-19 18:08:26.000000000","message":"convert instance to what? Please name the function in a way that it will be clear in the calling location what is actually being converted from and to...","commit_id":"6ec7b5e3091853508f2d9201d723dd5e89c43a15"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c0331d0e80109f2942282f3b26c900515715a0aa","unresolved":false,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"def _convert_ip(ip):"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    floating_ips \u003d [fip[\u0027address\u0027] for fip in ip.get(\u0027floating_ips\u0027, [])]"},{"line_number":71,"context_line":"    return objects.fixed_ip.FixedIP("},{"line_number":72,"context_line":"        address\u003dip[\u0027address\u0027],"}],"source_content_type":"text/x-python","patch_set":25,"id":"dada55a8_65785d90","line":69,"updated":"2016-07-19 18:08:26.000000000","message":"nit: extraneous blank line here...","commit_id":"6ec7b5e3091853508f2d9201d723dd5e89c43a15"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c0331d0e80109f2942282f3b26c900515715a0aa","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        floating_ips\u003dfloating_ips)"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"def _convert_ips(ips):"},{"line_number":77,"context_line":"    return objects.fixed_ip.FixedIPList("},{"line_number":78,"context_line":"        objects\u003d[_convert_ip(ip) for ip in ips])"},{"line_number":79,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"dada55a8_c56f89c3","line":76,"updated":"2016-07-19 18:08:26.000000000","message":"Similar remark about convert to what for these below functions... something like:\n\n nova_to_osvif_XXX()\n\nmight work? e.g. nova_to_osvif_ip()","commit_id":"6ec7b5e3091853508f2d9201d723dd5e89c43a15"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c0331d0e80109f2942282f3b26c900515715a0aa","unresolved":false,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    :returns: os_vif.objects.route.Route instance"},{"line_number":87,"context_line":"    \"\"\""},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    return objects.route.Route("},{"line_number":90,"context_line":"        cidr\u003droute[\u0027cidr\u0027],"},{"line_number":91,"context_line":"        gateway\u003droute[\u0027gateway\u0027][\u0027address\u0027],"}],"source_content_type":"text/x-python","patch_set":25,"id":"dada55a8_a57255ab","line":88,"updated":"2016-07-19 18:08:26.000000000","message":"femtonits: here and throughout, there are extraneous blank lines after docstrings...","commit_id":"6ec7b5e3091853508f2d9201d723dd5e89c43a15"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"649e2b2a71b227178957f7477e84828288cd48b7","unresolved":false,"context_lines":[{"line_number":145,"context_line":"    :returns: os_vif.objects.network.Network instance"},{"line_number":146,"context_line":"    \"\"\""},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"    netobj \u003d objects.network.Network("},{"line_number":149,"context_line":"        id\u003dnetwork[\u0027id\u0027],"},{"line_number":150,"context_line":"        subnets\u003d_convert_subnets(network[\u0027subnets\u0027]))"},{"line_number":151,"context_line":"    if network[\"bridge\"] is not None:"}],"source_content_type":"text/x-python","patch_set":25,"id":"bacf61ea_2086b013","line":148,"updated":"2016-08-02 16:14:54.000000000","message":"There are several other fields in the object, do we not need to set those?\n\nhttps://github.com/openstack/os-vif/blob/1.0.0/os_vif/objects/network.py#L26\n\nLike:\n\n        \u0027multi_host\u0027: fields.BooleanField(),\n        \u0027should_provide_bridge\u0027: fields.BooleanField(),\n        \u0027should_provide_vlan\u0027: fields.BooleanField(),\n        \u0027bridge_interface\u0027: fields.StringField(nullable\u003dTrue),\n        \u0027vlan\u0027: fields.IntegerField(nullable\u003dTrue),","commit_id":"6ec7b5e3091853508f2d9201d723dd5e89c43a15"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"984b69645679fe079f009a82a6a14abd962011e1","unresolved":false,"context_lines":[{"line_number":39,"context_line":"def _get_vif_name(vif):"},{"line_number":40,"context_line":"    \"\"\"Get a VIF device name"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    :param vif: os_vif.objects.vif.VIFBase instance"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    Get a string suitable for use as a host OS network"},{"line_number":45,"context_line":"    device name"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_b4c77e63","line":42,"range":{"start_line":42,"start_character":16,"end_line":42,"end_character":42},"updated":"2016-08-02 17:38:08.000000000","message":"I think this is actually nova.network.model.VIF. It would have to be given the VIF object in os-vif doesn\u0027t have dict compat.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"984b69645679fe079f009a82a6a14abd962011e1","unresolved":false,"context_lines":[{"line_number":55,"context_line":"def _get_hybrid_bridge_name(vif):"},{"line_number":56,"context_line":"    \"\"\"Get a bridge device name"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    :param vif: os_vif.objects.vif.VIFBase instance"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    Get a string suitable for use as a host OS bridge"},{"line_number":61,"context_line":"    device name"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_949b5a72","line":58,"range":{"start_line":58,"start_character":16,"end_line":58,"end_character":42},"updated":"2016-08-02 17:38:08.000000000","message":"nova.network.model.VIF","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"984b69645679fe079f009a82a6a14abd962011e1","unresolved":false,"context_lines":[{"line_number":69,"context_line":"def _is_firewall_required(vif):"},{"line_number":70,"context_line":"    \"\"\"Check if local firewall is required"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    :param vif: os_vif.objects.vif.VIFBase instance"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    :returns: True if local firewall is required"},{"line_number":75,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_34af0e7f","line":72,"range":{"start_line":72,"start_character":16,"end_line":72,"end_character":42},"updated":"2016-08-02 17:38:08.000000000","message":"nova.network.model.VIF","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"85adf746f2589fc22bc706709874633fb31935d7","unresolved":false,"context_lines":[{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    if vif.is_neutron_filtering_enabled():"},{"line_number":78,"context_line":"        return False"},{"line_number":79,"context_line":"    if CONF.firewall_driver !\u003d \"nova.virt.firewall.NoopFirewallDriver\":"},{"line_number":80,"context_line":"        return True"},{"line_number":81,"context_line":"    return False"},{"line_number":82,"context_line":""},{"line_number":83,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_ef2f63a5","line":80,"range":{"start_line":79,"start_character":4,"end_line":80,"end_character":19},"updated":"2016-08-02 17:52:53.000000000","message":"I don\u0027t think we have a test for this.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"984b69645679fe079f009a82a6a14abd962011e1","unresolved":false,"context_lines":[{"line_number":203,"context_line":""},{"line_number":204,"context_line":"    if network.get_meta(\"multi_host\") is not None:"},{"line_number":205,"context_line":"        netobj.multi_host \u003d network.get_meta(\"multi_host\")"},{"line_number":206,"context_line":"    if network.get_meta(\"should_provide_bridge\") is not None:"},{"line_number":207,"context_line":"        netobj.should_provide_bridge \u003d \\"},{"line_number":208,"context_line":"            network.get_meta(\"should_provide_bridge\")"},{"line_number":209,"context_line":"    if network.get_meta(\"bridge_interface\") is not None:"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_748866c7","line":206,"updated":"2016-08-02 17:38:08.000000000","message":"This must be a thing that neutron can send over?","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"984b69645679fe079f009a82a6a14abd962011e1","unresolved":false,"context_lines":[{"line_number":208,"context_line":"            network.get_meta(\"should_provide_bridge\")"},{"line_number":209,"context_line":"    if network.get_meta(\"bridge_interface\") is not None:"},{"line_number":210,"context_line":"        netobj.bridge_interface \u003d network.get_meta(\"bridge_interface\")"},{"line_number":211,"context_line":"    if network.get_meta(\"should_provide_vlan\") is not None:"},{"line_number":212,"context_line":"        netobj.should_provide_vlan \u003d network.get_meta(\"should_provide_vlan\")"},{"line_number":213,"context_line":"    if network.get_meta(\"vlan\") is not None:"},{"line_number":214,"context_line":"        netobj.vlan \u003d network.get_meta(\"vlan\")"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_948fdacb","line":211,"updated":"2016-08-02 17:38:08.000000000","message":"ditto","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"984b69645679fe079f009a82a6a14abd962011e1","unresolved":false,"context_lines":[{"line_number":225,"context_line":"    :returns: a os_vif.objects.vif.VIFBase instance"},{"line_number":226,"context_line":"    \"\"\""},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"    return cls("},{"line_number":229,"context_line":"        id\u003dvif[\u0027id\u0027],"},{"line_number":230,"context_line":"        address\u003dvif[\u0027address\u0027],"},{"line_number":231,"context_line":"        network\u003d_nova_to_osvif_network(vif[\u0027network\u0027]),"},{"line_number":232,"context_line":"        has_traffic_filtering\u003dvif.is_neutron_filtering_enabled(),"},{"line_number":233,"context_line":"        **kwargs)"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"# VIF_TYPE_BRIDGE \u003d \u0027bridge\u0027"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_8f3067ea","line":233,"range":{"start_line":228,"start_character":4,"end_line":233,"end_character":17},"updated":"2016-08-02 17:38:08.000000000","message":"Why don\u0027t set this?\n\nhttps://github.com/openstack/os-vif/blob/1.0.0/os_vif/objects/vif.py#L43\n\nOr \u0027active\u0027 or \u0027port_profile\u0027?","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4f669d03db2af64260c41edc69ba2b49938511d1","unresolved":false,"context_lines":[{"line_number":225,"context_line":"    :returns: a os_vif.objects.vif.VIFBase instance"},{"line_number":226,"context_line":"    \"\"\""},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"    return cls("},{"line_number":229,"context_line":"        id\u003dvif[\u0027id\u0027],"},{"line_number":230,"context_line":"        address\u003dvif[\u0027address\u0027],"},{"line_number":231,"context_line":"        network\u003d_nova_to_osvif_network(vif[\u0027network\u0027]),"},{"line_number":232,"context_line":"        has_traffic_filtering\u003dvif.is_neutron_filtering_enabled(),"},{"line_number":233,"context_line":"        **kwargs)"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"# VIF_TYPE_BRIDGE \u003d \u0027bridge\u0027"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_357265ae","line":233,"range":{"start_line":228,"start_character":4,"end_line":233,"end_character":17},"in_reply_to":"bacf61ea_8f3067ea","updated":"2016-08-02 19:30:39.000000000","message":"Oh I see port_profile set below for ovs.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"984b69645679fe079f009a82a6a14abd962011e1","unresolved":false,"context_lines":[{"line_number":238,"context_line":"    obj \u003d _get_vif_instance("},{"line_number":239,"context_line":"        vif,"},{"line_number":240,"context_line":"        objects.vif.VIFBridge,"},{"line_number":241,"context_line":"        plugin\u003d\"linux_bridge\","},{"line_number":242,"context_line":"        vif_name\u003d_get_vif_name(vif))"},{"line_number":243,"context_line":"    if vif[\"network\"][\"bridge\"] is not None:"},{"line_number":244,"context_line":"        obj.bridge_name \u003d vif[\"network\"][\"bridge\"]"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_6f12535a","line":241,"range":{"start_line":241,"start_character":8,"end_line":241,"end_character":29},"updated":"2016-08-02 17:38:08.000000000","message":"At some point we should change https://github.com/openstack/os-vif/blob/1.0.0/os_vif/objects/vif.py#L37 to an enum field and then we can use those enum constants from os-vif here in nova and make sure we don\u0027t send garbage over.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"692ab47f58a0db877247881042a35f2790e30cb5","unresolved":false,"context_lines":[{"line_number":238,"context_line":"    obj \u003d _get_vif_instance("},{"line_number":239,"context_line":"        vif,"},{"line_number":240,"context_line":"        objects.vif.VIFBridge,"},{"line_number":241,"context_line":"        plugin\u003d\"linux_bridge\","},{"line_number":242,"context_line":"        vif_name\u003d_get_vif_name(vif))"},{"line_number":243,"context_line":"    if vif[\"network\"][\"bridge\"] is not None:"},{"line_number":244,"context_line":"        obj.bridge_name \u003d vif[\"network\"][\"bridge\"]"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_71b971ec","line":241,"range":{"start_line":241,"start_character":8,"end_line":241,"end_character":29},"in_reply_to":"bacf61ea_6f12535a","updated":"2016-08-03 13:00:53.000000000","message":"Nope it cannot ever be an enum. We explicitly want people to be free to create arbitrary plugins without needing a lock-step upgrade of nova.\n\nIn future Nova will enumerate what plugins it has installed and pass this info Neutron, along with their versions and suppported VIF types. This will allow Neutron to ensure it only sends back a request for a config that actally exists on the compute node.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4f669d03db2af64260c41edc69ba2b49938511d1","unresolved":false,"context_lines":[{"line_number":246,"context_line":""},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"# VIF_TYPE_OVS \u003d \u0027ovs\u0027"},{"line_number":249,"context_line":"def _nova_to_osvif_vif_ovs(vif):"},{"line_number":250,"context_line":"    profile \u003d objects.vif.VIFPortProfileOpenVSwitch("},{"line_number":251,"context_line":"        interface_id\u003dvif.get(\u0027ovs_interfaceid\u0027) or vif[\u0027id\u0027])"},{"line_number":252,"context_line":"    if _is_firewall_required(vif) or vif.is_hybrid_plug_enabled():"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_b55e5510","line":249,"updated":"2016-08-02 19:30:39.000000000","message":"Random question, but os-vif has configuration options, e.g.:\n\nhttps://github.com/openstack/os-vif/blob/1.0.0/vif_plug_ovs/ovs.py#L42\n\nShouldn\u0027t we be exposing those through into the nova.conf.sample that we generate, like we do for oslo library config options?","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"692ab47f58a0db877247881042a35f2790e30cb5","unresolved":false,"context_lines":[{"line_number":246,"context_line":""},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"# VIF_TYPE_OVS \u003d \u0027ovs\u0027"},{"line_number":249,"context_line":"def _nova_to_osvif_vif_ovs(vif):"},{"line_number":250,"context_line":"    profile \u003d objects.vif.VIFPortProfileOpenVSwitch("},{"line_number":251,"context_line":"        interface_id\u003dvif.get(\u0027ovs_interfaceid\u0027) or vif[\u0027id\u0027])"},{"line_number":252,"context_line":"    if _is_firewall_required(vif) or vif.is_hybrid_plug_enabled():"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_f17ba127","line":249,"in_reply_to":"bacf61ea_b55e5510","updated":"2016-08-03 13:00:53.000000000","message":"See earlier comment. It would be possible to special case the 2 built-in plugins, but we can\u0027t do this for plugins in general since we won\u0027t have any direct dependancy on any plugins long term.  All the code in this file is just a short term hack until Neutron sends us serialized VIF objects directly.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1771f22bfab2dfa6d16dff51ddd72e71e8183fe5","unresolved":false,"context_lines":[{"line_number":339,"context_line":""},{"line_number":340,"context_line":"    :param vif: a nova.network.model.VIF instance"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    :returns: a os_vif.objects.vif.VIFBase subclass"},{"line_number":343,"context_line":"    \"\"\""},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"    LOG.debug(\"Converting VIF %s\", str(vif))"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_610ee7e6","line":342,"updated":"2016-08-02 18:17:11.000000000","message":"Might want to note here that returning None means there isn\u0027t an os-vif implementation for this vif type in nova yet and the old style stuff should happen.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"984b69645679fe079f009a82a6a14abd962011e1","unresolved":false,"context_lines":[{"line_number":342,"context_line":"    :returns: a os_vif.objects.vif.VIFBase subclass"},{"line_number":343,"context_line":"    \"\"\""},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"    LOG.debug(\"Converting VIF %s\", str(vif))"},{"line_number":346,"context_line":""},{"line_number":347,"context_line":"    funcname \u003d \"_nova_to_osvif_vif_\" + vif[\u0027type\u0027].replace(\".\", \"_\")"},{"line_number":348,"context_line":"    func \u003d getattr(sys.modules[__name__], funcname, None)"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_f42f36a5","line":345,"range":{"start_line":345,"start_character":35,"end_line":345,"end_character":43},"updated":"2016-08-02 17:38:08.000000000","message":"Do we need to stringify this?","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"85adf746f2589fc22bc706709874633fb31935d7","unresolved":false,"context_lines":[{"line_number":348,"context_line":"    func \u003d getattr(sys.modules[__name__], funcname, None)"},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"    if not func:"},{"line_number":351,"context_line":"        raise exception.NovaException("},{"line_number":352,"context_line":"            \"Unsupported VIF type %(type)s convert \u0027%(func)s\u0027\" %"},{"line_number":353,"context_line":"            {\u0027type\u0027: vif[\u0027type\u0027], \u0027func\u0027: funcname})"},{"line_number":354,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_8ff8674b","line":351,"updated":"2016-08-02 17:52:53.000000000","message":"Is this just a programming error or can we get into a situation like this because of backlevel os-vif library? Just wondering if we need something in the error message about checking config or os-vif version dependencies.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"692ab47f58a0db877247881042a35f2790e30cb5","unresolved":false,"context_lines":[{"line_number":348,"context_line":"    func \u003d getattr(sys.modules[__name__], funcname, None)"},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"    if not func:"},{"line_number":351,"context_line":"        raise exception.NovaException("},{"line_number":352,"context_line":"            \"Unsupported VIF type %(type)s convert \u0027%(func)s\u0027\" %"},{"line_number":353,"context_line":"            {\u0027type\u0027: vif[\u0027type\u0027], \u0027func\u0027: funcname})"},{"line_number":354,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_312099f7","line":351,"in_reply_to":"bacf61ea_8ff8674b","updated":"2016-08-03 13:00:53.000000000","message":"We\u0027d only get here if someone added a new VIF type without defining a conversion function here. We won\u0027t be getting new vif types now we have os-vif, so its an error really.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"85adf746f2589fc22bc706709874633fb31935d7","unresolved":false,"context_lines":[{"line_number":354,"context_line":""},{"line_number":355,"context_line":"    try:"},{"line_number":356,"context_line":"        vifobj \u003d func(vif)"},{"line_number":357,"context_line":"        LOG.debug(\"Converted object %s\", str(vifobj))"},{"line_number":358,"context_line":"        return vifobj"},{"line_number":359,"context_line":"    except NotImplementedError:"},{"line_number":360,"context_line":"        LOG.debug(\"No conversion for VIF type %s yet\","}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_af2e0ba3","line":357,"range":{"start_line":357,"start_character":41,"end_line":357,"end_character":44},"updated":"2016-08-02 17:52:53.000000000","message":"Do we need to stringify?","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"15a8ce56f6bf109c2075edb3180cdde8ac2c2b38","unresolved":false,"context_lines":[{"line_number":172,"context_line":"    if subnet[\u0027cidr\u0027] is not None:"},{"line_number":173,"context_line":"        obj.cidr \u003d subnet[\u0027cidr\u0027]"},{"line_number":174,"context_line":"    if subnet[\u0027gateway\u0027] is not None:"},{"line_number":175,"context_line":"        obj.gateway \u003d subnet[\u0027gateway\u0027][\u0027address\u0027]"},{"line_number":176,"context_line":"    return obj"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"bacf61ea_c2067184","line":175,"range":{"start_line":175,"start_character":8,"end_line":175,"end_character":50},"updated":"2016-08-03 20:40:32.000000000","message":"This is blowing up:\n\nhttp://logs.openstack.org/95/350595/1/check/gate-tempest-dsvm-full-ubuntu-xenial/28f7ffe/logs/screen-n-cpu.txt.gz#_2016-08-03_17_53_45_292","commit_id":"6e0f2833912d4357a767bd1771ff0e300273fb59"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7579d465dba415a2a951c7185006aade70fd4f75","unresolved":false,"context_lines":[{"line_number":343,"context_line":"def nova_to_osvif_vif(vif):"},{"line_number":344,"context_line":"    \"\"\"Convert a Nova VIF model to an os-vif object"},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"    :param vif: a nova.network.model.VIF instance"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    Attempt to convert a nova VIF instance into an os-vif"},{"line_number":349,"context_line":"    VIF object, pointing to a suitable plugin. This will"}],"source_content_type":"text/x-python","patch_set":27,"id":"bacf61ea_9f9332fa","line":346,"updated":"2016-08-03 20:23:20.000000000","message":"super nit: params usually come after the description/body in the docstring.","commit_id":"6e0f2833912d4357a767bd1771ff0e300273fb59"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2c68dee0cbf3f156836909ab6b1242db8e046054","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        interface\u003droute[\u0027interface\u0027])"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    if (route[\u0027gateway\u0027] is not None and"},{"line_number":141,"context_line":"        route[\u0027gateway\u0027][\u0027address\u0027] is not None):"},{"line_number":142,"context_line":"        obj.gateway \u003d route[\u0027gateway\u0027][\u0027address\u0027]"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"    return obj"}],"source_content_type":"text/x-python","patch_set":28,"id":"bacf61ea_9ee93bba","line":141,"updated":"2016-08-04 18:29:19.000000000","message":"I don\u0027t think we have a test for this case which is what was failing in the gate.","commit_id":"fa2fe2b9fd99c22c974f1980c2887b846ae5ca74"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c538a241d1cce7a00fe22bad9302ee3b50a31767","unresolved":false,"context_lines":[{"line_number":93,"context_line":"        uuid\u003dinstance.uuid,"},{"line_number":94,"context_line":"        name\u003dinstance.name)"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    if (instance.obj_attr_is_set(\"project_id\") and"},{"line_number":97,"context_line":"            instance.project_id is not None):"},{"line_number":98,"context_line":"        info.project_id \u003d instance.project_id"},{"line_number":99,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"bacf61ea_8abf04cf","line":96,"range":{"start_line":96,"start_character":8,"end_line":96,"end_character":46},"updated":"2016-08-05 16:27:26.000000000","message":"Note that you can do this:\n\n if \u0027project_id\u0027 in instance\n\nwhich is much more readable.\n\nWhen is project_id ever not set on an instance? The only time I can imagine when this would be is during unit tests.","commit_id":"a8a0d89fdd7d8b85d64368747618bdf38199b9c3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c538a241d1cce7a00fe22bad9302ee3b50a31767","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        id\u003dvif[\u0027id\u0027],"},{"line_number":236,"context_line":"        address\u003dvif[\u0027address\u0027],"},{"line_number":237,"context_line":"        network\u003d_nova_to_osvif_network(vif[\u0027network\u0027]),"},{"line_number":238,"context_line":"        has_traffic_filtering\u003dvif.is_neutron_filtering_enabled(),"},{"line_number":239,"context_line":"        preserve_on_delete\u003dvif[\u0027preserve_on_delete\u0027],"},{"line_number":240,"context_line":"        active\u003dvif[\u0027active\u0027],"},{"line_number":241,"context_line":"        **kwargs)"}],"source_content_type":"text/x-python","patch_set":29,"id":"bacf61ea_ca845cd4","line":238,"updated":"2016-08-05 16:27:26.000000000","message":"The whole point of this wrapper function is to default this property?","commit_id":"a8a0d89fdd7d8b85d64368747618bdf38199b9c3"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c538a241d1cce7a00fe22bad9302ee3b50a31767","unresolved":false,"context_lines":[{"line_number":358,"context_line":"    LOG.debug(\"Converting VIF %s\", vif)"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":"    funcname \u003d \"_nova_to_osvif_vif_\" + vif[\u0027type\u0027].replace(\".\", \"_\")"},{"line_number":361,"context_line":"    func \u003d getattr(sys.modules[__name__], funcname, None)"},{"line_number":362,"context_line":""},{"line_number":363,"context_line":"    if not func:"},{"line_number":364,"context_line":"        raise exception.NovaException("}],"source_content_type":"text/x-python","patch_set":29,"id":"bacf61ea_0aae742d","line":361,"updated":"2016-08-05 16:27:26.000000000","message":"Presumably vif[type] can\u0027t really be trusted, so I guess this is the safest way to look up the name ensuring it\u0027s only something in this file. But, still makes me a little nervous. It would be trivial to make a decorator at the top which registered each of the methods above in a list so we choose from those.\n\nI know, I\u0027m sure this is sufficient.","commit_id":"a8a0d89fdd7d8b85d64368747618bdf38199b9c3"}],"nova/tests/unit/network/test_os_vif_util.py":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c0331d0e80109f2942282f3b26c900515715a0aa","unresolved":false,"context_lines":[{"line_number":381,"context_line":""},{"line_number":382,"context_line":"        self.assertRaises(exception.NovaException,"},{"line_number":383,"context_line":"                          os_vif_util.convert_vif,"},{"line_number":384,"context_line":"                          vif)"}],"source_content_type":"text/x-python","patch_set":25,"id":"dada55a8_056a41b3","line":384,"updated":"2016-07-19 18:08:26.000000000","message":"Excellent unit tests, Dan.","commit_id":"6ec7b5e3091853508f2d9201d723dd5e89c43a15"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"85adf746f2589fc22bc706709874633fb31935d7","unresolved":false,"context_lines":[{"line_number":33,"context_line":"    def assertObjEqual(self, actual, expect):"},{"line_number":34,"context_line":"        actual.obj_reset_changes(recursive\u003dTrue)"},{"line_number":35,"context_line":"        expect.obj_reset_changes(recursive\u003dTrue)"},{"line_number":36,"context_line":"        self.assertEqual(actual.obj_to_primitive(),"},{"line_number":37,"context_line":"                         expect.obj_to_primitive())"},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    def test_nova_to_osvif_instance(self):"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_cf089fdc","line":36,"range":{"start_line":36,"start_character":13,"end_line":36,"end_character":24},"updated":"2016-08-02 17:52:53.000000000","message":"nit: args should be (expected, actual) for an accurate error message if this fails.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"85adf746f2589fc22bc706709874633fb31935d7","unresolved":false,"context_lines":[{"line_number":78,"context_line":"                ],"},{"line_number":79,"context_line":"                version\u003d4),"},{"line_number":80,"context_line":"            model.FixedIP("},{"line_number":81,"context_line":"                address\u003d\"2001::beef\","},{"line_number":82,"context_line":"                version\u003d6),"},{"line_number":83,"context_line":"        ]"},{"line_number":84,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_efb803eb","line":81,"range":{"start_line":81,"start_character":25,"end_line":81,"end_character":35},"updated":"2016-08-02 17:52:53.000000000","message":"2001: space beef?!","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"85adf746f2589fc22bc706709874633fb31935d7","unresolved":false,"context_lines":[{"line_number":133,"context_line":""},{"line_number":134,"context_line":"    def test_nova_to_osvif_subnets(self):"},{"line_number":135,"context_line":"        subnets \u003d ["},{"line_number":136,"context_line":"            model.Subnet(cidr\u003d\"192.168.1.0/24\","},{"line_number":137,"context_line":"                         dns\u003d["},{"line_number":138,"context_line":"                             model.IP("},{"line_number":139,"context_line":"                                 address\u003d\"192.168.1.1\","}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_8fc0672c","line":136,"range":{"start_line":136,"start_character":25,"end_line":136,"end_character":46},"updated":"2016-08-02 17:52:53.000000000","message":"According to the code cidr is optional so we could have a test wrinkle for that.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"85adf746f2589fc22bc706709874633fb31935d7","unresolved":false,"context_lines":[{"line_number":142,"context_line":"                                 address\u003d\"192.168.1.2\","},{"line_number":143,"context_line":"                                 type\u003d\"dns\"),"},{"line_number":144,"context_line":"                         ],"},{"line_number":145,"context_line":"                         gateway\u003dmodel.IP("},{"line_number":146,"context_line":"                             address\u003d\"192.168.1.254\","},{"line_number":147,"context_line":"                             type\u003d\u0027gateway\u0027),"},{"line_number":148,"context_line":"                         ips\u003d["},{"line_number":149,"context_line":"                             model.FixedIP("},{"line_number":150,"context_line":"                                 address\u003d\"192.168.1.100\","}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_afda8b15","line":147,"range":{"start_line":145,"start_character":25,"end_line":147,"end_character":44},"updated":"2016-08-02 17:52:53.000000000","message":"ditto","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"85adf746f2589fc22bc706709874633fb31935d7","unresolved":false,"context_lines":[{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        self.assertObjEqual(actual, expect)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    def test_nova_to_osvif_network_labelled_no_bridge(self):"},{"line_number":228,"context_line":"        network \u003d model.Network("},{"line_number":229,"context_line":"            id\u003d\"b82c1929-051e-481d-8110-4669916c7915\","},{"line_number":230,"context_line":"            label\u003d\"Demo Net\","}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_4f22ef02","line":227,"updated":"2016-08-02 17:52:53.000000000","message":"Could have a test to test some of the meta stuff like multi_host.\n\nAlso, s/labelled/labeled/.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2c68dee0cbf3f156836909ab6b1242db8e046054","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                                 address\u003d\"192.168.1.2\","},{"line_number":223,"context_line":"                                 type\u003d\"dns\"),"},{"line_number":224,"context_line":"                         ],"},{"line_number":225,"context_line":"                         gateway\u003dNone,"},{"line_number":226,"context_line":"                         ips\u003d["},{"line_number":227,"context_line":"                             model.FixedIP("},{"line_number":228,"context_line":"                                 address\u003d\"192.168.1.100\","}],"source_content_type":"text/x-python","patch_set":28,"id":"bacf61ea_5e0a238b","line":225,"updated":"2016-08-04 18:29:19.000000000","message":"This is the same as the subnet defined above, this would be a better wrinkle if you defined the gateway but it doesn\u0027t have an address value set in it.","commit_id":"fa2fe2b9fd99c22c974f1980c2887b846ae5ca74"}],"nova/tests/unit/objects/test_objects.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1771f22bfab2dfa6d16dff51ddd72e71e8183fe5","unresolved":false,"context_lines":[{"line_number":1207,"context_line":"}"},{"line_number":1208,"context_line":""},{"line_number":1209,"context_line":""},{"line_number":1210,"context_line":"def get_nova_objects():"},{"line_number":1211,"context_line":"    all_classes \u003d base.NovaObjectRegistry.obj_classes()"},{"line_number":1212,"context_line":"    nova_classes \u003d {}"},{"line_number":1213,"context_line":"    for name in all_classes:"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_c15eb311","line":1210,"updated":"2016-08-02 18:17:11.000000000","message":"nit: might not be a bad idea to have a comment that this is for filtering out non-nova object things in the registry, like os-vif objects.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"c538a241d1cce7a00fe22bad9302ee3b50a31767","unresolved":false,"context_lines":[{"line_number":1224,"context_line":"            base.NovaObject.OBJ_PROJECT_NAMESPACE):"},{"line_number":1225,"context_line":"            continue"},{"line_number":1226,"context_line":"        nova_classes[name] \u003d objclasses"},{"line_number":1227,"context_line":"    return nova_classes"},{"line_number":1228,"context_line":""},{"line_number":1229,"context_line":""},{"line_number":1230,"context_line":"class TestObjectVersions(test.NoDBTestCase):"}],"source_content_type":"text/x-python","patch_set":29,"id":"bacf61ea_2a289899","line":1227,"updated":"2016-08-05 16:27:26.000000000","message":"I officially don\u0027t like this. I have no better ideas, but I feel compelled to say it.","commit_id":"a8a0d89fdd7d8b85d64368747618bdf38199b9c3"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8f335ee0fb52a25da53788905937a5aad428ccaf","unresolved":false,"context_lines":[{"line_number":6154,"context_line":""},{"line_number":6155,"context_line":"                filterref \u003d \u0027./devices/interface/filterref\u0027"},{"line_number":6156,"context_line":"                vif \u003d network_info[0]"},{"line_number":6157,"context_line":"                nic_id \u003d vif[\u0027address\u0027].lower().replace(\u0027:\u0027, \u0027\u0027)"},{"line_number":6158,"context_line":"                fw \u003d firewall.NWFilterFirewall(drvr)"},{"line_number":6159,"context_line":"                instance_filter_name \u003d fw._instance_filter_name(instance_ref,"},{"line_number":6160,"context_line":"                                                                nic_id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_737125e0","line":6157,"updated":"2016-04-19 09:38:50.000000000","message":"why do we need the lower for the mac?","commit_id":"4e0a47394b7304ad8b2cdeaecc5abb42b080da66"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"f86a3e31b2ac028932715e0db4a5df52ea087c8c","unresolved":false,"context_lines":[{"line_number":6154,"context_line":""},{"line_number":6155,"context_line":"                filterref \u003d \u0027./devices/interface/filterref\u0027"},{"line_number":6156,"context_line":"                vif \u003d network_info[0]"},{"line_number":6157,"context_line":"                nic_id \u003d vif[\u0027address\u0027].lower().replace(\u0027:\u0027, \u0027\u0027)"},{"line_number":6158,"context_line":"                fw \u003d firewall.NWFilterFirewall(drvr)"},{"line_number":6159,"context_line":"                instance_filter_name \u003d fw._instance_filter_name(instance_ref,"},{"line_number":6160,"context_line":"                                                                nic_id)"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_b79bb765","line":6157,"in_reply_to":"1a122d0e_737125e0","updated":"2016-05-04 13:56:17.000000000","message":"Because the ovo mac address field type will lowercase its value, and we need to match it","commit_id":"4e0a47394b7304ad8b2cdeaecc5abb42b080da66"}],"nova/virt/libvirt/vif.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"1c7b943f118acbf22d962a86fcd6db9302685b82","unresolved":false,"context_lines":[{"line_number":233,"context_line":"    return cfg"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"def plug(vif, instance):"},{"line_number":237,"context_line":"    vif \u003d nova_os_vif.convert(vif, instance)"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":"    os_vif.plug(vif)"},{"line_number":240,"context_line":""},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"def unplug(vif, instance):"},{"line_number":243,"context_line":"    vif \u003d nova_os_vif.convert(vif, instance)"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"    os_vif.unplug(vif)"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"class VIFPluginLinuxBridge(plugin.PluginBase):"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a68dd71_adf769f4","line":245,"range":{"start_line":236,"start_character":0,"end_line":245,"end_character":22},"updated":"2016-01-20 12:29:12.000000000","message":"i assume the existing plug and unplug classes below will also be removed in the next version of the patch?","commit_id":"70b8bf411bc475eb9056e8e14e621cf99b5c0538"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a0338c1e350e17146a24044d506e52ff0070110d","unresolved":false,"context_lines":[{"line_number":477,"context_line":"        func \u003d getattr(self, profilefunc, None)"},{"line_number":478,"context_line":"        if not func:"},{"line_number":479,"context_line":"            raise exception.NovaException("},{"line_number":480,"context_line":"                _LE(\"Unsupported VIF port profile type %(obj)s func %(func)s\") %"},{"line_number":481,"context_line":"                {\u0027obj\u0027: vif.port_profile.obj_name(), \u0027func\u0027: profilefunc })"},{"line_number":482,"context_line":""},{"line_number":483,"context_line":"        func(vif.port_profile, conf)"}],"source_content_type":"text/x-python","patch_set":5,"id":"dae33548_222243a3","line":480,"updated":"2016-02-18 15:51:10.000000000","message":"same comment as before","commit_id":"a11b0616eff11b99681a5b61daeb16bd2b9fd4c9"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a0338c1e350e17146a24044d506e52ff0070110d","unresolved":false,"context_lines":[{"line_number":500,"context_line":"        func \u003d getattr(self, viffunc, None)"},{"line_number":501,"context_line":"        if not func:"},{"line_number":502,"context_line":"            raise exception.NovaException("},{"line_number":503,"context_line":"                _LE(\"Unsupported VIF type %(obj)s func %(func)s\") %"},{"line_number":504,"context_line":"                {\u0027obj\u0027: vif.obj_name(), \u0027func\u0027: viffunc })"},{"line_number":505,"context_line":"        func(instance, vif, conf)"},{"line_number":506,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"dae33548_2259a335","line":503,"updated":"2016-02-18 15:51:10.000000000","message":"ditto","commit_id":"a11b0616eff11b99681a5b61daeb16bd2b9fd4c9"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8f335ee0fb52a25da53788905937a5aad428ccaf","unresolved":false,"context_lines":[{"line_number":482,"context_line":"        func \u003d getattr(self, profilefunc, None)"},{"line_number":483,"context_line":"        if not func:"},{"line_number":484,"context_line":"            raise exception.NovaException("},{"line_number":485,"context_line":"                _LE(\"Unsupported VIF port profile type %(obj)s func \""},{"line_number":486,"context_line":"                    \"%(func)s\") %"},{"line_number":487,"context_line":"                {\u0027obj\u0027: vif.port_profile.obj_name(), \u0027func\u0027: profilefunc})"},{"line_number":488,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_d35f593f","line":485,"updated":"2016-04-19 09:38:50.000000000","message":"please remove the log hint","commit_id":"4e0a47394b7304ad8b2cdeaecc5abb42b080da66"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8f335ee0fb52a25da53788905937a5aad428ccaf","unresolved":false,"context_lines":[{"line_number":506,"context_line":"        func \u003d getattr(self, viffunc, None)"},{"line_number":507,"context_line":"        if not func:"},{"line_number":508,"context_line":"            raise exception.NovaException("},{"line_number":509,"context_line":"                _LE(\"Unsupported VIF type %(obj)s func %(func)s\") %"},{"line_number":510,"context_line":"                {\u0027obj\u0027: vif.obj_name(), \u0027func\u0027: viffunc})"},{"line_number":511,"context_line":"        func(instance, vif, conf)"},{"line_number":512,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"1a122d0e_3335edf6","line":509,"updated":"2016-04-19 09:38:50.000000000","message":"ditto","commit_id":"4e0a47394b7304ad8b2cdeaecc5abb42b080da66"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c0331d0e80109f2942282f3b26c900515715a0aa","unresolved":false,"context_lines":[{"line_number":2,"context_line":"# Copyright (C) 2011 Nicira, Inc"},{"line_number":3,"context_line":"# Copyright 2011 OpenStack Foundation"},{"line_number":4,"context_line":"# All Rights Reserved."},{"line_number":5,"context_line":"# Copyright 2016 Red Hat, Inc."},{"line_number":6,"context_line":"#"},{"line_number":7,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":8,"context_line":"#    not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":25,"id":"dada55a8_e53d6dbc","line":5,"updated":"2016-07-19 18:08:26.000000000","message":"Probably could have split this patch into the utils and os_vif dependency additions and a second patch with the vif.py libvirt module changes, but I can kind of understand putting everything into the same patch...","commit_id":"6ec7b5e3091853508f2d9201d723dd5e89c43a15"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1771f22bfab2dfa6d16dff51ddd72e71e8183fe5","unresolved":false,"context_lines":[{"line_number":84,"context_line":"    def _normalize_vif_type(self, vif_type):"},{"line_number":85,"context_line":"        return vif_type.replace(\u00272.1q\u0027, \u00272q\u0027)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    def get_vif_devname(self, vif):"},{"line_number":88,"context_line":"        if \u0027devname\u0027 in vif:"},{"line_number":89,"context_line":"            return vif[\u0027devname\u0027]"},{"line_number":90,"context_line":"        return (\"nic\" + vif[\u0027id\u0027])[:network_model.NIC_NAME_LEN]"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_a412ad56","line":87,"updated":"2016-08-02 18:17:11.000000000","message":"This is duplicated with _get_vif_name in os_vif_util, we should consolidate.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"692ab47f58a0db877247881042a35f2790e30cb5","unresolved":false,"context_lines":[{"line_number":84,"context_line":"    def _normalize_vif_type(self, vif_type):"},{"line_number":85,"context_line":"        return vif_type.replace(\u00272.1q\u0027, \u00272q\u0027)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    def get_vif_devname(self, vif):"},{"line_number":88,"context_line":"        if \u0027devname\u0027 in vif:"},{"line_number":89,"context_line":"            return vif[\u0027devname\u0027]"},{"line_number":90,"context_line":"        return (\"nic\" + vif[\u0027id\u0027])[:network_model.NIC_NAME_LEN]"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_b16c8909","line":87,"in_reply_to":"bacf61ea_a412ad56","updated":"2016-08-03 13:00:53.000000000","message":"In doing this conversion, I\u0027ve explicitly left all code in this file untouched if it relates to a VIF type we\u0027re not converting, as I don\u0027t want to touch code paths that impact things other than linux bridge + ovs.\n\nAs such I don\u0027t want to start refactoring this class to share more code/methods as you suggest.\n\nThis method will go away once all VIF types are converted in Ocata, so its a short lived duplication","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1771f22bfab2dfa6d16dff51ddd72e71e8183fe5","unresolved":false,"context_lines":[{"line_number":190,"context_line":"                (\"qvo%s\" % iface_id)[:network_model.NIC_NAME_LEN])"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    @staticmethod"},{"line_number":193,"context_line":"    def is_no_op_firewall():"},{"line_number":194,"context_line":"        return CONF.firewall_driver \u003d\u003d \"nova.virt.firewall.NoopFirewallDriver\""},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    def get_firewall_required(self, vif):"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_212b4f35","line":193,"updated":"2016-08-02 18:17:11.000000000","message":"nit: maybe move this to os_vif_util so we can re-use it in _is_firewall_required also.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"692ab47f58a0db877247881042a35f2790e30cb5","unresolved":false,"context_lines":[{"line_number":190,"context_line":"                (\"qvo%s\" % iface_id)[:network_model.NIC_NAME_LEN])"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"    @staticmethod"},{"line_number":193,"context_line":"    def is_no_op_firewall():"},{"line_number":194,"context_line":"        return CONF.firewall_driver \u003d\u003d \"nova.virt.firewall.NoopFirewallDriver\""},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    def get_firewall_required(self, vif):"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_d1213db6","line":193,"in_reply_to":"bacf61ea_212b4f35","updated":"2016-08-03 13:00:53.000000000","message":"Again, I don\u0027t want to start refactoring this file more broadly. This commit provides the minimal changes to convert only the linuxbridge + ovs plugins, leaving all other vif type codepaths untouched to avoid accidental regressions in them","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1771f22bfab2dfa6d16dff51ddd72e71e8183fe5","unresolved":false,"context_lines":[{"line_number":193,"context_line":"    def is_no_op_firewall():"},{"line_number":194,"context_line":"        return CONF.firewall_driver \u003d\u003d \"nova.virt.firewall.NoopFirewallDriver\""},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    def get_firewall_required(self, vif):"},{"line_number":197,"context_line":"        if vif.is_neutron_filtering_enabled():"},{"line_number":198,"context_line":"            return False"},{"line_number":199,"context_line":"        if self.is_no_op_firewall():"},{"line_number":200,"context_line":"            return False"},{"line_number":201,"context_line":"        return True"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def get_firewall_required_os_vif(self, vif):"},{"line_number":204,"context_line":"        if vif.has_traffic_filtering:"},{"line_number":205,"context_line":"            return False"},{"line_number":206,"context_line":"        if self.is_no_op_firewall():"},{"line_number":207,"context_line":"            return False"},{"line_number":208,"context_line":"        return True"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"    def get_config_bridge(self, instance, vif, image_meta,"},{"line_number":211,"context_line":"                          inst_type, virt_type, host):"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_61c9c7ac","line":208,"range":{"start_line":196,"start_character":4,"end_line":208,"end_character":19},"updated":"2016-08-02 18:17:11.000000000","message":"nit: we could have a single method that checks the type on the vif object to see what we should do.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"16b12963714ccc2b030bddcda5d58c2a47d10612","unresolved":false,"context_lines":[{"line_number":193,"context_line":"    def is_no_op_firewall():"},{"line_number":194,"context_line":"        return CONF.firewall_driver \u003d\u003d \"nova.virt.firewall.NoopFirewallDriver\""},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    def get_firewall_required(self, vif):"},{"line_number":197,"context_line":"        if vif.is_neutron_filtering_enabled():"},{"line_number":198,"context_line":"            return False"},{"line_number":199,"context_line":"        if self.is_no_op_firewall():"},{"line_number":200,"context_line":"            return False"},{"line_number":201,"context_line":"        return True"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    def get_firewall_required_os_vif(self, vif):"},{"line_number":204,"context_line":"        if vif.has_traffic_filtering:"},{"line_number":205,"context_line":"            return False"},{"line_number":206,"context_line":"        if self.is_no_op_firewall():"},{"line_number":207,"context_line":"            return False"},{"line_number":208,"context_line":"        return True"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"    def get_config_bridge(self, instance, vif, image_meta,"},{"line_number":211,"context_line":"                          inst_type, virt_type, host):"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_594f2fbe","line":208,"range":{"start_line":196,"start_character":4,"end_line":208,"end_character":19},"in_reply_to":"bacf61ea_61c9c7ac","updated":"2016-08-03 13:52:15.000000000","message":"I really dislike methods which magically accept completely different classes in a single parameter - nicer to be explicit about what each does","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1771f22bfab2dfa6d16dff51ddd72e71e8183fe5","unresolved":false,"context_lines":[{"line_number":425,"context_line":"        designer.set_vif_bandwidth_config(conf, inst_type)"},{"line_number":426,"context_line":"        return conf"},{"line_number":427,"context_line":""},{"line_number":428,"context_line":"    def _set_config_VIFGeneric(self, instance, vif, conf):"},{"line_number":429,"context_line":"        conf.net_type \u003d \"ethernet\""},{"line_number":430,"context_line":"        conf.target_dev \u003d vif.vif_name"},{"line_number":431,"context_line":"        conf.script \u003d \"\""}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_445bf12e","line":428,"updated":"2016-08-02 18:17:11.000000000","message":"Hmm, we don\u0027t support this yet, at least not in os_vif_util.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e7b5a2bec0e745278662d31fbda6dc85569eb3c0","unresolved":false,"context_lines":[{"line_number":446,"context_line":"        conf.target_dev \u003d vif.vif_name"},{"line_number":447,"context_line":"        self._set_config_VIFPortProfile(instance, vif, conf)"},{"line_number":448,"context_line":""},{"line_number":449,"context_line":"    def _set_config_VIFDirect(self, instance, vif, conf):"},{"line_number":450,"context_line":"        conf.net_type \u003d \"direct\""},{"line_number":451,"context_line":"        if hasattr(vif, \"vlan_name\"):"},{"line_number":452,"context_line":"            conf.source_dev \u003d vif.vlan_name"},{"line_number":453,"context_line":"        else:"},{"line_number":454,"context_line":"            conf.source_dev \u003d vif.dev_name"},{"line_number":455,"context_line":"        conf.target_dev \u003d vif.vif_name"},{"line_number":456,"context_line":"        conf.source_mode \u003d vif.mode"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"        if hasattr(vif, \"port_profile\"):"},{"line_number":459,"context_line":"            self._set_config_VIFPortProfile(instance, vif, conf)"},{"line_number":460,"context_line":""},{"line_number":461,"context_line":"    def _set_config_VIFVHostUser(self, instance, vif, conf):"},{"line_number":462,"context_line":"        conf.net_type \u003d \"vhostuser\""},{"line_number":463,"context_line":"        conf.vhostuser_type \u003d \"unix\""},{"line_number":464,"context_line":"        conf.vhostuser_path \u003d vif.path"},{"line_number":465,"context_line":"        conf.vhostuser_mode \u003d vif.mode"},{"line_number":466,"context_line":""},{"line_number":467,"context_line":"    def _set_config_VIFHostDevice(self, instance, vif, conf):"},{"line_number":468,"context_line":"        conf.net_type \u003d \"hostdev\""},{"line_number":469,"context_line":"        conf.source_dev \u003d vif.dev_address"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"    def _set_config_VIFPortProfileOpenVSwitch(self, profile, conf):"},{"line_number":472,"context_line":"        conf.vporttype \u003d \"openvswitch\""},{"line_number":473,"context_line":"        conf.add_vport_param(\"interfaceid\","},{"line_number":474,"context_line":"                             profile.interface_id)"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":"    def _set_config_VIFPortProfile8021Qbg(self, profile, conf):"},{"line_number":477,"context_line":"        conf.vporttype \u003d \"802.1Qbg\""},{"line_number":478,"context_line":"        conf.add_vport_param(\"managerid\","},{"line_number":479,"context_line":"                             profile.manager_id)"},{"line_number":480,"context_line":"        conf.add_vport_param(\"typeid\","},{"line_number":481,"context_line":"                             profile.type_id)"},{"line_number":482,"context_line":"        conf.add_vport_param(\"typeidversion\","},{"line_number":483,"context_line":"                             profile.type_id_version)"},{"line_number":484,"context_line":"        conf.add_vport_param(\"instanceid\","},{"line_number":485,"context_line":"                             profile.instance_id)"},{"line_number":486,"context_line":""},{"line_number":487,"context_line":"    def _set_config_VIFPortProfile8021Qbh(self, profile, conf):"},{"line_number":488,"context_line":"        conf.vporttype \u003d \"802.1Qbh\""},{"line_number":489,"context_line":"        conf.add_vport_param(\"profileid\","},{"line_number":490,"context_line":"                             profile.profile_id)"},{"line_number":491,"context_line":""},{"line_number":492,"context_line":"    def _set_config_VIFPortProfile(self, instance, vif, conf):"},{"line_number":493,"context_line":"        # Set any port profile that may be required"},{"line_number":494,"context_line":"        profilefunc \u003d \"_set_config_\" + vif.port_profile.obj_name()"},{"line_number":495,"context_line":"        func \u003d getattr(self, profilefunc, None)"},{"line_number":496,"context_line":"        if not func:"},{"line_number":497,"context_line":"            raise exception.NovaException("},{"line_number":498,"context_line":"                \"Unsupported VIF port profile type %(obj)s func %(func)s\" %"},{"line_number":499,"context_line":"                {\u0027obj\u0027: vif.port_profile.obj_name(), \u0027func\u0027: profilefunc})"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"        func(vif.port_profile, conf)"},{"line_number":502,"context_line":""},{"line_number":503,"context_line":"    def _get_config_os_vif(self, instance, vif, image_meta, inst_type,"},{"line_number":504,"context_line":"                           virt_type, host):"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_c4e36132","line":501,"range":{"start_line":449,"start_character":0,"end_line":501,"end_character":36},"updated":"2016-08-02 18:19:00.000000000","message":"We don\u0027t support any of these in os_vif_util do we? I\u0027m not sure why we\u0027re building the support for these into this change unless I\u0027m missing something.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"dc17efa0b953d0a194ae2f353129c09155f191f3","unresolved":false,"context_lines":[{"line_number":446,"context_line":"        conf.target_dev \u003d vif.vif_name"},{"line_number":447,"context_line":"        self._set_config_VIFPortProfile(instance, vif, conf)"},{"line_number":448,"context_line":""},{"line_number":449,"context_line":"    def _set_config_VIFDirect(self, instance, vif, conf):"},{"line_number":450,"context_line":"        conf.net_type \u003d \"direct\""},{"line_number":451,"context_line":"        if hasattr(vif, \"vlan_name\"):"},{"line_number":452,"context_line":"            conf.source_dev \u003d vif.vlan_name"},{"line_number":453,"context_line":"        else:"},{"line_number":454,"context_line":"            conf.source_dev \u003d vif.dev_name"},{"line_number":455,"context_line":"        conf.target_dev \u003d vif.vif_name"},{"line_number":456,"context_line":"        conf.source_mode \u003d vif.mode"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"        if hasattr(vif, \"port_profile\"):"},{"line_number":459,"context_line":"            self._set_config_VIFPortProfile(instance, vif, conf)"},{"line_number":460,"context_line":""},{"line_number":461,"context_line":"    def _set_config_VIFVHostUser(self, instance, vif, conf):"},{"line_number":462,"context_line":"        conf.net_type \u003d \"vhostuser\""},{"line_number":463,"context_line":"        conf.vhostuser_type \u003d \"unix\""},{"line_number":464,"context_line":"        conf.vhostuser_path \u003d vif.path"},{"line_number":465,"context_line":"        conf.vhostuser_mode \u003d vif.mode"},{"line_number":466,"context_line":""},{"line_number":467,"context_line":"    def _set_config_VIFHostDevice(self, instance, vif, conf):"},{"line_number":468,"context_line":"        conf.net_type \u003d \"hostdev\""},{"line_number":469,"context_line":"        conf.source_dev \u003d vif.dev_address"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"    def _set_config_VIFPortProfileOpenVSwitch(self, profile, conf):"},{"line_number":472,"context_line":"        conf.vporttype \u003d \"openvswitch\""},{"line_number":473,"context_line":"        conf.add_vport_param(\"interfaceid\","},{"line_number":474,"context_line":"                             profile.interface_id)"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":"    def _set_config_VIFPortProfile8021Qbg(self, profile, conf):"},{"line_number":477,"context_line":"        conf.vporttype \u003d \"802.1Qbg\""},{"line_number":478,"context_line":"        conf.add_vport_param(\"managerid\","},{"line_number":479,"context_line":"                             profile.manager_id)"},{"line_number":480,"context_line":"        conf.add_vport_param(\"typeid\","},{"line_number":481,"context_line":"                             profile.type_id)"},{"line_number":482,"context_line":"        conf.add_vport_param(\"typeidversion\","},{"line_number":483,"context_line":"                             profile.type_id_version)"},{"line_number":484,"context_line":"        conf.add_vport_param(\"instanceid\","},{"line_number":485,"context_line":"                             profile.instance_id)"},{"line_number":486,"context_line":""},{"line_number":487,"context_line":"    def _set_config_VIFPortProfile8021Qbh(self, profile, conf):"},{"line_number":488,"context_line":"        conf.vporttype \u003d \"802.1Qbh\""},{"line_number":489,"context_line":"        conf.add_vport_param(\"profileid\","},{"line_number":490,"context_line":"                             profile.profile_id)"},{"line_number":491,"context_line":""},{"line_number":492,"context_line":"    def _set_config_VIFPortProfile(self, instance, vif, conf):"},{"line_number":493,"context_line":"        # Set any port profile that may be required"},{"line_number":494,"context_line":"        profilefunc \u003d \"_set_config_\" + vif.port_profile.obj_name()"},{"line_number":495,"context_line":"        func \u003d getattr(self, profilefunc, None)"},{"line_number":496,"context_line":"        if not func:"},{"line_number":497,"context_line":"            raise exception.NovaException("},{"line_number":498,"context_line":"                \"Unsupported VIF port profile type %(obj)s func %(func)s\" %"},{"line_number":499,"context_line":"                {\u0027obj\u0027: vif.port_profile.obj_name(), \u0027func\u0027: profilefunc})"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"        func(vif.port_profile, conf)"},{"line_number":502,"context_line":""},{"line_number":503,"context_line":"    def _get_config_os_vif(self, instance, vif, image_meta, inst_type,"},{"line_number":504,"context_line":"                           virt_type, host):"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_dfa00aa8","line":501,"range":{"start_line":449,"start_character":0,"end_line":501,"end_character":36},"in_reply_to":"bacf61ea_ac08b229","updated":"2016-08-03 20:08:11.000000000","message":"OK, cool, I thought I was missing something pretty huge. :)","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"692ab47f58a0db877247881042a35f2790e30cb5","unresolved":false,"context_lines":[{"line_number":446,"context_line":"        conf.target_dev \u003d vif.vif_name"},{"line_number":447,"context_line":"        self._set_config_VIFPortProfile(instance, vif, conf)"},{"line_number":448,"context_line":""},{"line_number":449,"context_line":"    def _set_config_VIFDirect(self, instance, vif, conf):"},{"line_number":450,"context_line":"        conf.net_type \u003d \"direct\""},{"line_number":451,"context_line":"        if hasattr(vif, \"vlan_name\"):"},{"line_number":452,"context_line":"            conf.source_dev \u003d vif.vlan_name"},{"line_number":453,"context_line":"        else:"},{"line_number":454,"context_line":"            conf.source_dev \u003d vif.dev_name"},{"line_number":455,"context_line":"        conf.target_dev \u003d vif.vif_name"},{"line_number":456,"context_line":"        conf.source_mode \u003d vif.mode"},{"line_number":457,"context_line":""},{"line_number":458,"context_line":"        if hasattr(vif, \"port_profile\"):"},{"line_number":459,"context_line":"            self._set_config_VIFPortProfile(instance, vif, conf)"},{"line_number":460,"context_line":""},{"line_number":461,"context_line":"    def _set_config_VIFVHostUser(self, instance, vif, conf):"},{"line_number":462,"context_line":"        conf.net_type \u003d \"vhostuser\""},{"line_number":463,"context_line":"        conf.vhostuser_type \u003d \"unix\""},{"line_number":464,"context_line":"        conf.vhostuser_path \u003d vif.path"},{"line_number":465,"context_line":"        conf.vhostuser_mode \u003d vif.mode"},{"line_number":466,"context_line":""},{"line_number":467,"context_line":"    def _set_config_VIFHostDevice(self, instance, vif, conf):"},{"line_number":468,"context_line":"        conf.net_type \u003d \"hostdev\""},{"line_number":469,"context_line":"        conf.source_dev \u003d vif.dev_address"},{"line_number":470,"context_line":""},{"line_number":471,"context_line":"    def _set_config_VIFPortProfileOpenVSwitch(self, profile, conf):"},{"line_number":472,"context_line":"        conf.vporttype \u003d \"openvswitch\""},{"line_number":473,"context_line":"        conf.add_vport_param(\"interfaceid\","},{"line_number":474,"context_line":"                             profile.interface_id)"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":"    def _set_config_VIFPortProfile8021Qbg(self, profile, conf):"},{"line_number":477,"context_line":"        conf.vporttype \u003d \"802.1Qbg\""},{"line_number":478,"context_line":"        conf.add_vport_param(\"managerid\","},{"line_number":479,"context_line":"                             profile.manager_id)"},{"line_number":480,"context_line":"        conf.add_vport_param(\"typeid\","},{"line_number":481,"context_line":"                             profile.type_id)"},{"line_number":482,"context_line":"        conf.add_vport_param(\"typeidversion\","},{"line_number":483,"context_line":"                             profile.type_id_version)"},{"line_number":484,"context_line":"        conf.add_vport_param(\"instanceid\","},{"line_number":485,"context_line":"                             profile.instance_id)"},{"line_number":486,"context_line":""},{"line_number":487,"context_line":"    def _set_config_VIFPortProfile8021Qbh(self, profile, conf):"},{"line_number":488,"context_line":"        conf.vporttype \u003d \"802.1Qbh\""},{"line_number":489,"context_line":"        conf.add_vport_param(\"profileid\","},{"line_number":490,"context_line":"                             profile.profile_id)"},{"line_number":491,"context_line":""},{"line_number":492,"context_line":"    def _set_config_VIFPortProfile(self, instance, vif, conf):"},{"line_number":493,"context_line":"        # Set any port profile that may be required"},{"line_number":494,"context_line":"        profilefunc \u003d \"_set_config_\" + vif.port_profile.obj_name()"},{"line_number":495,"context_line":"        func \u003d getattr(self, profilefunc, None)"},{"line_number":496,"context_line":"        if not func:"},{"line_number":497,"context_line":"            raise exception.NovaException("},{"line_number":498,"context_line":"                \"Unsupported VIF port profile type %(obj)s func %(func)s\" %"},{"line_number":499,"context_line":"                {\u0027obj\u0027: vif.port_profile.obj_name(), \u0027func\u0027: profilefunc})"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"        func(vif.port_profile, conf)"},{"line_number":502,"context_line":""},{"line_number":503,"context_line":"    def _get_config_os_vif(self, instance, vif, image_meta, inst_type,"},{"line_number":504,"context_line":"                           virt_type, host):"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_ac08b229","line":501,"range":{"start_line":449,"start_character":0,"end_line":501,"end_character":36},"in_reply_to":"bacf61ea_c4e36132","updated":"2016-08-03 13:00:53.000000000","message":"Earlier versions of the patch did include themm in os_vif_util, but I ripped it out because it was not testable until we actually create os-vif plugins for those other vif types.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1771f22bfab2dfa6d16dff51ddd72e71e8183fe5","unresolved":false,"context_lines":[{"line_number":522,"context_line":"                {\u0027obj\u0027: vif.obj_name(), \u0027func\u0027: viffunc})"},{"line_number":523,"context_line":"        func(instance, vif, conf)"},{"line_number":524,"context_line":""},{"line_number":525,"context_line":"        # (vladikr) Not setting up driver and queues for vhostuser"},{"line_number":526,"context_line":"        # as queues are not supported in Libvirt until version 1.2.17"},{"line_number":527,"context_line":"        if (conf.net_type \u003d\u003d \"vhostuser\" and"},{"line_number":528,"context_line":"            not host.has_min_version(MIN_LIBVIRT_VHOSTUSER_MQ)):"},{"line_number":529,"context_line":"            LOG.debug(\u0027Queues are not a vhostuser supported feature.\u0027)"},{"line_number":530,"context_line":"            conf.driver_name \u003d None"},{"line_number":531,"context_line":"            conf.vhost_queues \u003d None"},{"line_number":532,"context_line":""},{"line_number":533,"context_line":"        designer.set_vif_bandwidth_config(conf, inst_type)"},{"line_number":534,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_c45ec111","line":531,"range":{"start_line":525,"start_character":8,"end_line":531,"end_character":36},"updated":"2016-08-02 18:17:11.000000000","message":"Why is this in here? We don\u0027t support vhostuser yet in os_vif_util.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1771f22bfab2dfa6d16dff51ddd72e71e8183fe5","unresolved":false,"context_lines":[{"line_number":600,"context_line":"                linux_net.create_ivs_vif_port(v2_name, iface_id,"},{"line_number":601,"context_line":"                                              vif[\u0027address\u0027], instance.uuid)"},{"line_number":602,"context_line":""},{"line_number":603,"context_line":"    def plug_ovs_hybrid(self, instance, vif):"},{"line_number":604,"context_line":"        \"\"\"Plug using hybrid strategy"},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        Create a per-VIF linux bridge, then link that bridge to the OVS"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_01672b26","line":603,"updated":"2016-08-02 18:17:11.000000000","message":"Why do we still have this? I figured it would be handled for us in os-vif?","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"16b12963714ccc2b030bddcda5d58c2a47d10612","unresolved":false,"context_lines":[{"line_number":600,"context_line":"                linux_net.create_ivs_vif_port(v2_name, iface_id,"},{"line_number":601,"context_line":"                                              vif[\u0027address\u0027], instance.uuid)"},{"line_number":602,"context_line":""},{"line_number":603,"context_line":"    def plug_ovs_hybrid(self, instance, vif):"},{"line_number":604,"context_line":"        \"\"\"Plug using hybrid strategy"},{"line_number":605,"context_line":""},{"line_number":606,"context_line":"        Create a per-VIF linux bridge, then link that bridge to the OVS"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_59fd6f3a","line":603,"in_reply_to":"bacf61ea_01672b26","updated":"2016-08-03 13:52:15.000000000","message":"This method is called from multiple places, only one of which was removed","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1771f22bfab2dfa6d16dff51ddd72e71e8183fe5","unresolved":false,"context_lines":[{"line_number":820,"context_line":"    def _plug_os_vif(self, instance, vif):"},{"line_number":821,"context_line":"        instance_info \u003d os_vif_util.nova_to_osvif_instance(instance)"},{"line_number":822,"context_line":""},{"line_number":823,"context_line":"        os_vif.plug(vif, instance_info)"},{"line_number":824,"context_line":""},{"line_number":825,"context_line":"    def plug(self, instance, vif):"},{"line_number":826,"context_line":"        vif_type \u003d vif[\u0027type\u0027]"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_8185dbae","line":823,"updated":"2016-08-02 18:17:11.000000000","message":"We should probably handle any os-vif specific exceptions here and convert them to NovaExceptions so we don\u0027t leak things up the stack.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1771f22bfab2dfa6d16dff51ddd72e71e8183fe5","unresolved":false,"context_lines":[{"line_number":850,"context_line":"                  \"vif_type\u003d%s\") % vif_type)"},{"line_number":851,"context_line":"        func(instance, vif)"},{"line_number":852,"context_line":""},{"line_number":853,"context_line":"    def unplug_ovs_hybrid(self, instance, vif):"},{"line_number":854,"context_line":"        \"\"\"UnPlug using hybrid strategy"},{"line_number":855,"context_line":""},{"line_number":856,"context_line":"        Unhook port from OVS, unhook port from bridge, delete"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_01c0cbdf","line":853,"updated":"2016-08-02 18:17:11.000000000","message":"Do we still need this? I figured this would be handled for us in os-vif.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":1779,"name":"Daniel Berrange","email":"berrange@redhat.com","username":"berrange"},"change_message_id":"16b12963714ccc2b030bddcda5d58c2a47d10612","unresolved":false,"context_lines":[{"line_number":850,"context_line":"                  \"vif_type\u003d%s\") % vif_type)"},{"line_number":851,"context_line":"        func(instance, vif)"},{"line_number":852,"context_line":""},{"line_number":853,"context_line":"    def unplug_ovs_hybrid(self, instance, vif):"},{"line_number":854,"context_line":"        \"\"\"UnPlug using hybrid strategy"},{"line_number":855,"context_line":""},{"line_number":856,"context_line":"        Unhook port from OVS, unhook port from bridge, delete"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_0426eeb9","line":853,"in_reply_to":"bacf61ea_01c0cbdf","updated":"2016-08-03 13:52:15.000000000","message":"Again, there\u0027s other vif types still calling this code","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1771f22bfab2dfa6d16dff51ddd72e71e8183fe5","unresolved":false,"context_lines":[{"line_number":1036,"context_line":"    def _unplug_os_vif(self, instance, vif):"},{"line_number":1037,"context_line":"        instance_info \u003d os_vif_util.nova_to_osvif_instance(instance)"},{"line_number":1038,"context_line":""},{"line_number":1039,"context_line":"        os_vif.unplug(vif, instance_info)"},{"line_number":1040,"context_line":""},{"line_number":1041,"context_line":"    def unplug(self, instance, vif):"},{"line_number":1042,"context_line":"        vif_type \u003d vif[\u0027type\u0027]"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_415e4377","line":1039,"updated":"2016-08-02 18:17:11.000000000","message":"Shouldn\u0027t we handle os_vif specific exceptions here and convert them to NovaExceptions in case anything higher up the stack is handling those, like this?\n\nhttps://github.com/openstack/nova/blob/a774399108903c1f3e2366b059e4dbacffd82a4b/nova/virt/libvirt/driver.py#L769\n\nI was just looking at all of the ways this can fail:\n\nhttps://github.com/openstack/os-vif/blob/1.0.0/os_vif/__init__.py#L81","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e4d62c259b73e461a49ece36dfda5fea89c6b7ba","unresolved":false,"context_lines":[{"line_number":1051,"context_line":"                _(\"vif_type parameter must be present \""},{"line_number":1052,"context_line":"                  \"for this vif_driver implementation\"))"},{"line_number":1053,"context_line":""},{"line_number":1054,"context_line":"        # Try os-vif codepath first"},{"line_number":1055,"context_line":"        vif_obj \u003d os_vif_util.nova_to_osvif_vif(vif)"},{"line_number":1056,"context_line":"        if vif_obj is not None:"},{"line_number":1057,"context_line":"            self._unplug_os_vif(instance, vif_obj)"},{"line_number":1058,"context_line":"            return"},{"line_number":1059,"context_line":""},{"line_number":1060,"context_line":"        # Legacy non-os-vif codepath"},{"line_number":1061,"context_line":"        vif_slug \u003d self._normalize_vif_type(vif_type)"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_e475c532","line":1058,"range":{"start_line":1054,"start_character":8,"end_line":1058,"end_character":18},"updated":"2016-08-02 18:26:36.000000000","message":"How do the unit tests for this work in test_vif? We\u0027d be getting the VIF object and calling os_vif.unplug but we don\u0027t actually have any plugins registered do we?","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"f71f55969e783f68dcaebf28237c8ec29fc90cbb","unresolved":false,"context_lines":[{"line_number":1051,"context_line":"                _(\"vif_type parameter must be present \""},{"line_number":1052,"context_line":"                  \"for this vif_driver implementation\"))"},{"line_number":1053,"context_line":""},{"line_number":1054,"context_line":"        # Try os-vif codepath first"},{"line_number":1055,"context_line":"        vif_obj \u003d os_vif_util.nova_to_osvif_vif(vif)"},{"line_number":1056,"context_line":"        if vif_obj is not None:"},{"line_number":1057,"context_line":"            self._unplug_os_vif(instance, vif_obj)"},{"line_number":1058,"context_line":"            return"},{"line_number":1059,"context_line":""},{"line_number":1060,"context_line":"        # Legacy non-os-vif codepath"},{"line_number":1061,"context_line":"        vif_slug \u003d self._normalize_vif_type(vif_type)"}],"source_content_type":"text/x-python","patch_set":26,"id":"bacf61ea_d71f91b5","line":1058,"range":{"start_line":1054,"start_character":8,"end_line":1058,"end_character":18},"in_reply_to":"bacf61ea_e475c532","updated":"2016-08-02 18:55:43.000000000","message":"We don\u0027 test this, because I pulled this change down and added an exception in here:\n\nhttp://paste.openstack.org/show/545721/\n\nAnd everything passed.","commit_id":"4f81a6b26dd2f9af0cc64beeca2e936b1ce3659c"}]}