Re: [systemd-devel] Why don't remote file systems wait for network-online.target?
Am 2015-03-10 12:40, schrieb Martin Pitt: we got a report [1] that NFS fstab mounts (sometimes) aren't being mounted at boot as the network is still down: | mount[866]: mount.nfs: Network is unreachable | systemd[1]: mnt-server.mount mount process exited, code=exited status=32 | systemd[1]: Failed to mount /mnt/server. | systemd[1]: Dependency failed for Remote File Systems. | systemd[1]: Job remote-fs.target/start failed with result 'dependency'. | systemd[1]: Unit mnt-server.mount entered failed state. At the moment, neither network-fs-pre.target has no dependencies at all, and the fstab-generator-created .mount units only have Before=remote-fs.target and no other dependencies. I can't reproduce that under Debian Jessie with systemd-215. Individual NFS mounts (mnt-server.mount in your case) are ordered like this here: After=systemd-journald.socket remote-fs-pre.target network.target network-online.target system.slice -.mount Wants=network-online.target system.slice This is part of mount unit's DefaultDependencies, see: http://cgit.freedesktop.org/systemd/systemd/tree/src/core/mount.c#n366 (mount_is_network checks for fstype or _netdev; and nfs is in the list of filesystems considered network.) Christian ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Why don't remote file systems wait for network-online.target?
В Tue, 10 Mar 2015 12:40:40 +0100
Martin Pitt martin.p...@ubuntu.com пишет:
Hello all,
we got a report [1] that NFS fstab mounts (sometimes) aren't being
mounted at boot as the network is still down:
| mount[866]: mount.nfs: Network is unreachable
| systemd[1]: mnt-server.mount mount process exited, code=exited status=32
| systemd[1]: Failed to mount /mnt/server.
| systemd[1]: Dependency failed for Remote File Systems.
| systemd[1]: Job remote-fs.target/start failed with result 'dependency'.
| systemd[1]: Unit mnt-server.mount entered failed state.
At the moment, neither network-fs-pre.target has no dependencies at
all, and the fstab-generator-created .mount units only have
Before=remote-fs.target and no other dependencies.
Dependency on network-online.target is supposed to be implicitly added
(oh, those implicit undocumented dependencies ...)
src/core/mount.c:mount_add_default_dependencies()
...
if (mount_is_network(p)) {
after = SPECIAL_REMOTE_FS_PRE_TARGET;
after2 = SPECIAL_NETWORK_TARGET;
online = SPECIAL_NETWORK_ONLINE_TARGET;
...
if (online) {
r = unit_add_two_dependencies_by_name(UNIT(m), UNIT_WANTS,
UNIT_AFTER, online, NULL, true);
If it does not happen, either default dependencies are not applied or
systemd does not consider those filesystems network.
Wouldn't it make sense to make network-fs-pre.target
Wants/After=network-online to fix this? If you have/rely on NFS
mounts, then you usually have some static/always working network
connection via networkd/auto-connection in NM/ifupdown/etc, which all
integrate into network-online.target.
Thanks,
Martin
[1] https://launchpad.net/bugs/1429975
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/5] fsckd: Don't use strjoina on gettext() call
Le 09/03/2015 19:11, Lennart Poettering a écrit :
So I fixed a number of issues, but there are some bits I am not really
keen to fix, mostly because it's hard for me to test as I don't use a
file system that still requires fsck...
- /dev/console needs to be opened only on demand but not continously,
to keep the SAK window as small as possible.
- A clear regime needs to be enforced: either functions log errors on
their own, or they don't. In the latter case the calling function
needs to log errors instead. Currently some of the functions (like
send_message_plymouth_socket()) log some errors but don't log
others. And sometimes if they currently do log the functions
invoking them log each error a second time! (As is the case for
send_message_plymouth() which calls send_message_plymouth_socket()).
- We should avoid mixing stack allocation calls and function
calls. See man page of alloca() about this. Hence: invoking
strjoina() around gettext calls is *not* OK.
And there's probably more to fix...
Anyway, please look into fixing this, I am kinda relying on patches
for this, as I don't use this myself. Fedora isn't set up for it, nor
do I use a file system that still requires fsck at boot...
Those 3 points should be fixed or at least enhanced with the set of
patches I'm attaching now. Please look in particular on the 02 one as I
changed back some errors values introduced by your recent commits (I'm
unsure if that was just an oversight). I tried to rationalize the log
erroring to be more coherent (and especially avoiding double erroring),
hope this is what you wanted.
I didn't touch to the communication (and so, global flow) from
systemd-fsck to systemd-fsckd point yet as changing it raises quite some
issues as discussed in the other part of the thread (how to ensure the
unit instance is in error if fsck fails or is cancel? How to even send
the cancel message request itself if we don't have any communication back?)
Feel free to point any mistake.
Didier
From 4d279e1818dd05dccdf4595a5bf1f3dc32c3e8c2 Mon Sep 17 00:00:00 2001
From: Didier Roche didro...@ubuntu.com
Date: Tue, 10 Mar 2015 08:58:23 +0100
Subject: [PATCH 1/5] fsckd: Don't use strjoina on gettext() call
---
src/fsckd/fsckd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index dedc828..3fc923b 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -242,7 +242,7 @@ static int plymouth_send_message(int plymouth_fd, const char *message, bool upda
}
static int manager_send_plymouth_message(Manager *m, const char *message) {
-const char *plymouth_cancel_message = NULL;
+_cleanup_free_ const char *plymouth_cancel_message = NULL;
int r;
r = manager_connect_plymouth(m);
@@ -258,7 +258,7 @@ static int manager_send_plymouth_message(Manager *m, const char *message) {
m-plymouth_cancel_sent = true;
-plymouth_cancel_message = strjoina(fsckd-cancel-msg:, _(Press Ctrl+C to cancel all filesystem checks in progress));
+plymouth_cancel_message = strjoin(fsckd-cancel-msg:, _(Press Ctrl+C to cancel all filesystem checks in progress), NULL);
r = plymouth_send_message(m-plymouth_fd, plymouth_cancel_message, false);
if (r 0)
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] path_is_mount_point: handle false positive on some fs
On Mon, Mar 09, 2015 at 11:27:09AM +0100, Didier Roche wrote: Also we could on the longer term maybe getting the whole path_is_mount_point() logic into libmount from util-linux, using mnt_get_mountpoint() (but this one only use st_dev comparison presently)? mnt_table_find_mountpoint() http://ftp.kernel.org/pub/linux/utils/util-linux/v2.26/libmount-docs/libmount-Table-of-filesystems.html#mnt-table-find-mountpoint Karel -- Karel Zak k...@redhat.com http://karelzak.blogspot.com ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes
2015-03-10 12:41 GMT+01:00 Shawn Landden sh...@churchofgit.com:
---
TODO | 2 -
man/systemd.socket.xml | 7 ++-
src/core/service.c | 41 -
src/libsystemd/sd-resolve/test-resolve.c | 2 +-
src/shared/socket-util.c | 76
+++-
src/shared/socket-util.h | 4 +-
src/timesync/timesyncd-server.h | 2 +-
7 files changed, 106 insertions(+), 28 deletions(-)
diff --git a/TODO b/TODO
index ae32388..780084a 100644
--- a/TODO
+++ b/TODO
@@ -164,8 +164,6 @@ Features:
* as soon as we have kdbus, and sender timestamps, revisit coalescing
multiple parallel daemon reloads:
http://lists.freedesktop.org/archives/systemd-devel/2014-December/025862.html
-* set $REMOTE_IP (or $REMOTE_ADDR/$REMOTE_PORT) environment variable when
doing per-connection socket activation. use format introduced by xinetd or
CGI for this
-
* the install state probably shouldn't get confused by generated units,
think dbus1/kdbus compat!
* in systemctl list-unit-files: show the install value the presets would
suggest for a service in a third column
diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 3938345..6808179 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -357,7 +357,12 @@
daemons designed for usage with
citerefentryrefentrytitleinetd/refentrytitlemanvolnum8/manvolnum/citerefentry
to work unmodified with systemd socket
-activation./para/listitem
+activation./para
+
+paraFor IPv4 and IPv6 connections the
varnameREMOTE_ADDR/varname
+environment variable will contain the remote IP, and
varnameREMOTE_PORT/varname
+will contain the remote port. This is the same as the format used by
CGI.
+For SOCK_RAW the port is the IP protocol./para/listitem
/varlistentry
varlistentry
diff --git a/src/core/service.c b/src/core/service.c
index cc4ea19..bcfce96 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1095,7 +1095,7 @@ static int service_spawn(
if (r 0)
goto fail;
-our_env = new0(char*, 4);
+our_env = new0(char*, 6);
if (!our_env) {
r = -ENOMEM;
goto fail;
@@ -1119,6 +1119,45 @@ static int service_spawn(
goto fail;
}
+if (UNIT_DEREF(s-accept_socket)) {
+union sockaddr_union sa;
+socklen_t salen = sizeof(sa);
+
+r = getpeername(s-socket_fd, sa.sa, salen);
+if (r 0) {
+r = -errno;
+goto fail;
+}
+
+if (IN_SET(sa.sa.sa_family, AF_INET, AF_INET6)) {
+_cleanup_free_ char *addr = NULL;
+char *t;
+int port;
+
+r = sockaddr_pretty(sa.sa, salen, true, false,
addr);
+if (r 0)
+goto fail;
+
+t = strappend(REMOTE_ADDR=, addr);
+if (!t) {
+r = -ENOMEM;
+goto fail;
+}
+our_env[n_env++] = t;
+
+port = sockaddr_port(sa.sa);
+if (port 0) {
+r = port;
+goto fail;
+}
+
+if (asprintf((our_env + n_env++), REMOTE_PORT=%u,
port) 0) {
+r = -ENOMEM;
+goto fail;
+}
+}
+}
+
final_env = strv_env_merge(2, UNIT(s)-manager-environment,
our_env, NULL);
if (!final_env) {
r = -ENOMEM;
diff --git a/src/libsystemd/sd-resolve/test-resolve.c
b/src/libsystemd/sd-resolve/test-resolve.c
index 3187ce9..354a407 100644
--- a/src/libsystemd/sd-resolve/test-resolve.c
+++ b/src/libsystemd/sd-resolve/test-resolve.c
@@ -46,7 +46,7 @@ static int getaddrinfo_handler(sd_resolve_query *q, int
ret, const struct addrin
for (i = ai; i; i = i-ai_next) {
_cleanup_free_ char *addr = NULL;
-assert_se(sockaddr_pretty(i-ai_addr, i-ai_addrlen, false,
addr) == 0);
+assert_se(sockaddr_pretty(i-ai_addr, i-ai_addrlen, false,
true, addr) == 0);
puts(addr);
}
diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
index 74d90fa..0d87cb1 100644
--- a/src/shared/socket-util.c
+++ b/src/shared/socket-util.c
@@ -297,7 +297,7 @@ int socket_address_print(const SocketAddress *a, char
**ret) {
[systemd-devel] [PATCH 2/5] fsckd: Fix some error return values
From 689c3c1808bd286ed96d36e4fc7ff875e2477697 Mon Sep 17 00:00:00 2001
From: Didier Roche didro...@ubuntu.com
Date: Tue, 10 Mar 2015 09:01:11 +0100
Subject: [PATCH 2/5] fsckd: Fix some error return values
Change slightly recent changes introduced by
0b02c7c36dbb6f2ec7434eb8d18e0410ee1cc74c and
d42688ef21a695f8a30b0d899dafdc6ff1ee0973:
- do not return 0 when log_oom() couldn't create the manager.
- returns 0 if we connect successfully to plymouth.
---
src/fsckd/fsckd.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 3fc923b..9393379 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -220,7 +220,7 @@ static int manager_connect_plymouth(Manager *m) {
goto fail;
}
-return 1;
+return 0;
fail:
manager_disconnect_plymouth(m);
@@ -406,10 +406,8 @@ static int manager_new_connection_handler(sd_event_source *s, int fd, uint32_t r
log_debug(New fsck client connected to fd: %d, new_client_fd);
c = new0(Client, 1);
-if (!c) {
-log_oom();
-return 0;
-}
+if (!c)
+return log_oom();
c-fd = new_client_fd;
new_client_fd = -1;
@@ -417,7 +415,7 @@ static int manager_new_connection_handler(sd_event_source *s, int fd, uint32_t r
r = sd_event_add_io(m-event, c-event_source, c-fd, EPOLLIN, client_progress_handler, c);
if (r 0) {
log_oom();
-return 0;
+return r;
}
LIST_PREPEND(clients, m-clients, c);
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] Why don't remote file systems wait for network-online.target?
Hello all, we got a report [1] that NFS fstab mounts (sometimes) aren't being mounted at boot as the network is still down: | mount[866]: mount.nfs: Network is unreachable | systemd[1]: mnt-server.mount mount process exited, code=exited status=32 | systemd[1]: Failed to mount /mnt/server. | systemd[1]: Dependency failed for Remote File Systems. | systemd[1]: Job remote-fs.target/start failed with result 'dependency'. | systemd[1]: Unit mnt-server.mount entered failed state. At the moment, neither network-fs-pre.target has no dependencies at all, and the fstab-generator-created .mount units only have Before=remote-fs.target and no other dependencies. Wouldn't it make sense to make network-fs-pre.target Wants/After=network-online to fix this? If you have/rely on NFS mounts, then you usually have some static/always working network connection via networkd/auto-connection in NM/ifupdown/etc, which all integrate into network-online.target. Thanks, Martin [1] https://launchpad.net/bugs/1429975 -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 3/5] fsckd: Reduce the SAK window when writing to console
From 3e877d1d493476f63fa6af7997914f93b50218bd Mon Sep 17 00:00:00 2001
From: Didier Roche didro...@ubuntu.com
Date: Tue, 10 Mar 2015 09:57:38 +0100
Subject: [PATCH 3/5] fsckd: Reduce the SAK window when writing to console
We don't want to keep /dev/console open all the time, but only open it when
needed, to avoid interfering with SAK.
---
src/fsckd/fsckd.c | 66 ---
1 file changed, 39 insertions(+), 27 deletions(-)
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 9393379..f73d23b 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -76,12 +76,12 @@ typedef struct Manager {
LIST_HEAD(Client, clients);
unsigned n_clients;
-int clear;
+size_t clear;
int connection_fd;
sd_event_source *connection_event_source;
-FILE *console;
+bool show_status_console;
double percent;
int numdevices;
@@ -99,6 +99,36 @@ static void manager_free(Manager *m);
DEFINE_TRIVIAL_CLEANUP_FUNC(Client*, client_free);
DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
+static int manager_write_console(Manager *m, const char *message) {
+_cleanup_fclose_ FILE *console = NULL;
+int l;
+size_t j;
+
+assert(m);
+
+if (!m-show_status_console)
+return 0;
+
+/* Reduce the SAK window by opening and closing console on every request */
+console = fopen(/dev/console, we);
+if (!console)
+return -errno;
+
+if (message) {
+fprintf(console, \r%s\r%n, message, l);
+if (m-clear (size_t)l)
+m-clear = (size_t)l;
+} else {
+fputc('\r', console);
+for (j = 0; j m-clear; j++)
+fputc(' ', console);
+fputc('\r', console);
+}
+fflush(console);
+
+return 0;
+}
+
static double compute_percent(int pass, size_t cur, size_t max) {
/* Values stolen from e2fsck */
@@ -284,7 +314,7 @@ static int manager_update_global_progress(Manager *m) {
Client *current = NULL;
_cleanup_free_ char *console_message = NULL;
_cleanup_free_ char *fsck_message = NULL;
-int current_numdevices = 0, l = 0, r;
+int current_numdevices = 0, r;
double current_percent = 100;
/* get the overall percentage */
@@ -311,19 +341,15 @@ static int manager_update_global_progress(Manager *m) {
if (asprintf(fsck_message, fsckd:%d:%3.1f:%s, m-numdevices, m-percent, console_message) 0)
return -ENOMEM;
-/* write to console */
-if (m-console) {
-fprintf(m-console, \r%s\r%n, console_message, l);
-fflush(m-console);
-}
+r = manager_write_console(m, console_message);
+if (r 0)
+return r;
/* try to connect to plymouth and send message */
r = manager_send_plymouth_message(m, fsck_message);
if (r 0)
log_debug(Couldn't send message to plymouth);
-if (l m-clear)
-m-clear = l;
}
return 0;
}
@@ -435,15 +461,7 @@ static void manager_free(Manager *m) {
return;
/* clear last line */
-if (m-console m-clear 0) {
-unsigned j;
-
-fputc('\r', m-console);
-for (j = 0; j (unsigned) m-clear; j++)
-fputc(' ', m-console);
-fputc('\r', m-console);
-fflush(m-console);
-}
+manager_write_console(m, NULL);
sd_event_source_unref(m-connection_event_source);
safe_close(m-connection_fd);
@@ -453,9 +471,6 @@ static void manager_free(Manager *m) {
manager_disconnect_plymouth(m);
-if (m-console)
-fclose(m-console);
-
sd_event_unref(m-event);
free(m);
@@ -479,11 +494,8 @@ static int manager_new(Manager **ret, int fd) {
if (r 0)
return r;
-if (access(/run/systemd/show-status, F_OK) = 0) {
-m-console = fopen(/dev/console, we);
-if (!m-console)
-return -errno;
-}
+if (access(/run/systemd/show-status, F_OK) = 0)
+m-show_status_console = true;
r = sd_event_add_io(m-event, m-connection_event_source, fd, EPOLLIN, manager_new_connection_handler, m);
if (r 0)
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 5/5] fsckd: clean up log messages
From 32c1aec9bddf21b1380eb8f7b801468d3875e2a9 Mon Sep 17 00:00:00 2001
From: Didier Roche didro...@ubuntu.com
Date: Tue, 10 Mar 2015 10:18:00 +0100
Subject: [PATCH 5/5] fsckd: clean up log messages
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Avoid double logs printing. Not that we don't return
manager_update_global_progress() to the handler callback as if the console or
plymouth isn't available momentarily, we still desire to handle future
fd progress events if those are available again (like cancellation, reportsâ¦)
---
src/fsckd/fsckd.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 52d69cd..5e5784b 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -354,8 +354,7 @@ static int manager_update_global_progress(Manager *m) {
/* try to connect to plymouth and send message */
r = manager_send_plymouth_message(m, fsck_message);
if (r 0)
-log_debug(Couldn't send message to plymouth);
-
+return r;
}
return 0;
}
@@ -384,9 +383,7 @@ static int client_progress_handler(sd_event_source *s, int fd, uint32_t revents,
else {
log_warning(Closing bad behaving fsck client connection at fd %d, client-fd);
client_free(client);
-r = manager_update_global_progress(m);
-if (r 0)
-log_warning_errno(r, Couldn't update global progress: %m);
+manager_update_global_progress(m);
}
return 0;
}
@@ -410,9 +407,7 @@ static int client_progress_handler(sd_event_source *s, int fd, uint32_t revents,
} else
log_error_errno(r, Unknown error while trying to read fsck data: %m);
-r = manager_update_global_progress(m);
-if (r 0)
-log_warning_errno(r, Couldn't update global progress: %m);
+manager_update_global_progress(m);
return 0;
}
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 4/5] fsckd: check if plymouth is running before attempting connection
From fd28f24d9eaa16737cbc8f33b8fe1a806dc291b1 Mon Sep 17 00:00:00 2001
From: Didier Roche didro...@ubuntu.com
Date: Tue, 10 Mar 2015 10:05:19 +0100
Subject: [PATCH 4/5] fsckd: check if plymouth is running before attempting
connection
---
src/fsckd/fsckd.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index f73d23b..52d69cd 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -231,6 +231,9 @@ static int manager_connect_plymouth(Manager *m) {
union sockaddr_union sa = PLYMOUTH_SOCKET;
int r;
+if (!plymouth_running())
+return 1;
+
/* try to connect or reconnect if sending a message */
if (m-plymouth_fd = 0)
return 0;
@@ -278,6 +281,9 @@ static int manager_send_plymouth_message(Manager *m, const char *message) {
r = manager_connect_plymouth(m);
if (r 0)
return r;
+/* 0 means that plymouth isn't running, do not send any message yet */
+else if (r 0)
+return 0;
if (!m-plymouth_cancel_sent) {
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes
---
TODO | 2 -
man/systemd.socket.xml | 7 ++-
src/core/service.c | 41 -
src/libsystemd/sd-resolve/test-resolve.c | 2 +-
src/shared/socket-util.c | 76 +++-
src/shared/socket-util.h | 4 +-
src/timesync/timesyncd-server.h | 2 +-
7 files changed, 106 insertions(+), 28 deletions(-)
diff --git a/TODO b/TODO
index ae32388..780084a 100644
--- a/TODO
+++ b/TODO
@@ -164,8 +164,6 @@ Features:
* as soon as we have kdbus, and sender timestamps, revisit coalescing multiple
parallel daemon reloads:
http://lists.freedesktop.org/archives/systemd-devel/2014-December/025862.html
-* set $REMOTE_IP (or $REMOTE_ADDR/$REMOTE_PORT) environment variable when
doing per-connection socket activation. use format introduced by xinetd or CGI
for this
-
* the install state probably shouldn't get confused by generated units, think
dbus1/kdbus compat!
* in systemctl list-unit-files: show the install value the presets would
suggest for a service in a third column
diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 3938345..6808179 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -357,7 +357,12 @@
daemons designed for usage with
citerefentryrefentrytitleinetd/refentrytitlemanvolnum8/manvolnum/citerefentry
to work unmodified with systemd socket
-activation./para/listitem
+activation./para
+
+paraFor IPv4 and IPv6 connections the varnameREMOTE_ADDR/varname
+environment variable will contain the remote IP, and
varnameREMOTE_PORT/varname
+will contain the remote port. This is the same as the format used by
CGI.
+For SOCK_RAW the port is the IP protocol./para/listitem
/varlistentry
varlistentry
diff --git a/src/core/service.c b/src/core/service.c
index cc4ea19..bcfce96 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1095,7 +1095,7 @@ static int service_spawn(
if (r 0)
goto fail;
-our_env = new0(char*, 4);
+our_env = new0(char*, 6);
if (!our_env) {
r = -ENOMEM;
goto fail;
@@ -1119,6 +1119,45 @@ static int service_spawn(
goto fail;
}
+if (UNIT_DEREF(s-accept_socket)) {
+union sockaddr_union sa;
+socklen_t salen = sizeof(sa);
+
+r = getpeername(s-socket_fd, sa.sa, salen);
+if (r 0) {
+r = -errno;
+goto fail;
+}
+
+if (IN_SET(sa.sa.sa_family, AF_INET, AF_INET6)) {
+_cleanup_free_ char *addr = NULL;
+char *t;
+int port;
+
+r = sockaddr_pretty(sa.sa, salen, true, false, addr);
+if (r 0)
+goto fail;
+
+t = strappend(REMOTE_ADDR=, addr);
+if (!t) {
+r = -ENOMEM;
+goto fail;
+}
+our_env[n_env++] = t;
+
+port = sockaddr_port(sa.sa);
+if (port 0) {
+r = port;
+goto fail;
+}
+
+if (asprintf((our_env + n_env++), REMOTE_PORT=%u,
port) 0) {
+r = -ENOMEM;
+goto fail;
+}
+}
+}
+
final_env = strv_env_merge(2, UNIT(s)-manager-environment, our_env,
NULL);
if (!final_env) {
r = -ENOMEM;
diff --git a/src/libsystemd/sd-resolve/test-resolve.c
b/src/libsystemd/sd-resolve/test-resolve.c
index 3187ce9..354a407 100644
--- a/src/libsystemd/sd-resolve/test-resolve.c
+++ b/src/libsystemd/sd-resolve/test-resolve.c
@@ -46,7 +46,7 @@ static int getaddrinfo_handler(sd_resolve_query *q, int ret,
const struct addrin
for (i = ai; i; i = i-ai_next) {
_cleanup_free_ char *addr = NULL;
-assert_se(sockaddr_pretty(i-ai_addr, i-ai_addrlen, false,
addr) == 0);
+assert_se(sockaddr_pretty(i-ai_addr, i-ai_addrlen, false,
true, addr) == 0);
puts(addr);
}
diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
index 74d90fa..0d87cb1 100644
--- a/src/shared/socket-util.c
+++ b/src/shared/socket-util.c
@@ -297,7 +297,7 @@ int socket_address_print(const SocketAddress *a, char
**ret) {
return 0;
}
-return sockaddr_pretty(a-sockaddr.sa, a-size, false, ret);
+return sockaddr_pretty(a-sockaddr.sa, a-size, false, true, ret);
}
bool
[systemd-devel] Specification for org.freedesktop.DisplayManager DBus API?
(Note 1: please CC me) (Note 2: I have CC-ed two contacts from gnome-shell, one from gdm, one from lightdm, and the system-logind ML) Hello, There are currently several ways to ask the DM to switch to the login screen: - gdm3 uses org.gnome.DisplayManager.LocalDisplayFactory CreateTransientDisplay - lightdm uses org.freedesktop.DisplayManager SwitchToGreeter - old gdm, and kdm seems to use a socket without DBus - ... I am not involved in those projects, but I recently needed to switch to LightDM to have guest sessions, while keeping gnome-shell ([debian-guest-sessions]). gnome-shell send a CreateTransientDisplay when Switch user, without any effect. I proposed a patch to gnome-shell [gnome-shell-SwitchToGreeter], which currently waiting for an implementation in gdm [gdm-SwitchToGreeter] and a written specification. This mail is to start a discussion about this DisplayManager specification. The current DBus interfaces used by lightDM seems to be a good starting point [lightDM-API], but some of its API may overlap with logind. Also, how can this work in the Wayland world? What is the way forward to have the same API in all DM? Refs: [debian-guest-sessions]: http://anonscm.debian.org/cgit/users/sathieu/guest-sessions.git/tree/ [gnome-shell-SwitchToGreeter]: https://bugzilla.gnome.org/show_bug.cgi?id=745940 [gdm-SwitchToGreeter]: https://bugzilla.gnome.org/show_bug.cgi?id=745938 [lightDM-API]: http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/vivid/lightdm/vivid/view/head:/src/lightdm.c#L784 Regards Mathieu Parent ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Real-time permission affected by root login on other tty
On Sun, Mar 8, 2015 at 11:27 PM, Lennart Poettering lenn...@poettering.net wrote: Most likely you have RT cgroup scheduling enabled in the kernel, and some unit uses CPUShares=, CPUAccounting=, CPUQuota*= in the unit file. If a unit does that this will move the unit's cgroup into the cpu cgroup controller, as well as all its parent and sibling cgroups (and thus units). I can only find a commented out CPUAccounting= in /etc/systemd/system.conf. No other configuration file mention the above parameters. I'm on a fresh install of Arch. When a process is in a cgroup in the cpu controller, and no RT budget is set for that cgroup, then RT is not available to it. This is very unfortunate. I'd love to assign an RT budget by default from systmed, but this isn't really doable, since there's no sane RT budget one could assign a cgroup given the current semantics of it (which require that all RT budgets of cgroups within another cgroup must sum up to less than 1/1...). Here's the output of /proc/self/cgroup when logging in as non-root user while root is logged in other vconsole or via ssh: 8:memory:/user.slice/user-1000.slice 7:blkio:/user.slice/user-1000.slice 6:cpuset:/ 5:cpu,cpuacct:/user.slice/user-1000.slice 4:devices:/user.slice/user-1000.slice 3:net_cls:/ 2:freezer:/ 1:name=systemd:/user.slice/user-1000.slice/session-c3.scope When logging in as non-root alone, or when downgrading to systemd-216: 8:memory:/ 7:blkio:/ 6:cpuset:/ 5:cpu,cpuacct:/ 4:devices:/ 3:net_cls:/ 2:freezer:/ 1:name=systemd:/user.slice/user-1000.slice/session-c1.scope -- Lars ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Why don't remote file systems wait for network-online.target?
2015-03-10 12:40 GMT+01:00 Martin Pitt martin.p...@ubuntu.com: Hello all, we got a report [1] that NFS fstab mounts (sometimes) aren't being mounted at boot as the network is still down: | mount[866]: mount.nfs: Network is unreachable | systemd[1]: mnt-server.mount mount process exited, code=exited status=32 | systemd[1]: Failed to mount /mnt/server. | systemd[1]: Dependency failed for Remote File Systems. | systemd[1]: Job remote-fs.target/start failed with result 'dependency'. | systemd[1]: Unit mnt-server.mount entered failed state. At the moment, neither network-fs-pre.target has no dependencies at all, and the fstab-generator-created .mount units only have Before=remote-fs.target and no other dependencies. Wouldn't it make sense to make network-fs-pre.target Wants/After=network-online to fix this? If you have/rely on NFS mounts, then you usually have some static/always working network connection via networkd/auto-connection in NM/ifupdown/etc, which all integrate into network-online.target. Could you share the relevant /etc/fstab line? -- Why is it that all of the instruments seeking intelligent life in the universe are pointed away from Earth? ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes
On Tue, Mar 10, 2015 at 01:21:27PM +0100, Ronny Chevalier wrote:
2015-03-10 12:41 GMT+01:00 Shawn Landden sh...@churchofgit.com:
---
TODO | 2 -
man/systemd.socket.xml | 7 ++-
src/core/service.c | 41 -
src/libsystemd/sd-resolve/test-resolve.c | 2 +-
src/shared/socket-util.c | 76
+++-
src/shared/socket-util.h | 4 +-
src/timesync/timesyncd-server.h | 2 +-
7 files changed, 106 insertions(+), 28 deletions(-)
Applied (with the fix for IN_SET).
Zbyszek
diff --git a/TODO b/TODO
index ae32388..780084a 100644
--- a/TODO
+++ b/TODO
@@ -164,8 +164,6 @@ Features:
* as soon as we have kdbus, and sender timestamps, revisit coalescing
multiple parallel daemon reloads:
http://lists.freedesktop.org/archives/systemd-devel/2014-December/025862.html
-* set $REMOTE_IP (or $REMOTE_ADDR/$REMOTE_PORT) environment variable when
doing per-connection socket activation. use format introduced by xinetd or
CGI for this
-
* the install state probably shouldn't get confused by generated units,
think dbus1/kdbus compat!
* in systemctl list-unit-files: show the install value the presets would
suggest for a service in a third column
diff --git a/man/systemd.socket.xml b/man/systemd.socket.xml
index 3938345..6808179 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -357,7 +357,12 @@
daemons designed for usage with
citerefentryrefentrytitleinetd/refentrytitlemanvolnum8/manvolnum/citerefentry
to work unmodified with systemd socket
-activation./para/listitem
+activation./para
+
+paraFor IPv4 and IPv6 connections the
varnameREMOTE_ADDR/varname
+environment variable will contain the remote IP, and
varnameREMOTE_PORT/varname
+will contain the remote port. This is the same as the format used
by CGI.
+For SOCK_RAW the port is the IP protocol./para/listitem
/varlistentry
varlistentry
diff --git a/src/core/service.c b/src/core/service.c
index cc4ea19..bcfce96 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1095,7 +1095,7 @@ static int service_spawn(
if (r 0)
goto fail;
-our_env = new0(char*, 4);
+our_env = new0(char*, 6);
if (!our_env) {
r = -ENOMEM;
goto fail;
@@ -1119,6 +1119,45 @@ static int service_spawn(
goto fail;
}
+if (UNIT_DEREF(s-accept_socket)) {
+union sockaddr_union sa;
+socklen_t salen = sizeof(sa);
+
+r = getpeername(s-socket_fd, sa.sa, salen);
+if (r 0) {
+r = -errno;
+goto fail;
+}
+
+if (IN_SET(sa.sa.sa_family, AF_INET, AF_INET6)) {
+_cleanup_free_ char *addr = NULL;
+char *t;
+int port;
+
+r = sockaddr_pretty(sa.sa, salen, true, false,
addr);
+if (r 0)
+goto fail;
+
+t = strappend(REMOTE_ADDR=, addr);
+if (!t) {
+r = -ENOMEM;
+goto fail;
+}
+our_env[n_env++] = t;
+
+port = sockaddr_port(sa.sa);
+if (port 0) {
+r = port;
+goto fail;
+}
+
+if (asprintf((our_env + n_env++),
REMOTE_PORT=%u, port) 0) {
+r = -ENOMEM;
+goto fail;
+}
+}
+}
+
final_env = strv_env_merge(2, UNIT(s)-manager-environment,
our_env, NULL);
if (!final_env) {
r = -ENOMEM;
diff --git a/src/libsystemd/sd-resolve/test-resolve.c
b/src/libsystemd/sd-resolve/test-resolve.c
index 3187ce9..354a407 100644
--- a/src/libsystemd/sd-resolve/test-resolve.c
+++ b/src/libsystemd/sd-resolve/test-resolve.c
@@ -46,7 +46,7 @@ static int getaddrinfo_handler(sd_resolve_query *q, int
ret, const struct addrin
for (i = ai; i; i = i-ai_next) {
_cleanup_free_ char *addr = NULL;
-assert_se(sockaddr_pretty(i-ai_addr, i-ai_addrlen,
false, addr) == 0);
+assert_se(sockaddr_pretty(i-ai_addr, i-ai_addrlen,
false, true, addr) == 0);
puts(addr);
}
diff --git
Re: [systemd-devel] Why don't remote file systems wait for network-online.target?
Hey Andrei, Andrei Borzenkov [2015-03-10 15:12 +0300]: Dependency on network-online.target is supposed to be implicitly added (oh, those implicit undocumented dependencies ...) src/core/mount.c:mount_add_default_dependencies() Ah, thanks for pointing out. Indeed, I was confused by them not being in the generated /run/systemd/generator/mnt-server.mount, but systemctl show indeed does show them. So it's all clear now, they do wait on network-online.target, this is just a no-op right now because NetworkManager-wait-online.service isn't activated. Thanks, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] fix strict aliasing issue in src/libsystemd-network/sd-dhcp-client.c
---
src/libsystemd-network/sd-dhcp-client.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/libsystemd-network/sd-dhcp-client.c
b/src/libsystemd-network/sd-dhcp-client.c
index 4224e01..a477ccc 100644
--- a/src/libsystemd-network/sd-dhcp-client.c
+++ b/src/libsystemd-network/sd-dhcp-client.c
@@ -1469,7 +1469,7 @@ static int client_receive_message_udp(sd_event_source *s,
int fd,
_cleanup_free_ DHCPMessage *message = NULL;
int buflen = 0, len, r;
const struct ether_addr zero_mac = { { 0, 0, 0, 0, 0, 0 } };
-const struct ether_addr *expected_chaddr = NULL;
+bool expect_chaddr;
uint8_t expected_hlen = 0;
assert(s);
@@ -1514,11 +1514,11 @@ static int client_receive_message_udp(sd_event_source
*s, int fd,
if (client-arp_type == ARPHRD_ETHER) {
expected_hlen = ETH_ALEN;
-expected_chaddr = (const struct ether_addr *)
client-mac_addr;
+expect_chaddr = true;
} else {
/* Non-ethernet links expect zero chaddr */
expected_hlen = 0;
- expected_chaddr = zero_mac;
+ expect_chaddr = false;
}
if (message-hlen != expected_hlen) {
@@ -1526,7 +1526,10 @@ static int client_receive_message_udp(sd_event_source
*s, int fd,
return 0;
}
-if (memcmp(message-chaddr[0], expected_chaddr, ETH_ALEN)) {
+if (memcmp(message-chaddr[0], expect_chaddr ?
+ (void *)client-mac_addr :
+ (void *)zero_mac,
+ETH_ALEN)) {
log_dhcp_client(client, received chaddr does not match
expected: ignoring);
return 0;
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] [PATCH v2] util: add rename_noreplace
On Tue, 10.03.15 17:34, Alban Crequy (alban.cre...@gmail.com) wrote:
-r = renameat2(AT_FDCWD, i-temp_path, AT_FDCWD,
i-final_path, RENAME_NOREPLACE);
+r = rename_noreplace(AT_FDCWD, i-temp_path, AT_FDCWD,
i-final_path);
if (r 0) {
r = log_error_errno(errno, Failed to move RAW file
into place: %m);
goto finish;
If rename_noreplace() would return -errno like all the other calls
we define, then this would become:
if (r 0) {
log_error_errno(r, Failed to move RAW file into place: %m);
goto finish;
}
Which is both shorter and more inline with the rest of our code...
-if (renameat2(AT_FDCWD, t, AT_FDCWD, to, replace ? 0 :
RENAME_NOREPLACE) 0) {
+if (replace) {
+r = renameat(AT_FDCWD, t, AT_FDCWD, to);
+} else {
+r = rename_noreplace(AT_FDCWD, t, AT_FDCWD, to);
+}
+if (r 0) {
Please, no {} for single-line if blocks. See CODING_STYLE.
+ret = unlinkat(olddirfd, oldpath, 0);
+if (ret 0)
+unlinkat(newdirfd, newpath, 0);
Recently we started prefixing calls like this where we knowingly
ignore the return value with casts to (void). This tells code checkers
like Coverity that we *knowingly* ignore the error condition
here. Hence:
(void) unlinkat(newdirfd, newpath, 0);
instead of:
unlinkat(newdirfd, newpath, 0);
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/5] fsckd: Fix some error return values
Le 10/03/2015 11:41, Lennart Poettering a écrit :
On Tue, 10.03.15 11:33, Didier Roche (didro...@ubuntu.com) wrote:
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 3fc923b..9393379 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -220,7 +220,7 @@ static int manager_connect_plymouth(Manager *m) {
goto fail;
}
-return 1;
+return 0;
Oh, well, this doesn't matter too much I guess, since we don't check
for the precise value, but just 0 or = 0...
Returning 1 in this case is actually a bit of a pattern I am trying to
adopt across the codebase: return 0 if all is OK but we didn't do
anything. Return 1 if all is OK and we actually did something. In
this case this distinction is of course entirely irrelevant, but it
was more of an automatism...
fail:
manager_disconnect_plymouth(m);
@@ -406,10 +406,8 @@ static int manager_new_connection_handler(sd_event_source
*s, int fd, uint32_t r
log_debug(New fsck client connected to fd: %d, new_client_fd);
c = new0(Client, 1);
-if (!c) {
-log_oom();
-return 0;
-}
+if (!c)
+return log_oom();
Well, I think logging and ignoring the OOM condition in this case is a
good idea. THink about the effect of returning an error up here: when
a handler callback returns an error sd-event will disable the event
source automatically. This means if fsckd hits an OOM once here, it
will become completely unresponsive, as it never processes incoming
connections anymore. However, if we simply log, close the connection,
and continue without propagating an error up, we are slightly more
robust: sure, the connection will fail, but subsequent connections
might not...
c-fd = new_client_fd;
new_client_fd = -1;
@@ -417,7 +415,7 @@ static int manager_new_connection_handler(sd_event_source
*s, int fd, uint32_t r
r = sd_event_add_io(m-event, c-event_source, c-fd, EPOLLIN,
client_progress_handler, c);
if (r 0) {
log_oom();
-return 0;
+return r;
Same here.
Thanks for the explanations, I dropped that patch. Note that I needed to
do a slight change to the paradigm in the replacement of patch 04_*
Didier
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] [PATCH v3] util: add rename_noreplace
From: Alban Crequy al...@endocode.com
renameat2() exists since Linux 3.15 but btrfs support for the flag
RENAME_NOREPLACE was added later.
This patch implements a fallback when renameat2() returns EINVAL.
EINVAL is the error returned when the filesystem does not support one of
the flags.
---
src/import/import-raw.c| 5 +++--
src/import/import-tar.c| 5 +++--
src/import/pull-raw.c | 4 ++--
src/import/pull-tar.c | 5 +++--
src/shared/copy.c | 13 ++---
src/shared/machine-image.c | 5 +++--
src/shared/machine-pool.c | 5 +++--
src/shared/util.c | 41 +
src/shared/util.h | 2 ++
9 files changed, 70 insertions(+), 15 deletions(-)
diff --git a/src/import/import-raw.c b/src/import/import-raw.c
index 15e5eb2..c53b0e4 100644
--- a/src/import/import-raw.c
+++ b/src/import/import-raw.c
@@ -245,8 +245,9 @@ static int raw_import_finish(RawImport *i) {
(void) rm_rf_dangerous(i-final_path, false, true, false);
}
-if (renameat2(AT_FDCWD, i-temp_path, AT_FDCWD, i-final_path,
RENAME_NOREPLACE) 0)
-return log_error_errno(errno, Failed to move image into
place: %m);
+r = rename_noreplace(AT_FDCWD, i-temp_path, AT_FDCWD, i-final_path);
+if (r 0)
+return log_error_errno(r, Failed to move image into place:
%m);
free(i-temp_path);
i-temp_path = NULL;
diff --git a/src/import/import-tar.c b/src/import/import-tar.c
index d5b6dad..65262d8 100644
--- a/src/import/import-tar.c
+++ b/src/import/import-tar.c
@@ -201,8 +201,9 @@ static int tar_import_finish(TarImport *i) {
(void) rm_rf_dangerous(i-final_path, false, true, false);
}
-if (renameat2(AT_FDCWD, i-temp_path, AT_FDCWD, i-final_path,
RENAME_NOREPLACE) 0)
-return log_error_errno(errno, Failed to move image into
place: %m);
+r = rename_noreplace(AT_FDCWD, i-temp_path, AT_FDCWD, i-final_path);
+if (r 0)
+return log_error_errno(r, Failed to move image into place:
%m);
free(i-temp_path);
i-temp_path = NULL;
diff --git a/src/import/pull-raw.c b/src/import/pull-raw.c
index d1d77d5..988fc59 100644
--- a/src/import/pull-raw.c
+++ b/src/import/pull-raw.c
@@ -384,9 +384,9 @@ static void raw_pull_job_on_finished(PullJob *j) {
if (r 0)
goto finish;
-r = renameat2(AT_FDCWD, i-temp_path, AT_FDCWD, i-final_path,
RENAME_NOREPLACE);
+r = rename_noreplace(AT_FDCWD, i-temp_path, AT_FDCWD,
i-final_path);
if (r 0) {
-r = log_error_errno(errno, Failed to move RAW file
into place: %m);
+r = log_error_errno(r, Failed to move RAW file into
place: %m);
goto finish;
}
diff --git a/src/import/pull-tar.c b/src/import/pull-tar.c
index 504642f..84ad571 100644
--- a/src/import/pull-tar.c
+++ b/src/import/pull-tar.c
@@ -281,8 +281,9 @@ static void tar_pull_job_on_finished(PullJob *j) {
if (r 0)
goto finish;
-if (renameat2(AT_FDCWD, i-temp_path, AT_FDCWD, i-final_path,
RENAME_NOREPLACE) 0) {
-r = log_error_errno(errno, Failed to rename to final
image name: %m);
+r = rename_noreplace(AT_FDCWD, i-temp_path, AT_FDCWD,
i-final_path);
+if (r 0) {
+r = log_error_errno(r, Failed to rename to final
image name: %m);
goto finish;
}
diff --git a/src/shared/copy.c b/src/shared/copy.c
index 0239a58..edbbb3c 100644
--- a/src/shared/copy.c
+++ b/src/shared/copy.c
@@ -404,9 +404,16 @@ int copy_file_atomic(const char *from, const char *to,
mode_t mode, bool replace
if (r 0)
return r;
-if (renameat2(AT_FDCWD, t, AT_FDCWD, to, replace ? 0 :
RENAME_NOREPLACE) 0) {
-unlink_noerrno(t);
-return -errno;
+if (replace) {
+r = renameat(AT_FDCWD, t, AT_FDCWD, to);
+if (r 0)
+r = -errno;
+} else {
+r = rename_noreplace(AT_FDCWD, t, AT_FDCWD, to);
+}
+if (r 0) {
+(void) unlink_noerrno(t);
+return r;
}
return 0;
diff --git a/src/shared/machine-image.c b/src/shared/machine-image.c
index c6d2850..440a569 100644
--- a/src/shared/machine-image.c
+++ b/src/shared/machine-image.c
@@ -440,8 +440,9 @@ int image_rename(Image *i, const char *new_name) {
if (!nn)
return -ENOMEM;
-if (renameat2(AT_FDCWD, i-path, AT_FDCWD, new_path, RENAME_NOREPLACE)
0)
-return -errno;
+r = rename_noreplace(AT_FDCWD, i-path, AT_FDCWD, new_path);
+if (r 0)
+return r;
Re: [systemd-devel] [PATCH] path_is_mount_point: handle false positive on some fs
On Tue, 10.03.15 18:01, Didier Roche (didro...@ubuntu.com) wrote: The context is bug https://bugs.launchpad.net/ubuntu/+source/systemd/+bug/1411140, where systemd-machine-id-commit unit is entering in failed state (the binary handles gracefully the fact that it can't unmount the file) on a default ubuntu live system (which is a squashfs/overlayfs system). Hmm, it's not clear to me by that bug what precisely fails... Any idea? Maybe it would be better to just handle that failure gracefully... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH V3] path-util.c: fix path_is_mount_point() for symlinks
On Tue, 10.03.15 17:29, Lennart Poettering (lenn...@poettering.net) wrote:
On Tue, 10.03.15 17:26, Lennart Poettering (lenn...@poettering.net) wrote:
On Fri, 20.02.15 13:25, har...@redhat.com (har...@redhat.com) wrote:
Sorry for the late review!
--- a/src/shared/path-util.c
+++ b/src/shared/path-util.c
@@ -456,9 +456,9 @@ int path_is_mount_point(const char *t, bool
allow_symlink) {
union file_handle_union h = FILE_HANDLE_INIT;
int mount_id = -1, mount_id_parent = -1;
-_cleanup_free_ char *parent = NULL;
struct stat a, b;
int r;
+_cleanup_close_ int fd = -1;
bool nosupp = false;
/* We are not actually interested in the file handles, but
@@ -468,7 +468,15 @@ int path_is_mount_point(const char *t, bool
allow_symlink) {
if (path_equal(t, /))
return 1;
-r = name_to_handle_at(AT_FDCWD, t, h.handle, mount_id,
allow_symlink ? AT_SYMLINK_FOLLOW : 0);
+fd = openat(AT_FDCWD, t,
O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC|(allow_symlink ? 0 : O_PATH));
+if (fd 0) {
+if (errno == ENOENT)
+return 0;
We eat up this error? Why not pass the -ENOENT up?
Hmm, as I see know the existing code also eats up ENOENT... Not
entirely sure why though.
But patch looks good then, please commit!
I commited this now. Thanks!
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 4/5] fsckd: check if plymouth is running before attempting connection
Le 10/03/2015 11:44, Lennart Poettering a écrit :
On Tue, 10.03.15 11:34, Didier Roche (didro...@ubuntu.com) wrote:
I think it would make more sense to return 0 when ply isn't running,
and 1 if it is, no?
Did this in the attached patch. Due to this, I needed then to return 1
even if we did not reconnect to plymouth but was already connected,
which slightly break the paradigm of return 0 if we did nothing, and 1
if the action changed something and is successful.
Is that ok?
Cheers,
Didier
From 73ce3f737e1211a7d182553cbc55727a04a18d4c Mon Sep 17 00:00:00 2001
From: Didier Roche didro...@ubuntu.com
Date: Tue, 10 Mar 2015 10:05:19 +0100
Subject: [PATCH 2/2] fsckd: check if plymouth is running before attempting
connection
---
src/fsckd/fsckd.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index f23f272..70abb07 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -231,9 +231,12 @@ static int manager_connect_plymouth(Manager *m) {
union sockaddr_union sa = PLYMOUTH_SOCKET;
int r;
+if (!plymouth_running())
+return 0;
+
/* try to connect or reconnect if sending a message */
if (m-plymouth_fd = 0)
-return 0;
+return 1;
m-plymouth_fd = socket(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
if (m-plymouth_fd 0)
@@ -278,6 +281,9 @@ static int manager_send_plymouth_message(Manager *m, const char *message) {
r = manager_connect_plymouth(m);
if (r 0)
return r;
+/* 0 means that plymouth isn't running, do not send any message yet */
+else if (r == 0)
+return 0;
if (!m-plymouth_cancel_sent) {
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] 9 commits - configure.ac .gitignore Makefile.am Makefile-man.am man/systemd-fsckd.service.xml man/systemd-f...@.service.xml po/de.po po/el.po po/fr.po po/hu.po po
On Tue, Mar 10, 2015 at 11:49:05AM +0100, Lennart Poettering wrote: On Tue, 10.03.15 08:21, Martin Pitt (martin.p...@ubuntu.com) wrote: Please, be more careful with complex code like this, this needs more rounds of review before something like this can be merged... Okay. It went through 4 rounds of review on the ML and several folks commented, there were no more outstanding/unaddressed review comments, and it seemed to me that we had consensus on IRC to land this now. Procedurally, what should I/we do next time before landing such large patches? Wait for an explicit ack on the ML from you? Well, if it's complex code like this, get Zbigniew or Kay or Tom or so to say it's OK... I acked the patches, and actually asked Martin to push them. I overlooked the issues — sorry. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] How to factory reset?
Hi Lennart, thanks for taking the time to answer! It is highly appreciated. On Tue, Mar 10, 2015 at 5:01 PM, Lennart Poettering lenn...@poettering.net wrote: So you want not just factory reset, but actually a stateless system, where every single boot is basically a factory reset? Yes, but I do have a state that I want to be applied by default at all times. My recommendation would be to set this up within the initrd: mount a tmpfs as /, then mount the physical /usr into it and transition to the host OS. That is a great idea! I was so focused on having a tmpfs on /etc that I did not even think about that:-/ I would like to keep e.g. logs between reboots, so maybe I can just have /var mounted from somewhere else. There's currently no explicit support for this to make this easy from the initrd. There are flags to set the root filesystem and equivalent flags for the usr one, so that should not be too hard to do. (It is available in nspawn though with the --volatile=) switch. But it's on the todo list to add that, so that what I describe above is easily available. We also should provide a scheme that one can flush /etc explicitly ones for a factory reset, via a kernel cmdline option. Please do not do that. Even if all filesystems are encrypted you could factory-reset random computers you have access to, simply by editing the bootloader configuration file usually found in the poorly protected EFI partition! Better have a unit that deletes /etc before the system is shut down. That way you at least need to have root access to the running machine to trigger a factory reset. That keeps at least people with encrypted drives save:-) That said, I figure it should suffice to add entries for / and /usr to the fstab and get the initrd to make use of it. I do not have no stinking fstab:-) My boxes are built up from bits and pieces of configuration. The mount units make are just so much nicer to handle than having several pieces edit the same file:-) Yes, dbus is currently not compatible with stateless bootups. PAM is neither. For PAM we ship a tmpfiles snippet to work around this, for dbus we don't though, since kdbus kinda makes the problem go away... I do have state for PAM, dbus and everything else, I was just not able to get it into place in time for those services to start properly. There are quite a few services with a condition of writable /etc, and with the setup I attempted /etc was writable, even before I managed to put my tmpfs into place and extract my configuration there. nspawn's --volatile= switch is how i did most of my own testing... I did not use --volatile since I need to boot from an image that is considerably different from the one on my development machine. So I will have to set up a tmpfs manually and mount a usr there and then just use normal nspawn with --directory. Having --volatile=/path/to/usr/directory would be nice to have for the experiments I do right now. I guess that is not so very common that it makes sense to consider sending in a patch for that. Well, nspawn isn't. But systemd will, if it finds /etc empty. It will create a machine ID, and apply presets and stuff... I *have* a machine ID and everything. Can I get that information into place somehow *before* systemd creates all of that? Thanks again for your reply. You did provide some food for further experiments: 1) Extract my etc-tarball to /usr/share/factory/etc and remove /etc from the root-image. Keep the etc.mount unit. 2) Try a empty root image with my etc-tarball extracted to /usr/share/factory/etc and no etc.mount. 3) Add var.mount to the setup of 2) to keep the logs. With a bit of luck that should stop all the units that have a condition on a writable /etc from getting started. I'll get back with results:-) Best Regards, Tobias ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] How to factory reset?
On Tue, 10.03.15 18:13, Tobias Hunger (tobias.hun...@gmail.com) wrote: So you want not just factory reset, but actually a stateless system, where every single boot is basically a factory reset? Yes, but I do have a state that I want to be applied by default at all times. Well, you want the factory defaults to be applied when the machine comes up, right? My recommendation would be to set this up within the initrd: mount a tmpfs as /, then mount the physical /usr into it and transition to the host OS. That is a great idea! I was so focused on having a tmpfs on /etc that I did not even think about that:-/ I would like to keep e.g. logs between reboots, so maybe I can just have /var mounted from somewhere else. Hmm, currently we focussed on two models: a) fully volatile, meaning / as tmpfs, with /usr mounted from physical media b) with only volatile state, but persitent configuration, meaning / mounted from physical media, and /var as tmpfs. Your model appears to be different from that. You actually want /var from from physical media, but /etc from tmpfs? That would be kinda the opposite of b)... Do you actually want all of /var mounted of physical media? If you are interested in just logging, maybe just adding a normal mount for /var/log/ should suffice, leaving the rest of /var on tmpfs? (It is available in nspawn though with the --volatile=) switch. But it's on the todo list to add that, so that what I describe above is easily available. We also should provide a scheme that one can flush /etc explicitly ones for a factory reset, via a kernel cmdline option. Please do not do that. Even if all filesystems are encrypted you could factory-reset random computers you have access to, simply by editing the bootloader configuration file usually found in the poorly protected EFI partition! Well, if you have access to the kernel cmdline you can do whatever you want. init=/bin/sh is infinitely more powerful than just being able to flush out /etc... Better have a unit that deletes /etc before the system is shut down. That way you at least need to have root access to the running machine to trigger a factory reset. That keeps at least people with encrypted drives save:-) Well, factory resets are supposed to be something that gets you back into a defined state if you fucked up your system. In such a case it might not be possible to boot up anymore to reset the state... Hence having this on the kernel cmdline is kinda a necessity to make this useful in real-life... Having --volatile=/path/to/usr/directory would be nice to have for the experiments I do right now. I guess that is not so very common that it makes sense to consider sending in a patch for that. Hmm, what precisely are you suggesting this would do? Well, nspawn isn't. But systemd will, if it finds /etc empty. It will create a machine ID, and apply presets and stuff... I *have* a machine ID and everything. Can I get that information into place somehow *before* systemd creates all of that? presets and machined ID are applied by PID 1, before it begins with starting any units, hence *really* early on. Note though that actually /etc/machine-id is used as flag for is /etc empty. If the file exists it is assumed that /etc is provisioned properly. If it is missing PID 1 will generate the machiend id and apply presets. Note though that some services like ldconfig.service also want to write to /etc, to generate some files, if they are missing... If you want to do something before that you have to order those units explicitly before each one of them. Note though that much like /usr, /etc is something that we assume is premounted when systemd is started, and where we do not support mounting it after systemd began its work. I mean, /etc is usually where moutns are configured, but if youw ant to mount someting on /etc, then how is that to be found? Hence, if you want /etc to be volatile, better do that in the initrd... Thanks again for your reply. You did provide some food for further experiments: 1) Extract my etc-tarball to /usr/share/factory/etc and remove /etc from the root-image. Keep the etc.mount unit. This will not work. Please do not work with a an etc.mount unit. Instead do the stuff in the initrd... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/5] fsckd: Don't use strjoina on gettext() call
Le 10/03/2015 11:36, Lennart Poettering a écrit :
On Tue, 10.03.15 11:32, Didier Roche (didro...@ubuntu.com) wrote:
static int manager_send_plymouth_message(Manager *m, const char *message) {
-const char *plymouth_cancel_message = NULL;
+_cleanup_free_ const char *plymouth_cancel_message = NULL;
int r;
r = manager_connect_plymouth(m);
@@ -258,7 +258,7 @@ static int manager_send_plymouth_message(Manager *m, const
char *message) {
m-plymouth_cancel_sent = true;
-plymouth_cancel_message = strjoina(fsckd-cancel-msg:, _(Press Ctrl+C to cancel all filesystem checks in progress));
+plymouth_cancel_message = strjoin(fsckd-cancel-msg:, _(Press
Ctrl+C to cancel all filesystem checks in progress), NULL);
Well, there's an OOM check missing now, as strjoin() can fail...
That said, I think using strjoina is actually a good idea here, the
string is not unbounded. Hence I think it would be a good idea to
first assign the result of _() to a const char* variable in one step,
and then concat it in a second...
Makes sense. Here is the suggested change.
Didier
PS: just posting a couple of patches from your suggestion, I'll work on
the bigger discussion of passing the fsck pipe to systemd-fsckd later in
the week.
From ab84557e5840ff0a606b4b6f694b655b85bdcd45 Mon Sep 17 00:00:00 2001
From: Didier Roche didro...@ubuntu.com
Date: Tue, 10 Mar 2015 08:58:23 +0100
Subject: [PATCH 1/2] fsckd: Don't use strjoina on gettext() call
---
src/fsckd/fsckd.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 3c587a3..f23f272 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -272,7 +272,7 @@ static int plymouth_send_message(int plymouth_fd, const char *message, bool upda
}
static int manager_send_plymouth_message(Manager *m, const char *message) {
-const char *plymouth_cancel_message = NULL;
+_cleanup_free_ const char *plymouth_cancel_message = NULL, *l10n_cancel_message = NULL;
int r;
r = manager_connect_plymouth(m);
@@ -288,7 +288,8 @@ static int manager_send_plymouth_message(Manager *m, const char *message) {
m-plymouth_cancel_sent = true;
-plymouth_cancel_message = strjoina(fsckd-cancel-msg:, _(Press Ctrl+C to cancel all filesystem checks in progress));
+l10n_cancel_message = _(Press Ctrl+C to cancel all filesystem checks in progress);
+plymouth_cancel_message = strjoina(fsckd-cancel-msg:, l10n_cancel_message);
r = plymouth_send_message(m-plymouth_fd, plymouth_cancel_message, false);
if (r 0)
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] [PATCH v2] util: add rename_noreplace
On Tue, Mar 10, 2015 at 05:34:07PM +0100, Alban Crequy wrote:
From: Alban Crequy al...@endocode.com
renameat2() exists since Linux 3.15 but btrfs support for the flag
RENAME_NOREPLACE was added later.
This patch implements a fallback when renameat2() returns EINVAL.
EINVAL is the error returned when the filesystem does not support one of
the flags.
---
src/import/import-raw.c| 2 +-
src/import/import-tar.c| 2 +-
src/import/pull-raw.c | 2 +-
src/import/pull-tar.c | 2 +-
src/shared/copy.c | 7 ++-
src/shared/machine-image.c | 2 +-
src/shared/machine-pool.c | 2 +-
src/shared/util.c | 32
src/shared/util.h | 2 ++
9 files changed, 46 insertions(+), 7 deletions(-)
diff --git a/src/import/import-raw.c b/src/import/import-raw.c
index 15e5eb2..e354023 100644
--- a/src/import/import-raw.c
+++ b/src/import/import-raw.c
@@ -245,7 +245,7 @@ static int raw_import_finish(RawImport *i) {
(void) rm_rf_dangerous(i-final_path, false, true, false);
}
-if (renameat2(AT_FDCWD, i-temp_path, AT_FDCWD, i-final_path,
RENAME_NOREPLACE) 0)
+if (rename_noreplace(AT_FDCWD, i-temp_path, AT_FDCWD,
i-final_path) 0)
return log_error_errno(errno, Failed to move image into
place: %m);
Change rename_noreplace to return -errno on error, 0 on success, and
then this part should look like this:
r = rename_noreplace(AT_FDCWD, i-temp_path, AT_FDCWD, i-final_path);
if (r 0)
return log_error_errno(r, Failed to move image into place: %m);
(Similarly for other places of course).
Zbyszek
free(i-temp_path);
diff --git a/src/import/import-tar.c b/src/import/import-tar.c
index d5b6dad..dcbe721 100644
--- a/src/import/import-tar.c
+++ b/src/import/import-tar.c
@@ -201,7 +201,7 @@ static int tar_import_finish(TarImport *i) {
(void) rm_rf_dangerous(i-final_path, false, true, false);
}
-if (renameat2(AT_FDCWD, i-temp_path, AT_FDCWD, i-final_path,
RENAME_NOREPLACE) 0)
+if (rename_noreplace(AT_FDCWD, i-temp_path, AT_FDCWD,
i-final_path) 0)
return log_error_errno(errno, Failed to move image into
place: %m);
free(i-temp_path);
diff --git a/src/import/pull-raw.c b/src/import/pull-raw.c
index d1d77d5..d4b17e7 100644
--- a/src/import/pull-raw.c
+++ b/src/import/pull-raw.c
@@ -384,7 +384,7 @@ static void raw_pull_job_on_finished(PullJob *j) {
if (r 0)
goto finish;
-r = renameat2(AT_FDCWD, i-temp_path, AT_FDCWD,
i-final_path, RENAME_NOREPLACE);
+r = rename_noreplace(AT_FDCWD, i-temp_path, AT_FDCWD,
i-final_path);
if (r 0) {
r = log_error_errno(errno, Failed to move RAW file
into place: %m);
goto finish;
diff --git a/src/import/pull-tar.c b/src/import/pull-tar.c
index 504642f..69df7af 100644
--- a/src/import/pull-tar.c
+++ b/src/import/pull-tar.c
@@ -281,7 +281,7 @@ static void tar_pull_job_on_finished(PullJob *j) {
if (r 0)
goto finish;
-if (renameat2(AT_FDCWD, i-temp_path, AT_FDCWD,
i-final_path, RENAME_NOREPLACE) 0) {
+if (rename_noreplace(AT_FDCWD, i-temp_path, AT_FDCWD,
i-final_path) 0) {
r = log_error_errno(errno, Failed to rename to
final image name: %m);
goto finish;
}
diff --git a/src/shared/copy.c b/src/shared/copy.c
index 0239a58..e578d03 100644
--- a/src/shared/copy.c
+++ b/src/shared/copy.c
@@ -404,7 +404,12 @@ int copy_file_atomic(const char *from, const char *to,
mode_t mode, bool replace
if (r 0)
return r;
-if (renameat2(AT_FDCWD, t, AT_FDCWD, to, replace ? 0 :
RENAME_NOREPLACE) 0) {
+if (replace) {
+r = renameat(AT_FDCWD, t, AT_FDCWD, to);
+} else {
+r = rename_noreplace(AT_FDCWD, t, AT_FDCWD, to);
+}
+if (r 0) {
unlink_noerrno(t);
return -errno;
}
diff --git a/src/shared/machine-image.c b/src/shared/machine-image.c
index c6d2850..ad5bff4 100644
--- a/src/shared/machine-image.c
+++ b/src/shared/machine-image.c
@@ -440,7 +440,7 @@ int image_rename(Image *i, const char *new_name) {
if (!nn)
return -ENOMEM;
-if (renameat2(AT_FDCWD, i-path, AT_FDCWD, new_path,
RENAME_NOREPLACE) 0)
+if (rename_noreplace(AT_FDCWD, i-path, AT_FDCWD, new_path) 0)
return -errno;
/* Restore the immutable bit, if it was set before */
diff --git a/src/shared/machine-pool.c b/src/shared/machine-pool.c
index e7671a3..7e902a3 100644
--- a/src/shared/machine-pool.c
+++ b/src/shared/machine-pool.c
Re: [systemd-devel] [PATCH] [PATCH v3] util: add rename_noreplace
On Tue, 10.03.15 18:15, Alban Crequy (alban.cre...@gmail.com) wrote: From: Alban Crequy al...@endocode.com renameat2() exists since Linux 3.15 but btrfs support for the flag RENAME_NOREPLACE was added later. This patch implements a fallback when renameat2() returns EINVAL. EINVAL is the error returned when the filesystem does not support one of the flags. Thanks! Applied. (With some minor fixes) Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] fix strict aliasing violations in src/udev/udev-builtin-usb_id.c
also switch to inttypes.h
---
src/udev/udev-builtin-usb_id.c | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/src/udev/udev-builtin-usb_id.c b/src/udev/udev-builtin-usb_id.c
index ab0d96e..b42b32e 100644
--- a/src/udev/udev-builtin-usb_id.c
+++ b/src/udev/udev-builtin-usb_id.c
@@ -28,6 +28,7 @@
#include ctype.h
#include fcntl.h
#include errno.h
+#include inttypes.h
#include udev.h
@@ -153,15 +154,15 @@ static int dev_if_packed_info(struct udev_device *dev,
char *ifs_str, size_t len
int pos = 0;
unsigned strpos = 0;
struct usb_interface_descriptor {
-u_int8_tbLength;
-u_int8_tbDescriptorType;
-u_int8_tbInterfaceNumber;
-u_int8_tbAlternateSetting;
-u_int8_tbNumEndpoints;
-u_int8_tbInterfaceClass;
-u_int8_tbInterfaceSubClass;
-u_int8_tbInterfaceProtocol;
-u_int8_tiInterface;
+uint8_tbLength;
+uint8_tbDescriptorType;
+uint8_tbInterfaceNumber;
+uint8_tbAlternateSetting;
+uint8_tbNumEndpoints;
+uint8_tbInterfaceClass;
+uint8_tbInterfaceSubClass;
+uint8_tbInterfaceProtocol;
+uint8_tiInterface;
} __attribute__((packed));
if (asprintf(filename, %s/descriptors,
udev_device_get_syspath(dev)) 0)
@@ -179,21 +180,20 @@ static int dev_if_packed_info(struct udev_device *dev,
char *ifs_str, size_t len
ifs_str[0] = '\0';
while (pos size strpos+7 len-2) {
-struct usb_interface_descriptor *desc;
+char *desc = buf[pos];
char if_str[8];
-desc = (struct usb_interface_descriptor *) buf[pos];
-if (desc-bLength 3)
+if (desc[offsetof(struct usb_interface_descriptor, bLength)]
3)
break;
-pos += desc-bLength;
+pos += desc[offsetof(struct usb_interface_descriptor,
bLength)];
-if (desc-bDescriptorType != USB_DT_INTERFACE)
+if (desc[offsetof(struct usb_interface_descriptor,
bDescriptorType)] != USB_DT_INTERFACE)
continue;
if (snprintf(if_str, 8, :%02x%02x%02x,
- desc-bInterfaceClass,
- desc-bInterfaceSubClass,
- desc-bInterfaceProtocol) != 7)
+ desc[offsetof(struct usb_interface_descriptor,
bInterfaceClass)],
+ desc[offsetof(struct usb_interface_descriptor,
bInterfaceSubClass)],
+ desc[offsetof(struct usb_interface_descriptor,
bInterfaceProtocol)]) != 7)
continue;
if (strstr(ifs_str, if_str) != NULL)
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] build: generate pkg-config files during configure
Generate pkg-config files during configure as God (Havoc) intended. This fixes all of systemd's pkg-config files when cross-compiling (and possibly other use cases). (Note: I might've missed some things to tidy up in Makefile.am, but not 100% sure.) Signed-off-by: Jeff Waugh j...@bethesignal.org --- Makefile.am | 3 --- configure.ac | 10 ++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/Makefile.am b/Makefile.am index 3539e03..d2ebc81 100644 --- a/Makefile.am +++ b/Makefile.am @@ -6442,9 +6442,6 @@ man/%: man/%.in sysctl.d/%: sysctl.d/%.in $(SED_PROCESS) -%.pc: %.pc.in - $(SED_PROCESS) - %.conf: %.conf.in $(SED_PROCESS) diff --git a/configure.ac b/configure.ac index 29111f5..21b8008 100644 --- a/configure.ac +++ b/configure.ac @@ -1510,6 +1510,16 @@ AC_CONFIG_FILES([ docs/libudev/version.xml docs/gudev/Makefile docs/gudev/version.xml +src/libudev/libudev.pc +src/compat-libs/libsystemd-id128.pc +src/compat-libs/libsystemd-daemon.pc +src/compat-libs/libsystemd-login.pc +src/compat-libs/libsystemd-journal.pc +src/libsystemd/sd-id128/libsystemd-id128.pc +src/libsystemd/libsystemd.pc +src/udev/udev.pc +src/core/systemd.pc +src/gudev/gudev-1.0.pc ]) AC_OUTPUT -- 1.9.1 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] fix compiler warning
warning: pointer/integer type mismatch in conditional expression
---
src/shared/socket-util.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/shared/socket-util.c b/src/shared/socket-util.c
index 5820279..73e1177 100644
--- a/src/shared/socket-util.c
+++ b/src/shared/socket-util.c
@@ -475,8 +475,8 @@ int sockaddr_port(const struct sockaddr *_sa) {
return -EAFNOSUPPORT;
return ntohs(sa-sa.sa_family == AF_INET6 ?
- sa-in6.sin6_port :
- sa-in.sin_port);
+ (uint16_t)sa-in6.sin6_port :
+ (uint16_t)sa-in.sin_port);
}
int sockaddr_pretty(const struct sockaddr *_sa, socklen_t salen, bool
translate_ipv6, bool include_port, char **ret) {
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] fix strict aliasing issues in src/udev/udev-ctrl.c
it is ironic that
The only purpose of this structure is to cast the structure pointer
passed in addr in order to avoid compiler warnings. See EXAMPLE below.
from bind(2)
---
src/udev/udev-ctrl.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/src/udev/udev-ctrl.c b/src/udev/udev-ctrl.c
index c0c5981..61d3c5b 100644
--- a/src/udev/udev-ctrl.c
+++ b/src/udev/udev-ctrl.c
@@ -18,6 +18,7 @@
#include sys/socket.h
#include sys/un.h
+#include socket-util.h
#include udev.h
/* wire protocol magic must match */
@@ -55,7 +56,7 @@ struct udev_ctrl {
int refcount;
struct udev *udev;
int sock;
-struct sockaddr_un saddr;
+union sockaddr_union saddr;
socklen_t addrlen;
bool bound;
bool cleanup_socket;
@@ -94,9 +95,9 @@ struct udev_ctrl *udev_ctrl_new_from_fd(struct udev *udev,
int fd) {
if (r 0)
log_warning_errno(errno, could not set SO_PASSCRED: %m);
-uctrl-saddr.sun_family = AF_LOCAL;
-strscpy(uctrl-saddr.sun_path, sizeof(uctrl-saddr.sun_path),
/run/udev/control);
-uctrl-addrlen = offsetof(struct sockaddr_un, sun_path) +
strlen(uctrl-saddr.sun_path);
+uctrl-saddr.un.sun_family = AF_LOCAL;
+strscpy(uctrl-saddr.un.sun_path, sizeof(uctrl-saddr.un.sun_path),
/run/udev/control);
+uctrl-addrlen = offsetof(struct sockaddr_un, sun_path) +
strlen(uctrl-saddr.un.sun_path);
return uctrl;
}
@@ -108,10 +109,10 @@ int udev_ctrl_enable_receiving(struct udev_ctrl *uctrl) {
int err;
if (!uctrl-bound) {
-err = bind(uctrl-sock, (struct sockaddr *)uctrl-saddr,
uctrl-addrlen);
+err = bind(uctrl-sock, uctrl-saddr.sa, uctrl-addrlen);
if (err 0 errno == EADDRINUSE) {
-unlink(uctrl-saddr.sun_path);
-err = bind(uctrl-sock, (struct sockaddr
*)uctrl-saddr, uctrl-addrlen);
+unlink(uctrl-saddr.un.sun_path);
+err = bind(uctrl-sock, uctrl-saddr.sa,
uctrl-addrlen);
}
if (err 0) {
@@ -160,7 +161,7 @@ int udev_ctrl_cleanup(struct udev_ctrl *uctrl) {
if (uctrl == NULL)
return 0;
if (uctrl-cleanup_socket)
-unlink(uctrl-saddr.sun_path);
+unlink(uctrl-saddr.un.sun_path);
return 0;
}
@@ -249,7 +250,7 @@ static int ctrl_send(struct udev_ctrl *uctrl, enum
udev_ctrl_msg_type type, int
ctrl_msg_wire.intval = intval;
if (!uctrl-connected) {
-if (connect(uctrl-sock, (struct sockaddr *)uctrl-saddr,
uctrl-addrlen) 0) {
+if (connect(uctrl-sock, uctrl-saddr.sa, uctrl-addrlen)
0) {
err = -errno;
goto out;
}
--
2.2.1.209.g41e5f3a
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] sd_journal_add_match if not using the form of FIELD=value
On Mon, 09.03.15 07:44, Mantas Mikulėnas (graw...@gmail.com) wrote: On Mon, Mar 9, 2015 at 12:53 AM, Lennart Poettering lenn...@poettering.net wrote: On Fri, 06.03.15 21:28, Chris Morgan (chmor...@gmail.com) wrote: http://www.freedesktop.org/software/systemd/man/sd_journal_add_match.html is pretty clear that the matches are in the form of 'FIELD=value' but it doesn't mention the why. What if I've written a field like FIELD, can I then match on it as FIELD? Hmm, not sure I understand what you mean? The journal stores key/value pairs, on display and when parsing we denote them in the form of an uppercase fied name, followed by a =, followed by any kind of data. Hence, just FIELD is not something the journald would or could store. If you try to pass this to journald for it to write, it would drop this, because it's malformed and not a key/value pair. I think the idea was just to *match* all entries that have a given key *present* but with any value? The database does not make look-ups like this efficient, it's not indexed for that. You can build this by combining sd_journal_query_unique() and then use the field/value pairs this fine to find the entries. But this is not efficient. 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] logind: lid switch action not executed on small open/close delays
On 03/06/2015 02:47 PM, David Herrmann wrote: Hi On Wed, Jan 28, 2015 at 8:31 PM, Lennart Poettering lenn...@poettering.net wrote: On Sat, 17.01.15 18:36, David Herrmann (dh.herrm...@gmail.com) wrote: I think it's reasonable to allow setting the base-timeout in /etc/systemd/logind.conf, but I want to know Lennart's opinion first. Preferably, this timeout was a kernel-timeout on the bus-probe itself. But we don't have anything like this, so we need the excessive timeouts in user-space as we cannot know whether we're just scheduled late or whether the bus-probe really takes that long... Meh... My system takes 5s to boot, so I could easily set those timeouts to 10s and everything would be fine. And I also don't care for hotplug changes while the system is off/suspended, hence, I could even disable the timeout and everything would just work. Not really sure how to proceed... I think making the timeout configurable in logind.conf would be OK. Done. http://cgit.freedesktop.org/systemd/systemd/commit/?id=9d10cbee89ca7f82d29b9cb27bef11e23e3803ba Thanks David Compiled from git and HoldOffTimeSec=0 works as expected. Thank you very much! Regards, Alad ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH V3] path-util.c: fix path_is_mount_point() for symlinks
On Fri, 20.02.15 13:25, har...@redhat.com (har...@redhat.com) wrote:
Sorry for the late review!
--- a/src/shared/path-util.c
+++ b/src/shared/path-util.c
@@ -456,9 +456,9 @@ int path_is_mount_point(const char *t, bool
allow_symlink) {
union file_handle_union h = FILE_HANDLE_INIT;
int mount_id = -1, mount_id_parent = -1;
-_cleanup_free_ char *parent = NULL;
struct stat a, b;
int r;
+_cleanup_close_ int fd = -1;
bool nosupp = false;
/* We are not actually interested in the file handles, but
@@ -468,7 +468,15 @@ int path_is_mount_point(const char *t, bool
allow_symlink) {
if (path_equal(t, /))
return 1;
-r = name_to_handle_at(AT_FDCWD, t, h.handle, mount_id,
allow_symlink ? AT_SYMLINK_FOLLOW : 0);
+fd = openat(AT_FDCWD, t,
O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC|(allow_symlink ? 0 : O_PATH));
+if (fd 0) {
+if (errno == ENOENT)
+return 0;
We eat up this error? Why not pass the -ENOENT up?
Otherwise I like this!
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCHv3] bus-proxy: add support for GetConnectionCredentials method
GetConnectionCredentials method was added to dbus-1 specification
more than one year ago. This method should return [...] as many
credentials as possible for the process connected to the server,
but at this moment only UnixUserID, LinuxSecurityLabel and
ProcessID are defined by the specification. We should add support
for next credentials after extending dbus-1 spec.
diff --git a/src/bus-proxyd/driver.c b/src/bus-proxyd/driver.c
index 3c613e4..e63a95d 100644
--- a/src/bus-proxyd/driver.c
+++ b/src/bus-proxyd/driver.c
@@ -49,9 +49,6 @@ static int get_creds_by_name(sd_bus *bus, const char *name,
uint64_t mask, sd_bu
if (r 0)
return r;
-if ((c-mask mask) != mask)
-return -ENOTSUP;
-
*_creds = c;
c = NULL;
@@ -109,6 +106,10 @@ int bus_proxy_process_driver(sd_bus *a, sd_bus *b,
sd_bus_message *m, SharedPoli
method name=\RemoveMatch\\n
arg type=\s\ direction=\in\/\n
/method\n
+ method name=\GetConnectionCredentials\\n
+ arg type=\s\ direction=\in\/\n
+ arg type=\a{sv}\ direction=\out\/\n
+ /method\n
method
name=\GetConnectionSELinuxSecurityContext\\n
arg type=\s\ direction=\in\/\n
arg type=\ay\ direction=\out\/\n
@@ -212,6 +213,72 @@ int bus_proxy_process_driver(sd_bus *a, sd_bus *b,
sd_bus_message *m, SharedPoli
return synthetic_reply_method_return(m, NULL);
+} else if (sd_bus_message_is_method_call(m, org.freedesktop.DBus,
GetConnectionCredentials)) {
+_cleanup_bus_creds_unref_ sd_bus_creds *creds = NULL;
+_cleanup_bus_message_unref_ sd_bus_message *reply = NULL;
+_cleanup_bus_error_free_ sd_bus_error error =
SD_BUS_ERROR_NULL;
+
+if (!sd_bus_message_has_signature(m, s))
+return synthetic_reply_method_error(m,
SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_INVALID_ARGS, Invalid parameters));
+
+r = get_creds_by_message(a, m,
SD_BUS_CREDS_PID|SD_BUS_CREDS_EUID|SD_BUS_CREDS_SELINUX_CONTEXT, creds,
error);
+if (r 0)
+return synthetic_reply_method_errno(m, r, error);
+
+r = sd_bus_message_new_method_return(m, reply);
+if (r 0)
+return synthetic_reply_method_errno(m, r, NULL);
+
+r = sd_bus_message_open_container(reply, 'a', {sv});
+if (r 0)
+return synthetic_reply_method_errno(m, r, NULL);
+
+/* Due to i.e. namespace translations some data might be
missing */
+
+if (creds-mask SD_BUS_CREDS_PID) {
+r = sd_bus_message_append(reply, {sv}, ProcessID,
u, (uint32_t) creds-pid);
+if (r 0)
+return synthetic_reply_method_errno(m, r,
NULL);
+}
+
+if (creds-mask SD_BUS_CREDS_EUID) {
+r = sd_bus_message_append(reply, {sv}, UnixUserID,
u, (uint32_t) creds-euid);
+if (r 0)
+return synthetic_reply_method_errno(m, r,
NULL);
+}
+
+if (creds-mask SD_BUS_CREDS_SELINUX_CONTEXT) {
+r = sd_bus_message_open_container(reply, 'e', sv);
+if (r 0)
+return synthetic_reply_method_errno(m, r,
NULL);
+
+r = sd_bus_message_append(reply, s,
LinuxSecurityLabel);
+if (r 0)
+return synthetic_reply_method_errno(m, r,
NULL);
+
+r = sd_bus_message_open_container(reply, 'v', ay);
+if (r 0)
+return synthetic_reply_method_errno(m, r,
NULL);
+
+r = sd_bus_message_append_array(reply, 'y',
creds-label, strlen(creds-label));
+if (r 0)
+return synthetic_reply_method_errno(m, r,
NULL);
+
+r = sd_bus_message_close_container(reply);
+if (r 0)
+return synthetic_reply_method_errno(m, r,
NULL);
+
+r = sd_bus_message_close_container(reply);
+if (r 0)
+return synthetic_reply_method_errno(m, r,
NULL);
+}
+
+r = sd_bus_message_close_container(reply);
+if (r 0)
+return synthetic_reply_method_errno(m, r, NULL);
+
+return synthetic_driver_send(m-bus, reply);
+
} else if (sd_bus_message_is_method_call(m,
Re: [systemd-devel] tmpfiles.d specifier support on argument when operating on files
On Wed, 18.02.15 18:17, Cristian Rodríguez (crrodrig...@opensuse.org) wrote:
From ee8e4f440def745b6f0655b897e65d48321e46c5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cristian=20Rodr=C3=ADguez?= crrodrig...@opensuse.org
Date: Wed, 18 Feb 2015 18:09:16 -0300
Subject: [PATCH] tmpfiles: implement specifier substitution for file
argument
Only for L and C types where it makes sense.
---
src/tmpfiles/tmpfiles.c | 12
1 file changed, 12 insertions(+)
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 88ba7e4..6de477b 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -1568,6 +1568,18 @@ static int parse_line(const char *fname, unsigned
line, const char *buffer) {
}
}
+if(IN_SET(i.type, CREATE_SYMLINK, COPY_FILES)) {
I figure it makes sense on more than these, for example the ones that
write strings to files. Also, there's a space missing after the if
and before the first (.
+if(i.argument) {
Similar here.
+_cleanup_free_ char *resolved_iarg = NULL;
+r = specifier_printf(i.argument, specifier_table,
NULL, resolved_iarg);
+if(r 0)
+return log_error_errno(r, [%s:%u] Failed to
replace specifiers: %s, fname, line, path);
+r = free_and_strdup(i.argument, resolved_iarg);
+if(r 0)
+return log_oom();
No need to duplicate the already allocated string again. Just assign
resolved_iarg to i.argument, and free that before.
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH] util: add rename_noreplace
From: Alban Crequy al...@endocode.com
renameat2() exists since Linux 3.15 but btrfs support for the flag
RENAME_NOREPLACE was added later.
This patch implements a fallback when renameat2() returns EINVAL.
EINVAL is the error returned when the filesystem does not support one of
the flags.
---
src/import/import-raw.c| 2 +-
src/import/import-tar.c| 2 +-
src/import/pull-raw.c | 2 +-
src/import/pull-tar.c | 2 +-
src/shared/copy.c | 13 ++---
src/shared/machine-image.c | 2 +-
src/shared/machine-pool.c | 2 +-
src/shared/util.c | 34 ++
src/shared/util.h | 2 ++
9 files changed, 52 insertions(+), 9 deletions(-)
diff --git a/src/import/import-raw.c b/src/import/import-raw.c
index 15e5eb2..e354023 100644
--- a/src/import/import-raw.c
+++ b/src/import/import-raw.c
@@ -245,7 +245,7 @@ static int raw_import_finish(RawImport *i) {
(void) rm_rf_dangerous(i-final_path, false, true, false);
}
-if (renameat2(AT_FDCWD, i-temp_path, AT_FDCWD, i-final_path,
RENAME_NOREPLACE) 0)
+if (rename_noreplace(AT_FDCWD, i-temp_path, AT_FDCWD, i-final_path)
0)
return log_error_errno(errno, Failed to move image into
place: %m);
free(i-temp_path);
diff --git a/src/import/import-tar.c b/src/import/import-tar.c
index d5b6dad..dcbe721 100644
--- a/src/import/import-tar.c
+++ b/src/import/import-tar.c
@@ -201,7 +201,7 @@ static int tar_import_finish(TarImport *i) {
(void) rm_rf_dangerous(i-final_path, false, true, false);
}
-if (renameat2(AT_FDCWD, i-temp_path, AT_FDCWD, i-final_path,
RENAME_NOREPLACE) 0)
+if (rename_noreplace(AT_FDCWD, i-temp_path, AT_FDCWD, i-final_path)
0)
return log_error_errno(errno, Failed to move image into
place: %m);
free(i-temp_path);
diff --git a/src/import/pull-raw.c b/src/import/pull-raw.c
index d1d77d5..d4b17e7 100644
--- a/src/import/pull-raw.c
+++ b/src/import/pull-raw.c
@@ -384,7 +384,7 @@ static void raw_pull_job_on_finished(PullJob *j) {
if (r 0)
goto finish;
-r = renameat2(AT_FDCWD, i-temp_path, AT_FDCWD, i-final_path,
RENAME_NOREPLACE);
+r = rename_noreplace(AT_FDCWD, i-temp_path, AT_FDCWD,
i-final_path);
if (r 0) {
r = log_error_errno(errno, Failed to move RAW file
into place: %m);
goto finish;
diff --git a/src/import/pull-tar.c b/src/import/pull-tar.c
index 504642f..69df7af 100644
--- a/src/import/pull-tar.c
+++ b/src/import/pull-tar.c
@@ -281,7 +281,7 @@ static void tar_pull_job_on_finished(PullJob *j) {
if (r 0)
goto finish;
-if (renameat2(AT_FDCWD, i-temp_path, AT_FDCWD, i-final_path,
RENAME_NOREPLACE) 0) {
+if (rename_noreplace(AT_FDCWD, i-temp_path, AT_FDCWD,
i-final_path) 0) {
r = log_error_errno(errno, Failed to rename to final
image name: %m);
goto finish;
}
diff --git a/src/shared/copy.c b/src/shared/copy.c
index 0239a58..458a654 100644
--- a/src/shared/copy.c
+++ b/src/shared/copy.c
@@ -404,9 +404,16 @@ int copy_file_atomic(const char *from, const char *to,
mode_t mode, bool replace
if (r 0)
return r;
-if (renameat2(AT_FDCWD, t, AT_FDCWD, to, replace ? 0 :
RENAME_NOREPLACE) 0) {
-unlink_noerrno(t);
-return -errno;
+if (replace) {
+if (renameat(AT_FDCWD, t, AT_FDCWD, to) 0) {
+unlink_noerrno(t);
+return -errno;
+}
+} else {
+if (rename_noreplace(AT_FDCWD, t, AT_FDCWD, to) 0) {
+unlink_noerrno(t);
+return -errno;
+}
}
return 0;
diff --git a/src/shared/machine-image.c b/src/shared/machine-image.c
index c6d2850..ad5bff4 100644
--- a/src/shared/machine-image.c
+++ b/src/shared/machine-image.c
@@ -440,7 +440,7 @@ int image_rename(Image *i, const char *new_name) {
if (!nn)
return -ENOMEM;
-if (renameat2(AT_FDCWD, i-path, AT_FDCWD, new_path, RENAME_NOREPLACE)
0)
+if (rename_noreplace(AT_FDCWD, i-path, AT_FDCWD, new_path) 0)
return -errno;
/* Restore the immutable bit, if it was set before */
diff --git a/src/shared/machine-pool.c b/src/shared/machine-pool.c
index e7671a3..7e902a3 100644
--- a/src/shared/machine-pool.c
+++ b/src/shared/machine-pool.c
@@ -140,7 +140,7 @@ static int setup_machine_raw(uint64_t size, sd_bus_error
*error) {
goto fail;
}
-if (renameat2(AT_FDCWD, tmp, AT_FDCWD, /var/lib/machines.raw,
RENAME_NOREPLACE) 0) {
+if (rename_noreplace(AT_FDCWD, tmp,
Re: [systemd-devel] [PATCH v2] tmpfiles: port to unquote_many_words()
On Mon, 09.03.15 15:11, daurnimator (q...@daurnimator.com) wrote:
Applied! Thanks!
(Usually we prefer commits with proper real name attributions, though...)
---
TODO| 2 --
man/tmpfiles.d.xml | 2 ++
src/tmpfiles/tmpfiles.c | 21 +++--
3 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/TODO b/TODO
index 60efaaf..4d5e2b6 100644
--- a/TODO
+++ b/TODO
@@ -226,8 +226,6 @@ Features:
* exponential backoff in timesyncd and resolved when we cannot reach a server
-* tmpfiles: port to unquote_many_words(), similar to sysusers
-
* unquote_many_words() should probably be used by a lot of code that
currently uses FOREACH_WORD and friends. For example, most conf
parsing callbacks should use it.
diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index 4bd0fcf..bc98d8b 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -118,6 +118,8 @@
d/run/user 0755 root root 10d -
L/tmp/foobar ---- /dev/null/programlisting
+paraFields may be enclosed within quotes and contain C-style
escapes./para
+
refsect2
titleType/title
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index 652fe5f..10c5a63 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -1506,23 +1506,25 @@ static int parse_line(const char *fname, unsigned
line, const char *buffer) {
_cleanup_(item_free_contents) Item i = {};
ItemArray *existing;
Hashmap *h;
-int r, c = -1, pos;
+int r, pos;
bool force = false, boot = false;
assert(fname);
assert(line = 1);
assert(buffer);
-r = sscanf(buffer,
- %ms %ms %ms %ms %ms %ms %n,
+r = unquote_many_words(buffer,
action,
path,
mode,
user,
group,
age,
- c);
-if (r 2) {
+ i.argument,
+ NULL);
+if (r 0)
+return log_error_errno(r, [%s:%u] Failed to parse line:
%m, fname, line);
+else if (r 2) {
log_error([%s:%u] Syntax error., fname, line);
return -EIO;
}
@@ -1559,15 +1561,6 @@ static int parse_line(const char *fname, unsigned
line, const char *buffer) {
return r;
}
-if (c = 0) {
-c += strspn(buffer+c, WHITESPACE);
-if (buffer[c] != 0 (buffer[c] != '-' || buffer[c+1] !=
0)) {
-i.argument = unquote(buffer+c, \);
-if (!i.argument)
-return log_oom();
-}
-}
-
switch (i.type) {
case CREATE_FILE:
--
2.3.1
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] 9 commits - configure.ac .gitignore Makefile.am Makefile-man.am man/systemd-fsckd.service.xml man/systemd-f...@.service.xml po/de.po po/el.po po/fr.po po/hu.po po
On Tue, 10.03.15 08:33, Martin Pitt (martin.p...@ubuntu.com) wrote: Lennart Poettering [2015-03-09 19:11 +0100]: Anyway, please look into fixing this, I am kinda relying on patches for this, as I don't use this myself. Fedora isn't set up for it, nor do I use a file system that still requires fsck at boot... Yep, we'll fix those. But for the record, this can be tested easily anywhere by replacing /usr/sbin/fsck with test/mocks/fsck (I'm on btrfs myself..) Hmm, is there also a ply mocker? That would be good! Lennart Poettering [2015-03-09 19:45 +0100]: And in a similar way client_progress_handler() is hosed too. Even worse: if a client sends messages byte-wise (which is absolutely OK on SOCK_STREAM) it will be kicked off the connection. We did talk about using a SOCK_DGRAM socket, but for some reason they can only be used in a connectionless mode on the server, i. e. you cannot listen() on them. This means that s-fsckd will never know in a timely fashion when a client disconnected or died, so you can only stop doing stuff after some generously long timeout. That's why this uses a SOCK_STREAM, but it does use send() and recv() to send messages in one block. To guard against broken/malicious clients, fsckd kicks connections which send malformed data. This could potentially made more explicit by using SOCK_SEQPACKET; we'd still need to check for short/malformed data of course, so that check can't go away entirely. But it at least stops clients trying to open in stream mode and send characters. But the only client here is our own s-fsck, so this is all internal anyway.. SOCK_SEQPACKET is what you want, it's connection based, as knows datagrams. That said, I don't think this should be necessary at all: fsckd should be connected directly with each fsck's stdout and parse the raw data directly, which means SOCK_STREAM needs to be used as stdout should be a stream fd. That didn't actually come up during review, but that sounds like a nice idea! I see two structures: - s-fsck runs fsck and forwards its stdout/err fds to fsckd, and then fsckd does all the parsing. This would also automatically eliminate the intermediate socket handling from above. Well, again: fsck should *not* 'forward'. It should not be bothered at all with processing stdout of the actual fsck tool at all. Instead it should open a AF_UNIX/SOCK_STREAM connection to fsckd, use shutdown() to close the read side, and then make the resulting fd the fd passed to fsck's -C parameter. That way, the fsck tool will pass the progress data *directly* to systemd-fsckd, and systemd-fsck will never see it at all! Or, more radically: - eliminate the current functionality of s-fsck and just make that send a request to s-fsckd to check a new device name. s-fsckd would then spawn off /usr/sbin/fsck by itself and start parsing its output. That's more complex handling of its children (but not too bad), and completely avoids passing around data through sockets, and s-fsck would be a trivial five-liner. Does that sound better/easier? I think it would be more obvious for admins if fsck stays part of the systemd-fsck@.service services, so that its resources/logs and so on are accounted to it, rather than a centralized fsckd.service. Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 4/5] fsckd: check if plymouth is running before attempting connection
On Tue, 10.03.15 11:34, Didier Roche (didro...@ubuntu.com) wrote:
I think it would make more sense to return 0 when ply isn't running,
and 1 if it is, no?
From fd28f24d9eaa16737cbc8f33b8fe1a806dc291b1 Mon Sep 17 00:00:00 2001
From: Didier Roche didro...@ubuntu.com
Date: Tue, 10 Mar 2015 10:05:19 +0100
Subject: [PATCH 4/5] fsckd: check if plymouth is running before attempting
connection
---
src/fsckd/fsckd.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index f73d23b..52d69cd 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -231,6 +231,9 @@ static int manager_connect_plymouth(Manager *m) {
union sockaddr_union sa = PLYMOUTH_SOCKET;
int r;
+if (!plymouth_running())
+return 1;
+
/* try to connect or reconnect if sending a message */
if (m-plymouth_fd = 0)
return 0;
@@ -278,6 +281,9 @@ static int manager_send_plymouth_message(Manager *m,
const char *message) {
r = manager_connect_plymouth(m);
if (r 0)
return r;
+/* 0 means that plymouth isn't running, do not send any message
yet */
+else if (r 0)
+return 0;
if (!m-plymouth_cancel_sent) {
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] add REMOTE_ADDR and REMOTE_PORT for Accept=yes
On Mon, 09.03.15 23:18, Ronny Chevalier (chevalier.ro...@gmail.com) wrote:
+if (!(our_env[n_env++] =
strappend(REMOTE_ADDR=, addr))) {
In newer code we try to avoid making assignments and doing if checks
in the same line.
Taking this out would be pretty gruesome because of use of the
post-increment operator.
Maybe by using a temporary variable like this:
char *env_str;
[...]
env_str = strappend(REMOTE_ADDR, addr);
if (!env_str) {
r = -ENOMEM;
goto fail;
}
our_env[n_env++] = env_str;
And the same with the other one ?
Yes, I much rpefer this way. (well, except the missing = in the
strappend() invocation...)
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 2/5] fsckd: Fix some error return values
On Tue, 10.03.15 11:33, Didier Roche (didro...@ubuntu.com) wrote:
diff --git a/src/fsckd/fsckd.c b/src/fsckd/fsckd.c
index 3fc923b..9393379 100644
--- a/src/fsckd/fsckd.c
+++ b/src/fsckd/fsckd.c
@@ -220,7 +220,7 @@ static int manager_connect_plymouth(Manager *m) {
goto fail;
}
-return 1;
+return 0;
Oh, well, this doesn't matter too much I guess, since we don't check
for the precise value, but just 0 or = 0...
Returning 1 in this case is actually a bit of a pattern I am trying to
adopt across the codebase: return 0 if all is OK but we didn't do
anything. Return 1 if all is OK and we actually did something. In
this case this distinction is of course entirely irrelevant, but it
was more of an automatism...
fail:
manager_disconnect_plymouth(m);
@@ -406,10 +406,8 @@ static int
manager_new_connection_handler(sd_event_source *s, int fd, uint32_t r
log_debug(New fsck client connected to fd: %d, new_client_fd);
c = new0(Client, 1);
-if (!c) {
-log_oom();
-return 0;
-}
+if (!c)
+return log_oom();
Well, I think logging and ignoring the OOM condition in this case is a
good idea. THink about the effect of returning an error up here: when
a handler callback returns an error sd-event will disable the event
source automatically. This means if fsckd hits an OOM once here, it
will become completely unresponsive, as it never processes incoming
connections anymore. However, if we simply log, close the connection,
and continue without propagating an error up, we are slightly more
robust: sure, the connection will fail, but subsequent connections
might not...
c-fd = new_client_fd;
new_client_fd = -1;
@@ -417,7 +415,7 @@ static int manager_new_connection_handler(sd_event_source
*s, int fd, uint32_t r
r = sd_event_add_io(m-event, c-event_source, c-fd, EPOLLIN,
client_progress_handler, c);
if (r 0) {
log_oom();
-return 0;
+return r;
Same here.
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] path_is_mount_point: handle false positive on some fs
On Mon, 09.03.15 11:27, Didier Roche (didro...@ubuntu.com) wrote: However, some file systems (seems overlayfs at least) would report a major(st_dev) as 0 on directories and not on files. The current path_is_mount_point() fallback logic would thus reports that every files is a mount point. The enclosed patch introduces a second fallback to read /proc/mounts, and ensures there is no shadowing by later mounting of parent directories. This slower path is only used when the 2 first methods failed to determine with assurance if the path is a mount point or not. Humppf, I am not convinced this is really a good idea. I mean, we use name_to_handle_at() precisely for the reason that we don't want to parse /proc/self/mountinfo, since that is really cumbersome if you want to do it properly, given that mounts can obstruct other mounts and symlink/realpath complexity and stuff. Note that the st_dev thing is the traditional way to detect whether one crosses a file system boundary. It's used for this by tools like cp, find, We slightly enhance this by using name_to_handle_at(), so that we can also detect bind mounts from file systems onto themselves. Now, if overlayfs breaks the same file system logic of all other tools, I am not convinced that systemd should not be broken by it too.. It sounds surprising that we should work around this in systemd, but not in all other tools. Or to turn this around: instead of patching systemd the better idea would probably be to teach overlayfs name_to_handle_at() support, so that there's a way to detect the mount id from such a file system in a safe way. Hmm, where precisely did you run into problems with this? Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH V3] path-util.c: fix path_is_mount_point() for symlinks
On Tue, 10.03.15 17:26, Lennart Poettering (lenn...@poettering.net) wrote:
On Fri, 20.02.15 13:25, har...@redhat.com (har...@redhat.com) wrote:
Sorry for the late review!
--- a/src/shared/path-util.c
+++ b/src/shared/path-util.c
@@ -456,9 +456,9 @@ int path_is_mount_point(const char *t, bool
allow_symlink) {
union file_handle_union h = FILE_HANDLE_INIT;
int mount_id = -1, mount_id_parent = -1;
-_cleanup_free_ char *parent = NULL;
struct stat a, b;
int r;
+_cleanup_close_ int fd = -1;
bool nosupp = false;
/* We are not actually interested in the file handles, but
@@ -468,7 +468,15 @@ int path_is_mount_point(const char *t, bool
allow_symlink) {
if (path_equal(t, /))
return 1;
-r = name_to_handle_at(AT_FDCWD, t, h.handle, mount_id,
allow_symlink ? AT_SYMLINK_FOLLOW : 0);
+fd = openat(AT_FDCWD, t,
O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC|(allow_symlink ? 0 : O_PATH));
+if (fd 0) {
+if (errno == ENOENT)
+return 0;
We eat up this error? Why not pass the -ENOENT up?
Hmm, as I see know the existing code also eats up ENOENT... Not
entirely sure why though.
But patch looks good then, please commit!
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] How to factory reset?
On Tue, Mar 10, 2015 at 6:38 PM, Lennart Poettering lenn...@poettering.net wrote: On Tue, 10.03.15 18:13, Tobias Hunger (tobias.hun...@gmail.com) wrote: So you want not just factory reset, but actually a stateless system, where every single boot is basically a factory reset? Yes, but I do have a state that I want to be applied by default at all times. Well, you want the factory defaults to be applied when the machine comes up, right? Kind of:-) I have a somewhat strange mix of ideas taken from docker and Revisiting How We Put Together Linux Systems:-) Basically I have a system a bit like docker that creates images for all the machines I use each night:, very similar to what docker does: You have a file, that defines some base image (which may be empty) and allows you to further customize that image, creating a new image. Unlike docker I create snapshots in a btrfs filesystem, mostly following the ideas you brought forward in the Revisiting blog. The end result is a root:system:vendor:arch and a usr:vendor:arch:timestamp snapshot per physical or virtual machine I run. So initial install of a system is really simple: Partition the HDD, set up a btrfs volume, copy root:s:v:a and a current usr:v:a:t over the network, run a script that updates gummiboot based on usr:v:a:t subvolumes found and reboot. Updates are even simpler: Copy a new usr:v:a:t over, update gummiboot using the same script, reboot. Currently /etc is a symlink to /usr/someplace/etc. That works well, but has the downside that -- since /usr is mounted ro -- I can not just experiment with different configurations anymore. Well, not without remounting /usr rw that is. I think I can do better with all those cool ideas like factory reset and stateless system and such floating around:-) Hmm, currently we focussed on two models: a) fully volatile, meaning / as tmpfs, with /usr mounted from physical media b) with only volatile state, but persitent configuration, meaning / mounted from physical media, and /var as tmpfs. Your model appears to be different from that. You actually want /var from from physical media, but /etc from tmpfs? That would be kinda the opposite of b)... Yes, that is what I want. I have a fully configured system (with that configuration stored in /usr/someplace/etc), I want that system to reset to that configuration regularly (in my case each time it boots), but keep the state. Do you actually want all of /var mounted of physical media? If you are interested in just logging, maybe just adding a normal mount for /var/log/ should suffice, leaving the rest of /var on tmpfs? Most of the data is in /var/log, but depending on services configured other directories in /var might be interesting, too. I might very well decide to deploy a mail server into a VM using my automated setup. I can then boot that and tweak the configuration, restarting the service as necessary. If I screw up then I want a reboot to take me back to something that works. If I improve on the configuration I will just update my image definition and have the change as the default the next time I update. I definitely do not want to loose mails by rebooting the production VM into that updated image, so the state needs to stick around. This is probably not a very common use-case, I admit that. Well, factory resets are supposed to be something that gets you back into a defined state if you fucked up your system. In such a case it might not be possible to boot up anymore to reset the state... Hence having this on the kernel cmdline is kinda a necessity to make this useful in real-life... Hmmm. Right. I assume you are going to nuking /etc without any further user interaction, so my worry is probably unfounded. Having --volatile=/path/to/usr/directory would be nice to have for the experiments I do right now. I guess that is not so very common that it makes sense to consider sending in a patch for that. Hmm, what precisely are you suggesting this would do? Have / of the machine be a tmpfs and mount /path/to/usr/ into as /usr there. Forget about this, I do not think that would be widely useful and as script fixes me up nicely:-) presets and machined ID are applied by PID 1, before it begins with starting any units, hence *really* early on. Note though that actually /etc/machine-id is used as flag for is /etc empty. If the file exists it is assumed that /etc is provisioned properly. If it is missing PID 1 will generate the machiend id and apply presets. An OS installer could put the machine-id into /usr just fine (and so can I) and systemd could just get it from there if available before generating a new one. It would be nice if the machine-id did not change during to a factory reset: It is used in the logs and changing it will basically make historical log data much harder to analyze. IIRC networkd also uses it to generate persistent MAC addresses for containers and such. So far this seems to be the only critical
[systemd-devel] [PATCH 3/4] Update the man page of tmpfiles.d(5), to document the new h/H command.
Update the man page of tmpfiles.d(5), to document the new h/H command. --- man/tmpfiles.d.xml | 32 1 file changed, 32 insertions(+) diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml index 8815bf9..469deeb 100644 --- a/man/tmpfiles.d.xml +++ b/man/tmpfiles.d.xml @@ -303,6 +303,37 @@ /varlistentry varlistentry + termvarnameh/varname/term + listitemparaSet file/directory attributes. Lines of this type + accept shell-style globs in place of normal path names./para + + paraThe format of the argument field is varname[+-=][aAcCdDeijsStTu] + /varname/para + + paraThe prefix varname+/varname causes the + attribute(s) to be added; varname-/varname causes the + attribute(s) to be removed; varname=/varname + causes the attributes to set exactly as the following letters./para + paraThe letters 'aAcCdDeijsStTu' select the new + attributes for the files, see + citerefentryrefentrytitlechattr/refentrytitle + manvolnum1/manvolnum/citerefentry for further information. + /para + paraPassing only varname=/varname as argument, + reset all the file attributes./para + + /listitem +/varlistentry + +varlistentry + termvarnameH/varname/term + listitemparaRecursively set file/directory attributes. Lines + of this type accept shell-style globs in place of normal + path names. + /para/listitem +/varlistentry + +varlistentry termvarnamea/varname/term termvarnamea+/varname/term listitemparaSet POSIX ACLs (access control lists). If @@ -529,6 +560,7 @@ citerefentry project='man-pages'refentrytitlesetfattr/refentrytitlemanvolnum1/manvolnum/citerefentry, citerefentry project='man-pages'refentrytitlesetfacl/refentrytitlemanvolnum1/manvolnum/citerefentry, citerefentry project='man-pages'refentrytitlegetfacl/refentrytitlemanvolnum1/manvolnum/citerefentry + citerefentry project='man-pages'refentrytitlechattr/refentrytitlemanvolnum1/manvolnum/citerefentry /para /refsect1 -- 2.1.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH V2] Allow systemd-tmpfiles to set file/directory attributes
Please, forgot these patches: there is a bug inside. Sorry for the noise. BR G.Baroncelli On 2015-03-10 20:36, Goffredo Baroncelli wrote: Hi all, This set of patches add two new line types to the tmpfiles files format. These new types of line are 'h' and 'H' (the recursively version), and allow to change the file/directory attributes, like chattr(1) does. One of the motivation of these patches is to get rid of the commit 11689d2a which force the NOCOW flag for the journal files. This was needed because systemd-journald has very poor performance when the filesytem is BTRFS due to its the COW behavior. My concern is that the NOCOW flag also prevent BTRFS to rebuild a corrupted file from a good copy if it is available. With this patch, now the NOCOW flag can be set by systemd-tmpfiles. See [1] for further information. BR G.Baroncelli Changelog: v1: first issue v2: accepted several suggestion on the style; added function change_attr_fd(); used the _cleanup_close_; returned negative errno; [1] Re: [systemd-devel] [RFC][PATCH] Add option to enable COW for journal file https://www.mail-archive.com/systemd-devel@lists.freedesktop.org/msg28724.html -- gpg @keyserver.linux.it: Goffredo Baroncelli kreijackATinwind.it Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH V3] Allow systemd-tmpfiles to set file/directory attributes
Hi all, This set of patches add two new line types to the tmpfiles files format. These new types of line are 'h' and 'H' (the recursively version), and allow to change the file/directory attributes, like chattr(1) does. One of the motivation of these patches is to get rid of the commit 11689d2a which force the NOCOW flag for the journal files. This was needed because systemd-journald has very poor performance when the filesytem is BTRFS due to its the COW behavior. My concern is that the NOCOW flag also prevent BTRFS to rebuild a corrupted file from a good copy if it is available. With this patch, now the NOCOW flag can be set by systemd-tmpfiles. See [1] for further information. BR G.Baroncelli Changelog: v1: first issue v2: accepted several suggestion on the style; added function change_attr_fd(); used the _cleanup_close_; returned negative errno; v3: swapped arguments of change_attr_fd() in path_set_attrib() [1] Re: [systemd-devel] [RFC][PATCH] Add option to enable COW for journal file https://www.mail-archive.com/systemd-devel@lists.freedesktop.org/msg28724.html -- gpg @keyserver.linux.it: Goffredo Baroncelli kreijackATinwind.it Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] Deadlocks with reloading jobs which are part of current transaction
On Wed, 2015-02-04 at 23:48 +0200, Uoti Urpala wrote: On Wed, 2015-02-04 at 21:57 +0100, Lennart Poettering wrote: currently being started. You are suggesting that the reload can suppressed when a start is already enqueued, but that's really not the case, because you first have to run m-c-c.s, before you can reload... If you mean literally running systemctl restart ... So unless I completely misunderstood your example, it seems that this does NOT demonstrate any problems with removing the blocking. Discussion seems to have died again. How to proceed with fixing this? Is there anything more I can clarify about why the current behavior (that is, when service startup is queued, have reload requests block until the service is up) is wrong and why the fix would be valid? ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/4] Add change_attr_fd()
Add change_attr_fd() function to modify the file/directory attribute.
---
src/shared/util.c | 22 ++
src/shared/util.h | 1 +
2 files changed, 23 insertions(+)
diff --git a/src/shared/util.c b/src/shared/util.c
index ba035ca..56097ec 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -7832,6 +7832,28 @@ int chattr_path(const char *p, bool b, unsigned mask) {
return chattr_fd(fd, b, mask);
}
+int change_attr_fd(int fd, unsigned value, unsigned mask) {
+unsigned old_attr, new_attr;
+
+assert(fd = 0);
+
+if (mask == 0)
+return 0;
+
+if (ioctl(fd, FS_IOC_GETFLAGS, old_attr) 0)
+return -errno;
+
+new_attr = (old_attr ~mask) |(value mask);
+
+if (new_attr == old_attr)
+return 0;
+
+if (ioctl(fd, FS_IOC_SETFLAGS, new_attr) 0)
+return -errno;
+
+return 0;
+}
+
int read_attr_fd(int fd, unsigned *ret) {
assert(fd = 0);
diff --git a/src/shared/util.h b/src/shared/util.h
index a83b588..a0bc883 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -1052,6 +1052,7 @@ int same_fd(int a, int b);
int chattr_fd(int fd, bool b, unsigned mask);
int chattr_path(const char *p, bool b, unsigned mask);
+int change_attr_fd(int fd, unsigned value, unsigned mask);
int read_attr_fd(int fd, unsigned *ret);
int read_attr_path(const char *p, unsigned *ret);
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH V2] Allow systemd-tmpfiles to set file/directory attributes
Hi all, This set of patches add two new line types to the tmpfiles files format. These new types of line are 'h' and 'H' (the recursively version), and allow to change the file/directory attributes, like chattr(1) does. One of the motivation of these patches is to get rid of the commit 11689d2a which force the NOCOW flag for the journal files. This was needed because systemd-journald has very poor performance when the filesytem is BTRFS due to its the COW behavior. My concern is that the NOCOW flag also prevent BTRFS to rebuild a corrupted file from a good copy if it is available. With this patch, now the NOCOW flag can be set by systemd-tmpfiles. See [1] for further information. BR G.Baroncelli Changelog: v1: first issue v2: accepted several suggestion on the style; added function change_attr_fd(); used the _cleanup_close_; returned negative errno; [1] Re: [systemd-devel] [RFC][PATCH] Add option to enable COW for journal file https://www.mail-archive.com/systemd-devel@lists.freedesktop.org/msg28724.html -- gpg @keyserver.linux.it: Goffredo Baroncelli kreijackATinwind.it Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 1/4] Add change_attr_fd()
Add change_attr_fd() function to modify the file/directory attribute.
---
src/shared/util.c | 22 ++
src/shared/util.h | 1 +
2 files changed, 23 insertions(+)
diff --git a/src/shared/util.c b/src/shared/util.c
index ba035ca..56097ec 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -7832,6 +7832,28 @@ int chattr_path(const char *p, bool b, unsigned mask) {
return chattr_fd(fd, b, mask);
}
+int change_attr_fd(int fd, unsigned value, unsigned mask) {
+unsigned old_attr, new_attr;
+
+assert(fd = 0);
+
+if (mask == 0)
+return 0;
+
+if (ioctl(fd, FS_IOC_GETFLAGS, old_attr) 0)
+return -errno;
+
+new_attr = (old_attr ~mask) |(value mask);
+
+if (new_attr == old_attr)
+return 0;
+
+if (ioctl(fd, FS_IOC_SETFLAGS, new_attr) 0)
+return -errno;
+
+return 0;
+}
+
int read_attr_fd(int fd, unsigned *ret) {
assert(fd = 0);
diff --git a/src/shared/util.h b/src/shared/util.h
index a83b588..a0bc883 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -1052,6 +1052,7 @@ int same_fd(int a, int b);
int chattr_fd(int fd, bool b, unsigned mask);
int chattr_path(const char *p, bool b, unsigned mask);
+int change_attr_fd(int fd, unsigned value, unsigned mask);
int read_attr_fd(int fd, unsigned *ret);
int read_attr_path(const char *p, unsigned *ret);
--
2.1.4
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 4/4] Add a new tmpfiles.d snippets to set the NOCOW the journal.
Add a new tmpfiles.d snippets to set the NOCOW attributes for the journal files. This allow better perfomance when the root file system is BTRFS. Pay attention that the NOCOW flags disables the checksum and prevent scrub to rebuild a corrupted journal. --- tmpfiles.d/journal-nocow.conf | 12 1 file changed, 12 insertions(+) create mode 100644 tmpfiles.d/journal-nocow.conf diff --git a/tmpfiles.d/journal-nocow.conf b/tmpfiles.d/journal-nocow.conf new file mode 100644 index 000..43a4f2b --- /dev/null +++ b/tmpfiles.d/journal-nocow.conf @@ -0,0 +1,12 @@ +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +# See tmpfiles.d(5) for details + + +# set the journal file as NOCOW; only valid for BTRFS filesystem +h /var/log/journal/%m - - - - +C -- 2.1.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 1/3] Allow systemd-tmpfiles to set the file/directory attributes
On 2015-03-08 23:06, Lennart Poettering wrote:
On Sun, 08.03.15 12:48, Goffredo Baroncelli (kreij...@libero.it) wrote:
dev_t major_minor;
+int attrib_value;
+int attrib_mask;
int appears to be a strange choice for a bitmask. The existing
chattr_fd() and chattr_path() calls use unsigned for this, so this
should too...
I read the code of chattr, and in this code these values are int.
Checking the definition of the ioctls I found:
#define FS_IOC_SETFLAGS _IOW('f', 2, long)
#define FS_IOC_GETFLAGS _IOR('f', 1, long)
so the ioctl argument seems to be a *signed long value*
Anyway I changed they to unsigned long.
bool uid_set:1;
bool gid_set:1;
bool mode_set:1;
bool age_set:1;
bool mask_perms:1;
+bool attrib_set:1;
bool keep_first_level:1;
@@ -762,6 +768,130 @@ static int path_set_acls(Item *item, const char *path)
{
return 0;
}
+static int get_attrib_from_arg(Item *item) {
+struct attributes_list_t { int value; char ch; } ;
The _t suffix is usually reserved for typedefs...
Also, it appears unnecessary to define a struct here at all, it can
just be anonymously defined when delcaring the array.
ok
+static struct attributes_list_t attributes[] = {
+{ FS_NOATIME_FL, 'A' }, /* do not update atime */
+{ FS_SYNC_FL, 'S' }, /* Synchronous updates */
+{ FS_DIRSYNC_FL, 'D' }, /* dirsync behaviour
(directories only) */
+{ FS_APPEND_FL, 'a' },/* writes to file may
only append */
+{ FS_COMPR_FL, 'c' }, /* Compress file */
+{ FS_NODUMP_FL, 'd' },/* do not dump file */
+{ FS_EXTENT_FL, 'e'}, /* Top of directory
hierarchies*/
+{ FS_IMMUTABLE_FL, 'i' }, /* Immutable file */
+{ FS_JOURNAL_DATA_FL, 'j' }, /* Reserved for ext3 */
+{ FS_SECRM_FL, 's' }, /* Secure deletion */
+{ FS_UNRM_FL, 'u' }, /* Undelete */
+{ FS_NOTAIL_FL, 't' },/* file tail should not
be merged */
+{ FS_TOPDIR_FL, 'T' },/* Top of directory
hierarchies*/
+{ FS_NOCOW_FL, 'C' }, /* Do not cow file */
+{ 0, 0 }
+};
Indenting borked.
const missing.
ok
+char *p = item-argument;
+enum { MODE_ADD, MODE_DEL, MODE_SET } mode = MODE_ADD;
So far we avoided defining enums in single lines, we line-broke them
nicely, once for each enum value. This should be done here too.
ok
+int value=0, mask=0;
Spaces after and before the = please.
+
+if (!p) {
+log_error(\%s\: setting ATTR need an argument,
item-path);
+return -1;
Please use explicit error codes, like -EINVAL, don't make up numeric
error codes like -1.
ok
Also see CODING_STYLE
+}
+
+if (*p == '+') {
+mode = MODE_ADD;
+p++;
+} else if (*p == '-') {
+mode = MODE_DEL;
+p++;
+} else if (*p == '=') {
+mode = MODE_SET;
+p++;
+}
+
+if (!*p mode != MODE_SET) {
+log_error(\%s\: setting ATTR: argument is empty,
item-path);
+return -4;
Error code
ok
+}
+for ( ; *p ; p++ ) {
Weird spaces...
ok
+int i;
+for ( i = 0; attributes[i].ch attributes[i].ch != *p
;i++);
Weird spaces...
Also, please add an explicit line break before the ;, so that it is
clear that this is a for loop without a body.
ok
+if (!attributes[i].ch) {
+log_error(\%s\: setting ATTR: unknown attr '%c',
+item-path, *p);
We don't break lines this eagerly in systemd.
ok
+return -2;
Error code...
+}
+if (mode == MODE_ADD || mode == MODE_SET)
+value |= attributes[i].value;
+else
+value = ~attributes[i].value;
+mask |= attributes[i].value;
+}
+
+if (mode == MODE_SET) {
+int i;
+for ( i = 0; attributes[i].ch;i++)
Weird spaces...
ok
+
+static int path_set_attrib(Item *item, const char *path) {
+int fd, r, f;
+struct stat st;
+
+/* do nothing */
+if (item-attrib_mask == 0 || !item-attrib_set)
+return 0;
+
+if (!lstat(path, st)
+!S_ISREG(st.st_mode) !S_ISDIR(st.st_mode)) {
+return 0;
+}
[systemd-devel] [PATCH 2/4] Allow systemd-tmpfiles to set the file/directory attributes
Allow systemd-tmpfiles to set the file/directory attributes, like chattr(1)
does. Two more commands are added: 'H' and 'h' to set the attributes,
recursively and not.
---
src/tmpfiles/tmpfiles.c | 140
1 file changed, 140 insertions(+)
diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index c948d4d..165605f 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -40,6 +40,7 @@
#include sys/types.h
#include sys/param.h
#include sys/xattr.h
+#include linux/fs.h
#include log.h
#include util.h
@@ -90,6 +91,8 @@ typedef enum ItemType {
ADJUST_MODE = 'm', /* legacy, 'z' is identical to this */
RELABEL_PATH = 'z',
RECURSIVE_RELABEL_PATH = 'Z',
+SET_ATTRIB = 'h',
+RECURSIVE_SET_ATTRIB = 'H',
} ItemType;
typedef struct Item {
@@ -108,12 +111,15 @@ typedef struct Item {
usec_t age;
dev_t major_minor;
+int attrib_value;
+int attrib_mask;
bool uid_set:1;
bool gid_set:1;
bool mode_set:1;
bool age_set:1;
bool mask_perms:1;
+bool attrib_set:1;
bool keep_first_level:1;
@@ -762,6 +768,115 @@ static int path_set_acls(Item *item, const char *path) {
return 0;
}
+static int get_attrib_from_arg(Item *item) {
+static const struct { int value; char ch; } attributes[] = {
+{ FS_NOATIME_FL, 'A' }, /* do not update atime */
+{ FS_SYNC_FL, 'S' }, /* Synchronous updates */
+{ FS_DIRSYNC_FL, 'D' }, /* dirsync behaviour (directories
only) */
+{ FS_APPEND_FL, 'a' },/* writes to file may only append */
+{ FS_COMPR_FL, 'c' }, /* Compress file */
+{ FS_NODUMP_FL, 'd' },/* do not dump file */
+{ FS_EXTENT_FL, 'e'}, /* Top of directory hierarchies*/
+{ FS_IMMUTABLE_FL, 'i' }, /* Immutable file */
+{ FS_JOURNAL_DATA_FL, 'j' }, /* Reserved for ext3 */
+{ FS_SECRM_FL, 's' }, /* Secure deletion */
+{ FS_UNRM_FL, 'u' }, /* Undelete */
+{ FS_NOTAIL_FL, 't' },/* file tail should not be merged */
+{ FS_TOPDIR_FL, 'T' },/* Top of directory hierarchies*/
+{ FS_NOCOW_FL, 'C' }, /* Do not cow file */
+{ 0, 0 }
+};
+char *p = item-argument;
+enum {
+MODE_ADD,
+MODE_DEL,
+MODE_SET
+} mode = MODE_ADD;
+unsigned long value = 0, mask = 0;
+
+if (!p) {
+log_error(\%s\: setting ATTR need an argument, item-path);
+return -EINVAL;
+}
+
+if (*p == '+') {
+mode = MODE_ADD;
+p++;
+} else if (*p == '-') {
+mode = MODE_DEL;
+p++;
+} else if (*p == '=') {
+mode = MODE_SET;
+p++;
+}
+
+if (!*p mode != MODE_SET) {
+log_error(\%s\: setting ATTR: argument is empty,
item-path);
+return -EINVAL;
+}
+for (; *p ; p++) {
+int i;
+for (i = 0; attributes[i].ch attributes[i].ch != *p; i++)
+;
+if (!attributes[i].ch) {
+log_error(\%s\: setting ATTR: unknown attr '%c',
item-path, *p);
+return -EINVAL;
+}
+if (mode == MODE_ADD || mode == MODE_SET)
+value |= attributes[i].value;
+else
+value = ~attributes[i].value;
+mask |= attributes[i].value;
+}
+
+if (mode == MODE_SET) {
+int i;
+for (i = 0; attributes[i].ch; i++)
+mask |= attributes[i].value;
+}
+
+assert(mask);
+
+item-attrib_mask = mask;
+item-attrib_value = value;
+item-attrib_set = true;
+
+return 0;
+
+}
+
+static int path_set_attrib(Item *item, const char *path) {
+_cleanup_close_ int fd = -1;
+int r;
+unsigned f;
+struct stat st;
+
+/* do nothing */
+if (item-attrib_mask == 0 || !item-attrib_set)
+return 0;
+
+if (lstat(path, st) == 0
+!S_ISREG(st.st_mode) !S_ISDIR(st.st_mode)) {
+return 0;
+}
+
+fd = open(path, O_RDONLY|O_NONBLOCK|O_CLOEXEC);
+
+if (fd 0)
+return log_error_errno(errno, Cannot open \%s\: %m, path);
+
+f = item-attrib_value item-attrib_mask;
+if (!S_ISDIR(st.st_mode))
+f = ~FS_DIRSYNC_FL;
+r = change_attr_fd(fd, f, item-attrib_mask);
+if (r 0)
+return log_error_errno(errno,
+Cannot
Re: [systemd-devel] [systemd-commits] 9 commits - configure.ac .gitignore Makefile.am Makefile-man.am man/systemd-fsckd.service.xml man/systemd-f...@.service.xml po/de.po po/el.po po/fr.po po/hu.po po
Lennart Poettering [2015-03-09 19:11 +0100]: Anyway, please look into fixing this, I am kinda relying on patches for this, as I don't use this myself. Fedora isn't set up for it, nor do I use a file system that still requires fsck at boot... Yep, we'll fix those. But for the record, this can be tested easily anywhere by replacing /usr/sbin/fsck with test/mocks/fsck (I'm on btrfs myself..) Lennart Poettering [2015-03-09 19:45 +0100]: And in a similar way client_progress_handler() is hosed too. Even worse: if a client sends messages byte-wise (which is absolutely OK on SOCK_STREAM) it will be kicked off the connection. We did talk about using a SOCK_DGRAM socket, but for some reason they can only be used in a connectionless mode on the server, i. e. you cannot listen() on them. This means that s-fsckd will never know in a timely fashion when a client disconnected or died, so you can only stop doing stuff after some generously long timeout. That's why this uses a SOCK_STREAM, but it does use send() and recv() to send messages in one block. To guard against broken/malicious clients, fsckd kicks connections which send malformed data. This could potentially made more explicit by using SOCK_SEQPACKET; we'd still need to check for short/malformed data of course, so that check can't go away entirely. But it at least stops clients trying to open in stream mode and send characters. But the only client here is our own s-fsck, so this is all internal anyway.. Lennart Poettering [2015-03-09 19:47 +0100]: Oh, and I am only realizing now that the whole thing doesn't do *at all* what we discussed. The idea was to invoke the actual fsck tools with their stdout connected directly to fsckd. Instead the old systemd-fsck tool still is the one that parses the fsck output and which now simply forwards that info. That didn't actually come up during review, but that sounds like a nice idea! I see two structures: - s-fsck runs fsck and forwards its stdout/err fds to fsckd, and then fsckd does all the parsing. This would also automatically eliminate the intermediate socket handling from above. Or, more radically: - eliminate the current functionality of s-fsck and just make that send a request to s-fsckd to check a new device name. s-fsckd would then spawn off /usr/sbin/fsck by itself and start parsing its output. That's more complex handling of its children (but not too bad), and completely avoids passing around data through sockets, and s-fsck would be a trivial five-liner. Does that sound better/easier? Thanks, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] 9 commits - configure.ac .gitignore Makefile.am Makefile-man.am man/systemd-fsckd.service.xml man/systemd-f...@.service.xml po/de.po po/el.po po/fr.po po/hu.po po
Hey Lennart, Lennart Poettering [2015-03-09 18:39 +0100]: I looked in more detail at the fsckd code that was commited a few weeks ago, and I cannot say I liked what I saw. The code still has all kinds of issues, including memory corruptions, fd handling errors, and tons and tons of incorrect error handling (I think more times the code passes an incorrect error to log_xyz_errno() than a correct one!). Argh, thanks for spotting them! And there are conceptional issues with the code too (for example, it's not OK to keep /dev/console open, this breaks the SAK key logic...) Ah, good point. This isn't really new (previously, s-fsck kept it open all the time), so we weren't aware of that SAK issue, but of course that should be fixed. Please, be more careful with complex code like this, this needs more rounds of review before something like this can be merged... Okay. It went through 4 rounds of review on the ML and several folks commented, there were no more outstanding/unaddressed review comments, and it seemed to me that we had consensus on IRC to land this now. Procedurally, what should I/we do next time before landing such large patches? Wait for an explicit ack on the ML from you? Thanks and sorry for the hassle, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [systemd-commits] 9 commits - configure.ac .gitignore Makefile.am Makefile-man.am man/systemd-fsckd.service.xml man/systemd-f...@.service.xml po/de.po po/el.po po/fr.po po/hu.po po
Le 10/03/2015 08:33, Martin Pitt a écrit : Lennart Poettering [2015-03-09 19:11 +0100]: Oh, and I am only realizing now that the whole thing doesn't do *at all* what we discussed. The idea was to invoke the actual fsck tools with their stdout connected directly to fsckd. Instead the old systemd-fsck tool still is the one that parses the fsck output and which now simply forwards that info. That didn't actually come up during review, but that sounds like a nice idea! I see two structures: - s-fsck runs fsck and forwards its stdout/err fds to fsckd, and then fsckd does all the parsing. This would also automatically eliminate the intermediate socket handling from above. We will still need the cancel message back socket (so below). Or, more radically: - eliminate the current functionality of s-fsck and just make that send a request to s-fsckd to check a new device name. s-fsckd would then spawn off /usr/sbin/fsck by itself and start parsing its output. That's more complex handling of its children (but not too bad), and completely avoids passing around data through sockets, and s-fsck would be a trivial five-liner. Just a note if we go that path, then systemd-fsck*.service would always be successful as only passing the request, but then, any cancellation of a fsck in progress or worse, any fsck failing won't mark the corresponding systemd-fsck@ instance as failed. Cheers, Didier ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] How to factory reset?
On Tue, Mar 10, 2015 at 9:33 PM, Tobias Hunger tobias.hun...@gmail.com wrote: presets and machined ID are applied by PID 1, before it begins with starting any units, hence *really* early on. Note though that actually /etc/machine-id is used as flag for is /etc empty. If the file exists it is assumed that /etc is provisioned properly. If it is missing PID 1 will generate the machiend id and apply presets. An OS installer could put the machine-id into /usr just fine (and so can I) and systemd could just get it from there if available before generating a new one. It would be nice if the machine-id did not change during to a factory reset: It is used in the logs and changing it will basically make historical log data much harder to analyze. IIRC networkd also uses it to generate persistent MAC addresses for containers and such. So far this seems to be the only critical piece of information that can not get restored via tmpfiles.d. Thinking about this a bit longer: /usr does not seem like a good idea. The machine-id is supposed to be unique and usr-images are meant to be reused. Maybe provide a kernel command line option to force the machine-id if none is set yet? Best Regards, Tobias ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] How to factory reset?
On Tue, Mar 10, 2015 at 11:13 AM, Tobias Hunger tobias.hun...@gmail.com wrote: Even if all filesystems are encrypted you could factory-reset random computers you have access to, simply by editing the bootloader configuration file usually found in the poorly protected EFI partition! If you're concerned about bootloader configuration modification as a threat vector, then it needs to go on an encrypted volume. This suggests an initial bootloader configuration that only enables the user to supply a passphrase/key file to unlock that volume, and then load a new bootloader configuration file. GRUB2 kinda does support this. The ESP grub.cfg can handle the cryptodisk and luksopen to grant access to the encrypted volume; and configfile command to load a new grub.cfg located on that volume. And from there the boot is normal including reading kernel and initramfs from the encrypted volume. -- Chris Murphy ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] SELinux labels on unix sockets
Lennart Poettering lenn...@poettering.net writes: On Fri, 06.03.15 13:04, Jan Synáček (jsyna...@redhat.com) wrote: Hello, when systemd creates a socket file, it explicitly calls a selinux procedure to label it. I don't think that is needed, as the kernel does the right thing when the socket is created. Am I missing something? Why is the explicit labeling in place? Well, it's complicated. If we use socket activation we label a socket taking into account the label of the binary that is eventually started for it. And then, for file system sockets the kernel could traditionally only derive the label to use from the directory the socket was created in, which makes it difficult to have multiple sockets in the same directory with different labels, which is pretty frequently done though. That said, I think this limitation was lifted a while back in the kernel, and the policy can now also take the socket file name into consideration and derive different labels automatically. Ultimately, I only superficially understand the selinux code. We rely on patches from Dan co to keep it up-to-date. Better keep him in the loop. If there is a way to specify the automatic labeling of the socket files according to their names, and not the directory that they reside in, in the policy, then the code that does the explicit labeling isn't necessary. If not, the code would label the sockets incorrectly, which is what actually happens now. Plus the fact that systemd doesn't correctly re-require the libselinux handle (meaning that policy updates/reloads are not recognized) on policy changes makes the logic not work. I've tried to write a small piece of code that would execute whenever a policy is modified, but failed to do so. Calling selinux_set_callback(SELINUX_CB_POLICYLOAD, cb) doesn't do anything. So, I think that the code that explictly labels the socket files should be removed. It would be nice to hear from Dan, though. Cheers, -- Jan Synacek Software Engineer, Red Hat signature.asc Description: PGP signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
