)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cf0fe2087ba2913b63ce0721f7c0035df5207b84","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Sean Mooney \u003cwork@seanmooney.info\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-02-28 11:35:08 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add free for claimed, allocated devices"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently freeing PCI resources for instance frees"},{"line_number":10,"context_line":"both claimed and allocated resources."},{"line_number":11,"context_line":"Extend the API and remove this constraint."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The change is required to enable SR-IOV live migration."},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"9fdfeff1_f448328b","line":11,"range":{"start_line":7,"start_character":0,"end_line":11,"end_character":42},"updated":"2019-03-04 19:26:00.000000000","message":"This doesn\u0027t make sense to me. The git commit summary is saying \"add free for claimed, allocated devices\" but the first line of the commit description is \"Currently freeing PCI resources for instance frees both claimed and allocated resources\", followed by \"Extend the API and remove this constraint\". I don\u0027t see what the constraint is?","commit_id":"7d9e3882e1a54ce421c36380817a1f619ccf250d"},{"author":{"_account_id":28714,"name":"Adrian Chiris","email":"adrianc@nvidia.com","username":"adrianc"},"change_message_id":"144d3a14ca6753d7beac857c09772b7b707206d1","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Sean Mooney \u003cwork@seanmooney.info\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-02-28 11:35:08 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add free for claimed, allocated devices"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently freeing PCI resources for instance frees"},{"line_number":10,"context_line":"both claimed and allocated resources."},{"line_number":11,"context_line":"Extend the API and remove this constraint."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The change is required to enable SR-IOV live migration."},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"9fdfeff1_a9d9de84","line":11,"range":{"start_line":7,"start_character":0,"end_line":11,"end_character":42},"in_reply_to":"9fdfeff1_aab79ef5","updated":"2019-03-05 15:29:37.000000000","message":"Yea, re-reading it now, I agree.\nI will re-spin with an updated commit message.","commit_id":"7d9e3882e1a54ce421c36380817a1f619ccf250d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c7e13d569e74ab7ff5f2ed3f10d7836f5d0370dc","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Sean Mooney \u003cwork@seanmooney.info\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-02-28 11:35:08 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add free for claimed, allocated devices"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently freeing PCI resources for instance frees"},{"line_number":10,"context_line":"both claimed and allocated resources."},{"line_number":11,"context_line":"Extend the API and remove this constraint."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"The change is required to enable SR-IOV live migration."},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":18,"id":"9fdfeff1_aab79ef5","line":11,"range":{"start_line":7,"start_character":0,"end_line":11,"end_character":42},"in_reply_to":"9fdfeff1_f448328b","updated":"2019-03-05 01:16:29.000000000","message":"OK, after reading Moshe\u0027s response in the code, I understand what you\u0027re trying to do in this patch. Definitely need to reword the commit message because it really doesn\u0027t help.\n\nHow\u0027s this for a commit message?\n\n sep methods to free claim\u0027d and alloc\u0027d devs\n\n The existing PCI manager exposes only a single free_instance()\n method that frees both claimed and allocated devices associated with\n an instance. In subsequent patches in this series, we need to free either\n the claimed or allocated devices associated with an instance, depending\n on whether the device has been claimed or allocated as part of a live migration.\n\n This patch simply breaks the single free_instance() method into a\n free_allocated_devices() and free_claimed_devices() method, allowing\n the subsequent patches to call the appropriate method during live migration.","commit_id":"7d9e3882e1a54ce421c36380817a1f619ccf250d"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"82df64d2e09f11d14cbcfd3da0ff0da13941f52b","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Sean Mooney \u003cwork@seanmooney.info\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-03-06 18:30:18 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Sep methods to free claimed and allocated devs"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"The existing PCI manager exposes a single method to free PCI resources"},{"line_number":10,"context_line":"free_instance(), which frees both claimed and allocated PCI resources"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":21,"id":"5fc1f717_c6cb04a3","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":3},"updated":"2019-03-07 01:33:08.000000000","message":"Separate (I\u0027m assuming Jay didn\u0027t mean \u0027sep\u0027 literally, but boo Jay (sorry :)) for using it in the first place)","commit_id":"209e55d4d3290727b6040010b8c2cdaba1ec6cfb"}],"nova/pci/manager.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1076713dea8707cac3a1c9cafc7f7a80e14d3605","unresolved":false,"context_lines":[{"line_number":291,"context_line":""},{"line_number":292,"context_line":"    def _free_instance(self,"},{"line_number":293,"context_line":"                       instance,"},{"line_number":294,"context_line":"                       dev_status_to_free\u003d(fields.PciDeviceStatus.CLAIMED,"},{"line_number":295,"context_line":"                                           fields.PciDeviceStatus.ALLOCATED)):"},{"line_number":296,"context_line":"        \"\"\"Free device that are in device_status_to_free status for instance."},{"line_number":297,"context_line":""},{"line_number":298,"context_line":"        :param instance: instance object"}],"source_content_type":"text/x-python","patch_set":4,"id":"dfd5e7cf_0649e3ce","line":295,"range":{"start_line":294,"start_character":42,"end_line":295,"end_character":76},"updated":"2019-01-09 17:57:30.000000000","message":"nit: Maybe make this a constant? It is a tuple though so we don\u0027t have to worry about anything modifying like a list/set.","commit_id":"90c350fce0a30e59de753fddd6648513cc326ef7"},{"author":{"_account_id":28714,"name":"Adrian Chiris","email":"adrianc@nvidia.com","username":"adrianc"},"change_message_id":"321d68335a446304aadf0429eb304aebe1b1c658","unresolved":false,"context_lines":[{"line_number":291,"context_line":""},{"line_number":292,"context_line":"    def _free_instance(self,"},{"line_number":293,"context_line":"                       instance,"},{"line_number":294,"context_line":"                       dev_status_to_free\u003d(fields.PciDeviceStatus.CLAIMED,"},{"line_number":295,"context_line":"                                           fields.PciDeviceStatus.ALLOCATED)):"},{"line_number":296,"context_line":"        \"\"\"Free device that are in device_status_to_free status for instance."},{"line_number":297,"context_line":""},{"line_number":298,"context_line":"        :param instance: instance object"}],"source_content_type":"text/x-python","patch_set":4,"id":"bfdaf3ff_c1452c72","line":295,"range":{"start_line":294,"start_character":42,"end_line":295,"end_character":76},"in_reply_to":"dfd5e7cf_0649e3ce","updated":"2019-01-15 11:17:15.000000000","message":"I believe having the tuple here better conveys the intended behaviour of _free_instance() when dev_status_to_free is not provided, in my mind it saves the reader to lookup the constant value.\n\nlet me know if you feel this change is needed.","commit_id":"90c350fce0a30e59de753fddd6648513cc326ef7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1076713dea8707cac3a1c9cafc7f7a80e14d3605","unresolved":false,"context_lines":[{"line_number":297,"context_line":""},{"line_number":298,"context_line":"        :param instance: instance object"},{"line_number":299,"context_line":"        :param dev_status_to_free: list of expected device statuses"},{"line_number":300,"context_line":"        which this method will free."},{"line_number":301,"context_line":"        :return:"},{"line_number":302,"context_line":"        \"\"\""},{"line_number":303,"context_line":"        # Note(yjiang5): When an instance is resized, the devices in the"}],"source_content_type":"text/x-python","patch_set":4,"id":"dfd5e7cf_c6143ba3","line":300,"range":{"start_line":300,"start_character":0,"end_line":300,"end_character":36},"updated":"2019-01-09 17:57:30.000000000","message":"nit: indent","commit_id":"90c350fce0a30e59de753fddd6648513cc326ef7"},{"author":{"_account_id":28714,"name":"Adrian Chiris","email":"adrianc@nvidia.com","username":"adrianc"},"change_message_id":"321d68335a446304aadf0429eb304aebe1b1c658","unresolved":false,"context_lines":[{"line_number":297,"context_line":""},{"line_number":298,"context_line":"        :param instance: instance object"},{"line_number":299,"context_line":"        :param dev_status_to_free: list of expected device statuses"},{"line_number":300,"context_line":"        which this method will free."},{"line_number":301,"context_line":"        :return:"},{"line_number":302,"context_line":"        \"\"\""},{"line_number":303,"context_line":"        # Note(yjiang5): When an instance is resized, the devices in the"}],"source_content_type":"text/x-python","patch_set":4,"id":"bfdaf3ff_e14ab07e","line":300,"range":{"start_line":300,"start_character":0,"end_line":300,"end_character":36},"in_reply_to":"dfd5e7cf_c6143ba3","updated":"2019-01-15 11:17:15.000000000","message":"Done","commit_id":"90c350fce0a30e59de753fddd6648513cc326ef7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1076713dea8707cac3a1c9cafc7f7a80e14d3605","unresolved":false,"context_lines":[{"line_number":298,"context_line":"        :param instance: instance object"},{"line_number":299,"context_line":"        :param dev_status_to_free: list of expected device statuses"},{"line_number":300,"context_line":"        which this method will free."},{"line_number":301,"context_line":"        :return:"},{"line_number":302,"context_line":"        \"\"\""},{"line_number":303,"context_line":"        # Note(yjiang5): When an instance is resized, the devices in the"},{"line_number":304,"context_line":"        # destination node are claimed to the instance in prep_resize stage."}],"source_content_type":"text/x-python","patch_set":4,"id":"dfd5e7cf_86d153d0","line":301,"range":{"start_line":301,"start_character":0,"end_line":301,"end_character":16},"updated":"2019-01-09 17:57:30.000000000","message":"nit: drop","commit_id":"90c350fce0a30e59de753fddd6648513cc326ef7"},{"author":{"_account_id":28714,"name":"Adrian Chiris","email":"adrianc@nvidia.com","username":"adrianc"},"change_message_id":"321d68335a446304aadf0429eb304aebe1b1c658","unresolved":false,"context_lines":[{"line_number":298,"context_line":"        :param instance: instance object"},{"line_number":299,"context_line":"        :param dev_status_to_free: list of expected device statuses"},{"line_number":300,"context_line":"        which this method will free."},{"line_number":301,"context_line":"        :return:"},{"line_number":302,"context_line":"        \"\"\""},{"line_number":303,"context_line":"        # Note(yjiang5): When an instance is resized, the devices in the"},{"line_number":304,"context_line":"        # destination node are claimed to the instance in prep_resize stage."}],"source_content_type":"text/x-python","patch_set":4,"id":"bfdaf3ff_815f2440","line":301,"range":{"start_line":301,"start_character":0,"end_line":301,"end_character":16},"in_reply_to":"dfd5e7cf_86d153d0","updated":"2019-01-15 11:17:15.000000000","message":"Done","commit_id":"90c350fce0a30e59de753fddd6648513cc326ef7"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1076713dea8707cac3a1c9cafc7f7a80e14d3605","unresolved":false,"context_lines":[{"line_number":310,"context_line":"                if dev.instance_uuid \u003d\u003d instance[\u0027uuid\u0027]:"},{"line_number":311,"context_line":"                    self._free_device(dev)"},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"    def free_instance_allocations(self, context, instance):"},{"line_number":314,"context_line":"        if self.allocations.pop(instance[\u0027uuid\u0027], None):"},{"line_number":315,"context_line":"            self._free_instance(instance, fields.PciDeviceStatus.ALLOCATED)"},{"line_number":316,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"dfd5e7cf_86eab372","line":313,"range":{"start_line":313,"start_character":0,"end_line":313,"end_character":59},"updated":"2019-01-09 17:57:30.000000000","message":"Fancy adding docstrings? :) Here and below","commit_id":"90c350fce0a30e59de753fddd6648513cc326ef7"},{"author":{"_account_id":28714,"name":"Adrian Chiris","email":"adrianc@nvidia.com","username":"adrianc"},"change_message_id":"321d68335a446304aadf0429eb304aebe1b1c658","unresolved":false,"context_lines":[{"line_number":310,"context_line":"                if dev.instance_uuid \u003d\u003d instance[\u0027uuid\u0027]:"},{"line_number":311,"context_line":"                    self._free_device(dev)"},{"line_number":312,"context_line":""},{"line_number":313,"context_line":"    def free_instance_allocations(self, context, instance):"},{"line_number":314,"context_line":"        if self.allocations.pop(instance[\u0027uuid\u0027], None):"},{"line_number":315,"context_line":"            self._free_instance(instance, fields.PciDeviceStatus.ALLOCATED)"},{"line_number":316,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"bfdaf3ff_41593c58","line":313,"range":{"start_line":313,"start_character":0,"end_line":313,"end_character":59},"in_reply_to":"dfd5e7cf_86eab372","updated":"2019-01-15 11:17:15.000000000","message":"Done","commit_id":"90c350fce0a30e59de753fddd6648513cc326ef7"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"ce689750d580a1e63d1bbbfd2ec44d0885aad3c1","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        for dev in freed_devs:"},{"line_number":290,"context_line":"            self.stats.add_device(dev)"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"    def _free_instance(self,"},{"line_number":293,"context_line":"                       instance,"},{"line_number":294,"context_line":"                       dev_status_to_free):"},{"line_number":295,"context_line":"        \"\"\"Free device that are in device_status_to_free status for instance."},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"        :param instance: instance object"}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_9683f856","line":294,"range":{"start_line":292,"start_character":0,"end_line":294,"end_character":43},"updated":"2019-01-21 12:44:00.000000000","message":"this can fit one line","commit_id":"cca4c66375974622e42ca096a8c20e09d437a11f"},{"author":{"_account_id":28714,"name":"Adrian Chiris","email":"adrianc@nvidia.com","username":"adrianc"},"change_message_id":"400522adf51acc748334ade1e19fa7eb4298582b","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        for dev in freed_devs:"},{"line_number":290,"context_line":"            self.stats.add_device(dev)"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"    def _free_instance(self,"},{"line_number":293,"context_line":"                       instance,"},{"line_number":294,"context_line":"                       dev_status_to_free):"},{"line_number":295,"context_line":"        \"\"\"Free device that are in device_status_to_free status for instance."},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"        :param instance: instance object"}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_f96cb09d","line":294,"range":{"start_line":292,"start_character":0,"end_line":294,"end_character":43},"in_reply_to":"9fdfeff1_9683f856","updated":"2019-01-27 16:19:23.000000000","message":"pycharm black magic...","commit_id":"cca4c66375974622e42ca096a8c20e09d437a11f"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"ce689750d580a1e63d1bbbfd2ec44d0885aad3c1","unresolved":false,"context_lines":[{"line_number":305,"context_line":"        # information, not the claimed one. So we can\u0027t use"},{"line_number":306,"context_line":"        # instance[\u0027pci_devices\u0027] to check the devices to be freed."},{"line_number":307,"context_line":"        for dev in self.pci_devs:"},{"line_number":308,"context_line":"            if dev.status \u003d\u003d dev_status_to_free:"},{"line_number":309,"context_line":"                if dev.instance_uuid \u003d\u003d instance[\u0027uuid\u0027]:"},{"line_number":310,"context_line":"                    self._free_device(dev)"},{"line_number":311,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_f68da47b","line":308,"range":{"start_line":308,"start_character":29,"end_line":308,"end_character":47},"updated":"2019-01-21 12:44:00.000000000","message":"it better to validate that that the dev_status_to_free in one of (fields.PciDeviceStatus.CLAIMED,                                       fields.PciDeviceStatus.ALLOCATED) in code","commit_id":"cca4c66375974622e42ca096a8c20e09d437a11f"},{"author":{"_account_id":28714,"name":"Adrian Chiris","email":"adrianc@nvidia.com","username":"adrianc"},"change_message_id":"400522adf51acc748334ade1e19fa7eb4298582b","unresolved":false,"context_lines":[{"line_number":305,"context_line":"        # information, not the claimed one. So we can\u0027t use"},{"line_number":306,"context_line":"        # instance[\u0027pci_devices\u0027] to check the devices to be freed."},{"line_number":307,"context_line":"        for dev in self.pci_devs:"},{"line_number":308,"context_line":"            if dev.status \u003d\u003d dev_status_to_free:"},{"line_number":309,"context_line":"                if dev.instance_uuid \u003d\u003d instance[\u0027uuid\u0027]:"},{"line_number":310,"context_line":"                    self._free_device(dev)"},{"line_number":311,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_f9644724","line":308,"range":{"start_line":308,"start_character":29,"end_line":308,"end_character":47},"in_reply_to":"9fdfeff1_84c12683","updated":"2019-01-27 16:19:23.000000000","message":"TBH dont have a special preference here, i can move yjiang5 note to free_instance(), duplicate the logic and remove _free_instance().","commit_id":"cca4c66375974622e42ca096a8c20e09d437a11f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5b895140c73a6366fae82800f5420a4badbbe093","unresolved":false,"context_lines":[{"line_number":305,"context_line":"        # information, not the claimed one. So we can\u0027t use"},{"line_number":306,"context_line":"        # instance[\u0027pci_devices\u0027] to check the devices to be freed."},{"line_number":307,"context_line":"        for dev in self.pci_devs:"},{"line_number":308,"context_line":"            if dev.status \u003d\u003d dev_status_to_free:"},{"line_number":309,"context_line":"                if dev.instance_uuid \u003d\u003d instance[\u0027uuid\u0027]:"},{"line_number":310,"context_line":"                    self._free_device(dev)"},{"line_number":311,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_84c12683","line":308,"range":{"start_line":308,"start_character":29,"end_line":308,"end_character":47},"in_reply_to":"9fdfeff1_f68da47b","updated":"2019-01-25 14:50:56.000000000","message":"Yeah, actually, do you want to just duplicate this function into the two callers? Should be simpler to grok, I imagine","commit_id":"cca4c66375974622e42ca096a8c20e09d437a11f"},{"author":{"_account_id":28714,"name":"Adrian Chiris","email":"adrianc@nvidia.com","username":"adrianc"},"change_message_id":"400522adf51acc748334ade1e19fa7eb4298582b","unresolved":false,"context_lines":[{"line_number":305,"context_line":"        # information, not the claimed one. So we can\u0027t use"},{"line_number":306,"context_line":"        # instance[\u0027pci_devices\u0027] to check the devices to be freed."},{"line_number":307,"context_line":"        for dev in self.pci_devs:"},{"line_number":308,"context_line":"            if dev.status \u003d\u003d dev_status_to_free:"},{"line_number":309,"context_line":"                if dev.instance_uuid \u003d\u003d instance[\u0027uuid\u0027]:"},{"line_number":310,"context_line":"                    self._free_device(dev)"},{"line_number":311,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"9fdfeff1_b9b55f80","line":308,"range":{"start_line":308,"start_character":29,"end_line":308,"end_character":47},"in_reply_to":"9fdfeff1_f68da47b","updated":"2019-01-27 16:19:23.000000000","message":"technically this can work if new PCIDeviceStatus are defined. however my docstring implies otherwise so might as well add a check for it.","commit_id":"cca4c66375974622e42ca096a8c20e09d437a11f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5d10cb4bd07ac3a91b8e582e55de606fdf9f43ef","unresolved":false,"context_lines":[{"line_number":295,"context_line":"        :param context: user request context (nova.context.RequestContext)"},{"line_number":296,"context_line":"        :param instance: instance object"},{"line_number":297,"context_line":"        \"\"\""},{"line_number":298,"context_line":"        if self.allocations.pop(instance[\u0027uuid\u0027], None):"},{"line_number":299,"context_line":"            for dev in self.pci_devs:"},{"line_number":300,"context_line":"                if (dev.status \u003d\u003d fields.PciDeviceStatus.ALLOCATED and"},{"line_number":301,"context_line":"                        dev.instance_uuid \u003d\u003d instance[\u0027uuid\u0027]):"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fdfeff1_8167d351","line":298,"range":{"start_line":298,"start_character":0,"end_line":298,"end_character":56},"updated":"2019-02-11 17:26:46.000000000","message":"style nit:\n\n  if not self.allocations.pop(instance[\u0027uuid\u0027], None):\n      return\n\n  for dev in self.pci_devs:\n      ...","commit_id":"df494543dad41ae8c4526d16dcd024465c0303af"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"db630b7fd729d5046819320954fcfd76429df8d2","unresolved":false,"context_lines":[{"line_number":295,"context_line":"        :param context: user request context (nova.context.RequestContext)"},{"line_number":296,"context_line":"        :param instance: instance object"},{"line_number":297,"context_line":"        \"\"\""},{"line_number":298,"context_line":"        if self.allocations.pop(instance[\u0027uuid\u0027], None):"},{"line_number":299,"context_line":"            for dev in self.pci_devs:"},{"line_number":300,"context_line":"                if (dev.status \u003d\u003d fields.PciDeviceStatus.ALLOCATED and"},{"line_number":301,"context_line":"                        dev.instance_uuid \u003d\u003d instance[\u0027uuid\u0027]):"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fdfeff1_5740c935","line":298,"range":{"start_line":298,"start_character":0,"end_line":298,"end_character":56},"in_reply_to":"9fdfeff1_8167d351","updated":"2019-02-11 18:40:34.000000000","message":"i think that only makes sense when you have code that woudl be out of the if.\n\nthis function normally never returns anything so adding a return statement feels a little strange.\n\nbut they are both fairly equivalent.","commit_id":"df494543dad41ae8c4526d16dcd024465c0303af"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5d10cb4bd07ac3a91b8e582e55de606fdf9f43ef","unresolved":false,"context_lines":[{"line_number":307,"context_line":"        :param context: user request context (nova.context.RequestContext)"},{"line_number":308,"context_line":"        :param instance: instance object"},{"line_number":309,"context_line":"        \"\"\""},{"line_number":310,"context_line":"        if self.claims.pop(instance[\u0027uuid\u0027], None):"},{"line_number":311,"context_line":"            for dev in self.pci_devs:"},{"line_number":312,"context_line":"                if (dev.status \u003d\u003d fields.PciDeviceStatus.CLAIMED and"},{"line_number":313,"context_line":"                        dev.instance_uuid \u003d\u003d instance[\u0027uuid\u0027]):"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fdfeff1_616c2f36","line":310,"range":{"start_line":310,"start_character":0,"end_line":310,"end_character":51},"updated":"2019-02-11 17:26:46.000000000","message":"ditto","commit_id":"df494543dad41ae8c4526d16dcd024465c0303af"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c7e13d569e74ab7ff5f2ed3f10d7836f5d0370dc","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        for dev in freed_devs:"},{"line_number":315,"context_line":"            self.stats.add_device(dev)"},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"    def free_instance_allocations(self, context, instance):"},{"line_number":318,"context_line":"        \"\"\"Free devices that are in ALLOCATED state for instance."},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"        :param context: user request context (nova.context.RequestContext)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_0ac6ea69","line":317,"range":{"start_line":317,"start_character":8,"end_line":317,"end_character":33},"updated":"2019-03-05 01:16:29.000000000","message":"recommend free_allocated_devices()","commit_id":"7d9e3882e1a54ce421c36380817a1f619ccf250d"},{"author":{"_account_id":28714,"name":"Adrian Chiris","email":"adrianc@nvidia.com","username":"adrianc"},"change_message_id":"144d3a14ca6753d7beac857c09772b7b707206d1","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        for dev in freed_devs:"},{"line_number":315,"context_line":"            self.stats.add_device(dev)"},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"    def free_instance_allocations(self, context, instance):"},{"line_number":318,"context_line":"        \"\"\"Free devices that are in ALLOCATED state for instance."},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"        :param context: user request context (nova.context.RequestContext)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_892f3a87","line":317,"range":{"start_line":317,"start_character":8,"end_line":317,"end_character":33},"in_reply_to":"9fdfeff1_0ac6ea69","updated":"2019-03-05 15:29:37.000000000","message":"So,the intent was to use the same wording as the original free_instance()\n\ni.e we will now have:\nfree_instance() \nfree_instance_claims()\nfree_instance_allocations()\n\nseems more consistent to me, but can be changed.","commit_id":"7d9e3882e1a54ce421c36380817a1f619ccf250d"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9babb131ab5d2123ce70770b99c155e46d8db5fa","unresolved":false,"context_lines":[{"line_number":314,"context_line":"        for dev in freed_devs:"},{"line_number":315,"context_line":"            self.stats.add_device(dev)"},{"line_number":316,"context_line":""},{"line_number":317,"context_line":"    def free_instance_allocations(self, context, instance):"},{"line_number":318,"context_line":"        \"\"\"Free devices that are in ALLOCATED state for instance."},{"line_number":319,"context_line":""},{"line_number":320,"context_line":"        :param context: user request context (nova.context.RequestContext)"}],"source_content_type":"text/x-python","patch_set":18,"id":"5fc1f717_a45e01e4","line":317,"range":{"start_line":317,"start_character":8,"end_line":317,"end_character":33},"in_reply_to":"9fdfeff1_892f3a87","updated":"2019-03-06 14:38:23.000000000","message":"free_instance_allocations does overlap a bit with the idea or plamcent allcoaiton but since we dont track pci device in placmenet i think its fine.","commit_id":"7d9e3882e1a54ce421c36380817a1f619ccf250d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c7e13d569e74ab7ff5f2ed3f10d7836f5d0370dc","unresolved":false,"context_lines":[{"line_number":326,"context_line":"                        dev.instance_uuid \u003d\u003d instance[\u0027uuid\u0027]):"},{"line_number":327,"context_line":"                    self._free_device(dev)"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"    def free_instance_claims(self, context, instance):"},{"line_number":330,"context_line":"        \"\"\"Free devices that are in CLAIMED state for instance."},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"        :param context: user request context (nova.context.RequestContext)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_6ac9b677","line":329,"range":{"start_line":329,"start_character":8,"end_line":329,"end_character":28},"updated":"2019-03-05 01:16:29.000000000","message":"recommend free_claimed_devices()","commit_id":"7d9e3882e1a54ce421c36380817a1f619ccf250d"},{"author":{"_account_id":28714,"name":"Adrian Chiris","email":"adrianc@nvidia.com","username":"adrianc"},"change_message_id":"144d3a14ca6753d7beac857c09772b7b707206d1","unresolved":false,"context_lines":[{"line_number":326,"context_line":"                        dev.instance_uuid \u003d\u003d instance[\u0027uuid\u0027]):"},{"line_number":327,"context_line":"                    self._free_device(dev)"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"    def free_instance_claims(self, context, instance):"},{"line_number":330,"context_line":"        \"\"\"Free devices that are in CLAIMED state for instance."},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"        :param context: user request context (nova.context.RequestContext)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_4978728e","line":329,"range":{"start_line":329,"start_character":8,"end_line":329,"end_character":28},"in_reply_to":"9fdfeff1_6ac9b677","updated":"2019-03-05 15:29:37.000000000","message":"same as the above","commit_id":"7d9e3882e1a54ce421c36380817a1f619ccf250d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cf0fe2087ba2913b63ce0721f7c0035df5207b84","unresolved":false,"context_lines":[{"line_number":350,"context_line":"        # information, not the claimed one. So we can\u0027t use"},{"line_number":351,"context_line":"        # instance[\u0027pci_devices\u0027] to check the devices to be freed."},{"line_number":352,"context_line":"        self.free_instance_allocations(context, instance)"},{"line_number":353,"context_line":"        self.free_instance_claims(context, instance)"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"    def update_pci_for_instance(self, context, instance, sign):"},{"line_number":356,"context_line":"        \"\"\"Update PCI usage information if devices are de/allocated."}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_f416f258","line":353,"updated":"2019-03-04 19:26:00.000000000","message":"the above makes no functional change to the code at all...","commit_id":"7d9e3882e1a54ce421c36380817a1f619ccf250d"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"c7e13d569e74ab7ff5f2ed3f10d7836f5d0370dc","unresolved":false,"context_lines":[{"line_number":350,"context_line":"        # information, not the claimed one. So we can\u0027t use"},{"line_number":351,"context_line":"        # instance[\u0027pci_devices\u0027] to check the devices to be freed."},{"line_number":352,"context_line":"        self.free_instance_allocations(context, instance)"},{"line_number":353,"context_line":"        self.free_instance_claims(context, instance)"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"    def update_pci_for_instance(self, context, instance, sign):"},{"line_number":356,"context_line":"        \"\"\"Update PCI usage information if devices are de/allocated."}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_ead4c621","line":353,"in_reply_to":"9fdfeff1_7f5c1b4c","updated":"2019-03-05 01:16:29.000000000","message":"I see. OK, my suggestion is to make that clear in the commit message, since it\u0027s not at all obvious.","commit_id":"7d9e3882e1a54ce421c36380817a1f619ccf250d"},{"author":{"_account_id":12171,"name":"Moshe Levi","email":"moshele@nvidia.com","username":"moshele"},"change_message_id":"38669013b4c2e0d3228e878709794eca4890e6fb","unresolved":false,"context_lines":[{"line_number":350,"context_line":"        # information, not the claimed one. So we can\u0027t use"},{"line_number":351,"context_line":"        # instance[\u0027pci_devices\u0027] to check the devices to be freed."},{"line_number":352,"context_line":"        self.free_instance_allocations(context, instance)"},{"line_number":353,"context_line":"        self.free_instance_claims(context, instance)"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"    def update_pci_for_instance(self, context, instance, sign):"},{"line_number":356,"context_line":"        \"\"\"Update PCI usage information if devices are de/allocated."}],"source_content_type":"text/x-python","patch_set":18,"id":"9fdfeff1_7f5c1b4c","line":353,"in_reply_to":"9fdfeff1_f416f258","updated":"2019-03-04 20:05:25.000000000","message":"The change is that we expose 2 methods free_instance_claims and free_instance_allocations instead of freeing all the instance pci devices. This is required for the following:\nfree_instance_allocations - \nVM has allocate pci device. In the check dest method we need to claim pci device on the dest node. on post live migration we need to free only the allocations and and the claims become the allocations. \nfree_instance_claims -  check dest method we need to claim pci device on the dest node. live migration failed we need to free the claims.\n\nAnother approach to solve this will be to keep the old implementation of _free_instance and to free only the pci devices that belong to the node that is calling the free.","commit_id":"7d9e3882e1a54ce421c36380817a1f619ccf250d"}],"nova/tests/unit/pci/test_manager.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"1076713dea8707cac3a1c9cafc7f7a80e14d3605","unresolved":false,"context_lines":[{"line_number":500,"context_line":"        self.assertIsNone(self.tracker.allocations.get(instance_uuid))"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    def test_free_claims_and_allocations(self):"},{"line_number":503,"context_line":"        pci_requests_obj \u003d self._create_pci_requests_object("},{"line_number":504,"context_line":"            [{\u0027count\u0027: 1, \u0027spec\u0027: [{\u0027vendor_id\u0027: \u0027v\u0027}]}])"},{"line_number":505,"context_line":"        allocated_devs \u003d self.tracker.claim_instance(mock.sentinel.context,"},{"line_number":506,"context_line":"                                    pci_requests_obj, None)"},{"line_number":507,"context_line":"        self.tracker.allocate_instance(self.inst)"},{"line_number":508,"context_line":"        claimed_devs \u003d self.tracker.claim_instance(mock.sentinel.context,"},{"line_number":509,"context_line":"                                    pci_requests_obj, None)"},{"line_number":510,"context_line":"        allocated_dev \u003d allocated_devs[0]"},{"line_number":511,"context_line":"        claimed_dev \u003d claimed_devs[0]"},{"line_number":512,"context_line":"        instance_uuid \u003d self.inst[\u0027uuid\u0027]"},{"line_number":513,"context_line":"        self.assertEqual(1, len(self.tracker.allocations.get(instance_uuid)))"},{"line_number":514,"context_line":"        self.assertIn(allocated_dev.id,"},{"line_number":515,"context_line":"                      [pci_dev.id for pci_dev in"},{"line_number":516,"context_line":"                       self.tracker.allocations.get(instance_uuid)])"},{"line_number":517,"context_line":"        self.assertEqual(1, len(self.tracker.claims.get(instance_uuid)))"},{"line_number":518,"context_line":"        self.assertIn(claimed_dev.id,"},{"line_number":519,"context_line":"                      [pci_dev.id for pci_dev in"},{"line_number":520,"context_line":"                       self.tracker.claims.get(instance_uuid)])"},{"line_number":521,"context_line":"        self.tracker.free_instance_allocations(mock.sentinel.context,"},{"line_number":522,"context_line":"                                               self.inst)"},{"line_number":523,"context_line":"        self.assertIsNone(self.tracker.allocations.get(instance_uuid))"},{"line_number":524,"context_line":"        self.assertEqual(1, len(self.tracker.claims.get(instance_uuid)))"},{"line_number":525,"context_line":"        self.tracker.free_instance_claims(mock.sentinel.context, self.inst)"},{"line_number":526,"context_line":"        self.assertIsNone(self.tracker.claims.get(instance_uuid))"},{"line_number":527,"context_line":"        free_devs \u003d self.tracker.pci_stats.get_free_devs()"},{"line_number":528,"context_line":"        self.assertEqual(len(fake_db_devs), len(free_devs))"},{"line_number":529,"context_line":""},{"line_number":530,"context_line":""},{"line_number":531,"context_line":"class PciGetInstanceDevs(test.NoDBTestCase):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dfd5e7cf_6651cf62","line":528,"range":{"start_line":503,"start_character":0,"end_line":528,"end_character":59},"updated":"2019-01-09 17:57:30.000000000","message":"I know we don\u0027t always have them elsewhere, but comments explaining your logic as you go through this would be mighty appreciated. Also, this could conceivably be split in two but I\u0027m not that bothered about that","commit_id":"90c350fce0a30e59de753fddd6648513cc326ef7"},{"author":{"_account_id":28714,"name":"Adrian Chiris","email":"adrianc@nvidia.com","username":"adrianc"},"change_message_id":"321d68335a446304aadf0429eb304aebe1b1c658","unresolved":false,"context_lines":[{"line_number":500,"context_line":"        self.assertIsNone(self.tracker.allocations.get(instance_uuid))"},{"line_number":501,"context_line":""},{"line_number":502,"context_line":"    def test_free_claims_and_allocations(self):"},{"line_number":503,"context_line":"        pci_requests_obj \u003d self._create_pci_requests_object("},{"line_number":504,"context_line":"            [{\u0027count\u0027: 1, \u0027spec\u0027: [{\u0027vendor_id\u0027: \u0027v\u0027}]}])"},{"line_number":505,"context_line":"        allocated_devs \u003d self.tracker.claim_instance(mock.sentinel.context,"},{"line_number":506,"context_line":"                                    pci_requests_obj, None)"},{"line_number":507,"context_line":"        self.tracker.allocate_instance(self.inst)"},{"line_number":508,"context_line":"        claimed_devs \u003d self.tracker.claim_instance(mock.sentinel.context,"},{"line_number":509,"context_line":"                                    pci_requests_obj, None)"},{"line_number":510,"context_line":"        allocated_dev \u003d allocated_devs[0]"},{"line_number":511,"context_line":"        claimed_dev \u003d claimed_devs[0]"},{"line_number":512,"context_line":"        instance_uuid \u003d self.inst[\u0027uuid\u0027]"},{"line_number":513,"context_line":"        self.assertEqual(1, len(self.tracker.allocations.get(instance_uuid)))"},{"line_number":514,"context_line":"        self.assertIn(allocated_dev.id,"},{"line_number":515,"context_line":"                      [pci_dev.id for pci_dev in"},{"line_number":516,"context_line":"                       self.tracker.allocations.get(instance_uuid)])"},{"line_number":517,"context_line":"        self.assertEqual(1, len(self.tracker.claims.get(instance_uuid)))"},{"line_number":518,"context_line":"        self.assertIn(claimed_dev.id,"},{"line_number":519,"context_line":"                      [pci_dev.id for pci_dev in"},{"line_number":520,"context_line":"                       self.tracker.claims.get(instance_uuid)])"},{"line_number":521,"context_line":"        self.tracker.free_instance_allocations(mock.sentinel.context,"},{"line_number":522,"context_line":"                                               self.inst)"},{"line_number":523,"context_line":"        self.assertIsNone(self.tracker.allocations.get(instance_uuid))"},{"line_number":524,"context_line":"        self.assertEqual(1, len(self.tracker.claims.get(instance_uuid)))"},{"line_number":525,"context_line":"        self.tracker.free_instance_claims(mock.sentinel.context, self.inst)"},{"line_number":526,"context_line":"        self.assertIsNone(self.tracker.claims.get(instance_uuid))"},{"line_number":527,"context_line":"        free_devs \u003d self.tracker.pci_stats.get_free_devs()"},{"line_number":528,"context_line":"        self.assertEqual(len(fake_db_devs), len(free_devs))"},{"line_number":529,"context_line":""},{"line_number":530,"context_line":""},{"line_number":531,"context_line":"class PciGetInstanceDevs(test.NoDBTestCase):"}],"source_content_type":"text/x-python","patch_set":4,"id":"bfdaf3ff_618fa0cc","line":528,"range":{"start_line":503,"start_character":0,"end_line":528,"end_character":59},"in_reply_to":"dfd5e7cf_6651cf62","updated":"2019-01-15 11:17:15.000000000","message":"Done","commit_id":"90c350fce0a30e59de753fddd6648513cc326ef7"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"3ec9627fde7c7d2e247d9e97a396b721e56fdb22","unresolved":false,"context_lines":[{"line_number":547,"context_line":"        self.assertIsNone(self.tracker.allocations.get(instance_uuid))"},{"line_number":548,"context_line":""},{"line_number":549,"context_line":"    def test_free_instance_claims(self):"},{"line_number":550,"context_line":"        # Create an InstancePCIRequest object"},{"line_number":551,"context_line":"        pci_requests_obj \u003d self._create_pci_requests_object("},{"line_number":552,"context_line":"            [{\u0027count\u0027: 1, \u0027spec\u0027: [{\u0027vendor_id\u0027: \u0027v\u0027}]}])"},{"line_number":553,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"5fc1f717_0e1355b7","line":550,"updated":"2019-03-07 15:44:21.000000000","message":"just a nit for future patches... comments like this (or below like \"Free instance claims\") aren\u0027t particularly useful since they are redundant with the name of the function called directly beneath them.\n\nI know it\u0027s often good practice to code up a test by first outlining the different test steps into code comments and then \"filling in\" the test by calling the code for each of those steps, but generally good to remove single-line comments that simply repeat the function you\u0027re calling below them.","commit_id":"2a3179affb07e26202c7c7896b40368843bdc47a"}]}