[caiman-discuss] Code Review Request for bugs 9594, 9051 and 8638

[Evan Layton]

Ethan Quach wrote:
> Evan,
> 
> be_mount.c
> ------------------
> 965 - I don't think you need to call to zfs_prop_set() here.
> We haven't changed the mountpoint yet.

@$%$# left over cruft from other changes. Nice catch!

> 
> 1005 - In the error message, you want to use *orig_mntpnt
> here instead of *tmp_mnpnt because this is where we're trying
> to reset it back to the original mountpoint.

Right it should have been *orig_mntpnt, changed.

I also noticed while giving these error conditions another
look that I had missed setting *tmp_mntpnt and *orig_mntpnt
to NULL after freeing them so I fixed this as well.

> 
> 1029-1030 - this comment seems wrong.  In be_mount_pool(),
> we always populate orig_mntpnt, so this wouldn't ever be
> NULL.  For this to be true, you need to move line 970 up
> to be right after line 954, which I think would work.

Yes it now reads:

  *              orig_mntpnt - The original mountpoint for the pool this is
  *                            set the dataset mountpoint property back to
  *                            it's original value in the case where a
  *                            temporary mountpoint was used.


changes made, webrev updated.

-evan

> 
> 
> thanks,
> -ethan
> 
> 
> 
> Evan Layton wrote:
>> As per our conversations I've made all the requested changes and 
>> updated the webrev at http://cr.opensolaris.org/~evanl/9594v2 let me 
>> know if you're OK with pushing the current set of changes.
>>
>> Thanks!
>> -evan
>>
>> Evan Layton wrote:
>>> Ethan Quach wrote:
>>>>
>>>>
>>>> libbe.h
>>>> -----------
>>>> 180 - I think you want this to be 0x00000004 not 3 :-)
>>>>
>>>>
>>>> be_mount.c
>>>> ------------------
>>>> 330 - So that the NO_ZONES flag doesn't stomp on the
>>>> SHARED_FS flag, I think we can just move this check
>>>> into line 361which becomes:
>>>>
>>>>   if (getzoneid() == GLOBAL_ZONEID &&
>>>>       be_get_uuid(bt.obe_root_ds, &uu) == 0 &&
>>>>       !(flags & BE_MOUNT_FLAG_NO_ZONES)) {
>>>>
>>>>
>>>> Line 372 can then be removed as well.
>>>>
>>>>
>>>
>>> fixed...
>>> _______________________________________________
>>> caiman-discuss mailing list
>>> caiman-discuss at opensolaris.org
>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>