Re: [PATCH v4 3/7] CIFS: Add O_DENY* open flags support

[Pavel Shilovsky]

2013/4/8 Jeff Layton <jlayton@xxxxxxxxxx>:
> On Fri,  5 Apr 2013 20:57:51 +0400
> Pavel Shilovsky <piastry@xxxxxxxxxxx> wrote:
>
>> Construct share_access value from O_DENY* flags and send it to
>> the server.
>>
>> Signed-off-by: Pavel Shilovsky <piastry@xxxxxxxxxxx>
>> ---
>>  fs/cifs/cifsglob.h | 16 +++++++++++++++-
>>  fs/cifs/dir.c      |  3 +++
>>  fs/cifs/file.c     |  4 ++++
>>  fs/locks.c         |  8 ++++++++
>>  4 files changed, 30 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> index cd4bbf3..7e876b9 100644
>> --- a/fs/cifs/cifsglob.h
>> +++ b/fs/cifs/cifsglob.h
>> @@ -465,7 +465,7 @@ struct smb_vol {
>>                        CIFS_MOUNT_CIFS_BACKUPUID | CIFS_MOUNT_CIFS_BACKUPGID)
>>
>>  #define CIFS_MS_MASK (MS_RDONLY | MS_MANDLOCK | MS_NOEXEC | MS_NOSUID | \
>> -                   MS_NODEV | MS_SYNCHRONOUS)
>> +                   MS_NODEV | MS_SYNCHRONOUS | MS_SHARELOCK)
>>
>>  struct cifs_mnt_data {
>>       struct cifs_sb_info *cifs_sb;
>> @@ -947,6 +947,20 @@ struct cifsFileInfo {
>>       struct work_struct oplock_break; /* work for oplock breaks */
>>  };
>>
>> +static inline int
>> +cifs_get_share_flags(unsigned int flags)
>> +{
>> +     int share_access = 0;
>> +
>> +     if (!(flags & O_DENYREAD))
>> +             share_access |= FILE_SHARE_READ;
>> +     if (!(flags & O_DENYWRITE))
>> +             share_access |= FILE_SHARE_WRITE;
>> +     if (!(flags & O_DENYDELETE))
>> +             share_access |= FILE_SHARE_DELETE;
>> +     return share_access;
>> +}
>> +
>>  struct cifs_io_parms {
>>       __u16 netfid;
>>  #ifdef CONFIG_CIFS_SMB2
>> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
>> index 267b608..d4331de 100644
>> --- a/fs/cifs/dir.c
>> +++ b/fs/cifs/dir.c
>> @@ -294,6 +294,9 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid,
>>       else
>>               cFYI(1, "Create flag not set in create function");
>>
>> +     if (IS_SHARELOCK(inode))
>> +             share_access = cifs_get_share_flags(oflags);
>> +
>>       /*
>>        * BB add processing to set equivalent of mode - e.g. via CreateX with
>>        * ACLs
>> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
>> index 0d524a7..9394b2b 100644
>> --- a/fs/cifs/file.c
>> +++ b/fs/cifs/file.c
>> @@ -210,6 +210,8 @@ cifs_nt_open(char *full_path, struct inode *inode, struct cifs_sb_info *cifs_sb,
>>   *********************************************************************/
>>
>>       disposition = cifs_get_disposition(f_flags);
>> +     if (IS_SHARELOCK(inode))
>> +             share_access = cifs_get_share_flags(f_flags);
>>
>>       /* BB pass O_SYNC flag through on file attributes .. BB */
>>
>> @@ -645,6 +647,8 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flush)
>>       }
>>
>>       desired_access = cifs_convert_flags(cfile->f_flags);
>> +     if (IS_SHARELOCK(inode))
>> +             share_access = cifs_get_share_flags(cfile->f_flags);
>>
>>       if (backup_cred(cifs_sb))
>>               create_options |= CREATE_OPEN_BACKUP_INTENT;
>> diff --git a/fs/locks.c b/fs/locks.c
>> index 1eccc75..2184ddd 100644
>> --- a/fs/locks.c
>> +++ b/fs/locks.c
>> @@ -846,10 +846,18 @@ deny_lock_file(struct file *filp)
>>  {
>>       struct file_lock *lock;
>>       int error = 0;
>> +     const char *fsname = filp->f_path.dentry->d_inode->i_sb->s_type->name;
>>
>>       if (!IS_SHARELOCK(filp->f_path.dentry->d_inode))
>>               return error;
>>
>> +     /*
>> +      * Don't set a lock on CIFS file systems because they can process it
>> +      * themselves.
>> +      */
>> +     if (!strncmp(fsname, "cifs", 4))
>> +             return error;
>> +
>
> NAK....
>
> This is really nasty. Instead of doing this, you should instead create
> a new file_system_type->fs_flags value. Then, set that flag for cifs
> and nfs4, and test it here.

It seems I missed this fs_flags opportunity. Thank you for pointing it
out - I will fix and repost.

--
Best regards,
Pavel Shilovsky.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/