Re: [PATCH 1/2] btrfs-progs: add 'btrfs inspect-internal csum-dump' command

[Johannes Thumshirn <jthumshirn@suse.de>]

On Tue, Apr 09, 2019 at 05:34:22PM +0800, Su Yue wrote:
> 
> 
> On 2019/4/8 9:31 PM, Johannes Thumshirn wrote:
> > Add a 'btrfs inspect-internal csum-dump' command to dump the on-disk
> > checksums of a file.
> > 
> > The dump command first uses the FIEMAP ioctl() to get a map of the file's
> > extents and then uses the BTRFS_TREE_SEARCH_V2 ioctl() to get the
> > checksums for these extents.
> > 
> > Using FIEMAP instead of the BTRFS_TREE_SEARCH_V2 ioctl() to get the
> > extents allows us to quickly filter out any holes in the file, as this is
> > already done for us in the kernel.
> > 
> 
> Looks much better than V1. Some comments bellow.
> 

Thanks.

[...]

> > +	fiemap = calloc(1, sizeof(*fiemap));
> > +	if (!fiemap)
> > +		return -1;
> 
> -ENOMEM is better.

OK.

> > +
> > +	fiemap->fm_length = ~0;
> > +
> > +	ret = ioctl(fd, FS_IOC_FIEMAP, fiemap);
> > +	if (ret)
> > +		goto free_fiemap;
> > +
> > +	ext_size = fiemap->fm_mapped_extents * sizeof(struct fiemap_extent);
> > +
> > +	tmp = realloc(fiemap, sizeof(*fiemap) + ext_size);
> > +	if (!tmp)
> 
> ret = -ENOMEM;
> else btrfs_get_extent_csum() will return 0.
> 

Good catch, thanks

[...]

> > +	fd = open(filename, O_RDONLY);
> > +	if (fd < 0) {
> > +		error("cannot open file %s:%m\n", filename);
> 
> error() does break a line, no need of '\n'.
> > +		return 1;
> 
> -errno is better here.

OK.
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850