On Tue, May 13, 2008 at 09:59:22AM -0600, Jerry Jelinek wrote:
> I have posted a webrev at:
>
> http://cr.opensolaris.org/~gjelinek/webrev/
>
> for bug:
>
> 6553514 native zone svr4 pkg code should be moved into zone callbacks
>
> This is a refactoring of the code so that it is
> easier to support different brands going forward.
> In particular this will help with the IPS support
> for the Indiana release, although it will also
> benefit any distro that wants to use brands to provide
> zone support.
>

some initial comments starting with all the easy stuff.
ed

----------
usr/src/Makefile.lint
usr/src/cmd/zoneadm/Makefile
usr/src/cmd/zoneadm/zoneadm.h
usr/src/lib/libbrand/common/libbrand.h
usr/src/lib/libbrand/common/mapfile-vers
usr/src/lib/libzonecfg/Makefile.com
usr/src/lib/libzonecfg/common/mapfile-vers

- ok

----------
usr/src/head/libzonecfg.h

- nit: for consistency please remove "handle" from
  zonecfg_dev_manifest(...)

----------
usr/src/lib/brand/native/zone/Makefile

- nit: instead of:
        include ../../../../cmd/Makefile.targ
  how about
        include $(SRC)/cmd/Makefile.targ

- why do you need -I$(SRCDIR)?
  (there are no .h files in usr/src/lib/brand/native/zone/.)

- isn't _REENTRANT defined by higher level makefiles?

----------
usr/src/lib/brand/native/zone/config.xml

- should /usr/lib/brand/native/postclone be folded into sw_support
  for consistency?

- nit: perhaps you could delete all the empty tags?

----------
usr/src/lib/libbrand/dtd/brand.dtd.1

- given that we have postattach and predetach, perhaps we should have
  preattach instead of attach and postdetach instead of detach?

- could the comments be explain some of the differences in system and
  zone state between when
        - the attach and postattach callbacks are invoked?
        - the predetach and detach callbacks are invoked?

- the attach comment only talks about the -F argument, but the man page
  also documents -u and -n.  are those arguments ever passed to the
  callback?  if so, instead of documenting individual arguments in
  the comments perhaps they comment should just reference the
  zoneadm man page?

- given that native is not becomming a brand, does this clone comment
  make sense anymore?
   If no hook is provided, the internal zoneadm cloning code will be used.

- i'm confused.  you seem to be introducing hooks that no brands is
  using:
        clone
        uninstall
  are theses going to be needed for IPS?

- please update the comments to describe how the return codes are
  handled (or ignored) for all the new callbacks.
_______________________________________________
zones-discuss mailing list
zones-discuss@opensolaris.org

Reply via email to