)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4690f6db9073796eb07765d4652f9d79b3fd4d0f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1fa4df85_a7f314e7","updated":"2020-03-18 16:19:15.000000000","message":"I\u0027m not sure about this. As I understand it, you\u0027re hoping to repurpose the \u0027InstanceNUMACell.cpu_pinning\u0027 field to both track the list of cores that should be pinned and then, later, the actual pinning information. I haven\u0027t yet seen the patch to do the former but I assume it comes later in the series and, because \u0027cpu_pinning\u0027 is a dict of ints, you\u0027re setting the values to e.g. \u0027-1\u0027. However, is there any reason to do this instead of adding a new \u0027pcpuset\u0027 field like we did for the (host) \u0027NUMACell\u0027 object? This is what I was expecting and I think it would be clearer. What do you think?","commit_id":"3d5c26c101dd1dacdd486217614b80af534ff454"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4f767e526282b3b0df07cdec8973b5f8b143d6b4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1fa4df85_6204184b","in_reply_to":"1fa4df85_39fdfcb0","updated":"2020-03-19 13:58:00.000000000","message":"\u003e yes, if we have separated filed, then the cpu_pinning won\u0027t be\n \u003e mixed with many meaning. Like if the cpu_pinning is None, then it\n \u003e is dedicated instance. If the cpu_pinning is dict, then it may be a\n \u003e mixed instance. But in the end, we fulfill with real vcpu and pcpu\n \u003e mapping.\n\nYes, good point. That adds to the confusion.\n\n \u003e If we use a new pcpuset field, then we have to deal with the\n \u003e upgrade, we need to migrate existing instance db records, to fill\n \u003e the new field.\n\nNot necessarily. More on this in a moment.\n\n \u003e So another way is that we simplify the cpu_pinning. For mix and\n \u003e dedicated, we always set it as dict, the key is cpu id, value is\n \u003e -1. Then later we fill value as real pcpu id. Then we discard the\n \u003e special value \u0027None\u0027.\n\nOkay, so at the moment we\u0027re saying we have this situation:\n\n  shared\n    cpuset \u003d {0, 1, 2, 3}\n    cpu_pinning \u003d None\n\n  dedicated\n    cpuset \u003d {0, 1, 2, 3}\n    cpu_pinning \u003d None\n\nNOTE: \u0027cpu_pinning\u0027 would get populated after scheduling in both cases.\n\nWith this patchset we\u0027d change that to, e.g.:\n\n  shared\n    cpuset \u003d {0, 1, 2, 3}\n    cpu_pinning \u003d None\n\n  mixed\n    cpuset \u003d {0, 1, 2, 3}\n    cpu_pinning \u003d {2: -1, 3: -1}\n\n  dedicated\n    cpuset \u003d {0, 1, 2, 3}\n    cpu_pinning \u003d None\n\nAlex is suggesting we can tweak that further to:\n\n  shared\n    cpuset \u003d {0, 1, 2, 3}\n    cpu_pinning \u003d None\n\n  mixed\n    cpuset \u003d {0, 1, 2, 3}\n    cpu_pinning \u003d {2: -1, 3: -1}\n\n  dedicated\n    cpuset \u003d {0, 1, 2, 3}\n    cpu_pinning \u003d {0: -1, 1: -1, 2: -1, 3: -1}\n\nI\u0027m suggesting instead we tweak it to:\n\n  shared\n    cpuset \u003d {0, 1, 2, 3}\n    pcpuset \u003d None\n    cpu_pinning \u003d None\n\n  mixed\n    cpuset \u003d {0, 1}\n    pcpuset \u003d {2, 3}\n    cpu_pinning \u003d None\n\n  dedicated\n    cpuset \u003d None\n    pcpuset \u003d {0, 1, 2, 3}\n    cpu_pinning \u003d None\n\nI don\u0027t think we need DB migrations. Not yet at least. We could simply add a check like this:\n\n  if cell.cpu_policy \u003d DEDICATED and \u0027pcpuset\u0027 not in cell:\n      cell.pcpuset \u003d cell.cpuset\n      cell.cpuset \u003d None\n\nWe did something similar for the host cell here [1]. We would need a migration eventually if we wanted to remove that check, but it\u0027s only three lines so maybe it\u0027s not worth worrying about?\n\nThoughts? I\u0027m not going to block on this but I do think the clear cpuset/pcpuset split would be clearer.\n\n[1] https://github.com/openstack/nova/blob/20.1.0/nova/virt/hardware.py#L1113-L1114","commit_id":"3d5c26c101dd1dacdd486217614b80af534ff454"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"9eb91dc3dd3d72b3bee9b73163cc8f0fde041adc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1fa4df85_bac9c359","in_reply_to":"1fa4df85_4ee339da","updated":"2020-03-20 11:25:14.000000000","message":"@Alex and @Stephen, now it seems I got some sense of the concept of \u0027nova upgrade\u0027, and seems not too much things need to be changed for such kind of upgrade, in term of \u0027pcpuset\u0027 approach. \n\nHow about I submit another subsequent \u0027refactor\u0027 patch based my current series, as Alex suggested,  then we make decision between \u0027cpu_pinning\u0027 approach and \u0027pcpuset\u0027 approach.","commit_id":"3d5c26c101dd1dacdd486217614b80af534ff454"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"91722f7e8ac20f3657c52b9087eeb3fe31d898b6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1fa4df85_4ee339da","in_reply_to":"1fa4df85_6204184b","updated":"2020-03-20 05:17:24.000000000","message":"\u003e I don\u0027t think we need DB migrations. Not yet at least. We could\n \u003e simply add a check like this:\n \u003e \n \u003e if cell.cpu_policy \u003d DEDICATED and \u0027pcpuset\u0027 not in cell:\n \u003e cell.pcpuset \u003d cell.cpuset\n \u003e cell.cpuset \u003d None\n \u003e \n \u003e We did something similar for the host cell here [1]. We would need\n \u003e a migration eventually if we wanted to remove that check, but it\u0027s\n \u003e only three lines so maybe it\u0027s not worth worrying about?\n \u003e \n\nI\u0027m thinking of few things:\n\n\nFirst, cell.cpu_pinning is still being the mapping of vcpu and pcpu. We still need that.\n\n\nSeconds, Instance numa object should be different from host numa object. When all the nodes upgrade to Train release, the host numa object\u0027s pcpuset field should be fulfilled with value. But the instance numa object is not, except we do the data migration. So I guess that three lines shouldn\u0027t just be in one place, it should be in the libvirt virt driver and in the part of generating guest numa config. So probably we add those code inside instance numa object loading method, and that do the data migration also. \n\n\nThrid, we should tweak this API a little https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/server_topology.py#L54, and I guess we should have somewhere similar in hardware.py which is using the \"cell.vcpuset\", but now it needs the \"cell.vcpuset + cell.pcpuset\".\n\n\n \u003e Thoughts? I\u0027m not going to block on this but I do think the clear\n \u003e cpuset/pcpuset split would be clearer.\n\n\nAgree with that is more clear for unload the cpu_pinning field.\n\n\nSo I\u0027m thinking... Can this be done in the last patch of the series, and it is more like a refactor? \n\n\n \u003e \n \u003e [1] https://github.com/openstack/nova/blob/20.1.0/nova/virt/hardware.py#L1113-L1114","commit_id":"3d5c26c101dd1dacdd486217614b80af534ff454"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"94775362cbceeea5434fe1041f284ebc98e8b728","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1fa4df85_593eb829","in_reply_to":"1fa4df85_a7f314e7","updated":"2020-03-19 07:54:09.000000000","message":"Yes, you caught my thoughts about my using of \u0027InstanceNUMACell.cpu_pinning\u0027.\n\nI code like this and not using a \u0027pcpuset\u0027 field, as you proposed, is just because this is simple enough, especially there is no DB upgrade task. One confusing thing in my proposal might be I used a value \u0027-1\u0027 for the undetermined host CPU. \n\nIn the drafting of the spec of \u0027use-pcpu-and-vcpu-in-one-instance\u0027, I have had some preliminary discussion with sean-k-mooney and alex. Hope they can comment also.","commit_id":"3d5c26c101dd1dacdd486217614b80af534ff454"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"72bacd1282e8f49c1f60aea3ab77875046790b11","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"1fa4df85_39fdfcb0","in_reply_to":"1fa4df85_a7f314e7","updated":"2020-03-19 12:25:09.000000000","message":"yes, if we have separated filed, then the cpu_pinning won\u0027t be mixed with many meaning. Like if the cpu_pinning is None, then it is dedicated instance. If the cpu_pinning is dict, then it may be a mixed instance. But in the end, we fulfill with real vcpu and pcpu mapping.\n\nIf we use a new pcpuset field, then we have to deal with the upgrade, we need to migrate existing instance db records, to fill the new field.\n\nSo another way is that we simplify the cpu_pinning. For mix and dedicated, we always set it as dict, the key is cpu id, value is -1. Then later we fill value as real pcpu id. Then we discard the special value \u0027None\u0027.","commit_id":"3d5c26c101dd1dacdd486217614b80af534ff454"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4690f6db9073796eb07765d4652f9d79b3fd4d0f","unresolved":false,"context_lines":[{"line_number":15,"context_line":"take a CPU allocation pollicy other than \u0027SHARED\u0027."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Here we want scheduler firstly checks the \u0027cpu_pinning\u0027 field,"},{"line_number":18,"context_line":"and looks for the CPUs for pinning from its key if the fileld"},{"line_number":19,"context_line":"is not none. This supports to let part of instance CPUs to be"},{"line_number":20,"context_line":"\u0027dedicated\u0027 ones, and the remaining instance CPUs being \u0027shared\u0027"},{"line_number":21,"context_line":"ones, which will use host CPUs sharing with other instances.g"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1fa4df85_87405837","line":18,"range":{"start_line":18,"start_character":55,"end_line":18,"end_character":61},"updated":"2020-03-18 16:19:15.000000000","message":"field","commit_id":"3d5c26c101dd1dacdd486217614b80af534ff454"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"94775362cbceeea5434fe1041f284ebc98e8b728","unresolved":false,"context_lines":[{"line_number":15,"context_line":"take a CPU allocation pollicy other than \u0027SHARED\u0027."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Here we want scheduler firstly checks the \u0027cpu_pinning\u0027 field,"},{"line_number":18,"context_line":"and looks for the CPUs for pinning from its key if the fileld"},{"line_number":19,"context_line":"is not none. This supports to let part of instance CPUs to be"},{"line_number":20,"context_line":"\u0027dedicated\u0027 ones, and the remaining instance CPUs being \u0027shared\u0027"},{"line_number":21,"context_line":"ones, which will use host CPUs sharing with other instances.g"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1fa4df85_799db455","line":18,"range":{"start_line":18,"start_character":55,"end_line":18,"end_character":61},"in_reply_to":"1fa4df85_87405837","updated":"2020-03-19 07:54:09.000000000","message":"Done","commit_id":"3d5c26c101dd1dacdd486217614b80af534ff454"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4690f6db9073796eb07765d4652f9d79b3fd4d0f","unresolved":false,"context_lines":[{"line_number":18,"context_line":"and looks for the CPUs for pinning from its key if the fileld"},{"line_number":19,"context_line":"is not none. This supports to let part of instance CPUs to be"},{"line_number":20,"context_line":"\u0027dedicated\u0027 ones, and the remaining instance CPUs being \u0027shared\u0027"},{"line_number":21,"context_line":"ones, which will use host CPUs sharing with other instances.g"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: I385e78bcd07eb3f4c311bc942b6a04ff8007f6a5"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1fa4df85_a73b14c6","line":21,"range":{"start_line":21,"start_character":60,"end_line":21,"end_character":61},"updated":"2020-03-18 16:19:15.000000000","message":"drop","commit_id":"3d5c26c101dd1dacdd486217614b80af534ff454"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"94775362cbceeea5434fe1041f284ebc98e8b728","unresolved":false,"context_lines":[{"line_number":18,"context_line":"and looks for the CPUs for pinning from its key if the fileld"},{"line_number":19,"context_line":"is not none. This supports to let part of instance CPUs to be"},{"line_number":20,"context_line":"\u0027dedicated\u0027 ones, and the remaining instance CPUs being \u0027shared\u0027"},{"line_number":21,"context_line":"ones, which will use host CPUs sharing with other instances.g"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: I385e78bcd07eb3f4c311bc942b6a04ff8007f6a5"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1fa4df85_19acc08a","line":21,"range":{"start_line":21,"start_character":60,"end_line":21,"end_character":61},"in_reply_to":"1fa4df85_a73b14c6","updated":"2020-03-19 07:54:09.000000000","message":"Done","commit_id":"3d5c26c101dd1dacdd486217614b80af534ff454"}],"nova/tests/unit/virt/test_hardware.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4690f6db9073796eb07765d4652f9d79b3fd4d0f","unresolved":false,"context_lines":[{"line_number":2598,"context_line":"        else:"},{"line_number":2599,"context_line":"            self.assertIn(instance_cell.id, cell_ids)"},{"line_number":2600,"context_line":""},{"line_number":2601,"context_line":"        host_cpus \u003d set(instance_cell.cpu_pinning.values())"},{"line_number":2602,"context_line":"        inst_cpus \u003d set(instance_cell.cpu_pinning.keys())"},{"line_number":2603,"context_line":"        self.assertEqual(len(host_cpus), len(inst_cpus))"},{"line_number":2604,"context_line":""},{"line_number":2605,"context_line":"    def assertPinningPreferThreads(self, instance_cell, host_cell):"},{"line_number":2606,"context_line":"        \"\"\"Make sure we are preferring threads."}],"source_content_type":"text/x-python","patch_set":1,"id":"1fa4df85_c7b5f0ef","line":2603,"range":{"start_line":2601,"start_character":0,"end_line":2603,"end_character":56},"updated":"2020-03-18 16:19:15.000000000","message":"What are you trying to check here? Previously we were making sure that the \u0027cpu_pinning\u0027 field was populated with every core listed in \u0027cpuset\u0027. That\u0027s no longer a useful check in this design so perhaps we want to just get rid of this? Or perhaps check this only if \u0027instance_cell.cpu_policy \u003d\u003d dedicated\u0027 ?\n\nIf we go with the idea of adding a new \u0027pcpuset\u0027 field to track pinned CPUs, then we could update this to compare the size of \u0027pcpuset\u0027 and \u0027cpu_pinning\u0027 instead.","commit_id":"3d5c26c101dd1dacdd486217614b80af534ff454"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"94775362cbceeea5434fe1041f284ebc98e8b728","unresolved":false,"context_lines":[{"line_number":2598,"context_line":"        else:"},{"line_number":2599,"context_line":"            self.assertIn(instance_cell.id, cell_ids)"},{"line_number":2600,"context_line":""},{"line_number":2601,"context_line":"        host_cpus \u003d set(instance_cell.cpu_pinning.values())"},{"line_number":2602,"context_line":"        inst_cpus \u003d set(instance_cell.cpu_pinning.keys())"},{"line_number":2603,"context_line":"        self.assertEqual(len(host_cpus), len(inst_cpus))"},{"line_number":2604,"context_line":""},{"line_number":2605,"context_line":"    def assertPinningPreferThreads(self, instance_cell, host_cell):"},{"line_number":2606,"context_line":"        \"\"\"Make sure we are preferring threads."}],"source_content_type":"text/x-python","patch_set":1,"id":"1fa4df85_f965241e","line":2603,"range":{"start_line":2601,"start_character":0,"end_line":2603,"end_character":56},"in_reply_to":"1fa4df85_c7b5f0ef","updated":"2020-03-19 07:54:09.000000000","message":"I try to catch if host CPUs are allocated for vCPUs, and fill them in the \u0027cpu_pining\u0027 value field. If host CPUs are not determined for vCPUs, then the value of \u0027cpu_pinning\u0027 is -1. After a set() operation, len(set(cpu_pinning.values)) will always be 1. \n\nFor example, if instance cell with 4 vCPUs is not  pinned, then len(set(cpu_pinning.keys))\u003d4,\nand len(set(cpu_pinning.values)) \u003d len(set([-1,-1,-1,-1])) \u003d 1. Which should report the issue for un-pinning.\n\nThis code has problem when there is only one CPU in instance cell. I\u0027ll redesign it and consider your suggestion.","commit_id":"3d5c26c101dd1dacdd486217614b80af534ff454"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4f767e526282b3b0df07cdec8973b5f8b143d6b4","unresolved":false,"context_lines":[{"line_number":2598,"context_line":"        else:"},{"line_number":2599,"context_line":"            self.assertIn(instance_cell.id, cell_ids)"},{"line_number":2600,"context_line":""},{"line_number":2601,"context_line":"        host_cpus \u003d set(instance_cell.cpu_pinning.values())"},{"line_number":2602,"context_line":"        inst_cpus \u003d set(instance_cell.cpu_pinning.keys())"},{"line_number":2603,"context_line":"        self.assertEqual(len(host_cpus), len(inst_cpus))"},{"line_number":2604,"context_line":""},{"line_number":2605,"context_line":"    def assertPinningPreferThreads(self, instance_cell, host_cell):"},{"line_number":2606,"context_line":"        \"\"\"Make sure we are preferring threads."}],"source_content_type":"text/x-python","patch_set":1,"id":"1fa4df85_a29bf014","line":2603,"range":{"start_line":2601,"start_character":0,"end_line":2603,"end_character":56},"in_reply_to":"1fa4df85_f965241e","updated":"2020-03-19 13:58:00.000000000","message":"\u003e I try to catch if host CPUs are allocated for vCPUs, and fill them\n \u003e in the \u0027cpu_pining\u0027 value field. If host CPUs are not determined\n \u003e for vCPUs, then the value of \u0027cpu_pinning\u0027 is -1. After a set()\n \u003e operation, len(set(cpu_pinning.values)) will always be 1.\n \u003e \n \u003e For example, if instance cell with 4 vCPUs is not  pinned, then\n \u003e len(set(cpu_pinning.keys))\u003d4,\n \u003e and len(set(cpu_pinning.values)) \u003d len(set([-1,-1,-1,-1])) \u003d 1.\n \u003e Which should report the issue for un-pinning.\n\nAh, it\u0027s a uniqueness check. Some comments would be good so when you rework this, whatever you choose to do\n\n \u003e This code has problem when there is only one CPU in instance cell.\n \u003e I\u0027ll redesign it and consider your suggestion.","commit_id":"3d5c26c101dd1dacdd486217614b80af534ff454"}],"nova/virt/hardware.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"4690f6db9073796eb07765d4652f9d79b3fd4d0f","unresolved":false,"context_lines":[{"line_number":882,"context_line":"    # We firstly look for the instance cell CPU that to be pinned with host"},{"line_number":883,"context_line":"    # CPU by checking the cell\u0027s \u0027cpu_pinning\u0027 field. If it is not None, then"},{"line_number":884,"context_line":"    # get them from the key of this object. If \u0027cpu_pinning\u0027 is None, then we"},{"line_number":885,"context_line":"    # think all CPUs of this cell wanted to be pinned with the host CPUs."},{"line_number":886,"context_line":"    if instance_cell.cpu_pinning is not None:"},{"line_number":887,"context_line":"        cpus_to_pin \u003d set(instance_cell.cpu_pinning.keys())"},{"line_number":888,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"1fa4df85_a78f942c","line":885,"updated":"2020-03-18 16:19:15.000000000","message":"I don\u0027t understand how this is populated. We don\u0027t populate this field until the bottom of this function [1], right? I assume there\u0027s a patch coming later to do this. If so, can you explain so in the commit message?\n\n[1] https://review.opendev.org/#/c/713352/1/nova/virt/hardware.py@995","commit_id":"3d5c26c101dd1dacdd486217614b80af534ff454"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"94775362cbceeea5434fe1041f284ebc98e8b728","unresolved":false,"context_lines":[{"line_number":882,"context_line":"    # We firstly look for the instance cell CPU that to be pinned with host"},{"line_number":883,"context_line":"    # CPU by checking the cell\u0027s \u0027cpu_pinning\u0027 field. If it is not None, then"},{"line_number":884,"context_line":"    # get them from the key of this object. If \u0027cpu_pinning\u0027 is None, then we"},{"line_number":885,"context_line":"    # think all CPUs of this cell wanted to be pinned with the host CPUs."},{"line_number":886,"context_line":"    if instance_cell.cpu_pinning is not None:"},{"line_number":887,"context_line":"        cpus_to_pin \u003d set(instance_cell.cpu_pinning.keys())"},{"line_number":888,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"1fa4df85_993af0e4","line":885,"in_reply_to":"1fa4df85_43b32b4e","updated":"2020-03-19 07:54:09.000000000","message":"@alex I can improve the readability of \u0027cpu_pinning\u0027 and explain more for \u0027cpu_pinning\u0027. But it seems we need to discuss the pros and cons of \u0027cpu_pinning\u0027 approach and \u0027pcpuset\u0027 approach first. Would you please make comments in the commit message?","commit_id":"3d5c26c101dd1dacdd486217614b80af534ff454"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"14447828e35a241151141475c6a0e4ea3499447a","unresolved":false,"context_lines":[{"line_number":882,"context_line":"    # We firstly look for the instance cell CPU that to be pinned with host"},{"line_number":883,"context_line":"    # CPU by checking the cell\u0027s \u0027cpu_pinning\u0027 field. If it is not None, then"},{"line_number":884,"context_line":"    # get them from the key of this object. If \u0027cpu_pinning\u0027 is None, then we"},{"line_number":885,"context_line":"    # think all CPUs of this cell wanted to be pinned with the host CPUs."},{"line_number":886,"context_line":"    if instance_cell.cpu_pinning is not None:"},{"line_number":887,"context_line":"        cpus_to_pin \u003d set(instance_cell.cpu_pinning.keys())"},{"line_number":888,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"1fa4df85_43b32b4e","line":885,"in_reply_to":"1fa4df85_a78f942c","updated":"2020-03-19 00:53:17.000000000","message":"I also suggest to comment at here https://github.com/openstack/nova/blob/master/nova/objects/instance_numa.py#L68\n\nTo explain what is mean for not existing, what is mean for None, what is mean for having a dict.","commit_id":"3d5c26c101dd1dacdd486217614b80af534ff454"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"94775362cbceeea5434fe1041f284ebc98e8b728","unresolved":false,"context_lines":[{"line_number":882,"context_line":"    # We firstly look for the instance cell CPU that to be pinned with host"},{"line_number":883,"context_line":"    # CPU by checking the cell\u0027s \u0027cpu_pinning\u0027 field. If it is not None, then"},{"line_number":884,"context_line":"    # get them from the key of this object. If \u0027cpu_pinning\u0027 is None, then we"},{"line_number":885,"context_line":"    # think all CPUs of this cell wanted to be pinned with the host CPUs."},{"line_number":886,"context_line":"    if instance_cell.cpu_pinning is not None:"},{"line_number":887,"context_line":"        cpus_to_pin \u003d set(instance_cell.cpu_pinning.keys())"},{"line_number":888,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"1fa4df85_f94e8494","line":885,"in_reply_to":"1fa4df85_a78f942c","updated":"2020-03-19 07:54:09.000000000","message":"Yes. I think you got my idea. I also replied in the commit message.","commit_id":"3d5c26c101dd1dacdd486217614b80af534ff454"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"change_message_id":"f7ef92c5e34b9983e89cddf178c6dcebe747b05c","unresolved":false,"context_lines":[{"line_number":4674,"context_line":"        pin_cpuset.id \u003d vcpu"},{"line_number":4675,"context_line":""},{"line_number":4676,"context_line":"        if (object_numa_cell.cpu_pinning and"},{"line_number":4677,"context_line":"                vcpu in object_numa_cell.cpu_pinning):"},{"line_number":4678,"context_line":"            pin_cpuset.cpuset \u003d set([object_numa_cell.cpu_pinning[vcpu]])"},{"line_number":4679,"context_line":"        else:"},{"line_number":4680,"context_line":"            pin_cpuset.cpuset \u003d host_cell.cpuset"}],"source_content_type":"text/x-python","patch_set":1,"id":"1fa4df85_034f7392","line":4677,"updated":"2020-03-19 02:34:32.000000000","message":"this is worth a separated patch, since it isn\u0027t related to scheduler.","commit_id":"3d5c26c101dd1dacdd486217614b80af534ff454"},{"author":{"_account_id":30209,"name":"Huaqiang","email":"huaqiang.wang@intel.com","username":"Huaqiang.Wang"},"change_message_id":"94775362cbceeea5434fe1041f284ebc98e8b728","unresolved":false,"context_lines":[{"line_number":4674,"context_line":"        pin_cpuset.id \u003d vcpu"},{"line_number":4675,"context_line":""},{"line_number":4676,"context_line":"        if (object_numa_cell.cpu_pinning and"},{"line_number":4677,"context_line":"                vcpu in object_numa_cell.cpu_pinning):"},{"line_number":4678,"context_line":"            pin_cpuset.cpuset \u003d set([object_numa_cell.cpu_pinning[vcpu]])"},{"line_number":4679,"context_line":"        else:"},{"line_number":4680,"context_line":"            pin_cpuset.cpuset \u003d host_cell.cpuset"}],"source_content_type":"text/x-python","patch_set":1,"id":"1fa4df85_b916cc54","line":4677,"in_reply_to":"1fa4df85_034f7392","updated":"2020-03-19 07:54:09.000000000","message":"I\u0027ll separate it as well as the test cases.","commit_id":"3d5c26c101dd1dacdd486217614b80af534ff454"}]}