Re: [systemd-devel] udev firmware loading

[Colin Guthrie]

'Twas brillig, and Richard Hughes at 27/11/13 09:31 did gyre and
gimble: Hi all,

 I've been porting a lot of the gnome-packagekit functionality to
 gnome-software these last few months. One thing that used to work
 well, but I've not seen a bugreport about in *years* is the
 install-package-for-missing-firmware thing.

 IIRC, udev used to call /lib/udev/firmware.sh which used to create a
 file in /var/run/PackageKit/udev with the missing firmware file name.
 I can't see any reference to this in udev-builtin-firmware and
 wondered if this was removed deliberately.

AFAIUI, the kernel loads firmware directly these days.

Unless you explicitly tell it, udev will not even compile the
src/udev/udev-builtin-firmware.c file.

There is also config on the kernel side too, so it really depends on how
things are shipped I guess. That said, there is no mention of
firmware.sh anywhere so not sure that even if you did compile things
with appropriate support whether or not you'd actually have that script run.

 If installing-a-package-for-missing-firmware is something you don't
 want to support any more, that's fine, just let me know and I can
 delete a ton of code in PackageKit and gnome-packagekit. It was a
 little-used feature, but a nice bit of polish that made more hardware
 work out-of-the-box.

Not sure if some other mechanism could be used, but I'm sure people here
could elaborate.

 The always install all firmware files, they are tiny argument works
 for me as well, so don't be afraid of using that defence. :)

WFM ;)

Col

-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited http://www.tribalogic.net/
Open Source:
  Mageia Contributor http://www.mageia.org/
  PulseAudio Hacker http://www.pulseaudio.org/
  Trac Hacker http://trac.edgewall.org/
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] udev firmware loading

[Kay Sievers]

On Wed, Nov 27, 2013 at 10:31 AM, Richard Hughes hughsi...@gmail.com wrote:

 I've been porting a lot of the gnome-packagekit functionality to
 gnome-software these last few months. One thing that used to work
 well, but I've not seen a bugreport about in *years* is the
 install-package-for-missing-firmware thing.

 IIRC, udev used to call /lib/udev/firmware.sh which used to create a
 file in /var/run/PackageKit/udev with the missing firmware file name.
 I can't see any reference to this in udev-builtin-firmware and
 wondered if this was removed deliberately.

It was /run/udev/firmware-missing/*, udev cannot access /var.

All of that is gone and will not come back, udev has no idea about
device firmware and does no longer want to know about it. There is no
way to tell these days what firmware was requested or loaded, it's all
happening in the kernel only.

Unless we want to invent a mechanism to queue up/log that information
in the kernel and communicate that to userspace, you can just delete
all the code that handles this in userspace.

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

Re: [systemd-devel] fstab, rootfs on btrfs

[Kay Sievers]

On Wed, Nov 27, 2013 at 5:16 AM, Chris Murphy li...@colorremedies.com wrote:

 In Fedora 20, by default anaconda sets fs_passno in fstab to 1 for / on 
 btrfs. During offline updates, this is causing systemd-fstab-generator to 
 freak out not finding fsck.btrfs.

 https://bugzilla.redhat.com/show_bug.cgi?id=1034563

Right, it should not set 1 for btrfs.

 For some time I've been suggesting that fstab should use fs_passno 0 for 
 btrfs file systems:
 https://bugzilla.redhat.com/show_bug.cgi?id=862871

See here:
  
http://cgit.freedesktop.org/systemd/systemd/commit/?id=94192cdaf652c9717f15274504ed315126c07a93

 But because of this suggestion by an XFS dev, I'm wondering if that's not a 
 good idea. Or if we should expect some smarter behavior on the part of 
 systemd (now or in the future) when it comes to devices that take a long time 
 to appear?
 http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg29231.html

It makes no sense, to ship a dead program, just to please broken
configs. There should be no fsck for btrfs or any other fs that does
not need a check.

 It doesn't seem to me that for file systems that don't require an fs check, 
 that fstab should indicate it does require an fs check, just to inhibit hissy 
 fits by other processes not liking that the device is missing. But maybe I'm 
 missing something.

Such hacks are plain wrong and not needed for today's systems.

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

Re: [systemd-devel] udev firmware loading

[Tom Gundersen]

On Wed, Nov 27, 2013 at 12:53 PM, Martin Pitt martin.p...@ubuntu.com wrote:
 Kay Sievers [2013-11-27 12:47 +0100]:
 All of that is gone and will not come back, udev has no idea about
 device firmware and does no longer want to know about it. There is no
 way to tell these days what firmware was requested or loaded, it's all
 happening in the kernel only.

 Unless we want to invent a mechanism to queue up/log that information
 in the kernel and communicate that to userspace, you can just delete
 all the code that handles this in userspace.

 Too bad, as all this has already worked in the past. There used to be
 a firmware subsystem uevent for firmware loading, that should at
 least be generated if the kernel cannot load the firmware by itself?
 udev itself doesn't need to know about it any more than it needs to
 know what to do about a new block device, but one could then continue
 to attach services like PackageKit in userspace which can map a
 requested firmware to a package to install.

 This used to be a way to make more exotic hardware (like DVB-T sticks
 or the dreaded Broadcom wifis) just work.

Couldn't the kernel simply log (one of these new fance structured log
messages it now supports) whenever it fails to load a piece of
firmware?

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

Re: [systemd-devel] udev firmware loading

[Kay Sievers]

On Wed, Nov 27, 2013 at 12:53 PM, Martin Pitt martin.p...@ubuntu.com wrote:
 Kay Sievers [2013-11-27 12:47 +0100]:
 All of that is gone and will not come back, udev has no idea about
 device firmware and does no longer want to know about it. There is no
 way to tell these days what firmware was requested or loaded, it's all
 happening in the kernel only.

 Unless we want to invent a mechanism to queue up/log that information
 in the kernel and communicate that to userspace, you can just delete
 all the code that handles this in userspace.

 Too bad, as all this has already worked in the past. There used to be
 a firmware subsystem uevent for firmware loading, that should at
 least be generated if the kernel cannot load the firmware by itself?

No, it shouldn't. The firmware uevents/class was a big mistake, and a
broken concept. It is wrong to create fake devices inside the kernel
just to establish a conceptually broken sunc transaction to userspace
which might lock until a timeout.

Also many driver authors were not be able to handle the concept of
async requests, and their stuff was just too broken for the reality of
the complex udev firmware fake device requester.

 udev itself doesn't need to know about it any more than it needs to
 know what to do about a new block device, but one could then continue
 to attach services like PackageKit in userspace which can map a
 requested firmware to a package to install.

No, udev has no business really in loading firmware, or to carry out
any such information.

 This used to be a way to make more exotic hardware (like DVB-T sticks
 or the dreaded Broadcom wifis) just work.

There was a lot more not working with the broken idea of a
fake-device-firmware-requester, than it ever made working. It should
never come back, just go away. :)

There are tons of drivers which request like 5 file names in a row and
use the on they find, leaving names requested which are not expected
to exists, but all works fine.

There is a lot of hardware which just checks if there is a newer
firmware, but there will never be.

Then there is microcode updates using the firmware loader, requesting
stuff which almost never exist.

If we want/need that information back, it needs to be a new facility, and
udev will not be involved. But I don't see how useful such info would be,
there are a lot of requests today which are fine to never get answered,
and which should not trigger any userspace reaction to it.

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

Re: [systemd-devel] udev firmware loading

[Kay Sievers]

On Wed, Nov 27, 2013 at 1:02 PM, Tom Gundersen t...@jklm.no wrote:

 Couldn't the kernel simply log (one of these new fance structured log
 messages it now supports) whenever it fails to load a piece of
 firmware?

Yes, drivers could explicitly log fatal failures, ones they cannot
recover from without firmware; a default message for ever unfulfilled
firmware request would not make much sense though.

It would need a MESSSAGE_ID= and a few key/value pairs with the file
name, the driver name and such to send out. The journal could get a
service activation feature by message id ...

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

[systemd-devel] Hard-coded /bin/mount in systemd

[Hannes Reinecke]

Hi all,

for some reason systemd has /bin/mount hardcoded in
src/core/mount.c:mount_enter_mounting()

Which is a bit odd, seeing that everyting moved to /usr/bin.
So we always have to do a symlink here, which really is a bit annoying.

Is this by design or a simple left-over?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke   zSeries  Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Hard-coded /bin/mount in systemd

[Mantas Mikulėnas]

On Wed, Nov 27, 2013 at 3:31 PM, Hannes Reinecke h...@suse.de wrote:

 Hi all,

 for some reason systemd has /bin/mount hardcoded in
 src/core/mount.c:mount_enter_mounting()

 Which is a bit odd, seeing that everyting moved to /usr/bin.
 So we always have to do a symlink here, which really is a bit annoying.

 Is this by design or a simple left-over?

If *everything* moved to /usr/bin, then /bin itself has to be a
symlink anyway (as many tools expect and some standards require
specific commands to be in /bin).

-- 
Mantas Mikulėnas graw...@gmail.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Hard-coded /bin/mount in systemd

[Hannes Reinecke]

On 11/27/2013 02:58 PM, Mantas Mikulėnas wrote:
 On Wed, Nov 27, 2013 at 3:31 PM, Hannes Reinecke h...@suse.de wrote:

 Hi all,

 for some reason systemd has /bin/mount hardcoded in
 src/core/mount.c:mount_enter_mounting()

 Which is a bit odd, seeing that everyting moved to /usr/bin.
 So we always have to do a symlink here, which really is a bit annoying.

 Is this by design or a simple left-over?
 
 If *everything* moved to /usr/bin, then /bin itself has to be a
 symlink anyway (as many tools expect and some standards require
 specific commands to be in /bin).
 
Ah. IIRC it was _systemd_ which initiated the move to /usr, so I
found it slightly odd to rely on a location which it has just
obsoleted ...
Or, rather, to have a hard-coded location to start with.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke   zSeries  Storage
h...@suse.de  +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH v2 02/26] dhcp: Add DHCP client initialization

[Patrik Flykt]

Hi,

On Mon, 2013-11-25 at 23:59 +0100, Lennart Poettering wrote:
 On Mon, 25.11.13 09:13, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:
 
  +
  +DHCPClient *sd_dhcp_client_new(void)
  +{
  +DHCPClient *client;
  +
  +client = new0(DHCPClient, 1);
  +if (!client)
  +return NULL;
  +
  +client-state = DHCP_STATE_INIT;
  +client-index = -1;
  +
  +client-req_opts_size = ELEMENTSOF(default_req_opts);
  +
  +client-req_opts = memdup(default_req_opts,
  client-req_opts_size);
 
 Missing OOM check?

Indeed.

  +
  +return client;
  +}
  diff --git a/src/systemd/sd-dhcp-client.h b/src/systemd/sd-dhcp-client.h
  new file mode 100644
  index 000..65caf59
  --- /dev/null
  +++ b/src/systemd/sd-dhcp-client.h
  @@ -0,0 +1,33 @@
  +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
  +
  +#pragma once
 
 If this is supposed to be public API, then it should not use the pragma
 once stuff. That's a gcc'ism, which is fine for internal code, but for
 external code we try hard to stay with C89. Please use #ifdef checks for
 the public headers hence.

Missed that one. I'll add #ifdefs here.

  +#include netinet/in.h
  +
  +typedef struct DHCPClient DHCPClient;
 
 This also needs prefixing. The other public structures like this were
 usually called in the style sd_dhcp_client.

'typedef struct sd_dhcp_client sd_dhcp_client;' is fine here, I suppose?

  +int sd_dhcp_client_set_request_option(DHCPClient *client, const
  uint8_t option);
 
 Hmm, what's a const uint8_t parameter supposed to be? const only
 really makes sense with pointer parameters, but this ins't one...

Approximately a silly typo too late one evening :-)

  +int sd_dhcp_client_set_request_address(DHCPClient *client,
  +   const struct in_addr
  *last_address);
 
 struct in_addr? Didn't you plan to use simple uint32_t for this?
 
 Also, if this is supposed to cover ipv6 one day too, maybe call this
 sd_dhcp_client_set_request_address4() or so?

Internally an uint32_t is fine for IPv4 handling.

In the public API I tried to be consistent with a future IPv6
implementation that probably should use 'struct in6_addr' when passing
IPv6 addresses. IPv6 could of course use some other structure as well,
but I expected people to be most familiar with the already existing
'struct in6_addr'. From that it followed that using 'in_addr' in the
public part for IPv4 would be consistent and logical. I have no problems
using an uint32_t in the public API also if that is wanted.

I really haven't decided on function names for DHCPv6, I was thinking
sd_dhcp6_* would be a good prefix to start with.

  +int sd_dhcp_client_set_index(DHCPClient *client, const int 
  interface_index);
  +
  +DHCPClient *sd_dhcp_client_new(void);
 
 Hmm, for the public APIs we prefer returning newly allocated objects via
 a call-by-ref pointer, and return an error code as return code. i.e.:
 
 int sd_dhcp_client_new(sd_dhcp_client *ret);
 
 or something similar. This leaves more room open to indicate more than
 just OOM errors to the caller.

Ok, I'll do that.

Cheers,

Patrik

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

Re: [systemd-devel] [PATCH v2 00/26] Initial DHCP v4 library implementation

[Patrik Flykt]

Hi,

On Mon, 2013-11-25 at 23:52 +0100, Lennart Poettering wrote:

 Of course, timer callbacks should only be called once, ence ONESHOT
 mode is the default -- unless you change that explicitly.

Event loop code said ONESHOT for timers, and I don't think I managed to
change that.

 If this doesn't solve the issue, I wouldn't be surprised if there was
 still a bug lurking with the sd-event code. Note that I fixed a bug
 like this last week, did you check with currentl git of sd-event if
 the issue persists?

I did the testing with the code from the beginning of last week, then
rebased on top of the latest so the bug may have disappeared already.
I'll take another look.

Cheers,

Patrik



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

Re: [systemd-devel] [PATCH v2 02/26] dhcp: Add DHCP client initialization

[Tom Gundersen]

On Wed, Nov 27, 2013 at 3:13 PM, Patrik Flykt
patrik.fl...@linux.intel.com wrote:

 Hi,

 On Mon, 2013-11-25 at 23:59 +0100, Lennart Poettering wrote:
 On Mon, 25.11.13 09:13, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

  +
  +DHCPClient *sd_dhcp_client_new(void)
  +{
  +DHCPClient *client;
  +
  +client = new0(DHCPClient, 1);
  +if (!client)
  +return NULL;
  +
  +client-state = DHCP_STATE_INIT;
  +client-index = -1;
  +
  +client-req_opts_size = ELEMENTSOF(default_req_opts);
  +
  +client-req_opts = memdup(default_req_opts,
  client-req_opts_size);

 Missing OOM check?

 Indeed.

  +
  +return client;
  +}
  diff --git a/src/systemd/sd-dhcp-client.h b/src/systemd/sd-dhcp-client.h
  new file mode 100644
  index 000..65caf59
  --- /dev/null
  +++ b/src/systemd/sd-dhcp-client.h
  @@ -0,0 +1,33 @@
  +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
  +
  +#pragma once

 If this is supposed to be public API, then it should not use the pragma
 once stuff. That's a gcc'ism, which is fine for internal code, but for
 external code we try hard to stay with C89. Please use #ifdef checks for
 the public headers hence.

 Missed that one. I'll add #ifdefs here.

  +#include netinet/in.h
  +
  +typedef struct DHCPClient DHCPClient;

 This also needs prefixing. The other public structures like this were
 usually called in the style sd_dhcp_client.

 'typedef struct sd_dhcp_client sd_dhcp_client;' is fine here, I suppose?

  +int sd_dhcp_client_set_request_option(DHCPClient *client, const
  uint8_t option);

 Hmm, what's a const uint8_t parameter supposed to be? const only
 really makes sense with pointer parameters, but this ins't one...

 Approximately a silly typo too late one evening :-)

  +int sd_dhcp_client_set_request_address(DHCPClient *client,
  +   const struct in_addr
  *last_address);

 struct in_addr? Didn't you plan to use simple uint32_t for this?

 Also, if this is supposed to cover ipv6 one day too, maybe call this
 sd_dhcp_client_set_request_address4() or so?

 Internally an uint32_t is fine for IPv4 handling.

 In the public API I tried to be consistent with a future IPv6
 implementation that probably should use 'struct in6_addr' when passing
 IPv6 addresses. IPv6 could of course use some other structure as well,
 but I expected people to be most familiar with the already existing
 'struct in6_addr'. From that it followed that using 'in_addr' in the
 public part for IPv4 would be consistent and logical. I have no problems
 using an uint32_t in the public API also if that is wanted.

Using 'struct in_addr' in the public api makes sense to me. Using an
uint32_t wouldn't be a problem, but I'm anyway just using the struct
in networkd, so I don't really see the benefit.

 I really haven't decided on function names for DHCPv6, I was thinking
 sd_dhcp6_* would be a good prefix to start with.

  +int sd_dhcp_client_set_index(DHCPClient *client, const int 
  interface_index);
  +
  +DHCPClient *sd_dhcp_client_new(void);

 Hmm, for the public APIs we prefer returning newly allocated objects via
 a call-by-ref pointer, and return an error code as return code. i.e.:

 int sd_dhcp_client_new(sd_dhcp_client *ret);

 or something similar. This leaves more room open to indicate more than
 just OOM errors to the caller.

 Ok, I'll do that.

 Cheers,

 Patrik

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

Re: [systemd-devel] Hard-coded /bin/mount in systemd

[Colin Guthrie]

'Twas brillig, and Hannes Reinecke at 27/11/13 14:12 did gyre and gimble:
 On 11/27/2013 02:58 PM, Mantas Mikulėnas wrote:
 On Wed, Nov 27, 2013 at 3:31 PM, Hannes Reinecke h...@suse.de wrote:

 Hi all,

 for some reason systemd has /bin/mount hardcoded in
 src/core/mount.c:mount_enter_mounting()

 Which is a bit odd, seeing that everyting moved to /usr/bin.
 So we always have to do a symlink here, which really is a bit annoying.

 Is this by design or a simple left-over?

 If *everything* moved to /usr/bin, then /bin itself has to be a
 symlink anyway (as many tools expect and some standards require
 specific commands to be in /bin).

 Ah. IIRC it was _systemd_ which initiated the move to /usr, so I
 found it slightly odd to rely on a location which it has just
 obsoleted ...
 Or, rather, to have a hard-coded location to start with.

It's essentially just a practicality thing.

Although systemd thinks that a split /usr setup is crack, it *is*
still supported (see the --enable-split-usr option to configure)

Despite being blamed for the sky falling in because systemd printed a
warning message (and that is all!) when it was started when /usr is not
available, it does still try and cope and mount it. If all is well then
there will be no major ill effects

So if it it were not hard coded it would still have to support calling
it both as /bin/mount or /usr/bin/mount via some kind of conditional
(based on the --enable-split-usr configure switch) which will be uglier
in the code. If we know that a properly managed/unified /usr will always
have a /bin - /usr/bin symlink, then this conditional is not needed and
the filesystem just takes care of it for us. This is how it's done just now.

Hope that explains it a bit.

Col

-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited http://www.tribalogic.net/
Open Source:
  Mageia Contributor http://www.mageia.org/
  PulseAudio Hacker http://www.pulseaudio.org/
  Trac Hacker http://trac.edgewall.org/
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Hard-coded /bin/mount in systemd

[Zbigniew Jędrzejewski-Szmek]

On Wed, Nov 27, 2013 at 03:12:02PM +0100, Hannes Reinecke wrote:
 On 11/27/2013 02:58 PM, Mantas Mikulėnas wrote:
  On Wed, Nov 27, 2013 at 3:31 PM, Hannes Reinecke h...@suse.de wrote:
 
  Hi all,
 
  for some reason systemd has /bin/mount hardcoded in
  src/core/mount.c:mount_enter_mounting()
 
  Which is a bit odd, seeing that everyting moved to /usr/bin.
  So we always have to do a symlink here, which really is a bit annoying.
 
  Is this by design or a simple left-over?
  
  If *everything* moved to /usr/bin, then /bin itself has to be a
  symlink anyway (as many tools expect and some standards require
  specific commands to be in /bin).
  
 Ah. IIRC it was _systemd_ which initiated the move to /usr, so I
 found it slightly odd to rely on a location which it has just
 obsoleted ...
 Or, rather, to have a hard-coded location to start with.
It's a pragmatic choice: we know that /bin/mount will work both for
separate-/usr and merged-/usr. We *could* introduce a configure time
check for the location of mount, but there would be no practical
gain. Not having /bin at all is very unlikely, since /bin/sh is
hardcoded in many more places.

C.f. https://bugs.freedesktop.org/show_bug.cgi?id=55248, where gentoo
had kexec in a different place. In that case a practical reason
existed.

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

Re: [systemd-devel] [PATCH v2 05/26] dhcp: Add option appending and parsing

[Patrik Flykt]

Hi,

On Tue, 2013-11-26 at 00:08 +0100, Lennart Poettering wrote:
 On Mon, 25.11.13 09:13, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:
 
  +#include stdint.h
  +
  +#include protocol.h
  +
  +int dhcp_option_append(uint8_t **buf, int *buflen, uint8_t code,
 
  
 
 This is a memory size, right? It should be size_t rather than int, then, no?

size_t it is.

  +static int parse_options(uint8_t *buf, int32_t buflen, int *overload,
 
 ^^ const missing?

Here the idea was to go over the whole buffer and calling the callback
with the proper code, len and value. Start of 'buf', i.e. 'code' in the
function, needs to be advanced to the next option, so its not a const
value.

  + int *message_type, dhcp_option_cb_t cb,
  + void *user_data)
  +{
  +uint8_t *code = buf;
  +uint8_t *len;
  +
  +while (buflen  0) {
  +switch (*code) {
  +case DHCP_OPTION_PAD:
  +buflen -= 1;
  +code++;
  +break;
  +
  +case DHCP_OPTION_END:
  +return 0;
  +
  +case DHCP_OPTION_MESSAGE_TYPE:
  +buflen -= 3;
  +if (buflen  0)
  +return -ENOBUFS;
  +
  +len = code + 1;
  +if (*len != 1)
  +return -EINVAL;
  +
  +if (message_type)
  +*message_type = *(len + 1);
 
 Feels weird that message_type is an int, but you fill an uint8_t into
 it, no?
 
  +if (buflen  0)
  +return -EINVAL;
  +
  +if (cb)
  +cb(*code, *len, len + 1, user_data);
 
 I'd always be careful with callback functions like this, since people
 might alter the objects you rely on while you iterate through things and
 then you are seriously confused... I'd always do iterator-like
 interfaces instead. (Doesn't matter here though, as this is an internal
 interface only...)

True. I'll take this as a warning for future code.

  +int dhcp_option_parse(DHCPMessage *message, int32_t len,
 
 Hmm, an int32_t length? Signed? And shouldn't it be size_t again?

size_t.

  +  dhcp_option_cb_t cb, void *user_data)
  +{
  +int overload = 0;
  +int message_type = -ENOMSG;
  +uint8_t *opt = (uint8_t *)(message + 1);
  +int res;
  +
  +if (message == NULL || len  0)
  +return -EINVAL;
  +
  +len -= sizeof(DHCPMessage);
  +len -= 4;
  +if (len  0)
  +return -EINVAL;
 
 Wouldn't it be better to check len = sizeof(DHCPMessage) + 4 first?
 Sounds more logical than relying on a signed size...

With the size_t changes, yes.

Cheers,

Patrik

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

Re: [systemd-devel] [PATCH v2 19/26] dhcp: Add timeout and main loop support

[Patrik Flykt]

Hi,

On Tue, 2013-11-26 at 05:32 +0100, Zbigniew Jędrzejewski-Szmek wrote:
  +static int client_timeout_resend(sd_event_source *s, uint64_t usec,
  + void *userdata)
  +{
...
  +
  +return 0;
  +
  +error:
  +client_stop(client, err);
  +
  +return 0;
 Return err?

Here I didn't want to spill the error back to the event handling loop
code as the situation is handled internally by stopping the client and
giving the error reason to a registered callback (in patch 25/26) via
'client_stop()'.

Cheers,

Patrik

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

Re: [systemd-devel] [PATCH v2 23/26] dhcp: Process DHCP Ack/Nak message

[Patrik Flykt]

On Tue, 2013-11-26 at 00:26 +0100, Lennart Poettering wrote:
 On Mon, 25.11.13 09:13, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:
 
  -static int client_receive_offer(DHCPClient *client,
  -DHCPPacket *offer, int len)
  +static int client_verify_headers(DHCPClient *client,
  + DHCPPacket *message, int len)
 
 Thhis all looks like size_t len is the right param here, not int.

Ok.

  +lease = new0(DHCPLease, 1);
  +if (!lease)
  +return -ENOBUFS;
 
 OOM should result in ENOMEM, nothing else please. ENOBUFS is something
 else: it indicates some local limit to be hit, not a global memory
 shortage.

Forgotten from the previous fixing, sorry!

Cheers,

Patrik


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

Re: [systemd-devel] [PATCH v2 12/26] dhcp: Add function for sending a raw packet

[Patrik Flykt]

On Tue, 2013-11-26 at 00:10 +0100, Lennart Poettering wrote:
 On Mon, 25.11.13 09:13, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:
 
  +int dhcp_network_send_raw_packet(int index, void *packet, int len);
 
 Should be const void* packet, no?
 
 And size_t len, no?

Indeed.

Patrik


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

Re: [systemd-devel] [PATCH v2 05/26] dhcp: Add option appending and parsing

[Zbigniew Jędrzejewski-Szmek]

On Wed, Nov 27, 2013 at 04:51:42PM +0200, Patrik Flykt wrote:
 On Tue, 2013-11-26 at 00:08 +0100, Lennart Poettering wrote:
  On Mon, 25.11.13 09:13, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:
   +static int parse_options(uint8_t *buf, int32_t buflen, int *overload,
  
  ^^ const missing?
 
 Here the idea was to go over the whole buffer and calling the callback
 with the proper code, len and value. Start of 'buf', i.e. 'code' in the
 function, needs to be advanced to the next option, so its not a const
 value.
It's the uint8_t's that are supposed to be const, not the pointer.
I.e. 'const uint8_t *buf', not 'uint8_t* const buf'.

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

Re: [systemd-devel] [PATCH v2 19/26] dhcp: Add timeout and main loop support

[Zbigniew Jędrzejewski-Szmek]

On Wed, Nov 27, 2013 at 04:57:19PM +0200, Patrik Flykt wrote:
 
   Hi,
 
 On Tue, 2013-11-26 at 05:32 +0100, Zbigniew Jędrzejewski-Szmek wrote:
   +static int client_timeout_resend(sd_event_source *s, uint64_t usec,
   + void *userdata)
   +{
 ...
   +
   +return 0;
   +
   +error:
   +client_stop(client, err);
   +
   +return 0;
  Return err?
 
 Here I didn't want to spill the error back to the event handling loop
 code as the situation is handled internally by stopping the client and
 giving the error reason to a registered callback (in patch 25/26) via
 'client_stop()'.
Oh, OK. So maybe add a comment, this will help avoid misunderstandings
in the future :)

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

Re: [systemd-devel] [PATCH v2 20/26] dhcp: Handle received DHCP Offer message

[Patrik Flykt]

Hi,

On Tue, 2013-11-26 at 00:23 +0100, Lennart Poettering wrote:
 On Mon, 25.11.13 09:13, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:
 
  +static int client_receive_raw_message(sd_event_source *s, int fd,
  +  uint32_t revents, void *userdata)
  +{
  +DHCPClient *client = userdata;
  +int len, buflen;
  +uint8_t *buf;
  +uint8_t tmp;
  +DHCPPacket *message;
  +
  +buflen = sizeof(DHCPPacket) + DHCP_CLIENT_MIN_OPTIONS_SIZE;
  +buf = malloc0(buflen);
  +if (!buf) {
  +read(fd, tmp, 1);
  +return 0;
  +}
 
 Hmm, how long is this message? Given that you don't keep the memory
 around, why not just allocate this on the stack? Either with alloca, or
 even directly as an uint8_t array? Given that this seems to be
 relatively short, this should not be an issue and is one error source
 less...

This one would be the minimum IPv4 packet size, 576 bytes. Should I go
with alloca() here?

  +
  +len = read(fd, buf, buflen);
  +if (len  0)
  +goto error;
  +
  +message = (DHCPPacket *)buf;
  +
  +switch (client-state) {
  +case DHCP_STATE_SELECTING:
  +
  +if (client_receive_offer(client, message, len) = 0) {
  +
  +close(client-fd);
  +client-fd = -1;
  +client-receive_message =
  +
  sd_event_source_unref(client-receive_message);
 
 
 You should unref the source before closing the fd, as this internally
 still references the fd.

Ok.

  -int dhcp_network_send_raw_packet(int index, void *packet, int len)
  +int dhcp_network_send_raw_socket(int s, const union sockaddr_union *link,
  + void *packet, int len)
 
 Hmm, what's the story regarding blocking here? What happens if the
 socket is full? This call will block then, which sucks. Also, if you are
 not using SOCK_NONBLOCK on these sockets, epoll behaviour can be weird,
 as the socket layer takes the freedom to wake you up sometimes without
 any packet to read. (We had the issue in Avahi).
 
 It sounds to me as if you should set SOCK_NONBLOCK and then set a socket
 send buffer (SO_SNDBUF) as large as useful, and simply consider it a
 real error if you the ever get EAGAIN on sending...

I'll fix...

Patrik


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

[systemd-devel] [PATCH] networkd: Initialize variable to NULL

[Patrik Flykt]

If any number of arguments are given, _cleanup_manager_free_ is used
with unitialized memory causing a crash.
---
 src/network/networkd.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/network/networkd.c b/src/network/networkd.c
index 1d43361..360afba 100644
--- a/src/network/networkd.c
+++ b/src/network/networkd.c
@@ -25,7 +25,7 @@
 #include networkd.h
 
 int main(int argc, char *argv[]) {
-_cleanup_manager_free_ Manager *m;
+_cleanup_manager_free_ Manager *m = NULL;
 int r;
 
 log_set_target(LOG_TARGET_AUTO);
-- 
1.7.10.4

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

Re: [systemd-devel] Hard-coded /bin/mount in systemd

[Kay Sievers]

On Wed, Nov 27, 2013 at 3:12 PM, Hannes Reinecke h...@suse.de wrote:
 On 11/27/2013 02:58 PM, Mantas Mikulėnas wrote:
 On Wed, Nov 27, 2013 at 3:31 PM, Hannes Reinecke h...@suse.de wrote:
 for some reason systemd has /bin/mount hardcoded in
 src/core/mount.c:mount_enter_mounting()

 Which is a bit odd, seeing that everyting moved to /usr/bin.
 So we always have to do a symlink here, which really is a bit annoying.

 Is this by design or a simple left-over?

 If *everything* moved to /usr/bin, then /bin itself has to be a
 symlink anyway (as many tools expect and some standards require
 specific commands to be in /bin).

 Ah. IIRC it was _systemd_ which initiated the move to /usr, so I
 found it slightly odd to rely on a location which it has just
 obsoleted ...

It's not obsoleted, it's still part of the legacy API, which will not
go away anytime soon. In a proper system with only one /usr, the
location does not matter, everything works the same.

 Or, rather, to have a hard-coded location to start with.

We support exactly two configurations:
the (conceptually pointless and confusing) legacy split-/user with
/bin/mount, and the one proper single /usr, where /bin must be a
symlink to /usr/bin.

Nothing else is interesting to support, matters or makes sense in any
way. In both supported configurations /bin/mount works just fine.

In the very long run, we will get rid of the split-/usr support and at
that point just hard-code things like /usr/bin/mount; there is no need
really for a configuration switch here.

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

Re: [systemd-devel] [PATCH] networkd: Initialize variable to NULL

[Tom Gundersen]

On Wed, Nov 27, 2013 at 4:28 PM, Patrik Flykt
patrik.fl...@linux.intel.com wrote:
 If any number of arguments are given, _cleanup_manager_free_ is used
 with unitialized memory causing a crash.
 ---
  src/network/networkd.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/src/network/networkd.c b/src/network/networkd.c
 index 1d43361..360afba 100644
 --- a/src/network/networkd.c
 +++ b/src/network/networkd.c
 @@ -25,7 +25,7 @@
  #include networkd.h

  int main(int argc, char *argv[]) {
 -_cleanup_manager_free_ Manager *m;
 +_cleanup_manager_free_ Manager *m = NULL;
  int r;

  log_set_target(LOG_TARGET_AUTO);
 --
 1.7.10.4


Thanks! Applied.

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

Re: [systemd-devel] [PATCH] core/socket: we only want one event on standard sockets

[Shawn Landden]

On Tue, Nov 26, 2013 at 7:57 PM, David Timothy Strauss
da...@davidstrauss.net wrote:
 On Wed, Nov 27, 2013 at 7:57 AM, Shawn Landden sh...@churchofgit.com wrote:
 Are you sure applications can handle the extra file descriptor of
 passing both the sockfd
 and the acceptfd in this case? I don't see why they wouldn't just do
 the accept() themselves?

 Can you explain what you mean here, and how it differs from the
 existing MaxConnections.

 Actually, MaxConnections was exactly what I was thinking of. I agree
 with you now on Distribute=true only being useful with Accept=false.
The current code I have has Distribute take a number, but it certainly
would be possible to use
the MaxConnections number---however I don't do that as connections
isn't quite the
right word, and we can't easily change it because it is part of the API.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 3/7] libsystemd-bus: add kdbus support for sd_bus_list_names()

[Lukasz Skalski]

My 2 cents inline.

On 11/15/2013 07:32 PM, Daniel Mack wrote:

kdbus will tell us the minimum buffer size it needs in case the default
8kb buffer doesn't suffice.
---
  src/libsystemd-bus/bus-control.c | 100 ++-
  1 file changed, 68 insertions(+), 32 deletions(-)

diff --git a/src/libsystemd-bus/bus-control.c b/src/libsystemd-bus/bus-control.c
index 5c9e746..562513b 100644
--- a/src/libsystemd-bus/bus-control.c
+++ b/src/libsystemd-bus/bus-control.c
@@ -180,43 +180,79 @@ _public_ int sd_bus_list_names(sd_bus *bus, char ***l) {
  if (bus_pid_changed(bus))
  return -ECHILD;

-r = sd_bus_call_method(
-bus,
-org.freedesktop.DBus,
-/,
-org.freedesktop.DBus,
-ListNames,
-NULL,
-reply1,
-NULL);
-if (r  0)
-return r;
+if (bus-is_kernel) {
+_cleanup_free_ struct kdbus_cmd_names *names = NULL;
+struct kdbus_cmd_name *name;
+size_t size;
+
+/* assume 8k size first. If that doesn't suffice, kdbus will 
tell us
+ * how big the buffer needs to be.  */
+size = 8192;
+
+retry:
+names = realloc(names, size);
+if (!names)
+return log_oom();
+
+names-size = size;


+ names-flags=KDBUS_NAME_LIST_UNIQUE_NAMES;

ListNames should returns a list of all (both well-known and unique 
names) currently-owned names on the bus.



+
+r = ioctl(sd_bus_get_fd(bus), KDBUS_CMD_NAME_LIST, names);
+if (r  0) {
+if (errno == ENOBUFS  size != names-size) {
+size = names-size;
+goto retry;
+}

-r = sd_bus_call_method(
-bus,
-org.freedesktop.DBus,
-/,
-org.freedesktop.DBus,
-ListActivatableNames,
-NULL,
-reply2,
-NULL);
-if (r  0)
-return r;
+return -errno;
+}

-r = bus_message_read_strv_extend(reply1, x);
-if (r  0) {
-strv_free(x);
-return r;
-}
+KDBUS_PART_FOREACH(name, names, names) {
+r = strv_extend(x, name-name);
+if (r  0)
+return log_oom();
+}

-r = bus_message_read_strv_extend(reply2, x);
-if (r  0) {
-strv_free(x);
-return r;
+*l = x;
+} else {
+r = sd_bus_call_method(
+bus,
+org.freedesktop.DBus,
+/,
+org.freedesktop.DBus,
+ListNames,
+NULL,
+reply1,
+NULL);
+if (r  0)
+return r;
+
+r = sd_bus_call_method(
+bus,
+org.freedesktop.DBus,
+/,
+org.freedesktop.DBus,
+ListActivatableNames,
+NULL,
+reply2,
+NULL);
+if (r  0)
+return r;
+
+r = bus_message_read_strv_extend(reply1, x);
+if (r  0) {
+strv_free(x);
+return r;
+}
+
+r = bus_message_read_strv_extend(reply2, x);
+if (r  0) {
+strv_free(x);
+return r;
+}
+
+*l = strv_uniq(x);
  }

-*l = strv_uniq(x);
  return 0;
  }




--
Lukasz Skalski
Samsung RD Institute Poland
Samsung Electronics
l.skal...@partner.samsung.com

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

Re: [systemd-devel] udev firmware loading

[Matthias Clasen]

On Wed, 2013-11-27 at 13:11 +0100, Kay Sievers wrote:

 It would need a MESSSAGE_ID= and a few key/value pairs with the file
 name, the driver name and such to send out. The journal could get a
 service activation feature by message id ...

Where does that stand, btw ? Hasn't activate-by-message-id been on the
roadmap for a while ? That would let us do some neat things and make the
journal much more useful.

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

Re: [systemd-devel] [PATCH 3/7] libsystemd-bus: add kdbus support for sd_bus_list_names()

[Daniel Mack]

On 11/27/2013 04:59 PM, Lukasz Skalski wrote:
 My 2 cents inline.
 
 On 11/15/2013 07:32 PM, Daniel Mack wrote:
 kdbus will tell us the minimum buffer size it needs in case the default
 8kb buffer doesn't suffice.
 ---
   src/libsystemd-bus/bus-control.c | 100 
 ++-
   1 file changed, 68 insertions(+), 32 deletions(-)

 diff --git a/src/libsystemd-bus/bus-control.c 
 b/src/libsystemd-bus/bus-control.c
 index 5c9e746..562513b 100644
 --- a/src/libsystemd-bus/bus-control.c
 +++ b/src/libsystemd-bus/bus-control.c
 @@ -180,43 +180,79 @@ _public_ int sd_bus_list_names(sd_bus *bus, char ***l) 
 {
   if (bus_pid_changed(bus))
   return -ECHILD;

 -r = sd_bus_call_method(
 -bus,
 -org.freedesktop.DBus,
 -/,
 -org.freedesktop.DBus,
 -ListNames,
 -NULL,
 -reply1,
 -NULL);
 -if (r  0)
 -return r;
 +if (bus-is_kernel) {
 +_cleanup_free_ struct kdbus_cmd_names *names = NULL;
 +struct kdbus_cmd_name *name;
 +size_t size;
 +
 +/* assume 8k size first. If that doesn't suffice, kdbus 
 will tell us
 + * how big the buffer needs to be.  */
 +size = 8192;
 +
 +retry:
 +names = realloc(names, size);
 +if (!names)
 +return log_oom();
 +
 +names-size = size;
 
 + names-flags=KDBUS_NAME_LIST_UNIQUE_NAMES;
 
 ListNames should returns a list of all (both well-known and unique 
 names) currently-owned names on the bus.

Jup, I added the KDBUS_NAME_LIST_UNIQUE_NAMES kdbus feature *after* I
sent out the first version of this patch. It's already fixed in my local
tree :)


Thanks,
Daniel

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

Re: [systemd-devel] udev firmware loading

[Kay Sievers]

On Wed, Nov 27, 2013 at 5:04 PM, Matthias Clasen mcla...@redhat.com wrote:
 On Wed, 2013-11-27 at 13:11 +0100, Kay Sievers wrote:

 It would need a MESSSAGE_ID= and a few key/value pairs with the file
 name, the driver name and such to send out. The journal could get a
 service activation feature by message id ...

 Where does that stand, btw ? Hasn't activate-by-message-id been on the
 roadmap for a while ? That would let us do some neat things and make the
 journal much more useful.

There are no news or anything I know of which would be in progress.
Something just needs to start doing/using that, I guess.

In general, there is a lot of interest in that area from groups
working on logging, and enterprise storage.

Also IBM mainframe people, who are for historic reasons pretty well
organized and a lot less chaotic in that area than the Linux average;
they wanted to look into it with their logging and error handling.

It's just as always I guess: someone rally interested in that facility
needs to start working on it. :) The infrastructure in the kernel and
userspace is all in place, there is not much missing.

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

Re: [systemd-devel] Hard-coded /bin/mount in systemd

[Karel Zak]

On Wed, Nov 27, 2013 at 04:41:27PM +0100, Kay Sievers wrote:
 We support exactly two configurations:
 the (conceptually pointless and confusing) legacy split-/user with
 /bin/mount, and the one proper single /usr, where /bin must be a
 symlink to /usr/bin.

BTW, it's not about systemd only. The same is valid for 
/sbin/{mount,fsck,mkfs}.fs helpers, agetty where we call /bin/login,
etc. etc.

 Nothing else is interesting to support, matters or makes sense in any
 way. In both supported configurations /bin/mount works just fine.
 
 In the very long run, we will get rid of the split-/usr support and at
 that point just hard-code things like /usr/bin/mount; there is no need
 really for a configuration switch here.

Yep.

Karel

-- 
 Karel Zak  k...@redhat.com
 http://karelzak.blogspot.com
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] util: don't consider trailing whitespaces as an empty string in split_quoted

[Lukas Nykryn]

---
 src/shared/util.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/shared/util.c b/src/shared/util.c
index 3a4d196..c68ab09 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -383,7 +383,9 @@ char *split_quoted(const char *c, size_t *l, char **state) {
 
 current += strspn(current, WHITESPACE);
 
-if (*current == '\'') {
+if (*current == 0)
+return NULL;
+else if (*current == '\'') {
 current ++;
 
 for (e = current; *e; e++) {
-- 
1.8.3.1

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

Re: [systemd-devel] [PATCH] util: don't consider trailing whitespaces as an empty string in split_quoted

[Tom Gundersen]

On Wed, Nov 27, 2013 at 6:00 PM, Lukas Nykryn lnyk...@redhat.com wrote:
 ---
  src/shared/util.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

 diff --git a/src/shared/util.c b/src/shared/util.c
 index 3a4d196..c68ab09 100644
 --- a/src/shared/util.c
 +++ b/src/shared/util.c
 @@ -383,7 +383,9 @@ char *split_quoted(const char *c, size_t *l, char 
 **state) {

  current += strspn(current, WHITESPACE);

 -if (*current == '\'') {
 +if (*current == 0)
 +return NULL;
 +else if (*current == '\'') {
  current ++;

  for (e = current; *e; e++) {
 --
 1.8.3.1

Dave,

Is this the proper fix to the /proc/cmdline bug you told me about some
time ago? What happened to that in the end?

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

Re: [systemd-devel] [PATCH] util: don't consider trailing whitespaces as an empty string in split_quoted

[Dave Reisner]

On Wed, Nov 27, 2013 at 06:45:06PM +0100, Tom Gundersen wrote:
 On Wed, Nov 27, 2013 at 6:00 PM, Lukas Nykryn lnyk...@redhat.com wrote:
  ---
   src/shared/util.c | 4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)
 
  diff --git a/src/shared/util.c b/src/shared/util.c
  index 3a4d196..c68ab09 100644
  --- a/src/shared/util.c
  +++ b/src/shared/util.c
  @@ -383,7 +383,9 @@ char *split_quoted(const char *c, size_t *l, char 
  **state) {
 
   current += strspn(current, WHITESPACE);
 
  -if (*current == '\'') {
  +if (*current == 0)
  +return NULL;
  +else if (*current == '\'') {
   current ++;
 
   for (e = current; *e; e++) {
  --
  1.8.3.1
 
 Dave,
 
 Is this the proper fix to the /proc/cmdline bug you told me about some
 time ago? What happened to that in the end?
 
 -t

Oh, interesting... Yeah, this looks like it would. I suppose da66338e1
could be reverted if this is merged.

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

[systemd-devel] [RFC 00/12] Bugfixes for CONFIG_VT=n

[David Herrmann]

Hi

So booting without VTs is still a horrible experience on desktop-linux. If
anyone tried, they were presumably running away in tears. This series is a new
attempt to fix that.

The first 4 patches are just small fixes/helpers. Patch 5 to 8 introduce sd-gfx
and patch 9-11 add tests/examples for sd-gfx. The final patch adds a first user
of sd-gfx to replace the kernel-internal terminal-emulator. The commit-msgs
should explain each change at length so I won't repeat it here.

To anyone interested in testing: If you apply this series, you can run
./test-kbd to test keyboard handling, ./test-gfx to test graphics rendering
and ./systemd-consoled to run the terminal-emulator. Note that you must run
all these from *within* an existing logind-session. That usually means from a
TEXT-VT. Also note that all these programs currently ignore the underlying VT
entirely (in case you run with CONFIG_VT=y). This has the side-effect, that all
keystrokes go to both, the VT and the application. This will obviously be
changed in later versions, but I wanted to avoid any VT calls in these helpers.
I currently work on logind to fix that.

I had to strip the font-patches due to ML size-constraints. The full series is
always available on the-often-rebased-console-branch at:
  http://cgit.freedesktop.org/~dvdhrm/systemd/log/?h=console

Notes on where this is going, and *why*, below..

Happy Testing!
David


What is wrong with CONFIG_VT?
=

The VT layer in the kernel implements a rather complex keyboard layer (similar
to XKB, but less powerful), implements hard-coded device hotplug filters, input
handling, graphics-pipeline handling, font-rendering, terminal-emulation,
cursor handling and more. All this is used to render a basic text-UI *in the
kernel*!
However, moving UI handling into the kernel has a lot of disadvantages (the
same reason why user-space exists.. look it up). For completeness, some rather
obvious reasons are:
 - non-swappable memory (even if you have no swap-partition, user-space apps
   can avoid loading all fonts/keymaps into memory, which kernel-space cannot)
 - security critical (any bug in the UI layer may allow *kernel-level*
   privileges to attackers)
 - stability (user-space apps can be easily restarted on failure; in
   kernel-space, all kinds of memory may get overwritten..)
 - code-duplication (we already have all this code in user-space, why copy it
   into kernel-space?)
 - simplicity (we wanna keep the kernel simple and slim, which prevents adding
   more complex features like fully internationalized fonts and keymaps, which
   are *required* for emergency consoles with given keyboards)
 - configurability/controllability (ever tried assigning more/less CPU-time
   to a given VT?)
 - ...

So what reasons exist to have a UI layer in kernel-space? I only found 3
reasons, if someone knows more, let me know. I highly doubt there's more..
 1) Early boot debugging
 2) Panic/Oops screen
 3) kdb kernel debugging
For 1) and 2) we have an experimental fblog/drmlog kernel module which just
prints the kernel log to all screens. This only requires ASCII glyphs and raw
access to DRM FBs. All the VT cruft can be dropped..
For 3): You're welcome to enable CONFIG_VT if you need a terminal-emulator for
kernel-debugging. My experience is that a serial-console is far more helpful and
reliable and avoids calling DRM modesetting in atomic-context (ugh? This fact
really makes me doubt anyone uses kdb.. Use kgdb!).


Long-term plan
==

So the long-term plan for this series is to set CONFIG_VT=n. This actually
already works and you can boot your machine and start sessions like kmscon.
Even Xorg works if you apply a one-line patch. However, without VTs, legacy
sessions like Xorg will run in exclusive mode. To allow session switching, you
need systemd-logind. But even session-switching is already implemented. Though,
no real application uses that, yet (except for weston and some example code).

So most of the work is to fix all these applications to support VT-less mode.
This is already ongoing and will work soon (hopefully..).

So what does this series implement?
Well, if you run without VTs, you lack any kind of fallback console or fallback
login-screen. To guarantee that even without VTs there's a system-console and
login-screen, this series adds the most basic implementations of those to
systemd. This will *never* turn into a full-blown full-featured console. No
eye-candy, no useless features, NO BACKGROUND IMAGES!!!
The idea is to provide a fallback in systemd. If you want a fancy console with
more features, install kmscon and it will simply replace the systemd-fallback.
However, that means more memory-footprint, more CPU-requirements and a lot more
dependencies.

sd-gfx is a helper library in systemd to unify all the graphics, screen and font
handling. All following applications will use it, including:
 (daemon-names are subject to change!)
 - systemd-consoled: A systemd console 

[systemd-devel] [RFC 03/12] bus: add two new bus_*_map_*_properties() helpers

[David Herrmann]

This splits the core of bus_map_all_properties() out into a new helper
called bus_message_map_all_properties(). Instead of sending a blocking
dbus call, this helper takes the response message as argument and parses
it. So the normal use-case is to send an async GetAll() request and once
you get the response, pass it to the helper to map the properties.

The existing bus_map_all_properties() helper is now just a small wrapper
around sd_bus_call_method() and the new helper.

Furthermore, a second helper is added which parses PropertiesChanged
dbus signals. Whenever you get such a signal, you can pass the message to
a new helper called bus_message_map_properties_changed(). It parses the
PropertiesChanged payload for changed properties and maps them. In case
the payload only contains invalidation-requests and no new values, this
functions returns the number of successfully mapped properties that were
invalidated. Thus, if it returns 0, you should send a GetAll() or Get()
request.

Both helpers allow very convenient handling of dbus-properties in a
non-blocking fashion. This is required for graphics-applications and other
programs that need low-latency responses. In all other cases, the existing
blocking bus_map_all_properties() helper should be enough.
---
 src/libsystemd-bus/bus-util.c | 87 ++-
 src/libsystemd-bus/bus-util.h |  6 +++
 2 files changed, 68 insertions(+), 25 deletions(-)

diff --git a/src/libsystemd-bus/bus-util.c b/src/libsystemd-bus/bus-util.c
index 2daf8c1..0e520c1 100644
--- a/src/libsystemd-bus/bus-util.c
+++ b/src/libsystemd-bus/bus-util.c
@@ -838,31 +838,12 @@ static int map_basic(sd_bus *bus, const char *member, 
sd_bus_message *m, sd_bus_
 return r;
 }
 
-int bus_map_all_properties(sd_bus *bus,
-   const char *destination,
-   const char *path,
-   const struct bus_properties_map *map,
-   void *userdata) {
-_cleanup_bus_message_unref_ sd_bus_message *m = NULL;
+int bus_message_map_all_properties(sd_bus_message *m,
+   const struct bus_properties_map *map,
+   void *userdata) {
 _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
 int r;
 
-assert(bus);
-assert(destination);
-assert(path);
-assert(map);
-
-r = sd_bus_call_method( bus,
-destination,
-path,
-org.freedesktop.DBus.Properties,
-GetAll,
-error,
-m,
-s, );
-if (r  0)
-return r;
-
 r = sd_bus_message_enter_container(m, SD_BUS_TYPE_ARRAY, {sv});
 if (r  0)
 return r;
@@ -895,9 +876,9 @@ int bus_map_all_properties(sd_bus *bus,
 
 v = (uint8_t *)userdata + prop-offset;
 if (map[i].set)
-r = prop-set(bus, member, m, error, v);
+r = prop-set(m-bus, member, m, error, v);
 else
-r = map_basic(bus, member, m, error, v);
+r = map_basic(m-bus, member, m, error, v);
 
 r = sd_bus_message_exit_container(m);
 if (r  0)
@@ -913,7 +894,63 @@ int bus_map_all_properties(sd_bus *bus,
 return r;
 }
 
-return r;
+return sd_bus_message_exit_container(m);
+}
+
+int bus_map_all_properties(sd_bus *bus,
+   const char *destination,
+   const char *path,
+   const struct bus_properties_map *map,
+   void *userdata) {
+_cleanup_bus_message_unref_ sd_bus_message *m = NULL;
+_cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
+int r;
+
+assert(bus);
+assert(destination);
+assert(path);
+assert(map);
+
+r = sd_bus_call_method( bus,
+destination,
+path,
+org.freedesktop.DBus.Properties,
+GetAll,
+error,
+m,
+s, );
+if (r  0)
+return r;
+
+return bus_message_map_all_properties(m, map, userdata);
+}
+
+int bus_message_map_properties_changed(sd_bus_message *m,
+   const struct bus_properties_map *map,
+   void *userdata) {
+const char *member;
+int r, invalidated, i;
+
+/* skip interface, but allow callers to do that themselves */
+sd_bus_message_skip(m, s);
+
+r = 

[systemd-devel] [RFC 05/12] gfx: add sd-gfx library with unifont section

[David Herrmann]

If we want to interact with a user in the initrd, during
emergency-situations, in single-user mode, or in any other rather limited
situation, we currently rely on the kernel to do input and graphics access
for us. More precisely, we rely on the VT layer to do keyboard parsing and
font rendering. Or in other words: The *kernel* provides our UI!

This is bad for the same reasons we don't put any other UI into the
kernel. However, there are a few reasons to keep a minimal UI in the
kernel:
 1) show panic-screen with kernel oops message
 2) show log-screen during early boot
 3) allow kernel-debugging via kdb
While people using kdb are encouraged to keep VTs, for 1) and 2) there is
a replacement via fblog/drmlog.

So we run out of reasons to keep a UI in the kernel. However, to allow
moving the UI handling to userspace, we need a bunch of helpers:
 - keyboard handling: convert keycodes into keysyms and modifiers/..
 - modesetting: program the gfx pipeline and provide a rendering
infrastructure
 - fonts: to render text, we need some basic fonts
 - hotplugging: device detection and assignment during runtime
(Note that all these are implemented (often quite rudimentary) in the
kernel to allow a basic UI.)

This patch introduces sd-gfx, a systemd-internal library dealing with all
these things. Note that it is designed to be exported some day, but for
now we keep it internal.
While the library itself will be kept small and almost self-contained, it
is designed to be extensible and can be used in rather complex graphics
applications. For systemd, we plan to add a basic emergency-console, an
initrd password-query, kernel-log-screen and a fallback login-screen.
These allow booting without CONFIG_VT and provide a basic system for
seats without VTs (either CONFIT_VT=n or seats != seat0).

As a first step, we add the required header+build-chain and add the
font-handling. To avoid heavy font-pipelines in systemd, we only provide
a statically-sized fallback-font based on GNU-Unifont. It contains glyphs
for the *whole* Base-Multilingual-Plane of Unicode and thus allows
internationalized text.

The make-unifont.py script is used by the make update-unifont custom
target to regenerate the unifont files. As this can take quite some time,
we check the result into git so only maintainers need to run this.

The binary file contains all glyphs in a compressed 1-bit-per-pixel format
for all 8x16 and 16x16 glyphs. It is linked directly into libsystemd-gfx
via the *.bin makefile target. Binary size is 2.1MB, but thanks to paging,
the kernel only loads required pages into memory. A ASCII-only screen thus
only needs 40k VIRT mem.
---
Hi

I removed the unifont-data from this patch so this will not apply cleanly.
However, the ML would reject the huge patch otherwise. If anyone is interested
in the raw patch, the series is available on:
  http://cgit.freedesktop.org/~dvdhrm/systemd/log/?h=console

Thanks
David

 Makefile.am  |31 +
 configure.ac |10 +
 make-unifont.py  |   138 +
 src/libsystemd-gfx/.gitignore| 1 +
 src/libsystemd-gfx/Makefile  | 1 +
 src/libsystemd-gfx/gfx-unifont.c |   273 +
 src/libsystemd-gfx/unifont.bin   |   Bin 0 - 2162688 bytes
 src/libsystemd-gfx/unifont.hex   | 63488 +
 src/systemd/sd-gfx.h |65 +
 9 files changed, 64007 insertions(+)
 create mode 100755 make-unifont.py
 create mode 100644 src/libsystemd-gfx/.gitignore
 create mode 12 src/libsystemd-gfx/Makefile
 create mode 100644 src/libsystemd-gfx/gfx-unifont.c
 create mode 100644 src/libsystemd-gfx/unifont.bin
 create mode 100644 src/libsystemd-gfx/unifont.hex
 create mode 100644 src/systemd/sd-gfx.h

diff --git a/Makefile.am b/Makefile.am
index ce27e82..2edb091 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3853,6 +3853,37 @@ EXTRA_DIST += \
 endif
 
 # 
--
+if HAVE_GFX
+noinst_LTLIBRARIES += \
+   libsystemd-gfx.la
+
+libsystemd_gfx_la_SOURCES = \
+   src/libsystemd-gfx/sd-gfx.h \
+   src/libsystemd-gfx/gfx-unifont.c
+
+libsystemd_gfx_la_CFLAGS = \
+   $(AM_CFLAGS)
+
+libsystemd_gfx_la_LIBADD = \
+   libsystemd-shared.la \
+   src/libsystemd-gfx/unifont.bin.lo
+
+src/libsystemd-gfx/unifont.bin: make-unifont.py src/libsystemd-gfx/unifont.hex
+   $(AM_V_GEN)cat $(top_srcdir)/src/libsystemd-gfx/unifont.hex | $(PYTHON) 
$ $@
+
+src/libsystemd-gfx/unifont.cmp: make-unifont.py src/libsystemd-gfx/unifont.bin
+   $(AM_V_GEN)cat $(top_srcdir)/src/libsystemd-gfx/unifont.bin | $(PYTHON) 
$ verify $@
+
+update-unifont: src/libsystemd-gfx/unifont.bin src/libsystemd-gfx/unifont.cmp
+   @RET=`diff -u src/libsystemd-gfx/unifont.hex 
src/libsystemd-gfx/unifont.cmp | wc -l` ; \
+   if test x$$? != x0 -o x$$RET != x0 ; then \
+   echo Generated Unifont-file differs from original; 

[systemd-devel] [RFC 01/12] event: allow EPOLLET as event flag

[David Herrmann]

EPOLLET enables edge-triggered mode (see epoll(7) for more). For most
use-cases, level-triggered is just fine, but for master-TTYs we need
edge-triggered to catch EPOLLHUP. master-TTYs signal EPOLLHUP if no client
is connected, but a client may connect some time later (same happens
during vhangup(2)).

However, epoll doesn't allow masking EPOLLHUP so it's signaled constantly.
To avoid this, edge-triggered mode is needed.
---
 src/libsystemd-bus/sd-event.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/libsystemd-bus/sd-event.c b/src/libsystemd-bus/sd-event.c
index 6a6581b..b5ddf71 100644
--- a/src/libsystemd-bus/sd-event.c
+++ b/src/libsystemd-bus/sd-event.c
@@ -584,7 +584,7 @@ _public_ int sd_event_add_io(
 
 assert_return(e, -EINVAL);
 assert_return(fd = 0, -EINVAL);
-assert_return(!(events  
~(EPOLLIN|EPOLLOUT|EPOLLRDHUP|EPOLLPRI|EPOLLERR|EPOLLHUP)), -EINVAL);
+assert_return(!(events  
~(EPOLLIN|EPOLLOUT|EPOLLRDHUP|EPOLLPRI|EPOLLERR|EPOLLHUP|EPOLLET)), -EINVAL);
 assert_return(callback, -EINVAL);
 assert_return(ret, -EINVAL);
 assert_return(e-state != SD_EVENT_FINISHED, -ESTALE);
@@ -1022,7 +1022,7 @@ _public_ int 
sd_event_source_set_io_events(sd_event_source *s, uint32_t events)
 
 assert_return(s, -EINVAL);
 assert_return(s-type == SOURCE_IO, -EDOM);
-assert_return(!(events  
~(EPOLLIN|EPOLLOUT|EPOLLRDHUP|EPOLLPRI|EPOLLERR|EPOLLHUP)), -EINVAL);
+assert_return(!(events  
~(EPOLLIN|EPOLLOUT|EPOLLRDHUP|EPOLLPRI|EPOLLERR|EPOLLHUP|EPOLLET)), -EINVAL);
 assert_return(s-event-state != SD_EVENT_FINISHED, -ESTALE);
 assert_return(!event_pid_changed(s-event), -ECHILD);
 
-- 
1.8.4.2

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

[systemd-devel] [RFC 04/12] build: add target to link binary sources

[David Herrmann]

In several situations we want to link a binary file into our executable
and access it from our C code. The easiest way is to transform it into a
C-array and compile it as usual. However, for large files (1MB) such
compilations can take a considerable amount of time or even fail on
low-memory systems.

This adds a new automake-target to link binary sources directly. Instead
of transforming it into a C-array, we simply use ld -r to create an
object file via:
  ld -r -o my-source.bin.o --format=binary my-source.bin
We also use -z noexecstack to mark my-source.bin.o to not require an
executable stack.

As we only want to support read-only data sources here, we do some
post-processing to mark the object as read-only via:
  objcopy --rename-section .data=.rodata,alloc,load,readonly,data,contents 
my-source.bin.o

As libtool requires *.lo files, we cannot link this object-file
directly. Thus, we also create a fake *.lo file for such objects which
libtool can use. Note that libtool actually *requires* the comment-section
in *.lo files (ugh?!) so we need to fake that, too.

How to use this helper?
 - put your binary source file into the tree as:
 src/somewhere/something.bin
 - for the library you want to link that to, add this to mylib_LIBADD:
 src/somewhere/something.bin.lo
This causes the helper to create src/somewhere/something.bin.[o,lo] and
it will be linked as a normal object file.

GNU-ld automatically creates 3 symbols for such objects, but the important
symbols are:
  extern const char _binary_src_somewhere_something_bin_start[];
  extern const char _binary_src_somewhere_something_bin_end[];
Use these to access start/end of the binary section.

I tested this with in-tree and out-of-tree builds, with GNU-ld and
GNU-gold and cross-compilation. All worked fine..
---
 Makefile.am  | 19 +++
 configure.ac |  5 +
 2 files changed, 24 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 4aa2bdf..ce27e82 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -632,6 +632,25 @@ EXTRA_DIST += \
xml_helper.py
 
 # 
--
+CLEANFILES += *.bin.lo *.bin.o
+
+%.bin.lo: %.bin
+   $(AM_V_GEN)$(LD) -r -o $*.bin.o -z noexecstack --format=binary $
+   $(AM_V_at)$(OBJCOPY) --rename-section 
.data=.rodata,alloc,load,readonly,data,contents $*.bin.o
+   $(AM_V_at)echo # $@ - a libtool object file $@
+   $(AM_V_at)echo # Generated by $(shell $(LIBTOOL) --version | head -n 
1) $@
+   $(AM_V_at)echo # $@
+   $(AM_V_at)echo # Please DO NOT delete this file! $@
+   $(AM_V_at)echo # It is necessary for linking the library. $@
+   $(AM_V_at)echo $@
+   $(AM_V_at)echo # Name of the PIC object. $@
+   $(AM_V_at)echo pic_object='$(notdir $*).bin.o' $@
+   $(AM_V_at)echo $@
+   $(AM_V_at)echo # Name of the non-PIC object $@
+   $(AM_V_at)echo non_pic_object='$(notdir $*).bin.o' $@
+   $(AM_V_at)echo $@
+
+# 
--
 noinst_LTLIBRARIES += \
libsystemd-rtnl.la
 
diff --git a/configure.ac b/configure.ac
index c0656f4..3fd05da 100644
--- a/configure.ac
+++ b/configure.ac
@@ -101,6 +101,11 @@ if test -z $GPERF ; then
 AC_MSG_ERROR([*** gperf not found])
 fi
 
+AC_CHECK_TOOL([OBJCOPY], objcopy)
+if test -z $OBJCOPY ; then
+AC_MSG_ERROR([*** objcopy not found])
+fi
+
 # 
--
 address_sanitizer_cflags=
 address_sanitizer_cppflags=
-- 
1.8.4.2

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

[systemd-devel] [RFC 02/12] ring: add basic ring-buffer helper

[David Herrmann]

This adds a very straightforward ring-buffer implementation to
libsystemd-shared. Ring-buffers allow pushing data to, and pulling data
from, both ends of a buffer without modifying existing/remaining buffer
content. This is very useful for network buffers and asynchronous writes.

This implementation only contains the most basic functions to push data
onto the end of the buffer and pull data from the front. More helpers may
be added once needed.
---
 Makefile.am   |   2 +
 src/shared/ring.c | 213 ++
 src/shared/ring.h |  54 ++
 3 files changed, 269 insertions(+)
 create mode 100644 src/shared/ring.c
 create mode 100644 src/shared/ring.h

diff --git a/Makefile.am b/Makefile.am
index 0119751..4aa2bdf 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -768,6 +768,8 @@ libsystemd_shared_la_SOURCES = \
src/shared/net-util.h \
src/shared/errno-list.c \
src/shared/errno-list.h \
+   src/shared/ring.h \
+   src/shared/ring.c \
src/shared/syscall-list.c \
src/shared/syscall-list.h
 
diff --git a/src/shared/ring.c b/src/shared/ring.c
new file mode 100644
index 000..f60f098
--- /dev/null
+++ b/src/shared/ring.c
@@ -0,0 +1,213 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2013 David Herrmann dh.herrm...@gmail.com
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see http://www.gnu.org/licenses/.
+***/
+
+#include errno.h
+#include stdlib.h
+#include string.h
+#include sys/uio.h
+
+#include ring.h
+#include util.h
+
+#define RING_MASK(_r, _v) ((_v)  ((_r)-size - 1))
+
+void ring_flush(Ring *r) {
+r-start = 0;
+r-end = 0;
+}
+
+void ring_clear(Ring *r) {
+free(r-buf);
+memset(r, 0, sizeof(*r));
+}
+
+/*
+ * Resize ring-buffer to size @nsize. @nsize must be a power-of-2, otherwise
+ * ring operations will behave incorrectly.
+ */
+static int ring_resize(Ring *r, size_t nsize) {
+char *buf;
+
+buf = malloc(nsize);
+if (!buf)
+return -ENOMEM;
+
+if (r-end == r-start) {
+r-end = 0;
+r-start = 0;
+} else if (r-end  r-start) {
+memcpy(buf, r-buf[r-start], r-end - r-start);
+
+r-end -= r-start;
+r-start = 0;
+} else {
+memcpy(buf, r-buf[r-start], r-size - r-start);
+memcpy(buf[r-size - r-start], r-buf, r-end);
+
+r-end += r-size - r-start;
+r-start = 0;
+}
+
+free(r-buf);
+r-buf = buf;
+r-size = nsize;
+
+return 0;
+}
+
+/* Compute next higher power-of-2 of @v. Returns 4096 in case v is 0. */
+static size_t ring_pow2(size_t v) {
+size_t i;
+
+if (!v)
+return 4096;
+
+--v;
+
+for (i = 1; i  8 * sizeof(size_t); i *= 2)
+v |= v  i;
+
+return ++v;
+}
+
+/*
+ * Resize ring-buffer to provide enough room for @add bytes of new data. This
+ * resizes the buffer if it is too small. It returns -ENOMEM on OOM and 0 on
+ * success.
+ */
+static int ring_grow(Ring *r, size_t add) {
+size_t len;
+
+/*
+ * Note that end == start means empty buffer. Hence, we can never
+ * fill the last byte of a buffer. That means, we must account for an
+ * additional byte here (end == start-byte).
+ */
+
+if (r-end  r-start)
+len = r-start - r-end;
+else
+len = r-start + r-size - r-end;
+
+/* don't use = as end == start would be ambigious */
+if (len  add)
+return 0;
+
+/* +1 for additional end == start byte */
+len = r-size + add - len + 1;
+len = ring_pow2(len);
+
+if (len = r-size)
+return -ENOMEM;
+
+return ring_resize(r, len);
+}
+
+/*
+ * Push @len bytes from @u8 into the ring buffer. The buffer is resized if it
+ * is too small. -ENOMEM is returned on OOM, 0 on success.
+ */
+int ring_push(Ring *r, const char *u8, size_t len) {
+int err;
+size_t l;
+
+err = ring_grow(r, len);
+if (err  0)
+return err;
+
+if (r-start = r-end) {
+l = r-size - r-end;
+if (l  len)
+l = len;
+

[systemd-devel] [RFC 09/12] gfx: add kbd test

[David Herrmann]

The test-kbd helper simply attaches to the current session (or in case
--all is passed to the system) and prints all keyboard events generated by
attached keyboards.

It's meant to be used to debug sd_gfx_kbd/monitor issues regarding
keymap/kbd setup.
---
 .gitignore|   1 +
 Makefile.am   |  16 +++
 src/libsystemd-gfx/test-kbd.c | 314 ++
 3 files changed, 331 insertions(+)
 create mode 100644 src/libsystemd-gfx/test-kbd.c

diff --git a/.gitignore b/.gitignore
index f8f6c8a..f4921f5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -131,6 +131,7 @@
 /test-journal-init
 /test-journal-syslog
 /test-journal-verify
+/test-kbd
 /test-libudev
 /test-list
 /test-log
diff --git a/Makefile.am b/Makefile.am
index 189cc89..e8822b2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3878,6 +3878,22 @@ libsystemd_gfx_la_LIBADD = \
libudev-internal.la \
src/libsystemd-gfx/unifont.bin.lo
 
+test_kbd_SOURCES = \
+   src/libsystemd-gfx/test-kbd.c
+
+test_kbd_CFLAGS = \
+   $(AM_CFLAGS) \
+   $(GFX_CFLAGS)
+
+test_kbd_LDADD = \
+   $(GFX_LIBS) \
+   libsystemd-bus-internal.la \
+   libsystemd-shared.la \
+   libsystemd-gfx.la
+
+tests += \
+   test-kbd
+
 src/libsystemd-gfx/unifont.bin: make-unifont.py src/libsystemd-gfx/unifont.hex
$(AM_V_GEN)cat $(top_srcdir)/src/libsystemd-gfx/unifont.hex | $(PYTHON) 
$ $@
 
diff --git a/src/libsystemd-gfx/test-kbd.c b/src/libsystemd-gfx/test-kbd.c
new file mode 100644
index 000..b902171
--- /dev/null
+++ b/src/libsystemd-gfx/test-kbd.c
@@ -0,0 +1,314 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2013 David Herrmann dh.herrm...@gmail.com
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see http://www.gnu.org/licenses/.
+***/
+
+#include errno.h
+#include fcntl.h
+#include getopt.h
+#include inttypes.h
+#include libevdev/libevdev.h
+#include stdbool.h
+#include stdlib.h
+#include string.h
+#include strings.h
+#include unistd.h
+#include xkbcommon/xkbcommon.h
+
+#include build.h
+#include def.h
+#include log.h
+#include macro.h
+#include missing.h
+#include sd-bus.h
+#include sd-daemon.h
+#include sd-event.h
+#include sd-gfx.h
+#include util.h
+
+typedef struct Test Test;
+
+struct Test {
+sd_event *event;
+sd_event_source *sigs[_NSIG];
+sd_gfx_monitor *mon;
+};
+
+static bool arg_all;
+static const char *arg_gpus;
+static const char *arg_keymap;
+
+static void test_kbd_fn(sd_gfx_kbd *kbd, void *fn_data, sd_gfx_kbd_event *ev) {
+unsigned int i;
+const char *t;
+char s[128];
+uint32_t c;
+
+printf( %5d, ev-keycode);
+t = libevdev_event_code_get_name(EV_KEY, ev-keycode);
+printf( | %20s, t ? : unknown);
+
+printf( | %s, (ev-mods  SD_GFX_SHIFT) ? SHIFT :  );
+printf( | %s, (ev-mods  SD_GFX_CAPSL) ? CAPSL :  );
+printf( | %s, (ev-mods  SD_GFX_CTRL) ? CTRL : );
+printf( | %s, (ev-mods  SD_GFX_ALT) ? ALT :);
+printf( | %s, (ev-mods  SD_GFX_LOGO) ? LOGO : );
+
+printf( |);
+for (i = 0; i  ev-sym_count; ++i) {
+xkb_keysym_get_name(ev-keysyms[i], s, sizeof(s));
+printf( %20s, s);
+}
+
+printf( |);
+for (i = 0; i  ev-sym_count; ++i) {
+c = ev-codepoints[i];
+if (c == 0x)
+continue;
+if (c  0x1f)
+continue;
+if (c = 0x7f  c = 0x9f)
+continue;
+
+/* Just a proof-of-concept hack. This works because glibc uses
+ * UCS-4 as the internal wchar_t encoding. */
+printf( %lc, ev-codepoints[i]);
+}
+
+printf(\n);
+}
+
+static void test_add(Test *t, sd_gfx_kbd *kbd) {
+sd_gfx_kbd_set_event_fn(kbd, test_kbd_fn);
+}
+
+static void test_remove(Test *t, sd_gfx_kbd *kbd) {
+sd_gfx_kbd_set_event_fn(kbd, NULL);
+}
+
+static void test_event_fn(sd_gfx_monitor *mon, void *fn_data, 
sd_gfx_monitor_event *ev) {
+struct Test *t = fn_data;
+
+switch (ev-type) {
+case SD_GFX_MONITOR_CREATE:
+if (ev-devtype == SD_GFX_DEV_KBD)
+test_add(t, ev-kbd);
+else
+  

[systemd-devel] [RFC 11/12] gfx: add unbuilt GL test

[David Herrmann]

The test-gl helper shows how sd_gfx_card can be used to get a full OpenGL
context on the device. It is not added to the build-tools as it requires
mesa and might break on Khronos header-updates (yes, they break API *and*
ABI compatibility often!).
---
 .gitignore   |   1 +
 Makefile.am  |  18 +++
 configure.ac |   3 +
 src/libsystemd-gfx/test-gl.c | 342 +++
 4 files changed, 364 insertions(+)
 create mode 100644 src/libsystemd-gfx/test-gl.c

diff --git a/.gitignore b/.gitignore
index a61f68d..c856412 100644
--- a/.gitignore
+++ b/.gitignore
@@ -116,6 +116,7 @@
 /test-event
 /test-fileio
 /test-gfx
+/test-gl
 /test-hashmap
 /test-hostname
 /test-id128
diff --git a/Makefile.am b/Makefile.am
index aa17876..1e8aeed 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3886,6 +3886,19 @@ test_gfx_LDADD = \
libsystemd-shared.la \
libsystemd-gfx.la
 
+test_gl_SOURCES = \
+   src/libsystemd-gfx/test-gl.c
+
+test_gl_CFLAGS = \
+   $(AM_CFLAGS) \
+   $(GFX_GL_CFLAGS)
+
+test_gl_LDADD = \
+   $(GFX_GL_LIBS) \
+   libsystemd-bus-internal.la \
+   libsystemd-shared.la \
+   libsystemd-gfx.la
+
 test_kbd_SOURCES = \
src/libsystemd-gfx/test-kbd.c
 
@@ -3903,6 +3916,11 @@ tests += \
test-gfx \
test-kbd
 
+if HAVE_GFX_GL
+# Uncomment this to enable test-gl builds
+#tests += test-gl
+endif
+
 src/libsystemd-gfx/unifont.bin: make-unifont.py src/libsystemd-gfx/unifont.hex
$(AM_V_GEN)cat $(top_srcdir)/src/libsystemd-gfx/unifont.hex | $(PYTHON) 
$ $@
 
diff --git a/configure.ac b/configure.ac
index b76a86d..bf3fce3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -299,8 +299,11 @@ if test x$enable_gfx != xno; then
 if test x$have_gfx = xno -a x$enable_gfx = xyes; then
 AC_MSG_ERROR([*** sd-gfx support requested, but libraries not 
found])
 fi
+PKG_CHECK_MODULES(GFX_GL, [ libdrm = 2.4.47 gbm = 9.2.3 egl glesv2 ],
+[AC_DEFINE(HAVE_GFX_GL, 1, [Define if sd-gfx GL examples are 
built]) have_gfx_gl=yes], have_gfx_gl=no)
 fi
 AM_CONDITIONAL(HAVE_GFX, [test $have_gfx = yes])
+AM_CONDITIONAL(HAVE_GFX_GL, [test $have_gfx_gl = yes])
 
 # 
--
 have_blkid=no
diff --git a/src/libsystemd-gfx/test-gl.c b/src/libsystemd-gfx/test-gl.c
new file mode 100644
index 000..733b451
--- /dev/null
+++ b/src/libsystemd-gfx/test-gl.c
@@ -0,0 +1,342 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2013 David Herrmann dh.herrm...@gmail.com
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see http://www.gnu.org/licenses/.
+***/
+
+#define EGL_EGLEXT_PROTOTYPES
+#define GL_GLEXT_PROTOTYPES
+
+#include errno.h
+#include fcntl.h
+#include inttypes.h
+#include stdlib.h
+#include string.h
+#include unistd.h
+
+#include drm.h
+#include drm_fourcc.h
+#include EGL/egl.h
+#include EGL/eglext.h
+#include gbm.h
+#include GLES2/gl2.h
+#include GLES2/gl2ext.h
+
+#include def.h
+#include log.h
+#include macro.h
+#include missing.h
+#include sd-event.h
+#include sd-gfx.h
+#include util.h
+
+struct plane {
+struct gbm_surface *gbm;
+EGLSurface egl;
+
+struct gbm_surface *t_gbm;
+EGLSurface t_egl;
+};
+
+static int gl_fd;
+static struct gbm_device *gl_device;
+static EGLDisplay gl_display;
+static EGLConfig gl_config;
+static EGLContext gl_context;
+
+static void err(const char *format, ...) {
+va_list args;
+
+fprintf(stderr, ERROR: );
+va_start(args, format);
+vfprintf(stderr, format, args);
+va_end(args);
+fprintf(stderr, \n);
+
+_exit(1);
+}
+
+static void fb_unlink(sd_gfx_fb *fb, void *fn_data) {
+struct gbm_bo *bo = fn_data;
+
+gbm_bo_set_user_data(bo, NULL, NULL);
+}
+
+static void fb_unpin(sd_gfx_fb *fb, void *fn_data) {
+struct gbm_bo *bo = fn_data;
+sd_gfx_plane *plane = sd_gfx_fb_get_plane(fb);
+struct plane *p = sd_gfx_plane_get_fn_data(plane);
+
+gbm_surface_release_buffer(p-gbm, bo);
+}
+
+static void fb_destroy(struct gbm_bo *bo, void *data) {
+sd_gfx_fb *fb = data;
+
+if (!fb)
+return;
+
+err(fb_destroy());
+}
+
+static sd_gfx_fb *bo_to_fb(sd_gfx_plane *plane, struct 

[systemd-devel] [RFC 10/12] gfx: add graphics test

[David Herrmann]

The test-gfx helper simply renders a solid-color on all active pipelines.
Useful to debug monitor-hotplugging and GPU selection.
---
 .gitignore|   1 +
 Makefile.am   |   9 ++
 src/libsystemd-gfx/test-gfx.c | 302 ++
 3 files changed, 312 insertions(+)
 create mode 100644 src/libsystemd-gfx/test-gfx.c

diff --git a/.gitignore b/.gitignore
index f4921f5..a61f68d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -115,6 +115,7 @@
 /test-env-replace
 /test-event
 /test-fileio
+/test-gfx
 /test-hashmap
 /test-hostname
 /test-id128
diff --git a/Makefile.am b/Makefile.am
index e8822b2..aa17876 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3878,6 +3878,14 @@ libsystemd_gfx_la_LIBADD = \
libudev-internal.la \
src/libsystemd-gfx/unifont.bin.lo
 
+test_gfx_SOURCES = \
+   src/libsystemd-gfx/test-gfx.c
+
+test_gfx_LDADD = \
+   libsystemd-bus-internal.la \
+   libsystemd-shared.la \
+   libsystemd-gfx.la
+
 test_kbd_SOURCES = \
src/libsystemd-gfx/test-kbd.c
 
@@ -3892,6 +3900,7 @@ test_kbd_LDADD = \
libsystemd-gfx.la
 
 tests += \
+   test-gfx \
test-kbd
 
 src/libsystemd-gfx/unifont.bin: make-unifont.py src/libsystemd-gfx/unifont.hex
diff --git a/src/libsystemd-gfx/test-gfx.c b/src/libsystemd-gfx/test-gfx.c
new file mode 100644
index 000..36157b6
--- /dev/null
+++ b/src/libsystemd-gfx/test-gfx.c
@@ -0,0 +1,302 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2013 David Herrmann dh.herrm...@gmail.com
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see http://www.gnu.org/licenses/.
+***/
+
+#include errno.h
+#include fcntl.h
+#include getopt.h
+#include inttypes.h
+#include stdbool.h
+#include stdlib.h
+#include string.h
+#include strings.h
+#include unistd.h
+
+#include build.h
+#include def.h
+#include log.h
+#include macro.h
+#include missing.h
+#include sd-bus.h
+#include sd-daemon.h
+#include sd-event.h
+#include sd-gfx.h
+#include util.h
+
+typedef struct Test Test;
+
+struct Test {
+sd_event *event;
+sd_event_source *sigs[_NSIG];
+sd_gfx_monitor *mon;
+};
+
+static bool arg_all;
+static const char *arg_gpus;
+static const char *arg_keymap;
+
+static void test_pipe_draw(sd_gfx_pipe *pipe) {
+sd_gfx_plane *primary;
+sd_gfx_fb *fb;
+
+primary = sd_gfx_pipe_get_primary_plane(pipe);
+if (!primary) {
+log_error(no primary plane on pipe);
+return;
+}
+
+fb = sd_gfx_plane_get_front(primary);
+if (!fb) {
+log_error(no front-fb on primary plane);
+return;
+}
+
+sd_gfx_fb_fill(fb,
+   0xffaabb00,
+   0,
+   0,
+   sd_gfx_fb_get_width(fb),
+   sd_gfx_fb_get_height(fb));
+}
+
+static void test_card_fn(sd_gfx_card *card, void *fn_data, sd_gfx_card_event 
*ev) {
+switch (ev-type) {
+case SD_GFX_CARD_PIPE_WAKE_UP:
+test_pipe_draw(ev-pipe);
+break;
+}
+}
+
+static void test_add(Test *t, sd_gfx_card *card) {
+sd_gfx_card_set_event_fn(card, test_card_fn);
+}
+
+static void test_remove(Test *t, sd_gfx_card *card) {
+sd_gfx_card_set_event_fn(card, NULL);
+}
+
+static void test_event_fn(sd_gfx_monitor *mon, void *fn_data, 
sd_gfx_monitor_event *ev) {
+struct Test *t = fn_data;
+
+switch (ev-type) {
+case SD_GFX_MONITOR_CREATE:
+if (ev-devtype == SD_GFX_DEV_CARD)
+test_add(t, ev-card);
+else
+log_notice(unknown device of type %d, ev-devtype);
+break;
+case SD_GFX_MONITOR_DESTROY:
+if (ev-devtype == SD_GFX_DEV_CARD)
+test_remove(t, ev-card);
+break;
+default:
+log_warning(unhandled monitor event: %d, ev-type);
+break;
+}
+}
+
+static int test_signal_fn(sd_event_source *s, const struct signalfd_siginfo 
*ssi, void *data) {
+Test *t = data;
+
+log_notice(catched signal %d, exiting.., (int)ssi-ssi_signo);
+sd_event_request_quit(t-event);
+
+return 0;
+}
+
+static int test_new(Test **out) {
+   

[systemd-devel] [RFC 06/12] gfx: add keyboard layer

[David Herrmann]

To replace the kernel kbd layer, we need keyboard handling in user-space.
The current de-facto standard is XKB and luckily, in the last 3 years
there was an effort to make XKB indepedent of X11, namely libxkbcommon.

libxkbcommon provides keyboard handling compatible (and even superior) to
classic XKB. The default wayland kbd-protocol was designed around the
xkbcommon API and libxkbcommon is used by all known wayland-compositors.
Its only dependency is glibc. However, you also need the keymap-files,
which are part of xkeyboard-config, which itself has no dependencies and
only provides the uncompiled keymap source files.

The kbd sd-gfx layer takes an evdev fd as input and provides keyboard
presses as events. Everything in between is handled transparently to the
application.

To access the kernel evdev layer, we use libevdev. It is a small wrapper
around the evdev ioctls with no dependencies but libc. The main reason it
was created is to handle SYN_DROPPED (input even-queue overflow)
transparently. It is a mandatory dependency of xf86-input-evdev as of 2013
and will probably be used in most other wayland compositors, too.

The sd_gfx_kbd API should be straightforward. The only thing to mention is
async-sleep support. That is, whenever your session is moved into
background, the evdev fd is revoked. sd_gfx_kbd detects that and puts the
device asleep. Explicit sleep is also supported via sd_gfx_kbd_sleep().
You need to provide a new fd to wake the device up.
---
 Makefile.am  |   8 +-
 configure.ac |   7 +-
 src/libsystemd-gfx/gfx-kbd.c | 629 +++
 src/systemd/sd-gfx.h |  84 ++
 4 files changed, 725 insertions(+), 3 deletions(-)
 create mode 100644 src/libsystemd-gfx/gfx-kbd.c

diff --git a/Makefile.am b/Makefile.am
index 2edb091..6ecbd50 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3859,12 +3859,18 @@ noinst_LTLIBRARIES += \
 
 libsystemd_gfx_la_SOURCES = \
src/libsystemd-gfx/sd-gfx.h \
+   src/libsystemd-gfx/gfx-kbd.c \
src/libsystemd-gfx/gfx-unifont.c
 
 libsystemd_gfx_la_CFLAGS = \
-   $(AM_CFLAGS)
+   $(AM_CFLAGS) \
+   $(GFX_CFLAGS)
 
 libsystemd_gfx_la_LIBADD = \
+   $(GFX_LIBS) \
+   libsystemd-bus-internal.la \
+   libsystemd-daemon-internal.la \
+   libsystemd-id128-internal.la \
libsystemd-shared.la \
src/libsystemd-gfx/unifont.bin.lo
 
diff --git a/configure.ac b/configure.ac
index 354673a..74c37ae 100644
--- a/configure.ac
+++ b/configure.ac
@@ -294,8 +294,11 @@ AM_CONDITIONAL(HAVE_KMOD, [test $have_kmod = yes])
 have_gfx=no
 AC_ARG_ENABLE(gfx, AS_HELP_STRING([--disable-gfx], [disable sd-gfx graphics 
and input support]))
 if test x$enable_gfx != xno; then
-AC_DEFINE(HAVE_GFX, 1, [Define if sd-gfx is built])
-have_gfx=yes
+PKG_CHECK_MODULES(GFX, [ libevdev = 0.4 xkbcommon = 0.3 ],
+[AC_DEFINE(HAVE_GFX, 1, [Define if sd-gfx is built]) 
have_gfx=yes], have_gfx=no)
+if test x$have_gfx = xno -a x$enable_gfx = xyes; then
+AC_MSG_ERROR([*** sd-gfx support requested, but libraries not 
found])
+fi
 fi
 AM_CONDITIONAL(HAVE_GFX, [test $have_gfx = yes])
 
diff --git a/src/libsystemd-gfx/gfx-kbd.c b/src/libsystemd-gfx/gfx-kbd.c
new file mode 100644
index 000..4de21dd
--- /dev/null
+++ b/src/libsystemd-gfx/gfx-kbd.c
@@ -0,0 +1,629 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2013 David Herrmann dh.herrm...@gmail.com
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see http://www.gnu.org/licenses/.
+***/
+
+#include errno.h
+#include fcntl.h
+#include inttypes.h
+#include libevdev/libevdev.h
+#include stdbool.h
+#include stdlib.h
+#include string.h
+#include strings.h
+#include unistd.h
+#include xkbcommon/xkbcommon.h
+
+#include def.h
+#include log.h
+#include macro.h
+#include missing.h
+#include sd-event.h
+#include sd-gfx.h
+#include time-util.h
+#include util.h
+
+enum {
+SD_GFX__LED_NUML,
+SD_GFX__LED_CAPSL,
+SD_GFX__LED_SCROLLL,
+SD_GFX__LED_COUNT,
+};
+
+struct sd_gfx_kbd {
+char *name;
+struct libevdev *evdev;
+int fd;
+struct xkb_state *state;
+sd_event *ev;
+sd_event_source *fd_source;
+sd_event_source *timer;
+sd_gfx_kbd_event_fn event_fn;
+

[systemd-devel] [RFC 12/12] console: add systemd-consoled

[David Herrmann]

systemd-consoled is a very basic terminal-emulator to replace the
in-kernel VT layer. It is based on libtsm as emulation layer (which itself
has no external dependencies).

systemd-consoled expects to be run in a logind-session. The caller must
have already setup the session and prepared it for systemd-consoled. This
is usually done by login-managers.
---
 .gitignore  |   1 +
 Makefile.am |  24 +++
 configure.ac|  17 ++
 src/console/Makefile|   1 +
 src/console/consoled-pty.c  | 391 
 src/console/consoled-screen.c   | 170 +
 src/console/consoled-terminal.c | 371 ++
 src/console/consoled.c  | 278 
 src/console/consoled.h  | 128 +
 9 files changed, 1381 insertions(+)
 create mode 12 src/console/Makefile
 create mode 100644 src/console/consoled-pty.c
 create mode 100644 src/console/consoled-screen.c
 create mode 100644 src/console/consoled-terminal.c
 create mode 100644 src/console/consoled.c
 create mode 100644 src/console/consoled.h

diff --git a/.gitignore b/.gitignore
index c856412..0f563c3 100644
--- a/.gitignore
+++ b/.gitignore
@@ -36,6 +36,7 @@
 /systemd-cgls
 /systemd-cgroups-agent
 /systemd-cgtop
+/systemd-consoled
 /systemd-coredump
 /systemd-coredumpctl
 /systemd-cryptsetup
diff --git a/Makefile.am b/Makefile.am
index 1e8aeed..b5f1fd9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3937,6 +3937,30 @@ update-unifont: src/libsystemd-gfx/unifont.bin 
src/libsystemd-gfx/unifont.cmp
 endif
 
 # 
--
+if ENABLE_CONSOLED
+rootlibexec_PROGRAMS += \
+   systemd-consoled
+
+systemd_consoled_SOURCES = \
+   src/console/consoled.h \
+   src/console/consoled.c \
+   src/console/consoled-pty.c \
+   src/console/consoled-screen.c \
+   src/console/consoled-terminal.c
+
+systemd_consoled_CFLAGS = \
+   $(AM_CFLAGS) \
+   $(CONSOLED_CFLAGS)
+
+systemd_consoled_LDADD = \
+   $(CONSOLED_LIBS) \
+   libsystemd-bus.la \
+   libsystemd-daemon.la \
+   libsystemd-gfx.la \
+   libsystemd-shared.la
+endif
+
+# 
--
 if ENABLE_NETWORKD
 rootlibexec_PROGRAMS += \
systemd-networkd
diff --git a/configure.ac b/configure.ac
index bf3fce3..924637c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -306,6 +306,22 @@ AM_CONDITIONAL(HAVE_GFX, [test $have_gfx = yes])
 AM_CONDITIONAL(HAVE_GFX_GL, [test $have_gfx_gl = yes])
 
 # 
--
+have_consoled=no
+AC_ARG_ENABLE(consoled, AS_HELP_STRING([--disable-consoled], [disable system 
console]))
+if test x$enable_consoled != xno; then
+if test x$have_gfx = xno -a x$enable_consoled = xyes; then
+AC_MSG_ERROR([*** consoled support requested, but libraries 
not found])
+elif test x$have_gfx = xyes ; then
+PKG_CHECK_MODULES(CONSOLED, [ libtsm = 3 ],
+[AC_DEFINE(ENABLE_CONSOLED, 1, [Define if system 
console is built]) have_consoled=yes], have_consoled=no)
+if test x$have_consoled = xno -a x$enable_consoled = xyes; 
then
+AC_MSG_ERROR([*** consoled support requested, but 
libraries not found])
+fi
+fi
+fi
+AM_CONDITIONAL(ENABLE_CONSOLED, [test $have_consoled = yes])
+
+# 
--
 have_blkid=no
 AC_ARG_ENABLE(blkid, AS_HELP_STRING([--disable-blkid], [disable blkid 
support]))
 if test x$enable_blkid != xno; then
@@ -1095,6 +,7 @@ AC_MSG_RESULT([
 efi: ${have_efi}
 kmod:${have_kmod}
 sd-gfx:  ${have_gfx}
+consoled:${have_consoled}
 blkid:   ${have_blkid}
 nss-myhostname:  ${have_myhostname}
 gudev:   ${enable_gudev}
diff --git a/src/console/Makefile b/src/console/Makefile
new file mode 12
index 000..d0b0e8e
--- /dev/null
+++ b/src/console/Makefile
@@ -0,0 +1 @@
+../Makefile
\ No newline at end of file
diff --git a/src/console/consoled-pty.c b/src/console/consoled-pty.c
new file mode 100644
index 000..c3defc1
--- /dev/null
+++ b/src/console/consoled-pty.c
@@ -0,0 +1,391 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2013 David Herrmann dh.herrm...@gmail.com
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in 

Re: [systemd-devel] fstab, rootfs on btrfs

[Tom Gundersen]

On Wed, Nov 27, 2013 at 8:15 PM, Chris Murphy li...@colorremedies.com wrote:

 On Nov 27, 2013, at 4:53 AM, Kay Sievers k...@vrfy.org wrote:

 On Wed, Nov 27, 2013 at 5:16 AM, Chris Murphy li...@colorremedies.com 
 wrote:

 In Fedora 20, by default anaconda sets fs_passno in fstab to 1 for / on 
 btrfs. During offline updates, this is causing systemd-fstab-generator to 
 freak out not finding fsck.btrfs.

 https://bugzilla.redhat.com/show_bug.cgi?id=1034563

 Right, it should not set 1 for btrfs.

 Since it can be mounted rw from the get go, is it going to far to plan for 
 one day dropping the initial ro mount, remount rw sequence?

As far as I know there is no point in ever doing the remount, just set
the kernel commandline params correctly and mount it with the correct
options in the initrd.

 Today it already works to set the bootloader kernel parameter to mount btrfs 
 rw from the start, and remove the rootfs fstab entry entirely.

Yup, this should always work.

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

Re: [systemd-devel] [PATCH] core/socket: we only want one event on standard sockets

[Lennart Poettering]

On Wed, 27.11.13 07:46, Shawn Landden (sh...@churchofgit.com) wrote:

 
 On Tue, Nov 26, 2013 at 7:57 PM, David Timothy Strauss
 da...@davidstrauss.net wrote:
  On Wed, Nov 27, 2013 at 7:57 AM, Shawn Landden sh...@churchofgit.com 
  wrote:
  Are you sure applications can handle the extra file descriptor of
  passing both the sockfd
  and the acceptfd in this case? I don't see why they wouldn't just do
  the accept() themselves?
 
  Can you explain what you mean here, and how it differs from the
  existing MaxConnections.
 
  Actually, MaxConnections was exactly what I was thinking of. I agree
  with you now on Distribute=true only being useful with Accept=false.
 The current code I have has Distribute take a number, but it certainly
 would be possible to use
 the MaxConnections number---however I don't do that as connections
 isn't quite the
 right word, and we can't easily change it because it is part of the API.

Yeah, I agree. Let's leave MaxConnections= and Distribute= independent.

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] French translation for systemd

[Sylvain Plantefeve]

Hi Gentlemen,

Just a little ping to get fixed the few typos in the french translation
of the journal's catalog.

See the patch in attachment.

Thanks!


2013/11/19 Sylvain Plantefeve sylvain.plantef...@gmail.com

 You're right, my bad.

 Here is an updated patch to fix these issues.


 Le 18 novembre 2013 17:22, Jean-Michel Pollion 
 jeanmichel.poll...@gmail.com a écrit :

 2013/11/18 Sylvain Plantefeve sylvain.plantef...@gmail.com

 Ok, thanks, I'll have a look at it.

 BTW, while re-reading the catalog again, I found a couple of typos...

 ---
 diff --git a/catalog/systemd-fr.catalog b/catalog/systemd-fr.catalog
 index 92cc15f..dd7af91 100644
 --- a/catalog/systemd-fr.catalog
 +++ b/catalog/systemd-fr.catalog
 @@ -128,7 +128,7 @@ Subject: Le démarrage du système est terminé
  Defined-By: systemd
  Support: http://lists.freedesktop.org/mailman/listinfo/systemd-devel

 -Tous les services nécéssaires au démarrage du système ont tous été
 lancés
 +Tous les services nécessaires au démarrage du système ont tous été
 lancés
  avec succès. Notez que cela ne signifie pas que le système est
 maintenant au
  repos, car des services peuvent encore être en train de terminer leur
  démarrage.


 That's still an incorrect sentence in French, using tous twice is
 wrong, you should use either one of these instead :
 - Tous les services nécessaires au démarrage du système ont été lancés...
 - Les services nécessaires au démarrage du système ont tous été lancés...

 Either one should be OK.



diff --git a/catalog/systemd-fr.catalog b/catalog/systemd-fr.catalog
index 92cc15f..3ec7d8e 100644
--- a/catalog/systemd-fr.catalog
+++ b/catalog/systemd-fr.catalog
@@ -128,8 +128,8 @@ Subject: Le démarrage du système est terminé
 Defined-By: systemd
 Support: http://lists.freedesktop.org/mailman/listinfo/systemd-devel
 
-Tous les services nécéssaires au démarrage du système ont tous été lancés
-avec succès. Notez que cela ne signifie pas que le système est maintenant au
+Tous les services nécessaires au démarrage du système ont été lancés avec
+succès. Notez que cela ne signifie pas que le système est maintenant au
 repos, car des services peuvent encore être en train de terminer leur
 démarrage.
 
@@ -137,7 +137,7 @@ Le chargement du noyau a nécessité @KERNEL_USEC@ microsecondes.
 
 Le chargement du « RAM disk » initial a nécessité @INITRD_USEC@ microsecondes.
 
-Le chargement de l'espace utilisateur a nécéssité @USERSPACE_USEC@ microsecondes.
+Le chargement de l'espace utilisateur a nécessité @USERSPACE_USEC@ microsecondes.
 
 -- 6bbd95ee977941e497c48be27c254128 fr
 Subject: Le système entre dans l'état de repos (sleep state) @SLEEP@
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Hard-coded /bin/mount in systemd

[Lennart Poettering]

On Wed, 27.11.13 14:31, Hannes Reinecke (h...@suse.de) wrote:

 Hi all,
 
 for some reason systemd has /bin/mount hardcoded in
 src/core/mount.c:mount_enter_mounting()
 
 Which is a bit odd, seeing that everyting moved to /usr/bin.
 So we always have to do a symlink here, which really is a bit annoying.
 
 Is this by design or a simple left-over?

Well, /bin/mount is kinda API of Linux, you cannot just change that. All
systems must provide that, the same way as /bin/sh, reagrdless whether
/ and /usr is merged or not. There's really not much point in making this
configurable, we don't really want to support systems which move things
wildly around into the strangest places.

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] Hard-coded /bin/mount in systemd

[Lennart Poettering]

On Wed, 27.11.13 15:12, Hannes Reinecke (h...@suse.de) wrote:

 On 11/27/2013 02:58 PM, Mantas Mikulėnas wrote:
  On Wed, Nov 27, 2013 at 3:31 PM, Hannes Reinecke h...@suse.de wrote:
 
  Hi all,
 
  for some reason systemd has /bin/mount hardcoded in
  src/core/mount.c:mount_enter_mounting()
 
  Which is a bit odd, seeing that everyting moved to /usr/bin.
  So we always have to do a symlink here, which really is a bit annoying.
 
  Is this by design or a simple left-over?
  
  If *everything* moved to /usr/bin, then /bin itself has to be a
  symlink anyway (as many tools expect and some standards require
  specific commands to be in /bin).
  
 Ah. IIRC it was _systemd_ which initiated the move to /usr, so I
 found it slightly odd to rely on a location which it has just
 obsoleted ...
 Or, rather, to have a hard-coded location to start with.

systemd supports split /usr just fine really.

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] fstab, rootfs on btrfs

[Lennart Poettering]

On Wed, 27.11.13 12:15, Chris Murphy (li...@colorremedies.com) wrote:

 
 
 On Nov 27, 2013, at 4:53 AM, Kay Sievers k...@vrfy.org wrote:
 
  On Wed, Nov 27, 2013 at 5:16 AM, Chris Murphy li...@colorremedies.com 
  wrote:
  
  In Fedora 20, by default anaconda sets fs_passno in fstab to 1 for / on 
  btrfs. During offline updates, this is causing systemd-fstab-generator to 
  freak out not finding fsck.btrfs.
  
  https://bugzilla.redhat.com/show_bug.cgi?id=1034563
  
  Right, it should not set 1 for btrfs.
 
 Since it can be mounted rw from the get go, is it going to far to plan
 for one day dropping the initial ro mount, remount rw sequence?

Hmm, no, not really. If no initrd is used it's already bad enough that
we run fsck while we have the file system mounted, but it would be even
worse if we'd write to the file system.

The recommended logic is: use an initrd, and fsck the rootfs before
mounting, and do so writable. The legacy logic is: use no initrd, and
let the kernel mount read-only, then file system check, then remount
writable.

Or actaully, it's even more complex than that: the remount step after
fsck does more than just mount writable, it will actually apply the
mount options in /etc/fstab to the root file system, whatever those
settings might be. Writability is just one of the.

Lennart

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

Re: [systemd-devel] [systemd-commits] man/systemd-socket-proxyd.xml src/socket-proxy TODO

[Lennart Poettering]

On Wed, 27.11.13 14:09, David Timothy Strauss (da...@davidstrauss.net) wrote:

  If this is about running multiple things in the same Network namespace,
  then there are certainly other, better ways to achieve the same. For
  example, we could introduce JoinPrivateNetwork=$SERVICE or so which
  would allow one service to join the network namespace of another. That
  makes this much smoother and more powerful, too.
 
 That would be wonderful. I'd love to shed the shell scripts.

This is implemented now in JoinsNamespaceOf= and will affect both
PrivateNetwork= and PrivateTmp=.

I also took the liberty to revert the --listener patch, since it is
unnecessary now, and I updated the examples in the man page to not use
shell scripts anymore. I didn't test them though, please check if
everything's OK now.

Lennart

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

Re: [systemd-devel] [PATCH v2 02/26] dhcp: Add DHCP client initialization

[Lennart Poettering]

On Wed, 27.11.13 16:13, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

   +int sd_dhcp_client_set_request_address(DHCPClient *client,
   +   const struct in_addr
   *last_address);
  
  struct in_addr? Didn't you plan to use simple uint32_t for this?
  
  Also, if this is supposed to cover ipv6 one day too, maybe call this
  sd_dhcp_client_set_request_address4() or so?
 
 Internally an uint32_t is fine for IPv4 handling.
 
 In the public API I tried to be consistent with a future IPv6
 implementation that probably should use 'struct in6_addr' when passing
 IPv6 addresses. IPv6 could of course use some other structure as well,
 but I expected people to be most familiar with the already existing
 'struct in6_addr'. From that it followed that using 'in_addr' in the
 public part for IPv4 would be consistent and logical. I have no problems
 using an uint32_t in the public API also if that is wanted.

OK, sticking to libc definitions and keeping this in sync certainly
makes some sense.

 I really haven't decided on function names for DHCPv6, I was thinking
 sd_dhcp6_* would be a good prefix to start with.

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] [PATCH v2 05/26] dhcp: Add option appending and parsing

[Lennart Poettering]

On Wed, 27.11.13 16:51, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

 On Tue, 2013-11-26 at 00:08 +0100, Lennart Poettering wrote:
  On Mon, 25.11.13 09:13, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:
  
   +#include stdint.h
   +
   +#include protocol.h
   +
   +int dhcp_option_append(uint8_t **buf, int *buflen, uint8_t code,
  
   
  
  This is a memory size, right? It should be size_t rather than int, then, no?
 
 size_t it is.
 
   +static int parse_options(uint8_t *buf, int32_t buflen, int *overload,
  
  ^^ const missing?
 
 Here the idea was to go over the whole buffer and calling the callback
 with the proper code, len and value. Start of 'buf', i.e. 'code' in the
 function, needs to be advanced to the next option, so its not a const
 value.

The const in const uint8_t *buf declares that the buffer this points
*to* is constant, not the pointer itself. You are welcome to alter the
pointer as much as you want if you do this. Note the difference between
const uint8_t * buf and uint8_t * const buf...

The latter makes little sense, but the former makes a lot of sense.

A function declared as 

  void foo(const int a);

makes little sense, as this just means that the function internally
won't change the internal copy of a, which is pretty useless
information. 

  void foo(const int *a);

however makes a ton of sense, since it indicates that the buffer passed
in won't be changed, while the function can do whatever it likes with
the pointer copy it got internally...

Lennart

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

Re: [systemd-devel] [PATCH v2 19/26] dhcp: Add timeout and main loop support

[Lennart Poettering]

On Wed, 27.11.13 16:57, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

 
   Hi,
 
 On Tue, 2013-11-26 at 05:32 +0100, Zbigniew Jędrzejewski-Szmek wrote:
   +static int client_timeout_resend(sd_event_source *s, uint64_t usec,
   + void *userdata)
   +{
 ...
   +
   +return 0;
   +
   +error:
   +client_stop(client, err);
   +
   +return 0;
  Return err?
 
 Here I didn't want to spill the error back to the event handling loop
 code as the situation is handled internally by stopping the client and
 giving the error reason to a registered callback (in patch 25/26) via
 'client_stop()'.

Just to mention this in this context:

I think I'll rework sd-event to not ripple up the error values returned
by event callbacks. It makes code too fragile, and for example in PID 1
we never want to have th event loop fail, just because some event
handler returned negative. We should keep event erros local to the event
handling and not turn them into global failures of the entire program.

Instead I'll probably change things to eat up any error returned by an
event callback, however the event source is turned off when this
happens, to keep the error local.

Lennart

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

Re: [systemd-devel] [PATCH v2 20/26] dhcp: Handle received DHCP Offer message

[Lennart Poettering]

On Wed, 27.11.13 17:13, Patrik Flykt (patrik.fl...@linux.intel.com) wrote:

   +buflen = sizeof(DHCPPacket) + DHCP_CLIENT_MIN_OPTIONS_SIZE;
   +buf = malloc0(buflen);
   +if (!buf) {
   +read(fd, tmp, 1);
   +return 0;
   +}
  
  Hmm, how long is this message? Given that you don't keep the memory
  around, why not just allocate this on the stack? Either with alloca, or
  even directly as an uint8_t array? Given that this seems to be
  relatively short, this should not be an issue and is one error source
  less...
 
 This one would be the minimum IPv4 packet size, 576 bytes. Should I go
 with alloca() here?

As a rule of thumb I'd alway prefer stack allocation for things  2K or
so... In userspace that should be totally OK, and if you never need this
buffer outside of this call, then stack allocation should be fine.

Note that allocating this as explicit array on the stack is usually the
better option than alloca(), since alloca() can get very confusing when
used inside of loops.

Hence:

   uint8_t buf[sizeof(DHCPPacket) + DHCP_CLIENT_MIN_OPTIONS_SIZE]; 

sounds better, if you can avoid:

   void *buf;
   buf = alloca0(sizeof(DHCPPacket) + DHCP_CLIENT_MIN_OPTIONS_SIZE);

which is still better than:

   _cleanup_free_ void *buf;
   buf = malloc0(sizeof(DHCPPacket) + DHCP_CLIENT_MIN_OPTIONS_SIZE);
   if (!buf)
   ...

All of course only if the the size is not huge, and if you never need
the thing outside of the local scope...

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] fstab, rootfs on btrfs

[Chris Murphy]

On Nov 27, 2013, at 12:52 PM, Lennart Poettering lenn...@poettering.net wrote:

 On Wed, 27.11.13 12:15, Chris Murphy (li...@colorremedies.com) wrote:
 
 On Nov 27, 2013, at 4:53 AM, Kay Sievers k...@vrfy.org wrote:
 
 Right, it should not set 1 for btrfs.
 
 Since it can be mounted rw from the get go, is it going to far to plan
 for one day dropping the initial ro mount, remount rw sequence?
 
 Hmm, no, not really. If no initrd is used it's already bad enough that
 we run fsck while we have the file system mounted, but it would be even
 worse if we'd write to the file system.

Such concern doesn't seem indicated for btrfs. It checks and fixes itself at rw 
mount time, which either works or fails, and if it fails the fs doesn't mount.


 The recommended logic is: use an initrd, and fsck the rootfs before
 mounting, and do so writable. The legacy logic is: use no initrd, and
 let the kernel mount read-only, then file system check, then remount
 writable.

Well the recommended logic for btrfs differs substantially. The use of btrfsck 
--repair is dead last on the list of things to try, it's definitely not the 
first like is accepted with other file systems.

If rw fails, then systemd should remount with option 'recovery'. If that fails, 
and it's a multiple device volume, then remount with option  'degraded'. And if 
that also fails, then remount with 'ro,recovery' - and maybe (?) we should go 
to emergency.target at that point rather than fully boot.

This sort of conditional remounting attempts isn't something fstab can do.

 
 Or actaully, it's even more complex than that: the remount step after
 fsck does more than just mount writable, it will actually apply the
 mount options in /etc/fstab to the root file system, whatever those
 settings might be. Writability is just one of the.

Understood, but if default mount options are wisely chosen by btrfs devs, then 
fstab is obviated for most use cases. The most typical btrfs mount option 
needed is specifying a subvolume for rootfs, which must be done with 
rootflags=subvol= anyway because only putting it in fstab is too late.

And for other use cases, like compression, that can be done with either 
rootflags= or fstab.


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

Re: [systemd-devel] [RFC 04/12] build: add target to link binary sources

[Greg KH]

On Wed, Nov 27, 2013 at 07:48:39PM +0100, David Herrmann wrote:
 In several situations we want to link a binary file into our executable
 and access it from our C code. The easiest way is to transform it into a
 C-array and compile it as usual. However, for large files (1MB) such
 compilations can take a considerable amount of time or even fail on
 low-memory systems.
 
 This adds a new automake-target to link binary sources directly. Instead
 of transforming it into a C-array, we simply use ld -r to create an
 object file via:
   ld -r -o my-source.bin.o --format=binary my-source.bin
 We also use -z noexecstack to mark my-source.bin.o to not require an
 executable stack.
 
 As we only want to support read-only data sources here, we do some
 post-processing to mark the object as read-only via:
   objcopy --rename-section .data=.rodata,alloc,load,readonly,data,contents 
 my-source.bin.o
 
 As libtool requires *.lo files, we cannot link this object-file
 directly. Thus, we also create a fake *.lo file for such objects which
 libtool can use. Note that libtool actually *requires* the comment-section
 in *.lo files (ugh?!) so we need to fake that, too.
 
 How to use this helper?
  - put your binary source file into the tree as:
  src/somewhere/something.bin
  - for the library you want to link that to, add this to mylib_LIBADD:
  src/somewhere/something.bin.lo
 This causes the helper to create src/somewhere/something.bin.[o,lo] and
 it will be linked as a normal object file.
 
 GNU-ld automatically creates 3 symbols for such objects, but the important
 symbols are:
   extern const char _binary_src_somewhere_something_bin_start[];
   extern const char _binary_src_somewhere_something_bin_end[];
 Use these to access start/end of the binary section.
 
 I tested this with in-tree and out-of-tree builds, with GNU-ld and
 GNU-gold and cross-compilation. All worked fine..

That's crazy, very nice job in doing this :)

I tried to do this a while ago, and gave up and just used a perl script
and a .c file with a big array, like 'xxd -i' can create.  I think I'll
steal this idea for other projects if you don't mind, as it makes
things easier just to use the linker.

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

Re: [systemd-devel] [RFC 00/12] Bugfixes for CONFIG_VT=n

[Greg KH]

On Wed, Nov 27, 2013 at 07:48:35PM +0100, David Herrmann wrote:
 Hi
 
 So booting without VTs is still a horrible experience on desktop-linux. If
 anyone tried, they were presumably running away in tears. This series is a new
 attempt to fix that.
 
 The first 4 patches are just small fixes/helpers. Patch 5 to 8 introduce 
 sd-gfx
 and patch 9-11 add tests/examples for sd-gfx. The final patch adds a first 
 user
 of sd-gfx to replace the kernel-internal terminal-emulator. The commit-msgs
 should explain each change at length so I won't repeat it here.
 
 To anyone interested in testing: If you apply this series, you can run
 ./test-kbd to test keyboard handling, ./test-gfx to test graphics 
 rendering
 and ./systemd-consoled to run the terminal-emulator. Note that you must run
 all these from *within* an existing logind-session. That usually means from a
 TEXT-VT. Also note that all these programs currently ignore the underlying VT
 entirely (in case you run with CONFIG_VT=y). This has the side-effect, that 
 all
 keystrokes go to both, the VT and the application. This will obviously be
 changed in later versions, but I wanted to avoid any VT calls in these 
 helpers.
 I currently work on logind to fix that.
 
 I had to strip the font-patches due to ML size-constraints. The full series is
 always available on the-often-rebased-console-branch at:
   http://cgit.freedesktop.org/~dvdhrm/systemd/log/?h=console
 
 Notes on where this is going, and *why*, below..

Very nice work, keep it up.

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

Re: [systemd-devel] [PATCH] util: don't consider trailing whitespaces as an empty string in split_quoted

[Lennart Poettering]

On Wed, 27.11.13 18:00, Lukas Nykryn (lnyk...@redhat.com) wrote:

I merged a different patch that simply moves the earlier NUL byte check
after the jumping over whitespace, so that we don't need two checks. I
also added a test case for this, so that this doesn't break again.

(That sad, the whole function is moronic, since it doesn't really do any
sane unescaping of quotes inside of strings... We should rewrite this
one day, and maybe do it in a smart way that allocates a dynamically
sized string of the current word on the stack...)

 ---
  src/shared/util.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
 
 diff --git a/src/shared/util.c b/src/shared/util.c
 index 3a4d196..c68ab09 100644
 --- a/src/shared/util.c
 +++ b/src/shared/util.c
 @@ -383,7 +383,9 @@ char *split_quoted(const char *c, size_t *l, char 
 **state) {
  
  current += strspn(current, WHITESPACE);
  
 -if (*current == '\'') {
 +if (*current == 0)
 +return NULL;
 +else if (*current == '\'') {
  current ++;
  
  for (e = current; *e; e++) {


Lennart

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

Re: [systemd-devel] [RFC 01/12] event: allow EPOLLET as event flag

[Lennart Poettering]

On Wed, 27.11.13 19:48, David Herrmann (dh.herrm...@gmail.com) wrote:

Looks obvious. Applied.

 EPOLLET enables edge-triggered mode (see epoll(7) for more). For most
 use-cases, level-triggered is just fine, but for master-TTYs we need
 edge-triggered to catch EPOLLHUP. master-TTYs signal EPOLLHUP if no client
 is connected, but a client may connect some time later (same happens
 during vhangup(2)).
 
 However, epoll doesn't allow masking EPOLLHUP so it's signaled constantly.
 To avoid this, edge-triggered mode is needed.
 ---
  src/libsystemd-bus/sd-event.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/src/libsystemd-bus/sd-event.c b/src/libsystemd-bus/sd-event.c
 index 6a6581b..b5ddf71 100644
 --- a/src/libsystemd-bus/sd-event.c
 +++ b/src/libsystemd-bus/sd-event.c
 @@ -584,7 +584,7 @@ _public_ int sd_event_add_io(
  
  assert_return(e, -EINVAL);
  assert_return(fd = 0, -EINVAL);
 -assert_return(!(events  
 ~(EPOLLIN|EPOLLOUT|EPOLLRDHUP|EPOLLPRI|EPOLLERR|EPOLLHUP)), -EINVAL);
 +assert_return(!(events  
 ~(EPOLLIN|EPOLLOUT|EPOLLRDHUP|EPOLLPRI|EPOLLERR|EPOLLHUP|EPOLLET)), -EINVAL);
  assert_return(callback, -EINVAL);
  assert_return(ret, -EINVAL);
  assert_return(e-state != SD_EVENT_FINISHED, -ESTALE);
 @@ -1022,7 +1022,7 @@ _public_ int 
 sd_event_source_set_io_events(sd_event_source *s, uint32_t events)
  
  assert_return(s, -EINVAL);
  assert_return(s-type == SOURCE_IO, -EDOM);
 -assert_return(!(events  
 ~(EPOLLIN|EPOLLOUT|EPOLLRDHUP|EPOLLPRI|EPOLLERR|EPOLLHUP)), -EINVAL);
 +assert_return(!(events  
 ~(EPOLLIN|EPOLLOUT|EPOLLRDHUP|EPOLLPRI|EPOLLERR|EPOLLHUP|EPOLLET)), -EINVAL);
  assert_return(s-event-state != SD_EVENT_FINISHED, -ESTALE);
  assert_return(!event_pid_changed(s-event), -ECHILD);
  


Lennart

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

Re: [systemd-devel] [RFC 02/12] ring: add basic ring-buffer helper

[Lennart Poettering]

On Wed, 27.11.13 19:48, David Herrmann (dh.herrm...@gmail.com) wrote:

 +void ring_flush(Ring *r) {
 +r-start = 0;
 +r-end = 0;
 +}
 +
 +void ring_clear(Ring *r) {
 +free(r-buf);
 +memset(r, 0, sizeof(*r));

zero(*r) is a tiny bit nicer.

 +}
 +
 +/*
 + * Resize ring-buffer to size @nsize. @nsize must be a power-of-2, otherwise
 + * ring operations will behave incorrectly.
 + */
 +static int ring_resize(Ring *r, size_t nsize) {
 +char *buf;

Hmm, char suggests a bit that this is about text. But it's mostly raw
bytes, right? So I'd always use uint8_t for these things, it feels so
much rawer... Not that it would matter much...

 +
 +buf = malloc(nsize);
 +if (!buf)
 +return -ENOMEM;
 +
 +if (r-end == r-start) {
 +r-end = 0;
 +r-start = 0;

Hmm, so if end and start match the buffer is empty? So you can never
fill it entirely? Or am I missing something?

I'd always maintain start index + fill level instead of a start
index + end index, to avoid the confusion regarding the fill-level...

 + * Push @len bytes from @u8 into the ring buffer. The buffer is resized if it
 + * is too small. -ENOMEM is returned on OOM, 0 on success.
 + */
 +int ring_push(Ring *r, const char *u8, size_t len) {

So, here you call the parameter u8 suggesting unsigned bytes, but you
use char, which indicates a text string, and is
signed... Confusing. I'd recommend using const void * here by default,
since all types implicitly downgrade to const void* without casting...

Lennart

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

Re: [systemd-devel] [RFC 03/12] bus: add two new bus_*_map_*_properties() helpers

[Lennart Poettering]

On Wed, 27.11.13 19:48, David Herrmann (dh.herrm...@gmail.com) wrote:

 +/* skip interface, but allow callers to do that themselves */
 +sd_bus_message_skip(m, s);

This feels a bit like taping over bugs. I'd suggest adding an additional
parameter const char **interface to the function which if non-NULL is
updated with the interface name. If it is NULL we'd just skip th
interface as above...

if (interface)
  r = sd_bus_message_read(s, interface);
else
  r = sd_bus_message_skip(m, s);

...

Lennart

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

Re: [systemd-devel] [RFC 05/12] gfx: add sd-gfx library with unifont section

[Lennart Poettering]

On Wed, 27.11.13 19:48, David Herrmann (dh.herrm...@gmail.com) wrote:

 This patch introduces sd-gfx, a systemd-internal library dealing with all
 these things. Note that it is designed to be exported some day, but for
 now we keep it internal.
 While the library itself will be kept small and almost self-contained, it
 is designed to be extensible and can be used in rather complex graphics
 applications. For systemd, we plan to add a basic emergency-console, an
 initrd password-query, kernel-log-screen and a fallback login-screen.
 These allow booting without CONFIG_VT and provide a basic system for
 seats without VTs (either CONFIT_VT=n or seats != seat0).

BTW, one more use of this I'd like to see: if we boot-up and detect that
no AC and very low battery we should show a pretty screen and block the
boot until AC is plugged in or the user hits some magic key...

 +static int gfx_glyph_render(gfx_glyph *g, unsigned int ppi, const struct 
 unifont_data *u) {
 +g-buf.width = u-cells * unifont_width;
 +g-buf.height = unifont_height;
 +g-buf.stride = u-cells * unifont_stride;
 +g-buf.format = SD_GFX_BUFFER_FORMAT_A1;
 +
 +g-buf.data = calloc(unifont_height, unifont_max_cells * 
 unifont_stride);
 +if (!g-buf.data)
 +return -ENOMEM;

Nice, this must be the first use of alloc() I have ever seen that
actually takes benefit of the weird parameters it takes ;-)

 +
 +gfx_glyph_blend(g, ppi, u);
 +return 0;
 +}
 +
 +int sd_gfx_font_new(sd_gfx_font **out, unsigned int ppi) {

Hmm, so far we followed the rough rule that parameters we pass out are
listed last on the function parameter list. 

int sd_gfx_font_new(unsigned ppi, sd_gfx_fond **out);



 +sd_gfx_font *font;
 +int r;
 +
 +ppi = CLAMP(ppi, 10U, 1000U);
 +
 +font = calloc(1, sizeof(*font));

font = new0(sd_gfx_font, 1);

 +font-glyphs = hashmap_new(trivial_hash_func, trivial_compare_func);
 +if (!font-glyphs) {
 +free(font);
 +return log_oom();

Hmm, library code should never log. Please just return -ENOMEM
here. log_oom() should be used in main programs only really...

 +void sd_gfx_font_free(sd_gfx_font *font) {
 +gfx_glyph *g;

For public APIs we are pretty defensive and check all parameters we
take. Given you kinda made this a public library it might make sense to
follow that rule here, too. assert_return() is particularly useful for
this. (Well, not for the instance above, bug if you return a negative
errno it is super-useful).

Most destructors we have tend to accept NULL pointers too. It makes
clean-up code a lot easier (especially in combination with gcc cleanup
attributes). libc free() takes NULL pointers too, so this is a natural
extension.


 +void sd_gfx_font_render(sd_gfx_font *font, uint32_t id, const uint32_t 
 *ucs4, size_t len, sd_gfx_buffer **out) {
 +const struct unifont_data *u;
 +gfx_glyph *g;
 +
 +if (!len)
 +goto error;
 +
 +if (!id)
 +id = *ucs4;
 +
 +g = hashmap_get(font-glyphs, INT_TO_PTR(id));
 +if (g)
 +goto out;
 +
 +g = calloc(1, sizeof(*g));

g = new(gfx_glyph, 1);

 +if (!g) {
 +log_oom();
 +goto error;
 +}
 +
 +u = unifont_get(*ucs4);
 +if (gfx_glyph_render(g, font-ppi, u)  0) {
 +log_oom();
 +free(g);
 +goto error;
 +}
 +
 +while (--len) {
 +u = unifont_get(*++ucs4);
 +gfx_glyph_blend(g, font-ppi, u);
 +}
 +
 +if (hashmap_put(font-glyphs, INT_TO_PTR(id), g)  0) {
 +log_oom();
 +free(g-buf.data);
 +free(g);
 +goto error;
 +}
 +
 +goto out;
 +
 +error:
 +g = font-fallback;
 +out:
 +*out = g-buf;
 +}

Hmm, we so far followed to rule to not clobber return parameters on
failure. i.e. we try hard to fill any return values in until we know
that nothing can fail anymore. This makes the contract for the caller
much easier who can refrain from freeing/resetting any parameters if a
call failed...

Lennart

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

Re: [systemd-devel] [RFC 06/12] gfx: add keyboard layer

[Lennart Poettering]

On Wed, 27.11.13 19:48, David Herrmann (dh.herrm...@gmail.com) wrote:

 +
 +enum {
 +SD_GFX__LED_NUML,
 +SD_GFX__LED_CAPSL,
 +SD_GFX__LED_SCROLLL,
 +SD_GFX__LED_COUNT,
 +};

Double underscores?

 +
 +struct sd_gfx_kbd {
 +char *name;
 +struct libevdev *evdev;
 +int fd;
 +struct xkb_state *state;
 +sd_event *ev;
 +sd_event_source *fd_source;
 +sd_event_source *timer;
 +sd_gfx_kbd_event_fn event_fn;
 +void *data;
 +void *fn_data;
 +
 +unsigned int delay;
 +unsigned int rate;
 +
 +unsigned int sym_count;
 +sd_gfx_kbd_event event;
 +sd_gfx_kbd_event repeat;
 +
 +xkb_mod_index_t xkb_mods[SD_GFX__MOD_COUNT];
 +xkb_led_index_t xkb_leds[SD_GFX__LED_COUNT];
 +
 +unsigned int awake : 1;
 +unsigned int need_sync : 1;
 +unsigned int repeating : 1;

If these are bools, then make them bools! C99 bools (i.e. the type
bool from stdbool.h) are awesome for bit fields! [ Please use C99 bools
everywhere in internal code, and int as bool type for public APIs,
since that's what pre-C99 code usually did, despite the stupidity this
results in when people use bit fields ]

 +if (r  0) {
 +if (r == -EACCES)
 +log_debug(kbd %s: EACCES on led-update, kbd-name);
 +else
 +log_error(kbd %s: cannot update leds: %d,
 +  kbd-name, r);

Please do not log from libraries. (Well, except when that's the primary
purpose of your function, or when you do so on LOG_DEBUG level, since
that is not visible normally anyway.)

Lennart

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

Re: [systemd-devel] [RFC 07/12] gfx: add graphics layer

[Lennart Poettering]

On Wed, 27.11.13 19:48, David Herrmann (dh.herrm...@gmail.com) wrote:

 +typedef struct gfx_config_pipe gfx_config_pipe;
 +typedef struct gfx_connector gfx_connector;
 +typedef struct gfx_encoder gfx_encoder;
 +
 +/* clock-wise framebuffer rotation */
 +enum {
 +GFX_ROTATE_0,
 +GFX_ROTATE_90,
 +GFX_ROTATE_180,
 +GFX_ROTATE_270,
 +};
 +
 +struct gfx_config_pipe {
 +unsigned int config_id;
 +char **connectors;
 +char *mode;
 +unsigned int rotate;
 +
 +unsigned int disable : 1;

C99 bool plz! (here and everywhere else...)

 +memset(handles, 0, sizeof(handles));
 +memset(strides, 0, sizeof(strides));
 +memset(offsets, 0, sizeof(offsets));

zero(handles) is so much nicer... (here and everywhere...)

 +static int gfx_pipe_new(sd_gfx_pipe **out, sd_gfx_card *card, drmModeCrtc 
 *crtc, unsigned int connector_count) {
 +sd_gfx_pipe *pipe;
 +int r;
 +
 +pipe = calloc(1, sizeof(*pipe));
 +if (!pipe)
 +return log_oom();
 +
 +pipe-ref = 1;
 +pipe-card = card;
 +pipe-id = card-pipe_ids;
 +pipe-crtc_id = crtc-crtc_id;
 +pipe-page_flip_cnt = 1;
 +
 +pipe-connectors = calloc(connector_count, 
 sizeof(*pipe-connectors));
 +if (!pipe-connectors) {
 +r = log_oom();
 +goto err_pipe;
 +}
 +
 +pipe-want_connectors = calloc(connector_count, 
 sizeof(*pipe-want_connectors));
 +if (!pipe-want_connectors) {
 +r = log_oom();
 +goto err_connectors;
 +}
 +
 +pipe-connector_ids = calloc(connector_count, 
 sizeof(*pipe-connector_ids));
 +if (!pipe-connector_ids) {
 +r = log_oom();
 +goto err_want_connectors;
 +}
 +
 +pipe-want_connector_ids = calloc(connector_count, 
 sizeof(*pipe-want_connector_ids));
 +if (!pipe-want_connector_ids) {
 +r = log_oom();
 +goto err_connector_ids;
 +}
 +
 +r = gfx_plane_new_primary(pipe-primary, pipe);
 +if (r  0)
 +goto err_want_connector_ids;
 +
 +gfx_pipe_refresh(pipe, crtc);
 +
 +*out = pipe;
 +return 0;
 +
 +err_want_connector_ids:
 +free(pipe-want_connector_ids);
 +err_connector_ids:
 +free(pipe-connector_ids);
 +err_want_connectors:
 +free(pipe-want_connectors);
 +err_connectors:
 +free(pipe-connectors);
 +err_pipe:
 +free(pipe);
 +return r;
 +}
 +

For clean-up code like this I usually find it nicer to simply have a
destructor function that is robust to destruct half-initialized objects
and then just invoke this here.

 +/* framebuffer */
 +
 +typedef void (*sd_gfx_fb_unlink_fn) (sd_gfx_fb *fb, void *fn_data);
 +typedef void (*sd_gfx_fb_unpin_fn) (sd_gfx_fb *fb, void *fn_data);

For the types that actually feel like primitive types (in contrast to
objects), we usually appended a libc style _t to our names. 

I can't really say much about the actual drm stuff going on here, just
my usualy whining about logging from lib code, C99 bools, zero()...

Lennart

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

Re: [systemd-devel] [RFC 08/12] gfx: add monitor

[Lennart Poettering]

On Wed, 27.11.13 19:48, David Herrmann (dh.herrm...@gmail.com) wrote:

 While all previous sd-gfx interfaces are self-contained and can be used
 directly on selected devices, this adds an interface to connect them all
 together. The sd_gfx_monitor can be used to monitor a session for device
 hotplugging and other device events. It is optional but is supposed to be
 the foundation of all systemd-helpers that use sd-gfx.
 
 The main function of sd_gfx_monitor is to watch the system for udev-events
 of gpus and keyboards. For each of them, an sd_gfx_card or sd_gfx_kbd
 device is created and advertised to the application. Furthermore,
 systemd-localed integration is provided so keymap changes are immediately
 noticed and applied to active sd_gfx_kbd devices.
 
 An sd_gfx_monitor can run in two modes:
  - system mode: In system mode, no dbus, no logind and localed are assumed
 to be running and seat-information is ignored. This mode
 allows to run applications in initrds or emergency
 situations. It simply takes all devices it can find and
 tries to use them. However, this obviously requires to be
 root, otherwise, devices cannot be accessed.
  - session mode: In session mode, the monitor assumes to be running in an
  logind session. If not, it returns an error. The monitor
  will call TakeControl on the active session and get
  device-access via logind. Only devices attached to the
  session will be used and no you're not required to be
  root. The caller is responsible of setting up the session
  before spawning the monitor.
 
 Note that monitor setup is a blocking call as it is usually called during
 application setup (and making that async would have no gain). But at
 runtime, the monitor runs all dbus-calls and other calls asynchronously.
 
 The sd_gfx_monitor interface is designed for fallbacks and basic system
 applications. It does not allow per-device configurations or any advanced
 eye-candy. It is trimmed for usability and simplicity, and it is optimized
 for fallback/emergency situations. Thus, the monitor provides some basic
 configuration options via the kernel command-line. systemd.gpus= allows to
 filter the GPUs to be used (by default, *all* connected GPUs are used
 together). systemd.keymap= allows to change the keymap in case localed is
 configured incorrectly.
 
 The sd_gfx_monitor interfaces has the nice side-effect that all
 applications using it will share the same configuration. They will have
 the same monitor-setup, the same keymap setup and use the same devices. So
 if you system-boot fails, you can set systemd.XY= to boot with a working
 configuration and all systemd-internal applications will just work.
 
 If we ever export sd-gfx, most users probably want more configurability
 (like per-device keymaps) and thus will not use sd_gfx_monitor. However,
 for all fallbacks, it is the perfect base.

Hmm, this bit sounds a bit too high-level for including it in a library,
no? I mean, we can certainly share this bit inside of systemd, but this
sounds too specialized to ever become a public lib, no?

 +r = sd_bus_message_read_basic(m, SD_BUS_TYPE_UNIX_FD, fd);
 +if (r  0)
 +goto error;
 +
 +r = sd_bus_message_read_basic(m, SD_BUS_TYPE_BOOLEAN, paused);
 +if (r  0)
 +goto error;

Why in two steps? sd_bus_message_read(m, hb, fd, paused) should work too?

 +static int gfx_logind_resume_fn(sd_bus *bus, sd_bus_message *m, void *data, 
 sd_bus_error *err) {
 +sd_gfx_monitor *mon = data;
 +gfx_device *dev;
 +int r, fd;
 +uint32_t major, minor;
 +dev_t devt;
 +
 +r = sd_bus_message_read_basic(m, SD_BUS_TYPE_UINT32, major);
 +if (r  0)
 +goto error;
 +
 +r = sd_bus_message_read_basic(m, SD_BUS_TYPE_UINT32, minor);
 +if (r  0)
 +goto error;
 +
 +r = sd_bus_message_read_basic(m, SD_BUS_TYPE_UNIX_FD, fd);
 +if (r  0)
 +goto error;

Same here... One call should suffice...

 +static int gfx_logind_pause_fn(sd_bus *bus, sd_bus_message *m, void *data, 
 sd_bus_error *err) {
 +sd_gfx_monitor *mon = data;
 +gfx_device *dev;
 +int r;
 +const char *type;
 +uint32_t major, minor;
 +dev_t devt;
 +
 +r = sd_bus_message_read_basic(m, SD_BUS_TYPE_UINT32, major);
 +if (r  0)
 +goto error;
 +
 +r = sd_bus_message_read_basic(m, SD_BUS_TYPE_UINT32, minor);
 +if (r  0)
 +goto error;
 +
 +r = sd_bus_message_read_basic(m, SD_BUS_TYPE_STRING, type);
 +if (r  0)
 +goto error;

and here...

 +r = asprintf(v,
 + type='signal',
 + sender='org.freedesktop.login1',
 + 

Re: [systemd-devel] [RFC 12/12] console: add systemd-consoled

[Lennart Poettering]

On Wed, 27.11.13 19:48, David Herrmann (dh.herrm...@gmail.com) wrote:

 +
 +pid_t pty_new(unsigned short term_width, unsigned short term_height, 
 Terminal *t, Pty **out) {
 +Pty *pty;
 +pid_t pid;
 +int fd, comm[2], slave, r;
 +char d;
 +
 +pty = calloc(1, sizeof(*pty));
 +if (!pty)
 +return -ENOMEM;
 +
 +fd = posix_openpt(O_RDWR | O_NOCTTY | O_CLOEXEC | O_NONBLOCK);
 +if (fd  0) {
 +free(pty);
 +return -errno;
 +}
 +
 +r = sd_event_add_io(t-m-event,
 +fd,
 +EPOLLHUP | EPOLLERR | EPOLLIN | EPOLLOUT | 
 EPOLLET,
 +pty_io_fn,
 +pty,
 +pty-fd_source);
 +if (r  0) {
 +close(fd);
 +free(pty);
 +return r;
 +}
 +
 +r = sd_event_add_defer(t-m-event, pty_idle_fn, pty, 
 pty-idle_source);
 +if (r  0) {
 +sd_event_source_set_enabled(pty-fd_source, SD_EVENT_OFF);
 +sd_event_source_unref(pty-fd_source);
 +close(fd);
 +free(pty);
 +return r;
 +}
 +
 +r = pipe2(comm, O_CLOEXEC);
 +if (r  0) {
 +r = -errno;
 +sd_event_source_set_enabled(pty-idle_source, SD_EVENT_OFF);
 +sd_event_source_unref(pty-idle_source);
 +sd_event_source_set_enabled(pty-fd_source, SD_EVENT_OFF);
 +sd_event_source_unref(pty-fd_source);
 +close(fd);
 +free(pty);
 +return r;
 +}
 +
 +pid = fork();
 +if (pid  0) {
 +/* error */
 +pid = -errno;
 +close(comm[0]);
 +close(comm[1]);
 +sd_event_source_set_enabled(pty-idle_source, SD_EVENT_OFF);
 +sd_event_source_unref(pty-idle_source);
 +sd_event_source_set_enabled(pty-fd_source, SD_EVENT_OFF);
 +sd_event_source_unref(pty-fd_source);
 +close(fd);
 +free(pty);
 +return pid;

Grr. Just define a label to jump to to clean everything up that is
initialized, and skip over the bits that isn't. Duplicating the
destruction logic on every if block is just wrong...

Lennart

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

Re: [systemd-devel] [RFC 00/12] Bugfixes for CONFIG_VT=n

[Lennart Poettering]

On Wed, 27.11.13 19:48, David Herrmann (dh.herrm...@gmail.com) wrote:

Looks pretty good. Commented on the invidiaul patches, but mostly looks
good.

I am not sure about the whole approach of making this a potentially
public library though... Especially the monitor stuff sounds so
specific. Commiting to a stable API for that given that there are very
few other projects who could ever make use of this sounds dangerous at
best. Especially since the api exposes bit values for enums and things,
which makes me especially afraid... 

Lennart

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

[systemd-devel] Seeing logging as user

[Cecil Westerhof]

I have to give a presentation about systemd/journald. One of the things 
I want to show is that you do not need an administrator to see the log 
messages that are generated by you.


I did:
- makedir /var/log/journal
- systemctl kill -s SIGUSR1 systemd-journald

As user cecil I did:
printf Create an error\n | logger

When I as root do:
journalctl _UID=uid

I see:
Nov 27 23:26:38 Equus.Decebal.nl cecil[25091]: Create an error

But when I do the same as cecil, I see nothing. What is going wrong here?


Met vriendelijke groet,



Cecil Westerhof
Engineer
mobiel +31 - 6 - 25 00 38 81

--

Snow B.V.
Unix Specialists
De Ooyen 11
4191 PB Geldermalsen

http://www.snow.nl
tel. +31 - 345 - 65 66 66
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Seeing logging as user

[Kay Sievers]

On Thu, Nov 28, 2013 at 12:21 AM, Cecil Westerhof
cecil.wester...@snow.nl wrote:
 I have to give a presentation about systemd/journald. One of the things I
 want to show is that you do not need an administrator to see the log
 messages that are generated by you.

 I did:
 - makedir /var/log/journal
 - systemctl kill -s SIGUSR1 systemd-journald

 As user cecil I did:
 printf Create an error\n | logger

 When I as root do:
 journalctl _UID=uid

 I see:
 Nov 27 23:26:38 Equus.Decebal.nl cecil[25091]: Create an error

 But when I do the same as cecil, I see nothing. What is going wrong here?

http://www.freedesktop.org/software/systemd/man/systemd-journald.service.html#Access%20Control

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

Re: [systemd-devel] [RFC 02/12] ring: add basic ring-buffer helper

[Lucas De Marchi]

On Wed, Nov 27, 2013 at 4:48 PM, David Herrmann dh.herrm...@gmail.com wrote:
 This adds a very straightforward ring-buffer implementation to
 libsystemd-shared. Ring-buffers allow pushing data to, and pulling data
 from, both ends of a buffer without modifying existing/remaining buffer
 content. This is very useful for network buffers and asynchronous writes.

 This implementation only contains the most basic functions to push data
 onto the end of the buffer and pull data from the front. More helpers may
 be added once needed.
 ---
  Makefile.am   |   2 +
  src/shared/ring.c | 213 
 ++
  src/shared/ring.h |  54 ++
  3 files changed, 269 insertions(+)
  create mode 100644 src/shared/ring.c
  create mode 100644 src/shared/ring.h

 diff --git a/Makefile.am b/Makefile.am
 index 0119751..4aa2bdf 100644
 --- a/Makefile.am
 +++ b/Makefile.am
 @@ -768,6 +768,8 @@ libsystemd_shared_la_SOURCES = \
 src/shared/net-util.h \
 src/shared/errno-list.c \
 src/shared/errno-list.h \
 +   src/shared/ring.h \
 +   src/shared/ring.c \
 src/shared/syscall-list.c \
 src/shared/syscall-list.h

 diff --git a/src/shared/ring.c b/src/shared/ring.c
 new file mode 100644
 index 000..f60f098
 --- /dev/null
 +++ b/src/shared/ring.c
 @@ -0,0 +1,213 @@
 +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
 +
 +/***
 +  This file is part of systemd.
 +
 +  Copyright 2013 David Herrmann dh.herrm...@gmail.com
 +
 +  systemd is free software; you can redistribute it and/or modify it
 +  under the terms of the GNU Lesser General Public License as published by
 +  the Free Software Foundation; either version 2.1 of the License, or
 +  (at your option) any later version.
 +
 +  systemd is distributed in the hope that it will be useful, but
 +  WITHOUT ANY WARRANTY; without even the implied warranty of
 +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
 +  Lesser General Public License for more details.
 +
 +  You should have received a copy of the GNU Lesser General Public License
 +  along with systemd; If not, see http://www.gnu.org/licenses/.
 +***/
 +
 +#include errno.h
 +#include stdlib.h
 +#include string.h
 +#include sys/uio.h
 +
 +#include ring.h
 +#include util.h
 +
 +#define RING_MASK(_r, _v) ((_v)  ((_r)-size - 1))
 +
 +void ring_flush(Ring *r) {
 +r-start = 0;
 +r-end = 0;
 +}
 +
 +void ring_clear(Ring *r) {
 +free(r-buf);
 +memset(r, 0, sizeof(*r));
 +}
 +
 +/*
 + * Resize ring-buffer to size @nsize. @nsize must be a power-of-2, otherwise
 + * ring operations will behave incorrectly.
 + */
 +static int ring_resize(Ring *r, size_t nsize) {
 +char *buf;
 +
 +buf = malloc(nsize);
 +if (!buf)
 +return -ENOMEM;
 +
 +if (r-end == r-start) {
 +r-end = 0;
 +r-start = 0;
 +} else if (r-end  r-start) {
 +memcpy(buf, r-buf[r-start], r-end - r-start);
 +
 +r-end -= r-start;
 +r-start = 0;
 +} else {
 +memcpy(buf, r-buf[r-start], r-size - r-start);
 +memcpy(buf[r-size - r-start], r-buf, r-end);
 +
 +r-end += r-size - r-start;
 +r-start = 0;
 +}
 +
 +free(r-buf);
 +r-buf = buf;
 +r-size = nsize;
 +
 +return 0;
 +}
 +
 +/* Compute next higher power-of-2 of @v. Returns 4096 in case v is 0. */
 +static size_t ring_pow2(size_t v) {
 +size_t i;
 +
 +if (!v)
 +return 4096;
 +
 +--v;
 +
 +for (i = 1; i  8 * sizeof(size_t); i *= 2)
 +v |= v  i;
 +
 +return ++v;

If you are interested, take a look in
https://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/?id=3ba7f59e84857eb4dbe56a68fc7a3ffe8a650393
for a shorter and faster version of this.


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

Re: [systemd-devel] [systemd-commits] man/systemd-socket-proxyd.xml src/socket-proxy TODO

[David Timothy Strauss]

Wow, that was fast. I was working on my own patch to submit to the list
today, but I'll just consider it an education in config and exec
configuration.

I'll make sure everything looks good with the CoreOS folks, too.

Thanks!
On Nov 28, 2013 6:17 AM, Lennart Poettering lenn...@poettering.net
wrote:

 On Wed, 27.11.13 14:09, David Timothy Strauss (da...@davidstrauss.net)
 wrote:

   If this is about running multiple things in the same Network namespace,
   then there are certainly other, better ways to achieve the same. For
   example, we could introduce JoinPrivateNetwork=$SERVICE or so which
   would allow one service to join the network namespace of another. That
   makes this much smoother and more powerful, too.
 
  That would be wonderful. I'd love to shed the shell scripts.

 This is implemented now in JoinsNamespaceOf= and will affect both
 PrivateNetwork= and PrivateTmp=.

 I also took the liberty to revert the --listener patch, since it is
 unnecessary now, and I updated the examples in the man page to not use
 shell scripts anymore. I didn't test them though, please check if
 everything's OK now.

 Lennart

 --
 Lennart Poettering, Red Hat

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

Re: [systemd-devel] [RFC 04/12] build: add target to link binary sources

[David Herrmann]

Hi

On Wed, Nov 27, 2013 at 10:31 PM, Greg KH gre...@linuxfoundation.org wrote:
 On Wed, Nov 27, 2013 at 07:48:39PM +0100, David Herrmann wrote:
 In several situations we want to link a binary file into our executable
 and access it from our C code. The easiest way is to transform it into a
 C-array and compile it as usual. However, for large files (1MB) such
 compilations can take a considerable amount of time or even fail on
 low-memory systems.

 This adds a new automake-target to link binary sources directly. Instead
 of transforming it into a C-array, we simply use ld -r to create an
 object file via:
   ld -r -o my-source.bin.o --format=binary my-source.bin
 We also use -z noexecstack to mark my-source.bin.o to not require an
 executable stack.

 As we only want to support read-only data sources here, we do some
 post-processing to mark the object as read-only via:
   objcopy --rename-section .data=.rodata,alloc,load,readonly,data,contents 
 my-source.bin.o

 As libtool requires *.lo files, we cannot link this object-file
 directly. Thus, we also create a fake *.lo file for such objects which
 libtool can use. Note that libtool actually *requires* the comment-section
 in *.lo files (ugh?!) so we need to fake that, too.

 How to use this helper?
  - put your binary source file into the tree as:
  src/somewhere/something.bin
  - for the library you want to link that to, add this to mylib_LIBADD:
  src/somewhere/something.bin.lo
 This causes the helper to create src/somewhere/something.bin.[o,lo] and
 it will be linked as a normal object file.

 GNU-ld automatically creates 3 symbols for such objects, but the important
 symbols are:
   extern const char _binary_src_somewhere_something_bin_start[];
   extern const char _binary_src_somewhere_something_bin_end[];
 Use these to access start/end of the binary section.

 I tested this with in-tree and out-of-tree builds, with GNU-ld and
 GNU-gold and cross-compilation. All worked fine..

 That's crazy, very nice job in doing this :)

 I tried to do this a while ago, and gave up and just used a perl script
 and a .c file with a big array, like 'xxd -i' can create.  I think I'll
 steal this idea for other projects if you don't mind, as it makes
 things easier just to use the linker.

I figured that out about 1 year ago, took me way too long to get working.
Please, go ahead and copy it! And if any issue comes up, let me know.

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

Re: [systemd-devel] [systemd-commits] man/crypttab.xml

[Zbigniew Jędrzejewski-Szmek]

On Mon, Nov 25, 2013 at 12:31:24AM -0800, Lukas Nykryn wrote:
  man/crypttab.xml |2 --
  1 file changed, 2 deletions(-)
 
 New commits:
 commit 517dcac840fe8d5bf30a05c0084eff219af10a4a
 Author: Lukas Nykryn lnyk...@redhat.com
 Date:   Mon Nov 25 09:31:09 2013 +0100
 
 Revert man: suggest using hash= atribut for swap in example
 
 This reverts commit fa7abba2328eb2d23a7e27708f86f5013059ddcf.
Reason?

Zbyszek

 diff --git a/man/crypttab.xml b/man/crypttab.xml
 index fae39e7..90d8ce9 100644
 --- a/man/crypttab.xml
 +++ b/man/crypttab.xml
 @@ -363,8 +363,6 @@
  swap   /dev/sda7   /dev/urandom swap
  truecrypt  /dev/sda2   /etc/container_password  tcrypt
  hidden /mnt/tc_hidden  /null
 tcrypt-hidden,tcrypt-keyfile=/etc/keyfile/programlisting
 -paraNote that the default hash algorithm is 
 ripemd160. If you use your system
 -in FIPS mode, please specify supported hash 
 algorithm (e.g.: hash=sha1)./para
  /example
  /refsect1
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel