[systemd-devel] [PATCH] swap: use swapon -o

[Lennart Poettering]

On Wed, 20.05.15 17:54, Karel Zak (kzak at redhat.com) wrote:

> This patch simplify swapon usage in systemd. The command swapon(8)
> since util-linux v2.26 supports "-o <list>". The idea is exactly the
> same like for mount(8). The -o specifies options in fstab-compatible
> way. For systemd it means that it does not have to care about things
> like "discard" or another swapon specific options.

I figure the README should be updated as part of the patch to require
util-linux 2.26 then.

>  static void swap_enter_activating(Swap *s) {
> -        _cleanup_free_ char *discard = NULL;
> -        int r, priority = -1;
> +        _cleanup_free_ char *opts = NULL;
> +        int r;
>  
>          assert(s);
>  
> @@ -726,13 +726,19 @@ static void swap_enter_activating(Swap *s) {
>          s->control_command = s->exec_command + SWAP_EXEC_ACTIVATE;
>  
>          if (s->from_fragment) {
> -                fstab_filter_options(s->parameters_fragment.options, "discard\0", NULL, &discard, NULL);
> +                int priority = -1;
> +
> +                r = fstab_find_pri(s->parameters_fragment.options, &priority);
> +                if (r < 0)
> +                        log_notice_errno(r, "Failed to parse swap
> priority \"%s\", ignoring: %m", s->parameters_fragment.options);

So far we logged such non-fatal errors as LOG_WARNING. Could you
upgrade this from log_notice_errno() to log_warning_errno() please?

(I am aware the the previous code used LOG_NOTICE, but this should be
fixed I think)

>  
> -                priority = s->parameters_fragment.priority;
> -                if (priority < 0) {
> -                        r = fstab_find_pri(s->parameters_fragment.options, &priority);
> +                if (r <= 0 && s->parameters_fragment.priority >= 0) {
> +                        if (s->parameters_fragment.options)
> +                                r = asprintf(&opts, "%s,pri=%i", s->parameters_fragment.options, s->parameters_fragment.priority);
> +                        else
> +                                r = asprintf(&opts, "pri=%i", s->parameters_fragment.priority);
>                          if (r < 0)
> -                                log_notice_errno(r, "Failed to
> -                parse swap priority \"%s\", ignoring: %m",
> -                s->parameters_fragment.options);
> +                                goto fail;
>                  }

I think it might be a good idea to log a warning if the priority is
specified both in s->parameters_frament.priority and in
s->parameters_fragment.options.

Otherwise looks good!

Lennart

-- 
Lennart Poettering, Red Hat