Discussion:
[PATCH V3] Allow systemd-tmpfiles to set file/directory attributes
(too old to reply)
Goffredo Baroncelli
2015-03-10 20:07:39 UTC
Permalink
Hi all,
This set of patches add two new line types to the tmpfiles files format.
These new types of line are 'h' and 'H' (the recursively version), and
allow to change the file/directory attributes, like chattr(1) does.

One of the motivation of these patches is to get rid of the commit
11689d2a which force the NOCOW flag for the journal files. This was
needed because systemd-journald has very poor performance when the
filesytem is BTRFS due to its the COW behavior. My concern is that
the NOCOW flag also prevent BTRFS to rebuild a corrupted file from a
good copy if it is available.

With this patch, now the NOCOW flag can be set by systemd-tmpfiles.
See [1] for further information.

BR
G.Baroncelli

Changelog:
v1: first issue
v2: accepted several suggestion on the style; added function
change_attr_fd(); used the _cleanup_close_; returned
negative errno;
v3: swapped arguments of change_attr_fd() in path_set_attrib()


[1] Re: [systemd-devel] [RFC][PATCH] Add option to enable COW for journal file
https://www.mail-archive.com/systemd-***@lists.freedesktop.org/msg28724.html

--
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
Goffredo Baroncelli
2015-03-10 20:07:40 UTC
Permalink
Add change_attr_fd() function to modify the file/directory attribute.
---
src/shared/util.c | 22 ++++++++++++++++++++++
src/shared/util.h | 1 +
2 files changed, 23 insertions(+)

diff --git a/src/shared/util.c b/src/shared/util.c
index ba035ca..56097ec 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -7832,6 +7832,28 @@ int chattr_path(const char *p, bool b, unsigned mask) {
return chattr_fd(fd, b, mask);
}

+int change_attr_fd(int fd, unsigned value, unsigned mask) {
+ unsigned old_attr, new_attr;
+
+ assert(fd >= 0);
+
+ if (mask == 0)
+ return 0;
+
+ if (ioctl(fd, FS_IOC_GETFLAGS, &old_attr) < 0)
+ return -errno;
+
+ new_attr = (old_attr & ~mask) |(value & mask);
+
+ if (new_attr == old_attr)
+ return 0;
+
+ if (ioctl(fd, FS_IOC_SETFLAGS, &new_attr) < 0)
+ return -errno;
+
+ return 0;
+}
+
int read_attr_fd(int fd, unsigned *ret) {
assert(fd >= 0);

diff --git a/src/shared/util.h b/src/shared/util.h
index a83b588..a0bc883 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -1052,6 +1052,7 @@ int same_fd(int a, int b);

int chattr_fd(int fd, bool b, unsigned mask);
int chattr_path(const char *p, bool b, unsigned mask);
+int change_attr_fd(int fd, unsigned value, unsigned mask);

int read_attr_fd(int fd, unsigned *ret);
int read_attr_path(const char *p, unsigned *ret);
--
2.1.4
Goffredo Baroncelli
2015-03-10 20:07:43 UTC
Permalink
Add a new tmpfiles.d snippets to set the NOCOW attributes for the
journal files. This allow better perfomance when the root file system
is BTRFS. Pay attention that the NOCOW flags disables the checksum and
prevent scrub to rebuild a corrupted journal.
---
tmpfiles.d/journal-nocow.conf | 12 ++++++++++++
1 file changed, 12 insertions(+)
create mode 100644 tmpfiles.d/journal-nocow.conf

diff --git a/tmpfiles.d/journal-nocow.conf b/tmpfiles.d/journal-nocow.conf
new file mode 100644
index 0000000..43a4f2b
--- /dev/null
+++ b/tmpfiles.d/journal-nocow.conf
@@ -0,0 +1,12 @@
+# This file is part of systemd.
+#
+# systemd is free software; you can redistribute it and/or modify it
+# under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation; either version 2.1 of the License, or
+# (at your option) any later version.
+
+# See tmpfiles.d(5) for details
+
+
+# set the journal file as NOCOW; only valid for BTRFS filesystem
+h /var/log/journal/%m - - - - +C
--
2.1.4
Goffredo Baroncelli
2015-03-10 20:07:41 UTC
Permalink
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.
---
src/tmpfiles/tmpfiles.c | 140 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 140 insertions(+)

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index c948d4d..165605f 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,115 @@ static int path_set_acls(Item *item, const char *path) {
return 0;
}

+static int get_attrib_from_arg(Item *item) {
+ static const struct { int value; char ch; } 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;
+ unsigned long value = 0, mask = 0;
+
+ if (!p) {
+ log_error("\"%s\": setting ATTR need an argument", item->path);
+ return -EINVAL;
+ }
+
+ 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 -EINVAL;
+ }
+ 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 -EINVAL;
+ }
+ 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 = value;
+ item->attrib_set = true;
+
+ return 0;
+
+}
+
+static int path_set_attrib(Item *item, const char *path) {
+ _cleanup_close_ int fd = -1;
+ int r;
+ unsigned f;
+ struct stat st;
+
+ /* do nothing */
+ if (item->attrib_mask == 0 || !item->attrib_set)
+ return 0;
+
+ if (lstat(path, &st) == 0 &&
+ !S_ISREG(st.st_mode) && !S_ISDIR(st.st_mode)) {
+ return 0;
+ }
+
+ fd = open(path, O_RDONLY|O_NONBLOCK|O_CLOEXEC);
+
+ if (fd < 0)
+ return log_error_errno(errno, "Cannot open \"%s\": %m", path);
+
+ f = item->attrib_value & item->attrib_mask;
+ if (!S_ISDIR(st.st_mode))
+ f &= ~FS_DIRSYNC_FL;
+ r = change_attr_fd(fd, f, item->attrib_mask);
+ if (r < 0)
+ return log_error_errno(errno,
+ "Cannot set attrib for \"%s\", value=0x%08x, mask=0x%08x: %m",
+ path, item->attrib_value, item->attrib_mask);
+
+ return 0;
+}
+
static int write_one_file(Item *i, const char *path) {
_cleanup_close_ int fd = -1;
int flags, r = 0;
@@ -1203,6 +1318,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 +1396,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 +1782,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
Zbigniew Jędrzejewski-Szmek
2015-03-16 03:12:35 UTC
Permalink
Post by Goffredo Baroncelli
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.
---
src/tmpfiles/tmpfiles.c | 140 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 140 insertions(+)
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index c948d4d..165605f 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,115 @@ static int path_set_acls(Item *item, const char *path) {
return 0;
}
+static int get_attrib_from_arg(Item *item) {
+ static const struct { int value; char ch; } 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;
+ unsigned long value = 0, mask = 0;
+
+ if (!p) {
+ log_error("\"%s\": setting ATTR need an argument", item->path);
+ return -EINVAL;
+ }
+
+ 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 -EINVAL;
+ }
+ for (; *p ; p++) {
+ int i;
+ for (i = 0; attributes[i].ch && attributes[i].ch != *p; i++)
+ ;
Why not order the table the other way:

static const int attributes[] = {
[(uint8_t)'A'] = FS_NOATIME_FL, /* do not update atime */
...
};

if ((uint8_t)*p > ELEMENTSOF(attributes) || attributes[(uint8_t)*p] == 0)
log_error("\"%s\": setting ATTR: unknown attr '%c'", item->path, *p);
return -EINVAL;
}

Not looping is always nicer.
Post by Goffredo Baroncelli
+ if (!attributes[i].ch) {
+ log_error("\"%s\": setting ATTR: unknown attr '%c'", item->path, *p);
+ return -EINVAL;
+ }
+ 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;
This simply FS_NOATIME_FL|FS_SYNC_FL|... Even if the compiler can optimize
that away, it seems better to just write the OR expression in full rather
than loop.
Post by Goffredo Baroncelli
+ }
+
+ assert(mask);
+
+ item->attrib_mask = mask;
+ item->attrib_value = value;
+ item->attrib_set = true;
+
+ return 0;
+
+}
+
+static int path_set_attrib(Item *item, const char *path) {
+ _cleanup_close_ int fd = -1;
+ int r;
+ unsigned f;
+ struct stat st;
+
+ /* do nothing */
+ if (item->attrib_mask == 0 || !item->attrib_set)
+ return 0;
+
+ if (lstat(path, &st) == 0 &&
+ !S_ISREG(st.st_mode) && !S_ISDIR(st.st_mode)) {
+ return 0;
+ }
Is it OK to ignore lstat error? If yes, please add a comment why.

Also no braces aroudn single statemnets.
Post by Goffredo Baroncelli
+
+ fd = open(path, O_RDONLY|O_NONBLOCK|O_CLOEXEC);
+
+ if (fd < 0)
+ return log_error_errno(errno, "Cannot open \"%s\": %m", path);
+
+ f = item->attrib_value & item->attrib_mask;
+ if (!S_ISDIR(st.st_mode))
+ f &= ~FS_DIRSYNC_FL;
+ r = change_attr_fd(fd, f, item->attrib_mask);
+ if (r < 0)
+ return log_error_errno(errno,
+ "Cannot set attrib for \"%s\", value=0x%08x, mask=0x%08x: %m",
+ path, item->attrib_value, item->attrib_mask);
+
+ return 0;
+}
+
Zbyszek
Goffredo Baroncelli
2015-03-16 18:49:20 UTC
Permalink
Post by Zbigniew Jędrzejewski-Szmek
Post by Goffredo Baroncelli
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.
---
src/tmpfiles/tmpfiles.c | 140 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 140 insertions(+)
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index c948d4d..165605f 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,115 @@ static int path_set_acls(Item *item, const char *path) {
return 0;
}
+static int get_attrib_from_arg(Item *item) {
+ static const struct { int value; char ch; } 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;
+ unsigned long value = 0, mask = 0;
+
+ if (!p) {
+ log_error("\"%s\": setting ATTR need an argument", item->path);
+ return -EINVAL;
+ }
+
+ 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 -EINVAL;
+ }
+ for (; *p ; p++) {
+ int i;
+ for (i = 0; attributes[i].ch && attributes[i].ch != *p; i++)
+ ;
static const int attributes[] = {
[(uint8_t)'A'] = FS_NOATIME_FL, /* do not update atime */
...
};
if ((uint8_t)*p > ELEMENTSOF(attributes) || attributes[(uint8_t)*p] == 0)
log_error("\"%s\": setting ATTR: unknown attr '%c'", item->path, *p);
return -EINVAL;
}
Not looping is always nicer.
I find this idea very elegant, my only concern was the memory occupation: the ascii code
for the 't' letter is about 120, this means that the array will have a size of ~240 bytes...
Ok that the PC have several GBs....
Post by Zbigniew Jędrzejewski-Szmek
Post by Goffredo Baroncelli
+ if (!attributes[i].ch) {
+ log_error("\"%s\": setting ATTR: unknown attr '%c'", item->path, *p);
+ return -EINVAL;
+ }
+ 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;
This simply FS_NOATIME_FL|FS_SYNC_FL|... Even if the compiler can optimize
that away, it seems better to just write the OR expression in full rather
than loop.
The point is to remember to update the mask... Anyway I will
add a constant with all the flag OR-ed near the flag definitions.
Post by Zbigniew Jędrzejewski-Szmek
Post by Goffredo Baroncelli
+ }
+
+ assert(mask);
+
+ item->attrib_mask = mask;
+ item->attrib_value = value;
+ item->attrib_set = true;
+
+ return 0;
+
+}
+
+static int path_set_attrib(Item *item, const char *path) {
+ _cleanup_close_ int fd = -1;
+ int r;
+ unsigned f;
+ struct stat st;
+
+ /* do nothing */
+ if (item->attrib_mask == 0 || !item->attrib_set)
+ return 0;
+
+ if (lstat(path, &st) == 0 &&
+ !S_ISREG(st.st_mode) && !S_ISDIR(st.st_mode)) {
+ return 0;
+ }
Is it OK to ignore lstat error? If yes, please add a comment why.
Yes, it is ok, because if "path" is source of an error, the open() below
will report it. I will add a comment.
Post by Zbigniew Jędrzejewski-Szmek
Also no braces aroudn single statemnets.
ok
Post by Zbigniew Jędrzejewski-Szmek
Post by Goffredo Baroncelli
+
+ fd = open(path, O_RDONLY|O_NONBLOCK|O_CLOEXEC);
+
+ if (fd < 0)
+ return log_error_errno(errno, "Cannot open \"%s\": %m", path);
+
+ f = item->attrib_value & item->attrib_mask;
+ if (!S_ISDIR(st.st_mode))
+ f &= ~FS_DIRSYNC_FL;
+ r = change_attr_fd(fd, f, item->attrib_mask);
+ if (r < 0)
+ return log_error_errno(errno,
+ "Cannot set attrib for \"%s\", value=0x%08x, mask=0x%08x: %m",
+ path, item->attrib_value, item->attrib_mask);
+
+ return 0;
+}
+
Zbyszek
Thank for the review
--
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5
Goffredo Baroncelli
2015-03-10 20:07:42 UTC
Permalink
Update the man page of tmpfiles.d(5), to document the new h/H command.
---
man/tmpfiles.d.xml | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)

diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index 8815bf9..469deeb 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -303,6 +303,37 @@
</varlistentry>

<varlistentry>
+ <term><varname>h</varname></term>
+ <listitem><para>Set file/directory attributes. Lines of this type
+ accept shell-style globs in place of normal path names.</para>
+
+ <para>The format of the argument field is <varname>[+-=][aAcCdDeijsStTu]
+ </varname></para>
+
+ <para>The prefix <varname>+</varname> causes the
+ attribute(s) to be added; <varname>-</varname> causes the
+ attribute(s) to be removed; <varname>=</varname>
+ causes the attributes to set exactly as the following letters.</para>
+ <para>The letters 'aAcCdDeijsStTu' select the new
+ attributes for the files, see
+ <citerefentry><refentrytitle>chattr</refentrytitle>
+ <manvolnum>1</manvolnum></citerefentry> for further information.
+ </para>
+ <para>Passing only <varname>=</varname> as argument,
+ reset all the file attributes.</para>
+
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><varname>H</varname></term>
+ <listitem><para>Recursively set file/directory attributes. Lines
+ of this type accept shell-style globs in place of normal
+ path names.
+ </para></listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><varname>a</varname></term>
<term><varname>a+</varname></term>
<listitem><para>Set POSIX ACLs (access control lists). If
@@ -529,6 +560,7 @@
<citerefentry project='man-pages'><refentrytitle>setfattr</refentrytitle><manvolnum>1</manvolnum></citerefentry>,
<citerefentry project='man-pages'><refentrytitle>setfacl</refentrytitle><manvolnum>1</manvolnum></citerefentry>,
<citerefentry project='man-pages'><refentrytitle>getfacl</refentrytitle><manvolnum>1</manvolnum></citerefentry>
+ <citerefentry project='man-pages'><refentrytitle>chattr</refentrytitle><manvolnum>1</manvolnum></citerefentry>
</para>
</refsect1>
--
2.1.4
Zbigniew Jędrzejewski-Szmek
2015-03-16 03:24:36 UTC
Permalink
Post by Goffredo Baroncelli
Update the man page of tmpfiles.d(5), to document the new h/H command.
---
man/tmpfiles.d.xml | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index 8815bf9..469deeb 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -303,6 +303,37 @@
</varlistentry>
<varlistentry>
+ <term><varname>h</varname></term>
+ <listitem><para>Set file/directory attributes. Lines of this type
+ accept shell-style globs in place of normal path names.</para>
+
+ <para>The format of the argument field is <varname>[+-=][aAcCdDeijsStTu]
+ </varname></para>
+
+ <para>The prefix <varname>+</varname> causes the
+ attribute(s) to be added; <varname>-</varname> causes the
+ attribute(s) to be removed; <varname>=</varname>
+ causes the attributes to set exactly as the following letters.</para>
What happens if neither of the three prefix lettes is used? This
should be documented.
Post by Goffredo Baroncelli
+ <para>The letters 'aAcCdDeijsStTu' select the new
<literal> instead of ''.
Post by Goffredo Baroncelli
+ attributes for the files, see
+ <citerefentry><refentrytitle>chattr</refentrytitle>
+ <manvolnum>1</manvolnum></citerefentry> for further information.
+ </para>
+ <para>Passing only <varname>=</varname> as argument,
+ reset all the file attributes.</para>
resets

So, is this description accurate? Operations on the attributes are
explicitly limited to the ones corresponding to the letters above (by
using a mask). But files can have other attributes, and the kernel might
define new attributes as some point. So maybe add a sentence like
"When operating on attributes, system-tmpfiles limits itself to the
attributes corresponding to the letters listed above. All other attributes
will be left untouched, even with <literal>=</literal>."

Zbyszek
Post by Goffredo Baroncelli
+
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><varname>H</varname></term>
+ <listitem><para>Recursively set file/directory attributes. Lines
+ of this type accept shell-style globs in place of normal
+ path names.
+ </para></listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><varname>a</varname></term>
<term><varname>a+</varname></term>
<listitem><para>Set POSIX ACLs (access control lists). If
@@ -529,6 +560,7 @@
<citerefentry project='man-pages'><refentrytitle>setfattr</refentrytitle><manvolnum>1</manvolnum></citerefentry>,
<citerefentry project='man-pages'><refentrytitle>setfacl</refentrytitle><manvolnum>1</manvolnum></citerefentry>,
<citerefentry project='man-pages'><refentrytitle>getfacl</refentrytitle><manvolnum>1</manvolnum></citerefentry>
+ <citerefentry project='man-pages'><refentrytitle>chattr</refentrytitle><manvolnum>1</manvolnum></citerefentry>
</para>
</refsect1>
--
2.1.4
_______________________________________________
systemd-devel mailing list
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Goffredo Baroncelli
2015-03-16 18:51:17 UTC
Permalink
Post by Zbigniew Jędrzejewski-Szmek
Post by Goffredo Baroncelli
Update the man page of tmpfiles.d(5), to document the new h/H command.
---
man/tmpfiles.d.xml | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index 8815bf9..469deeb 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -303,6 +303,37 @@
</varlistentry>
<varlistentry>
+ <term><varname>h</varname></term>
+ <listitem><para>Set file/directory attributes. Lines of this type
+ accept shell-style globs in place of normal path names.</para>
+
+ <para>The format of the argument field is <varname>[+-=][aAcCdDeijsStTu]
+ </varname></para>
+
+ <para>The prefix <varname>+</varname> causes the
+ attribute(s) to be added; <varname>-</varname> causes the
+ attribute(s) to be removed; <varname>=</varname>
+ causes the attributes to set exactly as the following letters.</para>
What happens if neither of the three prefix lettes is used? This
should be documented.
ok
Post by Zbigniew Jędrzejewski-Szmek
Post by Goffredo Baroncelli
+ <para>The letters 'aAcCdDeijsStTu' select the new
<literal> instead of ''.
ok
Post by Zbigniew Jędrzejewski-Szmek
Post by Goffredo Baroncelli
+ attributes for the files, see
+ <citerefentry><refentrytitle>chattr</refentrytitle>
+ <manvolnum>1</manvolnum></citerefentry> for further information.
+ </para>
+ <para>Passing only <varname>=</varname> as argument,
+ reset all the file attributes.</para>
resets
So, is this description accurate? Operations on the attributes are
explicitly limited to the ones corresponding to the letters above (by
using a mask). But files can have other attributes, and the kernel might
define new attributes as some point. So maybe add a sentence like
"When operating on attributes, system-tmpfiles limits itself to the
attributes corresponding to the letters listed above. All other attributes
will be left untouched, even with <literal>=</literal>."
Zbyszek
You are right, good catch !
Post by Zbigniew Jędrzejewski-Szmek
Post by Goffredo Baroncelli
+
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><varname>H</varname></term>
+ <listitem><para>Recursively set file/directory attributes. Lines
+ of this type accept shell-style globs in place of normal
+ path names.
+ </para></listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><varname>a</varname></term>
<term><varname>a+</varname></term>
<listitem><para>Set POSIX ACLs (access control lists). If
@@ -529,6 +560,7 @@
<citerefentry project='man-pages'><refentrytitle>setfattr</refentrytitle><manvolnum>1</manvolnum></citerefentry>,
<citerefentry project='man-pages'><refentrytitle>setfacl</refentrytitle><manvolnum>1</manvolnum></citerefentry>,
<citerefentry project='man-pages'><refentrytitle>getfacl</refentrytitle><manvolnum>1</manvolnum></citerefentry>
+ <citerefentry project='man-pages'><refentrytitle>chattr</refentrytitle><manvolnum>1</manvolnum></citerefentry>
</para>
</refsect1>
--
2.1.4
_______________________________________________
systemd-devel mailing list
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
Loading...