fix(vhd): vhd chain should be ordered from oldest to newest

@xen-orchestra/backups/_cleanVm.js

@@ -51,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) {
@@ -197,7 +197,7 @@ exports.cleanVm = async function cleanVm(
   // remove broken VHDs
   await asyncMap(vhds, async path => {
     try {
-      await Disposable.use(openVhd(handler, path, { checkSecondFooter: !interruptedVhds.has(path) }), async vhd => {
+      await Disposable.use(openVhd(handler, path, { checkSecondFooter: !interruptedVhds.has(path) }), vhd => {
         if (vhd.footer.diskType === DISK_TYPES.DIFFERENCING) {
           const parent = resolve('/', dirname(path), vhd.header.parentUnicodeName)
           vhdParents[path] = parent
@@ -382,7 +382,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 }
 
@@ -406,7 +406,7 @@ exports.cleanVm = async function cleanVm(
       if (child !== undefined) {
         const chain = getUsedChildChainOrDelete(child)
         if (chain !== undefined) {
-          chain.push(vhd)
+          chain.unshift(vhd)
           return chain
         }
       }

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 merging failing during merge (PR [#6317](https://github.com/vatesfr/xen-orchestra/pull/6317))
 
 ### Packages to release
 
@@ -32,6 +33,8 @@
 <!--packages-start-->
 
 - @vates/async-each major
+- @xen-orchestra/backups minor
 - xo-web minor
+- vhd-lib patch
 
 <!--packages-end-->

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)

packages/vhd-lib/Vhd/VhdSynthetic.js

@@ -15,17 +15,16 @@ 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),
+      parentLocatorEntry: cloneDeep(vhd.header.parentLocatorEntry),
       tableOffset: FOOTER_SIZE + HEADER_SIZE,
       parentTimestamp: rootVhd.header.parentTimestamp,
       parentUnicodeName: rootVhd.header.parentUnicodeName,
@@ -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)
 

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('.')

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),