2015-03-10 0:17 GMT+01:00 Goffredo Baroncelli <kreij...@libero.it>: > On 2015-03-08 15:00, Ronny Chevalier wrote: >> 2015-03-08 12:48 GMT+01:00 Goffredo Baroncelli <kreij...@libero.it>: >>> From: Goffredo Baroncelli <kreij...@inwind.it> >>> >> >> Hi, >> >>> Allow systemd-tmpfiles to set the file/directory attributes, like chattr(1) >>> does. Two more commands are added: 'H' and 'h' to set the attributes, >>> recursively and not. >>> >>> Signed-off-by: Goffredo Baroncelli <kreij...@inwind.it> >> >> No Signed-off-by in systemd. >> >>> --- >>> src/tmpfiles/tmpfiles.c | 155 >>> ++++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 155 insertions(+) >>> >>> diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c >>> index c948d4d..cb77047 100644 >>> --- a/src/tmpfiles/tmpfiles.c >>> +++ b/src/tmpfiles/tmpfiles.c >>> @@ -40,6 +40,7 @@ >>> #include <sys/types.h> >>> #include <sys/param.h> >>> #include <sys/xattr.h> >>> +#include <linux/fs.h> >>> >>> #include "log.h" >>> #include "util.h" >>> @@ -90,6 +91,8 @@ typedef enum ItemType { >>> ADJUST_MODE = 'm', /* legacy, 'z' is identical to this */ >>> RELABEL_PATH = 'z', >>> RECURSIVE_RELABEL_PATH = 'Z', >>> + SET_ATTRIB = 'h', >>> + RECURSIVE_SET_ATTRIB = 'H', >>> } ItemType; >>> >>> typedef struct Item { >>> @@ -108,12 +111,15 @@ typedef struct Item { >>> usec_t age; >>> >>> dev_t major_minor; >>> + int attrib_value; >>> + int attrib_mask; >>> >>> bool uid_set:1; >>> bool gid_set:1; >>> bool mode_set:1; >>> bool age_set:1; >>> bool mask_perms:1; >>> + bool attrib_set:1; >>> >>> bool keep_first_level:1; >>> >>> @@ -762,6 +768,130 @@ static int path_set_acls(Item *item, const char >>> *path) { >>> return 0; >>> } >>> >>> +static int get_attrib_from_arg(Item *item) { >>> + struct attributes_list_t { int value; char ch; } ; >>> + static struct attributes_list_t attributes[] = { >>> + { FS_NOATIME_FL, 'A' }, /* do not update atime */ >>> + { FS_SYNC_FL, 'S' }, /* Synchronous updates */ >>> + { FS_DIRSYNC_FL, 'D' }, /* dirsync behaviour >>> (directories only) */ >>> + { FS_APPEND_FL, 'a' }, /* writes to file may >>> only append */ >>> + { FS_COMPR_FL, 'c' }, /* Compress file */ >>> + { FS_NODUMP_FL, 'd' }, /* do not dump file */ >>> + { FS_EXTENT_FL, 'e'}, /* Top of directory >>> hierarchies*/ >>> + { FS_IMMUTABLE_FL, 'i' }, /* Immutable file */ >>> + { FS_JOURNAL_DATA_FL, 'j' }, /* Reserved for ext3 >>> */ >>> + { FS_SECRM_FL, 's' }, /* Secure deletion */ >>> + { FS_UNRM_FL, 'u' }, /* Undelete */ >>> + { FS_NOTAIL_FL, 't' }, /* file tail should not >>> be merged */ >>> + { FS_TOPDIR_FL, 'T' }, /* Top of directory >>> hierarchies*/ >>> + { FS_NOCOW_FL, 'C' }, /* Do not cow file */ >>> + { 0, 0 } >>> + }; >>> + char *p = item->argument; >>> + enum { MODE_ADD, MODE_DEL, MODE_SET } mode = MODE_ADD; >>> + int value=0, mask=0; >>> + >>> + if (!p) { >>> + log_error("\"%s\": setting ATTR need an argument", >>> item->path); >>> + return -1; >> >> Please use errno style error code. In this case -EINVAL is appropriate. > ok >> >>> + } >>> + >>> + if (*p == '+') { >>> + mode = MODE_ADD; >>> + p++; >>> + } else if (*p == '-') { >>> + mode = MODE_DEL; >>> + p++; >>> + } else if (*p == '=') { >>> + mode = MODE_SET; >>> + p++; >>> + } >>> + >>> + if (!*p && mode != MODE_SET) { >>> + log_error("\"%s\": setting ATTR: argument is empty", >>> item->path); >>> + return -4; >> >> Same here. > ok >> >>> + } >>> + for ( ; *p ; p++ ) { >>> + int i; >>> + for ( i = 0; attributes[i].ch && attributes[i].ch != *p >>> ;i++); >>> + if (!attributes[i].ch) { >>> + log_error("\"%s\": setting ATTR: unknown attr >>> '%c'", >>> + item->path, *p); >>> + return -2; >> >> Same here. > ok >> >>> + } >>> + if (mode == MODE_ADD || mode == MODE_SET) >>> + value |= attributes[i].value; >>> + else >>> + value &= ~attributes[i].value; >>> + mask |= attributes[i].value; >>> + } >>> + >>> + if (mode == MODE_SET) { >>> + int i; >>> + for ( i = 0; attributes[i].ch;i++) >>> + mask |= attributes[i].value; >>> + } >>> + >>> + assert(mask); >>> + >>> + item->attrib_mask |= mask; >>> + item->attrib_value &= ~mask; >>> + item->attrib_value |= value; >>> + item->attrib_set = true; >>> + >>> + >> >> Useless newline. > ok >> >>> + return 0; >>> + >>> +} >>> + >>> +static int path_set_attrib(Item *item, const char *path) { >>> + int fd, r, f; >>> + struct stat st; >>> + >>> + /* do nothing */ >>> + if (item->attrib_mask == 0 || !item->attrib_set) >>> + return 0; >>> + >>> + if (!lstat(path, &st) && >>> + !S_ISREG(st.st_mode) && !S_ISDIR(st.st_mode)) { >>> + return 0; >>> + } >>> +#ifdef O_LARGEFILE >>> + fd = open (path, O_RDONLY|O_NONBLOCK|O_LARGEFILE); >>> +#else >>> + fd = open (path, O_RDONLY|O_NONBLOCK); >> >> You should add a O_CLOEXEC. > Why ? Am not against your suggestion, but I am not able the understand the > reason: this code doesn't fork, > so O_CLOEXEC is not useful.
Sure, just an habit in the CODING_STYLE to ensure that no fd leaks. > >> >>> +#endif >>> + if (fd == -1) { >>> + log_error_errno(errno, "Cannot open \"%s\": %m", path); >>> + return -1; >> >> return log_error_errno(...); >> >> It returns the errno code provided. > ok >> >>> + } >>> + r = ioctl (fd, FS_IOC_GETFLAGS, &f); >>> + if (r<0) { >>> + int save_errno = errno; >> >> There is a macro for this: PROTECT_ERRNO > what nice macro ! >> >>> + log_error_errno(errno, "Cannot get attrib for \"%s\": %m", >>> path); >>> + close (fd); >>> + errno = save_errno; >>> + return r; >>> + } >>> + >>> + f &= ~item->attrib_mask; >>> + f |= (item->attrib_value & item->attrib_mask); >>> + if (!S_ISDIR(st.st_mode)) >>> + f &= ~FS_DIRSYNC_FL; >>> + r = ioctl (fd, FS_IOC_SETFLAGS, &f); >>> + if (r < 0) { >>> + int save_errno = errno; >> >> Same here, PROTECT_ERRNO > ok >> >>> + log_error_errno(errno, >>> + "Cannot set attrib for \"%s\", value=0x%08x, >>> mask=0x%08x: %m", >>> + path, item->attrib_value, item->attrib_mask); >>> + close (fd); >>> + errno = save_errno; >>> + return r; >>> + } >>> + close (fd); >> >> You can use _cleanup_close_ for this fd. > Unfortunately no; I exit from the function before opening the fd... _cleanup_close_ int fd = -1; The close will be a nop. >> >> Also I am not sure but everywhere in the code all function call are >> without a space before the parenthese. > I correct it >> >>> + return 0; >>> +} >>> + >>> static int write_one_file(Item *i, const char *path) { >>> _cleanup_close_ int fd = -1; >>> int flags, r = 0; >>> @@ -1203,6 +1333,18 @@ static int create_item(Item *i) { >>> if (r < 0) >>> return r; >>> break; >>> + >>> + case SET_ATTRIB: >>> + r = glob_item(i, path_set_attrib, false); >>> + if (r < 0) >>> + return r; >>> + break; >>> + >>> + case RECURSIVE_SET_ATTRIB: >>> + r = glob_item(i, path_set_attrib, true); >>> + if (r < 0) >>> + return r; >>> + break; >>> } >>> >>> log_debug("%s created successfully.", i->path); >>> @@ -1269,6 +1411,8 @@ static int remove_item(Item *i) { >>> case RECURSIVE_SET_XATTR: >>> case SET_ACL: >>> case RECURSIVE_SET_ACL: >>> + case SET_ATTRIB: >>> + case RECURSIVE_SET_ATTRIB: >>> break; >>> >>> case REMOVE_PATH: >>> @@ -1653,6 +1797,17 @@ static int parse_line(const char *fname, unsigned >>> line, const char *buffer) { >>> return r; >>> break; >>> >>> + case SET_ATTRIB: >>> + case RECURSIVE_SET_ATTRIB: >>> + if (!i.argument) { >>> + log_error("[%s:%u] Set attrib requires argument.", >>> fname, line); >>> + return -EBADMSG; >>> + } >>> + r = get_attrib_from_arg(&i); >>> + if (r < 0) >>> + return r; >>> + break; >>> + >>> default: >>> log_error("[%s:%u] Unknown command type '%c'.", fname, >>> line, (char) i.type); >>> return -EBADMSG; >>> -- >>> 2.1.4 >>> >>> _______________________________________________ >>> systemd-devel mailing list >>> systemd-devel@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/systemd-devel >> > > > -- > gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it> > Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5 _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel