[linux-cifs-client] Re: [PATCH 2/4] cifs: use common code for
turning off ATTR_READONLY in cifs_unlink
Steve French
smfrench at gmail.com
Tue Sep 16 23:35:58 GMT 2008
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 at 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 at 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
-------------- next part --------------
HTML attachment scrubbed and removed
More information about the linux-cifs-client
mailing list