)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c37029e3f18e99ec133e6e26db355084b147d8d9","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Ildiko Vancsa \u003cildiko.vancsa@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2017-07-05 18:29:24 +0200"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add Cinder v3 detach to swap_volume"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This adds the support for new style volume attachments"},{"line_number":10,"context_line":"to the swap volume operation."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"3f1d235d_8a5b3bf4","line":7,"range":{"start_line":7,"start_character":14,"end_line":7,"end_character":20},"updated":"2017-07-06 22:50:11.000000000","message":"It\u0027s really much more than detach, it\u0027s basically handling the entire swap volume flow for the new style attachments. Maybe a better title would be:\n\n\"Handle Cinder 3.27 style attachments in swap_volume\"","commit_id":"49d412b8937b8b27e2ff5663d2ded5984fb930ac"}],"nova/compute/api.py":[{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6115a5b7f82b47b865309e76fad7ed08b7a6af27","unresolved":false,"context_lines":[{"line_number":3857,"context_line":"        else:"},{"line_number":3858,"context_line":"            # This is a new-style attachment so for the volume that we are"},{"line_number":3859,"context_line":"            # going to swap to, create a new volume attachment."},{"line_number":3860,"context_line":"            new_attachment_id \u003d self.volume_api.attachment_create("},{"line_number":3861,"context_line":"                context, new_volume[\u0027id\u0027], instance.uuid).id"},{"line_number":3862,"context_line":""},{"line_number":3863,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"1f013ff3_e926eeb7","line":3860,"updated":"2017-05-16 11:07:01.000000000","message":"technically, we should probably error out if the min_service_version is not high enough to mean that the new swap_volume_api is available\n\nlonger term, it should be a logical impossibility given the way attach required a higher min compute version, but its good to follow the rules to make sure we are not missing something here.","commit_id":"503657cac9e423270049279da0b2ea46f649dbe7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"42d4e023c8758608ae2eb28381b2a8946a40b1c5","unresolved":false,"context_lines":[{"line_number":3857,"context_line":"        else:"},{"line_number":3858,"context_line":"            # This is a new-style attachment so for the volume that we are"},{"line_number":3859,"context_line":"            # going to swap to, create a new volume attachment."},{"line_number":3860,"context_line":"            new_attachment_id \u003d self.volume_api.attachment_create("},{"line_number":3861,"context_line":"                context, new_volume[\u0027id\u0027], instance.uuid).id"},{"line_number":3862,"context_line":""},{"line_number":3863,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"bf091321_43514cd8","line":3860,"in_reply_to":"1f013ff3_e926eeb7","updated":"2017-06-08 16:27:03.000000000","message":"This patch makes swap volume behavior depend on if the existing attached volume we\u0027re swapping from was attached using the new APIs, as discussed in the spec and PTG - so attach happens at the very end of the series of changes. If we\u0027re going to do min service version checks, which I think we said we would/could, then that would happen in the volume attach change at the end I think.","commit_id":"503657cac9e423270049279da0b2ea46f649dbe7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6115a5b7f82b47b865309e76fad7ed08b7a6af27","unresolved":false,"context_lines":[{"line_number":3865,"context_line":"                    context, instance\u003dinstance,"},{"line_number":3866,"context_line":"                    old_volume_id\u003dold_volume[\u0027id\u0027],"},{"line_number":3867,"context_line":"                    new_volume_id\u003dnew_volume[\u0027id\u0027],"},{"line_number":3868,"context_line":"                    new_attachment_id\u003dnew_attachment_id)"},{"line_number":3869,"context_line":"        except Exception:"},{"line_number":3870,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":3871,"context_line":"                self.volume_api.roll_detaching(context, old_volume[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"1f013ff3_a947c61d","line":3868,"updated":"2017-05-16 11:07:01.000000000","message":"Can\u0027t we store this new_attachment_id in the BDM, like how we stash the connector.\n\nI think we need this in too many places to not keep it in the same place that the regular attachment_id. Otherwise I worry we will drop this somewhere when we try any tidy up things.","commit_id":"503657cac9e423270049279da0b2ea46f649dbe7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"23f24beb20b5fa33807e99587390c3d34cefc5a2","unresolved":false,"context_lines":[{"line_number":3865,"context_line":"                    context, instance\u003dinstance,"},{"line_number":3866,"context_line":"                    old_volume_id\u003dold_volume[\u0027id\u0027],"},{"line_number":3867,"context_line":"                    new_volume_id\u003dnew_volume[\u0027id\u0027],"},{"line_number":3868,"context_line":"                    new_attachment_id\u003dnew_attachment_id)"},{"line_number":3869,"context_line":"        except Exception:"},{"line_number":3870,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":3871,"context_line":"                self.volume_api.roll_detaching(context, old_volume[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"bf091321_c312fcb1","line":3868,"in_reply_to":"1f013ff3_a947c61d","updated":"2017-06-08 16:41:31.000000000","message":"We only have one attachment_id in the BDM, and it\u0027s for the existing (old) attachment. We can\u0027t overwrite that with the new attachment_id. We only have one BDM too, so we can\u0027t update it\u0027s attachment_id until the swap is actually complete, which happens in the compute:\n\nhttps://review.openstack.org/#/c/456971/6/nova/compute/manager.py@5090","commit_id":"503657cac9e423270049279da0b2ea46f649dbe7"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"6115a5b7f82b47b865309e76fad7ed08b7a6af27","unresolved":false,"context_lines":[{"line_number":3869,"context_line":"        except Exception:"},{"line_number":3870,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":3871,"context_line":"                self.volume_api.roll_detaching(context, old_volume[\u0027id\u0027])"},{"line_number":3872,"context_line":"                if new_attachment_id is None:"},{"line_number":3873,"context_line":"                    self.volume_api.unreserve_volume(context, new_volume[\u0027id\u0027])"},{"line_number":3874,"context_line":"                else:"},{"line_number":3875,"context_line":"                    self.volume_api.attachment_delete("},{"line_number":3876,"context_line":"                        context, new_attachment_id)"},{"line_number":3877,"context_line":""},{"line_number":3878,"context_line":"    @check_instance_lock"},{"line_number":3879,"context_line":"    @check_instance_state(vm_state\u003d[vm_states.ACTIVE, vm_states.PAUSED,"}],"source_content_type":"text/x-python","patch_set":6,"id":"1f013ff3_499ba2c8","line":3876,"range":{"start_line":3872,"start_character":16,"end_line":3876,"end_character":51},"updated":"2017-05-16 11:07:01.000000000","message":"existing bug here I think, but this is largely nonsense because the rpcapi is a cast not a call.","commit_id":"503657cac9e423270049279da0b2ea46f649dbe7"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"23f24beb20b5fa33807e99587390c3d34cefc5a2","unresolved":false,"context_lines":[{"line_number":3869,"context_line":"        except Exception:"},{"line_number":3870,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":3871,"context_line":"                self.volume_api.roll_detaching(context, old_volume[\u0027id\u0027])"},{"line_number":3872,"context_line":"                if new_attachment_id is None:"},{"line_number":3873,"context_line":"                    self.volume_api.unreserve_volume(context, new_volume[\u0027id\u0027])"},{"line_number":3874,"context_line":"                else:"},{"line_number":3875,"context_line":"                    self.volume_api.attachment_delete("},{"line_number":3876,"context_line":"                        context, new_attachment_id)"},{"line_number":3877,"context_line":""},{"line_number":3878,"context_line":"    @check_instance_lock"},{"line_number":3879,"context_line":"    @check_instance_state(vm_state\u003d[vm_states.ACTIVE, vm_states.PAUSED,"}],"source_content_type":"text/x-python","patch_set":6,"id":"bf091321_63fd7039","line":3876,"range":{"start_line":3872,"start_character":16,"end_line":3876,"end_character":51},"in_reply_to":"1f013ff3_499ba2c8","updated":"2017-06-08 16:41:31.000000000","message":"True, unless we failed to cast for some reason, then we\u0027ll never get here. If the message never gets to the compute, then the new attachment is left dangling on the cinder side, but that\u0027s always been the case, and probably why they have a force detach API.","commit_id":"503657cac9e423270049279da0b2ea46f649dbe7"},{"author":{"_account_id":24147,"name":"Mark Giles","email":"mark.giles@oracle.com","username":"mark-giles"},"change_message_id":"e3472be79f1f212ceeed40957a926aaa2e39af84","unresolved":false,"context_lines":[{"line_number":3920,"context_line":"            # This is a new-style attachment so for the volume that we are"},{"line_number":3921,"context_line":"            # going to swap to, create a new volume attachment."},{"line_number":3922,"context_line":"            new_attachment_id \u003d self.volume_api.attachment_create("},{"line_number":3923,"context_line":"                context, new_volume[\u0027id\u0027], instance.uuid).id"},{"line_number":3924,"context_line":""},{"line_number":3925,"context_line":"        try:"},{"line_number":3926,"context_line":"            self.compute_rpcapi.swap_volume("}],"source_content_type":"text/x-python","patch_set":13,"id":"5f201791_e841d23a","line":3923,"range":{"start_line":3923,"start_character":57,"end_line":3923,"end_character":60},"updated":"2017-06-26 20:45:05.000000000","message":"This needs to be [\u0027id\u0027] (not .id) since the result of attachment_create is a dict.","commit_id":"36bde23c34b376b6ae1367df244d7f60100ff54e"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"0d7b7697b0182e776cced346435857fec120ed36","unresolved":false,"context_lines":[{"line_number":3920,"context_line":"            # This is a new-style attachment so for the volume that we are"},{"line_number":3921,"context_line":"            # going to swap to, create a new volume attachment."},{"line_number":3922,"context_line":"            new_attachment_id \u003d self.volume_api.attachment_create("},{"line_number":3923,"context_line":"                context, new_volume[\u0027id\u0027], instance.uuid).id"},{"line_number":3924,"context_line":""},{"line_number":3925,"context_line":"        try:"},{"line_number":3926,"context_line":"            self.compute_rpcapi.swap_volume("}],"source_content_type":"text/x-python","patch_set":13,"id":"5f201791_0f407406","line":3923,"range":{"start_line":3923,"start_character":57,"end_line":3923,"end_character":60},"in_reply_to":"5f201791_e841d23a","updated":"2017-06-27 15:41:13.000000000","message":"Done","commit_id":"36bde23c34b376b6ae1367df244d7f60100ff54e"}],"nova/compute/manager.py":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d560e030ce81150a18317363452547c3585ec6d8","unresolved":false,"context_lines":[{"line_number":3942,"context_line":"            self.instance_events.clear_events_for_instance(instance)"},{"line_number":3943,"context_line":""},{"line_number":3944,"context_line":"    def _terminate_volume_connections(self, context, instance, bdms,"},{"line_number":3945,"context_line":"                                      connector, keep_reserved\u003dFalse):"},{"line_number":3946,"context_line":"        for bdm in bdms:"},{"line_number":3947,"context_line":"            if bdm.is_volume:"},{"line_number":3948,"context_line":"                if bdm.attachment_id:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7ffa3b31_57c07de1","line":3945,"range":{"start_line":3945,"start_character":48,"end_line":3945,"end_character":68},"updated":"2017-04-20 09:51:48.000000000","message":"You are changing the behaviour of the existing calls with this that until now only called terminate_connection and thus want to keep the volume attached. IMHO we can drop this and the if block below.","commit_id":"b474c28318feda48c4c925f2216eecf7323356b3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"66637a132be08b19598a87e1b07e6ea33722567a","unresolved":false,"context_lines":[{"line_number":3942,"context_line":"            self.instance_events.clear_events_for_instance(instance)"},{"line_number":3943,"context_line":""},{"line_number":3944,"context_line":"    def _terminate_volume_connections(self, context, instance, bdms,"},{"line_number":3945,"context_line":"                                      connector, keep_reserved\u003dFalse):"},{"line_number":3946,"context_line":"        for bdm in bdms:"},{"line_number":3947,"context_line":"            if bdm.is_volume:"},{"line_number":3948,"context_line":"                if bdm.attachment_id:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7ffa3b31_ad1be2c7","line":3945,"range":{"start_line":3945,"start_character":48,"end_line":3945,"end_character":68},"in_reply_to":"7ffa3b31_57c07de1","updated":"2017-04-20 16:28:46.000000000","message":"Yeah I don\u0027t want to create attachments in a method called _terminate_volume_connections. We should handle this differently on the calling side, or create another helper method that re-uses _terminate_volume_connections and creates an attachment if needed.","commit_id":"b474c28318feda48c4c925f2216eecf7323356b3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"57bf238ade1eb36543bb5962fe581898e9ec62c8","unresolved":false,"context_lines":[{"line_number":3946,"context_line":"        for bdm in bdms:"},{"line_number":3947,"context_line":"            if bdm.is_volume:"},{"line_number":3948,"context_line":"                if bdm.attachment_id:"},{"line_number":3949,"context_line":"                    self.volume_api.attachment_delete(context,"},{"line_number":3950,"context_line":"                                                      bdm.attachment_id)"},{"line_number":3951,"context_line":"                    if keep_reserved:"},{"line_number":3952,"context_line":"                        self.volume_api.attachment_create(context,"},{"line_number":3953,"context_line":"                                                          bdm.volume_id,"},{"line_number":3954,"context_line":"                                                          instance.uuid)"},{"line_number":3955,"context_line":"                else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7ffa3b31_48544cfe","line":3952,"range":{"start_line":3949,"start_character":20,"end_line":3952,"end_character":66},"updated":"2017-04-20 16:38:19.000000000","message":"It sucks that we can\u0027t just atomically update the attachment to drop the connector which would keep the volume reserved. Either way, to make this atomic now we have to create the new attachment before we delete the old attachment.","commit_id":"b474c28318feda48c4c925f2216eecf7323356b3"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"d560e030ce81150a18317363452547c3585ec6d8","unresolved":false,"context_lines":[{"line_number":3949,"context_line":"                    self.volume_api.attachment_delete(context,"},{"line_number":3950,"context_line":"                                                      bdm.attachment_id)"},{"line_number":3951,"context_line":"                    if keep_reserved:"},{"line_number":3952,"context_line":"                        self.volume_api.attachment_create(context,"},{"line_number":3953,"context_line":"                                                          bdm.volume_id,"},{"line_number":3954,"context_line":"                                                          instance.uuid)"},{"line_number":3955,"context_line":"                else:"},{"line_number":3956,"context_line":"                    self.volume_api.terminate_connection(context,"},{"line_number":3957,"context_line":"                                                         bdm.volume_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"7ffa3b31_57f2bd71","line":3954,"range":{"start_line":3952,"start_character":24,"end_line":3954,"end_character":72},"updated":"2017-04-20 09:51:48.000000000","message":"We have lost track of attachment_id at this point right?","commit_id":"b474c28318feda48c4c925f2216eecf7323356b3"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7dcd40e29ec6d02a05b987597d79829e2bfe718b","unresolved":false,"context_lines":[{"line_number":4972,"context_line":"                          instance\u003dinstance)"},{"line_number":4973,"context_line":"                if bdm.attachment_id:"},{"line_number":4974,"context_line":"                    attachment_id \u003d self.volume_api.attachment_create("},{"line_number":4975,"context_line":"                        context, bdm.volume_id, instance.uuid)"},{"line_number":4976,"context_line":"                self._terminate_volume_connections(context, instance, [bdm])"},{"line_number":4977,"context_line":"                values \u003d {\u0027attachment_id\u0027: attachment_id}"},{"line_number":4978,"context_line":"                bdm.update(values)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5ff73747_9da80378","line":4975,"range":{"start_line":4975,"start_character":33,"end_line":4975,"end_character":47},"updated":"2017-04-21 20:41:16.000000000","message":"bdm.volume_id at this point is the old volume because we haven\u0027t updated the BDM yet, that happens on L5051. So this isn\u0027t taking into account if we failed or not. What the code was doing before this was, if we failed to swap on the driver, it would terminate the connection for the new volume. If we successfully swapped in the driver, we\u0027d terminate the connection for the old volume and then update the bdm on L5051 with the new volume\u0027s connection_info.\n\nWe initialized the connection for the new volume on L4906.\n\nSo I think what you really need to do here, is have two attachments temporarily. Where we\u0027d call initialize_connection for the new volume on L4906, we need to create a new attachment for the instance and new volume there.\n\nIf swap is successful, we delete the old volume attachment and update the attachment_id in the bdm to the new attachment for the new volume.\n\nIf swap failed, we delete the new volume attachment for the new volume and don\u0027t have to change anything in the BDM.","commit_id":"91dee482d4aeb0345df22b95a229e027ccdd306c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"96510252302b47bd265a6f040089ec47774e672a","unresolved":false,"context_lines":[{"line_number":4972,"context_line":"                          instance\u003dinstance)"},{"line_number":4973,"context_line":"                if bdm.attachment_id:"},{"line_number":4974,"context_line":"                    attachment_id \u003d self.volume_api.attachment_create("},{"line_number":4975,"context_line":"                        context, bdm.volume_id, instance.uuid)"},{"line_number":4976,"context_line":"                self._terminate_volume_connections(context, instance, [bdm])"},{"line_number":4977,"context_line":"                values \u003d {\u0027attachment_id\u0027: attachment_id}"},{"line_number":4978,"context_line":"                bdm.update(values)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5ff73747_00aec8e6","line":4975,"range":{"start_line":4975,"start_character":33,"end_line":4975,"end_character":47},"in_reply_to":"5ff73747_7ddc1718","updated":"2017-04-21 21:25:38.000000000","message":"I think I understand now. If bdm.attachment_id is set on the old volume, it was created with the new flow. We only do that when we\u0027re using cinder 3.27 and all computes are upgraded.\n\nGiven that, in the compute API we create the new attachment for the new volume which reserves it (since there is no connector). Then we could pass the attachment ID for the new volume over rpc to the compute to use it. Alternatively, the compute could look up the new attachment from cinder, but I don\u0027t know what that REST API looks like since it\u0027s not documented. That would keep us from changing the RPC API though, which might be nice.\n\nAnyway, we do create the new attachment in the API and we\u0027ll key off that in the compute based on bdm.attachment_id, since that\u0027s what the API is going to key off of also.","commit_id":"91dee482d4aeb0345df22b95a229e027ccdd306c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5698f67e40459dddd241ab73a42f84086a83172c","unresolved":false,"context_lines":[{"line_number":4972,"context_line":"                          instance\u003dinstance)"},{"line_number":4973,"context_line":"                if bdm.attachment_id:"},{"line_number":4974,"context_line":"                    attachment_id \u003d self.volume_api.attachment_create("},{"line_number":4975,"context_line":"                        context, bdm.volume_id, instance.uuid)"},{"line_number":4976,"context_line":"                self._terminate_volume_connections(context, instance, [bdm])"},{"line_number":4977,"context_line":"                values \u003d {\u0027attachment_id\u0027: attachment_id}"},{"line_number":4978,"context_line":"                bdm.update(values)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5ff73747_7ddc1718","line":4975,"range":{"start_line":4975,"start_character":33,"end_line":4975,"end_character":47},"in_reply_to":"5ff73747_9da80378","updated":"2017-04-21 20:44:12.000000000","message":"This was also called out a bit in John\u0027s spec:\n\nhttps://specs.openstack.org/openstack/nova-specs/specs/pike/approved/cinder-new-attach-apis.html#swap-volume\n\nAlthough I\u0027m less clear on where things happen in that spec, i.e. I don\u0027t know if we actually do anything in the API.","commit_id":"91dee482d4aeb0345df22b95a229e027ccdd306c"},{"author":{"_account_id":17814,"name":"fengzhr","email":"fengzhr@awcloud.com","username":"space"},"change_message_id":"fc24e7c8a7397e23be002e495b76cd35ecf0e8a9","unresolved":false,"context_lines":[{"line_number":4974,"context_line":"                    attachment_id \u003d self.volume_api.attachment_create("},{"line_number":4975,"context_line":"                        context, bdm.volume_id, instance.uuid)"},{"line_number":4976,"context_line":"                self._terminate_volume_connections(context, instance, [bdm])"},{"line_number":4977,"context_line":"                values \u003d {\u0027attachment_id\u0027: attachment_id}"},{"line_number":4978,"context_line":"                bdm.update(values)"},{"line_number":4979,"context_line":"                bdm.save()"},{"line_number":4980,"context_line":"            # NOTE(lyarwood): The following call to"}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_f6564b2a","line":4977,"range":{"start_line":4977,"start_character":43,"end_line":4977,"end_character":56},"updated":"2017-04-21 06:28:01.000000000","message":"Uninitialized variables. So the unit test failed.","commit_id":"91dee482d4aeb0345df22b95a229e027ccdd306c"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"6e678ee2155475e7ab2877bfcc0ee4b42d4b3c9d","unresolved":false,"context_lines":[{"line_number":4970,"context_line":"                LOG.debug(\"swap_volume: removing Cinder connection for \""},{"line_number":4971,"context_line":"                          \"%(volume)s\", {\u0027volume\u0027: conn_volume},"},{"line_number":4972,"context_line":"                          instance\u003dinstance)"},{"line_number":4973,"context_line":"                if bdm.attachment_id:"},{"line_number":4974,"context_line":"                    attachment_id \u003d self.volume_api.attachment_create("},{"line_number":4975,"context_line":"                        context, bdm.volume_id, instance.uuid)"},{"line_number":4976,"context_line":"                self._terminate_volume_connections(context, instance, [bdm])"},{"line_number":4977,"context_line":"                values \u003d {\u0027attachment_id\u0027: attachment_id}"},{"line_number":4978,"context_line":"                bdm.update(values)"},{"line_number":4979,"context_line":"                bdm.save()"},{"line_number":4980,"context_line":"            # NOTE(lyarwood): The following call to"},{"line_number":4981,"context_line":"            # os-migrate-volume-completion returns a dict containing"},{"line_number":4982,"context_line":"            # save_volume_id, this volume id has two possible values :"}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_fc85fe07","line":4979,"range":{"start_line":4973,"start_character":0,"end_line":4979,"end_character":26},"updated":"2017-04-21 15:23:04.000000000","message":"I don\u0027t think this is correct, we are trying to remove access to the old volume from the host here, we shouldn\u0027t need to keep it reserved.","commit_id":"91dee482d4aeb0345df22b95a229e027ccdd306c"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"1e8635abfb16a60a23d0c8060b0c76d80a85ad10","unresolved":false,"context_lines":[{"line_number":4970,"context_line":"                LOG.debug(\"swap_volume: removing Cinder connection for \""},{"line_number":4971,"context_line":"                          \"%(volume)s\", {\u0027volume\u0027: conn_volume},"},{"line_number":4972,"context_line":"                          instance\u003dinstance)"},{"line_number":4973,"context_line":"                if bdm.attachment_id:"},{"line_number":4974,"context_line":"                    attachment_id \u003d self.volume_api.attachment_create("},{"line_number":4975,"context_line":"                        context, bdm.volume_id, instance.uuid)"},{"line_number":4976,"context_line":"                self._terminate_volume_connections(context, instance, [bdm])"},{"line_number":4977,"context_line":"                values \u003d {\u0027attachment_id\u0027: attachment_id}"},{"line_number":4978,"context_line":"                bdm.update(values)"},{"line_number":4979,"context_line":"                bdm.save()"},{"line_number":4980,"context_line":"            # NOTE(lyarwood): The following call to"},{"line_number":4981,"context_line":"            # os-migrate-volume-completion returns a dict containing"},{"line_number":4982,"context_line":"            # save_volume_id, this volume id has two possible values :"}],"source_content_type":"text/x-python","patch_set":2,"id":"7ffa3b31_72a6b5ad","line":4979,"range":{"start_line":4973,"start_character":0,"end_line":4979,"end_character":26},"in_reply_to":"7ffa3b31_fc85fe07","updated":"2017-04-21 16:17:31.000000000","message":"We actually need to know how we are going to create and store the new attachment_id on L4925 before we can work this out, urgh.","commit_id":"91dee482d4aeb0345df22b95a229e027ccdd306c"},{"author":{"_account_id":9562,"name":"Ildiko Vancsa","email":"ildiko.vancsa@gmail.com","username":"ildikov"},"change_message_id":"1d2f29ddaaeb8876025da92053c8e1a3bf2bf347","unresolved":false,"context_lines":[{"line_number":5017,"context_line":"                        # TODO(mriedem): should this just delete the old"},{"line_number":5018,"context_line":"                        # attachment? Would that mess up"},{"line_number":5019,"context_line":"                        # migrate_volume_completion?"},{"line_number":5020,"context_line":"                        self.volume_api.attachment_update("},{"line_number":5021,"context_line":"                            context, bdm.attachment_id, connector\u003dNone)"},{"line_number":5022,"context_line":""},{"line_number":5023,"context_line":"            # NOTE(lyarwood): The following call to"}],"source_content_type":"text/x-python","patch_set":5,"id":"5ff73747_aa4086a6","line":5020,"range":{"start_line":5020,"start_character":40,"end_line":5020,"end_character":57},"updated":"2017-04-27 08:01:54.000000000","message":"This should be attachment_delete here to remove the old volume connection and the corresponding attachment.\n\nattachment_update with connector\u003dNone throws HTTPBadRequest currently, see my comment on the earlier patch.","commit_id":"f73c76ce830666311b51c34cad844cd71769cfa1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6ba204cb5bb96030eb25c6f04866ad339a07b9fd","unresolved":false,"context_lines":[{"line_number":5017,"context_line":"                        # TODO(mriedem): should this just delete the old"},{"line_number":5018,"context_line":"                        # attachment? Would that mess up"},{"line_number":5019,"context_line":"                        # migrate_volume_completion?"},{"line_number":5020,"context_line":"                        self.volume_api.attachment_update("},{"line_number":5021,"context_line":"                            context, bdm.attachment_id, connector\u003dNone)"},{"line_number":5022,"context_line":""},{"line_number":5023,"context_line":"            # NOTE(lyarwood): The following call to"}],"source_content_type":"text/x-python","patch_set":5,"id":"5ff73747_740c92a4","line":5020,"range":{"start_line":5020,"start_character":40,"end_line":5020,"end_character":57},"in_reply_to":"5ff73747_aa4086a6","updated":"2017-04-27 16:42:20.000000000","message":"OK.","commit_id":"f73c76ce830666311b51c34cad844cd71769cfa1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"724adde806945752ee7fa4d0d385a0c5abd2f91e","unresolved":false,"context_lines":[{"line_number":5095,"context_line":"            # and delete the old attachment."},{"line_number":5096,"context_line":"            values[\u0027attachment_id\u0027] \u003d new_attachment_id"},{"line_number":5097,"context_line":"            try:"},{"line_number":5098,"context_line":"                self.volume_api.attachment_delete(context, bdm.attachment_id)"},{"line_number":5099,"context_line":"            except exception.VolumeAttachmentNotFound:"},{"line_number":5100,"context_line":"                # In case Cinder deleted the old attachment and old volume"},{"line_number":5101,"context_line":"                # when swapping to the new volume, we just ignore this."}],"source_content_type":"text/x-python","patch_set":5,"id":"5ff73747_d380eff7","line":5098,"updated":"2017-04-28 20:20:19.000000000","message":"This can go away when we change attachment_update above to attachment_delete.","commit_id":"f73c76ce830666311b51c34cad844cd71769cfa1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"db57aee78656767034e59c00e18989906f073c54","unresolved":false,"context_lines":[{"line_number":5095,"context_line":"                    self.volume_api.terminate_connection(context,"},{"line_number":5096,"context_line":"                                                         conn_volume,"},{"line_number":5097,"context_line":"                                                         connector)"},{"line_number":5098,"context_line":"                else:"},{"line_number":5099,"context_line":"                    # This is a new style volume attachment. If we failed, then"},{"line_number":5100,"context_line":"                    # the new attachment was already deleted above in the"},{"line_number":5101,"context_line":"                    # exception block and we have nothing more to do here. If"}],"source_content_type":"text/x-python","patch_set":7,"id":"7f231b9d_f1dd4370","line":5098,"updated":"2017-06-15 14:53:26.000000000","message":"Probably need to pull over the state checking that jgriffith is doing here:\n\nhttps://review.openstack.org/#/c/330285/50/nova/compute/manager.py@5094","commit_id":"40d6feb59d69851e9468279669c4340a01d69f60"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c37029e3f18e99ec133e6e26db355084b147d8d9","unresolved":false,"context_lines":[{"line_number":5023,"context_line":"            # the host connector, which will give us back the new attachment"},{"line_number":5024,"context_line":"            # connection_info."},{"line_number":5025,"context_line":"            new_cinfo \u003d self.volume_api.attachment_update("},{"line_number":5026,"context_line":"                context, new_attachment_id, connector).connection_info"},{"line_number":5027,"context_line":""},{"line_number":5028,"context_line":"        old_cinfo \u003d jsonutils.loads(bdm[\u0027connection_info\u0027])"},{"line_number":5029,"context_line":"        if old_cinfo and \u0027serial\u0027 not in old_cinfo:"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_0d3982d5","line":5026,"range":{"start_line":5026,"start_character":54,"end_line":5026,"end_character":70},"updated":"2017-07-06 22:50:11.000000000","message":"I saw it pointed out that attachment_create returns a dict but attachment_update returns a VolumeAttachment object from python-cinderclient - that\u0027s confusing. Nothing really Nova can do about it, but the usage is weird on the client side.","commit_id":"49d412b8937b8b27e2ff5663d2ded5984fb930ac"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c37029e3f18e99ec133e6e26db355084b147d8d9","unresolved":false,"context_lines":[{"line_number":5097,"context_line":"                    self.volume_api.attachment_delete("},{"line_number":5098,"context_line":"                        context, new_attachment_id)"},{"line_number":5099,"context_line":"        finally:"},{"line_number":5100,"context_line":"            conn_volume \u003d new_volume_id if failed else old_volume_id"},{"line_number":5101,"context_line":"            if new_cinfo:"},{"line_number":5102,"context_line":"                LOG.debug(\"swap_volume: removing Cinder connection \""},{"line_number":5103,"context_line":"                          \"for volume %(volume)s\", {\u0027volume\u0027: conn_volume},"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_0d74e2cd","line":5100,"range":{"start_line":5100,"start_character":12,"end_line":5100,"end_character":68},"updated":"2017-07-06 22:50:11.000000000","message":"Note to self: doing everything in the finally block is terribly confusing. For the non-failed cases, we should just do that right after driver.swap_volume comes back. That\u0027s a later cleanup once we have everything working though.","commit_id":"49d412b8937b8b27e2ff5663d2ded5984fb930ac"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bc399cbef4f96ce777be4eda076ec6d44b8bf2bf","unresolved":false,"context_lines":[{"line_number":5097,"context_line":"                    self.volume_api.attachment_delete("},{"line_number":5098,"context_line":"                        context, new_attachment_id)"},{"line_number":5099,"context_line":"        finally:"},{"line_number":5100,"context_line":"            conn_volume \u003d new_volume_id if failed else old_volume_id"},{"line_number":5101,"context_line":"            if new_cinfo:"},{"line_number":5102,"context_line":"                LOG.debug(\"swap_volume: removing Cinder connection \""},{"line_number":5103,"context_line":"                          \"for volume %(volume)s\", {\u0027volume\u0027: conn_volume},"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_6226bc6a","line":5100,"range":{"start_line":5100,"start_character":12,"end_line":5100,"end_character":68},"in_reply_to":"3f1d235d_0d74e2cd","updated":"2017-07-07 05:26:30.000000000","message":"Added a TODO.","commit_id":"49d412b8937b8b27e2ff5663d2ded5984fb930ac"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c37029e3f18e99ec133e6e26db355084b147d8d9","unresolved":false,"context_lines":[{"line_number":5142,"context_line":"                                                          old_volume_id,"},{"line_number":5143,"context_line":"                                                          new_volume_id,"},{"line_number":5144,"context_line":"                                                          error\u003dfailed)"},{"line_number":5145,"context_line":"            LOG.debug(\"swap_volume: Cinder migrate_volume_completion \""},{"line_number":5146,"context_line":"                      \"returned: %(comp_ret)s\", {\u0027comp_ret\u0027: comp_ret},"},{"line_number":5147,"context_line":"                      instance\u003dinstance)"},{"line_number":5148,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_4d3c1ae0","line":5145,"updated":"2017-07-06 22:50:11.000000000","message":"nit: we could intend this since we don\u0027t call migrate_volume_completion in all cases.","commit_id":"49d412b8937b8b27e2ff5663d2ded5984fb930ac"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bc399cbef4f96ce777be4eda076ec6d44b8bf2bf","unresolved":false,"context_lines":[{"line_number":5142,"context_line":"                                                          old_volume_id,"},{"line_number":5143,"context_line":"                                                          new_volume_id,"},{"line_number":5144,"context_line":"                                                          error\u003dfailed)"},{"line_number":5145,"context_line":"            LOG.debug(\"swap_volume: Cinder migrate_volume_completion \""},{"line_number":5146,"context_line":"                      \"returned: %(comp_ret)s\", {\u0027comp_ret\u0027: comp_ret},"},{"line_number":5147,"context_line":"                      instance\u003dinstance)"},{"line_number":5148,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_2224445d","line":5145,"in_reply_to":"3f1d235d_4d3c1ae0","updated":"2017-07-07 05:26:30.000000000","message":"Done","commit_id":"49d412b8937b8b27e2ff5663d2ded5984fb930ac"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"7945d03e57a964e4248c872e413d619206f9e1b5","unresolved":false,"context_lines":[{"line_number":5032,"context_line":"                                                                new_volume_id,"},{"line_number":5033,"context_line":"                                                                old_volume_id,"},{"line_number":5034,"context_line":"                                                                connector,"},{"line_number":5035,"context_line":"                                                                instance,"},{"line_number":5036,"context_line":"                                                                bdm)"},{"line_number":5037,"context_line":"            # NOTE(lyarwood): The Libvirt driver, the only virt driver"},{"line_number":5038,"context_line":"            # currently implementing swap_volume, will modify the contents of"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f1d235d_318e892e","side":"PARENT","line":5035,"range":{"start_line":5035,"start_character":0,"end_line":5035,"end_character":73},"updated":"2017-07-10 18:23:24.000000000","message":"instance was removed as a parameter to _init_volume_connection(). just noting this. nothing wrong with this since it doesn\u0027t look like instance is used for anything in that function anyway... perhaps originally it was passed to LOG functions.","commit_id":"5da190ff12a676bb8773127a0bb25a2120ef1e66"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bbae77f6fd804bb677f5024b3e58203d2f341efd","unresolved":false,"context_lines":[{"line_number":5032,"context_line":"                                                                new_volume_id,"},{"line_number":5033,"context_line":"                                                                old_volume_id,"},{"line_number":5034,"context_line":"                                                                connector,"},{"line_number":5035,"context_line":"                                                                instance,"},{"line_number":5036,"context_line":"                                                                bdm)"},{"line_number":5037,"context_line":"            # NOTE(lyarwood): The Libvirt driver, the only virt driver"},{"line_number":5038,"context_line":"            # currently implementing swap_volume, will modify the contents of"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f1d235d_5b9a3e6e","side":"PARENT","line":5035,"range":{"start_line":5035,"start_character":0,"end_line":5035,"end_character":73},"in_reply_to":"3f1d235d_318e892e","updated":"2017-07-10 19:13:28.000000000","message":"Yeah I removed it because it was annoying me as this code is already quite complicated to follow and extra unused stuff just makes it worse.","commit_id":"5da190ff12a676bb8773127a0bb25a2120ef1e66"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"7945d03e57a964e4248c872e413d619206f9e1b5","unresolved":false,"context_lines":[{"line_number":5180,"context_line":"        # swap volume API flows."},{"line_number":5181,"context_line":"        is_cinder_migration \u003d ("},{"line_number":5182,"context_line":"            True if old_volume[\u0027status\u0027] in (\u0027retyping\u0027,"},{"line_number":5183,"context_line":"                                             \u0027migrating\u0027) else False)"},{"line_number":5184,"context_line":"        old_vol_size \u003d old_volume[\u0027size\u0027]"},{"line_number":5185,"context_line":"        new_vol_size \u003d self.volume_api.get(context, new_volume_id)[\u0027size\u0027]"},{"line_number":5186,"context_line":"        if new_vol_size \u003e old_vol_size:"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f1d235d_e5860bdc","line":5183,"updated":"2017-07-10 18:23:24.000000000","message":"code style nit... in the same three lines of code, you could have done this, which (IMHO) is more readable:\n\n is_cinder_migration \u003d False\n if old_volume[\u0027status\u0027] in (\u0027retyping\u0027, \u0027migrating\u0027):\n      is_cinder_migration \u003d True\n\nyou could even have done:\n\n is_cinder_mig \u003d old_volume[\u0027status\u0027] in (\u0027retyping\u0027, \u0027migrating\u0027)","commit_id":"ae53c224579cc16d25ca2ee2134a706b4904a7a4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bbae77f6fd804bb677f5024b3e58203d2f341efd","unresolved":false,"context_lines":[{"line_number":5180,"context_line":"        # swap volume API flows."},{"line_number":5181,"context_line":"        is_cinder_migration \u003d ("},{"line_number":5182,"context_line":"            True if old_volume[\u0027status\u0027] in (\u0027retyping\u0027,"},{"line_number":5183,"context_line":"                                             \u0027migrating\u0027) else False)"},{"line_number":5184,"context_line":"        old_vol_size \u003d old_volume[\u0027size\u0027]"},{"line_number":5185,"context_line":"        new_vol_size \u003d self.volume_api.get(context, new_volume_id)[\u0027size\u0027]"},{"line_number":5186,"context_line":"        if new_vol_size \u003e old_vol_size:"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f1d235d_7b761a41","line":5183,"in_reply_to":"3f1d235d_e5860bdc","updated":"2017-07-10 19:13:28.000000000","message":"Yeah, it is - will update if have to respin for anything.","commit_id":"ae53c224579cc16d25ca2ee2134a706b4904a7a4"}],"nova/compute/rpcapi.py":[{"author":{"_account_id":17386,"name":"Steve Noyes","email":"steve.noyes@oracle.com","username":"ssn1ssn"},"change_message_id":"0fc3d1d3818cc2206dbb2ae6c7308aed0edb8fa1","unresolved":false,"context_lines":[{"line_number":331,"context_line":"        \u0027mitaka\u0027: \u00274.11\u0027,"},{"line_number":332,"context_line":"        \u0027newton\u0027: \u00274.13\u0027,"},{"line_number":333,"context_line":"        \u0027ocata\u0027: \u00274.13\u0027,"},{"line_number":334,"context_line":"    }"},{"line_number":335,"context_line":""},{"line_number":336,"context_line":"    def __init__(self):"},{"line_number":337,"context_line":"        super(ComputeAPI, self).__init__()"}],"source_content_type":"text/x-python","patch_set":5,"id":"5ff73747_3420a1b0","line":334,"updated":"2017-04-25 15:29:51.000000000","message":"does 4.14 need to be added to this dict?","commit_id":"f73c76ce830666311b51c34cad844cd71769cfa1"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"4e32b9eaf2fe415ad7f20d5e88ded416ef751532","unresolved":false,"context_lines":[{"line_number":331,"context_line":"        \u0027mitaka\u0027: \u00274.11\u0027,"},{"line_number":332,"context_line":"        \u0027newton\u0027: \u00274.13\u0027,"},{"line_number":333,"context_line":"        \u0027ocata\u0027: \u00274.13\u0027,"},{"line_number":334,"context_line":"    }"},{"line_number":335,"context_line":""},{"line_number":336,"context_line":"    def __init__(self):"},{"line_number":337,"context_line":"        super(ComputeAPI, self).__init__()"}],"source_content_type":"text/x-python","patch_set":5,"id":"5ff73747_411d80bb","line":334,"in_reply_to":"5ff73747_3420a1b0","updated":"2017-04-26 15:53:50.000000000","message":"Good question, but no, those get updated after we release, e.g. this is when the ocata alias was added after ocata was released (so we added the alias in pike/master):\n\nhttps://review.openstack.org/#/c/425958/","commit_id":"f73c76ce830666311b51c34cad844cd71769cfa1"}],"nova/tests/unit/compute/test_compute_api.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c37029e3f18e99ec133e6e26db355084b147d8d9","unresolved":false,"context_lines":[{"line_number":2424,"context_line":"    def test_swap_volume_volume_api_usage(self):"},{"line_number":2425,"context_line":"        self._test_swap_volume()"},{"line_number":2426,"context_line":""},{"line_number":2427,"context_line":"    def test_swap_volume_volume_api_usage_new_attach_flow(self):"},{"line_number":2428,"context_line":"        self._test_swap_volume(attachment_id\u003duuids.attachment_id)"},{"line_number":2429,"context_line":""},{"line_number":2430,"context_line":"    def test_swap_volume_with_swap_volume_error_new_attach_flow(self):"},{"line_number":2431,"context_line":"        self._test_swap_volume(expected_exception\u003dAttributeError,"},{"line_number":2432,"context_line":"                               attachment_id\u003duuids.attachment_id)"},{"line_number":2433,"context_line":""},{"line_number":2434,"context_line":"    def _test_swap_volume(self, expected_exception\u003dNone, attachment_id\u003dNone):"},{"line_number":2435,"context_line":"        volumes \u003d self._get_volumes_for_test_swap_volume()"},{"line_number":2436,"context_line":"        instance \u003d self._get_instance_for_test_swap_volume()"},{"line_number":2437,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_7a0c2a93","line":2434,"range":{"start_line":2427,"start_character":0,"end_line":2434,"end_character":77},"updated":"2017-07-06 22:50:11.000000000","message":"Using these common test methods for handling all cases gets messy and it\u0027s not really asserting all things. For example, if attachment_id is not None, we should be asserting that mock_swap_volume is called with attachment_id\u003duuids.attachment_id, and if attachment_id is None, we should be asserting that mock_swap_volume was called with attachment_id\u003dNone.\n\nSimilar for reserve/unreserve assertions based on whether or not those are called if attachment_id is None or not.\n\nI don\u0027t want to make a big deal out of this, but it would be better if we either (1) created new standalone unit tests for the new style attachment cases or (2) change this to make some proper assertions in the non-expected_exception case, as described above.","commit_id":"49d412b8937b8b27e2ff5663d2ded5984fb930ac"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bc399cbef4f96ce777be4eda076ec6d44b8bf2bf","unresolved":false,"context_lines":[{"line_number":2424,"context_line":"    def test_swap_volume_volume_api_usage(self):"},{"line_number":2425,"context_line":"        self._test_swap_volume()"},{"line_number":2426,"context_line":""},{"line_number":2427,"context_line":"    def test_swap_volume_volume_api_usage_new_attach_flow(self):"},{"line_number":2428,"context_line":"        self._test_swap_volume(attachment_id\u003duuids.attachment_id)"},{"line_number":2429,"context_line":""},{"line_number":2430,"context_line":"    def test_swap_volume_with_swap_volume_error_new_attach_flow(self):"},{"line_number":2431,"context_line":"        self._test_swap_volume(expected_exception\u003dAttributeError,"},{"line_number":2432,"context_line":"                               attachment_id\u003duuids.attachment_id)"},{"line_number":2433,"context_line":""},{"line_number":2434,"context_line":"    def _test_swap_volume(self, expected_exception\u003dNone, attachment_id\u003dNone):"},{"line_number":2435,"context_line":"        volumes \u003d self._get_volumes_for_test_swap_volume()"},{"line_number":2436,"context_line":"        instance \u003d self._get_instance_for_test_swap_volume()"},{"line_number":2437,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_253e8ec0","line":2434,"range":{"start_line":2427,"start_character":0,"end_line":2434,"end_character":77},"in_reply_to":"3f1d235d_7a0c2a93","updated":"2017-07-07 05:26:30.000000000","message":"Done","commit_id":"49d412b8937b8b27e2ff5663d2ded5984fb930ac"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c37029e3f18e99ec133e6e26db355084b147d8d9","unresolved":false,"context_lines":[{"line_number":1870,"context_line":"                    \u0027instance_uuid\u0027: uuids.instance,"},{"line_number":1871,"context_line":"                    \u0027connection_info\u0027: \u0027{\"foo\": \"bar\"}\u0027,"},{"line_number":1872,"context_line":"                    \u0027volume_id\u0027: uuids.old_volume,"},{"line_number":1873,"context_line":"                    \u0027attachment_id\u0027: None})"},{"line_number":1874,"context_line":""},{"line_number":1875,"context_line":"        def fake_vol_api_roll_detaching(context, volume_id):"},{"line_number":1876,"context_line":"            self.assertTrue(uuidutils.is_uuid_like(volume_id))"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_d0431d3e","line":1873,"range":{"start_line":1873,"start_character":37,"end_line":1873,"end_character":41},"updated":"2017-07-06 22:50:11.000000000","message":"So we have 0 test coverage for the new-style attachment swap_volume flow in the compute manager. We\u0027re going to need to add tests for that, including the various edge cases where things fail. There are at least 4 tests here for the new style attachments:\n\n1. Happy path for new-style volume migration, initiated by Cinder.\n\n2. Happy path for new-style volume migration, initiated via Nova API (not a cinder migration/retype).\n\n3. Error case where attachment_update fails.\n\n4. Error case where driver.swap_volume fails.\n\nI suggest that rather than try to lump those tests into this existing infrastructure, which is already complicated just for old-style attachments, we write those new tests using new individual test cases.","commit_id":"49d412b8937b8b27e2ff5663d2ded5984fb930ac"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bc399cbef4f96ce777be4eda076ec6d44b8bf2bf","unresolved":false,"context_lines":[{"line_number":1870,"context_line":"                    \u0027instance_uuid\u0027: uuids.instance,"},{"line_number":1871,"context_line":"                    \u0027connection_info\u0027: \u0027{\"foo\": \"bar\"}\u0027,"},{"line_number":1872,"context_line":"                    \u0027volume_id\u0027: uuids.old_volume,"},{"line_number":1873,"context_line":"                    \u0027attachment_id\u0027: None})"},{"line_number":1874,"context_line":""},{"line_number":1875,"context_line":"        def fake_vol_api_roll_detaching(context, volume_id):"},{"line_number":1876,"context_line":"            self.assertTrue(uuidutils.is_uuid_like(volume_id))"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_96c0f6e2","line":1873,"range":{"start_line":1873,"start_character":37,"end_line":1873,"end_character":41},"in_reply_to":"3f1d235d_d0431d3e","updated":"2017-07-07 05:26:30.000000000","message":"Done","commit_id":"49d412b8937b8b27e2ff5663d2ded5984fb930ac"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"7945d03e57a964e4248c872e413d619206f9e1b5","unresolved":false,"context_lines":[{"line_number":2288,"context_line":"                self.context, uuids.new_attachment_id)"},{"line_number":2289,"context_line":"            # After a failed swap volume, since it was not a"},{"line_number":2290,"context_line":"            # Cinder-initiated call, we don\u0027t call migrate_volume_completion."},{"line_number":2291,"context_line":"            migrate_volume_completion.assert_not_called()"},{"line_number":2292,"context_line":""},{"line_number":2293,"context_line":"    @mock.patch.object(fake_driver.FakeDriver,"},{"line_number":2294,"context_line":"                       \u0027check_can_live_migrate_source\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"3f1d235d_6031c9ec","line":2291,"updated":"2017-07-10 18:23:24.000000000","message":"excellent unit test additions. ++","commit_id":"ae53c224579cc16d25ca2ee2134a706b4904a7a4"}],"nova/tests/unit/compute/test_rpcapi.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"c37029e3f18e99ec133e6e26db355084b147d8d9","unresolved":false,"context_lines":[{"line_number":448,"context_line":"                instance\u003dself.fake_instance_obj, old_volume_id\u003d\u0027oldid\u0027,"},{"line_number":449,"context_line":"                new_volume_id\u003d\u0027newid\u0027, new_attachment_id\u003duuids.attachment_id,"},{"line_number":450,"context_line":"                version\u003d\u00274.17\u0027)"},{"line_number":451,"context_line":"        # TODO(mriedem): test the 4.0 case?"},{"line_number":452,"context_line":""},{"line_number":453,"context_line":"    def test_restore_instance(self):"},{"line_number":454,"context_line":"        self._test_compute_api(\u0027restore_instance\u0027, \u0027cast\u0027,"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_da8b3ebd","line":451,"updated":"2017-07-06 22:50:11.000000000","message":"We still need to handle this. We should have a separate test where can_send_version returns False so we remove the attachment_id kwargs and fallback to using the 4.0 version.","commit_id":"49d412b8937b8b27e2ff5663d2ded5984fb930ac"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bc399cbef4f96ce777be4eda076ec6d44b8bf2bf","unresolved":false,"context_lines":[{"line_number":448,"context_line":"                instance\u003dself.fake_instance_obj, old_volume_id\u003d\u0027oldid\u0027,"},{"line_number":449,"context_line":"                new_volume_id\u003d\u0027newid\u0027, new_attachment_id\u003duuids.attachment_id,"},{"line_number":450,"context_line":"                version\u003d\u00274.17\u0027)"},{"line_number":451,"context_line":"        # TODO(mriedem): test the 4.0 case?"},{"line_number":452,"context_line":""},{"line_number":453,"context_line":"    def test_restore_instance(self):"},{"line_number":454,"context_line":"        self._test_compute_api(\u0027restore_instance\u0027, \u0027cast\u0027,"}],"source_content_type":"text/x-python","patch_set":17,"id":"3f1d235d_0248282a","line":451,"in_reply_to":"3f1d235d_da8b3ebd","updated":"2017-07-07 05:26:30.000000000","message":"Done","commit_id":"49d412b8937b8b27e2ff5663d2ded5984fb930ac"}]}