diff --git a/lib/Backend/StorageBackend.php b/lib/Backend/StorageBackend.php index 7972c74..7126f15 100644 --- a/lib/Backend/StorageBackend.php +++ b/lib/Backend/StorageBackend.php @@ -412,7 +412,8 @@ final class StorageBackend implements IBackend, IReadsAhead { } } } catch (\Throwable) { - // read the usual way below, which reports what is wrong + // read the usual way below, which reports what is wrong, over a new connection + $this->smbReconnect(); } $fh = $this->get($path); try { @@ -555,7 +556,8 @@ final class StorageBackend implements IBackend, IReadsAhead { try { $state->lseek($fh, $offset, SEEK_SET, $url); for ($got = 0; $got < $length;) { - $chunk = (string)$state->read($fh, min(self::SMB_READ, $length - $got), $url); + $want = min(self::SMB_READ, $length - $got); + $chunk = self::smbRead($state, $fh, $want, $url); if ($chunk === '') { throw new BackendException('Short read in ' . $url); } @@ -590,22 +592,67 @@ final class StorageBackend implements IBackend, IReadsAhead { } catch (\Throwable) { return null; } + $data = ''; try { - $data = ''; do { - /** @psalm-suppress MixedMethodCall */ - $chunk = (string)$state->read($fh, self::SMB_READ, $url); + $chunk = self::smbRead($state, $fh, self::SMB_READ, $url); $data .= $chunk; } while ($chunk !== ''); - return $data; } catch (\Throwable) { - return null; - } finally { - try { - /** @psalm-suppress MixedMethodCall */ - $state->close($fh, $url); - } catch (\Throwable) { - } + $data = null; + } + try { + /** @psalm-suppress MixedMethodCall */ + $state->close($fh, $url); + } catch (\Throwable) { + } + if ($data === null) { + $this->smbReconnect(); // a read failed: the usual way reads it over a new connection + } + return $data; + } + + /** + * One read of at most $bytes through the native state, checked. When libsmbclient fails a read + * (seen when the connection to the server dropped during a long restore), the smbclient + * extension does not return false but a string whose length is -1, because it stores + * smbc_read()'s -1 in an unsigned length; icewind passes it on. Counting that as data made + * the read loop go backwards forever at 100% CPU (DR test 2026-11-05, after 392 GiB). + * + * @param resource $fh + * @psalm-suppress MixedMethodCall, MixedArgument the state is icewind's NativeState + */ + private static function smbRead(object $state, $fh, int $bytes, string $url): string { + $chunk = $state->read($fh, $bytes, $url); + $n = is_string($chunk) ? strlen($chunk) : -1; + if ($n < 0 || $n > $bytes) { + throw new BackendException('Read failed in ' . $url); + } + return $chunk; + } + + /** + * After a failed read: drop icewind's connection state, so the next read (the usual way, or + * the next range) connects again instead of failing on a connection that is gone. + * + * @psalm-suppress UndefinedClass icewind/smb comes with files_external, not with this app + */ + private function smbReconnect(): void { + if (!$this->storage instanceof \OCA\Files_External\Lib\Storage\SMB) { + return; + } + try { + (function (): void { + /** @psalm-suppress UndefinedThisPropertyFetch */ + $share = $this->share; + if ($share instanceof \Icewind\SMB\Native\NativeShare) { + (function (): void { + /** @psalm-suppress UndefinedThisPropertyAssignment */ + $this->state = null; + })->call($share); + } + })->call($this->storage); + } catch (\Throwable) { } } @@ -657,12 +704,15 @@ final class StorageBackend implements IBackend, IReadsAhead { try { foreach ($abs as $i => [$path, $offset, $length]) { $fh = null; + $out = false; try { $chunks = $this->smbChunks($path, $offset, $length); $out = $chunks === null ? false : fopen($file, 'w+b'); if ($chunks !== null && $out !== false) { foreach ($chunks as $chunk) { - fwrite($out, $chunk); + if (fwrite($out, $chunk) !== strlen($chunk)) { + throw new BackendException('Cannot write to the spool'); + } } $fh = ftell($out) === $length && rewind($out) ? $out : null; if ($fh === null) { @@ -670,7 +720,11 @@ final class StorageBackend implements IBackend, IReadsAhead { } } } catch (\Throwable) { - $fh = null; // read the usual way, which reports why + if (is_resource($out)) { + fclose($out); + } + $fh = null; // read the usual way, which reports why, over a new connection + $this->smbReconnect(); } finally { @unlink($file); // an open stream keeps its data } diff --git a/lib/Repository/Repository.php b/lib/Repository/Repository.php index f2c81d9..954e56e 100644 --- a/lib/Repository/Repository.php +++ b/lib/Repository/Repository.php @@ -224,14 +224,18 @@ final class Repository { * @param array|null $skipped salvage: an array to collect the files that could * not be read or decrypted (path => reason), which are then left out instead of * stopping the restore; null (default) stops at the first such file + * @param bool $keepUnchanged leave a file that is already in place with the size and + * modification time of the snapshot, as a backup takes such a file as unchanged: + * a restore that was stopped and runs again goes on where it was, without reading + * what it already restored from the location again */ - public function restore(string $snapshotId, string $target, string $prefix = '', bool $stripPrefix = false, ?callable $heartbeat = null, ?array &$skipped = null): int { + public function restore(string $snapshotId, string $target, string $prefix = '', bool $stripPrefix = false, ?callable $heartbeat = null, ?array &$skipped = null, bool $keepUnchanged = false): int { if ($prefix !== '' && !self::isSafePath(rtrim($prefix, '/'))) { throw new RepositoryException('Invalid path prefix'); } - $plan = $this->readAhead($snapshotId, $prefix, $target); + $plan = $this->readAhead($snapshotId, $prefix, $target, $stripPrefix, $keepUnchanged); try { - return $this->restoreEntries($snapshotId, $target, $prefix, $stripPrefix, $heartbeat, $plan, $skipped); + return $this->restoreEntries($snapshotId, $target, $prefix, $stripPrefix, $heartbeat, $plan, $skipped, $keepUnchanged); } finally { $plan?->close(); if ($plan !== null) { @@ -245,7 +249,7 @@ final class Repository { * pass over the snapshot lists the ranges of packs it needs, in order. The spool is a * folder in the target, which has room for the restore anyway. */ - private function readAhead(string $snapshotId, string $prefix, string $target): ?ReadAheadPlan { + private function readAhead(string $snapshotId, string $prefix, string $target, bool $stripPrefix, bool $keepUnchanged): ?ReadAheadPlan { if (!$this->backend instanceof IReadsAhead || !$this->backend->canReadAhead()) { return null; } @@ -260,6 +264,9 @@ final class Repository { if (!self::underPrefix($entry['p'], $prefix)) { continue; } + if ($keepUnchanged && self::inPlace(self::destination($target, $entry['p'], $prefix, $stripPrefix), $entry)) { + continue; + } foreach ($entry['b'] as $id) { try { [$pack, $offset, $length] = $this->index->get($id); @@ -289,18 +296,19 @@ final class Repository { } /** @param array|null $skipped see restore() */ - private function restoreEntries(string $snapshotId, string $target, string $prefix, bool $stripPrefix, ?callable $heartbeat, ?ReadAheadPlan $plan, ?array &$skipped): int { + private function restoreEntries(string $snapshotId, string $target, string $prefix, bool $stripPrefix, ?callable $heartbeat, ?ReadAheadPlan $plan, ?array &$skipped, bool $keepUnchanged): int { $count = 0; $cleaned = []; foreach ($this->streamEntries($snapshotId) as $entry) { if (!self::underPrefix($entry['p'], $prefix)) { continue; } - $relPath = $entry['p']; - if ($stripPrefix && $prefix !== '') { - $relPath = $entry['p'] === rtrim($prefix, '/') ? basename($entry['p']) : substr($entry['p'], strlen($prefix)); + $dest = self::destination($target, $entry['p'], $prefix, $stripPrefix); + if ($keepUnchanged && self::inPlace($dest, $entry)) { + // below the target as written by an earlier run of this restore (checked then) + $count++; + continue; } - $dest = rtrim($target, '/') . '/' . $relPath; if (!is_dir(dirname($dest))) { mkdir(dirname($dest), 0700, true); } @@ -358,6 +366,26 @@ final class Repository { return $count; } + /** Where restore() writes the snapshot path $path. */ + private static function destination(string $target, string $path, string $prefix, bool $stripPrefix): string { + $relPath = $path; + if ($stripPrefix && $prefix !== '') { + $relPath = $path === rtrim($prefix, '/') ? basename($path) : substr($path, strlen($prefix)); + } + return rtrim($target, '/') . '/' . $relPath; + } + + /** + * A regular file (no link) at $dest with the size and modification time of the snapshot + * entry: what a backup takes as unchanged, and what a finished restore of it leaves. + * + * @param array{s: int, m: int, ...} $entry + */ + private static function inPlace(string $dest, array $entry): bool { + clearstatcache(true, $dest); + return !is_link($dest) && is_file($dest) && filesize($dest) === $entry['s'] && filemtime($dest) === $entry['m']; + } + /** $prefix is "" (everything) or ends with "/"; the path without that "/" is a file to restore too. */ private static function underPrefix(string $path, string $prefix): bool { return $prefix === '' || str_starts_with($path, $prefix) || $path === rtrim($prefix, '/'); diff --git a/lib/Service/RestoreFullService.php b/lib/Service/RestoreFullService.php index 0aa3dd0..1749aa9 100644 --- a/lib/Service/RestoreFullService.php +++ b/lib/Service/RestoreFullService.php @@ -186,7 +186,7 @@ final class RestoreFullService { } if ($dbError !== null) { $salvageDir = $dataDir . '/ngb-salvage-' . substr($snapshotId, 0, 12); - $dataFiles = $this->restore->restoreToDirectory($target, $snapshotId, 'data', $salvageDir . '/data', stripPrefix: true, heartbeat: $beat, skipped: $skipped); + $dataFiles = $this->restore->restoreToDirectory($target, $snapshotId, 'data', $salvageDir . '/data', stripPrefix: true, heartbeat: $beat, skipped: $skipped, keepUnchanged: true); $this->restore->restoreToDirectory($target, $snapshotId, 'config', $salvageDir . '/config', stripPrefix: true, heartbeat: $beat, skipped: $skipped); return ['dbReport' => ['skipped' => [], 'restored' => [], 'created' => [], 'missing' => []], 'dataFiles' => $dataFiles, 'configKeys' => 0, 'dbConsistent' => true, 'missingApps' => $check['missingApps'], 'skipped' => $skipped ?? [], 'dbError' => $dbError, @@ -209,7 +209,7 @@ final class RestoreFullService { $backupConfig = []; } - $dataFiles = $this->restore->restoreToDirectory($target, $snapshotId, 'data', $dataDir, stripPrefix: true, heartbeat: $beat, skipped: $skipped); + $dataFiles = $this->restore->restoreToDirectory($target, $snapshotId, 'data', $dataDir, stripPrefix: true, heartbeat: $beat, skipped: $skipped, keepUnchanged: true); // Last step: nothing after this may need $this->targets->repository($target) again. if ($backupConfig !== []) { diff --git a/lib/Service/RestoreService.php b/lib/Service/RestoreService.php index 0bc238c..8df6e0a 100644 --- a/lib/Service/RestoreService.php +++ b/lib/Service/RestoreService.php @@ -169,15 +169,18 @@ final class RestoreService { * @param bool $stripPrefix write into $directory itself instead of $directory/$prefix (see * Repository::restore()); used to restore "data/" or "config/" straight into the * real data/config directory for occ backup:restore:full. + * @param bool $keepUnchanged leave files already in place with the snapshot's size and + * modification time (see Repository::restore()): a full restore that runs again + * goes on where it stopped */ - public function restoreToDirectory(Target $target, string $snapshotId, string $prefix, string $directory, bool $stripPrefix = false, ?callable $heartbeat = null, ?array &$skipped = null): int { + public function restoreToDirectory(Target $target, string $snapshotId, string $prefix, string $directory, bool $stripPrefix = false, ?callable $heartbeat = null, ?array &$skipped = null, bool $keepUnchanged = false): int { if ($prefix !== '' && !Repository::isSafePath($prefix)) { throw new \InvalidArgumentException('Invalid path: ' . $prefix); } if (!is_dir($directory) && !mkdir($directory, 0750, true) && !is_dir($directory)) { throw new \RuntimeException("Cannot create $directory"); } - return $this->withSharedLock($target, function (callable $refresh) use ($target, $snapshotId, $prefix, $directory, $stripPrefix, $heartbeat, &$skipped): int { + return $this->withSharedLock($target, function (callable $refresh) use ($target, $snapshotId, $prefix, $directory, $stripPrefix, $heartbeat, &$skipped, $keepUnchanged): int { $own = LeaseService::throttled($refresh); $beat = static function () use ($own, $heartbeat): void { $own(); @@ -185,7 +188,7 @@ final class RestoreService { $heartbeat(); } }; - return $this->targets->repository($target)->restore($snapshotId, $directory, trim($prefix, '/') === '' ? '' : trim($prefix, '/') . '/', $stripPrefix, $beat, $skipped); + return $this->targets->repository($target)->restore($snapshotId, $directory, trim($prefix, '/') === '' ? '' : trim($prefix, '/') . '/', $stripPrefix, $beat, $skipped, $keepUnchanged); }, 3600); } diff --git a/tests/Unit/Repository/RepositoryTest.php b/tests/Unit/Repository/RepositoryTest.php index 27a5241..c94c8fe 100644 --- a/tests/Unit/Repository/RepositoryTest.php +++ b/tests/Unit/Repository/RepositoryTest.php @@ -101,6 +101,34 @@ class RepositoryTest extends TestCase { $this->assertFileExists($stripped . '/note-1.txt'); } + public function testARestoreThatRunsAgainKeepsWhatIsInPlaceAndRestoresTheRest(): void { + $location = $this->tempDir(); + $backend = new LocalBackend($location); + $repo = Repository::init($backend, 'pw'); + $r = $repo->backupDirectory($this->src); + $target = $this->tempDir(); + $repo->restore($r['snapshot'], $target); + + // as a stopped restore leaves it: one file not there yet, one with other content + unlink($target . '/docs/d1/note-1.txt'); + file_put_contents($target . '/docs/d0/note-0.txt', 'something else'); + $this->assertSame(34, $repo->restore($r['snapshot'], $target, keepUnchanged: true)); + $this->assertSame(self::hashTree($this->src), self::hashTree($target)); + + // all in place: nothing is read from the location, which could not give it anyway now + self::removeTree($location . '/packs'); + $this->assertSame(34, Repository::open($backend, 'pw')->restore($r['snapshot'], $target, keepUnchanged: true)); + $this->expectException(\Throwable::class); + Repository::open($backend, 'pw')->restore($r['snapshot'], $target); + } + + private static function removeTree(string $dir): void { + foreach (new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($dir, \FilesystemIterator::SKIP_DOTS), \RecursiveIteratorIterator::CHILD_FIRST) as $f) { + $f->isDir() ? rmdir($f->getPathname()) : unlink($f->getPathname()); + } + rmdir($dir); + } + public function testRestoreRemovesTemporariesOfAStoppedRestore(): void { $repo = Repository::init($this->backend, 'pw'); $r = $repo->backupDirectory($this->src);