)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"28322c9c3300e965f047516ef58656a3c9afb294","unresolved":false,"context_lines":[{"line_number":12,"context_line":"the BDM connection_info. Use the stored connector when"},{"line_number":13,"context_line":"we detect that we have this wrong host situation."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Since the above fix does not work for existing attachments"},{"line_number":16,"context_line":"(made prior to this patch), also add \u0027instance_host\u0027 to"},{"line_number":17,"context_line":"to the connector when we detect that the instance host is"},{"line_number":18,"context_line":"different from the connector host. With this additional"},{"line_number":19,"context_line":"information, cinder drivers can be fixed to do the right"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7a5de9d1_33f63d90","line":16,"range":{"start_line":15,"start_character":0,"end_line":16,"end_character":26},"updated":"2016-01-28 10:39:32.000000000","message":"...because existing attachments don\u0027t have the original connector info stashed in the bdm.connection_info.","commit_id":"e9520ad751065e5b33b7c6b8efb1bca2c1405d31"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"7b23cb57e5f1f16216c269d7cf05b4f9cc4dff4b","unresolved":false,"context_lines":[{"line_number":12,"context_line":"the BDM connection_info. Use the stored connector when"},{"line_number":13,"context_line":"we detect that we have this wrong host situation."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Since the above fix does not work for existing attachments"},{"line_number":16,"context_line":"(made prior to this patch), also add \u0027instance_host\u0027 to"},{"line_number":17,"context_line":"to the connector when we detect that the instance host is"},{"line_number":18,"context_line":"different from the connector host. With this additional"},{"line_number":19,"context_line":"information, cinder drivers can be fixed to do the right"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7a5de9d1_503fcb23","line":16,"range":{"start_line":15,"start_character":0,"end_line":16,"end_character":26},"in_reply_to":"7a5de9d1_33f63d90","updated":"2016-01-29 22:34:08.000000000","message":"Done","commit_id":"e9520ad751065e5b33b7c6b8efb1bca2c1405d31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"28322c9c3300e965f047516ef58656a3c9afb294","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Since the above fix does not work for existing attachments"},{"line_number":16,"context_line":"(made prior to this patch), also add \u0027instance_host\u0027 to"},{"line_number":17,"context_line":"to the connector when we detect that the instance host is"},{"line_number":18,"context_line":"different from the connector host. With this additional"},{"line_number":19,"context_line":"information, cinder drivers can be fixed to do the right"},{"line_number":20,"context_line":"thing when the instance host does not match the connector"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7a5de9d1_d349e9d1","line":17,"range":{"start_line":17,"start_character":7,"end_line":17,"end_character":16},"updated":"2016-01-28 10:39:32.000000000","message":"connector from the current host (which is not the correct host for the volume connection on the original host) - right?","commit_id":"e9520ad751065e5b33b7c6b8efb1bca2c1405d31"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"7b23cb57e5f1f16216c269d7cf05b4f9cc4dff4b","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Since the above fix does not work for existing attachments"},{"line_number":16,"context_line":"(made prior to this patch), also add \u0027instance_host\u0027 to"},{"line_number":17,"context_line":"to the connector when we detect that the instance host is"},{"line_number":18,"context_line":"different from the connector host. With this additional"},{"line_number":19,"context_line":"information, cinder drivers can be fixed to do the right"},{"line_number":20,"context_line":"thing when the instance host does not match the connector"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7a5de9d1_5026eb15","line":17,"range":{"start_line":17,"start_character":7,"end_line":17,"end_character":16},"in_reply_to":"7a5de9d1_d349e9d1","updated":"2016-01-29 22:34:08.000000000","message":"Moving instance_host part to a separate patch. Will try to clarify these points in that one.","commit_id":"e9520ad751065e5b33b7c6b8efb1bca2c1405d31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"28322c9c3300e965f047516ef58656a3c9afb294","unresolved":false,"context_lines":[{"line_number":15,"context_line":"Since the above fix does not work for existing attachments"},{"line_number":16,"context_line":"(made prior to this patch), also add \u0027instance_host\u0027 to"},{"line_number":17,"context_line":"to the connector when we detect that the instance host is"},{"line_number":18,"context_line":"different from the connector host. With this additional"},{"line_number":19,"context_line":"information, cinder drivers can be fixed to do the right"},{"line_number":20,"context_line":"thing when the instance host does not match the connector"},{"line_number":21,"context_line":"host. Cinder drivers rely on various parts of the connector"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7a5de9d1_b3178da2","line":18,"range":{"start_line":18,"start_character":19,"end_line":18,"end_character":28},"updated":"2016-01-28 10:39:32.000000000","message":"Again, current connector host, right?","commit_id":"e9520ad751065e5b33b7c6b8efb1bca2c1405d31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"28322c9c3300e965f047516ef58656a3c9afb294","unresolved":false,"context_lines":[{"line_number":16,"context_line":"(made prior to this patch), also add \u0027instance_host\u0027 to"},{"line_number":17,"context_line":"to the connector when we detect that the instance host is"},{"line_number":18,"context_line":"different from the connector host. With this additional"},{"line_number":19,"context_line":"information, cinder drivers can be fixed to do the right"},{"line_number":20,"context_line":"thing when the instance host does not match the connector"},{"line_number":21,"context_line":"host. Cinder drivers rely on various parts of the connector"},{"line_number":22,"context_line":"to properly terminate, but if that is not reliable some"},{"line_number":23,"context_line":"drivers can discover this information given the volume"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7a5de9d1_33d95df4","line":20,"range":{"start_line":19,"start_character":44,"end_line":20,"end_character":5},"updated":"2016-01-28 10:39:32.000000000","message":"What is the right thing?\n\nI guess the idea is just somehow telling cinder that \u0027this connector is bogus but we don\u0027t know the correct connector because we never stored that information anywhere for this attachment\u0027.","commit_id":"e9520ad751065e5b33b7c6b8efb1bca2c1405d31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"28322c9c3300e965f047516ef58656a3c9afb294","unresolved":false,"context_lines":[{"line_number":19,"context_line":"information, cinder drivers can be fixed to do the right"},{"line_number":20,"context_line":"thing when the instance host does not match the connector"},{"line_number":21,"context_line":"host. Cinder drivers rely on various parts of the connector"},{"line_number":22,"context_line":"to properly terminate, but if that is not reliable some"},{"line_number":23,"context_line":"drivers can discover this information given the volume"},{"line_number":24,"context_line":"and host."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Change-Id: I793f2996fc0af1c321a240ad9348dc9bce816030"},{"line_number":27,"context_line":"Closes-Bug: #1522496"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7a5de9d1_b34f8d91","line":24,"range":{"start_line":22,"start_character":23,"end_line":24,"end_character":9},"updated":"2016-01-28 10:39:32.000000000","message":"Can\u0027t the cinder drivers figure this out anyway? Like, can\u0027t it validate what\u0027s passed in and if it\u0027s wrong, rebuild the information - which it sounds like they are going to be doing anyway?  Or is it too much to add the validation checking on every terminate_connection call?","commit_id":"e9520ad751065e5b33b7c6b8efb1bca2c1405d31"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"e17dc28b189eaa6c02d58c6526ad4e512cb47a46","unresolved":false,"context_lines":[{"line_number":19,"context_line":"information, cinder drivers can be fixed to do the right"},{"line_number":20,"context_line":"thing when the instance host does not match the connector"},{"line_number":21,"context_line":"host. Cinder drivers rely on various parts of the connector"},{"line_number":22,"context_line":"to properly terminate, but if that is not reliable some"},{"line_number":23,"context_line":"drivers can discover this information given the volume"},{"line_number":24,"context_line":"and host."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"Change-Id: I793f2996fc0af1c321a240ad9348dc9bce816030"},{"line_number":27,"context_line":"Closes-Bug: #1522496"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"7a5de9d1_37f369a0","line":24,"range":{"start_line":22,"start_character":23,"end_line":24,"end_character":9},"in_reply_to":"7a5de9d1_b34f8d91","updated":"2016-01-28 19:05:05.000000000","message":"Currently some would raise an error (let\u0027s call that validate), but some would either ignore the error or just happily try to remove access from the new host if found (oops). It looks like not all of them can fix this. Those could be fixed to fail more reliably. I\u0027m not sure if leaving dangling connections would be considered OK by some vendors(?). If the \u0027instance_host\u0027 approach is used, it would be easy to code a default fail that drivers need to override with a fix. That might be a hard sell. My approach was to let drivers provide their fixes on their own schedule as a bugfix for M (and L). Maybe it would make more sense to enforce a fix in N API discussions(?).","commit_id":"e9520ad751065e5b33b7c6b8efb1bca2c1405d31"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"7773e6e3a2d30af2aaa5e1a4325a4eeff5760171","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Provide correct connector for evacuate terminate"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"During evacuate a local connector is built. This is the"},{"line_number":10,"context_line":"wrong connector to use for cinder terminate_connection."},{"line_number":11,"context_line":"In order to fix this, store the initial connector with"},{"line_number":12,"context_line":"the BDM connection_info. Use the stored connector when"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"3a57f1b5_dde8d6e4","line":9,"updated":"2016-02-11 15:36:54.000000000","message":"nit - evacuation","commit_id":"08ff7b101494e311f88fffd7aeea26dbf6f8568c"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"7daa6bc0f11e7db679f0019277bc64f0b1e82009","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Provide correct connector for evacuate terminate"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"During evacuate a local connector is built. This is the"},{"line_number":10,"context_line":"wrong connector to use for cinder terminate_connection."},{"line_number":11,"context_line":"In order to fix this, store the initial connector with"},{"line_number":12,"context_line":"the BDM connection_info. Use the stored connector when"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"3a57f1b5_e3a2b7fc","line":9,"in_reply_to":"3a57f1b5_dde8d6e4","updated":"2016-02-12 03:23:56.000000000","message":"Done","commit_id":"08ff7b101494e311f88fffd7aeea26dbf6f8568c"}],"nova/compute/manager.py":[{"author":{"_account_id":1865,"name":"Andrea Rosa","email":"andrea.rosa@gmail.com","username":"andrea-rosa-m"},"change_message_id":"f802dfa4061d3a56d78ebef8ba419825ab70c2f4","unresolved":false,"context_lines":[{"line_number":4782,"context_line":"                connection_info \u003d jsonutils.loads(bdm.connection_info)"},{"line_number":4783,"context_line":"                if connection_info:"},{"line_number":4784,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4785,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4786,"context_line":"        except Exception as e:"},{"line_number":4787,"context_line":"            LOG.debug(\u0027_detach_volume attempt to fix connector raised: %s.\u0027,"},{"line_number":4788,"context_line":"                      six.text_type(e))"}],"source_content_type":"text/x-python","patch_set":1,"id":"da6ed579_a2aaeb17","line":4785,"updated":"2016-01-12 16:13:55.000000000","message":"once you have set the correct value for the connector I think it should be saved in the DB","commit_id":"280d0fa8e82ee4491fe6681c57e2c7214baab559"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"615022b843b9b6e25ef4e2754246efb3d9a0e6f1","unresolved":false,"context_lines":[{"line_number":4782,"context_line":"                connection_info \u003d jsonutils.loads(bdm.connection_info)"},{"line_number":4783,"context_line":"                if connection_info:"},{"line_number":4784,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4785,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4786,"context_line":"        except Exception as e:"},{"line_number":4787,"context_line":"            LOG.debug(\u0027_detach_volume attempt to fix connector raised: %s.\u0027,"},{"line_number":4788,"context_line":"                      six.text_type(e))"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a68dd71_30d593b8","line":4785,"in_reply_to":"9a68dd71_ba32d90e","updated":"2016-01-22 11:06:21.000000000","message":"Apologies again but I don\u0027t follow, you\u0027ve fixed the issue by ensuring terminate_connection is called with the correct connector data. Thus the next time nova calls initialize_connection the current hosts connector data is used correctly.\n\nWhy do we need to duplicate the above logic in each drivers terminate_connection method on the Cinder side?","commit_id":"280d0fa8e82ee4491fe6681c57e2c7214baab559"},{"author":{"_account_id":1865,"name":"Andrea Rosa","email":"andrea.rosa@gmail.com","username":"andrea-rosa-m"},"change_message_id":"7319ddd2fe797728b98b23d75560fb83d54a90ff","unresolved":false,"context_lines":[{"line_number":4782,"context_line":"                connection_info \u003d jsonutils.loads(bdm.connection_info)"},{"line_number":4783,"context_line":"                if connection_info:"},{"line_number":4784,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4785,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4786,"context_line":"        except Exception as e:"},{"line_number":4787,"context_line":"            LOG.debug(\u0027_detach_volume attempt to fix connector raised: %s.\u0027,"},{"line_number":4788,"context_line":"                      six.text_type(e))"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba63e191_e458dafc","line":4785,"in_reply_to":"ba63e191_2cd902f4","updated":"2016-01-19 17:13:37.000000000","message":"What about save it if destroy_bdm is False? do you think is better?","commit_id":"280d0fa8e82ee4491fe6681c57e2c7214baab559"},{"author":{"_account_id":1865,"name":"Andrea Rosa","email":"andrea.rosa@gmail.com","username":"andrea-rosa-m"},"change_message_id":"924b66ee134458854b499e020c08a7b5bc970cb6","unresolved":false,"context_lines":[{"line_number":4782,"context_line":"                connection_info \u003d jsonutils.loads(bdm.connection_info)"},{"line_number":4783,"context_line":"                if connection_info:"},{"line_number":4784,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4785,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4786,"context_line":"        except Exception as e:"},{"line_number":4787,"context_line":"            LOG.debug(\u0027_detach_volume attempt to fix connector raised: %s.\u0027,"},{"line_number":4788,"context_line":"                      six.text_type(e))"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a68dd71_bfd8ddf0","line":4785,"in_reply_to":"ba63e191_47518161","updated":"2016-01-20 16:36:07.000000000","message":"I think you are right, this is the field used by existing attachments, pre-patch.","commit_id":"280d0fa8e82ee4491fe6681c57e2c7214baab559"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"c064071e7f0787be4d412cf884280304c747267e","unresolved":false,"context_lines":[{"line_number":4782,"context_line":"                connection_info \u003d jsonutils.loads(bdm.connection_info)"},{"line_number":4783,"context_line":"                if connection_info:"},{"line_number":4784,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4785,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4786,"context_line":"        except Exception as e:"},{"line_number":4787,"context_line":"            LOG.debug(\u0027_detach_volume attempt to fix connector raised: %s.\u0027,"},{"line_number":4788,"context_line":"                      six.text_type(e))"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a68dd71_ba32d90e","line":4785,"in_reply_to":"ba63e191_47518161","updated":"2016-01-21 23:36:27.000000000","message":"I\u0027m adding \"instance_host\" because I need a way for Cinder drivers to detect that attachments that were made before this patch only have the old wrong connector. A Cinder driver can use that tip to fix things. First to do so is in this patch https://review.openstack.org/#/c/266098/1/cinder/volume/drivers/hpe/hpe_3par_common.py  Otherwise nova can only fix this on new attachments.  I don\u0027t see a need to save instance_host. That one is intentionally pulled from the current instance info.","commit_id":"280d0fa8e82ee4491fe6681c57e2c7214baab559"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"af2e58a61ea8c38c03922d9fb851e49e0d143cf8","unresolved":false,"context_lines":[{"line_number":4782,"context_line":"                connection_info \u003d jsonutils.loads(bdm.connection_info)"},{"line_number":4783,"context_line":"                if connection_info:"},{"line_number":4784,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4785,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4786,"context_line":"        except Exception as e:"},{"line_number":4787,"context_line":"            LOG.debug(\u0027_detach_volume attempt to fix connector raised: %s.\u0027,"},{"line_number":4788,"context_line":"                      six.text_type(e))"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba63e191_47518161","line":4785,"in_reply_to":"ba63e191_e458dafc","updated":"2016-01-20 09:52:28.000000000","message":"No I don\u0027t think that is needed, we would only be adding an instance_host value pointing to the host we are about to detach from. I\u0027m not even sure of the value of adding instance_host to the connector in the first place. I can only find two references to it in the Cinder code base within some hpe3par tests. Apologies in advance if I\u0027m missing the point here.","commit_id":"280d0fa8e82ee4491fe6681c57e2c7214baab559"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"390e3656b8c99eff017ec446a4e8447b388c4377","unresolved":false,"context_lines":[{"line_number":4782,"context_line":"                connection_info \u003d jsonutils.loads(bdm.connection_info)"},{"line_number":4783,"context_line":"                if connection_info:"},{"line_number":4784,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4785,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4786,"context_line":"        except Exception as e:"},{"line_number":4787,"context_line":"            LOG.debug(\u0027_detach_volume attempt to fix connector raised: %s.\u0027,"},{"line_number":4788,"context_line":"                      six.text_type(e))"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba63e191_2cd902f4","line":4785,"in_reply_to":"da6ed579_a2aaeb17","updated":"2016-01-19 15:45:34.000000000","message":"I disagree, at this point the connector data is for another host and is already associated with a bdm in the DB. We could write this back to the bdm if we update the instance_host but in all likelihood the bdm will be destroyed below anyway.","commit_id":"280d0fa8e82ee4491fe6681c57e2c7214baab559"},{"author":{"_account_id":1865,"name":"Andrea Rosa","email":"andrea.rosa@gmail.com","username":"andrea-rosa-m"},"change_message_id":"f802dfa4061d3a56d78ebef8ba419825ab70c2f4","unresolved":false,"context_lines":[{"line_number":4783,"context_line":"                if connection_info:"},{"line_number":4784,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4785,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4786,"context_line":"        except Exception as e:"},{"line_number":4787,"context_line":"            LOG.debug(\u0027_detach_volume attempt to fix connector raised: %s.\u0027,"},{"line_number":4788,"context_line":"                      six.text_type(e))"},{"line_number":4789,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"da6ed579_fd6f2690","line":4786,"range":{"start_line":4786,"start_character":8,"end_line":4786,"end_character":30},"updated":"2016-01-12 16:13:55.000000000","message":"we usually prefer to avoid to use \"except Exception\" but we prefer to define the specific exceptions we know that could be raised. In that case the only dangerous line of code it seems when we loads the connection_info at line 4782.","commit_id":"280d0fa8e82ee4491fe6681c57e2c7214baab559"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"c064071e7f0787be4d412cf884280304c747267e","unresolved":false,"context_lines":[{"line_number":4783,"context_line":"                if connection_info:"},{"line_number":4784,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4785,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4786,"context_line":"        except Exception as e:"},{"line_number":4787,"context_line":"            LOG.debug(\u0027_detach_volume attempt to fix connector raised: %s.\u0027,"},{"line_number":4788,"context_line":"                      six.text_type(e))"},{"line_number":4789,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9a68dd71_e551960f","line":4786,"range":{"start_line":4786,"start_character":8,"end_line":4786,"end_character":30},"in_reply_to":"da6ed579_fd6f2690","updated":"2016-01-21 23:36:27.000000000","message":"I was being over-protective. Also, some tests cause exceptions because they use bogus fake connectors. My thinking behind the broad try/except is that I wanted this bug-fix to just log and continue if _anything_ went wrong. Some cinder drivers might prefer to remain as-is when I can\u0027t properly do the fix. Please reply if that is not acceptable.","commit_id":"280d0fa8e82ee4491fe6681c57e2c7214baab559"},{"author":{"_account_id":1865,"name":"Andrea Rosa","email":"andrea.rosa@gmail.com","username":"andrea-rosa-m"},"change_message_id":"7ba5e97e33d79381e73c1a8b1af7a4ae4b40b3e5","unresolved":false,"context_lines":[{"line_number":4796,"context_line":"                if connection_info:"},{"line_number":4797,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4798,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4799,"context_line":"        except Exception as e:"},{"line_number":4800,"context_line":"            LOG.debug(\u0027_detach_volume attempt to fix connector raised: %s.\u0027,"},{"line_number":4801,"context_line":"                      six.text_type(e))"},{"line_number":4802,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a68dd71_720bc12a","line":4799,"range":{"start_line":4799,"start_character":7,"end_line":4799,"end_character":30},"updated":"2016-01-25 09:34:13.000000000","message":"looking at the code I think that the except should go just around the jsonutils.loads statement, checking at the definition of the json.loads I found that \n\"If the data being deserialized is not a valid JSON document, a JSONDecodeError will be raised.\"\nwhat about wrapping just that line in an try except and then catch just the JSONDecodeError exception?","commit_id":"bba7ab27a2ae4d6a5e79bed909276df74135e9aa"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"c25e6053007db98442eb49ac7fd37cb8fac342c1","unresolved":false,"context_lines":[{"line_number":4796,"context_line":"                if connection_info:"},{"line_number":4797,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4798,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4799,"context_line":"        except Exception as e:"},{"line_number":4800,"context_line":"            LOG.debug(\u0027_detach_volume attempt to fix connector raised: %s.\u0027,"},{"line_number":4801,"context_line":"                      six.text_type(e))"},{"line_number":4802,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a68dd71_b6fca372","line":4799,"range":{"start_line":4799,"start_character":7,"end_line":4799,"end_character":30},"in_reply_to":"9a68dd71_720bc12a","updated":"2016-01-25 23:59:04.000000000","message":"The same jsonutils.loads happens in _driver_detach_volume before here. So I made that return connection_info for re-use. That only left a potential error on a bad connector (probably only to be hit in tests) so I added and if and got rid of the try/except completely.","commit_id":"bba7ab27a2ae4d6a5e79bed909276df74135e9aa"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ca8987c1204676091564983c6a91e5777d165768","unresolved":false,"context_lines":[{"line_number":4791,"context_line":"        # was stored in connection_info instead (if we have one)."},{"line_number":4792,"context_line":"        # Also add the instance host to the connector so cinder can figure"},{"line_number":4793,"context_line":"        # things out either way."},{"line_number":4794,"context_line":"        if isinstance(connector, dict):"},{"line_number":4795,"context_line":"            instance_host \u003d instance.get(\u0027host\u0027)"},{"line_number":4796,"context_line":"            if connector.get(\u0027host\u0027) !\u003d instance_host:"},{"line_number":4797,"context_line":"                if connection_info:"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a5de9d1_e9cb111d","line":4794,"updated":"2016-01-27 10:07:12.000000000","message":"This is gross, why wouldn\u0027t it be a dict? The virt driver base class says it will be a dict:\n\nhttps://github.com/openstack/nova/blob/master/nova/virt/driver.py#L1396\n\nAnd we treat the connector like a dict in the volume_api.initialize_connection method (if things fail).","commit_id":"ce1f2f245b59885c0ccf295ac231935b930f3b24"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"0ac3ad9b5fad1f68dd521ba6ff099b49828d77c0","unresolved":false,"context_lines":[{"line_number":4791,"context_line":"        # was stored in connection_info instead (if we have one)."},{"line_number":4792,"context_line":"        # Also add the instance host to the connector so cinder can figure"},{"line_number":4793,"context_line":"        # things out either way."},{"line_number":4794,"context_line":"        if isinstance(connector, dict):"},{"line_number":4795,"context_line":"            instance_host \u003d instance.get(\u0027host\u0027)"},{"line_number":4796,"context_line":"            if connector.get(\u0027host\u0027) !\u003d instance_host:"},{"line_number":4797,"context_line":"                if connection_info:"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a5de9d1_340e3935","line":4794,"in_reply_to":"7a5de9d1_3a639bb3","updated":"2016-01-27 22:45:20.000000000","message":"Done","commit_id":"ce1f2f245b59885c0ccf295ac231935b930f3b24"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"d4b782324fb5f8bee261777a76dd95dc09a01fce","unresolved":false,"context_lines":[{"line_number":4791,"context_line":"        # was stored in connection_info instead (if we have one)."},{"line_number":4792,"context_line":"        # Also add the instance host to the connector so cinder can figure"},{"line_number":4793,"context_line":"        # things out either way."},{"line_number":4794,"context_line":"        if isinstance(connector, dict):"},{"line_number":4795,"context_line":"            instance_host \u003d instance.get(\u0027host\u0027)"},{"line_number":4796,"context_line":"            if connector.get(\u0027host\u0027) !\u003d instance_host:"},{"line_number":4797,"context_line":"                if connection_info:"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a5de9d1_3a639bb3","line":4794,"in_reply_to":"7a5de9d1_e9cb111d","updated":"2016-01-27 15:04:07.000000000","message":"OK. I\u0027ll just have to change existing tests. I agree this is ugly, but thought it was safest.","commit_id":"ce1f2f245b59885c0ccf295ac231935b930f3b24"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"20dab4035365350af8b70c0d7bd72f4ce8562113","unresolved":false,"context_lines":[{"line_number":4793,"context_line":"        # things out either way."},{"line_number":4794,"context_line":"        if isinstance(connector, dict):"},{"line_number":4795,"context_line":"            instance_host \u003d instance.get(\u0027host\u0027)"},{"line_number":4796,"context_line":"            if connector.get(\u0027host\u0027) !\u003d instance_host:"},{"line_number":4797,"context_line":"                if connection_info:"},{"line_number":4798,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4799,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a5de9d1_1b49fe8b","line":4796,"updated":"2016-01-27 16:37:24.000000000","message":"destroy_bdm is False in the rebuild/evacuate case, it seems like this would be safer if we only did this stuff when we know we\u0027re doing an evacuate.","commit_id":"ce1f2f245b59885c0ccf295ac231935b930f3b24"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"6709ebc0721e0c63879f2afc4180631555061f59","unresolved":false,"context_lines":[{"line_number":4793,"context_line":"        # things out either way."},{"line_number":4794,"context_line":"        if isinstance(connector, dict):"},{"line_number":4795,"context_line":"            instance_host \u003d instance.get(\u0027host\u0027)"},{"line_number":4796,"context_line":"            if connector.get(\u0027host\u0027) !\u003d instance_host:"},{"line_number":4797,"context_line":"                if connection_info:"},{"line_number":4798,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4799,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a5de9d1_51043514","line":4796,"in_reply_to":"7a5de9d1_1b49fe8b","updated":"2016-01-27 17:00:56.000000000","message":"agreed. will do","commit_id":"ce1f2f245b59885c0ccf295ac231935b930f3b24"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"0ac3ad9b5fad1f68dd521ba6ff099b49828d77c0","unresolved":false,"context_lines":[{"line_number":4793,"context_line":"        # things out either way."},{"line_number":4794,"context_line":"        if isinstance(connector, dict):"},{"line_number":4795,"context_line":"            instance_host \u003d instance.get(\u0027host\u0027)"},{"line_number":4796,"context_line":"            if connector.get(\u0027host\u0027) !\u003d instance_host:"},{"line_number":4797,"context_line":"                if connection_info:"},{"line_number":4798,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4799,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"}],"source_content_type":"text/x-python","patch_set":5,"id":"7a5de9d1_d413255b","line":4796,"in_reply_to":"7a5de9d1_51043514","updated":"2016-01-27 22:45:20.000000000","message":"Done","commit_id":"ce1f2f245b59885c0ccf295ac231935b930f3b24"},{"author":{"_account_id":10628,"name":"Peter Wang","email":"peter.wang13@emc.com","username":"peter_wang"},"change_message_id":"248c173d9d0c4320ea54970ec93fbd6f08de01a5","unresolved":false,"context_lines":[{"line_number":4796,"context_line":"            if connector.get(\u0027host\u0027) !\u003d instance_host:"},{"line_number":4797,"context_line":"                if connection_info:"},{"line_number":4798,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4799,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4800,"context_line":""},{"line_number":4801,"context_line":"        self.volume_api.terminate_connection(context, volume_id, connector)"},{"line_number":4802,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"9a68dd71_41ad2330","line":4799,"updated":"2016-01-27 03:42:45.000000000","message":"Why not simply overwrite the \u0027host\u0027 field with failed host?\nthis is concise and no driver fix needed.\n\nit\u0027s unreasonable to let cinder driver to decide what to do here, this is nova environment change, i do think it\u0027s necessary to expose these logic to cinder, it doesn\u0027t help anything here.\n\nCorrect me if I did not fully understand the situation you are.\n\nThanks\nPeter","commit_id":"ce1f2f245b59885c0ccf295ac231935b930f3b24"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"06095b7f5a505893adcf10586c72800d583ee6b0","unresolved":false,"context_lines":[{"line_number":4796,"context_line":"            if connector.get(\u0027host\u0027) !\u003d instance_host:"},{"line_number":4797,"context_line":"                if connection_info:"},{"line_number":4798,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4799,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4800,"context_line":""},{"line_number":4801,"context_line":"        self.volume_api.terminate_connection(context, volume_id, connector)"},{"line_number":4802,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a5de9d1_e9e1bbff","line":4799,"in_reply_to":"7a5de9d1_2ee279d6","updated":"2016-01-27 16:12:35.000000000","message":"I guess you do point out the instance_host key in the commit message, and if I understand it, it\u0027s basically a flag to tell newer versions of cinder drivers that they are talking to a new enough version of nova to provide this info? Or would you also be planning on backporting this change to like stable/liberty?","commit_id":"ce1f2f245b59885c0ccf295ac231935b930f3b24"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"36209dfe5aa205b2c84ef0eed4b58cbe46e4c8cd","unresolved":false,"context_lines":[{"line_number":4796,"context_line":"            if connector.get(\u0027host\u0027) !\u003d instance_host:"},{"line_number":4797,"context_line":"                if connection_info:"},{"line_number":4798,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4799,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4800,"context_line":""},{"line_number":4801,"context_line":"        self.volume_api.terminate_connection(context, volume_id, connector)"},{"line_number":4802,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a5de9d1_1eaa4cd5","line":4799,"in_reply_to":"7a5de9d1_5aad3787","updated":"2016-01-27 16:40:43.000000000","message":"I guess this helps for context:\n\nhttps://review.openstack.org/#/c/266098/7/cinder/volume/drivers/hpe/hpe_3par_common.py\n\nThat\u0027s at least more clear to me than the LeftHand change.","commit_id":"ce1f2f245b59885c0ccf295ac231935b930f3b24"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"28f1f068650ab6b36e7ae5dd47b684836609e52d","unresolved":false,"context_lines":[{"line_number":4796,"context_line":"            if connector.get(\u0027host\u0027) !\u003d instance_host:"},{"line_number":4797,"context_line":"                if connection_info:"},{"line_number":4798,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4799,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4800,"context_line":""},{"line_number":4801,"context_line":"        self.volume_api.terminate_connection(context, volume_id, connector)"},{"line_number":4802,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a5de9d1_2ee279d6","line":4799,"in_reply_to":"7a5de9d1_5aad3787","updated":"2016-01-27 16:01:06.000000000","message":"That needs to be clearly explained in the commit message because this is technically a change in the API between nova and cinder so we need to think this through.","commit_id":"ce1f2f245b59885c0ccf295ac231935b930f3b24"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"d4b782324fb5f8bee261777a76dd95dc09a01fce","unresolved":false,"context_lines":[{"line_number":4796,"context_line":"            if connector.get(\u0027host\u0027) !\u003d instance_host:"},{"line_number":4797,"context_line":"                if connection_info:"},{"line_number":4798,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4799,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4800,"context_line":""},{"line_number":4801,"context_line":"        self.volume_api.terminate_connection(context, volume_id, connector)"},{"line_number":4802,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a5de9d1_5aad3787","line":4799,"in_reply_to":"7a5de9d1_695ba155","updated":"2016-01-27 15:04:07.000000000","message":"The connector would still have wrong wwpns and initiator. Cinder drivers are using these. I need a way for Cinder to detect that nova doesn\u0027t know the real connector.","commit_id":"ce1f2f245b59885c0ccf295ac231935b930f3b24"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ca8987c1204676091564983c6a91e5777d165768","unresolved":false,"context_lines":[{"line_number":4796,"context_line":"            if connector.get(\u0027host\u0027) !\u003d instance_host:"},{"line_number":4797,"context_line":"                if connection_info:"},{"line_number":4798,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4799,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4800,"context_line":""},{"line_number":4801,"context_line":"        self.volume_api.terminate_connection(context, volume_id, connector)"},{"line_number":4802,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a5de9d1_695ba155","line":4799,"in_reply_to":"9a68dd71_41ad2330","updated":"2016-01-27 10:07:12.000000000","message":"Agree, we should just override host if that\u0027s the problem key, and cinder should be expecting the host key in the connector dict. This is otherwise changing the API in cinder (at least to expect a new magic key in the dict). Long-term we really want to model the connector as a versioned object so we can\u0027t simply do stuff like this and pass it around and hope that cinder is at a version new enough to be able to handle it.","commit_id":"ce1f2f245b59885c0ccf295ac231935b930f3b24"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7466c098667bb1fe2f96d77df29758d8d5fe1b86","unresolved":false,"context_lines":[{"line_number":4783,"context_line":"                self.notifier.info(context, \u0027volume.usage\u0027,"},{"line_number":4784,"context_line":"                                   compute_utils.usage_volume_info(vol_usage))"},{"line_number":4785,"context_line":""},{"line_number":4786,"context_line":"        connection_info \u003d self._driver_detach_volume(context, instance, bdm)"},{"line_number":4787,"context_line":"        connector \u003d self.driver.get_volume_connector(instance)"},{"line_number":4788,"context_line":""},{"line_number":4789,"context_line":"        if destroy_bdm:"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a5de9d1_d55551c6","line":4786,"updated":"2016-01-28 10:16:41.000000000","message":"The connection_info here is from the original initialize-connection call on the original volume attach, which starting with this change contains the original connector information (for the original host that the volume was connected to). (thinking/reviewing out loud to follow the flow here)","commit_id":"e9520ad751065e5b33b7c6b8efb1bca2c1405d31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7466c098667bb1fe2f96d77df29758d8d5fe1b86","unresolved":false,"context_lines":[{"line_number":4784,"context_line":"                                   compute_utils.usage_volume_info(vol_usage))"},{"line_number":4785,"context_line":""},{"line_number":4786,"context_line":"        connection_info \u003d self._driver_detach_volume(context, instance, bdm)"},{"line_number":4787,"context_line":"        connector \u003d self.driver.get_volume_connector(instance)"},{"line_number":4788,"context_line":""},{"line_number":4789,"context_line":"        if destroy_bdm:"},{"line_number":4790,"context_line":"            # If the volume is attached to another host (evacuate) then"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a5de9d1_35db857e","line":4787,"updated":"2016-01-28 10:16:41.000000000","message":"So here we get the volume connector, which in the case of the libvirt driver is using os-brick, which is using CONF.host for the compute host we\u0027re on. Which you\u0027re saying is potentially not the host that the volume is connected to (in the case of evacuate because we\u0027re rebuilding the instance on a new host).","commit_id":"e9520ad751065e5b33b7c6b8efb1bca2c1405d31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7466c098667bb1fe2f96d77df29758d8d5fe1b86","unresolved":false,"context_lines":[{"line_number":4786,"context_line":"        connection_info \u003d self._driver_detach_volume(context, instance, bdm)"},{"line_number":4787,"context_line":"        connector \u003d self.driver.get_volume_connector(instance)"},{"line_number":4788,"context_line":""},{"line_number":4789,"context_line":"        if destroy_bdm:"},{"line_number":4790,"context_line":"            # If the volume is attached to another host (evacuate) then"},{"line_number":4791,"context_line":"            # this connector is for the wrong host. Use the connector that"},{"line_number":4792,"context_line":"            # was stored in connection_info instead (if we have one)."}],"source_content_type":"text/x-python","patch_set":6,"id":"7a5de9d1_82d6bdc3","line":4789,"updated":"2016-01-28 10:16:41.000000000","message":"If you want to filter for evacuate, I think this is actually supposed to be if not destroy_bdm.","commit_id":"e9520ad751065e5b33b7c6b8efb1bca2c1405d31"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"7b23cb57e5f1f16216c269d7cf05b4f9cc4dff4b","unresolved":false,"context_lines":[{"line_number":4786,"context_line":"        connection_info \u003d self._driver_detach_volume(context, instance, bdm)"},{"line_number":4787,"context_line":"        connector \u003d self.driver.get_volume_connector(instance)"},{"line_number":4788,"context_line":""},{"line_number":4789,"context_line":"        if destroy_bdm:"},{"line_number":4790,"context_line":"            # If the volume is attached to another host (evacuate) then"},{"line_number":4791,"context_line":"            # this connector is for the wrong host. Use the connector that"},{"line_number":4792,"context_line":"            # was stored in connection_info instead (if we have one)."}],"source_content_type":"text/x-python","patch_set":6,"id":"7a5de9d1_104ab3ac","line":4789,"in_reply_to":"7a5de9d1_82d6bdc3","updated":"2016-01-29 22:34:08.000000000","message":"Good catch. That one little change wasn\u0027t properly tested. :(  Will W-I-P it and get that done.","commit_id":"e9520ad751065e5b33b7c6b8efb1bca2c1405d31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7466c098667bb1fe2f96d77df29758d8d5fe1b86","unresolved":false,"context_lines":[{"line_number":4796,"context_line":"            if connector.get(\u0027host\u0027) !\u003d instance_host:"},{"line_number":4797,"context_line":"                if connection_info:"},{"line_number":4798,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4799,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4800,"context_line":""},{"line_number":4801,"context_line":"        self.volume_api.terminate_connection(context, volume_id, connector)"},{"line_number":4802,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7a5de9d1_5841980a","line":4799,"updated":"2016-01-28 10:16:41.000000000","message":"If we\u0027ve detected that the volume connector on *this* host is not the same as the original instance host, then why doesn\u0027t just passing the stashed connector from connection_info do what we want for terminate_connection? Why do we need the instance_host key? Wouldn\u0027t the original connector.host \u003d\u003d instance.host anyway?","commit_id":"e9520ad751065e5b33b7c6b8efb1bca2c1405d31"},{"author":{"_account_id":1865,"name":"Andrea Rosa","email":"andrea.rosa@gmail.com","username":"andrea-rosa-m"},"change_message_id":"1114a2a5fcf6caf2cb16c5309eebbf573beb6f83","unresolved":false,"context_lines":[{"line_number":4796,"context_line":"            if connector.get(\u0027host\u0027) !\u003d instance_host:"},{"line_number":4797,"context_line":"                if connection_info:"},{"line_number":4798,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4799,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4800,"context_line":""},{"line_number":4801,"context_line":"        self.volume_api.terminate_connection(context, volume_id, connector)"},{"line_number":4802,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7a5de9d1_df631919","line":4799,"in_reply_to":"7a5de9d1_5841980a","updated":"2016-01-28 16:06:12.000000000","message":"My understanding is that we need a way to inform cinder that the information passed are not correct and therefore cinder needs to take some actions, probably (not 100% sure) the key point is here: https://review.openstack.org/#/c/266098/7/cinder/volume/drivers/hpe/hpe_3par_common.py@1164\n\n@Mark am I right? If so what about to overwrite the \u0027host\u0027 with the correct value and then add a new flag key but call it in a different way (\"amend_initiator\", yes probably a terrible name) to make it clearer?\n\nUsing insance_host is too confusing.","commit_id":"e9520ad751065e5b33b7c6b8efb1bca2c1405d31"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"d356b9ab8f7eee2a03019b5b0dcbaa147653ff56","unresolved":false,"context_lines":[{"line_number":4796,"context_line":"            if connector.get(\u0027host\u0027) !\u003d instance_host:"},{"line_number":4797,"context_line":"                if connection_info:"},{"line_number":4798,"context_line":"                    connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4799,"context_line":"                connector[\u0027instance_host\u0027] \u003d instance_host"},{"line_number":4800,"context_line":""},{"line_number":4801,"context_line":"        self.volume_api.terminate_connection(context, volume_id, connector)"},{"line_number":4802,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"7a5de9d1_c37c9b74","line":4799,"in_reply_to":"7a5de9d1_df631919","updated":"2016-01-28 17:26:18.000000000","message":"Makes sense, but... Since there are many Cinder drivers and they can have different approaches, I think it\u0027s better to fix all the existing connector values or leave them alone. If we have host/initiator inconsistent (by only fixing host) then we\u0027d have a new kind of bug until all drivers adapt. New bug might be worse or not. I considered \"clearing\" the initiator and wwpns, but that also would not be handled well today. So \"naming\" changes, I\u0027m open to, but I don\u0027t think overwriting the host and leaving the rest is a good idea.","commit_id":"e9520ad751065e5b33b7c6b8efb1bca2c1405d31"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"3a3aa8ce0ad449a68506b7794c07012f4377cf2b","unresolved":false,"context_lines":[{"line_number":4787,"context_line":"        connector \u003d self.driver.get_volume_connector(instance)"},{"line_number":4788,"context_line":""},{"line_number":4789,"context_line":"        if connection_info and not destroy_bdm and ("},{"line_number":4790,"context_line":"           connector.get(\u0027host\u0027) !\u003d instance.host):"},{"line_number":4791,"context_line":"            # If the volume is attached to another host (evacuate) then"},{"line_number":4792,"context_line":"            # this connector is for the wrong host. Use the connector that"},{"line_number":4793,"context_line":"            # was stored in connection_info instead (if we have one)."}],"source_content_type":"text/x-python","patch_set":9,"id":"5a5ae5dd_c5efb685","line":4790,"range":{"start_line":4790,"start_character":11,"end_line":4790,"end_character":49},"updated":"2016-02-04 14:49:25.000000000","message":"This seems like a big assumption - we basically say here - if hosts are the same - we\u0027re all good - which seems like it make a lot of assumptions about os-brick/cinder internals.\n\nThis \"smells\" to me like this validation should be part of os-brick?","commit_id":"434792a27b93f3b8fc79703a5cb9ada2de9174b0"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"1786c3e0a88e425e8b310f9f8211c64b2f35bdd0","unresolved":false,"context_lines":[{"line_number":4787,"context_line":"        connector \u003d self.driver.get_volume_connector(instance)"},{"line_number":4788,"context_line":""},{"line_number":4789,"context_line":"        if connection_info and not destroy_bdm and ("},{"line_number":4790,"context_line":"           connector.get(\u0027host\u0027) !\u003d instance.host):"},{"line_number":4791,"context_line":"            # If the volume is attached to another host (evacuate) then"},{"line_number":4792,"context_line":"            # this connector is for the wrong host. Use the connector that"},{"line_number":4793,"context_line":"            # was stored in connection_info instead (if we have one)."}],"source_content_type":"text/x-python","patch_set":9,"id":"5a5ae5dd_84b457d1","line":4790,"range":{"start_line":4790,"start_character":11,"end_line":4790,"end_character":49},"in_reply_to":"5a5ae5dd_c5efb685","updated":"2016-02-04 17:39:35.000000000","message":"Agree, but as an evacuate bug-fix with backport potential, I\u0027m trying to focus on the evacuate use case. Expanding on that seems more like a future feature or other bugs. Comments might help, but I get too wordy. Suggestions for how to proceed?","commit_id":"434792a27b93f3b8fc79703a5cb9ada2de9174b0"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"e90edfda5e2681daa0473cf2486a99de2d6bca15","unresolved":false,"context_lines":[{"line_number":4787,"context_line":"        connector \u003d self.driver.get_volume_connector(instance)"},{"line_number":4788,"context_line":""},{"line_number":4789,"context_line":"        if connection_info and not destroy_bdm and ("},{"line_number":4790,"context_line":"           connector.get(\u0027host\u0027) !\u003d instance.host):"},{"line_number":4791,"context_line":"            # If the volume is attached to another host (evacuate) then"},{"line_number":4792,"context_line":"            # this connector is for the wrong host. Use the connector that"},{"line_number":4793,"context_line":"            # was stored in connection_info instead (if we have one)."}],"source_content_type":"text/x-python","patch_set":9,"id":"5a5ae5dd_36280b9a","line":4790,"in_reply_to":"5a5ae5dd_c5efb685","updated":"2016-02-05 15:25:59.000000000","message":"Well, cinder should really be storing the connector, validating it before use etc but that\u0027s outside  the scope of this fix. I believe Walter is looking into this [1].\n\n[1] http://lists.openstack.org/pipermail/openstack-dev/2016-February/085795.html","commit_id":"434792a27b93f3b8fc79703a5cb9ada2de9174b0"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"3a3aa8ce0ad449a68506b7794c07012f4377cf2b","unresolved":false,"context_lines":[{"line_number":4791,"context_line":"            # If the volume is attached to another host (evacuate) then"},{"line_number":4792,"context_line":"            # this connector is for the wrong host. Use the connector that"},{"line_number":4793,"context_line":"            # was stored in connection_info instead (if we have one)."},{"line_number":4794,"context_line":"            connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4795,"context_line":""},{"line_number":4796,"context_line":"        self.volume_api.terminate_connection(context, volume_id, connector)"},{"line_number":4797,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"5a5ae5dd_e503d20b","line":4794,"range":{"start_line":4794,"start_character":57,"end_line":4794,"end_character":69},"updated":"2016-02-04 14:49:25.000000000","message":"So we do the wrong thing here even though we know we are doing the wrong thing? Should we at least log this somehow? but ideally not do this since we know it\u0027s a bug?","commit_id":"434792a27b93f3b8fc79703a5cb9ada2de9174b0"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"7721d5134c0413af01b81cebb71e2a246d39a979","unresolved":false,"context_lines":[{"line_number":4791,"context_line":"            # If the volume is attached to another host (evacuate) then"},{"line_number":4792,"context_line":"            # this connector is for the wrong host. Use the connector that"},{"line_number":4793,"context_line":"            # was stored in connection_info instead (if we have one)."},{"line_number":4794,"context_line":"            connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4795,"context_line":""},{"line_number":4796,"context_line":"        self.volume_api.terminate_connection(context, volume_id, connector)"},{"line_number":4797,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"5a5ae5dd_3c60d3a4","line":4794,"in_reply_to":"5a5ae5dd_760a037b","updated":"2016-02-06 05:10:05.000000000","message":"Done. Removed \"or connector\". Added logging. I agree that for a bug-fix it may be best to continue as-is which could work for some. My child patch-set allows more cinder drivers to bug-fix the existing connections problem -- if that lands and the cinder patches land.","commit_id":"434792a27b93f3b8fc79703a5cb9ada2de9174b0"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"e90edfda5e2681daa0473cf2486a99de2d6bca15","unresolved":false,"context_lines":[{"line_number":4791,"context_line":"            # If the volume is attached to another host (evacuate) then"},{"line_number":4792,"context_line":"            # this connector is for the wrong host. Use the connector that"},{"line_number":4793,"context_line":"            # was stored in connection_info instead (if we have one)."},{"line_number":4794,"context_line":"            connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4795,"context_line":""},{"line_number":4796,"context_line":"        self.volume_api.terminate_connection(context, volume_id, connector)"},{"line_number":4797,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"5a5ae5dd_760a037b","line":4794,"in_reply_to":"5a5ae5dd_7f5be62e","updated":"2016-02-05 15:25:59.000000000","message":"So we don\u0027t always fail here, terminate_connection is a noop in some drivers and others might not fail given the current hosts connector. IMHO logging and attempting to terminate_connection with the wrong connector is enough for this patch.","commit_id":"434792a27b93f3b8fc79703a5cb9ada2de9174b0"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"1786c3e0a88e425e8b310f9f8211c64b2f35bdd0","unresolved":false,"context_lines":[{"line_number":4791,"context_line":"            # If the volume is attached to another host (evacuate) then"},{"line_number":4792,"context_line":"            # this connector is for the wrong host. Use the connector that"},{"line_number":4793,"context_line":"            # was stored in connection_info instead (if we have one)."},{"line_number":4794,"context_line":"            connector \u003d connection_info.get(\u0027connector\u0027) or connector"},{"line_number":4795,"context_line":""},{"line_number":4796,"context_line":"        self.volume_api.terminate_connection(context, volume_id, connector)"},{"line_number":4797,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"5a5ae5dd_7f5be62e","line":4794,"range":{"start_line":4794,"start_character":57,"end_line":4794,"end_character":69},"in_reply_to":"5a5ae5dd_e503d20b","updated":"2016-02-04 17:39:35.000000000","message":"Again, I\u0027m limiting impact by continuing in case someone somewhere thought this was working for them. If y\u0027all agree on this, we could fail here. That would disable evac for all boot-from-vol or vol-attached instances that had attachments existing before the patch.","commit_id":"434792a27b93f3b8fc79703a5cb9ada2de9174b0"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"7773e6e3a2d30af2aaa5e1a4325a4eeff5760171","unresolved":false,"context_lines":[{"line_number":4788,"context_line":"        connection_info \u003d self._driver_detach_volume(context, instance, bdm)"},{"line_number":4789,"context_line":"        connector \u003d self.driver.get_volume_connector(instance)"},{"line_number":4790,"context_line":""},{"line_number":4791,"context_line":"        if connection_info and not destroy_bdm and ("},{"line_number":4792,"context_line":"           connector.get(\u0027host\u0027) !\u003d instance.host):"},{"line_number":4793,"context_line":"            # If the volume is attached to another host (evacuate) then"},{"line_number":4794,"context_line":"            # this connector is for the wrong host. Use the connector that"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a57f1b5_1dde6e3f","line":4791,"updated":"2016-02-11 15:36:54.000000000","message":"Why are we checking destroy_bdm again? We call terminate_connection and thus hit issues before bdm.destroy regardless of this being true or false.","commit_id":"08ff7b101494e311f88fffd7aeea26dbf6f8568c"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"7daa6bc0f11e7db679f0019277bc64f0b1e82009","unresolved":false,"context_lines":[{"line_number":4788,"context_line":"        connection_info \u003d self._driver_detach_volume(context, instance, bdm)"},{"line_number":4789,"context_line":"        connector \u003d self.driver.get_volume_connector(instance)"},{"line_number":4790,"context_line":""},{"line_number":4791,"context_line":"        if connection_info and not destroy_bdm and ("},{"line_number":4792,"context_line":"           connector.get(\u0027host\u0027) !\u003d instance.host):"},{"line_number":4793,"context_line":"            # If the volume is attached to another host (evacuate) then"},{"line_number":4794,"context_line":"            # this connector is for the wrong host. Use the connector that"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a57f1b5_1005d712","line":4791,"in_reply_to":"3a57f1b5_1dde6e3f","updated":"2016-02-12 03:23:56.000000000","message":"This helps limit this evacuate bug fix to the evacuate use case. It came from reviewer feedback:\nhttps://review.openstack.org/#/c/266095/5/nova/compute/manager.py@4796","commit_id":"08ff7b101494e311f88fffd7aeea26dbf6f8568c"},{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"7773e6e3a2d30af2aaa5e1a4325a4eeff5760171","unresolved":false,"context_lines":[{"line_number":4802,"context_line":"                                \"%(ihost)s, but connector host is %(chost)s.\"),"},{"line_number":4803,"context_line":"                            {\u0027ihost\u0027: instance.host,"},{"line_number":4804,"context_line":"                             \u0027chost\u0027: connector.get(\u0027host\u0027)})"},{"line_number":4805,"context_line":"            elif stashed_connector.get(\u0027host\u0027) !\u003d instance.host:"},{"line_number":4806,"context_line":"                LOG.warning(_LW(\"Host mismatch detected in stashed \""},{"line_number":4807,"context_line":"                                \"connector. Will use local connector. \""},{"line_number":4808,"context_line":"                                \"Instance host is %(ihost)s. \""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a57f1b5_3de1eafe","line":4805,"updated":"2016-02-11 15:36:54.000000000","message":"Apologies if I missed this before but when can this happen? This would mean that both the stashed and local connectors are referencing a different host that still leaves us calling terminate_connection with an invalid connector below.","commit_id":"08ff7b101494e311f88fffd7aeea26dbf6f8568c"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"7daa6bc0f11e7db679f0019277bc64f0b1e82009","unresolved":false,"context_lines":[{"line_number":4802,"context_line":"                                \"%(ihost)s, but connector host is %(chost)s.\"),"},{"line_number":4803,"context_line":"                            {\u0027ihost\u0027: instance.host,"},{"line_number":4804,"context_line":"                             \u0027chost\u0027: connector.get(\u0027host\u0027)})"},{"line_number":4805,"context_line":"            elif stashed_connector.get(\u0027host\u0027) !\u003d instance.host:"},{"line_number":4806,"context_line":"                LOG.warning(_LW(\"Host mismatch detected in stashed \""},{"line_number":4807,"context_line":"                                \"connector. Will use local connector. \""},{"line_number":4808,"context_line":"                                \"Instance host is %(ihost)s. \""}],"source_content_type":"text/x-python","patch_set":10,"id":"3a57f1b5_034a7355","line":4805,"in_reply_to":"3a57f1b5_3de1eafe","updated":"2016-02-12 03:23:56.000000000","message":"It doesn\u0027t happen. It would be a bug if it did. But when I added the extra logging I decided that it wasn\u0027t right to bury this logical possibility in either of the other 2 messages. This is clearer at the cost of an elif logwarn.","commit_id":"08ff7b101494e311f88fffd7aeea26dbf6f8568c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"aa9b7143228dd8c2f497dc3a01aa55510eff3696","unresolved":false,"context_lines":[{"line_number":4795,"context_line":"            # was stored in connection_info instead (if we have one, and it"},{"line_number":4796,"context_line":"            # is for the expected host)."},{"line_number":4797,"context_line":"            stashed_connector \u003d connection_info.get(\u0027connector\u0027)"},{"line_number":4798,"context_line":"            if not stashed_connector:"},{"line_number":4799,"context_line":"                # Volume was attached before we began stashing connectors"},{"line_number":4800,"context_line":"                LOG.warning(_LW(\"Host mismatch detected, but stashed \""},{"line_number":4801,"context_line":"                                \"connector not found. Instance host is \""}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_f693cff8","line":4798,"range":{"start_line":4798,"start_character":12,"end_line":4798,"end_character":37},"updated":"2016-03-01 16:36:17.000000000","message":"So this is the we don\u0027t know case and we use the local connector...","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"aa9b7143228dd8c2f497dc3a01aa55510eff3696","unresolved":false,"context_lines":[{"line_number":4798,"context_line":"            if not stashed_connector:"},{"line_number":4799,"context_line":"                # Volume was attached before we began stashing connectors"},{"line_number":4800,"context_line":"                LOG.warning(_LW(\"Host mismatch detected, but stashed \""},{"line_number":4801,"context_line":"                                \"connector not found. Instance host is \""},{"line_number":4802,"context_line":"                                \"%(ihost)s, but connector host is %(chost)s.\"),"},{"line_number":4803,"context_line":"                            {\u0027ihost\u0027: instance.host,"},{"line_number":4804,"context_line":"                             \u0027chost\u0027: connector.get(\u0027host\u0027)})"}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_f6774f00","line":4801,"range":{"start_line":4801,"start_character":33,"end_line":4801,"end_character":42},"updated":"2016-03-01 16:36:17.000000000","message":"nit: for context when reading these warnings, I\u0027d call this \u0027volume connector\u0027.","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"5c6197f11516b8a91e6f03f47f1ce716dd27329b","unresolved":false,"context_lines":[{"line_number":4798,"context_line":"            if not stashed_connector:"},{"line_number":4799,"context_line":"                # Volume was attached before we began stashing connectors"},{"line_number":4800,"context_line":"                LOG.warning(_LW(\"Host mismatch detected, but stashed \""},{"line_number":4801,"context_line":"                                \"connector not found. Instance host is \""},{"line_number":4802,"context_line":"                                \"%(ihost)s, but connector host is %(chost)s.\"),"},{"line_number":4803,"context_line":"                            {\u0027ihost\u0027: instance.host,"},{"line_number":4804,"context_line":"                             \u0027chost\u0027: connector.get(\u0027host\u0027)})"}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_c2b07b8e","line":4801,"range":{"start_line":4801,"start_character":33,"end_line":4801,"end_character":42},"in_reply_to":"7af24918_f6774f00","updated":"2016-03-01 21:58:56.000000000","message":"Done","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"9db9fb2b5dda780f1550d8aa527c91c35b9c7463","unresolved":false,"context_lines":[{"line_number":4798,"context_line":"            if not stashed_connector:"},{"line_number":4799,"context_line":"                # Volume was attached before we began stashing connectors"},{"line_number":4800,"context_line":"                LOG.warning(_LW(\"Host mismatch detected, but stashed \""},{"line_number":4801,"context_line":"                                \"connector not found. Instance host is \""},{"line_number":4802,"context_line":"                                \"%(ihost)s, but connector host is %(chost)s.\"),"},{"line_number":4803,"context_line":"                            {\u0027ihost\u0027: instance.host,"},{"line_number":4804,"context_line":"                             \u0027chost\u0027: connector.get(\u0027host\u0027)})"}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_8974eccf","line":4801,"range":{"start_line":4801,"start_character":33,"end_line":4801,"end_character":42},"in_reply_to":"7af24918_f6774f00","updated":"2016-03-01 16:54:24.000000000","message":"Yes, please!","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"aa9b7143228dd8c2f497dc3a01aa55510eff3696","unresolved":false,"context_lines":[{"line_number":4802,"context_line":"                                \"%(ihost)s, but connector host is %(chost)s.\"),"},{"line_number":4803,"context_line":"                            {\u0027ihost\u0027: instance.host,"},{"line_number":4804,"context_line":"                             \u0027chost\u0027: connector.get(\u0027host\u0027)})"},{"line_number":4805,"context_line":"            elif stashed_connector.get(\u0027host\u0027) !\u003d instance.host:"},{"line_number":4806,"context_line":"                LOG.warning(_LW(\"Host mismatch detected in stashed \""},{"line_number":4807,"context_line":"                                \"connector. Will use local connector. \""},{"line_number":4808,"context_line":"                                \"Instance host is %(ihost)s. \""}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_56bba372","line":4805,"range":{"start_line":4805,"start_character":12,"end_line":4805,"end_character":64},"updated":"2016-03-01 16:36:17.000000000","message":"Is this the evacuate case?","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"5c6197f11516b8a91e6f03f47f1ce716dd27329b","unresolved":false,"context_lines":[{"line_number":4802,"context_line":"                                \"%(ihost)s, but connector host is %(chost)s.\"),"},{"line_number":4803,"context_line":"                            {\u0027ihost\u0027: instance.host,"},{"line_number":4804,"context_line":"                             \u0027chost\u0027: connector.get(\u0027host\u0027)})"},{"line_number":4805,"context_line":"            elif stashed_connector.get(\u0027host\u0027) !\u003d instance.host:"},{"line_number":4806,"context_line":"                LOG.warning(_LW(\"Host mismatch detected in stashed \""},{"line_number":4807,"context_line":"                                \"connector. Will use local connector. \""},{"line_number":4808,"context_line":"                                \"Instance host is %(ihost)s. \""}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_62408757","line":4805,"range":{"start_line":4805,"start_character":12,"end_line":4805,"end_character":64},"in_reply_to":"7af24918_56bba372","updated":"2016-03-01 21:58:56.000000000","message":"evacuate case covers parent block also. This is the unexpected case where stashed is still not right. Should never happens, but logging if it does.","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"aa9b7143228dd8c2f497dc3a01aa55510eff3696","unresolved":false,"context_lines":[{"line_number":4804,"context_line":"                             \u0027chost\u0027: connector.get(\u0027host\u0027)})"},{"line_number":4805,"context_line":"            elif stashed_connector.get(\u0027host\u0027) !\u003d instance.host:"},{"line_number":4806,"context_line":"                LOG.warning(_LW(\"Host mismatch detected in stashed \""},{"line_number":4807,"context_line":"                                \"connector. Will use local connector. \""},{"line_number":4808,"context_line":"                                \"Instance host is %(ihost)s. \""},{"line_number":4809,"context_line":"                                \"Local connector host is %(chost)s. \""},{"line_number":4810,"context_line":"                                \"Stashed connector host is %(schost)s.\"),"}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_2905d8c1","line":4807,"range":{"start_line":4807,"start_character":44,"end_line":4807,"end_character":68},"updated":"2016-03-01 16:36:17.000000000","message":"But the local connector host doesn\u0027t match the instance host either, from line 4792, so how do we know that\u0027s better than the stashed_connector?","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ee51be1e0f6cc18222c6502bcece2015e9a8e827","unresolved":false,"context_lines":[{"line_number":4804,"context_line":"                             \u0027chost\u0027: connector.get(\u0027host\u0027)})"},{"line_number":4805,"context_line":"            elif stashed_connector.get(\u0027host\u0027) !\u003d instance.host:"},{"line_number":4806,"context_line":"                LOG.warning(_LW(\"Host mismatch detected in stashed \""},{"line_number":4807,"context_line":"                                \"connector. Will use local connector. \""},{"line_number":4808,"context_line":"                                \"Instance host is %(ihost)s. \""},{"line_number":4809,"context_line":"                                \"Local connector host is %(chost)s. \""},{"line_number":4810,"context_line":"                                \"Stashed connector host is %(schost)s.\"),"}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_e976503d","line":4807,"range":{"start_line":4807,"start_character":44,"end_line":4807,"end_character":68},"in_reply_to":"7af24918_2905d8c1","updated":"2016-03-01 16:39:57.000000000","message":"I guess this came up in an earlier review too:\n\nhttps://review.openstack.org/#/c/266095/10/nova/compute/manager.py@4805\n\nSo if we\u0027re going to leave this in here, I think we need some comments around the conditionals in here, or maybe this should log an error if it\u0027s not expected to happen?","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"5c6197f11516b8a91e6f03f47f1ce716dd27329b","unresolved":false,"context_lines":[{"line_number":4804,"context_line":"                             \u0027chost\u0027: connector.get(\u0027host\u0027)})"},{"line_number":4805,"context_line":"            elif stashed_connector.get(\u0027host\u0027) !\u003d instance.host:"},{"line_number":4806,"context_line":"                LOG.warning(_LW(\"Host mismatch detected in stashed \""},{"line_number":4807,"context_line":"                                \"connector. Will use local connector. \""},{"line_number":4808,"context_line":"                                \"Instance host is %(ihost)s. \""},{"line_number":4809,"context_line":"                                \"Local connector host is %(chost)s. \""},{"line_number":4810,"context_line":"                                \"Stashed connector host is %(schost)s.\"),"}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_7dcf8cb0","line":4807,"range":{"start_line":4807,"start_character":44,"end_line":4807,"end_character":68},"in_reply_to":"7af24918_e976503d","updated":"2016-03-01 21:58:56.000000000","message":"Done. Added comment and made it _LE. I think it would take a DB out-of-sync to cause this, but if someone hits it at least it\u0027ll be logged.","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"aa9b7143228dd8c2f497dc3a01aa55510eff3696","unresolved":false,"context_lines":[{"line_number":4811,"context_line":"                            {\u0027ihost\u0027: instance.host,"},{"line_number":4812,"context_line":"                             \u0027chost\u0027: connector.get(\u0027host\u0027),"},{"line_number":4813,"context_line":"                             \u0027schost\u0027: stashed_connector.get(\u0027host\u0027)})"},{"line_number":4814,"context_line":"            else:"},{"line_number":4815,"context_line":"                LOG.debug(\"Host mismatch detected. Found usable stashed \""},{"line_number":4816,"context_line":"                          \"connector. Instance host is %(ihost)s. \""},{"line_number":4817,"context_line":"                          \"Local connector host was %(chost)s.\","}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_5649e38a","line":4814,"range":{"start_line":4814,"start_character":12,"end_line":4814,"end_character":17},"updated":"2016-03-01 16:36:17.000000000","message":"What is this case? I guess I\u0027m confused with this case and the one before it where the stashed_connector host is not the same as the instance.host.","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"5c6197f11516b8a91e6f03f47f1ce716dd27329b","unresolved":false,"context_lines":[{"line_number":4811,"context_line":"                            {\u0027ihost\u0027: instance.host,"},{"line_number":4812,"context_line":"                             \u0027chost\u0027: connector.get(\u0027host\u0027),"},{"line_number":4813,"context_line":"                             \u0027schost\u0027: stashed_connector.get(\u0027host\u0027)})"},{"line_number":4814,"context_line":"            else:"},{"line_number":4815,"context_line":"                LOG.debug(\"Host mismatch detected. Found usable stashed \""},{"line_number":4816,"context_line":"                          \"connector. Instance host is %(ihost)s. \""},{"line_number":4817,"context_line":"                          \"Local connector host was %(chost)s.\","}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_5d79f0fb","line":4814,"range":{"start_line":4814,"start_character":12,"end_line":4814,"end_character":17},"in_reply_to":"7af24918_5649e38a","updated":"2016-03-01 21:58:56.000000000","message":"Added a little comment. Didn\u0027t want to repeat the log message too much, but I think the 1-liner will help.","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"aa9b7143228dd8c2f497dc3a01aa55510eff3696","unresolved":false,"context_lines":[{"line_number":4812,"context_line":"                             \u0027chost\u0027: connector.get(\u0027host\u0027),"},{"line_number":4813,"context_line":"                             \u0027schost\u0027: stashed_connector.get(\u0027host\u0027)})"},{"line_number":4814,"context_line":"            else:"},{"line_number":4815,"context_line":"                LOG.debug(\"Host mismatch detected. Found usable stashed \""},{"line_number":4816,"context_line":"                          \"connector. Instance host is %(ihost)s. \""},{"line_number":4817,"context_line":"                          \"Local connector host was %(chost)s.\","},{"line_number":4818,"context_line":"                          {\u0027ihost\u0027: instance.host,"},{"line_number":4819,"context_line":"                           \u0027chost\u0027: connector.get(\u0027host\u0027)})"},{"line_number":4820,"context_line":"                connector \u003d stashed_connector"}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_16363b03","line":4817,"range":{"start_line":4815,"start_character":26,"end_line":4817,"end_character":63},"updated":"2016-03-01 16:36:17.000000000","message":"Might be helpful to also log the stashed_connector host in here.","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"5c6197f11516b8a91e6f03f47f1ce716dd27329b","unresolved":false,"context_lines":[{"line_number":4812,"context_line":"                             \u0027chost\u0027: connector.get(\u0027host\u0027),"},{"line_number":4813,"context_line":"                             \u0027schost\u0027: stashed_connector.get(\u0027host\u0027)})"},{"line_number":4814,"context_line":"            else:"},{"line_number":4815,"context_line":"                LOG.debug(\"Host mismatch detected. Found usable stashed \""},{"line_number":4816,"context_line":"                          \"connector. Instance host is %(ihost)s. \""},{"line_number":4817,"context_line":"                          \"Local connector host was %(chost)s.\","},{"line_number":4818,"context_line":"                          {\u0027ihost\u0027: instance.host,"},{"line_number":4819,"context_line":"                           \u0027chost\u0027: connector.get(\u0027host\u0027)})"},{"line_number":4820,"context_line":"                connector \u003d stashed_connector"}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_9d8378cc","line":4817,"range":{"start_line":4815,"start_character":26,"end_line":4817,"end_character":63},"in_reply_to":"7af24918_16363b03","updated":"2016-03-01 21:58:56.000000000","message":"Done","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"}],"nova/tests/unit/compute/test_compute_mgr.py":[{"author":{"_account_id":10135,"name":"Lee Yarwood","display_name":"Lee Yarwood","email":"lyarwood@redhat.com","username":"lyarwood"},"change_message_id":"b33d24a765c3429c06805ec0cc957bd6628506ad","unresolved":false,"context_lines":[{"line_number":2324,"context_line":"                )"},{"line_number":2325,"context_line":"                bdm.destroy.assert_called_once_with()"},{"line_number":2326,"context_line":""},{"line_number":2327,"context_line":"    def test__driver_detach_volume_return(self):"},{"line_number":2328,"context_line":"        \"\"\"_driver_detach_volume returns the connection_info from loads().\"\"\""},{"line_number":2329,"context_line":"        with mock.patch.object(jsonutils, \u0027loads\u0027) as loads:"},{"line_number":2330,"context_line":"            conn_info_str \u003d \u0027test-expected-loads-param\u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a5de9d1_3824dc0b","line":2327,"updated":"2016-01-29 14:35:43.000000000","message":"nit- double underscore","commit_id":"e9520ad751065e5b33b7c6b8efb1bca2c1405d31"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"7b23cb57e5f1f16216c269d7cf05b4f9cc4dff4b","unresolved":false,"context_lines":[{"line_number":2324,"context_line":"                )"},{"line_number":2325,"context_line":"                bdm.destroy.assert_called_once_with()"},{"line_number":2326,"context_line":""},{"line_number":2327,"context_line":"    def test__driver_detach_volume_return(self):"},{"line_number":2328,"context_line":"        \"\"\"_driver_detach_volume returns the connection_info from loads().\"\"\""},{"line_number":2329,"context_line":"        with mock.patch.object(jsonutils, \u0027loads\u0027) as loads:"},{"line_number":2330,"context_line":"            conn_info_str \u003d \u0027test-expected-loads-param\u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a5de9d1_9070e3dd","line":2327,"in_reply_to":"7a5de9d1_3824dc0b","updated":"2016-01-29 22:34:08.000000000","message":"Was intentional. \"test_\"+methodname (method name starts with _).","commit_id":"e9520ad751065e5b33b7c6b8efb1bca2c1405d31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bbf68116a920913842d4b3eb2a1d11c4bbb80bf2","unresolved":false,"context_lines":[{"line_number":2197,"context_line":"        bdm.device_name \u003d \u0027vdb\u0027"},{"line_number":2198,"context_line":"        bdm_get.return_value \u003d bdm"},{"line_number":2199,"context_line":""},{"line_number":2200,"context_line":"        detach.return_value \u003d {}"},{"line_number":2201,"context_line":""},{"line_number":2202,"context_line":"        with mock.patch.object(self.compute, \u0027volume_api\u0027) as volume_api:"},{"line_number":2203,"context_line":"            with mock.patch.object(self.compute, \u0027driver\u0027) as driver:"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a5de9d1_2ee060cc","line":2200,"updated":"2016-02-01 23:59:37.000000000","message":"nit: we could even leave this out since the code is handling the case that connection_info is returned as None (not likely, but you do have a conditional to handle it). I also bring it up in case you plan on backporting this to stable/liberty, it might make the backport cleaner.","commit_id":"2094bc3cd3fca8df200a97c22a82d08f1963d030"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"0440d2d076efaa7c70f992f6ca511fa085bcb7ef","unresolved":false,"context_lines":[{"line_number":2197,"context_line":"        bdm.device_name \u003d \u0027vdb\u0027"},{"line_number":2198,"context_line":"        bdm_get.return_value \u003d bdm"},{"line_number":2199,"context_line":""},{"line_number":2200,"context_line":"        detach.return_value \u003d {}"},{"line_number":2201,"context_line":""},{"line_number":2202,"context_line":"        with mock.patch.object(self.compute, \u0027volume_api\u0027) as volume_api:"},{"line_number":2203,"context_line":"            with mock.patch.object(self.compute, \u0027driver\u0027) as driver:"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a5de9d1_f6a69e0d","line":2200,"in_reply_to":"7a5de9d1_2ee060cc","updated":"2016-02-02 17:59:45.000000000","message":"This can be None or {}. I thought None looked weird. Leaving this out doesn\u0027t work because the sentinel fails get(\u0027host\u0027). I could replace the sentinel with a fake connector, but instead I fixed this with the one line and added new tests for the new path.","commit_id":"2094bc3cd3fca8df200a97c22a82d08f1963d030"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bbf68116a920913842d4b3eb2a1d11c4bbb80bf2","unresolved":false,"context_lines":[{"line_number":2319,"context_line":"            bdm \u003d mock.Mock()"},{"line_number":2320,"context_line":"            bdm.connection_info \u003d conn_info_str"},{"line_number":2321,"context_line":"            loads.return_value \u003d {\u0027test-loads-key\u0027: \u0027test loads return value\u0027}"},{"line_number":2322,"context_line":"            any_mock \u003d mock.Mock()"},{"line_number":2323,"context_line":""},{"line_number":2324,"context_line":"            ret \u003d self.compute._driver_detach_volume(any_mock, any_mock, bdm)"},{"line_number":2325,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7a5de9d1_ce3d0c2c","line":2322,"range":{"start_line":2322,"start_character":12,"end_line":2322,"end_character":34},"updated":"2016-02-01 23:59:37.000000000","message":"Why not just use mock.ANY?","commit_id":"2094bc3cd3fca8df200a97c22a82d08f1963d030"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"0440d2d076efaa7c70f992f6ca511fa085bcb7ef","unresolved":false,"context_lines":[{"line_number":2319,"context_line":"            bdm \u003d mock.Mock()"},{"line_number":2320,"context_line":"            bdm.connection_info \u003d conn_info_str"},{"line_number":2321,"context_line":"            loads.return_value \u003d {\u0027test-loads-key\u0027: \u0027test loads return value\u0027}"},{"line_number":2322,"context_line":"            any_mock \u003d mock.Mock()"},{"line_number":2323,"context_line":""},{"line_number":2324,"context_line":"            ret \u003d self.compute._driver_detach_volume(any_mock, any_mock, bdm)"},{"line_number":2325,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"7a5de9d1_d60da2e8","line":2322,"range":{"start_line":2322,"start_character":12,"end_line":2322,"end_character":34},"in_reply_to":"7a5de9d1_ce3d0c2c","updated":"2016-02-02 17:59:45.000000000","message":"mock.ANY is a matcher. I needed fake methods. I can make this read better using self.context and a fake instance. Next patch.","commit_id":"2094bc3cd3fca8df200a97c22a82d08f1963d030"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"1ea90d1af39a3f97a28508adea16093325a1b883","unresolved":false,"context_lines":[{"line_number":2319,"context_line":"            bdm \u003d mock.Mock()"},{"line_number":2320,"context_line":"            bdm.connection_info \u003d conn_info_str"},{"line_number":2321,"context_line":"            loads.return_value \u003d {\u0027test-loads-key\u0027: \u0027test loads return value\u0027}"},{"line_number":2322,"context_line":"            any_mock \u003d mock.Mock()"},{"line_number":2323,"context_line":""},{"line_number":2324,"context_line":"            ret \u003d self.compute._driver_detach_volume(any_mock, any_mock, bdm)"},{"line_number":2325,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"5a5ae5dd_9abb8f72","line":2322,"range":{"start_line":2322,"start_character":12,"end_line":2322,"end_character":34},"in_reply_to":"7a5de9d1_d60da2e8","updated":"2016-02-03 18:35:17.000000000","message":"Done","commit_id":"2094bc3cd3fca8df200a97c22a82d08f1963d030"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bbf68116a920913842d4b3eb2a1d11c4bbb80bf2","unresolved":false,"context_lines":[{"line_number":2321,"context_line":"            loads.return_value \u003d {\u0027test-loads-key\u0027: \u0027test loads return value\u0027}"},{"line_number":2322,"context_line":"            any_mock \u003d mock.Mock()"},{"line_number":2323,"context_line":""},{"line_number":2324,"context_line":"            ret \u003d self.compute._driver_detach_volume(any_mock, any_mock, bdm)"},{"line_number":2325,"context_line":""},{"line_number":2326,"context_line":"            self.assertTrue(loads.return_value, ret)"},{"line_number":2327,"context_line":"            self.assertFalse(loads.assert_called_once_with(conn_info_str))"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a5de9d1_4e503c32","line":2324,"updated":"2016-02-01 23:59:37.000000000","message":"I\u0027m surprised this method isn\u0027t raising an exception just given the lack of mocking going on for the other parts of the method, maybe some of that is stubbed out in the test class setup already. Just seems pretty bare bones. But I\u0027m also too lazy right now to dig in further.","commit_id":"2094bc3cd3fca8df200a97c22a82d08f1963d030"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"0440d2d076efaa7c70f992f6ca511fa085bcb7ef","unresolved":false,"context_lines":[{"line_number":2321,"context_line":"            loads.return_value \u003d {\u0027test-loads-key\u0027: \u0027test loads return value\u0027}"},{"line_number":2322,"context_line":"            any_mock \u003d mock.Mock()"},{"line_number":2323,"context_line":""},{"line_number":2324,"context_line":"            ret \u003d self.compute._driver_detach_volume(any_mock, any_mock, bdm)"},{"line_number":2325,"context_line":""},{"line_number":2326,"context_line":"            self.assertTrue(loads.return_value, ret)"},{"line_number":2327,"context_line":"            self.assertFalse(loads.assert_called_once_with(conn_info_str))"}],"source_content_type":"text/x-python","patch_set":8,"id":"7a5de9d1_564d32c2","line":2324,"in_reply_to":"7a5de9d1_4e503c32","updated":"2016-02-02 17:59:45.000000000","message":"No data/metadata to encrypt, logging for unknown instance, calling SmallFakeDriver.  I only added this test because I added a return. So less is good.","commit_id":"2094bc3cd3fca8df200a97c22a82d08f1963d030"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"1ea90d1af39a3f97a28508adea16093325a1b883","unresolved":false,"context_lines":[{"line_number":2321,"context_line":"            loads.return_value \u003d {\u0027test-loads-key\u0027: \u0027test loads return value\u0027}"},{"line_number":2322,"context_line":"            any_mock \u003d mock.Mock()"},{"line_number":2323,"context_line":""},{"line_number":2324,"context_line":"            ret \u003d self.compute._driver_detach_volume(any_mock, any_mock, bdm)"},{"line_number":2325,"context_line":""},{"line_number":2326,"context_line":"            self.assertTrue(loads.return_value, ret)"},{"line_number":2327,"context_line":"            self.assertFalse(loads.assert_called_once_with(conn_info_str))"}],"source_content_type":"text/x-python","patch_set":8,"id":"5a5ae5dd_1aa8ff1e","line":2324,"in_reply_to":"7a5de9d1_564d32c2","updated":"2016-02-03 18:35:17.000000000","message":"Done","commit_id":"2094bc3cd3fca8df200a97c22a82d08f1963d030"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"bbf68116a920913842d4b3eb2a1d11c4bbb80bf2","unresolved":false,"context_lines":[{"line_number":2324,"context_line":"            ret \u003d self.compute._driver_detach_volume(any_mock, any_mock, bdm)"},{"line_number":2325,"context_line":""},{"line_number":2326,"context_line":"            self.assertTrue(loads.return_value, ret)"},{"line_number":2327,"context_line":"            self.assertFalse(loads.assert_called_once_with(conn_info_str))"},{"line_number":2328,"context_line":""},{"line_number":2329,"context_line":"    def _test_rescue(self, clean_shutdown\u003dTrue):"},{"line_number":2330,"context_line":"        instance \u003d fake_instance.fake_instance_obj("}],"source_content_type":"text/x-python","patch_set":8,"id":"7a5de9d1_eee6c8ae","line":2327,"updated":"2016-02-01 23:59:37.000000000","message":"This looks odd, asserting false on an assertion call.","commit_id":"2094bc3cd3fca8df200a97c22a82d08f1963d030"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"0440d2d076efaa7c70f992f6ca511fa085bcb7ef","unresolved":false,"context_lines":[{"line_number":2324,"context_line":"            ret \u003d self.compute._driver_detach_volume(any_mock, any_mock, bdm)"},{"line_number":2325,"context_line":""},{"line_number":2326,"context_line":"            self.assertTrue(loads.return_value, ret)"},{"line_number":2327,"context_line":"            self.assertFalse(loads.assert_called_once_with(conn_info_str))"},{"line_number":2328,"context_line":""},{"line_number":2329,"context_line":"    def _test_rescue(self, clean_shutdown\u003dTrue):"},{"line_number":2330,"context_line":"        instance \u003d fake_instance.fake_instance_obj("}],"source_content_type":"text/x-python","patch_set":8,"id":"7a5de9d1_f6e9be7f","line":2327,"in_reply_to":"7a5de9d1_eee6c8ae","updated":"2016-02-02 17:59:45.000000000","message":"Really odd that it actually works. Will fix.","commit_id":"2094bc3cd3fca8df200a97c22a82d08f1963d030"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"1ea90d1af39a3f97a28508adea16093325a1b883","unresolved":false,"context_lines":[{"line_number":2324,"context_line":"            ret \u003d self.compute._driver_detach_volume(any_mock, any_mock, bdm)"},{"line_number":2325,"context_line":""},{"line_number":2326,"context_line":"            self.assertTrue(loads.return_value, ret)"},{"line_number":2327,"context_line":"            self.assertFalse(loads.assert_called_once_with(conn_info_str))"},{"line_number":2328,"context_line":""},{"line_number":2329,"context_line":"    def _test_rescue(self, clean_shutdown\u003dTrue):"},{"line_number":2330,"context_line":"        instance \u003d fake_instance.fake_instance_obj("}],"source_content_type":"text/x-python","patch_set":8,"id":"5a5ae5dd_ba63eb19","line":2327,"in_reply_to":"7a5de9d1_f6e9be7f","updated":"2016-02-03 18:35:17.000000000","message":"Done","commit_id":"2094bc3cd3fca8df200a97c22a82d08f1963d030"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"aa9b7143228dd8c2f497dc3a01aa55510eff3696","unresolved":false,"context_lines":[{"line_number":2247,"context_line":"    def test_detach_volume_evacuate_mismatch(self):"},{"line_number":2248,"context_line":"        \"\"\"Test coverage for evacuate with connector mismatch."},{"line_number":2249,"context_line":""},{"line_number":2250,"context_line":"        For evacuate, if the stashed connector also has the wrong host,"},{"line_number":2251,"context_line":"        then log it and stay with the local connector."},{"line_number":2252,"context_line":"        \"\"\""},{"line_number":2253,"context_line":"        conn_info_str \u003d \u0027{\"connector\": {\"host\": \"other-host\"}}\u0027"},{"line_number":2254,"context_line":"        self._test_detach_volume_evacuate(conn_info_str)"}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_c9bb940d","line":2251,"range":{"start_line":2250,"start_character":8,"end_line":2251,"end_character":54},"updated":"2016-03-01 16:36:17.000000000","message":"This is where I\u0027m confused in the code, who do you know it\u0027s the wrong host? I thought the assumption was that if the (local) connector host didn\u0027t match the instance.host, it was wrong and we get it from the stashed connector. I guess in this case the \u0027wrong\u0027 host is just because the local connector and/or stashed connector hosts don\u0027t match the instance.host. But for evacuate, don\u0027t we expect the instance.host to have changed so they won\u0027t match by design?","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"5c6197f11516b8a91e6f03f47f1ce716dd27329b","unresolved":false,"context_lines":[{"line_number":2247,"context_line":"    def test_detach_volume_evacuate_mismatch(self):"},{"line_number":2248,"context_line":"        \"\"\"Test coverage for evacuate with connector mismatch."},{"line_number":2249,"context_line":""},{"line_number":2250,"context_line":"        For evacuate, if the stashed connector also has the wrong host,"},{"line_number":2251,"context_line":"        then log it and stay with the local connector."},{"line_number":2252,"context_line":"        \"\"\""},{"line_number":2253,"context_line":"        conn_info_str \u003d \u0027{\"connector\": {\"host\": \"other-host\"}}\u0027"},{"line_number":2254,"context_line":"        self._test_detach_volume_evacuate(conn_info_str)"}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_dd77c0d2","line":2251,"range":{"start_line":2250,"start_character":8,"end_line":2251,"end_character":54},"in_reply_to":"7af24918_c9bb940d","updated":"2016-03-01 21:58:56.000000000","message":"for evac instance.host doesn\u0027t match local connector host. So we look for stashed. Upon re-checking the stashed one if we are still out-of-sync -- I think it would take a DB out-of-sync of other fail to get in this state. The code is there to at least log this. Debatable whether to log it or assume it is impossible.","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"06ee3a4cc162101384efb08ec1ffff35b11079b1","unresolved":false,"context_lines":[{"line_number":2313,"context_line":"                                                     instance,"},{"line_number":2314,"context_line":"                                                     bdm)"},{"line_number":2315,"context_line":""},{"line_number":2316,"context_line":"            self.assertTrue(loads.return_value, ret)"},{"line_number":2317,"context_line":"            loads.assert_called_once_with(conn_info_str)"},{"line_number":2318,"context_line":""},{"line_number":2319,"context_line":"    def _test_rescue(self, clean_shutdown\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":12,"id":"7af24918_4800be01","line":2316,"range":{"start_line":2316,"start_character":17,"end_line":2316,"end_character":27},"updated":"2016-03-02 19:58:22.000000000","message":"Did you mean assertEqual here?","commit_id":"1f2d2d46d576d12563512cd964b4b332b3a6706f"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"bd77d597fa3fcc62922e1de265c23ef7c4fc0505","unresolved":false,"context_lines":[{"line_number":2313,"context_line":"                                                     instance,"},{"line_number":2314,"context_line":"                                                     bdm)"},{"line_number":2315,"context_line":""},{"line_number":2316,"context_line":"            self.assertTrue(loads.return_value, ret)"},{"line_number":2317,"context_line":"            loads.assert_called_once_with(conn_info_str)"},{"line_number":2318,"context_line":""},{"line_number":2319,"context_line":"    def _test_rescue(self, clean_shutdown\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":12,"id":"7af24918_a878622c","line":2316,"range":{"start_line":2316,"start_character":17,"end_line":2316,"end_character":27},"in_reply_to":"7af24918_4800be01","updated":"2016-03-02 20:07:27.000000000","message":"Done","commit_id":"1f2d2d46d576d12563512cd964b4b332b3a6706f"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"9d099f5d3047a5ec00c670c9e479e9712eee23c1","unresolved":false,"context_lines":[{"line_number":2313,"context_line":"                                                     instance,"},{"line_number":2314,"context_line":"                                                     bdm)"},{"line_number":2315,"context_line":""},{"line_number":2316,"context_line":"            self.assertTrue(loads.return_value, ret)"},{"line_number":2317,"context_line":"            loads.assert_called_once_with(conn_info_str)"},{"line_number":2318,"context_line":""},{"line_number":2319,"context_line":"    def _test_rescue(self, clean_shutdown\u003dTrue):"}],"source_content_type":"text/x-python","patch_set":12,"id":"7af24918_a83d02bd","line":2316,"range":{"start_line":2316,"start_character":17,"end_line":2316,"end_character":27},"in_reply_to":"7af24918_4800be01","updated":"2016-03-02 20:05:18.000000000","message":"Yes. Darn!  Fix coming.","commit_id":"1f2d2d46d576d12563512cd964b4b332b3a6706f"}],"nova/tests/unit/volume/test_cinder.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"aa9b7143228dd8c2f497dc3a01aa55510eff3696","unresolved":false,"context_lines":[{"line_number":291,"context_line":""},{"line_number":292,"context_line":"    @mock.patch(\u0027nova.volume.cinder.cinderclient\u0027)"},{"line_number":293,"context_line":"    def test_initialize_connection(self, mock_cinderclient):"},{"line_number":294,"context_line":"        return_dict \u003d {\u0027foo\u0027: \u0027bar\u0027}"},{"line_number":295,"context_line":"        mock_cinderclient.return_value.volumes. \\"},{"line_number":296,"context_line":"            initialize_connection.return_value \u003d return_dict"},{"line_number":297,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_76269f69","line":294,"range":{"start_line":294,"start_character":8,"end_line":294,"end_character":19},"updated":"2016-03-01 16:36:17.000000000","message":"nit: I\u0027d name this connection_info so it\u0027s a tad bit more clear, but it\u0027s not a big deal.","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"5c6197f11516b8a91e6f03f47f1ce716dd27329b","unresolved":false,"context_lines":[{"line_number":291,"context_line":""},{"line_number":292,"context_line":"    @mock.patch(\u0027nova.volume.cinder.cinderclient\u0027)"},{"line_number":293,"context_line":"    def test_initialize_connection(self, mock_cinderclient):"},{"line_number":294,"context_line":"        return_dict \u003d {\u0027foo\u0027: \u0027bar\u0027}"},{"line_number":295,"context_line":"        mock_cinderclient.return_value.volumes. \\"},{"line_number":296,"context_line":"            initialize_connection.return_value \u003d return_dict"},{"line_number":297,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_3804f213","line":294,"range":{"start_line":294,"start_character":8,"end_line":294,"end_character":19},"in_reply_to":"7af24918_76269f69","updated":"2016-03-01 21:58:56.000000000","message":"Done","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"}],"nova/volume/cinder.py":[{"author":{"_account_id":1865,"name":"Andrea Rosa","email":"andrea.rosa@gmail.com","username":"andrea-rosa-m"},"change_message_id":"f802dfa4061d3a56d78ebef8ba419825ab70c2f4","unresolved":false,"context_lines":[{"line_number":354,"context_line":"                context).volumes.initialize_connection(volume_id, connector)"},{"line_number":355,"context_line":"            if connection_info:"},{"line_number":356,"context_line":"                if isinstance(connection_info, dict):"},{"line_number":357,"context_line":"                    connection_info[\u0027connector\u0027] \u003d connector"},{"line_number":358,"context_line":"            else:"},{"line_number":359,"context_line":"                connection_info \u003d {\u0027connector\u0027: connector}"},{"line_number":360,"context_line":"            return connection_info"}],"source_content_type":"text/x-python","patch_set":1,"id":"da6ed579_d877d0d2","line":357,"updated":"2016-01-12 16:13:55.000000000","message":"is it safe to overwrite what we get back from the volume API?\nI\u0027d prefer to check that the connector is not present before overwriting it","commit_id":"280d0fa8e82ee4491fe6681c57e2c7214baab559"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"321ed1fbd1fcc30a64b7526402193572bcb31011","unresolved":false,"context_lines":[{"line_number":354,"context_line":"                context).volumes.initialize_connection(volume_id, connector)"},{"line_number":355,"context_line":"            if connection_info:"},{"line_number":356,"context_line":"                if isinstance(connection_info, dict):"},{"line_number":357,"context_line":"                    connection_info[\u0027connector\u0027] \u003d connector"},{"line_number":358,"context_line":"            else:"},{"line_number":359,"context_line":"                connection_info \u003d {\u0027connector\u0027: connector}"},{"line_number":360,"context_line":"            return connection_info"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a68dd71_3551e9d8","line":357,"in_reply_to":"9a68dd71_45c6629b","updated":"2016-01-22 18:52:32.000000000","message":"Done","commit_id":"280d0fa8e82ee4491fe6681c57e2c7214baab559"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"c064071e7f0787be4d412cf884280304c747267e","unresolved":false,"context_lines":[{"line_number":354,"context_line":"                context).volumes.initialize_connection(volume_id, connector)"},{"line_number":355,"context_line":"            if connection_info:"},{"line_number":356,"context_line":"                if isinstance(connection_info, dict):"},{"line_number":357,"context_line":"                    connection_info[\u0027connector\u0027] \u003d connector"},{"line_number":358,"context_line":"            else:"},{"line_number":359,"context_line":"                connection_info \u003d {\u0027connector\u0027: connector}"},{"line_number":360,"context_line":"            return connection_info"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a68dd71_45c6629b","line":357,"in_reply_to":"da6ed579_d877d0d2","updated":"2016-01-21 23:36:27.000000000","message":"Good idea.  Will add check and logging in case someone hits that issue in the future.","commit_id":"280d0fa8e82ee4491fe6681c57e2c7214baab559"},{"author":{"_account_id":1865,"name":"Andrea Rosa","email":"andrea.rosa@gmail.com","username":"andrea-rosa-m"},"change_message_id":"f802dfa4061d3a56d78ebef8ba419825ab70c2f4","unresolved":false,"context_lines":[{"line_number":357,"context_line":"                    connection_info[\u0027connector\u0027] \u003d connector"},{"line_number":358,"context_line":"            else:"},{"line_number":359,"context_line":"                connection_info \u003d {\u0027connector\u0027: connector}"},{"line_number":360,"context_line":"            return connection_info"},{"line_number":361,"context_line":"        except cinder_exception.ClientException as ex:"},{"line_number":362,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":363,"context_line":"                LOG.error(_LE(\u0027Initialize connection failed for volume \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"da6ed579_1309cb2c","line":360,"updated":"2016-01-12 16:13:55.000000000","message":"you change the semantic of the method, in the original code None is one of the potential return value with this change when we should return None we actually return a dictionary.\nThat could be a good thing to do but I expect to put some changes or tests in the code where this method is used.","commit_id":"280d0fa8e82ee4491fe6681c57e2c7214baab559"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"c064071e7f0787be4d412cf884280304c747267e","unresolved":false,"context_lines":[{"line_number":357,"context_line":"                    connection_info[\u0027connector\u0027] \u003d connector"},{"line_number":358,"context_line":"            else:"},{"line_number":359,"context_line":"                connection_info \u003d {\u0027connector\u0027: connector}"},{"line_number":360,"context_line":"            return connection_info"},{"line_number":361,"context_line":"        except cinder_exception.ClientException as ex:"},{"line_number":362,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":363,"context_line":"                LOG.error(_LE(\u0027Initialize connection failed for volume \u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a68dd71_40b17046","line":360,"in_reply_to":"da6ed579_1309cb2c","updated":"2016-01-21 23:36:27.000000000","message":"If it matters, Cinder should never return None here, but anyway, if it did we\u0027ll add our connector to {}. I don\u0027t think there is any code that will care about that. If you meant test cases, I have that covered. I think you meant somewhere else though.","commit_id":"280d0fa8e82ee4491fe6681c57e2c7214baab559"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ca8987c1204676091564983c6a91e5777d165768","unresolved":false,"context_lines":[{"line_number":403,"context_line":"            # Add connector to connection_info, if there is some unexpected"},{"line_number":404,"context_line":"            # conflict doing this, then log a warning and leave it as-is."},{"line_number":405,"context_line":"            if connection_info:"},{"line_number":406,"context_line":"                if isinstance(connection_info, dict):"},{"line_number":407,"context_line":"                    if connection_info.get(\u0027connector\u0027):"},{"line_number":408,"context_line":"                        LOG.warning(_LW(\"Cannot add connector to %(vol)s \""},{"line_number":409,"context_line":"                                        \"connection info because \""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a5de9d1_c91175b1","line":406,"updated":"2016-01-27 10:07:12.000000000","message":"Why would it not be a dict?","commit_id":"ce1f2f245b59885c0ccf295ac231935b930f3b24"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"0ac3ad9b5fad1f68dd521ba6ff099b49828d77c0","unresolved":false,"context_lines":[{"line_number":403,"context_line":"            # Add connector to connection_info, if there is some unexpected"},{"line_number":404,"context_line":"            # conflict doing this, then log a warning and leave it as-is."},{"line_number":405,"context_line":"            if connection_info:"},{"line_number":406,"context_line":"                if isinstance(connection_info, dict):"},{"line_number":407,"context_line":"                    if connection_info.get(\u0027connector\u0027):"},{"line_number":408,"context_line":"                        LOG.warning(_LW(\"Cannot add connector to %(vol)s \""},{"line_number":409,"context_line":"                                        \"connection info because \""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a5de9d1_711da372","line":406,"in_reply_to":"7a5de9d1_11b55dcc","updated":"2016-01-27 22:45:20.000000000","message":"Done","commit_id":"ce1f2f245b59885c0ccf295ac231935b930f3b24"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"6709ebc0721e0c63879f2afc4180631555061f59","unresolved":false,"context_lines":[{"line_number":403,"context_line":"            # Add connector to connection_info, if there is some unexpected"},{"line_number":404,"context_line":"            # conflict doing this, then log a warning and leave it as-is."},{"line_number":405,"context_line":"            if connection_info:"},{"line_number":406,"context_line":"                if isinstance(connection_info, dict):"},{"line_number":407,"context_line":"                    if connection_info.get(\u0027connector\u0027):"},{"line_number":408,"context_line":"                        LOG.warning(_LW(\"Cannot add connector to %(vol)s \""},{"line_number":409,"context_line":"                                        \"connection info because \""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a5de9d1_11b55dcc","line":406,"in_reply_to":"7a5de9d1_c91175b1","updated":"2016-01-27 17:00:56.000000000","message":"Just over-protecting against invalid .get(). Should be OK with just \"if connection_info\", but I may need to fix more tests.","commit_id":"ce1f2f245b59885c0ccf295ac231935b930f3b24"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5b693535d52803efb34412221b5e19efdce881b8","unresolved":false,"context_lines":[{"line_number":404,"context_line":"            # conflict doing this, then log a warning and leave it as-is."},{"line_number":405,"context_line":"            if connection_info:"},{"line_number":406,"context_line":"                if isinstance(connection_info, dict):"},{"line_number":407,"context_line":"                    if connection_info.get(\u0027connector\u0027):"},{"line_number":408,"context_line":"                        LOG.warning(_LW(\"Cannot add connector to %(vol)s \""},{"line_number":409,"context_line":"                                        \"connection info because \""},{"line_number":410,"context_line":"                                        \"initialize_connection returned a \""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a5de9d1_89ca874e","line":407,"updated":"2016-01-27 16:15:29.000000000","message":"Would this actually happen? Cinder isn\u0027t returning this value is it - or planning to? As far as I know only nova is storing the connection_info in the database.","commit_id":"ce1f2f245b59885c0ccf295ac231935b930f3b24"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"0ac3ad9b5fad1f68dd521ba6ff099b49828d77c0","unresolved":false,"context_lines":[{"line_number":404,"context_line":"            # conflict doing this, then log a warning and leave it as-is."},{"line_number":405,"context_line":"            if connection_info:"},{"line_number":406,"context_line":"                if isinstance(connection_info, dict):"},{"line_number":407,"context_line":"                    if connection_info.get(\u0027connector\u0027):"},{"line_number":408,"context_line":"                        LOG.warning(_LW(\"Cannot add connector to %(vol)s \""},{"line_number":409,"context_line":"                                        \"connection info because \""},{"line_number":410,"context_line":"                                        \"initialize_connection returned a \""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a5de9d1_31172b53","line":407,"in_reply_to":"7a5de9d1_11b9dde9","updated":"2016-01-27 22:45:20.000000000","message":"Done","commit_id":"ce1f2f245b59885c0ccf295ac231935b930f3b24"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"6709ebc0721e0c63879f2afc4180631555061f59","unresolved":false,"context_lines":[{"line_number":404,"context_line":"            # conflict doing this, then log a warning and leave it as-is."},{"line_number":405,"context_line":"            if connection_info:"},{"line_number":406,"context_line":"                if isinstance(connection_info, dict):"},{"line_number":407,"context_line":"                    if connection_info.get(\u0027connector\u0027):"},{"line_number":408,"context_line":"                        LOG.warning(_LW(\"Cannot add connector to %(vol)s \""},{"line_number":409,"context_line":"                                        \"connection info because \""},{"line_number":410,"context_line":"                                        \"initialize_connection returned a \""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a5de9d1_11b9dde9","line":407,"in_reply_to":"7a5de9d1_89ca874e","updated":"2016-01-27 17:00:56.000000000","message":"Not expected. This was added based on what-if reviewer feedback. Probably better to remove this and only add it back if there is some real future need.","commit_id":"ce1f2f245b59885c0ccf295ac231935b930f3b24"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"7466c098667bb1fe2f96d77df29758d8d5fe1b86","unresolved":false,"context_lines":[{"line_number":399,"context_line":"        try:"},{"line_number":400,"context_line":"            connection_info \u003d cinderclient("},{"line_number":401,"context_line":"                context).volumes.initialize_connection(volume_id,"},{"line_number":402,"context_line":"                                                       connector) or {}"},{"line_number":403,"context_line":"            connection_info[\u0027connector\u0027] \u003d connector"},{"line_number":404,"context_line":"            return connection_info"},{"line_number":405,"context_line":"        except cinder_exception.ClientException as ex:"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a5de9d1_f5696d8c","line":402,"range":{"start_line":402,"start_character":65,"end_line":402,"end_character":71},"updated":"2016-01-28 10:16:41.000000000","message":"We don\u0027t need this - if we don\u0027t get anything back from cinder it\u0027s a failure.","commit_id":"e9520ad751065e5b33b7c6b8efb1bca2c1405d31"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"7b23cb57e5f1f16216c269d7cf05b4f9cc4dff4b","unresolved":false,"context_lines":[{"line_number":399,"context_line":"        try:"},{"line_number":400,"context_line":"            connection_info \u003d cinderclient("},{"line_number":401,"context_line":"                context).volumes.initialize_connection(volume_id,"},{"line_number":402,"context_line":"                                                       connector) or {}"},{"line_number":403,"context_line":"            connection_info[\u0027connector\u0027] \u003d connector"},{"line_number":404,"context_line":"            return connection_info"},{"line_number":405,"context_line":"        except cinder_exception.ClientException as ex:"}],"source_content_type":"text/x-python","patch_set":6,"id":"7a5de9d1_f07fd7a8","line":402,"range":{"start_line":402,"start_character":65,"end_line":402,"end_character":71},"in_reply_to":"7a5de9d1_f5696d8c","updated":"2016-01-29 22:34:08.000000000","message":"Done","commit_id":"e9520ad751065e5b33b7c6b8efb1bca2c1405d31"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"aa9b7143228dd8c2f497dc3a01aa55510eff3696","unresolved":false,"context_lines":[{"line_number":397,"context_line":"        try:"},{"line_number":398,"context_line":"            connection_info \u003d cinderclient("},{"line_number":399,"context_line":"                context).volumes.initialize_connection(volume_id, connector)"},{"line_number":400,"context_line":"            connection_info[\u0027connector\u0027] \u003d connector"},{"line_number":401,"context_line":"            return connection_info"},{"line_number":402,"context_line":"        except cinder_exception.ClientException as ex:"},{"line_number":403,"context_line":"            with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_96c1ab83","line":400,"range":{"start_line":400,"start_character":12,"end_line":400,"end_character":52},"updated":"2016-03-01 16:36:17.000000000","message":"The only thing I worry about with this is adding more serialized content to the block_device_mappings.connection_info column in the DB, but it\u0027s a MediumText field which is 2^24 characters for MySQL so we should be OK. Honestly, the TEXT type is 2^16 and I\u0027m wondering if anything larger than that would even work with passing content over RPC so if we start blowing up the size of the bdm.connection_info column, we\u0027ll probably have bigger issues.","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"96320a0dc887c3f166d73fec9ee74c16542789e6","unresolved":false,"context_lines":[{"line_number":397,"context_line":"        try:"},{"line_number":398,"context_line":"            connection_info \u003d cinderclient("},{"line_number":399,"context_line":"                context).volumes.initialize_connection(volume_id, connector)"},{"line_number":400,"context_line":"            connection_info[\u0027connector\u0027] \u003d connector"},{"line_number":401,"context_line":"            return connection_info"},{"line_number":402,"context_line":"        except cinder_exception.ClientException as ex:"},{"line_number":403,"context_line":"            with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_9c46e121","line":400,"range":{"start_line":400,"start_character":12,"end_line":400,"end_character":52},"in_reply_to":"7af24918_11faba42","updated":"2016-03-01 19:19:57.000000000","message":"Just thinking out loud, but we could always migrate the connector data later to the cinder volume attachment if/when that API is available, so this would be a hack in nova until that\u0027s available (but it is a thing that hemna is looking to get into cinder).","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"87837f59758f02c6a2e08b0f8a8e795fc2cfce97","unresolved":false,"context_lines":[{"line_number":397,"context_line":"        try:"},{"line_number":398,"context_line":"            connection_info \u003d cinderclient("},{"line_number":399,"context_line":"                context).volumes.initialize_connection(volume_id, connector)"},{"line_number":400,"context_line":"            connection_info[\u0027connector\u0027] \u003d connector"},{"line_number":401,"context_line":"            return connection_info"},{"line_number":402,"context_line":"        except cinder_exception.ClientException as ex:"},{"line_number":403,"context_line":"            with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_5768b2f5","line":400,"range":{"start_line":400,"start_character":12,"end_line":400,"end_character":52},"in_reply_to":"7af24918_11faba42","updated":"2016-03-01 19:49:11.000000000","message":"This may allow Nova evacuate to properly re-send the connector, but this doesn\u0027t solve the force-detach issue.  Also, this only works for new attach calls that happen after this is in place.  It won\u0027t fix the evacuate calls for existing attachments.","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"2640588697eb2153c59d15ecbaee832b99db023a","unresolved":false,"context_lines":[{"line_number":397,"context_line":"        try:"},{"line_number":398,"context_line":"            connection_info \u003d cinderclient("},{"line_number":399,"context_line":"                context).volumes.initialize_connection(volume_id, connector)"},{"line_number":400,"context_line":"            connection_info[\u0027connector\u0027] \u003d connector"},{"line_number":401,"context_line":"            return connection_info"},{"line_number":402,"context_line":"        except cinder_exception.ClientException as ex:"},{"line_number":403,"context_line":"            with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_11faba42","line":400,"range":{"start_line":400,"start_character":12,"end_line":400,"end_character":52},"in_reply_to":"7af24918_6cfece6c","updated":"2016-03-01 19:14:52.000000000","message":"Ideally cinder would store the connector information with the volume attachment in the cinder database, then it wouldn\u0027t need to rely on nova figuring this out and passing it over to cinder (and storing it in nova\u0027s database). I\u0027m hearing from the mailing list (scottda/hemna) that there will be some volume attachment update API in cinder in Newton to help deal with multiattach issues, would that also be useful here?","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"},{"author":{"_account_id":5511,"name":"Nikola Dipanov","email":"ndipanov@redhat.com","username":"ndipanov"},"change_message_id":"9db9fb2b5dda780f1550d8aa527c91c35b9c7463","unresolved":false,"context_lines":[{"line_number":397,"context_line":"        try:"},{"line_number":398,"context_line":"            connection_info \u003d cinderclient("},{"line_number":399,"context_line":"                context).volumes.initialize_connection(volume_id, connector)"},{"line_number":400,"context_line":"            connection_info[\u0027connector\u0027] \u003d connector"},{"line_number":401,"context_line":"            return connection_info"},{"line_number":402,"context_line":"        except cinder_exception.ClientException as ex:"},{"line_number":403,"context_line":"            with excutils.save_and_reraise_exception():"}],"source_content_type":"text/x-python","patch_set":11,"id":"7af24918_6cfece6c","line":400,"range":{"start_line":400,"start_character":12,"end_line":400,"end_character":52},"in_reply_to":"7af24918_96c1ab83","updated":"2016-03-01 16:54:24.000000000","message":"Yeah - I\u0027d like to see this get similar treatment as new_ and old_ flavor and PCI devices etc. meaning it should probably be a DB column on it\u0027s own at the very least.\n\nThe problem with that is that it requires massive paperwork and can\u0027t be merged as a bugfixe etc.\n\nSo we are stuck with this because leaving the bug in would be worse. A classic lose-lose if you ask me.","commit_id":"9a248c954b23ecbe1853e3f291a87630cb42040c"}]}