Re: [systemd-devel] [PATCHv2] "-" prefix for InaccessibleDirectories and ReadOnlyDirectories

2013-08-23 Thread Zbigniew Jędrzejewski-Szmek
On Wed, Aug 21, 2013 at 04:43:55PM +0200, Maciej Wereski wrote:
> -effect.
> +effect.
> +If paths in 
> ReadOnlyDirectories=
> +or 
> InaccessibleDirectories=
> +start with "-", then errors will be
> +supressed if path doesn't 
> exist.
I reworded it a bit. We also have  tag for literals.

> -if (!path_is_absolute(rvalue)) {
> -log_syntax(unit, LOG_ERR, filename, line, EINVAL,
> -   "Not an absolute path, ignoring: %s", rvalue);
> -return 0;
> -}
> +if (streq(lvalue, "InaccessibleDirectories") || streq(lvalue, 
> "ReadOnlyDirectories")) {
> +if (!path_is_absolute(rvalue) && (rvalue[0] != '-' || 
> !path_is_absolute(rvalue+1)))
> +goto fail;
> +} else if (!path_is_absolute(rvalue)) 
> +goto fail;
I also reworded this part bit. 'goto cleanup's is something that we're trying to
erradicate, without making the code uglier.

> -if (!path_is_absolute(n)) {
> -log_syntax(unit, LOG_ERR, filename, line, EINVAL,
> -   "Not an absolute path, ignoring: %s", 
> rvalue);
> -continue;
> -}
> +if (streq(lvalue, "InaccessibleDirectories") || 
> streq(lvalue, "ReadOnlyDirectories")) {
> +if (!path_is_absolute(n) && (n[0] != '-' || 
> !path_is_absolute(n+1)))
> +goto fail;
> +} else if (!path_is_absolute(n))
> +goto fail;
Here you changed continue to return 0, which changes the semantics.

Applied, with fixups.

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCHv2] "-" prefix for InaccessibleDirectories and ReadOnlyDirectories

2013-08-21 Thread Maciej Wereski
---
v2:
- modify manpage
---
 TODO |  3 ---
 man/systemd.exec.xml |  6 +-
 src/core/namespace.c | 12 +++-
 src/shared/conf-parser.c | 27 +--
 4 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/TODO b/TODO
index 9bc14fd..97f2bcf 100644
--- a/TODO
+++ b/TODO
@@ -287,9 +287,6 @@ Features:
 
 * timedate: have global on/off switches for auto-time (NTP), and auto-timezone 
that connman can subscribe to.
 
-* Honour "-" prefix for InaccessibleDirectories= and ReadOnlyDirectories= to
-  suppress errors of the specified path doesn't exist
-
 * dev-setup.c: when running in a container, create a tiny stub udev
   database with the systemd tag set for all network interfaces found,
   so that libudev reports them as present, and systemd's .device units
diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml
index c0e1d86..93be660 100644
--- a/man/systemd.exec.xml
+++ b/man/systemd.exec.xml
@@ -828,7 +828,11 @@
 the empty string is assigned to this
 option the specific list is reset, and
 all prior assignments have no
-effect.
+effect.
+If paths in 
ReadOnlyDirectories=
+or InaccessibleDirectories=
+start with "-", then errors will be
+supressed if path doesn't 
exist.
 
 
 
diff --git a/src/core/namespace.c b/src/core/namespace.c
index 7e33d84..16b132b 100644
--- a/src/core/namespace.c
+++ b/src/core/namespace.c
@@ -51,6 +51,7 @@ typedef struct BindMount {
 const char *path;
 MountMode mode;
 bool done;
+bool ignore;
 } BindMount;
 
 static int append_mounts(BindMount **p, char **strv, MountMode mode) {
@@ -58,6 +59,13 @@ static int append_mounts(BindMount **p, char **strv, 
MountMode mode) {
 
 STRV_FOREACH(i, strv) {
 
+(*p)->ignore = false;
+
+if ((mode == INACCESSIBLE || mode == READONLY) && (*i)[0] == 
'-') {
+(*p)->ignore = true;
+(*i)++;
+}
+
 if (!path_is_absolute(*i))
 return -EINVAL;
 
@@ -155,6 +163,8 @@ static int apply_mount(
 r = mount(what, m->path, NULL, MS_BIND|MS_REC, NULL);
 if (r >= 0)
 log_debug("Successfully mounted %s to %s", what, m->path);
+else if (m->ignore && errno == ENOENT)
+r = 0;
 
 return r;
 }
@@ -168,7 +178,7 @@ static int make_read_only(BindMount *m) {
 return 0;
 
 r = mount(NULL, m->path, NULL, MS_BIND|MS_REMOUNT|MS_RDONLY|MS_REC, 
NULL);
-if (r < 0)
+if (r < 0 && !(m->ignore && errno == ENOENT))
 return -errno;
 
 return 0;
diff --git a/src/shared/conf-parser.c b/src/shared/conf-parser.c
index 2303d9a..b27c7eb 100644
--- a/src/shared/conf-parser.c
+++ b/src/shared/conf-parser.c
@@ -611,11 +611,11 @@ int config_parse_path(const char *unit,
 return 0;
 }
 
-if (!path_is_absolute(rvalue)) {
-log_syntax(unit, LOG_ERR, filename, line, EINVAL,
-   "Not an absolute path, ignoring: %s", rvalue);
-return 0;
-}
+if (streq(lvalue, "InaccessibleDirectories") || streq(lvalue, 
"ReadOnlyDirectories")) {
+if (!path_is_absolute(rvalue) && (rvalue[0] != '-' || 
!path_is_absolute(rvalue+1)))
+goto fail;
+} else if (!path_is_absolute(rvalue)) 
+goto fail;
 
 n = strdup(rvalue);
 if (!n)
@@ -627,6 +627,9 @@ int config_parse_path(const char *unit,
 *s = n;
 
 return 0;
+fail:
+log_syntax(unit, LOG_ERR, filename, line, EINVAL, "Not an absolute 
path, ignoring: %s", rvalue);
+return 0;
 }
 
 int config_parse_strv(const char *unit,
@@ -724,11 +727,11 @@ int config_parse_path_strv(const char *unit,
 continue;
 }
 
-if (!path_is_absolute(n)) {
-log_syntax(unit, LOG_ERR, filename, line, EINVAL,
-   "Not an absolute path, ignoring: %s", 
rvalue);
-continue;
-}
+if (streq(lvalue, "InaccessibleDirectories") || streq(lvalue, 
"ReadOnlyDirectories")) {
+if (!path_is_absolute(n) && (n[0] != '-' || 
!path_is_absolute(n+1)))
+goto fail;
+} else if (!path_is_absolute(n))
+goto fail;
 
 path_kill_slashes(n);
 r = strv_extend(sv, n);
@@ -737,6 +740,10 @@ int config_parse_path_strv(const char *unit,
 }