[linux-cifs-client] cifs: set SYNCHRONIZE bit when opening for cifs_rename_pending_delete
diff mbox

Message ID 1235843943-22563-1-git-send-email-jlayton@redhat.com
State New, archived
Headers show

Commit Message

Jeff Layton Feb. 28, 2009, 5:59 p.m. UTC
From: Jeff Layton <jlayton@tupile.poochiereds.net>

Discovered at Connectathon 2009...

Some servers apparently require that the filehandle passed to a trans2
rename be opened with the SYNCHRONIZE bit set. If it isn't then the
server may throw back an error.

This patch makes the cifs-capable connectathon tests pass when run
against BlueArc servers. I've also heard rumors that Win2k requires
that the file be opened with this bit set as well, but I haven't
confirmed it.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/cifs/inode.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

Comments

Jeff Layton March 3, 2009, 7:37 p.m. UTC | #1
On Sat, 28 Feb 2009 12:59:02 -0500
Jeff Layton <jlayton@redhat.com> wrote:

> From: Jeff Layton <jlayton@tupile.poochiereds.net>
> 
> Discovered at Connectathon 2009...
> 
> Some servers apparently require that the filehandle passed to a trans2
> rename be opened with the SYNCHRONIZE bit set. If it isn't then the
> server may throw back an error.
> 
> This patch makes the cifs-capable connectathon tests pass when run
> against BlueArc servers. I've also heard rumors that Win2k requires
> that the file be opened with this bit set as well, but I haven't
> confirmed it.
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>

After doing a bit more research on this, I think this patch is not
correct. The SYNCHRONIZE bit should never be sent on over the wire
calls. If BlueArc's servers require this then it's bug on their end.
Please disregard this patch.


> ---
>  fs/cifs/inode.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 050552c..a80d86d 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -872,8 +872,9 @@ cifs_rename_pending_delete(char *full_path, struct dentry *dentry, int xid)
>  	FILE_BASIC_INFO *info_buf = NULL;
>  
>  	rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN,
> -			 DELETE|FILE_WRITE_ATTRIBUTES, CREATE_NOT_DIR,
> -			 &netfid, &oplock, NULL, cifs_sb->local_nls,
> +			 DELETE|FILE_WRITE_ATTRIBUTES|SYNCHRONIZE,
> +			 CREATE_NOT_DIR, &netfid, &oplock, NULL,
> +			 cifs_sb->local_nls,
>  			 cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
>  	if (rc != 0)
>  		goto out;
> @@ -1349,7 +1350,7 @@ cifs_do_rename(int xid, struct dentry *from_dentry, const char *fromPath,
>  		return rc;
>  
>  	/* open the file to be renamed -- we need DELETE perms */
> -	rc = CIFSSMBOpen(xid, pTcon, fromPath, FILE_OPEN, DELETE,
> +	rc = CIFSSMBOpen(xid, pTcon, fromPath, FILE_OPEN, DELETE|SYNCHRONIZE,
>  			 CREATE_NOT_DIR, &srcfid, &oplock, NULL,
>  			 cifs_sb->local_nls, cifs_sb->mnt_cifs_flags &
>  				CIFS_MOUNT_MAP_SPECIAL_CHR);
Steve French March 3, 2009, 8:30 p.m. UTC | #2
On Tue, Mar 3, 2009 at 1:37 PM, Jeff Layton <jlayton@redhat.com> wrote:
> On Sat, 28 Feb 2009 12:59:02 -0500
> Jeff Layton <jlayton@redhat.com> wrote:
>
>> From: Jeff Layton <jlayton@tupile.poochiereds.net>
>>
>> Discovered at Connectathon 2009...
>>
>> Some servers apparently require that the filehandle passed to a trans2
>> rename be opened with the SYNCHRONIZE bit set. If it isn't then the
>> server may throw back an error.
>>
>> This patch makes the cifs-capable connectathon tests pass when run
>> against BlueArc servers. I've also heard rumors that Win2k requires
>> that the file be opened with this bit set as well, but I haven't
>> confirmed it.

I agree
I have confirmed that there are a couple of cases when Windows clients can
send this bit, but the bit is supposed to be cleared, and none of
those operations
required this bit be set (and in fact their documentation forbids it to be set).

Patch
diff mbox

diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 050552c..a80d86d 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -872,8 +872,9 @@  cifs_rename_pending_delete(char *full_path, struct dentry *dentry, int xid)
 	FILE_BASIC_INFO *info_buf = NULL;
 
 	rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN,
-			 DELETE|FILE_WRITE_ATTRIBUTES, CREATE_NOT_DIR,
-			 &netfid, &oplock, NULL, cifs_sb->local_nls,
+			 DELETE|FILE_WRITE_ATTRIBUTES|SYNCHRONIZE,
+			 CREATE_NOT_DIR, &netfid, &oplock, NULL,
+			 cifs_sb->local_nls,
 			 cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
 	if (rc != 0)
 		goto out;
@@ -1349,7 +1350,7 @@  cifs_do_rename(int xid, struct dentry *from_dentry, const char *fromPath,
 		return rc;
 
 	/* open the file to be renamed -- we need DELETE perms */
-	rc = CIFSSMBOpen(xid, pTcon, fromPath, FILE_OPEN, DELETE,
+	rc = CIFSSMBOpen(xid, pTcon, fromPath, FILE_OPEN, DELETE|SYNCHRONIZE,
 			 CREATE_NOT_DIR, &srcfid, &oplock, NULL,
 			 cifs_sb->local_nls, cifs_sb->mnt_cifs_flags &
 				CIFS_MOUNT_MAP_SPECIAL_CHR);