On Wed, Jan 25, 2017 at 01:28:21PM +0100, Andreas Gruenbacher wrote:
> Omar,
>
> On Wed, Jan 25, 2017 at 3:38 AM, Omar Sandoval <osan...@osandov.com> wrote:
> > From: Omar Sandoval <osan...@fb.com>
> >
> > When you snapshot a subvolume containing a subvolume, you get a
> > placeholder read-only directory where the subvolume would be. These
> > directory inodes have ->i_ops set to btrfs_dir_ro_inode_operations.
> > Previously, this didn't include the xattr operation callbacks. The
> > conversion to xattr_handlers missed this case, leading to bogus attempts
> > to set xattrs on these inodes. This manifested itself as failures when
> > running delayed inodes.
> >
> > To fix this, clear the IOP_XATTR in ->i_opflags on these inodes.
> >
> > Fixes: 6c6ef9f26e59 ("xattr: Stop calling {get,set,remove}xattr inode
> > operations")
> > Cc: Andreas Gruenbacher <agrue...@redhat.com>
> > Reported-by: Chris Murphy <li...@colorremedies.com>
> > Signed-off-by: Omar Sandoval <osan...@fb.com>
> > ---
> > Applies to v4.10-rc4. Chris, this fixes the issue for me, could you please
> > test
> > it out? Andreas, does this make sense? I'll try to cook up an xfstest for
> > this.
>
> this change looks good.
>
> Are those directories really read-only though? They have the S_IWUSR
> permission set, and an update_time iop.
Hm, so these inodes don't have an on-disk [mca]time, it's only in
memory. ->update_time() just updates the in-memory time stamp, which is
kind of weird. Not sure why these even have S_IWUSR; they don't have a
->create() iop either. 0555 really makes more sense here. I wonder if
anyone would care if we made that change...
> Also, the get_acl and set_acl iops seem dead: they were not called
> before because the xattr iops were not defined in
> btrfs_dir_ro_inode_operations, and they are not called now because
> IOP_XATTR is cleared. Could you please check that as well?
Yeah, those shouldn't be there, either. I'll get rid of them in v2.
On Wed, Jan 25, 2017 at 01:29:51PM +0100, Andreas Gruenbacher wrote:
> On Wed, Jan 25, 2017 at 3:38 AM, Omar Sandoval <osan...@osandov.com> wrote:
> > Forgot to cc stable, but 4.9 needs this.
>
> Huh, stable not CCed again?
I was hoping Dave would add it when he applied it, but since I'm going
to send out a v2, I'll just do it then.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
