refactor(vhd-lib): code cleanup

CHANGELOG.unreleased.md

@@ -29,9 +29,9 @@
 
 <!--packages-start-->
 
-- vhd-lib minor
-- xo-server minor
 - @vates/read-chunk major
 - @xen-orchestra/backups minor
+- vhd-lib minor
+- xo-server minor
 
 <!--packages-end-->

packages/vhd-lib/Vhd/VhdAbstract.js

@@ -104,7 +104,7 @@ exports.VhdAbstract = class VhdAbstract {
    *
    * @returns {number} the merged data size
    */
-  async coalesceBlock(child, blockId) {
+  async mergeBlock(child, blockId) {
     const block = await child.readBlock(blockId)
     await this.writeEntireBlock(block)
     return block.data.length

packages/vhd-lib/Vhd/VhdDirectory.integ.spec.js

@@ -54,7 +54,7 @@ test('Can coalesce block', async () => {
     await childDirectoryVhd.readBlockAllocationTable()
 
     let childBlockData = (await childDirectoryVhd.readBlock(0)).data
-    await parentVhd.coalesceBlock(childDirectoryVhd, 0)
+    await parentVhd.mergeBlock(childDirectoryVhd, 0)
     await parentVhd.writeFooter()
     await parentVhd.writeBlockAllocationTable()
     let parentBlockData = (await parentVhd.readBlock(0)).data
@@ -64,7 +64,7 @@ test('Can coalesce block', async () => {
     await expect(childDirectoryVhd.readBlock(0)).rejects.toThrowError()
 
     childBlockData = (await childFileVhd.readBlock(1)).data
-    await parentVhd.coalesceBlock(childFileVhd, 1)
+    await parentVhd.mergeBlock(childFileVhd, 1)
     await parentVhd.writeFooter()
     await parentVhd.writeBlockAllocationTable()
     parentBlockData = (await parentVhd.readBlock(1)).data

packages/vhd-lib/Vhd/VhdDirectory.js

@@ -142,13 +142,13 @@ exports.VhdDirectory = class VhdDirectory extends VhdAbstract {
     return test(this.#blockTable, blockId)
   }
 
-  _getChunkPath(partName) {
+  #getChunkPath(partName) {
     return this._path + '/' + partName
   }
 
   async _readChunk(partName) {
     // here we can implement compression and / or crypto
-    const buffer = await this._handler.readFile(this._getChunkPath(partName))
+    const buffer = await this._handler.readFile(this.#getChunkPath(partName))
 
     const uncompressed = await this.#compressor.decompress(buffer)
     return {
@@ -164,18 +164,18 @@ exports.VhdDirectory = class VhdDirectory extends VhdAbstract {
     )
 
     const compressed = await this.#compressor.compress(buffer)
-    return this._handler.outputFile(this._getChunkPath(partName), compressed, this._opts)
+    return this._handler.outputFile(this.#getChunkPath(partName), compressed, this._opts)
   }
 
   // put block in subdirectories to limit impact when doing directory listing
-  _getBlockPath(blockId) {
+  #getBlockPath(blockId) {
     const blockPrefix = Math.floor(blockId / 1e3)
     const blockSuffix = blockId - blockPrefix * 1e3
     return `blocks/${blockPrefix}/${blockSuffix}`
   }
 
-  getFullBlockPath(blockId) {
-    return this._getChunkPath(this._getBlockPath(blockId))
+  _getFullBlockPath(blockId) {
+    return this.#getChunkPath(this.#getBlockPath(blockId))
   }
 
   async readHeaderAndFooter() {
@@ -204,7 +204,7 @@ exports.VhdDirectory = class VhdDirectory extends VhdAbstract {
     if (onlyBitmap) {
       throw new Error(`reading 'bitmap of block' ${blockId} in a VhdDirectory is not implemented`)
     }
-    const { buffer } = await this._readChunk(this._getBlockPath(blockId))
+    const { buffer } = await this._readChunk(this.#getBlockPath(blockId))
     return {
       id: blockId,
       bitmap: buffer.slice(0, this.bitmapSize),
@@ -246,16 +246,36 @@ exports.VhdDirectory = class VhdDirectory extends VhdAbstract {
   // only works if data are in the same handler
   // and if the full block is modified in child ( which is the case with xcp)
   // and if the compression type is same on both sides
-  async coalesceBlock(child, blockId) {
-    if (!child.isBlockBased() || this._handler !== child._handler || child.compressionType !== this.compressionType) {
-      return super.coalesceBlock(child, blockId)
+  async mergeBlock(child, blockId, isResumingMerge = false) {
+    if (
+      !child.isBlockBased() ||
+      this._handler !== child._handler ||
+      child.compressionType !== this.compressionType ||
+      child.compressionType === 'MIXED'
+    ) {
+      return super.mergeBlock(child, blockId)
     }
-    await this._handler.rename(child.getFullBlockPath(blockId), this.getFullBlockPath(blockId))
+    try {
+      await this._handler.rename(child._getFullBlockPath(blockId), this._getFullBlockPath(blockId))
+    } catch (error) {
+      if (error.code === 'ENOENT' && isResumingMerge === true) {
+        // when resuming, the blocks moved since the last merge state write are
+        // not in the child anymore but it should be ok
+
+        // it will throw an error if block is missing in parent
+        // won't detect if the block was already in parent and is broken/missing in child
+        const { data } = await this.readBlock(blockId)
+        assert.strictEqual(data.length, this.header.blockSize)
+      } else {
+        throw error
+      }
+    }
+    setBitmap(this.#blockTable, blockId)
     return sectorsToBytes(this.sectorsPerBlock)
   }
 
   async writeEntireBlock(block) {
-    await this._writeChunk(this._getBlockPath(block.id), block.buffer)
+    await this._writeChunk(this.#getBlockPath(block.id), block.buffer)
     setBitmap(this.#blockTable, block.id)
   }
 

packages/vhd-lib/Vhd/VhdFile.integ.spec.js

@@ -222,14 +222,14 @@ test('Can coalesce block', async () => {
     const childDirectoryVhd = yield openVhd(handler, childDirectoryName)
     await childDirectoryVhd.readBlockAllocationTable()
 
-    await parentVhd.coalesceBlock(childFileVhd, 0)
+    await parentVhd.mergeBlock(childFileVhd, 0)
     await parentVhd.writeFooter()
     await parentVhd.writeBlockAllocationTable()
     let parentBlockData = (await parentVhd.readBlock(0)).data
     let childBlockData = (await childFileVhd.readBlock(0)).data
     expect(parentBlockData).toEqual(childBlockData)
 
-    await parentVhd.coalesceBlock(childDirectoryVhd, 0)
+    await parentVhd.mergeBlock(childDirectoryVhd, 0)
     await parentVhd.writeFooter()
     await parentVhd.writeBlockAllocationTable()
     parentBlockData = (await parentVhd.readBlock(0)).data

packages/vhd-lib/Vhd/VhdSynthetic.js

@@ -47,7 +47,7 @@ const VhdSynthetic = class VhdSynthetic extends VhdAbstract {
     const compressionType = this.vhds[0].compressionType
     for (let i = 0; i < this.vhds.length; i++) {
       if (compressionType !== this.vhds[i].compressionType) {
-        return undefined
+        return 'MIXED'
       }
     }
     return compressionType
@@ -95,16 +95,16 @@ const VhdSynthetic = class VhdSynthetic extends VhdAbstract {
     return await this.#getVhdWithBlock(blockId).readBlock(blockId, onlyBitmap)
   }
 
-  async coalesceBlock(child, blockId) {
+  async mergeBlock(child, blockId) {
     throw new Error(`can't coalesce block into a vhd synthetic`)
   }
 
   _readParentLocatorData(id) {
     return this.#vhds[this.#vhds.length - 1]._readParentLocatorData(id)
   }
-  getFullBlockPath(blockId) {
+  _getFullBlockPath(blockId) {
     const vhd = this.#getVhdWithBlock(blockId)
-    return vhd.getFullBlockPath(blockId)
+    return vhd._getFullBlockPath(blockId)
   }
   isBlockBased() {
     return this.#vhds.every(vhd => vhd.isBlockBased())

packages/vhd-lib/merge.integ.spec.js

@@ -9,7 +9,7 @@ const { getHandler } = require('@xen-orchestra/fs')
 const { pFromCallback } = require('promise-toolbox')
 
 const { VhdFile, chainVhd, mergeVhd } = require('./index')
-const { cleanupVhds } = require('./merge')
+const { _cleanupVhds: cleanupVhds } = require('./merge')
 
 const { checkFile, createRandomFile, convertFromRawToVhd } = require('./tests/utils')
 

packages/vhd-lib/merge.js

@@ -72,7 +72,7 @@ function cleanupVhds(handler, parent, children, { logInfo = noop, remove = false
     }),
   ])
 }
-module.exports.cleanupVhds = cleanupVhds
+module.exports._cleanupVhds = cleanupVhds
 
 // Merge one or multiple vhd child into vhd parent.
 // childPath can be array to create a synthetic VHD from multiple VHDs
@@ -168,23 +168,7 @@ module.exports.mergeVhd = limitConcurrency(2)(async function merge(
       toMerge,
       async blockId => {
         merging.add(blockId)
-        try {
-          mergeState.mergedDataSize += await parentVhd.coalesceBlock(childVhd, blockId)
-        } catch (error) {
-          if (error.code === 'ENOENT' && isResuming === true) {
-            // when resuming, the blocks moved since the last merge state write are
-            // not in the child anymore but it should be ok
-
-            // it will throw an error if block is missing in parent
-            // won't detect if the block was already in parent and is broken/missing in child
-            const { data } = await parentVhd.readBlock(blockId)
-            assert.strictEqual(data.length, parentVhd.header.blockSize)
-
-            mergeState.mergedDataSize += parentVhd.header.blockSize
-          } else {
-            throw error
-          }
-        }
+        mergeState.mergedDataSize += await parentVhd.mergeBlock(childVhd, blockId, isResuming)
 
         merging.delete(blockId)