Tim,

The changes look good.  Thanks for working out all the kinks with
this bug.


-ethan


Tim Knitter wrote:
> Ethan Quach wrote:
>   
>> Tim Knitter wrote:
>>     
>>> Ethan Quach wrote:
>>>       
>>>> Tim Knitter wrote:
>>>>         
>>>>> Ethan Quach wrote:
>>>>>           
>>>>>> Tim,
>>>>>>
>>>>>> be_mount.c:
>>>>>> ---------------
>>>>>> 1556-1563 - is this chunk really necessary?  The printed debug 
>>>>>> message at 1564
>>>>>>             
>>>>> I wouldn't say necessary but I would say better since the whole 
>>>>> reason for doing this is to give the user a nice message.
>>>>>           
>>>> I don't see how its better, its just redundant.  So in debug mode, 
>>>> when we hit
>>>> this error, with your changes we get:
>>>>
>>>> umount_shared_fs: insufficient permissions <mountpoint>: permission 
>>>> denied
>>>> umount_shared_fs: failed to unmount shared file system <mountpoint>: 
>>>> permission denied
>>>>
>>>>
>>>> How is that better?  If you're worried about what gets returned to 
>>>> the caller (and
>>>> ultimately beadm), then the errno_to_be_err() call at 1569 sets ret to
>>>> BE_ERR_PERM or some such anyway.
>>>>
>>>>         
>>> Your right. Once I made the following change suggested by Evan, that 
>>> chunk is no longer needed:
>>>
>>> 1569                                 ret = errno_to_be_err(err);
>>>
>>> I removed the chunk, tested and updated the webrev.
>>>       
>> The only thing left that I can see that might make a difference is at
>> 1569, instead of setting ret, just return ret there.  That way when we
>> loop around and try to umount some other shared file system, ret won't
>> be masked with an EBUSY or something like that.
>>
>>     
>
> Oh right. I initially had it returning on the first occurrence. 
>
> I also added a catch for BE_ERR_ACCESS in beadm.py for all the subcommands.
>
> webrev updated and code tested.
>
> Thanks
> Tim
>
>   
>> thanks,
>> -ethan
>>
>>
>>     
>>> Thanks
>>> Tim
>>>
>>>       
>>>> -ethan
>>>>
>>>>
>>>>         
>> _______________________________________________
>> caiman-discuss mailing list
>> caiman-discuss at opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>     
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>   

Reply via email to