On Sat, Jul 29, 2017 at 12:48:04AM +0000, Josef Bacik wrote: > On Fri, Jul 28, 2017 at 09:26:29PM -0300, Ernesto A. Fernández wrote: > > + ret = __btrfs_set_acl(trans, inode, acl, type); > > + if (ret) > > + goto out; > > + > > + inode->i_mode = mode; > > + inode_inc_iversion(inode); > > + inode->i_ctime = current_time(inode); > > + set_bit(BTRFS_INODE_COPY_EVERYTHING, &BTRFS_I(inode)->runtime_flags); > > This only needs to be set if we actually set the xattr. I'd fix setxattr to > call it every time it's called.
I had not thought of that, thank you. If I'm understanding this correctly the issue would be only when setting a NULL default acl on an inode that is not a directory. In that case I probably shouldn't be calling btrfs_update_inode either, but I can't move that back to setxattr. Perhaps __btrfs_set_acl could return an error in that case, like -ENOTDIR, and then we can set ret back to 0 before returning from btrfs_set_acl. > > + ret = btrfs_update_inode(trans, root, inode); > > + BUG_ON(ret); > > No BUG_ON, return the error. The call to BUG_ON was already there before my patch, only inside the __btrfs_setxattr function. Since I didn't know the reason I thought it was best not to change it. I'll do as you say in the next version. Thank you for your review. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
