fix(backups): clean snapshot correctly on failure

The previous implementation only considered version upgrades and did not take into account the installation of missing patches.

See zammad#21487
Introduced by 85ec261

@xen-orchestra/backups/RemoteAdapter.mjs

@@ -191,13 +191,14 @@ export class RemoteAdapter {
   // check if we will be allowed to merge a a vhd created in this adapter
   // with the vhd at path `path`
   async isMergeableParent(packedParentUid, path) {
-    return await Disposable.use(openVhd(this.handler, path), vhd => {
+    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
       }
 
-      const isVhdDirectory = vhd instanceof VhdDirectory
+      // 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()

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

@@ -2,6 +2,7 @@ import { asyncEach } from '@vates/async-each'
 import { decorateMethodsWith } from '@vates/decorate-with'
 import { defer } from 'golike-defer'
 import assert from 'node:assert'
+import * as UUID from 'uuid'
 import isVhdDifferencingDisk from 'vhd-lib/isVhdDifferencingDisk.js'
 import mapValues from 'lodash/mapValues.js'
 
@@ -9,11 +10,48 @@ import { AbstractRemote } from './_AbstractRemote.mjs'
 import { forkDeltaExport } from './_forkDeltaExport.mjs'
 import { IncrementalRemoteWriter } from '../_writers/IncrementalRemoteWriter.mjs'
 import { Task } from '../../Task.mjs'
+import { Disposable } from 'promise-toolbox'
+import { openVhd } from 'vhd-lib'
+import { getVmBackupDir } from '../../_getVmBackupDir.mjs'
 
 class IncrementalRemoteVmBackupRunner extends AbstractRemote {
   _getRemoteWriter() {
     return IncrementalRemoteWriter
   }
+  async _selectBaseVm(metadata) {
+    // for each disk , get the parent
+    const baseUuidToSrcVdi = new Map()
+
+    // no previous backup for a base( =key) backup
+    if (metadata.isBase) {
+      return
+    }
+    await asyncEach(Object.entries(metadata.vdis), async ([id, vdi]) => {
+      const isDifferencing = metadata.isVhdDifferencing[`${id}.vhd`]
+      if (isDifferencing) {
+        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)
+        })
+      }
+    })
+
+    const presentBaseVdis = new Map(baseUuidToSrcVdi)
+    await this._callWriters(
+      writer => presentBaseVdis.size !== 0 && writer.checkBaseVdis(presentBaseVdis),
+      'writer.checkBaseVdis()',
+      false
+    )
+    // check if the parent vdi are present in all the remotes
+    baseUuidToSrcVdi.forEach((srcVdiUuid, baseUuid) => {
+      if (!presentBaseVdis.has(baseUuid)) {
+        throw new Error(`Missing vdi ${baseUuid} which is a base for a delta`)
+      }
+    })
+    // yeah , let's go
+  }
   async _run($defer) {
     const transferList = await this._computeTransferList(({ mode }) => mode === 'delta')
     await this._callWriters(async writer => {
@@ -26,7 +64,7 @@ class IncrementalRemoteVmBackupRunner extends AbstractRemote {
     if (transferList.length > 0) {
       for (const metadata of transferList) {
         assert.strictEqual(metadata.mode, 'delta')
-
+        await this._selectBaseVm(metadata)
         await this._callWriters(writer => writer.prepare({ isBase: metadata.isBase }), 'writer.prepare()')
         const incrementalExport = await this._sourceRemoteAdapter.readIncrementalVmBackup(metadata, undefined, {
           useChain: false,
@@ -50,6 +88,17 @@ class IncrementalRemoteVmBackupRunner extends AbstractRemote {
             }),
           'writer.transfer()'
         )
+        // this will update parent name with the needed alias
+        await this._callWriters(
+          writer =>
+            writer.updateUuidAndChain({
+              isVhdDifferencing,
+              timestamp: metadata.timestamp,
+              vdis: incrementalExport.vdis,
+            }),
+          'writer.updateUuidAndChain()'
+        )
+
         await this._callWriters(writer => writer.cleanup(), 'writer.cleanup()')
         // for healthcheck
         this._tags = metadata.vm.tags

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

@@ -78,6 +78,18 @@ export const IncrementalXapi = class IncrementalXapiVmBackupRunner extends Abstr
       'writer.transfer()'
     )
 
+    // we want to control the uuid of the vhd in the chain
+    // and ensure they are correctly chained
+    await this._callWriters(
+      writer =>
+        writer.updateUuidAndChain({
+          isVhdDifferencing,
+          timestamp,
+          vdis: deltaExport.vdis,
+        }),
+      'writer.updateUuidAndChain()'
+    )
+
     this._baseVm = exportedVm
 
     if (baseVm !== undefined) {
@@ -133,7 +145,7 @@ export const IncrementalXapi = class IncrementalXapiVmBackupRunner extends Abstr
       ])
       const srcVdi = srcVdis[snapshotOf]
       if (srcVdi !== undefined) {
-        baseUuidToSrcVdi.set(baseUuid, srcVdi)
+        baseUuidToSrcVdi.set(baseUuid, srcVdi.uuid)
       } else {
         debug('ignore snapshot VDI because no longer present on VM', {
           vdi: baseUuid,
@@ -154,18 +166,18 @@ export const IncrementalXapi = class IncrementalXapiVmBackupRunner extends Abstr
     }
 
     const fullVdisRequired = new Set()
-    baseUuidToSrcVdi.forEach((srcVdi, baseUuid) => {
+    baseUuidToSrcVdi.forEach((srcVdiUuid, baseUuid) => {
       if (presentBaseVdis.has(baseUuid)) {
         debug('found base VDI', {
           base: baseUuid,
-          vdi: srcVdi.uuid,
+          vdi: srcVdiUuid,
         })
       } else {
         debug('missing base VDI', {
           base: baseUuid,
-          vdi: srcVdi.uuid,
+          vdi: srcVdiUuid,
         })
-        fullVdisRequired.add(srcVdi.uuid)
+        fullVdisRequired.add(srcVdiUuid)
       }
     })
 

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

@@ -2,6 +2,7 @@ import assert from 'node:assert'
 import groupBy from 'lodash/groupBy.js'
 import ignoreErrors from 'promise-toolbox/ignoreErrors'
 import { asyncMap } from '@xen-orchestra/async-map'
+import { createLogger } from '@xen-orchestra/log'
 import { decorateMethodsWith } from '@vates/decorate-with'
 import { defer } from 'golike-defer'
 import { formatDateTime } from '@xen-orchestra/xapi'
@@ -10,6 +11,8 @@ import { getOldEntries } from '../../_getOldEntries.mjs'
 import { Task } from '../../Task.mjs'
 import { Abstract } from './_Abstract.mjs'
 
+const { info, warn } = createLogger('xo:backups:AbstractXapi')
+
 export const AbstractXapi = class AbstractXapiVmBackupRunner extends Abstract {
   constructor({
     config,
@@ -171,6 +174,20 @@ export const AbstractXapi = class AbstractXapiVmBackupRunner extends Abstract {
     }
   }
 
+  // this will delete current snapshot in case of failure
+  // to ensure any retry will start with a clean state, especially in the case of rolling snapshots
+  #removeCurrentSnapshotOnFailure() {
+    if (this._mustDoSnapshot() && this._exportedVm !== undefined) {
+      info('will delete snapshot on failure', { vm: this._vm, snapshot: this._exportedVm })
+      assert.notStrictEqual(
+        this._vm.$ref,
+        this._exportedVm.$ref,
+        'there should have a snapshot, but vm and snapshot have the same ref'
+      )
+      return this._xapi.VM_destroy(this._exportedVm.$ref)
+    }
+  }
+
   async _fetchJobSnapshots() {
     const jobId = this._jobId
     const vmRef = this._vm.$ref
@@ -271,11 +288,17 @@ export const AbstractXapi = class AbstractXapiVmBackupRunner extends Abstract {
           await this._exportedVm.update_blocked_operations({ pool_migrate, migrate_send })
         }
       }
+    } catch (error) {
+      try {
+        await this.#removeCurrentSnapshotOnFailure()
+      } catch (removeSnapshotError) {
+        warn('fail removing current snapshot', { error: removeSnapshotError })
+      }
+      throw error
     } finally {
       if (startAfter) {
         ignoreErrors.call(vm.$callAsync('start', false, false))
       }
-
       await this._fetchJobSnapshots()
       await this._removeUnusedSnapshots()
     }

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

@@ -1,17 +1,15 @@
 import assert from 'node:assert'
 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, openVhd } from 'vhd-lib'
 import { createLogger } from '@xen-orchestra/log'
 import { decorateClass } from '@vates/decorate-with'
 import { defer } from 'golike-defer'
-import { dirname } from 'node:path'
+import { dirname, basename } from 'node:path'
 
 import { formatFilenameDate } from '../../_filenameDate.mjs'
 import { getOldEntries } from '../../_getOldEntries.mjs'
-import { TAG_BASE_DELTA } from '../../_incrementalVm.mjs'
 import { Task } from '../../Task.mjs'
 
 import { MixinRemoteWriter } from './_MixinRemoteWriter.mjs'
@@ -23,42 +21,45 @@ import { Disposable } from 'promise-toolbox'
 const { warn } = createLogger('xo:backups:DeltaBackupWriter')
 
 export class IncrementalRemoteWriter extends MixinRemoteWriter(AbstractIncrementalWriter) {
+  #parentVdiPaths
+  #vhds
   async checkBaseVdis(baseUuidToSrcVdi) {
+    this.#parentVdiPaths = {}
     const { handler } = this._adapter
     const adapter = this._adapter
 
     const vdisDir = `${this._vmBackupDir}/vdis/${this._job.id}`
 
-    await asyncMap(baseUuidToSrcVdi, async ([baseUuid, srcVdi]) => {
-      let found = false
+    await asyncMap(baseUuidToSrcVdi, async ([baseUuid, srcVdiUuid]) => {
+      let parentDestPath
+      const vhdDir = `${vdisDir}/${srcVdiUuid}`
       try {
-        const vhds = await handler.list(`${vdisDir}/${srcVdi.uuid}`, {
+        const vhds = await handler.list(vhdDir, {
           filter: _ => _[0] !== '.' && _.endsWith('.vhd'),
           ignoreMissing: true,
           prependDir: true,
         })
         const packedBaseUuid = packUuid(baseUuid)
-        await asyncMap(vhds, async path => {
-          try {
-            await checkVhdChain(handler, path)
-            // Warning, this should not be written as found = found || await adapter.isMergeableParent(packedBaseUuid, path)
-            //
-            // since all the checks of a path are done in parallel, found would be containing
-            // only the last answer of isMergeableParent which is probably not the right one
-            // this led to the support tickets  https://help.vates.fr/#ticket/zoom/4751 , 4729, 4665 and 4300
+        // the last one is probably the right one
 
-            const isMergeable = await adapter.isMergeableParent(packedBaseUuid, path)
-            found = found || isMergeable
+        for (let i = vhds.length - 1; i >= 0 && parentDestPath === undefined; i--) {
+          const path = vhds[i]
+          try {
+            if (await adapter.isMergeableParent(packedBaseUuid, path)) {
+              parentDestPath = path
+            }
           } catch (error) {
             warn('checkBaseVdis', { error })
-            await ignoreErrors.call(VhdAbstract.unlink(handler, path))
           }
-        })
+        }
       } catch (error) {
         warn('checkBaseVdis', { error })
       }
-      if (!found) {
+      // no usable parent => the runner will have to decide to fall back to a full or stop backup
+      if (parentDestPath === undefined) {
         baseUuidToSrcVdi.delete(baseUuid)
+      } else {
+        this.#parentVdiPaths[vhdDir] = parentDestPath
       }
     })
   }
@@ -123,6 +124,44 @@ export class IncrementalRemoteWriter extends MixinRemoteWriter(AbstractIncrement
     }
   }
 
+  async updateUuidAndChain({ isVhdDifferencing, vdis }) {
+    assert.notStrictEqual(
+      this.#vhds,
+      undefined,
+      '_transfer must be called before updateUuidAndChain for incremental backups'
+    )
+
+    const parentVdiPaths = this.#parentVdiPaths
+    const { handler } = this._adapter
+    const vhds = this.#vhds
+    await asyncEach(Object.entries(vdis), async ([id, vdi]) => {
+      const isDifferencing = isVhdDifferencing[`${id}.vhd`]
+      const path = `${this._vmBackupDir}/${vhds[id]}`
+      if (isDifferencing) {
+        assert.notStrictEqual(
+          parentVdiPaths,
+          'checkbasevdi must be called before updateUuidAndChain for incremental backups'
+        )
+        const parentPath = parentVdiPaths[dirname(path)]
+        // we are in a incremental backup
+        // we already computed the chain in checkBaseVdis
+        assert.notStrictEqual(parentPath, undefined, 'A differential VHD must have a parent')
+        // forbid any kind of loop
+        assert.ok(basename(parentPath) < basename(path), `vhd must be sorted to be chained`)
+        await chainVhd(handler, parentPath, handler, path)
+      }
+
+      // set the correct UUID in the VHD if needed
+      await Disposable.use(openVhd(handler, path), async vhd => {
+        if (!vhd.footer.uuid.equals(packUuid(vdi.uuid))) {
+          vhd.footer.uuid = packUuid(vdi.uuid)
+          await vhd.readBlockAllocationTable() // required by writeFooter()
+          await vhd.writeFooter()
+        }
+      })
+    })
+  }
+
   async _deleteOldEntries() {
     const adapter = this._adapter
     const oldEntries = this._oldEntries
@@ -141,16 +180,10 @@ export class IncrementalRemoteWriter extends MixinRemoteWriter(AbstractIncrement
     const jobId = job.id
     const handler = adapter.handler
 
-    let metadataContent = await this._isAlreadyTransferred(timestamp)
-    if (metadataContent !== undefined) {
-      // skip backup while being vigilant to not stuck the forked stream
-      Task.info('This backup has already been transfered')
-      Object.values(deltaExport.streams).forEach(stream => stream.destroy())
-      return { size: 0 }
-    }
-
     const basename = formatFilenameDate(timestamp)
-    const vhds = mapValues(
+    // update this.#vhds before eventually skipping transfer, so that
+    // updateUuidAndChain has all the mandatory data
+    const vhds = (this.#vhds = mapValues(
       deltaExport.vdis,
       vdi =>
         `vdis/${jobId}/${
@@ -160,7 +193,15 @@ export class IncrementalRemoteWriter extends MixinRemoteWriter(AbstractIncrement
               vdi.uuid
             : vdi.$snapshot_of$uuid
         }/${adapter.getVhdFileName(basename)}`
-    )
+    ))
+
+    let metadataContent = await this._isAlreadyTransferred(timestamp)
+    if (metadataContent !== undefined) {
+      // skip backup while being vigilant to not stuck the forked stream
+      Task.info('This backup has already been transfered')
+      Object.values(deltaExport.streams).forEach(stream => stream.destroy())
+      return { size: 0 }
+    }
 
     metadataContent = {
       isVhdDifferencing,
@@ -176,38 +217,13 @@ export class IncrementalRemoteWriter extends MixinRemoteWriter(AbstractIncrement
       vm,
       vmSnapshot,
     }
+
     const { size } = await Task.run({ name: 'transfer' }, async () => {
       let transferSize = 0
       await asyncEach(
-        Object.entries(deltaExport.vdis),
-        async ([id, vdi]) => {
+        Object.keys(deltaExport.vdis),
+        async id => {
           const path = `${this._vmBackupDir}/${vhds[id]}`
-
-          const isDifferencing = isVhdDifferencing[`${id}.vhd`]
-          let parentPath
-          if (isDifferencing) {
-            const vdiDir = dirname(path)
-            parentPath = (
-              await handler.list(vdiDir, {
-                filter: filename => filename[0] !== '.' && filename.endsWith('.vhd'),
-                prependDir: true,
-              })
-            )
-              .sort()
-              .pop()
-
-            assert.notStrictEqual(
-              parentPath,
-              undefined,
-              `missing parent of ${id} in ${dirname(path)}, looking for ${vdi.other_config[TAG_BASE_DELTA]}`
-            )
-
-            parentPath = parentPath.slice(1) // remove leading slash
-
-            // TODO remove when this has been done before the export
-            await checkVhd(handler, parentPath)
-          }
-
           // don't write it as transferSize += await async function
           // since i += await asyncFun lead to race condition
           // as explained : https://eslint.org/docs/latest/rules/require-atomic-updates
@@ -219,17 +235,6 @@ export class IncrementalRemoteWriter extends MixinRemoteWriter(AbstractIncrement
             writeBlockConcurrency: this._config.writeBlockConcurrency,
           })
           transferSize += transferSizeOneDisk
-
-          if (isDifferencing) {
-            await chainVhd(handler, parentPath, handler, path)
-          }
-
-          // set the correct UUID in the VHD
-          await Disposable.use(openVhd(handler, path), async vhd => {
-            vhd.footer.uuid = packUuid(vdi.uuid)
-            await vhd.readBlockAllocationTable() // required by writeFooter()
-            await vhd.writeFooter()
-          })
         },
         {
           concurrency: settings.diskPerVmConcurrency,

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

@@ -1,3 +1,4 @@
+import assert from 'node:assert'
 import { asyncMap, asyncMapSettled } from '@xen-orchestra/async-map'
 import ignoreErrors from 'promise-toolbox/ignoreErrors'
 import { formatDateTime } from '@xen-orchestra/xapi'
@@ -14,6 +15,7 @@ import find from 'lodash/find.js'
 
 export class IncrementalXapiWriter extends MixinXapiWriter(AbstractIncrementalWriter) {
   async checkBaseVdis(baseUuidToSrcVdi, baseVm) {
+    assert.notStrictEqual(baseVm, undefined)
     const sr = this._sr
     const replicatedVm = listReplicatedVms(sr.$xapi, this._job.id, sr.uuid, this._vmUuid).find(
       vm => vm.other_config[TAG_COPY_SRC] === baseVm.uuid
@@ -36,7 +38,9 @@ export class IncrementalXapiWriter extends MixinXapiWriter(AbstractIncrementalWr
       }
     }
   }
-
+  updateUuidAndChain() {
+    // nothing to do, the chaining is not modified in this case
+  }
   prepare({ isFull }) {
     // create the task related to this export and ensure all methods are called in this context
     const task = new Task({

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

@@ -5,6 +5,10 @@ export class AbstractIncrementalWriter extends AbstractWriter {
     throw new Error('Not implemented')
   }
 
+  updateUuidAndChain() {
+    throw new Error('Not implemented')
+  }
+
   cleanup() {
     throw new Error('Not implemented')
   }

@xen-orchestra/backups/docs/VM backups/README.md

@@ -230,6 +230,7 @@ Settings are described in [`@xen-orchestra/backups/\_runners/VmsXapi.mjs``](http
     - `checkBaseVdis(baseUuidToSrcVdi, baseVm)`
     - `prepare({ isFull })`
     - `transfer({ timestamp, deltaExport, sizeContainers })`
+    - `updateUuidAndChain({ isVhdDifferencing, vdis })`
     - `cleanup()`
     - `healthCheck()` // is not executed if no health check sr or tag doesn't match
   - **Full**

@xen-orchestra/lite/src/libs/xapi-stats.ts

@@ -50,7 +50,17 @@ const RRD_POINTS_PER_STEP: { [key in RRD_STEP]: number } = {
 // Utils
 // -------------------------------------------------------------------
 
-function convertNanToNull(value: number) {
+function parseNumber(value: number | string) {
+  // Starting from XAPI 23.31, numbers in the JSON payload are encoded as
+  // strings to support NaN, Infinity and -Infinity
+  if (typeof value === 'string') {
+    const asNumber = +value
+    if (isNaN(asNumber) && value !== 'NaN') {
+      throw new Error('cannot parse number: ' + value)
+    }
+    value = asNumber
+  }
+
   return isNaN(value) ? null : value
 }
 
@@ -59,7 +69,7 @@ function convertNanToNull(value: number) {
 // -------------------------------------------------------------------
 
 const computeValues = (dataRow: any, legendIndex: number, transformValue = identity) =>
-  map(dataRow, ({ values }) => transformValue(convertNanToNull(values[legendIndex])))
+  map(dataRow, ({ values }) => transformValue(parseNumber(values[legendIndex])))
 
 const createGetProperty = (obj: object, property: string, defaultValue: unknown) =>
   defaults(obj, { [property]: defaultValue })[property] as any
@@ -319,8 +329,14 @@ export default class XapiStats {
       },
       abortSignal,
     })
-    // eslint-disable-next-line import/no-named-as-default-member -- https://github.com/json5/json5/issues/287
-    return JSON5.parse(await resp.text())
+    const text = await resp.text()
+    try {
+      // starting from XAPI 23.31, the response is valid JSON
+      return JSON.parse(text)
+    } catch (error) {
+      // eslint-disable-next-line import/no-named-as-default-member -- https://github.com/json5/json5/issues/287
+      return JSON5.parse(text)
+    }
   }
 
   // To avoid multiple requests, we keep a cache for the stats and
@@ -383,7 +399,10 @@ export default class XapiStats {
         abortSignal,
       })
 
-      const actualStep = json.meta.step as number
+      const actualStep = parseNumber(json.meta.step)
+      if (actualStep !== step) {
+        throw new FaultyGranularity(`Unable to get the true granularity: ${actualStep}`)
+      }
 
       if (json.data.length > 0) {
         // fetched data is organized from the newest to the oldest
@@ -407,14 +426,15 @@ export default class XapiStats {
 
           let stepStats = xoObjectStats[actualStep]
           let cacheStepStats = cacheXoObjectStats[actualStep]
-          if (stepStats === undefined || stepStats.endTimestamp !== json.meta.end) {
+          const endTimestamp = parseNumber(json.meta.end)
+          if (stepStats === undefined || stepStats.endTimestamp !== endTimestamp) {
             stepStats = xoObjectStats[actualStep] = {
-              endTimestamp: json.meta.end,
+              endTimestamp,
               interval: actualStep,
               canBeExpired: false,
             }
             cacheStepStats = cacheXoObjectStats[actualStep] = {
-              endTimestamp: json.meta.end,
+              endTimestamp,
               interval: actualStep,
               canBeExpired: true,
             }
@@ -438,10 +458,6 @@ export default class XapiStats {
           })
         })
       }
-
-      if (actualStep !== step) {
-        throw new FaultyGranularity(`Unable to get the true granularity: ${actualStep}`)
-      }
     } catch (error) {
       if (error instanceof Error && error.name === 'AbortError') {
         return

CHANGELOG.unreleased.md

@@ -9,6 +9,10 @@
 
 - Disable search engine indexing via a `robots.txt`
 - [Stats] Support format used by XAPI 23.31
+- [REST API] Export host [SMT](https://en.wikipedia.org/wiki/Simultaneous_multithreading) status at `/hosts/:id/smt` [Forum#71374](https://xcp-ng.org/forum/post/71374)
+- [Home & REST API] `$container` field of an halted VM now points to a host if a VDI is on a local storage [Forum#71769](https://xcp-ng.org/forum/post/71769)
+- [Size Input] Ability to select two new units in the dropdown (`TiB`, `PiB`) (PR [#7382](https://github.com/vatesfr/xen-orchestra/pull/7382))
+
 
 ### Bug fixes
 
@@ -17,6 +21,8 @@
 - [Settings/XO Config] Sort backups from newest to oldest
 - [Plugins/audit] Don't log `tag.getAllConfigured` calls
 - [Remotes] Correctly clear error when the remote is tested with success
+- [Import/VMWare] Fix importing last snapshot (PR [#7370](https://github.com/vatesfr/xen-orchestra/pull/7370))
+- [Host/Reboot] Fix false positive warning when restarting an host after updates (PR [#7366](https://github.com/vatesfr/xen-orchestra/pull/7366))
 
 ### Packages to release
 
@@ -40,6 +46,6 @@
 - vhd-lib patch
 - xo-server minor
 - xo-server-audit patch
-- xo-web patch
+- xo-web minor
 
 <!--packages-end-->

CONTRIBUTING.md

@@ -1,127 +0,0 @@
-# Contributing to Xen Orchestra
-
-First off, thanks for taking the time to contribute! ❤️
-
-All types of contributions are encouraged and valued. See the [Table of Contents](#table-of-contents) for different ways to help and details about how this project handles them. Please make sure to read the relevant section before making your contribution. It will make it a lot easier for us maintainers and smooth out the experience for all involved. The community looks forward to your contributions. 🎉
-
-> And if you like the project, but just don't have time to contribute, that's fine. There are other easy ways to support the project and show your appreciation, which we would also be very happy about:
->
-> - Star the project
-> - Tweet about it
-> - Refer this project in your project's readme
-> - Mention the project at local meetups and tell your friends/colleagues
-
-## Table of Contents
-
-- [Code of Conduct](#code-of-conduct)
-- [I Have a Question](#i-have-a-question)
-- [I Want To Contribute](#i-want-to-contribute)
-- [Reporting Bugs](#reporting-bugs)
-- [Suggesting Enhancements](#suggesting-enhancements)
-- [Your First Code Contribution](#your-first-code-contribution)
-- [Improving The Documentation](#improving-the-documentation)
-- [Styleguides](#styleguides)
-- [Commit Messages](#commit-messages)
-- [Join The Project Team](#join-the-project-team)
-
-## Code of Conduct
-
-This project and everyone participating in it is governed by the
-[Xen Orchestra Code of Conduct](https://github.com/vatesfr/xen-orchestra/blob/master/CODE_OF_CONDUCT.md).
-By participating, you are expected to uphold this code. Please report unacceptable behavior
-to julien.fontanet@vates.fr.
-
-## I Have a Question
-
-> If you want to ask a question, we assume that you have read the available [Documentation](https://xen-orchestra.com/docs/).
-
-Before you ask a question, it is best to search for existing [topics on our forum](https://xcp-ng.org/forum/category/12/xen-orchestra) that might help you. In case you have found a suitable topic and still need clarification, you can write your question in this thread. It is also advisable to search the internet for answers first.
-
-If you then still feel the need to ask a question and need clarification, we recommend the following:
-
-- Open a new Topic.
-- Provide as much context as you can about what you're running into.
-- Provide project and platform versions (nodejs, npm, etc), depending on what seems relevant.
-
-The community will then answer you as soon as possible.
-
-## I Want To Contribute
-
-> ### Legal Notice
->
-> When contributing to this project, you must agree that you have authored 100% of the content, that you have the necessary rights to the content and that the content you contribute may be provided under the project license.
-
-### Reporting Bugs
-
-#### Before Submitting a Bug Report
-
-A good bug report shouldn't leave others needing to chase you up for more information. Therefore, we ask you to investigate carefully, collect information and describe the issue in detail in your report. Please complete the following steps in advance to help us fix any potential bug as fast as possible.
-
-- Make sure that you are using the latest version.
-- Determine if your bug is really a bug and not an error on your side e.g. using incompatible environment components/versions (Make sure that you have read the [documentation](https://xen-orchestra.com/docs/). If you are looking for support, you might want to check [this section](#i-have-a-question)).
-- To see if other users have experienced (and potentially already solved) the same issue you are having, check if there is not already a bug report existing for your bug or error in the [bug tracker](https://github.com/vatesfr/xen-orchestra/issues?q=label%3Abug).
-- Also make sure to search the internet (including Stack Overflow) to see if users outside of the GitHub community have discussed the issue.
-- Collect information about the bug:
-- Stack trace (Traceback)
-- OS, Platform and Version (Windows, Linux, macOS, x86, ARM)
-- Version of the interpreter, compiler, SDK, runtime environment, package manager, depending on what seems relevant.
-- Possibly your input and the output
-- Can you reliably reproduce the issue? And can you also reproduce it with older versions?
-
-#### How Do I Submit a Good Bug Report?
-
-> You must never report security related issues, vulnerabilities or bugs including sensitive information to the issue tracker, or elsewhere in public. Instead sensitive bugs must be reported on [this dedicated page](https://github.com/vatesfr/xen-orchestra/security/advisories/new).
-
-We use GitHub issues to track bugs and errors. If you run into an issue with the project:
-
-- Open an [Issue](https://github.com/vatesfr/xen-orchestra/issues/new/choose). (Since we can't be sure at this point whether it is a bug or not, we ask you not to talk about a bug yet and not to label the issue.)
-- Explain the behavior you would expect and the actual behavior.
-- Please provide as much context as possible and describe the _reproduction steps_ that someone else can follow to recreate the issue on their own. This usually includes your code. For good bug reports you should isolate the problem and create a reduced test case.
-- Provide the information you collected in the previous section.
-
-### Suggesting Enhancements
-
-This section guides you through submitting an enhancement suggestion for Xen Orchestra, **including completely new features and minor improvements to existing functionality**. Following these guidelines will help maintainers and the community to understand your suggestion and find related suggestions.
-
-#### Before Submitting an Enhancement
-
-- Make sure that you are using the latest version.
-- Read the [documentation](https://xen-orchestra.com/docs/) carefully and find out if the functionality is already covered, maybe by an individual configuration.
-- Perform a [search](https://github.com/vatesfr/xen-orchestra/issues) to see if the enhancement has already been suggested. If it has, add a comment to the existing issue instead of opening a new one.
-- Find out whether your idea fits with the scope and aims of the project. It's up to you to make a strong case to convince the project's developers of the merits of this feature. Keep in mind that we want features that will be useful to the majority of our users and not just a small subset. If you're just targeting a minority of users, consider writing an add-on/plugin library.
-
-#### How Do I Submit a Good Enhancement Suggestion?
-
-Enhancement suggestions are tracked as [GitHub issues](https://github.com/vatesfr/xen-orchestra/issues).
-
-- Use a **clear and descriptive title** for the issue to identify the suggestion.
-- Provide a **step-by-step description of the suggested enhancement** in as many details as possible.
-- **Describe the current behavior** and **explain which behavior you expected to see instead** and why. At this point you can also tell which alternatives do not work for you.
-- You may want to **include screenshots and animated GIFs** which help you demonstrate the steps or point out the part which the suggestion is related to. You can use [this tool](https://www.cockos.com/licecap/) to record GIFs on macOS and Windows, and [this tool](https://github.com/colinkeenan/silentcast) or [this tool](https://github.com/GNOME/byzanz) on Linux.
-- **Explain why this enhancement would be useful** to most Xen Orchestra users. You may also want to point out the other projects that solved it better and which could serve as inspiration.
-
-### Your First Code Contribution
-
-<!-- TODO
-include Setup of env, IDE and typical getting started instructions?
-
--->
-
-### Improving The Documentation
-
-<!-- TODO
-Updating, improving and correcting the documentation
-
--->
-
-## Styleguides
-
-### Commit Messages
-
-<!-- TODO
-
--->
-
-## Attribution
-
-This guide is based on the **contributing-gen**. [Make your own](https://github.com/bttger/contributing-gen)!

docs/xoa.md

@@ -93,6 +93,21 @@ Follow the instructions:
 
 You can also download XOA from xen-orchestra.com in an XVA file. Once you've got the XVA file, you can import it with `xe vm-import filename=xoa_unified.xva` or via XenCenter.
 
+If you want to use static IP address for your appliance:
+
+```sh
+xe vm-param-set uuid="$uuid" \
+  xenstore-data:vm-data/ip="$ip" \
+  xenstore-data:vm-data/netmask="$netmask" \
+  xenstore-data:vm-data/gateway="$gateway"
+```
+
+If you want to replace the default DNS server:
+
+```sh
+xe vm-param-set uuid="$uuid" xenstore-data:vm-data/dns="$dns"
+```
+
 After the VM is imported, you just need to start it with `xe vm-start vm="XOA"` or with XenCenter.
 
 ## First console connection

packages/vhd-lib/Vhd/VhdSynthetic.js

@@ -46,9 +46,9 @@ const VhdSynthetic = class VhdSynthetic extends VhdAbstract {
   }
 
   get compressionType() {
-    const compressionType = this.vhds[0].compressionType
-    for (let i = 0; i < this.vhds.length; i++) {
-      if (compressionType !== this.vhds[i].compressionType) {
+    const compressionType = this.#vhds[0].compressionType
+    for (let i = 0; i < this.#vhds.length; i++) {
+      if (compressionType !== this.#vhds[i].compressionType) {
         return 'MIXED'
       }
     }

packages/xo-server/src/api/host.mjs

@@ -1,3 +1,4 @@
+import semver from 'semver'
 import { createLogger } from '@xen-orchestra/log'
 import assert from 'assert'
 import { format } from 'json-rpc-peer'
@@ -136,13 +137,38 @@ export async function restart({
     const pool = this.getObject(host.$poolId, 'pool')
     const master = this.getObject(pool.master, 'host')
     const hostRebootRequired = host.rebootRequired
-    if (hostRebootRequired && host.id !== master.id && host.version === master.version) {
-      throw incorrectState({
-        actual: hostRebootRequired,
-        expected: false,
-        object: master.id,
-        property: 'rebootRequired',
-      })
+
+    // we are currently in an host upgrade process
+    if (hostRebootRequired && host.id !== master.id) {
+      // this error is not ideal but it means that the pool master must be fully upgraded/rebooted before the current host can be rebooted.
+      //
+      // there is a single error for the 3 cases because the client must handle them the same way
+      const throwError = () =>
+        incorrectState({
+          actual: hostRebootRequired,
+          expected: false,
+          object: master.id,
+          property: 'rebootRequired',
+        })
+
+      if (semver.lt(master.version, host.version)) {
+        log.error(`master version (${master.version}) is older than the host version (${host.version})`, {
+          masterId: master.id,
+          hostId: host.id,
+        })
+        throwError()
+      }
+
+      if (semver.eq(master.version, host.version)) {
+        if ((await this.getXapi(host).listMissingPatches(master._xapiId)).length > 0) {
+          log.error('master has missing patches', { masterId: master.id })
+          throwError()
+        }
+        if (master.rebootRequired) {
+          log.error('master needs to reboot')
+          throwError()
+        }
+      }
     }
   }
 

packages/xo-server/src/xapi-object-to-xo.mjs

@@ -328,6 +328,34 @@ const TRANSFORMS = {
 
     const { creation } = xoData.extract(obj) ?? {}
 
+    let $container
+    if (obj.resident_on !== 'OpaqueRef:NULL') {
+      // resident_on is set when the VM is running (or paused or suspended on a host)
+      $container = link(obj, 'resident_on')
+    } else {
+      // if the VM is halted, the $container is the pool
+      $container = link(obj, 'pool')
+
+      // unless one of its VDI is on a non shared SR
+      //
+      // linked objects may not be there when this code run, and it will only be
+      // refreshed when the VM XAPI record change, this value is not guaranteed
+      // to be up-to-date, but it practice it appears to work fine thanks to
+      // `VBDs` and `current_operations` changing when a VDI is
+      // added/removed/migrated
+      for (const vbd of obj.$VBDs) {
+        const sr = vbd?.$VDI?.$SR
+        if (sr !== undefined && !sr.shared) {
+          const pbd = sr.$PBDs[0]
+          const hostId = pbd && link(pbd, 'host')
+          if (hostId !== undefined) {
+            $container = hostId
+            break
+          }
+        }
+      }
+    }
+
     const vm = {
       // type is redefined after for controllers/, templates &
       // snapshots.
@@ -422,8 +450,7 @@ const TRANSFORMS = {
       xenTools,
       ...getVmGuestToolsProps(obj),
 
-      // TODO: handle local VMs (`VM.get_possible_hosts()`).
-      $container: isRunning ? link(obj, 'resident_on') : link(obj, 'pool'),
+      $container,
       $VBDs: link(obj, 'VBDs'),
 
       // TODO: dedupe

packages/xo-server/src/xo-mixins/migrate-vm.mjs

@@ -280,7 +280,7 @@ export default class MigrateVm {
             const stream = vhd.stream()
             await vdi.$importContent(stream, { format: VDI_FORMAT_VHD })
           }
-          return vhd
+          return { vdi, vhd }
         })
       )
     )

packages/xo-server/src/xo-mixins/remotes.mjs

@@ -1,5 +1,6 @@
 import asyncMapSettled from '@xen-orchestra/async-map/legacy.js'
 import { basename } from 'path'
+import { createLogger } from '@xen-orchestra/log'
 import { format, parse } from 'xo-remote-parser'
 import {
   DEFAULT_ENCRYPTION_ALGORITHM,
@@ -17,17 +18,35 @@ import { Remotes } from '../models/remote.mjs'
 
 // ===================================================================
 
+const { warn } = createLogger('xo:mixins:remotes')
+
 const obfuscateRemote = ({ url, ...remote }) => {
   const parsedUrl = parse(url)
   remote.url = format(sensitiveValues.obfuscate(parsedUrl))
   return remote
 }
 
-function validatePath(url) {
-  const { path } = parse(url)
+// these properties should be defined on the remote object itself and not as
+// part of the remote URL
+//
+// there is a bug somewhere that keep putting them into the URL, this list
+// is here to help track it
+const INVALID_URL_PARAMS = ['benchmarks', 'id', 'info', 'name', 'proxy', 'enabled', 'error', 'url']
+
+function validateUrl(url) {
+  const parsedUrl = parse(url)
+
+  const { path } = parsedUrl
   if (path !== undefined && basename(path) === 'xo-vm-backups') {
     throw invalidParameters('remote url should not end with xo-vm-backups')
   }
+
+  for (const param of INVALID_URL_PARAMS) {
+    if (Object.hasOwn(parsedUrl, param)) {
+      // log with stack trace
+      warn(new Error('invalid remote URL param ' + param))
+    }
+  }
 }
 
 export default class {
@@ -182,6 +201,22 @@ export default class {
     if (remote === undefined) {
       throw noSuchObject(id, 'remote')
     }
+
+    const parsedUrl = parse(remote.url)
+    let fixed = false
+    for (const param of INVALID_URL_PARAMS) {
+      if (Object.hasOwn(parsedUrl, param)) {
+        // delete the value to trace its real origin when it's added back
+        // with `updateRemote()`
+        delete parsedUrl[param]
+        fixed = true
+      }
+    }
+    if (fixed) {
+      remote.url = format(parsedUrl)
+      this._remotes.update(remote).catch(warn)
+    }
+
     return remote
   }
 
@@ -202,7 +237,7 @@ export default class {
   }
 
   async createRemote({ name, options, proxy, url }) {
-    validatePath(url)
+    validateUrl(url)
 
     const params = {
       enabled: false,
@@ -219,6 +254,10 @@ export default class {
   }
 
   updateRemote(id, { enabled, name, options, proxy, url }) {
+    if (url !== undefined) {
+      validateUrl(url)
+    }
+
     const handlers = this._handlers
     const handler = handlers[id]
     if (handler !== undefined) {
@@ -238,7 +277,7 @@ export default class {
   @synchronized()
   async _updateRemote(id, { url, ...props }) {
     if (url !== undefined) {
-      validatePath(url)
+      validateUrl(url)
     }
 
     const remote = await this._getRemote(id)

packages/xo-server/src/xo-mixins/rest-api.mjs

@@ -253,6 +253,10 @@ export default class RestApi {
           const host = req.xapiObject
           res.json(await host.$xapi.listMissingPatches(host))
         },
+
+        async smt({ xapiObject }, res) {
+          res.json({ enabled: await xapiObject.$xapi.isHyperThreadingEnabled(xapiObject.$id) })
+        },
       }
 
       collections.pools.routes = {

packages/xo-web/src/common/form/index.js

@@ -138,7 +138,7 @@ export class Range extends Component {
 
 export Toggle from './toggle'
 
-const UNITS = ['kiB', 'MiB', 'GiB']
+const UNITS = ['kiB', 'MiB', 'GiB', 'TiB', 'PiB']
 const DEFAULT_UNIT = 'GiB'
 
 export class SizeInput extends BaseComponent {