Thanks for the review.  All comments taken as is unless noted below...


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

> 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
> 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
>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>> Evan,
>> I will review this today, Monday October 6.
>> Joe
>> _______________________________________________
>> caiman-discuss mailing list
>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
> _______________________________________________
> caiman-discuss mailing list
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
zones-discuss mailing list

Reply via email to