On 3/24/16 6:15 AM, jtulak@xxxxxxxxxx wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> UPDATE
> o disable finobt when crc=0 no matter if user used -m finobt=X
> o split line > 80 chars
> o remove unused variable
> o add omitted finobtflag
> o change variables in spinodes case to look like surrounding code
> o add I_ALIGN reqval
Just FYI - generally, the patch changelog goes below the "---"
so it doesn't end up as part of the changelog in git.
...
(sorry for being late to the review party btw...)
> } else {
> /*
> * The kernel doesn't currently support crc=0,finobt=1
> - * filesystems. If crcs are not enabled and the user has
> - * explicitly turned them off then silently turn them off
> - * to avoid an unnecessary warning. If the user explicitly
> - * tried to use crc=0,finobt=1, then issue a warning before
> - * turning them off.
> + * filesystems. If crcs are not enabled and the user has not
> + * explicitly turned finobt on, then silently turn it off to
> + * avoid an unnecessary warning. If the user explicitly tried
> + * to use crc=0,finobt=1, then issue a warning before turning
> + * them off.
> */
> - if (finobt && finobtflag) {
> - fprintf(stderr,
> -_("warning: finobt not supported without CRC support, disabled.\n"));
> + if (sb_feat.finobt){
> + if (sb_feat.finobtflag) {
> + fprintf(stderr,
> + _("warning: finobt not supported without CRC support, disabled.\n"));
> + }
> + sb_feat.finobt = 0;
> }
> - finobt = 0;
> }
Is there any other case in mkfs where options are automatically disabled?
I don't think so .. I'd just prefer a failure here, not a fix-up, even
with the warning. But I guess that's how it was before, so probably
not something to change in this patch. So never mind. :)
But, do we need the extra indentation?
if (sb_feat.finobt && sb_feat.finobtflag) {
fprintf(stderr,
_("warning: finobt not supported without CRC support, disabled.\n"));
}
sb_feat.finobt = 0;
would suffice as before, no? Meh. Not a big deal I guess....
> @@ -2962,7 +3038,7 @@ _("size %s specified for log subvolume is too large,
> maximum is %lld blocks\n"),
> /*
> * Free INO btree root block
> */
> - if (!finobt) {
> + if (!sb_feat.finobt){
^ please fix whitespace :)
> xfs_perag_put(pag);
> continue;
> }
>
|