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.
>
more comments below.
all that's left is sw_support.c, i'll get to that tommorow.
ed
----------
usr/src/cmd/zoneadm/zoneadm.c
- isn't it kinda weird to have the brand specific cli options come
before
the brand name when cloning a zone?
- 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 spaces between the flag and the option) or:
zoneadm -z foo attach -Fu ...
- in clone_func(), "done:" always calls zonecfg_release_lock_file(), but
there are paths that can invoke "goto done;" without first calling
zonecfg_grab_lock_file().
- in clone_func(), it seems like the is_system_labeled() check should
be moved out of here and into the native brand postclone callback.
- why is there no locking around the pre-detach hook?
- in dryrun_get_brand(), use STDIN_FILENO instead of 0.
- in dryrun_get_brand(), you might want to always read the manifest
into a temporary file. with the current code specifying a manifest
path
of "/dev/fd/0" (which is a standard substitute for "-") won't work.
- in the case of a forced attach why don't we run the brand callback?
- might it simplify zoneadm to just include locking for help request
subcommands?
- in attach_func(), if the force flag is specified it seems like we'll
always call zonecfg_release_lock_file(). (Even if we didn't aquire
the lock beforehand.)
- uninstall_func() has lots of error returns that don't call
zonecfg_release_lock_file().
_______________________________________________
zones-discuss mailing list
zones-discuss@opensolaris.org
