)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"ef0286a8110cfd0e8d3a907c3f64bc5e7aca023c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"0d267ec5_c7c86b0c","updated":"2025-06-03 11:04:03.000000000","message":"I think this may introduce counting twice some migrations.","commit_id":"4ade40692990416ba0b803cd5eb2d8c75cff69f4"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"03f1acd864de7c56b1f8da8006240026b88bc869","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d8b1cd4e_d890fdf1","updated":"2025-06-02 09:51:53.000000000","message":"check-rdo","commit_id":"4ade40692990416ba0b803cd5eb2d8c75cff69f4"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"32a95e20582da3f6c34ca17b51b893c659382298","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"245930da_f25a33c4","updated":"2025-06-05 18:04:28.000000000","message":"I think that your fix makes sense. We could eventually add a tempest test for this scenario. Do you think that is possible? Thanks","commit_id":"81ce8640ce24c500f59399cda34aa3ba1e85fabf"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"e660bcdca72ea53d67a36acaec8e7ff88272bf54","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"a6c9249e_16a75d37","in_reply_to":"245930da_f25a33c4","updated":"2025-06-06 06:19:28.000000000","message":"Yes, I think it should be possible, we don\u0027t currently have any test for the volume migration part, but it\u0027s planned to add more tempest testing for this strategy","commit_id":"81ce8640ce24c500f59399cda34aa3ba1e85fabf"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"304fda47eb8e41b7de8899544ddeb3861ee031ff","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":7,"id":"93c05a94_f617ed9c","updated":"2025-06-19 16:21:30.000000000","message":"I think the behavior is now more accurate.","commit_id":"7f307b54aee72747bd00680899ca5d09956a9a7d"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"48bff1f4265bf234de1bbe18d31d073d9d576b6d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"2234f727_c1375a0c","updated":"2025-09-02 12:41:34.000000000","message":"Fix lgtm, thanks!","commit_id":"1690012d0f440f253a1c277d53307441a3be2aa3"}],"releasenotes/notes/zone-migration-with-attached-volume-without-compute-nodes-ec4b1329e2b58279.yaml":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b70c9964f299bfc9a8593d051f4d7310b6b3df42","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    The zone migration strategy no longer fails when when an audit is created"},{"line_number":5,"context_line":"    with defined storage_pools but no compute_nodes parameters, and"},{"line_number":6,"context_line":"    with_attached_volume is set to True. The strategy now creates the required"},{"line_number":7,"context_line":"    instance migrations for the instances that have the volumes to migrate attached,"},{"line_number":8,"context_line":"    but relies on Nova to find a suitable destination."},{"line_number":9,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":3,"id":"9a8aeaef_b609c2a2","line":6,"range":{"start_line":4,"start_character":2,"end_line":6,"end_character":40},"updated":"2025-06-13 11:31:10.000000000","message":"this could be improved.\n\ndid it only fail \"with defined storage_pools but no compute_nodes parameters,\"\nand alos fail with with_attached_volume is set to True?\n\n\n\n```suggestion\n    The zone migration strategy no longer fails when storage_pools is defiend,\n    compute_nodes is not provided, and with_attached_volume is set to True.\n    The strategy now creates the required ...\n```","commit_id":"81ce8640ce24c500f59399cda34aa3ba1e85fabf"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"84e1504f3516fc2e9d8511d92dbc5125019f47d3","unresolved":true,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    The zone migration strategy no longer fails when when an audit is created"},{"line_number":5,"context_line":"    with defined storage_pools but no compute_nodes parameters, and"},{"line_number":6,"context_line":"    with_attached_volume is set to True. The strategy now creates the required"},{"line_number":7,"context_line":"    instance migrations for the instances that have the volumes to migrate attached,"},{"line_number":8,"context_line":"    but relies on Nova to find a suitable destination."},{"line_number":9,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":3,"id":"e224a446_d2099c38","line":6,"range":{"start_line":4,"start_character":2,"end_line":6,"end_character":40},"in_reply_to":"9a8aeaef_b609c2a2","updated":"2025-06-13 14:20:10.000000000","message":"ack, I see how it could be ambiguous, I rephrased is a bit and hopefully is clearer now","commit_id":"81ce8640ce24c500f59399cda34aa3ba1e85fabf"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"a9611b50c4621cdc1c75629afa78ec62b41bd7bf","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - |"},{"line_number":4,"context_line":"    The zone migration strategy no longer fails when when an audit is created"},{"line_number":5,"context_line":"    with defined storage_pools but no compute_nodes parameters, and"},{"line_number":6,"context_line":"    with_attached_volume is set to True. The strategy now creates the required"},{"line_number":7,"context_line":"    instance migrations for the instances that have the volumes to migrate attached,"},{"line_number":8,"context_line":"    but relies on Nova to find a suitable destination."},{"line_number":9,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":3,"id":"f36ccbee_21d5dfe2","line":6,"range":{"start_line":4,"start_character":2,"end_line":6,"end_character":40},"in_reply_to":"e224a446_d2099c38","updated":"2025-06-17 13:34:45.000000000","message":"Done","commit_id":"81ce8640ce24c500f59399cda34aa3ba1e85fabf"}],"watcher/decision_engine/strategy/strategies/zone_migration.py":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"ef0286a8110cfd0e8d3a907c3f64bc5e7aca023c","unresolved":true,"context_lines":[{"line_number":341,"context_line":"                        if self.is_live(instance):"},{"line_number":342,"context_line":"                            self.live_count +\u003d 1"},{"line_number":343,"context_line":"                        elif self.is_cold(instance):"},{"line_number":344,"context_line":"                            self.cold_count +\u003d 1"},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"    def is_live(self, instance):"},{"line_number":347,"context_line":"        status \u003d getattr(instance, \u0027status\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"f468c524_14c2d1a1","line":344,"updated":"2025-06-03 11:04:03.000000000","message":"I think this may double count instances which are already included in:\n\n```\n        for instance in targets.get(\u0027instance\u0027, []):\n            if self.is_live(instance):\n                self.live_count +\u003d 1\n            elif self.is_cold(instance):\n                self.cold_count +\u003d 1\n```\n\nWe should somehow check that this instances are not counted already.","commit_id":"4ade40692990416ba0b803cd5eb2d8c75cff69f4"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"46d6a8fad6932d415c5916f1d46353d5b41e71f7","unresolved":false,"context_lines":[{"line_number":341,"context_line":"                        if self.is_live(instance):"},{"line_number":342,"context_line":"                            self.live_count +\u003d 1"},{"line_number":343,"context_line":"                        elif self.is_cold(instance):"},{"line_number":344,"context_line":"                            self.cold_count +\u003d 1"},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"    def is_live(self, instance):"},{"line_number":347,"context_line":"        status \u003d getattr(instance, \u0027status\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"892272c9_e64dc4a0","line":344,"in_reply_to":"1dc5cd1f_5ccc2e66","updated":"2025-06-06 07:36:54.000000000","message":"Done","commit_id":"4ade40692990416ba0b803cd5eb2d8c75cff69f4"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"a93940ed0708a7b2d5838aba71f86b00d0050d9c","unresolved":true,"context_lines":[{"line_number":341,"context_line":"                        if self.is_live(instance):"},{"line_number":342,"context_line":"                            self.live_count +\u003d 1"},{"line_number":343,"context_line":"                        elif self.is_cold(instance):"},{"line_number":344,"context_line":"                            self.cold_count +\u003d 1"},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"    def is_live(self, instance):"},{"line_number":347,"context_line":"        status \u003d getattr(instance, \u0027status\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1dc5cd1f_5ccc2e66","line":344,"in_reply_to":"f468c524_14c2d1a1","updated":"2025-06-03 15:20:42.000000000","message":"right, I forgot to come back to this, in PS 3 I changed the code to build a dictionary of migrated instances to avoid double counting, and added a few unit tests for this method","commit_id":"4ade40692990416ba0b803cd5eb2d8c75cff69f4"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"cbc2c44f0b00d7b787e5ab7d3ad2943f74356b88","unresolved":true,"context_lines":[{"line_number":334,"context_line":"                    instances \u003d [self.nova.find_instance(dic.get(\u0027server_id\u0027))"},{"line_number":335,"context_line":"                                 for dic in volume.attachments]"},{"line_number":336,"context_line":"                    for instance in instances:"},{"line_number":337,"context_line":"                        instances_to_migrate[instance.id] \u003d instance"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        instances_targets \u003d targets.get(\u0027instance\u0027, [])"},{"line_number":340,"context_line":"        instances_to_migrate.update({"}],"source_content_type":"text/x-python","patch_set":3,"id":"a967a5dd_06327bb5","line":337,"range":{"start_line":337,"start_character":0,"end_line":337,"end_character":2},"updated":"2025-06-06 07:13:56.000000000","message":"This should work, but a simpler approach may be to just check if the instance is running in a host which is listed as a src_node in the compute_nodes list in audit config. Do we have the host in the instance object as returned by find_instance?.","commit_id":"81ce8640ce24c500f59399cda34aa3ba1e85fabf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0609f5f7a041900f7664cb1b86ea9a42cc1e4023","unresolved":true,"context_lines":[{"line_number":334,"context_line":"                    instances \u003d [self.nova.find_instance(dic.get(\u0027server_id\u0027))"},{"line_number":335,"context_line":"                                 for dic in volume.attachments]"},{"line_number":336,"context_line":"                    for instance in instances:"},{"line_number":337,"context_line":"                        instances_to_migrate[instance.id] \u003d instance"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        instances_targets \u003d targets.get(\u0027instance\u0027, [])"},{"line_number":340,"context_line":"        instances_to_migrate.update({"}],"source_content_type":"text/x-python","patch_set":3,"id":"4f9544e3_51b24d09","line":337,"range":{"start_line":337,"start_character":0,"end_line":337,"end_character":2},"in_reply_to":"2eaf2eec_b01f1f21","updated":"2025-06-17 11:47:11.000000000","message":"`with_attached_volume` should only affect the ordering of the computed actions, not whether an action is done.\n\n`with_attached_volume` shoudl only  have an effect if you are migrating instnaces\n\nso it shoudl only have an effect if compute nodes are defeind.\n\nit shoudl have no effect if only stroage backend are refenrced.","commit_id":"81ce8640ce24c500f59399cda34aa3ba1e85fabf"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"f6c17106df29e476276ac48739e5879765ecd8e4","unresolved":true,"context_lines":[{"line_number":334,"context_line":"                    instances \u003d [self.nova.find_instance(dic.get(\u0027server_id\u0027))"},{"line_number":335,"context_line":"                                 for dic in volume.attachments]"},{"line_number":336,"context_line":"                    for instance in instances:"},{"line_number":337,"context_line":"                        instances_to_migrate[instance.id] \u003d instance"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        instances_targets \u003d targets.get(\u0027instance\u0027, [])"},{"line_number":340,"context_line":"        instances_to_migrate.update({"}],"source_content_type":"text/x-python","patch_set":3,"id":"d841f233_306babf8","line":337,"range":{"start_line":337,"start_character":0,"end_line":337,"end_character":2},"in_reply_to":"2eaf2eec_b01f1f21","updated":"2025-06-17 10:42:51.000000000","message":"you\u0027re right, after the dicussion last week I updated the patch to decouple the migrations, but I meant to revisit the patch beause I also noticed the problem you mention. I\u0027ll try to update the patch today.\n\nI agree with you that with you other patch, this parameter is now much less useful, however I think we should still keep it, and I think it should fairly easy to maintain its functionality, while only migrating the instances that the user has requested.","commit_id":"81ce8640ce24c500f59399cda34aa3ba1e85fabf"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"b954228fda07340f50107d5e6f65a3a1acdad028","unresolved":true,"context_lines":[{"line_number":334,"context_line":"                    instances \u003d [self.nova.find_instance(dic.get(\u0027server_id\u0027))"},{"line_number":335,"context_line":"                                 for dic in volume.attachments]"},{"line_number":336,"context_line":"                    for instance in instances:"},{"line_number":337,"context_line":"                        instances_to_migrate[instance.id] \u003d instance"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        instances_targets \u003d targets.get(\u0027instance\u0027, [])"},{"line_number":340,"context_line":"        instances_to_migrate.update({"}],"source_content_type":"text/x-python","patch_set":3,"id":"abaa1a77_90224a0b","line":337,"range":{"start_line":337,"start_character":0,"end_line":337,"end_character":2},"in_reply_to":"389142f2_7ed611a6","updated":"2025-06-11 08:19:50.000000000","message":"You are correct, thanks!","commit_id":"81ce8640ce24c500f59399cda34aa3ba1e85fabf"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"48b020706cdf1548900c9b2d6fb7bdd44c1ed832","unresolved":true,"context_lines":[{"line_number":334,"context_line":"                    instances \u003d [self.nova.find_instance(dic.get(\u0027server_id\u0027))"},{"line_number":335,"context_line":"                                 for dic in volume.attachments]"},{"line_number":336,"context_line":"                    for instance in instances:"},{"line_number":337,"context_line":"                        instances_to_migrate[instance.id] \u003d instance"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        instances_targets \u003d targets.get(\u0027instance\u0027, [])"},{"line_number":340,"context_line":"        instances_to_migrate.update({"}],"source_content_type":"text/x-python","patch_set":3,"id":"2eaf2eec_b01f1f21","line":337,"range":{"start_line":337,"start_character":0,"end_line":337,"end_character":2},"in_reply_to":"4301a82a_2cd06471","updated":"2025-06-17 07:00:05.000000000","message":"I\u0027m lost now. I see in your last patchset, you totally removed the `if self.with_attached_volume` conditional so now that parameter is actually doing nothing, right?. \n\nMy understanding of the way the with_attached_volume was documented:\n\nFalse: Instances will migrate after all volumes migrate.\nTrue: An instance will migrate after the attached volumes migrate.\n\nis that it\u0027s related to the fact that the strategy didn\u0027t mix volume and instances migration in the past (that we are addressing in https://review.opendev.org/c/openstack/watcher/+/952116 ) and the fact that this was being executed as a continous strategy (we are inferring this because of multiple implementation details along the strategy). The `will migrate after all volumes migrate` actually meant in different strategy executions, it is unrelated to the order in the same actionplan (where iiuc the volumes are always migrated first). So, the way this was working was, running the strategy as a continuous one, first executions of the audit did only migrate volumes *and* the attached instances only if `with_attached_volume` was enabled. Only when all the volumes were migrated or retyped, the following executions of the audit did the instances migrations.\n\nSo, we need to think the behavior we intend for with_attached_volumes assuming it will be able to migrate volumes and instances in the same run (as it will be with https://review.opendev.org/c/openstack/watcher/+/952116). With this PS, the strategy will not longer create instances migrations as part of the volume migration calculation for instances attached to the migrated volumes, but also will not create migrations for the instances until we also merge #952116. Assuming, that we merge both, instances migrations would be calculated only based on the compute node lists, and `with_attached_volumes` parameter is not used anywhere. Is that the intention? if so, we should document it. btw, in that case we should also remove https://github.com/openstack/watcher/blob/master/watcher/decision_engine/strategy/strategies/zone_migration.py#L292-L296 in this patch or rebase it on #952116.\n\nFor me the question is, assumging we merge #952116, does the with_attached_volumes parameter, as documented, makes any sense? the only case that i can think of it\u0027s the case where the number of instances to be migrated is \u003e parallel_total or parallel_per_host an in that case enabling with_attached_volume would enforce migrating the attached instances to the volumes to be included in the plan before the rest of them. For that, we\u0027d need to keep the current code, but improve the instance_migration method to just ignore instances in the case where the host where the instance is running is not included as src_node in the compute_nodes parameter, would that make sense?","commit_id":"81ce8640ce24c500f59399cda34aa3ba1e85fabf"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"a9611b50c4621cdc1c75629afa78ec62b41bd7bf","unresolved":true,"context_lines":[{"line_number":334,"context_line":"                    instances \u003d [self.nova.find_instance(dic.get(\u0027server_id\u0027))"},{"line_number":335,"context_line":"                                 for dic in volume.attachments]"},{"line_number":336,"context_line":"                    for instance in instances:"},{"line_number":337,"context_line":"                        instances_to_migrate[instance.id] \u003d instance"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        instances_targets \u003d targets.get(\u0027instance\u0027, [])"},{"line_number":340,"context_line":"        instances_to_migrate.update({"}],"source_content_type":"text/x-python","patch_set":3,"id":"6f3a3074_be96e4b5","line":337,"range":{"start_line":337,"start_character":0,"end_line":337,"end_character":2},"in_reply_to":"4f9544e3_51b24d09","updated":"2025-06-17 13:34:45.000000000","message":"patcheset 4 should do this now, if there are no `compute_nodes` in the user input, no instances will be migrated, regardless of the value of `with_attached_volume`","commit_id":"81ce8640ce24c500f59399cda34aa3ba1e85fabf"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"84e1504f3516fc2e9d8511d92dbc5125019f47d3","unresolved":true,"context_lines":[{"line_number":334,"context_line":"                    instances \u003d [self.nova.find_instance(dic.get(\u0027server_id\u0027))"},{"line_number":335,"context_line":"                                 for dic in volume.attachments]"},{"line_number":336,"context_line":"                    for instance in instances:"},{"line_number":337,"context_line":"                        instances_to_migrate[instance.id] \u003d instance"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        instances_targets \u003d targets.get(\u0027instance\u0027, [])"},{"line_number":340,"context_line":"        instances_to_migrate.update({"}],"source_content_type":"text/x-python","patch_set":3,"id":"4301a82a_2cd06471","line":337,"range":{"start_line":337,"start_character":0,"end_line":337,"end_character":2},"in_reply_to":"7a6a19c4_8e7a30aa","updated":"2025-06-13 14:20:10.000000000","message":"ack noted about the distinction  between hypervisor_hostname and host, thanks for that.\n\nAbout the priority, you\u0027re correct, I wrongly assumed it would filter out nodes or volumes, but it does not, I\u0027ll look into implementing Alfredo\u0027s suggestion to simplify.\n\nFinally, about `with_attached_volume`, I agree that the behaviour is confusing and unexpected, as discussed elsewhere, I\u0027ll change the patch to decouple instance and volume migrations, and that should also simplify the code that counts the migrations","commit_id":"81ce8640ce24c500f59399cda34aa3ba1e85fabf"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"46d6a8fad6932d415c5916f1d46353d5b41e71f7","unresolved":true,"context_lines":[{"line_number":334,"context_line":"                    instances \u003d [self.nova.find_instance(dic.get(\u0027server_id\u0027))"},{"line_number":335,"context_line":"                                 for dic in volume.attachments]"},{"line_number":336,"context_line":"                    for instance in instances:"},{"line_number":337,"context_line":"                        instances_to_migrate[instance.id] \u003d instance"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        instances_targets \u003d targets.get(\u0027instance\u0027, [])"},{"line_number":340,"context_line":"        instances_to_migrate.update({"}],"source_content_type":"text/x-python","patch_set":3,"id":"389142f2_7ed611a6","line":337,"range":{"start_line":337,"start_character":0,"end_line":337,"end_character":2},"in_reply_to":"a967a5dd_06327bb5","updated":"2025-06-06 07:36:54.000000000","message":"I don\u0027t think that would work in all cases, because not all instances from all src_node in the input will be added to the targets. If the user sets the `prioriy` parameter, some might be filtered out.\n\nOut of curiosity, I checked and we could indeed retrive the host from the instance object, with `getattr(server, \u0027OS-EXT-SRV-ATTR:hypervisor_hostname\u0027)`","commit_id":"81ce8640ce24c500f59399cda34aa3ba1e85fabf"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"b70c9964f299bfc9a8593d051f4d7310b6b3df42","unresolved":true,"context_lines":[{"line_number":334,"context_line":"                    instances \u003d [self.nova.find_instance(dic.get(\u0027server_id\u0027))"},{"line_number":335,"context_line":"                                 for dic in volume.attachments]"},{"line_number":336,"context_line":"                    for instance in instances:"},{"line_number":337,"context_line":"                        instances_to_migrate[instance.id] \u003d instance"},{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        instances_targets \u003d targets.get(\u0027instance\u0027, [])"},{"line_number":340,"context_line":"        instances_to_migrate.update({"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a6a19c4_8e7a30aa","line":337,"range":{"start_line":337,"start_character":0,"end_line":337,"end_character":2},"in_reply_to":"abaa1a77_90224a0b","updated":"2025-06-13 11:31:10.000000000","message":"i have not looked at the priorty code btu i would fid it surpsing if it elimianted candiates.\n\nprioriy is really just ment to influcne order but we can revistit that later.\n\n\nthe documentaiton for `with_attached_volume`  \n\n```\nFalse: Instances will migrate after all volumes migrate.\nTrue: An instance will migrate after the attached volumes migrate.\n```\n\nthat means that we will migrate the instnace and the voluem regardless of if its True or false\n\nso the count should not change.\n\nonly the order.\n\nso it feels like there is something incorrect here.\n\n`getattr(server, \u0027OS-EXT-SRV-ATTR:hypervisor_hostname\u0027)`\nand\n`getattr(server, \u0027OS-EXT-SRV-ATTR:host\u0027)`\n\nare two very diffent things\n\nthey can and often ar the same but we cannot assume that the are.\n\nthe host value is what we need when cold/live migrating\n\nit is used as the rpc endpoing for the comptue service that is managing the workload amoung other things\n\nthe hypervior_hostname is the value reported by the virt driver ake what virsh hostname returns in the case of libvirt.\n\nthis is not guarenteed to be the same as host.\n\nthe hypervior_hostname is what we shoudl use for metrics form promethous for host level metrics.\n\nwe should try to use those two values diffenrtly because somethimes the difference is important.","commit_id":"81ce8640ce24c500f59399cda34aa3ba1e85fabf"}],"watcher/tests/decision_engine/strategy/strategies/test_zone_migration.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"aa5998633bbbf46bedf3a56c009b5669bbf2fe47","unresolved":true,"context_lines":[{"line_number":901,"context_line":"        self.m_n_helper.find_instance.return_value \u003d instance_on_src1"},{"line_number":902,"context_line":"        targets \u003d {\u0027volume\u0027: [volume_on_src1]}"},{"line_number":903,"context_line":"        self.strategy.set_migration_count(targets)"},{"line_number":904,"context_line":"        self.assertEqual(0, self.strategy.live_count)"},{"line_number":905,"context_line":"        self.assertEqual(0, self.strategy.cold_count)"},{"line_number":906,"context_line":"        self.assertEqual(1, self.strategy.volume_update_count)"},{"line_number":907,"context_line":"        self.assertEqual(0, self.strategy.volume_count)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9e48177c_7060037f","line":905,"range":{"start_line":904,"start_character":0,"end_line":905,"end_character":53},"updated":"2025-06-13 11:38:07.000000000","message":"the -1 is kind of because of this.\n\nthere si a conflict between how that parmater is defiend and what your trying to do\n\nyou are trying to allow usign the zone migration strage to only migrate the volumes\nregardless of if they are attached to vms.\n\n`with_attached_volume` is not `should` the instnace be migrated its `when` will they be migrated.\n\nso we cant really use it ot alter this behaivor.","commit_id":"81ce8640ce24c500f59399cda34aa3ba1e85fabf"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"a9611b50c4621cdc1c75629afa78ec62b41bd7bf","unresolved":false,"context_lines":[{"line_number":901,"context_line":"        self.m_n_helper.find_instance.return_value \u003d instance_on_src1"},{"line_number":902,"context_line":"        targets \u003d {\u0027volume\u0027: [volume_on_src1]}"},{"line_number":903,"context_line":"        self.strategy.set_migration_count(targets)"},{"line_number":904,"context_line":"        self.assertEqual(0, self.strategy.live_count)"},{"line_number":905,"context_line":"        self.assertEqual(0, self.strategy.cold_count)"},{"line_number":906,"context_line":"        self.assertEqual(1, self.strategy.volume_update_count)"},{"line_number":907,"context_line":"        self.assertEqual(0, self.strategy.volume_count)"}],"source_content_type":"text/x-python","patch_set":3,"id":"76495c26_05a85569","line":905,"range":{"start_line":904,"start_character":0,"end_line":905,"end_character":53},"in_reply_to":"9e48177c_7060037f","updated":"2025-06-17 13:34:45.000000000","message":"I removed these tests in patchset 4, since the set_migration_count is no longer modified in this patch","commit_id":"81ce8640ce24c500f59399cda34aa3ba1e85fabf"}]}
