refactor(backups/checkBaseVdi): use uuid, don't check vhd multiple times

This commit is contained in:
Florent Beauchamp 2024-02-16 16:05:24 +00:00 committed by Julien Fontanet
parent 56b427c09c
commit c7b5b715a3
4 changed files with 30 additions and 25 deletions

View File

@ -191,13 +191,14 @@ export class RemoteAdapter {
// check if we will be allowed to merge a a vhd created in this adapter // check if we will be allowed to merge a a vhd created in this adapter
// with the vhd at path `path` // with the vhd at path `path`
async isMergeableParent(packedParentUid, path) { async isMergeableParent(packedParentUid, path) {
return await Disposable.use(openVhd(this.handler, path), vhd => { return await Disposable.use(VhdSynthetic.fromVhdChain(this.handler, path), vhd => {
// this baseUuid is not linked with this vhd // this baseUuid is not linked with this vhd
if (!vhd.footer.uuid.equals(packedParentUid)) { if (!vhd.footer.uuid.equals(packedParentUid)) {
return false return false
} }
const isVhdDirectory = vhd instanceof VhdDirectory // check if all the chain is composed of vhd directory
const isVhdDirectory = vhd.checkVhdsClass(VhdDirectory)
return isVhdDirectory return isVhdDirectory
? this.useVhdDirectory() && this.#getCompressionType() === vhd.compressionType ? this.useVhdDirectory() && this.#getCompressionType() === vhd.compressionType
: !this.useVhdDirectory() : !this.useVhdDirectory()

View File

@ -133,7 +133,7 @@ export const IncrementalXapi = class IncrementalXapiVmBackupRunner extends Abstr
]) ])
const srcVdi = srcVdis[snapshotOf] const srcVdi = srcVdis[snapshotOf]
if (srcVdi !== undefined) { if (srcVdi !== undefined) {
baseUuidToSrcVdi.set(baseUuid, srcVdi) baseUuidToSrcVdi.set(baseUuid, srcVdi.uuid)
} else { } else {
debug('ignore snapshot VDI because no longer present on VM', { debug('ignore snapshot VDI because no longer present on VM', {
vdi: baseUuid, vdi: baseUuid,
@ -154,18 +154,18 @@ export const IncrementalXapi = class IncrementalXapiVmBackupRunner extends Abstr
} }
const fullVdisRequired = new Set() const fullVdisRequired = new Set()
baseUuidToSrcVdi.forEach((srcVdi, baseUuid) => { baseUuidToSrcVdi.forEach((srcVdiUuid, baseUuid) => {
if (presentBaseVdis.has(baseUuid)) { if (presentBaseVdis.has(baseUuid)) {
debug('found base VDI', { debug('found base VDI', {
base: baseUuid, base: baseUuid,
vdi: srcVdi.uuid, vdi: srcVdiUuid,
}) })
} else { } else {
debug('missing base VDI', { debug('missing base VDI', {
base: baseUuid, base: baseUuid,
vdi: srcVdi.uuid, vdi: srcVdiUuid,
}) })
fullVdisRequired.add(srcVdi.uuid) fullVdisRequired.add(srcVdiUuid)
} }
}) })

View File

@ -1,9 +1,8 @@
import assert from 'node:assert' import assert from 'node:assert'
import mapValues from 'lodash/mapValues.js' import mapValues from 'lodash/mapValues.js'
import ignoreErrors from 'promise-toolbox/ignoreErrors'
import { asyncEach } from '@vates/async-each' import { asyncEach } from '@vates/async-each'
import { asyncMap } from '@xen-orchestra/async-map' import { asyncMap } from '@xen-orchestra/async-map'
import { chainVhd, checkVhdChain, openVhd, VhdAbstract } from 'vhd-lib' import { chainVhd, openVhd } from 'vhd-lib'
import { createLogger } from '@xen-orchestra/log' import { createLogger } from '@xen-orchestra/log'
import { decorateClass } from '@vates/decorate-with' import { decorateClass } from '@vates/decorate-with'
import { defer } from 'golike-defer' import { defer } from 'golike-defer'
@ -23,42 +22,45 @@ import { Disposable } from 'promise-toolbox'
const { warn } = createLogger('xo:backups:DeltaBackupWriter') const { warn } = createLogger('xo:backups:DeltaBackupWriter')
export class IncrementalRemoteWriter extends MixinRemoteWriter(AbstractIncrementalWriter) { export class IncrementalRemoteWriter extends MixinRemoteWriter(AbstractIncrementalWriter) {
#parentVdiPaths
#vhds
async checkBaseVdis(baseUuidToSrcVdi) { async checkBaseVdis(baseUuidToSrcVdi) {
this.#parentVdiPaths = {}
const { handler } = this._adapter const { handler } = this._adapter
const adapter = this._adapter const adapter = this._adapter
const vdisDir = `${this._vmBackupDir}/vdis/${this._job.id}` const vdisDir = `${this._vmBackupDir}/vdis/${this._job.id}`
await asyncMap(baseUuidToSrcVdi, async ([baseUuid, srcVdi]) => { await asyncMap(baseUuidToSrcVdi, async ([baseUuid, srcVdiUuid]) => {
let found = false let parentDestPath
const vhdDir = `${vdisDir}/${srcVdiUuid}`
try { try {
const vhds = await handler.list(`${vdisDir}/${srcVdi.uuid}`, { const vhds = await handler.list(vhdDir, {
filter: _ => _[0] !== '.' && _.endsWith('.vhd'), filter: _ => _[0] !== '.' && _.endsWith('.vhd'),
ignoreMissing: true, ignoreMissing: true,
prependDir: true, prependDir: true,
}) })
const packedBaseUuid = packUuid(baseUuid) const packedBaseUuid = packUuid(baseUuid)
await asyncMap(vhds, async path => { // the last one is probably the right one
try {
await checkVhdChain(handler, path)
// Warning, this should not be written as found = found || await adapter.isMergeableParent(packedBaseUuid, path)
//
// since all the checks of a path are done in parallel, found would be containing
// only the last answer of isMergeableParent which is probably not the right one
// this led to the support tickets https://help.vates.fr/#ticket/zoom/4751 , 4729, 4665 and 4300
const isMergeable = await adapter.isMergeableParent(packedBaseUuid, path) for (let i = vhds.length - 1; i >= 0 && parentDestPath === undefined; i--) {
found = found || isMergeable const path = vhds[i]
try {
if (await adapter.isMergeableParent(packedBaseUuid, path)) {
parentDestPath = path
}
} catch (error) { } catch (error) {
warn('checkBaseVdis', { error }) warn('checkBaseVdis', { error })
await ignoreErrors.call(VhdAbstract.unlink(handler, path))
} }
}) }
} catch (error) { } catch (error) {
warn('checkBaseVdis', { error }) warn('checkBaseVdis', { error })
} }
if (!found) { // no usable parent => the runner will have to decide to fall back to a full or stop backup
if (parentDestPath === undefined) {
baseUuidToSrcVdi.delete(baseUuid) baseUuidToSrcVdi.delete(baseUuid)
} else {
this.#parentVdiPaths[vhdDir] = parentDestPath
} }
}) })
} }

View File

@ -1,3 +1,4 @@
import assert from 'node:assert'
import { asyncMap, asyncMapSettled } from '@xen-orchestra/async-map' import { asyncMap, asyncMapSettled } from '@xen-orchestra/async-map'
import ignoreErrors from 'promise-toolbox/ignoreErrors' import ignoreErrors from 'promise-toolbox/ignoreErrors'
import { formatDateTime } from '@xen-orchestra/xapi' import { formatDateTime } from '@xen-orchestra/xapi'
@ -14,6 +15,7 @@ import find from 'lodash/find.js'
export class IncrementalXapiWriter extends MixinXapiWriter(AbstractIncrementalWriter) { export class IncrementalXapiWriter extends MixinXapiWriter(AbstractIncrementalWriter) {
async checkBaseVdis(baseUuidToSrcVdi, baseVm) { async checkBaseVdis(baseUuidToSrcVdi, baseVm) {
assert.notStrictEqual(baseVm, undefined)
const sr = this._sr const sr = this._sr
const replicatedVm = listReplicatedVms(sr.$xapi, this._job.id, sr.uuid, this._vmUuid).find( const replicatedVm = listReplicatedVms(sr.$xapi, this._job.id, sr.uuid, this._vmUuid).find(
vm => vm.other_config[TAG_COPY_SRC] === baseVm.uuid vm => vm.other_config[TAG_COPY_SRC] === baseVm.uuid