)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"6252f016d78571da47a4c9eb04e78029a8ccb947","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"25bce0d4_02e14967","updated":"2026-04-04 09:09:42.000000000","message":"recheck dep updated","commit_id":"0571c0560a9573759cec7577fabf3c9adfba681f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"34f8a5e4e4989e442e26eea94b1cc3f43cf1958a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"a50eb15e_1634b3b3","updated":"2026-04-05 18:27:15.000000000","message":"recheck dep updated","commit_id":"0278a0db25a48bf3701962762e36d35bab14bc34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"cecd30cf4870308d36ec6d84bf7cb7ac6a2f2b4b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"d4c022e2_e7bf1568","updated":"2026-04-05 20:59:20.000000000","message":"recheck dep updated","commit_id":"0278a0db25a48bf3701962762e36d35bab14bc34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c835e439ae42849d733a4dc65dbeeaf836a42a43","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"efbc8959_735ed915","updated":"2026-04-05 23:52:30.000000000","message":"recheck dep updated","commit_id":"0278a0db25a48bf3701962762e36d35bab14bc34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0d72be0ba8bb9595b97f599a50340b6e9c777ed5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f68a66eb_7f0eb13f","updated":"2026-04-05 00:07:01.000000000","message":"recheck dep updated","commit_id":"0278a0db25a48bf3701962762e36d35bab14bc34"}],"nova/crypto.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7b68dfc6cd454aeead5a5596f15b6067444f7219","unresolved":true,"context_lines":[{"line_number":272,"context_line":"                  secret_uuid, instance\u003dinstance)"},{"line_number":273,"context_line":"    except castellan_exception.KeyManagerError as e:"},{"line_number":274,"context_line":"        _handle_key_manager_error_forbidden(e, instance)"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    del instance.system_metadata[\u0027vtpm_secret_uuid\u0027]"},{"line_number":277,"context_line":"    instance.save()"},{"line_number":278,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"e60c485b_077742a7","line":275,"updated":"2026-04-06 16:07:26.000000000","message":"Not that it\u0027s wrong, but.. I\u0027m struggling to think of why we would need this coverage with (or because of) user support. With user we shouldn\u0027t be deleting the secret in barbican during the migration right?","commit_id":"0278a0db25a48bf3701962762e36d35bab14bc34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"425ec3db109f8879d69e9c2eba8ef5c7012df331","unresolved":true,"context_lines":[{"line_number":272,"context_line":"                  secret_uuid, instance\u003dinstance)"},{"line_number":273,"context_line":"    except castellan_exception.KeyManagerError as e:"},{"line_number":274,"context_line":"        _handle_key_manager_error_forbidden(e, instance)"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    del instance.system_metadata[\u0027vtpm_secret_uuid\u0027]"},{"line_number":277,"context_line":"    instance.save()"},{"line_number":278,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"f7e17e2f_8c93756e","line":275,"in_reply_to":"e60c485b_077742a7","updated":"2026-04-06 18:47:18.000000000","message":"Hm, that\u0027s fair. I think I probably thought since I need to do it for get/create might as well cover delete too since none of them are handling it now \u003d\u003e all of them are handling it now.\n\nDefinitely need to add it for get/create because live migration maybe makes it more likely someone would hit Forbidden and to handle it better knowing people are going to do it.\n\nIt\u0027s true the secret will never be deleted from Barbican during any migration. Only instance delete will be a safe time to delete it. So this would cover something like say, an admin trying to delete a different user\u0027s instance that has vTPM.\n\nSo this could also be its own patch, to handle Forbidden across the board. Maybe also worth splitting out thusly.","commit_id":"0278a0db25a48bf3701962762e36d35bab14bc34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"50ce480fe9d74456ab230a40215d48c65c20405d","unresolved":true,"context_lines":[{"line_number":272,"context_line":"                  secret_uuid, instance\u003dinstance)"},{"line_number":273,"context_line":"    except castellan_exception.KeyManagerError as e:"},{"line_number":274,"context_line":"        _handle_key_manager_error_forbidden(e, instance)"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    del instance.system_metadata[\u0027vtpm_secret_uuid\u0027]"},{"line_number":277,"context_line":"    instance.save()"},{"line_number":278,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"210f0b74_d167369f","line":275,"in_reply_to":"f7e17e2f_8c93756e","updated":"2026-04-07 03:40:03.000000000","message":"This is split out at https://review.opendev.org/c/openstack/nova/+/983504","commit_id":"0278a0db25a48bf3701962762e36d35bab14bc34"}],"nova/tests/fixtures/libvirt.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7b68dfc6cd454aeead5a5596f15b6067444f7219","unresolved":true,"context_lines":[{"line_number":2067,"context_line":""},{"line_number":2068,"context_line":"    def _add_secret(self, secret):"},{"line_number":2069,"context_line":"        self._secrets[secret._uuid] \u003d secret"},{"line_number":2070,"context_line":"        self._removed_secrets.pop(secret._uuid, None)"},{"line_number":2071,"context_line":""},{"line_number":2072,"context_line":"    def _remove_secret(self, secret):"},{"line_number":2073,"context_line":"        self._removed_secrets[secret._uuid] \u003d self._secrets.pop(secret._uuid)"}],"source_content_type":"text/x-python","patch_set":3,"id":"c766fd4c_76a38aac","line":2070,"updated":"2026-04-06 16:07:26.000000000","message":"This kinda breaks my brain a bit.. I understand why we\u0027re tracking the undefining of the secrets, but not sure I get why we\u0027re removing said tracking when we add it :P","commit_id":"0278a0db25a48bf3701962762e36d35bab14bc34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"50ce480fe9d74456ab230a40215d48c65c20405d","unresolved":true,"context_lines":[{"line_number":2067,"context_line":""},{"line_number":2068,"context_line":"    def _add_secret(self, secret):"},{"line_number":2069,"context_line":"        self._secrets[secret._uuid] \u003d secret"},{"line_number":2070,"context_line":"        self._removed_secrets.pop(secret._uuid, None)"},{"line_number":2071,"context_line":""},{"line_number":2072,"context_line":"    def _remove_secret(self, secret):"},{"line_number":2073,"context_line":"        self._removed_secrets[secret._uuid] \u003d self._secrets.pop(secret._uuid)"}],"source_content_type":"text/x-python","patch_set":3,"id":"93a9f11a_93ccb626","line":2070,"in_reply_to":"540a2512_3a5884d0","updated":"2026-04-07 03:40:03.000000000","message":"I removed this in part because Claude agreed that the assert was not really helping much for how confusing it was 🙂","commit_id":"0278a0db25a48bf3701962762e36d35bab14bc34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"425ec3db109f8879d69e9c2eba8ef5c7012df331","unresolved":true,"context_lines":[{"line_number":2067,"context_line":""},{"line_number":2068,"context_line":"    def _add_secret(self, secret):"},{"line_number":2069,"context_line":"        self._secrets[secret._uuid] \u003d secret"},{"line_number":2070,"context_line":"        self._removed_secrets.pop(secret._uuid, None)"},{"line_number":2071,"context_line":""},{"line_number":2072,"context_line":"    def _remove_secret(self, secret):"},{"line_number":2073,"context_line":"        self._removed_secrets[secret._uuid] \u003d self._secrets.pop(secret._uuid)"}],"source_content_type":"text/x-python","patch_set":3,"id":"540a2512_3a5884d0","line":2070,"in_reply_to":"c766fd4c_76a38aac","updated":"2026-04-06 18:47:18.000000000","message":"Yeah maybe I shouldn\u0027t have done this but I was being paranoid about if a secret got created and then deleted and then created again, I didn\u0027t want my \"assert had secret\" to be potentially representing the first create/delete event vs a second create/delete event. I\u0027m not sure if that makes sense but basically I didn\u0027t want one initial secret deletion to be \"remembered\" for the rest of the test if it was really multiple create/delete events.\n\nI can remove this if you think it\u0027s not helping.","commit_id":"0278a0db25a48bf3701962762e36d35bab14bc34"}],"nova/tests/functional/libvirt/test_vtpm.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7b68dfc6cd454aeead5a5596f15b6067444f7219","unresolved":true,"context_lines":[{"line_number":221,"context_line":"        instance \u003d objects.Instance.get_by_uuid(ctx, instance_uuid)"},{"line_number":222,"context_line":"        secret_uuid \u003d instance.system_metadata[\u0027vtpm_secret_uuid\u0027]"},{"line_number":223,"context_line":"        self.assertEqual(secret_uuid, s.UUIDString())"},{"line_number":224,"context_line":"        # We expect this secret to not show as having been undefined."},{"line_number":225,"context_line":"        conn \u003d host.driver._host.get_connection()"},{"line_number":226,"context_line":"        self.assertNotIn(secret_uuid, conn._removed_secrets)"},{"line_number":227,"context_line":"        return s"}],"source_content_type":"text/x-python","patch_set":3,"id":"bfa52be4_83b0bb14","line":224,"updated":"2026-04-06 16:07:26.000000000","message":"This too seems like a lot of negatives.. I think this is saying \"we expect this secret to be present and accessible\". However, user mode should have undefined secrets right? So I\u0027m still wondering why we\u0027re needing to make this change in order to support user mode... I thought maybe just for asserting in the interim between setting the secret and then undefining it, but I\u0027m not sure I see this below.\n\nMaybe this is just for internal fixture consistency checking or something? If so, maybe we could just say that here. If not, then.. I dunno, I\u0027m confused :)","commit_id":"0278a0db25a48bf3701962762e36d35bab14bc34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"425ec3db109f8879d69e9c2eba8ef5c7012df331","unresolved":true,"context_lines":[{"line_number":221,"context_line":"        instance \u003d objects.Instance.get_by_uuid(ctx, instance_uuid)"},{"line_number":222,"context_line":"        secret_uuid \u003d instance.system_metadata[\u0027vtpm_secret_uuid\u0027]"},{"line_number":223,"context_line":"        self.assertEqual(secret_uuid, s.UUIDString())"},{"line_number":224,"context_line":"        # We expect this secret to not show as having been undefined."},{"line_number":225,"context_line":"        conn \u003d host.driver._host.get_connection()"},{"line_number":226,"context_line":"        self.assertNotIn(secret_uuid, conn._removed_secrets)"},{"line_number":227,"context_line":"        return s"}],"source_content_type":"text/x-python","patch_set":3,"id":"fdce4323_26bdf5d0","line":224,"in_reply_to":"bfa52be4_83b0bb14","updated":"2026-04-06 18:47:18.000000000","message":"Yes that\u0027s right \"expect it to be present and accessible\". And yes \u0027user\u0027 mode will have undefined secrets.\n\nOn the code comment, I think it\u0027s obviously about the new assert on L226 but the reason I was thinking to do it is because I wanted to assert that the secret has _never_ been undefined (in the case of \u0027host\u0027). I wanted to be able to catch a potential mistake such as mixing the \u0027user\u0027 and \u0027host\u0027 code paths and not realizing it.\n\nBut yeah, it doesn\u0027t seem related to \u0027user\u0027 specifically ... it might be a mess up during the commit slicing I did. I\u0027ll check and see where I start needing the return value of this.\n\nBasically as I added \u0027user\u0027 and \u0027deployment\u0027 mode support I was thinking the asserts needed to be a lot more thorough in general to catch any mistakes with secret handling between the modes.","commit_id":"0278a0db25a48bf3701962762e36d35bab14bc34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"50ce480fe9d74456ab230a40215d48c65c20405d","unresolved":true,"context_lines":[{"line_number":221,"context_line":"        instance \u003d objects.Instance.get_by_uuid(ctx, instance_uuid)"},{"line_number":222,"context_line":"        secret_uuid \u003d instance.system_metadata[\u0027vtpm_secret_uuid\u0027]"},{"line_number":223,"context_line":"        self.assertEqual(secret_uuid, s.UUIDString())"},{"line_number":224,"context_line":"        # We expect this secret to not show as having been undefined."},{"line_number":225,"context_line":"        conn \u003d host.driver._host.get_connection()"},{"line_number":226,"context_line":"        self.assertNotIn(secret_uuid, conn._removed_secrets)"},{"line_number":227,"context_line":"        return s"}],"source_content_type":"text/x-python","patch_set":3,"id":"6596296f_560b90f1","line":224,"in_reply_to":"fdce4323_26bdf5d0","updated":"2026-04-07 03:40:03.000000000","message":"Same","commit_id":"0278a0db25a48bf3701962762e36d35bab14bc34"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7b68dfc6cd454aeead5a5596f15b6067444f7219","unresolved":true,"context_lines":[{"line_number":8440,"context_line":"                    destroy_vifs\u003dTrue,"},{"line_number":8441,"context_line":"                    cleanup_instance_dir\u003dcleanup_instance_dir,"},{"line_number":8442,"context_line":"                    cleanup_instance_disks\u003dcleanup_instance_disks)"},{"line_number":8443,"context_line":"                self._host.delete_secret(\u0027vtpm\u0027, instance.uuid)"},{"line_number":8444,"context_line":"                raise exception.VirtualInterfaceCreateException()"},{"line_number":8445,"context_line":"        except Exception:"},{"line_number":8446,"context_line":"            # Any other error, be sure to clean up"}],"source_content_type":"text/x-python","patch_set":3,"id":"43c92de0_a9669b30","line":8443,"updated":"2026-04-06 16:07:26.000000000","message":"NOTE to self: This deletes only if present.\n\nSeems like this is maybe a fix for user mode independent of live migration, no? If so, maybe we should split it out so we can/could backport?","commit_id":"0278a0db25a48bf3701962762e36d35bab14bc34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"425ec3db109f8879d69e9c2eba8ef5c7012df331","unresolved":true,"context_lines":[{"line_number":8440,"context_line":"                    destroy_vifs\u003dTrue,"},{"line_number":8441,"context_line":"                    cleanup_instance_dir\u003dcleanup_instance_dir,"},{"line_number":8442,"context_line":"                    cleanup_instance_disks\u003dcleanup_instance_disks)"},{"line_number":8443,"context_line":"                self._host.delete_secret(\u0027vtpm\u0027, instance.uuid)"},{"line_number":8444,"context_line":"                raise exception.VirtualInterfaceCreateException()"},{"line_number":8445,"context_line":"        except Exception:"},{"line_number":8446,"context_line":"            # Any other error, be sure to clean up"}],"source_content_type":"text/x-python","patch_set":3,"id":"9d79e760_224c5257","line":8443,"in_reply_to":"43c92de0_a9669b30","updated":"2026-04-06 18:47:18.000000000","message":"Hm yeah, I guess that\u0027s true. It became apparent because of \u0027user\u0027 and \u0027deployment\u0027 modes since they are focused on deleting the libvirt secret whenever possible -- and made more aware of when a secret might be erroneously left behind. But it would also apply in general, unrelated to vTPM live migration.\n\nSo yeah I could split it out.","commit_id":"0278a0db25a48bf3701962762e36d35bab14bc34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"50ce480fe9d74456ab230a40215d48c65c20405d","unresolved":true,"context_lines":[{"line_number":8440,"context_line":"                    destroy_vifs\u003dTrue,"},{"line_number":8441,"context_line":"                    cleanup_instance_dir\u003dcleanup_instance_dir,"},{"line_number":8442,"context_line":"                    cleanup_instance_disks\u003dcleanup_instance_disks)"},{"line_number":8443,"context_line":"                self._host.delete_secret(\u0027vtpm\u0027, instance.uuid)"},{"line_number":8444,"context_line":"                raise exception.VirtualInterfaceCreateException()"},{"line_number":8445,"context_line":"        except Exception:"},{"line_number":8446,"context_line":"            # Any other error, be sure to clean up"}],"source_content_type":"text/x-python","patch_set":3,"id":"025305b0_f3855c59","line":8443,"in_reply_to":"9d79e760_224c5257","updated":"2026-04-07 03:40:03.000000000","message":"This is split out at https://review.opendev.org/c/openstack/nova/+/983505","commit_id":"0278a0db25a48bf3701962762e36d35bab14bc34"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7b68dfc6cd454aeead5a5596f15b6067444f7219","unresolved":true,"context_lines":[{"line_number":11879,"context_line":"                # secret there and if the live migration is successful, we"},{"line_number":11880,"context_line":"                # undefine it during post_live_migration. So, if live migration"},{"line_number":11881,"context_line":"                # fails and we won\u0027t reach post_live_migration, we need to"},{"line_number":11882,"context_line":"                # undefine it here."},{"line_number":11883,"context_line":"                self._host.delete_secret(\u0027vtpm\u0027, instance.uuid)"},{"line_number":11884,"context_line":"        finally:"},{"line_number":11885,"context_line":"            # NOTE(gcb): Failed block live migration may leave instance"}],"source_content_type":"text/x-python","patch_set":3,"id":"c577489c_1cd396d6","line":11882,"updated":"2026-04-06 16:07:26.000000000","message":"I guess this is not wrong, but it seems like it probably belongs in the deployment change, because right now this mentions a mode that can\u0027t actually get here, right?","commit_id":"0278a0db25a48bf3701962762e36d35bab14bc34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"50ce480fe9d74456ab230a40215d48c65c20405d","unresolved":true,"context_lines":[{"line_number":11879,"context_line":"                # secret there and if the live migration is successful, we"},{"line_number":11880,"context_line":"                # undefine it during post_live_migration. So, if live migration"},{"line_number":11881,"context_line":"                # fails and we won\u0027t reach post_live_migration, we need to"},{"line_number":11882,"context_line":"                # undefine it here."},{"line_number":11883,"context_line":"                self._host.delete_secret(\u0027vtpm\u0027, instance.uuid)"},{"line_number":11884,"context_line":"        finally:"},{"line_number":11885,"context_line":"            # NOTE(gcb): Failed block live migration may leave instance"}],"source_content_type":"text/x-python","patch_set":3,"id":"6eba6334_551dbd5a","line":11882,"in_reply_to":"90f66f5e_cf4032aa","updated":"2026-04-07 03:40:03.000000000","message":"Updated this to remove mention of \u0027deployment\u0027 because that code isn\u0027t even a thing yet.","commit_id":"0278a0db25a48bf3701962762e36d35bab14bc34"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"425ec3db109f8879d69e9c2eba8ef5c7012df331","unresolved":true,"context_lines":[{"line_number":11879,"context_line":"                # secret there and if the live migration is successful, we"},{"line_number":11880,"context_line":"                # undefine it during post_live_migration. So, if live migration"},{"line_number":11881,"context_line":"                # fails and we won\u0027t reach post_live_migration, we need to"},{"line_number":11882,"context_line":"                # undefine it here."},{"line_number":11883,"context_line":"                self._host.delete_secret(\u0027vtpm\u0027, instance.uuid)"},{"line_number":11884,"context_line":"        finally:"},{"line_number":11885,"context_line":"            # NOTE(gcb): Failed block live migration may leave instance"}],"source_content_type":"text/x-python","patch_set":3,"id":"90f66f5e_cf4032aa","line":11882,"in_reply_to":"c577489c_1cd396d6","updated":"2026-04-06 18:47:18.000000000","message":"Ah yeah I think this does belong here (if we keep the comment, I mean) but I need to make it say \"user and deployment\" wherever it says \u0027deployment\u0027. Because it\u0027s a distinction that applies to both.\n\nI thought a code comment might be warranted because prior to \u0027user\u0027 and \u0027deployment\u0027, you might expect delete of the secret during rollback to only apply to \u0027host\u0027 mode since \u0027user\u0027 and \u0027deployment\u0027 always delete the secret soon after creating it. It\u0027s just there\u0027s a window that a someone reading the code might not realize between pre_live_migration and guest creation where deleting it during rollback would be necessary for \u0027user\u0027 and \u0027deployment\u0027.\n\nMaybe I just need to rephrase the comment better.","commit_id":"0278a0db25a48bf3701962762e36d35bab14bc34"}]}