| Message ID | 20211113031408.17521-1-wangyugui@e16-tech.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: fix discard support check | expand |
On 2021/11/13 11:14, Wang Yugui wrote: > [BUG] > mkfs.btrfs(v5.15) output a message even if the disk is a HDD without > TRIM/DISCARD support. > Performing full device TRIM /dev/sdc2 (326.03GiB) ... > > [CAUSE] > mkfs.btrfs check TRIM/DISCARD support through the content of > queue/discard_granularity, but compare it against a wrong value. > > When hdd without TRIM/DISCARD support, the content of > queue/discard_granularity is '0' '\n' '\0', rather than '0' '\0'. Can we get rid of such bad comparison and just go strtoll() and compare the value? Thanks, Qu > > [FIX] > compare it against the right value. > > Fixes: c50c448518bb ("btrfs-progs: do sysfs detection of device discard capability") > Signed-off-by: Wang Yugui <wangyugui@e16-tech.com> > --- > common/device-utils.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/common/device-utils.c b/common/device-utils.c > index 74a25879..76d5c584 100644 > --- a/common/device-utils.c > +++ b/common/device-utils.c > @@ -64,7 +64,7 @@ static int discard_supported(const char *device) > pr_verbose(3, "cannot read discard_granularity for %s\n", device); > return 0; > } else { > - if (buf[0] == '0' && buf[1] == 0) { > + if (buf[0] == '0' && buf[1] == '\n') { > pr_verbose(3, "%s: discard_granularity %s\n", device, buf); > return 0; > } >
Hi, > On 2021/11/13 11:14, Wang Yugui wrote: > > [BUG] > > mkfs.btrfs(v5.15) output a message even if the disk is a HDD without > > TRIM/DISCARD support. > > Performing full device TRIM /dev/sdc2 (326.03GiB) ... > > > > [CAUSE] > > mkfs.btrfs check TRIM/DISCARD support through the content of > > queue/discard_granularity, but compare it against a wrong value. > > > > When hdd without TRIM/DISCARD support, the content of > > queue/discard_granularity is '0' '\n' '\0', rather than '0' '\0'. > > Can we get rid of such bad comparison and just go strtoll() and compare > the value? strtoll() or strcmp() is a good choice for refact. but now just a direct fix. Best Regards Wang Yugui (wangyugui@e16-tech.com) 2021/11/13 > > > > > [FIX] > > compare it against the right value. > > > > Fixes: c50c448518bb ("btrfs-progs: do sysfs detection of device discard capability") > > Signed-off-by: Wang Yugui <wangyugui@e16-tech.com> > > --- > > common/device-utils.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/common/device-utils.c b/common/device-utils.c > > index 74a25879..76d5c584 100644 > > --- a/common/device-utils.c > > +++ b/common/device-utils.c > > @@ -64,7 +64,7 @@ static int discard_supported(const char *device) > > pr_verbose(3, "cannot read discard_granularity for %s\n", device); > > return 0; > > } else { > > - if (buf[0] == '0' && buf[1] == 0) { > > + if (buf[0] == '0' && buf[1] == '\n') { > > pr_verbose(3, "%s: discard_granularity %s\n", device, buf); > > return 0; > > } > >
On 13.11.21 г. 9:04, Wang Yugui wrote: > Hi, > >> On 2021/11/13 11:14, Wang Yugui wrote: >>> [BUG] >>> mkfs.btrfs(v5.15) output a message even if the disk is a HDD without >>> TRIM/DISCARD support. >>> Performing full device TRIM /dev/sdc2 (326.03GiB) ... >>> >>> [CAUSE] >>> mkfs.btrfs check TRIM/DISCARD support through the content of >>> queue/discard_granularity, but compare it against a wrong value. >>> >>> When hdd without TRIM/DISCARD support, the content of >>> queue/discard_granularity is '0' '\n' '\0', rather than '0' '\0'. >> >> Can we get rid of such bad comparison and just go strtoll() and compare >> the value? > > strtoll() or strcmp() is a good choice for refact. but now just a > direct fix. NACK, Use one of the conversion functions to convert the string to a number and do a != 0 comparison. atoi() would be sufficient and deals with whitespace/trailing \n character gracefully. I think it's high time we ditch the "let's do a quick and dirty fix" mentality which leads to unmaintainable gunk in the long-term as it tends to rarely get fixed afterwards. <snip>
diff --git a/common/device-utils.c b/common/device-utils.c index 74a25879..76d5c584 100644 --- a/common/device-utils.c +++ b/common/device-utils.c @@ -64,7 +64,7 @@ static int discard_supported(const char *device) pr_verbose(3, "cannot read discard_granularity for %s\n", device); return 0; } else { - if (buf[0] == '0' && buf[1] == 0) { + if (buf[0] == '0' && buf[1] == '\n') { pr_verbose(3, "%s: discard_granularity %s\n", device, buf); return 0; }
[BUG] mkfs.btrfs(v5.15) output a message even if the disk is a HDD without TRIM/DISCARD support. Performing full device TRIM /dev/sdc2 (326.03GiB) ... [CAUSE] mkfs.btrfs check TRIM/DISCARD support through the content of queue/discard_granularity, but compare it against a wrong value. When hdd without TRIM/DISCARD support, the content of queue/discard_granularity is '0' '\n' '\0', rather than '0' '\0'. [FIX] compare it against the right value. Fixes: c50c448518bb ("btrfs-progs: do sysfs detection of device discard capability") Signed-off-by: Wang Yugui <wangyugui@e16-tech.com> --- common/device-utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)