)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"18ffbf9fefc3ec6cf9da96a45c28a5bd22a16a28","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Nikola Dipanov \u003cndipanov@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-12-31 09:12:47 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"neturon: Update the port with a MAC address for PFs"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In case of an SR-IOV PF as a neutron port, we should really update"},{"line_number":10,"context_line":"Neutron with the proper MAC, as it is known only once the instance has"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"fa69d971_a94de773","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":7},"updated":"2015-12-31 10:33:20.000000000","message":"neturon \u003d\u003e neutron","commit_id":"8d603af6746870dd25582b39e6ac315cb967e6c8"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"90ab493ee2d114780d3af7ae81e968a277377257","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Nikola Dipanov \u003cndipanov@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-12-31 09:12:47 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"neturon: Update the port with a MAC address for PFs"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In case of an SR-IOV PF as a neutron port, we should really update"},{"line_number":10,"context_line":"Neutron with the proper MAC, as it is known only once the instance has"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"fa69d971_09abd375","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":7},"in_reply_to":"fa69d971_a94de773","updated":"2015-12-31 10:38:57.000000000","message":"Done","commit_id":"8d603af6746870dd25582b39e6ac315cb967e6c8"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0de30cebdd6cbdc33af95f2b05592e6d014f6ade","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Nikola Dipanov \u003cndipanov@redhat.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2015-12-31 11:03:17 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"neturon: Update the port with a MAC address for PFs"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"In case of an SR-IOV PF as a neutron port, we should really update"},{"line_number":10,"context_line":"Neutron with the proper MAC, as it is known only once the instance has"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"fa69d971_633b72d2","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":7},"updated":"2016-01-03 08:08:29.000000000","message":"Neutron?","commit_id":"9075185e6de749117d6808bdb8032cbef30f1bf6"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"13c0df8d7d5572335d0af64bcc76c9a796e5171a","unresolved":false,"context_lines":[{"line_number":15,"context_line":"port updates that happen during network allocation."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I7eafcaa9147bb9cbe7c7c8cda34ec259be0b0eb5"},{"line_number":18,"context_line":"Implements: sriov-pf-passthrough-neutron-port"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":21,"id":"7a5de9d1_c94ca372","line":18,"updated":"2016-01-29 05:19:51.000000000","message":"This line should be \"Implements: blueprint sriov-pf-passthrough-neutron-port\".","commit_id":"13717ecea9a5b337b540d1033b25a1860fc8683d"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"d23908827d928f5f7eca2ef70c5809f2c3ff3e5d","unresolved":false,"context_lines":[{"line_number":15,"context_line":"port updates that happen during network allocation."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I7eafcaa9147bb9cbe7c7c8cda34ec259be0b0eb5"},{"line_number":18,"context_line":"Implements: sriov-pf-passthrough-neutron-port"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":21,"id":"5a5ae5dd_011e590c","line":18,"in_reply_to":"7a5de9d1_c94ca372","updated":"2016-02-03 17:04:35.000000000","message":"Yes!","commit_id":"13717ecea9a5b337b540d1033b25a1860fc8683d"}],"nova/network/neutronv2/api.py":[{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"18ffbf9fefc3ec6cf9da96a45c28a5bd22a16a28","unresolved":false,"context_lines":[{"line_number":627,"context_line":"                                                        request.pci_request_id,"},{"line_number":628,"context_line":"                                                        port_req_body,"},{"line_number":629,"context_line":"                                                        neutron\u003dneutron)"},{"line_number":630,"context_line":"                self._populate_mac_address(instance, request.pci_request_id,"},{"line_number":631,"context_line":"                                           port_req_body)"},{"line_number":632,"context_line":"                if request.port_id:"},{"line_number":633,"context_line":"                    port \u003d ports[request.port_id]"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa69d971_e9b99f2f","line":630,"updated":"2015-12-31 10:33:20.000000000","message":"what if the port is already created?","commit_id":"8d603af6746870dd25582b39e6ac315cb967e6c8"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"90ab493ee2d114780d3af7ae81e968a277377257","unresolved":false,"context_lines":[{"line_number":627,"context_line":"                                                        request.pci_request_id,"},{"line_number":628,"context_line":"                                                        port_req_body,"},{"line_number":629,"context_line":"                                                        neutron\u003dneutron)"},{"line_number":630,"context_line":"                self._populate_mac_address(instance, request.pci_request_id,"},{"line_number":631,"context_line":"                                           port_req_body)"},{"line_number":632,"context_line":"                if request.port_id:"},{"line_number":633,"context_line":"                    port \u003d ports[request.port_id]"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa69d971_69828f02","line":630,"in_reply_to":"fa69d971_e9b99f2f","updated":"2015-12-31 10:38:57.000000000","message":"That\u0027s the only realistic case (you can\u0027t really request an SR-IOV port from Nova cli. It will just get updated...","commit_id":"8d603af6746870dd25582b39e6ac315cb967e6c8"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"90ab493ee2d114780d3af7ae81e968a277377257","unresolved":false,"context_lines":[{"line_number":631,"context_line":"                                           port_req_body)"},{"line_number":632,"context_line":"                if request.port_id:"},{"line_number":633,"context_line":"                    port \u003d ports[request.port_id]"},{"line_number":634,"context_line":"                    port_client.update_port(port[\u0027id\u0027], port_req_body)"},{"line_number":635,"context_line":"                    preexisting_port_ids.append(port[\u0027id\u0027])"},{"line_number":636,"context_line":"                    ports_in_requested_order.append(port[\u0027id\u0027])"},{"line_number":637,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa69d971_8987c30e","line":634,"range":{"start_line":634,"start_character":32,"end_line":634,"end_character":43},"updated":"2015-12-31 10:38:57.000000000","message":"Here","commit_id":"8d603af6746870dd25582b39e6ac315cb967e6c8"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"18ffbf9fefc3ec6cf9da96a45c28a5bd22a16a28","unresolved":false,"context_lines":[{"line_number":711,"context_line":"            if pci_dev.dev_type \u003d\u003d obj_fields.PciDeviceType.SRIOV_PF:"},{"line_number":712,"context_line":"                try:"},{"line_number":713,"context_line":"                    mac \u003d pci_utils.get_mac_by_pci_address(pci_dev.address)"},{"line_number":714,"context_line":"                except exception.PciDeviceNotFoundById:"},{"line_number":715,"context_line":"                    LOG.warn("},{"line_number":716,"context_line":"                        _LW(\"Could not determine MAC address for %(addr)s\"),"},{"line_number":717,"context_line":"                        addr\u003dpci_dev.address)"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa69d971_294c9741","line":714,"updated":"2015-12-31 10:33:20.000000000","message":"this flow is not tested","commit_id":"8d603af6746870dd25582b39e6ac315cb967e6c8"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"90ab493ee2d114780d3af7ae81e968a277377257","unresolved":false,"context_lines":[{"line_number":711,"context_line":"            if pci_dev.dev_type \u003d\u003d obj_fields.PciDeviceType.SRIOV_PF:"},{"line_number":712,"context_line":"                try:"},{"line_number":713,"context_line":"                    mac \u003d pci_utils.get_mac_by_pci_address(pci_dev.address)"},{"line_number":714,"context_line":"                except exception.PciDeviceNotFoundById:"},{"line_number":715,"context_line":"                    LOG.warn("},{"line_number":716,"context_line":"                        _LW(\"Could not determine MAC address for %(addr)s\"),"},{"line_number":717,"context_line":"                        addr\u003dpci_dev.address)"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa69d971_a98f2722","line":714,"in_reply_to":"fa69d971_294c9741","updated":"2015-12-31 10:38:57.000000000","message":"fair enough","commit_id":"8d603af6746870dd25582b39e6ac315cb967e6c8"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a12f2cf4d9be578f28b3db0b763100c73495f938","unresolved":false,"context_lines":[{"line_number":613,"context_line":"        created_port_ids \u003d []"},{"line_number":614,"context_line":"        ports_in_requested_order \u003d []"},{"line_number":615,"context_line":"        nets_in_requested_order \u003d []"},{"line_number":616,"context_line":"        for request in ordered_networks:"},{"line_number":617,"context_line":"            # Network lookup for available network_id"},{"line_number":618,"context_line":"            network \u003d None"},{"line_number":619,"context_line":"            for net in nets:"}],"source_content_type":"text/x-python","patch_set":27,"id":"ba0121b8_0ae600b4","line":616,"updated":"2016-03-29 12:09:58.000000000","message":"will each network have its pci_request_id? where is this validated that they are different? If they are the same then you can and may have the same MAC address configured on the port (that is ok if it is different networks, but if it is on the same then boom!","commit_id":"43dfdf6636af283085ff41a031732a6e6942526f"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"bf0b45120b8f11a2d262d91e9052a73ff3582f4a","unresolved":false,"context_lines":[{"line_number":613,"context_line":"        created_port_ids \u003d []"},{"line_number":614,"context_line":"        ports_in_requested_order \u003d []"},{"line_number":615,"context_line":"        nets_in_requested_order \u003d []"},{"line_number":616,"context_line":"        for request in ordered_networks:"},{"line_number":617,"context_line":"            # Network lookup for available network_id"},{"line_number":618,"context_line":"            network \u003d None"},{"line_number":619,"context_line":"            for net in nets:"}],"source_content_type":"text/x-python","patch_set":27,"id":"ba0121b8_41fd6f39","line":616,"in_reply_to":"ba0121b8_0ae600b4","updated":"2016-03-30 09:15:33.000000000","message":"No, each network will not have pci_request_id, only the ones that come with SR-IOV ports\n\nActual PCI devices are selected during claims, and in case of a PF device, we pass the actual MAC of the device to Neutron (this is what this patch does) so unless there is a bug in the device claiming code (see pci/manager.py) there is no risk of duplicates.","commit_id":"43dfdf6636af283085ff41a031732a6e6942526f"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a12f2cf4d9be578f28b3db0b763100c73495f938","unresolved":false,"context_lines":[{"line_number":733,"context_line":"                try:"},{"line_number":734,"context_line":"                    mac \u003d pci_utils.get_mac_by_pci_address(pci_dev.address)"},{"line_number":735,"context_line":"                except exception.PciDeviceNotFoundById:"},{"line_number":736,"context_line":"                    LOG.warn("},{"line_number":737,"context_line":"                        _LW(\"Could not determine MAC address for %(addr)s\"),"},{"line_number":738,"context_line":"                        addr\u003dpci_dev.address)"},{"line_number":739,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":27,"id":"ba0121b8_aae4340d","line":736,"updated":"2016-03-29 12:09:58.000000000","message":"nit LOG.warn is deprecated in python 3","commit_id":"43dfdf6636af283085ff41a031732a6e6942526f"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"bf0b45120b8f11a2d262d91e9052a73ff3582f4a","unresolved":false,"context_lines":[{"line_number":733,"context_line":"                try:"},{"line_number":734,"context_line":"                    mac \u003d pci_utils.get_mac_by_pci_address(pci_dev.address)"},{"line_number":735,"context_line":"                except exception.PciDeviceNotFoundById:"},{"line_number":736,"context_line":"                    LOG.warn("},{"line_number":737,"context_line":"                        _LW(\"Could not determine MAC address for %(addr)s\"),"},{"line_number":738,"context_line":"                        addr\u003dpci_dev.address)"},{"line_number":739,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":27,"id":"ba0121b8_c19fbf01","line":736,"in_reply_to":"ba0121b8_aae4340d","updated":"2016-03-30 09:15:33.000000000","message":"Done","commit_id":"43dfdf6636af283085ff41a031732a6e6942526f"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a12f2cf4d9be578f28b3db0b763100c73495f938","unresolved":false,"context_lines":[{"line_number":734,"context_line":"                    mac \u003d pci_utils.get_mac_by_pci_address(pci_dev.address)"},{"line_number":735,"context_line":"                except exception.PciDeviceNotFoundById:"},{"line_number":736,"context_line":"                    LOG.warn("},{"line_number":737,"context_line":"                        _LW(\"Could not determine MAC address for %(addr)s\"),"},{"line_number":738,"context_line":"                        addr\u003dpci_dev.address)"},{"line_number":739,"context_line":"                else:"},{"line_number":740,"context_line":"                    port_req_body[\u0027port\u0027][\u0027mac_address\u0027] \u003d mac"}],"source_content_type":"text/x-python","patch_set":27,"id":"ba0121b8_4a4ab884","line":737,"updated":"2016-03-29 12:09:58.000000000","message":"I suggest also havingt he network details in this log message","commit_id":"43dfdf6636af283085ff41a031732a6e6942526f"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"5f8de45813de6fe2ad8277befda5891b7076d2de","unresolved":false,"context_lines":[{"line_number":734,"context_line":"                    mac \u003d pci_utils.get_mac_by_pci_address(pci_dev.address)"},{"line_number":735,"context_line":"                except exception.PciDeviceNotFoundById:"},{"line_number":736,"context_line":"                    LOG.warn("},{"line_number":737,"context_line":"                        _LW(\"Could not determine MAC address for %(addr)s\"),"},{"line_number":738,"context_line":"                        addr\u003dpci_dev.address)"},{"line_number":739,"context_line":"                else:"},{"line_number":740,"context_line":"                    port_req_body[\u0027port\u0027][\u0027mac_address\u0027] \u003d mac"}],"source_content_type":"text/x-python","patch_set":27,"id":"ba0121b8_d672b8ac","line":737,"in_reply_to":"ba0121b8_018a57bf","updated":"2016-03-30 10:37:36.000000000","message":"Actually I am not sure how useful the netwrok here is - if there is an exception here - it\u0027s purely related to the way PCI devices were configured (whitelisted) in Nova. It is something an adminwould have to fix (it\u0027s a config option). Network is really irrelevant if this blows up.\n\nActually thinking about this more - it should probably be an error","commit_id":"43dfdf6636af283085ff41a031732a6e6942526f"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"bf0b45120b8f11a2d262d91e9052a73ff3582f4a","unresolved":false,"context_lines":[{"line_number":734,"context_line":"                    mac \u003d pci_utils.get_mac_by_pci_address(pci_dev.address)"},{"line_number":735,"context_line":"                except exception.PciDeviceNotFoundById:"},{"line_number":736,"context_line":"                    LOG.warn("},{"line_number":737,"context_line":"                        _LW(\"Could not determine MAC address for %(addr)s\"),"},{"line_number":738,"context_line":"                        addr\u003dpci_dev.address)"},{"line_number":739,"context_line":"                else:"},{"line_number":740,"context_line":"                    port_req_body[\u0027port\u0027][\u0027mac_address\u0027] \u003d mac"}],"source_content_type":"text/x-python","patch_set":27,"id":"ba0121b8_018a57bf","line":737,"in_reply_to":"ba0121b8_4a4ab884","updated":"2016-03-30 09:15:33.000000000","message":"ok","commit_id":"43dfdf6636af283085ff41a031732a6e6942526f"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"a12f2cf4d9be578f28b3db0b763100c73495f938","unresolved":false,"context_lines":[{"line_number":735,"context_line":"                except exception.PciDeviceNotFoundById:"},{"line_number":736,"context_line":"                    LOG.warn("},{"line_number":737,"context_line":"                        _LW(\"Could not determine MAC address for %(addr)s\"),"},{"line_number":738,"context_line":"                        addr\u003dpci_dev.address)"},{"line_number":739,"context_line":"                else:"},{"line_number":740,"context_line":"                    port_req_body[\u0027port\u0027][\u0027mac_address\u0027] \u003d mac"},{"line_number":741,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"ba0121b8_8a6df006","line":738,"updated":"2016-03-29 12:09:58.000000000","message":"should we not be raising an exception here? will this instance be able to get traffic?","commit_id":"43dfdf6636af283085ff41a031732a6e6942526f"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"5f8de45813de6fe2ad8277befda5891b7076d2de","unresolved":false,"context_lines":[{"line_number":735,"context_line":"                except exception.PciDeviceNotFoundById:"},{"line_number":736,"context_line":"                    LOG.warn("},{"line_number":737,"context_line":"                        _LW(\"Could not determine MAC address for %(addr)s\"),"},{"line_number":738,"context_line":"                        addr\u003dpci_dev.address)"},{"line_number":739,"context_line":"                else:"},{"line_number":740,"context_line":"                    port_req_body[\u0027port\u0027][\u0027mac_address\u0027] \u003d mac"},{"line_number":741,"context_line":""}],"source_content_type":"text/x-python","patch_set":27,"id":"ba0121b8_d6ebd8ae","line":738,"in_reply_to":"ba0121b8_8a6df006","updated":"2016-03-30 10:37:36.000000000","message":"That is a good point. This basically 99% of the time indicates a mis-configuration, so we really should be handling this elsewhere. OTOH - if there is an issue with reading the mac address - it is not clear that we need to fail to boot just because of that. Looking at Neutron code, SR-IOV agend will not use the mac address currently for a PF port at all, so AFAICT - it\u0027s mainly to keep the Neutron DB clean. So for the purposes of this patch it seems fine although we may want to change it later.","commit_id":"43dfdf6636af283085ff41a031732a6e6942526f"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"3fa20c3ad80a7d9d683971739cf3e513d90f5f6b","unresolved":false,"context_lines":[{"line_number":709,"context_line":"        Populate it with SR-IOV related information"},{"line_number":710,"context_line":"        \"\"\""},{"line_number":711,"context_line":"        if pci_request_id:"},{"line_number":712,"context_line":"            pci_dev \u003d pci_manager.get_instance_pci_devs("},{"line_number":713,"context_line":"                instance, pci_request_id).pop()"},{"line_number":714,"context_line":"            devspec \u003d pci_whitelist.get_pci_device_devspec(pci_dev)"},{"line_number":715,"context_line":"            profile \u003d {\u0027pci_vendor_info\u0027: \"%s:%s\" % (pci_dev.vendor_id,"},{"line_number":716,"context_line":"                                                     pci_dev.product_id),"}],"source_content_type":"text/x-python","patch_set":29,"id":"ba0121b8_24745b66","line":713,"range":{"start_line":712,"start_character":12,"end_line":713,"end_character":47},"updated":"2016-03-31 10:54:37.000000000","message":"For example this code does the same thing for the very same reasons.","commit_id":"f1b4206b26bd1f3a2c3864a6449cc164c684f3b2"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"b63da264fe8e53714d99879362430a20a0ae223a","unresolved":false,"context_lines":[{"line_number":727,"context_line":"        Currently this is done only for PF passthrough."},{"line_number":728,"context_line":"        \"\"\""},{"line_number":729,"context_line":"        if pci_request_id is not None:"},{"line_number":730,"context_line":"            pci_dev \u003d pci_manager.get_instance_pci_devs("},{"line_number":731,"context_line":"                instance, pci_request_id).pop()"},{"line_number":732,"context_line":"            if pci_dev.dev_type \u003d\u003d obj_fields.PciDeviceType.SRIOV_PF:"},{"line_number":733,"context_line":"                try:"},{"line_number":734,"context_line":"                    mac \u003d pci_utils.get_mac_by_pci_address(pci_dev.address)"}],"source_content_type":"text/x-python","patch_set":29,"id":"ba0121b8_fd722d48","line":731,"range":{"start_line":730,"start_character":0,"end_line":731,"end_character":47},"updated":"2016-03-30 12:44:49.000000000","message":"you are assuming that you will have one pci device per pci request don\u0027t you want to validate that this is indeed the\ncase","commit_id":"f1b4206b26bd1f3a2c3864a6449cc164c684f3b2"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"60c40b521dd6119fcd11c5fe50996ae6d76e76e4","unresolved":false,"context_lines":[{"line_number":727,"context_line":"        Currently this is done only for PF passthrough."},{"line_number":728,"context_line":"        \"\"\""},{"line_number":729,"context_line":"        if pci_request_id is not None:"},{"line_number":730,"context_line":"            pci_dev \u003d pci_manager.get_instance_pci_devs("},{"line_number":731,"context_line":"                instance, pci_request_id).pop()"},{"line_number":732,"context_line":"            if pci_dev.dev_type \u003d\u003d obj_fields.PciDeviceType.SRIOV_PF:"},{"line_number":733,"context_line":"                try:"},{"line_number":734,"context_line":"                    mac \u003d pci_utils.get_mac_by_pci_address(pci_dev.address)"}],"source_content_type":"text/x-python","patch_set":29,"id":"ba0121b8_2e79dcc8","line":731,"range":{"start_line":730,"start_character":0,"end_line":731,"end_character":47},"in_reply_to":"ba0121b8_3ef74d8c","updated":"2016-03-31 10:14:03.000000000","message":"Ok so looking at this more - here\u0027s the thing: We really should never have more than one pci device per request when we get here, because we are dealing only Pci requests that are generated as part of network requests and this is hard-coded to be 1.\n\nhttps://github.com/openstack/nova/blob/410a87ae299fc304c40f6065ceb16d0051ff3c62/nova/network/neutronv2/api.py#L1077\n\nNo devices (an empty list) means the claim failed so we should never really get here. I guess we could codify those things somehow.","commit_id":"f1b4206b26bd1f3a2c3864a6449cc164c684f3b2"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"dfb5138ac47f7c4480535f33f1da116eeb516d5a","unresolved":false,"context_lines":[{"line_number":727,"context_line":"        Currently this is done only for PF passthrough."},{"line_number":728,"context_line":"        \"\"\""},{"line_number":729,"context_line":"        if pci_request_id is not None:"},{"line_number":730,"context_line":"            pci_dev \u003d pci_manager.get_instance_pci_devs("},{"line_number":731,"context_line":"                instance, pci_request_id).pop()"},{"line_number":732,"context_line":"            if pci_dev.dev_type \u003d\u003d obj_fields.PciDeviceType.SRIOV_PF:"},{"line_number":733,"context_line":"                try:"},{"line_number":734,"context_line":"                    mac \u003d pci_utils.get_mac_by_pci_address(pci_dev.address)"}],"source_content_type":"text/x-python","patch_set":29,"id":"ba0121b8_3ef74d8c","line":731,"range":{"start_line":730,"start_character":0,"end_line":731,"end_character":47},"in_reply_to":"ba0121b8_c353f569","updated":"2016-03-31 09:09:56.000000000","message":"All good points - thanks for the review guys!\n\nWill make this bit more robust.","commit_id":"f1b4206b26bd1f3a2c3864a6449cc164c684f3b2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b8c23d603e5edea128b70e5da9eef1b798b8d6e1","unresolved":false,"context_lines":[{"line_number":727,"context_line":"        Currently this is done only for PF passthrough."},{"line_number":728,"context_line":"        \"\"\""},{"line_number":729,"context_line":"        if pci_request_id is not None:"},{"line_number":730,"context_line":"            pci_dev \u003d pci_manager.get_instance_pci_devs("},{"line_number":731,"context_line":"                instance, pci_request_id).pop()"},{"line_number":732,"context_line":"            if pci_dev.dev_type \u003d\u003d obj_fields.PciDeviceType.SRIOV_PF:"},{"line_number":733,"context_line":"                try:"},{"line_number":734,"context_line":"                    mac \u003d pci_utils.get_mac_by_pci_address(pci_dev.address)"}],"source_content_type":"text/x-python","patch_set":29,"id":"ba0121b8_c353f569","line":731,"range":{"start_line":730,"start_character":0,"end_line":731,"end_character":47},"in_reply_to":"ba0121b8_fd722d48","updated":"2016-03-30 18:51:42.000000000","message":"Agreed, we could have another pci device that isn\u0027t the nic and we\u0027d get the wrong thing here right?\n\nAlso, are we guaranteed to have something in there just because we have a pci_request_id? If not, pop()ing the empty list will explode.","commit_id":"f1b4206b26bd1f3a2c3864a6449cc164c684f3b2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5dd474e8e1390f8988663b39f11dc16e21f066b0","unresolved":false,"context_lines":[{"line_number":647,"context_line":"                    bind_host_id\u003dbind_host_id)"},{"line_number":648,"context_line":"                self._populate_mac_address(instance, request.pci_request_id,"},{"line_number":649,"context_line":"                                           port_req_body)"},{"line_number":650,"context_line":"                if request.port_id:"},{"line_number":651,"context_line":"                    port \u003d ports[request.port_id]"},{"line_number":652,"context_line":"                    port_client.update_port(port[\u0027id\u0027], port_req_body)"},{"line_number":653,"context_line":"                    preexisting_port_ids.append(port[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":30,"id":"ba0121b8_91ec75f4","line":650,"range":{"start_line":650,"start_character":16,"end_line":650,"end_character":35},"updated":"2016-03-31 15:31:14.000000000","message":"The only way this works is if the sriov port is supplied when creating the server, right? If so, we should move the _populate_mac_address method in this block where we know a port ID was requested (and nova isn\u0027t allocating one automatically).","commit_id":"6eda146fcc31bab46c7bd2282dd4bbac125166d2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b48134610f4aa56507bbb7f113e63ff8f6b55bdd","unresolved":false,"context_lines":[{"line_number":647,"context_line":"                    bind_host_id\u003dbind_host_id)"},{"line_number":648,"context_line":"                self._populate_mac_address(instance, request.pci_request_id,"},{"line_number":649,"context_line":"                                           port_req_body)"},{"line_number":650,"context_line":"                if request.port_id:"},{"line_number":651,"context_line":"                    port \u003d ports[request.port_id]"},{"line_number":652,"context_line":"                    port_client.update_port(port[\u0027id\u0027], port_req_body)"},{"line_number":653,"context_line":"                    preexisting_port_ids.append(port[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":30,"id":"ba0121b8_9c7593cc","line":650,"range":{"start_line":650,"start_character":16,"end_line":650,"end_character":35},"in_reply_to":"ba0121b8_507ad488","updated":"2016-04-01 00:54:01.000000000","message":"Yeah, I know it\u0027s a no-op, I just prefer to limit the number of code paths we\u0027re hitting if we know we aren\u0027t going to need it. I\u0027m fine with leaving this in too, I wasn\u0027t -1 for this.","commit_id":"6eda146fcc31bab46c7bd2282dd4bbac125166d2"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"09daeb019b421935e54cc318c93ab518d9147985","unresolved":false,"context_lines":[{"line_number":647,"context_line":"                    bind_host_id\u003dbind_host_id)"},{"line_number":648,"context_line":"                self._populate_mac_address(instance, request.pci_request_id,"},{"line_number":649,"context_line":"                                           port_req_body)"},{"line_number":650,"context_line":"                if request.port_id:"},{"line_number":651,"context_line":"                    port \u003d ports[request.port_id]"},{"line_number":652,"context_line":"                    port_client.update_port(port[\u0027id\u0027], port_req_body)"},{"line_number":653,"context_line":"                    preexisting_port_ids.append(port[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":30,"id":"ba0121b8_507ad488","line":650,"range":{"start_line":650,"start_character":16,"end_line":650,"end_character":35},"in_reply_to":"ba0121b8_91ec75f4","updated":"2016-03-31 16:16:33.000000000","message":"It isn\u0027t going to make a difference since there is no other way that we would have a pci_request_id set on the network_request, and without it _populate_mac_address is a no-op. Also this is consistent with the above _populate_neutron_extension_values which updates bindings for SR-IOV port using the same check for pci_request_id","commit_id":"6eda146fcc31bab46c7bd2282dd4bbac125166d2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5dd474e8e1390f8988663b39f11dc16e21f066b0","unresolved":false,"context_lines":[{"line_number":722,"context_line":""},{"line_number":723,"context_line":"    @staticmethod"},{"line_number":724,"context_line":"    def _populate_mac_address(instance, pci_request_id, port_req_body):"},{"line_number":725,"context_line":"        \"\"\"Update the mac address if needed."},{"line_number":726,"context_line":""},{"line_number":727,"context_line":"        Currently this is done only for PF passthrough."},{"line_number":728,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":30,"id":"ba0121b8_51dadde8","line":725,"range":{"start_line":725,"start_character":11,"end_line":725,"end_character":44},"updated":"2016-03-31 15:31:14.000000000","message":"When I first read this I thought it was going to actually call out to neutron to update the port, but really this is setting the mac_address in the port_req_body. This is a bit-nicky but maybe you want to add something in the method description that this doesn\u0027t actually update the port in neutron, it just updates the port_req_body.","commit_id":"6eda146fcc31bab46c7bd2282dd4bbac125166d2"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"09daeb019b421935e54cc318c93ab518d9147985","unresolved":false,"context_lines":[{"line_number":722,"context_line":""},{"line_number":723,"context_line":"    @staticmethod"},{"line_number":724,"context_line":"    def _populate_mac_address(instance, pci_request_id, port_req_body):"},{"line_number":725,"context_line":"        \"\"\"Update the mac address if needed."},{"line_number":726,"context_line":""},{"line_number":727,"context_line":"        Currently this is done only for PF passthrough."},{"line_number":728,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":30,"id":"ba0121b8_2258b989","line":725,"range":{"start_line":725,"start_character":11,"end_line":725,"end_character":44},"in_reply_to":"ba0121b8_51dadde8","updated":"2016-03-31 16:16:33.000000000","message":"Will clarify - thanks","commit_id":"6eda146fcc31bab46c7bd2282dd4bbac125166d2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5dd474e8e1390f8988663b39f11dc16e21f066b0","unresolved":false,"context_lines":[{"line_number":735,"context_line":"                # only ever index a single device, which needs to be"},{"line_number":736,"context_line":"                # successfully claimed for this to be called as part of"},{"line_number":737,"context_line":"                # allocate_networks method"},{"line_number":738,"context_line":"                LOG.error(_LE(\"PCI request %s does not have a \""},{"line_number":739,"context_line":"                              \"unique device associated with it. Unable to \""},{"line_number":740,"context_line":"                              \"determine MAC address\"), pci_request)"},{"line_number":741,"context_line":"                return"},{"line_number":742,"context_line":"            pci_dev \u003d pci_devs[0]"},{"line_number":743,"context_line":"            if pci_dev.dev_type \u003d\u003d obj_fields.PciDeviceType.SRIOV_PF:"}],"source_content_type":"text/x-python","patch_set":30,"id":"ba0121b8_51c93d58","line":740,"range":{"start_line":738,"start_character":16,"end_line":740,"end_character":68},"updated":"2016-03-31 15:31:14.000000000","message":"nit: throw the instance\u003dinstance kwarg in the LOG.error method call for context.","commit_id":"6eda146fcc31bab46c7bd2282dd4bbac125166d2"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"09daeb019b421935e54cc318c93ab518d9147985","unresolved":false,"context_lines":[{"line_number":735,"context_line":"                # only ever index a single device, which needs to be"},{"line_number":736,"context_line":"                # successfully claimed for this to be called as part of"},{"line_number":737,"context_line":"                # allocate_networks method"},{"line_number":738,"context_line":"                LOG.error(_LE(\"PCI request %s does not have a \""},{"line_number":739,"context_line":"                              \"unique device associated with it. Unable to \""},{"line_number":740,"context_line":"                              \"determine MAC address\"), pci_request)"},{"line_number":741,"context_line":"                return"},{"line_number":742,"context_line":"            pci_dev \u003d pci_devs[0]"},{"line_number":743,"context_line":"            if pci_dev.dev_type \u003d\u003d obj_fields.PciDeviceType.SRIOV_PF:"}],"source_content_type":"text/x-python","patch_set":30,"id":"ba0121b8_423e8d28","line":740,"range":{"start_line":738,"start_character":16,"end_line":740,"end_character":68},"in_reply_to":"ba0121b8_51c93d58","updated":"2016-03-31 16:16:33.000000000","message":"Done","commit_id":"6eda146fcc31bab46c7bd2282dd4bbac125166d2"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5dd474e8e1390f8988663b39f11dc16e21f066b0","unresolved":false,"context_lines":[{"line_number":745,"context_line":"                    mac \u003d pci_utils.get_mac_by_pci_address(pci_dev.address)"},{"line_number":746,"context_line":"                except exception.PciDeviceNotFoundById:"},{"line_number":747,"context_line":"                    LOG.error("},{"line_number":748,"context_line":"                        _LE(\"Could not determine MAC address for %(addr)s\"),"},{"line_number":749,"context_line":"                        {\"addr\": pci_dev.address})"},{"line_number":750,"context_line":"                else:"},{"line_number":751,"context_line":"                    port_req_body[\u0027port\u0027][\u0027mac_address\u0027] \u003d mac"}],"source_content_type":"text/x-python","patch_set":30,"id":"ba0121b8_f12cd103","line":748,"range":{"start_line":748,"start_character":29,"end_line":748,"end_character":73},"updated":"2016-03-31 15:31:14.000000000","message":"We should probably include the PciDeviceNotFoundById error in here so the admin reading the logs has an idea of why it\u0027s not found (since this is an unexpected case).\n\nAlso, throw instance\u003dinstance kwarg in the LOG.error call for context.","commit_id":"6eda146fcc31bab46c7bd2282dd4bbac125166d2"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"09daeb019b421935e54cc318c93ab518d9147985","unresolved":false,"context_lines":[{"line_number":745,"context_line":"                    mac \u003d pci_utils.get_mac_by_pci_address(pci_dev.address)"},{"line_number":746,"context_line":"                except exception.PciDeviceNotFoundById:"},{"line_number":747,"context_line":"                    LOG.error("},{"line_number":748,"context_line":"                        _LE(\"Could not determine MAC address for %(addr)s\"),"},{"line_number":749,"context_line":"                        {\"addr\": pci_dev.address})"},{"line_number":750,"context_line":"                else:"},{"line_number":751,"context_line":"                    port_req_body[\u0027port\u0027][\u0027mac_address\u0027] \u003d mac"}],"source_content_type":"text/x-python","patch_set":30,"id":"ba0121b8_62047149","line":748,"range":{"start_line":748,"start_character":29,"end_line":748,"end_character":73},"in_reply_to":"ba0121b8_f12cd103","updated":"2016-03-31 16:16:33.000000000","message":"Done","commit_id":"6eda146fcc31bab46c7bd2282dd4bbac125166d2"}],"nova/tests/unit/network/test_neutronv2.py":[{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"13c0df8d7d5572335d0af64bcc76c9a796e5171a","unresolved":false,"context_lines":[{"line_number":3809,"context_line":""},{"line_number":3810,"context_line":"    @mock.patch.object(pci_manager, \u0027get_instance_pci_devs\u0027)"},{"line_number":3811,"context_line":"    @mock.patch.object(pci_utils, \u0027get_mac_by_pci_address\u0027)"},{"line_number":3812,"context_line":"    def test_populate_mac_address(self, mock_get_mac_by_pci_address,"},{"line_number":3813,"context_line":"                                  mock_get_instance_pci_devs):"},{"line_number":3814,"context_line":"        api \u003d neutronapi.API()"},{"line_number":3815,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"7a5de9d1_e9195f7a","line":3812,"range":{"start_line":3812,"start_character":8,"end_line":3812,"end_character":33},"updated":"2016-01-29 05:19:51.000000000","message":"This test method has 3 test cases.\nSo it would be better to split it to 3 test methods.\n\nAnd add the case for that \u0027pci_request_id\u0027 is 0 or None.","commit_id":"13717ecea9a5b337b540d1033b25a1860fc8683d"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"d23908827d928f5f7eca2ef70c5809f2c3ff3e5d","unresolved":false,"context_lines":[{"line_number":3809,"context_line":""},{"line_number":3810,"context_line":"    @mock.patch.object(pci_manager, \u0027get_instance_pci_devs\u0027)"},{"line_number":3811,"context_line":"    @mock.patch.object(pci_utils, \u0027get_mac_by_pci_address\u0027)"},{"line_number":3812,"context_line":"    def test_populate_mac_address(self, mock_get_mac_by_pci_address,"},{"line_number":3813,"context_line":"                                  mock_get_instance_pci_devs):"},{"line_number":3814,"context_line":"        api \u003d neutronapi.API()"},{"line_number":3815,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"5a5ae5dd_61a14da9","line":3812,"range":{"start_line":3812,"start_character":8,"end_line":3812,"end_character":33},"in_reply_to":"7a5de9d1_e9195f7a","updated":"2016-02-03 17:04:35.000000000","message":"Done","commit_id":"13717ecea9a5b337b540d1033b25a1860fc8683d"}]}