Tim, Thanks for the review. All comments taken as is unless noted below...
thanks, -ethan Tim Knitter wrote: > be_mount.c: > > 53: Why not add this struct to libbe_priv.h where the other structs live? > Because its only used privately in be_mount.c, not anywhere else. > Nit: > 294, 2208: The comment should spell out variable names (altroot) for clarity > sake. > I think altroot is pretty well understood. Its not even used as variable name at 2208, but I could change altroot -> alternate root, if that's what you were intending. > 546: failed to -> failed to mount a > actually, the "failed to" just needs to be nuked. > 547: dataset mountpoint -> dataset. Mountpoint > given the removal of the "failed to" portion, the sentence makes sense as is. > 575: boolean_t seems a better choice since this function only has 2 states to > return. Also calls to this function then don't have to compare against an > int. > Actually, I think this method (along with 525), need to be changed to return be_errno_t so that a more accurate message can ultimately be displayed. I'll make this adjustment. > 702: Spell out altroot in comment; alternate root > Again, I think altroot is well understood, but sure, I can change this... > 644,715,750... "zone_mounted_here" the "here" is ambiguous, suggest just > zone_mounted > "here" means we mounted it here in this function, so we know we need to unmount it before leaving, as a cleanup task. I think its more ambiguous without the "here". IMO "zone_mounted" sounds more like a status that the zone is currently mounted. thanks, -ethan > Thanks > Tim > > >> Evan Layton wrote: >> >>> Hello All, >>> >>> We're down to the wire on the zone support changes to SNAP upgrade and are >>> looking for code review comments. We'll be taking comments up until COB >>> Tuesday >>> October 7th. Your comments are as always welcome and appreciated. >>> >>> Defect 3686 is the blocker bug that was submitted to cover this work and >>> the >>> webrev is available at: >>> >>> http://cr.opensolaris.org/~equach/webrev.snap_zones/ >>> >>> Thank you in advance for your comments and help! >>> -evan >>> _______________________________________________ >>> caiman-discuss mailing list >>> [EMAIL PROTECTED] >>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>> >> Evan, >> >> >> I will review this today, Monday October 6. >> >> >> Joe >> _______________________________________________ >> caiman-discuss mailing list >> [EMAIL PROTECTED] >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >> > _______________________________________________ > caiman-discuss mailing list > [EMAIL PROTECTED] > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > _______________________________________________ zones-discuss mailing list zones-discuss@opensolaris.org
