From a5709dcc2203d83132840d44fb0aed813a8f0f07 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Tue, 28 Mar 2023 13:06:11 -0400 Subject: [PATCH] Copy with a reflink when exporting a tree to a directory special remote Remote.Directory makes a temp file, then calls this, and since the temp file exists, it prevented probing if CoW works. Note that deleting the empty file does mean there's a small window for a race. If another process is also exporting to the remote, that could let it make the same temp file. However, the temp filename actually has the processes's pid in it, which avoids that being a problem. This may have been a reversion caused by commits around 63d508e8855b2e61a725c906ea17d1c7f4a2e125, but I haven't gone back and tested to be sure. The directory special remote had supposedly supported CoW for this going back to about half a year before that. Sponsored-by: Graham Spencer on Patreon --- Annex/CopyFile.hs | 24 +++++++++++++++---- CHANGELOG | 1 + ...to_export_to_directory_special_remote.mdwn | 2 ++ 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/Annex/CopyFile.hs b/Annex/CopyFile.hs index 9fc8eafc53..0be9debd5f 100644 --- a/Annex/CopyFile.hs +++ b/Annex/CopyFile.hs @@ -31,17 +31,15 @@ newCopyCoWTried = CopyCoWTried <$> newEmptyMVar {- Copies a file is copy-on-write is supported. Otherwise, returns False. - - - The destination file must not exist yet, or it will fail to make a CoW copy, - - and will return false. + - The destination file must not exist yet (or may exist but be empty), + - or it will fail to make a CoW copy, and will return false. -} tryCopyCoW :: CopyCoWTried -> FilePath -> FilePath -> MeterUpdate -> IO Bool tryCopyCoW (CopyCoWTried copycowtried) src dest meterupdate = -- If multiple threads reach this at the same time, they -- will both try CoW, which is acceptable. ifM (isEmptyMVar copycowtried) - -- If dest exists, don't try CoW, since it would - -- have to be deleted first. - ( ifM (doesFileExist dest) + ( ifM destfilealreadypopulated ( return False , do ok <- docopycow @@ -61,6 +59,22 @@ tryCopyCoW (CopyCoWTried copycowtried) src dest meterupdate = where docopycow = watchFileSize dest meterupdate $ copyCoW CopyTimeStamps src dest + + dest' = toRawFilePath dest + + -- Check if the dest file already exists, which would prevent + -- probing CoW. If the file exists but is empty, there's no benefit + -- to resuming from it when CoW does not work, so remove it. + destfilealreadypopulated = + tryIO (R.getFileStatus dest') >>= \case + Left _ -> return False + Right st -> do + sz <- getFileSize' dest' st + if sz == 0 + then tryIO (removeFile dest) >>= \case + Right () -> return False + Left _ -> return True + else return True data CopyMethod = CopiedCoW | Copied diff --git a/CHANGELOG b/CHANGELOG index bc5022724c..afa1699777 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -9,6 +9,7 @@ git-annex (10.20230322) UNRELEASED; urgency=medium the view branch. * Windows: Support urls like "file:///c:/path" * addurl, importfeed: Fix failure when annex.securehashesonly is set. + * Copy with a reflink when exporting a tree to a directory special remote. -- Joey Hess Thu, 23 Mar 2023 15:04:41 -0400 diff --git a/doc/bugs/use_reflink_to_export_to_directory_special_remote.mdwn b/doc/bugs/use_reflink_to_export_to_directory_special_remote.mdwn index 759c751235..e716fceb22 100644 --- a/doc/bugs/use_reflink_to_export_to_directory_special_remote.mdwn +++ b/doc/bugs/use_reflink_to_export_to_directory_special_remote.mdwn @@ -40,3 +40,5 @@ After running, a `btrfs filesystem du ...` tells me that the `repo` and `import` --- In general: Thank you so much for this wonderful piece of software, I'm using it for years now and manage virtually everything with it (audio, video, pictures, important documents, papers, …). + +> [[fixed|done]] --[[Joey]] -- 2.30.2