)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"81f2d78a946e62d149c49679ea0ddbaee5e067ca","unresolved":true,"context_lines":[{"line_number":13,"context_line":"This is very surprising to users who store persistent data in the TPM,"},{"line_number":14,"context_line":"such as keys required to decrypt the main disk of the VM."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Signed-Off-By: jonas.schaefer@cloudandheat.com"},{"line_number":17,"context_line":"Change-Id: Iefb879428681003d6db604b70353a91913c92461"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"78fd3ff1_1d7e418b","line":16,"updated":"2025-10-01 05:24:21.000000000","message":"It seems the reference to the bug was removed.\n\nCloses-Bug: #2118888","commit_id":"d2c8d742569b5f83f71e9a7e9da759831eeddcc3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"72c9d2cc3163212eaea20242a37b72727fa70444","unresolved":false,"context_lines":[{"line_number":13,"context_line":"This is very surprising to users who store persistent data in the TPM,"},{"line_number":14,"context_line":"such as keys required to decrypt the main disk of the VM."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Signed-Off-By: jonas.schaefer@cloudandheat.com"},{"line_number":17,"context_line":"Change-Id: Iefb879428681003d6db604b70353a91913c92461"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"b4781f90_ec0920c8","line":16,"in_reply_to":"78fd3ff1_1d7e418b","updated":"2025-10-22 17:21:04.000000000","message":"Done","commit_id":"d2c8d742569b5f83f71e9a7e9da759831eeddcc3"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"771a4e881a4746a9b8c9b0a0f7b5337f5594c3e7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"f21af246_5bafd2c5","updated":"2025-07-23 14:10:17.000000000","message":"-1 is more because we are not fixign the underlying problem and we are missing the\napproate bug trackers.\n\nthis si more treating the symtboms and fixing one fo the sideffect of the incrorecct logic that was used to fix https://launchpad.net/bugs/1997352 orginally.\n\nso we shoudl really unwinde that patch and adress all 3 bugs properly\nhttps://bugs.launchpad.net/nova/+bug/1633447\nhttps://launchpad.net/bugs/1997352\nand\nhttps://bugs.launchpad.net/nova/+bug/2115870","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"e0fef25b16f1233eeafdec99d946a0a8579109d1","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"fd2f5165_3d250778","updated":"2025-07-23 07:20:23.000000000","message":"Also, we may need a bug reported in https://bugs.launchpad.net/nova . Please add `Closes-Bug: #\u003cnumber\u003e` tag, too.","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"e8dd152031711a7101d399d062f6482ec005f1c6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0dec13bf_fc8b88cd","updated":"2025-07-23 13:45:06.000000000","message":"Needs test coverage for sure.\n\nI\u0027m adding Sean as a reviewer. ISTR this having more impacts to things like migrations and so we suggested that this might be best to have a spec covering those points. Maybe Sean can opine.","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"224ccbe608cef92b7ad14c22bde4ec53d70c768f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7a51e439_c4260a88","updated":"2025-07-23 14:13:11.000000000","message":"i rebased https://review.opendev.org/c/openstack/nova/+/922354 to get new test results just an fyi but it shoudl still funciton correctly and restore the old behavior.","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"c5b7fc41439c9c8ba00917cc1d72cd6bb7c3817c","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"6a448b60_7a59b623","in_reply_to":"0dec13bf_fc8b88cd","updated":"2025-07-23 13:54:18.000000000","message":"(I\u0027m also open for feedback from Sean but) I think this is considered to be a bug fix, because it fixes the defeat caused by recent change in libvirt. We are clear about what we need and the change is quite limited (adding a new flag to undefine call), so I personally think we don\u0027t need a spec but detailed bug report may be enough. Alternatively we can use specless-bp but bug report may be preferred because we may need to backport this to fix this issue with stable releases with recent libvirt (For example this may affect RDO Epoxy + c9s).\n\nRegarding the migration impact, I think we have no impact, as long as we use the flag in undefine call, instead of modifying the xml element. (but as I pointed we have to make sure that the local libvirt supports the flag when adding the flag.)\n\n+1 for test coverage.","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":33742,"name":"Jonas Schäfer","email":"jonas.schaefer@cloudandheat.com","username":"jssfr"},"change_message_id":"7f36b49f7fe3e8603a60b6dd685c7e18c9161cf7","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8738f3c8_b18299c6","in_reply_to":"44ece186_594285da","updated":"2025-07-28 06:51:19.000000000","message":"Thanks for the fruitful discussion :-).\n\nI filed https://bugs.launchpad.net/nova/+bug/2118888 for the record.\n\nFrom the discussion, it\u0027s not fully clear how to proceed from here. To summarize:\n\n1. From what we know, this has always been broken. Likely nobody noticed because loss of vTPM data only really hurts when you tie stuff to it, which only Windows Bitlocker does by default?\n\n2. the KEEP_TPM flag is too new to use it unconditionally, because we need to support older libvirt versions.\n\n3. persistent_state is supported on older versions, but requires us to do manual cleanup of some kind (maybe we could patch the XML right before undefining the VM to remove the flag and then nova does the cleanup?)\n\n4. Ideally, vTPM state would live in the instance directory (which is definitely something for the next release rather than a bugfix, because it also needs a migration of existing vTPM data to that new place)\n\nFrom this I gather that I should probably take a stab at (3) to see if I can get it to fly? Does that make sense?","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d3ecc377f9e4a4077d90f760d67f7af893696148","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"6bf010a5_aa42b175","in_reply_to":"5484a490_4cc8a342","updated":"2025-08-05 12:29:39.000000000","message":"im inclined to agree that focusing on the new api and supporting libvirt 8.9.0 + is a reasonable thing to do and backport.\n\nthat avoid the need to modify the xml and the extra complxities that would cause for live migration ectra.\n\nwe intended the tpm data to be persistent form day 1, we also intened nova to have no bugs so we dont alwasy get what we intended. i think the behvior is surprising enough and impactful to operatror and end users that we should backprot this to stable branches with the explcit libvirt version check.\n\nto do that we need a upstream launchpad bug to detail the issue.\n@Jonas has filed https://bugs.launchpad.net/nova/+bug/2118888 which i think is a good start.\n\nthe bug is actully impacting other lifecycle operation (anything that does undefined)\nwhich we do during volume retype also.\n\nso we may want to address this in multiple patches and expand that bug or we may\nwant to file additional bugs for those other operations.\n\n@melwittt@gmail.com would you mind bring this to a future nova irc meeting so we can build a concencous on the path forward.\nthis is basiclly a +1 for your proposal.","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"574807b4e65a9b8c7bb61bc3ec73fd39bc70e8b9","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"592a8a04_90494411","in_reply_to":"6a448b60_7a59b623","updated":"2025-07-23 13:58:24.000000000","message":"Ack, I\u0027m just referring back to the whole NVRAM persistence thing. If it was a change in behavior in libvirt that we need to counteract, then sounds more straightforward, but as you note, a bug describing why the behavior has changed from what it used to be is needed, at the very least.","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"771a4e881a4746a9b8c9b0a0f7b5337f5594c3e7","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d5d3a43e_0b0a3746","in_reply_to":"6a448b60_7a59b623","updated":"2025-07-23 14:10:17.000000000","message":"i need ot  load context but the nvram and tpm data shoudl not be erased between hard reboots\n\nso just oging off the commit message this is clearly a bug and shoudl be fixed as one but we need a bug tracker and repoducer.\n\ni personally proposed a revert of https://review.opendev.org/c/openstack/nova/+/922354 but did not proceed with it until we had a actual fix.\n\n\nhttps://review.opendev.org/c/openstack/nova/+/906380 very much incorrectly started deleting the nvram info every undefine of the domain. \n\nthat is what added the currrent logic you are modifying.\n\nthe actual fix for https://launchpad.net/bugs/1997352 was to pass the correct flags to preserve the the nvram and vtpm deta when we undefine.\n\ni would presonally prefer sot start with the straight revert followed by actully fixing the orgina bug properly and ensureing we never remove the nvram or vtpm data simply by undefining.\n\nthat should only be done on a actula nova instance delete or as part of cleaning up a souce node as part of a a move operation like live/cold migraiton or shleve offload.\n\nwe also need to cover the evacuate cleanup path but that again is just a move op if a slightly6 non standard one.\n\nhttps://review.opendev.org/c/openstack/nova/+/906380 is also the root cause of \nhttps://bugs.launchpad.net/nova/+bug/2115870\n\nvolume retype  undefiens the domain and nukes the nvram breaking live migration.\n\nso https://review.opendev.org/c/openstack/nova/+/906380 was deeply problematic and that is why i orginally created the revert patch https://review.opendev.org/c/openstack/nova/+/922354 and ask for it to no be backported although i was too late to prevent the backport to 2024.1 https://review.opendev.org/c/openstack/nova/+/915051\n\ni dont think we shoudl have a spec for this because we shoudl fix this in 2024.1+\n\nwe intetionaly do not have this downstream by the way as i dreictly objected to backporting it for the same reaons. it broke more things then it fixed.","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":33742,"name":"Jonas Schäfer","email":"jonas.schaefer@cloudandheat.com","username":"jssfr"},"change_message_id":"bf54a1c53f96b70e422cec355e7faefbcafd5166","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"adcc9c18_87284539","in_reply_to":"6bf010a5_aa42b175","updated":"2025-08-15 11:21:16.000000000","message":"Implemented a version check and passing the flag depending on that.","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"de0452dc77e95d2bcb5bc7596efa2935ec75e7e1","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ffb496d2_c6ccc323","in_reply_to":"7c8c8417_8d1f77b3","updated":"2025-07-25 23:05:13.000000000","message":"Thanks for digging through all those details. It\u0027s surprising how obscure this has been with little to no documentation of pretty major behavior. Also surprising this was broken from day one but at least I did not see any reports of a problem until last week (?).\n\nI think the fix for this would be worth backporting, so I don\u0027t know if that could or should drive labeling it as a bug. The behavior of losing the data across reboots is significant.","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a2e4872e85c8dfec256827fe48c6a4fa542716ae","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"5484a490_4cc8a342","in_reply_to":"8738f3c8_b18299c6","updated":"2025-07-28 20:19:27.000000000","message":"Honestly I\u0027m not sure what a consensus would be among the team so I could see this being a Nova meeting topic or mailing list thread to see what everyone thinks and agree on an approach.\n\nMy inclination would be to add the KEEP_TPM flag now with a guard to only do it if libvirt in the environment is new enough and backport that to versions where it\u0027s possible or reasonably expected to use libvirt \u003e\u003d 8.9.0 [1]. And document it as a known issue for older libvirt versions.\n\nThis would fix it for anyone willing or able to update their libvirt but would obviously leave older libvirt with the problem. Personally I\u0027m not sure how far we want to bend with fixing for older libvirt versions given that this has been a problem since day one and it was due to an internal libvirt behavior we had no control over. We can discuss.\n\n[1] https://libvirt.org/news.html#v8-9-0-2022-11-01","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5bc26e8d5a262f2d71e1da56314c297c57a6a22b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8f04039d_c3fbd2e8","in_reply_to":"8e75e868_0a3b9606","updated":"2025-07-23 21:19:09.000000000","message":"I agree there are two separate issues, one for NVRAM and one for vTPM and that their fixes will be different.\n\nFor vTPM I believe it is not a regression in that originally libvirt defaults to clearing TPM data with no option for preserving it.\n\nThen later, after vTPM functionality was added to Nova, libvirt added a flag to allow preservation of the TPM data. This is a mailing list message from August 2022 where the idea of adding flags for preserving data was mentioned:\n\nhttps://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/PLFSLKCX2FXEYQL6SZ54ELLHH27OORH3/\n\nI think we should start using the VIR_DOMAIN_UNDEFINE_KEEP_TPM flag but I think of it more as an enhancement rather than a bug/defect in Nova (as the original behavior in libvirt was a limitation of libvirt).","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a7ca67e37cbe5035c1320fe24dcb285bed9055a5","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d9b31993_0ccbfbcb","in_reply_to":"8f04039d_c3fbd2e8","updated":"2025-07-25 18:37:40.000000000","message":"your right these are two related but seperate things\n\nwe should not be removing the tpm or nvram info when we undefien in geenral only hwen removing the vm form a host because it is delete or moved.\n\nso me its a bug as we intened to preserve the TPM data across reboots but i think we were unaware fo the orginal libvirt behyavior or it changed.\n\ni.e libvir did nto orginally delete the tpm data on undefine and started too after we added the feature.\n\nyou pointing ot a mailing list patch where VIR_DOMAIN_UNDEFINE_KEEP_TPM was added but i wonder if there is a previous patch where deleting of the file on undefine was added?\n\nwe had a very detialed design discusion about the lifecycle opts in the spec\nhttps://specs.openstack.org/openstack/nova-specs/specs/victoria/implemented/add-emulated-virtual-tpm.html#instance-lifecycle-operations\n\nif the data was not perserved across reboot we woudl have call that out.\n\nso either this was missed (posibel) or it used to be perseved and we were broken by a silent libvirt regresion (this feelles more likelty ot me)\n\nthat would make this a bug not a feature in my view.","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2f5f7da8552fb3423ff716d8c8617955462df56b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"7c8c8417_8d1f77b3","in_reply_to":"c71aa57d_6a750731","updated":"2025-07-25 19:25:16.000000000","message":"i have been reading the c code for the last 40 minutes or so they seam to have alwasy deleteed it but not docuemtned it orginally the documeantion of that poorly added writin only refencing transiten domains after we added supprot for thi  fetaure in nova\n\nthe do have an excption which is if the stroage is on shared storage they dont remove it\n\nhttps://github.com/libvirt/libvirt/commit/3c9968ec9a42ae6131ea147d3e3593df3f56bfd1\n\nim unfortunately forced to conclude that when we added tpm support\n\nhttps://github.com/openstack/nova/commit/e3b0412dda2c070e9d4d3341de22d9845b84b7c6\nther was no way to keep the data and no docs that it would be deleted\nhttps://github.com/libvirt/libvirt/commit/cc6c49f6cd18148da089b867ca390a4c49773350\n\n\nso this was broken form day one...","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"247c68f8683c97ad32fcb1db6714eac02625d9dd","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8e75e868_0a3b9606","in_reply_to":"d5d3a43e_0b0a3746","updated":"2025-07-23 14:38:51.000000000","message":"I\u0027m afraid we are mixing up different problems (one for nvram and the other for vtpm) I think we really need a bug report with details to determine whether we can fix these two in a single change, or we should treat these two separately.\n\nMy understanding is that removal of tpm state file is done by libvirt and we have a separate flag to keep/purge tpm state file, so we may need different fixes (which might be able to be done in a single change, if we come up with a common, consistent timing to purge state file safely).","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"bb77dd09ad26913e66a40e84809857e4562d00cb","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d6140c56_a5f3dfa9","in_reply_to":"d9b31993_0ccbfbcb","updated":"2025-07-25 19:13:30.000000000","message":"I agree it should have been called out in the spec if data would not be preserved across reboots and it\u0027s an interesting point whether libvirt itself may have had a regression. I thought not because if it had been a regression, why fix it by adding flags? And why would the flag not default to \"keep data\"?\n\nI\u0027ll anyway look through and see if I can find any prior info about libvirt TPM data.\n\nI can see the point of view that it does feel like a bug to behave in an unexpected way like this, losing data across a reboot and without being mentioned in the spec. It felt a bit weird for me to think of it as a defect in Nova when there was literally no way Nova could have preserved the TPM data at the time the feature was added (because of libvirt). But any kind of data loss also feels like an automatic defect so either way of labeling it I can see making sense in its own way.","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d77752a96589c40786c368f5eed296d03fab93d6","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c71aa57d_6a750731","in_reply_to":"d9b31993_0ccbfbcb","updated":"2025-07-25 19:19:27.000000000","message":"it looks like the inital supprot for persetient state was added in \ngithub.com/libvirt/libvirt/commit/cc6c49f6cd18148da089b867ca390a4c49773350\nbut that requried seeting  persistent_state\u003d\u0027yes\u0027 which we never did.\n\nthe vtpm feature is fundementaly unfit for it its intneded usecase without percitaince for the lifetime of the vm.\n\nits staggering to me that the libvirt docs dont call out that the tpm data will be lost in the libvirtdoamin xml.\n\nhttps://libvirt.org/formatdomain.html#tpm-device\n\nthe closet they have is \n```\npersistent_state\n\n    The persistent_state attribute indicates whether \u0027swtpm\u0027 TPM state is kept or not when a transient domain is powered off or undefined. This option can be used for preserving TPM state. By default the value is no. This attribute only works with the emulator backend. The accepted values are yes and no. Since 7.0.0\n```\n\nbut wee use persistent domans not transient domains.\n\n\nwe can use VIR_DOMAIN_UNDEFINE_KEEP_TPM  goign forward or we coudl consider also movign the tpm data undr the instance state dir sinc ewe can provide a path for the backing file to use as well. that new in libvirt 10.10.0 however.","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":33742,"name":"Jonas Schäfer","email":"jonas.schaefer@cloudandheat.com","username":"jssfr"},"change_message_id":"bf54a1c53f96b70e422cec355e7faefbcafd5166","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"4e7a5d7e_56b9dd98","in_reply_to":"fd2f5165_3d250778","updated":"2025-08-15 11:21:16.000000000","message":"Done.","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"e44ed83b12753e77c1ba21be37096314453d03d8","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"44ece186_594285da","in_reply_to":"ffb496d2_c6ccc323","updated":"2025-07-26 10:20:40.000000000","message":"just to recored this for properity.\n\nwe have an excpetion to the normal stable policy of backportig only bug for things that cause sever enduser or operational pain point.\n\nas and example we backport code to block numa live migration after it had been allowed for years since it did nto update the pinned cpus of the guest.\nwhy it could break the resouce thrakcer and you could have two vms using the same cores until the moved vm was rebooted.\n\nthat impacted both operator and users causing operational problems and user pain as the perfroamce was inconsitnet.\n\nregardless of if we call this a bug or a feature, lossing your bit locker encyption keys or anything else sotred in the tpm is a massive end user pain point when hard reboot is used to fix a lot of transitant error condtions.\n\ntis especially painful  since start calls hard reboot so if a host goes offline ever there is no way to start the vm to day without loosing the data.\n\nso i would file a bug, we can lable it an RFE bug if we want to refelct realisty but i think your right we shodl do the minimal change to pass VIR_DOMAIN_UNDEFINE_KEEP_TPM when we are not deleting the vm form the host\nand expiclty pass the flag to delete it otherwise.\n\nnote we undefien the domain during volume retype as well and in some other situations, we need ot properly presevice it in all cases we undefine where we are not deleting the vm form the host not just reboot to properly fix the bug.","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":33742,"name":"Jonas Schäfer","email":"jonas.schaefer@cloudandheat.com","username":"jssfr"},"change_message_id":"072cdc6e91cbf60a2996ccde78ccea00b9d573d9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"1362ff80_d661f4fc","updated":"2025-09-24 15:18:20.000000000","message":"Addressed the indentation issues, I have one question about the duplicated definition.","commit_id":"b62d1abbe3540d3eeef0cadaa1d830aee85fe868"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"76f75c6e8686b1e64c5569224bc26dad1e8e2cb2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"d5eefa25_f13860d8","updated":"2025-09-29 15:40:49.000000000","message":"I was going to try playing with this locally, but I don\u0027t think it\u0027s worth it. It defaults to the current behavior, with an override in the cases where we want to keep it.","commit_id":"d2c8d742569b5f83f71e9a7e9da759831eeddcc3"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5d68761471777816feee068e5e75086aa3435b2a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"c9a33aa3_f8f4cbcd","updated":"2026-01-27 23:28:19.000000000","message":"This looks OK to me. I think it would be worthy of a release note but it\u0027s been sitting for a while and has a +2 from Dan, so going ahead and +Wing","commit_id":"2399a296e3ff21e47e9b86b517b6ae9f8f525c01"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"56794a6d6136896dac9618d87f18c3b83cc5fbff","unresolved":true,"context_lines":[{"line_number":1759,"context_line":"        self.assertTrue("},{"line_number":1760,"context_line":"    drvr._may_keep_vtpm,"},{"line_number":1761,"context_line":"    \"LibvirtDriver did not correctly detect libvirt version supporting \""},{"line_number":1762,"context_line":"    \"KEEP_TPM\")"},{"line_number":1763,"context_line":""},{"line_number":1764,"context_line":"    @mock.patch.object(libvirt_driver.LibvirtDriver,"},{"line_number":1765,"context_line":"                       \u0027_register_all_undefined_instance_details\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"d7ead1d2_709475fe","line":1762,"updated":"2025-09-22 15:31:25.000000000","message":"This may have come from the aggressive\u003d3 setting on autopep8, but to me this is really bad and hard to read indenting. Can you move this so it visually nests underneath the `assertTrue()` please?","commit_id":"b62d1abbe3540d3eeef0cadaa1d830aee85fe868"},{"author":{"_account_id":33742,"name":"Jonas Schäfer","email":"jonas.schaefer@cloudandheat.com","username":"jssfr"},"change_message_id":"072cdc6e91cbf60a2996ccde78ccea00b9d573d9","unresolved":false,"context_lines":[{"line_number":1759,"context_line":"        self.assertTrue("},{"line_number":1760,"context_line":"    drvr._may_keep_vtpm,"},{"line_number":1761,"context_line":"    \"LibvirtDriver did not correctly detect libvirt version supporting \""},{"line_number":1762,"context_line":"    \"KEEP_TPM\")"},{"line_number":1763,"context_line":""},{"line_number":1764,"context_line":"    @mock.patch.object(libvirt_driver.LibvirtDriver,"},{"line_number":1765,"context_line":"                       \u0027_register_all_undefined_instance_details\u0027,"}],"source_content_type":"text/x-python","patch_set":6,"id":"ee72273b_a016148f","line":1762,"in_reply_to":"d7ead1d2_709475fe","updated":"2025-09-24 15:18:20.000000000","message":"Fixed (no idea what happened there, actually).","commit_id":"b62d1abbe3540d3eeef0cadaa1d830aee85fe868"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"56794a6d6136896dac9618d87f18c3b83cc5fbff","unresolved":true,"context_lines":[{"line_number":1778,"context_line":"        self.assertFalse("},{"line_number":1779,"context_line":"    drvr._may_keep_vtpm,"},{"line_number":1780,"context_line":"    \"LibvirtDriver did not correctly detect libvirt version which does \""},{"line_number":1781,"context_line":"    \"not support KEEP_TPM\")"},{"line_number":1782,"context_line":""},{"line_number":1783,"context_line":"    def test__check_multipath_misconfiguration(self):"},{"line_number":1784,"context_line":"        self.flags(volume_use_multipath\u003dFalse, volume_enforce_multipath\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":6,"id":"f567458b_a7d836b6","line":1781,"updated":"2025-09-22 15:31:25.000000000","message":"Same here.","commit_id":"b62d1abbe3540d3eeef0cadaa1d830aee85fe868"},{"author":{"_account_id":33742,"name":"Jonas Schäfer","email":"jonas.schaefer@cloudandheat.com","username":"jssfr"},"change_message_id":"072cdc6e91cbf60a2996ccde78ccea00b9d573d9","unresolved":false,"context_lines":[{"line_number":1778,"context_line":"        self.assertFalse("},{"line_number":1779,"context_line":"    drvr._may_keep_vtpm,"},{"line_number":1780,"context_line":"    \"LibvirtDriver did not correctly detect libvirt version which does \""},{"line_number":1781,"context_line":"    \"not support KEEP_TPM\")"},{"line_number":1782,"context_line":""},{"line_number":1783,"context_line":"    def test__check_multipath_misconfiguration(self):"},{"line_number":1784,"context_line":"        self.flags(volume_use_multipath\u003dFalse, volume_enforce_multipath\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a569b41_d505910e","line":1781,"in_reply_to":"f567458b_a7d836b6","updated":"2025-09-24 15:18:20.000000000","message":"Fixed.","commit_id":"b62d1abbe3540d3eeef0cadaa1d830aee85fe868"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"56794a6d6136896dac9618d87f18c3b83cc5fbff","unresolved":true,"context_lines":[{"line_number":19220,"context_line":""},{"line_number":19221,"context_line":"    @mock.patch.object(host.Host, \"get_guest\")"},{"line_number":19222,"context_line":"    def test_undefine_domain_disarms_keep_vtpm_if_not_supported("},{"line_number":19223,"context_line":"        self, mock_get):"},{"line_number":19224,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":19225,"context_line":"        drvr._may_keep_vtpm \u003d False  # normally set by init_host"},{"line_number":19226,"context_line":"        instance \u003d objects.Instance(**self.test_instance)"}],"source_content_type":"text/x-python","patch_set":6,"id":"77790cfc_f25847d9","line":19223,"updated":"2025-09-22 15:31:25.000000000","message":"Also this.. looks like code inside the function, but it\u0027s really the parameter list.","commit_id":"b62d1abbe3540d3eeef0cadaa1d830aee85fe868"},{"author":{"_account_id":33742,"name":"Jonas Schäfer","email":"jonas.schaefer@cloudandheat.com","username":"jssfr"},"change_message_id":"072cdc6e91cbf60a2996ccde78ccea00b9d573d9","unresolved":false,"context_lines":[{"line_number":19220,"context_line":""},{"line_number":19221,"context_line":"    @mock.patch.object(host.Host, \"get_guest\")"},{"line_number":19222,"context_line":"    def test_undefine_domain_disarms_keep_vtpm_if_not_supported("},{"line_number":19223,"context_line":"        self, mock_get):"},{"line_number":19224,"context_line":"        drvr \u003d libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)"},{"line_number":19225,"context_line":"        drvr._may_keep_vtpm \u003d False  # normally set by init_host"},{"line_number":19226,"context_line":"        instance \u003d objects.Instance(**self.test_instance)"}],"source_content_type":"text/x-python","patch_set":6,"id":"d4e73ec8_36dcd8d1","line":19223,"in_reply_to":"77790cfc_f25847d9","updated":"2025-09-24 15:18:20.000000000","message":"I start to wonder whether my tab key was borked while I wrote this...","commit_id":"b62d1abbe3540d3eeef0cadaa1d830aee85fe868"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"72c9d2cc3163212eaea20242a37b72727fa70444","unresolved":true,"context_lines":[{"line_number":1768,"context_line":"    @mock.patch.object(host.Host, \u0027has_min_version\u0027)"},{"line_number":1769,"context_line":"    def test_keep_tpm_unsupported(self, mock_version):"},{"line_number":1770,"context_line":"        def version_check(lv_ver\u003dNone, **kwargs):"},{"line_number":1771,"context_line":"            if lv_ver \u003d\u003d (8, 9, 0):"},{"line_number":1772,"context_line":"                return False"},{"line_number":1773,"context_line":"            return True"},{"line_number":1774,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"2056908b_9e5ac635","line":1771,"range":{"start_line":1771,"start_character":25,"end_line":1771,"end_character":34},"updated":"2025-10-22 17:21:04.000000000","message":"we should proably be using MIN_VERSION_INT_FOR_KEEP_TPM here\n\nbut also the version check is not quite right \nit would be \n if lv_ver \u003c (8, 9, 0):\n\nwe are we doing this insetad of just returning false \n\n@mock.patch.object(host.Host, \u0027has_min_version\u0027, return_value\u003dFalse)","commit_id":"ac0d621d2e0c7d9c4b5269b9b67e8bab38c988c9"},{"author":{"_account_id":38429,"name":"Henry Richter","email":"henry.richter@cloudandheat.com","username":"h3nryc0ding"},"change_message_id":"712987e831f953112b54eb3414b6dbd6695db9ff","unresolved":true,"context_lines":[{"line_number":1768,"context_line":"    @mock.patch.object(host.Host, \u0027has_min_version\u0027)"},{"line_number":1769,"context_line":"    def test_keep_tpm_unsupported(self, mock_version):"},{"line_number":1770,"context_line":"        def version_check(lv_ver\u003dNone, **kwargs):"},{"line_number":1771,"context_line":"            if lv_ver \u003d\u003d (8, 9, 0):"},{"line_number":1772,"context_line":"                return False"},{"line_number":1773,"context_line":"            return True"},{"line_number":1774,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"caefb819_550f7e5b","line":1771,"range":{"start_line":1771,"start_character":25,"end_line":1771,"end_character":34},"in_reply_to":"2056908b_9e5ac635","updated":"2025-11-05 15:30:16.000000000","message":"I switched to using the `MIN_VERSION_INT_FOR_KEEP_TPM` instead.\n\nI also tried using `return_value\u003dFalse`, but `init_host` calls `has_min_version` for other features too, so the test blows up before our check. The `side_effect` is needed to fail only the TPM version check, which is why we\u0027re checking for `\u003d\u003d` instead of replicating the drivers `\u003e\u003d` logic","commit_id":"ac0d621d2e0c7d9c4b5269b9b67e8bab38c988c9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"774e0096003158a949c4edd82a34edf5d01df83f","unresolved":false,"context_lines":[{"line_number":1768,"context_line":"    @mock.patch.object(host.Host, \u0027has_min_version\u0027)"},{"line_number":1769,"context_line":"    def test_keep_tpm_unsupported(self, mock_version):"},{"line_number":1770,"context_line":"        def version_check(lv_ver\u003dNone, **kwargs):"},{"line_number":1771,"context_line":"            if lv_ver \u003d\u003d (8, 9, 0):"},{"line_number":1772,"context_line":"                return False"},{"line_number":1773,"context_line":"            return True"},{"line_number":1774,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"2109280f_fd5ce188","line":1771,"range":{"start_line":1771,"start_character":25,"end_line":1771,"end_character":34},"in_reply_to":"caefb819_550f7e5b","updated":"2025-11-25 14:39:50.000000000","message":"Yeah, have to be targeted to only fail the one we are interested in.","commit_id":"ac0d621d2e0c7d9c4b5269b9b67e8bab38c988c9"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"72c9d2cc3163212eaea20242a37b72727fa70444","unresolved":false,"context_lines":[{"line_number":19236,"context_line":"        )"},{"line_number":19237,"context_line":""},{"line_number":19238,"context_line":"        # Check that it truly forces it to False and doesn\u0027t do a `not` or"},{"line_number":19239,"context_line":"        # something weird :-)."},{"line_number":19240,"context_line":"        fake_guest.reset_mock()"},{"line_number":19241,"context_line":"        drvr._undefine_domain(instance, keep_vtpm\u003dFalse)"},{"line_number":19242,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"0f4e0872_3c167c33","line":19239,"updated":"2025-10-22 17:21:04.000000000","message":"ok :)","commit_id":"ac0d621d2e0c7d9c4b5269b9b67e8bab38c988c9"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"8e67795c2418d8df11dccd51b1572e90cc5c4024","unresolved":true,"context_lines":[{"line_number":1765,"context_line":"        :param cleanup_instance_dir: If the instance dir should be removed"},{"line_number":1766,"context_line":"        :param cleanup_instance_disks: If the instance disks should be removed."},{"line_number":1767,"context_line":"            Also removes ephemeral encryption secrets, if present."},{"line_number":1768,"context_line":"        :param destroy_secrets: If the cinder volume encryption secrets should"},{"line_number":1769,"context_line":"            be deleted."},{"line_number":1770,"context_line":"        \"\"\""},{"line_number":1771,"context_line":"        # zero the data on backend pmem device"},{"line_number":1772,"context_line":"        vpmems \u003d self._get_vpmems(instance)"}],"source_content_type":"text/x-python","patch_set":2,"id":"f3aeb3f7_87b2a945","line":1769,"range":{"start_line":1768,"start_character":32,"end_line":1769,"end_character":23},"updated":"2025-07-23 07:17:47.000000000","message":"Maybe this should be updated, too.","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":33742,"name":"Jonas Schäfer","email":"jonas.schaefer@cloudandheat.com","username":"jssfr"},"change_message_id":"bf54a1c53f96b70e422cec355e7faefbcafd5166","unresolved":true,"context_lines":[{"line_number":1765,"context_line":"        :param cleanup_instance_dir: If the instance dir should be removed"},{"line_number":1766,"context_line":"        :param cleanup_instance_disks: If the instance disks should be removed."},{"line_number":1767,"context_line":"            Also removes ephemeral encryption secrets, if present."},{"line_number":1768,"context_line":"        :param destroy_secrets: If the cinder volume encryption secrets should"},{"line_number":1769,"context_line":"            be deleted."},{"line_number":1770,"context_line":"        \"\"\""},{"line_number":1771,"context_line":"        # zero the data on backend pmem device"},{"line_number":1772,"context_line":"        vpmems \u003d self._get_vpmems(instance)"}],"source_content_type":"text/x-python","patch_set":2,"id":"e87dca4c_c9cc7a82","line":1769,"range":{"start_line":1768,"start_character":32,"end_line":1769,"end_character":23},"in_reply_to":"f3aeb3f7_87b2a945","updated":"2025-08-15 11:21:16.000000000","message":"Fixed (with the new argument which is used).","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"eed383285e331749e6c61c6011b1210f45fdc75d","unresolved":true,"context_lines":[{"line_number":1841,"context_line":"                self._cleanup_ephemeral_encryption_secrets("},{"line_number":1842,"context_line":"                    context, instance, block_device_info)"},{"line_number":1843,"context_line":""},{"line_number":1844,"context_line":"        self._undefine_domain(instance, destroy_secrets\u003ddestroy_secrets)"},{"line_number":1845,"context_line":""},{"line_number":1846,"context_line":"    def _cleanup_ephemeral_encryption_secrets("},{"line_number":1847,"context_line":"        self, context, instance, block_device_info"}],"source_content_type":"text/x-python","patch_set":2,"id":"e8c7bef0_43cb0527","line":1844,"range":{"start_line":1844,"start_character":56,"end_line":1844,"end_character":71},"updated":"2025-08-05 16:27:35.000000000","message":"Since this is really for cinder volumes, I wonder if this us really the right flag to use. Like, there are times where we\u0027re deleting an instance where we want the volumes to remain.\n\nWould it not be better to tie this to `cleanup_instance_disks` like L1833?","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"56794a6d6136896dac9618d87f18c3b83cc5fbff","unresolved":false,"context_lines":[{"line_number":1841,"context_line":"                self._cleanup_ephemeral_encryption_secrets("},{"line_number":1842,"context_line":"                    context, instance, block_device_info)"},{"line_number":1843,"context_line":""},{"line_number":1844,"context_line":"        self._undefine_domain(instance, destroy_secrets\u003ddestroy_secrets)"},{"line_number":1845,"context_line":""},{"line_number":1846,"context_line":"    def _cleanup_ephemeral_encryption_secrets("},{"line_number":1847,"context_line":"        self, context, instance, block_device_info"}],"source_content_type":"text/x-python","patch_set":2,"id":"24f05902_c7258490","line":1844,"range":{"start_line":1844,"start_character":56,"end_line":1844,"end_character":71},"in_reply_to":"bfe6d3da_8e437281","updated":"2025-09-22 15:31:25.000000000","message":"The local customs are that nobody does and they clutter the view until people get annoyed. But yeah, if you\u0027re addressing something no longer needing discussion, please resolve :)","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":33742,"name":"Jonas Schäfer","email":"jonas.schaefer@cloudandheat.com","username":"jssfr"},"change_message_id":"bf54a1c53f96b70e422cec355e7faefbcafd5166","unresolved":true,"context_lines":[{"line_number":1841,"context_line":"                self._cleanup_ephemeral_encryption_secrets("},{"line_number":1842,"context_line":"                    context, instance, block_device_info)"},{"line_number":1843,"context_line":""},{"line_number":1844,"context_line":"        self._undefine_domain(instance, destroy_secrets\u003ddestroy_secrets)"},{"line_number":1845,"context_line":""},{"line_number":1846,"context_line":"    def _cleanup_ephemeral_encryption_secrets("},{"line_number":1847,"context_line":"        self, context, instance, block_device_info"}],"source_content_type":"text/x-python","patch_set":2,"id":"bfe6d3da_8e437281","line":1844,"range":{"start_line":1844,"start_character":56,"end_line":1844,"end_character":71},"in_reply_to":"e8c7bef0_43cb0527","updated":"2025-08-15 11:21:16.000000000","message":"Fixed in the new patch revision. I\u0027m not familiar with the customs here, do I resolve this thread or do you resolve it?","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"03e42fec91cf6e7e2cf71395d305e6aa6716f112","unresolved":true,"context_lines":[{"line_number":225,"context_line":"# vIOMMU model value `virtio` minimal support version"},{"line_number":226,"context_line":"MIN_LIBVIRT_VIOMMU_VIRTIO_MODEL \u003d (8, 3, 0)"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"MIN_LIBVIRT_KEEP_TPM \u003d (8, 9, 0)"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"# Virtuozzo driver support"},{"line_number":231,"context_line":"MIN_VIRTUOZZO_VERSION \u003d (7, 0, 0)"}],"source_content_type":"text/x-python","patch_set":6,"id":"c87df1a8_a3f78102","line":228,"updated":"2025-09-24 16:53:50.000000000","message":"Right here.","commit_id":"b62d1abbe3540d3eeef0cadaa1d830aee85fe868"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"76f75c6e8686b1e64c5569224bc26dad1e8e2cb2","unresolved":false,"context_lines":[{"line_number":225,"context_line":"# vIOMMU model value `virtio` minimal support version"},{"line_number":226,"context_line":"MIN_LIBVIRT_VIOMMU_VIRTIO_MODEL \u003d (8, 3, 0)"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"MIN_LIBVIRT_KEEP_TPM \u003d (8, 9, 0)"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"# Virtuozzo driver support"},{"line_number":231,"context_line":"MIN_VIRTUOZZO_VERSION \u003d (7, 0, 0)"}],"source_content_type":"text/x-python","patch_set":6,"id":"b7b07ebd_72766c5a","line":228,"in_reply_to":"c87df1a8_a3f78102","updated":"2025-09-29 15:40:49.000000000","message":"Done","commit_id":"b62d1abbe3540d3eeef0cadaa1d830aee85fe868"}],"nova/virt/libvirt/guest.py":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"8e67795c2418d8df11dccd51b1572e90cc5c4024","unresolved":true,"context_lines":[{"line_number":301,"context_line":"            flags \u003d libvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE"},{"line_number":302,"context_line":"            flags |\u003d libvirt.VIR_DOMAIN_UNDEFINE_NVRAM"},{"line_number":303,"context_line":"            if not destroy_secrets:"},{"line_number":304,"context_line":"                flags |\u003d libvirt.VIR_DOMAIN_UNDEFINE_KEEP_TPM"},{"line_number":305,"context_line":"            self._domain.undefineFlags(flags)"},{"line_number":306,"context_line":"        except libvirt.libvirtError:"},{"line_number":307,"context_line":"            LOG.debug(\"Error from libvirt during undefineFlags for guest \""}],"source_content_type":"text/x-python","patch_set":2,"id":"48ed270c_36808d01","line":304,"range":{"start_line":304,"start_character":33,"end_line":304,"end_character":61},"updated":"2025-07-23 07:17:47.000000000","message":"This flag was added in libvirt 8.9.0, but the current supported minimum version of libvirt is still 8.0.0 . Should we pass this flag only when libvirt is actually \u003e\u003d 8.9.0 ?\n(Although older libvirt daemon might ignore this flag, we should consider the case where libvirt-python \u003c 8.9.0 is used at least)\n\nAlternatively we can inject persistent_state to domain xml which is available since 7.0.0 .","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"718de788f45382cce51e16d5023ba413270a6d7d","unresolved":true,"context_lines":[{"line_number":301,"context_line":"            flags \u003d libvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE"},{"line_number":302,"context_line":"            flags |\u003d libvirt.VIR_DOMAIN_UNDEFINE_NVRAM"},{"line_number":303,"context_line":"            if not destroy_secrets:"},{"line_number":304,"context_line":"                flags |\u003d libvirt.VIR_DOMAIN_UNDEFINE_KEEP_TPM"},{"line_number":305,"context_line":"            self._domain.undefineFlags(flags)"},{"line_number":306,"context_line":"        except libvirt.libvirtError:"},{"line_number":307,"context_line":"            LOG.debug(\"Error from libvirt during undefineFlags for guest \""}],"source_content_type":"text/x-python","patch_set":2,"id":"9c8e7709_d0267d7d","line":304,"range":{"start_line":304,"start_character":33,"end_line":304,"end_character":61},"in_reply_to":"27adb402_d65ec042","updated":"2025-07-23 09:25:18.000000000","message":"Although the document does not explain this clearly, the option affects persistent domain, too. See https://gitlab.com/libvirt/libvirt/-/blob/master/src/qemu/qemu_tpm.c#L991-993 .\n\nHowever as you said with persistent_state flag set we have to remove the state file manually or pass VIR_DOMAIN_UNDEFINE_TPM to undefine call so it does not ease the logic.\n\nI think the easier way is to use the VIR_DOMAIN_UNDEFINE_KEEP_TPM, but add the logic to omit the flag if libvirt does not support it actually.","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":33742,"name":"Jonas Schäfer","email":"jonas.schaefer@cloudandheat.com","username":"jssfr"},"change_message_id":"bf54a1c53f96b70e422cec355e7faefbcafd5166","unresolved":true,"context_lines":[{"line_number":301,"context_line":"            flags \u003d libvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE"},{"line_number":302,"context_line":"            flags |\u003d libvirt.VIR_DOMAIN_UNDEFINE_NVRAM"},{"line_number":303,"context_line":"            if not destroy_secrets:"},{"line_number":304,"context_line":"                flags |\u003d libvirt.VIR_DOMAIN_UNDEFINE_KEEP_TPM"},{"line_number":305,"context_line":"            self._domain.undefineFlags(flags)"},{"line_number":306,"context_line":"        except libvirt.libvirtError:"},{"line_number":307,"context_line":"            LOG.debug(\"Error from libvirt during undefineFlags for guest \""}],"source_content_type":"text/x-python","patch_set":2,"id":"43cf9c7c_5b102485","line":304,"range":{"start_line":304,"start_character":33,"end_line":304,"end_character":61},"in_reply_to":"3049111a_a2132310","updated":"2025-08-15 11:21:16.000000000","message":"Now gated behind the version check.","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"76f75c6e8686b1e64c5569224bc26dad1e8e2cb2","unresolved":false,"context_lines":[{"line_number":301,"context_line":"            flags \u003d libvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE"},{"line_number":302,"context_line":"            flags |\u003d libvirt.VIR_DOMAIN_UNDEFINE_NVRAM"},{"line_number":303,"context_line":"            if not destroy_secrets:"},{"line_number":304,"context_line":"                flags |\u003d libvirt.VIR_DOMAIN_UNDEFINE_KEEP_TPM"},{"line_number":305,"context_line":"            self._domain.undefineFlags(flags)"},{"line_number":306,"context_line":"        except libvirt.libvirtError:"},{"line_number":307,"context_line":"            LOG.debug(\"Error from libvirt during undefineFlags for guest \""}],"source_content_type":"text/x-python","patch_set":2,"id":"6713d680_8a1c4d03","line":304,"range":{"start_line":304,"start_character":33,"end_line":304,"end_character":61},"in_reply_to":"43cf9c7c_5b102485","updated":"2025-09-29 15:40:49.000000000","message":"Done","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":33742,"name":"Jonas Schäfer","email":"jonas.schaefer@cloudandheat.com","username":"jssfr"},"change_message_id":"6c2b6333f651400058e5840eecb8e008ec00cf55","unresolved":true,"context_lines":[{"line_number":301,"context_line":"            flags \u003d libvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE"},{"line_number":302,"context_line":"            flags |\u003d libvirt.VIR_DOMAIN_UNDEFINE_NVRAM"},{"line_number":303,"context_line":"            if not destroy_secrets:"},{"line_number":304,"context_line":"                flags |\u003d libvirt.VIR_DOMAIN_UNDEFINE_KEEP_TPM"},{"line_number":305,"context_line":"            self._domain.undefineFlags(flags)"},{"line_number":306,"context_line":"        except libvirt.libvirtError:"},{"line_number":307,"context_line":"            LOG.debug(\"Error from libvirt during undefineFlags for guest \""}],"source_content_type":"text/x-python","patch_set":2,"id":"27adb402_d65ec042","line":304,"range":{"start_line":304,"start_character":33,"end_line":304,"end_character":61},"in_reply_to":"48ed270c_36808d01","updated":"2025-07-23 07:28:08.000000000","message":"Thanks for the quick feedback!\n\nWe considered persistent_state, but it was not fully clear if that helps. The documentation states this affects \"transient\" domains, which I\u0027m not sure this is. Even if it affected the domain, it would prevent removal when we actually want to get rid of the data.\n\nWe\u0027d have to remove the persistent_state flag with an XML update instead, correct?","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"70ad8dcc6fdc0a10cca157719f1d08a1edeae3b9","unresolved":true,"context_lines":[{"line_number":301,"context_line":"            flags \u003d libvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE"},{"line_number":302,"context_line":"            flags |\u003d libvirt.VIR_DOMAIN_UNDEFINE_NVRAM"},{"line_number":303,"context_line":"            if not destroy_secrets:"},{"line_number":304,"context_line":"                flags |\u003d libvirt.VIR_DOMAIN_UNDEFINE_KEEP_TPM"},{"line_number":305,"context_line":"            self._domain.undefineFlags(flags)"},{"line_number":306,"context_line":"        except libvirt.libvirtError:"},{"line_number":307,"context_line":"            LOG.debug(\"Error from libvirt during undefineFlags for guest \""}],"source_content_type":"text/x-python","patch_set":2,"id":"3049111a_a2132310","line":304,"range":{"start_line":304,"start_character":33,"end_line":304,"end_character":61},"in_reply_to":"9c8e7709_d0267d7d","updated":"2025-07-26 10:32:22.000000000","message":"persistent_state is an option but if we use that nova need to clean it up when deleteing the vm from a host, that means we need to updeate teh cold migrate code path and shleve/evacuate to properly handel that.\n\nit is available for older version and it would be requried in older branches i think.\n\nperhaps we need to do both in 3 patches. add supprot for generating the persitent_state atibute and manually deleting the tpm data, then add support for usign the VIR_DOMAIN_UNDEFINE_KEEP_TPM in a master only fix, finally consider movign the tpm data to the instnace state dir just to keep it located with the rest of the instnace state in a future release.\n\n\nwe sould not need to remove the persistent_state flag to have the data delete we woudl need to manually clean it up or pass VIR_DOMAIN_UNDEFINE_TPM\n\ni think the overall impedence missmatch here is, as a user of libvirt i and i suppoect most others expect the the tpm data and nvram files to have the same lifecycle as the vm disks or console file which are independent of the lifecycle of the domain definition. that is not the case but that is not docuemnted properly be libvirt.\n\nover time i think we should move to that model and ensure we move the tpm and any other vm state into the nova instance dir and manage it form nova\u0027s code consitnetly rather then having some managed by libvirt and some manage by nova.\n\nthere are several places in nova where we undefine the deomain or convert it to a transitnt domain today in snapshot or volume resize workflows that would also need to be udpated to not loos the tpm or nvram data.","commit_id":"9dbd6c939ebc5a6f57eb583cf2d51c3043c93fc0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2da273bf86728e01fee1c26054a4e8bac0fb6421","unresolved":true,"context_lines":[{"line_number":299,"context_line":"            yield VCPUInfo("},{"line_number":300,"context_line":"                id\u003dvcpu[0], cpu\u003dvcpu[3], state\u003dvcpu[1], time\u003dvcpu[2])"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def delete_configuration(self, keep_vtpm\u003dFalse):"},{"line_number":303,"context_line":"        \"\"\"Undefines a domain from hypervisor."},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"        :param keep_vtpm: If true, the vTPM data will be preserved. Otherwise,"}],"source_content_type":"text/x-python","patch_set":4,"id":"edf8fcdd_e414d58b","line":302,"updated":"2025-08-15 10:54:03.000000000","message":"i mentioned this on irc but i would be tempeted to take flags as the keyword arg\n\n```suggestion\n    DEFALUT_UNDEFINE_FLAGS \u003d libvirt.VIR_DOMAIN_UNDEFINE_MANAGED_SAVE\n    DEFALUT_UNDEFINE_FLAGS |\u003d libvirt.VIR_DOMAIN_UNDEFINE_NVRAM\n    def delete_configuration(self, flags\u003dDEFALUT_UNDEFINE_FLAGS):\n```\n\nlater when we are fixign NVRAM we can remove it form the default and just pass it when needed but this will keep the admitely broken behavior.\n\nwhen you invoke this fucntion for a vm that has vgpu you can just do \n\nguest.delete_configuration(flags\u003dguest.DEFALUT_UNDEFINE_FLAGS|IR_DOMAIN_UNDEFINE_KEEP_TPM)\n\nassuming it new enough or leave it at the default otherwise.","commit_id":"9adcf451ab11df1020e5bb11f8fadfb9829a636e"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"72c9d2cc3163212eaea20242a37b72727fa70444","unresolved":false,"context_lines":[{"line_number":299,"context_line":"            yield VCPUInfo("},{"line_number":300,"context_line":"                id\u003dvcpu[0], cpu\u003dvcpu[3], state\u003dvcpu[1], time\u003dvcpu[2])"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def delete_configuration(self, keep_vtpm\u003dFalse):"},{"line_number":303,"context_line":"        \"\"\"Undefines a domain from hypervisor."},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"        :param keep_vtpm: If true, the vTPM data will be preserved. Otherwise,"}],"source_content_type":"text/x-python","patch_set":4,"id":"eaab8bb2_9d0d1c4a","line":302,"in_reply_to":"3e6bfa9b_4f68e20f","updated":"2025-10-22 17:21:04.000000000","message":"Acknowledged","commit_id":"9adcf451ab11df1020e5bb11f8fadfb9829a636e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"56794a6d6136896dac9618d87f18c3b83cc5fbff","unresolved":true,"context_lines":[{"line_number":299,"context_line":"            yield VCPUInfo("},{"line_number":300,"context_line":"                id\u003dvcpu[0], cpu\u003dvcpu[3], state\u003dvcpu[1], time\u003dvcpu[2])"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def delete_configuration(self, keep_vtpm\u003dFalse):"},{"line_number":303,"context_line":"        \"\"\"Undefines a domain from hypervisor."},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"        :param keep_vtpm: If true, the vTPM data will be preserved. Otherwise,"}],"source_content_type":"text/x-python","patch_set":4,"id":"3e6bfa9b_4f68e20f","line":302,"in_reply_to":"cad48c9d_2e1bfa17","updated":"2025-09-22 15:31:25.000000000","message":"I think I prefer the boolean behavior flag as you have it.","commit_id":"9adcf451ab11df1020e5bb11f8fadfb9829a636e"},{"author":{"_account_id":33742,"name":"Jonas Schäfer","email":"jonas.schaefer@cloudandheat.com","username":"jssfr"},"change_message_id":"c5798ce4f2406ce9274cf6f5b4ab85325f0c36f4","unresolved":true,"context_lines":[{"line_number":299,"context_line":"            yield VCPUInfo("},{"line_number":300,"context_line":"                id\u003dvcpu[0], cpu\u003dvcpu[3], state\u003dvcpu[1], time\u003dvcpu[2])"},{"line_number":301,"context_line":""},{"line_number":302,"context_line":"    def delete_configuration(self, keep_vtpm\u003dFalse):"},{"line_number":303,"context_line":"        \"\"\"Undefines a domain from hypervisor."},{"line_number":304,"context_line":""},{"line_number":305,"context_line":"        :param keep_vtpm: If true, the vTPM data will be preserved. Otherwise,"}],"source_content_type":"text/x-python","patch_set":4,"id":"cad48c9d_2e1bfa17","line":302,"in_reply_to":"edf8fcdd_e414d58b","updated":"2025-08-15 11:22:20.000000000","message":"Do you want me to add that to this patch?\n\nI\u0027m not sure how \"tempted\" you are as a reviewer :-).","commit_id":"9adcf451ab11df1020e5bb11f8fadfb9829a636e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"56794a6d6136896dac9618d87f18c3b83cc5fbff","unresolved":true,"context_lines":[{"line_number":104,"context_line":"}"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"MIN_VERSION_INT_FOR_KEEP_TPM \u003d versionutils.convert_version_to_int((8, 9, 0))"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"class Guest(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"96e4709c_81e1eb4d","line":107,"updated":"2025-09-22 15:31:25.000000000","message":"Why duplicate this when it\u0027s defined in the previous file?","commit_id":"b62d1abbe3540d3eeef0cadaa1d830aee85fe868"},{"author":{"_account_id":33742,"name":"Jonas Schäfer","email":"jonas.schaefer@cloudandheat.com","username":"jssfr"},"change_message_id":"072cdc6e91cbf60a2996ccde78ccea00b9d573d9","unresolved":true,"context_lines":[{"line_number":104,"context_line":"}"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"MIN_VERSION_INT_FOR_KEEP_TPM \u003d versionutils.convert_version_to_int((8, 9, 0))"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"class Guest(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"dea11e85_56d955e8","line":107,"in_reply_to":"96e4709c_81e1eb4d","updated":"2025-09-24 15:18:20.000000000","message":"Uh, I\u0027m not sure where you\u0027re seeing the duplicated definition? At least my local `ag` only finds the one in guest.py and searching for `/versionutils\\.convert_version_to_int.+8,\\s*9,\\s*0/` only finds that one instance, too.\n\nCan you point me at the other location?","commit_id":"b62d1abbe3540d3eeef0cadaa1d830aee85fe868"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"76f75c6e8686b1e64c5569224bc26dad1e8e2cb2","unresolved":false,"context_lines":[{"line_number":104,"context_line":"}"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"MIN_VERSION_INT_FOR_KEEP_TPM \u003d versionutils.convert_version_to_int((8, 9, 0))"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"class Guest(object):"}],"source_content_type":"text/x-python","patch_set":6,"id":"ab47cd23_4d0cd3be","line":107,"in_reply_to":"dea11e85_56d955e8","updated":"2025-09-29 15:40:49.000000000","message":"Done","commit_id":"b62d1abbe3540d3eeef0cadaa1d830aee85fe868"}]}