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

[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 @@
+ 
+http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd";>
+
+
+
+http://www.w3.org/2001/XInclude";>
+
+  
+syst

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

[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?= 
> > 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

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

[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 @@
>  
>  
>  
> +  h
> +  Set file/directory attributes. Lines of this type
> +  accept shell-style globs in place of normal path names.
> +
> +  The format of the argument field is 
> [+-=][aAcCdDeijsStTu]
> +  
> +
> +  The prefix + causes the
> +  attribute(s) to be added; - causes the
> +  attribute(s) to be removed; =
> +  causes the attributes to set exactly as the following 
> letters.
What happens if neither of the three prefix lettes is used? This
should be documented.

> +  The letters 'aAcCdDeijsStTu' select the new
 instead of ''.

> +  attributes for the files, see
> +  chattr
> +  1 for further information.
> +  
> +  Passing only = as argument,
> +  reset all the file attributes.
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 =."

Zbyszek

> +
> +  
> +
> +
> +
> +  H
> +  Recursively set file/directory attributes. Lines
> +  of this type accept shell-style globs in place of normal
> +  path names.
> +  
> +
> +
> +
>a
>a+
>Set POSIX ACLs (access control lists). If
> @@ -529,6 +560,7 @@
> project='man-pages'>setfattr1,
> project='man-pages'>setfacl1,
> project='man-pages'>getfacl1
> +   project='man-pages'>chattr1
>  
>
>  
> -- 
> 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 2/4] Allow systemd-tmpfiles to set the file/directory attributes

[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 
>  #include 
>  #include 
> +#include 
>  
>  #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->attri

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]

[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] console-getty.service: don't start when /dev/console is missing

[Zbigniew Jędrzejewski-Szmek]

Looks reasonable.

Zbyszek

On Fri, Mar 13, 2015 at 12:57:18PM +0100, Lukas Nykryn wrote:
> From: Jan Pazdziora 
> 
> 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] journal: Introduce journal-network

[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 @@
> + 
> + +"http://www.oasis-open.org/docbook/xml/4.2/docbookx.dtd";>
> +
> +
> +
> + xmlns:xi="http://www.w3.org/2001/XInclude";>
> +
> +  
> +systemd-journal-network.service
> +systemd
> +
> +
> +  
> +Developer
> +Susant
> +Sahani
> +ssah...@gmail.com
> +  
> +
> +  
> +
> +  
> +systemd-journal-n

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

[Zbigniew Jędrzejewski-Szmek]

On Fri, Mar 13, 2015 at 05:47:28PM +, lucas.de.mar...@gmail.com wrote:
> From: Lucas De Marchi 
> 
> 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

[Zbigniew Jędrzejewski-Szmek]

On Sun, Mar 15, 2015 at 04:26:14PM -0700, ali...@she-devel.com wrote:
> From: Alison Chaiken 
> 
> ---
>  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
> -logind.conf.d/ configuration subdirectories
> +*.conf.d/ 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

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

[alison]

From: Alison Chaiken 

---
 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
-logind.conf.d/ configuration subdirectories
+*.conf.d/ 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] network: add UseNTP DHCP option

[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 @@
>
>  
>  
> +  UseNTP=
> +  
> +When 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.
> +  
> +
> +
>UseMTU=
>
>  When 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

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

[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 4/6] make in_addr_from_string() accept ipv4 addresses even when using AF_INET6

[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 2/2] fsck: Add support for EFI variable based fsck indication

[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
>  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 4/6] make in_addr_from_string() accept ipv4 addresses even when using AF_INET6

[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] [PATCH 4/6] make in_addr_from_string() accept ipv4 addresses even when using AF_INET6

[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

[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
 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 2/2] fsck: Add support for EFI variable based fsck indication

[Kay Sievers]

On Sun, Mar 15, 2015 at 8:08 PM, Zbigniew Jędrzejewski-Szmek
 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 2/6] fix strict aliasing violations in src/udev/udev-builtin-usb_id.c

[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 
>  #include 
>  #include 
> +#include 
>  
>  #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

[Zbigniew Jędrzejewski-Szmek]

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?

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

[Kay Sievers]

On Sun, Mar 15, 2015 at 7:59 PM, Andrei Borzenkov  wrote:
> В Sun, 15 Mar 2015 19:48:10 +0100
> Zbigniew Jędrzejewski-Szmek  пишет:

>> > 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

[Michael Biebl]

This was suposed to go to the list.

2015-03-13 21:25 GMT+01:00 Michael Biebl :
> 2015-03-13 20:25 GMT+01:00 Michael Marineau :
>> 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] Enabling timesyncd in virtual machines

[Kay Sievers]

On Sun, Mar 15, 2015 at 7:50 PM, Zbigniew Jędrzejewski-Szmek
 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
>>  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

[Andrei Borzenkov]

В Sun, 15 Mar 2015 19:48:10 +0100
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
> >  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  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 2/2] fsck: Add support for EFI variable based fsck indication

[Kay Sievers]

On Sun, Mar 15, 2015 at 7:48 PM, Zbigniew Jędrzejewski-Szmek
 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

[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
>  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/2] fsck: Add support for EFI variable based fsck indication

[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
>  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  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] Enabling timesyncd in virtual machines

[Kay Sievers]

On Fri, Mar 13, 2015 at 8:25 PM, Michael Marineau
 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] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily

[Kay Sievers]

On Sun, Mar 15, 2015 at 7:31 PM, Zbigniew Jędrzejewski-Szmek
 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] [PATCH] shutdown: add kexec loading, avoid calling `kexec` binary unnessecarily

[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
>  
>  
>  
> +  list-kernels
> +
> +  
> +List kernels ordered by version.
> +
> +  
> +
> +
> +
>start 
> PATTERN...
>  
>
> @@ -1550,7 +1559,7 @@ kobject-uevent 1 systemd-udevd-kernel.socket 
> systemd-udevd.service
>  
>  
>  
> -  kexec
> +  kexec 
> VERSIONKERNEL-ARG...
>  
>
>  Shut 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.
> +
> +Also allows specifying the version and optionally
> +extra kernel parameters to append. If no version is specified
> +then the most recent kernel is booted.
>
>  
>  
> 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 .
>  ***/
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -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 +207,6 @@ int main(int argc, char *argv[]) {
>  log_info("Sending SIGKILL to remaining processes...");
>  broadcast_signal(SIGKILL, true, false);
>  
> -in_container = detect_container(NULL) > 0;
> -
>  need_umo

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

[Kay Sievers]

On Sun, Mar 15, 2015 at 4:44 PM, Zbigniew Jędrzejewski-Szmek
 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  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] udev breaks hostap linkage of raw/user interfaces, causing wpa_supplicant problems

[Kay Sievers]

On Sun, Mar 15, 2015 at 4:07 PM, Thomas Richter  wrote:
> On 15.03.2015 15:38, Kay Sievers wrote:
>
>> 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.

The kernel driver can tell userspace what kind of name it used to
create the device. Maybe the driver you use add that information.

Here is the relevant kernel side:
  
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=685343fc3ba61a1f6eef361b786601123db16c28

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

Re: [systemd-devel] [v3 2/4] update TODO

[Zbigniew Jędrzejewski-Szmek]

On Fri, Feb 27, 2015 at 05:04:22PM -0800, Shawn Landden wrote:
> ---
>  TODO | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/TODO b/TODO
> index e8e4800..1f5bfaa 100644
> --- a/TODO
> +++ b/TODO
> @@ -32,6 +32,8 @@ External:
>  * When lz4 gets an API for lz4 command output, make use of it to
>compress coredumps in a way compatible with /usr/bin/lz4.
>  
> +* Fix emacs for Lennart so we can get rid of the Makefile hack littering git
> +
Is it even possible? Those Makefiles make it possible to chdir to a
directory and work from there. Even git works nicer then, because
diffs and greps do not include the full path.

Zbyszek
___
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

[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 @@
>  systemd-poweroff.service
>  systemd-reboot.service
>  systemd-kexec.service
> -/usr/lib/systemd/systemd-shutdown
>
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

>  
>
> 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 @@
>
>  systemd-shutdownd.service
>  systemd-shutdownd.socket
> -/usr/lib/systemd/systemd-shutdownd
>
>  
>
> 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 @@
>  systemd-suspend.service
>  systemd-hibernate.service
>  systemd-hybrid-sleep.service
> -/usr/lib/systemd/system-sleep
>
>  
>
> -- 
> 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] fix compiler warning

[Shawn Landden]

On Sun, Mar 15, 2015 at 4:53 AM, Ronny Chevalier 
wrote:

> 2015-03-14 17:54 GMT+01:00 Shawn Landden :
> > 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 :
> >> > 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

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

[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  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] udev breaks hostap linkage of raw/user interfaces, causing wpa_supplicant problems

[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

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

[Kay Sievers]

On Sun, Mar 15, 2015 at 1:40 PM, Thomas Richter  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

[Kay Sievers]

On Sun, Mar 15, 2015 at 11:56 AM, Jan Janssen  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

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

[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] [PATCH] fix compiler warning

[Ronny Chevalier]

2015-03-14 17:54 GMT+01:00 Shawn Landden :
> On Sat, Mar 14, 2015 at 6:31 AM, Ronny Chevalier 
> wrote:
>>
>> 2015-03-11 4:42 GMT+01:00 Shawn Landden :
>> > 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()

[Ronny Chevalier]

2015-03-15 3:27 GMT+01:00 Shawn Landden :
> 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

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

[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 @@
   
 
   
+--firmware
+
+
+  Indicate to the firmware to boot into EFI setup on machines
+  that support it if reboot is used. Note that
+  this is only supported if the machine was booted in EFI mode.
+
+  
+
+  
 --root=
 
 
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 mode.");
+
+r = efi_get_variable(EFI_VENDOR_GLOBAL, 
"OsIndicationsSupported", NULL, &v, &s);
+   

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

[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 @@
   
 
   
+--fsck-mode=
+
+
+  Control file system check behavior for next boot on EFI 
systems.
+
+  One of auto, force and
+  skip. See 
systemd-fsck8
+  for details. Note that this requires the system to be booted in EFI 
mode and
+  that kernel command line parameters take precedence.
+
+  
+
+  
+--fsck-repair=
+
+
+  Control file system check repair behavior for next boot on EFI 
systems.
+
+  One of preen, yes and
+  no. See 
systemd-fsck8
+  for details. Note that this requires the system to be booted in EFI 
mode and
+  that kernel command line parameters take precedence.
+
+  
+
+  
 --root=
 
 
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 = *(EfiSystemdFsckMode *)v;
+
+switch (value) {
+case EFI_SYSTEMD_FSCK_MODE_AUTO:
+arg_force = arg_skip = false;
+break;
+case EFI_SYSTEMD_FSCK_MODE_FORCE:
+arg_force = true;
+break;
+case EFI_SYSTEMD_FSCK_MODE_SKIP:
+arg_skip = true;
+