Re: [PATCH 15/19 v3] mkfs: don't treat files as though they are block devices

[Jan Tulak <jtulak@xxxxxxxxxx>]

On Fri, Apr 29, 2016 at 9:11 PM, Eric Sandeen <sandeen@xxxxxxxxxxx> wrote:

On 4/29/16 9:47 AM, Jan Tulak wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> If the device is actually a file, and "-d file" is not specified,
> mkfs will try to treat it as a block device and get stuff wrong.
> Image files don't necessarily have the same sector sizes as the
> block device or filesystem underlying the image file, nor should we
> be issuing discard ioctls on image files.
>
> To fix this sanely, only require "-d file" if the device name is
> invalid to trigger creation of the file. Otherwise, use stat() to
> determine if the device is a file or block device and deal with that
> appropriately by setting the "isfile" variables and turning off
> direct IO. Then ensure that we check the "isfile" options before
> doing things that are specific to block devices.
>
> Other file/blockdev issues fixed:
>Â Â Â Â- use getstr to detect specifying the data device name
>Â Â Â Â Âtwice.
>Â Â Â Â- check file/size/name parameters before anything else.
>Â Â Â Â- overwrite checks need to be done before the image file is
>Â Â Â Â Âopened and potentially truncated.
>Â Â Â Â- blkid_get_topology() should not be called for image files,
>Â Â Â Â Âso warn when it is called that way.
>Â Â Â Â- zero_old_xfs_structures() emits a spurious error:
>Â Â Â Â Â Â Â Â"existing superblock read failed: Success"
>Â Â Â Â Âwhen it is run on a truncated image file. Don't warn if we
>Â Â Â Â Âsee this problem on an image file.
>Â Â Â Â- Don't issue discards on image files.
>Â Â Â Â- Use fsync() for image files, not BLKFLSBUF in
>Â Â Â Â Âplatform_flush_device() for Linux.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> Signed-off-by: Jan Tulak <jtulak@xxxxxxxxxx>
>
> ---
> CHANGES:
> * read image file size in advance of O_TRUNC in case of dfile&&dcreat
> Signed-off-by: Jan Tulak <jtulak@xxxxxxxxxx>
> ---
> libxfs/init.c Â| 21 ++++++-
> libxfs/linux.c | 11 +++-
>Â mkfs/xfs_mkfs.c | 182 ++++++++++++++++++++++++++++++++++++++------------------
>Â 3 files changed, 154 insertions(+), 60 deletions(-)
>
> diff --git a/libxfs/init.c b/libxfs/init.c
> index 8d747e8..c7ae00d 100644
> --- a/libxfs/init.c
> +++ b/libxfs/init.c
> @@ -253,8 +253,15 @@ libxfs_init(libxfs_init_t *a)
>Â Â Â Ârtname = a->rtname;
>Â Â Â Âa->dfd = a->logfd = a->rtfd = -1;
>Â Â Â Âa->ddev = a->logdev = a->rtdev = 0;
> -Â Â Âa->dbsize = a->lbsize = a->rtbsize = 0;
> -Â Â Âa->dsize = a->logBBsize = a->logBBstart = a->rtsize = 0;
> +Â Â Âa->lbsize = a->rtbsize = 0;
> +Â Â Âa->logBBsize = a->logBBstart = a->rtsize = 0;
> +
> +Â Â Â// We can reset dbsize only when it is not a file, or we won't
> +Â Â Â// truncate it. Otherwise, we loose the size of the file forever.

please don't use c++ comments in xfsprogs, we use /* comments */

Sorry. â:-[ â

Â

...

>Â static void
> +check_device_type(
> +  Âconst char   *name,
> +  Âint      Â*isfile,
> +  Âbool      no_size,
> +  Âbool      no_name,
> +  Âint      Â*create,
> +  Âbool      force_overwrite,
> +  Âconst char   *optname)
> +{
> +Â Â Âstruct stat64 statbuf;
> +Â Â Â/*
> +Â Â Âif (*isfile && (no_size || no_name)) {
> +Â Â Â Â Â Â Âfprintf(stderr,
> +Â Â Â_("if -%s file then -%s name and -%s size are required\n"),
> +Â Â Â Â Â Â Â Â Â Â Âoptname, optname, optname);
> +Â Â Â Â Â Â Âusage();
> +Â Â Â}*/

What is this?

âAgain :-[

I commented it out during the development and then forgot to really delete it.â

Â

Jan, I'm just going to go back to the original patches posted in your V2 series,
and either give them Reviewed-by's, or send followup fix-up patches with a

Eric Sandeen <sandeen@xxxxxxxxxx>: fixed up foo, bar, baz

tag and a Reviewed-by to go with it, I think that might be the fastest path to
finally getting this stuff merged.

âOK, whatever you think is the best. :-)â

Thanks,

Jan

Â

Thanks,
-Eric

_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs

--

Jan Tulak

jtulak@xxxxxxxxxxÂ/ jan@xxxxxxxx