fix: ensure merge are made along a vhd chain

vatesfr · Apr 20, 2022 · f476c22 · f476c22
1 parent af1cfc3
commit f476c22
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 11 deletions.
diff --git a/@xen-orchestra/backups/_cleanVm.js b/@xen-orchestra/backups/_cleanVm.js
@@ -37,9 +37,10 @@ const computeVhdsSize = (handler, vhdPaths) =>
 // and all the others will deleted
 async function mergeVhdChain(chain, { handler, onLog, remove, merge }) {
   assert(chain.length >= 2)
+  // chain is parent -> child -> grand child > ..
   const chainCopy = [...chain]
   const parent = chainCopy.pop()
-  const children = chainCopy.reverse()
+  const children = chainCopy
 
   if (merge) {
     onLog(`merging ${children.length} children into ${parent}`)
@@ -59,7 +60,7 @@ async function mergeVhdChain(chain, { handler, onLog, remove, merge }) {
     })
 
     clearInterval(handle)
-    const mergeTargetChild = children.pop()
+    const mergeTargetChild = children.shift()
     await Promise.all([
       VhdAbstract.rename(handler, parent, mergeTargetChild),
       asyncMap(children, child => {

diff --git a/@xen-orchestra/backups/writers/DeltaBackupWriter.js b/@xen-orchestra/backups/writers/DeltaBackupWriter.js
@@ -6,7 +6,6 @@ const mapValues = require('lodash/mapValues.js')
 const ignoreErrors = require('promise-toolbox/ignoreErrors')
 const { asyncMap } = require('@xen-orchestra/async-map')
 const { chainVhd, checkVhdChain, openVhd, VhdAbstract } = require('vhd-lib')
-const { createLogger } = require('@xen-orchestra/log')
 const { dirname } = require('path')
 
 const { formatFilenameDate } = require('../_filenameDate.js')
@@ -20,8 +19,6 @@ const { checkVhd } = require('./_checkVhd.js')
 const { packUuid } = require('./_packUuid.js')
 const { Disposable } = require('promise-toolbox')
 
-const { warn } = createLogger('xo:backups:DeltaBackupWriter')
-
 exports.DeltaBackupWriter = class DeltaBackupWriter extends MixinBackupWriter(AbstractDeltaWriter) {
   async checkBaseVdis(baseUuidToSrcVdi) {
     const { handler } = this._adapter
@@ -33,11 +30,13 @@ exports.DeltaBackupWriter = class DeltaBackupWriter extends MixinBackupWriter(Ab
 
     await asyncMap(baseUuidToSrcVdi, async ([baseUuid, srcVdi]) => {
       let found = false
+      let foldersEmpty = true
       try {
         const vhds = await handler.list(`${vdisDir}/${srcVdi.uuid}`, {
           filter: _ => _[0] !== '.' && _.endsWith('.vhd'),
           prependDir: true,
         })
+        foldersEmpty = foldersEmpty || vhds.length > 0
         const packedBaseUuid = packUuid(baseUuid)
         await asyncMap(vhds, async path => {
           try {
@@ -51,14 +50,20 @@ exports.DeltaBackupWriter = class DeltaBackupWriter extends MixinBackupWriter(Ab
             const isMergeable = await adapter.isMergeableParent(packedBaseUuid, path)
             found = found || isMergeable
           } catch (error) {
-            warn('checkBaseVdis', { error })
+            Task.warning(`Error while checking existing VHD ${vdisDir}/${srcVdi.uuid} : ${error.toString()}`)
             await ignoreErrors.call(VhdAbstract.unlink(handler, path))
           }
         })
       } catch (error) {
-        warn('checkBaseVdis', { error })
+        Task.warning(
+          `Impossible to open ${vdisDir}/${srcVdi.uuid} folder to list precedent backups: ${error.toString()}`
+        )
       }
       if (!found) {
+        // don't show warning if it's the first backup
+        if (!foldersEmpty) {
+          Task.warning(`Impossible to find the base of ${srcVdi.uuid} for a delta : fallback to a full `)
+        }
         baseUuidToSrcVdi.delete(baseUuid)
       }
     })

diff --git a/packages/vhd-lib/merge.js b/packages/vhd-lib/merge.js
@@ -3,6 +3,7 @@
 // TODO: remove once completely merged in vhd.js
 
 const assert = require('assert')
+const UUID = require('uuid')
 const noop = require('./_noop')
 const { createLogger } = require('@xen-orchestra/log')
 const { limitConcurrency } = require('limit-concurrency-decorator')
@@ -59,13 +60,17 @@ module.exports = limitConcurrency(2)(async function merge(
     const childVhd = yield openVhd(childHandler, childPath)
 
     const concurrency = childVhd instanceof VhdDirectory ? 16 : 1
+
+    // merge should be along a vhd chain
+    assert.strictEqual(UUID.stringify(childVhd.header.parentUuid), UUID.stringify(parentVhd.footer.uuid))
+    const parentDiskType = parentVhd.footer.diskType
+    assert(parentDiskType === DISK_TYPES.DIFFERENCING || parentDiskType === DISK_TYPES.DYNAMIC)
+    assert.strictEqual(childVhd.footer.diskType, DISK_TYPES.DIFFERENCING)
+
     if (mergeState === undefined) {
       assert.strictEqual(childVhd.header.blockSize, parentVhd.header.blockSize)
-
-      const parentDiskType = parentVhd.footer.diskType
-      assert(parentDiskType === DISK_TYPES.DIFFERENCING || parentDiskType === DISK_TYPES.DYNAMIC)
-      assert.strictEqual(childVhd.footer.diskType, DISK_TYPES.DIFFERENCING)
     } else {
+      // vhd should not have changed to resume
       assert.strictEqual(parentVhd.header.checksum, mergeState.parent.header)
       assert.strictEqual(childVhd.header.checksum, mergeState.child.header)
     }