)]}'
{"nova/volume/cinder.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"50dd83d30bbf0ac68c3e734e50d885750f729729","unresolved":false,"context_lines":[{"line_number":327,"context_line":"        d[\u0027shared_targets\u0027] \u003d vol.shared_targets"},{"line_number":328,"context_line":"        d[\u0027service_uuid\u0027] \u003d vol.service_uuid"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"    if hasattr(vol, \u0027migration_status\u0027):"},{"line_number":331,"context_line":"        d[\u0027migration_status\u0027] \u003d vol.migration_status"},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"    return d"}],"source_content_type":"text/x-python","patch_set":1,"id":"dfbec78f_0162433f","line":330,"updated":"2019-05-07 12:30:33.000000000","message":"I didn\u0027t think about this before, but \u0027migration_status\u0027 is not listed as an optional parameter on the volume in the cinder API, so this doesn\u0027t really need the hasattr. It makes for less unit test damage and such, but not technically something we should consider optional.","commit_id":"91282f879cb453e4baa6c6decb4fff849c7a1b2a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"96b4eea359b8f44d9b1c4707dd4d4d9895dad94a","unresolved":false,"context_lines":[{"line_number":327,"context_line":"        d[\u0027shared_targets\u0027] \u003d vol.shared_targets"},{"line_number":328,"context_line":"        d[\u0027service_uuid\u0027] \u003d vol.service_uuid"},{"line_number":329,"context_line":""},{"line_number":330,"context_line":"    if hasattr(vol, \u0027migration_status\u0027):"},{"line_number":331,"context_line":"        d[\u0027migration_status\u0027] \u003d vol.migration_status"},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"    return d"}],"source_content_type":"text/x-python","patch_set":1,"id":"dfbec78f_7041810b","line":330,"in_reply_to":"dfbec78f_0162433f","updated":"2019-05-07 21:13:16.000000000","message":"Hmm, looking at the cinder code and a tempest run I\u0027ve changed my mind, since I\u0027m not seeing migration_status in this GET response during tempest:\n\n{\"volume\": {\"id\": \"0a93295e-ef75-47b1-b97f-7cd042d0bdfb\", \"status\": \"available\", \"size\": 1, \"availability_zone\": \"nova\", \"created_at\": \"2019-05-07T17:59:15.000000\", \"updated_at\": \"2019-05-07T18:01:01.000000\", \"attachments\": [], \"name\": \"tempest-TestVolumeMigrateRetypeAttached-volume-1716148619\", \"description\": null, \"volume_type\": \"tempest-volume-type-TestVolumeMigrateRetypeAttached-1588848544\", \"snapshot_id\": null, \"source_volid\": null, \"metadata\": {}, \"links\": [{\"rel\": \"self\", \"href\": \"https://10.211.1.104/volume/v3/9787e06930ec4d7e9b814972655123e3/volumes/0a93295e-ef75-47b1-b97f-7cd042d0bdfb\"}, {\"rel\": \"bookmark\", \"href\": \"https://10.211.1.104/volume/9787e06930ec4d7e9b814972655123e3/volumes/0a93295e-ef75-47b1-b97f-7cd042d0bdfb\"}], \"user_id\": \"dab90e3c4e6f4d30b13928f6f43cbacb\", \"bootable\": \"true\", \"encrypted\": false, \"replication_status\": null, \"consistencygroup_id\": null, \"multiattach\": false, \"os-vol-tenant-attr:tenant_id\": \"9787e06930ec4d7e9b814972655123e3\", \"volume_image_metadata\": {\"signature_verified\": \"False\", \"image_id\": \"f556bfe8-03ef-4b59-bbc6-4d1d5235478c\", \"image_name\": \"cirros-0.4.0-x86_64-disk\", \"checksum\": \"443b7623e27ecf03dc9e01ee93f67afe\", \"container_format\": \"bare\", \"disk_format\": \"qcow2\", \"min_disk\": \"0\", \"min_ram\": \"0\", \"size\": \"12716032\"}}}\n\nAnd looking at their view builder code, it\u0027s only returned for an admin context:\n\nhttps://github.com/openstack/cinder/blob/779979789c12d66686fd1fe163bf963c00502136/cinder/api/v2/views/volumes.py#L95\n\nBut I guess since volume migration (not retype) is admin-only by default, and swap volume in nova is admin-only by default, we should get this back in most cases during a volume migration. Now if someone messes with the cinder policy to allow non-admins to perform a volume migration, this is going to be all sorts of busted.","commit_id":"91282f879cb453e4baa6c6decb4fff849c7a1b2a"}]}