Ethan Quach wrote:
> Tim Knitter wrote:
>>
>> Ethan,
>>
>> Nit:
>> 586: Can we change newmp to be consistent with the other variables 
>> that handle mountpoints, e.g new_mntpt or something similar.
>>
>> 882: Can you explain in the comment why we try to re-create a BE when 
>> it is requested to be an auto-named BE?
>>
>> 988: successfuly... spelling
>>
>> 998: mounted -> mount
>>
>> 1120: Line not needed since free() handles NULL pointers.
> 
> We do this in a lot of places and the reason is that checking the
> handle against NULL and not calling free() is cheaper then actually
> calling free() with a NULL handle.
> 

Ah, OK. Good point.
 
>> Nits:
>> 1886: More descriptive variable name please.
>>
>> 2142,2286: * Error occurred while processing a dataset subordinate to 
>> this one.
>> -> Error occurred while processing a subordinate dataset.
>>
>> "to this one" seems redundant.
> 
> I'll use "child dataset" instead.

ok.

Tim

> 
> 
> thanks,
> -ethan
> 
> 
>> Thanks
>> Tim
>>
>>> This fix address both 3844 and 3901.  Upon creation of a boot
>>> environment, we now make sure that its mountable before deeming
>>> it a success.  We also now catch zone creation errors instead of
>>> silently ignoring them.  Upon creation failure, we now cleanup
>>> the incomplete boot environment instead of leaving its bits and
>>> pieces around.
>>>
>>> I've tested this with cases where the source BE has faulty
>>> subordinate datasets, and also where just some random error
>>> happened while processing a subordinate dataset.  I've also tested
>>> these above scenarios with and without zones, and where copying
>>> a zone caused an error.
>>>
>>>
>>> Webrev:
>>> ----------
>>> http://cr.opensolaris.org/~equach/webrev.3844.3901
>>>
>>> Defects:
>>> ----------
>>> 3844 be_copy_zones() silently ignores 
>>> zoneshttp://defect.opensolaris.org/bz/show_bug.cgi?id=3844
>>>
>>> 3901 libbe: a failed beadm create can fail to clean up a partially 
>>> created BE and leaves it mounted
>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=3901
>>>
>>>
>>> thanks,
>>> -ethan
>>>
>>> _______________________________________________
>>> caiman-discuss mailing list
>>> caiman-discuss at opensolaris.org
>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

Reply via email to