Re: [ceph-users] cephfs snap mkdir strange timestamp

Re: [ceph-users] cephfs snap mkdir strange timestamp

[Luis Henriques]

[ CC'ing Jeff and ceph-devel@ ]

On Tue, Mar 10, 2020 at 11:41:19AM +0100, Marc Roos wrote:
>  
> 
> If I make a directory in linux the directory has the date of now, why is 
> this not with creating a snap dir? Is this not a bug? One expects this 
> to be the same as in linux not????

I've noticed that long time ago, but I never checked the fuse client
behaviour, which turns out to be different.  The fuse client seems to set
ctime and atime to the same value as the parent directory (see
Client::open_snapdir()).

The patch below mimics that behaviour, by simply copying those timestamps
from the parent inode.

Cheers,
--
Luis

From 2c8e06e66e5453ddf8b634cf1689a812dc05a0c6 Mon Sep 17 00:00:00 2001
From: Luis Henriques <lhenriques@suse.com>
Date: Tue, 10 Mar 2020 13:35:11 +0000
Subject: [PATCH] ceph: fix snapshot dir ctime and mtime

The .snap directory timestamps are kept at 0 (1970-01-01 00:00), which
isn't consistent with what the fuse client does.  This patch makes the
behaviour consistent, by setting these timestamps to those of the parent
directory.

Signed-off-by: Luis Henriques <lhenriques@suse.com>
---
 fs/ceph/inode.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
index d01710a16a4a..f4e78ade0871 100644
--- a/fs/ceph/inode.c
+++ b/fs/ceph/inode.c
@@ -82,6 +82,8 @@ struct inode *ceph_get_snapdir(struct inode *parent)
 	inode->i_mode = parent->i_mode;
 	inode->i_uid = parent->i_uid;
 	inode->i_gid = parent->i_gid;
+	inode->i_mtime = parent->i_mtime;
+	inode->i_ctime = parent->i_ctime;
 	inode->i_op = &ceph_snapdir_iops;
 	inode->i_fop = &ceph_snapdir_fops;
 	ci->i_snap_caps = CEPH_CAP_PIN; /* so we can open */

Re: [ceph-users] cephfs snap mkdir strange timestamp

[Jeff Layton]

On Tue, 2020-03-10 at 13:46 +0000, Luis Henriques wrote:
> [ CC'ing Jeff and ceph-devel@ ]
> 
> On Tue, Mar 10, 2020 at 11:41:19AM +0100, Marc Roos wrote:
> >  
> > 
> > If I make a directory in linux the directory has the date of now, why is 
> > this not with creating a snap dir? Is this not a bug? One expects this 
> > to be the same as in linux not????
> 
> I've noticed that long time ago, but I never checked the fuse client
> behaviour, which turns out to be different.  The fuse client seems to set
> ctime and atime to the same value as the parent directory (see
> Client::open_snapdir()).
> 
> The patch below mimics that behaviour, by simply copying those timestamps
> from the parent inode.
> 
> Cheers,
> --
> Luis
> 
> From 2c8e06e66e5453ddf8b634cf1689a812dc05a0c6 Mon Sep 17 00:00:00 2001
> From: Luis Henriques <lhenriques@suse.com>
> Date: Tue, 10 Mar 2020 13:35:11 +0000
> Subject: [PATCH] ceph: fix snapshot dir ctime and mtime
> 
> The .snap directory timestamps are kept at 0 (1970-01-01 00:00), which
> isn't consistent with what the fuse client does.  This patch makes the
> behaviour consistent, by setting these timestamps to those of the parent
> directory.
> 
> Signed-off-by: Luis Henriques <lhenriques@suse.com>
> ---
>  fs/ceph/inode.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index d01710a16a4a..f4e78ade0871 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -82,6 +82,8 @@ struct inode *ceph_get_snapdir(struct inode *parent)
>  	inode->i_mode = parent->i_mode;
>  	inode->i_uid = parent->i_uid;
>  	inode->i_gid = parent->i_gid;
> +	inode->i_mtime = parent->i_mtime;
> +	inode->i_ctime = parent->i_ctime;
>  	inode->i_op = &ceph_snapdir_iops;
>  	inode->i_fop = &ceph_snapdir_fops;
>  	ci->i_snap_caps = CEPH_CAP_PIN; /* so we can open */

What about the atime, and the ci->i_btime ?
-- 
Jeff Layton <jlayton@kernel.org>

Re: [ceph-users] cephfs snap mkdir strange timestamp

[Luis Henriques]

On Tue, Mar 10, 2020 at 01:39:29PM -0400, Jeff Layton wrote:
...
> > Signed-off-by: Luis Henriques <lhenriques@suse.com>
> > ---
> >  fs/ceph/inode.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> > index d01710a16a4a..f4e78ade0871 100644
> > --- a/fs/ceph/inode.c
> > +++ b/fs/ceph/inode.c
> > @@ -82,6 +82,8 @@ struct inode *ceph_get_snapdir(struct inode *parent)
> >  	inode->i_mode = parent->i_mode;
> >  	inode->i_uid = parent->i_uid;
> >  	inode->i_gid = parent->i_gid;
> > +	inode->i_mtime = parent->i_mtime;
> > +	inode->i_ctime = parent->i_ctime;
> >  	inode->i_op = &ceph_snapdir_iops;
> >  	inode->i_fop = &ceph_snapdir_fops;
> >  	ci->i_snap_caps = CEPH_CAP_PIN; /* so we can open */
> 
> What about the atime, and the ci->i_btime ?

Yeah, probably makes sense too, although the fuse client doesn't seem to
touch atime (it does change btime, I missed that).  I'll send v2 in a bit.

Cheers,
--
Luis

RE: [ceph-users] cephfs snap mkdir strange timestamp

[Marc Roos]

Sorry I am not really developer nor now details about POSIX/fs's. But if 
I may ask, why are you getting this time from the parent?



-----Original Message-----
Sent: 11 March 2020 11:32
To: Jeff Layton
Cc: Marc Roos; ceph-users; ceph-devel@vger.kernel.org
Subject: Re: [ceph-users] cephfs snap mkdir strange timestamp

On Tue, Mar 10, 2020 at 01:39:29PM -0400, Jeff Layton wrote:
...
> > Signed-off-by: Luis Henriques <lhenriques@suse.com>
> > ---
> >  fs/ceph/inode.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c index 
> > d01710a16a4a..f4e78ade0871 100644
> > --- a/fs/ceph/inode.c
> > +++ b/fs/ceph/inode.c
> > @@ -82,6 +82,8 @@ struct inode *ceph_get_snapdir(struct inode 
*parent)
> >  	inode->i_mode = parent->i_mode;
> >  	inode->i_uid = parent->i_uid;
> >  	inode->i_gid = parent->i_gid;
> > +	inode->i_mtime = parent->i_mtime;
> > +	inode->i_ctime = parent->i_ctime;
> >  	inode->i_op = &ceph_snapdir_iops;
> >  	inode->i_fop = &ceph_snapdir_fops;
> >  	ci->i_snap_caps = CEPH_CAP_PIN; /* so we can open */
> 
> What about the atime, and the ci->i_btime ?

Yeah, probably makes sense too, although the fuse client doesn't seem to 
touch atime (it does change btime, I missed that).  I'll send v2 in a 
bit.

Cheers,
--
Luis

Re: [ceph-users] cephfs snap mkdir strange timestamp

[Luis Henriques]

On Wed, Mar 11, 2020 at 11:36:56AM +0100, Marc Roos wrote:
> 
> Sorry I am not really developer nor now details about POSIX/fs's. But if 
> I may ask, why are you getting this time from the parent?

The hidden .snap directory is ceph-specific, and not really POSIX
compliant.  It's handled in a special way and doesn't really have all the
typical directory attributes (such as timestamps).  Hence, the fuse client
is getting these attributes from the parent directory instead of showing
the funny 1970-01-01 (epoch) date.

Cheers,
--
Luis

> 
> 
> 
> -----Original Message-----
> Sent: 11 March 2020 11:32
> To: Jeff Layton
> Cc: Marc Roos; ceph-users; ceph-devel@vger.kernel.org
> Subject: Re: [ceph-users] cephfs snap mkdir strange timestamp
> 
> On Tue, Mar 10, 2020 at 01:39:29PM -0400, Jeff Layton wrote:
> ...
> > > Signed-off-by: Luis Henriques <lhenriques@suse.com>
> > > ---
> > >  fs/ceph/inode.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c index 
> > > d01710a16a4a..f4e78ade0871 100644
> > > --- a/fs/ceph/inode.c
> > > +++ b/fs/ceph/inode.c
> > > @@ -82,6 +82,8 @@ struct inode *ceph_get_snapdir(struct inode 
> *parent)
> > >  	inode->i_mode = parent->i_mode;
> > >  	inode->i_uid = parent->i_uid;
> > >  	inode->i_gid = parent->i_gid;
> > > +	inode->i_mtime = parent->i_mtime;
> > > +	inode->i_ctime = parent->i_ctime;
> > >  	inode->i_op = &ceph_snapdir_iops;
> > >  	inode->i_fop = &ceph_snapdir_fops;
> > >  	ci->i_snap_caps = CEPH_CAP_PIN; /* so we can open */
> > 
> > What about the atime, and the ci->i_btime ?
> 
> Yeah, probably makes sense too, although the fuse client doesn't seem to 
> touch atime (it does change btime, I missed that).  I'll send v2 in a 
> bit.
> 
> Cheers,
> --
> Luis
> 
> 

Re: [ceph-users] cephfs snap mkdir strange timestamp

[Luis Henriques]

On Tue, Mar 10, 2020 at 01:39:29PM -0400, Jeff Layton wrote:
...
> What about the atime, and the ci->i_btime ?

FYI, I've also created PR#33879 to also get the atime in the fuse client.

Cheers,
--
Luis