[systemd-devel] [PATCH] Add support for supplying an exit status code to systemctl exit

2014-11-06 Thread Vito Caputo
The capability of directly propagating a return code out to the caller of
systemd --user from within something like an OnFailure unit has utility.

This also contains a minor fixup to the documentation adding exit to the
--force section.

Cheers,
Vito Caputo

---
 man/systemctl.xml | 11 ++-
 src/core/dbus-manager.c   |  6 +-
 src/core/main.c   |  2 +-
 src/core/manager.c|  1 +
 src/core/manager.h|  1 +
 src/systemctl/systemctl.c | 20 ++--
 6 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/man/systemctl.xml b/man/systemctl.xml
index 7cbaa6c..25ad2f7 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -432,9 +432,9 @@ along with systemd; If not, see 
http://www.gnu.org/licenses/.
   paraWhen used with commandenable/command, overwrite
   any existing conflicting symlinks./para

-  paraWhen used with commandhalt/command,
-  commandpoweroff/command, commandreboot/command or
-  commandkexec/command, execute the selected operation
+ paraWhen used with commandexit/command,
commandhalt/command,
+ commandpoweroff/command, commandreboot/command or
+ commandkexec/command, execute the selected operation
   without shutting down all units. However, all processes will
   be killed forcibly and all file systems are unmounted or
   remounted read-only. This is hence a drastic but relatively
@@ -1485,13 +1485,14 @@ kobject-uevent 1 systemd-udevd-kernel.socket
systemd-udevd.service
   /listitem
 /varlistentry
 varlistentry
-  termcommandexit/command/term
+  termcommandexit
optionalreplaceableSTATUS/replaceable/optional/command/term

   listitem
 paraAsk the systemd manager to quit. This is only
 supported for user service managers (i.e. in conjunction
 with the option--user/option option) and will fail
-otherwise./para
+otherwise.  Used in conjunction with --force a status code may
be
+   propagated into the sytemd manager's exit code./para
   /listitem

 /varlistentry
diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
index c54abd3..78f7f6d 100644
--- a/src/core/dbus-manager.c
+++ b/src/core/dbus-manager.c
@@ -1140,6 +1140,10 @@ static int method_exit(sd_bus *bus, sd_bus_message
*message, void *userdata, sd_
 if (m-running_as == SYSTEMD_SYSTEM)
 return sd_bus_error_setf(error,
SD_BUS_ERROR_NOT_SUPPORTED, Exit is only supported for user service
managers.);

+r = sd_bus_message_read(message, i, m-exit_retval);
+if (r  0)
+return r;
+
 m-exit_code = MANAGER_EXIT;

 return sd_bus_reply_method_return(message, NULL);
@@ -1918,7 +1922,7 @@ const sd_bus_vtable bus_manager_vtable[] = {
 SD_BUS_METHOD(RemoveSnapshot, s, NULL, method_remove_snapshot,
0),
 SD_BUS_METHOD(Reload, NULL, NULL, method_reload,
SD_BUS_VTABLE_UNPRIVILEGED),
 SD_BUS_METHOD(Reexecute, NULL, NULL, method_reexecute,
SD_BUS_VTABLE_UNPRIVILEGED),
-SD_BUS_METHOD(Exit, NULL, NULL, method_exit, 0),
+SD_BUS_METHOD(Exit, i, NULL, method_exit, 0),
 SD_BUS_METHOD(Reboot, NULL, NULL, method_reboot,
SD_BUS_VTABLE_CAPABILITY(CAP_SYS_BOOT)),
 SD_BUS_METHOD(PowerOff, NULL, NULL, method_poweroff,
SD_BUS_VTABLE_CAPABILITY(CAP_SYS_BOOT)),
 SD_BUS_METHOD(Halt, NULL, NULL, method_halt,
SD_BUS_VTABLE_CAPABILITY(CAP_SYS_BOOT)),
diff --git a/src/core/main.c b/src/core/main.c
index d48604e..2481f5c 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1737,7 +1737,7 @@ int main(int argc, char *argv[]) {
 switch (m-exit_code) {

 case MANAGER_EXIT:
-retval = EXIT_SUCCESS;
+retval = m-exit_retval;
 log_debug(Exit.);
 goto finish;

diff --git a/src/core/manager.c b/src/core/manager.c
index 129f6dd..e417514 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -550,6 +550,7 @@ int manager_new(SystemdRunningAs running_as, bool
test_run, Manager **_m) {

 m-running_as = running_as;
 m-exit_code = _MANAGER_EXIT_CODE_INVALID;
+m-exit_retval = EXIT_SUCCESS;
 m-default_timer_accuracy_usec = USEC_PER_MINUTE;

 m-idle_pipe[0] = m-idle_pipe[1] = m-idle_pipe[2] =
m-idle_pipe[3] = -1;
diff --git a/src/core/manager.h b/src/core/manager.h
index ab72548..ef96e03 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -232,6 +232,7 @@ struct Manager {
 /* Flags */
 SystemdRunningAs running_as;
 ManagerExitCode exit_code:5;
+int exit_retval;

 bool dispatching_load_queue:1;
 bool dispatching_dbus_queue:1;
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index d9e9c2a..11c7e55 100644
--- a/src/systemctl/systemctl.c
+++ 

Re: [systemd-devel] [PATCH] Add support for supplying an exit status code to systemctl exit

2014-11-06 Thread Lennart Poettering
On Thu, 06.11.14 18:09, Vito Caputo (vito.cap...@coreos.com) wrote:

 The capability of directly propagating a return code out to the caller of
 systemd --user from within something like an OnFailure unit has utility.
 
 This also contains a minor fixup to the documentation adding exit to the
 --force section.

Patch is borked, is line-broken. Please resend non-line-broken
version! It's so hard to review otherwise.

 
 Cheers,
 Vito Caputo
 
 ---
  man/systemctl.xml | 11 ++-
  src/core/dbus-manager.c   |  6 +-
  src/core/main.c   |  2 +-
  src/core/manager.c|  1 +
  src/core/manager.h|  1 +
  src/systemctl/systemctl.c | 20 ++--
  6 files changed, 32 insertions(+), 9 deletions(-)
 
 diff --git a/man/systemctl.xml b/man/systemctl.xml
 index 7cbaa6c..25ad2f7 100644
 --- a/man/systemctl.xml
 +++ b/man/systemctl.xml
 @@ -432,9 +432,9 @@ along with systemd; If not, see 
 http://www.gnu.org/licenses/.
paraWhen used with commandenable/command, overwrite
any existing conflicting symlinks./para
 
 -  paraWhen used with commandhalt/command,
 -  commandpoweroff/command, commandreboot/command or
 -  commandkexec/command, execute the selected operation
 + paraWhen used with commandexit/command,
 commandhalt/command,
 + commandpoweroff/command, commandreboot/command or
 + commandkexec/command, execute the selected operation
without shutting down all units. However, all processes will
be killed forcibly and all file systems are unmounted or
remounted read-only. This is hence a drastic but relatively
 @@ -1485,13 +1485,14 @@ kobject-uevent 1 systemd-udevd-kernel.socket
 systemd-udevd.service
/listitem
  /varlistentry
  varlistentry
 -  termcommandexit/command/term
 +  termcommandexit
 optionalreplaceableSTATUS/replaceable/optional/command/term
 
listitem
  paraAsk the systemd manager to quit. This is only
  supported for user service managers (i.e. in conjunction
  with the option--user/option option) and will fail
 -otherwise./para
 +otherwise.  Used in conjunction with --force a status code may
 be
 +   propagated into the sytemd manager's exit code./para
/listitem
 
  /varlistentry
 diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c
 index c54abd3..78f7f6d 100644
 --- a/src/core/dbus-manager.c
 +++ b/src/core/dbus-manager.c
 @@ -1140,6 +1140,10 @@ static int method_exit(sd_bus *bus, sd_bus_message
 *message, void *userdata, sd_
  if (m-running_as == SYSTEMD_SYSTEM)
  return sd_bus_error_setf(error,
 SD_BUS_ERROR_NOT_SUPPORTED, Exit is only supported for user service
 managers.);
 
 +r = sd_bus_message_read(message, i, m-exit_retval);
 +if (r  0)
 +return r;
 +
  m-exit_code = MANAGER_EXIT;
 
  return sd_bus_reply_method_return(message, NULL);
 @@ -1918,7 +1922,7 @@ const sd_bus_vtable bus_manager_vtable[] = {
  SD_BUS_METHOD(RemoveSnapshot, s, NULL, method_remove_snapshot,
 0),
  SD_BUS_METHOD(Reload, NULL, NULL, method_reload,
 SD_BUS_VTABLE_UNPRIVILEGED),
  SD_BUS_METHOD(Reexecute, NULL, NULL, method_reexecute,
 SD_BUS_VTABLE_UNPRIVILEGED),
 -SD_BUS_METHOD(Exit, NULL, NULL, method_exit, 0),
 +SD_BUS_METHOD(Exit, i, NULL, method_exit, 0),
  SD_BUS_METHOD(Reboot, NULL, NULL, method_reboot,
 SD_BUS_VTABLE_CAPABILITY(CAP_SYS_BOOT)),
  SD_BUS_METHOD(PowerOff, NULL, NULL, method_poweroff,
 SD_BUS_VTABLE_CAPABILITY(CAP_SYS_BOOT)),
  SD_BUS_METHOD(Halt, NULL, NULL, method_halt,
 SD_BUS_VTABLE_CAPABILITY(CAP_SYS_BOOT)),
 diff --git a/src/core/main.c b/src/core/main.c
 index d48604e..2481f5c 100644
 --- a/src/core/main.c
 +++ b/src/core/main.c
 @@ -1737,7 +1737,7 @@ int main(int argc, char *argv[]) {
  switch (m-exit_code) {
 
  case MANAGER_EXIT:
 -retval = EXIT_SUCCESS;
 +retval = m-exit_retval;
  log_debug(Exit.);
  goto finish;
 
 diff --git a/src/core/manager.c b/src/core/manager.c
 index 129f6dd..e417514 100644
 --- a/src/core/manager.c
 +++ b/src/core/manager.c
 @@ -550,6 +550,7 @@ int manager_new(SystemdRunningAs running_as, bool
 test_run, Manager **_m) {
 
  m-running_as = running_as;
  m-exit_code = _MANAGER_EXIT_CODE_INVALID;
 +m-exit_retval = EXIT_SUCCESS;
  m-default_timer_accuracy_usec = USEC_PER_MINUTE;
 
  m-idle_pipe[0] = m-idle_pipe[1] = m-idle_pipe[2] =
 m-idle_pipe[3] = -1;
 diff --git a/src/core/manager.h b/src/core/manager.h
 index ab72548..ef96e03 100644
 --- a/src/core/manager.h
 +++ b/src/core/manager.h
 @@ -232,6 +232,7 @@ struct Manager {
  /* Flags */
  

Re: [systemd-devel] [PATCH] Add support for supplying an exit status code to systemctl exit

2014-11-06 Thread Lennart Poettering
On Thu, 06.11.14 18:09, Vito Caputo (vito.cap...@coreos.com) wrote:

 
 +r = sd_bus_message_read(message, i, m-exit_retval);
 +if (r  0)
 +return r;
 +
  m-exit_code = MANAGER_EXIT;
 
  return sd_bus_reply_method_return(message, NULL);
 @@ -1918,7 +1922,7 @@ const sd_bus_vtable bus_manager_vtable[] = {
  SD_BUS_METHOD(RemoveSnapshot, s, NULL, method_remove_snapshot,
 0),
  SD_BUS_METHOD(Reload, NULL, NULL, method_reload,
 SD_BUS_VTABLE_UNPRIVILEGED),
  SD_BUS_METHOD(Reexecute, NULL, NULL, method_reexecute,
 SD_BUS_VTABLE_UNPRIVILEGED),
 -SD_BUS_METHOD(Exit, NULL, NULL, method_exit, 0),
 +SD_BUS_METHOD(Exit, i, NULL, method_exit, 0),

I am not opposed to having this, but the bus interfaces are API we
cannot just change signatures of the calls. Please add a new call
ExitWithCode() or so as an alternative call with exit code.

 @@ -4875,6 +4875,22 @@ static int daemon_reload(sd_bus *bus, char **args) {
  if (r  0)
  return bus_log_create_error(r);
 
 +if (streq(method, Exit)) {
 +int retval = EXIT_SUCCESS;
 +
 +if (strv_length(args)  1) {
 +r = safe_atoi(args[1], retval);
 +if (r  0) {
 +log_error(Invalid exit status: %s,
 strerror(-r));
 +return -EINVAL;
 +}
 +}
 +
 +r = sd_bus_message_append(m, i, retval);
 +if (r  0)
 +return bus_log_create_error(r);
 +}

This check really needs to be within the if block that checks if
arg_action is ACTION_SYSTEMCTL (the one with the assert). The
parameter should only be parsed if we are invoked as systemctl, not
as any of the other symlinked tools like reboot or so (not that we'd
expose Exit() in those other tools, but please let's keep this clean).

Lennart

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