[Ocfs2-tools-devel] [PATCH] libocfs2: ocfs2_write_super() should update all superblocks

[Tao Ma]

Looks great. Just one concern.

Joel Becker wrote:
> Normal operations should keep the backup superblocks up to date.  Thus,
> ocfs2_write_super() is modified to refresh the backups.
> ocfs2_write_primary_super() is introduced for operations that want to
> leave the backups untouched.
> 
> The backup superblock APIs are slightly modified for clarity.  In
> particular, ocfs2_refresh_backup_supers() is introduced to refresh all
> backups.  It does nothing if backup superblocks are not enabled.  The
> old API are kept but deprecated.
> 
> The rest of the tools now use the new APIs.  This includes using
> ocfs2_write_primary_super() where necessary.
> 
> Signed-off-by: Joel Becker <joel.becker at oracle.com>
> @@ -1552,38 +1539,14 @@ skip_cluster_start:
>  		block_signals(SIG_BLOCK);
>  		ret = ocfs2_write_super(fs);
Here, you write the backups twice if the user's option is add 
backup-support(since we have already written once in 
update_backup_super). And the old solution will remove backup flag if 
the backup update fails(see below), but now I am not sure whether it is 
appropriate.


>  		if (ret) {
> -			com_err(opts.progname, ret, "while writing superblock");
> +			com_err(opts.progname, ret,
> +				"while writing superblock(s)");
>  			goto unlock;
>  		}
>  		block_signals(SIG_UNBLOCK);
> -		printf("Wrote Superblock\n");
> -
> -		/* superblock's information has changed.
> -		 * We need to synchronize the backup blocks if needed.
> -		 * We also have to admit that if upd_backup_super is set,
> -		 * there is no need to refresh the backups since they are
> -		 * written above by update_backup_super.
> -		 */
> -		if (!upd_backup_super &&
> -		    OCFS2_HAS_COMPAT_FEATURE(OCFS2_RAW_SB(fs->fs_super),
> -					     OCFS2_FEATURE_COMPAT_BACKUP_SB)) {
> -			block_signals(SIG_BLOCK);
> -			ret = refresh_backup_super(fs);
> -			block_signals(SIG_UNBLOCK);
> -			if (ret) {
> -				printf("Unable to refresh backup superblocks. "
> -					"Please run fsck.ocfs2 before running "
> -					"tunefs.ocfs2 to re-enable "
> -					"backup superblocks.");
> -				OCFS2_CLEAR_COMPAT_FEATURE(
> -					OCFS2_RAW_SB(fs->fs_super),
> -					OCFS2_FEATURE_COMPAT_BACKUP_SB);
> -				block_signals(SIG_BLOCK);
> -				ocfs2_write_super(fs);
> -				block_signals(SIG_UNBLOCK);
> -			}
> -		}
> +		printf("Wrote Superblock(s)\n");
>  	}
> +
>  online_resize_unlock:
>  	if (online_resize)
>  		online_resize_unlock(fs);

Regards,
Tao