[nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Orion Poplawski]

I filed a bug here: https://bugzilla.redhat.com/show_bug.cgi?id=808112

Description of problem:

client A:
touch blah
ln -s blah blahlink

client B:
open("blahlink", O_RDONLY|O_CREAT, 0666) = -1 EEXIST (File exists)
$ ls -l blahlink
lrwxrwxrwx. 1 orion cora 4 Mar 29 09:30 blahlink -> blah
open("blahlink", O_RDONLY|O_CREAT, 0666) = 3

Removing and recreating the link on client A restores the problem.

earth:/export/home/orion on /home/orion type nfs
(rw,noatime,intr,rsize=32768,wsize=32768,actimeo=1,sloppy,vers=4)

I thought this might be the same as the recent stat() issue brought up in
relation to viminfo files, but it fails on kernels with that fix.

I can't reproduce on RHEL5 kernel 2.6.18-308.1.1.el5, but I can on
2.6.42.9-2.fc15.x86_64 through 3.4.0-0.rc0.git1.2.fc18.x86_64.

Re: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Myklebust, Trond]

T24gVGh1LCAyMDEyLTAzLTI5IGF0IDE2OjI4ICswMDAwLCBPcmlvbiBQb3BsYXdza2kgd3JvdGU6
DQo+IEkgZmlsZWQgYSBidWcgaGVyZTogaHR0cHM6Ly9idWd6aWxsYS5yZWRoYXQuY29tL3Nob3df
YnVnLmNnaT9pZD04MDgxMTINCj4gDQo+IERlc2NyaXB0aW9uIG9mIHByb2JsZW06DQo+IA0KPiBj
bGllbnQgQToNCj4gdG91Y2ggYmxhaA0KPiBsbiAtcyBibGFoIGJsYWhsaW5rDQo+IA0KPiBjbGll
bnQgQjoNCj4gb3BlbigiYmxhaGxpbmsiLCBPX1JET05MWXxPX0NSRUFULCAwNjY2KSA9IC0xIEVF
WElTVCAoRmlsZSBleGlzdHMpDQoNClRoYXQgc291bmRzIG1vcmUgbGlrZSBhIHNlcnZlciBidWcu
IEl0IHNob3VsZG4ndCBiZSByZXBseWluZw0KTkZTNEVSUl9FWElTVCBoZXJlLCBzaW5jZSB0aGlz
IGlzbid0IGFuIGV4Y2x1c2l2ZSBjcmVhdGU7IGl0IHNob3VsZA0KcmF0aGVyIGJlIHJlcGx5aW5n
IHdpdGggTkZTNEVSUl9TWU1MSU5LLg0KDQpXaGljaCBzZXJ2ZXIgYXJlIHlvdSB0ZXN0aW5nIGFn
YWluc3QsIGFuZCB3aGF0IGRvZXMgdGhlIHdpcmVzaGFyayB0cmFjZQ0Kc2hvdz8NCg0KLS0gDQpU
cm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHANClRy
b25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQp3d3cubmV0YXBwLmNvbQ0KDQo=

Re: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Myklebust, Trond]

T24gVGh1LCAyMDEyLTAzLTI5IGF0IDExOjMyIC0wNjAwLCBPcmlvbiBQb3BsYXdza2kgd3JvdGU6
DQo+IE9uIDAzLzI5LzIwMTIgMTA6NTQgQU0sIE15a2xlYnVzdCwgVHJvbmQgd3JvdGU6DQo+ID4g
T24gVGh1LCAyMDEyLTAzLTI5IGF0IDE2OjI4ICswMDAwLCBPcmlvbiBQb3BsYXdza2kgd3JvdGU6
DQo+ID4+IEkgZmlsZWQgYSBidWcgaGVyZTogaHR0cHM6Ly9idWd6aWxsYS5yZWRoYXQuY29tL3No
b3dfYnVnLmNnaT9pZD04MDgxMTINCj4gPj4NCj4gPj4gRGVzY3JpcHRpb24gb2YgcHJvYmxlbToN
Cj4gPj4NCj4gPj4gY2xpZW50IEE6DQo+ID4+IHRvdWNoIGJsYWgNCj4gPj4gbG4gLXMgYmxhaCBi
bGFobGluaw0KPiA+Pg0KPiA+PiBjbGllbnQgQjoNCj4gPj4gb3BlbigiYmxhaGxpbmsiLCBPX1JE
T05MWXxPX0NSRUFULCAwNjY2KSA9IC0xIEVFWElTVCAoRmlsZSBleGlzdHMpDQo+ID4NCj4gPiBU
aGF0IHNvdW5kcyBtb3JlIGxpa2UgYSBzZXJ2ZXIgYnVnLiBJdCBzaG91bGRuJ3QgYmUgcmVwbHlp
bmcNCj4gPiBORlM0RVJSX0VYSVNUIGhlcmUsIHNpbmNlIHRoaXMgaXNuJ3QgYW4gZXhjbHVzaXZl
IGNyZWF0ZTsgaXQgc2hvdWxkDQo+ID4gcmF0aGVyIGJlIHJlcGx5aW5nIHdpdGggTkZTNEVSUl9T
WU1MSU5LLg0KPiA+DQo+ID4gV2hpY2ggc2VydmVyIGFyZSB5b3UgdGVzdGluZyBhZ2FpbnN0LCBh
bmQgd2hhdCBkb2VzIHRoZSB3aXJlc2hhcmsgdHJhY2UNCj4gPiBzaG93Pw0KPiA+DQo+IA0KPiBX
ZWxsLCBJIGNhbiByZXByb2R1Y2UgaXQgYWdhaW5zdCBhbiBFTDYuMiBzZXJ2ZXIgd2l0aCBhbiBF
TDYuMiBjbGllbnQgd2l0aCB2My4gDQo+ICAgRmlyc3QgdGVzdHMgd2VyZSB3aXRoIGFuZCBFTDUu
OCBzZXJ2ZXIuICBJIGNhbid0IHJlcHJvZHVjZSB3aXRoIEZlZG9yYSANCj4gY2xpZW50cyBhZ2Fp
bnN0IHRoZSBFTDYuMiBzZXJ2ZXIgd2l0aCB2My4NCj4gDQo+IFRoZSBFTDYuMiBjbGllbnQvc2Vy
dmVyIGV4Y2hhbmdlIGluIG5mc2VsLmxvZywgRjE1L0VMNi4yIGV4Y2hhbmdlIGluIG5mc2YxNS5s
b2cNCj4gDQo+IFRoZSBFTDYuMiBjbGllbnQgYXBwZWFycyB0byBiZSB1c2luZyBhIHZlcnNpb24g
NCBjYWxsIGV2ZW4gdGhvdWdoIEkgdGhpbmsgaXQncyANCj4gbW91bnRlZCB2MzoNCj4gDQo+IHNh
Z2E6L2V4cG9ydC9zdyBvbiAvZGF0YS9zdyB0eXBlIG5mcyANCj4gKHJ3LG5vYXRpbWUsaW50cixy
c2l6ZT0zMjc2OCx3c2l6ZT0zMjc2OCxzbG9wcHksYWRkcj0xMC4xMC4xMC4yKQ0KDQoNCkl0IGhh
cyBnb3QgdG8gYmUgbW91bnRlZCB2NC4gSSdkIGNoZWNrIC9wcm9jL21vdW50cy4NCg0KPiBHb2lu
ZyBiYWNrIHRvIHY0IG9uIEVMNS44IHNlcnZlcjogbmZzdjRlbC5sb2csIG5mc3Y0ZjE4LmxvZw0K
PiANCj4gQm90aCBnZXQgTkZTNEVSUl9FWElTVCBpbiB0aGlzIGNhc2UuDQoNCldoaWNoIGlzIGFu
IG9idmlvdXMgc2VydmVyIGJ1ZzogaXQgc2hvdWxkIGJlIHNlbmRpbmcgTkZTNEVSUl9TWU1MSU5L
IGluDQpyZXBseSB0byB0aGF0IE9QRU4uDQoNCkJydWNlPw0KDQotLSANClRyb25kIE15a2xlYnVz
dA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJvbmQuTXlrbGVidXN0
QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29tDQoNCg==

Re: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Orion Poplawski]

On 03/29/2012 11:40 AM, Myklebust, Trond wrote:
> On Thu, 2012-03-29 at 11:32 -0600, Orion Poplawski wrote:
>> On 03/29/2012 10:54 AM, Myklebust, Trond wrote:
>>> On Thu, 2012-03-29 at 16:28 +0000, Orion Poplawski wrote:
>>>> I filed a bug here: https://bugzilla.redhat.com/show_bug.cgi?id=808112
>>>>
>>>> Description of problem:
>>>>
>>>> client A:
>>>> touch blah
>>>> ln -s blah blahlink
>>>>
>>>> client B:
>>>> open("blahlink", O_RDONLY|O_CREAT, 0666) = -1 EEXIST (File exists)
>>>
>>> That sounds more like a server bug. It shouldn't be replying
>>> NFS4ERR_EXIST here, since this isn't an exclusive create; it should
>>> rather be replying with NFS4ERR_SYMLINK.
>>>
>>> Which server are you testing against, and what does the wireshark trace
>>> show?
>>>
>>
>> Well, I can reproduce it against an EL6.2 server with an EL6.2 client with v3.
>>    First tests were with and EL5.8 server.  I can't reproduce with Fedora
>> clients against the EL6.2 server with v3.
>>
>> The EL6.2 client/server exchange in nfsel.log, F15/EL6.2 exchange in nfsf15.log
>>
>> The EL6.2 client appears to be using a version 4 call even though I think it's
>> mounted v3:
>>
>> saga:/export/sw on /data/sw type nfs
>> (rw,noatime,intr,rsize=32768,wsize=32768,sloppy,addr=10.10.10.2)
>
>
> It has got to be mounted v4. I'd check /proc/mounts.
>

saga:/export/sw /data/sw nfs 
rw,noatime,vers=3,rsize=32768,wsize=32768,namlen=255,hard,proto=tcp,timeo=600,retrans=2,sec=sys,mountaddr=10.10.10.2,mountvers=3,mountport=51114,mountproto=udp,local_lock=none,addr=10.10.10.2 
0 0

But I see:

Network File System, Ops(3): PUTFH ACCESS GETATTR
     [Program Version: 4]
     [V4 Procedure: COMP (1)]


>> Going back to v4 on EL5.8 server: nfsv4el.log, nfsv4f18.log
>>
>> Both get NFS4ERR_EXIST in this case.
>
> Which is an obvious server bug: it should be sending NFS4ERR_SYMLINK in
> reply to that OPEN.
>
> Bruce?
>

I can reproduce with a 3.4.0-0.rc0.git1.2.fc18 server as well.

-- 
Orion Poplawski
Technical Manager                     303-415-9701 x222
NWRA, Boulder Office                  FAX: 303-415-9702
3380 Mitchell Lane                       orion@nwra.com
Boulder, CO 80301                   http://www.nwra.com

Re: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Dr James Bruce Fields]

On Thu, Mar 29, 2012 at 12:07:17PM -0600, Orion Poplawski wrote:
> On 03/29/2012 11:40 AM, Myklebust, Trond wrote:
> >>Going back to v4 on EL5.8 server: nfsv4el.log, nfsv4f18.log
> >>
> >>Both get NFS4ERR_EXIST in this case.
> >
> >Which is an obvious server bug: it should be sending NFS4ERR_SYMLINK in
> >reply to that OPEN.
> >
> >Bruce?
> >
> 
> I can reproduce with a 3.4.0-0.rc0.git1.2.fc18 server as well.

Hm.  So how about this?  (Untested.)

Probably there should be a pynfs test too.

I'm assuming it should still be ERR_EXIST in the exclusive,
exclusive4_1, and guarded cases.

--b.

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 7423d71..2bfcad4 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1457,9 +1457,12 @@ do_nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
 
 		switch (createmode) {
 		case NFS3_CREATE_UNCHECKED:
-			if (! S_ISREG(dchild->d_inode->i_mode))
-				err = nfserr_exist;
-			else if (truncp) {
+			if (! S_ISREG(dchild->d_inode->i_mode)) {
+				if (rqstp->rq_vers == 4)
+					err = nfserr_symlink;
+				else
+					err = nfserr_exist;
+			} else if (truncp) {
 				/* in nfsv4, we need to treat this case a little
 				 * differently.  we don't want to truncate the
 				 * file now; this would be wrong if the OPEN

Re: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Myklebust, Trond]

T24gVGh1LCAyMDEyLTAzLTI5IGF0IDE1OjMxIC0wNDAwLCBEciBKYW1lcyBCcnVjZSBGaWVsZHMg
d3JvdGU6DQo+IE9uIFRodSwgTWFyIDI5LCAyMDEyIGF0IDEyOjA3OjE3UE0gLTA2MDAsIE9yaW9u
IFBvcGxhd3NraSB3cm90ZToNCj4gPiBPbiAwMy8yOS8yMDEyIDExOjQwIEFNLCBNeWtsZWJ1c3Qs
IFRyb25kIHdyb3RlOg0KPiA+ID4+R29pbmcgYmFjayB0byB2NCBvbiBFTDUuOCBzZXJ2ZXI6IG5m
c3Y0ZWwubG9nLCBuZnN2NGYxOC5sb2cNCj4gPiA+Pg0KPiA+ID4+Qm90aCBnZXQgTkZTNEVSUl9F
WElTVCBpbiB0aGlzIGNhc2UuDQo+ID4gPg0KPiA+ID5XaGljaCBpcyBhbiBvYnZpb3VzIHNlcnZl
ciBidWc6IGl0IHNob3VsZCBiZSBzZW5kaW5nIE5GUzRFUlJfU1lNTElOSyBpbg0KPiA+ID5yZXBs
eSB0byB0aGF0IE9QRU4uDQo+ID4gPg0KPiA+ID5CcnVjZT8NCj4gPiA+DQo+ID4gDQo+ID4gSSBj
YW4gcmVwcm9kdWNlIHdpdGggYSAzLjQuMC0wLnJjMC5naXQxLjIuZmMxOCBzZXJ2ZXIgYXMgd2Vs
bC4NCj4gDQo+IEhtLiAgU28gaG93IGFib3V0IHRoaXM/ICAoVW50ZXN0ZWQuKQ0KPiANCj4gUHJv
YmFibHkgdGhlcmUgc2hvdWxkIGJlIGEgcHluZnMgdGVzdCB0b28uDQo+IA0KPiBJJ20gYXNzdW1p
bmcgaXQgc2hvdWxkIHN0aWxsIGJlIEVSUl9FWElTVCBpbiB0aGUgZXhjbHVzaXZlLA0KPiBleGNs
dXNpdmU0XzEsIGFuZCBndWFyZGVkIGNhc2VzLg0KPiANCj4gLS1iLg0KPiANCj4gZGlmZiAtLWdp
dCBhL2ZzL25mc2QvdmZzLmMgYi9mcy9uZnNkL3Zmcy5jDQo+IGluZGV4IDc0MjNkNzEuLjJiZmNh
ZDQgMTAwNjQ0DQo+IC0tLSBhL2ZzL25mc2QvdmZzLmMNCj4gKysrIGIvZnMvbmZzZC92ZnMuYw0K
PiBAQCAtMTQ1Nyw5ICsxNDU3LDEyIEBAIGRvX25mc2RfY3JlYXRlKHN0cnVjdCBzdmNfcnFzdCAq
cnFzdHAsIHN0cnVjdCBzdmNfZmggKmZocCwNCj4gIA0KPiAgCQlzd2l0Y2ggKGNyZWF0ZW1vZGUp
IHsNCj4gIAkJY2FzZSBORlMzX0NSRUFURV9VTkNIRUNLRUQ6DQo+IC0JCQlpZiAoISBTX0lTUkVH
KGRjaGlsZC0+ZF9pbm9kZS0+aV9tb2RlKSkNCj4gLQkJCQllcnIgPSBuZnNlcnJfZXhpc3Q7DQo+
IC0JCQllbHNlIGlmICh0cnVuY3ApIHsNCj4gKwkJCWlmICghIFNfSVNSRUcoZGNoaWxkLT5kX2lu
b2RlLT5pX21vZGUpKSB7DQo+ICsJCQkJaWYgKHJxc3RwLT5ycV92ZXJzID09IDQpDQo+ICsJCQkJ
CWVyciA9IG5mc2Vycl9zeW1saW5rOw0KPiArCQkJCWVsc2UNCj4gKwkJCQkJZXJyID0gbmZzZXJy
X2V4aXN0Ow0KDQpOby4gVGhpcyBzaG91bGQgX25ldmVyXyByZXR1cm4gTkZTNEVSUl9FWElTVC4N
Cg0KSXQgc2hvdWxkIHJldHVybg0KICAgICAgKiBORlM0RVJSX0lTRElSIGlmIHRoZSBvYmplY3Qg
aXMgYSBkaXJlY3RvcnkNCiAgICAgICogTkZTNEVSUl9TWU1MSU5LIGlmIGl0IGlzIHN5bWJvbGlj
IGxpbmssDQogICAgICAqIGVpdGhlciBORlM0RVJSX1dST05HX1RZUEUgKE5GU3Y0LjEpIG9yIE5G
UzRFUlJfU1lNTElOSyAoTkZTdjQuMCkNCiAgICAgICAgaWYgdGhlIG9iamVjdCBpcyBhbnkgb3Ro
ZXIgbm9uLXJlZ3VsYXIgZmlsZS4NCg0KDQo+ICsJCQl9IGVsc2UgaWYgKHRydW5jcCkgew0KPiAg
CQkJCS8qIGluIG5mc3Y0LCB3ZSBuZWVkIHRvIHRyZWF0IHRoaXMgY2FzZSBhIGxpdHRsZQ0KPiAg
CQkJCSAqIGRpZmZlcmVudGx5LiAgd2UgZG9uJ3Qgd2FudCB0byB0cnVuY2F0ZSB0aGUNCj4gIAkJ
CQkgKiBmaWxlIG5vdzsgdGhpcyB3b3VsZCBiZSB3cm9uZyBpZiB0aGUgT1BFTg0KDQotLSANClRy
b25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBtYWludGFpbmVyDQoNCk5ldEFwcA0KVHJv
bmQuTXlrbGVidXN0QG5ldGFwcC5jb20NCnd3dy5uZXRhcHAuY29tDQoNCg==

Re: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Myklebust, Trond]

T24gVGh1LCAyMDEyLTAzLTI5IGF0IDE2OjE2IC0wNDAwLCBUcm9uZCBNeWtsZWJ1c3Qgd3JvdGU6
DQo+IE9uIFRodSwgMjAxMi0wMy0yOSBhdCAxNTozMSAtMDQwMCwgRHIgSmFtZXMgQnJ1Y2UgRmll
bGRzIHdyb3RlOg0KPiA+IE9uIFRodSwgTWFyIDI5LCAyMDEyIGF0IDEyOjA3OjE3UE0gLTA2MDAs
IE9yaW9uIFBvcGxhd3NraSB3cm90ZToNCj4gPiA+IE9uIDAzLzI5LzIwMTIgMTE6NDAgQU0sIE15
a2xlYnVzdCwgVHJvbmQgd3JvdGU6DQo+ID4gPiA+PkdvaW5nIGJhY2sgdG8gdjQgb24gRUw1Ljgg
c2VydmVyOiBuZnN2NGVsLmxvZywgbmZzdjRmMTgubG9nDQo+ID4gPiA+Pg0KPiA+ID4gPj5Cb3Ro
IGdldCBORlM0RVJSX0VYSVNUIGluIHRoaXMgY2FzZS4NCj4gPiA+ID4NCj4gPiA+ID5XaGljaCBp
cyBhbiBvYnZpb3VzIHNlcnZlciBidWc6IGl0IHNob3VsZCBiZSBzZW5kaW5nIE5GUzRFUlJfU1lN
TElOSyBpbg0KPiA+ID4gPnJlcGx5IHRvIHRoYXQgT1BFTi4NCj4gPiA+ID4NCj4gPiA+ID5CcnVj
ZT8NCj4gPiA+ID4NCj4gPiA+IA0KPiA+ID4gSSBjYW4gcmVwcm9kdWNlIHdpdGggYSAzLjQuMC0w
LnJjMC5naXQxLjIuZmMxOCBzZXJ2ZXIgYXMgd2VsbC4NCj4gPiANCj4gPiBIbS4gIFNvIGhvdyBh
Ym91dCB0aGlzPyAgKFVudGVzdGVkLikNCj4gPiANCj4gPiBQcm9iYWJseSB0aGVyZSBzaG91bGQg
YmUgYSBweW5mcyB0ZXN0IHRvby4NCj4gPiANCj4gPiBJJ20gYXNzdW1pbmcgaXQgc2hvdWxkIHN0
aWxsIGJlIEVSUl9FWElTVCBpbiB0aGUgZXhjbHVzaXZlLA0KPiA+IGV4Y2x1c2l2ZTRfMSwgYW5k
IGd1YXJkZWQgY2FzZXMuDQo+ID4gDQo+ID4gLS1iLg0KPiA+IA0KPiA+IGRpZmYgLS1naXQgYS9m
cy9uZnNkL3Zmcy5jIGIvZnMvbmZzZC92ZnMuYw0KPiA+IGluZGV4IDc0MjNkNzEuLjJiZmNhZDQg
MTAwNjQ0DQo+ID4gLS0tIGEvZnMvbmZzZC92ZnMuYw0KPiA+ICsrKyBiL2ZzL25mc2QvdmZzLmMN
Cj4gPiBAQCAtMTQ1Nyw5ICsxNDU3LDEyIEBAIGRvX25mc2RfY3JlYXRlKHN0cnVjdCBzdmNfcnFz
dCAqcnFzdHAsIHN0cnVjdCBzdmNfZmggKmZocCwNCj4gPiAgDQo+ID4gIAkJc3dpdGNoIChjcmVh
dGVtb2RlKSB7DQo+ID4gIAkJY2FzZSBORlMzX0NSRUFURV9VTkNIRUNLRUQ6DQo+ID4gLQkJCWlm
ICghIFNfSVNSRUcoZGNoaWxkLT5kX2lub2RlLT5pX21vZGUpKQ0KPiA+IC0JCQkJZXJyID0gbmZz
ZXJyX2V4aXN0Ow0KPiA+IC0JCQllbHNlIGlmICh0cnVuY3ApIHsNCj4gPiArCQkJaWYgKCEgU19J
U1JFRyhkY2hpbGQtPmRfaW5vZGUtPmlfbW9kZSkpIHsNCj4gPiArCQkJCWlmIChycXN0cC0+cnFf
dmVycyA9PSA0KQ0KPiA+ICsJCQkJCWVyciA9IG5mc2Vycl9zeW1saW5rOw0KPiA+ICsJCQkJZWxz
ZQ0KPiA+ICsJCQkJCWVyciA9IG5mc2Vycl9leGlzdDsNCj4gDQo+IE5vLiBUaGlzIHNob3VsZCBf
bmV2ZXJfIHJldHVybiBORlM0RVJSX0VYSVNULg0KPiANCj4gSXQgc2hvdWxkIHJldHVybg0KPiAg
ICAgICAqIE5GUzRFUlJfSVNESVIgaWYgdGhlIG9iamVjdCBpcyBhIGRpcmVjdG9yeQ0KPiAgICAg
ICAqIE5GUzRFUlJfU1lNTElOSyBpZiBpdCBpcyBzeW1ib2xpYyBsaW5rLA0KPiAgICAgICAqIGVp
dGhlciBORlM0RVJSX1dST05HX1RZUEUgKE5GU3Y0LjEpIG9yIE5GUzRFUlJfU1lNTElOSyAoTkZT
djQuMCkNCj4gICAgICAgICBpZiB0aGUgb2JqZWN0IGlzIGFueSBvdGhlciBub24tcmVndWxhciBm
aWxlLg0KDQpCYXNpY2FsbHksIGlmIGFuIG9iamVjdCBhbHJlYWR5IGV4aXN0cyB3aXRoIHRoYXQg
bmFtZSwgdGhlbg0KTkZTM19DUkVBVEVfVU5DSEVDS0VEIHNob3VsZCBiZSB0cmVhdGVkIGFzIGlm
IGl0IGlzIGFuIG9yZGluYXJ5IE9QRU4gKGluDQp0aGUgY2FzZSBvZiBORlN2NCkgb3IgTE9PS1VQ
IChpbiB0aGUgY2FzZSBvZiBORlN2MykuDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBO
RlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNv
bQ0Kd3d3Lm5ldGFwcC5jb20NCg0K

Re: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Dr James Bruce Fields]

On Thu, Mar 29, 2012 at 08:16:05PM +0000, Myklebust, Trond wrote:
> On Thu, 2012-03-29 at 15:31 -0400, Dr James Bruce Fields wrote:
> > On Thu, Mar 29, 2012 at 12:07:17PM -0600, Orion Poplawski wrote:
> > > On 03/29/2012 11:40 AM, Myklebust, Trond wrote:
> > > >>Going back to v4 on EL5.8 server: nfsv4el.log, nfsv4f18.log
> > > >>
> > > >>Both get NFS4ERR_EXIST in this case.
> > > >
> > > >Which is an obvious server bug: it should be sending NFS4ERR_SYMLINK in
> > > >reply to that OPEN.
> > > >
> > > >Bruce?
> > > >
> > > 
> > > I can reproduce with a 3.4.0-0.rc0.git1.2.fc18 server as well.
> > 
> > Hm.  So how about this?  (Untested.)
> > 
> > Probably there should be a pynfs test too.
> > 
> > I'm assuming it should still be ERR_EXIST in the exclusive,
> > exclusive4_1, and guarded cases.
> > 
> > --b.
> > 
> > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > index 7423d71..2bfcad4 100644
> > --- a/fs/nfsd/vfs.c
> > +++ b/fs/nfsd/vfs.c
> > @@ -1457,9 +1457,12 @@ do_nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
> >  
> >  		switch (createmode) {
> >  		case NFS3_CREATE_UNCHECKED:
> > -			if (! S_ISREG(dchild->d_inode->i_mode))
> > -				err = nfserr_exist;
> > -			else if (truncp) {
> > +			if (! S_ISREG(dchild->d_inode->i_mode)) {
> > +				if (rqstp->rq_vers == 4)
> > +					err = nfserr_symlink;
> > +				else
> > +					err = nfserr_exist;
> 
> No. This should _never_ return NFS4ERR_EXIST.

That "else" is the v3 CREATE case.  I don't see the alternative there?

> It should return
>       * NFS4ERR_ISDIR if the object is a directory
>       * NFS4ERR_SYMLINK if it is symbolic link,
>       * either NFS4ERR_WRONG_TYPE (NFSv4.1) or NFS4ERR_SYMLINK (NFSv4.0)
>         if the object is any other non-regular file.

Makes sense.

--b.

> 
> 
> > +			} else if (truncp) {
> >  				/* in nfsv4, we need to treat this case a little
> >  				 * differently.  we don't want to truncate the
> >  				 * file now; this would be wrong if the OPEN
> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer
> 
> NetApp
> Trond.Myklebust@netapp.com
> www.netapp.com
> 

Re: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Myklebust, Trond]

T24gVGh1LCAyMDEyLTAzLTI5IGF0IDE2OjQzIC0wNDAwLCBEciBKYW1lcyBCcnVjZSBGaWVsZHMg
d3JvdGU6DQo+IE9uIFRodSwgTWFyIDI5LCAyMDEyIGF0IDA4OjE2OjA1UE0gKzAwMDAsIE15a2xl
YnVzdCwgVHJvbmQgd3JvdGU6DQo+ID4gT24gVGh1LCAyMDEyLTAzLTI5IGF0IDE1OjMxIC0wNDAw
LCBEciBKYW1lcyBCcnVjZSBGaWVsZHMgd3JvdGU6DQo+ID4gPiBPbiBUaHUsIE1hciAyOSwgMjAx
MiBhdCAxMjowNzoxN1BNIC0wNjAwLCBPcmlvbiBQb3BsYXdza2kgd3JvdGU6DQo+ID4gPiA+IE9u
IDAzLzI5LzIwMTIgMTE6NDAgQU0sIE15a2xlYnVzdCwgVHJvbmQgd3JvdGU6DQo+ID4gPiA+ID4+
R29pbmcgYmFjayB0byB2NCBvbiBFTDUuOCBzZXJ2ZXI6IG5mc3Y0ZWwubG9nLCBuZnN2NGYxOC5s
b2cNCj4gPiA+ID4gPj4NCj4gPiA+ID4gPj5Cb3RoIGdldCBORlM0RVJSX0VYSVNUIGluIHRoaXMg
Y2FzZS4NCj4gPiA+ID4gPg0KPiA+ID4gPiA+V2hpY2ggaXMgYW4gb2J2aW91cyBzZXJ2ZXIgYnVn
OiBpdCBzaG91bGQgYmUgc2VuZGluZyBORlM0RVJSX1NZTUxJTksgaW4NCj4gPiA+ID4gPnJlcGx5
IHRvIHRoYXQgT1BFTi4NCj4gPiA+ID4gPg0KPiA+ID4gPiA+QnJ1Y2U/DQo+ID4gPiA+ID4NCj4g
PiA+ID4gDQo+ID4gPiA+IEkgY2FuIHJlcHJvZHVjZSB3aXRoIGEgMy40LjAtMC5yYzAuZ2l0MS4y
LmZjMTggc2VydmVyIGFzIHdlbGwuDQo+ID4gPiANCj4gPiA+IEhtLiAgU28gaG93IGFib3V0IHRo
aXM/ICAoVW50ZXN0ZWQuKQ0KPiA+ID4gDQo+ID4gPiBQcm9iYWJseSB0aGVyZSBzaG91bGQgYmUg
YSBweW5mcyB0ZXN0IHRvby4NCj4gPiA+IA0KPiA+ID4gSSdtIGFzc3VtaW5nIGl0IHNob3VsZCBz
dGlsbCBiZSBFUlJfRVhJU1QgaW4gdGhlIGV4Y2x1c2l2ZSwNCj4gPiA+IGV4Y2x1c2l2ZTRfMSwg
YW5kIGd1YXJkZWQgY2FzZXMuDQo+ID4gPiANCj4gPiA+IC0tYi4NCj4gPiA+IA0KPiA+ID4gZGlm
ZiAtLWdpdCBhL2ZzL25mc2QvdmZzLmMgYi9mcy9uZnNkL3Zmcy5jDQo+ID4gPiBpbmRleCA3NDIz
ZDcxLi4yYmZjYWQ0IDEwMDY0NA0KPiA+ID4gLS0tIGEvZnMvbmZzZC92ZnMuYw0KPiA+ID4gKysr
IGIvZnMvbmZzZC92ZnMuYw0KPiA+ID4gQEAgLTE0NTcsOSArMTQ1NywxMiBAQCBkb19uZnNkX2Ny
ZWF0ZShzdHJ1Y3Qgc3ZjX3Jxc3QgKnJxc3RwLCBzdHJ1Y3Qgc3ZjX2ZoICpmaHAsDQo+ID4gPiAg
DQo+ID4gPiAgCQlzd2l0Y2ggKGNyZWF0ZW1vZGUpIHsNCj4gPiA+ICAJCWNhc2UgTkZTM19DUkVB
VEVfVU5DSEVDS0VEOg0KPiA+ID4gLQkJCWlmICghIFNfSVNSRUcoZGNoaWxkLT5kX2lub2RlLT5p
X21vZGUpKQ0KPiA+ID4gLQkJCQllcnIgPSBuZnNlcnJfZXhpc3Q7DQo+ID4gPiAtCQkJZWxzZSBp
ZiAodHJ1bmNwKSB7DQo+ID4gPiArCQkJaWYgKCEgU19JU1JFRyhkY2hpbGQtPmRfaW5vZGUtPmlf
bW9kZSkpIHsNCj4gPiA+ICsJCQkJaWYgKHJxc3RwLT5ycV92ZXJzID09IDQpDQo+ID4gPiArCQkJ
CQllcnIgPSBuZnNlcnJfc3ltbGluazsNCj4gPiA+ICsJCQkJZWxzZQ0KPiA+ID4gKwkJCQkJZXJy
ID0gbmZzZXJyX2V4aXN0Ow0KPiA+IA0KPiA+IE5vLiBUaGlzIHNob3VsZCBfbmV2ZXJfIHJldHVy
biBORlM0RVJSX0VYSVNULg0KPiANCj4gVGhhdCAiZWxzZSIgaXMgdGhlIHYzIENSRUFURSBjYXNl
LiAgSSBkb24ndCBzZWUgdGhlIGFsdGVybmF0aXZlIHRoZXJlPw0KDQpUcmVhdCBpdCBhcyBhIExP
T0tVUC4NCg0KLS0gDQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5l
cg0KDQpOZXRBcHANClRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQp3d3cubmV0YXBwLmNvbQ0K
DQo=

Re: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Dr James Bruce Fields]

On Thu, Mar 29, 2012 at 08:42:24PM +0000, Myklebust, Trond wrote:
> On Thu, 2012-03-29 at 16:16 -0400, Trond Myklebust wrote:
> > On Thu, 2012-03-29 at 15:31 -0400, Dr James Bruce Fields wrote:
> > > On Thu, Mar 29, 2012 at 12:07:17PM -0600, Orion Poplawski wrote:
> > > > On 03/29/2012 11:40 AM, Myklebust, Trond wrote:
> > > > >>Going back to v4 on EL5.8 server: nfsv4el.log, nfsv4f18.log
> > > > >>
> > > > >>Both get NFS4ERR_EXIST in this case.
> > > > >
> > > > >Which is an obvious server bug: it should be sending NFS4ERR_SYMLINK in
> > > > >reply to that OPEN.
> > > > >
> > > > >Bruce?
> > > > >
> > > > 
> > > > I can reproduce with a 3.4.0-0.rc0.git1.2.fc18 server as well.
> > > 
> > > Hm.  So how about this?  (Untested.)
> > > 
> > > Probably there should be a pynfs test too.
> > > 
> > > I'm assuming it should still be ERR_EXIST in the exclusive,
> > > exclusive4_1, and guarded cases.
> > > 
> > > --b.
> > > 
> > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > > index 7423d71..2bfcad4 100644
> > > --- a/fs/nfsd/vfs.c
> > > +++ b/fs/nfsd/vfs.c
> > > @@ -1457,9 +1457,12 @@ do_nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
> > >  
> > >  		switch (createmode) {
> > >  		case NFS3_CREATE_UNCHECKED:
> > > -			if (! S_ISREG(dchild->d_inode->i_mode))
> > > -				err = nfserr_exist;
> > > -			else if (truncp) {
> > > +			if (! S_ISREG(dchild->d_inode->i_mode)) {
> > > +				if (rqstp->rq_vers == 4)
> > > +					err = nfserr_symlink;
> > > +				else
> > > +					err = nfserr_exist;
> > 
> > No. This should _never_ return NFS4ERR_EXIST.
> > 
> > It should return
> >       * NFS4ERR_ISDIR if the object is a directory
> >       * NFS4ERR_SYMLINK if it is symbolic link,
> >       * either NFS4ERR_WRONG_TYPE (NFSv4.1) or NFS4ERR_SYMLINK (NFSv4.0)
> >         if the object is any other non-regular file.
> 
> Basically, if an object already exists with that name, then
> NFS3_CREATE_UNCHECKED should be treated as if it is an ordinary OPEN (in
> the case of NFSv4) or LOOKUP (in the case of NFSv3).

Oh, so in the v3 case this should be nfs_ok, and it's up to the client
to check attributes on the result and decide what to do?

(Is this written down someplace?)

--b.

Re: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Myklebust, Trond]

T24gVGh1LCAyMDEyLTAzLTI5IGF0IDE2OjUwIC0wNDAwLCBEciBKYW1lcyBCcnVjZSBGaWVsZHMg
d3JvdGU6DQo+IE9uIFRodSwgTWFyIDI5LCAyMDEyIGF0IDA4OjQyOjI0UE0gKzAwMDAsIE15a2xl
YnVzdCwgVHJvbmQgd3JvdGU6DQo+ID4gT24gVGh1LCAyMDEyLTAzLTI5IGF0IDE2OjE2IC0wNDAw
LCBUcm9uZCBNeWtsZWJ1c3Qgd3JvdGU6DQo+ID4gPiBPbiBUaHUsIDIwMTItMDMtMjkgYXQgMTU6
MzEgLTA0MDAsIERyIEphbWVzIEJydWNlIEZpZWxkcyB3cm90ZToNCj4gPiA+ID4gT24gVGh1LCBN
YXIgMjksIDIwMTIgYXQgMTI6MDc6MTdQTSAtMDYwMCwgT3Jpb24gUG9wbGF3c2tpIHdyb3RlOg0K
PiA+ID4gPiA+IE9uIDAzLzI5LzIwMTIgMTE6NDAgQU0sIE15a2xlYnVzdCwgVHJvbmQgd3JvdGU6
DQo+ID4gPiA+ID4gPj5Hb2luZyBiYWNrIHRvIHY0IG9uIEVMNS44IHNlcnZlcjogbmZzdjRlbC5s
b2csIG5mc3Y0ZjE4LmxvZw0KPiA+ID4gPiA+ID4+DQo+ID4gPiA+ID4gPj5Cb3RoIGdldCBORlM0
RVJSX0VYSVNUIGluIHRoaXMgY2FzZS4NCj4gPiA+ID4gPiA+DQo+ID4gPiA+ID4gPldoaWNoIGlz
IGFuIG9idmlvdXMgc2VydmVyIGJ1ZzogaXQgc2hvdWxkIGJlIHNlbmRpbmcgTkZTNEVSUl9TWU1M
SU5LIGluDQo+ID4gPiA+ID4gPnJlcGx5IHRvIHRoYXQgT1BFTi4NCj4gPiA+ID4gPiA+DQo+ID4g
PiA+ID4gPkJydWNlPw0KPiA+ID4gPiA+ID4NCj4gPiA+ID4gPiANCj4gPiA+ID4gPiBJIGNhbiBy
ZXByb2R1Y2Ugd2l0aCBhIDMuNC4wLTAucmMwLmdpdDEuMi5mYzE4IHNlcnZlciBhcyB3ZWxsLg0K
PiA+ID4gPiANCj4gPiA+ID4gSG0uICBTbyBob3cgYWJvdXQgdGhpcz8gIChVbnRlc3RlZC4pDQo+
ID4gPiA+IA0KPiA+ID4gPiBQcm9iYWJseSB0aGVyZSBzaG91bGQgYmUgYSBweW5mcyB0ZXN0IHRv
by4NCj4gPiA+ID4gDQo+ID4gPiA+IEknbSBhc3N1bWluZyBpdCBzaG91bGQgc3RpbGwgYmUgRVJS
X0VYSVNUIGluIHRoZSBleGNsdXNpdmUsDQo+ID4gPiA+IGV4Y2x1c2l2ZTRfMSwgYW5kIGd1YXJk
ZWQgY2FzZXMuDQo+ID4gPiA+IA0KPiA+ID4gPiAtLWIuDQo+ID4gPiA+IA0KPiA+ID4gPiBkaWZm
IC0tZ2l0IGEvZnMvbmZzZC92ZnMuYyBiL2ZzL25mc2QvdmZzLmMNCj4gPiA+ID4gaW5kZXggNzQy
M2Q3MS4uMmJmY2FkNCAxMDA2NDQNCj4gPiA+ID4gLS0tIGEvZnMvbmZzZC92ZnMuYw0KPiA+ID4g
PiArKysgYi9mcy9uZnNkL3Zmcy5jDQo+ID4gPiA+IEBAIC0xNDU3LDkgKzE0NTcsMTIgQEAgZG9f
bmZzZF9jcmVhdGUoc3RydWN0IHN2Y19ycXN0ICpycXN0cCwgc3RydWN0IHN2Y19maCAqZmhwLA0K
PiA+ID4gPiAgDQo+ID4gPiA+ICAJCXN3aXRjaCAoY3JlYXRlbW9kZSkgew0KPiA+ID4gPiAgCQlj
YXNlIE5GUzNfQ1JFQVRFX1VOQ0hFQ0tFRDoNCj4gPiA+ID4gLQkJCWlmICghIFNfSVNSRUcoZGNo
aWxkLT5kX2lub2RlLT5pX21vZGUpKQ0KPiA+ID4gPiAtCQkJCWVyciA9IG5mc2Vycl9leGlzdDsN
Cj4gPiA+ID4gLQkJCWVsc2UgaWYgKHRydW5jcCkgew0KPiA+ID4gPiArCQkJaWYgKCEgU19JU1JF
RyhkY2hpbGQtPmRfaW5vZGUtPmlfbW9kZSkpIHsNCj4gPiA+ID4gKwkJCQlpZiAocnFzdHAtPnJx
X3ZlcnMgPT0gNCkNCj4gPiA+ID4gKwkJCQkJZXJyID0gbmZzZXJyX3N5bWxpbms7DQo+ID4gPiA+
ICsJCQkJZWxzZQ0KPiA+ID4gPiArCQkJCQllcnIgPSBuZnNlcnJfZXhpc3Q7DQo+ID4gPiANCj4g
PiA+IE5vLiBUaGlzIHNob3VsZCBfbmV2ZXJfIHJldHVybiBORlM0RVJSX0VYSVNULg0KPiA+ID4g
DQo+ID4gPiBJdCBzaG91bGQgcmV0dXJuDQo+ID4gPiAgICAgICAqIE5GUzRFUlJfSVNESVIgaWYg
dGhlIG9iamVjdCBpcyBhIGRpcmVjdG9yeQ0KPiA+ID4gICAgICAgKiBORlM0RVJSX1NZTUxJTksg
aWYgaXQgaXMgc3ltYm9saWMgbGluaywNCj4gPiA+ICAgICAgICogZWl0aGVyIE5GUzRFUlJfV1JP
TkdfVFlQRSAoTkZTdjQuMSkgb3IgTkZTNEVSUl9TWU1MSU5LIChORlN2NC4wKQ0KPiA+ID4gICAg
ICAgICBpZiB0aGUgb2JqZWN0IGlzIGFueSBvdGhlciBub24tcmVndWxhciBmaWxlLg0KPiA+IA0K
PiA+IEJhc2ljYWxseSwgaWYgYW4gb2JqZWN0IGFscmVhZHkgZXhpc3RzIHdpdGggdGhhdCBuYW1l
LCB0aGVuDQo+ID4gTkZTM19DUkVBVEVfVU5DSEVDS0VEIHNob3VsZCBiZSB0cmVhdGVkIGFzIGlm
IGl0IGlzIGFuIG9yZGluYXJ5IE9QRU4gKGluDQo+ID4gdGhlIGNhc2Ugb2YgTkZTdjQpIG9yIExP
T0tVUCAoaW4gdGhlIGNhc2Ugb2YgTkZTdjMpLg0KPiANCj4gT2gsIHNvIGluIHRoZSB2MyBjYXNl
IHRoaXMgc2hvdWxkIGJlIG5mc19vaywgYW5kIGl0J3MgdXAgdG8gdGhlIGNsaWVudA0KPiB0byBj
aGVjayBhdHRyaWJ1dGVzIG9uIHRoZSByZXN1bHQgYW5kIGRlY2lkZSB3aGF0IHRvIGRvPw0KPiAN
Cj4gKElzIHRoaXMgd3JpdHRlbiBkb3duIHNvbWVwbGFjZT8pDQoNCkluIFJGQzE4MTM6ICJVTkNI
RUNLRUQgbWVhbnMgdGhhdCB0aGUgZmlsZSBzaG91bGQgYmUgY3JlYXRlZCB3aXRob3V0DQpjaGVj
a2luZyBmb3IgdGhlIGV4aXN0ZW5jZSBvZiBhIGR1cGxpY2F0ZSBmaWxlIGluIHRoZSBzYW1lIGRp
cmVjdG9yeS4iDQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50
YWluZXINCg0KTmV0QXBwDQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5j
b20NCg0K

Re: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Dr James Bruce Fields]

On Thu, Mar 29, 2012 at 08:56:57PM +0000, Myklebust, Trond wrote:
> On Thu, 2012-03-29 at 16:50 -0400, Dr James Bruce Fields wrote:
> > On Thu, Mar 29, 2012 at 08:42:24PM +0000, Myklebust, Trond wrote:
> > > On Thu, 2012-03-29 at 16:16 -0400, Trond Myklebust wrote:
> > > > On Thu, 2012-03-29 at 15:31 -0400, Dr James Bruce Fields wrote:
> > > > > On Thu, Mar 29, 2012 at 12:07:17PM -0600, Orion Poplawski wrote:
> > > > > > On 03/29/2012 11:40 AM, Myklebust, Trond wrote:
> > > > > > >>Going back to v4 on EL5.8 server: nfsv4el.log, nfsv4f18.log
> > > > > > >>
> > > > > > >>Both get NFS4ERR_EXIST in this case.
> > > > > > >
> > > > > > >Which is an obvious server bug: it should be sending NFS4ERR_SYMLINK in
> > > > > > >reply to that OPEN.
> > > > > > >
> > > > > > >Bruce?
> > > > > > >
> > > > > > 
> > > > > > I can reproduce with a 3.4.0-0.rc0.git1.2.fc18 server as well.
> > > > > 
> > > > > Hm.  So how about this?  (Untested.)
> > > > > 
> > > > > Probably there should be a pynfs test too.
> > > > > 
> > > > > I'm assuming it should still be ERR_EXIST in the exclusive,
> > > > > exclusive4_1, and guarded cases.
> > > > > 
> > > > > --b.
> > > > > 
> > > > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > > > > index 7423d71..2bfcad4 100644
> > > > > --- a/fs/nfsd/vfs.c
> > > > > +++ b/fs/nfsd/vfs.c
> > > > > @@ -1457,9 +1457,12 @@ do_nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
> > > > >  
> > > > >  		switch (createmode) {
> > > > >  		case NFS3_CREATE_UNCHECKED:
> > > > > -			if (! S_ISREG(dchild->d_inode->i_mode))
> > > > > -				err = nfserr_exist;
> > > > > -			else if (truncp) {
> > > > > +			if (! S_ISREG(dchild->d_inode->i_mode)) {
> > > > > +				if (rqstp->rq_vers == 4)
> > > > > +					err = nfserr_symlink;
> > > > > +				else
> > > > > +					err = nfserr_exist;
> > > > 
> > > > No. This should _never_ return NFS4ERR_EXIST.
> > > > 
> > > > It should return
> > > >       * NFS4ERR_ISDIR if the object is a directory
> > > >       * NFS4ERR_SYMLINK if it is symbolic link,
> > > >       * either NFS4ERR_WRONG_TYPE (NFSv4.1) or NFS4ERR_SYMLINK (NFSv4.0)
> > > >         if the object is any other non-regular file.
> > > 
> > > Basically, if an object already exists with that name, then
> > > NFS3_CREATE_UNCHECKED should be treated as if it is an ordinary OPEN (in
> > > the case of NFSv4) or LOOKUP (in the case of NFSv3).
> > 
> > Oh, so in the v3 case this should be nfs_ok, and it's up to the client
> > to check attributes on the result and decide what to do?
> > 
> > (Is this written down someplace?)
> 
> In RFC1813: "UNCHECKED means that the file should be created without
> checking for the existence of a duplicate file in the same directory."

I don't understand how to apply that sentence to the case of a
preexisting non-regular-file in the same directory.

(Actually, I don't understand it in the case of an existing regular file
either--to me "create without checking for existence" sounds like "even
if a file already exists, you should clobber it and create a new one
anyway"....)

Anyway, something like the following (untested) should change v3 to
return nfs_ok in this case, and v4 to return the same errors it would on
a non-create open.

--b.

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 2ed14df..8256efd 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -235,17 +235,17 @@ do_open_lookup(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_o
 		 */
 		if (open->op_createmode == NFS4_CREATE_EXCLUSIVE && status == 0)
 			open->op_bmval[1] = (FATTR4_WORD1_TIME_ACCESS |
-						FATTR4_WORD1_TIME_MODIFY);
+							FATTR4_WORD1_TIME_MODIFY);
 	} else {
 		status = nfsd_lookup(rqstp, current_fh,
 				     open->op_fname.data, open->op_fname.len, resfh);
 		fh_unlock(current_fh);
-		if (status)
-			goto out;
-		status = nfsd_check_obj_isreg(resfh);
 	}
 	if (status)
 		goto out;
+	status = nfsd_check_obj_isreg(resfh);
+	if (status)
+		goto out;
 
 	if (is_create_with_attrs(open) && open->op_acl != NULL)
 		do_set_nfs4_acl(rqstp, resfh, open->op_acl, open->op_bmval);
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 7423d71..890f439 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1458,7 +1458,7 @@ do_nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		switch (createmode) {
 		case NFS3_CREATE_UNCHECKED:
 			if (! S_ISREG(dchild->d_inode->i_mode))
-				err = nfserr_exist;
+				goto out;
 			else if (truncp) {
 				/* in nfsv4, we need to treat this case a little
 				 * differently.  we don't want to truncate the

Re: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Dr James Bruce Fields]

On Thu, Mar 29, 2012 at 05:08:38PM -0400, Dr James Bruce Fields wrote:
> Anyway, something like the following (untested) should change v3 to
> return nfs_ok in this case, and v4 to return the same errors it would on
> a non-create open.

Looking at the history, I think the v3 behavior has been there from the
start.  I wonder why we've never gotten a bug report?

Looking at wireshark.... I guess the client always does a lookup first,
so we never hit this case (unless someone replaces the file by a
non-regular-file between a lookup and a create?)

--b.

RE: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Peter Staubach]

Hi.

I think that the code to allow CREATE operations which end up opening existing, non-regular files is there for diskless operation.

I vaguely recall some applications specifying O_CREATE to open and expecting the open of a non-regular file, which exists already, to succeed.

		ps


-----Original Message-----
From: linux-nfs-owner@vger.kernel.org [mailto:linux-nfs-owner@vger.kernel.org] On Behalf Of Dr James Bruce Fields
Sent: Thursday, March 29, 2012 5:09 PM
To: Myklebust, Trond
Cc: Orion Poplawski; linux-nfs@vger.kernel.org
Subject: Re: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

On Thu, Mar 29, 2012 at 08:56:57PM +0000, Myklebust, Trond wrote:
> On Thu, 2012-03-29 at 16:50 -0400, Dr James Bruce Fields wrote:
> > On Thu, Mar 29, 2012 at 08:42:24PM +0000, Myklebust, Trond wrote:
> > > On Thu, 2012-03-29 at 16:16 -0400, Trond Myklebust wrote:
> > > > On Thu, 2012-03-29 at 15:31 -0400, Dr James Bruce Fields wrote:
> > > > > On Thu, Mar 29, 2012 at 12:07:17PM -0600, Orion Poplawski wrote:
> > > > > > On 03/29/2012 11:40 AM, Myklebust, Trond wrote:
> > > > > > >>Going back to v4 on EL5.8 server: nfsv4el.log, 
> > > > > > >>nfsv4f18.log
> > > > > > >>
> > > > > > >>Both get NFS4ERR_EXIST in this case.
> > > > > > >
> > > > > > >Which is an obvious server bug: it should be sending 
> > > > > > >NFS4ERR_SYMLINK in reply to that OPEN.
> > > > > > >
> > > > > > >Bruce?
> > > > > > >
> > > > > > 
> > > > > > I can reproduce with a 3.4.0-0.rc0.git1.2.fc18 server as well.
> > > > > 
> > > > > Hm.  So how about this?  (Untested.)
> > > > > 
> > > > > Probably there should be a pynfs test too.
> > > > > 
> > > > > I'm assuming it should still be ERR_EXIST in the exclusive, 
> > > > > exclusive4_1, and guarded cases.
> > > > > 
> > > > > --b.
> > > > > 
> > > > > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 
> > > > > 7423d71..2bfcad4 100644
> > > > > --- a/fs/nfsd/vfs.c
> > > > > +++ b/fs/nfsd/vfs.c
> > > > > @@ -1457,9 +1457,12 @@ do_nfsd_create(struct svc_rqst *rqstp, 
> > > > > struct svc_fh *fhp,
> > > > >  
> > > > >  		switch (createmode) {
> > > > >  		case NFS3_CREATE_UNCHECKED:
> > > > > -			if (! S_ISREG(dchild->d_inode->i_mode))
> > > > > -				err = nfserr_exist;
> > > > > -			else if (truncp) {
> > > > > +			if (! S_ISREG(dchild->d_inode->i_mode)) {
> > > > > +				if (rqstp->rq_vers == 4)
> > > > > +					err = nfserr_symlink;
> > > > > +				else
> > > > > +					err = nfserr_exist;
> > > > 
> > > > No. This should _never_ return NFS4ERR_EXIST.
> > > > 
> > > > It should return
> > > >       * NFS4ERR_ISDIR if the object is a directory
> > > >       * NFS4ERR_SYMLINK if it is symbolic link,
> > > >       * either NFS4ERR_WRONG_TYPE (NFSv4.1) or NFS4ERR_SYMLINK (NFSv4.0)
> > > >         if the object is any other non-regular file.
> > > 
> > > Basically, if an object already exists with that name, then 
> > > NFS3_CREATE_UNCHECKED should be treated as if it is an ordinary 
> > > OPEN (in the case of NFSv4) or LOOKUP (in the case of NFSv3).
> > 
> > Oh, so in the v3 case this should be nfs_ok, and it's up to the 
> > client to check attributes on the result and decide what to do?
> > 
> > (Is this written down someplace?)
> 
> In RFC1813: "UNCHECKED means that the file should be created without 
> checking for the existence of a duplicate file in the same directory."

I don't understand how to apply that sentence to the case of a preexisting non-regular-file in the same directory.

(Actually, I don't understand it in the case of an existing regular file either--to me "create without checking for existence" sounds like "even if a file already exists, you should clobber it and create a new one
anyway"....)

Anyway, something like the following (untested) should change v3 to return nfs_ok in this case, and v4 to return the same errors it would on a non-create open.

--b.

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 2ed14df..8256efd 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -235,17 +235,17 @@ do_open_lookup(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_o
 		 */
 		if (open->op_createmode == NFS4_CREATE_EXCLUSIVE && status == 0)
 			open->op_bmval[1] = (FATTR4_WORD1_TIME_ACCESS |
-						FATTR4_WORD1_TIME_MODIFY);
+							FATTR4_WORD1_TIME_MODIFY);
 	} else {
 		status = nfsd_lookup(rqstp, current_fh,
 				     open->op_fname.data, open->op_fname.len, resfh);
 		fh_unlock(current_fh);
-		if (status)
-			goto out;
-		status = nfsd_check_obj_isreg(resfh);
 	}
 	if (status)
 		goto out;
+	status = nfsd_check_obj_isreg(resfh);
+	if (status)
+		goto out;
 
 	if (is_create_with_attrs(open) && open->op_acl != NULL)
 		do_set_nfs4_acl(rqstp, resfh, open->op_acl, open->op_bmval); diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 7423d71..890f439 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1458,7 +1458,7 @@ do_nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		switch (createmode) {
 		case NFS3_CREATE_UNCHECKED:
 			if (! S_ISREG(dchild->d_inode->i_mode))
-				err = nfserr_exist;
+				goto out;
 			else if (truncp) {
 				/* in nfsv4, we need to treat this case a little
 				 * differently.  we don't want to truncate the
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Myklebust, Trond]

T24gRnJpLCAyMDEyLTAzLTMwIGF0IDEzOjEyIC0wNDAwLCBQZXRlciBTdGF1YmFjaCB3cm90ZToN
Cj4gSGkuDQo+IA0KPiBJIHRoaW5rIHRoYXQgdGhlIGNvZGUgdG8gYWxsb3cgQ1JFQVRFIG9wZXJh
dGlvbnMgd2hpY2ggZW5kIHVwIG9wZW5pbmcgZXhpc3RpbmcsIG5vbi1yZWd1bGFyIGZpbGVzIGlz
IHRoZXJlIGZvciBkaXNrbGVzcyBvcGVyYXRpb24uDQo+IA0KPiBJIHZhZ3VlbHkgcmVjYWxsIHNv
bWUgYXBwbGljYXRpb25zIHNwZWNpZnlpbmcgT19DUkVBVEUgdG8gb3BlbiBhbmQgZXhwZWN0aW5n
IHRoZSBvcGVuIG9mIGEgbm9uLXJlZ3VsYXIgZmlsZSwgd2hpY2ggZXhpc3RzIGFscmVhZHksIHRv
IHN1Y2NlZWQuDQoNClllcywgYW5kIHRoaXMgaXMgd2hhdCBQT1NJWCBleHBlY3RzIHRvby4gVGhl
IGV4Y2VwdGlvbiBpcyB3aGVuIHlvdSBvcGVuDQphIGRpcmVjdG9yeSB3aXRoIE9fQ1JFQVRFOiB0
aGF0IHdpbGwgZW5kIHVwIHJldHVybmluZyBFSVNESVIgdG8gdGhlDQphcHBsaWNhdGlvbi4NCg0K
Q2hlZXJzDQogIFRyb25kDQoNCj4gCQlwcw0KPiANCj4gDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2Fn
ZS0tLS0tDQo+IEZyb206IGxpbnV4LW5mcy1vd25lckB2Z2VyLmtlcm5lbC5vcmcgW21haWx0bzps
aW51eC1uZnMtb3duZXJAdmdlci5rZXJuZWwub3JnXSBPbiBCZWhhbGYgT2YgRHIgSmFtZXMgQnJ1
Y2UgRmllbGRzDQo+IFNlbnQ6IFRodXJzZGF5LCBNYXJjaCAyOSwgMjAxMiA1OjA5IFBNDQo+IFRv
OiBNeWtsZWJ1c3QsIFRyb25kDQo+IENjOiBPcmlvbiBQb3BsYXdza2k7IGxpbnV4LW5mc0B2Z2Vy
Lmtlcm5lbC5vcmcNCj4gU3ViamVjdDogUmU6IFtuZnN2NF0gb3BlbihPX0NSRUFUKSByZXR1cm5z
IEVFWElTVFMgb24gc3ltYm9saWMgbGluayBjcmVhdGVkIG9uIGFub3RoZXIgc3lzdGVtIHVudGls
IHN0YXQoKWVkDQo+IA0KPiBPbiBUaHUsIE1hciAyOSwgMjAxMiBhdCAwODo1Njo1N1BNICswMDAw
LCBNeWtsZWJ1c3QsIFRyb25kIHdyb3RlOg0KPiA+IE9uIFRodSwgMjAxMi0wMy0yOSBhdCAxNjo1
MCAtMDQwMCwgRHIgSmFtZXMgQnJ1Y2UgRmllbGRzIHdyb3RlOg0KPiA+ID4gT24gVGh1LCBNYXIg
MjksIDIwMTIgYXQgMDg6NDI6MjRQTSArMDAwMCwgTXlrbGVidXN0LCBUcm9uZCB3cm90ZToNCj4g
PiA+ID4gT24gVGh1LCAyMDEyLTAzLTI5IGF0IDE2OjE2IC0wNDAwLCBUcm9uZCBNeWtsZWJ1c3Qg
d3JvdGU6DQo+ID4gPiA+ID4gT24gVGh1LCAyMDEyLTAzLTI5IGF0IDE1OjMxIC0wNDAwLCBEciBK
YW1lcyBCcnVjZSBGaWVsZHMgd3JvdGU6DQo+ID4gPiA+ID4gPiBPbiBUaHUsIE1hciAyOSwgMjAx
MiBhdCAxMjowNzoxN1BNIC0wNjAwLCBPcmlvbiBQb3BsYXdza2kgd3JvdGU6DQo+ID4gPiA+ID4g
PiA+IE9uIDAzLzI5LzIwMTIgMTE6NDAgQU0sIE15a2xlYnVzdCwgVHJvbmQgd3JvdGU6DQo+ID4g
PiA+ID4gPiA+ID4+R29pbmcgYmFjayB0byB2NCBvbiBFTDUuOCBzZXJ2ZXI6IG5mc3Y0ZWwubG9n
LCANCj4gPiA+ID4gPiA+ID4gPj5uZnN2NGYxOC5sb2cNCj4gPiA+ID4gPiA+ID4gPj4NCj4gPiA+
ID4gPiA+ID4gPj5Cb3RoIGdldCBORlM0RVJSX0VYSVNUIGluIHRoaXMgY2FzZS4NCj4gPiA+ID4g
PiA+ID4gPg0KPiA+ID4gPiA+ID4gPiA+V2hpY2ggaXMgYW4gb2J2aW91cyBzZXJ2ZXIgYnVnOiBp
dCBzaG91bGQgYmUgc2VuZGluZyANCj4gPiA+ID4gPiA+ID4gPk5GUzRFUlJfU1lNTElOSyBpbiBy
ZXBseSB0byB0aGF0IE9QRU4uDQo+ID4gPiA+ID4gPiA+ID4NCj4gPiA+ID4gPiA+ID4gPkJydWNl
Pw0KPiA+ID4gPiA+ID4gPiA+DQo+ID4gPiA+ID4gPiA+IA0KPiA+ID4gPiA+ID4gPiBJIGNhbiBy
ZXByb2R1Y2Ugd2l0aCBhIDMuNC4wLTAucmMwLmdpdDEuMi5mYzE4IHNlcnZlciBhcyB3ZWxsLg0K
PiA+ID4gPiA+ID4gDQo+ID4gPiA+ID4gPiBIbS4gIFNvIGhvdyBhYm91dCB0aGlzPyAgKFVudGVz
dGVkLikNCj4gPiA+ID4gPiA+IA0KPiA+ID4gPiA+ID4gUHJvYmFibHkgdGhlcmUgc2hvdWxkIGJl
IGEgcHluZnMgdGVzdCB0b28uDQo+ID4gPiA+ID4gPiANCj4gPiA+ID4gPiA+IEknbSBhc3N1bWlu
ZyBpdCBzaG91bGQgc3RpbGwgYmUgRVJSX0VYSVNUIGluIHRoZSBleGNsdXNpdmUsIA0KPiA+ID4g
PiA+ID4gZXhjbHVzaXZlNF8xLCBhbmQgZ3VhcmRlZCBjYXNlcy4NCj4gPiA+ID4gPiA+IA0KPiA+
ID4gPiA+ID4gLS1iLg0KPiA+ID4gPiA+ID4gDQo+ID4gPiA+ID4gPiBkaWZmIC0tZ2l0IGEvZnMv
bmZzZC92ZnMuYyBiL2ZzL25mc2QvdmZzLmMgaW5kZXggDQo+ID4gPiA+ID4gPiA3NDIzZDcxLi4y
YmZjYWQ0IDEwMDY0NA0KPiA+ID4gPiA+ID4gLS0tIGEvZnMvbmZzZC92ZnMuYw0KPiA+ID4gPiA+
ID4gKysrIGIvZnMvbmZzZC92ZnMuYw0KPiA+ID4gPiA+ID4gQEAgLTE0NTcsOSArMTQ1NywxMiBA
QCBkb19uZnNkX2NyZWF0ZShzdHJ1Y3Qgc3ZjX3Jxc3QgKnJxc3RwLCANCj4gPiA+ID4gPiA+IHN0
cnVjdCBzdmNfZmggKmZocCwNCj4gPiA+ID4gPiA+ICANCj4gPiA+ID4gPiA+ICAJCXN3aXRjaCAo
Y3JlYXRlbW9kZSkgew0KPiA+ID4gPiA+ID4gIAkJY2FzZSBORlMzX0NSRUFURV9VTkNIRUNLRUQ6
DQo+ID4gPiA+ID4gPiAtCQkJaWYgKCEgU19JU1JFRyhkY2hpbGQtPmRfaW5vZGUtPmlfbW9kZSkp
DQo+ID4gPiA+ID4gPiAtCQkJCWVyciA9IG5mc2Vycl9leGlzdDsNCj4gPiA+ID4gPiA+IC0JCQll
bHNlIGlmICh0cnVuY3ApIHsNCj4gPiA+ID4gPiA+ICsJCQlpZiAoISBTX0lTUkVHKGRjaGlsZC0+
ZF9pbm9kZS0+aV9tb2RlKSkgew0KPiA+ID4gPiA+ID4gKwkJCQlpZiAocnFzdHAtPnJxX3ZlcnMg
PT0gNCkNCj4gPiA+ID4gPiA+ICsJCQkJCWVyciA9IG5mc2Vycl9zeW1saW5rOw0KPiA+ID4gPiA+
ID4gKwkJCQllbHNlDQo+ID4gPiA+ID4gPiArCQkJCQllcnIgPSBuZnNlcnJfZXhpc3Q7DQo+ID4g
PiA+ID4gDQo+ID4gPiA+ID4gTm8uIFRoaXMgc2hvdWxkIF9uZXZlcl8gcmV0dXJuIE5GUzRFUlJf
RVhJU1QuDQo+ID4gPiA+ID4gDQo+ID4gPiA+ID4gSXQgc2hvdWxkIHJldHVybg0KPiA+ID4gPiA+
ICAgICAgICogTkZTNEVSUl9JU0RJUiBpZiB0aGUgb2JqZWN0IGlzIGEgZGlyZWN0b3J5DQo+ID4g
PiA+ID4gICAgICAgKiBORlM0RVJSX1NZTUxJTksgaWYgaXQgaXMgc3ltYm9saWMgbGluaywNCj4g
PiA+ID4gPiAgICAgICAqIGVpdGhlciBORlM0RVJSX1dST05HX1RZUEUgKE5GU3Y0LjEpIG9yIE5G
UzRFUlJfU1lNTElOSyAoTkZTdjQuMCkNCj4gPiA+ID4gPiAgICAgICAgIGlmIHRoZSBvYmplY3Qg
aXMgYW55IG90aGVyIG5vbi1yZWd1bGFyIGZpbGUuDQo+ID4gPiA+IA0KPiA+ID4gPiBCYXNpY2Fs
bHksIGlmIGFuIG9iamVjdCBhbHJlYWR5IGV4aXN0cyB3aXRoIHRoYXQgbmFtZSwgdGhlbiANCj4g
PiA+ID4gTkZTM19DUkVBVEVfVU5DSEVDS0VEIHNob3VsZCBiZSB0cmVhdGVkIGFzIGlmIGl0IGlz
IGFuIG9yZGluYXJ5IA0KPiA+ID4gPiBPUEVOIChpbiB0aGUgY2FzZSBvZiBORlN2NCkgb3IgTE9P
S1VQIChpbiB0aGUgY2FzZSBvZiBORlN2MykuDQo+ID4gPiANCj4gPiA+IE9oLCBzbyBpbiB0aGUg
djMgY2FzZSB0aGlzIHNob3VsZCBiZSBuZnNfb2ssIGFuZCBpdCdzIHVwIHRvIHRoZSANCj4gPiA+
IGNsaWVudCB0byBjaGVjayBhdHRyaWJ1dGVzIG9uIHRoZSByZXN1bHQgYW5kIGRlY2lkZSB3aGF0
IHRvIGRvPw0KPiA+ID4gDQo+ID4gPiAoSXMgdGhpcyB3cml0dGVuIGRvd24gc29tZXBsYWNlPykN
Cj4gPiANCj4gPiBJbiBSRkMxODEzOiAiVU5DSEVDS0VEIG1lYW5zIHRoYXQgdGhlIGZpbGUgc2hv
dWxkIGJlIGNyZWF0ZWQgd2l0aG91dCANCj4gPiBjaGVja2luZyBmb3IgdGhlIGV4aXN0ZW5jZSBv
ZiBhIGR1cGxpY2F0ZSBmaWxlIGluIHRoZSBzYW1lIGRpcmVjdG9yeS4iDQo+IA0KPiBJIGRvbid0
IHVuZGVyc3RhbmQgaG93IHRvIGFwcGx5IHRoYXQgc2VudGVuY2UgdG8gdGhlIGNhc2Ugb2YgYSBw
cmVleGlzdGluZyBub24tcmVndWxhci1maWxlIGluIHRoZSBzYW1lIGRpcmVjdG9yeS4NCj4gDQo+
IChBY3R1YWxseSwgSSBkb24ndCB1bmRlcnN0YW5kIGl0IGluIHRoZSBjYXNlIG9mIGFuIGV4aXN0
aW5nIHJlZ3VsYXIgZmlsZSBlaXRoZXItLXRvIG1lICJjcmVhdGUgd2l0aG91dCBjaGVja2luZyBm
b3IgZXhpc3RlbmNlIiBzb3VuZHMgbGlrZSAiZXZlbiBpZiBhIGZpbGUgYWxyZWFkeSBleGlzdHMs
IHlvdSBzaG91bGQgY2xvYmJlciBpdCBhbmQgY3JlYXRlIGEgbmV3IG9uZQ0KPiBhbnl3YXkiLi4u
LikNCj4gDQo+IEFueXdheSwgc29tZXRoaW5nIGxpa2UgdGhlIGZvbGxvd2luZyAodW50ZXN0ZWQp
IHNob3VsZCBjaGFuZ2UgdjMgdG8gcmV0dXJuIG5mc19vayBpbiB0aGlzIGNhc2UsIGFuZCB2NCB0
byByZXR1cm4gdGhlIHNhbWUgZXJyb3JzIGl0IHdvdWxkIG9uIGEgbm9uLWNyZWF0ZSBvcGVuLg0K
PiANCj4gLS1iLg0KPiANCj4gZGlmZiAtLWdpdCBhL2ZzL25mc2QvbmZzNHByb2MuYyBiL2ZzL25m
c2QvbmZzNHByb2MuYyBpbmRleCAyZWQxNGRmLi44MjU2ZWZkIDEwMDY0NA0KPiAtLS0gYS9mcy9u
ZnNkL25mczRwcm9jLmMNCj4gKysrIGIvZnMvbmZzZC9uZnM0cHJvYy5jDQo+IEBAIC0yMzUsMTcg
KzIzNSwxNyBAQCBkb19vcGVuX2xvb2t1cChzdHJ1Y3Qgc3ZjX3Jxc3QgKnJxc3RwLCBzdHJ1Y3Qg
c3ZjX2ZoICpjdXJyZW50X2ZoLCBzdHJ1Y3QgbmZzZDRfbw0KPiAgCQkgKi8NCj4gIAkJaWYgKG9w
ZW4tPm9wX2NyZWF0ZW1vZGUgPT0gTkZTNF9DUkVBVEVfRVhDTFVTSVZFICYmIHN0YXR1cyA9PSAw
KQ0KPiAgCQkJb3Blbi0+b3BfYm12YWxbMV0gPSAoRkFUVFI0X1dPUkQxX1RJTUVfQUNDRVNTIHwN
Cj4gLQkJCQkJCUZBVFRSNF9XT1JEMV9USU1FX01PRElGWSk7DQo+ICsJCQkJCQkJRkFUVFI0X1dP
UkQxX1RJTUVfTU9ESUZZKTsNCj4gIAl9IGVsc2Ugew0KPiAgCQlzdGF0dXMgPSBuZnNkX2xvb2t1
cChycXN0cCwgY3VycmVudF9maCwNCj4gIAkJCQkgICAgIG9wZW4tPm9wX2ZuYW1lLmRhdGEsIG9w
ZW4tPm9wX2ZuYW1lLmxlbiwgcmVzZmgpOw0KPiAgCQlmaF91bmxvY2soY3VycmVudF9maCk7DQo+
IC0JCWlmIChzdGF0dXMpDQo+IC0JCQlnb3RvIG91dDsNCj4gLQkJc3RhdHVzID0gbmZzZF9jaGVj
a19vYmpfaXNyZWcocmVzZmgpOw0KPiAgCX0NCj4gIAlpZiAoc3RhdHVzKQ0KPiAgCQlnb3RvIG91
dDsNCj4gKwlzdGF0dXMgPSBuZnNkX2NoZWNrX29ial9pc3JlZyhyZXNmaCk7DQo+ICsJaWYgKHN0
YXR1cykNCj4gKwkJZ290byBvdXQ7DQo+ICANCj4gIAlpZiAoaXNfY3JlYXRlX3dpdGhfYXR0cnMo
b3BlbikgJiYgb3Blbi0+b3BfYWNsICE9IE5VTEwpDQo+ICAJCWRvX3NldF9uZnM0X2FjbChycXN0
cCwgcmVzZmgsIG9wZW4tPm9wX2FjbCwgb3Blbi0+b3BfYm12YWwpOyBkaWZmIC0tZ2l0IGEvZnMv
bmZzZC92ZnMuYyBiL2ZzL25mc2QvdmZzLmMgaW5kZXggNzQyM2Q3MS4uODkwZjQzOSAxMDA2NDQN
Cj4gLS0tIGEvZnMvbmZzZC92ZnMuYw0KPiArKysgYi9mcy9uZnNkL3Zmcy5jDQo+IEBAIC0xNDU4
LDcgKzE0NTgsNyBAQCBkb19uZnNkX2NyZWF0ZShzdHJ1Y3Qgc3ZjX3Jxc3QgKnJxc3RwLCBzdHJ1
Y3Qgc3ZjX2ZoICpmaHAsDQo+ICAJCXN3aXRjaCAoY3JlYXRlbW9kZSkgew0KPiAgCQljYXNlIE5G
UzNfQ1JFQVRFX1VOQ0hFQ0tFRDoNCj4gIAkJCWlmICghIFNfSVNSRUcoZGNoaWxkLT5kX2lub2Rl
LT5pX21vZGUpKQ0KPiAtCQkJCWVyciA9IG5mc2Vycl9leGlzdDsNCj4gKwkJCQlnb3RvIG91dDsN
Cj4gIAkJCWVsc2UgaWYgKHRydW5jcCkgew0KPiAgCQkJCS8qIGluIG5mc3Y0LCB3ZSBuZWVkIHRv
IHRyZWF0IHRoaXMgY2FzZSBhIGxpdHRsZQ0KPiAgCQkJCSAqIGRpZmZlcmVudGx5LiAgd2UgZG9u
J3Qgd2FudCB0byB0cnVuY2F0ZSB0aGUNCj4gLS0NCj4gVG8gdW5zdWJzY3JpYmUgZnJvbSB0aGlz
IGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJlIGxpbnV4LW5mcyIgaW4gdGhlIGJvZHkg
b2YgYSBtZXNzYWdlIHRvIG1ham9yZG9tb0B2Z2VyLmtlcm5lbC5vcmcgTW9yZSBtYWpvcmRvbW8g
aW5mbyBhdCAgaHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sDQoNCi0t
IA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMgY2xpZW50IG1haW50YWluZXINCg0KTmV0QXBw
DQpUcm9uZC5NeWtsZWJ1c3RAbmV0YXBwLmNvbQ0Kd3d3Lm5ldGFwcC5jb20NCg0K

Re: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Orion Poplawski]

On 03/29/2012 03:17 PM, Dr James Bruce Fields wrote:
> On Thu, Mar 29, 2012 at 05:08:38PM -0400, Dr James Bruce Fields wrote:
>> Anyway, something like the following (untested) should change v3 to
>> return nfs_ok in this case, and v4 to return the same errors it would on
>> a non-create open.
>
> Looking at the history, I think the v3 behavior has been there from the
> start.  I wonder why we've never gotten a bug report?
>
> Looking at wireshark.... I guess the client always does a lookup first,
> so we never hit this case (unless someone replaces the file by a
> non-regular-file between a lookup and a create?)
>
> --b.

So, is this all set to eventually make it into the mainline kernel?  Or is 
there still something I can do to help move it along?

Thanks everyone,

   Orion

-- 
Orion Poplawski
Technical Manager                     303-415-9701 x222
NWRA, Boulder Office                  FAX: 303-415-9702
3380 Mitchell Lane                       orion@nwra.com
Boulder, CO 80301                   http://www.nwra.com

Re: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Bruce Fields]

On Thu, Apr 05, 2012 at 10:35:40AM -0600, Orion Poplawski wrote:
> On 03/29/2012 03:17 PM, Dr James Bruce Fields wrote:
> >On Thu, Mar 29, 2012 at 05:08:38PM -0400, Dr James Bruce Fields wrote:
> >>Anyway, something like the following (untested) should change v3 to
> >>return nfs_ok in this case, and v4 to return the same errors it would on
> >>a non-create open.
> >
> >Looking at the history, I think the v3 behavior has been there from the
> >start.  I wonder why we've never gotten a bug report?
> >
> >Looking at wireshark.... I guess the client always does a lookup first,
> >so we never hit this case (unless someone replaces the file by a
> >non-regular-file between a lookup and a create?)
> >
> >--b.
> 
> So, is this all set to eventually make it into the mainline kernel?
> Or is there still something I can do to help move it along?

If you could confirm whether the patch in 

	http://www.spinics.net/lists/linux-nfs/msg28840.html

fixes your problem, that would help.

--b.

Re: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Orion Poplawski]

On 04/05/2012 10:53 AM, Bruce Fields wrote:
> On Thu, Apr 05, 2012 at 10:35:40AM -0600, Orion Poplawski wrote:
>> On 03/29/2012 03:17 PM, Dr James Bruce Fields wrote:
>>> On Thu, Mar 29, 2012 at 05:08:38PM -0400, Dr James Bruce Fields wrote:
>>>> Anyway, something like the following (untested) should change v3 to
>>>> return nfs_ok in this case, and v4 to return the same errors it would on
>>>> a non-create open.
>>>
>>> Looking at the history, I think the v3 behavior has been there from the
>>> start.  I wonder why we've never gotten a bug report?
>>>
>>> Looking at wireshark.... I guess the client always does a lookup first,
>>> so we never hit this case (unless someone replaces the file by a
>>> non-regular-file between a lookup and a create?)
>>>
>>> --b.
>>
>> So, is this all set to eventually make it into the mainline kernel?
>> Or is there still something I can do to help move it along?
>
> If you could confirm whether the patch in
>
> 	http://www.spinics.net/lists/linux-nfs/msg28840.html
>
> fixes your problem, that would help.
>
> --b.

I applied it to 2.6.32-220.7.1.el6.x86_64 and it appears to have fixed the 
issue.  Can't be sure yet if it broke anything else though...


-- 
Orion Poplawski
Technical Manager                     303-415-9701 x222
NWRA, Boulder Office                  FAX: 303-415-9702
3380 Mitchell Lane                       orion@nwra.com
Boulder, CO 80301                   http://www.nwra.com

Re: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Bruce Fields]

On Thu, Apr 05, 2012 at 02:17:27PM -0600, Orion Poplawski wrote:
> On 04/05/2012 10:53 AM, Bruce Fields wrote:
> >On Thu, Apr 05, 2012 at 10:35:40AM -0600, Orion Poplawski wrote:
> >>On 03/29/2012 03:17 PM, Dr James Bruce Fields wrote:
> >>>On Thu, Mar 29, 2012 at 05:08:38PM -0400, Dr James Bruce Fields wrote:
> >>>>Anyway, something like the following (untested) should change v3 to
> >>>>return nfs_ok in this case, and v4 to return the same errors it would on
> >>>>a non-create open.
> >>>
> >>>Looking at the history, I think the v3 behavior has been there from the
> >>>start.  I wonder why we've never gotten a bug report?
> >>>
> >>>Looking at wireshark.... I guess the client always does a lookup first,
> >>>so we never hit this case (unless someone replaces the file by a
> >>>non-regular-file between a lookup and a create?)
> >>>
> >>>--b.
> >>
> >>So, is this all set to eventually make it into the mainline kernel?
> >>Or is there still something I can do to help move it along?
> >
> >If you could confirm whether the patch in
> >
> >	http://www.spinics.net/lists/linux-nfs/msg28840.html
> >
> >fixes your problem, that would help.
> >
> >--b.
> 
> I applied it to 2.6.32-220.7.1.el6.x86_64 and it appears to have
> fixed the issue.  Can't be sure yet if it broke anything else
> though...

Great, thanks.  I'm applying as follows.  I want to run in through some
basic regression tests and then I'll see about getting it upstream.
(Though it's a bug we've apparently lived with since the beginning of
time, so it may need to wait for the next merge window.)

--b.

Re: [nfsv4] open(O_CREAT) returns EEXISTS on symbolic link created on another system until stat()ed

[Bruce Fields]

On Mon, Apr 09, 2012 at 06:32:21PM -0400, Bruce Fields wrote:
> On Thu, Apr 05, 2012 at 02:17:27PM -0600, Orion Poplawski wrote:
> > On 04/05/2012 10:53 AM, Bruce Fields wrote:
> > >On Thu, Apr 05, 2012 at 10:35:40AM -0600, Orion Poplawski wrote:
> > >>On 03/29/2012 03:17 PM, Dr James Bruce Fields wrote:
> > >>>On Thu, Mar 29, 2012 at 05:08:38PM -0400, Dr James Bruce Fields wrote:
> > >>>>Anyway, something like the following (untested) should change v3 to
> > >>>>return nfs_ok in this case, and v4 to return the same errors it would on
> > >>>>a non-create open.
> > >>>
> > >>>Looking at the history, I think the v3 behavior has been there from the
> > >>>start.  I wonder why we've never gotten a bug report?
> > >>>
> > >>>Looking at wireshark.... I guess the client always does a lookup first,
> > >>>so we never hit this case (unless someone replaces the file by a
> > >>>non-regular-file between a lookup and a create?)
> > >>>
> > >>>--b.
> > >>
> > >>So, is this all set to eventually make it into the mainline kernel?
> > >>Or is there still something I can do to help move it along?
> > >
> > >If you could confirm whether the patch in
> > >
> > >	http://www.spinics.net/lists/linux-nfs/msg28840.html
> > >
> > >fixes your problem, that would help.
> > >
> > >--b.
> > 
> > I applied it to 2.6.32-220.7.1.el6.x86_64 and it appears to have
> > fixed the issue.  Can't be sure yet if it broke anything else
> > though...
> 
> Great, thanks.  I'm applying as follows.

Whoops, forgot to append the patch.--b.

commit 02f661e5012665a3994e454a3f2602843ef65d59
Author: J. Bruce Fields <bfields@redhat.com>
Date:   Mon Apr 9 18:06:49 2012 -0400

    nfsd: don't fail unchecked creates of non-special files
    
    Allow a v3 unchecked open of a non-regular file succeed as if it were a
    lookup; typically a client in such a case will want to fall back on a
    local open, so succeeding and giving it the filehandle is more useful
    than failing with nfserr_exist, which makes it appear that nothing at
    all exists by that name.
    
    Similarly for v4, on an open-create, return the same errors we would on
    an attempt to open a non-regular file, instead of returning
    nfserr_exist.
    
    This fixes a problem found doing a v4 open of a symlink with
    O_RDONLY|O_CREAT, which resulted in the current client returning EEXIST.
    
    Thanks also to Trond for analysis.
    
    Cc: stable@kernel.org
    Reported-by: Orion Poplawski <orion@cora.nwra.com>
    Tested-by: Orion Poplawski <orion@cora.nwra.com>
    Signed-off-by: J. Bruce Fields <bfields@redhat.com>

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 2ed14df..8256efd 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -235,17 +235,17 @@ do_open_lookup(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_o
 		 */
 		if (open->op_createmode == NFS4_CREATE_EXCLUSIVE && status == 0)
 			open->op_bmval[1] = (FATTR4_WORD1_TIME_ACCESS |
-						FATTR4_WORD1_TIME_MODIFY);
+							FATTR4_WORD1_TIME_MODIFY);
 	} else {
 		status = nfsd_lookup(rqstp, current_fh,
 				     open->op_fname.data, open->op_fname.len, resfh);
 		fh_unlock(current_fh);
-		if (status)
-			goto out;
-		status = nfsd_check_obj_isreg(resfh);
 	}
 	if (status)
 		goto out;
+	status = nfsd_check_obj_isreg(resfh);
+	if (status)
+		goto out;
 
 	if (is_create_with_attrs(open) && open->op_acl != NULL)
 		do_set_nfs4_acl(rqstp, resfh, open->op_acl, open->op_bmval);
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 296d671..5686661 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -1458,7 +1458,7 @@ do_nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		switch (createmode) {
 		case NFS3_CREATE_UNCHECKED:
 			if (! S_ISREG(dchild->d_inode->i_mode))
-				err = nfserr_exist;
+				goto out;
 			else if (truncp) {
 				/* in nfsv4, we need to treat this case a little
 				 * differently.  we don't want to truncate the