cifs: don't update uniqueid in cifs_fattr_to_inode

We use this value to find an inode within the hash bucket, so we can't
change this without re-hashing the inode. For now, treat this value
as immutable.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/cifs/inode.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

On Fri, May 7, 2010 at 9:03 AM, Jeff Layton <jlayton@redhat.com> wrote:
> We use this value to find an inode within the hash bucket, so we can't
> change this without re-hashing the inode. For now, treat this value
> as immutable.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>  fs/cifs/inode.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 35ec117..ee35c97 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -137,7 +137,6 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
>                inode->i_mode = fattr->cf_mode;
>
>        cifs_i->cifsAttrs = fattr->cf_cifsattrs;
> -       cifs_i->uniqueid = fattr->cf_uniqueid;
>
>        if (fattr->cf_flags & CIFS_FATTR_NEED_REVAL)
>                cifs_i->time = 0;

Seems like there is a case where this is worse (imagine renaming a
file, creating a new
file with the old path)
- We have an inode with particular values (size, attributes etc.) that
are newly updated
- We leave the old inode number

So we have a new different file with the same name but the old inode
number, with
the new file's attributes which seems confusing
to me.   Perhaps in this case it is safer to mark the old inode stale
and unhash it
(similar to your other patch)

On Fri, 7 May 2010 09:32:57 -0500
Steve French <smfrench@gmail.com> wrote:

> On Fri, May 7, 2010 at 9:03 AM, Jeff Layton <jlayton@redhat.com> wrote:
> > We use this value to find an inode within the hash bucket, so we can't
> > change this without re-hashing the inode. For now, treat this value
> > as immutable.
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> >  fs/cifs/inode.c |    1 -
> >  1 files changed, 0 insertions(+), 1 deletions(-)
> >
> > diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> > index 35ec117..ee35c97 100644
> > --- a/fs/cifs/inode.c
> > +++ b/fs/cifs/inode.c
> > @@ -137,7 +137,6 @@ cifs_fattr_to_inode(struct inode *inode, struct cifs_fattr *fattr)
> >                inode->i_mode = fattr->cf_mode;
> >
> >        cifs_i->cifsAttrs = fattr->cf_cifsattrs;
> > -       cifs_i->uniqueid = fattr->cf_uniqueid;
> >
> >        if (fattr->cf_flags & CIFS_FATTR_NEED_REVAL)
> >                cifs_i->time = 0;
> 
> Seems like there is a case where this is worse (imagine renaming a
> file, creating a new
> file with the old path)
> - We have an inode with particular values (size, attributes etc.) that
> are newly updated
> - We leave the old inode number
> 
> So we have a new different file with the same name but the old inode
> number, with
> the new file's attributes which seems confusing
> to me.   Perhaps in this case it is safer to mark the old inode stale
> and unhash it
> (similar to your other patch)
> 
> 

Good point. Let me think on this a bit. We clearly have a bug here, but
you may be right that it's not terribly harmful. I think we have some
work to do in this area however.