)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"4bffa10782d5dae5dacc8c0abc50a52b6b81bb1b","unresolved":false,"context_lines":[{"line_number":10,"context_line":"is valid so enforce this in the driver."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Also validate access level since there is a utility"},{"line_number":13,"context_line":"routine to check boty access type and access level."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Closes-bug: #1816420"},{"line_number":16,"context_line":"Change-Id: I6c96f861b30ef7ccac05a7c199a62f0d69044c3a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"5fc1f717_b9f84d7c","line":13,"range":{"start_line":13,"start_character":17,"end_line":13,"end_character":21},"updated":"2019-03-20 22:26:00.000000000","message":"both","commit_id":"85f2fb993135fb3ff6cb8bbf4937732bfd7776bb"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"94413a552e0e8df793831b39c5ec0a4e19143b7b","unresolved":false,"context_lines":[{"line_number":10,"context_line":"is valid so enforce this in the driver."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Also validate access level since there is a utility"},{"line_number":13,"context_line":"routine to check boty access type and access level."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Closes-bug: #1816420"},{"line_number":16,"context_line":"Change-Id: I6c96f861b30ef7ccac05a7c199a62f0d69044c3a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"5fc1f717_b90ccd42","line":13,"range":{"start_line":13,"start_character":17,"end_line":13,"end_character":21},"in_reply_to":"5fc1f717_b9f84d7c","updated":"2019-03-20 22:45:15.000000000","message":"Woops, thanks!","commit_id":"85f2fb993135fb3ff6cb8bbf4937732bfd7776bb"}],"manila/share/drivers/ganesha/__init__.py":[{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"7c9ee88320f41a3a9476333768a8ae4f951c2311","unresolved":false,"context_lines":[{"line_number":161,"context_line":"            self.ganesha.restart_service()"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        for rule in add_rules:"},{"line_number":164,"context_line":"            self._allow_access(\u0027/\u0027, share, rule)"},{"line_number":165,"context_line":"        for rule in delete_rules:"},{"line_number":166,"context_line":"            self._deny_access(\u0027/\u0027, share, rule)"},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_d6cafa4d","line":164,"range":{"start_line":164,"start_character":0,"end_line":164,"end_character":48},"updated":"2019-03-05 19:56:09.000000000","message":"same as below, try, except and return rule with \u0027error\u0027 state..\n\nerror_rules[rule[\u0027id\u0027]] \u003d { \u0027state\u0027: constants.ACCESS_STATE_ERROR}\n\nSee https://github.com/openstack/manila/blob/a75fe3d/manila/share/driver.py#L748","commit_id":"f90047b15a7e665cee1cff4737fb290e285c49b5"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"7be67b406e0c180966893c2a402fc44f5886fd0f","unresolved":false,"context_lines":[{"line_number":161,"context_line":"            self.ganesha.restart_service()"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        for rule in add_rules:"},{"line_number":164,"context_line":"            self._allow_access(\u0027/\u0027, share, rule)"},{"line_number":165,"context_line":"        for rule in delete_rules:"},{"line_number":166,"context_line":"            self._deny_access(\u0027/\u0027, share, rule)"},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_6908a92b","line":164,"range":{"start_line":164,"start_character":0,"end_line":164,"end_character":48},"in_reply_to":"5fc1f717_d6cafa4d","updated":"2019-03-10 12:15:04.000000000","message":"Agree but see reply below","commit_id":"f90047b15a7e665cee1cff4737fb290e285c49b5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"7c9ee88320f41a3a9476333768a8ae4f951c2311","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        for rule in add_rules:"},{"line_number":164,"context_line":"            self._allow_access(\u0027/\u0027, share, rule)"},{"line_number":165,"context_line":"        for rule in delete_rules:"},{"line_number":166,"context_line":"            self._deny_access(\u0027/\u0027, share, rule)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"class GaneshaNASHelper2(GaneshaNASHelper):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_3639fe40","line":166,"range":{"start_line":166,"start_character":12,"end_line":166,"end_character":47},"updated":"2019-03-05 19:56:09.000000000","message":"If the wrong rule re-appears here, won\u0027t this remove_export operation fail? Can we just check for the access_type and ignore it, per the note in [1]\n\n[1] https://github.com/openstack/manila/blob/a75fe3d/manila/share/driver.py#L748","commit_id":"f90047b15a7e665cee1cff4737fb290e285c49b5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"f94711e22a470672e2748adab48b29b31b7e0893","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        for rule in add_rules:"},{"line_number":164,"context_line":"            self._allow_access(\u0027/\u0027, share, rule)"},{"line_number":165,"context_line":"        for rule in delete_rules:"},{"line_number":166,"context_line":"            self._deny_access(\u0027/\u0027, share, rule)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"class GaneshaNASHelper2(GaneshaNASHelper):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_0feec587","line":166,"range":{"start_line":166,"start_character":12,"end_line":166,"end_character":47},"in_reply_to":"5fc1f717_291b8181","updated":"2019-03-10 19:43:01.000000000","message":"Yes, perhaps exception handling here is appropriate to ignore errors for:\n- removing a rule that was inappropriately added, or\n- a rule that was never added","commit_id":"f90047b15a7e665cee1cff4737fb290e285c49b5"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"7be67b406e0c180966893c2a402fc44f5886fd0f","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        for rule in add_rules:"},{"line_number":164,"context_line":"            self._allow_access(\u0027/\u0027, share, rule)"},{"line_number":165,"context_line":"        for rule in delete_rules:"},{"line_number":166,"context_line":"            self._deny_access(\u0027/\u0027, share, rule)"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"class GaneshaNASHelper2(GaneshaNASHelper):"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_291b8181","line":166,"range":{"start_line":166,"start_character":12,"end_line":166,"end_character":47},"in_reply_to":"5fc1f717_3639fe40","updated":"2019-03-10 12:15:04.000000000","message":"We could but I want to see if we can safely remove garbage (like if someone had done a rule for cephx for NFS b/c of this bug) from the driver\u0027s export config.  We\u0027d still of course need to ignore it in the sense that we wouldn\u0027t ask the back end to act on the delete operation.","commit_id":"f90047b15a7e665cee1cff4737fb290e285c49b5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"7c9ee88320f41a3a9476333768a8ae4f951c2311","unresolved":false,"context_lines":[{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        wanted_rw_clients, wanted_ro_clients \u003d [], []"},{"line_number":243,"context_line":"        for rule in access_rules:"},{"line_number":244,"context_line":"            ganesha_utils.validate_access_rule_type(rule)"},{"line_number":245,"context_line":"            rule \u003d ganesha_utils.fixup_access_rule(rule)"},{"line_number":246,"context_line":"            if rule[\u0027access_level\u0027] \u003d\u003d \u0027rw\u0027:"},{"line_number":247,"context_line":"                wanted_rw_clients.append(rule[\u0027access_to\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_16b5e2ee","line":244,"range":{"start_line":244,"start_character":12,"end_line":244,"end_character":57},"updated":"2019-03-05 19:56:09.000000000","message":"This can raise an exception and set the whole batch of access rules to \"error\" state. We need to try..except and return a rule update for this rule with state set to \"error\" here..","commit_id":"f90047b15a7e665cee1cff4737fb290e285c49b5"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"7be67b406e0c180966893c2a402fc44f5886fd0f","unresolved":false,"context_lines":[{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        wanted_rw_clients, wanted_ro_clients \u003d [], []"},{"line_number":243,"context_line":"        for rule in access_rules:"},{"line_number":244,"context_line":"            ganesha_utils.validate_access_rule_type(rule)"},{"line_number":245,"context_line":"            rule \u003d ganesha_utils.fixup_access_rule(rule)"},{"line_number":246,"context_line":"            if rule[\u0027access_level\u0027] \u003d\u003d \u0027rw\u0027:"},{"line_number":247,"context_line":"                wanted_rw_clients.append(rule[\u0027access_to\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_4c2cdb98","line":244,"range":{"start_line":244,"start_character":12,"end_line":244,"end_character":57},"in_reply_to":"5fc1f717_16b5e2ee","updated":"2019-03-10 12:15:04.000000000","message":"Yes, you are right.\n\nThe only driver I see returning rule updates in this way is isilon.  Cephfs Native returns access keys in the update but not error states.  So only a couple drivers are actually causing any updates to be processed by _process_driver_rule_updates in the manager\u0027s access helper and only one is giving it error states.\n\nDrivers like generic, lvm, Netapp are also validating each access rule in a loop up front and fail the whole batch without returning individual access rule error states.\n\nAm I missing something here?\n\nI mention this stuff not because I don\u0027t want to return rule updates as you suggest but rather to make sure that my understanding of the current lay of the land is correct -- we need to update cephfs native as well and we really should drive some kind of campaign to get all the drivers up to date relative to the access-rule refactor spec from Ocata.","commit_id":"f90047b15a7e665cee1cff4737fb290e285c49b5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"f94711e22a470672e2748adab48b29b31b7e0893","unresolved":false,"context_lines":[{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        wanted_rw_clients, wanted_ro_clients \u003d [], []"},{"line_number":243,"context_line":"        for rule in access_rules:"},{"line_number":244,"context_line":"            ganesha_utils.validate_access_rule_type(rule)"},{"line_number":245,"context_line":"            rule \u003d ganesha_utils.fixup_access_rule(rule)"},{"line_number":246,"context_line":"            if rule[\u0027access_level\u0027] \u003d\u003d \u0027rw\u0027:"},{"line_number":247,"context_line":"                wanted_rw_clients.append(rule[\u0027access_to\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_afb21193","line":244,"range":{"start_line":244,"start_character":12,"end_line":244,"end_character":57},"in_reply_to":"5fc1f717_4c2cdb98","updated":"2019-03-10 19:43:01.000000000","message":"*Long answer alert*\n\nYour understanding is correct. If those drivers are doing some sort of validation and failing the entire batch, those are bugs. \n\nWe initially implemented batch-processing in a half-baked manner, and then we attempted fixing user experience issues (which rule is problematic?) and concurrency (rule updates would get lost all the time at the share manager) over multiple releases as multiple bug fixes. The goal for all the fixes before [1] was that we didn\u0027t want to change the driver interface in any way, because we already had drivers that implemented batch processing. BUT, without affecting the driver interface, we could only fix half of the user experience problems. \n\n \n\nPrior to [1], ALL access rules of a share would be set to *error* if a driver failed somewhere. This was terrible, because the only recourse for the end user would be to guess which specific access rule failed and act on it to ensure the other rules are fine. \n\n \n\nSo, in [1], we acknowledged that we could not provide a better user experience with batch processing without the share manager parsing driver feedback on individual rules. (This was the prior behavior when we didn\u0027t do batch processing). [1] changed the way we performed batch processing at the driver. We feed the driver with smaller batches of rules that they can modify. With this, we ensured that smaller batches errored out. For example, if a share has 3 access rules, and 4 more were added in quick succession, a driver failing to apply the newer/smaller batch of 4 rules would only result in the new/4 rules being set to \"error\".  If a driver implemented the interface correctly, it can control which rules are really supposed to be in \"error\" of the 4. \n\n \n\nDriver authors were informed about this change at the PTG and over IRC iirc, but ofcourse, like you note, we haven\u0027t fixed this up in the first party drivers.\n\nWe could open bugs against all these drivers and get their maintainers to fix them like we want \n\n[1] https://review.openstack.org/#/c/369668/","commit_id":"f90047b15a7e665cee1cff4737fb290e285c49b5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"4bffa10782d5dae5dacc8c0abc50a52b6b81bb1b","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        \"\"\"Allow access to the share.\"\"\""},{"line_number":131,"context_line":"        ganesha_utils.validate_access_rule("},{"line_number":132,"context_line":"            self.supported_access_types, self.supported_access_levels,"},{"line_number":133,"context_line":"            access, True)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        access \u003d ganesha_utils.fixup_access_rule(access)"},{"line_number":136,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"5fc1f717_199de158","line":133,"range":{"start_line":133,"start_character":20,"end_line":133,"end_character":24},"updated":"2019-03-20 22:26:00.000000000","message":"abort\u003dTrue","commit_id":"85f2fb993135fb3ff6cb8bbf4937732bfd7776bb"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"94413a552e0e8df793831b39c5ec0a4e19143b7b","unresolved":false,"context_lines":[{"line_number":130,"context_line":"        \"\"\"Allow access to the share.\"\"\""},{"line_number":131,"context_line":"        ganesha_utils.validate_access_rule("},{"line_number":132,"context_line":"            self.supported_access_types, self.supported_access_levels,"},{"line_number":133,"context_line":"            access, True)"},{"line_number":134,"context_line":""},{"line_number":135,"context_line":"        access \u003d ganesha_utils.fixup_access_rule(access)"},{"line_number":136,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"5fc1f717_3919bd87","line":133,"range":{"start_line":133,"start_character":20,"end_line":133,"end_character":24},"in_reply_to":"5fc1f717_199de158","updated":"2019-03-20 22:45:15.000000000","message":"Agree.","commit_id":"85f2fb993135fb3ff6cb8bbf4937732bfd7776bb"}]}