May 2023 - Devel - NFS Ganesha List Archives

Change in ...nfs-ganesha[next]: Allow EXPORT pseudo path to be changed during export update
by Frank Filz (GerritHub) 20 Feb '24

20 Feb '24

Frank Filz has uploaded this change for review. ( https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/490334 ) Change subject: Allow EXPORT pseudo path to be changed during export update ...................................................................... Allow EXPORT pseudo path to be changed during export update This also fully allows adding or removing NFSv4 support from an export since we can now handle the PseudoFS swizzing that occurs. Note that an explicit PseudoFS export may be removed or added, though you can not change it from export_id 0 because we currently don't allow changing the export_id. Note that this patch doesn't handle DBUS add or remove export though that is an option to improve. I may add them to this patch (it wouldn't be that hard) but I want to get this reviewed as is right now. There are implications to a client of changing the PseudoFS. I have tested moving an export in the PseudoFS with a client mounted. The client will be able to continue accessing the export, though it may see an ESTALE error if it navigates out of the export. The current working directory will go bad and the pwd comment will fail indicating a disconnected mount. I have also seen referencing .. from the root of the export wrapping around back to the root (I believe this is how disconnected mounts are set up). FSAL_PSEUDO lookups and create handles (PUTFH or any use of an NFSv3 handle where the inode isn't cached) which fail during an export update are instead turned into ERR_FSAL_DELAY which turns into NFS4ERR_DELAY or NFS3ERR_JUKEBOX to force the client to retry under the completed update. Change-Id: I507dc17a651936936de82303ff1291677ce136be Signed-off-by: Frank S. Filz <ffilzlnx(a)mindspring.com> --- M src/FSAL/FSAL_PSEUDO/handle.c M src/MainNFSD/libganesha_nfsd.ver M src/Protocols/NFS/nfs4_pseudo.c M src/include/export_mgr.h M src/include/nfs_proto_functions.h M src/support/export_mgr.c M src/support/exports.c 7 files changed, 560 insertions(+), 203 deletions(-) git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha refs/changes/34/490334/1 -- To view, visit https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/490334 To unsubscribe, or for help writing mail filters, visit https://review.gerrithub.io/settings Gerrit-Project: ffilz/nfs-ganesha Gerrit-Branch: next Gerrit-Change-Id: I507dc17a651936936de82303ff1291677ce136be Gerrit-Change-Number: 490334 Gerrit-PatchSet: 1 Gerrit-Owner: Frank Filz <ffilzlnx(a)mindspring.com> Gerrit-MessageType: newchange

2 2

Help needed on FD
by Alok Sinha 31 Jan '24

31 Jan '24

FD created at below path does not have correct link in /proc/self/fd. With vfs_readlink_by_handle , I can get a relative path but cannot get the absolute path of the FD. I need a suggestion for one of the below to get around a production bug: - How to get the full path of the FD? - How to get the parent vfs_fsal_obj_handle? - Anyway to bypass this flow by changing any config file? #0 vfs_create_handle (exp_hdl=0x55801aca22c0, hdl_desc=0x7f93889b1600, handle=0x7f93889b13f8, attrs_out=0x7f93889b1430) at /home/alok/pub/splfs-cache-2.8.3/src/FSAL/FSAL_VFS/handle.c:2020 #1 0x00007f94a1f0e13b in mdcache_locate_host (fh_desc=0x7f93889b1600, export=0x55801aca1f20, entry=0x7f93889b1578, attrs_out=0x0) at /home/alok/pub/splfs-cache-2.8.3/src/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_helpers.c:1109 #2 0x00007f94a1f02455 in mdcache_create_handle (exp_hdl=0x55801aca1f20, fh_desc=0x7f93889b1600, handle=0x7f93889b15d8, attrs_out=0x0) at /home/alok/pub/splfs-cache-2.8.3/src/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_handle.c:1613 #3 0x00007f94a1ec5ead in nfs4_mds_putfh (data=0x7f93201119e0) at /home/alok/pub/splfs-cache-2.8.3/src/Protocols/NFS/nfs4_op_putfh.c:211 #4 0x00007f94a1ec60d3 in nfs4_op_putfh (op=0x7f93201186d0, data=0x7f93201119e0, resp=0x7f932010ab20) at /home/alok/pub/splfs-cache-2.8.3/src/Protocols/NFS/nfs4_op_putfh.c:281 #5 0x00007f94a1ea96ae in process_one_op (data=0x7f93201119e0, status=0x7f93889b1b48) at /home/alok/pub/splfs-cache-2.8.3/src/Protocols/NFS/nfs4_Compound.c:920 #6 0x00007f94a1eaa8c5 in nfs4_Compound (arg=0x7f9320138d78, req=0x7f93201385a0, res=0x7f9320106840) at /home/alok/pub/splfs-cache-2.8.3/src/Protocols/NFS/nfs4_Compound.c:1329 #7 0x00007f94a1de8567 in nfs_rpc_process_request (reqdata=0x7f93201385a0) ---Type <return> to continue, or q <return> to quit--- at /home/alok/pub/splfs-cache-2.8.3/src/MainNFSD/nfs_worker_thread.c:1484 #8 0x00007f94a1de8854 in nfs_rpc_valid_NFS (req=0x7f93201385a0) at /home/alok/pub/splfs-cache-2.8.3/src/MainNFSD/nfs_worker_thread.c:1591 #9 0x00007f94a13b6faf in svc_vc_decode (req=0x7f93201385a0) at /home/alok/pub/splfs-cache-2.8.3/src/libntirpc/src/svc_vc.c:829 #10 0x00007f94a13b3225 in svc_request (xprt=0x7f9320000ce0, xdrs=0x7f9320066260) at /home/alok/pub/splfs-cache-2.8.3/src/libntirpc/src/svc_rqst.c:793 #11 0x00007f94a13b6ec0 in svc_vc_recv (xprt=0x7f9320000ce0) at /home/alok/pub/splfs-cache-2.8.3/src/libntirpc/src/svc_vc.c:802 #12 0x00007f94a13b31a5 in svc_rqst_xprt_task (wpe=0x7f9320000f00) at /home/alok/pub/splfs-cache-2.8.3/src/libntirpc/src/svc_rqst.c:774 #13 0x00007f94a13bcab0 in work_pool_thread (arg=0x7f9218003340) at /home/alok/pub/splfs-cache-2.8.3/src/libntirpc/src/work_pool.c:184 #14 0x00007f94a1badea5 in start_thread () from /lib64/libpthread.so.0 #15 0x00007f94a16ce9fd in clone () from /lib64/libc.so.6 -- Alok Sinha www.spillbox.io https://youtu.be/U-YupjLQ9bU

2 2

[XS] Change in ...nfs-ganesha[next]: build: Fix developer warning for LSB module
by Martin Schwenke (GerritHub) 30 May '23

30 May '23

Martin Schwenke has uploaded this change for review. ( https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/554766?usp=email ) Change subject: build: Fix developer warning for LSB module ...................................................................... build: Fix developer warning for LSB module CMake Warning (dev) at /usr/share/cmake-3.25/Modules/FindPackageHandleStandardArgs.cmake:438 (message): The package name passed to `find_package_handle_standard_args` (LSB_RELEASE) does not match the name of the calling package (LSB). This can lead to problems in calling code that expects `find_package` result variables (e.g., `_FOUND`) to follow a certain pattern. Call Stack (most recent call first): cmake/modules/FindLSB.cmake:85 (find_package_handle_standard_args) CMakeLists.txt:1632 (find_package) This warning is for project developers. Use -Wno-dev to suppress it. Rename the module to LSBRelease and make that the first argument to find_package_handle_standard_args(). Other options are: * Pass LSB as the first argument. This just seems confusing. * Rename the module to LSB_RELEASE. Given that there are no cmake modules with names containing underscores, this doesn't seem like the right solution. Signed-off-by: Martin Schwenke <mschwenke(a)ddn.com> Change-Id: If0e3c80378db8aa97e8f0b6f69d91ef0f8201e3f --- M src/CMakeLists.txt R src/cmake/modules/FindLSBRelease.cmake 2 files changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha refs/changes/66/554766/1 -- To view, visit https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/554766?usp=email To unsubscribe, or for help writing mail filters, visit https://review.gerrithub.io/settings Gerrit-Project: ffilz/nfs-ganesha Gerrit-Branch: next Gerrit-Change-Id: If0e3c80378db8aa97e8f0b6f69d91ef0f8201e3f Gerrit-Change-Number: 554766 Gerrit-PatchSet: 1 Gerrit-Owner: Martin Schwenke <martin(a)meltin.net> Gerrit-MessageType: newchange

1 0

[L] Change in ...nfs-ganesha[next]: build: Fix Python detection
by Martin Schwenke (GerritHub) 30 May '23

30 May '23

Martin Schwenke has uploaded this change for review. ( https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/554765?usp=email ) Change subject: build: Fix Python detection ...................................................................... build: Fix Python detection Commit c43049fc42b0a1d39b48ddbf93af56223ba5f9ee introduced copies of several cmake modules, which are now breaking Python detection. The commit message does not give details of benefits of these versions of the modules. It is unclear if they were different to the upstream cmake modules at the time and this can't be resolved without an archaeological dig. Whatever the case, the modules have almost certainly been improved upstream to work around the reason for their addition here. Just delete the duplicate modules introduced by the above commit. This leaves src/cmake/modules/FindGTest.cmake as the only apparent duplicate module. Note that this change introduces a developer warning: CMake Warning (dev) at /usr/share/cmake-3.25/Modules/FindPackageHandleStandardArgs.cmake:438 (message): The package name passed to `find_package_handle_standard_args` (LSB_RELEASE) does not match the name of the calling package (LSB). This can lead to problems in calling code that expects `find_package` result variables (e.g., `_FOUND`) to follow a certain pattern. Call Stack (most recent call first): cmake/modules/FindLSB.cmake:85 (find_package_handle_standard_args) CMakeLists.txt:1632 (find_package) This warning is for project developers. Use -Wno-dev to suppress it. Given that this is only a warning, this can be fixed in a subsequent commit. Signed-off-by: Martin Schwenke <mschwenke(a)ddn.com> Change-Id: I0cb1fb4d3dd9674d52720095961ed6b56dda6c45 --- D src/cmake/modules/CMakeParseArguments.cmake D src/cmake/modules/FindPackageHandleStandardArgs.cmake D src/cmake/modules/FindPackageMessage.cmake D src/cmake/modules/FindPkgConfig.cmake 4 files changed, 0 insertions(+), 898 deletions(-) git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha refs/changes/65/554765/1 -- To view, visit https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/554765?usp=email To unsubscribe, or for help writing mail filters, visit https://review.gerrithub.io/settings Gerrit-Project: ffilz/nfs-ganesha Gerrit-Branch: next Gerrit-Change-Id: I0cb1fb4d3dd9674d52720095961ed6b56dda6c45 Gerrit-Change-Number: 554765 Gerrit-PatchSet: 1 Gerrit-Owner: Martin Schwenke <martin(a)meltin.net> Gerrit-MessageType: newchange

1 0

[S] Change in ...nfs-ganesha[next]: GPFS: fix 40 byte sized old file handles
by Name of user not set (GerritHub) 30 May '23

30 May '23

Attention is currently required from: Malahal. Hello Malahal, I'd like you to do a code review. Please visit https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/554760?usp=email to review the following change. Change subject: GPFS: fix 40 byte sized old file handles ...................................................................... GPFS: fix 40 byte sized old file handles Some versions may have returned 40 byte sized file handles. Make them same as 48 byte sized file handles to avoid multiple entries in the object cache for the same file system object. Change-Id: I242faa62cfc3c5d88db86a1096ef4720ce4c6590 Signed-off-by: Malahal Naineni <malahal(a)us.ibm.com> (cherry picked from commit 706e5dce49d7bbb07ea005d1b74eada8b0f1a761) --- M src/FSAL/FSAL_GPFS/handle.c 1 file changed, 13 insertions(+), 0 deletions(-) git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha refs/changes/60/554760/1 -- To view, visit https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/554760?usp=email To unsubscribe, or for help writing mail filters, visit https://review.gerrithub.io/settings Gerrit-Project: ffilz/nfs-ganesha Gerrit-Branch: next Gerrit-Change-Id: I242faa62cfc3c5d88db86a1096ef4720ce4c6590 Gerrit-Change-Number: 554760 Gerrit-PatchSet: 1 Gerrit-Owner: skmprabhu252(a)gmail.com Gerrit-Reviewer: Malahal <malahal(a)gmail.com> Gerrit-Attention: Malahal <malahal(a)gmail.com> Gerrit-MessageType: newchange

1 0

MDCache LRU management queries
by Gaurav Sharma 22 May '23

22 May '23

Hi Ganesha Team, In MDCache we have two arrays named LRU and CHUNK_LRU to store the lru entry in their respective queues. For the CHUNK_LRU, the function lru_bump_chunk() will move the entry to MRU of L1. For LRU there is a function adjust_lru() which does the similar job. Question : The function adjust_lru() is only called if the flag is LRU_REQ_INITIAL and that is happening only during unexport. 1. Can you please suggest how the entries in LRU queue are moving to MRU of the queue? 2. How LRU entries are moved from L2 to L1 queue? from L1 to L2 is done by the background thread. Regards, Gaurav Sharma

2 1

Announce Push of V5.2
by Frank Filz 19 May '23

19 May '23

Branch next Tag:V5.2 Merge Highlights * Fix btrfs typo in log messages * Fix cmake so USE_FSAL_CEPH_FS_NONBLOCKING_IO will be used * Unlock work_mutex in fsal_complete_io only if we got it * For async I/O the callback must create it's own op_context * monitoring: C++ cleanup to build with userspace-rcu-14.0.0 * NLM: Fix NSM reference count double-decrement * Multi-client same hostname issue - open owner refcount fix Signed-off-by: Frank S. Filz <ffilzlnx(a)mindspring.com> Contents: 9a84f015b Frank S. Filz V5.2 2aa4494cd Deepak Arumugam Sankara Subramanian Multi-client same hostname issue - open owner refcount fix 8f6ee41cd Martin Schwenke NLM: Fix NSM reference count double-decrement 622d7f68b Kaleb S. KEITHLEY monitoring: C++ cleanup to build with userspace-rcu-14.0.0 10389577b Frank S. Filz For async I/O the callback must create it's own op_context 43d984e2c Frank S. Filz Unlock work_mutex in fsal_complete_io only if we got it ed66984ae Frank S. Filz CMake - actually do USE_FSAL_CEPH_FS_NONBLOCKING_IO 5a747e92a Frank S. Filz Change btfs to btrfs in two log messages

1 0

Understanding ganesha's idmapping behavior for specific principals
by Dipit Grover 15 May '23

15 May '23

Hi team, Looking at the Ganesha nfs idmapper code (at https://github.com/nfs-ganesha/nfs-ganesha/blob/next/src/idmapper/idmapper.…) I wish to understand the rationale behind resolving principals of the form "nfs/*", "host/*" and "root/*" to the "root" user. I could not find any mention of it in the NFS protocol RFCs. Is there a specific use-case that this code enables? Thanks, Dipit

1 0

[M] Change in ...nfs-ganesha[next]: Pass failure to add clientid to recovery database back to client
by Frank Filz (GerritHub) 11 May '23

11 May '23

Frank Filz has uploaded this change for review. ( https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553781?usp=email ) Change subject: Pass failure to add clientid to recovery database back to client ...................................................................... Pass failure to add clientid to recovery database back to client Previously we would log an error, but leave the client with no clue that it would not be able to reclaim state in the case of a server failure. The failure will be reported to the client as NFS4ERR_SERVERFAULT on SET_CLIENDID_CONFIRM or CREATE_SESSION. Change-Id: If68312abcd06ff5bcc010ebd84d1421717b839a3 Signed-off-by: Frank S. Filz <ffilzlnx(a)mindspring.com> --- M src/SAL/nfs4_clientid.c M src/SAL/nfs4_recovery.c M src/SAL/recovery/recovery_fs.c M src/SAL/recovery/recovery_fs.h M src/SAL/recovery/recovery_rados.h M src/SAL/recovery/recovery_rados_kv.c M src/SAL/recovery/recovery_rados_ng.c M src/include/sal_data.h M src/include/sal_functions.h 9 files changed, 74 insertions(+), 20 deletions(-) git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha refs/changes/81/553781/1 -- To view, visit https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553781?usp=email To unsubscribe, or for help writing mail filters, visit https://review.gerrithub.io/settings Gerrit-Project: ffilz/nfs-ganesha Gerrit-Branch: next Gerrit-Change-Id: If68312abcd06ff5bcc010ebd84d1421717b839a3 Gerrit-Change-Number: 553781 Gerrit-PatchSet: 1 Gerrit-Owner: Frank Filz <ffilzlnx(a)mindspring.com> Gerrit-MessageType: newchange

1 0

[M] Change in ...nfs-ganesha[next]: For async I/O the callback must create it's own op_context
by Frank Filz (GerritHub) 11 May '23

11 May '23

Frank Filz has uploaded this change for review. ( https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553780?usp=email ) Change subject: For async I/O the callback must create it's own op_context ...................................................................... For async I/O the callback must create it's own op_context We can't pass the orginal caller's op_context, it may still need it. Just instantiate a simple op_context for the callback. The callback op_context needs an export reference but we can take it when we create the op_context because until the protocol callback is made, we know the protocol request's op_context with its reference for the export MUST still be valid. Change-Id: I061c7eed8105b3283a243335b5ea63a86225b85b Signed-off-by: Frank S. Filz <ffilzlnx(a)mindspring.com> --- M src/FSAL/FSAL_CEPH/handle.c M src/FSAL/FSAL_MEM/mem_handle.c M src/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_file.c M src/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_int.h 4 files changed, 98 insertions(+), 47 deletions(-) git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha refs/changes/80/553780/1 -- To view, visit https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553780?usp=email To unsubscribe, or for help writing mail filters, visit https://review.gerrithub.io/settings Gerrit-Project: ffilz/nfs-ganesha Gerrit-Branch: next Gerrit-Change-Id: I061c7eed8105b3283a243335b5ea63a86225b85b Gerrit-Change-Number: 553780 Gerrit-PatchSet: 1 Gerrit-Owner: Frank Filz <ffilzlnx(a)mindspring.com> Gerrit-MessageType: newchange

1 0

[S] Change in ...nfs-ganesha[next]: Unlock work_mutex in fsal_complete_io only if we got it
by Frank Filz (GerritHub) 11 May '23

11 May '23

Frank Filz has uploaded this change for review. ( https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553779?usp=email ) Change subject: Unlock work_mutex in fsal_complete_io only if we got it ...................................................................... Unlock work_mutex in fsal_complete_io only if we got it Change-Id: I280f806f0ae394bdc4f3ca1a79a2346fb38db35b Signed-off-by: Frank S. Filz <ffilzlnx(a)mindspring.com> --- M src/FSAL/commonlib.c 1 file changed, 17 insertions(+), 4 deletions(-) git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha refs/changes/79/553779/1 -- To view, visit https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553779?usp=email To unsubscribe, or for help writing mail filters, visit https://review.gerrithub.io/settings Gerrit-Project: ffilz/nfs-ganesha Gerrit-Branch: next Gerrit-Change-Id: I280f806f0ae394bdc4f3ca1a79a2346fb38db35b Gerrit-Change-Number: 553779 Gerrit-PatchSet: 1 Gerrit-Owner: Frank Filz <ffilzlnx(a)mindspring.com> Gerrit-MessageType: newchange

1 0

[S] Change in ...nfs-ganesha[next]: CMake - actually do USE_FSAL_CEPH_FS_NONBLOCKING_IO
by Frank Filz (GerritHub) 11 May '23

11 May '23

Frank Filz has uploaded this change for review. ( https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553778?usp=email ) Change subject: CMake - actually do USE_FSAL_CEPH_FS_NONBLOCKING_IO ...................................................................... CMake - actually do USE_FSAL_CEPH_FS_NONBLOCKING_IO Change-Id: I24032d23088344f72cb72734d6dd8b4042ef95e9 Signed-off-by: Frank S. Filz <ffilzlnx(a)mindspring.com> --- M src/include/config-h.in.cmake 1 file changed, 11 insertions(+), 0 deletions(-) git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha refs/changes/78/553778/1 -- To view, visit https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553778?usp=email To unsubscribe, or for help writing mail filters, visit https://review.gerrithub.io/settings Gerrit-Project: ffilz/nfs-ganesha Gerrit-Branch: next Gerrit-Change-Id: I24032d23088344f72cb72734d6dd8b4042ef95e9 Gerrit-Change-Number: 553778 Gerrit-PatchSet: 1 Gerrit-Owner: Frank Filz <ffilzlnx(a)mindspring.com> Gerrit-MessageType: newchange

1 0

[M] Change in ...nfs-ganesha[next]: Multi-client same hostname issue - open owner refcount fix
by Name of user not set (GerritHub) 10 May '23

10 May '23

deepakarumugam.s(a)nutanix.com has uploaded this change for review. ( https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553739?usp=email ) Change subject: Multi-client same hostname issue - open owner refcount fix ...................................................................... Multi-client same hostname issue - open owner refcount fix This is a followup to https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553234 Issue/Bug: After cherry-picking the changes our code flow for creating a nfs4 open owner looks like this (1) owner.refcount = 1 (memcpy from key to owner accomplishes this) (2) mutex_lock(clid_mutex) add owner to clid_list mutex_unlock(clid_mutex) (3) owner.refcount = 1 (again) The problem is when the client id expiration code gets interleaved between (2) and (3) it does something like this, mutex_lock(clid_mutex) atomic_inc(owner.refcount) // value goes up to 2 mutex_unlock(clid_mutex) Now when step (3) gets executed after the client id expiration code it resets refcount from 2 to 1. This causes unnecessary freeing of open_owner structures and lots of crashes segfaults etc. Solution: We remove (3) (ie) resetting refcount to 1 after adding it to the list. This code is also used for nlm owner initializations. nlm code assumes that get_state_owner takes care of the refcount so now we have to take care of the refcount in nlm callback Change-Id: I8ed40adee9b60fb3101aafb1b675c7c782a1f640 Signed-off-by: Deepak Arumugam Sankara Subramanian <deepakarumugam.s(a)nutanix.com> --- M src/SAL/nlm_owner.c M src/SAL/state_misc.c 2 files changed, 50 insertions(+), 2 deletions(-) git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha refs/changes/39/553739/1 -- To view, visit https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553739?usp=email To unsubscribe, or for help writing mail filters, visit https://review.gerrithub.io/settings Gerrit-Project: ffilz/nfs-ganesha Gerrit-Branch: next Gerrit-Change-Id: I8ed40adee9b60fb3101aafb1b675c7c782a1f640 Gerrit-Change-Number: 553739 Gerrit-PatchSet: 1 Gerrit-Owner: deepakarumugam.s(a)nutanix.com Gerrit-MessageType: newchange

1 0

[S] Change in ...nfs-ganesha[next]: NLM: Fix NSM reference count double-decrement
by Martin Schwenke (GerritHub) 04 May '23

04 May '23

Martin Schwenke has uploaded this change for review. ( https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553524?usp=email ) Change subject: NLM: Fix NSM reference count double-decrement ...................................................................... NLM: Fix NSM reference count double-decrement Fixes: https://github.com/nfs-ganesha/nfs-ganesha/issues/937 Commit 5febadaa98fb53bc4c3f2dab7793d6afb3847073 moves the NSM reference count decrement before a possible error is handled. Commit 275539c02ad3984e17d6d78b494bd9328cf466c7 (authored before the above, merged after) causes the old sm_unmonitor() function (renamed to nsm_unmonitor_no_retry()) to be called twice if the first attempt fails. This means the reference count is likely to be decremented twice on error. The commit message for 5febadaa98fb53bc4c3f2dab7793d6afb3847073 says the reference count decrement needs to be before the call to nsm_disconnect(). However, it was already before the call to nsm_disconnect() in the successful case, and it doesn't seem sane to decrement the reference count on failure. Therefore, partially reverting 5febadaa98fb53bc4c3f2dab7793d6afb3847073 by moving the reference count back to where it was. Signed-off-by: Martin Schwenke <mschwenke(a)ddn.com> Change-Id: I595e5193f945fe7bcb30cb05d7e2de31f69b7920 --- M src/Protocols/NLM/nsm.c 1 file changed, 30 insertions(+), 1 deletion(-) git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha refs/changes/24/553524/1 -- To view, visit https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553524?usp=email To unsubscribe, or for help writing mail filters, visit https://review.gerrithub.io/settings Gerrit-Project: ffilz/nfs-ganesha Gerrit-Branch: next Gerrit-Change-Id: I595e5193f945fe7bcb30cb05d7e2de31f69b7920 Gerrit-Change-Number: 553524 Gerrit-PatchSet: 1 Gerrit-Owner: Martin Schwenke <martin(a)meltin.net> Gerrit-MessageType: newchange

1 0

Broken reference count in NSM retry?
by Martin Schwenke 04 May '23

04 May '23

I came across this while we were trying to debug https://github.com/nfs-ganesha/nfs-ganesha/issues/680 (not quite there yet). The following commit was merged fairly recently, partly at my request: commit 275539c02ad3984e17d6d78b494bd9328cf466c7 Author: Malahal Naineni <malahal(a)us.ibm.com> AuthorDate: Wed Oct 24 14:04:34 2018 +0530 Commit: Frank S. Filz <ffilzlnx(a)mindspring.com> CommitDate: Fri Nov 18 10:33:13 2022 -0800 Retry communication with NSM service It retries SM_MON and SM_UNMON on failure, forcing a disconnect on error. The above commit was written before the following one, but was merged after: commit 5febadaa98fb53bc4c3f2dab7793d6afb3847073 Author: Gaurav B. Gangalwar <gaurav.gangalwar(a)gmail.com> AuthorDate: Tue Feb 19 10:42:57 2019 -0500 Commit: Frank S. Filz <ffilzlnx(a)mindspring.com> CommitDate: Fri Feb 22 10:24:07 2019 -0800 Reduce nsm_count before nsm_disconnect as we will not be able to disconnect nsm client in case of RPC failures. Change-Id: I0cd5f0614e89f447dc7c5ac278cf03224add05e2 Signed-off-by: Gaurav B. Gangalwar <gaurav.gangalwar(a)gmail.com> It moves nsm_count-- before the last error check. The combination of both patches means that, on error, an SM_UNMON request can reduce nsm_count by 2, making it negative. Oops! The 2nd commit claims to "Reduce nsm_count before nsm_disconnect". However, when you look at the patch and the code, the call to nsm_disconnect() is after the final LogDebug() in the patch context, so nsm_count-- was always before the call to nsm_disconnect(). So, unless there's a something subtle going on (e.g. it really wants to decrement the reference count before the call to clnt_req_release()) then the patch doesn't do what the commit message says. In my mind, the simplest solution to the double-decrement-on-retry issue is to move nsm_count-- back to where it was before, so it is only done on success. If that seems sane then I'm happy to submit a patch doing that. Thanks! peace & happiness, martin

1 0

[M] Change in ...nfs-ganesha[next]: monitoring: C++ cleanup to build with userspace-rcu-14.0.0
by Kaleb KEITHLEY (GerritHub) 02 May '23

02 May '23

Kaleb KEITHLEY has uploaded this change for review. ( https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553407?usp=email ) Change subject: monitoring: C++ cleanup to build with userspace-rcu-14.0.0 ...................................................................... monitoring: C++ cleanup to build with userspace-rcu-14.0.0 New version of userspace-rcu has now has broad c++ support and the original monitoring implementation fails to build as a result of the use nested 'extern "C" { ... }'. Note that nested 'extern "C"' is tricky at best to use. This patch adds safe, non-nested extern "C" protection to the minimal set of ganesha internal headers necessary to build monitoring. The one hack here is the wrapping of the #include <rpc/xdr_inline.h> in include/gsh_rpc.h. gsh_rpc is one of just a couple of ntirpc headers that does not have its own '#ifdef __cplusplus extern "C" { ... } #endif' wrapper. Change-Id: I680ad2c79648ef44c54ad43e653db113722b66a9 ChangeId: I5834b8061f01569d1d51f5882739c23d7072cf1a Signed-off-by: Kaleb S. KEITHLEY <kkeithle(a)redhat.com> --- M src/include/config_parsing.h M src/include/display.h M src/include/gsh_list.h M src/include/gsh_refstr.h M src/include/gsh_rpc.h M src/include/gsh_types.h M src/include/log.h M src/include/monitoring.h M src/include/mount.h M src/include/nfs23.h M src/include/nfs_convert.h M src/monitoring/monitoring.cc 12 files changed, 122 insertions(+), 2 deletions(-) git pull ssh://review.gerrithub.io:29418/ffilz/nfs-ganesha refs/changes/07/553407/1 -- To view, visit https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/553407?usp=email To unsubscribe, or for help writing mail filters, visit https://review.gerrithub.io/settings Gerrit-Project: ffilz/nfs-ganesha Gerrit-Branch: next Gerrit-Change-Id: I680ad2c79648ef44c54ad43e653db113722b66a9 Gerrit-Change-Number: 553407 Gerrit-PatchSet: 1 Gerrit-Owner: Kaleb KEITHLEY <kaleb(a)redhat.com> Gerrit-MessageType: newchange

1 0