From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-f195.google.com ([209.85.160.195]:45899 "EHLO mail-qt1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727141AbeJQHbU (ORCPT ); Wed, 17 Oct 2018 03:31:20 -0400 Received: by mail-qt1-f195.google.com with SMTP id e10-v6so27946493qtq.12 for ; Tue, 16 Oct 2018 16:38:35 -0700 (PDT) Date: Tue, 16 Oct 2018 20:38:29 -0300 From: Ernesto =?utf-8?Q?A=2E_Fern=C3=A1ndez?= To: Viacheslav Dubeyko Cc: linux-fsdevel@vger.kernel.org, Andrew Morton Subject: Re: [PATCH] hfsplus: return file attributes on statx Message-ID: <20181016233829.vmhlxqebaygzghmh@eaf> References: <20181014163558.sxorxlzjqccq2lpw@eaf> <1539649583.3133.8.camel@slavad-ubuntu-14.04> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1539649583.3133.8.camel@slavad-ubuntu-14.04> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon, Oct 15, 2018 at 05:26:23PM -0700, Viacheslav Dubeyko wrote: > On Sun, 2018-10-14 at 13:35 -0300, Ernesto A. Fernández wrote: > > The immutable, append-only and no-dump attributes can only be retrieved > > with an ioctl; implement the ->getattr() method to return them on statx. > > Do not return the inode birthtime yet, because the issue of how best to > > handle the post-2038 timestamps is still under discussion. > > > > As far as I can see, the stable branch doesn't contain the inode > birthtime yet. So, I believe we have no troubles with it. What stable branch? What are you talking about? Of course the inode birthtime is in the code, how could it not be? > > > This patch is needed to pass xfstests generic/424. > > Do you mean that the patch isn't been tested yet? Do it needs to wait > the testing result report before taking the patch? Otherwise, it looks > weird to have such remark in the comment section of the patch. Look, I'm not a native speaker either, but I think that's a pretty simple sentence. You need this patch if you want xfstests generic/424 to pass. > > > > > Signed-off-by: Ernesto A. Fernández > > --- > > fs/hfsplus/dir.c | 1 + > > fs/hfsplus/hfsplus_fs.h | 2 ++ > > fs/hfsplus/inode.c | 21 +++++++++++++++++++++ > > 3 files changed, 24 insertions(+) > > > > diff --git a/fs/hfsplus/dir.c b/fs/hfsplus/dir.c > > index f37662675c3a..29a9dcfbe81f 100644 > > --- a/fs/hfsplus/dir.c > > +++ b/fs/hfsplus/dir.c > > @@ -565,6 +565,7 @@ const struct inode_operations hfsplus_dir_inode_operations = { > > .symlink = hfsplus_symlink, > > .mknod = hfsplus_mknod, > > .rename = hfsplus_rename, > > + .getattr = hfsplus_getattr, > > .listxattr = hfsplus_listxattr, > > }; > > > > diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h > > index dd7ad9f13e3a..b8471bf05def 100644 > > --- a/fs/hfsplus/hfsplus_fs.h > > +++ b/fs/hfsplus/hfsplus_fs.h > > @@ -488,6 +488,8 @@ void hfsplus_inode_write_fork(struct inode *inode, > > struct hfsplus_fork_raw *fork); > > int hfsplus_cat_read_inode(struct inode *inode, struct hfs_find_data *fd); > > int hfsplus_cat_write_inode(struct inode *inode); > > +int hfsplus_getattr(const struct path *path, struct kstat *stat, > > + u32 request_mask, unsigned int query_flags); > > int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end, > > int datasync); > > > > diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c > > index d7ab9d8c4b67..d131c8ea7eb6 100644 > > --- a/fs/hfsplus/inode.c > > +++ b/fs/hfsplus/inode.c > > @@ -270,6 +270,26 @@ static int hfsplus_setattr(struct dentry *dentry, struct iattr *attr) > > return 0; > > } > > > > +int hfsplus_getattr(const struct path *path, struct kstat *stat, > > + u32 request_mask, unsigned int query_flags) > > +{ > > + struct inode *inode = d_inode(path->dentry); > > + struct hfsplus_inode_info *hip = HFSPLUS_I(inode); > > + > > + if (inode->i_flags & S_APPEND) > > + stat->attributes |= STATX_ATTR_APPEND; > > + if (inode->i_flags & S_IMMUTABLE) > > + stat->attributes |= STATX_ATTR_IMMUTABLE; > > + if (hip->userflags & HFSPLUS_FLG_NODUMP) > > + stat->attributes |= STATX_ATTR_NODUMP; > > + > > + stat->attributes_mask |= STATX_ATTR_APPEND | STATX_ATTR_IMMUTABLE | > > + STATX_ATTR_NODUMP; > > + > > + generic_fillattr(inode, stat); > > + return 0; > > +} > > + > > int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end, > > int datasync) > > { > > @@ -329,6 +349,7 @@ int hfsplus_file_fsync(struct file *file, loff_t start, loff_t end, > > > > static const struct inode_operations hfsplus_file_inode_operations = { > > .setattr = hfsplus_setattr, > > + .getattr = hfsplus_getattr, > > .listxattr = hfsplus_listxattr, > > }; > > > > The patch looks good. I don't see any issue with it. > > Reviewed-by: Vyacheslav Dubeyko > > Thanks, > Vyacheslav Dubeyko. > >