)]}'
{"cinder/backup/drivers/vzstorage.py":[{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"61d4a9b64af573019075f10a6dd1883ddd59d833","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        self.backup_mount_perms \u003d CONF.vzstorage_backup_mount_perms"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"        if not self.backup_share:"},{"line_number":66,"context_line":"            raise exception.ConfigNotFound(_("},{"line_number":67,"context_line":"                \u0027Required option vzstorage_backup_share is not set\u0027))"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        backup_path \u003d self._init_backup_path()"}],"source_content_type":"text/x-python","patch_set":9,"id":"5ff73747_e0e053cf","line":66,"updated":"2017-05-03 13:28:05.000000000","message":"Please, use InvalidConfigurationValue here","commit_id":"ac89db5885497a162b705fa4447775f2fbaf3064"},{"author":{"_account_id":21284,"name":"Pavel Gluschak (scsnow)","email":"pglushchak@virtuozzo.com","username":"scsnow"},"change_message_id":"9e955afd25740e4a23724812cec12c85c2235eb0","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        self.backup_mount_perms \u003d CONF.vzstorage_backup_mount_perms"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"        if not self.backup_share:"},{"line_number":66,"context_line":"            raise exception.ConfigNotFound(_("},{"line_number":67,"context_line":"                \u0027Required option vzstorage_backup_share is not set\u0027))"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"        backup_path \u003d self._init_backup_path()"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_ff22b7b8","line":66,"in_reply_to":"5ff73747_e0e053cf","updated":"2017-05-11 15:03:56.000000000","message":"Done","commit_id":"ac89db5885497a162b705fa4447775f2fbaf3064"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"a0ba7d7558b2550e7961a6053e5710832541c9f5","unresolved":false,"context_lines":[{"line_number":34,"context_line":"vzstorage_backup_service_opts \u003d ["},{"line_number":35,"context_line":"    cfg.StrOpt(\u0027vzstorage_backup_share\u0027,"},{"line_number":36,"context_line":"               help\u003d(\u0027VzStorage share in format \u0027"},{"line_number":37,"context_line":"                     \u0027[MDS1[,MDS2],...:/]\u003cCLUSTER NAME\u003e[:PASSWORD]\u0027)),"},{"line_number":38,"context_line":"    cfg.StrOpt(\u0027vzstorage_backup_mount_point\u0027,"},{"line_number":39,"context_line":"               default\u003d\u0027$state_path/backup_mount\u0027,"},{"line_number":40,"context_line":"               help\u003d\u0027Mount point for VzStorage share.\u0027),"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f436f4f_af7f2ca9","line":37,"updated":"2017-08-15 18:43:37.000000000","message":"If this option can include a password, it needs to be marked secret\u003dTrue.\n\nI would suggest including the password as a separate option so the backup_share can still be logged.","commit_id":"b05fa4acf872dc740ca4d48e14b43b62ff6e2292"},{"author":{"_account_id":21284,"name":"Pavel Gluschak (scsnow)","email":"pglushchak@virtuozzo.com","username":"scsnow"},"change_message_id":"04fa143d6c01cfbaf03f4717d501617c4d346043","unresolved":false,"context_lines":[{"line_number":34,"context_line":"vzstorage_backup_service_opts \u003d ["},{"line_number":35,"context_line":"    cfg.StrOpt(\u0027vzstorage_backup_share\u0027,"},{"line_number":36,"context_line":"               help\u003d(\u0027VzStorage share in format \u0027"},{"line_number":37,"context_line":"                     \u0027[MDS1[,MDS2],...:/]\u003cCLUSTER NAME\u003e[:PASSWORD]\u0027)),"},{"line_number":38,"context_line":"    cfg.StrOpt(\u0027vzstorage_backup_mount_point\u0027,"},{"line_number":39,"context_line":"               default\u003d\u0027$state_path/backup_mount\u0027,"},{"line_number":40,"context_line":"               help\u003d\u0027Mount point for VzStorage share.\u0027),"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f436f4f_82b308e9","line":37,"in_reply_to":"9f436f4f_af7f2ca9","updated":"2017-08-16 14:43:52.000000000","message":"I agree to mark this option with secret.\nCurrently it\u0027s not possible to make password as a separate option here, because this change would require RemoteFSClient rework.","commit_id":"b05fa4acf872dc740ca4d48e14b43b62ff6e2292"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"a0ba7d7558b2550e7961a6053e5710832541c9f5","unresolved":false,"context_lines":[{"line_number":49,"context_line":"               help\u003d\u0027Mount permissions for vstorage-mount.\u0027),"},{"line_number":50,"context_line":"    cfg.StrOpt(\u0027vzstorage_backup_log_path\u0027,"},{"line_number":51,"context_line":"               default\u003d\u0027/var/log/vstorage/cinder-backup.log.gz\u0027,"},{"line_number":52,"context_line":"               help\u003d\u0027Log path used by vstorage-mount.\u0027),"},{"line_number":53,"context_line":"]"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f436f4f_8f534839","line":52,"updated":"2017-08-15 18:43:37.000000000","message":"Why is this not in the Cinder log dir by default?","commit_id":"b05fa4acf872dc740ca4d48e14b43b62ff6e2292"},{"author":{"_account_id":21284,"name":"Pavel Gluschak (scsnow)","email":"pglushchak@virtuozzo.com","username":"scsnow"},"change_message_id":"04fa143d6c01cfbaf03f4717d501617c4d346043","unresolved":false,"context_lines":[{"line_number":49,"context_line":"               help\u003d\u0027Mount permissions for vstorage-mount.\u0027),"},{"line_number":50,"context_line":"    cfg.StrOpt(\u0027vzstorage_backup_log_path\u0027,"},{"line_number":51,"context_line":"               default\u003d\u0027/var/log/vstorage/cinder-backup.log.gz\u0027,"},{"line_number":52,"context_line":"               help\u003d\u0027Log path used by vstorage-mount.\u0027),"},{"line_number":53,"context_line":"]"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":15,"id":"9f436f4f_622a64db","line":52,"in_reply_to":"9f436f4f_8f534839","updated":"2017-08-16 14:43:52.000000000","message":"Because both vstorage volume backend and nova stores vstorage logs under /var/log/vstorage. Also this log path is used by vstorage itself. So I don\u0027t want to mix up things.","commit_id":"b05fa4acf872dc740ca4d48e14b43b62ff6e2292"},{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"e518fd0a6520ab024b3e445bff12446b8e73f95e","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        self.backup_mount_perms \u003d CONF.vzstorage_backup_mount_perms"},{"line_number":68,"context_line":"        self.backup_log_path \u003d CONF.vzstorage_backup_log_path"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        if not self.backup_share:"},{"line_number":71,"context_line":"            raise exception.InvalidConfigurationValue(_("},{"line_number":72,"context_line":"                \u0027Required option vzstorage_backup_share is not set\u0027))"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        backup_path \u003d self._init_backup_path()"},{"line_number":75,"context_line":"        LOG.debug(\"Using VzStorage backup path: %s\", backup_path)"}],"source_content_type":"text/x-python","patch_set":18,"id":"9f436f4f_5ca357b2","line":72,"range":{"start_line":70,"start_character":8,"end_line":72,"end_character":68},"updated":"2017-08-21 13:02:10.000000000","message":"Please, move it to check_for_setup_error method [1]\n\n[1] https://github.com/openstack/cinder/blob/master/cinder/interface/backup_driver.py#L135","commit_id":"67ced0b3e89ce97e0336984e37e5c7dac881fe61"},{"author":{"_account_id":21284,"name":"Pavel Gluschak (scsnow)","email":"pglushchak@virtuozzo.com","username":"scsnow"},"change_message_id":"0301e1bde9c9910403b277639a7e17a3e1e167c1","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        self.backup_mount_perms \u003d CONF.vzstorage_backup_mount_perms"},{"line_number":68,"context_line":"        self.backup_log_path \u003d CONF.vzstorage_backup_log_path"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        if not self.backup_share:"},{"line_number":71,"context_line":"            raise exception.InvalidConfigurationValue(_("},{"line_number":72,"context_line":"                \u0027Required option vzstorage_backup_share is not set\u0027))"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        backup_path \u003d self._init_backup_path()"},{"line_number":75,"context_line":"        LOG.debug(\"Using VzStorage backup path: %s\", backup_path)"}],"source_content_type":"text/x-python","patch_set":18,"id":"7f287b81_fd97a58f","line":72,"range":{"start_line":70,"start_character":8,"end_line":72,"end_character":68},"in_reply_to":"7f287b81_1a86f7dc","updated":"2017-08-24 16:13:40.000000000","message":"This is required for PosixBackupDriver, that we inherit from.","commit_id":"67ced0b3e89ce97e0336984e37e5c7dac881fe61"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"d5a26529ff2118460bcdc34075ce47b9a8e9158b","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        self.backup_mount_perms \u003d CONF.vzstorage_backup_mount_perms"},{"line_number":68,"context_line":"        self.backup_log_path \u003d CONF.vzstorage_backup_log_path"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        if not self.backup_share:"},{"line_number":71,"context_line":"            raise exception.InvalidConfigurationValue(_("},{"line_number":72,"context_line":"                \u0027Required option vzstorage_backup_share is not set\u0027))"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        backup_path \u003d self._init_backup_path()"},{"line_number":75,"context_line":"        LOG.debug(\"Using VzStorage backup path: %s\", backup_path)"}],"source_content_type":"text/x-python","patch_set":18,"id":"7f287b81_1a86f7dc","line":72,"range":{"start_line":70,"start_character":8,"end_line":72,"end_character":68},"in_reply_to":"7f287b81_bdb72b3b","updated":"2017-08-24 15:59:01.000000000","message":"\u003e However valid backup_share is needed for class instantiation.\n\nThis should not be the case.","commit_id":"67ced0b3e89ce97e0336984e37e5c7dac881fe61"},{"author":{"_account_id":21284,"name":"Pavel Gluschak (scsnow)","email":"pglushchak@virtuozzo.com","username":"scsnow"},"change_message_id":"735ee0ccb36e3179fd3c5280098e602c69d5f79f","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        self.backup_mount_perms \u003d CONF.vzstorage_backup_mount_perms"},{"line_number":68,"context_line":"        self.backup_log_path \u003d CONF.vzstorage_backup_log_path"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"        if not self.backup_share:"},{"line_number":71,"context_line":"            raise exception.InvalidConfigurationValue(_("},{"line_number":72,"context_line":"                \u0027Required option vzstorage_backup_share is not set\u0027))"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        backup_path \u003d self._init_backup_path()"},{"line_number":75,"context_line":"        LOG.debug(\"Using VzStorage backup path: %s\", backup_path)"}],"source_content_type":"text/x-python","patch_set":18,"id":"7f287b81_bdb72b3b","line":72,"range":{"start_line":70,"start_character":8,"end_line":72,"end_character":68},"in_reply_to":"9f436f4f_5ca357b2","updated":"2017-08-22 11:16:47.000000000","message":"I cannot simply move this check in check_for_setup_error method because it is called when driver class is already instantiated [1]. However valid backup_share is needed for class instantiation.\n\n[1] https://github.com/openstack/cinder/blob/master/cinder/backup/manager.py#L130-L131","commit_id":"67ced0b3e89ce97e0336984e37e5c7dac881fe61"},{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"d5a26529ff2118460bcdc34075ce47b9a8e9158b","unresolved":false,"context_lines":[{"line_number":71,"context_line":"            raise exception.InvalidConfigurationValue(_("},{"line_number":72,"context_line":"                \u0027Required option vzstorage_backup_share is not set\u0027))"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        backup_path \u003d self._init_backup_path()"},{"line_number":75,"context_line":"        LOG.debug(\"Using VzStorage backup path: %s\", backup_path)"},{"line_number":76,"context_line":"        super(VzStorageBackupDriver, self).__init__(context,"},{"line_number":77,"context_line":"                                                    db\u003ddb,"}],"source_content_type":"text/x-python","patch_set":18,"id":"7f287b81_da674f51","line":74,"range":{"start_line":74,"start_character":8,"end_line":74,"end_character":46},"updated":"2017-08-24 15:59:01.000000000","message":"This should not be done in __init__.","commit_id":"67ced0b3e89ce97e0336984e37e5c7dac881fe61"},{"author":{"_account_id":21284,"name":"Pavel Gluschak (scsnow)","email":"pglushchak@virtuozzo.com","username":"scsnow"},"change_message_id":"bc7670a33e90681a74fc26c4132379f3faaedec4","unresolved":false,"context_lines":[{"line_number":71,"context_line":"            raise exception.InvalidConfigurationValue(_("},{"line_number":72,"context_line":"                \u0027Required option vzstorage_backup_share is not set\u0027))"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        backup_path \u003d self._init_backup_path()"},{"line_number":75,"context_line":"        LOG.debug(\"Using VzStorage backup path: %s\", backup_path)"},{"line_number":76,"context_line":"        super(VzStorageBackupDriver, self).__init__(context,"},{"line_number":77,"context_line":"                                                    db\u003ddb,"}],"source_content_type":"text/x-python","patch_set":18,"id":"7f287b81_1da831d9","line":74,"range":{"start_line":74,"start_character":8,"end_line":74,"end_character":46},"in_reply_to":"7f287b81_da674f51","updated":"2017-08-24 16:12:07.000000000","message":"So where should it be? NFS backup driver initializes share in constructor as well [1].\n\n[1] https://github.com/openstack/cinder/blob/master/cinder/backup/drivers/nfs.py#L55","commit_id":"67ced0b3e89ce97e0336984e37e5c7dac881fe61"}],"cinder/backup/manager.py":[{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"61d4a9b64af573019075f10a6dd1883ddd59d833","unresolved":false,"context_lines":[{"line_number":460,"context_line":"                                              backup_device.is_snapshot)"},{"line_number":461,"context_line":"            try:"},{"line_number":462,"context_line":"                device_path \u003d attach_info[\u0027device\u0027][\u0027path\u0027]"},{"line_number":463,"context_line":"                if (isinstance(device_path, six.string_types) and"},{"line_number":464,"context_line":"                        not os.path.isdir(device_path)):"},{"line_number":465,"context_line":"                    if backup_device.secure_enabled:"},{"line_number":466,"context_line":"                        with open(device_path) as device_file:"}],"source_content_type":"text/x-python","patch_set":9,"id":"5ff73747_0078dff5","line":463,"updated":"2017-05-03 13:28:05.000000000","message":"IMO, these changes are out os scope of this patch and should be done in a separate one","commit_id":"ac89db5885497a162b705fa4447775f2fbaf3064"},{"author":{"_account_id":21284,"name":"Pavel Gluschak (scsnow)","email":"pglushchak@virtuozzo.com","username":"scsnow"},"change_message_id":"9e955afd25740e4a23724812cec12c85c2235eb0","unresolved":false,"context_lines":[{"line_number":460,"context_line":"                                              backup_device.is_snapshot)"},{"line_number":461,"context_line":"            try:"},{"line_number":462,"context_line":"                device_path \u003d attach_info[\u0027device\u0027][\u0027path\u0027]"},{"line_number":463,"context_line":"                if (isinstance(device_path, six.string_types) and"},{"line_number":464,"context_line":"                        not os.path.isdir(device_path)):"},{"line_number":465,"context_line":"                    if backup_device.secure_enabled:"},{"line_number":466,"context_line":"                        with open(device_path) as device_file:"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_9f1bb37f","line":463,"in_reply_to":"5ff73747_0078dff5","updated":"2017-05-11 15:03:56.000000000","message":"Change-Id: I2e34cf6de49db853cacf85f3c3373fdbd64fed83","commit_id":"ac89db5885497a162b705fa4447775f2fbaf3064"}],"cinder/tests/unit/backup/drivers/test_backup_vzstorage.py":[{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"38ef1e555a3a83009da01718adb01ee36f88ed17","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"\"\"\"Tests for VzStorage backup driver.\"\"\""},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import io"},{"line_number":19,"context_line":"import mock"},{"line_number":20,"context_line":"import os"},{"line_number":21,"context_line":"from os_brick.remotefs import remotefs"}],"source_content_type":"text/x-python","patch_set":14,"id":"ff0f0b1f_9e8a8867","line":18,"updated":"2017-05-23 18:23:09.000000000","message":"io and os imports should be in a separate group first for standard imports.","commit_id":"e5df812eea50a08bc76c308cfcb7fac242c29da9"}],"cinder/tests/unit/backup/test_backup.py":[{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"61d4a9b64af573019075f10a6dd1883ddd59d833","unresolved":false,"context_lines":[{"line_number":578,"context_line":"    @mock.patch(\u0027cinder.volume.rpcapi.VolumeAPI.get_backup_device\u0027)"},{"line_number":579,"context_line":"    @mock.patch(\u0027cinder.utils.temporary_chown\u0027)"},{"line_number":580,"context_line":"    @mock.patch(\u0027six.moves.builtins.open\u0027)"},{"line_number":581,"context_line":"    @mock.patch.object(os.path, \u0027isdir\u0027, return_value\u003dFalse)"},{"line_number":582,"context_line":"    def test_create_backup(self, mock_isdir, mock_open, mock_temporary_chown,"},{"line_number":583,"context_line":"                           mock_get_backup_device, mock_get_conn):"},{"line_number":584,"context_line":"        \"\"\"Test normal backup creation.\"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"5ff73747_c05af781","line":581,"updated":"2017-05-03 13:28:05.000000000","message":"Please, add tests for the new code paths","commit_id":"ac89db5885497a162b705fa4447775f2fbaf3064"},{"author":{"_account_id":21284,"name":"Pavel Gluschak (scsnow)","email":"pglushchak@virtuozzo.com","username":"scsnow"},"change_message_id":"9e955afd25740e4a23724812cec12c85c2235eb0","unresolved":false,"context_lines":[{"line_number":578,"context_line":"    @mock.patch(\u0027cinder.volume.rpcapi.VolumeAPI.get_backup_device\u0027)"},{"line_number":579,"context_line":"    @mock.patch(\u0027cinder.utils.temporary_chown\u0027)"},{"line_number":580,"context_line":"    @mock.patch(\u0027six.moves.builtins.open\u0027)"},{"line_number":581,"context_line":"    @mock.patch.object(os.path, \u0027isdir\u0027, return_value\u003dFalse)"},{"line_number":582,"context_line":"    def test_create_backup(self, mock_isdir, mock_open, mock_temporary_chown,"},{"line_number":583,"context_line":"                           mock_get_backup_device, mock_get_conn):"},{"line_number":584,"context_line":"        \"\"\"Test normal backup creation.\"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"3f044301_13c7a298","line":581,"in_reply_to":"5ff73747_c05af781","updated":"2017-05-11 15:03:56.000000000","message":"Change-Id: I2e34cf6de49db853cacf85f3c3373fdbd64fed83","commit_id":"ac89db5885497a162b705fa4447775f2fbaf3064"}],"cinder/volume/drivers/vzstorage.py":[{"author":{"_account_id":4523,"name":"Eric Harney","email":"eharney@redhat.com","username":"eharney"},"change_message_id":"a0ba7d7558b2550e7961a6053e5710832541c9f5","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        ploop_dev \u003d m.group(1)"},{"line_number":142,"context_line":"        if self.chown:"},{"line_number":143,"context_line":"            uid \u003d os.getuid()"},{"line_number":144,"context_line":"            self.execute(\u0027chown\u0027, uid, ploop_dev, run_as_root\u003dTrue)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"        return ploop_dev"},{"line_number":147,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"9f436f4f_8fa58849","line":144,"updated":"2017-08-15 18:43:37.000000000","message":"Should the __exit__ method not chown the file back to the original owner?","commit_id":"b05fa4acf872dc740ca4d48e14b43b62ff6e2292"},{"author":{"_account_id":21284,"name":"Pavel Gluschak (scsnow)","email":"pglushchak@virtuozzo.com","username":"scsnow"},"change_message_id":"04fa143d6c01cfbaf03f4717d501617c4d346043","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        ploop_dev \u003d m.group(1)"},{"line_number":142,"context_line":"        if self.chown:"},{"line_number":143,"context_line":"            uid \u003d os.getuid()"},{"line_number":144,"context_line":"            self.execute(\u0027chown\u0027, uid, ploop_dev, run_as_root\u003dTrue)"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"        return ploop_dev"},{"line_number":147,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"9f436f4f_828428d4","line":144,"in_reply_to":"9f436f4f_8fa58849","updated":"2017-08-16 14:43:52.000000000","message":"This is not needed. `ploop unmount` (performed in __exit__) destroys ploop device file.","commit_id":"b05fa4acf872dc740ca4d48e14b43b62ff6e2292"}]}