The webrev at http://cr.opensolaris.org/~evanl/9594 has been updated
with these changes.
-evan
Evan Layton wrote:
> sanjay nadkarni (Laptop) wrote:
>>
>> be_activate.c:
>> 601: it should be if (err == 0)
>> That said I don't quite follow the logic 599 - 603.
>> if err is zero then err is assigned to ret which has been set to zero
>> 2 lines above.
>
> This is to make sure we don't miss any errors from above. However it
> should have been:
>
> 596
> 597 cleanup:
> 598 if (pool_mounted) {
> 599 int ret = 0;
> 600 ret = be_unmount_pool(pool_zhp, orig_mntpt);
> 601 if (err == 0)
> 602 err = ret;
> 603 }
> 604 free(orig_mntpt);
> 605 ZFS_CLOSE(pool_zhp);
> 606
>
>
>>
>> be_mount.c
>> nit. for better readability you might consider defining null macros
>> for flags or priv_flags
>> such as:
>> #define NULL_FLAGS 0
>> #define NULL_PRIV_FLAGS 0
>> since _be_mount is called from a number of places.
>
> I agree that will definitely make things more readable. I'll change this
> throughout the code and I'll send out an updated webrev as soon as I
> have this changed.
>
>>
>> be_mount.c:
>> typos:
>> 925 - fucntion->function.
>> 933: I think you mean "if"
>> The original mountpoint for the pool if a temporary mount
>> point was needed.
>
> Fixed.
>
> Thanks for looking at this for me!
>
> -evan
>
>>
>>
>> -Sanjay
>>
>> Evan Layton wrote:
>>> I need to get a code review for the following fixes.
>>>
>>> webrev:
>>> ------------
>>> http://cr.opensolaris.org/~evanl/9594
>>>
>>>
>>> Bugids:
>>> -----------
>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=8638
>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=9051
>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=9594
>>>
>>> Some back ground:
>>> The main bug is 9594 which was an oversight in the original design
>>> where we did not take into account the possibility that the root
>>> pool dataset (/rpool) was not mounted. To fix this we attempt to
>>> mount it at it's normal mount point, if that mount fails we attempt
>>> to mount it on a temporary mount point.
>>>
>>> We also discovered we were erroneously checking for error conditions
>>> from libzfs property checking functions such as zfs_prop_get_int().
>>> These functions are expected to allows succeed and do not set the
>>> libzfs error codes. The incorrect error checking was also fixed as
>>> part of this fix.
>>>
>>>
>>> Thanks,
>>> -evan
>>> _______________________________________________
>>> caiman-discuss mailing list
>>> caiman-discuss at opensolaris.org
>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>
>
