On Thu, Sep 05, 2013 at 11:28:20PM -0600, Andreas Dilger wrote:
> If the data is compressed, I think that just setting a flag in the
> fiemap_extent structure is not enough. It should also distinguish
> between the logical extent length and the physical extent length,
> by adding something like the fe_log_length field:
That's right, I've overlooked the spare 64bit fields.
> I'm a bit indecisive on whether the existing fe_length should be
> the physical length (which is what filefrag is reporting for the
> existing compressed files), or if it should be the logical length.
IMO it must be the logical length, otherwise it would break existing
applications (cp).
The relevant parts of the structure would look like this:
__u64 fe_length; /* logical length in bytes for this extent */
__u64 fe_phys_length; /* physical length in bytes (DATA_COMPRESSED) */
> For uncompressed files they would be the same, except fe_log_length
> is currently always zero. I don't think it makes sense to ever
> have an allocated extent with a logical length of zero, so tools
> could easily distinguish this case (without the DATA_COMPRESSED flag).
Looks like if fe_length contains the physical length it complicates
things unnecessarily.
I'd expect that fe_phys_length is valid iff an additional fe_flag bit is
set. That way we don't need to touch any other existing filesystem and
won't break userspace.
> Without this separation for compressed files, it isn't clear to
> the user if the file is sparse with holes all over it, even in the
> presence of the DATA_COMPRESSED flag. Also, it isn't clear for
> tools like tar or cp what parts of the file they should copy.
A hole is represented by a missing extent, right? Then I don't see how
the tools could get confused, the compression is completely transparent.
The main point of the flag is to enhance extent description, and with
the physical length available, to compute the compressed size of the
file. (*)
I'll spin V2 if the proposed change is ok.
david
(*)
This has been asked for for a long time, a separate ioctl patch
exists [1] but we did not completely agree that it's the right approach
and that some other interface should be used instead.
[1] http://repo.or.cz/w/linux-2.6/btrfs-unstable.git/commit/aa50490d57dd2035fc82f4070c13edba40926c69
Similar to FIEMAP, the ioctl goes through extents in a given file range
but calculates only the compressed (physical) size.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
