fix(v2v): normalize absolute path

@xen-orchestra/backup-archive/src/disks/RemoteDisk.mjs

@@ -229,6 +229,30 @@ export class RemoteDisk extends RandomAccessDisk {
     throw new Error(`isDifferencing must be implemented`)
   }
 
+  /**
+   * Abstract
+   * @returns {Promise<boolean>}
+   */
+  async isDirectory() {
+    throw new Error(`isDirectory must be implemented`)
+  }
+
+  /**
+   * Checks whether this disk (or chain) can act as the merge-target parent for a disk
+   * about to be written with the given reference configuration.
+   *
+   * Base check: this disk's own uuid matches the expected parent uuid. Subclasses should
+   * override to add implementation-specific compatibility checks (storage sub-format,
+   * compression, ...) — calling `super.isMergeableParent()` first.
+   *
+   * @param {string} parentUuid
+   * @param {Object} referenceDiskConfig - describes what this remote would write for a new disk
+   * @returns {Promise<boolean>}
+   */
+  async isMergeableParent(parentUuid, referenceDiskConfig) {
+    return this.getUuid() === parentUuid
+  }
+
   /**
    * Abstract
    * Rename alias/disk

@xen-orchestra/backup-archive/src/disks/RemoteVhdDisk.mjs

@@ -433,6 +433,38 @@ export class RemoteVhdDisk extends RemoteDisk {
     return this.#isDifferencing
   }
 
+  /**
+   * Returns the compression codec id. Only meaningful for VHD directories;
+   * undefined for plain VHDs.
+   * @returns {string | undefined}
+   */
+  #getCompressionType() {
+    if (this.#vhd === undefined) {
+      throw new Error(`can't check compressionType of a RemoteVhdDisk before init`)
+    }
+    return this.#vhd instanceof VhdDirectory ? this.#vhd.compressionType : undefined
+  }
+
+  /**
+   * Adds VHD-specific compatibility checks on top of the uuid check: the storage
+   * sub-format (VHD directory vs plain) and, for directories, the compression codec
+   * must match what this remote would write for a new disk.
+   * @param {string} parentUuid
+   * @param {Object} referenceDiskConfig
+   * @param {boolean} referenceDiskConfig.useVhdDirectory
+   * @param {string} [referenceDiskConfig.compressionType]
+   * @returns {Promise<boolean>}
+   */
+  async isMergeableParent(parentUuid, referenceDiskConfig) {
+    if (!(await super.isMergeableParent(parentUuid, referenceDiskConfig))) {
+      return false
+    }
+
+    const { useVhdDirectory, compressionType } = referenceDiskConfig
+    const isDirectory = await this.isDirectory()
+    return isDirectory ? useVhdDirectory && this.#getCompressionType() === compressionType : !useVhdDirectory
+  }
+
   /**
    * Rename alias/disk
    * @param {string} newPath

@xen-orchestra/backups/RemoteAdapter.integ.mjs

@@ -0,0 +1,144 @@
+import test from 'node:test'
+import { strict as assert } from 'node:assert'
+
+import tmp from 'tmp'
+import fs from 'fs-extra'
+import * as uuid from 'uuid'
+import { getHandler } from '@xen-orchestra/fs'
+import { pFromCallback } from 'promise-toolbox'
+import { RemoteAdapter } from './RemoteAdapter.mjs'
+import { VHDFOOTER, VHDHEADER } from './tests.fixtures.mjs'
+import { VhdFile, VhdAbstract, Constants, VhdDirectory } from 'vhd-lib'
+import { dirname, basename } from 'node:path'
+import { rimraf } from 'rimraf'
+
+const { beforeEach, afterEach, describe } = test
+
+let tempDir, handler, jobId, vdiId, basePath, relativePath
+const rootPath = 'xo-vm-backups/VMUUID/'
+
+beforeEach(async () => {
+  tempDir = await pFromCallback(cb => tmp.dir(cb))
+  jobId = uniqueId()
+  vdiId = uniqueId()
+  relativePath = `vdis/${jobId}/${vdiId}`
+  basePath = `${rootPath}/${relativePath}`
+  await fs.mkdirp(`${tempDir}/${basePath}`)
+})
+
+afterEach(async () => {
+  await rimraf(tempDir)
+  await handler.forget()
+})
+
+const uniqueId = () => uuid.v1()
+const uniqueIdBuffer = () => uuid.v1({}, Buffer.alloc(16))
+
+async function getAdapter({ useVhdDirectory = false, vhdDirectoryCompression, remoteCompressionType } = {}) {
+  handler = getHandler({ url: `file://${tempDir}`, useVhdDirectory, compressionType: remoteCompressionType })
+  await handler.sync()
+  return new RemoteAdapter(handler, { vhdDirectoryCompression })
+}
+
+// mirrors the helper in _cleanVm.integ.mjs, with compression support for VHD directories.
+// VHD directories are only openable through their alias (see RemoteVhdDisk.init), so
+// `path` here is always the alias-less name; the alias is created as `${path}.alias.vhd`.
+async function generateVhd(path, opts = {}) {
+  let vhd
+
+  let dataPath = path
+  if (opts.mode === 'directory') {
+    dataPath = dirname(path) + '/data/' + basename(path)
+    await handler.mkdir(dirname(path) + '/data/')
+    await handler.mkdir(dataPath)
+    vhd = new VhdDirectory(handler, dataPath, { compression: opts.compression })
+  } else {
+    const fd = await handler.openFile(dataPath, 'wx')
+    vhd = new VhdFile(handler, fd)
+  }
+
+  vhd.header = { ...VHDHEADER, ...opts.header }
+  vhd.footer = { ...VHDFOOTER, ...opts.footer, uuid: opts.uuid ?? uniqueIdBuffer() }
+
+  if (vhd.header.parentUuid) {
+    vhd.footer.diskType = Constants.DISK_TYPES.DIFFERENCING
+  } else {
+    vhd.footer.diskType = Constants.DISK_TYPES.DYNAMIC
+  }
+
+  if (opts.mode === 'directory') {
+    await VhdAbstract.createAlias(handler, path + '.alias.vhd', dataPath)
+  }
+
+  await vhd.writeBlockAllocationTable()
+  await vhd.writeHeader()
+  await vhd.writeFooter()
+  return vhd
+}
+
+describe('RemoteAdapter#isMergeableParent', { concurrency: 1 }, () => {
+  for (const diskIsDirectory of [false, true]) {
+    for (const useVhdDirectory of [false, true]) {
+      for (const compressionMatches of [false, true]) {
+        for (const uuidMatches of [false, true]) {
+          const expected = uuidMatches && (diskIsDirectory ? useVhdDirectory && compressionMatches : !useVhdDirectory)
+
+          test(`diskIsDirectory=${diskIsDirectory} useVhdDirectory=${useVhdDirectory} compressionMatches=${compressionMatches} uuidMatches=${uuidMatches} -> ${expected}`, async () => {
+            const targetCompression = 'brotli'
+            const adapter = await getAdapter({ useVhdDirectory, vhdDirectoryCompression: targetCompression })
+            const targetUuid = uniqueIdBuffer()
+
+            await generateVhd(`${basePath}/disk.vhd`, {
+              mode: diskIsDirectory ? 'directory' : 'plain',
+              compression: diskIsDirectory ? (compressionMatches ? targetCompression : 'gzip') : undefined,
+              uuid: uuidMatches ? targetUuid : uniqueIdBuffer(),
+            })
+            const path = diskIsDirectory ? `${basePath}/disk.vhd.alias.vhd` : `${basePath}/disk.vhd`
+
+            assert.equal(await adapter.isMergeableParent(targetUuid, path), expected)
+          })
+        }
+      }
+    }
+  }
+})
+
+describe('RemoteAdapter#isMergeableParent compression resolution', { concurrency: 1 }, () => {
+  test('the per-remote compressionType override wins over the adapter default', async () => {
+    const adapter = await getAdapter({
+      useVhdDirectory: true,
+      vhdDirectoryCompression: 'gzip', // config.toml-style default
+      remoteCompressionType: 'brotli', // per-remote override, should be the one actually compared against
+    })
+    const targetUuid = uniqueIdBuffer()
+    await generateVhd(`${basePath}/disk.vhd`, { mode: 'directory', compression: 'brotli', uuid: targetUuid })
+
+    assert.equal(await adapter.isMergeableParent(targetUuid, `${basePath}/disk.vhd.alias.vhd`), true)
+  })
+
+  test('a disk compressed with the adapter default (not the override) is rejected', async () => {
+    const adapter = await getAdapter({
+      useVhdDirectory: true,
+      vhdDirectoryCompression: 'gzip',
+      remoteCompressionType: 'brotli',
+    })
+    const targetUuid = uniqueIdBuffer()
+    // disk uses the config default, not the remote override that's actually in effect
+    await generateVhd(`${basePath}/disk.vhd`, { mode: 'directory', compression: 'gzip', uuid: targetUuid })
+
+    assert.equal(await adapter.isMergeableParent(targetUuid, `${basePath}/disk.vhd.alias.vhd`), false)
+  })
+
+  test("a remote-level 'none' override means no compression, ignoring the adapter default", async () => {
+    const adapter = await getAdapter({
+      useVhdDirectory: true,
+      vhdDirectoryCompression: 'brotli',
+      remoteCompressionType: 'none',
+    })
+    const targetUuid = uniqueIdBuffer()
+    // no compression option => VhdDirectory's own getCompressionType() is undefined, matching 'none'
+    await generateVhd(`${basePath}/disk.vhd`, { mode: 'directory', uuid: targetUuid })
+
+    assert.equal(await adapter.isMergeableParent(targetUuid, `${basePath}/disk.vhd.alias.vhd`), true)
+  })
+})

@xen-orchestra/backups/RemoteAdapter.mjs

@@ -1,7 +1,7 @@
 import { asyncEach } from '@vates/async-each'
 import { asyncMap, asyncMapSettled } from '@xen-orchestra/async-map'
 import { createLogger } from '@xen-orchestra/log'
-import { VhdDirectory, VhdSynthetic } from 'vhd-lib'
+import { stringify } from 'uuid'
 import { decorateMethodsWith } from '@vates/decorate-with'
 import { dirname, join, resolve } from 'node:path'
 import { synchronized } from 'decorator-synchronized'
@@ -20,7 +20,7 @@ import { isMetadataFile } from './_backupType.mjs'
 import { isValidXva } from './_isValidXva.mjs'
 import { watchStreamSize } from './_watchStreamSize.mjs'
 
-import { RemoteVhdDisk, openDiskChain } from '@xen-orchestra/backup-archive/disks'
+import { RemoteVhdDisk, openDiskChain, openDisposableDisk } from '@xen-orchestra/backup-archive/disks'
 import { toVhdStream, writeToVhdDirectory } from 'vhd-lib/disk-consumer/index.mjs'
 import { ReadAhead } from '@xen-orchestra/disk-transform'
 
@@ -65,19 +65,17 @@ export class RemoteAdapter {
 
   // check if we will be allowed to merge a vhd created in this adapter
   // with the vhd at path `path`
+  //
+  // only the candidate disk itself is checked (no ancestor walk): mergeBlock() already
+  // degrades gracefully when parent/child formats differ, so chain-wide uniformity isn't
+  // a requirement here.
   async isMergeableParent(packedParentUid, path) {
-    return await Disposable.use(VhdSynthetic.fromVhdChain(this.handler, path), vhd => {
-      // this baseUuid is not linked with this vhd
-      if (!vhd.footer.uuid.equals(packedParentUid)) {
-        return false
-      }
-
-      // check if all the chain is composed of vhd directory
-      const isVhdDirectory = vhd.checkVhdsClass(VhdDirectory)
-      return isVhdDirectory
-        ? this.useVhdDirectory() && this.#getCompressionType() === vhd.compressionType
-        : !this.useVhdDirectory()
-    })
+    return await Disposable.use(openDisposableDisk({ handler: this.handler, path, ignoreBlockIndexes: true }), disk =>
+      disk.isMergeableParent(stringify(packedParentUid), {
+        useVhdDirectory: this.useVhdDirectory(),
+        compressionType: this.#getCompressionType(),
+      })
+    )
   }
 
   async #removeVmBackupsFromCache(backups) {
@@ -206,7 +204,8 @@ export class RemoteAdapter {
   }
 
   #getCompressionType() {
-    return this._vhdDirectoryCompression
+    const compressionType = this.handler.vhdDirectoryCompression() ?? this._vhdDirectoryCompression
+    return compressionType === 'none' ? undefined : compressionType
   }
 
   useVhdDirectory() {
@@ -460,7 +459,7 @@ export class RemoteAdapter {
           path,
           concurrency: writeBlockConcurrency,
           validator,
-          compression: 'brotli',
+          compression: this.#getCompressionType(),
           uuid,
           parentUuid,
           parentPath,

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

@@ -2,13 +2,12 @@ import { createLogger } from '@xen-orchestra/log'
 
 import { asyncEach } from '@vates/async-each'
 import assert from 'node:assert'
-import * as UUID from 'uuid'
 
 import { AbstractRemote } from './_AbstractRemote.mjs'
 import { forkDeltaExport } from './_forkDeltaExport.mjs'
 import { IncrementalRemoteWriter } from '../_writers/IncrementalRemoteWriter.mjs'
 import { Disposable } from 'promise-toolbox'
-import { openVhd } from 'vhd-lib'
+import { openDisposableDisk } from '@xen-orchestra/backup-archive/disks'
 import { getVmBackupDir } from '../../_getVmBackupDir.mjs'
 import { SynchronizedDisk, ThrottledDisk } from '@xen-orchestra/disk-transform'
 import { AggregatedIncrementalRemoteWriter } from '../_writers/AggregatedIncrementalRemoteWriter.mjs'
@@ -44,9 +43,12 @@ class IncrementalRemoteVmBackupRunner extends AbstractRemote {
         const vmDir = getVmBackupDir(metadata.vm.uuid)
         const path = `${vmDir}/${metadata.vhds[id]}`
         // don't catch error : we can't recover if the source vhd are missing
-        await Disposable.use(openVhd(this._sourceRemoteAdapter._handler, path), vhd => {
-          baseUuidToSrcVdi.set(UUID.stringify(vhd.header.parentUuid), vdi.$snapshot_of$uuid)
-        })
+        await Disposable.use(
+          openDisposableDisk({ handler: this._sourceRemoteAdapter._handler, path, ignoreBlockIndexes: true }),
+          disk => {
+            baseUuidToSrcVdi.set(disk.getParentUuid(), vdi.$snapshot_of$uuid)
+          }
+        )
       }
     })
 

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

@@ -14,9 +14,8 @@ import { getOldEntries } from '../../_getOldEntries.mjs'
 
 import { MixinRemoteWriter } from './_MixinRemoteWriter.mjs'
 import { AbstractIncrementalWriter } from './_AbstractIncrementalWriter.mjs'
-import { checkVhd } from './_checkVhd.mjs'
+import { checkDisk } from './_checkDisk.mjs'
 import { packUuid } from './_packUuid.mjs'
-
 const { warn } = createLogger('xo:backups:DeltaBackupWriter')
 
 export class IncrementalRemoteWriter extends MixinRemoteWriter(AbstractIncrementalWriter) {
@@ -201,7 +200,7 @@ export class IncrementalRemoteWriter extends MixinRemoteWriter(AbstractIncrement
           }
 
           const transferred = await adapter.writeVhd(path, disk, {
-            validator: tmpPath => checkVhd(handler, tmpPath),
+            validator: tmpPath => checkDisk(handler, tmpPath),
             writeBlockConcurrency: this._config.writeBlockConcurrency,
             uuid: packUuid(vdi.uuid),
             parentUuid,

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

@@ -0,0 +1,6 @@
+import { openDisposableDisk } from '@xen-orchestra/backup-archive/disks'
+import Disposable from 'promise-toolbox/Disposable'
+
+export async function checkDisk(handler, path) {
+  await Disposable.use(openDisposableDisk({ handler, path }), () => {})
+}

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

@@ -1,6 +0,0 @@
-import { openVhd } from 'vhd-lib'
-import Disposable from 'promise-toolbox/Disposable'
-
-export async function checkVhd(handler, path) {
-  await Disposable.use(openVhd(handler, path), () => {})
-}

@xen-orchestra/fs/src/abstract.js

@@ -609,6 +609,10 @@ export default class RemoteHandlerAbstract {
     return this._remote.useVhdDirectory ?? false
   }
 
+  vhdDirectoryCompression() {
+    return this._remote.compressionType
+  }
+
   async _closeFile(fd) {
     throw new Error('Not implemented')
   }

@xen-orchestra/fs/src/abstract.test.js

@@ -13,9 +13,9 @@ import tmp from 'tmp'
 const TIMEOUT = 12e5
 
 class TestHandler extends AbstractHandler {
-  constructor() {
+  constructor(remoteOverrides = {}) {
     const options = { timeout: TIMEOUT, withRetry: [] }
-    super({ url: 'test://' }, options)
+    super({ url: 'test://', ...remoteOverrides }, options)
     Object.defineProperty(this, 'isEncrypted', {
       get: () => false, // encryption is tested separately
     })
@@ -58,6 +58,16 @@ describe('closeFile()', () => {
   })
 })
 
+describe('vhdDirectoryCompression()', () => {
+  it('is undefined when not set on the remote', () => {
+    assert.equal(new TestHandler().vhdDirectoryCompression(), undefined)
+  })
+
+  it('reads the remote-level compressionType override', () => {
+    assert.equal(new TestHandler({ compressionType: 'brotli' }).vhdDirectoryCompression(), 'brotli')
+  })
+})
+
 describe('getInfo()', () => {
   it('throws in case of timeout', async () => {
     const testHandler = new TestHandler()

@xen-orchestra/fs/src/types/fs.mts

@@ -7,6 +7,7 @@ export interface RemoteInfo {
   encryptionKey?: string
   immutable?: boolean
   useVhdDirectory?: boolean
+  compressionType?: string
   [key: string]: unknown
 }
 
@@ -162,5 +163,6 @@ export abstract class RemoteHandlerAbstract {
   abstract test(): Promise<TestResult>
   abstract isImmutable(): boolean
   abstract useVhdDirectory(): boolean
+  abstract vhdDirectoryCompression(): string | undefined
   abstract addPrefix(prefix: string): RemoteHandlerAbstract
 }

@xen-orchestra/vmware-explorer/esxi.mjs

@@ -2,7 +2,7 @@ import { Client } from '@vates/node-vsphere-soap'
 import { createLogger } from '@xen-orchestra/log'
 import { dirname, join } from 'node:path'
 import { EventEmitter } from 'node:events'
-import { strictEqual, notStrictEqual } from 'node:assert'
+import { strictEqual } from 'node:assert'
 import { Agent } from 'undici'
 
 import parseVmdk from './parsers/vmdk.mjs'
@@ -17,6 +17,15 @@ import fs from 'node:fs/promises'
 
 const { info, warn } = createLogger('xo:vmware-explorer:esxi')
 
+// a datastore path can be expressed in multiple ways depending on the source:
+// - `ds:///vmfs/volumes/<uuid>/` in the `summary.url` reported by vCenter
+// - `/vmfs/volumes/<uuid>` in the `summary.url` reported by an ESXi host
+// - `/vmfs/volumes/<uuid>/<dir>/<file>.vmdk` in the vmx of a VM (typical of vSAN)
+// normalize them to compare a file path with a datastore path
+function normalizeDatastorePath(path) {
+  return path.replace(/^ds:\/\//, '').replace(/\/+$/, '')
+}
+
 export const VDDK_LIB_DIR = '/usr/local/lib/vddk'
 export const VDDK_LIB_PATH = `${VDDK_LIB_DIR}/vmware-vix-disklib-distrib`
 let nbdPort = 11000
@@ -68,14 +77,23 @@ export default class Esxi extends EventEmitter {
     const res = await this.search('Datacenter', ['name', 'datastore'])
     await Promise.all(
       Object.values(res).map(async ({ datastore, name }) => {
-        if (datastore.ManagedObjectReference === undefined) {
+        if (datastore?.ManagedObjectReference === undefined) {
+          warn('datacenter without any datastore', { datacenter: name })
           return
         }
+        // a datacenter with a single datastore gives an object instead of an array
+        const managedObjectReferences = [datastore.ManagedObjectReference].flat()
         await Promise.all(
-          datastore.ManagedObjectReference.map(async ({ $value }) => {
+          managedObjectReferences.map(async ({ $value }) => {
             // get the datastore name
             const res = await this.fetchProperty('Datastore', $value, 'name')
-            this.#dcPaths[res._] = name
+            // the property is wrapped in an object only when the response carries attributes
+            const dataStoreName = typeof res === 'string' ? res : res?._
+            if (dataStoreName === undefined) {
+              warn("can't read the name of a datastore", { datacenter: name, datastore: $value, res })
+              return
+            }
+            this.#dcPaths[dataStoreName] = name
           })
         )
       })
@@ -83,14 +101,15 @@ export default class Esxi extends EventEmitter {
   }
 
   #findDatacenter(dataStore) {
-    try {
-      notStrictEqual(this.#dcPaths, undefined)
-      notStrictEqual(this.#dcPaths[dataStore], undefined)
-    } catch (error) {
+    const dcPath = this.#dcPaths?.[dataStore]
+    if (dcPath === undefined) {
       warn("can't find datacenter for datastore", { datacenters: this.#dcPaths, dataStore })
+      const error = new Error(`can't find the datacenter of the datastore ${dataStore}`)
+      error.dataStore = dataStore
+      error.datastores = Object.keys(this.#dcPaths ?? {})
       throw error
     }
-    return this.#dcPaths[dataStore]
+    return dcPath
   }
 
   #exec(cmd, args) {
@@ -251,14 +270,23 @@ export default class Esxi extends EventEmitter {
 
   async #inspectVmdk(dataStores, currentDataStore, currentPath, filePath) {
     let diskDataStore, diskPath
-    if (filePath.startsWith('/')) {
-      // disk is on another datastore
-      Object.keys(dataStores).forEach(dataStoreUrl => {
-        if (filePath.startsWith(dataStoreUrl)) {
-          diskDataStore = dataStores[dataStoreUrl].name
-          diskPath = filePath.substring(dataStoreUrl.length + 1)
+    if (/^(ds:\/\/)?\//.test(filePath)) {
+      // the disk is referenced by an absolute path: find the datastore containing it
+      const normalizedFilePath = normalizeDatastorePath(filePath)
+      for (const [dataStoreUrl, dataStore] of Object.entries(dataStores)) {
+        const prefix = normalizeDatastorePath(dataStoreUrl)
+        if (normalizedFilePath.startsWith(prefix + '/')) {
+          diskDataStore = dataStore.name
+          diskPath = normalizedFilePath.substring(prefix.length + 1)
+          break
         }
-      })
+      }
+      if (diskDataStore === undefined) {
+        const error = new Error(`can't find the datastore containing the disk ${filePath}`)
+        error.datastoreUrls = Object.keys(dataStores)
+        error.filePath = filePath
+        throw error
+      }
     } else {
       diskDataStore = currentDataStore
       diskPath = currentPath + '/' + filePath