On Wed, Sep 19, 2018 at 02:56:16PM +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@xxxxxxxxxx>
> ---
> mkfs/xfs_mkfs.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 2e53c1e8..81d9859a 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,8 +2418,16 @@ open_devices(
> xi->dsize &= sector_mask;
> xi->rtsize &= sector_mask;
> xi->logBBsize &= (uint64_t)-1 << (max(cfg->lsectorlog, 10) - BBSHIFT);
> +}
>
> -
> +static void
> +discard_data(
Perhaps discard_devices(), since this function can DISCARD more than
just the data device.
> + struct libxfs_xinit *xi,
> + bool discard)
> +{
> + /*
> + * This function has to be called after libxfs has been initialized.
> + */
> if (!discard)
> return;
>
While we're on the topic, I notice that we skip discard for any device
that's actually a file. Seeing as fallocate(PUNCH_HOLE) works on files
(and block devices), is there a reason why we avoid punching out fs
image files?
> @@ -3901,7 +3908,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 +3959,11 @@ main(
> exit(0);
> }
>
> + /*
> + * All values have been validated, discard the old device layout.
> + */
> + discard_data(&xi, (discard && !dry_run));
if (discard && !dry_run)
discard_devices(&xi);
?
--D
> +
> /*
> * we need the libxfs buffer cache from here on in.
> */
> --
> 2.18.0
>
