)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e95109c73effc7fa52d926bada6bf625e885e4e0","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Eric M Gonzalez \u003ceric@awnix.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-12-20 10:43:21 -0600"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add support for EXCLUSIVE_LOCK and JOURNALING features"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Ceph Jewel introduced the ability to mirror pools. A requirement for"},{"line_number":10,"context_line":"this is that the RBD_FEATURE_EXCLUSIVE_LOCK and RBD_FEATURE_JOURNALING"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1a6eadb0_3ce68cb4","line":7,"updated":"2016-12-20 17:27:34.000000000","message":"-1: This summary is not good, no mention to Ceph backup","commit_id":"bf4cdd4c438afcdf78e892f27d09daf8a340d162"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"eac12c62791d773b34f5694cd7e9cad43b76fe49","unresolved":false,"context_lines":[{"line_number":13,"context_line":"are stated during backup image creation the \"rbd features default\""},{"line_number":14,"context_line":"setting of the Ceph cluster is ignored."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"This patch adds a new configuration option, \u0027backup_ceph_journals\u0027 which"},{"line_number":17,"context_line":"in turn allows the enabling of the new journaling and exclusive-lock"},{"line_number":18,"context_line":"features to be set on creation of a backup image. These new features"},{"line_number":19,"context_line":"allow a mirrored `cinder_backup` pool to automatically mirror the backup"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"da4df55a_8d223eb8","line":16,"range":{"start_line":16,"start_character":45,"end_line":16,"end_character":65},"updated":"2016-12-22 11:54:24.000000000","message":"-1: this needs an update","commit_id":"5e4f049dde4d5c7dc56b276453650164d61584db"}],"cinder/backup/drivers/ceph.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e95109c73effc7fa52d926bada6bf625e885e4e0","unresolved":false,"context_lines":[{"line_number":88,"context_line":"               help\u003d\u0027RBD stripe unit to use when creating a backup image.\u0027),"},{"line_number":89,"context_line":"    cfg.IntOpt(\u0027backup_ceph_stripe_count\u0027, default\u003d0,"},{"line_number":90,"context_line":"               help\u003d\u0027RBD stripe count to use when creating a backup image.\u0027),"},{"line_number":91,"context_line":"    cfg.BoolOpt(\"backup_ceph_journals\", default\u003dFalse,"},{"line_number":92,"context_line":"                help\u003d\"If True, apply JOURNALING and EXCLUSIVE_LOCK feature \""},{"line_number":93,"context_line":"                     \"bits to the backup RBD objects\"),"},{"line_number":94,"context_line":"    cfg.BoolOpt(\u0027restore_discard_excess_bytes\u0027, default\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a6eadb0_d70c933d","line":91,"updated":"2016-12-20 17:27:34.000000000","message":"-1: I think we should improve the name, maybe something like backup_ceph_per_image_replication since it speaks to the functionality provided to the user and not the ceph functionality required for it.","commit_id":"bf4cdd4c438afcdf78e892f27d09daf8a340d162"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e95109c73effc7fa52d926bada6bf625e885e4e0","unresolved":false,"context_lines":[{"line_number":228,"context_line":"    @property"},{"line_number":229,"context_line":"    def _supports_exclusive_lock(self):"},{"line_number":230,"context_line":"        \"\"\"Determine if exclusive-lock is supported by our version of librbd.\"\"\""},{"line_number":231,"context_line":"        return hasattr(self.rbd, \u0027RBD_FEATURE_EXCLUSIVE_LOCK\u0027)"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    @property"},{"line_number":234,"context_line":"    def _supports_journaling(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a6eadb0_77badfe2","line":231,"updated":"2016-12-20 17:27:34.000000000","message":"-1: afaik we don\u0027t require to check the exclusive lock","commit_id":"bf4cdd4c438afcdf78e892f27d09daf8a340d162"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"e95109c73effc7fa52d926bada6bf625e885e4e0","unresolved":false,"context_lines":[{"line_number":250,"context_line":"                old_format \u003d False"},{"line_number":251,"context_line":"                features |\u003d self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK"},{"line_number":252,"context_line":"                # journaling requires exclusive_lock"},{"line_number":253,"context_line":"                if self._supports_journaling:"},{"line_number":254,"context_line":"                    # old_format is already set to False"},{"line_number":255,"context_line":"                    features |\u003d self.rbd.RBD_FEATURE_JOURNALING"},{"line_number":256,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1a6eadb0_5772434a","line":253,"updated":"2016-12-20 17:27:34.000000000","message":"-1: this should be checked on init and if it\u0027s not supported we should raise an exception to prevent users from thinking that it\u0027s working when it\u0027s actually not.  And we should just return the feature here if the configuration option is enabled.","commit_id":"bf4cdd4c438afcdf78e892f27d09daf8a340d162"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0d175ec044c81cbdbe146cf6905064ff7da386b6","unresolved":false,"context_lines":[{"line_number":88,"context_line":"               help\u003d\u0027RBD stripe unit to use when creating a backup image.\u0027),"},{"line_number":89,"context_line":"    cfg.IntOpt(\u0027backup_ceph_stripe_count\u0027, default\u003d0,"},{"line_number":90,"context_line":"               help\u003d\u0027RBD stripe count to use when creating a backup image.\u0027),"},{"line_number":91,"context_line":"    cfg.BoolOpt(\u0027backup_ceph_journals\u0027, default\u003dFalse,"},{"line_number":92,"context_line":"                help\u003d\u0027If True, apply JOURNALING and EXCLUSIVE_LOCK feature \u0027"},{"line_number":93,"context_line":"                     \u0027bits to the backup RBD objects\u0027),"},{"line_number":94,"context_line":"    cfg.BoolOpt(\u0027restore_discard_excess_bytes\u0027, default\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a6eadb0_1b71b644","line":91,"updated":"2016-12-21 09:59:49.000000000","message":"-1: As I mentioned on patch 1, I don\u0027t think this is the best name for the configuration option.","commit_id":"806ccd771a33281bc938ddd4474ac7951c427689"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0d175ec044c81cbdbe146cf6905064ff7da386b6","unresolved":false,"context_lines":[{"line_number":245,"context_line":"        if self._supports_stripingv2:"},{"line_number":246,"context_line":"            old_format \u003d False"},{"line_number":247,"context_line":"            features |\u003d self.rbd.RBD_FEATURE_STRIPINGV2"},{"line_number":248,"context_line":"        if CONF.backup_ceph_journals:"},{"line_number":249,"context_line":"            if self._supports_exclusive_lock:"},{"line_number":250,"context_line":"                old_format \u003d False"},{"line_number":251,"context_line":"                features |\u003d self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK"},{"line_number":252,"context_line":"                # journaling requires exclusive_lock"},{"line_number":253,"context_line":"                if self._supports_journaling:"},{"line_number":254,"context_line":"                    # old_format is already set to False"},{"line_number":255,"context_line":"                    features |\u003d self.rbd.RBD_FEATURE_JOURNALING"},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"        return (old_format, features)"},{"line_number":258,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"1a6eadb0_bb96024a","line":255,"range":{"start_line":248,"start_character":0,"end_line":255,"end_character":63},"updated":"2016-12-21 09:59:49.000000000","message":"-1: I still believe this should be checked on driver initialization and stop execution if configuration option and librbd are not in sync.\n\nIn any case we only want to enable these features if we can enable both so it should at least be:\n\n if CONF.backup_ceph_journals:\n     if (self._supports_exclusive_lock and\n             self._supports_journaling):\n         old_format \u003d False\n         features |\u003d (self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK |\n                      self.rbd.RBD_FEATURE_JOURNALING)\n\nBut I insist we should allow the service to start if it\u0027s enabled but not supported, so this would en up as:\n\n if CONF.backup_ceph_journals:\n     old_format \u003d False\n     features |\u003d (self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK |\n                  self.rbd.RBD_FEATURE_JOURNALING)","commit_id":"806ccd771a33281bc938ddd4474ac7951c427689"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"eac12c62791d773b34f5694cd7e9cad43b76fe49","unresolved":false,"context_lines":[{"line_number":88,"context_line":"               help\u003d\u0027RBD stripe unit to use when creating a backup image.\u0027),"},{"line_number":89,"context_line":"    cfg.IntOpt(\u0027backup_ceph_stripe_count\u0027, default\u003d0,"},{"line_number":90,"context_line":"               help\u003d\u0027RBD stripe count to use when creating a backup image.\u0027),"},{"line_number":91,"context_line":"    cfg.BoolOpt(\u0027backup_ceph_per_image_replication\u0027, default\u003dFalse,"},{"line_number":92,"context_line":"                help\u003d\u0027If True, apply JOURNALING and EXCLUSIVE_LOCK feature \u0027"},{"line_number":93,"context_line":"                     \u0027bits to the backup RBD objects to allow mirroring\u0027),"},{"line_number":94,"context_line":"    cfg.BoolOpt(\u0027restore_discard_excess_bytes\u0027, default\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":4,"id":"da4df55a_ad4e22bf","line":91,"updated":"2016-12-22 11:54:24.000000000","message":"nit: The name I suggested was way too long... Do you have any better ideas?  All I can come up with is \"backup_ceph_replicates\"","commit_id":"5e4f049dde4d5c7dc56b276453650164d61584db"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"301610891259befec6256b856d7ad905affbaa17","unresolved":false,"context_lines":[{"line_number":88,"context_line":"               help\u003d\u0027RBD stripe unit to use when creating a backup image.\u0027),"},{"line_number":89,"context_line":"    cfg.IntOpt(\u0027backup_ceph_stripe_count\u0027, default\u003d0,"},{"line_number":90,"context_line":"               help\u003d\u0027RBD stripe count to use when creating a backup image.\u0027),"},{"line_number":91,"context_line":"    cfg.BoolOpt(\u0027backup_ceph_per_image_replication\u0027, default\u003dFalse,"},{"line_number":92,"context_line":"                help\u003d\u0027If True, apply JOURNALING and EXCLUSIVE_LOCK feature \u0027"},{"line_number":93,"context_line":"                     \u0027bits to the backup RBD objects to allow mirroring\u0027),"},{"line_number":94,"context_line":"    cfg.BoolOpt(\u0027restore_discard_excess_bytes\u0027, default\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a3c09a3_42434642","line":91,"in_reply_to":"da4df55a_a4638ab0","updated":"2017-01-17 11:24:08.000000000","message":"I like the new name, which now I realize is basically the one you had at the begining  :-)","commit_id":"5e4f049dde4d5c7dc56b276453650164d61584db"},{"author":{"_account_id":24638,"name":"Eric M Gonzalez (tasker)","email":"eric@awnix.com","username":"egrh3"},"change_message_id":"937d4bc887449a6b76614e4ba916a742845784ff","unresolved":false,"context_lines":[{"line_number":88,"context_line":"               help\u003d\u0027RBD stripe unit to use when creating a backup image.\u0027),"},{"line_number":89,"context_line":"    cfg.IntOpt(\u0027backup_ceph_stripe_count\u0027, default\u003d0,"},{"line_number":90,"context_line":"               help\u003d\u0027RBD stripe count to use when creating a backup image.\u0027),"},{"line_number":91,"context_line":"    cfg.BoolOpt(\u0027backup_ceph_per_image_replication\u0027, default\u003dFalse,"},{"line_number":92,"context_line":"                help\u003d\u0027If True, apply JOURNALING and EXCLUSIVE_LOCK feature \u0027"},{"line_number":93,"context_line":"                     \u0027bits to the backup RBD objects to allow mirroring\u0027),"},{"line_number":94,"context_line":"    cfg.BoolOpt(\u0027restore_discard_excess_bytes\u0027, default\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":4,"id":"da4df55a_a4638ab0","line":91,"in_reply_to":"da4df55a_ad4e22bf","updated":"2016-12-22 21:18:05.000000000","message":"not really.\n\nI thought of `backup_ceph_pool_mirroring`, but that still looks like it enables the mirroring instead of supporting it. I spent several minutes trying to thing of something better, but couldn\u0027t. perhaps something simpler -- something that speaks to what the config option actually does? maybe, \"backup_ceph_image_journals\"?","commit_id":"5e4f049dde4d5c7dc56b276453650164d61584db"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"eac12c62791d773b34f5694cd7e9cad43b76fe49","unresolved":false,"context_lines":[{"line_number":189,"context_line":"        if CONF.backup_ceph_per_image_replication and \\"},{"line_number":190,"context_line":"                not (self._supports_journaling and"},{"line_number":191,"context_line":"                     self._supports_exclusive_lock):"},{"line_number":192,"context_line":"            LOG.warning(_LW(\"RBD journaling not supported - unable to \""},{"line_number":193,"context_line":"                            \"support per image mirroring in backup pool\"))"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        self._ceph_backup_user \u003d utils.convert_str(CONF.backup_ceph_user)"}],"source_content_type":"text/x-python","patch_set":4,"id":"da4df55a_4d3a1638","line":192,"range":{"start_line":192,"start_character":16,"end_line":192,"end_character":23},"updated":"2016-12-22 11:54:24.000000000","message":"-1: This should be logged as an error, since it\u0027s a big one.","commit_id":"5e4f049dde4d5c7dc56b276453650164d61584db"},{"author":{"_account_id":24638,"name":"Eric M Gonzalez (tasker)","email":"eric@awnix.com","username":"egrh3"},"change_message_id":"937d4bc887449a6b76614e4ba916a742845784ff","unresolved":false,"context_lines":[{"line_number":189,"context_line":"        if CONF.backup_ceph_per_image_replication and \\"},{"line_number":190,"context_line":"                not (self._supports_journaling and"},{"line_number":191,"context_line":"                     self._supports_exclusive_lock):"},{"line_number":192,"context_line":"            LOG.warning(_LW(\"RBD journaling not supported - unable to \""},{"line_number":193,"context_line":"                            \"support per image mirroring in backup pool\"))"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"        self._ceph_backup_user \u003d utils.convert_str(CONF.backup_ceph_user)"}],"source_content_type":"text/x-python","patch_set":4,"id":"da4df55a_04cff67f","line":192,"range":{"start_line":192,"start_character":16,"end_line":192,"end_character":23},"in_reply_to":"da4df55a_4d3a1638","updated":"2016-12-22 21:18:05.000000000","message":"It is big, but I tend to thing of errors as \"stop the press\" kind of events. I don\u0027t think the backup should fail or refuse to run if the feature isn\u0027t set. If I tell cinder ..\n\n.. I see your argument now. I want a backup and expected this feature to be set, and it wasn\u0027t. This is not the backup I wanted.\n\nYea, I can make an error and raise an exception.","commit_id":"5e4f049dde4d5c7dc56b276453650164d61584db"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"eac12c62791d773b34f5694cd7e9cad43b76fe49","unresolved":false,"context_lines":[{"line_number":254,"context_line":""},{"line_number":255,"context_line":"        if CONF.backup_ceph_per_image_replication:"},{"line_number":256,"context_line":"            # journaling requires exclusive_lock; check both together"},{"line_number":257,"context_line":"            if self._supports_exclusive_lock and self._supports_journaling:"},{"line_number":258,"context_line":"                old_format \u003d False"},{"line_number":259,"context_line":"                features |\u003d (self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK |"},{"line_number":260,"context_line":"                             self.rbd.RBD_FEATURE_JOURNALING)"}],"source_content_type":"text/x-python","patch_set":4,"id":"da4df55a_6d26fa17","line":257,"updated":"2016-12-22 11:54:24.000000000","message":"-1: This should go in the same check as L255","commit_id":"5e4f049dde4d5c7dc56b276453650164d61584db"},{"author":{"_account_id":24638,"name":"Eric M Gonzalez (tasker)","email":"eric@awnix.com","username":"egrh3"},"change_message_id":"937d4bc887449a6b76614e4ba916a742845784ff","unresolved":false,"context_lines":[{"line_number":254,"context_line":""},{"line_number":255,"context_line":"        if CONF.backup_ceph_per_image_replication:"},{"line_number":256,"context_line":"            # journaling requires exclusive_lock; check both together"},{"line_number":257,"context_line":"            if self._supports_exclusive_lock and self._supports_journaling:"},{"line_number":258,"context_line":"                old_format \u003d False"},{"line_number":259,"context_line":"                features |\u003d (self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK |"},{"line_number":260,"context_line":"                             self.rbd.RBD_FEATURE_JOURNALING)"}],"source_content_type":"text/x-python","patch_set":4,"id":"da4df55a_c426ce67","line":257,"in_reply_to":"da4df55a_6d26fa17","updated":"2016-12-22 21:18:05.000000000","message":"ahh. good idea. wilco.","commit_id":"5e4f049dde4d5c7dc56b276453650164d61584db"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"74f18d448770732286f8ac92d5aecb23946b7ba5","unresolved":false,"context_lines":[{"line_number":253,"context_line":"                features |\u003d (self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK |"},{"line_number":254,"context_line":"                             self.rbd.RBD_FEATURE_JOURNALING)"},{"line_number":255,"context_line":"            else:"},{"line_number":256,"context_line":"                LOG.error(_LE(\"RBD journaling not supported - unable to \""},{"line_number":257,"context_line":"                              \"support per image mirroring in backup pool\"))"},{"line_number":258,"context_line":"                raise exception.BackupInvalidCephArgs("},{"line_number":259,"context_line":"                    _(\"Image Journaling set but RBD backend does \""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_62dcc273","line":256,"updated":"2017-01-17 11:25:07.000000000","message":"-1: I think this error should go in the driver initialization and prevent the service from starting, since we\u0027ll never be able to get a valid backup and this is an issue that the administrator should see quickly and resolve, and it should never reach the user.","commit_id":"8d00bde54c644961c77f269cd09b1ab8f8151b08"},{"author":{"_account_id":24638,"name":"Eric M Gonzalez (tasker)","email":"eric@awnix.com","username":"egrh3"},"change_message_id":"59a28535b231fc7418ba6a1fefaaac6c973be7a2","unresolved":false,"context_lines":[{"line_number":253,"context_line":"                features |\u003d (self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK |"},{"line_number":254,"context_line":"                             self.rbd.RBD_FEATURE_JOURNALING)"},{"line_number":255,"context_line":"            else:"},{"line_number":256,"context_line":"                LOG.error(_LE(\"RBD journaling not supported - unable to \""},{"line_number":257,"context_line":"                              \"support per image mirroring in backup pool\"))"},{"line_number":258,"context_line":"                raise exception.BackupInvalidCephArgs("},{"line_number":259,"context_line":"                    _(\"Image Journaling set but RBD backend does \""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_d35c437f","line":256,"in_reply_to":"7a3c09a3_62dcc273","updated":"2017-01-18 00:05:24.000000000","message":"I\u0027ve moved the exception into the driver\u0027s initialization, but my testing has shown that the backup service still continues to start despite the change. Looking into it, I\u0027ve noticed that the manager doesn\u0027t ask for the driver until it attempts an operation. (cf. cinder/backup/manager#L425, run_backup() ).\n\nIn order to get the service to cease starting in the event that the RBD version does not support the new features, I believe that I\u0027ll have to rewrite the backup_manager to load the driver during the manager\u0027s initialization. I fear that this: a) might have an impact on the other drivers that Cinder Backup supports, b) is beyond the scope of this patch, and c) works counter to the way I understand the design of the abstraction of the manager and the drivers.\n\nDo you know of another driver that does something like this?\n\nI understand what you\u0027re trying to get at by having the error in the administrator\u0027s hands (admin gives the GO/NO-GO) and the users never seeing it (the service \"just works\" or \"doesn\u0027t\"), but I don\u0027t know of a way to achieve this. Within that absence of knowledge, I compromised by implementing the exception inside _get_rbd_support() so that the backup service would still operate on any extisting backup images (for deletion or restoration) and only fail if a new backup was attempted. If the backup creation fails, the backup metadata is updated to \"error\", the logs will state what happened, and the failed backup could be deleted.","commit_id":"8d00bde54c644961c77f269cd09b1ab8f8151b08"},{"author":{"_account_id":24638,"name":"Eric M Gonzalez (tasker)","email":"eric@awnix.com","username":"egrh3"},"change_message_id":"3efcf492fc503240a60b4100df57540fa2d189cd","unresolved":false,"context_lines":[{"line_number":253,"context_line":"                features |\u003d (self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK |"},{"line_number":254,"context_line":"                             self.rbd.RBD_FEATURE_JOURNALING)"},{"line_number":255,"context_line":"            else:"},{"line_number":256,"context_line":"                LOG.error(_LE(\"RBD journaling not supported - unable to \""},{"line_number":257,"context_line":"                              \"support per image mirroring in backup pool\"))"},{"line_number":258,"context_line":"                raise exception.BackupInvalidCephArgs("},{"line_number":259,"context_line":"                    _(\"Image Journaling set but RBD backend does \""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_d39b6364","line":256,"in_reply_to":"7a3c09a3_62dcc273","updated":"2017-01-18 00:02:36.000000000","message":"I\u0027ve moved the exception into the driver\u0027s initialization, but my testing has shown that the backup service still continues to start despite the change. Looking into it, I\u0027ve noticed that the manager doesn\u0027t ask for the driver until it attempts an operation. (cf. cinder/backup/manager#L425, run_backup() ).\n\nIn order to get the service to cease starting in the event that the RBD version does not support the new features, I believe that I\u0027ll have to rewrite the backup_manager to load the driver during the manager\u0027s initialization. I fear that this: a) might have an impact on the other drivers that Cinder Backup supports, b) is beyond the scope of this patch, and c) works counter to the way I understand the design of the abstraction of the manager and the drivers.\n\nDo you know of another driver that does something like this?\n\nI understand what you\u0027re trying to get at by having the error in the administrator\u0027s hands (admin gives the GO/NO-GO) and the users never seeing it (the service \"just works\" or \"doesn\u0027t\"), but I don\u0027t know of a way to achieve this. Within that absence of knowledge, I compromised by implementing the exception inside _get_rbd_support() so that the backup service would still operate on any extisting backup images (for deletion or restoration) and only fail if a new backup was attempted. If the backup creation fails, the backup metadata is updated to \"error\", the logs will state what happened, and the failed backup could be deleted.","commit_id":"8d00bde54c644961c77f269cd09b1ab8f8151b08"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6b58f124c130c77b3fe3c455ac560823cc33caf3","unresolved":false,"context_lines":[{"line_number":253,"context_line":"                features |\u003d (self.rbd.RBD_FEATURE_EXCLUSIVE_LOCK |"},{"line_number":254,"context_line":"                             self.rbd.RBD_FEATURE_JOURNALING)"},{"line_number":255,"context_line":"            else:"},{"line_number":256,"context_line":"                LOG.error(_LE(\"RBD journaling not supported - unable to \""},{"line_number":257,"context_line":"                              \"support per image mirroring in backup pool\"))"},{"line_number":258,"context_line":"                raise exception.BackupInvalidCephArgs("},{"line_number":259,"context_line":"                    _(\"Image Journaling set but RBD backend does \""}],"source_content_type":"text/x-python","patch_set":5,"id":"7a3c09a3_60b8fa75","line":256,"in_reply_to":"7a3c09a3_d35c437f","updated":"2017-01-18 11:12:10.000000000","message":"I think we can accept this as a temporary fix, since it actually fixes the issue, until we implement the required mechanism to prevent the service from starting, but then we need to have a FIXME comment indicating that this should be done at the backup service start to prevent the service from starting altogether.","commit_id":"8d00bde54c644961c77f269cd09b1ab8f8151b08"}],"releasenotes/notes/backup-ceph-driver-journaling-exculsive-lock-features-6b6044138a288a83.yaml":[{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"d7eb85e0276941db9fa0bd1fc9162700b8f0ff7f","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"prelude: \u003e"},{"line_number":3,"context_line":"  - Ceph Jewel introduced the ability to mirror pools. A requirement for this"},{"line_number":4,"context_line":"    is that the RBD_FEATURE_EXCLUSIVE_LOCK and RBD_FEATURE_JOURNALING bits be"},{"line_number":5,"context_line":"    set per image. The Ceph backup driver explicitly sets only the "}],"source_content_type":"text/x-yaml","patch_set":2,"id":"1a6eadb0_2002da0b","line":2,"range":{"start_line":2,"start_character":8,"end_line":2,"end_character":10},"updated":"2016-12-20 20:48:49.000000000","message":"??","commit_id":"4350516b7fc3a10c4f8bd144bcb982829ccd1e81"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"d7eb85e0276941db9fa0bd1fc9162700b8f0ff7f","unresolved":false,"context_lines":[{"line_number":2,"context_line":"prelude: \u003e"},{"line_number":3,"context_line":"  - Ceph Jewel introduced the ability to mirror pools. A requirement for this"},{"line_number":4,"context_line":"    is that the RBD_FEATURE_EXCLUSIVE_LOCK and RBD_FEATURE_JOURNALING bits be"},{"line_number":5,"context_line":"    set per image. The Ceph backup driver explicitly sets only the "},{"line_number":6,"context_line":"    RBD_FEATURE_STRIPINGV2 and/or RBD_FEATURE_LAYERING features. When these are"},{"line_number":7,"context_line":"    stated during backup image creation the \"rbd features default\" setting of"},{"line_number":8,"context_line":"    the Ceph cluster is ignored."}],"source_content_type":"text/x-yaml","patch_set":2,"id":"1a6eadb0_00307e00","line":5,"range":{"start_line":5,"start_character":66,"end_line":5,"end_character":67},"updated":"2016-12-20 20:48:49.000000000","message":"Trailling","commit_id":"4350516b7fc3a10c4f8bd144bcb982829ccd1e81"},{"author":{"_account_id":10058,"name":"Erlon R. Cruz","email":"erlon.rodrigues.cruz@canonical.com","username":"sombrafam"},"change_message_id":"d7eb85e0276941db9fa0bd1fc9162700b8f0ff7f","unresolved":false,"context_lines":[{"line_number":11,"context_line":"    turn allows the enabling of the new journaling and exclusive-lock features"},{"line_number":12,"context_line":"    to be set on creation of a backup image. These new features allow a mirrored"},{"line_number":13,"context_line":"    `cinder_backup` pool to automatically mirror the backup images per the Ceph"},{"line_number":14,"context_line":"    mirror settings. "},{"line_number":15,"context_line":"features:"},{"line_number":16,"context_line":"  - add RBD_FEATURE_EXCLUSIVE_LOCK to new images"},{"line_number":17,"context_line":"  - add RBD_FEATURE_JOURNALING to new images"}],"source_content_type":"text/x-yaml","patch_set":2,"id":"1a6eadb0_8012ce60","line":14,"range":{"start_line":14,"start_character":20,"end_line":14,"end_character":21},"updated":"2016-12-20 20:48:49.000000000","message":"same","commit_id":"4350516b7fc3a10c4f8bd144bcb982829ccd1e81"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0d175ec044c81cbdbe146cf6905064ff7da386b6","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"prelude: \u003e"},{"line_number":3,"context_line":"  - Ceph Jewel introduced the ability to mirror pools. A requirement for this"},{"line_number":4,"context_line":"    is that the RBD_FEATURE_EXCLUSIVE_LOCK and RBD_FEATURE_JOURNALING bits be"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"1a6eadb0_9b2226a2","line":1,"updated":"2016-12-21 09:59:49.000000000","message":"-1: This release note file is not correct.  The idea behind our release notes is that they are all merged into a global list [1], so not all release notes need all sections.  In this case it should only contain a features section, no prelude and certainly no issues (since no issue is introduced in the release by this patch that users should be aware).\n\nAnd the feature should be only 1 bullet point describing what\u0027s the new feature as in \"ceph backup driver supports pool mirroring via configuration option yyyyy\"\n\n[1] http://docs.openstack.org/releasenotes/cinder/newton.html","commit_id":"806ccd771a33281bc938ddd4474ac7951c427689"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0d175ec044c81cbdbe146cf6905064ff7da386b6","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"prelude: \u003e"},{"line_number":3,"context_line":"  - Ceph Jewel introduced the ability to mirror pools. A requirement for this"},{"line_number":4,"context_line":"    is that the RBD_FEATURE_EXCLUSIVE_LOCK and RBD_FEATURE_JOURNALING bits be"},{"line_number":5,"context_line":"    set per image. The Ceph backup driver explicitly sets only the "}],"source_content_type":"text/x-yaml","patch_set":3,"id":"1a6eadb0_e0d9abf4","line":2,"range":{"start_line":2,"start_character":9,"end_line":2,"end_character":10},"updated":"2016-12-21 09:59:49.000000000","message":"-1: This needs to be removed","commit_id":"806ccd771a33281bc938ddd4474ac7951c427689"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0d175ec044c81cbdbe146cf6905064ff7da386b6","unresolved":false,"context_lines":[{"line_number":2,"context_line":"prelude: \u003e"},{"line_number":3,"context_line":"  - Ceph Jewel introduced the ability to mirror pools. A requirement for this"},{"line_number":4,"context_line":"    is that the RBD_FEATURE_EXCLUSIVE_LOCK and RBD_FEATURE_JOURNALING bits be"},{"line_number":5,"context_line":"    set per image. The Ceph backup driver explicitly sets only the "},{"line_number":6,"context_line":"    RBD_FEATURE_STRIPINGV2 and/or RBD_FEATURE_LAYERING features. When these are"},{"line_number":7,"context_line":"    stated during backup image creation the \"rbd features default\" setting of"},{"line_number":8,"context_line":"    the Ceph cluster is ignored."}],"source_content_type":"text/x-yaml","patch_set":3,"id":"1a6eadb0_a037b34b","line":5,"range":{"start_line":5,"start_character":66,"end_line":5,"end_character":67},"updated":"2016-12-21 09:59:49.000000000","message":"nit: trailing space","commit_id":"806ccd771a33281bc938ddd4474ac7951c427689"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0d175ec044c81cbdbe146cf6905064ff7da386b6","unresolved":false,"context_lines":[{"line_number":11,"context_line":"    turn allows the enabling of the new journaling and exclusive-lock features"},{"line_number":12,"context_line":"    to be set on creation of a backup image. These new features allow a mirrored"},{"line_number":13,"context_line":"    `cinder_backup` pool to automatically mirror the backup images per the Ceph"},{"line_number":14,"context_line":"    mirror settings. "},{"line_number":15,"context_line":"features:"},{"line_number":16,"context_line":"  - add RBD_FEATURE_EXCLUSIVE_LOCK to new images"},{"line_number":17,"context_line":"  - add RBD_FEATURE_JOURNALING to new images"}],"source_content_type":"text/x-yaml","patch_set":3,"id":"1a6eadb0_40133fb8","line":14,"range":{"start_line":14,"start_character":20,"end_line":14,"end_character":21},"updated":"2016-12-21 09:59:49.000000000","message":"nit: trailing space","commit_id":"806ccd771a33281bc938ddd4474ac7951c427689"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"91ccb85b458be300a8524b13bcbb9568e709b0c6","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"    - Add new BoolOpt ``backup_ceph_image_journals`` for enabling the"},{"line_number":4,"context_line":"      image features required to support mirroring of Cinder backup pool."}],"source_content_type":"text/x-yaml","patch_set":6,"id":"fa31d9ce_e0a25ffb","line":3,"range":{"start_line":3,"start_character":0,"end_line":3,"end_character":4},"updated":"2017-02-14 15:28:14.000000000","message":"Should just be 2 spaces.\n\ns/Add/Added/","commit_id":"4b09ab3b588c39739b6df91a40126a4b6180d1c0"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"91ccb85b458be300a8524b13bcbb9568e709b0c6","unresolved":false,"context_lines":[{"line_number":1,"context_line":"---"},{"line_number":2,"context_line":"features:"},{"line_number":3,"context_line":"    - Add new BoolOpt ``backup_ceph_image_journals`` for enabling the"},{"line_number":4,"context_line":"      image features required to support mirroring of Cinder backup pool."}],"source_content_type":"text/x-yaml","patch_set":6,"id":"fa31d9ce_a0d14759","line":4,"updated":"2017-02-14 15:28:14.000000000","message":"Can you add \"Ceph\" in here to make it clear which driver this is for, even though that should be obvious based on the config option name?","commit_id":"4b09ab3b588c39739b6df91a40126a4b6180d1c0"}]}