Re: [systemd-devel] sd_journal_add_match if not using the form of "FIELD=value"

2015-03-08 Thread Mantas Mikulėnas 
On Mon, Mar 9, 2015 at 12:53 AM, Lennart Poettering 
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?

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

Re: [systemd-devel] Possible systemd segfault switching from 216 to 219 in fedora upgrade

2015-03-08 Thread Chris Murphy 
On Sun, Mar 8, 2015 at 4:32 PM, Lennart Poettering
 wrote:
> On Thu, 05.03.15 22:07, James Hogarth (james.hoga...@gmail.com) wrote:

>> This naturally means that the serialization/deserialization needs to
>> be forwards *and* backwards compatible between 216 and 219 for this to
>> work.
>
> Yeah, but no. Allowing uprgades is one thing, allowing downgrades a
> completely different one, and nothing we want to support.

I don't see how it's even practical if you wanted to. It would also
need to support systemd-208 for Fedora 20, and possibly systemd-204
for Fedora 19.

Users must be able to upgrade to Fedora n from Fedora n-2 and n-1, as
they're supported systems still. Arguably users ought to be able to
upgrade out of the most recent EOL'd Fedora, n-3.

It makes me look forward to atomically updated/upgraded stateless
systems so we can avoid these kinds of musical chairs figuring out
nutty layout assembly to make sure updates actually work.

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

Re: [systemd-devel] [PATCH] fix strict aliasing violations in src/udev/udev-builtin-usb_id.c

2015-03-08 Thread Shawn Landden 
On Sun, Mar 8, 2015 at 4:11 PM, Lennart Poettering 
wrote:

> On Thu, 05.03.15 04:58, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl)
> wrote:
>
> > > +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,21 @@ 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;
> > > +struct usb_interface_descriptor desc;
> > >  char if_str[8];
> > >
> > > -desc = (struct usb_interface_descriptor *) &buf[pos];
> > > -if (desc->bLength < 3)
> > > +memcpy(&desc, &buf[pos], sizeof(desc));
> > Copying it seems suboptimal. But is this actually an aliasing
> > violation? buf is a char array, and [1] says: "a character type
> > may alias any other type".
> >
> > [1]
> https://gcc.gnu.org/onlinedocs/gcc-4.6.1/gcc/Optimize-Options.html#index-fstrict_002daliasing-825
>
> Also, I greatly prefer using unions for these things, to make the
> aliasing explicit, rather than copying things.
>
> The other solution I had was to use offsetof() to basically make it an
enum, but that made the code was quite a bit more verbose.

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



-- 
Shawn Landden
___
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-08 Thread Shawn Landden 
the xinetd REMOTE_IP is a fedora extension so I think we should avoid it.

On Sun, Mar 8, 2015 at 4:24 PM, Shawn Landden  wrote:

> ---
>  TODO   |  2 --
>  man/systemd.socket.xml |  6 +-
>  src/core/service.c | 47
> +++
>  3 files changed, 52 insertions(+), 3 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..20f1e0c 100644
> --- a/man/systemd.socket.xml
> +++ b/man/systemd.socket.xml
> @@ -357,7 +357,11 @@
>  daemons designed for usage with
>
>  
> inetd8
>  to work unmodified with systemd socket
> -activation.
> +activation.
> +For IPv4 and IPv6 connections the
> REMOTE_ADDR
> +environment variable will be set with remote IP, and
> REMOTE_PORT
> +environment variable set to the remote port, similar to CGI
> +(for SOCK_RAW the port is the IP protocol).
>
>
>
> diff --git a/src/core/service.c b/src/core/service.c
> index cc4ea19..6a690ac 100644
> --- a/src/core/service.c
> +++ b/src/core/service.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include "async.h"
>  #include "manager.h"
> @@ -1119,6 +1120,52 @@ static int service_spawn(
>  goto fail;
>  }
>
> +if (s->accept_socket.unit) {
> +union sockaddr_union sa;
> +socklen_t salen = sizeof(sa);
> +_cleanup_free_ char *remote_addr = NULL;
> +char a[MAX(INET6_ADDRSTRLEN, INET_ADDRSTRLEN)];
> +
> +r = getpeername(s->socket_fd, &sa.sa, &salen);
> +if (r < 0) {
> +r = -errno;
> +goto fail;
> +}
> +
> +if (sa.sa.sa_family == AF_INET ||
> +sa.sa.sa_family == AF_INET6) {
> +if (inet_ntop(sa.sa.sa_family,
> +  /* this field of the API is kinda
> braindead,
> +   * should take head of struct so it
> can be passed the union...*/
> +  sa.sa.sa_family == AF_INET6 ?
> +&sa.in6.sin6_addr :
> +&sa.in.sin_addr,
> +  a, sizeof(a)) == NULL) {
> +r = -errno;
> +goto fail;
> +}
> +
> +if (asprintf(our_env + n_env++,
> + "REMOTE_ADDR=%s",
> + /* musl and glibc inet_ntop()
> present v4-mapped addresses in :::a.b.c.d form */
> + sa.sa.sa_family == AF_INET6 &&
> strchr(a, '.') ?
> +   strempty(startswith(a, ":::"))
> :
> +   a) < 0) {
> +r = -ENOMEM;
> +goto fail;
> +}
> +
> +if (asprintf(our_env + n_env++,
> + "REMOTE_PORT=%u",
> + ntohs(sa.sa.sa_family == AF_INET6 ?
> + sa.in6.sin6_port :
> + sa.in.sin_port)) < 0) {
> +r = -ENOMEM;
> +goto fail;
> +}
> +}
> +}
> +
>  final_env = strv_env_merge(2, UNIT(s)->manager->environment,
> our_env, NULL);
>  if (!final_env) {
>  r = -ENOMEM;
> --
> 2.2.1.209.g41e5f3a
>
>


-- 
Shawn Landden
___
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

2015-03-08 Thread Shawn Landden 
---
 TODO   |  2 --
 man/systemd.socket.xml |  6 +-
 src/core/service.c | 47 +++
 3 files changed, 52 insertions(+), 3 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..20f1e0c 100644
--- a/man/systemd.socket.xml
+++ b/man/systemd.socket.xml
@@ -357,7 +357,11 @@
 daemons designed for usage with
 
inetd8
 to work unmodified with systemd socket
-activation.
+activation.
+For IPv4 and IPv6 connections the REMOTE_ADDR
+environment variable will be set with remote IP, and 
REMOTE_PORT
+environment variable set to the remote port, similar to CGI
+(for SOCK_RAW the port is the IP protocol).
   
 
   
diff --git a/src/core/service.c b/src/core/service.c
index cc4ea19..6a690ac 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "async.h"
 #include "manager.h"
@@ -1119,6 +1120,52 @@ static int service_spawn(
 goto fail;
 }
 
+if (s->accept_socket.unit) {
+union sockaddr_union sa;
+socklen_t salen = sizeof(sa);
+_cleanup_free_ char *remote_addr = NULL;
+char a[MAX(INET6_ADDRSTRLEN, INET_ADDRSTRLEN)];
+
+r = getpeername(s->socket_fd, &sa.sa, &salen);
+if (r < 0) {
+r = -errno;
+goto fail;
+}
+
+if (sa.sa.sa_family == AF_INET ||
+sa.sa.sa_family == AF_INET6) {
+if (inet_ntop(sa.sa.sa_family,
+  /* this field of the API is kinda 
braindead,
+   * should take head of struct so it can 
be passed the union...*/
+  sa.sa.sa_family == AF_INET6 ?
+&sa.in6.sin6_addr :
+&sa.in.sin_addr,
+  a, sizeof(a)) == NULL) {
+r = -errno;
+goto fail;
+}
+
+if (asprintf(our_env + n_env++,
+ "REMOTE_ADDR=%s",
+ /* musl and glibc inet_ntop() present 
v4-mapped addresses in :::a.b.c.d form */
+ sa.sa.sa_family == AF_INET6 && strchr(a, 
'.') ?
+   strempty(startswith(a, ":::")) :
+   a) < 0) {
+r = -ENOMEM;
+goto fail;
+}
+
+if (asprintf(our_env + n_env++,
+ "REMOTE_PORT=%u",
+ ntohs(sa.sa.sa_family == AF_INET6 ?
+ sa.in6.sin6_port :
+ sa.in.sin_port)) < 0) {
+r = -ENOMEM;
+goto fail;
+}
+}
+}
+
 final_env = strv_env_merge(2, UNIT(s)->manager->environment, our_env, 
NULL);
 if (!final_env) {
 r = -ENOMEM;
-- 
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/RFC] FuseMAC: user space MAC in systemd

2015-03-08 Thread Lennart Poettering 
On Mon, 02.03.15 22:49, Topi Miettinen (toiwo...@gmail.com) wrote:

> Intercept and filter filesystem operations of processes launched
> by systemd with FUSE.
> 
> Implement learning, enforcing and auto enforcing/learning modes,
> enabled with new exec directive FuseMAC.
> 
> FS operations can be filtered by access type (e.g. getattr/read,
> cf. AppArmor or TOMOYO Linux) or for more fine grained control,
> which area of the file is being accessed.
> 
> Due to limitations of FUSE, API file systems can't be intercepted.
> 
> Also the patch seems to trigger bugs in kernel (hang CPU).

Hmm, if I understand this patch right, then you proxy all file system
access through a userspace fuse tool to enforce additional access
restrictions?

Well, I am pretty sure that systemd should not be in the business of
implementing a new access control mechanism. It's fine to expose ones
that are supported in the kernel in ways, or even using things like
namespacing to implement access control, but it really shouldn't be
systemd that is the one enforcing file access rights here, I am very
sure. It might be the place to encode and configure policy, but not
the place to enforce it.

I think this is better done outside of systemd, and quite frankly, in
the kernel, already for performance reasons.

Sorry!

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] fix strict aliasing violations in src/udev/udev-builtin-usb_id.c

2015-03-08 Thread Lennart Poettering 
On Thu, 05.03.15 04:58, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:

> > +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,21 @@ 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;
> > +struct usb_interface_descriptor desc;
> >  char if_str[8];
> >  
> > -desc = (struct usb_interface_descriptor *) &buf[pos];
> > -if (desc->bLength < 3)
> > +memcpy(&desc, &buf[pos], sizeof(desc));
> Copying it seems suboptimal. But is this actually an aliasing
> violation? buf is a char array, and [1] says: "a character type
> may alias any other type".
> 
> [1] 
> https://gcc.gnu.org/onlinedocs/gcc-4.6.1/gcc/Optimize-Options.html#index-fstrict_002daliasing-825

Also, I greatly prefer using unions for these things, to make the
aliasing explicit, rather than copying things.

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] Unable to remove images using machinectl

2015-03-08 Thread Lennart Poettering 
On Tue, 03.03.15 14:22, Erik Johnson (e...@saltstack.com) wrote:

> On Mon, Mar 02, 2015 at 11:01:44PM +0100, Lennart Poettering wrote:
> >On Mon, 02.03.15 14:10, Erik Johnson (e...@saltstack.com) wrote:
> >
> >>On Mon, Mar 02, 2015 at 07:51:35PM +0100, Lennart Poettering wrote:
> >>>On Mon, 02.03.15 11:06, Erik Johnson (e...@saltstack.com) wrote:
> >>>
> I'm getting a similar error to the one described in the following post
> from a couple weeks ago:
> 
> https://www.mail-archive.com/systemd-devel@lists.freedesktop.org/msg28255.html
> 
> I get an "access denied" error when running machinectl remove, even as
> root.
> >>>
> >>>This was a bug in the dbus policy. It should be fixed with this commit:
> >>>
> >>>http://cgit.freedesktop.org/systemd/systemd/commit/src/machine/org.freedesktop.machine1.conf?id=72c3897f77a7352618ea76b880a6764f52d6327b
> >>>
> >>>Lennart
> >>>
> >>>--
> >>>Lennart Poettering, Red Hat
> >>
> >>
> >>Thanks. I applied the patch, restarted dbus, and now I get the
> >>following after a 20-30 second pause:
> >>
> >>Could not remove image: Activation of org.freedesktop.machine1 timed out
> >
> >dbus is not a service that cannot be restarted during normal
> >operation. This is a well-known limitation of dbus. Reloading
> >configuration should be sufficient.
> >
> >You probably need to reboot now to get back to a working system...
> >
> >Lennart
> >
> >-- 
> >Lennart Poettering, Red Hat
> 
> 
> OK. After rebooting, it's still not working. Were the necessary changes
> limited to that one commit?

Oh, umm, so there are actually more changes necessary: machined lacked
the right caps to execute the deletion ioctl.

Changing the CapabilityBoundingSet= line in systemd-machined to this
should make this work:

CapabilityBoundingSet=CAP_KILL CAP_SYS_PTRACE CAP_SYS_ADMIN CAP_SETGID 
CAP_SYS_CHROOT CAP_DAC_READ_SEARCH CAP_DAC_OVERRIDE

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] sd_journal_add_match if not using the form of "FIELD=value"

2015-03-08 Thread Lennart Poettering 
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 presume that sd_journal_add_match is doing an identical value match
> that would preclude searching for "FIELD=value" using just "FIELD"?

The matches check fields against values. That's all, really.

Lennart

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

Re: [systemd-devel] "systemd --test" fails

2015-03-08 Thread Lennart Poettering 
On Fri, 06.03.15 11:23, Frank Steiner (fsteiner-ma...@bio.ifi.lmu.de) wrote:

> Hi,
> 
> for debugging an ordering cycle during boot I tried to run
>   systemd --test --system  --unit=multi-user.target --log-level=debug
> as user (as it doesn't work as root).
> 
> The result is just:
> systemd 210 running in system mode. (+PAM +LIBWRAP +AUDIT +SELINUX -IMA 
> +SYSVINIT +LIBCRYPTSETUP +GCRYPT +ACL +XZ +SECCOMP +APPARMOR)
> Detected architecture 'x86-64'.
> Failed to set hostname to : Operation not permitted
> Failed to open /dev/tty0: Permission denied
> Using cgroup controller name=systemd. File system hierarchy is at 
> /sys/fs/cgroup/systemd/user.slice/user-0.slice/session-2.scope.
> Release agent already installed.
> Failed to create root cgroup hierarchy: Permission denied
> Failed to allocate manager object: Permission denied
> 
> With --used the two "Failed" lines disappear, but nothing else is shown.
> 
> This is a SLES 12 system. Any idea what I'm doing wrong? What permissions 
> does the user need to run the test mode? Adding him to the root group 
> didn't suffice.

210 is really old. THis has been fixed a while back in newer
versions. Please update or ask your distribution to backport the fixes.

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] Notification socket and chroot vs PrivateNetwork conflict (abstract vs file-system)

2015-03-08 Thread Lennart Poettering 
On Sat, 07.03.15 00:20, Alban Crequy (alban.cre...@gmail.com) wrote:

> > I figure we could open a new mount namespace and mount the file system
> > socket into the chroot, but not sure I like the idea...
> 
> Maybe that's the way to do it... but where would you bind mount the
> socket file? in $CHROOT/tmp which should be writeable when
> PrivateTmp=true? Of course it will not work if the daemon is doing the
> chroot itself instead of relying on systemd's RootDirectory.

/tmp is not a place for sockets.

Whatever code sets up the execution environment which wants to allow
notifications would have the responsibility to bind mount the
notification socket... I mean, the notification socket isn't really
too different from other sockets. If you run your php program in a
chroot, and you want ot to access your mysql server via AF_UNIX, then
you need to mount the socket over too, that's really the same story here...

> 
> The same problem exists even without using
> PrivateNetwork/RootDirectory when the service starts a container with
> "nspawn --private-network" and the program inside the container wants
> to notify when it's ready. This has the same root cause: the service
> runs in a new mount/chroot and a new network namespace.

This is not a "problem". This is a feature. I mean, you asked for
isolation, hence you got isolation...

I am pretty sure that there should not be any way for container member
processes to communication via assumed-to-be-local IPC to processes
outside of the container, unless they do so with the container
manager. In this case meaning: if you want notification like this,
then your container manager needs to proxy that.

> There is also the additional problem that the program inside the
> container runs in a different cgroup (/system.slice/docker-... for
> docker containers, or /machine.slice... for nspawn containers).
> 
> There is the tool "sdnotify-proxy" to proxy the notify socket from
> systemd to a socket file which can be bind mounted in the container.
> sdnotify-proxy works, but I would like to know if someone finds a
> better way for containers :)

I am not sure I understand what "sdnotify-proxy" does and who needs it?

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] Notification socket and chroot vs PrivateNetwork conflict (abstract vs file-system)

2015-03-08 Thread Lennart Poettering 
On Thu, 05.03.15 12:16, Alban Crequy (alban.cre...@gmail.com) wrote:

> > Hmm, but what would you do for a service that has both PrivateNetwork
> > and chroot enabled?
> >
> > I am all open for shifting things around again, but I inda would
> > prefer a solution that works universally in the end...
> >
> > Ideas?
> >
> > I figure we could open a new mount namespace and mount the file system
> > socket into the chroot, but not sure I like the idea...
> 
> I don't know what is the best solution but using a socket file seems
> better than using an abstract unix socket: processes in a
> systemd-nspawn container could discover the notify socket of the host
> in /proc/net/unix (if it does not use a new network namespace) and
> send garbage file descriptors with SCM_RIGHTS from the container to
> the host. Systemd on the host does the right thing: it closes the fds
> when the datagram was not sent by a managed unit. 

Well, this is only an issue if people do not use network
namespacing. But if they don't use it they should not expect that much
isolation. It's the deal they make...

> Does
> manager_get_unit_by_pid() matches the exact cgroup path only or does
> it also matches a prefix path? I wonder about nspawn containers
> started by a service unit on the host.

No, it cares for subtree membership only. THat said, access can be
restricted via NotifyAccess=. If people set it to NotifyAccess=all
they of course need to know what they are doing...

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 nest slices under system.slice

2015-03-08 Thread Lennart Poettering 
On Thu, 05.03.15 13:55, Umut Tezduyar Lindskog (u...@tezduyar.com) wrote:

> Hi,
> 
> How do I add a slice that is inside the system.slice?

The name is basically the cgroup path. i.e. foo-bar-baz.slice is a
slice below foo-bar.slice, which is below foo.slice, which is below
-.slice.

> Following slice gets nested to -.slice where I want to nest it inside
> system.slice (just like instantaneous service slices).
> 
> hello.slice

Name it "system-hello.slice" and it will be below "system.slice".

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] Possible systemd segfault switching from 216 to 219 in fedora upgrade

2015-03-08 Thread Lennart Poettering 
On Thu, 05.03.15 22:07, James Hogarth (james.hoga...@gmail.com) wrote:

> > Tried to put together a reduced testcase via a yum installroot style
> > container to switch-root into to see what that behaviour is like and
> > do see a segfault - not certain if this is the same being seen during
> > the fedup switch-root though...
> >
> > Any ideas to get a better grasp on this?
> 
> So it's actually slightly more complicated than I had originally
> thought (thanks #fedora-qa) after a brief chat with wwoods.
> 
> The path taken in the process is the initrd used by fedup is built
> from the newer Fedora release (ie in the present testing this contains
> systemd-219).
> 
> This starts up and then carries out a switch-root to the actual system
> which in this case has systemd-216.

We don't support downgrades really. The reexec stuff should work fine
for upgrades, but downgrades is nothing we could even remotely test,
or even think/know about to work. fedup really shouldn't do that.

> The reason for this is to simplify finding out where mount points are
> for a clean upgrade - it's been felt the easiest way is to just 'ask'
> the actual system to do this.
> 
> After the mount points are all up switch-root is used to switch back
> to the initrd setup so that the upgrades can be carried out on teh
> non-running system... so we have a systemd-216 to 219 transition here.
> 
> This naturally means that the serialization/deserialization needs to
> be forwards *and* backwards compatible between 216 and 219 for this to
> work.

Yeah, but no. Allowing uprgades is one thing, allowing downgrades a
completely different one, and nothing we want to support.

> >From the logs that I've pulled (see the various attachments in
> https://bugzilla.redhat.com/show_bug.cgi?id=1185604 for them) it would
> appear the 219 -> 216 process is fine but then switching back from 216
> -> 219 is failing with the associated segfault.
> 
> There appears to be a couple of options here:
> 
> 1) Try to get a workable reduced test case or better debugging from
> the 216 -> 219 transition to work out why that is failing.
> 2) Have some sort of generator or call or similar that allows the
> systemd-newer in the initrd to parse the unit files and fstab of the
> installed system and carry out any mounting itself rather than using
> switch-root to the installed system and asking it to do so. This would
> then eliminate the jumping backwards and forwards between systemd
> versions during the upgrade process.

I am not really sure I follow here...

> Any thoughts on either of these options to try to get a way
> forwards... or is there any additional debugging or diagnostics that I
> can provide to help?

Well, it might be possible to get coredump out of the thing, by
disabling the core_pattern stuff, and first booting into init=/bin/sh,
then setting RLIMIT_CORE with ulimit in the shell, and then execing
systemd with the raised limit. THen, use gdb to extract the stack
trace from it?

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] Real-time permission affected by root login on other tty

2015-03-08 Thread Lennart Poettering 
On Sat, 07.03.15 20:04, Lars Christensen (lar...@belunktum.dk) wrote:

> With systemd 218 and 219, I have a strange issue that processes running as
> root cannot acquire realtime priority (SCHED_RR, priority 22). Further, if
> (and only if) root is logged in (via tty or ssh), users processes can also
> not acquire realtime priority. If root is NOT logged in, user processes CAN
> acquire real time priority(!).

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). 

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...).

THis is something that needs to be cleaned up in the kernel, and then
we can expose this nicer in systemd. For now, my recommendation would
be to disable the RT cgroup stuff in the kernel, and thus forego the
whole problem.

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] DBus api of systemd user instance

2015-03-08 Thread Lennart Poettering 
On Sat, 07.03.15 08:45, Mantas Mikulėnas (graw...@gmail.com) wrote:

> On Fri, Mar 6, 2015 at 6:23 PM, Ragnar Thomsen  wrote:
> 
> > Hey List,
> >
> > Does the user instance of systemd expose a dbus api?
> >
> 
> Yes, that's what `systemctl` uses.
> 
> 
> > If yes, how does one access it?
> >
> 
> Much like the system instance – either over the DBus "user" bus, or over
> the dedicated private socket ($XDG_RUNTIME_DIR/systemd/private).

The latter is "private", as the name suggests. Do not access it from
external programs, it is systemd's internal hack around ordering
issues with dbus, and nobody but systemd's own tools should access
it. It is going away when kdbus arrives, if you make use of it, then
your application will break.

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/3] Allow systemd-tmpfiles to set the file/directory attributes

2015-03-08 Thread Lennart Poettering 
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...

>  
>  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.

> +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.

> +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.

> +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.

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

> +}
> +for ( ; *p ; p++ ) {

Weird spaces...

> +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.

> +if (!attributes[i].ch) {
> +log_error("\"%s\": setting ATTR: unknown attr '%c'",
> +item->path, *p);

We don't break lines this eagerly in systemd.

> +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...

> +
> +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;
> +}

Please avoid using boolean checks for numeric values. 

But more importantly, why check this at all? Sounds completely Ok to
apply this to anything

> +#ifdef O_LARGEFILE
> +fd = open (path, O_RDONLY|O_NONBLOCK|O_LARGEFILE);
> +#else
> +fd = open (path, O_RDONLY|O_NONBLOCK);
> +#endif

systemd sets AC_SYS_LARGEFILE

Re: [systemd-devel] [PATCH] core/socket: Add REMOTE_IP environment variable for Accept=true

2015-03-08 Thread Lennart Poettering 
On Fri, 06.03.15 13:10, Shawn Landden (sh...@churchofgit.com) wrote:

> On Thu, Mar 5, 2015 at 3:18 AM, Lennart Poettering 
> wrote:
> 
> > On Wed, 04.03.15 15:18, Shawn Landden (sh...@churchofgit.com) wrote:
> >
> > Can't this just use getpeername_pretty()?
> >
> > Then I can't force it to only ipv4 and ipv6.

It wouldn't be too difficult to explicitly check for that before.

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] Reliably waiting for udevd to finish processing triggered events

2015-03-08 Thread Lennart Poettering 
On Fri, 06.03.15 14:22, Daniel Drake (dr...@endlessm.com) wrote:

> Hi,
> 
> I'm looking at some issues with the plymouth boot splash system, and
> why it intermittently fails to get graphics on screen.
> 
> plymouth watches for the creation of drm display devices during boot.
> If it finds one, it starts a graphical splash and that is that.
> However, if the system finishes loading drivers and no drm device is
> available, it falls back onto a fbdev-based splash or a text-based
> boot. Once it has made that choice there is no turning back, it
> basically ignores drm devices if they become available later.

To my knowledge newer versions don't do this anymore and actively
watch drm devices coming.

> Firstly, plymouth does the above when it loads in the initramfs. The
> initramfs will trigger udev events for all devices, but if systemd
> finds the root filesystem before plymouth finds the drm device, udevd
> is immediately killed by systemd as it changes to
> switch-root.target.
>
> udevd has not processed the drm device at this point, so
> udev_device_get_is_initialized() returns false when plymouth inquires.
> As udevd is killed, it removes /run/udev/queue in its exit path;
> plymouth sees this and (like udevsettle would) assumes that this
> apparently empty queue means that driver loading is complete. But no
> drm devices are available and initialized, so it falls back to textual
> boot for the rest of boot.
> 
> The killing of udev seems heavy-handed here, and the way it removes
> the queue file on exit (without first at least going through the
> already-pending events) seems to kill any possibility of a program
> like udevsettle or plymouth knowing if udev finished loading all
> drivers while the initramfs transitions to the real root.

No, applications should not watch the queue. And the file is internal
to udev anyway. If you watch it, you get to keep the pieces.

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] Improve log notice when unprivileged users run journalctl executable (reformatted)

2015-03-08 Thread Lennart Poettering 
On Sun, 08.03.15 10:56, Martin Pitt (martin.p...@ubuntu.com) wrote:

> Gautier Pelloux-Prayer [2015-03-08  9:33 +0100]:
> > This patch modifies this feedback by giving permission-hint to the user:
> > 
> > No journal files were found. Users in the 'systemd-journal' group
> > may access more messages.
> 
> If we do this, can we please advertise the standard LSB "adm" group
> instead of this systemd specific group? (IMHO it should have been
> "adm" right from the start, but that's harder to change now)

No, we introduced a group of its own for this for a reason. See the
NEWS section for version 198.

Lennart

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

[systemd-devel] Now that I have systemd, do I still need JSVC (Java daemon library)?

2015-03-08 Thread Tom Dworzanski 
Hello all, I’m learning system. I’m wondering if it’s a complete replacement 
for JSVC. I posted a detailed question with some sample configuration on a 
StackOverflow question: 
http://stackoverflow.com/questions/28894008/what-benefit-do-i-get-from-jsvc-over-just-using-systemd
 There weren't any answers so now I set it up with a 50 point bounty. 24 hours 
since the bounty still nothing. I’m writing to draw someone with knowledge of 
both to the question in hopes of getting an answer on this list or the site. 
Thank you in advance for anyone experienced enough to help. Tom Dworzanski



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

Re: [systemd-devel] [PATCH 2/2] po: update Russian translation - pluralize fsckd

2015-03-08 Thread Zbigniew Jędrzejewski-Szmek 
On Sun, Mar 08, 2015 at 06:51:41PM +0300, Sergey Ptashnick wrote:
> On 08.03.2015 17:46, Zbigniew Jędrzejewski-Szmek wrote:
> > On Sun, Mar 08, 2015 at 05:41:28PM +0300, Sergey Ptashnick wrote:
> >> Correctly pluralize strings for fsckd.
> > This does not apply: the context is changed. Can you rebase?
> > 
> > Zbyszek
> 
> Oops, sorry. Fixed.
Hm, I still see different line numbers. But since it's only a few lines,
I fixed it up by hand.

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

Re: [systemd-devel] [PATCH 2/2] po: update Russian translation - pluralize fsckd

2015-03-08 Thread Sergey Ptashnick 
On 08.03.2015 17:46, Zbigniew Jędrzejewski-Szmek wrote:
> On Sun, Mar 08, 2015 at 05:41:28PM +0300, Sergey Ptashnick wrote:
>> Correctly pluralize strings for fsckd.
> This does not apply: the context is changed. Can you rebase?
> 
> Zbyszek

Oops, sorry. Fixed.

Subject: [PATCH 2/2] po: update Russian translation - pluralize fsckd

Correctly pluralize strings for fsckd.
---
 po/ru.po |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/po/ru.po b/po/ru.po
index 72ecd6f..abd1264 100644
--- a/po/ru.po
+++ b/po/ru.po
@@ -7,12 +7,13 @@ msgstr ""
 "Project-Id-Version: systemd\n"
 "Report-Msgid-Bugs-To: 0comff...@inbox.ru\n"
 "POT-Creation-Date: 2013-03-24 19:22+0300\n"
-"PO-Revision-Date: 2015-03-02 00:25+0300\n"
+"PO-Revision-Date: 2015-03-08 17:15+0300\n"
 "Last-Translator: Sergey Ptashnick <0comff...@inbox.ru>\n"
 "Language: ru\n"
 "MIME-Version: 1.0\n"
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
+"Plural-Forms: nplurals=3; plural=(n%10==1 && n%100!=11 ? 0 : n%10>=2 && n%10<=4 && (n%100<10 || n%100>=20) ? 1 : 2)\n"
 
 #: ../src/hostname/org.freedesktop.hostname1.policy.in.h:1
 msgid "Set host name"
@@ -338,9 +339,11 @@ msgstr "Чтобы заставить systemd перечитать конфиг
 msgid "Press Ctrl+C to cancel all filesystem checks in progress"
 msgstr "Чтобы прервать все запущенные проверки файловых систем, нажмите Ctrl+C"
 
+# There is no difference between "на 2 дисках" (plural==1) and "на 5 дисках" (plural==2)
 #: ../src/fsckd/fsckd.c:227
 #, c-format
 msgid "Checking in progress on %d disk (%3.1f%% complete)"
 msgid_plural "Checking in progress on %d disks (%3.1f%% complete)"
 msgstr[0] "Проверяется целостность файловой системы на %d диске (выполнено %3.1f%%)"
 msgstr[1] "Проверяется целостность файловых систем на %d дисках (выполнено %3.1f%%)"
+msgstr[2] "Проверяется целостность файловых систем на %d дисках (выполнено %3.1f%%)"
-- 
1.7.2.5

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

Re: [systemd-devel] [PATCH] Improve log notice when unprivileged users run journalctl executable (reformatted)

2015-03-08 Thread Zbigniew Jędrzejewski-Szmek 
On Sun, Mar 08, 2015 at 03:31:25PM +0100, Zbigniew Jędrzejewski-Szmek wrote:
> On Sun, Mar 08, 2015 at 09:33:24AM +0100, Gautier Pelloux-Prayer wrote:
> > Hi list,
> > 
> > Currently, when user runs journalctl without extra privileges, output
> > is:
> > 
> > No journal files were found.
> > 
> > This patch modifies this feedback by giving permission-hint to the user:
> > 
> > No journal files were found. Users in the 'systemd-journal' group
> > may access more messages.
> > 
> > It should help new comers to understand that some extra privileges can
> > be useful to retrieve logs.
> The code in access_check() tries to distinguish the case where
> no files are resent and where the user is not allowed to access them.
> A message similar to what you are proposing appears just a few lines
> down.
Oh, I think I found the problem. Can you check again with the latest git?

Zbyszek

> If you're seeing this message journal files being present, then it
> means that the check is somehow wrong and should be fixed.
> 
> BTW., I now noticed the this logic needs to be updated for recent
> changes to ACL handling. If you do any changes, be sure to pull the
> latest git.
> 
> Zbyszek
> 
> > 
> > /Gautier
> > 
> > diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
> > index 56435ff..7f4c160 100644
> > --- a/src/journal/journalctl.c
> > +++ b/src/journal/journalctl.c
> > @@ -1581,12 +1581,13 @@ static int access_check(sd_journal *j) {
> >  Iterator it;
> >  void *code;
> >  int r = 0;
> > -
> > +bool is_privileged = (geteuid() == 0) ||
> > (in_group("systemd-journal") > 0);
> >  assert(j);
> >  
> >  if (set_isempty(j->errors)) {
> >  if (ordered_hashmap_isempty(j->files))
> > -log_notice("No journal files were found.");
> > +log_notice("No journal files were found.%s",
> > +  is_privileged?"":" Users in the
> > 'systemd-journal' group may access more messages.");
> >  return 0;
> >  }
> >  
> > @@ -1594,9 +1595,7 @@ static int access_check(sd_journal *j) {
> >  #ifdef HAVE_ACL
> >  /* If /var/log/journal doesn't even exist,
> >   * unprivileged users have no access at all */
> > -if (access("/var/log/journal", F_OK) < 0 &&
> > -geteuid() != 0 &&
> > -in_group("systemd-journal") <= 0) {
> > +if (access("/var/log/journal", F_OK) < 0 && !
> > is_privileged) {
> >  log_error("Unprivileged users cannot access
> > messages, unless persistent log storage is\n"
> >"enabled. Users in the
> > 'systemd-journal' group may always access messages.");
> >  return -EACCES;
> > @@ -1610,7 +1609,7 @@ static int access_check(sd_journal *j) {
> >  return r;
> >  }
> >  #else
> > -if (geteuid() != 0 && in_group("systemd-journal") <= 0)
> > {
> > +if (!is_privileged) {
> >  log_error("Unprivileged users cannot access
> > messages. Users in the 'systemd-journal' group\n"
> >"group may access messages.");
> >  return -EACCES;
> > 
> > 
> 
> > From 0f973d231d057866d8626e680b80bded24103af0 Mon Sep 17 00:00:00 2001
> > From: Gautier Pelloux-Prayer 
> > Date: Sun, 11 Jan 2015 12:00:18 +0100
> > Subject: [PATCH] Improve log notice when unprivileged users run journalctl
> >  executable
> > 
> > ---
> >  src/journal/journalctl.c | 11 +--
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
> > index 56435ff..7f4c160 100644
> > --- a/src/journal/journalctl.c
> > +++ b/src/journal/journalctl.c
> > @@ -1581,12 +1581,13 @@ static int access_check(sd_journal *j) {
> >  Iterator it;
> >  void *code;
> >  int r = 0;
> > -
> > +bool is_privileged = (geteuid() == 0) || 
> > (in_group("systemd-journal") > 0);
> >  assert(j);
> >  
> >  if (set_isempty(j->errors)) {
> >  if (ordered_hashmap_isempty(j->files))
> > -log_notice("No journal files were found.");
> > +log_notice("No journal files were found.%s",
> > +  is_privileged?"":" Users in the 
> > 'systemd-journal' group may access more messages.");
> >  return 0;
> >  }
> >  
> > @@ -1594,9 +1595,7 @@ static int access_check(sd_journal *j) {
> >  #ifdef HAVE_ACL
> >  /* If /var/log/journal doesn't even exist,
> >   * unprivileged users have no access at all */
> > -if (access("/var/log/journal", F_OK) < 0 &&
> > -geteuid() != 0 &&
> > -in_group("systemd-journal") <

Re: [systemd-devel] [PATCH 2/3] Update the man page of tmpfiles.d(5), to document the new h/H command.

2015-03-08 Thread Ronny Chevalier 
2015-03-08 12:48 GMT+01:00 Goffredo Baroncelli :
> From: Goffredo Baroncelli 
>
> Update the man page of tmpfiles.d(5), to document the new h/H command.
>
> Signed-off-by: Goffredo Baroncelli 

No Signed-off-by.

Also, why not merge the 3 commits in one ? I don't see why separating
the man page update of the new feature in another commit is useful ?

> ---
>  man/tmpfiles.d.xml | 32 
>  1 file changed, 32 insertions(+)
>
> diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
> index 8815bf9..f9074dd 100644
> --- a/man/tmpfiles.d.xml
> +++ b/man/tmpfiles.d.xml
> @@ -303,6 +303,37 @@
>  
>
>  
> +  h
> +  Set file/directory attributes. Lines of this type
> +  accept shell-style globs in place of normal path names.
> +
> +  The format of agrument field is 
> [+-=][aAcCdDeijsStTu]

the argument*

> +  
> +
> +  The prefix + causes the
> +  attribute(s) to be added; - causes the
> +  attribute(s) to be removed; =
> +  causes the attributes to set exactly as the following 
> letters.
> +  The letters 'aAcCdDeijsStTu' select the new
> +  attributes for the files, see
> +  chattr
> +  1 for further information.
> +  
> +  Passing only = as argument,
> +  reset all the file attributes.
> +
> +  
> +
> +
> +
> +  H
> +  Recursively set file/directory attributes Lines

A . is missing before Lines.

> +  of this type accept shell-style globs in place of normal
> +  path names.
> +  
> +
> +
> +
>a
>a+
>Set POSIX ACLs (access control lists). If
> @@ -529,6 +560,7 @@
> project='man-pages'>setfattr1,
> project='man-pages'>setfacl1,
> project='man-pages'>getfacl1
> +   project='man-pages'>chattr1
>  
>
>
> --
> 2.1.4
>
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 1/2] po: update Russian translation - importd

2015-03-08 Thread Zbigniew Jędrzejewski-Szmek 
On Sun, Mar 08, 2015 at 05:41:23PM +0300, Sergey Ptashnick wrote:
> Add strings for importd, by analogy with 1eb37584a8.
Applied 1/2.

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

Re: [systemd-devel] [PATCH 2/2] po: update Russian translation - pluralize fsckd

2015-03-08 Thread Zbigniew Jędrzejewski-Szmek 
On Sun, Mar 08, 2015 at 05:41:28PM +0300, Sergey Ptashnick wrote:
> Correctly pluralize strings for fsckd.
This does not apply: the context is changed. Can you rebase?

Zbyszek


> ---
>  po/ru.po |5 -
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/po/ru.po b/po/ru.po
> index 72ecd6f..abd1264 100644
> --- a/po/ru.po
> +++ b/po/ru.po
> @@ -7,12 +7,13 @@ msgstr ""
>  "Project-Id-Version: systemd\n"
>  "Report-Msgid-Bugs-To: 0comff...@inbox.ru\n"
>  "POT-Creation-Date: 2013-03-24 19:22+0300\n"
> -"PO-Revision-Date: 2015-02-19 03:27+0300\n"
> +"PO-Revision-Date: 2015-03-08 17:15+0300\n"
>  "Last-Translator: Sergey Ptashnick <0comff...@inbox.ru>\n"
>  "Language: ru\n"
>  "MIME-Version: 1.0\n"
>  "Content-Type: text/plain; charset=UTF-8\n"
>  "Content-Transfer-Encoding: 8bit\n"
> +"Plural-Forms: nplurals=3; plural=(n%10==1 && n%100!=11 ? 0 : n%10>=2 && 
> n%10<=4 && (n%100<10 || n%100>=20) ? 1 : 2)\n"
>  
>  #: ../src/hostname/org.freedesktop.hostname1.policy.in.h:1
>  msgid "Set host name"
> @@ -338,9 +339,11 @@ msgstr "Чтобы заставить systemd перечитать конфиг
>  msgid "Press Ctrl+C to cancel all filesystem checks in progress"
>  msgstr "Чтобы прервать все запущенные проверки файловых систем, нажмите 
> Ctrl+C"
>  
> +# There is no difference between "на 2 дисках" (plural==1) and "на 5 дисках" 
> (plural==2)
>  #: ../src/fsckd/fsckd.c:227
>  #, c-format
>  msgid "Checking in progress on %d disk (%3.1f%% complete)"
>  msgid_plural "Checking in progress on %d disks (%3.1f%% complete)"
>  msgstr[0] "Проверяется целостность файловой системы на %d диске (выполнено 
> %3.1f%%)"
>  msgstr[1] "Проверяется целостность файловых систем на %d дисках (выполнено 
> %3.1f%%)"
> +msgstr[2] "Проверяется целостность файловых систем на %d дисках (выполнено 
> %3.1f%%)"
> -- 
> 1.7.2.5
> ___
> systemd-devel mailing list
> systemd-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 1/2] po: update Russian translation - importd

2015-03-08 Thread Sergey Ptashnick 
Add strings for importd, by analogy with 1eb37584a8.
---
 po/ru.po |   10 +-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/po/ru.po b/po/ru.po
index 1ffeec6..72ecd6f 100644
--- a/po/ru.po
+++ b/po/ru.po
@@ -39,10 +39,18 @@ msgid "Authentication is required to set local machine 
information."
 msgstr "Чтобы настроить информацию о компьютере, необходимо пройти 
аутентификацию."
 
 #: ../src/import/org.freedesktop.import1.policy.in.h:1
+msgid "Import a VM or container image"
+msgstr "Импортировать образ виртуальной машины или контейнера"
+
+#: ../src/import/org.freedesktop.import1.policy.in.h:2
+msgid "Authentication is required to import a VM or container image"
+msgstr "Чтобы импортировать образ виртуальной машины или контейнера, 
необходимо пройти аутентификацию."
+
+#: ../src/import/org.freedesktop.import1.policy.in.h:3
 msgid "Download a VM or container image"
 msgstr "Загрузить образ виртуальной машины или контейнера"
 
-#: ../src/import/org.freedesktop.import1.policy.in.h:2
+#: ../src/import/org.freedesktop.import1.policy.in.h:4
 msgid "Authentication is required to download a VM or container image"
 msgstr "Чтобы загрузить образ виртуальной машины или контейнера, необходимо 
пройти аутентификацию."
 
-- 
1.7.2.5
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/2] po: update Russian translation - pluralize fsckd

2015-03-08 Thread Sergey Ptashnick 
Correctly pluralize strings for fsckd.
---
 po/ru.po |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/po/ru.po b/po/ru.po
index 72ecd6f..abd1264 100644
--- a/po/ru.po
+++ b/po/ru.po
@@ -7,12 +7,13 @@ msgstr ""
 "Project-Id-Version: systemd\n"
 "Report-Msgid-Bugs-To: 0comff...@inbox.ru\n"
 "POT-Creation-Date: 2013-03-24 19:22+0300\n"
-"PO-Revision-Date: 2015-02-19 03:27+0300\n"
+"PO-Revision-Date: 2015-03-08 17:15+0300\n"
 "Last-Translator: Sergey Ptashnick <0comff...@inbox.ru>\n"
 "Language: ru\n"
 "MIME-Version: 1.0\n"
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
+"Plural-Forms: nplurals=3; plural=(n%10==1 && n%100!=11 ? 0 : n%10>=2 && 
n%10<=4 && (n%100<10 || n%100>=20) ? 1 : 2)\n"
 
 #: ../src/hostname/org.freedesktop.hostname1.policy.in.h:1
 msgid "Set host name"
@@ -338,9 +339,11 @@ msgstr "Чтобы заставить systemd перечитать конфиг
 msgid "Press Ctrl+C to cancel all filesystem checks in progress"
 msgstr "Чтобы прервать все запущенные проверки файловых систем, нажмите Ctrl+C"
 
+# There is no difference between "на 2 дисках" (plural==1) and "на 5 дисках" 
(plural==2)
 #: ../src/fsckd/fsckd.c:227
 #, c-format
 msgid "Checking in progress on %d disk (%3.1f%% complete)"
 msgid_plural "Checking in progress on %d disks (%3.1f%% complete)"
 msgstr[0] "Проверяется целостность файловой системы на %d диске (выполнено 
%3.1f%%)"
 msgstr[1] "Проверяется целостность файловых систем на %d дисках (выполнено 
%3.1f%%)"
+msgstr[2] "Проверяется целостность файловых систем на %d дисках (выполнено 
%3.1f%%)"
-- 
1.7.2.5
___
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

2015-03-08 Thread Zbigniew Jędrzejewski-Szmek 
On Sun, Mar 08, 2015 at 03:00:38PM +0100, Ronny Chevalier wrote:
> 2015-03-08 12:48 GMT+01:00 Goffredo Baroncelli :
> > From: Goffredo Baroncelli 
> >
> 
> Hi,
> 
> > 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.
> >
> > Signed-off-by: Goffredo Baroncelli 
> 
> No Signed-off-by in systemd.
> 
> > ---
> >  src/tmpfiles/tmpfiles.c | 155 
> > 
> >  1 file changed, 155 insertions(+)
> >
> > diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
> > index c948d4d..cb77047 100644
> > --- a/src/tmpfiles/tmpfiles.c
> > +++ b/src/tmpfiles/tmpfiles.c
> > @@ -40,6 +40,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  #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,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; } ;
> > +static struct attributes_list_t attributes[] = {
Also const.

> > +{ FS_NOATIME_FL, 'A' },   /* do not update atime */
This indentation is excessive.

> > +{ 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;
> > +int value=0, mask=0;
> > +
> > +if (!p) {
> > +log_error("\"%s\": setting ATTR need an argument", 
> > item->path);
> > +return -1;
> 
> Please use errno style error code. In this case -EINVAL is appropriate.
> 
> > +}
> > +
> > +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;
> 
> Same here.
> 
> > +}
> > +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 -2;
> 
> Same here.
> 
> > +}
> > +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);

Re: [systemd-devel] [PATCH 3/3] Add a new tmpfiles.d snippets to set the NOCOW the journal.

2015-03-08 Thread Zbigniew Jędrzejewski-Szmek 
On Sun, Mar 08, 2015 at 12:48:27PM +0100, Goffredo Baroncelli wrote:
> From: Goffredo Baroncelli 
> 
> 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 corruputed journal.
> 
> Signed-off-by: Goffredo Baroncelli 
Please drop the SOB.

> +# set the journal file as NOCOW; only valid for BTRFS filesystem
> +h /var/log/journal/%m - - - - +C
Shouldn't this be 'H'?

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

Re: [systemd-devel] [PATCH] Improve log notice when unprivileged users run journalctl executable (reformatted)

2015-03-08 Thread Zbigniew Jędrzejewski-Szmek 
On Sun, Mar 08, 2015 at 09:33:24AM +0100, Gautier Pelloux-Prayer wrote:
> Hi list,
> 
> Currently, when user runs journalctl without extra privileges, output
> is:
> 
> No journal files were found.
> 
> This patch modifies this feedback by giving permission-hint to the user:
> 
> No journal files were found. Users in the 'systemd-journal' group
> may access more messages.
> 
> It should help new comers to understand that some extra privileges can
> be useful to retrieve logs.
The code in access_check() tries to distinguish the case where
no files are resent and where the user is not allowed to access them.
A message similar to what you are proposing appears just a few lines
down.

If you're seeing this message journal files being present, then it
means that the check is somehow wrong and should be fixed.

BTW., I now noticed the this logic needs to be updated for recent
changes to ACL handling. If you do any changes, be sure to pull the
latest git.

Zbyszek

> 
> /Gautier
> 
> diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
> index 56435ff..7f4c160 100644
> --- a/src/journal/journalctl.c
> +++ b/src/journal/journalctl.c
> @@ -1581,12 +1581,13 @@ static int access_check(sd_journal *j) {
>  Iterator it;
>  void *code;
>  int r = 0;
> -
> +bool is_privileged = (geteuid() == 0) ||
> (in_group("systemd-journal") > 0);
>  assert(j);
>  
>  if (set_isempty(j->errors)) {
>  if (ordered_hashmap_isempty(j->files))
> -log_notice("No journal files were found.");
> +log_notice("No journal files were found.%s",
> +  is_privileged?"":" Users in the
> 'systemd-journal' group may access more messages.");
>  return 0;
>  }
>  
> @@ -1594,9 +1595,7 @@ static int access_check(sd_journal *j) {
>  #ifdef HAVE_ACL
>  /* If /var/log/journal doesn't even exist,
>   * unprivileged users have no access at all */
> -if (access("/var/log/journal", F_OK) < 0 &&
> -geteuid() != 0 &&
> -in_group("systemd-journal") <= 0) {
> +if (access("/var/log/journal", F_OK) < 0 && !
> is_privileged) {
>  log_error("Unprivileged users cannot access
> messages, unless persistent log storage is\n"
>"enabled. Users in the
> 'systemd-journal' group may always access messages.");
>  return -EACCES;
> @@ -1610,7 +1609,7 @@ static int access_check(sd_journal *j) {
>  return r;
>  }
>  #else
> -if (geteuid() != 0 && in_group("systemd-journal") <= 0)
> {
> +if (!is_privileged) {
>  log_error("Unprivileged users cannot access
> messages. Users in the 'systemd-journal' group\n"
>"group may access messages.");
>  return -EACCES;
> 
> 

> From 0f973d231d057866d8626e680b80bded24103af0 Mon Sep 17 00:00:00 2001
> From: Gautier Pelloux-Prayer 
> Date: Sun, 11 Jan 2015 12:00:18 +0100
> Subject: [PATCH] Improve log notice when unprivileged users run journalctl
>  executable
> 
> ---
>  src/journal/journalctl.c | 11 +--
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
> index 56435ff..7f4c160 100644
> --- a/src/journal/journalctl.c
> +++ b/src/journal/journalctl.c
> @@ -1581,12 +1581,13 @@ static int access_check(sd_journal *j) {
>  Iterator it;
>  void *code;
>  int r = 0;
> -
> +bool is_privileged = (geteuid() == 0) || 
> (in_group("systemd-journal") > 0);
>  assert(j);
>  
>  if (set_isempty(j->errors)) {
>  if (ordered_hashmap_isempty(j->files))
> -log_notice("No journal files were found.");
> +log_notice("No journal files were found.%s",
> +  is_privileged?"":" Users in the 'systemd-journal' 
> group may access more messages.");
>  return 0;
>  }
>  
> @@ -1594,9 +1595,7 @@ static int access_check(sd_journal *j) {
>  #ifdef HAVE_ACL
>  /* If /var/log/journal doesn't even exist,
>   * unprivileged users have no access at all */
> -if (access("/var/log/journal", F_OK) < 0 &&
> -geteuid() != 0 &&
> -in_group("systemd-journal") <= 0) {
> +if (access("/var/log/journal", F_OK) < 0 && !is_privileged) {
>  log_error("Unprivileged users cannot access 
> messages, unless persistent log storage is\n"
>"enabled. Users in the 'systemd-journal' 
> group may always access messages.");
>  return -EACCES;
> @@ -1610,7 +1609,7 @@ static

Re: [systemd-devel] [PATCH 1/3] Allow systemd-tmpfiles to set the file/directory attributes

2015-03-08 Thread Ronny Chevalier 
2015-03-08 12:48 GMT+01:00 Goffredo Baroncelli :
> From: Goffredo Baroncelli 
>

Hi,

> 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.
>
> Signed-off-by: Goffredo Baroncelli 

No Signed-off-by in systemd.

> ---
>  src/tmpfiles/tmpfiles.c | 155 
> 
>  1 file changed, 155 insertions(+)
>
> diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
> index c948d4d..cb77047 100644
> --- a/src/tmpfiles/tmpfiles.c
> +++ b/src/tmpfiles/tmpfiles.c
> @@ -40,6 +40,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #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,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; } ;
> +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 }
> +};
> +char *p = item->argument;
> +enum { MODE_ADD, MODE_DEL, MODE_SET } mode = MODE_ADD;
> +int value=0, mask=0;
> +
> +if (!p) {
> +log_error("\"%s\": setting ATTR need an argument", 
> item->path);
> +return -1;

Please use errno style error code. In this case -EINVAL is appropriate.

> +}
> +
> +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;

Same here.

> +}
> +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 -2;

Same here.

> +}
> +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 &= ~mask;
> +item->attrib_value |= value;
> +item->attrib_set = true;
> +
> +

Useless newline.

> +return 0;
> +
> +}
> +
> +static int path_set_attrib(Item *item, const char *path) {
> +int fd, r, f;
> +struct stat st;
> +
> +/* do nothing */
> +if (item->attrib_mask == 0 || !i

[systemd-devel] [PATCH 2/3] Update the man page of tmpfiles.d(5), to document the new h/H command.

2015-03-08 Thread Goffredo Baroncelli 
From: Goffredo Baroncelli 

Update the man page of tmpfiles.d(5), to document the new h/H command.

Signed-off-by: Goffredo Baroncelli 
---
 man/tmpfiles.d.xml | 32 
 1 file changed, 32 insertions(+)

diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index 8815bf9..f9074dd 100644
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -303,6 +303,37 @@
 
 
 
+  h
+  Set file/directory attributes. Lines of this type
+  accept shell-style globs in place of normal path names.
+
+  The format of agrument field is [+-=][aAcCdDeijsStTu]
+  
+
+  The prefix + causes the
+  attribute(s) to be added; - causes the
+  attribute(s) to be removed; =
+  causes the attributes to set exactly as the following letters.
+  The letters 'aAcCdDeijsStTu' select the new
+  attributes for the files, see
+  chattr
+  1 for further information.
+  
+  Passing only = as argument,
+  reset all the file attributes.
+
+  
+
+
+
+  H
+  Recursively set file/directory attributes Lines
+  of this type accept shell-style globs in place of normal
+  path names.
+  
+
+
+
   a
   a+
   Set POSIX ACLs (access control lists). If
@@ -529,6 +560,7 @@
   setfattr1,
   setfacl1,
   getfacl1
+  chattr1
 
   
 
-- 
2.1.4

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

[systemd-devel] [PATCH 3/3] Add a new tmpfiles.d snippets to set the NOCOW the journal.

2015-03-08 Thread Goffredo Baroncelli 
From: Goffredo Baroncelli 

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 corruputed journal.

Signed-off-by: Goffredo Baroncelli 
---
 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

[systemd-devel] [PATCH 1/3] Allow systemd-tmpfiles to set the file/directory attributes

2015-03-08 Thread Goffredo Baroncelli 
From: Goffredo Baroncelli 

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.

Signed-off-by: Goffredo Baroncelli 
---
 src/tmpfiles/tmpfiles.c | 155 
 1 file changed, 155 insertions(+)

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index c948d4d..cb77047 100644
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #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,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; } ;
+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 }
+};
+char *p = item->argument;
+enum { MODE_ADD, MODE_DEL, MODE_SET } mode = MODE_ADD;
+int value=0, mask=0;
+
+if (!p) {
+log_error("\"%s\": setting ATTR need an argument", item->path);
+return -1;
+}
+
+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;
+}
+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 -2;
+}
+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 &= ~mask;
+item->attrib_value |= value;
+item->attrib_set = true;
+
+
+return 0;
+
+}
+
+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;
+}
+#ifdef O_LARGEFILE
+fd = open (path, O_RDONLY|O_NONBLOCK|O_LARGEFILE);
+#else
+fd = open (path, O_RDONLY|O_NONBLOCK);
+#endif
+if (fd == -1) {
+log_error_errno(errno, "Cannot open \"%s\": %m", path);
+return -1;
+}
+r = ioctl

[systemd-devel] [PATCH] Allow systemd-tmpfiles to set file/directory attributes

2015-03-08 Thread Goffredo Baroncelli 

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', 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

[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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] Improve log notice when unprivileged users run journalctl executable (reformatted)

2015-03-08 Thread Tomasz Torcz 
On Sun, Mar 08, 2015 at 10:56:25AM +0100, Martin Pitt wrote:
> Gautier Pelloux-Prayer [2015-03-08  9:33 +0100]:
> > This patch modifies this feedback by giving permission-hint to the user:
> > 
> > No journal files were found. Users in the 'systemd-journal' group
> > may access more messages.
> 
> If we do this, can we please advertise the standard LSB "adm" group
> instead of this systemd specific group? (IMHO it should have been
> "adm" right from the start, but that's harder to change now)

  It was until 
http://cgit.freedesktop.org/systemd/systemd/commit/?id=a24c64f03f9c5c0304451d8542fee853187a5168

-- 
Tomasz Torcz "God, root, what's the difference?"
xmpp: zdzich...@chrome.pl "God is more forgiving."

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

Re: [systemd-devel] [PATCH] Improve log notice when unprivileged users run journalctl executable (reformatted)

2015-03-08 Thread Martin Pitt 
Gautier Pelloux-Prayer [2015-03-08  9:33 +0100]:
> This patch modifies this feedback by giving permission-hint to the user:
> 
> No journal files were found. Users in the 'systemd-journal' group
> may access more messages.

If we do this, can we please advertise the standard LSB "adm" group
instead of this systemd specific group? (IMHO it should have been
"adm" right from the start, but that's harder to change now)

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] Improve log notice when unprivileged users run journalctl executable (reformatted)

2015-03-08 Thread Gautier Pelloux-Prayer 
Hi list,

Currently, when user runs journalctl without extra privileges, output
is:

No journal files were found.

This patch modifies this feedback by giving permission-hint to the user:

No journal files were found. Users in the 'systemd-journal' group
may access more messages.

It should help new comers to understand that some extra privileges can
be useful to retrieve logs.

/Gautier

diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index 56435ff..7f4c160 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -1581,12 +1581,13 @@ static int access_check(sd_journal *j) {
 Iterator it;
 void *code;
 int r = 0;
-
+bool is_privileged = (geteuid() == 0) ||
(in_group("systemd-journal") > 0);
 assert(j);
 
 if (set_isempty(j->errors)) {
 if (ordered_hashmap_isempty(j->files))
-log_notice("No journal files were found.");
+log_notice("No journal files were found.%s",
+  is_privileged?"":" Users in the
'systemd-journal' group may access more messages.");
 return 0;
 }
 
@@ -1594,9 +1595,7 @@ static int access_check(sd_journal *j) {
 #ifdef HAVE_ACL
 /* If /var/log/journal doesn't even exist,
  * unprivileged users have no access at all */
-if (access("/var/log/journal", F_OK) < 0 &&
-geteuid() != 0 &&
-in_group("systemd-journal") <= 0) {
+if (access("/var/log/journal", F_OK) < 0 && !
is_privileged) {
 log_error("Unprivileged users cannot access
messages, unless persistent log storage is\n"
   "enabled. Users in the
'systemd-journal' group may always access messages.");
 return -EACCES;
@@ -1610,7 +1609,7 @@ static int access_check(sd_journal *j) {
 return r;
 }
 #else
-if (geteuid() != 0 && in_group("systemd-journal") <= 0)
{
+if (!is_privileged) {
 log_error("Unprivileged users cannot access
messages. Users in the 'systemd-journal' group\n"
   "group may access messages.");
 return -EACCES;


From 0f973d231d057866d8626e680b80bded24103af0 Mon Sep 17 00:00:00 2001
From: Gautier Pelloux-Prayer 
Date: Sun, 11 Jan 2015 12:00:18 +0100
Subject: [PATCH] Improve log notice when unprivileged users run journalctl
 executable

---
 src/journal/journalctl.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index 56435ff..7f4c160 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -1581,12 +1581,13 @@ static int access_check(sd_journal *j) {
 Iterator it;
 void *code;
 int r = 0;
-
+bool is_privileged = (geteuid() == 0) || (in_group("systemd-journal") > 0);
 assert(j);
 
 if (set_isempty(j->errors)) {
 if (ordered_hashmap_isempty(j->files))
-log_notice("No journal files were found.");
+log_notice("No journal files were found.%s",
+  is_privileged?"":" Users in the 'systemd-journal' group may access more messages.");
 return 0;
 }
 
@@ -1594,9 +1595,7 @@ static int access_check(sd_journal *j) {
 #ifdef HAVE_ACL
 /* If /var/log/journal doesn't even exist,
  * unprivileged users have no access at all */
-if (access("/var/log/journal", F_OK) < 0 &&
-geteuid() != 0 &&
-in_group("systemd-journal") <= 0) {
+if (access("/var/log/journal", F_OK) < 0 && !is_privileged) {
 log_error("Unprivileged users cannot access messages, unless persistent log storage is\n"
   "enabled. Users in the 'systemd-journal' group may always access messages.");
 return -EACCES;
@@ -1610,7 +1609,7 @@ static int access_check(sd_journal *j) {
 return r;
 }
 #else
-if (geteuid() != 0 && in_group("systemd-journal") <= 0) {
+if (!is_privileged) {
 log_error("Unprivileged users cannot access messages. Users in the 'systemd-journal' group\n"
   "group may access messages.");
 return -EACCES;
-- 
2.1.4

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