xfs
[Top] [All Lists]

Re: [PATCH v3 1/2] xfstests: add fssum tool

To: Jan Schmidt <list.xfs@xxxxxxxxxxxxx>
Subject: Re: [PATCH v3 1/2] xfstests: add fssum tool
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Thu, 08 Aug 2013 12:40:55 -0500
Cc: xfs@xxxxxxxxxxx, sbehrens@xxxxxxxxxxxxxxxx, linux-btrfs@xxxxxxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1375949833-1104-2-git-send-email-list.xfs@xxxxxxxxxxxxx>
References: <1375949833-1104-1-git-send-email-list.xfs@xxxxxxxxxxxxx> <1375949833-1104-2-git-send-email-list.xfs@xxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) Gecko/20130801 Thunderbird/17.0.8
On 8/8/13 3:17 AM, Jan Schmidt wrote:
> fssum is a tool to build a recursive checksum for a file system. The home
> repository of fssum is
> 
>     git://git.kernel.org/pub/scm/linux/kernel/git/arne/far-progs.git
> 
> It is added as an optional target, because it depends on glibc >= 2.15 for
> SEEK_HOLE / SEEK_DATA. The test to be added using fssum will just be skipped
> if fssum wasn't built.
> 
> Signed-off-by: Jan Schmidt <list.xfs@xxxxxxxxxxxxx>
> ---
>  .gitignore    |    1 +
>  common/config |    2 +
>  src/Makefile  |   11 +-
>  src/fssum.c   |  819 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 832 insertions(+), 1 deletions(-)
>  create mode 100644 src/fssum.c
> 
> diff --git a/.gitignore b/.gitignore
> index 11594aa..c2fc6e3 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -45,6 +45,7 @@
>  /src/fill
>  /src/fill2
>  /src/fs_perms
> +/src/fssum
>  /src/fstest
>  /src/fsync-tester
>  /src/ftrunc
> diff --git a/common/config b/common/config
> index 67c1498..c8bee29 100644
> --- a/common/config
> +++ b/common/config
> @@ -146,6 +146,8 @@ export SED_PROG="`set_prog_path sed`"
>  export BC_PROG="`set_prog_path bc`"
>  [ "$BC_PROG" = "" ] && _fatal "bc not found"
>  
> +export FSSUM_PROG="`set_prog_path fssum $here/src/fssum`"

So this will pick up a local copy of fssum if it exists;
is that really desired?  (If there's any difference in
behavior, then the one in src/ presumably would need to
be fixed...)

> +
>  export PS_ALL_FLAGS="-ef"
>  
>  export DF_PROG="`set_prog_path df`"
> diff --git a/src/Makefile b/src/Makefile
> index cc679e8..10a4d3c 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -20,10 +20,14 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize 
> preallo_rw_pattern_reader \
>       stale_handle pwrite_mmap_blocked t_dir_offset2 seek_sanity_test \
>       seek_copy_test t_readdir_1 t_readdir_2 fsync-tester
>  
> +OPT_TARGETS = fssum
> +

I'm not sure how this helps . . .

>  SUBDIRS =
>  
>  LLDLIBS = $(LIBATTR) $(LIBHANDLE) $(LIBACL)
>  
> +OPT_LDLIBS = -lssl -lcrypto

Hm, new deps.  I guess it's not a huge problem, these should always
be available, right?

>  ifeq ($(HAVE_XLOG_ASSIGN_LSN), true)
>  LINUX_TARGETS += loggen
>  endif
> @@ -60,7 +64,7 @@ CFILES = $(TARGETS:=.c)
>  LDIRT = $(TARGETS)
>  
>  
> -default: depend $(TARGETS) $(SUBDIRS)
> +default: depend $(TARGETS) $(OPT_TARGETS) $(SUBDIRS)

Anyway, OPT_TARGETS isn't optional, because you still build it by default.  :)

>  depend: .dep
>  
> @@ -70,11 +74,16 @@ $(TARGETS): $(LIBTEST)
>       @echo "    [CC]    $@"
>       $(Q)$(LTLINK) $@.c -o $@ $(CFLAGS) $(LDFLAGS) $(LDLIBS) $(LIBTEST)
>  
> +$(OPT_TARGETS): $(LIBTEST)
> +     @echo "    [CC]    $@"
> +     -$(Q)$(LTLINK) $@.c -o $@ $(CFLAGS) $(LDFLAGS) $(LDLIBS) $(OPT_LDLIBS) 
> $(LIBTEST)

Oh, I see, you ignore the error.  Well, that's still pretty ugly.
I'd really rather you just add the #defines as I suggested in my
reply to [PATCH 0/2], so it'll build for everyone.

Thanks,
-Eric

> +
>  LINKTEST = $(LTLINK) $@.c -o $@ $(CFLAGS) $(LDFLAGS)
>  
>  install: default $(addsuffix -install,$(SUBDIRS))
>       $(INSTALL) -m 755 -d $(PKG_LIB_DIR)/src
>       $(LTINSTALL) -m 755 $(TARGETS) $(PKG_LIB_DIR)/src
> +     -$(LTINSTALL) -m 755 $(OPT_TARGETS) $(PKG_LIB_DIR)/src
>       $(LTINSTALL) -m 755 fill2attr fill2fs fill2fs_check scaleread.sh 
> $(PKG_LIB_DIR)/src
>       $(LTINSTALL) -m 644 dumpfile $(PKG_LIB_DIR)/src
>  

<Prev in Thread] Current Thread [Next in Thread>