)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8cb4966c1442beb3aad9abb6a38212e21248b932","unresolved":true,"context_lines":[{"line_number":30,"context_line":"The default path (no encryption_engine in connection_info) is unchanged:"},{"line_number":31,"context_line":"\u003cencryption\u003e is placed under \u003cdisk\u003e and QEMU handles decryption."},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"Depends-On: \u003ccinder rbd-encryption-engine-extra-spec review\u003e"},{"line_number":34,"context_line":"Change-Id: I09562c3f676112c56ce4309fb12cba19b8ff7747"},{"line_number":35,"context_line":"Signed-off-by: ravi.tilak \u003cravi.tilak@juspay.in\u003e"},{"line_number":36,"context_line":"Signed-off-by: sasitilak \u003crsasitilak0987@gmail.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"08759683_e7f4b12a","line":33,"updated":"2026-05-06 19:29:18.000000000","message":"Since you have uploaded it, I think you can go ahead and link https://review.opendev.org/c/openstack/cinder/+/986322 here.","commit_id":"72246a46b570d1ab59dbb00e9abbb4ac48893d36"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8cb4966c1442beb3aad9abb6a38212e21248b932","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"88958774_bf4f0ca9","updated":"2026-05-06 19:29:18.000000000","message":"This looks right to me based on my experience with libvirt librbd engine when I worked on local disk encryption and I see the value in adding this support.\n\nBe aware that this change would need at least a specless blueprint for Nova [1] for approval before we would merge it. And also obviously the Cinder spec/blueprint + implementation patch would be required to merge first.\n\nThe second thing is we will want to see before merging is Tempest test(s) working with this change -- I assume there are already existing RBD volume encryption tests somewhere, so what we would want is for you to add a variant which sets the Cinder volume extra spec appropriately and we get to the end-to-end working in CI. You can achieve this using Depends-On across projects.\n\n[1] https://docs.openstack.org/nova/latest/contributor/blueprints.html","commit_id":"72246a46b570d1ab59dbb00e9abbb4ac48893d36"}],"nova/virt/libvirt/config.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8cb4966c1442beb3aad9abb6a38212e21248b932","unresolved":true,"context_lines":[{"line_number":1395,"context_line":"                # instead of QEMU\u0027s own LUKS block driver."},{"line_number":1396,"context_line":"                source.append(self.volume_encryption.format_dom())"},{"line_number":1397,"context_line":"            else:"},{"line_number":1398,"context_line":"                dev.append(self.volume_encryption.format_dom())"},{"line_number":1399,"context_line":""},{"line_number":1400,"context_line":"        return dev"},{"line_number":1401,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"b530d2f5_9bf538ce","line":1398,"updated":"2026-05-06 19:29:18.000000000","message":"To be clear, I\u0027m not asking you to change this but to just share my thoughts. Although I guess since this patch is about RBD only, changing this across the board could be considered an unrelated change or unrelated bug fix. So take my comment with a grain of salt. Perhaps it would be better as a unrelated follow up or an unrelated separate change ahead of this one.\n\nI doubt you need to if-else engine this ... as far as I know, `\u003cencryption\u003e` was always supposed to be a child element of `\u003csource\u003e` as seen in the partial ephemeral encryption work that happened [1]:\n\n```\n        if self.ephemeral_encryption:\n            # NOTE(melwitt): \u003cencryption\u003e should be a sub element of \u003csource\u003e\n            # in order to ensure the image uses encryption.\n            # See the following for more details:\n            #   https://libvirt.org/formatdomain.html#hard-drives-floppy-disks-cdroms\n            #   https://bugzilla.redhat.com/show_bug.cgi?id\u003d1371022#c13\n            source.append(self.ephemeral_encryption.format_dom())\n```\n\nFWIW when I tested qcow2, raw, and rbd disks in Tempest [2] the placement of \u003cencryption\u003e under \u003csource\u003e worked in every case.\n\nIf you read those reference links in the code comments, I think it\u0027s all saying that. It would be great if you could read over it and let me know if your interpretation is the same. And if so, might be useful to cite those links under your code comment here too for future reference.\n\nAll that said, I could also see the desire to be maximally paranoid here, in which case, leave the if-else as-is.\n\nMy minor concern with the if-else would be that it might be confusing for those who will read this code in the future to think librbd is unique in this when it probably isn\u0027t. Although, I have not really understood how this works with \u003cencryption\u003e NOT under \u003csource\u003e. Not very helpful of libvirt to allow both in only certain cases 😛\n\n[1] https://github.com/openstack/nova/blob/ff4ea2dea9fa5d14662d11f7399be12ca0d3e5ce/nova/virt/libvirt/config.py#L1334-L1340\n[2] https://review.opendev.org/c/openstack/nova/+/862416","commit_id":"72246a46b570d1ab59dbb00e9abbb4ac48893d36"}],"nova/virt/libvirt/volume/net.py":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8cb4966c1442beb3aad9abb6a38212e21248b932","unresolved":true,"context_lines":[{"line_number":98,"context_line":"                conf.volume_encryption is not None and"},{"line_number":99,"context_line":"                engine \u003d\u003d \u0027librbd\u0027):"},{"line_number":100,"context_line":"            conf.volume_encryption.engine \u003d \u0027librbd\u0027"},{"line_number":101,"context_line":"        return conf"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    def extend_volume(self, connection_info, instance, requested_size):"},{"line_number":104,"context_line":"        # There is nothing to do for network volumes. Cinder already"}],"source_content_type":"text/x-python","patch_set":1,"id":"b9c0adfd_937d9141","line":101,"updated":"2026-05-06 19:29:18.000000000","message":"I think this should go in _get_net_config() above, under where there is already a check on L85. Just add another `if` under there after auth config is set.\n\nOther than that, I think this looks correct as conf.volume_encryption is populated in the super() call before we get here.","commit_id":"72246a46b570d1ab59dbb00e9abbb4ac48893d36"}]}