[systemd-devel] [PATCH 1/2] Allow disable of SysV init/rcN.d support at compile time

2010-09-20 Thread Fabiano Fidencio
This patch adds a cpp definition HAVE_SYSV_COMPAT that is used to
isolate code dealing with /etc/init.d and /etc/rcN.d for systems where
it does not make sense (one that does not use sysv or one that is fully
systemd native).

The patch tries to be as little intrusive as possible, however in
order to minimize the number of #ifdef'ed regions I've reordered some
code in path-lookup.c:lookup_paths_init() where all code dealing with
sysv is now isolated under running_as == MANAGER_SYSTEM as well.

Moreover, In struct Service, some fields were rearranged to reduce
the number of ifdefs.

Lennart's suggestions were fixed and squashed with the original patch,
that was sent by Gustavo Sverzut Barbieri (barbi...@profusion.mobi).
---
 configure.ac|   17 ++--
 src/build.h |8 +++-
 src/dbus-manager.c  |   49 ++--
 src/dbus-service.c  |   31 +++
 src/load-fragment.c |6 +++
 src/main.c  |   22 ++-
 src/manager.c   |2 +
 src/manager.h   |2 +
 src/path-lookup.c   |  102 +++---
 src/path-lookup.h   |2 +
 src/service.c   |   26 +
 src/service.h   |6 ++-
 src/systemctl.c |   19 -
 13 files changed, 220 insertions(+), 72 deletions(-)

diff --git a/configure.ac b/configure.ac
index e3c1fdd..27f40a5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -310,7 +310,8 @@ case $with_distro in
 M4_DISTRO_FLAG=-DTARGET_ARCH=1
 ;;
 gentoo)
-SYSTEM_SYSVRCND_PATH=/etc
+SYSTEM_SYSVINIT_PATH=
+SYSTEM_SYSVRCND_PATH=
 SPECIAL_SYSLOG_SERVICE=syslog-ng.service
 AC_DEFINE(TARGET_GENTOO, [], [Target is Gentoo])
 M4_DISTRO_FLAG=-DTARGET_GENTOO=1
@@ -322,10 +323,6 @@ case $with_distro in
 M4_DISTRO_FLAG=-DTARGET_SLACKWARE=1
 ;;
 other)
-AS_IF([test x$with_sysvinit_path = x],
-[AC_MSG_ERROR([With --distro=other, you must pass 
--with-sysvinit-path= to configure])])
-AS_IF([test x$with_sysvrcd_path = x],
-[AC_MSG_ERROR([With --distro=other, you must pass 
--with-sysvrcd-path= to configure])])
 AS_IF([test x$with_syslog_service = x],
 [AC_MSG_ERROR([With --distro=other, you must pass 
--with-syslog-service= to configure])])
 ;;
@@ -357,6 +354,15 @@ AC_SUBST(SYSTEM_SYSVRCND_PATH)
 AC_SUBST(SPECIAL_SYSLOG_SERVICE)
 AC_SUBST(M4_DISTRO_FLAG)
 
+if test x${SYSTEM_SYSVINIT_PATH} != x -a x${SYSTEM_SYSVRCND_PATH} != 
x; then
+AC_DEFINE(HAVE_SYSV_COMPAT, [], [SysV init scripts and rcN.d links are 
supported.])
+SYSTEM_SYSV_COMPAT=enabled
+elif test x${SYSTEM_SYSVINIT_PATH} != x -o x${SYSTEM_SYSVRCND_PATH} != 
x; then
+AC_MSG_ERROR([*** You need both --with-sysvinit-path=PATH and 
--with-sysvrcd-path=PATH to enable SysV compatibility support, or both empty to 
disable it.])
+else
+SYSTEM_SYSV_COMPAT=disabled
+fi
+
 AM_CONDITIONAL(TARGET_FEDORA, test x$with_distro = xfedora)
 AM_CONDITIONAL(TARGET_SUSE, test x$with_distro = xsuse)
 AM_CONDITIONAL(TARGET_DEBIAN, test x$with_distro = xdebian)
@@ -416,6 +422,7 @@ echo 
 $PACKAGE_NAME $VERSION
 
 Distribution:${with_distro}
+SysV compatibility:  ${SYSTEM_SYSV_COMPAT}
 SysV init scripts:   ${SYSTEM_SYSVINIT_PATH}
 SysV rc?.d directories:  ${SYSTEM_SYSVRCND_PATH}
 Syslog service:  ${SPECIAL_SYSLOG_SERVICE}
diff --git a/src/build.h b/src/build.h
index 5cb69be..b7f647a 100644
--- a/src/build.h
+++ b/src/build.h
@@ -46,6 +46,12 @@
 #define _SELINUX_FEATURE_ -SELINUX
 #endif
 
-#define SYSTEMD_FEATURES _PAM_FEATURE_   _LIBWRAP_FEATURE_   
_AUDIT_FEATURE_   _SELINUX_FEATURE_
+#ifdef HAVE_SYSV
+#define _SYSVINIT_FEATURE_ +SYSVINIT
+#else
+#define _SYSVINIT_FEATURE_ -SYSVINIT
+#endif
+
+#define SYSTEMD_FEATURES _PAM_FEATURE_   _LIBWRAP_FEATURE_   
_AUDIT_FEATURE_   _SELINUX_FEATURE_   _SYSVINIT_FEATURE_
 
 #endif
diff --git a/src/dbus-manager.c b/src/dbus-manager.c
index cffa547..3b79415 100644
--- a/src/dbus-manager.c
+++ b/src/dbus-manager.c
@@ -27,14 +27,16 @@
 #include strv.h
 #include bus-errors.h
 
-#define BUS_MANAGER_INTERFACE   \
- interface name=\org.freedesktop.systemd1.Manager\\n  \
+#define BUS_MANAGER_INTERFACE_BEGIN \
+ interface name=\org.freedesktop.systemd1.Manager\\n
+
+#define BUS_MANAGER_INTERFACE_METHODS   \
   method name=\GetUnit\\n \
arg name=\name\ type=\s\ direction=\in\/\n \
arg name=\unit\ type=\o\ direction=\out\/\n\
   /method\n \
   method 

Re: [systemd-devel] [PATCH 1/2] Allow disable of SysV init/rcN.d support at compile time

2010-09-20 Thread Lennart Poettering
On Mon, 20.09.10 16:33, Fabiano Fidencio (fiden...@profusion.mobi) wrote:

  static DEFINE_CONFIG_PARSE_ENUM(config_parse_kill_mode, kill_mode, KillMode, 
 Failed to parse kill mode);
  
 @@ -1446,7 +1448,9 @@ static void dump_items(FILE *f, const ConfigItem 
 *items) {
  { config_parse_exec, PATH [ARGUMENT [...]] },
  { config_parse_service_type, SERVICETYPE },
  { config_parse_service_restart,  SERVICERESTART },
 +#ifdef HAVE_SYSV_COMPAT
  { config_parse_sysv_priority,SYSVPRIORITY },
 +#endif
  { config_parse_kill_mode,KILLMODE },
  { config_parse_kill_signal,  SIGNAL },
  { config_parse_listen,   SOCKET [...] },
 @@ -1597,7 +1601,9 @@ static int load_from_path(Unit *u, const char *path) {
  { PermissionsStartOnly,   config_parse_bool,
 u-service.permissions_start_only,  Service },
  { RootDirectoryStartOnly, config_parse_bool,
 u-service.root_directory_start_only,   Service },
  { RemainAfterExit,config_parse_bool,
 u-service.remain_after_exit,   Service },
 +#ifdef HAVE_SYSV_COMPAT
  { SysVStartPriority,  config_parse_sysv_priority,   
 u-service.sysv_start_priority, Service },
 +#endif
  { NonBlocking,config_parse_bool,
 u-service.exec_context.non_blocking,   Service },
  { BusName,config_parse_string_printf,   
 u-service.bus_name,Service },
  { NotifyAccess,   config_parse_notify_access,   
 u-service.notify_access,   Service },

Hmpf, as mentioned in my earlier mail I blieve it is really important
that we still parse unit files correctly that have been written with syv
in mind even on a system where sysv is disabled. This change would
generate parse failures in this case.

Could you add config_parse_warn_compat() which would return successful,
but print one log_debug() messages warning that the requested setting is
not available because the feature was disabled at compile time? This
should be done generic enough so that we later on can use it in mmore
cases like this.

 +#ifdef HAVE_SYSV_COMPAT
  case SIGTERM:
  if (m-running_as == MANAGER_SYSTEM) {
  /* This is for compatibility with the
 @@ -1967,6 +1968,7 @@ static int manager_process_signal_fd(Manager *m) {
  m-exit_code = MANAGER_REEXECUTE;
  break;
  }
 +#endif

Nah, this please keep around. SIGTERM is SIGTERM. There's no point in
making it a NOP in non-sysv builds, as reexecution makes sense both with
and without sysv.

 +#ifdef HAVE_SYSV_COMPAT
  .check_gc = service_check_gc,
 +#else
 +.check_gc = NULL,
 +#endif

You can leave the NULL assignment out since everything not set
explicitly here defaults to NULL anyway.

  .check_snapshot = service_check_snapshot,
  
  .sigchld_event = service_sigchld_event,
 @@ -3009,5 +3031,9 @@ const UnitVTable service_vtable = {
  .bus_message_handler = bus_service_message_handler,
  .bus_invalidating_properties =  bus_service_invalidating_properties,
  
 +#ifdef HAVE_SYSV_COMPAT
  .enumerate = service_enumerate
 +#else
 +.enumerate = NULL
 +#endif

Same here.

 @@ -117,19 +117,21 @@ struct Service {
  bool bus_name_good:1;
  bool forbid_restart:1;
  bool got_socket_fd:1;
 +#ifdef HAVE_SYSV_COMPAT
  bool sysv_has_lsb:1;
  bool sysv_enabled:1;
 -
 -int socket_fd;
  int sysv_start_priority;
  
  char *sysv_path;
  char *sysv_runlevels;
 +#endif
  
  char *bus_name;
  
  char *status_text;
  
 +int socket_fd;
 +

Grmbl. ints are 32bit. On 64bit systems this would hence create 4 byte
hole that is unnecessary, if you put it here. Please move it up, next to
after the pid_t definitions. (Yes, i am a pahole nazi).

 -if ((c = exit_status_to_string(p-status, i-is_sysv 
 ? EXIT_STATUS_LSB : EXIT_STATUS_SYSTEMD)))
 +#ifdef HAVE_SYSV_COMPAT
 +if ((c = exit_status_to_string(p-status, 
 EXIT_STATUS_LSB)))
 +#else
 +if ((c = exit_status_to_string(p-status, 
 EXIT_STATUS_SYSTEMD)))
 +#endif

Hey, this is quite a change. the HAVE_SYSV_COMPAT branch still needs to
check i-is_sysv!

 -if ((c = 
 exit_status_to_string(i-exit_status, i-is_sysv ? EXIT_STATUS_LSB : 
 EXIT_STATUS_SYSTEMD)))
 +#ifdef HAVE_SYSV_COMPAT
 +if ((c = 
 exit_status_to_string(i-exit_status, EXIT_STATUS_LSB)))
 +#else
 +