)]}'
{"bin/quantum-rootwrap-dom0":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"29d91dfdbacdd77ddb28eb826437f303f5fdae03","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"def load_configuration(exec_name, config_file):"},{"line_number":52,"context_line":"    config \u003d ConfigParser.RawConfigParser()"},{"line_number":53,"context_line":"    config.read(config_file)"},{"line_number":54,"context_line":"    try:"},{"line_number":55,"context_line":"        url \u003d config.get(\"DEFAULT\", \"xenapi_connection_url\")"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"AAAALn%2F%2F5Mc%3D","line":52,"updated":"2012-10-30 12:48:00.000000000","message":"It seems wrong not to use the standard config libs. What does the normal root wrap do?","commit_id":"5c7ee582780786fff457c60526f9c04226aaf22a"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"def load_configuration(exec_name, config_file):"},{"line_number":52,"context_line":"    config \u003d ConfigParser.RawConfigParser()"},{"line_number":53,"context_line":"    config.read(config_file)"},{"line_number":54,"context_line":"    try:"},{"line_number":55,"context_line":"        url \u003d config.get(\"DEFAULT\", \"xenapi_connection_url\")"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"AAAALn%2F%2F4u0%3D","line":52,"in_reply_to":"AAAALn%2F%2F5Mc%3D","updated":"2012-10-30 17:08:27.000000000","message":"The normal root wrap uses exactly this pattern:\n\nhttps://github.com/openstack/quantum/blob/master/bin/quantum-rootwrap\n\nThe quantum rootwrap is based on the nova one, so this one is consistent.","commit_id":"5c7ee582780786fff457c60526f9c04226aaf22a"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"29d91dfdbacdd77ddb28eb826437f303f5fdae03","unresolved":false,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"def run_command(url, username, password, user_args):"},{"line_number":65,"context_line":"    session \u003d XenAPI.Session(url)"},{"line_number":66,"context_line":"    session.login_with_password(username, password)"},{"line_number":67,"context_line":"    host \u003d session.xenapi.session.get_this_host(session.handle)"},{"line_number":68,"context_line":"    result \u003d session.xenapi.host.call_plugin(host, \u0027netwrap\u0027, \u0027run_command\u0027,"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"AAAALn%2F%2F5Mg%3D","line":65,"updated":"2012-10-30 12:48:00.000000000","message":"I worry about the lack of session management here.\nWe are looking at pulling that into OpenStack common, so it should be easier to share with Cinder, Nova and Quantum.\nBut this is probably OK for now, till that is ready - maybe raise a bug to track that extra work?","commit_id":"5c7ee582780786fff457c60526f9c04226aaf22a"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"def run_command(url, username, password, user_args):"},{"line_number":65,"context_line":"    session \u003d XenAPI.Session(url)"},{"line_number":66,"context_line":"    session.login_with_password(username, password)"},{"line_number":67,"context_line":"    host \u003d session.xenapi.session.get_this_host(session.handle)"},{"line_number":68,"context_line":"    result \u003d session.xenapi.host.call_plugin(host, \u0027netwrap\u0027, \u0027run_command\u0027,"}],"source_content_type":"application/octet-stream","patch_set":1,"id":"AAAALn%2F%2F4tk%3D","line":65,"in_reply_to":"AAAALn%2F%2F5Mg%3D","updated":"2012-10-30 17:08:27.000000000","message":"A rootwrap invocation is a one-off and nothing will remain in memory across invocations.  Given that, I\u0027m not sure where we would see advantage in using session management while using a rootwrap approach to interaction with dom0.  Am I missing something?\n\nThere is nothing preventing a move away from a rootwrap approach and ensuring that commands can be executed either in-process (targeting dom0 with session management) or out-of-process (non-xs/xcp deployments).  It will take some effort to clean things up to support this, but it would definitely be preferable.  What\u0027s your timeline on adding session management to common?","commit_id":"5c7ee582780786fff457c60526f9c04226aaf22a"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"unresolved":false,"context_lines":[{"line_number":61,"context_line":"    return url, username, password"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"def run_command(url, username, password, user_args):"},{"line_number":65,"context_line":"    session \u003d XenAPI.Session(url)"},{"line_number":66,"context_line":"    session.login_with_password(username, password)"},{"line_number":67,"context_line":"    host \u003d session.xenapi.session.get_this_host(session.handle)"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"AAAALn%2F%2F0zo%3D","line":64,"updated":"2012-11-02 22:01:38.000000000","message":"Any particular reason why you didn\u0027t do any error handling when calling xenapi? For example to see that url is a proper url? I can see cases where people \u0027forget\u0027 to set the configuration file and cryptic errors come up.","commit_id":"55cb0326fb18d02bb0b3b496b91c2a8515d2d0de"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":61,"context_line":"    return url, username, password"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"def run_command(url, username, password, user_args):"},{"line_number":65,"context_line":"    session \u003d XenAPI.Session(url)"},{"line_number":66,"context_line":"    session.login_with_password(username, password)"},{"line_number":67,"context_line":"    host \u003d session.xenapi.session.get_this_host(session.handle)"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"AAAAMH%2F%2F7so%3D","line":64,"in_reply_to":"AAAALn%2F%2F0zo%3D","updated":"2012-11-15 10:55:04.000000000","message":"Done.","commit_id":"55cb0326fb18d02bb0b3b496b91c2a8515d2d0de"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":39,"context_line":"    exec_name \u003d sys.argv.pop(0)"},{"line_number":40,"context_line":"    # argv[0] required; path to conf file"},{"line_number":41,"context_line":"    if len(sys.argv) \u003c 2:"},{"line_number":42,"context_line":"        print \"%s: %s\" % (exec_name, \"No command specified\")"},{"line_number":43,"context_line":"        sys.exit(RC_NOCOMMAND)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    config_file \u003d sys.argv.pop(0)"}],"source_content_type":"application/octet-stream","patch_set":7,"id":"AAAAM3%2F%2FrMs%3D","line":42,"updated":"2013-01-09 22:57:00.000000000","message":"There\u0027s no need to use a formatter for the second parameter","commit_id":"f726979d2dbe8b7367e72bf5ed26a8112c721540"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    exec_name \u003d sys.argv.pop(0)"},{"line_number":40,"context_line":"    # argv[0] required; path to conf file"},{"line_number":41,"context_line":"    if len(sys.argv) \u003c 2:"},{"line_number":42,"context_line":"        print \"%s: %s\" % (exec_name, \"No command specified\")"},{"line_number":43,"context_line":"        sys.exit(RC_NOCOMMAND)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    config_file \u003d sys.argv.pop(0)"}],"source_content_type":"application/octet-stream","patch_set":7,"id":"AAAAM3%2F%2Fixw%3D","line":42,"in_reply_to":"AAAAM3%2F%2Fq1g%3D","updated":"2013-01-18 08:47:57.000000000","message":"Done","commit_id":"f726979d2dbe8b7367e72bf5ed26a8112c721540"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":39,"context_line":"    exec_name \u003d sys.argv.pop(0)"},{"line_number":40,"context_line":"    # argv[0] required; path to conf file"},{"line_number":41,"context_line":"    if len(sys.argv) \u003c 2:"},{"line_number":42,"context_line":"        print \"%s: %s\" % (exec_name, \"No command specified\")"},{"line_number":43,"context_line":"        sys.exit(RC_NOCOMMAND)"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    config_file \u003d sys.argv.pop(0)"}],"source_content_type":"application/octet-stream","patch_set":7,"id":"AAAAM3%2F%2Fq1g%3D","line":42,"in_reply_to":"AAAAM3%2F%2FrMs%3D","updated":"2013-01-10 08:14:52.000000000","message":"This was copy-pasted from quantum-rootwrap, but I will fix.","commit_id":"f726979d2dbe8b7367e72bf5ed26a8112c721540"},{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"unresolved":false,"context_lines":[{"line_number":1,"context_line":"#!/usr/bin/env python"},{"line_number":2,"context_line":""},{"line_number":3,"context_line":"# Copyright (c) 2012 Openstack, LLC."},{"line_number":4,"context_line":"# All Rights Reserved."}],"source_content_type":"application/octet-stream","patch_set":9,"id":"AAAAM3%2F%2FIeo%3D","line":1,"updated":"2013-02-06 02:35:26.000000000","message":"should this maybe be called rootwrap-xenapi? or rootwrap-xen-dom0?  I\u0027m wondering if dom0 isn\u0027t distinct enough to non-xen for them to know to ignore this file.","commit_id":"1d0f88db300447c64c021bb37a085edd4f52259d"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"unresolved":false,"context_lines":[{"line_number":1,"context_line":"#!/usr/bin/env python"},{"line_number":2,"context_line":""},{"line_number":3,"context_line":"# Copyright (c) 2012 Openstack, LLC."},{"line_number":4,"context_line":"# All Rights Reserved."}],"source_content_type":"application/octet-stream","patch_set":9,"id":"AAAAM3%2F%2B9SY%3D","line":1,"in_reply_to":"AAAAM3%2F%2FIeo%3D","updated":"2013-02-13 20:25:06.000000000","message":"Done.","commit_id":"1d0f88db300447c64c021bb37a085edd4f52259d"},{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"unresolved":false,"context_lines":[{"line_number":74,"context_line":"    host \u003d session.xenapi.session.get_this_host(session.handle)"},{"line_number":75,"context_line":"    result \u003d session.xenapi.host.call_plugin(host, \u0027netwrap\u0027, \u0027run_command\u0027,"},{"line_number":76,"context_line":"                                             {\u0027cmd\u0027: json.dumps(user_args)})"},{"line_number":77,"context_line":"    return json.loads(result)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"def main():"}],"source_content_type":"application/octet-stream","patch_set":9,"id":"AAAAM3%2F%2FIgA%3D","line":77,"updated":"2013-02-06 02:35:26.000000000","message":"I am not super familiar with the design principles of rootwrap, but it seems like a goal was to return a specific error code in the 90\u0027s if the error was specific to rootwrap,  rather than due to the underlying call failing.  What happens here if the login fails, or the host is unreachable?  Right now, I suspect the exception would just propagate to the top level, causing main to exit, but this doesn\u0027t seem inline with how rootwrap has traditionally done error handling.  ttx or someone with more knowledge about the goals of rootwrap design might be better to comment here.","commit_id":"1d0f88db300447c64c021bb37a085edd4f52259d"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"unresolved":false,"context_lines":[{"line_number":74,"context_line":"    host \u003d session.xenapi.session.get_this_host(session.handle)"},{"line_number":75,"context_line":"    result \u003d session.xenapi.host.call_plugin(host, \u0027netwrap\u0027, \u0027run_command\u0027,"},{"line_number":76,"context_line":"                                             {\u0027cmd\u0027: json.dumps(user_args)})"},{"line_number":77,"context_line":"    return json.loads(result)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"def main():"}],"source_content_type":"application/octet-stream","patch_set":9,"id":"AAAAM3%2F%2B9SU%3D","line":77,"in_reply_to":"AAAAM3%2F%2FIgA%3D","updated":"2013-02-13 20:25:06.000000000","message":"I\u0027ve added error handling that hopefully addresses your concern.","commit_id":"1d0f88db300447c64c021bb37a085edd4f52259d"},{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"unresolved":false,"context_lines":[{"line_number":80,"context_line":"def main():"},{"line_number":81,"context_line":"    exec_name, config_file, user_args \u003d parse_args()"},{"line_number":82,"context_line":"    url, username, password \u003d load_configuration(exec_name, config_file)"},{"line_number":83,"context_line":"    return run_command(url, username, password, user_args)"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"}],"source_content_type":"application/octet-stream","patch_set":9,"id":"AAAAM3%2F%2FIfU%3D","line":83,"updated":"2013-02-06 02:35:26.000000000","message":"Was originally confused why you didn\u0027t apply filters here, but I see you now apply them in the dom0 plugin itself.  \n\nwhat was the motivation for redoing that logic elsewhere?","commit_id":"1d0f88db300447c64c021bb37a085edd4f52259d"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"unresolved":false,"context_lines":[{"line_number":80,"context_line":"def main():"},{"line_number":81,"context_line":"    exec_name, config_file, user_args \u003d parse_args()"},{"line_number":82,"context_line":"    url, username, password \u003d load_configuration(exec_name, config_file)"},{"line_number":83,"context_line":"    return run_command(url, username, password, user_args)"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"}],"source_content_type":"application/octet-stream","patch_set":9,"id":"AAAAM3%2F%2B9SQ%3D","line":83,"in_reply_to":"AAAAM3%2F%2FGGk%3D","updated":"2013-02-13 20:25:06.000000000","message":"Agreed.  Checks on both sides are best.  Done.","commit_id":"1d0f88db300447c64c021bb37a085edd4f52259d"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"unresolved":false,"context_lines":[{"line_number":80,"context_line":"def main():"},{"line_number":81,"context_line":"    exec_name, config_file, user_args \u003d parse_args()"},{"line_number":82,"context_line":"    url, username, password \u003d load_configuration(exec_name, config_file)"},{"line_number":83,"context_line":"    return run_command(url, username, password, user_args)"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"}],"source_content_type":"application/octet-stream","patch_set":9,"id":"AAAAM3%2F%2FGGk%3D","line":83,"in_reply_to":"AAAAM3%2F%2FIfU%3D","updated":"2013-02-07 09:19:53.000000000","message":"The Dom0 checks are needed for the security of that plugin, but I guess there you could keep using the filters here too, in case the filters drift apart.","commit_id":"1d0f88db300447c64c021bb37a085edd4f52259d"},{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"unresolved":false,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"},{"line_number":87,"context_line":"    print main()"}],"source_content_type":"application/octet-stream","patch_set":9,"id":"AAAAM3%2F%2FIfw%3D","line":87,"updated":"2013-02-06 02:35:26.000000000","message":"the standard rootwrap always exits with either a rootwrap error code, or the return code of the command.  Should we be doing the same here?","commit_id":"1d0f88db300447c64c021bb37a085edd4f52259d"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"unresolved":false,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":""},{"line_number":86,"context_line":"if __name__ \u003d\u003d \u0027__main__\u0027:"},{"line_number":87,"context_line":"    print main()"}],"source_content_type":"application/octet-stream","patch_set":9,"id":"AAAAM3%2F%2B9SM%3D","line":87,"in_reply_to":"AAAAM3%2F%2FIfw%3D","updated":"2013-02-13 20:25:06.000000000","message":"This rootwrap already exits with a rootwrap error code if an error occurs (look for sys.exit).  I\u0027ve updated command invocation to do the same.","commit_id":"1d0f88db300447c64c021bb37a085edd4f52259d"}],"bin/quantum-rootwrap-xen-dom0":[{"author":{"_account_id":4395,"name":"Aaron Rosen","email":"aaronorosen@gmail.com","username":"arosen"},"change_message_id":"ac7d272ce886c2b70adbedf74a3645fa11afdec2","unresolved":false,"context_lines":[{"line_number":84,"context_line":"    if os.path.exists(os.path.join(possible_topdir, \"quantum\", \"__init__.py\")):"},{"line_number":85,"context_line":"        sys.path.insert(0, possible_topdir)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    from quantum.rootwrap import wrapper"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    # Execute command if it matches any of the loaded filters"},{"line_number":90,"context_line":"    filters \u003d wrapper.load_filters(filters_path)"}],"source_content_type":"application/octet-stream","patch_set":13,"id":"AAAAOn%2F%2FlL4%3D","line":87,"updated":"2013-05-03 01:27:19.000000000","message":"why isn\u0027t this at the top?","commit_id":"7ff0c53564f46bdd6f47cb1ed9e6d2460d088b1e"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"unresolved":false,"context_lines":[{"line_number":84,"context_line":"    if os.path.exists(os.path.join(possible_topdir, \"quantum\", \"__init__.py\")):"},{"line_number":85,"context_line":"        sys.path.insert(0, possible_topdir)"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"    from quantum.rootwrap import wrapper"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    # Execute command if it matches any of the loaded filters"},{"line_number":90,"context_line":"    filters \u003d wrapper.load_filters(filters_path)"}],"source_content_type":"application/octet-stream","patch_set":13,"id":"AAAAPH%2F%2FseY%3D","line":87,"in_reply_to":"AAAAOn%2F%2FlL4%3D","updated":"2013-05-26 07:20:25.000000000","message":"This is copied directly from quantum-rootwrap, and as per the comment on line 81, appears related to allowing this script to run from the branch.  If it needs to be changed, I would suggest filing a bug targeting both files rather than holding up this review.","commit_id":"7ff0c53564f46bdd6f47cb1ed9e6d2460d088b1e"}],"etc/rootwrap.conf":[{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"unresolved":false,"context_lines":[{"line_number":2,"context_line":"# List of directories to load filter definitions from (separated by \u0027,\u0027)."},{"line_number":3,"context_line":"# These directories MUST all be only writeable by root !"},{"line_number":4,"context_line":"filters_path\u003d/etc/quantum/rootwrap.d,/usr/share/quantum/rootwrap"},{"line_number":5,"context_line":""},{"line_number":6,"context_line":"[XENAPI]"},{"line_number":7,"context_line":"xenapi_connection_url\u003d\u003cNone\u003e"},{"line_number":8,"context_line":"xenapi_connection_username\u003droot"}],"source_content_type":"text/plain","patch_set":9,"id":"AAAAM3%2F%2FIfc%3D","line":5,"updated":"2013-02-06 02:35:26.000000000","message":"For those not familiar with the term XENAPI, we should probably add a comment making very clear that these fields are only used with the in conjunction with XenServer/XCP.","commit_id":"1d0f88db300447c64c021bb37a085edd4f52259d"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"unresolved":false,"context_lines":[{"line_number":2,"context_line":"# List of directories to load filter definitions from (separated by \u0027,\u0027)."},{"line_number":3,"context_line":"# These directories MUST all be only writeable by root !"},{"line_number":4,"context_line":"filters_path\u003d/etc/quantum/rootwrap.d,/usr/share/quantum/rootwrap"},{"line_number":5,"context_line":""},{"line_number":6,"context_line":"[XENAPI]"},{"line_number":7,"context_line":"xenapi_connection_url\u003d\u003cNone\u003e"},{"line_number":8,"context_line":"xenapi_connection_username\u003droot"}],"source_content_type":"text/plain","patch_set":9,"id":"AAAAM3%2F%2B9Sk%3D","line":5,"in_reply_to":"AAAAM3%2F%2FIfc%3D","updated":"2013-02-13 20:25:06.000000000","message":"Done.","commit_id":"1d0f88db300447c64c021bb37a085edd4f52259d"}],"etc/rootwrap_dom0.conf":[{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":1,"context_line":"[DEFAULT]"},{"line_number":2,"context_line":"xenapi_connection_url\u003d\u003cNone\u003e"},{"line_number":3,"context_line":"xenapi_connection_username\u003droot"},{"line_number":4,"context_line":"xenapi_connection_password\u003d\u003cNone\u003e"}],"source_content_type":"text/plain","patch_set":7,"id":"AAAAM3%2F%2FrMA%3D","line":1,"updated":"2013-01-09 22:57:00.000000000","message":"packaging-wise, I wonder whether it\u0027s better to create a XCP section in the main rootwrap.conf file","commit_id":"f726979d2dbe8b7367e72bf5ed26a8112c721540"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":1,"context_line":"[DEFAULT]"},{"line_number":2,"context_line":"xenapi_connection_url\u003d\u003cNone\u003e"},{"line_number":3,"context_line":"xenapi_connection_username\u003droot"},{"line_number":4,"context_line":"xenapi_connection_password\u003d\u003cNone\u003e"}],"source_content_type":"text/plain","patch_set":7,"id":"AAAAM3%2F%2Fixs%3D","line":1,"in_reply_to":"AAAAM3%2F%2Fq1c%3D","updated":"2013-01-18 08:47:57.000000000","message":"Done","commit_id":"f726979d2dbe8b7367e72bf5ed26a8112c721540"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":1,"context_line":"[DEFAULT]"},{"line_number":2,"context_line":"xenapi_connection_url\u003d\u003cNone\u003e"},{"line_number":3,"context_line":"xenapi_connection_username\u003droot"},{"line_number":4,"context_line":"xenapi_connection_password\u003d\u003cNone\u003e"}],"source_content_type":"text/plain","patch_set":7,"id":"AAAAM3%2F%2Fq1c%3D","line":1,"in_reply_to":"AAAAM3%2F%2FrMA%3D","updated":"2013-01-10 08:14:52.000000000","message":"The intention was to isolate the dom0 config from the regular config, but I\u0027m happy to make the suggested change if you would prefer.","commit_id":"f726979d2dbe8b7367e72bf5ed26a8112c721540"}],"plugins/xenserver/xenapi/etc/xapi.d/plugins/netwrap":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"29d91dfdbacdd77ddb28eb826437f303f5fdae03","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        Exception.__init__(self, *args)"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"def _run_command(cmd):"},{"line_number":43,"context_line":"    \"\"\"Abstracts out the basics of issuing system commands. If the command"},{"line_number":44,"context_line":"    returns anything in stderr, a PluginError is raised with that information."},{"line_number":45,"context_line":"    Otherwise, the output from stdout is returned."}],"source_content_type":"application/octet-stream","patch_set":1,"id":"AAAALn%2F%2F5MY%3D","line":42,"updated":"2012-10-30 12:48:00.000000000","message":"It would be nice to share code with nova plugin, but maybe that is a pipe dream.","commit_id":"5c7ee582780786fff457c60526f9c04226aaf22a"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":39,"context_line":"        Exception.__init__(self, *args)"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"def _run_command(cmd):"},{"line_number":43,"context_line":"    \"\"\"Abstracts out the basics of issuing system commands. If the command"},{"line_number":44,"context_line":"    returns anything in stderr, a PluginError is raised with that information."},{"line_number":45,"context_line":"    Otherwise, the output from stdout is returned."}],"source_content_type":"application/octet-stream","patch_set":1,"id":"AAAALn%2F%2F4tg%3D","line":42,"in_reply_to":"AAAALn%2F%2F5MY%3D","updated":"2012-10-30 17:08:27.000000000","message":"I agree with your sentiment, but when I asked markmcclain whether quantum should depend on nova (which would allow the code sharing you speak of), but he indicated that it was preferable to allow quantum to be independently distributed.","commit_id":"5c7ee582780786fff457c60526f9c04226aaf22a"}],"quantum/agent/linux/ip_lib.py":[{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":31,"context_line":"            return self._as_root(options, command, args)"},{"line_number":32,"context_line":"        else:"},{"line_number":33,"context_line":"            # Use the root helper by default to ensure that commands"},{"line_number":34,"context_line":"            # can execute in dom0 when running under XS/XCP."},{"line_number":35,"context_line":"            return self._execute(options, command, args, self.root_helper)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def _as_root(self, options, command, args, use_root_namespace\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2FrLw%3D","line":34,"updated":"2013-01-09 22:57:00.000000000","message":"I guess that does not matter for the namespace case as they\u0027re not supported in dom0?","commit_id":"f726979d2dbe8b7367e72bf5ed26a8112c721540"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":31,"context_line":"            return self._as_root(options, command, args)"},{"line_number":32,"context_line":"        else:"},{"line_number":33,"context_line":"            # Use the root helper by default to ensure that commands"},{"line_number":34,"context_line":"            # can execute in dom0 when running under XS/XCP."},{"line_number":35,"context_line":"            return self._execute(options, command, args, self.root_helper)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def _as_root(self, options, command, args, use_root_namespace\u003dFalse):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2Fq1U%3D","line":34,"in_reply_to":"AAAAM3%2F%2FrLw%3D","updated":"2013-01-10 08:14:52.000000000","message":"As of XCP 1.6, namespace support is lacking in the default dom0 kernel.  If dom0 starts supporting namespaces, though, dom0 support will work as expected since _as_root() uses the root helper by default.","commit_id":"f726979d2dbe8b7367e72bf5ed26a8112c721540"},{"author":{"_account_id":2592,"name":"Mark McClain","email":"mark@mcclain.xyz","username":"markmcclain"},"change_message_id":"b75ed403941957e63b947621fd7e75d9ebee1289","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        else:"},{"line_number":33,"context_line":"            # Use the root helper by default to ensure that commands"},{"line_number":34,"context_line":"            # can execute in dom0 when running under XS/XCP."},{"line_number":35,"context_line":"            return self._execute(options, command, args, self.root_helper)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def _as_root(self, options, command, args, use_root_namespace\u003dFalse):"},{"line_number":38,"context_line":"        if not self.root_helper:"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2FIWc%3D","line":35,"updated":"2013-02-06 04:51:59.000000000","message":"Adding root helper here seems to force all command execution to root just be defining it in non-xen uses.  Would it be better to add a flag like \u0027force_root\u0027 to base init method to be explicit that even normally unprivileged commands should be run as root?","commit_id":"1d0f88db300447c64c021bb37a085edd4f52259d"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"unresolved":false,"context_lines":[{"line_number":32,"context_line":"        else:"},{"line_number":33,"context_line":"            # Use the root helper by default to ensure that commands"},{"line_number":34,"context_line":"            # can execute in dom0 when running under XS/XCP."},{"line_number":35,"context_line":"            return self._execute(options, command, args, self.root_helper)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def _as_root(self, options, command, args, use_root_namespace\u003dFalse):"},{"line_number":38,"context_line":"        if not self.root_helper:"}],"source_content_type":"text/x-python","patch_set":9,"id":"AAAAM3%2F%2B9Ts%3D","line":35,"in_reply_to":"AAAAM3%2F%2FIWc%3D","updated":"2013-02-13 20:25:06.000000000","message":"Done.","commit_id":"1d0f88db300447c64c021bb37a085edd4f52259d"}],"quantum/agent/linux/ovs_lib.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"29d91dfdbacdd77ddb28eb826437f303f5fdae03","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        self.br_name \u003d br_name"},{"line_number":47,"context_line":"        self.root_helper \u003d root_helper"},{"line_number":48,"context_line":"        self.re_id \u003d self.re_compile_id()"},{"line_number":49,"context_line":"        self.in_dom0 \u003d in_dom0"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def re_compile_id(self):"},{"line_number":52,"context_line":"        external \u003d \u0027external_ids\\s*\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2F5Mk%3D","line":49,"updated":"2012-10-30 12:48:00.000000000","message":"Not sure why we need this option here, could this not be a configuration option?","commit_id":"5c7ee582780786fff457c60526f9c04226aaf22a"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        self.br_name \u003d br_name"},{"line_number":47,"context_line":"        self.root_helper \u003d root_helper"},{"line_number":48,"context_line":"        self.re_id \u003d self.re_compile_id()"},{"line_number":49,"context_line":"        self.in_dom0 \u003d in_dom0"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    def re_compile_id(self):"},{"line_number":52,"context_line":"        external \u003d \u0027external_ids\\s*\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2F4tU%3D","line":49,"in_reply_to":"AAAALn%2F%2F5Mk%3D","updated":"2012-10-30 17:08:27.000000000","message":"It is a configuration option.  It is set in ovs_quantum_plugin.ini, provided to the agent, and then provided to this class on bridge instantiation by the agent.","commit_id":"5c7ee582780786fff457c60526f9c04226aaf22a"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":292,"context_line":"      )"},{"line_number":293,"context_line":"    \"\"\", re.VERBOSE)"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"    def get_port_mac_map(self):"},{"line_number":296,"context_line":"        \"\"\"Return a map of port names to mac addresses for the bridge.\"\"\""},{"line_number":297,"context_line":"        # ofctl will be executed local to the bridge via a normal or"},{"line_number":298,"context_line":"        # dom0 root wrapper."}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2FrIs%3D","line":295,"updated":"2013-01-09 22:57:00.000000000","message":"We always bring up the local port interfaces, so you should be able to see them with ip.\nHave you tried using iplib for getting the mac address, instead of ofctl?","commit_id":"f726979d2dbe8b7367e72bf5ed26a8112c721540"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":292,"context_line":"      )"},{"line_number":293,"context_line":"    \"\"\", re.VERBOSE)"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"    def get_port_mac_map(self):"},{"line_number":296,"context_line":"        \"\"\"Return a map of port names to mac addresses for the bridge.\"\"\""},{"line_number":297,"context_line":"        # ofctl will be executed local to the bridge via a normal or"},{"line_number":298,"context_line":"        # dom0 root wrapper."}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2Fiqs%3D","line":295,"in_reply_to":"AAAAM3%2F%2FqxA%3D","updated":"2013-01-18 08:47:57.000000000","message":"Done","commit_id":"f726979d2dbe8b7367e72bf5ed26a8112c721540"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":292,"context_line":"      )"},{"line_number":293,"context_line":"    \"\"\", re.VERBOSE)"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"    def get_port_mac_map(self):"},{"line_number":296,"context_line":"        \"\"\"Return a map of port names to mac addresses for the bridge.\"\"\""},{"line_number":297,"context_line":"        # ofctl will be executed local to the bridge via a normal or"},{"line_number":298,"context_line":"        # dom0 root wrapper."}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2FqxA%3D","line":295,"in_reply_to":"AAAAM3%2F%2FrIs%3D","updated":"2013-01-10 08:14:52.000000000","message":"I agree that iplib should be used for finding the local port mac as you suggest, and will make that change.\n\nI think the current implementation is an artifact of having written the l2 and dhcp agent support and then splitting them into separate reviews.  get_port_mac_map() was written with the intent of finding the macs for potential uplink ports, and I reused that functionality for finding the local port mac without looking for a simpler solution.","commit_id":"f726979d2dbe8b7367e72bf5ed26a8112c721540"},{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":311,"context_line":"            return mac"},{"line_number":312,"context_line":"        else:"},{"line_number":313,"context_line":"            msg \u003d \u0027Unable to determine mac address for %s\u0027 % self.br_name"},{"line_number":314,"context_line":"            raise Exception(msg)"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"def get_bridge_for_iface(root_helper, iface):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2FrLg%3D","line":314,"updated":"2013-01-09 22:57:00.000000000","message":"I\u0027m the last guy on earth that could tell you about Pythonism, but have you considered the following:\n\ntry:\n    return port_mac_map[self.br_name]\nexcept Keyerror:\n   rise(\u0027Unable to determine mac address for %s\u0027 %  self.br_name)","commit_id":"f726979d2dbe8b7367e72bf5ed26a8112c721540"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":311,"context_line":"            return mac"},{"line_number":312,"context_line":"        else:"},{"line_number":313,"context_line":"            msg \u003d \u0027Unable to determine mac address for %s\u0027 % self.br_name"},{"line_number":314,"context_line":"            raise Exception(msg)"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"def get_bridge_for_iface(root_helper, iface):"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2Fq1I%3D","line":314,"in_reply_to":"AAAAM3%2F%2FrLg%3D","updated":"2013-01-10 08:14:52.000000000","message":"It\u0027s my preference to avoid looking for exceptions where conditional tests are an option, but I can use the suggested approach if you would prefer.","commit_id":"f726979d2dbe8b7367e72bf5ed26a8112c721540"}],"quantum/plugins/openvswitch/agent/ovs_quantum_agent.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"29d91dfdbacdd77ddb28eb826437f303f5fdae03","unresolved":false,"context_lines":[{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def setup_rpc(self):"},{"line_number":170,"context_line":"        mac \u003d self.int_br.get_local_port_mac()"},{"line_number":171,"context_line":"        self.agent_id \u003d \u0027%s%s\u0027 % (\u0027ovs\u0027, (mac.replace(\":\", \"\")))"},{"line_number":172,"context_line":"        self.topic \u003d topics.AGENT"},{"line_number":173,"context_line":"        self.plugin_rpc \u003d agent_rpc.PluginApi(topics.PLUGIN)"},{"line_number":174,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2F5MM%3D","line":171,"updated":"2012-10-30 12:48:00.000000000","message":"Does getting the different mac address matter here? We just need a consistent unique id for this service, presumably? Or does it mean more than that in Quantum land?","commit_id":"5c7ee582780786fff457c60526f9c04226aaf22a"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def setup_rpc(self):"},{"line_number":170,"context_line":"        mac \u003d self.int_br.get_local_port_mac()"},{"line_number":171,"context_line":"        self.agent_id \u003d \u0027%s%s\u0027 % (\u0027ovs\u0027, (mac.replace(\":\", \"\")))"},{"line_number":172,"context_line":"        self.topic \u003d topics.AGENT"},{"line_number":173,"context_line":"        self.plugin_rpc \u003d agent_rpc.PluginApi(topics.PLUGIN)"},{"line_number":174,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2F4tE%3D","line":171,"in_reply_to":"AAAALn%2F%2F5MM%3D","updated":"2012-10-30 17:08:27.000000000","message":"I\u0027m just being consistent.  I needed to support discovering the mac addresses of interfaces associated with a bridge to ensure that the agent would avoid molesting the domU interface plugged into the dom0 bridge, so it was an easy change to ensure that the agent id was based on the mac of the primary bridge regardless of where the bridge was located.\n\nQuantum devs - thoughts?","commit_id":"5c7ee582780786fff457c60526f9c04226aaf22a"}],"quantum/plugins/openvswitch/agent/xenapi/contrib/rpmbuild/SPECS/openstack-quantum-xen-plugins.spec":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"unresolved":false,"context_lines":[{"line_number":1,"context_line":"Name:           openstack-quantum-xen-plugins"},{"line_number":2,"context_line":"Version:        2013.1"},{"line_number":3,"context_line":"Release:        1"},{"line_number":4,"context_line":"Summary:        Files for XenAPI support."},{"line_number":5,"context_line":"License:        ASL 2.0"},{"line_number":6,"context_line":"Group:          Applications/Utilities"}],"source_content_type":"text/x-rpm-spec","patch_set":3,"id":"AAAALn%2F%2F0zY%3D","line":3,"updated":"2012-11-02 22:01:38.000000000","message":"you might want to get Version and Release from the builder script.","commit_id":"55cb0326fb18d02bb0b3b496b91c2a8515d2d0de"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Name:           openstack-quantum-xen-plugins"},{"line_number":2,"context_line":"Version:        2013.1"},{"line_number":3,"context_line":"Release:        1"},{"line_number":4,"context_line":"Summary:        Files for XenAPI support."},{"line_number":5,"context_line":"License:        ASL 2.0"},{"line_number":6,"context_line":"Group:          Applications/Utilities"}],"source_content_type":"text/x-rpm-spec","patch_set":3,"id":"AAAAMH%2F%2F7sM%3D","line":3,"in_reply_to":"AAAALn%2F%2F0zY%3D","updated":"2012-11-15 10:55:04.000000000","message":"I\u0027m no an RPM guru, but does your suggestion imply hard-coding this info, but in a different place?","commit_id":"55cb0326fb18d02bb0b3b496b91c2a8515d2d0de"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"unresolved":false,"context_lines":[{"line_number":1,"context_line":"Name:           openstack-quantum-xen-plugins"},{"line_number":2,"context_line":"Version:        2013.1"},{"line_number":3,"context_line":"Release:        1"},{"line_number":4,"context_line":"Summary:        Files for XenAPI support."},{"line_number":5,"context_line":"License:        ASL 2.0"},{"line_number":6,"context_line":"Group:          Applications/Utilities"}],"source_content_type":"text/x-rpm-spec","patch_set":3,"id":"AAAAMH%2F%2F7GE%3D","line":3,"in_reply_to":"AAAAMH%2F%2F7sM%3D","updated":"2012-11-15 18:15:38.000000000","message":"we already have version information in the source tree; we could use that one. I can show you how to do it.","commit_id":"55cb0326fb18d02bb0b3b496b91c2a8515d2d0de"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"unresolved":false,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"%files"},{"line_number":29,"context_line":"%defattr(-,root,root,-)"},{"line_number":30,"context_line":"/etc/xapi.d/plugins/netwrap"}],"source_content_type":"text/x-rpm-spec","patch_set":3,"id":"AAAALn%2F%2F0zA%3D","line":30,"updated":"2012-11-02 22:01:38.000000000","message":"if we add more plugins in the source tree, the spec breaks. As we don\u0027t have CI in place to catch that I wonder if it\u0027s safer to use * instead of \u0027netwrap\u0027","commit_id":"55cb0326fb18d02bb0b3b496b91c2a8515d2d0de"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"%files"},{"line_number":29,"context_line":"%defattr(-,root,root,-)"},{"line_number":30,"context_line":"/etc/xapi.d/plugins/netwrap"}],"source_content_type":"text/x-rpm-spec","patch_set":3,"id":"AAAAMH%2F%2F7n8%3D","line":30,"in_reply_to":"AAAALn%2F%2F0zA%3D","updated":"2012-11-15 10:55:04.000000000","message":"I\u0027m following Nova\u0027s example, and I\u0027m pretty sure the reason for not specifying * is that these attributes are set at installation time and this rpm should only target files it installs rather than the entire plugins directory.","commit_id":"55cb0326fb18d02bb0b3b496b91c2a8515d2d0de"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"unresolved":false,"context_lines":[{"line_number":27,"context_line":""},{"line_number":28,"context_line":"%files"},{"line_number":29,"context_line":"%defattr(-,root,root,-)"},{"line_number":30,"context_line":"/etc/xapi.d/plugins/netwrap"}],"source_content_type":"text/x-rpm-spec","patch_set":3,"id":"AAAAMH%2F%2F7Fs%3D","line":30,"in_reply_to":"AAAAMH%2F%2F7n8%3D","updated":"2012-11-15 18:15:38.000000000","message":"the %files macro is used to list the files to include (with the right attrs) in the package. If you look a little earlier, you do change the attributes for all the plugins in the directory already, so I am not sure I follow your point. Even if you specified * you are still targeting just the files that pertain this package.","commit_id":"55cb0326fb18d02bb0b3b496b91c2a8515d2d0de"}],"quantum/plugins/openvswitch/agent/xenapi/etc/xapi.d/plugins/netwrap":[{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":61,"context_line":"def run_command(session, args):"},{"line_number":62,"context_line":"    cmd \u003d json.loads(args.get(\u0027cmd\u0027))"},{"line_number":63,"context_line":"    if cmd and cmd[0] not in ALLOWED_CMDS:"},{"line_number":64,"context_line":"        msg \u003d \"Dom0 execution of \u0027%s\u0027 is not permitted\" % cmd[0]"},{"line_number":65,"context_line":"        raise PluginError(msg)"},{"line_number":66,"context_line":"    result \u003d _run_command(cmd)"},{"line_number":67,"context_line":"    return json.dumps(result)"}],"source_content_type":"application/octet-stream","patch_set":7,"id":"AAAAM3%2F%2FrIY%3D","line":64,"updated":"2013-01-09 22:57:00.000000000","message":"All messages that are printed to output should be internationalized with gettext. Is that applicable to dom0 as well?","commit_id":"f726979d2dbe8b7367e72bf5ed26a8112c721540"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":61,"context_line":"def run_command(session, args):"},{"line_number":62,"context_line":"    cmd \u003d json.loads(args.get(\u0027cmd\u0027))"},{"line_number":63,"context_line":"    if cmd and cmd[0] not in ALLOWED_CMDS:"},{"line_number":64,"context_line":"        msg \u003d \"Dom0 execution of \u0027%s\u0027 is not permitted\" % cmd[0]"},{"line_number":65,"context_line":"        raise PluginError(msg)"},{"line_number":66,"context_line":"    result \u003d _run_command(cmd)"},{"line_number":67,"context_line":"    return json.dumps(result)"}],"source_content_type":"application/octet-stream","patch_set":7,"id":"AAAAM3%2F%2FisY%3D","line":64,"in_reply_to":"AAAAM3%2F%2Fq0A%3D","updated":"2013-01-18 08:47:57.000000000","message":"I\u0027ve i18n\u0027d the text, as per the nova model.  As with nova, quantum\u0027s xenapi plugin will allow localization but not ship with any.","commit_id":"f726979d2dbe8b7367e72bf5ed26a8112c721540"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":61,"context_line":"def run_command(session, args):"},{"line_number":62,"context_line":"    cmd \u003d json.loads(args.get(\u0027cmd\u0027))"},{"line_number":63,"context_line":"    if cmd and cmd[0] not in ALLOWED_CMDS:"},{"line_number":64,"context_line":"        msg \u003d \"Dom0 execution of \u0027%s\u0027 is not permitted\" % cmd[0]"},{"line_number":65,"context_line":"        raise PluginError(msg)"},{"line_number":66,"context_line":"    result \u003d _run_command(cmd)"},{"line_number":67,"context_line":"    return json.dumps(result)"}],"source_content_type":"application/octet-stream","patch_set":7,"id":"AAAAM3%2F%2Fq0A%3D","line":64,"in_reply_to":"AAAAM3%2F%2FrIY%3D","updated":"2013-01-10 08:14:52.000000000","message":"Good catch.  I was probably avoiding doing this subconsciously, but all text in quantum code should be i18n regardless of execution environment.\n\nI\u0027ve used nova\u0027s xenapi plugins as a guide to this implementation, and there is a mix of i18n/non-i18n text defined.  Curiously, though gettext is configured by the nova plugins, no translation files appear to be distributed with the rpm, so even though some text is i18n, it doesn\u0027t appear that localization is possible.\n\nShould the quantum xenapi plugin rpm be held to a higher standard, or would you be satisfied with the nova approach of i18n\u0027d code without translation file distribution?","commit_id":"f726979d2dbe8b7367e72bf5ed26a8112c721540"},{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"unresolved":false,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"ALLOWED_CMDS \u003d ["},{"line_number":36,"context_line":"    \u0027ip\u0027,"},{"line_number":37,"context_line":"    \u0027ovs-ofctl\u0027,"},{"line_number":38,"context_line":"    \u0027ovs-vsctl\u0027,"},{"line_number":39,"context_line":"    ]"}],"source_content_type":"application/octet-stream","patch_set":9,"id":"AAAAM3%2F%2FIfA%3D","line":36,"updated":"2013-02-06 02:35:26.000000000","message":"in traditional rootwrap, we had issue because the \u0027ip netns xxx exec\u0027 command was a loophole that allowed running all kinds of other commands.  Seems like we may need to worry about that here too.","commit_id":"1d0f88db300447c64c021bb37a085edd4f52259d"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"unresolved":false,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"ALLOWED_CMDS \u003d ["},{"line_number":36,"context_line":"    \u0027ip\u0027,"},{"line_number":37,"context_line":"    \u0027ovs-ofctl\u0027,"},{"line_number":38,"context_line":"    \u0027ovs-vsctl\u0027,"},{"line_number":39,"context_line":"    ]"}],"source_content_type":"application/octet-stream","patch_set":9,"id":"AAAAM3%2F%2B9TI%3D","line":36,"in_reply_to":"AAAAM3%2F%2FGGQ%3D","updated":"2013-02-13 20:25:06.000000000","message":"The default kernels in the current versions of XS/XCP do not support ip namespaces as far as I know, but I agree this should be addressed.  I\u0027m afraid I\u0027ve run out of time to do it in the follow-on patch, though.  Would you like to have a bug to ensure this gets fixed in the future?","commit_id":"1d0f88db300447c64c021bb37a085edd4f52259d"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"unresolved":false,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"ALLOWED_CMDS \u003d ["},{"line_number":36,"context_line":"    \u0027ip\u0027,"},{"line_number":37,"context_line":"    \u0027ovs-ofctl\u0027,"},{"line_number":38,"context_line":"    \u0027ovs-vsctl\u0027,"},{"line_number":39,"context_line":"    ]"}],"source_content_type":"application/octet-stream","patch_set":9,"id":"AAAAM3%2F%2FGGQ%3D","line":36,"in_reply_to":"AAAAM3%2F%2FIfA%3D","updated":"2013-02-07 09:19:53.000000000","message":"ouch, probably should.","commit_id":"1d0f88db300447c64c021bb37a085edd4f52259d"},{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"unresolved":false,"context_lines":[{"line_number":70,"context_line":""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"if __name__ \u003d\u003d \"__main__\":"},{"line_number":73,"context_line":"    XenAPIPlugin.dispatch({\"run_command\": run_command})"}],"source_content_type":"application/octet-stream","patch_set":9,"id":"AAAAM3%2F%2FIe8%3D","line":73,"updated":"2013-02-06 02:35:26.000000000","message":"I don\u0027t really have my head wrapped around what XenAPIPlugin.dispatch() does... will it convert a PluginError exception into json?","commit_id":"1d0f88db300447c64c021bb37a085edd4f52259d"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"unresolved":false,"context_lines":[{"line_number":70,"context_line":""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"if __name__ \u003d\u003d \"__main__\":"},{"line_number":73,"context_line":"    XenAPIPlugin.dispatch({\"run_command\": run_command})"}],"source_content_type":"application/octet-stream","patch_set":9,"id":"AAAAM3%2F%2B5EQ%3D","line":73,"in_reply_to":"AAAAM3%2F%2B8vA%3D","updated":"2013-02-15 11:41:58.000000000","message":"The dispatch method of XenAPIPlugin serializes any output coming from the upcall method into the xmlrpc response in the following form:\n\n[\u0027XENAPI_PLUGIN_FAILURE\u0027, methodname, e.__class__.__name__, str(e)])\n\nwhere methodname is the name of the upcall and e is the exception raised.\n\nSo, for example, if the method named \u0027foo\u0027 raises the Exception \u0027PluginError(\u0027ouch, something went wrong!\u0027), the xmlrpc caller will see a XenAPI.Failure raised with the following content:\n\n[\u0027XENAPI_PLUGIN_FAILURE\u0027, \u0027foo\u0027, \u0027PluginError\u0027, \u0027ouch, something went wrong!\u0027]","commit_id":"1d0f88db300447c64c021bb37a085edd4f52259d"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"unresolved":false,"context_lines":[{"line_number":70,"context_line":""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"if __name__ \u003d\u003d \"__main__\":"},{"line_number":73,"context_line":"    XenAPIPlugin.dispatch({\"run_command\": run_command})"}],"source_content_type":"application/octet-stream","patch_set":9,"id":"AAAAM3%2F%2B8vA%3D","line":73,"in_reply_to":"AAAAM3%2F%2FIe8%3D","updated":"2013-02-13 20:25:06.000000000","message":"Armando and John could provide specifics, but XenAPI does communicate (and log) exceptions to the caller.","commit_id":"1d0f88db300447c64c021bb37a085edd4f52259d"},{"author":{"_account_id":5044,"name":"Mate Lakat","email":"mlakat@suse.com","username":"mate-lakat"},"change_message_id":"ca24674c51f68feac2f7b255f6db646bb76067bb","unresolved":false,"context_lines":[{"line_number":35,"context_line":"ALLOWED_CMDS \u003d ["},{"line_number":36,"context_line":"    \u0027ip\u0027,"},{"line_number":37,"context_line":"    \u0027ovs-ofctl\u0027,"},{"line_number":38,"context_line":"    \u0027ovs-vsctl\u0027,"},{"line_number":39,"context_line":"    ]"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":13,"id":"AAAAOn%2F%2FbrQ%3D","line":38,"updated":"2013-05-09 20:21:56.000000000","message":"I had to put iptables-save and iptables-restore here as well.","commit_id":"7ff0c53564f46bdd6f47cb1ed9e6d2460d088b1e"},{"author":{"_account_id":5044,"name":"Mate Lakat","email":"mlakat@suse.com","username":"mate-lakat"},"change_message_id":"ca24674c51f68feac2f7b255f6db646bb76067bb","unresolved":false,"context_lines":[{"line_number":35,"context_line":"ALLOWED_CMDS \u003d ["},{"line_number":36,"context_line":"    \u0027ip\u0027,"},{"line_number":37,"context_line":"    \u0027ovs-ofctl\u0027,"},{"line_number":38,"context_line":"    \u0027ovs-vsctl\u0027,"},{"line_number":39,"context_line":"    ]"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":13,"id":"AAAAO3%2F%2F%2B38%3D","line":38,"in_reply_to":"AAAAOn%2F%2FbrQ%3D","updated":"2013-05-15 16:02:22.000000000","message":"Because I forgot to turn off security groups, so it is good as it is.","commit_id":"7ff0c53564f46bdd6f47cb1ed9e6d2460d088b1e"}],"quantum/plugins/openvswitch/common/config.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"29d91dfdbacdd77ddb28eb826437f303f5fdae03","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"ovs_opts \u003d ["},{"line_number":31,"context_line":"    cfg.StrOpt(\u0027integration_bridge\u0027, default\u003d\u0027br-int\u0027),"},{"line_number":32,"context_line":"    cfg.BoolOpt(\u0027integration_bridge_in_dom0\u0027, default\u003dFalse),"},{"line_number":33,"context_line":"    cfg.BoolOpt(\u0027enable_tunneling\u0027, default\u003dFalse),"},{"line_number":34,"context_line":"    cfg.StrOpt(\u0027tunnel_bridge\u0027, default\u003d\u0027br-tun\u0027),"},{"line_number":35,"context_line":"    cfg.StrOpt(\u0027local_ip\u0027, default\u003d\u0027\u0027),"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2F5MI%3D","line":32,"updated":"2012-10-30 12:48:00.000000000","message":"I am not sure I have seen that being used here?","commit_id":"5c7ee582780786fff457c60526f9c04226aaf22a"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"ovs_opts \u003d ["},{"line_number":31,"context_line":"    cfg.StrOpt(\u0027integration_bridge\u0027, default\u003d\u0027br-int\u0027),"},{"line_number":32,"context_line":"    cfg.BoolOpt(\u0027integration_bridge_in_dom0\u0027, default\u003dFalse),"},{"line_number":33,"context_line":"    cfg.BoolOpt(\u0027enable_tunneling\u0027, default\u003dFalse),"},{"line_number":34,"context_line":"    cfg.StrOpt(\u0027tunnel_bridge\u0027, default\u003d\u0027br-tun\u0027),"},{"line_number":35,"context_line":"    cfg.StrOpt(\u0027local_ip\u0027, default\u003d\u0027\u0027),"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2F4tA%3D","line":32,"in_reply_to":"AAAALn%2F%2F5MI%3D","updated":"2012-10-30 17:08:27.000000000","message":"It is being used: See https://review.openstack.org/#/c/15022/1/quantum/plugins/openvswitch/agent/ovs_quantum_agent.py","commit_id":"5c7ee582780786fff457c60526f9c04226aaf22a"}],"quantum/tests/unit/openvswitch/test_ovs_tunnel.py":[{"author":{"_account_id":261,"name":"Salvatore Orlando","email":"salv.orlando@gmail.com","username":"salvatore-orlando"},"unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        self.mox.StubOutWithMock(ovs_quantum_agent.OVSQuantumAgent,"},{"line_number":79,"context_line":"                                 \u0027setup_rpc\u0027)"},{"line_number":80,"context_line":"        ovs_quantum_agent.OVSQuantumAgent.setup_rpc()"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def tearDown(self):"},{"line_number":83,"context_line":"        self.mox.UnsetStubs()"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2FrIQ%3D","line":80,"updated":"2013-01-09 22:57:00.000000000","message":"I wonder if this is not reducing coverage. Can you just stub the function that you are now using in place of get_interface_mac (the one which executes ovs-ofctl)","commit_id":"f726979d2dbe8b7367e72bf5ed26a8112c721540"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        self.mox.StubOutWithMock(ovs_quantum_agent.OVSQuantumAgent,"},{"line_number":79,"context_line":"                                 \u0027setup_rpc\u0027)"},{"line_number":80,"context_line":"        ovs_quantum_agent.OVSQuantumAgent.setup_rpc()"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def tearDown(self):"},{"line_number":83,"context_line":"        self.mox.UnsetStubs()"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2Fiq8%3D","line":80,"in_reply_to":"AAAAM3%2F%2Fqr8%3D","updated":"2013-01-18 08:47:57.000000000","message":"Done","commit_id":"f726979d2dbe8b7367e72bf5ed26a8112c721540"},{"author":{"_account_id":2035,"name":"Maru Newby","email":"marun@redhat.com","username":"maru"},"change_message_id":"4206a017ca4f37e6ece4c65b71ee1094c329db66","unresolved":false,"context_lines":[{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        self.mox.StubOutWithMock(ovs_quantum_agent.OVSQuantumAgent,"},{"line_number":79,"context_line":"                                 \u0027setup_rpc\u0027)"},{"line_number":80,"context_line":"        ovs_quantum_agent.OVSQuantumAgent.setup_rpc()"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"    def tearDown(self):"},{"line_number":83,"context_line":"        self.mox.UnsetStubs()"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAM3%2F%2Fqr8%3D","line":80,"in_reply_to":"AAAAM3%2F%2FrIQ%3D","updated":"2013-01-10 08:14:52.000000000","message":"Good catch.  I couldn\u0027t seem to run this test module in isolation without stubbing out setup_rpc(), but tox runs fine when only the mac retrieval call is stubbed out.","commit_id":"f726979d2dbe8b7367e72bf5ed26a8112c721540"}]}