)]}'
{"nova/objects/pci_device_pool.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b6781d1b42a36806f6eccc87686b04ce0c04668c","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    fields \u003d {"},{"line_number":33,"context_line":"        \u0027product_id\u0027: fields.StringField(),"},{"line_number":34,"context_line":"        \u0027vendor_id\u0027: fields.StringField(),"},{"line_number":35,"context_line":"        \u0027numa_node\u0027: fields.IntegerField(nullable\u003dTrue),"},{"line_number":36,"context_line":"        \u0027tags\u0027: fields.DictOfNullableStringsField(),"},{"line_number":37,"context_line":"        \u0027count\u0027: fields.IntegerField(),"},{"line_number":38,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aaa499b_45d8d91d","line":35,"updated":"2015-04-10 20:28:08.000000000","message":"I\u0027m very unfamiliar with this PCI code, but I see that PciDeviceStats is consuming compute_node.pci_device_pools and has some \u0027pool_keys\u0027 where \u0027numa_node\u0027 is one of them.\n\nWhere did we fail to have tests that covered a case that this wasn\u0027t set on the PciDevicePool object so we got a string rather than an int and have we shored those up now?","commit_id":"8840cfda51585bdf0cc4d51a87d882e4bbc05d70"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bf99ed7429ac0e6a454f0dbdec8a6b4c21474ce8","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    fields \u003d {"},{"line_number":33,"context_line":"        \u0027product_id\u0027: fields.StringField(),"},{"line_number":34,"context_line":"        \u0027vendor_id\u0027: fields.StringField(),"},{"line_number":35,"context_line":"        \u0027numa_node\u0027: fields.IntegerField(nullable\u003dTrue),"},{"line_number":36,"context_line":"        \u0027tags\u0027: fields.DictOfNullableStringsField(),"},{"line_number":37,"context_line":"        \u0027count\u0027: fields.IntegerField(),"},{"line_number":38,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aaa499b_a5267d14","line":35,"in_reply_to":"7aaa499b_45d8d91d","updated":"2015-04-10 20:31:21.000000000","message":"Should there be a more detailed test on NUMATopologyFilter?  I see in the unit tests for that it sets pci_stats\u003dNone on the fake host.","commit_id":"8840cfda51585bdf0cc4d51a87d882e4bbc05d70"},{"author":{"_account_id":11647,"name":"Przemyslaw Czesnowicz","email":"przemyslaw.czesnowicz@intel.com","username":"pczesno"},"change_message_id":"0482703bd9630cf850674cc1d33ff006944f30cd","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    fields \u003d {"},{"line_number":33,"context_line":"        \u0027product_id\u0027: fields.StringField(),"},{"line_number":34,"context_line":"        \u0027vendor_id\u0027: fields.StringField(),"},{"line_number":35,"context_line":"        \u0027numa_node\u0027: fields.IntegerField(nullable\u003dTrue),"},{"line_number":36,"context_line":"        \u0027tags\u0027: fields.DictOfNullableStringsField(),"},{"line_number":37,"context_line":"        \u0027count\u0027: fields.IntegerField(),"},{"line_number":38,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aaa499b_70736e81","line":35,"in_reply_to":"7aaa499b_51a40256","updated":"2015-04-13 15:36:03.000000000","message":"Thanks Nikola, that\u0027s way better explanation than i would give.","commit_id":"8840cfda51585bdf0cc4d51a87d882e4bbc05d70"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"f10e151b98c9a3b570e23e9e6e09f4550f51b5e2","unresolved":false,"context_lines":[{"line_number":32,"context_line":"    fields \u003d {"},{"line_number":33,"context_line":"        \u0027product_id\u0027: fields.StringField(),"},{"line_number":34,"context_line":"        \u0027vendor_id\u0027: fields.StringField(),"},{"line_number":35,"context_line":"        \u0027numa_node\u0027: fields.IntegerField(nullable\u003dTrue),"},{"line_number":36,"context_line":"        \u0027tags\u0027: fields.DictOfNullableStringsField(),"},{"line_number":37,"context_line":"        \u0027count\u0027: fields.IntegerField(),"},{"line_number":38,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aaa499b_51a40256","line":35,"in_reply_to":"7aaa499b_a5267d14","updated":"2015-04-13 10:38:52.000000000","message":"Well the logic of claiming/consuming instances is not what caused this bug (and it is well tested). What caused it is moving to and from objects in the resource tracker, and then dropping an important field due to that.\n\nSo in the resource tracker we have pci pool dicts (would contain numa_node as a key), but then we convert them to objects [1] by using PciDevicePool.from_dict, which would drop the field and save it as part of tags.\n\nUpon reading them in on the scheduler side, we call PciDevicePool.to_dict, which of course will not know about the \u0027numa_node\u0027 since it\u0027s not on the object. For filtering to work numa_node needs to match (and it\u0027s actually gone) so all requests fail.\n\nThe code in [1] merged 2 months after the patch that introduces NUMA awareness of PCI devices.\n\nTL; DR - this got borken by our neverending objects fuckery - it was not broken by definition as you seem to imply with your comments here.\n\nThe commit message is misleading though so I will update it now.\n\n[1] https://github.com/openstack/nova/blob/6b69b705481958bc55f9d4e29732b09a45f392a5/nova/scheduler/client/report.py#L64","commit_id":"8840cfda51585bdf0cc4d51a87d882e4bbc05d70"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"194c8d068962e6eb0cd844086a914e774537e488","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        \u0027count\u0027: fields.IntegerField(),"},{"line_number":38,"context_line":"        }"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":41,"context_line":"        target_version \u003d utils.convert_version_to_tuple(target_version)"},{"line_number":42,"context_line":"        if target_version \u003c (1, 1) and \u0027numa_node\u0027 in primitive:"},{"line_number":43,"context_line":"            del primitive[\u0027numa_node\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aaa499b_a572fd7a","line":40,"updated":"2015-04-10 20:18:04.000000000","message":"We should have a unit test that validates this does what it\u0027s supposed to.","commit_id":"8840cfda51585bdf0cc4d51a87d882e4bbc05d70"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"f10e151b98c9a3b570e23e9e6e09f4550f51b5e2","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        \u0027count\u0027: fields.IntegerField(),"},{"line_number":38,"context_line":"        }"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":41,"context_line":"        target_version \u003d utils.convert_version_to_tuple(target_version)"},{"line_number":42,"context_line":"        if target_version \u003c (1, 1) and \u0027numa_node\u0027 in primitive:"},{"line_number":43,"context_line":"            del primitive[\u0027numa_node\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aaa499b_b637bcaf","line":40,"in_reply_to":"7aaa499b_a572fd7a","updated":"2015-04-13 10:38:52.000000000","message":"A lot of these are not tested, and simply additive changes are borderline irrelevant. I would not block on this personally.","commit_id":"8840cfda51585bdf0cc4d51a87d882e4bbc05d70"},{"author":{"_account_id":11647,"name":"Przemyslaw Czesnowicz","email":"przemyslaw.czesnowicz@intel.com","username":"pczesno"},"change_message_id":"0482703bd9630cf850674cc1d33ff006944f30cd","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        \u0027count\u0027: fields.IntegerField(),"},{"line_number":38,"context_line":"        }"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":41,"context_line":"        target_version \u003d utils.convert_version_to_tuple(target_version)"},{"line_number":42,"context_line":"        if target_version \u003c (1, 1) and \u0027numa_node\u0027 in primitive:"},{"line_number":43,"context_line":"            del primitive[\u0027numa_node\u0027]"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aaa499b_306de698","line":40,"in_reply_to":"7aaa499b_a572fd7a","updated":"2015-04-13 15:36:03.000000000","message":"Done","commit_id":"8840cfda51585bdf0cc4d51a87d882e4bbc05d70"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"db01812d78617c498dfc57ca902cb9f65c6d7d10","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        pool \u003d cls()"},{"line_number":52,"context_line":"        pool.vendor_id \u003d pool_dict.pop(\"vendor_id\")"},{"line_number":53,"context_line":"        pool.product_id \u003d pool_dict.pop(\"product_id\")"},{"line_number":54,"context_line":"        pool.numa_node \u003d pool_dict.pop(\"numa_node\", None)"},{"line_number":55,"context_line":"        pool.count \u003d pool_dict.pop(\"count\")"},{"line_number":56,"context_line":"        pool.tags \u003d {}"},{"line_number":57,"context_line":"        pool.tags.update(pool_dict)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aaa499b_67ad2031","line":54,"updated":"2015-04-10 17:46:36.000000000","message":"So I wonder if it\u0027s possible to actually use PciStats.pool_keys here somehow, so that we a) avoid future breakage b) avoid code repetition like this (I think pool_keys should actually live in this object)\n\nNot going to block this bugfix on it though.","commit_id":"8840cfda51585bdf0cc4d51a87d882e4bbc05d70"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"4f7689ffb6b8c8bf4fee6867c1b7e9f363cbb0fe","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        pool \u003d cls()"},{"line_number":52,"context_line":"        pool.vendor_id \u003d pool_dict.pop(\"vendor_id\")"},{"line_number":53,"context_line":"        pool.product_id \u003d pool_dict.pop(\"product_id\")"},{"line_number":54,"context_line":"        pool.numa_node \u003d pool_dict.pop(\"numa_node\", None)"},{"line_number":55,"context_line":"        pool.count \u003d pool_dict.pop(\"count\")"},{"line_number":56,"context_line":"        pool.tags \u003d {}"},{"line_number":57,"context_line":"        pool.tags.update(pool_dict)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aaa499b_78523a15","line":54,"in_reply_to":"7aaa499b_67ad2031","updated":"2015-04-13 14:25:42.000000000","message":"Indeed that makes sense","commit_id":"8840cfda51585bdf0cc4d51a87d882e4bbc05d70"},{"author":{"_account_id":11647,"name":"Przemyslaw Czesnowicz","email":"przemyslaw.czesnowicz@intel.com","username":"pczesno"},"change_message_id":"0482703bd9630cf850674cc1d33ff006944f30cd","unresolved":false,"context_lines":[{"line_number":51,"context_line":"        pool \u003d cls()"},{"line_number":52,"context_line":"        pool.vendor_id \u003d pool_dict.pop(\"vendor_id\")"},{"line_number":53,"context_line":"        pool.product_id \u003d pool_dict.pop(\"product_id\")"},{"line_number":54,"context_line":"        pool.numa_node \u003d pool_dict.pop(\"numa_node\", None)"},{"line_number":55,"context_line":"        pool.count \u003d pool_dict.pop(\"count\")"},{"line_number":56,"context_line":"        pool.tags \u003d {}"},{"line_number":57,"context_line":"        pool.tags.update(pool_dict)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7aaa499b_f0d3fe36","line":54,"in_reply_to":"7aaa499b_78523a15","updated":"2015-04-13 15:36:03.000000000","message":"i\u0027ll add that to my todo list, if you don\u0027t mind","commit_id":"8840cfda51585bdf0cc4d51a87d882e4bbc05d70"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2427dd070b65eb62d5bf7d9683b94fbf95af313b","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        \u0027count\u0027: fields.IntegerField(),"},{"line_number":38,"context_line":"        }"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":41,"context_line":"        target_version \u003d utils.convert_version_to_tuple(target_version)"},{"line_number":42,"context_line":"        if target_version \u003c (1, 1) and \u0027numa_node\u0027 in primitive:"},{"line_number":43,"context_line":"            del primitive[\u0027numa_node\u0027]"}],"source_content_type":"text/x-python","patch_set":2,"id":"7aaa499b_6e94b036","line":40,"updated":"2015-04-13 15:13:12.000000000","message":"Asked for a unit test in the previous patch set, so -1 on that.","commit_id":"9c1d10c34394d78f932072d1d9415fdda7fa73ff"}]}