feat(backups): don't do snapshot of stopped VM if we use hash compares against parent instead of snapshots comparison

@xen-orchestra/backups/_runners/_vmRunners/IncrementalXapi.mjs

@@ -26,7 +26,16 @@ export const IncrementalXapi = class IncrementalXapiVmBackupRunner extends Abstr
   }
 
   _mustDoSnapshot() {
-    return true
+    const vm = this._vm
+
+    const settings = this._settings
+    return (
+      settings.unconditionalSnapshot ||
+      (!settings.offlineBackup && vm.power_state === 'Running') ||
+      settings.snapshotRetention !== 0 ||
+      settings.fullInterval !== 1 ||
+      settings.deltaComputationMode === 'AGAINST_PARENT_VHD'
+    )
   }
 
   async _copy() {

@xen-orchestra/backups/_runners/_writers/IncrementalRemoteWriter.mjs

@@ -3,7 +3,7 @@ import mapValues from 'lodash/mapValues.js'
 import ignoreErrors from 'promise-toolbox/ignoreErrors'
 import { asyncEach } from '@vates/async-each'
 import { asyncMap } from '@xen-orchestra/async-map'
-import { chainVhd, checkVhdChain, openVhd, VhdAbstract } from 'vhd-lib'
+import { chainVhd, checkVhdChain, openVhd, VhdAbstract, VhdSynthetic } from 'vhd-lib'
 import { createLogger } from '@xen-orchestra/log'
 import { decorateClass } from '@vates/decorate-with'
 import { defer } from 'golike-defer'
@@ -183,6 +183,7 @@ export class IncrementalRemoteWriter extends MixinRemoteWriter(AbstractIncrement
 
           const isDifferencing = isVhdDifferencing[`${id}.vhd`]
           let parentPath
+          let parentVhd
           if (isDifferencing) {
             const vdiDir = dirname(path)
             parentPath = (
@@ -204,6 +205,11 @@ export class IncrementalRemoteWriter extends MixinRemoteWriter(AbstractIncrement
 
             // TODO remove when this has been done before the export
             await checkVhd(handler, parentPath)
+            if(settings.deltaComputationMode === 'AGAINST_PARENT_VHD'){
+              const {dispose, value } =  await VhdSynthetic.fromVhdChain(handler, parentPath)
+              parentVhd = value
+              $defer(()=>dispose())
+            }
           }
 
           // don't write it as transferSize += await async function
@@ -213,6 +219,7 @@ export class IncrementalRemoteWriter extends MixinRemoteWriter(AbstractIncrement
             // no checksum for VHDs, because they will be invalidated by
             // merges and chainings
             checksum: false,
+            parentVhd,
             validator: tmpPath => checkVhd(handler, tmpPath),
             writeBlockConcurrency: this._config.writeBlockConcurrency,
           })

packages/vhd-lib/Vhd/VhdAbstract.js

@@ -84,6 +84,9 @@ exports.VhdAbstract = class VhdAbstract {
   readBlockAllocationTable() {
     throw new Error(`reading block allocation table is not implemented`)
   }
+  readBlockHashes() {
+    throw new Error(`reading block hashes table is not implemented`)
+  }
 
   /**
    * @typedef {Object} BitmapBlock
@@ -104,6 +107,10 @@ exports.VhdAbstract = class VhdAbstract {
     throw new Error(`reading  ${onlyBitmap ? 'bitmap of block' : 'block'} ${blockId} is not implemented`)
   }
 
+  getBlockHash(blockId){
+    throw new Error(`reading block hash ${blockId} is not implemented`)
+  }
+
   /**
    * coalesce the block with id blockId from the child vhd into
    * this vhd

packages/vhd-lib/Vhd/VhdDirectory.js

@@ -4,6 +4,7 @@ const { unpackHeader, unpackFooter, sectorsToBytes } = require('./_utils')
 const { createLogger } = require('@xen-orchestra/log')
 const { fuFooter, fuHeader, checksumStruct } = require('../_structs')
 const { test, set: setBitmap } = require('../_bitmap')
+const { hashBlock } = require('../hashBlock')
 const { VhdAbstract } = require('./VhdAbstract')
 const assert = require('assert')
 const { synchronized } = require('decorator-synchronized')
@@ -75,6 +76,7 @@ function getCompressor(compressorType) {
 
 exports.VhdDirectory = class VhdDirectory extends VhdAbstract {
   #uncheckedBlockTable
+  #blockHashes
   #header
   footer
   #compressor
@@ -140,6 +142,17 @@ exports.VhdDirectory = class VhdDirectory extends VhdAbstract {
     this.#blockTable = buffer
   }
 
+  async readBlockHashes() {
+    try {
+      const { buffer } = await this._readChunk('hashes')
+      this.#blockHashes = JSON.parse(buffer)
+    } catch (err) {
+      if (err.code !== 'ENOENT') {
+        throw err
+      }
+    }
+  }
+
   containsBlock(blockId) {
     return test(this.#blockTable, blockId)
   }
@@ -177,6 +190,11 @@ exports.VhdDirectory = class VhdDirectory extends VhdAbstract {
     const blockSuffix = blockId - blockPrefix * 1e3
     return `blocks/${blockPrefix}/${blockSuffix}`
   }
+  getBlockHash(blockId) {
+    if (this.#blockHashes !== undefined) {
+      return this.#blockHashes[blockId]
+    }
+  }
 
   _getFullBlockPath(blockId) {
     return this.#getChunkPath(this.#getBlockPath(blockId))
@@ -209,6 +227,10 @@ exports.VhdDirectory = class VhdDirectory extends VhdAbstract {
       throw new Error(`reading 'bitmap of block' ${blockId} in a VhdDirectory is not implemented`)
     }
     const { buffer } = await this._readChunk(this.#getBlockPath(blockId))
+    const hash = this.getBlockHash(blockId)
+    if (hash) {
+      assert.strictEqual(hash, hash(buffer))
+    }
     return {
       id: blockId,
       bitmap: buffer.slice(0, this.bitmapSize),
@@ -244,7 +266,7 @@ exports.VhdDirectory = class VhdDirectory extends VhdAbstract {
     assert.notStrictEqual(this.#blockTable, undefined, 'Block allocation table has not been read')
     assert.notStrictEqual(this.#blockTable.length, 0, 'Block allocation table is empty')
 
-    return this._writeChunk('bat', this.#blockTable)
+    return Promise.all([this._writeChunk('bat', this.#blockTable), this._writeChunk('hashes', this.#blockHashes)])
   }
 
   // only works if data are in the same handler
@@ -265,8 +287,11 @@ exports.VhdDirectory = class VhdDirectory extends VhdAbstract {
       await this._handler.rename(childBlockPath, this._getFullBlockPath(blockId))
       if (!blockExists) {
         setBitmap(this.#blockTable, blockId)
+        this.#blockHashes[blockId] = child.getBlockHash(blockId)
         await this.writeBlockAllocationTable()
       }
+      // @todo block hashes changs may be lost if the vhd merging fail
+      // should migrate to writing bat from time to time, sync with the metadata
     } catch (error) {
       if (error.code === 'ENOENT' && isResumingMerge === true) {
         // when resuming, the blocks moved since the last merge state write are
@@ -287,6 +312,7 @@ exports.VhdDirectory = class VhdDirectory extends VhdAbstract {
   async writeEntireBlock(block) {
     await this._writeChunk(this.#getBlockPath(block.id), block.buffer)
     setBitmap(this.#blockTable, block.id)
+    this.#blockHashes[block.id] = hashBlock(block.buffer)
   }
 
   async _readParentLocatorData(id) {

packages/vhd-lib/Vhd/VhdSynthetic.js

@@ -96,6 +96,10 @@ const VhdSynthetic = class VhdSynthetic extends VhdAbstract {
     assert(false, `no such block ${blockId}`)
   }
 
+  async getBlockHash(blockId){
+    return this.#getVhdWithBlock(blockId).getBlockHash(blockId)
+  }
+
   async readBlock(blockId, onlyBitmap = false) {
     // only read the content of the first vhd containing this block
     return await this.#getVhdWithBlock(blockId).readBlock(blockId, onlyBitmap)

packages/vhd-lib/createVhdDirectoryFromStream.js

@@ -1,6 +1,7 @@
 'use strict'
 
 const { createLogger } = require('@xen-orchestra/log')
+const { hashBlock } = require('./hashBlock.js')
 const { parseVhdStream } = require('./parseVhdStream.js')
 const { VhdDirectory } = require('./Vhd/VhdDirectory.js')
 const { Disposable } = require('promise-toolbox')
@@ -8,7 +9,7 @@ const { asyncEach } = require('@vates/async-each')
 
 const { warn } = createLogger('vhd-lib:createVhdDirectoryFromStream')
 
-const buildVhd = Disposable.wrap(async function* (handler, path, inputStream, { concurrency, compression }) {
+const buildVhd = Disposable.wrap(async function* (handler, path, inputStream, { concurrency, compression, parentVhd }) {
   const vhd = yield VhdDirectory.create(handler, path, { compression })
   await asyncEach(
     parseVhdStream(inputStream),
@@ -24,6 +25,10 @@ const buildVhd = Disposable.wrap(async function* (handler, path, inputStream, {
           await vhd.writeParentLocator({ ...item, data: item.buffer })
           break
         case 'block':
+          if (parentVhd !== undefined && hashBlock(item.buffer) === parentVhd.getBlockHash(item.id)) {
+            // already in parent
+            return
+          }
           await vhd.writeEntireBlock(item)
           break
         case 'bat':
@@ -45,10 +50,10 @@ exports.createVhdDirectoryFromStream = async function createVhdDirectoryFromStre
   handler,
   path,
   inputStream,
-  { validator, concurrency = 16, compression } = {}
+  { validator, concurrency = 16, compression, parentVhd } = {}
 ) {
   try {
-    const size = await buildVhd(handler, path, inputStream, { concurrency, compression })
+    const size = await buildVhd(handler, path, inputStream, { concurrency, compression, parentVhd })
     if (validator !== undefined) {
       await validator.call(this, path)
     }

packages/vhd-lib/hashBlock.js

@@ -0,0 +1,12 @@
+'use strict'
+
+const { createHash } = require('node:crypto')
+
+// using xxhash as for xva would make smaller hash and the collision risk would be low for the dedup,
+// since we have a tuple(index, hash), but it would be notable if
+// we implement dedup on top of this later
+// at most, a 2TB full vhd will use 32MB for its hashes
+// and this file is compressed with vhd block
+exports.hashBlock = function (buffer) {
+  return createHash('sha256').update(buffer).digest('hex')
+}