Currently stripe unit/swidth checking logic is all over xfsprogs.
So, refactor the same code snippet into a single validation helper
xfs_validate_stripe_factors(), including:
- integer overflows of either value
- sunit and swidth alignment wrt sector size
- if either sunit or swidth are zero, both should be zero
- swidth must be a multiple of sunit
Signed-off-by: Gao Xiang <hsiangkao@xxxxxxxxxx>
---
changes since v1:
- several update (errant space, full names...) suggested by Darrick;
- rearrange into a unique handler in calc_stripe_factors();
- add libfrog_stripeval_str[] yet I'm not sure if it needs localization;
- update po translalation due to (%lld type -> %d).
(I'd still like to post it in advance...)
libfrog/topology.c | 68 +++++++++++++++++++++++++++++++++++++++++
libfrog/topology.h | 17 +++++++++++
mkfs/xfs_mkfs.c | 76 ++++++++++++++++++++++++----------------------
po/pl.po | 4 +--
4 files changed, 126 insertions(+), 39 deletions(-)
diff --git a/libfrog/topology.c b/libfrog/topology.c
index b1b470c9..1ce151fd 100644
--- a/libfrog/topology.c
+++ b/libfrog/topology.c
@@ -174,6 +174,59 @@ out:
return ret;
}
+/*
+ * This accepts either
+ * - (sectersize != 0) dsu (in bytes) / dsw (which is mulplier of dsu)
+ * or
+ * - (sectersize == 0) dunit / dwidth (in 512b sector size)
+ * and return sunit/swidth in sectors.
+ */
+enum libfrog_stripeval
+libfrog_validate_stripe_factors(
+ int sectorsize,
+ int *sunitp,
+ int *swidthp)
+{
+ int sunit = *sunitp;
+ int swidth = *swidthp;
+
+ if (sectorsize) {
+ long long big_swidth;
+
+ if (sunit % sectorsize)
+ return LIBFROG_STRIPEVAL_SUNIT_MISALIGN;
+
+ sunit = (int)BTOBBT(sunit);
+ big_swidth = (long long)sunit * swidth;
+
+ if (big_swidth > INT_MAX)
+ return LIBFROG_STRIPEVAL_SWIDTH_OVERFLOW;
+ swidth = big_swidth;
+ }
+
+ if ((sunit && !swidth) || (!sunit && swidth))
+ return LIBFROG_STRIPEVAL_PARTIAL_VALID;
+
+ if (sunit > swidth)
+ return LIBFROG_STRIPEVAL_SUNIT_TOO_LARGE;
+
+ if (sunit && (swidth % sunit))
+ return LIBFROG_STRIPEVAL_SWIDTH_MISALIGN;
+
+ *sunitp = sunit;
+ *swidthp = swidth;
+ return LIBFROG_STRIPEVAL_OK;
+}
+
+const char *libfrog_stripeval_str[] = {
+ "OK",
+ "SUNIT_MISALIGN",
+ "SWIDTH_OVERFLOW",
+ "PARTIAL_VALID",
+ "SUNIT_TOO_LARGE",
+ "SWIDTH_MISALIGN",
+};
+
static void blkid_get_topology(
const char *device,
int *sunit,
@@ -187,6 +240,7 @@ static void blkid_get_topology(
blkid_probe pr;
unsigned long val;
struct stat statbuf;
+ enum libfrog_stripeval error;
/* can't get topology info from a file */
if (!stat(device, &statbuf) && S_ISREG(statbuf.st_mode)) {
@@ -230,6 +284,20 @@ static void blkid_get_topology(
*sunit = *sunit >> 9;
*swidth = *swidth >> 9;
+ error = libfrog_validate_stripe_factors(0, sunit, swidth);
+ if (error) {
+ fprintf(stderr,
+_("%s: Volume reports invalid sunit (%d bytes) and swidth (%d bytes) %s, ignoring.\n"),
+ progname, BBTOB(*sunit), BBTOB(*swidth),
+ libfrog_stripeval_str[error]);
+ /*
+ * if firmware is broken, just give up and set both to zero,
+ * we can't trust information from this device.
+ */
+ *sunit = 0;
+ *swidth = 0;
+ }
+
if (blkid_topology_get_alignment_offset(tp) != 0) {
fprintf(stderr,
_("warning: device is not properly aligned %s\n"),
diff --git a/libfrog/topology.h b/libfrog/topology.h
index 6fde868a..507fe121 100644
--- a/libfrog/topology.h
+++ b/libfrog/topology.h
@@ -36,4 +36,21 @@ extern int
check_overwrite(
const char *device);
+enum libfrog_stripeval {
+ LIBFROG_STRIPEVAL_OK = 0,
+ LIBFROG_STRIPEVAL_SUNIT_MISALIGN,
+ LIBFROG_STRIPEVAL_SWIDTH_OVERFLOW,
+ LIBFROG_STRIPEVAL_PARTIAL_VALID,
+ LIBFROG_STRIPEVAL_SUNIT_TOO_LARGE,
+ LIBFROG_STRIPEVAL_SWIDTH_MISALIGN,
+};
+
+extern const char *libfrog_stripeval_str[];
+
+enum libfrog_stripeval
+libfrog_validate_stripe_factors(
+ int sectorsize,
+ int *sunitp,
+ int *swidthp);
+
#endif /* __LIBFROG_TOPOLOGY_H__ */
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 2e6cd280..f7b38b36 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -2255,14 +2255,14 @@ calc_stripe_factors(
struct cli_params *cli,
struct fs_topology *ft)
{
- long long int big_dswidth;
- int dsunit = 0;
- int dswidth = 0;
- int lsunit = 0;
- int dsu = 0;
- int dsw = 0;
- int lsu = 0;
- bool use_dev = false;
+ int dsunit = 0;
+ int dswidth = 0;
+ int lsunit = 0;
+ int dsu = 0;
+ int dsw = 0;
+ int lsu = 0;
+ bool use_dev = false;
+ enum libfrog_stripeval error;
if (cli_opt_set(&dopts, D_SUNIT))
dsunit = cli->dsunit;
@@ -2289,29 +2289,40 @@ _("both data su and data sw options must be specified\n"));
usage();
}
- if (dsu % cfg->sectorsize) {
- fprintf(stderr,
-_("data su must be a multiple of the sector size (%d)\n"), cfg->sectorsize);
- usage();
- }
-
- dsunit = (int)BTOBBT(dsu);
- big_dswidth = (long long int)dsunit * dsw;
- if (big_dswidth > INT_MAX) {
- fprintf(stderr,
-_("data stripe width (%lld) is too large of a multiple of the data stripe unit (%d)\n"),
- big_dswidth, dsunit);
- usage();
- }
- dswidth = big_dswidth;
+ dsunit = dsu;
+ dswidth = dsw;
+ error = libfrog_validate_stripe_factors(cfg->sectorsize,
+ &dsunit, &dswidth);
+ } else {
+ error = libfrog_validate_stripe_factors(0, &dsunit, &dswidth);
}
- if ((dsunit && !dswidth) || (!dsunit && dswidth) ||
- (dsunit && (dswidth % dsunit != 0))) {
+ switch (error) {
+ case LIBFROG_STRIPEVAL_OK:
+ break;
+ case LIBFROG_STRIPEVAL_SUNIT_MISALIGN:
+ fprintf(stderr,
+_("data su must be a multiple of the sector size (%d)\n"), cfg->sectorsize);
+ usage();
+ break;
+ case LIBFROG_STRIPEVAL_SWIDTH_OVERFLOW:
+ fprintf(stderr,
+_("data stripe width (%d) is too large of a multiple of the data stripe unit (%d)\n"),
+ dsw, dsunit);
+ usage();
+ break;
+ case LIBFROG_STRIPEVAL_PARTIAL_VALID:
+ case LIBFROG_STRIPEVAL_SWIDTH_MISALIGN:
fprintf(stderr,
_("data stripe width (%d) must be a multiple of the data stripe unit (%d)\n"),
dswidth, dsunit);
usage();
+ break;
+ default:
+ fprintf(stderr,
+_("invalid data stripe unit (%d), width (%d) %s\n"),
+ dsunit, dswidth, libfrog_stripeval_str[error]);
+ usage();
}
/* If sunit & swidth were manually specified as 0, same as noalign */
@@ -2328,18 +2339,9 @@ _("data stripe width (%d) must be a multiple of the data stripe unit (%d)\n"),
/* if no stripe config set, use the device default */
if (!dsunit) {
- /* Ignore nonsense from device. XXX add more validation */
- if (ft->dsunit && ft->dswidth == 0) {
- fprintf(stderr,
-_("%s: Volume reports stripe unit of %d bytes and stripe width of 0, ignoring.\n"),
- progname, BBTOB(ft->dsunit));
- ft->dsunit = 0;
- ft->dswidth = 0;
- } else {
- dsunit = ft->dsunit;
- dswidth = ft->dswidth;
- use_dev = true;
- }
+ dsunit = ft->dsunit;
+ dswidth = ft->dswidth;
+ use_dev = true;
} else {
/* check and warn if user-specified alignment is sub-optimal */
if (ft->dsunit && ft->dsunit != dsunit) {
diff --git a/po/pl.po b/po/pl.po
index 87109f6b..02d2258f 100644
--- a/po/pl.po
+++ b/po/pl.po
@@ -9085,10 +9085,10 @@ msgstr "su danych musi być wielokrotnością rozmiaru sektora (%d)\n"
#: .././mkfs/xfs_mkfs.c:2267
#, c-format
msgid ""
-"data stripe width (%lld) is too large of a multiple of the data stripe unit "
+"data stripe width (%d) is too large of a multiple of the data stripe unit "
"(%d)\n"
msgstr ""
-"szerokość pasa danych (%lld) jest zbyt dużą wielokrotnością jednostki pasa "
+"szerokość pasa danych (%d) jest zbyt dużą wielokrotnością jednostki pasa "
"danych (%d)\n"
#: .././mkfs/xfs_mkfs.c:2276
--
2.18.1
