| Message ID | c4d20fc5-150b-7afc-e2fd-2358e52acb9c@sandeen.net (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [V2] mkfs: tidy up discard notifications | expand |
On Sat, Dec 14, 2019 at 12:53:56PM -0600, Eric Sandeen wrote: > Only notify user of discard operations if the first one succeeds, > and be sure to print a trailing newline if we stop early. > > Signed-off-by: Eric Sandeen <sandeen@redhat.com> > --- > > V2: Logic is hard. ;) If I messed this one up, take it back Darrick. :) > > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c > index 4bfdebf6..606f79da 100644 > --- a/mkfs/xfs_mkfs.c > +++ b/mkfs/xfs_mkfs.c > @@ -1251,10 +1251,6 @@ discard_blocks(dev_t dev, uint64_t nsectors, int quiet) > fd = libxfs_device_to_fd(dev); > if (fd <= 0) > return; > - if (!quiet) { > - printf("Discarding blocks..."); > - fflush(stdout); > - } > > /* The block discarding happens in smaller batches so it can be > * interrupted prematurely > @@ -1267,12 +1263,20 @@ discard_blocks(dev_t dev, uint64_t nsectors, int quiet) > * not necessary for the mkfs functionality but just an > * optimization. However we should stop on error. > */ > - if (platform_discard_blocks(fd, offset, tmp_step)) > + if (platform_discard_blocks(fd, offset, tmp_step) == 0) { > + if (offset == 0 && !quiet) { > + printf("Discarding blocks..."); > + fflush(stdout); > + } > + } else { > + if (offset > 0 && !quiet) > + printf("\n"); Maybe we should say failed? Or ... eh... whatever, the format happens regardless. Looks ok, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > return; > + } > > offset += tmp_step; > } > - if (!quiet) > + if (offset > 0 && !quiet) > printf("Done.\n"); > } > > >
On 12/14/19 2:21 PM, Darrick J. Wong wrote: > On Sat, Dec 14, 2019 at 12:53:56PM -0600, Eric Sandeen wrote: >> Only notify user of discard operations if the first one succeeds, >> and be sure to print a trailing newline if we stop early. >> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com> >> --- >> >> V2: Logic is hard. ;) If I messed this one up, take it back Darrick. :) >> >> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c >> index 4bfdebf6..606f79da 100644 >> --- a/mkfs/xfs_mkfs.c >> +++ b/mkfs/xfs_mkfs.c >> @@ -1251,10 +1251,6 @@ discard_blocks(dev_t dev, uint64_t nsectors, int quiet) >> fd = libxfs_device_to_fd(dev); >> if (fd <= 0) >> return; >> - if (!quiet) { >> - printf("Discarding blocks..."); >> - fflush(stdout); >> - } >> >> /* The block discarding happens in smaller batches so it can be >> * interrupted prematurely >> @@ -1267,12 +1263,20 @@ discard_blocks(dev_t dev, uint64_t nsectors, int quiet) >> * not necessary for the mkfs functionality but just an >> * optimization. However we should stop on error. >> */ >> - if (platform_discard_blocks(fd, offset, tmp_step)) >> + if (platform_discard_blocks(fd, offset, tmp_step) == 0) { >> + if (offset == 0 && !quiet) { >> + printf("Discarding blocks..."); >> + fflush(stdout); >> + } >> + } else { >> + if (offset > 0 && !quiet) >> + printf("\n"); > > Maybe we should say failed? Or ... eh... whatever, the format happens > regardless. Yeah... a "failed" printf will launch support calls & emails. :) > Looks ok, > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Thanks Darrick. -Eric > --D > >> return; >> + } >> >> offset += tmp_step; >> } >> - if (!quiet) >> + if (offset > 0 && !quiet) >> printf("Done.\n"); >> } >> >> >> >
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c index 4bfdebf6..606f79da 100644 --- a/mkfs/xfs_mkfs.c +++ b/mkfs/xfs_mkfs.c @@ -1251,10 +1251,6 @@ discard_blocks(dev_t dev, uint64_t nsectors, int quiet) fd = libxfs_device_to_fd(dev); if (fd <= 0) return; - if (!quiet) { - printf("Discarding blocks..."); - fflush(stdout); - } /* The block discarding happens in smaller batches so it can be * interrupted prematurely @@ -1267,12 +1263,20 @@ discard_blocks(dev_t dev, uint64_t nsectors, int quiet) * not necessary for the mkfs functionality but just an * optimization. However we should stop on error. */ - if (platform_discard_blocks(fd, offset, tmp_step)) + if (platform_discard_blocks(fd, offset, tmp_step) == 0) { + if (offset == 0 && !quiet) { + printf("Discarding blocks..."); + fflush(stdout); + } + } else { + if (offset > 0 && !quiet) + printf("\n"); return; + } offset += tmp_step; } - if (!quiet) + if (offset > 0 && !quiet) printf("Done.\n"); }
Only notify user of discard operations if the first one succeeds, and be sure to print a trailing newline if we stop early. Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- V2: Logic is hard. ;) If I messed this one up, take it back Darrick. :)