)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"bde17b764bb4aca7f8ec70a1c6ee33edb524b10e","unresolved":false,"context_lines":[{"line_number":15,"context_line":"\u0027--incremental\u003dFalse\u0027, so we can use the parent_id to ensure whether"},{"line_number":16,"context_line":"do the full backup in rbd driver or not."},{"line_number":17,"context_line":"If the incremental flag \u0027--incremental\u0027 is not specified, this patch will"},{"line_number":18,"context_line":"always create a new full backup for rbd volume."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: I516b7c82b05b26e81195f7f106d43a9e0804082d"},{"line_number":21,"context_line":"Closes-Bug: #1810270"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":51,"id":"7faddb67_a9daa074","line":18,"updated":"2019-07-18 09:09:11.000000000","message":"nit: Please fix the length of the commit message to wrap at 72 characters, as per https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure","commit_id":"9345ffff97747b3b7364f18a935b32128584a61b"}],"cinder/backup/api.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"53ecae2a09955bf92ac44e14c8dcaa4970c53401","unresolved":false,"context_lines":[{"line_number":318,"context_line":"            backup \u003d objects.Backup(context\u003dcontext, **kwargs)"},{"line_number":319,"context_line":"            backup.create()"},{"line_number":320,"context_line":"            if latest_backup:"},{"line_number":321,"context_line":"                backup.service_metadata \u003d latest_backup.service_metadata"},{"line_number":322,"context_line":"            if not snapshot_id:"},{"line_number":323,"context_line":"                backup.data_timestamp \u003d backup.created_at"},{"line_number":324,"context_line":"                backup.save()"}],"source_content_type":"text/x-python","patch_set":29,"id":"5fc1f717_be3a41ca","line":321,"updated":"2019-04-04 11:55:52.000000000","message":"This is Ceph specific, other backup drivers use it for the object_prefix, so it feels wrong to inherit this from parent to children...","commit_id":"ee408f49bc9fcc7ba89d1d2506b4d7328b5cc76b"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"c5fac8e74e90689bd878a4e4e11d4520d5bbd948","unresolved":false,"context_lines":[{"line_number":318,"context_line":"            backup \u003d objects.Backup(context\u003dcontext, **kwargs)"},{"line_number":319,"context_line":"            backup.create()"},{"line_number":320,"context_line":"            if latest_backup:"},{"line_number":321,"context_line":"                backup.service_metadata \u003d latest_backup.service_metadata"},{"line_number":322,"context_line":"            if not snapshot_id:"},{"line_number":323,"context_line":"                backup.data_timestamp \u003d backup.created_at"},{"line_number":324,"context_line":"                backup.save()"}],"source_content_type":"text/x-python","patch_set":29,"id":"5fc1f717_5668d3b8","line":321,"in_reply_to":"5fc1f717_be3a41ca","updated":"2019-04-05 05:44:42.000000000","message":"emm, It works but looks weird indeed. :(","commit_id":"ee408f49bc9fcc7ba89d1d2506b4d7328b5cc76b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ac0e3b551e44b2f214af5cc4b538afae95f058a0","unresolved":false,"context_lines":[{"line_number":317,"context_line":"            }"},{"line_number":318,"context_line":"            backup \u003d objects.Backup(context\u003dcontext, **kwargs)"},{"line_number":319,"context_line":"            backup.create()"},{"line_number":320,"context_line":"            if latest_backup:"},{"line_number":321,"context_line":"                backup.parent \u003d latest_backup"},{"line_number":322,"context_line":"                self.backup_rpcapi.create_backup(context, backup)"},{"line_number":323,"context_line":"            if not snapshot_id:"}],"source_content_type":"text/x-python","patch_set":30,"id":"5fc1f717_b4b32fe9","line":320,"updated":"2019-04-05 10:19:22.000000000","message":"nit: You can move this around L280:\n\n parent \u003d None\n if latest_backup:\n     parent \u003d latest_backup\n\n\nAnd then on L315 you add\n       \u0027parent\u0027: parent,","commit_id":"a6308d42a32b5b92bca33296f55ac749d60a1d95"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b8657c35258f90fbd8e6cd21085d3b0b094d6b6b","unresolved":false,"context_lines":[{"line_number":277,"context_line":"                msg \u003d _(\u0027No backups available to do an incremental backup.\u0027)"},{"line_number":278,"context_line":"                raise exception.InvalidBackup(reason\u003dmsg)"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        parent_id \u003d None"},{"line_number":281,"context_line":"        if latest_backup:"},{"line_number":282,"context_line":"            parent \u003d latest_backup"},{"line_number":283,"context_line":"            parent_id \u003d latest_backup.id"}],"source_content_type":"text/x-python","patch_set":31,"id":"5fc1f717_b2b1a8e2","line":280,"updated":"2019-04-10 09:59:02.000000000","message":"-1:  missing the default value, if we don\u0027t have this we could reach L317 without the parent variable being defined\n\n parent \u003d None","commit_id":"cda3cd1cf9b8fc8c22b68b1e35530990b0b5b87b"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"dc55a5f08b44236c76e5c9d6800cdd36938cd353","unresolved":false,"context_lines":[{"line_number":277,"context_line":"                msg \u003d _(\u0027No backups available to do an incremental backup.\u0027)"},{"line_number":278,"context_line":"                raise exception.InvalidBackup(reason\u003dmsg)"},{"line_number":279,"context_line":""},{"line_number":280,"context_line":"        parent_id \u003d None"},{"line_number":281,"context_line":"        if latest_backup:"},{"line_number":282,"context_line":"            parent \u003d latest_backup"},{"line_number":283,"context_line":"            parent_id \u003d latest_backup.id"}],"source_content_type":"text/x-python","patch_set":31,"id":"5fc1f717_781d5f0d","line":280,"in_reply_to":"5fc1f717_b2b1a8e2","updated":"2019-04-10 11:51:00.000000000","message":"Done","commit_id":"cda3cd1cf9b8fc8c22b68b1e35530990b0b5b87b"}],"cinder/backup/drivers/ceph.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"48d91c128784326a074d70ad20c5be6444fc8ab8","unresolved":false,"context_lines":[{"line_number":655,"context_line":"        updates \u003d {}"},{"line_number":656,"context_line":"        base_name \u003d self._get_backup_base_name(volume_id, diff_format\u003dTrue)"},{"line_number":657,"context_line":"        image_created \u003d False"},{"line_number":658,"context_line":"        # If backup.parent_id is None, that means to create a full backup"},{"line_number":659,"context_line":"        # for rbd. This will fix the bug that can\u0027t create a full backup"},{"line_number":660,"context_line":"        # when incremental\u003dFalse."},{"line_number":661,"context_line":"        full_backup \u003d False"},{"line_number":662,"context_line":"        if backup.parent_id is None:"},{"line_number":663,"context_line":"            full_backup \u003d True"},{"line_number":664,"context_line":"            full_base_name \u003d self._get_backup_base_name(volume_id, backup.id)"},{"line_number":665,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"},{"line_number":666,"context_line":"                                  backup.container)) as client:"},{"line_number":667,"context_line":"            # If from_snap does not exist at the destination (and the"}],"source_content_type":"text/x-python","patch_set":9,"id":"bfdaf3ff_87e29c2e","line":664,"range":{"start_line":658,"start_character":8,"end_line":664,"end_character":77},"updated":"2019-01-16 07:24:36.000000000","message":"Can we check the \u0027parent_id\u0027 in the backup method (L#923) and use the \u0027do_full_backup\u0027 (L#942) to create the full backup","commit_id":"c21778b196f435f68c0d593a1a1f9d0bcdf24eab"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"eb85f622cf437ea44adeb33227c3e1e86990d4f2","unresolved":false,"context_lines":[{"line_number":655,"context_line":"        updates \u003d {}"},{"line_number":656,"context_line":"        base_name \u003d self._get_backup_base_name(volume_id, diff_format\u003dTrue)"},{"line_number":657,"context_line":"        image_created \u003d False"},{"line_number":658,"context_line":"        # If backup.parent_id is None, that means to create a full backup"},{"line_number":659,"context_line":"        # for rbd. This will fix the bug that can\u0027t create a full backup"},{"line_number":660,"context_line":"        # when incremental\u003dFalse."},{"line_number":661,"context_line":"        full_backup \u003d False"},{"line_number":662,"context_line":"        if backup.parent_id is None:"},{"line_number":663,"context_line":"            full_backup \u003d True"},{"line_number":664,"context_line":"            full_base_name \u003d self._get_backup_base_name(volume_id, backup.id)"},{"line_number":665,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"},{"line_number":666,"context_line":"                                  backup.container)) as client:"},{"line_number":667,"context_line":"            # If from_snap does not exist at the destination (and the"}],"source_content_type":"text/x-python","patch_set":9,"id":"9fdfeff1_d8e0dc99","line":664,"range":{"start_line":658,"start_character":8,"end_line":664,"end_character":77},"in_reply_to":"bfdaf3ff_87e29c2e","updated":"2019-01-19 06:16:57.000000000","message":"Since L#923  function full backup is for non-rbd src volume(non-rbd volume will always do full backup). It will read the data from src volume and then write to rbd image. So I think that way is a little heavy for rbd volume\u0027s full backup. We can just reuse the _rbd_diff_transfer method like L#673.","commit_id":"c21778b196f435f68c0d593a1a1f9d0bcdf24eab"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"48d91c128784326a074d70ad20c5be6444fc8ab8","unresolved":false,"context_lines":[{"line_number":687,"context_line":"                # Create new base image"},{"line_number":688,"context_line":"                self._create_base_image(base_name, length, client)"},{"line_number":689,"context_line":"                image_created \u003d True"},{"line_number":690,"context_line":"            elif full_backup:"},{"line_number":691,"context_line":"                # Create new base image"},{"line_number":692,"context_line":"                from_snap \u003d None"},{"line_number":693,"context_line":"                self._create_base_image(full_base_name, length, client)"},{"line_number":694,"context_line":"                base_name \u003d full_base_name"},{"line_number":695,"context_line":"                image_created \u003d True"},{"line_number":696,"context_line":"            else:"},{"line_number":697,"context_line":"                # If a from_snap is defined and is present in the source volume"},{"line_number":698,"context_line":"                # image but does not exist in the backup base then we look down"}],"source_content_type":"text/x-python","patch_set":9,"id":"bfdaf3ff_a7c6c099","line":695,"range":{"start_line":690,"start_character":12,"end_line":695,"end_character":36},"updated":"2019-01-16 07:24:36.000000000","message":"we already have a _full_backup method (L#761) which i think also does the same thing as _create_base_image method.\nis there any additional advantage to this code?","commit_id":"c21778b196f435f68c0d593a1a1f9d0bcdf24eab"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"eb85f622cf437ea44adeb33227c3e1e86990d4f2","unresolved":false,"context_lines":[{"line_number":687,"context_line":"                # Create new base image"},{"line_number":688,"context_line":"                self._create_base_image(base_name, length, client)"},{"line_number":689,"context_line":"                image_created \u003d True"},{"line_number":690,"context_line":"            elif full_backup:"},{"line_number":691,"context_line":"                # Create new base image"},{"line_number":692,"context_line":"                from_snap \u003d None"},{"line_number":693,"context_line":"                self._create_base_image(full_base_name, length, client)"},{"line_number":694,"context_line":"                base_name \u003d full_base_name"},{"line_number":695,"context_line":"                image_created \u003d True"},{"line_number":696,"context_line":"            else:"},{"line_number":697,"context_line":"                # If a from_snap is defined and is present in the source volume"},{"line_number":698,"context_line":"                # image but does not exist in the backup base then we look down"}],"source_content_type":"text/x-python","patch_set":9,"id":"9fdfeff1_78ef7087","line":695,"range":{"start_line":690,"start_character":12,"end_line":695,"end_character":36},"in_reply_to":"bfdaf3ff_a7c6c099","updated":"2019-01-19 06:16:57.000000000","message":"The reason is replied in the above comment. thanks.","commit_id":"c21778b196f435f68c0d593a1a1f9d0bcdf24eab"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"fa936be49f7873ffd2494a82b239706fd0dc2c0d","unresolved":false,"context_lines":[{"line_number":655,"context_line":"        updates \u003d {}"},{"line_number":656,"context_line":"        base_name \u003d self._get_backup_base_name(volume_id, diff_format\u003dTrue)"},{"line_number":657,"context_line":"        image_created \u003d False"},{"line_number":658,"context_line":"        # If backup.parent_id is None, that means to create a full backup"},{"line_number":659,"context_line":"        # for rbd. This will fix the bug that can\u0027t create a full backup"},{"line_number":660,"context_line":"        # when incremental\u003dFalse."},{"line_number":661,"context_line":"        full_backup \u003d False"},{"line_number":662,"context_line":"        if backup.parent_id is None:"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_7aa6cdad","line":659,"range":{"start_line":658,"start_character":8,"end_line":659,"end_character":18},"updated":"2019-01-31 02:01:22.000000000","message":"I think this should be part of method\u0027s docstring. Because currently, the process not only creates an incremental backup anymore.","commit_id":"fa484a7bf944a86948bbb392207319d848fd4ec4"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"fa936be49f7873ffd2494a82b239706fd0dc2c0d","unresolved":false,"context_lines":[{"line_number":658,"context_line":"        # If backup.parent_id is None, that means to create a full backup"},{"line_number":659,"context_line":"        # for rbd. This will fix the bug that can\u0027t create a full backup"},{"line_number":660,"context_line":"        # when incremental\u003dFalse."},{"line_number":661,"context_line":"        full_backup \u003d False"},{"line_number":662,"context_line":"        if backup.parent_id is None:"},{"line_number":663,"context_line":"            full_backup \u003d True"},{"line_number":664,"context_line":"            full_base_name \u003d self._get_backup_base_name(volume_id, backup.id)"},{"line_number":665,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"},{"line_number":666,"context_line":"                                  backup.container)) as client:"},{"line_number":667,"context_line":"            # If from_snap does not exist at the destination (and the"},{"line_number":668,"context_line":"            # destination exists), this implies a previous backup has failed."},{"line_number":669,"context_line":"            # In this case we will force a full backup."},{"line_number":670,"context_line":"            #"},{"line_number":671,"context_line":"            # TODO(dosaboy): find a way to repair the broken backup"},{"line_number":672,"context_line":"            #"},{"line_number":673,"context_line":"            if base_name not in eventlet.tpool.Proxy(self.rbd.RBD()).list("},{"line_number":674,"context_line":"                    ioctx\u003dclient.ioctx):"},{"line_number":675,"context_line":"                src_vol_snapshots \u003d self.get_backup_snaps(source_rbd_image)"},{"line_number":676,"context_line":"                if src_vol_snapshots:"},{"line_number":677,"context_line":"                    # If there are source volume snapshots but base does not"},{"line_number":678,"context_line":"                    # exist then we delete it and set from_snap to None"},{"line_number":679,"context_line":"                    LOG.debug(\"Volume \u0027%(volume)s\u0027 has stale source \""},{"line_number":680,"context_line":"                              \"snapshots so deleting them.\","},{"line_number":681,"context_line":"                              {\u0027volume\u0027: volume_id})"},{"line_number":682,"context_line":"                for snap in src_vol_snapshots:"},{"line_number":683,"context_line":"                    from_snap \u003d snap[\u0027name\u0027]"},{"line_number":684,"context_line":"                    source_rbd_image.remove_snap(from_snap)"},{"line_number":685,"context_line":"                from_snap \u003d None"},{"line_number":686,"context_line":""},{"line_number":687,"context_line":"                # Create new base image"},{"line_number":688,"context_line":"                self._create_base_image(base_name, length, client)"},{"line_number":689,"context_line":"                image_created \u003d True"},{"line_number":690,"context_line":"            elif full_backup:"},{"line_number":691,"context_line":"                # Create new base image"},{"line_number":692,"context_line":"                from_snap \u003d None"},{"line_number":693,"context_line":"                self._create_base_image(full_base_name, length, client)"},{"line_number":694,"context_line":"                base_name \u003d full_base_name"},{"line_number":695,"context_line":"                image_created \u003d True"},{"line_number":696,"context_line":"            else:"},{"line_number":697,"context_line":"                # If a from_snap is defined and is present in the source volume"},{"line_number":698,"context_line":"                # image but does not exist in the backup base then we look down"},{"line_number":699,"context_line":"                # the list of source volume snapshots and find the latest one"},{"line_number":700,"context_line":"                # for which a backup snapshot exist in the backup base. Until"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fdfeff1_0c02eb6f","line":697,"range":{"start_line":661,"start_character":8,"end_line":697,"end_character":39},"updated":"2019-01-31 02:01:22.000000000","message":"In my opinion, this looks a bit untidy. I think these two options could help to clarify the code:\n\n1. Create two methods for rbd:\n\n```\nif not backup.parent_id:\n    self.perform_full_rbd_backup(...)\nelse:\n    self.perform_incremental_rbd_backup(...)\n```\n\n\n2. Split all in a big if:\n```\nif not backup.parent_id:\n    from_snap \u003d None\n    image_created \u003d True\n    base_name \u003d self._get_backup_base_name(volume_id, backup.id)\n            with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,\n                                  backup.container)) as client:\n                self._create_base_image(base_name, length, client)\n\n```\n\nIn this way, avoid call two times `self._get_backup_base_name`.\nI prefer the first option.","commit_id":"fa484a7bf944a86948bbb392207319d848fd4ec4"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f1b921cbe0bb6fd4a393e2194dc5eb22d987d702","unresolved":false,"context_lines":[{"line_number":692,"context_line":"                from_snap \u003d None"},{"line_number":693,"context_line":""},{"line_number":694,"context_line":"                # Create new base image"},{"line_number":695,"context_line":"                self._create_base_image(base_name, length, client)"},{"line_number":696,"context_line":"                image_created \u003d True"},{"line_number":697,"context_line":"            else:"},{"line_number":698,"context_line":"                # If a from_snap is defined and is present in the source volume"}],"source_content_type":"text/x-python","patch_set":20,"id":"5fc1f717_b42a867b","line":695,"updated":"2019-03-21 14:42:22.000000000","message":"If this is an incremental method it should never create a base image, it should fail if it can\u0027t find the image for the parent.","commit_id":"07babdb687b519253c8d8a86d0022a7aab69bb44"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"d72a48c8ee4b3e7cfa1e47e8ef190a45e68e0f2f","unresolved":false,"context_lines":[{"line_number":692,"context_line":"                from_snap \u003d None"},{"line_number":693,"context_line":""},{"line_number":694,"context_line":"                # Create new base image"},{"line_number":695,"context_line":"                self._create_base_image(base_name, length, client)"},{"line_number":696,"context_line":"                image_created \u003d True"},{"line_number":697,"context_line":"            else:"},{"line_number":698,"context_line":"                # If a from_snap is defined and is present in the source volume"}],"source_content_type":"text/x-python","patch_set":20,"id":"5fc1f717_18e31f36","line":695,"in_reply_to":"5fc1f717_b42a867b","updated":"2019-03-26 13:30:24.000000000","message":"Done","commit_id":"07babdb687b519253c8d8a86d0022a7aab69bb44"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"1844919963baf33b92e0d86952dbbf78cb6f73b6","unresolved":false,"context_lines":[{"line_number":735,"context_line":"            # find the first full backup name as the base_name"},{"line_number":736,"context_line":"            backup_id \u003d backup.parent_id"},{"line_number":737,"context_line":"            last_incr \u003d backup_id"},{"line_number":738,"context_line":"            while(True):"},{"line_number":739,"context_line":"                backup_tmp \u003d self.db.backup_get(self.context, backup_id)"},{"line_number":740,"context_line":"                if backup_tmp.parent_id:"},{"line_number":741,"context_line":"                    backup_id \u003d backup_tmp.parent_id"}],"source_content_type":"text/x-python","patch_set":20,"id":"5fc1f717_c8546557","line":738,"range":{"start_line":738,"start_character":17,"end_line":738,"end_character":18},"updated":"2019-03-20 20:02:49.000000000","message":"I\u0027m surprised flake8 didn\u0027t flag this for not having a space  before (","commit_id":"07babdb687b519253c8d8a86d0022a7aab69bb44"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"b36a3897f56183adf18aa68cc92a90074256dccc","unresolved":false,"context_lines":[{"line_number":735,"context_line":"            # find the first full backup name as the base_name"},{"line_number":736,"context_line":"            backup_id \u003d backup.parent_id"},{"line_number":737,"context_line":"            last_incr \u003d backup_id"},{"line_number":738,"context_line":"            while(True):"},{"line_number":739,"context_line":"                backup_tmp \u003d self.db.backup_get(self.context, backup_id)"},{"line_number":740,"context_line":"                if backup_tmp.parent_id:"},{"line_number":741,"context_line":"                    backup_id \u003d backup_tmp.parent_id"}],"source_content_type":"text/x-python","patch_set":20,"id":"5fc1f717_f5689ef5","line":738,"range":{"start_line":738,"start_character":17,"end_line":738,"end_character":18},"in_reply_to":"5fc1f717_c8546557","updated":"2019-03-26 12:14:58.000000000","message":"Done","commit_id":"07babdb687b519253c8d8a86d0022a7aab69bb44"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"794ba40721ab604baa3a7428919e9f690e42fda5","unresolved":false,"context_lines":[{"line_number":736,"context_line":"            backup_id \u003d backup.parent_id"},{"line_number":737,"context_line":"            last_incr \u003d backup_id"},{"line_number":738,"context_line":"            while(True):"},{"line_number":739,"context_line":"                backup_tmp \u003d self.db.backup_get(self.context, backup_id)"},{"line_number":740,"context_line":"                if backup_tmp.parent_id:"},{"line_number":741,"context_line":"                    backup_id \u003d backup_tmp.parent_id"},{"line_number":742,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":20,"id":"5fc1f717_3939af3e","line":739,"updated":"2019-03-21 14:25:09.000000000","message":"I\u0027m not sure why this is looping back to find the full -- the ceph backup driver was supposed to be able to do backups incrementally based on the previous incr backup AFAIK.","commit_id":"07babdb687b519253c8d8a86d0022a7aab69bb44"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"24cbe1eb91123692ac4b524c9b16efb2e012595a","unresolved":false,"context_lines":[{"line_number":736,"context_line":"            backup_id \u003d backup.parent_id"},{"line_number":737,"context_line":"            last_incr \u003d backup_id"},{"line_number":738,"context_line":"            while(True):"},{"line_number":739,"context_line":"                backup_tmp \u003d self.db.backup_get(self.context, backup_id)"},{"line_number":740,"context_line":"                if backup_tmp.parent_id:"},{"line_number":741,"context_line":"                    backup_id \u003d backup_tmp.parent_id"},{"line_number":742,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":20,"id":"5fc1f717_09de613d","line":739,"in_reply_to":"5fc1f717_1e4227ac","updated":"2019-03-27 00:56:14.000000000","message":"I got your point,  so we didn\u0027t need to find the base image and then find all the backup snapshot to see what is from_snap. Just use the volume_id + parent_id to do the incr backup, right?","commit_id":"07babdb687b519253c8d8a86d0022a7aab69bb44"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"bc88af6cf1ad401654b51c884fcfae6b6671beab","unresolved":false,"context_lines":[{"line_number":736,"context_line":"            backup_id \u003d backup.parent_id"},{"line_number":737,"context_line":"            last_incr \u003d backup_id"},{"line_number":738,"context_line":"            while(True):"},{"line_number":739,"context_line":"                backup_tmp \u003d self.db.backup_get(self.context, backup_id)"},{"line_number":740,"context_line":"                if backup_tmp.parent_id:"},{"line_number":741,"context_line":"                    backup_id \u003d backup_tmp.parent_id"},{"line_number":742,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":20,"id":"5fc1f717_1e4227ac","line":739,"in_reply_to":"5fc1f717_38161b4b","updated":"2019-03-26 14:21:54.000000000","message":"You wouldn\u0027t need to check the DB, you could use the `container` or `service_metadata` fields to store the \"base_name\" when you create a full backup, and then all incremental backups copy that value.\n\nIn any case, we already have the volume id and the parent\u0027s backup id, so we should be OK and not need to store anything.","commit_id":"07babdb687b519253c8d8a86d0022a7aab69bb44"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"24d4ddc06c265d375fa31bd49f9c16fb556aac76","unresolved":false,"context_lines":[{"line_number":736,"context_line":"            backup_id \u003d backup.parent_id"},{"line_number":737,"context_line":"            last_incr \u003d backup_id"},{"line_number":738,"context_line":"            while(True):"},{"line_number":739,"context_line":"                backup_tmp \u003d self.db.backup_get(self.context, backup_id)"},{"line_number":740,"context_line":"                if backup_tmp.parent_id:"},{"line_number":741,"context_line":"                    backup_id \u003d backup_tmp.parent_id"},{"line_number":742,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":20,"id":"5fc1f717_a3fe65ac","line":739,"in_reply_to":"5fc1f717_3939af3e","updated":"2019-03-26 03:25:23.000000000","message":"Yes Eric,  We still do backups incrementally based on the previous snap, but we need the base name to find the latest snap and then do the incr backup.  Before this patch,  we just use volume_id as base_name,  but now we use volume_id+backup_id, so we need to loop back to find the right base_name so that can find the right snaps.","commit_id":"07babdb687b519253c8d8a86d0022a7aab69bb44"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"42a45da067d07c645946594aca2ab0b74b714122","unresolved":false,"context_lines":[{"line_number":736,"context_line":"            backup_id \u003d backup.parent_id"},{"line_number":737,"context_line":"            last_incr \u003d backup_id"},{"line_number":738,"context_line":"            while(True):"},{"line_number":739,"context_line":"                backup_tmp \u003d self.db.backup_get(self.context, backup_id)"},{"line_number":740,"context_line":"                if backup_tmp.parent_id:"},{"line_number":741,"context_line":"                    backup_id \u003d backup_tmp.parent_id"},{"line_number":742,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":20,"id":"5fc1f717_b59a165f","line":739,"in_reply_to":"5fc1f717_a3fe65ac","updated":"2019-03-26 11:46:31.000000000","message":"You already have the volume_id and the backup_id of the previous snapshot, so why check the DB?\nAlso, why change the base_name?","commit_id":"07babdb687b519253c8d8a86d0022a7aab69bb44"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"b36a3897f56183adf18aa68cc92a90074256dccc","unresolved":false,"context_lines":[{"line_number":736,"context_line":"            backup_id \u003d backup.parent_id"},{"line_number":737,"context_line":"            last_incr \u003d backup_id"},{"line_number":738,"context_line":"            while(True):"},{"line_number":739,"context_line":"                backup_tmp \u003d self.db.backup_get(self.context, backup_id)"},{"line_number":740,"context_line":"                if backup_tmp.parent_id:"},{"line_number":741,"context_line":"                    backup_id \u003d backup_tmp.parent_id"},{"line_number":742,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":20,"id":"5fc1f717_38161b4b","line":739,"in_reply_to":"5fc1f717_b59a165f","updated":"2019-03-26 12:14:58.000000000","message":"Since before there is only one full backup, it\u0027s using the volume_id as base_name. After this full backup,  all backup else is incremental backup whether you specify it or not.  Now we need to make the \u0027--incremental\u0027 really works.  So there will be more than one full backup,  that means we couldn\u0027t only use volume_id to distinguish those full backups. So I use the volume_id + backup_id. That\u0027s also why I need to loop find the original base_image.","commit_id":"07babdb687b519253c8d8a86d0022a7aab69bb44"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f1b921cbe0bb6fd4a393e2194dc5eb22d987d702","unresolved":false,"context_lines":[{"line_number":740,"context_line":"                if backup_tmp.parent_id:"},{"line_number":741,"context_line":"                    backup_id \u003d backup_tmp.parent_id"},{"line_number":742,"context_line":"                    continue"},{"line_number":743,"context_line":"                break"},{"line_number":744,"context_line":"            base_name \u003d self._get_backup_base_name(volume_id, backup_id)"},{"line_number":745,"context_line":"            # Perform incremental rbd backup"},{"line_number":746,"context_line":"            container \u003d backup.container"}],"source_content_type":"text/x-python","patch_set":20,"id":"5fc1f717_195f1342","line":743,"updated":"2019-03-21 14:42:22.000000000","message":"We should not need to access the DB to find out the base name.  That would only be necessary if we found acceptable to  ignore the provided parent_id, which it\u0027s not now.\n\nWhat we need is to do in the incremental is use the volume id and the parent id to find the snapshot we want to use as the base for our incremental.","commit_id":"07babdb687b519253c8d8a86d0022a7aab69bb44"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"24d4ddc06c265d375fa31bd49f9c16fb556aac76","unresolved":false,"context_lines":[{"line_number":740,"context_line":"                if backup_tmp.parent_id:"},{"line_number":741,"context_line":"                    backup_id \u003d backup_tmp.parent_id"},{"line_number":742,"context_line":"                    continue"},{"line_number":743,"context_line":"                break"},{"line_number":744,"context_line":"            base_name \u003d self._get_backup_base_name(volume_id, backup_id)"},{"line_number":745,"context_line":"            # Perform incremental rbd backup"},{"line_number":746,"context_line":"            container \u003d backup.container"}],"source_content_type":"text/x-python","patch_set":20,"id":"5fc1f717_43e8e1b1","line":743,"in_reply_to":"5fc1f717_195f1342","updated":"2019-03-26 03:25:23.000000000","message":"Emm, this cloud be better, we didn\u0027t need to find the base_name first and then find the snapshot. Will see how to change this.","commit_id":"07babdb687b519253c8d8a86d0022a7aab69bb44"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"f1b921cbe0bb6fd4a393e2194dc5eb22d987d702","unresolved":false,"context_lines":[{"line_number":798,"context_line":"        # backup.BACKUP_ID.snap.TIMESTAMP. So we need to extract the"},{"line_number":799,"context_line":"        # backup_id of the parent only from from_snap and set it as"},{"line_number":800,"context_line":"        # parent_id"},{"line_number":801,"context_line":"        if from_snap:"},{"line_number":802,"context_line":"            parent_id \u003d self._get_backup_id_from_snap(from_snap)"},{"line_number":803,"context_line":"            updates \u003d {\u0027parent_id\u0027: parent_id}"},{"line_number":804,"context_line":"        return updates"}],"source_content_type":"text/x-python","patch_set":20,"id":"5fc1f717_94e8aa16","line":801,"updated":"2019-03-21 14:42:22.000000000","message":"We should no longer need from_snap, as it should always be the same as parent_id, if it\u0027s not, somebody removed stuff from the backend, which they shouldn\u0027t, and we should fail and log it accordingly in the incremental method.","commit_id":"07babdb687b519253c8d8a86d0022a7aab69bb44"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"bc88af6cf1ad401654b51c884fcfae6b6671beab","unresolved":false,"context_lines":[{"line_number":798,"context_line":"        # backup.BACKUP_ID.snap.TIMESTAMP. So we need to extract the"},{"line_number":799,"context_line":"        # backup_id of the parent only from from_snap and set it as"},{"line_number":800,"context_line":"        # parent_id"},{"line_number":801,"context_line":"        if from_snap:"},{"line_number":802,"context_line":"            parent_id \u003d self._get_backup_id_from_snap(from_snap)"},{"line_number":803,"context_line":"            updates \u003d {\u0027parent_id\u0027: parent_id}"},{"line_number":804,"context_line":"        return updates"}],"source_content_type":"text/x-python","patch_set":20,"id":"5fc1f717_9e361751","line":801,"in_reply_to":"5fc1f717_23b90c14","updated":"2019-03-26 14:21:54.000000000","message":"The parent_id has already been set by the backup manager, and we should never use a different backup than the one passed by the manager.  This is from when we didn\u0027t support \"--incremental\" and we would do incremental anyway, so we had to instruct the manager to update the DB.","commit_id":"07babdb687b519253c8d8a86d0022a7aab69bb44"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"d72a48c8ee4b3e7cfa1e47e8ef190a45e68e0f2f","unresolved":false,"context_lines":[{"line_number":798,"context_line":"        # backup.BACKUP_ID.snap.TIMESTAMP. So we need to extract the"},{"line_number":799,"context_line":"        # backup_id of the parent only from from_snap and set it as"},{"line_number":800,"context_line":"        # parent_id"},{"line_number":801,"context_line":"        if from_snap:"},{"line_number":802,"context_line":"            parent_id \u003d self._get_backup_id_from_snap(from_snap)"},{"line_number":803,"context_line":"            updates \u003d {\u0027parent_id\u0027: parent_id}"},{"line_number":804,"context_line":"        return updates"}],"source_content_type":"text/x-python","patch_set":20,"id":"5fc1f717_23b90c14","line":801,"in_reply_to":"5fc1f717_94e8aa16","updated":"2019-03-26 13:30:24.000000000","message":"I think we need it, since we also do full backup in #731 in this _backup_rbd method, so if there is full backup, we didn\u0027t need to update the parent_id.","commit_id":"07babdb687b519253c8d8a86d0022a7aab69bb44"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"24cbe1eb91123692ac4b524c9b16efb2e012595a","unresolved":false,"context_lines":[{"line_number":798,"context_line":"        # backup.BACKUP_ID.snap.TIMESTAMP. So we need to extract the"},{"line_number":799,"context_line":"        # backup_id of the parent only from from_snap and set it as"},{"line_number":800,"context_line":"        # parent_id"},{"line_number":801,"context_line":"        if from_snap:"},{"line_number":802,"context_line":"            parent_id \u003d self._get_backup_id_from_snap(from_snap)"},{"line_number":803,"context_line":"            updates \u003d {\u0027parent_id\u0027: parent_id}"},{"line_number":804,"context_line":"        return updates"}],"source_content_type":"text/x-python","patch_set":20,"id":"5fc1f717_29d55d1c","line":801,"in_reply_to":"5fc1f717_9e361751","updated":"2019-03-27 00:56:14.000000000","message":"okay, I got this point. Now we do the full backup really, so no need to update the parent_id.","commit_id":"07babdb687b519253c8d8a86d0022a7aab69bb44"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"85666b2930428c4ba85da4d35ac5afa7411aca59","unresolved":false,"context_lines":[{"line_number":657,"context_line":""},{"line_number":658,"context_line":"        return False"},{"line_number":659,"context_line":""},{"line_number":660,"context_line":"    def _full_rbd_backup(self, container, base_name, length):"},{"line_number":661,"context_line":"        \"\"\"Create the base_image for a full RBD backup.\"\"\""},{"line_number":662,"context_line":"        image_created \u003d True"},{"line_number":663,"context_line":"        from_snap \u003d None"}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_89533f16","line":660,"updated":"2019-03-26 14:47:50.000000000","message":"nit: imo this method should not have to return anything","commit_id":"83ace299b12598da87a469ba45bedb1982169a64"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"8ac95591f12f100302063b9ed8a60fc09c323c54","unresolved":false,"context_lines":[{"line_number":657,"context_line":""},{"line_number":658,"context_line":"        return False"},{"line_number":659,"context_line":""},{"line_number":660,"context_line":"    def _full_rbd_backup(self, container, base_name, length):"},{"line_number":661,"context_line":"        \"\"\"Create the base_image for a full RBD backup.\"\"\""},{"line_number":662,"context_line":"        image_created \u003d True"},{"line_number":663,"context_line":"        from_snap \u003d None"}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_29bebdd4","line":660,"in_reply_to":"5fc1f717_89533f16","updated":"2019-03-27 05:20:41.000000000","message":"Done","commit_id":"83ace299b12598da87a469ba45bedb1982169a64"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"85666b2930428c4ba85da4d35ac5afa7411aca59","unresolved":false,"context_lines":[{"line_number":660,"context_line":"    def _full_rbd_backup(self, container, base_name, length):"},{"line_number":661,"context_line":"        \"\"\"Create the base_image for a full RBD backup.\"\"\""},{"line_number":662,"context_line":"        image_created \u003d True"},{"line_number":663,"context_line":"        from_snap \u003d None"},{"line_number":664,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"},{"line_number":665,"context_line":"                                  container)) as client:"},{"line_number":666,"context_line":"            self._create_base_image(base_name, length, client)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_a9715baa","line":663,"updated":"2019-03-26 14:47:50.000000000","message":"nit: If you still want to return something you can add a comment before doing `return None, True`","commit_id":"83ace299b12598da87a469ba45bedb1982169a64"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"8ac95591f12f100302063b9ed8a60fc09c323c54","unresolved":false,"context_lines":[{"line_number":660,"context_line":"    def _full_rbd_backup(self, container, base_name, length):"},{"line_number":661,"context_line":"        \"\"\"Create the base_image for a full RBD backup.\"\"\""},{"line_number":662,"context_line":"        image_created \u003d True"},{"line_number":663,"context_line":"        from_snap \u003d None"},{"line_number":664,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"},{"line_number":665,"context_line":"                                  container)) as client:"},{"line_number":666,"context_line":"            self._create_base_image(base_name, length, client)"}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_49c1c953","line":663,"in_reply_to":"5fc1f717_a9715baa","updated":"2019-03-27 05:20:41.000000000","message":"Done","commit_id":"83ace299b12598da87a469ba45bedb1982169a64"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"85666b2930428c4ba85da4d35ac5afa7411aca59","unresolved":false,"context_lines":[{"line_number":666,"context_line":"            self._create_base_image(base_name, length, client)"},{"line_number":667,"context_line":"        return from_snap, image_created"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"    def _incremental_rbd_backup(self, container, base_name, length,"},{"line_number":670,"context_line":"                                source_rbd_image, volume_id, last_incr):"},{"line_number":671,"context_line":"        \"\"\"Select the last snap for an incremental backup from an RBD.\"\"\""},{"line_number":672,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_c93707b0","line":669,"updated":"2019-03-26 14:47:50.000000000","message":"-1: This whole method should not be necessary, as we should be able to form the name we return the \"from_snap\" value automatically using the volume id, the parent backup id, and the base name, right?","commit_id":"83ace299b12598da87a469ba45bedb1982169a64"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"8ac95591f12f100302063b9ed8a60fc09c323c54","unresolved":false,"context_lines":[{"line_number":666,"context_line":"            self._create_base_image(base_name, length, client)"},{"line_number":667,"context_line":"        return from_snap, image_created"},{"line_number":668,"context_line":""},{"line_number":669,"context_line":"    def _incremental_rbd_backup(self, container, base_name, length,"},{"line_number":670,"context_line":"                                source_rbd_image, volume_id, last_incr):"},{"line_number":671,"context_line":"        \"\"\"Select the last snap for an incremental backup from an RBD.\"\"\""},{"line_number":672,"context_line":""}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_69c7e565","line":669,"in_reply_to":"5fc1f717_c93707b0","updated":"2019-03-27 05:20:41.000000000","message":"since the from_snap value also has the timestamp. So couldn\u0027t automatically just using the volume_id, parent_id and base_name. see into the _get_most_recent_snap method.","commit_id":"83ace299b12598da87a469ba45bedb1982169a64"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"85666b2930428c4ba85da4d35ac5afa7411aca59","unresolved":false,"context_lines":[{"line_number":720,"context_line":"            from_snap, image_created \u003d self._full_rbd_backup(backup.container,"},{"line_number":721,"context_line":"                                                             base_name,"},{"line_number":722,"context_line":"                                                             length)"},{"line_number":723,"context_line":"        else:"},{"line_number":724,"context_line":"            # find the first full backup name as the base_name"},{"line_number":725,"context_line":"            backup_id \u003d backup.parent_id"},{"line_number":726,"context_line":"            last_incr \u003d backup_id"}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_69f7f38c","line":723,"updated":"2019-03-26 14:47:50.000000000","message":"Where are we taking into account that non RBD volumes cannot be incremental, and should fail if we try to do it?","commit_id":"83ace299b12598da87a469ba45bedb1982169a64"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"8ac95591f12f100302063b9ed8a60fc09c323c54","unresolved":false,"context_lines":[{"line_number":720,"context_line":"            from_snap, image_created \u003d self._full_rbd_backup(backup.container,"},{"line_number":721,"context_line":"                                                             base_name,"},{"line_number":722,"context_line":"                                                             length)"},{"line_number":723,"context_line":"        else:"},{"line_number":724,"context_line":"            # find the first full backup name as the base_name"},{"line_number":725,"context_line":"            backup_id \u003d backup.parent_id"},{"line_number":726,"context_line":"            last_incr \u003d backup_id"}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_09850107","line":723,"in_reply_to":"5fc1f717_69f7f38c","updated":"2019-03-27 05:20:41.000000000","message":"In #988 we have done this,  non RBD volumes only can do full backup even there is \u0027--inremental\u003dTrue\u0027.","commit_id":"83ace299b12598da87a469ba45bedb1982169a64"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"85666b2930428c4ba85da4d35ac5afa7411aca59","unresolved":false,"context_lines":[{"line_number":725,"context_line":"            backup_id \u003d backup.parent_id"},{"line_number":726,"context_line":"            last_incr \u003d backup_id"},{"line_number":727,"context_line":"            while (True):"},{"line_number":728,"context_line":"                backup_tmp \u003d self.db.backup_get(self.context, backup_id)"},{"line_number":729,"context_line":"                if backup_tmp.parent_id:"},{"line_number":730,"context_line":"                    backup_id \u003d backup_tmp.parent_id"},{"line_number":731,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_6948532c","line":728,"updated":"2019-03-26 14:47:50.000000000","message":"-1: No using the DB directly.  We should just store the base name in the \"service_metadata\" when doing a full backup and copy it when doing incremental backups.","commit_id":"83ace299b12598da87a469ba45bedb1982169a64"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"8ac95591f12f100302063b9ed8a60fc09c323c54","unresolved":false,"context_lines":[{"line_number":725,"context_line":"            backup_id \u003d backup.parent_id"},{"line_number":726,"context_line":"            last_incr \u003d backup_id"},{"line_number":727,"context_line":"            while (True):"},{"line_number":728,"context_line":"                backup_tmp \u003d self.db.backup_get(self.context, backup_id)"},{"line_number":729,"context_line":"                if backup_tmp.parent_id:"},{"line_number":730,"context_line":"                    backup_id \u003d backup_tmp.parent_id"},{"line_number":731,"context_line":"                    continue"}],"source_content_type":"text/x-python","patch_set":21,"id":"5fc1f717_a97f0d13","line":728,"in_reply_to":"5fc1f717_6948532c","updated":"2019-03-27 05:20:41.000000000","message":"Done","commit_id":"83ace299b12598da87a469ba45bedb1982169a64"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c1926b92ac80faa5d1de30faa96500c8be362b64","unresolved":false,"context_lines":[{"line_number":698,"context_line":"                        self.rbd.Image(client.ioctx, base_name,"},{"line_number":699,"context_line":"                                       read_only\u003dTrue))"},{"line_number":700,"context_line":"                    try:"},{"line_number":701,"context_line":"                        snap \u003d self._get_backup_snap_name(base_rbd,"},{"line_number":702,"context_line":"                                                          base_name, last_incr)"},{"line_number":703,"context_line":"                        if snap:"},{"line_number":704,"context_line":"                            from_snap \u003d snap"}],"source_content_type":"text/x-python","patch_set":25,"id":"5fc1f717_5ad4f21f","line":701,"updated":"2019-03-29 10:14:51.000000000","message":"If we use the `created_at` field converted to timestamp instead of `time.time()`, then we can use the parent\u0027s created_at field together with the other info we have to know the exact name.\n\nFor backward compatibility with existing backups we would have to revert to the _get_backup_snap_name if the other name doesn\u0027t exist.\n\nThis would be an interested improvement, but definitely out of the scope of what this patch is aiming to fix.","commit_id":"f95ccf91a46636e76ac883785e5b40be4db26c2f"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"aa3eae8cb3b56e0d36f5b1d6b7a72e7b9a3dc5b0","unresolved":false,"context_lines":[{"line_number":698,"context_line":"                        self.rbd.Image(client.ioctx, base_name,"},{"line_number":699,"context_line":"                                       read_only\u003dTrue))"},{"line_number":700,"context_line":"                    try:"},{"line_number":701,"context_line":"                        snap \u003d self._get_backup_snap_name(base_rbd,"},{"line_number":702,"context_line":"                                                          base_name, last_incr)"},{"line_number":703,"context_line":"                        if snap:"},{"line_number":704,"context_line":"                            from_snap \u003d snap"}],"source_content_type":"text/x-python","patch_set":25,"id":"5fc1f717_aa97b6eb","line":701,"in_reply_to":"5fc1f717_5ad4f21f","updated":"2019-03-30 02:51:48.000000000","message":"Agree,  this would be a good improvement. But I also worried about even if we can get the exact name of snap, we still need to find all the snap from rbd and see if the name is really existing in ceph backend, in case there are some exceptions happened.","commit_id":"f95ccf91a46636e76ac883785e5b40be4db26c2f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c1926b92ac80faa5d1de30faa96500c8be362b64","unresolved":false,"context_lines":[{"line_number":727,"context_line":"                                                             length)"},{"line_number":728,"context_line":"        else:"},{"line_number":729,"context_line":"            # find the base name from the parent backup\u0027s service_metadata"},{"line_number":730,"context_line":"            parent_backup \u003d self.db.backup_get(self.context, backup.parent_id)"},{"line_number":731,"context_line":"            base_name \u003d parent_backup.service_metadata"},{"line_number":732,"context_line":"            # Perform incremental rbd backup"},{"line_number":733,"context_line":"            container \u003d backup.container"}],"source_content_type":"text/x-python","patch_set":25,"id":"5fc1f717_7aa52eb4","line":730,"updated":"2019-03-29 10:14:51.000000000","message":"-1: No DB access on drivers.  You have to make the manager pass that information.\n\nYou have 2 ways of doing this:\n\n- Passing a new parameter to the `backup` method with the parent Backup OVO.\n\n- Adding a `parent` field on the Backup OVO and setting it  when doing the creation of an incremental backup.\n\nThe second approach is the right one, and it\u0027s trivial, since you would just need to set it on the backup API:\n\n   backup.parent \u003d latest_backup        \n   self.backup_rpcapi.create_backup(context, backup)\n\nThat way you can access the service_metadata with:\n\n   base_name \u003d backup.parent.service_metadata","commit_id":"f95ccf91a46636e76ac883785e5b40be4db26c2f"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"f4d9bdd3ea4167dd95a5b91cc3d3593db338d516","unresolved":false,"context_lines":[{"line_number":727,"context_line":"                                                             length)"},{"line_number":728,"context_line":"        else:"},{"line_number":729,"context_line":"            # find the base name from the parent backup\u0027s service_metadata"},{"line_number":730,"context_line":"            parent_backup \u003d self.db.backup_get(self.context, backup.parent_id)"},{"line_number":731,"context_line":"            base_name \u003d parent_backup.service_metadata"},{"line_number":732,"context_line":"            # Perform incremental rbd backup"},{"line_number":733,"context_line":"            container \u003d backup.container"}],"source_content_type":"text/x-python","patch_set":25,"id":"5fc1f717_65b4a1d2","line":730,"in_reply_to":"5fc1f717_7aa52eb4","updated":"2019-03-30 03:22:21.000000000","message":"I think there is a more better way is we just need to give the latest_backup.service_metadata to backup.service_metadata in api. and then access the service_metadata with\nbase_name \u003dbackup.service_metadata.\n\nSo this will no need to change the backup OVO.  Because I find it\u0027s a little heavy change to backup OVA when setting a new field \u0027parent\u0027.","commit_id":"f95ccf91a46636e76ac883785e5b40be4db26c2f"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"aa3eae8cb3b56e0d36f5b1d6b7a72e7b9a3dc5b0","unresolved":false,"context_lines":[{"line_number":727,"context_line":"                                                             length)"},{"line_number":728,"context_line":"        else:"},{"line_number":729,"context_line":"            # find the base name from the parent backup\u0027s service_metadata"},{"line_number":730,"context_line":"            parent_backup \u003d self.db.backup_get(self.context, backup.parent_id)"},{"line_number":731,"context_line":"            base_name \u003d parent_backup.service_metadata"},{"line_number":732,"context_line":"            # Perform incremental rbd backup"},{"line_number":733,"context_line":"            container \u003d backup.container"}],"source_content_type":"text/x-python","patch_set":25,"id":"5fc1f717_ea8cded8","line":730,"in_reply_to":"5fc1f717_7aa52eb4","updated":"2019-03-30 02:51:48.000000000","message":"okay","commit_id":"f95ccf91a46636e76ac883785e5b40be4db26c2f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c1926b92ac80faa5d1de30faa96500c8be362b64","unresolved":false,"context_lines":[{"line_number":740,"context_line":"                                                                    volume_id,"},{"line_number":741,"context_line":"                                                                    last_incr)"},{"line_number":742,"context_line":"        backup.service_metadata \u003d base_name"},{"line_number":743,"context_line":"        backup.save()"},{"line_number":744,"context_line":""},{"line_number":745,"context_line":"        LOG.debug(\"Using --from-snap \u0027%(snap)s\u0027 for incremental backup of \""},{"line_number":746,"context_line":"                  \"volume %(volume)s.\","}],"source_content_type":"text/x-python","patch_set":25,"id":"5fc1f717_3add6640","line":743,"updated":"2019-03-29 10:14:51.000000000","message":"-1: No direct DB access.  Just return the data you want to get updated:\n\n   return {\u0027service_metadata\u0027: base_name}","commit_id":"f95ccf91a46636e76ac883785e5b40be4db26c2f"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"aa3eae8cb3b56e0d36f5b1d6b7a72e7b9a3dc5b0","unresolved":false,"context_lines":[{"line_number":740,"context_line":"                                                                    volume_id,"},{"line_number":741,"context_line":"                                                                    last_incr)"},{"line_number":742,"context_line":"        backup.service_metadata \u003d base_name"},{"line_number":743,"context_line":"        backup.save()"},{"line_number":744,"context_line":""},{"line_number":745,"context_line":"        LOG.debug(\"Using --from-snap \u0027%(snap)s\u0027 for incremental backup of \""},{"line_number":746,"context_line":"                  \"volume %(volume)s.\","}],"source_content_type":"text/x-python","patch_set":25,"id":"5fc1f717_aac016e2","line":743,"in_reply_to":"5fc1f717_3add6640","updated":"2019-03-30 02:51:48.000000000","message":"Done","commit_id":"f95ccf91a46636e76ac883785e5b40be4db26c2f"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"fff273846a299a992bf3e28f5955d151046cec09","unresolved":false,"context_lines":[{"line_number":984,"context_line":"            try:"},{"line_number":985,"context_line":"                updates \u003d self._backup_rbd(backup, volume_file, volume.name,"},{"line_number":986,"context_line":"                                           length)"},{"line_number":987,"context_line":"            except exception.BackupRBDOperationFailed:"},{"line_number":988,"context_line":"                LOG.debug(\"Forcing full backup of volume %s.\", volume.id)"},{"line_number":989,"context_line":"                do_full_backup \u003d True"},{"line_number":990,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_340304c1","line":987,"range":{"start_line":987,"start_character":12,"end_line":987,"end_character":54},"updated":"2019-04-02 15:04:27.000000000","message":"This indicates that the correct description above is still \"attempt incremental backup\".\n\nOr does this flow need to be updated now?","commit_id":"f98373ce78afc8e22ab21331433ca21fb3efebf3"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"f1d8039d79a8faccb95d3418b5970b8f4463650c","unresolved":false,"context_lines":[{"line_number":984,"context_line":"            try:"},{"line_number":985,"context_line":"                updates \u003d self._backup_rbd(backup, volume_file, volume.name,"},{"line_number":986,"context_line":"                                           length)"},{"line_number":987,"context_line":"            except exception.BackupRBDOperationFailed:"},{"line_number":988,"context_line":"                LOG.debug(\"Forcing full backup of volume %s.\", volume.id)"},{"line_number":989,"context_line":"                do_full_backup \u003d True"},{"line_number":990,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":28,"id":"5fc1f717_826b83ef","line":987,"range":{"start_line":987,"start_character":12,"end_line":987,"end_character":54},"in_reply_to":"5fc1f717_340304c1","updated":"2019-04-03 13:20:33.000000000","message":"I think we can update this flow, if _backup_rbd failed, we delete this backup.","commit_id":"f98373ce78afc8e22ab21331433ca21fb3efebf3"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"53ecae2a09955bf92ac44e14c8dcaa4970c53401","unresolved":false,"context_lines":[{"line_number":682,"context_line":"                    \"backup.\") % base_name)"},{"line_number":683,"context_line":"                LOG.error(msg)"},{"line_number":684,"context_line":"                raise exception.BackupOperationError(msg)"},{"line_number":685,"context_line":"            else:"},{"line_number":686,"context_line":"                # If a from_snap is defined and is present in the source volume"},{"line_number":687,"context_line":"                # image but does not exist in the backup base then we look down"},{"line_number":688,"context_line":"                # the list of source volume snapshots and find the latest one"}],"source_content_type":"text/x-python","patch_set":29,"id":"5fc1f717_9e1505e8","line":685,"updated":"2019-04-04 11:55:52.000000000","message":"nit: No need for this else, the if section above will never continue here. Removing it will decrease indentation.","commit_id":"ee408f49bc9fcc7ba89d1d2506b4d7328b5cc76b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"53ecae2a09955bf92ac44e14c8dcaa4970c53401","unresolved":false,"context_lines":[{"line_number":689,"context_line":"                # for which a backup snapshot exist in the backup base. Until"},{"line_number":690,"context_line":"                # that snapshot is reached, we delete all the other snapshots"},{"line_number":691,"context_line":"                # for which backup snapshot does not exist."},{"line_number":692,"context_line":"                from_snap \u003d self._get_most_recent_snap(source_rbd_image,"},{"line_number":693,"context_line":"                                                       base_name, client)"},{"line_number":694,"context_line":""},{"line_number":695,"context_line":"                backup_id \u003d self._get_backup_id_from_snap(from_snap)"}],"source_content_type":"text/x-python","patch_set":29,"id":"5fc1f717_3eb731db","line":692,"updated":"2019-04-04 11:55:52.000000000","message":"-1: Now that we can have multiple full backups this could be destroying snapshots on the source volume that belong to incremental backups from other full backups!","commit_id":"ee408f49bc9fcc7ba89d1d2506b4d7328b5cc76b"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"23159b708c261f51b8f6189e887ca40a2f6d22f4","unresolved":false,"context_lines":[{"line_number":689,"context_line":"                # for which a backup snapshot exist in the backup base. Until"},{"line_number":690,"context_line":"                # that snapshot is reached, we delete all the other snapshots"},{"line_number":691,"context_line":"                # for which backup snapshot does not exist."},{"line_number":692,"context_line":"                from_snap \u003d self._get_most_recent_snap(source_rbd_image,"},{"line_number":693,"context_line":"                                                       base_name, client)"},{"line_number":694,"context_line":""},{"line_number":695,"context_line":"                backup_id \u003d self._get_backup_id_from_snap(from_snap)"}],"source_content_type":"text/x-python","patch_set":29,"id":"5fc1f717_1d911147","line":692,"in_reply_to":"5fc1f717_3eb731db","updated":"2019-04-04 19:36:35.000000000","message":"In the future, we should refactor \"_get_most_recent_snap\" to take into account that we may now have multiple full backups.\n\nFor this patch, we can ignore the refactoring and just search for the snapshot we want (the one from the parent) and if it\u0027s not there, we fail.","commit_id":"ee408f49bc9fcc7ba89d1d2506b4d7328b5cc76b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"53ecae2a09955bf92ac44e14c8dcaa4970c53401","unresolved":false,"context_lines":[{"line_number":692,"context_line":"                from_snap \u003d self._get_most_recent_snap(source_rbd_image,"},{"line_number":693,"context_line":"                                                       base_name, client)"},{"line_number":694,"context_line":""},{"line_number":695,"context_line":"                backup_id \u003d self._get_backup_id_from_snap(from_snap)"},{"line_number":696,"context_line":"                if not backup_id \u003d\u003d last_incr:"},{"line_number":697,"context_line":"                    base_rbd \u003d eventlet.tpool.Proxy("},{"line_number":698,"context_line":"                        self.rbd.Image(client.ioctx, base_name,"}],"source_content_type":"text/x-python","patch_set":29,"id":"5fc1f717_3e61713a","line":695,"updated":"2019-04-04 11:55:52.000000000","message":"-1: I believe this backup_id could be from a different base_name","commit_id":"ee408f49bc9fcc7ba89d1d2506b4d7328b5cc76b"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"23159b708c261f51b8f6189e887ca40a2f6d22f4","unresolved":false,"context_lines":[{"line_number":692,"context_line":"                from_snap \u003d self._get_most_recent_snap(source_rbd_image,"},{"line_number":693,"context_line":"                                                       base_name, client)"},{"line_number":694,"context_line":""},{"line_number":695,"context_line":"                backup_id \u003d self._get_backup_id_from_snap(from_snap)"},{"line_number":696,"context_line":"                if not backup_id \u003d\u003d last_incr:"},{"line_number":697,"context_line":"                    base_rbd \u003d eventlet.tpool.Proxy("},{"line_number":698,"context_line":"                        self.rbd.Image(client.ioctx, base_name,"}],"source_content_type":"text/x-python","patch_set":29,"id":"5fc1f717_976ba31d","line":695,"in_reply_to":"5fc1f717_3e61713a","updated":"2019-04-04 19:36:35.000000000","message":"Indeed. We don\u0027t need this anymore since we are not using _get_most_recent_snap.","commit_id":"ee408f49bc9fcc7ba89d1d2506b4d7328b5cc76b"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"fe65b563f269efaaa7a6d6f39d472da325925e23","unresolved":false,"context_lines":[{"line_number":727,"context_line":"                                                             length)"},{"line_number":728,"context_line":"        else:"},{"line_number":729,"context_line":"            # find the base name from the parent backup\u0027s service_metadata"},{"line_number":730,"context_line":"            base_name \u003d backup.service_metadata"},{"line_number":731,"context_line":"            # Perform incremental rbd backup"},{"line_number":732,"context_line":"            container \u003d backup.container"},{"line_number":733,"context_line":"            rbd_img \u003d source_rbd_image"}],"source_content_type":"text/x-python","patch_set":29,"id":"5fc1f717_0281cac3","line":730,"range":{"start_line":730,"start_character":11,"end_line":730,"end_character":47},"updated":"2019-04-03 19:44:54.000000000","message":"I\u0027m not sure about this. Maybe @Gorka can clarify if we should add a `parent` field on the Backup OVO and setting it when doing the creation of an incremental backup.","commit_id":"ee408f49bc9fcc7ba89d1d2506b4d7328b5cc76b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"53ecae2a09955bf92ac44e14c8dcaa4970c53401","unresolved":false,"context_lines":[{"line_number":727,"context_line":"                                                             length)"},{"line_number":728,"context_line":"        else:"},{"line_number":729,"context_line":"            # find the base name from the parent backup\u0027s service_metadata"},{"line_number":730,"context_line":"            base_name \u003d backup.service_metadata"},{"line_number":731,"context_line":"            # Perform incremental rbd backup"},{"line_number":732,"context_line":"            container \u003d backup.container"},{"line_number":733,"context_line":"            rbd_img \u003d source_rbd_image"}],"source_content_type":"text/x-python","patch_set":29,"id":"5fc1f717_5e623daf","line":730,"range":{"start_line":730,"start_character":11,"end_line":730,"end_character":47},"in_reply_to":"5fc1f717_0281cac3","updated":"2019-04-04 11:55:52.000000000","message":"This will work, but I think it feels wrong to inherit this field for other backup drivers that use this field for something else.\n\nI still think that setting the parent at the API layer is the better approach.\n\nYou can add the parent field to the Backup OVO and not add any load or save method for the field.  Just add a comment saying that this field is only set when the OVO is created.  That way you can simplify the coding required for this fix.","commit_id":"ee408f49bc9fcc7ba89d1d2506b4d7328b5cc76b"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"23159b708c261f51b8f6189e887ca40a2f6d22f4","unresolved":false,"context_lines":[{"line_number":727,"context_line":"                                                             length)"},{"line_number":728,"context_line":"        else:"},{"line_number":729,"context_line":"            # find the base name from the parent backup\u0027s service_metadata"},{"line_number":730,"context_line":"            base_name \u003d backup.service_metadata"},{"line_number":731,"context_line":"            # Perform incremental rbd backup"},{"line_number":732,"context_line":"            container \u003d backup.container"},{"line_number":733,"context_line":"            rbd_img \u003d source_rbd_image"}],"source_content_type":"text/x-python","patch_set":29,"id":"5fc1f717_fdb685dc","line":730,"range":{"start_line":730,"start_character":11,"end_line":730,"end_character":47},"in_reply_to":"5fc1f717_5e623daf","updated":"2019-04-04 19:36:35.000000000","message":"+1","commit_id":"ee408f49bc9fcc7ba89d1d2506b4d7328b5cc76b"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"23159b708c261f51b8f6189e887ca40a2f6d22f4","unresolved":false,"context_lines":[{"line_number":784,"context_line":""},{"line_number":785,"context_line":"        return {\u0027service_metadata\u0027: base_name}"},{"line_number":786,"context_line":""},{"line_number":787,"context_line":"    def _get_backup_id_from_snap(self, from_snap):"},{"line_number":788,"context_line":"        if not from_snap:"},{"line_number":789,"context_line":"            LOG.debug(\"Backup base \u0027%s\u0027 has no snapshots\", from_snap)"},{"line_number":790,"context_line":"            return None"},{"line_number":791,"context_line":"        parent_id \u003d from_snap.split(\u0027.\u0027)"},{"line_number":792,"context_line":"        return parent_id[1]"},{"line_number":793,"context_line":""},{"line_number":794,"context_line":"    def _file_is_rbd(self, volume_file):"},{"line_number":795,"context_line":"        \"\"\"Returns True if the volume_file is actually an RBD image.\"\"\""}],"source_content_type":"text/x-python","patch_set":29,"id":"5fc1f717_b7381f3b","line":792,"range":{"start_line":787,"start_character":1,"end_line":792,"end_character":27},"updated":"2019-04-04 19:36:35.000000000","message":"We don\u0027t need this method anymore.","commit_id":"ee408f49bc9fcc7ba89d1d2506b4d7328b5cc76b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ac0e3b551e44b2f214af5cc4b538afae95f058a0","unresolved":false,"context_lines":[{"line_number":669,"context_line":""},{"line_number":670,"context_line":"    def _incremental_rbd_backup(self, container, base_name, length,"},{"line_number":671,"context_line":"                                source_rbd_image, volume_id, last_incr):"},{"line_number":672,"context_line":"        \"\"\"Select the last snap for an incremental backup from an RBD.\"\"\""},{"line_number":673,"context_line":""},{"line_number":674,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"},{"line_number":675,"context_line":"                                  container)) as client:"}],"source_content_type":"text/x-python","patch_set":30,"id":"5fc1f717_945ab380","line":672,"updated":"2019-04-05 10:19:22.000000000","message":"nit: We should probably LOG.debug the info we are using for the incremental backup.","commit_id":"a6308d42a32b5b92bca33296f55ac749d60a1d95"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ac0e3b551e44b2f214af5cc4b538afae95f058a0","unresolved":false,"context_lines":[{"line_number":673,"context_line":""},{"line_number":674,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"},{"line_number":675,"context_line":"                                  container)) as client:"},{"line_number":676,"context_line":"            # If can\u0027t find the base_name, that means the incremental backup"},{"line_number":677,"context_line":"            # is failed, so raise the exception."},{"line_number":678,"context_line":"            if base_name not in eventlet.tpool.Proxy(self.rbd.RBD()).list("},{"line_number":679,"context_line":"                    ioctx\u003dclient.ioctx):"},{"line_number":680,"context_line":"                msg \u003d (_("}],"source_content_type":"text/x-python","patch_set":30,"id":"5fc1f717_f415379c","line":677,"range":{"start_line":676,"start_character":55,"end_line":677,"end_character":24},"updated":"2019-04-05 10:19:22.000000000","message":"nit: suggestion: \"that means we can\u0027t do the incremental backup, so raise the exception.\"","commit_id":"a6308d42a32b5b92bca33296f55ac749d60a1d95"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ac0e3b551e44b2f214af5cc4b538afae95f058a0","unresolved":false,"context_lines":[{"line_number":692,"context_line":"                                                       last_incr)"},{"line_number":693,"context_line":"                if from_snap is None:"},{"line_number":694,"context_line":"                    msg \u003d (_("},{"line_number":695,"context_line":"                        \"Can\u0027t find snap name from parent %s.\")"},{"line_number":696,"context_line":"                        % last_incr)"},{"line_number":697,"context_line":"                    LOG.error(msg)"},{"line_number":698,"context_line":"                    raise exception.BackupOperationError(msg)"}],"source_content_type":"text/x-python","patch_set":30,"id":"5fc1f717_d4603b38","line":695,"updated":"2019-04-05 10:19:22.000000000","message":"?: Maybe we should log the base_name as well","commit_id":"a6308d42a32b5b92bca33296f55ac749d60a1d95"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ac0e3b551e44b2f214af5cc4b538afae95f058a0","unresolved":false,"context_lines":[{"line_number":695,"context_line":"                        \"Can\u0027t find snap name from parent %s.\")"},{"line_number":696,"context_line":"                        % last_incr)"},{"line_number":697,"context_line":"                    LOG.error(msg)"},{"line_number":698,"context_line":"                    raise exception.BackupOperationError(msg)"},{"line_number":699,"context_line":"            finally:"},{"line_number":700,"context_line":"                base_rbd.close()"},{"line_number":701,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"5fc1f717_34497fb6","line":698,"range":{"start_line":698,"start_character":36,"end_line":698,"end_character":56},"updated":"2019-04-05 10:19:22.000000000","message":"?: Why are we raising BackupOperationError instead of BackupRBDOperationError?","commit_id":"a6308d42a32b5b92bca33296f55ac749d60a1d95"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ac0e3b551e44b2f214af5cc4b538afae95f058a0","unresolved":false,"context_lines":[{"line_number":712,"context_line":""},{"line_number":713,"context_line":"        # If backup.parent_id is None, that means to create a full backup"},{"line_number":714,"context_line":"        # for rbd. This will fix the bug that can\u0027t create a full backup"},{"line_number":715,"context_line":"        # when incremental\u003dFalse."},{"line_number":716,"context_line":"        if backup.parent_id is None:"},{"line_number":717,"context_line":"            base_name \u003d self._get_backup_base_name(volume_id, backup.id)"},{"line_number":718,"context_line":"            # Perform full rbd backup"}],"source_content_type":"text/x-python","patch_set":30,"id":"5fc1f717_b401ef6c","line":715,"updated":"2019-04-05 10:19:22.000000000","message":"nit: No need to mention that this is fixing a bug.","commit_id":"a6308d42a32b5b92bca33296f55ac749d60a1d95"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ac0e3b551e44b2f214af5cc4b538afae95f058a0","unresolved":false,"context_lines":[{"line_number":893,"context_line":"        LOG.debug(\"Found snapshot \u0027%s\u0027\", snaps[0])"},{"line_number":894,"context_line":"        return snaps[0]"},{"line_number":895,"context_line":""},{"line_number":896,"context_line":"    def _get_most_recent_snap(self, rbd_image, base_name, client):"},{"line_number":897,"context_line":"        \"\"\"Get the most recent backup snapshot of the provided image."},{"line_number":898,"context_line":""},{"line_number":899,"context_line":"        Returns name of most recent backup snapshot or None if there are no"}],"source_content_type":"text/x-python","patch_set":30,"id":"5fc1f717_341bbfca","line":896,"updated":"2019-04-05 10:19:22.000000000","message":"-1: We can remove this method since it is no longer used","commit_id":"a6308d42a32b5b92bca33296f55ac749d60a1d95"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ac0e3b551e44b2f214af5cc4b538afae95f058a0","unresolved":false,"context_lines":[{"line_number":967,"context_line":"        if self._file_is_rbd(volume_file):"},{"line_number":968,"context_line":"            # If volume an RBD, attempt incremental or full backup."},{"line_number":969,"context_line":"            LOG.debug(\"Volume file is RBD: attempting incremental or full \""},{"line_number":970,"context_line":"                      \"backup.\")"},{"line_number":971,"context_line":"            try:"},{"line_number":972,"context_line":"                updates \u003d self._backup_rbd(backup, volume_file, volume.name,"},{"line_number":973,"context_line":"                                           length)"}],"source_content_type":"text/x-python","patch_set":30,"id":"5fc1f717_54f8cb7a","line":970,"updated":"2019-04-05 10:19:22.000000000","message":"nit: I would just say that it\u0027s attempting optimized backup, not mention it is full or incremental.","commit_id":"a6308d42a32b5b92bca33296f55ac749d60a1d95"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ac0e3b551e44b2f214af5cc4b538afae95f058a0","unresolved":false,"context_lines":[{"line_number":1201,"context_line":"        \"\"\""},{"line_number":1202,"context_line":"        length \u003d int(volume.size) * units.Gi"},{"line_number":1203,"context_line":""},{"line_number":1204,"context_line":"        base_name \u003d self._get_backup_base_name(backup.volume_id,"},{"line_number":1205,"context_line":"                                               diff_format\u003dTrue)"},{"line_number":1206,"context_line":""},{"line_number":1207,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient("}],"source_content_type":"text/x-python","patch_set":30,"id":"5fc1f717_b46ecffd","line":1204,"updated":"2019-04-05 10:19:22.000000000","message":"-1: Does the restore actually work?  I mean, this is getting the wrong base_name, as it uses volume-%s.base format, and we should be checking the service_metadata first, and then if it\u0027s None or empty use this method.\n\nPlease also check other calls to _get_backup_base_name to ensure it takes into account that we now use base names in the form of volume-%s-backup-%s.","commit_id":"a6308d42a32b5b92bca33296f55ac749d60a1d95"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b8657c35258f90fbd8e6cd21085d3b0b094d6b6b","unresolved":false,"context_lines":[{"line_number":323,"context_line":"        \"\"\""},{"line_number":324,"context_line":"        # Ensure no unicode"},{"line_number":325,"context_line":"        if diff_format:"},{"line_number":326,"context_line":"            if backup_id is None:"},{"line_number":327,"context_line":"                return utils.convert_str(\"volume-%s.backup.base\" % volume_id)"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":31,"id":"5fc1f717_12d75cf0","line":326,"updated":"2019-04-10 09:59:02.000000000","message":"-1: This method will return None if diff_format is True and backup_id is not None.\n\nIs that really what we want?  If it is we need a comment explaining why","commit_id":"cda3cd1cf9b8fc8c22b68b1e35530990b0b5b87b"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"17a80cd3e8a7af918a4e6d494f04b14397e0a2e2","unresolved":false,"context_lines":[{"line_number":323,"context_line":"        \"\"\""},{"line_number":324,"context_line":"        # Ensure no unicode"},{"line_number":325,"context_line":"        if diff_format:"},{"line_number":326,"context_line":"            if backup_id is None:"},{"line_number":327,"context_line":"                return utils.convert_str(\"volume-%s.backup.base\" % volume_id)"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":31,"id":"3fce034c_c5e69217","line":326,"in_reply_to":"3fce034c_33924ef8","updated":"2019-04-13 03:31:04.000000000","message":"Make senses, just need to adjust the volume_id and backup_id to know whether is diff_format\u003dTrue or not.","commit_id":"cda3cd1cf9b8fc8c22b68b1e35530990b0b5b87b"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"dc55a5f08b44236c76e5c9d6800cdd36938cd353","unresolved":false,"context_lines":[{"line_number":323,"context_line":"        \"\"\""},{"line_number":324,"context_line":"        # Ensure no unicode"},{"line_number":325,"context_line":"        if diff_format:"},{"line_number":326,"context_line":"            if backup_id is None:"},{"line_number":327,"context_line":"                return utils.convert_str(\"volume-%s.backup.base\" % volume_id)"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":31,"id":"5fc1f717_f892af4a","line":326,"in_reply_to":"5fc1f717_12d75cf0","updated":"2019-04-10 11:51:00.000000000","message":"I think if diff_format is True and also pass the backup_id, that we can give an exception to tell the developer how to use this method correctly.","commit_id":"cda3cd1cf9b8fc8c22b68b1e35530990b0b5b87b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b914280e728ce1112431d4288bfc06ab3db1d823","unresolved":false,"context_lines":[{"line_number":323,"context_line":"        \"\"\""},{"line_number":324,"context_line":"        # Ensure no unicode"},{"line_number":325,"context_line":"        if diff_format:"},{"line_number":326,"context_line":"            if backup_id is None:"},{"line_number":327,"context_line":"                return utils.convert_str(\"volume-%s.backup.base\" % volume_id)"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":31,"id":"3fce034c_33924ef8","line":326,"in_reply_to":"5fc1f717_7113355d","updated":"2019-04-12 10:00:56.000000000","message":"If we can never receive the backup_id and diff_format both to True, then we don\u0027t need the diff_format parameter.\n\nJust with the backup_id is enough, if it\u0027s None we return L327, and if it\u0027s not we return L333, right?","commit_id":"cda3cd1cf9b8fc8c22b68b1e35530990b0b5b87b"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"22c3226bf17fbf035a67d947201827ceb696d2d8","unresolved":false,"context_lines":[{"line_number":323,"context_line":"        \"\"\""},{"line_number":324,"context_line":"        # Ensure no unicode"},{"line_number":325,"context_line":"        if diff_format:"},{"line_number":326,"context_line":"            if backup_id is None:"},{"line_number":327,"context_line":"                return utils.convert_str(\"volume-%s.backup.base\" % volume_id)"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":31,"id":"5fc1f717_7113355d","line":326,"in_reply_to":"5fc1f717_f892af4a","updated":"2019-04-10 17:04:53.000000000","message":"Sorry, this part is a WIP. Because if we have the backup_id, we should use the service_metadata of the parent.","commit_id":"cda3cd1cf9b8fc8c22b68b1e35530990b0b5b87b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b8657c35258f90fbd8e6cd21085d3b0b094d6b6b","unresolved":false,"context_lines":[{"line_number":673,"context_line":"                                source_rbd_image, volume_id, last_incr):"},{"line_number":674,"context_line":"        \"\"\"Select the last snapshot for a RBD incremental backup.\"\"\""},{"line_number":675,"context_line":""},{"line_number":676,"context_line":"        LOG.debug \u003d (\"Trying to perform an incremental backup with container: \""},{"line_number":677,"context_line":"                     \"%(container)s, base_name: %(base)s, source RBD image: \""},{"line_number":678,"context_line":"                     \"%(source)s, volume ID %(volume)s and last incremental \""},{"line_number":679,"context_line":"                     \"backup ID: %(incr)s.\","}],"source_content_type":"text/x-python","patch_set":31,"id":"5fc1f717_d22b14e8","line":676,"updated":"2019-04-10 09:59:02.000000000","message":"-1: It\u0027s not an assignment, it\u0027s a method call: LOG.debug(","commit_id":"cda3cd1cf9b8fc8c22b68b1e35530990b0b5b87b"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"dc55a5f08b44236c76e5c9d6800cdd36938cd353","unresolved":false,"context_lines":[{"line_number":673,"context_line":"                                source_rbd_image, volume_id, last_incr):"},{"line_number":674,"context_line":"        \"\"\"Select the last snapshot for a RBD incremental backup.\"\"\""},{"line_number":675,"context_line":""},{"line_number":676,"context_line":"        LOG.debug \u003d (\"Trying to perform an incremental backup with container: \""},{"line_number":677,"context_line":"                     \"%(container)s, base_name: %(base)s, source RBD image: \""},{"line_number":678,"context_line":"                     \"%(source)s, volume ID %(volume)s and last incremental \""},{"line_number":679,"context_line":"                     \"backup ID: %(incr)s.\","}],"source_content_type":"text/x-python","patch_set":31,"id":"5fc1f717_78c79f46","line":676,"in_reply_to":"5fc1f717_d22b14e8","updated":"2019-04-10 11:51:00.000000000","message":"Done","commit_id":"cda3cd1cf9b8fc8c22b68b1e35530990b0b5b87b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b8657c35258f90fbd8e6cd21085d3b0b094d6b6b","unresolved":false,"context_lines":[{"line_number":681,"context_line":"                      \u0027base\u0027: base_name,"},{"line_number":682,"context_line":"                      \u0027source\u0027: source_rbd_image,"},{"line_number":683,"context_line":"                      \u0027volume\u0027: volume_id,"},{"line_number":684,"context_line":"                      \u0027incr\u0027: last_incr"},{"line_number":685,"context_line":"                      })"},{"line_number":686,"context_line":""},{"line_number":687,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"}],"source_content_type":"text/x-python","patch_set":31,"id":"5fc1f717_b2ff6864","line":684,"updated":"2019-04-10 09:59:02.000000000","message":"nit: if you are going to add the }) in a different line, please add a , at the end of the previous line. That way if someone adds a new element later the diff will only have the added lines as changes.","commit_id":"cda3cd1cf9b8fc8c22b68b1e35530990b0b5b87b"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"dc55a5f08b44236c76e5c9d6800cdd36938cd353","unresolved":false,"context_lines":[{"line_number":681,"context_line":"                      \u0027base\u0027: base_name,"},{"line_number":682,"context_line":"                      \u0027source\u0027: source_rbd_image,"},{"line_number":683,"context_line":"                      \u0027volume\u0027: volume_id,"},{"line_number":684,"context_line":"                      \u0027incr\u0027: last_incr"},{"line_number":685,"context_line":"                      })"},{"line_number":686,"context_line":""},{"line_number":687,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"}],"source_content_type":"text/x-python","patch_set":31,"id":"5fc1f717_b8e667e6","line":684,"in_reply_to":"5fc1f717_b2ff6864","updated":"2019-04-10 11:51:00.000000000","message":"Done","commit_id":"cda3cd1cf9b8fc8c22b68b1e35530990b0b5b87b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b8657c35258f90fbd8e6cd21085d3b0b094d6b6b","unresolved":false,"context_lines":[{"line_number":686,"context_line":""},{"line_number":687,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"},{"line_number":688,"context_line":"                                  container)) as client:"},{"line_number":689,"context_line":"            # If can\u0027t find the base_name, hat means we can\u0027t do the"},{"line_number":690,"context_line":"            # incremental backup, so raise the exception."},{"line_number":691,"context_line":"            if base_name not in eventlet.tpool.Proxy(self.rbd.RBD()).list("},{"line_number":692,"context_line":"                    ioctx\u003dclient.ioctx):"}],"source_content_type":"text/x-python","patch_set":31,"id":"5fc1f717_72c7c046","line":689,"range":{"start_line":689,"start_character":43,"end_line":689,"end_character":46},"updated":"2019-04-10 09:59:02.000000000","message":"nit: that","commit_id":"cda3cd1cf9b8fc8c22b68b1e35530990b0b5b87b"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"dc55a5f08b44236c76e5c9d6800cdd36938cd353","unresolved":false,"context_lines":[{"line_number":686,"context_line":""},{"line_number":687,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"},{"line_number":688,"context_line":"                                  container)) as client:"},{"line_number":689,"context_line":"            # If can\u0027t find the base_name, hat means we can\u0027t do the"},{"line_number":690,"context_line":"            # incremental backup, so raise the exception."},{"line_number":691,"context_line":"            if base_name not in eventlet.tpool.Proxy(self.rbd.RBD()).list("},{"line_number":692,"context_line":"                    ioctx\u003dclient.ioctx):"}],"source_content_type":"text/x-python","patch_set":31,"id":"5fc1f717_18d77bf0","line":689,"range":{"start_line":689,"start_character":43,"end_line":689,"end_character":46},"in_reply_to":"5fc1f717_72c7c046","updated":"2019-04-10 11:51:00.000000000","message":"Done","commit_id":"cda3cd1cf9b8fc8c22b68b1e35530990b0b5b87b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"b8657c35258f90fbd8e6cd21085d3b0b094d6b6b","unresolved":false,"context_lines":[{"line_number":1195,"context_line":"        length \u003d int(volume.size) * units.Gi"},{"line_number":1196,"context_line":""},{"line_number":1197,"context_line":"        base_name \u003d backup.parent.service_metadata"},{"line_number":1198,"context_line":"        if base_name is None:"},{"line_number":1199,"context_line":"            base_name \u003d self._get_backup_base_name(backup.volume_id,"},{"line_number":1200,"context_line":"                                                   diff_format\u003dTrue)"},{"line_number":1201,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"5fc1f717_f2e090ce","line":1198,"updated":"2019-04-10 09:59:02.000000000","message":"nit: what if it\u0027s an empty string?  better do:\n\n if not base_name:","commit_id":"cda3cd1cf9b8fc8c22b68b1e35530990b0b5b87b"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"dc55a5f08b44236c76e5c9d6800cdd36938cd353","unresolved":false,"context_lines":[{"line_number":1195,"context_line":"        length \u003d int(volume.size) * units.Gi"},{"line_number":1196,"context_line":""},{"line_number":1197,"context_line":"        base_name \u003d backup.parent.service_metadata"},{"line_number":1198,"context_line":"        if base_name is None:"},{"line_number":1199,"context_line":"            base_name \u003d self._get_backup_base_name(backup.volume_id,"},{"line_number":1200,"context_line":"                                                   diff_format\u003dTrue)"},{"line_number":1201,"context_line":""}],"source_content_type":"text/x-python","patch_set":31,"id":"5fc1f717_98fccb72","line":1198,"in_reply_to":"5fc1f717_f2e090ce","updated":"2019-04-10 11:51:00.000000000","message":"Done","commit_id":"cda3cd1cf9b8fc8c22b68b1e35530990b0b5b87b"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"94694d01bd8f67ccd37b3b5d92c0955ffc8e3e3c","unresolved":false,"context_lines":[{"line_number":322,"context_line":"        format."},{"line_number":323,"context_line":"        \"\"\""},{"line_number":324,"context_line":"        # Ensure no unicode"},{"line_number":325,"context_line":"        if diff_format:"},{"line_number":326,"context_line":"            if backup_id:"},{"line_number":327,"context_line":"                msg \u003d _(\"Backup id is not required when diff_format is True\")"},{"line_number":328,"context_line":"                raise exception.InvalidParameterValue(msg)"},{"line_number":329,"context_line":"            return utils.convert_str(\"volume-%s.backup.base\" % volume_id)"},{"line_number":330,"context_line":"        else:"},{"line_number":331,"context_line":"            if backup_id is None:"},{"line_number":332,"context_line":"                msg \u003d _(\"Backup id required\")"},{"line_number":333,"context_line":"                raise exception.InvalidParameterValue(msg)"},{"line_number":334,"context_line":"            return utils.convert_str(\"volume-%s.backup.%s\""},{"line_number":335,"context_line":"                                     % (volume_id, backup_id))"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":"    def _discard_bytes(self, volume, offset, length):"},{"line_number":338,"context_line":"        \"\"\"Trim length bytes from offset."}],"source_content_type":"text/x-python","patch_set":33,"id":"3fce034c_88219aa2","line":335,"range":{"start_line":325,"start_character":0,"end_line":335,"end_character":62},"updated":"2019-04-11 22:22:42.000000000","message":"When we use the method with \"diff_format\u003dFalse\" it assumes that we are looking for an incremental\u0027s base_name. Otherwise, it assumes that is a full backup. \n\nAs Gorka said, this is failing when trying to get the base_name from an incremental backup. This happens because instead of using the \"backup.parent.service_metadata\", it uses the backup\u0027s id. Because of thath, we need to find a way to look for the \"parent\" attribute from the backup parameter. \n\nLog: http://paste.openstack.org/show/749217/","commit_id":"90b342e20e5d323bd8293fc5aa9fd4a04db1cfef"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"94694d01bd8f67ccd37b3b5d92c0955ffc8e3e3c","unresolved":false,"context_lines":[{"line_number":1195,"context_line":"        \"\"\""},{"line_number":1196,"context_line":"        length \u003d int(volume.size) * units.Gi"},{"line_number":1197,"context_line":""},{"line_number":1198,"context_line":"        base_name \u003d backup.parent.service_metadata"},{"line_number":1199,"context_line":"        if not base_name:"},{"line_number":1200,"context_line":"            base_name \u003d self._get_backup_base_name(backup.volume_id,"},{"line_number":1201,"context_line":"                                                   diff_format\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":33,"id":"3fce034c_e8edbe25","line":1198,"range":{"start_line":1198,"start_character":7,"end_line":1198,"end_character":50},"updated":"2019-04-11 22:22:42.000000000","message":"In case we\u0027d like to restore a full backup, backup.parent is empty so this will fail.","commit_id":"90b342e20e5d323bd8293fc5aa9fd4a04db1cfef"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"72232ebb4463ecae863b65e7509835b0d102df37","unresolved":false,"context_lines":[{"line_number":729,"context_line":"        # Otherwise performs incremental rbd backup"},{"line_number":730,"context_line":"        else:"},{"line_number":731,"context_line":"            # Find the base name from the parent backup\u0027s service_metadata"},{"line_number":732,"context_line":"            base_name \u003d backup.parent.service_metadata"},{"line_number":733,"context_line":"            container \u003d backup.container"},{"line_number":734,"context_line":"            rbd_img \u003d source_rbd_image"},{"line_number":735,"context_line":"            last_incr \u003d backup.parent_id"}],"source_content_type":"text/x-python","patch_set":35,"id":"3fce034c_97ef2aaf","line":732,"range":{"start_line":732,"start_character":12,"end_line":732,"end_character":54},"updated":"2019-04-16 22:50:30.000000000","message":"We should probably use _get_backup_base_name because of L725.","commit_id":"f7409cdb429a55d8a6ea766ae4dc124fae76106f"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"0dea3e72e0623beeb42ba4fd09830bed2c97dc42","unresolved":false,"context_lines":[{"line_number":324,"context_line":"        if not backup:"},{"line_number":325,"context_line":"            return utils.convert_str(\"volume-%s.backup.base\" % volume_id)"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        if backup.parent:"},{"line_number":328,"context_line":"            return backup.parent.service_metadata"},{"line_number":329,"context_line":"        return utils.convert_str(\"volume-%s.backup.%s\""},{"line_number":330,"context_line":"                                 % (volume_id, backup.id))"}],"source_content_type":"text/x-python","patch_set":38,"id":"bfb3d3c7_67d010f9","line":327,"updated":"2019-05-20 06:07:45.000000000","message":"\"Deleting new backups\"\n1. Create 1 full and 1 incremental in Rocky\n2. Upgrade to Train\n3. Create 2 incremental in Train.\n\nWhen I try to delete the last two incremental I received this error: \"AttributeError: \u0027Backup\u0027 object has no attribute \u0027_obj_parent\u0027\"[1]. It\u0027s weird because it only happens when deleting backups and not when creating them. (Both processes uses the same \"_get_backup_base_name\" method).\n\n[1]: http://paste.openstack.org/show/751657/","commit_id":"525124475fd0246c5d21670eaf75d35ad2d65f8c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"61e8025c55cdd44de1a14766f49141d6910975a0","unresolved":false,"context_lines":[{"line_number":323,"context_line":"        # Ensure no unicode"},{"line_number":324,"context_line":"        if not backup:"},{"line_number":325,"context_line":"            return utils.convert_str(\"volume-%s.backup.base\" % volume_id)"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        if backup.parent:"},{"line_number":328,"context_line":"            base_name \u003d backup.parent.service_metadata"},{"line_number":329,"context_line":"            # when upgrading from an old openstack version with created backups"}],"source_content_type":"text/x-python","patch_set":40,"id":"9fb8cfa7_72ed0e8a","line":326,"updated":"2019-06-05 10:11:52.000000000","message":"I believe we should be checking the backup.service_metadata as well.  After all, all new backups after this change will have it.\n\n  if backup.service_metadata:\n      return backup.service_metadata","commit_id":"69d3dafc78a2abd095a60a20a2b2235247731492"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"61e8025c55cdd44de1a14766f49141d6910975a0","unresolved":false,"context_lines":[{"line_number":324,"context_line":"        if not backup:"},{"line_number":325,"context_line":"            return utils.convert_str(\"volume-%s.backup.base\" % volume_id)"},{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        if backup.parent:"},{"line_number":328,"context_line":"            base_name \u003d backup.parent.service_metadata"},{"line_number":329,"context_line":"            # when upgrading from an old openstack version with created backups"},{"line_number":330,"context_line":"            # all the new backups will have the parent atribute. However, when"}],"source_content_type":"text/x-python","patch_set":40,"id":"9fb8cfa7_12b29295","line":327,"updated":"2019-06-05 10:11:52.000000000","message":"I would add a comment saying that the parent field will only be present when creating a new incremental backup, as it is filled by cinder-api.","commit_id":"69d3dafc78a2abd095a60a20a2b2235247731492"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"61e8025c55cdd44de1a14766f49141d6910975a0","unresolved":false,"context_lines":[{"line_number":326,"context_line":""},{"line_number":327,"context_line":"        if backup.parent:"},{"line_number":328,"context_line":"            base_name \u003d backup.parent.service_metadata"},{"line_number":329,"context_line":"            # when upgrading from an old openstack version with created backups"},{"line_number":330,"context_line":"            # all the new backups will have the parent atribute. However, when"},{"line_number":331,"context_line":"            # creating an incremental from an old version backup, the parent"},{"line_number":332,"context_line":"            # backup won\u0027t have a service_metadata attribute. In that case we"},{"line_number":333,"context_line":"            # should use the default RBD backup format."},{"line_number":334,"context_line":"            if not base_name:"},{"line_number":335,"context_line":"                base_name \u003d utils.convert_str(\"volume-%s.backup.base\""},{"line_number":336,"context_line":"                                              % volume_id)"}],"source_content_type":"text/x-python","patch_set":40,"id":"9fb8cfa7_323a3634","line":333,"range":{"start_line":329,"start_character":0,"end_line":333,"end_character":55},"updated":"2019-06-05 10:11:52.000000000","message":"This comment seems a little hard to read for me.  Maybe something shorter would be easier.  ie: Old backups don\u0027t have the base name in the service_metadata, so we use the default RBD backup base.","commit_id":"69d3dafc78a2abd095a60a20a2b2235247731492"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"61e8025c55cdd44de1a14766f49141d6910975a0","unresolved":false,"context_lines":[{"line_number":692,"context_line":""},{"line_number":693,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"},{"line_number":694,"context_line":"                                  container)) as client:"},{"line_number":695,"context_line":"            # If can\u0027t find the base_name, that means we can\u0027t do the"},{"line_number":696,"context_line":"            # incremental backup, so raise the exception."},{"line_number":697,"context_line":"            if base_name not in eventlet.tpool.Proxy(self.rbd.RBD()).list("},{"line_number":698,"context_line":"                    ioctx\u003dclient.ioctx):"},{"line_number":699,"context_line":"                msg \u003d (_("},{"line_number":700,"context_line":"                    \"Can\u0027t find backup base image %s when do incremental \""},{"line_number":701,"context_line":"                    \"backup.\") % base_name)"},{"line_number":702,"context_line":"                LOG.error(msg)"},{"line_number":703,"context_line":"                raise exception.BackupOperationError(msg)"},{"line_number":704,"context_line":""},{"line_number":705,"context_line":"            base_rbd \u003d eventlet.tpool.Proxy(self.rbd.Image(client.ioctx,"},{"line_number":706,"context_line":"                                                           base_name,"}],"source_content_type":"text/x-python","patch_set":40,"id":"9fb8cfa7_5299ea6b","line":703,"range":{"start_line":695,"start_character":0,"end_line":703,"end_character":57},"updated":"2019-06-05 10:11:52.000000000","message":"Why don\u0027t we remove this and just try to get the base image directly and raise an exception if we cannot find it?  It\u0027s probably faster since most of the time the image will be there...","commit_id":"69d3dafc78a2abd095a60a20a2b2235247731492"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"61e8025c55cdd44de1a14766f49141d6910975a0","unresolved":false,"context_lines":[{"line_number":743,"context_line":"            container \u003d backup.container"},{"line_number":744,"context_line":"            rbd_img \u003d source_rbd_image"},{"line_number":745,"context_line":"            last_incr \u003d backup.parent_id"},{"line_number":746,"context_line":"            from_snap, image_created \u003d self._incremental_rbd_backup(container,"},{"line_number":747,"context_line":"                                                                    base_name,"},{"line_number":748,"context_line":"                                                                    length,"},{"line_number":749,"context_line":"                                                                    rbd_img,"}],"source_content_type":"text/x-python","patch_set":40,"id":"9fb8cfa7_72b98ee7","line":746,"updated":"2019-06-05 10:11:52.000000000","message":"Why don\u0027t we pass the backup object instead of passing the container and the last_incr as separate parameters?","commit_id":"69d3dafc78a2abd095a60a20a2b2235247731492"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"61e8025c55cdd44de1a14766f49141d6910975a0","unresolved":false,"context_lines":[{"line_number":793,"context_line":"                          {\u0027snapshot\u0027: new_snap, \u0027volume\u0027: volume_id})"},{"line_number":794,"context_line":"                source_rbd_image.remove_snap(new_snap)"},{"line_number":795,"context_line":""},{"line_number":796,"context_line":"        return {\u0027service_metadata\u0027: base_name}"},{"line_number":797,"context_line":""},{"line_number":798,"context_line":"    def _file_is_rbd(self, volume_file):"},{"line_number":799,"context_line":"        \"\"\"Returns True if the volume_file is actually an RBD image.\"\"\""}],"source_content_type":"text/x-python","patch_set":40,"id":"9fb8cfa7_12e7f214","line":796,"updated":"2019-06-05 10:11:52.000000000","message":"Storing the base_name like this limits our capacity to use the field for anything else in the future.  We could store it as JSON...\n\n  return {\u0027service_metadata\u0027: \u0027{\"base\":\"%s\"}\u0027 % base_name}","commit_id":"69d3dafc78a2abd095a60a20a2b2235247731492"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"61e8025c55cdd44de1a14766f49141d6910975a0","unresolved":false,"context_lines":[{"line_number":970,"context_line":"            try:"},{"line_number":971,"context_line":"                updates \u003d self._backup_rbd(backup, volume_file, volume.name,"},{"line_number":972,"context_line":"                                           length)"},{"line_number":973,"context_line":"            except (exception.BackupRBDOperationFailed or"},{"line_number":974,"context_line":"                    exception.BackupOperationError):"},{"line_number":975,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":976,"context_line":"                    self.delete_backup(backup)"}],"source_content_type":"text/x-python","patch_set":40,"id":"9fb8cfa7_5246ca7e","line":973,"updated":"2019-06-05 10:11:52.000000000","message":"This is not right, you should not use or here, a comma should be used instead.\n\n   except (exception.BackupRBDOperationFailed,\n           exception.BackupOperationError):","commit_id":"69d3dafc78a2abd095a60a20a2b2235247731492"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"61e8025c55cdd44de1a14766f49141d6910975a0","unresolved":false,"context_lines":[{"line_number":976,"context_line":"                    self.delete_backup(backup)"},{"line_number":977,"context_line":"        else:"},{"line_number":978,"context_line":"            LOG.debug(\"Volume file is NOT RBD: will do full backup.\")"},{"line_number":979,"context_line":"            do_full_backup \u003d True"},{"line_number":980,"context_line":""},{"line_number":981,"context_line":"        if do_full_backup:"},{"line_number":982,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":40,"id":"9fb8cfa7_f271fe0d","line":979,"updated":"2019-06-05 10:11:52.000000000","message":"Shouldn\u0027t we fail if they requested an incremental backup?  If we are supposed to honor the incremental feature we should fail when we cannot do incremental, which is always the case with non RBD volumes.","commit_id":"69d3dafc78a2abd095a60a20a2b2235247731492"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"61e8025c55cdd44de1a14766f49141d6910975a0","unresolved":false,"context_lines":[{"line_number":1009,"context_line":"                                  backup.container)) as client:"},{"line_number":1010,"context_line":"            # If a source snapshot is provided we assume the base is diff"},{"line_number":1011,"context_line":"            # format."},{"line_number":1012,"context_line":"            backup_name \u003d None"},{"line_number":1013,"context_line":"            if src_snap:"},{"line_number":1014,"context_line":"                backup_name \u003d self._get_backup_base_name(backup.volume_id)"},{"line_number":1015,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":40,"id":"9fb8cfa7_92c76213","line":1012,"updated":"2019-06-05 10:11:52.000000000","message":"Why do we initialize this variable if it\u0027s going to get changed 100% of the time on L1014 or L1016 right after?","commit_id":"69d3dafc78a2abd095a60a20a2b2235247731492"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"61e8025c55cdd44de1a14766f49141d6910975a0","unresolved":false,"context_lines":[{"line_number":1040,"context_line":"        shrink it to the size of the original backup so we need to"},{"line_number":1041,"context_line":"        post-process and resize it back to its expected size."},{"line_number":1042,"context_line":"        \"\"\""},{"line_number":1043,"context_line":"        backup_base \u003d self._get_backup_base_name(backup.volume_id)"},{"line_number":1044,"context_line":""},{"line_number":1045,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"},{"line_number":1046,"context_line":"                                  backup.container)) as client:"}],"source_content_type":"text/x-python","patch_set":40,"id":"9fb8cfa7_353c60be","line":1043,"updated":"2019-06-05 10:11:52.000000000","message":"This doesn\u0027t look right to me, as it will always return \"volume-%s.backup.base\".","commit_id":"69d3dafc78a2abd095a60a20a2b2235247731492"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"61e8025c55cdd44de1a14766f49141d6910975a0","unresolved":false,"context_lines":[{"line_number":1064,"context_line":"        rbd_user \u003d restore_file.rbd_user"},{"line_number":1065,"context_line":"        rbd_pool \u003d restore_file.rbd_pool"},{"line_number":1066,"context_line":"        rbd_conf \u003d restore_file.rbd_conf"},{"line_number":1067,"context_line":"        base_name \u003d self._get_backup_base_name(backup.volume_id)"},{"line_number":1068,"context_line":""},{"line_number":1069,"context_line":"        LOG.debug(\"Attempting incremental restore from base\u003d\u0027%(base)s\u0027 \""},{"line_number":1070,"context_line":"                  \"snap\u003d\u0027%(snap)s\u0027\","}],"source_content_type":"text/x-python","patch_set":40,"id":"9fb8cfa7_95766ca4","line":1067,"updated":"2019-06-05 10:11:52.000000000","message":"This doesn\u0027t look right to me, as it will always return \"volume-%s.backup.base\".","commit_id":"69d3dafc78a2abd095a60a20a2b2235247731492"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"61e8025c55cdd44de1a14766f49141d6910975a0","unresolved":false,"context_lines":[{"line_number":1196,"context_line":"        \"\"\""},{"line_number":1197,"context_line":"        length \u003d int(volume.size) * units.Gi"},{"line_number":1198,"context_line":""},{"line_number":1199,"context_line":"        if backup.parent:"},{"line_number":1200,"context_line":"            base_name \u003d backup.parent.service_metadata"},{"line_number":1201,"context_line":"        else:"},{"line_number":1202,"context_line":"            base_name \u003d self._get_backup_base_name(backup.volume_id)"}],"source_content_type":"text/x-python","patch_set":40,"id":"9fb8cfa7_5509d429","line":1199,"updated":"2019-06-05 10:11:52.000000000","message":"This will never happen.  The parent is only set on the backup creation API call, therefore we will never have the field set when restoring.\n\nA different matter is if we check backup.service_metadata, which will be set for new volumes.\n\nThat\u0027s why I think it\u0027s a good idea to add that check in the _get_backup_base_name method itself.","commit_id":"69d3dafc78a2abd095a60a20a2b2235247731492"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"10e93f706eadd11e44d14742f294541801a16df7","unresolved":false,"context_lines":[{"line_number":711,"context_line":"                        \"base name image %(base)s.\") %"},{"line_number":712,"context_line":"                        {\u0027incr\u0027: last_incr, \u0027base\u0027: base_name})"},{"line_number":713,"context_line":"                    LOG.error(msg)"},{"line_number":714,"context_line":"                    raise exception.BackupRBDOperationError(msg)"},{"line_number":715,"context_line":"            finally:"},{"line_number":716,"context_line":"                base_rbd.close()"},{"line_number":717,"context_line":""}],"source_content_type":"text/x-python","patch_set":43,"id":"9fb8cfa7_1f22ce02","line":714,"range":{"start_line":714,"start_character":36,"end_line":714,"end_character":59},"updated":"2019-07-03 06:51:34.000000000","message":"I do not find this BackupRBDOperationError in cinder.exception. Do you define it?","commit_id":"fa4a2699ced1cb6ef269b50070325247d3a33e9a"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"84d19a7e3b74eb63148c2b9b426b20d39290db5f","unresolved":false,"context_lines":[{"line_number":711,"context_line":"                        \"base name image %(base)s.\") %"},{"line_number":712,"context_line":"                        {\u0027incr\u0027: last_incr, \u0027base\u0027: base_name})"},{"line_number":713,"context_line":"                    LOG.error(msg)"},{"line_number":714,"context_line":"                    raise exception.BackupRBDOperationError(msg)"},{"line_number":715,"context_line":"            finally:"},{"line_number":716,"context_line":"                base_rbd.close()"},{"line_number":717,"context_line":""}],"source_content_type":"text/x-python","patch_set":43,"id":"7faddb67_6a4cef41","line":714,"range":{"start_line":714,"start_character":36,"end_line":714,"end_character":59},"in_reply_to":"9fb8cfa7_1f22ce02","updated":"2019-07-12 02:51:58.000000000","message":"Done","commit_id":"fa4a2699ced1cb6ef269b50070325247d3a33e9a"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"ef47158b7e063a61c77aef7f729b91d178857be7","unresolved":false,"context_lines":[{"line_number":728,"context_line":""},{"line_number":729,"context_line":"        # If backup.parent_id is None performs full RBD backup"},{"line_number":730,"context_line":"        if backup.parent_id is None:"},{"line_number":731,"context_line":"            base_name \u003d self._get_backup_base_name(volume_id, backup)"},{"line_number":732,"context_line":"            from_snap, image_created \u003d self._full_rbd_backup(backup.container,"},{"line_number":733,"context_line":"                                                             base_name,"},{"line_number":734,"context_line":"                                                             length)"}],"source_content_type":"text/x-python","patch_set":45,"id":"7faddb67_e4d1cffc","line":731,"range":{"start_line":731,"start_character":62,"end_line":731,"end_character":68},"updated":"2019-07-15 17:23:51.000000000","message":"same","commit_id":"7b3e7482484962c32644b3eb2cf67fe39b6c0984"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"ef47158b7e063a61c77aef7f729b91d178857be7","unresolved":false,"context_lines":[{"line_number":735,"context_line":"        # Otherwise performs incremental rbd backup"},{"line_number":736,"context_line":"        else:"},{"line_number":737,"context_line":"            # Find the base name from the parent backup\u0027s service_metadata"},{"line_number":738,"context_line":"            base_name \u003d self._get_backup_base_name(volume_id, backup)"},{"line_number":739,"context_line":"            rbd_img \u003d source_rbd_image"},{"line_number":740,"context_line":"            from_snap, image_created \u003d self._incremental_rbd_backup(backup,"},{"line_number":741,"context_line":"                                                                    base_name,"}],"source_content_type":"text/x-python","patch_set":45,"id":"7faddb67_04cf4b56","line":738,"range":{"start_line":738,"start_character":62,"end_line":738,"end_character":68},"updated":"2019-07-15 17:23:51.000000000","message":"same","commit_id":"7b3e7482484962c32644b3eb2cf67fe39b6c0984"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"ef47158b7e063a61c77aef7f729b91d178857be7","unresolved":false,"context_lines":[{"line_number":800,"context_line":"        image."},{"line_number":801,"context_line":"        \"\"\""},{"line_number":802,"context_line":"        volume_id \u003d backup.volume_id"},{"line_number":803,"context_line":"        backup_name \u003d self._get_backup_base_name(volume_id, backup)"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"},{"line_number":806,"context_line":"                                  backup.container)) as client:"}],"source_content_type":"text/x-python","patch_set":45,"id":"7faddb67_a4d757ee","line":803,"range":{"start_line":803,"start_character":60,"end_line":803,"end_character":66},"updated":"2019-07-15 17:23:51.000000000","message":"same","commit_id":"7b3e7482484962c32644b3eb2cf67fe39b6c0984"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"ef47158b7e063a61c77aef7f729b91d178857be7","unresolved":false,"context_lines":[{"line_number":1012,"context_line":"                backup_name \u003d self._get_backup_base_name(backup.volume_id)"},{"line_number":1013,"context_line":"            else:"},{"line_number":1014,"context_line":"                backup_name \u003d self._get_backup_base_name(backup.volume_id,"},{"line_number":1015,"context_line":"                                                         backup)"},{"line_number":1016,"context_line":""},{"line_number":1017,"context_line":"            # Retrieve backup volume"},{"line_number":1018,"context_line":"            src_rbd \u003d eventlet.tpool.Proxy(self.rbd.Image(client.ioctx,"}],"source_content_type":"text/x-python","patch_set":45,"id":"7faddb67_64de3f12","line":1015,"range":{"start_line":1015,"start_character":57,"end_line":1015,"end_character":63},"updated":"2019-07-15 17:23:51.000000000","message":"Please pass backup\u003dbackup here, which is less confusing/error-prone for optional args.","commit_id":"7b3e7482484962c32644b3eb2cf67fe39b6c0984"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"ef47158b7e063a61c77aef7f729b91d178857be7","unresolved":false,"context_lines":[{"line_number":1038,"context_line":"        shrink it to the size of the original backup so we need to"},{"line_number":1039,"context_line":"        post-process and resize it back to its expected size."},{"line_number":1040,"context_line":"        \"\"\""},{"line_number":1041,"context_line":"        backup_base \u003d self._get_backup_base_name(backup.volume_id, backup)"},{"line_number":1042,"context_line":""},{"line_number":1043,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"},{"line_number":1044,"context_line":"                                  backup.container)) as client:"}],"source_content_type":"text/x-python","patch_set":45,"id":"7faddb67_84e33bd9","line":1041,"range":{"start_line":1041,"start_character":67,"end_line":1041,"end_character":73},"updated":"2019-07-15 17:23:51.000000000","message":"same","commit_id":"7b3e7482484962c32644b3eb2cf67fe39b6c0984"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"ef47158b7e063a61c77aef7f729b91d178857be7","unresolved":false,"context_lines":[{"line_number":1062,"context_line":"        rbd_user \u003d restore_file.rbd_user"},{"line_number":1063,"context_line":"        rbd_pool \u003d restore_file.rbd_pool"},{"line_number":1064,"context_line":"        rbd_conf \u003d restore_file.rbd_conf"},{"line_number":1065,"context_line":"        base_name \u003d self._get_backup_base_name(backup.volume_id, backup)"},{"line_number":1066,"context_line":""},{"line_number":1067,"context_line":"        LOG.debug(\"Attempting incremental restore from base\u003d\u0027%(base)s\u0027 \""},{"line_number":1068,"context_line":"                  \"snap\u003d\u0027%(snap)s\u0027\","}],"source_content_type":"text/x-python","patch_set":45,"id":"7faddb67_44e9c3b4","line":1065,"range":{"start_line":1065,"start_character":65,"end_line":1065,"end_character":71},"updated":"2019-07-15 17:23:51.000000000","message":"same","commit_id":"7b3e7482484962c32644b3eb2cf67fe39b6c0984"},{"author":{"_account_id":7198,"name":"Jay Bryant","email":"jungleboyj@electronicjungle.net","username":"jsbryant"},"change_message_id":"7446e75d2a18aa4cafc81dffa3403957310dd0c8","unresolved":false,"context_lines":[{"line_number":970,"context_line":"                    self.delete_backup(backup)"},{"line_number":971,"context_line":"        else:"},{"line_number":972,"context_line":"            if backup.parent_id:"},{"line_number":973,"context_line":"                LOG.debug(\"Volume file is NOT RBD: can\u0027t perform\""},{"line_number":974,"context_line":"                          \"incremental backup.\")"},{"line_number":975,"context_line":"                raise exception.BackupRBDOperationFailed"},{"line_number":976,"context_line":"            LOG.debug(\"Volume file is NOT RBD: will do full backup.\")"}],"source_content_type":"text/x-python","patch_set":50,"id":"7faddb67_2f90fb94","line":973,"range":{"start_line":973,"start_character":57,"end_line":973,"end_character":65},"updated":"2019-07-17 19:17:06.000000000","message":"missing space at eol.","commit_id":"5e70208e0b5c7f6e079b30204c45cc56334b48b0"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"bde17b764bb4aca7f8ec70a1c6ee33edb524b10e","unresolved":false,"context_lines":[{"line_number":328,"context_line":"        if backup.service_metadata:"},{"line_number":329,"context_line":"            return json.loads(backup.service_metadata)[\"base\"]"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        # \u0027parent\u0027 field will only be present when creating a new incremental"},{"line_number":332,"context_line":"        # backup. This is filled by cinder-api"},{"line_number":333,"context_line":"        if backup.parent:"},{"line_number":334,"context_line":"            # Old backups don\u0027t have the base name in the service_metadata,"}],"source_content_type":"text/x-python","patch_set":51,"id":"7faddb67_36cdaf30","line":331,"updated":"2019-07-18 09:09:11.000000000","message":"nit: Referring to the incremental backup as new may be confusing, as it will also be present for older backups since parent_id was previously set by the ceph backup driver itself.","commit_id":"9345ffff97747b3b7364f18a935b32128584a61b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"bde17b764bb4aca7f8ec70a1c6ee33edb524b10e","unresolved":false,"context_lines":[{"line_number":333,"context_line":"        if backup.parent:"},{"line_number":334,"context_line":"            # Old backups don\u0027t have the base name in the service_metadata,"},{"line_number":335,"context_line":"            # so we use the default RBD backup base."},{"line_number":336,"context_line":"            base_name \u003d utils.convert_str(\"volume-%s.backup.base\" % volume_id)"},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"            if backup.parent.service_metadata:"},{"line_number":339,"context_line":"                base_name \u003d json.loads(backup.parent.service_metadata)[\"base\"]"}],"source_content_type":"text/x-python","patch_set":51,"id":"7faddb67_36f60f65","line":336,"updated":"2019-07-18 09:09:11.000000000","message":"-1: This line should be part of an `else` clause on L340 to avoid doing unnecessary work when the parent has the base name.\n\n            if backup.parent.service_metadata:\n                base_name \u003d json.loads(backup.parent.service_metadata)[\"base\"]\n            else:\n                base_name \u003d utils.convert_str(\"volume-%s.backup.base\" % volume_id)\n\n            return base_name\n\n\nOr we can change it to:\n\n\n            if backup.parent.service_metadata:\n                return json.loads(backup.parent.service_metadata)[\"base\"]\n            return utils.convert_str(\"volume-%s.backup.base\" % volume_id)","commit_id":"9345ffff97747b3b7364f18a935b32128584a61b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"bde17b764bb4aca7f8ec70a1c6ee33edb524b10e","unresolved":false,"context_lines":[{"line_number":491,"context_line":"        if base_name is None:"},{"line_number":492,"context_line":"            try_diff_format \u003d True"},{"line_number":493,"context_line":""},{"line_number":494,"context_line":"            base_name \u003d self._get_backup_base_name(volume_id, backup\u003dbackup)"},{"line_number":495,"context_line":"            LOG.debug(\"Trying diff format basename\u003d\u0027%(basename)s\u0027 for \""},{"line_number":496,"context_line":"                      \"backup base image of volume %(volume)s.\","},{"line_number":497,"context_line":"                      {\u0027basename\u0027: base_name, \u0027volume\u0027: volume_id})"}],"source_content_type":"text/x-python","patch_set":51,"id":"7faddb67_b65d1f48","line":494,"updated":"2019-07-18 09:09:11.000000000","message":"This is OK, but I disagree with Eric on the usefulness of being explicit here when the variable name and the parameter name are the same, as there can be no confusion. It\u0027s a different matter when we pass a boolean literal.","commit_id":"9345ffff97747b3b7364f18a935b32128584a61b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"bde17b764bb4aca7f8ec70a1c6ee33edb524b10e","unresolved":false,"context_lines":[{"line_number":956,"context_line":"        volume_file.seek(0)"},{"line_number":957,"context_line":"        length \u003d self._get_volume_size_gb(volume)"},{"line_number":958,"context_line":""},{"line_number":959,"context_line":"        do_full_backup \u003d False"},{"line_number":960,"context_line":"        if backup.snapshot_id:"},{"line_number":961,"context_line":"            do_full_backup \u003d True"},{"line_number":962,"context_line":"        elif self._file_is_rbd(volume_file):"}],"source_content_type":"text/x-python","patch_set":51,"id":"7faddb67_a910604f","line":959,"updated":"2019-07-18 09:09:11.000000000","message":"nit: We could move this line to L963","commit_id":"9345ffff97747b3b7364f18a935b32128584a61b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"bde17b764bb4aca7f8ec70a1c6ee33edb524b10e","unresolved":false,"context_lines":[{"line_number":1195,"context_line":"        \"\"\""},{"line_number":1196,"context_line":"        length \u003d int(volume.size) * units.Gi"},{"line_number":1197,"context_line":""},{"line_number":1198,"context_line":"        base_name \u003d self._get_backup_base_name(backup.volume_id)"},{"line_number":1199,"context_line":""},{"line_number":1200,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient("},{"line_number":1201,"context_line":"                                  self, backup.container)) as client:"}],"source_content_type":"text/x-python","patch_set":51,"id":"7faddb67_0950940a","line":1198,"updated":"2019-07-18 09:09:11.000000000","message":"-1: This doesn\u0027t look right to me.  I believe we need to pass the backup as well so we can get the base name we stored during creation in the backup\u0027s metadata.\n\nYou can confirm that it is wrong looking at the backup logs in the Ceph job.  There are incremental backups, yet all the restores are going through the full restore directly, not even trying to do the diff.","commit_id":"9345ffff97747b3b7364f18a935b32128584a61b"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"e76eb4f1164596637aeb891aa68a3e7e8cec7209","unresolved":false,"context_lines":[{"line_number":1005,"context_line":"        \"\"\""},{"line_number":1006,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"},{"line_number":1007,"context_line":"                                  backup.container)) as client:"},{"line_number":1008,"context_line":"            # If a source snapshot is provided we assume the base is diff"},{"line_number":1009,"context_line":"            # format."},{"line_number":1010,"context_line":"            if src_snap:"},{"line_number":1011,"context_line":"                backup_name \u003d self._get_backup_base_name(backup.volume_id)"},{"line_number":1012,"context_line":"            else:"},{"line_number":1013,"context_line":"                backup_name \u003d self._get_backup_base_name(backup.volume_id,"},{"line_number":1014,"context_line":"                                                         backup\u003dbackup)"},{"line_number":1015,"context_line":""},{"line_number":1016,"context_line":"            # Retrieve backup volume"},{"line_number":1017,"context_line":"            src_rbd \u003d eventlet.tpool.Proxy(self.rbd.Image(client.ioctx,"}],"source_content_type":"text/x-python","patch_set":52,"id":"7faddb67_3315ab40","line":1014,"range":{"start_line":1008,"start_character":0,"end_line":1014,"end_character":71},"updated":"2019-07-19 08:46:34.000000000","message":"Shall we just use backup_name \u003d self._get_backup_base_name(backup.volume_id, backup\u003dbackup)?","commit_id":"da5d2ee104989c81f7f955a86de85146d99a58fe"},{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"824dd85f1ec7f801dafa0260593865c108ad5382","unresolved":false,"context_lines":[{"line_number":1005,"context_line":"        \"\"\""},{"line_number":1006,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"},{"line_number":1007,"context_line":"                                  backup.container)) as client:"},{"line_number":1008,"context_line":"            # If a source snapshot is provided we assume the base is diff"},{"line_number":1009,"context_line":"            # format."},{"line_number":1010,"context_line":"            if src_snap:"},{"line_number":1011,"context_line":"                backup_name \u003d self._get_backup_base_name(backup.volume_id)"},{"line_number":1012,"context_line":"            else:"},{"line_number":1013,"context_line":"                backup_name \u003d self._get_backup_base_name(backup.volume_id,"},{"line_number":1014,"context_line":"                                                         backup\u003dbackup)"},{"line_number":1015,"context_line":""},{"line_number":1016,"context_line":"            # Retrieve backup volume"},{"line_number":1017,"context_line":"            src_rbd \u003d eventlet.tpool.Proxy(self.rbd.Image(client.ioctx,"}],"source_content_type":"text/x-python","patch_set":52,"id":"7faddb67_2ec8e0a3","line":1014,"range":{"start_line":1008,"start_character":0,"end_line":1014,"end_character":71},"in_reply_to":"7faddb67_13c3cf58","updated":"2019-07-19 10:40:42.000000000","message":"Yeah, IMHO, I think we can.\nAnd I have test it in my env. It\u0027s ok with volume(ceph). But I think it can be ok for volume(non-ceph) with backup(ceph) too. How do you think?","commit_id":"da5d2ee104989c81f7f955a86de85146d99a58fe"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"e2d0bc8a9f66f8943aa062f3993a67cdd08330a4","unresolved":false,"context_lines":[{"line_number":1005,"context_line":"        \"\"\""},{"line_number":1006,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"},{"line_number":1007,"context_line":"                                  backup.container)) as client:"},{"line_number":1008,"context_line":"            # If a source snapshot is provided we assume the base is diff"},{"line_number":1009,"context_line":"            # format."},{"line_number":1010,"context_line":"            if src_snap:"},{"line_number":1011,"context_line":"                backup_name \u003d self._get_backup_base_name(backup.volume_id)"},{"line_number":1012,"context_line":"            else:"},{"line_number":1013,"context_line":"                backup_name \u003d self._get_backup_base_name(backup.volume_id,"},{"line_number":1014,"context_line":"                                                         backup\u003dbackup)"},{"line_number":1015,"context_line":""},{"line_number":1016,"context_line":"            # Retrieve backup volume"},{"line_number":1017,"context_line":"            src_rbd \u003d eventlet.tpool.Proxy(self.rbd.Image(client.ioctx,"}],"source_content_type":"text/x-python","patch_set":52,"id":"7faddb67_c3745e01","line":1014,"range":{"start_line":1008,"start_character":0,"end_line":1014,"end_character":71},"in_reply_to":"7faddb67_2ec8e0a3","updated":"2019-07-23 01:00:02.000000000","message":"I think for volume(non-ceph), it will always be full restore here. So it will ok.","commit_id":"da5d2ee104989c81f7f955a86de85146d99a58fe"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"0f8f66303cbca546025c62986df2e707aa849a27","unresolved":false,"context_lines":[{"line_number":1005,"context_line":"        \"\"\""},{"line_number":1006,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"},{"line_number":1007,"context_line":"                                  backup.container)) as client:"},{"line_number":1008,"context_line":"            # If a source snapshot is provided we assume the base is diff"},{"line_number":1009,"context_line":"            # format."},{"line_number":1010,"context_line":"            if src_snap:"},{"line_number":1011,"context_line":"                backup_name \u003d self._get_backup_base_name(backup.volume_id)"},{"line_number":1012,"context_line":"            else:"},{"line_number":1013,"context_line":"                backup_name \u003d self._get_backup_base_name(backup.volume_id,"},{"line_number":1014,"context_line":"                                                         backup\u003dbackup)"},{"line_number":1015,"context_line":""},{"line_number":1016,"context_line":"            # Retrieve backup volume"},{"line_number":1017,"context_line":"            src_rbd \u003d eventlet.tpool.Proxy(self.rbd.Image(client.ioctx,"}],"source_content_type":"text/x-python","patch_set":52,"id":"7faddb67_13c3cf58","line":1014,"range":{"start_line":1008,"start_character":0,"end_line":1014,"end_character":71},"in_reply_to":"7faddb67_3315ab40","updated":"2019-07-19 09:31:19.000000000","message":"I also have a little confusion here, why don\u0027t\u0027 we just pass in the base_name that got in _restore_volume method #1198-#1201?","commit_id":"da5d2ee104989c81f7f955a86de85146d99a58fe"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"296788fe39b05009b3233e0f01c6c59c5d5a4bfb","unresolved":false,"context_lines":[{"line_number":326,"context_line":"            return utils.convert_str(\"volume-%s.backup.base\" % volume_id)"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"        if backup.service_metadata:"},{"line_number":329,"context_line":"            return json.loads(backup.service_metadata)[\"base\"]"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        # \u0027parent\u0027 field will only be present in incremental backups. This is"},{"line_number":332,"context_line":"        # filled by cinder-api"}],"source_content_type":"text/x-python","patch_set":55,"id":"7faddb67_77852c07","line":329,"range":{"start_line":329,"start_character":19,"end_line":329,"end_character":62},"updated":"2019-08-22 23:12:35.000000000","message":"In python2, this is going to be \u0027unicode\u0027 type ... only pointing this out because of the comment at line 324.  Should probably wrap this call and the one at line 337 with convert_str so that we\u0027re always dealing with a \u0027str\u0027 in both py2 and py3.  (Or restructure the function to return from a single point where the base_name is always converted.)","commit_id":"02a9011d1bd2e8b096a2848701f3f2c080c3b888"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"296788fe39b05009b3233e0f01c6c59c5d5a4bfb","unresolved":false,"context_lines":[{"line_number":1006,"context_line":"        \"\"\""},{"line_number":1007,"context_line":"        with eventlet.tpool.Proxy(rbd_driver.RADOSClient(self,"},{"line_number":1008,"context_line":"                                  backup.container)) as client:"},{"line_number":1009,"context_line":"            # If a source snapshot is provided we assume the base is diff"},{"line_number":1010,"context_line":"            # format."},{"line_number":1011,"context_line":"            if src_snap:"},{"line_number":1012,"context_line":"                backup_name \u003d self._get_backup_base_name(backup.volume_id,"},{"line_number":1013,"context_line":"                                                         backup\u003dbackup)"}],"source_content_type":"text/x-python","patch_set":55,"id":"7faddb67_1785d8ae","line":1010,"range":{"start_line":1009,"start_character":0,"end_line":1010,"end_character":21},"updated":"2019-08-22 23:12:35.000000000","message":"I\u0027m not sure how accurate this comment is anymore given that the _get_backup_base_name function has gotten more complicated.  (This may be a nit.)","commit_id":"02a9011d1bd2e8b096a2848701f3f2c080c3b888"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"e00733cb5d538a485abfa9660e29536b722e99a9","unresolved":false,"context_lines":[{"line_number":315,"context_line":"        ioctx.close()"},{"line_number":316,"context_line":"        client.shutdown()"},{"line_number":317,"context_line":""},{"line_number":318,"context_line":"    def format_base_name(service_metadata):"},{"line_number":319,"context_line":"        base_name \u003d json.loads(service_metadata)[\"base\"]"},{"line_number":320,"context_line":"        return utils.convert_str(base_name)"},{"line_number":321,"context_line":""}],"source_content_type":"text/x-python","patch_set":56,"id":"7faddb67_f0e5943f","line":318,"updated":"2019-08-23 18:19:30.000000000","message":"Could follow the convention here of calling this \"_format_base_name\" since it\u0027s not a public method.","commit_id":"2bda8f687a13b3f50b83229c6c09aeb2b0e0cee6"}],"cinder/exception.py":[{"author":{"_account_id":28706,"name":"Boxiang Zhu","email":"bxzhu_5355@163.com","username":"ZhuBoxiang"},"change_message_id":"320fa37f57d5181458b9c1f6a64ee34e4529799c","unresolved":false,"context_lines":[{"line_number":785,"context_line":"    message \u003d _(\"Backup RBD operation failed\")"},{"line_number":786,"context_line":""},{"line_number":787,"context_line":""},{"line_number":788,"context_line":"class BackupRBDOperationError(BackupDriverException):"},{"line_number":789,"context_line":"    message \u003d _(\"Backup RBD operation failed\")"},{"line_number":790,"context_line":""},{"line_number":791,"context_line":""}],"source_content_type":"text/x-python","patch_set":44,"id":"7faddb67_2a9cb768","line":788,"range":{"start_line":788,"start_character":6,"end_line":788,"end_character":29},"updated":"2019-07-12 03:17:59.000000000","message":"IMHO, one little question is that I found the err msg is the same info as BackupRBDOperationFailed. Why we need a new exception with another name?","commit_id":"775721e70791a1f2a27ae12e835f795ca747fad2"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"4ac942c8117f8815cf68f9d277b435e554901cf2","unresolved":false,"context_lines":[{"line_number":785,"context_line":"    message \u003d _(\"Backup RBD operation failed\")"},{"line_number":786,"context_line":""},{"line_number":787,"context_line":""},{"line_number":788,"context_line":"class BackupRBDOperationError(BackupDriverException):"},{"line_number":789,"context_line":"    message \u003d _(\"Backup RBD operation failed\")"},{"line_number":790,"context_line":""},{"line_number":791,"context_line":""}],"source_content_type":"text/x-python","patch_set":44,"id":"7faddb67_2f110553","line":788,"range":{"start_line":788,"start_character":6,"end_line":788,"end_character":29},"in_reply_to":"7faddb67_2a9cb768","updated":"2019-07-15 15:19:24.000000000","message":"I read the comments looking for an explanation and I can\u0027t find one. As you said, there isn\u0027t a need for a new exception with another name.","commit_id":"775721e70791a1f2a27ae12e835f795ca747fad2"}],"cinder/objects/backup.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ac0e3b551e44b2f214af5cc4b538afae95f058a0","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    # Version 1.4: Add restore_volume_id"},{"line_number":41,"context_line":"    # Version 1.5: Add metadata"},{"line_number":42,"context_line":"    # Version 1.6: Add encryption_key_id"},{"line_number":43,"context_line":"    VERSION \u003d \u00271.6\u0027"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    OPTIONAL_FIELDS \u003d (\u0027metadata\u0027,)"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":30,"id":"5fc1f717_d4db5b1e","line":43,"updated":"2019-04-05 10:19:22.000000000","message":"-1: You need to bump the version, update the comment, and add the new version in cinder/objects/base.py as version 1.36","commit_id":"a6308d42a32b5b92bca33296f55ac749d60a1d95"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ac0e3b551e44b2f214af5cc4b538afae95f058a0","unresolved":false,"context_lines":[{"line_number":111,"context_line":""},{"line_number":112,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":113,"context_line":"        \"\"\"Make an object representation compatible with a target version.\"\"\""},{"line_number":114,"context_line":"        super(Backup, self).obj_make_compatible(primitive, target_version)"},{"line_number":115,"context_line":"        target_version \u003d versionutils.convert_version_to_tuple(target_version)"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":30,"id":"5fc1f717_d404bb80","line":114,"updated":"2019-04-05 10:19:22.000000000","message":"-1: You need to drop the parent field when converting to version 1.6 or earlier.  Otherwise we break rolling upgrades.\n\nYou can see an example in the same method of the Volume OVO.","commit_id":"a6308d42a32b5b92bca33296f55ac749d60a1d95"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"72232ebb4463ecae863b65e7509835b0d102df37","unresolved":false,"context_lines":[{"line_number":112,"context_line":""},{"line_number":113,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":114,"context_line":"        \"\"\"Make an object representation compatible with a target version.\"\"\""},{"line_number":115,"context_line":"        added_fields \u003d (((1, 7), (\u0027parent\u0027)))"},{"line_number":116,"context_line":""},{"line_number":117,"context_line":"        super(Backup, self).obj_make_compatible(primitive, target_version)"},{"line_number":118,"context_line":"        target_version \u003d versionutils.convert_version_to_tuple(target_version)"}],"source_content_type":"text/x-python","patch_set":35,"id":"3fce034c_bc6d6d33","line":115,"range":{"start_line":115,"start_character":8,"end_line":115,"end_character":45},"updated":"2019-04-16 22:50:30.000000000","message":"openstack-tox-lower-constraints is failing because of this.\n\n```\ncinder.tests.unit.objects.test_objects.TestObjectVersions.test_obj_make_compatible\n\"TypeError: \u0027\u003c\u0027 not supported between instances of \u0027tuple\u0027 and \u0027int\u0027\"\n```\n\nThis should be:\n`added_fields \u003d (((1, 7), (\u0027parent\u0027,)),)`","commit_id":"f7409cdb429a55d8a6ea766ae4dc124fae76106f"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"0dea3e72e0623beeb42ba4fd09830bed2c97dc42","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    # Version 1.7: Add parent"},{"line_number":44,"context_line":"    VERSION \u003d \u00271.7\u0027"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    OPTIONAL_FIELDS \u003d (\u0027metadata\u0027,)"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"    fields \u003d {"},{"line_number":49,"context_line":"        \u0027id\u0027: fields.UUIDField(),"}],"source_content_type":"text/x-python","patch_set":38,"id":"bfb3d3c7_e7b720d8","line":46,"updated":"2019-05-20 06:07:45.000000000","message":"I think we need to add \u0027parent\u0027 to OPTIONAL_FIELDS. \n\n\"Creating backups\"\n1. Create 1 full and 1 incremental in Rocky\n2. Upgrade to Train\n3. Create one incremental in Train. This doesn\u0027t work because of \"ObjectActionError: Object action obj_load_attr failed because attribute parent not lazy-loadable \" [1]. I think the fix is to add \u0027parent\u0027 to OPTIONAL_FIELDS. \n\n\n[1] http://paste.openstack.org/show/751656/","commit_id":"525124475fd0246c5d21670eaf75d35ad2d65f8c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"61e8025c55cdd44de1a14766f49141d6910975a0","unresolved":false,"context_lines":[{"line_number":182,"context_line":"                self.metadata \u003d db.backup_metadata_update(self._context,"},{"line_number":183,"context_line":"                                                          self.id, metadata,"},{"line_number":184,"context_line":"                                                          True)"},{"line_number":185,"context_line":"            if \u0027parent\u0027 in updates:"},{"line_number":186,"context_line":"                updates.pop(\u0027parent\u0027, None)"},{"line_number":187,"context_line":"            db.backup_update(self._context, self.id, updates)"},{"line_number":188,"context_line":""}],"source_content_type":"text/x-python","patch_set":40,"id":"9fb8cfa7_49670f4f","line":185,"updated":"2019-06-05 10:11:52.000000000","message":"nit: We don\u0027t need to check, we can directly do the pop.  Since we are giving a default value it will not fail even if it\u0027s not present.","commit_id":"69d3dafc78a2abd095a60a20a2b2235247731492"}],"cinder/objects/base.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"61e8025c55cdd44de1a14766f49141d6910975a0","unresolved":false,"context_lines":[{"line_number":147,"context_line":"OBJ_VERSIONS.add(\u00271.36\u0027, {\u0027RequestSpec\u0027: \u00271.4\u0027})"},{"line_number":148,"context_line":"OBJ_VERSIONS.add(\u00271.37\u0027, {\u0027RequestSpec\u0027: \u00271.5\u0027})"},{"line_number":149,"context_line":"OBJ_VERSIONS.add(\u00271.38\u0027, {\u0027Backup\u0027: \u00271.7\u0027})"},{"line_number":150,"context_line":"OBJ_VERSIONS.add(\u00271.39\u0027, {\u0027BackupImport\u0027: \u00271.7\u0027})"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"class CinderObjectRegistry(base.VersionedObjectRegistry):"}],"source_content_type":"text/x-python","patch_set":40,"id":"9fb8cfa7_894c27c8","line":150,"updated":"2019-06-05 10:11:52.000000000","message":"-1: We don\u0027t need an additional history version for this, please add it in 1.38:\n\n   OBJ_VERSIONS.add(\u00271.38\u0027, {\u0027Backup\u0027: \u00271.7\u0027, \u0027BackupImport\u0027: \u00271.7\u0027})","commit_id":"69d3dafc78a2abd095a60a20a2b2235247731492"}],"cinder/tests/unit/backup/drivers/test_backup_ceph.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"48d91c128784326a074d70ad20c5be6444fc8ab8","unresolved":false,"context_lines":[{"line_number":597,"context_line":""},{"line_number":598,"context_line":"        self.mock_rbd.RBD().list.return_value \u003d [backup_name]"},{"line_number":599,"context_line":""},{"line_number":600,"context_line":"        with mock.patch.object(self.service, \u0027_rbd_diff_transfer\u0027), \\"},{"line_number":601,"context_line":"                mock.patch.object(self.service, \u0027_create_base_image\u0027) as \\"},{"line_number":602,"context_line":"                mock_create_base_image, mock.patch.object("},{"line_number":603,"context_line":"                rbd_driver, \u0027RADOSClient\u0027) as mock_rados_client:"},{"line_number":604,"context_line":"            client \u003d mock.Mock()"},{"line_number":605,"context_line":"            mock_rados_client.return_value.__enter__.return_value \u003d client"},{"line_number":606,"context_line":"            image \u003d self.service.rbd.Image()"}],"source_content_type":"text/x-python","patch_set":9,"id":"bfdaf3ff_e75a88f1","line":603,"range":{"start_line":600,"start_character":8,"end_line":603,"end_character":64},"updated":"2019-01-16 07:24:36.000000000","message":"kindly use paranthesis \u0027()\u0027 to wrap code instead of \u0027\\\u0027","commit_id":"c21778b196f435f68c0d593a1a1f9d0bcdf24eab"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"eb85f622cf437ea44adeb33227c3e1e86990d4f2","unresolved":false,"context_lines":[{"line_number":597,"context_line":""},{"line_number":598,"context_line":"        self.mock_rbd.RBD().list.return_value \u003d [backup_name]"},{"line_number":599,"context_line":""},{"line_number":600,"context_line":"        with mock.patch.object(self.service, \u0027_rbd_diff_transfer\u0027), \\"},{"line_number":601,"context_line":"                mock.patch.object(self.service, \u0027_create_base_image\u0027) as \\"},{"line_number":602,"context_line":"                mock_create_base_image, mock.patch.object("},{"line_number":603,"context_line":"                rbd_driver, \u0027RADOSClient\u0027) as mock_rados_client:"},{"line_number":604,"context_line":"            client \u003d mock.Mock()"},{"line_number":605,"context_line":"            mock_rados_client.return_value.__enter__.return_value \u003d client"},{"line_number":606,"context_line":"            image \u003d self.service.rbd.Image()"}],"source_content_type":"text/x-python","patch_set":9,"id":"9fdfeff1_98f4945d","line":603,"range":{"start_line":600,"start_character":8,"end_line":603,"end_character":64},"in_reply_to":"bfdaf3ff_e75a88f1","updated":"2019-01-19 06:16:57.000000000","message":"Try \u0027()\u0027, seems not to work well :(","commit_id":"c21778b196f435f68c0d593a1a1f9d0bcdf24eab"},{"author":{"_account_id":20813,"name":"Sofia Enriquez","email":"lsofia.enriquez@gmail.com","username":"enriquetaso"},"change_message_id":"4904d04dbf14a923267aac237b2068230150a7d6","unresolved":false,"context_lines":[{"line_number":465,"context_line":"        self.mock_rbd.RBD.list \u003d mock.Mock()"},{"line_number":466,"context_line":"        self.mock_rbd.RBD.list.return_value \u003d [backup_name]"},{"line_number":467,"context_line":""},{"line_number":468,"context_line":"        with mock.patch.object(self.service, \u0027_backup_metadata\u0027):"},{"line_number":469,"context_line":"            with mock.patch.object(self.service, \u0027get_backup_snaps\u0027) as \\"},{"line_number":470,"context_line":"                    mock_get_backup_snaps:"},{"line_number":471,"context_line":"                with mock.patch.object(self.service, \u0027_full_backup\u0027) as \\"},{"line_number":472,"context_line":"                        mock_full_backup:"},{"line_number":473,"context_line":"                    with mock.patch.object(self.service,"},{"line_number":474,"context_line":"                                           \u0027_try_delete_base_image\u0027):"},{"line_number":475,"context_line":"                        with tempfile.NamedTemporaryFile() as test_file:"},{"line_number":476,"context_line":"                            checksum \u003d hashlib.sha256()"},{"line_number":477,"context_line":"                            image \u003d self.service.rbd.Image()"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fdfeff1_0e792792","line":474,"range":{"start_line":468,"start_character":8,"end_line":474,"end_character":69},"updated":"2019-02-21 19:23:56.000000000","message":"This comment is just a suggestion.\nMaybe we can add \"_full_rbd_backup\" and then check `self.assertFalse(_full_rbd_backup.called)`.","commit_id":"00a613897da8556e51654d87b8340251dde5d4b6"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"7f0e7648f753a05f4842d4d6322f6578184419b5","unresolved":false,"context_lines":[{"line_number":172,"context_line":"        self.backup.container \u003d \"backups\""},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"        # Create alternate backup with parent"},{"line_number":175,"context_line":"        self.alt_backup_id \u003d str(uuid.uuid4())"},{"line_number":176,"context_line":"        self._create_backup_db_entry(self.alt_backup_id, self.volume_id,"},{"line_number":177,"context_line":"                                     self.volume_size)"},{"line_number":178,"context_line":""}],"source_content_type":"text/x-python","patch_set":37,"id":"ffb9cba7_48d7094d","line":175,"range":{"start_line":175,"start_character":33,"end_line":175,"end_character":45},"updated":"2019-04-23 15:19:15.000000000","message":"Should use fake_constants.BACKUP2_ID here.\n\nDynamically generating UUIDs in unit tests is generally discouraged because it makes it hard to write elastic recheck queries for unit test failures.\n\n(I see that this method does this a handful of times, maybe something good to fix in a separate patch.)","commit_id":"37fc5572d47943479d3963fa3a0b7035977638e5"},{"author":{"_account_id":8846,"name":"Hao Wang","display_name":"Hao Wang","email":"sxmatch1986@gmail.com","username":"sxmatch"},"change_message_id":"e7ddb9d20e29d7d33671e8525b88e5f28b4cc49b","unresolved":false,"context_lines":[{"line_number":172,"context_line":"        self.backup.container \u003d \"backups\""},{"line_number":173,"context_line":""},{"line_number":174,"context_line":"        # Create alternate backup with parent"},{"line_number":175,"context_line":"        self.alt_backup_id \u003d str(uuid.uuid4())"},{"line_number":176,"context_line":"        self._create_backup_db_entry(self.alt_backup_id, self.volume_id,"},{"line_number":177,"context_line":"                                     self.volume_size)"},{"line_number":178,"context_line":""}],"source_content_type":"text/x-python","patch_set":37,"id":"ffb9cba7_1df4cece","line":175,"range":{"start_line":175,"start_character":33,"end_line":175,"end_character":45},"in_reply_to":"ffb9cba7_48d7094d","updated":"2019-04-25 07:17:43.000000000","message":"Done","commit_id":"37fc5572d47943479d3963fa3a0b7035977638e5"}],"releasenotes/notes/support-incremental-backup-completion-in-rbd-1f2165fefcc470d1.yaml":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"1844919963baf33b92e0d86952dbbf78cb6f73b6","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"fixes:"},{"line_number":3,"context_line":"  - This fixes the bug that Ceph RBD backend ignores the `--incremental`"},{"line_number":4,"context_line":"    option when creating a volume backup. The first backup of a given volume"},{"line_number":5,"context_line":"    is always a full backup, and each subsequent backup is always an"},{"line_number":6,"context_line":"    incremental backup. Now Cinder supports to create a new full backup if the"}],"source_content_type":"text/x-yaml","patch_set":20,"id":"5fc1f717_a88eb9cd","line":3,"range":{"start_line":3,"start_character":4,"end_line":3,"end_character":8},"updated":"2019-03-20 20:02:49.000000000","message":"This what? There is no context in the resulting release notes.\n\nMight be better as:\n\nFixed issue where all Ceph RBD backups would be incremental after the first one. The driver now honors whether ``--incremental`` is specified or not.","commit_id":"07babdb687b519253c8d8a86d0022a7aab69bb44"}]}