Re: [systemd-devel] [PATCH v2] fstab-generator: add x-systemd.requires and x-systemd.requires-mounts-for

[Lennart Poettering]

On Thu, 14.05.15 13:27, Karel Zak (k...@redhat.com) wrote:

> +r = fstab_filter_options(opts, "x-systemd.requires\0", NULL, &arg, 
> NULL);
> +if (r < 0)
> +return log_warning_errno(r, "Failed to parse options: %m");
> +if (r == 0)
> +return 0;
> +
> +if (*arg == '/') {
> +r = unit_name_mangle_with_suffix(arg, UNIT_NAME_NOGLOB, 
> ".mount", &unit);
> +if (r < 0)
> +return log_error_errno(r, "Failed to generate unit 
> name: %m");
> +} else
> +unit = arg;

No need to explicitly check *arg for '/', just invoke
unit_name_mangle_with_suffix() immediately, without condition, as it
internally does pretty much the same check anyway. i.e. remove the if
check, remove the second branch, just invoke the function immediatey
and unconditionally, it will already do the right thing...

> +fprintf(f, "After=%1$s\nRequires=%1$s\n", unit);
> +
> +if (unit == arg)
> +unit = NULL;
> +
> +return 0;
> +}
> +
> +#define REQUIRES_MOUNTS_OPT"x-systemd.requires-mounts-for="

I'd just use the literal string everywhere. We only use this in this
one file and the macro isn't really that much shorter than the real
string, there's really no need to define a macro for this... 

> +
> +static int write_requires_mounts_for(FILE *f, const char *opts) {
> +_cleanup_free_ char **optsv = NULL, **paths = NULL, *res = NULL;
> +char **s;
> +
> +assert(f);
> +assert(opts);
> +
> +optsv = strv_split(opts, ",");
> +if (!optsv)
> +return log_oom();
> +
> +STRV_FOREACH(s, optsv) {
> +char *arg;
> +
> +if (!startswith(*s, REQUIRES_MOUNTS_OPT))
> +continue;
> +arg = *s + strlen(REQUIRES_MOUNTS_OPT);
> +if (!*arg)
> +return log_warning("Failed to parse option " 
> REQUIRES_MOUNTS_OPT);
> +if (!paths)
> +paths = strv_new(arg, NULL);
> +else
> +strv_push(&paths, arg);
> +}

I figure this logic should really be made generic and added to
fstab-util.c or so.

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH v2] fstab-generator: add x-systemd.requires and x-systemd.requires-mounts-for

[systemd github import bot]

Patchset imported to github.
Pull request:


--
Generated by https://github.com/haraldh/mail2git
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH v2] fstab-generator: add x-systemd.requires and x-systemd.requires-mounts-for

[Karel Zak]

On Thu, May 14, 2015 at 01:27:40PM +0200, Karel Zak wrote:
> +static int write_requires_mounts_for(FILE *f, const char *opts) {
> +_cleanup_free_ char **optsv = NULL, **paths = NULL, *res = NULL;

 _cleanup_strv_free_ **optsv = NULL, **paths = NULL;

 Do I need to resend the patch? ;-)

Karel

-- 
 Karel Zak  
 http://karelzak.blogspot.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH v2] fstab-generator: add x-systemd.requires and x-systemd.requires-mounts-for

[Karel Zak]

Currently we have no way how to specify dependencies between fstab
entries (or another units) in the /etc/fstab. It means that users are
forced to bypass fstab and write .mount units manually.

The patch introduces new systemd fstab options:

x-systemd.requires=

 - to specify dependence an another mount (PATH is translated to unit name)

x-systemd.requires=

 - to specify dependence on arbitrary UNIT

x-systemd.requires-mounts-for=

 - to specify dependence on another paths, implemented by
   RequiresMountsFor=. The option may be specified more than once.

For example two bind mounts where B depends on A:

 /mnt/test/A/mnt/test/A nonebind,defaults
 /mnt/test/A/mnt/test/B nonebind,x-systemd.requires=/mnt/test/A

More complex example with overlay FS where one mount point depends on
"low" and "upper" directories:

 /dev/sdc1   /mnt/lowext4 defaults
 /dev/sdc2   /mnt/high   ext4 defaults
 overlay /mnt/merged overlay  
lowerdir=/mnt/low,upperdir=/mnt/high/data,workdir=/mnt/high/work,x-systemd.requires-mounts-for=/mnt/low,x-systemd.requires-mounts-for=mnt/high

References: https://bugzilla.redhat.com/show_bug.cgi?id=812826
---

v2:
 - rename x-systemd.after to x-systemd.requires
 - allow to specify x-systemd.requires-mounts-for= more than once
 - propagate errors
 - fix some typos

 man/systemd.mount.xml | 20 +
 src/fstab-generator/fstab-generator.c | 83 +++
 2 files changed, 103 insertions(+)

diff --git a/man/systemd.mount.xml b/man/systemd.mount.xml
index 862f42e..58ad6a6 100644
--- a/man/systemd.mount.xml
+++ b/man/systemd.mount.xml
@@ -139,6 +139,26 @@
 
 
   
+x-systemd.requires=
+
+Configures After= and 
Requires= dependency
+between the mount and another mount or systemd unit. The argument is 
absolute path to
+another mount point or unit name. See After= and 
Requires= in
+
systemd.unit5
+for details.
+  
+
+  
+x-systemd.requires-mounts-for=
+
+Configures RequiresMountsFor= 
dependency between the mount and
+another mount. The argument must be absolute path. This option may be 
specified more than once.
+See RequiresMountsFor= in
+
systemd.unit5
+for details.
+   
+
+  
 x-systemd.automount
 
 An automount unit will be created for the file
diff --git a/src/fstab-generator/fstab-generator.c 
b/src/fstab-generator/fstab-generator.c
index 167ec60..29517e7 100644
--- a/src/fstab-generator/fstab-generator.c
+++ b/src/fstab-generator/fstab-generator.c
@@ -177,6 +177,71 @@ static int write_idle_timeout(FILE *f, const char *where, 
const char *opts) {
 return 0;
 }
 
+static int write_requires_after(FILE *f, const char *opts) {
+_cleanup_free_ char *arg = NULL, *unit = NULL;
+int r;
+
+assert(f);
+assert(opts);
+
+r = fstab_filter_options(opts, "x-systemd.requires\0", NULL, &arg, 
NULL);
+if (r < 0)
+return log_warning_errno(r, "Failed to parse options: %m");
+if (r == 0)
+return 0;
+
+if (*arg == '/') {
+r = unit_name_mangle_with_suffix(arg, UNIT_NAME_NOGLOB, 
".mount", &unit);
+if (r < 0)
+return log_error_errno(r, "Failed to generate unit 
name: %m");
+} else
+unit = arg;
+
+fprintf(f, "After=%1$s\nRequires=%1$s\n", unit);
+
+if (unit == arg)
+unit = NULL;
+
+return 0;
+}
+
+#define REQUIRES_MOUNTS_OPT"x-systemd.requires-mounts-for="
+
+static int write_requires_mounts_for(FILE *f, const char *opts) {
+_cleanup_free_ char **optsv = NULL, **paths = NULL, *res = NULL;
+char **s;
+
+assert(f);
+assert(opts);
+
+optsv = strv_split(opts, ",");
+if (!optsv)
+return log_oom();
+
+STRV_FOREACH(s, optsv) {
+char *arg;
+
+if (!startswith(*s, REQUIRES_MOUNTS_OPT))
+continue;
+arg = *s + strlen(REQUIRES_MOUNTS_OPT);
+if (!*arg)
+return log_warning("Failed to parse option " 
REQUIRES_MOUNTS_OPT);
+if (!paths)
+paths = strv_new(arg, NULL);
+else
+strv_push(&paths, arg);
+}
+
+if (paths) {
+res = strv_join(paths, " ");
+if (!res)
+return log_oom();
+fprintf(f, "RequiresMountsFor=%s\n", res);
+}
+
+return 0;
+}
+
 static int add_mount(
 const char *what,
 const char *where,
@@ -251,6 +316,15 @@ static int add_mount(
 if (post && !noauto && !nofail && !automount)
 fprintf(f, "Before=%s\n", post);
 
+if (!automount && opts) {
+ r