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
>