)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2753cc2050ddabcebf7fd6cac89595e1606c5c38","unresolved":false,"context_lines":[{"line_number":20,"context_line":"consistent way."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I71cf7526286e2b1140804e7aeac93c11dd8b7583"},{"line_number":23,"context_line":"Implements: blueprint pci-generate-stats"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"7a740942_f2409509","line":23,"updated":"2015-12-10 14:53:08.000000000","message":"This should be pci-stats-generate","commit_id":"6196e3af868b1cba95acb20b385b60d57987258e"},{"author":{"_account_id":6508,"name":"Dmitry Stepanenko","email":"dstepanenko@mirantis.com","username":"dstepanenko"},"change_message_id":"6445029b87ea7d297b710c31f036f8cc5913772c","unresolved":false,"context_lines":[{"line_number":20,"context_line":"consistent way."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Change-Id: I71cf7526286e2b1140804e7aeac93c11dd8b7583"},{"line_number":23,"context_line":"Implements: blueprint pci-generate-stats"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":14,"id":"7a740942_da5faf73","line":23,"in_reply_to":"7a740942_f2409509","updated":"2015-12-10 15:42:02.000000000","message":"Done","commit_id":"6196e3af868b1cba95acb20b385b60d57987258e"}],"nova/pci/manager.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"aeabe88bf61de67b7aab05d897af89bf347b50fc","unresolved":false,"context_lines":[{"line_number":175,"context_line":"            self.stats.add_device(dev_obj)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    def _claim_instance(self, context, instance, prefix\u003d\u0027\u0027):"},{"line_number":178,"context_line":"        def _pci_device_prop_match(pci_dev, specs):"},{"line_number":179,"context_line":"            \"\"\"Check if the pci_dev meet spec requirement"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"            Specs is a list of PCI device property requirements."}],"source_content_type":"text/x-python","patch_set":16,"id":"7a740942_59b7edd7","line":178,"updated":"2015-12-12 16:28:16.000000000","message":"It makes sense that _matching_devices is a closure in _pci_device_prop_match but it isn\u0027t clear to me why _pci_device_prop_match is a closure in _claim_instance? Should it be a staticmethod up one level? I\u0027m not entirely sure, just asking.","commit_id":"0e50360039803e9270053eaf5964d7c37f51ca89"},{"author":{"_account_id":6508,"name":"Dmitry Stepanenko","email":"dstepanenko@mirantis.com","username":"dstepanenko"},"change_message_id":"6cee5e6e767b0c616563ee4daf1666e1ce58b33c","unresolved":false,"context_lines":[{"line_number":175,"context_line":"            self.stats.add_device(dev_obj)"},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"    def _claim_instance(self, context, instance, prefix\u003d\u0027\u0027):"},{"line_number":178,"context_line":"        def _pci_device_prop_match(pci_dev, specs):"},{"line_number":179,"context_line":"            \"\"\"Check if the pci_dev meet spec requirement"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"            Specs is a list of PCI device property requirements."}],"source_content_type":"text/x-python","patch_set":16,"id":"5a710552_2bfa3ebe","line":178,"in_reply_to":"7a740942_59b7edd7","updated":"2015-12-15 14:43:41.000000000","message":"Done","commit_id":"0e50360039803e9270053eaf5964d7c37f51ca89"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"bb345355f955acc4c2bc826d2111b5429fa0836d","unresolved":false,"context_lines":[{"line_number":181,"context_line":"            instance_cells \u003d instance_numa_topology.cells"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"        devs \u003d self.stats.consume_requests(pci_requests.requests,"},{"line_number":184,"context_line":"                                           instance_cells)"},{"line_number":185,"context_line":"        if not devs:"},{"line_number":186,"context_line":"            return None"},{"line_number":187,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"3a57f1b5_19b010dd","side":"PARENT","line":184,"updated":"2016-02-10 13:10:06.000000000","message":"I was expecting to see a removal of the PciDevStats.consume_requests() method in this patch, since this is the only place that calls that code.","commit_id":"b01de1932a587a98c6d203535cd05d724f21c375"},{"author":{"_account_id":6508,"name":"Dmitry Stepanenko","email":"dstepanenko@mirantis.com","username":"dstepanenko"},"change_message_id":"c3b453fe1d97f5006a3dd1d41e23b0892a95940a","unresolved":false,"context_lines":[{"line_number":181,"context_line":"            instance_cells \u003d instance_numa_topology.cells"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"        devs \u003d self.stats.consume_requests(pci_requests.requests,"},{"line_number":184,"context_line":"                                           instance_cells)"},{"line_number":185,"context_line":"        if not devs:"},{"line_number":186,"context_line":"            return None"},{"line_number":187,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"3a57f1b5_81ec4990","side":"PARENT","line":184,"in_reply_to":"3a57f1b5_19b010dd","updated":"2016-02-12 15:07:19.000000000","message":"Done","commit_id":"b01de1932a587a98c6d203535cd05d724f21c375"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"bb345355f955acc4c2bc826d2111b5429fa0836d","unresolved":false,"context_lines":[{"line_number":28,"context_line":"from nova.pci import whitelist"},{"line_number":29,"context_line":"from nova.virt import hardware"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"import six"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"CONF \u003d cfg.CONF"},{"line_number":34,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":30,"id":"3a57f1b5_59b688dd","line":31,"updated":"2016-02-10 13:10:06.000000000","message":"This import needs to be right below:\n\n from oslo_serialization import jsonutils\n\nbecause it is a third-party library.","commit_id":"5628fc8bfd9da0af61a9e18350e3f24e2e8ab644"},{"author":{"_account_id":6508,"name":"Dmitry Stepanenko","email":"dstepanenko@mirantis.com","username":"dstepanenko"},"change_message_id":"c3b453fe1d97f5006a3dd1d41e23b0892a95940a","unresolved":false,"context_lines":[{"line_number":28,"context_line":"from nova.pci import whitelist"},{"line_number":29,"context_line":"from nova.virt import hardware"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"import six"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"CONF \u003d cfg.CONF"},{"line_number":34,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":30,"id":"3a57f1b5_afff2700","line":31,"in_reply_to":"3a57f1b5_59b688dd","updated":"2016-02-12 15:07:19.000000000","message":"Done","commit_id":"5628fc8bfd9da0af61a9e18350e3f24e2e8ab644"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"0f62319dab5c730afba0766a570520b155f20154","unresolved":false,"context_lines":[{"line_number":28,"context_line":"from nova.pci import whitelist"},{"line_number":29,"context_line":"from nova.virt import hardware"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"import six"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"CONF \u003d cfg.CONF"},{"line_number":34,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":30,"id":"3a57f1b5_433e40db","line":31,"in_reply_to":"3a57f1b5_59b688dd","updated":"2016-02-11 18:03:58.000000000","message":"agree, move to the second section","commit_id":"5628fc8bfd9da0af61a9e18350e3f24e2e8ab644"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"bb345355f955acc4c2bc826d2111b5429fa0836d","unresolved":false,"context_lines":[{"line_number":172,"context_line":"            self.stats.add_device(dev_obj)"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"    def _claim_instance(self, context, instance, prefix\u003d\u0027\u0027):"},{"line_number":175,"context_line":"        def _pci_device_prop_match(pci_dev, specs):"},{"line_number":176,"context_line":"            \"\"\"Check if the pci_dev meet spec requirement"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"            Specs is a list of PCI device property requirements."}],"source_content_type":"text/x-python","patch_set":30,"id":"3a57f1b5_79bb44c5","line":175,"updated":"2016-02-10 13:10:06.000000000","message":"Please move this function to be a module-level function instead of nested inside _claim_instance()","commit_id":"5628fc8bfd9da0af61a9e18350e3f24e2e8ab644"},{"author":{"_account_id":6508,"name":"Dmitry Stepanenko","email":"dstepanenko@mirantis.com","username":"dstepanenko"},"change_message_id":"c3b453fe1d97f5006a3dd1d41e23b0892a95940a","unresolved":false,"context_lines":[{"line_number":172,"context_line":"            self.stats.add_device(dev_obj)"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"    def _claim_instance(self, context, instance, prefix\u003d\u0027\u0027):"},{"line_number":175,"context_line":"        def _pci_device_prop_match(pci_dev, specs):"},{"line_number":176,"context_line":"            \"\"\"Check if the pci_dev meet spec requirement"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"            Specs is a list of PCI device property requirements."}],"source_content_type":"text/x-python","patch_set":30,"id":"3a57f1b5_afd08754","line":175,"in_reply_to":"3a57f1b5_79bb44c5","updated":"2016-02-12 15:07:19.000000000","message":"Done","commit_id":"5628fc8bfd9da0af61a9e18350e3f24e2e8ab644"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"0f62319dab5c730afba0766a570520b155f20154","unresolved":false,"context_lines":[{"line_number":185,"context_line":""},{"line_number":186,"context_line":"            \"\"\""},{"line_number":187,"context_line":"            def _matching_devices(spec):"},{"line_number":188,"context_line":"                return pci_dev.status \u003d\u003d fields.PciDeviceStatus.AVAILABLE \\"},{"line_number":189,"context_line":"                        and all(getattr(pci_dev, k, None) \u003d\u003d v"},{"line_number":190,"context_line":"                        for k, v in six.iteritems(spec))"},{"line_number":191,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"3a57f1b5_a304d417","line":188,"range":{"start_line":188,"start_character":74,"end_line":188,"end_character":75},"updated":"2016-02-11 18:03:58.000000000","message":"try not to use backslashes \u0027\\\u0027, use brackets to avoid them","commit_id":"5628fc8bfd9da0af61a9e18350e3f24e2e8ab644"},{"author":{"_account_id":6508,"name":"Dmitry Stepanenko","email":"dstepanenko@mirantis.com","username":"dstepanenko"},"change_message_id":"c3b453fe1d97f5006a3dd1d41e23b0892a95940a","unresolved":false,"context_lines":[{"line_number":185,"context_line":""},{"line_number":186,"context_line":"            \"\"\""},{"line_number":187,"context_line":"            def _matching_devices(spec):"},{"line_number":188,"context_line":"                return pci_dev.status \u003d\u003d fields.PciDeviceStatus.AVAILABLE \\"},{"line_number":189,"context_line":"                        and all(getattr(pci_dev, k, None) \u003d\u003d v"},{"line_number":190,"context_line":"                        for k, v in six.iteritems(spec))"},{"line_number":191,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"3a57f1b5_892432c5","line":188,"range":{"start_line":188,"start_character":74,"end_line":188,"end_character":75},"in_reply_to":"3a57f1b5_a304d417","updated":"2016-02-12 15:07:19.000000000","message":"Done","commit_id":"5628fc8bfd9da0af61a9e18350e3f24e2e8ab644"},{"author":{"_account_id":10618,"name":"Pavel Kholkin","email":"p.v.holkin@mail.ru","username":"pkholkin"},"change_message_id":"0f62319dab5c730afba0766a570520b155f20154","unresolved":false,"context_lines":[{"line_number":266,"context_line":""},{"line_number":267,"context_line":"    def get_free_devs(self):"},{"line_number":268,"context_line":"        return [dev for dev in self.pci_devs"},{"line_number":269,"context_line":"                    if dev.status \u003d\u003d fields.PciDeviceStatus.AVAILABLE]"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def _free_device(self, dev, instance\u003dNone):"},{"line_number":272,"context_line":"        freed_devs \u003d dev.free(instance)"}],"source_content_type":"text/x-python","patch_set":30,"id":"3a57f1b5_8e069f1b","line":269,"updated":"2016-02-11 18:03:58.000000000","message":"don\u0027t like this indent","commit_id":"5628fc8bfd9da0af61a9e18350e3f24e2e8ab644"},{"author":{"_account_id":6508,"name":"Dmitry Stepanenko","email":"dstepanenko@mirantis.com","username":"dstepanenko"},"change_message_id":"c3b453fe1d97f5006a3dd1d41e23b0892a95940a","unresolved":false,"context_lines":[{"line_number":266,"context_line":""},{"line_number":267,"context_line":"    def get_free_devs(self):"},{"line_number":268,"context_line":"        return [dev for dev in self.pci_devs"},{"line_number":269,"context_line":"                    if dev.status \u003d\u003d fields.PciDeviceStatus.AVAILABLE]"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def _free_device(self, dev, instance\u003dNone):"},{"line_number":272,"context_line":"        freed_devs \u003d dev.free(instance)"}],"source_content_type":"text/x-python","patch_set":30,"id":"3a57f1b5_c470cfa6","line":269,"in_reply_to":"3a57f1b5_8e069f1b","updated":"2016-02-12 15:07:19.000000000","message":"Done","commit_id":"5628fc8bfd9da0af61a9e18350e3f24e2e8ab644"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"51025d6b190f436b757fbd72c3ca589e8e0ae03c","unresolved":false,"context_lines":[{"line_number":19,"context_line":"from oslo_config import cfg"},{"line_number":20,"context_line":"from oslo_log import log as logging"},{"line_number":21,"context_line":"from oslo_serialization import jsonutils"},{"line_number":22,"context_line":"from nova.i18n import _LW, _LE"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"from nova import exception"},{"line_number":25,"context_line":"from nova import objects"}],"source_content_type":"text/x-python","patch_set":31,"id":"3a57f1b5_d9f8b1c5","line":22,"updated":"2016-02-12 14:48:24.000000000","message":"Why this line moved out from the nova imports block ?","commit_id":"c238e4fff97071011f5f22122797db1a5da4efa8"},{"author":{"_account_id":6508,"name":"Dmitry Stepanenko","email":"dstepanenko@mirantis.com","username":"dstepanenko"},"change_message_id":"68f4a63e35295c6993513c604f3b1bbcabb094b1","unresolved":false,"context_lines":[{"line_number":19,"context_line":"from oslo_config import cfg"},{"line_number":20,"context_line":"from oslo_log import log as logging"},{"line_number":21,"context_line":"from oslo_serialization import jsonutils"},{"line_number":22,"context_line":"from nova.i18n import _LW, _LE"},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"from nova import exception"},{"line_number":25,"context_line":"from nova import objects"}],"source_content_type":"text/x-python","patch_set":31,"id":"3a57f1b5_c4fa0875","line":22,"in_reply_to":"3a57f1b5_d9f8b1c5","updated":"2016-02-12 15:07:11.000000000","message":"Because it\u0027s third party library. It was discussed in the previous patch","commit_id":"c238e4fff97071011f5f22122797db1a5da4efa8"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"51025d6b190f436b757fbd72c3ca589e8e0ae03c","unresolved":false,"context_lines":[{"line_number":208,"context_line":"                continue"},{"line_number":209,"context_line":"            spec \u003d request.spec"},{"line_number":210,"context_line":"            filtered_devs \u003d [dev for dev in self.pci_devs"},{"line_number":211,"context_line":"                            if self._pci_device_prop_match(dev, spec)]"},{"line_number":212,"context_line":"            # Some systems don\u0027t report numa node info for pci devices, in"},{"line_number":213,"context_line":"            # that case None is reported in pci_device.numa_node, by adding"},{"line_number":214,"context_line":"            # None to numa_cells we allow assigning those devices to instances"}],"source_content_type":"text/x-python","patch_set":31,"id":"3a57f1b5_59e4c1df","line":211,"updated":"2016-02-12 14:48:24.000000000","message":"Nit: I don\u0027t like this indent, \"if\" should be aligned with \"dev\".","commit_id":"c238e4fff97071011f5f22122797db1a5da4efa8"},{"author":{"_account_id":6508,"name":"Dmitry Stepanenko","email":"dstepanenko@mirantis.com","username":"dstepanenko"},"change_message_id":"68f4a63e35295c6993513c604f3b1bbcabb094b1","unresolved":false,"context_lines":[{"line_number":208,"context_line":"                continue"},{"line_number":209,"context_line":"            spec \u003d request.spec"},{"line_number":210,"context_line":"            filtered_devs \u003d [dev for dev in self.pci_devs"},{"line_number":211,"context_line":"                            if self._pci_device_prop_match(dev, spec)]"},{"line_number":212,"context_line":"            # Some systems don\u0027t report numa node info for pci devices, in"},{"line_number":213,"context_line":"            # that case None is reported in pci_device.numa_node, by adding"},{"line_number":214,"context_line":"            # None to numa_cells we allow assigning those devices to instances"}],"source_content_type":"text/x-python","patch_set":31,"id":"3a57f1b5_e4a2a498","line":211,"in_reply_to":"3a57f1b5_59e4c1df","updated":"2016-02-12 15:07:11.000000000","message":"Done","commit_id":"c238e4fff97071011f5f22122797db1a5da4efa8"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"40b9717e9b657c7d1a1cabf223e804db672fbffe","unresolved":false,"context_lines":[{"line_number":28,"context_line":"from nova.pci import whitelist"},{"line_number":29,"context_line":"from nova.virt import hardware"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"import six"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"CONF \u003d cfg.CONF"},{"line_number":34,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":32,"id":"dae33548_1b240ac3","line":31,"updated":"2016-02-17 18:06:38.000000000","message":"Both of these imports seem out of order with the rules.","commit_id":"43b0db18f6443f305c078ba5d7595bdc44c05ff1"},{"author":{"_account_id":6508,"name":"Dmitry Stepanenko","email":"dstepanenko@mirantis.com","username":"dstepanenko"},"change_message_id":"c307684ad5c43b174e7193a3486875c0c463d7b8","unresolved":false,"context_lines":[{"line_number":28,"context_line":"from nova.pci import whitelist"},{"line_number":29,"context_line":"from nova.virt import hardware"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"import six"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"CONF \u003d cfg.CONF"},{"line_number":34,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":32,"id":"bae84128_eb22acb8","line":31,"in_reply_to":"dae33548_1b240ac3","updated":"2016-02-20 11:21:15.000000000","message":"Done","commit_id":"43b0db18f6443f305c078ba5d7595bdc44c05ff1"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"7edaf554063a294dad45a2e619fc50981696131e","unresolved":false,"context_lines":[{"line_number":254,"context_line":"        if devs:"},{"line_number":255,"context_line":"            self.allocations[instance[\u0027uuid\u0027]] +\u003d devs"},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"    def claim_instance(self, context, instance):"},{"line_number":258,"context_line":"        if not self.pci_devs:"},{"line_number":259,"context_line":"            return"},{"line_number":260,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"7af24918_6cc7f945","line":257,"updated":"2016-03-01 14:39:04.000000000","message":"Go ahead and put a doc comment here that states claim_instance() is *not* thread-safe and that the COMPUTE_NODE_SEMAPHORE needs to be held while calling this.","commit_id":"2e3206f883981497ced80082f51e81b1b92d6aab"},{"author":{"_account_id":6508,"name":"Dmitry Stepanenko","email":"dstepanenko@mirantis.com","username":"dstepanenko"},"change_message_id":"aa8635cef5bb545bfce19a6da8d0214f687cffd3","unresolved":false,"context_lines":[{"line_number":254,"context_line":"        if devs:"},{"line_number":255,"context_line":"            self.allocations[instance[\u0027uuid\u0027]] +\u003d devs"},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"    def claim_instance(self, context, instance):"},{"line_number":258,"context_line":"        if not self.pci_devs:"},{"line_number":259,"context_line":"            return"},{"line_number":260,"context_line":""}],"source_content_type":"text/x-python","patch_set":39,"id":"7af24918_ecc81e77","line":257,"in_reply_to":"7af24918_6cc7f945","updated":"2016-03-01 18:23:43.000000000","message":"Done","commit_id":"2e3206f883981497ced80082f51e81b1b92d6aab"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"7edaf554063a294dad45a2e619fc50981696131e","unresolved":false,"context_lines":[{"line_number":266,"context_line":""},{"line_number":267,"context_line":"    def get_free_devs(self):"},{"line_number":268,"context_line":"        return [dev for dev in self.pci_devs"},{"line_number":269,"context_line":"                if dev.status \u003d\u003d fields.PciDeviceStatus.AVAILABLE]"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def _free_device(self, dev, instance\u003dNone):"},{"line_number":272,"context_line":"        freed_devs \u003d dev.free(instance)"}],"source_content_type":"text/x-python","patch_set":39,"id":"7af24918_4c35dd44","line":269,"updated":"2016-03-01 14:39:04.000000000","message":"How about making this a method get_available_devices() on nova.objects.PciDeviceList?","commit_id":"2e3206f883981497ced80082f51e81b1b92d6aab"},{"author":{"_account_id":6508,"name":"Dmitry Stepanenko","email":"dstepanenko@mirantis.com","username":"dstepanenko"},"change_message_id":"aa8635cef5bb545bfce19a6da8d0214f687cffd3","unresolved":false,"context_lines":[{"line_number":266,"context_line":""},{"line_number":267,"context_line":"    def get_free_devs(self):"},{"line_number":268,"context_line":"        return [dev for dev in self.pci_devs"},{"line_number":269,"context_line":"                if dev.status \u003d\u003d fields.PciDeviceStatus.AVAILABLE]"},{"line_number":270,"context_line":""},{"line_number":271,"context_line":"    def _free_device(self, dev, instance\u003dNone):"},{"line_number":272,"context_line":"        freed_devs \u003d dev.free(instance)"}],"source_content_type":"text/x-python","patch_set":39,"id":"7af24918_e83f6323","line":269,"in_reply_to":"7af24918_4c35dd44","updated":"2016-03-01 18:23:43.000000000","message":"Done","commit_id":"2e3206f883981497ced80082f51e81b1b92d6aab"}],"nova/tests/unit/pci/test_manager.py":[{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"aeabe88bf61de67b7aab05d897af89bf347b50fc","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        self.tracker.update_pci_for_migration(None, self.inst)"},{"line_number":276,"context_line":"        free_devs \u003d self.tracker.get_free_devs()"},{"line_number":277,"context_line":"        self.tracker.update_pci_for_migration(None, self.inst, sign\u003d-1)"},{"line_number":278,"context_line":"        free_devs \u003d self.tracker.get_free_devs()"},{"line_number":279,"context_line":"        self.assertEqual(len(free_devs), 3)"},{"line_number":280,"context_line":"        self.assertEqual(set([dev.vendor_id for"},{"line_number":281,"context_line":"                              dev in self.tracker.pci_devs]),"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a740942_79bc29bb","line":278,"updated":"2015-12-12 16:28:16.000000000","message":"Is there a duplicate assignment of free_devs here or is there a side effect in get_free_devs()? If the latter itmight make sense to have a clarifying comment.","commit_id":"0e50360039803e9270053eaf5964d7c37f51ca89"},{"author":{"_account_id":6508,"name":"Dmitry Stepanenko","email":"dstepanenko@mirantis.com","username":"dstepanenko"},"change_message_id":"6cee5e6e767b0c616563ee4daf1666e1ce58b33c","unresolved":false,"context_lines":[{"line_number":275,"context_line":"        self.tracker.update_pci_for_migration(None, self.inst)"},{"line_number":276,"context_line":"        free_devs \u003d self.tracker.get_free_devs()"},{"line_number":277,"context_line":"        self.tracker.update_pci_for_migration(None, self.inst, sign\u003d-1)"},{"line_number":278,"context_line":"        free_devs \u003d self.tracker.get_free_devs()"},{"line_number":279,"context_line":"        self.assertEqual(len(free_devs), 3)"},{"line_number":280,"context_line":"        self.assertEqual(set([dev.vendor_id for"},{"line_number":281,"context_line":"                              dev in self.tracker.pci_devs]),"}],"source_content_type":"text/x-python","patch_set":16,"id":"5a710552_28d24401","line":278,"in_reply_to":"7a740942_79bc29bb","updated":"2015-12-15 14:43:41.000000000","message":"Nope, it\u0027s on the right place. We claimed instance and checked that 2 out of 3 pci devices were allocated. After that we delete an instance and check if this 2 claimed pci devices were freed.","commit_id":"0e50360039803e9270053eaf5964d7c37f51ca89"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"a0d42a61246f27cf032fbd06020944f9bab5675f","unresolved":false,"context_lines":[{"line_number":276,"context_line":"    def test_update_pci_for_migration_out(self, mock_get):"},{"line_number":277,"context_line":"        self._create_pci_requests_object(mock_get, fake_pci_requests)"},{"line_number":278,"context_line":"        self.tracker.update_pci_for_migration(None, self.inst)"},{"line_number":279,"context_line":"        free_devs \u003d self.tracker.get_free_devs()"},{"line_number":280,"context_line":"        self.tracker.update_pci_for_migration(None, self.inst, sign\u003d-1)"},{"line_number":281,"context_line":"        free_devs \u003d self.tracker.get_free_devs()"},{"line_number":282,"context_line":"        self.assertEqual(len(free_devs), 3)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5a710552_0db1928f","line":279,"updated":"2015-12-15 16:10:53.000000000","message":"Sorry I guess I wasn\u0027t quite clear. What I meant was that the line above (#279) assigns to a value that is never used _and_ get_free_devs has no side effects. Therefore either line 279 can be removed (if you do that and run the test everything still passes) _or_ you should add an assert directly after it to confirm the value of free_devs before running `update_pci_for_migration` the second time.","commit_id":"398b013c2c373f36dfdbbfd053e3a82175f059be"},{"author":{"_account_id":6508,"name":"Dmitry Stepanenko","email":"dstepanenko@mirantis.com","username":"dstepanenko"},"change_message_id":"5493463351c77b7c198a0f273d36ac18617f0a39","unresolved":false,"context_lines":[{"line_number":276,"context_line":"    def test_update_pci_for_migration_out(self, mock_get):"},{"line_number":277,"context_line":"        self._create_pci_requests_object(mock_get, fake_pci_requests)"},{"line_number":278,"context_line":"        self.tracker.update_pci_for_migration(None, self.inst)"},{"line_number":279,"context_line":"        free_devs \u003d self.tracker.get_free_devs()"},{"line_number":280,"context_line":"        self.tracker.update_pci_for_migration(None, self.inst, sign\u003d-1)"},{"line_number":281,"context_line":"        free_devs \u003d self.tracker.get_free_devs()"},{"line_number":282,"context_line":"        self.assertEqual(len(free_devs), 3)"}],"source_content_type":"text/x-python","patch_set":17,"id":"5a710552_78b79186","line":279,"in_reply_to":"5a710552_0db1928f","updated":"2015-12-16 08:23:53.000000000","message":"Chris, actually you were quite clear, but I missed the line I should\u0027ve change :). My bad. Now it should look better","commit_id":"398b013c2c373f36dfdbbfd053e3a82175f059be"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"d2ac9af52ef92b1b75b5907e5cfddd594df5b978","unresolved":false,"context_lines":[{"line_number":273,"context_line":"    def test_update_pci_for_migration_out(self, mock_get):"},{"line_number":274,"context_line":"        self._create_pci_requests_object(mock_get, fake_pci_requests)"},{"line_number":275,"context_line":"        self.tracker.update_pci_for_migration(None, self.inst)"},{"line_number":276,"context_line":"        free_devs \u003d self.tracker.get_free_devs()"},{"line_number":277,"context_line":"        self.tracker.update_pci_for_migration(None, self.inst, sign\u003d-1)"},{"line_number":278,"context_line":"        free_devs \u003d self.tracker.get_free_devs()"},{"line_number":279,"context_line":"        self.assertEqual(len(free_devs), 3)"}],"source_content_type":"text/x-python","patch_set":20,"id":"fa69d971_0e8ba4c4","line":276,"updated":"2016-01-04 14:14:54.000000000","message":"There should be an assertion to validate the value of free_devs here. Perhaps:\n\n      self.assertEqual(len(free_devs), 1)","commit_id":"a076023a42c755bcb62109afb2173a75cc052ff0"},{"author":{"_account_id":6508,"name":"Dmitry Stepanenko","email":"dstepanenko@mirantis.com","username":"dstepanenko"},"change_message_id":"7755e06e35a6220b401fb694bbca3f5059fee268","unresolved":false,"context_lines":[{"line_number":273,"context_line":"    def test_update_pci_for_migration_out(self, mock_get):"},{"line_number":274,"context_line":"        self._create_pci_requests_object(mock_get, fake_pci_requests)"},{"line_number":275,"context_line":"        self.tracker.update_pci_for_migration(None, self.inst)"},{"line_number":276,"context_line":"        free_devs \u003d self.tracker.get_free_devs()"},{"line_number":277,"context_line":"        self.tracker.update_pci_for_migration(None, self.inst, sign\u003d-1)"},{"line_number":278,"context_line":"        free_devs \u003d self.tracker.get_free_devs()"},{"line_number":279,"context_line":"        self.assertEqual(len(free_devs), 3)"}],"source_content_type":"text/x-python","patch_set":20,"id":"9a68dd71_3be40f3a","line":276,"in_reply_to":"fa69d971_0e8ba4c4","updated":"2016-01-20 13:19:52.000000000","message":"Done","commit_id":"a076023a42c755bcb62109afb2173a75cc052ff0"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"bb345355f955acc4c2bc826d2111b5429fa0836d","unresolved":false,"context_lines":[{"line_number":239,"context_line":"                    for status in fields.PciDeviceStatus.ALL]"},{"line_number":240,"context_line":"        devs.append(mock.MagicMock(status\u003dfields.PciDeviceStatus.AVAILABLE))"},{"line_number":241,"context_line":"        self.tracker.pci_devs \u003d devs"},{"line_number":242,"context_line":"        self.assertEqual([devs[0], devs[-1]], self.tracker.get_free_devs())"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"    @mock.patch(\u0027nova.objects.InstancePCIRequests.get_by_instance\u0027)"},{"line_number":245,"context_line":"    def test_pci_claim_instance_with_numa_fail(self, mock_get):"}],"source_content_type":"text/x-python","patch_set":30,"id":"3a57f1b5_39b5cccc","line":242,"updated":"2016-02-10 13:10:06.000000000","message":"Please use something other than devs[0] and devs[-1] above. It\u0027s not clear that AVAILABLE is the first device status in fields.PciDeviceStats.ALL. Something like this would be a clearer test IMHO:\n\n devs \u003d [mock.MagicMock(status\u003dfields.PciDeviceStatus.ALLOCATED)]\n self.tracker.pci_devs \u003d devs\n self.assertEqual(0, len(self.tracker.get_free_devs()))\n devs.append(mock.MagicMock(status\u003dfields.PciDeviceStatus.AVAILABLE))\n self.assertEqual(1, len(self.tracker.get_free_devs()))","commit_id":"5628fc8bfd9da0af61a9e18350e3f24e2e8ab644"},{"author":{"_account_id":6508,"name":"Dmitry Stepanenko","email":"dstepanenko@mirantis.com","username":"dstepanenko"},"change_message_id":"c3b453fe1d97f5006a3dd1d41e23b0892a95940a","unresolved":false,"context_lines":[{"line_number":239,"context_line":"                    for status in fields.PciDeviceStatus.ALL]"},{"line_number":240,"context_line":"        devs.append(mock.MagicMock(status\u003dfields.PciDeviceStatus.AVAILABLE))"},{"line_number":241,"context_line":"        self.tracker.pci_devs \u003d devs"},{"line_number":242,"context_line":"        self.assertEqual([devs[0], devs[-1]], self.tracker.get_free_devs())"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"    @mock.patch(\u0027nova.objects.InstancePCIRequests.get_by_instance\u0027)"},{"line_number":245,"context_line":"    def test_pci_claim_instance_with_numa_fail(self, mock_get):"}],"source_content_type":"text/x-python","patch_set":30,"id":"3a57f1b5_1c151e3f","line":242,"in_reply_to":"3a57f1b5_39b5cccc","updated":"2016-02-12 15:07:19.000000000","message":"Done","commit_id":"5628fc8bfd9da0af61a9e18350e3f24e2e8ab644"}]}