)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"343964c26d74fc21242c83ccb970a957f2722fd2","unresolved":true,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Handle missing dst_pool parameter in zone_migration"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Unlike Nova, Cinder does not support calling the \u0027os-migrate_volume\u0027[1]"},{"line_number":10,"context_line":"action without a host or a cluster. For volume migrations of type"},{"line_number":11,"context_line":"\u0027migrate\u0027 in watcher the dst_pool is required, but for other migrations"},{"line_number":12,"context_line":"that migrate the volumes to different types is not needed. This"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"cf6a727a_ae066948","line":9,"range":{"start_line":9,"start_character":68,"end_line":9,"end_character":71},"updated":"2025-08-05 17:16:22.000000000","message":"you forgot the link :)","commit_id":"a67e3d827ac3659a90ae0b0787888bf2528c244f"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"440eac356828aec52281ea042479afaa1ce8fcec","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Handle missing dst_pool parameter in zone_migration"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Unlike Nova, Cinder does not support calling the \u0027os-migrate_volume\u0027[1]"},{"line_number":10,"context_line":"action without a host or a cluster. For volume migrations of type"},{"line_number":11,"context_line":"\u0027migrate\u0027 in watcher the dst_pool is required, but for other migrations"},{"line_number":12,"context_line":"that migrate the volumes to different types is not needed. This"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":11,"id":"900f67aa_9e29e25e","line":9,"range":{"start_line":9,"start_character":68,"end_line":9,"end_character":71},"in_reply_to":"cf6a727a_ae066948","updated":"2025-08-13 10:06:43.000000000","message":"fixed, thanks!","commit_id":"a67e3d827ac3659a90ae0b0787888bf2528c244f"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"4b0ded69e9cb54cfa473daf85fc5f2936e2ccbc3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"799fd01d_b14bd8ed","updated":"2025-05-21 15:05:24.000000000","message":"The proposal looks good. There are still some discussions about if we should continue with this approach, thinking on backporting or not (schema change).\nWaiting some inputs from Sean that is looking at this.\nThanks Joan.","commit_id":"ffd8c770b4b809f5f7c7e99ddaa2ad3f9d0a5d53"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"e79485ed40b61a59b43a2d1e5d22421fae2ce605","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"d8371788_4c7bf377","updated":"2025-06-11 06:54:46.000000000","message":"overall looks good, I saw some comments around https://review.opendev.org/c/openstack/watcher/+/949703 HTTP 400 error in tests. It is merged now, Do we want to update the tests?","commit_id":"499cc10b103159a2f0dc1f0eb6d349f65c026b4e"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"5a2c6ee7d14bd65d1da37263a267734c198853f7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"8766eae9_4337ed71","in_reply_to":"d8371788_4c7bf377","updated":"2025-06-11 08:08:23.000000000","message":"that\u0027s right, thanks for noticing Chandan, I\u0027ve updated the tests to check the HTTP reponse code and removed the comment","commit_id":"499cc10b103159a2f0dc1f0eb6d349f65c026b4e"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"6afbee3418b1b060dbb16a1b67d1517c0807b81b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"e8138dc8_20940954","updated":"2025-06-11 10:22:56.000000000","message":"Thank Joan!","commit_id":"30177b8b27983e41dfc8d52738b8ba217a0f840e"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"343964c26d74fc21242c83ccb970a957f2722fd2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"299b00af_2a7c2445","updated":"2025-08-05 17:16:22.000000000","message":"the fix looks good, just added some concerns about the added tests and the release notes","commit_id":"a67e3d827ac3659a90ae0b0787888bf2528c244f"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"150b1abc5652505b4e536838b08e41f81232eb82","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"2d1bbe9c_aaebbcb5","updated":"2025-09-09 12:42:24.000000000","message":"lgtm, ty for the updates","commit_id":"6dccf395099897e66dc98772481920ce3f70e33f"}],"doc/source/strategies/zone_migration.rst":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"4b0ded69e9cb54cfa473daf85fc5f2936e2ccbc3","unresolved":true,"context_lines":[{"line_number":103,"context_line":"                                      volumes migrate(mandatory)."},{"line_number":104,"context_line":"``dst_pool``  string    None          Storage pool to which"},{"line_number":105,"context_line":"                                      volumes migrate(mandatory)."},{"line_number":106,"context_line":"``src_type``  string    None          Source volume type(mandatory)."},{"line_number":107,"context_line":"``dst_type``  string    None          Destination volume type"},{"line_number":108,"context_line":"                                      (mandatory)."},{"line_number":109,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d \u003d\u003d\u003d\u003d\u003d\u003d\u003d \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":4,"id":"d7f27d2c_c7d8868f","line":106,"range":{"start_line":106,"start_character":38,"end_line":106,"end_character":68},"updated":"2025-05-21 15:05:24.000000000","message":"trying to understand where srt_type is being used. It is filtering the volumes based on src_type? I don\u0027t think that needs to be mandatory tbh, but this is a different issue.","commit_id":"ffd8c770b4b809f5f7c7e99ddaa2ad3f9d0a5d53"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"0a37e5576dda8cb35721f4b0ab46571412034d8e","unresolved":true,"context_lines":[{"line_number":103,"context_line":"                                      volumes migrate(mandatory)."},{"line_number":104,"context_line":"``dst_pool``  string    None          Storage pool to which"},{"line_number":105,"context_line":"                                      volumes migrate(mandatory)."},{"line_number":106,"context_line":"``src_type``  string    None          Source volume type(mandatory)."},{"line_number":107,"context_line":"``dst_type``  string    None          Destination volume type"},{"line_number":108,"context_line":"                                      (mandatory)."},{"line_number":109,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d \u003d\u003d\u003d\u003d\u003d\u003d\u003d \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":4,"id":"fc5347cd_14ed07b6","line":106,"range":{"start_line":106,"start_character":38,"end_line":106,"end_character":68},"in_reply_to":"30d2a321_126ccb19","updated":"2025-05-22 09:43:51.000000000","message":"actually you\u0027re right, after looking more carefully, src_type is not used at all, I\u0027ll file a bug for that, since there are valid use cases the parameter should provide that are currently not supported","commit_id":"ffd8c770b4b809f5f7c7e99ddaa2ad3f9d0a5d53"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"6b08c91316b05803e32892cd2d8a19fd371f200c","unresolved":true,"context_lines":[{"line_number":103,"context_line":"                                      volumes migrate(mandatory)."},{"line_number":104,"context_line":"``dst_pool``  string    None          Storage pool to which"},{"line_number":105,"context_line":"                                      volumes migrate(mandatory)."},{"line_number":106,"context_line":"``src_type``  string    None          Source volume type(mandatory)."},{"line_number":107,"context_line":"``dst_type``  string    None          Destination volume type"},{"line_number":108,"context_line":"                                      (mandatory)."},{"line_number":109,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d \u003d\u003d\u003d\u003d\u003d\u003d\u003d \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":4,"id":"30d2a321_126ccb19","line":106,"range":{"start_line":106,"start_character":38,"end_line":106,"end_character":68},"in_reply_to":"d7f27d2c_c7d8868f","updated":"2025-05-21 17:35:20.000000000","message":"IIUC it\u0027s required because of this https://github.com/openstack/watcher/blob/26e36e1620bdf2433d2aaadafce612271b0497a3/watcher/decision_engine/strategy/strategies/zone_migration.py#L408. Depending on the type of volume and its state, the migration procedure is quite different. The type of migration is chosen per volume depending on its state, it can\u0027t really be influenced by the user input, so to be able to migrate volumes we basically need to require all parameters. It\u0027s not great UX, and we probably should revisit this in the future, but I suspect fixing this would require a significant refactor","commit_id":"ffd8c770b4b809f5f7c7e99ddaa2ad3f9d0a5d53"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"440eac356828aec52281ea042479afaa1ce8fcec","unresolved":false,"context_lines":[{"line_number":103,"context_line":"                                      volumes migrate(mandatory)."},{"line_number":104,"context_line":"``dst_pool``  string    None          Storage pool to which"},{"line_number":105,"context_line":"                                      volumes migrate(mandatory)."},{"line_number":106,"context_line":"``src_type``  string    None          Source volume type(mandatory)."},{"line_number":107,"context_line":"``dst_type``  string    None          Destination volume type"},{"line_number":108,"context_line":"                                      (mandatory)."},{"line_number":109,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d \u003d\u003d\u003d\u003d\u003d\u003d\u003d \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d \u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"}],"source_content_type":"text/x-rst","patch_set":4,"id":"e8157e6e_a976daff","line":106,"range":{"start_line":106,"start_character":38,"end_line":106,"end_character":68},"in_reply_to":"fc5347cd_14ed07b6","updated":"2025-08-13 10:06:43.000000000","message":"Done","commit_id":"ffd8c770b4b809f5f7c7e99ddaa2ad3f9d0a5d53"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"0ac94a66bf65cd39b4eaf842c5cbe4f976ba2f7e","unresolved":true,"context_lines":[{"line_number":102,"context_line":"``src_pool``  string    None          Storage pool from which"},{"line_number":103,"context_line":"                                      volumes migrate(mandatory)."},{"line_number":104,"context_line":"``dst_pool``  string    None          Storage pool to which"},{"line_number":105,"context_line":"                                      volumes migrate(mandatory)."},{"line_number":106,"context_line":"``src_type``  string    None          Source volume type(mandatory)."},{"line_number":107,"context_line":"``dst_type``  string    None          Destination volume type"},{"line_number":108,"context_line":"                                      (mandatory)."}],"source_content_type":"text/x-rst","patch_set":7,"id":"fc6b31e9_0dce2c12","line":105,"range":{"start_line":105,"start_character":0,"end_line":105,"end_character":2},"updated":"2025-05-29 07:39:51.000000000","message":"We are really not making this mandatory, right?. In case it\u0027s not passed we are triggering a pure retyping for the volumes where actual type !\u003d dst_type.","commit_id":"90bbdd89973dae0ae226ee8e697960fd2c41d72f"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"440eac356828aec52281ea042479afaa1ce8fcec","unresolved":false,"context_lines":[{"line_number":102,"context_line":"``src_pool``  string    None          Storage pool from which"},{"line_number":103,"context_line":"                                      volumes migrate(mandatory)."},{"line_number":104,"context_line":"``dst_pool``  string    None          Storage pool to which"},{"line_number":105,"context_line":"                                      volumes migrate(mandatory)."},{"line_number":106,"context_line":"``src_type``  string    None          Source volume type(mandatory)."},{"line_number":107,"context_line":"``dst_type``  string    None          Destination volume type"},{"line_number":108,"context_line":"                                      (mandatory)."}],"source_content_type":"text/x-rst","patch_set":7,"id":"f6f03a7b_3aeb0243","line":105,"range":{"start_line":105,"start_character":0,"end_line":105,"end_character":2},"in_reply_to":"daa3f7c4_e03c47f5","updated":"2025-08-13 10:06:43.000000000","message":"Done","commit_id":"90bbdd89973dae0ae226ee8e697960fd2c41d72f"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"de262338e3d9df09d8a8a35af35115911532621b","unresolved":true,"context_lines":[{"line_number":102,"context_line":"``src_pool``  string    None          Storage pool from which"},{"line_number":103,"context_line":"                                      volumes migrate(mandatory)."},{"line_number":104,"context_line":"``dst_pool``  string    None          Storage pool to which"},{"line_number":105,"context_line":"                                      volumes migrate(mandatory)."},{"line_number":106,"context_line":"``src_type``  string    None          Source volume type(mandatory)."},{"line_number":107,"context_line":"``dst_type``  string    None          Destination volume type"},{"line_number":108,"context_line":"                                      (mandatory)."}],"source_content_type":"text/x-rst","patch_set":7,"id":"daa3f7c4_e03c47f5","line":105,"range":{"start_line":105,"start_character":0,"end_line":105,"end_character":2},"in_reply_to":"fc6b31e9_0dce2c12","updated":"2025-06-02 09:53:40.000000000","message":"right, I forgot to update the code, I\u0027ll fix it in the next PS, thanks","commit_id":"90bbdd89973dae0ae226ee8e697960fd2c41d72f"}],"releasenotes/notes/zone-migration-missing-dst-node-bd0377af1f1ed245.yaml":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"0ac94a66bf65cd39b4eaf842c5cbe4f976ba2f7e","unresolved":true,"context_lines":[{"line_number":6,"context_line":"    This brings the implementation of the strategy in line with the the api schema"},{"line_number":7,"context_line":"    where dest_node is optional."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"    If a dst_pool is not passed, the audit will now fail, as Cinder can\u0027t compute a"},{"line_number":10,"context_line":"    destination host when migrating available volumes like Nova does, so a dst_pool should be required."},{"line_number":11,"context_line":"    This aligns the api schema for the zone migration strategy  with its implementation."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"    See: https://bugs.launchpad.net/watcher/+bug/2108988 for more details."}],"source_content_type":"text/x-yaml","patch_set":7,"id":"ef862a2c_f66680f8","line":12,"range":{"start_line":9,"start_character":3,"end_line":12,"end_character":1},"updated":"2025-05-29 07:39:51.000000000","message":"iiuc, this is not longer true.","commit_id":"90bbdd89973dae0ae226ee8e697960fd2c41d72f"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"440eac356828aec52281ea042479afaa1ce8fcec","unresolved":false,"context_lines":[{"line_number":6,"context_line":"    This brings the implementation of the strategy in line with the the api schema"},{"line_number":7,"context_line":"    where dest_node is optional."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"    If a dst_pool is not passed, the audit will now fail, as Cinder can\u0027t compute a"},{"line_number":10,"context_line":"    destination host when migrating available volumes like Nova does, so a dst_pool should be required."},{"line_number":11,"context_line":"    This aligns the api schema for the zone migration strategy  with its implementation."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"    See: https://bugs.launchpad.net/watcher/+bug/2108988 for more details."}],"source_content_type":"text/x-yaml","patch_set":7,"id":"8def6aff_66660ac0","line":12,"range":{"start_line":9,"start_character":3,"end_line":12,"end_character":1},"in_reply_to":"c6a960cd_8559b40e","updated":"2025-08-13 10:06:43.000000000","message":"Done","commit_id":"90bbdd89973dae0ae226ee8e697960fd2c41d72f"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"de262338e3d9df09d8a8a35af35115911532621b","unresolved":true,"context_lines":[{"line_number":6,"context_line":"    This brings the implementation of the strategy in line with the the api schema"},{"line_number":7,"context_line":"    where dest_node is optional."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"    If a dst_pool is not passed, the audit will now fail, as Cinder can\u0027t compute a"},{"line_number":10,"context_line":"    destination host when migrating available volumes like Nova does, so a dst_pool should be required."},{"line_number":11,"context_line":"    This aligns the api schema for the zone migration strategy  with its implementation."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"    See: https://bugs.launchpad.net/watcher/+bug/2108988 for more details."}],"source_content_type":"text/x-yaml","patch_set":7,"id":"c6a960cd_8559b40e","line":12,"range":{"start_line":9,"start_character":3,"end_line":12,"end_character":1},"in_reply_to":"ef862a2c_f66680f8","updated":"2025-06-02 09:53:40.000000000","message":"yep, I\u0027ll change it in the next PS","commit_id":"90bbdd89973dae0ae226ee8e697960fd2c41d72f"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"343964c26d74fc21242c83ccb970a957f2722fd2","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"    If a dst_pool is not passed, the strategy will not migrate some volumes, as Cinder can\u0027t compute a"},{"line_number":10,"context_line":"    destination host when migrating available volumes like Nova does. If a dst_pool is not passed,"},{"line_number":11,"context_line":"    the statregy will retype the volumes that require it, but do nothing for volumes that have the dst_type."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"    See: https://bugs.launchpad.net/watcher/+bug/2108988 for more details."}],"source_content_type":"text/x-yaml","patch_set":11,"id":"4e6e4d6c_782371a5","line":11,"range":{"start_line":11,"start_character":8,"end_line":11,"end_character":16},"updated":"2025-08-05 17:16:22.000000000","message":"typo: strategy","commit_id":"a67e3d827ac3659a90ae0b0787888bf2528c244f"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"440eac356828aec52281ea042479afaa1ce8fcec","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"    If a dst_pool is not passed, the strategy will not migrate some volumes, as Cinder can\u0027t compute a"},{"line_number":10,"context_line":"    destination host when migrating available volumes like Nova does. If a dst_pool is not passed,"},{"line_number":11,"context_line":"    the statregy will retype the volumes that require it, but do nothing for volumes that have the dst_type."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"    See: https://bugs.launchpad.net/watcher/+bug/2108988 for more details."}],"source_content_type":"text/x-yaml","patch_set":11,"id":"763c3f89_4de30fc3","line":11,"range":{"start_line":11,"start_character":8,"end_line":11,"end_character":16},"in_reply_to":"4e6e4d6c_782371a5","updated":"2025-08-13 10:06:43.000000000","message":"Done","commit_id":"a67e3d827ac3659a90ae0b0787888bf2528c244f"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"343964c26d74fc21242c83ccb970a957f2722fd2","unresolved":true,"context_lines":[{"line_number":7,"context_line":"    where dest_node is optional."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"    If a dst_pool is not passed, the strategy will not migrate some volumes, as Cinder can\u0027t compute a"},{"line_number":10,"context_line":"    destination host when migrating available volumes like Nova does. If a dst_pool is not passed,"},{"line_number":11,"context_line":"    the statregy will retype the volumes that require it, but do nothing for volumes that have the dst_type."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"    See: https://bugs.launchpad.net/watcher/+bug/2108988 for more details."}],"source_content_type":"text/x-yaml","patch_set":11,"id":"33820dbf_8c1dc5a2","line":12,"range":{"start_line":10,"start_character":69,"end_line":12,"end_character":0},"updated":"2025-08-05 17:16:22.000000000","message":"it was confusing to me, I was able to understand after looking at the code.\nI think that the most important is to let this clear in the strategy documentation, but maybe we can explain like: \"if src_type and dst_type are equal a migration is only performed tif a dst_pool is provided, otherwise the volume will be skipped/ignored.\"","commit_id":"a67e3d827ac3659a90ae0b0787888bf2528c244f"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"440eac356828aec52281ea042479afaa1ce8fcec","unresolved":false,"context_lines":[{"line_number":7,"context_line":"    where dest_node is optional."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"    If a dst_pool is not passed, the strategy will not migrate some volumes, as Cinder can\u0027t compute a"},{"line_number":10,"context_line":"    destination host when migrating available volumes like Nova does. If a dst_pool is not passed,"},{"line_number":11,"context_line":"    the statregy will retype the volumes that require it, but do nothing for volumes that have the dst_type."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"    See: https://bugs.launchpad.net/watcher/+bug/2108988 for more details."}],"source_content_type":"text/x-yaml","patch_set":11,"id":"df029805_05777186","line":12,"range":{"start_line":10,"start_character":69,"end_line":12,"end_character":0},"in_reply_to":"33820dbf_8c1dc5a2","updated":"2025-08-13 10:06:43.000000000","message":"ack, thanks for the suggestion Doug, done!","commit_id":"a67e3d827ac3659a90ae0b0787888bf2528c244f"}],"watcher/decision_engine/strategy/strategies/zone_migration.py":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"36a561a0f6ef7851e4aa3df304a936c764d9a126","unresolved":true,"context_lines":[{"line_number":405,"context_line":"            LOG.debug(src_type)"},{"line_number":406,"context_line":"            LOG.debug(\"%s %s\", dst_pool, dst_type)"},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"            if self.is_available(volume):"},{"line_number":409,"context_line":"                if src_type \u003d\u003d dst_type:"},{"line_number":410,"context_line":"                    self._volume_migrate(volume, dst_pool)"},{"line_number":411,"context_line":"                else:"},{"line_number":412,"context_line":"                    self._volume_retype(volume, dst_type)"},{"line_number":413,"context_line":"            elif self.is_in_use(volume):"},{"line_number":414,"context_line":"                self._volume_update(volume, dst_type)"},{"line_number":415,"context_line":""},{"line_number":416,"context_line":"                # if with_attached_volume is True, migrate attaching instances"},{"line_number":417,"context_line":"                if self.with_attached_volume:"}],"source_content_type":"text/x-python","patch_set":4,"id":"efb12277_fe246017","line":414,"range":{"start_line":408,"start_character":0,"end_line":414,"end_character":53},"updated":"2025-05-22 07:45:31.000000000","message":"My understanding of this logic together with the description in https://github.com/openstack/watcher/blob/master/watcher/applier/actions/volume_migration.py#L37-L42 is:\n\n1. If volume is detached (is_available\u003d\u003dTrue):\n\n- If src_type \u003d\u003d dst_type, it considers it a real migration (moving volume to a different pool) which requires a dst_pool (according to api, host is optional https://docs.openstack.org/api-ref/block-storage/v3/#migrate-a-volume but I\u0027m not totally sure how that works and the code in watcher to migrate seems to require it).\n\n- If src_type !\u003d dst_type, it considers it a retype in the same pool. That does not require a dst_pool (https://docs.openstack.org/api-ref/block-storage/v3/#retype-a-volume), only accepts a dst_type. According to the API doc, apparently cinder can retype an in-use volume with certain limitations, so I guess watcher maintainers decided to support only retype dettached volumes. \n\n2. If volume is attached (is_in_use\u003d\u003dTrue). It can only do a \"swap\" migration which does not use the dst_pool.\n\n\nIMO there is a valid use case for not providing the dst_pool, which is the case where a user wants to retype all the volumes in a pool which have a type to a different type. In that case we\u0027d want to specify the src_pool, dst_type and potentially src_type (to retype only volumes in a specific one). In that case the only thing we should do is to manage the case of dettached volumes where src_type \u003d\u003d to dst_type if dst_pool is not provided. In that case, the driver should do nothing with it, maybe left a message in the log.\n\nSaid that, for that use case, we still have one more issue, for the case of the volumes attached, we are always doing a swap, even in the case where the src_type \u003d\u003d dst_type, which i think it should do nothing.\n\nMy suggestion is to first define which is the expected behavior we want based on the different parameter combinations and status of the volumes and fix based on that. IMO, valid use cases would be retype and migrate (move to a different pool), but we should consider if we support the combination of both.","commit_id":"ffd8c770b4b809f5f7c7e99ddaa2ad3f9d0a5d53"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"440eac356828aec52281ea042479afaa1ce8fcec","unresolved":false,"context_lines":[{"line_number":405,"context_line":"            LOG.debug(src_type)"},{"line_number":406,"context_line":"            LOG.debug(\"%s %s\", dst_pool, dst_type)"},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"            if self.is_available(volume):"},{"line_number":409,"context_line":"                if src_type \u003d\u003d dst_type:"},{"line_number":410,"context_line":"                    self._volume_migrate(volume, dst_pool)"},{"line_number":411,"context_line":"                else:"},{"line_number":412,"context_line":"                    self._volume_retype(volume, dst_type)"},{"line_number":413,"context_line":"            elif self.is_in_use(volume):"},{"line_number":414,"context_line":"                self._volume_update(volume, dst_type)"},{"line_number":415,"context_line":""},{"line_number":416,"context_line":"                # if with_attached_volume is True, migrate attaching instances"},{"line_number":417,"context_line":"                if self.with_attached_volume:"}],"source_content_type":"text/x-python","patch_set":4,"id":"8b218034_0a0dd650","line":414,"range":{"start_line":408,"start_character":0,"end_line":414,"end_character":53},"in_reply_to":"a8c367f5_9e2b68fe","updated":"2025-08-13 10:06:43.000000000","message":"Done","commit_id":"ffd8c770b4b809f5f7c7e99ddaa2ad3f9d0a5d53"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"0a37e5576dda8cb35721f4b0ab46571412034d8e","unresolved":true,"context_lines":[{"line_number":405,"context_line":"            LOG.debug(src_type)"},{"line_number":406,"context_line":"            LOG.debug(\"%s %s\", dst_pool, dst_type)"},{"line_number":407,"context_line":""},{"line_number":408,"context_line":"            if self.is_available(volume):"},{"line_number":409,"context_line":"                if src_type \u003d\u003d dst_type:"},{"line_number":410,"context_line":"                    self._volume_migrate(volume, dst_pool)"},{"line_number":411,"context_line":"                else:"},{"line_number":412,"context_line":"                    self._volume_retype(volume, dst_type)"},{"line_number":413,"context_line":"            elif self.is_in_use(volume):"},{"line_number":414,"context_line":"                self._volume_update(volume, dst_type)"},{"line_number":415,"context_line":""},{"line_number":416,"context_line":"                # if with_attached_volume is True, migrate attaching instances"},{"line_number":417,"context_line":"                if self.with_attached_volume:"}],"source_content_type":"text/x-python","patch_set":4,"id":"a8c367f5_9e2b68fe","line":414,"range":{"start_line":408,"start_character":0,"end_line":414,"end_character":53},"in_reply_to":"efb12277_fe246017","updated":"2025-05-22 09:43:51.000000000","message":"Thanks for the summary Alfredo, you\u0027re correct that the curent approach creates suboptimal UX. Just a not about point 2, the volume migration in cinder does require a host, either through the host parameter or the cluster, which is why both are listed as optional in the api docs, but one of the two needs to be passed","commit_id":"ffd8c770b4b809f5f7c7e99ddaa2ad3f9d0a5d53"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"0ac94a66bf65cd39b4eaf842c5cbe4f976ba2f7e","unresolved":true,"context_lines":[{"line_number":409,"context_line":"                if src_type \u003d\u003d dst_type:"},{"line_number":410,"context_line":"                    if dst_pool is None:"},{"line_number":411,"context_line":"                        LOG.warning(\"volume %s will not be migrated because \""},{"line_number":412,"context_line":"                                    \"it\u0027s already in a pool of type %s and \""},{"line_number":413,"context_line":"                                    \"\u0027dst_pool\u0027 was not specified for src_pool\""},{"line_number":414,"context_line":"                                    \" %s\", volume.name, dst_type,"},{"line_number":415,"context_line":"                                    pool)"}],"source_content_type":"text/x-python","patch_set":7,"id":"0a976d42_a7e0016b","line":412,"range":{"start_line":412,"start_character":37,"end_line":412,"end_character":74},"updated":"2025-05-29 07:39:51.000000000","message":"This message is confusing for me. The relevant factor is that the volume already has the type dst_type, not that the pool is of a type. Note that multiple types can be assigned to a pool, but the only important factor is that the volume itself has the type.\n\nActually, strictly speaking, storage pools are not of a type. The volumes are in a type that can be assigned (or compatible) to one or more storage pools. That\u0027s why they are Volume Types, not Pool Types.","commit_id":"90bbdd89973dae0ae226ee8e697960fd2c41d72f"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"440eac356828aec52281ea042479afaa1ce8fcec","unresolved":false,"context_lines":[{"line_number":409,"context_line":"                if src_type \u003d\u003d dst_type:"},{"line_number":410,"context_line":"                    if dst_pool is None:"},{"line_number":411,"context_line":"                        LOG.warning(\"volume %s will not be migrated because \""},{"line_number":412,"context_line":"                                    \"it\u0027s already in a pool of type %s and \""},{"line_number":413,"context_line":"                                    \"\u0027dst_pool\u0027 was not specified for src_pool\""},{"line_number":414,"context_line":"                                    \" %s\", volume.name, dst_type,"},{"line_number":415,"context_line":"                                    pool)"}],"source_content_type":"text/x-python","patch_set":7,"id":"0c2d4b90_991b0a49","line":412,"range":{"start_line":412,"start_character":37,"end_line":412,"end_character":74},"in_reply_to":"03f17d1e_42998988","updated":"2025-08-13 10:06:43.000000000","message":"Done","commit_id":"90bbdd89973dae0ae226ee8e697960fd2c41d72f"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"de262338e3d9df09d8a8a35af35115911532621b","unresolved":true,"context_lines":[{"line_number":409,"context_line":"                if src_type \u003d\u003d dst_type:"},{"line_number":410,"context_line":"                    if dst_pool is None:"},{"line_number":411,"context_line":"                        LOG.warning(\"volume %s will not be migrated because \""},{"line_number":412,"context_line":"                                    \"it\u0027s already in a pool of type %s and \""},{"line_number":413,"context_line":"                                    \"\u0027dst_pool\u0027 was not specified for src_pool\""},{"line_number":414,"context_line":"                                    \" %s\", volume.name, dst_type,"},{"line_number":415,"context_line":"                                    pool)"}],"source_content_type":"text/x-python","patch_set":7,"id":"03f17d1e_42998988","line":412,"range":{"start_line":412,"start_character":37,"end_line":412,"end_character":74},"in_reply_to":"0a976d42_a7e0016b","updated":"2025-06-02 09:53:40.000000000","message":"ack, I rephrased the warning and hopefully now it\u0027s clearer, thanks for the clarification on volume types!","commit_id":"90bbdd89973dae0ae226ee8e697960fd2c41d72f"}],"watcher/tests/api/v1/test_audits.py":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"343964c26d74fc21242c83ccb970a957f2722fd2","unresolved":true,"context_lines":[{"line_number":1173,"context_line":"            \"audit:update\": \"rule:default\"})"},{"line_number":1174,"context_line":""},{"line_number":1175,"context_line":""},{"line_number":1176,"context_line":"class TestAuditZoneMigration(TestPost):"},{"line_number":1177,"context_line":"    def setUp(self):"},{"line_number":1178,"context_line":"        super(TestAuditZoneMigration, self).setUp()"},{"line_number":1179,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"c1e3d28f_233437ac","line":1176,"range":{"start_line":1176,"start_character":29,"end_line":1176,"end_character":37},"updated":"2025-08-05 17:16:22.000000000","message":"This will also inherit all tests from TestPost and run they again, under TestAuditZoneMigration class, without changing the behavior of the test in the end. I guess that you want to reuse the methods setUp and prepare_audit_*, so you could create a new base class for post or just copy the behavior here...","commit_id":"a67e3d827ac3659a90ae0b0787888bf2528c244f"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"440eac356828aec52281ea042479afaa1ce8fcec","unresolved":false,"context_lines":[{"line_number":1173,"context_line":"            \"audit:update\": \"rule:default\"})"},{"line_number":1174,"context_line":""},{"line_number":1175,"context_line":""},{"line_number":1176,"context_line":"class TestAuditZoneMigration(TestPost):"},{"line_number":1177,"context_line":"    def setUp(self):"},{"line_number":1178,"context_line":"        super(TestAuditZoneMigration, self).setUp()"},{"line_number":1179,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"339b53dd_b2d9b66a","line":1176,"range":{"start_line":1176,"start_character":29,"end_line":1176,"end_character":37},"in_reply_to":"c1e3d28f_233437ac","updated":"2025-08-13 10:06:43.000000000","message":"you\u0027re right, good catch, fixed in PS12, thanks!","commit_id":"a67e3d827ac3659a90ae0b0787888bf2528c244f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"22b64c0f07d3de45cecfba910dbae81cb03284c7","unresolved":false,"context_lines":[{"line_number":539,"context_line":""},{"line_number":540,"context_line":"    def _simulate_rpc_audit_create(self, audit):"},{"line_number":541,"context_line":"        audit.create()"},{"line_number":542,"context_line":"        return audit"},{"line_number":543,"context_line":""},{"line_number":544,"context_line":"    def setUp(self):"},{"line_number":545,"context_line":"        super(TestPostBase, self).setUp()"}],"source_content_type":"text/x-python","patch_set":15,"id":"19404d50_e4b9f401","line":542,"updated":"2025-09-11 20:30:20.000000000","message":"we shoudl revit this at a later date\nbecause it spossible to actully do RPCS iof we set it up right usign teh oslo.messaging in memory driver indated of rabbit for testing.\n\nbut this is how the test was workign before so that is out of scope of your change.","commit_id":"fe56660c444216bf235b2404ddeeba5b91c9cb8c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"22b64c0f07d3de45cecfba910dbae81cb03284c7","unresolved":true,"context_lines":[{"line_number":1255,"context_line":"        return audit_dict"},{"line_number":1256,"context_line":""},{"line_number":1257,"context_line":"    @mock.patch.object(deapi.DecisionEngineAPI, \u0027trigger_audit\u0027)"},{"line_number":1258,"context_line":"    def test_create_audit_zone_migration_without_dst_pool(self,"},{"line_number":1259,"context_line":"                                                          mock_trigger_audit):"},{"line_number":1260,"context_line":"        mock_trigger_audit.return_value \u003d mock.ANY"},{"line_number":1261,"context_line":"        zm_params \u003d {"},{"line_number":1262,"context_line":"            \u0027storage_pools\u0027: ["}],"source_content_type":"text/x-python","patch_set":15,"id":"0379e5c2_f24361ec","line":1259,"range":{"start_line":1258,"start_character":4,"end_line":1259,"end_character":78},"updated":"2025-09-11 20:30:20.000000000","message":"nit: we shoudl prefer this style.\n\n```suggestion\n    def test_create_audit_zone_migration_without_dst_pool(\n        self, mock_trigger_audit):\n```\n\nif it valid pep8 and passes ci then we coudl not really -1 for style things like thsi but please prefer to move all args to a new line if you need to wrap","commit_id":"fe56660c444216bf235b2404ddeeba5b91c9cb8c"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"22b64c0f07d3de45cecfba910dbae81cb03284c7","unresolved":false,"context_lines":[{"line_number":1269,"context_line":"        audit_input_dict \u003d self._prepare_audit_params(zm_params)"},{"line_number":1270,"context_line":""},{"line_number":1271,"context_line":"        response \u003d self.post_json(\u0027/audits\u0027, audit_input_dict)"},{"line_number":1272,"context_line":"        self.assertEqual(\"application/json\", response.content_type)"},{"line_number":1273,"context_line":"        self.assertEqual(HTTPStatus.CREATED, response.status_int)"},{"line_number":1274,"context_line":""},{"line_number":1275,"context_line":"    @mock.patch.object(deapi.DecisionEngineAPI, \u0027trigger_audit\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"41aafa3d_b4c0a813","line":1272,"range":{"start_line":1272,"start_character":8,"end_line":1272,"end_character":67},"updated":"2025-09-11 20:30:20.000000000","message":"sure we can assert this but it the only content_type we supprot so we dont really need to do this in every test.","commit_id":"fe56660c444216bf235b2404ddeeba5b91c9cb8c"}],"watcher/tests/decision_engine/strategy/strategies/test_zone_migration.py":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"4b0ded69e9cb54cfa473daf85fc5f2936e2ccbc3","unresolved":true,"context_lines":[{"line_number":301,"context_line":"        global_efficacy_value \u003d solution.global_efficacy[2].get(\u0027value\u0027, 0)"},{"line_number":302,"context_line":"        self.assertEqual(100, global_efficacy_value)"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"    def test_execute_migrate_volume_no_dst_pool(self):"},{"line_number":305,"context_line":"        volume_on_src1 \u003d self.fake_volume(host\u003d\"src1@back1#pool1\","},{"line_number":306,"context_line":"                                          id\u003dvolume_uuid_mapping[\"volume_1\"],"},{"line_number":307,"context_line":"                                          name\u003d\"volume_1\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"8bb27425_5b0693c0","side":"PARENT","line":304,"range":{"start_line":304,"start_character":0,"end_line":304,"end_character":54},"updated":"2025-05-21 15:05:24.000000000","message":"ack, now dst_pool is required and validated by its schema, in strategy context. We may be missing some testing on strategy context side I think, but that\u0027s another issue to solve.","commit_id":"26e36e1620bdf2433d2aaadafce612271b0497a3"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"440eac356828aec52281ea042479afaa1ce8fcec","unresolved":false,"context_lines":[{"line_number":301,"context_line":"        global_efficacy_value \u003d solution.global_efficacy[2].get(\u0027value\u0027, 0)"},{"line_number":302,"context_line":"        self.assertEqual(100, global_efficacy_value)"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"    def test_execute_migrate_volume_no_dst_pool(self):"},{"line_number":305,"context_line":"        volume_on_src1 \u003d self.fake_volume(host\u003d\"src1@back1#pool1\","},{"line_number":306,"context_line":"                                          id\u003dvolume_uuid_mapping[\"volume_1\"],"},{"line_number":307,"context_line":"                                          name\u003d\"volume_1\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"b321b54f_55ac00b0","side":"PARENT","line":304,"range":{"start_line":304,"start_character":0,"end_line":304,"end_character":54},"in_reply_to":"4fb4873e_cef0112c","updated":"2025-08-13 10:06:43.000000000","message":"Done","commit_id":"26e36e1620bdf2433d2aaadafce612271b0497a3"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"6b08c91316b05803e32892cd2d8a19fd371f200c","unresolved":true,"context_lines":[{"line_number":301,"context_line":"        global_efficacy_value \u003d solution.global_efficacy[2].get(\u0027value\u0027, 0)"},{"line_number":302,"context_line":"        self.assertEqual(100, global_efficacy_value)"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"    def test_execute_migrate_volume_no_dst_pool(self):"},{"line_number":305,"context_line":"        volume_on_src1 \u003d self.fake_volume(host\u003d\"src1@back1#pool1\","},{"line_number":306,"context_line":"                                          id\u003dvolume_uuid_mapping[\"volume_1\"],"},{"line_number":307,"context_line":"                                          name\u003d\"volume_1\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"4fb4873e_cef0112c","side":"PARENT","line":304,"range":{"start_line":304,"start_character":0,"end_line":304,"end_character":54},"in_reply_to":"8bb27425_5b0693c0","updated":"2025-05-21 17:35:20.000000000","message":"I\u0027ve added a unit test to validate the schema change, tomorrow I\u0027ll try to come back and add more test cases to test more parameters","commit_id":"26e36e1620bdf2433d2aaadafce612271b0497a3"}]}