On 14/07/10 07:58, jeff.liu wrote: > Pádraig Brady wrote: > > Hello All, > > I am very sorry for the late response! I have an urgent task need to deliver > in the past few weeks. > > Thanks for all your suggestions, I would like to improve the fiemap-copy > accordingly, so does the > following sentence is the final decision? > >> I don't see the need for new options TBH >> I see fiemap just as a way to efficiently detect/read holes, >> and should have no bearing on the destination. >> >> cp --sparse=auto (this is currently what cp does by default) >> recreate the original fiemap holes or resort to existing >> heuristic if fiemap not available >> cp --sparse=never >> write all data, but use fiemap if available to efficiently read >> cp --sparse=always >> recreate original holes and perhaps extend add to them for >> other runs of zero bytes. Without having looked at the code >> I see this as a little tricky to mix with fiemap. >> Now since fiemap is only an optimization we can skip it >> completely for this uncommon case if too tricky (just add a FIXME for now).
Joel, overlapped with my response and concurred. > Current code handle 'cp --sparse=auto' and 'cp --sparse=always' in same way > since these two options > all setup 'make_holes' to True, though '--sparse=auto' use a heuristic to > determine whether SRC_NAME > contains any sparse blocks. > > For 'cp --sparse=never', when detected holes from SRC file, do not lseek(2) > against DST file, > instead, write ZEROs to DST file, Am I right? right > > If so, IMHO, we can pass 'sparse_mode' to fiemap_copy(), then decide how to > do operation against DST > file according to its mode. > > In addition to this, I'd like to fix another issue in current code as Paul > suggested, > 1. Do not allocate buffer in fiemap_copy() for data pass between SRC and DST > files, instead, take > advantage of the buffer allocated outside. good > 2. Performance optimization, invoke fallocate(2) if an extent flag is > UNWRITTEN > For this case, maybe we have to wait until fallocate interface become stable > just as Pádraig > methoned before: > http://lists.gnu.org/archive/html/bug-coreutils/2009-05/msg00260.html If you decide to do that, then please do it as a separate patch. cheers, Pádraig.
