On Mon, Jun 19, 2023 at 01:59:57PM +0530, Ritesh Harjani wrote:
> Dave Chinner <david@xxxxxxxxxxxxx> writes:
> > + if (sbp->sb_logsunit > 1) {
> > + if (sbp->sb_logsunit % sbp->sb_blocksize) {
> > + xfs_notice(mp,
> > + "log stripe unit %u bytes must be a multiple of block size",
> > + sbp->sb_logsunit);
> > + return -EFSCORRUPTED;
> > + }
> > + if (sbp->sb_logsunit > XLOG_MAX_RECORD_BSIZE) {
> > + xfs_notice(mp,
> > + "log stripe unit %u bytes must be a multiple of block size",
> > + sbp->sb_logsunit);
>
> I guess this xfs_notice message needs to be corrected.
>
> > + return -EFSCORRUPTED;
> > + }
> > + }
> > +
> > +
> > +
> > +
>
> too many new lines here ^^^
Fixed.
>
> > - * We can, however, reject mounts for CRC format filesystems, as the
> > + * We can, however, reject mounts for V5 format filesystems, as the
> > * mkfs binary being used to make the filesystem should never create a
> > * filesystem with a log that is too small.
> > */
> > min_logfsbs = xfs_log_calc_minimum_size(mp);
> > -
> > if (mp->m_sb.sb_logblocks < min_logfsbs) {
> > xfs_warn(mp,
> > "Log size %d blocks too small, minimum size is %d blocks",
> > mp->m_sb.sb_logblocks, min_logfsbs);
> > error = -EINVAL;
>
> Are we using this error now?
I'm not changing the existing error for this case.
>
> > - } else if (mp->m_sb.sb_logblocks > XFS_MAX_LOG_BLOCKS) {
> > - xfs_warn(mp,
> > - "Log size %d blocks too large, maximum size is %lld blocks",
> > - mp->m_sb.sb_logblocks, XFS_MAX_LOG_BLOCKS);
> > - error = -EINVAL;
> > - } else if (XFS_FSB_TO_B(mp, mp->m_sb.sb_logblocks) > XFS_MAX_LOG_BYTES) {
> > - xfs_warn(mp,
> > - "log size %lld bytes too large, maximum size is %lld bytes",
> > - XFS_FSB_TO_B(mp, mp->m_sb.sb_logblocks),
> > - XFS_MAX_LOG_BYTES);
> > - error = -EINVAL;
> > - } else if (mp->m_sb.sb_logsunit > 1 &&
> > - mp->m_sb.sb_logsunit % mp->m_sb.sb_blocksize) {
> > - xfs_warn(mp,
> > - "log stripe unit %u bytes must be a multiple of block size",
> > - mp->m_sb.sb_logsunit);
> > - error = -EINVAL;
> > - fatal = true;
> > - }
> > - if (error) {
> > +
> > /*
> > * Log check errors are always fatal on v5; or whenever bad
> > * metadata leads to a crash.
> > */
> > - if (fatal) {
> > + if (xfs_has_crc(mp)) {
> > xfs_crit(mp, "AAIEEE! Log failed size checks. Abort!");
> > ASSERT(0);
> > goto out_free_log;
>
> yes, only here in goto out_free_log we will return "error".
> Then why not shift error = -EINVAL in this if block?
Because I tried to change as little as needed to fix the issue. I
didn't think it was necessary to move it from where it was....
Fixed.
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
