Edward Pilatowicz wrote:
> On Tue, May 27, 2008 at 08:48:17AM -0600, Jerry Jelinek wrote:
>> I have updated the webrev at:
>> 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.)
> - 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.)
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
> - in main(), an invalid cmd value will result in returning an
> uninitialzed err value. makes me wonder, is this code linting
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.
zones-discuss mailing list