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

2014-10-29 Thread Jan Synacek
Lennart Poettering lenn...@poettering.net writes:

 On Tue, 28.10.14 13:14, Lennart Poettering (lenn...@poettering.net) wrote:

 On Thu, 23.10.14 16:39, Lennart Poettering (lenn...@poettering.net) wrote:
 
 Heya,
 
  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.
 
 I am hacking up a patch for this now, since I really want to get the
 new release out of the door soon.

 OK, landed that patch now. Didn't test it much though. Please test!

 Lennart

Works well on my system.

Thanks again!

-- 
Jan Synacek
Software Engineer, Red Hat


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


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

2014-10-28 Thread Lennart Poettering
On Thu, 23.10.14 16:39, Lennart Poettering (lenn...@poettering.net) wrote:

Heya,

 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.

I am hacking up a patch for this now, since I really want to get the
new release out of the door soon.

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: rework discard

2014-10-28 Thread Lennart Poettering
On Tue, 28.10.14 13:14, Lennart Poettering (lenn...@poettering.net) wrote:

 On Thu, 23.10.14 16:39, Lennart Poettering (lenn...@poettering.net) wrote:
 
 Heya,
 
  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.
 
 I am hacking up a patch for this now, since I really want to get the
 new release out of the door soon.

OK, landed that patch now. Didn't test it much though. Please test!

Lennart

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


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

2014-10-23 Thread Jan Synacek
Instead of a dedicated Discard option, use more general Options. When
the swapon command learns -o, it will be possible to pass the value of
Options as is. The code now assumes that the only possible value to
Options is related to discard.

http://lists.freedesktop.org/archives/systemd-devel/2014-October/024132.html
---
 src/core/dbus-swap.c  |  8 +++
 src/core/load-fragment-gperf.gperf.m4 |  2 +-
 src/core/swap.c   | 28 +++---
 src/core/swap.h   |  3 ++-
 src/fstab-generator/fstab-generator.c | 44 +++
 5 files changed, 25 insertions(+), 60 deletions(-)

diff --git a/src/core/dbus-swap.c b/src/core/dbus-swap.c
index c854716..0792cb4 100644
--- a/src/core/dbus-swap.c
+++ b/src/core/dbus-swap.c
@@ -55,7 +55,7 @@ static int property_get_priority(
 return sd_bus_message_append(reply, i, p);
 }
 
-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 ?: ;
 else
-p = none;
+p = ;
 return sd_bus_message_append(reply, s, p);
 }
 
@@ -84,7 +84,7 @@ const sd_bus_vtable bus_swap_vtable[] = {
 SD_BUS_VTABLE_START(0),
 SD_BUS_PROPERTY(What, s, NULL, offsetof(Swap, what), 
SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
 SD_BUS_PROPERTY(Priority, i, property_get_priority, 0, 
SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
-SD_BUS_PROPERTY(Discard, s, property_get_discard, 0, 
SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
+SD_BUS_PROPERTY(Options, s, property_get_options, 0, 
SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
 SD_BUS_PROPERTY(TimeoutUSec, t, bus_property_get_usec, 
offsetof(Swap, timeout_usec), SD_BUS_VTABLE_PROPERTY_CONST),
 SD_BUS_PROPERTY(ControlPID, u, bus_property_get_pid, 
offsetof(Swap, control_pid), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
 SD_BUS_PROPERTY(Result, s, property_get_result, offsetof(Swap, 
result), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE),
diff --git a/src/core/load-fragment-gperf.gperf.m4 
b/src/core/load-fragment-gperf.gperf.m4
index 8805411..19a79d5 100644
--- a/src/core/load-fragment-gperf.gperf.m4
+++ b/src/core/load-fragment-gperf.gperf.m4
@@ -297,7 +297,7 @@ Automount.DirectoryMode, config_parse_mode, 
 0,
 m4_dnl
 Swap.What,   config_parse_path,  0,
 offsetof(Swap, parameters_fragment.what)
 Swap.Priority,   config_parse_int,   0,
 offsetof(Swap, parameters_fragment.priority)
-Swap.Discard,config_parse_string,0,
 offsetof(Swap, parameters_fragment.discard)
+Swap.Options,config_parse_string,0,
 offsetof(Swap, parameters_fragment.options)
 Swap.TimeoutSec, config_parse_sec,   0,
 offsetof(Swap, timeout_usec)
 EXEC_CONTEXT_CONFIG_ITEMS(Swap)m4_dnl
 CGROUP_CONTEXT_CONFIG_ITEMS(Swap)m4_dnl
diff --git a/src/core/swap.c b/src/core/swap.c
index b2ca048..c183c32 100644
--- a/src/core/swap.c
+++ b/src/core/swap.c
@@ -152,8 +152,8 @@ static void swap_done(Unit *u) {
 free(s-parameters_fragment.what);
 s-parameters_fragment.what = NULL;
 
-free(s-parameters_fragment.discard);
-s-parameters_fragment.discard = NULL;
+free(s-parameters_fragment.options);
+s-parameters_fragment.options = NULL;
 
 s-exec_runtime = exec_runtime_unref(s-exec_runtime);
 exec_command_done_array(s-exec_command, _SWAP_EXEC_COMMAND_MAX);
@@ -607,12 +607,15 @@ static void swap_dump(Unit *u, FILE *f, const char 
*prefix) {
 fprintf(f,
 %sPriority: %i\n
 %sNoAuto: %s\n
-%sNoFail: %s\n
-%sDiscard: %s\n,
+%sNoFail: %s\n,
 prefix, p-priority,
 prefix, yes_no(p-noauto),
-prefix, yes_no(p-nofail),
-prefix, p-discard ?: none);
+prefix, yes_no(p-nofail));
+
+if (p-options)
+fprintf(f,
+%sOptions: %s\n,
+prefix, p-options);
 
 if (s-control_pid  0)
 fprintf(f,
@@ -741,7 +744,7 @@ fail:
 
 static void swap_enter_activating(Swap *s) {
 int r, priority;
-char *discard;
+char *discard = NULL;
 
 assert(s);
 
@@ -750,7 +753,8 @@ static void 

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

2014-10-23 Thread Lennart Poettering
On Thu, 23.10.14 14:58, Jan Synacek (jsyna...@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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel