Re: [PATCH V4 19/20] xfsprogs: Add support for upgrading to NREXT64 feature

Chandan Babu R
2022-01-07T16:17:25Z

On 05 Jan 2022 at 06:47, Darrick J. Wong wrote:
> On Tue, Dec 14, 2021 at 02:18:10PM +0530, Chandan Babu R wrote:
>> This commit adds support to xfs_repair to allow upgrading an existing
>> filesystem to support per-inode large extent counters.
>>
>> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
>> ---
>> repair/globals.c | 1 +
>> repair/globals.h | 1 +
>> repair/phase2.c | 35 ++++++++++++++++++++++++++++++++++-
>> repair/xfs_repair.c | 11 +++++++++++
>> 4 files changed, 47 insertions(+), 1 deletion(-)
>>
>> diff --git a/repair/globals.c b/repair/globals.c
>> index d89507b1..2f29391a 100644
>> --- a/repair/globals.c
>> +++ b/repair/globals.c
>> @@ -53,6 +53,7 @@ bool add_bigtime; /* add support for timestamps up to 2486 */
>> bool add_finobt; /* add free inode btrees */
>> bool add_reflink; /* add reference count btrees */
>> bool add_rmapbt; /* add reverse mapping btrees */
>> +bool add_nrext64;
>>
>> /* misc status variables */
>>
>> diff --git a/repair/globals.h b/repair/globals.h
>> index 53ff2532..af0bcb6b 100644
>> --- a/repair/globals.h
>> +++ b/repair/globals.h
>> @@ -94,6 +94,7 @@ extern bool add_bigtime; /* add support for timestamps up to 2486 */
>> extern bool add_finobt; /* add free inode btrees */
>> extern bool add_reflink; /* add reference count btrees */
>> extern bool add_rmapbt; /* add reverse mapping btrees */
>> +extern bool add_nrext64;
>>
>> /* misc status variables */
>>
>> diff --git a/repair/phase2.c b/repair/phase2.c
>> index c811ed5d..c9db3281 100644
>> --- a/repair/phase2.c
>> +++ b/repair/phase2.c
>> @@ -191,6 +191,7 @@ check_new_v5_geometry(
>> struct xfs_perag *pag;
>> xfs_agnumber_t agno;
>> xfs_ino_t rootino;
>> + uint old_bm_maxlevels[2];
>> int min_logblocks;
>> int error;
>>
>> @@ -201,6 +202,12 @@ check_new_v5_geometry(
>> memcpy(&old_sb, &mp->m_sb, sizeof(struct xfs_sb));
>> memcpy(&mp->m_sb, new_sb, sizeof(struct xfs_sb));
>>
>> + old_bm_maxlevels[0] = mp->m_bm_maxlevels[0];
>> + old_bm_maxlevels[1] = mp->m_bm_maxlevels[1];
>> +
>> + xfs_bmap_compute_maxlevels(mp, XFS_DATA_FORK);
>> + xfs_bmap_compute_maxlevels(mp, XFS_ATTR_FORK);
>
> Ahh... I see why you added my (evil) patch that allows upgrading a
> filesystem to reflink -- you need the check_new_v5_geometry function so
> that you can check if the log size is big enough to handle larger bmbt
> trees.
>
> Hmm, I guess I should work on separating this from the actual
> rmap/reflink/finobt upgrade code, since I have no idea if we /ever/ want
> to support that.
>

I can do that. I will include the trimmed down version of the patch before
posting the patchset once again.

> --D
>
>> +
>> /* Do we have a big enough log? */
>> min_logblocks = libxfs_log_calc_minimum_size(mp);
>> if (old_sb.sb_logblocks < min_logblocks) {
>> @@ -288,6 +295,9 @@ check_new_v5_geometry(
>> pag->pagi_init = 0;
>> }
>>
>> + mp->m_bm_maxlevels[0] = old_bm_maxlevels[0];
>> + mp->m_bm_maxlevels[1] = old_bm_maxlevels[1];
>> +
>> /*
>> * Put back the old superblock.
>> */
>> @@ -366,6 +376,28 @@ set_rmapbt(
>> return true;
>> }
>>
>> +static bool
>> +set_nrext64(
>> + struct xfs_mount *mp,
>> + struct xfs_sb *new_sb)
>> +{
>> + if (!xfs_sb_version_hascrc(&mp->m_sb)) {
>> + printf(
>> + _("Nrext64 only supported on V5 filesystems.\n"));
>> + exit(0);
>> + }
>> +
>> + if (xfs_sb_version_hasnrext64(&mp->m_sb)) {
>> + printf(_("Filesystem already supports nrext64.\n"));
>> + exit(0);
>> + }
>> +
>> + printf(_("Adding nrext64 to filesystem.\n"));
>> + new_sb->sb_features_incompat |= XFS_SB_FEAT_INCOMPAT_NREXT64;
>> + new_sb->sb_features_incompat |= XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR;
>> + return true;
>> +}
>> +
>> /* Perform the user's requested upgrades on filesystem. */
>> static void
>> upgrade_filesystem(
>> @@ -388,7 +420,8 @@ upgrade_filesystem(
>> dirty |= set_reflink(mp, &new_sb);
>> if (add_rmapbt)
>> dirty |= set_rmapbt(mp, &new_sb);
>> -
>> + if (add_nrext64)
>> + dirty |= set_nrext64(mp, &new_sb);
>> if (!dirty)
>> return;
>>
>> diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
>> index e250a5bf..96c9bb56 100644
>> --- a/repair/xfs_repair.c
>> +++ b/repair/xfs_repair.c
>> @@ -70,6 +70,7 @@ enum c_opt_nums {
>> CONVERT_FINOBT,
>> CONVERT_REFLINK,
>> CONVERT_RMAPBT,
>> + CONVERT_NREXT64,
>> C_MAX_OPTS,
>> };
>>
>> @@ -80,6 +81,7 @@ static char *c_opts[] = {
>> [CONVERT_FINOBT] = "finobt",
>> [CONVERT_REFLINK] = "reflink",
>> [CONVERT_RMAPBT] = "rmapbt",
>> + [CONVERT_NREXT64] = "nrext64",
>> [C_MAX_OPTS] = NULL,
>> };
>>
>> @@ -357,6 +359,15 @@ process_args(int argc, char **argv)
>> _("-c rmapbt only supports upgrades\n"));
>> add_rmapbt = true;
>> break;
>> + case CONVERT_NREXT64:
>> + if (!val)
>> + do_abort(
>> + _("-c nrext64 requires a parameter\n"));
>> + if (strtol(val, NULL, 0) != 1)
>> + do_abort(
>> + _("-c nrext64 only supports upgrades\n"));
>> + add_nrext64 = true;
>> + break;
>> default:
>> unknown('c', val);
>> break;
>> --
>> 2.30.2
>>

--
chandan