)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"011bab78966e3fa84cc6e40ae0649f7e5fa5bf66","unresolved":false,"context_lines":[{"line_number":15,"context_line":"For a fresh compute node deploy, no reshaping will be necessary"},{"line_number":16,"context_line":"and the VGPU inventory will start on the child provider."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Part of blueprint reshape-provider-tree"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Co-Authored-By: Sylvain Bauza \u003csbauza@free.fr\u003e"},{"line_number":21,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"9fdfeff1_e29caf5e","line":18,"updated":"2019-01-25 02:37:11.000000000","message":"Going through the blueprint status etherpad [1], I notice again that you had mentioned you thought this change is more appropriate under the vgpu-stein blueprint. So I\u0027m not sure if you wanted to update this as such.\n\n[1] https://etherpad.openstack.org/p/nova-stein-blueprint-status","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6d9f464863466bc906017829d02a51d555694dc8","unresolved":false,"context_lines":[{"line_number":15,"context_line":"For a fresh compute node deploy, no reshaping will be necessary"},{"line_number":16,"context_line":"and the VGPU inventory will start on the child provider."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Part of blueprint reshape-provider-tree"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Co-Authored-By: Sylvain Bauza \u003csbauza@free.fr\u003e"},{"line_number":21,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"9fdfeff1_046d0831","line":18,"in_reply_to":"9fdfeff1_8acb6d70","updated":"2019-02-13 10:51:05.000000000","message":"Done","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6305329c08c72ff213efb40c4f33de8750eee714","unresolved":false,"context_lines":[{"line_number":15,"context_line":"For a fresh compute node deploy, no reshaping will be necessary"},{"line_number":16,"context_line":"and the VGPU inventory will start on the child provider."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Part of blueprint reshape-provider-tree"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Co-Authored-By: Sylvain Bauza \u003csbauza@free.fr\u003e"},{"line_number":21,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":12,"id":"9fdfeff1_8acb6d70","line":18,"in_reply_to":"9fdfeff1_e29caf5e","updated":"2019-01-25 16:37:33.000000000","message":"It\u0027s not a big deal. This doesn\u0027t make any VGPU support changes, like supporting multiple types. This is a dependency for blueprint vgpu-stein and definitely related to the reshape stuff, but I don\u0027t really care what we call it. The commit message could say it\u0027s related to both blueprints.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"}],"doc/source/admin/virtual-gpu.rst":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b6c7b3ec76115576a24c1007527c00e6e221ff52","unresolved":false,"context_lines":[{"line_number":51,"context_line":"#. Restart the ``nova-compute`` service."},{"line_number":52,"context_line":""},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"   .. warning::"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        Changing the type is possible but since existing physical GPUs can\u0027t"},{"line_number":57,"context_line":"        address multiple guests having different types, that will make Nova"}],"source_content_type":"text/x-rst","patch_set":15,"id":"9fdfeff1_217ec3e3","line":54,"range":{"start_line":54,"start_character":6,"end_line":54,"end_character":13},"updated":"2019-02-15 17:26:51.000000000","message":"Why is this warning coming in with this change set? It seems unrelated to reshaping.","commit_id":"8022e4dc6a1df3e14e6fec12d9994df91c987fcd"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b6c7b3ec76115576a24c1007527c00e6e221ff52","unresolved":false,"context_lines":[{"line_number":55,"context_line":""},{"line_number":56,"context_line":"        Changing the type is possible but since existing physical GPUs can\u0027t"},{"line_number":57,"context_line":"        address multiple guests having different types, that will make Nova"},{"line_number":58,"context_line":"        returning you a NoValidHost if existing instances with the original"},{"line_number":59,"context_line":"        type are created. Accordingly, it\u0027s highly recommended to rather deploy"},{"line_number":60,"context_line":"        the new type to new compute nodes that don\u0027t already have workloads and"},{"line_number":61,"context_line":"        rebuild instances on the nodes that need to change types."}],"source_content_type":"text/x-rst","patch_set":15,"id":"9fdfeff1_a1cdd35d","line":58,"range":{"start_line":58,"start_character":8,"end_line":58,"end_character":17},"updated":"2019-02-15 17:26:51.000000000","message":"return","commit_id":"8022e4dc6a1df3e14e6fec12d9994df91c987fcd"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b6c7b3ec76115576a24c1007527c00e6e221ff52","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        Changing the type is possible but since existing physical GPUs can\u0027t"},{"line_number":57,"context_line":"        address multiple guests having different types, that will make Nova"},{"line_number":58,"context_line":"        returning you a NoValidHost if existing instances with the original"},{"line_number":59,"context_line":"        type are created. Accordingly, it\u0027s highly recommended to rather deploy"},{"line_number":60,"context_line":"        the new type to new compute nodes that don\u0027t already have workloads and"},{"line_number":61,"context_line":"        rebuild instances on the nodes that need to change types."},{"line_number":62,"context_line":""}],"source_content_type":"text/x-rst","patch_set":15,"id":"9fdfeff1_210b238e","line":59,"range":{"start_line":59,"start_character":66,"end_line":59,"end_character":72},"updated":"2019-02-15 17:26:51.000000000","message":"instead","commit_id":"8022e4dc6a1df3e14e6fec12d9994df91c987fcd"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b6c7b3ec76115576a24c1007527c00e6e221ff52","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        Changing the type is possible but since existing physical GPUs can\u0027t"},{"line_number":57,"context_line":"        address multiple guests having different types, that will make Nova"},{"line_number":58,"context_line":"        returning you a NoValidHost if existing instances with the original"},{"line_number":59,"context_line":"        type are created. Accordingly, it\u0027s highly recommended to rather deploy"},{"line_number":60,"context_line":"        the new type to new compute nodes that don\u0027t already have workloads and"},{"line_number":61,"context_line":"        rebuild instances on the nodes that need to change types."},{"line_number":62,"context_line":""}],"source_content_type":"text/x-rst","patch_set":15,"id":"9fdfeff1_8140cfa6","line":59,"range":{"start_line":59,"start_character":13,"end_line":59,"end_character":24},"updated":"2019-02-15 17:26:51.000000000","message":"still exist?","commit_id":"8022e4dc6a1df3e14e6fec12d9994df91c987fcd"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f340723fa97ddca98f8230ebe2a672f13d1f9fab","unresolved":false,"context_lines":[{"line_number":18533,"context_line":"                               allocations\u003dallocations)"},{"line_number":18534,"context_line":"        self.assertIn(\u0027Unexpected VGPU resource allocation on provider %s\u0027"},{"line_number":18535,"context_line":"                      % uuids.other_rp, six.text_type(ex))"},{"line_number":18536,"context_line":""},{"line_number":18537,"context_line":""},{"line_number":18538,"context_line":"class TraitsComparisonMixin(object):"},{"line_number":18539,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_71f14695","line":18536,"updated":"2019-01-28 13:45:00.000000000","message":"I think couple of test cases are missing:\n1) config contains more than one item in enabled_vgpu_types\n2) instance has allocation in placement but not found on the hypervisor","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6d9f464863466bc906017829d02a51d555694dc8","unresolved":false,"context_lines":[{"line_number":18533,"context_line":"                               allocations\u003dallocations)"},{"line_number":18534,"context_line":"        self.assertIn(\u0027Unexpected VGPU resource allocation on provider %s\u0027"},{"line_number":18535,"context_line":"                      % uuids.other_rp, six.text_type(ex))"},{"line_number":18536,"context_line":""},{"line_number":18537,"context_line":""},{"line_number":18538,"context_line":"class TraitsComparisonMixin(object):"},{"line_number":18539,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_2a67b3eb","line":18536,"in_reply_to":"9fdfeff1_71f14695","updated":"2019-02-13 10:51:05.000000000","message":"\u003e I think couple of test cases are missing:\n \u003e 1) config contains more than one item in enabled_vgpu_types\n\nCan address it in a followup as it\u0027s unrelated to the change itself.\n\n \u003e 2) instance has allocation in placement but not found on the\n \u003e hypervisor\n\nAgain, unrelated to the change (and I need to understand what you mean by \"not found on hypervisor\")\nFWIW, we already have a test that checks whether we can allocate when we no longer have mdevs left :\nhttps://github.com/openstack/nova/blob/b291581/nova/tests/unit/virt/libvirt/test_driver.py#L20731","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b6c7b3ec76115576a24c1007527c00e6e221ff52","unresolved":false,"context_lines":[{"line_number":18178,"context_line":"        self.assertEqual(set([\u0027HW_CPU_X86_AVX512F\u0027, \u0027HW_CPU_X86_BMI\u0027]),"},{"line_number":18179,"context_line":"                         self.pt.data(self.cn_rp[\u0027uuid\u0027]).traits)"},{"line_number":18180,"context_line":""},{"line_number":18181,"context_line":"    def test_update_provider_tree_with_vgpus(self):"},{"line_number":18182,"context_line":"        pci_devices \u003d [\u0027pci_0000_06_00_0\u0027, \u0027pci_0000_07_00_0\u0027]"},{"line_number":18183,"context_line":"        gpu_inventory_dicts \u003d {"},{"line_number":18184,"context_line":"            pci_devices[0]: {\u0027total\u0027: 16,"}],"source_content_type":"text/x-python","patch_set":15,"id":"9fdfeff1_13d3d002","line":18181,"range":{"start_line":18181,"start_character":8,"end_line":18181,"end_character":44},"updated":"2019-02-15 17:26:51.000000000","message":"iiuc, this one is testing the \"stein green field\" case, i.e. where we\u0027re already running stein with no VGPUs and then we add VGPUs. So there\u0027s no reshape - they just go straight into the child providers. Right?","commit_id":"8022e4dc6a1df3e14e6fec12d9994df91c987fcd"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"124de5b1c6a065e1559d35933586deff46ed00a8","unresolved":false,"context_lines":[{"line_number":18178,"context_line":"        self.assertEqual(set([\u0027HW_CPU_X86_AVX512F\u0027, \u0027HW_CPU_X86_BMI\u0027]),"},{"line_number":18179,"context_line":"                         self.pt.data(self.cn_rp[\u0027uuid\u0027]).traits)"},{"line_number":18180,"context_line":""},{"line_number":18181,"context_line":"    def test_update_provider_tree_with_vgpus(self):"},{"line_number":18182,"context_line":"        pci_devices \u003d [\u0027pci_0000_06_00_0\u0027, \u0027pci_0000_07_00_0\u0027]"},{"line_number":18183,"context_line":"        gpu_inventory_dicts \u003d {"},{"line_number":18184,"context_line":"            pci_devices[0]: {\u0027total\u0027: 16,"}],"source_content_type":"text/x-python","patch_set":15,"id":"9fdfeff1_090be6d4","line":18181,"range":{"start_line":18181,"start_character":8,"end_line":18181,"end_character":44},"in_reply_to":"9fdfeff1_13d3d002","updated":"2019-02-26 11:14:15.000000000","message":"Correct, we just verify that we pass the correct inventories that are in children RPs when the virt driver gives us some pGPU resources.","commit_id":"8022e4dc6a1df3e14e6fec12d9994df91c987fcd"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b6c7b3ec76115576a24c1007527c00e6e221ff52","unresolved":false,"context_lines":[{"line_number":18385,"context_line":"        self.driver.update_provider_tree("},{"line_number":18386,"context_line":"            self.pt, self.cn_rp[\u0027name\u0027], allocations\u003dallocations)"},{"line_number":18387,"context_line":"        # We should have two new VGPU child providers in the tree under the"},{"line_number":18388,"context_line":"        # compute node root provider."},{"line_number":18389,"context_line":"        compute_node_tree_uuids \u003d self.pt.get_provider_uuids("},{"line_number":18390,"context_line":"            self.cn_rp[\u0027name\u0027])"},{"line_number":18391,"context_line":"        self.assertEqual(3, len(compute_node_tree_uuids))"},{"line_number":18392,"context_line":"        rp_per_pci_device \u003d {}"},{"line_number":18393,"context_line":"        # The VGPU child providers should be the 2nd and 3rd UUIDs in that list"},{"line_number":18394,"context_line":"        for rp_uuid in compute_node_tree_uuids[1:]:"},{"line_number":18395,"context_line":"            # The VGPU inventory should be on the VGPU child provider"},{"line_number":18396,"context_line":"            pgpu_provider_data \u003d self.pt.data(rp_uuid)"},{"line_number":18397,"context_line":"            # We want to map the PCI device with the RP UUID"},{"line_number":18398,"context_line":"            if pci_devices[0] in pgpu_provider_data.name:"},{"line_number":18399,"context_line":"                rp_per_pci_device[pci_devices[0]] \u003d rp_uuid"},{"line_number":18400,"context_line":"            elif pci_devices[1] in pgpu_provider_data.name:"},{"line_number":18401,"context_line":"                rp_per_pci_device[pci_devices[1]] \u003d rp_uuid"},{"line_number":18402,"context_line":"        # Make sure we have two child resource providers"},{"line_number":18403,"context_line":"        self.assertEqual(2, len(rp_per_pci_device))"},{"line_number":18404,"context_line":""},{"line_number":18405,"context_line":"        # The compute node root provider should not have VGPU inventory."},{"line_number":18406,"context_line":"        del inventory[orc.VGPU]"},{"line_number":18407,"context_line":"        self.assertEqual(inventory, self.pt.data(self.cn_rp[\u0027uuid\u0027]).inventory)"},{"line_number":18408,"context_line":"        # consumer1 should now have allocations against two providers,"},{"line_number":18409,"context_line":"        # MEMORY_MB on the root compute node provider and VGPU on the child"},{"line_number":18410,"context_line":"        # provider."}],"source_content_type":"text/x-python","patch_set":15,"id":"9fdfeff1_b32c4402","line":18407,"range":{"start_line":18388,"start_character":0,"end_line":18407,"end_character":79},"updated":"2019-02-15 17:26:51.000000000","message":"This could be deduped.","commit_id":"8022e4dc6a1df3e14e6fec12d9994df91c987fcd"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"124de5b1c6a065e1559d35933586deff46ed00a8","unresolved":false,"context_lines":[{"line_number":18385,"context_line":"        self.driver.update_provider_tree("},{"line_number":18386,"context_line":"            self.pt, self.cn_rp[\u0027name\u0027], allocations\u003dallocations)"},{"line_number":18387,"context_line":"        # We should have two new VGPU child providers in the tree under the"},{"line_number":18388,"context_line":"        # compute node root provider."},{"line_number":18389,"context_line":"        compute_node_tree_uuids \u003d self.pt.get_provider_uuids("},{"line_number":18390,"context_line":"            self.cn_rp[\u0027name\u0027])"},{"line_number":18391,"context_line":"        self.assertEqual(3, len(compute_node_tree_uuids))"},{"line_number":18392,"context_line":"        rp_per_pci_device \u003d {}"},{"line_number":18393,"context_line":"        # The VGPU child providers should be the 2nd and 3rd UUIDs in that list"},{"line_number":18394,"context_line":"        for rp_uuid in compute_node_tree_uuids[1:]:"},{"line_number":18395,"context_line":"            # The VGPU inventory should be on the VGPU child provider"},{"line_number":18396,"context_line":"            pgpu_provider_data \u003d self.pt.data(rp_uuid)"},{"line_number":18397,"context_line":"            # We want to map the PCI device with the RP UUID"},{"line_number":18398,"context_line":"            if pci_devices[0] in pgpu_provider_data.name:"},{"line_number":18399,"context_line":"                rp_per_pci_device[pci_devices[0]] \u003d rp_uuid"},{"line_number":18400,"context_line":"            elif pci_devices[1] in pgpu_provider_data.name:"},{"line_number":18401,"context_line":"                rp_per_pci_device[pci_devices[1]] \u003d rp_uuid"},{"line_number":18402,"context_line":"        # Make sure we have two child resource providers"},{"line_number":18403,"context_line":"        self.assertEqual(2, len(rp_per_pci_device))"},{"line_number":18404,"context_line":""},{"line_number":18405,"context_line":"        # The compute node root provider should not have VGPU inventory."},{"line_number":18406,"context_line":"        del inventory[orc.VGPU]"},{"line_number":18407,"context_line":"        self.assertEqual(inventory, self.pt.data(self.cn_rp[\u0027uuid\u0027]).inventory)"},{"line_number":18408,"context_line":"        # consumer1 should now have allocations against two providers,"},{"line_number":18409,"context_line":"        # MEMORY_MB on the root compute node provider and VGPU on the child"},{"line_number":18410,"context_line":"        # provider."}],"source_content_type":"text/x-python","patch_set":15,"id":"9fdfeff1_0924465f","line":18407,"range":{"start_line":18388,"start_character":0,"end_line":18407,"end_character":79},"in_reply_to":"9fdfeff1_b32c4402","updated":"2019-02-26 11:14:15.000000000","message":"Sure, but I prefer to have a specific verification here.","commit_id":"8022e4dc6a1df3e14e6fec12d9994df91c987fcd"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"fa47ba38c507f7763a830821b7ce70779aa7ada8","unresolved":false,"context_lines":[{"line_number":18181,"context_line":"        # root compute node provider inventory is unchanged"},{"line_number":18182,"context_line":"        self.assertEqual(inventory,"},{"line_number":18183,"context_line":"                         (self.pt.data(self.cn_rp[\u0027uuid\u0027])).inventory)"},{"line_number":18184,"context_line":"        # We should have two new pGPU child providers in the tree under the"},{"line_number":18185,"context_line":"        # compute node root provider."},{"line_number":18186,"context_line":"        compute_node_tree_uuids \u003d self.pt.get_provider_uuids("},{"line_number":18187,"context_line":"            self.cn_rp[\u0027name\u0027])"},{"line_number":18188,"context_line":"        self.assertEqual(3, len(compute_node_tree_uuids))"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_77d1e9fa","line":18185,"range":{"start_line":18184,"start_character":8,"end_line":18185,"end_character":37},"updated":"2019-02-28 16:01:22.000000000","message":"Why we have separated resource provider for same type vgpu? I didn\u0027t see the spec talk about this. But I guess this is something people already discussed, just want to what is the reason behind this. Since we always have problem when we want to request resource across the RP.","commit_id":"054eb3a652f340d819f1b04567d0ed5e207ed733"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"354e618a3a6a4dcb6853e591993b2112bc1a9c5d","unresolved":false,"context_lines":[{"line_number":18181,"context_line":"        # root compute node provider inventory is unchanged"},{"line_number":18182,"context_line":"        self.assertEqual(inventory,"},{"line_number":18183,"context_line":"                         (self.pt.data(self.cn_rp[\u0027uuid\u0027])).inventory)"},{"line_number":18184,"context_line":"        # We should have two new pGPU child providers in the tree under the"},{"line_number":18185,"context_line":"        # compute node root provider."},{"line_number":18186,"context_line":"        compute_node_tree_uuids \u003d self.pt.get_provider_uuids("},{"line_number":18187,"context_line":"            self.cn_rp[\u0027name\u0027])"},{"line_number":18188,"context_line":"        self.assertEqual(3, len(compute_node_tree_uuids))"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_75ff9765","line":18185,"range":{"start_line":18184,"start_character":8,"end_line":18185,"end_character":37},"in_reply_to":"9fdfeff1_33db74ed","updated":"2019-03-01 00:51:03.000000000","message":"Yea, I know, but they are same type resource. We won\u0027t separate the VCPU just because they are in different socket...In future yes, we want to separate VCPU as socket for cache stuff, but that is because the affinity.","commit_id":"054eb3a652f340d819f1b04567d0ed5e207ed733"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"575a5454dc1db91062c4899c68041122343454cc","unresolved":false,"context_lines":[{"line_number":18181,"context_line":"        # root compute node provider inventory is unchanged"},{"line_number":18182,"context_line":"        self.assertEqual(inventory,"},{"line_number":18183,"context_line":"                         (self.pt.data(self.cn_rp[\u0027uuid\u0027])).inventory)"},{"line_number":18184,"context_line":"        # We should have two new pGPU child providers in the tree under the"},{"line_number":18185,"context_line":"        # compute node root provider."},{"line_number":18186,"context_line":"        compute_node_tree_uuids \u003d self.pt.get_provider_uuids("},{"line_number":18187,"context_line":"            self.cn_rp[\u0027name\u0027])"},{"line_number":18188,"context_line":"        self.assertEqual(3, len(compute_node_tree_uuids))"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_8961adab","line":18185,"range":{"start_line":18184,"start_character":8,"end_line":18185,"end_character":37},"in_reply_to":"9fdfeff1_46aaebd1","updated":"2019-03-02 02:19:06.000000000","message":"If all the PGPU provides same type VGPUs, it doesn\u0027t matter allcoate VGPUs from which pGPU. And as we say, tracking the VGPU on which PGPU is tracking by nova, not placement.","commit_id":"054eb3a652f340d819f1b04567d0ed5e207ed733"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"afa6a5b7315538018c6d16dc2ee6289b3f54a50b","unresolved":false,"context_lines":[{"line_number":18181,"context_line":"        # root compute node provider inventory is unchanged"},{"line_number":18182,"context_line":"        self.assertEqual(inventory,"},{"line_number":18183,"context_line":"                         (self.pt.data(self.cn_rp[\u0027uuid\u0027])).inventory)"},{"line_number":18184,"context_line":"        # We should have two new pGPU child providers in the tree under the"},{"line_number":18185,"context_line":"        # compute node root provider."},{"line_number":18186,"context_line":"        compute_node_tree_uuids \u003d self.pt.get_provider_uuids("},{"line_number":18187,"context_line":"            self.cn_rp[\u0027name\u0027])"},{"line_number":18188,"context_line":"        self.assertEqual(3, len(compute_node_tree_uuids))"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_46aaebd1","line":18185,"range":{"start_line":18184,"start_character":8,"end_line":18185,"end_character":37},"in_reply_to":"9fdfeff1_75ff9765","updated":"2019-03-01 04:18:32.000000000","message":"The provider IDs are associated with the PCI addresses, and libvirt needs to know (and be able to rederive in certain cases) which allocation corresponds to which physical device.","commit_id":"054eb3a652f340d819f1b04567d0ed5e207ed733"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"8f61cf51bcde6d8762a0edb9ddf46cc543756dc2","unresolved":false,"context_lines":[{"line_number":18181,"context_line":"        # root compute node provider inventory is unchanged"},{"line_number":18182,"context_line":"        self.assertEqual(inventory,"},{"line_number":18183,"context_line":"                         (self.pt.data(self.cn_rp[\u0027uuid\u0027])).inventory)"},{"line_number":18184,"context_line":"        # We should have two new pGPU child providers in the tree under the"},{"line_number":18185,"context_line":"        # compute node root provider."},{"line_number":18186,"context_line":"        compute_node_tree_uuids \u003d self.pt.get_provider_uuids("},{"line_number":18187,"context_line":"            self.cn_rp[\u0027name\u0027])"},{"line_number":18188,"context_line":"        self.assertEqual(3, len(compute_node_tree_uuids))"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_33db74ed","line":18185,"range":{"start_line":18184,"start_character":8,"end_line":18185,"end_character":37},"in_reply_to":"9fdfeff1_77d1e9fa","updated":"2019-02-28 16:30:08.000000000","message":"The compute node can have multiple physical GPUs, each of which will have a resource provider record in placement and each of which will have separate inventory of VGPU resources.","commit_id":"054eb3a652f340d819f1b04567d0ed5e207ed733"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"af88e265e0005ea90cba070bc99baf3dd0194dd2","unresolved":false,"context_lines":[{"line_number":18181,"context_line":"        # root compute node provider inventory is unchanged"},{"line_number":18182,"context_line":"        self.assertEqual(inventory,"},{"line_number":18183,"context_line":"                         (self.pt.data(self.cn_rp[\u0027uuid\u0027])).inventory)"},{"line_number":18184,"context_line":"        # We should have two new pGPU child providers in the tree under the"},{"line_number":18185,"context_line":"        # compute node root provider."},{"line_number":18186,"context_line":"        compute_node_tree_uuids \u003d self.pt.get_provider_uuids("},{"line_number":18187,"context_line":"            self.cn_rp[\u0027name\u0027])"},{"line_number":18188,"context_line":"        self.assertEqual(3, len(compute_node_tree_uuids))"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_a3eb5b76","line":18185,"range":{"start_line":18184,"start_character":8,"end_line":18185,"end_character":37},"in_reply_to":"9fdfeff1_782a1030","updated":"2019-03-04 14:05:37.000000000","message":"Like Eric said, we agreed on using pGPU \u003d\u003d child RP here since we would need a new reshape once we have multiple types. That\u0027s why since PS5 we have this.","commit_id":"054eb3a652f340d819f1b04567d0ed5e207ed733"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b30ba66a71938fdf903ed49063ac16133a2f6379","unresolved":false,"context_lines":[{"line_number":18181,"context_line":"        # root compute node provider inventory is unchanged"},{"line_number":18182,"context_line":"        self.assertEqual(inventory,"},{"line_number":18183,"context_line":"                         (self.pt.data(self.cn_rp[\u0027uuid\u0027])).inventory)"},{"line_number":18184,"context_line":"        # We should have two new pGPU child providers in the tree under the"},{"line_number":18185,"context_line":"        # compute node root provider."},{"line_number":18186,"context_line":"        compute_node_tree_uuids \u003d self.pt.get_provider_uuids("},{"line_number":18187,"context_line":"            self.cn_rp[\u0027name\u0027])"},{"line_number":18188,"context_line":"        self.assertEqual(3, len(compute_node_tree_uuids))"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_782a1030","line":18185,"range":{"start_line":18184,"start_character":8,"end_line":18185,"end_character":37},"in_reply_to":"9fdfeff1_8961adab","updated":"2019-03-02 08:21:56.000000000","message":"Perhaps we could artificially combine them today, but soon we will will support multiple types, divided along pgpu lines. And also affinity as you say. So consolidating now would require another reshape later, whereas keeping them separated now is (harmless and) seeing us up for the future.\n\nSylvain and others should feel free to chime in here :P","commit_id":"054eb3a652f340d819f1b04567d0ed5e207ed733"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":25638,"name":"Naichuan Sun","email":"naichuan.sun@citrix.com","username":"naichuans"},"change_message_id":"bb2d34b28324f11076483c23c1977cdb89d1f3ee","unresolved":false,"context_lines":[{"line_number":6558,"context_line":"            raise exception.ReshapeNeeded()"},{"line_number":6559,"context_line":""},{"line_number":6560,"context_line":"        # Create the VGPU child provider if it does not already exist."},{"line_number":6561,"context_line":"        # TODO(mriedem): What are other virt drivers like the xenapi driver"},{"line_number":6562,"context_line":"        # doing with respect to naming child providers? Is there some"},{"line_number":6563,"context_line":"        # convention we should follow?"},{"line_number":6564,"context_line":"        vgpu_rp_name \u003d \u0027%s_VGPU\u0027 % nodename"},{"line_number":6565,"context_line":"        if not provider_tree.exists(vgpu_rp_name):"},{"line_number":6566,"context_line":"            # This is the first time creating the child provider so add"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_18e02597","line":6563,"range":{"start_line":6561,"start_character":0,"end_line":6563,"end_character":38},"updated":"2018-09-04 07:42:30.000000000","message":"Hi, Matt. In order to support multiple PGPUG, we add uuid of PGPUG to a rp name.\n\nGPU_RP_PREFIX \u003d \"GPU_\"\nrp_name \u003d (GPU_RP_PREFIX + grp_id)","commit_id":"c27c9bb39d86600de49014ecf1dfccbb185b75c8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"75624ee687a485f9908a4a3305a80e58db79032e","unresolved":false,"context_lines":[{"line_number":6561,"context_line":"        # TODO(mriedem): What are other virt drivers like the xenapi driver"},{"line_number":6562,"context_line":"        # doing with respect to naming child providers? Is there some"},{"line_number":6563,"context_line":"        # convention we should follow?"},{"line_number":6564,"context_line":"        vgpu_rp_name \u003d \u0027%s_VGPU\u0027 % nodename"},{"line_number":6565,"context_line":"        if not provider_tree.exists(vgpu_rp_name):"},{"line_number":6566,"context_line":"            # This is the first time creating the child provider so add"},{"line_number":6567,"context_line":"            # it to the tree under the root node provider."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_cd736a4d","line":6564,"range":{"start_line":6564,"start_character":23,"end_line":6564,"end_character":43},"updated":"2018-09-07 23:36:17.000000000","message":"If we name this generically, we\u0027ll have to figure out how to rename it later when we have one per type. That\u0027s something we can\u0027t do today without really awkward machinations. So I suggest we figure out what that naming convention is going to look like now and come right out of the gate using it here.","commit_id":"c27c9bb39d86600de49014ecf1dfccbb185b75c8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"d983272601751d33778529ce46de3367ad626218","unresolved":false,"context_lines":[{"line_number":6561,"context_line":"        # TODO(mriedem): What are other virt drivers like the xenapi driver"},{"line_number":6562,"context_line":"        # doing with respect to naming child providers? Is there some"},{"line_number":6563,"context_line":"        # convention we should follow?"},{"line_number":6564,"context_line":"        vgpu_rp_name \u003d \u0027%s_VGPU\u0027 % nodename"},{"line_number":6565,"context_line":"        if not provider_tree.exists(vgpu_rp_name):"},{"line_number":6566,"context_line":"            # This is the first time creating the child provider so add"},{"line_number":6567,"context_line":"            # it to the tree under the root node provider."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_4da95d7a","line":6564,"range":{"start_line":6564,"start_character":23,"end_line":6564,"end_character":43},"in_reply_to":"3f79a3b5_0c463622","updated":"2018-09-11 20:35:17.000000000","message":"FWIW, the spec we approved last cycle wasn\u0027t mentioning the naming convention we should follow, my bad. \nhttps://specs.openstack.org/openstack/nova-specs/specs/rocky/approved/vgpu-rocky.html\n\nI know we\u0027re not forced to follow the same naming convention between virt drivers, but I\u0027d consider worth it to make sure that both Xen and libvirt do the same, hence my proposal would be \"GPU_%s\" where %s being the GPU type name (we already know it and I can help on the implementation).","commit_id":"c27c9bb39d86600de49014ecf1dfccbb185b75c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b61ec7b0faa4cc66fd17a3c32f13f8d74b50a20a","unresolved":false,"context_lines":[{"line_number":6561,"context_line":"        # TODO(mriedem): What are other virt drivers like the xenapi driver"},{"line_number":6562,"context_line":"        # doing with respect to naming child providers? Is there some"},{"line_number":6563,"context_line":"        # convention we should follow?"},{"line_number":6564,"context_line":"        vgpu_rp_name \u003d \u0027%s_VGPU\u0027 % nodename"},{"line_number":6565,"context_line":"        if not provider_tree.exists(vgpu_rp_name):"},{"line_number":6566,"context_line":"            # This is the first time creating the child provider so add"},{"line_number":6567,"context_line":"            # it to the tree under the root node provider."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_3f808ebf","line":6564,"range":{"start_line":6564,"start_character":23,"end_line":6564,"end_character":43},"in_reply_to":"3f79a3b5_4da95d7a","updated":"2018-09-13 22:20:11.000000000","message":"It has to also have the root provider (compute node) name in it as a namespace because resource provider names must be globally unique (there is a unique constraint on RP.name in the DB).","commit_id":"c27c9bb39d86600de49014ecf1dfccbb185b75c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d95719a0aab2051b56c9fc6fa8909dbf3fecef07","unresolved":false,"context_lines":[{"line_number":6561,"context_line":"        # TODO(mriedem): What are other virt drivers like the xenapi driver"},{"line_number":6562,"context_line":"        # doing with respect to naming child providers? Is there some"},{"line_number":6563,"context_line":"        # convention we should follow?"},{"line_number":6564,"context_line":"        vgpu_rp_name \u003d \u0027%s_VGPU\u0027 % nodename"},{"line_number":6565,"context_line":"        if not provider_tree.exists(vgpu_rp_name):"},{"line_number":6566,"context_line":"            # This is the first time creating the child provider so add"},{"line_number":6567,"context_line":"            # it to the tree under the root node provider."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_0c463622","line":6564,"range":{"start_line":6564,"start_character":23,"end_line":6564,"end_character":43},"in_reply_to":"3f79a3b5_cd736a4d","updated":"2018-09-09 16:29:24.000000000","message":"Yeah I was worried we\u0027d need another reshape once we started supporting multiples.","commit_id":"c27c9bb39d86600de49014ecf1dfccbb185b75c8"},{"author":{"_account_id":25638,"name":"Naichuan Sun","email":"naichuan.sun@citrix.com","username":"naichuans"},"change_message_id":"ab40de7ea9acc4a8ecc858007f505816389403af","unresolved":false,"context_lines":[{"line_number":6561,"context_line":"        # TODO(mriedem): What are other virt drivers like the xenapi driver"},{"line_number":6562,"context_line":"        # doing with respect to naming child providers? Is there some"},{"line_number":6563,"context_line":"        # convention we should follow?"},{"line_number":6564,"context_line":"        vgpu_rp_name \u003d \u0027%s_VGPU\u0027 % nodename"},{"line_number":6565,"context_line":"        if not provider_tree.exists(vgpu_rp_name):"},{"line_number":6566,"context_line":"            # This is the first time creating the child provider so add"},{"line_number":6567,"context_line":"            # it to the tree under the root node provider."},{"line_number":6568,"context_line":"            provider_tree.new_child(vgpu_rp_name, nodename)"},{"line_number":6569,"context_line":"        vgpu_rp \u003d provider_tree.data(vgpu_rp_name)"},{"line_number":6570,"context_line":""},{"line_number":6571,"context_line":"        if allocations is not None:"},{"line_number":6572,"context_line":"            LOG.info(\u0027Reshaping tree; moving VGPU allocations from root \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_7824b90d","line":6569,"range":{"start_line":6564,"start_character":0,"end_line":6569,"end_character":50},"updated":"2018-09-04 07:36:57.000000000","message":"Hi, Matt, libvirt only support one vgpu rp? If there are multiple PGPUs on multiple compute nodes with different type, could we deal with them?","commit_id":"c27c9bb39d86600de49014ecf1dfccbb185b75c8"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"7ddc1fef9c310653a38675e272927c0610ab70a3","unresolved":false,"context_lines":[{"line_number":6561,"context_line":"        # TODO(mriedem): What are other virt drivers like the xenapi driver"},{"line_number":6562,"context_line":"        # doing with respect to naming child providers? Is there some"},{"line_number":6563,"context_line":"        # convention we should follow?"},{"line_number":6564,"context_line":"        vgpu_rp_name \u003d \u0027%s_VGPU\u0027 % nodename"},{"line_number":6565,"context_line":"        if not provider_tree.exists(vgpu_rp_name):"},{"line_number":6566,"context_line":"            # This is the first time creating the child provider so add"},{"line_number":6567,"context_line":"            # it to the tree under the root node provider."},{"line_number":6568,"context_line":"            provider_tree.new_child(vgpu_rp_name, nodename)"},{"line_number":6569,"context_line":"        vgpu_rp \u003d provider_tree.data(vgpu_rp_name)"},{"line_number":6570,"context_line":""},{"line_number":6571,"context_line":"        if allocations is not None:"},{"line_number":6572,"context_line":"            LOG.info(\u0027Reshaping tree; moving VGPU allocations from root \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_7ae6e618","line":6569,"range":{"start_line":6564,"start_character":0,"end_line":6569,"end_character":50},"in_reply_to":"3f79a3b5_33f6625f","updated":"2018-09-04 14:18:43.000000000","message":"When I started to write the implementation, I created one child Resource Provider *per GPU type*.\n\nSee https://review.openstack.org/#/c/564215/7/nova/virt/libvirt/driver.py@5840\n\nGiven we only support one type for the moment, I\u0027m fine with creating a child RP now and then create the new children RPs (each for a new type) once the operator provides more than one type. That wouldn\u0027t be an upgrade issue then, because the existing type would already be in a child RP.","commit_id":"c27c9bb39d86600de49014ecf1dfccbb185b75c8"},{"author":{"_account_id":25638,"name":"Naichuan Sun","email":"naichuan.sun@citrix.com","username":"naichuans"},"change_message_id":"ba6224bf727ca3c388633284cd99f913174a623d","unresolved":false,"context_lines":[{"line_number":6561,"context_line":"        # TODO(mriedem): What are other virt drivers like the xenapi driver"},{"line_number":6562,"context_line":"        # doing with respect to naming child providers? Is there some"},{"line_number":6563,"context_line":"        # convention we should follow?"},{"line_number":6564,"context_line":"        vgpu_rp_name \u003d \u0027%s_VGPU\u0027 % nodename"},{"line_number":6565,"context_line":"        if not provider_tree.exists(vgpu_rp_name):"},{"line_number":6566,"context_line":"            # This is the first time creating the child provider so add"},{"line_number":6567,"context_line":"            # it to the tree under the root node provider."},{"line_number":6568,"context_line":"            provider_tree.new_child(vgpu_rp_name, nodename)"},{"line_number":6569,"context_line":"        vgpu_rp \u003d provider_tree.data(vgpu_rp_name)"},{"line_number":6570,"context_line":""},{"line_number":6571,"context_line":"        if allocations is not None:"},{"line_number":6572,"context_line":"            LOG.info(\u0027Reshaping tree; moving VGPU allocations from root \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_c7f63390","line":6569,"range":{"start_line":6564,"start_character":0,"end_line":6569,"end_character":50},"in_reply_to":"3f79a3b5_6543a240","updated":"2018-09-05 01:29:55.000000000","message":"Hi, Matt and Sylvain. The previous vgpu codes base on multiple vgpu rp support. Refer to reshaper, there would be a problem: only check allocation argument in update_provider_tree, we can\u0027t decide which gpu own the allocation. It is troublesome to match the allocation with hypervisor side.\nSo I think support only on vgpu rp is OK, which get ride of many troubles.","commit_id":"c27c9bb39d86600de49014ecf1dfccbb185b75c8"},{"author":{"_account_id":25638,"name":"Naichuan Sun","email":"naichuan.sun@citrix.com","username":"naichuans"},"change_message_id":"3a9e9b4ed41c6b563f8829962ecd0c6e2334ea8b","unresolved":false,"context_lines":[{"line_number":6561,"context_line":"        # TODO(mriedem): What are other virt drivers like the xenapi driver"},{"line_number":6562,"context_line":"        # doing with respect to naming child providers? Is there some"},{"line_number":6563,"context_line":"        # convention we should follow?"},{"line_number":6564,"context_line":"        vgpu_rp_name \u003d \u0027%s_VGPU\u0027 % nodename"},{"line_number":6565,"context_line":"        if not provider_tree.exists(vgpu_rp_name):"},{"line_number":6566,"context_line":"            # This is the first time creating the child provider so add"},{"line_number":6567,"context_line":"            # it to the tree under the root node provider."},{"line_number":6568,"context_line":"            provider_tree.new_child(vgpu_rp_name, nodename)"},{"line_number":6569,"context_line":"        vgpu_rp \u003d provider_tree.data(vgpu_rp_name)"},{"line_number":6570,"context_line":""},{"line_number":6571,"context_line":"        if allocations is not None:"},{"line_number":6572,"context_line":"            LOG.info(\u0027Reshaping tree; moving VGPU allocations from root \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_33f6625f","line":6569,"range":{"start_line":6564,"start_character":0,"end_line":6569,"end_character":50},"in_reply_to":"3f79a3b5_7824b90d","updated":"2018-09-04 08:34:06.000000000","message":"Sorry, just for multiple GPUs in on compute node. According to xenapi previous design, we create one rp for one GPU.","commit_id":"c27c9bb39d86600de49014ecf1dfccbb185b75c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7bd85961fa18d0467d8f4ebe159842282e1c0cf2","unresolved":false,"context_lines":[{"line_number":6561,"context_line":"        # TODO(mriedem): What are other virt drivers like the xenapi driver"},{"line_number":6562,"context_line":"        # doing with respect to naming child providers? Is there some"},{"line_number":6563,"context_line":"        # convention we should follow?"},{"line_number":6564,"context_line":"        vgpu_rp_name \u003d \u0027%s_VGPU\u0027 % nodename"},{"line_number":6565,"context_line":"        if not provider_tree.exists(vgpu_rp_name):"},{"line_number":6566,"context_line":"            # This is the first time creating the child provider so add"},{"line_number":6567,"context_line":"            # it to the tree under the root node provider."},{"line_number":6568,"context_line":"            provider_tree.new_child(vgpu_rp_name, nodename)"},{"line_number":6569,"context_line":"        vgpu_rp \u003d provider_tree.data(vgpu_rp_name)"},{"line_number":6570,"context_line":""},{"line_number":6571,"context_line":"        if allocations is not None:"},{"line_number":6572,"context_line":"            LOG.info(\u0027Reshaping tree; moving VGPU allocations from root \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_6543a240","line":6569,"range":{"start_line":6564,"start_character":0,"end_line":6569,"end_character":50},"in_reply_to":"3f79a3b5_7ae6e618","updated":"2018-09-04 18:37:33.000000000","message":"Sorry I haven\u0027t followed all of those other changes. But this was something I was wondering about because Dan always referred to needing to create multiple child providers because there would be one per host numa node, so this would be a migration that hits all libvirt compute hosts.\n\nBut what I\u0027m not sure about is how much of blueprint vgpu-rocky stuff needs to be co-mingled with the actual reshape routine we need for the upgrade. In other words, we don\u0027t really want to introduce new feature support during the upgrade/reshape, right? The intent is to upgrade and reshape an existing VGPU resource inventory from the root provider and put it on a child provider. We can\u0027t have multiple VGPU types in Rocky, right? That was a limitation which was going to be addressed via blueprint vgpu-rocky as Sylvain pointed out.\n\nSo how much needs to be done to upgrade existing VGPU inventory/allocations *from* queens/rocky to make it work in stein and then we can add in the multiple child providers support?","commit_id":"c27c9bb39d86600de49014ecf1dfccbb185b75c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d3a165c1633559e0806aa84141ee2e9092342f46","unresolved":false,"context_lines":[{"line_number":6561,"context_line":"        # TODO(mriedem): What are other virt drivers like the xenapi driver"},{"line_number":6562,"context_line":"        # doing with respect to naming child providers? Is there some"},{"line_number":6563,"context_line":"        # convention we should follow?"},{"line_number":6564,"context_line":"        vgpu_rp_name \u003d \u0027%s_VGPU\u0027 % nodename"},{"line_number":6565,"context_line":"        if not provider_tree.exists(vgpu_rp_name):"},{"line_number":6566,"context_line":"            # This is the first time creating the child provider so add"},{"line_number":6567,"context_line":"            # it to the tree under the root node provider."},{"line_number":6568,"context_line":"            provider_tree.new_child(vgpu_rp_name, nodename)"},{"line_number":6569,"context_line":"        vgpu_rp \u003d provider_tree.data(vgpu_rp_name)"},{"line_number":6570,"context_line":""},{"line_number":6571,"context_line":"        if allocations is not None:"},{"line_number":6572,"context_line":"            LOG.info(\u0027Reshaping tree; moving VGPU allocations from root \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_05cc1932","line":6569,"range":{"start_line":6564,"start_character":0,"end_line":6569,"end_character":50},"in_reply_to":"3f79a3b5_c7f63390","updated":"2018-09-05 14:11:30.000000000","message":"Given how close we are to the PTG where this can be discussed as a group, I suggest we hold off on approving this change until we\u0027re sure it is the right thing to do and doesn\u0027t restrict us from adding more vgpu child providers later.","commit_id":"c27c9bb39d86600de49014ecf1dfccbb185b75c8"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"75624ee687a485f9908a4a3305a80e58db79032e","unresolved":false,"context_lines":[{"line_number":6596,"context_line":"                                      \u0027consumer_uuid\u0027: consumer_uuid,"},{"line_number":6597,"context_line":"                                      \u0027alloc_data\u0027: alloc_data,"},{"line_number":6598,"context_line":"                                      \u0027root_uuid\u0027: root_node.uuid})"},{"line_number":6599,"context_line":"                            raise exception.ReshapeFailed(error\u003dmsg)"},{"line_number":6600,"context_line":"                        # Add the VGPU allocation to the VGPU provider."},{"line_number":6601,"context_line":"                        alloc_data[\u0027allocations\u0027][vgpu_rp.uuid] \u003d {"},{"line_number":6602,"context_line":"                            \u0027resources\u0027: {"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_cdc5aa5d","line":6599,"range":{"start_line":6599,"start_character":44,"end_line":6599,"end_character":57},"updated":"2018-09-07 23:36:17.000000000","message":"This isn\u0027t the intent (per design) of ReshapeFailed - it is only supposed to signal a failure of the POST /reshaper call in update_from_provider_tree.\n\nThat said, with the code as currently written in the resource tracker, this will have a sane effect, so perhaps we want to reconsider that design point.\n\nIf we do decide to go that way, the update_provider_tree docstring needs to mention that it can raise ReshapeFailed. Or we may want to use a different exception so the log (waaay at the top of the resource tracker stack) will distinguish between \"POST /reshaper blew up\" and \"We couldn\u0027t even do the work to get that far\".","commit_id":"c27c9bb39d86600de49014ecf1dfccbb185b75c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d95719a0aab2051b56c9fc6fa8909dbf3fecef07","unresolved":false,"context_lines":[{"line_number":6596,"context_line":"                                      \u0027consumer_uuid\u0027: consumer_uuid,"},{"line_number":6597,"context_line":"                                      \u0027alloc_data\u0027: alloc_data,"},{"line_number":6598,"context_line":"                                      \u0027root_uuid\u0027: root_node.uuid})"},{"line_number":6599,"context_line":"                            raise exception.ReshapeFailed(error\u003dmsg)"},{"line_number":6600,"context_line":"                        # Add the VGPU allocation to the VGPU provider."},{"line_number":6601,"context_line":"                        alloc_data[\u0027allocations\u0027][vgpu_rp.uuid] \u003d {"},{"line_number":6602,"context_line":"                            \u0027resources\u0027: {"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_4c598efa","line":6599,"range":{"start_line":6599,"start_character":44,"end_line":6599,"end_character":57},"in_reply_to":"3f79a3b5_cdc5aa5d","updated":"2018-09-09 16:29:24.000000000","message":"\u003e This isn\u0027t the intent (per design) of ReshapeFailed - it is only supposed to signal a failure of the POST /reshaper call in update_from_provider_tree.\n\nYup I saw that\u0027s the only place it was docstring-ed (which normally isn\u0027t a good indicator of where an exception can/should be used, but I know you in particular care about this). If we restrict it to just that operation, we likely need a comment on the exception itself.\n\n\u003e That said, with the code as currently written in the resource tracker, this will have a sane effect, so perhaps we want to reconsider that design point.\n\nYup that\u0027s why I figured usage here was OK.\n\n\u003e If we do decide to go that way, the update_provider_tree docstring needs to mention that it can raise ReshapeFailed.\n\nYeah. Doesn\u0027t matter a ton to me, but I re-used it here because we\u0027re in the process of reshaping, so ultimately whether we failed here or before actually making POST /reshape call, I\u0027m not sure it really matters. If we added something new, I\u0027m not sure what we\u0027d call it - ReshapeSetupFailed? PreReshapeFailure?","commit_id":"c27c9bb39d86600de49014ecf1dfccbb185b75c8"},{"author":{"_account_id":25638,"name":"Naichuan Sun","email":"naichuan.sun@citrix.com","username":"naichuans"},"change_message_id":"ab40de7ea9acc4a8ecc858007f505816389403af","unresolved":false,"context_lines":[{"line_number":6598,"context_line":"                                      \u0027root_uuid\u0027: root_node.uuid})"},{"line_number":6599,"context_line":"                            raise exception.ReshapeFailed(error\u003dmsg)"},{"line_number":6600,"context_line":"                        # Add the VGPU allocation to the VGPU provider."},{"line_number":6601,"context_line":"                        alloc_data[\u0027allocations\u0027][vgpu_rp.uuid] \u003d {"},{"line_number":6602,"context_line":"                            \u0027resources\u0027: {"},{"line_number":6603,"context_line":"                                rc_fields.ResourceClass.VGPU:"},{"line_number":6604,"context_line":"                                    resources[rc_fields.ResourceClass.VGPU]"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_58f9bd7d","line":6601,"range":{"start_line":6601,"start_character":24,"end_line":6601,"end_character":63},"updated":"2018-09-04 07:36:57.000000000","message":"Looks duplicate with line 6580. Maybe better to use: allocations[vgpu_rp.uuid]?","commit_id":"c27c9bb39d86600de49014ecf1dfccbb185b75c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d95719a0aab2051b56c9fc6fa8909dbf3fecef07","unresolved":false,"context_lines":[{"line_number":6598,"context_line":"                                      \u0027root_uuid\u0027: root_node.uuid})"},{"line_number":6599,"context_line":"                            raise exception.ReshapeFailed(error\u003dmsg)"},{"line_number":6600,"context_line":"                        # Add the VGPU allocation to the VGPU provider."},{"line_number":6601,"context_line":"                        alloc_data[\u0027allocations\u0027][vgpu_rp.uuid] \u003d {"},{"line_number":6602,"context_line":"                            \u0027resources\u0027: {"},{"line_number":6603,"context_line":"                                rc_fields.ResourceClass.VGPU:"},{"line_number":6604,"context_line":"                                    resources[rc_fields.ResourceClass.VGPU]"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_ec73ba7e","line":6601,"range":{"start_line":6601,"start_character":24,"end_line":6601,"end_character":63},"in_reply_to":"3f79a3b5_58f9bd7d","updated":"2018-09-09 16:29:24.000000000","message":"Sure. I think this was written before I had to change how the earlier code around L6580 was changed to handle some py3 support.","commit_id":"c27c9bb39d86600de49014ecf1dfccbb185b75c8"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ceab0d1f772df6fd91ccad730b10fdb4517bcf45","unresolved":false,"context_lines":[{"line_number":6598,"context_line":"                                      \u0027consumer_uuid\u0027: consumer_uuid,"},{"line_number":6599,"context_line":"                                      \u0027alloc_data\u0027: alloc_data,"},{"line_number":6600,"context_line":"                                      \u0027root_uuid\u0027: root_node.uuid})"},{"line_number":6601,"context_line":"                            raise exception.ReshapeFailed(error\u003dmsg)"},{"line_number":6602,"context_line":"                        # Add the VGPU allocation to the VGPU provider."},{"line_number":6603,"context_line":"                        alloc_data[\u0027allocations\u0027][vgpu_rp.uuid] \u003d {"},{"line_number":6604,"context_line":"                            \u0027resources\u0027: {"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_3c88676f","line":6601,"range":{"start_line":6601,"start_character":44,"end_line":6601,"end_character":57},"updated":"2018-09-17 15:55:39.000000000","message":"Forgot to address Eric\u0027s comment here.","commit_id":"ff0c440c5d131f0a27752fd6e5aa4d6ba0d7b464"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ceab0d1f772df6fd91ccad730b10fdb4517bcf45","unresolved":false,"context_lines":[{"line_number":6600,"context_line":"                                      \u0027root_uuid\u0027: root_node.uuid})"},{"line_number":6601,"context_line":"                            raise exception.ReshapeFailed(error\u003dmsg)"},{"line_number":6602,"context_line":"                        # Add the VGPU allocation to the VGPU provider."},{"line_number":6603,"context_line":"                        alloc_data[\u0027allocations\u0027][vgpu_rp.uuid] \u003d {"},{"line_number":6604,"context_line":"                            \u0027resources\u0027: {"},{"line_number":6605,"context_line":"                                rc_fields.ResourceClass.VGPU:"},{"line_number":6606,"context_line":"                                    resources[rc_fields.ResourceClass.VGPU]"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f79a3b5_1c832b58","line":6603,"updated":"2018-09-17 15:55:39.000000000","message":"Forgot to address Naichuan\u0027s comment here.","commit_id":"ff0c440c5d131f0a27752fd6e5aa4d6ba0d7b464"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b515399b904e4d824c01df6e33e9f183555dffb3","unresolved":false,"context_lines":[{"line_number":5765,"context_line":"        requested_types \u003d CONF.devices.enabled_vgpu_types[:1]"},{"line_number":5766,"context_line":"        return requested_types"},{"line_number":5767,"context_line":""},{"line_number":5768,"context_line":"    # TODO(sbauza): Remove in a later patch."},{"line_number":5769,"context_line":"    def _get_vgpu_total(self):"},{"line_number":5770,"context_line":"        \"\"\"Returns the number of total available vGPUs for any GPU type that is"},{"line_number":5771,"context_line":"        enabled with the enabled_vgpu_types CONF option."}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_c185430c","line":5768,"updated":"2018-09-24 19:00:05.000000000","message":"\"a later patch\" isn\u0027t particularly helpful. How about describing the condition of removal?","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"722b4071e47400627cc4c7e8b6a7a4855bda761d","unresolved":false,"context_lines":[{"line_number":5765,"context_line":"        requested_types \u003d CONF.devices.enabled_vgpu_types[:1]"},{"line_number":5766,"context_line":"        return requested_types"},{"line_number":5767,"context_line":""},{"line_number":5768,"context_line":"    # TODO(sbauza): Remove in a later patch."},{"line_number":5769,"context_line":"    def _get_vgpu_total(self):"},{"line_number":5770,"context_line":"        \"\"\"Returns the number of total available vGPUs for any GPU type that is"},{"line_number":5771,"context_line":"        enabled with the enabled_vgpu_types CONF option."}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_188dde7b","line":5768,"in_reply_to":"3f79a3b5_c185430c","updated":"2018-09-26 10:23:25.000000000","message":"Done","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"bbce058085e300b78777231dd2fbc430d9a78523","unresolved":false,"context_lines":[{"line_number":5794,"context_line":"        The returned dict is keyed by the vGPU type name and the values"},{"line_number":5795,"context_line":"        are an inventory dict."},{"line_number":5796,"context_line":""},{"line_number":5797,"context_line":"        :returns: dict of dict"},{"line_number":5798,"context_line":"        \"\"\""},{"line_number":5799,"context_line":""},{"line_number":5800,"context_line":"        # Bail out early if operator doesn\u0027t care about providing vGPUs"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_f7eb5dda","line":5797,"range":{"start_line":5797,"start_character":18,"end_line":5797,"end_character":30},"updated":"2018-09-24 19:01:20.000000000","message":"See L6561","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"722b4071e47400627cc4c7e8b6a7a4855bda761d","unresolved":false,"context_lines":[{"line_number":5794,"context_line":"        The returned dict is keyed by the vGPU type name and the values"},{"line_number":5795,"context_line":"        are an inventory dict."},{"line_number":5796,"context_line":""},{"line_number":5797,"context_line":"        :returns: dict of dict"},{"line_number":5798,"context_line":"        \"\"\""},{"line_number":5799,"context_line":""},{"line_number":5800,"context_line":"        # Bail out early if operator doesn\u0027t care about providing vGPUs"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_9113ee65","line":5797,"range":{"start_line":5797,"start_character":18,"end_line":5797,"end_character":30},"in_reply_to":"3f79a3b5_f7eb5dda","updated":"2018-09-26 10:23:25.000000000","message":"Done","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b515399b904e4d824c01df6e33e9f183555dffb3","unresolved":false,"context_lines":[{"line_number":5802,"context_line":"            return {}"},{"line_number":5803,"context_line":"        requested_types \u003d CONF.devices.enabled_vgpu_types"},{"line_number":5804,"context_line":"        inventories \u003d {}"},{"line_number":5805,"context_line":"        # Count the already created (but possibly not assigned to a guest)"},{"line_number":5806,"context_line":"        # mdevs for all the supported types"},{"line_number":5807,"context_line":"        mediated_devices \u003d self._get_mediated_devices(requested_types)"},{"line_number":5808,"context_line":"        for mdev in mediated_devices:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_1cdf2067","line":5805,"range":{"start_line":5805,"start_character":20,"end_line":5805,"end_character":35},"updated":"2018-09-24 19:00:05.000000000","message":"Might be worth noting here that by \"already created\" you are referring to the physical GPU Linux driver \"creating\" the mdev device records in sysfs. They aren\u0027t \"creating\" anything in reality, just a pointer to the vGPU virtual device context that may be passed to a guest, right?","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"722b4071e47400627cc4c7e8b6a7a4855bda761d","unresolved":false,"context_lines":[{"line_number":5802,"context_line":"            return {}"},{"line_number":5803,"context_line":"        requested_types \u003d CONF.devices.enabled_vgpu_types"},{"line_number":5804,"context_line":"        inventories \u003d {}"},{"line_number":5805,"context_line":"        # Count the already created (but possibly not assigned to a guest)"},{"line_number":5806,"context_line":"        # mdevs for all the supported types"},{"line_number":5807,"context_line":"        mediated_devices \u003d self._get_mediated_devices(requested_types)"},{"line_number":5808,"context_line":"        for mdev in mediated_devices:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_78909297","line":5805,"range":{"start_line":5805,"start_character":20,"end_line":5805,"end_character":35},"in_reply_to":"3f79a3b5_1cdf2067","updated":"2018-09-26 10:23:25.000000000","message":"Done","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b515399b904e4d824c01df6e33e9f183555dffb3","unresolved":false,"context_lines":[{"line_number":5813,"context_line":"        # types."},{"line_number":5814,"context_line":"        mdev_capable_devices \u003d self._get_mdev_capable_devices(requested_types)"},{"line_number":5815,"context_line":"        for dev in mdev_capable_devices:"},{"line_number":5816,"context_line":"            dev_name \u003d dev[\u0027dev_id\u0027]"},{"line_number":5817,"context_line":"            # NOTE(sbauza): Until we support multiple types, we only loop once"},{"line_number":5818,"context_line":"            for _type in dev[\u0027types\u0027]:"},{"line_number":5819,"context_line":"                left_vgpus \u003d dev[\u0027types\u0027][_type][\u0027availableInstances\u0027]"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_1c8ac063","line":5816,"updated":"2018-09-24 19:00:05.000000000","message":"Might be worth mentioning here that dev_id \u003d\u003d the PCI device address of the physical GPU...","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"722b4071e47400627cc4c7e8b6a7a4855bda761d","unresolved":false,"context_lines":[{"line_number":5813,"context_line":"        # types."},{"line_number":5814,"context_line":"        mdev_capable_devices \u003d self._get_mdev_capable_devices(requested_types)"},{"line_number":5815,"context_line":"        for dev in mdev_capable_devices:"},{"line_number":5816,"context_line":"            dev_name \u003d dev[\u0027dev_id\u0027]"},{"line_number":5817,"context_line":"            # NOTE(sbauza): Until we support multiple types, we only loop once"},{"line_number":5818,"context_line":"            for _type in dev[\u0027types\u0027]:"},{"line_number":5819,"context_line":"                left_vgpus \u003d dev[\u0027types\u0027][_type][\u0027availableInstances\u0027]"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_589356a5","line":5816,"in_reply_to":"3f79a3b5_1c8ac063","updated":"2018-09-26 10:23:25.000000000","message":"Done","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b515399b904e4d824c01df6e33e9f183555dffb3","unresolved":false,"context_lines":[{"line_number":5814,"context_line":"        mdev_capable_devices \u003d self._get_mdev_capable_devices(requested_types)"},{"line_number":5815,"context_line":"        for dev in mdev_capable_devices:"},{"line_number":5816,"context_line":"            dev_name \u003d dev[\u0027dev_id\u0027]"},{"line_number":5817,"context_line":"            # NOTE(sbauza): Until we support multiple types, we only loop once"},{"line_number":5818,"context_line":"            for _type in dev[\u0027types\u0027]:"},{"line_number":5819,"context_line":"                left_vgpus \u003d dev[\u0027types\u0027][_type][\u0027availableInstances\u0027]"},{"line_number":5820,"context_line":"                # TODO(sbauza): Once we support multiple typles, check which"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_1cfba0be","line":5817,"range":{"start_line":5817,"start_character":61,"end_line":5817,"end_character":78},"updated":"2018-09-24 19:00:05.000000000","message":"I don\u0027t see where you are looping \"only once\"?","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"722b4071e47400627cc4c7e8b6a7a4855bda761d","unresolved":false,"context_lines":[{"line_number":5814,"context_line":"        mdev_capable_devices \u003d self._get_mdev_capable_devices(requested_types)"},{"line_number":5815,"context_line":"        for dev in mdev_capable_devices:"},{"line_number":5816,"context_line":"            dev_name \u003d dev[\u0027dev_id\u0027]"},{"line_number":5817,"context_line":"            # NOTE(sbauza): Until we support multiple types, we only loop once"},{"line_number":5818,"context_line":"            for _type in dev[\u0027types\u0027]:"},{"line_number":5819,"context_line":"                left_vgpus \u003d dev[\u0027types\u0027][_type][\u0027availableInstances\u0027]"},{"line_number":5820,"context_line":"                # TODO(sbauza): Once we support multiple typles, check which"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_9879ee83","line":5817,"range":{"start_line":5817,"start_character":61,"end_line":5817,"end_character":78},"in_reply_to":"3f79a3b5_1cfba0be","updated":"2018-09-26 10:23:25.000000000","message":"Done","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b515399b904e4d824c01df6e33e9f183555dffb3","unresolved":false,"context_lines":[{"line_number":5817,"context_line":"            # NOTE(sbauza): Until we support multiple types, we only loop once"},{"line_number":5818,"context_line":"            for _type in dev[\u0027types\u0027]:"},{"line_number":5819,"context_line":"                left_vgpus \u003d dev[\u0027types\u0027][_type][\u0027availableInstances\u0027]"},{"line_number":5820,"context_line":"                # TODO(sbauza): Once we support multiple typles, check which"},{"line_number":5821,"context_line":"                # PCI devices are set for this type"},{"line_number":5822,"context_line":"                # NOTE(sbauza): a PCI device will always support only one type"},{"line_number":5823,"context_line":"                inv_per_parent \u003d inventories.setdefault(dev_name, {\u0027total\u0027: 0})"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_7c797480","line":5820,"range":{"start_line":5820,"start_character":57,"end_line":5820,"end_character":63},"updated":"2018-09-24 19:00:05.000000000","message":"types","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"722b4071e47400627cc4c7e8b6a7a4855bda761d","unresolved":false,"context_lines":[{"line_number":5817,"context_line":"            # NOTE(sbauza): Until we support multiple types, we only loop once"},{"line_number":5818,"context_line":"            for _type in dev[\u0027types\u0027]:"},{"line_number":5819,"context_line":"                left_vgpus \u003d dev[\u0027types\u0027][_type][\u0027availableInstances\u0027]"},{"line_number":5820,"context_line":"                # TODO(sbauza): Once we support multiple typles, check which"},{"line_number":5821,"context_line":"                # PCI devices are set for this type"},{"line_number":5822,"context_line":"                # NOTE(sbauza): a PCI device will always support only one type"},{"line_number":5823,"context_line":"                inv_per_parent \u003d inventories.setdefault(dev_name, {\u0027total\u0027: 0})"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_d87f6667","line":5820,"range":{"start_line":5820,"start_character":57,"end_line":5820,"end_character":63},"in_reply_to":"3f79a3b5_7c797480","updated":"2018-09-26 10:23:25.000000000","message":"Done","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b515399b904e4d824c01df6e33e9f183555dffb3","unresolved":false,"context_lines":[{"line_number":5819,"context_line":"                left_vgpus \u003d dev[\u0027types\u0027][_type][\u0027availableInstances\u0027]"},{"line_number":5820,"context_line":"                # TODO(sbauza): Once we support multiple typles, check which"},{"line_number":5821,"context_line":"                # PCI devices are set for this type"},{"line_number":5822,"context_line":"                # NOTE(sbauza): a PCI device will always support only one type"},{"line_number":5823,"context_line":"                inv_per_parent \u003d inventories.setdefault(dev_name, {\u0027total\u0027: 0})"},{"line_number":5824,"context_line":"                inv_per_parent[\u0027total\u0027] +\u003d left_vgpus"},{"line_number":5825,"context_line":"                inv_per_parent.update({"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_5c903897","line":5822,"updated":"2018-09-24 19:00:05.000000000","message":"For now.\n\nI\u0027m sure the hardware vendors will randomly change this in the future.","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"722b4071e47400627cc4c7e8b6a7a4855bda761d","unresolved":false,"context_lines":[{"line_number":5819,"context_line":"                left_vgpus \u003d dev[\u0027types\u0027][_type][\u0027availableInstances\u0027]"},{"line_number":5820,"context_line":"                # TODO(sbauza): Once we support multiple typles, check which"},{"line_number":5821,"context_line":"                # PCI devices are set for this type"},{"line_number":5822,"context_line":"                # NOTE(sbauza): a PCI device will always support only one type"},{"line_number":5823,"context_line":"                inv_per_parent \u003d inventories.setdefault(dev_name, {\u0027total\u0027: 0})"},{"line_number":5824,"context_line":"                inv_per_parent[\u0027total\u0027] +\u003d left_vgpus"},{"line_number":5825,"context_line":"                inv_per_parent.update({"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_38b1ba45","line":5822,"in_reply_to":"3f79a3b5_5c903897","updated":"2018-09-26 10:23:25.000000000","message":"Done","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b515399b904e4d824c01df6e33e9f183555dffb3","unresolved":false,"context_lines":[{"line_number":5826,"context_line":"                    \u0027min_unit\u0027: 1,"},{"line_number":5827,"context_line":"                    \u0027step_size\u0027: 1,"},{"line_number":5828,"context_line":"                    # FIXME(sbauza): Set this depending on the hardware vendor"},{"line_number":5829,"context_line":"                    \u0027max_unit\u0027: inv_per_parent[\u0027total\u0027],"},{"line_number":5830,"context_line":"                })"},{"line_number":5831,"context_line":""},{"line_number":5832,"context_line":"        return inventories"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_9c0fb0df","line":5829,"updated":"2018-09-24 19:00:05.000000000","message":"allocation_ratio is missing here. I think it\u0027s pretty safe to hard-code it to 1.0 right now.","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"722b4071e47400627cc4c7e8b6a7a4855bda761d","unresolved":false,"context_lines":[{"line_number":5826,"context_line":"                    \u0027min_unit\u0027: 1,"},{"line_number":5827,"context_line":"                    \u0027step_size\u0027: 1,"},{"line_number":5828,"context_line":"                    # FIXME(sbauza): Set this depending on the hardware vendor"},{"line_number":5829,"context_line":"                    \u0027max_unit\u0027: inv_per_parent[\u0027total\u0027],"},{"line_number":5830,"context_line":"                })"},{"line_number":5831,"context_line":""},{"line_number":5832,"context_line":"        return inventories"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_18b4fe35","line":5829,"in_reply_to":"3f79a3b5_9c0fb0df","updated":"2018-09-26 10:23:25.000000000","message":"Done","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"bbce058085e300b78777231dd2fbc430d9a78523","unresolved":false,"context_lines":[{"line_number":6558,"context_line":"        \"reshape\" the tree if necessary on first start of this compute service"},{"line_number":6559,"context_line":"        in Stein."},{"line_number":6560,"context_line":""},{"line_number":6561,"context_line":"        :param inventories_dict: Dictionary of inventories for VGPU class"},{"line_number":6562,"context_line":"        :param provider_tree: The ProviderTree to update."},{"line_number":6563,"context_line":"        :param nodename: The ComputeNode.hypervisor_hostname, also known as"},{"line_number":6564,"context_line":"            the name of the root node provider in the tree for this host."}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_57dfb137","line":6561,"range":{"start_line":6561,"start_character":33,"end_line":6561,"end_character":73},"updated":"2018-09-24 19:01:20.000000000","message":"Can we get some more detail on what this looks like? Pawing through the code, I think it\u0027s something like:\n\n { $dev_id: {\n       \u0027total\u0027: $total,\n       \u0027min_unit\u0027: 1,\n       \u0027step_size\u0027: 1,\n       \u0027max_unit\u0027: $total,\n   },\n   ...\n }\n\n...where $dev_id looks like... what?\n\nCould you also mention that the dict is expected to be nonempty when we get here (due to L6534)?","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"722b4071e47400627cc4c7e8b6a7a4855bda761d","unresolved":false,"context_lines":[{"line_number":6558,"context_line":"        \"reshape\" the tree if necessary on first start of this compute service"},{"line_number":6559,"context_line":"        in Stein."},{"line_number":6560,"context_line":""},{"line_number":6561,"context_line":"        :param inventories_dict: Dictionary of inventories for VGPU class"},{"line_number":6562,"context_line":"        :param provider_tree: The ProviderTree to update."},{"line_number":6563,"context_line":"        :param nodename: The ComputeNode.hypervisor_hostname, also known as"},{"line_number":6564,"context_line":"            the name of the root node provider in the tree for this host."}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_831e455d","line":6561,"range":{"start_line":6561,"start_character":33,"end_line":6561,"end_character":73},"in_reply_to":"3f79a3b5_57dfb137","updated":"2018-09-26 10:23:25.000000000","message":"Done","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b515399b904e4d824c01df6e33e9f183555dffb3","unresolved":false,"context_lines":[{"line_number":6582,"context_line":"            LOG.info(\u0027Requesting provider tree reshape in order to move \u0027"},{"line_number":6583,"context_line":"                     \u0027VGPU inventory from the root compute node provider \u0027"},{"line_number":6584,"context_line":"                     \u0027%s to a child provider.\u0027, nodename)"},{"line_number":6585,"context_line":"            raise exception.ReshapeNeeded()"},{"line_number":6586,"context_line":""},{"line_number":6587,"context_line":"        # Create the VGPU child providers if they do not already exist."},{"line_number":6588,"context_line":"        # TODO(mriedem): For the moment, _get_supported_vgpu_types() only"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_fc99047a","line":6585,"updated":"2018-09-24 19:00:05.000000000","message":"++","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b515399b904e4d824c01df6e33e9f183555dffb3","unresolved":false,"context_lines":[{"line_number":6598,"context_line":"        pgpu_rps \u003d {}"},{"line_number":6599,"context_line":"        for pgpu_dev_id, inventory in inventories_dict.items():"},{"line_number":6600,"context_line":"            # For each physical GPU, we create a child provider"},{"line_number":6601,"context_line":"            pgpu_rp_name \u003d \u0027%s_VGPU_%s\u0027 % (nodename, pgpu_dev_id)"},{"line_number":6602,"context_line":"            if not provider_tree.exists(pgpu_rp_name):"},{"line_number":6603,"context_line":"                # This is the first time creating the child provider so add"},{"line_number":6604,"context_line":"                # it to the tree under the root node provider."}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_019a5b6e","line":6601,"updated":"2018-09-24 19:00:05.000000000","message":"This is not a good idea to name the provider like this. The provider name should not include a resource class that the provider provides. Nor should the driver-reported vGPU type be part of the name.\n\nA better name would be something like {cn}_GPU{pgpu_index} or {cn}_PGPU{pgpu_index}. The important part of the provider name is an indication of what the provider is, not its inventory. For example, you wouldn\u0027t call a NUMA node provider \"{cn}_VCPU_NUMA\"....\n\nI think in the past, I\u0027d recommended naming the device provider names using the persistent address of the device. So, if we wanted to use our hierarchical naming convention thing, that would something like this:\n\n {hypervisor_hostname}_{pci_address}\n\nwhich would be unique and well-structured. I\u0027m not sure we even need to include a \"type\" of provider in the provider name. What value does that give us anyway?","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"bbce058085e300b78777231dd2fbc430d9a78523","unresolved":false,"context_lines":[{"line_number":6598,"context_line":"        pgpu_rps \u003d {}"},{"line_number":6599,"context_line":"        for pgpu_dev_id, inventory in inventories_dict.items():"},{"line_number":6600,"context_line":"            # For each physical GPU, we create a child provider"},{"line_number":6601,"context_line":"            pgpu_rp_name \u003d \u0027%s_VGPU_%s\u0027 % (nodename, pgpu_dev_id)"},{"line_number":6602,"context_line":"            if not provider_tree.exists(pgpu_rp_name):"},{"line_number":6603,"context_line":"                # This is the first time creating the child provider so add"},{"line_number":6604,"context_line":"                # it to the tree under the root node provider."}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_b785e50c","line":6601,"range":{"start_line":6601,"start_character":26,"end_line":6601,"end_character":65},"updated":"2018-09-24 19:01:20.000000000","message":"This naming convention should be called out in the docstring.\n\nAlso, since this is the name of the *provider*, it should be s/VGPU/PGPU/. The *inventory* (resource class) is VGPU, but the *provider* is PGPU.\n\nAlso, didn\u0027t we say we wanted to include the VGPU *type* in the name? If it\u0027s never possible for the same PGPU to host multiple types at once, I suppose that\u0027s not strictly necessary, but it might be nice e.g. visually when dumping the providers, or for debugging.","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"f8389d19cdac5117c5275d3b71528ea068013a18","unresolved":false,"context_lines":[{"line_number":6598,"context_line":"        pgpu_rps \u003d {}"},{"line_number":6599,"context_line":"        for pgpu_dev_id, inventory in inventories_dict.items():"},{"line_number":6600,"context_line":"            # For each physical GPU, we create a child provider"},{"line_number":6601,"context_line":"            pgpu_rp_name \u003d \u0027%s_VGPU_%s\u0027 % (nodename, pgpu_dev_id)"},{"line_number":6602,"context_line":"            if not provider_tree.exists(pgpu_rp_name):"},{"line_number":6603,"context_line":"                # This is the first time creating the child provider so add"},{"line_number":6604,"context_line":"                # it to the tree under the root node provider."}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_3d6c2106","line":6601,"range":{"start_line":6601,"start_character":26,"end_line":6601,"end_character":65},"in_reply_to":"3f79a3b5_4edccd44","updated":"2018-09-26 20:46:56.000000000","message":"Then a single VGPU resource class just isn\u0027t going to cut it. What you seem to be describing are two different types of consumable resources, not a single resource class (VGPU) with different traits being used to differentiate capabilities of the provider.\n\nI still think having a single PGPU support multiple vGPU types and needing to dynamically adjust inventory for those different custom resource classes representing the different vGPU types is an effort in futility, but whatever, such is the way of the hardware vendor world.","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"87cdd8f1dd0176a788d69d4a23e2adbf604cf6b8","unresolved":false,"context_lines":[{"line_number":6598,"context_line":"        pgpu_rps \u003d {}"},{"line_number":6599,"context_line":"        for pgpu_dev_id, inventory in inventories_dict.items():"},{"line_number":6600,"context_line":"            # For each physical GPU, we create a child provider"},{"line_number":6601,"context_line":"            pgpu_rp_name \u003d \u0027%s_VGPU_%s\u0027 % (nodename, pgpu_dev_id)"},{"line_number":6602,"context_line":"            if not provider_tree.exists(pgpu_rp_name):"},{"line_number":6603,"context_line":"                # This is the first time creating the child provider so add"},{"line_number":6604,"context_line":"                # it to the tree under the root node provider."}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_6dd4324c","line":6601,"range":{"start_line":6601,"start_character":26,"end_line":6601,"end_character":65},"in_reply_to":"3f79a3b5_51a9d622","updated":"2018-09-26 14:37:57.000000000","message":"\u003e For the types, I think that if we would accept multiple types per\n \u003e PGPU (something definetly long-term), we would have to reshape\n \u003e anyway.\n\nIf we include the type now, there\u0027s a decent chance we won\u0027t have to reshape in order to support multiple types. If we don\u0027t include the type now, we\u0027re *guaranteeing* we\u0027ll have to reshape.","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9d38585a465d8648c96473de994d223eb4970587","unresolved":false,"context_lines":[{"line_number":6598,"context_line":"        pgpu_rps \u003d {}"},{"line_number":6599,"context_line":"        for pgpu_dev_id, inventory in inventories_dict.items():"},{"line_number":6600,"context_line":"            # For each physical GPU, we create a child provider"},{"line_number":6601,"context_line":"            pgpu_rp_name \u003d \u0027%s_VGPU_%s\u0027 % (nodename, pgpu_dev_id)"},{"line_number":6602,"context_line":"            if not provider_tree.exists(pgpu_rp_name):"},{"line_number":6603,"context_line":"                # This is the first time creating the child provider so add"},{"line_number":6604,"context_line":"                # it to the tree under the root node provider."}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_4edccd44","line":6601,"range":{"start_line":6601,"start_character":26,"end_line":6601,"end_character":65},"in_reply_to":"3f79a3b5_68bd0002","updated":"2018-09-26 15:57:39.000000000","message":"Okay, I\u0027m not going to fall on my sword for this, but just so we\u0027re clear what we\u0027re talking about:\n\nIf a *single* PGPU is able to host *multiple* VGPU types, we\u0027re going to have *multiple* resource providers per PGPU, one per type. With a naming convention like {hypervisor_hostname}_PGPU_{pci_address}, this would result in each of those providers having the same name, which is forbidden. So when we move to supporting multiple types per PGPU, we\u0027re going to have to come up with a new naming convention and rename the existing providers before we can start adding new ones. If we come up with an unambiguous naming convention now, we can just start adding new ones right away.\n\n/me drops it","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e2c08b02589b73f513304ea5895fdb6d826ddf71","unresolved":false,"context_lines":[{"line_number":6598,"context_line":"        pgpu_rps \u003d {}"},{"line_number":6599,"context_line":"        for pgpu_dev_id, inventory in inventories_dict.items():"},{"line_number":6600,"context_line":"            # For each physical GPU, we create a child provider"},{"line_number":6601,"context_line":"            pgpu_rp_name \u003d \u0027%s_VGPU_%s\u0027 % (nodename, pgpu_dev_id)"},{"line_number":6602,"context_line":"            if not provider_tree.exists(pgpu_rp_name):"},{"line_number":6603,"context_line":"                # This is the first time creating the child provider so add"},{"line_number":6604,"context_line":"                # it to the tree under the root node provider."}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_68bd0002","line":6601,"range":{"start_line":6601,"start_character":26,"end_line":6601,"end_character":65},"in_reply_to":"3f79a3b5_6d80b29d","updated":"2018-09-26 15:06:27.000000000","message":"I tend to agree with Jay. The other problem is that we push upfront the type name which is totally vendor-specific (eg. nvidia-11, which is absolutely not driven by libvirt, where they only namespace the vendor name, not the ending string (here 11)","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"9d0d31d980536af79bd793427a774bb0ae83d440","unresolved":false,"context_lines":[{"line_number":6598,"context_line":"        pgpu_rps \u003d {}"},{"line_number":6599,"context_line":"        for pgpu_dev_id, inventory in inventories_dict.items():"},{"line_number":6600,"context_line":"            # For each physical GPU, we create a child provider"},{"line_number":6601,"context_line":"            pgpu_rp_name \u003d \u0027%s_VGPU_%s\u0027 % (nodename, pgpu_dev_id)"},{"line_number":6602,"context_line":"            if not provider_tree.exists(pgpu_rp_name):"},{"line_number":6603,"context_line":"                # This is the first time creating the child provider so add"},{"line_number":6604,"context_line":"                # it to the tree under the root node provider."}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_6d80b29d","line":6601,"range":{"start_line":6601,"start_character":26,"end_line":6601,"end_character":65},"in_reply_to":"3f79a3b5_6dd4324c","updated":"2018-09-26 14:51:45.000000000","message":"@efried: I don\u0027t understand that statement. What does having the vGPU type in the provider name have to do with whether or not something is reshaped in the future?\n\nI don\u0027t see any reason to name the provider with a type at all. It\u0027s a PCI device that is providing some resources. Just name the provider the PCI device address, prefixed by the compute node\u0027s hypervisor_hostname to make the name unique.\n\nLet\u0027s not encode more information into the provider name than needed.","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"722b4071e47400627cc4c7e8b6a7a4855bda761d","unresolved":false,"context_lines":[{"line_number":6598,"context_line":"        pgpu_rps \u003d {}"},{"line_number":6599,"context_line":"        for pgpu_dev_id, inventory in inventories_dict.items():"},{"line_number":6600,"context_line":"            # For each physical GPU, we create a child provider"},{"line_number":6601,"context_line":"            pgpu_rp_name \u003d \u0027%s_VGPU_%s\u0027 % (nodename, pgpu_dev_id)"},{"line_number":6602,"context_line":"            if not provider_tree.exists(pgpu_rp_name):"},{"line_number":6603,"context_line":"                # This is the first time creating the child provider so add"},{"line_number":6604,"context_line":"                # it to the tree under the root node provider."}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_51a9d622","line":6601,"range":{"start_line":6601,"start_character":26,"end_line":6601,"end_character":65},"in_reply_to":"3f79a3b5_b785e50c","updated":"2018-09-26 10:23:25.000000000","message":"Eric, Jay, thanks for your very good insights.\nI agree with Jay on the fact it should be {cn}_{pci_address}\n\nFor the types, I think that if we would accept multiple types per PGPU (something definetly long-term), we would have to reshape anyway.\nFor the moment, the \u0027\u0027type\u0027\u0027 (see the brackets) should be a list of traits for one resource provider (the PGPU).","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"bbce058085e300b78777231dd2fbc430d9a78523","unresolved":false,"context_lines":[{"line_number":6608,"context_line":""},{"line_number":6609,"context_line":"            # The VGPU inventory goes on a child provider of the given root"},{"line_number":6610,"context_line":"            # node, identified by $nodename."},{"line_number":6611,"context_line":"            pgpu_inventory \u003d {}"},{"line_number":6612,"context_line":"            pgpu_inventory[rc_fields.ResourceClass.VGPU] \u003d inventory"},{"line_number":6613,"context_line":"            provider_tree.update_inventory(pgpu_rp_name, pgpu_inventory)"},{"line_number":6614,"context_line":""},{"line_number":6615,"context_line":"        if allocations is not None:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_9799c96e","line":6612,"range":{"start_line":6611,"start_character":12,"end_line":6612,"end_character":68},"updated":"2018-09-24 19:01:20.000000000","message":"nit: collapse","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b515399b904e4d824c01df6e33e9f183555dffb3","unresolved":false,"context_lines":[{"line_number":6621,"context_line":"            for consumer_uuid, alloc_data in allocations.items():"},{"line_number":6622,"context_line":"                # Copy and iterate over the current set of providers to avoid"},{"line_number":6623,"context_line":"                # modifying keys while iterating."},{"line_number":6624,"context_line":"                allocations \u003d alloc_data[\u0027allocations\u0027]"},{"line_number":6625,"context_line":"                for rp_uuid in list(allocations):"},{"line_number":6626,"context_line":"                    resources \u003d allocations[rp_uuid][\u0027resources\u0027]"},{"line_number":6627,"context_line":"                    if rc_fields.ResourceClass.VGPU in resources:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_bc014c70","line":6624,"range":{"start_line":6624,"start_character":16,"end_line":6624,"end_character":30},"updated":"2018-09-24 19:00:05.000000000","message":"here, you just shadowed the original allocations variable argument to this function.\n\nCan you name this variable something different?","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"722b4071e47400627cc4c7e8b6a7a4855bda761d","unresolved":false,"context_lines":[{"line_number":6621,"context_line":"            for consumer_uuid, alloc_data in allocations.items():"},{"line_number":6622,"context_line":"                # Copy and iterate over the current set of providers to avoid"},{"line_number":6623,"context_line":"                # modifying keys while iterating."},{"line_number":6624,"context_line":"                allocations \u003d alloc_data[\u0027allocations\u0027]"},{"line_number":6625,"context_line":"                for rp_uuid in list(allocations):"},{"line_number":6626,"context_line":"                    resources \u003d allocations[rp_uuid][\u0027resources\u0027]"},{"line_number":6627,"context_line":"                    if rc_fields.ResourceClass.VGPU in resources:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_eb46f550","line":6624,"range":{"start_line":6624,"start_character":16,"end_line":6624,"end_character":30},"in_reply_to":"3f79a3b5_bc014c70","updated":"2018-09-26 10:23:25.000000000","message":"Done","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b515399b904e4d824c01df6e33e9f183555dffb3","unresolved":false,"context_lines":[{"line_number":6622,"context_line":"                # Copy and iterate over the current set of providers to avoid"},{"line_number":6623,"context_line":"                # modifying keys while iterating."},{"line_number":6624,"context_line":"                allocations \u003d alloc_data[\u0027allocations\u0027]"},{"line_number":6625,"context_line":"                for rp_uuid in list(allocations):"},{"line_number":6626,"context_line":"                    resources \u003d allocations[rp_uuid][\u0027resources\u0027]"},{"line_number":6627,"context_line":"                    if rc_fields.ResourceClass.VGPU in resources:"},{"line_number":6628,"context_line":"                        # We\u0027ve found VGPU allocations on a provider. It"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_3c3a1c35","line":6625,"range":{"start_line":6625,"start_character":31,"end_line":6625,"end_character":49},"updated":"2018-09-24 19:00:05.000000000","message":"Why are you doing this? Why not use .items() or .keys()?","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"722b4071e47400627cc4c7e8b6a7a4855bda761d","unresolved":false,"context_lines":[{"line_number":6622,"context_line":"                # Copy and iterate over the current set of providers to avoid"},{"line_number":6623,"context_line":"                # modifying keys while iterating."},{"line_number":6624,"context_line":"                allocations \u003d alloc_data[\u0027allocations\u0027]"},{"line_number":6625,"context_line":"                for rp_uuid in list(allocations):"},{"line_number":6626,"context_line":"                    resources \u003d allocations[rp_uuid][\u0027resources\u0027]"},{"line_number":6627,"context_line":"                    if rc_fields.ResourceClass.VGPU in resources:"},{"line_number":6628,"context_line":"                        # We\u0027ve found VGPU allocations on a provider. It"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_4b3849ca","line":6625,"range":{"start_line":6625,"start_character":31,"end_line":6625,"end_character":49},"in_reply_to":"3f79a3b5_3c3a1c35","updated":"2018-09-26 10:23:25.000000000","message":"Done","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b515399b904e4d824c01df6e33e9f183555dffb3","unresolved":false,"context_lines":[{"line_number":6640,"context_line":"                                      \u0027consumer_uuid\u0027: consumer_uuid,"},{"line_number":6641,"context_line":"                                      \u0027alloc_data\u0027: alloc_data,"},{"line_number":6642,"context_line":"                                      \u0027root_uuid\u0027: root_node.uuid})"},{"line_number":6643,"context_line":"                            raise exception.ReshapeFailed(error\u003dmsg)"},{"line_number":6644,"context_line":"                        # Find which physical GPU corresponds to this"},{"line_number":6645,"context_line":"                        # allocation"},{"line_number":6646,"context_line":"                        try:"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_7ccc1434","line":6643,"updated":"2018-09-24 19:00:05.000000000","message":"✓","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b515399b904e4d824c01df6e33e9f183555dffb3","unresolved":false,"context_lines":[{"line_number":6645,"context_line":"                        # allocation"},{"line_number":6646,"context_line":"                        try:"},{"line_number":6647,"context_line":"                            instance \u003d objects.Instance.get_by_uuid("},{"line_number":6648,"context_line":"                                consumer_uuid)"},{"line_number":6649,"context_line":"                        except exception.InstanceNotFound:"},{"line_number":6650,"context_line":"                            # If we found a VGPU allocation against a consumer"},{"line_number":6651,"context_line":"                            # which is not an instance, the only left case for"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_bceb8c79","line":6648,"updated":"2018-09-24 19:00:05.000000000","message":"An idea for a future improvement here, but we generally do not want to make N calls to Instance.get_by_uuid() when we can do a single call ahead of time to Instance.get_by_uuids() and pass in all the instance UUIDs at once.","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"722b4071e47400627cc4c7e8b6a7a4855bda761d","unresolved":false,"context_lines":[{"line_number":6645,"context_line":"                        # allocation"},{"line_number":6646,"context_line":"                        try:"},{"line_number":6647,"context_line":"                            instance \u003d objects.Instance.get_by_uuid("},{"line_number":6648,"context_line":"                                consumer_uuid)"},{"line_number":6649,"context_line":"                        except exception.InstanceNotFound:"},{"line_number":6650,"context_line":"                            # If we found a VGPU allocation against a consumer"},{"line_number":6651,"context_line":"                            # which is not an instance, the only left case for"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_6b6c25d0","line":6648,"in_reply_to":"3f79a3b5_bceb8c79","updated":"2018-09-26 10:23:25.000000000","message":"Agreed.","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b515399b904e4d824c01df6e33e9f183555dffb3","unresolved":false,"context_lines":[{"line_number":6659,"context_line":"                                   % {\u0027rp_uuid\u0027: rp_uuid,"},{"line_number":6660,"context_line":"                                      \u0027consumer_uuid\u0027: consumer_uuid,"},{"line_number":6661,"context_line":"                                      \u0027alloc_data\u0027: alloc_data})"},{"line_number":6662,"context_line":"                            raise exception.ReshapeFailed(error\u003dmsg)"},{"line_number":6663,"context_line":"                        mdevs \u003d self._get_all_assigned_mediated_devices("},{"line_number":6664,"context_line":"                            instance)"},{"line_number":6665,"context_line":"                        vgpu_count_per_pgpu \u003d defaultdict(int)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_fcf8244b","line":6662,"updated":"2018-09-24 19:00:05.000000000","message":"✓","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"b515399b904e4d824c01df6e33e9f183555dffb3","unresolved":false,"context_lines":[{"line_number":6663,"context_line":"                        mdevs \u003d self._get_all_assigned_mediated_devices("},{"line_number":6664,"context_line":"                            instance)"},{"line_number":6665,"context_line":"                        vgpu_count_per_pgpu \u003d defaultdict(int)"},{"line_number":6666,"context_line":"                        for mdev in list(mdevs.keys()):"},{"line_number":6667,"context_line":"                            # Count how many vGPUs are in use for this instance"},{"line_number":6668,"context_line":"                            dev_info \u003d self._get_mediated_device_information("},{"line_number":6669,"context_line":"                                mdev)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_975a29f0","line":6666,"range":{"start_line":6666,"start_character":36,"end_line":6666,"end_character":55},"updated":"2018-09-24 19:00:05.000000000","message":"just do:\n\n for mdev in mdevs.keys():","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"9d38585a465d8648c96473de994d223eb4970587","unresolved":false,"context_lines":[{"line_number":6663,"context_line":"                        mdevs \u003d self._get_all_assigned_mediated_devices("},{"line_number":6664,"context_line":"                            instance)"},{"line_number":6665,"context_line":"                        vgpu_count_per_pgpu \u003d defaultdict(int)"},{"line_number":6666,"context_line":"                        for mdev in list(mdevs.keys()):"},{"line_number":6667,"context_line":"                            # Count how many vGPUs are in use for this instance"},{"line_number":6668,"context_line":"                            dev_info \u003d self._get_mediated_device_information("},{"line_number":6669,"context_line":"                                mdev)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_aebb210d","line":6666,"range":{"start_line":6666,"start_character":36,"end_line":6666,"end_character":55},"in_reply_to":"3f79a3b5_088ecc78","updated":"2018-09-26 15:57:39.000000000","message":"IMO this is not a good reason to suck up extra memory and processor.\n\nBut it\u0027s moot in py3, and we\u0027re probably talking about a very small number of mdevs anyway, so whatever.","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e2c08b02589b73f513304ea5895fdb6d826ddf71","unresolved":false,"context_lines":[{"line_number":6663,"context_line":"                        mdevs \u003d self._get_all_assigned_mediated_devices("},{"line_number":6664,"context_line":"                            instance)"},{"line_number":6665,"context_line":"                        vgpu_count_per_pgpu \u003d defaultdict(int)"},{"line_number":6666,"context_line":"                        for mdev in list(mdevs.keys()):"},{"line_number":6667,"context_line":"                            # Count how many vGPUs are in use for this instance"},{"line_number":6668,"context_line":"                            dev_info \u003d self._get_mediated_device_information("},{"line_number":6669,"context_line":"                                mdev)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_088ecc78","line":6666,"range":{"start_line":6666,"start_character":36,"end_line":6666,"end_character":55},"in_reply_to":"3f79a3b5_0d6a5e80","updated":"2018-09-26 15:06:27.000000000","message":"Sure, but .keys() helps to remember it\u0027s not just a list but rather a dict. Call it \"maintenance\"","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"87cdd8f1dd0176a788d69d4a23e2adbf604cf6b8","unresolved":false,"context_lines":[{"line_number":6663,"context_line":"                        mdevs \u003d self._get_all_assigned_mediated_devices("},{"line_number":6664,"context_line":"                            instance)"},{"line_number":6665,"context_line":"                        vgpu_count_per_pgpu \u003d defaultdict(int)"},{"line_number":6666,"context_line":"                        for mdev in list(mdevs.keys()):"},{"line_number":6667,"context_line":"                            # Count how many vGPUs are in use for this instance"},{"line_number":6668,"context_line":"                            dev_info \u003d self._get_mediated_device_information("},{"line_number":6669,"context_line":"                                mdev)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_0d6a5e80","line":6666,"range":{"start_line":6666,"start_character":36,"end_line":6666,"end_character":55},"in_reply_to":"3f79a3b5_4a861919","updated":"2018-09-26 14:37:57.000000000","message":"Should actually be\n\n for mdev in mdevs\n\nwhich iterates over the keys in both py2 and py3 without creating any extra artifacts.\n\n\u003cpedantic\u003e\nAs written, in py2 this creates a list, then copies it, then iterates over the copy. In py3 it creates an iterator, then a list from that iterator, then iterates over the list.\n\nIn py3, `for mdev in mdevs.keys()` is equivalent to `for mdev in mdevs`, but in py2 it creates a list, then iterates over the list.\n\u003c/pedantic\u003e","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"722b4071e47400627cc4c7e8b6a7a4855bda761d","unresolved":false,"context_lines":[{"line_number":6663,"context_line":"                        mdevs \u003d self._get_all_assigned_mediated_devices("},{"line_number":6664,"context_line":"                            instance)"},{"line_number":6665,"context_line":"                        vgpu_count_per_pgpu \u003d defaultdict(int)"},{"line_number":6666,"context_line":"                        for mdev in list(mdevs.keys()):"},{"line_number":6667,"context_line":"                            # Count how many vGPUs are in use for this instance"},{"line_number":6668,"context_line":"                            dev_info \u003d self._get_mediated_device_information("},{"line_number":6669,"context_line":"                                mdev)"}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_4a861919","line":6666,"range":{"start_line":6666,"start_character":36,"end_line":6666,"end_character":55},"in_reply_to":"3f79a3b5_975a29f0","updated":"2018-09-26 10:23:25.000000000","message":"given .keys() gives a `list` for py2 but a `dict_keys` for pty3, that\u0027s why I\u0027m calling list() but given we are just looping against an iterator, fine.","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"bbce058085e300b78777231dd2fbc430d9a78523","unresolved":false,"context_lines":[{"line_number":6694,"context_line":"                        # And remove the VGPU allocation from the"},{"line_number":6695,"context_line":"                        # root node provider."},{"line_number":6696,"context_line":"                        del resources[rc_fields.ResourceClass.VGPU]"},{"line_number":6697,"context_line":""},{"line_number":6698,"context_line":"    def get_available_resource(self, nodename):"},{"line_number":6699,"context_line":"        \"\"\"Retrieve resource information."},{"line_number":6700,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_f7a6fd54","line":6697,"updated":"2018-09-24 19:01:20.000000000","message":"I\u0027m missing the part where you remove the VGPU inventory from the root provider. Expected something like:\n\n if rc_fields.ResourceClass.VGPU in root_node.inventory\n     del root_node.inventory[rc_fields.ResourceClass.VGPU]\n     provider_tree.update_inventory(nodename, root_node.inventory)","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"722b4071e47400627cc4c7e8b6a7a4855bda761d","unresolved":false,"context_lines":[{"line_number":6694,"context_line":"                        # And remove the VGPU allocation from the"},{"line_number":6695,"context_line":"                        # root node provider."},{"line_number":6696,"context_line":"                        del resources[rc_fields.ResourceClass.VGPU]"},{"line_number":6697,"context_line":""},{"line_number":6698,"context_line":"    def get_available_resource(self, nodename):"},{"line_number":6699,"context_line":"        \"\"\"Retrieve resource information."},{"line_number":6700,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"3f79a3b5_cb4c5977","line":6697,"in_reply_to":"3f79a3b5_f7a6fd54","updated":"2018-09-26 10:23:25.000000000","message":"Good point. Done","commit_id":"504262ccc33fa6f8fcb877d15bc784ad90baeb8b"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"93dd47f8bf7aa08fe3281eb288a4f9c402420174","unresolved":false,"context_lines":[{"line_number":6716,"context_line":"                            # actually correcly allocate the existing resources"},{"line_number":6717,"context_line":"                            LOG.warning(msg)"},{"line_number":6718,"context_line":"                        # Add the VGPU allocation for each VGPU provider"},{"line_number":6719,"context_line":"                        for pgpu_dev_id, pgpu_rp in pgpu_rps.items():"},{"line_number":6720,"context_line":"                            if vgpu_count_per_pgpu[pgpu_dev_id] \u003e 0:"},{"line_number":6721,"context_line":"                                allocs[pgpu_rp.uuid] \u003d {"},{"line_number":6722,"context_line":"                                    \u0027resources\u0027: {"},{"line_number":6723,"context_line":"                                        rc_fields.ResourceClass.VGPU:"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_8e61333b","line":6720,"range":{"start_line":6719,"start_character":24,"end_line":6720,"end_character":68},"updated":"2018-11-07 08:10:44.000000000","message":"nit, sounds like we can just do:\n\nfor pgpu_dev_id, usage in vgpu_count_per_pgpu.items():\n    if usage \u003e 0:\n        allocs[pgpu_rps[pgpu_dev_id]] \u003d {.....}\n\nwithout interate all the pgpus.","commit_id":"df6dd72ca237b8a2c7e3701be5befee23f68f5de"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3c82a030604ccc09841e7ba90fe9847c3ea4bea0","unresolved":false,"context_lines":[{"line_number":6716,"context_line":"                            # actually correcly allocate the existing resources"},{"line_number":6717,"context_line":"                            LOG.warning(msg)"},{"line_number":6718,"context_line":"                        # Add the VGPU allocation for each VGPU provider"},{"line_number":6719,"context_line":"                        for pgpu_dev_id, pgpu_rp in pgpu_rps.items():"},{"line_number":6720,"context_line":"                            if vgpu_count_per_pgpu[pgpu_dev_id] \u003e 0:"},{"line_number":6721,"context_line":"                                allocs[pgpu_rp.uuid] \u003d {"},{"line_number":6722,"context_line":"                                    \u0027resources\u0027: {"},{"line_number":6723,"context_line":"                                        rc_fields.ResourceClass.VGPU:"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_84c28b34","line":6720,"range":{"start_line":6719,"start_character":24,"end_line":6720,"end_character":68},"in_reply_to":"3f79a3b5_8e61333b","updated":"2018-11-21 18:50:09.000000000","message":"Do you mean:\n\nallocs[pgpu_rps[pgpu_dev_id].uuid] \u003d {...}\n\n?\n\nOtherwise yeah.","commit_id":"df6dd72ca237b8a2c7e3701be5befee23f68f5de"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"b6c7b3ec76115576a24c1007527c00e6e221ff52","unresolved":false,"context_lines":[{"line_number":5763,"context_line":"        return requested_types"},{"line_number":5764,"context_line":""},{"line_number":5765,"context_line":"    # TODO(sbauza): Remove this method once we start supporting multiple types"},{"line_number":5766,"context_line":"    def _get_vgpu_total(self):"},{"line_number":5767,"context_line":"        \"\"\"Returns the number of total available vGPUs for any GPU type that is"},{"line_number":5768,"context_line":"        enabled with the enabled_vgpu_types CONF option."},{"line_number":5769,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"bfdaf3ff_7ac71414","line":5766,"range":{"start_line":5766,"start_character":8,"end_line":5766,"end_character":23},"updated":"2019-02-15 17:26:51.000000000","message":"This method appears to be used only in test. Is that expected at this point? If so, perhaps it would be best to move the method into the test suite.\n\n...but looking at the test, it doesn\u0027t seem to be playing with multiple types.","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ff9049938920d1250d5ec4553285daae5b712d89","unresolved":false,"context_lines":[{"line_number":5763,"context_line":"        return requested_types"},{"line_number":5764,"context_line":""},{"line_number":5765,"context_line":"    # TODO(sbauza): Remove this method once we start supporting multiple types"},{"line_number":5766,"context_line":"    def _get_vgpu_total(self):"},{"line_number":5767,"context_line":"        \"\"\"Returns the number of total available vGPUs for any GPU type that is"},{"line_number":5768,"context_line":"        enabled with the enabled_vgpu_types CONF option."},{"line_number":5769,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_7f314da2","line":5766,"range":{"start_line":5766,"start_character":8,"end_line":5766,"end_character":23},"updated":"2019-01-23 23:36:57.000000000","message":"Why not remove it now? Nothing is using it anymore.","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"75bcda430447607fcade61cf148b1746b3062d4e","unresolved":false,"context_lines":[{"line_number":5784,"context_line":"        vgpus +\u003d len(mediated_devices)"},{"line_number":5785,"context_line":"        return vgpus"},{"line_number":5786,"context_line":""},{"line_number":5787,"context_line":"    def _get_gpu_inventories(self):"},{"line_number":5788,"context_line":"        \"\"\"Returns the inventories for each physical GPU for a specific type"},{"line_number":5789,"context_line":"        supported by the enabled_vgpu_types CONF option."},{"line_number":5790,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_4cdc8a16","line":5787,"updated":"2018-11-22 20:53:00.000000000","message":"I had trouble wrapping my head around this method - maybe I\u0027m slow, but it seems to me it could be made clearer by first counting what we need to count, *then* setting those values in the inventories dict.\n\nSo for instance, we could even split this into two help methods: one counts mediated devices, and returns a dict of pci_name -\u003e count, the other counts mdev capable devices, and returns a dic of pci_name -\u003e availableInstances, and finally back in _get_gpu_inventories, we combine those two counts and put it in the final inventories dict. Does that make sense? Is it actually clearer than what\u0027s here now?","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d68e7a085bd4a5a8d82fc6069e67308796b5b5b1","unresolved":false,"context_lines":[{"line_number":5784,"context_line":"        vgpus +\u003d len(mediated_devices)"},{"line_number":5785,"context_line":"        return vgpus"},{"line_number":5786,"context_line":""},{"line_number":5787,"context_line":"    def _get_gpu_inventories(self):"},{"line_number":5788,"context_line":"        \"\"\"Returns the inventories for each physical GPU for a specific type"},{"line_number":5789,"context_line":"        supported by the enabled_vgpu_types CONF option."},{"line_number":5790,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_5f6d8096","line":5787,"in_reply_to":"3f79a3b5_4cdc8a16","updated":"2019-01-23 00:02:13.000000000","message":"Done","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"8263479a7b2326775f2fb96644ecad74a46ad73a","unresolved":false,"context_lines":[{"line_number":5803,"context_line":"        # Bail out early if operator doesn\u0027t care about providing vGPUs"},{"line_number":5804,"context_line":"        if not CONF.devices.enabled_vgpu_types:"},{"line_number":5805,"context_line":"            return {}"},{"line_number":5806,"context_line":"        requested_types \u003d CONF.devices.enabled_vgpu_types"},{"line_number":5807,"context_line":"        inventories \u003d {}"},{"line_number":5808,"context_line":"        # Count the existing mediated devices that have been set in sysfs and"},{"line_number":5809,"context_line":"        # supporting the requested types (but possibly not assigned to a guest,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_b177b53c","line":5806,"updated":"2018-11-22 17:22:44.000000000","message":"Not that it matters, but you presumably could avoid the double dip into CONF here, if you felt like it.","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"d68e7a085bd4a5a8d82fc6069e67308796b5b5b1","unresolved":false,"context_lines":[{"line_number":5803,"context_line":"        # Bail out early if operator doesn\u0027t care about providing vGPUs"},{"line_number":5804,"context_line":"        if not CONF.devices.enabled_vgpu_types:"},{"line_number":5805,"context_line":"            return {}"},{"line_number":5806,"context_line":"        requested_types \u003d CONF.devices.enabled_vgpu_types"},{"line_number":5807,"context_line":"        inventories \u003d {}"},{"line_number":5808,"context_line":"        # Count the existing mediated devices that have been set in sysfs and"},{"line_number":5809,"context_line":"        # supporting the requested types (but possibly not assigned to a guest,"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_1c7946fb","line":5806,"in_reply_to":"3f79a3b5_b177b53c","updated":"2019-01-23 00:02:13.000000000","message":"Done","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"75bcda430447607fcade61cf148b1746b3062d4e","unresolved":false,"context_lines":[{"line_number":6598,"context_line":"        :raises: nova.exception.ReshapeFailed if the requested tree reshape"},{"line_number":6599,"context_line":"            fails for whatever reason."},{"line_number":6600,"context_line":"        \"\"\""},{"line_number":6601,"context_line":"        # Check to see if the root compute node provider in the tree for"},{"line_number":6602,"context_line":"        # this host already has VGPU inventory because if it does, and"},{"line_number":6603,"context_line":"        # we\u0027re not currently reshaping (allocations is None), we need"},{"line_number":6604,"context_line":"        # to indicate that a reshape is needed to move the VGPU inventory"},{"line_number":6605,"context_line":"        # onto a child provider in the tree."},{"line_number":6606,"context_line":"        root_node \u003d provider_tree.data(nodename)"},{"line_number":6607,"context_line":"        if (rc_fields.ResourceClass.VGPU in root_node.inventory and"},{"line_number":6608,"context_line":"                allocations is None):"},{"line_number":6609,"context_line":"            LOG.info(\u0027Requesting provider tree reshape in order to move \u0027"},{"line_number":6610,"context_line":"                     \u0027VGPU inventory from the root compute node provider \u0027"},{"line_number":6611,"context_line":"                     \u0027%s to a child provider.\u0027, nodename)"},{"line_number":6612,"context_line":"            raise exception.ReshapeNeeded()"},{"line_number":6613,"context_line":""},{"line_number":6614,"context_line":"        # Create the VGPU child providers if they do not already exist."},{"line_number":6615,"context_line":"        # TODO(mriedem): For the moment, _get_supported_vgpu_types() only"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_07550bc1","line":6612,"range":{"start_line":6601,"start_character":0,"end_line":6612,"end_character":43},"updated":"2018-11-22 20:53:00.000000000","message":"Maybe split this off into a helper method?","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b216086e399d00d0d3d73d9336e74b4d96eb8ff4","unresolved":false,"context_lines":[{"line_number":6598,"context_line":"        :raises: nova.exception.ReshapeFailed if the requested tree reshape"},{"line_number":6599,"context_line":"            fails for whatever reason."},{"line_number":6600,"context_line":"        \"\"\""},{"line_number":6601,"context_line":"        # Check to see if the root compute node provider in the tree for"},{"line_number":6602,"context_line":"        # this host already has VGPU inventory because if it does, and"},{"line_number":6603,"context_line":"        # we\u0027re not currently reshaping (allocations is None), we need"},{"line_number":6604,"context_line":"        # to indicate that a reshape is needed to move the VGPU inventory"},{"line_number":6605,"context_line":"        # onto a child provider in the tree."},{"line_number":6606,"context_line":"        root_node \u003d provider_tree.data(nodename)"},{"line_number":6607,"context_line":"        if (rc_fields.ResourceClass.VGPU in root_node.inventory and"},{"line_number":6608,"context_line":"                allocations is None):"},{"line_number":6609,"context_line":"            LOG.info(\u0027Requesting provider tree reshape in order to move \u0027"},{"line_number":6610,"context_line":"                     \u0027VGPU inventory from the root compute node provider \u0027"},{"line_number":6611,"context_line":"                     \u0027%s to a child provider.\u0027, nodename)"},{"line_number":6612,"context_line":"            raise exception.ReshapeNeeded()"},{"line_number":6613,"context_line":""},{"line_number":6614,"context_line":"        # Create the VGPU child providers if they do not already exist."},{"line_number":6615,"context_line":"        # TODO(mriedem): For the moment, _get_supported_vgpu_types() only"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_39bbe572","line":6612,"range":{"start_line":6601,"start_character":0,"end_line":6612,"end_character":43},"in_reply_to":"3f79a3b5_07550bc1","updated":"2019-01-23 23:23:59.000000000","message":"Done","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"75bcda430447607fcade61cf148b1746b3062d4e","unresolved":false,"context_lines":[{"line_number":6611,"context_line":"                     \u0027%s to a child provider.\u0027, nodename)"},{"line_number":6612,"context_line":"            raise exception.ReshapeNeeded()"},{"line_number":6613,"context_line":""},{"line_number":6614,"context_line":"        # Create the VGPU child providers if they do not already exist."},{"line_number":6615,"context_line":"        # TODO(mriedem): For the moment, _get_supported_vgpu_types() only"},{"line_number":6616,"context_line":"        # returns one single type but that will be changed once we support"},{"line_number":6617,"context_line":"        # multiple types."},{"line_number":6618,"context_line":"        # Note that we can\u0027t support multiple vgpu types until a reshape has"},{"line_number":6619,"context_line":"        # been performed on the vgpu resources provided by the root provider,"},{"line_number":6620,"context_line":"        # if any."},{"line_number":6621,"context_line":"        # TODO(sbauza): The operator can change the types, so in this case, we"},{"line_number":6622,"context_line":"        # need to raise a ReshapeNeeded exception when checking the inventories"},{"line_number":6623,"context_line":""},{"line_number":6624,"context_line":"        # Dict of PGPU RPs keyed by their libvirt PCI name"},{"line_number":6625,"context_line":"        pgpu_rps \u003d {}"},{"line_number":6626,"context_line":"        for pgpu_dev_id, inventory in inventories_dict.items():"},{"line_number":6627,"context_line":"            # For each physical GPU, we create a child provider"},{"line_number":6628,"context_line":"            pgpu_rp_name \u003d \u0027%s_%s\u0027 % (nodename, pgpu_dev_id)"},{"line_number":6629,"context_line":"            if not provider_tree.exists(pgpu_rp_name):"},{"line_number":6630,"context_line":"                # This is the first time creating the child provider so add"},{"line_number":6631,"context_line":"                # it to the tree under the root node provider."},{"line_number":6632,"context_line":"                provider_tree.new_child(pgpu_rp_name, nodename)"},{"line_number":6633,"context_line":"                pgpu_rp \u003d provider_tree.data(pgpu_rp_name)"},{"line_number":6634,"context_line":"                pgpu_rps[pgpu_dev_id] \u003d pgpu_rp"},{"line_number":6635,"context_line":""},{"line_number":6636,"context_line":"            # The VGPU inventory goes on a child provider of the given root"},{"line_number":6637,"context_line":"            # node, identified by $nodename."},{"line_number":6638,"context_line":"            pgpu_inventory \u003d {rc_fields.ResourceClass.VGPU: inventory}"},{"line_number":6639,"context_line":"            provider_tree.update_inventory(pgpu_rp_name, pgpu_inventory)"},{"line_number":6640,"context_line":""},{"line_number":6641,"context_line":"        if allocations is not None:"},{"line_number":6642,"context_line":"            LOG.info(\u0027Reshaping tree; moving VGPU allocations from root \u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_674abf25","line":6639,"range":{"start_line":6614,"start_character":0,"end_line":6639,"end_character":72},"updated":"2018-11-22 20:53:00.000000000","message":"This could be a helper method as well, no? It\u0027ll need to return pgpu_rps.","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b216086e399d00d0d3d73d9336e74b4d96eb8ff4","unresolved":false,"context_lines":[{"line_number":6611,"context_line":"                     \u0027%s to a child provider.\u0027, nodename)"},{"line_number":6612,"context_line":"            raise exception.ReshapeNeeded()"},{"line_number":6613,"context_line":""},{"line_number":6614,"context_line":"        # Create the VGPU child providers if they do not already exist."},{"line_number":6615,"context_line":"        # TODO(mriedem): For the moment, _get_supported_vgpu_types() only"},{"line_number":6616,"context_line":"        # returns one single type but that will be changed once we support"},{"line_number":6617,"context_line":"        # multiple types."},{"line_number":6618,"context_line":"        # Note that we can\u0027t support multiple vgpu types until a reshape has"},{"line_number":6619,"context_line":"        # been performed on the vgpu resources provided by the root provider,"},{"line_number":6620,"context_line":"        # if any."},{"line_number":6621,"context_line":"        # TODO(sbauza): The operator can change the types, so in this case, we"},{"line_number":6622,"context_line":"        # need to raise a ReshapeNeeded exception when checking the inventories"},{"line_number":6623,"context_line":""},{"line_number":6624,"context_line":"        # Dict of PGPU RPs keyed by their libvirt PCI name"},{"line_number":6625,"context_line":"        pgpu_rps \u003d {}"},{"line_number":6626,"context_line":"        for pgpu_dev_id, inventory in inventories_dict.items():"},{"line_number":6627,"context_line":"            # For each physical GPU, we create a child provider"},{"line_number":6628,"context_line":"            pgpu_rp_name \u003d \u0027%s_%s\u0027 % (nodename, pgpu_dev_id)"},{"line_number":6629,"context_line":"            if not provider_tree.exists(pgpu_rp_name):"},{"line_number":6630,"context_line":"                # This is the first time creating the child provider so add"},{"line_number":6631,"context_line":"                # it to the tree under the root node provider."},{"line_number":6632,"context_line":"                provider_tree.new_child(pgpu_rp_name, nodename)"},{"line_number":6633,"context_line":"                pgpu_rp \u003d provider_tree.data(pgpu_rp_name)"},{"line_number":6634,"context_line":"                pgpu_rps[pgpu_dev_id] \u003d pgpu_rp"},{"line_number":6635,"context_line":""},{"line_number":6636,"context_line":"            # The VGPU inventory goes on a child provider of the given root"},{"line_number":6637,"context_line":"            # node, identified by $nodename."},{"line_number":6638,"context_line":"            pgpu_inventory \u003d {rc_fields.ResourceClass.VGPU: inventory}"},{"line_number":6639,"context_line":"            provider_tree.update_inventory(pgpu_rp_name, pgpu_inventory)"},{"line_number":6640,"context_line":""},{"line_number":6641,"context_line":"        if allocations is not None:"},{"line_number":6642,"context_line":"            LOG.info(\u0027Reshaping tree; moving VGPU allocations from root \u0027"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_1c57ef04","line":6639,"range":{"start_line":6614,"start_character":0,"end_line":6639,"end_character":72},"in_reply_to":"3f79a3b5_674abf25","updated":"2019-01-23 23:23:59.000000000","message":"Done","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"75bcda430447607fcade61cf148b1746b3062d4e","unresolved":false,"context_lines":[{"line_number":6638,"context_line":"            pgpu_inventory \u003d {rc_fields.ResourceClass.VGPU: inventory}"},{"line_number":6639,"context_line":"            provider_tree.update_inventory(pgpu_rp_name, pgpu_inventory)"},{"line_number":6640,"context_line":""},{"line_number":6641,"context_line":"        if allocations is not None:"},{"line_number":6642,"context_line":"            LOG.info(\u0027Reshaping tree; moving VGPU allocations from root \u0027"},{"line_number":6643,"context_line":"                     \u0027provider %s to child providers %s.\u0027, root_node.uuid,"},{"line_number":6644,"context_line":"                     pgpu_rps.values())"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_c7ef13f8","line":6641,"updated":"2018-11-22 20:53:00.000000000","message":"What follows is actually surprisingly clear, despite the ridiculous nesting. I\u0027d still refactor it, if possible - see following comments.","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"75bcda430447607fcade61cf148b1746b3062d4e","unresolved":false,"context_lines":[{"line_number":6639,"context_line":"            provider_tree.update_inventory(pgpu_rp_name, pgpu_inventory)"},{"line_number":6640,"context_line":""},{"line_number":6641,"context_line":"        if allocations is not None:"},{"line_number":6642,"context_line":"            LOG.info(\u0027Reshaping tree; moving VGPU allocations from root \u0027"},{"line_number":6643,"context_line":"                     \u0027provider %s to child providers %s.\u0027, root_node.uuid,"},{"line_number":6644,"context_line":"                     pgpu_rps.values())"},{"line_number":6645,"context_line":"            # For each consumer in the allocations dict, look for VGPU"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_87f59bcb","line":6642,"updated":"2018-11-22 20:53:00.000000000","message":"Start a new method here - name it reshape() or something.","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b216086e399d00d0d3d73d9336e74b4d96eb8ff4","unresolved":false,"context_lines":[{"line_number":6639,"context_line":"            provider_tree.update_inventory(pgpu_rp_name, pgpu_inventory)"},{"line_number":6640,"context_line":""},{"line_number":6641,"context_line":"        if allocations is not None:"},{"line_number":6642,"context_line":"            LOG.info(\u0027Reshaping tree; moving VGPU allocations from root \u0027"},{"line_number":6643,"context_line":"                     \u0027provider %s to child providers %s.\u0027, root_node.uuid,"},{"line_number":6644,"context_line":"                     pgpu_rps.values())"},{"line_number":6645,"context_line":"            # For each consumer in the allocations dict, look for VGPU"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_3cf673fb","line":6642,"in_reply_to":"3f79a3b5_87f59bcb","updated":"2019-01-23 23:23:59.000000000","message":"Done","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"75bcda430447607fcade61cf148b1746b3062d4e","unresolved":false,"context_lines":[{"line_number":6651,"context_line":"                for rp_uuid in list(allocs):"},{"line_number":6652,"context_line":"                    resources \u003d allocs[rp_uuid][\u0027resources\u0027]"},{"line_number":6653,"context_line":"                    if rc_fields.ResourceClass.VGPU in resources:"},{"line_number":6654,"context_line":"                        # We\u0027ve found VGPU allocations on a provider. It"},{"line_number":6655,"context_line":"                        # should be the root node provider."},{"line_number":6656,"context_line":"                        if rp_uuid !\u003d root_node.uuid:"},{"line_number":6657,"context_line":"                            # Something is wrong - VGPU inventory should"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_47d22335","line":6654,"updated":"2018-11-22 20:53:00.000000000","message":"Start a new method here and call it from reshape(). Name it update_allocations() or something.","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b216086e399d00d0d3d73d9336e74b4d96eb8ff4","unresolved":false,"context_lines":[{"line_number":6651,"context_line":"                for rp_uuid in list(allocs):"},{"line_number":6652,"context_line":"                    resources \u003d allocs[rp_uuid][\u0027resources\u0027]"},{"line_number":6653,"context_line":"                    if rc_fields.ResourceClass.VGPU in resources:"},{"line_number":6654,"context_line":"                        # We\u0027ve found VGPU allocations on a provider. It"},{"line_number":6655,"context_line":"                        # should be the root node provider."},{"line_number":6656,"context_line":"                        if rp_uuid !\u003d root_node.uuid:"},{"line_number":6657,"context_line":"                            # Something is wrong - VGPU inventory should"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_3c12f30a","line":6654,"in_reply_to":"3f79a3b5_47d22335","updated":"2019-01-23 23:23:59.000000000","message":"Done","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"75bcda430447607fcade61cf148b1746b3062d4e","unresolved":false,"context_lines":[{"line_number":6653,"context_line":"                    if rc_fields.ResourceClass.VGPU in resources:"},{"line_number":6654,"context_line":"                        # We\u0027ve found VGPU allocations on a provider. It"},{"line_number":6655,"context_line":"                        # should be the root node provider."},{"line_number":6656,"context_line":"                        if rp_uuid !\u003d root_node.uuid:"},{"line_number":6657,"context_line":"                            # Something is wrong - VGPU inventory should"},{"line_number":6658,"context_line":"                            # only be on the root node provider if we are"},{"line_number":6659,"context_line":"                            # reshaping the tree."},{"line_number":6660,"context_line":"                            msg \u003d (_(\u0027Unexpected VGPU resource allocation \u0027"},{"line_number":6661,"context_line":"                                     \u0027on provider %(rp_uuid)s for consumer \u0027"},{"line_number":6662,"context_line":"                                     \u0027%(consumer_uuid)s: %(alloc_data)s. \u0027"},{"line_number":6663,"context_line":"                                     \u0027Expected VGPU allocation to be on root \u0027"},{"line_number":6664,"context_line":"                                     \u0027compute node provider %(root_uuid)s.\u0027)"},{"line_number":6665,"context_line":"                                   % {\u0027rp_uuid\u0027: rp_uuid,"},{"line_number":6666,"context_line":"                                      \u0027consumer_uuid\u0027: consumer_uuid,"},{"line_number":6667,"context_line":"                                      \u0027alloc_data\u0027: alloc_data,"},{"line_number":6668,"context_line":"                                      \u0027root_uuid\u0027: root_node.uuid})"},{"line_number":6669,"context_line":"                            raise exception.ReshapeFailed(error\u003dmsg)"},{"line_number":6670,"context_line":"                        # Find which physical GPU corresponds to this"},{"line_number":6671,"context_line":"                        # allocation"},{"line_number":6672,"context_line":"                        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_27b607fe","line":6669,"range":{"start_line":6656,"start_character":0,"end_line":6669,"end_character":68},"updated":"2018-11-22 20:53:00.000000000","message":"This can be a check_inventory_is_on_root() helper.","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b216086e399d00d0d3d73d9336e74b4d96eb8ff4","unresolved":false,"context_lines":[{"line_number":6653,"context_line":"                    if rc_fields.ResourceClass.VGPU in resources:"},{"line_number":6654,"context_line":"                        # We\u0027ve found VGPU allocations on a provider. It"},{"line_number":6655,"context_line":"                        # should be the root node provider."},{"line_number":6656,"context_line":"                        if rp_uuid !\u003d root_node.uuid:"},{"line_number":6657,"context_line":"                            # Something is wrong - VGPU inventory should"},{"line_number":6658,"context_line":"                            # only be on the root node provider if we are"},{"line_number":6659,"context_line":"                            # reshaping the tree."},{"line_number":6660,"context_line":"                            msg \u003d (_(\u0027Unexpected VGPU resource allocation \u0027"},{"line_number":6661,"context_line":"                                     \u0027on provider %(rp_uuid)s for consumer \u0027"},{"line_number":6662,"context_line":"                                     \u0027%(consumer_uuid)s: %(alloc_data)s. \u0027"},{"line_number":6663,"context_line":"                                     \u0027Expected VGPU allocation to be on root \u0027"},{"line_number":6664,"context_line":"                                     \u0027compute node provider %(root_uuid)s.\u0027)"},{"line_number":6665,"context_line":"                                   % {\u0027rp_uuid\u0027: rp_uuid,"},{"line_number":6666,"context_line":"                                      \u0027consumer_uuid\u0027: consumer_uuid,"},{"line_number":6667,"context_line":"                                      \u0027alloc_data\u0027: alloc_data,"},{"line_number":6668,"context_line":"                                      \u0027root_uuid\u0027: root_node.uuid})"},{"line_number":6669,"context_line":"                            raise exception.ReshapeFailed(error\u003dmsg)"},{"line_number":6670,"context_line":"                        # Find which physical GPU corresponds to this"},{"line_number":6671,"context_line":"                        # allocation"},{"line_number":6672,"context_line":"                        try:"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_3ced138b","line":6669,"range":{"start_line":6656,"start_character":0,"end_line":6669,"end_character":68},"in_reply_to":"3f79a3b5_27b607fe","updated":"2019-01-23 23:23:59.000000000","message":"Done, but seems kind of excessive at this point to be creating a separate method for a one line check...","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b216086e399d00d0d3d73d9336e74b4d96eb8ff4","unresolved":false,"context_lines":[{"line_number":6676,"context_line":"                            Instance \u003d collections.namedtuple("},{"line_number":6677,"context_line":"                                \u0027Instance\u0027, [\u0027uuid\u0027, \u0027name\u0027])"},{"line_number":6678,"context_line":"                            instance \u003d Instance(uuid\u003dconsumer_uuid,"},{"line_number":6679,"context_line":"                                                name\u003d\u0027fake\u0027)"},{"line_number":6680,"context_line":"                            mdevs \u003d self._get_all_assigned_mediated_devices("},{"line_number":6681,"context_line":"                                instance)"},{"line_number":6682,"context_line":"                        except exception.InstanceNotFound:"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_dccf277d","line":6679,"updated":"2019-01-23 23:23:59.000000000","message":"Why not just use name\u003dconsumer_uuid here as well? The long-term fix is probably to change Host.get_guest to take an instance.uuid rather than an Instance object.","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b216086e399d00d0d3d73d9336e74b4d96eb8ff4","unresolved":false,"context_lines":[{"line_number":6679,"context_line":"                                                name\u003d\u0027fake\u0027)"},{"line_number":6680,"context_line":"                            mdevs \u003d self._get_all_assigned_mediated_devices("},{"line_number":6681,"context_line":"                                instance)"},{"line_number":6682,"context_line":"                        except exception.InstanceNotFound:"},{"line_number":6683,"context_line":"                            # If we found a VGPU allocation against a consumer"},{"line_number":6684,"context_line":"                            # which is not an instance, the only left case for"},{"line_number":6685,"context_line":"                            # Nova would be a migration but we don\u0027t support"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_5cbc97db","line":6682,"updated":"2019-01-23 23:23:59.000000000","message":"I don\u0027t think we can hit this InstanceNotFound can we? _get_all_assigned_mediated_devices handles InstanceNotFound and returns an empty dict.","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"75bcda430447607fcade61cf148b1746b3062d4e","unresolved":false,"context_lines":[{"line_number":6667,"context_line":"                                      \u0027alloc_data\u0027: alloc_data,"},{"line_number":6668,"context_line":"                                      \u0027root_uuid\u0027: root_node.uuid})"},{"line_number":6669,"context_line":"                            raise exception.ReshapeFailed(error\u003dmsg)"},{"line_number":6670,"context_line":"                        # Find which physical GPU corresponds to this"},{"line_number":6671,"context_line":"                        # allocation"},{"line_number":6672,"context_line":"                        try:"},{"line_number":6673,"context_line":"                            # FIXME(sbauza): We don\u0027t really need an Instance"},{"line_number":6674,"context_line":"                            # object, but given some libvirt.host logs needs"},{"line_number":6675,"context_line":"                            # to have an instance name, just provide a fake one"},{"line_number":6676,"context_line":"                            Instance \u003d collections.namedtuple("},{"line_number":6677,"context_line":"                                \u0027Instance\u0027, [\u0027uuid\u0027, \u0027name\u0027])"},{"line_number":6678,"context_line":"                            instance \u003d Instance(uuid\u003dconsumer_uuid,"},{"line_number":6679,"context_line":"                                                name\u003d\u0027fake\u0027)"},{"line_number":6680,"context_line":"                            mdevs \u003d self._get_all_assigned_mediated_devices("},{"line_number":6681,"context_line":"                                instance)"},{"line_number":6682,"context_line":"                        except exception.InstanceNotFound:"},{"line_number":6683,"context_line":"                            # If we found a VGPU allocation against a consumer"},{"line_number":6684,"context_line":"                            # which is not an instance, the only left case for"},{"line_number":6685,"context_line":"                            # Nova would be a migration but we don\u0027t support"},{"line_number":6686,"context_line":"                            # this at the moment."},{"line_number":6687,"context_line":"                            msg \u003d (_(\u0027Unexpected VGPU resource allocation \u0027"},{"line_number":6688,"context_line":"                                     \u0027on provider %(rp_uuid)s for consumer \u0027"},{"line_number":6689,"context_line":"                                     \u0027%(consumer_uuid)s: %(alloc_data)s. \u0027"},{"line_number":6690,"context_line":"                                     \u0027The allocation is made against a \u0027"},{"line_number":6691,"context_line":"                                     \u0027non-existing instance.\u0027)"},{"line_number":6692,"context_line":"                                   % {\u0027rp_uuid\u0027: rp_uuid,"},{"line_number":6693,"context_line":"                                      \u0027consumer_uuid\u0027: consumer_uuid,"},{"line_number":6694,"context_line":"                                      \u0027alloc_data\u0027: alloc_data})"},{"line_number":6695,"context_line":"                            raise exception.ReshapeFailed(error\u003dmsg)"},{"line_number":6696,"context_line":""},{"line_number":6697,"context_line":"                        vgpu_count_per_pgpu \u003d defaultdict(int)"},{"line_number":6698,"context_line":"                        for mdev_uuid in mdevs.keys():"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_07b18b00","line":6695,"range":{"start_line":6670,"start_character":0,"end_line":6695,"end_character":68},"updated":"2018-11-22 20:53:00.000000000","message":"This can be a get_mdevs_for_instance() helper that returns the mdevs from L6680.","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b216086e399d00d0d3d73d9336e74b4d96eb8ff4","unresolved":false,"context_lines":[{"line_number":6667,"context_line":"                                      \u0027alloc_data\u0027: alloc_data,"},{"line_number":6668,"context_line":"                                      \u0027root_uuid\u0027: root_node.uuid})"},{"line_number":6669,"context_line":"                            raise exception.ReshapeFailed(error\u003dmsg)"},{"line_number":6670,"context_line":"                        # Find which physical GPU corresponds to this"},{"line_number":6671,"context_line":"                        # allocation"},{"line_number":6672,"context_line":"                        try:"},{"line_number":6673,"context_line":"                            # FIXME(sbauza): We don\u0027t really need an Instance"},{"line_number":6674,"context_line":"                            # object, but given some libvirt.host logs needs"},{"line_number":6675,"context_line":"                            # to have an instance name, just provide a fake one"},{"line_number":6676,"context_line":"                            Instance \u003d collections.namedtuple("},{"line_number":6677,"context_line":"                                \u0027Instance\u0027, [\u0027uuid\u0027, \u0027name\u0027])"},{"line_number":6678,"context_line":"                            instance \u003d Instance(uuid\u003dconsumer_uuid,"},{"line_number":6679,"context_line":"                                                name\u003d\u0027fake\u0027)"},{"line_number":6680,"context_line":"                            mdevs \u003d self._get_all_assigned_mediated_devices("},{"line_number":6681,"context_line":"                                instance)"},{"line_number":6682,"context_line":"                        except exception.InstanceNotFound:"},{"line_number":6683,"context_line":"                            # If we found a VGPU allocation against a consumer"},{"line_number":6684,"context_line":"                            # which is not an instance, the only left case for"},{"line_number":6685,"context_line":"                            # Nova would be a migration but we don\u0027t support"},{"line_number":6686,"context_line":"                            # this at the moment."},{"line_number":6687,"context_line":"                            msg \u003d (_(\u0027Unexpected VGPU resource allocation \u0027"},{"line_number":6688,"context_line":"                                     \u0027on provider %(rp_uuid)s for consumer \u0027"},{"line_number":6689,"context_line":"                                     \u0027%(consumer_uuid)s: %(alloc_data)s. \u0027"},{"line_number":6690,"context_line":"                                     \u0027The allocation is made against a \u0027"},{"line_number":6691,"context_line":"                                     \u0027non-existing instance.\u0027)"},{"line_number":6692,"context_line":"                                   % {\u0027rp_uuid\u0027: rp_uuid,"},{"line_number":6693,"context_line":"                                      \u0027consumer_uuid\u0027: consumer_uuid,"},{"line_number":6694,"context_line":"                                      \u0027alloc_data\u0027: alloc_data})"},{"line_number":6695,"context_line":"                            raise exception.ReshapeFailed(error\u003dmsg)"},{"line_number":6696,"context_line":""},{"line_number":6697,"context_line":"                        vgpu_count_per_pgpu \u003d defaultdict(int)"},{"line_number":6698,"context_line":"                        for mdev_uuid in mdevs.keys():"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_ff3bfdc4","line":6695,"range":{"start_line":6670,"start_character":0,"end_line":6695,"end_character":68},"in_reply_to":"3f79a3b5_07b18b00","updated":"2019-01-23 23:23:59.000000000","message":"Done","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"75bcda430447607fcade61cf148b1746b3062d4e","unresolved":false,"context_lines":[{"line_number":6694,"context_line":"                                      \u0027alloc_data\u0027: alloc_data})"},{"line_number":6695,"context_line":"                            raise exception.ReshapeFailed(error\u003dmsg)"},{"line_number":6696,"context_line":""},{"line_number":6697,"context_line":"                        vgpu_count_per_pgpu \u003d defaultdict(int)"},{"line_number":6698,"context_line":"                        for mdev_uuid in mdevs.keys():"},{"line_number":6699,"context_line":"                            # libvirt name is like"},{"line_number":6700,"context_line":"                            # mdev_00ead764_fdc0_46b6_8db9_2963f5c815b4"},{"line_number":6701,"context_line":"                            dev_name \u003d \"mdev_\" + mdev_uuid.replace(\u0027-\u0027, \u0027_\u0027)"},{"line_number":6702,"context_line":"                            # Count how many vGPUs are in use for this instance"},{"line_number":6703,"context_line":"                            dev_info \u003d self._get_mediated_device_information("},{"line_number":6704,"context_line":"                                dev_name)"},{"line_number":6705,"context_line":"                            pgpu_dev_id \u003d dev_info[\u0027parent\u0027]"},{"line_number":6706,"context_line":"                            vgpu_count_per_pgpu[pgpu_dev_id] +\u003d 1"},{"line_number":6707,"context_line":"                        # We need to make sure we found all the mediated"},{"line_number":6708,"context_line":"                        # devices that correspond to an allocation"},{"line_number":6709,"context_line":"                        if (sum(vgpu_count_per_pgpu.values()) !\u003d"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_67bf1fcb","line":6706,"range":{"start_line":6697,"start_character":0,"end_line":6706,"end_character":65},"updated":"2018-11-22 20:53:00.000000000","message":"This could be count_vgpus_per_pgpu() that returns the vgpu_count_per_pgpu dict.","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b216086e399d00d0d3d73d9336e74b4d96eb8ff4","unresolved":false,"context_lines":[{"line_number":6694,"context_line":"                                      \u0027alloc_data\u0027: alloc_data})"},{"line_number":6695,"context_line":"                            raise exception.ReshapeFailed(error\u003dmsg)"},{"line_number":6696,"context_line":""},{"line_number":6697,"context_line":"                        vgpu_count_per_pgpu \u003d defaultdict(int)"},{"line_number":6698,"context_line":"                        for mdev_uuid in mdevs.keys():"},{"line_number":6699,"context_line":"                            # libvirt name is like"},{"line_number":6700,"context_line":"                            # mdev_00ead764_fdc0_46b6_8db9_2963f5c815b4"},{"line_number":6701,"context_line":"                            dev_name \u003d \"mdev_\" + mdev_uuid.replace(\u0027-\u0027, \u0027_\u0027)"},{"line_number":6702,"context_line":"                            # Count how many vGPUs are in use for this instance"},{"line_number":6703,"context_line":"                            dev_info \u003d self._get_mediated_device_information("},{"line_number":6704,"context_line":"                                dev_name)"},{"line_number":6705,"context_line":"                            pgpu_dev_id \u003d dev_info[\u0027parent\u0027]"},{"line_number":6706,"context_line":"                            vgpu_count_per_pgpu[pgpu_dev_id] +\u003d 1"},{"line_number":6707,"context_line":"                        # We need to make sure we found all the mediated"},{"line_number":6708,"context_line":"                        # devices that correspond to an allocation"},{"line_number":6709,"context_line":"                        if (sum(vgpu_count_per_pgpu.values()) !\u003d"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_5f9b29c8","line":6706,"range":{"start_line":6697,"start_character":0,"end_line":6706,"end_character":65},"in_reply_to":"3f79a3b5_67bf1fcb","updated":"2019-01-23 23:23:59.000000000","message":"Done","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"75bcda430447607fcade61cf148b1746b3062d4e","unresolved":false,"context_lines":[{"line_number":6704,"context_line":"                                dev_name)"},{"line_number":6705,"context_line":"                            pgpu_dev_id \u003d dev_info[\u0027parent\u0027]"},{"line_number":6706,"context_line":"                            vgpu_count_per_pgpu[pgpu_dev_id] +\u003d 1"},{"line_number":6707,"context_line":"                        # We need to make sure we found all the mediated"},{"line_number":6708,"context_line":"                        # devices that correspond to an allocation"},{"line_number":6709,"context_line":"                        if (sum(vgpu_count_per_pgpu.values()) !\u003d"},{"line_number":6710,"context_line":"                                resources[rc_fields.ResourceClass.VGPU]):"},{"line_number":6711,"context_line":"                            msg \u003d (\u0027Unexpected VGPU resource allocation \u0027"},{"line_number":6712,"context_line":"                                   \u0027on provider %(rp_uuid)s for consumer \u0027"},{"line_number":6713,"context_line":"                                   \u0027%(consumer_uuid)s: %(alloc_data)s. \u0027"},{"line_number":6714,"context_line":"                                   \u0027Allocations differ from actual use.\u0027 % {"},{"line_number":6715,"context_line":"                                       \u0027rp_uuid\u0027: rp_uuid,"},{"line_number":6716,"context_line":"                                       \u0027consumer_uuid\u0027: consumer_uuid,"},{"line_number":6717,"context_line":"                                       \u0027alloc_data\u0027: alloc_data})"},{"line_number":6718,"context_line":"                            # Don\u0027t make it blocking, just make sure you"},{"line_number":6719,"context_line":"                            # actually correcly allocate the existing resources"},{"line_number":6720,"context_line":"                            LOG.warning(msg)"},{"line_number":6721,"context_line":"                        # Add the VGPU allocation for each VGPU provider"},{"line_number":6722,"context_line":"                        for pgpu_dev_id, pgpu_rp in pgpu_rps.items():"},{"line_number":6723,"context_line":"                            if vgpu_count_per_pgpu[pgpu_dev_id] \u003e 0:"}],"source_content_type":"text/x-python","patch_set":10,"id":"3f79a3b5_6794ff42","line":6720,"range":{"start_line":6707,"start_character":0,"end_line":6720,"end_character":44},"updated":"2018-11-22 20:53:00.000000000","message":"This could be check_allocations_match_real_use()","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"b216086e399d00d0d3d73d9336e74b4d96eb8ff4","unresolved":false,"context_lines":[{"line_number":6704,"context_line":"                                dev_name)"},{"line_number":6705,"context_line":"                            pgpu_dev_id \u003d dev_info[\u0027parent\u0027]"},{"line_number":6706,"context_line":"                            vgpu_count_per_pgpu[pgpu_dev_id] +\u003d 1"},{"line_number":6707,"context_line":"                        # We need to make sure we found all the mediated"},{"line_number":6708,"context_line":"                        # devices that correspond to an allocation"},{"line_number":6709,"context_line":"                        if (sum(vgpu_count_per_pgpu.values()) !\u003d"},{"line_number":6710,"context_line":"                                resources[rc_fields.ResourceClass.VGPU]):"},{"line_number":6711,"context_line":"                            msg \u003d (\u0027Unexpected VGPU resource allocation \u0027"},{"line_number":6712,"context_line":"                                   \u0027on provider %(rp_uuid)s for consumer \u0027"},{"line_number":6713,"context_line":"                                   \u0027%(consumer_uuid)s: %(alloc_data)s. \u0027"},{"line_number":6714,"context_line":"                                   \u0027Allocations differ from actual use.\u0027 % {"},{"line_number":6715,"context_line":"                                       \u0027rp_uuid\u0027: rp_uuid,"},{"line_number":6716,"context_line":"                                       \u0027consumer_uuid\u0027: consumer_uuid,"},{"line_number":6717,"context_line":"                                       \u0027alloc_data\u0027: alloc_data})"},{"line_number":6718,"context_line":"                            # Don\u0027t make it blocking, just make sure you"},{"line_number":6719,"context_line":"                            # actually correcly allocate the existing resources"},{"line_number":6720,"context_line":"                            LOG.warning(msg)"},{"line_number":6721,"context_line":"                        # Add the VGPU allocation for each VGPU provider"},{"line_number":6722,"context_line":"                        for pgpu_dev_id, pgpu_rp in pgpu_rps.items():"},{"line_number":6723,"context_line":"                            if vgpu_count_per_pgpu[pgpu_dev_id] \u003e 0:"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_7f9e2db9","line":6720,"range":{"start_line":6707,"start_character":0,"end_line":6720,"end_character":44},"in_reply_to":"3f79a3b5_6794ff42","updated":"2019-01-23 23:23:59.000000000","message":"Done","commit_id":"81d82d1560a04b06f0903b934e915a38d4ae610c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ff9049938920d1250d5ec4553285daae5b712d89","unresolved":false,"context_lines":[{"line_number":5839,"context_line":"        requested_types \u003d CONF.devices.enabled_vgpu_types[:1]"},{"line_number":5840,"context_line":"        return requested_types"},{"line_number":5841,"context_line":""},{"line_number":5842,"context_line":"    def _get_vgpu_total(self):"},{"line_number":5843,"context_line":"        \"\"\"Returns the number of total available vGPUs for any GPU type that is"},{"line_number":5844,"context_line":"        enabled with the enabled_vgpu_types CONF option."},{"line_number":5845,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":11,"id":"9fdfeff1_ffe1bdfd","side":"PARENT","line":5842,"range":{"start_line":5842,"start_character":8,"end_line":5842,"end_character":23},"updated":"2019-01-23 23:36:57.000000000","message":"More test code needs to be removed with this.","commit_id":"56811efa3583dfa3c03f9e43a9802bbe21e45bbd"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2bb01f01f4a2bc6cf2b2c1dc8cf746b004e0cb18","unresolved":false,"context_lines":[{"line_number":26,"context_line":"\"\"\""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"import binascii"},{"line_number":29,"context_line":"import collections"},{"line_number":30,"context_line":"from collections import defaultdict"},{"line_number":31,"context_line":"from collections import deque"},{"line_number":32,"context_line":"import contextlib"},{"line_number":33,"context_line":"import errno"},{"line_number":34,"context_line":"import functools"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_e77dab1c","line":31,"range":{"start_line":29,"start_character":0,"end_line":31,"end_character":29},"updated":"2019-01-24 15:52:17.000000000","message":"femto-nit: we\u0027re already importing collections. Not entirely sure why we couldn\u0027t just do uses collections.defaultdict instead of importing an object from the collections module.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a16dd9be40d7af0793ad8a7853c2ae6cd455bd6e","unresolved":false,"context_lines":[{"line_number":26,"context_line":"\"\"\""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"import binascii"},{"line_number":29,"context_line":"import collections"},{"line_number":30,"context_line":"from collections import defaultdict"},{"line_number":31,"context_line":"from collections import deque"},{"line_number":32,"context_line":"import contextlib"},{"line_number":33,"context_line":"import errno"},{"line_number":34,"context_line":"import functools"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_a6fef2ab","line":31,"range":{"start_line":29,"start_character":0,"end_line":31,"end_character":29},"in_reply_to":"9fdfeff1_527ac0b9","updated":"2019-02-08 10:37:58.000000000","message":"Done","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4b91de98397a35a6c34a8e72927b8180cfd830b3","unresolved":false,"context_lines":[{"line_number":26,"context_line":"\"\"\""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"import binascii"},{"line_number":29,"context_line":"import collections"},{"line_number":30,"context_line":"from collections import defaultdict"},{"line_number":31,"context_line":"from collections import deque"},{"line_number":32,"context_line":"import contextlib"},{"line_number":33,"context_line":"import errno"},{"line_number":34,"context_line":"import functools"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_527ac0b9","line":31,"range":{"start_line":29,"start_character":0,"end_line":31,"end_character":29},"in_reply_to":"9fdfeff1_e77dab1c","updated":"2019-01-31 15:16:37.000000000","message":"Yeah, we could","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2bb01f01f4a2bc6cf2b2c1dc8cf746b004e0cb18","unresolved":false,"context_lines":[{"line_number":5842,"context_line":""},{"line_number":5843,"context_line":"    def _count_mediated_devices(self, enabled_vgpu_types):"},{"line_number":5844,"context_line":"        \"\"\"Counts the existing mediated devices that have been set in sysfs"},{"line_number":5845,"context_line":"        filtered by $enabled_vgpu_types (but possibly not assigned to a guest,"},{"line_number":5846,"context_line":"        ie. the guest descriptor not yet using it)"},{"line_number":5847,"context_line":""},{"line_number":5848,"context_line":"        :param enabled_vgpu_types: list of enabled VGPU types on this host"},{"line_number":5849,"context_line":"        :returns: dict, keyed by the physical GPU PCI device, to an integer"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_a777a33a","line":5846,"range":{"start_line":5845,"start_character":39,"end_line":5846,"end_character":50},"updated":"2019-01-24 15:52:17.000000000","message":"this part of the docstring is not necessary and really just makes it confusing.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a16dd9be40d7af0793ad8a7853c2ae6cd455bd6e","unresolved":false,"context_lines":[{"line_number":5842,"context_line":""},{"line_number":5843,"context_line":"    def _count_mediated_devices(self, enabled_vgpu_types):"},{"line_number":5844,"context_line":"        \"\"\"Counts the existing mediated devices that have been set in sysfs"},{"line_number":5845,"context_line":"        filtered by $enabled_vgpu_types (but possibly not assigned to a guest,"},{"line_number":5846,"context_line":"        ie. the guest descriptor not yet using it)"},{"line_number":5847,"context_line":""},{"line_number":5848,"context_line":"        :param enabled_vgpu_types: list of enabled VGPU types on this host"},{"line_number":5849,"context_line":"        :returns: dict, keyed by the physical GPU PCI device, to an integer"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_86016eb2","line":5846,"range":{"start_line":5845,"start_character":39,"end_line":5846,"end_character":50},"in_reply_to":"9fdfeff1_3a9713ef","updated":"2019-02-08 10:37:58.000000000","message":"Done","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"674804f16132be3f40211aee17d9aaf05e87676d","unresolved":false,"context_lines":[{"line_number":5842,"context_line":""},{"line_number":5843,"context_line":"    def _count_mediated_devices(self, enabled_vgpu_types):"},{"line_number":5844,"context_line":"        \"\"\"Counts the existing mediated devices that have been set in sysfs"},{"line_number":5845,"context_line":"        filtered by $enabled_vgpu_types (but possibly not assigned to a guest,"},{"line_number":5846,"context_line":"        ie. the guest descriptor not yet using it)"},{"line_number":5847,"context_line":""},{"line_number":5848,"context_line":"        :param enabled_vgpu_types: list of enabled VGPU types on this host"},{"line_number":5849,"context_line":"        :returns: dict, keyed by the physical GPU PCI device, to an integer"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_3a9713ef","line":5846,"range":{"start_line":5845,"start_character":39,"end_line":5846,"end_character":50},"in_reply_to":"9fdfeff1_a777a33a","updated":"2019-01-28 21:32:33.000000000","message":"I kinda like it, it helps clarify what is being counted.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2bb01f01f4a2bc6cf2b2c1dc8cf746b004e0cb18","unresolved":false,"context_lines":[{"line_number":5847,"context_line":""},{"line_number":5848,"context_line":"        :param enabled_vgpu_types: list of enabled VGPU types on this host"},{"line_number":5849,"context_line":"        :returns: dict, keyed by the physical GPU PCI device, to an integer"},{"line_number":5850,"context_line":"            count of each device"},{"line_number":5851,"context_line":"        \"\"\""},{"line_number":5852,"context_line":"        counts_per_parent \u003d {}"},{"line_number":5853,"context_line":"        mediated_devices \u003d self._get_mediated_devices(types\u003denabled_vgpu_types)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_e76ccb37","line":5850,"range":{"start_line":5850,"start_character":17,"end_line":5850,"end_character":32},"updated":"2019-01-24 15:52:17.000000000","message":"I think you meant \"dict, keyed by parent GPU libvirt PCI device ID, of number of mdev device handles for that GPU\"","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4efaf4ac12f644fe74a1f559ad9b0c3c9d13514a","unresolved":false,"context_lines":[{"line_number":5847,"context_line":""},{"line_number":5848,"context_line":"        :param enabled_vgpu_types: list of enabled VGPU types on this host"},{"line_number":5849,"context_line":"        :returns: dict, keyed by the physical GPU PCI device, to an integer"},{"line_number":5850,"context_line":"            count of each device"},{"line_number":5851,"context_line":"        \"\"\""},{"line_number":5852,"context_line":"        counts_per_parent \u003d {}"},{"line_number":5853,"context_line":"        mediated_devices \u003d self._get_mediated_devices(types\u003denabled_vgpu_types)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_203498fb","line":5850,"range":{"start_line":5850,"start_character":17,"end_line":5850,"end_character":32},"in_reply_to":"9fdfeff1_1aa24f4c","updated":"2019-01-30 22:08:28.000000000","message":"\u003e Also, alignment?\n \u003e \n \u003e :returns blah: blah blah\n \u003e blah\n\n\u003clater, after reading more code further down\u003e Ah, you just indent extra lines by a single tab. Ignore me then :)","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a16dd9be40d7af0793ad8a7853c2ae6cd455bd6e","unresolved":false,"context_lines":[{"line_number":5847,"context_line":""},{"line_number":5848,"context_line":"        :param enabled_vgpu_types: list of enabled VGPU types on this host"},{"line_number":5849,"context_line":"        :returns: dict, keyed by the physical GPU PCI device, to an integer"},{"line_number":5850,"context_line":"            count of each device"},{"line_number":5851,"context_line":"        \"\"\""},{"line_number":5852,"context_line":"        counts_per_parent \u003d {}"},{"line_number":5853,"context_line":"        mediated_devices \u003d self._get_mediated_devices(types\u003denabled_vgpu_types)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_e6f4fad1","line":5850,"range":{"start_line":5850,"start_character":17,"end_line":5850,"end_character":32},"in_reply_to":"9fdfeff1_32585c54","updated":"2019-02-08 10:37:58.000000000","message":"Done","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4b91de98397a35a6c34a8e72927b8180cfd830b3","unresolved":false,"context_lines":[{"line_number":5847,"context_line":""},{"line_number":5848,"context_line":"        :param enabled_vgpu_types: list of enabled VGPU types on this host"},{"line_number":5849,"context_line":"        :returns: dict, keyed by the physical GPU PCI device, to an integer"},{"line_number":5850,"context_line":"            count of each device"},{"line_number":5851,"context_line":"        \"\"\""},{"line_number":5852,"context_line":"        counts_per_parent \u003d {}"},{"line_number":5853,"context_line":"        mediated_devices \u003d self._get_mediated_devices(types\u003denabled_vgpu_types)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_32585c54","line":5850,"range":{"start_line":5850,"start_character":17,"end_line":5850,"end_character":32},"in_reply_to":"9fdfeff1_e76ccb37","updated":"2019-01-31 15:16:37.000000000","message":"\u003e I think you meant \"dict, keyed by parent GPU libvirt PCI device ID,\n \u003e of number of mdev device handles for that GPU\"\n\nYeah, thanks for the better semantics ;)","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"674804f16132be3f40211aee17d9aaf05e87676d","unresolved":false,"context_lines":[{"line_number":5847,"context_line":""},{"line_number":5848,"context_line":"        :param enabled_vgpu_types: list of enabled VGPU types on this host"},{"line_number":5849,"context_line":"        :returns: dict, keyed by the physical GPU PCI device, to an integer"},{"line_number":5850,"context_line":"            count of each device"},{"line_number":5851,"context_line":"        \"\"\""},{"line_number":5852,"context_line":"        counts_per_parent \u003d {}"},{"line_number":5853,"context_line":"        mediated_devices \u003d self._get_mediated_devices(types\u003denabled_vgpu_types)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_1aa24f4c","line":5850,"range":{"start_line":5850,"start_character":17,"end_line":5850,"end_character":32},"in_reply_to":"9fdfeff1_e76ccb37","updated":"2019-01-28 21:32:33.000000000","message":"Also, alignment?\n\n  :returns blah: blah blah\n                 blah","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2bb01f01f4a2bc6cf2b2c1dc8cf746b004e0cb18","unresolved":false,"context_lines":[{"line_number":5849,"context_line":"        :returns: dict, keyed by the physical GPU PCI device, to an integer"},{"line_number":5850,"context_line":"            count of each device"},{"line_number":5851,"context_line":"        \"\"\""},{"line_number":5852,"context_line":"        counts_per_parent \u003d {}"},{"line_number":5853,"context_line":"        mediated_devices \u003d self._get_mediated_devices(types\u003denabled_vgpu_types)"},{"line_number":5854,"context_line":"        for mdev in mediated_devices:"},{"line_number":5855,"context_line":"            parent \u003d mdev[\u0027parent\u0027]"},{"line_number":5856,"context_line":"            counts_per_parent.setdefault(parent, 0)"},{"line_number":5857,"context_line":"            counts_per_parent[parent] +\u003d 1"},{"line_number":5858,"context_line":"        return counts_per_parent"},{"line_number":5859,"context_line":""},{"line_number":5860,"context_line":"    def _count_mdev_capable_devices(self, enabled_vgpu_types):"},{"line_number":5861,"context_line":"        \"\"\"Counts the mdev-capable devices on this host filtered by"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_a74fa3bb","line":5858,"range":{"start_line":5852,"start_character":0,"end_line":5858,"end_character":32},"updated":"2019-01-24 15:52:17.000000000","message":"could have used a collections.defaultdict to make this simpler:\n\n counts_per_parent \u003d collections.defaultdict(int)\n for mdev in self._get_mediated_devices(types\u003denabled_vgpu_types):\n     counts_per_parent[mdev[\u0027parent\u0027]] +\u003d 1\n return counts_per_parent","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"4b91de98397a35a6c34a8e72927b8180cfd830b3","unresolved":false,"context_lines":[{"line_number":5849,"context_line":"        :returns: dict, keyed by the physical GPU PCI device, to an integer"},{"line_number":5850,"context_line":"            count of each device"},{"line_number":5851,"context_line":"        \"\"\""},{"line_number":5852,"context_line":"        counts_per_parent \u003d {}"},{"line_number":5853,"context_line":"        mediated_devices \u003d self._get_mediated_devices(types\u003denabled_vgpu_types)"},{"line_number":5854,"context_line":"        for mdev in mediated_devices:"},{"line_number":5855,"context_line":"            parent \u003d mdev[\u0027parent\u0027]"},{"line_number":5856,"context_line":"            counts_per_parent.setdefault(parent, 0)"},{"line_number":5857,"context_line":"            counts_per_parent[parent] +\u003d 1"},{"line_number":5858,"context_line":"        return counts_per_parent"},{"line_number":5859,"context_line":""},{"line_number":5860,"context_line":"    def _count_mdev_capable_devices(self, enabled_vgpu_types):"},{"line_number":5861,"context_line":"        \"\"\"Counts the mdev-capable devices on this host filtered by"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_b239acf2","line":5858,"range":{"start_line":5852,"start_character":0,"end_line":5858,"end_character":32},"in_reply_to":"9fdfeff1_3200ca01","updated":"2019-01-31 15:16:37.000000000","message":"Good point","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6dac5a94544c54275555060becb887e6b322bf63","unresolved":false,"context_lines":[{"line_number":5849,"context_line":"        :returns: dict, keyed by the physical GPU PCI device, to an integer"},{"line_number":5850,"context_line":"            count of each device"},{"line_number":5851,"context_line":"        \"\"\""},{"line_number":5852,"context_line":"        counts_per_parent \u003d {}"},{"line_number":5853,"context_line":"        mediated_devices \u003d self._get_mediated_devices(types\u003denabled_vgpu_types)"},{"line_number":5854,"context_line":"        for mdev in mediated_devices:"},{"line_number":5855,"context_line":"            parent \u003d mdev[\u0027parent\u0027]"},{"line_number":5856,"context_line":"            counts_per_parent.setdefault(parent, 0)"},{"line_number":5857,"context_line":"            counts_per_parent[parent] +\u003d 1"},{"line_number":5858,"context_line":"        return counts_per_parent"},{"line_number":5859,"context_line":""},{"line_number":5860,"context_line":"    def _count_mdev_capable_devices(self, enabled_vgpu_types):"},{"line_number":5861,"context_line":"        \"\"\"Counts the mdev-capable devices on this host filtered by"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_3200ca01","line":5858,"range":{"start_line":5852,"start_character":0,"end_line":5858,"end_character":32},"in_reply_to":"9fdfeff1_a74fa3bb","updated":"2019-01-24 20:17:04.000000000","message":"Yeah I remember thinking I should go back and do that because collections.defaultdict(int) is used that way in another method later.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a16dd9be40d7af0793ad8a7853c2ae6cd455bd6e","unresolved":false,"context_lines":[{"line_number":5849,"context_line":"        :returns: dict, keyed by the physical GPU PCI device, to an integer"},{"line_number":5850,"context_line":"            count of each device"},{"line_number":5851,"context_line":"        \"\"\""},{"line_number":5852,"context_line":"        counts_per_parent \u003d {}"},{"line_number":5853,"context_line":"        mediated_devices \u003d self._get_mediated_devices(types\u003denabled_vgpu_types)"},{"line_number":5854,"context_line":"        for mdev in mediated_devices:"},{"line_number":5855,"context_line":"            parent \u003d mdev[\u0027parent\u0027]"},{"line_number":5856,"context_line":"            counts_per_parent.setdefault(parent, 0)"},{"line_number":5857,"context_line":"            counts_per_parent[parent] +\u003d 1"},{"line_number":5858,"context_line":"        return counts_per_parent"},{"line_number":5859,"context_line":""},{"line_number":5860,"context_line":"    def _count_mdev_capable_devices(self, enabled_vgpu_types):"},{"line_number":5861,"context_line":"        \"\"\"Counts the mdev-capable devices on this host filtered by"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_2613e20f","line":5858,"range":{"start_line":5852,"start_character":0,"end_line":5858,"end_character":32},"in_reply_to":"9fdfeff1_b239acf2","updated":"2019-02-08 10:37:58.000000000","message":"Done","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2bb01f01f4a2bc6cf2b2c1dc8cf746b004e0cb18","unresolved":false,"context_lines":[{"line_number":5880,"context_line":"                # NOTE(sbauza): Even if we support multiple types, Nova will"},{"line_number":5881,"context_line":"                # only use one per physical GPU."},{"line_number":5882,"context_line":"                counts_per_dev[dev_name] +\u003d available"},{"line_number":5883,"context_line":"        return counts_per_dev"},{"line_number":5884,"context_line":""},{"line_number":5885,"context_line":"    def _get_gpu_inventories(self):"},{"line_number":5886,"context_line":"        \"\"\"Returns the inventories for each physical GPU for a specific type"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_c74ae7aa","line":5883,"updated":"2019-01-24 15:52:17.000000000","message":"similar comment about using collections.defaultdict(int)","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a16dd9be40d7af0793ad8a7853c2ae6cd455bd6e","unresolved":false,"context_lines":[{"line_number":5880,"context_line":"                # NOTE(sbauza): Even if we support multiple types, Nova will"},{"line_number":5881,"context_line":"                # only use one per physical GPU."},{"line_number":5882,"context_line":"                counts_per_dev[dev_name] +\u003d available"},{"line_number":5883,"context_line":"        return counts_per_dev"},{"line_number":5884,"context_line":""},{"line_number":5885,"context_line":"    def _get_gpu_inventories(self):"},{"line_number":5886,"context_line":"        \"\"\"Returns the inventories for each physical GPU for a specific type"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_462dc644","line":5883,"in_reply_to":"9fdfeff1_c74ae7aa","updated":"2019-02-08 10:37:58.000000000","message":"Done","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f340723fa97ddca98f8230ebe2a672f13d1f9fab","unresolved":false,"context_lines":[{"line_number":5883,"context_line":"        return counts_per_dev"},{"line_number":5884,"context_line":""},{"line_number":5885,"context_line":"    def _get_gpu_inventories(self):"},{"line_number":5886,"context_line":"        \"\"\"Returns the inventories for each physical GPU for a specific type"},{"line_number":5887,"context_line":"        supported by the enabled_vgpu_types CONF option."},{"line_number":5888,"context_line":""},{"line_number":5889,"context_line":"        :returns: dict, keyed by libvirt PCI name, of dicts like:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_ada19496","line":5886,"range":{"start_line":5886,"start_character":57,"end_line":5886,"end_character":76},"updated":"2019-01-28 13:45:00.000000000","message":"How does this specific type is specified for this function? How does this appear in the return value?","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f340723fa97ddca98f8230ebe2a672f13d1f9fab","unresolved":false,"context_lines":[{"line_number":5895,"context_line":"                     \u0027reserved\u0027: 0,"},{"line_number":5896,"context_line":"                     \u0027allocation_ratio\u0027: 1.0,"},{"line_number":5897,"context_line":"                    }"},{"line_number":5898,"context_line":"                }"},{"line_number":5899,"context_line":"        \"\"\""},{"line_number":5900,"context_line":""},{"line_number":5901,"context_line":"        # Bail out early if operator doesn\u0027t care about providing vGPUs"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_0d91c047","line":5898,"updated":"2019-01-28 13:45:00.000000000","message":"This example feels incomplete to me. The code below does counting total two different ways.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a16dd9be40d7af0793ad8a7853c2ae6cd455bd6e","unresolved":false,"context_lines":[{"line_number":5895,"context_line":"                     \u0027reserved\u0027: 0,"},{"line_number":5896,"context_line":"                     \u0027allocation_ratio\u0027: 1.0,"},{"line_number":5897,"context_line":"                    }"},{"line_number":5898,"context_line":"                }"},{"line_number":5899,"context_line":"        \"\"\""},{"line_number":5900,"context_line":""},{"line_number":5901,"context_line":"        # Bail out early if operator doesn\u0027t care about providing vGPUs"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_5c402108","line":5898,"in_reply_to":"9fdfeff1_0d91c047","updated":"2019-02-08 10:37:58.000000000","message":"No, the code below merges two sources of information into one single dictionary which can be, as an example, the above.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2bb01f01f4a2bc6cf2b2c1dc8cf746b004e0cb18","unresolved":false,"context_lines":[{"line_number":5899,"context_line":"        \"\"\""},{"line_number":5900,"context_line":""},{"line_number":5901,"context_line":"        # Bail out early if operator doesn\u0027t care about providing vGPUs"},{"line_number":5902,"context_line":"        enabled_vgpu_types \u003d CONF.devices.enabled_vgpu_types"},{"line_number":5903,"context_line":"        if not enabled_vgpu_types:"},{"line_number":5904,"context_line":"            return {}"},{"line_number":5905,"context_line":"        inventories \u003d {}"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_2735f346","line":5902,"range":{"start_line":5902,"start_character":29,"end_line":5902,"end_character":60},"updated":"2019-01-24 15:52:17.000000000","message":"any reason not to continue using the self._get_supported_vgpu_types() method?","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6dac5a94544c54275555060becb887e6b322bf63","unresolved":false,"context_lines":[{"line_number":5899,"context_line":"        \"\"\""},{"line_number":5900,"context_line":""},{"line_number":5901,"context_line":"        # Bail out early if operator doesn\u0027t care about providing vGPUs"},{"line_number":5902,"context_line":"        enabled_vgpu_types \u003d CONF.devices.enabled_vgpu_types"},{"line_number":5903,"context_line":"        if not enabled_vgpu_types:"},{"line_number":5904,"context_line":"            return {}"},{"line_number":5905,"context_line":"        inventories \u003d {}"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_522c6e98","line":5902,"range":{"start_line":5902,"start_character":29,"end_line":5902,"end_character":60},"in_reply_to":"9fdfeff1_2735f346","updated":"2019-01-24 20:17:04.000000000","message":"Hmm, I don\u0027t think so - I think Sylvain wrote this part but just maybe wasn\u0027t thinking about using _get_supported_vgpu_types - we probably should though. I could post a follow up.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a16dd9be40d7af0793ad8a7853c2ae6cd455bd6e","unresolved":false,"context_lines":[{"line_number":5899,"context_line":"        \"\"\""},{"line_number":5900,"context_line":""},{"line_number":5901,"context_line":"        # Bail out early if operator doesn\u0027t care about providing vGPUs"},{"line_number":5902,"context_line":"        enabled_vgpu_types \u003d CONF.devices.enabled_vgpu_types"},{"line_number":5903,"context_line":"        if not enabled_vgpu_types:"},{"line_number":5904,"context_line":"            return {}"},{"line_number":5905,"context_line":"        inventories \u003d {}"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_66a56ab5","line":5902,"range":{"start_line":5902,"start_character":29,"end_line":5902,"end_character":60},"in_reply_to":"9fdfeff1_522c6e98","updated":"2019-02-08 10:37:58.000000000","message":"Done","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f340723fa97ddca98f8230ebe2a672f13d1f9fab","unresolved":false,"context_lines":[{"line_number":5903,"context_line":"        if not enabled_vgpu_types:"},{"line_number":5904,"context_line":"            return {}"},{"line_number":5905,"context_line":"        inventories \u003d {}"},{"line_number":5906,"context_line":"        count_per_parent \u003d self._count_mediated_devices(enabled_vgpu_types)"},{"line_number":5907,"context_line":"        for dev_name, count in count_per_parent.items():"},{"line_number":5908,"context_line":"            inventories[dev_name] \u003d {\u0027total\u0027: count}"},{"line_number":5909,"context_line":"        # Filter how many available mdevs we can create for all the supported"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_110c7a39","line":5906,"range":{"start_line":5906,"start_character":56,"end_line":5906,"end_character":74},"updated":"2019-01-28 13:45:00.000000000","message":"If we really want to restrict to support only one vgpu types then here we should call _get_supported_vgpu_types() to get only a single type we decided to support.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a16dd9be40d7af0793ad8a7853c2ae6cd455bd6e","unresolved":false,"context_lines":[{"line_number":5903,"context_line":"        if not enabled_vgpu_types:"},{"line_number":5904,"context_line":"            return {}"},{"line_number":5905,"context_line":"        inventories \u003d {}"},{"line_number":5906,"context_line":"        count_per_parent \u003d self._count_mediated_devices(enabled_vgpu_types)"},{"line_number":5907,"context_line":"        for dev_name, count in count_per_parent.items():"},{"line_number":5908,"context_line":"            inventories[dev_name] \u003d {\u0027total\u0027: count}"},{"line_number":5909,"context_line":"        # Filter how many available mdevs we can create for all the supported"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_26af6295","line":5906,"range":{"start_line":5906,"start_character":56,"end_line":5906,"end_character":74},"in_reply_to":"9fdfeff1_110c7a39","updated":"2019-02-08 10:37:58.000000000","message":"Done","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f340723fa97ddca98f8230ebe2a672f13d1f9fab","unresolved":false,"context_lines":[{"line_number":5908,"context_line":"            inventories[dev_name] \u003d {\u0027total\u0027: count}"},{"line_number":5909,"context_line":"        # Filter how many available mdevs we can create for all the supported"},{"line_number":5910,"context_line":"        # types."},{"line_number":5911,"context_line":"        count_per_dev \u003d self._count_mdev_capable_devices(enabled_vgpu_types)"},{"line_number":5912,"context_line":"        # Combine the counts into the dict that we return to the caller."},{"line_number":5913,"context_line":"        for dev_name, count in count_per_dev.items():"},{"line_number":5914,"context_line":"            inv_per_parent \u003d inventories.setdefault("}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_f11f7658","line":5911,"range":{"start_line":5911,"start_character":57,"end_line":5911,"end_character":75},"updated":"2019-01-28 13:45:00.000000000","message":"ditto: use _get_supported_vgpu_types() instead","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a16dd9be40d7af0793ad8a7853c2ae6cd455bd6e","unresolved":false,"context_lines":[{"line_number":5908,"context_line":"            inventories[dev_name] \u003d {\u0027total\u0027: count}"},{"line_number":5909,"context_line":"        # Filter how many available mdevs we can create for all the supported"},{"line_number":5910,"context_line":"        # types."},{"line_number":5911,"context_line":"        count_per_dev \u003d self._count_mdev_capable_devices(enabled_vgpu_types)"},{"line_number":5912,"context_line":"        # Combine the counts into the dict that we return to the caller."},{"line_number":5913,"context_line":"        for dev_name, count in count_per_dev.items():"},{"line_number":5914,"context_line":"            inv_per_parent \u003d inventories.setdefault("}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_c693f65b","line":5911,"range":{"start_line":5911,"start_character":57,"end_line":5911,"end_character":75},"in_reply_to":"9fdfeff1_f11f7658","updated":"2019-02-08 10:37:58.000000000","message":"Done","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f340723fa97ddca98f8230ebe2a672f13d1f9fab","unresolved":false,"context_lines":[{"line_number":5911,"context_line":"        count_per_dev \u003d self._count_mdev_capable_devices(enabled_vgpu_types)"},{"line_number":5912,"context_line":"        # Combine the counts into the dict that we return to the caller."},{"line_number":5913,"context_line":"        for dev_name, count in count_per_dev.items():"},{"line_number":5914,"context_line":"            inv_per_parent \u003d inventories.setdefault("},{"line_number":5915,"context_line":"                dev_name, {\u0027total\u0027: 0})"},{"line_number":5916,"context_line":"            inv_per_parent[\u0027total\u0027] +\u003d count"},{"line_number":5917,"context_line":"            inv_per_parent.update({"},{"line_number":5918,"context_line":"                \u0027min_unit\u0027: 1,"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_2d91244f","line":5915,"range":{"start_line":5914,"start_character":1,"end_line":5915,"end_character":39},"updated":"2019-01-28 13:45:00.000000000","message":"Cannot we assume that L5908 made sure that inventories[dev_name] already exists?","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a16dd9be40d7af0793ad8a7853c2ae6cd455bd6e","unresolved":false,"context_lines":[{"line_number":5911,"context_line":"        count_per_dev \u003d self._count_mdev_capable_devices(enabled_vgpu_types)"},{"line_number":5912,"context_line":"        # Combine the counts into the dict that we return to the caller."},{"line_number":5913,"context_line":"        for dev_name, count in count_per_dev.items():"},{"line_number":5914,"context_line":"            inv_per_parent \u003d inventories.setdefault("},{"line_number":5915,"context_line":"                dev_name, {\u0027total\u0027: 0})"},{"line_number":5916,"context_line":"            inv_per_parent[\u0027total\u0027] +\u003d count"},{"line_number":5917,"context_line":"            inv_per_parent.update({"},{"line_number":5918,"context_line":"                \u0027min_unit\u0027: 1,"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_fcb9d514","line":5915,"range":{"start_line":5914,"start_character":1,"end_line":5915,"end_character":39},"in_reply_to":"9fdfeff1_2d91244f","updated":"2019-02-08 10:37:58.000000000","message":"Not really, we count from two different sources. See artom\u0027s explanation before, but I\u0027ll try to make it clear :\n* first, we count the mediated devices (ie. the fake sysfs objects) and then we lookup their parent (the PCI address of the mdev-capable device). That\u0027s what we do L5906\n* secondly, we lookup the mdev-capable devices to see how many mdevs they can create (as *left resources* unfortunately, given \u0027total\u0027 is the number of mdevs we can create now, not what is the total amount of mdevs the device can allocate).\n\nThat\u0027s why I need to merge/zip those two sources into one single dict keyed by the PCI address of a mdev-capable device","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f340723fa97ddca98f8230ebe2a672f13d1f9fab","unresolved":false,"context_lines":[{"line_number":5925,"context_line":"                \u0027max_unit\u0027: inv_per_parent[\u0027total\u0027],"},{"line_number":5926,"context_line":"            })"},{"line_number":5927,"context_line":""},{"line_number":5928,"context_line":"        return inventories"},{"line_number":5929,"context_line":""},{"line_number":5930,"context_line":"    def _get_instance_capabilities(self):"},{"line_number":5931,"context_line":"        \"\"\"Get hypervisor instance capabilities"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_0dba20c0","line":5928,"updated":"2019-01-28 13:45:00.000000000","message":"I\u0027m a bit lost of the calculation here. It seems we are counting total value two different ways but I don\u0027t know why.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"674804f16132be3f40211aee17d9aaf05e87676d","unresolved":false,"context_lines":[{"line_number":5925,"context_line":"                \u0027max_unit\u0027: inv_per_parent[\u0027total\u0027],"},{"line_number":5926,"context_line":"            })"},{"line_number":5927,"context_line":""},{"line_number":5928,"context_line":"        return inventories"},{"line_number":5929,"context_line":""},{"line_number":5930,"context_line":"    def _get_instance_capabilities(self):"},{"line_number":5931,"context_line":"        \"\"\"Get hypervisor instance capabilities"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_b0b31f4d","line":5928,"in_reply_to":"9fdfeff1_0dba20c0","updated":"2019-01-28 21:32:33.000000000","message":"It\u0027s still hard to grok (the way my brain works, at least), but it makes sense:\n\nYou have MDEVs, which have already been allocated [1] - this is what _count_mediated_devices() counts.\n\nAnd you have MDEV-capable devices, with a certain number of availableInstances [2], which is what _count_mdev_capable_devices() counts.\n\nBecause the total # of devices is allocated MDEVs + availableInstances, we add those up.\n\nI don\u0027t know how much I want to haggle variable names and such, but here\u0027s what would make more sense to my brain:\n\n  available_instances \u003d _count_mdev_capable_devices() # same dict as before\n  allocated_devices \u003d _count_mediated_devices() # same dict as before\n\n  # first add those 2 together\n  for pccid, num_available in available_instances.items():\n    # loop over available_instances every mdev-capable device is in there\n    total_per_pcci[pciid] \u003d num_available + allocated_devices[pciid]\n\n  # and now populate the inventories dict:\n  for pciid, total in total_per_pciid.items():\n    inventories[pciid] \u003d {\n      min_unit: 1,\n      blah\n      total: total\n    }\n\n[1] https://libvirt.org/drvnodedev.html#MDEV\n[2] https://libvirt.org/drvnodedev.html#MDEVCap","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a16dd9be40d7af0793ad8a7853c2ae6cd455bd6e","unresolved":false,"context_lines":[{"line_number":5925,"context_line":"                \u0027max_unit\u0027: inv_per_parent[\u0027total\u0027],"},{"line_number":5926,"context_line":"            })"},{"line_number":5927,"context_line":""},{"line_number":5928,"context_line":"        return inventories"},{"line_number":5929,"context_line":""},{"line_number":5930,"context_line":"    def _get_instance_capabilities(self):"},{"line_number":5931,"context_line":"        \"\"\"Get hypervisor instance capabilities"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_3ccafd6d","line":5928,"in_reply_to":"9fdfeff1_b0b31f4d","updated":"2019-02-08 10:37:58.000000000","message":"There can be PCI addresses of mdev-capable devices that aren\u0027t yet having already created mdevs.\nFor that reason, we absoluty need to have two distinct loops (at O(N) complexity where N is the number of PCI addresses)","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f340723fa97ddca98f8230ebe2a672f13d1f9fab","unresolved":false,"context_lines":[{"line_number":6219,"context_line":"            for device in cfg.devices:"},{"line_number":6220,"context_line":"                if isinstance(device, vconfig.LibvirtConfigGuestHostdevMDEV):"},{"line_number":6221,"context_line":"                    allocated_mdevs[device.uuid] \u003d guest.uuid"},{"line_number":6222,"context_line":"        return allocated_mdevs"},{"line_number":6223,"context_line":""},{"line_number":6224,"context_line":"    @staticmethod"},{"line_number":6225,"context_line":"    def _vgpu_allocations(allocations):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_4dc34839","line":6222,"updated":"2019-01-28 13:45:00.000000000","message":"unrelated to this patch but this function feels like two separate function glued together by a conditional.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a16dd9be40d7af0793ad8a7853c2ae6cd455bd6e","unresolved":false,"context_lines":[{"line_number":6219,"context_line":"            for device in cfg.devices:"},{"line_number":6220,"context_line":"                if isinstance(device, vconfig.LibvirtConfigGuestHostdevMDEV):"},{"line_number":6221,"context_line":"                    allocated_mdevs[device.uuid] \u003d guest.uuid"},{"line_number":6222,"context_line":"        return allocated_mdevs"},{"line_number":6223,"context_line":""},{"line_number":6224,"context_line":"    @staticmethod"},{"line_number":6225,"context_line":"    def _vgpu_allocations(allocations):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_5caa6185","line":6222,"in_reply_to":"9fdfeff1_4dc34839","updated":"2019-02-08 10:37:58.000000000","message":"I don\u0027t disagree, we could address it in a follow-up","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f340723fa97ddca98f8230ebe2a672f13d1f9fab","unresolved":false,"context_lines":[{"line_number":6630,"context_line":"            \u0027reserved\u0027: self._get_reserved_host_disk_gb_from_config(),"},{"line_number":6631,"context_line":"        }"},{"line_number":6632,"context_line":""},{"line_number":6633,"context_line":"        # NOTE(sbauza): For the moment, the libvirt driver only supports"},{"line_number":6634,"context_line":"        # providing the total number of virtual GPUs for a single GPU type. If"},{"line_number":6635,"context_line":"        # you have multiple physical GPUs, each of them providing multiple GPU"},{"line_number":6636,"context_line":"        # types, only one type will be used for each of the physical GPUs."},{"line_number":6637,"context_line":"        # If one of the 2 pGPUs doesn\u0027t support \u0027nvidia-35\u0027, it won\u0027t be used."}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_6ddf2cd6","line":6634,"range":{"start_line":6633,"start_character":23,"end_line":6634,"end_character":74},"updated":"2019-01-28 13:45:00.000000000","message":"Where this is enforced? _get_mdev_capabilities_for_dev() allows more than one types.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a16dd9be40d7af0793ad8a7853c2ae6cd455bd6e","unresolved":false,"context_lines":[{"line_number":6630,"context_line":"            \u0027reserved\u0027: self._get_reserved_host_disk_gb_from_config(),"},{"line_number":6631,"context_line":"        }"},{"line_number":6632,"context_line":""},{"line_number":6633,"context_line":"        # NOTE(sbauza): For the moment, the libvirt driver only supports"},{"line_number":6634,"context_line":"        # providing the total number of virtual GPUs for a single GPU type. If"},{"line_number":6635,"context_line":"        # you have multiple physical GPUs, each of them providing multiple GPU"},{"line_number":6636,"context_line":"        # types, only one type will be used for each of the physical GPUs."},{"line_number":6637,"context_line":"        # If one of the 2 pGPUs doesn\u0027t support \u0027nvidia-35\u0027, it won\u0027t be used."}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_705989fc","line":6634,"range":{"start_line":6633,"start_character":23,"end_line":6634,"end_character":74},"in_reply_to":"9fdfeff1_6ddf2cd6","updated":"2019-02-08 10:37:58.000000000","message":"I forgot in L5902 to use _get_supported_vgpu_types() but now all the calls for getting the VGPU types will be using the method, which only supports only one type at the moment.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f340723fa97ddca98f8230ebe2a672f13d1f9fab","unresolved":false,"context_lines":[{"line_number":6633,"context_line":"        # NOTE(sbauza): For the moment, the libvirt driver only supports"},{"line_number":6634,"context_line":"        # providing the total number of virtual GPUs for a single GPU type. If"},{"line_number":6635,"context_line":"        # you have multiple physical GPUs, each of them providing multiple GPU"},{"line_number":6636,"context_line":"        # types, only one type will be used for each of the physical GPUs."},{"line_number":6637,"context_line":"        # If one of the 2 pGPUs doesn\u0027t support \u0027nvidia-35\u0027, it won\u0027t be used."},{"line_number":6638,"context_line":"        # TODO(sbauza): Use traits to make a better world."},{"line_number":6639,"context_line":"        inventories_dict \u003d self._get_gpu_inventories()"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_eda1dc47","line":6636,"range":{"start_line":6636,"start_character":17,"end_line":6636,"end_character":43},"updated":"2019-01-28 13:45:00.000000000","message":"Which one? The first in the list of types? Is this enforced somewhere?","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a16dd9be40d7af0793ad8a7853c2ae6cd455bd6e","unresolved":false,"context_lines":[{"line_number":6633,"context_line":"        # NOTE(sbauza): For the moment, the libvirt driver only supports"},{"line_number":6634,"context_line":"        # providing the total number of virtual GPUs for a single GPU type. If"},{"line_number":6635,"context_line":"        # you have multiple physical GPUs, each of them providing multiple GPU"},{"line_number":6636,"context_line":"        # types, only one type will be used for each of the physical GPUs."},{"line_number":6637,"context_line":"        # If one of the 2 pGPUs doesn\u0027t support \u0027nvidia-35\u0027, it won\u0027t be used."},{"line_number":6638,"context_line":"        # TODO(sbauza): Use traits to make a better world."},{"line_number":6639,"context_line":"        inventories_dict \u003d self._get_gpu_inventories()"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_506ec596","line":6636,"range":{"start_line":6636,"start_character":17,"end_line":6636,"end_character":43},"in_reply_to":"9fdfeff1_eda1dc47","updated":"2019-02-08 10:37:58.000000000","message":"Yup, L5838 and below.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2bb01f01f4a2bc6cf2b2c1dc8cf746b004e0cb18","unresolved":false,"context_lines":[{"line_number":6634,"context_line":"        # providing the total number of virtual GPUs for a single GPU type. If"},{"line_number":6635,"context_line":"        # you have multiple physical GPUs, each of them providing multiple GPU"},{"line_number":6636,"context_line":"        # types, only one type will be used for each of the physical GPUs."},{"line_number":6637,"context_line":"        # If one of the 2 pGPUs doesn\u0027t support \u0027nvidia-35\u0027, it won\u0027t be used."},{"line_number":6638,"context_line":"        # TODO(sbauza): Use traits to make a better world."},{"line_number":6639,"context_line":"        inventories_dict \u003d self._get_gpu_inventories()"},{"line_number":6640,"context_line":"        if inventories_dict:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_874fff90","line":6637,"range":{"start_line":6637,"start_character":10,"end_line":6637,"end_character":59},"updated":"2019-01-24 15:52:17.000000000","message":"is this really hard-coded to \u0027nvidia-35\u0027?","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a16dd9be40d7af0793ad8a7853c2ae6cd455bd6e","unresolved":false,"context_lines":[{"line_number":6634,"context_line":"        # providing the total number of virtual GPUs for a single GPU type. If"},{"line_number":6635,"context_line":"        # you have multiple physical GPUs, each of them providing multiple GPU"},{"line_number":6636,"context_line":"        # types, only one type will be used for each of the physical GPUs."},{"line_number":6637,"context_line":"        # If one of the 2 pGPUs doesn\u0027t support \u0027nvidia-35\u0027, it won\u0027t be used."},{"line_number":6638,"context_line":"        # TODO(sbauza): Use traits to make a better world."},{"line_number":6639,"context_line":"        inventories_dict \u003d self._get_gpu_inventories()"},{"line_number":6640,"context_line":"        if inventories_dict:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_30f8e1c2","line":6637,"range":{"start_line":6637,"start_character":10,"end_line":6637,"end_character":59},"in_reply_to":"9fdfeff1_874fff90","updated":"2019-02-08 10:37:58.000000000","message":"Done","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6dac5a94544c54275555060becb887e6b322bf63","unresolved":false,"context_lines":[{"line_number":6634,"context_line":"        # providing the total number of virtual GPUs for a single GPU type. If"},{"line_number":6635,"context_line":"        # you have multiple physical GPUs, each of them providing multiple GPU"},{"line_number":6636,"context_line":"        # types, only one type will be used for each of the physical GPUs."},{"line_number":6637,"context_line":"        # If one of the 2 pGPUs doesn\u0027t support \u0027nvidia-35\u0027, it won\u0027t be used."},{"line_number":6638,"context_line":"        # TODO(sbauza): Use traits to make a better world."},{"line_number":6639,"context_line":"        inventories_dict \u003d self._get_gpu_inventories()"},{"line_number":6640,"context_line":"        if inventories_dict:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_d237fee2","line":6637,"range":{"start_line":6637,"start_character":10,"end_line":6637,"end_character":59},"in_reply_to":"9fdfeff1_874fff90","updated":"2019-01-24 20:17:04.000000000","message":"I\u0027m not sure what that comment is about - Sylvain?","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a16dd9be40d7af0793ad8a7853c2ae6cd455bd6e","unresolved":false,"context_lines":[{"line_number":6634,"context_line":"        # providing the total number of virtual GPUs for a single GPU type. If"},{"line_number":6635,"context_line":"        # you have multiple physical GPUs, each of them providing multiple GPU"},{"line_number":6636,"context_line":"        # types, only one type will be used for each of the physical GPUs."},{"line_number":6637,"context_line":"        # If one of the 2 pGPUs doesn\u0027t support \u0027nvidia-35\u0027, it won\u0027t be used."},{"line_number":6638,"context_line":"        # TODO(sbauza): Use traits to make a better world."},{"line_number":6639,"context_line":"        inventories_dict \u003d self._get_gpu_inventories()"},{"line_number":6640,"context_line":"        if inventories_dict:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_90118d07","line":6637,"range":{"start_line":6637,"start_character":10,"end_line":6637,"end_character":59},"in_reply_to":"9fdfeff1_d237fee2","updated":"2019-02-08 10:37:58.000000000","message":"oh no, sorry. Changing it.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"06261e534675443f892d857589220703b760098a","unresolved":false,"context_lines":[{"line_number":6668,"context_line":"        :param nodename: The ComputeNode.hypervisor_hostname, also known as"},{"line_number":6669,"context_line":"            the name of the root node provider in the tree for this host."},{"line_number":6670,"context_line":"        :param allocations: If not None, indicates a reshape was requested and"},{"line_number":6671,"context_line":"            should be performed."},{"line_number":6672,"context_line":"        :returns: The ProviderTree root node named by the `nodename` parameter."},{"line_number":6673,"context_line":"        :raises: nova.exception.ReshapeNeeded if ``allocations`` is None and"},{"line_number":6674,"context_line":"            the method determines a reshape of the tree is needed, i.e. VGPU"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_b61b5771","line":6671,"range":{"start_line":6671,"start_character":12,"end_line":6671,"end_character":18},"updated":"2019-02-12 09:33:28.000000000","message":"I\u0027ll change it to \u0027could\u0027 since it\u0027s not always done when you pass allocations\u003dNone (ie. only when you have the root RP inventory having VGPU RC)","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f340723fa97ddca98f8230ebe2a672f13d1f9fab","unresolved":false,"context_lines":[{"line_number":6674,"context_line":"            the method determines a reshape of the tree is needed, i.e. VGPU"},{"line_number":6675,"context_line":"            inventory and allocations must be migrated from the root node"},{"line_number":6676,"context_line":"            provider to a child provider of VGPU resources in the tree."},{"line_number":6677,"context_line":"        \"\"\""},{"line_number":6678,"context_line":"        root_node \u003d provider_tree.data(nodename)"},{"line_number":6679,"context_line":"        if (rc_fields.ResourceClass.VGPU in root_node.inventory and"},{"line_number":6680,"context_line":"                allocations is None):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_cd43b86b","line":6677,"updated":"2019-01-28 13:45:00.000000000","message":"It feels like this function does two totally different things. 1) selects returns the root node of the provider tree 2) raises ReshapeNeeded if reshape is needed. I think this should not be mixed in a single function.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"06261e534675443f892d857589220703b760098a","unresolved":false,"context_lines":[{"line_number":6674,"context_line":"            the method determines a reshape of the tree is needed, i.e. VGPU"},{"line_number":6675,"context_line":"            inventory and allocations must be migrated from the root node"},{"line_number":6676,"context_line":"            provider to a child provider of VGPU resources in the tree."},{"line_number":6677,"context_line":"        \"\"\""},{"line_number":6678,"context_line":"        root_node \u003d provider_tree.data(nodename)"},{"line_number":6679,"context_line":"        if (rc_fields.ResourceClass.VGPU in root_node.inventory and"},{"line_number":6680,"context_line":"                allocations is None):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_9678938f","line":6677,"in_reply_to":"9fdfeff1_4b7c5d82","updated":"2019-02-12 09:33:28.000000000","message":"Done","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4efaf4ac12f644fe74a1f559ad9b0c3c9d13514a","unresolved":false,"context_lines":[{"line_number":6674,"context_line":"            the method determines a reshape of the tree is needed, i.e. VGPU"},{"line_number":6675,"context_line":"            inventory and allocations must be migrated from the root node"},{"line_number":6676,"context_line":"            provider to a child provider of VGPU resources in the tree."},{"line_number":6677,"context_line":"        \"\"\""},{"line_number":6678,"context_line":"        root_node \u003d provider_tree.data(nodename)"},{"line_number":6679,"context_line":"        if (rc_fields.ResourceClass.VGPU in root_node.inventory and"},{"line_number":6680,"context_line":"                allocations is None):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_4b7c5d82","line":6677,"in_reply_to":"9fdfeff1_cd43b86b","updated":"2019-01-30 22:08:28.000000000","message":"Yeah, I\u0027d keep out the root_node getting in _update_provider_tree_for_vgpu","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f340723fa97ddca98f8230ebe2a672f13d1f9fab","unresolved":false,"context_lines":[{"line_number":6715,"context_line":"        # Note that we can\u0027t support multiple vgpu types until a reshape has"},{"line_number":6716,"context_line":"        # been performed on the vgpu resources provided by the root provider,"},{"line_number":6717,"context_line":"        # if any."},{"line_number":6718,"context_line":"        # TODO(sbauza): The operator can change the types, so in this case, we"},{"line_number":6719,"context_line":"        # need to raise a ReshapeNeeded exception when checking the inventories"},{"line_number":6720,"context_line":""},{"line_number":6721,"context_line":"        # Dict of PGPU RPs keyed by their libvirt PCI name"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_314f3e6c","line":6718,"range":{"start_line":6718,"start_character":24,"end_line":6718,"end_character":57},"updated":"2019-01-28 13:45:00.000000000","message":"Can we prevent the operator to change the config until the driver does not support change such config? Or can nova-compute simply fail if the config is changed to prevent accepting but ignoring such change?","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"06261e534675443f892d857589220703b760098a","unresolved":false,"context_lines":[{"line_number":6715,"context_line":"        # Note that we can\u0027t support multiple vgpu types until a reshape has"},{"line_number":6716,"context_line":"        # been performed on the vgpu resources provided by the root provider,"},{"line_number":6717,"context_line":"        # if any."},{"line_number":6718,"context_line":"        # TODO(sbauza): The operator can change the types, so in this case, we"},{"line_number":6719,"context_line":"        # need to raise a ReshapeNeeded exception when checking the inventories"},{"line_number":6720,"context_line":""},{"line_number":6721,"context_line":"        # Dict of PGPU RPs keyed by their libvirt PCI name"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_843b66c6","line":6718,"range":{"start_line":6718,"start_character":24,"end_line":6718,"end_character":57},"in_reply_to":"9fdfeff1_314f3e6c","updated":"2019-02-12 09:33:28.000000000","message":"I can write a note on https://docs.openstack.org/nova/latest/admin/virtual-gpu.html#enable-gpu-types-compute that mentions that if you change the type, you won\u0027t be able to create new instances since the total will become equal to the number of existing mdevs.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2bb01f01f4a2bc6cf2b2c1dc8cf746b004e0cb18","unresolved":false,"context_lines":[{"line_number":6734,"context_line":"            # node, identified by $nodename."},{"line_number":6735,"context_line":"            pgpu_inventory \u003d {rc_fields.ResourceClass.VGPU: inventory}"},{"line_number":6736,"context_line":"            provider_tree.update_inventory(pgpu_rp_name, pgpu_inventory)"},{"line_number":6737,"context_line":"        return pgpu_rps"},{"line_number":6738,"context_line":""},{"line_number":6739,"context_line":"    @staticmethod"},{"line_number":6740,"context_line":"    def _assert_is_root_provider("}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_e3703ea8","line":6737,"updated":"2019-01-24 15:52:17.000000000","message":"✔","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f340723fa97ddca98f8230ebe2a672f13d1f9fab","unresolved":false,"context_lines":[{"line_number":6783,"context_line":"            or no mediated devices were found to be assigned to the instance"},{"line_number":6784,"context_line":"            indicating VGPU allocations are out of sync with the hypervisor"},{"line_number":6785,"context_line":"        \"\"\""},{"line_number":6786,"context_line":"        # FIXME(sbauza): We don\u0027t really need an Instance"},{"line_number":6787,"context_line":"        # object, but given some libvirt.host logs needs"},{"line_number":6788,"context_line":"        # to have an instance name, just provide a fake one"},{"line_number":6789,"context_line":"        Instance \u003d collections.namedtuple(\u0027Instance\u0027, [\u0027uuid\u0027, \u0027name\u0027])"},{"line_number":6790,"context_line":"        instance \u003d Instance(uuid\u003dinstance_uuid, name\u003dinstance_uuid)"},{"line_number":6791,"context_line":"        mdevs \u003d self._get_all_assigned_mediated_devices(instance)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_31397ea3","line":6788,"range":{"start_line":6786,"start_character":0,"end_line":6788,"end_character":59},"updated":"2019-01-28 13:45:00.000000000","message":"besides that single log statement, the function docs on that code path are all stating that they are expecting a Instance object. It would be nice to get a followup patch that removes the dependency towards the Instance object.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"06261e534675443f892d857589220703b760098a","unresolved":false,"context_lines":[{"line_number":6783,"context_line":"            or no mediated devices were found to be assigned to the instance"},{"line_number":6784,"context_line":"            indicating VGPU allocations are out of sync with the hypervisor"},{"line_number":6785,"context_line":"        \"\"\""},{"line_number":6786,"context_line":"        # FIXME(sbauza): We don\u0027t really need an Instance"},{"line_number":6787,"context_line":"        # object, but given some libvirt.host logs needs"},{"line_number":6788,"context_line":"        # to have an instance name, just provide a fake one"},{"line_number":6789,"context_line":"        Instance \u003d collections.namedtuple(\u0027Instance\u0027, [\u0027uuid\u0027, \u0027name\u0027])"},{"line_number":6790,"context_line":"        instance \u003d Instance(uuid\u003dinstance_uuid, name\u003dinstance_uuid)"},{"line_number":6791,"context_line":"        mdevs \u003d self._get_all_assigned_mediated_devices(instance)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_588c837e","line":6788,"range":{"start_line":6786,"start_character":0,"end_line":6788,"end_character":59},"in_reply_to":"9fdfeff1_31397ea3","updated":"2019-02-12 09:33:28.000000000","message":"Well, yeah maybe","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2bb01f01f4a2bc6cf2b2c1dc8cf746b004e0cb18","unresolved":false,"context_lines":[{"line_number":6803,"context_line":"                   % {\u0027rp_uuid\u0027: rp_uuid, \u0027consumer_uuid\u0027: instance_uuid,"},{"line_number":6804,"context_line":"                      \u0027alloc_data\u0027: alloc_data})"},{"line_number":6805,"context_line":"            raise exception.ReshapeFailed(error\u003dmsg)"},{"line_number":6806,"context_line":"        return mdevs"},{"line_number":6807,"context_line":""},{"line_number":6808,"context_line":"    def _count_vgpus_per_pgpu(self, mdev_uuids):"},{"line_number":6809,"context_line":"        \"\"\"Count the number of VGPUs per physical GPU mediated device."}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_43882a70","line":6806,"updated":"2019-01-24 15:52:17.000000000","message":"✔","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2bb01f01f4a2bc6cf2b2c1dc8cf746b004e0cb18","unresolved":false,"context_lines":[{"line_number":6812,"context_line":"        :return: dict, keyed by PGPU device ID, to count of VGPUs on that"},{"line_number":6813,"context_line":"            device"},{"line_number":6814,"context_line":"        \"\"\""},{"line_number":6815,"context_line":"        vgpu_count_per_pgpu \u003d defaultdict(int)"},{"line_number":6816,"context_line":"        for mdev_uuid in mdev_uuids:"},{"line_number":6817,"context_line":"            # libvirt name is like mdev_00ead764_fdc0_46b6_8db9_2963f5c815b4"},{"line_number":6818,"context_line":"            dev_name \u003d \"mdev_\" + mdev_uuid.replace(\u0027-\u0027, \u0027_\u0027)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_a398f6bf","line":6815,"updated":"2019-01-24 15:52:17.000000000","message":"heh, I see you used collections.defaultdict(int) here at least... :)","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2bb01f01f4a2bc6cf2b2c1dc8cf746b004e0cb18","unresolved":false,"context_lines":[{"line_number":6902,"context_line":"                    }"},{"line_number":6903,"context_line":"                }"},{"line_number":6904,"context_line":"        # And remove the VGPU allocation from the root node provider."},{"line_number":6905,"context_line":"        del resources[rc_fields.ResourceClass.VGPU]"},{"line_number":6906,"context_line":""},{"line_number":6907,"context_line":"    def _reshape_gpu_resources("},{"line_number":6908,"context_line":"            self, allocations, root_node, pgpu_rps):"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_433b4a9b","line":6905,"updated":"2019-01-24 15:52:17.000000000","message":"✔","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4efaf4ac12f644fe74a1f559ad9b0c3c9d13514a","unresolved":false,"context_lines":[{"line_number":6987,"context_line":"        # we\u0027re not currently reshaping (allocations is None), we need"},{"line_number":6988,"context_line":"        # to indicate that a reshape is needed to move the VGPU inventory"},{"line_number":6989,"context_line":"        # onto a child provider in the tree."},{"line_number":6990,"context_line":"        root_node \u003d self._is_reshape_needed("},{"line_number":6991,"context_line":"            provider_tree, nodename, allocations)"},{"line_number":6992,"context_line":""},{"line_number":6993,"context_line":"        # Ensure GPU providers are in the ProviderTree for the given inventory."}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_abc869a5","line":6990,"updated":"2019-01-30 22:08:28.000000000","message":"Notes to myself:\n\nSo _update_to_placement() in the resource tracker will call us the first time with None allocations. If the root compute node has VGPU inventory, we\u0027ll raise ReshapeNeeded. We then get called again, this time with allocations.\n\nHere\u0027s an ugly flow chart:\n\n  is_reshape_needed(allocs\u003dNone)\n             |\n             |\n         VGPUs in root compute?\n      no |                  | yes\n         |                  |\n  Carry on, skipping        |\n  _reshape_gpu_resources()  Raise ReshapeNeeded\n  because allocs\u003dNone              |\n                                   |\n                                 Get called again with\n                                   allocs\n                                   |\n                          Run _reshape_gpu_resources()","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"a16dd9be40d7af0793ad8a7853c2ae6cd455bd6e","unresolved":false,"context_lines":[{"line_number":6987,"context_line":"        # we\u0027re not currently reshaping (allocations is None), we need"},{"line_number":6988,"context_line":"        # to indicate that a reshape is needed to move the VGPU inventory"},{"line_number":6989,"context_line":"        # onto a child provider in the tree."},{"line_number":6990,"context_line":"        root_node \u003d self._is_reshape_needed("},{"line_number":6991,"context_line":"            provider_tree, nodename, allocations)"},{"line_number":6992,"context_line":""},{"line_number":6993,"context_line":"        # Ensure GPU providers are in the ProviderTree for the given inventory."}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_0ee6d1e9","line":6990,"in_reply_to":"9fdfeff1_abc869a5","updated":"2019-02-08 10:37:58.000000000","message":"It\u0027s better to read the spec first to understand the back and forths between the RT, the virt driver and Placement : \nhttps://specs.openstack.org/openstack/nova-specs/specs/stein/approved/reshape-provider-tree.html\n\nThere, you\u0027ll see the usage of the allocations parameter as a way to ask the virt driver whether we should reshape or not, and if so, the second call with the allocations param is for getting them https://specs.openstack.org/openstack/nova-specs/specs/stein/approved/reshape-provider-tree.html#allocations-parameter","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4efaf4ac12f644fe74a1f559ad9b0c3c9d13514a","unresolved":false,"context_lines":[{"line_number":6991,"context_line":"            provider_tree, nodename, allocations)"},{"line_number":6992,"context_line":""},{"line_number":6993,"context_line":"        # Ensure GPU providers are in the ProviderTree for the given inventory."},{"line_number":6994,"context_line":"        pgpu_rps \u003d self._ensure_pgpu_providers("},{"line_number":6995,"context_line":"            inventories_dict, provider_tree, nodename)"},{"line_number":6996,"context_line":""},{"line_number":6997,"context_line":"        if allocations is not None:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_6be5a137","line":6994,"updated":"2019-01-30 22:08:28.000000000","message":"This doesn\u0027t just passively ensure, it\u0027ll also *create* them if missing. Now, IIUC this should be idempotent if no reshape is necessary, but maybe we could guard this inside a \"is reshape needed\" if block to improve performance a bit?","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"06261e534675443f892d857589220703b760098a","unresolved":false,"context_lines":[{"line_number":6991,"context_line":"            provider_tree, nodename, allocations)"},{"line_number":6992,"context_line":""},{"line_number":6993,"context_line":"        # Ensure GPU providers are in the ProviderTree for the given inventory."},{"line_number":6994,"context_line":"        pgpu_rps \u003d self._ensure_pgpu_providers("},{"line_number":6995,"context_line":"            inventories_dict, provider_tree, nodename)"},{"line_number":6996,"context_line":""},{"line_number":6997,"context_line":"        if allocations is not None:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_c8108e5c","line":6994,"in_reply_to":"9fdfeff1_6be5a137","updated":"2019-02-12 09:33:28.000000000","message":"We *ensure* they are there. That\u0027s idempotent, right?","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f340723fa97ddca98f8230ebe2a672f13d1f9fab","unresolved":false,"context_lines":[{"line_number":6994,"context_line":"        pgpu_rps \u003d self._ensure_pgpu_providers("},{"line_number":6995,"context_line":"            inventories_dict, provider_tree, nodename)"},{"line_number":6996,"context_line":""},{"line_number":6997,"context_line":"        if allocations is not None:"},{"line_number":6998,"context_line":"            # Reshape VGPU provider inventory and allocations, moving them"},{"line_number":6999,"context_line":"            # from the root node provider to the child providers."},{"line_number":7000,"context_line":"            self._reshape_gpu_resources(allocations, root_node, pgpu_rps)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_f181b6fc","line":6997,"updated":"2019-01-28 13:45:00.000000000","message":"So we use the presence of the allocations to decide to trigger the reshape. This is OK now as the only way allocations are provided if the L6990 found that reshape is needed. BUT as soon as we have a second case for reshape then allocations might be provided even if no vgpu reshape is needed. So I would like to ask to be a bit more precise here.\n\ndef _update_provider_tree_for_vgpu():\n\n    _ensure_pgpu_providers()\n\n    if _is_reshape_needed():\n        _ensure_allocations_are_provided()\n        _reshape_gpu_resources()\n        _delete_gpu_resource_from_root()\n\n\nThis way _is_reshape_needed() could be made a simpler function that only returns a boolean as it\u0027s name suggests.\n\n_ensure_allocations_are_provided() could be as simple as (or even inlined):\n\n    if not allocations: \n        raise ReshapeNeeded()","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"06261e534675443f892d857589220703b760098a","unresolved":false,"context_lines":[{"line_number":6994,"context_line":"        pgpu_rps \u003d self._ensure_pgpu_providers("},{"line_number":6995,"context_line":"            inventories_dict, provider_tree, nodename)"},{"line_number":6996,"context_line":""},{"line_number":6997,"context_line":"        if allocations is not None:"},{"line_number":6998,"context_line":"            # Reshape VGPU provider inventory and allocations, moving them"},{"line_number":6999,"context_line":"            # from the root node provider to the child providers."},{"line_number":7000,"context_line":"            self._reshape_gpu_resources(allocations, root_node, pgpu_rps)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_918c1d36","line":6997,"in_reply_to":"9fdfeff1_2b8d5931","updated":"2019-02-12 09:33:28.000000000","message":"\u003e _is_reshape_needed is weird because it both indicates to our\n \u003e ressource tracker caller that a reshape is needed (iow, to call us\n \u003e again, see my comment on L6990), but its name suggests that it\u0027s\n \u003e used internally to decide wether to actually perform the reshape\n \u003e (which isn\u0027t the case, we just check for allocs\u003dNone).\n \u003e \n\nNo, we trigger the reshape if VGPU inventory is on the root RP (and allocs\u003dNone yeah).\n\n \u003e So I like Gibi\u0027s idea. _is_reshape_needed can raise if we need to\n \u003e called again with allocs in order to reshape, and return True if\n \u003e we\u0027ve been called with allocs and need to perform the reshape. And\n \u003e then that if block can guard everything else (L6994 for example).\n\nI\u0027ll see how to clarify the flow.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":8864,"name":"Artom Lifshitz","email":"notartom@gmail.com","username":"artom"},"change_message_id":"4efaf4ac12f644fe74a1f559ad9b0c3c9d13514a","unresolved":false,"context_lines":[{"line_number":6994,"context_line":"        pgpu_rps \u003d self._ensure_pgpu_providers("},{"line_number":6995,"context_line":"            inventories_dict, provider_tree, nodename)"},{"line_number":6996,"context_line":""},{"line_number":6997,"context_line":"        if allocations is not None:"},{"line_number":6998,"context_line":"            # Reshape VGPU provider inventory and allocations, moving them"},{"line_number":6999,"context_line":"            # from the root node provider to the child providers."},{"line_number":7000,"context_line":"            self._reshape_gpu_resources(allocations, root_node, pgpu_rps)"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_2b8d5931","line":6997,"in_reply_to":"9fdfeff1_f181b6fc","updated":"2019-01-30 22:08:28.000000000","message":"_is_reshape_needed is weird because it both indicates to our ressource tracker caller that a reshape is needed (iow, to call us again, see my comment on L6990), but its name suggests that it\u0027s used internally to decide wether to actually perform the reshape (which isn\u0027t the case, we just check for allocs\u003dNone).\n\nSo I like Gibi\u0027s idea. _is_reshape_needed can raise if we need to called again with allocs in order to reshape, and return True if we\u0027ve been called with allocs and need to perform the reshape. And then that if block can guard everything else (L6994 for example).","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2bb01f01f4a2bc6cf2b2c1dc8cf746b004e0cb18","unresolved":false,"context_lines":[{"line_number":6999,"context_line":"            # from the root node provider to the child providers."},{"line_number":7000,"context_line":"            self._reshape_gpu_resources(allocations, root_node, pgpu_rps)"},{"line_number":7001,"context_line":""},{"line_number":7002,"context_line":"        # Only delete the root inventory once the reshape is done"},{"line_number":7003,"context_line":"        if rc_fields.ResourceClass.VGPU in root_node.inventory:"},{"line_number":7004,"context_line":"            del root_node.inventory[rc_fields.ResourceClass.VGPU]"},{"line_number":7005,"context_line":"            provider_tree.update_inventory(nodename, root_node.inventory)"},{"line_number":7006,"context_line":""},{"line_number":7007,"context_line":"    def get_available_resource(self, nodename):"},{"line_number":7008,"context_line":"        \"\"\"Retrieve resource information."}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_c31dfa0f","line":7005,"range":{"start_line":7002,"start_character":0,"end_line":7005,"end_character":73},"updated":"2019-01-24 15:52:17.000000000","message":"should this block be indented one level? if allocations is None, there\u0027s no reason to want to execute this code, right? I mean, AFAICT, there should be no reason for the root provider to have vGPU inventory when allocations is None, but you never know...","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6dac5a94544c54275555060becb887e6b322bf63","unresolved":false,"context_lines":[{"line_number":6999,"context_line":"            # from the root node provider to the child providers."},{"line_number":7000,"context_line":"            self._reshape_gpu_resources(allocations, root_node, pgpu_rps)"},{"line_number":7001,"context_line":""},{"line_number":7002,"context_line":"        # Only delete the root inventory once the reshape is done"},{"line_number":7003,"context_line":"        if rc_fields.ResourceClass.VGPU in root_node.inventory:"},{"line_number":7004,"context_line":"            del root_node.inventory[rc_fields.ResourceClass.VGPU]"},{"line_number":7005,"context_line":"            provider_tree.update_inventory(nodename, root_node.inventory)"},{"line_number":7006,"context_line":""},{"line_number":7007,"context_line":"    def get_available_resource(self, nodename):"},{"line_number":7008,"context_line":"        \"\"\"Retrieve resource information."}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_d2aa9ed0","line":7005,"range":{"start_line":7002,"start_character":0,"end_line":7005,"end_character":73},"in_reply_to":"9fdfeff1_c31dfa0f","updated":"2019-01-24 20:17:04.000000000","message":"I was thinking about this as well. allocations is not None when _is_reshape_needed raises ReshapeNeeded because there is VGPU inventory on the root node provider and allocations is None. When the RT handles that ReshapeNeeded, it will pass in allocations:\n\nhttps://github.com/openstack/nova/blob/9419c3e05499e55beda93d664197a7b0f0011ff7/nova/compute/resource_tracker.py#L950\n\nWhich might not have any consumers of that VGPU inventory and that\u0027s why I think it\u0027s decoupled from the allocations parameter, but even then I think if there is VGPU inventory on the root provider we already caught it in _is_reshape_needed and if we did slide this under \"if allocations is not None:\" we\u0027d still execute it as part of a reshape so we don\u0027t need to make this check more than once when calling update_provider_tree. Hopefully that makes sense.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"06261e534675443f892d857589220703b760098a","unresolved":false,"context_lines":[{"line_number":6999,"context_line":"            # from the root node provider to the child providers."},{"line_number":7000,"context_line":"            self._reshape_gpu_resources(allocations, root_node, pgpu_rps)"},{"line_number":7001,"context_line":""},{"line_number":7002,"context_line":"        # Only delete the root inventory once the reshape is done"},{"line_number":7003,"context_line":"        if rc_fields.ResourceClass.VGPU in root_node.inventory:"},{"line_number":7004,"context_line":"            del root_node.inventory[rc_fields.ResourceClass.VGPU]"},{"line_number":7005,"context_line":"            provider_tree.update_inventory(nodename, root_node.inventory)"},{"line_number":7006,"context_line":""},{"line_number":7007,"context_line":"    def get_available_resource(self, nodename):"},{"line_number":7008,"context_line":"        \"\"\"Retrieve resource information."}],"source_content_type":"text/x-python","patch_set":12,"id":"9fdfeff1_880e0639","line":7005,"range":{"start_line":7002,"start_character":0,"end_line":7005,"end_character":73},"in_reply_to":"9fdfeff1_d2aa9ed0","updated":"2019-02-12 09:33:28.000000000","message":"Done","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"0e09c6a803aa2c6b6bfdc60765b835169c4412e6","unresolved":false,"context_lines":[{"line_number":6832,"context_line":"        \"\"\""},{"line_number":6833,"context_line":"        vgpu_count_per_pgpu \u003d collections.defaultdict(int)"},{"line_number":6834,"context_line":"        for mdev_uuid in mdev_uuids:"},{"line_number":6835,"context_line":"            # libvirt name is like mdev_00ead764_fdc0_46b6_8db9_2963f5c815b4"},{"line_number":6836,"context_line":"            dev_name \u003d \"mdev_\" + mdev_uuid.replace(\u0027-\u0027, \u0027_\u0027)"},{"line_number":6837,"context_line":"            # Count how many vGPUs are in use for this instance"},{"line_number":6838,"context_line":"            dev_info \u003d self._get_mediated_device_information(dev_name)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_bc66fa54","line":6835,"range":{"start_line":6835,"start_character":40,"end_line":6835,"end_character":76},"updated":"2019-03-05 16:24:35.000000000","message":"Wouldn\u0027t the libvirt name have dashes rather than underscores and that\u0027s why you\u0027re converting it?","commit_id":"054eb3a652f340d819f1b04567d0ed5e207ed733"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"681f5f9fcafca3a6c41bfe4a7e061f987a921385","unresolved":false,"context_lines":[{"line_number":6832,"context_line":"        \"\"\""},{"line_number":6833,"context_line":"        vgpu_count_per_pgpu \u003d collections.defaultdict(int)"},{"line_number":6834,"context_line":"        for mdev_uuid in mdev_uuids:"},{"line_number":6835,"context_line":"            # libvirt name is like mdev_00ead764_fdc0_46b6_8db9_2963f5c815b4"},{"line_number":6836,"context_line":"            dev_name \u003d \"mdev_\" + mdev_uuid.replace(\u0027-\u0027, \u0027_\u0027)"},{"line_number":6837,"context_line":"            # Count how many vGPUs are in use for this instance"},{"line_number":6838,"context_line":"            dev_info \u003d self._get_mediated_device_information(dev_name)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_1c92a6fd","line":6835,"range":{"start_line":6835,"start_character":40,"end_line":6835,"end_character":76},"in_reply_to":"9fdfeff1_bc66fa54","updated":"2019-03-05 16:26:31.000000000","message":"Oh I guess you\u0027re saying libvirt expects underscores for the call to device_lookup_by_name, the comment was confusing. Also, could have been nicer to just handle this in _get_mediated_device_information for all callers...","commit_id":"054eb3a652f340d819f1b04567d0ed5e207ed733"}],"releasenotes/notes/libvirt-stein-vgpu-reshape-a1fa23b8ad8aa966.yaml":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"f340723fa97ddca98f8230ebe2a672f13d1f9fab","unresolved":false,"context_lines":[{"line_number":5,"context_line":"    on start of the ``nova-compute`` service. This will result in moving"},{"line_number":6,"context_line":"    VGPU inventory from the root compute node resource provider to a nested"},{"line_number":7,"context_line":"    (child) resource provider in the tree and move any associated VGPU"},{"line_number":8,"context_line":"    allocations with it. This will be a one-time operation on startup in Stein."},{"line_number":9,"context_line":"    There is no end-user visible impact for this; it is for internal resource"},{"line_number":10,"context_line":"    tracking purposes. See the `spec`__ for more details."},{"line_number":11,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":12,"id":"9fdfeff1_31165eaa","line":8,"range":{"start_line":8,"start_character":25,"end_line":8,"end_character":79},"updated":"2019-01-28 13:45:00.000000000","message":"Except if the deployer changes the enabled_vgpu_types config and restarts the nova-compute. I think we at least need to to document that this is not supported and can cause unexpected side effects.","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"6d9f464863466bc906017829d02a51d555694dc8","unresolved":false,"context_lines":[{"line_number":5,"context_line":"    on start of the ``nova-compute`` service. This will result in moving"},{"line_number":6,"context_line":"    VGPU inventory from the root compute node resource provider to a nested"},{"line_number":7,"context_line":"    (child) resource provider in the tree and move any associated VGPU"},{"line_number":8,"context_line":"    allocations with it. This will be a one-time operation on startup in Stein."},{"line_number":9,"context_line":"    There is no end-user visible impact for this; it is for internal resource"},{"line_number":10,"context_line":"    tracking purposes. See the `spec`__ for more details."},{"line_number":11,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":12,"id":"9fdfeff1_cf55d91f","line":8,"range":{"start_line":8,"start_character":25,"end_line":8,"end_character":79},"in_reply_to":"9fdfeff1_31165eaa","updated":"2019-02-13 10:51:05.000000000","message":"Done","commit_id":"3150726396e60a9a9eea5492adc5ee98af92cda5"}]}