)]}'
{"cinder/volume/manager.py":[{"author":{"_account_id":16917,"name":"Alex O\u0027Rourke","email":"alex.orourke@hpe.com","username":"aorourke"},"change_message_id":"e30bff0f275e76ece4e74cd5ac6caca945b69cd6","unresolved":false,"context_lines":[{"line_number":3123,"context_line":"        self.db.volume_update(context, volume[\u0027id\u0027],"},{"line_number":3124,"context_line":"                              {\u0027replication_status\u0027: \u0027enabled\u0027})"},{"line_number":3125,"context_line":""},{"line_number":3126,"context_line":"    def _enable_replication_cg(self, context, volume):"},{"line_number":3127,"context_line":"        group \u003d self.db.consistencygroup_get("},{"line_number":3128,"context_line":"            context, volume[\u0027consistencygroup_id\u0027])"},{"line_number":3129,"context_line":"        volumes \u003d self.db.volume_get_all_by_group(context, group.id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_beb12f49","line":3126,"updated":"2015-10-08 19:55:57.000000000","message":"I would like something clarified. So if you enable replication on any volume that is in a cg, that entire cg becomes replicated? There will be no way to just replicated one volume that is in a cg? That is what it sounds like in the spec, to maintain consistency, but I just wanted to be sure.","commit_id":"b088e799f9c64d5d5011a689bb8c447aaed39e7a"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"c977ea608c94cc389b4e194e16448690f3f3be1c","unresolved":false,"context_lines":[{"line_number":3123,"context_line":"        self.db.volume_update(context, volume[\u0027id\u0027],"},{"line_number":3124,"context_line":"                              {\u0027replication_status\u0027: \u0027enabled\u0027})"},{"line_number":3125,"context_line":""},{"line_number":3126,"context_line":"    def _enable_replication_cg(self, context, volume):"},{"line_number":3127,"context_line":"        group \u003d self.db.consistencygroup_get("},{"line_number":3128,"context_line":"            context, volume[\u0027consistencygroup_id\u0027])"},{"line_number":3129,"context_line":"        volumes \u003d self.db.volume_get_all_by_group(context, group.id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_7e4717bc","line":3126,"in_reply_to":"7a2fa921_beb12f49","updated":"2015-10-08 20:59:27.000000000","message":"Yes.  I think that should be the desired behavior.  Once a volume is in a group, it can\u0027t be replicated by itself unless if it is removed from the group.","commit_id":"b088e799f9c64d5d5011a689bb8c447aaed39e7a"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"dc3e80de52df3332a95cc375601144c0941c6370","unresolved":false,"context_lines":[{"line_number":3136,"context_line":"            if vol[\u0027id\u0027] !\u003d volume[\u0027id\u0027]:"},{"line_number":3137,"context_line":"                self.db.volume_update(context,"},{"line_number":3138,"context_line":"                                      vol[\u0027id\u0027],"},{"line_number":3139,"context_line":"                                      {\u0027replication_status\u0027: \u0027enabling\u0027})"},{"line_number":3140,"context_line":""},{"line_number":3141,"context_line":"        try:"},{"line_number":3142,"context_line":"            rep_driver_data_list \u003d self.driver.replication_enable_cg("}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_3884575c","line":3139,"updated":"2015-10-08 19:06:29.000000000","message":"Maybe its part of the TODO mentioned, but on these methods where we are changing the status of the other volumes in the group shouldn\u0027t it be done over in cinder/volume/api.py on the other side of the rpcapi?","commit_id":"b088e799f9c64d5d5011a689bb8c447aaed39e7a"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"4c4495fe44d96f3030b0070c7764846fe24f7b71","unresolved":false,"context_lines":[{"line_number":3136,"context_line":"            if vol[\u0027id\u0027] !\u003d volume[\u0027id\u0027]:"},{"line_number":3137,"context_line":"                self.db.volume_update(context,"},{"line_number":3138,"context_line":"                                      vol[\u0027id\u0027],"},{"line_number":3139,"context_line":"                                      {\u0027replication_status\u0027: \u0027enabling\u0027})"},{"line_number":3140,"context_line":""},{"line_number":3141,"context_line":"        try:"},{"line_number":3142,"context_line":"            rep_driver_data_list \u003d self.driver.replication_enable_cg("}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_428035c0","line":3139,"in_reply_to":"7a2fa921_0dea73ac","updated":"2015-10-08 23:15:19.000000000","message":"If we modify volume api, then we should modify rpc api as well, otherwise, we are checking if volume is in a group in both volume api and manager?  It\u0027s just that it will expand the scope of the change if we start changing volume api.  I\u0027m afraid pretty soon phase 1 will be almost like phase 2.  I\u0027m trying to limit the scope of the phase 1 change so that people won\u0027t feel uncomfortable about bringing too many changes on top of replication V2.","commit_id":"b088e799f9c64d5d5011a689bb8c447aaed39e7a"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"912dd06f4f96476ec9eb63b4f319e9d4b0e81763","unresolved":false,"context_lines":[{"line_number":3136,"context_line":"            if vol[\u0027id\u0027] !\u003d volume[\u0027id\u0027]:"},{"line_number":3137,"context_line":"                self.db.volume_update(context,"},{"line_number":3138,"context_line":"                                      vol[\u0027id\u0027],"},{"line_number":3139,"context_line":"                                      {\u0027replication_status\u0027: \u0027enabling\u0027})"},{"line_number":3140,"context_line":""},{"line_number":3141,"context_line":"        try:"},{"line_number":3142,"context_line":"            rep_driver_data_list \u003d self.driver.replication_enable_cg("}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_0dea73ac","line":3139,"in_reply_to":"7a2fa921_324c6e9d","updated":"2015-10-08 22:00:22.000000000","message":"I\u0027m not sure I follow why this can\u0027t be done in phase 1... The code that would get modified is in the internal volume api.py file, not the user-facing rest api. Its not like a new api for cg being added which is what phase 2 presents in the spec (unless I\u0027m misunderstanding something).","commit_id":"b088e799f9c64d5d5011a689bb8c447aaed39e7a"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"c977ea608c94cc389b4e194e16448690f3f3be1c","unresolved":false,"context_lines":[{"line_number":3136,"context_line":"            if vol[\u0027id\u0027] !\u003d volume[\u0027id\u0027]:"},{"line_number":3137,"context_line":"                self.db.volume_update(context,"},{"line_number":3138,"context_line":"                                      vol[\u0027id\u0027],"},{"line_number":3139,"context_line":"                                      {\u0027replication_status\u0027: \u0027enabling\u0027})"},{"line_number":3140,"context_line":""},{"line_number":3141,"context_line":"        try:"},{"line_number":3142,"context_line":"            rep_driver_data_list \u003d self.driver.replication_enable_cg("}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_5ef57b54","line":3139,"in_reply_to":"7a2fa921_3884575c","updated":"2015-10-08 20:59:27.000000000","message":"So all these group level functions are pretty much a copy and paste from the volume level functions.  Therefore, however this is handled for a volume level function, we should be doing the same for a group level function.  Are you referring to the case when the replicated device is managed?  I don\u0027t see this is handled by the other side of the rpcapi for a volume replication.  I can add a TODO here.","commit_id":"b088e799f9c64d5d5011a689bb8c447aaed39e7a"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"70598820f3d29e7228904361fa8a9f81d06b67b5","unresolved":false,"context_lines":[{"line_number":3136,"context_line":"            if vol[\u0027id\u0027] !\u003d volume[\u0027id\u0027]:"},{"line_number":3137,"context_line":"                self.db.volume_update(context,"},{"line_number":3138,"context_line":"                                      vol[\u0027id\u0027],"},{"line_number":3139,"context_line":"                                      {\u0027replication_status\u0027: \u0027enabling\u0027})"},{"line_number":3140,"context_line":""},{"line_number":3141,"context_line":"        try:"},{"line_number":3142,"context_line":"            rep_driver_data_list \u003d self.driver.replication_enable_cg("}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_e7e4f6e0","line":3139,"in_reply_to":"7a2fa921_5ef57b54","updated":"2015-10-08 21:07:55.000000000","message":"Nah, nothing to do with managed/unmanaged or anything. So if we want to follow what the single-volume path does this loop changing status to \u0027enabling\u0027 should be over on the other side of the rpcapi in api.py like enable_replication  https://github.com/openstack/cinder/blob/master/cinder/volume/api.py#L1626\n\nOtherwise with the current code here we set the status of a single volume in the cg to \u0027enabling\u0027, call across the rpcapi to the manager code here and set the rest of the group here. Seems weird to me.","commit_id":"b088e799f9c64d5d5011a689bb8c447aaed39e7a"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"6182ae06bc4214ad4bd92acbb7a52996d2d5eca3","unresolved":false,"context_lines":[{"line_number":3136,"context_line":"            if vol[\u0027id\u0027] !\u003d volume[\u0027id\u0027]:"},{"line_number":3137,"context_line":"                self.db.volume_update(context,"},{"line_number":3138,"context_line":"                                      vol[\u0027id\u0027],"},{"line_number":3139,"context_line":"                                      {\u0027replication_status\u0027: \u0027enabling\u0027})"},{"line_number":3140,"context_line":""},{"line_number":3141,"context_line":"        try:"},{"line_number":3142,"context_line":"            rep_driver_data_list \u003d self.driver.replication_enable_cg("}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_324c6e9d","line":3139,"in_reply_to":"7a2fa921_e7e4f6e0","updated":"2015-10-08 21:46:52.000000000","message":"That line changes the status of the specific volume A in the db.\n\nhttps://github.com/openstack/cinder/blob/master/cinder/volume/api.py#L1626\n\nThe next line sends the enable_replication request over RPC.\n\nhttps://github.com/openstack/cinder/blob/master/cinder/volume/api.py#L1627\n\nThe request will be handled by line #3076 in this file.  If the volume A is in a CG, it will call line #3015 in this file and it will gets here.\n\nI think now I get what you are saying.  It is weird, but this is because phase 1 avoids making changes in Cinder API.  So we have to change status of other volumes in the manager to achieve that.\n\nIn phase 2, statuses of all volumes should be changed in Cinder API.","commit_id":"b088e799f9c64d5d5011a689bb8c447aaed39e7a"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"dc3e80de52df3332a95cc375601144c0941c6370","unresolved":false,"context_lines":[{"line_number":3144,"context_line":"        except exception.CinderException:"},{"line_number":3145,"context_line":"            err_msg \u003d (_(\"Enable replication for cg failed.\"))"},{"line_number":3146,"context_line":"            LOG.exception(err_msg, resource\u003dgroup)"},{"line_number":3147,"context_line":"            raise exception.VolumeBackendAPIException(data\u003derr_msg)"},{"line_number":3148,"context_line":"        try:"},{"line_number":3149,"context_line":"            if rep_driver_data_list:"},{"line_number":3150,"context_line":"                for data in rep_driver_data_list:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_16f04d90","line":3147,"updated":"2015-10-08 19:06:29.000000000","message":"Should we put the replication status into an error state before raising? Or is the intention to leave them in enabling?","commit_id":"b088e799f9c64d5d5011a689bb8c447aaed39e7a"},{"author":{"_account_id":16917,"name":"Alex O\u0027Rourke","email":"alex.orourke@hpe.com","username":"aorourke"},"change_message_id":"e30bff0f275e76ece4e74cd5ac6caca945b69cd6","unresolved":false,"context_lines":[{"line_number":3144,"context_line":"        except exception.CinderException:"},{"line_number":3145,"context_line":"            err_msg \u003d (_(\"Enable replication for cg failed.\"))"},{"line_number":3146,"context_line":"            LOG.exception(err_msg, resource\u003dgroup)"},{"line_number":3147,"context_line":"            raise exception.VolumeBackendAPIException(data\u003derr_msg)"},{"line_number":3148,"context_line":"        try:"},{"line_number":3149,"context_line":"            if rep_driver_data_list:"},{"line_number":3150,"context_line":"                for data in rep_driver_data_list:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_5e06db1d","line":3147,"in_reply_to":"7a2fa921_16f04d90","updated":"2015-10-08 19:55:57.000000000","message":"I agree, I don\u0027t think it should be stuck in enabling. In the single volume replication spec, I have a bug filed about this issue where it gets stuck in enabling state https://bugs.launchpad.net/cinder/+bug/1503439. It should be changed here.","commit_id":"b088e799f9c64d5d5011a689bb8c447aaed39e7a"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"c977ea608c94cc389b4e194e16448690f3f3be1c","unresolved":false,"context_lines":[{"line_number":3144,"context_line":"        except exception.CinderException:"},{"line_number":3145,"context_line":"            err_msg \u003d (_(\"Enable replication for cg failed.\"))"},{"line_number":3146,"context_line":"            LOG.exception(err_msg, resource\u003dgroup)"},{"line_number":3147,"context_line":"            raise exception.VolumeBackendAPIException(data\u003derr_msg)"},{"line_number":3148,"context_line":"        try:"},{"line_number":3149,"context_line":"            if rep_driver_data_list:"},{"line_number":3150,"context_line":"                for data in rep_driver_data_list:"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_6106961e","line":3147,"in_reply_to":"7a2fa921_5e06db1d","updated":"2015-10-08 20:59:27.000000000","message":"I think you are right.  The status should be changed to \"error\".","commit_id":"b088e799f9c64d5d5011a689bb8c447aaed39e7a"},{"author":{"_account_id":12924,"name":"Patrick East","email":"east.patrick@gmail.com","username":"patrick.east"},"change_message_id":"dc3e80de52df3332a95cc375601144c0941c6370","unresolved":false,"context_lines":[{"line_number":3154,"context_line":"        except exception.CinderException as ex:"},{"line_number":3155,"context_line":"            LOG.exception(_LE(\"Driver replication data update failed.\"),"},{"line_number":3156,"context_line":"                          resource\u003dgroup)"},{"line_number":3157,"context_line":"            raise exception.VolumeBackendAPIException(reason\u003dex)"},{"line_number":3158,"context_line":"        for vol in volumes:"},{"line_number":3159,"context_line":"            self.db.volume_update(context, vol[\u0027id\u0027],"},{"line_number":3160,"context_line":"                                  {\u0027replication_status\u0027: \u0027enabled\u0027})"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_16e36da5","line":3157,"updated":"2015-10-08 19:06:29.000000000","message":"Same here, seems like we shouldn\u0027t just leave them in \u0027enabling\u0027 if something broke and we caught it.","commit_id":"b088e799f9c64d5d5011a689bb8c447aaed39e7a"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"c977ea608c94cc389b4e194e16448690f3f3be1c","unresolved":false,"context_lines":[{"line_number":3154,"context_line":"        except exception.CinderException as ex:"},{"line_number":3155,"context_line":"            LOG.exception(_LE(\"Driver replication data update failed.\"),"},{"line_number":3156,"context_line":"                          resource\u003dgroup)"},{"line_number":3157,"context_line":"            raise exception.VolumeBackendAPIException(reason\u003dex)"},{"line_number":3158,"context_line":"        for vol in volumes:"},{"line_number":3159,"context_line":"            self.db.volume_update(context, vol[\u0027id\u0027],"},{"line_number":3160,"context_line":"                                  {\u0027replication_status\u0027: \u0027enabled\u0027})"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_e112a656","line":3157,"in_reply_to":"7a2fa921_16e36da5","updated":"2015-10-08 20:59:27.000000000","message":"Sure.","commit_id":"b088e799f9c64d5d5011a689bb8c447aaed39e7a"},{"author":{"_account_id":16917,"name":"Alex O\u0027Rourke","email":"alex.orourke@hpe.com","username":"aorourke"},"change_message_id":"29a34b5db71fabf5ec6ef5fb6a3a1217b4b5a8a7","unresolved":false,"context_lines":[{"line_number":3510,"context_line":""},{"line_number":3511,"context_line":"        Example response for replicating to a managed backend:"},{"line_number":3512,"context_line":"            {\u0027volume_id\u0027: volume[\u0027id\u0027],"},{"line_number":3513,"context_line":"             \u0027targets\u0027:[{\u0027managed_host\u0027: \u0027backend_name\u0027}...]"},{"line_number":3514,"context_line":""},{"line_number":3515,"context_line":"        Example response for replicating to an unmanaged backend:"},{"line_number":3516,"context_line":"            {\u0027volume_id\u0027: volume[\u0027id\u0027], \u0027targets\u0027:[{\u0027san_ip\u0027: \u00271.1.1.1\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"7a2fa921_6c189bd3","line":3513,"updated":"2015-10-09 18:31:50.000000000","message":"Just something to add for your next iteration, \u0027remote_device_id\u0027: \u0027vendor-id-for-target-device\u0027 is now a standard for both managed and unmanaged targets. \n\nhttps://review.openstack.org/#/c/232261","commit_id":"b088e799f9c64d5d5011a689bb8c447aaed39e7a"}]}