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
> 


Reply via email to