)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"dd0cece45f4f7923c9c0746a18bec369e2cc6317","unresolved":false,"context_lines":[{"line_number":5625,"context_line":"                              and volume."},{"line_number":5626,"context_line":""},{"line_number":5627,"context_line":"        \"\"\""},{"line_number":5628,"context_line":"        @utils.synchronized(instance.uuid)"},{"line_number":5629,"context_line":"        def do_detach_volume(context, volume_id, instance, attachment_id):"},{"line_number":5630,"context_line":"            bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":5631,"context_line":"                    context, volume_id, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fdfeff1_ad01736c","line":5628,"range":{"start_line":5628,"start_character":8,"end_line":5628,"end_character":42},"updated":"2019-02-13 18:23:09.000000000","message":"nit: could you do this via a with statement insstead.\n\ninventing the new fuction seam overkill","commit_id":"de0d24bb4c135cc3cf7c9efbd312ac9e35e04223"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2f38a95723056a777381b07fb5cfe981ef6e9e98","unresolved":false,"context_lines":[{"line_number":5625,"context_line":"                              and volume."},{"line_number":5626,"context_line":""},{"line_number":5627,"context_line":"        \"\"\""},{"line_number":5628,"context_line":"        @utils.synchronized(instance.uuid)"},{"line_number":5629,"context_line":"        def do_detach_volume(context, volume_id, instance, attachment_id):"},{"line_number":5630,"context_line":"            bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":5631,"context_line":"                    context, volume_id, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fdfeff1_cdc71712","line":5628,"range":{"start_line":5628,"start_character":8,"end_line":5628,"end_character":42},"in_reply_to":"9fdfeff1_ad01736c","updated":"2019-02-13 18:30:26.000000000","message":"actully its consistent with the other cases where we syncronise in the file so this is fine.","commit_id":"de0d24bb4c135cc3cf7c9efbd312ac9e35e04223"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"65edd67760c99b3933fb47710ca464f427644c2d","unresolved":false,"context_lines":[{"line_number":5627,"context_line":"        \"\"\""},{"line_number":5628,"context_line":"        @utils.synchronized(instance.uuid)"},{"line_number":5629,"context_line":"        def do_detach_volume(context, volume_id, instance, attachment_id):"},{"line_number":5630,"context_line":"            bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":5631,"context_line":"                    context, volume_id, instance.uuid)"},{"line_number":5632,"context_line":"            self._detach_volume(context, bdm, instance,"},{"line_number":5633,"context_line":"                                attachment_id\u003dattachment_id)"},{"line_number":5634,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"9fdfeff1_6d014b38","line":5631,"range":{"start_line":5630,"start_character":12,"end_line":5631,"end_character":54},"updated":"2019-02-13 18:31:45.000000000","message":"Why not fetch the BDM outside the lock?","commit_id":"f4451b6f2de16308ee9783ea44914f0ccf15ac01"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"8212fa498df6cfd5badb6e717fbb7a964ed25b6b","unresolved":false,"context_lines":[{"line_number":5626,"context_line":""},{"line_number":5627,"context_line":"        \"\"\""},{"line_number":5628,"context_line":"        bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":5629,"context_line":"                    context, volume_id, instance.uuid)"},{"line_number":5630,"context_line":""},{"line_number":5631,"context_line":"        @utils.synchronized(instance.uuid)"},{"line_number":5632,"context_line":"        def do_detach_volume(context, volume_id, instance, attachment_id):"}],"source_content_type":"text/x-python","patch_set":4,"id":"9fdfeff1_ed51bb37","line":5629,"range":{"start_line":5629,"start_character":16,"end_line":5629,"end_character":20},"updated":"2019-02-13 18:34:05.000000000","message":"guh","commit_id":"b35c6b3bfdab2f0277cc692393532d1b6ee7d113"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"2d33e4f251042f22042f478438964a4399f797d5","unresolved":false,"context_lines":[{"line_number":5625,"context_line":"                              and volume."},{"line_number":5626,"context_line":""},{"line_number":5627,"context_line":"        \"\"\""},{"line_number":5628,"context_line":"        bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":5629,"context_line":"                context, volume_id, instance.uuid)"},{"line_number":5630,"context_line":""},{"line_number":5631,"context_line":"        @utils.synchronized(instance.uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fdfeff1_f02d1095","line":5628,"updated":"2019-02-13 18:38:11.000000000","message":"I guess the only concern here would be if this call can race too, in the context that\u0027s being described in the bug. I don\u0027t know what get_by_volume_and_instance does, so *shrug*?","commit_id":"74ce522ad847ad84220ee155adf8b008f51c49d5"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"3aa1f296d2c442c465dad140db94a269adc60f3f","unresolved":false,"context_lines":[{"line_number":5625,"context_line":"                              and volume."},{"line_number":5626,"context_line":""},{"line_number":5627,"context_line":"        \"\"\""},{"line_number":5628,"context_line":"        bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":5629,"context_line":"                context, volume_id, instance.uuid)"},{"line_number":5630,"context_line":""},{"line_number":5631,"context_line":"        @utils.synchronized(instance.uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fdfeff1_1c678bb9","line":5628,"in_reply_to":"9fdfeff1_6122b4bb","updated":"2019-02-13 22:13:31.000000000","message":"OK, then let\u0027s change it back.","commit_id":"74ce522ad847ad84220ee155adf8b008f51c49d5"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"8947d17ad96e9572cda45c1c48abf933987aab81","unresolved":false,"context_lines":[{"line_number":5625,"context_line":"                              and volume."},{"line_number":5626,"context_line":""},{"line_number":5627,"context_line":"        \"\"\""},{"line_number":5628,"context_line":"        bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":5629,"context_line":"                context, volume_id, instance.uuid)"},{"line_number":5630,"context_line":""},{"line_number":5631,"context_line":"        @utils.synchronized(instance.uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fdfeff1_6122b4bb","line":5628,"in_reply_to":"9fdfeff1_e10b048b","updated":"2019-02-13 21:55:02.000000000","message":"Aside: if the record _has_ already been deleted when get_by_volume_and_instance is called, it will raise exception.VolumeBDMNotFound, which isn\u0027t handled anywhere. AFAICT, it would log an exception and set the instance fault with VolumeBDMNotFound.","commit_id":"74ce522ad847ad84220ee155adf8b008f51c49d5"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"97bf2c3461171dba27f313cc1c0421e4a92a2718","unresolved":false,"context_lines":[{"line_number":5625,"context_line":"                              and volume."},{"line_number":5626,"context_line":""},{"line_number":5627,"context_line":"        \"\"\""},{"line_number":5628,"context_line":"        bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":5629,"context_line":"                context, volume_id, instance.uuid)"},{"line_number":5630,"context_line":""},{"line_number":5631,"context_line":"        @utils.synchronized(instance.uuid)"}],"source_content_type":"text/x-python","patch_set":5,"id":"9fdfeff1_e10b048b","line":5628,"in_reply_to":"9fdfeff1_f02d1095","updated":"2019-02-13 21:37:49.000000000","message":"This looks up the block device mapping record for the attached volume from the database. After the volume is detached, this record will be destroyed.\n\nThis call can race, and if detach 2 finds the block device record while detach 1 is still in-progress and hasn\u0027t yet deleted the record, we will attempt to detach the volume a second time at the driver level and cinder level (see nova/virt/block_device.py) again, which might yield the same problem described in the bug.\n\nUnless I\u0027m missing something, it seems like we should lock around the block device record lookup here.","commit_id":"74ce522ad847ad84220ee155adf8b008f51c49d5"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"6f36d9016e369948a2ef8f92ea3f9d3e4be9c83b","unresolved":false,"context_lines":[{"line_number":5633,"context_line":"                              and volume."},{"line_number":5634,"context_line":""},{"line_number":5635,"context_line":"        \"\"\""},{"line_number":5636,"context_line":"        @utils.synchronized(instance.uuid)"},{"line_number":5637,"context_line":"        def do_detach_volume(context, volume_id, instance, attachment_id):"},{"line_number":5638,"context_line":"            bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":5639,"context_line":"                    context, volume_id, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9fdfeff1_ba0f552c","line":5636,"range":{"start_line":5636,"start_character":28,"end_line":5636,"end_character":41},"updated":"2019-02-14 11:22:52.000000000","message":"I know the original attach change uses instance.uuid but in \n this brave new world of multi attach support shouldn\u0027t this be on the volume_id to avoid races between detach calls against separate instances?","commit_id":"2e4e203c1a4c4f91dd1aabae8c8d7b745df99be7"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"a939c6476bcad85c8e1067b4b044f79c5139f4a7","unresolved":false,"context_lines":[{"line_number":5633,"context_line":"                              and volume."},{"line_number":5634,"context_line":""},{"line_number":5635,"context_line":"        \"\"\""},{"line_number":5636,"context_line":"        @utils.synchronized(instance.uuid)"},{"line_number":5637,"context_line":"        def do_detach_volume(context, volume_id, instance, attachment_id):"},{"line_number":5638,"context_line":"            bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":5639,"context_line":"                    context, volume_id, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9fdfeff1_dd96d33e","line":5636,"range":{"start_line":5636,"start_character":28,"end_line":5636,"end_character":41},"in_reply_to":"9fdfeff1_7d66c7bd","updated":"2019-02-14 12:36:28.000000000","message":"Yeah my apologies, I missed that we take this same lock when deleting instances.","commit_id":"2e4e203c1a4c4f91dd1aabae8c8d7b745df99be7"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"8015143298ffc45de68cba90c7d496851d8b05ea","unresolved":false,"context_lines":[{"line_number":5633,"context_line":"                              and volume."},{"line_number":5634,"context_line":""},{"line_number":5635,"context_line":"        \"\"\""},{"line_number":5636,"context_line":"        @utils.synchronized(instance.uuid)"},{"line_number":5637,"context_line":"        def do_detach_volume(context, volume_id, instance, attachment_id):"},{"line_number":5638,"context_line":"            bdm \u003d objects.BlockDeviceMapping.get_by_volume_and_instance("},{"line_number":5639,"context_line":"                    context, volume_id, instance.uuid)"}],"source_content_type":"text/x-python","patch_set":7,"id":"9fdfeff1_7d66c7bd","line":5636,"range":{"start_line":5636,"start_character":28,"end_line":5636,"end_character":41},"in_reply_to":"9fdfeff1_ba0f552c","updated":"2019-02-14 12:22:03.000000000","message":"for the specific race the volume_id would not work as that code flow is not atomic for attach and shutdown. So the same race will happen.\nI think that the multi-attach problem is orthogonal and should be investigated in another patch.","commit_id":"2e4e203c1a4c4f91dd1aabae8c8d7b745df99be7"}]}