summaryrefslogtreecommitdiffstats
path: root/xlators/protocol/client/src
Commit message (Collapse)AuthorAgeFilesLines
* server: Mount fails after reboot 1/3 gluster nodesMohit Agrawal2020-01-221-2/+1
| | | | | | | | | | | | | | | | | Problem: At the time of coming up one server node(1x3) after reboot client is unmounted.The client is unmounted because a client is getting AUTH_FAILED event and client call fini for the graph.The client is getting AUTH_FAILED because brick is not attached with a graph at that moment Solution: To avoid the unmounting the client graph throw ENOENT error from server in case if brick is not attached with server at the time of authenticate clients. Credits: Xavi Hernandez <xhernandez@redhat.com> Change-Id: Ie6fbd73cbcf23a35d8db8841b3b6036e87682f5e Fixes: bz#1793852 Signed-off-by: Mohit Agrawal <moagrawa@redhat.com>
* dictionary: remove the 'extra_free' parameterYaniv Kaul2020-01-211-11/+4
| | | | | | | | | | | | | | | This parameter may have been used in the past, but is no longer needed. Removing it and the few locations it was actually referenced. This allows to remove an extra memdup as well, that was not needed in the 1st place in server_setvolume() and unserialize_rsp_direntp() functions. A followup separate patch will remove extra_stdfree parmeter from the dictionary structure. Change-Id: Ica0ff0a330672373aaa60e808b7e76ec489a0fe3 updates: bz#1193929 Signed-off-by: Yaniv Kaul <ykaul@redhat.com>
* protocol/client: Fix a log entryAnoop C S2020-01-151-1/+1
| | | | | | | | | It has been a while since we removed lock healing logic from protocol client. So no need to mention that we healed locks after fd reopen. Change-Id: I24bd3f9e9f2942e306714b2cb83c229ae57c60ae Fixes: bz#1193929 Signed-off-by: Anoop C S <anoopcs@redhat.com>
* debug/io-stats: add an option to set volume-idAmar Tumballi2019-11-291-2/+5
| | | | | | | | | | | 'volume-id' is good to have for a graph for uniquely identifying it. Add it to graph->volume_id while generating volfile itself. This can be further used in many other places. Updates: #763 Change-Id: I80516d62d28a284e8ff4707841570ced97a37e73 Signed-off-by: Amar Tumballi <amar@kadalu.io>
* rpc: align structsYaniv Kaul2019-10-171-15/+15
| | | | | | | | | | | | | squash tens of warnings on padding of structs in afr structures. The warnings were found by manually added '-Wpadded' to the GCC command line. Also made relevant structs and definitions static, where it was applicable. Change-Id: Ib71a7e9c6179378f072d796d11172d086c343e53 updates: bz#1193929 Signed-off-by: Yaniv Kaul <ykaul@redhat.com>
* client xlator: misc. cleanupsYaniv Kaul2019-10-116-662/+474
| | | | | | | | | | | | | - remove dead code - move functions to be static - move some code that only needs to be executed under if branch - remove some dead assignments and redundant checks. No functional change, I hope. Change-Id: I93d952408197ecd2fa91c3f812a73c54242342fa updates: bz#1193929 Signed-off-by: Yaniv Kaul <ykaul@redhat.com>
* protocol/handshake: pass volume-id for extra checkAmar Tumballi2019-09-301-0/+50
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | With added check of volume-id during handshake, we can be sure to not connect with a brick if this gets re-used in another volume. This prevents any accidental issues which can happen with a stale client process lurking along. Also added test case for testing same volume name which would fetch a different volfile (ie, different bricks, different type), and a different volume name, but same brick. For reference: Currently a client<->server handshake happens in glusterfs through protocol/client translator (setvolume) to protocol/server using a dictionary which containes many keys. Rejection happens in server side if some of the required keys are missing in handshake dictionary. Till now, there was no single unique identifier to validate for a client to tell server if it is actually talking to a corresponding server. All we look in protocol/client is a key called 'remote-subvolume', which should match with a subvolume name in server volume file, and for any volume with same brick name (can be present in same cluster due to recreate), it would be same. This could cause major issue, when a client was connected to a given brick, in one volume would be connected to another volume's brick if its re-created/re-used. To prevent this behavior, we are now passing along 'volume-id' in handshake, which would be preserved for the life of client process, which can prevent this accidental connections. NOTE: This behavior wouldn't be applicable for user-snapshot enabled volumes, as snapshotted volume's would have different volume-id. Fixes: bz#1620580 Change-Id: Ie98286e94ce95ae09c2135fd6ec7d7c2ca1e8095 Signed-off-by: Amar Tumballi <amarts@redhat.com>
* protocol/client: don't reopen fds on which POSIX locks are held after a ↵Raghavendra G2019-09-127-6/+109
| | | | | | | | | | | | | | | | | | | reconnect Bricks cleanup any granted locks after a client disconnects and currently these locks are not healed after a reconnect. This means post reconnect a competing process could be granted a lock even though the first process which was granted locks has not unlocked. By not re-opening fds, subsequent operations on such fds will fail forcing the application to close the current fd and reopen a new one. This way we prevent any silent corruption. A new option "client.strict-locks" is introduced to control this behaviour. This option is set to "off" by default. Change-Id: Ieed545efea466cb5e8f5a36199aa26380c301b9e Signed-off-by: Raghavendra G <rgowdapp@redhat.com> updates: bz#1694920
* glusterd, rpc, glusterfsd: fix coverity defects and put required annotationsAtin Mukherjee2019-09-101-2/+6
| | | | | | | | | | | 1404965 - Null pointer dereference 1404316 - Program hangs 1401715 - Program hangs 1401713 - Program hangs Updates: bz#789278 Change-Id: I6e6575daafcb067bc910445f82a9d564f43b75a2 Signed-off-by: Atin Mukherjee <amukherj@redhat.com>
* graph/cleanup: Fix race in graph cleanupMohammed Rafi KC2019-09-051-2/+63
| | | | | | | | | | | | | | | | | We were unconditionally cleaning up the grap when we get child_down followed by parent_down. But this is prone to race condition when some of the bricks are already disconnected. In this case, even before the last child down is executed in the client xlator code,we might have freed the graph. Because the child_down event is alreadt recevied. To fix this race, we have introduced a check to see if all client xlator have cleared thier reconnect chain, and called the child_down for last time. Change-Id: I7d02813bc366dac733a836e0cd7b14a6fac52042 fixes: bz#1727329 Signed-off-by: Mohammed Rafi KC <rkavunga@redhat.com>
* logging: Structured logging reference PRAravinda VK2019-08-202-149/+127
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | To convert the existing `gf_msg` to `gf_smsg`: - Define `_STR` of respective Message ID as below(In `*-messages.h`) #define PC_MSG_REMOTE_OP_FAILED_STR "remote operation failed." - Change `gf_msg` to use `gf_smsg`. Convert values into fields and add any missing fields. Note: `errno` and `error` fields will be added automatically to log message in case errnum is specified. Example: gf_smsg( this->name, // Name or log domain GF_LOG_WARNING, // Log Level rsp.op_errno, // Error number PC_MSG_REMOTE_OP_FAILED, // Message ID "path=%s", local->loc.path, // Key Value 1 "gfid=%s", loc_gfid_utoa(&local->loc), // Key Value 2 NULL // Log End ); Key value pairs formatting Help: gf_slog( this->name, // Name or log domain GF_LOG_WARNING, // Log Level rsp.op_errno, // Error number PC_MSG_REMOTE_OP_FAILED, // Message ID "op=CREATE", // Static Key and Value "path=%s", local->loc.path, // Format for Value "brick-%d-status=%s", brkidx, brkstatus, // Use format for key and val NULL // Log End ); Before: [2019-07-03 08:16:18.226819] W [MSGID: 114031] [client-rpc-fops_v2.c \ :2633:client4_0_lookup_cbk] 0-gv3-client-0: remote operation failed. \ Path: / (00000000-0000-0000-0000-000000000001) [Transport endpoint \ is not connected] After: [2019-07-29 07:50:15.773765] W [MSGID: 114031] \ [client-rpc-fops_v2.c:2633:client4_0_lookup_cbk] 0-gv1-client-0: \ remote operation failed. [{path=/f1}, \ {gfid=00000000-0000-0000-0000-000000000000}, \ {errno=107}, {error=Transport endpoint is not connected}] To add new `gf_smsg`, Add a Message ID in respective `*-messages.h` file and the follow the steps mentioned above. Change-Id: I4e7d37f27f106ab398e991d931ba2ac7841a44b1 Updates: #657 Signed-off-by: Aravinda VK <avishwan@redhat.com>
* protocol/client - fixing a coverity issueBarak Sason2019-08-201-3/+3
| | | | | | | | | | Moved null pointer check up in order to avoid seg-fault CID: 1404258 Updates: bz#789278 Change-Id: Ib97e05302bfeb8fe38d6ce9870b9740cb576e492 Signed-off-by: Barak Sason <bsasonro@redhat.com>
* client-handshake.c: minor changes and removal of dead code.Yaniv Kaul2019-08-142-287/+54
| | | | | | | | | | - Removal of quite a bit of dead code. - Use dict_set_str_sizen and friends where applicable. - Moved some functions to be static and initialize values right away. Change-Id: Ic25b5da4028198694a0e24796dea375661eb66b9 updates: bz#1193929 Signed-off-by: Yaniv Kaul <ykaul@redhat.com>
* event: rename event_XXX with gf_ prefixedXiubo Li2019-07-291-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I hit one crash issue when using the libgfapi. In the libgfapi it will call glfs_poller() --> event_dispatch() in file api/src/glfs.c:721, and the event_dispatch() is defined by libgluster locally, the problem is the name of event_dispatch() is the extremly the same with the one from libevent package form the OS. For example, if a executable program Foo, which will also use and link the libevent and the libgfapi at the same time, I can hit the crash, like: kernel: glfs_glfspoll[68486]: segfault at 1c0 ip 00007fef006fd2b8 sp 00007feeeaffce30 error 4 in libevent-2.0.so.5.1.9[7fef006ed000+46000] The link for Foo is: lib_foo_LADD = -levent $(GFAPI_LIBS) It will crash. This is because the glfs_poller() is calling the event_dispatch() from the libevent, not the libglsuter. The gfapi link info : GFAPI_LIBS = -lacl -lgfapi -lglusterfs -lgfrpc -lgfxdr -luuid If I link Foo like: lib_foo_LADD = $(GFAPI_LIBS) -levent It will works well without any problem. And if Foo call one private lib, such as handler_glfs.so, and the handler_glfs.so will link the GFAPI_LIBS directly, while the Foo won't and it will dlopen(handler_glfs.so), then the crash will be hit everytime. The link info will be: foo_LADD = -levent libhandler_glfs_LIBADD = $(GFAPI_LIBS) I can avoid the crash temporarily by linking the GFAPI_LIBS in Foo too like: foo_LADD = $(GFAPI_LIBS) -levent libhandler_glfs_LIBADD = $(GFAPI_LIBS) But this is ugly since the Foo won't use any APIs from the GFAPI_LIBS. And in some cases when the --as-needed link option is added(on many dists it is added as default), then the crash is back again, the above workaround won't work. Fixes: #699 Change-Id: I38f0200b941bd1cff4bf3066fca2fc1f9a5263aa Signed-off-by: Xiubo Li <xiubli@redhat.com>
* (multiple files) use dict_allocate_and_serialize() where applicable.Yaniv Kaul2019-07-221-11/+3
| | | | | | | | This function does length, allocation and serialization for you. Change-Id: I142a259952a2fe83dd719442afaefe4a43a8e55e updates: bz#1193929 Signed-off-by: Yaniv Kaul <ykaul@redhat.com>
* protocol/client: propagte GF_EVENT_CHILD_PING only for connections to brickRaghavendra G2019-06-271-4/+12
| | | | | | | | | | | | | | | | | | | | | | | Two reasons: * ping responses from glusterd may not be relevant for Halo replication. Instead, it might be interested in only knowing whether the brick itself is responsive. * When a brick is killed, propagating GF_EVENT_CHILD_PING of ping response from glusterd results in GF_EVENT_DISCONNECT spuriously propagated to parent xlators. These DISCONNECT events are from the connections client establishes with glusterd as part of its reconnect logic. Without GF_EVENT_CHILD_PING, the last event propagated to parent xlators would be the first DISCONNECT event from brick and hence subsequent DISCONNECTS to glusterd are not propagated as protocol/client prevents same event being propagated to parent xlators consecutively. propagating GF_EVENT_CHILD_PING for ping responses from glusterd would change the last_sent_event to GF_EVENT_CHILD_PING and hence protocol/client cannot prevent subsequent DISCONNECT events Signed-off-by: Raghavendra G <rgowdapp@redhat.com> Fixes: bz#1716979 Change-Id: I50276680c52f05ca9e12149a3094923622d6eaef
* across: coverity fixesAmar Tumballi2019-06-031-0/+3
| | | | | | | | | | | | | | | * locks/posix.c: key was not freed in one of the cases. * locks/common.c: lock was being free'd out of context. * nfs/exports: handle case of missing free. * protocol/client: handle case of entry not freed. * storage/posix: handle possible case of double free CID: 1398628, 1400731, 1400732, 1400756, 1124796, 1325526 updates: bz#789278 Change-Id: Ieeaca890288bc4686355f6565f853dc8911344e8 Signed-off-by: Amar Tumballi <amarts@redhat.com> Signed-off-by: Sheetal Pamecha <spamecha@redhat.com>
* Fix some "Null pointer dereference" coverity issuesXavi Hernandez2019-05-261-1/+6
| | | | | | | | | | | | | | | | | | | | | | This patch fixes the following CID's: * 1124829 * 1274075 * 1274083 * 1274128 * 1274135 * 1274141 * 1274143 * 1274197 * 1274205 * 1274210 * 1274211 * 1288801 * 1398629 Change-Id: Ia7c86cfab3245b20777ffa296e1a59748040f558 Updates: bz#789278 Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>
* protocol: remove compound fopAmar Tumballi2019-04-294-2851/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | Compound fops are kept on wire as a backward compatibility with older AFR modules. The AFR module used beyond 4.x releases are not using compound fops. Hence removing the compound fop in the protocol code. Note that, compound-fops was already an 'option' in AFR, and completely removed since 4.1.x releases. So, point to note is, with this change, we have 2 ways to upgrade when clients of 3.x series are present. i) set 'use-compound-fops' option to 'false' on any volume which is of replica type. And then upgrade the servers. ii) Do a two step upgrade. First from current version (which will already be EOL if it's using compound) to a 4.1..6.x version, and then an upgrade to 7.x. Consider the overall code which we are removing for the option seems quite high, I believe it is worth it. updates: bz#1693692 Signed-off-by: Amar Tumballi <amarts@redhat.com> Change-Id: I0a8876d0367a15e1410ec845f251d5d3097ee593
* Replace memdup() with gf_memdup()Vijay Bellur2019-04-121-1/+1
| | | | | | | | | memdup() and gf_memdup() have the same implementation. Removed one API as the presence of both can be confusing. Change-Id: I562130c668457e13e4288e592792872d2e49887e updates: bz#1193929 Signed-off-by: Vijay Bellur <vbellur@redhat.com>
* client/fini: return fini after rpc cleanupMohammed Rafi KC2019-04-112-6/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | There is a race condition in rpc_transport later and client fini. Sequence of events to happen the race condition 1) When we want to destroy a graph, we send a parent down event first 2) Once parent down received on a client xlator, we will initiates a rpc disconnect 3) This will in turn generates a child down event. 4) When we process child down, we first do fini for Every xlator 5) On successful return of fini, we delete the graph Here after the step 5, there is a chance that the fini on client might not be finished. Because an rpc_tranpsort ref can race with the above sequence. So we have to wait till all rpc's are successfully freed before returning the fini from client Change-Id: I20145662d71fb837e448a4d3210d1fcb2855f2d4 fixes: bz#1659708 Signed-off-by: Mohammed Rafi KC <rkavunga@redhat.com>
* protocol: add an option to force using old-protocolAmar Tumballi2019-04-103-3/+24
| | | | | | | | | | | | | | As protocol implements every fop, and in general a large part of the codebase. Considering our regression is run mostly in 1 machine, there was no way of forcing the client to use old protocol (while new one is available). With this patch, a new 'testing' option is provided which forces client to use old protocol if found. This should help increase the code coverage by at least 10k lines overall. updates: bz#1693692 Change-Id: Ie45256f7dea250671b689c72b4b6f25037cef948 Signed-off-by: Amar Tumballi <amarts@redhat.com>
* mgmt/shd: Implement multiplexing in self heal daemonMohammed Rafi KC2019-04-011-1/+30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Problem: Shd daemon is per node, which means they create a graph with all volumes on it. While this is a great for utilizing resources, it is so good in terms of performance and managebility. Because self-heal daemons doesn't have capability to automatically reconfigure their graphs. So each time when any configurations changes happens to the volumes(replicate/disperse), we need to restart shd to bring the changes into the graph. Because of this all on going heal for all other volumes has to be stopped in the middle, and need to restart all over again. Solution: This changes makes shd as a per volume daemon, so that the graph will be generated for each volumes. When we want to start/reconfigure shd for a volume, we first search for an existing shd running on the node, if there is none, we will start a new process. If already a daemon is running for shd, then we will simply detach a graph for a volume and reatach the updated graph for the volume. This won't touch any of the on going operations for any other volumes on the shd daemon. Example of an shd graph when it is per volume graph ----------------------- | debug-iostat | ----------------------- / | \ / | \ --------- --------- ---------- | AFR-1 | | AFR-2 | | AFR-3 | -------- --------- ---------- A running shd daemon with 3 volumes will be like--> graph ----------------------- | debug-iostat | ----------------------- / | \ / | \ ------------ ------------ ------------ | volume-1 | | volume-2 | | volume-3 | ------------ ------------ ------------ Change-Id: Idcb2698be3eeb95beaac47125565c93370afbd99 fixes: bz#1659708 Signed-off-by: Mohammed Rafi KC <rkavunga@redhat.com>
* protocol/client: Do not fallback to anon-fd if fd is not openPranith Kumar K2019-03-311-1/+7
| | | | | | | | | | | | | | | | | | If an open comes on a file when a brick is down and after the brick comes up, a fop comes on the fd, client xlator would still wind the fop on anon-fd leading to wrong behavior of the fops in some cases. Example: If lk fop is issued on the fd just after the brick is up in the scenario above, lk fop will be sent on anon-fd instead of failing it on that client xlator. This lock will never be freed upon close of the fd as flush on anon-fd is invalid and is not wound below server xlator. As a fix, failing the fop unless the fd has FALLBACK_TO_ANON_FD flag. Change-Id: I77692d056660b2858e323bdabdfe0a381807cccc fixes bz#1390914 Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
* client-rpc: Fix the payload being sent on the wirePoornima G2019-03-296-244/+308
| | | | | | | | | | | | | | | | | | | The fops allocate 3 kind of payload(buffer) in the client xlator: - fop payload, this is the buffer allocated by the write and put fop - rsphdr paylod, this is the buffer required by the reply cbk of some fops like lookup, readdir. - rsp_paylod, this is the buffer required by the reply cbk of fops like readv etc. Currently, in the lookup and readdir fop the rsphdr is sent as payload, hence the allocated rsphdr buffer is also sent on the wire, increasing the bandwidth consumption on the wire. With this patch, the issue is fixed. Fixes: bz#1692093 Change-Id: Ie8158921f4db319e60ad5f52d851fa5c9d4a269b Signed-off-by: Poornima G <pgurusid@redhat.com>
* clang: Fix various missing checks for empty listShyamsundarR2018-12-141-4/+6
| | | | | | | | | | | | | | | | | | | | When using list_for_each_entry(_safe) functions, care needs to be taken that the list passed in are not empty, as these functions are not empty list safe. clag scan reported various points where this this pattern could be caught, and this patch fixes the same. Additionally the following changes are present in this patch, - Added an explicit op_ret setting in error case in the macro MAKE_INODE_HANDLE to address another clang issue reported - Minor refactoring of some functions in quota code, to address possible allocation failures in certain functions (which in turn cause possible empty lists to be passed around) Change-Id: I1e761a8d218708f714effb56fa643df2a3ea2cc7 Updates: bz#1622665 Signed-off-by: ShyamsundarR <srangana@redhat.com>
* copy_file_range support in GlusterFSRaghavendra Bhat2018-12-126-0/+265
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * libglusterfs changes to add new fop * Fuse changes: - Changes in fuse bridge xlator to receive and send responses * posix changes to perform the op on the backend filesystem * protocol and rpc changes for sending and receiving the fop * gfapi changes for performing the fop * tools: glfs-copy-file-range tool for testing copy_file_range fop - Although, copy_file_range support has been added to the upstream fuse kernel module, no release has been made yet of a kernel which contains the support. It is expected to come in the upcoming release of linux-4.20 So, as of now, executing copy_file_range fop on a fused based filesystem results in fuse kernel module sending read on the source fd and write on the destination fd. Therefore a small gfapi based tool has been written to be able test the copy_file_range fop. This tool is similar (in functionality) to the example program given in copy_file_range man page. So, running regular copy_file_range on a fuse mount point and running gfapi based glfs-copy-file-range tool gives some idea about how fast, the copy_file_range (or reflink) can be. On the local machine this was the result obtained. mount -t glusterfs workstation:new /mnt/glusterfs [root@workstation ~]# cd /mnt/glusterfs/ [root@workstation glusterfs]# ls file [root@workstation glusterfs]# cd [root@workstation ~]# time /tmp/a.out /mnt/glusterfs/file /mnt/glusterfs/new real 0m6.495s user 0m0.000s sys 0m1.439s [root@workstation ~]# time glfs-copy-file-range $(hostname) new /tmp/glfs.log /file /rrr OPEN_SRC: opening /file is success OPEN_DST: opening /rrr is success FSTAT_SRC: fstat on /rrr is success copy_file_range successful real 0m0.309s user 0m0.039s sys 0m0.017s This tool needs following arguments 1) hostname 2) volume name 3) log file path 4) source file path (relative to the gluster volume root) 5) destination file path (relative to the gluster volume root) "glfs-copy-file-range <hostname> <volume> <log file path> <source> <destination>" - Added a testcase as well to run glfs-copy-file-range tool * io-stats changes to capture the fop for profiling * NOTE: - Added conditional check to see whether the copy_file_range syscall is available or not. If not, then return ENOSYS. - Added conditional check for kernel minor version in fuse_kernel.h and fuse-bridge while referring to copy_file_range. And the kernel minor version is kept as it is. i.e. 24. Increment it in future when there is a kernel release which contains the support for copy_file_range fop in fuse kernel module. * The document which contains a writeup on this enhancement can be found at https://docs.google.com/document/d/1BSILbXr_knynNwxSyyu503JoTz5QFM_4suNIh2WwrSc/edit Change-Id: I280069c814dd21ce6ec3be00a884fc24ab692367 updates: #536 Signed-off-by: Raghavendra Bhat <raghavendra@redhat.com>
* all: add xlator_api to many translatorsAmar Tumballi2018-12-061-0/+15
| | | | | | Fixes: #164 Change-Id: I93ad6f0232a1dc534df099059f69951e1339086f Signed-off-by: Amar Tumballi <amarts@redhat.com>
* libglusterfs: Move devel headers under glusterfs directoryShyamsundarR2018-12-0512-36/+36
| | | | | | | | | | | | | | | | | | | | | | | | libglusterfs devel package headers are referenced in code using include semantics for a program, this while it works can be better especially when dealing with out of tree xlator builds or in general out of tree devel package usage. Towards this, the following changes are done, - moved all devel headers under a glusterfs directory - Included these headers using system header notation <> in all code outside of libglusterfs - Included these headers using own program notation "" within libglusterfs This change although big, is just moving around the headers and making it correct when including these headers from other sources. This helps us correctly include libglusterfs includes without namespace conflicts. Change-Id: Id2a98854e671a7ee5d73be44da5ba1a74252423b Updates: bz#1193929 Signed-off-by: ShyamsundarR <srangana@redhat.com>
* Multiple xlator .h files: remove unused private gf_* memory types.Yaniv Kaul2018-11-301-1/+0
| | | | | | | | | | | | | It seems there were quite a few unused enums (that in turn cause unndeeded memory allocation) in some xlators. I've removed them, hopefully not causing any damage. Compile-tested only! updates: bz#1193929 Signed-off-by: Yaniv Kaul <ykaul@redhat.com> Change-Id: I8252bd763dc1506e2d922496d896cd2fc0886ea7
* core: create a constant for default network timeoutXavi Hernandez2018-11-231-1/+1
| | | | | | | | | | A new constant named GF_NETWORK_TIMEOUT has been defined and all references to the hard-coded timeout of 42 seconds have been replaced with this constant. Change-Id: Id30f5ce4f1230f9288d9e300538624bcf1a6da27 fixes: bz#1652852 Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>
* protocol/client: unchecked return valueShwetha Acharya2018-11-201-1/+6
| | | | | | | | | | | | | | Problem: In client_process_response_v2, value returned from function client_post_common_dict is not checked for errors before being used. Solution: Added a check condition to resolve the issue. CID: 1390020 Change-Id: I4d297f33c8dd332ae5f6f21667a4871133b2b570 updates: bz#789278 Signed-off-by: Shwetha Acharya <sacharya@redhat.com>
* all: fix the format string exceptionsAmar Tumballi2018-11-051-2/+2
| | | | | | | | | | | | | | | | Currently, there are possibilities in few places, where a user-controlled (like filename, program parameter etc) string can be passed as 'fmt' for printf(), which can lead to segfault, if the user's string contains '%s', '%d' in it. While fixing it, makes sense to make the explicit check for such issues across the codebase, by making the format call properly. Fixes: CVE-2018-14661 Fixes: bz#1644763 Change-Id: Ib547293f2d9eb618594cbff0df3b9c800e88bde4 Signed-off-by: Amar Tumballi <amarts@redhat.com>
* protocol: remove the option 'verify-volfile-checksum'Amar Tumballi2018-11-051-84/+1
| | | | | | | | | | | | | | | | 'getspec' operation is not used between 'client' and 'server' ever since we have off-loaded volfile management to glusterd, ie, at least 7 years. No reason to keep the dead code! The removed option had no meaning, as glusterd didn't provide a way to set (or unset) this option. So, no regression should be observed from any of the existing glusterfs deployment, supported or unsupported. Updates: CVE-2018-14653 Updates: bz#1644756 Change-Id: I4a2e0f673c5bcd4644976a61dbd2d37003a428eb Signed-off-by: Amar Tumballi <amarts@redhat.com>
* Land part 2 of clang-format changesGluster Ant2018-09-128-19701/+19598
| | | | | Change-Id: Ia84cc24c8924e6d22d02ac15f611c10e26db99b4 Signed-off-by: Nigel Babu <nigelb@redhat.com>
* Land clang-format changesGluster Ant2018-09-124-789/+733
| | | | Change-Id: I6f5d8140a06f3c1b2d196849299f8d483028d33b
* Multiple files: calloc -> mallocYaniv Kaul2018-09-041-21/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | xlators/storage/posix/src/posix-inode-fd-ops.c: xlators/storage/posix/src/posix-helpers.c: xlators/storage/bd/src/bd.c: xlators/protocol/client/src/client-lk.c: xlators/performance/quick-read/src/quick-read.c: xlators/performance/io-cache/src/page.c xlators/nfs/server/src/nfs3-helpers.c xlators/nfs/server/src/nfs-fops.c xlators/nfs/server/src/mount3udp_svc.c xlators/nfs/server/src/mount3.c xlators/mount/fuse/src/fuse-helpers.c xlators/mount/fuse/src/fuse-bridge.c xlators/mgmt/glusterd/src/glusterd-utils.c xlators/mgmt/glusterd/src/glusterd-syncop.h xlators/mgmt/glusterd/src/glusterd-snapshot.c xlators/mgmt/glusterd/src/glusterd-rpc-ops.c xlators/mgmt/glusterd/src/glusterd-replace-brick.c xlators/mgmt/glusterd/src/glusterd-op-sm.c xlators/mgmt/glusterd/src/glusterd-mgmt.c xlators/meta/src/subvolumes-dir.c xlators/meta/src/graph-dir.c xlators/features/trash/src/trash.c xlators/features/shard/src/shard.h xlators/features/shard/src/shard.c xlators/features/marker/src/marker-quota.c xlators/features/locks/src/common.c xlators/features/leases/src/leases-internal.c xlators/features/gfid-access/src/gfid-access.c xlators/features/cloudsync/src/cloudsync-plugins/src/cloudsyncs3/src/libcloudsyncs3.c xlators/features/bit-rot/src/bitd/bit-rot.c xlators/features/bit-rot/src/bitd/bit-rot-scrub.c bxlators/encryption/crypt/src/metadata.c xlators/encryption/crypt/src/crypt.c xlators/performance/md-cache/src/md-cache.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible It doesn't make sense to calloc (allocate and clear) memory when the code right away fills that memory with data. It may be optimized by the compiler, or have a microscopic performance improvement. In some cases, also changed allocation size to be sizeof some struct or type instead of a pointer - easier to read. In some cases, removed redundant strlen() calls by saving the result into a variable. 1. Only done for the straightforward cases. There's room for improvement. 2. Please review carefully, especially for string allocation, with the terminating NULL string. Only compile-tested! .. and allocate memory as much as needed. xlators/nfs/server/src/mount3.c : Don't blindly allocate PATH_MAX, but strlen() the string and allocate appropriately. Also, align error messges. updates: bz#1193929 Original-Author: Yaniv Kaul <ykaul@redhat.com> Signed-off-by: Yaniv Kaul <ykaul@redhat.com> Signed-off-by: Yaniv Kaul <ykaul@redhat.com> Change-Id: Ibda6f33dd180b7f7694f20a12af1e9576fe197f5
* protocol: coverity fixesBhumika Goyal2018-08-222-8/+4
| | | | | | | | Fixes CID: 1389388 1389320 1274113 1388881 1388623 1124801 1124795 Change-Id: Ia72abc0560c959b0298f42e25abdfc5523755569 updates: bz#789278 Signed-off-by: Bhumika Goyal <bgoyal@redhat.com>
* xlators: protocol: Fix deferencing pointer after free coverity issuesBhumika Goyal2018-08-213-32/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | | The pointer of type struct iobuf * is getting dereferenced after getting freed by iobuf_unref function. Therefore, move this function after all the dereferences of this pointer type. Also, it is useful coding standard to have iobuf_unref just after iobref_add. So, move iobref_add too. Occurences found using Coccinelle script: @@ identifier rsphdr_iobuf; expression E; identifier func; @@ *iobuf_unref(rsphdr_iobuf); ... *E = func(rsphdr_iobuf); Fixes CID: 1390517, 1390278, 1388666, 1356588, 1356587 at [1]. and also some more occurences which were found using the above script but not caught by Coverity. [1]. https://scan6.coverity.com/reports.htm#v42388/p10714/fileInstanceId=84384920&defectInstanceId=25600709&mergedDefectId=1388666 Change-Id: I579e9d12698f14e9e24bc926c6efef16bac5c06c updates: bz#789278 Signed-off-by: Bhumika Goyal <bgoyal@redhat.com>
* build: rename event.h to gf-event.hNiels de Vos2018-07-271-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | Newer FreeBSD versions (noticed with 10.3-RELEASE) provide a event.h file that on occasion gets included instead of the libglusterfs file. When this happens, 'struct event_pool' will not be defined and building will fail with errors like: autoscale-threads.c:18:55: error: incomplete definition of type 'struct event_pool' int thread_count = pool->eventthreadcount; ~~~~^ autoscale-threads.c:17:16: note: forward declaration of 'struct event_pool' struct event_pool *pool = ctx->event_pool; ^ This problem is caused by 'pkg-config --cflags uuid' that adds /usr/local/include to the GF_CPPFLAGS. The use of libuuid is preferred so that the contrib/uuid/ directory can be removed. By renaming event.h to gf-event.h there is no conflict between the different event.h files anymore and compiling on FreeBSD works without issues. Change-Id: Ie69f6b8a4f8f8e9630d39a86693eb74674f0f763 Updates: bz#1607319 Signed-off-by: Niels de Vos <ndevos@redhat.com>
* All: run codespell on the code and fix issues.Yaniv Kaul2018-07-221-1/+1
| | | | | | | | | | | | Please review, it's not always just the comments that were fixed. I've had to revert of course all calls to creat() that were changed to create() ... Only compile-tested! Change-Id: I7d02e82d9766e272a7fd9cc68e51901d69e5aab5 updates: bz#1193929 Signed-off-by: Yaniv Kaul <ykaul@redhat.com>
* protocol/client: handle the fdctx_destroy properly with different versionsAmar Tumballi2018-07-052-74/+133
| | | | | | | | while adding the new version of RPC, this part was not handled properly Updates: bz#1193929 Change-Id: If4cc4c2db075221b9ed731bacb7cc035f7007c5b Signed-off-by: Amar Tumballi <amarts@redhat.com>
* client: remove the "connecting" state - it's not usedMichael Adam2018-06-213-6/+0
| | | | | | | | | The "connecting" state is not used anywhere really. It's only being set and printed. So remove it. Change-Id: I11fc8b0bdcda5a812d065543aa447d39957d3b38 fixes: bz#1583583 Signed-off-by: Michael Adam <obnox@samba.org>
* protocol/client: Remove code duplicationKrutika Dhananjay2018-06-153-119/+22
| | | | | | | | | | | | | | | client_submit_vec_request() which is used by WRITEV, and PUT and client_submit_request() used by the rest of the fops have almost similar code. However, there have been some more checks - such as whether setvolume was successful or not, and one more that is send-gid-specific - that have been missed out in the vectored version of the function. This patch fixes this code duplication. Change-Id: I363a28eeead6219cb1009dc831538153e8bd7d40 fixes: bz#1591580 Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com>
* rpc/clnt: Don't let consumers manage "connected" stateRaghavendra G2018-06-041-2/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The state management of "connected" in rpc is ad-hoc as far as the responsibility goes. Note that there is nothing wrong with functionality itself. rpc layer manages this state in disconnect codepath and has exposed an api to manage this one from consumers. Note that rpc layer never sets "connected" to true by itself, which forces the consumers to use this api to get a working rpc connection. The situation is best captured from a comment in code from Jeff Darcy in glusterfsd/src/gf-attach.c: -/* - * In a sane world, the generic RPC layer would be capable of tracking - * connection status by itself, with no help from us. It might invoke our - * callback if we had registered one, but only to provide information. Sadly, - * we don't live in that world. Instead, the callback *must* exist and *must* - * call rpc_clnt_{set,unset}_connected, because that's the only way those - * fields get set (with RPC both above and below us on the stack). If we don't - * do that, then rpc_clnt_submit doesn't think we're connected even when we - * are. It calls the socket code to reconnect, but the socket code tracks this - * stuff in a sane way so it knows we're connected and returns EINPROGRESS. - * Then we're stuck, connected but unable to use the connection. To make it - * work, we define and register this trivial callback. - */ Also, consumers of rpc know about state of connection only through the notifications sent by rpc-clnt. So, consumers don't have any extra information to manage the state and hence letting them manage the state is counter intuitive. This patch cleans that up and instead moves the responsibility of state management of rpc layer into itself. Change-Id: I31e641a60795fc480ca753917f4b2579f1e05094 Signed-off-by: Raghavendra G <rgowdapp@redhat.com> Fixes: bz#1585585
* protocol/client: Don't send fops till SETVOLUME is completeRaghavendra G2018-05-312-3/+9
| | | | | | | | | | | | | | | | | An earlier commit set conf->connected just after rpc layer sends RPC_CLNT_CONNECT event. However, success of socket level connection connection doesn't indicate brick stack is ready to receive fops, as an handshake has to be done b/w client and server after RPC_CLNT_CONNECT event. Any fop sent to brick in the window between, * protocol/client receiving RPC_CLNT_CONNECT event * protocol/client receiving a successful setvolume response can end up accessing an uninitialized brick stack. So, set conf->connected only after a successful SETVOLUME. Change-Id: I139a03d2da6b0d95a0d68391fcf54b00e749decf fixes: bz#1583937 Signed-off-by: Raghavendra G <rgowdapp@redhat.com>
* client/protocol: fix the log level for removexattr_cbkAmar Tumballi2018-05-172-2/+12
| | | | | | | | | | noticed that server protocol actually logs all the errors for removexattr as INFO, instead of WARNING like client, and hence, doesn't create a confusion in user. updates: bz#1576418 Change-Id: Ia6681e9ee433fda3c77a4509906c78333396e339 Signed-off-by: Amar Tumballi <amarts@redhat.com>
* protocol: Fix 4.0 client, parsing older iatt in dictShyamsundarR2018-03-101-0/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | In a mixed mode cluster involving 4.0 and older 3.x bricks, if clients are newer, then the iatt encoded in the dictionary can be of the older iatt format, which a newer client will map incorrectly to the newer structure. This causes failures in FOPs that depend on this iatt for some functionality (seen in mkdir operations failing as EIO, when DHT hits its internal setxattr call). The fix provided is to convert the iatt in the dict, based on which RPC version is used to communicate with the server. IOW, this is the reverse of change in commit "b966c7790e" Tested using a mixed mode cluster (i.e bricks in 3.12 and 4.0 versions) and a mixed set of clients, 3.12 and 4.0 clients. There is no regression test provided, as this needs a mixed mode cluster to test and validate. Change-Id: I454e54651ca836b9f7c28f45f51d5956106aefa9 BUG: 1554053 Signed-off-by: ShyamsundarR <srangana@redhat.com>
* protocol/client: fix memory corruptionXavi Hernandez2018-03-096-92/+78
| | | | | | | | | | | | | There was an issue when some accesses to saved_fds list were protected by the wrong mutex (lock instead of fd_lock). Additionally, the retrieval of fdctx from fd's context and any checks done on it have also been protected by fd_lock to avoid fdctx to become outdated just after retrieving it. Change-Id: If2910508bcb7d1ff23debb30291391f00903a6fe BUG: 1553129 Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>
* protcol/client: Insert dummy clnt-lk-version to avoid upgrade failureAnoop C S2018-02-141-0/+9
| | | | | | | | | | | | | | | | | With https://review.gluster.org/#/c/12363/ being merged, we no longer send client's lk-version to server side and the corresponding check on server is also removed. But when clients are upgraded prior to servers, the check for lk-version at server side fails and is reported back to clients resulting in disconnection. Since we don't have lock-recovery (lk-version and grace-timeout) logic anymore in code base our best bet would be to add client's default lk-version i.e, 1, into the dictionary just to make server side check pass and continue with remaining SETVOLUME operations. Change-Id: I441b67bd271d1e9ba9a7c08703e651c7a6bd945b BUG: 1544699 Signed-off-by: Anoop C S <anoopcs@redhat.com>