Pádraig Brady wrote: > On 16/06/10 07:45, Jim Meyering wrote: >> Paul Eggert wrote: >>> On 06/15/2010 02:43 PM, Jim Meyering wrote: >>>> 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. >>> >>> I assume that the solution would be used only with cp --sparse=always? >>> (Otherwise, it would amount to copying logical holes by default.) >> >> Right. >> I.e., use the same code that honors (or not) the "make_holes" variable. >> >>> If so, under this proposal, --sparse=always would copy logical holes, >>> --sparse=never would never copy holes, and --sparse=auto (the default) >>> would copy physical holes if supported, else it would copy logical >>> holes if the file seems to have enough physical holes, and otherwise >>> it would copy no holes. (Whew!) >> >> Right. With --sparse=always and fiemap support, it would take advantage >> of existing extents to minimize copying time, and for the nontrivial >> extents, it would detect/induce new holes when possible. >> >> Perhaps we need a new logical option that would make a difference >> only when there are nontrivial fiemap extents: >> - read nontrivial extents, as is done now >> - read them, *and* search-for/induce-holes as we do in legacy >> copy for --sparse=always. >> >> Or, putting it another way, perhaps we need a new command line >> option to control whether we even attempt a fiemap copy. >> IMHO the default should be to enable it, once all of the >> underlying bits are deemed to be stable enough. >> >> --fiemap >> --no-fiemap >> >> Then, --fiemap --sparse=never would do what the existing fiemap_copy >> function does, and --fiemap --sparse=always would work once the >> internal-to-fiemap_copy copying code is adjusted to use the >> hole-preserving code in copy_reg. >> >> Then, to guarantee the legacy --sparse=never behavior, >> one would have to specify --no-fiemap --sparse=never > > I would suggest not to add options like this, > and only do what's possible without new options > as it would push too many implementation details > to the docs/users IMHO.
Good point. Smaller/simpler would be better. It'd be great if we can eliminate as never-useful one of the new combinations. Currently on the fiemap-copy branch, once we get a single successful ioctl, the code will not bother with the usual hole-detecting/introducing technique implied by --sparse=always. Should it do that ever? Always? Does this need an option?
