)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"4d9c82284c0cc4504ad89733cac6dfef7b6470a9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ec4eed5d_8c28bb31","updated":"2022-07-13 23:37:53.000000000","message":"Going to respin to add bug, release note, and address nit.","commit_id":"78bd5b0b1c47d0c4076dc78a78e278d10638f10a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"13a88e44c51ca7ddafee57436cdd6050f84a866f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3f3c3a99_9eba5d7e","updated":"2022-07-12 09:20:19.000000000","message":"I do generally think we probably should have a bug and release note for this so holding +w to see if others agree.\n\nill ping some of the other cores about this later and see if they are ok with this as is or want. a respin but the code looks ok to me.\n\nwe would want a release note if we were to back port this I think and a bug so I would prefer to add them now","commit_id":"78bd5b0b1c47d0c4076dc78a78e278d10638f10a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3e23b16be65d8ac76c9731cf53e8265111d95e19","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"05955873_466121d1","updated":"2022-07-13 00:08:04.000000000","message":"I will hold off on respinning until I get a reply about the inline nit, so I can do them at the same time.","commit_id":"78bd5b0b1c47d0c4076dc78a78e278d10638f10a"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"da13ef48348224fcde4c3f95ea048bfe20a6c0dd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"c97dfe55_0473a8ed","updated":"2022-07-08 10:54:37.000000000","message":"If you want to backport this, you should probably create a bug and link to it in the commit message. Otherwise this looks sane. Nit inside but nothing block-worthy","commit_id":"78bd5b0b1c47d0c4076dc78a78e278d10638f10a"},{"author":{"_account_id":6962,"name":"Kashyap Chamarthy","email":"kchamart@redhat.com","username":"kashyapc"},"change_message_id":"bce557250798b0e43a0ef49801e90cb92a2f7b08","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"fcb3727e_e365b60d","updated":"2022-05-23 15:44:44.000000000","message":"Thanks for your continued work on this!  I completely lost track of it.\n\nThe CI failure itself seems unrelated to me.","commit_id":"78bd5b0b1c47d0c4076dc78a78e278d10638f10a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"2b0b3a21dda5a1168c2d61eac646c30648ef3b70","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e67e6529_66e09ed0","updated":"2022-05-26 16:19:46.000000000","message":"recheck bug 1946339","commit_id":"78bd5b0b1c47d0c4076dc78a78e278d10638f10a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3e23b16be65d8ac76c9731cf53e8265111d95e19","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9be1977c_f1c09c15","in_reply_to":"3f3c3a99_9eba5d7e","updated":"2022-07-13 00:08:04.000000000","message":"Ack, will do.","commit_id":"78bd5b0b1c47d0c4076dc78a78e278d10638f10a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3e23b16be65d8ac76c9731cf53e8265111d95e19","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d322fd17_500006e1","in_reply_to":"c97dfe55_0473a8ed","updated":"2022-07-13 00:08:04.000000000","message":"I was indecisive about it but since you mention it, I will create a bug and release note*.\n\n*given the oddness of this issue","commit_id":"78bd5b0b1c47d0c4076dc78a78e278d10638f10a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"b3849dc91ad3d64b8805448a62a3fdbda75df29f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"690c0760_351c98a7","updated":"2022-08-04 19:56:29.000000000","message":"I looked through the code some more and I think there is a problem with this patch and that we need to go with the other proposal https://review.opendev.org/c/openstack/nova/+/852002.\n\nI\u0027m going to pull this out of the gate.","commit_id":"058c9277ad0dc9b40f69d005592eea03999495e4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"12673d2e3ecbf60a42c188f05c8bd16e01169164","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"e4fb8261_aa9254a4","updated":"2022-08-16 10:16:22.000000000","message":"i see have not had time to reload the context of this.\nim not sure how much time i can spare for reviews this week but is your prefernce to move forward with the other patch.\n\ni dont have a stong preference either way as long as the other patch is updated with a release note.","commit_id":"1734bf7aa77456358e1ad4b8d7e11ce2a5f02743"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"da13ef48348224fcde4c3f95ea048bfe20a6c0dd","unresolved":true,"context_lines":[{"line_number":10329,"context_line":"                        # is fixed."},{"line_number":10330,"context_line":"                        # See the following commit for more details:"},{"line_number":10331,"context_line":"                        # https://github.com/qemu/qemu/commit/552de79bfdd5e9e53847eb3c6d6e4cd898a4370e"},{"line_number":10332,"context_line":"                        ctxt.reraise \u003d False"},{"line_number":10333,"context_line":"        except Exception as ex:"},{"line_number":10334,"context_line":"            LOG.warning(\"Error monitoring migration: %(ex)s\","},{"line_number":10335,"context_line":"                        {\"ex\": ex}, instance\u003dinstance, exc_info\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"e77bdf62_54e85b62","line":10332,"updated":"2022-07-08 10:54:37.000000000","message":"nit: you can do this far simpler in Python 3\n\n  except libvirt.libvirtError as ex:\n       errcode \u003d ex.get_error_code()\n       if errcode \u003d\u003d libvirt.VIR_ERR_INTERNAL_ERROR:\n           errmsg \u003d ex.get_error_message()\n           if (errmsg !\u003d \u0027internal error: migration was active, \u0027\n                         \u0027but no RAM info was set\u0027):\n               raise\n\nI think the only advantage to doing what you\u0027re doing is that if any of the code in the exception handler throws its own exception, we\u0027ll still raise the original exception. However, I\u0027m not certain about this and in this case it wouldn\u0027t matter since none of this should cause an exception.","commit_id":"78bd5b0b1c47d0c4076dc78a78e278d10638f10a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"cb529f9ca8d47140e4950d80a4948e875dc87087","unresolved":false,"context_lines":[{"line_number":10329,"context_line":"                        # is fixed."},{"line_number":10330,"context_line":"                        # See the following commit for more details:"},{"line_number":10331,"context_line":"                        # https://github.com/qemu/qemu/commit/552de79bfdd5e9e53847eb3c6d6e4cd898a4370e"},{"line_number":10332,"context_line":"                        ctxt.reraise \u003d False"},{"line_number":10333,"context_line":"        except Exception as ex:"},{"line_number":10334,"context_line":"            LOG.warning(\"Error monitoring migration: %(ex)s\","},{"line_number":10335,"context_line":"                        {\"ex\": ex}, instance\u003dinstance, exc_info\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"bc8e34ce_e999a9e7","line":10332,"in_reply_to":"291fb694_59fd444b","updated":"2022-07-20 15:39:11.000000000","message":"Done","commit_id":"78bd5b0b1c47d0c4076dc78a78e278d10638f10a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3e23b16be65d8ac76c9731cf53e8265111d95e19","unresolved":true,"context_lines":[{"line_number":10329,"context_line":"                        # is fixed."},{"line_number":10330,"context_line":"                        # See the following commit for more details:"},{"line_number":10331,"context_line":"                        # https://github.com/qemu/qemu/commit/552de79bfdd5e9e53847eb3c6d6e4cd898a4370e"},{"line_number":10332,"context_line":"                        ctxt.reraise \u003d False"},{"line_number":10333,"context_line":"        except Exception as ex:"},{"line_number":10334,"context_line":"            LOG.warning(\"Error monitoring migration: %(ex)s\","},{"line_number":10335,"context_line":"                        {\"ex\": ex}, instance\u003dinstance, exc_info\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"e91e7d62_c3e18469","line":10332,"in_reply_to":"a3dd510d_5a58f468","updated":"2022-07-13 00:08:04.000000000","message":"Maybe I\u0027m missing something but I put the save_and_reraise_exception() so that it will reraise if errcode !\u003d libvirt.VIR_ERR_INTERNAL_ERROR. And I thought that even with Python 3, we still have to use save_and_reraise_exception() in order to not lose the exception if there\u0027s an eventlet thread switch.\n\nOr are you saying there is no eventlet switchable code in the block? (I guess there isn\u0027t.) In that case, you would prefer the following?\n\n  except libvirt.libvirtError as ex:\n       errcode \u003d ex.get_error_code()\n       if errcode \u003d\u003d libvirt.VIR_ERR_INTERNAL_ERROR:\n           errmsg \u003d ex.get_error_message()\n           if (errmsg !\u003d \u0027internal error: migration was active, \u0027\n                         \u0027but no RAM info was set\u0027):\n               raise\n       else:\n           raise","commit_id":"78bd5b0b1c47d0c4076dc78a78e278d10638f10a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"64f25b5aedcc741cf47b38799e13054594f86651","unresolved":true,"context_lines":[{"line_number":10329,"context_line":"                        # is fixed."},{"line_number":10330,"context_line":"                        # See the following commit for more details:"},{"line_number":10331,"context_line":"                        # https://github.com/qemu/qemu/commit/552de79bfdd5e9e53847eb3c6d6e4cd898a4370e"},{"line_number":10332,"context_line":"                        ctxt.reraise \u003d False"},{"line_number":10333,"context_line":"        except Exception as ex:"},{"line_number":10334,"context_line":"            LOG.warning(\"Error monitoring migration: %(ex)s\","},{"line_number":10335,"context_line":"                        {\"ex\": ex}, instance\u003dinstance, exc_info\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"291fb694_59fd444b","line":10332,"in_reply_to":"b2a3d6af_85e26f63","updated":"2022-07-13 23:27:12.000000000","message":"Guh, OK this sounds familiar now and was discussed on a past review and it slipped my mind:\n\nhttps://review.opendev.org/c/openstack/nova/+/840260/2/nova/context.py#427\n\nI will go ahead and rewrite this to not use save_and_reraise_exception() and instead add it only to any backports proposed to branches that support python2.","commit_id":"78bd5b0b1c47d0c4076dc78a78e278d10638f10a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"13a88e44c51ca7ddafee57436cdd6050f84a866f","unresolved":true,"context_lines":[{"line_number":10329,"context_line":"                        # is fixed."},{"line_number":10330,"context_line":"                        # See the following commit for more details:"},{"line_number":10331,"context_line":"                        # https://github.com/qemu/qemu/commit/552de79bfdd5e9e53847eb3c6d6e4cd898a4370e"},{"line_number":10332,"context_line":"                        ctxt.reraise \u003d False"},{"line_number":10333,"context_line":"        except Exception as ex:"},{"line_number":10334,"context_line":"            LOG.warning(\"Error monitoring migration: %(ex)s\","},{"line_number":10335,"context_line":"                        {\"ex\": ex}, instance\u003dinstance, exc_info\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"a3dd510d_5a58f468","line":10332,"in_reply_to":"e77bdf62_54e85b62","updated":"2022-07-12 09:20:19.000000000","message":"hum im not sure either but \n with excutils.save_and_reraise_exception() as ctxt: might be useful if we back port this to python 2 supporting branches so I\u0027m ok with this in this case.\n \n I do generally prefer Stephen\u0027s suggestion.","commit_id":"78bd5b0b1c47d0c4076dc78a78e278d10638f10a"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a70086d2a2d71a8daf6081be71c461a268ce0387","unresolved":true,"context_lines":[{"line_number":10329,"context_line":"                        # is fixed."},{"line_number":10330,"context_line":"                        # See the following commit for more details:"},{"line_number":10331,"context_line":"                        # https://github.com/qemu/qemu/commit/552de79bfdd5e9e53847eb3c6d6e4cd898a4370e"},{"line_number":10332,"context_line":"                        ctxt.reraise \u003d False"},{"line_number":10333,"context_line":"        except Exception as ex:"},{"line_number":10334,"context_line":"            LOG.warning(\"Error monitoring migration: %(ex)s\","},{"line_number":10335,"context_line":"                        {\"ex\": ex}, instance\u003dinstance, exc_info\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"b2a3d6af_85e26f63","line":10332,"in_reply_to":"e91e7d62_c3e18469","updated":"2022-07-13 07:55:05.000000000","message":"in python3 i was under the impression that we nolonger needed to take extra precations for eventlet becasue the way tracebacks are stored in the excetion object we nolonger have to depend on global state so even if there is a userland or kernel thread context switch we are not dependt on thread local or global storage.\n\nwhich is a delta form python 2.7 https://peps.python.org/pep-3134/\n\ni could be worng about this but i know stephen has expressed a prefernce for nto using excutils.save_and_reraise_exception for some time and with the dropping of py 2.7 support a few releases ago i tought we were free to discontinue its use.\n\nmany expction blocks contain a log call which would mean that most would need \nsave_and_reraise_exception if you are correct about needing to protect form context switches.","commit_id":"78bd5b0b1c47d0c4076dc78a78e278d10638f10a"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"35c58da2d6dde2c11bb3af4f3c9084dc12bcb0af","unresolved":true,"context_lines":[{"line_number":10350,"context_line":"                # removed if and when the issue is fixed."},{"line_number":10351,"context_line":"                # See https://bugs.launchpad.net/nova/+bug/1982284 for more"},{"line_number":10352,"context_line":"                # details."},{"line_number":10353,"context_line":"                raise"},{"line_number":10354,"context_line":"        except Exception as ex:"},{"line_number":10355,"context_line":"            LOG.warning(\"Error monitoring migration: %(ex)s\","},{"line_number":10356,"context_line":"                        {\"ex\": ex}, instance\u003dinstance, exc_info\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1e201aac_4ce160ae","line":10353,"updated":"2022-08-04 20:04:22.000000000","message":"Having looked at the other patch [1], I think that ignoring the error in this manner will prevent us from calling post_live_migration [2].\n\nSo this will unfortunately not work.\n\n[1] https://review.opendev.org/c/openstack/nova/+/852002/1/nova/virt/libvirt/guest.py#676\n[2] https://github.com/openstack/nova/blob/adeea3d5e7d7337d2817dd5c46334c76c05995ef/nova/virt/libvirt/driver.py#L10239-L10244","commit_id":"1734bf7aa77456358e1ad4b8d7e11ce2a5f02743"}]}