)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"be1464e33d9a2dc1d2ce0eeca1a66b3edaec006d","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add the support for updating the status of the host."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We added a function into the workarounds plugin to get information"},{"line_number":10,"context_line":"about PCI devices that are passed on the pciback.hide dom0 command line."},{"line_number":11,"context_line":"Then we check if devices that are hidden from dom0 are available on the"},{"line_number":12,"context_line":"host and we retrieve information like vendor_id and product-id. This"},{"line_number":13,"context_line":"information will be used by the xenapi driver to match with the list"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"AAAAUH%2F%2FyME%3D","line":10,"updated":"2014-01-08 16:16:52.000000000","message":"I\u0027d like to see some brief details of _why_ we need to do things like check the pciback.hide command line.  If this doesn\u0027t fit in the commit message we can link to an external website (even if it\u0027s just the xen.org site).  Perhaps a link to the etherpad would be sufficient - but people are likely to be curious about this when they come to review it.","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"4bab7202dd897d53571606a3562d1733e0acd529","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add the support for updating the status of the host."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We added a function into the workarounds plugin to get information"},{"line_number":10,"context_line":"about PCI devices that are passed on the pciback.hide dom0 command line."},{"line_number":11,"context_line":"Then we check if devices that are hidden from dom0 are available on the"},{"line_number":12,"context_line":"host and we retrieve information like vendor_id and product-id. This"},{"line_number":13,"context_line":"information will be used by the xenapi driver to match with the list"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"AAAAUH%2F%2FtuI%3D","line":10,"in_reply_to":"AAAAUH%2F%2FyME%3D","updated":"2014-01-09 11:08:28.000000000","message":"I added the following explanation:\n\"The hide option is used to hide the devices from the normal guest drivers and assign them to pciback at boot on dom0 instead of their normal driver.\"","commit_id":"c15812575064e45110d107f116ba11b6431bc575"}],"nova/tests/virt/xenapi/test_xenapi.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d6e14e91dec799c7deffe4564dc4781315bb264a","unresolved":false,"context_lines":[{"line_number":2027,"context_line":"                          [\u0027[{\"vendor_id\":\"10de\",\"product_id\":\"11bf\"}]\u0027])"},{"line_number":2028,"context_line":"        # NOTE (guillaume-thouvenin): This pci whitelist will be used to"},{"line_number":2029,"context_line":"        # match with _plugin_xenhost_get_pci_device_details method in fake.py."},{"line_number":2030,"context_line":"        self.useFixture(conf_fixture.ConfFixture(CONF))"},{"line_number":2031,"context_line":"        stats \u003d self.conn.get_host_stats()"},{"line_number":2032,"context_line":"        self.assertEqual(len(stats[\u0027pci_passthrough_devices\u0027]), 1)"},{"line_number":2033,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BEyo%3D","line":2030,"updated":"2014-01-30 12:30:20.000000000","message":"please use something like this instead:\n\nself.flags(pci_passthrough_whitelist\u003d                [\u0027[{\"vendor_id\":\"10de\",\"product_id\":\"11bf\"}]\u0027])","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"07afbc4389f5d67ccf5d9673ba301caf1b275227","unresolved":false,"context_lines":[{"line_number":2027,"context_line":"                          [\u0027[{\"vendor_id\":\"10de\",\"product_id\":\"11bf\"}]\u0027])"},{"line_number":2028,"context_line":"        # NOTE (guillaume-thouvenin): This pci whitelist will be used to"},{"line_number":2029,"context_line":"        # match with _plugin_xenhost_get_pci_device_details method in fake.py."},{"line_number":2030,"context_line":"        self.useFixture(conf_fixture.ConfFixture(CONF))"},{"line_number":2031,"context_line":"        stats \u003d self.conn.get_host_stats()"},{"line_number":2032,"context_line":"        self.assertEqual(len(stats[\u0027pci_passthrough_devices\u0027]), 1)"},{"line_number":2033,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BEmA%3D","line":2030,"in_reply_to":"AAAAUH%2F%2BEyo%3D","updated":"2014-01-30 14:10:34.000000000","message":"Cool I don\u0027t know that.\nDone","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d6e14e91dec799c7deffe4564dc4781315bb264a","unresolved":false,"context_lines":[{"line_number":2029,"context_line":"        # match with _plugin_xenhost_get_pci_device_details method in fake.py."},{"line_number":2030,"context_line":"        self.useFixture(conf_fixture.ConfFixture(CONF))"},{"line_number":2031,"context_line":"        stats \u003d self.conn.get_host_stats()"},{"line_number":2032,"context_line":"        self.assertEqual(len(stats[\u0027pci_passthrough_devices\u0027]), 1)"},{"line_number":2033,"context_line":""},{"line_number":2034,"context_line":"    def test_host_state_missing_sr(self):"},{"line_number":2035,"context_line":"        def fake_safe_find_sr(session):"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BEzY%3D","line":2032,"updated":"2014-01-30 12:30:20.000000000","message":"I would love to see my suggested inner function explicitly fed values, particular to check the error cases.\n\nHowever this is a nice end to end test. Good work.","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"07afbc4389f5d67ccf5d9673ba301caf1b275227","unresolved":false,"context_lines":[{"line_number":2029,"context_line":"        # match with _plugin_xenhost_get_pci_device_details method in fake.py."},{"line_number":2030,"context_line":"        self.useFixture(conf_fixture.ConfFixture(CONF))"},{"line_number":2031,"context_line":"        stats \u003d self.conn.get_host_stats()"},{"line_number":2032,"context_line":"        self.assertEqual(len(stats[\u0027pci_passthrough_devices\u0027]), 1)"},{"line_number":2033,"context_line":""},{"line_number":2034,"context_line":"    def test_host_state_missing_sr(self):"},{"line_number":2035,"context_line":"        def fake_safe_find_sr(session):"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BEXs%3D","line":2032,"in_reply_to":"AAAAUH%2F%2BEzY%3D","updated":"2014-01-30 14:10:34.000000000","message":"Ok I think I see. You mean if I implement the method \"_parse_pci_device_string\" I will be able to mock it and then I will be able to test all cases included the error case. Am I right?","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"15d412d38dd74c41f40666f2907bb12cff83c54b","unresolved":false,"context_lines":[{"line_number":2028,"context_line":"        self.flags(pci_passthrough_whitelist\u003d"},{"line_number":2029,"context_line":"                   [\u0027[{\"vendor_id\":\"10de\", \"product_id\":\"11bf\"}]\u0027])"},{"line_number":2030,"context_line":"        stats \u003d self.conn.get_host_stats()"},{"line_number":2031,"context_line":"        self.assertEqual(len(stats[\u0027pci_passthrough_devices\u0027]), 1)"},{"line_number":2032,"context_line":""},{"line_number":2033,"context_line":"    def test_host_state_missing_sr(self):"},{"line_number":2034,"context_line":"        def fake_safe_find_sr(session):"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAUH%2F91Ic%3D","line":2031,"updated":"2014-02-03 15:23:59.000000000","message":"Could we also have a test that presents a device that\u0027s in the list returned from the plugin, but not using pciback?\nthe devices should then be 0 length.","commit_id":"801e95771566c562ec786cecfe8167b0268e67ba"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"2b81c05a364a4dc0eb41831e4a8c373dd1394d73","unresolved":false,"context_lines":[{"line_number":2028,"context_line":"        self.flags(pci_passthrough_whitelist\u003d"},{"line_number":2029,"context_line":"                   [\u0027[{\"vendor_id\":\"10de\", \"product_id\":\"11bf\"}]\u0027])"},{"line_number":2030,"context_line":"        stats \u003d self.conn.get_host_stats()"},{"line_number":2031,"context_line":"        self.assertEqual(len(stats[\u0027pci_passthrough_devices\u0027]), 1)"},{"line_number":2032,"context_line":""},{"line_number":2033,"context_line":"    def test_host_state_missing_sr(self):"},{"line_number":2034,"context_line":"        def fake_safe_find_sr(session):"}],"source_content_type":"text/x-python","patch_set":18,"id":"AAAAUH%2F9wqQ%3D","line":2031,"in_reply_to":"AAAAUH%2F91Ic%3D","updated":"2014-02-04 09:13:40.000000000","message":"Yes I can add this kind of test. In fact in the current test there are 4 devices. Two are not using pciback driver. For the other ones, one is in the pci white list and the other is not. So in this test I tried to check all possible values but I think that I can separate tests to be more understandable. I look at it.","commit_id":"801e95771566c562ec786cecfe8167b0268e67ba"}],"nova/virt/xenapi/client/session.py":[{"author":{"_account_id":642,"name":"Matt Dietz","email":"matthew.dietz@gmail.com","username":"cerberus"},"change_message_id":"a7ffb55372b979da54aedbecdd1bece9afece7f0","unresolved":false,"context_lines":[{"line_number":59,"context_line":"    # changed in development environments."},{"line_number":60,"context_line":"    # MAJOR VERSION: Incompatible changes with the plugins"},{"line_number":61,"context_line":"    # MINOR VERSION: Compatible changes, new plguins, etc"},{"line_number":62,"context_line":"    PLUGIN_REQUIRED_VERSION \u003d \u00271.1\u0027"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    def __init__(self, url, user, pw):"},{"line_number":65,"context_line":"        import XenAPI"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAUH%2F9uFc%3D","line":62,"updated":"2014-02-04 17:37:13.000000000","message":"Presumably you\u0027ll need to update this version as well.","commit_id":"204afe78fa4db94ab3a91400edb276f06f991d7d"}],"nova/virt/xenapi/driver.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"8d094c929642dec386b89f82859a807a715d70c1","unresolved":false,"context_lines":[{"line_number":470,"context_line":"               \u0027cpu_info\u0027: host_stats[\u0027host_cpu_info\u0027][\u0027cpu_count\u0027],"},{"line_number":471,"context_line":"               \u0027supported_instances\u0027: jsonutils.dumps("},{"line_number":472,"context_line":"                   host_stats[\u0027supported_instances\u0027]),"},{"line_number":473,"context_line":"               \u0027pci_passthrough_devices\u0027: jsonutils.dumps("},{"line_number":474,"context_line":"                   host_stats[\u0027pci_passthrough_devices\u0027])}"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":"        return dic"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2FxU0%3D","line":473,"updated":"2014-01-08 17:01:33.000000000","message":"I am guessing this is what we do for libvirt.","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"4bab7202dd897d53571606a3562d1733e0acd529","unresolved":false,"context_lines":[{"line_number":470,"context_line":"               \u0027cpu_info\u0027: host_stats[\u0027host_cpu_info\u0027][\u0027cpu_count\u0027],"},{"line_number":471,"context_line":"               \u0027supported_instances\u0027: jsonutils.dumps("},{"line_number":472,"context_line":"                   host_stats[\u0027supported_instances\u0027]),"},{"line_number":473,"context_line":"               \u0027pci_passthrough_devices\u0027: jsonutils.dumps("},{"line_number":474,"context_line":"                   host_stats[\u0027pci_passthrough_devices\u0027])}"},{"line_number":475,"context_line":""},{"line_number":476,"context_line":"        return dic"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2FtqM%3D","line":473,"in_reply_to":"AAAAUH%2F%2FxU0%3D","updated":"2014-01-09 11:08:28.000000000","message":"In libvirt, pci_passthrough_devices are updated through the method get_pci_passthrough_devices() that builds a list of PCI devices according to what libvirt returns. And it returns a json string from this list.\nIn our case the list is build during the call to update_status(). So in host_stats[\u0027pci_passthrough_devices\u0027] we have the list of PCI devices that is returned from the code added in our plugin and we build the json string from it.\n\nSo to get the pci list it is a little bit different but the json dump is the same yes.","commit_id":"c15812575064e45110d107f116ba11b6431bc575"}],"nova/virt/xenapi/host.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"8d094c929642dec386b89f82859a807a715d70c1","unresolved":false,"context_lines":[{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        :returns: a list of the assignable pci devices information"},{"line_number":165,"context_line":"        \"\"\""},{"line_number":166,"context_line":"        pci_list \u003d self._session.call_plugin_serialized("},{"line_number":167,"context_line":"            \u0027workarounds\u0027, \u0027get_passthrough_devices\u0027)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        # validate with pci whitelist"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2FxVA%3D","line":166,"updated":"2014-01-08 17:01:33.000000000","message":"I think pulling all the complex processing of /proc/cmdline into here makes quite a bit of sense, just so we can unit test it really.\n\nAlso, if the whitelist is empty, I would rather we never call this plugin method.","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"cd12aec836eb534b6ce3bc796f4784aa41f8f0e0","unresolved":false,"context_lines":[{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        :returns: a list of the assignable pci devices information"},{"line_number":165,"context_line":"        \"\"\""},{"line_number":166,"context_line":"        pci_list \u003d self._session.call_plugin_serialized("},{"line_number":167,"context_line":"            \u0027workarounds\u0027, \u0027get_passthrough_devices\u0027)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        # validate with pci whitelist"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2FtPg%3D","line":166,"in_reply_to":"AAAAUH%2F%2FtT0%3D","updated":"2014-01-09 11:19:24.000000000","message":"In fact I miss somethong about the /proc/cmdline because I compare its output to pci device that are available on dom0. So I cannot do this comparaison here unless I returned the /proc/cmdline and also a list of PCI devices available on dom0. Am I right?","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"4bab7202dd897d53571606a3562d1733e0acd529","unresolved":false,"context_lines":[{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        :returns: a list of the assignable pci devices information"},{"line_number":165,"context_line":"        \"\"\""},{"line_number":166,"context_line":"        pci_list \u003d self._session.call_plugin_serialized("},{"line_number":167,"context_line":"            \u0027workarounds\u0027, \u0027get_passthrough_devices\u0027)"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        # validate with pci whitelist"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAUH%2F%2FtT0%3D","line":166,"in_reply_to":"AAAAUH%2F%2FxVA%3D","updated":"2014-01-09 11:08:28.000000000","message":"About the /proc/cmdline I didn\u0027t think about it but I agree. So the plugin will just return the string of /proc/cmdline and all the complexity is done here. I will do it.\n\nAbout the whitelist I agree but I\u0027m not sure about how to do it. I can read the config file CONF.pci_passthrough_whitelist and see if it is empty or I can add a method is_empty() in the class PciHostDevicesWhiteList ? So I will be able to do something like if dev_filter.is_empty() return []","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"5820fb08a3cc4e9ccc35b04c96a4ec70d3ee5184","unresolved":false,"context_lines":[{"line_number":221,"context_line":"                         \u0027product_id\u0027: product_id[0],"},{"line_number":222,"context_line":"                         \u0027address\u0027: slot_id[0],"},{"line_number":223,"context_line":"                         \u0027dev_id\u0027: \u0027_\u0027.join([\u0027pci\u0027, slot_id[0]]),"},{"line_number":224,"context_line":"                         \u0027dev_type\u0027: \u0027type-PCI\u0027,"},{"line_number":225,"context_line":"                         \u0027status\u0027: \u0027available\u0027})"},{"line_number":226,"context_line":"                else:"},{"line_number":227,"context_line":"                    # NOTE: I put an empty else because I\u0027m not sure"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAUH%2F%2FY84%3D","line":224,"updated":"2014-01-14 09:50:46.000000000","message":"I still need to manage the type-PCI","commit_id":"24f4e981692ef339e5a6ec47666b22e038717f0a"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"e8360e1b22812ea9251801a7d085170fa46e2a41","unresolved":false,"context_lines":[{"line_number":178,"context_line":""},{"line_number":179,"context_line":"            dev_info: is a string return by lspci -vmmnk"},{"line_number":180,"context_line":"            \"\"\""},{"line_number":181,"context_line":"            pciback_regex \u003d re.compile(r\"Driver:\\tpciback\")"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"            if pciback_regex.findall(dev_info):"},{"line_number":184,"context_line":"                return True"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F%2FYo0%3D","line":181,"updated":"2014-01-14 11:16:14.000000000","message":"Does this need to be a regex?\n\nCould it just be \"return \u0027Driver:\\tpciback\u0027 in dev_info\"?","commit_id":"ce9718240fc12ec33a44afd53c5dbb33fc051763"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"df8d2e92066f70222aca6d9bd74587509eefd766","unresolved":false,"context_lines":[{"line_number":178,"context_line":""},{"line_number":179,"context_line":"            dev_info: is a string return by lspci -vmmnk"},{"line_number":180,"context_line":"            \"\"\""},{"line_number":181,"context_line":"            pciback_regex \u003d re.compile(r\"Driver:\\tpciback\")"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"            if pciback_regex.findall(dev_info):"},{"line_number":184,"context_line":"                return True"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F%2FYhc%3D","line":181,"in_reply_to":"AAAAUH%2F%2FYo0%3D","updated":"2014-01-14 12:09:33.000000000","message":"Yes I agree, it is a little bit overkill here :) In fact as I\u0027ve wrote three regex before this one I was influenced and do another one but it is not needed.","commit_id":"ce9718240fc12ec33a44afd53c5dbb33fc051763"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"e8360e1b22812ea9251801a7d085170fa46e2a41","unresolved":false,"context_lines":[{"line_number":180,"context_line":"            \"\"\""},{"line_number":181,"context_line":"            pciback_regex \u003d re.compile(r\"Driver:\\tpciback\")"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"            if pciback_regex.findall(dev_info):"},{"line_number":184,"context_line":"                return True"},{"line_number":185,"context_line":"            else:"},{"line_number":186,"context_line":"                return False"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F%2FYpA%3D","line":183,"updated":"2014-01-14 11:16:14.000000000","message":"If you keep regex, perhaps use\nreturn pciback_regex.findall(dev_info)","commit_id":"ce9718240fc12ec33a44afd53c5dbb33fc051763"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"df8d2e92066f70222aca6d9bd74587509eefd766","unresolved":false,"context_lines":[{"line_number":180,"context_line":"            \"\"\""},{"line_number":181,"context_line":"            pciback_regex \u003d re.compile(r\"Driver:\\tpciback\")"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"            if pciback_regex.findall(dev_info):"},{"line_number":184,"context_line":"                return True"},{"line_number":185,"context_line":"            else:"},{"line_number":186,"context_line":"                return False"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F%2FYhA%3D","line":183,"in_reply_to":"AAAAUH%2F%2FYpA%3D","updated":"2014-01-14 12:09:33.000000000","message":"In fact I will replace the _is_driver_pciback(dev_info) by the test you suggested above:\n\nif \"Driver:\\tpciback\" in dev_string_info:","commit_id":"ce9718240fc12ec33a44afd53c5dbb33fc051763"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"e8360e1b22812ea9251801a7d085170fa46e2a41","unresolved":false,"context_lines":[{"line_number":211,"context_line":"                vendor_id \u003d vendor_regex.findall(dev_string_info)"},{"line_number":212,"context_line":"                product_id \u003d product_regex.findall(dev_string_info)"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"                if slot_id and vendor_id and product_id:"},{"line_number":215,"context_line":"                    # find the type of PCI"},{"line_number":216,"context_line":"                    type_pci \u003d self._session.call_plugin_serialized("},{"line_number":217,"context_line":"                        \u0027xenhost\u0027, \u0027get_pci_type\u0027, slot_id[0])"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F%2FYnQ%3D","line":214,"updated":"2014-01-14 11:16:14.000000000","message":"If, for some reason, they are not populated (e.g. broken regex, different language output for lspci) then raise an exception that\u0027ll get caught higher up.","commit_id":"ce9718240fc12ec33a44afd53c5dbb33fc051763"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"df8d2e92066f70222aca6d9bd74587509eefd766","unresolved":false,"context_lines":[{"line_number":211,"context_line":"                vendor_id \u003d vendor_regex.findall(dev_string_info)"},{"line_number":212,"context_line":"                product_id \u003d product_regex.findall(dev_string_info)"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"                if slot_id and vendor_id and product_id:"},{"line_number":215,"context_line":"                    # find the type of PCI"},{"line_number":216,"context_line":"                    type_pci \u003d self._session.call_plugin_serialized("},{"line_number":217,"context_line":"                        \u0027xenhost\u0027, \u0027get_pci_type\u0027, slot_id[0])"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAUH%2F%2FYes%3D","line":214,"in_reply_to":"AAAAUH%2F%2FYnQ%3D","updated":"2014-01-14 12:09:33.000000000","message":"Is replacing pass by raise in the else statement is enough to raise the exception?\nI will check how it is done elsewhere.","commit_id":"ce9718240fc12ec33a44afd53c5dbb33fc051763"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"4aae7d3c4be54c1af0a52b9be09c77d25d1d8e7a","unresolved":false,"context_lines":[{"line_number":218,"context_line":"                else:"},{"line_number":219,"context_line":"                    # Something bad happens, we should not be here"},{"line_number":220,"context_line":"                    raise Exception(\"Failed to parse information about\""},{"line_number":221,"context_line":"                                    \" a pci device for passthrough\")"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"        # validate with pci whitelist"},{"line_number":224,"context_line":"        dev_filter \u003d pci_whitelist.get_pci_devices_filter()"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2FXoo%3D","line":221,"updated":"2014-01-14 15:08:47.000000000","message":"Bob: I\u0027m really not sure here if it is what you expected. Any comment on this exception?","commit_id":"9bdf6274a70e53fa813b32ee013df4f5d32006b2"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"fc4a3e81f51206ffbc3743b2fa8627f72681289f","unresolved":false,"context_lines":[{"line_number":218,"context_line":"                else:"},{"line_number":219,"context_line":"                    # Something bad happens, we should not be here"},{"line_number":220,"context_line":"                    raise Exception(\"Failed to parse information about\""},{"line_number":221,"context_line":"                                    \" a pci device for passthrough\")"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"        # validate with pci whitelist"},{"line_number":224,"context_line":"        dev_filter \u003d pci_whitelist.get_pci_devices_filter()"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2FXZo%3D","line":221,"in_reply_to":"AAAAUH%2F%2FXng%3D","updated":"2014-01-14 15:54:08.000000000","message":"Done","commit_id":"9bdf6274a70e53fa813b32ee013df4f5d32006b2"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"212b0d87f6a3b318bfe14f695a4bdb3aadffbce5","unresolved":false,"context_lines":[{"line_number":218,"context_line":"                else:"},{"line_number":219,"context_line":"                    # Something bad happens, we should not be here"},{"line_number":220,"context_line":"                    raise Exception(\"Failed to parse information about\""},{"line_number":221,"context_line":"                                    \" a pci device for passthrough\")"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"        # validate with pci whitelist"},{"line_number":224,"context_line":"        dev_filter \u003d pci_whitelist.get_pci_devices_filter()"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAUH%2F%2FXng%3D","line":221,"in_reply_to":"AAAAUH%2F%2FXoo%3D","updated":"2014-01-14 15:12:20.000000000","message":"Should use exception.NovaException - but I\u0027m happy with the message.  It does need to be internationalized with _(\"Failed to ... passthrough\").","commit_id":"9bdf6274a70e53fa813b32ee013df4f5d32006b2"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"bac535ac36b605a3139ba63444a4f18953ce028f","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                          \" a pci device for passthrough\"))"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"        # validate with pci whitelist"},{"line_number":225,"context_line":"        dev_filter \u003d pci_whitelist.get_pci_devices_filter()"},{"line_number":226,"context_line":"        return [dev for dev in passthrough_devices"},{"line_number":227,"context_line":"                if dev_filter.device_assignable(dev)]"},{"line_number":228,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"AAAAUH%2F%2FXVc%3D","line":225,"updated":"2014-01-14 16:08:01.000000000","message":"Maybe I can put dev_filter as an attribute like it is done in libvirt?\n\nself._dev_filter \u003d pci_whitelist.get_pci_devices_filter()","commit_id":"b758ee932fb4691c8f966bb54a54593f7b2711c3"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"008adc58429f41029b5fa2fe7f8cea73d08f5e3b","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        self._dev_filter \u003d pci_whitelist.get_pci_devices_filter()"},{"line_number":158,"context_line":"        self.update_status()"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"    def _get_passthrough_devices(self):"},{"line_number":161,"context_line":"        \"\"\"Get a list pci devices that are available for pci passthtough."},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        We use a plugin to get the output of the lspci command runs on dom0."}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAUH%2F%2BS6g%3D","line":160,"updated":"2014-01-28 10:07:59.000000000","message":"I think this function could do with a test.\nMock out https://review.openstack.org/#/c/63816/ and return a sample string, checking that this method gives the output you expect?","commit_id":"82c4296a32a147df92b8133da2eae7737ec3538c"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"8c4d85a918025b1705204f99f014b14528df3d19","unresolved":false,"context_lines":[{"line_number":157,"context_line":"        self._dev_filter \u003d pci_whitelist.get_pci_devices_filter()"},{"line_number":158,"context_line":"        self.update_status()"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"    def _get_passthrough_devices(self):"},{"line_number":161,"context_line":"        \"\"\"Get a list pci devices that are available for pci passthtough."},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        We use a plugin to get the output of the lspci command runs on dom0."}],"source_content_type":"text/x-python","patch_set":13,"id":"AAAAUH%2F%2BFDA%3D","line":160,"in_reply_to":"AAAAUH%2F%2BS6g%3D","updated":"2014-01-30 11:44:14.000000000","message":"IIUC it is done","commit_id":"82c4296a32a147df92b8133da2eae7737ec3538c"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d6e14e91dec799c7deffe4564dc4781315bb264a","unresolved":false,"context_lines":[{"line_number":154,"context_line":"        super(HostState, self).__init__()"},{"line_number":155,"context_line":"        self._session \u003d session"},{"line_number":156,"context_line":"        self._stats \u003d {}"},{"line_number":157,"context_line":"        self._dev_filter \u003d pci_whitelist.get_pci_devices_filter()"},{"line_number":158,"context_line":"        self.update_status()"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"    def _get_passthrough_devices(self):"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BE0g%3D","line":157,"updated":"2014-01-30 12:30:20.000000000","message":"_pci_device_filter might be a nicer name?","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"07afbc4389f5d67ccf5d9673ba301caf1b275227","unresolved":false,"context_lines":[{"line_number":154,"context_line":"        super(HostState, self).__init__()"},{"line_number":155,"context_line":"        self._session \u003d session"},{"line_number":156,"context_line":"        self._stats \u003d {}"},{"line_number":157,"context_line":"        self._dev_filter \u003d pci_whitelist.get_pci_devices_filter()"},{"line_number":158,"context_line":"        self.update_status()"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"    def _get_passthrough_devices(self):"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BEi8%3D","line":157,"in_reply_to":"AAAAUH%2F%2BE0g%3D","updated":"2014-01-30 14:10:34.000000000","message":"I used _dev_filter because I copied the name from libvirt but I agree for _pci_device_filter.\n\nDone","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d6e14e91dec799c7deffe4564dc4781315bb264a","unresolved":false,"context_lines":[{"line_number":174,"context_line":"            \"\"\""},{"line_number":175,"context_line":"            return re.compile(pattern.replace(\"hex\", r\"[\\da-fA-F]\"))"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"        slot_regex \u003d _compile_hex(r\"Slot:\\t\""},{"line_number":178,"context_line":"                                  r\"((?:hex{4}:)?\"  # Domain: (optional)"},{"line_number":179,"context_line":"                                  r\"hex{2}:\"        # Bus:"},{"line_number":180,"context_line":"                                  r\"hex{2}\\.\"       # Device."}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BE28%3D","line":177,"updated":"2014-01-30 12:30:20.000000000","message":"might be good to cache these in the model to save on compile time.","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"07afbc4389f5d67ccf5d9673ba301caf1b275227","unresolved":false,"context_lines":[{"line_number":174,"context_line":"            \"\"\""},{"line_number":175,"context_line":"            return re.compile(pattern.replace(\"hex\", r\"[\\da-fA-F]\"))"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"        slot_regex \u003d _compile_hex(r\"Slot:\\t\""},{"line_number":178,"context_line":"                                  r\"((?:hex{4}:)?\"  # Domain: (optional)"},{"line_number":179,"context_line":"                                  r\"hex{2}:\"        # Bus:"},{"line_number":180,"context_line":"                                  r\"hex{2}\\.\"       # Device."}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BEcc%3D","line":177,"in_reply_to":"AAAAUH%2F%2BE28%3D","updated":"2014-01-30 14:10:34.000000000","message":"The thing is that if you are using re.compile the compiled object are already cached for future use. Do you think that the cache mechanism used by re.compile may be too small and I should move the regex definition in the __init__ method of the class?","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d6e14e91dec799c7deffe4564dc4781315bb264a","unresolved":false,"context_lines":[{"line_number":182,"context_line":"        vendor_regex \u003d _compile_hex(r\"\\nVendor:\\t(hex+)\")"},{"line_number":183,"context_line":"        product_regex \u003d _compile_hex(r\"\\nDevice:\\t(hex+)\")"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        # Get a string of pci devices and split it into a"},{"line_number":186,"context_line":"        # list of devices. Devices are separated by a blank line."},{"line_number":187,"context_line":"        lspci_out \u003d self._session.call_plugin_serialized("},{"line_number":188,"context_line":"            \u0027xenhost\u0027, \u0027get_pci_device_details\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BE2o%3D","line":185,"updated":"2014-01-30 12:30:20.000000000","message":"I prefer less comments myself these days, but its a minor nit.","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"bb86ea9ff072f6bc130efaae8ac946a6315abf9a","unresolved":false,"context_lines":[{"line_number":182,"context_line":"        vendor_regex \u003d _compile_hex(r\"\\nVendor:\\t(hex+)\")"},{"line_number":183,"context_line":"        product_regex \u003d _compile_hex(r\"\\nDevice:\\t(hex+)\")"},{"line_number":184,"context_line":""},{"line_number":185,"context_line":"        # Get a string of pci devices and split it into a"},{"line_number":186,"context_line":"        # list of devices. Devices are separated by a blank line."},{"line_number":187,"context_line":"        lspci_out \u003d self._session.call_plugin_serialized("},{"line_number":188,"context_line":"            \u0027xenhost\u0027, \u0027get_pci_device_details\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F9f6w%3D","line":185,"in_reply_to":"AAAAUH%2F%2BE2o%3D","updated":"2014-02-06 14:16:38.000000000","message":"I modified the comment and I only explained why I\u0027m using \"\\n\\n\"","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d6e14e91dec799c7deffe4564dc4781315bb264a","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        # list of devices. Devices are separated by a blank line."},{"line_number":187,"context_line":"        lspci_out \u003d self._session.call_plugin_serialized("},{"line_number":188,"context_line":"            \u0027xenhost\u0027, \u0027get_pci_device_details\u0027)"},{"line_number":189,"context_line":"        pci_list \u003d lspci_out.split(\"\\n\\n\")"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        # For each device of the list, check if it uses the pciback"},{"line_number":192,"context_line":"        # kernel driver and if it does, add it in the list of"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BE1g%3D","line":189,"updated":"2014-01-30 12:30:20.000000000","message":"This seems a bit odd, splitlines() is more normal, would that not work in this case?","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"07afbc4389f5d67ccf5d9673ba301caf1b275227","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        # list of devices. Devices are separated by a blank line."},{"line_number":187,"context_line":"        lspci_out \u003d self._session.call_plugin_serialized("},{"line_number":188,"context_line":"            \u0027xenhost\u0027, \u0027get_pci_device_details\u0027)"},{"line_number":189,"context_line":"        pci_list \u003d lspci_out.split(\"\\n\\n\")"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        # For each device of the list, check if it uses the pciback"},{"line_number":192,"context_line":"        # kernel driver and if it does, add it in the list of"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BEg4%3D","line":189,"in_reply_to":"AAAAUH%2F%2BE1g%3D","updated":"2014-01-30 14:10:34.000000000","message":"The split is done on \"\\n\\n\" not on \"\\n\". That\u0027s why I added more comments is to say that I split on empty line, not on \"\\n\". Does it make sense?","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d6e14e91dec799c7deffe4564dc4781315bb264a","unresolved":false,"context_lines":[{"line_number":194,"context_line":"        passthrough_devices \u003d []"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"        for dev_string_info in pci_list:"},{"line_number":197,"context_line":"            if \"Driver:\\tpciback\" in dev_string_info:"},{"line_number":198,"context_line":"                # Before adding the device into the list we need to"},{"line_number":199,"context_line":"                # get information about it."},{"line_number":200,"context_line":"                slot_id \u003d slot_regex.findall(dev_string_info)"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BE1E%3D","line":197,"updated":"2014-01-30 12:30:20.000000000","message":"What happens in the else case? should we raise an exception? or do we just ignore those?","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"07afbc4389f5d67ccf5d9673ba301caf1b275227","unresolved":false,"context_lines":[{"line_number":194,"context_line":"        passthrough_devices \u003d []"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"        for dev_string_info in pci_list:"},{"line_number":197,"context_line":"            if \"Driver:\\tpciback\" in dev_string_info:"},{"line_number":198,"context_line":"                # Before adding the device into the list we need to"},{"line_number":199,"context_line":"                # get information about it."},{"line_number":200,"context_line":"                slot_id \u003d slot_regex.findall(dev_string_info)"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BEfE%3D","line":197,"in_reply_to":"AAAAUH%2F%2BE1E%3D","updated":"2014-01-30 14:10:34.000000000","message":"We just ignore. We just keep devices with pciback driver assigned. I will add a sentence that says we ignore device that don\u0027t use pciback driver in the comment above. I will make more comments then ;)","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d6e14e91dec799c7deffe4564dc4781315bb264a","unresolved":false,"context_lines":[{"line_number":197,"context_line":"            if \"Driver:\\tpciback\" in dev_string_info:"},{"line_number":198,"context_line":"                # Before adding the device into the list we need to"},{"line_number":199,"context_line":"                # get information about it."},{"line_number":200,"context_line":"                slot_id \u003d slot_regex.findall(dev_string_info)"},{"line_number":201,"context_line":"                vendor_id \u003d vendor_regex.findall(dev_string_info)"},{"line_number":202,"context_line":"                product_id \u003d product_regex.findall(dev_string_info)"},{"line_number":203,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BE0I%3D","line":200,"updated":"2014-01-30 12:30:20.000000000","message":"Can\u0027t we have a single regex that picks out all the component parts?","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"07afbc4389f5d67ccf5d9673ba301caf1b275227","unresolved":false,"context_lines":[{"line_number":197,"context_line":"            if \"Driver:\\tpciback\" in dev_string_info:"},{"line_number":198,"context_line":"                # Before adding the device into the list we need to"},{"line_number":199,"context_line":"                # get information about it."},{"line_number":200,"context_line":"                slot_id \u003d slot_regex.findall(dev_string_info)"},{"line_number":201,"context_line":"                vendor_id \u003d vendor_regex.findall(dev_string_info)"},{"line_number":202,"context_line":"                product_id \u003d product_regex.findall(dev_string_info)"},{"line_number":203,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BEcQ%3D","line":200,"in_reply_to":"AAAAUH%2F%2BE0I%3D","updated":"2014-01-30 14:10:34.000000000","message":"I think we can. I will give it a try to see if it is better :)","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"65668f8252745128e660284247be259d45a1f129","unresolved":false,"context_lines":[{"line_number":197,"context_line":"            if \"Driver:\\tpciback\" in dev_string_info:"},{"line_number":198,"context_line":"                # Before adding the device into the list we need to"},{"line_number":199,"context_line":"                # get information about it."},{"line_number":200,"context_line":"                slot_id \u003d slot_regex.findall(dev_string_info)"},{"line_number":201,"context_line":"                vendor_id \u003d vendor_regex.findall(dev_string_info)"},{"line_number":202,"context_line":"                product_id \u003d product_regex.findall(dev_string_info)"},{"line_number":203,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F9%2FuE%3D","line":200,"in_reply_to":"AAAAUH%2F%2BEcQ%3D","updated":"2014-01-31 07:34:56.000000000","message":"The problem I\u0027ve got is that the string that we parse is like:\n\"Slot:\\txx:xx.x\\nSomething:\\taaa\\nVendor:\\tbbb\\nDevice:\\tccc\\n...\"\nAnd playing with \\n and multi line  with regex is not so easy I found. So I found that having 3 different catches is more clean. Any performance issue or other kind of issue by doing this way?","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d6e14e91dec799c7deffe4564dc4781315bb264a","unresolved":false,"context_lines":[{"line_number":206,"context_line":"                    type_pci \u003d self._session.call_plugin_serialized("},{"line_number":207,"context_line":"                        \u0027xenhost\u0027, \u0027get_pci_type\u0027, slot_id[0])"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"                    passthrough_devices.append("},{"line_number":210,"context_line":"                        {\u0027label\u0027: \u0027_\u0027.join([\u0027label\u0027,"},{"line_number":211,"context_line":"                                            vendor_id[0],"},{"line_number":212,"context_line":"                                            product_id[0]]),"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BE14%3D","line":209,"updated":"2014-01-30 12:30:20.000000000","message":"please add a function for all this code, something like \"_parse_pci_device_string\"?\n\nIt will make this easier to read, and you can get rid of some of the comments. Also, its much easier to test in that way.\n\nYou an return None, the parsed dict, or raise an exception as appropriate.","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"07afbc4389f5d67ccf5d9673ba301caf1b275227","unresolved":false,"context_lines":[{"line_number":206,"context_line":"                    type_pci \u003d self._session.call_plugin_serialized("},{"line_number":207,"context_line":"                        \u0027xenhost\u0027, \u0027get_pci_type\u0027, slot_id[0])"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"                    passthrough_devices.append("},{"line_number":210,"context_line":"                        {\u0027label\u0027: \u0027_\u0027.join([\u0027label\u0027,"},{"line_number":211,"context_line":"                                            vendor_id[0],"},{"line_number":212,"context_line":"                                            product_id[0]]),"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BEcA%3D","line":209,"in_reply_to":"AAAAUH%2F%2BE14%3D","updated":"2014-01-30 14:10:34.000000000","message":"Yes agree. I will do that.","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"65668f8252745128e660284247be259d45a1f129","unresolved":false,"context_lines":[{"line_number":206,"context_line":"                    type_pci \u003d self._session.call_plugin_serialized("},{"line_number":207,"context_line":"                        \u0027xenhost\u0027, \u0027get_pci_type\u0027, slot_id[0])"},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"                    passthrough_devices.append("},{"line_number":210,"context_line":"                        {\u0027label\u0027: \u0027_\u0027.join([\u0027label\u0027,"},{"line_number":211,"context_line":"                                            vendor_id[0],"},{"line_number":212,"context_line":"                                            product_id[0]]),"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F9%2FuA%3D","line":209,"in_reply_to":"AAAAUH%2F%2BEcA%3D","updated":"2014-01-31 07:34:56.000000000","message":"Done","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d6e14e91dec799c7deffe4564dc4781315bb264a","unresolved":false,"context_lines":[{"line_number":218,"context_line":"                         \u0027status\u0027: \u0027available\u0027})"},{"line_number":219,"context_line":"                else:"},{"line_number":220,"context_line":"                    # Something bad happens, we should not be here"},{"line_number":221,"context_line":"                    raise exception.NovaException("},{"line_number":222,"context_line":"                        _(\"Failed to parse information about\""},{"line_number":223,"context_line":"                          \" a pci device for passthrough\"))"},{"line_number":224,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BE2c%3D","line":221,"updated":"2014-01-30 12:30:20.000000000","message":"we need a unit test for this case please.","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"07afbc4389f5d67ccf5d9673ba301caf1b275227","unresolved":false,"context_lines":[{"line_number":218,"context_line":"                         \u0027status\u0027: \u0027available\u0027})"},{"line_number":219,"context_line":"                else:"},{"line_number":220,"context_line":"                    # Something bad happens, we should not be here"},{"line_number":221,"context_line":"                    raise exception.NovaException("},{"line_number":222,"context_line":"                        _(\"Failed to parse information about\""},{"line_number":223,"context_line":"                          \" a pci device for passthrough\"))"},{"line_number":224,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BEbs%3D","line":221,"in_reply_to":"AAAAUH%2F%2BE2c%3D","updated":"2014-01-30 14:10:34.000000000","message":"Agree. I will do that","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"d6e14e91dec799c7deffe4564dc4781315bb264a","unresolved":false,"context_lines":[{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        # validate with pci whitelist"},{"line_number":226,"context_line":"        return [dev for dev in passthrough_devices"},{"line_number":227,"context_line":"                if self._dev_filter.device_assignable(dev)]"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"    def get_host_stats(self, refresh\u003dFalse):"},{"line_number":230,"context_line":"        \"\"\"Return the current state of the host. If \u0027refresh\u0027 is"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BE0A%3D","line":227,"updated":"2014-01-30 12:30:20.000000000","message":"I would add this in the above loop, only add to the list if the PCI device matches the whitelist filter.","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"07afbc4389f5d67ccf5d9673ba301caf1b275227","unresolved":false,"context_lines":[{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        # validate with pci whitelist"},{"line_number":226,"context_line":"        return [dev for dev in passthrough_devices"},{"line_number":227,"context_line":"                if self._dev_filter.device_assignable(dev)]"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"    def get_host_stats(self, refresh\u003dFalse):"},{"line_number":230,"context_line":"        \"\"\"Return the current state of the host. If \u0027refresh\u0027 is"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F%2BEW0%3D","line":227,"in_reply_to":"AAAAUH%2F%2BE0A%3D","updated":"2014-01-30 14:10:34.000000000","message":"Ok it makes sense. I will do that.","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"65668f8252745128e660284247be259d45a1f129","unresolved":false,"context_lines":[{"line_number":224,"context_line":""},{"line_number":225,"context_line":"        # validate with pci whitelist"},{"line_number":226,"context_line":"        return [dev for dev in passthrough_devices"},{"line_number":227,"context_line":"                if self._dev_filter.device_assignable(dev)]"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"    def get_host_stats(self, refresh\u003dFalse):"},{"line_number":230,"context_line":"        \"\"\"Return the current state of the host. If \u0027refresh\u0027 is"}],"source_content_type":"text/x-python","patch_set":14,"id":"AAAAUH%2F9%2Ft8%3D","line":227,"in_reply_to":"AAAAUH%2F%2BEW0%3D","updated":"2014-01-31 07:34:56.000000000","message":"Done","commit_id":"9031e97b49977a3a2298d682e08399627d24260b"},{"author":{"_account_id":642,"name":"Matt Dietz","email":"matthew.dietz@gmail.com","username":"cerberus"},"change_message_id":"a7ffb55372b979da54aedbecdd1bece9afece7f0","unresolved":false,"context_lines":[{"line_number":192,"context_line":"            product_id \u003d product_regex.findall(dev_string)"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"            if not slot_id or not vendor_id or not product_id:"},{"line_number":195,"context_line":"                # Something bad happens, we should not be here"},{"line_number":196,"context_line":"                raise exception.NovaException("},{"line_number":197,"context_line":"                    _(\"Failed to parse information about\""},{"line_number":198,"context_line":"                      \" a pci device for passthrough\"))"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAUH%2F9uGE%3D","line":195,"updated":"2014-02-04 17:37:13.000000000","message":"Nit-picking, but unnecessary comment: we know that because we\u0027re raising an exception.","commit_id":"204afe78fa4db94ab3a91400edb276f06f991d7d"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"fa5eb49682feeec77ff25b594fcd2414329b22c1","unresolved":false,"context_lines":[{"line_number":192,"context_line":"            product_id \u003d product_regex.findall(dev_string)"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"            if not slot_id or not vendor_id or not product_id:"},{"line_number":195,"context_line":"                # Something bad happens, we should not be here"},{"line_number":196,"context_line":"                raise exception.NovaException("},{"line_number":197,"context_line":"                    _(\"Failed to parse information about\""},{"line_number":198,"context_line":"                      \" a pci device for passthrough\"))"}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAUH%2F9qP0%3D","line":195,"in_reply_to":"AAAAUH%2F9uGE%3D","updated":"2014-02-05 09:44:41.000000000","message":"I\u0027m coming from C language so I\u0027m not familiar with exception. So you\u0027re right I removed this comment.","commit_id":"204afe78fa4db94ab3a91400edb276f06f991d7d"},{"author":{"_account_id":642,"name":"Matt Dietz","email":"matthew.dietz@gmail.com","username":"cerberus"},"change_message_id":"a7ffb55372b979da54aedbecdd1bece9afece7f0","unresolved":false,"context_lines":[{"line_number":197,"context_line":"                    _(\"Failed to parse information about\""},{"line_number":198,"context_line":"                      \" a pci device for passthrough\"))"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"            # find the type of PCI"},{"line_number":201,"context_line":"            type_pci \u003d self._session.call_plugin_serialized("},{"line_number":202,"context_line":"                \u0027xenhost\u0027, \u0027get_pci_type\u0027, slot_id[0])"},{"line_number":203,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAUH%2F9uF0%3D","line":200,"updated":"2014-02-04 17:37:13.000000000","message":"It\u0027s probably safe to assume you can remove this comment too.","commit_id":"204afe78fa4db94ab3a91400edb276f06f991d7d"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"fa5eb49682feeec77ff25b594fcd2414329b22c1","unresolved":false,"context_lines":[{"line_number":197,"context_line":"                    _(\"Failed to parse information about\""},{"line_number":198,"context_line":"                      \" a pci device for passthrough\"))"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"            # find the type of PCI"},{"line_number":201,"context_line":"            type_pci \u003d self._session.call_plugin_serialized("},{"line_number":202,"context_line":"                \u0027xenhost\u0027, \u0027get_pci_type\u0027, slot_id[0])"},{"line_number":203,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"AAAAUH%2F9qPs%3D","line":200,"in_reply_to":"AAAAUH%2F9uF0%3D","updated":"2014-02-05 09:44:41.000000000","message":"Done","commit_id":"204afe78fa4db94ab3a91400edb276f06f991d7d"}],"plugins/xenserver/xenapi/etc/xapi.d/plugins/nova_plugin_version":[{"author":{"_account_id":642,"name":"Matt Dietz","email":"matthew.dietz@gmail.com","username":"cerberus"},"change_message_id":"a7ffb55372b979da54aedbecdd1bece9afece7f0","unresolved":false,"context_lines":[{"line_number":23,"context_line":"# MINOR VERSION: Compatible changes, new plugins, etc"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"# 1.0 - Initial version."},{"line_number":26,"context_line":"# 1.1 - Added support for pci passthrough devices"},{"line_number":27,"context_line":"PLUGIN_VERSION \u003d \"1.1\""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def get_version(session):"}],"source_content_type":"application/octet-stream","patch_set":19,"id":"AAAAUH%2F9uG8%3D","line":26,"updated":"2014-02-04 17:37:13.000000000","message":"I\u0027m going to assume it worked like compute RPC versioning, but Bob Ball also incremented to 1.1 in a patch set I just approved, so you might need to bump to 1.2 or something.\n\nhttps://review.openstack.org/#/c/60253/8/plugins/xenserver/xenapi/etc/xapi.d/plugins/nova_plugin_version","commit_id":"204afe78fa4db94ab3a91400edb276f06f991d7d"}],"plugins/xenserver/xenapi/etc/xapi.d/plugins/workarounds":[{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"be1464e33d9a2dc1d2ce0eeca1a66b3edaec006d","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        utils.cleanup_staging_area(staging_path)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"def _get_hidden_pci(string):"},{"line_number":52,"context_line":"    \"\"\""},{"line_number":53,"context_line":"    Find the address of pci devices that are passed to pciback.hide parameter."},{"line_number":54,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAUH%2F%2FxlA%3D","line":51,"updated":"2014-01-08 16:16:52.000000000","message":"Don\u0027t use a keyword as an input to the function - use cmdline","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"c8b0df394cd13be95ed6b75660ef4aae155a87c5","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        utils.cleanup_staging_area(staging_path)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"def _get_hidden_pci(string):"},{"line_number":52,"context_line":"    \"\"\""},{"line_number":53,"context_line":"    Find the address of pci devices that are passed to pciback.hide parameter."},{"line_number":54,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAUH%2F%2FtGw%3D","line":51,"in_reply_to":"AAAAUH%2F%2FxlA%3D","updated":"2014-01-09 16:17:32.000000000","message":"Done","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"be1464e33d9a2dc1d2ce0eeca1a66b3edaec006d","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    \"\"\""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    pattern_pciback \u003d r\"pciback.hide\u003d(\\([\\da-f\\(\\):\\.]*\\))+\""},{"line_number":58,"context_line":"    pattern_bdf \u003d r\"\\((hex{0,4}:?hex{2}:hex{2}.hex{1})\\)\".replace(\"hex\", r\"[\\da-f]\")"},{"line_number":59,"context_line":"    bdf_list \u003d []"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    # Remove white space to support things like:"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAUH%2F%2FxkE%3D","line":58,"updated":"2014-01-08 16:16:52.000000000","message":"Can we have a quick explanation of the formats supported here, i.e. what should match?\nI guess that :00:00:0 shouldn\u0027t match, but it looks to me like it will?","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"c8b0df394cd13be95ed6b75660ef4aae155a87c5","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    \"\"\""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    pattern_pciback \u003d r\"pciback.hide\u003d(\\([\\da-f\\(\\):\\.]*\\))+\""},{"line_number":58,"context_line":"    pattern_bdf \u003d r\"\\((hex{0,4}:?hex{2}:hex{2}.hex{1})\\)\".replace(\"hex\", r\"[\\da-f]\")"},{"line_number":59,"context_line":"    bdf_list \u003d []"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    # Remove white space to support things like:"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAUH%2F%2Fslw%3D","line":58,"in_reply_to":"AAAAUH%2F%2FxkE%3D","updated":"2014-01-09 16:17:32.000000000","message":"Yes right the \u0027:\u0027 is misplaced. My attempt was to match DDDD:BB:DD.F and BB:DD.F. So the pattern that I will use will be:\n\nr\"((?:hex{4}:)?hex{2}:hex{2}\\.hex{1})\".replace(\"hex\", r\"[\\da-fA-F]\")\n\nI will add a comment about the pattern that should match.","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"be1464e33d9a2dc1d2ce0eeca1a66b3edaec006d","unresolved":false,"context_lines":[{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    # Remove white space to support things like:"},{"line_number":62,"context_line":"    # pciback.hide \u003d ( 08:00.1 ) ( 08:00.2 )"},{"line_number":63,"context_line":"    s \u003d string.replace(\" \", \"\")"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    for match in re.findall(pattern_pciback, s, re.I):"},{"line_number":66,"context_line":"        # match holds something like (bdf1)(bdf2)..(bdfN)"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAUH%2F%2FxlQ%3D","line":63,"updated":"2014-01-08 16:16:52.000000000","message":"I think that whitespace can\u0027t occur - if it occurs then the kernel will treat it as different arguments and not actually hide the devices.\n\nAlso, we discussed whether we should use lspci to check the module was \"pciback\"?  That would be safer than parsing the command line since it shows which devices Xen has allocated for pass through.","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"c8b0df394cd13be95ed6b75660ef4aae155a87c5","unresolved":false,"context_lines":[{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    # Remove white space to support things like:"},{"line_number":62,"context_line":"    # pciback.hide \u003d ( 08:00.1 ) ( 08:00.2 )"},{"line_number":63,"context_line":"    s \u003d string.replace(\" \", \"\")"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    for match in re.findall(pattern_pciback, s, re.I):"},{"line_number":66,"context_line":"        # match holds something like (bdf1)(bdf2)..(bdfN)"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAUH%2F%2Fs6c%3D","line":63,"in_reply_to":"AAAAUH%2F%2FxlQ%3D","updated":"2014-01-09 16:17:32.000000000","message":"The problem with using lspci to check that pciback is used is that it is supported since kernel 2.6. So should we say that if kernel is \u003c 2.6 you will not be able to use pci passthrough?","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"be1464e33d9a2dc1d2ce0eeca1a66b3edaec006d","unresolved":false,"context_lines":[{"line_number":74,"context_line":"    \"\"\" From a list, keeps pci devices that are available on the host. \"\"\""},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    vendor_product_regex \u003d re.compile("},{"line_number":77,"context_line":"        r\"(hex{4}):(hex{4})\".replace(\"hex\", r\"[\\da-f]\"),"},{"line_number":78,"context_line":"        re.I)"},{"line_number":79,"context_line":"    available_dev \u003d []"},{"line_number":80,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAUH%2F%2FxkQ%3D","line":77,"updated":"2014-01-08 16:16:52.000000000","message":"Perhaps this is worth having a \"_compile_regex\" which supports the \"hex\" keyword to avoid duplication with above (and use it on line 57 too?)","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"c8b0df394cd13be95ed6b75660ef4aae155a87c5","unresolved":false,"context_lines":[{"line_number":74,"context_line":"    \"\"\" From a list, keeps pci devices that are available on the host. \"\"\""},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    vendor_product_regex \u003d re.compile("},{"line_number":77,"context_line":"        r\"(hex{4}):(hex{4})\".replace(\"hex\", r\"[\\da-f]\"),"},{"line_number":78,"context_line":"        re.I)"},{"line_number":79,"context_line":"    available_dev \u003d []"},{"line_number":80,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAUH%2F%2FsDs%3D","line":77,"in_reply_to":"AAAAUH%2F%2FxkQ%3D","updated":"2014-01-09 16:17:32.000000000","message":"Yes good idea. I will add a function _compile_with_hex() that will support the hex keyword.","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"1cb461b097a28bb668b3ecdcc4421ed2a73e5a02","unresolved":false,"context_lines":[{"line_number":99,"context_line":"                              \u0027address\u0027: dev,"},{"line_number":100,"context_line":"                              \u0027dev_id\u0027: \u0027_\u0027.join([\u0027pci\u0027, dev]),"},{"line_number":101,"context_line":"                              \u0027dev_type\u0027: \u0027type-PCI\u0027,"},{"line_number":102,"context_line":"                              \u0027status\u0027: \u0027available\u0027})"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"    return available_dev"},{"line_number":105,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAUH%2F%2FyT4%3D","line":102,"updated":"2014-01-08 17:46:17.000000000","message":"If we want to support SR-IOV we also need to detect type-VF and type-PF. I see that this can be done trough sysfs so should I include it or can we start by the support of type-PCI only?\n\nIf we keep it like that II will add a note here for future work.","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"be1464e33d9a2dc1d2ce0eeca1a66b3edaec006d","unresolved":false,"context_lines":[{"line_number":104,"context_line":"    return available_dev"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"def get_passthrough_devices(session):"},{"line_number":108,"context_line":"    \"\"\"Get a list of pci devices that can be used for passthrough."},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    We get the list of devices that are passed as hide option to pciback"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAUH%2F%2Fxmg%3D","line":107,"updated":"2014-01-08 16:16:52.000000000","message":"On IRC we discussed that this should move to xenhost.  The pattern for a plugin supporting XenAPIPlugin.dispatch and utils.register_plugin_calls can be seen at https://review.openstack.org/#/c/60253/6/plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"4a3465c17db3715ab96a9ddd33cb3938075a8a42","unresolved":false,"context_lines":[{"line_number":104,"context_line":"    return available_dev"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"def get_passthrough_devices(session):"},{"line_number":108,"context_line":"    \"\"\"Get a list of pci devices that can be used for passthrough."},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    We get the list of devices that are passed as hide option to pciback"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAUH%2F%2FtHo%3D","line":107,"in_reply_to":"AAAAUH%2F%2FxXY%3D","updated":"2014-01-09 11:55:07.000000000","message":"I moved the code in xenhost.\n\nAbout the get_proc_cmdline I added a comment in the vm_utils side. Basically I want to match the proc cmdline and the devices that are actually available on dom0. That means I also need a get_pci_devices function that will return all pci devices right?","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"8d094c929642dec386b89f82859a807a715d70c1","unresolved":false,"context_lines":[{"line_number":104,"context_line":"    return available_dev"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"def get_passthrough_devices(session):"},{"line_number":108,"context_line":"    \"\"\"Get a list of pci devices that can be used for passthrough."},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    We get the list of devices that are passed as hide option to pciback"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAUH%2F%2FxXY%3D","line":107,"in_reply_to":"AAAAUH%2F%2Fxmg%3D","updated":"2014-01-08 17:01:33.000000000","message":"+1 on move to xenhost\n\nThinking about this, can we just create a get_proc_cmdline function, and keep all this complex code inside vm_utils where we can unit test it?","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"be1464e33d9a2dc1d2ce0eeca1a66b3edaec006d","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    try:"},{"line_number":120,"context_line":"        f \u003d open(\u0027/proc/cmdline\u0027, \u0027r\u0027)"},{"line_number":121,"context_line":"        cmdline \u003d f.read()"},{"line_number":122,"context_line":"        f.close()"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"        # First we need to get the list of pci devices that are"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAUH%2F%2FxmQ%3D","line":121,"updated":"2014-01-08 16:16:52.000000000","message":"Should protect this with a try / finally (note for reviewers: This is in dom0 with python 2.4 so can\u0027t use \"with\").","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"c8b0df394cd13be95ed6b75660ef4aae155a87c5","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    try:"},{"line_number":120,"context_line":"        f \u003d open(\u0027/proc/cmdline\u0027, \u0027r\u0027)"},{"line_number":121,"context_line":"        cmdline \u003d f.read()"},{"line_number":122,"context_line":"        f.close()"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"        # First we need to get the list of pci devices that are"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAUH%2F%2Fr6E%3D","line":121,"in_reply_to":"AAAAUH%2F%2FxW8%3D","updated":"2014-01-09 16:17:32.000000000","message":"yes, agree with try/finally between for the f.close","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"8d094c929642dec386b89f82859a807a715d70c1","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    try:"},{"line_number":120,"context_line":"        f \u003d open(\u0027/proc/cmdline\u0027, \u0027r\u0027)"},{"line_number":121,"context_line":"        cmdline \u003d f.read()"},{"line_number":122,"context_line":"        f.close()"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"        # First we need to get the list of pci devices that are"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAUH%2F%2FxW8%3D","line":121,"in_reply_to":"AAAAUH%2F%2FxmQ%3D","updated":"2014-01-08 17:01:33.000000000","message":"Yeah, we can\u0027t have try/except/finally either, but in this case I would just drop the except clause and have a try/finally for the f.close?","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"be1464e33d9a2dc1d2ce0eeca1a66b3edaec006d","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        # if the devices are present into dom0"},{"line_number":130,"context_line":"        passthrough_devices \u003d _get_available_dev(hidden_devices)"},{"line_number":131,"context_line":"    except:"},{"line_number":132,"context_line":"        # TODO: Log the error"},{"line_number":133,"context_line":"        pass"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    return passthrough_devices"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAUH%2F%2FxkI%3D","line":132,"updated":"2014-01-08 16:16:52.000000000","message":"What\u0027s the expected case for logging rather than throwing here?  I would think if the above code errors then the plugin should error - i.e. we shouldn\u0027t have the try/except block.","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"c8b0df394cd13be95ed6b75660ef4aae155a87c5","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        # if the devices are present into dom0"},{"line_number":130,"context_line":"        passthrough_devices \u003d _get_available_dev(hidden_devices)"},{"line_number":131,"context_line":"    except:"},{"line_number":132,"context_line":"        # TODO: Log the error"},{"line_number":133,"context_line":"        pass"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    return passthrough_devices"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAUH%2F%2Fr5I%3D","line":132,"in_reply_to":"AAAAUH%2F%2FxVY%3D","updated":"2014-01-09 16:17:32.000000000","message":"Ok yes I\u0027ve got it. If the open fails it will be catch as a XENAPI_PLUGIN_FAILURE.","commit_id":"c15812575064e45110d107f116ba11b6431bc575"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"8d094c929642dec386b89f82859a807a715d70c1","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        # if the devices are present into dom0"},{"line_number":130,"context_line":"        passthrough_devices \u003d _get_available_dev(hidden_devices)"},{"line_number":131,"context_line":"    except:"},{"line_number":132,"context_line":"        # TODO: Log the error"},{"line_number":133,"context_line":"        pass"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"    return passthrough_devices"}],"source_content_type":"application/octet-stream","patch_set":2,"id":"AAAAUH%2F%2FxVY%3D","line":132,"in_reply_to":"AAAAUH%2F%2FxkI%3D","updated":"2014-01-08 17:01:33.000000000","message":"+1","commit_id":"c15812575064e45110d107f116ba11b6431bc575"}],"plugins/xenserver/xenapi/etc/xapi.d/plugins/xenhost":[{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"1afbf7ac7b6cfca69c778ebb2f3bfe39490ef01d","unresolved":false,"context_lines":[{"line_number":421,"context_line":"                                  \"hex{2}:\"         # Bus:"},{"line_number":422,"context_line":"                                  \"hex{2}\\.\"        # Device."},{"line_number":423,"context_line":"                                  \"hex{1}\"          # Function"},{"line_number":424,"context_line":"                                  \")\")"},{"line_number":425,"context_line":"    bdf_list \u003d []"},{"line_number":426,"context_line":""},{"line_number":427,"context_line":"    for match in pciback_regex.findall(cmdline):"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"AAAAUH%2F%2FoFU%3D","line":424,"updated":"2014-01-10 10:58:04.000000000","message":"I very much prefer this!","commit_id":"616cbb914597714780a8050ac0adffadb9d322ea"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"faa722cae526a247d5e7d8b24c4e0ef264acb616","unresolved":false,"context_lines":[{"line_number":428,"context_line":"        # match holds something like (bdf1)(bdf2)..(bdfN)"},{"line_number":429,"context_line":"        for bdf in bdf_regex.findall(match):"},{"line_number":430,"context_line":"            bdf_list.append(bdf)"},{"line_number":431,"context_line":""},{"line_number":432,"context_line":"    return bdf_list"},{"line_number":433,"context_line":""},{"line_number":434,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":3,"id":"AAAAUH%2F%2FoWI%3D","line":431,"updated":"2014-01-10 10:09:11.000000000","message":"Should I use lspci to check that a pci devices is hidden from the command line, ie lspci -s xx:xx.x -k | grep pciback\nThe only problem I see if that -k option is only supported for kernel \u003e\u003d 2.6\nOtherwise it will be more efficient than parsing the proc command line and it removes all the complexity of the parsing.","commit_id":"616cbb914597714780a8050ac0adffadb9d322ea"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"89947504956bdc51ad7bfb51271252f287bcc51f","unresolved":false,"context_lines":[{"line_number":428,"context_line":"        # match holds something like (bdf1)(bdf2)..(bdfN)"},{"line_number":429,"context_line":"        for bdf in bdf_regex.findall(match):"},{"line_number":430,"context_line":"            bdf_list.append(bdf)"},{"line_number":431,"context_line":""},{"line_number":432,"context_line":"    return bdf_list"},{"line_number":433,"context_line":""},{"line_number":434,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":3,"id":"AAAAUH%2F%2Fm3k%3D","line":431,"in_reply_to":"AAAAUH%2F%2Fm%2Bo%3D","updated":"2014-01-10 14:56:42.000000000","message":"Sorry that it changes much of what\u0027s here - but I think it is a lot safer to use lspci output, so if you can post that then yes please.","commit_id":"616cbb914597714780a8050ac0adffadb9d322ea"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"24ee8524849ccf310916db179e046018fbcb1143","unresolved":false,"context_lines":[{"line_number":428,"context_line":"        # match holds something like (bdf1)(bdf2)..(bdfN)"},{"line_number":429,"context_line":"        for bdf in bdf_regex.findall(match):"},{"line_number":430,"context_line":"            bdf_list.append(bdf)"},{"line_number":431,"context_line":""},{"line_number":432,"context_line":"    return bdf_list"},{"line_number":433,"context_line":""},{"line_number":434,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":3,"id":"AAAAUH%2F%2Fm%2Bo%3D","line":431,"in_reply_to":"AAAAUH%2F%2FoG4%3D","updated":"2014-01-10 14:52:20.000000000","message":"No. The oldest kernel I saw is the one that is running on Xenserver 6.2\n\nI wrote the code by using the lspci output instead of parsing the command line and it is more clean I think. I can push it as patchset 4 if you want to see it.","commit_id":"616cbb914597714780a8050ac0adffadb9d322ea"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"1afbf7ac7b6cfca69c778ebb2f3bfe39490ef01d","unresolved":false,"context_lines":[{"line_number":428,"context_line":"        # match holds something like (bdf1)(bdf2)..(bdfN)"},{"line_number":429,"context_line":"        for bdf in bdf_regex.findall(match):"},{"line_number":430,"context_line":"            bdf_list.append(bdf)"},{"line_number":431,"context_line":""},{"line_number":432,"context_line":"    return bdf_list"},{"line_number":433,"context_line":""},{"line_number":434,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":3,"id":"AAAAUH%2F%2FoG4%3D","line":431,"in_reply_to":"AAAAUH%2F%2FoWI%3D","updated":"2014-01-10 10:58:04.000000000","message":"I\u0027m happy to depend on kernel \u003e\u003d 2.6; all supported versions of XenServer use \u003e\u003d 2.6 already - as does xenserver-core etc.\n\nHave you seen an environment where -k doesn\u0027t work?","commit_id":"616cbb914597714780a8050ac0adffadb9d322ea"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"faa722cae526a247d5e7d8b24c4e0ef264acb616","unresolved":false,"context_lines":[{"line_number":460,"context_line":"                              \u0027product_id\u0027: product_id,"},{"line_number":461,"context_line":"                              \u0027address\u0027: dev,"},{"line_number":462,"context_line":"                              \u0027dev_id\u0027: \u0027_\u0027.join([\u0027pci\u0027, dev]),"},{"line_number":463,"context_line":"                              \u0027dev_type\u0027: \u0027type-PCI\u0027,"},{"line_number":464,"context_line":"                              \u0027status\u0027: \u0027available\u0027})"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"    return available_dev"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"AAAAUH%2F%2FoVE%3D","line":463,"updated":"2014-01-10 10:09:11.000000000","message":"Should I also check other type or can I stay with type-PCI for the moment. I agree that it will be cleanner to set the right type. I looked in libvirt and they try to detect a file physfn (for VF) or  virtfn (for PF) in /sys/bus/pci/devices/xxxx:xx:xx.x\nShould be easy to add in fact :)","commit_id":"616cbb914597714780a8050ac0adffadb9d322ea"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"3abb122951e29e1d5c63a325f438f4664fddc44c","unresolved":false,"context_lines":[{"line_number":460,"context_line":"                              \u0027product_id\u0027: product_id,"},{"line_number":461,"context_line":"                              \u0027address\u0027: dev,"},{"line_number":462,"context_line":"                              \u0027dev_id\u0027: \u0027_\u0027.join([\u0027pci\u0027, dev]),"},{"line_number":463,"context_line":"                              \u0027dev_type\u0027: \u0027type-PCI\u0027,"},{"line_number":464,"context_line":"                              \u0027status\u0027: \u0027available\u0027})"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"    return available_dev"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"AAAAUH%2F%2Fmc0%3D","line":463,"in_reply_to":"AAAAUH%2F%2Fm3E%3D","updated":"2014-01-10 16:05:57.000000000","message":"Yes basically if ll /sys/bus/pci/devices/0000:81:10.5/ | grep physfn returns somethong then you are using type-VF, if you can grep virtfn, then you are type-PF and if nothing is grep then it is type-PCI.\n\nSo the idea is to write a function that takes the adress as parameter (0000:81:10.5) and return a string with the results of ls /sys/bus/pci/devices/0000:81:10.5/\n\nIt makes sense since a plugin is here to execute a command on the host. I will implement this.","commit_id":"616cbb914597714780a8050ac0adffadb9d322ea"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"89947504956bdc51ad7bfb51271252f287bcc51f","unresolved":false,"context_lines":[{"line_number":460,"context_line":"                              \u0027product_id\u0027: product_id,"},{"line_number":461,"context_line":"                              \u0027address\u0027: dev,"},{"line_number":462,"context_line":"                              \u0027dev_id\u0027: \u0027_\u0027.join([\u0027pci\u0027, dev]),"},{"line_number":463,"context_line":"                              \u0027dev_type\u0027: \u0027type-PCI\u0027,"},{"line_number":464,"context_line":"                              \u0027status\u0027: \u0027available\u0027})"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"    return available_dev"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"AAAAUH%2F%2Fm3E%3D","line":463,"in_reply_to":"AAAAUH%2F%2Fm7A%3D","updated":"2014-01-10 14:56:42.000000000","message":"I assume that /sys/bus/pci/devices/xxxx:xx:xx.x is interpreted by the libvirt code to say whether it should pass type-PCI or return the other types.\nI think that logic should stay on the nova side, and the relevant raw information (be it output from lspci or cating a file in /sys/bus/pci) should probably be passed from the plugin.","commit_id":"616cbb914597714780a8050ac0adffadb9d322ea"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"24ee8524849ccf310916db179e046018fbcb1143","unresolved":false,"context_lines":[{"line_number":460,"context_line":"                              \u0027product_id\u0027: product_id,"},{"line_number":461,"context_line":"                              \u0027address\u0027: dev,"},{"line_number":462,"context_line":"                              \u0027dev_id\u0027: \u0027_\u0027.join([\u0027pci\u0027, dev]),"},{"line_number":463,"context_line":"                              \u0027dev_type\u0027: \u0027type-PCI\u0027,"},{"line_number":464,"context_line":"                              \u0027status\u0027: \u0027available\u0027})"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"    return available_dev"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"AAAAUH%2F%2Fm7A%3D","line":463,"in_reply_to":"AAAAUH%2F%2FoGY%3D","updated":"2014-01-10 14:52:20.000000000","message":"What do you mean by raw content? Do you mean that the type is available from the lspci output for exemple? In fact I don\u0027t see how to get the type without using the sysfs.\n\nFor unit test it should be cool to put all the logic on the nova side as said by John. The thing that is intersting with using the lspci instead of the command line as you suggested is that it is easier to write the plugin that returns raw data obtained with \"lspci -vmmnk\" for pci devices that has pciback as driver.","commit_id":"616cbb914597714780a8050ac0adffadb9d322ea"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"1afbf7ac7b6cfca69c778ebb2f3bfe39490ef01d","unresolved":false,"context_lines":[{"line_number":460,"context_line":"                              \u0027product_id\u0027: product_id,"},{"line_number":461,"context_line":"                              \u0027address\u0027: dev,"},{"line_number":462,"context_line":"                              \u0027dev_id\u0027: \u0027_\u0027.join([\u0027pci\u0027, dev]),"},{"line_number":463,"context_line":"                              \u0027dev_type\u0027: \u0027type-PCI\u0027,"},{"line_number":464,"context_line":"                              \u0027status\u0027: \u0027available\u0027})"},{"line_number":465,"context_line":""},{"line_number":466,"context_line":"    return available_dev"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"AAAAUH%2F%2FoGY%3D","line":463,"in_reply_to":"AAAAUH%2F%2FoVE%3D","updated":"2014-01-10 10:58:04.000000000","message":"Yes, I think this should be added - however perhaps dev_type should be returned as the raw content, and logic that converts it should be on the nova side so can be unit tested?","commit_id":"616cbb914597714780a8050ac0adffadb9d322ea"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"1afbf7ac7b6cfca69c778ebb2f3bfe39490ef01d","unresolved":false,"context_lines":[{"line_number":487,"context_line":""},{"line_number":488,"context_line":"    # First we need to get the list of pci devices that are"},{"line_number":489,"context_line":"    # passed to pciback.hide option of the command line."},{"line_number":490,"context_line":"    hidden_devices \u003d _get_hidden_pci(cmdline)"},{"line_number":491,"context_line":""},{"line_number":492,"context_line":"    # Now we need to get information about vendor and product ID"},{"line_number":493,"context_line":"    # if the devices are present into dom0"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"AAAAUH%2F%2FoFc%3D","line":490,"updated":"2014-01-10 10:58:04.000000000","message":"I think this should be \"pciback_devices\" - hidden devices could be read by someone as \"things that I can\u0027t use\" if they don\u0027t have good knowledge of Xen PCI pass through.","commit_id":"616cbb914597714780a8050ac0adffadb9d322ea"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"24ee8524849ccf310916db179e046018fbcb1143","unresolved":false,"context_lines":[{"line_number":487,"context_line":""},{"line_number":488,"context_line":"    # First we need to get the list of pci devices that are"},{"line_number":489,"context_line":"    # passed to pciback.hide option of the command line."},{"line_number":490,"context_line":"    hidden_devices \u003d _get_hidden_pci(cmdline)"},{"line_number":491,"context_line":""},{"line_number":492,"context_line":"    # Now we need to get information about vendor and product ID"},{"line_number":493,"context_line":"    # if the devices are present into dom0"}],"source_content_type":"application/octet-stream","patch_set":3,"id":"AAAAUH%2F%2Fm6o%3D","line":490,"in_reply_to":"AAAAUH%2F%2FoFc%3D","updated":"2014-01-10 14:52:20.000000000","message":"Yes I completly agree","commit_id":"616cbb914597714780a8050ac0adffadb9d322ea"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"1aecc859f640837d933fd31bd696c954605db4b6","unresolved":false,"context_lines":[{"line_number":484,"context_line":""},{"line_number":485,"context_line":"    return passthrough_devices"},{"line_number":486,"context_line":""},{"line_number":487,"context_line":""},{"line_number":488,"context_line":"if __name__ \u003d\u003d \"__main__\":"},{"line_number":489,"context_line":"    # Support both serialized and non-serialized plugin approaches"},{"line_number":490,"context_line":"    _, methodname \u003d xmlrpclib.loads(sys.argv[1])"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAUH%2F%2FmYc%3D","line":487,"updated":"2014-01-10 16:16:51.000000000","message":"If it is ok with the code, the next step is to return the output of the lspci directly and parse it on the nova side. Agree?","commit_id":"eec6f5ddf4182466c66d90070dada3a798f9e0bf"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"1aecc859f640837d933fd31bd696c954605db4b6","unresolved":false,"context_lines":[{"line_number":490,"context_line":"    _, methodname \u003d xmlrpclib.loads(sys.argv[1])"},{"line_number":491,"context_line":"    if methodname in [\u0027get_passthrough_devices\u0027]:"},{"line_number":492,"context_line":"        utils.register_plugin_calls(get_passthrough_devices)"},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"    XenAPIPlugin.dispatch("},{"line_number":495,"context_line":"            {\"host_data\": host_data,"},{"line_number":496,"context_line":"            \"set_host_enabled\": set_host_enabled,"}],"source_content_type":"application/octet-stream","patch_set":4,"id":"AAAAUH%2F%2FmXw%3D","line":493,"updated":"2014-01-10 16:16:51.000000000","message":"I will add another method to get the output of the contents of the directory /sys/bus/pci/devices/ for a given address. It will allows to get information about the kind of PCI (type-PCI, type-VF or type-PV) also on the nova side. Agree?","commit_id":"eec6f5ddf4182466c66d90070dada3a798f9e0bf"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"e8360e1b22812ea9251801a7d085170fa46e2a41","unresolved":false,"context_lines":[{"line_number":398,"context_line":"    return out"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":""},{"line_number":401,"context_line":"def run_lspci_vmmnk(session):"},{"line_number":402,"context_line":"    \"\"\"Returns a string that is a list of pci devices."},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"    This string is obtained by running the command lspci. With -vmm option,"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"AAAAUH%2F%2FYoU%3D","line":401,"updated":"2014-01-14 11:16:14.000000000","message":"I\u0027d prefer a method name such as \"get_pci_device_details\"","commit_id":"ce9718240fc12ec33a44afd53c5dbb33fc051763"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"df8d2e92066f70222aca6d9bd74587509eefd766","unresolved":false,"context_lines":[{"line_number":398,"context_line":"    return out"},{"line_number":399,"context_line":""},{"line_number":400,"context_line":""},{"line_number":401,"context_line":"def run_lspci_vmmnk(session):"},{"line_number":402,"context_line":"    \"\"\"Returns a string that is a list of pci devices."},{"line_number":403,"context_line":""},{"line_number":404,"context_line":"    This string is obtained by running the command lspci. With -vmm option,"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"AAAAUH%2F%2FYdA%3D","line":401,"in_reply_to":"AAAAUH%2F%2FYoU%3D","updated":"2014-01-14 12:09:33.000000000","message":"Done","commit_id":"ce9718240fc12ec33a44afd53c5dbb33fc051763"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"f9dde143476c217885e393ec661b0bdaded56e23","unresolved":false,"context_lines":[{"line_number":408,"context_line":"    the \"-k\" option to get the kernel driver used if any."},{"line_number":409,"context_line":"    \"\"\""},{"line_number":410,"context_line":"    result \u003d _run_command([\"lspci\", \"-vmmnk\"])"},{"line_number":411,"context_line":"    return result"},{"line_number":412,"context_line":""},{"line_number":413,"context_line":""},{"line_number":414,"context_line":"def get_pci_type(session, pci_device):"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"AAAAUH%2F%2FYvo%3D","line":411,"updated":"2014-01-14 10:43:54.000000000","message":"I will compact it as: return _run_command(...)","commit_id":"ce9718240fc12ec33a44afd53c5dbb33fc051763"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"df8d2e92066f70222aca6d9bd74587509eefd766","unresolved":false,"context_lines":[{"line_number":408,"context_line":"    the \"-k\" option to get the kernel driver used if any."},{"line_number":409,"context_line":"    \"\"\""},{"line_number":410,"context_line":"    result \u003d _run_command([\"lspci\", \"-vmmnk\"])"},{"line_number":411,"context_line":"    return result"},{"line_number":412,"context_line":""},{"line_number":413,"context_line":""},{"line_number":414,"context_line":"def get_pci_type(session, pci_device):"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"AAAAUH%2F%2FYc4%3D","line":411,"in_reply_to":"AAAAUH%2F%2FYvo%3D","updated":"2014-01-14 12:09:33.000000000","message":"Done","commit_id":"ce9718240fc12ec33a44afd53c5dbb33fc051763"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"e8360e1b22812ea9251801a7d085170fa46e2a41","unresolved":false,"context_lines":[{"line_number":417,"context_line":"    pci-device -- The address of the pci device"},{"line_number":418,"context_line":"    \"\"\""},{"line_number":419,"context_line":"    # We need to add the domain if it is missing"},{"line_number":420,"context_line":"    if len(pci_device) \u003d\u003d len(\"BB:DD.F\"):"},{"line_number":421,"context_line":"        pci_device \u003d \"0000:\" + pci_device"},{"line_number":422,"context_line":"    output \u003d _run_command([\"ls\", \"/sys/bus/pci/devices/\" + pci_device + \"/\"])"},{"line_number":423,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":6,"id":"AAAAUH%2F%2FYoM%3D","line":420,"updated":"2014-01-14 11:16:14.000000000","message":"Rather than checking len, would it be safer to check for the number of colons?","commit_id":"ce9718240fc12ec33a44afd53c5dbb33fc051763"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"df8d2e92066f70222aca6d9bd74587509eefd766","unresolved":false,"context_lines":[{"line_number":417,"context_line":"    pci-device -- The address of the pci device"},{"line_number":418,"context_line":"    \"\"\""},{"line_number":419,"context_line":"    # We need to add the domain if it is missing"},{"line_number":420,"context_line":"    if len(pci_device) \u003d\u003d len(\"BB:DD.F\"):"},{"line_number":421,"context_line":"        pci_device \u003d \"0000:\" + pci_device"},{"line_number":422,"context_line":"    output \u003d _run_command([\"ls\", \"/sys/bus/pci/devices/\" + pci_device + \"/\"])"},{"line_number":423,"context_line":""}],"source_content_type":"application/octet-stream","patch_set":6,"id":"AAAAUH%2F%2FYaA%3D","line":420,"in_reply_to":"AAAAUH%2F%2FYoM%3D","updated":"2014-01-14 12:09:33.000000000","message":"Yes agree. And maybe shoud I check that pci_device is well formatted? I think that it is enough like this but I have a small doubt...","commit_id":"ce9718240fc12ec33a44afd53c5dbb33fc051763"},{"author":{"_account_id":6904,"name":"Guillaume Thouvenin","email":"gthouvenin@mirantis.com","username":"guillaume-thouvenin"},"change_message_id":"d5fdf6f53486037dd066e414616c5934008e0aca","unresolved":false,"context_lines":[{"line_number":426,"context_line":"    if \"virtfn\" in output:"},{"line_number":427,"context_line":"        return \"type-PF\""},{"line_number":428,"context_line":"    return \"type-PCI\""},{"line_number":429,"context_line":""},{"line_number":430,"context_line":""},{"line_number":431,"context_line":"if __name__ \u003d\u003d \"__main__\":"},{"line_number":432,"context_line":"    # Support both serialized and non-serialized plugin approaches"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"AAAAUH%2F%2FYx0%3D","line":429,"updated":"2014-01-14 10:36:53.000000000","message":"The logic is simple so should I move it on the nova side?","commit_id":"ce9718240fc12ec33a44afd53c5dbb33fc051763"},{"author":{"_account_id":6735,"name":"Bob Ball","email":"bob.ball@citrix.com","username":"bob-ball"},"change_message_id":"e8360e1b22812ea9251801a7d085170fa46e2a41","unresolved":false,"context_lines":[{"line_number":426,"context_line":"    if \"virtfn\" in output:"},{"line_number":427,"context_line":"        return \"type-PF\""},{"line_number":428,"context_line":"    return \"type-PCI\""},{"line_number":429,"context_line":""},{"line_number":430,"context_line":""},{"line_number":431,"context_line":"if __name__ \u003d\u003d \"__main__\":"},{"line_number":432,"context_line":"    # Support both serialized and non-serialized plugin approaches"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"AAAAUH%2F%2FYok%3D","line":429,"in_reply_to":"AAAAUH%2F%2FYx0%3D","updated":"2014-01-14 11:16:14.000000000","message":"I\u0027m quite happy with this logic being here","commit_id":"ce9718240fc12ec33a44afd53c5dbb33fc051763"}]}