Seth Forshee <seth.forshee@xxxxxxxxxxxxx> writes: > On Wed, Jun 29, 2016 at 03:18:24PM -0500, Eric W. Biederman wrote: >> "Michael j Theall" <mtheall@xxxxxxxxxx> writes: >> >> > Going by the patch I posted a couple of years ago: >> > https://sourceforge.net/p/fuse/mailman/message/33033653/ >> > >> > The only hole I see in your patch is that in setattr() you are not >> > updating the cached acl if the ATTR_MODE is updated. The other major >> > difference is that my version uses the get_acl/set_acl inode >> > operations but you use that plus the xattr handlers. I'm not >> > up-to-speed on the kernel so I'm not sure if you actually need to >> > implement both. >> >> That makes an interesting question. Is it desirable to keep >> inode->i_mode in sync with the posix acls in fuse or should a filesystem >> that supports posix acls worry about that? > > My first blush opinion is that the kernel should take care of this, not > the filesystems. Then a fuse filesystem which supports xattrs gets acl > support for free. Otherwise if a filesystem supports xattrs but not acls > internally, we have no way of knowing that in the kernel and they get > out of sync. > > However if some filesystems are already doing this internally then we > have redundancy. Presumably this would be harmless aside from the wasted > effort. Which means that in set_acl we need to something like: if (type == ACL_TYPE_ACCESS) { struct iattr attr; attr.ia_valid = ATTR_MODE; attr.ia_mode = inode->i_mode; ret = posix_acl_equiv_mode(acl, &attr.ia_mode); if (ret < 0) return ret; if (ret == 0) acl = NULL; if (attr.ia_mode != inode->i_mode) { ret = fuse_do_setattr(inode, &attr, NULL); if (ret < 0) return ret; } } In fuse_setattr should wind up looking something like: static int fuse_setattr(struct dentry *entry, struct iattr *attr) { struct inode *inode = d_inode(entry); int ret; if (!fuse_allow_current_process(get_fuse_conn(inode))) return -EACCES; if (attr->ia_valid & ATTR_FILE) ret = fuse_do_setattr(inode, attr, attr->ia_file); else ret = fuse_do_setattr(inode, attr, NULL); if (ret == 0 && attr->ia_valid & ATTR_MODE) ret = posix_acl_chmod(inode, inode->i_mode); return ret; } That should be enough to keep everything in sync with the existing fuse protocol. And then fuse filesystems won't have to care in general about the contents of acls (unless they choose to care). Eric -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
