Hello Thomas,
Am 09.03.2017 um 18:09 schrieb Thomas Lamprecht: > On 03/09/2017 05:26 PM, Stefan Priebe wrote: >> This allows us to use management software for files inside of /etc/pve. >> f.e. saltstack which rely on being able to set uid,gid and chmod >> >> Signed-off-by: Stefan Priebe <[email protected]> >> --- >> data/src/pmxcfs.c | 41 ++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 40 insertions(+), 1 deletion(-) >> >> diff --git a/data/src/pmxcfs.c b/data/src/pmxcfs.c >> index 1b6cbcc..aa81808 100644 >> --- a/data/src/pmxcfs.c >> +++ b/data/src/pmxcfs.c >> @@ -186,6 +186,43 @@ ret: >> return ret; >> } >> +static int cfs_fuse_chmod(const char *path, mode_t mode) >> +{ >> + const mode_t pve_mode = S_IRUSR | S_IWUSR | S_IRGRP; >> + int mode_i = mode & (S_IRWXU | S_IRWXG | S_IRWXO); >> + int pve_mode_i = pve_mode & (S_IRWXU | S_IRWXG | S_IRWXO); >> + > why not directly setting the pve_mode_i variable to > > (S_IRUSR | S_IWUSR | S_IRGRP); > > The expression (S_IRWXU | S_IRWXG | S_IRWXO) equals 0777, so doing a > binary and to pve_mode does not change anything, or am I mistaken? *urg* was a leftofter from trying different variations ;-) Sorry. >> + cfs_debug("enter cfs_fuse_mode %s", path); > did you mean: > enter cfs_fuse_chmod Yes, fixed for next patch. >> + int ret = -ENOSYS; >> + > EACCES would be more fitting here? Sure i was just trying to keep it backward compatible. So currrently you get not implemented so i kept it like that. Changed to EACCES for V2. > > man 2 chmod > > does not specifies ENOSYS, and implementing it and returning ENOSYS (aka > not implemented) seems strange to me? Right but it's the current behaviour of pmxcfs. Changed to EACCES anyway for V2. >> + if (pve_mode_i == mode_i) { >> + ret = 0; >> + goto ret; >> + } >> + >> + ret: >> + cfs_debug("leave cfs_fuse_mode %s (%d) mode: %o pve_mode: %o", >> path, ret, mode_i, pve_mode_i); >> + > > it looks like you mix intend styles here (above spaces and below tabs > for same intend level), > please avoid that Fixed for V2. > >> + return ret; >> +} > > While you adapt the C style we use in other calls, i.e. the ret: label > and gotos, > it over complicates stuff here as no cleanup must be made Yes the idea was just to keep the style for each function. > > Here a minimal implementation (totally untested™) > -- > static int cfs_fuse_chmod(const char *path, mode_t mode) > { > cfs_debug("enter cfs_fuse_chmod %s", path); > int ret = -EACCES; > > // asserts 0640, but allows setting UID and GID - some programs need > that > if (mode & ACCESSPERMS == (S_IRUSR | S_IWUSR | S_IRGRP)) > { > ret = 0; > } > cfs_debug("leave cfs_fuse_chmod %s (%d) mode: %o pve_mode: %o", > path, ret, mode_i, pve_mode_i); > > return ret; > } > -- > > >> + >> +static int cfs_fuse_chown(const char *path, uid_t user, gid_t group) >> +{ >> + cfs_debug("enter cfs_fuse_chown %s", path); >> + >> + int ret = -ENOSYS; > Also EACCES here? > > same here (above tabs and below spaces for same intend level), > > rest looks ok. >> + >> + if (user == 0 && group == cfs.gid) { >> + ret = 0; >> + goto ret; >> + } >> + >> + ret: >> + cfs_debug("leave cfs_fuse_chown %s (%d)", path, ret); >> + >> + return ret; >> +} >> + >> static int cfs_fuse_mkdir(const char *path, mode_t mode) >> { >> cfs_debug("enter cfs_fuse_mkdir %s", path); >> @@ -488,7 +525,9 @@ static struct fuse_operations fuse_ops = { >> .readlink = cfs_fuse_readlink, >> .utimens = cfs_fuse_utimens, >> .statfs = cfs_fuse_statfs, >> - .init = cfs_fuse_init >> + .init = cfs_fuse_init, >> + .chown = cfs_fuse_chown, >> + .chmod = cfs_fuse_chmod >> }; >> static char * > > > > _______________________________________________ > pve-devel mailing list > [email protected] > http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel _______________________________________________ pve-devel mailing list [email protected] http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
