glusterfs.git/xlators/storage, branch v3.4.7beta3

storage/posix: removing deleting entries in case of creation failures

2014-10-20T14:54:23+00:00

The code is not atomic enough to not to delete a dentry created by a
prallel dentry creation operation.

Change-Id: I9bd6d2aa9e7a1c0688c0a937b02a4b4f56d7aa3d
BUG: 1139988
Signed-off-by: Raghavendra G 
Reviewed-on: http://review.gluster.org/8327
Reviewed-by: Pranith Kumar Karampuri 
Tested-by: Gluster Build System 
Reviewed-by: Vijay Bellur 
Reviewed-on: http://review.gluster.org/8675
Reviewed-by: Kaleb KEITHLEY

cluster/dht: Fix races to avoid deletion of linkto

2014-10-20T14:53:56+00:00

 file

Explanation of Race between rebalance processes:
https://bugzilla.redhat.com/show_bug.cgi?id=1110694#c4

scenario-1:
===========

STATE 1:                          BRICK-1
only one brick                   Cached File
in the system

STATE 2:
Add brick-2                       BRICK-1                BRICK-2

STATE 3:                                       Lookup of File on brick-2
                                               by this node's rebalance
                                               will fail because hashed
                                               file is not created yet.
                                               So dht_lookup_everywhere is
                                               about to get called.

STATE 4:                         As part of lookup
                                 link file at brick-2
                                 will be created.

STATE 5:                         getxattr to check that
                                 cached file belongs to
                                 this node is done

STATE 6:

                                            dht_lookup_everywhere_cbk detects
                                            the link created by rebalance-1.
                                            It will unlink it.

STATE 7:                        getxattr at the link
                                file with "pathinfo" key
                                will be called will fail
                                as the link file is deleted
                                by rebalance on node-2

Fix:
So in the STATE 6, we should avoid the deletion of link file. Every time
dht_lookup_everywhere gets called, lookup will be performed on all the nodes.
So to avoid STATE 6, if linkto file is found, it is not deleted until valid
case is found in dht_lookup_everywhere_done.

Case 1: if linkto file points to cached node, and cached file exists,
        uwind with success.

Case 2: if linkto does not point to current cached node, and cached file
        exists:
        a) Unlink stale link file
        b) Create new link file

Case 3: Only linkto file exists:
        Delete linkto file

Case 4: Only cached file
        Create link file (Handled event without patch)

Case 5: Neither cached nor hashed file is present
        Return with ENOENT (handled even without patch)

Reviewed-on: http://review.gluster.org/8231

******************************************************************************

scenario-2:
===========
cluster/dht: Modified logic of linkto file deletion on non-hashed

Currently whenever dht_lookup_everywhere gets called, if in
dht_lookup_everywhere_cbk, a linkto file is found on non-hashed
subvolume, file is unlinked. But there are cases when this file
is under migration. Under such condition, we should avoid deletion
of file.

When  some other rebalance process changes the layout of parent
such that dst_file (w.r.t. migration) falls on non-hashed node,
then may be lookup could have found it as linkto file but just
before unlink, file  is under migration or already migrated
In such cased unlink can be avoided.

Race:
-------
If we have two bricks (brick-1 and brick-2) with initial file "a"
under BaseDir which is hashed as well as cached on (brick-1).

Assume "a"  hashing gives 44.

                              Brick-1              Brick-2

Initial Setup:               BaseDir/a             BaseDir
                             [1-50]                [51-100]

Now add new-brick Brick-3.

1. Rebalance-1 on node Node-1 (Brick-1 node) will reset
the BaseDir Layout.

2. After that it will perform
a)  Create linkto file on  new-hashed (brick-2)
b)  Perform file migration.

1.Rebalance-1 Fixes the base-layout:
                 Brick-1             Brick-2           Brick-3
                 ---------         ----------         ------------
                 BaseDir/a            BaseDir           BaseDir
                  [1-33]              [34-66]           [67-100]

2. Only a) is     BaseDir/a          BaseDir/a(linkto)   BaseDir
   performed                         Create linktofile

Now rebalance 2 on node-2 jumped in and it will perform
step 1 and 2-a.

After (rebal-2, step-1), it changes the layout of the BaseDir.
                    BaseDir/a     BaseDir/a(link)    BaseDir
                    [67-100]           [1-33]        [34-66]

For  (rebale-2, step-2), It will perform lookup at Brick-3 as w.r.t new
layout 44 falls for brick-3. But lookup will fail.
So  dht_lookup_everywhere gets called.

NOTE: On brick-2 by rebalance-1, a linkto file was created.

Currently that linkto files gets deleted by rebalance-2 lookup as it
is considered as stale linkto file.  But  with patch if rebalance is
already in progress or rebalance is over,  linkto file will not be
unlinked. If rebalance is in progress fd will be  open and if rebalance
is over then linkto file wont be set.

Reviewed-on: http://review.gluster.org/8345

*******************************************************************************

scenario-3:
===========

cluster/dht: Added keys in dht_lookup_everywhere_done

Case where both cached  (C1)  and hashed file are found,
but hash does not point to above cached node (C1), then
dont unlink if either fd-is-open on hashed or
linkto-xattr is not found.

Reviewed-on: http://review.gluster.org/8429
BUG: 1139995
Signed-off-by: Venkatesh Somyajulu 
Change-Id: I86d0a21d4c0501c45d837101ced4f96d6fedc5b9
Signed-off-by: Venkatesh Somyajulu 
Tested-by: Gluster Build System 
Reviewed-by: susant palai 
Reviewed-by: Raghavendra G 
Reviewed-by: Vijay Bellur 
Reviewed-on: http://review.gluster.org/8674
Reviewed-by: Kaleb KEITHLEY

core: fix Ubuntu code audit (cppcheck) results

2014-05-25T10:52:01+00:00

These block inclusion in Ubuntu Main repo.

AFAICT these are false positives:

[rpc/rpc-transport/rdma/src/rdma.c:3074]: (error) Address of local auto-variable assigned to a function parameter.
[xlators/features/marker/utils/src/gsyncd.c:99]: (error) Memory leak: str
[xlators/features/marker/utils/src/gsyncd.c:354]: (error) Memory leak: argv
[xlators/nfs/server/src/nlm4.c:1176]: (error) Possible null pointer dereference: fde

The remainder are fixed with this change-set:

[api/src/glfs-fops.c:700]: (error) Possible null pointer dereference: gio
[api/src/glfs-fops.c:702]: (error) Possible null pointer dereference: frame
[xlators/cluster/afr/src/afr-inode-write.c:375]: (error) Possible null pointer dereference: frame
[xlators/cluster/afr/src/afr-self-heal-common.c:1522]: (error) Possible null pointer dereference: local
[xlators/cluster/dht/src/dht-rebalance.c:1574]: (error) Possible null pointer dereference: ctx
[xlators/cluster/stripe/src/stripe.c:4407]: (error) Possible null pointer dereference: local
[xlators/mgmt/glusterd/src/glusterd-mountbroker.c:675]: (error) Possible null pointer dereference: cookieswitch
[xlators/mgmt/glusterd/src/glusterd-mountbroker.c:677]: (error) Possible null pointer dereference: cookieswitch
[xlators/mgmt/glusterd/src/glusterd-replace-brick.c:924]: (error) Resource leak: file
[xlators/mgmt/glusterd/src/glusterd-replace-brick.c:1008]: (error) Resource leak: file
[xlators/mgmt/glusterd/src/glusterd-sm.c:248]: (error) Possible null pointer dereference: new_ev_ctx
[xlators/mgmt/glusterd/src/glusterd-store.c:1250]: (error) Possible null pointer dereference: handle
[xlators/mgmt/glusterd/src/glusterd-utils.c:4272]: (error) Possible null pointer dereference: this
[xlators/mgmt/glusterd/src/glusterd-utils.c:5113]: (error) Possible null pointer dereference: this
[xlators/mount/fuse/src/fuse-bridge.c:4432]: (error) Uninitialized variable: finh
[xlators/mount/fuse/src/fuse-bridge.c:2927]: (error) Possible null pointer dereference: state
[xlators/mount/fuse/src/fuse-bridge.c:3226]: (error) Possible null pointer dereference: state
[xlators/storage/bd_map/src/bd_map.c:1504]: (error) Possible null pointer dereference: bd_fd
[xlators/storage/bd_map/src/bd_map.c:1728]: (error) Possible null pointer dereference: n_entry
[xlators/storage/bd_map/src/bd_map.c:1741]: (error) Possible null pointer dereference: n_entry
[xlators/performance/quick-read/src/quick-read.c:585]: (error) Possible null pointer dereference: iobuf

rerunning cppcheck --force afterwards:

Test code, don't care:
[extras/test/test-ffop.c:27]: (error) Buffer overrun possible for long command line arguments.

False positive after fix
[xlators/cluster/stripe/src/stripe.c:4407]: (error) Possible null pointer dereference: local

Still false positive:
[xlators/features/marker/utils/src/gsyncd.c:354]: (error) Memory leak: argv
[xlators/nfs/server/src/nlm4.c:1176]: (error) Possible null pointer dereference: fde

Not built, don't care:
[xlators/cluster/ha/src/ha.c:2699]: (error) Possible null pointer dereference: priv

Change-Id: I1fb849e9c042d3a3701cb05121d413e58e73d505
BUG: 1086460
Signed-off-by: Kaleb S. KEITHLEY 
Reviewed-on: http://review.gluster.org/7583
Reviewed-by: Vijay Bellur 
Tested-by: Gluster Build System

storage/posix: do not dereference gfid symlinks before posix_handle_mkdir_hashes()

2014-05-22T11:09:16+00:00

Whenever a new directory is created, its corresponding gfid file must
also be created. This was done first calling MAKE_HANDLE_PATH() to get
the path of the gfid file, then calling posix_handle_mkdir_hashes() to
create the parent directories of the gfid, and finally creating the
soft-link.

In normal circumstances, the gfid we want to create won't exist and
MAKE_HANDLE_PATH() will return a simple path to the new gfid. However if
the volume is damaged and a self-heal is running, it is possible that we
try to create an already existing gfid. In this case, MAKE_HANDLE_PATH()
will return a path to the directory instead of the path to the gfid.

To solve this problem, every time a path to a gfid is needed, a call to
MAKE_HANDLE_ABSPATH() is made instead of the call to MAKE_HANDLE_PATH().

BUG: 1099955
Change-Id: I5bcd2b3c38d172c75946f33519e057e76d960a24
Signed-off-by: Xavier Hernandez 
Reviewed-on: http://review.gluster.org/6737
Reviewed-by: Kaleb KEITHLEY 
Tested-by: Gluster Build System

storage/posix: lgetxattr called with invalid keys on the bricks

2014-03-24T15:25:25+00:00

More invalid keys have crept in since this was fixed. We need a better
strategy for avoiding this than the current noticed-in-an-strace...

Cleaned tabs while I'm at it.

Change-Id: I00bd10471c4e4caf32b8a5b38660268324aa7a10
BUG: 765202
Signed-off-by: Kaleb S. KEITHLEY 
Reviewed-on: http://review.gluster.org/7005
Tested-by: Gluster Build System 
Reviewed-by: Pranith Kumar Karampuri 
Reviewed-by: Jeff Darcy

Revert "core: fix errno for non-existent GFID"

2013-12-24T11:31:53+00:00

This reverts commit 837422858c2e4ab447879a4141361fd382645406

Change-Id: I0909f26ce088454bb14b3694b489c672286a4ae6
Reviewed-on: http://review.gluster.org/6575
Tested-by: Gluster Build System 
Reviewed-by: Vijay Bellur

posix: if brick-uid or brick-gid is not specified, do not set

2013-12-17T07:15:24+00:00

Current code would set owner uid/gid explicitly to 0/0 on start
even if none was specified. Fix it.

Change-Id: I72dec9e79c51bd1eb3af5334c42b7c23b01d0258
BUG: 1040275
Signed-off-by: Anand Avati 
Reviewed-on: http://review.gluster.org/6476
Tested-by: Gluster Build System 
Tested-by: Lukáš Bezdička 
Reviewed-by: Krishnan Parthasarathi 
Reviewed-by: Vijay Bellur 
Reviewed-on: http://review.gluster.org/6519
Reviewed-by: Lukáš Bezdička

core: fix errno for non-existent GFID

2013-11-26T19:49:05+00:00

When clients refer to a GFID which does not exist, the errno to
be returned in ESTALE (and not ENOENT). Even though ENOENT might
look "proper" most of the time, as the application eventually expects
ENOENT even if a parent directory does not exist, not returning
ESTALE results in resolvers (FUSE and GFAPI) to not retry resolution
in uncached mode. This can result in spurious ENOENTs during
concurrent path modification operations.

Change-Id: I7a06ea6d6a191739f2e9c6e333a1969615e05936
BUG: 1032894
Signed-off-by: Anand Avati 
Reviewed-on: http://review.gluster.org/6322
Tested-by: Gluster Build System

build: declare lvm_lv_from_name() if it is missing from lvm2app.h

2013-06-27T12:39:43+00:00

The bd-xlator can not be built successfully on certain Debian
distributions due to a missing declaration of lvm_lv_from_name(). This
function is available for linking, but it does not exist in the header
file.

This change adds a detection for lvm_lv_from_name() in both the library
for linking, and the declaration in the header file. If the 1st is
missing, the bd-xlator can not be built, and if only the 2nd one is
missing, we'll declare lvm_lv_from_name() ourselves. This makes it
possible to build the bd-xlator on the affected Debian distributions
too.

Change-Id: If1845f6b6d676793677ebbcc6daf9ff12f7c3fd6
BUG: 976946
Signed-off-by: Niels de Vos 
Reviewed-on: http://review.gluster.org/5260
Reviewed-by: Vijay Bellur 
Tested-by: Gluster Build System

dht,posix: support for case discovery

2013-06-08T21:51:22+00:00

This is support for discovering a filename in a given directory
which has a case insensitive match of a given name. It is implemented
as a virtual extended attribute on the directory where the required
filename is specified in the key.

E.g:

  sh# getfattr -e "text" -n user.glusterfs.get_real_filename:FiLe-B /mnt/samba/patchy
  getfattr: Removing leading '/' from absolute path names
  # file: mnt/samba/patchy
  user.glusterfs.get_real_filename:FiLe-B="file-b"

In reality, there can be multiple "answers" as the backend filesystem is
case sensitive and there can be multiple files which can strcasecamp()
successfully. In this case we pick the first matched file from the first
responding server.

If a matching file does not exist, we return ENOENT (and NOT ENODATA).
This way the caller can differentiate between "unsupported" glusterfs
API and file not existing.

This API is used by Samba VFS to perform efficient discovery of the real
filename without doing a full scan at the Samba level.

Change-Id: I53054c4067cba69e585fd0bbce004495bc6e39e8
BUG: 953694
Signed-off-by: Anand Avati 
Reviewed-on: http://review.gluster.org/5163
Tested-by: Gluster Build System 
Reviewed-by: Vijay Bellur