fix(@»en-orchestra/backups): better cleaning of broken vhd

must be merged after #6471 and all tests passing

@xen-orchestra/backups/_cleanVm.js

@@ -3,7 +3,7 @@
 const sum = require('lodash/sum')
 const UUID = require('uuid')
 const { asyncMap } = require('@xen-orchestra/async-map')
-const { Constants, openVhd, VhdAbstract, VhdFile } = require('vhd-lib')
+const { Constants, openVhd, VhdAbstract, VhdFile, BrokenVhdError } = require('vhd-lib')
 const { isVhdAlias, resolveVhdAlias } = require('vhd-lib/aliases')
 const { dirname, resolve } = require('path')
 const { DISK_TYPES } = Constants
@@ -242,7 +242,7 @@ exports.cleanVm = async function cleanVm(
     } catch (error) {
       vhds.delete(path)
       logWarn('VHD check error', { path, error })
-      if (error?.code === 'ERR_ASSERTION' && remove) {
+      if (error instanceof BrokenVhdError && remove) {
         logInfo('deleting broken VHD', { path })
         return VhdAbstract.unlink(handler, path)
       }

packages/vhd-lib/BrokenVhdError.js

@@ -0,0 +1,10 @@
+'use strict'
+class BrokenVhdError extends Error {
+  constructor(message, baseError) {
+    super(message)
+    this.code = 'BROKEN_VHD'
+    this.cause = baseError
+  }
+}
+
+module.exports = BrokenVhdError

packages/vhd-lib/Vhd/VhdDirectory.js

@@ -9,6 +9,7 @@ const assert = require('assert')
 const { synchronized } = require('decorator-synchronized')
 const promisify = require('promise-toolbox/promisify')
 const zlib = require('zlib')
+const BrokenVhdError = require('../BrokenVhdError')
 
 const { debug } = createLogger('vhd-lib:VhdDirectory')
 
@@ -110,7 +111,14 @@ exports.VhdDirectory = class VhdDirectory extends VhdAbstract {
     // EISDIR pathname refers to a directory and the access requested
     // involved writing (that is, O_WRONLY or O_RDWR is set).
     // reading the header ensure we have a well formed directory immediatly
-    await vhd.readHeaderAndFooter()
+    try {
+      await vhd.readHeaderAndFooter()
+    } catch (error) {
+      if (error.code === 'ERR_ASSERTION') {
+        throw new BrokenVhdError('Invalid header or footer', error)
+      }
+      throw error
+    }
     return {
       dispose: () => {},
       value: vhd,
@@ -185,18 +193,8 @@ exports.VhdDirectory = class VhdDirectory extends VhdAbstract {
   async readHeaderAndFooter() {
     await this.#readChunkFilters()
 
-    let bufHeader, bufFooter
-    try {
-      bufHeader = (await this._readChunk('header')).buffer
-      bufFooter = (await this._readChunk('footer')).buffer
-    } catch (error) {
-      // emit an AssertionError if the VHD is broken to stay as close as possible to the VhdFile API
-      if (error.code === 'ENOENT') {
-        assert(false, 'Header And Footer should exists')
-      } else {
-        throw error
-      }
-    }
+    const bufHeader = (await this._readChunk('header')).buffer
+    const bufFooter = (await this._readChunk('footer')).buffer
     const footer = unpackFooter(bufFooter)
     const header = unpackHeader(bufHeader, footer)
 

packages/vhd-lib/Vhd/VhdFile.js

@@ -10,6 +10,7 @@ const {
 } = require('../_constants')
 const { computeBatSize, sectorsToBytes, unpackHeader, unpackFooter, BUF_BLOCK_UNUSED } = require('./_utils')
 const { createLogger } = require('@xen-orchestra/log')
+const BrokenVhdError = require('../BrokenVhdError')
 const { fuFooter, fuHeader, checksumStruct } = require('../_structs')
 const { set: mapSetBit } = require('../_bitmap')
 const { VhdAbstract } = require('./VhdAbstract')
@@ -93,7 +94,14 @@ exports.VhdFile = class VhdFile extends VhdAbstract {
     // EISDIR pathname refers to a directory and the access requested
     // involved writing (that is, O_WRONLY or O_RDWR is set).
     // reading the header ensure we have a well formed file immediatly
-    await vhd.readHeaderAndFooter(checkSecondFooter)
+    try {
+      await vhd.readHeaderAndFooter(checkSecondFooter)
+    } catch (error) {
+      if (error.code === 'ERR_ASSERTION') {
+        throw new BrokenVhdError('Invalid header or footer', error)
+      }
+      throw error
+    }
     return {
       dispose: () => handler.closeFile(fd),
       value: vhd,

packages/vhd-lib/index.js

@@ -1,5 +1,6 @@
 'use strict'
 
+exports.BrokenVhdError = require('./BrokenVhdError')
 exports.chainVhd = require('./chain')
 exports.checkFooter = require('./checkFooter')
 exports.checkVhdChain = require('./checkChain')

packages/vhd-lib/openVhd.js

@@ -1,17 +1,37 @@
 'use strict'
 
+const { BrokenVhdError } = require('.')
 const { resolveVhdAlias } = require('./aliases')
 const { VhdDirectory } = require('./Vhd/VhdDirectory.js')
 const { VhdFile } = require('./Vhd/VhdFile.js')
 
+class AggregateError extends Error {
+  constructor(errors, message) {
+    super(message)
+    this.errors = errors
+  }
+}
+
 exports.openVhd = async function openVhd(handler, path, opts) {
   const resolved = await resolveVhdAlias(handler, path)
   try {
-    return await VhdFile.open(handler, resolved, opts)
-  } catch (e) {
-    if (e.code !== 'EISDIR') {
-      throw e
-    }
     return await VhdDirectory.open(handler, resolved, opts)
+  } catch (vhdDirectoryError) {
+    // it's a directory, but it's an invalid vhd
+    if (vhdDirectoryError instanceof BrokenVhdError) {
+      throw vhdDirectoryError
+    }
+
+    // it's not a vhd directory, try to open it as a vhd file
+    try {
+      return await VhdFile.open(handler, resolved, opts)
+    } catch (vhdFileError) {
+      // this is really a file that looks like a vhd but is broken
+      if (vhdFileError instanceof BrokenVhdError) {
+        throw vhdFileError
+      }
+      // the errors are not vhd related, throw both error to keep a trace
+      throw new AggregateError([vhdDirectoryError, vhdFileError])
+    }
   }
 }