[systemd-devel] [PATCH] swap: rework discard

[Lennart Poettering]

On Thu, 23.10.14 14:58, Jan Synacek (jsynacek at redhat.com) wrote:

> -static int property_get_discard(
> +static int property_get_options(
>                  sd_bus *bus,
>                  const char *path,
>                  const char *interface,
> @@ -72,9 +72,9 @@ static int property_get_discard(
>          assert(s);
>  
>          if (s->from_fragment)
> -                p = s->parameters_fragment.discard ?: "none";
> +                p = s->parameters_fragment.options ?: "";

We already have strempty() doing precisely this ternary
operation. That said, sd_bus_message_append() is actually smart enough
to treat NULL strings as empty strings, so you can really just pass
the field directly, no need to write this to "p" first.

>  static void swap_enter_activating(Swap *s) {
>          int r, priority;
> -        char *discard;
> +        char *discard = NULL;
>  
>          assert(s);
>  
> @@ -750,7 +753,8 @@ static void swap_enter_activating(Swap *s) {
>  
>          if (s->from_fragment) {
>                  priority = s->parameters_fragment.priority;
> -                discard = s->parameters_fragment.discard;
> +                if (s->parameters_fragment.options)
> +                        discard =
>                  strstr(s->parameters_fragment.options, "discard");

strstr() sounds ugly, since it doesn't detect field separators
properly. we probably need something like hasmntopt() here really....

> diff --git a/src/core/swap.h b/src/core/swap.h
> index 3482d65..f5809a1 100644
> --- a/src/core/swap.h
> +++ b/src/core/swap.h
> @@ -63,7 +63,8 @@ typedef enum SwapResult {
>  
>  typedef struct SwapParameters {
>          char *what;
> -        char *discard;
> +        char *options;
> +        /* XXX: Once swapon learns "-o", priority can fall under options as well. */
>          int priority;
>          bool noauto:1;
>          bool nofail:1;

In the long run we should probably stop parsing the priority option
too, and just pass it along to swapon unmodified. But I guess we can
make that change later.

>          noauto = !!hasmntopt(me, "noauto");
> +        discard = hasmntopt(me, "discard");
>  
>          name = unit_name_from_path(what, ".swap");
>          if (!name)
> @@ -169,7 +131,7 @@ static int add_swap(const char *what, struct mntent *me) {
>                  fprintf(f, "Priority=%i\n", pri);
>  
>          if (discard)
> -                fprintf(f, "Discard=%s\n", discard);
> +                fprintf(f, "Options=%s\n", discard);
>  

Hmm, I think the generator should already treat the option fields the
same way as I want it to work in the long run, i.e. just read it from
fstab and write it 1:1 into the unit's Options= string.
 
Lennart

-- 
Lennart Poettering, Red Hat