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.

>
> 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.


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

Reply via email to