)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"642973536db4d16c0ffe074bfa796892e6e67853","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8284d6c2_300e50a4","updated":"2022-09-29 01:46:18.000000000","message":"Thank you for highlighting the issue with \u0027disk.info\u0027. I have a couple of comments inline.","commit_id":"1b4f010b33fd498249170bc54e5e68d89f6d5bb4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"f596792ec8ea07c503c943bbb353b3da221ca601","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"23e849ac_2ab2bc7e","updated":"2022-09-14 08:55:51.000000000","message":"This change allow swap shrink and vice versa, also the size of disk.swap after resizing is also consistent.\n\nHowever on swap shrink, even after removing disk.swap, info regarding swap is still present in disk.info under \"data/nova/instances/\u003cinstance-id\u003e/\"\n\nIs it Okay ?","commit_id":"1b4f010b33fd498249170bc54e5e68d89f6d5bb4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"fc1267a53a32a99fd75bb04f8114fe0556689226","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d13947fa_c9c7b557","in_reply_to":"23e849ac_2ab2bc7e","updated":"2022-09-15 06:24:55.000000000","message":"@smooney, @gibizer\nAdding to above doubt,\nThe size of swap is also consistent after reverse resize.\n\nBut still I think, its not really freeing the compute host space.","commit_id":"1b4f010b33fd498249170bc54e5e68d89f6d5bb4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"86d6ffd47f770736d5870f8db028cea26119e958","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e83bc3fc_e0875253","in_reply_to":"d13947fa_c9c7b557","updated":"2023-02-15 18:12:37.000000000","message":"Done","commit_id":"1b4f010b33fd498249170bc54e5e68d89f6d5bb4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"09baddfa6d1f024a0312c36ef29890dd00e6410a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"57191a3e_ceba24da","updated":"2022-09-30 13:48:53.000000000","message":"Thanks melanie, for detail explanation of fix.\n\nI added inline what I found. \nI believe, right now disk.info is just getting re-write, assuming this disk may go to new host, and not updating it and hence we don\u0027t see and any change in content.\n\nAlso w.r.t to freeing swap space in host, I could not see, if used or refereed later anywhere, to update disk size. its just copy it in new host (please correct me if I understood wrong.)","commit_id":"36b9950c3eca15cc79d2930532ba761ea44df31e"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c99c3c2d46aef816cbb196d70d8048a94aa9e39a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"c1a526ce_4d5727bd","updated":"2022-10-13 01:22:07.000000000","message":"Apologies in advance for the long comments ... this issue has turned out to be more complicated than I thought.\n\nI did some more local testing with this because I kept thinking about the block_device_mapping DB table records and eventually found a problem ...\n\nIt\u0027s unfortunately not going to be enough to only exclude the swap BDM after the resize. Because the XML is regenerated in part from the BDM records in the DB, if these are not kept in sync with the current instance flavor, it can cause the libvirt driver to fail when generating the XML and then the instance goes into ERROR state and is unusable.\n\nTo reproduce the ERROR:\n\n1. Boot an instance with swap\n2. Resize to flavor with no swap and confirm resize\n3. Hard reboot the instance (openstack server reboot --hard \u003cinstance\u003e)\n4. Instance is now in ERROR state and it is not running\n\nThis ^ scenario is IMHO worse than the current behavior because with the current behavior the user gets an ERROR immediately as opposed to a potentially long time later when the user reboots or stop/starts the instance. For that reason, I think we need to handle the BDM DB records in this patch. I can help with this if you prefer as this problem seems keep getting bigger.\n\nMy -1 this time is because IMHO the potential long delay to an instance in ERROR state is a worse user experience than having it fail immediately.\n\nI will add some inline comments with my thoughts.","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"2d637dfbb6f8bb044e09838e6ec19166b26dd102","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"0ccd30f5_f7cb85b2","updated":"2022-10-11 07:18:13.000000000","message":"The change itself looks good but it\u0027s non-ideal that the func tests don\u0027t fail without the fix in place. Currently whether we fix the bug or not, these tests will pass.\n\nIt may be the case that there isn\u0027t a way to reproduce the behavior in the func test environment. I think it would be worthwhile to look into it a little. I\u0027m thinking about nova/tests/functional/libvirt/. I will check that later to see if anything jumps out.\n\nI did remember though that you have a good tempest test proposed for this [1], so it\u0027s good that it\u0027s not critical that we have func tests that repro the bug. I\u0027m just not sure how much value the currently proposed tests bring.\n\n[1] https://review.opendev.org/c/openstack/tempest/+/858885","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dfb3f2ad688ee94738b9eabf49a3ba6ac91b7fca","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"37ab698c_1315955c","in_reply_to":"c1a526ce_4d5727bd","updated":"2022-10-19 09:45:37.000000000","message":"i agree that is worse.\n\nalso agree we need to keep the db in sync correcly.","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ca1eca814d863519f889128366fa3d8a5ba7e20","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"6b82eb2f_de432d90","updated":"2023-02-13 21:08:00.000000000","message":"To summarize my -1, I think we need to:\n\n1) Remove the changes to the DefaultFlavorsFixture and instead create the flavors we need in the tests themselves. Doing this will eliminate all of the API samples changes and objects test changes\n\n2) Add verification of DB BDM records -- we want tests that will fail without the bug fix and pass with the bug fix\n\n3) Do the BDM update/sync logic in _finish_resize instead of _confirm_resize in order to keep consistency between the instance XML and the BDMs\n\n4) Add rollback logic to revert resize to undo any BDM record updates we may have made earlier in _finish_resize","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8810e68e8806e117e64ee8861c5cdefa7cb6f0c1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"1de69683_2492b6b7","updated":"2023-02-22 03:53:04.000000000","message":"This is looking really good now. A couple of small comments inline.","commit_id":"a93f15f8ed606a0b311f63250509e5c1bf32fdc8"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"d26fa327cac0bc785ac54216ccf4e8cfd9d045be","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"d4996562_bea0895d","updated":"2023-02-21 04:43:19.000000000","message":"recheck resize test failed while retrieving instance updated info from DB, on confirm resize.\n\nSame test passed in last tox run. \nLOG:\nsqlite3.InterfaceError: Cursor needed to be reset because of commit/rollback and can no longer be fetched from.\n2023-02-21 03:47:25,147 ERROR [nova.compute.manager] Confirm resize failed on source host host1. Resource allocations in the placement service will be removed regardless because the instance is now on the destination host host2. You can try hard rebooting the instance to correct its state.\nTraceback (most recent call last):\n  File \"/home/zuul/src/opendev.org/openstack/nova/.tox/functional-py310/lib/python3.10/site-packages/sqlalchemy/engine/cursor.py\", line 977, in fetchall\n    rows \u003d dbapi_cursor.fetchall()\nsqlite3.InterfaceError: Cursor needed to be reset because of commit/rollback and can no longer be fetched from.\n","commit_id":"a93f15f8ed606a0b311f63250509e5c1bf32fdc8"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"7f92f445e189382fe80804c51472f964c637bf11","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"1eab2164_90e35110","updated":"2023-02-21 05:38:47.000000000","message":"recheck test_resize_servers_with_mlx5","commit_id":"a93f15f8ed606a0b311f63250509e5c1bf32fdc8"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"42c5b45b3a90f8e91c52d89be16bb1266df7fc3b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"6b9d7acd_08ecc7c2","updated":"2023-02-21 03:34:42.000000000","message":"recheck timeout to attach an interface for tempest test (unrelated)","commit_id":"a93f15f8ed606a0b311f63250509e5c1bf32fdc8"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1d36a72c2c2b581d4723a802bc36742877c2ada9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"068405fb_e656d797","updated":"2023-03-02 21:34:48.000000000","message":"Unless I\u0027ve missed something, I think the newest change in nova/compute/manager.py has introduced a bug.\n\nI would start by adding the checks for the cases to your tests to 1) add coverage and 2) verify whether the bug I think I see is actually there. And then if those checks fail, update the code in manager.py to handle those cases.","commit_id":"9163a1d0bd0b69b69f0410a515bfcb58a0e26079"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"1cf0eb0a1ac78f84a240103dcf275e081e0ba58f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"c37f50ef_b7090239","updated":"2023-02-24 13:10:23.000000000","message":"recheck Connection via SSH timed out","commit_id":"9163a1d0bd0b69b69f0410a515bfcb58a0e26079"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"ecde943f0bc031a639cb55f0eb991e78b34bb91f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"d2c6f072_3a1c0a37","updated":"2023-02-23 09:25:41.000000000","message":"recheck tempest test test_volume_boot_pattern failed unrelated","commit_id":"9163a1d0bd0b69b69f0410a515bfcb58a0e26079"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"84ff0b8ad98853fe5ec121eb2fe84f1d21ab146d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"458794b8_42edc275","updated":"2023-02-24 05:35:35.000000000","message":"recheck unrelated fail","commit_id":"9163a1d0bd0b69b69f0410a515bfcb58a0e26079"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"8fba0db1e38f8a7a0068318c060b4ced15820f06","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"b8edba11_7f7e7b1e","updated":"2023-02-24 10:34:08.000000000","message":"recheck unrelated fail","commit_id":"9163a1d0bd0b69b69f0410a515bfcb58a0e26079"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a342b7508c17403ea3dda9952efdb0b72cf68767","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"a3da4798_dc0bf6bd","updated":"2023-03-16 00:10:53.000000000","message":"soft -1 to request adding one more assert for the number of swap BDMs (should only be one ever)\n\nChange itself seems good, I did a little testing with a local devstack and resizes with and without swap work correctly.","commit_id":"fb65f941aa61b23af936641741289457d8b4a798"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"ccc44d5d2547183b13d8df750d8ddf84895a5455","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"ee82e941_71eb9f9c","updated":"2023-03-16 10:18:45.000000000","message":"recheck snapshot test failed (unrelated)","commit_id":"5e9011ae07757ad7eae1de4699b2805c07305d05"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"3fa6ed7aeefcc87727fe65fc92be0080b3197a39","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"6f7c764f_4a08de23","updated":"2023-03-16 16:55:35.000000000","message":"recheck tempest basic ops server test ssh timeout (unrelated)","commit_id":"5e9011ae07757ad7eae1de4699b2805c07305d05"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e1742a408581c589e173765c514d25bdf37e9698","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"44d83f0f_68e272d4","updated":"2023-04-25 21:24:18.000000000","message":"Fair points by the other reviewers. The changes should be relatively simple to make.","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cc154cf01de3a5fd67aa20ad07ac5a0c20a45b78","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"1a98ec96_f5f953a2","updated":"2023-04-25 15:12:30.000000000","message":"I think the hasattr usage needs to change. TBH, I\u0027m not sure if that\u0027s really working has hasattr() should always be true for any managed field on an o.vo, unless I\u0027m forgetting something.\n\nI also think not re-querying the list we already have and unifying the logic like sylvain suggested is worth doing.","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"439c7c4a450c3486fa2f16ec7597c163e3f201b7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"fa597eca_8751adbc","updated":"2023-03-22 23:15:03.000000000","message":"Latest updates look good, thanks Amit!\n\nI\u0027m probably too heavily involved to +2 this now so I give this a strong +1 instead","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b7bbc77b458f49d775ffbe57352bf0de9f6b6b2a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"4e70d54b_d610c56e","updated":"2023-04-06 09:43:15.000000000","message":"it\u0027s long overdue, let\u0027s have it instead of bikeshedding. Thanks Amit for the hard work and thanks Mel for helping.","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"70dc29ec83a7d75fa9722d2aec8f894ad09369bc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"40082ae0_34b531a1","updated":"2023-03-20 13:28:08.000000000","message":"recheck volume boot pattern test failed (unrelated)","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b406bb455757a6365143538d64313b3a51e70911","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"6a8a78e7_1a83846f","updated":"2023-06-12 18:08:47.000000000","message":"I think something got confused in the conversation here...","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9e0c6dc7914c296478979166166ac609bace0132","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":26,"id":"8d434470_54c8ac9e","updated":"2023-06-12 18:29:13.000000000","message":"i find the logic in the if kind of had to follow\n\nits not would be nice if you labled each branch with the operations that it is fixign\n\ni.e.\n# resize  confrim  from swap to no swap\n# resize  confrim  from no swap to swap\n# revert  resize from swap to no swap\n# revert  resoze from no swap to swap\n\n\ni have suggested some refactorigns inline but basically i would early exit if neither flavor has swap and return the bdm un modifed.\n\ni dont have time this eventing ot check but i also wonder if \n\n# resize  confrim  from swap to swap (with differnt size)\n# revert resize  from swap to swap (with differnt size)\n\nneed to be covered or does that already work.\n\nlooking at just _update_bdm_on_swap_resize it does not handle those cases and the bdm are not updated.\n\n\ncan you point to where that is handeled today.","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"6382f92d9898a479b9d4e4f8485adc7e95e178cd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"05680a0f_6dd0047a","updated":"2023-05-10 08:51:40.000000000","message":"recheck timout on nova-ceph-multistore (unrelated )\n\ncmd\ncoredumpctl -o /opt/stack/logs//qemu.coredump dump /usr/bin/qemu-system-x86_64\nmsg\nnon-zero return code\nstdout\n/bin/bash: line 1: coredumpctl: command not found","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"48cc3edf303150ba3a83c39584f038056493d45f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":26,"id":"2e6590bb_dd8e6ab3","in_reply_to":"2c6fc65c_a36009c0","updated":"2023-07-19 07:59:09.000000000","message":"Done","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"7becee98ffbcd1cb076b8c2ae403c16f2583bc1b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":26,"id":"2c6fc65c_a36009c0","in_reply_to":"8d434470_54c8ac9e","updated":"2023-06-12 18:31:51.000000000","message":"by the way i dont have the context of this code loaded and i was not following the converstation between you dan and other previously but somethign feels off with this change but im too tire to review this properly this evening .","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c9617dd70ed2de7809ed0b95c8af0e05ccce2f00","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":27,"id":"d6d40028_1dfe7b9f","updated":"2023-06-13 22:37:07.000000000","message":"I think this has gotten a bit fragile in an attempt to make the code read more simply. Maybe clarity could instead be achieved by using \"helper\" variables, an example of what I mean:\n```\nswap_is_being_added \u003d not old_flavor.swap and new_flavor.swap\nswap_is_being_removed \u003d old_flavor.swap and not new_flavor.swap\n\nif swap_is_being_added or swap_is_being_removed:\n    updated_bdms \u003d self._update_bdm_on_swap_resize(...)\n```\n\nIMHO I also think it would be worthwhile to try again to avoid the query for a fresh BDM list from the database. If we make sure not to create or delete a BDM record unless we need to, I don\u0027t see why doing the list update in memory would not work?","commit_id":"a67e2908f21b2bf847d8e6c74f16ae4637262be6"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"dbf53a3a5eb6d8aa2ca44579300eead4635fba83","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":29,"id":"51d8055f_8ff71f58","updated":"2023-07-17 13:02:04.000000000","message":"recheck tempest test failed with SSH timeout unrelated","commit_id":"8aea4929970d8016a5f6b8eb26fce901d9ec6589"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"b2f2a65a1f743c35c1441e2f3129abffa744b73a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":30,"id":"17cf87ee_15347f5b","updated":"2023-08-22 05:30:45.000000000","message":"recheck \ncold migation test `test_cold_migrate_same_host_old_compute_disallow` failed with AssertionError: Notification compute_task.migrate_server.error hasn\u0027t been received. Received:\n\n`because` Failed to compute_task_migrate_server: No valid host was found. There are not enough hosts available.","commit_id":"83ff8854a641cd6be9cc8421fef7a140f5fdb4d2"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"69273d1821db43689993279284e071da3aecd4d9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":30,"id":"8d0da7f3_41d617ff","updated":"2023-08-18 05:10:46.000000000","message":"recheck placement test test_resource_provider_inventory failed unrelated","commit_id":"83ff8854a641cd6be9cc8421fef7a140f5fdb4d2"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e9eb3edc70dd57f0e460b5f01f8a482d6103bf70","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":31,"id":"66c6d5d1_e466609c","updated":"2023-10-05 12:57:27.000000000","message":"Just stopping after the first conditional. IMHO, you should explain your internal API, telling what exactly you return.","commit_id":"2c1e7adec2c690c86d28cbe85ee725c7c2b3cd60"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"17e35dcb1bb45d989e0de3eec7d8526514cc638e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":32,"id":"b116bbfd_3a37303b","updated":"2024-01-17 05:53:30.000000000","message":"few comments inline","commit_id":"8c4bbe0b7eda5ba2bf2c2cd48de5ecb313190469"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"d30c325b621445e4032e7a1b2bc781eb73884192","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":34,"id":"cceb596d_d7979094","updated":"2024-01-31 06:03:19.000000000","message":"recheck tempest-integrated-compute-rbac-old-defaults  TIME_OUT","commit_id":"d907b3b7415438a0d7d077e58af7208f9f2008f5"}],"nova/compute/manager.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"642973536db4d16c0ffe074bfa796892e6e67853","unresolved":true,"context_lines":[{"line_number":6053,"context_line":"        # refresh_conn_info\u003dTrue will update the BDM.connection_info value"},{"line_number":6054,"context_line":"        # in the database so we must do this before calling that method."},{"line_number":6055,"context_line":"        self._update_volume_attachments(context, instance, bdms)"},{"line_number":6056,"context_line":""},{"line_number":6057,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":6058,"context_line":"            context, instance, refresh_conn_info\u003dTrue, bdms\u003dbdms)"},{"line_number":6059,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"d81cdcfd_aa9d96fd","line":6056,"updated":"2022-09-29 01:46:18.000000000","message":"So far what I have tried and seems to work is something like the following here:\n\n if not instance.new_flavor.swap and instance.old_flavor.swap:\n     bdms \u003d [bdm for bdm in bdms if bdm.guest_format !\u003d \u0027swap\u0027]\n     \nAnd then when \u0027bdms\u0027 is passed to _get_instance_block_device_info() we will not get any \u0027swap\u0027 element in the block_device_info.\n\nPlease feel free to test that in case I\u0027ve missed something. The main point is that we don\u0027t want to pass any \u0027swap\u0027 bdms to the _get_instance_block_device_info() method.\n\nThat aside, I think this results in the same behavior as the current code in this patch, creating a swap disk when the new flavor has swap and not creating it if the flavor has no swap, based on my testing.\n\nIt also has the same behavior of still having the \u0027swap\u0027 entry in the \u0027disk.info\u0027 file after resizing to no swap. I found that there is currently no mechanism for removing any entries from it. As you may have already found, the only code that writes this file at most can update existing entries driver format [1]. I don\u0027t know if it ever hurts anything for stale no-longer-existing entries to remain in \u0027disk.info\u0027 ... Maybe or probably not. But it\u0027s something that would be ideal to fix later.\n\nAnd finally, this has the same behavior of not deleting any block_device_mapping DB records when going from swap to no swap. I found that similarly if you resize from no swap to swap, nova does not create a block_device_mapping DB record for the swap disk. In general, block_device_mapping DB records are never touched as a result of a resize, AFAICT.\n\nAll of this is to say, I don\u0027t think that this patch must address the \u0027disk.info\u0027 and block_device_mapping DB records mismatch. Only fixing the swap create vs no create behavior is the most important part and is an improvement over what we have today.\n\nAddressing \u0027disk.info\u0027 and block_device_mapping records can be separate fixes and they have to consider rolling back changes if something in the resize fails after we make updates. It will take more work.\n\nTo be specific, if we have a mismatch where there is a \u0027swap\u0027 block_device_mapping DB record when the instance should have no swap, I think a swap disk will incorrectly be created during the instance rescue code path. (And anywhere else the LibvirtDriver._create_image() method is called).\n\n[1] https://github.com/openstack/nova/blob/aad31e6ba489f720f5bdc765c132fd0f059a0329/nova/virt/libvirt/imagebackend.py#L385","commit_id":"1b4f010b33fd498249170bc54e5e68d89f6d5bb4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"09baddfa6d1f024a0312c36ef29890dd00e6410a","unresolved":false,"context_lines":[{"line_number":6053,"context_line":"        # refresh_conn_info\u003dTrue will update the BDM.connection_info value"},{"line_number":6054,"context_line":"        # in the database so we must do this before calling that method."},{"line_number":6055,"context_line":"        self._update_volume_attachments(context, instance, bdms)"},{"line_number":6056,"context_line":""},{"line_number":6057,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":6058,"context_line":"            context, instance, refresh_conn_info\u003dTrue, bdms\u003dbdms)"},{"line_number":6059,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"80ba0571_5ed0cfa8","line":6056,"in_reply_to":"d81cdcfd_aa9d96fd","updated":"2022-09-30 13:48:53.000000000","message":"ack,\nregarding disk.info, before control comes to _finish_resize.\nin compute.manager, while resizing_instance [1], control goes to libvirt.driver.migrate_disk_and_poweroff [2].\n\nso exactly in this place it makes a copy (conceptually at new host, I believe, where instance is getting migrated) of whole instance dir [3]\ndata/nova/instance/\u003cinstance-id\u003e, then recreate each file(except disk.swap)\nso disk.info get recreated (for resize updated ) here.\n\n\n[1] https://opendev.org/openstack/nova/src/commit/aad31e6ba489f720f5bdc765c132fd0f059a0329/nova/compute/manager.py#L5874\n[2] https://opendev.org/openstack/nova/src/branch/master/nova/virt/libvirt/driver.py#L11352\n\n[3] https://opendev.org/openstack/nova/src/commit/aad31e6ba489f720f5bdc765c132fd0f059a0329/nova/virt/libvirt/driver.py#L11416-L11448","commit_id":"1b4f010b33fd498249170bc54e5e68d89f6d5bb4"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"642973536db4d16c0ffe074bfa796892e6e67853","unresolved":true,"context_lines":[{"line_number":6069,"context_line":"                !\u003d instance.new_flavor.swap:"},{"line_number":6070,"context_line":"                block_device_info[\u0027swap\u0027][\u0027swap_size\u0027] \\"},{"line_number":6071,"context_line":"                    \u003d instance.new_flavor.swap"},{"line_number":6072,"context_line":""},{"line_number":6073,"context_line":"        # NOTE(mriedem): If the original vm_state was STOPPED, we don\u0027t"},{"line_number":6074,"context_line":"        # automatically power on the instance after it\u0027s migrated"},{"line_number":6075,"context_line":"        power_on \u003d old_vm_state !\u003d vm_states.STOPPED"}],"source_content_type":"text/x-python","patch_set":2,"id":"15afda19_3a323b86","line":6072,"updated":"2022-09-29 01:46:18.000000000","message":"I think this probably isn\u0027t the right way to fix this ... we generally don\u0027t want to modify block_device_info as that\u0027s generated based on the BlockDeviceMapping objects passed to _get_instance_block_device_info().\n\nI think what\u0027s happening here is we have a BlockDeviceMapping record in the database for the swap disk, which is causing us to create a swap disk as we pass the \u0027bdms\u0027 variable to _get_instance_block_device_info().\n\nWe want to exclude the \u0027swap\u0027 block_device_mapping DB records *before* calling _get_instance_block_device_info().","commit_id":"1b4f010b33fd498249170bc54e5e68d89f6d5bb4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"09baddfa6d1f024a0312c36ef29890dd00e6410a","unresolved":false,"context_lines":[{"line_number":6069,"context_line":"                !\u003d instance.new_flavor.swap:"},{"line_number":6070,"context_line":"                block_device_info[\u0027swap\u0027][\u0027swap_size\u0027] \\"},{"line_number":6071,"context_line":"                    \u003d instance.new_flavor.swap"},{"line_number":6072,"context_line":""},{"line_number":6073,"context_line":"        # NOTE(mriedem): If the original vm_state was STOPPED, we don\u0027t"},{"line_number":6074,"context_line":"        # automatically power on the instance after it\u0027s migrated"},{"line_number":6075,"context_line":"        power_on \u003d old_vm_state !\u003d vm_states.STOPPED"}],"source_content_type":"text/x-python","patch_set":2,"id":"b0a84b2c_895b5814","line":6072,"in_reply_to":"15afda19_3a323b86","updated":"2022-09-30 13:48:53.000000000","message":"Ack","commit_id":"1b4f010b33fd498249170bc54e5e68d89f6d5bb4"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c99c3c2d46aef816cbb196d70d8048a94aa9e39a","unresolved":true,"context_lines":[{"line_number":4687,"context_line":"                      instance\u003dinstance)"},{"line_number":4688,"context_line":"        else:"},{"line_number":4689,"context_line":"            vm_state \u003d vm_states.ACTIVE"},{"line_number":4690,"context_line":""},{"line_number":4691,"context_line":"        instance.vm_state \u003d vm_state"},{"line_number":4692,"context_line":"        instance.task_state \u003d None"},{"line_number":4693,"context_line":"        instance.save(expected_task_state\u003d[None, task_states.DELETING,"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a5de3ec_c46a628f","line":4690,"updated":"2022-10-13 01:22:07.000000000","message":"I\u0027m thinking maybe we could sync the BDM records here in the confirm resize path, that way we don\u0027t have to worry about un-deleting a BDM record during a revert. And server reboot is blocked when the instance is in the VERIFY_RESIZE state.\n\nI ended up writing code for this because I wasn\u0027t sure it would work and I didn\u0027t want to tell you something that wouldn\u0027t work:\n\n        # If we resized from swap to no swap or vice versa, we need to \n        # delete or create a BDM record to keep things in sync.\n        # Otherwise, the instance XML will not be generated correctly\n        # during a hard reboot or stop/start.\n        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid(\n            context, instance.uuid)\n        if not instance.new_flavor.swap and instance.old_flavor.swap:\n            swap_bdms \u003d [bdm for bdm in bdms if bdm.guest_format \u003d\u003d \u0027swap\u0027]\n            if swap_bdms:\n                swap_bdms[0].destroy()\n        elif instance.new_flavor.swap and not instance.old_flavor.swap:\n             bdm_dict \u003d block_device.create_blank_bdm(\n                 instance.new_flavor.swap, \u0027swap\u0027)\n             bdm_obj \u003d objects.BlockDeviceMapping(\n                 context, instance_uuid\u003dinstance.uuid, **bdm_dict)\n             bdm_obj.update_or_create()\n\nI put this together based on how we create BDM records during a server create [1][2][3].\n\nI tested locally:\n\n1. Resize from swap to no swap, confirm, then hard reboot\n2. Resize from no swap to swap, confirm, then hard reboot\n3. Resize from swap to no swap, revert, then hard reboot\n4. Resize from no swap to swap, revert, then hard reboot\n\nPlease feel free to do further testing around it, I may have missed something. And we don\u0027t have to fix it this exact way. If you find a nicer way to handle the problem, please feel free to propose it.\n\nI think we should also add a hard reboot to the tempest test to cover this case.\n\n[1] https://github.com/openstack/nova/blob/03d2715ed492350fa11908aea0fdd0265993e284/nova/compute/api.py#L926\n[2] https://github.com/openstack/nova/blob/03d2715ed492350fa11908aea0fdd0265993e284/nova/compute/api.py#L1002\n[3] https://github.com/openstack/nova/blob/03d2715ed492350fa11908aea0fdd0265993e284/nova/conductor/manager.py#L1446","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"20cdac3f1e7a9b8e041ac8829707b6aabe635596","unresolved":true,"context_lines":[{"line_number":4687,"context_line":"                      instance\u003dinstance)"},{"line_number":4688,"context_line":"        else:"},{"line_number":4689,"context_line":"            vm_state \u003d vm_states.ACTIVE"},{"line_number":4690,"context_line":""},{"line_number":4691,"context_line":"        instance.vm_state \u003d vm_state"},{"line_number":4692,"context_line":"        instance.task_state \u003d None"},{"line_number":4693,"context_line":"        instance.save(expected_task_state\u003d[None, task_states.DELETING,"}],"source_content_type":"text/x-python","patch_set":5,"id":"bf0f5afc_e28a1b7c","line":4690,"in_reply_to":"1a5de3ec_c46a628f","updated":"2022-10-19 09:50:39.000000000","message":"this feels kind of late to me i think finsih_resize is more correct as i woudl expect the bdms to be in sync with the xml.\n\nthis is an option i guess but i dont really like that the db and xml dont alighn in the verify_resize state. that said its a transtionary state so maybe its oke.\n\nin verify resize the current flavor is alredy the new flavor in the db which is why i think we shoudl be doing with we do with the falvor and have an old bdm and new bdms but presumable that woudl rewiured a db change and i woudl prefer if we could backport this so if we need to defer this to confirm i guess its ok.\n\n\nany reason not to do this at https://review.opendev.org/c/openstack/nova/+/857339/5/nova/compute/manager.py#6057","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"959e617936560916e0d904d2c621a174e8fd78a5","unresolved":true,"context_lines":[{"line_number":4687,"context_line":"                      instance\u003dinstance)"},{"line_number":4688,"context_line":"        else:"},{"line_number":4689,"context_line":"            vm_state \u003d vm_states.ACTIVE"},{"line_number":4690,"context_line":""},{"line_number":4691,"context_line":"        instance.vm_state \u003d vm_state"},{"line_number":4692,"context_line":"        instance.task_state \u003d None"},{"line_number":4693,"context_line":"        instance.save(expected_task_state\u003d[None, task_states.DELETING,"}],"source_content_type":"text/x-python","patch_set":5,"id":"a7b95b5f_45ee4dee","line":4690,"in_reply_to":"3fa599b5_689fec59","updated":"2022-11-02 12:57:39.000000000","message":"copied above snippet in code as it is, it worked for all new test cases.\nAdded Hard reboot in tempest tests - worked.\n\nbelow 2 unit tests failed on do_confirm_size:\n\nnova.tests.unit.compute.test_compute_mgr.ComputeManagerMigrationTestCase.test_confirm_resize_deletes_allocations_and_update_scheduler\nnova.tests.unit.compute.test_compute_mgr.ComputeManagerMigrationTestCase.test_confirm_resize_calls_virt_driver_with_old_pci\n\n\nwith Exception: This test uses methods that set internal oslo_db state, but it does not claim to use the database. This will conflict with the setup of tests that do use the database and cause failures later.","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"86d6ffd47f770736d5870f8db028cea26119e958","unresolved":false,"context_lines":[{"line_number":4687,"context_line":"                      instance\u003dinstance)"},{"line_number":4688,"context_line":"        else:"},{"line_number":4689,"context_line":"            vm_state \u003d vm_states.ACTIVE"},{"line_number":4690,"context_line":""},{"line_number":4691,"context_line":"        instance.vm_state \u003d vm_state"},{"line_number":4692,"context_line":"        instance.task_state \u003d None"},{"line_number":4693,"context_line":"        instance.save(expected_task_state\u003d[None, task_states.DELETING,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3e55ade3_e2cd4871","line":4690,"in_reply_to":"a7b95b5f_45ee4dee","updated":"2023-02-15 18:12:37.000000000","message":"Ack","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"09980a2bdddb28cca557f7293fc4a6f8c964c31a","unresolved":true,"context_lines":[{"line_number":4687,"context_line":"                      instance\u003dinstance)"},{"line_number":4688,"context_line":"        else:"},{"line_number":4689,"context_line":"            vm_state \u003d vm_states.ACTIVE"},{"line_number":4690,"context_line":""},{"line_number":4691,"context_line":"        instance.vm_state \u003d vm_state"},{"line_number":4692,"context_line":"        instance.task_state \u003d None"},{"line_number":4693,"context_line":"        instance.save(expected_task_state\u003d[None, task_states.DELETING,"}],"source_content_type":"text/x-python","patch_set":5,"id":"3fa599b5_689fec59","line":4690,"in_reply_to":"bf0f5afc_e28a1b7c","updated":"2022-10-19 23:30:43.000000000","message":"That\u0027s fair enough. I thought of resize confirm because it\u0027s the \"easy\" way with no rollback needed for if the user chooses to revert the resize instead of confirming it. And reboot (and other things) are blocked while the instance is in VERIFY_RESIZE state.\n\nBut, if bdms are left temporarily out of sync in general, if nova needed to regenerate the instance XML for some reason, it would not get returned to the same state it was previously in VERIFY_RESIZE. We block reboot or stop/start via vm_state, so I can\u0027t think of a \"normal\" way that situation could happen, off the top of my head. But things could always fail in some other unexpected way (DB errors, connection errors, etc) and if that somehow lead to us needing to regenerate the XML, it would be wrong.\n\nMaybe we could do it in finish_resize and re-create from scratch or re-delete based on the difference from what\u0027s in the database? (This would not reuse the existing soft deleted BDM record but based on the server create code I linked earlier, that should not cause a problem.) It would also not require the addition of Instance.old_bdms/new_bdms.\n\nOverall I agree with you and I think we just have to try/test some things to see if we can make it work without any DB changes.","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"09980a2bdddb28cca557f7293fc4a6f8c964c31a","unresolved":true,"context_lines":[{"line_number":6039,"context_line":"        network_info \u003d self.network_api.get_instance_nw_info(context, instance)"},{"line_number":6040,"context_line":""},{"line_number":6041,"context_line":"        instance.task_state \u003d task_states.RESIZE_FINISH"},{"line_number":6042,"context_line":"        instance.save(expected_task_state\u003dtask_states.RESIZE_MIGRATED)"},{"line_number":6043,"context_line":""},{"line_number":6044,"context_line":"        self._send_finish_resize_notifications("},{"line_number":6045,"context_line":"            context, instance, bdms, network_info,"}],"source_content_type":"text/x-python","patch_set":5,"id":"2cb8590e_53cf2bfa","line":6042,"updated":"2022-10-19 23:30:43.000000000","message":"Note to self: this is where we save the new flavor to instance.flavor in the database.","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"86d6ffd47f770736d5870f8db028cea26119e958","unresolved":false,"context_lines":[{"line_number":6039,"context_line":"        network_info \u003d self.network_api.get_instance_nw_info(context, instance)"},{"line_number":6040,"context_line":""},{"line_number":6041,"context_line":"        instance.task_state \u003d task_states.RESIZE_FINISH"},{"line_number":6042,"context_line":"        instance.save(expected_task_state\u003dtask_states.RESIZE_MIGRATED)"},{"line_number":6043,"context_line":""},{"line_number":6044,"context_line":"        self._send_finish_resize_notifications("},{"line_number":6045,"context_line":"            context, instance, bdms, network_info,"}],"source_content_type":"text/x-python","patch_set":5,"id":"a2a0ddd8_80d46315","line":6042,"in_reply_to":"2cb8590e_53cf2bfa","updated":"2023-02-15 18:12:37.000000000","message":"Ack","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c99c3c2d46aef816cbb196d70d8048a94aa9e39a","unresolved":true,"context_lines":[{"line_number":6059,"context_line":"        if not instance.new_flavor.swap and instance.old_flavor.swap:"},{"line_number":6060,"context_line":"            LOG.info(\"Excluding swap device from block device mapping\")"},{"line_number":6061,"context_line":"            bdms \u003d [bdm for bdm in bdms if bdm.guest_format !\u003d \u0027swap\u0027]"},{"line_number":6062,"context_line":""},{"line_number":6063,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":6064,"context_line":"            context, instance, refresh_conn_info\u003dTrue, bdms\u003dbdms)"},{"line_number":6065,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"41f1c888_116178e5","line":6062,"updated":"2022-10-13 01:22:07.000000000","message":"Something I ran into while testing stuff is I think I made a bad example to overwrite the \u0027bdms\u0027 variable that was passed in here (causing a side effect). So it\u0027s probably best to make a copy, I tried for example:\n\n bdms_copy \u003d bdms\n if not instance.new_flavor.swap and instance.old_flavor.swap:\n     bdms_copy \u003d [bdm for bdm in bdms if bdm.guest_format !\u003d \u0027swap\u0027]\n     \n block_device_info \u003d self._get_instance_block_device_info(\n     context, instance, refresh_conn_info\u003dTrue, bdms\u003dbdms_copy)","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"86d6ffd47f770736d5870f8db028cea26119e958","unresolved":false,"context_lines":[{"line_number":6059,"context_line":"        if not instance.new_flavor.swap and instance.old_flavor.swap:"},{"line_number":6060,"context_line":"            LOG.info(\"Excluding swap device from block device mapping\")"},{"line_number":6061,"context_line":"            bdms \u003d [bdm for bdm in bdms if bdm.guest_format !\u003d \u0027swap\u0027]"},{"line_number":6062,"context_line":""},{"line_number":6063,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":6064,"context_line":"            context, instance, refresh_conn_info\u003dTrue, bdms\u003dbdms)"},{"line_number":6065,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"67cb5c2c_425fbf9e","line":6062,"in_reply_to":"15b274d9_24032834","updated":"2023-02-15 18:12:37.000000000","message":"Ack","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dfb3f2ad688ee94738b9eabf49a3ba6ac91b7fca","unresolved":true,"context_lines":[{"line_number":6059,"context_line":"        if not instance.new_flavor.swap and instance.old_flavor.swap:"},{"line_number":6060,"context_line":"            LOG.info(\"Excluding swap device from block device mapping\")"},{"line_number":6061,"context_line":"            bdms \u003d [bdm for bdm in bdms if bdm.guest_format !\u003d \u0027swap\u0027]"},{"line_number":6062,"context_line":""},{"line_number":6063,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":6064,"context_line":"            context, instance, refresh_conn_info\u003dTrue, bdms\u003dbdms)"},{"line_number":6065,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"50cfa96e_ff74b408","line":6062,"in_reply_to":"41f1c888_116178e5","updated":"2022-10-19 09:45:37.000000000","message":"ya anythime your going to modify the thing your looping over make a copy.\n\ntechnially unlike other lanaguages its not an error to modify the thing your loppoing on in python but its not guarteed to do what you want.\n\nwith that said in this particalyar case i would not have excpected it to cause an error as the dict comprehention on the right should have created a copy beofre the asignment happended so thos e shoudl be equivlent if the orginal value of bdms is not required.","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"09980a2bdddb28cca557f7293fc4a6f8c964c31a","unresolved":true,"context_lines":[{"line_number":6059,"context_line":"        if not instance.new_flavor.swap and instance.old_flavor.swap:"},{"line_number":6060,"context_line":"            LOG.info(\"Excluding swap device from block device mapping\")"},{"line_number":6061,"context_line":"            bdms \u003d [bdm for bdm in bdms if bdm.guest_format !\u003d \u0027swap\u0027]"},{"line_number":6062,"context_line":""},{"line_number":6063,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":6064,"context_line":"            context, instance, refresh_conn_info\u003dTrue, bdms\u003dbdms)"},{"line_number":6065,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"15b274d9_24032834","line":6062,"in_reply_to":"50cfa96e_ff74b408","updated":"2022-10-19 23:30:43.000000000","message":"To be clear, it didn\u0027t cause any error ... what happened was when I was trying stuff I initially tried syncing the bdm records in finish_resize above. And to do that, I needed the original list of bdms that did not exclude the swap disk. But since \u0027bdms\u0027 had been overwritten, I no longer had the original list available.\n\nThat made me think in general, even if it doesn\u0027t cause an error now, probably best not to do it anyway... the next developer might get thrown off by it if they needed to add some handling of the bdms later on.","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c99c3c2d46aef816cbb196d70d8048a94aa9e39a","unresolved":true,"context_lines":[{"line_number":6061,"context_line":"            bdms \u003d [bdm for bdm in bdms if bdm.guest_format !\u003d \u0027swap\u0027]"},{"line_number":6062,"context_line":""},{"line_number":6063,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":6064,"context_line":"            context, instance, refresh_conn_info\u003dTrue, bdms\u003dbdms)"},{"line_number":6065,"context_line":""},{"line_number":6066,"context_line":"        # NOTE(mriedem): If the original vm_state was STOPPED, we don\u0027t"},{"line_number":6067,"context_line":"        # automatically power on the instance after it\u0027s migrated"}],"source_content_type":"text/x-python","patch_set":5,"id":"71451370_54230f5d","line":6064,"updated":"2022-10-13 01:22:07.000000000","message":"Something else I noticed while testing is that even after I started syncing the DB records (and thus the bdms here) I didn\u0027t get a wrong result going from no swap to swap (and not adding a new BDM until after resize confirm) ...\n\nI found it\u0027s because in some places, the driver is looking only at the *flavor* to make decisions about swap [1]. So even though we don\u0027t pass a swap BDM, our swap disk gets created anyway if the flavor has it.\n\nWhen I realized this, I wondered if we should do something in the driver to remove swap based on the flavor only ... but according to a code comment [2], using the flavor for this is not ideal and that we should be going toward using only BDMs for dealing with swap. I think adding more flavor code in the driver might add debt, so I\u0027m thinking maybe a simple approach with the BDMs is better for now. If anyone disagrees, please add a comment.\n\nMoving everything to BDM-only in the future will be its own refactoring project.\n\n[1] https://github.com/openstack/nova/blob/03d2715ed492350fa11908aea0fdd0265993e284/nova/virt/libvirt/blockinfo.py#L636-L641\n[2] https://github.com/openstack/nova/blob/03d2715ed492350fa11908aea0fdd0265993e284/nova/virt/libvirt/driver.py#L4687-L4696","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"86d6ffd47f770736d5870f8db028cea26119e958","unresolved":false,"context_lines":[{"line_number":6061,"context_line":"            bdms \u003d [bdm for bdm in bdms if bdm.guest_format !\u003d \u0027swap\u0027]"},{"line_number":6062,"context_line":""},{"line_number":6063,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":6064,"context_line":"            context, instance, refresh_conn_info\u003dTrue, bdms\u003dbdms)"},{"line_number":6065,"context_line":""},{"line_number":6066,"context_line":"        # NOTE(mriedem): If the original vm_state was STOPPED, we don\u0027t"},{"line_number":6067,"context_line":"        # automatically power on the instance after it\u0027s migrated"}],"source_content_type":"text/x-python","patch_set":5,"id":"d845d745_285028e7","line":6064,"in_reply_to":"5855e7f5_28bb5b45","updated":"2023-02-15 18:12:37.000000000","message":"Ack","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dfb3f2ad688ee94738b9eabf49a3ba6ac91b7fca","unresolved":true,"context_lines":[{"line_number":6061,"context_line":"            bdms \u003d [bdm for bdm in bdms if bdm.guest_format !\u003d \u0027swap\u0027]"},{"line_number":6062,"context_line":""},{"line_number":6063,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":6064,"context_line":"            context, instance, refresh_conn_info\u003dTrue, bdms\u003dbdms)"},{"line_number":6065,"context_line":""},{"line_number":6066,"context_line":"        # NOTE(mriedem): If the original vm_state was STOPPED, we don\u0027t"},{"line_number":6067,"context_line":"        # automatically power on the instance after it\u0027s migrated"}],"source_content_type":"text/x-python","patch_set":5,"id":"5855e7f5_28bb5b45","line":6064,"in_reply_to":"71451370_54230f5d","updated":"2022-10-19 09:45:37.000000000","message":"honestly i think we need to udpate this in resize_verify not confrim and we likely want to store teh old bdms until we confrim or revert too.","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ca1eca814d863519f889128366fa3d8a5ba7e20","unresolved":true,"context_lines":[{"line_number":4802,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":4803,"context_line":"                context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":4804,"context_line":"            bdm_obj.update_or_create()"},{"line_number":4805,"context_line":""},{"line_number":4806,"context_line":"        instance.vm_state \u003d vm_state"},{"line_number":4807,"context_line":"        instance.task_state \u003d None"},{"line_number":4808,"context_line":"        instance.save(expected_task_state\u003d[None, task_states.DELETING,"}],"source_content_type":"text/x-python","patch_set":8,"id":"1c05831f_0a71ae6f","line":4805,"updated":"2023-02-13 21:08:00.000000000","message":"Per the discussion above, we want to move the BDM update logic into _finish_resize instead of here in _confirm_resize. Note that you also need to undo the BDM updates if the user does a revert resize. You should be able to compare the flavor similarly to find whether to create or delete a BDM record during rollback.","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"b73659ea76c3e7e40004f2d302c39c90aa3cd6ab","unresolved":false,"context_lines":[{"line_number":4802,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":4803,"context_line":"                context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":4804,"context_line":"            bdm_obj.update_or_create()"},{"line_number":4805,"context_line":""},{"line_number":4806,"context_line":"        instance.vm_state \u003d vm_state"},{"line_number":4807,"context_line":"        instance.task_state \u003d None"},{"line_number":4808,"context_line":"        instance.save(expected_task_state\u003d[None, task_states.DELETING,"}],"source_content_type":"text/x-python","patch_set":8,"id":"22bbe4f1_727da1cb","line":4805,"in_reply_to":"1c05831f_0a71ae6f","updated":"2023-02-22 12:05:57.000000000","message":"Done","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ca1eca814d863519f889128366fa3d8a5ba7e20","unresolved":true,"context_lines":[{"line_number":6175,"context_line":"        # ignore device whose guest_format have swap"},{"line_number":6176,"context_line":"        if not instance.new_flavor.swap and instance.old_flavor.swap:"},{"line_number":6177,"context_line":"            LOG.info(\"Excluding swap device from block device mapping\")"},{"line_number":6178,"context_line":"            bdms \u003d [bdm for bdm in bdms if bdm.guest_format !\u003d \u0027swap\u0027]"},{"line_number":6179,"context_line":""},{"line_number":6180,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":6181,"context_line":"            context, instance, refresh_conn_info\u003dTrue, bdms\u003dbdms)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ebebfa2a_e16cad19","line":6178,"range":{"start_line":6178,"start_character":12,"end_line":6178,"end_character":16},"updated":"2023-02-13 21:08:00.000000000","message":"As explained in the comment below, this should be a copy, it shouldn\u0027t mutate the bdms arg that was passed in. That was my fault for giving a bad example previously.","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9fd988ad3f4562b8fdd35b327879458a0cadab0a","unresolved":false,"context_lines":[{"line_number":6175,"context_line":"        # ignore device whose guest_format have swap"},{"line_number":6176,"context_line":"        if not instance.new_flavor.swap and instance.old_flavor.swap:"},{"line_number":6177,"context_line":"            LOG.info(\"Excluding swap device from block device mapping\")"},{"line_number":6178,"context_line":"            bdms \u003d [bdm for bdm in bdms if bdm.guest_format !\u003d \u0027swap\u0027]"},{"line_number":6179,"context_line":""},{"line_number":6180,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":6181,"context_line":"            context, instance, refresh_conn_info\u003dTrue, bdms\u003dbdms)"}],"source_content_type":"text/x-python","patch_set":8,"id":"968d15cb_9010ec33","line":6178,"range":{"start_line":6178,"start_character":12,"end_line":6178,"end_character":16},"in_reply_to":"ebebfa2a_e16cad19","updated":"2023-02-22 11:00:36.000000000","message":"Done","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ca1eca814d863519f889128366fa3d8a5ba7e20","unresolved":true,"context_lines":[{"line_number":6178,"context_line":"            bdms \u003d [bdm for bdm in bdms if bdm.guest_format !\u003d \u0027swap\u0027]"},{"line_number":6179,"context_line":""},{"line_number":6180,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":6181,"context_line":"            context, instance, refresh_conn_info\u003dTrue, bdms\u003dbdms)"},{"line_number":6182,"context_line":""},{"line_number":6183,"context_line":"        # NOTE(mriedem): If the original vm_state was STOPPED, we don\u0027t"},{"line_number":6184,"context_line":"        # automatically power on the instance after it\u0027s migrated"}],"source_content_type":"text/x-python","patch_set":8,"id":"2c5c5b98_08628cf0","line":6181,"range":{"start_line":6181,"start_character":60,"end_line":6181,"end_character":64},"updated":"2023-02-13 21:08:00.000000000","message":"And pass the copy here instead of the original list.","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"b73659ea76c3e7e40004f2d302c39c90aa3cd6ab","unresolved":false,"context_lines":[{"line_number":6178,"context_line":"            bdms \u003d [bdm for bdm in bdms if bdm.guest_format !\u003d \u0027swap\u0027]"},{"line_number":6179,"context_line":""},{"line_number":6180,"context_line":"        block_device_info \u003d self._get_instance_block_device_info("},{"line_number":6181,"context_line":"            context, instance, refresh_conn_info\u003dTrue, bdms\u003dbdms)"},{"line_number":6182,"context_line":""},{"line_number":6183,"context_line":"        # NOTE(mriedem): If the original vm_state was STOPPED, we don\u0027t"},{"line_number":6184,"context_line":"        # automatically power on the instance after it\u0027s migrated"}],"source_content_type":"text/x-python","patch_set":8,"id":"4ab4692b_c678779e","line":6181,"range":{"start_line":6181,"start_character":60,"end_line":6181,"end_character":64},"in_reply_to":"2c5c5b98_08628cf0","updated":"2023-02-22 12:05:57.000000000","message":"Done","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8810e68e8806e117e64ee8861c5cdefa7cb6f0c1","unresolved":true,"context_lines":[{"line_number":5360,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":5361,"context_line":"                                                                 instance)"},{"line_number":5362,"context_line":""},{"line_number":5363,"context_line":"            if hasattr("},{"line_number":5364,"context_line":"                instance.new_flavor, \u0027swap\u0027) and hasattr("},{"line_number":5365,"context_line":"                    instance.old_flavor, \u0027swap\u0027):"},{"line_number":5366,"context_line":"                if not instance.new_flavor.swap and instance.old_flavor.swap:"},{"line_number":5367,"context_line":"                    bdm_dict \u003d block_device.create_blank_bdm("},{"line_number":5368,"context_line":"                        instance.old_flavor.swap, \u0027swap\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"6a4f6a13_c3e2112a","line":5365,"range":{"start_line":5363,"start_character":12,"end_line":5365,"end_character":49},"updated":"2023-02-22 03:53:04.000000000","message":"I don\u0027t think this is needed because \u0027swap\u0027 is not a nullable field on Flavor.","commit_id":"a93f15f8ed606a0b311f63250509e5c1bf32fdc8"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"4abd79f97dac980edaa99014ef3f7bcc08b4a17d","unresolved":false,"context_lines":[{"line_number":5360,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":5361,"context_line":"                                                                 instance)"},{"line_number":5362,"context_line":""},{"line_number":5363,"context_line":"            if hasattr("},{"line_number":5364,"context_line":"                instance.new_flavor, \u0027swap\u0027) and hasattr("},{"line_number":5365,"context_line":"                    instance.old_flavor, \u0027swap\u0027):"},{"line_number":5366,"context_line":"                if not instance.new_flavor.swap and instance.old_flavor.swap:"},{"line_number":5367,"context_line":"                    bdm_dict \u003d block_device.create_blank_bdm("},{"line_number":5368,"context_line":"                        instance.old_flavor.swap, \u0027swap\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"bab7894f_023b63b1","line":5365,"range":{"start_line":5363,"start_character":12,"end_line":5365,"end_character":49},"in_reply_to":"47140a51_4aa759ca","updated":"2023-03-03 11:20:10.000000000","message":"Done","commit_id":"a93f15f8ed606a0b311f63250509e5c1bf32fdc8"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9e35e21c6cb5175471f06dbf18ecf79bd27c5b66","unresolved":true,"context_lines":[{"line_number":5360,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":5361,"context_line":"                                                                 instance)"},{"line_number":5362,"context_line":""},{"line_number":5363,"context_line":"            if hasattr("},{"line_number":5364,"context_line":"                instance.new_flavor, \u0027swap\u0027) and hasattr("},{"line_number":5365,"context_line":"                    instance.old_flavor, \u0027swap\u0027):"},{"line_number":5366,"context_line":"                if not instance.new_flavor.swap and instance.old_flavor.swap:"},{"line_number":5367,"context_line":"                    bdm_dict \u003d block_device.create_blank_bdm("},{"line_number":5368,"context_line":"                        instance.old_flavor.swap, \u0027swap\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"47140a51_4aa759ca","line":5365,"range":{"start_line":5363,"start_character":12,"end_line":5365,"end_character":49},"in_reply_to":"4e40573e_9ac0692a","updated":"2023-02-22 13:34:27.000000000","message":"Ah, OK. I was going to say that not having new_flavor and old_flavor set isn\u0027t a real life scenario for resize and that the test should be changed instead of doing this. But, I also see that _delete_stashed_flavor_info could be called even when something in the _finish_revert_resize raises an exception (because of finally:).\n\nIf that happened and the instance stayed in the VERIFY_RESIZE state after the exception was raised and the user has to try requesting revert resize again, we could actually get here with old_flavor \u003d None and new_flavor \u003d None.\n\nSo I think what you have here is probably for the best.","commit_id":"a93f15f8ed606a0b311f63250509e5c1bf32fdc8"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9fd988ad3f4562b8fdd35b327879458a0cadab0a","unresolved":true,"context_lines":[{"line_number":5360,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":5361,"context_line":"                                                                 instance)"},{"line_number":5362,"context_line":""},{"line_number":5363,"context_line":"            if hasattr("},{"line_number":5364,"context_line":"                instance.new_flavor, \u0027swap\u0027) and hasattr("},{"line_number":5365,"context_line":"                    instance.old_flavor, \u0027swap\u0027):"},{"line_number":5366,"context_line":"                if not instance.new_flavor.swap and instance.old_flavor.swap:"},{"line_number":5367,"context_line":"                    bdm_dict \u003d block_device.create_blank_bdm("},{"line_number":5368,"context_line":"                        instance.old_flavor.swap, \u0027swap\u0027)"}],"source_content_type":"text/x-python","patch_set":14,"id":"4e40573e_9ac0692a","line":5365,"range":{"start_line":5363,"start_character":12,"end_line":5365,"end_character":49},"in_reply_to":"6a4f6a13_c3e2112a","updated":"2023-02-22 11:00:36.000000000","message":"for this test:\ncompute.test_compute_mgr.ComputeManagerMigrationTestCase.test_finish_revert_resize_network_calls_order\n\nWe do not have old or new flavor values here for instance:\n\n(Pdb) hasattr(instance, \u0027old_flavor\u0027) \nTrue\n(Pdb) instance.old_flavor\n\n(Pdb) instance.new_flavor\n\n(Pdb) instance.flavor\nFlavor(created_at\u003d\u003c?\u003e,deleted\u003d\u003c?\u003e,deleted_at\u003d\u003c?\u003e,description\u003d\u003c?\u003e,disabled\u003dFalse,ephemeral_gb\u003d1,extra_specs\u003d{},flavorid\u003d\u00271\u0027,id\u003d1,is_public\u003dTrue,memory_mb\u003d256,name\u003d\u0027flavor1\u0027,projects\u003d[],root_gb\u003d1,rxtx_factor\u003d1.0,swap\u003d0,updated_at\u003d\u003c?\u003e,vcpu_weight\u003d1,vcpus\u003d1)\n\nShould we check for flavor.swap here; in that case in this place we won\u0027t be having the other variable to compare with.","commit_id":"a93f15f8ed606a0b311f63250509e5c1bf32fdc8"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8810e68e8806e117e64ee8861c5cdefa7cb6f0c1","unresolved":true,"context_lines":[{"line_number":6178,"context_line":"            bdm_obj.update_or_create()"},{"line_number":6179,"context_line":""},{"line_number":6180,"context_line":"        # retrieve fresh after update"},{"line_number":6181,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":6182,"context_line":"            context, instance.uuid)"},{"line_number":6183,"context_line":""},{"line_number":6184,"context_line":"        instance.task_state \u003d task_states.RESIZE_FINISH"}],"source_content_type":"text/x-python","patch_set":14,"id":"de286088_6d3f8408","line":6181,"range":{"start_line":6181,"start_character":8,"end_line":6181,"end_character":12},"updated":"2023-02-22 03:53:04.000000000","message":"We should make a copy of the \u0027bdms\u0027 variable here instead of replacing what was passed in from the caller to _finish_resize on L6126. The idea is that we don\u0027t mutate anything from the caller\u0027s perspective ... if they pass in a variable, its value should not have changed after they called our method. We don\u0027t want to create a side effect.\n\nSo we will just want to use a new variable here instead. We could call it \u0027bdms_copy\u0027 or some other descriptive name. Then, on L6201, we will want to pass the new variable \u0027bdms_copy\u0027 to _update_volume_attachments and _get_instance_block_device_info and _complete_volume_attachments instead of passing variable \u0027bdms\u0027. Basically anything that was being called with \u0027bdms\u0027 after we made our copy.","commit_id":"a93f15f8ed606a0b311f63250509e5c1bf32fdc8"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9fd988ad3f4562b8fdd35b327879458a0cadab0a","unresolved":false,"context_lines":[{"line_number":6178,"context_line":"            bdm_obj.update_or_create()"},{"line_number":6179,"context_line":""},{"line_number":6180,"context_line":"        # retrieve fresh after update"},{"line_number":6181,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":6182,"context_line":"            context, instance.uuid)"},{"line_number":6183,"context_line":""},{"line_number":6184,"context_line":"        instance.task_state \u003d task_states.RESIZE_FINISH"}],"source_content_type":"text/x-python","patch_set":14,"id":"f0dcb5e0_e6a0ff5a","line":6181,"range":{"start_line":6181,"start_character":8,"end_line":6181,"end_character":12},"in_reply_to":"de286088_6d3f8408","updated":"2023-02-22 11:00:36.000000000","message":"Done","commit_id":"a93f15f8ed606a0b311f63250509e5c1bf32fdc8"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"df273fb024f0478f77007d566ae338f3b98977a8","unresolved":true,"context_lines":[{"line_number":6179,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":6180,"context_line":"                context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":6181,"context_line":"            bdm_obj.update_or_create()"},{"line_number":6182,"context_line":""},{"line_number":6183,"context_line":"        # retrieve fresh copy of bdm after update"},{"line_number":6184,"context_line":"        # original bdm values which were passed by caller"},{"line_number":6185,"context_line":"        # should not get updated."}],"source_content_type":"text/x-python","patch_set":16,"id":"1b097c0b_750d8aec","line":6182,"updated":"2023-02-22 14:36:48.000000000","message":"if flavor.swap is becoming 0, delete disk (from mapping) which has swap. (so once driver is done with xml, i.e all resize is done, instance will reload/reboot and disk will be fine)\n\nso always update bdm with new flavor swap, unless flavor.swap is shrinking to 0.","commit_id":"3514a6a6fd4c8fea3817de238106c793744eaf68"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"4abd79f97dac980edaa99014ef3f7bcc08b4a17d","unresolved":false,"context_lines":[{"line_number":6179,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":6180,"context_line":"                context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":6181,"context_line":"            bdm_obj.update_or_create()"},{"line_number":6182,"context_line":""},{"line_number":6183,"context_line":"        # retrieve fresh copy of bdm after update"},{"line_number":6184,"context_line":"        # original bdm values which were passed by caller"},{"line_number":6185,"context_line":"        # should not get updated."}],"source_content_type":"text/x-python","patch_set":16,"id":"249eb774_22e03023","line":6182,"in_reply_to":"1b097c0b_750d8aec","updated":"2023-03-03 11:20:10.000000000","message":"Done","commit_id":"3514a6a6fd4c8fea3817de238106c793744eaf68"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1d36a72c2c2b581d4723a802bc36742877c2ada9","unresolved":true,"context_lines":[{"line_number":5373,"context_line":"                        instance.old_flavor.swap, \u0027swap\u0027)"},{"line_number":5374,"context_line":"                    bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5375,"context_line":"                        context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":5376,"context_line":"                    bdm_obj.update_or_create()"},{"line_number":5377,"context_line":""},{"line_number":5378,"context_line":"            # retrieve fresh after update"},{"line_number":5379,"context_line":"            bdms_copy \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("}],"source_content_type":"text/x-python","patch_set":18,"id":"b92d533c_d9342245","line":5376,"updated":"2023-03-02 21:34:48.000000000","message":"I think there is an issue here ... if old flavor and new flavor both have no swap and have swap \u003d 0, then this will create a swap BDM which is not correct.\n\nEven if both flavors hasattr(\u0027swap\u0027) they could both be swap \u003d 0. We should add more test coverage in this patch to catch that case 😕 like verify that if both have swap \u003d 0 then there is no BDM for swap found, if both have swap verify that there is only one BDM for swap and that the size is correct.\n\nWe have to do a little more here to make sure we only create_blank_bdm if we are creating a new additional BDM. Else if we want to make an update to an existing swap BDM, we need to get it from the database, update its volume_size, then save() it.\n\n(The db api method that ends up being called for update_or_create [1] will only update an existing record if block device \u0027uuid\u0027 or \u0027device_name\u0027 have been updated on the BlockDeviceMapping object after retrieving it from the DB.)\n\n[1] https://github.com/openstack/nova/blob/6b0b009783770354f9387ae4d2d99fca05fb5acd/nova/db/main/api.py#L2879","commit_id":"9163a1d0bd0b69b69f0410a515bfcb58a0e26079"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"325c6c381f40d14bd4cf468d7e519c7aa6e48850","unresolved":false,"context_lines":[{"line_number":5373,"context_line":"                        instance.old_flavor.swap, \u0027swap\u0027)"},{"line_number":5374,"context_line":"                    bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5375,"context_line":"                        context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":5376,"context_line":"                    bdm_obj.update_or_create()"},{"line_number":5377,"context_line":""},{"line_number":5378,"context_line":"            # retrieve fresh after update"},{"line_number":5379,"context_line":"            bdms_copy \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("}],"source_content_type":"text/x-python","patch_set":18,"id":"80aca571_1edbd372","line":5376,"in_reply_to":"4e75b06c_72cb2dff","updated":"2023-03-16 08:02:22.000000000","message":"Done","commit_id":"9163a1d0bd0b69b69f0410a515bfcb58a0e26079"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a342b7508c17403ea3dda9952efdb0b72cf68767","unresolved":true,"context_lines":[{"line_number":5373,"context_line":"                        instance.old_flavor.swap, \u0027swap\u0027)"},{"line_number":5374,"context_line":"                    bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5375,"context_line":"                        context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":5376,"context_line":"                    bdm_obj.update_or_create()"},{"line_number":5377,"context_line":""},{"line_number":5378,"context_line":"            # retrieve fresh after update"},{"line_number":5379,"context_line":"            bdms_copy \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("}],"source_content_type":"text/x-python","patch_set":18,"id":"4e75b06c_72cb2dff","line":5376,"in_reply_to":"707574a0_2260632d","updated":"2023-03-16 00:10:53.000000000","message":"Excellent!","commit_id":"9163a1d0bd0b69b69f0410a515bfcb58a0e26079"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"4abd79f97dac980edaa99014ef3f7bcc08b4a17d","unresolved":true,"context_lines":[{"line_number":5373,"context_line":"                        instance.old_flavor.swap, \u0027swap\u0027)"},{"line_number":5374,"context_line":"                    bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5375,"context_line":"                        context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":5376,"context_line":"                    bdm_obj.update_or_create()"},{"line_number":5377,"context_line":""},{"line_number":5378,"context_line":"            # retrieve fresh after update"},{"line_number":5379,"context_line":"            bdms_copy \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("}],"source_content_type":"text/x-python","patch_set":18,"id":"707574a0_2260632d","line":5376,"in_reply_to":"b92d533c_d9342245","updated":"2023-03-03 11:20:10.000000000","message":"I have added one more test 0 to 0, so at revert there will be swap attribute, but _create_blank_bdm_ only if earlier had swap i.e old_flavor swap is non-zero.\n\nright now we have scenarios:\n   resize 0 -\u003e 0 -\u003e revert      // revert same\n   resize 0 -\u003e 0 -\u003e confirm     // confirm same\n   resize 0 -\u003e 1 -\u003e revert      // revert expand\n   resize 0 -\u003e 1 -\u003e confirm     // confirm expand\n   .\n   resize 1 -\u003e 0 -\u003e revert      // revert shrink\n   resize 1 -\u003e 0 -\u003e confirm     // confirm shrink\n   resize 1 -\u003e 2 -\u003e revert      // revert non-zero expand\n   resize 1 -\u003e 2 -\u003e confirm     // confirm non-zero expand\n   .\n   resize 2 -\u003e 1 -\u003e revert      // revert non-zero shrink\n   resize 2 -\u003e 1 -\u003e confirm     // confirm non-zero shrink\n   resize 2 -\u003e 2 -\u003e revert      // revert non-zero same\n   resize 2 -\u003e 2 -\u003e confirm     // confirm non-zero same","commit_id":"9163a1d0bd0b69b69f0410a515bfcb58a0e26079"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"1d36a72c2c2b581d4723a802bc36742877c2ada9","unresolved":true,"context_lines":[{"line_number":6177,"context_line":"                instance.new_flavor.swap, \u0027swap\u0027)"},{"line_number":6178,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":6179,"context_line":"                context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":6180,"context_line":"            bdm_obj.update_or_create()"},{"line_number":6181,"context_line":""},{"line_number":6182,"context_line":"        # retrieve fresh copy of bdm after update"},{"line_number":6183,"context_line":"        # original bdm values which were passed by caller"}],"source_content_type":"text/x-python","patch_set":18,"id":"0d57d869_7fb6626e","line":6180,"updated":"2023-03-02 21:34:48.000000000","message":"Similar here ... if new flavor and old flavor both have swap (and the instance already has a swap BDM), this will create another swap BDM which is not correct.","commit_id":"9163a1d0bd0b69b69f0410a515bfcb58a0e26079"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"4abd79f97dac980edaa99014ef3f7bcc08b4a17d","unresolved":true,"context_lines":[{"line_number":6177,"context_line":"                instance.new_flavor.swap, \u0027swap\u0027)"},{"line_number":6178,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":6179,"context_line":"                context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":6180,"context_line":"            bdm_obj.update_or_create()"},{"line_number":6181,"context_line":""},{"line_number":6182,"context_line":"        # retrieve fresh copy of bdm after update"},{"line_number":6183,"context_line":"        # original bdm values which were passed by caller"}],"source_content_type":"text/x-python","patch_set":18,"id":"7d2e4267_f1d43ead","line":6180,"in_reply_to":"0d57d869_7fb6626e","updated":"2023-03-03 11:20:10.000000000","message":"resize 1024 -\u003e 2048 and 2048 -\u003e 1024\nfor expand and shrink, unless swap is ever be 0, \nafter update both bdms (sent by caller) and bdms_copy has only one swap dev.\n\n(Pdb) [bdm.guest_format for bdm in bdms]\n[None, None, \u0027swap\u0027]\n\n(Pdb) [bdm.guest_format for bdm in bdms_copy]\n[None, None, \u0027swap\u0027]","commit_id":"9163a1d0bd0b69b69f0410a515bfcb58a0e26079"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"325c6c381f40d14bd4cf468d7e519c7aa6e48850","unresolved":false,"context_lines":[{"line_number":6177,"context_line":"                instance.new_flavor.swap, \u0027swap\u0027)"},{"line_number":6178,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":6179,"context_line":"                context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":6180,"context_line":"            bdm_obj.update_or_create()"},{"line_number":6181,"context_line":""},{"line_number":6182,"context_line":"        # retrieve fresh copy of bdm after update"},{"line_number":6183,"context_line":"        # original bdm values which were passed by caller"}],"source_content_type":"text/x-python","patch_set":18,"id":"50b8bf57_2ed3aa98","line":6180,"in_reply_to":"33e76c57_64af1bb7","updated":"2023-03-16 08:02:22.000000000","message":"Done","commit_id":"9163a1d0bd0b69b69f0410a515bfcb58a0e26079"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a342b7508c17403ea3dda9952efdb0b72cf68767","unresolved":true,"context_lines":[{"line_number":6177,"context_line":"                instance.new_flavor.swap, \u0027swap\u0027)"},{"line_number":6178,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":6179,"context_line":"                context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":6180,"context_line":"            bdm_obj.update_or_create()"},{"line_number":6181,"context_line":""},{"line_number":6182,"context_line":"        # retrieve fresh copy of bdm after update"},{"line_number":6183,"context_line":"        # original bdm values which were passed by caller"}],"source_content_type":"text/x-python","patch_set":18,"id":"33e76c57_64af1bb7","line":6180,"in_reply_to":"7d2e4267_f1d43ead","updated":"2023-03-16 00:10:53.000000000","message":"Hah, I totally missed this specialized code block [1] that deletes any existing swap BDMs if the new BDM is \u0027swap\u0027 /facepalm\n\n    # NOTE(xqueralt): Prevent from having multiple swap devices for the\n    # same instance. This will delete all the existing ones.\n    if block_device.new_format_is_swap(values):\n        query \u003d _block_device_mapping_get_query(context)\n        query \u003d query.filter_by(instance_uuid\u003dvalues[\u0027instance_uuid\u0027],\n                                source_type\u003d\u0027blank\u0027, guest_format\u003d\u0027swap\u0027)\n        query \u003d query.filter(models.BlockDeviceMapping.id !\u003d result.id)\n        query.soft_delete()\n        \nI was really not expecting that. Thanks for verifying the behavior.\n\n[1] https://github.com/openstack/nova/blob/3886f078dea50baa062c732a0bd9f653e35e09cc/nova/db/main/api.py#L2903-L2910","commit_id":"9163a1d0bd0b69b69f0410a515bfcb58a0e26079"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b7bbc77b458f49d775ffbe57352bf0de9f6b6b2a","unresolved":true,"context_lines":[{"line_number":5360,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":5361,"context_line":"                                                                 instance)"},{"line_number":5362,"context_line":""},{"line_number":5363,"context_line":"            if hasattr("},{"line_number":5364,"context_line":"                instance.new_flavor, \u0027swap\u0027) and hasattr("},{"line_number":5365,"context_line":"                    instance.old_flavor, \u0027swap\u0027):"},{"line_number":5366,"context_line":"                if instance.new_flavor.swap and not instance.old_flavor.swap:"}],"source_content_type":"text/x-python","patch_set":24,"id":"73ea5288_44df7b60","line":5363,"range":{"start_line":5363,"start_character":15,"end_line":5363,"end_character":22},"updated":"2023-04-06 09:43:15.000000000","message":"nit and FYI: instead of using stdlib hasattr, you should know that oslo.versionedobjects includes for each of the objects that are created a specific obj_attr_is_set() method that tells you whether the object field attribute is set :\nhttps://docs.openstack.org/oslo.versionedobjects/latest/reference/base.html#oslo_versionedobjects.base.VersionedObject.obj_attr_is_set\n\neg. here you can write :\n\n if instance.new_flavor.obj_attr_is_set(\u0027swap\u0027) and instance.old_flavor_obj_attr_is_set(\u0027swap\u0027):\n   \u003cdo the further checks below\u003e\n\nTBC, that doesn\u0027t hold my vote, again, it\u0027s just a FYI.","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"ba627966bd68ef6bef73555fa65d55f4ca9cdca0","unresolved":true,"context_lines":[{"line_number":5360,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":5361,"context_line":"                                                                 instance)"},{"line_number":5362,"context_line":""},{"line_number":5363,"context_line":"            if hasattr("},{"line_number":5364,"context_line":"                instance.new_flavor, \u0027swap\u0027) and hasattr("},{"line_number":5365,"context_line":"                    instance.old_flavor, \u0027swap\u0027):"},{"line_number":5366,"context_line":"                if instance.new_flavor.swap and not instance.old_flavor.swap:"}],"source_content_type":"text/x-python","patch_set":24,"id":"e1d158fc_683006e7","line":5363,"range":{"start_line":5363,"start_character":15,"end_line":5363,"end_character":22},"in_reply_to":"375b1247_e33cb0c6","updated":"2023-05-09 17:53:09.000000000","message":"without these changes also unit TC\u0027s are setting instance.new_flavor and old_flavor to None, not only the failed ones but others (passing ones) as well for example `ComputeRpcAPITestCase.test_finish_revert_resize`\n\nso adding a check for None like I was doing earlier using hasattr will be skipping those tests.","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"edb4d59d96bcd481fbc87632dfa572aa0c11f1c0","unresolved":false,"context_lines":[{"line_number":5360,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":5361,"context_line":"                                                                 instance)"},{"line_number":5362,"context_line":""},{"line_number":5363,"context_line":"            if hasattr("},{"line_number":5364,"context_line":"                instance.new_flavor, \u0027swap\u0027) and hasattr("},{"line_number":5365,"context_line":"                    instance.old_flavor, \u0027swap\u0027):"},{"line_number":5366,"context_line":"                if instance.new_flavor.swap and not instance.old_flavor.swap:"}],"source_content_type":"text/x-python","patch_set":24,"id":"06c2d34d_afc95633","line":5363,"range":{"start_line":5363,"start_character":15,"end_line":5363,"end_character":22},"in_reply_to":"56ad23df_69a39856","updated":"2023-07-14 16:54:56.000000000","message":"Done","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"9bd6a1da98db6a6b46b8972465afbcd507642193","unresolved":true,"context_lines":[{"line_number":5360,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":5361,"context_line":"                                                                 instance)"},{"line_number":5362,"context_line":""},{"line_number":5363,"context_line":"            if hasattr("},{"line_number":5364,"context_line":"                instance.new_flavor, \u0027swap\u0027) and hasattr("},{"line_number":5365,"context_line":"                    instance.old_flavor, \u0027swap\u0027):"},{"line_number":5366,"context_line":"                if instance.new_flavor.swap and not instance.old_flavor.swap:"}],"source_content_type":"text/x-python","patch_set":24,"id":"f923b2a8_f3c8214f","line":5363,"range":{"start_line":5363,"start_character":15,"end_line":5363,"end_character":22},"in_reply_to":"72f1a41f_a925d0a0","updated":"2023-04-25 15:27:09.000000000","message":"Cool with me, turning my vote then.","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cc154cf01de3a5fd67aa20ad07ac5a0c20a45b78","unresolved":true,"context_lines":[{"line_number":5360,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":5361,"context_line":"                                                                 instance)"},{"line_number":5362,"context_line":""},{"line_number":5363,"context_line":"            if hasattr("},{"line_number":5364,"context_line":"                instance.new_flavor, \u0027swap\u0027) and hasattr("},{"line_number":5365,"context_line":"                    instance.old_flavor, \u0027swap\u0027):"},{"line_number":5366,"context_line":"                if instance.new_flavor.swap and not instance.old_flavor.swap:"}],"source_content_type":"text/x-python","patch_set":24,"id":"72f1a41f_a925d0a0","line":5363,"range":{"start_line":5363,"start_character":15,"end_line":5363,"end_character":22},"in_reply_to":"73ea5288_44df7b60","updated":"2023-04-25 15:12:30.000000000","message":"Yeah, and this is the proper way to do this on any o.vo object, *not* using hasattr().","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3a5ac35c919767fb4ceea1144fd3ee7a6ac502a3","unresolved":true,"context_lines":[{"line_number":5360,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":5361,"context_line":"                                                                 instance)"},{"line_number":5362,"context_line":""},{"line_number":5363,"context_line":"            if hasattr("},{"line_number":5364,"context_line":"                instance.new_flavor, \u0027swap\u0027) and hasattr("},{"line_number":5365,"context_line":"                    instance.old_flavor, \u0027swap\u0027):"},{"line_number":5366,"context_line":"                if instance.new_flavor.swap and not instance.old_flavor.swap:"}],"source_content_type":"text/x-python","patch_set":24,"id":"dc37e4a3_d4023487","line":5363,"range":{"start_line":5363,"start_character":15,"end_line":5363,"end_character":22},"in_reply_to":"85b81d2b_8c955147","updated":"2023-04-25 15:48:30.000000000","message":"I don\u0027t think I\u0027d say it\u0027s wrong. For a lot of years, we didn\u0027t have the `__contains__` helper and we still have code using the ovo method directly. It\u0027s definitely shorter though. Point being, asking the o.vo-specific handler for whether or not it\u0027s set is the proper way, regardless of if you use the operator or direct-call approach. `hasattr()` is the *wrong* way.","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"2ff613dd27977e916a3e5239cc466a739a1ee2e2","unresolved":true,"context_lines":[{"line_number":5360,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":5361,"context_line":"                                                                 instance)"},{"line_number":5362,"context_line":""},{"line_number":5363,"context_line":"            if hasattr("},{"line_number":5364,"context_line":"                instance.new_flavor, \u0027swap\u0027) and hasattr("},{"line_number":5365,"context_line":"                    instance.old_flavor, \u0027swap\u0027):"},{"line_number":5366,"context_line":"                if instance.new_flavor.swap and not instance.old_flavor.swap:"}],"source_content_type":"text/x-python","patch_set":24,"id":"375b1247_e33cb0c6","line":5363,"range":{"start_line":5363,"start_character":15,"end_line":5363,"end_character":22},"in_reply_to":"dc37e4a3_d4023487","updated":"2023-05-03 10:28:34.000000000","message":"thanks all for review.\non replacing hasattr with obj_attr_is_set to verify if instance.new_flavor has \u0027swap\u0027 attribute few revert tests failed.\n\nfor unit tests in test_compute_mgr, values for both old and instance.new_flavor is set to None.\n```\ntest_finish_revert_resize_recalc_group_rp_mapping_missing_request_spec\ntest_finish_revert_resize_network_calls_order\ntest_finish_revert_resize_recalc_group_rp_mapping\ntest_finish_revert_resize_migration_context\n```\n\nhttps://048aa88193f15d1483ae-0f88ed887b9c4e564788b07ac2deace4.ssl.cf2.rackcdn.com/857339/25/check/openstack-tox-cover/c997939/testr_results.html\n\n\n```\n(Pdb) \u0027swap\u0027 in instance.new_flavor\n*** TypeError: argument of type \u0027NoneType\u0027 is not iterable\n\n(Pdb) dir(instance.new_flavor)\n[\u0027__bool__\u0027, \u0027__class__\u0027, \u0027__delattr__\u0027, \u0027__dir__\u0027, \u0027__doc__\u0027, \u0027__eq__\u0027, \u0027__format__\u0027, \u0027__ge__\u0027, \u0027__getattribute__\u0027, \u0027__gt__\u0027, \u0027__hash__\u0027, \u0027__init__\u0027, \u0027__init_subclass__\u0027, \u0027__le__\u0027, \u0027__lt__\u0027, \u0027__ne__\u0027, \u0027__new__\u0027, \u0027__reduce__\u0027, \u0027__reduce_ex__\u0027, \u0027__repr__\u0027, \u0027__setattr__\u0027, \u0027__sizeof__\u0027, \u0027__str__\u0027, \u0027__subclasshook__\u0027]\n```\n\n\nthis is one of the test:\nhttps://github.com/openstack/nova/blob/master/nova/tests/unit/compute/test_compute_mgr.py#L9306\nhere..\n```\n(Pdb) dict(self.instance)\n{\u0027id\u0027: 1, \u0027user_id\u0027: \u0027fake-user\u0027, \u0027project_id\u0027: \u0027fake-project\u0027, \u0027image_ref\u0027: None, \u0027kernel_id\u0027: None, \u0027ramdisk_id\u0027: None, \u0027hostname\u0027: None, \u0027launch_index\u0027: None, \u0027key_name\u0027: None, \u0027key_data\u0027: None, \u0027power_state\u0027: None, \u0027vm_state\u0027: \u0027active\u0027, \u0027task_state\u0027: None, \u0027memory_mb\u0027: 256, \u0027vcpus\u0027: 1, \u0027root_gb\u0027: 0, \u0027ephemeral_gb\u0027: 0, \u0027ephemeral_key_uuid\u0027: None, \u0027host\u0027: \u0027fake-host\u0027, \u0027node\u0027: None, \u0027instance_type_id\u0027: 1, \u0027user_data\u0027: None, \u0027reservation_id\u0027: None, \u0027launched_at\u0027: None, \u0027terminated_at\u0027: None, \u0027availability_zone\u0027: None, \u0027display_name\u0027: None, \u0027display_description\u0027: None, \u0027launched_on\u0027: None, \u0027locked\u0027: False, \u0027locked_by\u0027: None, \u0027os_type\u0027: None, \u0027architecture\u0027: None, \u0027vm_mode\u0027: None, \u0027uuid\u0027: \u0027a55feca1-adfc-4c9d-943b-ceadfe001001\u0027, \u0027root_device_name\u0027: None, \u0027default_ephemeral_device\u0027: None, \u0027default_swap_device\u0027: None, \u0027config_drive\u0027: None, \u0027access_ip_v4\u0027: None, \u0027access_ip_v6\u0027: None, \u0027auto_disk_config\u0027: False, \u0027progress\u0027: None, \u0027shutdown_terminate\u0027: False, \u0027disable_terminate\u0027: False, \u0027cell_name\u0027: None, \u0027metadata\u0027: {}, \u0027system_metadata\u0027: {}, \u0027info_cache\u0027: None, \u0027cleaned\u0027: False, \u0027tags\u0027: TagList(objects\u003d[]), \u0027flavor\u0027: Flavor(created_at\u003d\u003c?\u003e,deleted\u003d\u003c?\u003e,deleted_at\u003d\u003c?\u003e,description\u003d\u003c?\u003e,disabled\u003dFalse,ephemeral_gb\u003d1,extra_specs\u003d{},flavorid\u003d\u00271\u0027,id\u003d1,is_public\u003dTrue,memory_mb\u003d256,name\u003d\u0027flavor1\u0027,projects\u003d[],root_gb\u003d1,rxtx_factor\u003d1.0,swap\u003d0,updated_at\u003d\u003c?\u003e,vcpu_weight\u003d1,vcpus\u003d1), \u0027old_flavor\u0027: None, \u0027new_flavor\u0027: None, \u0027migration_context\u0027: MigrationContext(created_at\u003d\u003c?\u003e,deleted\u003d\u003c?\u003e,deleted_at\u003d\u003c?\u003e,instance_uuid\u003d\u003c?\u003e,migration_id\u003d\u003c?\u003e,new_numa_topology\u003d\u003c?\u003e,new_pci_devices\u003d\u003c?\u003e,new_pci_requests\u003d\u003c?\u003e,new_resources\u003d\u003c?\u003e,old_numa_topology\u003d\u003c?\u003e,old_pci_devices\u003d\u003c?\u003e,old_pci_requests\u003d\u003c?\u003e,old_resources\u003d\u003c?\u003e,updated_at\u003d\u003c?\u003e), \u0027keypairs\u0027: KeyPairList(objects\u003d[]), \u0027hidden\u0027: False, \u0027resources\u0027: None, \u0027created_at\u0027: datetime.datetime(1955, 11, 5, 0, 0, tzinfo\u003ddatetime.timezone.utc), \u0027updated_at\u0027: None, \u0027deleted_at\u0027: None, \u0027deleted\u0027: False, \u0027name\u0027: \u0027instance-00000001\u0027}\n```","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"6382f92d9898a479b9d4e4f8485adc7e95e178cd","unresolved":true,"context_lines":[{"line_number":5360,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":5361,"context_line":"                                                                 instance)"},{"line_number":5362,"context_line":""},{"line_number":5363,"context_line":"            if hasattr("},{"line_number":5364,"context_line":"                instance.new_flavor, \u0027swap\u0027) and hasattr("},{"line_number":5365,"context_line":"                    instance.old_flavor, \u0027swap\u0027):"},{"line_number":5366,"context_line":"                if instance.new_flavor.swap and not instance.old_flavor.swap:"}],"source_content_type":"text/x-python","patch_set":24,"id":"56ad23df_69a39856","line":5363,"range":{"start_line":5363,"start_character":15,"end_line":5363,"end_character":22},"in_reply_to":"e1d158fc_683006e7","updated":"2023-05-10 08:51:40.000000000","message":"making update bdm separate routine cleared things.\n\nso as we added functionality for updating BDM flavor having swap, we need to call this functionality only if instance has flavor.\n\nstill verified just to be sure and we are not skipping any resize tests.","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2b134b3724eeeef4ae01e0624015b03b63fc30d7","unresolved":true,"context_lines":[{"line_number":5360,"context_line":"            network_info \u003d self.network_api.get_instance_nw_info(context,"},{"line_number":5361,"context_line":"                                                                 instance)"},{"line_number":5362,"context_line":""},{"line_number":5363,"context_line":"            if hasattr("},{"line_number":5364,"context_line":"                instance.new_flavor, \u0027swap\u0027) and hasattr("},{"line_number":5365,"context_line":"                    instance.old_flavor, \u0027swap\u0027):"},{"line_number":5366,"context_line":"                if instance.new_flavor.swap and not instance.old_flavor.swap:"}],"source_content_type":"text/x-python","patch_set":24,"id":"85b81d2b_8c955147","line":5363,"range":{"start_line":5363,"start_character":15,"end_line":5363,"end_character":22},"in_reply_to":"f923b2a8_f3c8214f","updated":"2023-04-25 15:32:36.000000000","message":"actuly the proper way to do it is really to use in\n\n\nyou can do \n\nif \"field\" in ovo:\n\nbecause ovo base has an overloaded contains\n\nhttps://github.com/openstack/oslo.versionedobjects/blob/master/oslo_versionedobjects/base.py#L319-L324\n\nyou should not call .obj_attr_is_set on the ov directly.","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cc154cf01de3a5fd67aa20ad07ac5a0c20a45b78","unresolved":true,"context_lines":[{"line_number":5379,"context_line":""},{"line_number":5380,"context_line":"            # retrieve fresh in case there is bdm update"},{"line_number":5381,"context_line":"            bdms_copy \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5382,"context_line":"                context, instance.uuid)"},{"line_number":5383,"context_line":"            # revert_resize deleted any volume attachments for the instance"},{"line_number":5384,"context_line":"            # and created new ones to be used on this host, but we"},{"line_number":5385,"context_line":"            # have to update those attachments with the host connector so the"}],"source_content_type":"text/x-python","patch_set":24,"id":"688c8da8_79e09dd1","line":5382,"updated":"2023-04-25 15:12:30.000000000","message":"This is a pretty expensive thing to do when you already have the list. You\u0027re also doing this for every case, even if you haven\u0027t modified the list. IMHO it would be much better to just use the list you already have. It should be trivial to add the new swap bdm to the list or remove the one you\u0027re destroying right?","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"60a96dfcf4af765fbf6b035183ad27dbfbb3de33","unresolved":true,"context_lines":[{"line_number":5379,"context_line":""},{"line_number":5380,"context_line":"            # retrieve fresh in case there is bdm update"},{"line_number":5381,"context_line":"            bdms_copy \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5382,"context_line":"                context, instance.uuid)"},{"line_number":5383,"context_line":"            # revert_resize deleted any volume attachments for the instance"},{"line_number":5384,"context_line":"            # and created new ones to be used on this host, but we"},{"line_number":5385,"context_line":"            # have to update those attachments with the host connector so the"}],"source_content_type":"text/x-python","patch_set":24,"id":"eb61f29d_065fdcc4","line":5382,"in_reply_to":"13ced54a_bb9fc5d2","updated":"2023-06-09 09:34:05.000000000","message":"Dan, Sylvain can you please have another look on this patch.","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"9bd6a1da98db6a6b46b8972465afbcd507642193","unresolved":true,"context_lines":[{"line_number":5379,"context_line":""},{"line_number":5380,"context_line":"            # retrieve fresh in case there is bdm update"},{"line_number":5381,"context_line":"            bdms_copy \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5382,"context_line":"                context, instance.uuid)"},{"line_number":5383,"context_line":"            # revert_resize deleted any volume attachments for the instance"},{"line_number":5384,"context_line":"            # and created new ones to be used on this host, but we"},{"line_number":5385,"context_line":"            # have to update those attachments with the host connector so the"}],"source_content_type":"text/x-python","patch_set":24,"id":"78c7e0eb_bbfe17cf","line":5382,"in_reply_to":"688c8da8_79e09dd1","updated":"2023-04-25 15:27:09.000000000","message":"+1","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"2ff613dd27977e916a3e5239cc466a739a1ee2e2","unresolved":true,"context_lines":[{"line_number":5379,"context_line":""},{"line_number":5380,"context_line":"            # retrieve fresh in case there is bdm update"},{"line_number":5381,"context_line":"            bdms_copy \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5382,"context_line":"                context, instance.uuid)"},{"line_number":5383,"context_line":"            # revert_resize deleted any volume attachments for the instance"},{"line_number":5384,"context_line":"            # and created new ones to be used on this host, but we"},{"line_number":5385,"context_line":"            # have to update those attachments with the host connector so the"}],"source_content_type":"text/x-python","patch_set":24,"id":"13ced54a_bb9fc5d2","line":5382,"in_reply_to":"78c7e0eb_bbfe17cf","updated":"2023-05-03 10:28:34.000000000","message":"fixed \"retrieve fresh bdm copy\" to only retrieve if bdm list is modified and not everytime.\n\nalso I feel I have all possible test scenarios in place, and I noticed even if we do not retrieve a fresh bdm copy and use same bdms object for further process, still all tests passed.","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"edb4d59d96bcd481fbc87632dfa572aa0c11f1c0","unresolved":false,"context_lines":[{"line_number":5379,"context_line":""},{"line_number":5380,"context_line":"            # retrieve fresh in case there is bdm update"},{"line_number":5381,"context_line":"            bdms_copy \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5382,"context_line":"                context, instance.uuid)"},{"line_number":5383,"context_line":"            # revert_resize deleted any volume attachments for the instance"},{"line_number":5384,"context_line":"            # and created new ones to be used on this host, but we"},{"line_number":5385,"context_line":"            # have to update those attachments with the host connector so the"}],"source_content_type":"text/x-python","patch_set":24,"id":"6bb7bb03_2d98692b","line":5382,"in_reply_to":"eb61f29d_065fdcc4","updated":"2023-07-14 16:54:56.000000000","message":"Done","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b7bbc77b458f49d775ffbe57352bf0de9f6b6b2a","unresolved":false,"context_lines":[{"line_number":6168,"context_line":"        # delete or create a BDM record to keep things in sync."},{"line_number":6169,"context_line":"        # Otherwise, the instance XML will not be generated correctly"},{"line_number":6170,"context_line":"        # during a hard reboot or stop/start."},{"line_number":6171,"context_line":"        if not instance.new_flavor.swap and instance.old_flavor.swap:"},{"line_number":6172,"context_line":"            # shrink swap"},{"line_number":6173,"context_line":"            swap_bdms \u003d [bdm for bdm in bdms if bdm.guest_format \u003d\u003d \u0027swap\u0027]"},{"line_number":6174,"context_line":"            if swap_bdms:"}],"source_content_type":"text/x-python","patch_set":24,"id":"930a0811_8c017889","line":6171,"updated":"2023-04-06 09:43:15.000000000","message":"hum, here, you don\u0027t check whether the swap attribute exists, but everytime we have it hydrated so fair enough, this isn\u0027t needed.","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b7bbc77b458f49d775ffbe57352bf0de9f6b6b2a","unresolved":false,"context_lines":[{"line_number":6177,"context_line":"            # unless swap is shrinking to 0 we should update"},{"line_number":6178,"context_line":"            # bdm as per new flavor swap"},{"line_number":6179,"context_line":"            bdm_dict \u003d block_device.create_blank_bdm("},{"line_number":6180,"context_line":"                instance.new_flavor.swap, \u0027swap\u0027)"},{"line_number":6181,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":6182,"context_line":"                context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":6183,"context_line":"            bdm_obj.update_or_create()"}],"source_content_type":"text/x-python","patch_set":24,"id":"41a20899_d6cda80a","line":6180,"updated":"2023-04-06 09:43:15.000000000","message":"nit: the logic is quite similar to the revert case above, with the slight difference that in a case we remove the swap while in other the case, we allocate it.\n\nMeh, we may have made a private helper method that would do both with a parameter, or we could duplicate that logic like here, I\u0027m not really opinionated by that.","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"48cc3edf303150ba3a83c39584f038056493d45f","unresolved":false,"context_lines":[{"line_number":6177,"context_line":"            # unless swap is shrinking to 0 we should update"},{"line_number":6178,"context_line":"            # bdm as per new flavor swap"},{"line_number":6179,"context_line":"            bdm_dict \u003d block_device.create_blank_bdm("},{"line_number":6180,"context_line":"                instance.new_flavor.swap, \u0027swap\u0027)"},{"line_number":6181,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":6182,"context_line":"                context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":6183,"context_line":"            bdm_obj.update_or_create()"}],"source_content_type":"text/x-python","patch_set":24,"id":"5eb1c857_218ee954","line":6180,"in_reply_to":"3c4eb746_3a591465","updated":"2023-07-19 07:59:09.000000000","message":"Done","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"cc154cf01de3a5fd67aa20ad07ac5a0c20a45b78","unresolved":true,"context_lines":[{"line_number":6177,"context_line":"            # unless swap is shrinking to 0 we should update"},{"line_number":6178,"context_line":"            # bdm as per new flavor swap"},{"line_number":6179,"context_line":"            bdm_dict \u003d block_device.create_blank_bdm("},{"line_number":6180,"context_line":"                instance.new_flavor.swap, \u0027swap\u0027)"},{"line_number":6181,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":6182,"context_line":"                context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":6183,"context_line":"            bdm_obj.update_or_create()"}],"source_content_type":"text/x-python","patch_set":24,"id":"8bc7c25e_71e17347","line":6180,"in_reply_to":"41a20899_d6cda80a","updated":"2023-04-25 15:12:30.000000000","message":"Yeah it seems like a common routine to add or remove the swap bdm to a given list makes sense. It also hopefully means we\u0027d trust that it\u0027s right, test it specifically, and avoid the re-query in both places.","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"9bd6a1da98db6a6b46b8972465afbcd507642193","unresolved":true,"context_lines":[{"line_number":6177,"context_line":"            # unless swap is shrinking to 0 we should update"},{"line_number":6178,"context_line":"            # bdm as per new flavor swap"},{"line_number":6179,"context_line":"            bdm_dict \u003d block_device.create_blank_bdm("},{"line_number":6180,"context_line":"                instance.new_flavor.swap, \u0027swap\u0027)"},{"line_number":6181,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":6182,"context_line":"                context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":6183,"context_line":"            bdm_obj.update_or_create()"}],"source_content_type":"text/x-python","patch_set":24,"id":"e333722b_71481e98","line":6180,"in_reply_to":"8bc7c25e_71e17347","updated":"2023-04-25 15:27:09.000000000","message":"Fair enough, turning my vote so.","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"2ff613dd27977e916a3e5239cc466a739a1ee2e2","unresolved":true,"context_lines":[{"line_number":6177,"context_line":"            # unless swap is shrinking to 0 we should update"},{"line_number":6178,"context_line":"            # bdm as per new flavor swap"},{"line_number":6179,"context_line":"            bdm_dict \u003d block_device.create_blank_bdm("},{"line_number":6180,"context_line":"                instance.new_flavor.swap, \u0027swap\u0027)"},{"line_number":6181,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":6182,"context_line":"                context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":6183,"context_line":"            bdm_obj.update_or_create()"}],"source_content_type":"text/x-python","patch_set":24,"id":"3c4eb746_3a591465","line":6180,"in_reply_to":"96022c0c_501328f0","updated":"2023-05-03 10:28:34.000000000","message":"@melwitt, make a copy before adding the swap to bdm, do you mean we should do like below?\n\n```\nbdms_copy \u003d copy.deepcopy(bdms)\nself._update_bdm_on_swap_resize(contenxt, instance, bdms)     // this fucntionality we added updates BDM\n\nself._update_volume_attachments(context, instance, bdms)   // as bdms is updated one\n```\n\nalthough as mentioned in another comment (above), all tests pass even if we do not create fresh bdm.","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e1742a408581c589e173765c514d25bdf37e9698","unresolved":true,"context_lines":[{"line_number":6177,"context_line":"            # unless swap is shrinking to 0 we should update"},{"line_number":6178,"context_line":"            # bdm as per new flavor swap"},{"line_number":6179,"context_line":"            bdm_dict \u003d block_device.create_blank_bdm("},{"line_number":6180,"context_line":"                instance.new_flavor.swap, \u0027swap\u0027)"},{"line_number":6181,"context_line":"            bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":6182,"context_line":"                context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":6183,"context_line":"            bdm_obj.update_or_create()"}],"source_content_type":"text/x-python","patch_set":24,"id":"96022c0c_501328f0","line":6180,"in_reply_to":"e333722b_71481e98","updated":"2023-04-25 21:24:18.000000000","message":"Eh ... I was wary of mutating the \u0027bdms\u0027 variable passed into this method as introducing a side effect. So I would still make a copy of the list first before adding the swap bdm to the copy and not re-query the database.","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"61dfb9b82e3405c50be739d9fb1ef0575621c49a","unresolved":true,"context_lines":[{"line_number":4981,"context_line":"        # Otherwise, the instance XML will not be generated correctly"},{"line_number":4982,"context_line":"        # during a hard reboot or stop/start."},{"line_number":4983,"context_line":"        updated \u003d False"},{"line_number":4984,"context_line":"        if revert and instance.new_flavor.obj_attr_is_set("},{"line_number":4985,"context_line":"            \u0027swap\u0027) and instance.old_flavor.obj_attr_is_set(\u0027swap\u0027):"},{"line_number":4986,"context_line":"            if instance.new_flavor.swap and not instance.old_flavor.swap:"},{"line_number":4987,"context_line":"                # revert expand"}],"source_content_type":"text/x-python","patch_set":26,"id":"f7bb36d9_c3a2d30c","line":4984,"range":{"start_line":4984,"start_character":42,"end_line":4984,"end_character":57},"updated":"2023-06-13 20:57:07.000000000","message":"removed obj_attr_is_set, as checking for flavours before coming here to update bdm.","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"edb4d59d96bcd481fbc87632dfa572aa0c11f1c0","unresolved":false,"context_lines":[{"line_number":4981,"context_line":"        # Otherwise, the instance XML will not be generated correctly"},{"line_number":4982,"context_line":"        # during a hard reboot or stop/start."},{"line_number":4983,"context_line":"        updated \u003d False"},{"line_number":4984,"context_line":"        if revert and instance.new_flavor.obj_attr_is_set("},{"line_number":4985,"context_line":"            \u0027swap\u0027) and instance.old_flavor.obj_attr_is_set(\u0027swap\u0027):"},{"line_number":4986,"context_line":"            if instance.new_flavor.swap and not instance.old_flavor.swap:"},{"line_number":4987,"context_line":"                # revert expand"}],"source_content_type":"text/x-python","patch_set":26,"id":"e283f3a3_daebc2bc","line":4984,"range":{"start_line":4984,"start_character":42,"end_line":4984,"end_character":57},"in_reply_to":"f7bb36d9_c3a2d30c","updated":"2023-07-14 16:54:56.000000000","message":"Done","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9e0c6dc7914c296478979166166ac609bace0132","unresolved":true,"context_lines":[{"line_number":4981,"context_line":"        # Otherwise, the instance XML will not be generated correctly"},{"line_number":4982,"context_line":"        # during a hard reboot or stop/start."},{"line_number":4983,"context_line":"        updated \u003d False"},{"line_number":4984,"context_line":"        if revert and instance.new_flavor.obj_attr_is_set("},{"line_number":4985,"context_line":"            \u0027swap\u0027) and instance.old_flavor.obj_attr_is_set(\u0027swap\u0027):"},{"line_number":4986,"context_line":"            if instance.new_flavor.swap and not instance.old_flavor.swap:"},{"line_number":4987,"context_line":"                # revert expand"},{"line_number":4988,"context_line":"                swap_bdms \u003d ["}],"source_content_type":"text/x-python","patch_set":26,"id":"f10ad6e7_30676ccb","line":4985,"range":{"start_line":4984,"start_character":11,"end_line":4985,"end_character":67},"updated":"2023-06-12 18:29:13.000000000","message":"this is more complex then i woudl like.\n\ni would first start by pulling our some simple checks and exitiign early if niether have swap\n\n```\nold_flavor_has_swap \u003d \u0027swap\u0027 in instance.old_flavor\nnew_flavor_has_swap \u003d \u0027swap\u0027 in instance.new_flavor\n\nif not old_flavor_has_swap or new_flavor_has_swap:\n  # enither flavor has swap so early exit.\n  return bdms\n```\n\nthen store the swap field content if its set\n\n```\nold_flavor_swap \u003d old_flavor_has_swap and instance.old_flavor.swap\nnew_flavor_swap \u003d new_flavor_has_swap and instance.new_flavor.swap\n```\nnot those files will either hold false or the object if its present so you can safely use them in an if and if the iff passes you can use them as a swap object.\n\nand proveed with the initall check \n\n```\nif revert and old_flavor_has_swap and new_flavor_has_swap:\n\n```\n\nby the way i find doign the revert case first ot be a littel more confusing then the happy path.","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"48cc3edf303150ba3a83c39584f038056493d45f","unresolved":false,"context_lines":[{"line_number":4981,"context_line":"        # Otherwise, the instance XML will not be generated correctly"},{"line_number":4982,"context_line":"        # during a hard reboot or stop/start."},{"line_number":4983,"context_line":"        updated \u003d False"},{"line_number":4984,"context_line":"        if revert and instance.new_flavor.obj_attr_is_set("},{"line_number":4985,"context_line":"            \u0027swap\u0027) and instance.old_flavor.obj_attr_is_set(\u0027swap\u0027):"},{"line_number":4986,"context_line":"            if instance.new_flavor.swap and not instance.old_flavor.swap:"},{"line_number":4987,"context_line":"                # revert expand"},{"line_number":4988,"context_line":"                swap_bdms \u003d ["}],"source_content_type":"text/x-python","patch_set":26,"id":"fa3702eb_432a2a0d","line":4985,"range":{"start_line":4984,"start_character":11,"end_line":4985,"end_character":67},"in_reply_to":"07f71f56_88ae7d48","updated":"2023-07-19 07:59:09.000000000","message":"Done","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"edb4d59d96bcd481fbc87632dfa572aa0c11f1c0","unresolved":true,"context_lines":[{"line_number":4981,"context_line":"        # Otherwise, the instance XML will not be generated correctly"},{"line_number":4982,"context_line":"        # during a hard reboot or stop/start."},{"line_number":4983,"context_line":"        updated \u003d False"},{"line_number":4984,"context_line":"        if revert and instance.new_flavor.obj_attr_is_set("},{"line_number":4985,"context_line":"            \u0027swap\u0027) and instance.old_flavor.obj_attr_is_set(\u0027swap\u0027):"},{"line_number":4986,"context_line":"            if instance.new_flavor.swap and not instance.old_flavor.swap:"},{"line_number":4987,"context_line":"                # revert expand"},{"line_number":4988,"context_line":"                swap_bdms \u003d ["}],"source_content_type":"text/x-python","patch_set":26,"id":"07f71f56_88ae7d48","line":4985,"range":{"start_line":4984,"start_character":11,"end_line":4985,"end_character":67},"in_reply_to":"f10ad6e7_30676ccb","updated":"2023-07-14 16:54:56.000000000","message":"mostly followed from here.","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9e0c6dc7914c296478979166166ac609bace0132","unresolved":true,"context_lines":[{"line_number":4983,"context_line":"        updated \u003d False"},{"line_number":4984,"context_line":"        if revert and instance.new_flavor.obj_attr_is_set("},{"line_number":4985,"context_line":"            \u0027swap\u0027) and instance.old_flavor.obj_attr_is_set(\u0027swap\u0027):"},{"line_number":4986,"context_line":"            if instance.new_flavor.swap and not instance.old_flavor.swap:"},{"line_number":4987,"context_line":"                # revert expand"},{"line_number":4988,"context_line":"                swap_bdms \u003d ["},{"line_number":4989,"context_line":"                    bdm for bdm in bdms if bdm.guest_format \u003d\u003d \u0027swap\u0027]"}],"source_content_type":"text/x-python","patch_set":26,"id":"c426a82f_00543d08","line":4986,"range":{"start_line":4986,"start_character":12,"end_line":4986,"end_character":73},"updated":"2023-06-12 18:29:13.000000000","message":"```\nif new_flavor_swap and not old_flavor_swap:\n```","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"edb4d59d96bcd481fbc87632dfa572aa0c11f1c0","unresolved":false,"context_lines":[{"line_number":4983,"context_line":"        updated \u003d False"},{"line_number":4984,"context_line":"        if revert and instance.new_flavor.obj_attr_is_set("},{"line_number":4985,"context_line":"            \u0027swap\u0027) and instance.old_flavor.obj_attr_is_set(\u0027swap\u0027):"},{"line_number":4986,"context_line":"            if instance.new_flavor.swap and not instance.old_flavor.swap:"},{"line_number":4987,"context_line":"                # revert expand"},{"line_number":4988,"context_line":"                swap_bdms \u003d ["},{"line_number":4989,"context_line":"                    bdm for bdm in bdms if bdm.guest_format \u003d\u003d \u0027swap\u0027]"}],"source_content_type":"text/x-python","patch_set":26,"id":"9d8d7001_eb9a8b0d","line":4986,"range":{"start_line":4986,"start_character":12,"end_line":4986,"end_character":73},"in_reply_to":"c426a82f_00543d08","updated":"2023-07-14 16:54:56.000000000","message":"Ack","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9e0c6dc7914c296478979166166ac609bace0132","unresolved":true,"context_lines":[{"line_number":4990,"context_line":"                if swap_bdms:"},{"line_number":4991,"context_line":"                    swap_bdms[0].destroy()"},{"line_number":4992,"context_line":"                    updated \u003d True"},{"line_number":4993,"context_line":"            elif instance.old_flavor.swap:"},{"line_number":4994,"context_line":"                # revert shrink, if earlier swap was present"},{"line_number":4995,"context_line":"                bdm_dict \u003d block_device.create_blank_bdm("},{"line_number":4996,"context_line":"                    instance.old_flavor.swap, \u0027swap\u0027)"}],"source_content_type":"text/x-python","patch_set":26,"id":"05c28e1e_6f8463b1","line":4993,"range":{"start_line":4993,"start_character":12,"end_line":4993,"end_character":42},"updated":"2023-06-12 18:29:13.000000000","message":"```\nelif old_flavor_swap:\n```","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"edb4d59d96bcd481fbc87632dfa572aa0c11f1c0","unresolved":false,"context_lines":[{"line_number":4990,"context_line":"                if swap_bdms:"},{"line_number":4991,"context_line":"                    swap_bdms[0].destroy()"},{"line_number":4992,"context_line":"                    updated \u003d True"},{"line_number":4993,"context_line":"            elif instance.old_flavor.swap:"},{"line_number":4994,"context_line":"                # revert shrink, if earlier swap was present"},{"line_number":4995,"context_line":"                bdm_dict \u003d block_device.create_blank_bdm("},{"line_number":4996,"context_line":"                    instance.old_flavor.swap, \u0027swap\u0027)"}],"source_content_type":"text/x-python","patch_set":26,"id":"6da2f02b_006989bc","line":4993,"range":{"start_line":4993,"start_character":12,"end_line":4993,"end_character":42},"in_reply_to":"05c28e1e_6f8463b1","updated":"2023-07-14 16:54:56.000000000","message":"Ack","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9e0c6dc7914c296478979166166ac609bace0132","unresolved":true,"context_lines":[{"line_number":4999,"context_line":"                bdm_obj.update_or_create()"},{"line_number":5000,"context_line":"                updated \u003d True"},{"line_number":5001,"context_line":""},{"line_number":5002,"context_line":"        else:"},{"line_number":5003,"context_line":"            if not instance.new_flavor.swap and instance.old_flavor.swap:"},{"line_number":5004,"context_line":"                # shrink swap"},{"line_number":5005,"context_line":"                swap_bdms \u003d [bdm for bdm in bdms if bdm.guest_format \u003d\u003d \u0027swap\u0027]"}],"source_content_type":"text/x-python","patch_set":26,"id":"7cef2e9f_97e8667d","line":5002,"range":{"start_line":5002,"start_character":8,"end_line":5002,"end_character":12},"updated":"2023-06-12 18:29:13.000000000","message":"here you dont know if instance.new_flavor.swap or instance.old_flavor.swap\nare defiend in the case that revert\u003dfalse\n\nso with your current approch you need check for that in the ifs  within","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"dbf53a3a5eb6d8aa2ca44579300eead4635fba83","unresolved":false,"context_lines":[{"line_number":4999,"context_line":"                bdm_obj.update_or_create()"},{"line_number":5000,"context_line":"                updated \u003d True"},{"line_number":5001,"context_line":""},{"line_number":5002,"context_line":"        else:"},{"line_number":5003,"context_line":"            if not instance.new_flavor.swap and instance.old_flavor.swap:"},{"line_number":5004,"context_line":"                # shrink swap"},{"line_number":5005,"context_line":"                swap_bdms \u003d [bdm for bdm in bdms if bdm.guest_format \u003d\u003d \u0027swap\u0027]"}],"source_content_type":"text/x-python","patch_set":26,"id":"f03d3f85_594f2cb0","line":5002,"range":{"start_line":5002,"start_character":8,"end_line":5002,"end_character":12},"in_reply_to":"7cef2e9f_97e8667d","updated":"2023-07-17 13:02:04.000000000","message":"Done","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9e0c6dc7914c296478979166166ac609bace0132","unresolved":true,"context_lines":[{"line_number":5000,"context_line":"                updated \u003d True"},{"line_number":5001,"context_line":""},{"line_number":5002,"context_line":"        else:"},{"line_number":5003,"context_line":"            if not instance.new_flavor.swap and instance.old_flavor.swap:"},{"line_number":5004,"context_line":"                # shrink swap"},{"line_number":5005,"context_line":"                swap_bdms \u003d [bdm for bdm in bdms if bdm.guest_format \u003d\u003d \u0027swap\u0027]"},{"line_number":5006,"context_line":"                if swap_bdms:"}],"source_content_type":"text/x-python","patch_set":26,"id":"ff2b2580_bbf6bb50","line":5003,"range":{"start_line":5003,"start_character":11,"end_line":5003,"end_character":73},"updated":"2023-06-12 18:29:13.000000000","message":"but if you use the pulled out vairbales you can just do \n\n```\nif not new_flavor_swap and old_flavor_swap:\n```","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"edb4d59d96bcd481fbc87632dfa572aa0c11f1c0","unresolved":false,"context_lines":[{"line_number":5000,"context_line":"                updated \u003d True"},{"line_number":5001,"context_line":""},{"line_number":5002,"context_line":"        else:"},{"line_number":5003,"context_line":"            if not instance.new_flavor.swap and instance.old_flavor.swap:"},{"line_number":5004,"context_line":"                # shrink swap"},{"line_number":5005,"context_line":"                swap_bdms \u003d [bdm for bdm in bdms if bdm.guest_format \u003d\u003d \u0027swap\u0027]"},{"line_number":5006,"context_line":"                if swap_bdms:"}],"source_content_type":"text/x-python","patch_set":26,"id":"942bf26b_493a3c42","line":5003,"range":{"start_line":5003,"start_character":11,"end_line":5003,"end_character":73},"in_reply_to":"ff2b2580_bbf6bb50","updated":"2023-07-14 16:54:56.000000000","message":"Ack","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9e0c6dc7914c296478979166166ac609bace0132","unresolved":true,"context_lines":[{"line_number":5006,"context_line":"                if swap_bdms:"},{"line_number":5007,"context_line":"                    swap_bdms[0].destroy()"},{"line_number":5008,"context_line":"                    updated \u003d True"},{"line_number":5009,"context_line":"            elif instance.new_flavor.swap:"},{"line_number":5010,"context_line":"                # unless swap is shrinking to 0 we should update"},{"line_number":5011,"context_line":"                # bdm as per new flavor swap"},{"line_number":5012,"context_line":"                bdm_dict \u003d block_device.create_blank_bdm("}],"source_content_type":"text/x-python","patch_set":26,"id":"2b3c07bc_3f699f21","line":5009,"range":{"start_line":5009,"start_character":17,"end_line":5009,"end_character":41},"updated":"2023-06-12 18:29:13.000000000","message":"elif new_flavor_swap:","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"edb4d59d96bcd481fbc87632dfa572aa0c11f1c0","unresolved":false,"context_lines":[{"line_number":5006,"context_line":"                if swap_bdms:"},{"line_number":5007,"context_line":"                    swap_bdms[0].destroy()"},{"line_number":5008,"context_line":"                    updated \u003d True"},{"line_number":5009,"context_line":"            elif instance.new_flavor.swap:"},{"line_number":5010,"context_line":"                # unless swap is shrinking to 0 we should update"},{"line_number":5011,"context_line":"                # bdm as per new flavor swap"},{"line_number":5012,"context_line":"                bdm_dict \u003d block_device.create_blank_bdm("}],"source_content_type":"text/x-python","patch_set":26,"id":"2cad7fa3_5d912077","line":5009,"range":{"start_line":5009,"start_character":17,"end_line":5009,"end_character":41},"in_reply_to":"2b3c07bc_3f699f21","updated":"2023-07-14 16:54:56.000000000","message":"Ack","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9e0c6dc7914c296478979166166ac609bace0132","unresolved":true,"context_lines":[{"line_number":5015,"context_line":"                    context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":5016,"context_line":"                bdm_obj.update_or_create()"},{"line_number":5017,"context_line":"                updated \u003d True"},{"line_number":5018,"context_line":""},{"line_number":5019,"context_line":"        if updated:"},{"line_number":5020,"context_line":"            # retrieve fresh copy in case there is bdm update"},{"line_number":5021,"context_line":"            # original bdm values which were passed by caller"}],"source_content_type":"text/x-python","patch_set":26,"id":"6fe26429_cbaf5041","line":5018,"updated":"2023-06-12 18:29:13.000000000","message":"i dont hink you have actully covered all uses cases here..\n\nif we are confirming the migration and the old flavor had swap but the new flavor does not\n\nthe we dont cover that.\n\ni.e. revert\u003dfalse old_flavor_swap\u003d10 new_flavor_swap\u003d0\n\nthen i guess you ahve that covered with \n\nif not instance.new_flavor.swap and instance.old_flavor.swap:\nbut that is relying on not 0 \u003d\u003d ture\n\nwe  3 parmaters  so 8 possibel options.\n\nif both the old and new flavor dont have swap that reduces it to 6\nif both the old and new flaovr have swap that reduces it to 4.\nyou have 4 combinaiton here so that should be enough but the way the ifs are nested makes thsi harder to follow then if you didnt nest them.","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"61dfb9b82e3405c50be739d9fb1ef0575621c49a","unresolved":true,"context_lines":[{"line_number":5015,"context_line":"                    context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":5016,"context_line":"                bdm_obj.update_or_create()"},{"line_number":5017,"context_line":"                updated \u003d True"},{"line_number":5018,"context_line":""},{"line_number":5019,"context_line":"        if updated:"},{"line_number":5020,"context_line":"            # retrieve fresh copy in case there is bdm update"},{"line_number":5021,"context_line":"            # original bdm values which were passed by caller"}],"source_content_type":"text/x-python","patch_set":26,"id":"e3bd2e0d_9bec31d6","line":5018,"in_reply_to":"6fe26429_cbaf5041","updated":"2023-06-13 20:57:07.000000000","message":"`revert\u003dfalse old_flavor_swap\u003d10 new_flavor_swap\u003d0`\nthis is covered, updated variable name to be more clear.\n\n`if both the old and new flaovr have swap that reduces it to 4.`\nyes","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"dbf53a3a5eb6d8aa2ca44579300eead4635fba83","unresolved":false,"context_lines":[{"line_number":5015,"context_line":"                    context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":5016,"context_line":"                bdm_obj.update_or_create()"},{"line_number":5017,"context_line":"                updated \u003d True"},{"line_number":5018,"context_line":""},{"line_number":5019,"context_line":"        if updated:"},{"line_number":5020,"context_line":"            # retrieve fresh copy in case there is bdm update"},{"line_number":5021,"context_line":"            # original bdm values which were passed by caller"}],"source_content_type":"text/x-python","patch_set":26,"id":"2d9b4970_8a77155f","line":5018,"in_reply_to":"e3bd2e0d_9bec31d6","updated":"2023-07-17 13:02:04.000000000","message":"Done","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b406bb455757a6365143538d64313b3a51e70911","unresolved":true,"context_lines":[{"line_number":5019,"context_line":"        if updated:"},{"line_number":5020,"context_line":"            # retrieve fresh copy in case there is bdm update"},{"line_number":5021,"context_line":"            # original bdm values which were passed by caller"},{"line_number":5022,"context_line":"            # should not get updated."},{"line_number":5023,"context_line":"            return objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5024,"context_line":"                context, instance.uuid)"},{"line_number":5025,"context_line":"        return bdms"}],"source_content_type":"text/x-python","patch_set":26,"id":"7e49d8cb_d3e8864d","line":5022,"updated":"2023-06-12 18:08:47.000000000","message":"I don\u0027t understand why you\u0027re doing this. If you made changes, you literally have the changes right here, don\u0027t you? Why create a thing in a database and then query the whole list when you\u0027ve got that object that you can just insert into (or remove from) the list?","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"d290b48f8b2ec5fd27d81a20e2bb1e1d5960374b","unresolved":true,"context_lines":[{"line_number":5019,"context_line":"        if updated:"},{"line_number":5020,"context_line":"            # retrieve fresh copy in case there is bdm update"},{"line_number":5021,"context_line":"            # original bdm values which were passed by caller"},{"line_number":5022,"context_line":"            # should not get updated."},{"line_number":5023,"context_line":"            return objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5024,"context_line":"                context, instance.uuid)"},{"line_number":5025,"context_line":"        return bdms"}],"source_content_type":"text/x-python","patch_set":26,"id":"2aa3edf5_d5848f19","line":5022,"in_reply_to":"263b7ee5_17bc8810","updated":"2023-06-13 20:59:30.000000000","message":"ERROR: libvirt.libvirtError: Cannot access storage file: instance_data_path/disk.swap\n\nthat mean, bdm does not updated.","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"edb4d59d96bcd481fbc87632dfa572aa0c11f1c0","unresolved":true,"context_lines":[{"line_number":5019,"context_line":"        if updated:"},{"line_number":5020,"context_line":"            # retrieve fresh copy in case there is bdm update"},{"line_number":5021,"context_line":"            # original bdm values which were passed by caller"},{"line_number":5022,"context_line":"            # should not get updated."},{"line_number":5023,"context_line":"            return objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5024,"context_line":"                context, instance.uuid)"},{"line_number":5025,"context_line":"        return bdms"}],"source_content_type":"text/x-python","patch_set":26,"id":"8d035ead_69fc7658","line":5022,"in_reply_to":"2aa3edf5_d5848f19","updated":"2023-07-14 16:54:56.000000000","message":"here I am adding or updating only one bdm object. so either append bdm_object in bdms or retrieve new list.","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"61dfb9b82e3405c50be739d9fb1ef0575621c49a","unresolved":true,"context_lines":[{"line_number":5019,"context_line":"        if updated:"},{"line_number":5020,"context_line":"            # retrieve fresh copy in case there is bdm update"},{"line_number":5021,"context_line":"            # original bdm values which were passed by caller"},{"line_number":5022,"context_line":"            # should not get updated."},{"line_number":5023,"context_line":"            return objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5024,"context_line":"                context, instance.uuid)"},{"line_number":5025,"context_line":"        return bdms"}],"source_content_type":"text/x-python","patch_set":26,"id":"263b7ee5_17bc8810","line":5022,"in_reply_to":"7e49d8cb_d3e8864d","updated":"2023-06-13 20:57:07.000000000","message":"if we do not retrieve the bdms from DB, and send back the same bdms object, resize fails in actual manual test (while resize via OSC) I do not know the actual reason why. \nAutomated functional tests passed though.","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"48cc3edf303150ba3a83c39584f038056493d45f","unresolved":false,"context_lines":[{"line_number":5019,"context_line":"        if updated:"},{"line_number":5020,"context_line":"            # retrieve fresh copy in case there is bdm update"},{"line_number":5021,"context_line":"            # original bdm values which were passed by caller"},{"line_number":5022,"context_line":"            # should not get updated."},{"line_number":5023,"context_line":"            return objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5024,"context_line":"                context, instance.uuid)"},{"line_number":5025,"context_line":"        return bdms"}],"source_content_type":"text/x-python","patch_set":26,"id":"40e5d8fd_01856e22","line":5022,"in_reply_to":"8d035ead_69fc7658","updated":"2023-07-19 07:59:09.000000000","message":"Done","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"b406bb455757a6365143538d64313b3a51e70911","unresolved":true,"context_lines":[{"line_number":5413,"context_line":"            bdms_copy \u003d copy.deepcopy(bdms)"},{"line_number":5414,"context_line":"            if instance.new_flavor and instance.old_flavor:"},{"line_number":5415,"context_line":"                bdms_copy \u003d self._update_bdm_on_swap_resize("},{"line_number":5416,"context_line":"                    context, instance, bdms, revert\u003dTrue)"},{"line_number":5417,"context_line":""},{"line_number":5418,"context_line":"            # revert_resize deleted any volume attachments for the instance"},{"line_number":5419,"context_line":"            # and created new ones to be used on this host, but we"}],"source_content_type":"text/x-python","patch_set":26,"id":"f8ffb5d9_d5ece354","line":5416,"updated":"2023-06-12 18:08:47.000000000","message":"Since you\u0027re passing `bdms` here, you just wasted a deep copy on L5413 that you just throw away here. Per my comment above, I think you should be passing in the copy here and mutating the list to add/remove/update what is needed and then use the result. Isn\u0027t that the whole point of making the copy?","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"61dfb9b82e3405c50be739d9fb1ef0575621c49a","unresolved":false,"context_lines":[{"line_number":5413,"context_line":"            bdms_copy \u003d copy.deepcopy(bdms)"},{"line_number":5414,"context_line":"            if instance.new_flavor and instance.old_flavor:"},{"line_number":5415,"context_line":"                bdms_copy \u003d self._update_bdm_on_swap_resize("},{"line_number":5416,"context_line":"                    context, instance, bdms, revert\u003dTrue)"},{"line_number":5417,"context_line":""},{"line_number":5418,"context_line":"            # revert_resize deleted any volume attachments for the instance"},{"line_number":5419,"context_line":"            # and created new ones to be used on this host, but we"}],"source_content_type":"text/x-python","patch_set":26,"id":"0ba377e2_afcb1d35","line":5416,"in_reply_to":"f8ffb5d9_d5ece354","updated":"2023-06-13 20:57:07.000000000","message":"Ack","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c9617dd70ed2de7809ed0b95c8af0e05ccce2f00","unresolved":true,"context_lines":[{"line_number":4987,"context_line":"        - second while confirming or reverting resize."},{"line_number":4988,"context_line":"        \"\"\""},{"line_number":4989,"context_line":"        expand \u003d instance.new_flavor.swap \u003e instance.old_flavor.swap"},{"line_number":4990,"context_line":"        shrink \u003d not expand"},{"line_number":4991,"context_line":""},{"line_number":4992,"context_line":"        if resize_confirm:"},{"line_number":4993,"context_line":"            if shrink and not instance.new_flavor.swap:"}],"source_content_type":"text/x-python","patch_set":27,"id":"95f993ac_8c29b6aa","line":4990,"updated":"2023-06-13 22:37:07.000000000","message":"I\u0027m not sure the name of this variable is helpful ... shrink will be True if new_flavor.swap \u003d\u003d old_flavor.swap.\n\nAlso, his function (_update_bdm_on_swap_resize) is currently being called for all resize cases, not just cases that involve swap. I think it should only be called for cases involving swap and more specifically cases in which swap is being added or removed, no?","commit_id":"a67e2908f21b2bf847d8e6c74f16ae4637262be6"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"48cc3edf303150ba3a83c39584f038056493d45f","unresolved":false,"context_lines":[{"line_number":4987,"context_line":"        - second while confirming or reverting resize."},{"line_number":4988,"context_line":"        \"\"\""},{"line_number":4989,"context_line":"        expand \u003d instance.new_flavor.swap \u003e instance.old_flavor.swap"},{"line_number":4990,"context_line":"        shrink \u003d not expand"},{"line_number":4991,"context_line":""},{"line_number":4992,"context_line":"        if resize_confirm:"},{"line_number":4993,"context_line":"            if shrink and not instance.new_flavor.swap:"}],"source_content_type":"text/x-python","patch_set":27,"id":"5dd389ca_5c66416c","line":4990,"in_reply_to":"74890f9b_875ba3a8","updated":"2023-07-19 07:59:09.000000000","message":"Done","commit_id":"a67e2908f21b2bf847d8e6c74f16ae4637262be6"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"edb4d59d96bcd481fbc87632dfa572aa0c11f1c0","unresolved":true,"context_lines":[{"line_number":4987,"context_line":"        - second while confirming or reverting resize."},{"line_number":4988,"context_line":"        \"\"\""},{"line_number":4989,"context_line":"        expand \u003d instance.new_flavor.swap \u003e instance.old_flavor.swap"},{"line_number":4990,"context_line":"        shrink \u003d not expand"},{"line_number":4991,"context_line":""},{"line_number":4992,"context_line":"        if resize_confirm:"},{"line_number":4993,"context_line":"            if shrink and not instance.new_flavor.swap:"}],"source_content_type":"text/x-python","patch_set":27,"id":"74890f9b_875ba3a8","line":4990,"in_reply_to":"95f993ac_8c29b6aa","updated":"2023-07-14 16:54:56.000000000","message":"ack, removed.\nyes, its called everytime. \nI have added \u0027swap\u0027 check inside update function (it was suggested earlier by @Sean). the reason of not doing this check at caller is if some other function calls, there we do not need check for it again,","commit_id":"a67e2908f21b2bf847d8e6c74f16ae4637262be6"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c9617dd70ed2de7809ed0b95c8af0e05ccce2f00","unresolved":true,"context_lines":[{"line_number":4990,"context_line":"        shrink \u003d not expand"},{"line_number":4991,"context_line":""},{"line_number":4992,"context_line":"        if resize_confirm:"},{"line_number":4993,"context_line":"            if shrink and not instance.new_flavor.swap:"},{"line_number":4994,"context_line":"                # on 100% shrink"},{"line_number":4995,"context_line":"                # 1024, 0       -\u003e 0"},{"line_number":4996,"context_line":"                swap_bdms \u003d [bdm for bdm in bdms if bdm.guest_format \u003d\u003d \u0027swap\u0027]"}],"source_content_type":"text/x-python","patch_set":27,"id":"55c5c0cb_e31419ce","line":4993,"updated":"2023-06-13 22:37:07.000000000","message":"Note to self: This code block will be entered if old_flavor.swap \u003d 0 and new_flavor.swap \u003d 0.","commit_id":"a67e2908f21b2bf847d8e6c74f16ae4637262be6"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"48cc3edf303150ba3a83c39584f038056493d45f","unresolved":false,"context_lines":[{"line_number":4990,"context_line":"        shrink \u003d not expand"},{"line_number":4991,"context_line":""},{"line_number":4992,"context_line":"        if resize_confirm:"},{"line_number":4993,"context_line":"            if shrink and not instance.new_flavor.swap:"},{"line_number":4994,"context_line":"                # on 100% shrink"},{"line_number":4995,"context_line":"                # 1024, 0       -\u003e 0"},{"line_number":4996,"context_line":"                swap_bdms \u003d [bdm for bdm in bdms if bdm.guest_format \u003d\u003d \u0027swap\u0027]"}],"source_content_type":"text/x-python","patch_set":27,"id":"e5490a58_eefa9f88","line":4993,"in_reply_to":"55c5c0cb_e31419ce","updated":"2023-07-19 07:59:09.000000000","message":"Done","commit_id":"a67e2908f21b2bf847d8e6c74f16ae4637262be6"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c9617dd70ed2de7809ed0b95c8af0e05ccce2f00","unresolved":true,"context_lines":[{"line_number":5006,"context_line":"                    instance.new_flavor.swap, \u0027swap\u0027)"},{"line_number":5007,"context_line":"                bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5008,"context_line":"                    context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":5009,"context_line":"                bdm_obj.update_or_create()"},{"line_number":5010,"context_line":"        else:"},{"line_number":5011,"context_line":"            if expand and instance.new_flavor.swap and\\"},{"line_number":5012,"context_line":"                    not instance.old_flavor.swap:"}],"source_content_type":"text/x-python","patch_set":27,"id":"eca27251_da658f5f","line":5009,"updated":"2023-06-13 22:37:07.000000000","message":"I still don\u0027t like this, it creates a new BDM record in the database whether swap is being added or not, and it gets \"corrected\" in the DB layer [1]:\n\n```\n    # NOTE(xqueralt): Prevent from having multiple swap devices for the\n    # same instance. This will delete all the existing ones.\n    if block_device.new_format_is_swap(values):\n        query \u003d _block_device_mapping_get_query(context)\n        query \u003d query.filter_by(instance_uuid\u003dvalues[\u0027instance_uuid\u0027],\n                                source_type\u003d\u0027blank\u0027, guest_format\u003d\u0027swap\u0027)\n        query \u003d query.filter(models.BlockDeviceMapping.id !\u003d result.id)\n        query.soft_delete()\n```\n\nSo in cases where the instance had old_flavor.swap \u003e 0, this will insert a new BDM record and then delete the existing BDM record for no good reason, making unnecessary queries to the database.\n\nI also think it\u0027s fragile ... if someone refactored the above code in nova/db/main/api.py and stopped correcting records for swap, there would be an immediate bug in this code that relies on the correction.\n\n[1] https://github.com/openstack/nova/blob/308633f93aef5823ba0e03cb5284ed8f647af7e8/nova/db/main/api.py#L2903","commit_id":"a67e2908f21b2bf847d8e6c74f16ae4637262be6"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"48cc3edf303150ba3a83c39584f038056493d45f","unresolved":false,"context_lines":[{"line_number":5006,"context_line":"                    instance.new_flavor.swap, \u0027swap\u0027)"},{"line_number":5007,"context_line":"                bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5008,"context_line":"                    context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":5009,"context_line":"                bdm_obj.update_or_create()"},{"line_number":5010,"context_line":"        else:"},{"line_number":5011,"context_line":"            if expand and instance.new_flavor.swap and\\"},{"line_number":5012,"context_line":"                    not instance.old_flavor.swap:"}],"source_content_type":"text/x-python","patch_set":27,"id":"c862834e_d5d1ee7c","line":5009,"in_reply_to":"715fbdde_e43aa85a","updated":"2023-07-19 07:59:09.000000000","message":"Done","commit_id":"a67e2908f21b2bf847d8e6c74f16ae4637262be6"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"edb4d59d96bcd481fbc87632dfa572aa0c11f1c0","unresolved":true,"context_lines":[{"line_number":5006,"context_line":"                    instance.new_flavor.swap, \u0027swap\u0027)"},{"line_number":5007,"context_line":"                bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5008,"context_line":"                    context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":5009,"context_line":"                bdm_obj.update_or_create()"},{"line_number":5010,"context_line":"        else:"},{"line_number":5011,"context_line":"            if expand and instance.new_flavor.swap and\\"},{"line_number":5012,"context_line":"                    not instance.old_flavor.swap:"}],"source_content_type":"text/x-python","patch_set":27,"id":"715fbdde_e43aa85a","line":5009,"in_reply_to":"eca27251_da658f5f","updated":"2023-07-14 16:54:56.000000000","message":"ack, updated to only add new bdm object if required otherwise update the existing object swap size.","commit_id":"a67e2908f21b2bf847d8e6c74f16ae4637262be6"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c9617dd70ed2de7809ed0b95c8af0e05ccce2f00","unresolved":true,"context_lines":[{"line_number":5008,"context_line":"                    context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":5009,"context_line":"                bdm_obj.update_or_create()"},{"line_number":5010,"context_line":"        else:"},{"line_number":5011,"context_line":"            if expand and instance.new_flavor.swap and\\"},{"line_number":5012,"context_line":"                    not instance.old_flavor.swap:"},{"line_number":5013,"context_line":"                # revert expand, possible reverts"},{"line_number":5014,"context_line":"                # 0,    1024   -\u003e 0"}],"source_content_type":"text/x-python","patch_set":27,"id":"92191b7e_ac551200","line":5011,"range":{"start_line":5011,"start_character":54,"end_line":5011,"end_character":55},"updated":"2023-06-13 22:37:07.000000000","message":"Style: prefer using parentheses around a multiline statement rather than using backslashes:\n\nhttps://peps.python.org/pep-0008/#maximum-line-length","commit_id":"a67e2908f21b2bf847d8e6c74f16ae4637262be6"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"edb4d59d96bcd481fbc87632dfa572aa0c11f1c0","unresolved":false,"context_lines":[{"line_number":5008,"context_line":"                    context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":5009,"context_line":"                bdm_obj.update_or_create()"},{"line_number":5010,"context_line":"        else:"},{"line_number":5011,"context_line":"            if expand and instance.new_flavor.swap and\\"},{"line_number":5012,"context_line":"                    not instance.old_flavor.swap:"},{"line_number":5013,"context_line":"                # revert expand, possible reverts"},{"line_number":5014,"context_line":"                # 0,    1024   -\u003e 0"}],"source_content_type":"text/x-python","patch_set":27,"id":"a0d1f0c7_9c6a3485","line":5011,"range":{"start_line":5011,"start_character":54,"end_line":5011,"end_character":55},"in_reply_to":"92191b7e_ac551200","updated":"2023-07-14 16:54:56.000000000","message":"Ack","commit_id":"a67e2908f21b2bf847d8e6c74f16ae4637262be6"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"c9617dd70ed2de7809ed0b95c8af0e05ccce2f00","unresolved":true,"context_lines":[{"line_number":5025,"context_line":"                    instance.old_flavor.swap, \u0027swap\u0027)"},{"line_number":5026,"context_line":"                bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5027,"context_line":"                    context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":5028,"context_line":"                bdm_obj.update_or_create()"},{"line_number":5029,"context_line":""},{"line_number":5030,"context_line":"        return objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5031,"context_line":"            context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":27,"id":"f9284341_7b3f7757","line":5028,"updated":"2023-06-13 22:37:07.000000000","message":"Same issue here with unnecessary database queries.\n\nI think this could also possibly cause problems when you were trying to adjust the BDM list copy in memory to avoid querying for the entire BDM list again on L5030. Reason being, in cases where the DB layer \"correction\" runs, it will put the contents of the database out of sync with your list, at least from a BDM UUID perspective (the BDM record created in the database will have a new UUID different from the existing/old BDM record).","commit_id":"a67e2908f21b2bf847d8e6c74f16ae4637262be6"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"262df9b6254b0b8f9c0c16d22234803e9a4f914c","unresolved":false,"context_lines":[{"line_number":5025,"context_line":"                    instance.old_flavor.swap, \u0027swap\u0027)"},{"line_number":5026,"context_line":"                bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5027,"context_line":"                    context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":5028,"context_line":"                bdm_obj.update_or_create()"},{"line_number":5029,"context_line":""},{"line_number":5030,"context_line":"        return objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5031,"context_line":"            context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":27,"id":"b281b08f_5df4f927","line":5028,"in_reply_to":"d54a0f98_c4ca9640","updated":"2024-02-13 10:38:44.000000000","message":"Acknowledged","commit_id":"a67e2908f21b2bf847d8e6c74f16ae4637262be6"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9694c6b015e415b0d5e21b71994803d642a691a5","unresolved":true,"context_lines":[{"line_number":5025,"context_line":"                    instance.old_flavor.swap, \u0027swap\u0027)"},{"line_number":5026,"context_line":"                bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5027,"context_line":"                    context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":5028,"context_line":"                bdm_obj.update_or_create()"},{"line_number":5029,"context_line":""},{"line_number":5030,"context_line":"        return objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5031,"context_line":"            context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":27,"id":"d54a0f98_c4ca9640","line":5028,"in_reply_to":"f55d2148_1b303908","updated":"2023-08-31 16:50:03.000000000","message":"updated the bdm list instead of querying DB.\nsimilar to dangling bdms patch.","commit_id":"a67e2908f21b2bf847d8e6c74f16ae4637262be6"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"edb4d59d96bcd481fbc87632dfa572aa0c11f1c0","unresolved":true,"context_lines":[{"line_number":5025,"context_line":"                    instance.old_flavor.swap, \u0027swap\u0027)"},{"line_number":5026,"context_line":"                bdm_obj \u003d objects.BlockDeviceMapping("},{"line_number":5027,"context_line":"                    context, instance_uuid\u003dinstance.uuid, **bdm_dict)"},{"line_number":5028,"context_line":"                bdm_obj.update_or_create()"},{"line_number":5029,"context_line":""},{"line_number":5030,"context_line":"        return objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5031,"context_line":"            context, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":27,"id":"f55d2148_1b303908","line":5028,"in_reply_to":"f9284341_7b3f7757","updated":"2023-07-14 16:54:56.000000000","message":"Ack","commit_id":"a67e2908f21b2bf847d8e6c74f16ae4637262be6"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"48cc3edf303150ba3a83c39584f038056493d45f","unresolved":true,"context_lines":[{"line_number":5092,"context_line":""},{"line_number":5093,"context_line":"        return objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5094,"context_line":"            context, instance.uuid)"},{"line_number":5095,"context_line":""},{"line_number":5096,"context_line":"    @wrap_exception()"},{"line_number":5097,"context_line":"    @reverts_task_state"},{"line_number":5098,"context_line":"    @wrap_instance_event(prefix\u003d\u0027compute\u0027)"}],"source_content_type":"text/x-python","patch_set":29,"id":"af1693d2_d6550650","line":5095,"updated":"2023-07-19 07:59:09.000000000","message":"here instead of retrieving new bdm list from DB, I could have updated exsting bdms list on adding or deleting the bdm obj.\n\nbut I feel this is less confusing, once process of update is done, get fresh dm list.","commit_id":"8aea4929970d8016a5f6b8eb26fce901d9ec6589"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9694c6b015e415b0d5e21b71994803d642a691a5","unresolved":false,"context_lines":[{"line_number":5092,"context_line":""},{"line_number":5093,"context_line":"        return objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":5094,"context_line":"            context, instance.uuid)"},{"line_number":5095,"context_line":""},{"line_number":5096,"context_line":"    @wrap_exception()"},{"line_number":5097,"context_line":"    @reverts_task_state"},{"line_number":5098,"context_line":"    @wrap_instance_event(prefix\u003d\u0027compute\u0027)"}],"source_content_type":"text/x-python","patch_set":29,"id":"5e61838a_0b3e3895","line":5095,"in_reply_to":"af1693d2_d6550650","updated":"2023-08-31 16:50:03.000000000","message":"Done","commit_id":"8aea4929970d8016a5f6b8eb26fce901d9ec6589"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e9eb3edc70dd57f0e460b5f01f8a482d6103bf70","unresolved":true,"context_lines":[{"line_number":5021,"context_line":"        if not [bdm for bdm in bdms if bdm.obj_attr_is_set(\u0027guest_format\u0027)]:"},{"line_number":5022,"context_line":"            # this means these bdms are part of some test which do not have"},{"line_number":5023,"context_line":"            # all bdm attributes, i.e. \u0027swap\u0027"},{"line_number":5024,"context_line":"            return bdms"},{"line_number":5025,"context_line":""},{"line_number":5026,"context_line":"        old_flavor_has_swap \u003d \u0027swap\u0027 in instance.old_flavor"},{"line_number":5027,"context_line":"        new_flavor_has_swap \u003d \u0027swap\u0027 in instance.new_flavor"}],"source_content_type":"text/x-python","patch_set":31,"id":"ae69c603_4dd8e017","line":5024,"updated":"2023-10-05 12:57:27.000000000","message":"you return some BDMs objects here, but none of the callers I see use those.\n\nWhat\u0027s the exact in and out parameters ? maybe just a nit but you should explain the parameters and the returned objects in the docstring above.","commit_id":"2c1e7adec2c690c86d28cbe85ee725c7c2b3cd60"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"6a8e2adfdc0bc3f141aee18d5ad490f0bb640f62","unresolved":false,"context_lines":[{"line_number":5021,"context_line":"        if not [bdm for bdm in bdms if bdm.obj_attr_is_set(\u0027guest_format\u0027)]:"},{"line_number":5022,"context_line":"            # this means these bdms are part of some test which do not have"},{"line_number":5023,"context_line":"            # all bdm attributes, i.e. \u0027swap\u0027"},{"line_number":5024,"context_line":"            return bdms"},{"line_number":5025,"context_line":""},{"line_number":5026,"context_line":"        old_flavor_has_swap \u003d \u0027swap\u0027 in instance.old_flavor"},{"line_number":5027,"context_line":"        new_flavor_has_swap \u003d \u0027swap\u0027 in instance.new_flavor"}],"source_content_type":"text/x-python","patch_set":31,"id":"5861a3f0_43252b49","line":5024,"in_reply_to":"35483541_1a1e1679","updated":"2024-01-29 06:48:13.000000000","message":"I meant nothing will be accepted. so bdms are not updated because of this statement.\nthanks for noticing, fixed.\n\nadded params and returns in doc string","commit_id":"2c1e7adec2c690c86d28cbe85ee725c7c2b3cd60"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9dc28471c2c3a6e4cad50562fd4cb86a688eb8e7","unresolved":true,"context_lines":[{"line_number":5021,"context_line":"        if not [bdm for bdm in bdms if bdm.obj_attr_is_set(\u0027guest_format\u0027)]:"},{"line_number":5022,"context_line":"            # this means these bdms are part of some test which do not have"},{"line_number":5023,"context_line":"            # all bdm attributes, i.e. \u0027swap\u0027"},{"line_number":5024,"context_line":"            return bdms"},{"line_number":5025,"context_line":""},{"line_number":5026,"context_line":"        old_flavor_has_swap \u003d \u0027swap\u0027 in instance.old_flavor"},{"line_number":5027,"context_line":"        new_flavor_has_swap \u003d \u0027swap\u0027 in instance.new_flavor"}],"source_content_type":"text/x-python","patch_set":31,"id":"35483541_1a1e1679","line":5024,"in_reply_to":"ae69c603_4dd8e017","updated":"2023-10-05 18:08:34.000000000","message":"Ack, will fix it\n\nnothing will be returned as we are not accepting bdms at caller. the object refrence will get updated here in this function and used later.","commit_id":"2c1e7adec2c690c86d28cbe85ee725c7c2b3cd60"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e9eb3edc70dd57f0e460b5f01f8a482d6103bf70","unresolved":true,"context_lines":[{"line_number":5027,"context_line":"        new_flavor_has_swap \u003d \u0027swap\u0027 in instance.new_flavor"},{"line_number":5028,"context_line":""},{"line_number":5029,"context_line":"        if not old_flavor_has_swap and new_flavor_has_swap:"},{"line_number":5030,"context_line":"            # neither flavor has swap so early exit."},{"line_number":5031,"context_line":"            return bdms"},{"line_number":5032,"context_line":""},{"line_number":5033,"context_line":"        def _add_bdm_record(swap_size):"}],"source_content_type":"text/x-python","patch_set":31,"id":"2252371a_f0968b41","line":5030,"updated":"2023-10-05 12:57:27.000000000","message":"that\u0027s not what I can read in the conditional : \n\n\"if not a and b\" means to me \"if A doesn\u0027t exist but B exists\" (you forgot the parenthesis or said \"if not a and not b\"\n\n```\n\u003e\u003e\u003e a \u003d False\n\u003e\u003e\u003e b \u003d True\n\u003e\u003e\u003e not a and b\nTrue\n\u003e\u003e\u003e not a and not b\nFalse\n```","commit_id":"2c1e7adec2c690c86d28cbe85ee725c7c2b3cd60"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"17e35dcb1bb45d989e0de3eec7d8526514cc638e","unresolved":true,"context_lines":[{"line_number":5027,"context_line":"        new_flavor_has_swap \u003d \u0027swap\u0027 in instance.new_flavor"},{"line_number":5028,"context_line":""},{"line_number":5029,"context_line":"        if not old_flavor_has_swap and new_flavor_has_swap:"},{"line_number":5030,"context_line":"            # neither flavor has swap so early exit."},{"line_number":5031,"context_line":"            return bdms"},{"line_number":5032,"context_line":""},{"line_number":5033,"context_line":"        def _add_bdm_record(swap_size):"}],"source_content_type":"text/x-python","patch_set":31,"id":"8d8672f3_2127cd47","line":5030,"in_reply_to":"2252371a_f0968b41","updated":"2024-01-17 05:53:30.000000000","message":"this should be \nif not old_flavor_has_swap and not new_flavor_has_swap:\nOR\nif not (old_flavor_has_swap or new_flavor_has_swap):","commit_id":"2c1e7adec2c690c86d28cbe85ee725c7c2b3cd60"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"6a8e2adfdc0bc3f141aee18d5ad490f0bb640f62","unresolved":false,"context_lines":[{"line_number":5027,"context_line":"        new_flavor_has_swap \u003d \u0027swap\u0027 in instance.new_flavor"},{"line_number":5028,"context_line":""},{"line_number":5029,"context_line":"        if not old_flavor_has_swap and new_flavor_has_swap:"},{"line_number":5030,"context_line":"            # neither flavor has swap so early exit."},{"line_number":5031,"context_line":"            return bdms"},{"line_number":5032,"context_line":""},{"line_number":5033,"context_line":"        def _add_bdm_record(swap_size):"}],"source_content_type":"text/x-python","patch_set":31,"id":"cf699d31_90dc15fe","line":5030,"in_reply_to":"8d8672f3_2127cd47","updated":"2024-01-29 06:48:13.000000000","message":"updated thanks","commit_id":"2c1e7adec2c690c86d28cbe85ee725c7c2b3cd60"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e9eb3edc70dd57f0e460b5f01f8a482d6103bf70","unresolved":true,"context_lines":[{"line_number":5028,"context_line":""},{"line_number":5029,"context_line":"        if not old_flavor_has_swap and new_flavor_has_swap:"},{"line_number":5030,"context_line":"            # neither flavor has swap so early exit."},{"line_number":5031,"context_line":"            return bdms"},{"line_number":5032,"context_line":""},{"line_number":5033,"context_line":"        def _add_bdm_record(swap_size):"},{"line_number":5034,"context_line":"            bdm_dict \u003d block_device.create_blank_bdm("}],"source_content_type":"text/x-python","patch_set":31,"id":"8b9f3771_8d174067","line":5031,"updated":"2023-10-05 12:57:27.000000000","message":"again, I don\u0027t see where we get those returned BDMs.","commit_id":"2c1e7adec2c690c86d28cbe85ee725c7c2b3cd60"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"6a8e2adfdc0bc3f141aee18d5ad490f0bb640f62","unresolved":false,"context_lines":[{"line_number":5028,"context_line":""},{"line_number":5029,"context_line":"        if not old_flavor_has_swap and new_flavor_has_swap:"},{"line_number":5030,"context_line":"            # neither flavor has swap so early exit."},{"line_number":5031,"context_line":"            return bdms"},{"line_number":5032,"context_line":""},{"line_number":5033,"context_line":"        def _add_bdm_record(swap_size):"},{"line_number":5034,"context_line":"            bdm_dict \u003d block_device.create_blank_bdm("}],"source_content_type":"text/x-python","patch_set":31,"id":"d50fb33a_d40e4118","line":5031,"in_reply_to":"11f6b44b_5e41320a","updated":"2024-01-29 06:48:13.000000000","message":"Done","commit_id":"2c1e7adec2c690c86d28cbe85ee725c7c2b3cd60"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9dc28471c2c3a6e4cad50562fd4cb86a688eb8e7","unresolved":true,"context_lines":[{"line_number":5028,"context_line":""},{"line_number":5029,"context_line":"        if not old_flavor_has_swap and new_flavor_has_swap:"},{"line_number":5030,"context_line":"            # neither flavor has swap so early exit."},{"line_number":5031,"context_line":"            return bdms"},{"line_number":5032,"context_line":""},{"line_number":5033,"context_line":"        def _add_bdm_record(swap_size):"},{"line_number":5034,"context_line":"            bdm_dict \u003d block_device.create_blank_bdm("}],"source_content_type":"text/x-python","patch_set":31,"id":"11f6b44b_5e41320a","line":5031,"in_reply_to":"8b9f3771_8d174067","updated":"2023-10-05 18:08:34.000000000","message":"Ack","commit_id":"2c1e7adec2c690c86d28cbe85ee725c7c2b3cd60"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"17e35dcb1bb45d989e0de3eec7d8526514cc638e","unresolved":true,"context_lines":[{"line_number":5138,"context_line":"                # 1-0-0"},{"line_number":5139,"context_line":"                swap_bdms[0].destroy()"},{"line_number":5140,"context_line":"                bdms_to_delete.append(swap_bdms[0])"},{"line_number":5141,"context_line":"            elif old_flavor_swap \u003c new_flavor_swap:"},{"line_number":5142,"context_line":"                if swap_bdms:"},{"line_number":5143,"context_line":"                    # add new bdm"},{"line_number":5144,"context_line":"                    # 1-2-2"}],"source_content_type":"text/x-python","patch_set":32,"id":"13a3e908_d24110e5","line":5141,"range":{"start_line":5141,"start_character":17,"end_line":5141,"end_character":50},"updated":"2024-01-17 05:53:30.000000000","message":"if any of these values don\u0027t exist, we will end up with one of the conditions\nFalse \u003c new_flavor_swap\nOR\nold_flavor_swap \u003c False\n\nwhich doesn\u0027t look correct","commit_id":"8c4bbe0b7eda5ba2bf2c2cd48de5ecb313190469"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"6a8e2adfdc0bc3f141aee18d5ad490f0bb640f62","unresolved":true,"context_lines":[{"line_number":5138,"context_line":"                # 1-0-0"},{"line_number":5139,"context_line":"                swap_bdms[0].destroy()"},{"line_number":5140,"context_line":"                bdms_to_delete.append(swap_bdms[0])"},{"line_number":5141,"context_line":"            elif old_flavor_swap \u003c new_flavor_swap:"},{"line_number":5142,"context_line":"                if swap_bdms:"},{"line_number":5143,"context_line":"                    # add new bdm"},{"line_number":5144,"context_line":"                    # 1-2-2"}],"source_content_type":"text/x-python","patch_set":32,"id":"98b90453_b01d4488","line":5141,"range":{"start_line":5141,"start_character":17,"end_line":5141,"end_character":50},"in_reply_to":"13a3e908_d24110e5","updated":"2024-01-29 06:48:13.000000000","message":"old_flavor_swap will always be a value/int and not boolean.\n\nI understand the confusion is from statement at L5310 while saving`old_flavor_swap`\nso updated.","commit_id":"8c4bbe0b7eda5ba2bf2c2cd48de5ecb313190469"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"0a0e61c1806eef57bffd08865cf2936b50768e2c","unresolved":false,"context_lines":[{"line_number":5138,"context_line":"                # 1-0-0"},{"line_number":5139,"context_line":"                swap_bdms[0].destroy()"},{"line_number":5140,"context_line":"                bdms_to_delete.append(swap_bdms[0])"},{"line_number":5141,"context_line":"            elif old_flavor_swap \u003c new_flavor_swap:"},{"line_number":5142,"context_line":"                if swap_bdms:"},{"line_number":5143,"context_line":"                    # add new bdm"},{"line_number":5144,"context_line":"                    # 1-2-2"}],"source_content_type":"text/x-python","patch_set":32,"id":"b43cabc7_58ba43a4","line":5141,"range":{"start_line":5141,"start_character":17,"end_line":5141,"end_character":50},"in_reply_to":"98b90453_b01d4488","updated":"2024-02-15 04:16:28.000000000","message":"Done","commit_id":"8c4bbe0b7eda5ba2bf2c2cd48de5ecb313190469"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"17e35dcb1bb45d989e0de3eec7d8526514cc638e","unresolved":true,"context_lines":[{"line_number":5148,"context_line":"                    # 0-1-1"},{"line_number":5149,"context_line":"                    _add_bdm_record(new_flavor_swap)"},{"line_number":5150,"context_line":""},{"line_number":5151,"context_line":"            elif old_flavor_swap \u003e new_flavor_swap:"},{"line_number":5152,"context_line":"                # this is shrink confirm"},{"line_number":5153,"context_line":"                # add back removed bdm but do not add new bdm"},{"line_number":5154,"context_line":"                # only update value of existing swap bdm."}],"source_content_type":"text/x-python","patch_set":32,"id":"434daa4d_504c421c","line":5151,"range":{"start_line":5151,"start_character":17,"end_line":5151,"end_character":50},"updated":"2024-01-17 05:53:30.000000000","message":"similar case here and following checks\n\nI think we should be sure that both values exist before checking a condition like this","commit_id":"8c4bbe0b7eda5ba2bf2c2cd48de5ecb313190469"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"6a8e2adfdc0bc3f141aee18d5ad490f0bb640f62","unresolved":false,"context_lines":[{"line_number":5148,"context_line":"                    # 0-1-1"},{"line_number":5149,"context_line":"                    _add_bdm_record(new_flavor_swap)"},{"line_number":5150,"context_line":""},{"line_number":5151,"context_line":"            elif old_flavor_swap \u003e new_flavor_swap:"},{"line_number":5152,"context_line":"                # this is shrink confirm"},{"line_number":5153,"context_line":"                # add back removed bdm but do not add new bdm"},{"line_number":5154,"context_line":"                # only update value of existing swap bdm."}],"source_content_type":"text/x-python","patch_set":32,"id":"3788a606_99aa6b91","line":5151,"range":{"start_line":5151,"start_character":17,"end_line":5151,"end_character":50},"in_reply_to":"434daa4d_504c421c","updated":"2024-01-29 06:48:13.000000000","message":"Done","commit_id":"8c4bbe0b7eda5ba2bf2c2cd48de5ecb313190469"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6be1633114e78b8cd7891d19ba4d17aa176a1e74","unresolved":true,"context_lines":[{"line_number":5109,"context_line":"        if not [bdm for bdm in bdms if bdm.obj_attr_is_set(\u0027guest_format\u0027)]:"},{"line_number":5110,"context_line":"            # this means these bdms are part of call (some test probably)"},{"line_number":5111,"context_line":"            # which do not have swap attribute."},{"line_number":5112,"context_line":"            return"},{"line_number":5113,"context_line":""},{"line_number":5114,"context_line":"        old_flavor_has_swap \u003d \u0027swap\u0027 in instance.old_flavor"},{"line_number":5115,"context_line":"        new_flavor_has_swap \u003d \u0027swap\u0027 in instance.new_flavor"}],"source_content_type":"text/x-python","patch_set":34,"id":"bd710952_51f06bb2","line":5112,"updated":"2024-01-31 07:42:06.000000000","message":"we should not alter production code to make tests pass.\nwe can have production code that makes tesing easier like returning extra values or breaking out part fo a function into a seperte function to make asserts or mocks simpler but we should not be doing  ifs like this.\n\nif the data is in vlaid in produciton the tests should not be passing it.","commit_id":"d907b3b7415438a0d7d077e58af7208f9f2008f5"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"0a0e61c1806eef57bffd08865cf2936b50768e2c","unresolved":false,"context_lines":[{"line_number":5109,"context_line":"        if not [bdm for bdm in bdms if bdm.obj_attr_is_set(\u0027guest_format\u0027)]:"},{"line_number":5110,"context_line":"            # this means these bdms are part of call (some test probably)"},{"line_number":5111,"context_line":"            # which do not have swap attribute."},{"line_number":5112,"context_line":"            return"},{"line_number":5113,"context_line":""},{"line_number":5114,"context_line":"        old_flavor_has_swap \u003d \u0027swap\u0027 in instance.old_flavor"},{"line_number":5115,"context_line":"        new_flavor_has_swap \u003d \u0027swap\u0027 in instance.new_flavor"}],"source_content_type":"text/x-python","patch_set":34,"id":"ad9bb186_18e69f8a","line":5112,"in_reply_to":"bd710952_51f06bb2","updated":"2024-02-15 04:16:28.000000000","message":"Done","commit_id":"d907b3b7415438a0d7d077e58af7208f9f2008f5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6be1633114e78b8cd7891d19ba4d17aa176a1e74","unresolved":true,"context_lines":[{"line_number":5117,"context_line":"        if not (old_flavor_has_swap and new_flavor_has_swap):"},{"line_number":5118,"context_line":"            # if any one or both flavor do not have swap attribute"},{"line_number":5119,"context_line":"            return"},{"line_number":5120,"context_line":""},{"line_number":5121,"context_line":"        # store both swap values,"},{"line_number":5122,"context_line":"        old_flavor_swap \u003d instance.old_flavor.swap"},{"line_number":5123,"context_line":"        new_flavor_swap \u003d instance.new_flavor.swap"}],"source_content_type":"text/x-python","patch_set":34,"id":"b2a54d5f_6e728f98","line":5120,"updated":"2024-01-31 07:42:06.000000000","message":"old_flavor_has_swap and new_flaovr_has_swap should\nalways be true. swap is a a field on the object\nhttps://github.com/openstack/nova/blob/a66a0dd113d0b57acf9f69d76785be39479eebc0/nova/objects/flavor.py#L215\n\nyou are trying to detect if its set with this code correct.\n\nthe comment is incorect\n```\nTruth Table for AND\n   old  new   old AND new\n-------+---------+-------\n   T   T     T\n   T   F     F\n   F   T     F\n   F   F     F\n```\n```\nTruth Table for NOT AND\n   old   new   NOT(old AND new)\n-------+---------+-------------\n   T   T     False\n   T   F     False\n   F   T     False\n   F   F     True\n```\n\nthis if is retuning if neither flavor has requested swap.\n\nthat is an ok early return check as there is nothign to do in that case\nbut that is not what the comment says the code is doing","commit_id":"d907b3b7415438a0d7d077e58af7208f9f2008f5"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"0a0e61c1806eef57bffd08865cf2936b50768e2c","unresolved":false,"context_lines":[{"line_number":5117,"context_line":"        if not (old_flavor_has_swap and new_flavor_has_swap):"},{"line_number":5118,"context_line":"            # if any one or both flavor do not have swap attribute"},{"line_number":5119,"context_line":"            return"},{"line_number":5120,"context_line":""},{"line_number":5121,"context_line":"        # store both swap values,"},{"line_number":5122,"context_line":"        old_flavor_swap \u003d instance.old_flavor.swap"},{"line_number":5123,"context_line":"        new_flavor_swap \u003d instance.new_flavor.swap"}],"source_content_type":"text/x-python","patch_set":34,"id":"73ade39a_47b199e9","line":5120,"in_reply_to":"b2a54d5f_6e728f98","updated":"2024-02-15 04:16:28.000000000","message":"Done","commit_id":"d907b3b7415438a0d7d077e58af7208f9f2008f5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6be1633114e78b8cd7891d19ba4d17aa176a1e74","unresolved":true,"context_lines":[{"line_number":5124,"context_line":""},{"line_number":5125,"context_line":"        if old_flavor_swap \u003d\u003d new_flavor_swap:"},{"line_number":5126,"context_line":"            # if there is no change in swap value"},{"line_number":5127,"context_line":"            return"},{"line_number":5128,"context_line":""},{"line_number":5129,"context_line":"        def _add_bdm_record(swap_size):"},{"line_number":5130,"context_line":"            bdm_dict \u003d block_device.create_blank_bdm("}],"source_content_type":"text/x-python","patch_set":34,"id":"3ecf8e2e_979abe21","line":5127,"updated":"2024-01-31 07:42:06.000000000","message":"so acessing .swap should always be safe. its an int feild and should\ndefault to 0\n\nthis should be the only early out needed and you can remove the previos too.\n\nif either old_flavor_swap or new_flavor_swap is 0 \nand old_flavor_swap \u003d\u003d new_flavor_swap that means that both are 0\n\nso this is the only check you need to return early.\n\nwe only need to update the swap if both flavors dont have the same amount","commit_id":"d907b3b7415438a0d7d077e58af7208f9f2008f5"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"0a0e61c1806eef57bffd08865cf2936b50768e2c","unresolved":false,"context_lines":[{"line_number":5124,"context_line":""},{"line_number":5125,"context_line":"        if old_flavor_swap \u003d\u003d new_flavor_swap:"},{"line_number":5126,"context_line":"            # if there is no change in swap value"},{"line_number":5127,"context_line":"            return"},{"line_number":5128,"context_line":""},{"line_number":5129,"context_line":"        def _add_bdm_record(swap_size):"},{"line_number":5130,"context_line":"            bdm_dict \u003d block_device.create_blank_bdm("}],"source_content_type":"text/x-python","patch_set":34,"id":"cd781d7c_21d03791","line":5127,"in_reply_to":"3ecf8e2e_979abe21","updated":"2024-02-15 04:16:28.000000000","message":"Done","commit_id":"d907b3b7415438a0d7d077e58af7208f9f2008f5"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"0a0e61c1806eef57bffd08865cf2936b50768e2c","unresolved":true,"context_lines":[{"line_number":5140,"context_line":"            if not new_flavor_swap and swap_bdms:"},{"line_number":5141,"context_line":"                # 1-0-0"},{"line_number":5142,"context_line":"                swap_bdms[0].destroy()"},{"line_number":5143,"context_line":"                bdms_to_delete.append(swap_bdms[0])"},{"line_number":5144,"context_line":"            elif old_flavor_swap \u003c new_flavor_swap:"},{"line_number":5145,"context_line":"                if swap_bdms:"},{"line_number":5146,"context_line":"                    # 1-2-2"}],"source_content_type":"text/x-python","patch_set":34,"id":"9a5166ea_aff46d9a","line":5143,"updated":"2024-02-15 04:16:28.000000000","message":"this will be True only When\nnot False and True\n\nwhich means there is change in swap and we have bdm with swap","commit_id":"d907b3b7415438a0d7d077e58af7208f9f2008f5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"262df9b6254b0b8f9c0c16d22234803e9a4f914c","unresolved":true,"context_lines":[{"line_number":5133,"context_line":"        swap_bdms \u003d [bdm for bdm in bdms if bdm.guest_format \u003d\u003d \u0027swap\u0027]"},{"line_number":5134,"context_line":"        bdms_to_delete \u003d []"},{"line_number":5135,"context_line":""},{"line_number":5136,"context_line":"        if confirm:"},{"line_number":5137,"context_line":"            if not new_flavor_swap and swap_bdms:"},{"line_number":5138,"context_line":"                # 1-0-0"},{"line_number":5139,"context_line":"                # swap_bdms[0].destroy()"}],"source_content_type":"text/x-python","patch_set":35,"id":"2450eb11_baf1949c","line":5136,"updated":"2024-02-13 10:38:44.000000000","message":"this need to be simplifed\n\nthe only difference between the confirm and else branch \nis the new_flavor_swap vs old_flavor_swap\njust pull that out like this\n\nswap_value \u003d new_flavor_swap if confrim else old_flavor_swap\n\nthen you can remvoe the outer if and the entire else barnch\n\nalso you have a lot of commented code that needs to be cleaned up.","commit_id":"180511ebec3a8cf2228ad70f7c603ef36ecc09aa"}],"nova/tests/fixtures/nova.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ca1eca814d863519f889128366fa3d8a5ba7e20","unresolved":true,"context_lines":[{"line_number":758,"context_line":"                           **swap_defaults),"},{"line_number":759,"context_line":"            objects.Flavor(context\u003dctxt, memory_mb\u003d2048, vcpus\u003d1,"},{"line_number":760,"context_line":"                           root_gb\u003d20, flavorid\u003d\u00278\u0027, name\u003d\u0027m1.small.swap2\u0027,"},{"line_number":761,"context_line":"                           **swap_defaults2),"},{"line_number":762,"context_line":"            ]"},{"line_number":763,"context_line":"        for flavor in default_flavors:"},{"line_number":764,"context_line":"            flavor.create()"}],"source_content_type":"text/x-python","patch_set":8,"id":"296d7130_252bef4e","line":761,"updated":"2023-02-13 21:08:00.000000000","message":"We can avoid changing the fixture and causing api sample churn by creating the flavors we need for the new tests in the tests themselves instead of adding them to the fixture.","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"b73659ea76c3e7e40004f2d302c39c90aa3cd6ab","unresolved":false,"context_lines":[{"line_number":758,"context_line":"                           **swap_defaults),"},{"line_number":759,"context_line":"            objects.Flavor(context\u003dctxt, memory_mb\u003d2048, vcpus\u003d1,"},{"line_number":760,"context_line":"                           root_gb\u003d20, flavorid\u003d\u00278\u0027, name\u003d\u0027m1.small.swap2\u0027,"},{"line_number":761,"context_line":"                           **swap_defaults2),"},{"line_number":762,"context_line":"            ]"},{"line_number":763,"context_line":"        for flavor in default_flavors:"},{"line_number":764,"context_line":"            flavor.create()"}],"source_content_type":"text/x-python","patch_set":8,"id":"80afdfec_3ceb1a57","line":761,"in_reply_to":"296d7130_252bef4e","updated":"2023-02-22 12:05:57.000000000","message":"Done","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"}],"nova/tests/functional/test_servers.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"2d637dfbb6f8bb044e09838e6ec19166b26dd102","unresolved":true,"context_lines":[{"line_number":3085,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3086,"context_line":"        # after resize"},{"line_number":3087,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 1024)"},{"line_number":3088,"context_line":""},{"line_number":3089,"context_line":"    def _test_resize_to_same_host_instance_fails(self, failing_method,"},{"line_number":3090,"context_line":"                                                 event_name):"},{"line_number":3091,"context_line":"        \"\"\"Tests that when we resize to the same host and resize fails in"}],"source_content_type":"text/x-python","patch_set":5,"id":"b0e4093a_0f351738","line":3088,"updated":"2022-10-11 07:18:13.000000000","message":"Conceptually, these tests look good, but I found when I pulled this patch down and backed out the fix, the tests still pass:\n\n nova.tests.functional.test_servers.ServerMovingTests.test_resize_swap_shrink_to_zero [7.644790s] ... ok\nnova.tests.functional.test_servers.ServerMovingTests.test_resize_swap_expand_to_non_zero [7.748597s] ... ok\nnova.tests.functional.test_servers.ServerMovingTests.test_resize_swap_shrink_to_non_zero [8.186368s] ... ok\n\nOff the top of my head I don\u0027t know if there is any way we could reproduce the buggy behavior in the tests such that they would protect against regression.","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ca1eca814d863519f889128366fa3d8a5ba7e20","unresolved":true,"context_lines":[{"line_number":3085,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3086,"context_line":"        # after resize"},{"line_number":3087,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 1024)"},{"line_number":3088,"context_line":""},{"line_number":3089,"context_line":"    def _test_resize_to_same_host_instance_fails(self, failing_method,"},{"line_number":3090,"context_line":"                                                 event_name):"},{"line_number":3091,"context_line":"        \"\"\"Tests that when we resize to the same host and resize fails in"}],"source_content_type":"text/x-python","patch_set":5,"id":"ef31040d_ae83cd3b","line":3088,"in_reply_to":"b0e4093a_0f351738","updated":"2023-02-13 21:08:00.000000000","message":"I realized I think we could do this by verifying the state of the instance block device mappings before/after resize. That way the test would fail without the fix and pass with the fix.","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9fd988ad3f4562b8fdd35b327879458a0cadab0a","unresolved":false,"context_lines":[{"line_number":3085,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3086,"context_line":"        # after resize"},{"line_number":3087,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 1024)"},{"line_number":3088,"context_line":""},{"line_number":3089,"context_line":"    def _test_resize_to_same_host_instance_fails(self, failing_method,"},{"line_number":3090,"context_line":"                                                 event_name):"},{"line_number":3091,"context_line":"        \"\"\"Tests that when we resize to the same host and resize fails in"}],"source_content_type":"text/x-python","patch_set":5,"id":"7af0ac1f_6e0191ef","line":3088,"in_reply_to":"ef31040d_ae83cd3b","updated":"2023-02-22 11:00:36.000000000","message":"Done","commit_id":"5e2a02d4a29cf5e6082e8562bd7cdd66263a033c"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"645496d4df48cfcfc30a6ff1215376ee3258b7c8","unresolved":true,"context_lines":[{"line_number":3051,"context_line":"    def test_resize_swap_shrink_to_zero(self):"},{"line_number":3052,"context_line":"        \"\"\"Test server resize"},{"line_number":3053,"context_line":"        resize server flavor swap non-zero (1024) to zero"},{"line_number":3054,"context_line":"        \"\"\""},{"line_number":3055,"context_line":"        server \u003d self._create_server(flavor_id\u003dself.flavor4[\u0027id\u0027],"},{"line_number":3056,"context_line":"                    networks\u003d[])"},{"line_number":3057,"context_line":"        # before resize"}],"source_content_type":"text/x-python","patch_set":8,"id":"49836d92_7efea28a","line":3054,"updated":"2023-02-14 22:03:43.000000000","message":"At the beginning of the test you can do:\n\n self.api.microversion \u003d \u00272.47\u0027\n\nso that you\u0027ll call the nova API with a later microversion (default is the lowest, 2.1) because instance flavor embedded details that you want to assert on were added in microversion 2.47 [1].\n\n[1] https://docs.openstack.org/nova/latest/reference/api-microversion-history.html#id43","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ca1eca814d863519f889128366fa3d8a5ba7e20","unresolved":true,"context_lines":[{"line_number":3051,"context_line":"    def test_resize_swap_shrink_to_zero(self):"},{"line_number":3052,"context_line":"        \"\"\"Test server resize"},{"line_number":3053,"context_line":"        resize server flavor swap non-zero (1024) to zero"},{"line_number":3054,"context_line":"        \"\"\""},{"line_number":3055,"context_line":"        server \u003d self._create_server(flavor_id\u003dself.flavor4[\u0027id\u0027],"},{"line_number":3056,"context_line":"                    networks\u003d[])"},{"line_number":3057,"context_line":"        # before resize"}],"source_content_type":"text/x-python","patch_set":8,"id":"bbf133ff_2cd19d41","line":3054,"updated":"2023-02-13 21:08:00.000000000","message":"Before creating the server, you can create a flavor [1] with swap (I\u0027m copying the values you had added to the flavors fixture but you can omit/change whatever you need):\n\n flavor \u003d self._create_flavor(\n     name\u003d\u0027m1.small.swap1\u0027,\n     memory_mb\u003d2048,\n     vcpus\u003d1,\n     disk\u003d20,\n     ephemeral\u003d0,\n     swap\u003d1024,\n )\n \n[1] https://github.com/openstack/nova/blob/a2964417822bd1a4a83fa5c27282d2be1e18868a/nova/tests/functional/integrated_helpers.py#L369","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9fd988ad3f4562b8fdd35b327879458a0cadab0a","unresolved":false,"context_lines":[{"line_number":3051,"context_line":"    def test_resize_swap_shrink_to_zero(self):"},{"line_number":3052,"context_line":"        \"\"\"Test server resize"},{"line_number":3053,"context_line":"        resize server flavor swap non-zero (1024) to zero"},{"line_number":3054,"context_line":"        \"\"\""},{"line_number":3055,"context_line":"        server \u003d self._create_server(flavor_id\u003dself.flavor4[\u0027id\u0027],"},{"line_number":3056,"context_line":"                    networks\u003d[])"},{"line_number":3057,"context_line":"        # before resize"}],"source_content_type":"text/x-python","patch_set":8,"id":"728fe273_f2f13ba5","line":3054,"in_reply_to":"49836d92_7efea28a","updated":"2023-02-22 11:00:36.000000000","message":"Done","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9fd988ad3f4562b8fdd35b327879458a0cadab0a","unresolved":false,"context_lines":[{"line_number":3051,"context_line":"    def test_resize_swap_shrink_to_zero(self):"},{"line_number":3052,"context_line":"        \"\"\"Test server resize"},{"line_number":3053,"context_line":"        resize server flavor swap non-zero (1024) to zero"},{"line_number":3054,"context_line":"        \"\"\""},{"line_number":3055,"context_line":"        server \u003d self._create_server(flavor_id\u003dself.flavor4[\u0027id\u0027],"},{"line_number":3056,"context_line":"                    networks\u003d[])"},{"line_number":3057,"context_line":"        # before resize"}],"source_content_type":"text/x-python","patch_set":8,"id":"7df47a89_30bcffca","line":3054,"in_reply_to":"bbf133ff_2cd19d41","updated":"2023-02-22 11:00:36.000000000","message":"Done","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ca1eca814d863519f889128366fa3d8a5ba7e20","unresolved":true,"context_lines":[{"line_number":3053,"context_line":"        resize server flavor swap non-zero (1024) to zero"},{"line_number":3054,"context_line":"        \"\"\""},{"line_number":3055,"context_line":"        server \u003d self._create_server(flavor_id\u003dself.flavor4[\u0027id\u0027],"},{"line_number":3056,"context_line":"                    networks\u003d[])"},{"line_number":3057,"context_line":"        # before resize"},{"line_number":3058,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 1024)"},{"line_number":3059,"context_line":"        server \u003d self._resize_server(server, self.flavor2[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":8,"id":"f01bdf86_fe21689c","line":3056,"updated":"2023-02-13 21:08:00.000000000","message":"Then you can pass it here:\n\n server \u003d self._create_server(flavor_id\u003dflavor[\u0027id\u0027], networks\u003d[])","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9fd988ad3f4562b8fdd35b327879458a0cadab0a","unresolved":false,"context_lines":[{"line_number":3053,"context_line":"        resize server flavor swap non-zero (1024) to zero"},{"line_number":3054,"context_line":"        \"\"\""},{"line_number":3055,"context_line":"        server \u003d self._create_server(flavor_id\u003dself.flavor4[\u0027id\u0027],"},{"line_number":3056,"context_line":"                    networks\u003d[])"},{"line_number":3057,"context_line":"        # before resize"},{"line_number":3058,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 1024)"},{"line_number":3059,"context_line":"        server \u003d self._resize_server(server, self.flavor2[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":8,"id":"54628b89_fb357dbc","line":3056,"in_reply_to":"f01bdf86_fe21689c","updated":"2023-02-22 11:00:36.000000000","message":"Done","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ca1eca814d863519f889128366fa3d8a5ba7e20","unresolved":true,"context_lines":[{"line_number":3055,"context_line":"        server \u003d self._create_server(flavor_id\u003dself.flavor4[\u0027id\u0027],"},{"line_number":3056,"context_line":"                    networks\u003d[])"},{"line_number":3057,"context_line":"        # before resize"},{"line_number":3058,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 1024)"},{"line_number":3059,"context_line":"        server \u003d self._resize_server(server, self.flavor2[\u0027id\u0027])"},{"line_number":3060,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3061,"context_line":"        # after resize"}],"source_content_type":"text/x-python","patch_set":8,"id":"ac36588d_80995000","line":3058,"updated":"2023-02-13 21:08:00.000000000","message":"I think we should also verify the block device mapping records as they are separate from the instance.flavor. Maybe something like this:\n\n        # verify block device mapping\n        ctxt \u003d context.get_admin_context()\n        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid(\n            ctxt, server[\u0027id\u0027])\n        self.assertIn(\u0027swap\u0027, [bdm.guest_format for bdm in bdms])","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9fd988ad3f4562b8fdd35b327879458a0cadab0a","unresolved":false,"context_lines":[{"line_number":3055,"context_line":"        server \u003d self._create_server(flavor_id\u003dself.flavor4[\u0027id\u0027],"},{"line_number":3056,"context_line":"                    networks\u003d[])"},{"line_number":3057,"context_line":"        # before resize"},{"line_number":3058,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 1024)"},{"line_number":3059,"context_line":"        server \u003d self._resize_server(server, self.flavor2[\u0027id\u0027])"},{"line_number":3060,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3061,"context_line":"        # after resize"}],"source_content_type":"text/x-python","patch_set":8,"id":"26c10fa5_94b1c424","line":3058,"in_reply_to":"ac36588d_80995000","updated":"2023-02-22 11:00:36.000000000","message":"Done","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ca1eca814d863519f889128366fa3d8a5ba7e20","unresolved":true,"context_lines":[{"line_number":3059,"context_line":"        server \u003d self._resize_server(server, self.flavor2[\u0027id\u0027])"},{"line_number":3060,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3061,"context_line":"        # after resize"},{"line_number":3062,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 0)"},{"line_number":3063,"context_line":""},{"line_number":3064,"context_line":"    def test_resize_swap_shrink_to_non_zero(self):"},{"line_number":3065,"context_line":"        \"\"\"Test server resize"}],"source_content_type":"text/x-python","patch_set":8,"id":"9a037b2d_0bd53b4e","line":3062,"updated":"2023-02-13 21:08:00.000000000","message":"And then:\n\n        # verify no swap in block device mapping\n        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid(\n            ctxt, server[\u0027id\u0027])\n        self.assertNotIn(\u0027swap\u0027, [bdm.guest_format for bdm in bdms])","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9fd988ad3f4562b8fdd35b327879458a0cadab0a","unresolved":false,"context_lines":[{"line_number":3059,"context_line":"        server \u003d self._resize_server(server, self.flavor2[\u0027id\u0027])"},{"line_number":3060,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3061,"context_line":"        # after resize"},{"line_number":3062,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 0)"},{"line_number":3063,"context_line":""},{"line_number":3064,"context_line":"    def test_resize_swap_shrink_to_non_zero(self):"},{"line_number":3065,"context_line":"        \"\"\"Test server resize"}],"source_content_type":"text/x-python","patch_set":8,"id":"21c0f800_1d56f864","line":3062,"in_reply_to":"9a037b2d_0bd53b4e","updated":"2023-02-22 11:00:36.000000000","message":"Done","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ca1eca814d863519f889128366fa3d8a5ba7e20","unresolved":true,"context_lines":[{"line_number":3065,"context_line":"        \"\"\"Test server resize"},{"line_number":3066,"context_line":"        resize server flavor swap non-zero (2048) to"},{"line_number":3067,"context_line":"        non-zero (1024)"},{"line_number":3068,"context_line":"        \"\"\""},{"line_number":3069,"context_line":"        server \u003d self._create_server(flavor_id\u003dself.flavor5[\u0027id\u0027],"},{"line_number":3070,"context_line":"                    networks\u003d[])"},{"line_number":3071,"context_line":"        # before resize"}],"source_content_type":"text/x-python","patch_set":8,"id":"66488ae0_3fd811b6","line":3068,"updated":"2023-02-13 21:08:00.000000000","message":"Similar thing here:\n\n flavor1 \u003d self._create_flavor(\n     name\u003d\u0027m1.small.swap1\u0027,\n     memory_mb\u003d2048,\n     vcpus\u003d1,\n     disk\u003d20,\n     ephemeral\u003d0,\n     swap\u003d1024,\n )\n flavor2 \u003d self._create_flavor(\n     name\u003d\u0027m1.small.swap1\u0027,\n     memory_mb\u003d2048,\n     vcpus\u003d1,\n     disk\u003d20,\n     ephemeral\u003d0,\n     swap\u003d2048,\n )","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9fd988ad3f4562b8fdd35b327879458a0cadab0a","unresolved":false,"context_lines":[{"line_number":3065,"context_line":"        \"\"\"Test server resize"},{"line_number":3066,"context_line":"        resize server flavor swap non-zero (2048) to"},{"line_number":3067,"context_line":"        non-zero (1024)"},{"line_number":3068,"context_line":"        \"\"\""},{"line_number":3069,"context_line":"        server \u003d self._create_server(flavor_id\u003dself.flavor5[\u0027id\u0027],"},{"line_number":3070,"context_line":"                    networks\u003d[])"},{"line_number":3071,"context_line":"        # before resize"}],"source_content_type":"text/x-python","patch_set":8,"id":"05dfb2a7_e2b750ef","line":3068,"in_reply_to":"66488ae0_3fd811b6","updated":"2023-02-22 11:00:36.000000000","message":"Done","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ca1eca814d863519f889128366fa3d8a5ba7e20","unresolved":true,"context_lines":[{"line_number":3066,"context_line":"        resize server flavor swap non-zero (2048) to"},{"line_number":3067,"context_line":"        non-zero (1024)"},{"line_number":3068,"context_line":"        \"\"\""},{"line_number":3069,"context_line":"        server \u003d self._create_server(flavor_id\u003dself.flavor5[\u0027id\u0027],"},{"line_number":3070,"context_line":"                    networks\u003d[])"},{"line_number":3071,"context_line":"        # before resize"},{"line_number":3072,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 2048)"}],"source_content_type":"text/x-python","patch_set":8,"id":"f21f2e07_e20f3ed9","line":3069,"range":{"start_line":3069,"start_character":47,"end_line":3069,"end_character":59},"updated":"2023-02-13 21:08:00.000000000","message":"flavor2","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9fd988ad3f4562b8fdd35b327879458a0cadab0a","unresolved":false,"context_lines":[{"line_number":3066,"context_line":"        resize server flavor swap non-zero (2048) to"},{"line_number":3067,"context_line":"        non-zero (1024)"},{"line_number":3068,"context_line":"        \"\"\""},{"line_number":3069,"context_line":"        server \u003d self._create_server(flavor_id\u003dself.flavor5[\u0027id\u0027],"},{"line_number":3070,"context_line":"                    networks\u003d[])"},{"line_number":3071,"context_line":"        # before resize"},{"line_number":3072,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 2048)"}],"source_content_type":"text/x-python","patch_set":8,"id":"a1407b82_ce6bb327","line":3069,"range":{"start_line":3069,"start_character":47,"end_line":3069,"end_character":59},"in_reply_to":"f21f2e07_e20f3ed9","updated":"2023-02-22 11:00:36.000000000","message":"Done","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ca1eca814d863519f889128366fa3d8a5ba7e20","unresolved":true,"context_lines":[{"line_number":3069,"context_line":"        server \u003d self._create_server(flavor_id\u003dself.flavor5[\u0027id\u0027],"},{"line_number":3070,"context_line":"                    networks\u003d[])"},{"line_number":3071,"context_line":"        # before resize"},{"line_number":3072,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 2048)"},{"line_number":3073,"context_line":"        server \u003d self._resize_server(server, self.flavor4[\u0027id\u0027])"},{"line_number":3074,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3075,"context_line":"        # after resize"}],"source_content_type":"text/x-python","patch_set":8,"id":"c6a8f2b8_654172ef","line":3072,"updated":"2023-02-13 21:08:00.000000000","message":"Would be good to also verify the block device mapping:\n\n        # verify block device mapping\n        ctxt \u003d context.get_admin_context()\n        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid(\n            ctxt, server[\u0027id\u0027])\n        swap_bdms \u003d [bdm for bdm in bdms if bdm.guest_format \u003d\u003d \u0027swap\u0027]\n        self.assertEqual(1, len(swap_bdms))\n        self.assertEqual(2048, swap_bdms[0].volume_size)","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9fd988ad3f4562b8fdd35b327879458a0cadab0a","unresolved":false,"context_lines":[{"line_number":3069,"context_line":"        server \u003d self._create_server(flavor_id\u003dself.flavor5[\u0027id\u0027],"},{"line_number":3070,"context_line":"                    networks\u003d[])"},{"line_number":3071,"context_line":"        # before resize"},{"line_number":3072,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 2048)"},{"line_number":3073,"context_line":"        server \u003d self._resize_server(server, self.flavor4[\u0027id\u0027])"},{"line_number":3074,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3075,"context_line":"        # after resize"}],"source_content_type":"text/x-python","patch_set":8,"id":"0a07de05_4c0a3603","line":3072,"in_reply_to":"c6a8f2b8_654172ef","updated":"2023-02-22 11:00:36.000000000","message":"Done","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ca1eca814d863519f889128366fa3d8a5ba7e20","unresolved":true,"context_lines":[{"line_number":3070,"context_line":"                    networks\u003d[])"},{"line_number":3071,"context_line":"        # before resize"},{"line_number":3072,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 2048)"},{"line_number":3073,"context_line":"        server \u003d self._resize_server(server, self.flavor4[\u0027id\u0027])"},{"line_number":3074,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3075,"context_line":"        # after resize"},{"line_number":3076,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 1024)"}],"source_content_type":"text/x-python","patch_set":8,"id":"63c07b9b_80a4a959","line":3073,"range":{"start_line":3073,"start_character":45,"end_line":3073,"end_character":57},"updated":"2023-02-13 21:08:00.000000000","message":"flavor1","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9fd988ad3f4562b8fdd35b327879458a0cadab0a","unresolved":false,"context_lines":[{"line_number":3070,"context_line":"                    networks\u003d[])"},{"line_number":3071,"context_line":"        # before resize"},{"line_number":3072,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 2048)"},{"line_number":3073,"context_line":"        server \u003d self._resize_server(server, self.flavor4[\u0027id\u0027])"},{"line_number":3074,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3075,"context_line":"        # after resize"},{"line_number":3076,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 1024)"}],"source_content_type":"text/x-python","patch_set":8,"id":"56ba43b2_8dac22a7","line":3073,"range":{"start_line":3073,"start_character":45,"end_line":3073,"end_character":57},"in_reply_to":"63c07b9b_80a4a959","updated":"2023-02-22 11:00:36.000000000","message":"Done","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ca1eca814d863519f889128366fa3d8a5ba7e20","unresolved":true,"context_lines":[{"line_number":3073,"context_line":"        server \u003d self._resize_server(server, self.flavor4[\u0027id\u0027])"},{"line_number":3074,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3075,"context_line":"        # after resize"},{"line_number":3076,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 1024)"},{"line_number":3077,"context_line":""},{"line_number":3078,"context_line":"    def test_resize_swap_expand_to_non_zero(self):"},{"line_number":3079,"context_line":"        \"\"\"Test server resize"}],"source_content_type":"text/x-python","patch_set":8,"id":"5f8f063a_2d524ef7","line":3076,"updated":"2023-02-13 21:08:00.000000000","message":"Same here.","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9fd988ad3f4562b8fdd35b327879458a0cadab0a","unresolved":false,"context_lines":[{"line_number":3073,"context_line":"        server \u003d self._resize_server(server, self.flavor4[\u0027id\u0027])"},{"line_number":3074,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3075,"context_line":"        # after resize"},{"line_number":3076,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 1024)"},{"line_number":3077,"context_line":""},{"line_number":3078,"context_line":"    def test_resize_swap_expand_to_non_zero(self):"},{"line_number":3079,"context_line":"        \"\"\"Test server resize"}],"source_content_type":"text/x-python","patch_set":8,"id":"27406d5c_c4f76c37","line":3076,"in_reply_to":"5f8f063a_2d524ef7","updated":"2023-02-22 11:00:36.000000000","message":"Done","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ca1eca814d863519f889128366fa3d8a5ba7e20","unresolved":true,"context_lines":[{"line_number":3078,"context_line":"    def test_resize_swap_expand_to_non_zero(self):"},{"line_number":3079,"context_line":"        \"\"\"Test server resize"},{"line_number":3080,"context_line":"        resize server flavor swap from zero to non-zero (1024)"},{"line_number":3081,"context_line":"        \"\"\""},{"line_number":3082,"context_line":"        server \u003d self._create_server(flavor_id\u003dself.flavor2[\u0027id\u0027],"},{"line_number":3083,"context_line":"                    networks\u003d[])"},{"line_number":3084,"context_line":"        # before resize"}],"source_content_type":"text/x-python","patch_set":8,"id":"041a2e92_8ae9d19b","line":3081,"updated":"2023-02-13 21:08:00.000000000","message":"flavor \u003d self._create_flavor(\n     name\u003d\u0027m1.small.swap1\u0027,\n     memory_mb\u003d2048,\n     vcpus\u003d1,\n     disk\u003d20,\n     ephemeral\u003d0,\n     swap\u003d1024,\n )","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9fd988ad3f4562b8fdd35b327879458a0cadab0a","unresolved":false,"context_lines":[{"line_number":3078,"context_line":"    def test_resize_swap_expand_to_non_zero(self):"},{"line_number":3079,"context_line":"        \"\"\"Test server resize"},{"line_number":3080,"context_line":"        resize server flavor swap from zero to non-zero (1024)"},{"line_number":3081,"context_line":"        \"\"\""},{"line_number":3082,"context_line":"        server \u003d self._create_server(flavor_id\u003dself.flavor2[\u0027id\u0027],"},{"line_number":3083,"context_line":"                    networks\u003d[])"},{"line_number":3084,"context_line":"        # before resize"}],"source_content_type":"text/x-python","patch_set":8,"id":"6e9d97db_66d1411f","line":3081,"in_reply_to":"041a2e92_8ae9d19b","updated":"2023-02-22 11:00:36.000000000","message":"Done","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ca1eca814d863519f889128366fa3d8a5ba7e20","unresolved":true,"context_lines":[{"line_number":3082,"context_line":"        server \u003d self._create_server(flavor_id\u003dself.flavor2[\u0027id\u0027],"},{"line_number":3083,"context_line":"                    networks\u003d[])"},{"line_number":3084,"context_line":"        # before resize"},{"line_number":3085,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 0)"},{"line_number":3086,"context_line":"        server \u003d self._resize_server(server, self.flavor4[\u0027id\u0027])"},{"line_number":3087,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3088,"context_line":"        # after resize"}],"source_content_type":"text/x-python","patch_set":8,"id":"d150fb01_7669f2d2","line":3085,"updated":"2023-02-13 21:08:00.000000000","message":"Same here, you\u0027ll verify that there is no BDM with guest_format \u003d \u0027swap\u0027 for this server.","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9fd988ad3f4562b8fdd35b327879458a0cadab0a","unresolved":false,"context_lines":[{"line_number":3082,"context_line":"        server \u003d self._create_server(flavor_id\u003dself.flavor2[\u0027id\u0027],"},{"line_number":3083,"context_line":"                    networks\u003d[])"},{"line_number":3084,"context_line":"        # before resize"},{"line_number":3085,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 0)"},{"line_number":3086,"context_line":"        server \u003d self._resize_server(server, self.flavor4[\u0027id\u0027])"},{"line_number":3087,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3088,"context_line":"        # after resize"}],"source_content_type":"text/x-python","patch_set":8,"id":"8c364b21_789236bd","line":3085,"in_reply_to":"d150fb01_7669f2d2","updated":"2023-02-22 11:00:36.000000000","message":"Done","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ca1eca814d863519f889128366fa3d8a5ba7e20","unresolved":true,"context_lines":[{"line_number":3083,"context_line":"                    networks\u003d[])"},{"line_number":3084,"context_line":"        # before resize"},{"line_number":3085,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 0)"},{"line_number":3086,"context_line":"        server \u003d self._resize_server(server, self.flavor4[\u0027id\u0027])"},{"line_number":3087,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3088,"context_line":"        # after resize"},{"line_number":3089,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 1024)"}],"source_content_type":"text/x-python","patch_set":8,"id":"aad07dfe_300192b0","line":3086,"range":{"start_line":3086,"start_character":45,"end_line":3086,"end_character":57},"updated":"2023-02-13 21:08:00.000000000","message":"flavor","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9fd988ad3f4562b8fdd35b327879458a0cadab0a","unresolved":false,"context_lines":[{"line_number":3083,"context_line":"                    networks\u003d[])"},{"line_number":3084,"context_line":"        # before resize"},{"line_number":3085,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 0)"},{"line_number":3086,"context_line":"        server \u003d self._resize_server(server, self.flavor4[\u0027id\u0027])"},{"line_number":3087,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3088,"context_line":"        # after resize"},{"line_number":3089,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 1024)"}],"source_content_type":"text/x-python","patch_set":8,"id":"e20da6f1_57e31082","line":3086,"range":{"start_line":3086,"start_character":45,"end_line":3086,"end_character":57},"in_reply_to":"aad07dfe_300192b0","updated":"2023-02-22 11:00:36.000000000","message":"Done","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5ca1eca814d863519f889128366fa3d8a5ba7e20","unresolved":true,"context_lines":[{"line_number":3086,"context_line":"        server \u003d self._resize_server(server, self.flavor4[\u0027id\u0027])"},{"line_number":3087,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3088,"context_line":"        # after resize"},{"line_number":3089,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 1024)"},{"line_number":3090,"context_line":""},{"line_number":3091,"context_line":"    def _test_resize_to_same_host_instance_fails(self, failing_method,"},{"line_number":3092,"context_line":"                                                 event_name):"}],"source_content_type":"text/x-python","patch_set":8,"id":"fc3e1f69_ad050e06","line":3089,"updated":"2023-02-13 21:08:00.000000000","message":"Same here, verify we have a swap BDM now and that the size is 1024.","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"9fd988ad3f4562b8fdd35b327879458a0cadab0a","unresolved":false,"context_lines":[{"line_number":3086,"context_line":"        server \u003d self._resize_server(server, self.flavor4[\u0027id\u0027])"},{"line_number":3087,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3088,"context_line":"        # after resize"},{"line_number":3089,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 1024)"},{"line_number":3090,"context_line":""},{"line_number":3091,"context_line":"    def _test_resize_to_same_host_instance_fails(self, failing_method,"},{"line_number":3092,"context_line":"                                                 event_name):"}],"source_content_type":"text/x-python","patch_set":8,"id":"f5974838_7b378306","line":3089,"in_reply_to":"fc3e1f69_ad050e06","updated":"2023-02-22 11:00:36.000000000","message":"Done","commit_id":"9e670fe04be3f1fac5e519bf9339be5d83a3f119"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8810e68e8806e117e64ee8861c5cdefa7cb6f0c1","unresolved":true,"context_lines":[{"line_number":3112,"context_line":"        server \u003d self._create_server(flavor_id\u003dfl_1, networks\u003d[])"},{"line_number":3113,"context_line":"        # before resize"},{"line_number":3114,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 0)"},{"line_number":3115,"context_line":"        server \u003d self._resize_server(server, fl_2)"},{"line_number":3116,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3117,"context_line":"        # after resize"},{"line_number":3118,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 1024)"}],"source_content_type":"text/x-python","patch_set":14,"id":"f6eba471_4d3889e0","line":3115,"updated":"2023-02-22 03:53:04.000000000","message":"This is kind of a nit but we could copy L3121 - L3123 here to verify that we have synced the BDM records before resize confirm i.e. at the same time the instance XML has changed. And do the same for other tests where we went from no swap to swap or swap to no swap.","commit_id":"a93f15f8ed606a0b311f63250509e5c1bf32fdc8"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"4abd79f97dac980edaa99014ef3f7bcc08b4a17d","unresolved":false,"context_lines":[{"line_number":3112,"context_line":"        server \u003d self._create_server(flavor_id\u003dfl_1, networks\u003d[])"},{"line_number":3113,"context_line":"        # before resize"},{"line_number":3114,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 0)"},{"line_number":3115,"context_line":"        server \u003d self._resize_server(server, fl_2)"},{"line_number":3116,"context_line":"        server \u003d self._confirm_resize(server)"},{"line_number":3117,"context_line":"        # after resize"},{"line_number":3118,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], 1024)"}],"source_content_type":"text/x-python","patch_set":14,"id":"e2c54bc5_5f04097b","line":3115,"in_reply_to":"f6eba471_4d3889e0","updated":"2023-03-03 11:20:10.000000000","message":"Done","commit_id":"a93f15f8ed606a0b311f63250509e5c1bf32fdc8"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8810e68e8806e117e64ee8861c5cdefa7cb6f0c1","unresolved":true,"context_lines":[{"line_number":3120,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":3121,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":3122,"context_line":"            ctxt, server[\u0027id\u0027])"},{"line_number":3123,"context_line":"        self.assertIn(\u0027swap\u0027, [bdm.guest_format for bdm in bdms])"},{"line_number":3124,"context_line":""},{"line_number":3125,"context_line":"    def test_swap_shrink_2048_to_1024(self):"},{"line_number":3126,"context_line":"        \"\"\"Test server resize to non-zero"}],"source_content_type":"text/x-python","patch_set":14,"id":"e5a11c5c_24633a02","line":3123,"updated":"2023-02-22 03:53:04.000000000","message":"I think we should also verify the swap size here, i.e. assertEqual(1024, bdm.volume_size). And do the same for all of the other tests where swap went from 0 to N. If we created a new BDM record, we want to verify it was created with the correct size.","commit_id":"a93f15f8ed606a0b311f63250509e5c1bf32fdc8"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9e35e21c6cb5175471f06dbf18ecf79bd27c5b66","unresolved":true,"context_lines":[{"line_number":3120,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":3121,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":3122,"context_line":"            ctxt, server[\u0027id\u0027])"},{"line_number":3123,"context_line":"        self.assertIn(\u0027swap\u0027, [bdm.guest_format for bdm in bdms])"},{"line_number":3124,"context_line":""},{"line_number":3125,"context_line":"    def test_swap_shrink_2048_to_1024(self):"},{"line_number":3126,"context_line":"        \"\"\"Test server resize to non-zero"}],"source_content_type":"text/x-python","patch_set":14,"id":"bc31fd83_18870224","line":3123,"in_reply_to":"0faa0bd8_7443a841","updated":"2023-02-22 13:34:27.000000000","message":"No, volume_size is the size for whatever block device that BDM record represents. So if it\u0027s swap it will be the swap size. I think the name \"volume_size\" is confusing though because in OpenStack the word \"volume\" most often means a Cinder volume.","commit_id":"a93f15f8ed606a0b311f63250509e5c1bf32fdc8"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"47445bd9fc44c435d578b3c96aa56b7f8a36b42c","unresolved":true,"context_lines":[{"line_number":3120,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":3121,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":3122,"context_line":"            ctxt, server[\u0027id\u0027])"},{"line_number":3123,"context_line":"        self.assertIn(\u0027swap\u0027, [bdm.guest_format for bdm in bdms])"},{"line_number":3124,"context_line":""},{"line_number":3125,"context_line":"    def test_swap_shrink_2048_to_1024(self):"},{"line_number":3126,"context_line":"        \"\"\"Test server resize to non-zero"}],"source_content_type":"text/x-python","patch_set":14,"id":"0faa0bd8_7443a841","line":3123,"in_reply_to":"4593e58d_a70de4aa","updated":"2023-02-22 12:28:13.000000000","message":"also in manager.py we never updated volume_size","commit_id":"a93f15f8ed606a0b311f63250509e5c1bf32fdc8"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"4abd79f97dac980edaa99014ef3f7bcc08b4a17d","unresolved":false,"context_lines":[{"line_number":3120,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":3121,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":3122,"context_line":"            ctxt, server[\u0027id\u0027])"},{"line_number":3123,"context_line":"        self.assertIn(\u0027swap\u0027, [bdm.guest_format for bdm in bdms])"},{"line_number":3124,"context_line":""},{"line_number":3125,"context_line":"    def test_swap_shrink_2048_to_1024(self):"},{"line_number":3126,"context_line":"        \"\"\"Test server resize to non-zero"}],"source_content_type":"text/x-python","patch_set":14,"id":"be944b17_c2cee771","line":3123,"in_reply_to":"7ff4de35_db063329","updated":"2023-03-03 11:20:10.000000000","message":"Done","commit_id":"a93f15f8ed606a0b311f63250509e5c1bf32fdc8"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"a3e6ac0c476c33d1b904be622f235aba82d37506","unresolved":true,"context_lines":[{"line_number":3120,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":3121,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":3122,"context_line":"            ctxt, server[\u0027id\u0027])"},{"line_number":3123,"context_line":"        self.assertIn(\u0027swap\u0027, [bdm.guest_format for bdm in bdms])"},{"line_number":3124,"context_line":""},{"line_number":3125,"context_line":"    def test_swap_shrink_2048_to_1024(self):"},{"line_number":3126,"context_line":"        \"\"\"Test server resize to non-zero"}],"source_content_type":"text/x-python","patch_set":14,"id":"7ff4de35_db063329","line":3123,"in_reply_to":"bc31fd83_18870224","updated":"2023-02-22 14:20:56.000000000","message":"Ack","commit_id":"a93f15f8ed606a0b311f63250509e5c1bf32fdc8"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"b73659ea76c3e7e40004f2d302c39c90aa3cd6ab","unresolved":true,"context_lines":[{"line_number":3120,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":3121,"context_line":"        bdms \u003d objects.BlockDeviceMappingList.get_by_instance_uuid("},{"line_number":3122,"context_line":"            ctxt, server[\u0027id\u0027])"},{"line_number":3123,"context_line":"        self.assertIn(\u0027swap\u0027, [bdm.guest_format for bdm in bdms])"},{"line_number":3124,"context_line":""},{"line_number":3125,"context_line":"    def test_swap_shrink_2048_to_1024(self):"},{"line_number":3126,"context_line":"        \"\"\"Test server resize to non-zero"}],"source_content_type":"text/x-python","patch_set":14,"id":"4593e58d_a70de4aa","line":3123,"in_reply_to":"e5a11c5c_24633a02","updated":"2023-02-22 12:05:57.000000000","message":"isn\u0027t in general, flavor.swap size is different from bdm.volume_size\n\nI understand \nbdm.volume_size is root disk \nwhile swap is a virtual space, it will on root disk but not as root disk.\n\nalthough I could not see any size on bdm other then volume_size !!\n\nbelow is an example bdm having swap:\n\n{\u0027id\u0027: 3, \u0027uuid\u0027: \u0027d030aafc-2b29-4439-8094-a2856da8fdcb\u0027, \u0027instance_uuid\u0027: \u0027d7c17841-01b3-4c60-9028-d69acf4b0ce9\u0027, \u0027source_type\u0027: \u0027blank\u0027, \u0027destination_type\u0027: \u0027local\u0027, \n\t\n\t\u0027guest_format\u0027: \u0027swap\u0027, \n\t\n\t\u0027device_type\u0027: \u0027disk\u0027, \u0027disk_bus\u0027: None, \u0027boot_index\u0027: -1, \u0027device_name\u0027: \u0027/dev/sdc\u0027, \u0027delete_on_termination\u0027: True, \u0027snapshot_id\u0027: None, \u0027volume_id\u0027: None, \n\t\n\t\u0027volume_size\u0027: 1024, \n\t\n\t\u0027image_id\u0027:None, \u0027no_device\u0027: False, \u0027connection_info\u0027: None, \u0027tag\u0027: None, \u0027attachment_id\u0027: None, \u0027volume_type\u0027: None, \u0027encrypted\u0027: False, \u0027encryption_secret_uuid\u0027: None, \u0027encryption_format\u0027: None, \u0027encryption_options\u0027: None, \u0027created_at\u0027: datetime.datetime(2023, 2, 22, 11, 32, 41, 85280, tzinfo\u003ddatetime.timezone.utc), \u0027updated_at\u0027: datetime.datetime(2023, 2, 22, 11, 32, 41, 256668, tzinfo\u003ddatetime.timezone.utc), \u0027deleted_at\u0027: None, \u0027deleted\u0027: False}","commit_id":"a93f15f8ed606a0b311f63250509e5c1bf32fdc8"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"b73659ea76c3e7e40004f2d302c39c90aa3cd6ab","unresolved":true,"context_lines":[{"line_number":3153,"context_line":"        # after resize"},{"line_number":3154,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], swap2)"},{"line_number":3155,"context_line":"        # verify BDM"},{"line_number":3156,"context_line":"        self._verify_swap_resize_in_bdm(server[\u0027id\u0027], swap2)"},{"line_number":3157,"context_line":""},{"line_number":3158,"context_line":"    def test_swap_expand_revert_0_to_2048(self):"},{"line_number":3159,"context_line":"        \"\"\"Test server resize to zero"}],"source_content_type":"text/x-python","patch_set":15,"id":"ee4e7c67_bedeb380","line":3156,"updated":"2023-02-22 12:05:57.000000000","message":"all tests with non-zero to non-zero resize are failing","commit_id":"aaf0032dcba647b83fe599311d18b8131c83f288"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"a3e6ac0c476c33d1b904be622f235aba82d37506","unresolved":true,"context_lines":[{"line_number":3153,"context_line":"        # after resize"},{"line_number":3154,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], swap2)"},{"line_number":3155,"context_line":"        # verify BDM"},{"line_number":3156,"context_line":"        self._verify_swap_resize_in_bdm(server[\u0027id\u0027], swap2)"},{"line_number":3157,"context_line":""},{"line_number":3158,"context_line":"    def test_swap_expand_revert_0_to_2048(self):"},{"line_number":3159,"context_line":"        \"\"\"Test server resize to zero"}],"source_content_type":"text/x-python","patch_set":15,"id":"def4bb23_5ba04d74","line":3156,"in_reply_to":"b73693e1_3d5bdceb","updated":"2023-02-22 14:20:56.000000000","message":"I saw this comment late.\n\nin manager while we updating the bdm, I removed check which say if flavor swap is expanding, update bdm with new flavor.\n\nnow all tests are passing.","commit_id":"aaf0032dcba647b83fe599311d18b8131c83f288"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"4abd79f97dac980edaa99014ef3f7bcc08b4a17d","unresolved":false,"context_lines":[{"line_number":3153,"context_line":"        # after resize"},{"line_number":3154,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], swap2)"},{"line_number":3155,"context_line":"        # verify BDM"},{"line_number":3156,"context_line":"        self._verify_swap_resize_in_bdm(server[\u0027id\u0027], swap2)"},{"line_number":3157,"context_line":""},{"line_number":3158,"context_line":"    def test_swap_expand_revert_0_to_2048(self):"},{"line_number":3159,"context_line":"        \"\"\"Test server resize to zero"}],"source_content_type":"text/x-python","patch_set":15,"id":"a150553f_bd1049db","line":3156,"in_reply_to":"def4bb23_5ba04d74","updated":"2023-03-03 11:20:10.000000000","message":"Done","commit_id":"aaf0032dcba647b83fe599311d18b8131c83f288"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"9e35e21c6cb5175471f06dbf18ecf79bd27c5b66","unresolved":true,"context_lines":[{"line_number":3153,"context_line":"        # after resize"},{"line_number":3154,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], swap2)"},{"line_number":3155,"context_line":"        # verify BDM"},{"line_number":3156,"context_line":"        self._verify_swap_resize_in_bdm(server[\u0027id\u0027], swap2)"},{"line_number":3157,"context_line":""},{"line_number":3158,"context_line":"    def test_swap_expand_revert_0_to_2048(self):"},{"line_number":3159,"context_line":"        \"\"\"Test server resize to zero"}],"source_content_type":"text/x-python","patch_set":15,"id":"b73693e1_3d5bdceb","line":3156,"in_reply_to":"ee4e7c67_bedeb380","updated":"2023-02-22 13:34:27.000000000","message":"Ugh, I think you uncovered something about swap disk creation that I had forgotten about, which is that swap is the only place the BDM size is *not* used ... only the flavor is used 😑 [1].\n\nSo yeah, we can\u0027t verify swap size in the BDM for the non-zero to non-zero cases. Sorry about that.\n\nCould you please replace those verifications with code comments that say something like \"We cannot verify the BDM size for the non-zero to non-zero case because the libvirt driver uses only the flavor to create the swap disk\"? To help people like me remember why we can\u0027t do that.\n\n[1] https://github.com/openstack/nova/blob/439c672/nova/virt/libvirt/driver.py#L4705-L4730","commit_id":"aaf0032dcba647b83fe599311d18b8131c83f288"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a342b7508c17403ea3dda9952efdb0b72cf68767","unresolved":true,"context_lines":[{"line_number":3111,"context_line":"        if swap_size !\u003d 0:"},{"line_number":3112,"context_line":"            self.assertIn(\u0027swap\u0027, [bdm.guest_format for bdm in bdms])"},{"line_number":3113,"context_line":"            self.assertIn(swap_size, ["},{"line_number":3114,"context_line":"                bdm.volume_size for bdm in bdms if bdm.guest_format \u003d\u003d \u0027swap\u0027]"},{"line_number":3115,"context_line":"            )"},{"line_number":3116,"context_line":"        else:"},{"line_number":3117,"context_line":"            self.assertNotIn(\u0027swap\u0027, [bdm.guest_format for bdm in bdms])"}],"source_content_type":"text/x-python","patch_set":21,"id":"3950eec7_9e9cc48f","line":3114,"updated":"2023-03-16 00:10:53.000000000","message":"I think we should add one more assert that verifies we only have one swap BDM for the instance.","commit_id":"fb65f941aa61b23af936641741289457d8b4a798"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"325c6c381f40d14bd4cf468d7e519c7aa6e48850","unresolved":true,"context_lines":[{"line_number":3111,"context_line":"        if swap_size !\u003d 0:"},{"line_number":3112,"context_line":"            self.assertIn(\u0027swap\u0027, [bdm.guest_format for bdm in bdms])"},{"line_number":3113,"context_line":"            self.assertIn(swap_size, ["},{"line_number":3114,"context_line":"                bdm.volume_size for bdm in bdms if bdm.guest_format \u003d\u003d \u0027swap\u0027]"},{"line_number":3115,"context_line":"            )"},{"line_number":3116,"context_line":"        else:"},{"line_number":3117,"context_line":"            self.assertNotIn(\u0027swap\u0027, [bdm.guest_format for bdm in bdms])"}],"source_content_type":"text/x-python","patch_set":21,"id":"b87119de_8bcf62d8","line":3114,"in_reply_to":"3950eec7_9e9cc48f","updated":"2023-03-16 08:02:22.000000000","message":"Done","commit_id":"fb65f941aa61b23af936641741289457d8b4a798"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"7940debea53ffb9dfbc9b9921a5ad57b6841d0f2","unresolved":false,"context_lines":[{"line_number":3111,"context_line":"        if swap_size !\u003d 0:"},{"line_number":3112,"context_line":"            self.assertIn(\u0027swap\u0027, [bdm.guest_format for bdm in bdms])"},{"line_number":3113,"context_line":"            self.assertIn(swap_size, ["},{"line_number":3114,"context_line":"                bdm.volume_size for bdm in bdms if bdm.guest_format \u003d\u003d \u0027swap\u0027]"},{"line_number":3115,"context_line":"            )"},{"line_number":3116,"context_line":"        else:"},{"line_number":3117,"context_line":"            self.assertNotIn(\u0027swap\u0027, [bdm.guest_format for bdm in bdms])"}],"source_content_type":"text/x-python","patch_set":21,"id":"2dab75b7_71f314f1","line":3114,"in_reply_to":"b87119de_8bcf62d8","updated":"2023-03-16 17:33:27.000000000","message":"Done","commit_id":"fb65f941aa61b23af936641741289457d8b4a798"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a342b7508c17403ea3dda9952efdb0b72cf68767","unresolved":true,"context_lines":[{"line_number":3333,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], swap2)"},{"line_number":3334,"context_line":"        # verify block device mapping"},{"line_number":3335,"context_line":"        self._verify_swap_resize_in_bdm(server[\u0027id\u0027], swap2)"},{"line_number":3336,"context_line":""},{"line_number":3337,"context_line":"    def _server_created_with_host(self):"},{"line_number":3338,"context_line":"        hostname \u003d self.compute1.host"},{"line_number":3339,"context_line":"        server_req \u003d self._build_server("}],"source_content_type":"text/x-python","patch_set":21,"id":"0cd1e274_cea5fa81","line":3336,"updated":"2023-03-16 00:10:53.000000000","message":"Just for informational purposes, if you wanted to make these DRYer, you could for example:\n\n def _test_swap(self, swap1, swap2, confirm\u003dTrue):\n     fl_1 \u003d self._create_flavor(swap\u003dswap1)\n     fl_2 \u003d self._create_flavor(swap\u003dswap2)\n     server \u003d self._create_server(flavor_id\u003dfl_1, networks\u003d[])\n     # before resize\n     self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], swap1)\n\n     server \u003d self._resize_server(server, fl_2)\n     self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], swap2)\n     self._verify_swap_resize_in_bdm(server[\u0027id\u0027], swap2)\n     \n     if confirm:\n         server \u003d self._confirm_resize(server)\n         # after resize\n         self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], swap2)\n         # verify block device mapping\n         self._verify_swap_resize_in_bdm(server[\u0027id\u0027], swap2)\n     else:\n         server \u003d self._revert_resize(server)\n         # after revert\n         self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], swap1)\n         # verify block device mapping\n         self._verify_swap_resize_in_bdm(server[\u0027id\u0027], swap1)\n         \n def test_swap_2048_to_2048_shrink_revert(self):\n     self._test_swap(2048, 2048, confirm\u003dFalse)    \n \n def test_swap_2048_to_2048_shrink_confirm(self):\n     self._test_swap(2048, 2048, confirm\u003dTrue)\n     \nOr you could probably use ddt. Sometimes I find ddt to be a bit more complicated to read personally, but it\u0027s another way to do multiple similar tests with different inputs. In nova we usually decorate the test with @data and @unpack and enumerate the inputs there.","commit_id":"fb65f941aa61b23af936641741289457d8b4a798"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"325c6c381f40d14bd4cf468d7e519c7aa6e48850","unresolved":true,"context_lines":[{"line_number":3333,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], swap2)"},{"line_number":3334,"context_line":"        # verify block device mapping"},{"line_number":3335,"context_line":"        self._verify_swap_resize_in_bdm(server[\u0027id\u0027], swap2)"},{"line_number":3336,"context_line":""},{"line_number":3337,"context_line":"    def _server_created_with_host(self):"},{"line_number":3338,"context_line":"        hostname \u003d self.compute1.host"},{"line_number":3339,"context_line":"        server_req \u003d self._build_server("}],"source_content_type":"text/x-python","patch_set":21,"id":"a1e7b87b_937335b5","line":3336,"in_reply_to":"0cd1e274_cea5fa81","updated":"2023-03-16 08:02:22.000000000","message":"with ddt, in console test results are printed like this, name seems confusing.\n\n{7} nova.tests.functional.test_servers.ServerMovingTests.test_swap_resize_11__64__64__False_ [3.855350s] ... ok\n{3} nova.tests.functional.test_servers.ServerMovingTests.test_swap_resize_05__32__0__False_ [3.865369s] ... ok\n{6} nova.tests.functional.test_servers.ServerMovingTests.test_swap_resize_09__64__32__False_ [3.935587s] ... ok\n{2} nova.tests.functional.test_servers.ServerMovingTests.test_swap_resize_07__32__64__False_ [3.895968s] ... ok\n{1} nova.tests.functional.test_servers.ServerMovingTests.test_swap_resize_01__0__0__False_ [4.005309s] ... ok\n{4} nova.tests.functional.test_servers.ServerMovingTests.test_swap_resize_12__64__64__True_ [4.020613s] ... ok\n{0} nova.tests.functional.test_servers.ServerMovingTests.test_swap_resize_02__0__0__True_ [3.998772s] ... ok","commit_id":"fb65f941aa61b23af936641741289457d8b4a798"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"41ed89749554f93a7871fe6eb9e2e9fc3397166b","unresolved":true,"context_lines":[{"line_number":3333,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], swap2)"},{"line_number":3334,"context_line":"        # verify block device mapping"},{"line_number":3335,"context_line":"        self._verify_swap_resize_in_bdm(server[\u0027id\u0027], swap2)"},{"line_number":3336,"context_line":""},{"line_number":3337,"context_line":"    def _server_created_with_host(self):"},{"line_number":3338,"context_line":"        hostname \u003d self.compute1.host"},{"line_number":3339,"context_line":"        server_req \u003d self._build_server("}],"source_content_type":"text/x-python","patch_set":21,"id":"eec3b25f_7091089a","line":3336,"in_reply_to":"a1e7b87b_937335b5","updated":"2023-03-16 17:04:36.000000000","message":"I agree. That\u0027s why I personally don\u0027t like ddt in some cases when it makes it hard to read and understand. It\u0027s up to you how you prefer to do it.","commit_id":"fb65f941aa61b23af936641741289457d8b4a798"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"7940debea53ffb9dfbc9b9921a5ad57b6841d0f2","unresolved":false,"context_lines":[{"line_number":3333,"context_line":"        self.assertEqual(server[\u0027flavor\u0027][\u0027swap\u0027], swap2)"},{"line_number":3334,"context_line":"        # verify block device mapping"},{"line_number":3335,"context_line":"        self._verify_swap_resize_in_bdm(server[\u0027id\u0027], swap2)"},{"line_number":3336,"context_line":""},{"line_number":3337,"context_line":"    def _server_created_with_host(self):"},{"line_number":3338,"context_line":"        hostname \u003d self.compute1.host"},{"line_number":3339,"context_line":"        server_req \u003d self._build_server("}],"source_content_type":"text/x-python","patch_set":21,"id":"abc40274_929addc8","line":3336,"in_reply_to":"eec3b25f_7091089a","updated":"2023-03-16 17:33:27.000000000","message":"ACK, removed ddt.","commit_id":"fb65f941aa61b23af936641741289457d8b4a798"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"b7bbc77b458f49d775ffbe57352bf0de9f6b6b2a","unresolved":true,"context_lines":[{"line_number":52,"context_line":"from nova.virt import fake"},{"line_number":53,"context_line":"from nova import volume"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"CONF \u003d cfg.CONF"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":24,"id":"1ecd1b93_69908d7c","line":55,"updated":"2023-04-06 09:43:15.000000000","message":"unnecessary change","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"2ff613dd27977e916a3e5239cc466a739a1ee2e2","unresolved":false,"context_lines":[{"line_number":52,"context_line":"from nova.virt import fake"},{"line_number":53,"context_line":"from nova import volume"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"CONF \u003d cfg.CONF"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"LOG \u003d logging.getLogger(__name__)"}],"source_content_type":"text/x-python","patch_set":24,"id":"db80d3bb_1787e9a4","line":55,"in_reply_to":"1ecd1b93_69908d7c","updated":"2023-05-03 10:28:34.000000000","message":"having 2 blank lines after imports is not a pep8 requirement, but it seems clean, same way as adding 2 blank lines after a class, also its present in others tests so added here too.","commit_id":"1732f33c21c6f3c044a403e58e86ad64d8ed67b5"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"9e0c6dc7914c296478979166166ac609bace0132","unresolved":true,"context_lines":[{"line_number":3141,"context_line":"            # verify block device mapping"},{"line_number":3142,"context_line":"            self._verify_swap_resize_in_bdm(server[\u0027id\u0027], swap1)"},{"line_number":3143,"context_line":""},{"line_number":3144,"context_line":"    def test_swap_no_swap_resize_revert(self):"},{"line_number":3145,"context_line":"        self._test_swap_resize(0, 0, confirm\u003dFalse)"},{"line_number":3146,"context_line":""},{"line_number":3147,"context_line":"    def test_swap_no_swap_resize_confirm(self):"},{"line_number":3148,"context_line":"        self._test_swap_resize(0, 0)"},{"line_number":3149,"context_line":""},{"line_number":3150,"context_line":"    def test_swap_0_to_1024_expand_revert(self):"},{"line_number":3151,"context_line":"        self._test_swap_resize(0, 1024, confirm\u003dFalse)"},{"line_number":3152,"context_line":""},{"line_number":3153,"context_line":"    def test_swap_0_to_1024_expand_confirm(self):"},{"line_number":3154,"context_line":"        self._test_swap_resize(0, 1024)"},{"line_number":3155,"context_line":""},{"line_number":3156,"context_line":"    def test_swap_1024_to_0_shrink_revert(self):"},{"line_number":3157,"context_line":"        self._test_swap_resize(1024, 0, confirm\u003dFalse)"},{"line_number":3158,"context_line":""},{"line_number":3159,"context_line":"    def test_swap_1024_to_0_shrink_confirm(self):"},{"line_number":3160,"context_line":"        self._test_swap_resize(1024, 0)"},{"line_number":3161,"context_line":""},{"line_number":3162,"context_line":"    def test_swap_1024_to_2048_expand_revert(self):"},{"line_number":3163,"context_line":"        self._test_swap_resize(1024, 2048, confirm\u003dFalse)"},{"line_number":3164,"context_line":""},{"line_number":3165,"context_line":"    def test_swap_1024_to_2048_expand_confirm(self):"},{"line_number":3166,"context_line":"        self._test_swap_resize(1024, 2048)"},{"line_number":3167,"context_line":""},{"line_number":3168,"context_line":"    def test_swap_2048_to_1024_shrink_revert(self):"},{"line_number":3169,"context_line":"        self._test_swap_resize(2048, 1024, confirm\u003dFalse)"},{"line_number":3170,"context_line":""},{"line_number":3171,"context_line":"    def test_swap_2048_to_1024_shrink_confirm(self):"},{"line_number":3172,"context_line":"        self._test_swap_resize(2048, 1024)"},{"line_number":3173,"context_line":""},{"line_number":3174,"context_line":"    def test_swap_2048_to_2048_shrink_revert(self):"},{"line_number":3175,"context_line":"        self._test_swap_resize(2048, 2048, confirm\u003dFalse)"},{"line_number":3176,"context_line":""},{"line_number":3177,"context_line":"    def test_swap_2048_to_2048_shrink_confirm(self):"},{"line_number":3178,"context_line":"        self._test_swap_resize(2048, 2048)"},{"line_number":3179,"context_line":""},{"line_number":3180,"context_line":"    def _server_created_with_host(self):"},{"line_number":3181,"context_line":"        hostname \u003d self.compute1.host"}],"source_content_type":"text/x-python","patch_set":26,"id":"2442b17a_72689fb0","line":3178,"range":{"start_line":3144,"start_character":0,"end_line":3178,"end_character":42},"updated":"2023-06-12 18:29:13.000000000","message":"so there are 3 parematers so 2^3 or 8 combinations but you have\n12 test cases.\n\ni guess the extra 4 are when the swap increase(expand) or decrease(shrink) for both confirm and revert.\n\ni guess that is fine but in terms of the code you are testing that is the same a \n    def test_swap_2048_to_2048_shrink_revert(self):\n    def test_swap_2048_to_2048_shrink_confirm(self):\n\n\nalso you should remvoe shrik form both of those as the swap is not shrinking or expanding.\n\nthose are testing the same size.\n\nthius is a good example of a test set that could be done with DDT by the way","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"61dfb9b82e3405c50be739d9fb1ef0575621c49a","unresolved":true,"context_lines":[{"line_number":3141,"context_line":"            # verify block device mapping"},{"line_number":3142,"context_line":"            self._verify_swap_resize_in_bdm(server[\u0027id\u0027], swap1)"},{"line_number":3143,"context_line":""},{"line_number":3144,"context_line":"    def test_swap_no_swap_resize_revert(self):"},{"line_number":3145,"context_line":"        self._test_swap_resize(0, 0, confirm\u003dFalse)"},{"line_number":3146,"context_line":""},{"line_number":3147,"context_line":"    def test_swap_no_swap_resize_confirm(self):"},{"line_number":3148,"context_line":"        self._test_swap_resize(0, 0)"},{"line_number":3149,"context_line":""},{"line_number":3150,"context_line":"    def test_swap_0_to_1024_expand_revert(self):"},{"line_number":3151,"context_line":"        self._test_swap_resize(0, 1024, confirm\u003dFalse)"},{"line_number":3152,"context_line":""},{"line_number":3153,"context_line":"    def test_swap_0_to_1024_expand_confirm(self):"},{"line_number":3154,"context_line":"        self._test_swap_resize(0, 1024)"},{"line_number":3155,"context_line":""},{"line_number":3156,"context_line":"    def test_swap_1024_to_0_shrink_revert(self):"},{"line_number":3157,"context_line":"        self._test_swap_resize(1024, 0, confirm\u003dFalse)"},{"line_number":3158,"context_line":""},{"line_number":3159,"context_line":"    def test_swap_1024_to_0_shrink_confirm(self):"},{"line_number":3160,"context_line":"        self._test_swap_resize(1024, 0)"},{"line_number":3161,"context_line":""},{"line_number":3162,"context_line":"    def test_swap_1024_to_2048_expand_revert(self):"},{"line_number":3163,"context_line":"        self._test_swap_resize(1024, 2048, confirm\u003dFalse)"},{"line_number":3164,"context_line":""},{"line_number":3165,"context_line":"    def test_swap_1024_to_2048_expand_confirm(self):"},{"line_number":3166,"context_line":"        self._test_swap_resize(1024, 2048)"},{"line_number":3167,"context_line":""},{"line_number":3168,"context_line":"    def test_swap_2048_to_1024_shrink_revert(self):"},{"line_number":3169,"context_line":"        self._test_swap_resize(2048, 1024, confirm\u003dFalse)"},{"line_number":3170,"context_line":""},{"line_number":3171,"context_line":"    def test_swap_2048_to_1024_shrink_confirm(self):"},{"line_number":3172,"context_line":"        self._test_swap_resize(2048, 1024)"},{"line_number":3173,"context_line":""},{"line_number":3174,"context_line":"    def test_swap_2048_to_2048_shrink_revert(self):"},{"line_number":3175,"context_line":"        self._test_swap_resize(2048, 2048, confirm\u003dFalse)"},{"line_number":3176,"context_line":""},{"line_number":3177,"context_line":"    def test_swap_2048_to_2048_shrink_confirm(self):"},{"line_number":3178,"context_line":"        self._test_swap_resize(2048, 2048)"},{"line_number":3179,"context_line":""},{"line_number":3180,"context_line":"    def _server_created_with_host(self):"},{"line_number":3181,"context_line":"        hostname \u003d self.compute1.host"}],"source_content_type":"text/x-python","patch_set":26,"id":"d09661d5_50705d64","line":3178,"range":{"start_line":3144,"start_character":0,"end_line":3178,"end_character":42},"in_reply_to":"2442b17a_72689fb0","updated":"2023-06-13 20:57:07.000000000","message":"`remvoe shrik form both of those as the swap is not shrinking or expanding.\nthose are testing the same size.`: ack, I just made all possibe test scenarios and added them, removed suggested 2\u0027s.\n\n\n`thius is a good example of a test set that could be done with DDT by the way`: tried earlier with ddt, in console test results are printed like this, name seems confusing with added postfix, so removed it.\n`\n{7} nova.tests.functional.test_servers.ServerMovingTests.test_swap_resize_11__64__64__False_ [3.855350s] ... ok\n{3} nova.tests.functional.test_servers.ServerMovingTests.test_swap_resize_05__32__0__False_ [3.865369s] ... ok\n{6} nova.tests.functional.test_servers.ServerMovingTests.test_swap_resize_09__64__32__False_ [3.935587s] ... ok\n{2} nova.tests.functional.test_servers.ServerMovingTests.test_swap_resize_07__32__64__False_ [3.895968s] ... ok\n{1} nova.tests.functional.test_servers.ServerMovingTests.test_swap_resize_01__0__0__False_ [4.005309s] ... ok\n{4} nova.tests.functional.test_servers.ServerMovingTests.test_swap_resize_12__64__64__True_ [4.020613s] ... ok\n{0} nova.tests.functional.test_servers.ServerMovingTests.test_swap_resize_02__0__0__True_ [3.998772s] ... ok`","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"edb4d59d96bcd481fbc87632dfa572aa0c11f1c0","unresolved":false,"context_lines":[{"line_number":3141,"context_line":"            # verify block device mapping"},{"line_number":3142,"context_line":"            self._verify_swap_resize_in_bdm(server[\u0027id\u0027], swap1)"},{"line_number":3143,"context_line":""},{"line_number":3144,"context_line":"    def test_swap_no_swap_resize_revert(self):"},{"line_number":3145,"context_line":"        self._test_swap_resize(0, 0, confirm\u003dFalse)"},{"line_number":3146,"context_line":""},{"line_number":3147,"context_line":"    def test_swap_no_swap_resize_confirm(self):"},{"line_number":3148,"context_line":"        self._test_swap_resize(0, 0)"},{"line_number":3149,"context_line":""},{"line_number":3150,"context_line":"    def test_swap_0_to_1024_expand_revert(self):"},{"line_number":3151,"context_line":"        self._test_swap_resize(0, 1024, confirm\u003dFalse)"},{"line_number":3152,"context_line":""},{"line_number":3153,"context_line":"    def test_swap_0_to_1024_expand_confirm(self):"},{"line_number":3154,"context_line":"        self._test_swap_resize(0, 1024)"},{"line_number":3155,"context_line":""},{"line_number":3156,"context_line":"    def test_swap_1024_to_0_shrink_revert(self):"},{"line_number":3157,"context_line":"        self._test_swap_resize(1024, 0, confirm\u003dFalse)"},{"line_number":3158,"context_line":""},{"line_number":3159,"context_line":"    def test_swap_1024_to_0_shrink_confirm(self):"},{"line_number":3160,"context_line":"        self._test_swap_resize(1024, 0)"},{"line_number":3161,"context_line":""},{"line_number":3162,"context_line":"    def test_swap_1024_to_2048_expand_revert(self):"},{"line_number":3163,"context_line":"        self._test_swap_resize(1024, 2048, confirm\u003dFalse)"},{"line_number":3164,"context_line":""},{"line_number":3165,"context_line":"    def test_swap_1024_to_2048_expand_confirm(self):"},{"line_number":3166,"context_line":"        self._test_swap_resize(1024, 2048)"},{"line_number":3167,"context_line":""},{"line_number":3168,"context_line":"    def test_swap_2048_to_1024_shrink_revert(self):"},{"line_number":3169,"context_line":"        self._test_swap_resize(2048, 1024, confirm\u003dFalse)"},{"line_number":3170,"context_line":""},{"line_number":3171,"context_line":"    def test_swap_2048_to_1024_shrink_confirm(self):"},{"line_number":3172,"context_line":"        self._test_swap_resize(2048, 1024)"},{"line_number":3173,"context_line":""},{"line_number":3174,"context_line":"    def test_swap_2048_to_2048_shrink_revert(self):"},{"line_number":3175,"context_line":"        self._test_swap_resize(2048, 2048, confirm\u003dFalse)"},{"line_number":3176,"context_line":""},{"line_number":3177,"context_line":"    def test_swap_2048_to_2048_shrink_confirm(self):"},{"line_number":3178,"context_line":"        self._test_swap_resize(2048, 2048)"},{"line_number":3179,"context_line":""},{"line_number":3180,"context_line":"    def _server_created_with_host(self):"},{"line_number":3181,"context_line":"        hostname \u003d self.compute1.host"}],"source_content_type":"text/x-python","patch_set":26,"id":"bfd5a0f9_136e750b","line":3178,"range":{"start_line":3144,"start_character":0,"end_line":3178,"end_character":42},"in_reply_to":"d09661d5_50705d64","updated":"2023-07-14 16:54:56.000000000","message":"yes, updated.","commit_id":"54269de3e86e78b2b74c93b1df700aa4ee6f43a4"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"17e35dcb1bb45d989e0de3eec7d8526514cc638e","unresolved":true,"context_lines":[{"line_number":3344,"context_line":""},{"line_number":3345,"context_line":"    def test_swap_shrink_2048_to_1024_revert(self):"},{"line_number":3346,"context_line":"        self._test_swap_resize(2048, 1024, confirm\u003dFalse)"},{"line_number":3347,"context_line":""},{"line_number":3348,"context_line":"    def _server_created_with_host(self):"},{"line_number":3349,"context_line":"        hostname \u003d self.compute1.host"},{"line_number":3350,"context_line":"        server_req \u003d self._build_server("}],"source_content_type":"text/x-python","patch_set":32,"id":"ecb15a5f_1d3bdc37","line":3347,"updated":"2024-01-17 05:53:30.000000000","message":"we are missing the test where swap size does not exist in either the source flavor or the destination flavor","commit_id":"8c4bbe0b7eda5ba2bf2c2cd48de5ecb313190469"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"6be1633114e78b8cd7891d19ba4d17aa176a1e74","unresolved":true,"context_lines":[{"line_number":3344,"context_line":""},{"line_number":3345,"context_line":"    def test_swap_shrink_2048_to_1024_revert(self):"},{"line_number":3346,"context_line":"        self._test_swap_resize(2048, 1024, confirm\u003dFalse)"},{"line_number":3347,"context_line":""},{"line_number":3348,"context_line":"    def _server_created_with_host(self):"},{"line_number":3349,"context_line":"        hostname \u003d self.compute1.host"},{"line_number":3350,"context_line":"        server_req \u003d self._build_server("}],"source_content_type":"text/x-python","patch_set":32,"id":"decfa4d1_81a3b2e7","line":3347,"in_reply_to":"7f20cc91_0323937d","updated":"2024-01-31 07:42:06.000000000","message":"amit is correct this is an integer feild so it will never be a bool\n\nhowever rajat is correct that we dont have test coverage for when both values are 0\nor both values are the same non zero value\n\nwe should have two addtional test cases.","commit_id":"8c4bbe0b7eda5ba2bf2c2cd48de5ecb313190469"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"0a0e61c1806eef57bffd08865cf2936b50768e2c","unresolved":false,"context_lines":[{"line_number":3344,"context_line":""},{"line_number":3345,"context_line":"    def test_swap_shrink_2048_to_1024_revert(self):"},{"line_number":3346,"context_line":"        self._test_swap_resize(2048, 1024, confirm\u003dFalse)"},{"line_number":3347,"context_line":""},{"line_number":3348,"context_line":"    def _server_created_with_host(self):"},{"line_number":3349,"context_line":"        hostname \u003d self.compute1.host"},{"line_number":3350,"context_line":"        server_req \u003d self._build_server("}],"source_content_type":"text/x-python","patch_set":32,"id":"edd505ed_b0ffc21f","line":3347,"in_reply_to":"decfa4d1_81a3b2e7","updated":"2024-02-15 04:16:28.000000000","message":"ack, added tests, thanks","commit_id":"8c4bbe0b7eda5ba2bf2c2cd48de5ecb313190469"},{"author":{"_account_id":34860,"name":"Amit Uniyal","email":"auniyal@redhat.com","username":"auniyal"},"change_message_id":"6a8e2adfdc0bc3f141aee18d5ad490f0bb640f62","unresolved":true,"context_lines":[{"line_number":3344,"context_line":""},{"line_number":3345,"context_line":"    def test_swap_shrink_2048_to_1024_revert(self):"},{"line_number":3346,"context_line":"        self._test_swap_resize(2048, 1024, confirm\u003dFalse)"},{"line_number":3347,"context_line":""},{"line_number":3348,"context_line":"    def _server_created_with_host(self):"},{"line_number":3349,"context_line":"        hostname \u003d self.compute1.host"},{"line_number":3350,"context_line":"        server_req \u003d self._build_server("}],"source_content_type":"text/x-python","patch_set":32,"id":"7f20cc91_0323937d","line":3347,"in_reply_to":"ecb15a5f_1d3bdc37","updated":"2024-01-29 06:48:13.000000000","message":"if we do not provide swap for flavor creation the default value will be set to 0.\nswap\u003d0 is covered in other resize server test, where swap is not added at all.\n\nfor example: ServerMovingTests.test_resize_confirm_same_host","commit_id":"8c4bbe0b7eda5ba2bf2c2cd48de5ecb313190469"}]}