[systemd-devel] [PATCH V2] Allow systemd-tmpfiles to set file/directory attributes

[Goffredo Baroncelli]

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;


[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

[Goffredo Baroncelli]

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);

[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..31faeb6 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, item->attrib_mask, f);
+ 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;

[Goffredo Baroncelli]

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

[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>
+ <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>

[Goffredo Baroncelli]

Please, forgot these patches: there is a bug inside.
Sorry for the noise.

BR
G.Baroncelli

Post by Goffredo Baroncelli
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
v1: first issue
v2: accepted several suggestion on the style; added function
change_attr_fd(); used the _cleanup_close_; returned
negative errno;
[1] Re: [systemd-devel] [RFC][PATCH] Add option to enable COW for journal file