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

2015-05-25 Thread Karel Zak 
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.

swapon -o options-from-fstab

For backward compatibility the code cares about Priority: swap unit
field (for a case when Priority: is set, but pri= in the Options: is
missing).

References: 
http://lists.freedesktop.org/archives/systemd-devel/2014-October/023576.html
---
V2:
 - update README
 - add hint to systemd.swap man page
 - don't care about pri= in systed-fstab-generator at all
 - add warning about duplicate priority configuration
 - use warning rather than notice for non-parsable pri=

 README|  2 +-
 man/systemd.swap.xml  |  3 ++-
 src/core/swap.c   | 43 +++
 src/fstab-generator/fstab-generator.c | 28 ---
 4 files changed, 26 insertions(+), 50 deletions(-)

diff --git a/README b/README
index 039110e..2b8c68e 100644
--- a/README
+++ b/README
@@ -136,7 +136,7 @@ REQUIREMENTS:
 During runtime, you need the following additional
 dependencies:
 
-util-linux = v2.25 required
+util-linux = v2.26 required
 dbus = 1.4.0 (strictly speaking optional, but recommended)
 dracut (optional)
 PolicyKit (optional)
diff --git a/man/systemd.swap.xml b/man/systemd.swap.xml
index 5016f45..c398677 100644
--- a/man/systemd.swap.xml
+++ b/man/systemd.swap.xml
@@ -177,7 +177,8 @@
 
 listitemparaSwap priority to use when activating the swap
 device or file. This takes an integer. This setting is
-optional./para/listitem
+optional and ignored when priotiry is set by optionpri=/option in 
the
+varnameOptions=/varname option./para/listitem
   /varlistentry
 
   varlistentry
diff --git a/src/core/swap.c b/src/core/swap.c
index 12ebf84..193c8c3 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -717,8 +717,8 @@ fail:
 }
 
 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,21 @@ 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;
 
-priority = s-parameters_fragment.priority;
-if (priority  0) {
-r = fstab_find_pri(s-parameters_fragment.options, 
priority);
+r = fstab_find_pri(s-parameters_fragment.options, priority);
+if (r  0)
+log_warning_errno(r, Failed to parse swap priority 
\%s\, ignoring: %m, s-parameters_fragment.options);
+else if (r == 1  s-parameters_fragment.priority = 0)
+log_warning(Duplicate swap priority configuration by 
Priority and Options fields.);
+
+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;
 }
 }
 
@@ -740,24 +748,9 @@ static void swap_enter_activating(Swap *s) {
 if (r  0)
 goto fail;
 
-if (priority = 0) {
-char p[DECIMAL_STR_MAX(int)];
-
-sprintf(p, %i, priority);
-r = exec_command_append(s-control_command, -p, p, NULL);
-if (r  0)
-goto fail;
-}
-
-if (discard  !streq(discard, none)) {
-const char *discard_arg;
-
-if (streq(discard, all))
-discard_arg = --discard;
-else
-discard_arg = strjoina(--discard=, discard);
-
-r = exec_command_append(s-control_command, discard_arg, NULL);
+if (s-parameters_fragment.options || opts) {
+r = exec_command_append(s-control_command, -o,
+opts ? : s-parameters_fragment.options, NULL);
 if (r  0)
 goto fail;
 }
diff --git a/src/fstab-generator/fstab-generator.c

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

2015-05-25 Thread systemd github import bot 
Patchset imported to github.
Pull request:
https://github.com/systemd-devs/systemd/compare/master...systemd-mailing-devs:1432548683-7591-1-git-send-email-kzak%40redhat.com

--
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] swap: use swapon -o

2015-05-25 Thread Tom Gundersen 
Applied. Thanks!

Tom

On Mon, May 25, 2015 at 12:11 PM, Karel Zak k...@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.

 swapon -o options-from-fstab

 For backward compatibility the code cares about Priority: swap unit
 field (for a case when Priority: is set, but pri= in the Options: is
 missing).

 References: 
 http://lists.freedesktop.org/archives/systemd-devel/2014-October/023576.html
 ---
 V2:
  - update README
  - add hint to systemd.swap man page
  - don't care about pri= in systed-fstab-generator at all
  - add warning about duplicate priority configuration
  - use warning rather than notice for non-parsable pri=

  README|  2 +-
  man/systemd.swap.xml  |  3 ++-
  src/core/swap.c   | 43 
 +++
  src/fstab-generator/fstab-generator.c | 28 ---
  4 files changed, 26 insertions(+), 50 deletions(-)

 diff --git a/README b/README
 index 039110e..2b8c68e 100644
 --- a/README
 +++ b/README
 @@ -136,7 +136,7 @@ REQUIREMENTS:
  During runtime, you need the following additional
  dependencies:

 -util-linux = v2.25 required
 +util-linux = v2.26 required
  dbus = 1.4.0 (strictly speaking optional, but recommended)
  dracut (optional)
  PolicyKit (optional)
 diff --git a/man/systemd.swap.xml b/man/systemd.swap.xml
 index 5016f45..c398677 100644
 --- a/man/systemd.swap.xml
 +++ b/man/systemd.swap.xml
 @@ -177,7 +177,8 @@

  listitemparaSwap priority to use when activating the swap
  device or file. This takes an integer. This setting is
 -optional./para/listitem
 +optional and ignored when priotiry is set by optionpri=/option 
 in the
 +varnameOptions=/varname option./para/listitem
/varlistentry

varlistentry
 diff --git a/src/core/swap.c b/src/core/swap.c
 index 12ebf84..193c8c3 100644
 --- a/src/core/swap.c
 +++ b/src/core/swap.c
 @@ -717,8 +717,8 @@ fail:
  }

  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,21 @@ 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;

 -priority = s-parameters_fragment.priority;
 -if (priority  0) {
 -r = fstab_find_pri(s-parameters_fragment.options, 
 priority);
 +r = fstab_find_pri(s-parameters_fragment.options, 
 priority);
 +if (r  0)
 +log_warning_errno(r, Failed to parse swap priority 
 \%s\, ignoring: %m, s-parameters_fragment.options);
 +else if (r == 1  s-parameters_fragment.priority = 0)
 +log_warning(Duplicate swap priority configuration 
 by Priority and Options fields.);
 +
 +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;
  }
  }

 @@ -740,24 +748,9 @@ static void swap_enter_activating(Swap *s) {
  if (r  0)
  goto fail;

 -if (priority = 0) {
 -char p[DECIMAL_STR_MAX(int)];
 -
 -sprintf(p, %i, priority);
 -r = exec_command_append(s-control_command, -p, p, NULL);
 -if (r  0)
 -goto fail;
 -}
 -
 -if (discard  !streq(discard, none)) {
 -const char *discard_arg;
 -
 -if (streq(discard, all))
 -discard_arg = --discard;
 -else
 -discard_arg = strjoina(--discard=, discard);
 -
 -r = exec_command_append(s-control_command, discard_arg, 
 NULL);
 +if (s-parameters_fragment.options || opts) {
 +r = exec_command_append(s-control_command, -o,
 +

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

2015-05-21 Thread Lennart Poettering 
On Wed, 20.05.15 17:54, Karel Zak (k...@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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

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

2015-05-21 Thread Lennart Poettering 
On Thu, 21.05.15 18:02, Karel Zak (k...@redhat.com) wrote:

 On Thu, May 21, 2015 at 04:45:44PM +0200, Lennart Poettering wrote:
  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.
 
 This (priority specified by both fields) is standard fstab-generator 
 behaviour.
 Maybe the generator should be also updated to stop generate Priority:.

Yeah, I agree. It should pass the option string as is, given that we
can do so now, and not parse it in any way and generate secondary
fields from it.

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] swap: use swapon -o

2015-05-21 Thread Karel Zak 
On Thu, May 21, 2015 at 04:45:44PM +0200, Lennart Poettering wrote:
 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.

This (priority specified by both fields) is standard fstab-generator behaviour.
Maybe the generator should be also updated to stop generate Priority:.

Karel

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

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

2015-05-20 Thread systemd github import bot 
Patchset imported to github.
Pull request:
https://github.com/systemd-devs/systemd/compare/master...systemd-mailing-devs:1432137260-585-1-git-send-email-kzak%40redhat.com

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

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

2015-05-20 Thread Karel Zak 
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.

swapon -o options-from-fstab

For backward compatibility the code cares about Priority: swap unit
field (for a case when Priority: is set, but pri= in the Options: is
missing).

References: 
http://lists.freedesktop.org/archives/systemd-devel/2014-October/023576.html
---
 src/core/swap.c | 41 -
 1 file changed, 16 insertions(+), 25 deletions(-)

diff --git a/src/core/swap.c b/src/core/swap.c
index 12ebf84..658ac8b 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -717,8 +717,8 @@ fail:
 }
 
 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);
 
-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;
 }
 }
 
@@ -740,24 +746,9 @@ static void swap_enter_activating(Swap *s) {
 if (r  0)
 goto fail;
 
-if (priority = 0) {
-char p[DECIMAL_STR_MAX(int)];
-
-sprintf(p, %i, priority);
-r = exec_command_append(s-control_command, -p, p, NULL);
-if (r  0)
-goto fail;
-}
-
-if (discard  !streq(discard, none)) {
-const char *discard_arg;
-
-if (streq(discard, all))
-discard_arg = --discard;
-else
-discard_arg = strjoina(--discard=, discard);
-
-r = exec_command_append(s-control_command, discard_arg, NULL);
+if (s-parameters_fragment.options || opts) {
+r = exec_command_append(s-control_command, -o,
+opts ? : s-parameters_fragment.options, NULL);
 if (r  0)
 goto fail;
 }
-- 
2.1.0

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