feat(backup): make remote immutable #6928
Conversation
fbeauchamp
force-pushed
the
feat_immutable
branch
from
caf37eb to
3736909
Compare
fbeauchamp
changed the title
fbeauchamp
changed the title
fbeauchamp
force-pushed
the
feat_immutable
branch
from
3736909 to
7c9fd53
Compare
fbeauchamp
force-pushed
the
feat_immutable
branch
2 times, most recently
from
23b19ab to
9776785
Compare
fbeauchamp
force-pushed
the
feat_immutable
branch
7 times, most recently
from
87019e1 to
6d83b84
Compare
|
|
||
| ## Making a file immutable | ||
|
|
||
| when marking a file or a folder immutable, it create an alias file in `.immutable/<hash of full path>.<file|dir>.<extension>` containing the path to the folder/file and make this file also immutable. |
There was a problem hiding this comment.
IMHO we should add a directory level to prevent issues when lots of entries: hash[0]/hash[1].
It's pretty easy and can help if there are a lot of entries.
|
|
||
| ## Making a file immutable | ||
|
|
||
| when marking a file or a folder immutable, it create an alias file in `.immutable/<hash of full path>.<file|dir>.<extension>` containing the path to the folder/file and make this file also immutable. |
There was a problem hiding this comment.
The path format used for hashing must be properly documented: / vs \, relative vs absolute, leading or trailing slash, etc.
|
|
||
| ## Making a file immutable | ||
|
|
||
| when marking a file or a folder immutable, it create an alias file in `.immutable/<hash of full path>.<file|dir>.<extension>` containing the path to the folder/file and make this file also immutable. |
There was a problem hiding this comment.
I'm ill-at-ease with the fact that the content of these files will not be encrypted even if the rest of remote is.
What bothers me is the inconsistency.
Possible solutions:
- use symlinks instead
- put these files in a specific directory which will only contain unencrypted files.
There was a problem hiding this comment.
- symlinks can't be made immutable
chattr: Operation not supported while reading flags on source - I think we don't really need to have xo use the hidden index folder
|
|
||
| ## Configuring | ||
|
|
||
| this package uses app-conf to store its config. The application name id `immutable-backup` |
There was a problem hiding this comment.
IMHO, the application name should contain xo, example: xo-immutable-manager
fbeauchamp
force-pushed
the
feat_immutable
branch
from
40e38e6 to
052b93c
Compare
|
the merge process on vhd file rename files, it is correctly detected as a change event by chokidar, thus, not making he merged vhd immutable anew |
| } | ||
| await fs.writeFile(settingPath, | ||
| JSON.stringify({ | ||
| since: +new Date(), |
There was a problem hiding this comment.
| since: +new Date(), | |
| since: Date.now(), |
fbeauchamp
force-pushed
the
feat_immutable
branch
3 times, most recently
from
13f4af5 to
48dafaa
Compare
| async function handleExistingFile(root, immutabilityIndexPath, path) { | ||
| try { | ||
| // a vhd block directory is completly immutable | ||
| if (path.split('/').length === 8) { |
There was a problem hiding this comment.
should watch against dirname = .vhd
| // with awaitWriteFinish we have complete files here | ||
| // we can make them immutable | ||
|
|
||
| if (path.split('/').length === 8) { |
There was a problem hiding this comment.
should watch against dirname = .vhd
| if (path.split('/').length === 8) { | ||
| // watching a vhd block | ||
| // wait for header/footer and BAT before making this immutable recursively | ||
| const splitted = path.split('/') |
There was a problem hiding this comment.
path.split(nodePath.separator)
| const vmUuid = splitted[1] | ||
| const vdiUuid = splitted[4] | ||
| const uniqPath = `${vmUuid}/${vdiUuid}` | ||
| const { existing } = pendingVhds.get(uniqPath) |
There was a problem hiding this comment.
const { existing } = pendingVhds.get(uniqPath) ?? {}
| } else { | ||
| // already two of the key files,and we got the last one | ||
| if (existing === 2) { | ||
| Directory.makeImmutable(join(root, dirname(path))) |
There was a problem hiding this comment.
delete pendingvhds entry after
| await File.makeImmutable(testPath, immutabilityCachePath) | ||
| try { | ||
| await fs.writeFile(testPath, `test immut change 2 ${new Date()}`) | ||
| await fs.unlink(testPath) |
There was a problem hiding this comment.
unlink is not tested due to rejection from writeFile.
| } catch (error) { | ||
| if (error.code !== 'EPERM') { | ||
| throw error | ||
| } | ||
| } |
| for await (const { index, target } of listOlderTargets(immutabilityCachePath, immutabilityDuration)) { | ||
| await Directory.liftImmutability(target, immutabilityCachePath) | ||
| await fs.unlink(index) | ||
| if (target.match(/xo-vm-backups\/[^/]+\/[^/]+\.json$/)) { |
There was a problem hiding this comment.
extract this method
| const { remotes } = await loadConfig(APP_NAME, { | ||
| appDir: APP_DIR, | ||
| ignoreUnknownFormats: true, | ||
| }) |
| }) | ||
|
|
||
| for (const [remoteId, {indexPath,immutabilityDuration}] of Object.entries(remotes)) { | ||
| const basePath = indexPath ?? process.env.XDG_DATA_HOME ?? join('~', '.local', 'share') |
There was a problem hiding this comment.
I put the logic in the _loadConfig.mjs , my intention is that the config is either rejected or ready to use
| await fs.unlink(index) | ||
| if (target.match(/xo-vm-backups\/[^/]+\/[^/]+\.json$/)) { | ||
| // snipe vm metadata cache to force XO to update it | ||
| await fs.unlink(join(dirname(target), 'cache.json.gz')) |
@xen-orchestra/fs/src/abstract.js
Outdated
| get immutability(){ | ||
| return this.#immutability | ||
| } | ||
|
|
There was a problem hiding this comment.
IMHO, this should not be in fs, move into async RemoteAdapter.getImmutability() and it can have a cache.
@xen-orchestra/fs/src/local.js
Outdated
| return await this.#addSyncStackTrace(retry, () => fs.readFile(filePath, options), this.#retriesOnEagain) | ||
| const isPath = typeof file === 'string' | ||
| return await this.#addSyncStackTrace(retry, () => { | ||
| if(isPath){ |
There was a problem hiding this comment.
Prettier did not passed on this file.
| let fd | ||
| try { | ||
| // this will trigger e EPERM error if the file is immutable | ||
| fd = (await handler.openFile(path, 'r+')).fd |
There was a problem hiding this comment.
You don't need to do that, you can pass flags to readFile.
fbeauchamp
force-pushed
the
feat_immutable
branch
from
19e54c2 to
dc3462f
Compare
fbeauchamp
force-pushed
the
feat_immutable
branch
4 times, most recently
from
734136e to
8d16242
Compare
julien-f
force-pushed
the
feat_immutable
branch
3 times, most recently
from
0d9339b to
9494f64
Compare
julien-f
force-pushed
the
feat_immutable
branch
2 times, most recently
from
4147fc2 to
d0c7a8e
Compare
julien-f
force-pushed
the
feat_immutable
branch
from
d0c7a8e to
fff92a5
Compare
Description
in the doc folder of the new package
one watcher per vm should not overload the remote, even with a few thousands VMs saved ( default inotify allow 8192 watcher )
review by commit
Checklist
Fixes #007,See xoa-support#42,See https://...)Introduced byCHANGELOG.unreleased.md