)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":28271,"name":"Josephine Seifert","email":"josephine.seifert@cloudandheat.com","username":"josei"},"change_message_id":"3cb4651b3349bfd5e6995f91353a4b9ef3daa050","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5841b27a_90cf4d7d","updated":"2024-08-27 09:44:47.000000000","message":"recheck - merged os-brick patch will solve all tox-Errors","commit_id":"cd5cc5dcf39d3c282f9f1cecb7dddec4e41ae5b3"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"b763739a6fe3817ca64fe5ae0da25e57d6e5dbab","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"cd7fec31_e6941a5c","updated":"2025-08-15 08:53:42.000000000","message":"Dear Dan,\n\nthanks for the review! I tried addressing all your points appropriately.\n\nAnd for the record, since Sean raised that point, we do not need to up the os-brick version in the requirements because starting with 6.9.0 the necessary changes are already included [1] and we depend on 6.10.0 in Nova already.\n\n[1] https://github.com/openstack/os-brick/commit/2b42257eff03615816dc66d3171b87c22ad67eb1","commit_id":"dfb7adb1b6a504e44436dccb2d623d7df3a5fa6f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dd4cf04ad2902be7e3eeb020cee99b2e6e79aad1","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":3,"id":"e2bcafe4_14247757","updated":"2025-08-14 14:31:11.000000000","message":"Just a few nits that maybe add up to a -1. Overall, this seems like a net-zero change for us and it looks pretty well covered in the unit tests. I want to recheck it and look through the (now expired) devstack job logs while these items get addressed.\n\nIMHO, this is just a \"use a library function instead of our own stuff\" change, with the addition of a defensive support-the-new-generic-key-property-too change. Thus, fine to merge now and without a lot of fanfare.","commit_id":"dfb7adb1b6a504e44436dccb2d623d7df3a5fa6f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"14afaa62e86b285988daff475ff20dbd84e6ed36","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"a0448e74_03ef20c3","updated":"2025-08-14 14:31:22.000000000","message":"recheck get fresh logs","commit_id":"dfb7adb1b6a504e44436dccb2d623d7df3a5fa6f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"360ca247540f88f9c450aef406693d1dfae70f26","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"513a7bfc_c66285a1","in_reply_to":"cd7fec31_e6941a5c","updated":"2025-08-15 13:40:51.000000000","message":"Yep, I checked this and made a note in the previous patchset.","commit_id":"dfb7adb1b6a504e44436dccb2d623d7df3a5fa6f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"360ca247540f88f9c450aef406693d1dfae70f26","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"aff32bb4_59d76dd1","updated":"2025-08-15 13:40:51.000000000","message":"Changes look good, modulo the failed tests, which I assume have been addressed in the latest patch. I\u0027ll circle back when the CI report for that is back.","commit_id":"0418d9ab2d40db2aae8f6bd9d3ddc5f4482cbaf3"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a3bd6cf458f2a6bb26cf282e899699f4c202b5ae","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"40fe12e7_43794705","updated":"2025-08-15 19:19:19.000000000","message":"Noting that `get_passphrase_from_secret` was released in os-brick 6.9.0 [1] and we already have `os-brick\u003e\u003d6.10.0` in our requirements.txt, so no version bump is needed. Looks good to me.\n\n[1] https://github.com/openstack/os-brick/commit/2b42257eff03615816dc66d3171b87c22ad67eb1","commit_id":"fe87ac8cd1d791ecf0e0114ebed810b43af20008"},{"author":{"_account_id":37598,"name":"Ivan Anfimov","display_name":"Ivan Anfimov","email":"lazekteam@gmail.com","username":"anfimovir"},"change_message_id":"319cde2296022430594de2c610083a7ed37026b4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"5810e157_30a50a0b","updated":"2025-12-12 12:58:11.000000000","message":"recheck (depends now merged)","commit_id":"fe87ac8cd1d791ecf0e0114ebed810b43af20008"}],"nova/compute/api.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dd4cf04ad2902be7e3eeb020cee99b2e6e79aad1","unresolved":true,"context_lines":[{"line_number":737,"context_line":"        # cinder_encryption_key_id was against the original image as we are now"},{"line_number":738,"context_line":"        # booting from an encrypted volume."},{"line_number":739,"context_line":"        if image_properties.get(\u0027os_encrypt_key_id\u0027) and image_id:"},{"line_number":740,"context_line":"            reason \u003d _(\u0027Direct booting of an encrypted image is unsupported.\u0027)"},{"line_number":741,"context_line":"            raise exception.ImageUnacceptable(image_id\u003dimage_id,"},{"line_number":742,"context_line":"                                              reason\u003dreason)"},{"line_number":743,"context_line":"        # NOTE(mhen): cinder_encryption_key_id is being deprecated and will be"}],"source_content_type":"text/x-python","patch_set":3,"id":"a950a017_a10cf171","line":740,"updated":"2025-08-14 14:31:11.000000000","message":"This is a different error message from the existing one on L747. AFAIK, they\u0027re checking for exactly the same condition but raise different errors to the user which seems confusing to me. Your new one here seems more appropriate/generic, so maybe we stick with that, but... can we raise the same thing for both?","commit_id":"dfb7adb1b6a504e44436dccb2d623d7df3a5fa6f"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"ea8bad9d447984be3c1131348e5b9137f4232ced","unresolved":true,"context_lines":[{"line_number":737,"context_line":"        # cinder_encryption_key_id was against the original image as we are now"},{"line_number":738,"context_line":"        # booting from an encrypted volume."},{"line_number":739,"context_line":"        if image_properties.get(\u0027os_encrypt_key_id\u0027) and image_id:"},{"line_number":740,"context_line":"            reason \u003d _(\u0027Direct booting of an encrypted image is unsupported.\u0027)"},{"line_number":741,"context_line":"            raise exception.ImageUnacceptable(image_id\u003dimage_id,"},{"line_number":742,"context_line":"                                              reason\u003dreason)"},{"line_number":743,"context_line":"        # NOTE(mhen): cinder_encryption_key_id is being deprecated and will be"}],"source_content_type":"text/x-python","patch_set":3,"id":"cf6104ed_019af969","line":740,"in_reply_to":"a950a017_a10cf171","updated":"2025-08-15 08:17:34.000000000","message":"There was a reason for that.\n\nHistorically, only Cinder was ever able to actually create encrypted images (by dumping its LUKS-encrypted disks 1:1 into an image). For those images it attached \"cinder_encryption_key_id\".\n\nWith the rework we also enable users (or other services) to upload encrypted images, which can be consumed by Cinder. Those images will carry an \"os_encrypt_key_id\" property instead.\n\nSo, if it is \"cinder_encryption_key_id\", then it must\u0027ve originated from Cinder. That\u0027s why the message specifically mentions volumes in the related check. In the other case (\"os_encrypt_key_id\"), it may have originated from other users/services as well, so the message is a bit more generic there.\n\nBut now that you mention it, I think in the long run it won\u0027t matter, considering the old name is deprecated anyway and Cinder will move to \"os_encrypt_key_id\" with our patchsets too.\n\nI will change that, thanks for point it out.","commit_id":"dfb7adb1b6a504e44436dccb2d623d7df3a5fa6f"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"b763739a6fe3817ca64fe5ae0da25e57d6e5dbab","unresolved":false,"context_lines":[{"line_number":737,"context_line":"        # cinder_encryption_key_id was against the original image as we are now"},{"line_number":738,"context_line":"        # booting from an encrypted volume."},{"line_number":739,"context_line":"        if image_properties.get(\u0027os_encrypt_key_id\u0027) and image_id:"},{"line_number":740,"context_line":"            reason \u003d _(\u0027Direct booting of an encrypted image is unsupported.\u0027)"},{"line_number":741,"context_line":"            raise exception.ImageUnacceptable(image_id\u003dimage_id,"},{"line_number":742,"context_line":"                                              reason\u003dreason)"},{"line_number":743,"context_line":"        # NOTE(mhen): cinder_encryption_key_id is being deprecated and will be"}],"source_content_type":"text/x-python","patch_set":3,"id":"19e8e964_6adc8016","line":740,"in_reply_to":"cf6104ed_019af969","updated":"2025-08-15 08:53:42.000000000","message":"I have unified the check and error message now.","commit_id":"dfb7adb1b6a504e44436dccb2d623d7df3a5fa6f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dd4cf04ad2902be7e3eeb020cee99b2e6e79aad1","unresolved":true,"context_lines":[{"line_number":743,"context_line":"        # NOTE(mhen): cinder_encryption_key_id is being deprecated and will be"},{"line_number":744,"context_line":"        # superseded by os_encrypt_key_id. During the deprecation, the cinder-"},{"line_number":745,"context_line":"        # specific name is still supported."},{"line_number":746,"context_line":"        if image_properties.get(\u0027cinder_encryption_key_id\u0027) and image_id:"},{"line_number":747,"context_line":"            reason \u003d _(\u0027Direct booting of an image uploaded from an \u0027"},{"line_number":748,"context_line":"                       \u0027encrypted volume is unsupported.\u0027)"},{"line_number":749,"context_line":"            raise exception.ImageUnacceptable(image_id\u003dimage_id,"}],"source_content_type":"text/x-python","patch_set":3,"id":"6b5370d0_2f064931","line":746,"updated":"2025-08-14 14:31:11.000000000","message":"Seems like maybe we could DRY this up a bit and avoid the confusion that we\u0027re adding a new restriction here. Could we just have one conditional that checks for both/either and raises the same error?","commit_id":"dfb7adb1b6a504e44436dccb2d623d7df3a5fa6f"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"b763739a6fe3817ca64fe5ae0da25e57d6e5dbab","unresolved":false,"context_lines":[{"line_number":743,"context_line":"        # NOTE(mhen): cinder_encryption_key_id is being deprecated and will be"},{"line_number":744,"context_line":"        # superseded by os_encrypt_key_id. During the deprecation, the cinder-"},{"line_number":745,"context_line":"        # specific name is still supported."},{"line_number":746,"context_line":"        if image_properties.get(\u0027cinder_encryption_key_id\u0027) and image_id:"},{"line_number":747,"context_line":"            reason \u003d _(\u0027Direct booting of an image uploaded from an \u0027"},{"line_number":748,"context_line":"                       \u0027encrypted volume is unsupported.\u0027)"},{"line_number":749,"context_line":"            raise exception.ImageUnacceptable(image_id\u003dimage_id,"}],"source_content_type":"text/x-python","patch_set":3,"id":"6951e1c0_18750fc5","line":746,"in_reply_to":"6b5370d0_2f064931","updated":"2025-08-15 08:53:42.000000000","message":"Done","commit_id":"dfb7adb1b6a504e44436dccb2d623d7df3a5fa6f"}],"nova/compute/utils.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dd4cf04ad2902be7e3eeb020cee99b2e6e79aad1","unresolved":true,"context_lines":[{"line_number":63,"context_line":"# (lowercase) name."},{"line_number":64,"context_line":"# NOTE(mhen): The cinder_encryption_* attributes are being deprecated and"},{"line_number":65,"context_line":"# superseded by the os_encrypt_* counterparts. For the duration of the"},{"line_number":66,"context_line":"# deprecation timeframe, the old attribute names are still supported."},{"line_number":67,"context_line":"NON_INHERITABLE_IMAGE_PROPERTIES \u003d frozenset(["},{"line_number":68,"context_line":"    \u0027cinder_encryption_key_id\u0027,"},{"line_number":69,"context_line":"    \u0027cinder_encryption_key_deletion_policy\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"9d968219_56923402","line":66,"updated":"2025-08-14 14:31:11.000000000","message":"We try to never fully break image property compatibility, so I\u0027m not sure your verbiage about \"deprecation timeframe\" is correct here. Can you just leave the comment about \"being deprecated\" but remove the second sentence \"For the duration of the deprecation timeframe...\" part?","commit_id":"dfb7adb1b6a504e44436dccb2d623d7df3a5fa6f"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"b763739a6fe3817ca64fe5ae0da25e57d6e5dbab","unresolved":false,"context_lines":[{"line_number":63,"context_line":"# (lowercase) name."},{"line_number":64,"context_line":"# NOTE(mhen): The cinder_encryption_* attributes are being deprecated and"},{"line_number":65,"context_line":"# superseded by the os_encrypt_* counterparts. For the duration of the"},{"line_number":66,"context_line":"# deprecation timeframe, the old attribute names are still supported."},{"line_number":67,"context_line":"NON_INHERITABLE_IMAGE_PROPERTIES \u003d frozenset(["},{"line_number":68,"context_line":"    \u0027cinder_encryption_key_id\u0027,"},{"line_number":69,"context_line":"    \u0027cinder_encryption_key_deletion_policy\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"d53abe01_39480a7e","line":66,"in_reply_to":"9d968219_56923402","updated":"2025-08-15 08:53:42.000000000","message":"Done","commit_id":"dfb7adb1b6a504e44436dccb2d623d7df3a5fa6f"}],"nova/tests/unit/api/openstack/compute/test_servers.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dd4cf04ad2902be7e3eeb020cee99b2e6e79aad1","unresolved":true,"context_lines":[{"line_number":4617,"context_line":"                           status\u003d\u0027active\u0027,"},{"line_number":4618,"context_line":"                           properties\u003ddict("},{"line_number":4619,"context_line":"                               cinder_encryption_key_id\u003dfakes.FAKE_UUID)))"},{"line_number":4620,"context_line":"    def test_create_server_image_nonbootable_cinder(self, mock_show):"},{"line_number":4621,"context_line":"        self.req.body \u003d jsonutils.dump_as_bytes(self.body)"},{"line_number":4622,"context_line":""},{"line_number":4623,"context_line":"        expected_msg \u003d (\"Image {} is unacceptable: Direct booting of an image \""}],"source_content_type":"text/x-python","patch_set":3,"id":"0268ee9d_c23624b2","line":4620,"updated":"2025-08-14 14:31:11.000000000","message":"A comment here about testing the legacy/deprecated key property name would help distinguish it from the above.\n\nThis is not your fault, but passing a full image in a ` @mock.patch` is not super great, IMHO, and it\u0027s easy to look at these two tests and not spot the difference (only in the mock). Could also combine these into one test that tries the new and old properties to make it super explicit, especially once the error message is unified.","commit_id":"dfb7adb1b6a504e44436dccb2d623d7df3a5fa6f"},{"author":{"_account_id":27665,"name":"Markus Hentsch","email":"markus.hentsch@cloudandheat.com","username":"mhen"},"change_message_id":"b763739a6fe3817ca64fe5ae0da25e57d6e5dbab","unresolved":false,"context_lines":[{"line_number":4617,"context_line":"                           status\u003d\u0027active\u0027,"},{"line_number":4618,"context_line":"                           properties\u003ddict("},{"line_number":4619,"context_line":"                               cinder_encryption_key_id\u003dfakes.FAKE_UUID)))"},{"line_number":4620,"context_line":"    def test_create_server_image_nonbootable_cinder(self, mock_show):"},{"line_number":4621,"context_line":"        self.req.body \u003d jsonutils.dump_as_bytes(self.body)"},{"line_number":4622,"context_line":""},{"line_number":4623,"context_line":"        expected_msg \u003d (\"Image {} is unacceptable: Direct booting of an image \""}],"source_content_type":"text/x-python","patch_set":3,"id":"f30adf14_80a4228d","line":4620,"in_reply_to":"0268ee9d_c23624b2","updated":"2025-08-15 08:53:42.000000000","message":"I have now unified this into one method using a `ddt.data` decorator for just the property name differentiation and moved the image `mock.patch` into the test method itself. Also added the deprecation comment like in the other places.","commit_id":"dfb7adb1b6a504e44436dccb2d623d7df3a5fa6f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"360ca247540f88f9c450aef406693d1dfae70f26","unresolved":false,"context_lines":[{"line_number":4617,"context_line":"                           status\u003d\u0027active\u0027,"},{"line_number":4618,"context_line":"                           properties\u003ddict("},{"line_number":4619,"context_line":"                               cinder_encryption_key_id\u003dfakes.FAKE_UUID)))"},{"line_number":4620,"context_line":"    def test_create_server_image_nonbootable_cinder(self, mock_show):"},{"line_number":4621,"context_line":"        self.req.body \u003d jsonutils.dump_as_bytes(self.body)"},{"line_number":4622,"context_line":""},{"line_number":4623,"context_line":"        expected_msg \u003d (\"Image {} is unacceptable: Direct booting of an image \""}],"source_content_type":"text/x-python","patch_set":3,"id":"54de82ce_7991eda4","line":4620,"in_reply_to":"f30adf14_80a4228d","updated":"2025-08-15 13:40:51.000000000","message":"Perfect, thanks.","commit_id":"dfb7adb1b6a504e44436dccb2d623d7df3a5fa6f"}],"nova/virt/libvirt/driver.py":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dd4cf04ad2902be7e3eeb020cee99b2e6e79aad1","unresolved":false,"context_lines":[{"line_number":2248,"context_line":"            # below."},{"line_number":2249,"context_line":"            keymgr \u003d key_manager.API(CONF)"},{"line_number":2250,"context_line":"            key \u003d keymgr.get(context, encryption[\u0027encryption_key_id\u0027])"},{"line_number":2251,"context_line":"            passphrase \u003d brick_utils.get_passphrase_from_secret(key)"},{"line_number":2252,"context_line":""},{"line_number":2253,"context_line":"            # NOTE(lyarwood): Retain the behaviour of the original os-brick"},{"line_number":2254,"context_line":"            # encryptors and format any volume that does not identify as"}],"source_content_type":"text/x-python","patch_set":3,"id":"4c24536f_6a457dd9","line":2251,"updated":"2025-08-14 14:31:11.000000000","message":"Confirm this does exactly what we did before, with the extra behavior of not doing hexlify if it\u0027s already stored as a passphrase:\n\nhttps://github.com/openstack/os-brick/blob/c9b91057e7b7674730960b2ab7df259c61529281/os_brick/utils.py#L460-L492\n\nThat was merged in Aug 2024, several versions ago, so should be fine, dep-wise.","commit_id":"dfb7adb1b6a504e44436dccb2d623d7df3a5fa6f"}]}