glusterfs.git/rpc/rpc-lib/src/rpc-clnt.c, branch v3.7.16

changelog/rpc: Fix rpc_clnt_t mem leaks

2016-07-27T10:13:33+00:00

Backport of http://review.gluster.org/13658

PROBLEM:
   1. Freeing up rpc_clnt object might lead to crashes. Well,
      it was not a necessity to free rpc-clnt object till now
      because all the existing use cases needs to reconnect
      back on disconnects. Hence timer code was not taking
      ref on rpc-clnt object.

      Glusterd had some use-cases that led to crash due to
      ping-timer and they fixed only those code paths that
      involve ping-timer.

      Now, since changelog has an use-case where rpc-clnt
      need to be freed up, we need to fix timer code to take
      refs

   2. In changelog, because of issue 1, only mydata was being
      freed which is incorrect. And there are races where
      rpc-clnt object would access the freed mydata which
      would lead to crashes.

      Since changelog xlator resides on brick side and is long
      living process, if multiple libgfchangelog consumers
      register to changelog and disconnect/reconnect mulitple
      times, it would result in leak of 'rpc-clnt' object
      for every connect/disconnect.

SOLUTION:
   1. Handle ref/unref of 'rpc_clnt' structure in timer
      functions properly.
   2. In changelog, unref 'rpc_clnt' in RPC_CLNT_DISCONNECT
      after disabling timers and free mydata on RPC_CLNT_DESTROY.

RPC SETUP IN CHANGELOG:
   1. changelog xlator initiates rpc server say 'changelog_rpc_server'
   2. libgfchangelog initiates one rpc server say 'libgfchangelog_rpc_server'
   3. libgfchangelog initiates rpc client and connects to 'changelog_rpc_server'
   4. In return changelog_rpc_server initiates a rpc client and connects back
      to 'libgfchangelog_rpc_server'

REF/UNREF HANDLING IN TIMER FUNCTIONS:
Let's say rpc clnt refcount = 1
   1. Take the ref before reigstering callback to timer queue
           >>>>  rpc_clnt_ref (say ref count becomes = 2)
   2. Register a callback to timer say 'callback1'
   3. If register fails:
           >>>> rpc_clnt_unref (ref count = 1)
   4. On timer expiration, 'callback1' gets called. So unref rpc clnt at the end
      in 'callback1'. This is corresponding to ref taken in step 1
           >>>> rpc_clnt_unref (ref count = 1)
   5. The cycle from step-1 to step-4 continues....until timer cancel event happens
   6. timer cancel of say 'callback1'
           If timer cancel fails:
                 Do nothing, Step-4 would have unrefd
           If timer cancel succeeds:
                 >>>> rpc_clnt_unref (ref count = 1)

Change-Id: I91389bc511b8b1a17824941970ee8d2c29a74a09
BUG: 1359363
Signed-off-by: Kotresh HR 
(cherry picked from commit 637ce9e2e27e9f598a4a6c5a04cd339efaa62076)
Reviewed-on: http://review.gluster.org/14993
CentOS-regression: Gluster Build System 
Smoke: Gluster Build System 
NetBSD-regression: NetBSD Build System 
Reviewed-by: Raghavendra G

rpc: Connect back only if rpc is not disabled

2016-03-31T04:36:06+00:00

This is a backport of the below fix -
 http://review.gluster.org/13592

As discussed over http://thread.gmane.org/gmane.comp.file-systems.gluster.devel/14284,
patch #13456 caused a regression where in if there are any pending
rpc invocations, we end up accessing freed object. This patch
fixes it by allowing reconnect during rpc submit only if rpc
is not disabled.

This is to fix regression caused by below patch -
http://review.gluster.org/#/c/13507/

Change-Id: I4ef4dd52bd42368bb89129f98bc973e46c6a39f4
BUG: 1311441
Signed-off-by: Kotresh HR 
Reviewed-on: http://review.gluster.org/13592
Reviewed-on: http://review.gluster.org/13824
Tested-by: soumya k 
Smoke: Gluster Build System 
CentOS-regression: Gluster Build System 
NetBSD-regression: NetBSD Build System 
Reviewed-by: Jeff Darcy

rpc: Fix for rpc_transport_t leak

2016-03-24T15:02:18+00:00

The transport object needs to get unref'ed when the rpc clnt
object is getting destroyed. But currently in rpc_clnt_disable()
we set conn->trans to NULL before it gets unref'ed leading to
transport object leak.

This change is to fix it by setting conn-tran to NULL only when
it is being unref'ed.

This is backport of the below patch
	- http://review.gluster.org/13456

Change-Id: I79ba34e28ae19eb616035f36bbed1c2f47875b94
BUG: 1311441
Signed-off-by: Soumya Koduri 
Reviewed-on: http://review.gluster.org/13456
Reviewed-by: Kaleb KEITHLEY 
Reviewed-by: Jeff Darcy 
Reviewed-on: http://review.gluster.org/13507
Smoke: Gluster Build System 
NetBSD-regression: NetBSD Build System 
CentOS-regression: Gluster Build System

glusterd/rpc : Discard duplicate Disconnect events

2016-03-23T14:50:23+00:00

Backport of http://review.gluster.org/#/c/13790/

If a peer rpc disconnect event has been already processed, skip the furthers as
processing them are overheads and sometimes may lead to a crash like due to a
double free

Change-Id: Iec589ce85daf28fd5b267cb6fc82a4238e0e8adc
BUG: 1320374
Signed-off-by: Atin Mukherjee 
Reviewed-on: http://review.gluster.org/13790
Smoke: Gluster Build System 
NetBSD-regression: NetBSD Build System 
CentOS-regression: Gluster Build System 
Reviewed-by: Jeff Darcy 
Reviewed-on: http://review.gluster.org/13813

all: reduce "inline" usage

2016-01-18T09:02:34+00:00

There are three kinds of inline functions: plain inline, extern inline,
and static inline.  All three have been removed from .c files, except
those in "contrib" which aren't our problem.  Inlines in .h files, which
are overwhelmingly "static inline" already, have generally been left
alone.  Over time we should be able to "lower" these into .c files, but
that has to be done in a case-by-case fashion requiring more manual
effort.  This part was easy to do automatically without (as far as I can
tell) any ill effect.

In the process, several pieces of dead code were flagged by the
compiler, and were removed.

backport of Change-Id: I56a5e614735c9e0a6ee420dab949eac22e25c155,
http://review.gluster.org/11769, BUG: 1245331

Change-Id: Iba1efb0bc578ea4a5e9bf76b7bd93dc1be9eba44
BUG: 1283302
Signed-off-by: Kaleb S KEITHLEY 
Reviewed-on: http://review.gluster.org/12646
Smoke: Gluster Build System 
NetBSD-regression: NetBSD Build System 
CentOS-regression: Gluster Build System 
Reviewed-by: Niels de Vos

rpc: fix possible deadlock left behind in d448fd1

2015-11-03T05:53:39+00:00

See http://review.gluster.org/9613 for more details.

BUG: 1233019
Change-Id: I05ac0267b8c6f4e9b354acbbdf5469835455fb10
Signed-off-by: Krishnan Parthasarathi 
Reviewed-on: http://review.gluster.org/10821
Reviewed-by: Raghavendra G 
Tested-by: Raghavendra G 
(cherry picked from commit e902df70f8157db4db503b7ec3c2635b08b3dcb2)
Reviewed-on: http://review.gluster.org/11303
Tested-by: Gluster Build System

rpc: add owner xlator argument to rpc_clnt_new

2015-08-14T10:10:47+00:00

The @owner argument tells RPC layer the xlator that owns
the connection and to which xlator THIS needs be set during
network notifications like CONNECT and DISCONNECT.

Code paths that originate from the head of a (volume) graph and use
STACK_WIND ensure that the RPC local endpoint has the right xlator saved
in the frame of the call (callback pair). This guarantees that the
callback is executed in the right xlator context.

The client handshake process which includes fetching of brick ports from
glusterd, setting lk-version on the brick for the session, don't have
the correct xlator set in their frames. The problem lies with RPC
notifications. It doesn't have the provision to set THIS with the xlator
that is registered with the corresponding RPC programs. e.g,
RPC_CLNT_CONNECT event received by protocol/client doesn't have THIS set
to its xlator. This implies, call(-callbacks) originating from this
thread don't have the right xlator set too.

The fix would be to save the xlator registered with the RPC connection
during rpc_clnt_new. e.g, protocol/client's xlator would be saved with
the RPC connection that it 'owns'. RPC notifications such as CONNECT,
DISCONNECT, etc inherit THIS from the RPC connection's xlator.

Change-Id: I9dea2c35378c511d800ef58f7fa2ea5552f2c409
BUG: 1253212
Signed-off-by: Krishnan Parthasarathi 
Reviewed-on: http://review.gluster.org/11436
Tested-by: Gluster Build System 
Tested-by: NetBSD Build System 
Reviewed-by: Raghavendra G 
(cherry picked from commit f7668938cd7745d024f3d2884e04cd744d0a69ab)
Reviewed-on: http://review.gluster.org/11908
Reviewed-by: Pranith Kumar Karampuri

client, rpc: make ping-timeout configurable for glusterfs clients

2015-08-13T05:05:57+00:00

Change-Id: Idd94adb0457aaffce7330f56f98cebafa2c4dae8
BUG: 1250810
Signed-off-by: Krishnan Parthasarathi 
Reviewed-on: http://review.gluster.org/11818
Tested-by: Gluster Build System 
Reviewed-by: Raghavendra G 
Tested-by: NetBSD Build System 
(cherry picked from commit 3403370ebeaf16567b79022c6ac48b2e0cd50db5)
Reviewed-on: http://review.gluster.org/11848

rpc: call transport_unref only on non-NULL transport

2015-06-09T14:06:29+00:00

BUG: 1228601
Change-Id: Ifac4dd8c633081483e4eba9d7e5a89837b2a453a
Signed-off-by: Krishnan Parthasarathi 
Reviewed-on: http://review.gluster.org/11041
Reviewed-by: Raghavendra G 
Reviewed-on: http://review.gluster.org/11102
Tested-by: Gluster Build System 
Reviewed-by: Niels de Vos

glusterd: fix repeated connection to nfssvc failed msgs

2015-06-02T04:24:00+00:00

... and disable reconnect timer on rpc_clnt_disconnect.

Root Cause
----------

gluster-NFS service wouldn't be started if there are no
started volumes that have nfs service enabled for them.
Before this fix we would initiate a connect even when
the gluster-NFS service wasn't (re)started. Compounding
that glusterd_conn_disconnect doesn't disable reconnect
timer. So, it is possible that the reconnect timer was
in execution when  the timer event was attempted to be
removed.

Change-Id: Iadcb5cff9eafefa95eaf3a1a9413eeb682d3aaac
BUG: 1222065
Signed-off-by: Krishnan Parthasarathi 
Reviewed-on: http://review.gluster.org/10830
Reviewed-by: Atin Mukherjee 
Reviewed-by: Gaurav Kumar Garg 
Reviewed-by: Kaushal M 
Reviewed-on: http://review.gluster.org/10963
Tested-by: Gluster Build System 
Tested-by: NetBSD Build System 
Reviewed-by: Emmanuel Dreyfus