Re: [PATCH 1/1] xfs: fallback to readonly during recovery

[Aaron Sierra <asierra@xes-inc.com>]

> From: "Eric Sandeen" <sandeen@sandeen.net>
> Sent: Monday, February 10, 2020 3:43:50 PM

> On 2/10/20 3:10 PM, Vincent Fazio wrote:
>> Previously, XFS would fail to mount if there was an error during log
>> recovery. This can occur as a result of inevitable I/O errors when
>> trying to apply the log on read-only ATA devices since the ATA layer
>> does not support reporting a device as read-only.
>> 
>> Now, if there's an error during log recovery, fall back to norecovery
>> mode and mark the filesystem as read-only in the XFS and VFS layers.
>> 
>> This roughly approximates the 'errors=remount-ro' mount option in ext4
>> but is implicit and the scope only covers errors during log recovery.
>> Since XFS is the default filesystem for some distributions, this change
>> allows users to continue to use XFS on these read-only ATA devices.
> 
> What is the workload or scenario where you need this behavior?
> 
> I'm not a big fan of ~silently mounting a filesystem with latent errors,
> tbh, but maybe you can explain a bit more about the problem you're solving
> here?

Hi Eric,

We use SSDs from multiple vendors that can be configured at power-on (via
GPIO) to be read-write or write-protected. When write-protected we get I/O
errors for any writes that reach the device. We believe that behavior is
correct.

We have found that XFS fails during log recovery even when the log is clean
(apparently due to metadata writes immediately before actual recovery).
Vincent and I believe that mounting read-only without recovery should be
fine even when the log is not clean, since the filesystem will be consistent,
even if out-of-date.

Our customers' use often requires nonvolatile memory to be write-protected
or not based on the device being installed in a development or deployed
system. It is ideal for them to be able to mount their filesystems read-
write when possible and read-only when not without having to alter mount
options.

Aaron

> Thanks,
> -Eric
> 
>> Reviewed-by: Aaron Sierra <asierra@xes-inc.com>
>> Signed-off-by: Vincent Fazio <vfazio@xes-inc.com>
>> ---
>>  fs/xfs/xfs_log.c | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>> 
>> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
>> index f6006d94a581..f5b3528ee028 100644
>> --- a/fs/xfs/xfs_log.c
>> +++ b/fs/xfs/xfs_log.c
>> @@ -739,7 +739,6 @@ xfs_log_mount(
>>  			xfs_warn(mp, "log mount/recovery failed: error %d",
>>  				error);
>>  			xlog_recover_cancel(mp->m_log);
>> -			goto out_destroy_ail;
>>  		}
>>  	}
>>  
>> @@ -3873,10 +3872,17 @@ xfs_log_force_umount(
>>  	/*
>>  	 * If this happens during log recovery, don't worry about
>>  	 * locking; the log isn't open for business yet.
>> +	 *
>> +	 * Attempt a read-only, norecovery mount. Ensure the VFS layer is updated.
>>  	 */
>>  	if (!log ||
>>  	    log->l_flags & XLOG_ACTIVE_RECOVERY) {
>> -		mp->m_flags |= XFS_MOUNT_FS_SHUTDOWN;
>> +
>> +		xfs_notice(mp,
>> +"Falling back to no-recovery mode. Filesystem will be inconsistent.");
>> +		mp->m_flags |= (XFS_MOUNT_RDONLY | XFS_MOUNT_NORECOVERY);
>> +		mp->m_super->s_flags |= SB_RDONLY;
>> +
>>  		if (mp->m_sb_bp)
>>  			mp->m_sb_bp->b_flags |= XBF_DONE;
>>  		return 0;