[PATCH] Fix switch_root/pivot_root cut-n-paste error in configure.ac

[PATCH] Fix switch_root/pivot_root cut-n-paste error in configure.ac

[Andreas Henriksson]

The "linux only" check for pivot_root seems to suffer from
a cut-n-paste problem from the earlier switch_root part.

Signed-off-by: Andreas Henriksson <andreas@fatal.se>
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index db53dff..aae2456 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1193,7 +1193,7 @@ AC_ARG_ENABLE([pivot_root],
   [], [UL_DEFAULT_ENABLE([pivot_root], [check])]
 )
 UL_BUILD_INIT([pivot_root])
-UL_REQUIRES_LINUX([switch_root])
+UL_REQUIRES_LINUX([pivot_root])
 UL_REQUIRES_SYSCALL_CHECK([pivot_root], [UL_CHECK_SYSCALL([pivot_root])])
 AM_CONDITIONAL([BUILD_PIVOT_ROOT], [test "x$build_pivot_root" = xyes])
 
-- 
2.0.1

Re: [PATCH] Fix switch_root/pivot_root cut-n-paste error in configure.ac

[Karel Zak]

On Wed, Jul 23, 2014 at 07:39:02PM +0200, Andreas Henriksson wrote:
>  configure.ac | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

 Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH] Fix switch_root/pivot_root cut-n-paste error in configure.ac

[Dave Reisner]

On Wed, Jul 23, 2014 at 07:39:02PM +0200, Andreas Henriksson wrote:
> The "linux only" check for pivot_root seems to suffer from
> a cut-n-paste problem from the earlier switch_root part.
> 
> Signed-off-by: Andreas Henriksson <andreas@fatal.se>
> ---
>  configure.ac | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/configure.ac b/configure.ac
> index db53dff..aae2456 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1193,7 +1193,7 @@ AC_ARG_ENABLE([pivot_root],
>    [], [UL_DEFAULT_ENABLE([pivot_root], [check])]
>  )
>  UL_BUILD_INIT([pivot_root])
> -UL_REQUIRES_LINUX([switch_root])
> +UL_REQUIRES_LINUX([pivot_root])

Is this really correct? Seems to me like the check should simply be
removed. In sys-utils/pivot_root.c, we have the following:

  #define pivot_root(new_root,put_old) syscall(SYS_pivot_root,new_root,put_old)

So, the subsequent check for the syscall seems like the only necessary
check needed. As is, I think that u-l now fails to build in places where
the pivot_root wrapper doesn't exist in the libc, but the syscall exists
(does such a platform even exist?).

Cheers,
dave

>  UL_REQUIRES_SYSCALL_CHECK([pivot_root], [UL_CHECK_SYSCALL([pivot_root])])
>  AM_CONDITIONAL([BUILD_PIVOT_ROOT], [test "x$build_pivot_root" = xyes])
>  
> -- 
> 2.0.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe util-linux" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Fix switch_root/pivot_root cut-n-paste error in configure.ac

[Andreas Henriksson]

Hello Dave Reisner!

On Thu, Jul 24, 2014 at 12:52:26PM -0400, Dave Reisner wrote:
> Is this really correct? Seems to me like the check should simply be
> removed. In sys-utils/pivot_root.c, we have the following:
> 
>   #define pivot_root(new_root,put_old) syscall(SYS_pivot_root,new_root,put_old)
> 
> So, the subsequent check for the syscall seems like the only necessary
> check needed. As is, I think that u-l now fails to build in places where
> the pivot_root wrapper doesn't exist in the libc, but the syscall exists
> (does such a platform even exist?).

I cought this when trying to build on Debian GNU/kFreeBSD (with incorrect
configure flags for that platform) where the SYS_pivot_root check
failed the build.
Not knowing anything about non-linux architectures myself it was not obvious
to me why this was.

Passing --disable-pivot_root fixed the build and it would be more
obvious that this is the correct thing to do if it was pointed out that
this is only expected to work on Linux.
I consider the UL_REQUIRES_LINUX check as a more obvious
way to point out this error so people like myself doesn't need to
spend so much time scratching their heads over this.

>From what I can tell, Linux is the only system offering this syscall anyway.
Do you know of any non-linux system offering this?
If not, then I think the check is still correct and helpful.

Regards,
Andreas Henriksson

Re: [PATCH] Fix switch_root/pivot_root cut-n-paste error in configure.ac

[Dave Reisner]

On Thu, Jul 24, 2014 at 07:19:24PM +0200, Andreas Henriksson wrote:
> Hello Dave Reisner!
> 
> On Thu, Jul 24, 2014 at 12:52:26PM -0400, Dave Reisner wrote:
> > Is this really correct? Seems to me like the check should simply be
> > removed. In sys-utils/pivot_root.c, we have the following:
> > 
> >   #define pivot_root(new_root,put_old) syscall(SYS_pivot_root,new_root,put_old)
> > 
> > So, the subsequent check for the syscall seems like the only necessary
> > check needed. As is, I think that u-l now fails to build in places where
> > the pivot_root wrapper doesn't exist in the libc, but the syscall exists
> > (does such a platform even exist?).
> 
> I cought this when trying to build on Debian GNU/kFreeBSD (with incorrect
> configure flags for that platform) where the SYS_pivot_root check
> failed the build.
> Not knowing anything about non-linux architectures myself it was not obvious
> to me why this was.
> 
> Passing --disable-pivot_root fixed the build and it would be more
> obvious that this is the correct thing to do if it was pointed out that
> this is only expected to work on Linux.
> I consider the UL_REQUIRES_LINUX check as a more obvious
> way to point out this error so people like myself doesn't need to
> spend so much time scratching their heads over this.
> 
> From what I can tell, Linux is the only system offering this syscall anyway.
> Do you know of any non-linux system offering this?
> If not, then I think the check is still correct and helpful.

Ah, I wrongly assumed what UL_REQUIRES_LINUX does. Makes more sense
having now read the .m4 file, and I agree with your fix.

d

Re: [PATCH] Fix switch_root/pivot_root cut-n-paste error in configure.ac

[Karel Zak]

On Thu, Jul 24, 2014 at 01:26:27PM -0400, Dave Reisner wrote:
> Ah, I wrongly assumed what UL_REQUIRES_LINUX does. Makes more sense
> having now read the .m4 file, and I agree with your fix.

 Yep, I have created UL_REQUIRES_ to have an elegant way how to specify
 dependences between --disable-* options, utils, libs (with in package) 
 and external requirements. One day I'd like to write a blog about it ;-)

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH] Fix switch_root/pivot_root cut-n-paste error in configure.ac

[Karel Zak]

On Thu, Jul 24, 2014 at 12:52:26PM -0400, Dave Reisner wrote:
> On Wed, Jul 23, 2014 at 07:39:02PM +0200, Andreas Henriksson wrote:
> > The "linux only" check for pivot_root seems to suffer from
> > a cut-n-paste problem from the earlier switch_root part.
> > 
> > Signed-off-by: Andreas Henriksson <andreas@fatal.se>
> > ---
> >  configure.ac | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index db53dff..aae2456 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -1193,7 +1193,7 @@ AC_ARG_ENABLE([pivot_root],
> >    [], [UL_DEFAULT_ENABLE([pivot_root], [check])]
> >  )
> >  UL_BUILD_INIT([pivot_root])
> > -UL_REQUIRES_LINUX([switch_root])
> > +UL_REQUIRES_LINUX([pivot_root])
> 
> Is this really correct? Seems to me like the check should simply be
> removed. 

 It's definitely better to check that the system is Linux before you
 start to check for something more complicated. There is logical "and"
 between all UL_REQUIRES_*, so the first unsuccessful check disabled
 the rest.

> In sys-utils/pivot_root.c, we have the following:
> 
>   #define pivot_root(new_root,put_old) syscall(SYS_pivot_root,new_root,put_old)
> 
> So, the subsequent check for the syscall seems like the only necessary
> check needed. As is, I think that u-l now fails to build in places where
> the pivot_root wrapper doesn't exist in the libc, but the syscall exists
> (does such a platform even exist?).

 There is no glibc wrapper at all, in code we always call the syscall
 and in configure.ac we check for SYS_pivot_root sycall on Linux.
 Nowhere we care about libc.

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com