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. thanks, -ethan > > Thanks > Tim > >> >> -ethan >> >>
