)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"eca16f173f9398faac3237338f13f0405ba4cc1f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"cd6bc97d_1723e8bd","updated":"2022-02-21 18:49:10.000000000","message":"This needs tests","commit_id":"b609c03b966c0b885dd10fb36ddd025f24f9b219"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"e8c1a5bf5f0960b9fe4bff905fe442b8e5b5ada2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"301eeec2_efe5a733","updated":"2022-06-30 21:46:29.000000000","message":"recheck bug 1979047","commit_id":"646e85d2a997cfec12c6fe9187b4961c8f8267cb"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a9f68915e386267ee6aed43a986a9449089dcf67","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"14f73049_2479ff0e","updated":"2022-08-19 02:38:22.000000000","message":"Setting this to -W while I look into the encryption_options.","commit_id":"53101d127e2e60cdeadacf6166d0f84232cf4739"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"aa6e68c573799ea5e3c4bd365ea16d7419824abf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"cc6528d1_2502be31","updated":"2022-08-26 12:26:34.000000000","message":"droping +2 we talked about this a little offline.\n\nwe have 4 options\n\n1 parse and convert the cryptesetup  format value for the lvm config option.\n2 add new config options for the new feature\n3 hard code a sane default and revisit this next cycle (this is my vote)\n4 expose these via flavor extra specs/image propertiees in this cycle\n\n4 is possibel the best long term solution but i am suggesting 3\n\nbecause all of the other options technially need schduler support\nwhich idealy would be traits based to make move ops work\n\nif we only support one option for the encyption parmaters (the current default used by qemu) then we know it works on all hosts and we cna defer makign a choice about the way we express the encyption requirement and how to handel the schduler impact to next cycle.","commit_id":"53101d127e2e60cdeadacf6166d0f84232cf4739"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"841c32bd9cf14cf65fb60eab71e22983982235a4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"6deb027a_0e7b4050","in_reply_to":"cc6528d1_2502be31","updated":"2022-08-26 18:28:11.000000000","message":"\u003e droping +2 we talked about this a little offline.\n\u003e \n\u003e we have 4 options\n\u003e \n\u003e 1 parse and convert the cryptesetup  format value for the lvm config option.\n\u003e 2 add new config options for the new feature\n\u003e 3 hard code a sane default and revisit this next cycle (this is my vote)\n\u003e 4 expose these via flavor extra specs/image propertiees in this cycle\n\nWith regard to option 4, from what I can tell in the spec + code, it was not intended for the encryption_options to be chosen by the end user via extra_specs/image_properties. The only configurability I see is by the operator/deployer only in nova.conf.\n\nThat said, we could obviously consider providing the options to end users but I just wanted to mention I don\u0027t think that was the original design.\n\n\u003e 4 is possibel the best long term solution but i am suggesting 3\n\u003e \n\u003e because all of the other options technially need schduler support\n\u003e which idealy would be traits based to make move ops work\n\u003e \n\u003e if we only support one option for the encyption parmaters (the current default used by qemu) then we know it works on all hosts and we cna defer makign a choice about the way we express the encyption requirement and how to handel the schduler impact to next cycle.\n\nI am fine with option 3. The existing LVM config default is \"aes-xts-plain64\" and the defaults in qemu-img are cipher-alg \u003d aes256, cipher-mode \u003d xts, ivgen-alg \u003d plain64. These are consistent between LVM and QEMU, reinforcing the idea that they would be good defaults to hard code as a start.\n\nFWIW this is what I ended up with locally for parsing the existing LVM config option:\n\n    # NOTE(melwitt): CONF.ephemeral_storage_encryption.cipher\n    # has format:\n    #   \u003ccipher\u003e-\u003cchainmode\u003e-\u003civgen\u003e\n    #            \n    # CONF.ephemeral_storage_encryption.key_size contains the key\n    # size       \n    #            \n    # Equivalent options for qemu-img are separate and map like:\n    #   cipher-alg\u003d\u003ccipher\u003e-\u003ckeysize\u003e\n    #   cipher-mode\u003d\u003cchainmode\u003e\n    #   ivgen-alg\u003d\u003civgen\u003e\n    cipher, chainmode, ivgen \u003d (\n        CONF.ephemeral_storage_encryption.cipher.split(\u0027-\u0027)\n    )            \n    key_size \u003d CONF.ephemeral_storage_encryption.key_size\n    # From the CONF.ephemeral_storage_encryption.key_size help:\n    #   \"In XTS mode only half of the bits are used for encryption\n    #   key.\"    \n    if chainmode \u003d\u003d \u0027xts\u0027:\n        key_size \u003d int(key_size / 2)\n    cipher_alg \u003d f\u0027{cipher}-{key_size}\u0027\n    options \u003d {\n        \u0027cipher-alg\u0027: cipher_alg,\n        \u0027cipher-mode\u0027: chainmode,\n        \u0027ivgen-alg\u0027: ivgen,\n    }","commit_id":"53101d127e2e60cdeadacf6166d0f84232cf4739"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"36061d43bde1a47c660e722be27127a8dd34ee24","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"66bf586e_02530776","updated":"2023-01-13 21:54:58.000000000","message":"recheck old results","commit_id":"2eeefabde4f5f46e95849efe910027455ba70186"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"3b1c4605b30992561c8d87c675e38d31972bdf1d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"147d291b_096eb3e9","updated":"2022-10-18 14:07:53.000000000","message":"recheck unrelated grenade failure","commit_id":"2eeefabde4f5f46e95849efe910027455ba70186"}],"nova/privsep/qemu.py":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"eca16f173f9398faac3237338f13f0405ba4cc1f","unresolved":true,"context_lines":[{"line_number":109,"context_line":"                for option, value in encryption_options.items():"},{"line_number":110,"context_line":"                    if option in supported_encryption_options_luks:"},{"line_number":111,"context_line":"                        encryption_opts +\u003d ["},{"line_number":112,"context_line":"                            f\u0027-o encrypt.{option}\u003d{value}\u0027"},{"line_number":113,"context_line":"                        ]"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"            cmd +\u003d encryption_opts + [path, disk_size]"}],"source_content_type":"text/x-python","patch_set":2,"id":"f0527aee_9c69cadd","line":112,"range":{"start_line":112,"start_character":27,"end_line":112,"end_character":58},"updated":"2022-02-21 18:49:10.000000000","message":"Why are these not separate items? Bug? Test needed?","commit_id":"b609c03b966c0b885dd10fb36ddd025f24f9b219"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"03697c3c31d828ccaf003978a151f7c063da5238","unresolved":false,"context_lines":[{"line_number":109,"context_line":"                for option, value in encryption_options.items():"},{"line_number":110,"context_line":"                    if option in supported_encryption_options_luks:"},{"line_number":111,"context_line":"                        encryption_opts +\u003d ["},{"line_number":112,"context_line":"                            f\u0027-o encrypt.{option}\u003d{value}\u0027"},{"line_number":113,"context_line":"                        ]"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"            cmd +\u003d encryption_opts + [path, disk_size]"}],"source_content_type":"text/x-python","patch_set":2,"id":"7d4c6b75_bed8bbd7","line":112,"range":{"start_line":112,"start_character":27,"end_line":112,"end_character":58},"in_reply_to":"2d212b98_3b556ca9","updated":"2022-08-09 16:07:34.000000000","message":"Ack","commit_id":"b609c03b966c0b885dd10fb36ddd025f24f9b219"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"d923c7d8d33d0086f45d14cf53e4c7080988b001","unresolved":true,"context_lines":[{"line_number":109,"context_line":"                for option, value in encryption_options.items():"},{"line_number":110,"context_line":"                    if option in supported_encryption_options_luks:"},{"line_number":111,"context_line":"                        encryption_opts +\u003d ["},{"line_number":112,"context_line":"                            f\u0027-o encrypt.{option}\u003d{value}\u0027"},{"line_number":113,"context_line":"                        ]"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"            cmd +\u003d encryption_opts + [path, disk_size]"}],"source_content_type":"text/x-python","patch_set":2,"id":"2d212b98_3b556ca9","line":112,"range":{"start_line":112,"start_character":27,"end_line":112,"end_character":58},"in_reply_to":"f0527aee_9c69cadd","updated":"2022-04-05 05:38:12.000000000","message":"I had thought it doesn\u0027t matter but apparently technically passing args as a single string (though this is only part of one) will result in platform-dependent behavior [1]. I have changed it to separate items.\n\n[1] https://docs.python.org/3/library/subprocess.html#subprocess.Popen","commit_id":"b609c03b966c0b885dd10fb36ddd025f24f9b219"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bd6f466bc1bb965b43b3e48209cb778837c21ec5","unresolved":true,"context_lines":[{"line_number":41,"context_line":"    disk_format: str,"},{"line_number":42,"context_line":"    disk_size: ty.Union[str, int],"},{"line_number":43,"context_line":"    backing_file: ty.Optional[str] \u003d None,"},{"line_number":44,"context_line":"    encryption: ty.Optional[ty.Dict[str, ty.Any]] \u003d None"},{"line_number":45,"context_line":") -\u003e None:"},{"line_number":46,"context_line":"    \"\"\"Unprivileged image creation with qemu-img"},{"line_number":47,"context_line":"    :param path: Desired location of the disk image"}],"source_content_type":"text/x-python","patch_set":3,"id":"4b948454_b3fd0516","line":44,"updated":"2022-08-10 09:32:34.000000000","message":"just to be very clear passing dict of args that will be expanded in a privledge context is a secuity anti patern which we should be avoiding.","commit_id":"82ed895aa73b3ca77a3c23b0d7581dd9e17dd90f"}],"nova/virt/libvirt/utils.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"bd6f466bc1bb965b43b3e48209cb778837c21ec5","unresolved":true,"context_lines":[{"line_number":170,"context_line":"                \u0027-o\u0027, \u0027encrypt.key-secret\u003dsec\u0027,"},{"line_number":171,"context_line":"                \u0027-o\u0027, f\"encrypt.format\u003d{encryption.get(\u0027format\u0027)}\","},{"line_number":172,"context_line":"            ]"},{"line_number":173,"context_line":"            # Supported luks options:"},{"line_number":174,"context_line":"            #  cipher-alg\u003d\u003cstr\u003e       - Name of encryption cipher algorithm"},{"line_number":175,"context_line":"            #  cipher-mode\u003d\u003cstr\u003e      - Name of encryption cipher mode"},{"line_number":176,"context_line":"            #  hash-alg\u003d\u003cstr\u003e         - Name of encryption hash algorithm"},{"line_number":177,"context_line":"            #  iter-time\u003d\u003cnum\u003e        - Time to spend in PBKDF in milliseconds"},{"line_number":178,"context_line":"            #  ivgen-alg\u003d\u003cstr\u003e        - Name of IV generator algorithm"},{"line_number":179,"context_line":"            #  ivgen-hash-alg\u003d\u003cstr\u003e   - Name of IV generator hash algorithm"},{"line_number":180,"context_line":"            # FIXME(lyarwood): Validate this elsewhere?"},{"line_number":181,"context_line":"            supported_encryption_options_luks \u003d {"},{"line_number":182,"context_line":"                \u0027cipher-alg\u0027, \u0027cipher-mode\u0027, \u0027hash-alg\u0027, \u0027iter-time\u0027,"},{"line_number":183,"context_line":"                \u0027ivgen-alg\u0027, \u0027ivgen-hash-alg\u0027,"},{"line_number":184,"context_line":"            }"},{"line_number":185,"context_line":"            encryption_options \u003d encryption.get(\u0027options\u0027)"},{"line_number":186,"context_line":"            if encryption_options:"},{"line_number":187,"context_line":"                for option, value in encryption_options.items():"}],"source_content_type":"text/x-python","patch_set":8,"id":"babd85a7_96266bfe","line":184,"range":{"start_line":173,"start_character":10,"end_line":184,"end_character":13},"updated":"2022-08-10 09:32:34.000000000","message":"by the way i still dont think we should be exposing these options to endusers or admins.\n\nthat too low leve in my view.","commit_id":"959a3ab109b44c8d33d7daa750b39ec5d485acb9"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3865db058c2726a93e5d583ff1f16e08155492dd","unresolved":false,"context_lines":[{"line_number":170,"context_line":"                \u0027-o\u0027, \u0027encrypt.key-secret\u003dsec\u0027,"},{"line_number":171,"context_line":"                \u0027-o\u0027, f\"encrypt.format\u003d{encryption.get(\u0027format\u0027)}\","},{"line_number":172,"context_line":"            ]"},{"line_number":173,"context_line":"            # Supported luks options:"},{"line_number":174,"context_line":"            #  cipher-alg\u003d\u003cstr\u003e       - Name of encryption cipher algorithm"},{"line_number":175,"context_line":"            #  cipher-mode\u003d\u003cstr\u003e      - Name of encryption cipher mode"},{"line_number":176,"context_line":"            #  hash-alg\u003d\u003cstr\u003e         - Name of encryption hash algorithm"},{"line_number":177,"context_line":"            #  iter-time\u003d\u003cnum\u003e        - Time to spend in PBKDF in milliseconds"},{"line_number":178,"context_line":"            #  ivgen-alg\u003d\u003cstr\u003e        - Name of IV generator algorithm"},{"line_number":179,"context_line":"            #  ivgen-hash-alg\u003d\u003cstr\u003e   - Name of IV generator hash algorithm"},{"line_number":180,"context_line":"            # FIXME(lyarwood): Validate this elsewhere?"},{"line_number":181,"context_line":"            supported_encryption_options_luks \u003d {"},{"line_number":182,"context_line":"                \u0027cipher-alg\u0027, \u0027cipher-mode\u0027, \u0027hash-alg\u0027, \u0027iter-time\u0027,"},{"line_number":183,"context_line":"                \u0027ivgen-alg\u0027, \u0027ivgen-hash-alg\u0027,"},{"line_number":184,"context_line":"            }"},{"line_number":185,"context_line":"            encryption_options \u003d encryption.get(\u0027options\u0027)"},{"line_number":186,"context_line":"            if encryption_options:"},{"line_number":187,"context_line":"                for option, value in encryption_options.items():"}],"source_content_type":"text/x-python","patch_set":8,"id":"f86fb850_f83c87b3","line":184,"range":{"start_line":173,"start_character":10,"end_line":184,"end_character":13},"in_reply_to":"4d4896f0_0e1df66c","updated":"2022-10-10 22:40:07.000000000","message":"Done","commit_id":"959a3ab109b44c8d33d7daa750b39ec5d485acb9"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"841c32bd9cf14cf65fb60eab71e22983982235a4","unresolved":true,"context_lines":[{"line_number":170,"context_line":"                \u0027-o\u0027, \u0027encrypt.key-secret\u003dsec\u0027,"},{"line_number":171,"context_line":"                \u0027-o\u0027, f\"encrypt.format\u003d{encryption.get(\u0027format\u0027)}\","},{"line_number":172,"context_line":"            ]"},{"line_number":173,"context_line":"            # Supported luks options:"},{"line_number":174,"context_line":"            #  cipher-alg\u003d\u003cstr\u003e       - Name of encryption cipher algorithm"},{"line_number":175,"context_line":"            #  cipher-mode\u003d\u003cstr\u003e      - Name of encryption cipher mode"},{"line_number":176,"context_line":"            #  hash-alg\u003d\u003cstr\u003e         - Name of encryption hash algorithm"},{"line_number":177,"context_line":"            #  iter-time\u003d\u003cnum\u003e        - Time to spend in PBKDF in milliseconds"},{"line_number":178,"context_line":"            #  ivgen-alg\u003d\u003cstr\u003e        - Name of IV generator algorithm"},{"line_number":179,"context_line":"            #  ivgen-hash-alg\u003d\u003cstr\u003e   - Name of IV generator hash algorithm"},{"line_number":180,"context_line":"            # FIXME(lyarwood): Validate this elsewhere?"},{"line_number":181,"context_line":"            supported_encryption_options_luks \u003d {"},{"line_number":182,"context_line":"                \u0027cipher-alg\u0027, \u0027cipher-mode\u0027, \u0027hash-alg\u0027, \u0027iter-time\u0027,"},{"line_number":183,"context_line":"                \u0027ivgen-alg\u0027, \u0027ivgen-hash-alg\u0027,"},{"line_number":184,"context_line":"            }"},{"line_number":185,"context_line":"            encryption_options \u003d encryption.get(\u0027options\u0027)"},{"line_number":186,"context_line":"            if encryption_options:"},{"line_number":187,"context_line":"                for option, value in encryption_options.items():"}],"source_content_type":"text/x-python","patch_set":8,"id":"4d4896f0_0e1df66c","line":184,"range":{"start_line":173,"start_character":10,"end_line":184,"end_character":13},"in_reply_to":"babd85a7_96266bfe","updated":"2022-08-26 18:28:11.000000000","message":"(wrote this earlier)\n\nIt\u0027s at least not exposed to end users, only admins/deployers via nova.conf. For awhile I kept thinking the options were also an extra spec but they are not.","commit_id":"959a3ab109b44c8d33d7daa750b39ec5d485acb9"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"a9f68915e386267ee6aed43a986a9449089dcf67","unresolved":true,"context_lines":[{"line_number":189,"context_line":"                        encryption_opts +\u003d ["},{"line_number":190,"context_line":"                            \u0027-o\u0027,"},{"line_number":191,"context_line":"                            f\u0027encrypt.{option}\u003d{value}\u0027,"},{"line_number":192,"context_line":"                        ]"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"            # We need to execute the command while the NamedTemporaryFile still"},{"line_number":195,"context_line":"            # exists"}],"source_content_type":"text/x-python","patch_set":9,"id":"96962921_148b4e39","line":192,"updated":"2022-08-19 02:38:22.000000000","message":"I just realized it looks like there is more to do here.\n\nIn the encryption dict, it contains a \u0027cipher\u0027 key of the format \"\u003ccipher\u003e-\u003cchainmode\u003e-\u003civmode\u003e\" [1], so the separate pieces need to be parsed to pass to qemu-img [2].\n\n encryption_options \u003d {\u0027cipher\u0027: \u0027aes-xts-plain64\u0027}\n\nWhat\u0027s here currently will always give us only the default because \u0027cipher\u0027 is not in supported_encryption_options_luks.\n\nBased on [2], I *think* the conf option corresponds to the qemu-img options like this \"\u003ccipher-alg\u003e-\u003ccipher-mode\u003e-\u003civgen-alg\u003e\".\n\n[1] https://github.com/openstack/nova/blob/378d178cee45529dbb82d49fca877f1cc00bbf35/nova/conf/ephemeral_storage.py#L29\n[2] https://qemu-project.gitlab.io/qemu/system/images.html#cmdoption-image-formats-arg-qcow2","commit_id":"53101d127e2e60cdeadacf6166d0f84232cf4739"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"3865db058c2726a93e5d583ff1f16e08155492dd","unresolved":false,"context_lines":[{"line_number":189,"context_line":"                        encryption_opts +\u003d ["},{"line_number":190,"context_line":"                            \u0027-o\u0027,"},{"line_number":191,"context_line":"                            f\u0027encrypt.{option}\u003d{value}\u0027,"},{"line_number":192,"context_line":"                        ]"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"            # We need to execute the command while the NamedTemporaryFile still"},{"line_number":195,"context_line":"            # exists"}],"source_content_type":"text/x-python","patch_set":9,"id":"ee3d9a7a_21fc19ca","line":192,"in_reply_to":"2761579d_57b105ad","updated":"2022-10-10 22:40:07.000000000","message":"Done","commit_id":"53101d127e2e60cdeadacf6166d0f84232cf4739"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"841c32bd9cf14cf65fb60eab71e22983982235a4","unresolved":true,"context_lines":[{"line_number":189,"context_line":"                        encryption_opts +\u003d ["},{"line_number":190,"context_line":"                            \u0027-o\u0027,"},{"line_number":191,"context_line":"                            f\u0027encrypt.{option}\u003d{value}\u0027,"},{"line_number":192,"context_line":"                        ]"},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"            # We need to execute the command while the NamedTemporaryFile still"},{"line_number":195,"context_line":"            # exists"}],"source_content_type":"text/x-python","patch_set":9,"id":"2761579d_57b105ad","line":192,"in_reply_to":"96962921_148b4e39","updated":"2022-08-26 18:28:11.000000000","message":"(wrote this earlier)\n\nAfter thinking about this more ...\n\nThe CONF.ephemeral_storage.cipher option is the format cryptsetup uses which isn\u0027t the same as the encryption options being called out here.\n\nNot sure it would be a great idea to try to parse the cryptsetup expected format and translate it to the format qemu-img needs. Seems error prone.","commit_id":"53101d127e2e60cdeadacf6166d0f84232cf4739"}]}