[PATCH shepherd] support: Ignore errors on parent directories in mkdir-p.

[PATCH shepherd] support: Ignore errors on parent directories in mkdir-p.

[David Michael]

* modules/shepherd/support.scm (mkdir-p): Don't throw errors when
  mkdir fails and there are more subdirectory components to try.
---

Hi,

My use case for this is that I have a crazy Hurd setup that boots a
read-only root file system with a passive tmpfs translator on /run.
When mkdir-p runs with "/run/shepherd", it tries to mkdir "/run".  On
Hurd, mkdir first tests for a read-only file system, so mkdir-p catches
and throws EROFS instead of catching and ignoring EEXIST.  The init
process then dies when it tries to stat the non-existent /run/shepherd.

This patch ignores all errors from parent directories, assuming we only
really care about the status of creating the final path component.

Another possibility could be to try to change Hurd's error ordering
instead, but it seems to be acceptably standard behavior:

    If more than one error occurs in processing a function call, any one
    of the possible errors may be returned, as the order of detection is
    undefined.[0]

Can this be applied, or do you prefer another option?

Thanks.

David

[0] http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_03.html

 modules/shepherd/support.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/modules/shepherd/support.scm b/modules/shepherd/support.scm
index 9bc5f5d..3106212 100644
--- a/modules/shepherd/support.scm
+++ b/modules/shepherd/support.scm
@@ -172,7 +172,7 @@ output port, and PROC's result is returned."
                  (mkdir path))
              (loop tail path))
            (lambda args
-             (if (= EEXIST (system-error-errno args))
+             (if (or (not (null? tail)) (= EEXIST (system-error-errno args)))
                  (loop tail path)
                  (apply throw args))))))
       (() #t))))
-- 
2.5.0

Re: [PATCH shepherd] support: Ignore errors on parent directories in mkdir-p.

[Ludovic Courtès]

David Michael <fedora.dm0@gmail.com> skribis:

> My use case for this is that I have a crazy Hurd setup that boots a
> read-only root file system with a passive tmpfs translator on /run.
> When mkdir-p runs with "/run/shepherd", it tries to mkdir "/run".  On
> Hurd, mkdir first tests for a read-only file system, so mkdir-p catches
> and throws EROFS instead of catching and ignoring EEXIST.  The init
> process then dies when it tries to stat the non-existent /run/shepherd.
>
> This patch ignores all errors from parent directories, assuming we only
> really care about the status of creating the final path component.
>
> Another possibility could be to try to change Hurd's error ordering
> instead, but it seems to be acceptably standard behavior:
>
>     If more than one error occurs in processing a function call, any one
>     of the possible errors may be returned, as the order of detection is
>     undefined.[0]

Interesting!

I think that it’s a case where it would be beneficial for the Hurd to
follow what Linux does, which is to return EEXIST.

How does Coreutils’ ‘mkdir -p’ behave in this situation?  (I’ve looked
at mkdir-p.c in Gnulib but it’s a bit complicated…)

> Can this be applied, or do you prefer another option?

I would prefer not to hide the initial error like the proposed patch
does.

OTOH, it’s no big deal, so if it turns out to be too much of a problem
or adds too much latency to wait for the Hurd fix, we could apply this
patch.

WDYT?

Ludo’.

Re: [PATCH shepherd] support: Ignore errors on parent directories in mkdir-p.

[David Michael]

On Thu, Feb 4, 2016 at 11:47 AM, Ludovic Courtès <ludo@gnu.org> wrote:
> David Michael <fedora.dm0@gmail.com> skribis:
>
>> My use case for this is that I have a crazy Hurd setup that boots a
>> read-only root file system with a passive tmpfs translator on /run.
>> When mkdir-p runs with "/run/shepherd", it tries to mkdir "/run".  On
>> Hurd, mkdir first tests for a read-only file system, so mkdir-p catches
>> and throws EROFS instead of catching and ignoring EEXIST.  The init
>> process then dies when it tries to stat the non-existent /run/shepherd.
>>
>> This patch ignores all errors from parent directories, assuming we only
>> really care about the status of creating the final path component.
>>
>> Another possibility could be to try to change Hurd's error ordering
>> instead, but it seems to be acceptably standard behavior:
>>
>>     If more than one error occurs in processing a function call, any one
>>     of the possible errors may be returned, as the order of detection is
>>     undefined.[0]
>
> Interesting!
>
> I think that it’s a case where it would be beneficial for the Hurd to
> follow what Linux does, which is to return EEXIST.
>
> How does Coreutils’ ‘mkdir -p’ behave in this situation?  (I’ve looked
> at mkdir-p.c in Gnulib but it’s a bit complicated…)

After a quick glance it basically looks like after any error occurs
from a mkdir call, it tests if the path exists and is a directory, and
if so, proceeds ignoring that mkdir error.  If mkdir fails and the
directory doesn't exist afterwards, then the mkdir-p call fails with
that error.

That sounds like the best option to me: keeping mkdir-p independent of
the mkdir implementation, while preserving the error from the first
real problem it encounters.  I'll send a patch with that change
instead.

If you'd still prefer to change the Hurd behavior, we could copy the
bug-hurd list to discuss, since I'm not sure if such a change will
have unintended consequences elsewhere.

Thanks.

David

Re: [PATCH shepherd] support: Ignore errors on parent directories in mkdir-p.

[Ludovic Courtès]

David Michael <fedora.dm0@gmail.com> skribis:

> On Thu, Feb 4, 2016 at 11:47 AM, Ludovic Courtès <ludo@gnu.org> wrote:

[...]

>> I think that it’s a case where it would be beneficial for the Hurd to
>> follow what Linux does, which is to return EEXIST.
>>
>> How does Coreutils’ ‘mkdir -p’ behave in this situation?  (I’ve looked
>> at mkdir-p.c in Gnulib but it’s a bit complicated…)
>
> After a quick glance it basically looks like after any error occurs
> from a mkdir call, it tests if the path exists and is a directory, and
> if so, proceeds ignoring that mkdir error.  If mkdir fails and the
> directory doesn't exist afterwards, then the mkdir-p call fails with
> that error.
>
> That sounds like the best option to me: keeping mkdir-p independent of
> the mkdir implementation, while preserving the error from the first
> real problem it encounters.  I'll send a patch with that change
> instead.

OK, makes sense, though it incurs an additional ‘stat’ call.

> If you'd still prefer to change the Hurd behavior, we could copy the
> bug-hurd list to discuss, since I'm not sure if such a change will
> have unintended consequences elsewhere.

I think we should do both: submit the Hurd change, which I think is a
good thing in the long run, and work around the problem here.

Ludo’.