Below are my comments on the incremental: General: In be_list.c, be_snapshot, and be_rename, returns that used to say "be_errno_t", now say "non-zero". But in be_mount.c, they still say be_errno_t. Should be consistent.
be_list 182 I think it's easier for translation if it says "BE does not exist %s\n" be_mount 243 it's->its 296 Could altroot potentially be NULL here? 488 function in error message is wrong 546,762 filesystems's->filesystem's be_snapshot 69-74 indentation 274 I am not sure that it works for gettext if you put a constant in the string. Please double check. If it does work, then there a several "legacy"s to change in be_mount. 621-622 Since you are checking to see if a snapshot name exists, it be clearer to say something like "0 - success, if snapshot name exists", "1 - Failure, if snapshot name doesn't exist" 629 zhp_snap is defined, but not used Sue >> Hello again caimaniacs, >> >> Thanks for those that provided comments for the early code review. >> Posted here is the incremental webrev of the changes for code review >> comments plus other fixes that we've pushed since. >> >> http://cr.opensolaris.org/~equach/webrev.SnapUpgrade.2 >> >> >> Additionally as a reference, below is the webrev of the overall snap >> upgrade gate synced up against the current slim_source base. >> >> http://cr.opensolaris.org/~equach/webrev.SnapUpgrade >> >> >> Again comments welcomed, particularly from those who provided the >> initial comments on the early review. >> >> >> Thanks, >> -Ethan >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
