)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"551a4ecb0a2d920e7922047e12c98b9d951c0ce3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"211a5e2a_d2de1cb4","updated":"2022-12-13 10:27:04.000000000","message":"Hey, thank you for the change, a couple of comments","commit_id":"d01e0b5b93f04e56aa017d4c17a14f270a17b839"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"8e35e31b848b7ba4a0ab8fdf4a13859606c2c07b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b600364e_c5623a40","in_reply_to":"211a5e2a_d2de1cb4","updated":"2022-12-13 11:45:07.000000000","message":"Thanks for your review Sahid - I will address the comments with a follow-up commit once I\u0027ve had a chance to address them in my local dev environment","commit_id":"d01e0b5b93f04e56aa017d4c17a14f270a17b839"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"70e063224bf67377374bc26bcd5e76a911f3e9ce","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"b8990232_48cd4a72","in_reply_to":"b600364e_c5623a40","updated":"2022-12-22 12:19:34.000000000","message":"Done","commit_id":"d01e0b5b93f04e56aa017d4c17a14f270a17b839"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"5593319a8b99e7f49a549c4bd056a9cfae9f6979","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"eaf9886f_1fa3105f","updated":"2022-12-22 12:20:36.000000000","message":"FAO John Garbutt and Doug Szumski, would you kindly review? \n\nThanks","commit_id":"7ccba24b7ad19b1bc348de1de6d25af7f78303f9"},{"author":{"_account_id":16137,"name":"Tobias Urdin","email":"tobias.urdin@binero.com","username":"tobasco"},"change_message_id":"6e9c653411af2e5a9a86a9900dc9a344c0e13a99","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"01deb5d9_d06af593","updated":"2022-12-22 14:08:53.000000000","message":"This atleast warrants a release note and IMO now that the logic is even more than just calling it once I think we should add some testing as well.","commit_id":"7ccba24b7ad19b1bc348de1de6d25af7f78303f9"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"70e063224bf67377374bc26bcd5e76a911f3e9ce","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8eba1488_9683a2c8","updated":"2022-12-22 12:19:34.000000000","message":"This should address/resolve the comments\n\nMellanoxCI appears to be failing on multiple Nova PRs so this can be ignored/retried","commit_id":"7ccba24b7ad19b1bc348de1de6d25af7f78303f9"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"2865c7d9d9b16fc3ed1347e2e611528db5503f1d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"10d38113_a81dd66d","in_reply_to":"01deb5d9_d06af593","updated":"2022-12-22 16:18:21.000000000","message":"Release note added, hopefully it is comprehensive enough to provide the information needed for an operator to decide if they want to use this feature or not.\n\nThe original change did not have tests but I can try and add some though this will not be until next year now.","commit_id":"7ccba24b7ad19b1bc348de1de6d25af7f78303f9"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"29802aa16b02fb987ae632ee60b1b9682882bfcd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ad25e251_6363234a","in_reply_to":"10d38113_a81dd66d","updated":"2022-12-23 09:02:00.000000000","message":"Done","commit_id":"7ccba24b7ad19b1bc348de1de6d25af7f78303f9"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"3081c7da1af5e8ab1b8ad3fb38c122028d4eff9d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":6,"id":"91ff0db8_353196c5","updated":"2022-12-22 18:33:14.000000000","message":"Test added to check the correct number of calls to announce_self is made.\n\nThis is the first test I have written for Nova/OpenStack, so please let me know if it should be in a different directory/file, or if there is any general advice.\n\nI have tried to follow the existing test format as much as possible.","commit_id":"1e724549ada1aa805a366bcb95d6268f121307c0"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"702ebfbc946a064269ad66d54a09ba01a42f8c7c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"98dbe7dd_67516f1c","in_reply_to":"91ff0db8_353196c5","updated":"2023-01-06 11:07:13.000000000","message":"Done","commit_id":"1e724549ada1aa805a366bcb95d6268f121307c0"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"e6c492c2deb57c87fd25ff9bfc351810b92b6a85","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"2bdf55b2_ebffa85b","updated":"2023-01-02 15:19:54.000000000","message":"Hey thank you for this. One more thing.","commit_id":"b65de2174a69c8bb8bf2aebbbeca0dd383a86f93"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"1648f4e6e8b1eb9953fc9f13c8ea7053c076c4a1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"9f881a1e_98b33626","updated":"2023-01-04 15:01:31.000000000","message":"I have a different approach, let me know how do you feel with it. I understand this may be annoying as you already made a lot of efforts in the current change.","commit_id":"b65de2174a69c8bb8bf2aebbbeca0dd383a86f93"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"29802aa16b02fb987ae632ee60b1b9682882bfcd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"ca551c40_7bfd5e50","updated":"2022-12-23 09:02:00.000000000","message":"Release note links fix, test has been added.","commit_id":"b65de2174a69c8bb8bf2aebbbeca0dd383a86f93"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"702ebfbc946a064269ad66d54a09ba01a42f8c7c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"9fee9054_9fc5e2ce","updated":"2023-01-06 11:07:13.000000000","message":"Simplified approach based on Sahid\u0027s suggestion, so `enable_qemu_monitor_announce_self` is now an IntOpt rather than a BoolOpt\n\nAmended test and release note.\n\n\nI am keen to retain the interval setting that is introduced here - for certain fabrics it may be useful for operators. For us, 1 second was about the optimal value, however I can foresee situations where an operator may need to space out the announce_self calls to slightly longer.","commit_id":"8b5f17a52d60640899fbde16b42f86c859cae065"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"a99131810aca0a7faaafe380b27a79fa1e77b00c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"f231ddbc_591e1bf1","updated":"2023-01-10 09:17:23.000000000","message":"Thank a lot! looks pretty good, just have so;e requests, pleqse let me know if they mqke sense for you as-well!","commit_id":"8b5f17a52d60640899fbde16b42f86c859cae065"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"02a5a81c51cf3d9da48b24defc5009ae082a4eab","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"576b7c49_0545af3b","updated":"2023-01-16 12:42:39.000000000","message":"-1 because the current patch changes the datatype inplace and that is only allowed if the new type support a superset of the orgianal\n\nthis patch is going from bool/strign to int \"True\", \"yes\", \"y, are all valid values for bool based on how we parse them but not for int so the current patch would break upgrades.","commit_id":"f5653d3fa66ca04903fdfdd2407c05f8845c7875"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"d15998df46b22db7a9e28d287160ac8c2019e8ce","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"82b41763_954fe0fe","updated":"2023-01-16 12:32:59.000000000","message":"That is said I think we should move this option from workaround.\n\nBasically I would have deprecated the bool one in  `workaround.enable_qemu_monitor_announce_self`\n\nAnd have introduced `libvirt.enable_qemu_monitor_announce_self` as Int, with the new behavior.","commit_id":"f5653d3fa66ca04903fdfdd2407c05f8845c7875"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"325473f042fd53d32764566061070630824077e1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"830e971d_ed3a3171","in_reply_to":"82b41763_954fe0fe","updated":"2023-01-16 15:55:27.000000000","message":"Thanks for your comment Sahid.\n\nI\u0027m not sure this should be moving into libvirt - it *is* a workaround for a very specific use case, I don\u0027t know if there would be demand there for general users.\n\nMoving the option and deprecating an old one feels like over-engineering this relatively simple patch for what is probably not going to be used by 99% of operators.","commit_id":"f5653d3fa66ca04903fdfdd2407c05f8845c7875"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a8383d4f58077674a41558665e6bae0fdfded371","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"04d62691_e216f554","in_reply_to":"830e971d_ed3a3171","updated":"2023-01-16 23:14:18.000000000","message":"we should not move this out of worakounds.\n\nin theory this should not be requred this option","commit_id":"f5653d3fa66ca04903fdfdd2407c05f8845c7875"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"a8cdf8899d19c1aea1c64edc8e37b1ded0a45899","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"c07daad1_56c7d7e4","updated":"2023-01-16 16:38:51.000000000","message":"I\u0027m hoping this addresses all comments - this patch retains backwards compatibility at the cost of adding two options.\n\nThese are workaround options so for most users, they will never need to configure them. For those that do, such as myself, it is a useful addition.","commit_id":"7e1630f4200bddfc38b0d8669d0a6801ed484e02"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"5a8f18e03c90560f3b1d97fdd760b3e02f61ccee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"178b5fa2_b056865f","updated":"2023-01-18 10:56:44.000000000","message":"Looks good, just one thing regarding the options","commit_id":"56e011abebb23570341ed3fe402149af90a75b33"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"e8dd9c0567485ba9b0244083e93755f54b2f1a8c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"52b00a1a_d5b05d18","updated":"2023-01-17 09:33:04.000000000","message":"Thanks for the reviews.\n\nPatch-set 13 should address all comments and hopefully this should be acceptable to merge.","commit_id":"56e011abebb23570341ed3fe402149af90a75b33"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c9a85fdd102f9319bd287c6a2320481a3d4930b3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"3ea58463_4882b272","updated":"2023-01-18 14:16:35.000000000","message":"i mostly agree with shaid but also i woudl prefer to avoid the term retry and use count.","commit_id":"56e011abebb23570341ed3fe402149af90a75b33"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"1961672e89b4235144d7c4c8dbfe85e82c2e3ba0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"afe7377d_cbe07116","in_reply_to":"3ea58463_4882b272","updated":"2023-01-18 16:59:58.000000000","message":"That sounds sensible, amended","commit_id":"56e011abebb23570341ed3fe402149af90a75b33"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0ee11a68c44eb9af3e85b061c5e7c3a7debdae5c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"e73bc629_1a5185f6","updated":"2023-01-26 01:28:15.000000000","message":"I understand why we need to add these options but it feels pretty odd to me to add fine-tuning knobs to a [workarounds] option 😆 I don\u0027t have any better ideas really.\n\nSean and I chatted about it briefly and it seems like the sort of thing we\u0027d just hardcode being that it\u0027s a misfit workaround to begin with. But from what I understand, there is a high chance that whatever we hardcode might not be enough and we\u0027d be getting patches to tweak it in the future. I\u0027m not sure how bad of a thing that is really.\n\nOn the surface, it seems \"what\u0027s the harm in these little additions\" but thinking in a general sense, I hope [workarounds] doesn\u0027t turn into a sophisticated config where more options have sub-options and so on.\n\nTo be clear, I\u0027m not going to block this but I think it\u0027s probably best to get thoughts from additional reviewers like bauzas and gibi.","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"7b5e750bf84ab6bfbe9865f8e9dfa7676cc4c10d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":16,"id":"b08693c7_fea891d1","updated":"2023-01-25 09:10:11.000000000","message":"Please could you advise the next step to get this merged? It looks like it needs a +1 for workflow","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"2e126a4e6435ce1fd759b9925f3d786c072c8981","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"f47be3fb_69d3e66d","updated":"2023-01-19 14:12:29.000000000","message":"Sounds good thank you!","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"1961672e89b4235144d7c4c8dbfe85e82c2e3ba0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"999c5778_eeaa891e","updated":"2023-01-18 16:59:58.000000000","message":"Thank you Sahid and sean - please see what I hope is the final patchset - 16.","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"354edcf9673ba0e35be708831bbacf5fdf97ad41","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"bcb3f962_b6169a28","updated":"2023-01-20 10:02:42.000000000","message":"This looks good to me now, I like the move to count. I feel too involved with this downstream to +2.","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"f78e94a81e224a89b9002f95f1af7ed20d9f476e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"e94396a6_d5aa4159","updated":"2023-01-25 09:08:38.000000000","message":"pkvm- recheck","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"ff5d03d8bd5942a3831602496a3671cac5aba764","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"5d25a01f_e721f346","updated":"2023-01-24 14:46:20.000000000","message":"recheck","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"88c0fb851ddc9ed0f9fc0e01890875037e092a80","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":16,"id":"a8d0e341_8a0635bf","updated":"2023-01-24 14:45:53.000000000","message":"recheck","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"a2dbe5e9cfd537ed67a334d8c951e4654728bb3f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"32fab4ba_f368cecf","updated":"2023-01-19 14:12:50.000000000","message":"recheck timeout","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9ad7de3079e8452369ea48ec9c5388519fa1d8a7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"0407030e_2903ae45","updated":"2023-01-24 16:41:00.000000000","message":"thanks john for noting that this would be useful to you.\nim happy with the naming and release note in this version\n\nthe funcitonl test failure i think is unrelated there are several thigns broken and it passed on other python versions.","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"619e1655bd8c3993b749ab099d5e96098dff1788","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"7fd5b684_7e6731ad","in_reply_to":"6eb3c0a1_67417e24","updated":"2023-01-31 21:30:18.000000000","message":"I see, thank you for sharing these additional details. It is helpful to me to know the considerations that were taken ahead of the proposal, so thanks for that.","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"},{"author":{"_account_id":16137,"name":"Tobias Urdin","email":"tobias.urdin@binero.com","username":"tobasco"},"change_message_id":"9f08d7d04c9e8da999c0b53ac8512dcd0e904875","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"d0ab2ec6_467a6758","in_reply_to":"7fd5b684_7e6731ad","updated":"2023-03-10 07:38:17.000000000","message":"As the original author of the workaround I can indeed confirm that we are using this and it\u0027s the only reason why live migration is currently working for us using ML2/OVS.\n\nThere has been work in OVN to support binding multiple ports and then activating the destination port based on the RARP frames, unfortunately I have been able to test this nor are we running OVN, and removing this would also mean we\u0027d need to be sure the race condition between Neutron and Nova would not be an issue anymore.\n\nI really hope that we can remove this in the future.","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"f78e94a81e224a89b9002f95f1af7ed20d9f476e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"409d4b40_5da54707","in_reply_to":"a8d0e341_8a0635bf","updated":"2023-01-25 09:08:38.000000000","message":"Done","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"71082e625e6c6ca5bae6b0eb3a69d097de6910a8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"6eb3c0a1_67417e24","in_reply_to":"e73bc629_1a5185f6","updated":"2023-01-30 16:02:57.000000000","message":"Thanks for the review melwitt. I have addressed your two comments re: related options in `conf/workarounds.py`\n\nWhilst I cannot accurately gauge the usage of this workaround in the community, it does appear to be in use in not just my organization based on the fact that someone else committed the option of announce_self originally.\n\nI agree that the code-base shouldn\u0027t be bloated for the sake of it and workarounds given unnecessary sub-options and so on. As 99% of people probably don\u0027t even configure a workarounds section for Nova, this ought to be low impact to those users. For the users who have issues with certain live migration scenarios, this may be extremely useful.\n\nThere are a number of considerations when using announce_self, as fabrics may have an imposed arp packet limit over a period of time for example. Depending on the number of migrations that take place, it may not be suitable to send numerous attempts in quick succession.\n\nOr, in our case, we have to send 3 attempts to ensure a VM that has been live migrated is accessible on the network (I performed nearly 20,000 live migration tests in different scenarios to test this theory)\n\n\nThe hardcoded options simply didn\u0027t yield the desired effect for us, and I\u0027d wager that this is the case for other fabrics in the wild too - having a on/off toggle for this workaround isn\u0027t appropriate as the effectiveness depends on the particular use-case and thus fine-tuning is going to be required in a lot of cases.\n\n\nAny further comment welcomed, thank you","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"619e1655bd8c3993b749ab099d5e96098dff1788","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"514c6238_d2fcf85f","updated":"2023-01-31 21:30:18.000000000","message":"I feel convinced of the necessity of the workaround sub-options and why a hard-coding of reasonable values for the retry is unlikely to work well enough for all, given the wide variety of potential network fabrics. So, this looks OK to me.","commit_id":"fba851bf3a34562db9cdb783ae539556b8b7a329"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"9cf582c0d153e8a695f27c22c39d334838e300a3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"6663819d_4d51b649","updated":"2023-02-01 12:02:38.000000000","message":"Thanks for the approval melwitt and sean.\n\nI followed the existing code there which used caps for the config section - if it\u0027s rendered correctly then great. I\u0027m pleased to get this merged and over the line!","commit_id":"fba851bf3a34562db9cdb783ae539556b8b7a329"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"495310d40e6a7be9e4a7e05057021d3d1e54ed08","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"5f66c379_95741602","updated":"2023-01-31 16:42:55.000000000","message":"recheck","commit_id":"fba851bf3a34562db9cdb783ae539556b8b7a329"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5f5c067db018c067b2a33be8fb67c2a1260044c6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":17,"id":"c7b1b39c_2ec24fbb","in_reply_to":"6663819d_4d51b649","updated":"2023-02-01 17:37:40.000000000","message":"\u003e I followed the existing code there which used caps for the config section - if it\u0027s rendered correctly then great. I\u0027m pleased to get this merged and over the line!\n\nThere\u0027s a trick here in that the [DEFAULT] section is \"special\" and always capitalized 😆 I actually could find no doc about why, the most related thing I found is:\n\nhttps://stackoverflow.com/questions/124692/what-is-the-intended-use-of-the-default-section-in-config-files-used-by-configpa\n\nMaybe it\u0027s to make it look different since its settings will propagate to all other sections or maybe it\u0027s just tradition. I found some inconsistent capitalizations throughout our config doc but the hyperlinking works either way, which is the most important thing.","commit_id":"fba851bf3a34562db9cdb783ae539556b8b7a329"}],"nova/conf/workarounds.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"e6c492c2deb57c87fd25ff9bfc351810b92b6a85","unresolved":true,"context_lines":[{"line_number":378,"context_line":"                default\u003d3,"},{"line_number":379,"context_line":"                min\u003d0,"},{"line_number":380,"context_line":"                help\u003d\"\"\""},{"line_number":381,"context_line":"The number of times to re-send the announce_self command to the QEMU monitor,"},{"line_number":382,"context_line":"in addition to the first attempt. If max_retries is set to 3, there will be a"},{"line_number":383,"context_line":"total of four calls to the QEMU announce_self command."},{"line_number":384,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"25828b6a_bcf3b7c3","line":381,"updated":"2023-01-02 15:19:54.000000000","message":"I\u0027m not sure whether that really make sense to say if you put 3, 4 will be executed.\n\nI think the value should reflect the exact number of calls.","commit_id":"b65de2174a69c8bb8bf2aebbbeca0dd383a86f93"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"8b784bc5c97aaad666acb091c3a2f24c7d79d52f","unresolved":true,"context_lines":[{"line_number":378,"context_line":"                default\u003d3,"},{"line_number":379,"context_line":"                min\u003d0,"},{"line_number":380,"context_line":"                help\u003d\"\"\""},{"line_number":381,"context_line":"The number of times to re-send the announce_self command to the QEMU monitor,"},{"line_number":382,"context_line":"in addition to the first attempt. If max_retries is set to 3, there will be a"},{"line_number":383,"context_line":"total of four calls to the QEMU announce_self command."},{"line_number":384,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"80803aa7_18d1aa00","line":381,"in_reply_to":"25828b6a_bcf3b7c3","updated":"2023-01-03 10:16:49.000000000","message":"Thanks for the review Sahid.\n\nThis format follows the same style as a number of other existing Nova options that allow retries to be configured in addition to the first attempt of a call being made, e.g:\n\n* network_allocate_retries\n* http_retries \u003c- this option has very similar wording/explanation to this PR here\n\n\n----\n\nChanging the code so that `enable_qemu_monitor_announce_max_retries` equals the total number of calls to the qemu announce_self command, would mean that if a user set `enable_qemu_monitor_announce_max_retries \u003d True` *and* `enable_qemu_monitor_announce_max_retries \u003d 0` then the feature would not work.\n\nDefining a minimum value for `enable_qemu_monitor_announce_max_retries` as `1` also feels wrong if the parent option may not necessarily be enabled.\n\n\nWhat do you think? I can adjust the description to match that of `http_retries` if it\u0027s more readable?","commit_id":"b65de2174a69c8bb8bf2aebbbeca0dd383a86f93"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"bf63c30a2321afec60588484e8ef484b8d3fd37f","unresolved":true,"context_lines":[{"line_number":378,"context_line":"                default\u003d3,"},{"line_number":379,"context_line":"                min\u003d0,"},{"line_number":380,"context_line":"                help\u003d\"\"\""},{"line_number":381,"context_line":"The number of times to re-send the announce_self command to the QEMU monitor,"},{"line_number":382,"context_line":"in addition to the first attempt. If max_retries is set to 3, there will be a"},{"line_number":383,"context_line":"total of four calls to the QEMU announce_self command."},{"line_number":384,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ee069eab_1a2ef95d","line":381,"in_reply_to":"4fbc1622_578aa499","updated":"2023-01-05 10:45:03.000000000","message":"I like the suggestion of setting enable_qemu_monitor_announce_self to int.\n\nThis would require modifying the config option in workarounds.py for enable_qemu_monitor_announce_self from BoolOpt to IntOpt - is this OK?\n\n\nFor the retry_interval, for our env it was useful as it meant we could fine-tune the live-migrations to ensure the absolute minimal amount of downtime - it may be in other envs that people want to use a longer interval if they have a larger fabric that may take slightly longer to update, for example.","commit_id":"b65de2174a69c8bb8bf2aebbbeca0dd383a86f93"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"1648f4e6e8b1eb9953fc9f13c8ea7053c076c4a1","unresolved":true,"context_lines":[{"line_number":378,"context_line":"                default\u003d3,"},{"line_number":379,"context_line":"                min\u003d0,"},{"line_number":380,"context_line":"                help\u003d\"\"\""},{"line_number":381,"context_line":"The number of times to re-send the announce_self command to the QEMU monitor,"},{"line_number":382,"context_line":"in addition to the first attempt. If max_retries is set to 3, there will be a"},{"line_number":383,"context_line":"total of four calls to the QEMU announce_self command."},{"line_number":384,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"4fbc1622_578aa499","line":381,"in_reply_to":"80803aa7_18d1aa00","updated":"2023-01-04 15:01:31.000000000","message":"Quite a lot of options for the same thing.\n\nWhat about to convert the actual enable_qemu_monitor_announce_self to int. Then 0 would be deactivated and \u003e0 activated with number of retries? I think we can do that easily as bool convert well to int.\n\nAn other point. I would have probably not proposed an option for retry_interval. Basically we could hardcore a reasonable value (1s looks good), if in future we have the desire of setting it with reason we could then add an option.","commit_id":"b65de2174a69c8bb8bf2aebbbeca0dd383a86f93"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"702ebfbc946a064269ad66d54a09ba01a42f8c7c","unresolved":false,"context_lines":[{"line_number":378,"context_line":"                default\u003d3,"},{"line_number":379,"context_line":"                min\u003d0,"},{"line_number":380,"context_line":"                help\u003d\"\"\""},{"line_number":381,"context_line":"The number of times to re-send the announce_self command to the QEMU monitor,"},{"line_number":382,"context_line":"in addition to the first attempt. If max_retries is set to 3, there will be a"},{"line_number":383,"context_line":"total of four calls to the QEMU announce_self command."},{"line_number":384,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"3d8e5855_cbecaef5","line":381,"in_reply_to":"ee069eab_1a2ef95d","updated":"2023-01-06 11:07:13.000000000","message":"Done","commit_id":"b65de2174a69c8bb8bf2aebbbeca0dd383a86f93"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a8383d4f58077674a41558665e6bae0fdfded371","unresolved":true,"context_lines":[{"line_number":362,"context_line":"* :oslo.config:option:`DEFAULT.vif_plugging_timeout`"},{"line_number":363,"context_line":"\"\"\"),"},{"line_number":364,"context_line":"    cfg.IntOpt(\u0027enable_qemu_monitor_announce_self\u0027,"},{"line_number":365,"context_line":"                default\u003d0,"},{"line_number":366,"context_line":"                min\u003d0,"},{"line_number":367,"context_line":"                help\u003d\"\"\""},{"line_number":368,"context_line":"If set to 1 or more, the libvirt driver will try as a best effort to send"}],"source_content_type":"text/x-python","patch_set":9,"id":"11bc3b15_2e67133d","line":365,"updated":"2023-01-16 23:14:18.000000000","message":"as noted before you cannot change the data type form bool to int.\nthis patch cannot proceeed as written.\n\nplease revert this BoolOpt and add a new int opt for the number of anouchments to make.\n\ncfg.IntOpt(‘qemu_monitor_annouce_self_count’, default\u003d1, min\u003d1,…)","commit_id":"8b5f17a52d60640899fbde16b42f86c859cae065"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"e8dd9c0567485ba9b0244083e93755f54b2f1a8c","unresolved":false,"context_lines":[{"line_number":362,"context_line":"* :oslo.config:option:`DEFAULT.vif_plugging_timeout`"},{"line_number":363,"context_line":"\"\"\"),"},{"line_number":364,"context_line":"    cfg.IntOpt(\u0027enable_qemu_monitor_announce_self\u0027,"},{"line_number":365,"context_line":"                default\u003d0,"},{"line_number":366,"context_line":"                min\u003d0,"},{"line_number":367,"context_line":"                help\u003d\"\"\""},{"line_number":368,"context_line":"If set to 1 or more, the libvirt driver will try as a best effort to send"}],"source_content_type":"text/x-python","patch_set":9,"id":"8091e969_174103ae","line":365,"in_reply_to":"11bc3b15_2e67133d","updated":"2023-01-17 09:33:04.000000000","message":"This was reverted back to a bool in patch-set 12 which was uploaded yesterday.\n\nPlease note I have uploaded patch-set 13 now which addresses your other comments regarding the `enable` prefix.","commit_id":"8b5f17a52d60640899fbde16b42f86c859cae065"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a8383d4f58077674a41558665e6bae0fdfded371","unresolved":true,"context_lines":[{"line_number":377,"context_line":"* :oslo.config:option:`DEFAULT.compute_driver` (libvirt)"},{"line_number":378,"context_line":"\"\"\"),"},{"line_number":379,"context_line":"    cfg.IntOpt(\u0027enable_qemu_monitor_announce_self_interval\u0027,"},{"line_number":380,"context_line":"                default\u003d1,"},{"line_number":381,"context_line":"                min\u003d0,"},{"line_number":382,"context_line":"                help\u003d\"\"\""},{"line_number":383,"context_line":"The number of seconds to wait before sending the announce_self"}],"source_content_type":"text/x-python","patch_set":9,"id":"46114735_e9340fcf","line":380,"updated":"2023-01-16 23:14:18.000000000","message":"cfg.IntOpt(‘qemu_monitor_annouce_self_interval’,…)\n\nwe do not need the enbale_ prefix","commit_id":"8b5f17a52d60640899fbde16b42f86c859cae065"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"e8dd9c0567485ba9b0244083e93755f54b2f1a8c","unresolved":false,"context_lines":[{"line_number":377,"context_line":"* :oslo.config:option:`DEFAULT.compute_driver` (libvirt)"},{"line_number":378,"context_line":"\"\"\"),"},{"line_number":379,"context_line":"    cfg.IntOpt(\u0027enable_qemu_monitor_announce_self_interval\u0027,"},{"line_number":380,"context_line":"                default\u003d1,"},{"line_number":381,"context_line":"                min\u003d0,"},{"line_number":382,"context_line":"                help\u003d\"\"\""},{"line_number":383,"context_line":"The number of seconds to wait before sending the announce_self"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f2d6702_3675d866","line":380,"in_reply_to":"46114735_e9340fcf","updated":"2023-01-17 09:33:04.000000000","message":"Please see set 13","commit_id":"8b5f17a52d60640899fbde16b42f86c859cae065"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"02a5a81c51cf3d9da48b24defc5009ae082a4eab","unresolved":true,"context_lines":[{"line_number":359,"context_line":""},{"line_number":360,"context_line":"Related options:"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"* :oslo.config:option:`DEFAULT.vif_plugging_timeout`"},{"line_number":363,"context_line":"\"\"\"),"},{"line_number":364,"context_line":"    cfg.IntOpt(\u0027enable_qemu_monitor_announce_self\u0027,"},{"line_number":365,"context_line":"                default\u003d0,"},{"line_number":366,"context_line":"                min\u003d0,"},{"line_number":367,"context_line":"                help\u003d\"\"\""},{"line_number":368,"context_line":"If set to 1 or more, the libvirt driver will try as a best effort to send"},{"line_number":369,"context_line":"the announce-self command to the QEMU monitor the specified number of times,"},{"line_number":370,"context_line":"so that it generates RARP frames to update network switches in the post live"},{"line_number":371,"context_line":"migration phase on the destination."},{"line_number":372,"context_line":""},{"line_number":373,"context_line":"Please note that this causes the domain to be considered tainted by libvirt."},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"Related options:"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"* :oslo.config:option:`DEFAULT.compute_driver` (libvirt)"},{"line_number":378,"context_line":"\"\"\"),"},{"line_number":379,"context_line":"    cfg.IntOpt(\u0027enable_qemu_monitor_announce_self_interval\u0027,"},{"line_number":380,"context_line":"                default\u003d1,"},{"line_number":381,"context_line":"                min\u003d0,"}],"source_content_type":"text/x-python","patch_set":11,"id":"a9e65ebb_970cb4e1","line":378,"range":{"start_line":362,"start_character":0,"end_line":378,"end_character":5},"updated":"2023-01-16 12:42:39.000000000","message":"we have been dicussiong this on IRC so we cant change the data type form bool to int\n\nit will make enable_qemu_monitor_announce_self\u003dTrue invlaid\nand we may not require config change to upgrade.\n\nso my propoal is keep enable_qemu_monitor_announce_self unchnaged\n\nadd qemu_monitor_announce_self_retry and default to 1 or 3\n\n1 for current behavior 3 is what qemu does by default so that would also be an ok default.","commit_id":"f5653d3fa66ca04903fdfdd2407c05f8845c7875"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"a8cdf8899d19c1aea1c64edc8e37b1ded0a45899","unresolved":false,"context_lines":[{"line_number":359,"context_line":""},{"line_number":360,"context_line":"Related options:"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"* :oslo.config:option:`DEFAULT.vif_plugging_timeout`"},{"line_number":363,"context_line":"\"\"\"),"},{"line_number":364,"context_line":"    cfg.IntOpt(\u0027enable_qemu_monitor_announce_self\u0027,"},{"line_number":365,"context_line":"                default\u003d0,"},{"line_number":366,"context_line":"                min\u003d0,"},{"line_number":367,"context_line":"                help\u003d\"\"\""},{"line_number":368,"context_line":"If set to 1 or more, the libvirt driver will try as a best effort to send"},{"line_number":369,"context_line":"the announce-self command to the QEMU monitor the specified number of times,"},{"line_number":370,"context_line":"so that it generates RARP frames to update network switches in the post live"},{"line_number":371,"context_line":"migration phase on the destination."},{"line_number":372,"context_line":""},{"line_number":373,"context_line":"Please note that this causes the domain to be considered tainted by libvirt."},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"Related options:"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"* :oslo.config:option:`DEFAULT.compute_driver` (libvirt)"},{"line_number":378,"context_line":"\"\"\"),"},{"line_number":379,"context_line":"    cfg.IntOpt(\u0027enable_qemu_monitor_announce_self_interval\u0027,"},{"line_number":380,"context_line":"                default\u003d1,"},{"line_number":381,"context_line":"                min\u003d0,"}],"source_content_type":"text/x-python","patch_set":11,"id":"2dc01cb9_e3d3ae04","line":378,"range":{"start_line":362,"start_character":0,"end_line":378,"end_character":5},"in_reply_to":"6b6da2b2_81448761","updated":"2023-01-16 16:38:51.000000000","message":"Amended to my approach in patchset 8 - with some slight polishing.","commit_id":"f5653d3fa66ca04903fdfdd2407c05f8845c7875"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"325473f042fd53d32764566061070630824077e1","unresolved":true,"context_lines":[{"line_number":359,"context_line":""},{"line_number":360,"context_line":"Related options:"},{"line_number":361,"context_line":""},{"line_number":362,"context_line":"* :oslo.config:option:`DEFAULT.vif_plugging_timeout`"},{"line_number":363,"context_line":"\"\"\"),"},{"line_number":364,"context_line":"    cfg.IntOpt(\u0027enable_qemu_monitor_announce_self\u0027,"},{"line_number":365,"context_line":"                default\u003d0,"},{"line_number":366,"context_line":"                min\u003d0,"},{"line_number":367,"context_line":"                help\u003d\"\"\""},{"line_number":368,"context_line":"If set to 1 or more, the libvirt driver will try as a best effort to send"},{"line_number":369,"context_line":"the announce-self command to the QEMU monitor the specified number of times,"},{"line_number":370,"context_line":"so that it generates RARP frames to update network switches in the post live"},{"line_number":371,"context_line":"migration phase on the destination."},{"line_number":372,"context_line":""},{"line_number":373,"context_line":"Please note that this causes the domain to be considered tainted by libvirt."},{"line_number":374,"context_line":""},{"line_number":375,"context_line":"Related options:"},{"line_number":376,"context_line":""},{"line_number":377,"context_line":"* :oslo.config:option:`DEFAULT.compute_driver` (libvirt)"},{"line_number":378,"context_line":"\"\"\"),"},{"line_number":379,"context_line":"    cfg.IntOpt(\u0027enable_qemu_monitor_announce_self_interval\u0027,"},{"line_number":380,"context_line":"                default\u003d1,"},{"line_number":381,"context_line":"                min\u003d0,"}],"source_content_type":"text/x-python","patch_set":11,"id":"6b6da2b2_81448761","line":378,"range":{"start_line":362,"start_character":0,"end_line":378,"end_character":5},"in_reply_to":"a9e65ebb_970cb4e1","updated":"2023-01-16 15:55:27.000000000","message":"Thank you sean for the comment.\n\nI have read the IRC logs today so will fall back to my approach in patch-set 8... I\u0027ll upload this for review shortly and hopefully this can be merged soon","commit_id":"f5653d3fa66ca04903fdfdd2407c05f8845c7875"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"5a8f18e03c90560f3b1d97fdd760b3e02f61ccee","unresolved":true,"context_lines":[{"line_number":376,"context_line":"\"\"\"),"},{"line_number":377,"context_line":"    cfg.IntOpt(\u0027qemu_monitor_announce_self_retries\u0027,"},{"line_number":378,"context_line":"                default\u003d3,"},{"line_number":379,"context_line":"                min\u003d0,"},{"line_number":380,"context_line":"                help\u003d\"\"\""},{"line_number":381,"context_line":"The total number of times to send the announce_self command to the QEMU"},{"line_number":382,"context_line":"monitor when enable_qemu_monitor_announce_self is enabled."}],"source_content_type":"text/x-python","patch_set":13,"id":"de5fc007_23aa5a15","line":379,"updated":"2023-01-18 10:56:44.000000000","message":"What is happening if we set this to 0 where announce is enabled? I think min should be 1 right?","commit_id":"56e011abebb23570341ed3fe402149af90a75b33"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"1961672e89b4235144d7c4c8dbfe85e82c2e3ba0","unresolved":false,"context_lines":[{"line_number":376,"context_line":"\"\"\"),"},{"line_number":377,"context_line":"    cfg.IntOpt(\u0027qemu_monitor_announce_self_retries\u0027,"},{"line_number":378,"context_line":"                default\u003d3,"},{"line_number":379,"context_line":"                min\u003d0,"},{"line_number":380,"context_line":"                help\u003d\"\"\""},{"line_number":381,"context_line":"The total number of times to send the announce_self command to the QEMU"},{"line_number":382,"context_line":"monitor when enable_qemu_monitor_announce_self is enabled."}],"source_content_type":"text/x-python","patch_set":13,"id":"b4aa3688_dcf61c42","line":379,"in_reply_to":"ca341238_00b279eb","updated":"2023-01-18 16:59:58.000000000","message":"Done","commit_id":"56e011abebb23570341ed3fe402149af90a75b33"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c9a85fdd102f9319bd287c6a2320481a3d4930b3","unresolved":true,"context_lines":[{"line_number":376,"context_line":"\"\"\"),"},{"line_number":377,"context_line":"    cfg.IntOpt(\u0027qemu_monitor_announce_self_retries\u0027,"},{"line_number":378,"context_line":"                default\u003d3,"},{"line_number":379,"context_line":"                min\u003d0,"},{"line_number":380,"context_line":"                help\u003d\"\"\""},{"line_number":381,"context_line":"The total number of times to send the announce_self command to the QEMU"},{"line_number":382,"context_line":"monitor when enable_qemu_monitor_announce_self is enabled."}],"source_content_type":"text/x-python","patch_set":13,"id":"ca341238_00b279eb","line":379,"in_reply_to":"de5fc007_23aa5a15","updated":"2023-01-18 14:16:35.000000000","message":"i used count in my coment because retries is ambiguous\n\nretries imples that it could fail and we retry multiple times\nso i wanted to avodi that. so if we use count as the option name then min 1 would also be correct.","commit_id":"56e011abebb23570341ed3fe402149af90a75b33"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"5a8f18e03c90560f3b1d97fdd760b3e02f61ccee","unresolved":true,"context_lines":[{"line_number":387,"context_line":"\"\"\"),"},{"line_number":388,"context_line":"    cfg.IntOpt(\u0027qemu_monitor_announce_self_retry_interval\u0027,"},{"line_number":389,"context_line":"                default\u003d1,"},{"line_number":390,"context_line":"                min\u003d0,"},{"line_number":391,"context_line":"                help\u003d\"\"\""},{"line_number":392,"context_line":"The number of seconds to wait before re-sending the announce_self"},{"line_number":393,"context_line":"command to the QEMU monitor."}],"source_content_type":"text/x-python","patch_set":13,"id":"42dc539d_36347c01","line":390,"updated":"2023-01-18 10:56:44.000000000","message":"The min should be 1 here, right?","commit_id":"56e011abebb23570341ed3fe402149af90a75b33"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c9a85fdd102f9319bd287c6a2320481a3d4930b3","unresolved":true,"context_lines":[{"line_number":387,"context_line":"\"\"\"),"},{"line_number":388,"context_line":"    cfg.IntOpt(\u0027qemu_monitor_announce_self_retry_interval\u0027,"},{"line_number":389,"context_line":"                default\u003d1,"},{"line_number":390,"context_line":"                min\u003d0,"},{"line_number":391,"context_line":"                help\u003d\"\"\""},{"line_number":392,"context_line":"The number of seconds to wait before re-sending the announce_self"},{"line_number":393,"context_line":"command to the QEMU monitor."}],"source_content_type":"text/x-python","patch_set":13,"id":"b531bb90_b096a7be","line":390,"in_reply_to":"42dc539d_36347c01","updated":"2023-01-18 14:16:35.000000000","message":"yes this should also be 1","commit_id":"56e011abebb23570341ed3fe402149af90a75b33"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"1961672e89b4235144d7c4c8dbfe85e82c2e3ba0","unresolved":false,"context_lines":[{"line_number":387,"context_line":"\"\"\"),"},{"line_number":388,"context_line":"    cfg.IntOpt(\u0027qemu_monitor_announce_self_retry_interval\u0027,"},{"line_number":389,"context_line":"                default\u003d1,"},{"line_number":390,"context_line":"                min\u003d0,"},{"line_number":391,"context_line":"                help\u003d\"\"\""},{"line_number":392,"context_line":"The number of seconds to wait before re-sending the announce_self"},{"line_number":393,"context_line":"command to the QEMU monitor."}],"source_content_type":"text/x-python","patch_set":13,"id":"2880622c_ee4e9716","line":390,"in_reply_to":"b531bb90_b096a7be","updated":"2023-01-18 16:59:58.000000000","message":"Done","commit_id":"56e011abebb23570341ed3fe402149af90a75b33"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0ee11a68c44eb9af3e85b061c5e7c3a7debdae5c","unresolved":true,"context_lines":[{"line_number":383,"context_line":""},{"line_number":384,"context_line":"Related options:"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"* :oslo.config:option:`DEFAULT.compute_driver` (libvirt)"},{"line_number":387,"context_line":"\"\"\"),"},{"line_number":388,"context_line":"    cfg.IntOpt(\u0027qemu_monitor_announce_self_interval\u0027,"},{"line_number":389,"context_line":"                default\u003d1,"}],"source_content_type":"text/x-python","patch_set":16,"id":"dc58be89_2abb5cf0","line":386,"updated":"2023-01-26 01:28:15.000000000","message":"I would have thought the related option here would have been workarounds.enable_qemu_monitor_announce_self instead of compute_driver or at least in addition to.","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"71082e625e6c6ca5bae6b0eb3a69d097de6910a8","unresolved":false,"context_lines":[{"line_number":383,"context_line":""},{"line_number":384,"context_line":"Related options:"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"* :oslo.config:option:`DEFAULT.compute_driver` (libvirt)"},{"line_number":387,"context_line":"\"\"\"),"},{"line_number":388,"context_line":"    cfg.IntOpt(\u0027qemu_monitor_announce_self_interval\u0027,"},{"line_number":389,"context_line":"                default\u003d1,"}],"source_content_type":"text/x-python","patch_set":16,"id":"1e93c16a_d7645b0d","line":386,"in_reply_to":"dc58be89_2abb5cf0","updated":"2023-01-30 16:02:57.000000000","message":"Done","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"0ee11a68c44eb9af3e85b061c5e7c3a7debdae5c","unresolved":true,"context_lines":[{"line_number":394,"context_line":""},{"line_number":395,"context_line":"Related options:"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"* :oslo.config:option:`DEFAULT.compute_driver` (libvirt)"},{"line_number":398,"context_line":"\"\"\"),"},{"line_number":399,"context_line":"    cfg.BoolOpt(\u0027disable_compute_service_check_for_ffu\u0027,"},{"line_number":400,"context_line":"                default\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":16,"id":"78b2f887_e62c4d4d","line":397,"updated":"2023-01-26 01:28:15.000000000","message":"Ditto.","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"71082e625e6c6ca5bae6b0eb3a69d097de6910a8","unresolved":false,"context_lines":[{"line_number":394,"context_line":""},{"line_number":395,"context_line":"Related options:"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"* :oslo.config:option:`DEFAULT.compute_driver` (libvirt)"},{"line_number":398,"context_line":"\"\"\"),"},{"line_number":399,"context_line":"    cfg.BoolOpt(\u0027disable_compute_service_check_for_ffu\u0027,"},{"line_number":400,"context_line":"                default\u003dFalse,"}],"source_content_type":"text/x-python","patch_set":16,"id":"c444a1fe_cb3d6c71","line":397,"in_reply_to":"78b2f887_e62c4d4d","updated":"2023-01-30 16:02:57.000000000","message":"Done","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b9cb212f77316e7c6da163fbdf28bb02fb1cb211","unresolved":true,"context_lines":[{"line_number":383,"context_line":""},{"line_number":384,"context_line":"Related options:"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"* :oslo.config:option:`WORKAROUNDS.enable_qemu_monitor_announce_self` (libvirt)"},{"line_number":387,"context_line":"\"\"\"),"},{"line_number":388,"context_line":"    cfg.IntOpt(\u0027qemu_monitor_announce_self_interval\u0027,"},{"line_number":389,"context_line":"                default\u003d1,"}],"source_content_type":"text/x-python","patch_set":17,"id":"05038a8c_11b1c4a6","line":386,"updated":"2023-01-31 19:02:28.000000000","message":"+1 yep so this adresses melaines feedback\n\nand the rendered docs look correct \n\nhttps://298d8c269329d62262cb-86415e0a04f340ad7235be99b6135f85.ssl.cf2.rackcdn.com/867324/17/check/openstack-tox-docs/9e0aa8a/docs/configuration/config.html#workarounds.enable_qemu_monitor_announce_self","commit_id":"fba851bf3a34562db9cdb783ae539556b8b7a329"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"619e1655bd8c3993b749ab099d5e96098dff1788","unresolved":true,"context_lines":[{"line_number":383,"context_line":""},{"line_number":384,"context_line":"Related options:"},{"line_number":385,"context_line":""},{"line_number":386,"context_line":"* :oslo.config:option:`WORKAROUNDS.enable_qemu_monitor_announce_self` (libvirt)"},{"line_number":387,"context_line":"\"\"\"),"},{"line_number":388,"context_line":"    cfg.IntOpt(\u0027qemu_monitor_announce_self_interval\u0027,"},{"line_number":389,"context_line":"                default\u003d1,"}],"source_content_type":"text/x-python","patch_set":17,"id":"6e56e7ee_17610ce9","line":386,"range":{"start_line":386,"start_character":23,"end_line":386,"end_character":34},"updated":"2023-01-31 21:30:18.000000000","message":"Nit: the [workarounds] config section isn\u0027t all caps the way the [DEFAULT] section is. I noticed the hyperlink in the doc still works though, so maybe these are not case sensitive.\n\n(later) They are indeed not case sensitive, TIL:\n\nhttps://en.wikipedia.org/wiki/INI_file#Case_sensitivity","commit_id":"fba851bf3a34562db9cdb783ae539556b8b7a329"}],"nova/tests/unit/virt/libvirt/test_driver.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"a99131810aca0a7faaafe380b27a79fa1e77b00c","unresolved":true,"context_lines":[{"line_number":1823,"context_line":"        mock_guest.set_user_password.assert_called_once_with(\"root\", \"123\")"},{"line_number":1824,"context_line":""},{"line_number":1825,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_guest\u0027)"},{"line_number":1826,"context_line":"    def test_qemu_announce_self(self, mock_get_guest):"},{"line_number":1827,"context_line":"        # Enable the workaround, configure to call announce_self 3 times"},{"line_number":1828,"context_line":"        self.flags(enable_qemu_monitor_announce_self\u003d3, group\u003d\u0027workarounds\u0027)"},{"line_number":1829,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"9af2bf7f_b3c8adee","line":1826,"updated":"2023-01-10 09:17:23.000000000","message":"Could you add a test to validate the behavior when enable_qemu_monitor_announce_self\u003dTrue or False. To ensure the backward compatibility?","commit_id":"8b5f17a52d60640899fbde16b42f86c859cae065"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"8f99781d313ff2f76974e5652ee97605869c9d61","unresolved":false,"context_lines":[{"line_number":1823,"context_line":"        mock_guest.set_user_password.assert_called_once_with(\"root\", \"123\")"},{"line_number":1824,"context_line":""},{"line_number":1825,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_guest\u0027)"},{"line_number":1826,"context_line":"    def test_qemu_announce_self(self, mock_get_guest):"},{"line_number":1827,"context_line":"        # Enable the workaround, configure to call announce_self 3 times"},{"line_number":1828,"context_line":"        self.flags(enable_qemu_monitor_announce_self\u003d3, group\u003d\u0027workarounds\u0027)"},{"line_number":1829,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"450ebcc2_60b6ea51","line":1826,"in_reply_to":"1c8e0167_e107f7a8","updated":"2023-01-12 12:05:08.000000000","message":"So that is backward incompatible you can\u0027t do that.\n\nWhat is happening if we set the option to True?","commit_id":"8b5f17a52d60640899fbde16b42f86c859cae065"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"d931a5eb9d63d35bdcd22eac90eb84d7a9e1286b","unresolved":true,"context_lines":[{"line_number":1823,"context_line":"        mock_guest.set_user_password.assert_called_once_with(\"root\", \"123\")"},{"line_number":1824,"context_line":""},{"line_number":1825,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_guest\u0027)"},{"line_number":1826,"context_line":"    def test_qemu_announce_self(self, mock_get_guest):"},{"line_number":1827,"context_line":"        # Enable the workaround, configure to call announce_self 3 times"},{"line_number":1828,"context_line":"        self.flags(enable_qemu_monitor_announce_self\u003d3, group\u003d\u0027workarounds\u0027)"},{"line_number":1829,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"80609b12_da383b66","line":1826,"in_reply_to":"450ebcc2_60b6ea51","updated":"2023-01-12 15:04:04.000000000","message":"From your comment on Jan 04:\n`What about to convert the actual enable_qemu_monitor_announce_self to int. Then 0 would be deactivated and \u003e0 activated with number of retries? I think we can do that easily as bool convert well to int.`\n\n\nI changed the option in `workarounds.py` from a BoolOpt to IntOpt to be able to configure the number of retries.\n\n\nIt is backwards incompatible, an operator will need to change the option from True to `1` - but I\u0027m afraid I can\u0027t see how an operator would be able to configure either `enable_qemu_monitor_announce_self \u003d True` or `enable_qemu_monitor_announce_self \u003d 3` for example in their Nova configuration.\n\nCan you please help me understand what type of option you would expect `enable_qemu_monitor_announce_self` to be set as in workarounds.py to be able to retain backwards compatibility whilst also allowing an operator to set how many retries?\n\n\nI think it\u0027s worth highlighting that this change is effectively adding a feature for a very specific workaround that will not be used by many operators. Adding an option for those that need it did seem like the best approach and would maintain backwards compatibility - this is the approach I took up until patchset 8.\n\nWould you prefer I revert to that, to retain backwards compatibility at the cost of introducing an extra option?","commit_id":"8b5f17a52d60640899fbde16b42f86c859cae065"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"9f0f46582153a3ee37f0242be4ce4b5a01aa0b72","unresolved":true,"context_lines":[{"line_number":1823,"context_line":"        mock_guest.set_user_password.assert_called_once_with(\"root\", \"123\")"},{"line_number":1824,"context_line":""},{"line_number":1825,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_guest\u0027)"},{"line_number":1826,"context_line":"    def test_qemu_announce_self(self, mock_get_guest):"},{"line_number":1827,"context_line":"        # Enable the workaround, configure to call announce_self 3 times"},{"line_number":1828,"context_line":"        self.flags(enable_qemu_monitor_announce_self\u003d3, group\u003d\u0027workarounds\u0027)"},{"line_number":1829,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"a4718418_70c2ce6f","line":1826,"in_reply_to":"80609b12_da383b66","updated":"2023-01-13 08:08:20.000000000","message":"Sorry As, I was expecting that you would have done tests. What is happening if you set True to this option? An exception get raised?","commit_id":"8b5f17a52d60640899fbde16b42f86c859cae065"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a8383d4f58077674a41558665e6bae0fdfded371","unresolved":true,"context_lines":[{"line_number":1823,"context_line":"        mock_guest.set_user_password.assert_called_once_with(\"root\", \"123\")"},{"line_number":1824,"context_line":""},{"line_number":1825,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_guest\u0027)"},{"line_number":1826,"context_line":"    def test_qemu_announce_self(self, mock_get_guest):"},{"line_number":1827,"context_line":"        # Enable the workaround, configure to call announce_self 3 times"},{"line_number":1828,"context_line":"        self.flags(enable_qemu_monitor_announce_self\u003d3, group\u003d\u0027workarounds\u0027)"},{"line_number":1829,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"a5fe530f_0b25076a","line":1826,"in_reply_to":"85075b07_b36d7406","updated":"2023-01-16 23:14:18.000000000","message":"our upgrade requirement do not allow the type of a config option to be changed form into to bool so if we were to do that it would requrie the config option to be deprecated and a new one to be added. however as is said in my previoius revew the simplest solution is to keep the orginal enable config option unchange as a bool to opt into the behavior and just add the addtaional paramters.\n\nwe likely will eventually remove the workaround options and the self announch code at some point in the future. the code should really not be requried and its a workaorund for neutron backend that do not conform with the contract between nova and neutorn.\n\nthe contract between nova and neutron is that network-vif—plugged is only sent when the backend is fully configured. ovn and several other backend does not conform to that and this workaround option was added to workaround the bug in the neutron backend impelmation.\n\nwith that context it wold be incorrect to move this to the libvirt section as this is not something you should have to use if the neutron backend worked as intended.","commit_id":"8b5f17a52d60640899fbde16b42f86c859cae065"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"973c1c9b1ef428f7b41a3c7a4cf7c2af23865c25","unresolved":false,"context_lines":[{"line_number":1823,"context_line":"        mock_guest.set_user_password.assert_called_once_with(\"root\", \"123\")"},{"line_number":1824,"context_line":""},{"line_number":1825,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_guest\u0027)"},{"line_number":1826,"context_line":"    def test_qemu_announce_self(self, mock_get_guest):"},{"line_number":1827,"context_line":"        # Enable the workaround, configure to call announce_self 3 times"},{"line_number":1828,"context_line":"        self.flags(enable_qemu_monitor_announce_self\u003d3, group\u003d\u0027workarounds\u0027)"},{"line_number":1829,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"1c8e0167_e107f7a8","line":1826,"in_reply_to":"9af2bf7f_b3c8adee","updated":"2023-01-11 11:23:16.000000000","message":"`enable_qemu_monitor_announce_self` option has been changed from BoolOpt to IntOpt . Setting the option to `True` or `False` is not valid with this change so I don\u0027t see that the test for true/false is a valid case.\n\nOperators will need to amend their configuration from a bool to an int as a result of this change.","commit_id":"8b5f17a52d60640899fbde16b42f86c859cae065"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"70385015244d4f2155cfea263b9fb89a95fb688a","unresolved":true,"context_lines":[{"line_number":1823,"context_line":"        mock_guest.set_user_password.assert_called_once_with(\"root\", \"123\")"},{"line_number":1824,"context_line":""},{"line_number":1825,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_guest\u0027)"},{"line_number":1826,"context_line":"    def test_qemu_announce_self(self, mock_get_guest):"},{"line_number":1827,"context_line":"        # Enable the workaround, configure to call announce_self 3 times"},{"line_number":1828,"context_line":"        self.flags(enable_qemu_monitor_announce_self\u003d3, group\u003d\u0027workarounds\u0027)"},{"line_number":1829,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"e5916d3d_da17c582","line":1826,"in_reply_to":"a4718418_70c2ce6f","updated":"2023-01-13 10:02:23.000000000","message":"Hi, tests have been written on the basis of changing the option from a boolopt to an intopt.\n\nIf you try and specify \u0027True\u0027 for an `IntOpt` then the service will error because oslo_config fails to verify the configuration, similar to if you specify a value other than `0` or `1` for a BoolOpt.\n\n\nSo I believe this leaves two options:\n\n1) Revert to patch-set 8, where a new option specifying the number of retries is added, which retains backwards compatibility\n\nor...\n\n2) Introduce this change as it is, and update the release note that operators using this workaround will need to adjust their configuration from \u0027True\u0027 to an integer value for `enable_qemu_monitor_announce_self`\n\n\nAny guidance is appreciated, thanks for your help so far","commit_id":"8b5f17a52d60640899fbde16b42f86c859cae065"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"e8dd9c0567485ba9b0244083e93755f54b2f1a8c","unresolved":false,"context_lines":[{"line_number":1823,"context_line":"        mock_guest.set_user_password.assert_called_once_with(\"root\", \"123\")"},{"line_number":1824,"context_line":""},{"line_number":1825,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_guest\u0027)"},{"line_number":1826,"context_line":"    def test_qemu_announce_self(self, mock_get_guest):"},{"line_number":1827,"context_line":"        # Enable the workaround, configure to call announce_self 3 times"},{"line_number":1828,"context_line":"        self.flags(enable_qemu_monitor_announce_self\u003d3, group\u003d\u0027workarounds\u0027)"},{"line_number":1829,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"b2779973_f2686bc1","line":1826,"in_reply_to":"a5fe530f_0b25076a","updated":"2023-01-17 09:33:04.000000000","message":"Thanks for the additional context around this workaround, it\u0027s useful info.","commit_id":"8b5f17a52d60640899fbde16b42f86c859cae065"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"a8cdf8899d19c1aea1c64edc8e37b1ded0a45899","unresolved":false,"context_lines":[{"line_number":1823,"context_line":"        mock_guest.set_user_password.assert_called_once_with(\"root\", \"123\")"},{"line_number":1824,"context_line":""},{"line_number":1825,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_guest\u0027)"},{"line_number":1826,"context_line":"    def test_qemu_announce_self(self, mock_get_guest):"},{"line_number":1827,"context_line":"        # Enable the workaround, configure to call announce_self 3 times"},{"line_number":1828,"context_line":"        self.flags(enable_qemu_monitor_announce_self\u003d3, group\u003d\u0027workarounds\u0027)"},{"line_number":1829,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"85075b07_b36d7406","line":1826,"in_reply_to":"d263d331_d22e3c09","updated":"2023-01-16 16:38:51.000000000","message":"I believe deprecating options and moving them around is over-engineering this relatively small patch.","commit_id":"8b5f17a52d60640899fbde16b42f86c859cae065"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"32d4cafd2331d7e5bae8356d4875243966a80164","unresolved":true,"context_lines":[{"line_number":1823,"context_line":"        mock_guest.set_user_password.assert_called_once_with(\"root\", \"123\")"},{"line_number":1824,"context_line":""},{"line_number":1825,"context_line":"    @mock.patch(\u0027nova.virt.libvirt.host.Host.get_guest\u0027)"},{"line_number":1826,"context_line":"    def test_qemu_announce_self(self, mock_get_guest):"},{"line_number":1827,"context_line":"        # Enable the workaround, configure to call announce_self 3 times"},{"line_number":1828,"context_line":"        self.flags(enable_qemu_monitor_announce_self\u003d3, group\u003d\u0027workarounds\u0027)"},{"line_number":1829,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"d263d331_d22e3c09","line":1826,"in_reply_to":"e5916d3d_da17c582","updated":"2023-01-16 12:31:19.000000000","message":"There is a third solution which will be to deprecate `enable_qemu_monitor_announce_self` and provide  `enable_qemu_monitor_announce_self_retries` where 0 will be deactivated and \u003e0 will be the number of retry.","commit_id":"8b5f17a52d60640899fbde16b42f86c859cae065"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"551a4ecb0a2d920e7922047e12c98b9d951c0ce3","unresolved":true,"context_lines":[{"line_number":11027,"context_line":""},{"line_number":11028,"context_line":"    def _qemu_monitor_announce_self(self, instance):"},{"line_number":11029,"context_line":"        \"\"\"Send announce_self command to QEMU monitor."},{"line_number":11030,"context_line":""},{"line_number":11031,"context_line":"        This is to trigger generation of broadcast RARP frames to"},{"line_number":11032,"context_line":"        update network switches. This is best effort."},{"line_number":11033,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"c5d41408_8c577013","side":"PARENT","line":11030,"updated":"2022-12-13 10:27:04.000000000","message":"nit: unrelated change","commit_id":"9e2fdb4470603bf3c4f0d21560cced1eb748cd1b"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"70e063224bf67377374bc26bcd5e76a911f3e9ce","unresolved":false,"context_lines":[{"line_number":11027,"context_line":""},{"line_number":11028,"context_line":"    def _qemu_monitor_announce_self(self, instance):"},{"line_number":11029,"context_line":"        \"\"\"Send announce_self command to QEMU monitor."},{"line_number":11030,"context_line":""},{"line_number":11031,"context_line":"        This is to trigger generation of broadcast RARP frames to"},{"line_number":11032,"context_line":"        update network switches. This is best effort."},{"line_number":11033,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"6417dcf6_aba4404a","side":"PARENT","line":11030,"in_reply_to":"302a91c8_a1535b89","updated":"2022-12-22 12:19:34.000000000","message":"Done","commit_id":"9e2fdb4470603bf3c4f0d21560cced1eb748cd1b"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"8e35e31b848b7ba4a0ab8fdf4a13859606c2c07b","unresolved":true,"context_lines":[{"line_number":11027,"context_line":""},{"line_number":11028,"context_line":"    def _qemu_monitor_announce_self(self, instance):"},{"line_number":11029,"context_line":"        \"\"\"Send announce_self command to QEMU monitor."},{"line_number":11030,"context_line":""},{"line_number":11031,"context_line":"        This is to trigger generation of broadcast RARP frames to"},{"line_number":11032,"context_line":"        update network switches. This is best effort."},{"line_number":11033,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"302a91c8_a1535b89","side":"PARENT","line":11030,"in_reply_to":"c5d41408_8c577013","updated":"2022-12-13 11:45:07.000000000","message":"This patch builds on `_qemu_monitor_announce_self` unless I misunderstand your comment?","commit_id":"9e2fdb4470603bf3c4f0d21560cced1eb748cd1b"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"551a4ecb0a2d920e7922047e12c98b9d951c0ce3","unresolved":true,"context_lines":[{"line_number":11034,"context_line":"        if not CONF.workarounds.enable_qemu_monitor_announce_self:"},{"line_number":11035,"context_line":"            return"},{"line_number":11036,"context_line":""},{"line_number":11037,"context_line":"        LOG.info(\u0027Sending announce-self command to QEMU monitor\u0027,"},{"line_number":11038,"context_line":"                 instance\u003dinstance)"},{"line_number":11039,"context_line":""},{"line_number":11040,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"b3eceeeb_5f1e22a7","side":"PARENT","line":11037,"updated":"2022-12-13 10:27:04.000000000","message":"You can keep this line and pass to it the number of retry and interval configured.","commit_id":"9e2fdb4470603bf3c4f0d21560cced1eb748cd1b"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"70e063224bf67377374bc26bcd5e76a911f3e9ce","unresolved":false,"context_lines":[{"line_number":11034,"context_line":"        if not CONF.workarounds.enable_qemu_monitor_announce_self:"},{"line_number":11035,"context_line":"            return"},{"line_number":11036,"context_line":""},{"line_number":11037,"context_line":"        LOG.info(\u0027Sending announce-self command to QEMU monitor\u0027,"},{"line_number":11038,"context_line":"                 instance\u003dinstance)"},{"line_number":11039,"context_line":""},{"line_number":11040,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"6fbb8b5f_663a72e7","side":"PARENT","line":11037,"in_reply_to":"572ba23a_13b8c8c7","updated":"2022-12-22 12:19:34.000000000","message":"Done","commit_id":"9e2fdb4470603bf3c4f0d21560cced1eb748cd1b"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"8e35e31b848b7ba4a0ab8fdf4a13859606c2c07b","unresolved":true,"context_lines":[{"line_number":11034,"context_line":"        if not CONF.workarounds.enable_qemu_monitor_announce_self:"},{"line_number":11035,"context_line":"            return"},{"line_number":11036,"context_line":""},{"line_number":11037,"context_line":"        LOG.info(\u0027Sending announce-self command to QEMU monitor\u0027,"},{"line_number":11038,"context_line":"                 instance\u003dinstance)"},{"line_number":11039,"context_line":""},{"line_number":11040,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"572ba23a_13b8c8c7","side":"PARENT","line":11037,"in_reply_to":"b3eceeeb_5f1e22a7","updated":"2022-12-13 11:45:07.000000000","message":"Ack","commit_id":"9e2fdb4470603bf3c4f0d21560cced1eb748cd1b"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"551a4ecb0a2d920e7922047e12c98b9d951c0ce3","unresolved":true,"context_lines":[{"line_number":11042,"context_line":"        # add one to the max_retries so announce_self will get called if"},{"line_number":11043,"context_line":"        # retries \u003d\u003d 0"},{"line_number":11044,"context_line":"        max_attempts \u003d ("},{"line_number":11045,"context_line":"            CONF.workarounds.enable_qemu_monitor_announce_max_retries + 1)"},{"line_number":11046,"context_line":"        # enable_qemu_monitor_announce_retry_interval specified in seconds"},{"line_number":11047,"context_line":"        announce_pause \u003d ("},{"line_number":11048,"context_line":"            CONF.workarounds.enable_qemu_monitor_announce_retry_interval)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9414532b_23f82674","line":11045,"range":{"start_line":11045,"start_character":29,"end_line":11045,"end_character":69},"updated":"2022-12-13 10:27:04.000000000","message":"It\u0027s not really a \"max_retries\" It\u0027s more like the number of RARP we want QEMU to send, right?","commit_id":"d01e0b5b93f04e56aa017d4c17a14f270a17b839"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"8e35e31b848b7ba4a0ab8fdf4a13859606c2c07b","unresolved":true,"context_lines":[{"line_number":11042,"context_line":"        # add one to the max_retries so announce_self will get called if"},{"line_number":11043,"context_line":"        # retries \u003d\u003d 0"},{"line_number":11044,"context_line":"        max_attempts \u003d ("},{"line_number":11045,"context_line":"            CONF.workarounds.enable_qemu_monitor_announce_max_retries + 1)"},{"line_number":11046,"context_line":"        # enable_qemu_monitor_announce_retry_interval specified in seconds"},{"line_number":11047,"context_line":"        announce_pause \u003d ("},{"line_number":11048,"context_line":"            CONF.workarounds.enable_qemu_monitor_announce_retry_interval)"}],"source_content_type":"text/x-python","patch_set":1,"id":"a97c1021_d2b7c765","line":11045,"range":{"start_line":11045,"start_character":29,"end_line":11045,"end_character":69},"in_reply_to":"9414532b_23f82674","updated":"2022-12-13 11:45:07.000000000","message":"Strictly speaking it is the number of times the qemu monitor command announce_self is called which in turn triggers a batch of 5 garps to be triggered by qemu\n\nI\u0027m happy to rename this if there is a more suitable name though I\u0027m not sure I can think of one at the moment","commit_id":"d01e0b5b93f04e56aa017d4c17a14f270a17b839"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"70e063224bf67377374bc26bcd5e76a911f3e9ce","unresolved":false,"context_lines":[{"line_number":11042,"context_line":"        # add one to the max_retries so announce_self will get called if"},{"line_number":11043,"context_line":"        # retries \u003d\u003d 0"},{"line_number":11044,"context_line":"        max_attempts \u003d ("},{"line_number":11045,"context_line":"            CONF.workarounds.enable_qemu_monitor_announce_max_retries + 1)"},{"line_number":11046,"context_line":"        # enable_qemu_monitor_announce_retry_interval specified in seconds"},{"line_number":11047,"context_line":"        announce_pause \u003d ("},{"line_number":11048,"context_line":"            CONF.workarounds.enable_qemu_monitor_announce_retry_interval)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bc3bb1c6_2a6bac34","line":11045,"range":{"start_line":11045,"start_character":29,"end_line":11045,"end_character":69},"in_reply_to":"a97c1021_d2b7c765","updated":"2022-12-22 12:19:34.000000000","message":"I believe this name is descriptive enough, given it is retrying the qemu monitor command. \n\nHappy to hear suggestions on alternatives but I will mark this as resolved.","commit_id":"d01e0b5b93f04e56aa017d4c17a14f270a17b839"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"551a4ecb0a2d920e7922047e12c98b9d951c0ce3","unresolved":true,"context_lines":[{"line_number":11049,"context_line":""},{"line_number":11050,"context_line":"        while(current_attempt \u003c max_attempts):"},{"line_number":11051,"context_line":"            current_attempt +\u003d 1"},{"line_number":11052,"context_line":"            time.sleep(announce_pause)"},{"line_number":11053,"context_line":"            LOG.info(\u0027Sending announce-self command to QEMU monitor. \u0027"},{"line_number":11054,"context_line":"                     \u0027Attempt %(current_attempt)s of %(max_attempts)s\u0027,"},{"line_number":11055,"context_line":"                     {\u0027current_attempt\u0027: current_attempt,"}],"source_content_type":"text/x-python","patch_set":1,"id":"adac1e2d_07196005","line":11052,"updated":"2022-12-13 10:27:04.000000000","message":"Using time.sleep would block the whole main thread which would not allow greenthreads to be scheduled?","commit_id":"d01e0b5b93f04e56aa017d4c17a14f270a17b839"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"70e063224bf67377374bc26bcd5e76a911f3e9ce","unresolved":false,"context_lines":[{"line_number":11049,"context_line":""},{"line_number":11050,"context_line":"        while(current_attempt \u003c max_attempts):"},{"line_number":11051,"context_line":"            current_attempt +\u003d 1"},{"line_number":11052,"context_line":"            time.sleep(announce_pause)"},{"line_number":11053,"context_line":"            LOG.info(\u0027Sending announce-self command to QEMU monitor. \u0027"},{"line_number":11054,"context_line":"                     \u0027Attempt %(current_attempt)s of %(max_attempts)s\u0027,"},{"line_number":11055,"context_line":"                     {\u0027current_attempt\u0027: current_attempt,"}],"source_content_type":"text/x-python","patch_set":1,"id":"bb42a151_53e4b43b","line":11052,"in_reply_to":"6bfc32ad_f01bc65b","updated":"2022-12-22 12:19:34.000000000","message":"greenthread.sleep has been used instead:\nhttps://review.opendev.org/c/openstack/nova/+/867324/2/nova/virt/libvirt/driver.py#11052","commit_id":"d01e0b5b93f04e56aa017d4c17a14f270a17b839"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"8e35e31b848b7ba4a0ab8fdf4a13859606c2c07b","unresolved":true,"context_lines":[{"line_number":11049,"context_line":""},{"line_number":11050,"context_line":"        while(current_attempt \u003c max_attempts):"},{"line_number":11051,"context_line":"            current_attempt +\u003d 1"},{"line_number":11052,"context_line":"            time.sleep(announce_pause)"},{"line_number":11053,"context_line":"            LOG.info(\u0027Sending announce-self command to QEMU monitor. \u0027"},{"line_number":11054,"context_line":"                     \u0027Attempt %(current_attempt)s of %(max_attempts)s\u0027,"},{"line_number":11055,"context_line":"                     {\u0027current_attempt\u0027: current_attempt,"}],"source_content_type":"text/x-python","patch_set":1,"id":"6bfc32ad_f01bc65b","line":11052,"in_reply_to":"adac1e2d_07196005","updated":"2022-12-13 11:45:07.000000000","message":"I did wonder how this may best be achieved - I noticed there are a number of other instances using `time.sleep` in the libvirt driver code.\n\nI discussed this with a colleague and it was believe this approach would be OK, however it may warrant more thought","commit_id":"d01e0b5b93f04e56aa017d4c17a14f270a17b839"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"551a4ecb0a2d920e7922047e12c98b9d951c0ce3","unresolved":true,"context_lines":[{"line_number":11050,"context_line":"        while(current_attempt \u003c max_attempts):"},{"line_number":11051,"context_line":"            current_attempt +\u003d 1"},{"line_number":11052,"context_line":"            time.sleep(announce_pause)"},{"line_number":11053,"context_line":"            LOG.info(\u0027Sending announce-self command to QEMU monitor. \u0027"},{"line_number":11054,"context_line":"                     \u0027Attempt %(current_attempt)s of %(max_attempts)s\u0027,"},{"line_number":11055,"context_line":"                     {\u0027current_attempt\u0027: current_attempt,"},{"line_number":11056,"context_line":"                      \u0027max_attempts\u0027: max_attempts}, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":1,"id":"33ba7c89_5016a50e","line":11053,"updated":"2022-12-13 10:27:04.000000000","message":"Perhaps having this log in debug would be enough.","commit_id":"d01e0b5b93f04e56aa017d4c17a14f270a17b839"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"70e063224bf67377374bc26bcd5e76a911f3e9ce","unresolved":false,"context_lines":[{"line_number":11050,"context_line":"        while(current_attempt \u003c max_attempts):"},{"line_number":11051,"context_line":"            current_attempt +\u003d 1"},{"line_number":11052,"context_line":"            time.sleep(announce_pause)"},{"line_number":11053,"context_line":"            LOG.info(\u0027Sending announce-self command to QEMU monitor. \u0027"},{"line_number":11054,"context_line":"                     \u0027Attempt %(current_attempt)s of %(max_attempts)s\u0027,"},{"line_number":11055,"context_line":"                     {\u0027current_attempt\u0027: current_attempt,"},{"line_number":11056,"context_line":"                      \u0027max_attempts\u0027: max_attempts}, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":1,"id":"d3e065d5_222ecc72","line":11053,"in_reply_to":"2570faba_fa43b517","updated":"2022-12-22 12:19:34.000000000","message":"I have given this further thought and think it seems reasonable to proceed with it as an info message for a number of reasons:\n\n* this log will only ever appear if the initial `enable_qemu_monitor_announce_self` workaround is enabled and the original behaviour for this was to log at the info level\n\n* multiple log entries will only appear if the retry is subsequently configured to be \u003e1\n\nThis means unless an operator specifically desires multiple retries and configures it, the number of logs generated would be the same as the already merged behaviour (one line)","commit_id":"d01e0b5b93f04e56aa017d4c17a14f270a17b839"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"8e35e31b848b7ba4a0ab8fdf4a13859606c2c07b","unresolved":true,"context_lines":[{"line_number":11050,"context_line":"        while(current_attempt \u003c max_attempts):"},{"line_number":11051,"context_line":"            current_attempt +\u003d 1"},{"line_number":11052,"context_line":"            time.sleep(announce_pause)"},{"line_number":11053,"context_line":"            LOG.info(\u0027Sending announce-self command to QEMU monitor. \u0027"},{"line_number":11054,"context_line":"                     \u0027Attempt %(current_attempt)s of %(max_attempts)s\u0027,"},{"line_number":11055,"context_line":"                     {\u0027current_attempt\u0027: current_attempt,"},{"line_number":11056,"context_line":"                      \u0027max_attempts\u0027: max_attempts}, instance\u003dinstance)"}],"source_content_type":"text/x-python","patch_set":1,"id":"2570faba_fa43b517","line":11053,"in_reply_to":"33ba7c89_5016a50e","updated":"2022-12-13 11:45:07.000000000","message":"In our particular environment we wanted to be able to check that these were being triggered, however that seems valid to reduce log-spam so I\u0027ll amend to debug 😊","commit_id":"d01e0b5b93f04e56aa017d4c17a14f270a17b839"},{"author":{"_account_id":16137,"name":"Tobias Urdin","email":"tobias.urdin@binero.com","username":"tobasco"},"change_message_id":"6e43c65a207834fc28802266b19486a7d1305a90","unresolved":true,"context_lines":[{"line_number":11049,"context_line":""},{"line_number":11050,"context_line":"        while(current_attempt \u003c max_attempts):"},{"line_number":11051,"context_line":"            current_attempt +\u003d 1"},{"line_number":11052,"context_line":"            greenthread.sleep(announce_pause)"},{"line_number":11053,"context_line":"            LOG.info(\u0027Sending announce-self command to QEMU monitor. \u0027"},{"line_number":11054,"context_line":"                     \u0027Attempt %(current_attempt)s of %(max_attempts)s\u0027,"},{"line_number":11055,"context_line":"                     {\u0027current_attempt\u0027: current_attempt,"}],"source_content_type":"text/x-python","patch_set":2,"id":"1f2ddeb0_3a7ca050","line":11052,"range":{"start_line":11052,"start_character":12,"end_line":11052,"end_character":45},"updated":"2022-12-22 14:05:42.000000000","message":"This effectively adds one second where connectivity is affected after a live migration. Do you this after a announce_self() call instead?\n\nOther than that I guess this should be fine as the VM is already unpaused when this code is executed, but perhaps there is already some helper function available instead of doing the while loop here.","commit_id":"7ccba24b7ad19b1bc348de1de6d25af7f78303f9"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"2865c7d9d9b16fc3ed1347e2e611528db5503f1d","unresolved":false,"context_lines":[{"line_number":11049,"context_line":""},{"line_number":11050,"context_line":"        while(current_attempt \u003c max_attempts):"},{"line_number":11051,"context_line":"            current_attempt +\u003d 1"},{"line_number":11052,"context_line":"            greenthread.sleep(announce_pause)"},{"line_number":11053,"context_line":"            LOG.info(\u0027Sending announce-self command to QEMU monitor. \u0027"},{"line_number":11054,"context_line":"                     \u0027Attempt %(current_attempt)s of %(max_attempts)s\u0027,"},{"line_number":11055,"context_line":"                     {\u0027current_attempt\u0027: current_attempt,"}],"source_content_type":"text/x-python","patch_set":2,"id":"7e29dc8e_6d59d470","line":11052,"range":{"start_line":11052,"start_character":12,"end_line":11052,"end_character":45},"in_reply_to":"1f2ddeb0_3a7ca050","updated":"2022-12-22 16:18:21.000000000","message":"Thanks for your review.\n\nGood spot, thanks. I have added a conditional where it will only perform a pause after the first attempt.","commit_id":"7ccba24b7ad19b1bc348de1de6d25af7f78303f9"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"e6c492c2deb57c87fd25ff9bfc351810b92b6a85","unresolved":true,"context_lines":[{"line_number":11042,"context_line":"        # add one to the max_retries so announce_self will get called if"},{"line_number":11043,"context_line":"        # retries \u003d\u003d 0"},{"line_number":11044,"context_line":"        max_attempts \u003d ("},{"line_number":11045,"context_line":"            CONF.workarounds.enable_qemu_monitor_announce_max_retries + 1)"},{"line_number":11046,"context_line":"        # enable_qemu_monitor_announce_retry_interval specified in seconds"},{"line_number":11047,"context_line":"        announce_pause \u003d ("},{"line_number":11048,"context_line":"            CONF.workarounds.enable_qemu_monitor_announce_retry_interval)"}],"source_content_type":"text/x-python","patch_set":8,"id":"e634544d_4bc8ba2b","line":11045,"updated":"2023-01-02 15:19:54.000000000","message":"The number set should reflect the number of announces.","commit_id":"b65de2174a69c8bb8bf2aebbbeca0dd383a86f93"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"702ebfbc946a064269ad66d54a09ba01a42f8c7c","unresolved":false,"context_lines":[{"line_number":11042,"context_line":"        # add one to the max_retries so announce_self will get called if"},{"line_number":11043,"context_line":"        # retries \u003d\u003d 0"},{"line_number":11044,"context_line":"        max_attempts \u003d ("},{"line_number":11045,"context_line":"            CONF.workarounds.enable_qemu_monitor_announce_max_retries + 1)"},{"line_number":11046,"context_line":"        # enable_qemu_monitor_announce_retry_interval specified in seconds"},{"line_number":11047,"context_line":"        announce_pause \u003d ("},{"line_number":11048,"context_line":"            CONF.workarounds.enable_qemu_monitor_announce_retry_interval)"}],"source_content_type":"text/x-python","patch_set":8,"id":"6e65ca0a_6e4a3202","line":11045,"in_reply_to":"e634544d_4bc8ba2b","updated":"2023-01-06 11:07:13.000000000","message":"Done","commit_id":"b65de2174a69c8bb8bf2aebbbeca0dd383a86f93"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"a99131810aca0a7faaafe380b27a79fa1e77b00c","unresolved":true,"context_lines":[{"line_number":11027,"context_line":""},{"line_number":11028,"context_line":"    def _qemu_monitor_announce_self(self, instance):"},{"line_number":11029,"context_line":"        \"\"\"Send announce_self command to QEMU monitor."},{"line_number":11030,"context_line":""},{"line_number":11031,"context_line":"        This is to trigger generation of broadcast RARP frames to"},{"line_number":11032,"context_line":"        update network switches. This is best effort."},{"line_number":11033,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"077240e1_141efdaf","side":"PARENT","line":11030,"updated":"2023-01-10 09:17:23.000000000","message":"nit: unrelated change","commit_id":"9e2fdb4470603bf3c4f0d21560cced1eb748cd1b"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"973c1c9b1ef428f7b41a3c7a4cf7c2af23865c25","unresolved":false,"context_lines":[{"line_number":11027,"context_line":""},{"line_number":11028,"context_line":"    def _qemu_monitor_announce_self(self, instance):"},{"line_number":11029,"context_line":"        \"\"\"Send announce_self command to QEMU monitor."},{"line_number":11030,"context_line":""},{"line_number":11031,"context_line":"        This is to trigger generation of broadcast RARP frames to"},{"line_number":11032,"context_line":"        update network switches. This is best effort."},{"line_number":11033,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"c2812c89_bf09cae2","side":"PARENT","line":11030,"in_reply_to":"077240e1_141efdaf","updated":"2023-01-11 11:23:16.000000000","message":"Done","commit_id":"9e2fdb4470603bf3c4f0d21560cced1eb748cd1b"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"a99131810aca0a7faaafe380b27a79fa1e77b00c","unresolved":true,"context_lines":[{"line_number":11033,"context_line":"        if not CONF.workarounds.enable_qemu_monitor_announce_self:"},{"line_number":11034,"context_line":"            return"},{"line_number":11035,"context_line":""},{"line_number":11036,"context_line":"        # NOTE(as0) - the announce_self workaround feature has been patched to"},{"line_number":11037,"context_line":"        # allow operators to specify how many times the qemu announce_self"},{"line_number":11038,"context_line":"        # montior command should be called for certain cases where VMs are"},{"line_number":11039,"context_line":"        # inaccessible post live-migration"}],"source_content_type":"text/x-python","patch_set":9,"id":"8c22ed3a_3699f7c4","line":11036,"updated":"2023-01-10 09:17:23.000000000","message":"Instead of this comment, why not to just update the docstring?","commit_id":"8b5f17a52d60640899fbde16b42f86c859cae065"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"973c1c9b1ef428f7b41a3c7a4cf7c2af23865c25","unresolved":false,"context_lines":[{"line_number":11033,"context_line":"        if not CONF.workarounds.enable_qemu_monitor_announce_self:"},{"line_number":11034,"context_line":"            return"},{"line_number":11035,"context_line":""},{"line_number":11036,"context_line":"        # NOTE(as0) - the announce_self workaround feature has been patched to"},{"line_number":11037,"context_line":"        # allow operators to specify how many times the qemu announce_self"},{"line_number":11038,"context_line":"        # montior command should be called for certain cases where VMs are"},{"line_number":11039,"context_line":"        # inaccessible post live-migration"}],"source_content_type":"text/x-python","patch_set":9,"id":"7f94f8bb_7f181cda","line":11036,"in_reply_to":"8c22ed3a_3699f7c4","updated":"2023-01-11 11:23:16.000000000","message":"Removed comment and updated docstring.","commit_id":"8b5f17a52d60640899fbde16b42f86c859cae065"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9ad7de3079e8452369ea48ec9c5388519fa1d8a7","unresolved":true,"context_lines":[{"line_number":11049,"context_line":"            # Only use announce_pause after the first attempt to avoid"},{"line_number":11050,"context_line":"            # pausing before calling announce_self for the first attempt"},{"line_number":11051,"context_line":"            if current_attempt !\u003d 1:"},{"line_number":11052,"context_line":"                greenthread.sleep(announce_pause)"},{"line_number":11053,"context_line":""},{"line_number":11054,"context_line":"            LOG.info(\u0027Sending announce-self command to QEMU monitor. \u0027"},{"line_number":11055,"context_line":"                     \u0027Attempt %(current_attempt)s of %(max_attempts)s\u0027,"}],"source_content_type":"text/x-python","patch_set":16,"id":"86db94ed_a89ea992","line":11052,"updated":"2023-01-24 16:41:00.000000000","message":"ack\nthis will be aproximatly announce_pause before its woken up\nbut it may be longer if we are busy doing something else.\n\ni think that is fine however we don\u0027t need precise interval here.","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"},{"author":{"_account_id":35555,"name":"Aaron S","display_name":"as0","email":"as3310@protonmail.com","username":"as0"},"change_message_id":"f78e94a81e224a89b9002f95f1af7ed20d9f476e","unresolved":false,"context_lines":[{"line_number":11049,"context_line":"            # Only use announce_pause after the first attempt to avoid"},{"line_number":11050,"context_line":"            # pausing before calling announce_self for the first attempt"},{"line_number":11051,"context_line":"            if current_attempt !\u003d 1:"},{"line_number":11052,"context_line":"                greenthread.sleep(announce_pause)"},{"line_number":11053,"context_line":""},{"line_number":11054,"context_line":"            LOG.info(\u0027Sending announce-self command to QEMU monitor. \u0027"},{"line_number":11055,"context_line":"                     \u0027Attempt %(current_attempt)s of %(max_attempts)s\u0027,"}],"source_content_type":"text/x-python","patch_set":16,"id":"f1e986c3_e9fec28a","line":11052,"in_reply_to":"86db94ed_a89ea992","updated":"2023-01-25 09:08:38.000000000","message":"thanks, we aren\u0027t seeking absolute precision here so it should be OK IMO","commit_id":"c804fcce93e09eefcb73193b44e34f0b05941da3"}]}