Re: [PATCH v3] cifs: Add support of Alternate Data Streams

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 11, 2012 at 12:54 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> On Thu, 11 Oct 2012 00:33:48 -0500
> Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
>
>> On Wed, Oct 10, 2012 at 2:05 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>> > On Wed, 10 Oct 2012 13:25:59 -0400
>> > Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>> >
>> >> On Wed, 10 Oct 2012 12:22:16 -0500
>> >> Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote:
>> >>
>> >> > On Wed, Oct 10, 2012 at 11:14 AM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>> >> > > On Wed, 10 Oct 2012 10:11:46 -0500
>> >> > > shirishpargaonkar@xxxxxxxxx wrote:
>> >> > >
>> >> > >> From: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
>> >> > >>
>> >> > >>
>> >> > >> Add support of Alternate Data Streams (ads).
>> >> > >>
>> >> > >> The generic access flags that cifs client currently employs are sufficient
>> >> > >> for alternate data streams as well (MS-CIFS 2.2.4.64.1).
>> >> > >>
>> >> > >> The stream file and stream type are specified using : after the file name,
>> >> > >> so that is used to differentiate between a regular file and its
>> >> > >> alternate data streams and stream types.
>> >> > >> Since they all have the same file id, each path name,
>> >> > >> file name:stream name:stream type, has a separate inode with that same
>> >> > >> file id but a distinct private data (path name) in that inode to
>> >> > >> distinguish them.
>> >> > >>
>> >> > >> This scheme applies only to non-posix compliant servers such as Windows.
>> >> > >>
>> >> > >> One operation that does not work is Rename (0x7).
>> >> > >>
>> >> > >>
>> >> > >> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
>> >> > >> ---
>> >> > >>  fs/cifs/cifsfs.c   |    1 +
>> >> > >>  fs/cifs/cifsglob.h |    2 ++
>> >> > >>  fs/cifs/inode.c    |   33 ++++++++++++++++++++++++++++++++-
>> >> > >>  3 files changed, 35 insertions(+), 1 deletions(-)
>> >> > >>
>> >> > >> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
>> >> > >> index e7931cc..3068992 100644
>> >> > >> --- a/fs/cifs/cifsfs.c
>> >> > >> +++ b/fs/cifs/cifsfs.c
>> >> > >> @@ -264,6 +264,7 @@ cifs_evict_inode(struct inode *inode)
>> >> > >>  {
>> >> > >>       truncate_inode_pages(&inode->i_data, 0);
>> >> > >>       clear_inode(inode);
>> >> > >> +     kfree(inode->i_private);
>> >> > >>       cifs_fscache_release_inode_cookie(inode);
>> >> > >>  }
>> >> > >>
>> >> > >> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> >> > >> index f5af252..26d65c7 100644
>> >> > >> --- a/fs/cifs/cifsglob.h
>> >> > >> +++ b/fs/cifs/cifsglob.h
>> >> > >> @@ -1251,6 +1251,7 @@ struct dfs_info3_param {
>> >> > >>  #define CIFS_FATTR_DELETE_PENDING    0x2
>> >> > >>  #define CIFS_FATTR_NEED_REVAL                0x4
>> >> > >>  #define CIFS_FATTR_INO_COLLISION     0x8
>> >> > >> +#define CIFS_FATTR_ALTDATASTR                0x10
>> >> > >>
>> >> > >>  struct cifs_fattr {
>> >> > >>       u32             cf_flags;
>> >> > >> @@ -1268,6 +1269,7 @@ struct cifs_fattr {
>> >> > >>       struct timespec cf_atime;
>> >> > >>       struct timespec cf_mtime;
>> >> > >>       struct timespec cf_ctime;
>> >> > >> +     char            *cf_private;
>> >> > >>  };
>> >> > >>
>> >> > >>  static inline void free_dfs_info_param(struct dfs_info3_param *param)
>> >> > >> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
>> >> > >> index afdff79..93b010b 100644
>> >> > >> --- a/fs/cifs/inode.c
>> >> > >> +++ b/fs/cifs/inode.c
>> >> > >> @@ -619,6 +619,7 @@ cifs_get_inode_info(struct inode **inode, const char *full_path,
>> >> > >>       struct cifs_fattr fattr;
>> >> > >>       struct cifs_search_info *srchinf = NULL;
>> >> > >>
>> >> > >> +     fattr.cf_private = NULL;
>> >> > >>       tlink = cifs_sb_tlink(cifs_sb);
>> >> > >>       if (IS_ERR(tlink))
>> >> > >>               return PTR_ERR(tlink);
>> >> > >> @@ -746,14 +747,34 @@ cifs_get_inode_info(struct inode **inode, const char *full_path,
>> >> > >>       }
>> >> > >>
>> >> > >>       if (!*inode) {
>> >> > >> +             if (strstr(full_path, ":")) {
>> >
>> >                 I think you probably want strrchr here. You want to
>> >                 search the string backward. It's possible that the
>> >                 pathname might have multiple ':' characters in it.
>> >
>> >> > >> +                     fattr.cf_private = kstrdup(full_path, GFP_KERNEL);
>> >
>> >                 Is it really necessary to copy the entire path? This is
>> >                 a frequently traveled bit of code, and I think you
>> >                 could save some cycles by just copying the alternate
>> >                 data stream name.
>> >
>> >> > >> +                     if (!fattr.cf_private) {
>> >> > >> +                             rc = -ENOMEM;
>> >> > >> +                             goto cgii_exit;
>> >> > >> +                     }
>> >> > >> +                     fattr.cf_flags |= CIFS_FATTR_ALTDATASTR;
>> >> > >> +             }
>> >> > >> +
>> >> > >>               *inode = cifs_iget(sb, &fattr);
>> >> > >> -             if (!*inode)
>> >> > >> +             if (*inode) {
>> >> > >> +                     if (strstr(full_path, ":") &&
>> >> > >> +                                     !((*inode)->i_flags & S_PRIVATE)) {
>> >> > >> +                             (*inode)->i_private = kstrdup(fattr.cf_private,
>> >> > >> +                                                     GFP_KERNEL);
>> >> > >> +                             if ((*inode)->i_private)
>> >> > >> +                                     (*inode)->i_flags |= S_PRIVATE;
>> >> > >> +                             else
>> >> > >> +                                     rc = -ENOMEM;
>> >> > >> +                     }
>> >> > >> +             } else
>> >> > >>                       rc = -ENOMEM;
>> >> > >>       } else {
>> >> > >>               cifs_fattr_to_inode(*inode, &fattr);
>> >> > >>       }
>> >> > >>
>> >> > >>  cgii_exit:
>> >> > >> +     kfree(fattr.cf_private);
>> >> > >>       kfree(buf);
>> >> > >>       cifs_put_tlink(tlink);
>> >> > >>       return rc;
>> >> > >> @@ -784,6 +805,16 @@ cifs_find_inode(struct inode *inode, void *opaque)
>> >> > >>       if (S_ISDIR(inode->i_mode) && !hlist_empty(&inode->i_dentry))
>> >> > >>               fattr->cf_flags |= CIFS_FATTR_INO_COLLISION;
>> >> > >>
>> >> > >> +     /* looking for an inode of a alternate data stream (full pathname) */
>> >> > >> +     if (fattr->cf_flags & CIFS_FATTR_ALTDATASTR) {
>> >> > >> +             if (!(inode->i_flags & S_PRIVATE)) {
>> >> > >> +                     return 0;
>> >> > >> +             } else {
>> >> > >> +                     if (strcmp(inode->i_private, fattr->cf_private))
>> >> > >> +                             return 0;
>> >> > >> +             }
>> >> > >> +     }
>> >> > >> +
>> >> > >>       return 1;
>> >> > >>  }
>> >> > >>
>> >> > >
>> >
>> >
>> > At least in my cursory testing, it's possible to create an alternate
>> > data stream on a directory as well as a file. The resulting inode does
>> > not seem to be treated as a directory, but what are the implications if
>> > a particular server does?
>>
>> Which server would that be?  Do you mean like, can you create a
>> parallel alternate data stream directory which can get populated with
>> files (with alternate data streams)?  It is probably not possible,
>> an alternate data stream for a directory is used just the way one
>> is used for a file.
>>
>> >
>> > Also, what's the interaction here with "mapchars" mount option? If
>> > someone specifies that, what behavior should they expect wrt alternate
>> > data streams?
>>
>> It is same as if you were to mount without mapchars mount option.
>> If you mount with mapchars option and create alternate data streams, they
>> are accessible only with mapchars mount option.
>>
>
> ':' is one of the characters remapped when you specify mapchars, so I
> don't think it's something you can ignore.
>
> --
> Jeff Layton <jlayton@xxxxxxxxxx>

Jeff, so they will be separate stream names (one created using
nomapchars (default) mount option and one created using mapchars
mount option) and accessible with respective mount options only
from any client.

I think one more reason for linux cifs client to not even ask for oplocks
irrespective of the server behaviour.

Regards,

Shirish
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux