May I have 2 code reviewers for the following minor changes for:

PSARC/2010/008 Remove zoneadm install sub-option "-x nodataset"
6880288 retire zoneadm install -x nodataset option
6890415 zoneadm install fails but returns 0

http://cr.opensolaris.org/~batschul/nodataset/

thanks!
frankB

Notes:

for 6880288 - those changes only affect the behavior of the native brand, 
however
if being used with the 'ipkg' brand it totally confused the brand 
install/uninstall code,
for an example, see comments in: 
http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6880288

create_zfs_zonepath() will be modified in a follow up work, not
yet. either via new bug or via the already existing one:

6726123 zoneadm install should create zfs filesystems whenever possible

to give up it's fault tollerance, that it is doing that right now is okay,
the brand specific install scripts (and for the native brand 
/usr/lib/lu/lucreatezone)
will always make sure to create the $ZONEROOT and all corresponding sub 
dirs/sub datasets.

I droped the idea to include such changes here, as when we're doing this we can
simplify code in the brand install scripts as well because zoneadm.c itself
will always either create $ZONEROOT or fail. I decided to not intermix such
changes with this change only releated to the native brand. Baiscally
that create_zfs_zonepath() and zoneadm.c:install_func() were fault tollerant
here was because we supported both UFS and ZFS with the native brand. Now
since the native brand is basically dead with build 131 we can adjust.

I'll udpate 6726123 wrt. to create_zfs_zonepath() and $ZONEROOT though.

------------------------------------------

for 6890415 - it also occured to me that we're not always setting 'errno'
appropriately in zoneadm.c:install_func() apart from the bugs inital complaint
about clobbering potential failures of the brand specific install func reported
in 'err' later on in the :done label. 

usr/src/cmd/zoneadm/zoneadm.c:install_func() has the following code at the end:

   2989 done:
[...]
   2996         if (subproc_err == ZONE_SUBPROC_NOTCOMPLETE) {
   2997                 if ((err = cleanup_zonepath(zonepath, B_FALSE)) != 
Z_OK) {
   2998                         errno = err;
   2999                         zperror2(target_zone,
   3000                             gettext("cleaning up zonepath failed"));
   3001                 } else if ((err = zone_set_state(target_zone,
   3002                     ZONE_STATE_CONFIGURED)) != Z_OK) {
   3003                         errno = err;
   3004                         zperror2(target_zone, gettext("could not set 
state"));
   3005                 }
   3006         }

in lines that call cleanup_zonepath() and zone_set_state() we can clobber
a potential error from 'status=do_subproc()' calling the brand specific
install hook if either of those calls succeed with Z_OK.

   2955         status = do_subproc(cmdbuf);
   2956         if ((subproc_err =
   2957             subproc_status(gettext("brand-specific installation"), 
status,
   2958             B_FALSE)) != ZONE_SUBPROC_OK) {
   2959                 if (subproc_err == ZONE_SUBPROC_USAGE && !brand_help) {
   2960                         sub_usage(SHELP_INSTALL, CMD_INSTALL);
   2961                         zonecfg_release_lock_file(target_zone, lockfd);
   2962                         return (Z_ERR);
   2963                 }
   2964                 err = Z_ERR;
   2965                 goto done;
   2966         }

in that case, we'd return Z_OK from install_func() even though we shall be
returning the potential error from do_subproc() !

   3010         return ((err == Z_OK) ? Z_OK : Z_ERR);
   3011 }

so I added the missing 'errno' settings as well:

1) after executing the brand instalation function:

   2955         status = do_subproc(cmdbuf);
   2956         if ((subproc_err =
   2957             subproc_status(gettext("brand-specific installation"), 
status,
   2958             B_FALSE)) != ZONE_SUBPROC_OK) {
   2959                 if (subproc_err == ZONE_SUBPROC_USAGE && !brand_help) {
   2960                         sub_usage(SHELP_INSTALL, CMD_INSTALL);
   2961                         zonecfg_release_lock_file(target_zone, lockfd);
   2962                         return (Z_ERR);
   2963                 }
   2964                 err = Z_ERR;
   2965                 goto done;
   2966         }

2) after executing the brand specific post install func:

   2977         if (do_postinstall) {
   2978                 status = do_subproc(postcmdbuf);
   2979 
   2980                 if ((subproc_err =
   2981                     subproc_status(gettext("brand-specific 
post-install"),
   2982                     status, B_FALSE)) != ZONE_SUBPROC_OK) {
   2983                         err = Z_ERR;
   2984                         (void) zone_set_state(target_zone,
   2985                             ZONE_STATE_INCOMPLETE);
   2986                 }
   2987         }

in both cases we ought to set 'errno = subproc_err;' along with 'err = Z_ERR'.
_______________________________________________
zones-discuss mailing list
zones-discuss@opensolaris.org

Reply via email to