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 webrev with two bug fixes.  One of these was for
>> a bug I was hitting during testing of these changes
>> and there is a second bug in zlogin that came in
>> which I also fixed.  So, at a minimum, it would be
>> good to take a look at that additional file.
> here's my last batch of comments.
> (for sw_support.c i only looked at the new functions you introduced.)
> ed
> ----------
> general
> - iirc, there is still tons of zulu code in zoneadmd.  this code creates
>   the special live upgrade environment, and LU is tied to svr4
>   packaging, right?  so it seems like the zulu code is brand specific
>   and should also be factored out into "mount" callbacks.  (and iirc,
>   the "mount" operations isn't currenly supported by any branded zones.)
>   thoughts?

I don't think there is really any zulu code in zoneadmd but I'll
take a pass through to double check.  If I see anything like
that, I'll fix it up and update the webrev.

The mount code is not really svr4 pkg specific either, but is
specific to the native brand.  I have extensions to mount which
would allow us to mount any type of brand with very little change.
I added those with the update on attach work.  Being able to
mount any zone is something I've thought would be handy for a long
time now, so I'll take a look at that as I am making the pass through

> ----------
> usr/src/lib/brand/native/zone/sw_support.c
> - in main(),  an invalid cmd value will result in returning an
>   uninitialzed err value.  makes me wonder, is this code linting
>   properly?

I am not getting any lint warning from that, which is surprising.
I fixed this.

> - nit: in install_func(), status should be initialized to Z_ERR not 1.


> - in install_func(), if the snprintf() overflows, you'll pass a
>   a constant error value (Z_ERR) to subproc_status() (which is
>   expecting a waitpid() encoded status value).

That was old code that just got moved, but it is broken and I fixed

> - hm.  attach_func() looks for the -F force flag, but as discussed
>   previously, in the case of forced attach, the attach brand callback
>   is currently not executed.

I duplicated that when I split things up.  I have removed it.

Thanks again,

zones-discuss mailing list

Reply via email to