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.


- isn't it kinda weird to have the brand specific cli options come
  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

- 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
  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

- 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
zones-discuss mailing list

Reply via email to