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, ":")) {
>> + fattr.cf_private = kstrdup(full_path, GFP_KERNEL);
>> + 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;
>> }
>>
>
> I have real doubts as to whether this patch is necessary. Here's why:
>
> AIUI, the main concern is that you'll open the "normal" data stream on
> the file, cache a bunch of data. Then, if you open the alternate
> datastream, the kernel will see that as the same inode -- it'll look
> like a hardlink. Then when you go to read, you'll get back the cached
> data from the original datastream instead of the alternate one.
>
> The key point that's missing here is that servers do not hand out
> oplocks for alternate data streams. As long as you've mounted in
> cache=strict mode (which is the default for 3.7), then there should be
> no problem at all, right?
Jeff, where does it say that a server should_not/does_not hand out
oplocks for an alternate data stream?
I have looked for such a documentation and have not found.
The one I read,
http://msdn.microsoft.com/en-us/library/cc308443.aspx
does not mention/preclude alternate data streams from obtaining oplocks.
>
> When you go to open the alternate data stream, it won't matter if the
> original datastream had a bunch of data cached. You'll have no oplock
> for the file anymore and so you'll if effect be doing DIO to the server
> for the alternate datastream anyway.
>
> The only remaining question I have is whether the servers issue oplock
> breaks for the "normal" datastream when an alternate datastream is
> opened. For instance, suppose I do the following pseudocode:
>
> normal = open("file", ...);
>
> ...server issues an oplock for the open. If I then I do:
>
> alternate = open("file:alternate", ...);
>
> ...does the server issue an oplock break for the original oplock prior
> to performing second open?
>
> --
> Jeff Layton <jlayton@xxxxxxxxxx>
In the above case, server does not issue oplock break for the original oplock.
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
