* [PATCH 0/4] cifs: clean up cifs_unlink and fix silly-renames (try #2)
@ 2008-09-16 18:05 Jeff Layton
2008-09-16 18:05 ` [PATCH 1/4] cifs: clean up variables in cifs_unlink Jeff Layton
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Jeff Layton @ 2008-09-16 18:05 UTC (permalink / raw)
To: smfrench; +Cc: linux-fsdevel, linux-cifs-client, smfrench
This is try #2 of the patchset to clean up cifs_unlink and fix up the
silly-rename scheme. The main difference between this one and the
original patchset is the renaming of some variables in cifs_unlink to
more closely match generic VFS code and kernel coding style. It also
fixes a bug -- misspelled flag name in the open flags in the third
patch.
This patchset is a cleanup of the cifs_unlink() code. It moves most of
the "silly-rename" logic into a helper function and has cifs_unlink call
it. It also fixes the silly-rename logic, which didn't work quite right
in some cases and also adds a new function to properly set the
DELETE_ON_CLOSE bit on files that already exist. With this patchset,
CIFS VFS can now pass the op_chmod test in the Connectathon test suite
when tested against a win2k3 server.
The patchset is a little larger than the resulting code. I attempted to
preserve bisectability with it, so I suggest applying them in order. If
everything looks OK, this patchset is probably appropriate for 2.6.28.
Jeff Layton (4):
cifs: clean up variables in cifs_unlink
cifs: use common code for turning off ATTR_READONLY in cifs_unlink
cifs: move silly-rename logic into helper function
cifs: add function to set file disposition
fs/cifs/cifsfs.h | 2 +-
fs/cifs/cifsproto.h | 2 +
fs/cifs/cifssmb.c | 55 ++++++++++++
fs/cifs/inode.c | 235 ++++++++++++++++++++++++--------------------------
4 files changed, 171 insertions(+), 123 deletions(-)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/4] cifs: clean up variables in cifs_unlink
2008-09-16 18:05 [PATCH 0/4] cifs: clean up cifs_unlink and fix silly-renames (try #2) Jeff Layton
@ 2008-09-16 18:05 ` Jeff Layton
2008-09-16 18:05 ` [PATCH 2/4] cifs: use common code for turning off ATTR_READONLY " Jeff Layton
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Jeff Layton @ 2008-09-16 18:05 UTC (permalink / raw)
To: smfrench; +Cc: linux-fsdevel, linux-cifs-client, smfrench
Change parameters to cifs_unlink to match the ones used in the generic
VFS. Add some local variables to cut down on the amount of struct
dereferencing that needs to be done, and eliminate some unneeded NULL
pointer checks on the parent directory inode. Finally, rename pTcon
to "tcon" to more closely match standard kernel coding style.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/cifs/cifsfs.h | 2 +-
fs/cifs/inode.c | 90 ++++++++++++++++++++++++-----------------------------
2 files changed, 42 insertions(+), 50 deletions(-)
diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index 135c965..f7b4a5c 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -41,7 +41,7 @@ extern int cifs_create(struct inode *, struct dentry *, int,
struct nameidata *);
extern struct dentry *cifs_lookup(struct inode *, struct dentry *,
struct nameidata *);
-extern int cifs_unlink(struct inode *, struct dentry *);
+extern int cifs_unlink(struct inode *dir, struct dentry *dentry);
extern int cifs_hardlink(struct dentry *, struct inode *, struct dentry *);
extern int cifs_mknod(struct inode *, struct dentry *, int, dev_t);
extern int cifs_mkdir(struct inode *, struct dentry *, int);
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index ed2b5df..2f75714 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -665,40 +665,34 @@ struct inode *cifs_iget(struct super_block *sb, unsigned long ino)
return inode;
}
-int cifs_unlink(struct inode *inode, struct dentry *direntry)
+int cifs_unlink(struct inode *dir, struct dentry *dentry)
{
int rc = 0;
int xid;
- struct cifs_sb_info *cifs_sb;
- struct cifsTconInfo *pTcon;
char *full_path = NULL;
+ struct inode *inode = dentry->d_inode;
struct cifsInodeInfo *cifsInode;
+ struct super_block *sb = dir->i_sb;
+ struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
+ struct cifsTconInfo *tcon = cifs_sb->tcon;
FILE_BASIC_INFO *pinfo_buf;
- cFYI(1, ("cifs_unlink, inode = 0x%p", inode));
+ cFYI(1, ("cifs_unlink, dir=0x%p, dentry=0x%p", dir, dentry));
xid = GetXid();
- if (inode)
- cifs_sb = CIFS_SB(inode->i_sb);
- else
- cifs_sb = CIFS_SB(direntry->d_sb);
- pTcon = cifs_sb->tcon;
-
- /* Unlink can be called from rename so we can not grab the sem here
- since we deadlock otherwise */
-/* mutex_lock(&direntry->d_sb->s_vfs_rename_mutex);*/
- full_path = build_path_from_dentry(direntry);
-/* mutex_unlock(&direntry->d_sb->s_vfs_rename_mutex);*/
+ /* Unlink can be called from rename so we can not take the
+ * sb->s_vfs_rename_mutex here */
+ full_path = build_path_from_dentry(dentry);
if (full_path == NULL) {
FreeXid(xid);
return -ENOMEM;
}
- if ((pTcon->ses->capabilities & CAP_UNIX) &&
+ if ((tcon->ses->capabilities & CAP_UNIX) &&
(CIFS_UNIX_POSIX_PATH_OPS_CAP &
- le64_to_cpu(pTcon->fsUnixInfo.Capability))) {
- rc = CIFSPOSIXDelFile(xid, pTcon, full_path,
+ le64_to_cpu(tcon->fsUnixInfo.Capability))) {
+ rc = CIFSPOSIXDelFile(xid, tcon, full_path,
SMB_POSIX_UNLINK_FILE_TARGET, cifs_sb->local_nls,
cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
cFYI(1, ("posix del rc %d", rc));
@@ -706,31 +700,31 @@ int cifs_unlink(struct inode *inode, struct dentry *direntry)
goto psx_del_no_retry;
}
- rc = CIFSSMBDelFile(xid, pTcon, full_path, cifs_sb->local_nls,
+ rc = CIFSSMBDelFile(xid, tcon, full_path, cifs_sb->local_nls,
cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
psx_del_no_retry:
if (!rc) {
- if (direntry->d_inode)
- drop_nlink(direntry->d_inode);
+ if (inode)
+ drop_nlink(inode);
} else if (rc == -ENOENT) {
- d_drop(direntry);
+ d_drop(dentry);
} else if (rc == -ETXTBSY) {
int oplock = 0;
__u16 netfid;
- rc = CIFSSMBOpen(xid, pTcon, full_path, FILE_OPEN, DELETE,
+ rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN, DELETE,
CREATE_NOT_DIR | CREATE_DELETE_ON_CLOSE,
&netfid, &oplock, NULL, cifs_sb->local_nls,
cifs_sb->mnt_cifs_flags &
CIFS_MOUNT_MAP_SPECIAL_CHR);
if (rc == 0) {
- CIFSSMBRenameOpenFile(xid, pTcon, netfid, NULL,
+ CIFSSMBRenameOpenFile(xid, tcon, netfid, NULL,
cifs_sb->local_nls,
cifs_sb->mnt_cifs_flags &
CIFS_MOUNT_MAP_SPECIAL_CHR);
- CIFSSMBClose(xid, pTcon, netfid);
- if (direntry->d_inode)
- drop_nlink(direntry->d_inode);
+ CIFSSMBClose(xid, tcon, netfid);
+ if (inode)
+ drop_nlink(inode);
}
} else if (rc == -EACCES) {
/* try only if r/o attribute set in local lookup data? */
@@ -738,8 +732,8 @@ psx_del_no_retry:
if (pinfo_buf) {
/* ATTRS set to normal clears r/o bit */
pinfo_buf->Attributes = cpu_to_le32(ATTR_NORMAL);
- if (!(pTcon->ses->flags & CIFS_SES_NT4))
- rc = CIFSSMBSetPathInfo(xid, pTcon, full_path,
+ if (!(tcon->ses->flags & CIFS_SES_NT4))
+ rc = CIFSSMBSetPathInfo(xid, tcon, full_path,
pinfo_buf,
cifs_sb->local_nls,
cifs_sb->mnt_cifs_flags &
@@ -750,7 +744,7 @@ psx_del_no_retry:
if (rc == -EOPNOTSUPP) {
int oplock = 0;
__u16 netfid;
- /* rc = CIFSSMBSetAttrLegacy(xid, pTcon,
+ /* rc = CIFSSMBSetAttrLegacy(xid, tcon,
full_path,
(__u16)ATTR_NORMAL,
cifs_sb->local_nls);
@@ -761,7 +755,7 @@ psx_del_no_retry:
/* BB could scan to see if we already have it open
and pass in pid of opener to function */
- rc = CIFSSMBOpen(xid, pTcon, full_path,
+ rc = CIFSSMBOpen(xid, tcon, full_path,
FILE_OPEN, SYNCHRONIZE |
FILE_WRITE_ATTRIBUTES, 0,
&netfid, &oplock, NULL,
@@ -769,28 +763,28 @@ psx_del_no_retry:
cifs_sb->mnt_cifs_flags &
CIFS_MOUNT_MAP_SPECIAL_CHR);
if (rc == 0) {
- rc = CIFSSMBSetFileInfo(xid, pTcon,
+ rc = CIFSSMBSetFileInfo(xid, tcon,
pinfo_buf,
netfid,
current->tgid);
- CIFSSMBClose(xid, pTcon, netfid);
+ CIFSSMBClose(xid, tcon, netfid);
}
}
kfree(pinfo_buf);
}
if (rc == 0) {
- rc = CIFSSMBDelFile(xid, pTcon, full_path,
+ rc = CIFSSMBDelFile(xid, tcon, full_path,
cifs_sb->local_nls,
cifs_sb->mnt_cifs_flags &
CIFS_MOUNT_MAP_SPECIAL_CHR);
if (!rc) {
- if (direntry->d_inode)
- drop_nlink(direntry->d_inode);
+ if (inode)
+ drop_nlink(inode);
} else if (rc == -ETXTBSY) {
int oplock = 0;
__u16 netfid;
- rc = CIFSSMBOpen(xid, pTcon, full_path,
+ rc = CIFSSMBOpen(xid, tcon, full_path,
FILE_OPEN, DELETE,
CREATE_NOT_DIR |
CREATE_DELETE_ON_CLOSE,
@@ -799,30 +793,28 @@ psx_del_no_retry:
cifs_sb->mnt_cifs_flags &
CIFS_MOUNT_MAP_SPECIAL_CHR);
if (rc == 0) {
- CIFSSMBRenameOpenFile(xid, pTcon,
+ CIFSSMBRenameOpenFile(xid, tcon,
netfid, NULL,
cifs_sb->local_nls,
cifs_sb->mnt_cifs_flags &
CIFS_MOUNT_MAP_SPECIAL_CHR);
- CIFSSMBClose(xid, pTcon, netfid);
- if (direntry->d_inode)
- drop_nlink(direntry->d_inode);
+ CIFSSMBClose(xid, tcon, netfid);
+ if (inode)
+ drop_nlink(inode);
}
/* BB if rc = -ETXTBUSY goto the rename logic BB */
}
}
}
- if (direntry->d_inode) {
- cifsInode = CIFS_I(direntry->d_inode);
- cifsInode->time = 0; /* will force revalidate to get info
- when needed */
- direntry->d_inode->i_ctime = current_fs_time(inode->i_sb);
- }
if (inode) {
- inode->i_ctime = inode->i_mtime = current_fs_time(inode->i_sb);
cifsInode = CIFS_I(inode);
- cifsInode->time = 0; /* force revalidate of dir as well */
+ cifsInode->time = 0; /* will force revalidate to get info
+ when needed */
+ inode->i_ctime = current_fs_time(sb);
}
+ dir->i_ctime = dir->i_mtime = current_fs_time(sb);
+ cifsInode = CIFS_I(dir);
+ cifsInode->time = 0; /* force revalidate of dir as well */
kfree(full_path);
FreeXid(xid);
--
1.5.5.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/4] cifs: use common code for turning off ATTR_READONLY in cifs_unlink
2008-09-16 18:05 [PATCH 0/4] cifs: clean up cifs_unlink and fix silly-renames (try #2) Jeff Layton
2008-09-16 18:05 ` [PATCH 1/4] cifs: clean up variables in cifs_unlink Jeff Layton
@ 2008-09-16 18:05 ` Jeff Layton
2008-09-16 23:35 ` Steve French
2008-09-16 18:05 ` [PATCH 3/4] cifs: move silly-rename logic into helper function Jeff Layton
2008-09-16 18:05 ` [PATCH 4/4] cifs: add function to set file disposition Jeff Layton
3 siblings, 1 reply; 11+ messages in thread
From: Jeff Layton @ 2008-09-16 18:05 UTC (permalink / raw)
To: smfrench; +Cc: smfrench, linux-fsdevel, linux-cifs-client
We already have a cifs_set_file_info function that can flip DOS
attribute bits. Have cifs_unlink call it to handle turning ATTR_HIDDEN
on and ATTR_READONLY off when an unlink attempt returns -EACCES.
This also removes a level of indentation from cifs_unlink.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/cifs/inode.c | 118 +++++++++++++++++++++---------------------------------
1 files changed, 46 insertions(+), 72 deletions(-)
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 2f75714..783f4ad 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -29,6 +29,8 @@
#include "cifs_debug.h"
#include "cifs_fs_sb.h"
+static int cifs_set_file_info(struct inode *inode, struct iattr *attrs,
+ int xid, char *full_path, __u32 dosattr);
static void cifs_set_ops(struct inode *inode, const bool is_dfs_referral)
{
@@ -675,7 +677,8 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry)
struct super_block *sb = dir->i_sb;
struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
struct cifsTconInfo *tcon = cifs_sb->tcon;
- FILE_BASIC_INFO *pinfo_buf;
+ struct iattr *attrs;
+ __u32 dosattr;
cFYI(1, ("cifs_unlink, dir=0x%p, dentry=0x%p", dir, dentry));
@@ -728,84 +731,55 @@ psx_del_no_retry:
}
} else if (rc == -EACCES) {
/* try only if r/o attribute set in local lookup data? */
- pinfo_buf = kzalloc(sizeof(FILE_BASIC_INFO), GFP_KERNEL);
- if (pinfo_buf) {
- /* ATTRS set to normal clears r/o bit */
- pinfo_buf->Attributes = cpu_to_le32(ATTR_NORMAL);
- if (!(tcon->ses->flags & CIFS_SES_NT4))
- rc = CIFSSMBSetPathInfo(xid, tcon, full_path,
- pinfo_buf,
- cifs_sb->local_nls,
- cifs_sb->mnt_cifs_flags &
- CIFS_MOUNT_MAP_SPECIAL_CHR);
- else
- rc = -EOPNOTSUPP;
-
- if (rc == -EOPNOTSUPP) {
- int oplock = 0;
- __u16 netfid;
- /* rc = CIFSSMBSetAttrLegacy(xid, tcon,
- full_path,
- (__u16)ATTR_NORMAL,
- cifs_sb->local_nls);
- For some strange reason it seems that NT4 eats the
- old setattr call without actually setting the
- attributes so on to the third attempted workaround
- */
-
- /* BB could scan to see if we already have it open
- and pass in pid of opener to function */
- rc = CIFSSMBOpen(xid, tcon, full_path,
- FILE_OPEN, SYNCHRONIZE |
- FILE_WRITE_ATTRIBUTES, 0,
- &netfid, &oplock, NULL,
- cifs_sb->local_nls,
- cifs_sb->mnt_cifs_flags &
- CIFS_MOUNT_MAP_SPECIAL_CHR);
- if (rc == 0) {
- rc = CIFSSMBSetFileInfo(xid, tcon,
- pinfo_buf,
- netfid,
- current->tgid);
- CIFSSMBClose(xid, tcon, netfid);
- }
- }
- kfree(pinfo_buf);
+ attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
+ if (attrs == NULL) {
+ rc = -ENOMEM;
+ goto out_reval;
}
+
+ /* try to reset dos attributes */
+ cifsInode = CIFS_I(inode);
+ dosattr = cifsInode->cifsAttrs & ~ATTR_READONLY;
+ if (dosattr == 0)
+ dosattr |= ATTR_NORMAL;
+ dosattr |= ATTR_HIDDEN;
+
+ rc = cifs_set_file_info(inode, attrs, xid, full_path, dosattr);
+ kfree(attrs);
+ if (rc != 0)
+ goto out_reval;
+ rc = CIFSSMBDelFile(xid, tcon, full_path, cifs_sb->local_nls,
+ cifs_sb->mnt_cifs_flags &
+ CIFS_MOUNT_MAP_SPECIAL_CHR);
if (rc == 0) {
- rc = CIFSSMBDelFile(xid, tcon, full_path,
- cifs_sb->local_nls,
- cifs_sb->mnt_cifs_flags &
- CIFS_MOUNT_MAP_SPECIAL_CHR);
- if (!rc) {
+ if (inode)
+ drop_nlink(inode);
+ } else if (rc == -ETXTBSY) {
+ int oplock = 0;
+ __u16 netfid;
+
+ rc = CIFSSMBOpen(xid, tcon, full_path,
+ FILE_OPEN, DELETE,
+ CREATE_NOT_DIR |
+ CREATE_DELETE_ON_CLOSE,
+ &netfid, &oplock, NULL,
+ cifs_sb->local_nls,
+ cifs_sb->mnt_cifs_flags &
+ CIFS_MOUNT_MAP_SPECIAL_CHR);
+ if (rc == 0) {
+ CIFSSMBRenameOpenFile(xid, tcon,
+ netfid, NULL,
+ cifs_sb->local_nls,
+ cifs_sb->mnt_cifs_flags &
+ CIFS_MOUNT_MAP_SPECIAL_CHR);
+ CIFSSMBClose(xid, tcon, netfid);
if (inode)
drop_nlink(inode);
- } else if (rc == -ETXTBSY) {
- int oplock = 0;
- __u16 netfid;
-
- rc = CIFSSMBOpen(xid, tcon, full_path,
- FILE_OPEN, DELETE,
- CREATE_NOT_DIR |
- CREATE_DELETE_ON_CLOSE,
- &netfid, &oplock, NULL,
- cifs_sb->local_nls,
- cifs_sb->mnt_cifs_flags &
- CIFS_MOUNT_MAP_SPECIAL_CHR);
- if (rc == 0) {
- CIFSSMBRenameOpenFile(xid, tcon,
- netfid, NULL,
- cifs_sb->local_nls,
- cifs_sb->mnt_cifs_flags &
- CIFS_MOUNT_MAP_SPECIAL_CHR);
- CIFSSMBClose(xid, tcon, netfid);
- if (inode)
- drop_nlink(inode);
- }
- /* BB if rc = -ETXTBUSY goto the rename logic BB */
}
+ /* BB if rc = -ETXTBUSY goto the rename logic BB */
}
}
+out_reval:
if (inode) {
cifsInode = CIFS_I(inode);
cifsInode->time = 0; /* will force revalidate to get info
--
1.5.5.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/4] cifs: move silly-rename logic into helper function
2008-09-16 18:05 [PATCH 0/4] cifs: clean up cifs_unlink and fix silly-renames (try #2) Jeff Layton
2008-09-16 18:05 ` [PATCH 1/4] cifs: clean up variables in cifs_unlink Jeff Layton
2008-09-16 18:05 ` [PATCH 2/4] cifs: use common code for turning off ATTR_READONLY " Jeff Layton
@ 2008-09-16 18:05 ` Jeff Layton
2008-09-16 18:05 ` [PATCH 4/4] cifs: add function to set file disposition Jeff Layton
3 siblings, 0 replies; 11+ messages in thread
From: Jeff Layton @ 2008-09-16 18:05 UTC (permalink / raw)
To: smfrench; +Cc: linux-fsdevel, linux-cifs-client, smfrench
When a file is still open on the server, we attempt to set the
DELETE_ON_CLOSE bit and rename it to a new filename. When the
last opener closes the file, the server should delete it.
This patch moves this mechanism into a helper function and has
the two places in cifs_unlink that do silly-renames call it. It
also fixes the open flags to be correct.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/cifs/inode.c | 98 +++++++++++++++++++++++++++++++++----------------------
1 files changed, 59 insertions(+), 39 deletions(-)
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 783f4ad..03fde14 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -667,6 +667,59 @@ struct inode *cifs_iget(struct super_block *sb, unsigned long ino)
return inode;
}
+/*
+ * open the given file (if it isn't already), set the DELETE_ON_CLOSE bit
+ * and rename it to a random name that hopefully won't conflict with
+ * anything else.
+ */
+static int
+cifs_silly_rename(char *full_path, struct inode *inode, int xid)
+{
+ int oplock = 0;
+ int rc;
+ __u16 netfid;
+ struct cifsInodeInfo *cifsInode = CIFS_I(inode);
+ struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
+ struct cifsTconInfo *tcon = cifs_sb->tcon;
+ __u32 dosattr;
+ FILE_BASIC_INFO *info_buf;
+
+ rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN,
+ DELETE|FILE_WRITE_ATTRIBUTES,
+ CREATE_NOT_DIR|CREATE_DELETE_ON_CLOSE,
+ &netfid, &oplock, NULL, cifs_sb->local_nls,
+ cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
+ if (rc != 0)
+ goto out;
+
+ /* set ATTR_HIDDEN and clear ATTR_READONLY */
+ cifsInode = CIFS_I(inode);
+ dosattr = cifsInode->cifsAttrs & ~ATTR_READONLY;
+ if (dosattr == 0)
+ dosattr |= ATTR_NORMAL;
+ dosattr |= ATTR_HIDDEN;
+
+ info_buf = kzalloc(sizeof(*info_buf), GFP_KERNEL);
+ if (info_buf == NULL) {
+ rc = -ENOMEM;
+ goto out_close;
+ }
+ info_buf->Attributes = cpu_to_le32(dosattr);
+ rc = CIFSSMBSetFileInfo(xid, tcon, info_buf, netfid, current->tgid);
+ kfree(info_buf);
+ if (rc != 0)
+ goto out_close;
+
+ /* silly-rename the file */
+ rc = CIFSSMBRenameOpenFile(xid, tcon, netfid, NULL, cifs_sb->local_nls,
+ cifs_sb->mnt_cifs_flags &
+ CIFS_MOUNT_MAP_SPECIAL_CHR);
+out_close:
+ CIFSSMBClose(xid, tcon, netfid);
+out:
+ return rc;
+}
+
int cifs_unlink(struct inode *dir, struct dentry *dentry)
{
int rc = 0;
@@ -712,23 +765,9 @@ psx_del_no_retry:
} else if (rc == -ENOENT) {
d_drop(dentry);
} else if (rc == -ETXTBSY) {
- int oplock = 0;
- __u16 netfid;
-
- rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN, DELETE,
- CREATE_NOT_DIR | CREATE_DELETE_ON_CLOSE,
- &netfid, &oplock, NULL, cifs_sb->local_nls,
- cifs_sb->mnt_cifs_flags &
- CIFS_MOUNT_MAP_SPECIAL_CHR);
- if (rc == 0) {
- CIFSSMBRenameOpenFile(xid, tcon, netfid, NULL,
- cifs_sb->local_nls,
- cifs_sb->mnt_cifs_flags &
- CIFS_MOUNT_MAP_SPECIAL_CHR);
- CIFSSMBClose(xid, tcon, netfid);
- if (inode)
- drop_nlink(inode);
- }
+ rc = cifs_silly_rename(full_path, inode, xid);
+ if (rc == 0)
+ drop_nlink(inode);
} else if (rc == -EACCES) {
/* try only if r/o attribute set in local lookup data? */
attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
@@ -755,28 +794,9 @@ psx_del_no_retry:
if (inode)
drop_nlink(inode);
} else if (rc == -ETXTBSY) {
- int oplock = 0;
- __u16 netfid;
-
- rc = CIFSSMBOpen(xid, tcon, full_path,
- FILE_OPEN, DELETE,
- CREATE_NOT_DIR |
- CREATE_DELETE_ON_CLOSE,
- &netfid, &oplock, NULL,
- cifs_sb->local_nls,
- cifs_sb->mnt_cifs_flags &
- CIFS_MOUNT_MAP_SPECIAL_CHR);
- if (rc == 0) {
- CIFSSMBRenameOpenFile(xid, tcon,
- netfid, NULL,
- cifs_sb->local_nls,
- cifs_sb->mnt_cifs_flags &
- CIFS_MOUNT_MAP_SPECIAL_CHR);
- CIFSSMBClose(xid, tcon, netfid);
- if (inode)
- drop_nlink(inode);
- }
- /* BB if rc = -ETXTBUSY goto the rename logic BB */
+ rc = cifs_silly_rename(full_path, inode, xid);
+ if (rc == 0)
+ drop_nlink(inode);
}
}
out_reval:
--
1.5.5.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/4] cifs: add function to set file disposition
2008-09-16 18:05 [PATCH 0/4] cifs: clean up cifs_unlink and fix silly-renames (try #2) Jeff Layton
` (2 preceding siblings ...)
2008-09-16 18:05 ` [PATCH 3/4] cifs: move silly-rename logic into helper function Jeff Layton
@ 2008-09-16 18:05 ` Jeff Layton
2008-09-17 1:33 ` Steve French
3 siblings, 1 reply; 11+ messages in thread
From: Jeff Layton @ 2008-09-16 18:05 UTC (permalink / raw)
To: smfrench; +Cc: linux-fsdevel, linux-cifs-client, smfrench
The proper way to set the delete on close bit on an already existing
file is to use SET_FILE_INFO with an infolevel of
SMB_FILE_DISPOSITION_INFO. Add a function to do that and have the
silly-rename code use it.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/cifs/cifsproto.h | 2 +
fs/cifs/cifssmb.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++
fs/cifs/inode.c | 9 ++++++-
3 files changed, 64 insertions(+), 2 deletions(-)
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index bee053f..0cff7fe 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -179,6 +179,8 @@ extern int CIFSSMBSetPathInfo(const int xid, struct cifsTconInfo *tcon,
extern int CIFSSMBSetFileInfo(const int xid, struct cifsTconInfo *tcon,
const FILE_BASIC_INFO *data, __u16 fid,
__u32 pid_of_opener);
+extern int CIFSSMBSetFileDisposition(const int xid, struct cifsTconInfo *tcon,
+ bool delete_file, __u16 fid, __u32 pid_of_opener);
#if 0
extern int CIFSSMBSetAttrLegacy(int xid, struct cifsTconInfo *tcon,
char *fileName, __u16 dos_attributes,
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 3d68e28..7504d15 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -4876,6 +4876,61 @@ CIFSSMBSetFileInfo(const int xid, struct cifsTconInfo *tcon,
return rc;
}
+int
+CIFSSMBSetFileDisposition(const int xid, struct cifsTconInfo *tcon,
+ bool delete_file, __u16 fid, __u32 pid_of_opener)
+{
+ struct smb_com_transaction2_sfi_req *pSMB = NULL;
+ char *data_offset;
+ int rc = 0;
+ __u16 params, param_offset, offset, byte_count, count;
+
+ cFYI(1, ("Set File Disposition (via SetFileInfo)"));
+ rc = small_smb_init(SMB_COM_TRANSACTION2, 15, tcon, (void **) &pSMB);
+
+ if (rc)
+ return rc;
+
+ pSMB->hdr.Pid = cpu_to_le16((__u16)pid_of_opener);
+ pSMB->hdr.PidHigh = cpu_to_le16((__u16)(pid_of_opener >> 16));
+
+ params = 6;
+ pSMB->MaxSetupCount = 0;
+ pSMB->Reserved = 0;
+ pSMB->Flags = 0;
+ pSMB->Timeout = 0;
+ pSMB->Reserved2 = 0;
+ param_offset = offsetof(struct smb_com_transaction2_sfi_req, Fid) - 4;
+ offset = param_offset + params;
+
+ data_offset = (char *) (&pSMB->hdr.Protocol) + offset;
+
+ count = 1;
+ pSMB->MaxParameterCount = cpu_to_le16(2);
+ /* BB find max SMB PDU from sess */
+ pSMB->MaxDataCount = cpu_to_le16(1000);
+ pSMB->SetupCount = 1;
+ pSMB->Reserved3 = 0;
+ pSMB->SubCommand = cpu_to_le16(TRANS2_SET_FILE_INFORMATION);
+ byte_count = 3 /* pad */ + params + count;
+ pSMB->DataCount = cpu_to_le16(count);
+ pSMB->ParameterCount = cpu_to_le16(params);
+ pSMB->TotalDataCount = pSMB->DataCount;
+ pSMB->TotalParameterCount = pSMB->ParameterCount;
+ pSMB->ParameterOffset = cpu_to_le16(param_offset);
+ pSMB->DataOffset = cpu_to_le16(offset);
+ pSMB->Fid = fid;
+ pSMB->InformationLevel = cpu_to_le16(SMB_SET_FILE_DISPOSITION_INFO);
+ pSMB->Reserved4 = 0;
+ pSMB->hdr.smb_buf_length += byte_count;
+ pSMB->ByteCount = cpu_to_le16(byte_count);
+ *data_offset = delete_file ? 1 : 0;
+ rc = SendReceiveNoRsp(xid, tcon->ses, (struct smb_hdr *) pSMB, 0);
+ if (rc)
+ cFYI(1, ("Send error in SetFileDisposition = %d", rc));
+
+ return rc;
+}
int
CIFSSMBSetPathInfo(const int xid, struct cifsTconInfo *tcon,
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 03fde14..2f508a3 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -685,8 +685,7 @@ cifs_silly_rename(char *full_path, struct inode *inode, int xid)
FILE_BASIC_INFO *info_buf;
rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN,
- DELETE|FILE_WRITE_ATTRIBUTES,
- CREATE_NOT_DIR|CREATE_DELETE_ON_CLOSE,
+ DELETE|FILE_WRITE_ATTRIBUTES, CREATE_NOT_DIR,
&netfid, &oplock, NULL, cifs_sb->local_nls,
cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
if (rc != 0)
@@ -714,6 +713,12 @@ cifs_silly_rename(char *full_path, struct inode *inode, int xid)
rc = CIFSSMBRenameOpenFile(xid, tcon, netfid, NULL, cifs_sb->local_nls,
cifs_sb->mnt_cifs_flags &
CIFS_MOUNT_MAP_SPECIAL_CHR);
+ if (rc != 0)
+ goto out_close;
+
+ /* set DELETE_ON_CLOSE */
+ rc = CIFSSMBSetFileDisposition(xid, tcon, true, netfid, current->tgid);
+
out_close:
CIFSSMBClose(xid, tcon, netfid);
out:
--
1.5.5.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] cifs: use common code for turning off ATTR_READONLY in cifs_unlink
2008-09-16 18:05 ` [PATCH 2/4] cifs: use common code for turning off ATTR_READONLY " Jeff Layton
@ 2008-09-16 23:35 ` Steve French
2008-09-16 23:37 ` Steve French
2008-09-16 23:52 ` Jeff Layton
0 siblings, 2 replies; 11+ messages in thread
From: Steve French @ 2008-09-16 23:35 UTC (permalink / raw)
To: Jeff Layton; +Cc: linux-fsdevel, linux-cifs-client, smfrench
[-- Attachment #1.1: Type: text/plain, Size: 8627 bytes --]
I will commit the patch to the cifs git development tree but wanted to
consider things:
1) your patch changes from open with FILE_WRITE_ATTRIBUTES to open with
GENERIC_WRITE by calling cifs_set_file_info
2) we should probably do the Open then RenameOpenFile whether or not the
cifs_set_file_info fails
3) to remove the extern for a static in the c file, the function
cifs_set_file_info should be moved forward in the file
On Tue, Sep 16, 2008 at 1:05 PM, Jeff Layton <jlayton@redhat.com> wrote:
> We already have a cifs_set_file_info function that can flip DOS
> attribute bits. Have cifs_unlink call it to handle turning ATTR_HIDDEN
> on and ATTR_READONLY off when an unlink attempt returns -EACCES.
>
> This also removes a level of indentation from cifs_unlink.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> fs/cifs/inode.c | 118
> +++++++++++++++++++++---------------------------------
> 1 files changed, 46 insertions(+), 72 deletions(-)
>
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 2f75714..783f4ad 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -29,6 +29,8 @@
> #include "cifs_debug.h"
> #include "cifs_fs_sb.h"
>
> +static int cifs_set_file_info(struct inode *inode, struct iattr *attrs,
> + int xid, char *full_path, __u32 dosattr);
>
> static void cifs_set_ops(struct inode *inode, const bool is_dfs_referral)
> {
> @@ -675,7 +677,8 @@ int cifs_unlink(struct inode *dir, struct dentry
> *dentry)
> struct super_block *sb = dir->i_sb;
> struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
> struct cifsTconInfo *tcon = cifs_sb->tcon;
> - FILE_BASIC_INFO *pinfo_buf;
> + struct iattr *attrs;
> + __u32 dosattr;
>
> cFYI(1, ("cifs_unlink, dir=0x%p, dentry=0x%p", dir, dentry));
>
> @@ -728,84 +731,55 @@ psx_del_no_retry:
> }
> } else if (rc == -EACCES) {
> /* try only if r/o attribute set in local lookup data? */
> - pinfo_buf = kzalloc(sizeof(FILE_BASIC_INFO), GFP_KERNEL);
> - if (pinfo_buf) {
> - /* ATTRS set to normal clears r/o bit */
> - pinfo_buf->Attributes = cpu_to_le32(ATTR_NORMAL);
> - if (!(tcon->ses->flags & CIFS_SES_NT4))
> - rc = CIFSSMBSetPathInfo(xid, tcon,
> full_path,
> - pinfo_buf,
> - cifs_sb->local_nls,
> -
> cifs_sb->mnt_cifs_flags &
> -
> CIFS_MOUNT_MAP_SPECIAL_CHR);
> - else
> - rc = -EOPNOTSUPP;
> -
> - if (rc == -EOPNOTSUPP) {
> - int oplock = 0;
> - __u16 netfid;
> - /* rc = CIFSSMBSetAttrLegacy(xid, tcon,
> - full_path,
> -
> (__u16)ATTR_NORMAL,
> -
> cifs_sb->local_nls);
> - For some strange reason it seems that NT4 eats
> the
> - old setattr call without actually setting the
> - attributes so on to the third attempted
> workaround
> - */
> -
> - /* BB could scan to see if we already have it open
> - and pass in pid of opener to function */
> - rc = CIFSSMBOpen(xid, tcon, full_path,
> - FILE_OPEN, SYNCHRONIZE |
> - FILE_WRITE_ATTRIBUTES, 0,
> - &netfid, &oplock, NULL,
> - cifs_sb->local_nls,
> - cifs_sb->mnt_cifs_flags &
> -
> CIFS_MOUNT_MAP_SPECIAL_CHR);
> - if (rc == 0) {
> - rc = CIFSSMBSetFileInfo(xid, tcon,
> - pinfo_buf,
> - netfid,
> -
> current->tgid);
> - CIFSSMBClose(xid, tcon, netfid);
> - }
> - }
> - kfree(pinfo_buf);
> + attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
> + if (attrs == NULL) {
> + rc = -ENOMEM;
> + goto out_reval;
> }
> +
> + /* try to reset dos attributes */
> + cifsInode = CIFS_I(inode);
> + dosattr = cifsInode->cifsAttrs & ~ATTR_READONLY;
> + if (dosattr == 0)
> + dosattr |= ATTR_NORMAL;
> + dosattr |= ATTR_HIDDEN;
> +
> + rc = cifs_set_file_info(inode, attrs, xid, full_path,
> dosattr);
> + kfree(attrs);
> + if (rc != 0)
> + goto out_reval;
> + rc = CIFSSMBDelFile(xid, tcon, full_path,
> cifs_sb->local_nls,
> + cifs_sb->mnt_cifs_flags &
> + CIFS_MOUNT_MAP_SPECIAL_CHR);
> if (rc == 0) {
> - rc = CIFSSMBDelFile(xid, tcon, full_path,
> - cifs_sb->local_nls,
> - cifs_sb->mnt_cifs_flags &
> -
> CIFS_MOUNT_MAP_SPECIAL_CHR);
> - if (!rc) {
> + if (inode)
> + drop_nlink(inode);
> + } else if (rc == -ETXTBSY) {
> + int oplock = 0;
> + __u16 netfid;
> +
> + rc = CIFSSMBOpen(xid, tcon, full_path,
> + FILE_OPEN, DELETE,
> + CREATE_NOT_DIR |
> + CREATE_DELETE_ON_CLOSE,
> + &netfid, &oplock, NULL,
> + cifs_sb->local_nls,
> + cifs_sb->mnt_cifs_flags &
> + CIFS_MOUNT_MAP_SPECIAL_CHR);
> + if (rc == 0) {
> + CIFSSMBRenameOpenFile(xid, tcon,
> + netfid, NULL,
> + cifs_sb->local_nls,
> + cifs_sb->mnt_cifs_flags &
> + CIFS_MOUNT_MAP_SPECIAL_CHR);
> + CIFSSMBClose(xid, tcon, netfid);
> if (inode)
> drop_nlink(inode);
> - } else if (rc == -ETXTBSY) {
> - int oplock = 0;
> - __u16 netfid;
> -
> - rc = CIFSSMBOpen(xid, tcon, full_path,
> - FILE_OPEN, DELETE,
> - CREATE_NOT_DIR |
> - CREATE_DELETE_ON_CLOSE,
> - &netfid, &oplock, NULL,
> - cifs_sb->local_nls,
> - cifs_sb->mnt_cifs_flags &
> -
> CIFS_MOUNT_MAP_SPECIAL_CHR);
> - if (rc == 0) {
> - CIFSSMBRenameOpenFile(xid, tcon,
> - netfid, NULL,
> - cifs_sb->local_nls,
> - cifs_sb->mnt_cifs_flags &
> -
> CIFS_MOUNT_MAP_SPECIAL_CHR);
> - CIFSSMBClose(xid, tcon, netfid);
> - if (inode)
> - drop_nlink(inode);
> - }
> - /* BB if rc = -ETXTBUSY goto the rename logic BB */
> }
> + /* BB if rc = -ETXTBUSY goto the rename logic BB */
> }
> }
> +out_reval:
> if (inode) {
> cifsInode = CIFS_I(inode);
> cifsInode->time = 0; /* will force revalidate to get info
> --
> 1.5.5.1
>
>
--
Thanks,
Steve
[-- Attachment #1.2: Type: text/html, Size: 20180 bytes --]
[-- Attachment #2: Type: text/plain, Size: 172 bytes --]
_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@lists.samba.org
https://lists.samba.org/mailman/listinfo/linux-cifs-client
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] cifs: use common code for turning off ATTR_READONLY in cifs_unlink
2008-09-16 23:35 ` Steve French
@ 2008-09-16 23:37 ` Steve French
2008-09-16 23:52 ` Jeff Layton
1 sibling, 0 replies; 11+ messages in thread
From: Steve French @ 2008-09-16 23:37 UTC (permalink / raw)
To: linux-fsdevel
I will commit the patch to the cifs git development tree but wanted to
consider things:
1) your patch changes from open with FILE_WRITE_ATTRIBUTES to open
with GENERIC_WRITE by calling cifs_set_file_info
2) we should probably do the Open then RenameOpenFile whether or not
the cifs_set_file_info fails
3) to remove the extern for a static in the c file, the function
cifs_set_file_info should be moved forward in the file
On Tue, Sep 16, 2008 at 6:35 PM, Steve French <smfrench@gmail.com> wrote:
>
> I will commit the patch to the cifs git development tree but wanted to consider things:
>
> 1) your patch changes from open with FILE_WRITE_ATTRIBUTES to open with GENERIC_WRITE by calling cifs_set_file_info
> 2) we should probably do the Open then RenameOpenFile whether or not the cifs_set_file_info fails
> 3) to remove the extern for a static in the c file, the function cifs_set_file_info should be moved forward in the file
>
> On Tue, Sep 16, 2008 at 1:05 PM, Jeff Layton <jlayton@redhat.com> wrote:
>>
>> We already have a cifs_set_file_info function that can flip DOS
>> attribute bits. Have cifs_unlink call it to handle turning ATTR_HIDDEN
>> on and ATTR_READONLY off when an unlink attempt returns -EACCES.
>>
>> This also removes a level of indentation from cifs_unlink.
>>
>> Signed-off-by: Jeff Layton <jlayton@redhat.com>
>> ---
>> fs/cifs/inode.c | 118 +++++++++++++++++++++---------------------------------
>> 1 files changed, 46 insertions(+), 72 deletions(-)
>>
>> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
>> index 2f75714..783f4ad 100644
>> --- a/fs/cifs/inode.c
>> +++ b/fs/cifs/inode.c
>> @@ -29,6 +29,8 @@
>> #include "cifs_debug.h"
>> #include "cifs_fs_sb.h"
>>
>> +static int cifs_set_file_info(struct inode *inode, struct iattr *attrs,
>> + int xid, char *full_path, __u32 dosattr);
>>
>> static void cifs_set_ops(struct inode *inode, const bool is_dfs_referral)
>> {
>> @@ -675,7 +677,8 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry)
>> struct super_block *sb = dir->i_sb;
>> struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
>> struct cifsTconInfo *tcon = cifs_sb->tcon;
>> - FILE_BASIC_INFO *pinfo_buf;
>> + struct iattr *attrs;
>> + __u32 dosattr;
>>
>> cFYI(1, ("cifs_unlink, dir=0x%p, dentry=0x%p", dir, dentry));
>>
>> @@ -728,84 +731,55 @@ psx_del_no_retry:
>> }
>> } else if (rc == -EACCES) {
>> /* try only if r/o attribute set in local lookup data? */
>> - pinfo_buf = kzalloc(sizeof(FILE_BASIC_INFO), GFP_KERNEL);
>> - if (pinfo_buf) {
>> - /* ATTRS set to normal clears r/o bit */
>> - pinfo_buf->Attributes = cpu_to_le32(ATTR_NORMAL);
>> - if (!(tcon->ses->flags & CIFS_SES_NT4))
>> - rc = CIFSSMBSetPathInfo(xid, tcon, full_path,
>> - pinfo_buf,
>> - cifs_sb->local_nls,
>> - cifs_sb->mnt_cifs_flags &
>> - CIFS_MOUNT_MAP_SPECIAL_CHR);
>> - else
>> - rc = -EOPNOTSUPP;
>> -
>> - if (rc == -EOPNOTSUPP) {
>> - int oplock = 0;
>> - __u16 netfid;
>> - /* rc = CIFSSMBSetAttrLegacy(xid, tcon,
>> - full_path,
>> - (__u16)ATTR_NORMAL,
>> - cifs_sb->local_nls);
>> - For some strange reason it seems that NT4 eats the
>> - old setattr call without actually setting the
>> - attributes so on to the third attempted workaround
>> - */
>> -
>> - /* BB could scan to see if we already have it open
>> - and pass in pid of opener to function */
>> - rc = CIFSSMBOpen(xid, tcon, full_path,
>> - FILE_OPEN, SYNCHRONIZE |
>> - FILE_WRITE_ATTRIBUTES, 0,
>> - &netfid, &oplock, NULL,
>> - cifs_sb->local_nls,
>> - cifs_sb->mnt_cifs_flags &
>> - CIFS_MOUNT_MAP_SPECIAL_CHR);
>> - if (rc == 0) {
>> - rc = CIFSSMBSetFileInfo(xid, tcon,
>> - pinfo_buf,
>> - netfid,
>> - current->tgid);
>> - CIFSSMBClose(xid, tcon, netfid);
>> - }
>> - }
>> - kfree(pinfo_buf);
>> + attrs = kzalloc(sizeof(*attrs), GFP_KERNEL);
>> + if (attrs == NULL) {
>> + rc = -ENOMEM;
>> + goto out_reval;
>> }
>> +
>> + /* try to reset dos attributes */
>> + cifsInode = CIFS_I(inode);
>> + dosattr = cifsInode->cifsAttrs & ~ATTR_READONLY;
>> + if (dosattr == 0)
>> + dosattr |= ATTR_NORMAL;
>> + dosattr |= ATTR_HIDDEN;
>> +
>> + rc = cifs_set_file_info(inode, attrs, xid, full_path, dosattr);
>> + kfree(attrs);
>> + if (rc != 0)
>> + goto out_reval;
>> + rc = CIFSSMBDelFile(xid, tcon, full_path, cifs_sb->local_nls,
>> + cifs_sb->mnt_cifs_flags &
>> + CIFS_MOUNT_MAP_SPECIAL_CHR);
>> if (rc == 0) {
>> - rc = CIFSSMBDelFile(xid, tcon, full_path,
>> - cifs_sb->local_nls,
>> - cifs_sb->mnt_cifs_flags &
>> - CIFS_MOUNT_MAP_SPECIAL_CHR);
>> - if (!rc) {
>> + if (inode)
>> + drop_nlink(inode);
>> + } else if (rc == -ETXTBSY) {
>> + int oplock = 0;
>> + __u16 netfid;
>> +
>> + rc = CIFSSMBOpen(xid, tcon, full_path,
>> + FILE_OPEN, DELETE,
>> + CREATE_NOT_DIR |
>> + CREATE_DELETE_ON_CLOSE,
>> + &netfid, &oplock, NULL,
>> + cifs_sb->local_nls,
>> + cifs_sb->mnt_cifs_flags &
>> + CIFS_MOUNT_MAP_SPECIAL_CHR);
>> + if (rc == 0) {
>> + CIFSSMBRenameOpenFile(xid, tcon,
>> + netfid, NULL,
>> + cifs_sb->local_nls,
>> + cifs_sb->mnt_cifs_flags &
>> + CIFS_MOUNT_MAP_SPECIAL_CHR);
>> + CIFSSMBClose(xid, tcon, netfid);
>> if (inode)
>> drop_nlink(inode);
>> - } else if (rc == -ETXTBSY) {
>> - int oplock = 0;
>> - __u16 netfid;
>> -
>> - rc = CIFSSMBOpen(xid, tcon, full_path,
>> - FILE_OPEN, DELETE,
>> - CREATE_NOT_DIR |
>> - CREATE_DELETE_ON_CLOSE,
>> - &netfid, &oplock, NULL,
>> - cifs_sb->local_nls,
>> - cifs_sb->mnt_cifs_flags &
>> - CIFS_MOUNT_MAP_SPECIAL_CHR);
>> - if (rc == 0) {
>> - CIFSSMBRenameOpenFile(xid, tcon,
>> - netfid, NULL,
>> - cifs_sb->local_nls,
>> - cifs_sb->mnt_cifs_flags &
>> - CIFS_MOUNT_MAP_SPECIAL_CHR);
>> - CIFSSMBClose(xid, tcon, netfid);
>> - if (inode)
>> - drop_nlink(inode);
>> - }
>> - /* BB if rc = -ETXTBUSY goto the rename logic BB */
>> }
>> + /* BB if rc = -ETXTBUSY goto the rename logic BB */
>> }
>> }
>> +out_reval:
>> if (inode) {
>> cifsInode = CIFS_I(inode);
>> cifsInode->time = 0; /* will force revalidate to get info
>> --
>> 1.5.5.1
>>
>
>
>
> --
> Thanks,
>
> Steve
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] cifs: use common code for turning off ATTR_READONLY in cifs_unlink
2008-09-16 23:35 ` Steve French
2008-09-16 23:37 ` Steve French
@ 2008-09-16 23:52 ` Jeff Layton
2008-09-17 0:05 ` Steve French
1 sibling, 1 reply; 11+ messages in thread
From: Jeff Layton @ 2008-09-16 23:52 UTC (permalink / raw)
To: Steve French; +Cc: smfrench, linux-fsdevel, linux-cifs-client
On Tue, 16 Sep 2008 18:35:58 -0500
"Steve French" <smfrench@gmail.com> wrote:
> I will commit the patch to the cifs git development tree but wanted to
> consider things:
>
> 1) your patch changes from open with FILE_WRITE_ATTRIBUTES to open with
> GENERIC_WRITE by calling cifs_set_file_info
Ahh ok. Just got the other mail. Yes, we could end up using a superset
of FILE_WRITE_ATTRIBUTES if we already have an open fh we can use. I'm
assuming that's OK (and preferred), but we could change it if we think
it would be a problem.
> 2) we should probably do the Open then RenameOpenFile whether or not the
> cifs_set_file_info fails
You mean that we should go ahead and do the silly_rename logic if the
first CIFSSMBDelOpenFile call returns -ETXTBSY or -EACCES? That's
probably reasonable and more efficient overall. It'll also mean less
complex logic too. I like this idea.
> 3) to remove the extern for a static in the c file, the function
> cifs_set_file_info should be moved forward in the file
>
No objection here. I'll look at doing that.
Good observations all around. There's no huge hurry here, so let me
code up a respun set with some of these changes before you commit the
last 3. We might as well get it right the first time...
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] cifs: use common code for turning off ATTR_READONLY in cifs_unlink
2008-09-16 23:52 ` Jeff Layton
@ 2008-09-17 0:05 ` Steve French
0 siblings, 0 replies; 11+ messages in thread
From: Steve French @ 2008-09-17 0:05 UTC (permalink / raw)
To: Jeff Layton; +Cc: linux-fsdevel, linux-cifs-client
On Tue, Sep 16, 2008 at 6:52 PM, Jeff Layton <jlayton@redhat.com> wrote:
>> 3) to remove the extern for a static in the c file, the function
>> cifs_set_file_info should be moved forward in the file
>>
>
> No objection here. I'll look at doing that.
I changed that and merged it.into the cifs tree.
> Good observations all around. There's no huge hurry here, so let me
> code up a respun set with some of these changes before you commit the
> last 3. We might as well get it right the first time...
>
> --
> Jeff Layton <jlayton@redhat.com>
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/4] cifs: add function to set file disposition
2008-09-16 18:05 ` [PATCH 4/4] cifs: add function to set file disposition Jeff Layton
@ 2008-09-17 1:33 ` Steve French
2008-09-17 1:49 ` [linux-cifs-client] " Jeff Layton
0 siblings, 1 reply; 11+ messages in thread
From: Steve French @ 2008-09-17 1:33 UTC (permalink / raw)
To: Jeff Layton; +Cc: smfrench, linux-fsdevel, linux-cifs-client
Does this work with NT4?
On Tue, Sep 16, 2008 at 1:05 PM, Jeff Layton <jlayton@redhat.com> wrote:
> The proper way to set the delete on close bit on an already existing
> file is to use SET_FILE_INFO with an infolevel of
> SMB_FILE_DISPOSITION_INFO. Add a function to do that and have the
> silly-rename code use it.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> fs/cifs/cifsproto.h | 2 +
> fs/cifs/cifssmb.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++
> fs/cifs/inode.c | 9 ++++++-
> 3 files changed, 64 insertions(+), 2 deletions(-)
>
> diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> index bee053f..0cff7fe 100644
> --- a/fs/cifs/cifsproto.h
> +++ b/fs/cifs/cifsproto.h
> @@ -179,6 +179,8 @@ extern int CIFSSMBSetPathInfo(const int xid, struct cifsTconInfo *tcon,
> extern int CIFSSMBSetFileInfo(const int xid, struct cifsTconInfo *tcon,
> const FILE_BASIC_INFO *data, __u16 fid,
> __u32 pid_of_opener);
> +extern int CIFSSMBSetFileDisposition(const int xid, struct cifsTconInfo *tcon,
> + bool delete_file, __u16 fid, __u32 pid_of_opener);
> #if 0
> extern int CIFSSMBSetAttrLegacy(int xid, struct cifsTconInfo *tcon,
> char *fileName, __u16 dos_attributes,
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 3d68e28..7504d15 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -4876,6 +4876,61 @@ CIFSSMBSetFileInfo(const int xid, struct cifsTconInfo *tcon,
> return rc;
> }
>
> +int
> +CIFSSMBSetFileDisposition(const int xid, struct cifsTconInfo *tcon,
> + bool delete_file, __u16 fid, __u32 pid_of_opener)
> +{
> + struct smb_com_transaction2_sfi_req *pSMB = NULL;
> + char *data_offset;
> + int rc = 0;
> + __u16 params, param_offset, offset, byte_count, count;
> +
> + cFYI(1, ("Set File Disposition (via SetFileInfo)"));
> + rc = small_smb_init(SMB_COM_TRANSACTION2, 15, tcon, (void **) &pSMB);
> +
> + if (rc)
> + return rc;
> +
> + pSMB->hdr.Pid = cpu_to_le16((__u16)pid_of_opener);
> + pSMB->hdr.PidHigh = cpu_to_le16((__u16)(pid_of_opener >> 16));
> +
> + params = 6;
> + pSMB->MaxSetupCount = 0;
> + pSMB->Reserved = 0;
> + pSMB->Flags = 0;
> + pSMB->Timeout = 0;
> + pSMB->Reserved2 = 0;
> + param_offset = offsetof(struct smb_com_transaction2_sfi_req, Fid) - 4;
> + offset = param_offset + params;
> +
> + data_offset = (char *) (&pSMB->hdr.Protocol) + offset;
> +
> + count = 1;
> + pSMB->MaxParameterCount = cpu_to_le16(2);
> + /* BB find max SMB PDU from sess */
> + pSMB->MaxDataCount = cpu_to_le16(1000);
> + pSMB->SetupCount = 1;
> + pSMB->Reserved3 = 0;
> + pSMB->SubCommand = cpu_to_le16(TRANS2_SET_FILE_INFORMATION);
> + byte_count = 3 /* pad */ + params + count;
> + pSMB->DataCount = cpu_to_le16(count);
> + pSMB->ParameterCount = cpu_to_le16(params);
> + pSMB->TotalDataCount = pSMB->DataCount;
> + pSMB->TotalParameterCount = pSMB->ParameterCount;
> + pSMB->ParameterOffset = cpu_to_le16(param_offset);
> + pSMB->DataOffset = cpu_to_le16(offset);
> + pSMB->Fid = fid;
> + pSMB->InformationLevel = cpu_to_le16(SMB_SET_FILE_DISPOSITION_INFO);
> + pSMB->Reserved4 = 0;
> + pSMB->hdr.smb_buf_length += byte_count;
> + pSMB->ByteCount = cpu_to_le16(byte_count);
> + *data_offset = delete_file ? 1 : 0;
> + rc = SendReceiveNoRsp(xid, tcon->ses, (struct smb_hdr *) pSMB, 0);
> + if (rc)
> + cFYI(1, ("Send error in SetFileDisposition = %d", rc));
> +
> + return rc;
> +}
>
> int
> CIFSSMBSetPathInfo(const int xid, struct cifsTconInfo *tcon,
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 03fde14..2f508a3 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -685,8 +685,7 @@ cifs_silly_rename(char *full_path, struct inode *inode, int xid)
> FILE_BASIC_INFO *info_buf;
>
> rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN,
> - DELETE|FILE_WRITE_ATTRIBUTES,
> - CREATE_NOT_DIR|CREATE_DELETE_ON_CLOSE,
> + DELETE|FILE_WRITE_ATTRIBUTES, CREATE_NOT_DIR,
> &netfid, &oplock, NULL, cifs_sb->local_nls,
> cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
> if (rc != 0)
> @@ -714,6 +713,12 @@ cifs_silly_rename(char *full_path, struct inode *inode, int xid)
> rc = CIFSSMBRenameOpenFile(xid, tcon, netfid, NULL, cifs_sb->local_nls,
> cifs_sb->mnt_cifs_flags &
> CIFS_MOUNT_MAP_SPECIAL_CHR);
> + if (rc != 0)
> + goto out_close;
> +
> + /* set DELETE_ON_CLOSE */
> + rc = CIFSSMBSetFileDisposition(xid, tcon, true, netfid, current->tgid);
> +
> out_close:
> CIFSSMBClose(xid, tcon, netfid);
> out:
> --
> 1.5.5.1
>
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [linux-cifs-client] Re: [PATCH 4/4] cifs: add function to set file disposition
2008-09-17 1:33 ` Steve French
@ 2008-09-17 1:49 ` Jeff Layton
0 siblings, 0 replies; 11+ messages in thread
From: Jeff Layton @ 2008-09-17 1:49 UTC (permalink / raw)
To: Steve French; +Cc: linux-fsdevel, linux-cifs-client, smfrench
On Tue, 16 Sep 2008 20:33:49 -0500
"Steve French" <smfrench@gmail.com> wrote:
> Does this work with NT4?
>
Good question. I haven't tested it against NT4. Guess this gives me a
good reason to get a NT4 host set up. :)
> On Tue, Sep 16, 2008 at 1:05 PM, Jeff Layton <jlayton@redhat.com> wrote:
> > The proper way to set the delete on close bit on an already existing
> > file is to use SET_FILE_INFO with an infolevel of
> > SMB_FILE_DISPOSITION_INFO. Add a function to do that and have the
> > silly-rename code use it.
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > fs/cifs/cifsproto.h | 2 +
> > fs/cifs/cifssmb.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > fs/cifs/inode.c | 9 ++++++-
> > 3 files changed, 64 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
> > index bee053f..0cff7fe 100644
> > --- a/fs/cifs/cifsproto.h
> > +++ b/fs/cifs/cifsproto.h
> > @@ -179,6 +179,8 @@ extern int CIFSSMBSetPathInfo(const int xid, struct cifsTconInfo *tcon,
> > extern int CIFSSMBSetFileInfo(const int xid, struct cifsTconInfo *tcon,
> > const FILE_BASIC_INFO *data, __u16 fid,
> > __u32 pid_of_opener);
> > +extern int CIFSSMBSetFileDisposition(const int xid, struct cifsTconInfo *tcon,
> > + bool delete_file, __u16 fid, __u32 pid_of_opener);
> > #if 0
> > extern int CIFSSMBSetAttrLegacy(int xid, struct cifsTconInfo *tcon,
> > char *fileName, __u16 dos_attributes,
> > diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> > index 3d68e28..7504d15 100644
> > --- a/fs/cifs/cifssmb.c
> > +++ b/fs/cifs/cifssmb.c
> > @@ -4876,6 +4876,61 @@ CIFSSMBSetFileInfo(const int xid, struct cifsTconInfo *tcon,
> > return rc;
> > }
> >
> > +int
> > +CIFSSMBSetFileDisposition(const int xid, struct cifsTconInfo *tcon,
> > + bool delete_file, __u16 fid, __u32 pid_of_opener)
> > +{
> > + struct smb_com_transaction2_sfi_req *pSMB = NULL;
> > + char *data_offset;
> > + int rc = 0;
> > + __u16 params, param_offset, offset, byte_count, count;
> > +
> > + cFYI(1, ("Set File Disposition (via SetFileInfo)"));
> > + rc = small_smb_init(SMB_COM_TRANSACTION2, 15, tcon, (void **) &pSMB);
> > +
> > + if (rc)
> > + return rc;
> > +
> > + pSMB->hdr.Pid = cpu_to_le16((__u16)pid_of_opener);
> > + pSMB->hdr.PidHigh = cpu_to_le16((__u16)(pid_of_opener >> 16));
> > +
> > + params = 6;
> > + pSMB->MaxSetupCount = 0;
> > + pSMB->Reserved = 0;
> > + pSMB->Flags = 0;
> > + pSMB->Timeout = 0;
> > + pSMB->Reserved2 = 0;
> > + param_offset = offsetof(struct smb_com_transaction2_sfi_req, Fid) - 4;
> > + offset = param_offset + params;
> > +
> > + data_offset = (char *) (&pSMB->hdr.Protocol) + offset;
> > +
> > + count = 1;
> > + pSMB->MaxParameterCount = cpu_to_le16(2);
> > + /* BB find max SMB PDU from sess */
> > + pSMB->MaxDataCount = cpu_to_le16(1000);
> > + pSMB->SetupCount = 1;
> > + pSMB->Reserved3 = 0;
> > + pSMB->SubCommand = cpu_to_le16(TRANS2_SET_FILE_INFORMATION);
> > + byte_count = 3 /* pad */ + params + count;
> > + pSMB->DataCount = cpu_to_le16(count);
> > + pSMB->ParameterCount = cpu_to_le16(params);
> > + pSMB->TotalDataCount = pSMB->DataCount;
> > + pSMB->TotalParameterCount = pSMB->ParameterCount;
> > + pSMB->ParameterOffset = cpu_to_le16(param_offset);
> > + pSMB->DataOffset = cpu_to_le16(offset);
> > + pSMB->Fid = fid;
> > + pSMB->InformationLevel = cpu_to_le16(SMB_SET_FILE_DISPOSITION_INFO);
> > + pSMB->Reserved4 = 0;
> > + pSMB->hdr.smb_buf_length += byte_count;
> > + pSMB->ByteCount = cpu_to_le16(byte_count);
> > + *data_offset = delete_file ? 1 : 0;
> > + rc = SendReceiveNoRsp(xid, tcon->ses, (struct smb_hdr *) pSMB, 0);
> > + if (rc)
> > + cFYI(1, ("Send error in SetFileDisposition = %d", rc));
> > +
> > + return rc;
> > +}
> >
> > int
> > CIFSSMBSetPathInfo(const int xid, struct cifsTconInfo *tcon,
> > diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> > index 03fde14..2f508a3 100644
> > --- a/fs/cifs/inode.c
> > +++ b/fs/cifs/inode.c
> > @@ -685,8 +685,7 @@ cifs_silly_rename(char *full_path, struct inode *inode, int xid)
> > FILE_BASIC_INFO *info_buf;
> >
> > rc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN,
> > - DELETE|FILE_WRITE_ATTRIBUTES,
> > - CREATE_NOT_DIR|CREATE_DELETE_ON_CLOSE,
> > + DELETE|FILE_WRITE_ATTRIBUTES, CREATE_NOT_DIR,
> > &netfid, &oplock, NULL, cifs_sb->local_nls,
> > cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
> > if (rc != 0)
> > @@ -714,6 +713,12 @@ cifs_silly_rename(char *full_path, struct inode *inode, int xid)
> > rc = CIFSSMBRenameOpenFile(xid, tcon, netfid, NULL, cifs_sb->local_nls,
> > cifs_sb->mnt_cifs_flags &
> > CIFS_MOUNT_MAP_SPECIAL_CHR);
> > + if (rc != 0)
> > + goto out_close;
> > +
> > + /* set DELETE_ON_CLOSE */
> > + rc = CIFSSMBSetFileDisposition(xid, tcon, true, netfid, current->tgid);
> > +
> > out_close:
> > CIFSSMBClose(xid, tcon, netfid);
> > out:
> > --
> > 1.5.5.1
> >
> >
>
>
>
> --
> Thanks,
>
> Steve
> _______________________________________________
> linux-cifs-client mailing list
> linux-cifs-client@lists.samba.org
> https://lists.samba.org/mailman/listinfo/linux-cifs-client
>
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2008-09-17 1:53 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-16 18:05 [PATCH 0/4] cifs: clean up cifs_unlink and fix silly-renames (try #2) Jeff Layton
2008-09-16 18:05 ` [PATCH 1/4] cifs: clean up variables in cifs_unlink Jeff Layton
2008-09-16 18:05 ` [PATCH 2/4] cifs: use common code for turning off ATTR_READONLY " Jeff Layton
2008-09-16 23:35 ` Steve French
2008-09-16 23:37 ` Steve French
2008-09-16 23:52 ` Jeff Layton
2008-09-17 0:05 ` Steve French
2008-09-16 18:05 ` [PATCH 3/4] cifs: move silly-rename logic into helper function Jeff Layton
2008-09-16 18:05 ` [PATCH 4/4] cifs: add function to set file disposition Jeff Layton
2008-09-17 1:33 ` Steve French
2008-09-17 1:49 ` [linux-cifs-client] " Jeff Layton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).