You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@mesos.apache.org by Tomasz Janiszewski <ja...@gmail.com> on 2016/04/24 13:36:33 UTC

Review Request 46610: Fix 'pivot_root is not available' error on ARM.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46610/
-----------------------------------------------------------

Review request for mesos and haosdent huang.


Repository: mesos


Description
-------

Fix 'pivot_root is not available' error on ARM.


Diffs
-----

  src/linux/fs.cpp 2087b4ac1503e0fd085319b1017389f1f947536f 

Diff: https://reviews.apache.org/r/46610/diff/


Testing
-------


Thanks,

Tomasz Janiszewski

Re: Review Request 46610: Fix 'pivot_root is not available' error on ARM.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46610/#review130305
-----------------------------------------------------------



Patch looks great!

Reviews applied: [46610]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On April 24, 2016, 11:36 a.m., Tomasz Janiszewski wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46610/
> -----------------------------------------------------------
> 
> (Updated April 24, 2016, 11:36 a.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Fix 'pivot_root is not available' error on ARM.
> 
> 
> Diffs
> -----
> 
>   src/linux/fs.cpp 2087b4ac1503e0fd085319b1017389f1f947536f 
> 
> Diff: https://reviews.apache.org/r/46610/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 46610: Fix 'pivot_root is not available' error on ARM.

Posted by Tomasz Janiszewski <ja...@gmail.com>.


> On April 24, 2016, 12:40 p.m., haosdent huang wrote:
> >
> 
> haosdent huang wrote:
>     In additional, could you add @chenzhiwei and @vinodkone in reviewers? I think powerpc could drop
>     
>     ```
>     #elif __powerpc__ || __ppc__ || __powerpc64__ || __ppc64__
>       // A workaround for powerpc. The magic number '203' is the syscall
>       // number for 'pivot_root' on the powerpc architecture, see
>       // https://w3challs.com/syscalls/?arch=powerpc_64
>       int ret = ::syscall(203, newRoot.c_str(), putOld.c_str());
>     ```
>     
>     as well. And you need fill a ticket for this in https://issues.apache.org/jira/browse/MESOS and find a shepherd by sending email to dev mailing list.
> 
> Tomasz Janiszewski wrote:
>     It looks like powerpc is properly handled with [203](http://lxr.free-electrons.com/source/arch/powerpc/include/uapi/asm/unistd.h#L218)

Can we also drop
```
#elif __x86_64__
  // A workaround for systems that have an old glib but have a new
  // kernel. The magic number '155' is the syscall number for
  // 'pivot_root' on the x86_64 architecture, see
  // arch/x86/syscalls/syscall_64.tbl
  int ret = ::syscall(155, newRoot.c_str(), putOld.c_str());
```
?
If no then we probably can't drop powerpc and arm section.


- Tomasz


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46610/#review130303
-----------------------------------------------------------


On April 24, 2016, 11:36 a.m., Tomasz Janiszewski wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46610/
> -----------------------------------------------------------
> 
> (Updated April 24, 2016, 11:36 a.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Fix 'pivot_root is not available' error on ARM.
> 
> 
> Diffs
> -----
> 
>   src/linux/fs.cpp 2087b4ac1503e0fd085319b1017389f1f947536f 
> 
> Diff: https://reviews.apache.org/r/46610/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 46610: Fix 'pivot_root is not available' error on ARM.

Posted by haosdent <ha...@gmail.com>.

Thank you for you quick reply! `src/linux/fs.cpp` only effective under
Linux, so I think @Tomasz's patch should be fine. I verify add

```
#include <linux/unistd.h>
```

works on Ubuntu 14.04, CentOS 6, CentOS 7.

Anyway, let's see what others concerns after @Tomasz find a shepherd. Thank
you all!


On Mon, Apr 25, 2016 at 10:38 AM, zhiwei <zh...@gmail.com> wrote:

> According to the initial comment "A workaround for systems that have an
> old glib but have a new kernel" made by Ian, I am not sure which version of
> glib is old and which version of kernel if new.
>
> At the previous commit, we included a syscall.h to fix this issue[1], but
> I am not sure this is the right way, I asked Ian but did not get response,
> then I added an elseif which is the safe way.
>
> In my previous testing, I can successfully build Mesos by including the
> header file on RHEL7.x, Ubuntu 14.04(both x86_64 and ppc64le).
>
> I think it's ok to include a header file to solve this issue on all the
> platforms, but should at least make sure this works on the main OS and
> version(RHEL 7.x, Ubuntu 14.04/16.04 and others? Windows? Mac?).
>
>
> [1]: https://reviews.apache.org/r/42551/
>
>
> Thanks,
> Zhiwei
>
> On Mon, Apr 25, 2016 at 9:05 AM, haosdent <ha...@gmail.com> wrote:
>
>> Hi @idownes and @chenzhiwei. @Tomasz proposal to add the correct header
>> file `<linux/unistd.h>` for pivot_root syscall. According my understand, we
>> could drop these workaround code:
>>
>> ```
>> #elif __x86_64__
>>   // A workaround for systems that have an old glib but have a new
>>   // kernel. The magic number '155' is the syscall number for
>>   // 'pivot_root' on the x86_64 architecture, see
>>   // arch/x86/syscalls/syscall_64.tbl
>>   int ret = ::syscall(155, newRoot.c_str(), putOld.c_str());
>> ```
>>
>> ```
>> #elif __powerpc__ || __ppc__ || __powerpc64__ || __ppc64__
>>   // A workaround for powerpc. The magic number '203' is the syscall
>>   // number for 'pivot_root' on the powerpc architecture, see
>>   // https://w3challs.com/syscalls/?arch=powerpc_64
>>   int ret = ::syscall(203, newRoot.c_str(), putOld.c_str());
>> ```
>>
>> Would you help double check at your convenience? Thank you in advance.
>>
>> On Mon, Apr 25, 2016 at 9:00 AM, haosdent huang <ha...@gmail.com>
>> wrote:
>>
>>> This is an automatically generated e-mail. To reply, visit:
>>> https://reviews.apache.org/r/46610/
>>>
>>> On April 24th, 2016, 12:42 p.m. UTC, *haosdent huang* wrote:
>>>
>>> In additional, could you add @chenzhiwei and @vinodkone in reviewers? I think powerpc could drop
>>>
>>> #elif __powerpc__ || __ppc__ || __powerpc64__ || __ppc64__
>>>   // A workaround for powerpc. The magic number '203' is the syscall
>>>   // number for 'pivot_root' on the powerpc architecture, see
>>>   // https://w3challs.com/syscalls/?arch=powerpc_64
>>>   int ret = ::syscall(203, newRoot.c_str(), putOld.c_str());
>>>
>>> as well. And you need fill a ticket for this in
>>> https://issues.apache.org/jira/browse/MESOS and find a shepherd by
>>> sending email to dev mailing list.
>>>
>>> On April 24th, 2016, 7:57 p.m. UTC, *Tomasz Janiszewski* wrote:
>>>
>>> It looks like powerpc is properly handled with 203 <http://lxr.free-electrons.com/source/arch/powerpc/include/uapi/asm/unistd.h#L218>
>>>
>>> On April 24th, 2016, 8:19 p.m. UTC, *Tomasz Janiszewski* wrote:
>>>
>>> Can we also drop
>>>
>>> #elif __x86_64__
>>>   // A workaround for systems that have an old glib but have a new
>>>   // kernel. The magic number '155' is the syscall number for
>>>   // 'pivot_root' on the x86_64 architecture, see
>>>   // arch/x86/syscalls/syscall_64.tbl
>>>   int ret = ::syscall(155, newRoot.c_str(), putOld.c_str());
>>>
>>> ? If no then we probably can't drop powerpc and arm section.
>>>
>>> On April 25th, 2016, 12:32 a.m. UTC, *haosdent huang* wrote:
>>>
>>> Yes, I think could remove as well. I double check x86, arm, arm64, powerpc in 3.8 just now. <linux/unistd.h> already inlcudes them by <asm-generic/unistd.h> or <uapi/asm/unistd.h>.
>>>
>>> On April 25th, 2016, 12:36 a.m. UTC, *haosdent huang* wrote:
>>>
>>> By the way, we usually fill ticket number in Bugs: field in review board. You can fill MESOS-5263 it here.
>>>
>>> I need @idownes and @chenzhiwei to help check whether we could drop it after add the correct header file.
>>>
>>>
>>> - haosdent
>>>
>>> On April 24th, 2016, 11:36 a.m. UTC, Tomasz Janiszewski wrote:
>>> Review request for mesos and haosdent huang.
>>> By Tomasz Janiszewski.
>>>
>>> *Updated April 24, 2016, 11:36 a.m.*
>>> *Repository: * mesos
>>> Description
>>>
>>> Fix 'pivot_root is not available' error on ARM.
>>>
>>> Diffs
>>>
>>>    - src/linux/fs.cpp (2087b4ac1503e0fd085319b1017389f1f947536f)
>>>
>>> View Diff <https://reviews.apache.org/r/46610/diff/>
>>>
>>
>>
>>
>> --
>> Best Regards,
>> Haosdent Huang
>>
>
>


-- 
Best Regards,
Haosdent Huang

Re: Review Request 46610: Fix 'pivot_root is not available' error on ARM.

Posted by zhiwei <zh...@gmail.com>.

According to the initial comment "A workaround for systems that have an old
glib but have a new kernel" made by Ian, I am not sure which version of
glib is old and which version of kernel if new.

At the previous commit, we included a syscall.h to fix this issue[1], but I
am not sure this is the right way, I asked Ian but did not get response,
then I added an elseif which is the safe way.

In my previous testing, I can successfully build Mesos by including the
header file on RHEL7.x, Ubuntu 14.04(both x86_64 and ppc64le).

I think it's ok to include a header file to solve this issue on all the
platforms, but should at least make sure this works on the main OS and
version(RHEL 7.x, Ubuntu 14.04/16.04 and others? Windows? Mac?).


[1]: https://reviews.apache.org/r/42551/


Thanks,
Zhiwei

On Mon, Apr 25, 2016 at 9:05 AM, haosdent <ha...@gmail.com> wrote:

> Hi @idownes and @chenzhiwei. @Tomasz proposal to add the correct header
> file `<linux/unistd.h>` for pivot_root syscall. According my understand, we
> could drop these workaround code:
>
> ```
> #elif __x86_64__
>   // A workaround for systems that have an old glib but have a new
>   // kernel. The magic number '155' is the syscall number for
>   // 'pivot_root' on the x86_64 architecture, see
>   // arch/x86/syscalls/syscall_64.tbl
>   int ret = ::syscall(155, newRoot.c_str(), putOld.c_str());
> ```
>
> ```
> #elif __powerpc__ || __ppc__ || __powerpc64__ || __ppc64__
>   // A workaround for powerpc. The magic number '203' is the syscall
>   // number for 'pivot_root' on the powerpc architecture, see
>   // https://w3challs.com/syscalls/?arch=powerpc_64
>   int ret = ::syscall(203, newRoot.c_str(), putOld.c_str());
> ```
>
> Would you help double check at your convenience? Thank you in advance.
>
> On Mon, Apr 25, 2016 at 9:00 AM, haosdent huang <ha...@gmail.com>
> wrote:
>
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/46610/
>>
>> On April 24th, 2016, 12:42 p.m. UTC, *haosdent huang* wrote:
>>
>> In additional, could you add @chenzhiwei and @vinodkone in reviewers? I think powerpc could drop
>>
>> #elif __powerpc__ || __ppc__ || __powerpc64__ || __ppc64__
>>   // A workaround for powerpc. The magic number '203' is the syscall
>>   // number for 'pivot_root' on the powerpc architecture, see
>>   // https://w3challs.com/syscalls/?arch=powerpc_64
>>   int ret = ::syscall(203, newRoot.c_str(), putOld.c_str());
>>
>> as well. And you need fill a ticket for this in
>> https://issues.apache.org/jira/browse/MESOS and find a shepherd by
>> sending email to dev mailing list.
>>
>> On April 24th, 2016, 7:57 p.m. UTC, *Tomasz Janiszewski* wrote:
>>
>> It looks like powerpc is properly handled with 203 <http://lxr.free-electrons.com/source/arch/powerpc/include/uapi/asm/unistd.h#L218>
>>
>> On April 24th, 2016, 8:19 p.m. UTC, *Tomasz Janiszewski* wrote:
>>
>> Can we also drop
>>
>> #elif __x86_64__
>>   // A workaround for systems that have an old glib but have a new
>>   // kernel. The magic number '155' is the syscall number for
>>   // 'pivot_root' on the x86_64 architecture, see
>>   // arch/x86/syscalls/syscall_64.tbl
>>   int ret = ::syscall(155, newRoot.c_str(), putOld.c_str());
>>
>> ? If no then we probably can't drop powerpc and arm section.
>>
>> On April 25th, 2016, 12:32 a.m. UTC, *haosdent huang* wrote:
>>
>> Yes, I think could remove as well. I double check x86, arm, arm64, powerpc in 3.8 just now. <linux/unistd.h> already inlcudes them by <asm-generic/unistd.h> or <uapi/asm/unistd.h>.
>>
>> On April 25th, 2016, 12:36 a.m. UTC, *haosdent huang* wrote:
>>
>> By the way, we usually fill ticket number in Bugs: field in review board. You can fill MESOS-5263 it here.
>>
>> I need @idownes and @chenzhiwei to help check whether we could drop it after add the correct header file.
>>
>>
>> - haosdent
>>
>> On April 24th, 2016, 11:36 a.m. UTC, Tomasz Janiszewski wrote:
>> Review request for mesos and haosdent huang.
>> By Tomasz Janiszewski.
>>
>> *Updated April 24, 2016, 11:36 a.m.*
>> *Repository: * mesos
>> Description
>>
>> Fix 'pivot_root is not available' error on ARM.
>>
>> Diffs
>>
>>    - src/linux/fs.cpp (2087b4ac1503e0fd085319b1017389f1f947536f)
>>
>> View Diff <https://reviews.apache.org/r/46610/diff/>
>>
>
>
>
> --
> Best Regards,
> Haosdent Huang
>