[systemd-devel] how to handle filesystems that do not support d_type in readdirs dirent?

[Marius Tolzmann]

hello..

we are currently using reiserfs on our root-partition..

since reiserfs always sets d_type to DT_UNKNOWN in dirent entries some 
tools like systemd-tmpfiles do not work as expected.


in src/util.c:3905 DT_UNKNOWN is already included when checking file for 
type DT_REG or DT_LNK.


since there seems to be no special handling for symlinks (e.g. checking 
the type of the symlinks destionation) i just fixed scandir_filter to 
also accept DT_UNKNOWN.


(other) places where DT_REG checks may fail on reiserfs or other fs that 
don't support setting the correct type of file in d_type:


src/tty-ask-password-agent.c:510: if (de->d_type != DT_REG)
src/modules-load.c:45: if (d->d_type != DT_REG &&
src/tmpfiles.c:781: if (d->d_type != DT_REG &&

.. how is this supposed to be handled? if the type of a symlinks 
destination isn't checked the whole check could be skipped at all?


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

Re: [systemd-devel] how to handle filesystems that do not support d_type in readdirs dirent?

[Andrey Borzenkov]

On Thu, Mar 3, 2011 at 3:05 PM, Marius Tolzmann  wrote:
>
> hello..
>
> we are currently using reiserfs on our root-partition..
>
> since reiserfs always sets d_type to DT_UNKNOWN in dirent entries some tools
> like systemd-tmpfiles do not work as expected.
>

Hmm .. this could be the reason for my problem with utmp:

http://lists.freedesktop.org/archives/systemd-devel/2011-March/001433.html

> in src/util.c:3905 DT_UNKNOWN is already included when checking file for
> type DT_REG or DT_LNK.
>

Do not assume that everyone is having exactly the same sources as you
:) Either include exact commit number for reference, or better just
name function; they are mostly small enough.

> since there seems to be no special handling for symlinks (e.g. checking the
> type of the symlinks destionation) i just fixed scandir_filter to also
> accept DT_UNKNOWN.
>
> (other) places where DT_REG checks may fail on reiserfs or other fs that
> don't support setting the correct type of file in d_type:
>
> src/tty-ask-password-agent.c:510: if (de->d_type != DT_REG)
> src/modules-load.c:45: if (d->d_type != DT_REG &&
> src/tmpfiles.c:781: if (d->d_type != DT_REG &&
>
> .. how is this supposed to be handled?


Care to provide a patch to include missing DT_UNKNOWN in all places? :)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] how to handle filesystems that do not support d_type in readdirs dirent?

[Marius Tolzmann]

hi..

On 03/03/11 13:52, Andrey Borzenkov wrote:

since reiserfs always sets d_type to DT_UNKNOWN in dirent entries some tools
like systemd-tmpfiles do not work as expected.


Hmm .. this could be the reason for my problem with utmp:


it started with a missing utmp and i tracked it down to 
systemd-tempfiles not working at all on reiserfs..




in src/util.c:3905 DT_UNKNOWN is already included when checking file for
type DT_REG or DT_LNK.



Do not assume that everyone is having exactly the same sources as you
:) Either include exact commit number for reference, or better just
name function; they are mostly small enough.


oops.. sorry for that.. i thought i mentioned that i am using the 
systemd v19 sources from the official release..




since there seems to be no special handling for symlinks (e.g. checking the
type of the symlinks destionation) i just fixed scandir_filter to also
accept DT_UNKNOWN.

(other) places where DT_REG checks may fail on reiserfs or other fs that
don't support setting the correct type of file in d_type:

src/tty-ask-password-agent.c:510: if (de->d_type != DT_REG)
src/modules-load.c:45: if (d->d_type != DT_REG&&
src/tmpfiles.c:781: if (d->d_type != DT_REG&&

.. how is this supposed to be handled?


Care to provide a patch to include missing DT_UNKNOWN in all places? :)


the question is: how to fix?

why check for regular file/symlink at all when the destination of the 
symlink is not checked again..


so there are (at least) to options to (quick) fix this:
  - remove the whole check..
  - add DT_UNKNOWN and may be in addition check for regular file using
some stat() before reading the config in systemd-tmpfiles..

marius

--
Dipl.-Inf. Marius Tolzmann 
--.--
MPI f. molekulare Genetik |
Ihnestrasse 63-73, D-14195 Berlin |   ==> MarIuX GNU/Linux <==
Phone: +49 (0)30 8413 1709|
--^--
God put me on earth to accomplish a certain number of things.
Right now I am so far behind..
   ..I will never die. 
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] correctly mark system reboot

[Lennart Poettering]

On Thu, 03.03.11 09:14, Andrey Borzenkov (arvidj...@gmail.com) wrote:

> 
> On Thu, Mar 3, 2011 at 7:51 AM, Andrey Borzenkov  wrote:
> > On Thu, Mar 3, 2011 at 12:31 AM, Lennart Poettering
> >  wrote:
> >> On Wed, 02.03.11 11:41, Andrey Borzenkov (arvidj...@gmail.com) wrote:
> >>
> >>> It is expected that system will put "reboot" in wtmp to mark
> >>> when it starts coming up. This is looked for by "who -b" and is
> >>> used by "last" to calculate correct time spent by various programs.
> >>> Add systemd-update-utmp-reboot.service which is started as soon
> >>> as possible after local-fs.target to mark reboot.
> >>
> >> Hmm, systemd-update-utmp-runlevel.service should normally do that
> >> implicitly. When /lib/systemd/systemd-update-utmp is called with the
> >> "runlevel" argument then it will add the "reboot" entry if necessary
> >> automatically, followed by the "runlevel" entry.
> >>
> >> Are you suggesting that this automatic logic isn't working correctly?
> >>
> >
> > On my notebook "reboot" line is never added. What is funny, it appears
> > that on my test VM which has stripped down installation "reboot" does
> > actually appear. Which suggests some race condition or uninitialized
> > variable somewhere.
> >
> 
> Well, the problem is, on my notebook it *does* find previous runlevel
> entry and so never triggers on_reboot:
> 
> Mar  3 09:00:47 cooker systemd-update-utmp[2481]: After utmpxname
> Mar  3 09:00:47 cooker systemd-update-utmp[2481]: Found runlevel/previouos: 
> 0/0
> Mar  3 09:00:47 cooker systemd-update-utmp[2481]: Previous runlevel: 0
> Mar  3 09:00:47 cooker systemd-update-utmp[2481]: Current runlevel: 53
> Mar  3 09:00:47 cooker systemd-update-utmp[2481]: After utmpxname
> Mar  3 09:00:47 cooker systemd-update-utmp[2481]: Found runlevel/previouos: 
> 0/0
> 
> Because it works on stripped down installation, it appears that some
> package puts this entry there on startup.
> 
> I really do not know how to find who is messing up with utmp; it may
> be anything. So it looks like one of
> 
> - use explicit unit for "reboot" (and remove current magic from on_runlevel).
> 
> - call on_reboot() if previous run-level found was 0.
> 
> Personally I find explicit unit to be better understandable and easier
> to debug than hiding some magic inside binary that no one is aware of.

Hmm, here's a guess: the code relies that utmp is emptied? Maybe this
doesn't happen on your system?

Normally systemd-update-utmp-runlevel.service is run after
systemd-tmpfiles-setup.service, but maybe this doesn't work on your system?

Lennart

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

Re: [systemd-devel] [PATCH] correctly mark system reboot

[Lennart Poettering]

On Thu, 03.03.11 07:51, Andrey Borzenkov (arvidj...@gmail.com) wrote:

> 
> On Thu, Mar 3, 2011 at 12:31 AM, Lennart Poettering
>  wrote:
> > On Wed, 02.03.11 11:41, Andrey Borzenkov (arvidj...@gmail.com) wrote:
> >
> >> It is expected that system will put "reboot" in wtmp to mark
> >> when it starts coming up. This is looked for by "who -b" and is
> >> used by "last" to calculate correct time spent by various programs.
> >> Add systemd-update-utmp-reboot.service which is started as soon
> >> as possible after local-fs.target to mark reboot.
> >
> > Hmm, systemd-update-utmp-runlevel.service should normally do that
> > implicitly. When /lib/systemd/systemd-update-utmp is called with the
> > "runlevel" argument then it will add the "reboot" entry if necessary
> > automatically, followed by the "runlevel" entry.
> >
> > Are you suggesting that this automatic logic isn't working correctly?
> >
> 
> On my notebook "reboot" line is never added. What is funny, it appears
> that on my test VM which has stripped down installation "reboot" does
> actually appear. Which suggests some race condition or uninitialized
> variable somewhere.
> 
> Any suggestions how to debug it further?

Might want to check the activation timestamps shown in "systemctl show
systemd-tmpfiles-setup.service systemd-update-utmp-runlevel.service".

Lennart

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

Re: [systemd-devel] how to handle filesystems that do not support d_type in readdirs dirent?

[Andrey Borzenkov]

On Thu, Mar 3, 2011 at 5:48 PM, Marius Tolzmann  wrote:
>
> hi..
>
> On 03/03/11 13:52, Andrey Borzenkov wrote:
>>>
>>> since reiserfs always sets d_type to DT_UNKNOWN in dirent entries some
>>> tools
>>> like systemd-tmpfiles do not work as expected.
>>
>> Hmm .. this could be the reason for my problem with utmp:
>

Yep, it is fixed with my patch that follows. Thank you!

>
> the question is: how to fix?
>
> why check for regular file/symlink at all when the destination of the
> symlink is not checked again..
>
> so there are (at least) to options to (quick) fix this:
>  - remove the whole check..
>  - add DT_UNKNOWN and may be in addition check for regular file using
>    some stat() before reading the config in systemd-tmpfiles..
>

Yes, good question. This looks more like optimization that security,
so we do not try to open at least obviously wrong file types. But of
course rogue link to something like /proc/ioports could be quite
interesting :)

> so there are (at least) to options to (quick) fix this:
>  - remove the whole check..
>  - add DT_UNKNOWN and may be in addition check for regular file using
>some stat() before reading the config in systemd-tmpfiles..
>

If we are going to check for regular file, we should do it for DT_LNK as well.
The patch sent as follow-up does exactly this. Could you please test
if it fixes your issues (Tested-By is always good :) )
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] correctly mark system reboot

[Andrey Borzenkov]

On Thu, Mar 3, 2011 at 6:20 PM, Lennart Poettering
 wrote:
> On Thu, 03.03.11 09:14, Andrey Borzenkov (arvidj...@gmail.com) wrote:
>
>>
>> On Thu, Mar 3, 2011 at 7:51 AM, Andrey Borzenkov  wrote:
>> > On Thu, Mar 3, 2011 at 12:31 AM, Lennart Poettering
>> >  wrote:
>> >> On Wed, 02.03.11 11:41, Andrey Borzenkov (arvidj...@gmail.com) wrote:
>> >>
>> >>> It is expected that system will put "reboot" in wtmp to mark
>> >>> when it starts coming up. This is looked for by "who -b" and is
>> >>> used by "last" to calculate correct time spent by various programs.
>> >>> Add systemd-update-utmp-reboot.service which is started as soon
>> >>> as possible after local-fs.target to mark reboot.
>> >>
>> >> Hmm, systemd-update-utmp-runlevel.service should normally do that
>> >> implicitly. When /lib/systemd/systemd-update-utmp is called with the
>> >> "runlevel" argument then it will add the "reboot" entry if necessary
>> >> automatically, followed by the "runlevel" entry.
>> >>
>> >> Are you suggesting that this automatic logic isn't working correctly?
>> >>
>> >
>> > On my notebook "reboot" line is never added. What is funny, it appears
>> > that on my test VM which has stripped down installation "reboot" does
>> > actually appear. Which suggests some race condition or uninitialized
>> > variable somewhere.
>> >
>>
>> Well, the problem is, on my notebook it *does* find previous runlevel
>> entry and so never triggers on_reboot:
>>
>> Mar  3 09:00:47 cooker systemd-update-utmp[2481]: After utmpxname
>> Mar  3 09:00:47 cooker systemd-update-utmp[2481]: Found runlevel/previouos: 
>> 0/0
>> Mar  3 09:00:47 cooker systemd-update-utmp[2481]: Previous runlevel: 0
>> Mar  3 09:00:47 cooker systemd-update-utmp[2481]: Current runlevel: 53
>> Mar  3 09:00:47 cooker systemd-update-utmp[2481]: After utmpxname
>> Mar  3 09:00:47 cooker systemd-update-utmp[2481]: Found runlevel/previouos: 
>> 0/0
>>
>> Because it works on stripped down installation, it appears that some
>> package puts this entry there on startup.
>>
>> I really do not know how to find who is messing up with utmp; it may
>> be anything. So it looks like one of
>>
>> - use explicit unit for "reboot" (and remove current magic from on_runlevel).
>>
>> - call on_reboot() if previous run-level found was 0.
>>
>> Personally I find explicit unit to be better understandable and easier
>> to debug than hiding some magic inside binary that no one is aware of.
>
> Hmm, here's a guess: the code relies that utmp is emptied? Maybe this
> doesn't happen on your system?
>

Fixed in patch I am about to send.

> Normally systemd-update-utmp-runlevel.service is run after
> systemd-tmpfiles-setup.service, but maybe this doesn't work on your system?
>

it does.

I still believe that "one unit - one task" is better for understanding
and easier for debugging :)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] add extra check for DT_UNKNOWN files in directory scan

[Andrey Borzenkov]

Some file systems (at least, reiserfs) return DT_UNKNOWN for
every directory entry in readdir(). So far in several places
systemd filtered on DT_REG/DT_LNK and so skipped any file on
such filesystems. It affected systemd-update-utmp, systemd-modules-load
and systemd-tty-ask-password-agent.

This patch adds additional stat() check for file type for DT_UNKNOWN
case. Additionally, it adds check that DT_LNK really points to
regular file.

The patch fixes misterious runlevel entry in utmp discussed in
http://lists.freedesktop.org/archives/systemd-devel/2011-March/001433.html

Reported-By: tolzm...@molgen.mpg.de
Signed-off-by: Andrey Borzenkov 

---
 src/modules-load.c   |  137 +++---
 src/tmpfiles.c   |   22 ---
 src/tty-ask-password-agent.c |8 ++-
 3 files changed, 94 insertions(+), 73 deletions(-)

diff --git a/src/modules-load.c b/src/modules-load.c
index 3e3ccb0..7d733d1 100644
--- a/src/modules-load.c
+++ b/src/modules-load.c
@@ -43,12 +43,76 @@ static int scandir_filter(const struct dirent *d) {
 return 0;
 
 if (d->d_type != DT_REG &&
-d->d_type != DT_LNK)
+d->d_type != DT_LNK &&
+   d->d_type != DT_UNKNOWN)
 return 0;
 
 return endswith(d->d_name, ".conf");
 }
 
+static int load_modules_from_file(char *fn, char ***arguments, int 
*n_arguments, int *n_allocated)
+{
+   FILE *f;
+   int r = 0;
+
+   f = fopen(fn, "re");
+
+   if (!f) {
+   if (errno == ENOENT)
+   return 0;
+
+   log_error("Failed to open %s: %m", fn);
+   return -errno;
+   }
+
+   for (;;) {
+   char line[LINE_MAX], *l, *t;
+
+   if (!(fgets(line, sizeof(line), f)))
+   break;
+
+   l = strstrip(line);
+   if (*l == '#' || *l == 0)
+   continue;
+
+   if (!(t = strdup(l))) {
+   log_error("Failed to allocate module name.");
+   if (!r)
+   r = -ENOMEM;
+   continue;
+   }
+
+   if (*n_arguments >= *n_allocated) {
+   char **a;
+   unsigned m;
+
+   m = MAX(16U, *n_arguments*2);
+
+   if (!(a = realloc(*arguments, sizeof(char*) * (m+1 {
+   log_error("Failed to increase module array 
size.");
+   free(t);
+   if (!r)
+   r = -ENOMEM;
+   continue;
+   }
+
+   *arguments = a;
+   *n_allocated = m;
+   }
+
+   *arguments[(*n_arguments)++] = t;
+   }
+
+   if (ferror(f)) {
+   r = -EIO;
+   log_error("Failed to read from file: %m");
+   }
+
+   fclose(f);
+
+   return r;
+}
+
 int main(int argc, char *argv[]) {
 struct dirent **de = NULL;
 int r = EXIT_FAILURE, n, i;
@@ -86,70 +150,21 @@ int main(int argc, char *argv[]) {
 for (i = 0; i < n; i++) {
 int k;
 char *fn;
-FILE *f;
 
 k = asprintf(&fn, "/etc/modules-load.d/%s", de[i]->d_name);
-free(de[i]);
-
-if (k < 0) {
+if (k >= 0) {
+   struct stat st;
+
+   if (de[i]->d_type == DT_REG ||
+   (stat(fn, &st) >= 0 && S_ISREG(st.st_mode)))
+   if (load_modules_from_file(fn, &arguments, 
&n_arguments, &n_allocated) < 0)
+   r = EXIT_FAILURE;
+   free(fn);
+   } else {
 log_error("Failed to allocate file name.");
 r = EXIT_FAILURE;
-continue;
-}
-
-f = fopen(fn, "re");
-free(fn);
-
-if (!f) {
-if (errno == ENOENT)
-continue;
-
-log_error("Failed to open %s: %m", fn);
-r = EXIT_FAILURE;
-continue;
-}
-
-for (;;) {
-char line[LINE_MAX], *l, *t;
-
-if (!(fgets(line, sizeof(line), f)))
-break;
-
-l = strstrip(line);
-if (*l == '#' || *l == 0)
-continue;
-
-if (!(t = strdup(l))) {
-log_error("Failed to allocate module name.");
-continue;
-}
-
-if (n

Re: [systemd-devel] how to handle filesystems that do not support d_type in readdirs dirent?

[Lennart Poettering]

On Thu, 03.03.11 13:05, Marius Tolzmann (tolzm...@molgen.mpg.de) wrote:

> 
> hello..
> 
> we are currently using reiserfs on our root-partition..

Urks, people still use that cruft?
> 
> since reiserfs always sets d_type to DT_UNKNOWN in dirent entries
> some tools like systemd-tmpfiles do not work as expected.
> 
> in src/util.c:3905 DT_UNKNOWN is already included when checking file
> for type DT_REG or DT_LNK.
> 
> since there seems to be no special handling for symlinks (e.g.
> checking the type of the symlinks destionation) i just fixed
> scandir_filter to also accept DT_UNKNOWN.

Fixed this now the same way.

> (other) places where DT_REG checks may fail on reiserfs or other fs
> that don't support setting the correct type of file in d_type:
> 
> src/tty-ask-password-agent.c:510: if (de->d_type != DT_REG)

Shouldn't be a problem since this directory is in /dev, which we require
to be tmpfs or devtmpfs, which supports d_type. I have now added a comment to
clarify this.

> src/modules-load.c:45: if (d->d_type != DT_REG &&

Fixed this now, too.

> src/tmpfiles.c:781: if (d->d_type != DT_REG &&

This is the spot you already mentioned above, right?

> .. how is this supposed to be handled? if the type of a symlinks
> destination isn't checked the whole check could be skipped at all?

The check is a good if we can do it cheaply, but if we cannot it should
be acceptable in all these case if we don't do it.

Please check current git (in particular 1a6f4df) if it covers all issues
you raised.

Lennart

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

Re: [systemd-devel] [PATCH] add extra check for DT_UNKNOWN files in directory scan

[Lennart Poettering]

On Thu, 03.03.11 18:27, Andrey Borzenkov (arvidj...@gmail.com) wrote:

> Some file systems (at least, reiserfs) return DT_UNKNOWN for
> every directory entry in readdir(). So far in several places
> systemd filtered on DT_REG/DT_LNK and so skipped any file on
> such filesystems. It affected systemd-update-utmp, systemd-modules-load
> and systemd-tty-ask-password-agent.
> 
> This patch adds additional stat() check for file type for DT_UNKNOWN
> case. Additionally, it adds check that DT_LNK really points to
> regular file.
> 
> The patch fixes misterious runlevel entry in utmp discussed in
> http://lists.freedesktop.org/archives/systemd-devel/2011-March/001433.html

OOps, sorry. already applied a simpler fix before I saw this patch of
yours.

I think it is acceptable to skip these checks if the underlying fs
doesn't make them cheap. Doing an explicit stat() in case of DT_UNKNOWN
buys us very little since stat() plus open() is still vulnerable to
races.

So, I think the simple fix that is in right now is already more than
enough. Especially since those dirs are actually root-only writable,
hence not super security sensitive.

Lennart

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

Re: [systemd-devel] [PATCH] add extra check for DT_UNKNOWN files in directory scan

[Marius Tolzmann]

Hello..

this looks fine.. and is better than just adding DT_UNKOWN 8)

i additionally patched the cgroup-util.c (see attachment)

thx for the patch..

m.


On 03/03/11 16:27, Andrey Borzenkov wrote:

Some file systems (at least, reiserfs) return DT_UNKNOWN for
every directory entry in readdir(). So far in several places
systemd filtered on DT_REG/DT_LNK and so skipped any file on
such filesystems. It affected systemd-update-utmp, systemd-modules-load
and systemd-tty-ask-password-agent.

This patch adds additional stat() check for file type for DT_UNKNOWN
case. Additionally, it adds check that DT_LNK really points to
regular file.

The patch fixes misterious runlevel entry in utmp discussed in
http://lists.freedesktop.org/archives/systemd-devel/2011-March/001433.html

Reported-By: tolzm...@molgen.mpg.de
Signed-off-by: Andrey Borzenkov

---
  src/modules-load.c   |  137 +++---
  src/tmpfiles.c   |   22 ---
  src/tty-ask-password-agent.c |8 ++-
  3 files changed, 94 insertions(+), 73 deletions(-)


--
Dipl.-Inf. Marius Tolzmann 
--.--
MPI f. molekulare Genetik |
Ihnestrasse 63-73, D-14195 Berlin |   ==> MarIuX GNU/Linux <==
Phone: +49 (0)30 8413 1709|
--^--
God put me on earth to accomplish a certain number of things.
Right now I am so far behind..
   ..I will never die. 
diff -Naur systemd-19.orig/src/cgroup-util.c systemd-19/src/cgroup-util.c
--- systemd-19.orig/src/cgroup-util.c   2011-02-21 13:40:01.0 +0100
+++ systemd-19/src/cgroup-util.c2011-03-03 16:27:25.0 +0100
@@ -133,7 +133,18 @@
 while ((de = readdir(d))) {
 char *b;
 
-if (de->d_type != DT_DIR)
+if (de->d_type == DT_UNKNOWN) {
+struct stat st;
+
+if (fstatat(dirfd(d), de->d_name, &st, 
AT_SYMLINK_NOFOLLOW) < 0) {
+r = -errno;
+goto finish;
+}
+is_dir = S_ISDIR(st.st_mode);
+} else
+is_dir = de->d_type == DT_DIR;
+
+if (!is_dir)
 continue;
 
 if (streq(de->d_name, ".") ||
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] add extra check for DT_UNKNOWN files in directory scan

[Lennart Poettering]

On Thu, 03.03.11 16:36, Marius Tolzmann (tolzm...@molgen.mpg.de) wrote:

> Hello..
> 
> this looks fine.. and is better than just adding DT_UNKOWN 8)
> 
> i additionally patched the cgroup-util.c (see attachment)

Not necessary, since this is on cgroupfs and cgroupfs is fortunately not
as broken as reiserfs and does support DT_xxx just fine.

Lennart

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

Re: [systemd-devel] [PATCH] add extra check for DT_UNKNOWN files in directory scan

[Marius Tolzmann]

oops.. sorry.. wrong patch sent...

this won't compile.. but since lennart also fixed it it seems to be 
obsolete anyway... 8)



m.


On 03/03/11 16:36, Marius Tolzmann wrote:

Hello..

this looks fine.. and is better than just adding DT_UNKOWN 8)

i additionally patched the cgroup-util.c (see attachment)

thx for the patch..

m.


On 03/03/11 16:27, Andrey Borzenkov wrote:

Some file systems (at least, reiserfs) return DT_UNKNOWN for
every directory entry in readdir(). So far in several places
systemd filtered on DT_REG/DT_LNK and so skipped any file on
such filesystems. It affected systemd-update-utmp, systemd-modules-load
and systemd-tty-ask-password-agent.

This patch adds additional stat() check for file type for DT_UNKNOWN
case. Additionally, it adds check that DT_LNK really points to
regular file.

The patch fixes misterious runlevel entry in utmp discussed in
http://lists.freedesktop.org/archives/systemd-devel/2011-March/001433.html


Reported-By: tolzm...@molgen.mpg.de
Signed-off-by: Andrey Borzenkov

---
src/modules-load.c | 137 +++---
src/tmpfiles.c | 22 ---
src/tty-ask-password-agent.c | 8 ++-
3 files changed, 94 insertions(+), 73 deletions(-)




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



--
Dipl.-Inf. Marius Tolzmann 
--.--
MPI f. molekulare Genetik |
Ihnestrasse 63-73, D-14195 Berlin |   ==> MarIuX GNU/Linux <==
Phone: +49 (0)30 8413 1709|
--^--
God put me on earth to accomplish a certain number of things.
Right now I am so far behind..
   ..I will never die. 
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] add extra check for DT_UNKNOWN files in directory scan

[Marius Tolzmann]

On 03/03/11 16:38, Lennart Poettering wrote:

On Thu, 03.03.11 16:36, Marius Tolzmann (tolzm...@molgen.mpg.de) wrote:


Hello..

this looks fine.. and is better than just adding DT_UNKOWN 8)

i additionally patched the cgroup-util.c (see attachment)


Not necessary, since this is on cgroupfs and cgroupfs is fortunately not
as broken as reiserfs and does support DT_xxx just fine.


of course thats true... since it wasn't the right or working patch 
anyway 8).. sorry for sending this crap 8)


cheers, marius


--
Dipl.-Inf. Marius Tolzmann 
--.--
MPI f. molekulare Genetik |
Ihnestrasse 63-73, D-14195 Berlin |   ==> MarIuX GNU/Linux <==
Phone: +49 (0)30 8413 1709|
--^--
God put me on earth to accomplish a certain number of things.
Right now I am so far behind..
   ..I will never die. 
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] how to handle filesystems that do not support d_type in readdirs dirent?

[Marius Tolzmann]

hi

On 03/03/11 16:30, Lennart Poettering wrote:

On Thu, 03.03.11 13:05, Marius Tolzmann (tolzm...@molgen.mpg.de) wrote:



hello..

we are currently using reiserfs on our root-partition..


Urks, people still use that cruft?


at least we do..


.. how is this supposed to be handled? if the type of a symlinks
destination isn't checked the whole check could be skipped at all?


The check is a good if we can do it cheaply, but if we cannot it should
be acceptable in all these case if we don't do it.

Please check current git (in particular 1a6f4df) if it covers all issues
you raised.


i will check this ASAP...

thx for your support..


marius..

--
Dipl.-Inf. Marius Tolzmann 
--.--
MPI f. molekulare Genetik |
Ihnestrasse 63-73, D-14195 Berlin |   ==> MarIuX GNU/Linux <==
Phone: +49 (0)30 8413 1709|
--^--
God put me on earth to accomplish a certain number of things.
Right now I am so far behind..
   ..I will never die. 
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] how to handle filesystems that do not support d_type in readdirs dirent?

[Marius Tolzmann]

On 03/03/11 16:30, Lennart Poettering wrote:


Please check current git (in particular 1a6f4df) if it covers all issues
you raised.


patch 1a6f4df works fine and fixes our issues.. thx..

m.

--
Dipl.-Inf. Marius Tolzmann 
--.--
MPI f. molekulare Genetik |
Ihnestrasse 63-73, D-14195 Berlin |   ==> MarIuX GNU/Linux <==
Phone: +49 (0)30 8413 1709|
--^--
God put me on earth to accomplish a certain number of things.
Right now I am so far behind..
   ..I will never die. 
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] /usr on separate file system

[Tollef Fog Heen]

]] Lennart Poettering 

| > To boot a system, enough must be present on the root partition to
| > mount other filesystems. This includes utilities, configuration,
| > boot loader information, and other essential start-up data. /usr,
| > /opt, and /var are designed such that they may be located on other
| > partitions or filesystems.
| 
| Well, turns out no distro really follows the spec here, do they?

Given the number of Debian people I see running with separate /usr, I
believe it works just fine there and while it's a supported
configuration I'll patch the warning out of the Debian systemd packages,
at least.

-- 
Tollef Fog Heen
UNIX is user friendly, it's just picky about who its friends are
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] /usr on separate file system

[Kay Sievers]

On Thu, Mar 3, 2011 at 17:58, Tollef Fog Heen  wrote:
> ]] Lennart Poettering
>
> | > To boot a system, enough must be present on the root partition to
> | > mount other filesystems. This includes utilities, configuration,
> | > boot loader information, and other essential start-up data. /usr,
> | > /opt, and /var are designed such that they may be located on other
> | > partitions or filesystems.
> |
> | Well, turns out no distro really follows the spec here, do they?
>
> Given the number of Debian people I see running with separate /usr, I
> believe it works just fine there and while it's a supported
> configuration I'll patch the warning out of the Debian systemd packages,
> at least.

Sure, if you are willing to support stuff that uses D-Bus in the basic
target and all the other weird issues, I see no problem other than for
you. :)

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

Re: [systemd-devel] /usr on separate file system

[Lennart Poettering]

On Thu, 03.03.11 17:58, Tollef Fog Heen (tfh...@err.no) wrote:

> 
> ]] Lennart Poettering 
> 
> | > To boot a system, enough must be present on the root partition to
> | > mount other filesystems. This includes utilities, configuration,
> | > boot loader information, and other essential start-up data. /usr,
> | > /opt, and /var are designed such that they may be located on other
> | > partitions or filesystems.
> | 
> | Well, turns out no distro really follows the spec here, do they?
> 
> Given the number of Debian people I see running with separate /usr, I
> believe it works just fine there and while it's a supported
> configuration I'll patch the warning out of the Debian systemd packages,
> at least.

Well, it's of course up to you guys what you do there.

But it's a promise you are making there that you cannot keep. If you
want to support /usr on a separate partition then you'd need to do all
the work and move the PCI and USB databases to /, move libatasmart,
fix udisks, fix D-Bus and so on.

The least you should do is add a warning about this to your release
notes. 

The fact that most these things fail relatively gracefully should not
mislead you to believe that everything worked fine. Things still fail,
just not in a big gigantic atomic explosion scenario.

Lennart

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

Re: [systemd-devel] /usr on separate file system

[Tollef Fog Heen]

]] Lennart Poettering 

Hi,

| But it's a promise you are making there that you cannot keep. If you
| want to support /usr on a separate partition then you'd need to do all
| the work and move the PCI and USB databases to /, move libatasmart,
| fix udisks, fix D-Bus and so on.

I leave that to those who care about a separate /usr.  Personally, I'd
rather see the whole thing replace by a symlink to /. :-)

| The least you should do is add a warning about this to your release
| notes. 

I'll forward that to the people responsible for the release notes.

| The fact that most these things fail relatively gracefully should not
| mislead you to believe that everything worked fine. Things still fail,
| just not in a big gigantic atomic explosion scenario.

Would it work better if /usr was an automounted target?

Cheers,
-- 
Tollef Fog Heen
UNIX is user friendly, it's just picky about who its friends are
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] /usr on separate file system

[Tomasz Torcz]

On Thu, Mar 03, 2011 at 06:45:21PM +0100, Lennart Poettering wrote:
> On Thu, 03.03.11 17:58, Tollef Fog Heen (tfh...@err.no) wrote:
> 
> > 
> > ]] Lennart Poettering 
> > 
> > | > To boot a system, enough must be present on the root partition to
> > | > mount other filesystems. This includes utilities, configuration,
> > | > boot loader information, and other essential start-up data. /usr,
> > | > /opt, and /var are designed such that they may be located on other
> > | > partitions or filesystems.
> > | 
> > | Well, turns out no distro really follows the spec here, do they?
> > 
> > Given the number of Debian people I see running with separate /usr, I
> > believe it works just fine there and while it's a supported
> > configuration I'll patch the warning out of the Debian systemd packages,
> > at least.
> 
> Well, it's of course up to you guys what you do there.
> 
> But it's a promise you are making there that you cannot keep. If you
> want to support /usr on a separate partition then you'd need to do all
> the work and move the PCI and USB databases to /, move libatasmart,
> fix udisks, fix D-Bus and so on.
> The fact that most these things fail relatively gracefully should not
> mislead you to believe that everything worked fine. Things still fail,
> just not in a big gigantic atomic explosion scenario.

  I don't get it. What during the boot (before /usr is mounted) require pci.db,
usb ids, why udisks would be started?  I understand that full desktop session
need access to those, but we are talking about short window before starting
system and mounting /usr.  What will break?  Are the some udev rules needing
mapping between PCI ID and a name?  Anything else?

  (BTW, “yum remove libatasmart” suggest removal of udisks, nautilus, gvfs,
evolution and some GNOME parts.  Nothing related to boot).

-- 
Tomasz Torcz   72->|   80->|
xmpp: zdzich...@chrome.pl  72->|   80->|

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

Re: [systemd-devel] /usr on separate file system

[Tollef Fog Heen]

]] Tomasz Torcz 

|   I don't get it. What during the boot (before /usr is mounted) require 
pci.db,
| usb ids, why udisks would be started?

udev rules that reference the name rather than the USB/PCI vendor or
product ID is an example.  They're uncommon, but they might exist.

-- 
Tollef Fog Heen
UNIX is user friendly, it's just picky about who its friends are
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] /usr on separate file system

[Lennart Poettering]

On Thu, 03.03.11 19:42, Tollef Fog Heen (tfh...@err.no) wrote:

> 
> ]] Tomasz Torcz 
> 
> |   I don't get it. What during the boot (before /usr is mounted) require 
> pci.db,
> | usb ids, why udisks would be started?
> 
> udev rules that reference the name rather than the USB/PCI vendor or
> product ID is an example.  They're uncommon, but they might exist.

They are not so uncommon. I am pretty sure most Linux machines have at
least a couple since NM, MM, PA, GCM (at least) use them. Grep for
"pci-db" and "usb-db" in /lib/udev/rules.d/*.

Lennart

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

Re: [systemd-devel] /usr on separate file system

[Lennart Poettering]

On Thu, 03.03.11 19:29, Tollef Fog Heen (tfh...@err.no) wrote:

> | The least you should do is add a warning about this to your release
> | notes. 
> 
> I'll forward that to the people responsible for the release notes.
> 
> | The fact that most these things fail relatively gracefully should not
> | mislead you to believe that everything worked fine. Things still fail,
> | just not in a big gigantic atomic explosion scenario.
> 
> Would it work better if /usr was an automounted target?

That would probably blow up in your face, since a lot of programs used
during early boot end up accessing /usr and would stay stuck, since they
try to use locale stuff. I wouldn't be surprised if mount itself is one
of those.

Oh, yeah, good luck moving locale stuff to /, btw. ;-)

And to support Plymouth fully you'd have to move all the plymouth ttf
stuff and so to / too.

Lennart

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

Re: [systemd-devel] /usr on separate file system

[Lennart Poettering]

On Thu, 03.03.11 19:21, Tomasz Torcz (to...@pipebreaker.pl) wrote:

> > But it's a promise you are making there that you cannot keep. If you
> > want to support /usr on a separate partition then you'd need to do all
> > the work and move the PCI and USB databases to /, move libatasmart,
> > fix udisks, fix D-Bus and so on.
> > The fact that most these things fail relatively gracefully should not
> > mislead you to believe that everything worked fine. Things still fail,
> > just not in a big gigantic atomic explosion scenario.
> 
>   I don't get it. What during the boot (before /usr is mounted) require 
> pci.db,
> usb ids, why udisks would be started?  I understand that full desktop session
> need access to those, but we are talking about short window before starting
> system and mounting /usr.  What will break?  Are the some udev rules needing
> mapping between PCI ID and a name?  Anything else?

Well, udisks needs some rules inudev to be run to check if smart and
similar things are available. If that fails tehn udisks will not be able
to offer you support for these features.

I mean, again, things mostly fail gracefully. If you consider "graceful
failure" a synomym for "hey, this works perfectly", then well, be my
guest... ;-)

Lennart

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

Re: [systemd-devel] /usr on separate file system

[Pablo Hess]

>> Would it work better if /usr was an automounted target?
>
> That would probably blow up in your face, since a lot of programs used
> during early boot end up accessing /usr and would stay stuck

Aren't /usr/bin and /usr/sbin and /usr/lib supposed to house **only**
binaries and respective libraries that are **not** required for
boot-up? If so, then the right solution would be to move those
required binaries to /bin, /sbin.

Not supporting a separate /usr would be a major setback for systemd, IMO.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] /usr on separate file system

[Tomasz Torcz]

On Thu, Mar 03, 2011 at 05:39:03PM -0300, Pablo Hess wrote:
> Not supporting a separate /usr would be a major setback for systemd, IMO.

  Separate /usr has nothing to do with systemd.  It just the way current 
distribution
work.  Systemd is just a messenger, don't shot it because of friendly reminder.

-- 
Tomasz Torcz   72->|   80->|
xmpp: zdzich...@chrome.pl  72->|   80->|

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

Re: [systemd-devel] /usr on separate file system

[Lennart Poettering]

On Thu, 03.03.11 17:39, Pablo Hess (natunobi...@gentoobr.org) wrote:

> 
> >> Would it work better if /usr was an automounted target?
> >
> > That would probably blow up in your face, since a lot of programs used
> > during early boot end up accessing /usr and would stay stuck
> 
> Aren't /usr/bin and /usr/sbin and /usr/lib supposed to house **only**
> binaries and respective libraries that are **not** required for
> boot-up? If so, then the right solution would be to move those
> required binaries to /bin, /sbin.

Well, that's not the status quo. Quite a few programs install udev rules
that refer to binaries, libraries or data file in /usr. And the question
is really whether it's worth moving all those files. I.e. do you really
want the PCI/USB id databse in /lib? I am don't think so.

But really, I feel like I keep repeating myself like a broken
record. Please read up this thread, the LWN thread and the README of
systemd before keeping asking the same questions over and over again. I
do believe everything has already been said on this topic.

> Not supporting a separate /usr would be a major setback for systemd, IMO.

Why? systemd just warns you. systemd itself works fine with sperate
/usr. It's just a statement on the general ecosystem, a statement of
fact on the status quo.

systemd is just the messenger. Don't shoot the messenger.

And even if systemd was actively broken in supporting separate /usr I
fail to see how this would constitue a "major setback"...

Lennart

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

Re: [systemd-devel] /usr on separate file system

[Matthew Miller]

On Thu, Mar 03, 2011 at 09:51:52PM +0100, Lennart Poettering wrote:
> Why? systemd just warns you. systemd itself works fine with sperate
> /usr. It's just a statement on the general ecosystem, a statement of
> fact on the status quo.
> 
> systemd is just the messenger. Don't shoot the messenger.

If systemd works fine and you'll continue to commit to making it not be part
of the problem, I don't see why it *should* be the messenger. There's plenty
of ways in which I can seriously misconfigure my system that I don't expect
systemd to warn me about. Basically all you are doing is asking for people
to yell at you. :)

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

Re: [systemd-devel] /usr on separate file system

[Lennart Poettering]

On Thu, 03.03.11 17:02, Matthew Miller (mat...@mattdm.org) wrote:

> 
> On Thu, Mar 03, 2011 at 09:51:52PM +0100, Lennart Poettering wrote:
> > Why? systemd just warns you. systemd itself works fine with sperate
> > /usr. It's just a statement on the general ecosystem, a statement of
> > fact on the status quo.
> > 
> > systemd is just the messenger. Don't shoot the messenger.
> 
> If systemd works fine and you'll continue to commit to making it not be part
> of the problem, I don't see why it *should* be the messenger. There's plenty
> of ways in which I can seriously misconfigure my system that I don't expect
> systemd to warn me about. Basically all you are doing is asking for people
> to yell at you. :)

Well, we already get all those bugs about issues related to broken
separate /usr support all the time. It's a choice between bubonic plague
and cholera: either we get all those bugs and need to help folks track
them down each and every time, or we just educate folks what's going on
but we'll be yelled at while doing so.

It's a bit the same logic the kernel folks used when they introduced the
"tainted" flag for closed source modules.

I think it's better informing people what is going on instead of just
standing back and closing their bugs WONTFIX as they run into them.

Lennart

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