)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"6b57d565cd1f1ff995749db196c947cf213d9011","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"dfaa4782_ff0cafa5","updated":"2024-04-07 18:22:52.000000000","message":"\u003e the orginal logic was incorrect but so is the new logic.\n\n@sean, thanks for looking deeper into the issues with NVRAM handling and not just blindly reverting this particular change!","commit_id":"406d590a364d2c3ebc91e5f28f94011b158459d2"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"a7ec81323cbb4d17c9bb00b3d095f57b7ecad93d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6be3faea_6def30e8","updated":"2024-02-27 07:26:11.000000000","message":"Dan, Stephen, could you kindly take a look at this bugfix / patch?\n\nThis is pretty much a \"virsh undefine --nvam\" instead of \"virsh undefine\" to always achieve the goal of undefining a domain, no matter if it has NVRAM or not.\n\nIf I may quote the docs [1], \n\n\u003e If the domain has an nvram file and the flags are omitted, the undefine will fail.\n\n\n[1] https://www.libvirt.org/manpages/virsh.html#undefine","commit_id":"406d590a364d2c3ebc91e5f28f94011b158459d2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"a9863788a2bae7ae1f2886a727075df7d30db526","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"06de02f8_b5c050cd","updated":"2024-04-04 20:01:44.000000000","message":"Logs look free of errors on both q35 and pc jobs and the libvirt docs don\u0027t say anything to indicate that this isn\u0027t okay to always do. Since we\u0027re always doing this when booted with UEFI, it seems like this is no worse than current.\n\nHowever, I do wonder - it seems like we\u0027re maybe overzealously removing nvram currently. Like during swap volume, why are we deleting the nvram? Doesn\u0027t that potentially clear things like UEFI boot parameters and potentially other things (not TPM I guess, but similar)?\n\nEither way, as I mention, this doesn\u0027t increase the number of places we do this currently, it just makes us always do it regardless of the machine type and thus doesn\u0027t really impact the former question.","commit_id":"406d590a364d2c3ebc91e5f28f94011b158459d2"},{"author":{"_account_id":34150,"name":"Simon Hensel","email":"simon.hensel@inovex.de","username":"shensel"},"change_message_id":"b4e6f776e37fc78845646233fb2f20cf461315e0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"81172da8_0c73f33d","updated":"2024-03-13 09:54:48.000000000","message":"Thanks for the review and the +2 Stephen!\nIs there any chance this change can be merged?","commit_id":"406d590a364d2c3ebc91e5f28f94011b158459d2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"49cddab055c00ecb5f856b7b424cbd4efd59aaf9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"78f1d281_bdc50813","updated":"2024-04-03 14:18:11.000000000","message":"recheck build rebuilds expired and I want to see the logs","commit_id":"406d590a364d2c3ebc91e5f28f94011b158459d2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c81a2c3612c71c49f13e45c51fad33fee647a168","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"28ada551_844fe5fa","updated":"2024-02-13 09:09:33.000000000","message":"the rebuild should have deleted the nvram file so this sound like the wrong way to fix this.\n\nthis is not a full review i just quickly read the commit message","commit_id":"406d590a364d2c3ebc91e5f28f94011b158459d2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8fa88be1069493033643b06454dc40a997068868","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5a94faa4_5c1afd01","in_reply_to":"06de02f8_b5c050cd","updated":"2024-04-07 03:36:21.000000000","message":"we have a buch of bugs with the nvram file and are incorredctly deleteding it in a number of cases.\nthe nvram file can have user data in it and im not sure if its used by windows to store some bootloader setting so deleting this could be bad.","commit_id":"406d590a364d2c3ebc91e5f28f94011b158459d2"},{"author":{"_account_id":32755,"name":"Christian Rohmann","email":"christian.rohmann@inovex.de","username":"frittentheke"},"change_message_id":"b1fb10d14869ded7150d568f5be733d946a5fde9","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"38889bfa_ba108e6f","in_reply_to":"28ada551_844fe5fa","updated":"2024-02-13 12:46:55.000000000","message":"If you kindly read the bug report again [1], there are steps to reproduce this.\nThe reason behind the problem is actually that undefining domain with an NVRAM file cannot be undefined, thus resulting in:\n\n\"libvirt.libvirtError: Requested operation is not valid: cannot undefine domain with nvram\"\n\nSee [2] for the the option to also delete NVRAM via \"VIR_DOMAIN_UNDEFINE_NVRAM\", which is what [3] now does unconditionally. Before this patch the check for UEFI support looked at the current image. With a rebuild this \"current\" image is already of the non-uefi kind.\n\nIn any case, the expected result when calling \"delete_configuration\" on a guest is that ANY configuration is gone, no matter what has to be done to achieve this. Always including the deletion of NVRAM is actually not a fix for just this issue, but more of a cleanup to remove unnecessary conditionals. \n\n\n\n[1] https://bugs.launchpad.net/nova/+bug/1997352\n[2] https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainUndefineFlagsValues\n[3] https://review.opendev.org/c/openstack/nova/+/906380/1/nova/virt/libvirt/guest.py#302","commit_id":"406d590a364d2c3ebc91e5f28f94011b158459d2"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"8fa88be1069493033643b06454dc40a997068868","unresolved":true,"context_lines":[{"line_number":1565,"context_line":"        try:"},{"line_number":1566,"context_line":"            guest \u003d self._host.get_guest(instance)"},{"line_number":1567,"context_line":"            try:"},{"line_number":1568,"context_line":"                guest.delete_configuration()"},{"line_number":1569,"context_line":"            except libvirt.libvirtError as e:"},{"line_number":1570,"context_line":"                with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":1571,"context_line":"                    errcode \u003d e.get_error_code()"}],"source_content_type":"text/x-python","patch_set":1,"id":"1de042c1_13c43052","line":1568,"updated":"2024-04-07 03:36:21.000000000","message":"im not conviced this is correct.\nwe should only be delting the nvram file if we are delteing the vm or moving it to another host.\n\nhave we checked that with this change it does not delete the nvram file if we stop a vm?\n\nim still on pto and recovering from covid so i have not looked at this properly but we might need to revert this as i was not conviced when i previoulsy looked at this quikcly that the patch was correct.","commit_id":"406d590a364d2c3ebc91e5f28f94011b158459d2"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"08195a38b2d10b5239c24ca95d672492035166ed","unresolved":true,"context_lines":[{"line_number":1565,"context_line":"        try:"},{"line_number":1566,"context_line":"            guest \u003d self._host.get_guest(instance)"},{"line_number":1567,"context_line":"            try:"},{"line_number":1568,"context_line":"                guest.delete_configuration()"},{"line_number":1569,"context_line":"            except libvirt.libvirtError as e:"},{"line_number":1570,"context_line":"                with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":1571,"context_line":"                    errcode \u003d e.get_error_code()"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f75bc2f_019bf468","line":1568,"in_reply_to":"1de042c1_13c43052","updated":"2024-04-07 04:18:38.000000000","message":"https://bugs.launchpad.net/nova/+bug/1997352/comments/13\n\nthe orginal logic was incorrect but so is the new logic.\n\nwe should not be unconditionally deleting the nvram file like his lets disucss this at the ptg.","commit_id":"406d590a364d2c3ebc91e5f28f94011b158459d2"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"80f169f7fa67c9f1cd8efdb5ea46a62b7939fa72","unresolved":true,"context_lines":[{"line_number":1565,"context_line":"        try:"},{"line_number":1566,"context_line":"            guest \u003d self._host.get_guest(instance)"},{"line_number":1567,"context_line":"            try:"},{"line_number":1568,"context_line":"                guest.delete_configuration()"},{"line_number":1569,"context_line":"            except libvirt.libvirtError as e:"},{"line_number":1570,"context_line":"                with excutils.save_and_reraise_exception() as ctxt:"},{"line_number":1571,"context_line":"                    errcode \u003d e.get_error_code()"}],"source_content_type":"text/x-python","patch_set":1,"id":"cf35d2bd_a35e2cec","line":1568,"in_reply_to":"7f75bc2f_019bf468","updated":"2024-04-08 13:56:02.000000000","message":"You\u0027re confirming my suspicion that we\u0027re deleting it too often already right? I figured no harm in proceeding with this as it doesn\u0027t make the situation any worse.","commit_id":"406d590a364d2c3ebc91e5f28f94011b158459d2"}]}