Bug 764051 (GLUSTER-2319)

Summary:	DHT uses wrong subvolume for fsetxattr
Product:	[Community] GlusterFS	Reporter:	Jeff Darcy <jdarcy>
Component:	distribute	Assignee:	Amar Tumballi <amarts>
Status:	CLOSED CURRENTRELEASE	QA Contact:
Severity:	medium	Docs Contact:
Priority:	medium
Version:	mainline	CC:	amarts, gluster-bugs, lakshmipathi, sgowda, vijay, vraman
Target Milestone:	---
Target Release:	---
Hardware:	All
OS:	Linux
URL:	http://git.fedorahosted.org/git/?p=CloudFS.git;a=blob;f=patches/dht-fsetxattr.patch;h=4abee523672a74ec261f5bfdf40e52329b8e3192;hb=HEAD
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:		Type:	---
Regression:	RTNR	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:

Description Jeff Darcy 2011-01-25 15:36:05 UTC

Because it doesn't actually define an fsetxattr fop, DHT ends up using default_fsetxattr which passes through to FIRST_CHILD.  Unfortunately, since the open would have gone to the cached/hashed subvol for this file, this means we're passing to a different protocol/client than we used for the open and we get EBADFD back . . . most of the time.  1/N times, the first volume will be the correct one and the call will work properly.  This makes the behavior for open+fsetxattr both incorrect and inconsistent with the behavior for setxattr alone.

The patch at the attached URL fixes this problem by defining an fsetxattr fop which redirects to the appropriate subvolume the same way as other calls do.

Comment 1 shishir gowda 2011-01-27 04:10:12 UTC

Hi Jeff,

A minor change in your patch is needed:

err:
  47 +       op_errno = (op_errno == -1) ? errno : op_errno;
  48 +       DHT_STACK_UNWIND (setxattr, frame, -1, op_errno); <--unwind needs to for fsetxattr call and not setxattr call

Comment 2 Jeff Darcy 2011-01-27 09:44:34 UTC

Nice catch.  Thanks!

Comment 3 Amar Tumballi 2011-02-16 06:47:45 UTC

Hi Jeff,

I will be re-sending this patch to GlusterFS mainline. Hope thats fine right? If you have concerns please let me know so we will not apply it (or revert it if applied).

Regards,
Amar

Comment 4 Jeff Darcy 2011-02-16 12:04:59 UTC

(In reply to comment #3)
> I will be re-sending this patch to GlusterFS mainline. Hope thats fine right?
> If you have concerns please let me know so we will not apply it (or revert it
> if applied).

No concerns here; I think that's excellent.  Thanks, Amar.

Comment 5 Anand Avati 2011-02-19 04:32:36 UTC

PATCH: http://patches.gluster.com/patch/6213 in master (cluster/dht : implement a fsetxattr() command)