Re: [systemd-devel] [PATCH 2/2] tmpfiles: support globbing for w option
On Mon, Sep 03, 2012 at 05:13:18PM -0400, Dave Reisner wrote: > Break out the write logic into a separate function and simply use it as > a callback to glob_item. > > This allows users to consolidate writes to sysfs with multiple similar > pathnames, e.g. > > w /sys/class/block/sd[a-z]/queue/read_ahead_kb - - - - 1024 > --- > As is, and even without this patch, the example I point out won't work. > I suspect this is a kernel bug, which I've reported: > > https://bugzilla.kernel.org/show_bug.cgi?id=46981 > > Of course, I'm also happy to provide a workaround here, but I assume that > systemd upstream would rather just have the kernel bug fixed. I haven't > come across any other sysfs nodes (yet) that exhibit this broken behavior. Just to add to this, is there a particular reason writev was chosen? This is _always_ going to result in multiple writes, which sysfs won't necessarily play nicely with [0]. I'm not sure that's necessarily something that needs to be fixed in the kernel. On the other hand, using a higher level library call like fprintf would generally be a single write and avoids this problem entirely, kernel bug or not. d [0] A line such as "w /sys/class/block/sda/queue/scheduler noop" will result in an error in dmesg when it rejects the string which is a newline: Sep 03 21:19:14 rampage kernel: elevator: type not found Sep 03 21:19:14 rampage kernel: elevator: switch to failed > man/tmpfiles.d.xml | 2 +- > src/tmpfiles/tmpfiles.c | 135 > +--- > 2 files changed, 72 insertions(+), 65 deletions(-) > > diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml > index b7397b6..c6325a4 100644 > --- a/man/tmpfiles.d.xml > +++ b/man/tmpfiles.d.xml > @@ -114,7 +114,7 @@ L/tmp/foobar ---- > /dev/null > > > w > -Write the argument > parameter to a file, if it exists. > +Write the argument > parameter to a file, if it exists. Lines of this type accept shell-style > globs in place of normal path names. > > > > diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c > index e70332c..b44beee 100644 > --- a/src/tmpfiles/tmpfiles.c > +++ b/src/tmpfiles/tmpfiles.c > @@ -474,6 +474,75 @@ static int item_set_perms(Item *i, const char *path) { > return label_fix(path, false, false); > } > > +static int write_one_file(Item *i, const char *path) { > +int r, e, fd, flags; > +struct stat st; > +mode_t u; > + > +flags = i->type == CREATE_FILE ? O_CREAT|O_APPEND : > +i->type == TRUNCATE_FILE ? O_CREAT|O_TRUNC : 0; > + > +u = umask(0); > +label_context_set(path, S_IFREG); > +fd = open(path, > flags|O_NDELAY|O_CLOEXEC|O_WRONLY|O_NOCTTY|O_NOFOLLOW, i->mode); > +e = errno; > +label_context_clear(); > +umask(u); > +errno = e; > + > +if (fd < 0) { > +if (i->type == WRITE_FILE && errno == ENOENT) > +return 0; > + > +log_error("Failed to create file %s: %m", path); > +return -errno; > +} > + > +if (i->argument) { > +ssize_t n; > +size_t l; > +struct iovec iovec[2]; > +static const char new_line = '\n'; > + > +l = strlen(i->argument); > + > +zero(iovec); > +iovec[0].iov_base = i->argument; > +iovec[0].iov_len = l; > + > +iovec[1].iov_base = (void*) &new_line; > +iovec[1].iov_len = 1; > + > +n = writev(fd, iovec, 2); > + > +/* It's OK if we don't write the trailing > + * newline, hence we check for l, instead of > + * l+1 here. Files in /sys often refuse > + * writing of the trailing newline. */ > +if (n < 0 || (size_t) n < l) { > +log_error("Failed to write file %s: %s", path, n < 0 > ? strerror(-n) : "Short write"); > +close_nointr_nofail(fd); > +return n < 0 ? n : -EIO; > +} > +} > + > +if (stat(path, &st) < 0) { > +log_error("stat(%s) failed: %m", path); > +return -errno; > +} > + > +if (!S_ISREG(st.st_mode)) { > +log_error("%s is not a file.", path); > +return -EEXIST; > +} > + > +r = item_set_perms(i, path); > +if (r < 0) > +return r; > + > +return 0; > +} > + > static int recursive_relabel_children(Item *i, const char *path) { > DIR *d; > int ret = 0; > @@ -607,74 +676,12 @@ static int create_item(Item *i)
Re: [systemd-devel] Handling lid close in logind?
On Mon, Sep 3, 2012 at 5:17 AM, Richard Hughes wrote: > I don't think the desktop guys want any kind of authentication when > the lid is closed. How feasible would it be to do the suspend when the > inhibit is removed? I guess then it puts logind into a "active but > will suspend asap" state which might be difficult to handle. As I said on irc, the user experience I envision in this case is: - user closes lid - polkit dialog is shown (not useful because the lid is closed) - we beep a few times to cause the user to open the lid and see the dialog - if the lid is not opened, we forcefully suspend after a few seconds A similar situation where we can't wait for a policykit dialog to be answered is emergency shutdown when the battery runs empty. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] shared: logs-show: fflush after each output type
journalctl -f redirected to a pipe or file wasn't working for some output formats but was working for json. It turns out only json was doing an fflush. Make all output formats flush. --- src/shared/logs-show.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c index 60eb896..cafddf7 100644 --- a/src/shared/logs-show.c +++ b/src/shared/logs-show.c @@ -521,7 +521,6 @@ static int output_json(sd_journal *j, unsigned line, } fputs("\n}", stdout); -fflush(stdout); return 0; } @@ -560,13 +559,16 @@ static int (*output_funcs[_OUTPUT_MODE_MAX])(sd_journal*j, unsigned line, int output_journal(sd_journal *j, OutputMode mode, unsigned line, unsigned n_columns, OutputFlags flags) { +int ret; assert(mode >= 0); assert(mode < _OUTPUT_MODE_MAX); if (n_columns <= 0) n_columns = columns(); -return output_funcs[mode](j, line, n_columns, flags); +ret = output_funcs[mode](j, line, n_columns, flags); +fflush(stdout); +return ret; } int show_journal_by_unit( -- 1.7.11.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/2] logind: Ensure the user, seat and session files are updated when the session is closing.
--- src/login/logind-session.c | 5 + 1 file changed, 5 insertions(+) diff --git a/src/login/logind-session.c b/src/login/logind-session.c index 16d4955..6733020 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -702,9 +702,11 @@ int session_stop(Session *s) { seat_set_active(s->seat, NULL); seat_send_changed(s->seat, "Sessions\0"); +seat_save(s->seat); } user_send_changed(s->user, "Sessions\0"); +user_save(s->user); s->started = false; @@ -858,6 +860,9 @@ void session_remove_fifo(Session *s) { assert_se(epoll_ctl(s->manager->epoll_fd, EPOLL_CTL_DEL, s->fifo_fd, NULL) == 0); close_nointr_nofail(s->fifo_fd); s->fifo_fd = -1; + +session_save(s); +user_save(s->user); } if (s->fifo_path) { -- 1.7.12 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/2] logind: Avoid unnecesary rewrite of user file when switching sessions of the same user.
--- src/login/logind-seat.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index 0457121..9c7cc1f 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -261,7 +261,8 @@ int seat_set_active(Seat *s, Session *session) { if (old_active) { session_save(old_active); -user_save(old_active->user); +if (!session || session->user != old_active->user) +user_save(old_active->user); } return 0; -- 1.7.12 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] (no subject)
'Twas brillig, and Colin Guthrie at 03/09/12 23:46 did gyre and gimble: > Hi, > > OK, so these are the changes I'd propose to fix the issues mentioned > in this thread. I've not tested them so this is more for general > feedback as to whether this approach is generally a good one or not. > > The initial patch is just extra debugging I added to try and work out > why my gdm session is not marked with type=greeter so seems like a > generally useful change when in debug mode. It seems that while I still think the principle behind these changes makes sense, that the various env var files are simply not updated correctly when various changes occur: e.g. compare the output and note the discrepancy in the [Ss]tate= values: [colin@jimmy systemd (master)]$ loginctl show-session 1 Id=1 Timestamp=Tue, 04 Sep 2012 00:27:37 +0100 TimestampMonotonic=12876139 DefaultControlGroup=name=systemd:/user/gdm/1 VTNr=1 Display=:0 Remote=no Service=gdm-launch-environment Leader=3075 Audit=1 Type=x11 Class=user Active=no State=closing KillProcesses=no IdleHint=no IdleSinceHint=1346714860433194 IdleSinceHintMonotonic=16273801 Name=gdm [colin@jimmy systemd (master)]$ cat /run/systemd/sessions/1 # This is private data. Do not parse. UID=492 USER=gdm ACTIVE=0 STATE=online REMOTE=0 KILL_PROCESSES=0 TYPE=x11 CLASS=user CGROUP=/user/gdm/1 FIFO=/run/systemd/sessions/1.ref SEAT=seat0 DISPLAY=:0 SERVICE=gdm-launch-environment VTNR=1 LEADER=3075 AUDIT=1 I do have a patch that adds various calls to seat_save(), session_save(), user_save() at appropriate junctures, but I've not tested this yet and it would appear to be quite easy to get tied in knots and add infinite loops! Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] logind: Add a two new variables to the user session tracking file.
This counts 'online sessions' in addition to 'active sessions' and 'sessions'. In this context, an 'online session' covers all session in the 'active' state in addition to the explicit 'online' state. This provides an easy machanism to determin all relevant sessions easily (i.e. those that are not 'closing') and adds new semantics to the sd-login.c APIs sd_uid_get_sessions() and sd_uid_get_seats() where the require_active argument can be supplied as a value 2 which only lists sessions which are 'online'. This functionality should allow client applications to avoid deadlocks where they only exit when all sessions are complete, such as a the problem where PulseAudio will not exit until all sessions are gone, but in itself prevents the session from exiting. --- v2: Properly check for the !i->seat when listing online seats. --- src/login/logind-user.c | 28 src/login/sd-login.c| 4 ++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/login/logind-user.c b/src/login/logind-user.c index a6672ce..9dfead9 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -180,6 +180,20 @@ int user_save(User *u) { fputs(i->id, f); } +fputs("\nONLINE_SESSIONS=", f); +first = true; +LIST_FOREACH(sessions_by_user, i, u->sessions) { +if (session_get_state(i) == SESSION_CLOSING) +continue; + +if (first) +first = false; +else +fputc(' ', f); + +fputs(i->id, f); +} + fputs("\nACTIVE_SEATS=", f); first = true; LIST_FOREACH(sessions_by_user, i, u->sessions) { @@ -193,6 +207,20 @@ int user_save(User *u) { fputs(i->seat->id, f); } + +fputs("\nONLINE_SEATS=", f); +first = true; +LIST_FOREACH(sessions_by_user, i, u->sessions) { +if (session_get_state(i) == SESSION_CLOSING || !i->seat) +continue; + +if (first) +first = false; +else +fputc(' ', f); + +fputs(i->seat->id, f); +} fputc('\n', f); } diff --git a/src/login/sd-login.c b/src/login/sd-login.c index 1978a05..88dd510 100644 --- a/src/login/sd-login.c +++ b/src/login/sd-login.c @@ -259,11 +259,11 @@ static int uid_get_array(uid_t uid, const char *variable, char ***array) { } _public_ int sd_uid_get_sessions(uid_t uid, int require_active, char ***sessions) { -return uid_get_array(uid, require_active ? "ACTIVE_SESSIONS" : "SESSIONS", sessions); +return uid_get_array(uid, require_active == 2 ? "ONLINE_SESSIONS" : (require_active ? "ACTIVE_SESSIONS" : "SESSIONS"), sessions); } _public_ int sd_uid_get_seats(uid_t uid, int require_active, char ***seats) { -return uid_get_array(uid, require_active ? "ACTIVE_SEATS" : "SEATS", seats); +return uid_get_array(uid, require_active == 2 ? "ONLINE_SEATS" : (require_active ? "ACTIVE_SEATS" : "SEATS"), seats); } static int file_of_session(const char *session, char **_p) { -- 1.7.12 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 3/4] logind: Properly list the ACTIVE_SEATS in the user session tracking file.
Prevsiouly the first active seat for a user would never be listed and any subsequent seats would be concatenated on without any spaces. --- src/login/logind-user.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/login/logind-user.c b/src/login/logind-user.c index a33978c..a6672ce 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -189,7 +189,9 @@ int user_save(User *u) { if (first) first = false; else -fputs(i->seat->id, f); +fputc(' ', f); + +fputs(i->seat->id, f); } fputc('\n', f); } -- 1.7.12 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/4] pam: Add session class to the debug log.
--- src/login/pam-module.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/login/pam-module.c b/src/login/pam-module.c index 0727164..d7f4128 100644 --- a/src/login/pam-module.c +++ b/src/login/pam-module.c @@ -517,8 +517,8 @@ _public_ PAM_EXTERN int pam_sm_open_session( if (debug) pam_syslog(handle, LOG_DEBUG, "Asking logind to create session: " - "uid=%u pid=%u service=%s type=%s seat=%s vtnr=%u tty=%s display=%s remote=%s remote_user=%s remote_host=%s", - uid, pid, service, type, seat, vtnr, tty, display, yes_no(remote), remote_user, remote_host); + "uid=%u pid=%u service=%s type=%s class=%s seat=%s vtnr=%u tty=%s display=%s remote=%s remote_user=%s remote_host=%s", + uid, pid, service, type, class, seat, vtnr, tty, display, yes_no(remote), remote_user, remote_host); reply = dbus_connection_send_with_reply_and_block(bus, m, -1, &error); if (!reply) { -- 1.7.12 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/4] logind: If all user sessions are in closing state, set the overall status to closing.
PulseAudio for example will keep a client connection open provided at least one session exists. However, if all sessions are currently in the process of closing, we should flag that as the overall state appropriately to better reflect what is happening. Although this does better reflect the status for any given user, it does not actually solve the overall problem of PulseAudio still finding some sessions active and thus not exiting and therefore actually preventing the session from closing. Future commits will extend sd-login to cope with this situation. --- src/login/logind-user.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/login/logind-user.c b/src/login/logind-user.c index aa9c3f1..a33978c 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -568,15 +568,20 @@ void user_add_to_gc_queue(User *u) { UserState user_get_state(User *u) { Session *i; +bool all_closing = true; assert(u); -LIST_FOREACH(sessions_by_user, i, u->sessions) + +LIST_FOREACH(sessions_by_user, i, u->sessions) { if (session_is_active(i)) return USER_ACTIVE; +if (session_get_state(i) != SESSION_CLOSING) +all_closing = false; +} if (u->sessions) -return USER_ONLINE; +return all_closing ? USER_CLOSING : USER_ONLINE; if (user_check_linger_file(u) > 0) return USER_LINGERING; -- 1.7.12 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] (no subject)
Hi, OK, so these are the changes I'd propose to fix the issues mentioned in this thread. I've not tested them so this is more for general feedback as to whether this approach is generally a good one or not. The initial patch is just extra debugging I added to try and work out why my gdm session is not marked with type=greeter so seems like a generally useful change when in debug mode. Cheers Col ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 4/4] logind: Add a two new variables to the user session tracking file.
This counts 'online sessions' in addition to 'active sessions' and 'sessions'. In this context, an 'online session' covers all session in the 'active' state in addition to the explicit 'online' state. This provides an easy machanism to determin all relevant sessions easily (i.e. those that are not 'closing') and adds new semantics to the sd-login.c APIs sd_uid_get_sessions() and sd_uid_get_seats() where the require_active argument can be supplied as a value 2 which only lists sessions which are 'online'. This functionality should allow client applications to avoid deadlocks where they only exit when all sessions are complete, such as a the problem where PulseAudio will not exit until all sessions are gone, but in itself prevents the session from exiting. --- src/login/logind-user.c | 28 src/login/sd-login.c| 4 ++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/login/logind-user.c b/src/login/logind-user.c index a6672ce..3f7b413 100644 --- a/src/login/logind-user.c +++ b/src/login/logind-user.c @@ -180,6 +180,20 @@ int user_save(User *u) { fputs(i->id, f); } +fputs("\nONLINE_SESSIONS=", f); +first = true; +LIST_FOREACH(sessions_by_user, i, u->sessions) { +if (session_get_state(i) == SESSION_CLOSING) +continue; + +if (first) +first = false; +else +fputc(' ', f); + +fputs(i->id, f); +} + fputs("\nACTIVE_SEATS=", f); first = true; LIST_FOREACH(sessions_by_user, i, u->sessions) { @@ -193,6 +207,20 @@ int user_save(User *u) { fputs(i->seat->id, f); } + +fputs("\nONLINE_SEATS=", f); +first = true; +LIST_FOREACH(sessions_by_user, i, u->sessions) { +if (session_get_state(i) == SESSION_CLOSING) +continue; + +if (first) +first = false; +else +fputc(' ', f); + +fputs(i->seat->id, f); +} fputc('\n', f); } diff --git a/src/login/sd-login.c b/src/login/sd-login.c index 1978a05..88dd510 100644 --- a/src/login/sd-login.c +++ b/src/login/sd-login.c @@ -259,11 +259,11 @@ static int uid_get_array(uid_t uid, const char *variable, char ***array) { } _public_ int sd_uid_get_sessions(uid_t uid, int require_active, char ***sessions) { -return uid_get_array(uid, require_active ? "ACTIVE_SESSIONS" : "SESSIONS", sessions); +return uid_get_array(uid, require_active == 2 ? "ONLINE_SESSIONS" : (require_active ? "ACTIVE_SESSIONS" : "SESSIONS"), sessions); } _public_ int sd_uid_get_seats(uid_t uid, int require_active, char ***seats) { -return uid_get_array(uid, require_active ? "ACTIVE_SEATS" : "SEATS", seats); +return uid_get_array(uid, require_active == 2 ? "ONLINE_SEATS" : (require_active ? "ACTIVE_SEATS" : "SEATS"), seats); } static int file_of_session(const char *session, char **_p) { -- 1.7.12 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 2/2] tmpfiles: support globbing for w option
Break out the write logic into a separate function and simply use it as a callback to glob_item. This allows users to consolidate writes to sysfs with multiple similar pathnames, e.g. w /sys/class/block/sd[a-z]/queue/read_ahead_kb - - - - 1024 --- As is, and even without this patch, the example I point out won't work. I suspect this is a kernel bug, which I've reported: https://bugzilla.kernel.org/show_bug.cgi?id=46981 Of course, I'm also happy to provide a workaround here, but I assume that systemd upstream would rather just have the kernel bug fixed. I haven't come across any other sysfs nodes (yet) that exhibit this broken behavior. man/tmpfiles.d.xml | 2 +- src/tmpfiles/tmpfiles.c | 135 +--- 2 files changed, 72 insertions(+), 65 deletions(-) diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml index b7397b6..c6325a4 100644 --- a/man/tmpfiles.d.xml +++ b/man/tmpfiles.d.xml @@ -114,7 +114,7 @@ L/tmp/foobar ---- /dev/null w -Write the argument parameter to a file, if it exists. +Write the argument parameter to a file, if it exists. Lines of this type accept shell-style globs in place of normal path names. diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index e70332c..b44beee 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -474,6 +474,75 @@ static int item_set_perms(Item *i, const char *path) { return label_fix(path, false, false); } +static int write_one_file(Item *i, const char *path) { +int r, e, fd, flags; +struct stat st; +mode_t u; + +flags = i->type == CREATE_FILE ? O_CREAT|O_APPEND : +i->type == TRUNCATE_FILE ? O_CREAT|O_TRUNC : 0; + +u = umask(0); +label_context_set(path, S_IFREG); +fd = open(path, flags|O_NDELAY|O_CLOEXEC|O_WRONLY|O_NOCTTY|O_NOFOLLOW, i->mode); +e = errno; +label_context_clear(); +umask(u); +errno = e; + +if (fd < 0) { +if (i->type == WRITE_FILE && errno == ENOENT) +return 0; + +log_error("Failed to create file %s: %m", path); +return -errno; +} + +if (i->argument) { +ssize_t n; +size_t l; +struct iovec iovec[2]; +static const char new_line = '\n'; + +l = strlen(i->argument); + +zero(iovec); +iovec[0].iov_base = i->argument; +iovec[0].iov_len = l; + +iovec[1].iov_base = (void*) &new_line; +iovec[1].iov_len = 1; + +n = writev(fd, iovec, 2); + +/* It's OK if we don't write the trailing + * newline, hence we check for l, instead of + * l+1 here. Files in /sys often refuse + * writing of the trailing newline. */ +if (n < 0 || (size_t) n < l) { +log_error("Failed to write file %s: %s", path, n < 0 ? strerror(-n) : "Short write"); +close_nointr_nofail(fd); +return n < 0 ? n : -EIO; +} +} + +if (stat(path, &st) < 0) { +log_error("stat(%s) failed: %m", path); +return -errno; +} + +if (!S_ISREG(st.st_mode)) { +log_error("%s is not a file.", path); +return -EEXIST; +} + +r = item_set_perms(i, path); +if (r < 0) +return r; + +return 0; +} + static int recursive_relabel_children(Item *i, const char *path) { DIR *d; int ret = 0; @@ -607,74 +676,12 @@ static int create_item(Item *i) { case CREATE_FILE: case TRUNCATE_FILE: -case WRITE_FILE: { -int fd, flags; - -flags = i->type == CREATE_FILE ? O_CREAT|O_APPEND : -i->type == TRUNCATE_FILE ? O_CREAT|O_TRUNC : 0; - -u = umask(0); -label_context_set(i->path, S_IFREG); -fd = open(i->path, flags|O_NDELAY|O_CLOEXEC|O_WRONLY|O_NOCTTY|O_NOFOLLOW, i->mode); -e = errno; -label_context_clear(); -umask(u); -errno = e; - -if (fd < 0) { -if (i->type == WRITE_FILE && errno == ENOENT) -break; - -log_error("Failed to create file %s: %m", i->path); -return -errno; -} - -if (i->argument) { -ssize_t n; -size_t l; -
[systemd-devel] [PATCH 1/2] shared/install: ignore ENOENT from unit_file_can_install
When a service file contains an .include directive with a path that doesn't exist, systemctl list-unit-files will simply error out with: Failed to get unit file list: No such file or directory To reproduce: # echo ".include /this/doesnt/exist" >/etc/systemd/system/foo.service # systemctl daemon-reload # systemctl list-unit-files --- This is a repeat of a patch I posted previously, and later when it was reviewed, I wasn't able to reproduce: http://lists.freedesktop.org/archives/systemd-devel/2012-July/005766.html As for the journal logging, the message already exists: Sep 03 17:00:44 beatbox systemd[1]: Failed to open configuration file '/this/doesnt/exist': No such file or directory Should this be surfaced to the terminal as well? src/shared/install.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/install.c b/src/shared/install.c index ef1c3f5..2863a43 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -2007,7 +2007,7 @@ int unit_file_get_list( goto found; r = unit_file_can_install(&paths, root_dir, f->path, true); -if (r < 0) { +if (r < 0 && r != -ENOENT) { free(f->path); free(f); goto finish; -- 1.7.12 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] alsa-restore.service seems to be too early
03.09.2012, 22:40, "Colin Guthrie" : > I would lean towards this being a kernel problem or some other rule in > place which invalidates the action (e.g. perhaps it's restored but a > latter call issues an "alsactl init 1"). > > I don't know enough about udev or kernel side of things to really say > where the problem lies, so it's probably one to quiz Lennart and Kay > about once they are back from the sprints/conferences. I see, thanks, will wait for. P.S. I have not found an official community for systemd users, and have selected this mailing list. Is described problem appropriate to the list? I'd want to avoid any development process disturbance :) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] alsa-restore.service seems to be too early
'Twas brillig, and Вечный Студент at 03/09/12 11:34 did gyre and gimble: > 03.09.2012, 13:48, "Colin Guthrie" : >> Then you should probably try and debug this further - e.g. by rmmod'ing >> the module and inserting it and trying to work out why it's not run. You >> can always replace the rule with one that runs a script instead that >> writes debugging info or similar. >> >> In theory the control device should appear last to udev (something which >> we had to fight with a few years back with PulseAudio). When the control >> device appears it should be all ready. The only complication that I can >> think of is that there might be some kind of firmware loading issue that >> means that the control device appears before the device can really be used. >> >> If you do replace it with a script, try introducing a sleep in it > > Yes, thanks, sleeping does help (the card under question is the second one, > i.e. with suffix '1'): > > ~ $ cat /etc/udev/rules.d/99-zlocal.rules | grep alsa > ACTION=="add", SUBSYSTEM=="sound", KERNEL=="controlC1", KERNELS=="card1", > RUN+="/usr/local/bin/realsa.sh" > ~ $ cat /usr/local/bin/realsa.sh > #!/bin/bash > > sleep 1 > /usr/sbin/alsactl restore > >> to >> give the firmware time to load - this isn't a solution of course, but it >> might help work out if it's a timing thing and that, in turn, might >> highlight where the real solution lies. > > Sorry, I don't see real solution yet :) I would lean towards this being a kernel problem or some other rule in place which invalidates the action (e.g. perhaps it's restored but a latter call issues an "alsactl init 1"). I don't know enough about udev or kernel side of things to really say where the problem lies, so it's probably one to quiz Lennart and Kay about once they are back from the sprints/conferences. Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] gdm's PulseAudio not exiting.
'Twas brillig, and Colin Guthrie at 03/09/12 14:47 did gyre and gimble: > [colin@jimmy systemd-189]$ cat /run/systemd/users/492 > # This is private data. Do not parse. > NAME=gdm > STATE=online > CGROUP=/user/gdm > RUNTIME=/run/user/492 > DISPLAY=1 > SESSIONS=1 > SEATS=seat0 > ACTIVE_SESSIONS= > ACTIVE_SEATS= > > > It does not seem to reflect anything to do with the fact that the > session is closing. > > So, how can I differentiate between an State=active session and a > State=closing one via the sd_* methods? > > Does something extra need to get added to this env var and a new API > method added to sd-login.c to cope with this? > > > I would propose that STATE= variable gets updated to "closing" here (if > possible) and that sd_uid_get_sessions() is modified such that passing a > require_active value of 2 (or -1?) would only return if a session is > both SESSIONS=(>=1) and STATE=(active|online). > > > Alternatively, we could just make uid_get_array() always require that > STATE=(active|online) and make that hard coded. > > > WDYT? I realise this suggestion is not possible, as the STATE= value here is aggregate over multiple sessions. I thus change the proposal to add a new variable to the env file: ONLINE_SESSIONS= This is very similar to SESSIONS= but will exclude any sessions in the closing state. The implementation of sd_uid_get_sessions() can then either be changed to search for ONLINE_SESSIONS= vs. ACTIVE_SESSIONS=, or the require_active value can be end up with three values: 0 == SESSIONS, 1 == ACTIVE_SESSIONS, 2 == ONLINE_SESSIONS. Either that or a new function: sd_uid_get_online_sessions() be used instead. Personally I actually prefer the three-value approach as it degrades somewhat nicely. Opinions welcome. Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] journal: don't try to compress without XZ
otherwise the header contains the HEADER_INCOMPATIBLE_COMPRESSED flag even though the data is not compressed and reading the journal fails. --- Hi, I'm not sure if this is the correct place to do this, but the default 'compress = yes' must be ignored somewhere otherwise journalctl will not work with the default configuration and XZ support disabled. Regards, Michael src/journal/journal-file.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c index 79f7598..81ba45c 100644 --- a/src/journal/journal-file.c +++ b/src/journal/journal-file.c @@ -2007,7 +2007,9 @@ int journal_file_open( f->flags = flags; f->prot = prot_from_flags(flags); f->writable = (flags & O_ACCMODE) != O_RDONLY; +#ifdef HAVE_XZ f->compress = compress; +#endif f->seal = seal; if (mmap_cache) -- 1.7.10.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] libsystemd-core needs libsystemd-id128-internal
Otherwise compiling may fail with e.g.: ./.libs/libsystemd-core.a(libsystemd_core_la-condition.o): In function `test_host': [...]/systemd-189/src/core/condition.c:205: undefined reference to `sd_id128_from_string' [...]/systemd-189/src/core/condition.c:207: undefined reference to `sd_id128_get_machine' collect2: ld returned 1 exit status It seems that in most cases the compiler removes the relevant functions when optimizing. In some cases this does not happen, e.g. here with a PPC toolchain. --- Makefile.am |1 + 1 file changed, 1 insertion(+) diff --git a/Makefile.am b/Makefile.am index e5ace9b..138673e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1003,6 +1003,7 @@ libsystemd_core_la_LIBADD = \ libsystemd-label.la \ libsystemd-shared.la \ libsystemd-dbus.la \ + libsystemd-id128-internal.la \ libudev.la \ $(LIBWRAP_LIBS) \ $(PAM_LIBS) \ -- 1.7.10.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] gdm's PulseAudio not exiting.
Hi, Been poking about trying to work out why gdm's session would not exit. [colin@jimmy systemd-189]$ loginctl session-status 1 1 - gdm (492) Since: Mon, 03 Sep 2012 09:30:34 +0100; 4h 42min ago Leader: 3203 Seat: seat0; vc1 Display: :0 Service: gdm-launch-environment; type x11; class user State: closing CGroup: name=systemd:/user/gdm/1 ├ 3498 /usr/bin/pulseaudio --start --log-target=syslog └ 3707 /usr/lib64/pulse/gconf-helper So PA is running. Debugging a little further: [gdm@jimmy ~]$ pacmd list-clients Welcome to PulseAudio! Use "help" for usage information. >>> 2 client(s) logged in. index: 0 driver: owner module: 17 properties: application.name = "Login Session 1" systemd-login.session = "1" index: 21 driver: owner module: 24 properties: application.name = "UNIX socket client" So there is a client still around for module-systemd-login which prevents PA from doing it's idle exit. Looking at the PA code for this module, it seems that it just lists all the sessions via: r = sd_uid_get_sessions(getuid(), 0, &sessions); This means that it will return if there are any sessions, active or otherwise. I agree that if any session exits then the PA module should indeed keep a client active and prevent idle exit. The actual state of the session shouldn't really matter - if it exists, it's good enough (which allows for fast user switch etc.) However there is one caveat to that... if the session is in "State=closing", then it should be ignored by PA, as otherwise there is a deadlock. Sadly there is no call in sd-login.c that can filter sessions in this way. And even looking in the env file itself: [colin@jimmy systemd-189]$ cat /run/systemd/users/492 # This is private data. Do not parse. NAME=gdm STATE=online CGROUP=/user/gdm RUNTIME=/run/user/492 DISPLAY=1 SESSIONS=1 SEATS=seat0 ACTIVE_SESSIONS= ACTIVE_SEATS= It does not seem to reflect anything to do with the fact that the session is closing. So, how can I differentiate between an State=active session and a State=closing one via the sd_* methods? Does something extra need to get added to this env var and a new API method added to sd-login.c to cope with this? I would propose that STATE= variable gets updated to "closing" here (if possible) and that sd_uid_get_sessions() is modified such that passing a require_active value of 2 (or -1?) would only return if a session is both SESSIONS=(>=1) and STATE=(active|online). Alternatively, we could just make uid_get_array() always require that STATE=(active|online) and make that hard coded. WDYT? Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] rm_rf_children_dangerous: delete all descendants dangerously
Call rm_rf_children_dangerous() recursively rather than falling back to rm_rf_children(). This fixes a bug in systemd-tmpfiles. The problem can easily be reproduced by: # mount /dev/sda1 /mnt # mkdir /mnt/test # echo "D /mnt" > /root/test.conf # systemd-tmpfiles --remove /root/test.conf Attempted to remove disk file system, and we can't allow that. rm_rf(/root/test): Operation not permitted Reported-by: Lukas Jirkovsky --- src/shared/util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/util.c b/src/shared/util.c index 95b577b..d958cdc 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -3358,7 +3358,7 @@ int rm_rf_children_dangerous(int fd, bool only_dirs, bool honour_sticky, struct continue; } -r = rm_rf_children(subdir_fd, only_dirs, honour_sticky, root_dev); +r = rm_rf_children_dangerous(subdir_fd, only_dirs, honour_sticky, root_dev); if (r < 0 && ret == 0) ret = r; -- 1.7.12 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] tmpfiles: allow Age to be set to 0d
Mostly useful for testing purposes. Setting Age to 1s works just as well, but it is surprising that using 0 does not work. We had bug reports/confused users in the past, so it makes sense to change this. --- src/tmpfiles/tmpfiles.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index e70332c..7c2a754 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -406,7 +406,7 @@ static int clean_item(Item *i) { i->type != IGNORE_PATH) return 0; -if (!i->age_set || i->age <= 0) +if (!i->age_set || i->age < 0) return 0; n = now(CLOCK_REALTIME); -- 1.7.12 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] alsa-restore.service seems to be too early
03.09.2012, 13:48, "Colin Guthrie" : > Then you should probably try and debug this further - e.g. by rmmod'ing > the module and inserting it and trying to work out why it's not run. You > can always replace the rule with one that runs a script instead that > writes debugging info or similar. > > In theory the control device should appear last to udev (something which > we had to fight with a few years back with PulseAudio). When the control > device appears it should be all ready. The only complication that I can > think of is that there might be some kind of firmware loading issue that > means that the control device appears before the device can really be used. > > If you do replace it with a script, try introducing a sleep in it Yes, thanks, sleeping does help (the card under question is the second one, i.e. with suffix '1'): ~ $ cat /etc/udev/rules.d/99-zlocal.rules | grep alsa ACTION=="add", SUBSYSTEM=="sound", KERNEL=="controlC1", KERNELS=="card1", RUN+="/usr/local/bin/realsa.sh" ~ $ cat /usr/local/bin/realsa.sh #!/bin/bash sleep 1 /usr/sbin/alsactl restore > to > give the firmware time to load - this isn't a solution of course, but it > might help work out if it's a timing thing and that, in turn, might > highlight where the real solution lies. Sorry, I don't see real solution yet :) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] alsa-restore.service seems to be too early
[ Please don't drop the list from the CC ] 'Twas brillig, and Вечный Студент at 03/09/12 10:33 did gyre and gimble: > 03.09.2012, 13:05, "Colin Guthrie" : >> Yes this unit is just a part of a two part solution here. See the >> commit where it was introduced for an explanation: >> >> http://git.alsa-project.org/?p=alsa-utils.git;a=commitdiff;h=de7c3eff0e371ce155403bbcdcf81ee79266fa0f >> >> >> So the udev rule is the other part of the puzzle that should deal with >> restoration during hotplug events. >> >> You should have the file >> [/usr]/lib/udev/rules.d/90-alsa-restore.rules > > Thanks for the tip! In fact I have the rule file installed: > > ~ $ cat /usr/lib/udev/rules.d/90-alsa-restore.rules ACTION=="add", > SUBSYSTEM=="sound", KERNEL=="controlC*", KERNELS=="card*", \ > RUN+="/usr/sbin/alsactl restore $attr{number}" > > But the thing isn't done. I guess the issue is driver-specific. Then you should probably try and debug this further - e.g. by rmmod'ing the module and inserting it and trying to work out why it's not run. You can always replace the rule with one that runs a script instead that writes debugging info or similar. In theory the control device should appear last to udev (something which we had to fight with a few years back with PulseAudio). When the control device appears it should be all ready. The only complication that I can think of is that there might be some kind of firmware loading issue that means that the control device appears before the device can really be used. If you do replace it with a script, try introducing a sleep in it to give the firmware time to load - this isn't a solution of course, but it might help work out if it's a timing thing and that, in turn, might highlight where the real solution lies. > I think at my case some kind of - in initscripts terms - rc.local must > be used - a service with 'After=all' :) Is there a tool presenting in > some (pseudo)graphics form a graph with Before/After deps of active > units? I need to find that After=all... No there is no such concept. It's broken by design anyway so you should not follow this path for a solution to this problem - it needs to be fixed properly such that things happen in a proper async friendly manner and don't rely on theoretical moments in time when everything is "ready" (which don't really exist anyway - even if such solutions do work 99% of the time, they are still technically wrong!) Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Handling lid close in logind?
On 3 September 2012 09:17, Zbigniew Jędrzejewski-Szmek wrote: > Laptop emits a long series of angry beeps. If it starts really overheating, > thermal protection kicks in and the laptop shuts down. Unless you're a macbook, with the white thermoplastic cover. Gloopy mess :) I don't think the desktop guys want any kind of authentication when the lid is closed. How feasible would it be to do the suspend when the inhibit is removed? I guess then it puts logind into a "active but will suspend asap" state which might be difficult to handle. Richard. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] alsa-restore.service seems to be too early
'Twas brillig, and Вечный Студент at 02/09/12 15:48 did gyre and gimble: > In accordance with > > ~ $ cat /etc/modprobe.d/alsa.conf > options snd slots=snd-hda-intel,snd-hdsp,snd-usb-audio > > I have three sound cards. The second one's state (snd-hdsp) isn't restored > after booting up (say, Sample Clock Source parameter is set to default 48 KHz > rather to AutoSync). Starting 'alsactl restore' manually restores the state. > Used service is listed below. Where to dig in? > > > a > > //--- > ~ $ sudo systemctl | grep basic > basic.target loaded active activeBasic System > ~ $ cat /usr/lib/systemd/system/basic.target.wants/alsa-restore.service > [Unit] > Description=Restore Sound Card State > DefaultDependencies=no > After=sysinit.target > Before=shutdown.target > Conflicts=shutdown.target > > [Service] > Type=oneshot > ExecStart=-/usr/sbin/alsactl restore > StandardOutput=syslog Yes this unit is just a part of a two part solution here. See the commit where it was introduced for an explanation: http://git.alsa-project.org/?p=alsa-utils.git;a=commitdiff;h=de7c3eff0e371ce155403bbcdcf81ee79266fa0f So the udev rule is the other part of the puzzle that should deal with restoration during hotplug events. You should have the file [/usr]/lib/udev/rules.d/90-alsa-restore.rules Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Handling lid close in logind?
On Mon, Sep 03, 2012 at 09:01:06AM +0100, Richard Hughes wrote: > Hey, > > I know Lennart and Kay are still on walkabout, but I wanted to ask any > opinions on https://bugzilla.gnome.org/show_bug.cgi?id=680689 > > Basically: > > * User inhibits suspend, perhaps by doing an update or burning a CD > * User closes laptop > * Laptop shows polkit auth box, but the lid is closed and the user > doesn't see the dialog Laptop emits a long series of angry beeps. If it starts really overheating, thermal protection kicks in and the laptop shuts down. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Handling lid close in logind?
On Mon, 2012-09-03 at 09:01 +0100, Richard Hughes wrote: > Hey, > > I know Lennart and Kay are still on walkabout, but I wanted to ask any > opinions on https://bugzilla.gnome.org/show_bug.cgi?id=680689 > > Basically: > > * User inhibits suspend, perhaps by doing an update or burning a CD Once the application has finished, can't the inhibition be released, the polkit dialog disappear and the laptop suspend automatically? -- Mathieu ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] Handling lid close in logind?
Hey, I know Lennart and Kay are still on walkabout, but I wanted to ask any opinions on https://bugzilla.gnome.org/show_bug.cgi?id=680689 Basically: * User inhibits suspend, perhaps by doing an update or burning a CD * User closes laptop * Laptop shows polkit auth box, but the lid is closed and the user doesn't see the dialog * Users puts laptop in bag * Laptop overheats * User writes angry blog Ideas very welcome. Thanks. Richard. ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel