)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":3217,"name":"Ian Wells","username":"ijw-ubuntu"},"change_message_id":"c2634fc78b6ed487b1b38567d0f104d012d0cae9","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Fix incorrect port unbind request in nova on instance delete"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"During instance delete nova should only attempt to unbind a port"},{"line_number":10,"context_line":"if neutron sees any ports bound to this instance"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Without the conditional check nova may unbind wrong ports"},{"line_number":13,"context_line":"Neutron allows ports to be unbound and bound to a different instance"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"fa6399be_e059c45c","line":10,"updated":"2016-10-19 19:48:48.000000000","message":"This addresses the case where the port has administratively been unbound from the VM being deleted, I think, so this message could be a bit clearer.\n\nWhere a port is now *not* currently bound to this VM (whether it\u0027s bound elsewhere or not bound at all) we should skip the unbinding step because it risks detaching the port from wherever it\u0027s now attached to - and this is a valuable use case, not a weird corner case, because it allows the port of a VM to be re-used even if the VM delete takes time (e.g. because the compute node went down).","commit_id":"98887cf08a2d546461046dd693ec25ba17d09e25"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"39cdef52e2e78866a83120bc83ae3553e6cdb0b0","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     ijw-ubuntu \u003ciawells@cisco.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-11-16 20:37:55 +0000"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Fix incorrect port unbind request in nova on instance delete"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This addresses the case where the port has administratively been unbound"},{"line_number":10,"context_line":"from the VM being deleted."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"7a77a97e_654e9e97","line":7,"updated":"2016-11-16 20:58:27.000000000","message":"Don\u0027t have to mention nova here since the patch is submitted against the nova repo.","commit_id":"e7b0642845b2669adc6d2c98f4adc2a4cecb6461"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"39cdef52e2e78866a83120bc83ae3553e6cdb0b0","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This addresses the case where the port has administratively been unbound"},{"line_number":10,"context_line":"from the VM being deleted."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Where a port is now *not* currently bound to this VM"},{"line_number":13,"context_line":"(whether it\u0027s bound elsewhere or not bound at all) we should"},{"line_number":14,"context_line":"skip the unbinding step because it risks detaching the port"},{"line_number":15,"context_line":"from wherever it\u0027s now attached to - and this is a valuable"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"7a77a97e_4519a287","line":12,"updated":"2016-11-16 20:58:27.000000000","message":"s/Where a port is now *not* currently bound/When a port is *not* currently bound/","commit_id":"e7b0642845b2669adc6d2c98f4adc2a4cecb6461"},{"author":{"_account_id":3217,"name":"Ian Wells","username":"ijw-ubuntu"},"change_message_id":"aedaf3d35e0d2e502662be1fbcbc53e7e453a843","unresolved":false,"context_lines":[{"line_number":9,"context_line":"This addresses the case where the port has administratively been unbound"},{"line_number":10,"context_line":"from the VM being deleted."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Where a port is now *not* currently bound to this VM"},{"line_number":13,"context_line":"(whether it\u0027s bound elsewhere or not bound at all) we should"},{"line_number":14,"context_line":"skip the unbinding step because it risks detaching the port"},{"line_number":15,"context_line":"from wherever it\u0027s now attached to - and this is a valuable"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"7a77a97e_8728b09b","line":12,"in_reply_to":"7a77a97e_4519a287","updated":"2016-11-18 23:40:08.000000000","message":"The point was to emphasise that at the point of deletion it was not bound, though it may have been previously.","commit_id":"e7b0642845b2669adc6d2c98f4adc2a4cecb6461"},{"author":{"_account_id":17973,"name":"Ludovic Beliveau","email":"lbeliveau@gmail.com","username":"lbeliveau"},"change_message_id":"39cdef52e2e78866a83120bc83ae3553e6cdb0b0","unresolved":false,"context_lines":[{"line_number":12,"context_line":"Where a port is now *not* currently bound to this VM"},{"line_number":13,"context_line":"(whether it\u0027s bound elsewhere or not bound at all) we should"},{"line_number":14,"context_line":"skip the unbinding step because it risks detaching the port"},{"line_number":15,"context_line":"from wherever it\u0027s now attached to - and this is a valuable"},{"line_number":16,"context_line":"use case, not a weird corner case, because it allows the port"},{"line_number":17,"context_line":"of a VM to be re-used even if the VM delete takes time"},{"line_number":18,"context_line":"(e.g. because the compute node went down)."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"7a77a97e_e55aae4f","line":15,"updated":"2016-11-16 20:58:27.000000000","message":"s/valuable/valid/","commit_id":"e7b0642845b2669adc6d2c98f4adc2a4cecb6461"},{"author":{"_account_id":3217,"name":"Ian Wells","username":"ijw-ubuntu"},"change_message_id":"aedaf3d35e0d2e502662be1fbcbc53e7e453a843","unresolved":false,"context_lines":[{"line_number":12,"context_line":"Where a port is now *not* currently bound to this VM"},{"line_number":13,"context_line":"(whether it\u0027s bound elsewhere or not bound at all) we should"},{"line_number":14,"context_line":"skip the unbinding step because it risks detaching the port"},{"line_number":15,"context_line":"from wherever it\u0027s now attached to - and this is a valuable"},{"line_number":16,"context_line":"use case, not a weird corner case, because it allows the port"},{"line_number":17,"context_line":"of a VM to be re-used even if the VM delete takes time"},{"line_number":18,"context_line":"(e.g. because the compute node went down)."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"7a77a97e_e7524434","line":15,"in_reply_to":"7a77a97e_e55aae4f","updated":"2016-11-18 23:40:08.000000000","message":"No, this is correct as it is: \u0027valuable\u0027, as in worthwhile.","commit_id":"e7b0642845b2669adc6d2c98f4adc2a4cecb6461"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2f64056ffa1fc7d2a7107b294f20c48ccb11954f","unresolved":false,"context_lines":[{"line_number":15,"context_line":"the port from wherever it\u0027s now attached to.  This is a valuable"},{"line_number":16,"context_line":"use case, not a weird corner case, because it allows the port"},{"line_number":17,"context_line":"of a VM to be re-used even if the VM delete takes time"},{"line_number":18,"context_line":"(e.g. because the compute node went down)."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Closes-Bug: #1634269"},{"line_number":21,"context_line":"Change-Id: I16187e7b01b9158114a3ec5d79728c3d08cccbcb"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"9a57fde8_7f821b02","line":18,"updated":"2017-01-12 19:54:00.000000000","message":"I don\u0027t actually see this as a non-corner case. Ports are cheap. Why re-use them between VMs anyway?","commit_id":"6f1fe8da8106d91d7c190268aec1e0896d0cf811"},{"author":{"_account_id":3217,"name":"Ian Wells","username":"ijw-ubuntu"},"change_message_id":"3a0d014fbdb51dc1ca147464de6daf20c8b76783","unresolved":false,"context_lines":[{"line_number":15,"context_line":"the port from wherever it\u0027s now attached to.  This is a valuable"},{"line_number":16,"context_line":"use case, not a weird corner case, because it allows the port"},{"line_number":17,"context_line":"of a VM to be re-used even if the VM delete takes time"},{"line_number":18,"context_line":"(e.g. because the compute node went down)."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Closes-Bug: #1634269"},{"line_number":21,"context_line":"Change-Id: I16187e7b01b9158114a3ec5d79728c3d08cccbcb"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7a3c09a3_e8fd8f6f","line":18,"in_reply_to":"9a57fde8_7f821b02","updated":"2017-01-12 23:13:36.000000000","message":"\"because it allows the port of a VM to be re-used even if the VM delete takes time\" - ports are cheap, indeed, but this is the port holding the address and/or MAC that you want to reuse.  If you delete the port, you can lose the address before you get the chance to make a new one.  (Much the same reason that live migration is going the path of multiple-binds rather than second-ports.)","commit_id":"6f1fe8da8106d91d7c190268aec1e0896d0cf811"}],"nova/network/neutronv2/api.py":[{"author":{"_account_id":3217,"name":"Ian Wells","username":"ijw-ubuntu"},"change_message_id":"c2634fc78b6ed487b1b38567d0f104d012d0cae9","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"        ports \u003d set(ports) - ports_to_skip"},{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":"        # Reset device_id and device_owner for the ports that are skipped"},{"line_number":1173,"context_line":"        # Only unbind port if neutron sees this port as bound"},{"line_number":1174,"context_line":"        if data.get(\u0027ports\u0027, []):"},{"line_number":1175,"context_line":"            self._unbind_ports(context, ports_to_skip, neutron)"},{"line_number":1176,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa6399be_c054c055","line":1173,"updated":"2016-10-19 19:48:48.000000000","message":"If I understand, I think you want to only unbind the ports that are still bound *to this VM* (as they may have been manually unbound and bound to another VM).  If so, the comment needs updating.","commit_id":"98887cf08a2d546461046dd693ec25ba17d09e25"},{"author":{"_account_id":3217,"name":"Ian Wells","username":"ijw-ubuntu"},"change_message_id":"c2634fc78b6ed487b1b38567d0f104d012d0cae9","unresolved":false,"context_lines":[{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":"        # Reset device_id and device_owner for the ports that are skipped"},{"line_number":1173,"context_line":"        # Only unbind port if neutron sees this port as bound"},{"line_number":1174,"context_line":"        if data.get(\u0027ports\u0027, []):"},{"line_number":1175,"context_line":"            self._unbind_ports(context, ports_to_skip, neutron)"},{"line_number":1176,"context_line":"        else:"},{"line_number":1177,"context_line":"            LOG.debug(\u0027Skipping port unbind as neutron \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa6399be_409510e2","line":1174,"range":{"start_line":1174,"start_character":29,"end_line":1174,"end_character":33},"updated":"2016-10-19 19:48:48.000000000","message":"Here, you\u0027re testing all ports to see if no ports remain bound on the VM - there are no ports to unbind if no ports are bound.  However, a more correct answer is to unbind only the ports that are *both* still bound to this VM *and also* in the ports_to_skip: \n\n    self._unbind_ports(context, set(ports_to_skip).intersection(set(data.get(ports, []))), neutron)\n\n(or something a bit more Pythonic).","commit_id":"98887cf08a2d546461046dd693ec25ba17d09e25"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7e9d59bfc762ac186849f7d445394731345c79d8","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"        ports \u003d set(ports) - ports_to_skip"},{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":"        # Reset device_id and device_owner for the ports that are skipped"},{"line_number":1173,"context_line":"        self._unbind_ports(context, ports_to_skip, neutron)"},{"line_number":1174,"context_line":"        # Delete the rest of the ports"},{"line_number":1175,"context_line":"        self._delete_ports(neutron, instance, ports, raise_if_fail\u003dTrue)"},{"line_number":1176,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7a77a97e_2fb6d2f8","side":"PARENT","line":1173,"range":{"start_line":1173,"start_character":36,"end_line":1173,"end_character":49},"updated":"2016-11-19 02:05:26.000000000","message":"OK I think I understand now. Nova\u0027s getting the list of pre-existing ports passed in when the server was created, which is from the info_cache, which could be stale if the pre-existing port was unbound outside of nova. So ports_to_skip is the list of pre-existing ports that nova won\u0027t delete, but it\u0027s not refreshing those ports with neutron here to see if they are still bound...and then blindly unbinding them even though they may be bound to another instance at this point.","commit_id":"0f0d7584e0ee4772c42d0e327624005d85108edb"},{"author":{"_account_id":14924,"name":"AJAY KALAMBUR","email":"akalambu@cisco.com","username":"akalambu"},"change_message_id":"d81887ffed5a3891ca652e7085e2680c6ab2a1ca","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"        ports \u003d set(ports) - ports_to_skip"},{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":"        # Reset device_id and device_owner for the ports that are skipped"},{"line_number":1173,"context_line":"        self._unbind_ports(context, ports_to_skip, neutron)"},{"line_number":1174,"context_line":"        # Delete the rest of the ports"},{"line_number":1175,"context_line":"        self._delete_ports(neutron, instance, ports, raise_if_fail\u003dTrue)"},{"line_number":1176,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7a77a97e_6f936a5b","side":"PARENT","line":1173,"range":{"start_line":1173,"start_character":36,"end_line":1173,"end_character":49},"in_reply_to":"7a77a97e_2fb6d2f8","updated":"2016-11-19 02:12:32.000000000","message":"That is what is happening. Nova cache is stale and its blindly unbinding a port which is not even bound to this instance in neutron.\n\nThe port unbind is done outside of nova","commit_id":"0f0d7584e0ee4772c42d0e327624005d85108edb"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"2f64056ffa1fc7d2a7107b294f20c48ccb11954f","unresolved":false,"context_lines":[{"line_number":1170,"context_line":"        ports \u003d set(ports) - ports_to_skip"},{"line_number":1171,"context_line":""},{"line_number":1172,"context_line":"        # Reset device_id and device_owner for the ports that are skipped"},{"line_number":1173,"context_line":"        self._unbind_ports(context, ports_to_skip, neutron)"},{"line_number":1174,"context_line":"        # Delete the rest of the ports"},{"line_number":1175,"context_line":"        self._delete_ports(neutron, instance, ports, raise_if_fail\u003dTrue)"},{"line_number":1176,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"9a57fde8_9f117faf","side":"PARENT","line":1173,"range":{"start_line":1173,"start_character":36,"end_line":1173,"end_character":49},"in_reply_to":"7a77a97e_6f936a5b","updated":"2017-01-12 19:54:00.000000000","message":"So instead of changing all of this code here, why not simply refresh the cached info?","commit_id":"0f0d7584e0ee4772c42d0e327624005d85108edb"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bd491358702b070ee420358f654f297390f1819b","unresolved":false,"context_lines":[{"line_number":1645,"context_line":"        if len(exceptions) \u003e 0 and raise_if_fail:"},{"line_number":1646,"context_line":"            raise exceptions[0]"},{"line_number":1647,"context_line":""},{"line_number":1648,"context_line":"    def deallocate_for_instance(self, context, instance, **kwargs):"},{"line_number":1649,"context_line":"        \"\"\"Deallocate all network resources related to the instance.\"\"\""},{"line_number":1650,"context_line":"        LOG.debug(\u0027deallocate_for_instance()\u0027, instance\u003dinstance)"},{"line_number":1651,"context_line":"        # Nova\u0027s cache may not be upto date if port was unbound outside of nova"}],"source_content_type":"text/x-python","patch_set":10,"id":"5fc1f717_f79690f0","line":1648,"range":{"start_line":1648,"start_character":8,"end_line":1648,"end_character":31},"updated":"2019-03-12 13:35:38.000000000","message":"Can\u0027t you just decorate the method with @base_api.refresh_cache for the same affect?","commit_id":"a2e92679f96c1956e302824b07233078fd706976"}]}