| Message ID | 20180920092006.37596-1-jtulak@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [1/2,v2] mkfs: discard only after all validations | expand |
On Thu, Sep 20, 2018 at 11:20:06AM +0200, Jan Tulak wrote: > Discard should happen only when everything has been validated, just > before we start writing to the device. If it happens earlier, it is > possible that mkfs will abort, but managed to already wipe data. This > patch moves the discard to the latest possible moment. > > Signed-off-by: Jan Tulak <jtulak@redhat.com> Looks ok to me, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > CHANGES: > v2 > * discard_data -> discard_devices > * move the discard condition outside of the function > > --- > mkfs/xfs_mkfs.c | 22 ++++++++++++++++------ > 1 file changed, 16 insertions(+), 6 deletions(-) > > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c > index 2e53c1e8..c97c69e8 100644 > --- a/mkfs/xfs_mkfs.c > +++ b/mkfs/xfs_mkfs.c > @@ -2389,8 +2389,7 @@ _("log stripe unit (%d bytes) is too large (maximum is 256KiB)\n" > static void > open_devices( > struct mkfs_params *cfg, > - struct libxfs_xinit *xi, > - bool discard) > + struct libxfs_xinit *xi) > { > uint64_t sector_mask; > > @@ -2419,10 +2418,15 @@ open_devices( > xi->dsize &= sector_mask; > xi->rtsize &= sector_mask; > xi->logBBsize &= (uint64_t)-1 << (max(cfg->lsectorlog, 10) - BBSHIFT); > +} > > - > - if (!discard) > - return; > +static void > +discard_devices( > + struct libxfs_xinit *xi) > +{ > + /* > + * This function has to be called after libxfs has been initialized. > + */ > > if (!xi->disfile) > discard_blocks(xi->ddev, xi->dsize); > @@ -3901,7 +3905,7 @@ main( > /* > * Open and validate the device configurations > */ > - open_devices(&cfg, &xi, (discard && !dry_run)); > + open_devices(&cfg, &xi); > validate_datadev(&cfg, &cli); > validate_logdev(&cfg, &cli, &logfile); > validate_rtdev(&cfg, &cli, &rtfile); > @@ -3952,6 +3956,12 @@ main( > exit(0); > } > > + /* > + * All values have been validated, discard the old device layout. > + */ > + if (discard && !dry_run) > + discard_devices(&xi); > + > /* > * we need the libxfs buffer cache from here on in. > */ > -- > 2.18.0 >
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c index 2e53c1e8..c97c69e8 100644 --- a/mkfs/xfs_mkfs.c +++ b/mkfs/xfs_mkfs.c @@ -2389,8 +2389,7 @@ _("log stripe unit (%d bytes) is too large (maximum is 256KiB)\n" static void open_devices( struct mkfs_params *cfg, - struct libxfs_xinit *xi, - bool discard) + struct libxfs_xinit *xi) { uint64_t sector_mask; @@ -2419,10 +2418,15 @@ open_devices( xi->dsize &= sector_mask; xi->rtsize &= sector_mask; xi->logBBsize &= (uint64_t)-1 << (max(cfg->lsectorlog, 10) - BBSHIFT); +} - - if (!discard) - return; +static void +discard_devices( + struct libxfs_xinit *xi) +{ + /* + * This function has to be called after libxfs has been initialized. + */ if (!xi->disfile) discard_blocks(xi->ddev, xi->dsize); @@ -3901,7 +3905,7 @@ main( /* * Open and validate the device configurations */ - open_devices(&cfg, &xi, (discard && !dry_run)); + open_devices(&cfg, &xi); validate_datadev(&cfg, &cli); validate_logdev(&cfg, &cli, &logfile); validate_rtdev(&cfg, &cli, &rtfile); @@ -3952,6 +3956,12 @@ main( exit(0); } + /* + * All values have been validated, discard the old device layout. + */ + if (discard && !dry_run) + discard_devices(&xi); + /* * we need the libxfs buffer cache from here on in. */
Discard should happen only when everything has been validated, just before we start writing to the device. If it happens earlier, it is possible that mkfs will abort, but managed to already wipe data. This patch moves the discard to the latest possible moment. Signed-off-by: Jan Tulak <jtulak@redhat.com> --- CHANGES: v2 * discard_data -> discard_devices * move the discard condition outside of the function --- mkfs/xfs_mkfs.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)