diff --git a/packages/os/overlay/opt/umbrel-data/umbrel-data-mount b/packages/os/overlay/opt/umbrel-data/umbrel-data-mount index 34466d0b3..c3a2b2491 100755 --- a/packages/os/overlay/opt/umbrel-data/umbrel-data-mount +++ b/packages/os/overlay/opt/umbrel-data/umbrel-data-mount @@ -63,10 +63,21 @@ handle_failsafe_transition() { # Create final snapshot and sync incrementally # Using --raw to preserve encryption (sends encrypted blocks without needing key loaded) echo ">>> Creating final snapshot" + # Destroy any stale snapshot left over from a previous failed attempt so this + # is idempotent on retry and doesn't depend on errexit being suppressed here. + zfs destroy -r "${pool_name}@migration-final" 2>/dev/null || true zfs snapshot -r "${pool_name}@migration-final" echo ">>> Sending incremental changes to migration pool" - zfs send --raw --replicate --large-block --compressed -i @migration "${pool_name}@migration-final" | zfs receive -Fu "$migration_pool" + # If the final sync fails, don't rename the pools. Renaming would promote the + # migration pool (missing this last delta) and umbreld would then destroy the + # original pool, losing data. Abort instead so we boot back into the original pool. + if ! zfs send --raw --replicate --large-block --compressed -i @migration "${pool_name}@migration-final" | zfs receive -Fu "$migration_pool"; then + echo ">>> ERROR: Final sync failed, aborting migration and booting back into the original pool" + zpool export "$migration_pool" || true + zpool export "$pool_name" || true + return 1 + fi # Rename pools # umbrelos > umbrelos-previous-migration diff --git a/packages/umbreld/source/modules/hardware/raid.ts b/packages/umbreld/source/modules/hardware/raid.ts index 62d117e6c..795e0b303 100644 --- a/packages/umbreld/source/modules/hardware/raid.ts +++ b/packages/umbreld/source/modules/hardware/raid.ts @@ -1761,6 +1761,14 @@ export default class Raid { const pool = await this.getStatus() const previousPoolName = `${pool.name}-previous-migration` + // Make sure the new main pool actually came up before we destroy anything. + // If the boot script aborted the rename, the main pool won't exist and we + // must leave the previous pool intact instead of wiping our only copy. + if (!pool.exists) { + this.logger.error('Config indicates transition in progress but main pool not found, leaving previous pool intact') + return + } + // Verify the previous pool exists (should always exist if config says transitioning) const previousPool = await this.getPoolStatus(previousPoolName) if (!previousPool.exists) { diff --git a/packages/umbreld/source/modules/startup-migrations/index.ts b/packages/umbreld/source/modules/startup-migrations/index.ts index 22890f484..32b22b7bd 100644 --- a/packages/umbreld/source/modules/startup-migrations/index.ts +++ b/packages/umbreld/source/modules/startup-migrations/index.ts @@ -88,8 +88,21 @@ class Migration { async activateImportedDataDirectory() { const importData = `${this.umbreld.dataDirectory}/import` + const temporaryData = `${this.umbreld.dataDirectory}-import-temp` const importDataExists = await fse.exists(importData) - if (!importDataExists) return + + // Resume an interrupted activation. If we already moved import out to the temp + // dir but got killed before moving it into place, finish that second move now. + // Without this the imported data would be orphaned in the temp dir forever and + // we'd boot with the un-imported data. + if (!importDataExists) { + if (await fse.exists(temporaryData)) { + this.logger.log('Resuming interrupted data import...') + await fse.move(temporaryData, this.umbreld.dataDirectory, {overwrite: true}) + } + return + } + this.logger.log('Found Umbrel data to import, activating...') // We have to move the import dir parrallel to the data dir and then overwrte. // This is because fse.move doesn't work if the source is a subdirectory of the destination. @@ -97,7 +110,6 @@ class Migration { // On Rasperry Pi the data partition is small on the SD card and only the data dir on the // large external USB storage. We don't currently support data import on Pi so it's ok for now // but we'll need to handle this if we want to support it in the future. - const temporaryData = `${this.umbreld.dataDirectory}-import-temp` await fse.move(importData, temporaryData, {overwrite: true}) await fse.move(temporaryData, this.umbreld.dataDirectory, {overwrite: true}) }