Paul Eggert wrote: > I looked at the proposed fiemap patch for cp and see another issue > with it, which needs to be thought through more carefully. > > The proposed fiemap code tries to copy the _physical_ holes in the > source file. But currently cp copies the _logical_ holes: that is, it > skips over writing (and thus attempts to create physical holes) > whenever the source file contains a block of zeros, regardless of > whether that block of zeros is physically stored in the file system. > The current cp behavior is often the desired behavior (particularly > for cp --sparse=always) and the fiemap code shouldn't alter that. > > For example, if a fiemap_extent has the FIEMAP_EXTENT_UNWRITTEN flag > set, cp should treat that as a hole, because the extent is all zeros. > (This will greatly help performance in some cases.) Also, if an input > extent is read and a block of it is found to be zeros, cp should skip > over that block when writing. > > If the proposed fiemap code was intended to copy the physical holes, > then the code wasn't successful in implementing the intent, as there's > no guarantee that the extents of the destination are the same as the > extents of the source. If it is useful to have 'cp' copy the physical > holes, this could be supported via a new cp option or options, but > surely the default should continue to be to copy logical holes.
Good point! I think that copying physical holes via FIEMAP should be the default, when possible. One problem is that the current code on the fiemap-copy branch does not honor --sparse=WHEN when in fiemap-copying mode. The solution would seem to be to change the regular-file-copying loop in the fiemap_copy function to use the same hole-preserving code that is used in copy_reg.
