)]}'
{"nova/virt/block_device.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"180c2232ca82b36b3ed87313162aabb0e248131f","unresolved":false,"context_lines":[{"line_number":240,"context_line":"            connection_info \u003d volume_api.initialize_connection(context,"},{"line_number":241,"context_line":"                                                               volume_id,"},{"line_number":242,"context_line":"                                                               connector)"},{"line_number":243,"context_line":"        except Exception:"},{"line_number":244,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":245,"context_line":"                LOG.debug(\"Initialize connection failed for volume \""},{"line_number":246,"context_line":"                          \"%(volume_id)s at %(mountpoint)s.\","}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_9467dcb6","line":243,"updated":"2015-09-21 21:38:47.000000000","message":"I don\u0027t like the blanket catch on Exception really - what kind of error doe we get in the timeout case mentioned in the bug?  We should maybe be more specific in what we\u0027re trying to handle here?","commit_id":"df07e9c0c07b332f23566faf7a584cc9b68e0331"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"de5ded97406dc759fc4d6d375d84ce5d85173acf","unresolved":false,"context_lines":[{"line_number":240,"context_line":"            connection_info \u003d volume_api.initialize_connection(context,"},{"line_number":241,"context_line":"                                                               volume_id,"},{"line_number":242,"context_line":"                                                               connector)"},{"line_number":243,"context_line":"        except Exception:"},{"line_number":244,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":245,"context_line":"                LOG.debug(\"Initialize connection failed for volume \""},{"line_number":246,"context_line":"                          \"%(volume_id)s at %(mountpoint)s.\","}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_c0726b7c","line":243,"in_reply_to":"ba15a1d1_9467dcb6","updated":"2015-09-22 03:49:40.000000000","message":"Only call terminate_connection for specific error code related to timeout now. Added some comments there too.","commit_id":"df07e9c0c07b332f23566faf7a584cc9b68e0331"},{"author":{"_account_id":10439,"name":"Xi Yang","email":"yang.xi@99cloud.net","username":"phenom"},"change_message_id":"981263c7442893978d628dcd68aff63b03b60dc0","unresolved":false,"context_lines":[{"line_number":242,"context_line":"                                                               connector)"},{"line_number":243,"context_line":"        except Exception:"},{"line_number":244,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":245,"context_line":"                LOG.debug(\"Initialize connection failed for volume \""},{"line_number":246,"context_line":"                          \"%(volume_id)s at %(mountpoint)s.\","},{"line_number":247,"context_line":"                          {\u0027volume_id\u0027: volume_id,"},{"line_number":248,"context_line":"                           \u0027mountpoint\u0027: self[\u0027mount_device\u0027]},"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_d1dcb176","line":245,"updated":"2015-09-21 02:11:21.000000000","message":"Do we need a error message here? I am not sure whether debug is enough.","commit_id":"df07e9c0c07b332f23566faf7a584cc9b68e0331"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"180c2232ca82b36b3ed87313162aabb0e248131f","unresolved":false,"context_lines":[{"line_number":242,"context_line":"                                                               connector)"},{"line_number":243,"context_line":"        except Exception:"},{"line_number":244,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":245,"context_line":"                LOG.debug(\"Initialize connection failed for volume \""},{"line_number":246,"context_line":"                          \"%(volume_id)s at %(mountpoint)s.\","},{"line_number":247,"context_line":"                          {\u0027volume_id\u0027: volume_id,"},{"line_number":248,"context_line":"                           \u0027mountpoint\u0027: self[\u0027mount_device\u0027]},"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_b472207b","line":245,"in_reply_to":"ba15a1d1_118229c9","updated":"2015-09-21 21:38:47.000000000","message":"Most people have been adding a TODO in the code for these kinds of things to remind them to change from debug to something higher after string freeze.  Should probably do the same here.","commit_id":"df07e9c0c07b332f23566faf7a584cc9b68e0331"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"de5ded97406dc759fc4d6d375d84ce5d85173acf","unresolved":false,"context_lines":[{"line_number":242,"context_line":"                                                               connector)"},{"line_number":243,"context_line":"        except Exception:"},{"line_number":244,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":245,"context_line":"                LOG.debug(\"Initialize connection failed for volume \""},{"line_number":246,"context_line":"                          \"%(volume_id)s at %(mountpoint)s.\","},{"line_number":247,"context_line":"                          {\u0027volume_id\u0027: volume_id,"},{"line_number":248,"context_line":"                           \u0027mountpoint\u0027: self[\u0027mount_device\u0027]},"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_4005fbbf","line":245,"in_reply_to":"ba15a1d1_b472207b","updated":"2015-09-22 03:49:40.000000000","message":"Done","commit_id":"df07e9c0c07b332f23566faf7a584cc9b68e0331"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"97100dd7b9d3b95e33efc0bb0eeb2d0b2dba2c10","unresolved":false,"context_lines":[{"line_number":242,"context_line":"                                                               connector)"},{"line_number":243,"context_line":"        except Exception:"},{"line_number":244,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":245,"context_line":"                LOG.debug(\"Initialize connection failed for volume \""},{"line_number":246,"context_line":"                          \"%(volume_id)s at %(mountpoint)s.\","},{"line_number":247,"context_line":"                          {\u0027volume_id\u0027: volume_id,"},{"line_number":248,"context_line":"                           \u0027mountpoint\u0027: self[\u0027mount_device\u0027]},"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_118229c9","line":245,"in_reply_to":"ba15a1d1_d1dcb176","updated":"2015-09-21 03:11:26.000000000","message":"I use \"debug\" because there\u0027s string freeze now.  If Nova cores want to me to change this to \"error\", I\u0027ll change it.","commit_id":"df07e9c0c07b332f23566faf7a584cc9b68e0331"},{"author":{"_account_id":11637,"name":"jay.xu","email":"jay.xu@emc.com","username":"jay.xu"},"change_message_id":"8c6b42e1104f0bc933a7aa02840a3c964750d320","unresolved":false,"context_lines":[{"line_number":247,"context_line":"                          {\u0027volume_id\u0027: volume_id,"},{"line_number":248,"context_line":"                           \u0027mountpoint\u0027: self[\u0027mount_device\u0027]},"},{"line_number":249,"context_line":"                          context\u003dcontext, instance\u003dinstance)"},{"line_number":250,"context_line":"                volume_api.terminate_connection(context, volume_id,"},{"line_number":251,"context_line":"                                                connector)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"        if \u0027serial\u0027 not in connection_info:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_f13695e6","line":250,"updated":"2015-09-21 01:51:26.000000000","message":"Timeout at terminate_connection is still possible. ^_*","commit_id":"df07e9c0c07b332f23566faf7a584cc9b68e0331"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"de5ded97406dc759fc4d6d375d84ce5d85173acf","unresolved":false,"context_lines":[{"line_number":247,"context_line":"                          {\u0027volume_id\u0027: volume_id,"},{"line_number":248,"context_line":"                           \u0027mountpoint\u0027: self[\u0027mount_device\u0027]},"},{"line_number":249,"context_line":"                          context\u003dcontext, instance\u003dinstance)"},{"line_number":250,"context_line":"                volume_api.terminate_connection(context, volume_id,"},{"line_number":251,"context_line":"                                                connector)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"        if \u0027serial\u0027 not in connection_info:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_000f03e2","line":250,"in_reply_to":"ba15a1d1_914dd9c6","updated":"2015-09-22 03:49:40.000000000","message":"Added try except around terminate_connection.","commit_id":"df07e9c0c07b332f23566faf7a584cc9b68e0331"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"97100dd7b9d3b95e33efc0bb0eeb2d0b2dba2c10","unresolved":false,"context_lines":[{"line_number":247,"context_line":"                          {\u0027volume_id\u0027: volume_id,"},{"line_number":248,"context_line":"                           \u0027mountpoint\u0027: self[\u0027mount_device\u0027]},"},{"line_number":249,"context_line":"                          context\u003dcontext, instance\u003dinstance)"},{"line_number":250,"context_line":"                volume_api.terminate_connection(context, volume_id,"},{"line_number":251,"context_line":"                                                connector)"},{"line_number":252,"context_line":""},{"line_number":253,"context_line":"        if \u0027serial\u0027 not in connection_info:"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_914dd9c6","line":250,"in_reply_to":"ba15a1d1_f13695e6","updated":"2015-09-21 03:11:26.000000000","message":"Yes.  I didn\u0027t add try except around terminate_connection because this is already under \"with excutils.save_and_reraise_exception()\".  I had a discussion with Matt about this while reviewing the related patch: https://review.openstack.org/#/c/186742/.  This will also be consistent with how terminate_connection is called in the other places of this function.","commit_id":"df07e9c0c07b332f23566faf7a584cc9b68e0331"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"180c2232ca82b36b3ed87313162aabb0e248131f","unresolved":false,"context_lines":[{"line_number":320,"context_line":"        if not self[\u0027connection_info\u0027]:"},{"line_number":321,"context_line":"            return"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"        connector \u003d virt_driver.get_volume_connector(instance)"},{"line_number":324,"context_line":"        connection_info \u003d volume_api.initialize_connection(context,"},{"line_number":325,"context_line":"                                                           self.volume_id,"},{"line_number":326,"context_line":"                                                           connector)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_f40e58e4","line":323,"updated":"2015-09-21 21:38:47.000000000","message":"What about this?","commit_id":"df07e9c0c07b332f23566faf7a584cc9b68e0331"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"de5ded97406dc759fc4d6d375d84ce5d85173acf","unresolved":false,"context_lines":[{"line_number":320,"context_line":"        if not self[\u0027connection_info\u0027]:"},{"line_number":321,"context_line":"            return"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"        connector \u003d virt_driver.get_volume_connector(instance)"},{"line_number":324,"context_line":"        connection_info \u003d volume_api.initialize_connection(context,"},{"line_number":325,"context_line":"                                                           self.volume_id,"},{"line_number":326,"context_line":"                                                           connector)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba15a1d1_36cdadb7","line":323,"in_reply_to":"ba15a1d1_f40e58e4","updated":"2015-09-22 03:49:40.000000000","message":"This could time out too.  I created a wrapper function for handling initialize_connection so that the same code can be called here and above.","commit_id":"df07e9c0c07b332f23566faf7a584cc9b68e0331"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"1c7f6a1d50622b2c7c4db96c1e81691c321ab9ce","unresolved":false,"context_lines":[{"line_number":250,"context_line":"                # RPC time out between Cinder API and Cinder Volume"},{"line_number":251,"context_line":"                # results in error code 500; HA Proxy time out results"},{"line_number":252,"context_line":"                # in error code 504. There could be other reasons when"},{"line_number":253,"context_line":"                # the error code is 500 or 504."},{"line_number":254,"context_line":"                # If an error occurs during terminate_connection, it is"},{"line_number":255,"context_line":"                # caught and a message is logged."},{"line_number":256,"context_line":"                # If initialize_connection failed for other reasons,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_4362d3d9","line":253,"updated":"2015-09-22 12:17:31.000000000","message":"if there are other case lead to 500/504\nwill terminate_connection be a false step?","commit_id":"30baf1171557b4164b187aea3b9e31052988bf67"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"1816e4074c293871b395822dcb7de4e03bbd544c","unresolved":false,"context_lines":[{"line_number":250,"context_line":"                # RPC time out between Cinder API and Cinder Volume"},{"line_number":251,"context_line":"                # results in error code 500; HA Proxy time out results"},{"line_number":252,"context_line":"                # in error code 504. There could be other reasons when"},{"line_number":253,"context_line":"                # the error code is 500 or 504."},{"line_number":254,"context_line":"                # If an error occurs during terminate_connection, it is"},{"line_number":255,"context_line":"                # caught and a message is logged."},{"line_number":256,"context_line":"                # If initialize_connection failed for other reasons,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_deb238e5","line":253,"in_reply_to":"ba15a1d1_4362d3d9","updated":"2015-09-22 13:01:25.000000000","message":"If you call terminate_connection on a volume that is not mapped to the host, some drivers will return success (which is recommended behavior) and some drivers will throw an exception. This is something that could happen regardless of this fix.  Here the exception is caught and a message is logged.","commit_id":"30baf1171557b4164b187aea3b9e31052988bf67"},{"author":{"_account_id":10628,"name":"Peter Wang","email":"peter.wang13@emc.com","username":"peter_wang"},"change_message_id":"ed5a40031c7e57506342fab9c5862633a0710164","unresolved":false,"context_lines":[{"line_number":271,"context_line":"                                   \u0027mountpoint\u0027: self[\u0027mount_device\u0027]},"},{"line_number":272,"context_line":"                                  context\u003dcontext, instance\u003dinstance)"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"        return connection_info"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    @update_db"},{"line_number":277,"context_line":"    def attach(self, context, instance, volume_api, virt_driver,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_a6e748ab","line":274,"updated":"2015-09-22 09:06:49.000000000","message":"Better to initialize the `connection_info`\nI think:\nif volume_api.initialize_connection throws exception\nthere will be an `NameError: name \u0027connection_info\u0027` is not defined","commit_id":"30baf1171557b4164b187aea3b9e31052988bf67"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"1816e4074c293871b395822dcb7de4e03bbd544c","unresolved":false,"context_lines":[{"line_number":271,"context_line":"                                   \u0027mountpoint\u0027: self[\u0027mount_device\u0027]},"},{"line_number":272,"context_line":"                                  context\u003dcontext, instance\u003dinstance)"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"        return connection_info"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    @update_db"},{"line_number":277,"context_line":"    def attach(self, context, instance, volume_api, virt_driver,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_7cb6958b","line":274,"in_reply_to":"ba15a1d1_08c71a13","updated":"2015-09-22 13:01:25.000000000","message":"Good point.  Done.","commit_id":"30baf1171557b4164b187aea3b9e31052988bf67"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"579a6fc83e4d656016f2b287c1fd0c4620d25693","unresolved":false,"context_lines":[{"line_number":271,"context_line":"                                   \u0027mountpoint\u0027: self[\u0027mount_device\u0027]},"},{"line_number":272,"context_line":"                                  context\u003dcontext, instance\u003dinstance)"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"        return connection_info"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    @update_db"},{"line_number":277,"context_line":"    def attach(self, context, instance, volume_api, virt_driver,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_31acb434","line":274,"in_reply_to":"ba15a1d1_7cb6958b","updated":"2015-09-22 18:02:25.000000000","message":"You shouldn\u0027t get here if initialize_connection raises an exception, that\u0027s what excutils.save_and_reraise_exception() is for.","commit_id":"30baf1171557b4164b187aea3b9e31052988bf67"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"1c7f6a1d50622b2c7c4db96c1e81691c321ab9ce","unresolved":false,"context_lines":[{"line_number":271,"context_line":"                                   \u0027mountpoint\u0027: self[\u0027mount_device\u0027]},"},{"line_number":272,"context_line":"                                  context\u003dcontext, instance\u003dinstance)"},{"line_number":273,"context_line":""},{"line_number":274,"context_line":"        return connection_info"},{"line_number":275,"context_line":""},{"line_number":276,"context_line":"    @update_db"},{"line_number":277,"context_line":"    def attach(self, context, instance, volume_api, virt_driver,"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba15a1d1_08c71a13","line":274,"in_reply_to":"ba15a1d1_a6e748ab","updated":"2015-09-22 12:17:31.000000000","message":"++","commit_id":"30baf1171557b4164b187aea3b9e31052988bf67"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"18117947c71657e9d9f33adb95267f0618c034ae","unresolved":false,"context_lines":[{"line_number":225,"context_line":"                LOG.info(_LI(\u0027preserve multipath_id %s\u0027),"},{"line_number":226,"context_line":"                         connection_info[\u0027data\u0027][\u0027multipath_id\u0027])"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"    def _wrap_initialize_connection(self, context, instance, volume_api,"},{"line_number":229,"context_line":"                                    volume_id, connector):"},{"line_number":230,"context_line":"        connection_info \u003d None"},{"line_number":231,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_5144c872","line":228,"updated":"2015-09-22 18:18:08.000000000","message":"nit: I\u0027d just name this _initialize_connection.","commit_id":"44b37b5cbf3bfec89881e3097e2b5bb748897719"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"8335ed31ae7da7a4561c8a0c805ba79555c4080b","unresolved":false,"context_lines":[{"line_number":225,"context_line":"                LOG.info(_LI(\u0027preserve multipath_id %s\u0027),"},{"line_number":226,"context_line":"                         connection_info[\u0027data\u0027][\u0027multipath_id\u0027])"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"    def _wrap_initialize_connection(self, context, instance, volume_api,"},{"line_number":229,"context_line":"                                    volume_id, connector):"},{"line_number":230,"context_line":"        connection_info \u003d None"},{"line_number":231,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_b8e7403f","line":228,"in_reply_to":"ba15a1d1_5144c872","updated":"2015-09-22 20:47:07.000000000","message":"Done","commit_id":"44b37b5cbf3bfec89881e3097e2b5bb748897719"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"18117947c71657e9d9f33adb95267f0618c034ae","unresolved":false,"context_lines":[{"line_number":241,"context_line":"                LOG.debug(\"Initialize connection failed for volume \""},{"line_number":242,"context_line":"                          \"%(volume_id)s at %(mountpoint)s.\","},{"line_number":243,"context_line":"                          {\u0027volume_id\u0027: volume_id,"},{"line_number":244,"context_line":"                           \u0027mountpoint\u0027: self[\u0027mount_device\u0027]},"},{"line_number":245,"context_line":"                          context\u003dcontext, instance\u003dinstance)"},{"line_number":246,"context_line":"                # If timeout occurs during initialize_connection,"},{"line_number":247,"context_line":"                # the volume could be already mapped to the host, but"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_d158d858","line":244,"updated":"2015-09-22 18:18:08.000000000","message":"nit: self[\u0027mount_device\u0027] kind of grosses me out here.  I\u0027d prefer that we just pass in the mountpoint as a kwarg so the caller can supply it.","commit_id":"44b37b5cbf3bfec89881e3097e2b5bb748897719"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"8335ed31ae7da7a4561c8a0c805ba79555c4080b","unresolved":false,"context_lines":[{"line_number":241,"context_line":"                LOG.debug(\"Initialize connection failed for volume \""},{"line_number":242,"context_line":"                          \"%(volume_id)s at %(mountpoint)s.\","},{"line_number":243,"context_line":"                          {\u0027volume_id\u0027: volume_id,"},{"line_number":244,"context_line":"                           \u0027mountpoint\u0027: self[\u0027mount_device\u0027]},"},{"line_number":245,"context_line":"                          context\u003dcontext, instance\u003dinstance)"},{"line_number":246,"context_line":"                # If timeout occurs during initialize_connection,"},{"line_number":247,"context_line":"                # the volume could be already mapped to the host, but"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_78d0c853","line":244,"in_reply_to":"ba15a1d1_d158d858","updated":"2015-09-22 20:47:07.000000000","message":"Done","commit_id":"44b37b5cbf3bfec89881e3097e2b5bb748897719"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"18117947c71657e9d9f33adb95267f0618c034ae","unresolved":false,"context_lines":[{"line_number":261,"context_line":"                # If initialize_connection fails with other error codes,"},{"line_number":262,"context_line":"                # the exception will be raised without the attempt to"},{"line_number":263,"context_line":"                # call terminate_connection."},{"line_number":264,"context_line":"                if ex.code in (500, 504):"},{"line_number":265,"context_line":"                    try:"},{"line_number":266,"context_line":"                        volume_api.terminate_connection(context, volume_id,"},{"line_number":267,"context_line":"                                                        connector)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_14215e4d","line":264,"updated":"2015-09-22 18:18:08.000000000","message":"The 500 here could be for anything.  It could be for bugs in the cinder code that aren\u0027t caught.  And what\u0027s to prevent the terminate_connection call from timing out?  I guess this is a best attempt, but it shouldn\u0027t mean that we\u0027re going to actually successfully terminate the connection.\n\nFor the timeout that is caught in the cinder-api logs:\n\nhttps://launchpadlibrarian.net/218561960/cinder-api.log\n\nIt would be nice if we could actually map that to a specific client exception in python-cinderclient and then handle that to a specific CinderConnectTimeout exception in nova, but that\u0027s a bigger change, it\u0027s not backportable, and it doesn\u0027t mean we\u0027ll actually be able to terminate the connection.","commit_id":"44b37b5cbf3bfec89881e3097e2b5bb748897719"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"6531ec72826d53f214d038d59103afc51621ef04","unresolved":false,"context_lines":[{"line_number":261,"context_line":"                # If initialize_connection fails with other error codes,"},{"line_number":262,"context_line":"                # the exception will be raised without the attempt to"},{"line_number":263,"context_line":"                # call terminate_connection."},{"line_number":264,"context_line":"                if ex.code in (500, 504):"},{"line_number":265,"context_line":"                    try:"},{"line_number":266,"context_line":"                        volume_api.terminate_connection(context, volume_id,"},{"line_number":267,"context_line":"                                                        connector)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_b729948b","line":264,"in_reply_to":"ba15a1d1_14215e4d","updated":"2015-09-22 18:27:15.000000000","message":"Maybe we shouldn\u0027t check the code.  I\u0027m not sure what\u0027s best here.\n\nIn the case of neutron when spawning an instance, if we fail to associate or create ports for the instance, we roll those back automatically:\n\nhttps://github.com/openstack/nova/blob/master/nova/network/neutronv2/api.py#L722","commit_id":"44b37b5cbf3bfec89881e3097e2b5bb748897719"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"a8dd964f015cc61b51d5e32d19af98383c939868","unresolved":false,"context_lines":[{"line_number":261,"context_line":"                # If initialize_connection fails with other error codes,"},{"line_number":262,"context_line":"                # the exception will be raised without the attempt to"},{"line_number":263,"context_line":"                # call terminate_connection."},{"line_number":264,"context_line":"                if ex.code in (500, 504):"},{"line_number":265,"context_line":"                    try:"},{"line_number":266,"context_line":"                        volume_api.terminate_connection(context, volume_id,"},{"line_number":267,"context_line":"                                                        connector)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_82b690da","line":264,"in_reply_to":"ba15a1d1_14215e4d","updated":"2015-09-22 19:02:05.000000000","message":"You are right. There is a possibility that terminate_connection could time out, but that could happen in other places where terminate_connection is called too.  This is an attempt to roll back, but it may or may not be successful.  As you said, this is the best attempt.\n\nThere are other efforts on trying to solve this problem, but those are more involved and more controversial and we won\u0027t be able to backport them.  The contract between Cinder and Nova is still an on-going effort and will take a while for us to figure that out.  I think we should be able to find a better way to resolve this after we\u0027ve made progress on that effort.\n\nIt is still good to get this fix in.","commit_id":"44b37b5cbf3bfec89881e3097e2b5bb748897719"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"f72bfd8e3afc3a4f10af90dbf65c560ab31546dc","unresolved":false,"context_lines":[{"line_number":261,"context_line":"                # If initialize_connection fails with other error codes,"},{"line_number":262,"context_line":"                # the exception will be raised without the attempt to"},{"line_number":263,"context_line":"                # call terminate_connection."},{"line_number":264,"context_line":"                if ex.code in (500, 504):"},{"line_number":265,"context_line":"                    try:"},{"line_number":266,"context_line":"                        volume_api.terminate_connection(context, volume_id,"},{"line_number":267,"context_line":"                                                        connector)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_3db485f0","line":264,"in_reply_to":"ba15a1d1_1dc0a17b","updated":"2015-09-22 19:18:15.000000000","message":"Not getting what you are saying.  500 is the error code coming from ClientException in cinder client.","commit_id":"44b37b5cbf3bfec89881e3097e2b5bb748897719"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"f72bfd8e3afc3a4f10af90dbf65c560ab31546dc","unresolved":false,"context_lines":[{"line_number":261,"context_line":"                # If initialize_connection fails with other error codes,"},{"line_number":262,"context_line":"                # the exception will be raised without the attempt to"},{"line_number":263,"context_line":"                # call terminate_connection."},{"line_number":264,"context_line":"                if ex.code in (500, 504):"},{"line_number":265,"context_line":"                    try:"},{"line_number":266,"context_line":"                        volume_api.terminate_connection(context, volume_id,"},{"line_number":267,"context_line":"                                                        connector)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_bd6c159d","line":264,"in_reply_to":"ba15a1d1_82b690da","updated":"2015-09-22 19:18:15.000000000","message":"Maybe we don\u0027t check the code then.  500 is too general.  If there\u0027s a more specific code for timeout, it makes more sense to check it.  Since we don\u0027t have that, it is probably better not to check it.","commit_id":"44b37b5cbf3bfec89881e3097e2b5bb748897719"},{"author":{"_account_id":16839,"name":"Cale Rath","email":"ctrath@us.ibm.com","username":"ctrath"},"change_message_id":"e12acd43ef2018bfca35847a22ad58c7e64b0bd5","unresolved":false,"context_lines":[{"line_number":261,"context_line":"                # If initialize_connection fails with other error codes,"},{"line_number":262,"context_line":"                # the exception will be raised without the attempt to"},{"line_number":263,"context_line":"                # call terminate_connection."},{"line_number":264,"context_line":"                if ex.code in (500, 504):"},{"line_number":265,"context_line":"                    try:"},{"line_number":266,"context_line":"                        volume_api.terminate_connection(context, volume_id,"},{"line_number":267,"context_line":"                                                        connector)"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_1dc0a17b","line":264,"in_reply_to":"ba15a1d1_b729948b","updated":"2015-09-22 19:14:31.000000000","message":"It would probably be best to have Cinder to use a code when returning a 500.  I looked through their exception handling code in cinder/api/contrib/volume_actions.py -\u003e _initialize_connection(...) and it appears only messages are returned with the exceptions.","commit_id":"44b37b5cbf3bfec89881e3097e2b5bb748897719"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"18117947c71657e9d9f33adb95267f0618c034ae","unresolved":false,"context_lines":[{"line_number":263,"context_line":"                # call terminate_connection."},{"line_number":264,"context_line":"                if ex.code in (500, 504):"},{"line_number":265,"context_line":"                    try:"},{"line_number":266,"context_line":"                        volume_api.terminate_connection(context, volume_id,"},{"line_number":267,"context_line":"                                                        connector)"},{"line_number":268,"context_line":"                    except Exception:"},{"line_number":269,"context_line":"                        # TODO(xyang): This should be a warning or error"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_34910253","line":266,"updated":"2015-09-22 18:18:08.000000000","message":"It would be helpful to log a debug message before calling terminate_connection saying that initialize connection failed and we\u0027re attempting to rollback.","commit_id":"44b37b5cbf3bfec89881e3097e2b5bb748897719"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"8335ed31ae7da7a4561c8a0c805ba79555c4080b","unresolved":false,"context_lines":[{"line_number":263,"context_line":"                # call terminate_connection."},{"line_number":264,"context_line":"                if ex.code in (500, 504):"},{"line_number":265,"context_line":"                    try:"},{"line_number":266,"context_line":"                        volume_api.terminate_connection(context, volume_id,"},{"line_number":267,"context_line":"                                                        connector)"},{"line_number":268,"context_line":"                    except Exception:"},{"line_number":269,"context_line":"                        # TODO(xyang): This should be a warning or error"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_38ca50a2","line":266,"in_reply_to":"ba15a1d1_34910253","updated":"2015-09-22 20:47:07.000000000","message":"Done","commit_id":"44b37b5cbf3bfec89881e3097e2b5bb748897719"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"18117947c71657e9d9f33adb95267f0618c034ae","unresolved":false,"context_lines":[{"line_number":270,"context_line":"                        # message. Because of string freeze, it is added"},{"line_number":271,"context_line":"                        # as a debug message now. It should be changed to"},{"line_number":272,"context_line":"                        # warning/error after the string freeze."},{"line_number":273,"context_line":"                        LOG.debug(\"Terminate connection failed for volume \""},{"line_number":274,"context_line":"                                  \"%(volume_id)s at %(mountpoint)s.\","},{"line_number":275,"context_line":"                                  {\u0027volume_id\u0027: volume_id,"},{"line_number":276,"context_line":"                                   \u0027mountpoint\u0027: self[\u0027mount_device\u0027]},"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_94748e9c","line":273,"updated":"2015-09-22 18:18:08.000000000","message":"Log the error in here, not the traceback though, just the exception message.","commit_id":"44b37b5cbf3bfec89881e3097e2b5bb748897719"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"8335ed31ae7da7a4561c8a0c805ba79555c4080b","unresolved":false,"context_lines":[{"line_number":270,"context_line":"                        # message. Because of string freeze, it is added"},{"line_number":271,"context_line":"                        # as a debug message now. It should be changed to"},{"line_number":272,"context_line":"                        # warning/error after the string freeze."},{"line_number":273,"context_line":"                        LOG.debug(\"Terminate connection failed for volume \""},{"line_number":274,"context_line":"                                  \"%(volume_id)s at %(mountpoint)s.\","},{"line_number":275,"context_line":"                                  {\u0027volume_id\u0027: volume_id,"},{"line_number":276,"context_line":"                                   \u0027mountpoint\u0027: self[\u0027mount_device\u0027]},"}],"source_content_type":"text/x-python","patch_set":3,"id":"ba15a1d1_58cdc4b5","line":273,"in_reply_to":"ba15a1d1_94748e9c","updated":"2015-09-22 20:47:07.000000000","message":"Done","commit_id":"44b37b5cbf3bfec89881e3097e2b5bb748897719"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eea2b9dbda89bba65265099faa8f9edd6aacb334","unresolved":false,"context_lines":[{"line_number":229,"context_line":"                               volume_id, connector, mount_device):"},{"line_number":230,"context_line":"        connection_info \u003d None"},{"line_number":231,"context_line":"        try:"},{"line_number":232,"context_line":"            connection_info \u003d volume_api.initialize_connection(context,"},{"line_number":233,"context_line":"                                                               volume_id,"},{"line_number":234,"context_line":"                                                               connector)"},{"line_number":235,"context_line":"        except Exception as ex:"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a1a9d01_d6e3e731","line":232,"updated":"2015-09-28 17:33:00.000000000","message":"I just came across this today, but we have the same issue in the libvirt driver\u0027s post_live_migration method:\n\nhttps://review.openstack.org/#/c/202770/17/nova/virt/libvirt/driver.py\n\nThe initial attempt at fixing this bug was doing it in the cinder.py module:\n\nhttps://review.openstack.org/#/c/138664/\n\nWe should probably look at reviving ^ since that would handle this globally rather than just here in block_device.","commit_id":"a08312777d940c35b1fd592be163499cb5f32a4f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eea2b9dbda89bba65265099faa8f9edd6aacb334","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        except Exception as ex:"},{"line_number":236,"context_line":"            with excutils.save_and_reraise_exception():"},{"line_number":237,"context_line":"                try:"},{"line_number":238,"context_line":"                    # TODO(xyang): This should be a warning or error message."},{"line_number":239,"context_line":"                    # Because of string freeze, it is added as a debug message"},{"line_number":240,"context_line":"                    # now. It should be changed to warning/error after the"},{"line_number":241,"context_line":"                    # string freeze."}],"source_content_type":"text/x-python","patch_set":4,"id":"9a1a9d01_362103a0","line":238,"updated":"2015-09-28 17:33:00.000000000","message":"Mitaka is open and this would land in Mitaka, so I think we can just do the translated message object now.","commit_id":"a08312777d940c35b1fd592be163499cb5f32a4f"},{"author":{"_account_id":5997,"name":"Walt","display_name":"Hemna","email":"waboring@hemna.com","username":"walter-boring","status":"SAP"},"change_message_id":"b3037ca906a7bf41b8f9e7fb8e8d01094ee03f50","unresolved":false,"context_lines":[{"line_number":243,"context_line":"                              \"%(volume_id)s at %(mountpoint)s. Attempting \""},{"line_number":244,"context_line":"                              \"to roll back.\","},{"line_number":245,"context_line":"                              {\u0027volume_id\u0027: volume_id,"},{"line_number":246,"context_line":"                               \u0027mountpoint\u0027: mount_device},"},{"line_number":247,"context_line":"                              context\u003dcontext, instance\u003dinstance)"},{"line_number":248,"context_line":"                    volume_api.terminate_connection(context, volume_id,"},{"line_number":249,"context_line":"                                                    connector)"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a1a9d01_bf4d2974","line":246,"updated":"2015-09-25 16:27:13.000000000","message":"I\u0027m not sure of the value of mountpoint here as I believe that\u0027s the VM\u0027s device no?   /dev/vdX, and not the location on the compute host\u0027s device.    It could be confusing for folks looking in the logs and then looking for /dev/vdX on the compute host.","commit_id":"a08312777d940c35b1fd592be163499cb5f32a4f"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"eea2b9dbda89bba65265099faa8f9edd6aacb334","unresolved":false,"context_lines":[{"line_number":248,"context_line":"                    volume_api.terminate_connection(context, volume_id,"},{"line_number":249,"context_line":"                                                    connector)"},{"line_number":250,"context_line":"                except Exception as ex:"},{"line_number":251,"context_line":"                    # TODO(xyang): This should be a warning or error"},{"line_number":252,"context_line":"                    # message. Because of string freeze, it is added"},{"line_number":253,"context_line":"                    # as a debug message now. It should be changed to"},{"line_number":254,"context_line":"                    # warning/error after the string freeze."}],"source_content_type":"text/x-python","patch_set":4,"id":"9a1a9d01_b63513db","line":251,"updated":"2015-09-28 17:33:00.000000000","message":"ditto on mitaka not being under string freeze.","commit_id":"a08312777d940c35b1fd592be163499cb5f32a4f"}]}
