)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"39fb9b9a2760f92f6a2fd277719e890481a4b7b7","unresolved":true,"context_lines":[{"line_number":25,"context_line":"in all cases, resulting in the PCI device not being removed from the"},{"line_number":26,"context_line":"instance.pci_devices list."},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"This change fixes the issue by using a reference to the original instance"},{"line_number":29,"context_line":"object in the Claim object, rather than a clone. The PCI devices are"},{"line_number":30,"context_line":"then removed from the original object if the claim is aborted."},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"A few small changes were also required to the PCI manager"},{"line_number":33,"context_line":"to pass through an instance object to the free() method of the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"e746c33b_fe31bdf8","line":30,"range":{"start_line":28,"start_character":0,"end_line":30,"end_character":62},"updated":"2022-02-01 12:06:34.000000000","message":"do we ensure that not just the PCI devices but other resource descriptions like numa_topology is also properly cleaned up from the original instance object?","commit_id":"249ca3bc7903a79ef8d98f2e360cc71aab1916e1"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"39fb9b9a2760f92f6a2fd277719e890481a4b7b7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"f89343ed_adb3864f","updated":"2022-02-01 12:06:34.000000000","message":"I have some comments inline","commit_id":"249ca3bc7903a79ef8d98f2e360cc71aab1916e1"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"a017329331e0201eb90a89c1b6fce609b63aa15c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"de887c34_5ef5bc26","updated":"2022-11-17 16:31:36.000000000","message":"So this patch is simply an alternative to https://review.opendev.org/c/openstack/nova/+/710848\n\nI prefer the above patch over this one.","commit_id":"83376579f09c169c22b7ea93676919250b72c3a7"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"669194795aef1eb337bd40dcd1f8c11d8bf045cd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"11ee96fc_0c6d1503","updated":"2022-11-17 11:34:02.000000000","message":"just pushing a draft comment formn 1 and a half years ago...","commit_id":"83376579f09c169c22b7ea93676919250b72c3a7"}],"nova/compute/claims.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2c9dc5bb03e9b9a5e1fc8090a5ea492af4ce0224","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        been aborted."},{"line_number":86,"context_line":"        \"\"\""},{"line_number":87,"context_line":"        LOG.debug(\"Aborting claim: %s\", self, instance\u003dself.instance_orig)"},{"line_number":88,"context_line":"        self.tracker.abort_instance_claim(self.context, self.instance_orig,"},{"line_number":89,"context_line":"                                          self.nodename)"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    def _claim_test(self, compute_node, limits\u003dNone):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_f5090b2b","line":88,"range":{"start_line":88,"start_character":55,"end_line":88,"end_character":75},"updated":"2020-09-07 13:06:02.000000000","message":"this","commit_id":"6c6ab999c57e795686b2f565647c0c87e724f7c0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2c9dc5bb03e9b9a5e1fc8090a5ea492af4ce0224","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        been aborted."},{"line_number":186,"context_line":"        \"\"\""},{"line_number":187,"context_line":"        LOG.debug(\"Aborting claim: %s\", self, instance\u003dself.instance_orig)"},{"line_number":188,"context_line":"        self.tracker.drop_move_claim("},{"line_number":189,"context_line":"            self.context,"},{"line_number":190,"context_line":"            self.instance_orig, self.nodename,"},{"line_number":191,"context_line":"            instance_type\u003dself.instance_type)"},{"line_number":192,"context_line":"        self.instance_orig.drop_migration_context()"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"    def _test_pci(self):"},{"line_number":195,"context_line":"        \"\"\"Test whether this host can accept this claim\u0027s PCI requests. For"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_3538a3d1","line":192,"range":{"start_line":188,"start_character":7,"end_line":192,"end_character":51},"updated":"2020-09-07 13:06:02.000000000","message":"and this are also now using the orginal instance instead of the updated one and are part of the fix.\n\nwith the logs also updated to use the original instance.","commit_id":"6c6ab999c57e795686b2f565647c0c87e724f7c0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c7c722d5adb63e6e043292b7b76d60ea21f75e6f","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        super(Claim, self).__init__(migration\u003dmigration)"},{"line_number":63,"context_line":"        # Stash a copy of the instance at the current point of time"},{"line_number":64,"context_line":"        self.instance \u003d instance.obj_clone()"},{"line_number":65,"context_line":"        # NOTE(mgoddard) Also keep a copy of the original instance object to"},{"line_number":66,"context_line":"        # use if the claim is aborted. Using the original object ensures that"},{"line_number":67,"context_line":"        # changes such as PCI device deallocation are reflected in the object"},{"line_number":68,"context_line":"        # seen by the caller."}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_e088e122","line":65,"range":{"start_line":65,"start_character":35,"end_line":65,"end_character":41},"updated":"2020-10-13 10:00:20.000000000","message":"this is not a copy, this is the reference of the actively managed instance object. The copy is the self.instance cloned above.\n\nDo we actually depend on the that self.instance is a copy at the time of the claim constructor? Or we can simply use the active instance object everywhere?","commit_id":"5063bbcd623c663b224f224b9bcdc51456740f55"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"4685974fe641c826ea066a7c3f9b480535c93961","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        super(Claim, self).__init__(migration\u003dmigration)"},{"line_number":63,"context_line":"        # Stash a copy of the instance at the current point of time"},{"line_number":64,"context_line":"        self.instance \u003d instance.obj_clone()"},{"line_number":65,"context_line":"        # NOTE(mgoddard) Also keep a copy of the original instance object to"},{"line_number":66,"context_line":"        # use if the claim is aborted. Using the original object ensures that"},{"line_number":67,"context_line":"        # changes such as PCI device deallocation are reflected in the object"},{"line_number":68,"context_line":"        # seen by the caller."}],"source_content_type":"text/x-python","patch_set":2,"id":"7f6b1bfe_3662f817","line":65,"range":{"start_line":65,"start_character":35,"end_line":65,"end_character":41},"in_reply_to":"9f560f44_e088e122","updated":"2020-10-13 13:35:24.000000000","message":"Using the active instance object would be cleaner, I was just unsure of the consequences of doing so, and the reason for using a clone.","commit_id":"5063bbcd623c663b224f224b9bcdc51456740f55"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"bd5fed29f65ecb329d6524997eabf8cb39bfbeda","unresolved":false,"context_lines":[{"line_number":84,"context_line":"        \"\"\"Compute operation requiring claimed resources has failed or"},{"line_number":85,"context_line":"        been aborted."},{"line_number":86,"context_line":"        \"\"\""},{"line_number":87,"context_line":"        LOG.debug(\"Aborting claim: %s\", self, instance\u003dself.instance_orig)"},{"line_number":88,"context_line":"        self.tracker.abort_instance_claim(self.context, self.instance_orig,"},{"line_number":89,"context_line":"                                          self.nodename)"},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    def _claim_test(self, compute_node, limits\u003dNone):"},{"line_number":92,"context_line":"        \"\"\"Test if this claim can be satisfied given available resources and"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f6b1bfe_3fbdab68","line":89,"range":{"start_line":87,"start_character":0,"end_line":89,"end_character":56},"updated":"2020-10-13 15:07:11.000000000","message":"Having stared at this code a bit more, I think the fundamental issue is that the interaction between the RT and claims is inconsistent. Generally, a claim has read-only access to the RT to check if a claim fits. If successful, the RT is then responsible for allocating those resources to the instance (e.g. in instance_claim). In abort() however, the claim pulls the strings.\n\nA better design would be to implement the context manager in the RT, and handle the abort there.","commit_id":"5063bbcd623c663b224f224b9bcdc51456740f55"},{"author":{"_account_id":21813,"name":"Andrey Volkov","email":"m@amadev.ru","username":"avolkov"},"change_message_id":"2d98f263d999c1021cf425623dabe3632c5522ae","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    def __init__(self, context, instance, nodename, tracker, compute_node,"},{"line_number":61,"context_line":"                 pci_requests, migration\u003dNone, limits\u003dNone):"},{"line_number":62,"context_line":"        super(Claim, self).__init__(migration\u003dmigration)"},{"line_number":63,"context_line":"        # NOTE(mgoddard) Previously this made a clone of the instance object."},{"line_number":64,"context_line":"        # That caused issues with aborted claims, since the resources released"},{"line_number":65,"context_line":"        # (such as PCI devices) would still be present in the original instance"},{"line_number":66,"context_line":"        # object held by the caller."},{"line_number":67,"context_line":"        self.instance \u003d instance"},{"line_number":68,"context_line":"        self.nodename \u003d nodename"},{"line_number":69,"context_line":"        self.tracker \u003d tracker"},{"line_number":70,"context_line":"        self._pci_requests \u003d pci_requests"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f65232a_ea67ba24","line":67,"range":{"start_line":63,"start_character":0,"end_line":67,"end_character":32},"updated":"2020-10-26 08:41:27.000000000","message":"Not sure if there is strong reasoning behind \"Stash a copy of the instance\", but in case of move claim, compute manager \nprovides \"original\" instance directly like in (nova.compute.resource_tracker.ResourceTracker._drop_pci_devices)\n\nSo maybe claim should not modify instance by itself or its overthinking?","commit_id":"240acc1b7636582f99673c3de055e7eb0fdaf9bf"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"809cca0e556941bbd386896488ee772f7bbe06ae","unresolved":false,"context_lines":[{"line_number":60,"context_line":"    def __init__(self, context, instance, nodename, tracker, compute_node,"},{"line_number":61,"context_line":"                 pci_requests, migration\u003dNone, limits\u003dNone):"},{"line_number":62,"context_line":"        super(Claim, self).__init__(migration\u003dmigration)"},{"line_number":63,"context_line":"        # NOTE(mgoddard) Previously this made a clone of the instance object."},{"line_number":64,"context_line":"        # That caused issues with aborted claims, since the resources released"},{"line_number":65,"context_line":"        # (such as PCI devices) would still be present in the original instance"},{"line_number":66,"context_line":"        # object held by the caller."},{"line_number":67,"context_line":"        self.instance \u003d instance"},{"line_number":68,"context_line":"        self.nodename \u003d nodename"},{"line_number":69,"context_line":"        self.tracker \u003d tracker"},{"line_number":70,"context_line":"        self._pci_requests \u003d pci_requests"}],"source_content_type":"text/x-python","patch_set":3,"id":"3f65232a_65f63b92","line":67,"range":{"start_line":63,"start_character":0,"end_line":67,"end_character":32},"in_reply_to":"3f65232a_ea67ba24","updated":"2020-10-26 09:53:17.000000000","message":"I\u0027m not certain to be honest. The clone goes back a long time.\n\nUltimately, if the code path leads to a DB update, then it seems to me more correct if that change is reflected in the object.","commit_id":"240acc1b7636582f99673c3de055e7eb0fdaf9bf"}],"nova/objects/pci_device.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2c9dc5bb03e9b9a5e1fc8090a5ea492af4ce0224","unresolved":false,"context_lines":[{"line_number":437,"context_line":"        if old_status \u003d\u003d fields.PciDeviceStatus.ALLOCATED and instance:"},{"line_number":438,"context_line":"            # Notes(yjiang5): remove this check when instance object for"},{"line_number":439,"context_line":"            # compute manager is finished"},{"line_number":440,"context_line":"            matching \u003d [dev for dev in instance[\u0027pci_devices\u0027]"},{"line_number":441,"context_line":"                        if dev.id \u003d\u003d self.id]"},{"line_number":442,"context_line":"            if matching:"},{"line_number":443,"context_line":"                if isinstance(instance, dict):"},{"line_number":444,"context_line":"                    instance[\u0027pci_devices\u0027].remove(matching[0])"},{"line_number":445,"context_line":"                else:"},{"line_number":446,"context_line":"                    instance.pci_devices.objects.remove(matching[0])"},{"line_number":447,"context_line":"        return free_devs"},{"line_number":448,"context_line":""},{"line_number":449,"context_line":"    def is_available(self):"},{"line_number":450,"context_line":"        return self.status \u003d\u003d fields.PciDeviceStatus.AVAILABLE"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_3a5c104a","line":447,"range":{"start_line":440,"start_character":9,"end_line":447,"end_character":24},"updated":"2020-09-07 13:06:02.000000000","message":"this should have teh same behavior but its also less efficent as it creating a list instead of iterating the generator expression\n\nwhy have you changed this?\nit seams unrelated hence -1","commit_id":"6c6ab999c57e795686b2f565647c0c87e724f7c0"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"7802562be83146084820d0e25b414e1224cc6268","unresolved":false,"context_lines":[{"line_number":437,"context_line":"        if old_status \u003d\u003d fields.PciDeviceStatus.ALLOCATED and instance:"},{"line_number":438,"context_line":"            # Notes(yjiang5): remove this check when instance object for"},{"line_number":439,"context_line":"            # compute manager is finished"},{"line_number":440,"context_line":"            matching \u003d [dev for dev in instance[\u0027pci_devices\u0027]"},{"line_number":441,"context_line":"                        if dev.id \u003d\u003d self.id]"},{"line_number":442,"context_line":"            if matching:"},{"line_number":443,"context_line":"                if isinstance(instance, dict):"},{"line_number":444,"context_line":"                    instance[\u0027pci_devices\u0027].remove(matching[0])"},{"line_number":445,"context_line":"                else:"},{"line_number":446,"context_line":"                    instance.pci_devices.objects.remove(matching[0])"},{"line_number":447,"context_line":"        return free_devs"},{"line_number":448,"context_line":""},{"line_number":449,"context_line":"    def is_available(self):"},{"line_number":450,"context_line":"        return self.status \u003d\u003d fields.PciDeviceStatus.AVAILABLE"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_4e0f4e38","line":447,"range":{"start_line":440,"start_character":9,"end_line":447,"end_character":24},"in_reply_to":"9f560f44_3a5c104a","updated":"2020-10-12 13:28:21.000000000","message":"The behaviour is different if the device isn\u0027t in instance[\u0027pci_devices\u0027]. It\u0027s been a long time since I wrote this patch, but I imagine I was hitting that code path and got a StopIteration.","commit_id":"6c6ab999c57e795686b2f565647c0c87e724f7c0"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c7c722d5adb63e6e043292b7b76d60ea21f75e6f","unresolved":false,"context_lines":[{"line_number":435,"context_line":"        self.instance_uuid \u003d None"},{"line_number":436,"context_line":"        self.request_id \u003d None"},{"line_number":437,"context_line":"        if old_status \u003d\u003d fields.PciDeviceStatus.ALLOCATED and instance:"},{"line_number":438,"context_line":"            # Notes(yjiang5): remove this check when instance object for"},{"line_number":439,"context_line":"            # compute manager is finished"},{"line_number":440,"context_line":"            matching \u003d [dev for dev in instance[\u0027pci_devices\u0027]"},{"line_number":441,"context_line":"                        if dev.id \u003d\u003d self.id]"},{"line_number":442,"context_line":"            if matching:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_00e515db","line":439,"range":{"start_line":438,"start_character":0,"end_line":439,"end_character":41},"updated":"2020-10-13 10:00:20.000000000","message":"This should be top of the isinstance condition","commit_id":"5063bbcd623c663b224f224b9bcdc51456740f55"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"4685974fe641c826ea066a7c3f9b480535c93961","unresolved":false,"context_lines":[{"line_number":435,"context_line":"        self.instance_uuid \u003d None"},{"line_number":436,"context_line":"        self.request_id \u003d None"},{"line_number":437,"context_line":"        if old_status \u003d\u003d fields.PciDeviceStatus.ALLOCATED and instance:"},{"line_number":438,"context_line":"            # Notes(yjiang5): remove this check when instance object for"},{"line_number":439,"context_line":"            # compute manager is finished"},{"line_number":440,"context_line":"            matching \u003d [dev for dev in instance[\u0027pci_devices\u0027]"},{"line_number":441,"context_line":"                        if dev.id \u003d\u003d self.id]"},{"line_number":442,"context_line":"            if matching:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f6b1bfe_5904077c","line":439,"range":{"start_line":438,"start_character":0,"end_line":439,"end_character":41},"in_reply_to":"9f560f44_00e515db","updated":"2020-10-13 13:35:24.000000000","message":"Done","commit_id":"5063bbcd623c663b224f224b9bcdc51456740f55"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"669194795aef1eb337bd40dcd1f8c11d8bf045cd","unresolved":true,"context_lines":[{"line_number":487,"context_line":"        if old_status \u003d\u003d fields.PciDeviceStatus.ALLOCATED and instance:"},{"line_number":488,"context_line":"            # Notes(yjiang5): remove this check when instance object for"},{"line_number":489,"context_line":"            # compute manager is finished"},{"line_number":490,"context_line":"            existed \u003d next((dev for dev in instance[\u0027pci_devices\u0027]"},{"line_number":491,"context_line":"                if dev.id \u003d\u003d self.id))"},{"line_number":492,"context_line":"            if isinstance(instance, dict):"},{"line_number":493,"context_line":"                instance[\u0027pci_devices\u0027].remove(existed)"},{"line_number":494,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"777df7d6_04158d88","side":"PARENT","line":491,"range":{"start_line":490,"start_character":11,"end_line":491,"end_character":38},"updated":"2022-11-17 11:34:02.000000000","message":"nit: technically the new code is less efficnet since this was using a generator expression.\n\nif you wanted to prevent a stop iteration exception from being raised then the correct way to do that is to do \nexisted \u003d next((dev for dev in instance[\u0027pci_devices\u0027] if dev.id \u003d\u003d self.id), None)","commit_id":"b9c48afd1516023839dd32b96f3eece36b164a8c"}],"nova/pci/manager.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2c9dc5bb03e9b9a5e1fc8090a5ea492af4ce0224","unresolved":false,"context_lines":[{"line_number":291,"context_line":"                    instance[\u0027uuid\u0027], pci_dev, self.allocations)"},{"line_number":292,"context_line":"                self._remove_device_from_pci_mapping("},{"line_number":293,"context_line":"                    instance[\u0027uuid\u0027], pci_dev, self.claims)"},{"line_number":294,"context_line":"                self._free_device(pci_dev, instance)"},{"line_number":295,"context_line":"                break"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    def _remove_device_from_pci_mapping("}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_da30740a","line":294,"range":{"start_line":294,"start_character":14,"end_line":294,"end_character":52},"updated":"2020-09-07 13:06:02.000000000","message":"so passing instance is the fix?","commit_id":"6c6ab999c57e795686b2f565647c0c87e724f7c0"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"7802562be83146084820d0e25b414e1224cc6268","unresolved":false,"context_lines":[{"line_number":291,"context_line":"                    instance[\u0027uuid\u0027], pci_dev, self.allocations)"},{"line_number":292,"context_line":"                self._remove_device_from_pci_mapping("},{"line_number":293,"context_line":"                    instance[\u0027uuid\u0027], pci_dev, self.claims)"},{"line_number":294,"context_line":"                self._free_device(pci_dev, instance)"},{"line_number":295,"context_line":"                break"},{"line_number":296,"context_line":""},{"line_number":297,"context_line":"    def _remove_device_from_pci_mapping("}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_aefd4a0a","line":294,"range":{"start_line":294,"start_character":14,"end_line":294,"end_character":52},"in_reply_to":"9f560f44_da30740a","updated":"2020-10-12 13:28:21.000000000","message":"it\u0027s part of the fix, yes.","commit_id":"6c6ab999c57e795686b2f565647c0c87e724f7c0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2c9dc5bb03e9b9a5e1fc8090a5ea492af4ce0224","unresolved":false,"context_lines":[{"line_number":307,"context_line":"                pci_mapping.pop(instance_uuid, None)"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"    def _free_device(self, dev, instance\u003dNone):"},{"line_number":310,"context_line":"        freed_devs \u003d dev.free(instance)"},{"line_number":311,"context_line":"        stale \u003d self.stale.pop(dev.address, None)"},{"line_number":312,"context_line":"        if stale:"},{"line_number":313,"context_line":"            dev.update_device(stale)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9f560f44_35ab83d6","line":310,"range":{"start_line":310,"start_character":5,"end_line":310,"end_character":39},"updated":"2020-09-07 13:06:02.000000000","message":"so by passing instnace in stead of not we nolonger pass None here.\n\nwhich enables this uuid check\n\nhttps://github.com/openstack/nova/blob/master/nova/objects/pci_device.py#L407-L411\n\nto ensure the device is owned by the current instanace.\n\nbut more importantly \n\nhttps://github.com/openstack/nova/blob/master/nova/objects/pci_device.py#L442-L445\n\ncauses the   instance.pci_devices.objects to be updated.\n\nso yes this seams correct.","commit_id":"6c6ab999c57e795686b2f565647c0c87e724f7c0"}],"nova/tests/functional/notification_sample_tests/test_instance.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"39fb9b9a2760f92f6a2fd277719e890481a4b7b7","unresolved":true,"context_lines":[{"line_number":468,"context_line":"                \u0027reservation_id\u0027: server[\u0027reservation_id\u0027],"},{"line_number":469,"context_line":"                \u0027uuid\u0027: server[\u0027id\u0027],"},{"line_number":470,"context_line":"                \u0027host\u0027: None,"},{"line_number":471,"context_line":"                \u0027node\u0027: None,"},{"line_number":472,"context_line":"                \u0027fault.traceback\u0027: self.ANY},"},{"line_number":473,"context_line":"            actual\u003dself.notifier.versioned_notifications[3])"},{"line_number":474,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"127a8ead_9caa1242","line":471,"updated":"2022-02-01 12:06:34.000000000","message":"This is indicating that the change also reset the host/node of the instance object to None during an aborted claim. \n\nI think that is probably OK. As it seems logical that if the claim is aborted then the instance is not tight to the host. I\u0027m not sure that instance is now properly buried to cell0 though.\n\nAnyhow if others agree that resetting host/node is valid and does not break anything else then instead of overriding the notification sample file here, I suggest to change the content of the sample directly in doc/notification_samples/instance-create-error.json by adding a host and node key there to override the common value inherited from doc/notification_samples/common_payloads/InstancePayload.json","commit_id":"249ca3bc7903a79ef8d98f2e360cc71aab1916e1"}],"nova/tests/unit/compute/test_shelve.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c7c722d5adb63e6e043292b7b76d60ea21f75e6f","unresolved":false,"context_lines":[{"line_number":524,"context_line":"                # This is after we\u0027ve failed."},{"line_number":525,"context_line":"                self.assertIsNone(instance.host)"},{"line_number":526,"context_line":"                self.assertIsNone(instance.node)"},{"line_number":527,"context_line":"                self.assertEqual(task_states.SPAWNING, instance.task_state)"},{"line_number":528,"context_line":"                tracking[\u0027last_state\u0027] \u003d instance.task_state"},{"line_number":529,"context_line":"            else:"},{"line_number":530,"context_line":"                self.fail(\u0027Unexpected save!\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9f560f44_e02901f2","line":527,"updated":"2020-10-13 10:00:20.000000000","message":"This seem unrelated. Could you explain why this test sees a different behavior?","commit_id":"5063bbcd623c663b224f224b9bcdc51456740f55"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"a2925d11f29c9cd5903e373d2fca16ab00733882","unresolved":false,"context_lines":[{"line_number":524,"context_line":"                # This is after we\u0027ve failed."},{"line_number":525,"context_line":"                self.assertIsNone(instance.host)"},{"line_number":526,"context_line":"                self.assertIsNone(instance.node)"},{"line_number":527,"context_line":"                self.assertEqual(task_states.SPAWNING, instance.task_state)"},{"line_number":528,"context_line":"                tracking[\u0027last_state\u0027] \u003d instance.task_state"},{"line_number":529,"context_line":"            else:"},{"line_number":530,"context_line":"                self.fail(\u0027Unexpected save!\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f6b1bfe_7c08d127","line":527,"in_reply_to":"7f6b1bfe_392aebe6","updated":"2020-10-13 13:51:07.000000000","message":"Turns out the code never got here. The second save() would have been performed on the clone, which is not mocked.","commit_id":"5063bbcd623c663b224f224b9bcdc51456740f55"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"4685974fe641c826ea066a7c3f9b480535c93961","unresolved":false,"context_lines":[{"line_number":524,"context_line":"                # This is after we\u0027ve failed."},{"line_number":525,"context_line":"                self.assertIsNone(instance.host)"},{"line_number":526,"context_line":"                self.assertIsNone(instance.node)"},{"line_number":527,"context_line":"                self.assertEqual(task_states.SPAWNING, instance.task_state)"},{"line_number":528,"context_line":"                tracking[\u0027last_state\u0027] \u003d instance.task_state"},{"line_number":529,"context_line":"            else:"},{"line_number":530,"context_line":"                self.fail(\u0027Unexpected save!\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f6b1bfe_392aebe6","line":527,"in_reply_to":"9f560f44_e02901f2","updated":"2020-10-13 13:35:24.000000000","message":"I\u0027m afraid I don\u0027t fully understand, but the change that triggers this is passing the instance reference (rather than a clone) to abort_instance_claim in claims.py. I\u0027ll try to dig into it.","commit_id":"5063bbcd623c663b224f224b9bcdc51456740f55"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"39fb9b9a2760f92f6a2fd277719e890481a4b7b7","unresolved":true,"context_lines":[{"line_number":594,"context_line":"                # This is after we\u0027ve failed."},{"line_number":595,"context_line":"                self.assertIsNone(instance.host)"},{"line_number":596,"context_line":"                self.assertIsNone(instance.node)"},{"line_number":597,"context_line":"                self.assertEqual(task_states.SPAWNING, instance.task_state)"},{"line_number":598,"context_line":"                tracking[\u0027last_state\u0027] \u003d instance.task_state"},{"line_number":599,"context_line":"            else:"},{"line_number":600,"context_line":"                self.fail(\u0027Unexpected save!\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"8b5a57fe_2088494d","line":597,"updated":"2022-02-01 12:06:34.000000000","message":"That also indicates some change in how the instance object will look like after the aborted claim. I think here we want to have the original behavior so that the task state is eventually reset to None for the failed build","commit_id":"249ca3bc7903a79ef8d98f2e360cc71aab1916e1"}]}