refix

@xen-orchestra/backups/_runners/VmsXapi.mjs

@@ -111,93 +111,99 @@ export const VmsXapi = class VmsXapiBackupRunner extends Abstract {
 
         const preTakenTimestampByVmId = {}
         const failedSnapshotByVmId = {}
-        if (settings.synchronizedSnapshot) {
-          await Task.run({ properties: { name: 'snapshot VMs' } }, async () => {
-            await Disposable.use(
-              Disposable.all(vmIds.map(vmId => this._getRecord('VM', vmId).catch(noop))),
-              async vms => {
-                // remove vms that failed (already handled)
-                vms = vms.filter(_ => _ !== undefined)
-
-                const batchIds = selectSynchronizedSnapshotVms(settings.synchronizedSnapshot, vms)
-
-                await asyncEach(
-                  [...vms].filter(vm => batchIds.has(vm.uuid)),
-                  async vm => {
-                    const vmSettings = { ...settings, ...allSettings[vm.uuid] }
-                    const opts = {
-                      baseSettings,
-                      config,
-                      getSnapshotNameLabel,
-                      healthCheckSr,
-                      job,
-                      remoteAdapters,
-                      schedule,
-                      settings: vmSettings,
-                      srs,
-                      throttleGenerator,
-                      throttleStream,
-                      vm,
-                    }
-
-                    let vmBackup
-                    try {
-                      vmBackup = this._getVmBackup(job.mode, opts)
-                      if (
-                        !vmBackup._settings.offlineBackup &&
-                        !vmBackup._settings.offlineSnapshot &&
-                        (await vmBackup._mustDoSnapshot())
-                      ) {
-                        await vmBackup._prepareAndSnapshot()
-                        preTakenTimestampByVmId[vm.uuid] = vmBackup.timestamp
-                      }
-                    } catch (error) {
-                      failedSnapshotByVmId[vm.uuid] = error
-                      if (vmBackup !== undefined) {
-                        try {
-                          await vmBackup._fetchJobSnapshots()
-                          await vmBackup._removeUnusedSnapshots()
-                          await vmBackup._cleanMetadata()
-                        } catch (cleanupError) {
-                          // Best effort cleanup
-                        }
-                      }
-                    }
-                  },
-                  {
-                    concurrency: settings.snapshotConcurrency,
-                  }
-                )
-              }
-            )
-          })
-        }
-
-        const snapshotedVmIds = new Set(vmIds.filter(id => !(id in failedSnapshotByVmId)))
-        Object.entries(failedSnapshotByVmId).forEach(([vmId, error]) => {
-          Task.run(
-            {
-              properties: { id: vmId, name: 'backup VM', type: 'VM' },
-            },
-            () => Promise.reject(error)
-          ).catch(noop)
-        })
-
-        const queue = new Set(snapshotedVmIds)
         const taskByVmId = {}
         const nTriesByVmId = {}
 
-        const handleVm = vmUuid => {
-          const getVmTask = () => {
-            const started = taskByVmId[vmUuid] !== undefined
-            if (!started) {
-              taskByVmId[vmUuid] = new Task(taskStart)
-            }
-            return {
-              task: taskByVmId[vmUuid],
-              started,
-            }
+        const getVmTask = (vmUuid, name_label) => {
+          const started = taskByVmId[vmUuid] !== undefined
+          if (!started) {
+            taskByVmId[vmUuid] = new Task({ properties: { id: vmUuid, name: 'backup VM', type: 'VM', name_label } })
           }
+          return {
+            task: taskByVmId[vmUuid],
+            started,
+          }
+        }
+
+        if (settings.synchronizedSnapshot) {
+          await Disposable.use(
+            Disposable.all(vmIds.map(vmId => this._getRecord('VM', vmId).catch(noop))),
+            async vms => {
+              // remove vms that failed (already handled)
+              vms = vms.filter(_ => _ !== undefined)
+
+              const batchIds = selectSynchronizedSnapshotVms(settings.synchronizedSnapshot, vms)
+
+              if (batchIds.size > 0) {
+                Task.info('synchronized snapshot', { vms: Array.from(batchIds) })
+              }
+
+              await asyncEach(
+                [...vms].filter(vm => batchIds.has(vm.uuid)),
+                async vm => {
+                  // The task deliverately includes both the transfer (including the wait) and and backup.
+                  // This is done to have the snapshot reported under the VM.
+                  const { task } = getVmTask(vm.uuid, vm.name_label)
+
+                  const vmSettings = { ...settings, ...allSettings[vm.uuid] }
+                  const opts = {
+                    baseSettings,
+                    config,
+                    getSnapshotNameLabel,
+                    healthCheckSr,
+                    job,
+                    remoteAdapters,
+                    schedule,
+                    settings: vmSettings,
+                    srs,
+                    throttleGenerator,
+                    throttleStream,
+                    vm,
+                  }
+
+                  await task
+                    .runInside(async () => {
+                      let vmBackup
+                      try {
+                        vmBackup = this._getVmBackup(job.mode, opts)
+                        if (
+                          !vmBackup._settings.offlineBackup &&
+                          !vmBackup._settings.offlineSnapshot &&
+                          (await vmBackup._mustDoSnapshot())
+                        ) {
+                          await vmBackup._prepareAndSnapshot()
+                          preTakenTimestampByVmId[vm.uuid] = vmBackup.timestamp
+                        }
+                      } catch (error) {
+                        failedSnapshotByVmId[vm.uuid] = error
+                        if (vmBackup !== undefined) {
+                          try {
+                            await vmBackup._fetchJobSnapshots()
+                            await vmBackup._removeUnusedSnapshots()
+                            await vmBackup._cleanMetadata()
+                          } catch (cleanupError) {
+                            // Best effort cleanup
+                          }
+                        }
+
+                        throw error
+                      }
+                    })
+                    .catch(noop)
+                },
+                {
+                  concurrency: settings.snapshotConcurrency,
+                }
+              )
+            }
+          )
+        }
+
+        const snapshotedVmIds = new Set(vmIds.filter(id => !(id in failedSnapshotByVmId)))
+
+        const queue = new Set(snapshotedVmIds)
+
+        const handleVm = vmUuid => {
           const vmBackupFailed = async (error, task) => {
             if (isLastRun) {
               return task.failure(error)
@@ -218,17 +224,13 @@ export const VmsXapi = class VmsXapiBackupRunner extends Abstract {
           nTriesByVmId[vmUuid]++
 
           const vmSettings = { ...settings, ...allSettings[vmUuid] }
-          const taskStart = { properties: { id: vmUuid, name: 'backup VM', type: 'VM' } }
           const isLastRun = nTriesByVmId[vmUuid] === vmSettings.nRetriesVmBackupFailures + 1
 
           return this._getRecord('VM', vmUuid).then(
             disposableVm =>
               Disposable.use(disposableVm, async vm => {
-                if (taskStart.properties.name_label === undefined) {
-                  taskStart.properties.name_label = vm.name_label
-                }
+                const { task } = getVmTask(vmUuid, vm.name_label)
 
-                const { task } = getVmTask()
                 // error has to be caught in the task to prevent its failure, but handled outside the task to execute another task.run()
                 let taskError
                 return task
@@ -268,7 +270,7 @@ export const VmsXapi = class VmsXapiBackupRunner extends Abstract {
                   .catch(noop) // errors are handled by logs
               }),
             error => {
-              const { task: vmTask, started } = getVmTask()
+              const { task: vmTask, started } = getVmTask(vmUuid)
               if (!started) {
                 // the task is not started (except if it's a retry), and an unstarted task can't be failed
                 vmTask.start()

@xen-orchestra/backups/_runners/_selectSynchronizedSnapshotVms.mjs

@@ -8,6 +8,10 @@ export function selectSynchronizedSnapshotVms(synchronizedSnapshot, vms) {
   let matches
   if (typeof synchronizedSnapshot === 'string') {
     matches = vms.filter(vm => vm.tags.includes(synchronizedSnapshot))
+
+    if (matches.length === 0) {
+      Task.warning('no vm matched the synchronizedSnapshot tag', { synchronizedSnapshot })
+    }
   } else if (synchronizedSnapshot === true) {
     matches = vms
   } else {

@xen-orchestra/qa-test/tests/backup.synchronized.test.js

@@ -7,7 +7,7 @@ import { FilterBuilder } from '../client/FilterBuilder.js'
 import {
   assertFullOrDelta,
   assertFullOrDeltaForSr,
-  findTaskByMessage,
+  findInfoByMessage,
   generateBackupJobName,
   getDefaultSchedule,
   getScheduleKey,
@@ -134,7 +134,10 @@ describe('Backup basic tests', () => {
       assertBackupSuccess(result, 'Synchronized full backup')
       assertFullOrDelta(result, backupRepository.id, { mustBeFull: true })
 
-      assert(findTaskByMessage(result, 'snapshot VMs') === null, `No batch snapshot should have been performed`)
+      assert(
+        findInfoByMessage(result, 'synchronized snapshot') === null,
+        `No batch snapshot should have been performed`
+      )
     })
 
     it('should run a synchronized delta backup and reuse the batch snapshots as the delta base on the second run', async () => {

@xen-orchestra/qa-test/utils/backupUtils.js

@@ -1,5 +1,5 @@
 import { createLogger } from '@xen-orchestra/log'
-import { findTaskByMessage } from './index.js'
+import { findInfoByMessage } from './index.js'
 import assert from 'node:assert'
 import { getSyncedHandler } from '@xen-orchestra/fs'
 
@@ -167,11 +167,12 @@ export const assertBackupSuccess = (result, context = 'Backup') => {
  * @param {number} vmCount - Number of VMs concerned by this backup job
  */
 export const assertSynchronizedSnapshot = (result, vmCount) => {
-  const snapshotVmTask = findTaskByMessage(result, 'snapshot VMs')
-  assert(snapshotVmTask, `Synchronized backup should have batched snapshots`)
+  const synchronizedInfo = findInfoByMessage(result, 'synchronized snapshot')
+  assert(synchronizedInfo, `Synchronized backup should have batched snapshots`)
+  const batchedVms = synchronizedInfo.data?.vms
   assert(
-    snapshotVmTask.tasks && snapshotVmTask.tasks.length === vmCount,
-    `Synchronized backup should have made a snapshot per vm`
+    batchedVms?.length === vmCount,
+    `Synchronized backup should have made a snapshot per vm (expected ${vmCount}, found ${batchedVms?.length})`
   )
 
   // Collect every task with a given message across the whole task tree.
@@ -195,6 +196,10 @@ export const assertSynchronizedSnapshot = (result, vmCount) => {
   // 'snapshot' task per VM (the batched ones) and none taken again during the
   // per-VM backup. A larger count means a VM was snapshotted a second time at
   // transfer time, i.e. the backup did NOT reuse the synchronized snapshot.
+  //
+  // Those tasks are subtasks of the VM they belong to, batched or not, hence
+  // the recursive walk: the batch phase does not have a task of its own, it is
+  // only reported by the 'synchronized snapshot' info above.
   const snapshots = collectByMessage('snapshot')
   assert(
     snapshots.length === vmCount,
@@ -205,5 +210,6 @@ export const assertSynchronizedSnapshot = (result, vmCount) => {
   assert(transfers.length > 0, `Synchronized backup should have transfered snapshots.`)
 
   const earliestStart = Math.min(...transfers.map(t => t.start))
-  assert(snapshotVmTask.end <= earliestStart, `Synchronized backup snapshots should be complete before transfer starts`)
+  const latestSnapshotEnd = Math.max(...snapshots.map(t => t.end))
+  assert(latestSnapshotEnd <= earliestStart, `Synchronized backup snapshots should be complete before transfer starts`)
 }

@xen-orchestra/qa-test/utils/index.js

@@ -345,6 +345,21 @@ export function findTaskByMessage(logEntry, message) {
   return search(logEntry.tasks || [])
 }
 
+/**
+ * Finds the first info matching a given message name in a backup log entry.
+ *
+ * Only the infos of the log itself are searched: the infos emitted on the root
+ * task are merged into it (see `taskFormatAdapter()` in
+ * xo-server/src/xo-mixins/backups-ng-logs.mjs).
+ *
+ * @param {Object} logEntry - Backup log entry from XenOrchestra
+ * @param {string} message - Info message to search for (e.g., 'vms', 'synchronized snapshot')
+ * @returns {Object|null} The matching info or null if not found
+ */
+export function findInfoByMessage(logEntry, message) {
+  return logEntry.infos?.find(info => info.message === message) ?? null
+}
+
 // =============================================================================
 // BACKUP ASSERTION UTILITIES
 // =============================================================================

CHANGELOG.unreleased.md

@@ -16,6 +16,7 @@
 > Users must be able to say: "I had this issue, happy to know it's fixed"
 
 - [Servers] Fix servers stuck in `Connecting` state when using an HTTPS proxy that support HTTP/2 ([Forum#12502](https://xcp-ng.org/forum/topic/12502/xoa-6.9-update)) (PR [#10507](https://github.com/vatesfr/xen-orchestra/pull/10507))
+- [Backups] Fix synchronized backups task log showing VM count+1 (PR [#10518](https://github.com/vatesfr/xen-orchestra/pull/10518))
 
 ### Packages to release
 
@@ -33,6 +34,8 @@
 
 <!--packages-start-->
 
+- @xen-orchestra/backups patch
+- @xen-orchestra/qa-test patch
 - xen-api patch
 
 <!--packages-end-->

docs/docs/backups-and-dr/backup-features-and-settings.md

@@ -304,6 +304,24 @@ Since the VSS provider was removed from the guest tools and the `snapshot_with_q
 
 Both can be applied per VM with [backup modifier tags](#backup-modifier-tags). Old quiesced snapshots taken before the removal are still identified by an "info" icon in the VM's Snapshots tab.
 
+## Synchronized snapshots {#synchronized-snapshots}
+
+A backup job snapshots each VM right before exporting it, one VM after the other. When several VMs form a single system (an application server and its database, the nodes of a cluster...) their restore points end up minutes apart, and restoring the whole set gives you VMs that are not consistent with each other.
+
+Enable **Synchronize snapshots** in the _Advanced settings_ of a backup or replication job to snapshot every VM of the job up front, before the first export starts. All the restore points of a run are then taken within the same short window, so the whole job can be restored as a coherent set.
+
+The snapshots are taken in parallel, up to the job's snapshot concurrency (2 by default), and the VMs are not frozen: this narrows the window between the restore points, it does not make them simultaneous. The snapshot concurrency can be raised to narrow the window.
+
+:::note
+VMs that are exported without a snapshot are not part of the batch: offline backup, offline snapshot, or a halted VM in a full backup.
+:::
+
+Two consequences to keep in mind:
+
+- Each VM keeps its snapshot until its own export runs, so the snapshots taken first live for most of the job and grow as the VMs write to their disks. Expect more space used on the storage repository while the job runs, especially on thick-provisioned SRs.
+- In the backup logs, a VM's task starts when its snapshot is taken instead of when its export starts, so the duration reported for a VM includes the time it waited for its turn to export. The duration of the job itself is unchanged.
+
+
 ## Backup repositories (BR) {#remotes}
 
 Backup repositories (BR), formerly called _remotes_, are places where your _backup_ and _delta backup_ files will be stored.