[systemd-devel] [PATCH 2/3] Ignore the setting SELinuxContext if selinux is not enabled

2014-02-06 Thread Michael Scherer 
---
 src/core/execute.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/core/execute.c b/src/core/execute.c
index c02c768..474a4af 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -1569,7 +1569,7 @@ int exec_spawn(ExecCommand *command,
 }
 }
 #ifdef HAVE_SELINUX
-if (context-selinux_context) {
+if (context-selinux_context  use_selinux()) {
 err = 
security_check_context(context-selinux_context);
 if (err  0) {
 r = EXIT_SELINUX_CONTEXT;
-- 
1.8.5.3

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

Re: [systemd-devel] Howto run systemd within a linux container

2014-02-06 Thread Daniel P. Berrange 
On Wed, Feb 05, 2014 at 11:44:33PM +0100, Richard Weinberger wrote:
 Hi!
 
 We're heavily using Linux containers in our production environment.
 As modern Linux distributions move forward to systemd have to make sure that
 systemd works within our containers.
 
 Sadly we're facing issues with cgroups.
 Our testbed consists of openSUSE 13.1 with Linux 3.13.1 and libvirt 1.2.1.
 
 In a plain setup systemd stops immediately because it is unable to
 create the cgroup hierarchy.
 Mostly because the container uid 0 is in a user namespace and has no
 rights to do that.

FYI I have succesfully run Fedora 19 with systemd inside a container
with libvirt LXC, however, I did *not* enable user namespaces. Every
time I try user namespaces I find some other bug in either the kernel
or libvirt, so I wouldn't be surprised if yet more breakage has
occurred in user namepsaces :-(


 Next try, fool systemd by mounting a tmpfs to /sys/fs/cgroup/systemd/.
 This seems to work. openSUSE boots, I can start/stop services...
 Shutdown hangs forever, had no time to investigate so far.
 
 But is this tmpfs hack the correct way to run systemd in a container?
 I really don't think so.

Yeah that really shouldnt' be needed. When libvirt runs a container it
creates a cgroup just for that container to run in, and systemd should
be able to create its hierarchy under that location.

That said, I wonder if libvirt is perhaps forgetting to chown() the
cgroup to the UID/GID you've mapped for the root user. That would
certainly prevent systemd using it and could cause the sort of pain
you see.


Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] Howto run systemd within a linux container

2014-02-06 Thread Daniel P. Berrange 
On Thu, Feb 06, 2014 at 04:33:22PM +0100, Greg KH wrote:
 On Thu, Feb 06, 2014 at 10:55:01AM +, Daniel P. Berrange wrote:
  On Wed, Feb 05, 2014 at 11:44:33PM +0100, Richard Weinberger wrote:
   Hi!
   
   We're heavily using Linux containers in our production environment.
   As modern Linux distributions move forward to systemd have to make sure 
   that
   systemd works within our containers.
   
   Sadly we're facing issues with cgroups.
   Our testbed consists of openSUSE 13.1 with Linux 3.13.1 and libvirt 1.2.1.
   
   In a plain setup systemd stops immediately because it is unable to
   create the cgroup hierarchy.
   Mostly because the container uid 0 is in a user namespace and has no
   rights to do that.
  
  FYI I have succesfully run Fedora 19 with systemd inside a container
  with libvirt LXC, however, I did *not* enable user namespaces. Every
  time I try user namespaces I find some other bug in either the kernel
  or libvirt, so I wouldn't be surprised if yet more breakage has
  occurred in user namepsaces :-(
 
 Those bugs should now be fixed, if you don't enable the option, how are
 we supposed to know what is left to be done?  :)

I have in fact been building my own kernels for Fedora with user namespaces
enabled to debug / test this and have reported all the bugs I found so far.
Just saying that with the track record of bugs since the userns code first
merged, I wouldn't be surprised if there were still more things to iron
out as we try more real world apps like systemd.

Regads,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] tty: Set correct tty name in 'active' sysfs attribute

2014-02-06 Thread Hannes Reinecke 
On 02/05/2014 01:53 PM, David Herrmann wrote:
 Hi
 
 On Wed, Feb 5, 2014 at 11:11 AM, Hannes Reinecke h...@suse.de wrote:
 The 'active' sysfs attribute should refer to the currently
 active tty devices the console is running on, not the currently
 active console.
 The console structure doesn't refer to any device in sysfs,
 only the tty the console is running on has.
 So we need to print out the tty names in 'active', not
 the console names.

 Cc: Lennart Poettering lenn...@poettering.net
 Cc: Kay Sievers k...@vrfy.org
 Signed-off-by: Werner Fink wer...@suse.de
 Signed-off-by: Hannes Reinecke h...@suse.de
 ---
  drivers/tty/tty_io.c | 14 --
  1 file changed, 12 insertions(+), 2 deletions(-)

 diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
 index c74a00a..17db8ca 100644
 --- a/drivers/tty/tty_io.c
 +++ b/drivers/tty/tty_io.c
 @@ -3545,9 +3545,19 @@ static ssize_t show_cons_active(struct device *dev,
 if (i = ARRAY_SIZE(cs))
 break;
 }
 -   while (i--)
 +   while (i--) {
 +   const struct tty_driver *driver;
 +   const char *name = cs[i]-name;
 +   int index = cs[i]-index;
 +
 +   driver = cs[i]-device(cs[i], index);
 +   if (driver) {
 +   index += driver-name_base;
 +   name = driver-name;
 +   }
 count += sprintf(buf + count, %s%d%c,
 -cs[i]-name, cs[i]-index, i ? ' ':'\n');
 +name, index, i ? ' ':'\n');
 +   }
 
 Nice catch and indeed, systemd already relies on these names to be
 identical to their char-dev name. Fortunately, VTs and most serial
 devices register the console with the same name as the TTY, so we're
 fine.
 Two minor nitpicks:
 1) Could you use tty_line_name() instead of sprintf()? It's in the
 same file and avoids duplicating the name_base logic.
Ok. Not that it makes the patch nicer, but hey.

 2) Does it make sense to print the console-name if -device() returns
 NULL? Seems weird if we print console-names and tty-names in the same
 attribute. It's unlikely that it causes problems, but there might be
 conflicts.
 
This is basically a fallback; this is the old behaviour, which still
might be called for when coming across a tty which just has a stub
for the -device callback.
It's not that the '-device' callback is used that frequently, so I
wouldn't be surprised here.

Meanwhile I've sent a new patch, reviews are welcome there.

Cheers,

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

Re: [systemd-devel] [PATCH] udev: Adding zram to inappropriate block device

2014-02-06 Thread Greg KH 
On Thu, Feb 06, 2014 at 03:58:59AM +0100, Zbigniew Jędrzejewski-Szmek wrote:
 On Thu, Feb 06, 2014 at 03:27:07AM +0100, Greg KH wrote:
  On Thu, Feb 06, 2014 at 01:31:37AM +0100, Zbigniew Jędrzejewski-Szmek wrote:
   Patch applied.
   
   On Mon, Feb 03, 2014 at 10:33:37AM +, Jóhann B. Guðmundsson wrote:
On 02/03/2014 09:36 AM, Holger Schurig wrote:
with unit type ending in .zswap
No, not another unit type. Instead better amend .swap unit types to
also know about ZRAM.

However, isn't this a bit early? Shouldn't move ZRAM first move out of 
staging?

Ofcourse but when it does move out of staging we could have sorted
this implementation detail out which basically boils down to where
to set the partition sizes for the zram partitions (
tmpfiles.d/zram-conf or /etc/zram.d/zram-conf )

Do we want a script that basically just set this size based on
available memory per core in the udev rule.

factor=25
num_cpus=$(grep -c processor /proc/cpuinfo)
memtotal=$(grep MemTotal /proc/meminfo | sed 's/[^0-9]\+//g')
mem_by_cpu=$(($memtotal/$num_cpus*$factor/100*1024))
echo $mem_by_cpu
   
   [Side note: I'm reading Documentation/blockdev/zram.txt...
   It says 1. modprobe zram num_devices=4. I can't help thinking that
   we went over this with /dev/loop-control and others... Since this
   is a new interface, why does it repeat the same pitfall of not
   having a control device?]
  
  We can always change this now, quick, before 3.14 is out.
  
  What would a control device help with here?
 Right now you have to decide before loading the module how many
 devices you want. And also when trying to use a device (any device),
 you have to look for one. The same issues as with loop.

Given that the code doesn't have the ability to dynamically add/remove
devices, I think we are stuck with this, right?

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

Re: [systemd-devel] [PATCHv2] tty: Set correct tty name in 'active' sysfs attribute

2014-02-06 Thread Hannes Reinecke 
On 02/06/2014 04:29 PM, Greg Kroah-Hartman wrote:
 On Thu, Feb 06, 2014 at 03:27:43PM +0100, Hannes Reinecke wrote:
 The 'active' sysfs attribute should refer to the currently
 active tty devices the console is running on, not the currently
 active console.
 
 That's not what Documentation/ABI/sysfs-tty says:
  Shows the list of currently configured   
   
  console devices, like 'tty1 ttyS0'.  
   
  The last entry in the file is the active 
   
  device connected to /dev/console.
   
  The file supports poll() to detect virtual   
   
  console switches. 
 
The problem is indeed with 'console devices'. There is no such
thing; you only have tty devices where the console is running on.

 The console structure doesn't refer to any device in sysfs,
 only the tty the console is running on has.
 
 That sentance doesn't make sense.
 
 So we need to print out the tty names in 'active', not
 the console names.
 
 But that doesn't match the documentation.
 
 What exactly are you trying to fix here?  What is the problem that the
 current file has that is broken?  And as you are changing what this file
 means, what will break if the information in the file changes?
 
systemd is using the 'active' sysfs attribute to figure out on which
_tty_ device to start a getty on.
As soon as the console name and the tty name are different
you have no means of figuring out which _device_ to open.
AFAICS the console 'device' (ie the current entry in 'active')
doesn't have _any_ equivalent in sysfs; it just so happens that for
most console drivers the tty driver name is identical.
But this is not a requirement, and fails for drivers which have a
different device for the console and the tty.

EG on S/390 the 3270 tty has the devices

/dev/3270/tty1

but the console driver announces the name 'tty3270'.
So as per current rules the 'active' attribute contains

tty32700

which correct as per documentation, but doesn't have _any_
equivalent in sysfs.

Martin has the grubby details here.

But of course, the documentation should be updated to match the new
behavior.

Cheers,

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

Re: [systemd-devel] [PATCHv2] tty: Set correct tty name in 'active' sysfs attribute

2014-02-06 Thread Greg Kroah-Hartman 
On Thu, Feb 06, 2014 at 04:44:20PM +0100, Hannes Reinecke wrote:
 On 02/06/2014 04:29 PM, Greg Kroah-Hartman wrote:
  On Thu, Feb 06, 2014 at 03:27:43PM +0100, Hannes Reinecke wrote:
  The 'active' sysfs attribute should refer to the currently
  active tty devices the console is running on, not the currently
  active console.
  
  That's not what Documentation/ABI/sysfs-tty says:
   Shows the list of currently configured 
  
   console devices, like 'tty1 ttyS0'.
  
   The last entry in the file is the active   
  
   device connected to /dev/console.  
  
   The file supports poll() to detect virtual 
  
   console switches. 
  
 The problem is indeed with 'console devices'. There is no such
 thing; you only have tty devices where the console is running on.
 
  The console structure doesn't refer to any device in sysfs,
  only the tty the console is running on has.
  
  That sentance doesn't make sense.
  
  So we need to print out the tty names in 'active', not
  the console names.
  
  But that doesn't match the documentation.
  
  What exactly are you trying to fix here?  What is the problem that the
  current file has that is broken?  And as you are changing what this file
  means, what will break if the information in the file changes?
  
 systemd is using the 'active' sysfs attribute to figure out on which
 _tty_ device to start a getty on.
 As soon as the console name and the tty name are different
 you have no means of figuring out which _device_ to open.
 AFAICS the console 'device' (ie the current entry in 'active')
 doesn't have _any_ equivalent in sysfs; it just so happens that for
 most console drivers the tty driver name is identical.
 But this is not a requirement, and fails for drivers which have a
 different device for the console and the tty.
 
 EG on S/390 the 3270 tty has the devices
 
 /dev/3270/tty1
 
 but the console driver announces the name 'tty3270'.
 So as per current rules the 'active' attribute contains
 
 tty32700
 
 which correct as per documentation, but doesn't have _any_
 equivalent in sysfs.
 
 Martin has the grubby details here.
 
 But of course, the documentation should be updated to match the new
 behavior.

Ok, care to send an updated version, that fixes the Documentation as
well?  If Kay agrees that this is the correct solution, I'll be glad to
take it.

thanks,

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

Re: [systemd-devel] [PATCH] udev: Adding zram to inappropriate block device

2014-02-06 Thread Greg KH 
On Thu, Feb 06, 2014 at 04:54:59PM +, Jóhann B. Guðmundsson wrote:
 
 On 02/06/2014 03:39 PM, Greg KH wrote:
 Right now you have to decide before loading the module how many
 devices you want. And also when trying to use a device (any device),
 you have to look for one. The same issues as with loop.
 Given that the code doesn't have the ability to dynamically add/remove
 devices, I think we are stuck with this, right?
 
 This  may come as a completely stupid question but if the code is expected
 to be able to dynamically add/remove devices should not upstream ( kernel )
 nack inclusion of zram until it does?

I don't think the code is expected to be able to do that, have you
looked at it and seen where it does?

thanks,

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

Re: [systemd-devel] [PATCH] udev: Adding zram to inappropriate block device

2014-02-06 Thread Reindl Harald 

Am 06.02.2014 18:45, schrieb Greg KH:
 On Thu, Feb 06, 2014 at 04:54:59PM +, Jóhann B. Guðmundsson wrote:

 On 02/06/2014 03:39 PM, Greg KH wrote:
 Right now you have to decide before loading the module how many
 devices you want. And also when trying to use a device (any device),
 you have to look for one. The same issues as with loop.
 Given that the code doesn't have the ability to dynamically add/remove
 devices, I think we are stuck with this, right?

 This may come as a completely stupid question but if the code is expected
 to be able to dynamically add/remove devices should not upstream ( kernel )
 nack inclusion of zram until it does?
 
 I don't think the code is expected to be able to do that, have you
 looked at it and seen where it does?

please re-read both posts
in the last one clearly a not fails

fixed version:
 This may come as a completely stupid question but if the code is *not* 
 expected
 to be able to dynamically add/remove devices should not upstream ( kernel )
 nack inclusion of zram until it does?



signature.asc
Description: OpenPGP digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] systemd crashes if locale.conf contains invalid utf8 string

2014-02-06 Thread Goffredo Baroncelli 
In the parse_env_file_push() and load_env_file_push() functions, there
are two assert() call to check if the key or value parameters are utf8 valid.

If the strings aren't utf8 valid, assert does abort.

These function are used early by systemd to parse some files. For 
example '/etc/locale.conf'. In my case this file contained a not utf8
sequence, which is bad, but systemd crashed during the boot, which
is even worse !

The enclosed patch removes the assert and return -EINVAL if the
sequence is invalid. This is possible because the caller of these
function [1] checks the errors.
So the check of an invalid utf8 sequence is still performed, but
systemd doesn't crash anymore and logs the error.

BR
G.Baroncelli



[1] parse_env_file_internal(), invoked by load_env_file() and
parse_env_file()

-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5



diff --git a/src/shared/fileio.c b/src/shared/fileio.c
index ede8819..38af34b 100644
--- a/src/shared/fileio.c
+++ b/src/shared/fileio.c
@@ -534,35 +534,41 @@ fail:
 
 static int parse_env_file_push(const char *filename, unsigned line,
const char *key, char *value, void *userdata) {
-assert(utf8_is_valid(key));
 
-if (value  !utf8_is_valid(value))
+const char *k;
+va_list* ap = (va_list*) userdata;
+va_list aq;
+
+if (!utf8_is_valid(key)) {
+log_error(%s:%u: invalid UTF-8 for key '%s', ignoring.,
+  filename, line, key);
+return -EINVAL;
+}
+
+if (value  !utf8_is_valid(value)) {
 /* FIXME: filter UTF-8 */
-log_error(%s:%u: invalid UTF-8 for key %s: '%s', ignoring.,
+log_error(%s:%u: invalid UTF-8 value for key %s: '%s', 
ignoring.,
   filename, line, key, value);
-else {
-const char *k;
-va_list* ap = (va_list*) userdata;
-va_list aq;
+return -EINVAL;
+}
 
-va_copy(aq, *ap);
+va_copy(aq, *ap);
 
-while ((k = va_arg(aq, const char *))) {
-char **v;
+while ((k = va_arg(aq, const char *))) {
+char **v;
 
-v = va_arg(aq, char **);
+v = va_arg(aq, char **);
 
-if (streq(key, k)) {
-va_end(aq);
-free(*v);
-*v = value;
-return 1;
-}
+if (streq(key, k)) {
+va_end(aq);
+free(*v);
+*v = value;
+return 1;
 }
-
-va_end(aq);
 }
 
+va_end(aq);
+
 free(value);
 return 0;
 }
@@ -586,26 +592,31 @@ int parse_env_file(
 
 static int load_env_file_push(const char *filename, unsigned line,
   const char *key, char *value, void *userdata) {
-assert(utf8_is_valid(key));
+char ***m = userdata;
+char *p;
+int r;
 
-if (value  !utf8_is_valid(value))
+if (!utf8_is_valid(key)) {
+log_error(%s:%u: invalid UTF-8 for key '%s', ignoring.,
+  filename, line, key);
+return -EINVAL;
+}
+
+if (value  !utf8_is_valid(value)) {
 /* FIXME: filter UTF-8 */
-log_error(%s:%u: invalid UTF-8 for key %s: '%s', ignoring.,
+log_error(%s:%u: invalid UTF-8 value for key %s: '%s', 
ignoring.,
   filename, line, key, value);
-else {
-char ***m = userdata;
-char *p;
-int r;
+return -EINVAL;
+}
 
-p = strjoin(key, =, strempty(value), NULL);
-if (!p)
-return -ENOMEM;
+p = strjoin(key, =, strempty(value), NULL);
+if (!p)
+return -ENOMEM;
 
-r = strv_push(m, p);
-if (r  0) {
-free(p);
-return r;
-}
+r = strv_push(m, p);
+if (r  0) {
+free(p);
+return r;
 }
 
 free(value);


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

[systemd-devel] [PATCH] man: cryptsetup now allows partition device file in system mode

2014-02-06 Thread Jan Janssen 
---
 man/crypttab.xml | 9 +
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/man/crypttab.xml b/man/crypttab.xml
index 5f386e5..c563851 100644
--- a/man/crypttab.xml
+++ b/man/crypttab.xml
@@ -305,14 +305,7 @@
 
 listitemparaUse TrueCrypt in system
 encryption mode. This implies
-varnametcrypt/varname./para
-
-paraPlease note that when using this mode, 
the
-whole device needs to be given in the second
-field instead of the partition. For example: if
-literal/dev/sda2/literal is the system
-encrypted TrueCrypt patition, 
literal/dev/sda/literal
-has to be given./para/listitem
+   varnametcrypt/varname./para/listitem
 /varlistentry
 
 varlistentry
-- 
1.8.5.3

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

Re: [systemd-devel] [PATCH 1/3] Add SELinuxContext configuration item

2014-02-06 Thread David Timothy Strauss 
In order to maximize consistency with newly committed options in
systemd-nspawn, would it make sense to allow independent configuration
of the process and file labels instead?
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/7] logind: close races on user and session states during login

2014-02-06 Thread Djalal Harouni 
Currently the user and session states are not stable, they are affected
by several races during login:

1) session state:
   To get the session state the function session_get_state() is used.

Opening state:
At login the D-Bus CreateSession() method will call session_start() which
calls user_start() and session_start_scope() to queue the scope job and
set the session-scope_job

   =  session_get_state() == SESSION_OPENING   (correct)

Then execution will continue from session_send_create() which is called
by the D-Bus match_job_removed() signal. math_job_removed() will check if
this is the session scope and if this is the previously queued scope job.
If so it will clear the session-scope_job

   =  session_get_state() == SESSION_CLOSING   (incorrect)
  (session closing since fifo_fd == -1)

So scope job has finished and scope was created successfully, later the
session_send_create_reply() will wait for the session scope *and* the
user service to be successfully created.

  /* user service is still pending */
  if (s-scope_job || s-user-service_job))
 return 0

   =  session_get_state() == SESSION_CLOSING   (incorrect)

  else
 session_create_fifo()/* fifo_fd finally created */

   =  session_get_state() == SESSION_ACTIVE   (correct)

To sum it up, current state during login:
SESSION_OPENING=SESSION_CLOSINGx2=SESSION_ACTIVE

To fix the session state and remove the two incorrect SESSION_CLOSING,
we do not clear up the session-scope_job when we detect that this is
the session scope, we setup a temporary variable scope_job that will
get the current value of session-scope_job and update it if
necessary.

Add a new active variable to check if the session scope and user
service jobs are still being created.

Update session_jobs_replay() and session_send_create_reply() function to
receive the opening variable as an argument, so it will still wait for
the scope and service jobs to finish before creating the session fifo.

The session-scope_job will be cleared when session_jobs_reply()
finishes. This ensures that the state will just go from:
SESSION_OPENING = SESSION_ACTIVE

2) user state:
   To get the user state the function user_get_state() is used.

I'll add that the user_get_state() and session_get_state() do not have
the same logic when it comes to sessions, this will just add ambiguity.
user_get_state() calls session_is_active() before checking
session_get_state(), and session_is_active() will return true right from
the start since the logic is set during D-Bus CreateSession(). This will
we be fixed in the followup patches.

Opening state:
At login we have session_start() which calls user_start()

here we already:

   =  user_get_state() == USER_ACTIVE   (incorrect)
   (not fixed in this patch)

user_start() calls:
user_start_slice() queue the slice and set user-slice_job
user_start_service() queue the service and set user-service_job

   =  user_get_state() == USER_OPENING   (correct)

Then execution will continue from session_send_create() which is called
by the D-Bus match_job_removed() signal. math_job_removed() will check if
these are the user service and slice and if they are the previously queued
jobs. If so it will clear the: user-service_job and user-slice_job

   =  user_get_state() == USER_ACTIVE   (incorrect)
 (incorrect since the fifo_fd has not been created,
  here the state should stay USER_OPENING)

Later when the user service is created successfully,
session_send_create_reply() will also wait for the session scope to be
created. If so then session_send_create_reply() will continue and call
session_create_fifo()

   =  user_get_state() == USER_ACTIVE   (correct)
   (fifo_fd was created)

To fix this, we use the same logic as used to fix session states. In
order to remove the incorrect state when the fifo_fd is not created but
the state shows USER_ACTIVE, we do not clear the user-service_job and
user-slice_job right away, we store the state in a temporary variable
service_job and update it later if we detect that this is the user
service.

The new active variable will be used to check if the session scope and
user service are still being created. If so we'll wait for the next
match_job_removed() signal and continue, otherwise we proceed with
session_jobs_reply() and session_send_create_reply() in order to notify
clients.
---
 src/login/logind-dbus.c | 44 ++---
 src/login/logind-session-dbus.c |  8 +---
 src/login/logind-session.h  |  2 +-
 3 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 7b050fb..0560707 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -1919,7 +1919,9 @@ const sd_bus_vtable manager_vtable[] = {
 SD_BUS_VTABLE_END
 };
 
-static int session_jobs_reply(Session *s, const char *unit, const char 
*result) {
+/*

[systemd-devel] [PATCH 1/7] logind: add function session_jobs_reply() to unify the create reply

2014-02-06 Thread Djalal Harouni 
The session_send_create_reply() function which notifies clients about
session creation is used for both session and user units. Unify the
shared code in a new function session_jobs_reply().

The session_save() will be called unconditionally on sessions since it
does not make sense to only call it if '!session-started', this will
also allow to update the session state as soon as possible.
---
 src/login/logind-dbus.c | 46 --
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 4745961..7b050fb 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -1919,6 +1919,27 @@ const sd_bus_vtable manager_vtable[] = {
 SD_BUS_VTABLE_END
 };
 
+static int session_jobs_reply(Session *s, const char *unit, const char 
*result) {
+int r = 0;
+
+assert(s);
+assert(unit);
+
+if (!s-started)
+return r;
+
+if (streq(result, done))
+r = session_send_create_reply(s, NULL);
+else {
+_cleanup_bus_error_free_ sd_bus_error e = SD_BUS_ERROR_NULL;
+
+sd_bus_error_setf(e, BUS_ERROR_JOB_FAILED, Start job for 
unit %s failed with '%s', unit, result);
+r = session_send_create_reply(s, e);
+}
+
+return r;
+}
+
 int match_job_removed(sd_bus *bus, sd_bus_message *message, void *userdata, 
sd_bus_error *error) {
 const char *path, *result, *unit;
 Manager *m = userdata;
@@ -1958,18 +1979,9 @@ int match_job_removed(sd_bus *bus, sd_bus_message 
*message, void *userdata, sd_b
 session-scope_job = NULL;
 }
 
-if (session-started) {
-if (streq(result, done))
-session_send_create_reply(session, NULL);
-else {
-_cleanup_bus_error_free_ sd_bus_error e = 
SD_BUS_ERROR_NULL;
-
-sd_bus_error_setf(e, BUS_ERROR_JOB_FAILED, 
Start job for unit %s failed with '%s', unit, result);
-session_send_create_reply(session, e);
-}
-} else
-session_save(session);
+session_jobs_reply(session, unit, result);
 
+session_save(session);
 session_add_to_gc_queue(session);
 }
 
@@ -1987,17 +1999,7 @@ int match_job_removed(sd_bus *bus, sd_bus_message 
*message, void *userdata, sd_b
 }
 
 LIST_FOREACH(sessions_by_user, session, user-sessions) {
-if (!session-started)
-continue;
-
-if (streq(result, done))
-session_send_create_reply(session, NULL);
-else {
-_cleanup_bus_error_free_ sd_bus_error e = 
SD_BUS_ERROR_NULL;
-
-sd_bus_error_setf(e, BUS_ERROR_JOB_FAILED, 
Start job for unit %s failed with '%s', unit, result);
-session_send_create_reply(session, e);
-}
+session_jobs_reply(session, unit, result);
 }
 
 user_save(user);
-- 
1.8.3.1

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

[systemd-devel] [PATCH v2 0/7] logind: close races on user and session states

2014-02-06 Thread Djalal Harouni 
Summary:
Currently logind will not clear sessions on logout. The bug is confirmed
for getty and ssh logins. This series is preparation for next patches to
address that bug, it does not fix it.

However, this series also fixe real races on user and session states.
This ensures that user_save() and session_save() functions will write the
correct user and session state to the state files.


The logind bug was already discussed here:
http://lists.freedesktop.org/archives/systemd-devel/2014-January/015968.html

Patches were proposed, but they failed to address the bug since there
are other problems related to user and session states:
http://lists.freedesktop.org/archives/systemd-devel/2014-January/016372.html

A first version to fix these race conditions on user and sessions
states was proposed:
http://lists.freedesktop.org/archives/systemd-devel/2014-January/016373.html


This series is a v2, it will close all the discovered races on user and
session states. The commit logs will tell you the story of each case.


Now as noted above, this series fix real races and in the same time it
is needed to fix the logind bug where sessions are not cleaned after
logouts.

Proposed plan to clean logind closed sessions:
1) Make the user and session states stable (this series fix it).

2) Improve session_check_gc() and user_check_gc() to check if:
   the state is closing and the cgroup is empty.

3) Push session and user into the gc during logout, in pam_systemd
   method_release_session() 

Now I've a patch that implements 2) and 3) on top of 1), sometimes it
will work and successfully clean closed sessions, and sometimes it will
not. This is due to a race when we close the session and try to
terminate session processes and in the same time we are trying to see if
the cgroup is empty... which is another problem on its own.


So here are the patches, please consider this series since it will fix
real races and it will make sure that user_save() and session_save()
will write the correct state to the state files.


Patches 1,6,7 are code cleanup.

Patches 2,3,4,5 close races on user and session states.


Djalal Harouni (7):
0001 logind: add function session_jobs_reply() to unify the create reply
  unify shared code in a single function.

0002 logind: close races on user and session states during login
0003 logind: close races on session state at logout
0004 logind: close races on user state at logout
  close race conditions on user and session states.

0005 logind: just call session_get_state() to get the session state
  make user_get_state() consistent with session_get_state()

0006 logind: add user_is_opening() and session_is_opening()
0007 logind: do not call session_jobs_reply() on CLOSING
  make sure to not call session_send_create_reply() when jobs finish
  during closing state.

 src/login/logind-dbus.c | 87  
+++
 src/login/logind-session-dbus.c | 11 ---
 src/login/logind-session.c  | 10 +-
 src/login/logind-session.h  |  4 +++-
 src/login/logind-user.c | 20 +---
 src/login/logind-user.h |  3 +++
 6 files changed, 99 insertions(+), 36 deletions(-)

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

[systemd-devel] [PATCH 3/7] logind: close races on session state at logout

2014-02-06 Thread Djalal Harouni 
To get the state of the session, the session_get_state() is used.
This function will check if the session-scope_job is set then it will
automatically return SESSION_OPENING. This is buggy in the context of
session closing:

At logout or D-Bus TerminateSession() fifo_fd is removed:

   =  session_get_state() == SESSION_CLOSING   (correct)

Then we have session_stop() which calls session_stop_scope(), this
will queue the scope job to be removed and set the session-scope_job

   =  session_get_state() == SESSION_OPENING   (incorrect)

After the scope job is removed the state will be again correct

   =  session_get_state() == SESSION_CLOSING   (correct)

To fix this and make sure that the state will always be SESSION_CLOSING
we add a flag that is used to differentiate between SESSION_OPENING and
SESSION_CLOSING.

The 'scope_opening' flag will be set to true only during real session
opening in session_start_scope(), and it will be set to false just after
the session fifo fd was successfully created, which means that during
session closing it will be false.

And update session_get_state() to check if the 'scope_opening' is true
before returning the SESSION_OPENING, if it is not then SESSION_CLOSING
will be returned which is the correct behaviour.
---
 src/login/logind-session-dbus.c | 3 +++
 src/login/logind-session.c  | 4 +++-
 src/login/logind-session.h  | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c
index 54db864..099ade6 100644
--- a/src/login/logind-session-dbus.c
+++ b/src/login/logind-session-dbus.c
@@ -670,6 +670,9 @@ int session_send_create_reply(Session *s, sd_bus_error 
*error, bool opening) {
 if (fifo_fd  0)
 return fifo_fd;
 
+/* Clean this up as soon as we have the fifo_fd */
+s-scope_opening = false;
+
 /* Update the session state file before we notify the client
  * about the result. */
 session_save(s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index bec59c0..848e8a1 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -496,6 +496,8 @@ static int session_start_scope(Session *s) {
 
 free(s-scope_job);
 s-scope_job = job;
+/* session scope is being created */
+s-scope_opening = true;
 }
 }
 
@@ -880,7 +882,7 @@ void session_add_to_gc_queue(Session *s) {
 SessionState session_get_state(Session *s) {
 assert(s);
 
-if (s-scope_job)
+if (s-scope_opening)
 return SESSION_OPENING;
 
 if (s-fifo_fd  0)
diff --git a/src/login/logind-session.h b/src/login/logind-session.h
index ebe3902..205491a 100644
--- a/src/login/logind-session.h
+++ b/src/login/logind-session.h
@@ -110,6 +110,7 @@ struct Session {
 
 bool in_gc_queue:1;
 bool started:1;
+bool scope_opening:1; /* session scope is being created */
 
 sd_bus_message *create_message;
 
-- 
1.8.3.1

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

[systemd-devel] [PATCH 4/7] logind: close races on user state at logout

2014-02-06 Thread Djalal Harouni 
To get the state of the user, the user_get_state() is used.
This function will check if the user-slice_job or the user-service_job
are set then it will automatically return USER_OPENING. This is buggy
in the context of user closing:

At logout or D-Bus TerminateUser() calls user_stop()
user_stop() = session_stop() on sessions = session_stop_scope()

  =  user_get_state() == USER_CLOSING or USER_ACTIVE or USER_ONLINE
(incorrect)

  (depends on session closing execution and if we have finished the
  session scope jobs or not, if all the scopes are being queued then
  their session-scope_job will be set which means all sessions are in
  the OPENING_STATE, so user state will be USER_ONLINE).

  The previous patch improves session_get_state() logic, and if scopes
  are being queued during logout then sessions are in SESSION_CLOSING
  state which makes user_get_state() return USER_CLOSING.

So at user_stop()
user_stop_slice() queues the slice and sets user-slice_job
user_stop_service() queues the service and sets user-service_job

  =  user_get_state() == USER_OPENING  (incorrect)

After the slice and service jobs are removed the state will be again
correct.

  =  user_get_state() == USER_CLOSING  (correct)

To fix this and make sure that the state will always be USER_CLOSING
we add a flag that is used to differentiate between USER_OPENING and
USER_CLOSING when 'slice_job' and 'service_job' are set.

The 'slice_opening' flag for 'user-slice_job' and 'service_opening'
for 'user-service_job' are set to true only during real user opening,
later if the service and slice were successfully created their
corresponding 'opening' flag will be set to false, which means that
during user closing they are already false.

Update user_get_state() to check if 'slice_opening' or
'service_opening' are set, if so return USER_OPENING.
---
 src/login/logind-dbus.c | 2 ++
 src/login/logind-user.c | 6 +-
 src/login/logind-user.h | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 0560707..24482fd 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -2017,11 +2017,13 @@ int match_job_removed(sd_bus *bus, sd_bus_message 
*message, void *userdata, sd_b
 /* Clean this up after session_jobs_reply() */
 free(user-service_job);
 user-service_job = NULL;
+user-service_opening = false;
 }
 
 if (streq_ptr(path, user-slice_job)) {
 free(user-slice_job);
 user-slice_job = NULL;
+user-slice_opening = false;
 }
 
 user_save(user);
diff --git a/src/login/logind-user.c b/src/login/logind-user.c
index bdb6915..8183721 100644
--- a/src/login/logind-user.c
+++ b/src/login/logind-user.c
@@ -352,6 +352,8 @@ static int user_start_slice(User *u) {
 
 free(u-slice_job);
 u-slice_job = job;
+/* User slice is being created */
+u-slice_opening = true;
 }
 }
 
@@ -385,6 +387,8 @@ static int user_start_service(User *u) {
 
 free(u-service_job);
 u-service_job = job;
+/* User service is being created */
+u-service_opening = true;
 }
 }
 
@@ -637,7 +641,7 @@ UserState user_get_state(User *u) {
 
 assert(u);
 
-if (u-slice_job || u-service_job)
+if (u-slice_opening || u-service_opening)
 return USER_OPENING;
 
 LIST_FOREACH(sessions_by_user, i, u-sessions) {
diff --git a/src/login/logind-user.h b/src/login/logind-user.h
index 0062880..ac43361 100644
--- a/src/login/logind-user.h
+++ b/src/login/logind-user.h
@@ -61,6 +61,8 @@ struct User {
 
 bool in_gc_queue:1;
 bool started:1;
+bool service_opening:1; /* User service is being created */
+bool slice_opening:1; /* User slice is being created */
 
 LIST_HEAD(Session, sessions);
 LIST_FIELDS(User, gc_queue);
-- 
1.8.3.1

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

[systemd-devel] [PATCH 7/7] logind: do not call session_jobs_reply() on CLOSING

2014-02-06 Thread Djalal Harouni 
match_job_removed() signal is triggered when queued jobs finish during
session opening or closing.

Calling session_jobs_reply() during opening is valid, but during
session closing does not make sense.

The session_send_create_reply() function which is called by
session_jobs_reply() is able to detect if it was not called during
open time by checking the 'session-create_message'. However, making
session_jobs_reply() check session_is_opening() and user_is_opening()
is more comprehensive.
---
 src/login/logind-dbus.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 24482fd..4b71d9e 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -1930,6 +1930,10 @@ static int session_jobs_reply(Session *s, const char 
*unit, const char *result,
 if (!s-started)
 return r;
 
+/* Don't call me if this is not opening state */
+if (!session_is_opening(s)  !user_is_opening(s-user))
+return r;
+
 if (streq(result, done))
 r = session_send_create_reply(s, NULL, opening);
 else {
@@ -2010,6 +2014,7 @@ int match_job_removed(sd_bus *bus, sd_bus_message 
*message, void *userdata, sd_b
  * still being created this will be set to true,
  * otherwise it will be false */
 active = service_job || !!session-scope_job;
+
 session_jobs_reply(session, unit, result, active);
 }
 
-- 
1.8.3.1

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

Re: [systemd-devel] [PATCH] man: cryptsetup now allows partition device file in system mode

2014-02-06 Thread Tom Gundersen 
On Thu, Feb 6, 2014 at 8:33 PM, Jan Janssen medhe...@web.de wrote:
 ---
  man/crypttab.xml | 9 +
  1 file changed, 1 insertion(+), 8 deletions(-)

 diff --git a/man/crypttab.xml b/man/crypttab.xml
 index 5f386e5..c563851 100644
 --- a/man/crypttab.xml
 +++ b/man/crypttab.xml
 @@ -305,14 +305,7 @@

  listitemparaUse TrueCrypt in system
  encryption mode. This implies
 -varnametcrypt/varname./para
 -
 -paraPlease note that when using this mode, 
 the
 -whole device needs to be given in the second
 -field instead of the partition. For example: 
 if
 -literal/dev/sda2/literal is the system
 -encrypted TrueCrypt patition, 
 literal/dev/sda/literal
 -has to be given./para/listitem
 +   varnametcrypt/varname./para/listitem
  /varlistentry

  varlistentry


When/what release did this change? A reference in the commit message
would be useful.

Cheers,

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