Re: [systemd-devel] Per session systemd?
Hi On Mon, Mar 3, 2014 at 8:30 AM, Yuxuan Shui yshu...@gmail.com wrote: Hi, This mail might be a little bit later for the topic, but I would like to share my thoughts anyway. Before systemd 206 was released, there are a few users (I don't know how many of them are there, but there's a page about it on archlinux's wiki. So I guess it's a well-known use case.) who use systemd to manage their sessions. For example, they will exec systemd in their ~/.xinitrc, and have systemd start all their X applications. systemd --user ist started by PAM for any proper user login. So you could fix that by just using a proper login manager (if you don't want the big ones, there's small stuff like 'slim', too). Even if you don't want these, I still recommend to do a PAM authentication in your startup script. This might even be via pam_rootok so you don't have to enter any password. I know this kind of use case has never been explicitly supported by systemd, but it was a really nice _accidental_ feature. However, after the cgroup changes made in the systemd 206 release, it became impossible to use systemd in such way. I saw some user complaints, but the systemd developers seemed unmoved. Maybe because the original purpose of systemd --user is to start per-user systemd instances. There're hacks to make systemd usable under a X session. But that's very complicated, and contains many pitfalls (User have to set a lot of environmental variables, and this makes logind unhappy since the systemd user instance is not in the same session as X). Besides, there're reasonable use cases which can't be covered by a per-user systemd instance, like periodically starting a graphic application. Why is that not possible with per-user instances? So, I wrote a very dirty hack for my systemd, and have been using it till today. I add a 'User=' property to a session, and have systemd chown the cgroup to the given user, so I can start systemd in my .xinitrc as I used to. I admit this is probably a very bad hack, and I'm not sure if it will still work after the soon coming cgroup rework. That's why I'm writing this mail. I want to point out the reason behind use systemd as a session manager, so you will probably understand why I want to do this and help me. Since I can't get this done by myself with my limited systemd knowledge. Any help will be appreciated. It will be better if you can convince me that I'm stupid and this feature is totally useless. What's the problem with per-user systemd besides startup synchronization? (which is fixed by pam..) Our concept basically boils down to treating sessions as a banal ref-count to a user. Instead of putting all the big stuff into a session, we now move it to the user. You can still have multiple sessions, but they share the bulk of data now. On the one hand, this is required for stuff like firefox that can only run once per user. On the other hand, it seems rather natural to share contexts between multiple logins of the same user. The same 'user' cannot sit in front of two machines at the same time, so why allow two independent logins? Anyhow, there's a lot going on right now and it'll take some time until this is all implemented. But so far, there haven't been any use-cases that cannot be solved with per-user systemd. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Per session systemd?
Hi On Mon, Mar 3, 2014 at 12:16 PM, Yuxuan Shui yshu...@gmail.com wrote: Hi, After reading some more mails and thinking about it a bit more, I seems to have a better understanding. I know that a per-user systemd is used to start service which should only be started once for every user. But I also want systemd to be able to start applications for every session (e.g. window manager), which is hard to do with the currect systemd --user implementation. The idea is to run your window-manager only once per user. If you want multiple logins with the same window-manager, then your window manager should support that, not systemd. You wm daemon should simply be able to serve multiple sessions from one instance. Thanks David I think there're two solutions here. 1) A per-session systemd instance. That's possibly the most simple solution. The changes needed is adding a 'User=' property to session unit, and give the change the ownership of the session cgroup to the given user. Then the user could start systemd after he start X (e.g. put systemd into .xinitrc). Also systemd probably have to read configuration files from a different position as the systemd --user (e.g. $XDG_CONFIG_HOME/systemd/session). One advantage of this solutions is that systemd will automatically have all the environment variables set up during the X startup sequence. 2) Let the per-user systemd start service in session. I think this is what David meant. I don't know what changes are needed in systemd to do this. Since the session cgroup is owned by root, maybe the ownership should be changed to the user? Or a new systemd API to start service in a given session? I don't know. Also it seems to be hard to maintain different sets of environment variables needed to start applications in different sessions. Correct me if I'm wrong. Regards, Yuxuan Shui. On Mon, Mar 3, 2014 at 4:46 PM, Yuxuan Shui yshu...@gmail.com wrote: Hi, On Mon, Mar 3, 2014 at 4:11 PM, David Herrmann dh.herrm...@gmail.com wrote: Hi On Mon, Mar 3, 2014 at 8:30 AM, Yuxuan Shui yshu...@gmail.com wrote: Hi, This mail might be a little bit later for the topic, but I would like to share my thoughts anyway. Before systemd 206 was released, there are a few users (I don't know how many of them are there, but there's a page about it on archlinux's wiki. So I guess it's a well-known use case.) who use systemd to manage their sessions. For example, they will exec systemd in their ~/.xinitrc, and have systemd start all their X applications. systemd --user ist started by PAM for any proper user login. So you could fix that by just using a proper login manager (if you don't want the big ones, there's small stuff like 'slim', too). Even if you don't want these, I still recommend to do a PAM authentication in your startup script. This might even be via pam_rootok so you don't have to enter any password. Yea, I know that. The problem is this instance is started once per every user. And this systemd instance don't belong to the same session as the logged-in user, causing problems I described below. I know this kind of use case has never been explicitly supported by systemd, but it was a really nice _accidental_ feature. However, after the cgroup changes made in the systemd 206 release, it became impossible to use systemd in such way. I saw some user complaints, but the systemd developers seemed unmoved. Maybe because the original purpose of systemd --user is to start per-user systemd instances. There're hacks to make systemd usable under a X session. But that's very complicated, and contains many pitfalls (User have to set a lot of environmental variables, and this makes logind unhappy since the systemd user instance is not in the same session as X). Besides, there're reasonable use cases which can't be covered by a per-user systemd instance, like periodically starting a graphic application. Why is that not possible with per-user instances? Yes, it's possible, but it has many pitfalls as I described. Here I mean if you don't use those hacks, you can't do things like periodically starting a graphic application So, I wrote a very dirty hack for my systemd, and have been using it till today. I add a 'User=' property to a session, and have systemd chown the cgroup to the given user, so I can start systemd in my .xinitrc as I used to. I admit this is probably a very bad hack, and I'm not sure if it will still work after the soon coming cgroup rework. That's why I'm writing this mail. I want to point out the reason behind use systemd as a session manager, so you will probably understand why I want to do this and help me. Since I can't get this done by myself with my limited systemd knowledge. Any help will be appreciated. It will be better if you can convince me that I'm stupid and this feature is totally useless. What's the problem with per-user
Re: [systemd-devel] [RFC] bus: add sd_bus_emit_object_{added, removed}()
ping?
On Tue, Feb 18, 2014 at 12:02 AM, David Herrmann dh.herrm...@gmail.com wrote:
The ObjectManager dbus interface provides an InterfacesAdded signal to
notify others about new interfaces that are added to an object. The same
signal is also used to advertise new objects (by adding the first
interface to a given object path) and delete them.
However, our internal helpers sd_bus_emit_interfaces_{added,removed}()
cannot properly deal with built-in interfaces like DBus.Properties as
there's no vtable for it. Therefore, to avoid callers to track these
internal interfaces, we provide two separate helpers which explicitly add
these interfaces to the signal.
sd_bus_emit_object_added() traverses the list of all vtables and fallback
vtables (making sure a fallback never overwrites a real vtable!) and also
adds built-in interfaces.
sd_bus_emit_object_removed(): WIP
---
Hi
This is untested and I just wanted to get some feedback whether that's the way
to go. Given the previous discussion we decided on two new entry-points to add
and remove objects. For convenience, I now tried to omit any char
**interfaces
argument to object_added() and just try to figure them out on my own.
However, on object_removed() I cannot do that as node_vtable_get_userdata() is
very likely to return 0 for all these objects. So there is no way to figure
out which interfaces actually existed on that thing. We would require users to
call it *before* destroying/unlinking the actual object. I don't know whether
that's ok to assume?
If not, we can just add a char **interfaces argument, but then it would
differ
from object_added().. Not sure what sounds better..
Cheers
David
src/libsystemd/sd-bus/bus-objects.c | 210
src/systemd/sd-bus.h| 2 +
2 files changed, 212 insertions(+)
diff --git a/src/libsystemd/sd-bus/bus-objects.c
b/src/libsystemd/sd-bus/bus-objects.c
index b116a5d..0c099a3 100644
--- a/src/libsystemd/sd-bus/bus-objects.c
+++ b/src/libsystemd/sd-bus/bus-objects.c
@@ -2469,6 +2469,216 @@ _public_ int sd_bus_emit_interfaces_removed(sd_bus
*bus, const char *path, const
return sd_bus_emit_interfaces_removed_strv(bus, path, interfaces);
}
+static int object_added_append_all_prefix(
+sd_bus *bus,
+sd_bus_message *m,
+Set *s,
+const char *prefix,
+const char *path,
+bool require_fallback) {
+
+_cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
+const char *previous_interface = NULL;
+struct node_vtable *c;
+struct node *n;
+void *u = NULL;
+int r;
+
+assert(bus);
+assert(m);
+assert(s);
+assert(prefix);
+assert(path);
+
+n = hashmap_get(bus-nodes, prefix);
+if (!n)
+return 0;
+
+LIST_FOREACH(vtables, c, n-vtables) {
+if (require_fallback !c-is_fallback)
+continue;
+
+r = node_vtable_get_userdata(bus, path, c, u, error);
+if (r 0)
+return r;
+if (bus-nodes_modified)
+return 0;
+if (r == 0)
+continue;
+
+if (!streq_ptr(c-interface, previous_interface)) {
+/* interface already handled by a previous run? */
+if (set_get(s, c-interface))
+continue;
+
+/* prevent fallbacks from overwriting specific objs
*/
+r = set_put(s, c-interface);
+if (r 0)
+return r;
+
+if (previous_interface) {
+r = sd_bus_message_close_container(m);
+if (r 0)
+return r;
+
+r = sd_bus_message_close_container(m);
+if (r 0)
+return r;
+}
+
+r = sd_bus_message_open_container(m, 'e', sa{sv});
+if (r 0)
+return r;
+
+r = sd_bus_message_append_basic(m, 's',
c-interface);
+if (r 0)
+return r;
+
+r = sd_bus_message_open_container(m, 'a', {sv});
+if (r 0)
+return r;
+
+previous_interface = c-interface;
+}
+
+r = vtable_append_all_properties(bus, m, path, c, u, error);
+if (r 0)
+return r
Re: [systemd-devel] systemd-backlight and backlight level 0
Hi On Wed, Mar 5, 2014 at 6:46 PM, Josh Triplett j...@joshtriplett.org wrote: systemd-backlight saves backlight levels on shutdown, and restores them on startup. However, on some systems, backlight level 0 actually turns the backlight *off*; this can potentially make the system unusable. Complicating matters, on most systems, nothing pays attention to the brightness adjustment keys in text mode. I'd suggest one or both of the following two changes, to avoid a painful failure mode: - systemd-backlight should avoid saving/restoring a backlight level of 0, and have a minimum backlight level. (Possibly overridable via configuration, for people who *really* want to restore backlight level 0.) Never restoring val==0 seems fine to me. - Something ought to listen to the brightness keys (and perhaps other hotkeys) in pure text mode. systemd seems like a good place for such a something to live. We cannot do that. It requires keymap-handling (as brightness keys are handled on the keysym, not keycode level) and this is exclusive territory of the compositor (or other foreground session controllers). Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd-backlight and backlight level 0
Hi On Wed, Mar 5, 2014 at 8:31 PM, Josh Triplett j...@joshtriplett.org wrote: On Wed, Mar 05, 2014 at 07:10:51PM +0100, Lennart Poettering wrote: On Wed, 05.03.14 09:46, Josh Triplett (j...@joshtriplett.org) wrote: systemd-backlight saves backlight levels on shutdown, and restores them on startup. However, on some systems, backlight level 0 actually turns the backlight *off*; this can potentially make the system unusable. Complicating matters, on most systems, nothing pays attention to the brightness adjustment keys in text mode. I'd suggest one or both of the following two changes, to avoid a painful failure mode: - systemd-backlight should avoid saving/restoring a backlight level of 0, and have a minimum backlight level. (Possibly overridable via configuration, for people who *really* want to restore backlight level 0.) To deal with situations like this there's systemd.restore_state=0 on the kernel cmdline, see kernel-command-line(7). Yeah, I've seen that one; however, having to reboot the system and change the kernel command line to unbreak it seems less ideal than having the system avoid broken states to begin with. I'd expect this to be set on the recovery boot option. At I know some distros always provide two boot entries and to me this seems like the right place to set it. I could be convinced to fix brightness level 0 to 1 when restoring. But then again, I fear the next people will come then and say 1 is only marginally better than 0, i want the minimum to be 16!... Or even others saying Dude, I only got 3 brightness levels, and you took one away from me So I am not enthusiastic about the idea... Given the choice between maximum flexibility and never making the system unusable, I'll take the latter. Not that hard to make it configurable, if that proves necessary. On restore, I'd suggest reading max_brightness, and if the stored value falls under a threshold of ceil(max_brightness/SOME_DIVISOR), restore it to that value instead. (Ideally there should be some way to ask the driver what level of brightness will produce a non-zero value in actual_brightness, but no such mechanism seems to exist.) Does that sound reasonable? - Something ought to listen to the brightness keys (and perhaps other hotkeys) in pure text mode. systemd seems like a good place for such a something to live. That's definitely a job for the DE I am sure, so that can it do an OSD and all the other stuff. We do power button handling in logind only because what it does is relatively important and really close to the system lifecycle... But brightness keys (or volume keys..) are not close at all. I am really sure that that's for the DEs to handle, not us. DEs don't handle the text consoles. However, it does sound like this will have to wait for kmscon or equivalent. Yepp. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Suspending access to opened/active /dev/nodes during application runtime
Hi On Fri, Mar 7, 2014 at 7:45 PM, Lukasz Pawelczyk hav...@gmail.com wrote: Problem: Has anyone thought about a mechanism to limit/remove an access to a device during an application runtime? Meaning we have an application that has an open file descriptor to some /dev/node and depending on *something* it gains or looses the access to it gracefully (with or without a notification, but without any fatal consequences). Example: LXC. Imagine we have 2 separate containers. Both running full operating systems. Specifically with 2 X servers. Both running concurrently of course. Both need the same input devices (e.g. we have just one mouse). This creates a security problem when we want to have completely separate environments. One container is active (being displayed on a monitor and controlled with a mouse) while the other container runs evtest /dev/input/something and grabs the secret password user typed in the other. Solutions: The complete solution would comprise of 2 parts: - a mechanism that would allow to temporally hide a device from an open file descriptor. - a mechanism for deciding whether application/process/namespace should have an access to a specific device at a specific moment Let's focus on the first problem only, as it would need to be solved first anyway. I haven't found anything that would allow me to do it. There are a lot mechanisms that make it possible to restrict an access during open(): - DAC - ACL (controlled by hand or with uaccess) - LSM (in general) - device cgroups But all of those can't do a thing when the device is already opened and an application has a file descriptor. I don't see such mechanism in kernel sources either. I do imagine that it would not be possible for every device to handle such a thing (dri comes to mind) without breaking something (graphics card state in dri example). But there is class of simple input/output devices that would handle this without problems. I did implement some proof-of-concept solution for an evdev driver by allowing or disallowing events that go to evdev_client structure using some arbitrary condition. But this is far from a generic solution. My proof-of-concept is somewhat similar to this (I just found it): http://www.spinics.net/lists/linux-input/msg25547.html Though a little bit wider in scope. But neither is flawless nor generic. Has anyone had any thoughts about a similar problem? Lennart and Greg have already answered most of this, few notes from me: * EVIOCREVOKE and DRM_SET_MASTER/DROP_MASTER are real. We use them. They solve your problem for gfx and input devices. * EVIOCMUTE is *bad*. It is a privileged ioctl compared to EVIOCREVOKE, so we've never merged it. It neither has major advantages over revoke. So use EVIOCREVOKE. * A generic frevoke() syscall would solve all is, but is unlikely to ever appear upstream. Cheers David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC] bus: add sd_bus_emit_object_{added, removed}()
Hi
On Tue, Mar 11, 2014 at 7:27 PM, Lennart Poettering
lenn...@poettering.net wrote:
On Tue, 18.02.14 00:02, David Herrmann (dh.herrm...@gmail.com) wrote:
Sorry for the late review!
However, on object_removed() I cannot do that as node_vtable_get_userdata()
is
very likely to return 0 for all these objects. So there is no way to figure
out which interfaces actually existed on that thing. We would require users
to
call it *before* destroying/unlinking the actual object. I don't know whether
that's ok to assume?
I would assume that it is OK to assume that. People should call this
function before half-destroying their object. I mean we are not reading
the properties after all, just the interfaces and I think that should be
quite OK to require.
Certainly something to document though one day...
Yepp, will add docs once I commit it.
+static int object_added_append_all_prefix(
+sd_bus *bus,
+sd_bus_message *m,
+Set *s,
+const char *prefix,
+const char *path,
+bool require_fallback) {
+
+if (!streq_ptr(c-interface, previous_interface)) {
+/* interface already handled by a previous run? */
+if (set_get(s, c-interface))
+continue;
We actually allow multiple vtables with the same interface, and order
them together in the vtable list, so that we can iterate through them
easily with trivial duplicate reduction. Keeping a Set object here
appears unnecessary? Or did I miss something here?
A single call to object_added_append_all_prefix() doesn't need Set
*s as it's ordered (as you said). The problem is more subtle, see
object_added_append_all(), where I do this:
+r = object_added_append_all_prefix(bus, m, s, path, path, false);
+if (r 0)
+return r;
+if (bus-nodes_modified)
+return 0;
+
+prefix = alloca(strlen(path) + 1);
+OBJECT_PATH_FOREACH_PREFIX(prefix, path) {
+r = object_added_append_all_prefix(bus, m, s, prefix,
path, true);
+if (r 0)
+return r;
+if (bus-nodes_modified)
+return 0;
+}
First I call append_all_prefix() with the exact path, which cannot
ever fire the if (set_get()) protection, as it's the first run.
However, the prefix-runs which I do afterwards will traverse the
parents, which might *also* implement the interface. They use
require_fallback == true, so usually we're fine. However, if you
implement an interface explicitly on a child, but as fallback on the
parents, I _must_ avoid adding the interface twice. At least that's my
understanding of the vtable API, or should these be merged?
I looked at interfaces_added_append_one(), which skips parents if the
node already implements the interface. Therefore, I assumed I have to
skip parents, too. But I still need to traverse the parents, because
they might implement different interfaces that I haven't found, yet.
Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC][PATCH] sd-bus: split ref-counting of queues from ref-counting of the rest of the sd-bus object
Hi
On Sun, Mar 23, 2014 at 4:49 PM, Tom Gundersen t...@jklm.no wrote:
Introduce a new ref-count, n_ref_queues, which only protects the {r,w}queue of
a bus, and introduce bus_{un,}ref(), which are only available internally, and
which do not protect these queues.
Make sure that sd_bus_message object do not call sd_bus_ref(), but only the
internal bus_ref(). This is ok as the {r,w}queues should never be accessed via
the message object (as doing so would anyway not be thread-safe).
When the refcount on the queues reaches zero (even thought the refcount on the
bus itself may not, due to references held by messages in the queues), the
queues are flushed and their messages unref'ed. This avoids problems due to
mutual references between busses and their queued messages. In particular we
don't get an sd_bus_unref() - sd_bus_message_unref() - sd_bus_unref()
call-chain.
Moreover, we can now enforce that sd_bus_{un,}ref() is only ever called from
the same thread as created the bus (whereas bus_unref() may be called from a
different thread, as part of unref'ing a message being handled in a worker
thread).
Code looks good and it should work this way. But as I said earlier,
I'd prefer if we could avoid dual-refcounts and instead take the refs
on the object we _actually_ want. Messages don't need the -bus
pointer at all except for memfds, as far as I can see. So instead of
introducing a circular dependency bus-m-bus, why not take a
reference to the kdbus FD instead? Either by dup()ing the fd or
extracting it to a separate struct/refcount. We can still take an
sd_bus argument in message allocations, but just avoid ref'ing them.
With the current approach, m-bus becomes a pseudo reference as it's
not equivalent to real references. If we forced messages to be
referenced on the bus they're queued on, we could fix this more
easily, but we don't have this requirement. Therefore, I really wonder
whether keeping m-bus is making anyone happy?
Anyhow, I wanna hear first what Lennart had planned with m-bus..
maybe he can shed some light on this.
Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] use systemd.debug on the kernel command line, not debug
Hi On Thu, Apr 3, 2014 at 11:08 AM, Tom Gundersen t...@jklm.no wrote: Or is there actually a bug going on here? My impression from reading related discussions was that systemd.log_level=debug loglevel=debug triggers some bug (so in particular debug now triggers the bug). It was not clear to me whether boot itself hangs, or if log-in becomes impossible. Either way, I'm not able to reproduce it, so if anyone has any more info on this, that would be useful. There was a bug there which triggered the assertions. However, it has been fixed in -git for quite some time, so with -git systemd will _not_ log an unreasonable amount of information. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Add Mir to the list of session types
Hi On Thu, Apr 3, 2014 at 10:46 PM, Robert Ancell robert.anc...@canonical.com wrote: Add Mir to the list of session types. This is implemented for LightDM in lp:~robert-ancell/lightdm/xdg-session-desktop [1]. [1] https://code.launchpad.net/~robert-ancell/lightdm/xdg-session-desktop/+merge/214108 --- man/pam_systemd.xml | 5 +++-- man/sd_session_is_active.xml | 6 +++--- src/login/logind-session.c | 1 + src/login/logind-session.h | 1 + src/systemd/sd-login.h | 2 +- 5 files changed, 9 insertions(+), 6 deletions(-) Applied and pushed. But please use git-send-email next time as the white-spaces are all broken in that patch. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] names: take the registry write lock in kdbus_name_release()
Hi On Fri, Apr 18, 2014 at 3:16 AM, Djalal Harouni tix...@opendz.org wrote: Take the write lock in kdbus_name_release() instead of kdbus_cmd_name_release() in order to reduce the lock hold time. This change permits to convert the kdbus_bus_find_conn_by_id() call to kdbus_conn_find_peer() since the bus lock will also be taken later in kdbus_name_release(). Another advantage is that now kdbus_cmd_name_release() and kdbus_name_release() have the same semantic of kdbus_cmd_name_acquire() and kdbus_name_acquire() Signed-off-by: Djalal Harouni tix...@opendz.org Looks good to me. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Check the right variable for failed open()
Hi
On Thu, May 8, 2014 at 12:57 PM, Łukasz Stelmach l.stelm...@samsung.com wrote:
---
I am not quite sure but checking r in this if does not make much sense.
Nice catch. Applied.
Thanks
David
src/core/cgroup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 3aeaf56..29ab645 100644
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -920,7 +920,7 @@ int manager_setup_cgroup(Manager *m) {
safe_close(m-pin_cgroupfs_fd);
m-pin_cgroupfs_fd = open(path,
O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOCTTY|O_NONBLOCK);
-if (r 0) {
+if (m-pin_cgroupfs_fd 0) {
log_error(Failed to open pin file: %m);
return -errno;
}
--
1.9.1
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd pam and O_CLOEXEC problem
Hi On Mon, May 12, 2014 at 1:27 PM, dedede gfgfgf trtrtrtrtrtr s.kabano...@mail.ru wrote: Hello Colin. I am speaking about descriptors in systemd pam module. Application which use pam can not control it. Because of: session_fd = dup(session_fd); in pam-module.c we have now descriptor which does not have O_CLOEXEC set. So it will not be closed automatically during exec of children processes and because of this logind will not know that process which opened pam session exited. This behavior is intentional. This way we can track the main processes of the session and get notified when they died. You should be able to close the session by using pam_close_session(). Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd pam and O_CLOEXEC problem
Hi On Mon, May 12, 2014 at 1:49 PM, Sergei Kabanov s.kabano...@mail.ru wrote: Hello But all children will have that descriptor open. So when parent process will die logind will not be notified. And? That's intentional. If you want to close the session if the main process dies, use something like /bin/login does: call pam_close_session() on SIGCHLD. Note that if we set O_CLOEXEC, then the exec() after the pam-dance will immediately close the FIFO, thus also close the session. So we cannot set O_CLOEXEC. So please call pam_close_session() to notify systemd about session-deaths, or use something like this: for i in /proc/self/fd/* ; do close($i); done to close all file-descriptors before you exec(). But this is really just a nasty hack. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] GDM session is not activated
Hi On Thu, May 22, 2014 at 9:19 PM, Oleg Samarin osamari...@gmail.com wrote: I have two-seat configuration on my PC. The problem is that when I log in to GDM session on seat0, it rests being not active, and I can not activate it with 'loginctl activate' command. gdm is responsible of setting XDG_VTNR before doing the pam-authentication. Recent gdm version do this, so I guess this is a bug with your systemd+gdm combination. Can you tell me the systemd and gdm version you are running? The patch you're referring to has long been superceded, so it's hard to tell what's going wrong. Can you also paste your environment? (bash: export) Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 17/17] networkd: add dhcp server support
Hi On Tue, May 27, 2014 at 12:41 PM, Michael Biebl mbi...@gmail.com wrote: 2014-05-27 4:38 GMT+02:00 Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl: before looking at the code, a couple of general questions: - does the DHCP server have to be part of networkd? Isn't the job of acquiring addresses and giving out addresses separate and shouldn't two different processes be responsible? I have to agree with Zbigniew here. This looks like feature creep. I strongly disagree. One major example is Wifi-P2P which requires a DHCP-Server for ad-hoc P2P connections. A network-daemon manages the local address-space, so it should also be responsible of assigning those ranges to an ad-hoc DHCP server. In most current network-solutions it is awfully hard to integrate ad-hoc DHCP networks. Yes, everyone can configure a static pre-assigned DHCP range, but that's not going to work with dynamically created networks. This includes p2p-connections like Wifi-P2P, but also stuff like tethering, virtual devices and containers. Besides, the DHCP-server implementation isn't that much more complex than the client-side. I mean ConnMan requires ~1000 lines for that, I haven't looked at Toms patches so far, though. I'd really like to hear why exactly you guys think this is feature-creep? Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] joystick doesn't respect seat rule
Hi On Wed, May 28, 2014 at 11:07 PM, Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl wrote: On Wed, May 28, 2014 at 09:22:23PM +0200, Floris wrote: Op Wed, 28 May 2014 20:38:17 +0200 schreef Mantas Mikulėnas graw...@gmail.com: So the seat assignment works fine – gdm is added to the ACL. The problem is that the devnode is *world-readable*. and how can we fix this? It has always been like this: commit d638611c44672dea8d8a498dbe84c7d650ef2194 Author: Kay Sievers kay.siev...@vrfy.org Date: Fri Aug 10 12:36:59 2007 +0200 rules: add default rules I guess that the assumption is that this is relatively safe to allow public reading of joystick and mouse input. Kay, please correct me if necessary. Correct. joydev devices are always world-readable as games (SDL, steam, ...) rely on the device-nodes. There are some things to note about joydevs: 1) They are deprecated! No features are added, any new application is advised to use evdev exclusively (event devices). Same is true for mousedev and others. Rule of thumb is: evdev is the only node you should use. 2) Input handling other than mouse (which includes trackpads and touchscreens) and keyboard has never been figured out properly. There is XInput, XInput2 and more, but games still use joydev directly. They ignore seat assignments, they ignore permissions, they ignore evdev. It's a mess and there is currently no solution. No, there isn't even anyone working on it. It's a complex task. So unless someone wants to tackle it, we have to deal with the consequences. This includes: applications will ignore seat-assignments for joysticks, gamepads and more. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Why do we must use the close_all_fds(3) function in some files?
Hi
On Mon, Jun 16, 2014 at 2:32 PM, Geunsik Lim geunsik@gmail.com wrote:
Hi all,
Recently, i checked that there are some of the close_all_fds functions as
follows
Why we Systemd run this functions? Whey this functions need Systemd's
management?
invain@u1204lgs:/sandbox/tizentvfolder/systemd$ grep -R close_all_fds ./*
./src/nspawn.c:close_all_fds(NULL, 0);
./src/util.c:int close_all_fds(const int except[], unsigned n_except) {
./src/util.c:close_all_fds(NULL, 0);
./src/main.c:close_all_fds(NULL, 0);
./src/spawn-agent.c:close_all_fds(NULL, 0);
./src/execute.c:err = close_all_fds(socket_fd = 0 ?
socket_fd : fds,
./src/execute.c:err = close_all_fds(fds, n_fds);
./src/util.h:int close_all_fds(const int except[], unsigned n_except);
I didn't look for all occurrences, but usually this function is a
safety net: We set O_CLOEXEC on all FDs, therefore, on execve() they
get closed. However, in case we missed this somewhere, close_all_fds()
destroys those FDs for us. Furthermore, it also destroys any global
fds (stdin/stdout/...) in case we don't want to leak them into our
child.
Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 07/11] Support init=/bin/bash on the kernel command line
Hi On Mon, Jun 16, 2014 at 2:42 PM, Dr. Werner Fink wer...@suse.de wrote: On Fri, Jun 13, 2014 at 05:49:24PM +0200, Tom Gundersen wrote: On Fri, Jun 13, 2014 at 4:41 PM, Werner Fink wer...@suse.de wrote: From: Ruediger Oertel r...@suse.de Process 1 (aka init) needs to be started with an empty signal mask. That includes the process 1 that's started after the initrd is finished. When the initrd is using systemd (as it does with dracut based initrds) then it is systemd that calls the real init. Normally this is systemd again, except when the user uses for instance init=/bin/bash on the kernel command line. Why is this necessary for /bin/bash, but not for /lib/systemd/systemd? Please include the explanation in the commit message. I do not understand your question. AFAIK the signal mask is set by systemd accordingly to man:systemd.service(5) ... the only problem with this schem is that /bin/bash nor any other shell does reset its signal mask. I guess that systemd will support the init=/bin/bash on the kernel command line. IMHO this requires a clean signal mask. The question was, why is this fix related to init=/bin/bash? What does bash do different than systemd that it requires this fix? That information should be placed in the commit-message. The fix itself looks good and if you called it restore signal-mask before executing init= it'd be fine. However, calling it support init=/bin/bash implies that this is only needed for init=/bin/bash. This, however, is just a side-effect of this fix, because systemd simply doesn't care for the initial sigmask. I'll fix up the commit-msg and apply it. No need to resend. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/3] shared: add generic IPC barrier
/null +++ b/src/shared/barrier.c @@ -0,0 +1,442 @@ +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/ + +/*** + This file is part of systemd. + + Copyright 2014 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 limits.h +#include poll.h +#include stdbool.h +#include stdint.h +#include stdio.h +#include stdlib.h +#include string.h +#include sys/eventfd.h +#include sys/types.h +#include unistd.h + +#include barrier.h +#include macro.h +#include util.h + +/** + * Barriers + * This barrier implementation provides a simple synchronization method based + * on file-descriptors that can safely be used between threads and processes. A + * barrier object contains 2 shared counters based on eventfd. Both processes + * can now place barriers and wait for the other end to reach a random or + * specific barrier. + * Barriers are numbered, so you can either wait for the other end to reach any + * barrier or the last barrier that you placed. This way, you can use barriers + * for one-way *and* full synchronization. Note that even-though barriers are + * numbered, these numbers are internal and recycled once both sides reached the + * same barrier (implemented as a simple signed counter). It is thus not + * possible to address barriers by their ID. + * + * Barrier-API: Both ends can place as many barriers via barrier_place() as + * they want and each pair of barriers on both sides will be implicitly linked. + * Each side can use the barrier_wait/sync_*() family of calls to wait for the + * other side to place a specific barrier. barrier_wait_next() waits until the + * other side calls barrier_place(). No links between the barriers are + * considered and this simply serves as most basic asynchronous barrier. + * barrier_sync_next() is like barrier_wait_next() and waits for the other side + * to place their next barrier via barrier_place(). However, it only waits for + * barriers that are linked to a barrier we already placed. If the other side + * already placed more barriers than we did, barrier_sync_next() returns + * immediately. + * barrier_sync() extends barrier_sync_next() and waits until the other end + * placed as many barriers via barrier_place() as we did. If they already placed + * as many as we did (or more), it returns immediately. + * + * Additionally to basic barriers, an abortion event is available. + * barrier_abort() places an abortion event that cannot be undone. An abortion + * immediately cancels all placed barriers and replaces them. Any running and + * following wait/sync call besides barrier_wait_abortion() will immediately + * return false on both sides (otherwise, they always return true). + * barrier_abort() can be called multiple times on both ends and will be a + * no-op if already called on this side. + * barrier_wait_abortion() can be used to wait for the other side to call + * barrier_abort() and is the only wait/sync call that does not return + * immediately if we aborted outself. It only returns once the other side + * called barrier_abort(). + * + * Barriers can be used for in-process and inter-process synchronization. + * However, for in-process synchronization you could just use mutexes. + * Therefore, main target is IPC and we require both sides to *not* share the FD + * table. If that's given, barriers provide target tracking: If the remote side + * exit()s, an abortion event is implicitly queued on the other side. This way, + * a sync/wait call will be woken up if the remote side crashed or exited + * unexpectedly. However, note that these abortion events are only queued if the + * barrier-queue has been drained. Therefore, it is safe to place a barrier and + * exit. The other side can safely wait on the barrier even though the exit + * queued an abortion event. Usually, the abortion event would overwrite the + * barrier, however, that's not true for exit-abortion events. Those are only + * queued if the barrier-queue is drained (thus, the receiving side has placed + * more barriers than the remote side). + */ + +/** + * barrier_init() - Initialize a barrier object + * @obj: barrier to initialize + * + * This initializes a barrier object. The caller is responsible of allocating + * the memory and keeping it valid. The memory does not have to be zeroed + * beforehand. + * Two eventfd objects
[systemd-devel] [PATCH 2/3] nspawn: use Barrier API instead of eventfd-util
The Barrier-API simplifies cross-fork() synchronization a lot. Replace the
hard-coded eventfd-util implementation and drop it.
Compared to the old API, Barriers also handle exit() of the remote side as
abortion. This way, segfaults will not cause the parent to deadlock.
EINTR handling is currently ignored for any barrier-waits. This can easily
be added, but it isn't needed so far so I dropped it. EINTR handling in
general is ugly, anyway. You need to deal with pselect/ppoll/... variants
and make sure not to unblock signals at the wrong times. So genrally,
there's little use in adding it.
---
Makefile.am | 2 -
src/nspawn/nspawn.c | 136 -
src/shared/eventfd-util.c | 169 --
src/shared/eventfd-util.h | 43
4 files changed, 60 insertions(+), 290 deletions(-)
delete mode 100644 src/shared/eventfd-util.c
delete mode 100644 src/shared/eventfd-util.h
diff --git a/Makefile.am b/Makefile.am
index 039a83e..d7b0f90 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -832,8 +832,6 @@ libsystemd_shared_la_SOURCES = \
src/shared/barrier.h \
src/shared/async.c \
src/shared/async.h \
- src/shared/eventfd-util.c \
- src/shared/eventfd-util.h \
src/shared/copy.c \
src/shared/copy.h \
src/shared/base-filesystem.c \
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index bad93a5..e75cc28 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -40,7 +40,6 @@
#include sys/un.h
#include sys/socket.h
#include linux/netlink.h
-#include sys/eventfd.h
#include net/if.h
#include linux/veth.h
#include sys/personality.h
@@ -84,12 +83,12 @@
#include def.h
#include rtnl-util.h
#include udev-util.h
-#include eventfd-util.h
#include blkid-util.h
#include gpt.h
#include siphash24.h
#include copy.h
#include base-filesystem.h
+#include barrier.h
#ifdef HAVE_SECCOMP
#include seccomp-util.h
@@ -3074,12 +3073,18 @@ int main(int argc, char *argv[]) {
for (;;) {
ContainerStatus container_status;
-int eventfds[2] = { -1, -1 };
+_barrier_destroy_ Barrier barrier = { };
struct sigaction sa = {
.sa_handler = nop_handler,
.sa_flags = SA_NOCLDSTOP,
};
+r = barrier_init(barrier);
+if (r 0) {
+log_error(Cannot initialize IPC barrier: %s,
strerror(-r));
+goto finish;
+}
+
/* Child can be killed before execv(), so handle SIGCHLD
* in order to interrupt parent's blocking calls and
* give it a chance to call wait() and terminate. */
@@ -3095,9 +3100,9 @@ int main(int argc, char *argv[]) {
goto finish;
}
-pid = clone_with_eventfd(SIGCHLD|CLONE_NEWNS|
- (arg_share_system ? 0 :
CLONE_NEWIPC|CLONE_NEWPID|CLONE_NEWUTS)|
- (arg_private_network ? CLONE_NEWNET :
0), eventfds);
+pid = syscall(__NR_clone, SIGCHLD|CLONE_NEWNS|
+ (arg_share_system ? 0 :
CLONE_NEWIPC|CLONE_NEWPID|CLONE_NEWUTS)|
+ (arg_private_network ? CLONE_NEWNET
: 0), NULL);
if (pid 0) {
if (errno == EINVAL)
log_error(clone() failed, do you have
namespace support enabled in your kernel? (You need UTS, IPC, PID and NET
namespacing built in): %m);
@@ -3126,6 +3131,8 @@ int main(int argc, char *argv[]) {
};
char **env_use;
+barrier_set_role(barrier, BARRIER_CHILD);
+
envp[n_env] = strv_find_prefix(environ, TERM=);
if (envp[n_env])
n_env ++;
@@ -3151,26 +3158,26 @@ int main(int argc, char *argv[]) {
}
log_error(Failed to open console: %s,
strerror(-k));
-goto child_fail;
+_exit(EXIT_FAILURE);
}
if (dup2(STDIN_FILENO, STDOUT_FILENO) != STDOUT_FILENO
||
dup2(STDIN_FILENO, STDERR_FILENO) !=
STDERR_FILENO) {
log_error(Failed to duplicate console: %m);
-goto child_fail;
+_exit(EXIT_FAILURE);
}
if (setsid() 0) {
log_error(setsid() failed: %m);
-goto child_fail;
+_exit(EXIT_FAILURE);
[systemd-devel] [PATCH 0/3] Cross fork() synchronization via Barrier API
Hi While working on a PTY helper I needed a synchronized fork() where parent and child can both perform their setup before either proceeds. src/shared/eventfd-util.c already provides a coarse API for that, but lacks many features like implicit exit() handling. Therefore, I went ahead and wrote barrier.[ch] based on eventfd-util. Patch #1 adds the Barrier API with extensive tests, #2 converts nspawn to use it and #3 adds a PTY helper as another example. As I'm one of these old guys (cough) who never used containers, testing is welcome! But I'll try to setup nspawn as soon as possible. Code is also available at fdo in my @ui branch: http://cgit.freedesktop.org/~dvdhrm/systemd/log/?h=ui Comments welcome! David David Herrmann (3): shared: add generic IPC barrier nspawn: use Barrier API instead of eventfd-util shared: add PTY helper .gitignore| 2 + Makefile.am | 20 +- src/nspawn/nspawn.c | 136 +--- src/shared/barrier.c | 442 + src/shared/barrier.h | 97 + src/shared/eventfd-util.c | 169 --- src/shared/eventfd-util.h | 43 src/shared/pty.c | 542 ++ src/shared/pty.h | 61 ++ src/test/test-barrier.c | 460 +++ src/test/test-pty.c | 143 11 files changed, 1825 insertions(+), 290 deletions(-) create mode 100644 src/shared/barrier.c create mode 100644 src/shared/barrier.h delete mode 100644 src/shared/eventfd-util.c delete mode 100644 src/shared/eventfd-util.h create mode 100644 src/shared/pty.c create mode 100644 src/shared/pty.h create mode 100644 src/test/test-barrier.c create mode 100644 src/test/test-pty.c -- 2.0.1 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 3/3] shared: add PTY helper
This Pty API wraps the ugliness that is POSIX PTY. It takes care of:
- edge-triggered HUP handling (avoid heavy CPU-usage on vhangup)
- HUP vs. input-queue draining (handle HUP _after_ draining the whole
input queue)
- SIGCHLD vs. HUP (HUP is no reliable way to catch PTY deaths, always
use SIGCHLD. Otherwise, vhangup() and friends will break.)
- Output queue buffering (async EPOLLOUT handling)
- synchronous setup (via Barrier API)
At the same time, the PTY API does not execve(). It simply fork()s and
leaves everything else to the caller. Usually, they execve() but we
support other setups, too.
This will be needed by multiple UI binaries (systemd-console, systemd-er,
...) so it's placed in src/shared/. It's not strictly related to
libsystemd-ui, so it's not included there.
---
.gitignore | 1 +
Makefile.am | 9 +
src/shared/pty.c| 542
src/shared/pty.h| 61 ++
src/test/test-pty.c | 143 ++
5 files changed, 756 insertions(+)
create mode 100644 src/shared/pty.c
create mode 100644 src/shared/pty.h
create mode 100644 src/test/test-pty.c
diff --git a/.gitignore b/.gitignore
index 5289f0e..d942691 100644
--- a/.gitignore
+++ b/.gitignore
@@ -201,6 +201,7 @@
/test-path-util
/test-prioq
/test-ratelimit
+/test-pty
/test-replace-var
/test-resolve
/test-ring
diff --git a/Makefile.am b/Makefile.am
index d7b0f90..2344087 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -830,6 +830,8 @@ libsystemd_shared_la_SOURCES = \
src/shared/ring.h \
src/shared/barrier.c \
src/shared/barrier.h \
+ src/shared/pty.c \
+ src/shared/pty.h \
src/shared/async.c \
src/shared/async.h \
src/shared/copy.c \
@@ -1239,6 +1241,7 @@ tests += \
test-util \
test-ring \
test-barrier \
+ test-pty \
test-tmpfiles \
test-namespace \
test-date \
@@ -1415,6 +1418,12 @@ test_barrier_SOURCES = \
test_barrier_LDADD = \
libsystemd-core.la
+test_pty_SOURCES = \
+ src/test/test-pty.c
+
+test_pty_LDADD = \
+ libsystemd-core.la
+
test_tmpfiles_SOURCES = \
src/test/test-tmpfiles.c
diff --git a/src/shared/pty.c b/src/shared/pty.c
new file mode 100644
index 000..c82c277
--- /dev/null
+++ b/src/shared/pty.c
@@ -0,0 +1,542 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright 2014 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/.
+***/
+
+/*
+ * PTY
+ * A PTY object represents a single PTY connection between a master and a
+ * child. The child process is fork()ed so the caller controls what program
+ * will be run.
+ *
+ * Programs like /bin/login tend to perform a vhangup() on their TTY
+ * before running the login procedure. This also causes the pty master
+ * to get a EPOLLHUP event as long as no client has the TTY opened.
+ * This means, we cannot use the TTY connection as reliable way to track
+ * the client. Instead, we _must_ rely on the PID of the client to track
+ * them.
+ * However, this has the side effect that if the client forks and the
+ * parent exits, we loose them and restart the client. But this seems to
+ * be the expected behavior so we implement it here.
+ *
+ * Unfortunately, epoll always polls for EPOLLHUP so as long as the
+ * vhangup() is ongoing, we will _always_ get EPOLLHUP and cannot sleep.
+ * This gets worse if the client closes the TTY but doesn't exit.
+ * Therefore, the fd must be edge-triggered in the epoll-set so we
+ * only get the events once they change.
+ */
+
+#include errno.h
+#include fcntl.h
+#include limits.h
+#include pty.h
+#include signal.h
+#include stdbool.h
+#include stdint.h
+#include stdio.h
+#include stdlib.h
+#include string.h
+#include sys/epoll.h
+#include sys/eventfd.h
+#include sys/ioctl.h
+#include sys/types.h
+#include sys/uio.h
+#include sys/wait.h
+#include termios.h
+#include unistd.h
+
+#include barrier.h
+#include macro.h
+#include pty.h
+#include ring.h
+#include util.h
+
+#define PTY_BUFSIZE 16384
+
+struct Pty {
+unsigned long ref;
+Barrier barrier;
+int fd;
+pid_t child;
+sd_event_source *fd_source;
+sd_event_source *child_source;
+
+char in_buf[PTY_BUFSIZE];
+Ring
Re: [systemd-devel] [PATCH 2/3] nspawn: use Barrier API instead of eventfd-util
Hi On Mon, Jul 14, 2014 at 3:28 AM, Djalal Harouni tix...@opendz.org wrote: ppoll is atomic and it is handled by the kernel, so perhaps setting/restoring sigmask can be done easily! and for nspawn: IMO we need to receive SIGCHLD which implies EINTR. I say EINTR since not only for blocking read or infinite poll, but perhaps for all the other functions that the parent may do to setup the environment of the container, currently nspawn will set network interfaces before moving them into the container, it will also register the machine, and perhaps other operations... So having EINTR errors is useful here not only for direct reads, but for all the other calls that might block! IOW I think that nspawn should have an empty sig handler for SIGCHLD. Barrier reads already use poll and pipe to handle remote abortion since it can *not* be done by eventfd, yes this is perfect but for nspawn we can also achieve the same by combining eventfd and SICCHLD! What do you think if we make Barrier use: eventfd+pipe and/or eventfd+SIGCHLD ? Most complex fork/clone code should receive SIGCHLD, and think about nspawn! we do want it to be as lightweigh as possible, having 4 fds by default (2 eventfd + heavy pipe) may hit some resource limits quickly! compared to: 2 eventfd + empty sig handler! My first attempt was to use a signalfd on SIGCHLD + edge-triggered. If I don't read from the signalfd and only use it to wake up and wall waitid(WNOWAIT), I won't interfere with other signalfds. However, this wasn't really more lightweight than the pipe-method so i ditched it. Regarding dropping the pipe: pipe2() is _really_ fast. I mean, we're fork()ing and running like thousands of syscalls just during container setup. I cannot see how dropping one light pipe2 call is beneficial here? We also destroy the pipe before running the real container. So it's really just during setup. And it seems from the patch you are not checking barrier_place() return code, if the remote aborted ? That's fine. Abortions are remembered and the later barrier_sync() call will return immediately. Thanks for the patches, sure the API is really nice, I'll try to comment on #1 Thanks! David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Warnings from recent commits
Hi
On Thu, Jul 17, 2014 at 8:51 PM, Thomas H.P. Andersen pho...@gmail.com wrote:
From recent commits I have noticed the following new issues from
static analysis with scan-build and with clang. I am not sure how they
should be fixed (or even if) but I just though I would let you know.
1) src/shared/barrier.c in barrier_read starting at line 274
if (pfd[1].revents) {
len = read(b-them, buf, sizeof(buf));
...
} else if (pfd[0].revents (POLLHUP | POLLERR | POLLNVAL)) {
...
buf = BARRIER_ABORTION;
}
If neither if/else if are true then buf will be used unset.
This is a false-positive. poll() must return 0 in our case, so either
revents must be set. I added else continue; to suppress this
warning.
Thanks!
David
2) src/resolve/resolved-dns-scope.c in dns_scope_tcp_socket
if s-link is null then ifindex will not be set but will be used later in:
} else if (srv-family == AF_INET6) {
sa.in6.sin6_port = htobe16(53);
sa.in6.sin6_addr = srv-address.in6;
sa.in6.sin6_scope_id = ifindex;
salen = sizeof(sa.in6);
3) I see a couple of these:
In file included from src/resolve/resolved-gperf.c:8:
In file included from ./src/resolve/resolved.h:34:
In file included from ./src/resolve/resolved-dns-query.h:33:
In file included from ./src/resolve/resolved-dns-scope.h:33:
./src/resolve/resolved-dns-cache.h:45:3: warning: redefinition of
typedef 'DnsCacheItem' is a C11 feature [-Wtypedef-redefinition]
} DnsCacheItem;
^
./src/resolve/resolved-dns-cache.h:31:29: note: previous definition is here
typedef struct DnsCacheItem DnsCacheItem;
^
1 warning generated.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/3] logind: session: don't set /dev/ttyX owner to root on restore_vt
Hi On Mon, Aug 11, 2014 at 5:05 PM, Olivier Brunel j...@jjacky.com wrote: On 08/11/14 16:54, Lennart Poettering wrote: On Mon, 11.08.14 16:39, Olivier Brunel (j...@jjacky.com) wrote: On 08/11/14 16:25, Lennart Poettering wrote: On Fri, 08.08.14 20:45, Olivier Brunel (j...@jjacky.com) wrote: In session_prepare_vt() we set owner of /dev/ttyX to the user, as that is needed for things to work. However, we shouldn't reset it to root on session_restore_vt() since it could have in fact already been set to the user. I don't follow here, can't parse this. Could you please elaborate? I meant, before the call to session_prepare_vt() the owner of /dev/ttyX might not be root, but already set to the user. In which case setting it back to root might not be expected/best. But that sounds more as if session_restore_vt() should not be used as-is as cleanup path for session_prepare_vt(), no? E.g. if a log in on tty3 right now, /dev/tty3 will be owned by my user, then if a process try a TakeControl() and it failed (or after it's done) the ownership would be set to root, even though it wasn't actually root to begin with. Isn't this very theoretic? I mean, when does TakeControl() actually really fail for you IRL? Right, this was noticed when trying to start a second rootless X, since with systemd-215 things currently fail, and the ownership of /dev/ttyX would then be switched/changed to root. Wait, what? Can you please elaborate. Currently, only one process can be controller at a time, and session_prepare_vt() is called *after* the controller is set. Therefore, it is not called when you try starting a second controller. The only race I see is this: * start legacy Xserver (which doesn't use TakeControl()) which sets user-id on the TTY * start new Xserver which uses TakeControl() (and thus calls session_prepare_vt()) * stop new Xserver (thus drop Control and reset the TTY) In this case, the new xserver will try to start up, but fail as the old one is still running. Therefore, it *might* call ReleaseControl() and thus reset the TTY. However, you're not supposed to mix both and this is not a legitimate use-case. I mean, the old server is root and modifies the TTY by itself (without using systemd). Obviously, this is racy. From a systemd-logind perspective, this is the same as if you run sudo chown xyz /dev/tty manually. session_prepare_vt() could also check the owner first, and note what to reset to on session_restore_vt(), to effectively restore things as they were. Even though I don't see any problem here, I'd be fine with such a patch. Care to send one? Note that you probably need to store that information in the session-file (session_save() / session_load()) too. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/3] logind: session: set_controller should fail if prepare_vt fails
Hi
On Fri, Aug 8, 2014 at 8:45 PM, Olivier Brunel j...@jjacky.com wrote:
If controllers can expect logind to have prepared the VT (e.g. set it to
graphics mode, etc) then TakeControl() should fail if said preparation
failed (and session_restore_vt() was called).
---
src/login/logind-session.c | 15 +--
src/login/logind-session.h | 2 +-
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 905e73f..3f4e177 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -1003,14 +1003,14 @@ static int session_vt_fn(sd_event_source *source,
const struct signalfd_siginfo
return 0;
}
-void session_prepare_vt(Session *s) {
+int session_prepare_vt(Session *s) {
int vt, r;
struct vt_mode mode = { 0 };
sigset_t mask;
vt = session_open_vt(s);
if (vt 0)
-return;
+return vt;
This doesn't work. session_open_vt() returns -1 if the session has no
VT attached. You need to return 0 in that case, though:
if (s-vtnr 1)
return 0;
vt = session_open_vt(s);
if (vt 0)
return vt;
Thanks
David
r = fchown(vt, s-user-uid, -1);
if (r 0)
@@ -1042,11 +1042,12 @@ void session_prepare_vt(Session *s) {
if (r 0)
goto error;
-return;
+return 0;
error:
log_error(cannot mute VT %u for session %s (%d/%d), s-vtnr,
s-id, r, errno);
session_restore_vt(s);
+return r;
}
void session_restore_vt(Session *s) {
@@ -1123,8 +1124,6 @@ int session_set_controller(Session *s, const char
*sender, bool force) {
return r;
}
-session_swap_controller(s, t);
-
/* When setting a session controller, we forcibly mute the VT and set
* it into graphics-mode. Applications can override that by changing
* VT state after calling TakeControl(). However, this serves as a
good
@@ -1133,7 +1132,11 @@ int session_set_controller(Session *s, const char
*sender, bool force) {
* exits.
* If logind crashes/restarts, we restore the controller during
restart
* or reset the VT in case it crashed/exited, too. */
-session_prepare_vt(s);
+r = session_prepare_vt(s);
+if (r 0)
+return r;
+
+session_swap_controller(s, t);
return 0;
}
diff --git a/src/login/logind-session.h b/src/login/logind-session.h
index e62b76d..2ab3182 100644
--- a/src/login/logind-session.h
+++ b/src/login/logind-session.h
@@ -172,7 +172,7 @@ SessionClass session_class_from_string(const char *s)
_pure_;
const char *kill_who_to_string(KillWho k) _const_;
KillWho kill_who_from_string(const char *s) _pure_;
-void session_prepare_vt(Session *s);
+int session_prepare_vt(Session *s);
void session_restore_vt(Session *s);
bool session_is_controller(Session *s, const char *sender);
--
2.0.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 3/3] logind: session: Fix not allowing more than one controller
Hi On Mon, Aug 11, 2014 at 4:52 PM, Lennart Poettering lenn...@poettering.net wrote: On Mon, 11.08.14 16:46, Olivier Brunel (j...@jjacky.com) wrote: On 08/11/14 16:34, Lennart Poettering wrote: On Fri, 08.08.14 20:45, Olivier Brunel (j...@jjacky.com) wrote: While a session can only ever have one controller, there can be more than one session with a controller at a time. However, because of the handling of SIGUSR1 for handling VT switch, trying to set a controller on a session while another session had a controller would fail. I really don't feel comfortable with using SIGUSR1 for this, anyway I must say... SIGUSR1 and SIGUSR2 I think should be left for admins to communicate with the daemon for some simple operations, but using this internally sounds wrong. Now, the VT is so stupid to require a signa handler here, but I think using SIGRTMIN+1 or so might be the better choice here? Now, what makes we wonder here, shouldn't we just install a single signal event handler for this when logind initializes, and leaving it on until the very end? Just to note: the problem is that when the signal is called, there's no way of telling which VT it is about. I think it was only intended for one process to handle one VT, so there was no question. But if logind wants to handle more than one VT, to know on which VT to operate then I'm not sure it would actually be doable unless using a different signal per VT... The VT is such a crap. Gah... It is! Maybe we can iterate through the VTs we have fds for and simply acknowledge all of them? Wouldn't that work? With rtsigs we actually have the option to allocate one signal per vt if we want to. But anyway, David really shouild comment on this. David? No-one ran multiple new sessions so far, so we didn't notice this. But yeah, it's a real bug. What we should do is: 1) Register the signal handler globally (eg., logind.c). 2) Use some RT signal instead of SIGUSR1 3) When the signal is received, we should call VT_RELDISP on seat0-active-vtfd in case it is =0. I will prepare a patch. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] login: share VT-signal handler between sessions
sd-event does not allow multiple handlers for a single signal. However,
logind sets up signal handlers for each session with VT_PROCESS set (that
is, it has an active controller). Therefore, registering multiple such
controllers will fail.
Lets make the VT-handler global, as it's mostly trivial, anyway. This way,
the sessions don't have to take care of that and we can simply acknowledge
all VT-switch requests as we always did.
---
Hi Olivier
Can you give this a try? It should solve your issues.
Thanks
David
src/login/logind-session.c | 26 ++-
src/login/logind-session.h | 1 -
src/login/logind.c | 62 ++
3 files changed, 64 insertions(+), 25 deletions(-)
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index fdeacb1..26b6a90 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -153,8 +153,6 @@ void session_free(Session *s) {
hashmap_remove(s-manager-sessions, s-id);
-s-vt_source = sd_event_source_unref(s-vt_source);
-
free(s-state_file);
free(s);
}
@@ -994,19 +992,9 @@ static int session_open_vt(Session *s) {
return s-vtfd;
}
-static int session_vt_fn(sd_event_source *source, const struct
signalfd_siginfo *si, void *data) {
-Session *s = data;
-
-if (s-vtfd = 0)
-ioctl(s-vtfd, VT_RELDISP, 1);
-
-return 0;
-}
-
void session_prepare_vt(Session *s) {
int vt, r;
struct vt_mode mode = { 0 };
-sigset_t mask;
vt = session_open_vt(s);
if (vt 0)
@@ -1024,20 +1012,12 @@ void session_prepare_vt(Session *s) {
if (r 0)
goto error;
-sigemptyset(mask);
-sigaddset(mask, SIGUSR1);
-sigprocmask(SIG_BLOCK, mask, NULL);
-
-r = sd_event_add_signal(s-manager-event, s-vt_source, SIGUSR1,
session_vt_fn, s);
-if (r 0)
-goto error;
-
/* Oh, thanks to the VT layer, VT_AUTO does not work with KD_GRAPHICS.
* So we need a dummy handler here which just acknowledges *all* VT
* switch requests. */
mode.mode = VT_PROCESS;
-mode.relsig = SIGUSR1;
-mode.acqsig = SIGUSR1;
+mode.relsig = SIGRTMIN;
+mode.acqsig = SIGRTMIN + 1;
r = ioctl(vt, VT_SETMODE, mode);
if (r 0)
goto error;
@@ -1058,8 +1038,6 @@ void session_restore_vt(Session *s) {
if (vt 0)
return;
-s-vt_source = sd_event_source_unref(s-vt_source);
-
ioctl(vt, KDSETMODE, KD_TEXT);
if (read_one_line_file(/sys/module/vt/parameters/default_utf8,
utf8) = 0 *utf8 == '1')
diff --git a/src/login/logind-session.h b/src/login/logind-session.h
index e62b76d..562332c 100644
--- a/src/login/logind-session.h
+++ b/src/login/logind-session.h
@@ -98,7 +98,6 @@ struct Session {
Seat *seat;
unsigned int vtnr;
int vtfd;
-sd_event_source *vt_source;
pid_t leader;
uint32_t audit_id;
diff --git a/src/login/logind.c b/src/login/logind.c
index ec5529d..b470916 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -720,6 +720,46 @@ static int manager_connect_bus(Manager *m) {
return 0;
}
+static int manager_vt_switch(sd_event_source *src, const struct
signalfd_siginfo *si, void *data) {
+Manager *m = data;
+Session *active, *iter;
+
+/*
+ * We got a VT-switch signal and we have to acknowledge it immediately.
+ * Preferably, we'd just use m-seat0-active-vtfd, but unfortunately,
+ * old user-space might run multiple sessions on a single VT, *sigh*.
+ * Therefore, we have to iterate all sessions and find one with a vtfd
+ * on the requested VT.
+ * As only VTs with active controllers have VT_PROCESS set, our current
+ * notion of the active VT might be wrong (for instance if the switch
+ * happens while we setup VT_PROCESS). Therefore, read the current VT
+ * first and then use s-active-vtnr as reference. Note that this is
+ * not racy, as no further VT-switch can happen as long as we're in
+ * synchronous VT_PROCESS mode.
+ */
+
+seat_read_active_vt(m-seat0);
+
+active = m-seat0-active;
+if (!active || active-vtnr 1) {
+log_warning(Received VT_PROCESS signal without a registered
session on that VT.);
+return 0;
+}
+
+if (active-vtfd = 0) {
+ioctl(active-vtfd, VT_RELDISP, 1);
+} else {
+LIST_FOREACH(sessions_by_seat, iter, m-seat0-sessions) {
+if (iter-vtnr == active-vtnr iter-vtfd = 0) {
+ioctl(iter-vtfd, VT_RELDISP, 1);
+break;
+}
+}
+}
+
+return 0;
+}
+
static
Re: [systemd-devel] [PATCH 2/3] logind: session: set_controller should fail if prepare_vt fails
Hi
On Mon, Aug 11, 2014 at 6:13 PM, Lennart Poettering
lenn...@poettering.net wrote:
On Mon, 11.08.14 17:17, David Herrmann (dh.herrm...@gmail.com) wrote:
Hi
On Fri, Aug 8, 2014 at 8:45 PM, Olivier Brunel j...@jjacky.com wrote:
If controllers can expect logind to have prepared the VT (e.g. set it to
graphics mode, etc) then TakeControl() should fail if said preparation
failed (and session_restore_vt() was called).
---
src/login/logind-session.c | 15 +--
src/login/logind-session.h | 2 +-
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 905e73f..3f4e177 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -1003,14 +1003,14 @@ static int session_vt_fn(sd_event_source *source,
const struct signalfd_siginfo
return 0;
}
-void session_prepare_vt(Session *s) {
+int session_prepare_vt(Session *s) {
int vt, r;
struct vt_mode mode = { 0 };
sigset_t mask;
vt = session_open_vt(s);
if (vt 0)
-return;
+return vt;
This doesn't work. session_open_vt() returns -1 if the session has no
Humm. David, can we please stick to returning errors as negative errno
values, and not make up -1 as error in some cases? Please uniformly
only return negative errno codes...
So, please session_open_vt() should be fixed to return -EBUSY or -EINVAL
or something, but not -1...
Old legacy.. I fixed it up, changed the !CONFIG_VT case and applied the patch.
Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/3] logind: session: don't set /dev/ttyX owner to root on restore_vt
Hi On Mon, Aug 11, 2014 at 5:37 PM, Olivier Brunel j...@jjacky.com wrote: On 08/11/14 17:12, David Herrmann wrote: Wait, what? Can you please elaborate. Currently, only one process can Sorry, I meant e.g. having one rootless X on tt1 and starting another one on tty2. Currently this fails (see other patch/mail), and is how this was observed. Ah, so it's triggered by the same SIG-handler bug. be controller at a time, and session_prepare_vt() is called *after* the controller is set. Therefore, it is not called when you try starting a second controller. The only race I see is this: * start legacy Xserver (which doesn't use TakeControl()) which sets user-id on the TTY * start new Xserver which uses TakeControl() (and thus calls session_prepare_vt()) * stop new Xserver (thus drop Control and reset the TTY) In this case, the new xserver will try to start up, but fail as the old one is still running. Therefore, it *might* call ReleaseControl() and thus reset the TTY. However, you're not supposed to mix both and this is not a legitimate use-case. I mean, the old server is root and modifies the TTY by itself (without using systemd). Obviously, this is racy. From a systemd-logind perspective, this is the same as if you run sudo chown xyz /dev/tty manually. session_prepare_vt() could also check the owner first, and note what to reset to on session_restore_vt(), to effectively restore things as they were. Even though I don't see any problem here, I'd be fine with such a patch. Care to send one? Note that you probably need to store that information in the session-file (session_save() / session_load()) too. If you think the current behavior (setting to root on session_restore_vt()) is fine, I'm fine leaving it as is. Else, I could work on a patch as described. I just wanted to understand why that happens. And indeed, by fixing the SIG-handler bug, this is no longer necessary. I added log_error() statements to prepare_vt() so the log should be verbose enough. If you want to fix the fchown() thing, feel free to provide a patch. But I doubt anyone can trigger it with my fix applied. Thanks a lot! David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] login: share VT-signal handler between sessions
Hi On Mon, Aug 11, 2014 at 6:54 PM, Lennart Poettering lenn...@poettering.net wrote: On Mon, 11.08.14 18:46, Lennart Poettering (lenn...@poettering.net) wrote: With this code you block, but do not ignore SGRTMIN+1. Now, rtsigs actually are implemented in a queue, multiple instances of the same signal might be queued up. If you simply block dispatching, then the queue will eventually overrun blocking all other signals... Hence, I think there needs to be a dummy signal handler assigned to SIGRTMIN+1 here, so that the signals are dequeued... (Thinking about it, it might make sense to export a dummy signal handler from sd-event which people can just use here...) Actually just invoking ignore_signals() from shared/util.c should suffice for this case here, too... Nice catch. I totally forgot about that RT-signal behavior. I fixed this by using ignore_signals(). I will give this a try and then push out in case nothing shows up. Olivier: If this does not work for you, please let me know and we can debug this further. Thanks! David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] login: share VT-signal handler between sessions
Hi On Mon, Aug 11, 2014 at 7:57 PM, Olivier Brunel j...@jjacky.com wrote: On 08/11/14 18:21, David Herrmann wrote: sd-event does not allow multiple handlers for a single signal. However, logind sets up signal handlers for each session with VT_PROCESS set (that is, it has an active controller). Therefore, registering multiple such controllers will fail. Lets make the VT-handler global, as it's mostly trivial, anyway. This way, the sessions don't have to take care of that and we can simply acknowledge all VT-switch requests as we always did. --- Hi Olivier Can you give this a try? It should solve your issues. Yes, just tried it -- it does solve the issue. Applied. Thanks! David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Thoughts about /etc/crypttab keyscript options
Hi On Thu, Aug 14, 2014 at 7:44 PM, Lennart Poettering lenn...@poettering.net wrote: On Mon, 21.07.14 10:46, Marc Haber (mh+systemd-de...@zugschlus.de) wrote: Heya, I have read the thread (from 2012?) where those things were discussed here and I understand that I should replace my keyscript with a passwort agent. Things would then work like this: There's currently no streamlined support for stacking password questions really. You currently cannot take possession of specific password questions. Also note that we really should redesign the entire scheme around the kernel keyring as only transport for the keys (and the bus for signalling). I am a bit conservative in changing here too much for now, because we really should figure out that bit first. The hack you describe here should work, however, it's really an ugly hack. One thing you really need to take care of is to not cause recursive loops. That is, if your agent places a new *.ask file, it will be called on it again and *must* ignore it. Otherwise, you end up with an endless loop. Anyhow, I don't think we should support stacked agents. Agents are meant as an API to interact with users. They should not employ any logic/rules regarding the query itself. They're solely meant as GUI. That is, to solve your problem, I'd recommend to make systemd allow external scripts like keyscript= before placing *.ask files (or some other hookup or configuration, if scripts are not suitable for that). I have never worked with crypttab, though. I have to refer to Lennart here to tell whether that makes sense. I just want to make clear that once you query the ask-password tool, you will inevitably end up forwarding that request unchanged to an UI. Polkit provides .rule scripts for that. We don't have any equivalent for ask-password. I'm not sure whether that would make sense. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] compile with clang broken
Hi
On Thu, Aug 14, 2014 at 8:07 PM, Lennart Poettering
lenn...@poettering.net wrote:
On Fri, 18.07.14 16:02, Thomas H.P. Andersen (pho...@gmail.com) wrote:
1716f6dcf54d4c181c2e2558e3d5414f54c8d9ca (resolved: add LLMNR support
for looking up names) broke the build on clang.
src/resolve/resolved-manager.c:553:43: error: non-const static data
member must be initialized out of line
uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct in_pktinfo), sizeof(struct
in6_pktinfo)))
Moving the MAX(...) to a separate line fixes that problem but another
error then happens:
src/resolve/resolved-manager.c:554:25: error: fields must have a
constant size: 'variable length array in structure' extension will
never be supported
uint8_t buffer[CMSG_SPACE(size)
We have encountered the same problem before and Lennart was able to
write the code in a different way. Would this be possible here too?
My sugegstion here would be to maybe rewrite the MAX() macro to use
__builtin_constant_p() so that it becomes constant if the params are
constant, and only uses code block when it isn't. Or so...
http://lists.freedesktop.org/archives/systemd-devel/2014-August/021912.html
Hm, I don't know whether that works. See the description here:
https://gcc.gnu.org/onlinedocs/gcc-4.3.3/gcc/Other-Builtins.html
What you propose is something like my attached patch, I guess? Along
the lines of:
(__builtin_constant_p(A) __builtin_constant_p(B)) ?
((A) (B) ? (A) : (B)) :
({ OLD_MAX })
Thing is, the ELSE case is not considered a compile-time constant by
LLVM. Therefore, the whole expression is not considered a compile-time
constant. I don't know whether conditions with __builtin_constant_p()
are evaluated at the parser-step. The GCC example replaces the ELSE
case with -1, effectively making both compile-time constants.
I also added a test-case to make sure __builtin_constant_p doesn't
evaluate the arguments.
Can someone with LLVM set up give this a spin?
Thanks
David
diff --git a/src/shared/macro.h b/src/shared/macro.h
index 5619c32..18f5a79 100644
--- a/src/shared/macro.h
+++ b/src/shared/macro.h
@@ -140,6 +140,17 @@ static inline unsigned long ALIGN_POWER2(unsigned long u) {
_a _b ? _a : _b; \
})
+#undef MAX
+#define MAX(_A, _B) \
+__extension__ ( \
+(__builtin_constant_p(_A) __builtin_constant_p(_B)) \
+? (((_A) (_B)) ? (_A) : (_B)) \
+: ({\
+typeof(_A) _a = (_A); \
+typeof(_B) _b = (_B); \
+_a _b ? _a : _b; \
+}))
+
#define MAX3(x,y,z) \
__extension__ ({ \
typeof(x) _c = MAX(x,y); \
diff --git a/src/test/test-util.c b/src/test/test-util.c
index 1850f97..d348ac5 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -70,6 +70,20 @@ static void test_align_power2(void) {
}
}
+static void test_max(void) {
+/* try triggering compile-errors for non-const initializers */
+static const struct {
+int a;
+} val1 = {
+.a = MAX(10, 100),
+};
+int d = 0;
+
+assert_se(val1.a == 100);
+assert_se(MAX(++d, 0) == 1);
+assert_se(d == 1);
+}
+
static void test_first_word(void) {
assert_se(first_word(Hello, ));
assert_se(first_word(Hello, Hello));
@@ -926,6 +940,7 @@ int main(int argc, char *argv[]) {
test_streq_ptr();
test_align_power2();
+test_max();
test_first_word();
test_close_many();
test_parse_boolean();
diff --git a/src/shared/macro.h b/src/shared/macro.h
index 5619c32..18f5a79 100644
--- a/src/shared/macro.h
+++ b/src/shared/macro.h
@@ -140,6 +140,17 @@ static inline unsigned long ALIGN_POWER2(unsigned long u) {
_a _b ? _a : _b; \
})
+#undef MAX
+#define MAX(_A, _B) \
+__extension__ ( \
+(__builtin_constant_p(_A) __builtin_constant_p(_B)) \
+? (((_A) (_B)) ? (_A) : (_B)) \
+: ({\
+typeof(_A) _a = (_A); \
+typeof(_B) _b = (_B); \
+_a _b ? _a : _b; \
+}))
+
#define MAX3(x,y,z) \
Re: [systemd-devel] compile with clang broken
Hi
On Fri, Aug 15, 2014 at 11:35 AM, Thomas H.P. Andersen pho...@gmail.com wrote:
On Fri, Aug 15, 2014 at 10:55 AM, David Herrmann dh.herrm...@gmail.com
wrote:
Hi
On Thu, Aug 14, 2014 at 8:07 PM, Lennart Poettering
lenn...@poettering.net wrote:
On Fri, 18.07.14 16:02, Thomas H.P. Andersen (pho...@gmail.com) wrote:
1716f6dcf54d4c181c2e2558e3d5414f54c8d9ca (resolved: add LLMNR support
for looking up names) broke the build on clang.
src/resolve/resolved-manager.c:553:43: error: non-const static data
member must be initialized out of line
uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct in_pktinfo), sizeof(struct
in6_pktinfo)))
Moving the MAX(...) to a separate line fixes that problem but another
error then happens:
src/resolve/resolved-manager.c:554:25: error: fields must have a
constant size: 'variable length array in structure' extension will
never be supported
uint8_t buffer[CMSG_SPACE(size)
We have encountered the same problem before and Lennart was able to
write the code in a different way. Would this be possible here too?
My sugegstion here would be to maybe rewrite the MAX() macro to use
__builtin_constant_p() so that it becomes constant if the params are
constant, and only uses code block when it isn't. Or so...
http://lists.freedesktop.org/archives/systemd-devel/2014-August/021912.html
Hm, I don't know whether that works. See the description here:
https://gcc.gnu.org/onlinedocs/gcc-4.3.3/gcc/Other-Builtins.html
What you propose is something like my attached patch, I guess? Along
the lines of:
(__builtin_constant_p(A) __builtin_constant_p(B)) ?
((A) (B) ? (A) : (B)) :
({ OLD_MAX })
Thing is, the ELSE case is not considered a compile-time constant by
LLVM. Therefore, the whole expression is not considered a compile-time
constant. I don't know whether conditions with __builtin_constant_p()
are evaluated at the parser-step. The GCC example replaces the ELSE
case with -1, effectively making both compile-time constants.
I also added a test-case to make sure __builtin_constant_p doesn't
evaluate the arguments.
Can someone with LLVM set up give this a spin?
I still get:
src/resolve/resolved-manager.c:844:43: error: non-const static data
member must be initialized out of line
uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
in_pktinfo), sizeof(struct in6_pktinfo)))
^
Thanks for trying!
Result is as I expected. Evaluation takes place _after_ validating
compile-time constants, and thus __builtin_constant_p in combination
with ?: will not work if not both cases are constant. Maybe it works
with __builtin_choose_expr()?
Can you try the attached patch?
If that still doesn't work, I guess we're left with your proposed
solution below, or we add MAX_CONST() which just does (A B)?A:B.
I got the following to compile but I have not have time to test it at
all. Too ugly to live I guess...
diff --git a/src/resolve/resolved-dns-stream.c
b/src/resolve/resolved-dns-stream.c
index eb78587..1b415ae 100644
--- a/src/resolve/resolved-dns-stream.c
+++ b/src/resolve/resolved-dns-stream.c
@@ -62,10 +62,14 @@ static int dns_stream_complete(DnsStream *s, int error) {
}
static int dns_stream_identify(DnsStream *s) {
+const size_t size = __builtin_constant_p(
+MAX(sizeof(struct in_pktinfo), sizeof(struct in6_pktinfo))) ?
+MAX(sizeof(struct in_pktinfo), sizeof(struct in6_pktinfo)) :
0;
No reason to make size constant. You can use:
size_t size = MAX();
uint8_t buffer[...];
This will be similar to alloca(), I think... or maybe I'm wrong..
Thanks
David
union {
struct cmsghdr header; /* For alignment */
-uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
in_pktinfo), sizeof(struct in6_pktinfo)))
+uint8_t buffer[CMSG_SPACE(size)
+ EXTRA_CMSG_SPACE /* kernel appears
to require extra space */];
+
} control;
struct msghdr mh = {};
struct cmsghdr *cmsg;
@@ -73,6 +77,7 @@ static int dns_stream_identify(DnsStream *s) {
int r;
assert(s);
+assert(size 0);
if (s-identified)
return 0;
diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
index bfbdc7d..1342fb1 100644
--- a/src/resolve/resolved-manager.c
+++ b/src/resolve/resolved-manager.c
@@ -839,9 +839,12 @@ fail:
int manager_recv(Manager *m, int fd, DnsProtocol protocol, DnsPacket **ret) {
_cleanup_(dns_packet_unrefp) DnsPacket *p = NULL;
+const size_t size = __builtin_constant_p(
+MAX(sizeof(struct in_pktinfo), sizeof(struct in6_pktinfo))) ?
+MAX(sizeof(struct in_pktinfo), sizeof(struct in6_pktinfo)) :
0;
union {
struct cmsghdr header; /* For alignment */
-uint8_t buffer
Re: [systemd-devel] compile with clang broken
Hi
On Fri, Aug 15, 2014 at 11:38 AM, Lennart Poettering
lenn...@poettering.net wrote:
On Fri, 15.08.14 10:55, David Herrmann (dh.herrm...@gmail.com) wrote:
Hm, I don't know whether that works. See the description here:
https://gcc.gnu.org/onlinedocs/gcc-4.3.3/gcc/Other-Builtins.html
What you propose is something like my attached patch, I guess? Along
the lines of:
(__builtin_constant_p(A) __builtin_constant_p(B)) ?
((A) (B) ? (A) : (B)) :
({ OLD_MAX })
Yes, correct.
Thing is, the ELSE case is not considered a compile-time constant by
LLVM.
In that case __builtin_constant_p() would be entirely useless on LLVM,
right? And all uses by this construct in glibc would not work, right?
No, it's just useless for our case.
glibc uses __builtin_constant_p() heavily to validate parameters. For
instance, it's very handy to verify length restrictions and so on. And
I think it was introduced mainly to allow optimizations, not to allow
conditional compilations. But maybe __builtin_choose_expr() works like
that.
Example: strlen() can use __builtin_constant_p() to use sizeof() - 1
on constant expressions (ok, it cannot, because there might be a NUL
in the middle, but I guess you get the idea?).
Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] compile with clang broken
Hi On Fri, Aug 15, 2014 at 12:29 PM, Thomas H.P. Andersen pho...@gmail.com wrote: On Fri, Aug 15, 2014 at 11:49 AM, David Herrmann dh.herrm...@gmail.com wrote: Thanks for trying! Result is as I expected. Evaluation takes place _after_ validating compile-time constants, and thus __builtin_constant_p in combination with ?: will not work if not both cases are constant. Maybe it works with __builtin_choose_expr()? Can you try the attached patch? This patch works. It also needs the change to do the calculation to a seperate line. Also only if size is const, like so: const size_t size = MAX(sizeof(struct in_pktinfo), sizeof(struct in6_pktinfo)); Again, thanks for trying it out! I don't understand your comment, though. You're saying this works: const size_t size = MAX(...); uint8_t buffer[CMSG_SPACE(size) +...]; ...but this doesn't work: uint8_t buffer[CMSG_SPACE(MAX(...)) +...]; ...and this doesn't work either (mind the dropped 'const'): size_t size = MAX(...); uint8_t buffer[CMSG_SPACE(size) +...]; Hm. This is weird. Maybe CMSG_SPACE does something weird. I'll see. David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] compile with clang broken
Hi
On Fri, Aug 15, 2014 at 1:22 PM, Lennart Poettering
lenn...@poettering.net wrote:
On Fri, 15.08.14 13:00, David Herrmann (dh.herrm...@gmail.com) wrote:
src/resolve/resolved-dns-stream.c:67:43: error: non-const static data
member must be initialized out of line
uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
in_pktinfo), sizeof(struct in6_pktinfo)))
^
Ok, this can be fixed by adding const to the variables inside the ({
}) else-clause. But we then end up with:
error: statement expression not allowed at file scope
I wonder if there's *any* way how to implement a double-evalutation-free
all-type MAX() on LLVM... That'd be quite a limitation in LLVM...
I looked around and it seems like there's nothing we can do. Weird
thing is, LLVM allows const-initialization but not member-definition
with that macro. I really don't understand why..
I somehow think adding MAX_CONST which uses __builtin_constant_p and
assert_cc() is the easiest way here. That is, we use MAX_CONST() for
all cases where MAX fails. I think this is the easiest way to
guarantee no-one else changes the code to use MAX() again.
Furthermore, it guarantees that MAX_CONST is *really* called with
constant arguments.
Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] compile with clang broken
Hi
On Fri, Aug 15, 2014 at 1:53 PM, Lennart Poettering
lenn...@poettering.net wrote:
On Fri, 15.08.14 13:42, David Herrmann (dh.herrm...@gmail.com) wrote:
Hi
On Fri, Aug 15, 2014 at 1:22 PM, Lennart Poettering
lenn...@poettering.net wrote:
On Fri, 15.08.14 13:00, David Herrmann (dh.herrm...@gmail.com) wrote:
src/resolve/resolved-dns-stream.c:67:43: error: non-const static data
member must be initialized out of line
uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
in_pktinfo), sizeof(struct in6_pktinfo)))
^
Ok, this can be fixed by adding const to the variables inside the ({
}) else-clause. But we then end up with:
error: statement expression not allowed at file scope
I wonder if there's *any* way how to implement a double-evalutation-free
all-type MAX() on LLVM... That'd be quite a limitation in LLVM...
I looked around and it seems like there's nothing we can do. Weird
thing is, LLVM allows const-initialization but not member-definition
with that macro. I really don't understand why..
I somehow think adding MAX_CONST which uses __builtin_constant_p and
assert_cc() is the easiest way here. That is, we use MAX_CONST() for
all cases where MAX fails. I think this is the easiest way to
guarantee no-one else changes the code to use MAX() again.
Furthermore, it guarantees that MAX_CONST is *really* called with
constant arguments.
If that is what it takes, go ahead.
Let it be known though for all future: I think LLVM is stupid here.
Meh, static_assert() is not allowed there. _Pragma(error) works, but
is always evaluated even with __builtin_choose_expr(). This is all
stupid... That means MAX_CONST really just becomes AB?A:B.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] compile with clang broken
Hi
On Fri, Aug 15, 2014 at 1:53 PM, Lennart Poettering
lenn...@poettering.net wrote:
On Fri, 15.08.14 13:42, David Herrmann (dh.herrm...@gmail.com) wrote:
Hi
On Fri, Aug 15, 2014 at 1:22 PM, Lennart Poettering
lenn...@poettering.net wrote:
On Fri, 15.08.14 13:00, David Herrmann (dh.herrm...@gmail.com) wrote:
src/resolve/resolved-dns-stream.c:67:43: error: non-const static data
member must be initialized out of line
uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct
in_pktinfo), sizeof(struct in6_pktinfo)))
^
Ok, this can be fixed by adding const to the variables inside the ({
}) else-clause. But we then end up with:
error: statement expression not allowed at file scope
I wonder if there's *any* way how to implement a double-evalutation-free
all-type MAX() on LLVM... That'd be quite a limitation in LLVM...
I looked around and it seems like there's nothing we can do. Weird
thing is, LLVM allows const-initialization but not member-definition
with that macro. I really don't understand why..
I somehow think adding MAX_CONST which uses __builtin_constant_p and
assert_cc() is the easiest way here. That is, we use MAX_CONST() for
all cases where MAX fails. I think this is the easiest way to
guarantee no-one else changes the code to use MAX() again.
Furthermore, it guarantees that MAX_CONST is *really* called with
constant arguments.
If that is what it takes, go ahead.
Let it be known though for all future: I think LLVM is stupid here.
Ok, took me a while, but I now figured out how to cause compilation to
fail even in expressions that initialize types (_Static_assert is not
allowed there):
#define assert_const(expr)
((void)(__builtin_types_compatible_p(int[(expr) ? 1 : -1], int[1])))
Btw., I like that more than our current assert_cc() fallback. But I
leave it up to you to decide.
Anyhow, I found a way to make CONST_MAX work:
#define CONST_MAX(_A, _B)
(__builtin_choose_expr(__builtin_constant_p(_A)
__builtin_constant_p(_B), ((_A) (_B)) ? (_A) : (_B), (void)0))
This will return (void) in case _A or _B is not constant. Works fine
on LLVM, I now have to test it on gcc. If it works, I will commit it
and fix resolvd.
Cheers
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] compile with clang broken
Hi On Fri, Jul 18, 2014 at 4:02 PM, Thomas H.P. Andersen pho...@gmail.com wrote: 1716f6dcf54d4c181c2e2558e3d5414f54c8d9ca (resolved: add LLMNR support for looking up names) broke the build on clang. src/resolve/resolved-manager.c:553:43: error: non-const static data member must be initialized out of line uint8_t buffer[CMSG_SPACE(MAX(sizeof(struct in_pktinfo), sizeof(struct in6_pktinfo))) Moving the MAX(...) to a separate line fixes that problem but another error then happens: src/resolve/resolved-manager.c:554:25: error: fields must have a constant size: 'variable length array in structure' extension will never be supported uint8_t buffer[CMSG_SPACE(size) We have encountered the same problem before and Lennart was able to write the code in a different way. Would this be possible here too? I now pushed 3 commits which should fix this. Works fine with LLVM+clang and GCC here. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] compile with clang broken
Hi
On Fri, Aug 15, 2014 at 5:22 PM, Daniele Nicolodi dani...@grinta.net wrote:
On 15/08/2014 16:30, David Herrmann wrote:
Ok, took me a while, but I now figured out how to cause compilation to
fail even in expressions that initialize types (_Static_assert is not
allowed there):
#define assert_const(expr)
((void)(__builtin_types_compatible_p(int[(expr) ? 1 : -1], int[1])))
Btw., I like that more than our current assert_cc() fallback. But I
leave it up to you to decide.
Anyhow, I found a way to make CONST_MAX work:
#define CONST_MAX(_A, _B)
(__builtin_choose_expr(__builtin_constant_p(_A)
__builtin_constant_p(_B), ((_A) (_B)) ? (_A) : (_B), (void)0))
This will return (void) in case _A or _B is not constant. Works fine
on LLVM, I now have to test it on gcc. If it works, I will commit it
and fix resolvd.
Hello,
this may be completely stupid, but if the only use case you have for
CONST_MAX() is for computing the size of a data structure, I find
something like
#define MAXSIZE(A, B) sizeof(union { __typeof(A) a; __typeof(B) b;})
a little more clear and less magic, and I believe it has the same
guarantees that the solution you found.
Your MAXSIZE macro might add padding:
union A {
int a;
char b[5];
};
This union has size 8, not 5 (64bit). But CONST_MAX would return 5.
Not sure whether that really matters, though. And we could probably
add __packed__ to the definition.
However, I noticed that GCC complains about using
statement-expressions to initialize static-const structure members,
even with my 'const' annotations added to MAX. *sigh*
Thus, I think I'll keep CONST_MAX, as we'd require a 3rd macro otherwise.
If you know a way to unify them all, please lemme know.
Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH kdbus] handle: Return POLLOUT | POLLWRNORM mask when no messages are pending
Hi On Mon, Aug 18, 2014 at 4:15 PM, Marcel Holtmann mar...@holtmann.org wrote: Hi Lennart, To facility the feature of doing an asynchronous sending of messages when the bus is idle, make sure to return POLLOUT | POLLWRNORM from kdbus_handle_poll. Signed-off-by: Marcel Holtmann mar...@holtmann.org --- handle.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/handle.c b/handle.c index ac6868133280..fc15d28351b3 100644 --- a/handle.c +++ b/handle.c @@ -884,6 +884,8 @@ static unsigned int kdbus_handle_poll(struct file *file, mask |= POLLERR | POLLHUP; else if (!list_empty(conn-msg_list)) mask |= POLLIN | POLLRDNORM; + else + mask |= POLLOUT | POLLWRNORM; Hmm, what's your use case here? list_empty(conn-msg_list) only checks the incoming list of the current connection for pending messages. That doesn't tell you whether the bus is idle, or if your receiving end has messages pending ... if you are a good client citizen, then you only send messages when POLLOUT gets signaled. That is what this is allowing now. Blindly sending messages is never a good idea. You want to poll for POLLOUT first. This does not make a big difference for current kernel, but it allows future extensions when the clients are well behaving and just waiting for POLLOUT. Meaning once the kernel does not signal POLLOUT, no new messages will come from the client. Current code does not do this POLLOUT before sending a messages, but our kdbus client actually does that. It is the right thing to do. And we have been doing this with all of our protocols that are using asynchronous IO. No point in kdbus being any different. Well, kdbus keeps per-reciever buffers only, hence signalling on the kdbus fd when you are able to write is not really possible, since this information is not bound to the sender fd but only to the receiever of which there are many... If I understand you correctly you hence want the kdbus fd to always return EPOLLOUT then, because if a client wants to send something it can do that at any time? If that's the case then POLLOUT should really be ORed into the mask unconditionally, not just in some cases... So, I can sympathize with what you are trying to do. However, I think your patch doesn't do the right thing... It should really OR the POLLOUT into all masks always. always returning POLLOUT is also fine. As long as the fd signals POLLOUT and not just swallows it. Yes, we should add POLLOUT unconditionally. I will push the fix. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] kdbus: merge 3.17 branch into master?
Hi Greg On Tue, Aug 19, 2014 at 1:29 PM, Greg KH gre...@linuxfoundation.org wrote: Hi all, Now that memfd is merged into 3.17-rc1, should we merge the 3.17 branch in the kdbus tree into master as well? Right now, the test directory on master doesn't build properly on older kernels due to linux/memfd.h not being present, so it seems that we are already thinking the code is there: TARGET_CC kdbus-util.o kdbus-util.c:27:25: fatal error: linux/memfd.h: No such file or directory #include linux/memfd.h ^ I thought Daniel already cherry-picked the patches from the 3.17 branch into master? david@david-tp ~/dev/kdbus $ git log master..origin/3.17 --oneline b59133f metadata: drop support for kernels v3.15 307be39 Kconfig: depend on TMPFS 60a5b53 test: adopt new memfd interface 790a8a8 switch to upstream kernel memfd implementation (ABI+API break) 7219f86 metadata: follow 3.17 changes for current-start_time All these patches are available in master (with different sha1 due to cherry-pick). Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] compile with clang broken
Hi
On Sat, Aug 16, 2014 at 1:29 PM, Daniele Nicolodi dani...@grinta.net wrote:
On 16/08/2014 12:35, David Herrmann wrote:
On Fri, Aug 15, 2014 at 5:22 PM, Daniele Nicolodi dani...@grinta.net wrote:
this may be completely stupid, but if the only use case you have for
CONST_MAX() is for computing the size of a data structure, I find
something like
#define MAXSIZE(A, B) sizeof(union { __typeof(A) a; __typeof(B) b;})
a little more clear and less magic, and I believe it has the same
guarantees that the solution you found.
Your MAXSIZE macro might add padding:
This union has size 8, not 5 (64bit). But CONST_MAX would return 5.
Not sure whether that really matters, though. And we could probably
add __packed__ to the definition.
Indeed it does add padding. Adding the __packed__ attribute solves the
problem:
#define MAXSIZE(A, B) sizeof( \
union __attribute__((__packed__)) { \
__typeof(A) a; __typeof(B) b;})
However, I noticed that GCC complains about using
statement-expressions to initialize static-const structure members,
even with my 'const' annotations added to MAX. *sigh*
Thus, I think I'll keep CONST_MAX, as we'd require a 3rd macro otherwise.
My proposal was based on the fact that the only use of CONST_MAX there
is (was?) in the code was about array size declarations, and I find
MAXSIZE() much easier to understand.
I've added the macro now.
Thanks!
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] shared: make container_of() use unique variable names
If you stack container_of() macros, you will get warnings due to shadowing
variables of the parent context. To avoid this, use unique names for
variables.
Two new helpers are added:
UNIQ: This evaluates to a truly unique value never returned by any
evaluation of this macro. It's a shortcut for __COUNTER__.
UNIQ_T: Takes two arguments and concatenates them. It is a shortcut for
CONCATENATE, but meant to defined typed local variables.
As you usually want to use variables that you just defined, you need to
reference the same unique value at least two times. However, UNIQ returns
a new value on each evaluation, therefore, you have to pass the unique
values into the macro like this:
#define my_macro(a, b) __max_macro(UNIQ, UNIQ, (a), (b))
#define __my_macro(uniqa, uniqb, a, b) ({
typeof(a) UNIQ_T(A, uniqa) = (a);
typeof(b) UNIQ_T(B, uniqb) = (b);
MY_UNSAFE_MACRO(UNIQ_T(A, uniqa), UNIQ_T(B, uniqb));
})
This way, MY_UNSAFE_MACRO() can safely evaluate it's arguments multiple
times as they are local variables. But you can also stack invocations to
the macro my_macro() without clashing names.
This is the same as if you did:
#define my_macro(a, b) __max_macro(__COUNTER__, __COUNTER__, (a), (b))
#define __my_macro(prefixa, prefixb, a, b) ({
typeof(a) CONCATENATE(A, prefixa) = (a);
typeof(b) CONCATENATE(B, prefixb) = (b);
MY_UNSAFE_MACRO(CONCATENATE(A, prefixa), CONCATENATE(B,
prefixb));
})
...but in my opinion, the first macro is easier to write and read.
This patch starts by converting container_of() to use this new helper.
Other macros may follow (like MIN, MAX, CLAMP, ...).
---
src/shared/macro.h | 13 -
src/test/test-util.c | 19 +++
2 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/src/shared/macro.h b/src/shared/macro.h
index 2807bc7..e673480 100644
--- a/src/shared/macro.h
+++ b/src/shared/macro.h
@@ -79,6 +79,9 @@
#define XCONCATENATE(x, y) x ## y
#define CONCATENATE(x, y) XCONCATENATE(x, y)
+#define UNIQ_T(x, uniq) CONCATENATE(__unique_prefix_, CONCATENATE(x, uniq))
+#define UNIQ __COUNTER__
+
/* Rounds up */
#define ALIGN4(l) (((l) + 3) ~3)
@@ -122,13 +125,13 @@ static inline unsigned long ALIGN_POWER2(unsigned long u)
{
* @ptr: the pointer to the member.
* @type: the type of the container struct this is embedded in.
* @member: the name of the member within the struct.
- *
*/
-#define container_of(ptr, type, member) \
+#define container_of(ptr, type, member) __container_of(UNIQ, (ptr), type,
member)
+#define __container_of(uniq, ptr, type, member) \
__extension__ ({\
-const typeof( ((type *)0)-member ) *__mptr = (ptr); \
-(type *)( (char *)__mptr - offsetof(type,member) ); \
-})
+const typeof( ((type*)0)-member ) *UNIQ_T(A, uniq) = (ptr); \
+(type*)( (char *)UNIQ_T(A, uniq) - offsetof(type,member) ); \
+})
#undef MAX
#define MAX(a,b)\
diff --git a/src/test/test-util.c b/src/test/test-util.c
index 34d5f2e..7b2e71c 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -96,6 +96,24 @@ static void test_max(void) {
assert_cc(MAXSIZE(char, long) == sizeof(long));
}
+static void test_container_of(void) {
+struct mytype {
+uint8_t pad1[3];
+uint64_t v1;
+uint8_t pad2[2];
+uint32_t v2;
+} _packed_ myval = { };
+
+assert_cc(sizeof(myval) == 17);
+assert_se(container_of(myval.v1, struct mytype, v1) == myval);
+assert_se(container_of(myval.v2, struct mytype, v2) == myval);
+assert_se(container_of(container_of(myval.v2,
+ struct mytype,
+ v2)-v1,
+ struct mytype,
+ v1) == myval);
+}
+
static void test_first_word(void) {
assert_se(first_word(Hello, ));
assert_se(first_word(Hello, Hello));
@@ -1218,6 +1236,7 @@ int main(int argc, char *argv[]) {
test_streq_ptr();
test_align_power2();
test_max();
+test_container_of();
test_first_word();
test_close_many();
test_parse_boolean();
--
2.1.0
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 00/10] Device Management for systemd-logind
is inactive, all devices on this session are inactive, too. However, when a session is active, a device might stay inactive (for instance if reactivation failed). But logind guarantees that a device can never be used by anyone else than the foreground session. Compositors can rely on that for security reasons. logind itselfs takes care of revoking device access for inactive sessions (synchronized with session-switches!). It also tries to resume every device when a session is activated. But session-devices must not be used to watch session state! A compositor has to use the PropertiesChanged() signal plus the Active property of sessions for that! Session-devices do not replace this! On sessions with VTs, this is obviously replaced by the VT_SETMODE interface as usual. Now additionally to the interface mentioned above, this series introduces session-controllers. These try to prevent multiple compositors from running in parallel. That is, when a compositor starts up, it calls RequestControl() on the session in question. If there is already another compositor running, this will fail. The new device-management functions are limited to the active controller. No other functions make use of controllers. Note that the RequestControl() call might get a scope argument. So you can have a controller with scope graphics and one for scope sound, for example. On each scope only a single controller is allowed, but the scopes don't interfere. So logind makes sure that RequestDevice() on a graphics or input device requires the graphics scope. But now the API: RequestControl(bool force): For now this misses a scope argument, so it currently implies graphics scope. This function will make the caller the new controller of the given session. logind watches the system bus and automatically drops the controller once it disconnects. If there is already a controller for the given scope, this returns EBUSY. If @force is true, the active controller will be dropped and the caller will get the new controller. This function is restricted to callers with the same UID as the User of the given session. If @force is true, the caller must be root. So this call in combination with RequestDevice() allows us to run a compositor in a session as normal user. Yay! DropControl(): Drop control again. If the caller is not the current controller, this does nothing. Note that this call is optional. logind watches the bus for disconnect events and invokes this implicitly if a controller exits. David Herrmann (10): logind: listen actively for session devices logind: add infrastructure to watch busnames logind: add session controllers logind: make Session.Activate() lazy logind: introduce session-devices logind: rename vtconsole to seat0 logind: fix seat_can_tty() to check for VTs logind: fix session_activate(vtnr = 0) logind: extract has_vts() from can_multi_session() logind: implement generic multi-session Makefile.am | 2 + src/login/logind-dbus.c | 35 ++- src/login/logind-device.c | 39 ++- src/login/logind-device.h | 5 +- src/login/logind-seat.c | 73 -- src/login/logind-seat.h | 6 +- src/login/logind-session-dbus.c | 156 src/login/logind-session-device.c | 517 ++ src/login/logind-session-device.h | 58 + src/login/logind-session.c| 112 - src/login/logind-session.h| 8 + src/login/logind.c| 151 +-- src/login/logind.h| 12 +- 13 files changed, 1109 insertions(+), 65 deletions(-) create mode 100644 src/login/logind-session-device.c create mode 100644 src/login/logind-session-device.h -- 1.8.3.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 01/10] logind: listen actively for session devices
Session compositors need access to fbdev, DRM and evdev devices if they
control a session. To make logind pass them to sessions, we need to
listen for them actively.
However, we avoid creating new seats for non master-of-seat devices. Only
once a seat is created, we start remembering all other session devices. If
the last master-device is removed (even if there are other non-master
devices still available), we destroy the seat. This is the current
behavior, but we need to explicitly implement it now as there may be
non-master devices in the seat-devices list.
Unlike master devices, we don't care whether our list of non-master
devices is complete. We don't export this list but use it only as cache if
sessions request these devices. Hence, if a session requests a device that
is not in the list, we will simply look it up. However, once a session
requested a device, we must be notified of remove udev events. So we
must link the devices somehow into the device-list.
Regarding the implementation, we now sort the device list by the master
flag. This guarantees that master devices are at the front and non-master
devices at the tail of the list. Thus, we can easily test whether a seat
has a master device attached.
---
src/login/logind-device.c | 35 ++---
src/login/logind-device.h | 3 +-
src/login/logind-seat.c | 11 +--
src/login/logind-seat.h | 1 +
src/login/logind.c| 79 +--
src/login/logind.h| 6 ++--
6 files changed, 116 insertions(+), 19 deletions(-)
diff --git a/src/login/logind-device.c b/src/login/logind-device.c
index 51b1535..a9a9633 100644
--- a/src/login/logind-device.c
+++ b/src/login/logind-device.c
@@ -25,7 +25,7 @@
#include logind-device.h
#include util.h
-Device* device_new(Manager *m, const char *sysfs) {
+Device* device_new(Manager *m, const char *sysfs, bool master) {
Device *d;
assert(m);
@@ -48,6 +48,7 @@ Device* device_new(Manager *m, const char *sysfs) {
}
d-manager = m;
+d-master = master;
dual_timestamp_get(d-timestamp);
return d;
@@ -75,11 +76,16 @@ void device_detach(Device *d) {
LIST_REMOVE(Device, devices, d-seat-devices, d);
d-seat = NULL;
-seat_add_to_gc_queue(s);
-seat_send_changed(s, CanGraphical\0);
+if (!seat_has_master_device(s)) {
+seat_add_to_gc_queue(s);
+seat_send_changed(s, CanGraphical\0);
+}
}
void device_attach(Device *d, Seat *s) {
+Device *i;
+bool had_master;
+
assert(d);
assert(s);
@@ -90,7 +96,26 @@ void device_attach(Device *d, Seat *s) {
device_detach(d);
d-seat = s;
-LIST_PREPEND(Device, devices, s-devices, d);
+had_master = seat_has_master_device(s);
+
+/* We keep the device list sorted by the master flag. That is, master
+ * devices are at the front, other devices at the tail. As there is no
+ * way to easily add devices at the list-tail, we need to iterate the
+ * list to find the first non-master device when adding non-master
+ * devices. We assume there is only a few (normally 1) master devices
+ * per seat, so we iterate only a few times. */
+
+if (d-master || !s-devices) {
+LIST_PREPEND(Device, devices, s-devices, d);
+} else {
+LIST_FOREACH(devices, i, s-devices) {
+if (!i-devices_next || !i-master) {
+LIST_INSERT_AFTER(Device, devices, s-devices,
i, d);
+break;
+}
+}
+}
-seat_send_changed(s, CanGraphical\0);
+if (!had_master d-master)
+seat_send_changed(s, CanGraphical\0);
}
diff --git a/src/login/logind-device.h b/src/login/logind-device.h
index 3b15356..315f0e6 100644
--- a/src/login/logind-device.h
+++ b/src/login/logind-device.h
@@ -33,13 +33,14 @@ struct Device {
char *sysfs;
Seat *seat;
+bool master;
dual_timestamp timestamp;
LIST_FIELDS(struct Device, devices);
};
-Device* device_new(Manager *m, const char *sysfs);
+Device* device_new(Manager *m, const char *sysfs, bool master);
void device_free(Device *d);
void device_attach(Device *d, Seat *s);
void device_detach(Device *d);
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index 470d08b..2c60b8a 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -448,10 +448,17 @@ bool seat_can_tty(Seat *s) {
return seat_is_vtconsole(s);
}
+bool seat_has_master_device(Seat *s) {
+assert(s);
+
+/* device list is ordered by master flag */
+return !!s-devices s-devices-master;
+}
+
bool seat_can_graphical(Seat *s) {
assert(s);
-return !!s-devices;
+return
[systemd-devel] [PATCH 02/10] logind: add infrastructure to watch busnames
If we want to track bus-names to allow exclusive resource-access, we need
a way to get notified when a bus-name is gone. We make logind watch for
NameOwnerChanged dbus events and check whether the name is currently
watched. If it is, we remove it from the watch-list (notification for
other objects can be added in follow-up patches).
---
src/login/logind-dbus.c | 17
src/login/logind.c | 54 -
src/login/logind.h | 4
3 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 345df9f..eb62d28 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -2458,6 +2458,23 @@ DBusHandlerResult bus_message_filter(
HASHMAP_FOREACH(session, m-sessions, i)
session_add_to_gc_queue(session);
}
+
+} else if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS,
NameOwnerChanged)) {
+const char *name, *old, *new;
+char *key;
+
+if (!dbus_message_get_args(message, error,
+ DBUS_TYPE_STRING, name,
+ DBUS_TYPE_STRING, old,
+ DBUS_TYPE_STRING, new,
+ DBUS_TYPE_INVALID)) {
+log_error(Failed to parse NameOwnerChanged message:
%s, bus_error_message(error));
+goto finish;
+}
+
+if (*old !*new (key = hashmap_remove(m-busnames, old)))
{
+free(key);
+}
}
finish:
diff --git a/src/login/logind.c b/src/login/logind.c
index 54fb391..f4cef54 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -74,6 +74,7 @@ Manager *manager_new(void) {
m-users = hashmap_new(trivial_hash_func, trivial_compare_func);
m-inhibitors = hashmap_new(string_hash_func, string_compare_func);
m-buttons = hashmap_new(string_hash_func, string_compare_func);
+m-busnames = hashmap_new(string_hash_func, string_compare_func);
m-user_units = hashmap_new(string_hash_func, string_compare_func);
m-session_units = hashmap_new(string_hash_func, string_compare_func);
@@ -82,7 +83,7 @@ Manager *manager_new(void) {
m-inhibitor_fds = hashmap_new(trivial_hash_func,
trivial_compare_func);
m-button_fds = hashmap_new(trivial_hash_func, trivial_compare_func);
-if (!m-devices || !m-seats || !m-sessions || !m-users ||
!m-inhibitors || !m-buttons ||
+if (!m-devices || !m-seats || !m-sessions || !m-users ||
!m-inhibitors || !m-buttons || !m-busnames ||
!m-user_units || !m-session_units ||
!m-session_fds || !m-inhibitor_fds || !m-button_fds) {
manager_free(m);
@@ -111,6 +112,7 @@ void manager_free(Manager *m) {
Seat *s;
Inhibitor *i;
Button *b;
+char *n;
assert(m);
@@ -132,12 +134,16 @@ void manager_free(Manager *m) {
while ((b = hashmap_first(m-buttons)))
button_free(b);
+while ((n = hashmap_first(m-busnames)))
+free(hashmap_remove(m-busnames, n));
+
hashmap_free(m-devices);
hashmap_free(m-seats);
hashmap_free(m-sessions);
hashmap_free(m-users);
hashmap_free(m-inhibitors);
hashmap_free(m-buttons);
+hashmap_free(m-busnames);
hashmap_free(m-user_units);
hashmap_free(m-session_units);
@@ -361,6 +367,40 @@ int manager_add_button(Manager *m, const char *name,
Button **_button) {
return 0;
}
+int manager_watch_busname(Manager *m, const char *name) {
+char *n;
+int r;
+
+assert(m);
+assert(name);
+
+if (hashmap_get(m-busnames, name))
+return 0;
+
+n = strdup(name);
+if (!n)
+return -ENOMEM;
+
+r = hashmap_put(m-busnames, n, n);
+if (r 0) {
+free(n);
+return r;
+}
+
+return 0;
+}
+
+void manager_drop_busname(Manager *m, const char *name) {
+char *key;
+
+assert(m);
+assert(name);
+
+key = hashmap_remove(m-busnames, name);
+if (key)
+free(key);
+}
+
int manager_process_seat_device(Manager *m, struct udev_device *d) {
Device *device;
int r;
@@ -1039,6 +1079,18 @@ static int manager_connect_bus(Manager *m) {
dbus_bus_add_match(m-bus,
type='signal',
+ sender='DBUS_SERVICE_DBUS',
+ interface='DBUS_INTERFACE_DBUS',
+ member='NameOwnerChanged',
+ path='DBUS_PATH_DBUS',
+ error);
+if
[systemd-devel] [PATCH 03/10] logind: add session controllers
A session usually has only a single compositor or other application that
controls graphics and input devices on it. To avoid multiple applications
from hijacking each other's devices or even using the devices in parallel,
we add session controllers.
A session controller is an application that manages a session. Specific
API calls may be limited to controllers to avoid others from getting
unprivileged access to restricted resources. A session becomes a
controller by calling the RequestControl() dbus API call. It can drop it
via ReleaseControl().
logind tracks bus-names to release the controller once an application
closes the bus. We use the new bus-name tracking to do that. Note that
during ReleaseControl() we need to check whether some other session also
tracks the name before we remove it from the bus-name tracking list.
Currently, we only allow one controller at a time. However, the public API
does not enforce this restriction. So if it makes sense, we can allow
multiple controllers in parallel later. Or we can add a scope parameter,
which allows a different controller for graphics-devices, sound-devices
and whatever you want.
Note that currently you get -EBUSY if there is already a controller. You
can force the RequestControl() call (root-only) to drop the current
controller and recover the session during an emergency. To recover a seat,
this is not needed, though. You can simply create a new session or
force-activate it.
To become a session controller, a dbus caller must either be root or the
same user as the user of the session. This allows us to run a session
compositor as user and we no longer need any CAP_SYS_ADMIN.
---
src/login/logind-dbus.c | 8 +++
src/login/logind-session-dbus.c | 42
src/login/logind-session.c | 48 +
src/login/logind-session.h | 6 ++
src/login/logind.c | 10 +
5 files changed, 114 insertions(+)
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index eb62d28..a703e59 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -2472,8 +2472,16 @@ DBusHandlerResult bus_message_filter(
goto finish;
}
+/* drop all controllers owned by this name */
if (*old !*new (key = hashmap_remove(m-busnames, old)))
{
+Session *session;
+Iterator i;
+
free(key);
+
+HASHMAP_FOREACH(session, m-sessions, i)
+if (session_is_controller(session, old))
+session_drop_controller(session);
}
}
diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c
index 2cc4d85..b8b32cd 100644
--- a/src/login/logind-session-dbus.c
+++ b/src/login/logind-session-dbus.c
@@ -40,6 +40,10 @@
arg name=\who\ type=\s\/\n \
arg name=\signal\ type=\s\/\n\
/method\n \
+ method name=\RequestControl\/\n \
+ arg name=\force\ type=\b\/\n \
+ /method\n \
+ method name=\DropControl\/\n\
signal name=\Lock\/\n \
signal name=\Unlock\/\n \
property name=\Id\ type=\s\ access=\read\/\n\
@@ -366,6 +370,44 @@ static DBusHandlerResult session_message_dispatch(
if (!reply)
goto oom;
+} else if (dbus_message_is_method_call(message,
org.freedesktop.login1.Session, RequestControl)) {
+dbus_bool_t force;
+unsigned long ul;
+
+if (!dbus_message_get_args(
+message,
+error,
+DBUS_TYPE_BOOLEAN, force,
+DBUS_TYPE_INVALID))
+return bus_send_error_reply(connection, message,
error, -EINVAL);
+
+ul = dbus_bus_get_unix_user(connection,
dbus_message_get_sender(message), error);
+if (ul == (unsigned long) -1)
+return bus_send_error_reply(connection, message,
error, -EIO);
+
+if (ul != 0 (force || ul != s-user-uid))
+return bus_send_error_reply(connection, message, NULL,
-EPERM);
+
+r = session_set_controller(s,
bus_message_get_sender_with_fallback(message), force);
+if (r 0)
+return bus_send_error_reply(connection, message, NULL,
r);
+
+reply =
[systemd-devel] [PATCH 04/10] logind: make Session.Activate() lazy
Currently, Activate() calls chvt(), which does an ioctl(VT_ACTIVATE) and
immediately calls seat_set_active(). However, VTs are allowed to prevent
being deactivated. Therefore, logind cannot be sure the VT_ACTIVATE call
was actually successful.
Furthermore, compositors often need to clean up their devices before they
acknowledge the VT switch. The immediate call to seat_set_active() may
modify underlying ACLs, though. Thus, some compositors may fail cleaning
up their stuff. Moreover, the compositor being switched to (if listening
to logind instead of VTs) will not be able to activate its devices if the
old VT still has them active.
We could simply add an VT_WAITACTIVE call, which blocks until the given VT
is active. However, this can block forever if the compositor hangs.
So to fix this, we make Activate() lazy. That is, it only schedules a
session-switch but does not wait for it to complete. The caller can no
longer rely on it being immediate. Instead, a caller is required to wait
for the PropertiesChanged signal and read the Active field.
We could make Activate() wait asynchronously for the session-switch to
complete and then send the return-message afterwards. However, this would
add a lot of state-tracking with no real gain:
1) Sessions normally don't care whether Activate() was actually
successful as they currently _must_ wait for the VT activation to do
anything for real.
2) Error messages for failed session switches can be printed by logind
instead of the session issuing Activate().
3) Sessions that require synchronous Activate() calls can simply issue
the call and then wait for Active properties to change. This also
allows them to implement their own timeout.
This change prepares for multi-session on seats without VTs. Forced VT
switches are always bad as compositors cannot perform any cleanup. This
isn't strictly required, but may lead to loss of information and ambiguous
error messages.
So for multi-session on seats without VTs, we must wait for the current
session to clean-up before finalizing the session-switch. This requires
Activate() to be lazy as we cannot block here.
Note that we can always implement a timeout which allows us to guarantee
the session switch to happen. Nevertheless, this calls for a lazy
Activate().
---
src/login/logind-session.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 6c6a2c8..a11804a 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -359,8 +359,6 @@ int session_load(Session *s) {
}
int session_activate(Session *s) {
-int r;
-
assert(s);
assert(s-user);
@@ -375,11 +373,7 @@ int session_activate(Session *s) {
assert(seat_is_vtconsole(s-seat));
-r = chvt(s-vtnr);
-if (r 0)
-return r;
-
-return seat_set_active(s-seat, s);
+return chvt(s-vtnr);
}
static int session_link_x11_socket(Session *s) {
--
1.8.3.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 05/10] logind: introduce session-devices
\ type=\s\ access=\read\/\n\
@@ -408,6 +428,100 @@ static DBusHandlerResult session_message_dispatch(
if (!reply)
goto oom;
+} else if (dbus_message_is_method_call(message,
org.freedesktop.login1.Session, RequestDevice)) {
+const char *node;
+SessionDevice *sd;
+bool b;
+dbus_bool_t paused;
+
+if (!session_is_controller(s,
bus_message_get_sender_with_fallback(message)))
+return bus_send_error_reply(connection, message, NULL,
-EPERM);
+
+if (!dbus_message_get_args(
+message,
+error,
+DBUS_TYPE_STRING, node,
+DBUS_TYPE_INVALID))
+return bus_send_error_reply(connection, message,
error, -EINVAL);
+
+sd = hashmap_get(s-devices, node);
+if (sd) {
+/* We don't allow retrieving a device multiple times.
+ * The related ReleaseDevice call is not ref-counted.
+ * The caller should use dup() if it requires more than
+ * one fd (it would be functionally equivalent). */
+return bus_send_error_reply(connection, message,
error, -EBUSY);
+}
+
+r = session_device_new(s, node, sd);
+if (r 0)
+return bus_send_error_reply(connection, message, NULL,
r);
+
+reply = dbus_message_new_method_return(message);
+if (!reply) {
+session_device_free(sd);
+goto oom;
+}
+
+paused = !sd-active;
+b = dbus_message_append_args(
+reply,
+DBUS_TYPE_UNIX_FD, sd-fd,
+DBUS_TYPE_BOOLEAN, paused,
+DBUS_TYPE_INVALID);
+if (!b) {
+session_device_free(sd);
+return bus_send_error_reply(connection, message, NULL,
-ENOMEM);
+}
+
+} else if (dbus_message_is_method_call(message,
org.freedesktop.login1.Session, ReleaseDevice)) {
+const char *node;
+SessionDevice *sd;
+
+if (!session_is_controller(s,
bus_message_get_sender_with_fallback(message)))
+return bus_send_error_reply(connection, message, NULL,
-EPERM);
+
+if (!dbus_message_get_args(
+message,
+error,
+DBUS_TYPE_STRING, node,
+DBUS_TYPE_INVALID))
+return bus_send_error_reply(connection, message,
error, -EINVAL);
+
+sd = hashmap_get(s-devices, node);
+if (!sd)
+return bus_send_error_reply(connection, message, NULL,
-ENODEV);
+
+session_device_free(sd);
+
+reply = dbus_message_new_method_return(message);
+if (!reply)
+goto oom;
+
+} else if (dbus_message_is_method_call(message,
org.freedesktop.login1.Session, PauseDeviceComplete)) {
+const char *node;
+SessionDevice *sd;
+
+if (!session_is_controller(s,
bus_message_get_sender_with_fallback(message)))
+return bus_send_error_reply(connection, message, NULL,
-EPERM);
+
+if (!dbus_message_get_args(
+message,
+error,
+DBUS_TYPE_STRING, node,
+DBUS_TYPE_INVALID))
+return bus_send_error_reply(connection, message,
error, -EINVAL);
+
+sd = hashmap_get(s-devices, node);
+if (!sd)
+return bus_send_error_reply(connection, message, NULL,
-ENODEV);
+
+session_device_complete_pause(sd);
+
+reply = dbus_message_new_method_return(message);
+if (!reply)
+goto oom;
+
} else {
const BusBoundProperties bps[] = {
{ org.freedesktop.login1.Session,
bus_login_session_properties, s },
diff --git a/src/login/logind-session-device.c
b/src/login/logind-session-device.c
new file mode 100644
index 000..7c6b07f
--- /dev/null
+++ b/src/login/logind-session-device.c
@@ -0,0 +1,489 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright 2013 David Herrmann
+
+ systemd is free
[systemd-devel] [PATCH 06/10] logind: rename vtconsole to seat0
The seat-vtconsole member always points to the default seat seat0. Even
if VTs are disabled, it's used as default seat. Therefore, rename it to
seat0 to correctly state what it is.
This also changes the seat files in /run from IS_VTCONSOLE to IS_SEAT0. It
wasn't used by any code, yet, so this seems fine.
While we are at it, we also remove every if (s-vtconsole) as this
pointer is always valid!
---
src/login/logind-dbus.c| 8
src/login/logind-seat.c| 14 +++---
src/login/logind-seat.h| 2 +-
src/login/logind-session.c | 2 +-
src/login/logind.c | 8
src/login/logind.h | 2 +-
6 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index a703e59..8940aeb 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -404,8 +404,8 @@ static int bus_manager_create_session(Manager *m,
DBusMessage *message) {
int v;
if (!seat)
-seat = m-vtconsole;
-else if (seat != m-vtconsole)
+seat = m-seat0;
+else if (seat != m-seat0)
return -EINVAL;
v = vtnr_from_tty(tty);
@@ -420,8 +420,8 @@ static int bus_manager_create_session(Manager *m,
DBusMessage *message) {
} else if (tty_is_console(tty)) {
if (!seat)
-seat = m-vtconsole;
-else if (seat != m-vtconsole)
+seat = m-seat0;
+else if (seat != m-seat0)
return -EINVAL;
if (vtnr != 0)
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index dcaf0ac..88fd724 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -105,11 +105,11 @@ int seat_save(Seat *s) {
fprintf(f,
# This is private data. Do not parse.\n
-IS_VTCONSOLE=%i\n
+IS_SEAT0=%i\n
CAN_MULTI_SESSION=%i\n
CAN_TTY=%i\n
CAN_GRAPHICAL=%i\n,
-seat_is_vtconsole(s),
+seat_is_seat0(s),
seat_can_multi_session(s),
seat_can_tty(s),
seat_can_graphical(s));
@@ -429,16 +429,16 @@ int seat_attach_session(Seat *s, Session *session) {
return 0;
}
-bool seat_is_vtconsole(Seat *s) {
+bool seat_is_seat0(Seat *s) {
assert(s);
-return s-manager-vtconsole == s;
+return s-manager-seat0 == s;
}
bool seat_can_multi_session(Seat *s) {
assert(s);
-if (!seat_is_vtconsole(s))
+if (!seat_is_seat0(s))
return false;
/* If we can't watch which VT is in the foreground, we don't
@@ -450,7 +450,7 @@ bool seat_can_multi_session(Seat *s) {
bool seat_can_tty(Seat *s) {
assert(s);
-return seat_is_vtconsole(s);
+return seat_is_seat0(s);
}
bool seat_has_master_device(Seat *s) {
@@ -508,7 +508,7 @@ int seat_check_gc(Seat *s, bool drop_not_started) {
if (drop_not_started !s-started)
return 0;
-if (seat_is_vtconsole(s))
+if (seat_is_seat0(s))
return 1;
return seat_has_master_device(s);
diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index bd5390f..47fe89a 100644
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -60,7 +60,7 @@ int seat_preallocate_vts(Seat *s);
int seat_attach_session(Seat *s, Session *session);
-bool seat_is_vtconsole(Seat *s);
+bool seat_is_seat0(Seat *s);
bool seat_can_multi_session(Seat *s);
bool seat_can_tty(Seat *s);
bool seat_has_master_device(Seat *s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 69a8ad7..ae91650 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -386,7 +386,7 @@ int session_activate(Session *s) {
if (s-seat-active == s)
return 0;
-assert(seat_is_vtconsole(s-seat));
+assert(seat_is_seat0(s-seat));
return chvt(s-vtnr);
}
diff --git a/src/login/logind.c b/src/login/logind.c
index ad3ab81..d18bb08 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -852,7 +852,7 @@ int manager_dispatch_vcsa_udev(Manager *m) {
* VTs, to make sure our auto VTs never go away. */
if (name startswith(name, vcsa)
streq_ptr(udev_device_get_action(d), remove))
-r = seat_preallocate_vts(m-vtconsole);
+r = seat_preallocate_vts(m-seat0);
udev_device_unref(d);
@@ -877,9 +877,9 @@ int manager_dispatch_button_udev(Manager *m) {
int manager_dispatch_console(Manager *m) {
assert(m);
+assert(m-seat0);
-if (m-vtconsole)
-seat_read_active_vt(m-vtconsole);
+seat_read_active_vt(m-seat0);
return 0;
}
@@ -1537,7 +1537,7 @@ int
[systemd-devel] [PATCH 07/10] logind: fix seat_can_tty() to check for VTs
A seat provides text-logins if it has VTs. This is always limited to seat0
so the seat_is_seat0() check is correct. However, if VTs are disabled, no
seat provides text-logins so we also need to check for the console-fd.
This was previously:
return seat_is_vtconsole();
It looked right, but was functionally equivalent to seat_is_seat0(). The
rename of this helper made it more obvious that it is missing the VT test.
---
src/login/logind-seat.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index 88fd724..4c2c424 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -450,7 +450,7 @@ bool seat_can_multi_session(Seat *s) {
bool seat_can_tty(Seat *s) {
assert(s);
-return seat_is_seat0(s);
+return seat_is_seat0(s) s-manager-console_active_fd = 0;
}
bool seat_has_master_device(Seat *s) {
--
1.8.3.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 08/10] logind: fix session_activate(vtnr = 0)
VT numbers start with 1. If a session has vtnr == 0, we must not assume it
is running on a VT.
Note that this could trigger the assert() below as CreateSession() sets
vtnr to 0, not 0.
---
src/login/logind-session.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index ae91650..50ba6b8 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -377,7 +377,7 @@ int session_activate(Session *s) {
assert(s);
assert(s-user);
-if (s-vtnr 0)
+if (s-vtnr = 0)
return -ENOTSUP;
if (!s-seat)
--
1.8.3.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 09/10] logind: extract has_vts() from can_multi_session()
We currently use seat_can_multi_session() to test for two things:
* whether the seat can handle session-switching
* whether the seat has VTs
As both are currently logically equivalent, we didn't care. However, we
want to allow session-switching on seats without VTs, so split this helper
into:
* seat_can_multi_session(): whether session-switching is supported
* seat_has_vts(): whether the seat has VTs
Note that only one seat on a system can have VTs. There is only one set of
them. We automatically assign them to seat0 as usual.
With this patch in place, we can easily add new session-switching/tracking
methods without breaking any VT code as it is now protected by has_vts(),
no longer by can_multi_session().
---
src/login/logind-dbus.c| 2 +-
src/login/logind-seat.c| 32 ++--
src/login/logind-seat.h| 1 +
src/login/logind-session.c | 6 +++---
4 files changed, 19 insertions(+), 22 deletions(-)
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 8940aeb..5e42c0a 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -429,7 +429,7 @@ static int bus_manager_create_session(Manager *m,
DBusMessage *message) {
}
if (seat) {
-if (seat_can_multi_session(seat)) {
+if (seat_has_vts(seat)) {
if (vtnr 63)
return -EINVAL;
} else {
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index 4c2c424..f88738a 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -201,7 +201,7 @@ int seat_preallocate_vts(Seat *s) {
if (s-manager-n_autovts = 0)
return 0;
-if (!seat_can_multi_session(s))
+if (!seat_has_vts(s))
return 0;
for (i = 1; i = s-manager-n_autovts; i++) {
@@ -282,7 +282,7 @@ int seat_active_vt_changed(Seat *s, int vtnr) {
assert(s);
assert(vtnr = 1);
-if (!seat_can_multi_session(s))
+if (!seat_has_vts(s))
return -EINVAL;
log_debug(VT changed to %i, vtnr);
@@ -306,7 +306,7 @@ int seat_read_active_vt(Seat *s) {
assert(s);
-if (!seat_can_multi_session(s))
+if (!seat_has_vts(s))
return 0;
lseek(s-manager-console_active_fd, SEEK_SET, 0);
@@ -417,18 +417,20 @@ int seat_attach_session(Seat *s, Session *session) {
seat_send_changed(s, Sessions\0);
-/* Note that even if a seat is not multi-session capable it
- * still might have multiple sessions on it since old, dead
- * sessions might continue to be tracked until all their
- * processes are gone. The most recently added session
- * (i.e. the first in s-sessions) is the one that matters. */
-
-if (!seat_can_multi_session(s))
+/* On seats with VTs, the VT logic defines which session is active. On
+ * seats without VTs, we automatically activate the first session. */
+if (!seat_has_vts(s) !s-active)
seat_set_active(s, session);
return 0;
}
+bool seat_has_vts(Seat *s) {
+assert(s);
+
+return seat_is_seat0(s) s-manager-console_active_fd = 0;
+}
+
bool seat_is_seat0(Seat *s) {
assert(s);
@@ -438,19 +440,13 @@ bool seat_is_seat0(Seat *s) {
bool seat_can_multi_session(Seat *s) {
assert(s);
-if (!seat_is_seat0(s))
-return false;
-
-/* If we can't watch which VT is in the foreground, we don't
- * support VT switching */
-
-return s-manager-console_active_fd = 0;
+return seat_has_vts(s);
}
bool seat_can_tty(Seat *s) {
assert(s);
-return seat_is_seat0(s) s-manager-console_active_fd = 0;
+return seat_has_vts(s);
}
bool seat_has_master_device(Seat *s) {
diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index 47fe89a..d3438b8 100644
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -60,6 +60,7 @@ int seat_preallocate_vts(Seat *s);
int seat_attach_session(Seat *s, Session *session);
+bool seat_has_vts(Seat *s);
bool seat_is_seat0(Seat *s);
bool seat_can_multi_session(Seat *s);
bool seat_can_tty(Seat *s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 50ba6b8..e7fe0f6 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -205,7 +205,7 @@ int session_save(Session *s) {
if (s-service)
fprintf(f, SERVICE=%s\n, s-service);
-if (s-seat seat_can_multi_session(s-seat))
+if (s-seat seat_has_vts(s-seat))
fprintf(f, VTNR=%i\n, s-vtnr);
if (s-leader 0)
@@ -315,7 +315,7 @@ int session_load(Session *s) {
seat_attach_session(o, s);
}
-if (vtnr s-seat seat_can_multi_session(s-seat)) {
+if (vtnr s-seat seat_has_vts(s-seat)) {
[systemd-devel] [PATCH 10/10] logind: implement generic multi-session
This enables the multi-session capability for seats that don't have VTs.
For legacy seats with VTs, everything stays the same. However, all other
seats now also get the multi-session capability.
The only feature that was missing was session-switching. As logind can
force a session-switch and signal that via the Active property, we only
need a way to allow synchronized/delayed session switches. Compositors
need to cleanup some devices before acknowledging the session switch.
Therefore, we use the session-devices to give compositors a chance to
block a session-switch until they cleaned everything up.
If you activate a session on a seat without VTs, we send a PauseDevice
signal to the active session for every active device. Only once the
session acknowledged all these with a PauseDeviceComplete() call, we
perform the final session switch.
One important note is that delayed session-switching is meant for
backwards compatibility. New compositors or other sessions should really
try to deal correctly with forced session switches! They only need to
handle EACCES/EPERM from syscalls and treat them as PauseDevice signal.
Following logind patches will add a timeout to session-switches which
forces the switch if the active session does not react in a timely
fashion. Moreover, explicit ForceActivate() calls might also be supported.
Hence, sessions must not crash if their devices get paused.
---
src/login/logind-seat.c | 15 +++
src/login/logind-seat.h | 2 ++
src/login/logind-session-device.c | 28
src/login/logind-session-device.h | 1 +
src/login/logind-session.c| 31 ++-
5 files changed, 72 insertions(+), 5 deletions(-)
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index f88738a..4a4d40a 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -425,6 +425,21 @@ int seat_attach_session(Seat *s, Session *session) {
return 0;
}
+void seat_complete_switch(Seat *s) {
+Session *session;
+
+assert(s);
+
+/* if no session-switch is pending or if it got canceled, do nothing */
+if (!s-pending_switch)
+return;
+
+session = s-pending_switch;
+s-pending_switch = NULL;
+
+seat_set_active(s, session);
+}
+
bool seat_has_vts(Seat *s) {
assert(s);
diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index d3438b8..be6db6e 100644
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -38,6 +38,7 @@ struct Seat {
LIST_HEAD(Device, devices);
Session *active;
+Session *pending_switch;
LIST_HEAD(Session, sessions);
bool in_gc_queue:1;
@@ -59,6 +60,7 @@ int seat_read_active_vt(Seat *s);
int seat_preallocate_vts(Seat *s);
int seat_attach_session(Seat *s, Session *session);
+void seat_complete_switch(Seat *s);
bool seat_has_vts(Seat *s);
bool seat_is_seat0(Seat *s);
diff --git a/src/login/logind-session-device.c
b/src/login/logind-session-device.c
index 7c6b07f..7123955 100644
--- a/src/login/logind-session-device.c
+++ b/src/login/logind-session-device.c
@@ -452,10 +452,21 @@ void session_device_free(SessionDevice *sd) {
}
void session_device_complete_pause(SessionDevice *sd) {
+SessionDevice *iter;
+Iterator i;
+
if (!sd-active)
return;
session_device_stop(sd);
+
+/* if not all devices are paused, wait for further completion events */
+HASHMAP_FOREACH(iter, sd-session-devices, i)
+if (iter-active)
+return;
+
+/* complete any pending session switch */
+seat_complete_switch(sd-session-seat);
}
void session_device_resume_all(Session *s) {
@@ -487,3 +498,20 @@ void session_device_pause_all(Session *s) {
}
}
}
+
+unsigned int session_device_try_pause_all(Session *s) {
+SessionDevice *sd;
+Iterator i;
+unsigned int num_pending = 0;
+
+assert(s);
+
+HASHMAP_FOREACH(sd, s-devices, i) {
+if (sd-active) {
+session_device_notify(sd, SESSION_DEVICE_TRY_PAUSE);
+++num_pending;
+}
+}
+
+return num_pending;
+}
diff --git a/src/login/logind-session-device.h
b/src/login/logind-session-device.h
index 98e61e6..61732bc 100644
--- a/src/login/logind-session-device.h
+++ b/src/login/logind-session-device.h
@@ -55,3 +55,4 @@ void session_device_complete_pause(SessionDevice *sd);
void session_device_resume_all(Session *s);
void session_device_pause_all(Session *s);
+unsigned int session_device_try_pause_all(Session *s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index e7fe0f6..61366c1 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -99,6 +99,8 @@ void session_free(Session *s) {
if (s-seat) {
[systemd-devel] [PATCH] libudev: enumerate: fix NULL-deref for subsystem-matches
udev_device_get_subsystem() may return NULL if no subsystem could be
figured out by libudev. This might be due to OOM or if the device
disconnected between the udev_device_new() call and
udev_device_get_subsystem(). Therefore, we need to handle subsystem==NULL
safely.
Instead of testing for it in each helper, we treat subsystem==NULL as
empty subsystem in match_subsystem().
Backtrace of udev_enumerate with an input-device disconnecting in exactly
this time-frame:
(gdb) bt
#0 0x7569dc24 in strnlen () from /usr/lib/libc.so.6
#1 0x756d9e04 in fnmatch@@GLIBC_2.2.5 () from /usr/lib/libc.so.6
#2 0x75beb83d in match_subsystem (udev_enumerate=0x7a05f0,
subsystem=0x0) at src/libudev/libudev-enumerate.c:727
#3 0x75bebb30 in parent_add_child
(enumerate=enumerate@entry=0x7a05f0, path=optimized out) at
src/libudev/libudev-enumerate.c:834
#4 0x75bebc3f in parent_crawl_children
(enumerate=enumerate@entry=0x7a05f0, path=0x7a56b0
/sys/devices/shortened/input/input97, maxdepth=maxdepth@entry=254) at
src/libudev/libudev-enumerate.c:866
#5 0x75bebc54 in parent_crawl_children
(enumerate=enumerate@entry=0x7a05f0, path=0x79e8c0
/sys/devices/shortened/input, maxdepth=maxdepth@entry=255) at
src/libudev/libudev-enumerate.c:868
#6 0x75bebc54 in parent_crawl_children
(enumerate=enumerate@entry=0x7a05f0, path=path@entry=0x753190
/sys/devices/shortened, maxdepth=maxdepth@entry=256) at
src/libudev/libudev-enumerate.c:868
#7 0x75bec7df in scan_devices_children (enumerate=0x7a05f0) at
src/libudev/libudev-enumerate.c:882
#8 udev_enumerate_scan_devices (udev_enumerate=udev_enumerate@entry=0x7a05f0)
at src/libudev/libudev-enumerate.c:919
#9 0x75df8777 in random_caller () at some/file.c:181
---
src/libudev/libudev-enumerate.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/libudev/libudev-enumerate.c b/src/libudev/libudev-enumerate.c
index 5ccaabd..7065719 100644
--- a/src/libudev/libudev-enumerate.c
+++ b/src/libudev/libudev-enumerate.c
@@ -718,6 +718,8 @@ static bool match_subsystem(struct udev_enumerate
*udev_enumerate, const char *s
{
struct udev_list_entry *list_entry;
+subsystem = subsystem ? : ;
+
udev_list_entry_foreach(list_entry,
udev_list_get_entry(udev_enumerate-subsystem_nomatch_list)) {
if (fnmatch(udev_list_entry_get_name(list_entry), subsystem,
0) == 0)
return false;
--
1.8.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] build: check for build/link flags harder
Use AC_LINK_IFELSE instead of AC_COMPILE_IFELSE to test for flags that
might succeed during compilation but not during linking. An example is gcc
compiled with libssp support but gnu-ld without it. In this case
-fstack-protector works fine during compilation but fails during linking
as several internal helpers are missing.
---
m4/attributes.m4 | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/m4/attributes.m4 b/m4/attributes.m4
index 7e080da..aa53ef2 100644
--- a/m4/attributes.m4
+++ b/m4/attributes.m4
@@ -42,9 +42,9 @@ AC_DEFUN([CC_CHECK_FLAG_APPEND], [
AS_TR_SH([cc_cv_$2_$3]),
[eval AS_TR_SH([cc_save_$2])='${$2}'
eval AS_TR_SH([$2])='-Werror $3'
- AC_COMPILE_IFELSE([AC_LANG_SOURCE([int a = 0; int main(void) {
return a; } ])],
-[eval AS_TR_SH([cc_cv_$2_$3])='yes'],
-[eval AS_TR_SH([cc_cv_$2_$3])='no'])
+ AC_LINK_IFELSE([AC_LANG_SOURCE([int a = 0; int main(void) { return
a; } ])],
+ [eval AS_TR_SH([cc_cv_$2_$3])='yes'],
+ [eval AS_TR_SH([cc_cv_$2_$3])='no'])
eval AS_TR_SH([$2])='$cc_save_$2'])
AS_IF([eval test x$]AS_TR_SH([cc_cv_$2_$3])[ = xyes],
--
1.8.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2 04/10] logind: make Session.Activate() lazy
Currently, Activate() calls chvt(), which does an ioctl(VT_ACTIVATE) and
immediately calls seat_set_active(). However, VTs are allowed to prevent
being deactivated. Therefore, logind cannot be sure the VT_ACTIVATE call
was actually successful.
Furthermore, compositors often need to clean up their devices before they
acknowledge the VT switch. The immediate call to seat_set_active() may
modify underlying ACLs, though. Thus, some compositors may fail cleaning
up their stuff. Moreover, the compositor being switched to (if listening
to logind instead of VTs) will not be able to activate its devices if the
old VT still has them active.
We could simply add an VT_WAITACTIVE call, which blocks until the given VT
is active. However, this can block forever if the compositor hangs.
So to fix this, we make Activate() lazy. That is, it only schedules a
session-switch but does not wait for it to complete. The caller can no
longer rely on it being immediate. Instead, a caller is required to wait
for the PropertiesChanged signal and read the Active field.
We could make Activate() wait asynchronously for the session-switch to
complete and then send the return-message afterwards. However, this would
add a lot of state-tracking with no real gain:
1) Sessions normally don't care whether Activate() was actually
successful as they currently _must_ wait for the VT activation to do
anything for real.
2) Error messages for failed session switches can be printed by logind
instead of the session issuing Activate().
3) Sessions that require synchronous Activate() calls can simply issue
the call and then wait for Active properties to change. This also
allows them to implement their own timeout.
This change prepares for multi-session on seats without VTs. Forced VT
switches are always bad as compositors cannot perform any cleanup. This
isn't strictly required, but may lead to loss of information and ambiguous
error messages.
So for multi-session on seats without VTs, we must wait for the current
session to clean-up before finalizing the session-switch. This requires
Activate() to be lazy as we cannot block here.
Note that we can always implement a timeout which allows us to guarantee
the session switch to happen. Nevertheless, this calls for a lazy
Activate().
---
src/login/logind-session.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index fe5fa27..fa8b515 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -360,8 +360,6 @@ int session_load(Session *s) {
}
int session_activate(Session *s) {
-int r;
-
assert(s);
assert(s-user);
@@ -376,11 +374,7 @@ int session_activate(Session *s) {
assert(seat_is_vtconsole(s-seat));
-r = chvt(s-vtnr);
-if (r 0)
-return r;
-
-return seat_set_active(s-seat, s);
+return chvt(s-vtnr);
}
static int session_link_x11_socket(Session *s) {
--
1.8.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2 03/10] logind: add session controllers
A session usually has only a single compositor or other application that
controls graphics and input devices on it. To avoid multiple applications
from hijacking each other's devices or even using the devices in parallel,
we add session controllers.
A session controller is an application that manages a session. Specific
API calls may be limited to controllers to avoid others from getting
unprivileged access to restricted resources. A session becomes a
controller by calling the RequestControl() dbus API call. It can drop it
via ReleaseControl().
logind tracks bus-names to release the controller once an application
closes the bus. We use the new bus-name tracking to do that. Note that
during ReleaseControl() we need to check whether some other session also
tracks the name before we remove it from the bus-name tracking list.
Currently, we only allow one controller at a time. However, the public API
does not enforce this restriction. So if it makes sense, we can allow
multiple controllers in parallel later. Or we can add a scope parameter,
which allows a different controller for graphics-devices, sound-devices
and whatever you want.
Note that currently you get -EBUSY if there is already a controller. You
can force the RequestControl() call (root-only) to drop the current
controller and recover the session during an emergency. To recover a seat,
this is not needed, though. You can simply create a new session or
force-activate it.
To become a session controller, a dbus caller must either be root or the
same user as the user of the session. This allows us to run a session
compositor as user and we no longer need any CAP_SYS_ADMIN.
---
src/login/logind-dbus.c | 8 +++
src/login/logind-session-dbus.c | 42
src/login/logind-session.c | 48 +
src/login/logind-session.h | 6 ++
src/login/logind.c | 10 +
5 files changed, 114 insertions(+)
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 5decf81..113a2b7 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -2473,8 +2473,16 @@ DBusHandlerResult bus_message_filter(
goto finish;
}
+/* drop all controllers owned by this name */
if (*old !*new (key = hashmap_remove(m-busnames, old)))
{
+Session *session;
+Iterator i;
+
free(key);
+
+HASHMAP_FOREACH(session, m-sessions, i)
+if (session_is_controller(session, old))
+session_drop_controller(session);
}
}
diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c
index 2cc4d85..b8b32cd 100644
--- a/src/login/logind-session-dbus.c
+++ b/src/login/logind-session-dbus.c
@@ -40,6 +40,10 @@
arg name=\who\ type=\s\/\n \
arg name=\signal\ type=\s\/\n\
/method\n \
+ method name=\RequestControl\/\n \
+ arg name=\force\ type=\b\/\n \
+ /method\n \
+ method name=\DropControl\/\n\
signal name=\Lock\/\n \
signal name=\Unlock\/\n \
property name=\Id\ type=\s\ access=\read\/\n\
@@ -366,6 +370,44 @@ static DBusHandlerResult session_message_dispatch(
if (!reply)
goto oom;
+} else if (dbus_message_is_method_call(message,
org.freedesktop.login1.Session, RequestControl)) {
+dbus_bool_t force;
+unsigned long ul;
+
+if (!dbus_message_get_args(
+message,
+error,
+DBUS_TYPE_BOOLEAN, force,
+DBUS_TYPE_INVALID))
+return bus_send_error_reply(connection, message,
error, -EINVAL);
+
+ul = dbus_bus_get_unix_user(connection,
dbus_message_get_sender(message), error);
+if (ul == (unsigned long) -1)
+return bus_send_error_reply(connection, message,
error, -EIO);
+
+if (ul != 0 (force || ul != s-user-uid))
+return bus_send_error_reply(connection, message, NULL,
-EPERM);
+
+r = session_set_controller(s,
bus_message_get_sender_with_fallback(message), force);
+if (r 0)
+return bus_send_error_reply(connection, message, NULL,
r);
+
+reply =
[systemd-devel] [PATCH v2 06/10] logind: rename vtconsole to seat0
The seat-vtconsole member always points to the default seat seat0. Even
if VTs are disabled, it's used as default seat. Therefore, rename it to
seat0 to correctly state what it is.
This also changes the seat files in /run from IS_VTCONSOLE to IS_SEAT0. It
wasn't used by any code, yet, so this seems fine.
While we are at it, we also remove every if (s-vtconsole) as this
pointer is always valid!
---
src/login/logind-dbus.c| 8
src/login/logind-seat.c| 14 +++---
src/login/logind-seat.h| 2 +-
src/login/logind-session.c | 2 +-
src/login/logind.c | 8
src/login/logind.h | 2 +-
6 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 113a2b7..4a23c93 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -404,8 +404,8 @@ static int bus_manager_create_session(Manager *m,
DBusMessage *message) {
int v;
if (!seat)
-seat = m-vtconsole;
-else if (seat != m-vtconsole)
+seat = m-seat0;
+else if (seat != m-seat0)
return -EINVAL;
v = vtnr_from_tty(tty);
@@ -420,8 +420,8 @@ static int bus_manager_create_session(Manager *m,
DBusMessage *message) {
} else if (tty_is_console(tty)) {
if (!seat)
-seat = m-vtconsole;
-else if (seat != m-vtconsole)
+seat = m-seat0;
+else if (seat != m-seat0)
return -EINVAL;
if (vtnr != 0)
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index dcaf0ac..88fd724 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -105,11 +105,11 @@ int seat_save(Seat *s) {
fprintf(f,
# This is private data. Do not parse.\n
-IS_VTCONSOLE=%i\n
+IS_SEAT0=%i\n
CAN_MULTI_SESSION=%i\n
CAN_TTY=%i\n
CAN_GRAPHICAL=%i\n,
-seat_is_vtconsole(s),
+seat_is_seat0(s),
seat_can_multi_session(s),
seat_can_tty(s),
seat_can_graphical(s));
@@ -429,16 +429,16 @@ int seat_attach_session(Seat *s, Session *session) {
return 0;
}
-bool seat_is_vtconsole(Seat *s) {
+bool seat_is_seat0(Seat *s) {
assert(s);
-return s-manager-vtconsole == s;
+return s-manager-seat0 == s;
}
bool seat_can_multi_session(Seat *s) {
assert(s);
-if (!seat_is_vtconsole(s))
+if (!seat_is_seat0(s))
return false;
/* If we can't watch which VT is in the foreground, we don't
@@ -450,7 +450,7 @@ bool seat_can_multi_session(Seat *s) {
bool seat_can_tty(Seat *s) {
assert(s);
-return seat_is_vtconsole(s);
+return seat_is_seat0(s);
}
bool seat_has_master_device(Seat *s) {
@@ -508,7 +508,7 @@ int seat_check_gc(Seat *s, bool drop_not_started) {
if (drop_not_started !s-started)
return 0;
-if (seat_is_vtconsole(s))
+if (seat_is_seat0(s))
return 1;
return seat_has_master_device(s);
diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index bd5390f..47fe89a 100644
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -60,7 +60,7 @@ int seat_preallocate_vts(Seat *s);
int seat_attach_session(Seat *s, Session *session);
-bool seat_is_vtconsole(Seat *s);
+bool seat_is_seat0(Seat *s);
bool seat_can_multi_session(Seat *s);
bool seat_can_tty(Seat *s);
bool seat_has_master_device(Seat *s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 5a1e621..77eeb9c 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -387,7 +387,7 @@ int session_activate(Session *s) {
if (s-seat-active == s)
return 0;
-assert(seat_is_vtconsole(s-seat));
+assert(seat_is_seat0(s-seat));
return chvt(s-vtnr);
}
diff --git a/src/login/logind.c b/src/login/logind.c
index c99c284..702382a 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -858,7 +858,7 @@ int manager_dispatch_vcsa_udev(Manager *m) {
* VTs, to make sure our auto VTs never go away. */
if (name startswith(name, vcsa)
streq_ptr(udev_device_get_action(d), remove))
-r = seat_preallocate_vts(m-vtconsole);
+r = seat_preallocate_vts(m-seat0);
udev_device_unref(d);
@@ -883,9 +883,9 @@ int manager_dispatch_button_udev(Manager *m) {
int manager_dispatch_console(Manager *m) {
assert(m);
+assert(m-seat0);
-if (m-vtconsole)
-seat_read_active_vt(m-vtconsole);
+seat_read_active_vt(m-seat0);
return 0;
}
@@ -1543,7 +1543,7 @@ int
[systemd-devel] [PATCH v2 05/10] logind: introduce session-devices
\ type=\s\ access=\read\/\n\
@@ -408,6 +428,100 @@ static DBusHandlerResult session_message_dispatch(
if (!reply)
goto oom;
+} else if (dbus_message_is_method_call(message,
org.freedesktop.login1.Session, RequestDevice)) {
+const char *node;
+SessionDevice *sd;
+bool b;
+dbus_bool_t paused;
+
+if (!session_is_controller(s,
bus_message_get_sender_with_fallback(message)))
+return bus_send_error_reply(connection, message, NULL,
-EPERM);
+
+if (!dbus_message_get_args(
+message,
+error,
+DBUS_TYPE_STRING, node,
+DBUS_TYPE_INVALID))
+return bus_send_error_reply(connection, message,
error, -EINVAL);
+
+sd = hashmap_get(s-devices, node);
+if (sd) {
+/* We don't allow retrieving a device multiple times.
+ * The related ReleaseDevice call is not ref-counted.
+ * The caller should use dup() if it requires more than
+ * one fd (it would be functionally equivalent). */
+return bus_send_error_reply(connection, message,
error, -EBUSY);
+}
+
+r = session_device_new(s, node, sd);
+if (r 0)
+return bus_send_error_reply(connection, message, NULL,
r);
+
+reply = dbus_message_new_method_return(message);
+if (!reply) {
+session_device_free(sd);
+goto oom;
+}
+
+paused = !sd-active;
+b = dbus_message_append_args(
+reply,
+DBUS_TYPE_UNIX_FD, sd-fd,
+DBUS_TYPE_BOOLEAN, paused,
+DBUS_TYPE_INVALID);
+if (!b) {
+session_device_free(sd);
+return bus_send_error_reply(connection, message, NULL,
-ENOMEM);
+}
+
+} else if (dbus_message_is_method_call(message,
org.freedesktop.login1.Session, ReleaseDevice)) {
+const char *node;
+SessionDevice *sd;
+
+if (!session_is_controller(s,
bus_message_get_sender_with_fallback(message)))
+return bus_send_error_reply(connection, message, NULL,
-EPERM);
+
+if (!dbus_message_get_args(
+message,
+error,
+DBUS_TYPE_STRING, node,
+DBUS_TYPE_INVALID))
+return bus_send_error_reply(connection, message,
error, -EINVAL);
+
+sd = hashmap_get(s-devices, node);
+if (!sd)
+return bus_send_error_reply(connection, message, NULL,
-ENODEV);
+
+session_device_free(sd);
+
+reply = dbus_message_new_method_return(message);
+if (!reply)
+goto oom;
+
+} else if (dbus_message_is_method_call(message,
org.freedesktop.login1.Session, PauseDeviceComplete)) {
+const char *node;
+SessionDevice *sd;
+
+if (!session_is_controller(s,
bus_message_get_sender_with_fallback(message)))
+return bus_send_error_reply(connection, message, NULL,
-EPERM);
+
+if (!dbus_message_get_args(
+message,
+error,
+DBUS_TYPE_STRING, node,
+DBUS_TYPE_INVALID))
+return bus_send_error_reply(connection, message,
error, -EINVAL);
+
+sd = hashmap_get(s-devices, node);
+if (!sd)
+return bus_send_error_reply(connection, message, NULL,
-ENODEV);
+
+session_device_complete_pause(sd);
+
+reply = dbus_message_new_method_return(message);
+if (!reply)
+goto oom;
+
} else {
const BusBoundProperties bps[] = {
{ org.freedesktop.login1.Session,
bus_login_session_properties, s },
diff --git a/src/login/logind-session-device.c
b/src/login/logind-session-device.c
new file mode 100644
index 000..659f161
--- /dev/null
+++ b/src/login/logind-session-device.c
@@ -0,0 +1,489 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright 2013 David Herrmann
+
+ systemd is free
[systemd-devel] [PATCH v2 10/10] logind: implement generic multi-session
This enables the multi-session capability for seats that don't have VTs.
For legacy seats with VTs, everything stays the same. However, all other
seats now also get the multi-session capability.
The only feature that was missing was session-switching. As logind can
force a session-switch and signal that via the Active property, we only
need a way to allow synchronized/delayed session switches. Compositors
need to cleanup some devices before acknowledging the session switch.
Therefore, we use the session-devices to give compositors a chance to
block a session-switch until they cleaned everything up.
If you activate a session on a seat without VTs, we send a PauseDevice
signal to the active session for every active device. Only once the
session acknowledged all these with a PauseDeviceComplete() call, we
perform the final session switch.
One important note is that delayed session-switching is meant for
backwards compatibility. New compositors or other sessions should really
try to deal correctly with forced session switches! They only need to
handle EACCES/EPERM from syscalls and treat them as PauseDevice signal.
Following logind patches will add a timeout to session-switches which
forces the switch if the active session does not react in a timely
fashion. Moreover, explicit ForceActivate() calls might also be supported.
Hence, sessions must not crash if their devices get paused.
---
src/login/logind-seat.c | 15 +++
src/login/logind-seat.h | 2 ++
src/login/logind-session-device.c | 28
src/login/logind-session-device.h | 1 +
src/login/logind-session.c| 31 ++-
5 files changed, 72 insertions(+), 5 deletions(-)
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index f88738a..4a4d40a 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -425,6 +425,21 @@ int seat_attach_session(Seat *s, Session *session) {
return 0;
}
+void seat_complete_switch(Seat *s) {
+Session *session;
+
+assert(s);
+
+/* if no session-switch is pending or if it got canceled, do nothing */
+if (!s-pending_switch)
+return;
+
+session = s-pending_switch;
+s-pending_switch = NULL;
+
+seat_set_active(s, session);
+}
+
bool seat_has_vts(Seat *s) {
assert(s);
diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index d3438b8..be6db6e 100644
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -38,6 +38,7 @@ struct Seat {
LIST_HEAD(Device, devices);
Session *active;
+Session *pending_switch;
LIST_HEAD(Session, sessions);
bool in_gc_queue:1;
@@ -59,6 +60,7 @@ int seat_read_active_vt(Seat *s);
int seat_preallocate_vts(Seat *s);
int seat_attach_session(Seat *s, Session *session);
+void seat_complete_switch(Seat *s);
bool seat_has_vts(Seat *s);
bool seat_is_seat0(Seat *s);
diff --git a/src/login/logind-session-device.c
b/src/login/logind-session-device.c
index 659f161..af9d5a1 100644
--- a/src/login/logind-session-device.c
+++ b/src/login/logind-session-device.c
@@ -452,10 +452,21 @@ void session_device_free(SessionDevice *sd) {
}
void session_device_complete_pause(SessionDevice *sd) {
+SessionDevice *iter;
+Iterator i;
+
if (!sd-active)
return;
session_device_stop(sd);
+
+/* if not all devices are paused, wait for further completion events */
+HASHMAP_FOREACH(iter, sd-session-devices, i)
+if (iter-active)
+return;
+
+/* complete any pending session switch */
+seat_complete_switch(sd-session-seat);
}
void session_device_resume_all(Session *s) {
@@ -487,3 +498,20 @@ void session_device_pause_all(Session *s) {
}
}
}
+
+unsigned int session_device_try_pause_all(Session *s) {
+SessionDevice *sd;
+Iterator i;
+unsigned int num_pending = 0;
+
+assert(s);
+
+HASHMAP_FOREACH(sd, s-devices, i) {
+if (sd-active) {
+session_device_notify(sd, SESSION_DEVICE_TRY_PAUSE);
+++num_pending;
+}
+}
+
+return num_pending;
+}
diff --git a/src/login/logind-session-device.h
b/src/login/logind-session-device.h
index 98e61e6..61732bc 100644
--- a/src/login/logind-session-device.h
+++ b/src/login/logind-session-device.h
@@ -55,3 +55,4 @@ void session_device_complete_pause(SessionDevice *sd);
void session_device_resume_all(Session *s);
void session_device_pause_all(Session *s);
+unsigned int session_device_try_pause_all(Session *s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 906115a..722e3d3 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -100,6 +100,8 @@ void session_free(Session *s) {
if (s-seat) {
[systemd-devel] [PATCH v2 08/10] logind: fix session_activate(vtnr = 0)
VT numbers start with 1. If a session has vtnr == 0, we must not assume it
is running on a VT.
Note that this could trigger the assert() below as CreateSession() sets
vtnr to 0, not 0.
---
src/login/logind-session.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 77eeb9c..1df2a06 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -378,7 +378,7 @@ int session_activate(Session *s) {
assert(s);
assert(s-user);
-if (s-vtnr 0)
+if (s-vtnr = 0)
return -ENOTSUP;
if (!s-seat)
--
1.8.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2 09/10] logind: extract has_vts() from can_multi_session()
We currently use seat_can_multi_session() to test for two things:
* whether the seat can handle session-switching
* whether the seat has VTs
As both are currently logically equivalent, we didn't care. However, we
want to allow session-switching on seats without VTs, so split this helper
into:
* seat_can_multi_session(): whether session-switching is supported
* seat_has_vts(): whether the seat has VTs
Note that only one seat on a system can have VTs. There is only one set of
them. We automatically assign them to seat0 as usual.
With this patch in place, we can easily add new session-switching/tracking
methods without breaking any VT code as it is now protected by has_vts(),
no longer by can_multi_session().
---
src/login/logind-dbus.c| 2 +-
src/login/logind-seat.c| 32 ++--
src/login/logind-seat.h| 1 +
src/login/logind-session.c | 6 +++---
4 files changed, 19 insertions(+), 22 deletions(-)
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 4a23c93..fd8ee1b 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -429,7 +429,7 @@ static int bus_manager_create_session(Manager *m,
DBusMessage *message) {
}
if (seat) {
-if (seat_can_multi_session(seat)) {
+if (seat_has_vts(seat)) {
if (vtnr 63)
return -EINVAL;
} else {
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index 4c2c424..f88738a 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -201,7 +201,7 @@ int seat_preallocate_vts(Seat *s) {
if (s-manager-n_autovts = 0)
return 0;
-if (!seat_can_multi_session(s))
+if (!seat_has_vts(s))
return 0;
for (i = 1; i = s-manager-n_autovts; i++) {
@@ -282,7 +282,7 @@ int seat_active_vt_changed(Seat *s, int vtnr) {
assert(s);
assert(vtnr = 1);
-if (!seat_can_multi_session(s))
+if (!seat_has_vts(s))
return -EINVAL;
log_debug(VT changed to %i, vtnr);
@@ -306,7 +306,7 @@ int seat_read_active_vt(Seat *s) {
assert(s);
-if (!seat_can_multi_session(s))
+if (!seat_has_vts(s))
return 0;
lseek(s-manager-console_active_fd, SEEK_SET, 0);
@@ -417,18 +417,20 @@ int seat_attach_session(Seat *s, Session *session) {
seat_send_changed(s, Sessions\0);
-/* Note that even if a seat is not multi-session capable it
- * still might have multiple sessions on it since old, dead
- * sessions might continue to be tracked until all their
- * processes are gone. The most recently added session
- * (i.e. the first in s-sessions) is the one that matters. */
-
-if (!seat_can_multi_session(s))
+/* On seats with VTs, the VT logic defines which session is active. On
+ * seats without VTs, we automatically activate the first session. */
+if (!seat_has_vts(s) !s-active)
seat_set_active(s, session);
return 0;
}
+bool seat_has_vts(Seat *s) {
+assert(s);
+
+return seat_is_seat0(s) s-manager-console_active_fd = 0;
+}
+
bool seat_is_seat0(Seat *s) {
assert(s);
@@ -438,19 +440,13 @@ bool seat_is_seat0(Seat *s) {
bool seat_can_multi_session(Seat *s) {
assert(s);
-if (!seat_is_seat0(s))
-return false;
-
-/* If we can't watch which VT is in the foreground, we don't
- * support VT switching */
-
-return s-manager-console_active_fd = 0;
+return seat_has_vts(s);
}
bool seat_can_tty(Seat *s) {
assert(s);
-return seat_is_seat0(s) s-manager-console_active_fd = 0;
+return seat_has_vts(s);
}
bool seat_has_master_device(Seat *s) {
diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index 47fe89a..d3438b8 100644
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -60,6 +60,7 @@ int seat_preallocate_vts(Seat *s);
int seat_attach_session(Seat *s, Session *session);
+bool seat_has_vts(Seat *s);
bool seat_is_seat0(Seat *s);
bool seat_can_multi_session(Seat *s);
bool seat_can_tty(Seat *s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 1df2a06..906115a 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -206,7 +206,7 @@ int session_save(Session *s) {
if (s-service)
fprintf(f, SERVICE=%s\n, s-service);
-if (s-seat seat_can_multi_session(s-seat))
+if (s-seat seat_has_vts(s-seat))
fprintf(f, VTNR=%i\n, s-vtnr);
if (s-leader 0)
@@ -316,7 +316,7 @@ int session_load(Session *s) {
seat_attach_session(o, s);
}
-if (vtnr s-seat seat_can_multi_session(s-seat)) {
+if (vtnr s-seat seat_has_vts(s-seat)) {
[systemd-devel] [PATCH v2 07/10] logind: fix seat_can_tty() to check for VTs
A seat provides text-logins if it has VTs. This is always limited to seat0
so the seat_is_seat0() check is correct. However, if VTs are disabled, no
seat provides text-logins so we also need to check for the console-fd.
This was previously:
return seat_is_vtconsole();
It looked right, but was functionally equivalent to seat_is_seat0(). The
rename of this helper made it more obvious that it is missing the VT test.
---
src/login/logind-seat.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index 88fd724..4c2c424 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -450,7 +450,7 @@ bool seat_can_multi_session(Seat *s) {
bool seat_can_tty(Seat *s) {
assert(s);
-return seat_is_seat0(s);
+return seat_is_seat0(s) s-manager-console_active_fd = 0;
}
bool seat_has_master_device(Seat *s) {
--
1.8.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH v2 01/10] logind: listen actively for session devices
Session compositors need access to fbdev, DRM and evdev devices if they
control a session. To make logind pass them to sessions, we need to
listen for them actively.
However, we avoid creating new seats for non master-of-seat devices. Only
once a seat is created, we start remembering all other session devices. If
the last master-device is removed (even if there are other non-master
devices still available), we destroy the seat. This is the current
behavior, but we need to explicitly implement it now as there may be
non-master devices in the seat-devices list.
Unlike master devices, we don't care whether our list of non-master
devices is complete. We don't export this list but use it only as cache if
sessions request these devices. Hence, if a session requests a device that
is not in the list, we will simply look it up. However, once a session
requested a device, we must be notified of remove udev events. So we
must link the devices somehow into the device-list.
Regarding the implementation, we now sort the device list by the master
flag. This guarantees that master devices are at the front and non-master
devices at the tail of the list. Thus, we can easily test whether a seat
has a master device attached.
---
src/login/logind-device.c | 35 ++---
src/login/logind-device.h | 3 +-
src/login/logind-seat.c | 11 +--
src/login/logind-seat.h | 1 +
src/login/logind.c| 79 +--
src/login/logind.h| 6 ++--
6 files changed, 116 insertions(+), 19 deletions(-)
diff --git a/src/login/logind-device.c b/src/login/logind-device.c
index 51b1535..a9a9633 100644
--- a/src/login/logind-device.c
+++ b/src/login/logind-device.c
@@ -25,7 +25,7 @@
#include logind-device.h
#include util.h
-Device* device_new(Manager *m, const char *sysfs) {
+Device* device_new(Manager *m, const char *sysfs, bool master) {
Device *d;
assert(m);
@@ -48,6 +48,7 @@ Device* device_new(Manager *m, const char *sysfs) {
}
d-manager = m;
+d-master = master;
dual_timestamp_get(d-timestamp);
return d;
@@ -75,11 +76,16 @@ void device_detach(Device *d) {
LIST_REMOVE(Device, devices, d-seat-devices, d);
d-seat = NULL;
-seat_add_to_gc_queue(s);
-seat_send_changed(s, CanGraphical\0);
+if (!seat_has_master_device(s)) {
+seat_add_to_gc_queue(s);
+seat_send_changed(s, CanGraphical\0);
+}
}
void device_attach(Device *d, Seat *s) {
+Device *i;
+bool had_master;
+
assert(d);
assert(s);
@@ -90,7 +96,26 @@ void device_attach(Device *d, Seat *s) {
device_detach(d);
d-seat = s;
-LIST_PREPEND(Device, devices, s-devices, d);
+had_master = seat_has_master_device(s);
+
+/* We keep the device list sorted by the master flag. That is, master
+ * devices are at the front, other devices at the tail. As there is no
+ * way to easily add devices at the list-tail, we need to iterate the
+ * list to find the first non-master device when adding non-master
+ * devices. We assume there is only a few (normally 1) master devices
+ * per seat, so we iterate only a few times. */
+
+if (d-master || !s-devices) {
+LIST_PREPEND(Device, devices, s-devices, d);
+} else {
+LIST_FOREACH(devices, i, s-devices) {
+if (!i-devices_next || !i-master) {
+LIST_INSERT_AFTER(Device, devices, s-devices,
i, d);
+break;
+}
+}
+}
-seat_send_changed(s, CanGraphical\0);
+if (!had_master d-master)
+seat_send_changed(s, CanGraphical\0);
}
diff --git a/src/login/logind-device.h b/src/login/logind-device.h
index 3b15356..315f0e6 100644
--- a/src/login/logind-device.h
+++ b/src/login/logind-device.h
@@ -33,13 +33,14 @@ struct Device {
char *sysfs;
Seat *seat;
+bool master;
dual_timestamp timestamp;
LIST_FIELDS(struct Device, devices);
};
-Device* device_new(Manager *m, const char *sysfs);
+Device* device_new(Manager *m, const char *sysfs, bool master);
void device_free(Device *d);
void device_attach(Device *d, Seat *s);
void device_detach(Device *d);
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index 470d08b..2c60b8a 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -448,10 +448,17 @@ bool seat_can_tty(Seat *s) {
return seat_is_vtconsole(s);
}
+bool seat_has_master_device(Seat *s) {
+assert(s);
+
+/* device list is ordered by master flag */
+return !!s-devices s-devices-master;
+}
+
bool seat_can_graphical(Seat *s) {
assert(s);
-return !!s-devices;
+return
[systemd-devel] [PATCH] logind: implement generic multi-session
This enables the multi-session capability for seats that don't have VTs.
For legacy seats with VTs, everything stays the same. However, all other
seats now also get the multi-session capability.
The only feature that was missing was session-switching. As logind can
force a session-switch and signal that via the Active property, we only
need a way to allow synchronized/delayed session switches. Compositors
need to cleanup some devices before acknowledging the session switch.
Therefore, we use the session-devices to give compositors a chance to
block a session-switch until they cleaned everything up.
If you activate a session on a seat without VTs, we send a PauseDevice
signal to the active session for every active device. Only once the
session acknowledged all these with a PauseDeviceComplete() call, we
perform the final session switch.
One important note is that delayed session-switching is meant for
backwards compatibility. New compositors or other sessions should really
try to deal correctly with forced session switches! They only need to
handle EACCES/EPERM from syscalls and treat them as PauseDevice signal.
Following logind patches will add a timeout to session-switches which
forces the switch if the active session does not react in a timely
fashion. Moreover, explicit ForceActivate() calls might also be supported.
Hence, sessions must not crash if their devices get paused.
---
src/login/logind-seat.c | 15 +++
src/login/logind-seat.h | 2 ++
src/login/logind-session-device.c | 28
src/login/logind-session-device.h | 1 +
src/login/logind-session.c| 31 ++-
5 files changed, 72 insertions(+), 5 deletions(-)
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index f88738a..4a4d40a 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -425,6 +425,21 @@ int seat_attach_session(Seat *s, Session *session) {
return 0;
}
+void seat_complete_switch(Seat *s) {
+Session *session;
+
+assert(s);
+
+/* if no session-switch is pending or if it got canceled, do nothing */
+if (!s-pending_switch)
+return;
+
+session = s-pending_switch;
+s-pending_switch = NULL;
+
+seat_set_active(s, session);
+}
+
bool seat_has_vts(Seat *s) {
assert(s);
diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index d3438b8..be6db6e 100644
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -38,6 +38,7 @@ struct Seat {
LIST_HEAD(Device, devices);
Session *active;
+Session *pending_switch;
LIST_HEAD(Session, sessions);
bool in_gc_queue:1;
@@ -59,6 +60,7 @@ int seat_read_active_vt(Seat *s);
int seat_preallocate_vts(Seat *s);
int seat_attach_session(Seat *s, Session *session);
+void seat_complete_switch(Seat *s);
bool seat_has_vts(Seat *s);
bool seat_is_seat0(Seat *s);
diff --git a/src/login/logind-session-device.c
b/src/login/logind-session-device.c
index 80fd364..e92bb54 100644
--- a/src/login/logind-session-device.c
+++ b/src/login/logind-session-device.c
@@ -414,10 +414,21 @@ void session_device_free(SessionDevice *sd) {
}
void session_device_complete_pause(SessionDevice *sd) {
+SessionDevice *iter;
+Iterator i;
+
if (!sd-active)
return;
session_device_stop(sd);
+
+/* if not all devices are paused, wait for further completion events */
+HASHMAP_FOREACH(iter, sd-session-devices, i)
+if (iter-active)
+return;
+
+/* complete any pending session switch */
+seat_complete_switch(sd-session-seat);
}
void session_device_resume_all(Session *s) {
@@ -449,3 +460,20 @@ void session_device_pause_all(Session *s) {
}
}
}
+
+unsigned int session_device_try_pause_all(Session *s) {
+SessionDevice *sd;
+Iterator i;
+unsigned int num_pending = 0;
+
+assert(s);
+
+HASHMAP_FOREACH(sd, s-devices, i) {
+if (sd-active) {
+session_device_notify(sd, SESSION_DEVICE_TRY_PAUSE);
+++num_pending;
+}
+}
+
+return num_pending;
+}
diff --git a/src/login/logind-session-device.h
b/src/login/logind-session-device.h
index 511fce0..eac7ca7 100644
--- a/src/login/logind-session-device.h
+++ b/src/login/logind-session-device.h
@@ -57,3 +57,4 @@ void session_device_complete_pause(SessionDevice *sd);
void session_device_resume_all(Session *s);
void session_device_pause_all(Session *s);
+unsigned int session_device_try_pause_all(Session *s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index fcc1901..eea0bfb 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -100,6 +100,8 @@ void session_free(Session *s) {
if (s-seat) {
[systemd-devel] [PATCH] logind: introduce session-devices
)));
+if (!sd)
+return bus_send_error_reply(connection, message, NULL,
-ENODEV);
+
+session_device_complete_pause(sd);
+
+reply = dbus_message_new_method_return(message);
+if (!reply)
+goto oom;
+
} else {
const BusBoundProperties bps[] = {
{ org.freedesktop.login1.Session,
bus_login_session_properties, s },
diff --git a/src/login/logind-session-device.c
b/src/login/logind-session-device.c
new file mode 100644
index 000..80fd364
--- /dev/null
+++ b/src/login/logind-session-device.c
@@ -0,0 +1,451 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright 2013 David Herrmann
+
+ 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 assert.h
+#include fcntl.h
+#include libudev.h
+#include linux/input.h
+#include linux/ioctl.h
+#include string.h
+#include sys/ioctl.h
+#include sys/stat.h
+#include sys/types.h
+#include unistd.h
+
+#include dbus-common.h
+#include logind-session-device.h
+#include util.h
+#include missing.h
+
+enum SessionDeviceNotifications {
+SESSION_DEVICE_RESUME,
+SESSION_DEVICE_TRY_PAUSE,
+SESSION_DEVICE_PAUSE,
+SESSION_DEVICE_RELEASE,
+};
+
+static void session_device_notify(SessionDevice *sd, enum
SessionDeviceNotifications type) {
+_cleanup_dbus_message_unref_ DBusMessage *m = NULL;
+_cleanup_free_ char *path = NULL;
+const char *t = NULL;
+
+assert(sd);
+
+if (!sd-session-controller)
+return;
+
+path = session_bus_path(sd-session);
+if (!path)
+return;
+
+m = dbus_message_new_signal(path,
+org.freedesktop.login1.Session,
+(type == SESSION_DEVICE_RESUME) ?
ResumeDevice : PauseDevice);
+if (!m)
+return;
+
+if (!dbus_message_set_destination(m, sd-session-controller))
+return;
+
+switch (type) {
+case SESSION_DEVICE_RESUME:
+if (!dbus_message_append_args(m,
+ DBUS_TYPE_UINT32, major(sd-dev),
+ DBUS_TYPE_UINT32, minor(sd-dev),
+ DBUS_TYPE_UNIX_FD, sd-fd,
+ DBUS_TYPE_INVALID))
+return;
+break;
+case SESSION_DEVICE_TRY_PAUSE:
+t = pause;
+break;
+case SESSION_DEVICE_PAUSE:
+t = force;
+break;
+case SESSION_DEVICE_RELEASE:
+t = gone;
+break;
+default:
+return;
+}
+
+if (t !dbus_message_append_args(m,
+ DBUS_TYPE_UINT32, major(sd-dev),
+ DBUS_TYPE_UINT32, minor(sd-dev),
+ DBUS_TYPE_STRING, t,
+ DBUS_TYPE_INVALID))
+return;
+
+dbus_connection_send(sd-session-manager-bus, m, NULL);
+}
+
+static int sd_eviocrevoke(int fd) {
+static bool warned;
+int r;
+
+assert(fd = 0);
+
+r = ioctl(fd, EVIOCREVOKE, 1);
+if (r 0) {
+r = -errno;
+if (r == -EINVAL !warned) {
+warned = true;
+log_warning(kernel does not support
evdev-revocation);
+}
+}
+
+return 0;
+}
+
+static int sd_drmsetmaster(int fd) {
+int r;
+
+assert(fd = 0);
+
+r = ioctl(fd, DRM_IOCTL_SET_MASTER, 0);
+if (r 0)
+return -errno;
+
+return 0;
+}
+
+static int sd_drmdropmaster(int fd) {
+int r;
+
+assert(fd = 0);
+
+r = ioctl(fd, DRM_IOCTL_DROP_MASTER, 0);
+if (r 0)
+return -errno;
+
+return 0;
+}
+
+static int session_device_open(SessionDevice *sd, bool active) {
+int fd;
+
+assert(sd-type != DEVICE_TYPE_UNKNOWN);
+
+/* open device and try to get an udev_device from it */
+fd = open(sd
[systemd-devel] [PATCH] logind: fix build for ARM with sizeof(dev_t) sizeof(void*)
Unfortunately on ARM-32 systems dev_t can be 64bit and thus we cannot
store it easily in void* keys for hashtables. Fix that by passing a
pointer to the dev_t variable instead.
---
src/login/logind-session-dbus.c | 12 +++-
src/login/logind-session-device.c | 8 +++-
src/login/logind-session.c| 17 -
3 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c
index f793f99..5f6bafb 100644
--- a/src/login/logind-session-dbus.c
+++ b/src/login/logind-session-dbus.c
@@ -452,9 +452,7 @@ static DBusHandlerResult session_message_dispatch(
return bus_send_error_reply(connection, message,
error, -EINVAL);
dev = makedev(major, minor);
-assert_cc(sizeof(unsigned long) = sizeof(dev_t));
-
-sd = hashmap_get(s-devices, ULONG_TO_PTR((unsigned long)dev));
+sd = hashmap_get(s-devices, dev);
if (sd) {
/* We don't allow retrieving a device multiple times.
* The related ReleaseDevice call is not ref-counted.
@@ -487,6 +485,7 @@ static DBusHandlerResult session_message_dispatch(
} else if (dbus_message_is_method_call(message,
org.freedesktop.login1.Session, ReleaseDevice)) {
SessionDevice *sd;
uint32_t major, minor;
+dev_t dev;
if (!session_is_controller(s,
bus_message_get_sender_with_fallback(message)))
return bus_send_error_reply(connection, message, NULL,
-EPERM);
@@ -499,7 +498,8 @@ static DBusHandlerResult session_message_dispatch(
DBUS_TYPE_INVALID))
return bus_send_error_reply(connection, message,
error, -EINVAL);
-sd = hashmap_get(s-devices, ULONG_TO_PTR((unsigned
long)makedev(major, minor)));
+dev = makedev(major, minor);
+sd = hashmap_get(s-devices, dev);
if (!sd)
return bus_send_error_reply(connection, message, NULL,
-ENODEV);
@@ -512,6 +512,7 @@ static DBusHandlerResult session_message_dispatch(
} else if (dbus_message_is_method_call(message,
org.freedesktop.login1.Session, PauseDeviceComplete)) {
SessionDevice *sd;
uint32_t major, minor;
+dev_t dev;
if (!session_is_controller(s,
bus_message_get_sender_with_fallback(message)))
return bus_send_error_reply(connection, message, NULL,
-EPERM);
@@ -524,7 +525,8 @@ static DBusHandlerResult session_message_dispatch(
DBUS_TYPE_INVALID))
return bus_send_error_reply(connection, message,
error, -EINVAL);
-sd = hashmap_get(s-devices, ULONG_TO_PTR((unsigned
long)makedev(major, minor)));
+dev = makedev(major, minor);
+sd = hashmap_get(s-devices, dev);
if (!sd)
return bus_send_error_reply(connection, message, NULL,
-ENODEV);
diff --git a/src/login/logind-session-device.c
b/src/login/logind-session-device.c
index e92bb54..b45f9eb 100644
--- a/src/login/logind-session-device.c
+++ b/src/login/logind-session-device.c
@@ -369,9 +369,7 @@ int session_device_new(Session *s, dev_t dev, SessionDevice
**out) {
if (r 0)
goto error;
-assert_cc(sizeof(unsigned long) = sizeof(dev_t));
-
-r = hashmap_put(s-devices, ULONG_TO_PTR((unsigned long)sd-dev), sd);
+r = hashmap_put(s-devices, sd-dev, sd);
if (r 0) {
r = -ENOMEM;
goto error;
@@ -392,7 +390,7 @@ int session_device_new(Session *s, dev_t dev, SessionDevice
**out) {
return 0;
error:
-hashmap_remove(s-devices, ULONG_TO_PTR((unsigned long)sd-dev));
+hashmap_remove(s-devices, sd-dev);
free(sd-node);
free(sd);
return r;
@@ -407,7 +405,7 @@ void session_device_free(SessionDevice *sd) {
LIST_REMOVE(SessionDevice, sd_by_device, sd-device-session_devices,
sd);
-hashmap_remove(sd-session-devices, ULONG_TO_PTR((unsigned
long)sd-dev));
+hashmap_remove(sd-session-devices, sd-dev);
free(sd-node);
free(sd);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index eea0bfb..ce982ef 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -36,6 +36,21 @@
#include dbus-common.h
#include logind-session.h
+static unsigned devt_hash_func(const void *p) {
+uint64_t u = *(const dev_t*)p;
+
+return uint64_hash_func(u);
+}
+
+static int devt_compare_func(const void *_a, const void *_b) {
+dev_t a, b;
+
+a = *(const dev_t*) _a;
+b = *(const dev_t*) _b;
+
+return a b ? -1 : (a b ? 1 :
Re: [systemd-devel] Xorg+logind+DM issue: inactive graphical session for seat0
Hi On Tue, Oct 29, 2013 at 4:44 PM, Laércio de Sousa lbsous...@gmail.com wrote: I would append another approach to the list: * For non-seat0 seats, X server should open no VT at all. Currently, even with -sharevts option, it seems Xorg does open a VT, although it can't control this. Yes, please! We shouldn't touch VTs at all if we're not on seat0. I never understood why we have -sharedvt.. It doesn't make any sense. Furthermore, if we remove -sharedvt we could also run the xserver with CONFIG_VT=n. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] log: add log_errno() helper
Syscalls may fail for a lot of reasons, but most times these errors are
unexpected (we cannot recover). Especially when dealing with device nodes
that can be revoked asynchronously, a series of syscalls may start failing
at any point. Normally, we can silently ignore errors and just bail out,
but for debugging purposes log messages are quite helpful.
The log_errno() helper can be used in such situations where we don't
expect a syscall error, but also don't want to add a custom log-message to
reduce memory-consumption. The helper just prints the file+line+func
information and the errno-content.
Usage:
r = ioctl(fd, ..);
if (r 0)
return log_errno();
It is basically the same as log_oom() but generic for all kernel
errno-messages. If we added a custom log-message for each syscall failure,
our .text section would increase heavily.
---
src/shared/log.c | 5 +
src/shared/log.h | 6 ++
2 files changed, 11 insertions(+)
diff --git a/src/shared/log.c b/src/shared/log.c
index 8f4995a..5edb05f 100644
--- a/src/shared/log.c
+++ b/src/shared/log.c
@@ -713,6 +713,11 @@ int log_oom_internal(const char *file, int line, const
char *func) {
return -ENOMEM;
}
+int log_errno_internal(const char *file, int line, const char *func) {
+log_meta(LOG_ERR, file, line, func, Syscall failed unexpectedly: %m);
+return -errno;
+}
+
int log_struct_internal(
int level,
const char *file,
diff --git a/src/shared/log.h b/src/shared/log.h
index 0dc5c26..6f05f7c 100644
--- a/src/shared/log.h
+++ b/src/shared/log.h
@@ -116,6 +116,11 @@ int log_oom_internal(
int line,
const char *func);
+int log_errno_internal(
+const char *file,
+int line,
+const char *func);
+
/* This modifies the buffer passed! */
int log_dump_internal(
int level,
@@ -151,6 +156,7 @@ do { \
#define log_struct(level, ...) log_struct_internal(level, __FILE__, __LINE__,
__func__, __VA_ARGS__)
#define log_oom() log_oom_internal(__FILE__, __LINE__, __func__)
+#define log_errno() log_errno_internal(__FILE__, __LINE__, __func__)
/* This modifies the buffer passed! */
#define log_dump(level, buffer) log_dump_internal(level, __FILE__, __LINE__,
__func__, buffer)
--
1.8.4.2
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] log: add log_errno() helper
Hi On Fri, Nov 15, 2013 at 2:33 PM, Zbigniew Jędrzejewski-Szmek zbys...@in.waw.pl wrote: On Fri, Nov 15, 2013 at 11:22:59AM +0100, David Herrmann wrote: Syscalls may fail for a lot of reasons, but most times these errors are unexpected (we cannot recover). Especially when dealing with device nodes that can be revoked asynchronously, a series of syscalls may start failing at any point. Normally, we can silently ignore errors and just bail out, but for debugging purposes log messages are quite helpful. I'm not sold of the idea of completely generic message. If I'm sitting in front of a failed boot, seeing only Syscall failed unexpectedly: no such file or directory, I'm going to be pretty frustrated. Maybe there should be a custom message argument like ioctl AUTOFS_DEV_IOCTL_VERSION. This isn't going to increase the footprint too much, but will help undertand bug reprots. Or if you thyink it's too annoying to write, than maybe just print the file name and line automatically. file-name and line is actually what I wanted (does log_meta() suppress these?). Custom error-messages don't really make much sense. For example: Assume you have a function to open an input device. The function itself has to run a bunch of syscalls: - open() (open device node) - ioctl() (get device bits, like 10 EVIOC* ioctls) - write() (to initialize LED values) These are all mostly straightforward and shouldn't fail if the device is valid. However, they *can* fail for various reasons (daemon not running with enough privileges, device being unplugged during operation, ...) but it's almost impossible to detect these properly. So we want debug/error messages. If we now add a custom log_error() message for each of these calls, they will be something like EVIOCGABS failed: %m. To a user this is as helpful as syscall failed in $file:$line:$func. So I thought, we can avoid cluttering .text with useless error-messages and just print $file:$line:$func and %m. Does that make sense? Note that this obviously is only for developer error-messages. For functions like open() it makes sense to handle it separately. A message like cannot open /dev/$sth: Access denied even helps users, not only developers. The log_errno() is really intended for stuff like custom ioctls/etc. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] event: clear pending-state when re-arming timers
If a timer fires and is marked pending, but an application re-arms it
before it is dispatched, we now clear the pending state.
This fixes a bug where an application arms a timer, which fires and is
marked pending. But before it is dispatched, the application loses
interest in it and disables it. Now if the timer is re-armed and
re-enabled later, it will be immediately dispatched as it is still marked
pending.
This behavior is unexpected, so avoid it by clearing pending state when
re-arming timers. Note that applications have no way to clear pending
state themselves, so there's no current workaround.
---
src/libsystemd-bus/sd-event.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/libsystemd-bus/sd-event.c b/src/libsystemd-bus/sd-event.c
index 0996316..d01e82d 100644
--- a/src/libsystemd-bus/sd-event.c
+++ b/src/libsystemd-bus/sd-event.c
@@ -1241,6 +1241,7 @@ _public_ int sd_event_source_set_time(sd_event_source *s,
uint64_t usec) {
return 0;
s-time.next = usec;
+source_set_pending(s, false);
if (s-type == SOURCE_REALTIME) {
prioq_reshuffle(s-event-realtime_earliest, s,
s-time.earliest_index);
--
1.8.4.2
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd 208:trouble with inactive user sessions at non-seat0 seats
Hi
On Tue, Nov 19, 2013 at 7:13 PM, Laércio de Sousa lbsous...@gmail.com wrote:
Hi there!
I'm testing both Fedora 20 Beta and openSUSE 13.1 in my multiseat system
(with GNOME 3.10, GDM 3.10.0.1 and systemd 208). I'm currently observing a
strange behaviour which didn't occur in previous distro release (with GNOME
3.8.4 and systemd 204).
When I boot my system, gdm greeter session at both seat0 and non-seat0 seats
are active, as expected. However, when a user logs in at a non-seat0 seat,
the user session gets inactive. Nevertheless, I can activate it manually
with command loginctl activate user-session-id, but if that user logs
out, the new greeter session now becomes inactive.
I suspect this strange behaviour is related to that generic multi-session
support introduced in systemd 208. I can see that, when a user logs in, the
greeter and user sessions coexist for a while, one at closing state and
the other at online state, depending on whether the user logs in or out.
What could it be? Lack of generic multi-session support in GDM?
I've appended a patch which avoids the situation without an active
session. Can you give it a try? I won't be able to test my multi-seat
setup before Sunday, sorry.
Thanks
David
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index b30c4ce..d52e05b 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -228,7 +228,7 @@ int seat_apply_acls(Seat *s, Session *old_active) {
}
int seat_set_active(Seat *s, Session *session) {
-Session *old_active;
+Session *old_active, *i;
assert(s);
assert(!session || session-seat == s);
@@ -236,6 +236,13 @@ int seat_set_active(Seat *s, Session *session) {
if (session == s-active)
return 0;
+/* if no VTs, avoid having no active session */
+if (!seat_has_vts(s) !session) {
+LIST_FOREACH(sessions_by_seat, i, s-sessions)
+if (i != s-active)
+session = s-active;
+}
+
old_active = s-active;
s-active = session;
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] include: fix problem with __LINE__ macro expansion
Hi On Fri, Nov 22, 2013 at 2:09 PM, Lennart Poettering lenn...@poettering.net wrote: On Thu, 21.11.13 17:26, Lukasz Skalski (l.skal...@partner.samsung.com) wrote: Hi all, Macro __LINE__ in #define assert_cc() (src/shared/macro.h) is not properly expanded. Please find patch in attachment for review. Thanks! Seems David Herrmann has now fixed this thing slightly differently. I just noticed the compile-failure and fixed it up myself. But this patch definitely looks nicer. I now changed the helper-name to XCONCATENATE similar to XSTRINGIFY and added a UNIQUE macro to generate a unique name. Might be useful in other macros, too. Thanks David ___ 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
(basic terminal emulator) used as fallback session/console. - systemd-splashd: A boot-splash for initrds. This will not include eye-candy but is only required to accept password-input for encrypted root-partitions. Use Plymouth if you want eye-candy! - systemd-welcomed: A login-screen to spawn sessions. This basically replaces /sbin/agetty and /bin/login on a VT. After login it spawns systemd-consoled. - systemd-er: The emergency-room helper. Basically an even-more stripped down systemd-consoled designed for developers. It avoids session-management, seat-assignments, privilege-separation and is just an hopefully-always-working-emergency-console. It's not enabled by default. Think of it as a userspace-variant of magic sysrq. This is stupid, why do you.. ..add so many dependencies and code to systemd? You should consider that currently all that code is part of the *kernel*. All this does is moving it out of the kernel. The first place that comes to mind is systemd (and I couldn't imagine a better place, can you?). Knowing that systemd already depends on the kernel, technically this doesn't increase the codebase of systemd at all. ..kill my beloved kernel-console / linux-console? I don't. Just set CONFIG_VT=y. ..duplicate kmscon? Well, I wrote that and I consider it a successfull research project. Now it's time to write something useful based on the lessons learned with kmscon. No first attempt ever succeeds, right? ..miss the fact that a kernel-console is more reliable than user-space? It's not. Why do you think that? You need a running shell to do anything useful with a console. Where do you think this shell runs? (Hint: it's not kernel-space.. and not outer space.. I wonder what space is left? Oh, none, because your in-kernel fonts are non-swappable!) ..kill my panic/oops screen? I don't. See above. I wrote fblog/drmlog for that. ..change a running system? To make it better! ..add useful features like full-BMP-range Unicode-fonts, properly internationalized keymaps, readable fonts on high-DPI displays, access-separation, accessibility helpers and more to the system console? Wait.. that's obvious, isn't it? Besides, coding is fun! David David Herrmann (12): event: allow EPOLLET as event flag ring: add basic ring-buffer helper bus: add two new bus_*_map_*_properties() helpers build: add target to link binary sources gfx: add sd-gfx library with unifont section gfx: add keyboard layer gfx: add graphics layer gfx: add monitor gfx: add kbd test gfx: add graphics test gfx: add unbuilt GL test console: add systemd-consoled .gitignore | 4 + Makefile.am | 129 + configure.ac |38 + make-unifont.py | 138 + 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 + src/libsystemd-bus/bus-util.c|87 +- src/libsystemd-bus/bus-util.h| 6 + src/libsystemd-bus/sd-event.c| 4 +- src/libsystemd-gfx/.gitignore| 1 + src/libsystemd-gfx/Makefile | 1 + src/libsystemd-gfx/gfx-drm.c | 2551 ++ src/libsystemd-gfx/gfx-kbd.c | 629 + src/libsystemd-gfx/gfx-monitor.c | 1767 ++ src/libsystemd-gfx/gfx-unifont.c | 273 + src/libsystemd-gfx/test-gfx.c| 302 + src/libsystemd-gfx/test-gl.c | 342 + src/libsystemd-gfx/test-kbd.c| 314 + src/libsystemd-gfx/unifont.bin | Bin 0 - 2162688 bytes src/libsystemd-gfx/unifont.hex | 63488 + src/shared/ring.c| 213 + src/shared/ring.h|54 + src/systemd/sd-gfx.h | 405 + 27 files changed, 72058 insertions(+), 27 deletions(-) create mode 100755 make-unifont.py 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 create mode 100644 src/libsystemd-gfx/.gitignore create mode 12 src/libsystemd-gfx/Makefile create mode 100644 src/libsystemd-gfx/gfx-drm.c create mode 100644 src/libsystemd-gfx/gfx-kbd.c create mode 100644 src/libsystemd-gfx/gfx-monitor.c create mode 100644 src/libsystemd-gfx/gfx-unifont.c create mode 100644 src/libsystemd-gfx/test-gfx.c create mode 100644 src/libsystemd-gfx/test-gl.c create mode 100644 src/libsystemd-gfx/test-kbd.c create mode 100644 src/libsystemd-gfx
[systemd-devel] [RFC 03/12] bus: add two new bus_*_map_*_properties() helpers
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
;
generator probably broken ; \
+ exit 1 ; \
+ fi
+ @echo unifont.bin has been regenerated
+endif
+
+#
--
if ENABLE_NETWORKD
rootlibexec_PROGRAMS += \
systemd-networkd
diff --git a/configure.ac b/configure.ac
index 3fd05da..354673a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -291,6 +291,15 @@ fi
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
+fi
+AM_CONDITIONAL(HAVE_GFX, [test $have_gfx = yes])
+
+#
--
have_blkid=no
AC_ARG_ENABLE(blkid, AS_HELP_STRING([--disable-blkid], [disable blkid
support]))
if test x$enable_blkid != xno; then
@@ -1079,6 +1088,7 @@ AC_MSG_RESULT([
polkit: ${have_polkit}
efi: ${have_efi}
kmod:${have_kmod}
+sd-gfx: ${have_gfx}
blkid: ${have_blkid}
nss-myhostname: ${have_myhostname}
gudev: ${enable_gudev}
diff --git a/make-unifont.py b/make-unifont.py
new file mode 100755
index 000..375c8a4
--- /dev/null
+++ b/make-unifont.py
@@ -0,0 +1,138 @@
+# -*- Mode: python; coding: utf-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/.
+
+#
+# Parse a unifont.hex file and produce a compressed binary-format we use in
+# our gfx applications. Can also do the reverse to verify correctness.
+#
+
+from __future__ import print_function
+import re
+import sys
+import fileinput
+import struct
+
+#
+# Write bits array as binary output.
+#
+
+def write_bin_entry(entry):
+l = len(entry)
+if l != 32 and l != 64:
+entry = 0 * 64
+l = 0
+elif l 64:
+entry += 0 * (64 - l)
+
+sys.stdout.buffer.write(struct.pack('B', int(l / 32)))
+
+for i in range(0, 64, 2):
+c = int(entry[i:i+2], 16)
+sys.stdout.buffer.write(struct.pack('B', c))
+
+def write_bin(bits):
+for idx in range(len(bits)):
+write_bin_entry(bits[idx])
+
+#
+# Parse hex file into bits array
+#
+
+def parse_hex_line(bits, line):
+m = re.match(r^([0-9A-Fa-f]+):([0-9A-Fa-f]+)$, line)
+if m == None:
+return
+
+idx = int(m.group(1), 16)
+val = m.group(2)
+
+# insert skipped lines
+for i in range(len(bits), idx):
+bits.append()
+
+bits.insert(idx, val)
+
+def parse_hex():
+bits = []
+
+for line in sys.stdin:
+if not line:
+continue
+if line.startswith(#):
+continue
+
+parse_hex_line(bits, line)
+
+return bits
+
+#
+# Write bits array as text-file line-by-line to stdout.
+#
+
+def write_hex_line(idx, entry):
+if entry:
+print(%04X:%s % (idx, entry))
+
+def write_hex(bits):
+for idx, entry in enumerate(bits):
+if not entry:
+continue
+
+write_hex_line(idx, entry)
+
+#
+# Parse a binary file into bits so we can verify the correctness of a given
+# binary file.
+#
+
+def parse_bin_entry(bits, chunk):
+entry =
+l = struct.unpack('B', chunk[0:1])[0]
+for c in chunk[1:l*16+1]:
+entry += format(c, 02X)
+
+bits.append(entry)
+
+def parse_bin():
+bits = []
+
+while True:
+chunk = sys.stdin.buffer.read(33)
+if chunk:
+parse_bin_entry(bits, chunk)
+else:
+break
+
+return bits
+
+#
+# In normal mode we simply read line by line from standard-input (or first
+# argument) and write the binary-file to standard-output.
+#
+# In verify-mode, we read the binary-file from standard-input and write the
+# text-file line-by-line to standard-output.
+#
+
+if __name__ == __main__:
+if len(sys.argv) 1 and sys.argv[1] == verify:
+bits = parse_bin()
+write_hex(bits)
+else:
+bits = parse_hex()
+write_bin(bits)
diff --git a/src
[systemd-devel] [RFC 01/12] event: allow EPOLLET as event flag
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
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
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
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
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
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
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
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
Re: [systemd-devel] [RFC 04/12] build: add target to link binary sources
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] [RFC 00/12] Bugfixes for CONFIG_VT=n
Hi On Wed, Nov 27, 2013 at 11:44 PM, Lennart Poettering lenn...@poettering.net wrote: 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... I have no short-term plan to export sd-gfx. It's just like the 3rd time I happen to write a DRM abstraction and I really don't wanna do that again. Hence, I thought putting it into sd-gfx would reduce the burden to export it eventually. Furthermore, as there'll be like 5 daemons using it, it would be nice to have a clean API even if it's only systemd-internal (it forces one to write better code, imho). Last but not least, it makes debugging easier if there's a clean cut between sd-gfx and each application. But: the current API will never get exported as it is. Obviously, the monitor (and probably font) is better kept internal. But I see no reason to add sd-gfx-internal.h / sd-gfx-internal.la now. In case we ever export it, we would have to decide which parts to split off (which is a rather mechanical tasks, so no reason to account for it right now). So lets just say I hate camel-case object-names so I went for libsystemd-gfx to have an excuse to use lower-case-separated-by-underscore names, right? Lets treat sd-gfx as internal library for now. A few comments beforehand: - regarding logging in libraries: I *want* verbose errors/warnings in sd-gfx. Especially if modesetting, keymap-compilation or bus-requests fail, I want some rather verbose messages in the log. Considering that if a single page-flip fails, I will not abort but continue the modeset (you don't want your console to abort due to minor timing issues in the DRM driver, right?), so the application might not even get an -EXYZ error value. I tried adding a -log_fn() like libudev, but then again, sd-gfx is internal so I skipped that and just called log_meta() directly. Is that fine for now? If not, I can change all these to log_debug() and make the applications more verbose. - new0(), zero(), ..., yepp, kay already told me but I forgot.. I will fix them up. - moving out parameters to the end... will do so. Thanks for the review. Few more comments on the individual patches. David ___ 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
Hi
On Wed, Nov 27, 2013 at 10:53 PM, Lennart Poettering
lenn...@poettering.net wrote:
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...
Yepp, uint8_t it is.
+
+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...
Well, start+end is how ring-buffers were implemented historically. I
think the reason is that wrapping over is easier to calculate for
indices than for lengths (you cannot do: r-len = (r-len + 1)
RING_MASK). Even all examples on wikipedia use two indices
(http://en.wikipedia.org/wiki/Circular_buffer). And all kernel
ring-buffers use start/end..
Also note that the one special byte is not unused. It cannot be
filled, in case the buffer is full, that's right. But it's used during
normal buffer-operation if the occupied space moves along the
buffer.
If you can find me circular/ring-buffers that use start+len, I can fix
that up. Otherwise, I'd rather keep this close to existing
implementations. Besides, a lot of wrapping calculations get easier
if start/end are restricted by RING_MASK, which wouldn't be the case
for 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) {
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...
I usually use u8 for utf8.. no idea where that came from. But yes,
void* makes sense for input/output and uint8_t* for internal
operations.
Thanks
David
___
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
Hi
On Thu, Nov 28, 2013 at 1:27 AM, Lucas De Marchi
lucas.de.mar...@gmail.com wrote:
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.
Yepp, that one looks good. Didn't know there was a builtin to count
leading zeros.
Thanks
David
___
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
Hi On Wed, Nov 27, 2013 at 10:58 PM, Lennart Poettering lenn...@poettering.net wrote: 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); That doesn't work. The caller wants interface before calling this helper. You usually pass a different map-array depending on the interface, right? I can just require the caller to do the skip/read themselves? Thanks David ___ 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
Hi
On Wed, Nov 27, 2013 at 11:11 PM, Lennart Poettering
lenn...@poettering.net wrote:
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...
Yepp, sounds good. Would be like 200 lines to write that, I guess.
+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 ;-)
Do I get an award or something? :)
+
+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);
I actually dislike new0() to require the type. I'd prefer:
font = new0(font);
But this is horrible to read, so I went with calloc(1, sizeof(*xyz));
But the systemd code-base seems to use new0() consistently so I can
adjust all my calloc()s if you want? I'm fine with keeping consistency
here.
+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).
I wanna avoid these in fast-paths (the blending/blitting functions)
but for all the other ones, I can add assert()s.
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.
Oh, I know. This is presumably the only destructor where I somehow
forgot that. Fixed.
+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
Re: [systemd-devel] [RFC 06/12] gfx: add keyboard layer
Hi
On Wed, Nov 27, 2013 at 11:17 PM, Lennart Poettering
lenn...@poettering.net wrote:
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?
Yes.
+
+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 ]
You can use _Bool/bool for bitfields? Didn't know that. I always use
bool for boolean values, I only thought they don't work for
bitfields. Will fix that up.
Thanks
David
+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
Hi
On Wed, Nov 27, 2013 at 11:24 PM, Lennart Poettering
lenn...@poettering.net wrote:
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.
Ouh, indeed. I use calloc() anyway, so I could just make this one
single goto. Or make the destructor robust and call it instead, yepp.
Fixed.
+/* 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.
Ouh, libudev uses udev_log_fn so I followed that style. I thought
that's what we use for callback-prototypes. Is that just a left-over
from pre-systemd times? I can change it all to _t, if it is.
Thanks
David
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
Hi
On Wed, Nov 27, 2013 at 11:34 PM, Lennart Poettering
lenn...@poettering.net wrote:
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?
Yepp. As said in my comment on 0/12, I squashed it all in a single
header for now. If we export it, this will get moved into gfx-util.h
or similar.
+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?
Sweet! Fixed.
+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
Re: [systemd-devel] [RFC 12/12] console: add systemd-consoled
Hi
On Wed, Nov 27, 2013 at 11:39 PM, Lennart Poettering
lenn...@poettering.net wrote:
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...
Closed-source developers get paid by number of lines, why don't get
open-source developers honored by number of lines? I would excel in
that category!
Fixed.
Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] systemd 208:trouble with inactive user sessions at non-seat0 seats
Hi On Tue, Nov 26, 2013 at 2:33 PM, Laércio de Sousa lbsous...@gmail.com wrote: David, Looking at GDM debug and gdm-simple-slave.c source file, as well as loginctl show-seat output. I guess that GDM only requests user session activation for seats with CanMultiSession=yes, but currently systemd-logind still sets CanMultiSession=no for my non-seat0 seats. Does seat_can_multi_session() heuristics need some improvement since generic multi-session was introduced? I've applied your last patch. I confirm it solves my problem for now. Thanks for the confirmation. I pushed the revert to systemd-git so 209 will contain the fix. I will take care that it gets marked as fix for 208, too. Regarding gdm: I guess we cannot rely on CanMultiSession here. It's to risky that we break some other application. But maybe we can allow desktop-managers to modify the CanMultiSession property. So if they explicitly set it to 1 (which we don't do for seats without VTs), we activate the new logic. Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
