)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"34f01076f184b35f485e8ac2f2794bd77a9483a0","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The is_physical_function util function assume that if in the pci device"},{"line_number":10,"context_line":"path exits virtfn* then it is PF (physical function)."},{"line_number":11,"context_line":"the assumtion is wrong were the virtfn* exist  only if the PF"},{"line_number":12,"context_line":"has VF (virtual functions). if the PF has 0 VF(s) then the virtfn* does not exist."},{"line_number":13,"context_line":"The solution is to relay on the physfn file, therefore if pci device has physfn"},{"line_number":14,"context_line":"it means it VF otherwise it is a PF."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7a2fa921_423bd8d1","line":11,"updated":"2015-10-06 16:02:35.000000000","message":"nit: s/were/where","commit_id":"e692a5218d92b6ad51510a28d1db5adef64bf4c5"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"9a86e165d72889eaf14e9376833e24d63e10bed8","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The is_physical_function util function assume that if in the pci device"},{"line_number":10,"context_line":"path exits virtfn* then it is PF (physical function)."},{"line_number":11,"context_line":"the assumtion is wrong were the virtfn* exist  only if the PF"},{"line_number":12,"context_line":"has VF (virtual functions). if the PF has 0 VF(s) then the virtfn* does not exist."},{"line_number":13,"context_line":"The solution is to relay on the physfn file, therefore if pci device has physfn"},{"line_number":14,"context_line":"it means it VF otherwise it is a PF."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"da85f559_83236055","line":11,"in_reply_to":"7a2fa921_423bd8d1","updated":"2015-11-12 14:59:15.000000000","message":"Done","commit_id":"e692a5218d92b6ad51510a28d1db5adef64bf4c5"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"34f01076f184b35f485e8ac2f2794bd77a9483a0","unresolved":false,"context_lines":[{"line_number":10,"context_line":"path exits virtfn* then it is PF (physical function)."},{"line_number":11,"context_line":"the assumtion is wrong were the virtfn* exist  only if the PF"},{"line_number":12,"context_line":"has VF (virtual functions). if the PF has 0 VF(s) then the virtfn* does not exist."},{"line_number":13,"context_line":"The solution is to relay on the physfn file, therefore if pci device has physfn"},{"line_number":14,"context_line":"it means it VF otherwise it is a PF."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Closes-Bug: #1499204"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7a2fa921_a25c2c26","line":13,"updated":"2015-10-06 16:02:35.000000000","message":"nit: s/relay/rely/","commit_id":"e692a5218d92b6ad51510a28d1db5adef64bf4c5"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"34f01076f184b35f485e8ac2f2794bd77a9483a0","unresolved":false,"context_lines":[{"line_number":11,"context_line":"the assumtion is wrong were the virtfn* exist  only if the PF"},{"line_number":12,"context_line":"has VF (virtual functions). if the PF has 0 VF(s) then the virtfn* does not exist."},{"line_number":13,"context_line":"The solution is to relay on the physfn file, therefore if pci device has physfn"},{"line_number":14,"context_line":"it means it VF otherwise it is a PF."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Closes-Bug: #1499204"},{"line_number":17,"context_line":"Change-Id: I8990c36fb1d6c66093a465930ff3f0948dd64986"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7a2fa921_a273ccb0","line":14,"updated":"2015-10-06 16:02:35.000000000","message":"nit: s/it VF/it is a VF","commit_id":"e692a5218d92b6ad51510a28d1db5adef64bf4c5"}],"nova/pci/utils.py":[{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"4dc2a8ed5bbf7dbb2716ab123a3f2394e08c476e","unresolved":false,"context_lines":[{"line_number":32,"context_line":"                                             replace(\"hex\", \"[\\da-fA-F]\")."},{"line_number":33,"context_line":"                                             replace(\"oct\", \"[0-7]\"))"},{"line_number":34,"context_line":"_PCI_ADDRESS_REGEX \u003d re.compile(_PCI_ADDRESS_PATTERN)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"_VIRTFN_RE \u003d re.compile(\"virtfn\\d+\")"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_9c52cadc","line":35,"updated":"2015-09-25 04:09:55.000000000","message":"here","commit_id":"bba17a9a171e79df9f69b490c46a16d3e89f8266"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"4dc2a8ed5bbf7dbb2716ab123a3f2394e08c476e","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    try:"},{"line_number":95,"context_line":"        dev_info \u003d os.listdir(dev_path)"},{"line_number":96,"context_line":"        for dev_file in dev_info:"},{"line_number":97,"context_line":"            if dev_file.endswith(\u0027physfn\u0027):"},{"line_number":98,"context_line":"                return False"},{"line_number":99,"context_line":"        return True"},{"line_number":100,"context_line":"    except Exception:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_1c46ba23","line":97,"updated":"2015-09-25 04:09:55.000000000","message":"How about defining _PHYSFN_RE after _VIRTFN_RE and using it like\n\n if _PHYSFN_RE.match(dev_file):\n\nfor code readability ?","commit_id":"bba17a9a171e79df9f69b490c46a16d3e89f8266"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"429aeefc88769ad2664ce65d97f85b1240f65dcd","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    try:"},{"line_number":95,"context_line":"        dev_info \u003d os.listdir(dev_path)"},{"line_number":96,"context_line":"        for dev_file in dev_info:"},{"line_number":97,"context_line":"            if dev_file.endswith(\u0027physfn\u0027):"},{"line_number":98,"context_line":"                return False"},{"line_number":99,"context_line":"        return True"},{"line_number":100,"context_line":"    except Exception:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1a9d01_5387400f","line":97,"in_reply_to":"9a1a9d01_1c46ba23","updated":"2015-09-25 06:40:06.000000000","message":"Done","commit_id":"bba17a9a171e79df9f69b490c46a16d3e89f8266"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"0571c15468b0d08355ea09d44bcf291c6fdea4cf","unresolved":false,"context_lines":[{"line_number":95,"context_line":"    try:"},{"line_number":96,"context_line":"        dev_info \u003d os.listdir(dev_path)"},{"line_number":97,"context_line":"        for dev_file in dev_info:"},{"line_number":98,"context_line":"            if _PHYSFN_RE.match(dev_file):"},{"line_number":99,"context_line":"                return False"},{"line_number":100,"context_line":"        return True"},{"line_number":101,"context_line":"    except Exception:"}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_6fba8d0c","line":98,"updated":"2015-11-09 17:23:27.000000000","message":"Another solution would be to keep the existing code and add a check to see if sriov_numvfs is present in the path and if it is \u003e 0.  \n\nNot sure what is the best solution ...","commit_id":"e692a5218d92b6ad51510a28d1db5adef64bf4c5"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"34f01076f184b35f485e8ac2f2794bd77a9483a0","unresolved":false,"context_lines":[{"line_number":97,"context_line":"        for dev_file in dev_info:"},{"line_number":98,"context_line":"            if _PHYSFN_RE.match(dev_file):"},{"line_number":99,"context_line":"                return False"},{"line_number":100,"context_line":"        return True"},{"line_number":101,"context_line":"    except Exception:"},{"line_number":102,"context_line":"        LOG.error(_LE(\"PCI device %s not found\") % dev_path)"},{"line_number":103,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a2fa921_37f94081","line":100,"updated":"2015-10-06 16:02:35.000000000","message":"This would make all non-VF PCI devices as physical functions (not exactly in the sense of SR-IOV). On the other hand, defining this as is_virtual_function() would be more accurate (a PCI device containing a physfn). The caller can call !is_virtual_function() in place of calling is_physical_function(, which basically achieves the same as the original function ).","commit_id":"e692a5218d92b6ad51510a28d1db5adef64bf4c5"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"6da2929770040ec5a5e2dab91651932ac8be9adf","unresolved":false,"context_lines":[{"line_number":97,"context_line":"        for dev_file in dev_info:"},{"line_number":98,"context_line":"            if _PHYSFN_RE.match(dev_file):"},{"line_number":99,"context_line":"                return False"},{"line_number":100,"context_line":"        return True"},{"line_number":101,"context_line":"    except Exception:"},{"line_number":102,"context_line":"        LOG.error(_LE(\"PCI device %s not found\") % dev_path)"},{"line_number":103,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a2fa921_85aa57d3","line":100,"in_reply_to":"7a2fa921_37f94081","updated":"2015-10-06 16:22:35.000000000","message":"I see it been used only in the nova/pci/devspec.py \n\nhttps://github.com/openstack/nova/search?utf8\u003d%E2%9C%93\u0026q\u003dutils.is_physical_function\u0026type\u003dCode\n\nDid I miss something?","commit_id":"e692a5218d92b6ad51510a28d1db5adef64bf4c5"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"9a86e165d72889eaf14e9376833e24d63e10bed8","unresolved":false,"context_lines":[{"line_number":97,"context_line":"        for dev_file in dev_info:"},{"line_number":98,"context_line":"            if _PHYSFN_RE.match(dev_file):"},{"line_number":99,"context_line":"                return False"},{"line_number":100,"context_line":"        return True"},{"line_number":101,"context_line":"    except Exception:"},{"line_number":102,"context_line":"        LOG.error(_LE(\"PCI device %s not found\") % dev_path)"},{"line_number":103,"context_line":"        return False"}],"source_content_type":"text/x-python","patch_set":2,"id":"da85f559_03a450b7","line":100,"in_reply_to":"7a2fa921_85aa57d3","updated":"2015-11-12 14:59:15.000000000","message":"I change it to check if sriov_numvfs exist and if so it a PF","commit_id":"e692a5218d92b6ad51510a28d1db5adef64bf4c5"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"ebe85c226b6dacf5952b92b91009cd835576cdca","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        dev_path \u003d \"/sys/class/net/%s/device\" % ifname"},{"line_number":79,"context_line":"        dev_info \u003d os.listdir(dev_path)"},{"line_number":80,"context_line":"        for dev_file in dev_info:"},{"line_number":81,"context_line":"            if _VIRTFN_RE.match(dev_file):"},{"line_number":82,"context_line":"                return os.readlink(dev_path).strip(\"./\"), True"},{"line_number":83,"context_line":"        else:"},{"line_number":84,"context_line":"            return os.readlink(dev_path).strip(\"./\"), False"}],"source_content_type":"text/x-python","patch_set":3,"id":"da85f559_970aac78","line":81,"updated":"2015-11-12 20:13:39.000000000","message":"This is another instance of checking physical function by virtfn.","commit_id":"5c9da86081052ea2ac205e088de8a0345d5fb27b"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"d6bb7b544af64c6618f9d3ff680a169ea4562390","unresolved":false,"context_lines":[{"line_number":78,"context_line":"        dev_path \u003d \"/sys/class/net/%s/device\" % ifname"},{"line_number":79,"context_line":"        dev_info \u003d os.listdir(dev_path)"},{"line_number":80,"context_line":"        for dev_file in dev_info:"},{"line_number":81,"context_line":"            if _VIRTFN_RE.match(dev_file):"},{"line_number":82,"context_line":"                return os.readlink(dev_path).strip(\"./\"), True"},{"line_number":83,"context_line":"        else:"},{"line_number":84,"context_line":"            return os.readlink(dev_path).strip(\"./\"), False"}],"source_content_type":"text/x-python","patch_set":3,"id":"da85f559_abc0922b","line":81,"in_reply_to":"da85f559_970aac78","updated":"2015-11-13 08:42:08.000000000","message":"good catch I missed that :)","commit_id":"5c9da86081052ea2ac205e088de8a0345d5fb27b"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"fc0cd1d5c019bcf6062a494eb64a983c808390d9","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    sriov_vfnum_file \u003d dev_path + \u0027sriov_numvfs\u0027"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    if not os.path.isdir(dev_path):"},{"line_number":97,"context_line":"        LOG.error(_LE(\"PCI device %s not found\") % dev_path)"},{"line_number":98,"context_line":"    if os.path.isfile(sriov_vfnum_file):"},{"line_number":99,"context_line":"        return True"},{"line_number":100,"context_line":"    return False"}],"source_content_type":"text/x-python","patch_set":3,"id":"da85f559_0c8b1d20","line":97,"updated":"2015-11-12 15:41:24.000000000","message":"the above two lines are not necessary, I think.","commit_id":"5c9da86081052ea2ac205e088de8a0345d5fb27b"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"bb3c1bfa187112994ac99be59dc659029fc505b9","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    sriov_vfnum_file \u003d dev_path + \u0027sriov_numvfs\u0027"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    if not os.path.isdir(dev_path):"},{"line_number":97,"context_line":"        LOG.error(_LE(\"PCI device %s not found\") % dev_path)"},{"line_number":98,"context_line":"    if os.path.isfile(sriov_vfnum_file):"},{"line_number":99,"context_line":"        return True"},{"line_number":100,"context_line":"    return False"}],"source_content_type":"text/x-python","patch_set":3,"id":"da85f559_787aa3be","line":97,"in_reply_to":"da85f559_0c8b1d20","updated":"2015-11-12 16:42:16.000000000","message":"I think it is need to log an error when we get a pci device which doesn\u0027t have a path. also the old code did something like this with  os.listdir throw an exception. \n\nbut I think I should update the code \nto :\nif not os.path.isdir(dev_path)\n    LOG.error(_LE ....\nelif os.path.isfile ...\n    return True\nreturn False\n\nWhat do you thing?","commit_id":"5c9da86081052ea2ac205e088de8a0345d5fb27b"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"d6bb7b544af64c6618f9d3ff680a169ea4562390","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    sriov_vfnum_file \u003d dev_path + \u0027sriov_numvfs\u0027"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    if not os.path.isdir(dev_path):"},{"line_number":97,"context_line":"        LOG.error(_LE(\"PCI device %s not found\") % dev_path)"},{"line_number":98,"context_line":"    if os.path.isfile(sriov_vfnum_file):"},{"line_number":99,"context_line":"        return True"},{"line_number":100,"context_line":"    return False"}],"source_content_type":"text/x-python","patch_set":3,"id":"da85f559_6be14ad0","line":97,"in_reply_to":"da85f559_37a9807c","updated":"2015-11-13 08:42:08.000000000","message":"Done","commit_id":"5c9da86081052ea2ac205e088de8a0345d5fb27b"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"b5cea87f8185bffcc592a1628942ab022597945e","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    sriov_vfnum_file \u003d dev_path + \u0027sriov_numvfs\u0027"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    if not os.path.isdir(dev_path):"},{"line_number":97,"context_line":"        LOG.error(_LE(\"PCI device %s not found\") % dev_path)"},{"line_number":98,"context_line":"    if os.path.isfile(sriov_vfnum_file):"},{"line_number":99,"context_line":"        return True"},{"line_number":100,"context_line":"    return False"}],"source_content_type":"text/x-python","patch_set":3,"id":"da85f559_c2b159e1","line":97,"in_reply_to":"da85f559_787aa3be","updated":"2015-11-12 17:22:52.000000000","message":"listdir gives an exception if dev_path is not a dir. But isfile does not. That\u0027s why it\u0027s not needed.\n\nI\u0027d say simply:\n  return os.path.isfile(dev_path + \u0027sir_numvfs\u0027)","commit_id":"5c9da86081052ea2ac205e088de8a0345d5fb27b"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"ebe85c226b6dacf5952b92b91009cd835576cdca","unresolved":false,"context_lines":[{"line_number":94,"context_line":"    sriov_vfnum_file \u003d dev_path + \u0027sriov_numvfs\u0027"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"    if not os.path.isdir(dev_path):"},{"line_number":97,"context_line":"        LOG.error(_LE(\"PCI device %s not found\") % dev_path)"},{"line_number":98,"context_line":"    if os.path.isfile(sriov_vfnum_file):"},{"line_number":99,"context_line":"        return True"},{"line_number":100,"context_line":"    return False"}],"source_content_type":"text/x-python","patch_set":3,"id":"da85f559_37a9807c","line":97,"in_reply_to":"da85f559_c2b159e1","updated":"2015-11-12 20:13:39.000000000","message":"Hmm, we don\u0027t have something like is_valid_pci_address(). But combining both should be fine for our purpose. In that case, I would do:\n    if os.path.isdir(dev_path):\n       return os.path.isfile(dev_path + \u0027sir_numvfs\u0027)\n    else:\n       LOG.error(_LE(\"PCI device %s not found\") % dev_path)","commit_id":"5c9da86081052ea2ac205e088de8a0345d5fb27b"},{"author":{"_account_id":6685,"name":"Baodong (Robert) Li","email":"baoli@cisco.com","username":"baoli"},"change_message_id":"1bafa937f822b94108474dddda52dab539a3e7a5","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"_VIRTFN_RE \u003d re.compile(\"virtfn\\d+\")"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"_SRIOV_VFNUM \u003d \"sriov_vfnum\""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"def pci_device_prop_match(pci_dev, specs):"}],"source_content_type":"text/x-python","patch_set":4,"id":"da85f559_6492c259","line":38,"updated":"2015-11-13 16:48:00.000000000","message":"just found that the file name is \"sriov_numvfs\" on my system.","commit_id":"8c620017d96f9ab664ee78a3cac392685d21ec7a"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"cad8c28872ae0f29b1968baac1445da86fbfa365","unresolved":false,"context_lines":[{"line_number":35,"context_line":""},{"line_number":36,"context_line":"_VIRTFN_RE \u003d re.compile(\"virtfn\\d+\")"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"_SRIOV_VFNUM \u003d \"sriov_vfnum\""},{"line_number":39,"context_line":""},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"def pci_device_prop_match(pci_dev, specs):"}],"source_content_type":"text/x-python","patch_set":4,"id":"da85f559_ef22fda2","line":38,"in_reply_to":"da85f559_6492c259","updated":"2015-11-13 23:20:10.000000000","message":"Sorry about that it was correct in the previous patch","commit_id":"8c620017d96f9ab664ee78a3cac392685d21ec7a"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"7e8f19113ede3c73a77a8dbaec05eab2cd212288","unresolved":false,"context_lines":[{"line_number":90,"context_line":"def is_physical_function(domain, bus, slot, function):"},{"line_number":91,"context_line":"    dev_path \u003d \"/sys/bus/pci/devices/%(d)s:%(b)s:%(s)s.%(f)s/\" % {"},{"line_number":92,"context_line":"        \"d\": domain, \"b\": bus,"},{"line_number":93,"context_line":"        \"s\": slot, \"f\": function}"},{"line_number":94,"context_line":"    if os.path.isdir(dev_path):"},{"line_number":95,"context_line":"        sriov_totalvfs \u003d 0"},{"line_number":96,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a8ffd7b_654b5288","line":93,"updated":"2015-12-02 12:29:47.000000000","message":"Nitpick: Looks a bit awkward to me, maybe I would put those last two lines on the same line.","commit_id":"283cd4f49bc589e046ef12a9c38c8c59146de2ea"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"06c4844d371784414e589bfcbc4676593e7cc9a8","unresolved":false,"context_lines":[{"line_number":90,"context_line":"def is_physical_function(domain, bus, slot, function):"},{"line_number":91,"context_line":"    dev_path \u003d \"/sys/bus/pci/devices/%(d)s:%(b)s:%(s)s.%(f)s/\" % {"},{"line_number":92,"context_line":"        \"d\": domain, \"b\": bus,"},{"line_number":93,"context_line":"        \"s\": slot, \"f\": function}"},{"line_number":94,"context_line":"    if os.path.isdir(dev_path):"},{"line_number":95,"context_line":"        sriov_totalvfs \u003d 0"},{"line_number":96,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a8ffd7b_53593434","line":93,"in_reply_to":"9a8ffd7b_654b5288","updated":"2015-12-02 12:47:38.000000000","message":"Done","commit_id":"283cd4f49bc589e046ef12a9c38c8c59146de2ea"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"3c7b3277a1c97de99c87a7324eb0aba5320400e8","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                return (os.readlink(dev_path).strip(\"./\"),"},{"line_number":84,"context_line":"                        sriov_totalvfs \u003e 0)"},{"line_number":85,"context_line":"        except (IOError, ValueError):"},{"line_number":86,"context_line":"            return os.readlink(dev_path).strip(\"./\"), False"},{"line_number":87,"context_line":"    return None, False"},{"line_number":88,"context_line":""},{"line_number":89,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"7a740942_db4c51ca","line":86,"updated":"2015-12-07 17:40:58.000000000","message":"You could further simplify this code by having a \"is_physical\" variable and setting it accordingly in the two cases above.\n\nYou can then simply have this return statement: return os.readlink(dev_path).strip(\"./\"), is_physical\n\nBut this might be a question of style ...  I won\u0027t -1 it.","commit_id":"5e64bca37fadff862d7cc2c93e9aa769bb712036"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"64ca369ccb789d64eae24d1b08fc4f41471cdee4","unresolved":false,"context_lines":[{"line_number":4831,"context_line":"                            \u0027phys_function\u0027: phys_address,"},{"line_number":4832,"context_line":"                        }"},{"line_number":4833,"context_line":""},{"line_number":4834,"context_line":"            # Note(moshele): libvirt \u003c 1.3 reported virt_functions capability"},{"line_number":4835,"context_line":"            # only when VFs are enabled. The check below is a workaround"},{"line_number":4836,"context_line":"            # to get the currect report regardless of whether or not any"},{"line_number":4837,"context_line":"            # VFs are enabled for the device."},{"line_number":4838,"context_line":"            # libvirt 1.3 fix f391889f4e942e22b9ef8ecca492de05106ce41e"},{"line_number":4839,"context_line":"            is_physical_function \u003d pci_utils.is_physical_function("},{"line_number":4840,"context_line":"                *pci_utils.get_pci_address_fields(pci_address))"},{"line_number":4841,"context_line":"            if is_physical_function:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a8ffd7b_d0e79aaa","line":4838,"range":{"start_line":4834,"start_character":12,"end_line":4838,"end_character":70},"updated":"2015-12-01 13:34:31.000000000","message":"Do we want to do this only if libvirt is not doing the right thing? There are several places in the driver where we check the version and decide what to do based on that. Seems like a more complete approach.","commit_id":"ade963967d335d2723d02d0164e647fd637b19a9"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"fbefc04499b7b32febb3e1c466e2d82dc6fa678b","unresolved":false,"context_lines":[{"line_number":4835,"context_line":"            # only when VFs are enabled. The check below is a workaround"},{"line_number":4836,"context_line":"            # to get the currect report regardless of whether or not any"},{"line_number":4837,"context_line":"            # VFs are enabled for the device."},{"line_number":4838,"context_line":"            # libvirt 1.3 fix f391889f4e942e22b9ef8ecca492de05106ce41e"},{"line_number":4839,"context_line":"            is_physical_function \u003d pci_utils.is_physical_function("},{"line_number":4840,"context_line":"                *pci_utils.get_pci_address_fields(pci_address))"},{"line_number":4841,"context_line":"            if is_physical_function:"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a8ffd7b_158667db","line":4838,"in_reply_to":"9a8ffd7b_d0e79aaa","updated":"2015-12-01 16:21:58.000000000","message":"Done","commit_id":"ade963967d335d2723d02d0164e647fd637b19a9"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"7e8f19113ede3c73a77a8dbaec05eab2cd212288","unresolved":false,"context_lines":[{"line_number":4839,"context_line":""},{"line_number":4840,"context_line":"            # Note(moshele): libvirt \u003c 1.3 reported virt_functions capability"},{"line_number":4841,"context_line":"            # only when VFs are enabled. The check below is a workaround"},{"line_number":4842,"context_line":"            # to get the currect report regardless of whether or not any"},{"line_number":4843,"context_line":"            # VFs are enabled for the device."},{"line_number":4844,"context_line":"            if not self._host.has_min_version("},{"line_number":4845,"context_line":"                MIN_LIBVIRT_PF_WITH_NO_VFS_CAP_VERSION):"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a8ffd7b_28551fc1","line":4842,"updated":"2015-12-02 12:29:47.000000000","message":"s/currect/correct/","commit_id":"283cd4f49bc589e046ef12a9c38c8c59146de2ea"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"06c4844d371784414e589bfcbc4676593e7cc9a8","unresolved":false,"context_lines":[{"line_number":4839,"context_line":""},{"line_number":4840,"context_line":"            # Note(moshele): libvirt \u003c 1.3 reported virt_functions capability"},{"line_number":4841,"context_line":"            # only when VFs are enabled. The check below is a workaround"},{"line_number":4842,"context_line":"            # to get the currect report regardless of whether or not any"},{"line_number":4843,"context_line":"            # VFs are enabled for the device."},{"line_number":4844,"context_line":"            if not self._host.has_min_version("},{"line_number":4845,"context_line":"                MIN_LIBVIRT_PF_WITH_NO_VFS_CAP_VERSION):"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a8ffd7b_937a3c87","line":4842,"in_reply_to":"9a8ffd7b_28551fc1","updated":"2015-12-02 12:47:38.000000000","message":"Done","commit_id":"283cd4f49bc589e046ef12a9c38c8c59146de2ea"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"862139da9d8fbde1d19dfb179663ef6e01e3a2d9","unresolved":false,"context_lines":[{"line_number":4821,"context_line":"            hypervisor."},{"line_number":4822,"context_line":"            \"\"\""},{"line_number":4823,"context_line":"            for fun_cap in cfgdev.pci_capability.fun_capability:"},{"line_number":4824,"context_line":"                if len(fun_cap.device_addrs) !\u003d 0:"},{"line_number":4825,"context_line":"                    if (fun_cap.type \u003d\u003d \u0027virt_functions\u0027):"},{"line_number":4826,"context_line":"                        return {"},{"line_number":4827,"context_line":"                            \u0027dev_type\u0027: fields.PciDeviceType.SRIOV_PF,"}],"source_content_type":"text/x-python","patch_set":9,"id":"7a740942_62589057","line":4824,"updated":"2015-12-03 19:54:35.000000000","message":"when using libvirt \u003e\u003d 1.3 this check causing skip  the virt_functions  tag if no VF are enabled, so it is need to be remove","commit_id":"8d6da6dcac061fa3d8625da84b78508f986baa41"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"6286513d40e2d4d153d93da0319b508bfe776566","unresolved":false,"context_lines":[{"line_number":4822,"context_line":"            \"\"\""},{"line_number":4823,"context_line":"            for fun_cap in cfgdev.pci_capability.fun_capability:"},{"line_number":4824,"context_line":"                if len(fun_cap.device_addrs) !\u003d 0:"},{"line_number":4825,"context_line":"                    if (fun_cap.type \u003d\u003d \u0027virt_functions\u0027):"},{"line_number":4826,"context_line":"                        return {"},{"line_number":4827,"context_line":"                            \u0027dev_type\u0027: fields.PciDeviceType.SRIOV_PF,"},{"line_number":4828,"context_line":"                        }"}],"source_content_type":"text/x-python","patch_set":9,"id":"9a8ffd7b_be69408e","line":4825,"range":{"start_line":4825,"start_character":56,"end_line":4825,"end_character":57},"updated":"2015-12-02 14:43:48.000000000","message":"Not sure why these brackets are needed but meh\n\nJust for the record - this is the commit that fixes the issue we are working around here:\n\nhttps://libvirt.org/git/?p\u003dlibvirt.git;a\u003dcommit;h\u003df391889f4e942e22b9ef8ecca492de05106ce41e\n\nSo looking at it - this is correct. After that commit - virtual_functions will be present regardless.","commit_id":"8d6da6dcac061fa3d8625da84b78508f986baa41"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"862139da9d8fbde1d19dfb179663ef6e01e3a2d9","unresolved":false,"context_lines":[{"line_number":4822,"context_line":"            \"\"\""},{"line_number":4823,"context_line":"            for fun_cap in cfgdev.pci_capability.fun_capability:"},{"line_number":4824,"context_line":"                if len(fun_cap.device_addrs) !\u003d 0:"},{"line_number":4825,"context_line":"                    if (fun_cap.type \u003d\u003d \u0027virt_functions\u0027):"},{"line_number":4826,"context_line":"                        return {"},{"line_number":4827,"context_line":"                            \u0027dev_type\u0027: fields.PciDeviceType.SRIOV_PF,"},{"line_number":4828,"context_line":"                        }"}],"source_content_type":"text/x-python","patch_set":9,"id":"7a740942_02a42c55","line":4825,"in_reply_to":"9a8ffd7b_be69408e","updated":"2015-12-03 19:54:35.000000000","message":"yes I put in in the commit in line 425","commit_id":"8d6da6dcac061fa3d8625da84b78508f986baa41"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"6db4ade44ac49d8c4c455bdc7589cdc22c76e758","unresolved":false,"context_lines":[{"line_number":4825,"context_line":"                    return {"},{"line_number":4826,"context_line":"                        \u0027dev_type\u0027: fields.PciDeviceType.SRIOV_PF,"},{"line_number":4827,"context_line":"                    }"},{"line_number":4828,"context_line":"                if fun_cap.type \u003d\u003d \u0027phys_function\u0027:"},{"line_number":4829,"context_line":"                    phys_address \u003d \"%04x:%02x:%02x.%01x\" % ("},{"line_number":4830,"context_line":"                        fun_cap.device_addrs[0][0],"},{"line_number":4831,"context_line":"                        fun_cap.device_addrs[0][1],"}],"source_content_type":"text/x-python","patch_set":11,"id":"7a740942_5d010437","line":4828,"range":{"start_line":4828,"start_character":16,"end_line":4828,"end_character":51},"updated":"2015-12-08 14:25:32.000000000","message":"Does it make sense to check the length of fun_cap.device_addrs here to avoid IndexErrors? Seems like it does even if for our own sanity?","commit_id":"5e64bca37fadff862d7cc2c93e9aa769bb712036"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"19077006f9b715705c8b513615f26eb4b278c730","unresolved":false,"context_lines":[{"line_number":4825,"context_line":"                    return {"},{"line_number":4826,"context_line":"                        \u0027dev_type\u0027: fields.PciDeviceType.SRIOV_PF,"},{"line_number":4827,"context_line":"                    }"},{"line_number":4828,"context_line":"                if fun_cap.type \u003d\u003d \u0027phys_function\u0027:"},{"line_number":4829,"context_line":"                    phys_address \u003d \"%04x:%02x:%02x.%01x\" % ("},{"line_number":4830,"context_line":"                        fun_cap.device_addrs[0][0],"},{"line_number":4831,"context_line":"                        fun_cap.device_addrs[0][1],"}],"source_content_type":"text/x-python","patch_set":11,"id":"7a740942_88d92b86","line":4828,"in_reply_to":"7a740942_5d010437","updated":"2015-12-09 13:01:45.000000000","message":"IMO, the validation in not belong here I would expect that it be done in https://github.com/openstack/nova/blob/master/nova/virt/libvirt/config.py#L2071-L2130, but looking on the config.py file I don\u0027t seen any validation.\n\nI can add validation for the phys_function type that will require valid device address but I think it belong to different patch.","commit_id":"5e64bca37fadff862d7cc2c93e9aa769bb712036"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"3a886f9998d9e2550e0159f00fc732877a0191a0","unresolved":false,"context_lines":[{"line_number":4825,"context_line":"                    return {"},{"line_number":4826,"context_line":"                        \u0027dev_type\u0027: fields.PciDeviceType.SRIOV_PF,"},{"line_number":4827,"context_line":"                    }"},{"line_number":4828,"context_line":"                if fun_cap.type \u003d\u003d \u0027phys_function\u0027:"},{"line_number":4829,"context_line":"                    phys_address \u003d \"%04x:%02x:%02x.%01x\" % ("},{"line_number":4830,"context_line":"                        fun_cap.device_addrs[0][0],"},{"line_number":4831,"context_line":"                        fun_cap.device_addrs[0][1],"}],"source_content_type":"text/x-python","patch_set":11,"id":"7a740942_238ff876","line":4828,"in_reply_to":"7a740942_68dabfeb","updated":"2015-12-09 15:04:19.000000000","message":"Done","commit_id":"5e64bca37fadff862d7cc2c93e9aa769bb712036"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"973bd40d1a76c0c481864777bd61ab5204b9e8cc","unresolved":false,"context_lines":[{"line_number":4825,"context_line":"                    return {"},{"line_number":4826,"context_line":"                        \u0027dev_type\u0027: fields.PciDeviceType.SRIOV_PF,"},{"line_number":4827,"context_line":"                    }"},{"line_number":4828,"context_line":"                if fun_cap.type \u003d\u003d \u0027phys_function\u0027:"},{"line_number":4829,"context_line":"                    phys_address \u003d \"%04x:%02x:%02x.%01x\" % ("},{"line_number":4830,"context_line":"                        fun_cap.device_addrs[0][0],"},{"line_number":4831,"context_line":"                        fun_cap.device_addrs[0][1],"}],"source_content_type":"text/x-python","patch_set":11,"id":"7a740942_68dabfeb","line":4828,"in_reply_to":"7a740942_88d92b86","updated":"2015-12-09 13:13:03.000000000","message":"Well we could do validation as a separate patch, but that\u0027s not my point. Previously this code guarded agianst device.addrs having 0 length. currently we don\u0027t have that check but do attempt to get items from a list, which seems like a unintended change (even though I assume if it ever happens it\u0027s really a libvirt bug).\n\nMy point is only that we should not remove the check that was there entirely.","commit_id":"5e64bca37fadff862d7cc2c93e9aa769bb712036"}]}