)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"9fa8998ae8e65c33c75b99c66e6ea7f98bec528d","unresolved":false,"context_lines":[{"line_number":14,"context_line":"an empty string, only specifying physical_address, and only providing"},{"line_number":15,"context_line":"one of the two ids."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Added tests for these invalid entries."},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"Added exception/message for these invalid entries."},{"line_number":20,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1aa78d24_93addb9d","line":17,"updated":"2016-07-15 13:53:45.000000000","message":"You don\u0027t need to specify that you added unit tests for those.  Unit tests for patches are de facto mandatory for fixes.","commit_id":"5b65f93ca5f006ebc2e6be65889d9c6f77cdae17"}],"nova/exception.py":[{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"532fd11cfe7df662bb08391beee0ba2ea33d27a4","unresolved":false,"context_lines":[{"line_number":1748,"context_line":""},{"line_number":1749,"context_line":"class PciDeviceInvalidEntryFormat(NovaException):"},{"line_number":1750,"context_line":"    msg_fmt \u003d _(\"Invalid PCI Whitelist: \""},{"line_number":1751,"context_line":"                \"devname, address, or product_id and vendor_id must \""},{"line_number":1752,"context_line":"                \"be supplied.  Entries cannot contain empty fields.\")"},{"line_number":1753,"context_line":""},{"line_number":1754,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a1531f4_1492d457","line":1751,"range":{"start_line":1751,"start_character":17,"end_line":1751,"end_character":62},"updated":"2016-04-15 08:52:48.000000000","message":"From reading this message, I don\u0027t get the logic what\u0027s allowed and what not. It reads like this:\n\n    devname\n    -or-\n    address\n    -or-\n    product_id and vendor_id\n\nThe code and test cases show another logic.","commit_id":"d2ea305f07ac2a76819ead7bf333de29080b9d6f"},{"author":{"_account_id":21420,"name":"Gage Hugo","email":"gagehugo@gmail.com","username":"ghugo"},"change_message_id":"1c1c50b19016ef2002959b0b2e09f6b58453e589","unresolved":false,"context_lines":[{"line_number":1748,"context_line":""},{"line_number":1749,"context_line":"class PciDeviceInvalidEntryFormat(NovaException):"},{"line_number":1750,"context_line":"    msg_fmt \u003d _(\"Invalid PCI Whitelist: \""},{"line_number":1751,"context_line":"                \"devname, address, or product_id and vendor_id must \""},{"line_number":1752,"context_line":"                \"be supplied.  Entries cannot contain empty fields.\")"},{"line_number":1753,"context_line":""},{"line_number":1754,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_bdc12831","line":1751,"range":{"start_line":1751,"start_character":17,"end_line":1751,"end_character":62},"in_reply_to":"3a1531f4_1492d457","updated":"2016-04-15 19:29:21.000000000","message":"At a minimum, one of those choices should be provided along with physical_network to be a valid entry. I do agree that I could\u0027ve worded it better in order to be clearer.","commit_id":"d2ea305f07ac2a76819ead7bf333de29080b9d6f"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"532fd11cfe7df662bb08391beee0ba2ea33d27a4","unresolved":false,"context_lines":[{"line_number":1749,"context_line":"class PciDeviceInvalidEntryFormat(NovaException):"},{"line_number":1750,"context_line":"    msg_fmt \u003d _(\"Invalid PCI Whitelist: \""},{"line_number":1751,"context_line":"                \"devname, address, or product_id and vendor_id must \""},{"line_number":1752,"context_line":"                \"be supplied.  Entries cannot contain empty fields.\")"},{"line_number":1753,"context_line":""},{"line_number":1754,"context_line":""},{"line_number":1755,"context_line":"class PciDeviceWrongAddressFormat(NovaException):"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a1531f4_54ee0ced","line":1752,"range":{"start_line":1752,"start_character":30,"end_line":1752,"end_character":31},"updated":"2016-04-15 08:52:48.000000000","message":"nit: whitespace","commit_id":"d2ea305f07ac2a76819ead7bf333de29080b9d6f"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"9fa8998ae8e65c33c75b99c66e6ea7f98bec528d","unresolved":false,"context_lines":[{"line_number":1748,"context_line":""},{"line_number":1749,"context_line":"class PciDeviceInvalidEntryFormat(NovaException):"},{"line_number":1750,"context_line":"    msg_fmt \u003d _(\"Invalid PCI Whitelist: \""},{"line_number":1751,"context_line":"                \"The PCI whitelist must specify physical_network \""},{"line_number":1752,"context_line":"                \"as well as one of the following: \""},{"line_number":1753,"context_line":"                \"devname or address or product_id AND vendor_id.\""},{"line_number":1754,"context_line":"                \"Entries also cannot contain empty fields.\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_73030f77","line":1751,"updated":"2016-07-15 13:53:45.000000000","message":"physical_network is optional.  There are PCI devices that are not related to networking (like GPU, crypto devices, etc.).","commit_id":"5b65f93ca5f006ebc2e6be65889d9c6f77cdae17"},{"author":{"_account_id":21420,"name":"Gage Hugo","email":"gagehugo@gmail.com","username":"ghugo"},"change_message_id":"f06e7030f3ac6d69a571e28445f9c8f3e80d0644","unresolved":false,"context_lines":[{"line_number":1748,"context_line":""},{"line_number":1749,"context_line":"class PciDeviceInvalidEntryFormat(NovaException):"},{"line_number":1750,"context_line":"    msg_fmt \u003d _(\"Invalid PCI Whitelist: \""},{"line_number":1751,"context_line":"                \"The PCI whitelist must specify physical_network \""},{"line_number":1752,"context_line":"                \"as well as one of the following: \""},{"line_number":1753,"context_line":"                \"devname or address or product_id AND vendor_id.\""},{"line_number":1754,"context_line":"                \"Entries also cannot contain empty fields.\")"}],"source_content_type":"text/x-python","patch_set":3,"id":"fad55976_501744ed","line":1751,"in_reply_to":"1aa78d24_73030f77","updated":"2016-07-15 15:23:03.000000000","message":"ah ok, I\u0027ll change this then","commit_id":"5b65f93ca5f006ebc2e6be65889d9c6f77cdae17"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"9fa8998ae8e65c33c75b99c66e6ea7f98bec528d","unresolved":false,"context_lines":[{"line_number":1751,"context_line":"                \"The PCI whitelist must specify physical_network \""},{"line_number":1752,"context_line":"                \"as well as one of the following: \""},{"line_number":1753,"context_line":"                \"devname or address or product_id AND vendor_id.\""},{"line_number":1754,"context_line":"                \"Entries also cannot contain empty fields.\")"},{"line_number":1755,"context_line":""},{"line_number":1756,"context_line":""},{"line_number":1757,"context_line":"class PciDeviceWrongAddressFormat(NovaException):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_d3392341","line":1754,"updated":"2016-07-15 13:53:45.000000000","message":"It would be useful to report the invalid entry here in the debug string.","commit_id":"5b65f93ca5f006ebc2e6be65889d9c6f77cdae17"},{"author":{"_account_id":21420,"name":"Gage Hugo","email":"gagehugo@gmail.com","username":"ghugo"},"change_message_id":"f06e7030f3ac6d69a571e28445f9c8f3e80d0644","unresolved":false,"context_lines":[{"line_number":1751,"context_line":"                \"The PCI whitelist must specify physical_network \""},{"line_number":1752,"context_line":"                \"as well as one of the following: \""},{"line_number":1753,"context_line":"                \"devname or address or product_id AND vendor_id.\""},{"line_number":1754,"context_line":"                \"Entries also cannot contain empty fields.\")"},{"line_number":1755,"context_line":""},{"line_number":1756,"context_line":""},{"line_number":1757,"context_line":"class PciDeviceWrongAddressFormat(NovaException):"}],"source_content_type":"text/x-python","patch_set":3,"id":"fad55976_f0dc7848","line":1754,"in_reply_to":"1aa78d24_d3392341","updated":"2016-07-15 15:23:03.000000000","message":"fixed","commit_id":"5b65f93ca5f006ebc2e6be65889d9c6f77cdae17"}],"nova/pci/devspec.py":[{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"532fd11cfe7df662bb08391beee0ba2ea33d27a4","unresolved":false,"context_lines":[{"line_number":151,"context_line":"        if self.is_dev_name_empty():"},{"line_number":152,"context_line":"            raise exception.PciDeviceInvalidEntryFormat()"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        if not self.dev_name and not self.address:"},{"line_number":155,"context_line":"            if self.vendor_id \u003d\u003d ANY or self.product_id \u003d\u003d ANY:"},{"line_number":156,"context_line":"                raise exception.PciDeviceInvalidEntryFormat()"},{"line_number":157,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a1531f4_f4f2b805","line":154,"range":{"start_line":154,"start_character":11,"end_line":154,"end_character":28},"updated":"2016-04-15 08:52:48.000000000","message":"Isn\u0027t that already checked with \"is_dev_name_empty()\" from above?","commit_id":"d2ea305f07ac2a76819ead7bf333de29080b9d6f"},{"author":{"_account_id":21420,"name":"Gage Hugo","email":"gagehugo@gmail.com","username":"ghugo"},"change_message_id":"1c1c50b19016ef2002959b0b2e09f6b58453e589","unresolved":false,"context_lines":[{"line_number":151,"context_line":"        if self.is_dev_name_empty():"},{"line_number":152,"context_line":"            raise exception.PciDeviceInvalidEntryFormat()"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        if not self.dev_name and not self.address:"},{"line_number":155,"context_line":"            if self.vendor_id \u003d\u003d ANY or self.product_id \u003d\u003d ANY:"},{"line_number":156,"context_line":"                raise exception.PciDeviceInvalidEntryFormat()"},{"line_number":157,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_fd78c091","line":154,"range":{"start_line":154,"start_character":11,"end_line":154,"end_character":28},"in_reply_to":"3a1531f4_f4f2b805","updated":"2016-04-15 19:29:21.000000000","message":"Yes, but if they only provided devname and physical network, which is a valid whitelist entry, then by not checking if devname is valid here it will assume the entry is invalid since neither address nor vendor and product ids were provided.","commit_id":"d2ea305f07ac2a76819ead7bf333de29080b9d6f"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"532fd11cfe7df662bb08391beee0ba2ea33d27a4","unresolved":false,"context_lines":[{"line_number":184,"context_line":"    def get_tags(self):"},{"line_number":185,"context_line":"        return self.tags"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    def is_dev_name_empty(self):"},{"line_number":188,"context_line":"        if self.dev_name is None:"},{"line_number":189,"context_line":"            return False"},{"line_number":190,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a1531f4_74ad0888","line":187,"updated":"2016-04-15 08:52:48.000000000","message":"Could be a private method \"_id_dev_name_empty()\"","commit_id":"d2ea305f07ac2a76819ead7bf333de29080b9d6f"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"532fd11cfe7df662bb08391beee0ba2ea33d27a4","unresolved":false,"context_lines":[{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    def is_dev_name_empty(self):"},{"line_number":188,"context_line":"        if self.dev_name is None:"},{"line_number":189,"context_line":"            return False"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        return not (self.dev_name and not self.dev_name.isspace())"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a1531f4_54ef2cb0","line":189,"range":{"start_line":189,"start_character":12,"end_line":189,"end_character":24},"updated":"2016-04-15 08:52:48.000000000","message":"Shouldn\u0027t that be \"return True\"? Or is \"None\" considered to be non-empty? I have trouble to understand this here.","commit_id":"d2ea305f07ac2a76819ead7bf333de29080b9d6f"},{"author":{"_account_id":21420,"name":"Gage Hugo","email":"gagehugo@gmail.com","username":"ghugo"},"change_message_id":"1c1c50b19016ef2002959b0b2e09f6b58453e589","unresolved":false,"context_lines":[{"line_number":186,"context_line":""},{"line_number":187,"context_line":"    def is_dev_name_empty(self):"},{"line_number":188,"context_line":"        if self.dev_name is None:"},{"line_number":189,"context_line":"            return False"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        return not (self.dev_name and not self.dev_name.isspace())"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_5d09f42b","line":189,"range":{"start_line":189,"start_character":12,"end_line":189,"end_character":24},"in_reply_to":"3a1531f4_54ef2cb0","updated":"2016-04-15 19:29:21.000000000","message":"I was having trouble with python interpreting \"return self.dev_name\" as false if it was either None or \"\".  This eliminates the possibility of it being None and then we check if they just provided \"\" or \"   \".","commit_id":"d2ea305f07ac2a76819ead7bf333de29080b9d6f"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"532fd11cfe7df662bb08391beee0ba2ea33d27a4","unresolved":false,"context_lines":[{"line_number":188,"context_line":"        if self.dev_name is None:"},{"line_number":189,"context_line":"            return False"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        return not (self.dev_name and not self.dev_name.isspace())"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a1531f4_d4d5fcb9","line":191,"range":{"start_line":191,"start_character":15,"end_line":191,"end_character":66},"updated":"2016-04-15 08:52:48.000000000","message":"Can be simplified by:\n\n    return self.dev_name.strip()","commit_id":"d2ea305f07ac2a76819ead7bf333de29080b9d6f"},{"author":{"_account_id":21420,"name":"Gage Hugo","email":"gagehugo@gmail.com","username":"ghugo"},"change_message_id":"1c1c50b19016ef2002959b0b2e09f6b58453e589","unresolved":false,"context_lines":[{"line_number":188,"context_line":"        if self.dev_name is None:"},{"line_number":189,"context_line":"            return False"},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        return not (self.dev_name and not self.dev_name.isspace())"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_9d138c5c","line":191,"range":{"start_line":191,"start_character":15,"end_line":191,"end_character":66},"in_reply_to":"3a1531f4_d4d5fcb9","updated":"2016-04-15 19:29:21.000000000","message":"Thanks!","commit_id":"d2ea305f07ac2a76819ead7bf333de29080b9d6f"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"9fa8998ae8e65c33c75b99c66e6ea7f98bec528d","unresolved":false,"context_lines":[{"line_number":149,"context_line":"            raise exception.PciDeviceInvalidDeviceName()"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        if self.__is_dev_name_empty():"},{"line_number":152,"context_line":"            raise exception.PciDeviceInvalidEntryFormat()"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        if not self.dev_name and not self.address:"},{"line_number":155,"context_line":"            if self.vendor_id \u003d\u003d ANY or self.product_id \u003d\u003d ANY:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_3306a720","line":152,"updated":"2016-07-15 13:53:45.000000000","message":"Logic is wrong here.  devname can be empty if address is specified or if vendor_id and product_id is specified.","commit_id":"5b65f93ca5f006ebc2e6be65889d9c6f77cdae17"},{"author":{"_account_id":21420,"name":"Gage Hugo","email":"gagehugo@gmail.com","username":"ghugo"},"change_message_id":"f06e7030f3ac6d69a571e28445f9c8f3e80d0644","unresolved":false,"context_lines":[{"line_number":149,"context_line":"            raise exception.PciDeviceInvalidDeviceName()"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"        if self.__is_dev_name_empty():"},{"line_number":152,"context_line":"            raise exception.PciDeviceInvalidEntryFormat()"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"        if not self.dev_name and not self.address:"},{"line_number":155,"context_line":"            if self.vendor_id \u003d\u003d ANY or self.product_id \u003d\u003d ANY:"}],"source_content_type":"text/x-python","patch_set":3,"id":"fad55976_3e3f0127","line":152,"in_reply_to":"1aa78d24_3306a720","updated":"2016-07-15 15:23:03.000000000","message":"will fix","commit_id":"5b65f93ca5f006ebc2e6be65889d9c6f77cdae17"}],"nova/tests/unit/pci/test_devspec.py":[{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"532fd11cfe7df662bb08391beee0ba2ea33d27a4","unresolved":false,"context_lines":[{"line_number":143,"context_line":"        self.assertRaises(exception.PciDeviceInvalidDeviceName,"},{"line_number":144,"context_line":"                          devspec.PciDeviceSpec, pci_info)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"    def test_devname_empty_string(self):"},{"line_number":147,"context_line":"        pci_info \u003d {\"devname\": \"\", \"physical_network\": \"hr_net\"}"},{"line_number":148,"context_line":"        self.assertRaises(exception.PciDeviceInvalidEntryFormat,"},{"line_number":149,"context_line":"                          devspec.PciDeviceSpec, pci_info)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a1531f4_972a667d","line":146,"updated":"2016-04-15 08:52:48.000000000","message":"Maybe rename to \"test_devname_is_empty()\" and use these for \"devname\":\n\n    \"\"     # empty string\n    \"   \"  # whitespace string\n    None   # Nothing set\n\nAll three input values should raise the error IIUC.","commit_id":"d2ea305f07ac2a76819ead7bf333de29080b9d6f"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"532fd11cfe7df662bb08391beee0ba2ea33d27a4","unresolved":false,"context_lines":[{"line_number":149,"context_line":"                          devspec.PciDeviceSpec, pci_info)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def test_only_physical_address(self):"},{"line_number":152,"context_line":"        pci_info \u003d {\"physical_network\": \"hr_net\"}"},{"line_number":153,"context_line":"        self.assertRaises(exception.PciDeviceInvalidEntryFormat,"},{"line_number":154,"context_line":"                          devspec.PciDeviceSpec, pci_info)"},{"line_number":155,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"3a1531f4_f7f2ba05","line":152,"updated":"2016-04-15 08:52:48.000000000","message":"If I get the code right, we need \n\n    physical_address AND vendor_id AND product_id\n\nis that right? Leaving a comment WHY this test should fail would be good. It says \"only_physical_address\" which tells me that it needs *something* but I\u0027d like to know in a testcase what the *something* is.","commit_id":"d2ea305f07ac2a76819ead7bf333de29080b9d6f"},{"author":{"_account_id":11303,"name":"Markus Zoeller (markus_z)","email":"mzoeller@linux.vnet.ibm.com","username":"mzoeller"},"change_message_id":"532fd11cfe7df662bb08391beee0ba2ea33d27a4","unresolved":false,"context_lines":[{"line_number":153,"context_line":"        self.assertRaises(exception.PciDeviceInvalidEntryFormat,"},{"line_number":154,"context_line":"                          devspec.PciDeviceSpec, pci_info)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    def test_only_specified_product_id(self):"},{"line_number":157,"context_line":"        pci_info \u003d {\"product_id\": \"5057\", \"physical_network\": \"hr_net\"}"},{"line_number":158,"context_line":"        self.assertRaises(exception.PciDeviceInvalidEntryFormat,"},{"line_number":159,"context_line":"                          devspec.PciDeviceSpec, pci_info)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a1531f4_b75de2ee","line":156,"range":{"start_line":156,"start_character":8,"end_line":156,"end_character":38},"updated":"2016-04-15 08:52:48.000000000","message":"After reading this I think we can combine the testcase from above to a new one called \"test_physical_address_combinations()\" and handle the four scenarios in it:\n\n* physical address without any other entry \u003d\u003e error\n* physical address with vendor_id \u003d\u003e error \n* physical address with product_id \u003d\u003e error\n* physical address with vendor_id and product_id \u003d\u003e success\n\nThat way it would be properly documented what should fail and what should succeed.","commit_id":"d2ea305f07ac2a76819ead7bf333de29080b9d6f"}]}