)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"ad61c907c606ce9e7c4d42ee9f84c66c9f5d4cc8","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add allow_image_conversion config"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add new config option `allow_image_conversion`, when it\u0027s setted off,"},{"line_number":10,"context_line":"image disk_format and volume format must be the same format."},{"line_number":11,"context_line":"Otherwise will raise ImageUnacceptable exception."},{"line_number":12,"context_line":"`allow_image_conversion` config is bool option, default to `True`."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9204acdf_8b376144","line":9,"range":{"start_line":9,"start_character":58,"end_line":9,"end_character":64},"updated":"2022-04-29 14:48:06.000000000","message":"set to","commit_id":"44e4484b42e81d8094e825e784b31d276a392b83"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d092bbf743d3e3d0d6b6e8dad202a37c6ca3143d","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add allow_image_conversion config"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Add new config option `allow_image_conversion`, when it\u0027s setted off,"},{"line_number":10,"context_line":"image disk_format and volume format must be the same format."},{"line_number":11,"context_line":"Otherwise will raise ImageUnacceptable exception."},{"line_number":12,"context_line":"`allow_image_conversion` config is bool option, default to `True`."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"1c955dad_95304d78","line":9,"range":{"start_line":9,"start_character":58,"end_line":9,"end_character":64},"in_reply_to":"9204acdf_8b376144","updated":"2022-05-12 18:35:23.000000000","message":"Done","commit_id":"44e4484b42e81d8094e825e784b31d276a392b83"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"a7a1ede54fbf1fb5ff14f8f053e5e1a252afb26b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"8de469b6_c1886ca7","line":16,"updated":"2022-05-03 18:09:14.000000000","message":"This commit message explains the implementation, which we already have\nbecause it\u0027s included in the commit.  More useful here is the reasoning,\nwhich is nicely laid out here:\nhttps://bugs.launchpad.net/cinder/+bug/1970115/comments/5\n\nI think it\u0027s more useful to have that in the commit rather than linked\nto.","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"5ae061ba0e9cfbb9bf1d8d5908285c9d2e94f979","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"9ad323e3_c6069782","line":16,"in_reply_to":"8de469b6_c1886ca7","updated":"2022-05-04 04:21:32.000000000","message":"Done","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1004,"name":"Mohammed Naser","email":"mnaser@vexxhost.com","username":"mnaser"},"change_message_id":"cb2d13e5f736d400e8ac98d89d5214fb655c43bf","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f8210c47_1e831961","updated":"2022-04-28 17:42:44.000000000","message":"Far less intrusive and gets the job done, small nits to optimize the code and testing, otherwise great. 😄","commit_id":"193dca2dd3ff9a9026ed298d674578e893bfccd6"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"ad61c907c606ce9e7c4d42ee9f84c66c9f5d4cc8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"0a853a7c_637e3883","updated":"2022-04-29 14:48:06.000000000","message":"There are several things in this patch that need to be fixed please.","commit_id":"44e4484b42e81d8094e825e784b31d276a392b83"},{"author":{"_account_id":1004,"name":"Mohammed Naser","email":"mnaser@vexxhost.com","username":"mnaser"},"change_message_id":"754a6029762ec1d5c3d46bd3e5e164fa8b2dd901","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"67392b62_cc051e0b","updated":"2022-04-29 14:00:23.000000000","message":"e","commit_id":"44e4484b42e81d8094e825e784b31d276a392b83"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"33d9a090d0c5cf4cb6e3e63a9aeec7a2a83dfacd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"edd3ee6c_2f50698e","updated":"2022-05-03 18:34:42.000000000","message":"A few comments inline.","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"a7a1ede54fbf1fb5ff14f8f053e5e1a252afb26b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"b1fa3111_1d9ac961","updated":"2022-05-03 18:09:14.000000000","message":"A weak -1, I think it needs some cleanup but overall looks okay to me.\n","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"388814a4363b4897e8fb0f26432e11d72b31cfa8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"2c42ebf9_93716dea","updated":"2022-05-03 15:04:26.000000000","message":"The fail job cinder-plugin-ceph-tempest keep complaint about `No valid host was found. There are not enough hosts available.` and fail to create instance.","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c715a2137d1910f189fcc2d16d375378b1577d1d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"95da34d7_76381e18","updated":"2022-05-03 16:19:47.000000000","message":"green at least:)","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":1004,"name":"Mohammed Naser","email":"mnaser@vexxhost.com","username":"mnaser"},"change_message_id":"168bdcc3e290b019e4c648818e0d1ac2a0decf38","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"cdf89880_64a087fb","updated":"2022-04-29 18:30:02.000000000","message":"lgtm 😊","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":1004,"name":"Mohammed Naser","email":"mnaser@vexxhost.com","username":"mnaser"},"change_message_id":"3b706b90b44cc556768e0dc63bb52829e3516b74","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"bef7347e_3d072cc8","updated":"2022-05-03 13:24:13.000000000","message":"recheck","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":1004,"name":"Mohammed Naser","email":"mnaser@vexxhost.com","username":"mnaser"},"change_message_id":"4190fd847c95104937014db479e6c3208ca217c7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"e82d0043_55524193","updated":"2022-05-02 04:17:51.000000000","message":"recheck\n\nfailure unrelated to patch.","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"4d6859f62a0b90f7e691de1434511b278fffe986","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"4b771fa0_3bc845d5","in_reply_to":"2c42ebf9_93716dea","updated":"2022-05-03 15:15:55.000000000","message":"And Nova API socket timeout","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"48aa9723628d917ab2a2d881ba17b2ddaec62153","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"724d0d2d_51b276ed","updated":"2022-05-04 15:06:25.000000000","message":"A few issues noted inline.  I will put up a patch removing the mock CONF from the current tests, it should not be there and is getting in your way.","commit_id":"883d85305ea14f2be9626f22596f61bea0771cdf"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"f4451e375e32c277148f7d67f3a4317e92324bed","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"f771a666_84af15be","updated":"2022-05-04 16:32:42.000000000","message":"Patch removing the mock CONF object is https://review.opendev.org/c/openstack/cinder/+/840512","commit_id":"883d85305ea14f2be9626f22596f61bea0771cdf"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"07d8acd94f0386585e168e0b7d624fb52d206dda","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"1b3b8405_ec594355","updated":"2022-05-04 13:01:12.000000000","message":"recheck openstack-tox-py39 - fixtures timeout in cinder.tests.unit.backup.drivers.test_backup_swift.WindowsBackupSwiftTestCase.test_backup_zstd, cannot reproduce locally","commit_id":"883d85305ea14f2be9626f22596f61bea0771cdf"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"243878b6ea20547eff0a7590b79a34f08bea5a22","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"9084ce13_14dbdaba","updated":"2022-05-04 20:47:49.000000000","message":"recheck cinder-plugin-ceph-tempest - bunch of weird errors: 500s from keystone, some glance stuff, volumes failed to build","commit_id":"f52e124d8728f8b096aba2985731d6a22413fb81"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"f02cc5b232817ec519156155b377ecb73867f25d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"95cf84e3_e353f759","updated":"2022-05-11 20:11:56.000000000","message":"Brian has some additional changes he would like to make so I am downgrading my +2 for now.\n","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"3de2e7333237515c1b199515c0ae069416b5426e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"1c659e72_d6b8d09d","updated":"2022-05-05 03:25:09.000000000","message":"I had a bunch of comments and test requests, and it was easier to push a new patch set than to write out all the comments.  Feel free to change anything you don\u0027t like.","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"97af5a81c99b01279c3173673d9898b80f495102","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"b50af605_82602717","updated":"2022-05-11 20:11:57.000000000","message":"I think this needs some more work.  Here\u0027s what I\u0027m worried about.\n\nThis change is involved in 3 workflows:\n\n1. Upload volume to image - POST /v3/{project_id}/volumes/{volume_id}/action with the \u0027os-volume_upload_image\u0027 action\n- this has complete test coverage, so I\u0027m not worried about it\n\n2. Create a volume - POST /v3/{project_id}/volumes with an \u0027imageRef\u0027 attribute in the request body\n- the code that raises ImageUnacceptable isn\u0027t called until taskflow is running, so what will happen (I think) is that the user gets a 202, but the volume ultimately goes to \u0027error\u0027 status\n- I don\u0027t think the user gets any feedback about what went wrong?\n- there are some user messages created in cinder/volume/flows/api/create_volume.py, but not one that would handle ImageUnacceptable\n\n3. Reimage a volume - POST /v3/{project_id}/volumes/{volume_id}/action with the \u0027os-reimage\u0027 action\n- the check doesn\u0027t happen until *after* the rpcapi call is made; user gets a 202 but volume goes to \u0027error\u0027 status\n- I think you should add a user message for this case (code is in cinder/volume/manager.py, which creates user messages for other situations)\n","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"a37c89f89f27c4a936581af34be93ce6fc1da9b4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"e50769a0_e78a5bfb","updated":"2022-05-11 18:28:52.000000000","message":"It appears that my concerns have been addressed.","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6ba9073721560bebd166a4ce463add20c5918280","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"b33e71a0_dc897df0","updated":"2022-05-05 22:12:44.000000000","message":"Just want to follow up on Rico\u0027s earlier comment.  The failure in the 1:23 AM set of tests was in tempest-slow-py3 job.  That job reported success on the 10:30 AM result set; the failure there was in cinder-plugin-ceph-tempest, that had 85 failures, mostly in setUpClass, where there were server faults, 500s from Identity, and subnet allocation problems.","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"893272561f3480dfc02045608d879c0057164263","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"962b615b_466396ff","updated":"2022-05-05 14:49:16.000000000","message":"Recheck\n\nThe failed test: tempest.scenario.test_volume_migrate_attached.TestVolumeMigrateRetypeAttached.test_volume_retype_attached from tempest-slow-py3 shows volume migration_status can\u0027t reachout to success\nlet\u0027s see if that\u0027s a consist gate job error","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"cc92a82e0a52647d10c72e29259e456bb2518a9a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"cf8c5d58_5e75025d","updated":"2022-05-11 16:40:14.000000000","message":"This looks good to me, thanks for addressing the earlier concerns.\n","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"985fb0034764f5c9bd9c0c8ebd2bf9f8d644a814","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"34cfffef_4eba3f1c","updated":"2022-05-05 12:17:09.000000000","message":"recheck tempest-slow-py3 - tempest.scenario.test_volume_migrate_attached.TestVolumeMigrateRetypeAttached.test_volume_retype_attached","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"3d338eeb0273c9874711decc3e70ce32c64de647","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"9d4badf0_2350e206","in_reply_to":"b50af605_82602717","updated":"2022-05-12 08:53:26.000000000","message":"For the second and third cases: Just to clarify. Are you suggest we should at least catch ImageUnacceptable in [1]? An alternative solution is to perform this check under [2]? but it will cause extra performance to get image metadata.\n\n[1] https://review.opendev.org/c/openstack/cinder/+/839793/9/cinder/volume/flows/manager/create_volume.py#1016\n[2] cinder/volume/flows/api/create_volume.py","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d092bbf743d3e3d0d6b6e8dad202a37c6ca3143d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"45487304_474255c6","updated":"2022-05-12 18:35:23.000000000","message":"Thanks for the quick update!  Changes look good; see inline for some issues to address.\n\nWe should probably have some tests for the user messages; this patch may give you some ideas: https://review.opendev.org/c/openstack/cinder/+/786627\n\n","commit_id":"893543dad6e64d1638dba28f705ef9510dfebc1c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"dcc2ca60fa409955960333c94e71c52826d0f989","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"879a21b8_92643b22","updated":"2022-05-27 09:52:38.000000000","message":"-1 for the place where message is created. please see comments inline.","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"faa1832328fe8bfb7790e675af0ef1bc8419754d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"92990f79_74a824e0","updated":"2022-05-26 20:46:53.000000000","message":"Code looks good, as do user-facing strings.  Really good test coverage.","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"26421a4c768934d5860a215e150551aa1d6fecd2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"a7931685_48696b40","updated":"2022-05-16 14:59:10.000000000","message":"I encourage this review to be revised. Comments in-line. Otherwise, the patch as a whole looks good to me.","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"d9484c4f04adb1abc675e399f26fe3fdf5ebaed5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"bfe829db_49c0cc52","updated":"2022-05-16 17:06:11.000000000","message":"I\u0027m gonna retype part of Mohammed\u0027s command as I think those are nice feedback on glance option and worth some followup discussion/reply:\n```\n1. First of all, this is a tuneable knob, if you don\u0027t WANT to enable it, you don\u0027t have to.\n2. Glance does handle this just fine.  However, it is possible to have a environment where Nova creates VMs on local disks on hypervisors (QCOW2 is great for this), but Cinder volumes being created from QCOW2 images will absolutely *destroy* your control plane.  In this case, I would like to allow both QCOW2 and RAW, but I don\u0027t want QCOW2 being converted to RAW in my control plane if someone wants to boot from volume.\n3. Interoperable image import is a new feature.  It also relies on converting on the control plane, that is *not* functional at large scale unpredictable public clouds.\n\nIf you have a user that uploads a 200MB QCOW2 image that is 2TB in virtual size, then immediately start to spin up 100 VMs using it, good luck with this.  Cinder will download 200MB x 100 times, then convert 200MB x 100, and being 2TB in virtual size, it WILL write ~200TB to disk, and then upload 200T to Ceph.  It just doesn\u0027t scale.\n\nOnce again, it\u0027s a tuneable, if someone feels like don\u0027t want to change the behaviour, they don\u0027t have to.\n```\n\nIt\u0027s indeed current Glance options doesn\u0027t seems able to fix the issue without coding in cinder. IMO, as Mohammed mentioned, even you set glance image conversion plugin config [1] and disk_formats config [2] all right, What will happen is you gonna generate new image through qemu-img in cinder ([3]) anyway before upload it to glance. So Glance will always get the `right` format of file in this case. But under certain scale of request, your services will probably directly went down.\n\n[1] https://docs.openstack.org/glance/latest/admin/interoperable-image-import.html#the-image-conversion\n[2] https://opendev.org/openstack/glance/src/branch/master/glance/common/config.py#L95-L101\n[3] https://opendev.org/openstack/cinder/src/branch/master/cinder/image/image_utils.py#L381\n","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6c904d1fded10b63a35ca37563fcb0310533efd3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"c1d4137e_5820a1ca","updated":"2022-05-16 15:40:57.000000000","message":"I\u0027ve put in a lot of effort reviewing this patch, and if you trace back through its history you\u0027ll see that it has had to evolve to correctly address the 3 workflows that will be impacted by this change.  So I\u0027m not unsympathetic to the proposal.  I asked some other cores to look at it to make sure that there aren\u0027t any other workflows I missed, and the feedback I got was that we need to have some discussion about whether this configuration option is necessary.  So I asked Rajat to put a hold on the patch until the cinder team has had time to discuss it at our weekly meeting.\n\nI thought the first paragraph of Rajat\u0027s comment with the -2 was pretty clear about why it was assigned.  Maybe a -W is more appropriate, but I personally don\u0027t know if those are \"sticky\" like a -2 is, and we\u0027ve traditionally used -2s for this purpose in Cinder, particularly if there\u0027s already been a +2 on a patch on some point.\n\nSo, I\u0027m renewing the -2 on this patch to put a hold on it until after we\u0027ve had time to discuss it.","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6fe150fe390e6a29beac4c08716c8170f3df9996","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"e0e0962a_3afdb8e7","updated":"2022-05-18 14:22:58.000000000","message":"Lifting my -2, as we\u0027ve discussed the patch at today\u0027s cinder meeting.","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"b51c244f5e6531a6209c828dc6c23e5c7a5b337b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"36b5f59e_3ed679db","updated":"2022-05-16 03:47:18.000000000","message":"There are certain reasons why this doesn\u0027t seem like a valid approach (at least to me) so I would request the authors to join the cinder meeting[1] to discuss this in detail. I will lift my -2 when we reach a positive consensus regarding this approach.\n\nWe already have options in glance that could perform the functionality and since we are dealing with \"images\", this should better be handled at the glance side (with the existing mechanism).\n\nWe have a config option disk_formats[2] which allows us to choose which image format we want to allow or not.\n\nWe also have the image conversion plugin[3] that should handle most of the cases if we want to allow operations with minimal conversions:\n1) create volume from image: set the output format of image same as the volume format to avoid conversion with every new volume create request\n2) reimage volume: same as 1 for new image\n3) upload volume to image: provide image disk format same as the volume format\n\n[1] https://wiki.openstack.org/wiki/CinderMeetings#Weekly_Cinder_team_meeting\n[2] https://opendev.org/openstack/glance/src/branch/master/glance/common/config.py#L95-L101\n[3] https://docs.openstack.org/glance/latest/admin/interoperable-image-import.html#the-image-conversion","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"dfe50569fcba8a3b006190d996e7c4ef07b8be80","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"4faf67df_2d9d093d","updated":"2022-05-14 14:59:19.000000000","message":"recheck on timeout issue","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":33807,"name":"Jacob Wang","email":"jacob_wang1@dell.com","username":"jacob0522"},"change_message_id":"422eb8b82f87b4f4f42a6e460ce81a57de90c12f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"adbac87d_d55fd2bb","updated":"2022-05-16 03:36:13.000000000","message":"run-DellEMC PowerFlex CI","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":33807,"name":"Jacob Wang","email":"jacob_wang1@dell.com","username":"jacob0522"},"change_message_id":"22ad0367af1165e6100c503d91cfd1e006b67549","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"abb68b42_f9420c2e","updated":"2022-05-23 07:37:39.000000000","message":"run-DellEMC PowerStore CI","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8065969d675b869ea982b110328bd976a4b8959d","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":11,"id":"a2b875a2_c3fcd520","in_reply_to":"2fb21366_81c31f29","updated":"2022-05-16 15:20:57.000000000","message":"I\u0027m not sure how a -1 helps in this case when it gets removed with a patch update. My initial intentions of the -2 was just to hold further discussions for this (which I\u0027ve quite clearly mentioned in my comment) but looks like it has been taken into a different context and for the same reason I will lift it to clear things up.","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":1004,"name":"Mohammed Naser","email":"mnaser@vexxhost.com","username":"mnaser"},"change_message_id":"71ed92f8006b8d87156b3fce96e114419f348b06","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":11,"id":"c1753e79_02586399","in_reply_to":"36b5f59e_3ed679db","updated":"2022-05-16 14:29:06.000000000","message":"I fully disagree with this, and I fully disagree that this is a -2 worthy comment after 17 days of going back and forth on this patch.\n\n1. First of all, this is a tuneable knob, if you don\u0027t WANT to enable it, you don\u0027t have to.\n2. Glance does handle this just fine.  However, it is possible to have a environment where Nova creates VMs on local disks on hypervisors (QCOW2 is great for this), but Cinder volumes being created from QCOW2 images will absolutely *destroy* your control plane.  In this case, I would like to allow both QCOW2 and RAW, but I don\u0027t want QCOW2 being converted to RAW in my control plane if someone wants to boot from volume.\n3. Interoperable image import is a new feature.  It also relies on converting on the control plane, that is *not* functional at large scale unpredictable public clouds.\n\nIf you have a user that uploads a 200MB QCOW2 image that is 2TB in virtual size, then immediately start to spin up 100 VMs using it, good luck with this.  Cinder will download 200MB x 100 times, then convert 200MB x 100, and being 2TB in virtual size, it WILL write ~200TB to disk, and then upload 200T to Ceph.  It just doesn\u0027t scale.\n\nOnce again, it\u0027s a tuneable, if someone feels like don\u0027t want to change the behaviour, they don\u0027t have to.","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"4de9cb284e589c259d627687978ec7d09f0338fe","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":11,"id":"2fb21366_81c31f29","in_reply_to":"7b55bd20_6c9b4d2d","updated":"2022-05-16 15:15:59.000000000","message":"To be completely honest, this growing trend I\u0027m seeing of cores -2\u0027ing what amount to bug fixes, or paths for bug fixes, is really quite worrisome because we\u0027re making it more difficult to get a fix upstream than it would be to just maintain a fork downstream. A -1 could suffice for the exact same purpose to just force discussion in *most* of our projects.","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"5bc05fac006693ef8d9ae8e4861792361e56a3b2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"f68fb0e6_42c82d08","in_reply_to":"a2b875a2_c3fcd520","updated":"2022-05-25 16:55:58.000000000","message":"Addressed","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"f67842eee993d00254104bdedf43a0d025607379","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":11,"id":"7b55bd20_6c9b4d2d","in_reply_to":"c1753e79_02586399","updated":"2022-05-16 15:04:39.000000000","message":"Hi Mohammed Naser,\n\nApologies if the -2 seemed too harsh but just to clear it out, It is not to block the patch forever but just to hold it until we discuss this properly. I understand this took time and efforts but my ask is to wait until the Cinder meeting (which is in 2 days i.e. Wednesday) to bring this idea in the Cinder meeting so we can discuss this better. Hope you understand. :)","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"26421a4c768934d5860a215e150551aa1d6fecd2","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":11,"id":"cbe9d058_4f0799fe","in_reply_to":"c1753e79_02586399","updated":"2022-05-16 14:59:10.000000000","message":"We\u0027re seeing some sort of similar things in ironic where there is huge growing concern from operator practical experience about massive parallel operations being launched and then causing huge issues which are not theoretical in nature, but operationally relevant.\n\nIn ironic, we\u0027ve found we\u0027ve had to add options, guard rails, and things that generally don\u0027t seem very ideal just because not everything is perfect, and good is far better for our users than perfection.\n\nI encourage this -2 to be revised as such, this is an attempt for good in a realistic operator setting.\n\nAdditionally, the fact that a major OpenStack operator is disagreeing on this thread should be considered with a substantial amount of weight. I encourage the core reviewers of Cinder to consider this as well.","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"29333da40392565c306a92e14727ff3cc74fa32d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"48ba213e_9759e6d6","updated":"2022-05-31 11:57:36.000000000","message":"Comment inline.","commit_id":"3a3d1710509abe7c3af15df871b6b4d108c030ef"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d3e15eb9f4d46fa5a2dd293c1a060bf296566293","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"c4e6b381_59a3fc4f","updated":"2022-05-31 15:15:29.000000000","message":"Two nits noted inline if you need to put up a new patch.  Otherwise, revisions LGTM.  Glad that Rajat caught the issue of different ImageUnacceptable exceptions being caught by the code generating the user message.  ","commit_id":"41104531bace557cab2c92a40f3aef1ae66ffe49"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"9eb6e1e3f950a6662195bb2a160229fab2439e05","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"5731c7cd_b9bd2d72","updated":"2022-06-03 14:17:41.000000000","message":"Parent patch has merged.","commit_id":"a719525c1c5aa0d08af881dc1ee72f56cb5b7deb"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"4136df6c35ca103553b754c07c8391a2b036160c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"d018eb2b_7fabd04a","updated":"2022-06-02 21:45:31.000000000","message":"Revisions look good to me.  Good idea to use a custom exception for this case!","commit_id":"a719525c1c5aa0d08af881dc1ee72f56cb5b7deb"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"07dc3f3115e3de9039e3b375882822d38fb730ee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"abec4a08_f4e6a2a0","updated":"2022-06-01 05:45:51.000000000","message":"Thanks for addressing all my comments. LGTM.","commit_id":"a719525c1c5aa0d08af881dc1ee72f56cb5b7deb"}],"cinder/image/image_utils.py":[{"author":{"_account_id":1004,"name":"Mohammed Naser","email":"mnaser@vexxhost.com","username":"mnaser"},"change_message_id":"cb2d13e5f736d400e8ac98d89d5214fb655c43bf","unresolved":true,"context_lines":[{"line_number":712,"context_line":"                           run_as_root: bool \u003d True) -\u003e None:"},{"line_number":713,"context_line":"    qemu_img \u003d True"},{"line_number":714,"context_line":"    image_meta \u003d image_service.show(context, image_id)"},{"line_number":715,"context_line":""},{"line_number":716,"context_line":"    allow_image_compression \u003d CONF.allow_compression_on_image_upload"},{"line_number":717,"context_line":"    if image_meta and (image_meta.get(\u0027container_format\u0027) \u003d\u003d \u0027compressed\u0027):"},{"line_number":718,"context_line":"        if allow_image_compression is False:"}],"source_content_type":"text/x-python","patch_set":1,"id":"2cdc6c26_e1647498","line":715,"updated":"2022-04-28 17:42:44.000000000","message":"I think we should move that here:\n\ncheck_allow_image_conversion(disk_format, volume_format, image_id)\n\nThe reason being that if we don\u0027t, then we\u0027ve already downloaded the image for no reason and failed.","commit_id":"193dca2dd3ff9a9026ed298d674578e893bfccd6"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"de88d549897c865d5246be154d89ab3ee43d41b8","unresolved":false,"context_lines":[{"line_number":712,"context_line":"                           run_as_root: bool \u003d True) -\u003e None:"},{"line_number":713,"context_line":"    qemu_img \u003d True"},{"line_number":714,"context_line":"    image_meta \u003d image_service.show(context, image_id)"},{"line_number":715,"context_line":""},{"line_number":716,"context_line":"    allow_image_compression \u003d CONF.allow_compression_on_image_upload"},{"line_number":717,"context_line":"    if image_meta and (image_meta.get(\u0027container_format\u0027) \u003d\u003d \u0027compressed\u0027):"},{"line_number":718,"context_line":"        if allow_image_compression is False:"}],"source_content_type":"text/x-python","patch_set":1,"id":"2c3d6ef0_3ad5f101","line":715,"in_reply_to":"2cdc6c26_e1647498","updated":"2022-04-29 05:47:15.000000000","message":"Done","commit_id":"193dca2dd3ff9a9026ed298d674578e893bfccd6"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"4e32a33b75b244de6035a0eb1cf19d9b1172043b","unresolved":true,"context_lines":[{"line_number":694,"context_line":"        raise exception.ImageUnacceptable("},{"line_number":695,"context_line":"            reason\u003d_(\"Image disk_format (current: %s) should be same \""},{"line_number":696,"context_line":"                     \"format as volume_format (current: %s) when image \""},{"line_number":697,"context_line":"                     \"config `allow_image_conversion` is setted off.\""},{"line_number":698,"context_line":"                     ) % (disk_format, volume_format),"},{"line_number":699,"context_line":"            image_id\u003dimage_id)"},{"line_number":700,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"ba9c7102_5adab5aa","line":697,"range":{"start_line":697,"start_character":57,"end_line":697,"end_character":63},"updated":"2022-04-29 13:16:42.000000000","message":"\"set off\" would be more grammatically correct.","commit_id":"44e4484b42e81d8094e825e784b31d276a392b83"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"ad61c907c606ce9e7c4d42ee9f84c66c9f5d4cc8","unresolved":true,"context_lines":[{"line_number":694,"context_line":"        raise exception.ImageUnacceptable("},{"line_number":695,"context_line":"            reason\u003d_(\"Image disk_format (current: %s) should be same \""},{"line_number":696,"context_line":"                     \"format as volume_format (current: %s) when image \""},{"line_number":697,"context_line":"                     \"config `allow_image_conversion` is setted off.\""},{"line_number":698,"context_line":"                     ) % (disk_format, volume_format),"},{"line_number":699,"context_line":"            image_id\u003dimage_id)"},{"line_number":700,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"cf63d33c_82062ebb","line":697,"range":{"start_line":697,"start_character":57,"end_line":697,"end_character":63},"in_reply_to":"ba9c7102_5adab5aa","updated":"2022-04-29 14:48:06.000000000","message":"\"set to off\" I think would be best.","commit_id":"44e4484b42e81d8094e825e784b31d276a392b83"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"5ae061ba0e9cfbb9bf1d8d5908285c9d2e94f979","unresolved":false,"context_lines":[{"line_number":694,"context_line":"        raise exception.ImageUnacceptable("},{"line_number":695,"context_line":"            reason\u003d_(\"Image disk_format (current: %s) should be same \""},{"line_number":696,"context_line":"                     \"format as volume_format (current: %s) when image \""},{"line_number":697,"context_line":"                     \"config `allow_image_conversion` is setted off.\""},{"line_number":698,"context_line":"                     ) % (disk_format, volume_format),"},{"line_number":699,"context_line":"            image_id\u003dimage_id)"},{"line_number":700,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"fe83d3d5_f921aaf8","line":697,"range":{"start_line":697,"start_character":57,"end_line":697,"end_character":63},"in_reply_to":"cf63d33c_82062ebb","updated":"2022-05-04 04:21:32.000000000","message":"Done","commit_id":"44e4484b42e81d8094e825e784b31d276a392b83"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"33d9a090d0c5cf4cb6e3e63a9aeec7a2a83dfacd","unresolved":true,"context_lines":[{"line_number":73,"context_line":"    cfg.IntOpt(\u0027image_conversion_address_space_limit\u0027,"},{"line_number":74,"context_line":"               default\u003d1,"},{"line_number":75,"context_line":"               help\u003d\u0027Address space limit in gigabytes to convert the image\u0027),"},{"line_number":76,"context_line":"    cfg.BoolOpt(\u0027allow_image_conversion\u0027,"},{"line_number":77,"context_line":"                default\u003dTrue,"},{"line_number":78,"context_line":"                help\u003d\u0027Allow image conversion across different formats between \u0027"},{"line_number":79,"context_line":"                \u0027image disk_format and volume format.\u0027),"}],"source_content_type":"text/x-python","patch_set":5,"id":"82cf2575_e87e8b89","line":76,"range":{"start_line":76,"start_character":17,"end_line":76,"end_character":39},"updated":"2022-05-03 18:34:42.000000000","message":"I suggest calling this \u0027image_conversion_disallowed\u0027 to be consistent with the other image conversion options.  (It also flips your logic, but what you\u0027re doing is turning off something that is normally allowed.)","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"5ae061ba0e9cfbb9bf1d8d5908285c9d2e94f979","unresolved":false,"context_lines":[{"line_number":73,"context_line":"    cfg.IntOpt(\u0027image_conversion_address_space_limit\u0027,"},{"line_number":74,"context_line":"               default\u003d1,"},{"line_number":75,"context_line":"               help\u003d\u0027Address space limit in gigabytes to convert the image\u0027),"},{"line_number":76,"context_line":"    cfg.BoolOpt(\u0027allow_image_conversion\u0027,"},{"line_number":77,"context_line":"                default\u003dTrue,"},{"line_number":78,"context_line":"                help\u003d\u0027Allow image conversion across different formats between \u0027"},{"line_number":79,"context_line":"                \u0027image disk_format and volume format.\u0027),"}],"source_content_type":"text/x-python","patch_set":5,"id":"0b06b8b3_6d354292","line":76,"range":{"start_line":76,"start_character":17,"end_line":76,"end_character":39},"in_reply_to":"82cf2575_e87e8b89","updated":"2022-05-04 04:21:32.000000000","message":"Done","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"33d9a090d0c5cf4cb6e3e63a9aeec7a2a83dfacd","unresolved":true,"context_lines":[{"line_number":75,"context_line":"               help\u003d\u0027Address space limit in gigabytes to convert the image\u0027),"},{"line_number":76,"context_line":"    cfg.BoolOpt(\u0027allow_image_conversion\u0027,"},{"line_number":77,"context_line":"                default\u003dTrue,"},{"line_number":78,"context_line":"                help\u003d\u0027Allow image conversion across different formats between \u0027"},{"line_number":79,"context_line":"                \u0027image disk_format and volume format.\u0027),"},{"line_number":80,"context_line":"]"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":5,"id":"ce14d3ec_1a50742f","line":79,"range":{"start_line":78,"start_character":22,"end_line":79,"end_character":54},"updated":"2022-05-03 18:34:42.000000000","message":"This will be the only documentation of this feature, so you should say a bit more about what it\u0027s for and why someone would want to use it.","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"5ae061ba0e9cfbb9bf1d8d5908285c9d2e94f979","unresolved":false,"context_lines":[{"line_number":75,"context_line":"               help\u003d\u0027Address space limit in gigabytes to convert the image\u0027),"},{"line_number":76,"context_line":"    cfg.BoolOpt(\u0027allow_image_conversion\u0027,"},{"line_number":77,"context_line":"                default\u003dTrue,"},{"line_number":78,"context_line":"                help\u003d\u0027Allow image conversion across different formats between \u0027"},{"line_number":79,"context_line":"                \u0027image disk_format and volume format.\u0027),"},{"line_number":80,"context_line":"]"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":5,"id":"eb8abc39_acde6ce5","line":79,"range":{"start_line":78,"start_character":22,"end_line":79,"end_character":54},"in_reply_to":"ce14d3ec_1a50742f","updated":"2022-05-04 04:21:32.000000000","message":"Done","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"33d9a090d0c5cf4cb6e3e63a9aeec7a2a83dfacd","unresolved":true,"context_lines":[{"line_number":695,"context_line":"            reason\u003d_(\"Image disk_format (current: %s) should be same \""},{"line_number":696,"context_line":"                     \"format as volume_format (current: %s) when image \""},{"line_number":697,"context_line":"                     \"config `allow_image_conversion` is set to off.\""},{"line_number":698,"context_line":"                     ) % (disk_format, volume_format),"},{"line_number":699,"context_line":"            image_id\u003dimage_id)"},{"line_number":700,"context_line":""},{"line_number":701,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"6bfe92b4_279ba3ee","line":698,"updated":"2022-05-03 18:34:42.000000000","message":"Two things here:\n\n1. Please use named interpolation for a translated string with multiple variables.  See https://docs.openstack.org/oslo.i18n/latest/user/guidelines.html#adding-variables-to-translated-messages\n\n2. Ordinary end users will receive this message, so it\u0027s not helpful to tell them the configuration option name.  Also, the volume_format isn\u0027t something an end user can specify directly.  I suggest something like:\n\n  \"Image conversion is disabled.  The volume type you have requested requires that the image it is being created from be in \u0027%(volume_format)s\u0027 format, but the image you are using has the disk_format property \u0027%(disk_format)s\u0027.\"","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"5ae061ba0e9cfbb9bf1d8d5908285c9d2e94f979","unresolved":true,"context_lines":[{"line_number":695,"context_line":"            reason\u003d_(\"Image disk_format (current: %s) should be same \""},{"line_number":696,"context_line":"                     \"format as volume_format (current: %s) when image \""},{"line_number":697,"context_line":"                     \"config `allow_image_conversion` is set to off.\""},{"line_number":698,"context_line":"                     ) % (disk_format, volume_format),"},{"line_number":699,"context_line":"            image_id\u003dimage_id)"},{"line_number":700,"context_line":""},{"line_number":701,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"e4c52e24_ddac0117","line":698,"in_reply_to":"6bfe92b4_279ba3ee","updated":"2022-05-04 04:21:32.000000000","message":"Thanks for the message hints","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"6266daa3489060180d6abcdf0b14e3507bbd5b79","unresolved":false,"context_lines":[{"line_number":695,"context_line":"            reason\u003d_(\"Image disk_format (current: %s) should be same \""},{"line_number":696,"context_line":"                     \"format as volume_format (current: %s) when image \""},{"line_number":697,"context_line":"                     \"config `allow_image_conversion` is set to off.\""},{"line_number":698,"context_line":"                     ) % (disk_format, volume_format),"},{"line_number":699,"context_line":"            image_id\u003dimage_id)"},{"line_number":700,"context_line":""},{"line_number":701,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"2a9714ca_98b92ae9","line":698,"in_reply_to":"e4c52e24_ddac0117","updated":"2022-05-05 03:23:15.000000000","message":"Done","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"48aa9723628d917ab2a2d881ba17b2ddaec62153","unresolved":true,"context_lines":[{"line_number":73,"context_line":"    cfg.IntOpt(\u0027image_conversion_address_space_limit\u0027,"},{"line_number":74,"context_line":"               default\u003d1,"},{"line_number":75,"context_line":"               help\u003d\u0027Address space limit in gigabytes to convert the image\u0027),"},{"line_number":76,"context_line":"    cfg.BoolOpt(\u0027disable_image_conversion\u0027,"},{"line_number":77,"context_line":"                default\u003dFalse,"},{"line_number":78,"context_line":"                help\u003d\u0027Disallow image conversion across different formats \u0027"},{"line_number":79,"context_line":"                \u0027between image disk_format and volume format. \u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"5e4f6877_1ab5a0d8","line":76,"range":{"start_line":76,"start_character":17,"end_line":76,"end_character":41},"updated":"2022-05-04 15:06:25.000000000","message":"I really prefer this to be \u0027image_conversion_disable\u0027 so that an operator can grep a config file for \u0027image_conversion_\u0027 and see all the relevant options/settings.","commit_id":"883d85305ea14f2be9626f22596f61bea0771cdf"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c4c2147b31e9d40873fa1943a57347b03394578b","unresolved":false,"context_lines":[{"line_number":73,"context_line":"    cfg.IntOpt(\u0027image_conversion_address_space_limit\u0027,"},{"line_number":74,"context_line":"               default\u003d1,"},{"line_number":75,"context_line":"               help\u003d\u0027Address space limit in gigabytes to convert the image\u0027),"},{"line_number":76,"context_line":"    cfg.BoolOpt(\u0027disable_image_conversion\u0027,"},{"line_number":77,"context_line":"                default\u003dFalse,"},{"line_number":78,"context_line":"                help\u003d\u0027Disallow image conversion across different formats \u0027"},{"line_number":79,"context_line":"                \u0027between image disk_format and volume format. \u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"ef4fdd86_b040ccdd","line":76,"range":{"start_line":76,"start_character":17,"end_line":76,"end_character":41},"in_reply_to":"5e4f6877_1ab5a0d8","updated":"2022-05-04 17:04:17.000000000","message":"Done","commit_id":"883d85305ea14f2be9626f22596f61bea0771cdf"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"97af5a81c99b01279c3173673d9898b80f495102","unresolved":true,"context_lines":[{"line_number":79,"context_line":"                \u0027an image and when uploading a volume as an image.  Image \u0027"},{"line_number":80,"context_line":"                \u0027conversion consumes a large amount of system resources and \u0027"},{"line_number":81,"context_line":"                \u0027can cause performance problems on the cinder-volume node.  \u0027"},{"line_number":82,"context_line":"                \u0027When set True, this option disables image conversion.  User \u0027"},{"line_number":83,"context_line":"                \u0027requests specifying a disk_format that requires conversion \u0027"},{"line_number":84,"context_line":"                \u0027will result in a 400 (Bad Request) response with an \u0027"},{"line_number":85,"context_line":"                \u0027informative error message.\u0027),"},{"line_number":86,"context_line":"]"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":9,"id":"a8fa5907_b34091ad","line":85,"range":{"start_line":82,"start_character":72,"end_line":85,"end_character":44},"updated":"2022-05-11 20:11:57.000000000","message":"Looking into this more closely, this is actually only true for the upload-volume-as-image call.  The other API calls that would run into this return 202s and then bad stuff happens later (volume goes to error status).","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d092bbf743d3e3d0d6b6e8dad202a37c6ca3143d","unresolved":false,"context_lines":[{"line_number":79,"context_line":"                \u0027an image and when uploading a volume as an image.  Image \u0027"},{"line_number":80,"context_line":"                \u0027conversion consumes a large amount of system resources and \u0027"},{"line_number":81,"context_line":"                \u0027can cause performance problems on the cinder-volume node.  \u0027"},{"line_number":82,"context_line":"                \u0027When set True, this option disables image conversion.  User \u0027"},{"line_number":83,"context_line":"                \u0027requests specifying a disk_format that requires conversion \u0027"},{"line_number":84,"context_line":"                \u0027will result in a 400 (Bad Request) response with an \u0027"},{"line_number":85,"context_line":"                \u0027informative error message.\u0027),"},{"line_number":86,"context_line":"]"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":9,"id":"94745252_9be6ca27","line":85,"range":{"start_line":82,"start_character":72,"end_line":85,"end_character":44},"in_reply_to":"a8fa5907_b34091ad","updated":"2022-05-12 18:35:23.000000000","message":"Done","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"26421a4c768934d5860a215e150551aa1d6fecd2","unresolved":true,"context_lines":[{"line_number":73,"context_line":"    cfg.IntOpt(\u0027image_conversion_address_space_limit\u0027,"},{"line_number":74,"context_line":"               default\u003d1,"},{"line_number":75,"context_line":"               help\u003d\u0027Address space limit in gigabytes to convert the image\u0027),"},{"line_number":76,"context_line":"    cfg.BoolOpt(\u0027image_conversion_disable\u0027,"},{"line_number":77,"context_line":"                default\u003dFalse,"},{"line_number":78,"context_line":"                help\u003d\u0027Disallow image conversion when creating a volume from \u0027"},{"line_number":79,"context_line":"                \u0027an image and when uploading a volume as an image. Image \u0027"},{"line_number":80,"context_line":"                \u0027conversion consumes a large amount of system resources and \u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"e2182faf_cb304445","line":77,"range":{"start_line":76,"start_character":0,"end_line":77,"end_character":30},"updated":"2022-05-16 14:59:10.000000000","message":"personally, not a fan of the negative name with a false default, but... hey the help makes a lot of sense to me.","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"1ddd21611b65d0ba57f0861a509802db079a8d0c","unresolved":false,"context_lines":[{"line_number":73,"context_line":"    cfg.IntOpt(\u0027image_conversion_address_space_limit\u0027,"},{"line_number":74,"context_line":"               default\u003d1,"},{"line_number":75,"context_line":"               help\u003d\u0027Address space limit in gigabytes to convert the image\u0027),"},{"line_number":76,"context_line":"    cfg.BoolOpt(\u0027image_conversion_disable\u0027,"},{"line_number":77,"context_line":"                default\u003dFalse,"},{"line_number":78,"context_line":"                help\u003d\u0027Disallow image conversion when creating a volume from \u0027"},{"line_number":79,"context_line":"                \u0027an image and when uploading a volume as an image. Image \u0027"},{"line_number":80,"context_line":"                \u0027conversion consumes a large amount of system resources and \u0027"}],"source_content_type":"text/x-python","patch_set":11,"id":"f0900f12_aaeb1ddd","line":77,"range":{"start_line":76,"start_character":0,"end_line":77,"end_character":30},"in_reply_to":"e2182faf_cb304445","updated":"2022-05-25 16:57:00.000000000","message":"glad the help doc helps!:)","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"dcc2ca60fa409955960333c94e71c52826d0f989","unresolved":true,"context_lines":[{"line_number":696,"context_line":"                                   upload\u003dFalse):"},{"line_number":697,"context_line":"    if CONF.image_conversion_disable and disk_format !\u003d volume_format:"},{"line_number":698,"context_line":"        if upload:"},{"line_number":699,"context_line":"            msg \u003d _(\"Image conversion is disabled. The image disk_format \""},{"line_number":700,"context_line":"                    \"you have requested is \u0027%(disk_format)s\u0027, but your \""},{"line_number":701,"context_line":"                    \"volume can only be uploaded in the format \""},{"line_number":702,"context_line":"                    \"\u0027%(volume_format)s\u0027.\")"},{"line_number":703,"context_line":"        else:"},{"line_number":704,"context_line":"            msg \u003d _(\"Image conversion is disabled. The volume type you have \""},{"line_number":705,"context_line":"                    \"requested requires that the image it is being created \""}],"source_content_type":"text/x-python","patch_set":11,"id":"1851db50_f10d55a1","line":702,"range":{"start_line":699,"start_character":20,"end_line":702,"end_character":42},"updated":"2022-05-27 09:52:38.000000000","message":"in this case, what is the user supposed to do? create another volume matching the image format or request this operation with another image format?","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"52e4596a49165d960cbe9eebc886b97d3807ec52","unresolved":false,"context_lines":[{"line_number":696,"context_line":"                                   upload\u003dFalse):"},{"line_number":697,"context_line":"    if CONF.image_conversion_disable and disk_format !\u003d volume_format:"},{"line_number":698,"context_line":"        if upload:"},{"line_number":699,"context_line":"            msg \u003d _(\"Image conversion is disabled. The image disk_format \""},{"line_number":700,"context_line":"                    \"you have requested is \u0027%(disk_format)s\u0027, but your \""},{"line_number":701,"context_line":"                    \"volume can only be uploaded in the format \""},{"line_number":702,"context_line":"                    \"\u0027%(volume_format)s\u0027.\")"},{"line_number":703,"context_line":"        else:"},{"line_number":704,"context_line":"            msg \u003d _(\"Image conversion is disabled. The volume type you have \""},{"line_number":705,"context_line":"                    \"requested requires that the image it is being created \""}],"source_content_type":"text/x-python","patch_set":11,"id":"88128bd8_6b41ff1c","line":702,"range":{"start_line":699,"start_character":20,"end_line":702,"end_character":42},"in_reply_to":"1851db50_f10d55a1","updated":"2022-05-27 18:42:26.000000000","message":"This message only will show when during upload volume to image and we disable conversion. What user can do (on API case) is to provide the corresponding disk_format in os-volume_upload_image which they should be able to get the correct format information (volume_format).\nOr are you suggest we should add these in message?","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8f7414831489f7b1c8bd5d2d4a2d3eebee83b53c","unresolved":false,"context_lines":[{"line_number":696,"context_line":"                                   upload\u003dFalse):"},{"line_number":697,"context_line":"    if CONF.image_conversion_disable and disk_format !\u003d volume_format:"},{"line_number":698,"context_line":"        if upload:"},{"line_number":699,"context_line":"            msg \u003d _(\"Image conversion is disabled. The image disk_format \""},{"line_number":700,"context_line":"                    \"you have requested is \u0027%(disk_format)s\u0027, but your \""},{"line_number":701,"context_line":"                    \"volume can only be uploaded in the format \""},{"line_number":702,"context_line":"                    \"\u0027%(volume_format)s\u0027.\")"},{"line_number":703,"context_line":"        else:"},{"line_number":704,"context_line":"            msg \u003d _(\"Image conversion is disabled. The volume type you have \""},{"line_number":705,"context_line":"                    \"requested requires that the image it is being created \""}],"source_content_type":"text/x-python","patch_set":11,"id":"22f8ce18_f19712f3","line":702,"range":{"start_line":699,"start_character":20,"end_line":702,"end_character":42},"in_reply_to":"88128bd8_6b41ff1c","updated":"2022-05-30 18:22:14.000000000","message":"I think it would be helpful if that\u0027s the suggested method since we are blocking this operation and users might be lost about how they should proceed if they see this message.","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"dcc2ca60fa409955960333c94e71c52826d0f989","unresolved":false,"context_lines":[{"line_number":714,"context_line":"            image_id\u003dimage_id)"},{"line_number":715,"context_line":""},{"line_number":716,"context_line":""},{"line_number":717,"context_line":"def fetch_to_volume_format(context: context.RequestContext,"},{"line_number":718,"context_line":"                           image_service: glance.GlanceImageService,"},{"line_number":719,"context_line":"                           image_id: str,"},{"line_number":720,"context_line":"                           dest: str,"}],"source_content_type":"text/x-python","patch_set":11,"id":"d23052cd_6c5b5233","line":717,"range":{"start_line":717,"start_character":4,"end_line":717,"end_character":26},"updated":"2022-05-27 09:52:38.000000000","message":"all the calls to this method are while creating a bootable volume from image so we are good","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"}],"cinder/message/message_field.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d092bbf743d3e3d0d6b6e8dad202a37c6ca3143d","unresolved":true,"context_lines":[{"line_number":130,"context_line":"    REIMAGE_VOLUME_FAILED \u003d ("},{"line_number":131,"context_line":"        \u0027028\u0027,"},{"line_number":132,"context_line":"        _(\"Compute service failed to reimage volume.\"))"},{"line_number":133,"context_line":"    IMAGE_UNACCEPTABLE_FAILED \u003d ("},{"line_number":134,"context_line":"        \u0027029\u0027,"},{"line_number":135,"context_line":"        _(\"Image disk format not equal to the default volume format.\"))"},{"line_number":136,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"22cc3fd9_d4be91b7","line":133,"range":{"start_line":133,"start_character":4,"end_line":133,"end_character":30},"updated":"2022-05-12 18:35:23.000000000","message":"I know this is on the verge of bikeshedding, but how about IMAGE_FORMAT_UNACCEPTABLE for this?  (That way you leave room for future messages about other reasons why an image is unacceptable.)","commit_id":"893543dad6e64d1638dba28f705ef9510dfebc1c"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"dfe50569fcba8a3b006190d996e7c4ef07b8be80","unresolved":false,"context_lines":[{"line_number":130,"context_line":"    REIMAGE_VOLUME_FAILED \u003d ("},{"line_number":131,"context_line":"        \u0027028\u0027,"},{"line_number":132,"context_line":"        _(\"Compute service failed to reimage volume.\"))"},{"line_number":133,"context_line":"    IMAGE_UNACCEPTABLE_FAILED \u003d ("},{"line_number":134,"context_line":"        \u0027029\u0027,"},{"line_number":135,"context_line":"        _(\"Image disk format not equal to the default volume format.\"))"},{"line_number":136,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"2ad395bd_838aa45f","line":133,"range":{"start_line":133,"start_character":4,"end_line":133,"end_character":30},"in_reply_to":"22cc3fd9_d4be91b7","updated":"2022-05-14 14:59:19.000000000","message":"Done","commit_id":"893543dad6e64d1638dba28f705ef9510dfebc1c"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d092bbf743d3e3d0d6b6e8dad202a37c6ca3143d","unresolved":true,"context_lines":[{"line_number":132,"context_line":"        _(\"Compute service failed to reimage volume.\"))"},{"line_number":133,"context_line":"    IMAGE_UNACCEPTABLE_FAILED \u003d ("},{"line_number":134,"context_line":"        \u0027029\u0027,"},{"line_number":135,"context_line":"        _(\"Image disk format not equal to the default volume format.\"))"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    ALL \u003d (UNKNOWN_ERROR,"},{"line_number":138,"context_line":"           DRIVER_NOT_INITIALIZED,"}],"source_content_type":"text/x-python","patch_set":10,"id":"7e053af5_e3a1d84d","line":135,"range":{"start_line":135,"start_character":11,"end_line":135,"end_character":68},"updated":"2022-05-12 18:35:23.000000000","message":"How about:\n\nThe image disk format must be the same as the volume format for the volume type you are requesting.\n\n(because we know this will only be used in the download situation)","commit_id":"893543dad6e64d1638dba28f705ef9510dfebc1c"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"dfe50569fcba8a3b006190d996e7c4ef07b8be80","unresolved":false,"context_lines":[{"line_number":132,"context_line":"        _(\"Compute service failed to reimage volume.\"))"},{"line_number":133,"context_line":"    IMAGE_UNACCEPTABLE_FAILED \u003d ("},{"line_number":134,"context_line":"        \u0027029\u0027,"},{"line_number":135,"context_line":"        _(\"Image disk format not equal to the default volume format.\"))"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    ALL \u003d (UNKNOWN_ERROR,"},{"line_number":138,"context_line":"           DRIVER_NOT_INITIALIZED,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3907fe88_0cb57581","line":135,"range":{"start_line":135,"start_character":11,"end_line":135,"end_character":68},"in_reply_to":"7e053af5_e3a1d84d","updated":"2022-05-14 14:59:19.000000000","message":"Done","commit_id":"893543dad6e64d1638dba28f705ef9510dfebc1c"}],"cinder/tests/unit/test_image_utils.py":[{"author":{"_account_id":1004,"name":"Mohammed Naser","email":"mnaser@vexxhost.com","username":"mnaser"},"change_message_id":"cb2d13e5f736d400e8ac98d89d5214fb655c43bf","unresolved":true,"context_lines":[{"line_number":1212,"context_line":""},{"line_number":1213,"context_line":"        CONF.set_override(\u0027allow_image_conversion\u0027, True)"},{"line_number":1214,"context_line":"        self.assertIsNone(image_utils.check_allow_image_conversion("},{"line_number":1215,"context_line":"            \u0027foo\u0027, \u0027bar\u0027, fake.IMAGE_ID))"},{"line_number":1216,"context_line":""},{"line_number":1217,"context_line":"    def test_check_not_allow_image_conversion(self):"},{"line_number":1218,"context_line":"        CONF.set_override(\u0027allow_image_conversion\u0027, False)"}],"source_content_type":"text/x-python","patch_set":1,"id":"c85cdcd9_5b528c14","line":1215,"updated":"2022-04-28 17:42:44.000000000","message":"also maybe add a test here too that if they are identical it also works?","commit_id":"193dca2dd3ff9a9026ed298d674578e893bfccd6"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"de88d549897c865d5246be154d89ab3ee43d41b8","unresolved":false,"context_lines":[{"line_number":1212,"context_line":""},{"line_number":1213,"context_line":"        CONF.set_override(\u0027allow_image_conversion\u0027, True)"},{"line_number":1214,"context_line":"        self.assertIsNone(image_utils.check_allow_image_conversion("},{"line_number":1215,"context_line":"            \u0027foo\u0027, \u0027bar\u0027, fake.IMAGE_ID))"},{"line_number":1216,"context_line":""},{"line_number":1217,"context_line":"    def test_check_not_allow_image_conversion(self):"},{"line_number":1218,"context_line":"        CONF.set_override(\u0027allow_image_conversion\u0027, False)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1dc71328_ddf9fe01","line":1215,"in_reply_to":"c85cdcd9_5b528c14","updated":"2022-04-29 05:47:15.000000000","message":"Done","commit_id":"193dca2dd3ff9a9026ed298d674578e893bfccd6"},{"author":{"_account_id":1004,"name":"Mohammed Naser","email":"mnaser@vexxhost.com","username":"mnaser"},"change_message_id":"cb2d13e5f736d400e8ac98d89d5214fb655c43bf","unresolved":true,"context_lines":[{"line_number":1214,"context_line":"        self.assertIsNone(image_utils.check_allow_image_conversion("},{"line_number":1215,"context_line":"            \u0027foo\u0027, \u0027bar\u0027, fake.IMAGE_ID))"},{"line_number":1216,"context_line":""},{"line_number":1217,"context_line":"    def test_check_not_allow_image_conversion(self):"},{"line_number":1218,"context_line":"        CONF.set_override(\u0027allow_image_conversion\u0027, False)"},{"line_number":1219,"context_line":"        self.assertRaises("},{"line_number":1220,"context_line":"            exception.ImageUnacceptable,"}],"source_content_type":"text/x-python","patch_set":1,"id":"cfb2dcb7_b40e5653","line":1217,"updated":"2022-04-28 17:42:44.000000000","message":"maybe add a test that it won\u0027t raise any exceptions if allow_image_conversion\u003dFalse and both values are identical?","commit_id":"193dca2dd3ff9a9026ed298d674578e893bfccd6"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"de88d549897c865d5246be154d89ab3ee43d41b8","unresolved":false,"context_lines":[{"line_number":1214,"context_line":"        self.assertIsNone(image_utils.check_allow_image_conversion("},{"line_number":1215,"context_line":"            \u0027foo\u0027, \u0027bar\u0027, fake.IMAGE_ID))"},{"line_number":1216,"context_line":""},{"line_number":1217,"context_line":"    def test_check_not_allow_image_conversion(self):"},{"line_number":1218,"context_line":"        CONF.set_override(\u0027allow_image_conversion\u0027, False)"},{"line_number":1219,"context_line":"        self.assertRaises("},{"line_number":1220,"context_line":"            exception.ImageUnacceptable,"}],"source_content_type":"text/x-python","patch_set":1,"id":"58c9abe1_ee30ae16","line":1217,"in_reply_to":"cfb2dcb7_b40e5653","updated":"2022-04-29 05:47:15.000000000","message":"Done","commit_id":"193dca2dd3ff9a9026ed298d674578e893bfccd6"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"3ac0115d8d984b6e56652001a607dd19283ce4aa","unresolved":true,"context_lines":[{"line_number":1488,"context_line":"        run_as_root \u003d mock.sentinel.run_as_root"},{"line_number":1489,"context_line":""},{"line_number":1490,"context_line":"        tmp \u003d mock_temp.return_value.__enter__.return_value"},{"line_number":1491,"context_line":"        image_service.show.return_value \u003d None"},{"line_number":1492,"context_line":""},{"line_number":1493,"context_line":"        self.assertRaises("},{"line_number":1494,"context_line":"            exception.ImageUnacceptable,"}],"source_content_type":"text/x-python","patch_set":3,"id":"6da327d2_f01ccce5","side":"PARENT","line":1491,"updated":"2022-04-29 05:51:25.000000000","message":"Remove this test because there is no way image metadata can be None in real case, \nshow() always return dict (if not rasing any error). Please correct me if I take this wrong","commit_id":"3e068b5ce0990a576b55fea952319884c5c0ed6d"},{"author":{"_account_id":1004,"name":"Mohammed Naser","email":"mnaser@vexxhost.com","username":"mnaser"},"change_message_id":"754a6029762ec1d5c3d46bd3e5e164fa8b2dd901","unresolved":true,"context_lines":[{"line_number":1488,"context_line":"        run_as_root \u003d mock.sentinel.run_as_root"},{"line_number":1489,"context_line":""},{"line_number":1490,"context_line":"        tmp \u003d mock_temp.return_value.__enter__.return_value"},{"line_number":1491,"context_line":"        image_service.show.return_value \u003d None"},{"line_number":1492,"context_line":""},{"line_number":1493,"context_line":"        self.assertRaises("},{"line_number":1494,"context_line":"            exception.ImageUnacceptable,"}],"source_content_type":"text/x-python","patch_set":3,"id":"f9d1e115_44d16a3e","side":"PARENT","line":1491,"in_reply_to":"6da327d2_f01ccce5","updated":"2022-04-29 14:00:23.000000000","message":"Indeed, it looks like this change added it:\n\nhttps://opendev.org/openstack/cinder/commit/70a0cc921f5fbbc7a6b59b4e3c87d3f24eb15c63\n\nAlso, indeed, it looks like it *always* raises an exception, so that code is probably not really possible:\n\nhttps://opendev.org/openstack/cinder/src/branch/master/cinder/image/glance.py#L309-L322\n\nAt least, there\u0027s not really a way that I can imagine this returning None as well, so I\u0027m in agreement.","commit_id":"3e068b5ce0990a576b55fea952319884c5c0ed6d"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"5ae061ba0e9cfbb9bf1d8d5908285c9d2e94f979","unresolved":false,"context_lines":[{"line_number":1488,"context_line":"        run_as_root \u003d mock.sentinel.run_as_root"},{"line_number":1489,"context_line":""},{"line_number":1490,"context_line":"        tmp \u003d mock_temp.return_value.__enter__.return_value"},{"line_number":1491,"context_line":"        image_service.show.return_value \u003d None"},{"line_number":1492,"context_line":""},{"line_number":1493,"context_line":"        self.assertRaises("},{"line_number":1494,"context_line":"            exception.ImageUnacceptable,"}],"source_content_type":"text/x-python","patch_set":3,"id":"8756c085_95db3a23","side":"PARENT","line":1491,"in_reply_to":"f9d1e115_44d16a3e","updated":"2022-05-04 04:21:32.000000000","message":"Ack","commit_id":"3e068b5ce0990a576b55fea952319884c5c0ed6d"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"a7a1ede54fbf1fb5ff14f8f053e5e1a252afb26b","unresolved":true,"context_lines":[{"line_number":1216,"context_line":"    def test_check_allow_image_conversion_same_format(self):"},{"line_number":1217,"context_line":"        CONF.set_override(\u0027allow_image_conversion\u0027, True)"},{"line_number":1218,"context_line":"        self.assertIsNone(image_utils.check_allow_image_conversion("},{"line_number":1219,"context_line":"            \u0027foo\u0027, \u0027foo\u0027, fake.IMAGE_ID))"},{"line_number":1220,"context_line":""},{"line_number":1221,"context_line":"    def test_check_not_allow_image_conversion(self):"},{"line_number":1222,"context_line":"        CONF.set_override(\u0027allow_image_conversion\u0027, False)"}],"source_content_type":"text/x-python","patch_set":5,"id":"db8e74b1_90b1dc46","line":1219,"updated":"2022-05-03 18:09:14.000000000","message":"You could use ddt here and collapse 3 of these into 1.","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"5ae061ba0e9cfbb9bf1d8d5908285c9d2e94f979","unresolved":false,"context_lines":[{"line_number":1216,"context_line":"    def test_check_allow_image_conversion_same_format(self):"},{"line_number":1217,"context_line":"        CONF.set_override(\u0027allow_image_conversion\u0027, True)"},{"line_number":1218,"context_line":"        self.assertIsNone(image_utils.check_allow_image_conversion("},{"line_number":1219,"context_line":"            \u0027foo\u0027, \u0027foo\u0027, fake.IMAGE_ID))"},{"line_number":1220,"context_line":""},{"line_number":1221,"context_line":"    def test_check_not_allow_image_conversion(self):"},{"line_number":1222,"context_line":"        CONF.set_override(\u0027allow_image_conversion\u0027, False)"}],"source_content_type":"text/x-python","patch_set":5,"id":"a5084f00_60853e47","line":1219,"in_reply_to":"db8e74b1_90b1dc46","updated":"2022-05-04 04:21:32.000000000","message":"Done","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"48aa9723628d917ab2a2d881ba17b2ddaec62153","unresolved":true,"context_lines":[{"line_number":28,"context_line":"from cinder.tests.unit import test"},{"line_number":29,"context_line":"from cinder.volume import throttling"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"CONF \u003d cfg.CONF"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"class TestQemuImgInfo(test.TestCase):"}],"source_content_type":"text/x-python","patch_set":6,"id":"9bbc2e9f_45caa794","line":31,"updated":"2022-05-04 15:06:25.000000000","message":"Don\u0027t do this ... the cinder.tests.unit.test.TestCase has a \u0027flags\u0027 method that allows you to change config options within a specific test without having to worry about the option change affecting other tests.  See line 496.","commit_id":"883d85305ea14f2be9626f22596f61bea0771cdf"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c4c2147b31e9d40873fa1943a57347b03394578b","unresolved":false,"context_lines":[{"line_number":28,"context_line":"from cinder.tests.unit import test"},{"line_number":29,"context_line":"from cinder.volume import throttling"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"CONF \u003d cfg.CONF"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"class TestQemuImgInfo(test.TestCase):"}],"source_content_type":"text/x-python","patch_set":6,"id":"99f5cb7c_aba03234","line":31,"in_reply_to":"9bbc2e9f_45caa794","updated":"2022-05-04 17:04:17.000000000","message":"Done","commit_id":"883d85305ea14f2be9626f22596f61bea0771cdf"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"48aa9723628d917ab2a2d881ba17b2ddaec62153","unresolved":true,"context_lines":[{"line_number":493,"context_line":"            def show(self, context, image_id):"},{"line_number":494,"context_line":"                return metadata"},{"line_number":495,"context_line":""},{"line_number":496,"context_line":"        self.flags(verify_glance_signatures\u003d\u0027enabled\u0027)"},{"line_number":497,"context_line":"        mock_get.return_value \u003d BadVerifier()"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":"        self.assertRaises(exception.ImageSignatureVerificationException,"}],"source_content_type":"text/x-python","patch_set":6,"id":"3eadc599_c127ea7a","line":496,"range":{"start_line":496,"start_character":8,"end_line":496,"end_character":54},"updated":"2022-05-04 15:06:25.000000000","message":"Do this instead of importing the CONF object (it\u0027s a method of test.TestCase)","commit_id":"883d85305ea14f2be9626f22596f61bea0771cdf"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c4c2147b31e9d40873fa1943a57347b03394578b","unresolved":false,"context_lines":[{"line_number":493,"context_line":"            def show(self, context, image_id):"},{"line_number":494,"context_line":"                return metadata"},{"line_number":495,"context_line":""},{"line_number":496,"context_line":"        self.flags(verify_glance_signatures\u003d\u0027enabled\u0027)"},{"line_number":497,"context_line":"        mock_get.return_value \u003d BadVerifier()"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":"        self.assertRaises(exception.ImageSignatureVerificationException,"}],"source_content_type":"text/x-python","patch_set":6,"id":"acd6a5d3_8048caae","line":496,"range":{"start_line":496,"start_character":8,"end_line":496,"end_character":54},"in_reply_to":"3eadc599_c127ea7a","updated":"2022-05-04 17:04:17.000000000","message":"Done","commit_id":"883d85305ea14f2be9626f22596f61bea0771cdf"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"48aa9723628d917ab2a2d881ba17b2ddaec62153","unresolved":true,"context_lines":[{"line_number":743,"context_line":"        data.file_format \u003d output_format"},{"line_number":744,"context_line":"        data.backing_file \u003d None"},{"line_number":745,"context_line":"        temp_file \u003d mock_temp.return_value.__enter__.return_value"},{"line_number":746,"context_line":"        mock_conf.disable_image_conversion \u003d False"},{"line_number":747,"context_line":""},{"line_number":748,"context_line":"        output \u003d image_utils.upload_volume(ctxt, image_service, image_meta,"},{"line_number":749,"context_line":"                                           volume_path, compress\u003ddo_compress)"}],"source_content_type":"text/x-python","patch_set":6,"id":"700d31d6_4a276786","line":746,"updated":"2022-05-04 15:06:25.000000000","message":"This is the default value, so you shouldn\u0027t need to do this ... except, I guess, some joker mocked out the CONF object at line 726.\n\nI tried this locally, and if you remove line 726, fix the parameter list at line 733, and remove line 746, the test passes.  (That seems to be the case for all the changes you had to make except for test_defaults_compressed, which breaks, but not because of you.)\n\nThere are a bunch of other mocks of cinder.image.image_utils.CONF in tests you didn\u0027t have to touch.  Only reason I\u0027m mentioning that is that it may slow down refactoring.\n\nSince this isn\u0027t really your problem, and it should be a separate patch, I will put one up removing the mocked CONF object and you can rebase this patch on mine.","commit_id":"883d85305ea14f2be9626f22596f61bea0771cdf"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"97af5a81c99b01279c3173673d9898b80f495102","unresolved":false,"context_lines":[{"line_number":743,"context_line":"        data.file_format \u003d output_format"},{"line_number":744,"context_line":"        data.backing_file \u003d None"},{"line_number":745,"context_line":"        temp_file \u003d mock_temp.return_value.__enter__.return_value"},{"line_number":746,"context_line":"        mock_conf.disable_image_conversion \u003d False"},{"line_number":747,"context_line":""},{"line_number":748,"context_line":"        output \u003d image_utils.upload_volume(ctxt, image_service, image_meta,"},{"line_number":749,"context_line":"                                           volume_path, compress\u003ddo_compress)"}],"source_content_type":"text/x-python","patch_set":6,"id":"a7df1561_66a1a23e","line":746,"in_reply_to":"700d31d6_4a276786","updated":"2022-05-11 20:11:57.000000000","message":"Done","commit_id":"883d85305ea14f2be9626f22596f61bea0771cdf"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"48aa9723628d917ab2a2d881ba17b2ddaec62153","unresolved":true,"context_lines":[{"line_number":1219,"context_line":"    def test_check_image_conversion(self, conversion_opts):"},{"line_number":1220,"context_line":"        image_disk_format, volume_format, disable_image_conversion \u003d \\"},{"line_number":1221,"context_line":"            conversion_opts"},{"line_number":1222,"context_line":"        CONF.set_override(\u0027disable_image_conversion\u0027, disable_image_conversion)"},{"line_number":1223,"context_line":"        self.assertIsNone(image_utils.check_disable_image_conversion("},{"line_number":1224,"context_line":"            image_disk_format, volume_format, fake.IMAGE_ID))"},{"line_number":1225,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bb36c06b_a69e4ee0","line":1222,"updated":"2022-05-04 15:06:25.000000000","message":"use \u0027flags\u0027 for this instead (see line 496)","commit_id":"883d85305ea14f2be9626f22596f61bea0771cdf"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c4c2147b31e9d40873fa1943a57347b03394578b","unresolved":false,"context_lines":[{"line_number":1219,"context_line":"    def test_check_image_conversion(self, conversion_opts):"},{"line_number":1220,"context_line":"        image_disk_format, volume_format, disable_image_conversion \u003d \\"},{"line_number":1221,"context_line":"            conversion_opts"},{"line_number":1222,"context_line":"        CONF.set_override(\u0027disable_image_conversion\u0027, disable_image_conversion)"},{"line_number":1223,"context_line":"        self.assertIsNone(image_utils.check_disable_image_conversion("},{"line_number":1224,"context_line":"            image_disk_format, volume_format, fake.IMAGE_ID))"},{"line_number":1225,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"f681ea23_83e7d924","line":1222,"in_reply_to":"bb36c06b_a69e4ee0","updated":"2022-05-04 17:04:17.000000000","message":"Done","commit_id":"883d85305ea14f2be9626f22596f61bea0771cdf"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"48aa9723628d917ab2a2d881ba17b2ddaec62153","unresolved":true,"context_lines":[{"line_number":1224,"context_line":"            image_disk_format, volume_format, fake.IMAGE_ID))"},{"line_number":1225,"context_line":""},{"line_number":1226,"context_line":"    def test_check_disable_image_conversion(self):"},{"line_number":1227,"context_line":"        CONF.set_override(\u0027disable_image_conversion\u0027, True)"},{"line_number":1228,"context_line":"        self.assertRaises("},{"line_number":1229,"context_line":"            exception.ImageUnacceptable,"},{"line_number":1230,"context_line":"            image_utils.check_disable_image_conversion,"}],"source_content_type":"text/x-python","patch_set":6,"id":"cbdf3111_d92ad4f9","line":1227,"updated":"2022-05-04 15:06:25.000000000","message":"use \u0027flags\u0027 for this instead (see line 496)","commit_id":"883d85305ea14f2be9626f22596f61bea0771cdf"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"c4c2147b31e9d40873fa1943a57347b03394578b","unresolved":false,"context_lines":[{"line_number":1224,"context_line":"            image_disk_format, volume_format, fake.IMAGE_ID))"},{"line_number":1225,"context_line":""},{"line_number":1226,"context_line":"    def test_check_disable_image_conversion(self):"},{"line_number":1227,"context_line":"        CONF.set_override(\u0027disable_image_conversion\u0027, True)"},{"line_number":1228,"context_line":"        self.assertRaises("},{"line_number":1229,"context_line":"            exception.ImageUnacceptable,"},{"line_number":1230,"context_line":"            image_utils.check_disable_image_conversion,"}],"source_content_type":"text/x-python","patch_set":6,"id":"d7191b6f_12a1ccaf","line":1227,"in_reply_to":"cbdf3111_d92ad4f9","updated":"2022-05-04 17:04:17.000000000","message":"Done","commit_id":"883d85305ea14f2be9626f22596f61bea0771cdf"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"dcc2ca60fa409955960333c94e71c52826d0f989","unresolved":true,"context_lines":[{"line_number":1439,"context_line":"        self.assertFalse(mock_copy.called)"},{"line_number":1440,"context_line":"        self.assertFalse(mock_convert.called)"},{"line_number":1441,"context_line":""},{"line_number":1442,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.convert_image\u0027)"},{"line_number":1443,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.volume_utils.copy_volume\u0027)"},{"line_number":1444,"context_line":"    @mock.patch("},{"line_number":1445,"context_line":"        \u0027cinder.image.image_utils.replace_xenserver_image_with_coalesced_vhd\u0027)"},{"line_number":1446,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.is_xenserver_format\u0027,"},{"line_number":1447,"context_line":"                return_value\u003dFalse)"},{"line_number":1448,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.fetch\u0027)"},{"line_number":1449,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.qemu_img_info\u0027,"},{"line_number":1450,"context_line":"                side_effect\u003dprocessutils.ProcessExecutionError)"},{"line_number":1451,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.temporary_file\u0027)"},{"line_number":1452,"context_line":"    def test_no_qemu_img_no_metadata(self, mock_temp, mock_info,"},{"line_number":1453,"context_line":"                                     mock_fetch, mock_is_xen, mock_repl_xen,"},{"line_number":1454,"context_line":"                                     mock_copy, mock_convert):"},{"line_number":1455,"context_line":"        ctxt \u003d mock.sentinel.context"},{"line_number":1456,"context_line":"        image_service \u003d mock.Mock(temp_images\u003dNone)"},{"line_number":1457,"context_line":"        image_id \u003d mock.sentinel.image_id"},{"line_number":1458,"context_line":"        dest \u003d mock.sentinel.dest"},{"line_number":1459,"context_line":"        volume_format \u003d mock.sentinel.volume_format"},{"line_number":1460,"context_line":"        blocksize \u003d mock.sentinel.blocksize"},{"line_number":1461,"context_line":"        ctxt.user_id \u003d user_id \u003d mock.sentinel.user_id"},{"line_number":1462,"context_line":"        project_id \u003d mock.sentinel.project_id"},{"line_number":1463,"context_line":"        size \u003d 4321"},{"line_number":1464,"context_line":"        run_as_root \u003d mock.sentinel.run_as_root"},{"line_number":1465,"context_line":""},{"line_number":1466,"context_line":"        tmp \u003d mock_temp.return_value.__enter__.return_value"},{"line_number":1467,"context_line":"        image_service.show.return_value \u003d None"},{"line_number":1468,"context_line":""},{"line_number":1469,"context_line":"        self.assertRaises("},{"line_number":1470,"context_line":"            exception.ImageUnacceptable,"},{"line_number":1471,"context_line":"            image_utils.fetch_to_volume_format,"},{"line_number":1472,"context_line":"            ctxt, image_service, image_id, dest, volume_format, blocksize,"},{"line_number":1473,"context_line":"            user_id\u003duser_id, project_id\u003dproject_id, size\u003dsize,"},{"line_number":1474,"context_line":"            run_as_root\u003drun_as_root)"},{"line_number":1475,"context_line":""},{"line_number":1476,"context_line":"        image_service.show.assert_called_once_with(ctxt, image_id)"},{"line_number":1477,"context_line":"        mock_temp.assert_called_once_with(prefix\u003d\u0027image_download_%s_\u0027 %"},{"line_number":1478,"context_line":"                                          image_id)"},{"line_number":1479,"context_line":"        mock_info.assert_called_once_with(tmp,"},{"line_number":1480,"context_line":"                                          force_share\u003dFalse,"},{"line_number":1481,"context_line":"                                          run_as_root\u003drun_as_root)"},{"line_number":1482,"context_line":"        self.assertFalse(mock_fetch.called)"},{"line_number":1483,"context_line":"        self.assertFalse(mock_repl_xen.called)"},{"line_number":1484,"context_line":"        self.assertFalse(mock_copy.called)"},{"line_number":1485,"context_line":"        self.assertFalse(mock_convert.called)"},{"line_number":1486,"context_line":""},{"line_number":1487,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.check_virtual_size\u0027)"},{"line_number":1488,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.convert_image\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"133dbad1_ef30709e","side":"PARENT","line":1485,"range":{"start_line":1442,"start_character":0,"end_line":1485,"end_character":45},"updated":"2022-05-27 09:52:38.000000000","message":"what is the reason for removing this test? IIUC, this is testing the case when ``qemu-img info`` command fails which occurs here[1] and has no conflict with the new config option with the default value i.e. False.\n\n[1] https://opendev.org/openstack/cinder/src/branch/master/cinder/image/image_utils.py#L749","commit_id":"c8b6a44f710ab4884828d3d11bbf7a0b9c0bdbcf"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"52e4596a49165d960cbe9eebc886b97d3807ec52","unresolved":false,"context_lines":[{"line_number":1439,"context_line":"        self.assertFalse(mock_copy.called)"},{"line_number":1440,"context_line":"        self.assertFalse(mock_convert.called)"},{"line_number":1441,"context_line":""},{"line_number":1442,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.convert_image\u0027)"},{"line_number":1443,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.volume_utils.copy_volume\u0027)"},{"line_number":1444,"context_line":"    @mock.patch("},{"line_number":1445,"context_line":"        \u0027cinder.image.image_utils.replace_xenserver_image_with_coalesced_vhd\u0027)"},{"line_number":1446,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.is_xenserver_format\u0027,"},{"line_number":1447,"context_line":"                return_value\u003dFalse)"},{"line_number":1448,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.fetch\u0027)"},{"line_number":1449,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.qemu_img_info\u0027,"},{"line_number":1450,"context_line":"                side_effect\u003dprocessutils.ProcessExecutionError)"},{"line_number":1451,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.temporary_file\u0027)"},{"line_number":1452,"context_line":"    def test_no_qemu_img_no_metadata(self, mock_temp, mock_info,"},{"line_number":1453,"context_line":"                                     mock_fetch, mock_is_xen, mock_repl_xen,"},{"line_number":1454,"context_line":"                                     mock_copy, mock_convert):"},{"line_number":1455,"context_line":"        ctxt \u003d mock.sentinel.context"},{"line_number":1456,"context_line":"        image_service \u003d mock.Mock(temp_images\u003dNone)"},{"line_number":1457,"context_line":"        image_id \u003d mock.sentinel.image_id"},{"line_number":1458,"context_line":"        dest \u003d mock.sentinel.dest"},{"line_number":1459,"context_line":"        volume_format \u003d mock.sentinel.volume_format"},{"line_number":1460,"context_line":"        blocksize \u003d mock.sentinel.blocksize"},{"line_number":1461,"context_line":"        ctxt.user_id \u003d user_id \u003d mock.sentinel.user_id"},{"line_number":1462,"context_line":"        project_id \u003d mock.sentinel.project_id"},{"line_number":1463,"context_line":"        size \u003d 4321"},{"line_number":1464,"context_line":"        run_as_root \u003d mock.sentinel.run_as_root"},{"line_number":1465,"context_line":""},{"line_number":1466,"context_line":"        tmp \u003d mock_temp.return_value.__enter__.return_value"},{"line_number":1467,"context_line":"        image_service.show.return_value \u003d None"},{"line_number":1468,"context_line":""},{"line_number":1469,"context_line":"        self.assertRaises("},{"line_number":1470,"context_line":"            exception.ImageUnacceptable,"},{"line_number":1471,"context_line":"            image_utils.fetch_to_volume_format,"},{"line_number":1472,"context_line":"            ctxt, image_service, image_id, dest, volume_format, blocksize,"},{"line_number":1473,"context_line":"            user_id\u003duser_id, project_id\u003dproject_id, size\u003dsize,"},{"line_number":1474,"context_line":"            run_as_root\u003drun_as_root)"},{"line_number":1475,"context_line":""},{"line_number":1476,"context_line":"        image_service.show.assert_called_once_with(ctxt, image_id)"},{"line_number":1477,"context_line":"        mock_temp.assert_called_once_with(prefix\u003d\u0027image_download_%s_\u0027 %"},{"line_number":1478,"context_line":"                                          image_id)"},{"line_number":1479,"context_line":"        mock_info.assert_called_once_with(tmp,"},{"line_number":1480,"context_line":"                                          force_share\u003dFalse,"},{"line_number":1481,"context_line":"                                          run_as_root\u003drun_as_root)"},{"line_number":1482,"context_line":"        self.assertFalse(mock_fetch.called)"},{"line_number":1483,"context_line":"        self.assertFalse(mock_repl_xen.called)"},{"line_number":1484,"context_line":"        self.assertFalse(mock_copy.called)"},{"line_number":1485,"context_line":"        self.assertFalse(mock_convert.called)"},{"line_number":1486,"context_line":""},{"line_number":1487,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.check_virtual_size\u0027)"},{"line_number":1488,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.convert_image\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"65729b20_1dae1980","side":"PARENT","line":1485,"range":{"start_line":1442,"start_character":0,"end_line":1485,"end_character":45},"in_reply_to":"133dbad1_ef30709e","updated":"2022-05-27 18:42:26.000000000","message":"Remove this test because there is no way image metadata can be None in real case, show() always return dict (if not rasing any error)\nSome other information can be found here https://review.opendev.org/c/openstack/cinder/+/839793/3/cinder/tests/unit/test_image_utils.py#b1491","commit_id":"c8b6a44f710ab4884828d3d11bbf7a0b9c0bdbcf"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"97109738229acf3b2da1f85e74aa57be1702438d","unresolved":false,"context_lines":[{"line_number":1439,"context_line":"        self.assertFalse(mock_copy.called)"},{"line_number":1440,"context_line":"        self.assertFalse(mock_convert.called)"},{"line_number":1441,"context_line":""},{"line_number":1442,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.convert_image\u0027)"},{"line_number":1443,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.volume_utils.copy_volume\u0027)"},{"line_number":1444,"context_line":"    @mock.patch("},{"line_number":1445,"context_line":"        \u0027cinder.image.image_utils.replace_xenserver_image_with_coalesced_vhd\u0027)"},{"line_number":1446,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.is_xenserver_format\u0027,"},{"line_number":1447,"context_line":"                return_value\u003dFalse)"},{"line_number":1448,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.fetch\u0027)"},{"line_number":1449,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.qemu_img_info\u0027,"},{"line_number":1450,"context_line":"                side_effect\u003dprocessutils.ProcessExecutionError)"},{"line_number":1451,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.temporary_file\u0027)"},{"line_number":1452,"context_line":"    def test_no_qemu_img_no_metadata(self, mock_temp, mock_info,"},{"line_number":1453,"context_line":"                                     mock_fetch, mock_is_xen, mock_repl_xen,"},{"line_number":1454,"context_line":"                                     mock_copy, mock_convert):"},{"line_number":1455,"context_line":"        ctxt \u003d mock.sentinel.context"},{"line_number":1456,"context_line":"        image_service \u003d mock.Mock(temp_images\u003dNone)"},{"line_number":1457,"context_line":"        image_id \u003d mock.sentinel.image_id"},{"line_number":1458,"context_line":"        dest \u003d mock.sentinel.dest"},{"line_number":1459,"context_line":"        volume_format \u003d mock.sentinel.volume_format"},{"line_number":1460,"context_line":"        blocksize \u003d mock.sentinel.blocksize"},{"line_number":1461,"context_line":"        ctxt.user_id \u003d user_id \u003d mock.sentinel.user_id"},{"line_number":1462,"context_line":"        project_id \u003d mock.sentinel.project_id"},{"line_number":1463,"context_line":"        size \u003d 4321"},{"line_number":1464,"context_line":"        run_as_root \u003d mock.sentinel.run_as_root"},{"line_number":1465,"context_line":""},{"line_number":1466,"context_line":"        tmp \u003d mock_temp.return_value.__enter__.return_value"},{"line_number":1467,"context_line":"        image_service.show.return_value \u003d None"},{"line_number":1468,"context_line":""},{"line_number":1469,"context_line":"        self.assertRaises("},{"line_number":1470,"context_line":"            exception.ImageUnacceptable,"},{"line_number":1471,"context_line":"            image_utils.fetch_to_volume_format,"},{"line_number":1472,"context_line":"            ctxt, image_service, image_id, dest, volume_format, blocksize,"},{"line_number":1473,"context_line":"            user_id\u003duser_id, project_id\u003dproject_id, size\u003dsize,"},{"line_number":1474,"context_line":"            run_as_root\u003drun_as_root)"},{"line_number":1475,"context_line":""},{"line_number":1476,"context_line":"        image_service.show.assert_called_once_with(ctxt, image_id)"},{"line_number":1477,"context_line":"        mock_temp.assert_called_once_with(prefix\u003d\u0027image_download_%s_\u0027 %"},{"line_number":1478,"context_line":"                                          image_id)"},{"line_number":1479,"context_line":"        mock_info.assert_called_once_with(tmp,"},{"line_number":1480,"context_line":"                                          force_share\u003dFalse,"},{"line_number":1481,"context_line":"                                          run_as_root\u003drun_as_root)"},{"line_number":1482,"context_line":"        self.assertFalse(mock_fetch.called)"},{"line_number":1483,"context_line":"        self.assertFalse(mock_repl_xen.called)"},{"line_number":1484,"context_line":"        self.assertFalse(mock_copy.called)"},{"line_number":1485,"context_line":"        self.assertFalse(mock_convert.called)"},{"line_number":1486,"context_line":""},{"line_number":1487,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.check_virtual_size\u0027)"},{"line_number":1488,"context_line":"    @mock.patch(\u0027cinder.image.image_utils.convert_image\u0027)"}],"source_content_type":"text/x-python","patch_set":11,"id":"dc949f83_5bf6440e","side":"PARENT","line":1485,"range":{"start_line":1442,"start_character":0,"end_line":1485,"end_character":45},"in_reply_to":"65729b20_1dae1980","updated":"2022-05-31 18:16:31.000000000","message":"I think there is still value in modifying this test to remove the metadata part, as i agree it\u0027s not a trivial case for metadata to be None but if qemu-img command fails, which also is tested by this test. But let\u0027s leave it for a followup","commit_id":"c8b6a44f710ab4884828d3d11bbf7a0b9c0bdbcf"}],"cinder/tests/unit/volume/flows/test_create_volume_flow.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d3e15eb9f4d46fa5a2dd293c1a060bf296566293","unresolved":true,"context_lines":[{"line_number":2354,"context_line":"        image_volume \u003d fake_volume.fake_db_volume(size\u003d2)"},{"line_number":2355,"context_line":"        self.mock_db.volume_create.return_value \u003d image_volume"},{"line_number":2356,"context_line":"        image_id \u003d fakes.IMAGE_ID"},{"line_number":2357,"context_line":"        if image_conversion_disable:"},{"line_number":2358,"context_line":"            msg \u003d (\"Image conversion is disabled. The volume type you have \""},{"line_number":2359,"context_line":"                   \"requested requires that the image it is being created \""},{"line_number":2360,"context_line":"                   \"from be in \u0027%(volume_format)s\u0027 format, but the image \""}],"source_content_type":"text/x-python","patch_set":14,"id":"c24a3de9_b4121fd8","line":2357,"updated":"2022-05-31 15:15:29.000000000","message":"nit: might be worth adding a comment here saying something like:\n\n  # we\u0027re checking the exact message in this test because other code in this commit\n  # relies upon parsing it to determine the content of a user message\n\n(that way someone won\u0027t \"improve\" this test by mistake, and if the test breaks, a quick \u0027git blame\u0027 will find the commit and they can see what needs to be fixed)","commit_id":"41104531bace557cab2c92a40f3aef1ae66ffe49"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"d7f58868710d920098ce2482380ac809a8f760de","unresolved":false,"context_lines":[{"line_number":2354,"context_line":"        image_volume \u003d fake_volume.fake_db_volume(size\u003d2)"},{"line_number":2355,"context_line":"        self.mock_db.volume_create.return_value \u003d image_volume"},{"line_number":2356,"context_line":"        image_id \u003d fakes.IMAGE_ID"},{"line_number":2357,"context_line":"        if image_conversion_disable:"},{"line_number":2358,"context_line":"            msg \u003d (\"Image conversion is disabled. The volume type you have \""},{"line_number":2359,"context_line":"                   \"requested requires that the image it is being created \""},{"line_number":2360,"context_line":"                   \"from be in \u0027%(volume_format)s\u0027 format, but the image \""}],"source_content_type":"text/x-python","patch_set":14,"id":"f8046f7c_8879366d","line":2357,"in_reply_to":"c24a3de9_b4121fd8","updated":"2022-05-31 19:59:43.000000000","message":"Done","commit_id":"41104531bace557cab2c92a40f3aef1ae66ffe49"}],"cinder/tests/unit/volume/test_volume_reimage.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d3e15eb9f4d46fa5a2dd293c1a060bf296566293","unresolved":true,"context_lines":[{"line_number":60,"context_line":"                              self.context, volume, self.image_meta)"},{"line_number":61,"context_line":"            self.assertEqual(volume.previous_status, \u0027available\u0027)"},{"line_number":62,"context_line":"            self.assertEqual(volume.status, \u0027error\u0027)"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"            msg \u003d (\"Image conversion is disabled. The volume type you have \""},{"line_number":65,"context_line":"                   \"requested requires that the image it is being created \""},{"line_number":66,"context_line":"                   \"from be in \u0027%(volume_format)s\u0027 format, but the image \""}],"source_content_type":"text/x-python","patch_set":14,"id":"4fdd3308_f70d7c28","line":63,"updated":"2022-05-31 15:15:29.000000000","message":"nit: same comment as in test_create_volume_flow.py","commit_id":"41104531bace557cab2c92a40f3aef1ae66ffe49"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"d7f58868710d920098ce2482380ac809a8f760de","unresolved":false,"context_lines":[{"line_number":60,"context_line":"                              self.context, volume, self.image_meta)"},{"line_number":61,"context_line":"            self.assertEqual(volume.previous_status, \u0027available\u0027)"},{"line_number":62,"context_line":"            self.assertEqual(volume.status, \u0027error\u0027)"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"            msg \u003d (\"Image conversion is disabled. The volume type you have \""},{"line_number":65,"context_line":"                   \"requested requires that the image it is being created \""},{"line_number":66,"context_line":"                   \"from be in \u0027%(volume_format)s\u0027 format, but the image \""}],"source_content_type":"text/x-python","patch_set":14,"id":"07889532_3dae7ec0","line":63,"in_reply_to":"4fdd3308_f70d7c28","updated":"2022-05-31 19:59:43.000000000","message":"Done","commit_id":"41104531bace557cab2c92a40f3aef1ae66ffe49"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"97109738229acf3b2da1f85e74aa57be1702438d","unresolved":true,"context_lines":[{"line_number":61,"context_line":"            self.assertEqual(volume.previous_status, \u0027available\u0027)"},{"line_number":62,"context_line":"            self.assertEqual(volume.status, \u0027error\u0027)"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"            mock_cp_img.side_effect \u003d exception.ImageConversionNotAllowed("},{"line_number":65,"context_line":"                image_id\u003dself.image_meta[\u0027id\u0027], reason\u003d\u0027\u0027)"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"            with mock.patch.object("},{"line_number":68,"context_line":"                self.volume.message_api, \u0027create\u0027"},{"line_number":69,"context_line":"            ) as mock_msg_create:"},{"line_number":70,"context_line":"                self.assertRaises("},{"line_number":71,"context_line":"                    exception.ImageConversionNotAllowed, self.volume.reimage,"},{"line_number":72,"context_line":"                    self.context, volume, self.image_meta)"},{"line_number":73,"context_line":"                mock_msg_create.assert_called_with("},{"line_number":74,"context_line":"                    self.context,"},{"line_number":75,"context_line":"                    message_field.Action.REIMAGE_VOLUME,"},{"line_number":76,"context_line":"                    resource_uuid\u003dvolume.id,"},{"line_number":77,"context_line":"                    detail\u003dmessage_field.Detail.IMAGE_FORMAT_UNACCEPTABLE)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"            mock_cp_img.side_effect \u003d exception.ImageTooBig("},{"line_number":80,"context_line":"                image_id\u003dself.image_meta[\u0027id\u0027], reason\u003d\u0027\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"ce71e98a_714d115f","line":77,"range":{"start_line":64,"start_character":0,"end_line":77,"end_character":74},"updated":"2022-05-31 18:16:31.000000000","message":"why are we modifying this existing test and not creating another test for this case? by default image conversion is allowed so this test should not fail with ImageConversionNotAllowed exception. I think we need a new test here and should leave this one as it is.","commit_id":"3af7e3926cef7e4c810544c6ec19df1577b6b6f3"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"d7f58868710d920098ce2482380ac809a8f760de","unresolved":false,"context_lines":[{"line_number":61,"context_line":"            self.assertEqual(volume.previous_status, \u0027available\u0027)"},{"line_number":62,"context_line":"            self.assertEqual(volume.status, \u0027error\u0027)"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"            mock_cp_img.side_effect \u003d exception.ImageConversionNotAllowed("},{"line_number":65,"context_line":"                image_id\u003dself.image_meta[\u0027id\u0027], reason\u003d\u0027\u0027)"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"            with mock.patch.object("},{"line_number":68,"context_line":"                self.volume.message_api, \u0027create\u0027"},{"line_number":69,"context_line":"            ) as mock_msg_create:"},{"line_number":70,"context_line":"                self.assertRaises("},{"line_number":71,"context_line":"                    exception.ImageConversionNotAllowed, self.volume.reimage,"},{"line_number":72,"context_line":"                    self.context, volume, self.image_meta)"},{"line_number":73,"context_line":"                mock_msg_create.assert_called_with("},{"line_number":74,"context_line":"                    self.context,"},{"line_number":75,"context_line":"                    message_field.Action.REIMAGE_VOLUME,"},{"line_number":76,"context_line":"                    resource_uuid\u003dvolume.id,"},{"line_number":77,"context_line":"                    detail\u003dmessage_field.Detail.IMAGE_FORMAT_UNACCEPTABLE)"},{"line_number":78,"context_line":""},{"line_number":79,"context_line":"            mock_cp_img.side_effect \u003d exception.ImageTooBig("},{"line_number":80,"context_line":"                image_id\u003dself.image_meta[\u0027id\u0027], reason\u003d\u0027\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"1cf7064c_bc76a7cb","line":77,"range":{"start_line":64,"start_character":0,"end_line":77,"end_character":74},"in_reply_to":"ce71e98a_714d115f","updated":"2022-05-31 19:59:43.000000000","message":"right, will change it back","commit_id":"3af7e3926cef7e4c810544c6ec19df1577b6b6f3"}],"cinder/volume/flows/manager/create_volume.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"97af5a81c99b01279c3173673d9898b80f495102","unresolved":true,"context_lines":[{"line_number":795,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":796,"context_line":"                LOG.exception(\"Failed to copy image to volume \""},{"line_number":797,"context_line":"                              \"%(volume_id)s due to insufficient space\","},{"line_number":798,"context_line":"                              {\u0027volume_id\u0027: volume.id})"},{"line_number":799,"context_line":"        return model_update"},{"line_number":800,"context_line":""},{"line_number":801,"context_line":"    def _create_from_image_cache("}],"source_content_type":"text/x-python","patch_set":9,"id":"7369f6cc_6d9dd6af","line":798,"updated":"2022-05-11 20:11:57.000000000","message":"Presumably an operator would know that they\u0027ve set image_conversion_disable to True, so I don\u0027t think there\u0027s a reason to log ImageUnacceptable here (but this is where you\u0027d do it if you disagree).","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"5bc05fac006693ef8d9ae8e4861792361e56a3b2","unresolved":false,"context_lines":[{"line_number":795,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":796,"context_line":"                LOG.exception(\"Failed to copy image to volume \""},{"line_number":797,"context_line":"                              \"%(volume_id)s due to insufficient space\","},{"line_number":798,"context_line":"                              {\u0027volume_id\u0027: volume.id})"},{"line_number":799,"context_line":"        return model_update"},{"line_number":800,"context_line":""},{"line_number":801,"context_line":"    def _create_from_image_cache("}],"source_content_type":"text/x-python","patch_set":9,"id":"2979b890_33966912","line":798,"in_reply_to":"7369f6cc_6d9dd6af","updated":"2022-05-25 16:55:58.000000000","message":"Addressed","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"97af5a81c99b01279c3173673d9898b80f495102","unresolved":true,"context_lines":[{"line_number":989,"context_line":"                            if virtual_size and virtual_size !\u003d original_size:"},{"line_number":990,"context_line":"                                volume.size \u003d virtual_size"},{"line_number":991,"context_line":"                                volume.save()"},{"line_number":992,"context_line":"                        model_update \u003d self._create_from_image_download("},{"line_number":993,"context_line":"                            context,"},{"line_number":994,"context_line":"                            volume,"},{"line_number":995,"context_line":"                            image_location,"}],"source_content_type":"text/x-python","patch_set":9,"id":"6d6b1135_68da974a","line":992,"range":{"start_line":992,"start_character":44,"end_line":992,"end_character":71},"updated":"2022-05-11 20:11:57.000000000","message":"this is the call that will eventually encounter your check in image_utils","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"d7f58868710d920098ce2482380ac809a8f760de","unresolved":false,"context_lines":[{"line_number":989,"context_line":"                            if virtual_size and virtual_size !\u003d original_size:"},{"line_number":990,"context_line":"                                volume.size \u003d virtual_size"},{"line_number":991,"context_line":"                                volume.save()"},{"line_number":992,"context_line":"                        model_update \u003d self._create_from_image_download("},{"line_number":993,"context_line":"                            context,"},{"line_number":994,"context_line":"                            volume,"},{"line_number":995,"context_line":"                            image_location,"}],"source_content_type":"text/x-python","patch_set":9,"id":"4aaebae8_4d716f39","line":992,"range":{"start_line":992,"start_character":44,"end_line":992,"end_character":71},"in_reply_to":"6d6b1135_68da974a","updated":"2022-05-31 19:59:43.000000000","message":"Done","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"97af5a81c99b01279c3173673d9898b80f495102","unresolved":true,"context_lines":[{"line_number":1013,"context_line":"                            resource_uuid\u003dvolume.id,"},{"line_number":1014,"context_line":"                            detail\u003d"},{"line_number":1015,"context_line":"                            message_field.Detail.SIGNATURE_VERIFICATION_FAILED,"},{"line_number":1016,"context_line":"                            exception\u003derr)"},{"line_number":1017,"context_line":""},{"line_number":1018,"context_line":"            if should_create_cache_entry:"},{"line_number":1019,"context_line":"                # Update the newly created volume db entry before we clone it"}],"source_content_type":"text/x-python","patch_set":9,"id":"678ddb90_50a7114c","line":1016,"updated":"2022-05-11 20:11:57.000000000","message":"I think you should add an except block for ImageUnacceptable and create a message.  I don\u0027t think there\u0027s a detail field already defined for what you want; info about user messages is here: https://docs.openstack.org/cinder/latest/contributor/user_messages.html\n\nUnfortunately, the user messages API doesn\u0027t allow passing in the disk format and volume format values from the informative message in the exception, so you\u0027ll just have to make up a generic one that\u0027s clear enough.","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"160e757c9e9acd9c3e2dee7d4e6483d3eb01e378","unresolved":false,"context_lines":[{"line_number":1013,"context_line":"                            resource_uuid\u003dvolume.id,"},{"line_number":1014,"context_line":"                            detail\u003d"},{"line_number":1015,"context_line":"                            message_field.Detail.SIGNATURE_VERIFICATION_FAILED,"},{"line_number":1016,"context_line":"                            exception\u003derr)"},{"line_number":1017,"context_line":""},{"line_number":1018,"context_line":"            if should_create_cache_entry:"},{"line_number":1019,"context_line":"                # Update the newly created volume db entry before we clone it"}],"source_content_type":"text/x-python","patch_set":9,"id":"d0184ab6_3af3875e","line":1016,"in_reply_to":"678ddb90_50a7114c","updated":"2022-05-12 16:47:05.000000000","message":"Done","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d092bbf743d3e3d0d6b6e8dad202a37c6ca3143d","unresolved":true,"context_lines":[{"line_number":795,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":796,"context_line":"                LOG.exception(\"Failed to copy image to volume \""},{"line_number":797,"context_line":"                              \"%(volume_id)s due to insufficient space\","},{"line_number":798,"context_line":"                              {\u0027volume_id\u0027: volume.id})"},{"line_number":799,"context_line":"        return model_update"},{"line_number":800,"context_line":""},{"line_number":801,"context_line":"    def _create_from_image_cache("}],"source_content_type":"text/x-python","patch_set":10,"id":"a05e321e_c0963e87","line":798,"updated":"2022-05-12 18:35:23.000000000","message":"I\u0027m rethinking my suggestion not to log here, because we have that nice error message that has the disk_format and volume format mentioned in it, and that (a) won\u0027t make it into the user message, and (b) could be helpful to support people (maybe?).  I\u0027d suggest not logging it as an exception but just log the message from the exception at INFO level.  But I don\u0027t insist; do whatever you think would be the right thing.","commit_id":"893543dad6e64d1638dba28f705ef9510dfebc1c"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"d7f58868710d920098ce2482380ac809a8f760de","unresolved":false,"context_lines":[{"line_number":795,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":796,"context_line":"                LOG.exception(\"Failed to copy image to volume \""},{"line_number":797,"context_line":"                              \"%(volume_id)s due to insufficient space\","},{"line_number":798,"context_line":"                              {\u0027volume_id\u0027: volume.id})"},{"line_number":799,"context_line":"        return model_update"},{"line_number":800,"context_line":""},{"line_number":801,"context_line":"    def _create_from_image_cache("}],"source_content_type":"text/x-python","patch_set":10,"id":"90fe61e3_fc29f32c","line":798,"in_reply_to":"418084a8_7284771d","updated":"2022-05-31 19:59:43.000000000","message":"Ack","commit_id":"893543dad6e64d1638dba28f705ef9510dfebc1c"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"dfe50569fcba8a3b006190d996e7c4ef07b8be80","unresolved":true,"context_lines":[{"line_number":795,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":796,"context_line":"                LOG.exception(\"Failed to copy image to volume \""},{"line_number":797,"context_line":"                              \"%(volume_id)s due to insufficient space\","},{"line_number":798,"context_line":"                              {\u0027volume_id\u0027: volume.id})"},{"line_number":799,"context_line":"        return model_update"},{"line_number":800,"context_line":""},{"line_number":801,"context_line":"    def _create_from_image_cache("}],"source_content_type":"text/x-python","patch_set":10,"id":"dd1179b1_68cb9f4b","line":798,"in_reply_to":"a05e321e_c0963e87","updated":"2022-05-14 14:59:19.000000000","message":"IMO, prefer to keep it out of this patch (maybe a followup?:)), as it\u0027s not necessary and it changes the ImageTooBig exception behavior even without enabling the config `image_conversion_disable`.","commit_id":"893543dad6e64d1638dba28f705ef9510dfebc1c"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"5bc05fac006693ef8d9ae8e4861792361e56a3b2","unresolved":false,"context_lines":[{"line_number":795,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":796,"context_line":"                LOG.exception(\"Failed to copy image to volume \""},{"line_number":797,"context_line":"                              \"%(volume_id)s due to insufficient space\","},{"line_number":798,"context_line":"                              {\u0027volume_id\u0027: volume.id})"},{"line_number":799,"context_line":"        return model_update"},{"line_number":800,"context_line":""},{"line_number":801,"context_line":"    def _create_from_image_cache("}],"source_content_type":"text/x-python","patch_set":10,"id":"96852465_c3b4bd2c","line":798,"in_reply_to":"dd1179b1_68cb9f4b","updated":"2022-05-25 16:55:58.000000000","message":"Addressed","commit_id":"893543dad6e64d1638dba28f705ef9510dfebc1c"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"faa1832328fe8bfb7790e675af0ef1bc8419754d","unresolved":true,"context_lines":[{"line_number":795,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":796,"context_line":"                LOG.exception(\"Failed to copy image to volume \""},{"line_number":797,"context_line":"                              \"%(volume_id)s due to insufficient space\","},{"line_number":798,"context_line":"                              {\u0027volume_id\u0027: volume.id})"},{"line_number":799,"context_line":"        return model_update"},{"line_number":800,"context_line":""},{"line_number":801,"context_line":"    def _create_from_image_cache("}],"source_content_type":"text/x-python","patch_set":10,"id":"418084a8_7284771d","line":798,"in_reply_to":"dd1179b1_68cb9f4b","updated":"2022-05-26 20:46:53.000000000","message":"Sorry, I wasn\u0027t clear.  What I meant was to add another except block similar to this one, but catching exception.ImageUnacceptable, with save_and_reraise, and logging the message from the exception at info level.  But it can be a followup if it looks like it would be useful, it\u0027s not necessary for this patch.","commit_id":"893543dad6e64d1638dba28f705ef9510dfebc1c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"dcc2ca60fa409955960333c94e71c52826d0f989","unresolved":true,"context_lines":[{"line_number":1014,"context_line":"                            detail\u003d"},{"line_number":1015,"context_line":"                            message_field.Detail.SIGNATURE_VERIFICATION_FAILED,"},{"line_number":1016,"context_line":"                            exception\u003derr)"},{"line_number":1017,"context_line":"                except exception.ImageUnacceptable as err:"},{"line_number":1018,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":1019,"context_line":"                        self.message.create("},{"line_number":1020,"context_line":"                            context,"},{"line_number":1021,"context_line":"                            message_field.Action.COPY_IMAGE_TO_VOLUME,"},{"line_number":1022,"context_line":"                            resource_uuid\u003dvolume.id,"},{"line_number":1023,"context_line":"                            detail\u003d"},{"line_number":1024,"context_line":"                            message_field.Detail.IMAGE_FORMAT_UNACCEPTABLE,"},{"line_number":1025,"context_line":"                            exception\u003derr)"},{"line_number":1026,"context_line":""},{"line_number":1027,"context_line":"            if should_create_cache_entry:"},{"line_number":1028,"context_line":"                # Update the newly created volume db entry before we clone it"}],"source_content_type":"text/x-python","patch_set":11,"id":"7a9b6843_b78d883e","line":1025,"range":{"start_line":1017,"start_character":0,"end_line":1025,"end_character":42},"updated":"2022-05-27 09:52:38.000000000","message":"This is not correct. An ImageUnacceptable exception could occur because of many reasons as follows:\n\n1) When CONF.allow_compression_on_image_upload is False but container_format provided is \u0027compressed\u0027\n2) qemu-img command failed\n3) Wrong compression format provided etc\n\nWe should not misguide the user into thinking it\u0027s because of the format of image and volume. Also we should not assume that the config option is always set to True, we should add a check for that as well before creating specific messages like this.","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"52e4596a49165d960cbe9eebc886b97d3807ec52","unresolved":false,"context_lines":[{"line_number":1014,"context_line":"                            detail\u003d"},{"line_number":1015,"context_line":"                            message_field.Detail.SIGNATURE_VERIFICATION_FAILED,"},{"line_number":1016,"context_line":"                            exception\u003derr)"},{"line_number":1017,"context_line":"                except exception.ImageUnacceptable as err:"},{"line_number":1018,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":1019,"context_line":"                        self.message.create("},{"line_number":1020,"context_line":"                            context,"},{"line_number":1021,"context_line":"                            message_field.Action.COPY_IMAGE_TO_VOLUME,"},{"line_number":1022,"context_line":"                            resource_uuid\u003dvolume.id,"},{"line_number":1023,"context_line":"                            detail\u003d"},{"line_number":1024,"context_line":"                            message_field.Detail.IMAGE_FORMAT_UNACCEPTABLE,"},{"line_number":1025,"context_line":"                            exception\u003derr)"},{"line_number":1026,"context_line":""},{"line_number":1027,"context_line":"            if should_create_cache_entry:"},{"line_number":1028,"context_line":"                # Update the newly created volume db entry before we clone it"}],"source_content_type":"text/x-python","patch_set":11,"id":"f1e45403_ec439480","line":1025,"range":{"start_line":1017,"start_character":0,"end_line":1025,"end_character":42},"in_reply_to":"7a9b6843_b78d883e","updated":"2022-05-27 18:42:26.000000000","message":"How about we use error messages to check if it is the ImageUnacceptable error we\u0027re looking for. Because we might also face that other ImageUnacceptable jump out first before image_conversion_disable check. Make sure we don\u0027t rise here if it\u0027s other errors happen but also satisfy both  `CONF.image_conversion_disable` and `disk_format!\u003dvolume_type`.","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"90265128b6a9e249605a5a78c77057017ac5785a","unresolved":false,"context_lines":[{"line_number":1014,"context_line":"                            detail\u003d"},{"line_number":1015,"context_line":"                            message_field.Detail.SIGNATURE_VERIFICATION_FAILED,"},{"line_number":1016,"context_line":"                            exception\u003derr)"},{"line_number":1017,"context_line":"                except exception.ImageUnacceptable as err:"},{"line_number":1018,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":1019,"context_line":"                        self.message.create("},{"line_number":1020,"context_line":"                            context,"},{"line_number":1021,"context_line":"                            message_field.Action.COPY_IMAGE_TO_VOLUME,"},{"line_number":1022,"context_line":"                            resource_uuid\u003dvolume.id,"},{"line_number":1023,"context_line":"                            detail\u003d"},{"line_number":1024,"context_line":"                            message_field.Detail.IMAGE_FORMAT_UNACCEPTABLE,"},{"line_number":1025,"context_line":"                            exception\u003derr)"},{"line_number":1026,"context_line":""},{"line_number":1027,"context_line":"            if should_create_cache_entry:"},{"line_number":1028,"context_line":"                # Update the newly created volume db entry before we clone it"}],"source_content_type":"text/x-python","patch_set":11,"id":"2fa45603_ff6eb0e6","line":1025,"range":{"start_line":1017,"start_character":0,"end_line":1025,"end_character":42},"in_reply_to":"d0fa9a9c_8619e93b","updated":"2022-05-31 11:50:09.000000000","message":"A few thoughts here:\n\n- the important thing is to get useful info out to the user, so if the only way to do that is to parse the message in the exception, that\u0027s OK with me.  If we have a unit test that looks for the message, then if the message is changed, the test should break, and we should be able to detect that before a regression happens.  It\u0027s kind a stupid unit test, but \n\n- we could fix this (though I haven\u0027t thought it through) by refactoring the ImageUnacceptable exception and giving it a detail member that could take a value of type cinder.message.message_field.Detail, and then that field could be loaded at the point of the error, when you know exactly what caused it\n\n- or we could simply refactor the error messages for each of the ImageUnacceptable messages to include a specific string that the code could look for to decide what Detail to use.  Problem here is that we\u0027d need to keep a list of the specific strings somewhere, which would be dumb since we have the Detail fields defined already.\n\n- looking at the Exception message raised here is making me reconsider the design of our user messages ... it contains explicit detail about what happened without revealing any deployment details\n\nMost of this is beyond the scope of this change, though.","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"8f7414831489f7b1c8bd5d2d4a2d3eebee83b53c","unresolved":false,"context_lines":[{"line_number":1014,"context_line":"                            detail\u003d"},{"line_number":1015,"context_line":"                            message_field.Detail.SIGNATURE_VERIFICATION_FAILED,"},{"line_number":1016,"context_line":"                            exception\u003derr)"},{"line_number":1017,"context_line":"                except exception.ImageUnacceptable as err:"},{"line_number":1018,"context_line":"                    with excutils.save_and_reraise_exception():"},{"line_number":1019,"context_line":"                        self.message.create("},{"line_number":1020,"context_line":"                            context,"},{"line_number":1021,"context_line":"                            message_field.Action.COPY_IMAGE_TO_VOLUME,"},{"line_number":1022,"context_line":"                            resource_uuid\u003dvolume.id,"},{"line_number":1023,"context_line":"                            detail\u003d"},{"line_number":1024,"context_line":"                            message_field.Detail.IMAGE_FORMAT_UNACCEPTABLE,"},{"line_number":1025,"context_line":"                            exception\u003derr)"},{"line_number":1026,"context_line":""},{"line_number":1027,"context_line":"            if should_create_cache_entry:"},{"line_number":1028,"context_line":"                # Update the newly created volume db entry before we clone it"}],"source_content_type":"text/x-python","patch_set":11,"id":"d0fa9a9c_8619e93b","line":1025,"range":{"start_line":1017,"start_character":0,"end_line":1025,"end_character":42},"in_reply_to":"f1e45403_ec439480","updated":"2022-05-30 18:22:14.000000000","message":"I\u0027m not really a fan of using specific message to be used to raise an exception or in general for any conditional as we can change a log or error message any time causing the regression. I don\u0027t even prefer it in test cases but guess we don\u0027t have a choice there.\nHaving said that I will let other cores take a look and see what they think about it.","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"29333da40392565c306a92e14727ff3cc74fa32d","unresolved":true,"context_lines":[{"line_number":1026,"context_line":"                                resource_uuid\u003dvolume.id,"},{"line_number":1027,"context_line":"                                detail\u003d"},{"line_number":1028,"context_line":"                                message_field.Detail.IMAGE_FORMAT_UNACCEPTABLE,"},{"line_number":1029,"context_line":"                                exception\u003derr)"},{"line_number":1030,"context_line":"                    else:"},{"line_number":1031,"context_line":"                        raise"},{"line_number":1032,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"02935b91_7a5f1fed","line":1029,"range":{"start_line":1029,"start_character":32,"end_line":1029,"end_character":45},"updated":"2022-05-31 11:57:36.000000000","message":"Since you know what detail message you want to use, don\u0027t pass the exception.  (If you really want to know why not, the user messages contributor docs explain it.)","commit_id":"3a3d1710509abe7c3af15df871b6b4d108c030ef"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"986acaf18a6259073413c3598f654bf12c8a0e54","unresolved":false,"context_lines":[{"line_number":1026,"context_line":"                                resource_uuid\u003dvolume.id,"},{"line_number":1027,"context_line":"                                detail\u003d"},{"line_number":1028,"context_line":"                                message_field.Detail.IMAGE_FORMAT_UNACCEPTABLE,"},{"line_number":1029,"context_line":"                                exception\u003derr)"},{"line_number":1030,"context_line":"                    else:"},{"line_number":1031,"context_line":"                        raise"},{"line_number":1032,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"eddad3f3_83c00f90","line":1029,"range":{"start_line":1029,"start_character":32,"end_line":1029,"end_character":45},"in_reply_to":"02935b91_7a5f1fed","updated":"2022-05-31 13:49:39.000000000","message":"Done","commit_id":"3a3d1710509abe7c3af15df871b6b4d108c030ef"}],"cinder/volume/manager.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"97af5a81c99b01279c3173673d9898b80f495102","unresolved":true,"context_lines":[{"line_number":5322,"context_line":"                context, image_meta[\u0027id\u0027])"},{"line_number":5323,"context_line":"            image_location \u003d image_service.get_location(context, image_id)"},{"line_number":5324,"context_line":""},{"line_number":5325,"context_line":"            volume_utils.copy_image_to_volume(self.driver, context, volume,"},{"line_number":5326,"context_line":"                                              image_meta, image_location,"},{"line_number":5327,"context_line":"                                              image_service)"},{"line_number":5328,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"ba419bff_cc73525b","line":5325,"range":{"start_line":5325,"start_character":12,"end_line":5325,"end_character":46},"updated":"2022-05-11 20:11:57.000000000","message":"this is the call that will raise ImageUnacceptable","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"160e757c9e9acd9c3e2dee7d4e6483d3eb01e378","unresolved":false,"context_lines":[{"line_number":5322,"context_line":"                context, image_meta[\u0027id\u0027])"},{"line_number":5323,"context_line":"            image_location \u003d image_service.get_location(context, image_id)"},{"line_number":5324,"context_line":""},{"line_number":5325,"context_line":"            volume_utils.copy_image_to_volume(self.driver, context, volume,"},{"line_number":5326,"context_line":"                                              image_meta, image_location,"},{"line_number":5327,"context_line":"                                              image_service)"},{"line_number":5328,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"11710942_d3e9bfbb","line":5325,"range":{"start_line":5325,"start_character":12,"end_line":5325,"end_character":46},"in_reply_to":"ba419bff_cc73525b","updated":"2022-05-12 16:47:05.000000000","message":"Ack","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"dcc2ca60fa409955960333c94e71c52826d0f989","unresolved":true,"context_lines":[{"line_number":5348,"context_line":"                volume.previous_status \u003d volume.status"},{"line_number":5349,"context_line":"                volume.status \u003d \u0027error\u0027"},{"line_number":5350,"context_line":"                volume.save()"},{"line_number":5351,"context_line":"                if isinstance(err, exception.ImageUnacceptable):"},{"line_number":5352,"context_line":"                    self.message_api.create("},{"line_number":5353,"context_line":"                        context,"},{"line_number":5354,"context_line":"                        message_field.Action.REIMAGE_VOLUME,"},{"line_number":5355,"context_line":"                        resource_uuid\u003dvolume.id,"},{"line_number":5356,"context_line":"                        detail\u003d"},{"line_number":5357,"context_line":"                        message_field.Detail.IMAGE_FORMAT_UNACCEPTABLE,"},{"line_number":5358,"context_line":"                        exception\u003derr)"}],"source_content_type":"text/x-python","patch_set":11,"id":"dadd90ff_d9bf6865","line":5358,"range":{"start_line":5351,"start_character":0,"end_line":5358,"end_character":38},"updated":"2022-05-27 09:52:38.000000000","message":"similar case applies here as well, what happens if the ImageUnacceptable exception is raised because of any other cause than the format mismatch?\nAssuming image_conversion_disable\u003dFalse is the default, there might be only few instances where this message makes sense and we should only create this message if we are sure that image_conversion_disable\u003dTrue and the format actually mismatches. I think it makes more sense to create this in the check_image_conversion_disable method[1] before raising the ImageUnacceptable exception.\n\n[1] https://review.opendev.org/c/openstack/cinder/+/839793/11/cinder/image/image_utils.py","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"52e4596a49165d960cbe9eebc886b97d3807ec52","unresolved":false,"context_lines":[{"line_number":5348,"context_line":"                volume.previous_status \u003d volume.status"},{"line_number":5349,"context_line":"                volume.status \u003d \u0027error\u0027"},{"line_number":5350,"context_line":"                volume.save()"},{"line_number":5351,"context_line":"                if isinstance(err, exception.ImageUnacceptable):"},{"line_number":5352,"context_line":"                    self.message_api.create("},{"line_number":5353,"context_line":"                        context,"},{"line_number":5354,"context_line":"                        message_field.Action.REIMAGE_VOLUME,"},{"line_number":5355,"context_line":"                        resource_uuid\u003dvolume.id,"},{"line_number":5356,"context_line":"                        detail\u003d"},{"line_number":5357,"context_line":"                        message_field.Detail.IMAGE_FORMAT_UNACCEPTABLE,"},{"line_number":5358,"context_line":"                        exception\u003derr)"}],"source_content_type":"text/x-python","patch_set":11,"id":"d7737103_3eb01b5b","line":5358,"range":{"start_line":5351,"start_character":0,"end_line":5358,"end_character":38},"in_reply_to":"dadd90ff_d9bf6865","updated":"2022-05-27 18:42:26.000000000","message":"same as above, https://review.opendev.org/c/openstack/cinder/+/839793/11/cinder/volume/flows/manager/create_volume.py#1025\nIf it\u0027s okay with you, I think we can use error message to check?","commit_id":"6cf28dd9a97d9c589439fed155986ee5181b2dcc"}],"releasenotes/notes/allow_disable_image_conversion-ebf33ce9d5edf724.yaml":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"4e32a33b75b244de6035a0eb1cf19d9b1172043b","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Add new config option `allow_image_conversion`, when it\u0027s setted off,"},{"line_number":5,"context_line":"    image disk_format and volume format must be the same format."},{"line_number":6,"context_line":"    Otherwise will raise ImageUnacceptable exception."},{"line_number":7,"context_line":"    `allow_image_conversion` config is bool option, default to `True`."}],"source_content_type":"text/x-yaml","patch_set":4,"id":"e1cbad00_68f8acb2","line":4,"range":{"start_line":4,"start_character":62,"end_line":4,"end_character":72},"updated":"2022-04-29 13:16:42.000000000","message":"Maybe \"set to false\" would be more clear?","commit_id":"44e4484b42e81d8094e825e784b31d276a392b83"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"5ae061ba0e9cfbb9bf1d8d5908285c9d2e94f979","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Add new config option `allow_image_conversion`, when it\u0027s setted off,"},{"line_number":5,"context_line":"    image disk_format and volume format must be the same format."},{"line_number":6,"context_line":"    Otherwise will raise ImageUnacceptable exception."},{"line_number":7,"context_line":"    `allow_image_conversion` config is bool option, default to `True`."}],"source_content_type":"text/x-yaml","patch_set":4,"id":"c303b97c_94a51a36","line":4,"range":{"start_line":4,"start_character":62,"end_line":4,"end_character":72},"in_reply_to":"e1cbad00_68f8acb2","updated":"2022-05-04 04:21:32.000000000","message":"Done","commit_id":"44e4484b42e81d8094e825e784b31d276a392b83"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"4e32a33b75b244de6035a0eb1cf19d9b1172043b","unresolved":true,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Add new config option `allow_image_conversion`, when it\u0027s setted off,"},{"line_number":5,"context_line":"    image disk_format and volume format must be the same format."},{"line_number":6,"context_line":"    Otherwise will raise ImageUnacceptable exception."},{"line_number":7,"context_line":"    `allow_image_conversion` config is bool option, default to `True`."}],"source_content_type":"text/x-yaml","patch_set":4,"id":"3ede9d74_3ff7b851","line":6,"range":{"start_line":6,"start_character":4,"end_line":6,"end_character":53},"updated":"2022-04-29 13:16:42.000000000","message":"I think this is too low level of detail for an end user that would be reading the release note. Maybe something like \"Otherwise attempting to use an image in a different format than the requested volume will fail.\"","commit_id":"44e4484b42e81d8094e825e784b31d276a392b83"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"5ae061ba0e9cfbb9bf1d8d5908285c9d2e94f979","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    Add new config option `allow_image_conversion`, when it\u0027s setted off,"},{"line_number":5,"context_line":"    image disk_format and volume format must be the same format."},{"line_number":6,"context_line":"    Otherwise will raise ImageUnacceptable exception."},{"line_number":7,"context_line":"    `allow_image_conversion` config is bool option, default to `True`."}],"source_content_type":"text/x-yaml","patch_set":4,"id":"14e61268_3bf410eb","line":6,"range":{"start_line":6,"start_character":4,"end_line":6,"end_character":53},"in_reply_to":"3ede9d74_3ff7b851","updated":"2022-05-04 04:21:32.000000000","message":"Done","commit_id":"44e4484b42e81d8094e825e784b31d276a392b83"},{"author":{"_account_id":9236,"name":"Jon Bernard","email":"jobernar@redhat.com","username":"jbernard"},"change_message_id":"a7a1ede54fbf1fb5ff14f8f053e5e1a252afb26b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":5,"id":"0e13736c_0212daec","line":9,"updated":"2022-05-03 18:09:14.000000000","message":"I propose being more direct in naming this, instead of defining\n\u0027allow_image_converstion\u0027 and setting it to false, make the flag\n\u0027disable_image_conversion\u0027.  This way the cinder default of allow is\nimplied, I think it\u0027s less confusing this way, but just my opinion.","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"5ae061ba0e9cfbb9bf1d8d5908285c9d2e94f979","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"416e7e48_795d8ec4","line":9,"in_reply_to":"0e13736c_0212daec","updated":"2022-05-04 04:21:32.000000000","message":"Done","commit_id":"bc270ceb0deced44a625a057a23476e4baf19920"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"97af5a81c99b01279c3173673d9898b80f495102","unresolved":true,"context_lines":[{"line_number":5,"context_line":"    image disk_format and volume format must be the same format."},{"line_number":6,"context_line":"    Otherwise attempting to use an image in a different format than the"},{"line_number":7,"context_line":"    requested volume will fail."},{"line_number":8,"context_line":"    `image_conversion_disable` config is boolean option, default to `False`."}],"source_content_type":"text/x-yaml","patch_set":9,"id":"511f6f57_90f46e16","line":8,"updated":"2022-05-11 20:11:57.000000000","message":"nit: single backticks give you italics, I think you want double-backticks throughout in order to get monospace font.","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"160e757c9e9acd9c3e2dee7d4e6483d3eb01e378","unresolved":false,"context_lines":[{"line_number":5,"context_line":"    image disk_format and volume format must be the same format."},{"line_number":6,"context_line":"    Otherwise attempting to use an image in a different format than the"},{"line_number":7,"context_line":"    requested volume will fail."},{"line_number":8,"context_line":"    `image_conversion_disable` config is boolean option, default to `False`."}],"source_content_type":"text/x-yaml","patch_set":9,"id":"dfca3cb7_ce4c245d","line":8,"in_reply_to":"511f6f57_90f46e16","updated":"2022-05-12 16:47:05.000000000","message":"Done","commit_id":"ecd10f97dca1872774da7e4c62e0d3dc24fb7c0a"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"d092bbf743d3e3d0d6b6e8dad202a37c6ca3143d","unresolved":true,"context_lines":[{"line_number":5,"context_line":"    ``True``, image disk_format and volume format must be the same format."},{"line_number":6,"context_line":"    Otherwise attempting to use an image in a different format than the"},{"line_number":7,"context_line":"    requested volume will fail. ``image_conversion_disable`` config is"},{"line_number":8,"context_line":"    boolean option, default to ``False``."}],"source_content_type":"text/x-yaml","patch_set":10,"id":"ae15bcdc_46c08404","line":8,"updated":"2022-05-12 18:35:23.000000000","message":"Suggested re-write here:\nhttps://paste.opendev.org/show/b2BUlzqlKoCFUxKtJGn7/\n\n(You need to verify that what it says is actually true!)","commit_id":"893543dad6e64d1638dba28f705ef9510dfebc1c"},{"author":{"_account_id":12404,"name":"Rico Lin","email":"ricolin@ricolky.com","username":"rico.lin"},"change_message_id":"5bc05fac006693ef8d9ae8e4861792361e56a3b2","unresolved":false,"context_lines":[{"line_number":5,"context_line":"    ``True``, image disk_format and volume format must be the same format."},{"line_number":6,"context_line":"    Otherwise attempting to use an image in a different format than the"},{"line_number":7,"context_line":"    requested volume will fail. ``image_conversion_disable`` config is"},{"line_number":8,"context_line":"    boolean option, default to ``False``."}],"source_content_type":"text/x-yaml","patch_set":10,"id":"a0298364_0ae77748","line":8,"in_reply_to":"ae15bcdc_46c08404","updated":"2022-05-25 16:55:58.000000000","message":"Addressed","commit_id":"893543dad6e64d1638dba28f705ef9510dfebc1c"}]}