Re: [systemd-devel] [RFC 0/6] A network proxy management daemon, systemd-proxy-discoveryd

2015-04-13 Thread Dimitri John Ledkov
On 11 April 2015 at 13:41, Zbigniew Jędrzejewski-Szmek
 wrote:
> On Fri, Apr 10, 2015 at 03:17:37PM +0300, Tomasz Bursztyka wrote:
>> Hi,
>>
>> As it has been discussed in the systemd hackfest during the Linux Conference
>> Europe, one daemon could centralize the management of all network proxy
>> configurations. Idea is something user can do per-application (like in
>> firefox for instance) or broader (per-DM like in Gnome), user could do it
>> once and for all through such daemon and applications would then request it
>> to know whether or not a proxy has to be used and which one.
>>
>> As a notice, this is nothing new. Such standalone daemon has been already
>> done by the past, pacrunner. systemd-proxy-discoveryd will more or less
>> implement the same ideas with improvements. It will get rid of big JS
>> engines like spidermonkey or v8 which are overkill for the tiny PAC files
>> to be executed on, for instance. From pacrunner experience, APIs will be
>> also improved.
> Hi,
>
> the idea of having centralized proxy config is certainly nice. But the
> PAC files make me shiver. So the first question: is it really necessary
> to support PAC files? Are they widely used in corporate setting or something?
> Is there no saner standard?
>

Yes.
Yes.
No.

I only wish for system-wide WPAD support and PAC auto-parsing.

The standard started by netscape or some such, hence interpreted
javascript, and nobody came up with something more declarative /
deterministic that catched on.

> If the PAC files must be interpreted, I think this is the hardest
> part.  FindProxyForURL is started for every request, potentially
> hundreds of times per second and more. This means that starting a
> process per invocation is out of the question, and even starting a
> thread per invocation seems to be too much. But each call fall into an
> infinite loop and hang. So the run time of FindProxyForURL should be
> bounded. FindProxyForURL can also resolve names over the network,
> which would best be done asynchronously.
>

Well pac file is generally cached, and is static it its contents
(possibly many complex clauses if this / if that) but it's ideal to
keep it around for subsequent queries.

> Things in systemd are usually implemented through poll loops, which
> makes it easy to support thousands of concurrent "jobs". I'd think
> that this would be the best option here too, with a number of "workers"
> executing FindProxyForURL()s and stopping when name resolution is
> requested and continuing when the name is resolved.
>
> Zbyszek
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel



-- 
Regards,

Dimitri.

https://clearlinux.org
Open Source Technology Center
Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


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

2015-04-13 Thread Zbigniew Jędrzejewski-Szmek
On Thu, Apr 09, 2015 at 11:43:15PM +0530, Susant Sahani wrote:
>This tiny daemon enables to pull journal entries and push to a UDP
> multicast address in syslog RFC 5424 format. systemd-journal-syslogd
> runs with own user systemd-journal-syslog. It starts running after
> the network is up.
> 
> V2: Address Zbigniew's comments
>   1. Rename binary systemd-journal-syslogd
>   2. Fixed up man and added example
>   3. Error code check sd_event_add_signal
>   4. remove +User=systemd-journal-network from service file
>   5. remove opterr=0
>   6. assignment into declaration of mh
> 
> V3: Address Lennart's comments
>   1. add unicast events in the man
>   2. use fopen_temporary and fflush_and_check().
>   3. remove if (m->event_journal_input) {
>   4. use  sigprocmask_many
>   5. fix facility and priority
>   6. remove IP_MULTICAST_TTL and IP_PKTINFO
>   7. use safe_atoi
> ---
>  Makefile-man.am   |   8 +
>  Makefile.am   |  37 ++
>  man/systemd-journal-syslogd.service.xml   |  84 +
>  man/systemd-journal-syslogd.xml   | 156 
>  src/journal-remote/journal-syslog-conf.c  |  61 
>  src/journal-remote/journal-syslog-conf.h  |  39 ++
>  src/journal-remote/journal-syslog-gperf.gperf |  18 +
>  src/journal-remote/journal-syslog-manager.c   | 501 
> ++
>  src/journal-remote/journal-syslog-manager.h   |  70 
>  src/journal-remote/journal-syslog-network.c   | 201 +++
>  src/journal-remote/journal-syslogd.c  | 217 +++
>  src/journal-remote/journal-syslogd.conf.in|   2 +
>  units/systemd-journal-syslogd.service |  18 +
>  13 files changed, 1412 insertions(+)
>  create mode 100644 man/systemd-journal-syslogd.service.xml
>  create mode 100644 man/systemd-journal-syslogd.xml
>  create mode 100644 src/journal-remote/journal-syslog-conf.c
>  create mode 100644 src/journal-remote/journal-syslog-conf.h
>  create mode 100644 src/journal-remote/journal-syslog-gperf.gperf
>  create mode 100644 src/journal-remote/journal-syslog-manager.c
>  create mode 100644 src/journal-remote/journal-syslog-manager.h
>  create mode 100644 src/journal-remote/journal-syslog-network.c
>  create mode 100644 src/journal-remote/journal-syslogd.c
>  create mode 100644 src/journal-remote/journal-syslogd.conf.in
>  create mode 100644 units/systemd-journal-syslogd.service
> 
> diff --git a/Makefile-man.am b/Makefile-man.am
> index 2f3e5f2..437d488 100644
> --- a/Makefile-man.am
> +++ b/Makefile-man.am
> @@ -1380,6 +1380,14 @@ man/systemd-journal-gatewayd.socket.html: 
> man/systemd-journal-gatewayd.service.h
>  
>  endif
>  
> +MANPAGES += \
> +man/systemd-journal-syslogd.service.8 \
> +man/systemd-journal-syslogd.8
> +MANPAGES_ALIAS += \
> +man/systemd-journal-syslogd.8
> +man/systemd-journal-syslogd.8: man/systemd-journal-syslogd.service.8
> +man/systemd-journal-syslogd.html: man/systemd-journal-syslogd.service.html
> +
>  if HAVE_MYHOSTNAME
>  MANPAGES += \
>   man/nss-myhostname.8
> diff --git a/Makefile.am b/Makefile.am
> index 0a57389..0b843ac 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -4361,6 +4361,43 @@ EXTRA_DIST += \
>   src/journal-remote/journal-upload.conf.in
>  endif
>  
> +systemd_journal_syslogd_SOURCES = \
> + src/journal-remote/journal-syslog-manager.h \
> + src/journal-remote/journal-syslog-manager.c \
> + src/journal-remote/journal-syslog-conf.h \
> + src/journal-remote/journal-syslog-conf.c \
> + src/journal-remote/journal-syslog-network.c \
> + src/journal-remote/journal-syslogd.c
> +
> +nodist_systemd_journal_syslogd_SOURCES = \
> + src/journal-remote/journal-syslog-gperf.c
> +
> +EXTRA_DIST += \
> +src/journal-remote/journal-syslog-gperf.gperf
> +
> +CLEANFILES += \
> +src/journal-remote/journal-syslog-gperf.c
> +
> +systemd_journal_syslogd_LDADD = \
> + libsystemd-internal.la \
> + libsystemd-journal-internal.la \
> + libsystemd-shared.la
> +
> +rootlibexec_PROGRAMS += \
> + systemd-journal-syslogd
> +
> +nodist_systemunit_DATA += \
> + units/systemd-journal-syslogd.service
> +
> +EXTRA_DIST += \
> + units/systemd-journal-syslogd.service.in
> +
> +nodist_pkgsysconf_DATA += \
> + src/journal-remote/journal-syslogd.conf
> +
> +EXTRA_DIST += \
> + src/journal-remote/journal-syslogd.conf.in
> +
>  # using _CFLAGS = in the conditional below would suppress AM_CFLAGS
>  journalctl_CFLAGS = \
>   $(AM_CFLAGS)
> diff --git a/man/systemd-journal-syslogd.service.xml 
> b/man/systemd-journal-syslogd.service.xml
> new file mode 100644
> index 000..e837153
> --- /dev/null
> +++ b/man/systemd-journal-syslogd.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-syslogd.se

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

2015-04-13 Thread Zbigniew Jędrzejewski-Szmek
On Fri, Apr 10, 2015 at 12:45:34PM +0200, Lennart Poettering wrote:
> On Thu, 09.04.15 23:43, Susant Sahani (sus...@redhat.com) wrote:
> 
> >This tiny daemon enables to pull journal entries and push to a UDP
> > multicast address in syslog RFC 5424 format. systemd-journal-syslogd
> > runs with own user systemd-journal-syslog. It starts running after
> > the network is up.
> 
> Hmm, before we merge this, one more thing: I think we need a more
> explanatory name for this. If we call it "systemd-journal-syslogd",
> then people might assume this might be invovled with or necessary for
> systemd *reading* syslog traffic. However, that's not what it does, it
> only *sends* syslog traffic. Hence maybe call it
> "systemd-journal-syslog-emitd" or so?

Yeah, I have to agree very much here: "journal-syslogd" name is too similar
to "syslogd", the original syslog daemon, and is bound to cause endless 
confusion.
Maybe we should go for journal-netlogd, reminiscent of netconsole,
which is actually a similar thing. s-j-s-emitd is a bit too long.

I see some typos in the patch... I'll reply separately.

Zbyszek

> 
> Also, how can we tap into the multicast traffic for this? Is there any
> better tool for this around, that is commonly available and used?
> 
> How did you test this?
> 
> > +if (rename(temp_path, m->state_file) < 0) {
> > +r = -errno;
> > +goto finish;
> > +}
> > +
> > +free(temp_path);
> > +temp_path = NULL;
> > +
> > + finish:
> > +if (r < 0)
> > +log_error_errno(r, "Failed to save state %s: %m", 
> > m->state_file);
> > +
> 
> The failure path should remove the file temp_path here. Add something like:
> 
> if (temp_path)
> (void) unlink(temp_path);
> 
> 
> > +static int network_send(Manager *m, struct iovec *iovec, unsigned n_iovec) 
> > {
> > +struct msghdr mh = {
> > +.msg_iov = iovec,
> > +.msg_iovlen = n_iovec,
> > +};
> > +
> > +assert(m);
> > +assert(iovec);
> > +assert(n_iovec > 0);
> > +
> > +if (m->address.sockaddr.sa.sa_family == AF_INET) {
> > +mh.msg_name = &m->address.sockaddr.sa;
> > +mh.msg_namelen = sizeof(m->address.sockaddr.sa);
> 
> This actually looks wrong? This should be
> sizeof(m->address.sockaddr.in)?
> 
> > +} else if (m->address.sockaddr.sa.sa_family == AF_INET6) {
> > +mh.msg_name = &m->address.sockaddr.in6;
> 
> And this doesn't look right either, this should be
> &m->address.sockaddr.sa, otherwise this should generate a type error,
> no?
> 
> Lennart
> 
> -- 
> Lennart Poettering, Red Hat
> 
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH v2 1/2] input_id: Make test_pointer / test_keys return if they've found anything

2015-04-13 Thread Peter Hutterer
On Mon, Apr 13, 2015 at 03:08:53PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 13-04-15 14:41, Zbigniew Jędrzejewski-Szmek wrote:
> >On Mon, Apr 13, 2015 at 11:15:00AM +0200, Hans de Goede wrote:
> >>Make test_pointer / test_keys return a boolean indicating whether or not
> >>they've set any properties on the device.
> >>
> >>While touching allmost all test_bit() using lines anyways also remove
> >>the extra space between the function name and the '(' (coding style issue).
> >>
> >>Signed-off-by: Hans de Goede 
> >>---
> >>  src/udev/udev-builtin-input_id.c | 98 
> >> 
> >>  1 file changed, 60 insertions(+), 38 deletions(-)
> >>
> >>diff --git a/src/udev/udev-builtin-input_id.c 
> >>b/src/udev/udev-builtin-input_id.c
> >>index ecfc447..2c8a9ee 100644
> >>--- a/src/udev/udev-builtin-input_id.c
> >>+++ b/src/udev/udev-builtin-input_id.c
> >>@@ -126,7 +126,7 @@ static void get_cap_mask(struct udev_device *dev,
> >>  }
> >>
> >>  /* pointer devices */
> >>-static void test_pointers (struct udev_device *dev,
> >>+static bool test_pointers (struct udev_device *dev,
> > ^
> >Those spaces after function name declaration should go too.
> 
> That seems like something for a separate commit.
> 
> >And
> >the white-space changes should probably be a separate commit becuase
> >now it's hard to see what is going on ;)
> 
> Almost all changed lines are changed because they must be changed
> (adding { or }, changing indentation), only 5 or so changed lines
> are purely removing the extra space between test_bit and '(' which
> is exactly why I squashed in those changes.

I've split the commit up into a whitespace-only one and the new changes.
Series pushed as 37186823f3868612e0cbfdc65b2562ccbda61ada

Cheers,
   Peter


> >>@@ -135,77 +135,93 @@ static void test_pointers (struct udev_device *dev,
> >> bool test) {
> >>  int is_mouse = 0;
> >>  int is_touchpad = 0;
> >>+bool ret = false;
> >>
> >>-if (test_bit (INPUT_PROP_ACCELEROMETER, bitmask_props)) {
> >>+if (test_bit(INPUT_PROP_ACCELEROMETER, bitmask_props)) {
> >>  udev_builtin_add_property(dev, test, 
> >> "ID_INPUT_ACCELEROMETER", "1");
> >>-return;
> >>+return true;
> >>  }
> >>
> >>-if (!test_bit (EV_KEY, bitmask_ev)) {
> >>-if (test_bit (EV_ABS, bitmask_ev) &&
> >>-test_bit (ABS_X, bitmask_abs) &&
> >>-test_bit (ABS_Y, bitmask_abs) &&
> >>-test_bit (ABS_Z, bitmask_abs))
> >>+if (!test_bit(EV_KEY, bitmask_ev)) {
> >>+if (test_bit(EV_ABS, bitmask_ev) &&
> >>+test_bit(ABS_X, bitmask_abs) &&
> >>+test_bit(ABS_Y, bitmask_abs) &&
> >>+test_bit(ABS_Z, bitmask_abs)) {
> >>  udev_builtin_add_property(dev, test, 
> >> "ID_INPUT_ACCELEROMETER", "1");
> >>-return;
> >>+ret = true;
> >>+}
> >>+return ret;
> >>  }
> >>
> >>-if (test_bit (EV_ABS, bitmask_ev) &&
> >>-test_bit (ABS_X, bitmask_abs) && test_bit (ABS_Y, 
> >>bitmask_abs)) {
> >>-if (test_bit (BTN_STYLUS, bitmask_key) || test_bit 
> >>(BTN_TOOL_PEN, bitmask_key))
> >>+if (test_bit(EV_ABS, bitmask_ev) &&
> >>+test_bit(ABS_X, bitmask_abs) && test_bit(ABS_Y, bitmask_abs)) {
> >>+if (test_bit(BTN_STYLUS, bitmask_key) || 
> >>test_bit(BTN_TOOL_PEN, bitmask_key)) {
> >>  udev_builtin_add_property(dev, test, 
> >> "ID_INPUT_TABLET", "1");
> >>-else if (test_bit (BTN_TOOL_FINGER, bitmask_key) && 
> >>!test_bit (BTN_TOOL_PEN, bitmask_key))
> >>+ret = true;
> >>+} else if (test_bit(BTN_TOOL_FINGER, bitmask_key) && 
> >>!test_bit(BTN_TOOL_PEN, bitmask_key)) {
> >>  is_touchpad = 1;
> >>-else if (test_bit (BTN_MOUSE, bitmask_key))
> >>+} else if (test_bit(BTN_MOUSE, bitmask_key)) {
> >>  /* This path is taken by VMware's USB mouse, 
> >> which has
> >>   * absolute axes, but no touch/pressure button. */
> >>  is_mouse = 1;
> >>-else if (test_bit (BTN_TOUCH, bitmask_key))
> >>+} else if (test_bit(BTN_TOUCH, bitmask_key)) {
> >>  udev_builtin_add_property(dev, test, 
> >> "ID_INPUT_TOUCHSCREEN", "1");
> >>+ret = true;
> >>  /* joysticks don't necessarily have to have buttons; e. g.
> >>   * rudders/pedals are joystick-like, but buttonless; they 
> >> have
> >>   * other fancy axes */
> >>-else if (test_bit (BTN_TRIGGER, bitmask_key) ||
> >>-   

Re: [systemd-devel] issuing 'reboot' command does not print the familiar 'Restarting system.' message

2015-04-13 Thread Ani Sinha
On Sun, Apr 12, 2015 at 6:46 AM, Lennart Poettering
 wrote:
> On Fri, 10.04.15 12:35, Ani Sinha (a...@arista.com) wrote:
>
>> On Fri, Apr 10, 2015 at 10:18 AM, Ani Sinha  wrote:
>> > Thanks Lennart for the clarification. Much appreciated!
>> >
>> > Ani
>> >
>> >
>> > On Fri, Apr 10, 2015 at 8:58 AM, Lennart Poettering
>> >  wrote:
>> >> On Fri, 10.04.15 08:40, Ani Sinha (a...@arista.com) wrote:
>> >>
>> >>> On Fri, Apr 10, 2015 at 1:13 AM, Daniel Mack  wrote:
>> >>> > On 04/10/2015 04:18 AM, Ani Sinha wrote:
>> >>> >> OK I have one more question. Does every call path in the reboot
>> >>> >> command use the Linux reboot() sys call? I'm not familiar with dbus
>> >>> >> stuff but looking at the code seems to indicate that there might be
>> >>> >> some paths where reboot() is not issued. Just wanted to run by you
>> >>> >> guys since you guys know the code best.
>> >>> >
>> >>> > The reboot command is symlinked to systemctl, which is a multi-call
>> >>> > binary. When invoked as 'reboot', 'shutdown', 'halt', 'poweroff' etc, 
>> >>> > it
>> >>> > communicates with PID1 and tells it to start one of the shutdown
>> >>> > targets. This way, the system will shut down and stop services in a
>> >>> > clean way. Once the target is reached, the reboot() syscall is issued 
>> >>> > to
>> >>> > the kernel.
>> >>>
>> >>> Can you please point me to the code and function call that processes
>> >>> the 'reboot' target from PID 1?
>> >>
>> >> reboot.target pulls in systemd-reboot.service, which wraps
>> >> "/usr/bin/systemctl --force reboot", which issues the Reboot() call on
>> >> PID1's bus API, which causes it to execute /usr/lib/systemd-shutdown
>> >> as PID 1 which then kills everything and reboots.
>>
>> OK I see it now. shutdownd.c eventually issues 'shutdown -r now'. This
>> gets parsed by shutdown_parse_argv(). Eventually we end up calling
>> halt_main() ->halt_now() ->reboot(RB_AUTOBOOT).
>
> Hmm?
>
> shutdownd's only raison d'etre is to support scheduled shutdowns. It's
> not used unless you actually use scheduled shutdowns.

Yes you are right. I looked at shutdownd and later on also looked at
the core/shutdown.c, later being a more direct path to reboot() call.

If you just use
> "systemctl poweroff" or so, then it will *not* be used at all, and is
> not involved at all in the shutdown logic.

again, sorry for the confusion.

>
> And no! shutdownd will never call halt_main(), nor halt_now(), nor
> reboot(RB_AUTOBOUT). If you use shutdownd, then this will result in
> the equiavlent of "systemctl poweroff" and so on, just delayed a bit.

yes, it does not go through that code path directly. I was referring
to this piece of code where it execs "shutdown" :

execl(SYSTEMCTL_BINARY_PATH,
   "shutdown",
sw,
"now",
 (b.command.warn_wall && b.command.wall_message[0]) ?
b.command.wall_message :
 (b.command.warn_wall ? NULL : "--no-wall"),
  NULL);

I should have been clearer in my earlier comment.

>
> Please don't confuse shutdownd.c and shutdown.c. The former is just
> there for the scheduled shutdown bits, the latter is the always used
> last part of the shutdown logic.
>
> Lennart
>
> --
> Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] [PATCH] importd: add CAP_DAC_OVERRIDE capability

2015-04-13 Thread Lubomir Rintel
Fedora's filesystem package ships /usr/bin (and other directories) which are
not writable by its owner. machinectl pull-dkr (and possibly others) are not
able to extract those:

  14182 mkdirat(3, "usr", 0700)   = 0
  14182 mkdirat(3, "usr/bin", 0500)   = 0
  14182 openat(3, "usr/bin/[", 
O_WRONLY|O_CREAT|O_EXCL|O_NOCTTY|O_NONBLOCK|O_CLOEXEC, 0700) = -1 EACCES 
(Permission denied)
  ...
---
 units/systemd-importd.service.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/units/systemd-importd.service.in b/units/systemd-importd.service.in
index a540040..80d97c8 100644
--- a/units/systemd-importd.service.in
+++ b/units/systemd-importd.service.in
@@ -12,6 +12,6 @@ Documentation=man:systemd-importd.service(8)
 [Service]
 ExecStart=@rootlibexecdir@/systemd-importd
 BusName=org.freedesktop.import1
-CapabilityBoundingSet=CAP_CHOWN CAP_FOWNER CAP_FSETID CAP_MKNOD CAP_SETFCAP 
CAP_SYS_ADMIN CAP_SETPCAP
+CapabilityBoundingSet=CAP_CHOWN CAP_FOWNER CAP_FSETID CAP_MKNOD CAP_SETFCAP 
CAP_SYS_ADMIN CAP_SETPCAP CAP_DAC_OVERRIDE
 NoNewPrivileges=yes
 WatchdogSec=1min
-- 
2.1.0

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


Re: [systemd-devel] [RFC 3/6] proxy-discoveryd: Add PAC support through duktape js engine

2015-04-13 Thread Zbigniew Jędrzejewski-Szmek
On Mon, Apr 13, 2015 at 09:51:05AM -0700, Marcel Holtmann wrote:
> Hi Zbyszek,
> 
> >> --- a/Makefile.am
> >> +++ b/Makefile.am
> >> @@ -5895,12 +5895,19 @@ rootlibexec_PROGRAMS += \
> >>   systemd-proxy-discoveryd
> >> 
> >> systemd_proxy_discoveryd_SOURCES = \
> >> +   src/proxy-discovery/duktape.h \
> >> +   src/proxy-discovery/duktape.c \
> > These files are not included in the patch, how do you intend to ship 
> > them?
>  
>  I figured this could be included as a separate commit, when it will
>  be time to commit proxy-discoveryd someday.
>  Those 2 files represent more than 2 megabytes, so I did not want to
>  flood the ml.
> >>> duktape should not be embedded in systemd. It's a separate project,
> >>> which gets updates and has a life of its own.
> >>> 
> >>> Preferably, duktape would be packaged by distributions and we would
> >>> use it like any other external library. If that is not possible, we'll
> >>> have to look for some other option, but only then. For example,
> >>> git submodule is still better than embedding of a snapshot.
> >> 
> >> that is not how this actually works with duktape. The release versions are 
> >> building a single duktape.[ch] that you can only find in the releases. 
> >> Otherwise you have to stitch that manually. It is pretty much designed to 
> >> be included into other project as a copy. That is why nobody has actually 
> >> packages this so far.
> >> 
> >> So while I get that including copies of other libraries is generally bad 
> >> for security update reasons, but this one would qualify as an exception. 
> >> It is just that we have to monitor duktape upstream releases and with that 
> >> also update the duktape version in systemd.
> >> 
> >> It is something that is not the greatest solution, but something that is 
> >> feasible. Keep in mind that embedding your JS engine right into your 
> >> application allows for optimizations that you really want. We have build 
> >> PACrunner against V8 and MozJS and these are two really painful 
> >> experiences. For anybody wanting to build a small system, dealing with 
> >> these two upstream packages is a nightmare. It sort of works in a large 
> >> distribution like Fedora, but anything small it is not an option.
> > 
> > I know that embedding is the way that upstream suggests. But it is a
> > bad fit for the way that systemd is distributed. Our primary consumers
> > are distributions, and the first thing most distributions will do is
> > to rip out the embedded copy in order to use a shared one. Something
> > as big as a javascript engine is bound to have security updates and by
> > embedding it systemd we would be forced to a) follow upstream changes,
> > b) release systemd updates based on duktape releases. Not something that
> > we want to do or would do very well.
> > 
> > We really should try to base this on a javascript engine which
> > provides a shared library.
> 
> so do you know of a small Javascript engine with a proper license then? And 
> that also has no additional dependencies.

No, but I don't think this can be the guiding principle here. We
already have dependencies on a large number of things: gnutls,
docbook, unifont, libmicrohttpd, python, and a bunch of smaller
libraries. I know that another dependency might be difficult for
embedded systems, but a "small" library might actually be more work to
maintain the long run than bigger library which is easier to integrate.

> I was not kidding when I said the V8 and MozJS are a nightmare when you want 
> to build a small system. So these two are pretty much out of the question for 
> the PAC service.

I certainly can believe that ;)

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


Re: [systemd-devel] [PATCH] network: allow domain names up to 255 characters

2015-04-13 Thread Nick Owens
sorry, "supercalifragilisticexpialidocioussupercalifragilisticexpialidocious"
was a bad example. it would not be valid since it is longer than a
label.

On Sun, Apr 12, 2015 at 9:37 AM, Lennart Poettering
 wrote:
> On Sun, 12.04.15 09:21, Nick Owens (misch...@offblast.org) wrote:
>
>> On Sun, Apr 12, 2015 at 6:35 AM, Lennart Poettering
>>  wrote:
>> > On Fri, 10.04.15 13:03, Nick Owens (misch...@offblast.org) wrote:
>> >
>> >> From: mischief 
>> >>
>> >> The maximum domain name size is larger than the maximum host name size.
>> >> The smaller limit causes valid domains provided by DHCP or .network
>> >> files to be silently ignored.
>> >
>> > Hmm?
>> >
>> > Can you give an example?
>>
>> if you set the Domains key in the [Network] section of a
>> systemd.network file to a domain longer than a label, then it will be
>> ignored. the same is true if your DHCP server sends a domain in option
>> 15 (domain name) that is longer than a label. it will be ignored too.
>> both of these code paths call 'hostname_is_valid', which will fail if
>> passed something larger than a label, which a domain name can be.
>
> "longer than a label"? What do you mean by that? The function should
> perfectly consider multi-label names valid? Are you saying you cannot
> set the domain name "foo.bar."?

a multi-label name is valid. however, hostname_is_valid will reject
any domain name (multi-label or not) longer than HOST_NAME_MAX, the
size of a label.

>
> Hmm, so the DHCP spec explicitly declares that options 15 and 12 are
> about the DNS hostname, where RFC 1035 is normative. Our function
> hostname_is_valid() currently does not validate host names according
> to RFC 1035, but is in some way stricter (by enforcing Linux' own semantics
> on the length, and by limiting the charset drastically) and in other
> ways less strict (by not enforce label length.)
>
> I am pretty sure we should leave hostname_is_valid() the way it is, to
> be used when setting local hostnames and things like that. However,
> the DHCP code should really validate according to RFC 1035 instead,
> since that's what the spec says...
>
> Implementation-wise this probably means we should move
> src/resolve/resolved-dns-domain.[ch] into src/shared/dns-domain.[ch]
> and then add a call there that works similar to dns_name_normalize()
> but doesn't actually normalize, but simply validates.

the intention of the domainname_is_valid function in this patch is to
validate domain names, which can be up to 255 octets. there appears to
be no standardized macro for this size, so i added one.

>
> Hope that makes sense?
>
> Lennart
>
> --
> Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [RFC 3/6] proxy-discoveryd: Add PAC support through duktape js engine

2015-04-13 Thread Marcel Holtmann
Hi Zbyszek,

>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -5895,12 +5895,19 @@ rootlibexec_PROGRAMS += \
>>   systemd-proxy-discoveryd
>> 
>> systemd_proxy_discoveryd_SOURCES = \
>> +   src/proxy-discovery/duktape.h \
>> +   src/proxy-discovery/duktape.c \
> These files are not included in the patch, how do you intend to ship them?
 
 I figured this could be included as a separate commit, when it will
 be time to commit proxy-discoveryd someday.
 Those 2 files represent more than 2 megabytes, so I did not want to
 flood the ml.
>>> duktape should not be embedded in systemd. It's a separate project,
>>> which gets updates and has a life of its own.
>>> 
>>> Preferably, duktape would be packaged by distributions and we would
>>> use it like any other external library. If that is not possible, we'll
>>> have to look for some other option, but only then. For example,
>>> git submodule is still better than embedding of a snapshot.
>> 
>> that is not how this actually works with duktape. The release versions are 
>> building a single duktape.[ch] that you can only find in the releases. 
>> Otherwise you have to stitch that manually. It is pretty much designed to be 
>> included into other project as a copy. That is why nobody has actually 
>> packages this so far.
>> 
>> So while I get that including copies of other libraries is generally bad for 
>> security update reasons, but this one would qualify as an exception. It is 
>> just that we have to monitor duktape upstream releases and with that also 
>> update the duktape version in systemd.
>> 
>> It is something that is not the greatest solution, but something that is 
>> feasible. Keep in mind that embedding your JS engine right into your 
>> application allows for optimizations that you really want. We have build 
>> PACrunner against V8 and MozJS and these are two really painful experiences. 
>> For anybody wanting to build a small system, dealing with these two upstream 
>> packages is a nightmare. It sort of works in a large distribution like 
>> Fedora, but anything small it is not an option.
> 
> I know that embedding is the way that upstream suggests. But it is a
> bad fit for the way that systemd is distributed. Our primary consumers
> are distributions, and the first thing most distributions will do is
> to rip out the embedded copy in order to use a shared one. Something
> as big as a javascript engine is bound to have security updates and by
> embedding it systemd we would be forced to a) follow upstream changes,
> b) release systemd updates based on duktape releases. Not something that
> we want to do or would do very well.
> 
> We really should try to base this on a javascript engine which
> provides a shared library.

so do you know of a small Javascript engine with a proper license then? And 
that also has no additional dependencies.

I was not kidding when I said the V8 and MozJS are a nightmare when you want to 
build a small system. So these two are pretty much out of the question for the 
PAC service.

Regards

Marcel

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


Re: [systemd-devel] [RFC 3/6] proxy-discoveryd: Add PAC support through duktape js engine

2015-04-13 Thread Zbigniew Jędrzejewski-Szmek
On Mon, Apr 13, 2015 at 08:43:32AM -0700, Marcel Holtmann wrote:
> Hi Zbyszek,
> 
>  --- a/Makefile.am
>  +++ b/Makefile.am
>  @@ -5895,12 +5895,19 @@ rootlibexec_PROGRAMS += \
> systemd-proxy-discoveryd
>  
>  systemd_proxy_discoveryd_SOURCES = \
>  +   src/proxy-discovery/duktape.h \
>  +   src/proxy-discovery/duktape.c \
> >>> These files are not included in the patch, how do you intend to ship them?
> >> 
> >> I figured this could be included as a separate commit, when it will
> >> be time to commit proxy-discoveryd someday.
> >> Those 2 files represent more than 2 megabytes, so I did not want to
> >> flood the ml.
> > duktape should not be embedded in systemd. It's a separate project,
> > which gets updates and has a life of its own.
> > 
> > Preferably, duktape would be packaged by distributions and we would
> > use it like any other external library. If that is not possible, we'll
> > have to look for some other option, but only then. For example,
> > git submodule is still better than embedding of a snapshot.
> 
> that is not how this actually works with duktape. The release versions are 
> building a single duktape.[ch] that you can only find in the releases. 
> Otherwise you have to stitch that manually. It is pretty much designed to be 
> included into other project as a copy. That is why nobody has actually 
> packages this so far.
> 
> So while I get that including copies of other libraries is generally bad for 
> security update reasons, but this one would qualify as an exception. It is 
> just that we have to monitor duktape upstream releases and with that also 
> update the duktape version in systemd.
> 
> It is something that is not the greatest solution, but something that is 
> feasible. Keep in mind that embedding your JS engine right into your 
> application allows for optimizations that you really want. We have build 
> PACrunner against V8 and MozJS and these are two really painful experiences. 
> For anybody wanting to build a small system, dealing with these two upstream 
> packages is a nightmare. It sort of works in a large distribution like 
> Fedora, but anything small it is not an option.

I know that embedding is the way that upstream suggests. But it is a
bad fit for the way that systemd is distributed. Our primary consumers
are distributions, and the first thing most distributions will do is
to rip out the embedded copy in order to use a shared one. Something
as big as a javascript engine is bound to have security updates and by
embedding it systemd we would be forced to a) follow upstream changes,
b) release systemd updates based on duktape releases. Not something that
we want to do or would do very well.

We really should try to base this on a javascript engine which
provides a shared library.

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


Re: [systemd-devel] [RFC 3/6] proxy-discoveryd: Add PAC support through duktape js engine

2015-04-13 Thread Marcel Holtmann
Hi Zbyszek,

 --- a/Makefile.am
 +++ b/Makefile.am
 @@ -5895,12 +5895,19 @@ rootlibexec_PROGRAMS += \
systemd-proxy-discoveryd
 
 systemd_proxy_discoveryd_SOURCES = \
 +   src/proxy-discovery/duktape.h \
 +   src/proxy-discovery/duktape.c \
>>> These files are not included in the patch, how do you intend to ship them?
>> 
>> I figured this could be included as a separate commit, when it will
>> be time to commit proxy-discoveryd someday.
>> Those 2 files represent more than 2 megabytes, so I did not want to
>> flood the ml.
> duktape should not be embedded in systemd. It's a separate project,
> which gets updates and has a life of its own.
> 
> Preferably, duktape would be packaged by distributions and we would
> use it like any other external library. If that is not possible, we'll
> have to look for some other option, but only then. For example,
> git submodule is still better than embedding of a snapshot.

that is not how this actually works with duktape. The release versions are 
building a single duktape.[ch] that you can only find in the releases. 
Otherwise you have to stitch that manually. It is pretty much designed to be 
included into other project as a copy. That is why nobody has actually packages 
this so far.

So while I get that including copies of other libraries is generally bad for 
security update reasons, but this one would qualify as an exception. It is just 
that we have to monitor duktape upstream releases and with that also update the 
duktape version in systemd.

It is something that is not the greatest solution, but something that is 
feasible. Keep in mind that embedding your JS engine right into your 
application allows for optimizations that you really want. We have build 
PACrunner against V8 and MozJS and these are two really painful experiences. 
For anybody wanting to build a small system, dealing with these two upstream 
packages is a nightmare. It sort of works in a large distribution like Fedora, 
but anything small it is not an option.

Regards

Marcel

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


Re: [systemd-devel] [RFC 0/6] A network proxy management daemon, systemd-proxy-discoveryd

2015-04-13 Thread Dan Williams
On Sun, 2015-04-12 at 20:31 +0200, Lennart Poettering wrote:
> On Fri, 10.04.15 14:05, Dan Williams (d...@redhat.com) wrote:
> 
> > > > > So idea would basically be that we provide in all three daemons calls
> > > > > like:
> > > > > 
> > > > > SetAdditionalNTP(ias)
> > > > > SetAdditionalDNS(ia(uay)) 
> > > > 
> > > > I would strongly suggest using strings in the API for IP addresses, not
> > > > byte arrays.  
> > > 
> > > Why so?
> > 
> > It's much easier to use for languages other than C, like Python or JS or
> > Ruby or dbus-send or even C in many cases.  We originally used binary
> > addresses in the D-Bus interface for NetworkManager, and eventually
> > found that was the wrong choice for these reasons.  It's also easier to
> > pull in and out of D-Bus, with dbus-glib or GIO.  I'm not sure about
> > sd_bus though.
> 
> I am very much convinced that passing normalized data through dbus is
> the right thing. And that implies byte arrays for binary data such as
> IP addresses... We have structured data in dbus, that's pretty much
> the biggest benefit of dbus over raw sockets. We should use it and not
> chicken out, because our bindings suck and encode everything in
> formatted strings after all... If it's not easy to decode structured
> data with your bus binding, then the answer is to fix the bus binding,
> not to just revert to unstructured data.
> 
> > > > Also, remember that we want to push domains too, for split
> > > > DNS in the VPN or other cases.  So it really should be something like:
> > > > 
> > > > SetAdditionalDNS(ia(sas))  (index, array[domain, array[server]])
> > > > 
> > > > what were the (uay) arguments you proposed?
> > > 
> > > resolved supprots split DNS, but it will not allow multiple DNS
> > > servers with different domains on the same interface. Instead, you
> > > bind a set of DNS servers and a set of domains to an interface, and
> > > that's it, there's no further connection between servers and domains.
> > 
> > If I understand correctly, this means you cannot direct *.foobar.com to
> > one set of DNS servers,and *.baz.com to another set, assuming those are
> > bound to the same interface?  Why not allow that?
> 
> Well, resolved is not supposed to support every crazy set up people
> can think of. I mean, if they want that they can certainly run their
> own local DNS server. resolved is about figuring out the common
> usecases and covering them nicely and automatically, and little else.
> 
> > > I fail to see the strong usecase for allowing per-server domains. I
> > > mean in the VPn case you have one explicit interface for the VPN
> > > connection, so this should be covered with the current design.
> > 
> > IPSec doesn't give you an kernel netdev at all, it's just routing on an
> > existing interface.  So for that interface, you'd have both the VPN
> > servers and upstream servers, all bound to the same interface, and no
> > ability to direct VPN domains to the VPN servers since they are all on
> > the same interface?
> 
> IPSec used in that mode does not convey DNS server info, does it? In
> that case it's supposed to be transparent, but with such domain server
> config it wouldn't be transparent after all...

It certainly can convey DNS information.  Try the Red Hat VPN servers
with libreswan for an example.  Your eth0 will now have the original,
DHCP/static/etc provided LAN DNS servers, and secondary VPN-provided DNS
servers that can be tied to one or more domains that the VPN also
provided.

Dan

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


Re: [systemd-devel] [PATCH 2/2] Add +C attrib to the journal files directories

2015-04-13 Thread Lennart Poettering
On Sun, 12.04.15 20:30, Goffredo Baroncelli (kreij...@libero.it) wrote:

> From: Goffredo Baroncelli 
> 
> Add +C attrib to the journal files directories. The journal file format
> behaves bad on a BTRFS filesystem: the performances decrease during the
> time.
> To avoid this issue, this tmpfile.d snippet sets the NOCOW attribute to the
> journal files directories, so newly created journal files inherit the NCOOW
> attribute.
> 
> Be aware that the NOCOW attribute disables the BTRFS checksums, prevent BTRFS
> to rebuild a corrupted file in a RAIDx filesystem. However the perfomances
> increase.
> In a single disk filesystem (or a filesystem without redundancy) it is safe
> to use the NOCOW flags.

Applied this one with some changes. Other paztch will follow shortly.

Lennart

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


Re: [systemd-devel] [RFC 0/6] A network proxy management daemon, systemd-proxy-discoveryd

2015-04-13 Thread Marcel Holtmann
Hi Tomasz,

>> Have you looked into MuJS instead of duktape? http://mujs.com/
>> It has a C api similar to Lua, with all state encapsulated in an
>> opaque structure, that you interface with via a virtual stack.
> 
> It could be easily tested. I did so the PAC related code is contained in a 
> specific
> place, sharing nothing specific but a somehow generic internal api.
> I have personally no bold opinion at this time about which JS engine to use.

interesting to test, but it clearly states that its license is AGPL which is 
clearly more restrictive than the permissive license of duktape.

Regards

Marcel

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


Re: [systemd-devel] [PATCH] udevd: fix synchronization with settle when handling inotify events

2015-04-13 Thread Daniel Drake
On Sat, Apr 11, 2015 at 5:13 AM, David Herrmann  wrote:
> Nice catch!
>
> There's indeed a small race between handling inotify and queuing up
> the change-event. We need to re-loop there. One day we should switch
> to sd-event to avoid such bugs... I mean the symptom is inherent to
> queuing up events while handling them. Meh!

Thanks for reviewing this. Reading your comment, I wonder if there is
a small bug in the solution here.

Sometimes we may handle inotify events, but without generating change
events. After my change, we will loop again, but there may be no
events pending, in which case we will block on the 3 second timeout
before completing the next loop iteration and replying to settle's
ping message.

Do you agree? Should I improve this to only do the extra loop
iteration in the case where we generated change events, or somehow
make the next loop iteration have timeout 0 (non-blocking)?

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


Re: [systemd-devel] 'is-enabled' supported for legacy sysvinit scripts?

2015-04-13 Thread Martin Pitt
Jon Severinsson [2015-04-13 15:18 +0200]:
> Debian patches the systemd legacy sysv support to use update-rc.d (the Debian-
> native equivalent) instead of chkconfig, but update-rc.d does not have a is-
> enabled feature, so is-enabled will not work on legacy init scripts in the 
> Debian build of systemd...

For the record, this is tracked in https://bugs.debian.org/705254

Martin
-- 
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


[systemd-devel] systemd.network and IPv6 addrlabel

2015-04-13 Thread Christian Brunotte
Hello

I'm playing around with systemd's network configuration and try to convert the
following Debian configuration.

 iface eth1 inet6 auto
 # Static IP and random dynamic IPs for external targets
 autoconf 1
 use_tempaddr 2
 privext 1
 # My static IP
 up ip addr add 2001::0:222:33::/64 dev eth1
 # Label 1 is predefined as ::/0
 # Label 666 is for my static IP and networks I want to use it with
 up ip addrlabel add prefix 2001::0:222:33::/128 label 666
 up ip addrlabel add prefix 2001:::/32   label 666

Can I do this with systemd.network? Is there some kind of hoook to
start shell scripts like the up/pre-up/down/post-down hooks in the
Debian configuration?

Best Regards

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


Re: [systemd-devel] [RFC 4/6] proxy-discoveryd: Execute the PAC based proxy in a thread

2015-04-13 Thread Lennart Poettering
On Mon, 13.04.15 14:37, Tomasz Bursztyka (tomasz.burszt...@linux.intel.com) 
wrote:

> Hi Lennart and Marcel,
> 
> I'll stick to the thread solution for now, moving the dbus handling in the
> main process.
> I forgot to check about this issue.

I think the cancellation issue should not be ignored. Doing this as
thread is not really an option if we cannot cancel this properly after
a timeout.

I don't see why an out-of-process logic would be that hard... It could
be implemented in a pretty minimalistic way: create an
AF_UNIX/SOCK_SEQPACKET socket before forking, leave one end of it in
the daemon, and pass the other end to all the worker processes. Then,
when there's an URL to look up, just write it in one datagram into the
socket from the daemon side, and wait for one reply on it. Make all
worker threads read() from the other side (the same socket in all of
them), and the kernel will then deliver it to an idle worker thread
and all is good.

Lennart

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


Re: [systemd-devel] [RFC 2/6] proxy-discoveryd: Add the basics for parsing the default configuration

2015-04-13 Thread Lennart Poettering
On Sun, 12.04.15 13:03, Marcel Holtmann (mar...@holtmann.org) wrote:

> Hi Lennart,
> 
>  The config file will be in /etc/systemd/proxy/.conf
>  
>  It currently only load "Proxy" parts, with the key PAC. Rest is ignored.
>  The PAC keyword is a path to a .pac file (a specific js script for proxy
>  configuration).
>  
>  Only one PAC based proxy configuration will be loaded at a time.
> >>> 
> >>> (Just a side note: I figure in the long run we should probably track
> >>> PAC data per-interface (plus maybe one global setting), so that
> >>> clients can query this specifically for an interface, and so that we
> >>> can search PAC data over the right network. But I figure for now this
> >>> doesn't matter too much.).
> >> 
> >> why would you have a global PAC file. I think they should be all per
> >> interface and nothing else.
> > 
> > Well, maybe not a global PAC file, but probably an explicitly
> > configurable global HTTP proxy, if people want that... I mean, it is a
> > pretty common setting to have I figure, and the daemon should
> > proibably cover both PAC and straightforward proxy config...
> 
> yes that makes sense. So what we have done in PACrunner was that
> instead of a PAC file you could just give it the HTTP proxy
> address. And that would also work per interface. When then libproxy
> or someone did FindProxyForURL, the configured HTTP proxy URL was
> returned.

Yupp, definitely makes sense. networkd should have a setting
HttpProxy=  or so that proxy-discoveryd should pick up.

Oh, and there's one more thing I want: I figure for the bigger
webbrowsers which have their own JS engine, it might make sense to
export the discovered per-interface PAC files directly, so that they
can opt for making use of our WPAD logic, but can run the JS stuff on
their own. And this would then probably also mean synthesizing a PAC
file from that networkd information.

> I still wonder if it is a good idea to have a global proxy there. I
> would assume you just configure the proxy per interface in your
> network configuration file. This should be treated similar to DNS
> configuration. You want this per interface.

Well, we do have per-interface DNS as well as global DNS. We also have
per-interface NTP as well as global NTP. I am tempted to say that this
shouldn't be any different...

Lennart

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


Re: [systemd-devel] [PATCH] journal: don't complain about audit socket errors in a container.

2015-04-13 Thread Lennart Poettering
On Sun, 12.04.15 20:51, frank.thalb...@tuta.io (frank.thalb...@tuta.io) wrote:

> This fixes an issue within journald aborting when running inside
> archlinux container via systemd-nspawn on a debian host with audit
> enabled kernel.

What kind of containers are these? LXC? docker?

nspawn at least grants audit caps to containers. If you don't grant
audit caps you cannot boot distros like Fedora at all, since much of
the PAM audit code in Fedora is written to fail completely if audit is on
in the kernel, but cannot be used.

> +#ifdef HAVE_AUDIT
>   r = server_open_audit(s);
>   if (r < 0)
>   return r;
> +#endif

Hmm, exluding the audit code from the build if HAVE_AUDIT is not set
is certainly a good idea, but we generally try to keep #ifdeffery out
of .c files. More specifically, the journald-audit.c file should not
be compiled and linked at all on non-audit builds, and
journald-audit.h should contain the #ifdeffery that causes
server_open_audit() to become a NOP on such builds. Would be happy to
take a patch for that.

Lennart

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


Re: [systemd-devel] 'is-enabled' supported for legacy sysvinit scripts?

2015-04-13 Thread Lennart Poettering
On Sun, 12.04.15 21:00, Nusenu (nus...@openmailbox.org) wrote:

> Hi,
> 
> I'm trying to find out whether I'm looking at an ansible or systemctl
> bug.
> 
> Does systemctl aim to support the 'is-enabled' command for legacy sysv
> initscripts?

On Fedora, when systemctl encounters a sysv service it dispatches this
to chkconfig. I offered to merge similar code that dispatches this to
update-rc.d on Debian-derived distros, but so far I didn't get a good
patch for this from the Debian guys. Offer is still valid though...

Lennart

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


Re: [systemd-devel] SystemD, Gnome permission problems

2015-04-13 Thread Lennart Poettering
On Mon, 13.04.15 02:37, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:

> > Now, of these the first item is a work-around for broken daemons, and
> > this should really be better fixed in the daemons themselves. A daemon
> > that does not require tmpfiles is a good daemon. The third item is a
> > work-around against really broken semantics of X11, that cannot really
> > be fixed without breaking compat... But this issue is certainly not
> > something new code will fall for (hopefully)... The fourth item is a
> > work-around for broken semantics of UNIX, where files cannot be bound
> > to proper lifecycles of other objects, such as processes... "Aging"
> > these dirs is actually really nasty, since its cleanups are in no way
> > bound to the actual algorithms creating the files: code that generates
> > a lot of files in a short time, will be cleaned up only much later,
> > when the tmpfiles job happens to runs again...
> 
> But /tmp is also used for shared state (between services, but also
> between different applications run by the same user, different
> sessions of the same user, etc.). 

Not really. Shared state is what /run is for really.

Note that we kinda encourage all services to set PrivateTmp=, and
consider daemons which need a shared /tmp to be a thing of the past,
that should better be fixed.

If you want to pass files in /tmp around, that's fine, but it's almost
certainly something where aging is the wrong answer. I much better
lifecycle management for files in /tmp could be something like a
O_TMPFILE that however keeps the filenames in the directory visible as
long as the file is opened. i.e. a scheme where a file is deleted the
moment the last fd is closed for it, but until then is a normal file
with a filename and everything, where the filename can be used to
create additional fds, and all.

> Such files are not bound to any process, or any session, and should
> not be. For example, when I download stuff I often stick it in /tmp,
> so that it stays around if I need it, but goes away without me
> taking any concious action after some time.  Gnome's thumbnails are
> a bit like this: they are not bound to the life cycle of anything,
> we just want them to go away when they have not been used for a
> while.

I very much disagree. Especially the thumbnailing thing is something
where the clean-up should be primarily bound to size on disk
constraints, and tmpfiles really cannot help with size on disk...

Doing this based on aging via tmpfiles would be a hack, that people
might resort to only because proper size-based cleanup is missing.

> The application which creates thumbnails (or other similar caches)
> *should* take care to not use to much space when creating things rapidly.
> I assume that gnome does that already. But the application cannot really
> implement good time based cleanup, especially that time based cleanup
> is especially useful when the application is *not* running.

I am not sure what benefit it would be to have time-based clean-up for
this. Enforcing a disk space limit, plus replacement in LRU style
would make sense, but there's little point in deleting old entries
really.

And even if we say that time-based clean-up would be useful: the
*primary* logic should *still* be about disk space limits, nothing
else. And if you implement that in the thumbnailer, then you should
also implement the time-based cleanups.

Let's not forget that tmpfiles is actually a really dumb thing. It
looks at the file dates only, and has no idea at all about how files
fit together. It will remove individual files with completely
disregard of how they fit in the big picture, and related to other
files. Such a scheme is OK for cases like temporary files in /tmp,
since every file there probably can be considered independently of all
others. But for things like the thumbnailing it might actually not be
true at all...

> > Hence, if this is done at all, it really should be the user's code
> > that runs this, not the system code. 
>
> Well, the whole discussion started with the fact that this cannot be
> implemented as user code without being crippled.

Which kinda indicates that we probably shouldn't be doing that all ;-)

Lennart

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


Re: [systemd-devel] 'is-enabled' supported for legacy sysvinit scripts?

2015-04-13 Thread Jon Severinsson
måndagen den 13 april 2015 07:26:14 skrev  Nusenu:
> Hi Andrei,
> 
> thanks for your reply.
> 
> > systemctl should still support it if your system supports chkconfig
> > to manage initscripts.
> 
> chkconfig was indeed not installed, but even after installing it, the
> 
> is-enabled command output and return codes do not change:
> >> Output in such cases:
> >> 
> >> "Failed to get unit file state for FOO.service: No such file or director
> >> y"
> 
> Is this considered a bug?
> 
> thanks,
> Nusenu
> 
> testing on:
> Debian 8 (chkconfig installed) with systemd 215-14 and
> Ubuntu 15.04 with systemd 219-7ubuntu1 (no chkconfig available)

Debian patches the systemd legacy sysv support to use update-rc.d (the Debian-
native equivalent) instead of chkconfig, but update-rc.d does not have a is-
enabled feature, so is-enabled will not work on legacy init scripts in the 
Debian build of systemd...

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


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

2015-04-13 Thread Lennart Poettering
On Mon, 13.04.15 11:31, Lennart Poettering (lenn...@poettering.net) wrote:

> On Sun, 12.04.15 22:19, Goffredo Baroncelli (kreij...@libero.it) wrote:
> 
> > However the original code catch also the case where the file is a soft-link.
> > The same check is performed also by chattr(1); I suggest to leave the 
> > original
> > behavior, changing
> > 
> > fd = open(path, O_RDONLY|O_NONBLOCK|O_CLOEXEC);
> > in
> > fd = open(path, O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_NOFOLLOW);
> > 
> > and checking if the errno is ELOOP. In this case a further check is 
> > performed to 
> > verify if the file is a link or the error is due to a too many symbolic 
> > link.
> > Then an appropriate message error is printed.
> > 
> > What do you think ?
> 
> We should probably either follow symlinks for all of tmpfiles'
> operations or for none. 
> 
> While I generally believe that we probably shouldn't follow symlinks,
> it's really difficult to implement given that fchmodat() currenlty
> doesn't work with AT_SYMLINK_FOLLOW (according to the man page at
> least), and acl_set_file doesn't allow not following symlinks either... :-(
> 
> Hmm, I can't say I like this I must say.
> 
> ideas?

I now fixed much of the code to not follow symlinks anymore. With a
combination of O_PATH and using /proc/self/fd/%i for acl_set_file() I
managed to get all of the xattr, acl, file attribute, chmod, chown
code to not follow symlinks anymore.

I also documented this behaviour in the man page for the lines where
this applies.

I am pretty sure we should not follow symlinks when creating new file
objects or adjusting existing ones (with the notable exception of "w"
lines). Right now we still follow symlinks when creating dirs, fifos,
device nodes. We should fix that too.

Anyway, Goffredo, the file attribute code will not follow symlinks
anymore, hence this should settle this issue you raised.

Lennart

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


Re: [systemd-devel] [RFC 3/6] proxy-discoveryd: Add PAC support through duktape js engine

2015-04-13 Thread Zbigniew Jędrzejewski-Szmek
On Mon, Apr 13, 2015 at 01:24:57PM +0300, Tomasz Bursztyka wrote:
> Hi Tom,
> 
> >>--- a/Makefile.am
> >>+++ b/Makefile.am
> >>@@ -5895,12 +5895,19 @@ rootlibexec_PROGRAMS += \
> >> systemd-proxy-discoveryd
> >>
> >>  systemd_proxy_discoveryd_SOURCES = \
> >>+   src/proxy-discovery/duktape.h \
> >>+   src/proxy-discovery/duktape.c \
> >These files are not included in the patch, how do you intend to ship them?
> 
> I figured this could be included as a separate commit, when it will
> be time to commit proxy-discoveryd someday.
> Those 2 files represent more than 2 megabytes, so I did not want to
> flood the ml.
duktape should not be embedded in systemd. It's a separate project,
which gets updates and has a life of its own.

Preferably, duktape would be packaged by distributions and we would
use it like any other external library. If that is not possible, we'll
have to look for some other option, but only then. For example,
git submodule is still better than embedding of a snapshot.

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


Re: [systemd-devel] [PATCH v2 1/2] input_id: Make test_pointer / test_keys return if they've found anything

2015-04-13 Thread Hans de Goede

Hi,

On 13-04-15 14:41, Zbigniew Jędrzejewski-Szmek wrote:

On Mon, Apr 13, 2015 at 11:15:00AM +0200, Hans de Goede wrote:

Make test_pointer / test_keys return a boolean indicating whether or not
they've set any properties on the device.

While touching allmost all test_bit() using lines anyways also remove
the extra space between the function name and the '(' (coding style issue).

Signed-off-by: Hans de Goede 
---
  src/udev/udev-builtin-input_id.c | 98 
  1 file changed, 60 insertions(+), 38 deletions(-)

diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c
index ecfc447..2c8a9ee 100644
--- a/src/udev/udev-builtin-input_id.c
+++ b/src/udev/udev-builtin-input_id.c
@@ -126,7 +126,7 @@ static void get_cap_mask(struct udev_device *dev,
  }

  /* pointer devices */
-static void test_pointers (struct udev_device *dev,
+static bool test_pointers (struct udev_device *dev,

 ^
Those spaces after function name declaration should go too.


That seems like something for a separate commit.


And
the white-space changes should probably be a separate commit becuase
now it's hard to see what is going on ;)


Almost all changed lines are changed because they must be changed
(adding { or }, changing indentation), only 5 or so changed lines
are purely removing the extra space between test_bit and '(' which
is exactly why I squashed in those changes.

Regards,

Hans




Zbyszek


@@ -135,77 +135,93 @@ static void test_pointers (struct udev_device *dev,
 bool test) {
  int is_mouse = 0;
  int is_touchpad = 0;
+bool ret = false;

-if (test_bit (INPUT_PROP_ACCELEROMETER, bitmask_props)) {
+if (test_bit(INPUT_PROP_ACCELEROMETER, bitmask_props)) {
  udev_builtin_add_property(dev, test, "ID_INPUT_ACCELEROMETER", 
"1");
-return;
+return true;
  }

-if (!test_bit (EV_KEY, bitmask_ev)) {
-if (test_bit (EV_ABS, bitmask_ev) &&
-test_bit (ABS_X, bitmask_abs) &&
-test_bit (ABS_Y, bitmask_abs) &&
-test_bit (ABS_Z, bitmask_abs))
+if (!test_bit(EV_KEY, bitmask_ev)) {
+if (test_bit(EV_ABS, bitmask_ev) &&
+test_bit(ABS_X, bitmask_abs) &&
+test_bit(ABS_Y, bitmask_abs) &&
+test_bit(ABS_Z, bitmask_abs)) {
  udev_builtin_add_property(dev, test, "ID_INPUT_ACCELEROMETER", 
"1");
-return;
+ret = true;
+}
+return ret;
  }

-if (test_bit (EV_ABS, bitmask_ev) &&
-test_bit (ABS_X, bitmask_abs) && test_bit (ABS_Y, bitmask_abs)) {
-if (test_bit (BTN_STYLUS, bitmask_key) || test_bit 
(BTN_TOOL_PEN, bitmask_key))
+if (test_bit(EV_ABS, bitmask_ev) &&
+test_bit(ABS_X, bitmask_abs) && test_bit(ABS_Y, bitmask_abs)) {
+if (test_bit(BTN_STYLUS, bitmask_key) || 
test_bit(BTN_TOOL_PEN, bitmask_key)) {
  udev_builtin_add_property(dev, test, "ID_INPUT_TABLET", 
"1");
-else if (test_bit (BTN_TOOL_FINGER, bitmask_key) && !test_bit 
(BTN_TOOL_PEN, bitmask_key))
+ret = true;
+} else if (test_bit(BTN_TOOL_FINGER, bitmask_key) && 
!test_bit(BTN_TOOL_PEN, bitmask_key)) {
  is_touchpad = 1;
-else if (test_bit (BTN_MOUSE, bitmask_key))
+} else if (test_bit(BTN_MOUSE, bitmask_key)) {
  /* This path is taken by VMware's USB mouse, which has
   * absolute axes, but no touch/pressure button. */
  is_mouse = 1;
-else if (test_bit (BTN_TOUCH, bitmask_key))
+} else if (test_bit(BTN_TOUCH, bitmask_key)) {
  udev_builtin_add_property(dev, test, "ID_INPUT_TOUCHSCREEN", 
"1");
+ret = true;
  /* joysticks don't necessarily have to have buttons; e. g.
   * rudders/pedals are joystick-like, but buttonless; they have
   * other fancy axes */
-else if (test_bit (BTN_TRIGGER, bitmask_key) ||
- test_bit (BTN_A, bitmask_key) ||
- test_bit (BTN_1, bitmask_key) ||
- test_bit (ABS_RX, bitmask_abs) ||
- test_bit (ABS_RY, bitmask_abs) ||
- test_bit (ABS_RZ, bitmask_abs) ||
- test_bit (ABS_THROTTLE, bitmask_abs) ||
- test_bit (ABS_RUDDER, bitmask_abs) ||
- test_bit (ABS_WHEEL, bitmask_abs) ||
- test_bit (ABS_GAS, bitmask_abs) ||
- test_bit (ABS_BRAKE, bitmask_abs

Re: [systemd-devel] [PATCH 2/2] udev: Allow detection of udevadm settle timeout

2015-04-13 Thread Nir Soffer
On Sat, Apr 11, 2015 at 6:58 PM, David Herrmann  wrote:
>> A program running this tool can detect a timeout (expected) or an error
>> (unexpected), and can change the program flow based on this result.
>>
>> Without this, the only way to detect a timeout is to implement the timeout
>> in the program calling udevadm.
>
> I cannot really see a use-case here. I mean, yeah, the commit-message
> says it warns about timeouts but fails loudly on real errors. But
> again, what's the use-case? Why is a timeout not a real error? Why do
> you need to handle it differently?

Timeout means that the value I chose may be too small, or the machine
is overloaded. The administrator may need to configure the system
differently.

Other errors are not expected, and typically unexpected errors in an
underlying tool means getting the developer of the underlying tool
involved.

> Anyway, if it's only about diagnostics this patch seems fine to me.

Yes, it is mainly about diagnostics, and making it easier to debug and support.

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


Re: [systemd-devel] [PATCH v2 1/2] input_id: Make test_pointer / test_keys return if they've found anything

2015-04-13 Thread Zbigniew Jędrzejewski-Szmek
On Mon, Apr 13, 2015 at 11:15:00AM +0200, Hans de Goede wrote:
> Make test_pointer / test_keys return a boolean indicating whether or not
> they've set any properties on the device.
> 
> While touching allmost all test_bit() using lines anyways also remove
> the extra space between the function name and the '(' (coding style issue).
> 
> Signed-off-by: Hans de Goede 
> ---
>  src/udev/udev-builtin-input_id.c | 98 
> 
>  1 file changed, 60 insertions(+), 38 deletions(-)
> 
> diff --git a/src/udev/udev-builtin-input_id.c 
> b/src/udev/udev-builtin-input_id.c
> index ecfc447..2c8a9ee 100644
> --- a/src/udev/udev-builtin-input_id.c
> +++ b/src/udev/udev-builtin-input_id.c
> @@ -126,7 +126,7 @@ static void get_cap_mask(struct udev_device *dev,
>  }
>  
>  /* pointer devices */
> -static void test_pointers (struct udev_device *dev,
> +static bool test_pointers (struct udev_device *dev,
^
Those spaces after function name declaration should go too. And
the white-space changes should probably be a separate commit becuase
now it's hard to see what is going on ;)

Zbyszek

> @@ -135,77 +135,93 @@ static void test_pointers (struct udev_device *dev,
> bool test) {
>  int is_mouse = 0;
>  int is_touchpad = 0;
> +bool ret = false;
>  
> -if (test_bit (INPUT_PROP_ACCELEROMETER, bitmask_props)) {
> +if (test_bit(INPUT_PROP_ACCELEROMETER, bitmask_props)) {
>  udev_builtin_add_property(dev, test, 
> "ID_INPUT_ACCELEROMETER", "1");
> -return;
> +return true;
>  }
>  
> -if (!test_bit (EV_KEY, bitmask_ev)) {
> -if (test_bit (EV_ABS, bitmask_ev) &&
> -test_bit (ABS_X, bitmask_abs) &&
> -test_bit (ABS_Y, bitmask_abs) &&
> -test_bit (ABS_Z, bitmask_abs))
> +if (!test_bit(EV_KEY, bitmask_ev)) {
> +if (test_bit(EV_ABS, bitmask_ev) &&
> +test_bit(ABS_X, bitmask_abs) &&
> +test_bit(ABS_Y, bitmask_abs) &&
> +test_bit(ABS_Z, bitmask_abs)) {
>  udev_builtin_add_property(dev, test, 
> "ID_INPUT_ACCELEROMETER", "1");
> -return;
> +ret = true;
> +}
> +return ret;
>  }
>  
> -if (test_bit (EV_ABS, bitmask_ev) &&
> -test_bit (ABS_X, bitmask_abs) && test_bit (ABS_Y, bitmask_abs)) {
> -if (test_bit (BTN_STYLUS, bitmask_key) || test_bit 
> (BTN_TOOL_PEN, bitmask_key))
> +if (test_bit(EV_ABS, bitmask_ev) &&
> +test_bit(ABS_X, bitmask_abs) && test_bit(ABS_Y, bitmask_abs)) {
> +if (test_bit(BTN_STYLUS, bitmask_key) || 
> test_bit(BTN_TOOL_PEN, bitmask_key)) {
>  udev_builtin_add_property(dev, test, 
> "ID_INPUT_TABLET", "1");
> -else if (test_bit (BTN_TOOL_FINGER, bitmask_key) && 
> !test_bit (BTN_TOOL_PEN, bitmask_key))
> +ret = true;
> +} else if (test_bit(BTN_TOOL_FINGER, bitmask_key) && 
> !test_bit(BTN_TOOL_PEN, bitmask_key)) {
>  is_touchpad = 1;
> -else if (test_bit (BTN_MOUSE, bitmask_key))
> +} else if (test_bit(BTN_MOUSE, bitmask_key)) {
>  /* This path is taken by VMware's USB mouse, which 
> has
>   * absolute axes, but no touch/pressure button. */
>  is_mouse = 1;
> -else if (test_bit (BTN_TOUCH, bitmask_key))
> +} else if (test_bit(BTN_TOUCH, bitmask_key)) {
>  udev_builtin_add_property(dev, test, 
> "ID_INPUT_TOUCHSCREEN", "1");
> +ret = true;
>  /* joysticks don't necessarily have to have buttons; e. g.
>   * rudders/pedals are joystick-like, but buttonless; they 
> have
>   * other fancy axes */
> -else if (test_bit (BTN_TRIGGER, bitmask_key) ||
> - test_bit (BTN_A, bitmask_key) ||
> - test_bit (BTN_1, bitmask_key) ||
> - test_bit (ABS_RX, bitmask_abs) ||
> - test_bit (ABS_RY, bitmask_abs) ||
> - test_bit (ABS_RZ, bitmask_abs) ||
> - test_bit (ABS_THROTTLE, bitmask_abs) ||
> - test_bit (ABS_RUDDER, bitmask_abs) ||
> - test_bit (ABS_WHEEL, bitmask_abs) ||
> - test_bit (ABS_GAS, bitmask_abs) ||
> - test_bit (ABS_BRAKE, bitmask_abs))
> +} else if (test_bit(BTN_TRIGGER, bitmask_key) ||
> +   test_bit(BTN_A, bitmask_key) ||
> +   test_bit(BTN_1, bitmask_key) ||
> +  

Re: [systemd-devel] dbus-1/kdbus - question about 'queued owners'

2015-04-13 Thread Lukasz Skalski
On 04/13/2015 01:46 PM, Daniel Mack wrote:
> Hi Lukasz,
> 

Hi,

> [+dbus ML]
> 
> On 04/10/2015 04:20 PM, Lukasz Skalski wrote:
>> Currently I'm working on some testsuite (let's call it dbus1-spec-test)
>> for dbus-1 specification. My idea is to test dbus-1 specification
>> coverage on systems with dbus-daemon and on systems without dbus-daemon
>> (but with latest systemd, bus-proxyd and kdbus) which will allow us (and
>> all userspace apps) to smoothly switch to kdbus. First results of tests
>> are really good:
> 
> Nice, thanks a lot for doing this!
> 
>> My testsuite have found only one inconsistency between dbus-1 and
>> kdbus-enabled system. Problematic testcase (this one tests rather some
>> dbus-daemon/bus-proxyd behaviors than specification) is as follow:
>>
>> test_request_name_6 (void)
>> {
>>   GDBusConnection *connection_a;
>>   GDBusConnection *connection_b;
>>
>>   BusRequestNameFlagsReply request_reply;
>>   BusReleaseNameFlagsReply release_reply;
>>
>>   /* connect and set up two D-Bus client connections */
>>   connection_a = connect_to_bus();
>>   connection_b = connect_to_bus();
>>
>>   /* 'connection_a' - synchronously acquire name on the bus */
>>   request_reply = request_name (connection_a, "org.my.busname",
>> DBUS_NAME_FLAG_REPLACE_EXISTING |
>> DBUS_NAME_FLAG_DO_NOT_QUEUE);
>>
>>   /* 'connection_a' should be primary owner */
>>   CU_ASSERT (request_reply == DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER);
>>
>>   /* 'connection_b' tries to own the same well-known name */
>>   request_reply = request_name (connection_b, "org.my.busname",
>> DBUS_NAME_FLAG_REPLACE_EXISTING);
>>
>>   /* 'connection_b' should be appended to the queue */
>>   CU_ASSERT (request_reply == DBUS_REQUEST_NAME_REPLY_IN_QUEUE);
>>
>>   /* once again 'connection_b' tries to own the same name */
>>   request_reply = request_name (connection_b, "org.my.busname",
>> DBUS_NAME_FLAG_REPLACE_EXISTING);
>>
>>   /* and once again we should get the same return code */
>>   CU_ASSERT (request_reply == DBUS_REQUEST_NAME_REPLY_IN_QUEUE);
> 
> The D-Bus spec isn't totally clear about the return code here, but it is
> about what should happen:
> 
> "If replacement is not possible, *and the method caller is currently not
> in the queue*, the method caller is appended to the queue."
> 
> dbus-daemon seems to return IN_QUEUE to inform the caller that the name
> is in the queue, no matter if the call it replies to was responsible for
> creating the queue entry or if that was already the case.
> 
>>   /* 'connection_a' releases name */
>>   release_reply = release_name (connection_a, "org.my.busname");
>>   CU_ASSERT (release_reply == DBUS_RELEASE_NAME_REPLY_RELEASED);
>>
>>   /* 'connection_b' (now primary owner) also release name
>>   release_reply = release_name (connection_b, "org.my.busname");
>>   CU_ASSERT (release_reply == DBUS_RELEASE_NAME_REPLY_RELEASED);
>>
>>   /* 'connection_b' tries to release once again the same name */
>>   release_reply = release_name (connection_b, "org.my.busname");
>>
>>   /* This assert is source of failure - what we should get
>>  here: REPLY_RELEASED or NON_EXISTENT ? */
>>   CU_ASSERT (release_reply == DBUS_RELEASE_NAME_REPLY_NON_EXISTENT);
> 
> As per the spec, the same connection can only be queued up once for a
> given well-known name, so the correct error code should be NON_EXISTENT.
> 
>> Which solution, kdbus or dbus-daemon, do it correctly?
> 
> kdbus is wrong here IMO. The kernel patch you sent me offlist looks
> resonable. Care to rebase and post it to LKML?

Sure. Thanks for clarification!

> 
> 
> Thanks,
> Daniel
> 
> 

Cheers,
-- 
Lukasz Skalski
Samsung R&D Institute Poland
Samsung Electronics
l.skal...@samsung.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [RFC 0/6] A network proxy management daemon, systemd-proxy-discoveryd

2015-04-13 Thread Tomasz Bursztyka

Hi,


Have you looked into MuJS instead of duktape? http://mujs.com/
It has a C api similar to Lua, with all state encapsulated in an
opaque structure, that you interface with via a virtual stack.


It could be easily tested. I did so the PAC related code is contained in 
a specific

place, sharing nothing specific but a somehow generic internal api.
I have personally no bold opinion at this time about which JS engine to use.

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


Re: [systemd-devel] dbus-1/kdbus - question about 'queued owners'

2015-04-13 Thread Daniel Mack
Hi Lukasz,

[+dbus ML]

On 04/10/2015 04:20 PM, Lukasz Skalski wrote:
> Currently I'm working on some testsuite (let's call it dbus1-spec-test)
> for dbus-1 specification. My idea is to test dbus-1 specification
> coverage on systems with dbus-daemon and on systems without dbus-daemon
> (but with latest systemd, bus-proxyd and kdbus) which will allow us (and
> all userspace apps) to smoothly switch to kdbus. First results of tests
> are really good:

Nice, thanks a lot for doing this!

> My testsuite have found only one inconsistency between dbus-1 and
> kdbus-enabled system. Problematic testcase (this one tests rather some
> dbus-daemon/bus-proxyd behaviors than specification) is as follow:
> 
> test_request_name_6 (void)
> {
>   GDBusConnection *connection_a;
>   GDBusConnection *connection_b;
> 
>   BusRequestNameFlagsReply request_reply;
>   BusReleaseNameFlagsReply release_reply;
> 
>   /* connect and set up two D-Bus client connections */
>   connection_a = connect_to_bus();
>   connection_b = connect_to_bus();
> 
>   /* 'connection_a' - synchronously acquire name on the bus */
>   request_reply = request_name (connection_a, "org.my.busname",
> DBUS_NAME_FLAG_REPLACE_EXISTING |
> DBUS_NAME_FLAG_DO_NOT_QUEUE);
> 
>   /* 'connection_a' should be primary owner */
>   CU_ASSERT (request_reply == DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER);
> 
>   /* 'connection_b' tries to own the same well-known name */
>   request_reply = request_name (connection_b, "org.my.busname",
> DBUS_NAME_FLAG_REPLACE_EXISTING);
> 
>   /* 'connection_b' should be appended to the queue */
>   CU_ASSERT (request_reply == DBUS_REQUEST_NAME_REPLY_IN_QUEUE);
> 
>   /* once again 'connection_b' tries to own the same name */
>   request_reply = request_name (connection_b, "org.my.busname",
> DBUS_NAME_FLAG_REPLACE_EXISTING);
> 
>   /* and once again we should get the same return code */
>   CU_ASSERT (request_reply == DBUS_REQUEST_NAME_REPLY_IN_QUEUE);

The D-Bus spec isn't totally clear about the return code here, but it is
about what should happen:

"If replacement is not possible, *and the method caller is currently not
in the queue*, the method caller is appended to the queue."

dbus-daemon seems to return IN_QUEUE to inform the caller that the name
is in the queue, no matter if the call it replies to was responsible for
creating the queue entry or if that was already the case.

>   /* 'connection_a' releases name */
>   release_reply = release_name (connection_a, "org.my.busname");
>   CU_ASSERT (release_reply == DBUS_RELEASE_NAME_REPLY_RELEASED);
> 
>   /* 'connection_b' (now primary owner) also release name
>   release_reply = release_name (connection_b, "org.my.busname");
>   CU_ASSERT (release_reply == DBUS_RELEASE_NAME_REPLY_RELEASED);
> 
>   /* 'connection_b' tries to release once again the same name */
>   release_reply = release_name (connection_b, "org.my.busname");
> 
>   /* This assert is source of failure - what we should get
>  here: REPLY_RELEASED or NON_EXISTENT ? */
>   CU_ASSERT (release_reply == DBUS_RELEASE_NAME_REPLY_NON_EXISTENT);

As per the spec, the same connection can only be queued up once for a
given well-known name, so the correct error code should be NON_EXISTENT.

> Which solution, kdbus or dbus-daemon, do it correctly?

kdbus is wrong here IMO. The kernel patch you sent me offlist looks
resonable. Care to rebase and post it to LKML?


Thanks,
Daniel

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


Re: [systemd-devel] [RFC 5/6] proxy-discoveryd: Add the basic parts for handling DBus methods

2015-04-13 Thread Tomasz Bursztyka

Hi Lennart,


Hmm, is this right? Shouldn't we return the error code to the client
instead of eating up and returning "DIRECT"?

Also, why allocate "DIRECT" with strdup() at all?

There are no errors. Either you get a proxy directive or you return
DIRECT to indicate no proxy. What would you do in an error case
anyway. The backup is always assume no proxy.

Well, so far it has been our coding styles to propagate errors up if
they cause the invoked operations to fail, and allow the higher up
code deal with them and possibly eat them up. I mean, the HTTP client
can eat the error up and downgrade to DIRECT on its own, there's no
need to do this from our side already.

of course it can, but it also does not help you at all. You are not making 
anything better here.

I think that one clean interface to this needs to be a API
compatible libproxy. Similar to what we did in PACrunner since that
allows existing application to just use systemd-proxydiscoverd.

Well, clients have to deal with errors anyway, since they are talking
to this via dbus. I mean, the service might simply be missing on the
system, and the client code should then downgrade to DIRECT
anyway... Hence, the right client side behaviour is to eat up *all*
error conditions, regardless if they stem from the dbus code, the
socket calls used by dbus or ultimately from the proxy discovery
daemon...


Either way is fine to me, the end result will be the same anyway: the client
cannot do anything but trying to connect directly. It will need to be 
clearly

told in the api documentation.

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


Re: [systemd-devel] [RFC 4/6] proxy-discoveryd: Execute the PAC based proxy in a thread

2015-04-13 Thread Tomasz Bursztyka

Hi Lennart and Marcel,

I'll stick to the thread solution for now, moving the dbus handling in 
the main process.

I forgot to check about this issue.

We could have a timeout in the pac runtime, so it would cancel the 
duktape context
relevantly, so no need to track the threads from the main process and 
cancel them the

hard way.

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


Re: [systemd-devel] [RFC 3/6] proxy-discoveryd: Add PAC support through duktape js engine

2015-04-13 Thread Tomasz Bursztyka

Le 10/04/2015 18:49, Lennart Poettering a écrit :

On Fri, 10.04.15 15:17, Tomasz Bursztyka (tomasz.burszt...@linux.intel.com) 
wrote:


+struct PAC {
+duk_context *ctx;
+};
+
+static int get_addresses_from_interface(int ifindex, union in_addr_union 
*address) {
+struct ifreq ifr = {};
+int sk;
+
+sk = socket(AF_INET, SOCK_DGRAM, 0);
+if (sk < 0)
+return -1;

No made up errors please! Return -errno or so.


+
+ifr.ifr_ifindex = ifindex;
+
+if (ioctl(sk, SIOCGIFNAME, &ifr) < 0 || ioctl(sk, SIOCGIFADDR, &ifr) < 
0) {
+close(sk);
+return -1;

Same here...

Also, please don't use the old ioctls for querying network
information. Use netlink through sd-rtnl. You can look at the
systemd-resolved sources, they do this already, and this code should
probably do it very similar from that.


Yes, it was just to get something working as a PoC. But nothing to be 
pushed.
I started to work on an sd-rtnl part that will keep track of the 
interfaces, their IPs and

which one it tight to the default route.


+static int create_context(struct PAC *pac, char *pac_file, void *user_data) {
+duk_context *ctx;
+
+ctx = duk_create_heap(NULL, NULL, NULL, NULL, NULL);
+if (!ctx)
+return -ENOMEM;
+
+duk_push_global_object(ctx);
+duk_push_c_function(ctx, pac_dns_resolve, 1);
+duk_put_prop_string(ctx, -2, "dnsResolve");
+duk_push_c_function(ctx, pac_my_ip_address, 0);
+duk_put_prop_string(ctx, -2, "myIpAddress");
+
+duk_push_pointer(ctx, user_data);
+duk_put_prop_string(ctx, -2, "_user_data_");
+
+duk_pop(ctx);
+
+if (duk_peval_file(ctx, pac_file) != 0) {
+duk_destroy_heap(ctx);
+return -EINVAL;
+}

How is error handling done in duktape? The individual functions cannot
fail? And are any errors returned?


There are yes. Let's use them. I'll also hook the internal duktape 
failure functions etc...


And I'll apply the other comments as well

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


[systemd-devel] [PATCH] [RFC] umount: reduce verbosity

2015-04-13 Thread Alban Crequy
From: Alban Crequy 

When a systemd-nspawn container terminates, systemd umounts all bind
mounts that were mounted in the container and generates a log for each
umount.

This additional log_info was added by
bce93b7ac7642426039863493694d8c12812e2a7 for debugging shutdown. But
surely log_debug is enough.

I don't want to see them when when systemd is started with --log-target=null.
There are other log_info that I would like to mute on --log-target=null.
Is changing log_info to log_debug the correct approach?

This patch is useful for:
https://github.com/coreos/rkt/issues/690

When rkt is started with --debug, the systemd logs are printed. When rkt
is started without --debug, systemd is started with --log-target=null in
order to mute the logs.
---
 src/core/umount.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/core/umount.c b/src/core/umount.c
index bee267a..b5a466c 100644
--- a/src/core/umount.c
+++ b/src/core/umount.c
@@ -401,7 +401,7 @@ static int mount_points_list_umount(MountPoint **head, bool 
*changed, bool log_e
 /* Trying to umount. We don't force here since we rely
  * on busy NFS and FUSE file systems to return EBUSY
  * until we closed everything on top of them. */
-log_info("Unmounting %s.", m->path);
+log_debug("Unmounting %s.", m->path);
 if (umount2(m->path, 0) == 0) {
 if (changed)
 *changed = true;
-- 
2.1.4

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


Re: [systemd-devel] issuing 'reboot' command does not print the familiar 'Restarting system.' message

2015-04-13 Thread Daniel Mack
On 04/12/2015 03:46 PM, Lennart Poettering wrote:
> On Fri, Apr 10, 2015 at 10:18 AM, Ani Sinha  wrote:

>> OK I see it now. shutdownd.c eventually issues 'shutdown -r now'. This
>> > gets parsed by shutdown_parse_argv(). Eventually we end up calling
>> > halt_main() ->halt_now() ->reboot(RB_AUTOBOOT).
> Hmm?
> 
> shutdownd's only raison d'etre is to support scheduled shutdowns. It's
> not used unless you actually use scheduled shutdowns. If you just use
> "systemctl poweroff" or so, then it will *not* be used at all, and is
> not involved at all in the shutdown logic.
> 
> And no! shutdownd will never call halt_main(), nor halt_now(), nor
> reboot(RB_AUTOBOUT). If you use shutdownd, then this will result in
> the equiavlent of "systemctl poweroff" and so on, just delayed a bit.
> 
> Please don't confuse shutdownd.c and shutdown.c. The former is just
> there for the scheduled shutdown bits, the latter is the always used
> last part of the shutdown logic.

Also note that shutdownd is one of the oldest pieces of code in the
systemd project. I'm currently working on patches to replace it entirely
with a DBus API in logind.


Thanks,
Daniel

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


Re: [systemd-devel] [RFC 1/6] proxy-discoveryd: Basic core added

2015-04-13 Thread Tom Gundersen
On Mon, Apr 13, 2015 at 12:05 PM, Tomasz Bursztyka
 wrote:
>>> +int manager_new(Manager **ret);
>>> +Manager *manager_free(Manager *m);
>>> +
>>> +DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
>>> +#define _cleanup_manager_free_ _cleanup_(manager_freep)
>>
>> We generally try to avoid this define in internal code, and just use
>> _cleanup_(manager_freep) inline.
>
>
> Ok, I followed networkd but indeed other parts don't do that (machined for
> instance).

Do as I say, not as I do :P I didn't get the memo yet when I wrote the
initial networkd parts, I should fix this up to avoid confusion in the
future.

>>> diff --git a/units/.gitignore b/units/.gitignore
>>> index ad469c1..63ae1af 100644
>>> --- a/units/.gitignore
>>> +++ b/units/.gitignore
>>> @@ -51,6 +51,7 @@
>>>   /systemd-networkd.service
>>>   /systemd-nspawn@.service
>>>   /systemd-poweroff.service
>>> +/systemd-proxy-discoveryd.service
>>>   /systemd-quotacheck.service
>>>   /systemd-random-seed.service
>>>   /systemd-reboot.service
>>> diff --git a/units/systemd-proxy-discoveryd.service.in
>>> b/units/systemd-proxy-discoveryd.service.in
>>> new file mode 100644
>>> index 000..7ac02e0
>>> --- /dev/null
>>> +++ b/units/systemd-proxy-discoveryd.service.in
>>> @@ -0,0 +1,18 @@
>>> +# This file is part of systemd.
>>> +#
>>> +# systemd is free software; you can redistribute it and/or modify it
>>> +# under the terms of the GNU Lesser General Public License as published
>>> by
>>> +# the Free Software Foundation; either version 2.1 of the License, or
>>> +# (at your option) any later version.
>>> +
>>> +[Unit]
>>> +Description=Proxy service
>>> +DefaultDependencies=no
>>> +Requires=dbus.socket
>>> +After=dbus.socket
>>> +Before=remote-fs.target
>>> +
>>> +[Service]
>>> +Restart=on-failure
>>> +ExecStart=@rootlibexecdir@/systemd-proxy-discoveryd
>>> +StandardOutput=null
>>
>> Why this special handling of stdout? Is it the JS engine? Probably
>> should have a comment as it is a bit odd.
>
>
> I copy-pasted the .service file we had for pacrunner, this needs to be
> tailored for proxy-discoveryd.
>
> The stdout directing to /dev/null is to avoid the JS engine to print
> anything.
> Though for duktape specifically, we can hook the debug/printing/warning/...
> functions, so we could
> redirect messages to the logs.

That's what I thought. Would be nice to hook this up properly
internally I think. Either to just redirect to /dev/null there, or to
log correctly.

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


Re: [systemd-devel] [RFC 3/6] proxy-discoveryd: Add PAC support through duktape js engine

2015-04-13 Thread Tomasz Bursztyka

Hi Tom,


--- a/Makefile.am
+++ b/Makefile.am
@@ -5895,12 +5895,19 @@ rootlibexec_PROGRAMS += \
 systemd-proxy-discoveryd

  systemd_proxy_discoveryd_SOURCES = \
+   src/proxy-discovery/duktape.h \
+   src/proxy-discovery/duktape.c \

These files are not included in the patch, how do you intend to ship them?


I figured this could be included as a separate commit, when it will be 
time to commit proxy-discoveryd someday.
Those 2 files represent more than 2 megabytes, so I did not want to 
flood the ml.





 src/proxy-discovery/proxy-discoveryd.c \
 src/proxy-discovery/proxy-discoveryd.h \
 src/proxy-discovery/proxy-discoveryd-manager.c \
+   src/proxy-discovery/proxy-discoveryd-pac.c \
 src/proxy-discovery/proxy-discoveryd-proxy.c \
 src/proxy-discovery/proxy-discoveryd-proxy-gperf.c

+systemd_proxy_discoveryd_CFLAGS = \
+   $(AM_CFLAGS) \
+   -fno-fast-math

Hm, what's this all about?


duktape refuses to compile with fast-math directive. It can be forced 
however.

I just avoided it for the PoC.


+}
+
+address->in = ((struct sockaddr_in *) &ifr.ifr_addr)->sin_addr;
+
+close(sk);
+
+return 0;
+}

The above function is fine as part of a prototype, but for the final
version we should use rtnl like everywhere else, no?


Yep, I only did that for a working PoC. But rtnl based logic should be 
the way to go.



I'll apply the return code comments.

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


Re: [systemd-devel] [RFC 2/6] proxy-discoveryd: Add the basics for parsing the default configuration

2015-04-13 Thread Tomasz Bursztyka

Hi Marcel and Lennart,


The config file will be in /etc/systemd/proxy/.conf

It currently only load "Proxy" parts, with the key PAC. Rest is ignored.
The PAC keyword is a path to a .pac file (a specific js script for proxy
configuration).

Only one PAC based proxy configuration will be loaded at a time.

(Just a side note: I figure in the long run we should probably track
PAC data per-interface (plus maybe one global setting), so that
clients can query this specifically for an interface, and so that we
can search PAC data over the right network. But I figure for now this
doesn't matter too much.).

why would you have a global PAC file. I think they should be all per
interface and nothing else.

Well, maybe not a global PAC file, but probably an explicitly
configurable global HTTP proxy, if people want that... I mean, it is a
pretty common setting to have I figure, and the daemon should
proibably cover both PAC and straightforward proxy config...

yes that makes sense. So what we have done in PACrunner was that instead of a 
PAC file you could just give it the HTTP proxy address. And that would also 
work per interface. When then libproxy or someone did FindProxyForURL, the 
configured HTTP proxy URL was returned.

Of course in these situations, no PAC files are executed, but the D-Bus API for 
talking to systemd-proxydiscoverd to get the proxy to use can be still used.

I still wonder if it is a good idea to have a global proxy there. I would 
assume you just configure the proxy per interface in your network configuration 
file. This should be treated similar to DNS configuration. You want this per 
interface.


The main point of proxy/.conf files will be indeed to get manually 
set proxies.

Keywords such as: Protocol, Host, Port, etc...
We used to set everything through URIs in pacrunner, but it's finally 
easier to set stuff one by one: less room for bugs and errors.


Tom came with the idea we could do it expressive enough so even complex 
proxy setups could be done,

thus such PAC keyword could be removed eventually.
Then only dynamic context due to dhcp/wpad would generate a PAC based 
proxy configuration.


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


Re: [systemd-devel] [RFC 1/6] proxy-discoveryd: Basic core added

2015-04-13 Thread Tomasz Bursztyka

Hi Zbigniew,


+
+[Service]
+Restart=on-failure
+ExecStart=@rootlibexecdir@/systemd-proxy-discoveryd
+StandardOutput=null

What privileges does this daemon require? I'd guess it can run as normal
user at least... Since this is supposed to be executing untrusted javascript
code, let's lock it down heavily from the start.


I agree. It only requires to get access to dbus and netlink, so nothing 
specific to root.


And yes for the JS engine itself there should be more to be done: all JS 
context

should be fully contained. PAC files can be anything which sounds scary.

Tomasz


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


Re: [systemd-devel] [RFC 1/6] proxy-discoveryd: Basic core added

2015-04-13 Thread Tomasz Bursztyka

Hi Lennart,


+
+[Unit]
+Description=Proxy service
+DefaultDependencies=no

Hmm, should this really be an early-boot service? Can you explain?


That's a mistake indeed.


+Requires=dbus.socket
+After=dbus.socket
+Before=remote-fs.target

Which this dependency?


Took that quickly from pacrunner.service.
It does not look relevant.


+[Service]
+Restart=on-failure
+ExecStart=@rootlibexecdir@/systemd-proxy-discoveryd
+StandardOutput=null

Why this last line?


See my answer to Tom's comment.


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


Re: [systemd-devel] [RFC 1/6] proxy-discoveryd: Basic core added

2015-04-13 Thread Tomasz Bursztyka

Hi Tom,


+int manager_new(Manager **ret);
+Manager *manager_free(Manager *m);
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
+#define _cleanup_manager_free_ _cleanup_(manager_freep)

We generally try to avoid this define in internal code, and just use
_cleanup_(manager_freep) inline.


Ok, I followed networkd but indeed other parts don't do that (machined 
for instance).



diff --git a/units/.gitignore b/units/.gitignore
index ad469c1..63ae1af 100644
--- a/units/.gitignore
+++ b/units/.gitignore
@@ -51,6 +51,7 @@
  /systemd-networkd.service
  /systemd-nspawn@.service
  /systemd-poweroff.service
+/systemd-proxy-discoveryd.service
  /systemd-quotacheck.service
  /systemd-random-seed.service
  /systemd-reboot.service
diff --git a/units/systemd-proxy-discoveryd.service.in 
b/units/systemd-proxy-discoveryd.service.in
new file mode 100644
index 000..7ac02e0
--- /dev/null
+++ b/units/systemd-proxy-discoveryd.service.in
@@ -0,0 +1,18 @@
+# This file is part of systemd.
+#
+# systemd is free software; you can redistribute it and/or modify it
+# under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation; either version 2.1 of the License, or
+# (at your option) any later version.
+
+[Unit]
+Description=Proxy service
+DefaultDependencies=no
+Requires=dbus.socket
+After=dbus.socket
+Before=remote-fs.target
+
+[Service]
+Restart=on-failure
+ExecStart=@rootlibexecdir@/systemd-proxy-discoveryd
+StandardOutput=null

Why this special handling of stdout? Is it the JS engine? Probably
should have a comment as it is a bit odd.


I copy-pasted the .service file we had for pacrunner, this needs to be 
tailored for proxy-discoveryd.


The stdout directing to /dev/null is to avoid the JS engine to print 
anything.
Though for duktape specifically, we can hook the 
debug/printing/warning/... functions, so we could

redirect messages to the logs.

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


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

2015-04-13 Thread Lennart Poettering
On Sun, 12.04.15 22:19, Goffredo Baroncelli (kreij...@libero.it) wrote:

> However the original code catch also the case where the file is a soft-link.
> The same check is performed also by chattr(1); I suggest to leave the original
> behavior, changing
> 
> fd = open(path, O_RDONLY|O_NONBLOCK|O_CLOEXEC);
> in
>   fd = open(path, O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_NOFOLLOW);
> 
> and checking if the errno is ELOOP. In this case a further check is performed 
> to 
> verify if the file is a link or the error is due to a too many symbolic link.
> Then an appropriate message error is printed.
> 
> What do you think ?

We should probably either follow symlinks for all of tmpfiles'
operations or for none. 

While I generally believe that we probably shouldn't follow symlinks,
it's really difficult to implement given that fchmodat() currenlty
doesn't work with AT_SYMLINK_FOLLOW (according to the man page at
least), and acl_set_file doesn't allow not following symlinks either... :-(

Hmm, I can't say I like this I must say.

ideas?

Lennart

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


[systemd-devel] [PATCH v2 1/2] input_id: Make test_pointer / test_keys return if they've found anything

2015-04-13 Thread Hans de Goede
Make test_pointer / test_keys return a boolean indicating whether or not
they've set any properties on the device.

While touching allmost all test_bit() using lines anyways also remove
the extra space between the function name and the '(' (coding style issue).

Signed-off-by: Hans de Goede 
---
 src/udev/udev-builtin-input_id.c | 98 
 1 file changed, 60 insertions(+), 38 deletions(-)

diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c
index ecfc447..2c8a9ee 100644
--- a/src/udev/udev-builtin-input_id.c
+++ b/src/udev/udev-builtin-input_id.c
@@ -126,7 +126,7 @@ static void get_cap_mask(struct udev_device *dev,
 }
 
 /* pointer devices */
-static void test_pointers (struct udev_device *dev,
+static bool test_pointers (struct udev_device *dev,
const unsigned long* bitmask_ev,
const unsigned long* bitmask_abs,
const unsigned long* bitmask_key,
@@ -135,77 +135,93 @@ static void test_pointers (struct udev_device *dev,
bool test) {
 int is_mouse = 0;
 int is_touchpad = 0;
+bool ret = false;
 
-if (test_bit (INPUT_PROP_ACCELEROMETER, bitmask_props)) {
+if (test_bit(INPUT_PROP_ACCELEROMETER, bitmask_props)) {
 udev_builtin_add_property(dev, test, "ID_INPUT_ACCELEROMETER", 
"1");
-return;
+return true;
 }
 
-if (!test_bit (EV_KEY, bitmask_ev)) {
-if (test_bit (EV_ABS, bitmask_ev) &&
-test_bit (ABS_X, bitmask_abs) &&
-test_bit (ABS_Y, bitmask_abs) &&
-test_bit (ABS_Z, bitmask_abs))
+if (!test_bit(EV_KEY, bitmask_ev)) {
+if (test_bit(EV_ABS, bitmask_ev) &&
+test_bit(ABS_X, bitmask_abs) &&
+test_bit(ABS_Y, bitmask_abs) &&
+test_bit(ABS_Z, bitmask_abs)) {
 udev_builtin_add_property(dev, test, 
"ID_INPUT_ACCELEROMETER", "1");
-return;
+ret = true;
+}
+return ret;
 }
 
-if (test_bit (EV_ABS, bitmask_ev) &&
-test_bit (ABS_X, bitmask_abs) && test_bit (ABS_Y, bitmask_abs)) {
-if (test_bit (BTN_STYLUS, bitmask_key) || test_bit 
(BTN_TOOL_PEN, bitmask_key))
+if (test_bit(EV_ABS, bitmask_ev) &&
+test_bit(ABS_X, bitmask_abs) && test_bit(ABS_Y, bitmask_abs)) {
+if (test_bit(BTN_STYLUS, bitmask_key) || 
test_bit(BTN_TOOL_PEN, bitmask_key)) {
 udev_builtin_add_property(dev, test, 
"ID_INPUT_TABLET", "1");
-else if (test_bit (BTN_TOOL_FINGER, bitmask_key) && !test_bit 
(BTN_TOOL_PEN, bitmask_key))
+ret = true;
+} else if (test_bit(BTN_TOOL_FINGER, bitmask_key) && 
!test_bit(BTN_TOOL_PEN, bitmask_key)) {
 is_touchpad = 1;
-else if (test_bit (BTN_MOUSE, bitmask_key))
+} else if (test_bit(BTN_MOUSE, bitmask_key)) {
 /* This path is taken by VMware's USB mouse, which has
  * absolute axes, but no touch/pressure button. */
 is_mouse = 1;
-else if (test_bit (BTN_TOUCH, bitmask_key))
+} else if (test_bit(BTN_TOUCH, bitmask_key)) {
 udev_builtin_add_property(dev, test, 
"ID_INPUT_TOUCHSCREEN", "1");
+ret = true;
 /* joysticks don't necessarily have to have buttons; e. g.
  * rudders/pedals are joystick-like, but buttonless; they have
  * other fancy axes */
-else if (test_bit (BTN_TRIGGER, bitmask_key) ||
- test_bit (BTN_A, bitmask_key) ||
- test_bit (BTN_1, bitmask_key) ||
- test_bit (ABS_RX, bitmask_abs) ||
- test_bit (ABS_RY, bitmask_abs) ||
- test_bit (ABS_RZ, bitmask_abs) ||
- test_bit (ABS_THROTTLE, bitmask_abs) ||
- test_bit (ABS_RUDDER, bitmask_abs) ||
- test_bit (ABS_WHEEL, bitmask_abs) ||
- test_bit (ABS_GAS, bitmask_abs) ||
- test_bit (ABS_BRAKE, bitmask_abs))
+} else if (test_bit(BTN_TRIGGER, bitmask_key) ||
+   test_bit(BTN_A, bitmask_key) ||
+   test_bit(BTN_1, bitmask_key) ||
+   test_bit(ABS_RX, bitmask_abs) ||
+   test_bit(ABS_RY, bitmask_abs) ||
+   test_bit(ABS_RZ, bitmask_abs) ||
+   test_bit(ABS_THROTTLE, bitmask_abs) ||
+   test_bit(ABS_RUDDER, bi

[systemd-devel] [PATCH v2 2/2] input_id: Identify scroll-wheel device on Trust TB7300 tablet as keyboard

2015-04-13 Thread Hans de Goede
The Trust TB7300 (relabelled Waltop?) tablet has a scrollwheel which shows
up as a /dev/input/event# node all by itself. Currently input_id does not
set any ID_INPUT_FOO attr on this causing it it to not be recognized by
Xorg / libinput.

This commit fixes this by marking it with ID_INPUT_KEY.

Cc: Sjoerd Timmer 
Reported-by: Sjoerd Timmer 
Signed-off-by: Hans de Goede 
---
Changes in v2:
-Use test_pointers() / test_key return value()
---
 src/udev/udev-builtin-input_id.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/udev/udev-builtin-input_id.c b/src/udev/udev-builtin-input_id.c
index 2c8a9ee..ee7aaa8 100644
--- a/src/udev/udev-builtin-input_id.c
+++ b/src/udev/udev-builtin-input_id.c
@@ -265,6 +265,8 @@ static int builtin_input_id(struct udev_device *dev, int 
argc, char *argv[], boo
 unsigned long bitmask_rel[NBITS(REL_MAX)];
 unsigned long bitmask_props[NBITS(INPUT_PROP_MAX)];
 const char *sysname, *devnode;
+bool is_pointer;
+bool is_key;
 
 /* walk up the parental chain until we find the real input device; the
  * argument is very likely a subdevice of this, like eventN */
@@ -281,9 +283,14 @@ static int builtin_input_id(struct udev_device *dev, int 
argc, char *argv[], boo
 get_cap_mask(dev, pdev, "capabilities/rel", bitmask_rel, 
sizeof(bitmask_rel), test);
 get_cap_mask(dev, pdev, "capabilities/key", bitmask_key, 
sizeof(bitmask_key), test);
 get_cap_mask(dev, pdev, "properties", bitmask_props, 
sizeof(bitmask_props), test);
-test_pointers(dev, bitmask_ev, bitmask_abs, bitmask_key,
-  bitmask_rel, bitmask_props, test);
-test_key(dev, bitmask_ev, bitmask_key, test);
+is_pointer = test_pointers(dev, bitmask_ev, bitmask_abs,
+   bitmask_key, bitmask_rel,
+   bitmask_props, test);
+is_key = test_key(dev, bitmask_ev, bitmask_key, test);
+/* Some evdev nodes have only a scrollwheel */
+if (!is_pointer && !is_key && test_bit(EV_REL, bitmask_ev) &&
+(test_bit(REL_WHEEL, bitmask_rel) || test_bit(REL_HWHEEL, 
bitmask_rel)))
+udev_builtin_add_property(dev, test, "ID_INPUT_KEY", 
"1");
 }
 
 devnode = udev_device_get_devnode(dev);
-- 
2.3.4

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


Re: [systemd-devel] Check if systems is container in "systemd-remount-fs.service"

2015-04-13 Thread Lennart Poettering
On Mon, 13.04.15 02:31, Ivan Shapovalov (intelfx...@gmail.com) wrote:

> On 2015-04-09 at 10:04 +0200, Lennart Poettering wrote:
> > [...]
> > Also, current versions of fstab-generator skip device entries in
> > containers anyway, so I am not sure how you even managed to generate
> > an error in this case, unless you run a really old version of 
> > systemd.
> 
> Hmm, well, systemd-remount-fs.service is enabled statically, it 
> doesn't use systemd-fstab-generator...

True, but still: I think just skipping the whole service is wrong. It
might list mount options to apply to some file system (including API
VFS), and we should apply them.

This really sounds more as if you distro should not place unnecessary
entries in /etc/fstab, that cause failure like this...

Lennart

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


Re: [systemd-devel] [PATCH 5/6] network: fix strict aliasing issue

2015-04-13 Thread Lennart Poettering
On Sun, 12.04.15 18:32, Shawn Landden (sh...@churchofgit.com) wrote:

> On Sun, Apr 12, 2015 at 12:43 PM, Lennart Poettering
>  wrote:
> > On Wed, 11.03.15 08:13, Shawn Landden (sh...@churchofgit.com) wrote:
> >
> >> We shouldn't assume 64-bit arch with the way we do math either.
> >> (although I will submit a patch to glibc to add a uint64_t union
> >> alias)
> >
> > Hmm? uint64_t works fine on 32bit too. The compiler can do the
> > necessary emulation on its own... I don't see the need to change
> > anything here.
>
> Its also an unaligned access. in_addr.in6 is only 32-bit aligned. (in
> addition to being a strict aliasing violation.)

You are right. It is a possibly unaligned access.

Would be happy to take a patch that converts this to memcmp().

Lennart

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


Re: [systemd-devel] [PATCH v2] efi-boot-generator: Continue if /boot does not exist

2015-04-13 Thread Tobias Hunger
Hi Zbigniew,

thanks for merging my rumblings. I was thinking about merging those
lines, but at this point I really do not feel comfortable enough to
change code just to make it look better to me:-)

Maybe a few patches down the line I'll get to that point.

Now I just need to wait for a systemd 220 that actually will make it
into arch Linux and I can drop my hand-rolled packages:-)

Best Regards,
Tobias


On Mon, Apr 13, 2015 at 3:27 AM, Zbigniew Jędrzejewski-Szmek
 wrote:
> On Sun, Apr 12, 2015 at 06:49:41PM +0200, Lennart Poettering wrote:
>> On Sat, 11.04.15 02:13, Tobias Hunger (tobias.hun...@gmail.com) wrote:
>>
>> > /boot does not exist on a stateless system, so do not get
>> > confused by that.
>> > ---
>> >  src/efi-boot-generator/efi-boot-generator.c | 5 -
>> >  1 file changed, 4 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/src/efi-boot-generator/efi-boot-generator.c 
>> > b/src/efi-boot-generator/efi-boot-generator.c
>> > index 58c4cc2..5a9bfd3 100644
>> > --- a/src/efi-boot-generator/efi-boot-generator.c
>> > +++ b/src/efi-boot-generator/efi-boot-generator.c
>> > @@ -68,7 +68,10 @@ int main(int argc, char *argv[]) {
>> >  return EXIT_SUCCESS;
>> >  }
>> >
>> > -if (path_is_mount_point("/boot", true) <= 0 &&
>> > +r = path_is_mount_point("/boot", true);
>> > +if (r == -ENOENT) {
>> > +log_debug("/boot does not exist, continuing.");
>> > +} else if (r <= 0 &&
>> >  dir_is_empty("/boot") <= 0) {
>> >  log_debug("/boot already populated, exiting.");
>> >  return EXIT_SUCCESS;
>>
>> This was merged by Zbigniew earlier today.
> Yeah, sorry I forgot to write to the mailing list.
>
> I reformatted the patch a bit, I think it is more readable without
> splitting the condition in two lines.
>
> Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] 'is-enabled' supported for legacy sysvinit scripts?

2015-04-13 Thread Nusenu
> thanks for your reply.
> 
>> systemctl should still support it if your system supports
>> chkconfig to manage initscripts.
> 
> chkconfig was indeed not installed, but even after installing it,
> the is-enabled command output and return codes do not change:
> 
>>> Output in such cases:
>> 
>>> "Failed to get unit file state for FOO.service: No such file or
>>> director y"
> 
> Is this considered a bug?

since it works on RHEL 7 with systemd 208 I'll file this bug against
debian.

systemctl is-enabled tor
tor.service is not a native service, redirecting to /sbin/chkconfig.
Executing /sbin/chkconfig tor --level=5
enabled


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


Re: [systemd-devel] 'is-enabled' supported for legacy sysvinit scripts?

2015-04-13 Thread Nusenu
Hi Andrei,

thanks for your reply.

> systemctl should still support it if your system supports chkconfig
> to manage initscripts.

chkconfig was indeed not installed, but even after installing it, the
is-enabled command output and return codes do not change:

>> Output in such cases:
> 
>> "Failed to get unit file state for FOO.service: No such file or director
>> y"

Is this considered a bug?

thanks,
Nusenu

testing on:
Debian 8 (chkconfig installed) with systemd 215-14 and
Ubuntu 15.04 with systemd 219-7ubuntu1 (no chkconfig available)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel