Re: svn commit: r278323 - in head: etc/rc.d usr.sbin/jail

2015-02-12 Thread Garrett Cooper
On Feb 12, 2015, at 13:12, Garrett Cooper  wrote:

> On Feb 9, 2015, at 18:51, James Gritton  wrote:
> 
>> On 2015-02-06 22:23, Garrett Cooper wrote:
>>> On Feb 6, 2015, at 18:38, James Gritton  wrote:
 On 2015-02-06 19:23, Garrett Cooper wrote:
> I think you broke the Jenkins tests runs, and potentially jail support
> in some edgecases:
> https://jenkins.freebsd.org/job/FreeBSD_HEAD-tests2/651/
 Where do I go from here?  There error you refer to certainly seems 
 jail-related, which leads me to guess at something disconnected between 
 the matching rc.d/jail and jail(8) change (i.e. using the new rc file with 
 the old jail program).  But that's really just a wild guess.  Is there 
 somewhere I look for more information?  For example, where does Jenkins 
 actually do its thing?
 Sorry for being so stupid in this - Jenkins has only been on the very edge 
 of my awareness until now.
>>> I honestly don’t think it’s Jenkins because Jenkins runs in bhyve. I
>>> think you accidentally broke option handling in the jail configuration
>>> (please see my other reply about added “break;” statements).
>>> ...
>>> You can verify your changes by doing:
>>> % (cd /usr/tests/bin/pkill; sudo kyua test)
>> 
>> After some testing and looking around, I've decided the problem definitely 
>> isn't in rc.d where I thought it might be.  I've also decided it's probably 
>> not in my patch either.
>> 
>> I've run this kyua test on a 10 system (don't have current handy for such 
>> things at the moment), and sometimes I would see a failure and sometimes I 
>> wouldn't.  This was whether I was using the new or old jail code.  Later in 
>> the day, when the box was less loaded, it seemed to always pass.  Looking at 
>> the pkill-j_test script, I see jails being created with sleep commands both 
>> inside and outside the jail around its creation.  I'm guessing this script 
>> is very sensitive to timing issues that could be cause by (among other 
>> things) system load.  The jail commands in this script were also very 
>> simple, with the only parameters used being: path, name, ip4.addr, and 
>> command.  This isn't some kind of esoteric exercising of the jail(8) 
>> options, and I would expect if it works at one time it would work at 
>> another.  I've "hand-run" these particular jail commands and couldn't get 
>> them to fail (and the actual content of the jail(8) changes were tests 
>> already).
>> 
>> I looked at the freebsd-current (I think) list where the Jenkins errors are 
>> posted, and it's true it started failing the pkill-j test at the time I made 
>> my change. But it's also true that it had failed that test once the day 
>> before my change, and then started passing it again.  This particular test 
>> just seems to be fragile.
>> 
>> So I don't have anywhere else to go with this.  I'm going to assume jail(8) 
>> isn't the problem here.
> 
> The tests are racy and make some interesting assumptions. It appears that 
> WITNESS plays a part in it, and I bet VIMAGE (something that I don’t have in 
> my kernel config) plays a part in it too. I say this because I just ran into 
> the issue when running the tests in a tight loop on my VMware workstation 7 
> instance with code from r278636.
> 
> Doesn’t surprise me because before r272305, it was failing consistently on 
> head, so what Craig did in that commit helped, but it didn’t fully fix the 
> raciness of the tests.
> 
> I’m going to recompile my system with VIMAGE and see if that impacts 
> performance of the tests, and if so, I’ll adjust the sleep between setting up 
> the jailed instances, and waiting for them to be fully formed.
> 
> Thanks!
> 
> $ while : ; do sudo prove -rv pgrep-j_test.sh || break; done
> pgrep-j_test.sh .. 
> 1..3
> usage: pgrep [-LSfilnoqvx] [-d delim] [-F pidfile] [-G gid] [-M core] [-N 
> system]
> [-P ppid] [-U uid] [-c class] [-g pgrp] [-j jid]
> [-s sid] [-t tty] [-u euid] pattern ...
> not ok 1 - pgrep -j  # pgrep output: '', pidfile output: '74275 74278'
> ok 2 - pgrep -j any
> ok 3 - pgrep -j none
> Failed 1/3 subtests 
> 
> Test Summary Report
> ---
> pgrep-j_test.sh (Wstat: 0 Tests: 3 Failed: 1)
>  Failed test:  1
> Files=1, Tests=3,  5 wallclock secs ( 0.04 usr  0.02 sys +  0.02 cusr  0.55 
> csys =  0.63 CPU)
> Result: FAIL

This Jenkins run is interesting: 
https://jenkins.freebsd.org/job/FreeBSD_HEAD-tests2/686/testReport/junit/bin.pkill/pgrep-j_test/main/
 . The first run passed, but the second one didn’t (more output than expected). 
This error shouldn’t occur after r278636, but it definitely confirms the fact 
that the test is racy, in other ways.


signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: svn commit: r278323 - in head: etc/rc.d usr.sbin/jail

2015-02-12 Thread Garrett Cooper
On Feb 9, 2015, at 18:51, James Gritton  wrote:

> On 2015-02-06 22:23, Garrett Cooper wrote:
>> On Feb 6, 2015, at 18:38, James Gritton  wrote:
>>> On 2015-02-06 19:23, Garrett Cooper wrote:
 I think you broke the Jenkins tests runs, and potentially jail support
 in some edgecases:
 https://jenkins.freebsd.org/job/FreeBSD_HEAD-tests2/651/
>>> Where do I go from here?  There error you refer to certainly seems 
>>> jail-related, which leads me to guess at something disconnected between the 
>>> matching rc.d/jail and jail(8) change (i.e. using the new rc file with the 
>>> old jail program).  But that's really just a wild guess.  Is there 
>>> somewhere I look for more information?  For example, where does Jenkins 
>>> actually do its thing?
>>> Sorry for being so stupid in this - Jenkins has only been on the very edge 
>>> of my awareness until now.
>> I honestly don’t think it’s Jenkins because Jenkins runs in bhyve. I
>> think you accidentally broke option handling in the jail configuration
>> (please see my other reply about added “break;” statements).
>> ...
>> You can verify your changes by doing:
>> % (cd /usr/tests/bin/pkill; sudo kyua test)
> 
> After some testing and looking around, I've decided the problem definitely 
> isn't in rc.d where I thought it might be.  I've also decided it's probably 
> not in my patch either.
> 
> I've run this kyua test on a 10 system (don't have current handy for such 
> things at the moment), and sometimes I would see a failure and sometimes I 
> wouldn't.  This was whether I was using the new or old jail code.  Later in 
> the day, when the box was less loaded, it seemed to always pass.  Looking at 
> the pkill-j_test script, I see jails being created with sleep commands both 
> inside and outside the jail around its creation.  I'm guessing this script is 
> very sensitive to timing issues that could be cause by (among other things) 
> system load.  The jail commands in this script were also very simple, with 
> the only parameters used being: path, name, ip4.addr, and command.  This 
> isn't some kind of esoteric exercising of the jail(8) options, and I would 
> expect if it works at one time it would work at another.  I've "hand-run" 
> these particular jail commands and couldn't get them to fail (and the actual 
> content of the jail(8) changes were tests already).
> 
> I looked at the freebsd-current (I think) list where the Jenkins errors are 
> posted, and it's true it started failing the pkill-j test at the time I made 
> my change.  But it's also true that it had failed that test once the day 
> before my change, and then started passing it again.  This particular test 
> just seems to be fragile.
> 
> So I don't have anywhere else to go with this.  I'm going to assume jail(8) 
> isn't the problem here.

The tests are racy and make some interesting assumptions. It appears that 
WITNESS plays a part in it, and I bet VIMAGE (something that I don’t have in my 
kernel config) plays a part in it too. I say this because I just ran into the 
issue when running the tests in a tight loop on my VMware workstation 7 
instance with code from r278636.

Doesn’t surprise me because before r272305, it was failing consistently on 
head, so what Craig did in that commit helped, but it didn’t fully fix the 
raciness of the tests.

I’m going to recompile my system with VIMAGE and see if that impacts 
performance of the tests, and if so, I’ll adjust the sleep between setting up 
the jailed instances, and waiting for them to be fully formed.

Thanks!

$ while : ; do sudo prove -rv pgrep-j_test.sh || break; done
pgrep-j_test.sh .. 
1..3
usage: pgrep [-LSfilnoqvx] [-d delim] [-F pidfile] [-G gid] [-M core] [-N 
system]
 [-P ppid] [-U uid] [-c class] [-g pgrp] [-j jid]
 [-s sid] [-t tty] [-u euid] pattern ...
not ok 1 - pgrep -j  # pgrep output: '', pidfile output: '74275 74278'
ok 2 - pgrep -j any
ok 3 - pgrep -j none
Failed 1/3 subtests 

Test Summary Report
---
pgrep-j_test.sh (Wstat: 0 Tests: 3 Failed: 1)
  Failed test:  1
Files=1, Tests=3,  5 wallclock secs ( 0.04 usr  0.02 sys +  0.02 cusr  0.55 
csys =  0.63 CPU)
Result: FAIL


signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: svn commit: r278323 - in head: etc/rc.d usr.sbin/jail

2015-02-09 Thread James Gritton

On 2015-02-06 22:23, Garrett Cooper wrote:

On Feb 6, 2015, at 18:38, James Gritton  wrote:

On 2015-02-06 19:23, Garrett Cooper wrote:
I think you broke the Jenkins tests runs, and potentially jail 
support

in some edgecases:
https://jenkins.freebsd.org/job/FreeBSD_HEAD-tests2/651/


Where do I go from here?  There error you refer to certainly seems 
jail-related, which leads me to guess at something disconnected 
between the matching rc.d/jail and jail(8) change (i.e. using the new 
rc file with the old jail program).  But that's really just a wild 
guess.  Is there somewhere I look for more information?  For example, 
where does Jenkins actually do its thing?


Sorry for being so stupid in this - Jenkins has only been on the very 
edge of my awareness until now.


I honestly don’t think it’s Jenkins because Jenkins runs in bhyve. I
think you accidentally broke option handling in the jail configuration
(please see my other reply about added “break;” statements).

...

You can verify your changes by doing:

% (cd /usr/tests/bin/pkill; sudo kyua test)


After some testing and looking around, I've decided the problem 
definitely isn't in rc.d where I thought it might be.  I've also decided 
it's probably not in my patch either.


I've run this kyua test on a 10 system (don't have current handy for 
such things at the moment), and sometimes I would see a failure and 
sometimes I wouldn't.  This was whether I was using the new or old jail 
code.  Later in the day, when the box was less loaded, it seemed to 
always pass.  Looking at the pkill-j_test script, I see jails being 
created with sleep commands both inside and outside the jail around its 
creation.  I'm guessing this script is very sensitive to timing issues 
that could be cause by (among other things) system load.  The jail 
commands in this script were also very simple, with the only parameters 
used being: path, name, ip4.addr, and command.  This isn't some kind of 
esoteric exercising of the jail(8) options, and I would expect if it 
works at one time it would work at another.  I've "hand-run" these 
particular jail commands and couldn't get them to fail (and the actual 
content of the jail(8) changes were tests already).


I looked at the freebsd-current (I think) list where the Jenkins errors 
are posted, and it's true it started failing the pkill-j test at the 
time I made my change.  But it's also true that it had failed that test 
once the day before my change, and then started passing it again.  This 
particular test just seems to be fragile.


So I don't have anywhere else to go with this.  I'm going to assume 
jail(8) isn't the problem here.


- Jamie
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r278323 - in head: etc/rc.d usr.sbin/jail

2015-02-09 Thread NGie Cooper
On Sat, Feb 7, 2015 at 12:10 AM, Konstantin Belousov
 wrote:
> On Fri, Feb 06, 2015 at 09:23:47PM -0800, Garrett Cooper wrote:
>> pgrep uses /proc to determine whether or not a process is running. If it?s 
>> not properly mounted or the jail isn?t started properly, that could cause 
>> the issues seen here. I know because I?ve tried running these tests before 
>> in an attempt to fix them, and this was one of the things I ran into.
>>
> pgrep does not use /proc.

You're right. I'm not sure where I got it in my mind that that was the
case, but this wasn't it..
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r278323 - in head: etc/rc.d usr.sbin/jail

2015-02-07 Thread Konstantin Belousov
On Fri, Feb 06, 2015 at 09:23:47PM -0800, Garrett Cooper wrote:
> pgrep uses /proc to determine whether or not a process is running. If it?s 
> not properly mounted or the jail isn?t started properly, that could cause the 
> issues seen here. I know because I?ve tried running these tests before in an 
> attempt to fix them, and this was one of the things I ran into.
> 
pgrep does not use /proc.
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r278323 - in head: etc/rc.d usr.sbin/jail

2015-02-06 Thread James Gritton



On 2015-02-06 22:31, Garrett Cooper wrote:

On Feb 6, 2015, at 21:27, James Gritton  wrote:


On 2015-02-06 22:18, Garrett Cooper wrote:

On Feb 6, 2015, at 9:54, Jamie Gritton  wrote:

Modified: head/usr.sbin/jail/command.c
==
--- head/usr.sbin/jail/command.cFri Feb  6 17:43:13 2015
(r278322)
+++ head/usr.sbin/jail/command.cFri Feb  6 17:54:53 2015
(r278323)
@@ -112,6 +112,12 @@ next_command(struct cfjail *j)
if (!bool_param(j->intparams[IP_MOUNT_FDESCFS]))
continue;
j->comstring = &dummystring;
+   break;
+   case IP_MOUNT_PROCFS:
+   if (!bool_param(j->intparams[IP_MOUNT_PROCFS]))
+   continue;
+   j->comstring = &dummystring;
+   break;
Did you intend on adding another break? The code would previously 
fall

through to the next case statement...

case IP__OP:
case IP_STOP_TIMEOUT:
j->comstring = &dummystring;


Yes.  The code did indeed previously fall to the next case, but it was 
a no-op: the next case only had the same exact assignment that had 
just taken place (j->comstring = &dummystring).  The lack of a break 
that had existed before was just some sloppy coding that I didn't 
notice at the time because it didn't actually change any behavior.  
Nonetheless it seemed worth correcting when I noticed it.


True. I looked at the code afterwards and it looks ok. mount.procfs
doesn’t exist in my environment. Is that command correct?

$ which mount.procfs; echo $?
1


I added mount.procfs as a jail parameter, but it's not a command.  Just 
like the existing mount.devfs and mount.fdescfs aren't commands either.  
The reason these jail parameters exist is to ease the backward 
compatibility with the old rc-based jail system.  It should be a simple 
case of doing for procfs exactly what I did for the other two, but 
apparently it isn't.  It's likely related to something I'm missing in 
the proper way of modifying rc scripts.


- Jamie
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r278323 - in head: etc/rc.d usr.sbin/jail

2015-02-06 Thread Garrett Cooper
On Feb 6, 2015, at 21:31, Garrett Cooper  wrote:

> On Feb 6, 2015, at 21:27, James Gritton  wrote:
> 
>> On 2015-02-06 22:18, Garrett Cooper wrote:
>>> On Feb 6, 2015, at 9:54, Jamie Gritton  wrote:
 Modified: head/usr.sbin/jail/command.c
 ==
 --- head/usr.sbin/jail/command.c   Fri Feb  6 17:43:13 2015
 (r278322)
 +++ head/usr.sbin/jail/command.c   Fri Feb  6 17:54:53 2015
 (r278323)
 @@ -112,6 +112,12 @@ next_command(struct cfjail *j)
if (!bool_param(j->intparams[IP_MOUNT_FDESCFS]))
continue;
j->comstring = &dummystring;
 +  break;
 +  case IP_MOUNT_PROCFS:
 +  if (!bool_param(j->intparams[IP_MOUNT_PROCFS]))
 +  continue;
 +  j->comstring = &dummystring;
 +  break;
>>> Did you intend on adding another break? The code would previously fall
>>> through to the next case statement...
case IP__OP:
case IP_STOP_TIMEOUT:
j->comstring = &dummystring;
>> 
>> Yes.  The code did indeed previously fall to the next case, but it was a 
>> no-op: the next case only had the same exact assignment that had just taken 
>> place (j->comstring = &dummystring).  The lack of a break that had existed 
>> before was just some sloppy coding that I didn't notice at the time because 
>> it didn't actually change any behavior.  Nonetheless it seemed worth 
>> correcting when I noticed it.
> 
> True. I looked at the code afterwards and it looks ok. mount.procfs doesn’t 
> exist in my environment. Is that command correct?
> 
> $ which mount.procfs; echo $?
> 1

Nevermind — thinking Linux for a sec. I think I see the bug now. etc/rc.d/jail 
before this change used to pass ${rootdir} if it was defined, and the new code 
wasn’t passing that… Might want to doublecheck it, but adding the rootdir 
afterwards might fix that.


signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: svn commit: r278323 - in head: etc/rc.d usr.sbin/jail

2015-02-06 Thread James Gritton



On 2015-02-06 22:23, Garrett Cooper wrote:

On Feb 6, 2015, at 18:38, James Gritton  wrote:


On 2015-02-06 19:23, Garrett Cooper wrote:

On Feb 6, 2015, at 9:54, Jamie Gritton  wrote:

Author: jamie
Date: Fri Feb  6 17:54:53 2015
New Revision: 278323
URL: https://svnweb.freebsd.org/changeset/base/278323
Log:
Add mount.procfs jail parameter, so procfs can be mounted when a 
prison's

root is in its fstab.
Also fix a typo while I'm at it.
PR: 197237 197066
MFC after:  3 days
Modified:
head/etc/rc.d/jail
head/usr.sbin/jail/command.c
head/usr.sbin/jail/config.c
head/usr.sbin/jail/jail.8
head/usr.sbin/jail/jail.c
head/usr.sbin/jail/jailp.h
I think you broke the Jenkins tests runs, and potentially jail 
support

in some edgecases:
https://jenkins.freebsd.org/job/FreeBSD_HEAD-tests2/651/


Where do I go from here?  There error you refer to certainly seems 
jail-related, which leads me to guess at something disconnected 
between the matching rc.d/jail and jail(8) change (i.e. using the new 
rc file with the old jail program).  But that's really just a wild 
guess.  Is there somewhere I look for more information?  For example, 
where does Jenkins actually do its thing?


Sorry for being so stupid in this - Jenkins has only been on the very 
edge of my awareness until now.


I honestly don’t think it’s Jenkins because Jenkins runs in bhyve. I
think you accidentally broke option handling in the jail configuration
(please see my other reply about added “break;” statements).


Oh I figure I broke something, not blaming Jenkins or anything like that 
- merely voicing my ignorance of how it's all put together :-).  My 
guess is that the jail(8) changes were good (I tested those), and for 
now I've reverted the rc script change so the old (untouched) jail 
options will be used.



pgrep uses /proc to determine whether or not a process is running. If
it’s not properly mounted or the jail isn’t started properly, that
could cause the issues seen here. I know because I’ve tried running
these tests before in an attempt to fix them, and this was one of the
things I ran into.

You can verify your changes by doing:

% (cd /usr/tests/bin/pkill; sudo kyua test)

Cheers!


OK, now I'm getting somewhere!  I'll try that test.

- Jamie
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r278323 - in head: etc/rc.d usr.sbin/jail

2015-02-06 Thread Garrett Cooper
On Feb 6, 2015, at 21:27, James Gritton  wrote:

> On 2015-02-06 22:18, Garrett Cooper wrote:
>> On Feb 6, 2015, at 9:54, Jamie Gritton  wrote:
>>> Modified: head/usr.sbin/jail/command.c
>>> ==
>>> --- head/usr.sbin/jail/command.cFri Feb  6 17:43:13 2015
>>> (r278322)
>>> +++ head/usr.sbin/jail/command.cFri Feb  6 17:54:53 2015
>>> (r278323)
>>> @@ -112,6 +112,12 @@ next_command(struct cfjail *j)
>>> if (!bool_param(j->intparams[IP_MOUNT_FDESCFS]))
>>> continue;
>>> j->comstring = &dummystring;
>>> +   break;
>>> +   case IP_MOUNT_PROCFS:
>>> +   if (!bool_param(j->intparams[IP_MOUNT_PROCFS]))
>>> +   continue;
>>> +   j->comstring = &dummystring;
>>> +   break;
>> Did you intend on adding another break? The code would previously fall
>> through to the next case statement...
>>> case IP__OP:
>>> case IP_STOP_TIMEOUT:
>>> j->comstring = &dummystring;
> 
> Yes.  The code did indeed previously fall to the next case, but it was a 
> no-op: the next case only had the same exact assignment that had just taken 
> place (j->comstring = &dummystring).  The lack of a break that had existed 
> before was just some sloppy coding that I didn't notice at the time because 
> it didn't actually change any behavior.  Nonetheless it seemed worth 
> correcting when I noticed it.

True. I looked at the code afterwards and it looks ok. mount.procfs doesn’t 
exist in my environment. Is that command correct?

$ which mount.procfs; echo $?
1


signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: svn commit: r278323 - in head: etc/rc.d usr.sbin/jail

2015-02-06 Thread James Gritton

On 2015-02-06 22:18, Garrett Cooper wrote:

On Feb 6, 2015, at 9:54, Jamie Gritton  wrote:


Modified: head/usr.sbin/jail/command.c
==
--- head/usr.sbin/jail/command.cFri Feb  6 17:43:13 2015
(r278322)
+++ head/usr.sbin/jail/command.cFri Feb  6 17:54:53 2015
(r278323)
@@ -112,6 +112,12 @@ next_command(struct cfjail *j)
if (!bool_param(j->intparams[IP_MOUNT_FDESCFS]))
continue;
j->comstring = &dummystring;
+   break;
+   case IP_MOUNT_PROCFS:
+   if (!bool_param(j->intparams[IP_MOUNT_PROCFS]))
+   continue;
+   j->comstring = &dummystring;
+   break;


Did you intend on adding another break? The code would previously fall
through to the next case statement...


case IP__OP:
case IP_STOP_TIMEOUT:
j->comstring = &dummystring;


Yes.  The code did indeed previously fall to the next case, but it was a 
no-op: the next case only had the same exact assignment that had just 
taken place (j->comstring = &dummystring).  The lack of a break that had 
existed before was just some sloppy coding that I didn't notice at the 
time because it didn't actually change any behavior.  Nonetheless it 
seemed worth correcting when I noticed it.


- Jamie
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r278323 - in head: etc/rc.d usr.sbin/jail

2015-02-06 Thread Garrett Cooper
On Feb 6, 2015, at 18:38, James Gritton  wrote:

> On 2015-02-06 19:23, Garrett Cooper wrote:
>> On Feb 6, 2015, at 9:54, Jamie Gritton  wrote:
>>> Author: jamie
>>> Date: Fri Feb  6 17:54:53 2015
>>> New Revision: 278323
>>> URL: https://svnweb.freebsd.org/changeset/base/278323
>>> Log:
>>> Add mount.procfs jail parameter, so procfs can be mounted when a prison's
>>> root is in its fstab.
>>> Also fix a typo while I'm at it.
>>> PR: 197237 197066
>>> MFC after:  3 days
>>> Modified:
>>> head/etc/rc.d/jail
>>> head/usr.sbin/jail/command.c
>>> head/usr.sbin/jail/config.c
>>> head/usr.sbin/jail/jail.8
>>> head/usr.sbin/jail/jail.c
>>> head/usr.sbin/jail/jailp.h
>> I think you broke the Jenkins tests runs, and potentially jail support
>> in some edgecases:
>> https://jenkins.freebsd.org/job/FreeBSD_HEAD-tests2/651/
> 
> Where do I go from here?  There error you refer to certainly seems 
> jail-related, which leads me to guess at something disconnected between the 
> matching rc.d/jail and jail(8) change (i.e. using the new rc file with the 
> old jail program).  But that's really just a wild guess.  Is there somewhere 
> I look for more information?  For example, where does Jenkins actually do its 
> thing?
> 
> Sorry for being so stupid in this - Jenkins has only been on the very edge of 
> my awareness until now.

I honestly don’t think it’s Jenkins because Jenkins runs in bhyve. I think you 
accidentally broke option handling in the jail configuration (please see my 
other reply about added “break;” statements).

pgrep uses /proc to determine whether or not a process is running. If it’s not 
properly mounted or the jail isn’t started properly, that could cause the 
issues seen here. I know because I’ve tried running these tests before in an 
attempt to fix them, and this was one of the things I ran into.

You can verify your changes by doing:

% (cd /usr/tests/bin/pkill; sudo kyua test)

Cheers!


signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: svn commit: r278323 - in head: etc/rc.d usr.sbin/jail

2015-02-06 Thread Garrett Cooper
On Feb 6, 2015, at 21:23, Garrett Cooper  wrote:

> On Feb 6, 2015, at 18:38, James Gritton  wrote:
> 
>> On 2015-02-06 19:23, Garrett Cooper wrote:
>>> On Feb 6, 2015, at 9:54, Jamie Gritton  wrote:
 Author: jamie
 Date: Fri Feb  6 17:54:53 2015
 New Revision: 278323
 URL: https://svnweb.freebsd.org/changeset/base/278323
 Log:
 Add mount.procfs jail parameter, so procfs can be mounted when a prison's
 root is in its fstab.
 Also fix a typo while I'm at it.
 PR:197237 197066
 MFC after: 3 days
 Modified:
 head/etc/rc.d/jail
 head/usr.sbin/jail/command.c
 head/usr.sbin/jail/config.c
 head/usr.sbin/jail/jail.8
 head/usr.sbin/jail/jail.c
 head/usr.sbin/jail/jailp.h
>>> I think you broke the Jenkins tests runs, and potentially jail support
>>> in some edgecases:
>>> https://jenkins.freebsd.org/job/FreeBSD_HEAD-tests2/651/
>> 
>> Where do I go from here?  There error you refer to certainly seems 
>> jail-related, which leads me to guess at something disconnected between the 
>> matching rc.d/jail and jail(8) change (i.e. using the new rc file with the 
>> old jail program).  But that's really just a wild guess.  Is there somewhere 
>> I look for more information?  For example, where does Jenkins actually do 
>> its thing?
>> 
>> Sorry for being so stupid in this - Jenkins has only been on the very edge 
>> of my awareness until now.
> 
> I honestly don’t think it’s Jenkins because Jenkins runs in bhyve. I think 
> you accidentally broke option handling in the jail configuration (please see 
> my other reply about added “break;” statements).

because Jenkins -> because the Jenkins kyua run


signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: svn commit: r278323 - in head: etc/rc.d usr.sbin/jail

2015-02-06 Thread Garrett Cooper
On Feb 6, 2015, at 9:54, Jamie Gritton  wrote:

> Author: jamie
> Date: Fri Feb  6 17:54:53 2015
> New Revision: 278323
> URL: https://svnweb.freebsd.org/changeset/base/278323
> 
> Log:
>  Add mount.procfs jail parameter, so procfs can be mounted when a prison's
>  root is in its fstab.
> 
>  Also fix a typo while I'm at it.
> 
>  PR:  197237 197066
>  MFC after:   3 days
> 
> Modified:
>  head/etc/rc.d/jail
>  head/usr.sbin/jail/command.c
>  head/usr.sbin/jail/config.c
>  head/usr.sbin/jail/jail.8
>  head/usr.sbin/jail/jail.c
>  head/usr.sbin/jail/jailp.h
> 
> Modified: head/etc/rc.d/jail
> ==
> --- head/etc/rc.d/jailFri Feb  6 17:43:13 2015(r278322)
> +++ head/etc/rc.d/jailFri Feb  6 17:54:53 2015(r278323)
> @@ -28,7 +28,7 @@ extra_commands="config console status"
> 
> need_dad_wait=
> 
> -# extact_var jail name param num defval
> +# extract_var jail name param num defval
> # Extract value from ${jail_$jail_$name} or ${jail_$name} and
> # set it to $param.  If not defined, $defval is used.
> # When $num is [0-9]*, ${jail_$jail_$name$num} are looked up and
> @@ -233,8 +233,7 @@ parse_options()
>   fi
>   eval : \${jail_${_j}_procfs_enable:=${jail_procfs_enable:-NO}}
>   if checkyesno jail_${_j}_procfs_enable; then
> - echo "  mount += " \
> - "\"procfs ${_rootdir%/}/proc procfs rw 0 0\";"
> + echo "  mount.procfs;"
>   fi
> 
>   eval : \${jail_${_j}_mount_enable:=${jail_mount_enable:-NO}}
> 
> Modified: head/usr.sbin/jail/command.c
> ==
> --- head/usr.sbin/jail/command.c  Fri Feb  6 17:43:13 2015
> (r278322)
> +++ head/usr.sbin/jail/command.c  Fri Feb  6 17:54:53 2015
> (r278323)
> @@ -112,6 +112,12 @@ next_command(struct cfjail *j)
>   if (!bool_param(j->intparams[IP_MOUNT_FDESCFS]))
>   continue;
>   j->comstring = &dummystring;
> + break;
> + case IP_MOUNT_PROCFS:
> + if (!bool_param(j->intparams[IP_MOUNT_PROCFS]))
> + continue;
> + j->comstring = &dummystring;
> + break;

Did you intend on adding another break? The code would previously fall through 
to the next case statement...

>   case IP__OP:
>   case IP_STOP_TIMEOUT:
>   j->comstring = &dummystring;
> @@ -528,6 +534,32 @@ run_command(struct cfjail *j)
>   }
>   break;
> 
> + case IP_MOUNT_PROCFS:
> + argv = alloca(7 * sizeof(char *));
> + path = string_param(j->intparams[KP_PATH]);
> + if (path == NULL) {
> + jail_warnx(j, "mount.procfs: no path");
> + return -1;
> + }
> + devpath = alloca(strlen(path) + 6);
> + sprintf(devpath, "%s/proc", path);
> + if (check_path(j, "mount.procfs", devpath, 0,
> + down ? "procfs" : NULL) < 0)
> + return -1;
> + if (down) {
> + argv[0] = "/sbin/umount";
> + argv[1] = devpath;
> + argv[2] = NULL;
> + } else {
> + argv[0] = _PATH_MOUNT;
> + argv[1] = "-t";
> + argv[2] = "procfs";
> + argv[3] = ".";
> + argv[4] = devpath;
> + argv[5] = NULL;
> + }
> + break;
> +
>   case IP_COMMAND:
>   if (j->name != NULL)
>   goto default_command;
> 
> Modified: head/usr.sbin/jail/config.c
> ==
> --- head/usr.sbin/jail/config.c   Fri Feb  6 17:43:13 2015
> (r278322)
> +++ head/usr.sbin/jail/config.c   Fri Feb  6 17:54:53 2015
> (r278323)
> @@ -84,6 +84,7 @@ static const struct ipspec intparams[] =
> [IP_MOUNT] =  {"mount",   PF_INTERNAL | PF_REV},
> [IP_MOUNT_DEVFS] ={"mount.devfs", PF_INTERNAL | 
> PF_BOOL},
> [IP_MOUNT_FDESCFS] =  {"mount.fdescfs",   PF_INTERNAL | PF_BOOL},
> +[IP_MOUNT_PROCFS] =  {"mount.procfs",PF_INTERNAL | 
> PF_BOOL},
> [IP_MOUNT_FSTAB] ={"mount.fstab", PF_INTERNAL},
> [IP_STOP_TIMEOUT] =   {"stop.timeout",PF_INTERNAL | 
> PF_INT},
> [IP_VNET_INTERFACE] = {"vnet.interface",  PF_INTERNAL},
> 
> Modified: head/usr.sbin/jail/jail.8
> =

Re: svn commit: r278323 - in head: etc/rc.d usr.sbin/jail

2015-02-06 Thread James Gritton

On 2015-02-06 19:23, Garrett Cooper wrote:

On Feb 6, 2015, at 9:54, Jamie Gritton  wrote:


Author: jamie
Date: Fri Feb  6 17:54:53 2015
New Revision: 278323
URL: https://svnweb.freebsd.org/changeset/base/278323

Log:
 Add mount.procfs jail parameter, so procfs can be mounted when a 
prison's

 root is in its fstab.

 Also fix a typo while I'm at it.

 PR:197237 197066
 MFC after: 3 days

Modified:
 head/etc/rc.d/jail
 head/usr.sbin/jail/command.c
 head/usr.sbin/jail/config.c
 head/usr.sbin/jail/jail.8
 head/usr.sbin/jail/jail.c
 head/usr.sbin/jail/jailp.h


I think you broke the Jenkins tests runs, and potentially jail support
in some edgecases:
https://jenkins.freebsd.org/job/FreeBSD_HEAD-tests2/651/


Where do I go from here?  There error you refer to certainly seems 
jail-related, which leads me to guess at something disconnected between 
the matching rc.d/jail and jail(8) change (i.e. using the new rc file 
with the old jail program).  But that's really just a wild guess.  Is 
there somewhere I look for more information?  For example, where does 
Jenkins actually do its thing?


Sorry for being so stupid in this - Jenkins has only been on the very 
edge of my awareness until now.


- Jamie
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r278323 - in head: etc/rc.d usr.sbin/jail

2015-02-06 Thread Garrett Cooper
On Feb 6, 2015, at 9:54, Jamie Gritton  wrote:

> Author: jamie
> Date: Fri Feb  6 17:54:53 2015
> New Revision: 278323
> URL: https://svnweb.freebsd.org/changeset/base/278323
> 
> Log:
>  Add mount.procfs jail parameter, so procfs can be mounted when a prison's
>  root is in its fstab.
> 
>  Also fix a typo while I'm at it.
> 
>  PR:  197237 197066
>  MFC after:   3 days
> 
> Modified:
>  head/etc/rc.d/jail
>  head/usr.sbin/jail/command.c
>  head/usr.sbin/jail/config.c
>  head/usr.sbin/jail/jail.8
>  head/usr.sbin/jail/jail.c
>  head/usr.sbin/jail/jailp.h

I think you broke the Jenkins tests runs, and potentially jail support in some 
edgecases: https://jenkins.freebsd.org/job/FreeBSD_HEAD-tests2/651/


signature.asc
Description: Message signed with OpenPGP using GPGMail