From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A989A23EA97 for ; Sat, 4 Apr 2026 16:36:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775320607; cv=none; b=M/j2GV8SlHb2WTQ/euV9C9NTBJL7DmwLoRuZY9AsY6kQbEf5fLrSs8GWOmvDZtIaki4qivr9rLsWal8Js7C+Puuys4OuKQVlAmXbtU6/wjVIwIUWxINce0kZjZnY0sIMvn7DQ3+JQCNr+mgTWeOCN1r+c9iWptl/B6jPGfPHHlM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775320607; c=relaxed/simple; bh=sZXoe47neHD71OqcOAFkuD6YSfRptHS75Dm1d3Xq5X4=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=blqANdYfVfab7S3dRinn3qXL9+3GNWSt481MTz9gro7Jb8RPB2v3gugdNFyrRIGMjRmsGbGV7/0F+X1X5mPFv/dnmJfaTOWkNF956b6gqfrdbxqvf3424mJ30Z/F9gsaqKzJ6wmOmLLZ/qpAKZ+SERqrDb+0Aw5eGiEILugI4TI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=IO0ZqL3/; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="IO0ZqL3/" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2483BC19421; Sat, 4 Apr 2026 16:36:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1775320607; bh=sZXoe47neHD71OqcOAFkuD6YSfRptHS75Dm1d3Xq5X4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=IO0ZqL3/BB0BFaMkMtzdf25aavHyHyKI5XEae+7esxabeBhdc4JxdBOodxB04DqhL Mt/eX1ZnrAnYgDsOPOeSY1dg1we2ISm6XQnoJOtPptcxENm7dIoZiN0HR/s+GgvTZq jGUFnoTL1Nkmj6UEB+9nSN1IiYepeLBJFM394wiohN1//VvZWZpGmjkRQaiYOfn9eR O3tUUM1j4PBq8PzfgqzXLRr4ddHycO73qUokzdl+0W/Su1lxhzJzoAPuCupBvuT/s6 WV0bqw6T3raeRvLxumatd0b6a8UPxE+W7IKq/vP8NYKxso2DaVmt+V6XhibJptEzVB YTPBxbTCR0YSw== From: Zorro Lang To: linux-xfs@vger.kernel.org Cc: Eric Sandeen Subject: [PATCH 2/2] mkfs: unify validation behavior for data, log and rt dev Date: Sun, 5 Apr 2026 00:36:40 +0800 Message-ID: <20260404163640.1013997-3-zlang@kernel.org> X-Mailer: git-send-email 2.52.0 In-Reply-To: <20260404163640.1013997-1-zlang@kernel.org> References: <20260404163640.1013997-1-zlang@kernel.org> Precedence: bulk X-Mailing-List: linux-xfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit The current validation logic in validate_datadev, validate_logdev, and validate_rtdev is inconsistent and confusing when checking device sizes, particularly when handling file images. This patch unifies the validation flow by categorizing devices into two distinct cases: "regular file" and "block device". Validation is now performed separately for each case across all three subvolumes to ensure consistent behavior. Signed-off-by: Zorro Lang --- Hi, validate_datadev, validate_logdev and validate_rtdev, these three functions handle xi->*.size, cfg->*blocks, and cli->*size inconsistently while also juggling xi->*.isfile status. Three functions ideally have similar validation patterns, but instead of following a template, each function has its own custom implementation, which invites bugs, maintenance overhead and inconsistent behavior, especially for file images. For example, mkfs.xfs works on an empty data file with -d size=xxx: # mkfs.xfs -f -d name=/home/emptyfile,size=300m meta-data=/home/emptyfile isize=512 agcount=4, agsize=19200 blks = sectsz=512 attr=2, projid32bit=1 = crc=1 finobt=1, sparse=1, rmapbt=1 = reflink=1 bigtime=1 inobtcount=1 nrext64=1 = exchange=1 metadir=0 data = bsize=4096 blocks=76800, imaxpct=25 = sunit=0 swidth=0 blks naming =version 2 bsize=4096 ascii-ci=0, ftype=1, parent=1 log =internal log bsize=4096 blocks=16384, version=2 = sectsz=512 sunit=0 blks, lazy-count=1 realtime =none extsz=4096 blocks=0, rtextents=0 = rgcount=0 rgsize=0 extents = zoned=0 start=0 reserved=0 But for log or rt, we got below weird errors: # mkfs.xfs -f -l logdev=/home/emptyfile,size=128m /dev/pmem1 size 128m specified for log subvolume is too large, maximum is 0 blocks ... # mkfs.xfs -f -r rtdev=/home/emptyfile,size=128m /dev/pmem1 Invalid zero length rt subvolume found ... One said the "size=128m" is too large, maximum is 0 (??? due to the file size is 0). The other one ignored the "size=128m", just complained the empty file. Thanks, Zorro mkfs/xfs_mkfs.c | 115 ++++++++++++++++++++++++++++++------------------ 1 file changed, 72 insertions(+), 43 deletions(-) diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c index 9a93330f..5a2274ed 100644 --- a/mkfs/xfs_mkfs.c +++ b/mkfs/xfs_mkfs.c @@ -3839,34 +3839,37 @@ validate_datadev( { struct libxfs_init *xi = cli->xi; - if (!xi->data.size) { + if (!xi->data.isfile) { /* * if the device is a file, we can't validate the size here. * Instead, the file will be truncated to the correct length * later on. if it's not a file, we've got a dud device. */ - if (!xi->data.isfile) { + if (!xi->data.size) { fprintf(stderr, _("can't get size of data subvolume\n")); usage(); - } else { - if (!cli->dsize) { + } + if (cfg->dblocks) { + /* check the size fits into the underlying device */ + if (cfg->dblocks > DTOBT(xi->data.size, cfg->blocklog)) { fprintf(stderr, -_("Warning: Empty file needs a data subvolume size by -d size= option\n")); +_("size %s specified for data subvolume is too large, maximum is %lld blocks\n"), + cli->dsize, + (long long)DTOBT(xi->data.size, cfg->blocklog)); usage(); } + } else { + /* no user size, so use the full block device */ + cfg->dblocks = DTOBT(xi->data.size, cfg->blocklog); } - } else if (cfg->dblocks) { - /* check the size fits into the underlying device */ - if (cfg->dblocks > DTOBT(xi->data.size, cfg->blocklog)) { + } else { + if (!cfg->dblocks && !xi->data.size) { fprintf(stderr, -_("size %s specified for data subvolume is too large, maximum is %lld blocks\n"), - cli->dsize, - (long long)DTOBT(xi->data.size, cfg->blocklog)); +_("Warning: Empty data file needs a data subvolume size by -d size= option\n")); usage(); + } else if (xi->data.size && !cfg->dblocks) { + cfg->dblocks = DTOBT(xi->data.size, cfg->blocklog); } - } else { - /* no user size, so use the full block device */ - cfg->dblocks = DTOBT(xi->data.size, cfg->blocklog); } if (cfg->dblocks < XFS_MIN_DATA_BLOCKS(cfg)) { @@ -3925,19 +3928,31 @@ _("log size %lld too large for internal log\n"), usage(); } - if (!cfg->logblocks) { - if (xi->log.size == 0) { + if (!xi->log.isfile) { + if (!xi->log.size) { + fprintf(stderr, _("can't get size of log subvolume\n")); + usage(); + } else if (cfg->logblocks) { + /* check the size fits into the underlying device */ + if (cfg->logblocks > DTOBT(xi->log.size, cfg->blocklog)) { + fprintf(stderr, +_("size %s specified for log subvolume is too large, maximum is %lld blocks\n"), + cli->logsize, + (long long)DTOBT(xi->log.size, cfg->blocklog)); + usage(); + } + } else { + /* no user size, so use the full block device */ + cfg->logblocks = DTOBT(xi->log.size, cfg->blocklog); + } + } else { + if (!cfg->logblocks && !xi->log.size) { fprintf(stderr, -_("unable to get size of the log subvolume.\n")); +_("Warning: Empty log file needs a log subvolume size by -l size= option\n")); usage(); + } else if (xi->log.size && !cfg->logblocks) { + cfg->logblocks = DTOBT(xi->log.size, cfg->blocklog); } - cfg->logblocks = DTOBT(xi->log.size, cfg->blocklog); - } else if (cfg->logblocks > DTOBT(xi->log.size, cfg->blocklog)) { - fprintf(stderr, -_("size %s specified for log subvolume is too large, maximum is %lld blocks\n"), - cli->logsize, - (long long)DTOBT(xi->log.size, cfg->blocklog)); - usage(); } if (xi->log.bsize > cfg->lsectorsize) { @@ -3968,31 +3983,45 @@ _("size specified for non-existent rt subvolume\n")); cfg->rtbmblocks = 0; return; } - if (!xi->rt.size) { - fprintf(stderr, _("Invalid zero length rt subvolume found\n")); - usage(); - } - if (cli->rtsize) { - if (cfg->rtblocks > DTOBT(xi->rt.size, cfg->blocklog)) { - fprintf(stderr, + if (!xi->rt.isfile) { + if (!xi->rt.size) { + fprintf(stderr, _("can't get size of realtime subvolume\n")); + usage(); + } + if (cfg->rtblocks) { + /* check the size fits into the underlying device */ + if (cfg->rtblocks > DTOBT(xi->rt.size, cfg->blocklog)) { + fprintf(stderr, _("size %s specified for rt subvolume is too large, maximum is %lld blocks\n"), - cli->rtsize, - (long long)DTOBT(xi->rt.size, cfg->blocklog)); + cli->rtsize, + (long long)DTOBT(xi->rt.size, cfg->blocklog)); + usage(); + } + } else { + /* no user size, so use the full block device */ + if (zt->rt.nr_zones) { + cfg->rtblocks = DTOBT(zt->rt.nr_zones * zt->rt.zone_capacity, + cfg->blocklog); + } else { + cfg->rtblocks = DTOBT(xi->rt.size, cfg->blocklog); + } + } + } else { + if (!cfg->rtblocks && !xi->rt.size) { + fprintf(stderr, +_("Warning: Empty rt file needs a rt subvolume size by -r size= option\n")); usage(); + } else if (xi->rt.size && !cfg->rtblocks) { + cfg->rtblocks = DTOBT(xi->rt.size, cfg->blocklog); } - if (xi->rt.bsize > cfg->sectorsize) { - fprintf(stderr, _( + } + + if (xi->rt.bsize > cfg->sectorsize) { + fprintf(stderr, _( "Warning: the realtime subvolume sector size %u is less than the sector size\n\ reported by the device (%u).\n"), - cfg->sectorsize, xi->rt.bsize); - } - } else if (zt->rt.nr_zones) { - cfg->rtblocks = DTOBT(zt->rt.nr_zones * zt->rt.zone_capacity, - cfg->blocklog); - } else { - /* grab volume size */ - cfg->rtblocks = DTOBT(xi->rt.size, cfg->blocklog); + cfg->sectorsize, xi->rt.bsize); } cfg->rtextents = cfg->rtblocks / cfg->rtextblocks; -- 2.52.0