[SSSD] [sssd PR#560][comment] NSS: close files after mmap
URL: https://github.com/SSSD/sssd/pull/560 Title: #560: NSS: close files after mmap alexey-tikhonov commented: """ @ChrisKowalczyk, do you plan to work on this PR? Or may I take over? """ See the full comment at https://github.com/SSSD/sssd/pull/560#issuecomment-532171379 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
[SSSD] [sssd PR#560][comment] NSS: close files after mmap
URL: https://github.com/SSSD/sssd/pull/560 Title: #560: NSS: close files after mmap ChrisKowalczyk commented: """ Thanks for all the comments and quick feedback! Sorry for sharing a closed bug report, I will prepare some better description of this scenario. I will also make sure that the tests are not failing. @mzidek-rh , yes, I think your first comment was true for the second place we use mmap in sssd. I will follow @simo5 advice. @jhrozek , I think that sss_cache was not the case, and this happened just after starting sssd. But as I mentioned, I will double check everything, update PR and provide better description and logs. """ See the full comment at https://github.com/SSSD/sssd/pull/560#issuecomment-384952061 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#560][comment] NSS: close files after mmap
URL: https://github.com/SSSD/sssd/pull/560 Title: #560: NSS: close files after mmap mzidek-rh commented: """ @ChrisKowalczyk Hmm.. i thought we are using the fd in the memory cache operations, but it does not seem to be the case, so yes we do not need the fd, just the pointer to the mmaped region. Forget what I said and follow @simo5 's advice, sorry for confusion :) @jhrozek Right that is also one of the possible reasons, sss_cache can mark the cache as "recycled" and delete the file, the clients learn about this when they do next request to the cache and reload the new file. """ See the full comment at https://github.com/SSSD/sssd/pull/560#issuecomment-384601125 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#560][comment] NSS: close files after mmap
URL: https://github.com/SSSD/sssd/pull/560 Title: #560: NSS: close files after mmap jhrozek commented: """ I wonder if the caches were deleted because someone called sss_cache? """ See the full comment at https://github.com/SSSD/sssd/pull/560#issuecomment-384418479 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#560][comment] NSS: close files after mmap
URL: https://github.com/SSSD/sssd/pull/560 Title: #560: NSS: close files after mmap mzidek-rh commented: """ Hi, I do not know what issue is this patch trying to solve (I do not have Suse bugzilla account and I get Bugzilla – Access Denied), but the patch is not correct. We can not close the mmap cache file descriptor after the memory cache context is initialized. The reason why the mmap context keeps the FD open is so that it does not need to create a new FD every time when we do some operation on the mmap cache. So the fact that the sssd_nss responder is keeping the FD open in the context is by desing. I am not sure why the file is marked as deleted, but note that the mmap cache context is created when the sssd_nss responder is starting up, so when you manually delete the files (for example using the rm command) SSSD needs to be restarted, the old FD will be closed and new one opened. Not restarting SSSD after the mmap cache was removed with "rm" will make the mmap cache not working. Michal """ See the full comment at https://github.com/SSSD/sssd/pull/560#issuecomment-384339932 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#560][comment] NSS: close files after mmap
URL: https://github.com/SSSD/sssd/pull/560 Title: #560: NSS: close files after mmap ChrisKowalczyk commented: """ @fidencio, I'm surprised it caused any problems (I'd assume everybody says it ;) ). I'll have a look at the tests and provide an update. Thank you for the quick feedback! """ See the full comment at https://github.com/SSSD/sssd/pull/560#issuecomment-384333959 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#560][comment] NSS: close files after mmap
URL: https://github.com/SSSD/sssd/pull/560
Title: #560: NSS: close files after mmap
fidencio commented:
"""
@ChrisKowalczyk,
Firstly, sorry that I'm not going to review the patch itself (I'll leave it to
someone else).
What I'd like to bring up here is that a bunch of tests are failing due to the
change:
```
=== FAILURES
_ test_add_remove_add_file_user
_
Traceback (most recent call last):
File
"/home/brq/ffidenci/src/upstream/sssd/src/tests/intg/test_files_provider.py",
line 534, in test_add_remove_add_file_user
assert res == NssReturnCode.NOTFOUND
AssertionError: assert 1 == 0
+ where 0 = NssReturnCode.NOTFOUND
__ test_mod_user_shell
__
Traceback (most recent call last):
File
"/home/brq/ffidenci/src/upstream/sssd/src/tests/intg/test_files_provider.py",
line 561, in test_mod_user_shell
check_user(moduser)
File
"/home/brq/ffidenci/src/upstream/sssd/src/tests/intg/test_files_provider.py",
line 375, in check_user
assert found_user == exp_user
AssertionError: assert {'dir': '/hom... 'user1', ...} == {'dir': '/home...
'user1', ...}
Omitting 6 identical items, use -vv to show
Differing items:
{'shell': '/bin/bash'} != {'shell': '/bin/zsh'}
Full diff:
- {'dir': u'/home/user1',
? -
+ {'dir': '/home/user1',...
...Full output truncated (16 lines hidden), use '-vv' to show
__ test_user_no_shell
___
Traceback (most recent call last):
File
"/home/brq/ffidenci/src/upstream/sssd/src/tests/intg/test_files_provider.py",
line 597, in test_user_no_shell
check_user(incomplete_user_setup(setup_pw_with_canary, 'shell', ''))
File
"/home/brq/ffidenci/src/upstream/sssd/src/tests/intg/test_files_provider.py",
line 375, in check_user
assert found_user == exp_user
AssertionError: assert {'dir': '/hom... 'user1', ...} == {'dir': '/home...
'user1', ...}
Omitting 6 identical items, use -vv to show
Differing items:
{'shell': '/bin/bash'} != {'shell': ''}
Full diff:
- {'dir': u'/home/user1',
? -
+ {'dir': '/home/user1',...
...Full output truncated (14 lines hidden), use '-vv' to show
___ test_user_no_dir
Traceback (most recent call last):
File
"/home/brq/ffidenci/src/upstream/sssd/src/tests/intg/test_files_provider.py",
line 605, in test_user_no_dir
check_user(incomplete_user_setup(setup_pw_with_canary, 'dir', '/'))
File
"/home/brq/ffidenci/src/upstream/sssd/src/tests/intg/test_files_provider.py",
line 375, in check_user
assert found_user == exp_user
AssertionError: assert {'dir': '/hom... 'user1', ...} == {'dir': '/', '...
'user1', ...}
Omitting 6 identical items, use -vv to show
Differing items:
{'dir': '/home/user1'} != {'dir': '/'}
Full diff:
- {'dir': u'/home/user1',
+ {'dir': '/',
- 'gecos': u'User for tests',...
...Full output truncated (14 lines hidden), use '-vv' to show
__ test_user_no_gecos
___
Traceback (most recent call last):
File
"/home/brq/ffidenci/src/upstream/sssd/src/tests/intg/test_files_provider.py",
line 613, in test_user_no_gecos
check_user(incomplete_user_setup(setup_pw_with_canary, 'gecos', ''))
File
"/home/brq/ffidenci/src/upstream/sssd/src/tests/intg/test_files_provider.py",
line 375, in check_user
assert found_user == exp_user
AssertionError: assert {'dir': '/hom... 'user1', ...} == {'dir': '/home...
'user1', ...}
Omitting 6 identical items, use -vv to show
Differing items:
{'gecos': 'User for tests'} != {'gecos': ''}
Full diff:
- {'dir': u'/home/user1',
? -
+ {'dir': '/home/user1',...
...Full output truncated (14 lines hidden), use '-vv' to show
___ test_incomplete_user_fail
___
Traceback (most recent call last):
File
"/home/brq/ffidenci/src/upstream/sssd/src/tests/intg/test_files_provider.py",
line 641, in test_incomplete_user_fail
assert res == NssReturnCode.NOTFOUND
AssertionError: assert 1 == 0
+ where 0 = NssReturnCode.NOTFOUND
__ test_getgrnam_ghost
__
Traceback (most recent call last):
File
"/home/brq/ffidenci/src/upstream/sssd/src/tests/intg/test_files_provider.py",
line 857, in test_getgrnam_ghost
assert res == NssReturnCode.NOTFOUND
AssertionError: assert 1 == 0
+ where 0 = NssReturnCode.NOTFOUND
test_getgrnam_add_remove_ghosts
Traceback (most recent call last):
File
"/home/brq/ffidenci/src/upstream/sssd/src/tests/intg/test_files_provider.py",
line 968,
[SSSD] [sssd PR#560][comment] NSS: close files after mmap
URL: https://github.com/SSSD/sssd/pull/560 Title: #560: NSS: close files after mmap fidencio commented: """ retest this please """ See the full comment at https://github.com/SSSD/sssd/pull/560#issuecomment-384279543 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#560][comment] NSS: close files after mmap
URL: https://github.com/SSSD/sssd/pull/560 Title: #560: NSS: close files after mmap fidencio commented: """ ok to test """ See the full comment at https://github.com/SSSD/sssd/pull/560#issuecomment-384246997 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#560][comment] NSS: close files after mmap
URL: https://github.com/SSSD/sssd/pull/560 Title: #560: NSS: close files after mmap centos-ci commented: """ Can one of the admins verify this patch? """ See the full comment at https://github.com/SSSD/sssd/pull/560#issuecomment-384218285 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#560][comment] NSS: close files after mmap
URL: https://github.com/SSSD/sssd/pull/560 Title: #560: NSS: close files after mmap centos-ci commented: """ Can one of the admins verify this patch? """ See the full comment at https://github.com/SSSD/sssd/pull/560#issuecomment-384218286 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
