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 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 zoneadmd. > > ---------- > 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. Fixed. > - 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 it. > - 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, Jerry _______________________________________________ zones-discuss mailing list [email protected]
