Re: [zones-discuss] code review: native brand refactoring

2008-05-28 Thread Jerry Jelinek
Ed, My responses are in-line. Edward Pilatowicz wrote: On Tue, May 27, 2008 at 08:48:17AM -0600, Jerry Jelinek wrote: I have updated the webrev at: http://cr.opensolaris.org/~gjelinek/webrev/ This includes the changes for the feedback I have received so far. I also added the zlogin.c

Re: [zones-discuss] code review: native brand refactoring

2008-05-28 Thread Jerry Jelinek
Steve Lawrence wrote: Hey Jerry, Does this address this comment in 6621020: This appears to point out at least one bug in zlogin, namely that it keeps stdout_pipe[1] and stderr_pipe[1] from noninteractive_login() open when returning to the parent. Basically, I think the filer

Re: [zones-discuss] code review: native brand refactoring

2008-05-28 Thread Jerry Jelinek
Edward Pilatowicz wrote: - why are you parsing arguments by hand in stead of using getopt()? this will result in non-standard argument parsing and probably break things that used to work like: zoneadm -z foo clone -mcopy -sexport/zones/[EMAIL PROTECTED] ... (note the lack of

Re: [zones-discuss] code review: native brand refactoring

2008-05-28 Thread Nicolas Williams
See also: 6475075 zlogin i/o loop needs work 6263984 zlogin's i/o loop can sometimes drop data on child death On Wed, May 28, 2008 at 11:53:18AM -0500, Nicolas Williams wrote: On Tue, May 27, 2008 at 08:48:17AM -0600, Jerry Jelinek wrote: This includes the changes for the feedback I have

Re: [zones-discuss] code review: native brand refactoring

2008-05-28 Thread Edward Pilatowicz
On Wed, May 28, 2008 at 07:02:32AM -0600, Jerry Jelinek wrote: - why is there no locking around the pre-detach hook? That hasn't changed with this code but if I recall correctly, the idea was that predetach is not making any changes to the zone so we don't lock until actual changes to the

Re: [zones-discuss] code review: native brand refactoring

2008-05-28 Thread Jerry Jelinek
Nicolas Williams wrote: On Tue, May 27, 2008 at 08:48:17AM -0600, Jerry Jelinek wrote: This includes the changes for the feedback I have received so far. I also added the zlogin.c file to the webrev with two bug fixes. One of these was for a bug I was hitting during testing of these changes

Re: [zones-discuss] code review: native brand refactoring

2008-05-28 Thread Jerry Jelinek
Edward Pilatowicz wrote: On Wed, May 28, 2008 at 07:02:32AM -0600, Jerry Jelinek wrote: - why is there no locking around the pre-detach hook? That hasn't changed with this code but if I recall correctly, the idea was that predetach is not making any changes to the zone so we don't lock until

Re: [zones-discuss] code review: native brand refactoring

2008-05-28 Thread Edward Pilatowicz
On Wed, May 28, 2008 at 07:02:32AM -0600, Jerry Jelinek wrote: - in the case of a forced attach why don't we run the brand callback? That is really the definition of forced. We don't do anything when forced except change the state of the zone to installed. hm. that differs from my concept

Re: [zones-discuss] code review: native brand refactoring

2008-05-28 Thread Jerry Jelinek
Edward Pilatowicz wrote: On Wed, May 28, 2008 at 07:02:32AM -0600, Jerry Jelinek wrote: - in the case of a forced attach why don't we run the brand callback? That is really the definition of forced. We don't do anything when forced except change the state of the zone to installed. hm.

Re: [zones-discuss] code review: native brand refactoring

2008-05-28 Thread Edward Pilatowicz
On Tue, May 27, 2008 at 08:48:17AM -0600, Jerry Jelinek wrote: I have updated the webrev at: http://cr.opensolaris.org/~gjelinek/webrev/ This includes the changes for the feedback I have received so far. I also added the zlogin.c file to the webrev with two bug fixes. One of these was for

Re: [zones-discuss] code review: native brand refactoring

2008-05-28 Thread Jerry Jelinek
Ed, Responses below. Edward Pilatowicz wrote: On Tue, May 27, 2008 at 08:48:17AM -0600, Jerry Jelinek wrote: I have updated the webrev at: http://cr.opensolaris.org/~gjelinek/webrev/ This includes the changes for the feedback I have received so far. I also added the zlogin.c file to the

Re: [zones-discuss] code review: native brand refactoring

2008-05-28 Thread Edward Pilatowicz
On Wed, May 28, 2008 at 12:24:07PM -0600, Jerry Jelinek wrote: Ed, Responses below. Edward Pilatowicz wrote: On Tue, May 27, 2008 at 08:48:17AM -0600, Jerry Jelinek wrote: I have updated the webrev at: http://cr.opensolaris.org/~gjelinek/webrev/ This includes the changes for the

Re: [zones-discuss] code review: native brand refactoring

2008-05-28 Thread Edward Pilatowicz
On Wed, May 28, 2008 at 01:00:08PM -0600, Jerry Jelinek wrote: Edward Pilatowicz wrote: hm. from what i remember mount mode actually puts the zone root at zoneroot/a and lofs mounts stuff from the global zone at zoneroot, all so that the svr4 packaging code can enter the zone to do packaging