Re: [systemd-devel] Compatibility between D-Bus and kdbus

[Thiago Macieira]

On Wednesday 26 November 2014 00:46:50 David Herrmann wrote:
> We had "systemd-bus-driverd", which implemented org.freedesktop.DBus
> as normal service. However, this didn't work out as many dbus clients
> rely on this services to not be re-ordered in regard to external
> requests.
> 
> In particular, if gdbus runs AddMatch(), it assumes the match takes
> effect immediately. If it sends a method call to another service after
> installing the match, and this triggers a signal, gdbus assumes the
> AddMatch() call to have succeeded (without waiting for the reply).
> However, if org.freedesktop.DBus is not implemented by the bus, but by
> an external service, you cannot guarantee that messages targetted at
> different receivers don't get re-ordered, and there're no guarantees
> which process gets scheduled first.
> 
> This is a real bug and we couldn't figure out a way to fix it. Current
> DBus applications depend on org.freedesktop.DBus to be handled by the
> bus entity _in-order_. Therefore, we dropped systemd-bus-driverd and
> all the kdbus ioctls that we added to support this.

Aside from the connection-control mechanisms (AddMatch, RemoveMatch), did you 
see any problems?

User code usually shouldn't be doing AddMatch and RemoveMatch (the only case I 
can think of is dconf, which does its own parsing of the message payload). 
That's the domain of the binding, which will know that it connected to kdbus 
and won't be making those calls.

As I said in the reply to Lennart, I'm specifically looking for 
StartServiceByName. Everything else is emulatable anyway or nonsense in kdbus.

> > By the way, is there a way to ensure that a given connection is the first
> > connection? As soon as the bus creator is able to connect to the
> > /sys/fs/kdbus path, so is another process and therefore this other
> > process could maliciously acquire names it shouldn't.
> 
> Acquiring names requires matching policies. If you setup your policies
> in a way that two applications can acquire the same name, you're doing
> something wrong. Or maybe I don't understand your use-case?

No, it was my misunderstanding. The endpoint creator should set up a policy so 
that it itself can acquire the name(s) it needs, then deny any other 
connections owning names or TALKing to each other. This effectively creates a 
many-to-one bus system. E.g., a fake P2P.

> > Ok, so any application that connected to the "bus" bus can then create
> > custom endpoints. Correct?
> 
> No, custom endpoints can only be created by privileged users. On the
> system-bus, this means root, on the user-bus this means processes of
> the user itself.

Sorry, I meant application connected to the a user's "bus" bus. Like you said, 
that means any application with the same UID, which means all applications 
that were allowed to connect to "bus" in the first place.

> kdbus implements bus-based IPC. If you want P2P IPC, use one of the
> established transports.
> 
> Yes, kdbus has some handy features people like to see on unix-sockets
> (like flexible metadata transports), but our current policy is "fix
> unix sockets!", and "this can optionally be implemented later on".
> There is no plan to support P2P connections in the initial kdbus
> draft.

I'm not asking for much more for P2P connections in kdbus. I think that it's 
already doable by restricting an endpoint to a many-to-one system.

> Custom endpoints do _not_ create new buses. Really. You could create a
> custom bus and use it for just 2 connections, but then you could also
> just use socketpair(2). Note that there was some discussion on
> "anonymous buses", which would allow to create such buses on the fly.
> But again, this will not be part of the initial kdbus draft. If anyone
> cares, submit it as patches once kdbus is upstream.

As far as I can tell, an "anonymous bus" in the sense of a bus with a 
randomised name is doable by having the proper interface in the systemd1 
system bus service. An application would request the bus and receive as a 
reply the address of the bus, the file descriptor for the control, and a file 
descriptor to a privileged connection so it can control the policies.

This does not require kernel-side modifications, only in systemd.

> > Understood, but coming from userspace's perspective this seems ill-advised
> > and optimising for the wrong use-case. The common case is that syscalls
> > are not interrupted: with a timeout, there's one clock_gettime() call
> > *after* the interruption if any; with the timestamp, there's always one
> > before the syscall.
> 
> Use clock_gettime(CLOCK_MONOTONIC_COARSE). This reads the time from
> the VDSO, which is a simple memory read. No trap/syscall is involved.
> Downside is that you loose precision, but that should really be fine
> for this use-case.

I need to read a little more on the coarse monotonic clock to see what cases 
it's applicable for and when you should avoid it.

In this specific case, precision is not required, as a 25-second timeou

Re: [systemd-devel] Compatibility between D-Bus and kdbus

[Thiago Macieira]

On Wednesday 26 November 2014 01:25:18 Lennart Poettering wrote:
> > Thinking of non-system buses here.
> > 
> > If the variable is empty, I agree that it should have an equivalent of an
> > "autostart" mechanism, but I disagree on the solution and I also disagree
> > that distros should leave it empty.
> 
> Oh, no. No autostart please. No such concept exists in kdbus, and
> systemd/sd-bus will not support that either. In fact I refuse to support
> that even on dbus1 in sd-bus. Autostart is a kludge for systems where dbus
> is just an add-on, but that's completely out-of-focus for kdbus,
> systemd and sd-bus.

I didn't actually mean automatically starting the bus, sorry for the 
confusion. I meant automatic discovery only. Currently on dbus1, "autostart:" 
as a transport protocol means both auto discovery and automatic starting if 
the bus isn't running.

Please also note that the autostart solution has a valid use-case which is 
when a D-Bus application is launched in an environment where no bus had been 
started before. I understand this is out-of-scope for kdbus, since after all a 
regular user won't be able to create a kdbus bus if one wasn't provided by a 
privileged process before. In an environment where a kdbus bus wasn't 
provided, the only alternative is to fallback to dbus1.

> Note that even on systemd we will set $DBUS_SESSION_BUS_ADDRESS,
> simply because classic libdbus and gdbus won't work without
> it. However, we will actually set it to a fixed value.

That's all I asked for. Whether the value is constant or not is not relevant, 
as long as it gets set.

> > For one thing, the fallback address is expected to be there if there's a
> > proxy bus running. The current autostart mechanism relies on X being
> > present, so the fallback won't be found unless X is running and something
> > registered the proxy's socket address there.
> > 
> > For another, it's good practice to have it set and not depend on
> > autostart.
> > 
> > For a third, hardcoding kernel paths in userspace sounds like a poor idea.
> > The kdbus mountpoint may be elsewhere and whatever is creating buses may
> > not do it per user, but per session or other creation rule it may have.
> 
> No, we don't support weird setups where kdbusfs mounted
> elsewhere. This is a bew API we introduce here, and we can very much
> make decisions where stuff is to be mounted.

You may not support it in systemd, but from reading the kernel API that could 
happen with another implementation.

> Env vars are a hack, due to the awful inheritence logic, and we should
> really avoid using them, except where necessary for compat, and that's
> precisely to which level we'll support them in systemd.

Env vars actually match pretty well the concept of session, except when nested 
sessions happen without resetting the necessary environment variables (e.g., a 
screen(1), tmux(1) or Xnest sub-session).

I know you're designing systemd so that it won't provide session buses, which 
is why it feels like a kludge to you. I won't argue here. I'm satisfied that 
the env var gets set.

> > > > would be interesting to have:
> > > No, this is not supported in the current versions of kdbus
> > > anymore. Emulation of these calls must happen client side if it shall
> > > be supported.
> > 
> > That wouldn't be kdbus, but systemd doing it. Since systemd is the one
> > that
> > opens the bus, it can register the first connection and claim the
> > org.freedesktop.DBus service name, providing compatibility. So this isn't
> > a
> > feature request for kdbus but a feature request for systemd.
> 
> We initially tried to support that, but it's awfully racy, since the
> driver calls and calls to other services wouldn't be executed in
> strict order anymore... We removed this again after figuring out and
> decided that emulation can only happen client side, synchronous to the
> message stream if we want to guarantee correct ordering.

I'm not asking for AddMatch and connection control mechanisms. The one I 
really want is StartServiceByName, since it can't be emulated. Moreover, 
starting services is systemd's raison-d'être, so I feel it should be no 
problem for you to provide such a service.

It would be nice if UpdateActivationEnvironment worked. This functionality was 
added for people who need to update variables like XDG_DATA_DIRS after 
starting the bus. If this one isn't present, we can report "not implemented" 
and be fine with it. We'll just have to tell people to configure their systemd 
user session environments properly.

The same goes for ReloadConfig, but I'd prefer to know whether that failed (no 
config reloading is possible) or whether it happens automatically whether the 
call was made or not. ReloadConfig is important when there are new activatable 
services on a user's bus, such as newly-installed applications.

> > > The client side emulation can choose to either forward ReloadConifg
> > > and UpdateActivationEnvironment to the respect systemd calls, or 

Re: [systemd-devel] [PATCH] hwdb: add a new db for the DPI/frequency settings of mice

[Zbigniew Jędrzejewski-Szmek]

On Wed, Nov 26, 2014 at 08:36:55AM +1000, Peter Hutterer wrote:
> Plus, would a fix be welcome to ignore multiple leading whitespaces? This
> has caught me several times now and I keep making this mistake.
Yes, pleeeassse. This will fix 
https://bugs.freedesktop.org/show_bug.cgi?id=82311
too.

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

[systemd-devel] Scope units get SIGKILL when stopped, not SIGTERM

[Michael Chapman]

Hello,

When I stop a scope unit, it looks like all processes in it get a SIGKILL 
immediately, not a SIGTERM.


I believe this issue has been brought up before in 
http://lists.freedesktop.org/archives/systemd-devel/2014-October/024452.html, 
but there was no resolution then. That thread indicates that commit 
743970d2 was where the regression was introduced.


I noticed this problem specifically when using "reboot" from a shell (it 
happened to be over SSH, but I don't think that's significant). After 
reboot, the user's shell history did *not* contain any of the commands 
from that shell session, since it had been SIGKILLed.


Is there any solution for this problem on the horizon? Losing shell 
history is relatively minor in the grand scheme of things, but I could 
well imagine some other uses for scope units that expect an orderly 
shutdown.


- Michael

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

Re: [systemd-devel] Compatibility between D-Bus and kdbus

[Lennart Poettering]

On Wed, 26.11.14 00:46, David Herrmann (dh.herrm...@gmail.com) wrote:

> Custom endpoints do _not_ create new buses. Really. You could create a
> custom bus and use it for just 2 connections, but then you could also
> just use socketpair(2). Note that there was some discussion on
> "anonymous buses", which would allow to create such buses on the fly.
> But again, this will not be part of the initial kdbus draft. If anyone
> cares, submit it as patches once kdbus is upstream.

I gave up on the anonymous bus idea btw. With the current
kdbusfs-based approach creating a bus is so cheap that the benefit of
being anonymous is realy gone. We can implement a scheme like this
fully in userspace now, by simply using a randomized bus name... 

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] Compatibility between D-Bus and kdbus

[Lennart Poettering]

On Tue, 25.11.14 12:01, Thiago Macieira (thi...@kde.org) wrote:

> > Well, we don't need any env var really, as we enforce that the UID of
> > the user is included in the name of their bussess, and the busses are
> > cleaned up when the registrar dies. We don't have the risk of leaving
> > old busses around, or even by other users, hence all code can just
> > imply the path to use is kernel:path=/sys/fs/kdbus/0-system and
> > kernel:path=/sys/fs/kdbus/$UID-user and all is good, without ever
> > having to deal with env vars at all.
> > 
> > (of course, if env-vars are set they should be used, but the normal
> > codepaths in the distros should work without them.)
> 
> Thinking of non-system buses here.
> 
> If the variable is empty, I agree that it should have an equivalent of an 
> "autostart" mechanism, but I disagree on the solution and I also disagree 
> that 
> distros should leave it empty.

Oh, no. No autostart please. No such concept exists in kdbus, and
systemd/sd-bus will not support that either. In fact I refuse to support that
even on dbus1 in sd-bus. Autostart is a kludge for systems where dbus
is just an add-on, but that's completely out-of-focus for kdbus,
systemd and sd-bus.

Note that even on systemd we will set $DBUS_SESSION_BUS_ADDRESS,
simply because classic libdbus and gdbus won't work without
it. However, we will actually set it to a fixed value.

> For one thing, the fallback address is expected to be there if there's a 
> proxy 
> bus running. The current autostart mechanism relies on X being present, so 
> the 
> fallback won't be found unless X is running and something registered the 
> proxy's socket address there.
> 
> For another, it's good practice to have it set and not depend on autostart.
> 
> For a third, hardcoding kernel paths in userspace sounds like a poor idea. 
> The 
> kdbus mountpoint may be elsewhere and whatever is creating buses may not do 
> it 
> per user, but per session or other creation rule it may have.

No, we don't support weird setups where kdbusfs mounted
elsewhere. This is a bew API we introduce here, and we can very much
make decisions where stuff is to be mounted.

Env vars are a hack, due to the awful inheritence logic, and we should
really avoid using them, except where necessary for compat, and that's
precisely to which level we'll support them in systemd.

> > > would be interesting to have:
> > No, this is not supported in the current versions of kdbus
> > anymore. Emulation of these calls must happen client side if it shall
> > be supported.
> 
> That wouldn't be kdbus, but systemd doing it. Since systemd is the one that 
> opens the bus, it can register the first connection and claim the 
> org.freedesktop.DBus service name, providing compatibility. So this isn't a 
> feature request for kdbus but a feature request for systemd.

We initially tried to support that, but it's awfully racy, since the
driver calls and calls to other services wouldn't be executed in
strict order anymore... We removed this again after figuring out and
decided that emulation can only happen client side, synchronous to the
message stream if we want to guarantee correct ordering. 

> By the way, is there a way to ensure that a given connection is the first 
> connection? As soon as the bus creator is able to connect to the 
> /sys/fs/kdbus 
> path, so is another process and therefore this other process could 
> maliciously 
> acquire names it shouldn't.

When creating the bus the creator can pass policy to the kernel so
that there is no time window where the bus is accessible and open to
manipulation from untrusted clients.

> > > org.freedesktop.DBus.ReloadConfig
> > > org.freedesktop.DBus.StartServiceByName
> > > org.freedesktop.DBus.UpdateActivationEnvironment
> > > 
> > > Most of those would be just convenience for other, existing kdbus
> > > low-level
> > > calls, but ReloadConfig and UpdateActivationEnvironment are not available
> > > anywhere else. It's true that there's nothing stopping more CAP_IPC_OWNER
> > > connections from installing more activators, but the question is whether
> > > systemd will provide those for the activations it holds.
> > 
> > The client side emulation can choose to either forward ReloadConifg
> > and UpdateActivationEnvironment to the respect systemd calls, or just
> > return som "not supported" error.
> 
> Can't do that. What if it's a kdbus system that is not systemd?

Well, again, return "not supported" then. I mean, currently there is
no kdbus userspace implementation beyond kdbus, we cannot really
discuss something that doesn't exist...

> I don't mind forwarding to a well-known bus name, as long as we establish 
> that 
> there is such a service running on the bus that will accept those calls. But 
> if such a service exists, why can't it claim the
> org.freedesktop.DBus name?

Note that on dbus1 systemd systems we actually never provided
UpdateActivationEnvironment correctly (since services got forked off
PID 1, instead of dbus-

Re: [systemd-devel] [PATCH] hwdb: add a new db for the DPI/frequency settings of mice

[Zbigniew Jędrzejewski-Szmek]

On Wed, Nov 26, 2014 at 01:01:30AM +0100, Zbigniew Jędrzejewski-Szmek wrote:
> On Wed, Nov 26, 2014 at 07:30:51AM +1000, Peter Hutterer wrote:
> > On Tue, Nov 25, 2014 at 02:56:31PM +0100, Zbigniew Jędrzejewski-Szmek wrote:
> > > On Tue, Nov 25, 2014 at 09:35:16PM +1000, Peter Hutterer wrote:
> > > > Pointer acceleration for relative input devices (mice, trackballs, etc.)
> > > > applies to the deltas of the device. Alas, those deltas have no physical
> > > > reference point - a delta of 10 may be caused by a large movement of a 
> > > > low-dpi
> > > > mouse or by a minute movement of a high-dpi mouse.
> > > > Which makes pointer acceleration a bit useless and high-dpi devices
> > > > essentially unusable.
> > > > 
> > > > In an ideal world, we could read the DPI from the device directly and 
> > > > work
> > > > with that. In the world we actually live in, we need to compile this 
> > > > list
> > > > manually. This patch introduces the database, with the usual match 
> > > > formats and
> > > > a single property to be set on a device: MOUSE_DPI
> > > > 
> > > > That is either a single value for most mice, or a list of values for 
> > > > mice that
> > > > can change resolution at runtime. The exact format is detailed in the 
> > > > hwdb
> > > > file.
> > > > 
> > > > Note that we're explicitly overshooting the requirements we have for 
> > > > libinput
> > > > atm. Frequency could be detected in software and we don't actually use 
> > > > the
> > > > list of multiple resolutions (because we can't detect when they change
> > > > anyway). However, we might as well collect those values from the get-go,
> > > > adding/modifying what will eventually amount to hundreds of entries is a
> > > > bit cumbersome.
> > > > 
> > > > Note: we rely on the input_id builtin to tag us as mouse first, 
> > > > ordering of
> > > > the rules is important.
> > > > ---
> > > >  Makefile.am  |  4 ++-
> > > >  rules/70-mouse.hwdb  | 92 
> > > > 
> > > >  rules/70-mouse.rules | 15 +
> > > >  3 files changed, 110 insertions(+), 1 deletion(-)
> > > >  create mode 100644 rules/70-mouse.hwdb
> > > >  create mode 100644 rules/70-mouse.rules
> > > > 
> > > > diff --git a/Makefile.am b/Makefile.am
> > > > index 3f9f3fa..d2b0d02 100644
> > > > --- a/Makefile.am
> > > > +++ b/Makefile.am
> > > > @@ -3285,6 +3285,7 @@ dist_udevrules_DATA += \
> > > > rules/50-udev-default.rules \
> > > > rules/60-drm.rules \
> > > > rules/60-keyboard.rules \
> > > > +   rules/70-mouse.rules \
> > > > rules/60-persistent-storage-tape.rules \
> > > > rules/60-persistent-serial.rules \
> > > > rules/60-persistent-input.rules \
> > > > @@ -3311,7 +3312,8 @@ dist_udevhwdb_DATA = \
> > > > hwdb/20-acpi-vendor.hwdb \
> > > > hwdb/20-OUI.hwdb \
> > > > hwdb/20-net-ifname.hwdb \
> > > > -   hwdb/60-keyboard.hwdb
> > > > +   hwdb/60-keyboard.hwdb \
> > > > +   hwdb/70-mouse.hwdb
> > > Wouldn't 60-mouse be more appropriate? It is very similar to 60-keyboard.
> > 
> > I had that at first but 60-mouse sorts before 60-persistent-input.rules and
> > we partially rely on that, specifically the line
> > SUBSYSTEMS=="usb", ENV{ID_BUS}=="", IMPORT{builtin}="usb_id"
> > 
> > Want me to add that line to this ruleset here, or should we leave the
> > sorting as-is?
> I think you should *move* it to 60-mouse.rules.
> It seems better for something specific like persistent-input to depend
> on something generic like mouse.
> 
> 60-persistent-input.rules has SUBSYSTEMS=="bluetooth", 
> GOTO="persistent_input_end".
> That'd skip bluetooth which you need.
Scratch that. I don't think you need to move anything, because you
don't rely on anything from 60-persistent-input.rules. ID_INPUT_MOUSE
is provided by IMPORT{builtin}="input_id" which is run in
50-udev-default.rules.

Zbyszek

> > > >  udevconfdir = $(sysconfdir)/udev
> > > >  dist_udevconf_DATA = \
> > > > diff --git a/rules/70-mouse.hwdb b/rules/70-mouse.hwdb
> > > > new file mode 100644
> > > > index 000..75e8755
> > > > --- /dev/null
> > > > +++ b/rules/70-mouse.hwdb
> > > > @@ -0,0 +1,92 @@
> > > > +# This file is part of systemd.
> > > > +#
> > > > +# Database for the DPI setting of mice, trackballs, other pointer 
> > > > devices that
> > > > +# cannot be queried directly.
> > > > +#
> > > > +# The lookup keys are composed in:
> > > > +#   60-mouse.rules
> > > Typo (if you elect not to use change to 60 ;))
> > 
> > damn those last-minute changes :)
> > 
> > Cheers,
> >Peter
> > 
> > > Otherwise looks OK.
> > > 
> > > Zbyszek
> > > 
> > > > +# Note: The format of the "mouse:" prefix match key is a
> > > > +# contract between the rules file and the hardware data, it might
> > > > +# change in later revisions to support more or better matches, it
> > > > +# is not necessarily expected to be a stable ABI.
> > > > +#
> > > > +# Match string format:
> > > > +# mouse::vp

Re: [systemd-devel] [PATCH] hwdb: add a new db for the DPI/frequency settings of mice

[Zbigniew Jędrzejewski-Szmek]

On Wed, Nov 26, 2014 at 07:30:51AM +1000, Peter Hutterer wrote:
> On Tue, Nov 25, 2014 at 02:56:31PM +0100, Zbigniew Jędrzejewski-Szmek wrote:
> > On Tue, Nov 25, 2014 at 09:35:16PM +1000, Peter Hutterer wrote:
> > > Pointer acceleration for relative input devices (mice, trackballs, etc.)
> > > applies to the deltas of the device. Alas, those deltas have no physical
> > > reference point - a delta of 10 may be caused by a large movement of a 
> > > low-dpi
> > > mouse or by a minute movement of a high-dpi mouse.
> > > Which makes pointer acceleration a bit useless and high-dpi devices
> > > essentially unusable.
> > > 
> > > In an ideal world, we could read the DPI from the device directly and work
> > > with that. In the world we actually live in, we need to compile this list
> > > manually. This patch introduces the database, with the usual match 
> > > formats and
> > > a single property to be set on a device: MOUSE_DPI
> > > 
> > > That is either a single value for most mice, or a list of values for mice 
> > > that
> > > can change resolution at runtime. The exact format is detailed in the hwdb
> > > file.
> > > 
> > > Note that we're explicitly overshooting the requirements we have for 
> > > libinput
> > > atm. Frequency could be detected in software and we don't actually use the
> > > list of multiple resolutions (because we can't detect when they change
> > > anyway). However, we might as well collect those values from the get-go,
> > > adding/modifying what will eventually amount to hundreds of entries is a
> > > bit cumbersome.
> > > 
> > > Note: we rely on the input_id builtin to tag us as mouse first, ordering 
> > > of
> > > the rules is important.
> > > ---
> > >  Makefile.am  |  4 ++-
> > >  rules/70-mouse.hwdb  | 92 
> > > 
> > >  rules/70-mouse.rules | 15 +
> > >  3 files changed, 110 insertions(+), 1 deletion(-)
> > >  create mode 100644 rules/70-mouse.hwdb
> > >  create mode 100644 rules/70-mouse.rules
> > > 
> > > diff --git a/Makefile.am b/Makefile.am
> > > index 3f9f3fa..d2b0d02 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -3285,6 +3285,7 @@ dist_udevrules_DATA += \
> > >   rules/50-udev-default.rules \
> > >   rules/60-drm.rules \
> > >   rules/60-keyboard.rules \
> > > + rules/70-mouse.rules \
> > >   rules/60-persistent-storage-tape.rules \
> > >   rules/60-persistent-serial.rules \
> > >   rules/60-persistent-input.rules \
> > > @@ -3311,7 +3312,8 @@ dist_udevhwdb_DATA = \
> > >   hwdb/20-acpi-vendor.hwdb \
> > >   hwdb/20-OUI.hwdb \
> > >   hwdb/20-net-ifname.hwdb \
> > > - hwdb/60-keyboard.hwdb
> > > + hwdb/60-keyboard.hwdb \
> > > + hwdb/70-mouse.hwdb
> > Wouldn't 60-mouse be more appropriate? It is very similar to 60-keyboard.
> 
> I had that at first but 60-mouse sorts before 60-persistent-input.rules and
> we partially rely on that, specifically the line
> SUBSYSTEMS=="usb", ENV{ID_BUS}=="", IMPORT{builtin}="usb_id"
> 
> Want me to add that line to this ruleset here, or should we leave the
> sorting as-is?
I think you should *move* it to 60-mouse.rules.
It seems better for something specific like persistent-input to depend
on something generic like mouse.

60-persistent-input.rules has SUBSYSTEMS=="bluetooth", 
GOTO="persistent_input_end".
That'd skip bluetooth which you need.

> > >  udevconfdir = $(sysconfdir)/udev
> > >  dist_udevconf_DATA = \
> > > diff --git a/rules/70-mouse.hwdb b/rules/70-mouse.hwdb
> > > new file mode 100644
> > > index 000..75e8755
> > > --- /dev/null
> > > +++ b/rules/70-mouse.hwdb
> > > @@ -0,0 +1,92 @@
> > > +# This file is part of systemd.
> > > +#
> > > +# Database for the DPI setting of mice, trackballs, other pointer 
> > > devices that
> > > +# cannot be queried directly.
> > > +#
> > > +# The lookup keys are composed in:
> > > +#   60-mouse.rules
> > Typo (if you elect not to use change to 60 ;))
> 
> damn those last-minute changes :)
> 
> Cheers,
>Peter
> 
> > Otherwise looks OK.
> > 
> > Zbyszek
> > 
> > > +# Note: The format of the "mouse:" prefix match key is a
> > > +# contract between the rules file and the hardware data, it might
> > > +# change in later revisions to support more or better matches, it
> > > +# is not necessarily expected to be a stable ABI.
> > > +#
> > > +# Match string format:
> > > +# mouse::vp:name::
> > > +#
> > > +# Supported subsystems: usb, bluetooth
> > > +# vid/pid as 4-digit hex lowercase vendor/product
> > > +#
> > > +# if vid/pid is unavailable, use
> > > +# mouse:*:name::
> > > +# if name is unavailable, use
> > > +# mouse::vp:*
> > > +#
> > > +# For example, the following 5 matches all match the same mouse:
> > > +# mouse:usb:v17efp6019:name:Lenovo Optical USB Mouse:
> > > +# mouse:usb:*:name:Lenovo Optical USB Mouse:
> > > +# mouse:usb:v17efp6019:*
> > > +# mouse:*:name:Lenovo Optical USB Mouse:
> > > +#
> > > +# DPI settings are specified as
> > > +#MOUSE_DPI=[@]
> > > +#
> > > +# Where  is the resolut

Re: [systemd-devel] Compatibility between D-Bus and kdbus

[David Herrmann]

Hi Thiago

On Tue, Nov 25, 2014 at 9:01 PM, Thiago Macieira  wrote:
> On Tuesday 25 November 2014 17:11:36 Lennart Poettering wrote:
>> > == org.freedesktop.DBus connection ==
>> >
>> > Will systemd-kdbus provide that name on the bus so applications that make
>> > calls directly be able to continue working? I imagine the following
>> > methods
>>
>> > would be interesting to have:
>> No, this is not supported in the current versions of kdbus
>> anymore. Emulation of these calls must happen client side if it shall
>> be supported.
>
> That wouldn't be kdbus, but systemd doing it. Since systemd is the one that
> opens the bus, it can register the first connection and claim the
> org.freedesktop.DBus service name, providing compatibility. So this isn't a
> feature request for kdbus but a feature request for systemd.

We had "systemd-bus-driverd", which implemented org.freedesktop.DBus
as normal service. However, this didn't work out as many dbus clients
rely on this services to not be re-ordered in regard to external
requests.

In particular, if gdbus runs AddMatch(), it assumes the match takes
effect immediately. If it sends a method call to another service after
installing the match, and this triggers a signal, gdbus assumes the
AddMatch() call to have succeeded (without waiting for the reply).
However, if org.freedesktop.DBus is not implemented by the bus, but by
an external service, you cannot guarantee that messages targetted at
different receivers don't get re-ordered, and there're no guarantees
which process gets scheduled first.

This is a real bug and we couldn't figure out a way to fix it. Current
DBus applications depend on org.freedesktop.DBus to be handled by the
bus entity _in-order_. Therefore, we dropped systemd-bus-driverd and
all the kdbus ioctls that we added to support this.

I strongly recommend to either drop support for org.freedesktop.DBus
on any kdbus-aware DBus APIs, or fake it in the library. sd-bus
doesn't support it, and IIRC Ryan didn't want to fake it in gdbus
either. Applications are required to use explicit
add_match/remove_match library calls, instead of sending messages to
org.freedesktop.DBus.
Note that for legacy applications, we emulate org.freedesktop.DBus in
our proxy. So this is really just about applications that want to use
kdbus directly.

> By the way, is there a way to ensure that a given connection is the first
> connection? As soon as the bus creator is able to connect to the /sys/fs/kdbus
> path, so is another process and therefore this other process could maliciously
> acquire names it shouldn't.

Acquiring names requires matching policies. If you setup your policies
in a way that two applications can acquire the same name, you're doing
something wrong. Or maybe I don't understand your use-case?

>> > == Kernel API ==
>> > === Custom endpoints ===
>> >
>> > The docs say "To create a custom endpoint, use the KDBUS_CMD_ENDPOINT_MAKE
>> > ioctl". On what file descriptor? The one for the control file? Or can it
>> > be sent on any kdbus endpoint? I'm asking because I'm not sure what the
>> > permissions of the control file will be -- will any process be allowed to
>> > open it and create endpoints?
>>
>> if you want to create a new endpoint for an existing bus, then invoke
>> that ioctl on the bus fd. The control file after all is unrelated to
>> any bus, and thus wouldn#t know which bus you mean if we'd allow
>> invoking that ioctl on it.
>
> Ok, so any application that connected to the "bus" bus can then create custom
> endpoints. Correct?

No, custom endpoints can only be created by privileged users. On the
system-bus, this means root, on the user-bus this means processes of
the user itself.

> How does one get to install policies or activators on this custom bus if the
> opening connection is a regular, non-privileged process?

Policy holders and activators are privileged operations, like creating
custom endpoints. You need to open an endpoint and pass POLICY_HOLDER
or ACTIVATOR in KDBUS_CMD_HELLO to become a policy-holder or
activator. You will not be an ordinary connection, so you will not be
announced on the bus, nor can you send messages.

>> > But if that's the case, how would one implement a peer-to-peer connection?
>> > Or should it simply be a convention that P2P connections are really
>> > regular buses, except that no one owns any names, there are no policy
>> > restrictions and that the only two connections are :1.1 and :1.2?
>>
>> kdbus is not for peer-to-peer connections. If you want that use
>> AF_UNIX.
>
> Why?

kdbus implements bus-based IPC. If you want P2P IPC, use one of the
established transports.

Yes, kdbus has some handy features people like to see on unix-sockets
(like flexible metadata transports), but our current policy is "fix
unix sockets!", and "this can optionally be implemented later on".
There is no plan to support P2P connections in the initial kdbus
draft.

>> There's really no need for peer-to-peer connections really, at leas

Re: [systemd-devel] [PATCH] hwdb: add a new db for the DPI/frequency settings of mice

[Peter Hutterer]

On Tue, Nov 25, 2014 at 09:30:07PM +1000, Peter Hutterer wrote:
> Pointer acceleration for relative input devices (mice, trackballs, etc.)
> applies to the deltas of the device. Alas, those deltas have no physical
> reference point - a delta of 10 may be caused by a large movement of a low-dpi
> mouse or by a minute movement of a high-dpi mouse.
> Which makes pointer acceleration a bit useless and high-dpi devices
> essentially unusable.
> 
> In an ideal world, we could read the DPI from the device directly and work
> with that. In the world we actually live in, we need to compile this list
> manually. This patch introduces the database, with the usual match formats and
> a single property to be set on a device: MOUSE_DPI
> 
> That is either a single value for most mice, or a list of values for mice that
> can change resolution at runtime. The exact format is detailed in the hwdb
> file.
> 
> Note that we're explicitly overshooting the requirements we have for libinput
> atm. Frequency could be detected in software and we don't actually use the
> list of multiple resolutions (because we can't detect when they change
> anyway). However, we might as well collect those values from the get-go,
> adding/modifying what will eventually amount to hundreds of entries is a
> bit cumbersome.
> 
> Note: we rely on the input_id builtin to tag us as mouse first, ordering of
> the rules is important.

[...]

> ---
> +##
> +# Lenovo
> +##
> +
> +mouse:usb:v17efp6019:name:Lenovo Optical USB Mouse:
> +  MOUSE_DPI=1000@125

urgh, and I just noticed that I had a double-space here which caused the
whole thing to break for this mouse (but is hard to see in the udevadm
output...)

I'll send a new revision once we decide what to do with the naming.

Plus, would a fix be welcome to ignore multiple leading whitespaces? This
has caught me several times now and I keep making this mistake.

Cheers,
   Peter

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

Re: [systemd-devel] [PATCH] hwdb: add a new db for the DPI/frequency settings of mice

[Peter Hutterer]

On Tue, Nov 25, 2014 at 02:56:31PM +0100, Zbigniew Jędrzejewski-Szmek wrote:
> On Tue, Nov 25, 2014 at 09:35:16PM +1000, Peter Hutterer wrote:
> > Pointer acceleration for relative input devices (mice, trackballs, etc.)
> > applies to the deltas of the device. Alas, those deltas have no physical
> > reference point - a delta of 10 may be caused by a large movement of a 
> > low-dpi
> > mouse or by a minute movement of a high-dpi mouse.
> > Which makes pointer acceleration a bit useless and high-dpi devices
> > essentially unusable.
> > 
> > In an ideal world, we could read the DPI from the device directly and work
> > with that. In the world we actually live in, we need to compile this list
> > manually. This patch introduces the database, with the usual match formats 
> > and
> > a single property to be set on a device: MOUSE_DPI
> > 
> > That is either a single value for most mice, or a list of values for mice 
> > that
> > can change resolution at runtime. The exact format is detailed in the hwdb
> > file.
> > 
> > Note that we're explicitly overshooting the requirements we have for 
> > libinput
> > atm. Frequency could be detected in software and we don't actually use the
> > list of multiple resolutions (because we can't detect when they change
> > anyway). However, we might as well collect those values from the get-go,
> > adding/modifying what will eventually amount to hundreds of entries is a
> > bit cumbersome.
> > 
> > Note: we rely on the input_id builtin to tag us as mouse first, ordering of
> > the rules is important.
> > ---
> >  Makefile.am  |  4 ++-
> >  rules/70-mouse.hwdb  | 92 
> > 
> >  rules/70-mouse.rules | 15 +
> >  3 files changed, 110 insertions(+), 1 deletion(-)
> >  create mode 100644 rules/70-mouse.hwdb
> >  create mode 100644 rules/70-mouse.rules
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 3f9f3fa..d2b0d02 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -3285,6 +3285,7 @@ dist_udevrules_DATA += \
> > rules/50-udev-default.rules \
> > rules/60-drm.rules \
> > rules/60-keyboard.rules \
> > +   rules/70-mouse.rules \
> > rules/60-persistent-storage-tape.rules \
> > rules/60-persistent-serial.rules \
> > rules/60-persistent-input.rules \
> > @@ -3311,7 +3312,8 @@ dist_udevhwdb_DATA = \
> > hwdb/20-acpi-vendor.hwdb \
> > hwdb/20-OUI.hwdb \
> > hwdb/20-net-ifname.hwdb \
> > -   hwdb/60-keyboard.hwdb
> > +   hwdb/60-keyboard.hwdb \
> > +   hwdb/70-mouse.hwdb
> Wouldn't 60-mouse be more appropriate? It is very similar to 60-keyboard.

I had that at first but 60-mouse sorts before 60-persistent-input.rules and
we partially rely on that, specifically the line
SUBSYSTEMS=="usb", ENV{ID_BUS}=="", IMPORT{builtin}="usb_id"

Want me to add that line to this ruleset here, or should we leave the
sorting as-is?

> >  udevconfdir = $(sysconfdir)/udev
> >  dist_udevconf_DATA = \
> > diff --git a/rules/70-mouse.hwdb b/rules/70-mouse.hwdb
> > new file mode 100644
> > index 000..75e8755
> > --- /dev/null
> > +++ b/rules/70-mouse.hwdb
> > @@ -0,0 +1,92 @@
> > +# This file is part of systemd.
> > +#
> > +# Database for the DPI setting of mice, trackballs, other pointer devices 
> > that
> > +# cannot be queried directly.
> > +#
> > +# The lookup keys are composed in:
> > +#   60-mouse.rules
> Typo (if you elect not to use change to 60 ;))

damn those last-minute changes :)

Cheers,
   Peter

> Otherwise looks OK.
> 
> Zbyszek
> 
> > +# Note: The format of the "mouse:" prefix match key is a
> > +# contract between the rules file and the hardware data, it might
> > +# change in later revisions to support more or better matches, it
> > +# is not necessarily expected to be a stable ABI.
> > +#
> > +# Match string format:
> > +# mouse::vp:name::
> > +#
> > +# Supported subsystems: usb, bluetooth
> > +# vid/pid as 4-digit hex lowercase vendor/product
> > +#
> > +# if vid/pid is unavailable, use
> > +# mouse:*:name::
> > +# if name is unavailable, use
> > +# mouse::vp:*
> > +#
> > +# For example, the following 5 matches all match the same mouse:
> > +# mouse:usb:v17efp6019:name:Lenovo Optical USB Mouse:
> > +# mouse:usb:*:name:Lenovo Optical USB Mouse:
> > +# mouse:usb:v17efp6019:*
> > +# mouse:*:name:Lenovo Optical USB Mouse:
> > +#
> > +# DPI settings are specified as
> > +#MOUSE_DPI=[@]
> > +#
> > +# Where  is the resolution in dots per inch, and  the
> > +# optional sampling frequency in Hz.
> > +#
> > +# The value of MOUSE_DPI is:
> > +# - a single integer for single-resolution mice, e.g.
> > +#   MOUSE_DPI=800
> > +#   or, if the frequency is known:
> > +#   MOUSE_DPI=800@120
> > +# - a space-separated list of resolutions for multi-resolution mice.
> > +#   The default resolution must be prefixed by an asterisk, the resultions
> > +#   in the database must be as shipped by the manufacturer. e.g.
> > +#   MOUSE_DPI=400 *800 2000
> > +#
> > +#   The order of resolutions 

Re: [systemd-devel] Compatibility between D-Bus and kdbus

[Thiago Macieira]

On Tuesday 25 November 2014 17:11:36 Lennart Poettering wrote:
[snip]

Thanks for raising the resource limits.

> > == DBUS__BUS_ADDRESS ==
> > 
> > We probably discussed this. Should we specify that the address on the
> > 
> > environment variable should be of the form:
> >  kdbus:path=/sys/fs/kdbus/,uuid=[;fall back
> >  addresses]
> 
> Well, we don't need any env var really, as we enforce that the UID of
> the user is included in the name of their bussess, and the busses are
> cleaned up when the registrar dies. We don't have the risk of leaving
> old busses around, or even by other users, hence all code can just
> imply the path to use is kernel:path=/sys/fs/kdbus/0-system and
> kernel:path=/sys/fs/kdbus/$UID-user and all is good, without ever
> having to deal with env vars at all.
> 
> (of course, if env-vars are set they should be used, but the normal
> codepaths in the distros should work without them.)

Thinking of non-system buses here.

If the variable is empty, I agree that it should have an equivalent of an 
"autostart" mechanism, but I disagree on the solution and I also disagree that 
distros should leave it empty.

For one thing, the fallback address is expected to be there if there's a proxy 
bus running. The current autostart mechanism relies on X being present, so the 
fallback won't be found unless X is running and something registered the 
proxy's socket address there.

For another, it's good practice to have it set and not depend on autostart.

For a third, hardcoding kernel paths in userspace sounds like a poor idea. The 
kdbus mountpoint may be elsewhere and whatever is creating buses may not do it 
per user, but per session or other creation rule it may have.

So we should make sure code works when the env vars are missing, but we should 
recommend that they always be set.

> > == org.freedesktop.DBus connection ==
> > 
> > Will systemd-kdbus provide that name on the bus so applications that make
> > calls directly be able to continue working? I imagine the following
> > methods
> 
> > would be interesting to have:
> No, this is not supported in the current versions of kdbus
> anymore. Emulation of these calls must happen client side if it shall
> be supported.

That wouldn't be kdbus, but systemd doing it. Since systemd is the one that 
opens the bus, it can register the first connection and claim the 
org.freedesktop.DBus service name, providing compatibility. So this isn't a 
feature request for kdbus but a feature request for systemd.

By the way, is there a way to ensure that a given connection is the first 
connection? As soon as the bus creator is able to connect to the /sys/fs/kdbus 
path, so is another process and therefore this other process could maliciously 
acquire names it shouldn't.

> > org.freedesktop.DBus.ReloadConfig
> > org.freedesktop.DBus.StartServiceByName
> > org.freedesktop.DBus.UpdateActivationEnvironment
> > 
> > Most of those would be just convenience for other, existing kdbus
> > low-level
> > calls, but ReloadConfig and UpdateActivationEnvironment are not available
> > anywhere else. It's true that there's nothing stopping more CAP_IPC_OWNER
> > connections from installing more activators, but the question is whether
> > systemd will provide those for the activations it holds.
> 
> The client side emulation can choose to either forward ReloadConifg
> and UpdateActivationEnvironment to the respect systemd calls, or just
> return som "not supported" error.

Can't do that. What if it's a kdbus system that is not systemd?

I don't mind forwarding to a well-known bus name, as long as we establish that 
there is such a service running on the bus that will accept those calls. But 
if such a service exists, why can't it claim the org.freedesktop.DBus name?

> > == Kernel API ==
> > === Custom endpoints ===
> > 
> > The docs say "To create a custom endpoint, use the KDBUS_CMD_ENDPOINT_MAKE
> > ioctl". On what file descriptor? The one for the control file? Or can it
> > be sent on any kdbus endpoint? I'm asking because I'm not sure what the
> > permissions of the control file will be -- will any process be allowed to
> > open it and create endpoints?
> 
> if you want to create a new endpoint for an existing bus, then invoke
> that ioctl on the bus fd. The control file after all is unrelated to
> any bus, and thus wouldn#t know which bus you mean if we'd allow
> invoking that ioctl on it.

Ok, so any application that connected to the "bus" bus can then create custom 
endpoints. Correct?

How does one get to install policies or activators on this custom bus if the 
opening connection is a regular, non-privileged process?

> > But if that's the case, how would one implement a peer-to-peer connection?
> > Or should it simply be a convention that P2P connections are really
> > regular buses, except that no one owns any names, there are no policy
> > restrictions and that the only two connections are :1.1 and :1.2?
> 
> kdbus is not for peer-to-peer connections

[systemd-devel] [PATCH] journalctl: --follow with --since behaviour

[Andrej Manduch]

journalctl will print not only 10 lines but all relevant when --since is
in use

--- >8 ---

Hi,

When I tryed to run journalctl with --follow and --since arguments it
behaved very strangely.
First It prints logs from what I specified in --since argument, then
printed 10 lines (as is default in --follow) and when app put something
new in to log journalctl printed everithing from the last printed line.

How to reproduce:
1. run: journalctl -m --since 14:00 --follow
Then you'll see 10 lines of logs since 14:00. After that wait until some
app add something in the journal or just run `systemd-cat echo test`
2. After that journalctl will print every single line since 14:00 and will
follow as expected.

As long as --since and --follow will eventually print all relevant
lines, I seen no reason why not to print them right away and not after
first new message in journal.

Relevant bugzillas:
https://bugs.freedesktop.org/show_bug.cgi?id=71546
https://bugs.freedesktop.org/show_bug.cgi?id=64291
---
 src/journal/journalctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index b168d1e..904880b 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -712,7 +712,7 @@ static int parse_argv(int argc, char *argv[]) {
 assert_not_reached("Unhandled option");
 }
 
-if (arg_follow && !arg_no_tail && arg_lines == ARG_LINES_DEFAULT)
+if (arg_follow && !arg_no_tail && !arg_since && arg_lines == 
ARG_LINES_DEFAULT)
 arg_lines = 10;
 
 if (!!arg_directory + !!arg_file + !!arg_machine > 1) {
-- 
2.0.0.GIT

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

[systemd-devel] [PATCH] unit-name: fix escaping logic in unit_name_mangle_with_suffix().

[Ivan Shapovalov]

Make screened character set consistent with unit_name_mangle() by splitting off
the escaping loop into a separate function.

Before this fix, unit names such as `foo@bar.target` would get transformed
into `foo\x40bar.target` when unit_name_mangle_with_suffix() is used.

https://bugs.freedesktop.org/show_bug.cgi?id=86711
---
 src/shared/unit-name.c | 51 +-
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/src/shared/unit-name.c b/src/shared/unit-name.c
index 2ef8545..6c6d7f4 100644
--- a/src/shared/unit-name.c
+++ b/src/shared/unit-name.c
@@ -243,6 +243,30 @@ static char *do_escape(const char *f, char *t) {
 return t;
 }
 
+static char *do_escape_mangle(const char *f, enum unit_name_mangle 
allow_globs, char *t) {
+const char *valid_chars;
+
+assert(f);
+assert(IN_SET(allow_globs, MANGLE_GLOB, MANGLE_NOGLOB));
+assert(t);
+
+/* We'll only escape the obvious characters here, to play
+ * safe. */
+
+valid_chars = allow_globs == MANGLE_GLOB ? "@" VALID_CHARS "[]!-*?" : 
"@" VALID_CHARS;
+
+for (; *f; f++) {
+if (*f == '/')
+*(t++) = '-';
+else if (!strchr(valid_chars, *f))
+t = do_escape_char(*f, t);
+else
+*(t++) = *f;
+}
+
+return t;
+}
+
 char *unit_name_escape(const char *f) {
 char *r, *t;
 
@@ -482,11 +506,9 @@ int unit_name_from_dbus_path(const char *path, char 
**name) {
  *  sensible unit name.
  */
 char *unit_name_mangle(const char *name, enum unit_name_mangle allow_globs) {
-const char *valid_chars, *f;
 char *r, *t;
 
 assert(name);
-assert(IN_SET(allow_globs, MANGLE_GLOB, MANGLE_NOGLOB));
 
 if (is_device_path(name))
 return unit_name_from_path(name, ".device");
@@ -494,23 +516,11 @@ char *unit_name_mangle(const char *name, enum 
unit_name_mangle allow_globs) {
 if (path_is_absolute(name))
 return unit_name_from_path(name, ".mount");
 
-/* We'll only escape the obvious characters here, to play
- * safe. */
-
-valid_chars = allow_globs == MANGLE_GLOB ? "@" VALID_CHARS "[]!-*?" : 
"@" VALID_CHARS;
-
 r = new(char, strlen(name) * 4 + strlen(".service") + 1);
 if (!r)
 return NULL;
 
-for (f = name, t = r; *f; f++) {
-if (*f == '/')
-*(t++) = '-';
-else if (!strchr(valid_chars, *f))
-t = do_escape_char(*f, t);
-else
-*(t++) = *f;
-}
+t = do_escape_mangle(name, allow_globs, r);
 
 if (unit_name_to_type(name) < 0)
 strcpy(t, ".service");
@@ -526,10 +536,8 @@ char *unit_name_mangle(const char *name, enum 
unit_name_mangle allow_globs) {
  */
 char *unit_name_mangle_with_suffix(const char *name, enum unit_name_mangle 
allow_globs, const char *suffix) {
 char *r, *t;
-const char *f;
 
 assert(name);
-assert(IN_SET(allow_globs, MANGLE_GLOB, MANGLE_NOGLOB));
 assert(suffix);
 assert(suffix[0] == '.');
 
@@ -537,14 +545,7 @@ char *unit_name_mangle_with_suffix(const char *name, enum 
unit_name_mangle allow
 if (!r)
 return NULL;
 
-for (f = name, t = r; *f; f++) {
-if (*f == '/')
-*(t++) = '-';
-else if (!strchr(VALID_CHARS, *f))
-t = do_escape_char(*f, t);
-else
-*(t++) = *f;
-}
+t = do_escape_mangle(name, allow_globs, r);
 
 if (!endswith(name, suffix))
 strcpy(t, suffix);
-- 
2.1.3

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

[systemd-devel] [PATCH] rpcbind: systemd socket activation (v2)

[Steve Dickson]

This is based on a patch originally posted by Lennart Poettering:
.

That patch was not merged due to the lack of a shared library and
as systemd was seen to be too Fedora specific.

Systemd now provides a shared library, and it is (or very soon will
be) the default init system on all the major Linux distributions.

This version of the patch has three changes from the original:

 * It uses the shared library.
 * It comes with unit files.
 * It is rebased on top of master.

Please review the patch with "git show -b" or otherwise ignoring the
whitespace changes, or it will be extremely difficult to read.

v5: incorporated comments on the PKG_CHECK_MODULES macro.

v4: reorganized the changes to make the diff easier to read
remove systemd scripts.

v3: rebase
fix typos
listen on /run/rpcbind.sock, rather than /var/run/rpcbind.sock (the
latter is a symlink to the former, but this means the socket can be
created before /var is mounted)
NB: this version has been compile-tested only as I no longer use
rpcbind myself
v2: correctly enable systemd code at compile time
handle the case where not all the required sockets were supplied
listen on udp/tcp port 111 in addition to /var/run/rpcbind.sock
do not daemonize

Tom Gundersen (1):
  rpcbind: add support for systemd socket activation

 Makefile.am   |  6 +
 configure.ac  | 12 +
 src/rpcbind.c | 81 ++-
 3 files changed, 93 insertions(+), 6 deletions(-)

-- 
1.9.3

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

[systemd-devel] [PATCH] rpcbind: add support for systemd socket activation

[Steve Dickson]

From: Tom Gundersen 

Making rpcbind sockect activated will greatly simplify
its integration in systemd systems. In essence, other services
may now assume that rpcbind is always available, even during very
early boot. This means that we no longer need to worry about any
ordering dependencies.

Original-patch-by: Lennart Poettering 
Cc: systemd-devel@lists.freedesktop.org
Acked-by: Cristian Rodríguez
Signed-off-by: Tom Gundersen 
Signed-off-by: Steve Dickson 
---
 Makefile.am   |  6 +
 configure.ac  | 12 +
 src/rpcbind.c | 81 ++-
 3 files changed, 93 insertions(+), 6 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 8715082..c99566d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -41,6 +41,12 @@ rpcbind_SOURCES = \
src/warmstart.c
 rpcbind_LDADD = $(TIRPC_LIBS)
 
+if SYSTEMD
+AM_CPPFLAGS += $(SYSTEMD_CFLAGS) -DSYSTEMD
+
+rpcbind_LDADD += $(SYSTEMD_LIBS)
+endif
+
 rpcinfo_SOURCES =   src/rpcinfo.c
 rpcinfo_LDADD   =   $(TIRPC_LIBS)
 
diff --git a/configure.ac b/configure.ac
index 5a88cc7..967eb05 100644
--- a/configure.ac
+++ b/configure.ac
@@ -36,6 +36,18 @@ AC_SUBST([nss_modules], [$with_nss_modules])
 
 PKG_CHECK_MODULES([TIRPC], [libtirpc])
 
+PKG_PROG_PKG_CONFIG
+AC_ARG_WITH([systemdsystemunitdir],
+  AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd 
service files]),
+  [], [with_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir 
systemd)])
+  if test "x$with_systemdsystemunitdir" != xno; then
+AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])
+ PKG_CHECK_MODULES([SYSTEMD], [libsystemd], [],
+  [PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon], [],
+  AC_MSG_ERROR([libsystemd support requested but found]))])
+  fi
+AM_CONDITIONAL(SYSTEMD, [test -n "$with_systemdsystemunitdir" -a 
"x$with_systemdsystemunitdir" != xno ])
+
 AS_IF([test x$enable_libwrap = xyes], [
AC_CHECK_LIB([wrap], [hosts_access], ,
AC_MSG_ERROR([libwrap support requested but unable to find 
libwrap]))
diff --git a/src/rpcbind.c b/src/rpcbind.c
index e3462e3..f7c71ee 100644
--- a/src/rpcbind.c
+++ b/src/rpcbind.c
@@ -56,6 +56,9 @@
 #include 
 #endif
 #include 
+#ifdef SYSTEMD
+#include 
+#endif
 #include 
 #include 
 #include 
@@ -296,6 +299,7 @@ init_transport(struct netconfig *nconf)
u_int32_t host_addr[4];  /* IPv4 or IPv6 */
struct sockaddr_un sun;
mode_t oldmask;
+   int n;
 res = NULL;
 
if ((nconf->nc_semantics != NC_TPI_CLTS) &&
@@ -314,6 +318,76 @@ init_transport(struct netconfig *nconf)
fprintf(stderr, "[%d] - %s\n", i, *s);
}
 #endif
+   if (!__rpc_nconf2sockinfo(nconf, &si)) {
+   syslog(LOG_ERR, "cannot get information for %s",
+   nconf->nc_netid);
+   return (1);
+   }
+
+#ifdef SYSTEMD
+   n = sd_listen_fds(0);
+   if (n < 0) {
+   syslog(LOG_ERR, "failed to acquire systemd sockets: %s", 
strerror(-n));
+   return 1;
+   }
+
+   /* Try to find if one of the systemd sockets we were given match
+* our netconfig structure. */
+
+   for (fd = SD_LISTEN_FDS_START; fd < SD_LISTEN_FDS_START + n; fd++) {
+   struct __rpc_sockinfo si_other;
+   union {
+   struct sockaddr sa;
+   struct sockaddr_un un;
+   struct sockaddr_in in4;
+   struct sockaddr_in6 in6;
+   struct sockaddr_storage storage;
+   } sa;
+   socklen_t addrlen = sizeof(sa);
+
+   if (!__rpc_fd2sockinfo(fd, &si_other)) {
+   syslog(LOG_ERR, "cannot get information for fd %i", fd);
+   return 1;
+   }
+
+   if (si.si_af != si_other.si_af ||
+si.si_socktype != si_other.si_socktype ||
+si.si_proto != si_other.si_proto)
+   continue;
+
+   if (getsockname(fd, &sa.sa, &addrlen) < 0) {
+   syslog(LOG_ERR, "failed to query socket name: %s",
+   strerror(errno));
+   goto error;
+   }
+
+   /* Copy the address */
+   taddr.addr.maxlen = taddr.addr.len = addrlen;
+   taddr.addr.buf = malloc(addrlen);
+   if (taddr.addr.buf == NULL) {
+   syslog(LOG_ERR,
+   "cannot allocate memory for %s address",
+   nconf->nc_netid);
+   goto error;
+   }
+   memcpy(taddr.addr.buf, &sa, addrlen);
+
+   my_xprt = (SVCXPRT *)svc_tli_create(fd, nconf, &taddr,
+  RPC_MAXDATASIZE, RPC_MAXDATASIZE);
+   if (my_xprt == (SVCXPRT *)NULL) {
+  

Re: [systemd-devel] [PATCH] rpcbind: add support for systemd socket activation

[Steve Dickson]

Hello,

On 11/22/2014 09:24 PM, Zbigniew Jędrzejewski-Szmek wrote:
> On Fri, Nov 21, 2014 at 11:43:47AM -0500, Steve Dickson wrote:
>> From: Tom Gundersen 
>>
>> Making rpcbind sockect activated will greatly simplify
>> its integration in systemd systems. In essence, other services
>> may now assume that rpcbind is always available, even during very
>> early boot. This means that we no longer need to worry about any
>> ordering dependencies.
>>
>> This is based on a patch originally posted by Lennart Poettering:
>> .
>>
>> That patch was not merged due to the lack of a shared library and
>> as systemd was seen to be too Fedora specific.
>>
>> Systemd now provides a shared library, and it is (or very soon will
>> be) the default init system on all the major Linux distributions.
>>
>> This version of the patch has three changes from the original:
>>
>>  * It uses the shared library.
>>  * It comes with unit files.
>>  * It is rebased on top of master.
>>
>> Please review the patch with "git show -b" or otherwise ignoring the
>> whitespace changes, or it will be extremely difficult to read.
> 
> Thanks for working on this... Looks fine to me. Two suggestions
> below.
np... 

> 
>> v4: reorganized the changes to make the diff easier to read
>> remove systemd scripts.
>>
>> v3: rebase
>> fix typos
>> listen on /run/rpcbind.sock, rather than /var/run/rpcbind.sock (the
>> latter is a symlink to the former, but this means the socket can be
>> created before /var is mounted)
>> NB: this version has been compile-tested only as I no longer use
>> rpcbind myself
>> v2: correctly enable systemd code at compile time
>> handle the case where not all the required sockets were supplied
>> listen on udp/tcp port 111 in addition to /var/run/rpcbind.sock
>> do not daemonize
>>
>> Original-patch-by: Lennart Poettering 
>> Cc: Steve Dickson 
>> Cc: systemd-devel@lists.freedesktop.org
>> Acked-by: Cristian Rodríguez
>> Signed-off-by: Tom Gundersen 
>> Signed-off-by: Steve Dickson 
>> ---
>>  Makefile.am   |  6 +
>>  configure.ac  | 10 
>>  src/rpcbind.c | 81 
>> ++-
>>  3 files changed, 91 insertions(+), 6 deletions(-)
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index 8715082..c99566d 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -41,6 +41,12 @@ rpcbind_SOURCES = \
>>  src/warmstart.c
>>  rpcbind_LDADD = $(TIRPC_LIBS)
>>  
>> +if SYSTEMD
>> +AM_CPPFLAGS += $(SYSTEMD_CFLAGS) -DSYSTEMD
>> +
>> +rpcbind_LDADD += $(SYSTEMD_LIBS)
>> +endif
>> +
>>  rpcinfo_SOURCES =   src/rpcinfo.c
>>  rpcinfo_LDADD   =   $(TIRPC_LIBS)
>>  
>> diff --git a/configure.ac b/configure.ac
>> index 5a88cc7..fdad639 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -36,6 +36,16 @@ AC_SUBST([nss_modules], [$with_nss_modules])
>>  
>>  PKG_CHECK_MODULES([TIRPC], [libtirpc])
>>  
>> +PKG_PROG_PKG_CONFIG
>> +AC_ARG_WITH([systemdsystemunitdir],
>> +  AS_HELP_STRING([--with-systemdsystemunitdir=DIR], [Directory for systemd 
>> service files]),
>> +  [], [with_systemdsystemunitdir=$($PKG_CONFIG 
>> --variable=systemdsystemunitdir systemd)])
>> +  if test "x$with_systemdsystemunitdir" != xno; then
>> +AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])
>> +PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon])
> libsystemd-daemon got subsummed by libsystemd. So you should use something
> like
> 
>PKG_CHECK_MODULES([SYSTEMD], [libsystemd], [],
>   [PKG_CHECK_MODULES([SYSTEMD], [libsystemd-daemon], [],
>AC_MSG_ERROR([libsystemd support requested but found]))])
> 
> to get the new libary in preference to the old one.
Got it... 

> 
>> +  fi
>> +AM_CONDITIONAL(SYSTEMD, [test -n "$with_systemdsystemunitdir" -a 
>> "x$with_systemdsystemunitdir" != xno ])
>> +
>>  AS_IF([test x$enable_libwrap = xyes], [
>>  AC_CHECK_LIB([wrap], [hosts_access], ,
>>  AC_MSG_ERROR([libwrap support requested but unable to find 
>> libwrap]))
>> diff --git a/src/rpcbind.c b/src/rpcbind.c
>> index e3462e3..f7c71ee 100644
>> --- a/src/rpcbind.c
>> +++ b/src/rpcbind.c
>> @@ -56,6 +56,9 @@
>>  #include 
>>  #endif
>>  #include 
>> +#ifdef SYSTEMD
>> +#include 
>> +#endif
>>  #include 
>>  #include 
>>  #include 
>> @@ -296,6 +299,7 @@ init_transport(struct netconfig *nconf)
>>  u_int32_t host_addr[4];  /* IPv4 or IPv6 */
>>  struct sockaddr_un sun;
>>  mode_t oldmask;
>> +int n;
>>  res = NULL;
>>  
>>  if ((nconf->nc_semantics != NC_TPI_CLTS) &&
>> @@ -314,6 +318,76 @@ init_transport(struct netconfig *nconf)
>>  fprintf(stderr, "[%d] - %s\n", i, *s);
>>  }
>>  #endif
>> +if (!__rpc_nconf2sockinfo(nconf, &si)) {
>> +syslog(LOG_ERR, "cannot get information for %s",
>> +nconf->nc_netid);
>> +return (1);
>> +}
>> +
>> +#ifdef SYSTEMD
>> +n = sd_listen_fds(0);
>> +  

Re: [systemd-devel] Compatibility between D-Bus and kdbus

[Lennart Poettering]

On Mon, 24.11.14 18:40, Thiago Macieira (thi...@kde.org) wrote:

> Another thought that comes to mind: should we reserve the entire highest bit 
> in connection IDs for broadcasts? It would allow for the existence of 
> multicast groups in the future.

I discussed this quickly with the kdbus guys, and while none of us was
thrilled about the posibility of introducing such a concept we all
agreed that if it should be introduced one day it really should be
part of the well-known names concept, not the unique names
concept. Meaning: you'd put a label on a group, and join the group by
the label then, rather than via numeric ids...

In general, numeric ids are about being automatic, and well-known
names are about discoverability. But mcast memberships should never be
automatic, but only about discoverability, hence using unique ids for
identifying them sounds wrong.

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] Compatibility between D-Bus and kdbus

[Lennart Poettering]

On Mon, 24.11.14 18:40, Thiago Macieira (thi...@kde.org) wrote:

> I'm wondering if the same solution should be applied to the session bus. That 
> would have the unfortunate effect that applications that aren't ported to 
> know 
> about kdbus will always fallback to proxy functionality. It would be 
> unfortunate because the number of applications that need policy decisions on 
> the session bust must be asymptotically close to zero.

I figure this is up to the library implementation. I'd probably
simplify this and avoid duplicating the session bus name too.

> > Session bus access-control policy
> > =
> > 
> > In principle, people could configure the session bus to do the same
> > elaborate access-control as the system bus. In practice, this is not a
> > particularly useful thing to do, because there are many ways for
> > processes running under the same uid to subvert each other, particularly
> > if a LSM like SELinux or AppArmor is not used.
> > 
> > kdbus does not appear to make any attempt to protect a uid from itself:
> > the uid that created a bus is considered to be privileged on that bus. I
> > assume this means that the intention is that app sandboxing will use a
> > separate Unix uid, like it does on Android?
> > 
> > Unless there's an outcry from people who like LSMs, I'm inclined to say
> > that protecting same-uid session processes from each other is doomed to
> > failure, and hence that it's OK for DBUS_BUS_SESSION to connect to kdbus
> > without special precautions.
> 
> I don't understand this domain enough to be able to offer an opinion. I know 
> that Tizen will want SMACK security applied even between processes of the 
> same 
> UID. I just don't know whether that maps to what Lennart said about
> "labels".

SMACK and SELinux will have the chance to make stricter decisions
thang the baseline policy. The SMACK folks have posted patches that
add the right hooks to kdbus to make this possible.

> That's over 25% of the limit. Can this be made runtime-configurable?

Yes, that's the plan. THough currently they are compiled in.

I have now increased this to 1K.

> > In kdbus, each connection may own up to 64 well-known names; the system
> > dbus-daemon defaults to 512, and the session to 50 000. 64 is *probably*
> > enough, but I could potentially see this becoming an issue for services
> > that allocate one well-known name per $app_specific_thing, like
> > Telepathy (one name per Connection).
> 
> Also be made configurable, but please raise the default to 256 or
> 512.

Same, also increased now. To 256.

https://code.google.com/p/d-bus/source/detail?r=20ce3cfa9f65fc6a0be052ec64d9d796626f6630

> A couple of other items to discuss:
> 
> == DBUS__BUS_ADDRESS ==
> 
> We probably discussed this. Should we specify that the address on the 
> environment variable should be of the form:
> 
>  kdbus:path=/sys/fs/kdbus/,uuid=[;fall back addresses]

Well, we don't need any env var really, as we enforce that the UID of
the user is included in the name of their bussess, and the busses are
cleaned up when the registrar dies. We don't have the risk of leaving
old busses around, or even by other users, hence all code can just
imply the path to use is kernel:path=/sys/fs/kdbus/0-system and
kernel:path=/sys/fs/kdbus/$UID-user and all is good, without ever
having to deal with env vars at all.

(of course, if env-vars are set they should be used, but the normal
codepaths in the distros should work without them.)

> == org.freedesktop.DBus connection ==
> 
> Will systemd-kdbus provide that name on the bus so applications that make 
> calls directly be able to continue working? I imagine the following methods 
> would be interesting to have:

No, this is not supported in the current versions of kdbus
anymore. Emulation of these calls must happen client side if it shall
be supported.

> org.freedesktop.DBus.GetAdtAuditSessionData
> org.freedesktop.DBus.GetConnectionCredentials
> org.freedesktop.DBus.GetConnectionSELinuxSecurityContext
> org.freedesktop.DBus.GetConnectionUnixProcessID
> org.freedesktop.DBus.GetConnectionUnixUser
> org.freedesktop.DBus.GetId
> org.freedesktop.DBus.GetNameOwner
> org.freedesktop.DBus.ListActivatableNames
> org.freedesktop.DBus.ListNames
> org.freedesktop.DBus.ListQueuedOwners
> org.freedesktop.DBus.NameHasOwner
> org.freedesktop.DBus.ReloadConfig
> org.freedesktop.DBus.StartServiceByName
> org.freedesktop.DBus.UpdateActivationEnvironment
> 
> Most of those would be just convenience for other, existing kdbus low-level 
> calls, but ReloadConfig and UpdateActivationEnvironment are not available 
> anywhere else. It's true that there's nothing stopping more CAP_IPC_OWNER 
> connections from installing more activators, but the question is whether 
> systemd will provide those for the activations it holds.

The client side emulation can choose to either forward ReloadConifg
and UpdateActivationEnvironment to the respect systemd calls, or just
retur

Re: [systemd-devel] how to properly control the daemons or run advanced cmds

[Flavio Leitner]

On Tue, Nov 25, 2014 at 10:42:12AM +, Richard Maw wrote:
> On Tue, Nov 25, 2014 at 12:09:07AM -0200, Flavio Leitner wrote:
> > 
> > Hello,
> > 
> > The Open vSwitch is comprised by two daemons. One is a database and
> > another is the switch itself.
> > 
> > Currently we have the openvswitch.service which start/stop/reload the
> > service (both daemons) just fine.
> > 
> > However, we need to support hot-upgrade which means to stop the
> > vswitch daemon first, run a few special commands, reload the db
> > daemon and only then start the vswitch daemon.
> > 
> > I know about creating shell helpers for non-standard commands, but
> > since that needs to mess up with the daemons in a particular order,
> > I think systemd won't like the above external actions at all.
> > 
> > Any suggestion on how to handle this with systemd properly?
> 
> We've previously had discussion on how to handle hot-upgrade, though I
> don't remember enough details to find the message again.
> 
> I think the resolution was that the daemon in question must either
> re-exec itself, or have NotifyAccess=all set in the systemd unit, and
> it must notify systemd of the PID of the new version of the daemon by
> sending MAINPID=$NEWPID with sd_notify.

This is the current status output. Notice that everything is initialized
by a script passing all the parameters and initializing other things if
needed as well.

$ systemctl -l status openvswitch-nonetwork
openvswitch-nonetwork.service - Open vSwitch Internal Unit
   Loaded: loaded (/usr/lib/systemd/system/openvswitch-nonetwork.service; 
static)
   Active: active (exited) since Mon 2014-11-24 09:38:58 BRST; 1 day 3h ago
 Main PID: 1683 (code=exited, status=0/SUCCESS)
   CGroup: /system.slice/openvswitch-nonetwork.service
   ├─1795 ovsdb-server: monitoring pid 1796 (healthy)   
 
   ├─1796 ovsdb-server /etc/openvswitch/conf.db -vconsole:emer 
-vsyslog:err -vfile:info --remote=punix:/var/run/openvswitch/db.sock 
--private-key=db:Open_vSwitch,SSL,private_key 
--certificate=db:Open_vSwitch,SSL,certificate 
--bootstrap-ca-cert=db:Open_vSwitch,SSL,ca_cert --no-chdir 
--log-file=/var/log/openvswitch/ovsdb-server.log 
--pidfile=/var/run/openvswitch/ovsdb-server.pid --detach --monitor
   ├─1806 ovs-vswitchd: monitoring pid 1807 (healthy)
   └─1807 ovs-vswitchd unix:/var/run/openvswitch/db.sock -vconsole:emer 
-vsyslog:err -vfile:info --mlockall --no-chdir 
--log-file=/var/log/openvswitch/ovs-vswitchd.log 
--pidfile=/var/run/openvswitch/ovs-vswitchd.pid --detach --monitor


Since I need to stop completely vswitchd, then restart the ovsdb,
then run some commands and only after that start the vswitchd daemon,
it doesn't look like re-exec'ing itself will work or I am missing
something.

> Unless you overload ExecReload to mean hot-upgrade/graceful-reexec then
> you'll have to have an external tool instruct the service to upgrade
> itself, rather than issuing a `systemctl reload vswitch.service`.

It doesn't seem to be a good idea because it will surprise the users
in a bad way.  Those steps are required only for hot-upgrade or force
kernel module reloading which are unusual.

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

Re: [systemd-devel] [PATCH] hwdb: add a new db for the DPI/frequency settings of mice

[Zbigniew Jędrzejewski-Szmek]

On Tue, Nov 25, 2014 at 09:35:16PM +1000, Peter Hutterer wrote:
> Pointer acceleration for relative input devices (mice, trackballs, etc.)
> applies to the deltas of the device. Alas, those deltas have no physical
> reference point - a delta of 10 may be caused by a large movement of a low-dpi
> mouse or by a minute movement of a high-dpi mouse.
> Which makes pointer acceleration a bit useless and high-dpi devices
> essentially unusable.
> 
> In an ideal world, we could read the DPI from the device directly and work
> with that. In the world we actually live in, we need to compile this list
> manually. This patch introduces the database, with the usual match formats and
> a single property to be set on a device: MOUSE_DPI
> 
> That is either a single value for most mice, or a list of values for mice that
> can change resolution at runtime. The exact format is detailed in the hwdb
> file.
> 
> Note that we're explicitly overshooting the requirements we have for libinput
> atm. Frequency could be detected in software and we don't actually use the
> list of multiple resolutions (because we can't detect when they change
> anyway). However, we might as well collect those values from the get-go,
> adding/modifying what will eventually amount to hundreds of entries is a
> bit cumbersome.
> 
> Note: we rely on the input_id builtin to tag us as mouse first, ordering of
> the rules is important.
> ---
>  Makefile.am  |  4 ++-
>  rules/70-mouse.hwdb  | 92 
> 
>  rules/70-mouse.rules | 15 +
>  3 files changed, 110 insertions(+), 1 deletion(-)
>  create mode 100644 rules/70-mouse.hwdb
>  create mode 100644 rules/70-mouse.rules
> 
> diff --git a/Makefile.am b/Makefile.am
> index 3f9f3fa..d2b0d02 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -3285,6 +3285,7 @@ dist_udevrules_DATA += \
>   rules/50-udev-default.rules \
>   rules/60-drm.rules \
>   rules/60-keyboard.rules \
> + rules/70-mouse.rules \
>   rules/60-persistent-storage-tape.rules \
>   rules/60-persistent-serial.rules \
>   rules/60-persistent-input.rules \
> @@ -3311,7 +3312,8 @@ dist_udevhwdb_DATA = \
>   hwdb/20-acpi-vendor.hwdb \
>   hwdb/20-OUI.hwdb \
>   hwdb/20-net-ifname.hwdb \
> - hwdb/60-keyboard.hwdb
> + hwdb/60-keyboard.hwdb \
> + hwdb/70-mouse.hwdb
Wouldn't 60-mouse be more appropriate? It is very similar to 60-keyboard.

>  udevconfdir = $(sysconfdir)/udev
>  dist_udevconf_DATA = \
> diff --git a/rules/70-mouse.hwdb b/rules/70-mouse.hwdb
> new file mode 100644
> index 000..75e8755
> --- /dev/null
> +++ b/rules/70-mouse.hwdb
> @@ -0,0 +1,92 @@
> +# This file is part of systemd.
> +#
> +# Database for the DPI setting of mice, trackballs, other pointer devices 
> that
> +# cannot be queried directly.
> +#
> +# The lookup keys are composed in:
> +#   60-mouse.rules
Typo (if you elect not to use change to 60 ;))

Otherwise looks OK.

Zbyszek

> +# Note: The format of the "mouse:" prefix match key is a
> +# contract between the rules file and the hardware data, it might
> +# change in later revisions to support more or better matches, it
> +# is not necessarily expected to be a stable ABI.
> +#
> +# Match string format:
> +# mouse::vp:name::
> +#
> +# Supported subsystems: usb, bluetooth
> +# vid/pid as 4-digit hex lowercase vendor/product
> +#
> +# if vid/pid is unavailable, use
> +# mouse:*:name::
> +# if name is unavailable, use
> +# mouse::vp:*
> +#
> +# For example, the following 5 matches all match the same mouse:
> +# mouse:usb:v17efp6019:name:Lenovo Optical USB Mouse:
> +# mouse:usb:*:name:Lenovo Optical USB Mouse:
> +# mouse:usb:v17efp6019:*
> +# mouse:*:name:Lenovo Optical USB Mouse:
> +#
> +# DPI settings are specified as
> +#MOUSE_DPI=[@]
> +#
> +# Where  is the resolution in dots per inch, and  the
> +# optional sampling frequency in Hz.
> +#
> +# The value of MOUSE_DPI is:
> +# - a single integer for single-resolution mice, e.g.
> +#   MOUSE_DPI=800
> +#   or, if the frequency is known:
> +#   MOUSE_DPI=800@120
> +# - a space-separated list of resolutions for multi-resolution mice.
> +#   The default resolution must be prefixed by an asterisk, the resultions
> +#   in the database must be as shipped by the manufacturer. e.g.
> +#   MOUSE_DPI=400 *800 2000
> +#
> +#   The order of resolutions is as configured by the HW manufacturer or in
> +#   ascending order, whichever appropriate.
> +#
> +#   The frequency must be given to either none or all resolutions. If the
> +#   device supports multiple frequencies, the order of items is
> +#   MOUSE_DPI=r1@f1 r2@f1 r3@f1 r1@f2 r2@f2 r3@f2
> +#
> +#   If the default manufacturer-set resolution is unclear, a resolution of
> +#   800 or 1000 should be set as default, if available. If neither is
> +#   available, choose the "middle" resolution value of those available.
> +#
> +#   The list may contain a single item which must be marked with an
> +#   asterisk.
> +#
> +

Re: [systemd-devel] Service not restarting after Condition failed

[D.S. Ljungmark]

On 24/11/14 20:30, Umut Tezduyar Lindskog wrote:
> Hi
> 
> On Monday, November 24, 2014, D.S. Ljungmark  > wrote:
> 
> On 10/11/14 23:09, Lennart Poettering wrote:
> > On Thu, 30.10.14 18:47, D.S. Ljungmark (spi...@aanstoot.se
> ) wrote:
> >
> >> Hi
> >>  we have a service set to:
> >> ConditionFileNotEmpty=
> >>
> >> and
> >>
> >> Restart=Always
> >>
> >>
> >> This combination would (in my feebled mind) cause the service to
> restart
> >> once the Condition was fulfilled, but that doesn't seem to be the
> >> case.
> >
> > Conditions are something that are on-time evaluated right before we
> > would start a unit, and cause this starting to be shortcut. That's all
> > really. Restarts are only triggered when a running service dies, and
> > the start job queued by that will then check the conditions again. If
> > the condition doesn't hold then this start will not be executed, and
> > hence no restart ever again either...
> >
> >> Is there a way I can get a service to restart even after it has
> been set
> >> as inactive (dead) "start condition failed"?
> >
> > Nope, conditions are not for that. For the specific check of
> > file-not-empty there's no nice way to handle this, however for
> > directory-not-empty you could set up DirectoryNotEmpty=...
> >
> >> Should I simply remove the Condition, or something else?
> >
> > What precisely are you trying to do?
> >
> > Lennart
> 
> Basically, some files (config & certificates) may not exist on a system
> until it's provisioned properly, something that may take a while ( a few
> days)
> 
> After provisioning, we want the services depending on those file to
> start automatically.
> 
>  
> Path unit files should work for
> you,  http://www.freedesktop.org/software/systemd/man/systemd.path.html
> 
> Umut

Thanks, those would work well. I take it those should be in combination
to the ConditionFileNotEmpty=  for the same files?



//D.S.


-- 
8362 CB14 98AD 11EF CEB6  FA81 FCC3 7674 449E 3CFC



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

Re: [systemd-devel] networkd - how to start service right after the "link configured" stage?

[Tom Gundersen]

On Tue, Nov 25, 2014 at 1:02 PM, Peter Lemenkov  wrote:
> I'm currently doing a great stuff with systemd-networkd but one thing
> is holding me back. One of my services is starting too early. I'd love
> to delay its startup right after the "link configured" stage.So far
> I've got the followng in journal logs:
>
> el7 ~: sudo journalctl -M mycontainer -b --unit="systemd-networkd"
> -- Logs begin at Пн 2014-11-24 16:41:18 MSK, end at Вт 2014-11-25
> 14:53:14 MSK. --
> ноя 25 14:53:08 mycontainer systemd[1]: Starting Network Service...
> ноя 25 14:53:08 mycontainer systemd[1]: Started Network Service.
> ноя 25 14:53:08 mycontainer systemd-networkd[29]: host0   :
> gained carrier
> ноя 25 14:53:08 mycontainer systemd-networkd[29]: host0   :
> DHCPv4 address 192.168.122.169/24 via 192.168.122.1
> ноя 25 14:53:14 mycontainer systemd-networkd[29]: host0   :
> link configured
> el7 ~:
>
> So far I've got few questions:
>
> - Does networkd emit some D-Bus event when the network link is configured?

networkd currently does not have an dbus API (will get once we have kdbus).

> - If yes is it possible to add a systemd service triggered by this
> D-Bus message?
> - If no is there a way to start systemd service after the given
> network link is configured?

We have an (so far internal) library sd-network, which allows you to
monitor for events. /lib/systemd/networkd-wait-online uses this, and
it may fit your needs (it has various command-line options you may
tweak to make it do what you want).

HTH,

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

[systemd-devel] networkd - how to start service right after the "link configured" stage?

[Peter Lemenkov]

Hello All!
I'm currently doing a great stuff with systemd-networkd but one thing
is holding me back. One of my services is starting too early. I'd love
to delay its startup right after the "link configured" stage.So far
I've got the followng in journal logs:

el7 ~: sudo journalctl -M mycontainer -b --unit="systemd-networkd"
-- Logs begin at Пн 2014-11-24 16:41:18 MSK, end at Вт 2014-11-25
14:53:14 MSK. --
ноя 25 14:53:08 mycontainer systemd[1]: Starting Network Service...
ноя 25 14:53:08 mycontainer systemd[1]: Started Network Service.
ноя 25 14:53:08 mycontainer systemd-networkd[29]: host0   :
gained carrier
ноя 25 14:53:08 mycontainer systemd-networkd[29]: host0   :
DHCPv4 address 192.168.122.169/24 via 192.168.122.1
ноя 25 14:53:14 mycontainer systemd-networkd[29]: host0   :
link configured
el7 ~:

So far I've got few questions:

- Does networkd emit some D-Bus event when the network link is configured?
- If yes is it possible to add a systemd service triggered by this
D-Bus message?
- If no is there a way to start systemd service after the given
network link is configured?

-- 
With best regards, Peter Lemenkov.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] hwdb: add a new db for the DPI/frequency settings of mice

[Peter Hutterer]

Pointer acceleration for relative input devices (mice, trackballs, etc.)
applies to the deltas of the device. Alas, those deltas have no physical
reference point - a delta of 10 may be caused by a large movement of a low-dpi
mouse or by a minute movement of a high-dpi mouse.
Which makes pointer acceleration a bit useless and high-dpi devices
essentially unusable.

In an ideal world, we could read the DPI from the device directly and work
with that. In the world we actually live in, we need to compile this list
manually. This patch introduces the database, with the usual match formats and
a single property to be set on a device: MOUSE_DPI

That is either a single value for most mice, or a list of values for mice that
can change resolution at runtime. The exact format is detailed in the hwdb
file.

Note that we're explicitly overshooting the requirements we have for libinput
atm. Frequency could be detected in software and we don't actually use the
list of multiple resolutions (because we can't detect when they change
anyway). However, we might as well collect those values from the get-go,
adding/modifying what will eventually amount to hundreds of entries is a
bit cumbersome.

Note: we rely on the input_id builtin to tag us as mouse first, ordering of
the rules is important.
---
 Makefile.am  |  4 ++-
 rules/70-mouse.hwdb  | 92 
 rules/70-mouse.rules | 15 +
 3 files changed, 110 insertions(+), 1 deletion(-)
 create mode 100644 rules/70-mouse.hwdb
 create mode 100644 rules/70-mouse.rules

diff --git a/Makefile.am b/Makefile.am
index 3f9f3fa..d2b0d02 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3285,6 +3285,7 @@ dist_udevrules_DATA += \
rules/50-udev-default.rules \
rules/60-drm.rules \
rules/60-keyboard.rules \
+   rules/70-mouse.rules \
rules/60-persistent-storage-tape.rules \
rules/60-persistent-serial.rules \
rules/60-persistent-input.rules \
@@ -3311,7 +3312,8 @@ dist_udevhwdb_DATA = \
hwdb/20-acpi-vendor.hwdb \
hwdb/20-OUI.hwdb \
hwdb/20-net-ifname.hwdb \
-   hwdb/60-keyboard.hwdb
+   hwdb/60-keyboard.hwdb \
+   hwdb/70-mouse.hwdb
 
 udevconfdir = $(sysconfdir)/udev
 dist_udevconf_DATA = \
diff --git a/rules/70-mouse.hwdb b/rules/70-mouse.hwdb
new file mode 100644
index 000..75e8755
--- /dev/null
+++ b/rules/70-mouse.hwdb
@@ -0,0 +1,92 @@
+# This file is part of systemd.
+#
+# Database for the DPI setting of mice, trackballs, other pointer devices that
+# cannot be queried directly.
+#
+# The lookup keys are composed in:
+#   60-mouse.rules
+#
+# Note: The format of the "mouse:" prefix match key is a
+# contract between the rules file and the hardware data, it might
+# change in later revisions to support more or better matches, it
+# is not necessarily expected to be a stable ABI.
+#
+# Match string format:
+# mouse::vp:name::
+#
+# Supported subsystems: usb, bluetooth
+# vid/pid as 4-digit hex lowercase vendor/product
+#
+# if vid/pid is unavailable, use
+# mouse:*:name::
+# if name is unavailable, use
+# mouse::vp:*
+#
+# For example, the following 5 matches all match the same mouse:
+# mouse:usb:v17efp6019:name:Lenovo Optical USB Mouse:
+# mouse:usb:*:name:Lenovo Optical USB Mouse:
+# mouse:usb:v17efp6019:*
+# mouse:*:name:Lenovo Optical USB Mouse:
+#
+# DPI settings are specified as
+#MOUSE_DPI=[@]
+#
+# Where  is the resolution in dots per inch, and  the
+# optional sampling frequency in Hz.
+#
+# The value of MOUSE_DPI is:
+# - a single integer for single-resolution mice, e.g.
+#   MOUSE_DPI=800
+#   or, if the frequency is known:
+#   MOUSE_DPI=800@120
+# - a space-separated list of resolutions for multi-resolution mice.
+#   The default resolution must be prefixed by an asterisk, the resultions
+#   in the database must be as shipped by the manufacturer. e.g.
+#   MOUSE_DPI=400 *800 2000
+#
+#   The order of resolutions is as configured by the HW manufacturer or in
+#   ascending order, whichever appropriate.
+#
+#   The frequency must be given to either none or all resolutions. If the
+#   device supports multiple frequencies, the order of items is
+#   MOUSE_DPI=r1@f1 r2@f1 r3@f1 r1@f2 r2@f2 r3@f2
+#
+#   If the default manufacturer-set resolution is unclear, a resolution of
+#   800 or 1000 should be set as default, if available. If neither is
+#   available, choose the "middle" resolution value of those available.
+#
+#   The list may contain a single item which must be marked with an
+#   asterisk.
+#
+# Local changes to the a non-default resolution of the mouse (e.g. through
+# third-party software) must not be entered into this file, use a local
+# hwdb instead.
+#
+# To add local entries, create a new file
+#   /etc/udev/hwdb.d/60-mouse.hwdb
+# and add your rules there. To load the new rules execute (as root):
+#   udevadm hwdb --update
+#   udevadm trigger /dev/input/eventXX
+# where /dev/input/eventXX is the mouse in ques

Re: [systemd-devel] [systemd-commits] src/cryptsetup

[Quentin Lefebvre]

Hi,

On 24/11/2014 19:17, Zbigniew Jędrzejewski-Szmek wrote :

On Mon, Nov 24, 2014 at 07:03:27PM +0100, Quentin Lefebvre wrote:

On 24/11/2014 19:01, Zbigniew Jędrzejewski-Szmek wrote :

On Mon, Nov 24, 2014 at 06:44:25PM +0100, Quentin Lefebvre wrote:

Hi,

I tested your patch and actually it doesn't solve the bug.
For example, if "hash=sha512" is provided in /etc/crypttab, the
first >   if (!streq(arg_hash, "plain"))
is true, and the

+} else if (!key_file)

is not reached.

This is be design. My patch is quite different from your patch,
which I tried to make clear in the description.

If you specify hash=sha512, then you get hash=sha512.


Yes, and this is the problem.
cryptsetup ignores the hash, so that we should obtain hash=NULL for
it to work.

Systemd is not going to work around a bug in a different package.
Specifying a hash in the configuration if you don't want a hash
is an error, please just fix it there.


As I mention it in the bugreport 
(https://bugs.freedesktop.org/show_bug.cgi?id=52630), this is not 
exactly a cryptsetup bug, but rather the intended and documented way it 
works. Please see the "NOTES ON PASSPHRASE PROCESSING FOR PLAIN MODE" 
section, where it is clearly stated that hash processing is only used on 
*passphrases*.


So, I'm afraid commit 
http://cgit.freedesktop.org/systemd/systemd/commit/?id=8a52210c93 
doesn't make the job it should. Actually it doesn't solve a bug that 
definitely seems related to systemd, and it kind of breaks the previous 
logic of the code.


To be clear, when a hash algorithm is provided along with a key file for 
plain mode encryption, systemd-cryptsetup should, IMHO, ignore the hash 
algorithm as cryptsetup does.


Please don't get angry at me for insisting like this. I don't want to 
declare a futile war against anybody. I'm just a systemd user who wants 
the best from the software he uses. And I'm sure you're doing your best 
here.


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

Re: [systemd-devel] [PATCH] hwdb: fix a typo

[Martin Pitt]

Hey Peter,

Peter Hutterer [2014-11-25 20:45 +1000]:
> -#   /etc/udev/hwdb.d/70-keyboad.hwdb
> +#   /etc/udev/hwdb.d/70-keyboard.hwdb

Applied, thanks!

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

Re: [systemd-devel] how to properly control the daemons or run advanced cmds

[Richard Maw]

On Tue, Nov 25, 2014 at 12:09:07AM -0200, Flavio Leitner wrote:
> 
> Hello,
> 
> The Open vSwitch is comprised by two daemons. One is a database and
> another is the switch itself.
> 
> Currently we have the openvswitch.service which start/stop/reload the
> service (both daemons) just fine.
> 
> However, we need to support hot-upgrade which means to stop the
> vswitch daemon first, run a few special commands, reload the db
> daemon and only then start the vswitch daemon.
> 
> I know about creating shell helpers for non-standard commands, but
> since that needs to mess up with the daemons in a particular order,
> I think systemd won't like the above external actions at all.
> 
> Any suggestion on how to handle this with systemd properly?

We've previously had discussion on how to handle hot-upgrade, though I
don't remember enough details to find the message again.

I think the resolution was that the daemon in question must either
re-exec itself, or have NotifyAccess=all set in the systemd unit, and
it must notify systemd of the PID of the new version of the daemon by
sending MAINPID=$NEWPID with sd_notify.

Unless you overload ExecReload to mean hot-upgrade/graceful-reexec then
you'll have to have an external tool instruct the service to upgrade
itself, rather than issuing a `systemctl reload vswitch.service`.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] hwdb: fix a typo

[Peter Hutterer]

---
 hwdb/60-keyboard.hwdb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hwdb/60-keyboard.hwdb b/hwdb/60-keyboard.hwdb
index 533ad5b..46348b9 100644
--- a/hwdb/60-keyboard.hwdb
+++ b/hwdb/60-keyboard.hwdb
@@ -42,7 +42,7 @@
 # an input device use the commonly available tool: evtest(1).
 #
 # To update this file, create a new file
-#   /etc/udev/hwdb.d/70-keyboad.hwdb
+#   /etc/udev/hwdb.d/70-keyboard.hwdb
 # and add your rules there. To load the new rules execute (as root):
 #   udevadm hwdb --update
 #   udevadm trigger /dev/input/eventXX
-- 
2.1.0

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

Re: [systemd-devel] [PATCH] localed: forward xkbcommon errors

[David Herrmann]

Hi

On Tue, Nov 25, 2014 at 10:50 AM, Jan Synacek  wrote:
> David Herrmann  writes:
>> Hi Jan!
>
> Hello!
>
>> On Tue, Nov 25, 2014 at 9:23 AM, Jan Synacek  wrote:
>>> The errors are prefixed with "libxkbcommon", because they are quite
>>> confusing. With the prefix, we at least know where they come from.
>>> ---
>>>  src/locale/localed.c | 15 +++
>>>  1 file changed, 11 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/locale/localed.c b/src/locale/localed.c
>>> index 4e56382..ea54798 100644
>>> --- a/src/locale/localed.c
>>> +++ b/src/locale/localed.c
>>> @@ -1011,10 +1011,16 @@ static int method_set_vc_keyboard(sd_bus *bus, 
>>> sd_bus_message *m, void *userdata
>>>
>>>  #ifdef HAVE_XKBCOMMON
>>>  static void log_xkb(struct xkb_context *ctx, enum xkb_log_level lvl, const 
>>> char *format, va_list args) {
>>> -/* suppress xkb messages for now */
>>> +_cleanup_free_ char *fmt = NULL;
>>> +sd_bus_error *e;
>>> +
>>> +if (asprintf(&fmt, "libxkbcommon: %s", format) < 0)
>>> +(void) log_oom();
>>
>> I think you can safely use:
>>
>> char fmt[LINE_MAX];
>>
>> snprintf(fmt, sizeof(fmt), "libxkbcommon: %s", format);
>> e = xkb_context_get_user_data(ctx);
>> bus_error_setfv(e, SD_BUS_ERROR_INVALID_ARGS, fmt, args);
>>
>> We use LINE_MAX as explicit limit on a lot of these calls (and I think
>> it makes sense to prevent unbound allocations).
>
> I don't understand this. What do you mean by "unbound allocation"? That
> libxkbcommon could, in theory, return a huge format that would exhaust
> all memory?

Yeah. In debug mode, libxkbcommon can generate quite some traffic.
This is not about security, but about performance if we generate like
megabytes of data if a keymap is dumped. But on the other hand, if
someone requests this verbosity, maybe we should forward it.

Btw., why not concatenate all the output from xkbcommon? I think
bus_error_setfv() overwrites any previous error.

>>> +e = xkb_context_get_user_data(ctx);
>>> +bus_error_setfv(e, SD_BUS_ERROR_INVALID_ARGS, fmt, args);
>>>  }
>>>
>>> -static int verify_xkb_rmlvo(const char *model, const char *layout, const 
>>> char *variant, const char *options) {
>>> +static int verify_xkb_rmlvo(const char *model, const char *layout, const 
>>> char *variant, const char *options, sd_bus_error *error) {
>>>  const struct xkb_rule_names rmlvo = {
>>>  .model  = model,
>>>  .layout = layout,
>>> @@ -1033,6 +1039,7 @@ static int verify_xkb_rmlvo(const char *model, const 
>>> char *layout, const char *v
>>>  goto exit;
>>>  }
>>>
>>> +xkb_context_set_user_data(ctx, (void *)error);
>>>  xkb_context_set_log_fn(ctx, log_xkb);
>>>
>>>  km = xkb_keymap_new_from_names(ctx, &rmlvo, 
>>> XKB_KEYMAP_COMPILE_NO_FLAGS);
>>> @@ -1049,7 +1056,7 @@ exit:
>>>  return r;
>>>  }
>>>  #else
>>> -static int verify_xkb_rmlvo(const char *model, const char *layout, const 
>>> char *variant, const char *options) {
>>> +static int verify_xkb_rmlvo(const char *model, const char *layout, const 
>>> char *variant, const char *options, sd_bus_error *error) {
>>>  return 0;
>>>  }
>>>  #endif
>>> @@ -1087,7 +1094,7 @@ static int method_set_x11_keyboard(sd_bus *bus, 
>>> sd_bus_message *m, void *userdat
>>>  (options && !string_is_safe(options)))
>>>  return sd_bus_error_set_errnof(error, -EINVAL, 
>>> "Received invalid keyboard data");
>>>
>>> -r = verify_xkb_rmlvo(model, layout, variant, options);
>>> +r = verify_xkb_rmlvo(model, layout, variant, options, 
>>> error);
>>>  if (r < 0)
>>>  log_warning("Cannot compile XKB keymap for new x11 
>>> keyboard layout ('%s' / '%s' / '%s' / '%s'): %s",
>>>  strempty(model), strempty(layout), 
>>> strempty(variant), strempty(options), strerror(-r));
>>
>> I explicitly ignore errors from verify_xkb_rmlvo() and proceed.
>> libxkbcommon is still not 100% compatible to libxkb (and doesn't
>> intend to be that, I guess). As we write X11 configs here, I just
>> continue with a warning.
>>
>> If you call bus_error_setfv(), then sd-bus will return a method-error
>> to the caller. However, you also send a method-return in
>> method_set_x11_keyboard(). You thus end up with 2 calls.
>
> Hmm, I thought that bus_error_setfv() only fills the error struct. Is
> there any documentation that desribes how the internal bus_* stuff
> works?

Quite simple:

Whenever a callback via sd-bus is called, you get an sd_bus_error
object. If you set an error on it, or if you return "r < 0", then
sd-bus generates an error-reply. Otherwise, it does nothing.

That means, if you call sd_bus_reply_method_return(bus, NULL); like we
do at the bottom of method_set_x11_keyboard(), then you really should
not have set any error before. Furthermore, if you 

Re: [systemd-devel] [PATCH] localed: forward xkbcommon errors

[Jan Synacek]

David Herrmann  writes:
> Hi Jan!

Hello!

> On Tue, Nov 25, 2014 at 9:23 AM, Jan Synacek  wrote:
>> The errors are prefixed with "libxkbcommon", because they are quite
>> confusing. With the prefix, we at least know where they come from.
>> ---
>>  src/locale/localed.c | 15 +++
>>  1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/locale/localed.c b/src/locale/localed.c
>> index 4e56382..ea54798 100644
>> --- a/src/locale/localed.c
>> +++ b/src/locale/localed.c
>> @@ -1011,10 +1011,16 @@ static int method_set_vc_keyboard(sd_bus *bus, 
>> sd_bus_message *m, void *userdata
>>
>>  #ifdef HAVE_XKBCOMMON
>>  static void log_xkb(struct xkb_context *ctx, enum xkb_log_level lvl, const 
>> char *format, va_list args) {
>> -/* suppress xkb messages for now */
>> +_cleanup_free_ char *fmt = NULL;
>> +sd_bus_error *e;
>> +
>> +if (asprintf(&fmt, "libxkbcommon: %s", format) < 0)
>> +(void) log_oom();
>
> I think you can safely use:
>
> char fmt[LINE_MAX];
>
> snprintf(fmt, sizeof(fmt), "libxkbcommon: %s", format);
> e = xkb_context_get_user_data(ctx);
> bus_error_setfv(e, SD_BUS_ERROR_INVALID_ARGS, fmt, args);
>
> We use LINE_MAX as explicit limit on a lot of these calls (and I think
> it makes sense to prevent unbound allocations).

I don't understand this. What do you mean by "unbound allocation"? That
libxkbcommon could, in theory, return a huge format that would exhaust
all memory?

>> +e = xkb_context_get_user_data(ctx);
>> +bus_error_setfv(e, SD_BUS_ERROR_INVALID_ARGS, fmt, args);
>>  }
>>
>> -static int verify_xkb_rmlvo(const char *model, const char *layout, const 
>> char *variant, const char *options) {
>> +static int verify_xkb_rmlvo(const char *model, const char *layout, const 
>> char *variant, const char *options, sd_bus_error *error) {
>>  const struct xkb_rule_names rmlvo = {
>>  .model  = model,
>>  .layout = layout,
>> @@ -1033,6 +1039,7 @@ static int verify_xkb_rmlvo(const char *model, const 
>> char *layout, const char *v
>>  goto exit;
>>  }
>>
>> +xkb_context_set_user_data(ctx, (void *)error);
>>  xkb_context_set_log_fn(ctx, log_xkb);
>>
>>  km = xkb_keymap_new_from_names(ctx, &rmlvo, 
>> XKB_KEYMAP_COMPILE_NO_FLAGS);
>> @@ -1049,7 +1056,7 @@ exit:
>>  return r;
>>  }
>>  #else
>> -static int verify_xkb_rmlvo(const char *model, const char *layout, const 
>> char *variant, const char *options) {
>> +static int verify_xkb_rmlvo(const char *model, const char *layout, const 
>> char *variant, const char *options, sd_bus_error *error) {
>>  return 0;
>>  }
>>  #endif
>> @@ -1087,7 +1094,7 @@ static int method_set_x11_keyboard(sd_bus *bus, 
>> sd_bus_message *m, void *userdat
>>  (options && !string_is_safe(options)))
>>  return sd_bus_error_set_errnof(error, -EINVAL, 
>> "Received invalid keyboard data");
>>
>> -r = verify_xkb_rmlvo(model, layout, variant, options);
>> +r = verify_xkb_rmlvo(model, layout, variant, options, 
>> error);
>>  if (r < 0)
>>  log_warning("Cannot compile XKB keymap for new x11 
>> keyboard layout ('%s' / '%s' / '%s' / '%s'): %s",
>>  strempty(model), strempty(layout), 
>> strempty(variant), strempty(options), strerror(-r));
>
> I explicitly ignore errors from verify_xkb_rmlvo() and proceed.
> libxkbcommon is still not 100% compatible to libxkb (and doesn't
> intend to be that, I guess). As we write X11 configs here, I just
> continue with a warning.
>
> If you call bus_error_setfv(), then sd-bus will return a method-error
> to the caller. However, you also send a method-return in
> method_set_x11_keyboard(). You thus end up with 2 calls.

Hmm, I thought that bus_error_setfv() only fills the error struct. Is
there any documentation that desribes how the internal bus_* stuff
works?

> I'm not sure how to solve this. Furthermore, libxkbcommon can print a
> lot of informational warnings, and we shouldn't use just the last one.
> One idea would be to copy the same logic into localectl. You could
> also specify XKB_LOG_LEVEL and XKB_LOG_VERBOSITY as environment to get
> more information there.
> Yes, this would mean compiling the keymap twice, but it's for the sake
> of debugging output so I think it's fine.

I don't think that would work. You would have the same code on the
client and on the server and that might not be the same xkb
configuration.

> Thanks
> David

-- 
Jan Synacek
Software Engineer, Red Hat


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

Re: [systemd-devel] [PATCH] localed: forward xkbcommon errors

[David Herrmann]

Hi Jan!

On Tue, Nov 25, 2014 at 9:23 AM, Jan Synacek  wrote:
> The errors are prefixed with "libxkbcommon", because they are quite
> confusing. With the prefix, we at least know where they come from.
> ---
>  src/locale/localed.c | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/src/locale/localed.c b/src/locale/localed.c
> index 4e56382..ea54798 100644
> --- a/src/locale/localed.c
> +++ b/src/locale/localed.c
> @@ -1011,10 +1011,16 @@ static int method_set_vc_keyboard(sd_bus *bus, 
> sd_bus_message *m, void *userdata
>
>  #ifdef HAVE_XKBCOMMON
>  static void log_xkb(struct xkb_context *ctx, enum xkb_log_level lvl, const 
> char *format, va_list args) {
> -/* suppress xkb messages for now */
> +_cleanup_free_ char *fmt = NULL;
> +sd_bus_error *e;
> +
> +if (asprintf(&fmt, "libxkbcommon: %s", format) < 0)
> +(void) log_oom();

I think you can safely use:

char fmt[LINE_MAX];

snprintf(fmt, sizeof(fmt), "libxkbcommon: %s", format);
e = xkb_context_get_user_data(ctx);
bus_error_setfv(e, SD_BUS_ERROR_INVALID_ARGS, fmt, args);

We use LINE_MAX as explicit limit on a lot of these calls (and I think
it makes sense to prevent unbound allocations).

> +e = xkb_context_get_user_data(ctx);
> +bus_error_setfv(e, SD_BUS_ERROR_INVALID_ARGS, fmt, args);
>  }
>
> -static int verify_xkb_rmlvo(const char *model, const char *layout, const 
> char *variant, const char *options) {
> +static int verify_xkb_rmlvo(const char *model, const char *layout, const 
> char *variant, const char *options, sd_bus_error *error) {
>  const struct xkb_rule_names rmlvo = {
>  .model  = model,
>  .layout = layout,
> @@ -1033,6 +1039,7 @@ static int verify_xkb_rmlvo(const char *model, const 
> char *layout, const char *v
>  goto exit;
>  }
>
> +xkb_context_set_user_data(ctx, (void *)error);
>  xkb_context_set_log_fn(ctx, log_xkb);
>
>  km = xkb_keymap_new_from_names(ctx, &rmlvo, 
> XKB_KEYMAP_COMPILE_NO_FLAGS);
> @@ -1049,7 +1056,7 @@ exit:
>  return r;
>  }
>  #else
> -static int verify_xkb_rmlvo(const char *model, const char *layout, const 
> char *variant, const char *options) {
> +static int verify_xkb_rmlvo(const char *model, const char *layout, const 
> char *variant, const char *options, sd_bus_error *error) {
>  return 0;
>  }
>  #endif
> @@ -1087,7 +1094,7 @@ static int method_set_x11_keyboard(sd_bus *bus, 
> sd_bus_message *m, void *userdat
>  (options && !string_is_safe(options)))
>  return sd_bus_error_set_errnof(error, -EINVAL, 
> "Received invalid keyboard data");
>
> -r = verify_xkb_rmlvo(model, layout, variant, options);
> +r = verify_xkb_rmlvo(model, layout, variant, options, error);
>  if (r < 0)
>  log_warning("Cannot compile XKB keymap for new x11 
> keyboard layout ('%s' / '%s' / '%s' / '%s'): %s",
>  strempty(model), strempty(layout), 
> strempty(variant), strempty(options), strerror(-r));

I explicitly ignore errors from verify_xkb_rmlvo() and proceed.
libxkbcommon is still not 100% compatible to libxkb (and doesn't
intend to be that, I guess). As we write X11 configs here, I just
continue with a warning.

If you call bus_error_setfv(), then sd-bus will return a method-error
to the caller. However, you also send a method-return in
method_set_x11_keyboard(). You thus end up with 2 calls.

I'm not sure how to solve this. Furthermore, libxkbcommon can print a
lot of informational warnings, and we shouldn't use just the last one.
One idea would be to copy the same logic into localectl. You could
also specify XKB_LOG_LEVEL and XKB_LOG_VERBOSITY as environment to get
more information there.
Yes, this would mean compiling the keymap twice, but it's for the sake
of debugging output so I think it's fine.

Thanks
David

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

[systemd-devel] [PATCH] localed: forward xkbcommon errors

[Jan Synacek]

The errors are prefixed with "libxkbcommon", because they are quite
confusing. With the prefix, we at least know where they come from.
---
 src/locale/localed.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/locale/localed.c b/src/locale/localed.c
index 4e56382..ea54798 100644
--- a/src/locale/localed.c
+++ b/src/locale/localed.c
@@ -1011,10 +1011,16 @@ static int method_set_vc_keyboard(sd_bus *bus, 
sd_bus_message *m, void *userdata
 
 #ifdef HAVE_XKBCOMMON
 static void log_xkb(struct xkb_context *ctx, enum xkb_log_level lvl, const 
char *format, va_list args) {
-/* suppress xkb messages for now */
+_cleanup_free_ char *fmt = NULL;
+sd_bus_error *e;
+
+if (asprintf(&fmt, "libxkbcommon: %s", format) < 0)
+(void) log_oom();
+e = xkb_context_get_user_data(ctx);
+bus_error_setfv(e, SD_BUS_ERROR_INVALID_ARGS, fmt, args);
 }
 
-static int verify_xkb_rmlvo(const char *model, const char *layout, const char 
*variant, const char *options) {
+static int verify_xkb_rmlvo(const char *model, const char *layout, const char 
*variant, const char *options, sd_bus_error *error) {
 const struct xkb_rule_names rmlvo = {
 .model  = model,
 .layout = layout,
@@ -1033,6 +1039,7 @@ static int verify_xkb_rmlvo(const char *model, const char 
*layout, const char *v
 goto exit;
 }
 
+xkb_context_set_user_data(ctx, (void *)error);
 xkb_context_set_log_fn(ctx, log_xkb);
 
 km = xkb_keymap_new_from_names(ctx, &rmlvo, 
XKB_KEYMAP_COMPILE_NO_FLAGS);
@@ -1049,7 +1056,7 @@ exit:
 return r;
 }
 #else
-static int verify_xkb_rmlvo(const char *model, const char *layout, const char 
*variant, const char *options) {
+static int verify_xkb_rmlvo(const char *model, const char *layout, const char 
*variant, const char *options, sd_bus_error *error) {
 return 0;
 }
 #endif
@@ -1087,7 +1094,7 @@ static int method_set_x11_keyboard(sd_bus *bus, 
sd_bus_message *m, void *userdat
 (options && !string_is_safe(options)))
 return sd_bus_error_set_errnof(error, -EINVAL, 
"Received invalid keyboard data");
 
-r = verify_xkb_rmlvo(model, layout, variant, options);
+r = verify_xkb_rmlvo(model, layout, variant, options, error);
 if (r < 0)
 log_warning("Cannot compile XKB keymap for new x11 
keyboard layout ('%s' / '%s' / '%s' / '%s'): %s",
 strempty(model), strempty(layout), 
strempty(variant), strempty(options), strerror(-r));
-- 
1.9.3

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