diff --git a/@xen-orchestra/backups/_cleanVm.js b/@xen-orchestra/backups/_cleanVm.js index f8ec789069d..d6355087c1a 100644 --- a/@xen-orchestra/backups/_cleanVm.js +++ b/@xen-orchestra/backups/_cleanVm.js @@ -2,6 +2,7 @@ const assert = require('assert') const sum = require('lodash/sum') +const UUID = require('uuid') const { asyncMap } = require('@xen-orchestra/async-map') const { Constants, mergeVhd, openVhd, VhdAbstract, VhdFile } = require('vhd-lib') const { isVhdAlias, resolveVhdAlias } = require('vhd-lib/aliases') @@ -50,7 +51,7 @@ const computeVhdsSize = (handler, vhdPaths) => async function mergeVhdChain(chain, { handler, logInfo, remove, merge }) { assert(chain.length >= 2) const chainCopy = [...chain] - const parent = chainCopy.pop() + const parent = chainCopy.shift() const children = chainCopy if (merge) { @@ -187,6 +188,7 @@ exports.cleanVm = async function cleanVm( const handler = this._handler const vhdsToJSons = new Set() + const vhdById = new Map() const vhdParents = { __proto__: null } const vhdChildren = { __proto__: null } @@ -208,6 +210,27 @@ exports.cleanVm = async function cleanVm( } vhdChildren[parent] = path } + // Detect VHDs with the same UUIDs + // + // Due to a bug introduced in a1bcd35e2 + const duplicate = vhdById.get(UUID.stringify(vhd.footer.uuid)) + let vhdKept = vhd + if (duplicate !== undefined) { + logWarn('uuid is duplicated', { uuid: UUID.stringify(vhd.footer.uuid) }) + if (duplicate.containsAllDataOf(vhd)) { + logWarn(`should delete ${path}`) + vhdKept = duplicate + vhds.delete(path) + } else if (vhd.containsAllDataOf(duplicate)) { + logWarn(`should delete ${duplicate._path}`) + vhds.delete(duplicate._path) + } else { + logWarn(`same ids but different content`) + } + } else { + logInfo('not duplicate', UUID.stringify(vhd.footer.uuid), path) + } + vhdById.set(UUID.stringify(vhdKept.footer.uuid), vhdKept) }) } catch (error) { vhds.delete(path) @@ -362,7 +385,7 @@ exports.cleanVm = async function cleanVm( const unusedVhdsDeletion = [] const toMerge = [] { - // VHD chains (as list from child to ancestor) to merge indexed by last + // VHD chains (as list from oldest to most recent) to merge indexed by most recent // ancestor const vhdChainsToMerge = { __proto__: null } @@ -386,7 +409,7 @@ exports.cleanVm = async function cleanVm( if (child !== undefined) { const chain = getUsedChildChainOrDelete(child) if (chain !== undefined) { - chain.push(vhd) + chain.unshift(vhd) return chain } } diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index c5f0c267177..c4f6e030415 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -14,6 +14,7 @@ > Users must be able to say: “I had this issue, happy to know it's fixed” - [Tasks] Fix tasks not displayed when running CR backup job [Forum#6038](https://xcp-ng.org/forum/topic/6038/not-seeing-tasks-any-more-as-admin) (PR [#6315](https://github.com/vatesfr/xen-orchestra/pull/6315)) +- [Backup] Fix failing merge multiple VHDs at once (PR [#6317](https://github.com/vatesfr/xen-orchestra/pull/6317)) ### Packages to release @@ -32,6 +33,8 @@ - @vates/async-each major +- @xen-orchestra/backups minor +- vhd-lib patch - xo-web minor diff --git a/packages/vhd-lib/Vhd/VhdAbstract.js b/packages/vhd-lib/Vhd/VhdAbstract.js index 965a704e44e..938ee47a53b 100644 --- a/packages/vhd-lib/Vhd/VhdAbstract.js +++ b/packages/vhd-lib/Vhd/VhdAbstract.js @@ -343,4 +343,21 @@ exports.VhdAbstract = class VhdAbstract { stream.length = footer.currentSize return stream } + + async containsAllDataOf(child) { + await this.readBlockAllocationTable() + await child.readBlockAllocationTable() + for await (const block of child.blocks()) { + const { id, data: childData } = block + // block is in child not in parent + if (!this.containsBlock(id)) { + return false + } + const { data: parentData } = await this.readBlock(id) + if (!childData.equals(parentData)) { + return false + } + } + return true + } } diff --git a/packages/vhd-lib/Vhd/VhdSynthetic.integ.spec.js b/packages/vhd-lib/Vhd/VhdSynthetic.integ.spec.js index ce9182911e2..9d45174a8d9 100644 --- a/packages/vhd-lib/Vhd/VhdSynthetic.integ.spec.js +++ b/packages/vhd-lib/Vhd/VhdSynthetic.integ.spec.js @@ -55,7 +55,7 @@ test('It can read block and parent locator from a synthetic vhd', async () => { await bigVhd.readHeaderAndFooter() - const syntheticVhd = yield VhdSynthetic.open(handler, [smallVhdFileName, bigVhdFileName]) + const syntheticVhd = yield VhdSynthetic.open(handler, [bigVhdFileName, smallVhdFileName]) await syntheticVhd.readBlockAllocationTable() expect(syntheticVhd.header.diskType).toEqual(bigVhd.header.diskType) diff --git a/packages/vhd-lib/Vhd/VhdSynthetic.js b/packages/vhd-lib/Vhd/VhdSynthetic.js index 7c766569f74..33860d3d32d 100644 --- a/packages/vhd-lib/Vhd/VhdSynthetic.js +++ b/packages/vhd-lib/Vhd/VhdSynthetic.js @@ -15,14 +15,13 @@ const VhdSynthetic = class VhdSynthetic extends VhdAbstract { #vhds = [] get header() { - // this the VHD we want to synthetize - const vhd = this.#vhds[0] + // this the most recent vhd + const vhd = this.#vhds[this.#vhds.length - 1] // this is the root VHD - const rootVhd = this.#vhds[this.#vhds.length - 1] + const rootVhd = this.#vhds[0] // data of our synthetic VHD - // TODO: set parentLocatorEntry-s in header return { ...vhd.header, parentLocatorEntry: cloneDeep(rootVhd.header.parentLocatorEntry), @@ -34,10 +33,13 @@ const VhdSynthetic = class VhdSynthetic extends VhdAbstract { } get footer() { - // this is the root VHD - const rootVhd = this.#vhds[this.#vhds.length - 1] + // this the most recent vhd + const vhd = this.#vhds[this.#vhds.length - 1] + + // this is the oldest VHD + const rootVhd = this.#vhds[0] return { - ...this.#vhds[0].footer, + ...vhd.footer, dataOffset: FOOTER_SIZE, diskType: rootVhd.footer.diskType, } @@ -77,17 +79,21 @@ const VhdSynthetic = class VhdSynthetic extends VhdAbstract { await asyncMap(vhds, vhd => vhd.readHeaderAndFooter()) for (let i = 0, n = vhds.length - 1; i < n; ++i) { - const child = vhds[i] - const parent = vhds[i + 1] + const parent = vhds[i] + const child = vhds[i + 1] assert.strictEqual(child.footer.diskType, DISK_TYPES.DIFFERENCING) assert.strictEqual(UUID.stringify(child.header.parentUuid), UUID.stringify(parent.footer.uuid)) } } #getVhdWithBlock(blockId) { - const index = this.#vhds.findIndex(vhd => vhd.containsBlock(blockId)) - assert(index !== -1, `no such block ${blockId}`) - return this.#vhds[index] + for (let i = this.#vhds.length - 1; i >= 0; i--) { + const vhd = this.#vhds[i] + if (vhd.containsBlock(blockId)) { + return vhd + } + } + assert(false, `no such block ${blockId}`) } async readBlock(blockId, onlyBitmap = false) { @@ -120,7 +126,7 @@ VhdSynthetic.fromVhdChain = Disposable.factory(async function* fromVhdChain(hand const vhds = [] do { vhd = yield openVhd(handler, vhdPath) - vhds.push(vhd) + vhds.unshift(vhd) // from oldest to most recent vhdPath = resolveRelativeFromFile(vhdPath, vhd.header.parentUnicodeName) } while (vhd.footer.diskType !== DISK_TYPES.DYNAMIC) diff --git a/packages/vhd-lib/merge.integ.spec.js b/packages/vhd-lib/merge.integ.spec.js index 20790167afb..7333cf76fa5 100644 --- a/packages/vhd-lib/merge.integ.spec.js +++ b/packages/vhd-lib/merge.integ.spec.js @@ -217,8 +217,7 @@ test('it cleans vhd mergedfiles', async () => { await handler.writeFile('child2', 'child2Data') await handler.writeFile('child3', 'child3Data') - // childPath is from the grand children to the children - await cleanupVhds(handler, 'parent', ['child3', 'child2', 'child1'], { remove: true }) + await cleanupVhds(handler, 'parent', ['child1', 'child2', 'child3'], { remove: true }) // only child3 should stay, with the data of parent const [child3, ...other] = await handler.list('.') diff --git a/packages/vhd-lib/merge.js b/packages/vhd-lib/merge.js index b20e0a2da3b..d4c358eefda 100644 --- a/packages/vhd-lib/merge.js +++ b/packages/vhd-lib/merge.js @@ -59,7 +59,7 @@ function cleanupVhds(handler, parent, children, { logInfo = noop, remove = false if (!Array.isArray(children)) { children = [children] } - const mergeTargetChild = children.shift() + const mergeTargetChild = children.pop() return Promise.all([ VhdAbstract.rename(handler, parent, mergeTargetChild),