Re: [systemd-devel] [PATCH 2/2] fsck: Add support for EFI variable based fsck indication

2015-03-15 Thread Kay Sievers
On Sun, Mar 15, 2015 at 11:56 AM, Jan Janssen medhe...@web.de wrote:
 ---
  man/systemctl.xml  | 26 
  shell-completion/bash/systemctl.in |  8 -
  shell-completion/zsh/_systemctl.in |  2 ++
  src/fsck/fsck.c| 63 +
  src/shared/efivars.h   | 21 +++--
  src/systemctl/systemctl.c  | 64 
 +-

Fsck is really something from the past, it should not be extended
beyond its current support.

Using non-volatile EFI variables for normal system operations does not
sound right, we should not start using that fragile subsystem for
things like this, the kernel command line should be sufficient enough.

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


Re: [systemd-devel] udev breaks hostap linkage of raw/user interfaces, causing wpa_supplicant problems

2015-03-15 Thread Kay Sievers
On Sun, Mar 15, 2015 at 1:40 PM, Thomas Richter t...@math.tu-berlin.de wrote:
 udev seems to create a problem here with the hostap (prism2) kernel driver.
 Unlike many wifi devices, the hostap device driver always creates paired
 interfaces, a raw interface (wifiX) and a network interface (wlanX) that
 represents the configured network.

 Unfortunately, udev (or hostap?) does not seem to be aware of this linkage,
 and hence, if you have two wifi radios in your system, may rename the second
 (wlanX) without the first (wifiX), and hence causing a name mismatch between
 the two.

 In general, this is not a problem, however, wpa_supplicant seems to depend
 on the linkage of the names. Hence, if wifiX does not match wlanX,
 wpa_supplicant will be unable to provide a WPA2 connection over a hostap
 driven wifi connection.

 Even worse, the complete procedure is completely untransparent to the user,
 i.e. neither wpa_supplicant (nor network-manager, depending on
 wpa_supplicant) nor network-manager provide a useful error message.

 Any chance of fixing this problem? Is this only a configuration issue? Is
 this an issue of hostap? Is this an issue of wpa_supplicant?

What does:
  $ grep . /sys/class/net/*/name_assign_type
print?

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


Re: [systemd-devel] [PATCH 2/2] fsck: Add support for EFI variable based fsck indication

2015-03-15 Thread Zbigniew Jędrzejewski-Szmek
On Sun, Mar 15, 2015 at 03:23:19PM +0100, Kay Sievers wrote:
 On Sun, Mar 15, 2015 at 11:56 AM, Jan Janssen medhe...@web.de wrote:
  ---
   man/systemctl.xml  | 26 
   shell-completion/bash/systemctl.in |  8 -
   shell-completion/zsh/_systemctl.in |  2 ++
   src/fsck/fsck.c| 63 
  +
   src/shared/efivars.h   | 21 +++--
   src/systemctl/systemctl.c  | 64 
  +-
 
 Fsck is really something from the past, it should not be extended
 beyond its current support.
Filesystems which require fsck are in wide use and will be in wide use
for the forseeable future.

 the kernel command line should be sufficient enough.
The kernel command line is not a good fit for a few reasons. One is that
it can be relatively hard to change, another is that changes are usually 
non-volatile,
and yet another is that changing the commandline is a problem in the
scheme in which kernel+initrd+cmdline are signed together.

EFI variables are the right solution for EFI systems.

 Using non-volatile EFI variables for normal system operations does not
 sound right, we should not start using that fragile subsystem for
 things like this
Volatile indeed sounds better. Are there volatile variables which are survive
a reboot but are then erased automatically?

Zbyszek

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


Re: [systemd-devel] [PATCH] fix compiler warning

2015-03-15 Thread Shawn Landden
On Sun, Mar 15, 2015 at 4:53 AM, Ronny Chevalier chevalier.ro...@gmail.com
wrote:

 2015-03-14 17:54 GMT+01:00 Shawn Landden sh...@churchofgit.com:
  On Sat, Mar 14, 2015 at 6:31 AM, Ronny Chevalier 
 chevalier.ro...@gmail.com
  wrote:
 
  2015-03-11 4:42 GMT+01:00 Shawn Landden sh...@churchofgit.com:
   warning: pointer/integer type mismatch in conditional expression
   ---
src/shared/socket-util.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
  
   diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
   index 5820279..73e1177 100644
   --- a/src/shared/socket-util.c
   +++ b/src/shared/socket-util.c
   @@ -475,8 +475,8 @@ int sockaddr_port(const struct sockaddr *_sa) {
return -EAFNOSUPPORT;
  
return ntohs(sa-sa.sa_family == AF_INET6 ?
   -   sa-in6.sin6_port :
   -   sa-in.sin_port);
   +   (uint16_t)sa-in6.sin6_port :
   +   (uint16_t)sa-in.sin_port);
}
  
 
  Hi,
 
  I don't see any warning, and the man (man netinet_in.h) says that
  sin_port and sin6_port are of type in_port_t which is equivalent to
  uint16_t.
 
 
  in_port_t and in6_port_t. If the type returned by a ternary operator is
 not
  identical gcc-5 warns, even though they are both backed by uint16_t and
 thus
  there is no violation.

 netinet/in.h does not provide in6_port_t according to the manpage. I
 even check with the glibc and on master there is no mention of
 in6_port_t.
 Maybe you are using another libc?

 You are right, but has been a bug in an early gcc-5, which quite a few
incorrect warnings.

 
 
  Maybe I'm missing something?
 
  Ronny
  ___
  systemd-devel mailing list
  systemd-devel@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/systemd-devel
 
 
 
 
  --
  Shawn Landden
 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel




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


[systemd-devel] udev breaks hostap linkage of raw/user interfaces, causing wpa_supplicant problems

2015-03-15 Thread Thomas Richter

Hi folks,

udev seems to create a problem here with the hostap (prism2) kernel 
driver. Unlike many wifi devices, the hostap device driver always 
creates paired interfaces, a raw interface (wifiX) and a network 
interface (wlanX) that represents the configured network.


Unfortunately, udev (or hostap?) does not seem to be aware of this 
linkage, and hence, if you have two wifi radios in your system, may 
rename the second (wlanX) without the first (wifiX), and hence causing a 
name mismatch between the two.


In general, this is not a problem, however, wpa_supplicant seems to 
depend on the linkage of the names. Hence, if wifiX does not match 
wlanX, wpa_supplicant will be unable to provide a WPA2 connection over a 
hostap driven wifi connection.


Even worse, the complete procedure is completely untransparent to the 
user, i.e. neither wpa_supplicant (nor network-manager, depending on 
wpa_supplicant) nor network-manager provide a useful error message.


Any chance of fixing this problem? Is this only a configuration issue? 
Is this an issue of hostap? Is this an issue of wpa_supplicant?


Either way, it took me several hours of figuring out what was wrong

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


Re: [systemd-devel] udev breaks hostap linkage of raw/user interfaces, causing wpa_supplicant problems

2015-03-15 Thread Thomas Richter

On 15.03.2015 15:38, Kay Sievers wrote:

Hi Kay,


grep ./sys/class/net/*/name_assign_type


grep: /sys/class/net/eth0/name_assign_type: Invalid argument
grep: /sys/class/net/lo/name_assign_type: Invalid argument
grep: /sys/class/net/wifi0/name_assign_type: Invalid argument
grep: /sys/class/net/wlan0/name_assign_type: Invalid argument

Thus, apparently, nothing useful right now. This is run as root, and I 
currently fixed /etc/udev/rules.d/70-persistent-net.rules manually to 
get wifi0 and wlan0 as above.


This is a Debian wheezy, with a custom 3.18.7 kernel. The original 
debian kernel is too old (yes, really!) for this old laptop as it did 
not support the i830 graphics reliably.


Greetings,
Thomas

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


[systemd-devel] [PATCH 2/2] fsck: Add support for EFI variable based fsck indication

2015-03-15 Thread Jan Janssen
---
 man/systemctl.xml  | 26 
 shell-completion/bash/systemctl.in |  8 -
 shell-completion/zsh/_systemctl.in |  2 ++
 src/fsck/fsck.c| 63 +
 src/shared/efivars.h   | 21 +++--
 src/systemctl/systemctl.c  | 64 +-
 6 files changed, 173 insertions(+), 11 deletions(-)

diff --git a/man/systemctl.xml b/man/systemctl.xml
index 3e2bcde..8449d83 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -466,6 +466,32 @@
   /varlistentry
 
   varlistentry
+termoption--fsck-mode=/option/term
+
+listitem
+  paraControl file system check behavior for next boot on EFI 
systems./para
+
+  paraOne of literalauto/literal, literalforce/literal and
+  literalskip/literal. See 
citerefentryrefentrytitlesystemd-fsck/refentrytitlemanvolnum8/manvolnum/citerefentry
+  for details. Note that this requires the system to be booted in EFI 
mode and
+  that kernel command line parameters take precedence./para
+/listitem
+  /varlistentry
+
+  varlistentry
+termoption--fsck-repair=/option/term
+
+listitem
+  paraControl file system check repair behavior for next boot on EFI 
systems./para
+
+  paraOne of literalpreen/literal, literalyes/literal and
+  literalno/literal. See 
citerefentryrefentrytitlesystemd-fsck/refentrytitlemanvolnum8/manvolnum/citerefentry
+  for details. Note that this requires the system to be booted in EFI 
mode and
+  that kernel command line parameters take precedence./para
+/listitem
+  /varlistentry
+
+  varlistentry
 termoption--root=/option/term
 
 listitem
diff --git a/shell-completion/bash/systemctl.in 
b/shell-completion/bash/systemctl.in
index f14fe7a..cea28cd 100644
--- a/shell-completion/bash/systemctl.in
+++ b/shell-completion/bash/systemctl.in
@@ -93,7 +93,7 @@ _systemctl () {
[STANDALONE]='--all -a --reverse --after --before --defaults 
--fail --ignore-dependencies --failed --force -f --full -l --global
  --help -h --no-ask-password --no-block 
--no-legend --no-pager --no-reload --no-wall
  --quiet -q --privileged -P --system --user 
--version --runtime --recursive -r --firmware'
-  [ARG]='--host -H --kill-who --property -p --signal -s 
--type -t --state --root'
+  [ARG]='--host -H --kill-who --property -p --signal -s 
--type -t --state --root --fsck-mode --fsck-repair'
 )
 
 if __contains_word --user ${COMP_WORDS[*]}; then
@@ -118,6 +118,12 @@ _systemctl () {
 --kill-who)
 comps='all control main'
 ;;
+--fsck-mode)
+comps='auto force skip'
+;;
+--fsck-repair)
+comps='preen yes no'
+;;
 --root)
 comps=$(compgen -A directory -- $cur )
 compopt -o filenames
diff --git a/shell-completion/zsh/_systemctl.in 
b/shell-completion/zsh/_systemctl.in
index 1caf9a4..b8c82cc 100644
--- a/shell-completion/zsh/_systemctl.in
+++ b/shell-completion/zsh/_systemctl.in
@@ -377,6 +377,8 @@ _arguments -s \
 '--no-ask-password[Do not ask for system passwords]' \
 '--firmware[Reboot to EFI setup on machines that support it]' \
 '--kill-who=[Who to send signal to]:killwho:(main control all)' \
+'--fsck-mode=[Control filesystem check mode next boot on EFI 
systems]:fsckmode:(auto force skip)' \
+'--fsck-repair=[Mode of operation to use with filesystem 
check]:fsckrepair:(preen yes no)' \
 {-s+,--signal=}'[Which signal to send]:signal:_signals' \
 {-f,--force}'[When enabling unit files, override existing symlinks. When 
shutting down, execute action immediately]' \
 '--root=[Enable unit files in the specified root 
directory]:directory:_directories' \
diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c
index 6e46633..ef56bb0 100644
--- a/src/fsck/fsck.c
+++ b/src/fsck/fsck.c
@@ -40,6 +40,7 @@
 #include path-util.h
 #include socket-util.h
 #include fsckd/fsckd.h
+#include efivars.h
 
 static bool arg_skip = false;
 static bool arg_force = false;
@@ -130,6 +131,67 @@ static void test_files(void) {
 
 }
 
+static void parse_efi_vars(void) {
+int r;
+size_t s;
+_cleanup_free_ void *v = NULL;
+
+if (!is_efi_boot())
+return;
+
+r = efi_get_variable(EFI_VENDOR_SYSTEMD, FsckModeOneShot, NULL, v, 
s);
+if (r  0 || s != sizeof(EfiSystemdFsckMode))
+log_warning(Failed to read FsckModeOneShot EFI variable.);
+else {
+EfiSystemdFsckMode value = 

[systemd-devel] [PATCH 1/2] systemctl: Add reboot to firmware support

2015-03-15 Thread Jan Janssen
---
 man/systemctl.xml  | 10 
 shell-completion/bash/systemctl.in |  2 +-
 shell-completion/zsh/_systemctl.in |  1 +
 src/shared/efivars.h   |  7 +++---
 src/systemctl/systemctl.c  | 48 ++
 5 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/man/systemctl.xml b/man/systemctl.xml
index 50e6bc9..3e2bcde 100644
--- a/man/systemctl.xml
+++ b/man/systemctl.xml
@@ -456,6 +456,16 @@
   /varlistentry
 
   varlistentry
+termoption--firmware/option/term
+
+listitem
+  paraIndicate to the firmware to boot into EFI setup on machines
+  that support it if commandreboot/command is used. Note that
+  this is only supported if the machine was booted in EFI mode./para
+/listitem
+  /varlistentry
+
+  varlistentry
 termoption--root=/option/term
 
 listitem
diff --git a/shell-completion/bash/systemctl.in 
b/shell-completion/bash/systemctl.in
index 8063316..f14fe7a 100644
--- a/shell-completion/bash/systemctl.in
+++ b/shell-completion/bash/systemctl.in
@@ -92,7 +92,7 @@ _systemctl () {
 local -A OPTS=(
[STANDALONE]='--all -a --reverse --after --before --defaults 
--fail --ignore-dependencies --failed --force -f --full -l --global
  --help -h --no-ask-password --no-block 
--no-legend --no-pager --no-reload --no-wall
- --quiet -q --privileged -P --system --user 
--version --runtime --recursive -r'
+ --quiet -q --privileged -P --system --user 
--version --runtime --recursive -r --firmware'
   [ARG]='--host -H --kill-who --property -p --signal -s 
--type -t --state --root'
 )
 
diff --git a/shell-completion/zsh/_systemctl.in 
b/shell-completion/zsh/_systemctl.in
index 7f2d5ac..1caf9a4 100644
--- a/shell-completion/zsh/_systemctl.in
+++ b/shell-completion/zsh/_systemctl.in
@@ -375,6 +375,7 @@ _arguments -s \
 '--global[Enable/disable unit files globally]' \
 --no-reload[When enabling/disabling unit files, don't reload daemon 
configuration] \
 '--no-ask-password[Do not ask for system passwords]' \
+'--firmware[Reboot to EFI setup on machines that support it]' \
 '--kill-who=[Who to send signal to]:killwho:(main control all)' \
 {-s+,--signal=}'[Which signal to send]:signal:_signals' \
 {-f,--force}'[When enabling unit files, override existing symlinks. When 
shutting down, execute action immediately]' \
diff --git a/src/shared/efivars.h b/src/shared/efivars.h
index 2492893..7bdfb74 100644
--- a/src/shared/efivars.h
+++ b/src/shared/efivars.h
@@ -28,9 +28,10 @@
 
 #define EFI_VENDOR_LOADER 
SD_ID128_MAKE(4a,67,b0,82,0a,4c,41,cf,b6,c7,44,0b,29,bb,8c,4f)
 #define EFI_VENDOR_GLOBAL 
SD_ID128_MAKE(8b,e4,df,61,93,ca,11,d2,aa,0d,00,e0,98,03,2b,8c)
-#define EFI_VARIABLE_NON_VOLATILE   0x0001
-#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0002
-#define EFI_VARIABLE_RUNTIME_ACCESS 0x0004
+#define EFI_VARIABLE_NON_VOLATILE0x0001
+#define EFI_VARIABLE_BOOTSERVICE_ACCESS  0x0002
+#define EFI_VARIABLE_RUNTIME_ACCESS  0x0004
+#define EFI_OS_INDICATIONS_BOOT_TO_FW_UI 0x0001
 
 bool is_efi_boot(void);
 int is_efi_secure_boot(void);
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 3158a38..8aee3c4 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -68,6 +68,8 @@
 #include bus-common-errors.h
 #include mkdir.h
 #include dropin.h
+#include virt.h
+#include efivars.h
 
 static char **arg_types = NULL;
 static char **arg_states = NULL;
@@ -132,7 +134,7 @@ static char *arg_host = NULL;
 static unsigned arg_lines = 10;
 static OutputMode arg_output = OUTPUT_SHORT;
 static bool arg_plain = false;
-
+static bool arg_firmware = false;
 static bool original_stdout_is_tty;
 
 static int daemon_reload(sd_bus *bus, char **args);
@@ -2923,9 +2925,40 @@ static int start_special(sd_bus *bus, char **args) {
 if (r  0)
 return r;
 
-if (arg_force = 2  geteuid() != 0) {
-log_error(Must be root.);
-return -EPERM;
+if ((arg_firmware || arg_force = 2)  geteuid() != 0)
+return log_error_errno(EPERM, Must be root.);
+
+if (arg_firmware) {
+size_t s;
+uint64_t b;
+_cleanup_free_ void *v = NULL;
+
+if (a != ACTION_REBOOT)
+return log_error_errno(EINVAL, Must use reboot 
command to reboot to firmware.);
+else if (detect_container(NULL)  0)
+return log_error_errno(ENOTSUP, Cannot reboot to 
firmware from within a container.);
+else if (!is_efi_boot())
+return log_error_errno(ENOTSUP, Reboot to firmware 
requires the system to be booted in EFI 

Re: [systemd-devel] [PATCH] fix compiler warning

2015-03-15 Thread Ronny Chevalier
2015-03-14 17:54 GMT+01:00 Shawn Landden sh...@churchofgit.com:
 On Sat, Mar 14, 2015 at 6:31 AM, Ronny Chevalier chevalier.ro...@gmail.com
 wrote:

 2015-03-11 4:42 GMT+01:00 Shawn Landden sh...@churchofgit.com:
  warning: pointer/integer type mismatch in conditional expression
  ---
   src/shared/socket-util.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)
 
  diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
  index 5820279..73e1177 100644
  --- a/src/shared/socket-util.c
  +++ b/src/shared/socket-util.c
  @@ -475,8 +475,8 @@ int sockaddr_port(const struct sockaddr *_sa) {
   return -EAFNOSUPPORT;
 
   return ntohs(sa-sa.sa_family == AF_INET6 ?
  -   sa-in6.sin6_port :
  -   sa-in.sin_port);
  +   (uint16_t)sa-in6.sin6_port :
  +   (uint16_t)sa-in.sin_port);
   }
 

 Hi,

 I don't see any warning, and the man (man netinet_in.h) says that
 sin_port and sin6_port are of type in_port_t which is equivalent to
 uint16_t.


 in_port_t and in6_port_t. If the type returned by a ternary operator is not
 identical gcc-5 warns, even though they are both backed by uint16_t and thus
 there is no violation.

netinet/in.h does not provide in6_port_t according to the manpage. I
even check with the glibc and on master there is no mention of
in6_port_t.
Maybe you are using another libc?



 Maybe I'm missing something?

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




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


Re: [systemd-devel] [PATCH] path-lookup: use secure_getenv()

2015-03-15 Thread Ronny Chevalier
2015-03-15 3:27 GMT+01:00 Shawn Landden sh...@churchofgit.com:
 All these except user_data_home_dir() are certainly vectors for
 arbitrary code execution. These should use secure_getenv()
 ---

Hi,

I don't see why secure_getenv() is appropriate here? These functions
are never used in the libraries systemd provides, they are mostly used
by systemctl and the dbus manager. Can you provide more details?

  src/shared/path-lookup.c | 20 ++--
  1 file changed, 10 insertions(+), 10 deletions(-)

 diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c
 index fbf46cd..0fb86df 100644
 --- a/src/shared/path-lookup.c
 +++ b/src/shared/path-lookup.c
 @@ -33,7 +33,7 @@ int user_config_home(char **config_home) {
  const char *e;
  char *r;

 -e = getenv(XDG_CONFIG_HOME);
 +e = secure_getenv(XDG_CONFIG_HOME);
  if (e) {
  r = strappend(e, /systemd/user);
  if (!r)
 @@ -44,7 +44,7 @@ int user_config_home(char **config_home) {
  } else {
  const char *home;

 -home = getenv(HOME);
 +home = secure_getenv(HOME);
  if (home) {
  r = strappend(home, /.config/systemd/user);
  if (!r)
 @@ -62,7 +62,7 @@ int user_runtime_dir(char **runtime_dir) {
  const char *e;
  char *r;

 -e = getenv(XDG_RUNTIME_DIR);
 +e = secure_getenv(XDG_RUNTIME_DIR);
  if (e) {
  r = strappend(e, /systemd/user);
  if (!r)
 @@ -83,13 +83,13 @@ static int user_data_home_dir(char **dir, const char 
 *suffix) {
   * suggests because we assume that that is a link to
   * /etc/systemd/ anyway. */

 -e = getenv(XDG_DATA_HOME);
 +e = secure_getenv(XDG_DATA_HOME);
  if (e)
  res = strappend(e, suffix);
  else {
  const char *home;

 -home = getenv(HOME);
 +home = secure_getenv(HOME);
  if (home)
  res = strjoin(home, /.local/share, suffix, NULL);
  else
 @@ -146,7 +146,7 @@ static char** user_dirs(
  if (user_runtime_dir(runtime_dir)  0)
  return NULL;

 -e = getenv(XDG_CONFIG_DIRS);
 +e = secure_getenv(XDG_CONFIG_DIRS);
  if (e) {
  config_dirs = strv_split(e, :);
  if (!config_dirs)
 @@ -157,7 +157,7 @@ static char** user_dirs(
  if (r  0)
  return NULL;

 -e = getenv(XDG_DATA_DIRS);
 +e = secure_getenv(XDG_DATA_DIRS);
  if (e)
  data_dirs = strv_split(e, :);
  else
 @@ -248,7 +248,7 @@ int lookup_paths_init(

  /* First priority is whatever has been passed to us via env
   * vars */
 -e = getenv(SYSTEMD_UNIT_PATH);
 +e = secure_getenv(SYSTEMD_UNIT_PATH);
  if (e) {
  if (endswith(e, :)) {
  e = strndupa(e, strlen(e) - 1);
 @@ -340,7 +340,7 @@ int lookup_paths_init(
  #ifdef HAVE_SYSV_COMPAT
  /* /etc/init.d/ compatibility does not matter to users */

 -e = getenv(SYSTEMD_SYSVINIT_PATH);
 +e = secure_getenv(SYSTEMD_SYSVINIT_PATH);
  if (e) {
  p-sysvinit_path = path_split_and_make_absolute(e);
  if (!p-sysvinit_path)
 @@ -358,7 +358,7 @@ int lookup_paths_init(
  return -ENOMEM;
  }

 -e = getenv(SYSTEMD_SYSVRCND_PATH);
 +e = secure_getenv(SYSTEMD_SYSVRCND_PATH);
  if (e) {
  p-sysvrcnd_path = path_split_and_make_absolute(e);
  if (!p-sysvrcnd_path)
 --
 2.2.1.209.g41e5f3a

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


Re: [systemd-devel] [PATCH] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily

2015-03-15 Thread Zbigniew Jędrzejewski-Szmek
On Wed, Mar 11, 2015 at 05:22:18PM -0700, Shawn Landden wrote:
 Still use helper when Xen Dom0, to avoid duplicating some hairy code.
 
 I think the rbtree version was far more understandable as greedy_realloc0()
 is very messy to do correctly.
 
 Take fopenat() from lsof.
 Add opendirat()
 
 Future: generate BootLoaderSpec files for other kernel install locations
 
 v5: fix syscall invocation from draft version
 v6: usr either /boot/efi or /boot
 ---
  Makefile.am   |   4 +-
  TODO  |   3 -
  man/systemctl.xml |  15 ++-
  src/core/shutdown.c   |  30 --
  src/shared/fileio.c   |  69 +
  src/shared/fileio.h   |   5 +
  src/shared/missing.h  |  11 +++
  src/shared/rpmvercmp.c|  26 +++--
  src/shared/strv.c |   9 +-
  src/systemctl/bootspec.c  | 247 
 ++
  src/systemctl/bootspec.h  |  48 +
  src/systemctl/systemctl.c |  57 ++-
  12 files changed, 495 insertions(+), 29 deletions(-)
  create mode 100644 src/systemctl/bootspec.c
  create mode 100644 src/systemctl/bootspec.h
 
 diff --git a/Makefile.am b/Makefile.am
 index 353a7de..20a6484 100644
 --- a/Makefile.am
 +++ b/Makefile.am
 @@ -2736,7 +2736,9 @@ systemd_escape_LDADD = \
  
  # 
 -
  systemctl_SOURCES = \
 - src/systemctl/systemctl.c
 + src/systemctl/systemctl.c \
 + src/systemctl/bootspec.c \
 + src/systemctl/bootspec.h
  
  systemctl_LDADD = \
   libsystemd-units.la \
 diff --git a/TODO b/TODO
 index 6180077..9ba7be0 100644
 --- a/TODO
 +++ b/TODO
 @@ -86,9 +86,6 @@ Features:
  * maybe introduce WantsMountsFor=? Usecase:

 http://lists.freedesktop.org/archives/systemd-devel/2015-January/027729.html
  
 -* rework kexec logic to use new kexec_file_load() syscall, so that we
 -  don't have to call kexec tool anymore.
 -
  * The udev blkid built-in should expose a property that reflects
whether media was sensed in USB CF/SD card readers. This should then
be used to control SYSTEMD_READY=1/0 so that USB card readers aren't
 diff --git a/man/systemctl.xml b/man/systemctl.xml
 index 338c1d3..c550339 100644
 --- a/man/systemctl.xml
 +++ b/man/systemctl.xml
 @@ -607,6 +607,15 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
 systemd-udevd.service
  /varlistentry
  
  varlistentry
 +  termcommandlist-kernels/command/term
 +
 +  listitem
 +paraList kernels ordered by version.
 +/para
 +  /listitem
 +/varlistentry
 +
 +varlistentry
termcommandstart 
 replaceablePATTERN/replaceable.../command/term
  
listitem
 @@ -1550,7 +1559,7 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
 systemd-udevd.service
  /varlistentry
  
  varlistentry
 -  termcommandkexec/command/term
 +  termcommandkexec 
 optionalreplaceableVERSION/replaceable/optionaloptionalreplaceableKERNEL-ARG/replaceable.../optional/command/term
  
listitem
  paraShut down and reboot the system via kexec. This is
 @@ -1560,6 +1569,10 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
 systemd-udevd.service
  services is skipped, however all processes are killed and
  all file systems are unmounted or mounted read-only,
  immediately followed by the reboot./para
 +
 +paraAlso allows specifying the version and optionally
 +extra kernel parameters to append. If no version is specified
 +then the most recent kernel is booted./para
/listitem
  /varlistentry
  
 diff --git a/src/core/shutdown.c b/src/core/shutdown.c
 index 70a461e..616a70a 100644
 --- a/src/core/shutdown.c
 +++ b/src/core/shutdown.c
 @@ -19,6 +19,7 @@
along with systemd; If not, see http://www.gnu.org/licenses/.
  ***/
  
 +#include ctype.h
  #include sys/mman.h
  #include sys/reboot.h
  #include linux/reboot.h
 @@ -173,15 +174,21 @@ int main(int argc, char *argv[]) {
  goto error;
  }
  
 +in_container = !!detect_virtualization(NULL);
 +
  if (streq(arg_verb, reboot))
  cmd = RB_AUTOBOOT;
  else if (streq(arg_verb, poweroff))
  cmd = RB_POWER_OFF;
  else if (streq(arg_verb, halt))
  cmd = RB_HALT_SYSTEM;
 -else if (streq(arg_verb, kexec))
 -cmd = LINUX_REBOOT_CMD_KEXEC;
 -else {
 +else if (streq(arg_verb, kexec)) {
 +if (in_container) {
 +log_warning(Can't kexec from container. 
 Rebooting…);
 +cmd = RB_AUTOBOOT;
 +} else
 +cmd = LINUX_REBOOT_CMD_KEXEC;
 +} else {
  r = -EINVAL;
  log_error(Unknown action '%s'., arg_verb);
  goto error;
 @@ -200,8 

Re: [systemd-devel] [PATCH] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily

2015-03-15 Thread Kay Sievers
On Sun, Mar 15, 2015 at 7:31 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Wed, Mar 11, 2015 at 05:22:18PM -0700, Shawn Landden wrote:
 Still use helper when Xen Dom0, to avoid duplicating some hairy code.

 I think the rbtree version was far more understandable as greedy_realloc0()
 is very messy to do correctly.

 Take fopenat() from lsof.
 Add opendirat()

 Future: generate BootLoaderSpec files for other kernel install locations

 v5: fix syscall invocation from draft version
 v6: usr either /boot/efi or /boot

 +static int fopen_to_open(const char *m) {
 +int flags;
 +
 +if (strchr(m, '+'))
 +flags = O_RDWR;
 +else if (m[0] == 'r')
 +flags = O_RDONLY;
 +else if (m[0] == 'w' || m[0] == 'a')
 +flags = O_WRONLY;
 +else
 +return(0);
 +
 +if (m[0] == 'a')
 +flags |= O_APPEND|O_CREAT;
 +if (m[0] == 'w')
 +flags |= O_TRUNC|O_CREAT;
 +
 +flags |= O_NONBLOCK;
 +
 +return flags;
 +}
 I really dislike this part. The open() interface is so much better
 than fopen().

We first need a clear definition of what this functionality is
supposed to solve and how and why we want that in systemd, and that we
can hold the promise we are making here. Unless that is done and looks
reasonable and acceptable, I see no way of this being merged.

Please do not waste any more time with style questions. The whole
approach in this patch does not seem convincing, and the idea of
parsing boot loader specifics with systemd code can't work out from my
point of view.

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


Re: [systemd-devel] Enabling timesyncd in virtual machines

2015-03-15 Thread Zbigniew Jędrzejewski-Szmek
On Sun, Mar 15, 2015 at 07:47:46PM +0100, Kay Sievers wrote:
 On Fri, Mar 13, 2015 at 8:25 PM, Michael Marineau
 michael.marin...@coreos.com wrote:
  Greetings,
 
  Currently systemd-timesyncd.service includes
  ConditionVirtualization=no, disabling it in both containers and
  virtual machines. Each VM platform tends to deal with or ignore the
  time problem in their own special ways, KVM/QEMU has the kernel time
  source kvm-clock, Xen has had different schemes over the years, VMware
  expects a userspace daemon sync the clock, and other platforms are
  content to drift with the wind as far as I can tell.
 
  I don't know of a robust way to know if a platform needs a little
  extra help from userspace to keep the clock sane or not but it seems
  generally safer to try than to risk drifting. Does anyone know of a
  reason to leave timesyncd off by default? Otherwise switching to
  ConditionVirtualization=!container should be reasonable.
 
 Sounds reasonable. Until someone has a better or clear idea how to
 solve that reliably, I turned the option around.
What about adding

ConditionVirtualization=!kvm ?

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


Re: [systemd-devel] [PATCH 2/6] fix strict aliasing violations in src/udev/udev-builtin-usb_id.c

2015-03-15 Thread Zbigniew Jędrzejewski-Szmek
On Wed, Mar 11, 2015 at 08:13:45AM -0700, Shawn Landden wrote:
 ---
  src/udev/udev-builtin-usb_id.c | 16 
  1 file changed, 8 insertions(+), 8 deletions(-)
 
 diff --git a/src/udev/udev-builtin-usb_id.c b/src/udev/udev-builtin-usb_id.c
 index 6516d93..3c15b2f 100644
 --- a/src/udev/udev-builtin-usb_id.c
 +++ b/src/udev/udev-builtin-usb_id.c
 @@ -28,6 +28,7 @@
  #include ctype.h
  #include fcntl.h
  #include errno.h
 +#include inttypes.h
  
  #include udev.h
  
 @@ -179,21 +180,20 @@ static int dev_if_packed_info(struct udev_device *dev, 
 char *ifs_str, size_t len
  
  ifs_str[0] = '\0';
  while (pos  size  strpos+7  len-2) {
 -struct usb_interface_descriptor *desc;
 +unsigned char *desc = buf[pos];
  char if_str[8];
  
 -desc = (struct usb_interface_descriptor *) buf[pos];
 -if (desc-bLength  3)
 +if (desc[offsetof(struct usb_interface_descriptor, bLength)] 
  3)
This makes the code really ugly. What about doing it the other way:

  #define BUF_SIZE (18 + 65535)
  struct usb_interface_descriptor *buf;
  buf = alloca(BUF_SIZE);
  ...
  read(fd, (char*) buf, BUF_SIZE);

If I understand the aliasing rules, this is OK, because:
C99 6.5 7:
An object shall have its stored value accessed only by an lvalue expression 
that has one of the following types:
  ...
  — a character type.

?

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


Re: [systemd-devel] [PATCH 2/2] fsck: Add support for EFI variable based fsck indication

2015-03-15 Thread Reindl Harald


Am 15.03.2015 um 20:50 schrieb Kay Sievers:

On Sun, Mar 15, 2015 at 8:08 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:

On Sun, Mar 15, 2015 at 07:58:35PM +0100, Kay Sievers wrote:

If the filesytem is too dumb to have that info in the superblock flags
to store, to request a forced fsck, it is the problem of the file
system to fix and nothing we need to solve in systemd.

Are there filesystems which have such a flag?


Some have the not cleanly unmounted flag, or mount counters for regular
checks


and then comes https://bugzilla.redhat.com/show_bug.cgi?id=1105877



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


Re: [systemd-devel] [PATCH 4/6] make in_addr_from_string() accept ipv4 addresses even when using AF_INET6

2015-03-15 Thread Zbigniew Jędrzejewski-Szmek
On Wed, Mar 11, 2015 at 08:13:47AM -0700, Shawn Landden wrote:
 if we are going to have a function to fix up the deficiencies of
 inet_pton(), better go all the way.
 ---
  src/shared/in-addr-util.c | 17 +
  src/shared/in-addr-util.h |  1 +
  2 files changed, 18 insertions(+)
 
 diff --git a/src/shared/in-addr-util.c b/src/shared/in-addr-util.c
 index ade4012..b7532a6 100644
 --- a/src/shared/in-addr-util.c
 +++ b/src/shared/in-addr-util.c
 @@ -206,7 +206,18 @@ int in_addr_to_string(int family, const union 
 in_addr_union *u, char **ret) {
  return 0;
  }
  
 +struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) {
 +struct in6_addr r = { { {
 +0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xFF, 0xFF, 0, 0, 0, 0
 +} } };
 +
 +r.s6_addr32[3] = ipv4.s_addr;
 +
 +return r;
 +}
 +
  int in_addr_from_string(int family, const char *s, void *ret) {
 +struct in_addr v4mapped;
  
  assert(s);
  assert(ret);
 @@ -214,6 +225,12 @@ int in_addr_from_string(int family, const char *s, void 
 *ret) {
  if (!IN_SET(family, AF_INET, AF_INET6))
  return -EAFNOSUPPORT;
  
 +if (family == AF_INET6)
 +if (inet_pton(AF_INET, s, v4mapped) == 1) {
 +*((struct in6_addr *)ret) = 
 in_addr_to_in6_addr(v4mapped);
 +return 0;
 +}
 +
Hm, IIUC, an ipv6 address was specified, but the user gave an ipv4 address.
Automatically converting seems like a way to entice confusion...

Zbyszek

  errno = 0;
  if (inet_pton(family, s, ret) = 0)
  return errno ? -errno : -EINVAL;
 diff --git a/src/shared/in-addr-util.h b/src/shared/in-addr-util.h
 index 8b3a07c..f140ec9 100644
 --- a/src/shared/in-addr-util.h
 +++ b/src/shared/in-addr-util.h
 @@ -37,6 +37,7 @@ int in_addr_equal(int family, const union in_addr_union *a, 
 const union in_addr_
  int in_addr_prefix_intersect(int family, const union in_addr_union *a, 
 unsigned aprefixlen, const union in_addr_union *b, unsigned bprefixlen);
  int in_addr_prefix_next(int family, union in_addr_union *u, unsigned 
 prefixlen);
  int in_addr_to_string(int family, const union in_addr_union *u, char **ret);
 +struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) _const_;
  int in_addr_from_string(int family, const char *s, void *ret);
  int in_addr_from_string_auto(const char *s, int *family, union in_addr_union 
 *ret);
  unsigned char in_addr_netmask_to_prefixlen(const struct in_addr *addr);
 -- 
 2.2.1.209.g41e5f3a
 
 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 2/2] fsck: Add support for EFI variable based fsck indication

2015-03-15 Thread Zbigniew Jędrzejewski-Szmek
On Sun, Mar 15, 2015 at 08:50:03PM +0100, Kay Sievers wrote:
 On Sun, Mar 15, 2015 at 8:08 PM, Zbigniew Jędrzejewski-Szmek
 zbys...@in.waw.pl wrote:
  On Sun, Mar 15, 2015 at 07:58:35PM +0100, Kay Sievers wrote:
  If the filesytem is too dumb to have that info in the superblock flags
  to store, to request a forced fsck, it is the problem of the file
  system to fix and nothing we need to solve in systemd.
  Are there filesystems which have such a flag?
 
 Some have the not cleanly unmounted flag, or mount counters for regular
 checks. I don't know any details. Maybe this flag or counter could be
 mis-used to force such a check.
So this used to to be possible for ext4, by setting mount count one
below the mount limit. But mount limit now defaults to disabled, so
this stopped being possible.

Also, this suffers from the same problem that setting the flag requires modyfing
the filesystem. Just touching the super block is not that bad, but still,
doing it externally seems nicer.

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


Re: [systemd-devel] [PATCH 2/2] fsck: Add support for EFI variable based fsck indication

2015-03-15 Thread Kay Sievers
On Sun, Mar 15, 2015 at 4:44 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Sun, Mar 15, 2015 at 03:23:19PM +0100, Kay Sievers wrote:
 On Sun, Mar 15, 2015 at 11:56 AM, Jan Janssen medhe...@web.de wrote:
  ---
   man/systemctl.xml  | 26 
   shell-completion/bash/systemctl.in |  8 -
   shell-completion/zsh/_systemctl.in |  2 ++
   src/fsck/fsck.c| 63 
  +
   src/shared/efivars.h   | 21 +++--
   src/systemctl/systemctl.c  | 64 
  +-

 Fsck is really something from the past, it should not be extended
 beyond its current support.
 Filesystems which require fsck are in wide use and will be in wide use
 for the forseeable future.

It is legacy and does not need new features. It worked in the past and
will continue to work in the future, but it does not need new
questionable and possibly unreliable or dangerous features. The recent
merging of fsckd was already the wrong thing to do.

 the kernel command line should be sufficient enough.
 The kernel command line is not a good fit for a few reasons.

The kernel commandline woked fine in the past and will be fine today,
especially for such a legacy feature.

 and yet another is that changing the commandline is a problem in the
 scheme in which kernel+initrd+cmdline are signed together.

Such kernels will just not be used with a rootfs that needs fsck. It
makes no sense to sign andn seal the kernel for a legacy unsigned file
system.

 EFI variables are the right solution for EFI systems.

No, they are absolutely not. Changing the EFI flash comes with
unpredictable risks, the flash is not meant to or designed for be
written to during any normal operation.

To avoid any possible misunderstanding here:

Systemd will not use the fragile EFI flash store to configure services
or request system operation modes. The kernel command line is good
enough here.

You will not apply this patch.

 Using non-volatile EFI variables for normal system operations does not
 sound right, we should not start using that fragile subsystem for
 things like this
 Volatile indeed sounds better. Are there volatile variables which are survive
 a reboot but are then erased automatically?

No, there is only volatile (in RAM from the bootloader to the OS), or
non-volatile (writing to the flash).

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


Re: [systemd-devel] Enabling timesyncd in virtual machines

2015-03-15 Thread Kay Sievers
On Fri, Mar 13, 2015 at 8:25 PM, Michael Marineau
michael.marin...@coreos.com wrote:
 Greetings,

 Currently systemd-timesyncd.service includes
 ConditionVirtualization=no, disabling it in both containers and
 virtual machines. Each VM platform tends to deal with or ignore the
 time problem in their own special ways, KVM/QEMU has the kernel time
 source kvm-clock, Xen has had different schemes over the years, VMware
 expects a userspace daemon sync the clock, and other platforms are
 content to drift with the wind as far as I can tell.

 I don't know of a robust way to know if a platform needs a little
 extra help from userspace to keep the clock sane or not but it seems
 generally safer to try than to risk drifting. Does anyone know of a
 reason to leave timesyncd off by default? Otherwise switching to
 ConditionVirtualization=!container should be reasonable.

Sounds reasonable. Until someone has a better or clear idea how to
solve that reliably, I turned the option around.

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


Re: [systemd-devel] [PATCH 2/2] fsck: Add support for EFI variable based fsck indication

2015-03-15 Thread Zbigniew Jędrzejewski-Szmek
On Sun, Mar 15, 2015 at 06:48:24PM +0100, Kay Sievers wrote:
 On Sun, Mar 15, 2015 at 4:44 PM, Zbigniew Jędrzejewski-Szmek
 zbys...@in.waw.pl wrote:
  On Sun, Mar 15, 2015 at 03:23:19PM +0100, Kay Sievers wrote:
  On Sun, Mar 15, 2015 at 11:56 AM, Jan Janssen medhe...@web.de wrote:
   ---
man/systemctl.xml  | 26 
shell-completion/bash/systemctl.in |  8 -
shell-completion/zsh/_systemctl.in |  2 ++
src/fsck/fsck.c| 63 
   +
src/shared/efivars.h   | 21 +++--
src/systemctl/systemctl.c  | 64 
   +-
 
  Fsck is really something from the past, it should not be extended
  beyond its current support.
  Filesystems which require fsck are in wide use and will be in wide use
  for the forseeable future.
 
 It is legacy and does not need new features. It worked in the past and
 will continue to work in the future, but it does not need new
 questionable and possibly unreliable or dangerous features. The recent
 merging of fsckd was already the wrong thing to do.
Calling it legacy does not make it go away. If we had a stable non-fsck-using
filesystem available, we could start discussing removing fsck support.
But we don't. It's one thing to remove stuff once we have something
better, and completely different to remove support for widely used
things.

  the kernel command line should be sufficient enough.
  The kernel command line is not a good fit for a few reasons.
 
 The kernel commandline woked fine in the past and will be fine today,
 especially for such a legacy feature.
Support for /forcefsck (or whatever it was called) was removed with the
promise to provide a replacement which does not require touching the fs.
Kernel commandline is just too unwieldy for users.

  and yet another is that changing the commandline is a problem in the
  scheme in which kernel+initrd+cmdline are signed together.
 
 Such kernels will just not be used with a rootfs that needs fsck. It
 makes no sense to sign andn seal the kernel for a legacy unsigned file
 system.
OK. 

  EFI variables are the right solution for EFI systems.
 
 No, they are absolutely not. Changing the EFI flash comes with
 unpredictable risks, the flash is not meant to or designed for be
 written to during any normal operation.
Requesting fsck is not a normal operation. If the flash is suitable
to be written whenever the kernel is updated, it should be also OK
to request a fsck through it. For users of many distributions (and
kernel developers certainly), requesting fsck is a much rarer operation.

 To avoid any possible misunderstanding here:
 
 Systemd will not use the fragile EFI flash store to configure services
 or request system operation modes. The kernel command line is good
 enough here.
 
 You will not apply this patch.
I'd prefer to have a discussion and reach conclusions, not the other
way around.

  Using non-volatile EFI variables for normal system operations does not
  sound right, we should not start using that fragile subsystem for
  things like this
  Volatile indeed sounds better. Are there volatile variables which are 
  survive
  a reboot but are then erased automatically?
 
 No, there is only volatile (in RAM from the bootloader to the OS), or
 non-volatile (writing to the flash).
Thank you for the explanation.

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


Re: [systemd-devel] [PATCH 2/2] fsck: Add support for EFI variable based fsck indication

2015-03-15 Thread Andrei Borzenkov
В Sun, 15 Mar 2015 19:48:10 +0100
Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl пишет:

 On Sun, Mar 15, 2015 at 06:48:24PM +0100, Kay Sievers wrote:
  On Sun, Mar 15, 2015 at 4:44 PM, Zbigniew Jędrzejewski-Szmek
  zbys...@in.waw.pl wrote:
   On Sun, Mar 15, 2015 at 03:23:19PM +0100, Kay Sievers wrote:
   On Sun, Mar 15, 2015 at 11:56 AM, Jan Janssen medhe...@web.de wrote:
---
 man/systemctl.xml  | 26 
 shell-completion/bash/systemctl.in |  8 -
 shell-completion/zsh/_systemctl.in |  2 ++
 src/fsck/fsck.c| 63 
+
 src/shared/efivars.h   | 21 +++--
 src/systemctl/systemctl.c  | 64 
+-
  
   Fsck is really something from the past, it should not be extended
   beyond its current support.
   Filesystems which require fsck are in wide use and will be in wide use
   for the forseeable future.
  
  It is legacy and does not need new features. It worked in the past and
  will continue to work in the future, but it does not need new
  questionable and possibly unreliable or dangerous features. The recent
  merging of fsckd was already the wrong thing to do.
 Calling it legacy does not make it go away. If we had a stable non-fsck-using
 filesystem available, we could start discussing removing fsck support.
 But we don't. It's one thing to remove stuff once we have something
 better, and completely different to remove support for widely used
 things.
 
   the kernel command line should be sufficient enough.
   The kernel command line is not a good fit for a few reasons.
  
  The kernel commandline woked fine in the past and will be fine today,
  especially for such a legacy feature.
 Support for /forcefsck (or whatever it was called) was removed with the
 promise to provide a replacement which does not require touching the fs.
 Kernel commandline is just too unwieldy for users.
 

Why? If you need to run fsck on root filesystem you need console
access. Running it blind makes no sense. If you have console access you
can also modify kernel command line. Except when you are using sealed
bundle ... but it is explicitly excluded here.

   and yet another is that changing the commandline is a problem in the
   scheme in which kernel+initrd+cmdline are signed together.
  
  Such kernels will just not be used with a rootfs that needs fsck. It
  makes no sense to sign andn seal the kernel for a legacy unsigned file
  system.
 OK. 
 
   EFI variables are the right solution for EFI systems.
  
  No, they are absolutely not. Changing the EFI flash comes with
  unpredictable risks, the flash is not meant to or designed for be
  written to during any normal operation.
 Requesting fsck is not a normal operation. If the flash is suitable
 to be written whenever the kernel is updated, it should be also OK
 to request a fsck through it. For users of many distributions (and
 kernel developers certainly), requesting fsck is a much rarer operation.
 

That is rather an argument to *not* store kernel in ESP :)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 4/6] make in_addr_from_string() accept ipv4 addresses even when using AF_INET6

2015-03-15 Thread Shawn Landden
On Sun, Mar 15, 2015 at 1:07 PM, Zbigniew Jędrzejewski-Szmek 
zbys...@in.waw.pl wrote:

 On Wed, Mar 11, 2015 at 08:13:47AM -0700, Shawn Landden wrote:
  if we are going to have a function to fix up the deficiencies of
  inet_pton(), better go all the way.
  ---
   src/shared/in-addr-util.c | 17 +
   src/shared/in-addr-util.h |  1 +
   2 files changed, 18 insertions(+)
 
  diff --git a/src/shared/in-addr-util.c b/src/shared/in-addr-util.c
  index ade4012..b7532a6 100644
  --- a/src/shared/in-addr-util.c
  +++ b/src/shared/in-addr-util.c
  @@ -206,7 +206,18 @@ int in_addr_to_string(int family, const union
 in_addr_union *u, char **ret) {
   return 0;
   }
 
  +struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) {
  +struct in6_addr r = { { {
  +0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xFF, 0xFF, 0, 0, 0, 0
  +} } };
  +
  +r.s6_addr32[3] = ipv4.s_addr;
  +
  +return r;
  +}
  +
   int in_addr_from_string(int family, const char *s, void *ret) {
  +struct in_addr v4mapped;
 
   assert(s);
   assert(ret);
  @@ -214,6 +225,12 @@ int in_addr_from_string(int family, const char *s,
 void *ret) {
   if (!IN_SET(family, AF_INET, AF_INET6))
   return -EAFNOSUPPORT;
 
  +if (family == AF_INET6)
  +if (inet_pton(AF_INET, s, v4mapped) == 1) {
  +*((struct in6_addr *)ret) =
 in_addr_to_in6_addr(v4mapped);
  +return 0;
  +}
  +
 Hm, IIUC, an ipv6 address was specified, but the user gave an ipv4 address.
 Automatically converting seems like a way to entice confusion...

 Except that the linux kernel supports binding to ipv4 this way, (see
ipv6(7)).

In the inet_pton man page:

BUGS
   AF_INET6 does not recognize IPv4 addresses.  An explicit IPv4-mapped
IPv6 address must be supplied in src instead.


Zbyszek

   errno = 0;
   if (inet_pton(family, s, ret) = 0)
   return errno ? -errno : -EINVAL;
  diff --git a/src/shared/in-addr-util.h b/src/shared/in-addr-util.h
  index 8b3a07c..f140ec9 100644
  --- a/src/shared/in-addr-util.h
  +++ b/src/shared/in-addr-util.h
  @@ -37,6 +37,7 @@ int in_addr_equal(int family, const union
 in_addr_union *a, const union in_addr_
   int in_addr_prefix_intersect(int family, const union in_addr_union *a,
 unsigned aprefixlen, const union in_addr_union *b, unsigned bprefixlen);
   int in_addr_prefix_next(int family, union in_addr_union *u, unsigned
 prefixlen);
   int in_addr_to_string(int family, const union in_addr_union *u, char
 **ret);
  +struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) _const_;
   int in_addr_from_string(int family, const char *s, void *ret);
   int in_addr_from_string_auto(const char *s, int *family, union
 in_addr_union *ret);
   unsigned char in_addr_netmask_to_prefixlen(const struct in_addr *addr);
  --
  2.2.1.209.g41e5f3a
 
  ___
  systemd-devel mailing list
  systemd-devel@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/systemd-devel
 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel




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


Re: [systemd-devel] [v3 1/4] man: these binaries are internal APIs

2015-03-15 Thread Zbigniew Jędrzejewski-Szmek
On Fri, Feb 27, 2015 at 05:04:21PM -0800, Shawn Landden wrote:
 ---
  man/systemd-halt.service.xml  | 1 -
  man/systemd-shutdownd.service.xml | 1 -
  man/systemd-suspend.service.xml   | 1 -
  3 files changed, 3 deletions(-)
 
 diff --git a/man/systemd-halt.service.xml b/man/systemd-halt.service.xml
 index c94e2a1..7e7f8f2 100644
 --- a/man/systemd-halt.service.xml
 +++ b/man/systemd-halt.service.xml
 @@ -56,7 +56,6 @@
  parafilenamesystemd-poweroff.service/filename/para
  parafilenamesystemd-reboot.service/filename/para
  parafilenamesystemd-kexec.service/filename/para
 -parafilename/usr/lib/systemd/systemd-shutdown/filename/para
/refsynopsisdiv
Hm, I was going back and forth on this patch... You are certainly right that
the user should never call those binaries directly. But we do list other
binaries in the documentation in similar circumstances. And I think we should
continue to do so, for two reasons:
1. when trying to understand how everythig works it makes it easier to start
with the documentation and see what binaries are used to do various things.
2. when looking for something related, it makes it easier to distinguish
various commands with a similar name. (This systemd-sleep binary is not
the binary you're looking for. :))

Maybe we should move the binary to FILES section and mention that it should
not be called directly. But then this should be done also for systemd-journald
and others which are never called directly.

Zbyszek

  
refsect1
 diff --git a/man/systemd-shutdownd.service.xml 
 b/man/systemd-shutdownd.service.xml
 index 756949c..051d2ab 100644
 --- a/man/systemd-shutdownd.service.xml
 +++ b/man/systemd-shutdownd.service.xml
 @@ -52,7 +52,6 @@
refsynopsisdiv
  parafilenamesystemd-shutdownd.service/filename/para
  parafilenamesystemd-shutdownd.socket/filename/para
 -parafilename/usr/lib/systemd/systemd-shutdownd/filename/para
/refsynopsisdiv
  
refsect1
 diff --git a/man/systemd-suspend.service.xml b/man/systemd-suspend.service.xml
 index a8beb86..8e3df5f 100644
 --- a/man/systemd-suspend.service.xml
 +++ b/man/systemd-suspend.service.xml
 @@ -56,7 +56,6 @@
  parafilenamesystemd-suspend.service/filename/para
  parafilenamesystemd-hibernate.service/filename/para
  parafilenamesystemd-hybrid-sleep.service/filename/para
 -parafilename/usr/lib/systemd/system-sleep/filename/para
/refsynopsisdiv
  
refsect1
 -- 
 2.2.1.209.g41e5f3a
 
 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 2/2] fsck: Add support for EFI variable based fsck indication

2015-03-15 Thread Kay Sievers
On Sun, Mar 15, 2015 at 7:48 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Sun, Mar 15, 2015 at 06:48:24PM +0100, Kay Sievers wrote:

 It is legacy and does not need new features. It worked in the past and
 will continue to work in the future, but it does not need new
 questionable and possibly unreliable or dangerous features. The recent
 merging of fsckd was already the wrong thing to do.
 Calling it legacy does not make it go away. If we had a stable non-fsck-using
 filesystem available, we could start discussing removing fsck support.
 But we don't. It's one thing to remove stuff once we have something
 better, and completely different to remove support for widely used
 things.

Nobody talks about things going away, we just should not add more
non-trivial legacy support, that is all.

  the kernel command line should be sufficient enough.
  The kernel command line is not a good fit for a few reasons.

 The kernel commandline woked fine in the past and will be fine today,
 especially for such a legacy feature.
 Support for /forcefsck (or whatever it was called) was removed with the
 promise to provide a replacement which does not require touching the fs.
 Kernel commandline is just too unwieldy for users.

Writing to the file system content to request a check, which would
happen when things are already inconsistent, is a really stupid idea.

If the filesytem is too dumb to have that info in the superblock flags
to store, to request a forced fsck, it is the problem of the file
system to fix and nothing we need to solve in systemd.

 No, they are absolutely not. Changing the EFI flash comes with
 unpredictable risks, the flash is not meant to or designed for be
 written to during any normal operation.
 Requesting fsck is not a normal operation.

It is just a normal system operation. It needs to be fixed properly if
needed, not with dirty work-arounds like this.

 If the flash is suitable
 to be written whenever the kernel is updated, it should be also OK
 to request a fsck through it. For users of many distributions (and
 kernel developers certainly), requesting fsck is a much rarer operation.

Nobody would write to the flash on kernel updates, we only possibly
write to the ESP filesystem. The flash is not meant for such use
cases, it is known to brick all sorts of machines, and not to be
mis-used for such features.

 To avoid any possible misunderstanding here:

 Systemd will not use the fragile EFI flash store to configure services
 or request system operation modes. The kernel command line is good
 enough here.

 You will not apply this patch.
 I'd prefer to have a discussion and reach conclusions, not the other
 way around.

Sorry, there is nothing to discuss, systemd will not mis-use the
fragile firmware flash for normal operations, and especially not to
support legacy features.

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


Re: [systemd-devel] Enabling timesyncd in virtual machines

2015-03-15 Thread Michael Biebl
This was suposed to go to the list.

2015-03-13 21:25 GMT+01:00 Michael Biebl mbi...@gmail.com:
 2015-03-13 20:25 GMT+01:00 Michael Marineau michael.marin...@coreos.com:
 I don't know of a robust way to know if a platform needs a little
 extra help from userspace to keep the clock sane or not but it seems
 generally safer to try than to risk drifting. Does anyone know of a
 reason to leave timesyncd off by default? Otherwise switching to
 ConditionVirtualization=!container should be reasonable.

 Makes sense to me.
 Related to that, see https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=762343





-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 2/2] fsck: Add support for EFI variable based fsck indication

2015-03-15 Thread Kay Sievers
On Sun, Mar 15, 2015 at 7:59 PM, Andrei Borzenkov arvidj...@gmail.com wrote:
 В Sun, 15 Mar 2015 19:48:10 +0100
 Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl пишет:

  No, they are absolutely not. Changing the EFI flash comes with
  unpredictable risks, the flash is not meant to or designed for be
  written to during any normal operation.
 Requesting fsck is not a normal operation. If the flash is suitable
 to be written whenever the kernel is updated, it should be also OK
 to request a fsck through it. For users of many distributions (and
 kernel developers certainly), requesting fsck is a much rarer operation.


 That is rather an argument to *not* store kernel in ESP :)

Storing the kernel in the ESP has no relation to EFI variables. EFI
variables are only changed if the bootloader gets installed, not when
the kernel is updated.

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


Re: [systemd-devel] Enabling timesyncd in virtual machines

2015-03-15 Thread Kay Sievers
On Sun, Mar 15, 2015 at 7:50 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Sun, Mar 15, 2015 at 07:47:46PM +0100, Kay Sievers wrote:
 On Fri, Mar 13, 2015 at 8:25 PM, Michael Marineau
 michael.marin...@coreos.com wrote:
  Greetings,
 
  Currently systemd-timesyncd.service includes
  ConditionVirtualization=no, disabling it in both containers and
  virtual machines. Each VM platform tends to deal with or ignore the
  time problem in their own special ways, KVM/QEMU has the kernel time
  source kvm-clock, Xen has had different schemes over the years, VMware
  expects a userspace daemon sync the clock, and other platforms are
  content to drift with the wind as far as I can tell.
 
  I don't know of a robust way to know if a platform needs a little
  extra help from userspace to keep the clock sane or not but it seems
  generally safer to try than to risk drifting. Does anyone know of a
  reason to leave timesyncd off by default? Otherwise switching to
  ConditionVirtualization=!container should be reasonable.

 Sounds reasonable. Until someone has a better or clear idea how to
 solve that reliably, I turned the option around.
 What about adding

 ConditionVirtualization=!kvm ?

Even there, things like leap seconds are not covered. And I'm not sure
if the paravirtualized clock is always configured. I guess it's
safest bet to just keep timesyncd running, until someone knows for
sure when we can optimize it away for which environment.

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


Re: [systemd-devel] [PATCH 2/2] fsck: Add support for EFI variable based fsck indication

2015-03-15 Thread Kay Sievers
On Sun, Mar 15, 2015 at 8:08 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Sun, Mar 15, 2015 at 07:58:35PM +0100, Kay Sievers wrote:
 If the filesytem is too dumb to have that info in the superblock flags
 to store, to request a forced fsck, it is the problem of the file
 system to fix and nothing we need to solve in systemd.
 Are there filesystems which have such a flag?

Some have the not cleanly unmounted flag, or mount counters for regular
checks. I don't know any details. Maybe this flag or counter could be
mis-used to force such a check.

In any case, the superblock and corresponding fsck tool would be the
right place to put such a force check flag/logic, the info would be
stored locally with the actual filesystem, but would not require a r/w
mount to place that information.

Working around such missing basic filesystem features in systemd code
does not sound like the right approach.

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


Re: [systemd-devel] [PATCH 4/6] make in_addr_from_string() accept ipv4 addresses even when using AF_INET6

2015-03-15 Thread Zbigniew Jędrzejewski-Szmek
On Sun, Mar 15, 2015 at 01:28:05PM -0700, Shawn Landden wrote:
 On Sun, Mar 15, 2015 at 1:07 PM, Zbigniew Jędrzejewski-Szmek 
 zbys...@in.waw.pl wrote:
 
  On Wed, Mar 11, 2015 at 08:13:47AM -0700, Shawn Landden wrote:
   if we are going to have a function to fix up the deficiencies of
   inet_pton(), better go all the way.
   ---
src/shared/in-addr-util.c | 17 +
src/shared/in-addr-util.h |  1 +
2 files changed, 18 insertions(+)
  
   diff --git a/src/shared/in-addr-util.c b/src/shared/in-addr-util.c
   index ade4012..b7532a6 100644
   --- a/src/shared/in-addr-util.c
   +++ b/src/shared/in-addr-util.c
   @@ -206,7 +206,18 @@ int in_addr_to_string(int family, const union
  in_addr_union *u, char **ret) {
return 0;
}
  
   +struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) {
   +struct in6_addr r = { { {
   +0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xFF, 0xFF, 0, 0, 0, 0
   +} } };
   +
   +r.s6_addr32[3] = ipv4.s_addr;
   +
   +return r;
   +}
   +
int in_addr_from_string(int family, const char *s, void *ret) {
   +struct in_addr v4mapped;
  
assert(s);
assert(ret);
   @@ -214,6 +225,12 @@ int in_addr_from_string(int family, const char *s,
  void *ret) {
if (!IN_SET(family, AF_INET, AF_INET6))
return -EAFNOSUPPORT;
  
   +if (family == AF_INET6)
   +if (inet_pton(AF_INET, s, v4mapped) == 1) {
   +*((struct in6_addr *)ret) =
  in_addr_to_in6_addr(v4mapped);
   +return 0;
   +}
   +
  Hm, IIUC, an ipv6 address was specified, but the user gave an ipv4 address.
  Automatically converting seems like a way to entice confusion...
 
  Except that the linux kernel supports binding to ipv4 this way, (see
 ipv6(7)).
 
 In the inet_pton man page:
 
 BUGS
AF_INET6 does not recognize IPv4 addresses.  An explicit IPv4-mapped
 IPv6 address must be supplied in src instead.
Right, but looking at how in_addr_from_string is used, we usually call it
with an explicit family, and if it parses as ipv4, assume the family is ipv4,
otherwise parse it as ipv6. So your change would either be a noop (if
the check for ipv4 syntax was already done before), or would mess up this
detection.

What scenario are you trying to solve?

Zbyszek

 
 
 Zbyszek
 
errno = 0;
if (inet_pton(family, s, ret) = 0)
return errno ? -errno : -EINVAL;
   diff --git a/src/shared/in-addr-util.h b/src/shared/in-addr-util.h
   index 8b3a07c..f140ec9 100644
   --- a/src/shared/in-addr-util.h
   +++ b/src/shared/in-addr-util.h
   @@ -37,6 +37,7 @@ int in_addr_equal(int family, const union
  in_addr_union *a, const union in_addr_
int in_addr_prefix_intersect(int family, const union in_addr_union *a,
  unsigned aprefixlen, const union in_addr_union *b, unsigned bprefixlen);
int in_addr_prefix_next(int family, union in_addr_union *u, unsigned
  prefixlen);
int in_addr_to_string(int family, const union in_addr_union *u, char
  **ret);
   +struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) _const_;
int in_addr_from_string(int family, const char *s, void *ret);
int in_addr_from_string_auto(const char *s, int *family, union
  in_addr_union *ret);
unsigned char in_addr_netmask_to_prefixlen(const struct in_addr *addr);
   --
   2.2.1.209.g41e5f3a
  
   ___
   systemd-devel mailing list
   systemd-devel@lists.freedesktop.org
   http://lists.freedesktop.org/mailman/listinfo/systemd-devel
  ___
  systemd-devel mailing list
  systemd-devel@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/systemd-devel
 
 
 
 
 -- 
 Shawn Landden
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 4/6] make in_addr_from_string() accept ipv4 addresses even when using AF_INET6

2015-03-15 Thread Shawn Landden
On Sun, Mar 15, 2015 at 1:36 PM, Zbigniew Jędrzejewski-Szmek 
zbys...@in.waw.pl wrote:

 On Sun, Mar 15, 2015 at 01:28:05PM -0700, Shawn Landden wrote:
  On Sun, Mar 15, 2015 at 1:07 PM, Zbigniew Jędrzejewski-Szmek 
  zbys...@in.waw.pl wrote:
 
   On Wed, Mar 11, 2015 at 08:13:47AM -0700, Shawn Landden wrote:
if we are going to have a function to fix up the deficiencies of
inet_pton(), better go all the way.
---
 src/shared/in-addr-util.c | 17 +
 src/shared/in-addr-util.h |  1 +
 2 files changed, 18 insertions(+)
   
diff --git a/src/shared/in-addr-util.c b/src/shared/in-addr-util.c
index ade4012..b7532a6 100644
--- a/src/shared/in-addr-util.c
+++ b/src/shared/in-addr-util.c
@@ -206,7 +206,18 @@ int in_addr_to_string(int family, const union
   in_addr_union *u, char **ret) {
 return 0;
 }
   
+struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) {
+struct in6_addr r = { { {
+0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xFF, 0xFF, 0, 0, 0, 0
+} } };
+
+r.s6_addr32[3] = ipv4.s_addr;
+
+return r;
+}
+
 int in_addr_from_string(int family, const char *s, void *ret) {
+struct in_addr v4mapped;
   
 assert(s);
 assert(ret);
@@ -214,6 +225,12 @@ int in_addr_from_string(int family, const char
 *s,
   void *ret) {
 if (!IN_SET(family, AF_INET, AF_INET6))
 return -EAFNOSUPPORT;
   
+if (family == AF_INET6)
+if (inet_pton(AF_INET, s, v4mapped) == 1) {
+*((struct in6_addr *)ret) =
   in_addr_to_in6_addr(v4mapped);
+return 0;
+}
+
   Hm, IIUC, an ipv6 address was specified, but the user gave an ipv4
 address.
   Automatically converting seems like a way to entice confusion...
  
   Except that the linux kernel supports binding to ipv4 this way, (see
  ipv6(7)).
 
  In the inet_pton man page:
 
  BUGS
 AF_INET6 does not recognize IPv4 addresses.  An explicit
 IPv4-mapped
  IPv6 address must be supplied in src instead.
 Right, but looking at how in_addr_from_string is used, we usually call it
 with an explicit family, and if it parses as ipv4, assume the family is
 ipv4,
 otherwise parse it as ipv6. So your change would either be a noop (if
 the check for ipv4 syntax was already done before), or would mess up this
 detection.

 What scenario are you trying to solve?

 nothing is particular, I was just trying to give myself a reason not to
refactor these to just call inet_pton() directly as without this change the
wrapper function does nothing except check for NULL.

 Zbyszek

 
 
  Zbyszek
  
 errno = 0;
 if (inet_pton(family, s, ret) = 0)
 return errno ? -errno : -EINVAL;
diff --git a/src/shared/in-addr-util.h b/src/shared/in-addr-util.h
index 8b3a07c..f140ec9 100644
--- a/src/shared/in-addr-util.h
+++ b/src/shared/in-addr-util.h
@@ -37,6 +37,7 @@ int in_addr_equal(int family, const union
   in_addr_union *a, const union in_addr_
 int in_addr_prefix_intersect(int family, const union in_addr_union
 *a,
   unsigned aprefixlen, const union in_addr_union *b, unsigned
 bprefixlen);
 int in_addr_prefix_next(int family, union in_addr_union *u, unsigned
   prefixlen);
 int in_addr_to_string(int family, const union in_addr_union *u, char
   **ret);
+struct in6_addr in_addr_to_in6_addr(struct in_addr ipv4) _const_;
 int in_addr_from_string(int family, const char *s, void *ret);
 int in_addr_from_string_auto(const char *s, int *family, union
   in_addr_union *ret);
 unsigned char in_addr_netmask_to_prefixlen(const struct in_addr
 *addr);
--
2.2.1.209.g41e5f3a
   
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
   ___
   systemd-devel mailing list
   systemd-devel@lists.freedesktop.org
   http://lists.freedesktop.org/mailman/listinfo/systemd-devel
  
 
 
 
  --
  Shawn Landden
 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel




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


Re: [systemd-devel] [PATCH] network: add UseNTP DHCP option

2015-03-15 Thread Zbigniew Jędrzejewski-Szmek
On Fri, Mar 13, 2015 at 12:01:29PM -0700, Michael Marineau wrote:
 Despite having the internal logic in place to enable/disable using NTP
 servers provided by DHCP the network config didn't expose the option.
Applied.

Zbyszek

 ---
  man/systemd.network.xml  | 8 
  src/network/networkd-network-gperf.gperf | 1 +
  2 files changed, 9 insertions(+)
 
 diff --git a/man/systemd.network.xml b/man/systemd.network.xml
 index 5d7518b..95be132 100644
 --- a/man/systemd.network.xml
 +++ b/man/systemd.network.xml
 @@ -515,6 +515,14 @@
/listitem
  /varlistentry
  varlistentry
 +  termvarnameUseNTP=/varname/term
 +  listitem
 +paraWhen true (the default), the NTP servers received
 +from the DHCP server will be used by systemd-timesyncd
 +and take precedence over any statically configured ones./para
 +  /listitem
 +/varlistentry
 +varlistentry
termvarnameUseMTU=/varname/term
listitem
  paraWhen true, the interface maximum transmission unit
 diff --git a/src/network/networkd-network-gperf.gperf 
 b/src/network/networkd-network-gperf.gperf
 index 93df83a..8abf5bc 100644
 --- a/src/network/networkd-network-gperf.gperf
 +++ b/src/network/networkd-network-gperf.gperf
 @@ -60,6 +60,7 @@ Route.Metric,config_parse_route_priority,   
  0,
  Route.Scope, config_parse_route_scope,   0,  
0
  DHCP.ClientIdentifier,   config_parse_dhcp_client_identifier,0,  
offsetof(Network, dhcp_client_identifier)
  DHCP.UseDNS, config_parse_bool,  0,  
offsetof(Network, dhcp_dns)
 +DHCP.UseNTP, config_parse_bool,  0,  
offsetof(Network, dhcp_ntp)
  DHCP.UseMTU, config_parse_bool,  0,  
offsetof(Network, dhcp_mtu)
  DHCP.UseHostname,config_parse_bool,  0,  
offsetof(Network, dhcp_hostname)
  DHCP.UseDomains, config_parse_bool,  0,  
offsetof(Network, dhcp_domains)
 -- 
 2.0.5
 
 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH] man: standard-conf: change directory reference to wildcard

2015-03-15 Thread alison
From: Alison Chaiken alison_chai...@mentor.com

---
 man/standard-conf.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man/standard-conf.xml b/man/standard-conf.xml
index 36af459..004f53f 100644
--- a/man/standard-conf.xml
+++ b/man/standard-conf.xml
@@ -54,7 +54,7 @@
 directories, and has the lowest precedence; entries in a file in
 any configuration directory override entries in the single
 configuration file. Files in the
-filenamelogind.conf.d//filename configuration subdirectories
+filename*.conf.d//filename configuration subdirectories
 are sorted by their filename in lexicographic order, regardless of
 which of the subdirectories they reside in. If multiple files
 specify the same option, the entry in the file with the
-- 
2.1.4

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


Re: [systemd-devel] [PATCH] vconsole-setup: check error of child process

2015-03-15 Thread Zbigniew Jędrzejewski-Szmek
On Fri, Mar 13, 2015 at 05:47:28PM +, lucas.de.mar...@gmail.com wrote:
 From: Lucas De Marchi lucas.demar...@intel.com
 
 If we don't check the error of the child process, systemd-vconsole-setup
 would exit with 0 even if it could not really setup the console.
Applied.

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


Re: [systemd-devel] [PATCH] man: standard-conf: change directory reference to wildcard

2015-03-15 Thread Zbigniew Jędrzejewski-Szmek
On Sun, Mar 15, 2015 at 04:26:14PM -0700, ali...@she-devel.com wrote:
 From: Alison Chaiken alison_chai...@mentor.com
 
 ---
  man/standard-conf.xml | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/man/standard-conf.xml b/man/standard-conf.xml
 index 36af459..004f53f 100644
 --- a/man/standard-conf.xml
 +++ b/man/standard-conf.xml
 @@ -54,7 +54,7 @@
  directories, and has the lowest precedence; entries in a file in
  any configuration directory override entries in the single
  configuration file. Files in the
 -filenamelogind.conf.d//filename configuration subdirectories
 +filename*.conf.d//filename configuration subdirectories
  are sorted by their filename in lexicographic order, regardless of
  which of the subdirectories they reside in. If multiple files
  specify the same option, the entry in the file with the
Applied.

This text is still a bit heavy... but I don't see an easy wasy to improve it.

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


Re: [systemd-devel] [PATCH] console-getty.service: don't start when /dev/console is missing

2015-03-15 Thread Zbigniew Jędrzejewski-Szmek
Looks reasonable.

Zbyszek

On Fri, Mar 13, 2015 at 12:57:18PM +0100, Lukas Nykryn wrote:
 From: Jan Pazdziora jpazdzi...@redhat.com
 
 Create minimal image which runs systemd
 
FROM rhel7.1
RUN yum install -y /usr/bin/ps
ENV container docker
CMD [ /usr/sbin/init ]
 
 When you run the container without -t, the process
 
/sbin/agetty --noclear --keep-baud console 115200 38400 9600
 
 is not happy and checking the journal in the container, there is a stream of
 
 Mar 13 04:50:15 11bf07f59fff agetty[66]: /dev/console: No such file or 
 directory
 Mar 13 04:50:25 11bf07f59fff systemd[1]: console-getty.service holdoff time 
 over, scheduling restart.
 Mar 13 04:50:25 11bf07f59fff systemd[1]: Stopping Console Getty...
 Mar 13 04:50:25 11bf07f59fff systemd[1]: Starting Console Getty...
 Mar 13 04:50:25 11bf07f59fff systemd[1]: Started Console Getty.
 Mar 13 04:50:25 11bf07f59fff agetty[67]: /dev/console: No such file or 
 directory
 Mar 13 04:50:35 11bf07f59fff systemd[1]: console-getty.service holdoff time 
 over, scheduling restart.
 Mar 13 04:50:35 11bf07f59fff systemd[1]: Stopping Console Getty...
 Mar 13 04:50:35 11bf07f59fff systemd[1]: Starting Console Getty...
 Mar 13 04:50:35 11bf07f59fff systemd[1]: Started Console Getty.
 Mar 13 04:50:35 11bf07f59fff agetty[74]: /dev/console: No such file or 
 directory
 Mar 13 04:50:45 11bf07f59fff systemd[1]: console-getty.service holdoff time 
 over, scheduling restart.
 Mar 13 04:50:45 11bf07f59fff systemd[1]: Stopping Console Getty...
 Mar 13 04:50:45 11bf07f59fff systemd[1]: Starting Console Getty...
 ---
  units/console-getty.service.m4.in | 1 +
  1 file changed, 1 insertion(+)
 
 diff --git a/units/console-getty.service.m4.in 
 b/units/console-getty.service.m4.in
 index 8ac51a4..413d940 100644
 --- a/units/console-getty.service.m4.in
 +++ b/units/console-getty.service.m4.in
 @@ -9,6 +9,7 @@
  Description=Console Getty
  Documentation=man:agetty(8)
  After=systemd-user-sessions.service plymouth-quit-wait.service
 +ConditionPathExists=/dev/console
  m4_ifdef(`HAVE_SYSV_COMPAT',
  After=rc-local.service
  )m4_dnl
 -- 
 1.8.3.1
 
 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] core: don't change removed devices to state tentative [was: Re: [PATCH] unit: When stopping due to BindsTo=, log which unit caused it]

2015-03-15 Thread Zbigniew Jędrzejewski-Szmek
On Fri, Mar 13, 2015 at 08:35:59AM +0100, Martin Pitt wrote:
 Subject: [PATCH] core: don't change removed devices to state tentative
 
 Commit 628c89c introduced the tentative device state, which caused devices 
 to
 go from plugged to tentative on a remove uevent. This breaks the cleanup
 of stale mounts (see commit 3b48ce4), as that only applies to dead devices.
 
 The tentative state only really makes sense on adding a device when we don't
 know where it was coming from (i. e. not from udev). But when we get a device
 removal from udev we definitively know that it's gone, so change the device
 state back to dead as before 628c89c.
Looks good. (Lennart is travelling, so please just go ahead and push.)

Zbyszek

 diff --git a/src/core/device.c b/src/core/device.c
 index 6b489a4..098a000 100644
 --- a/src/core/device.c
 +++ b/src/core/device.c
 @@ -419,7 +419,7 @@ static void device_update_found_one(Device *d, bool add, 
 DeviceFound found, bool
  if (now) {
  if (d-found  DEVICE_FOUND_UDEV)
  device_set_state(d, DEVICE_PLUGGED);
 -else if (d-found != DEVICE_NOT_FOUND)
 +else if (add  d-found != DEVICE_NOT_FOUND)
  device_set_state(d, DEVICE_TENTATIVE);
  else
  device_set_state(d, DEVICE_DEAD);
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH 2/4] Allow systemd-tmpfiles to set the file/directory attributes

2015-03-15 Thread Zbigniew Jędrzejewski-Szmek
On Tue, Mar 10, 2015 at 09:07:41PM +0100, Goffredo Baroncelli wrote:
 Allow systemd-tmpfiles to set the file/directory attributes, like chattr(1)
 does. Two more commands are added: 'H' and 'h' to set the attributes,
 recursively and not.
 ---
  src/tmpfiles/tmpfiles.c | 140 
 
  1 file changed, 140 insertions(+)
 
 diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
 index c948d4d..165605f 100644
 --- a/src/tmpfiles/tmpfiles.c
 +++ b/src/tmpfiles/tmpfiles.c
 @@ -40,6 +40,7 @@
  #include sys/types.h
  #include sys/param.h
  #include sys/xattr.h
 +#include linux/fs.h
  
  #include log.h
  #include util.h
 @@ -90,6 +91,8 @@ typedef enum ItemType {
  ADJUST_MODE = 'm', /* legacy, 'z' is identical to this */
  RELABEL_PATH = 'z',
  RECURSIVE_RELABEL_PATH = 'Z',
 +SET_ATTRIB = 'h',
 +RECURSIVE_SET_ATTRIB = 'H',
  } ItemType;
  
  typedef struct Item {
 @@ -108,12 +111,15 @@ typedef struct Item {
  usec_t age;
  
  dev_t major_minor;
 +int attrib_value;
 +int attrib_mask;
  
  bool uid_set:1;
  bool gid_set:1;
  bool mode_set:1;
  bool age_set:1;
  bool mask_perms:1;
 +bool attrib_set:1;
  
  bool keep_first_level:1;
  
 @@ -762,6 +768,115 @@ static int path_set_acls(Item *item, const char *path) {
  return 0;
  }
  
 +static int get_attrib_from_arg(Item *item) {
 +static const struct { int value; char ch; } attributes[] = {
 +{ FS_NOATIME_FL, 'A' },   /* do not update atime */
 +{ FS_SYNC_FL, 'S' },  /* Synchronous updates */
 +{ FS_DIRSYNC_FL, 'D' },   /* dirsync behaviour (directories 
 only) */
 +{ FS_APPEND_FL, 'a' },/* writes to file may only append 
 */
 +{ FS_COMPR_FL, 'c' }, /* Compress file */
 +{ FS_NODUMP_FL, 'd' },/* do not dump file */
 +{ FS_EXTENT_FL, 'e'}, /* Top of directory hierarchies*/
 +{ FS_IMMUTABLE_FL, 'i' }, /* Immutable file */
 +{ FS_JOURNAL_DATA_FL, 'j' }, /* Reserved for ext3 */
 +{ FS_SECRM_FL, 's' }, /* Secure deletion */
 +{ FS_UNRM_FL, 'u' },  /* Undelete */
 +{ FS_NOTAIL_FL, 't' },/* file tail should not be merged 
 */
 +{ FS_TOPDIR_FL, 'T' },/* Top of directory hierarchies*/
 +{ FS_NOCOW_FL, 'C' }, /* Do not cow file */
 +{ 0, 0 }
 +};
 +char *p = item-argument;
 +enum {
 +MODE_ADD,
 +MODE_DEL,
 +MODE_SET
 +} mode = MODE_ADD;
 +unsigned long value = 0, mask = 0;
 +
 +if (!p) {
 +log_error(\%s\: setting ATTR need an argument, 
 item-path);
 +return -EINVAL;
 +}
 +
 +if (*p == '+') {
 +mode = MODE_ADD;
 +p++;
 +} else if (*p == '-') {
 +mode = MODE_DEL;
 +p++;
 +} else  if (*p == '=') {
 +mode = MODE_SET;
 +p++;
 +}
 +
 +if (!*p  mode != MODE_SET) {
 +log_error(\%s\: setting ATTR: argument is empty, 
 item-path);
 +return -EINVAL;
 +}
 +for (; *p ; p++) {
 +int i;
 +for (i = 0; attributes[i].ch  attributes[i].ch != *p; i++)
 +;
Why not order the table the other way:

static const int attributes[] = {
   [(uint8_t)'A'] = FS_NOATIME_FL,  /* do not update atime */
   ...
};

if ((uint8_t)*p  ELEMENTSOF(attributes) || attributes[(uint8_t)*p] == 
0)
log_error(\%s\: setting ATTR: unknown attr '%c', 
item-path, *p);
return -EINVAL;
}

Not looping is always nicer.

 +if (!attributes[i].ch) {
 +log_error(\%s\: setting ATTR: unknown attr '%c', 
 item-path, *p);
 +return -EINVAL;
 +}
 +if (mode == MODE_ADD || mode == MODE_SET)
 +value |= attributes[i].value;
 +else
 +value = ~attributes[i].value;
 +mask |= attributes[i].value;
 +}
 +
 +if (mode == MODE_SET) {
 +int i;
 +for (i = 0; attributes[i].ch; i++)
 +mask |= attributes[i].value;
This simply FS_NOATIME_FL|FS_SYNC_FL|... Even if the compiler can optimize
that away, it seems better to just write the OR expression in full rather
than loop.

 +}
 +
 +assert(mask);
 +
 +item-attrib_mask = mask;
 +item-attrib_value = value;
 +item-attrib_set = true;
 +
 +return 0;
 +
 +}
 +
 +static int 

Re: [systemd-devel] [PATCH 3/4] Update the man page of tmpfiles.d(5), to document the new h/H command.

2015-03-15 Thread Zbigniew Jędrzejewski-Szmek
On Tue, Mar 10, 2015 at 09:07:42PM +0100, Goffredo Baroncelli wrote:
 Update the man page of tmpfiles.d(5), to document the new h/H command.
 ---
  man/tmpfiles.d.xml | 32 
  1 file changed, 32 insertions(+)
 
 diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
 index 8815bf9..469deeb 100644
 --- a/man/tmpfiles.d.xml
 +++ b/man/tmpfiles.d.xml
 @@ -303,6 +303,37 @@
  /varlistentry
  
  varlistentry
 +  termvarnameh/varname/term
 +  listitemparaSet file/directory attributes. Lines of this type
 +  accept shell-style globs in place of normal path names./para
 +
 +  paraThe format of the argument field is 
 varname[+-=][aAcCdDeijsStTu]
 +  /varname/para
 +
 +  paraThe prefix varname+/varname causes the
 +  attribute(s) to be added; varname-/varname causes the
 +  attribute(s) to be removed; varname=/varname
 +  causes the attributes to set exactly as the following 
 letters./para
What happens if neither of the three prefix lettes is used? This
should be documented.

 +  paraThe letters 'aAcCdDeijsStTu' select the new
literal instead of ''.

 +  attributes for the files, see
 +  citerefentryrefentrytitlechattr/refentrytitle
 +  manvolnum1/manvolnum/citerefentry for further information.
 +  /para
 +  paraPassing only varname=/varname as argument,
 +  reset all the file attributes./para
resets

So, is this description accurate? Operations on the attributes are
explicitly limited to the ones corresponding to the letters above (by
using a mask). But files can have other attributes, and the kernel might
define new attributes as some point. So maybe add a sentence like
When operating on attributes, system-tmpfiles limits itself to the
attributes corresponding to the letters listed above. All other attributes
will be left untouched, even with literal=/literal.

Zbyszek

 +
 +  /listitem
 +/varlistentry
 +
 +varlistentry
 +  termvarnameH/varname/term
 +  listitemparaRecursively set file/directory attributes. Lines
 +  of this type accept shell-style globs in place of normal
 +  path names.
 +  /para/listitem
 +/varlistentry
 +
 +varlistentry
termvarnamea/varname/term
termvarnamea+/varname/term
listitemparaSet POSIX ACLs (access control lists). If
 @@ -529,6 +560,7 @@
citerefentry 
 project='man-pages'refentrytitlesetfattr/refentrytitlemanvolnum1/manvolnum/citerefentry,
citerefentry 
 project='man-pages'refentrytitlesetfacl/refentrytitlemanvolnum1/manvolnum/citerefentry,
citerefentry 
 project='man-pages'refentrytitlegetfacl/refentrytitlemanvolnum1/manvolnum/citerefentry
 +  citerefentry 
 project='man-pages'refentrytitlechattr/refentrytitlemanvolnum1/manvolnum/citerefentry
  /para
/refsect1
  
 -- 
 2.1.4
 
 ___
 systemd-devel mailing list
 systemd-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] journal: Introduce journal-network

2015-03-15 Thread Zbigniew Jędrzejewski-Szmek
On Fri, Mar 13, 2015 at 10:55:42PM +0530, Susant Sahani wrote:
This tiny daemon enables to pull journal entries and push to a UDP
 multicast address in syslog RFC 5424 format. journal-syslog-network runs with 
 own
 user systemd-journal-push. It starts running after the network is up.
Looks very nice. It indeed seems right to do this as a separate daemon.
Some comments below.

 ---
  Makefile-man.am|   8 +
  Makefile.am|  40 ++
  man/systemd-journal-network.service.xml|  84 +
  man/systemd-journal-network.xml| 115 ++
  src/journal-remote/journal-network-conf.c  |  61 
  src/journal-remote/journal-network-conf.h  |  32 ++
  src/journal-remote/journal-network-gperf.gperf |  18 +
  src/journal-remote/journal-network-manager.c   | 481 
 +
  src/journal-remote/journal-network-manager.h   |  70 
  src/journal-remote/journal-network-proto.c | 218 +++
  src/journal-remote/journal-network.c   | 218 +++
  src/journal-remote/journal-network.conf.in |   2 +
  units/systemd-journal-network.service.in   |  19 +
  13 files changed, 1366 insertions(+)
  create mode 100644 man/systemd-journal-network.service.xml
  create mode 100644 man/systemd-journal-network.xml
  create mode 100644 src/journal-remote/journal-network-conf.c
  create mode 100644 src/journal-remote/journal-network-conf.h
  create mode 100644 src/journal-remote/journal-network-gperf.gperf
  create mode 100644 src/journal-remote/journal-network-manager.c
  create mode 100644 src/journal-remote/journal-network-manager.h
  create mode 100644 src/journal-remote/journal-network-proto.c
  create mode 100644 src/journal-remote/journal-network.c
  create mode 100644 src/journal-remote/journal-network.conf.in
  create mode 100644 units/systemd-journal-network.service.in
 
 diff --git a/Makefile-man.am b/Makefile-man.am
 index 7a9612e..efd0cbc 100644
 --- a/Makefile-man.am
 +++ b/Makefile-man.am
 @@ -1357,6 +1357,14 @@ man/systemd-journal-gatewayd.socket.html: 
 man/systemd-journal-gatewayd.service.h
  
  endif
  
 +MANPAGES += \
 +man/systemd-journal-network.service.8 \
 +man/systemd-journal-network.8
 +MANPAGES_ALIAS += \
 +man/systemd-journal-network.8
 +man/systemd-journal-network.8: man/systemd-journal-network.service.8
 +man/systemd-journal-network.html: man/systemd-journal-network.service.html
 +
  if HAVE_MYHOSTNAME
  MANPAGES += \
   man/nss-myhostname.8
 diff --git a/Makefile.am b/Makefile.am
 index 856accb..ad1dff5 100644
 --- a/Makefile.am
 +++ b/Makefile.am
 @@ -4336,6 +4336,46 @@ EXTRA_DIST += \
   src/journal-remote/journal-upload.conf.in
  endif
  
 +rootlibexec_PROGRAMS += \
 + systemd-journal-network
I think this name will be confusing. Why not systemd-journal-syslog
or systemd-journal-multicast? Network is rather generic, and we already
have three other network-and-journal-related daemons.

 +
 +systemd_journal_network_SOURCES = \
 + src/journal-remote/journal-network-manager.h \
 + src/journal-remote/journal-network-manager.c \
 + src/journal-remote/journal-network-conf.h \
 + src/journal-remote/journal-network-conf.c \
 + src/journal-remote/journal-network-proto.c \
 + src/journal-remote/journal-network.c
 +
 +nodist_systemd_journal_network_SOURCES = \
 + src/journal-remote/journal-network-gperf.c
 +
 +EXTRA_DIST += \
 +src/journal-remote/journal-network-gperf.gperf
 +
 +CLEANFILES += \
 +src/journal-remote/journal-network-gperf.c
 +
 +systemd_journal_network_LDADD = \
 + libsystemd-internal.la \
 + libsystemd-journal-internal.la \
 + libsystemd-shared.la
 +
 +nodist_systemunit_DATA += \
 + units/systemd-journal-network.service
 +
 +EXTRA_DIST += \
 + units/systemd-journal-network.service.in
 +
 +nodist_pkgsysconf_DATA += \
 + src/journal-remote/journal-network.conf
 +
 +EXTRA_DIST += \
 + src/journal-remote/journal-network.conf.in
 +
 +CLEANFILES += \
 + src/journal-remote/journal-network.conf
You can drop that, CLEANFILES in now generated semi-automatically
in git.

  # using _CFLAGS = in the conditional below would suppress AM_CFLAGS
  journalctl_CFLAGS = \
   $(AM_CFLAGS)
 diff --git a/man/systemd-journal-network.service.xml 
 b/man/systemd-journal-network.service.xml
 new file mode 100644
 index 000..47a5b3e
 --- /dev/null
 +++ b/man/systemd-journal-network.service.xml
 @@ -0,0 +1,84 @@
 +?xml version='1.0'? !--*- Mode: nxml; nxml-child-indent: 2; 
 indent-tabs-mode: nil -*--
 +!DOCTYPE refentry PUBLIC -//OASIS//DTD DocBook XML V4.2//EN
 +http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd;
 +
 +!--
 +This file is part of systemd.
 +
 +Copyright 2015 Susant Sahani
 +
 +systemd is free software; you can redistribute it and/or modify it
 +under the terms of the GNU Lesser General Public License as published by
 +the Free Software Foundation; 

Re: [systemd-devel] [PATCH] journal: Introduce journal-network

2015-03-15 Thread Susant Sahani

Hi Zbigniew,
 Thanks for the review.

On 03/16/2015 07:47 AM, Zbigniew Jędrzejewski-Szmek wrote:

On Fri, Mar 13, 2015 at 10:55:42PM +0530, Susant Sahani wrote:

This tiny daemon enables to pull journal entries and push to a UDP
multicast address in syslog RFC 5424 format. journal-syslog-network runs with 
own
user systemd-journal-push. It starts running after the network is up.

Looks very nice. It indeed seems right to do this as a separate daemon.
Some comments below.

Thanks .



---
  Makefile-man.am|   8 +
  Makefile.am|  40 ++
  man/systemd-journal-network.service.xml|  84 +
  man/systemd-journal-network.xml| 115 ++
  src/journal-remote/journal-network-conf.c  |  61 
  src/journal-remote/journal-network-conf.h  |  32 ++
  src/journal-remote/journal-network-gperf.gperf |  18 +
  src/journal-remote/journal-network-manager.c   | 481 +
  src/journal-remote/journal-network-manager.h   |  70 
  src/journal-remote/journal-network-proto.c | 218 +++
  src/journal-remote/journal-network.c   | 218 +++
  src/journal-remote/journal-network.conf.in |   2 +
  units/systemd-journal-network.service.in   |  19 +
  13 files changed, 1366 insertions(+)
  create mode 100644 man/systemd-journal-network.service.xml
  create mode 100644 man/systemd-journal-network.xml
  create mode 100644 src/journal-remote/journal-network-conf.c
  create mode 100644 src/journal-remote/journal-network-conf.h
  create mode 100644 src/journal-remote/journal-network-gperf.gperf
  create mode 100644 src/journal-remote/journal-network-manager.c
  create mode 100644 src/journal-remote/journal-network-manager.h
  create mode 100644 src/journal-remote/journal-network-proto.c
  create mode 100644 src/journal-remote/journal-network.c
  create mode 100644 src/journal-remote/journal-network.conf.in
  create mode 100644 units/systemd-journal-network.service.in

diff --git a/Makefile-man.am b/Makefile-man.am
index 7a9612e..efd0cbc 100644
--- a/Makefile-man.am
+++ b/Makefile-man.am
@@ -1357,6 +1357,14 @@ man/systemd-journal-gatewayd.socket.html: 
man/systemd-journal-gatewayd.service.h
  
  endif
  
+MANPAGES += \

+man/systemd-journal-network.service.8 \
+man/systemd-journal-network.8
+MANPAGES_ALIAS += \
+man/systemd-journal-network.8
+man/systemd-journal-network.8: man/systemd-journal-network.service.8
+man/systemd-journal-network.html: man/systemd-journal-network.service.html
+
  if HAVE_MYHOSTNAME
  MANPAGES += \
man/nss-myhostname.8
diff --git a/Makefile.am b/Makefile.am
index 856accb..ad1dff5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -4336,6 +4336,46 @@ EXTRA_DIST += \
src/journal-remote/journal-upload.conf.in
  endif
  
+rootlibexec_PROGRAMS += \

+   systemd-journal-network

I think this name will be confusing. Why not systemd-journal-syslog
or systemd-journal-multicast? Network is rather generic, and we already
have three other network-and-journal-related daemons.
Yes I was confused with the naming. Indeed I named it as 
systemd-journal-syslog once too.
Later I was wondering naming it to syslog only make it restricted. If in 
future enhancements we decide to add

more features like sending in a different format .




+
+systemd_journal_network_SOURCES = \
+   src/journal-remote/journal-network-manager.h \
+   src/journal-remote/journal-network-manager.c \
+   src/journal-remote/journal-network-conf.h \
+   src/journal-remote/journal-network-conf.c \
+   src/journal-remote/journal-network-proto.c \
+   src/journal-remote/journal-network.c
+
+nodist_systemd_journal_network_SOURCES = \
+   src/journal-remote/journal-network-gperf.c
+
+EXTRA_DIST += \
+src/journal-remote/journal-network-gperf.gperf
+
+CLEANFILES += \
+src/journal-remote/journal-network-gperf.c
+
+systemd_journal_network_LDADD = \
+   libsystemd-internal.la \
+   libsystemd-journal-internal.la \
+   libsystemd-shared.la
+
+nodist_systemunit_DATA += \
+   units/systemd-journal-network.service
+
+EXTRA_DIST += \
+   units/systemd-journal-network.service.in
+
+nodist_pkgsysconf_DATA += \
+   src/journal-remote/journal-network.conf
+
+EXTRA_DIST += \
+   src/journal-remote/journal-network.conf.in
+
+CLEANFILES += \
+   src/journal-remote/journal-network.conf

You can drop that, CLEANFILES in now generated semi-automatically
in git.

Ok.

  # using _CFLAGS = in the conditional below would suppress AM_CFLAGS
  journalctl_CFLAGS = \
$(AM_CFLAGS)
diff --git a/man/systemd-journal-network.service.xml 
b/man/systemd-journal-network.service.xml
new file mode 100644
index 000..47a5b3e
--- /dev/null
+++ b/man/systemd-journal-network.service.xml
@@ -0,0 +1,84 @@
+?xml version='1.0'? !--*- Mode: nxml; nxml-child-indent: 2; indent-tabs-mode: 
nil -*--
+!DOCTYPE refentry PUBLIC 

Re: [systemd-devel] tmpfiles.d specifier support on argument when operating on files

2015-03-15 Thread Zbigniew Jędrzejewski-Szmek
On Mon, Mar 02, 2015 at 08:25:47PM +0100, Zbigniew Jędrzejewski-Szmek wrote:
 On Wed, Feb 18, 2015 at 06:17:17PM -0300, Cristian Rodríguez wrote:
  El 18/02/15 a las 07:10, Lennart Poettering escribió:
  On Tue, 17.02.15 17:35, Cristian Rodríguez (crrodrig...@opensuse.org) 
  wrote:
  
  Please fix this for all arguments, not just symlinks.
  
  diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
  index c948d4d..1b35b8e 100644
  --- a/src/tmpfiles/tmpfiles.c
  +++ b/src/tmpfiles/tmpfiles.c
  @@ -1590,6 +1590,12 @@ static int parse_line(const char *fname, unsigned 
  line, const char *buffer) {
i.argument = strappend(/usr/share/factory/, 
   i.path);
if (!i.argument)
return log_oom();
  +} else {
  +r = specifier_printf(i.argument,
  specifier_table, NULL, i.argument);
  
  Here's a memory leak, you need to free the old i.argument.
  
  Indentation! Please have a look at CODING_STYLE. You need to indent by
  8ch. 4ch indenting is not acceptable.
  
  +if (r  0) {
  +log_error([%s:%u] Failed to replace specifiers: 
  %s, fname, line, path);
  +return r;
  +}
  
  
  HI again:
  
  Is the attached version cool for you ?
 Hi,
 
 sorry for the delay. We seem to have trouble getting all patches
 reviewed recently.  Hopefully this is just
 conferences/vacations/fedora-alpha-releases, and things will
 return to normal.
 
  From ee8e4f440def745b6f0655b897e65d48321e46c5 Mon Sep 17 00:00:00 2001
  From: =?UTF-8?q?Cristian=20Rodr=C3=ADguez?= crrodrig...@opensuse.org
  Date: Wed, 18 Feb 2015 18:09:16 -0300
  Subject: [PATCH] tmpfiles: implement specifier substitution for file
   argument
  
  Only for L and C types where it makes sense.
  ---
   src/tmpfiles/tmpfiles.c | 12 
   1 file changed, 12 insertions(+)
  
  diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
  index 88ba7e4..6de477b 100644
  --- a/src/tmpfiles/tmpfiles.c
  +++ b/src/tmpfiles/tmpfiles.c
  @@ -1568,6 +1568,18 @@ static int parse_line(const char *fname, unsigned 
  line, const char *buffer) {
   }
   }
   
  +if(IN_SET(i.type, CREATE_SYMLINK, COPY_FILES)) {
 Please add a space after if and for...
 
  +if(i.argument) {
  +_cleanup_free_ char *resolved_iarg = NULL;
 ...and empty line after variable declarations.
 
  +r = specifier_printf(i.argument, specifier_table, 
  NULL, resolved_iarg);
  +if(r  0)
  +return log_error_errno(r, [%s:%u] Failed 
  to replace specifiers: %s, fname, line, path);
  +r = free_and_strdup(i.argument, resolved_iarg);
 There's no need to to use free_and_strdup here. resolved_arg was just
 newly allocated, so it can be used without any strdup.
 
  +if(r  0)
  +return log_oom();
  +}
  +}
  +
   switch (i.type) {
 
 Otherwise looks OK.

Ping?

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