fix(xo-server,rest-api): apply the backup guard to rolling pool operations started over REST (#10313)

* fix(xo-server,rest-api): apply the backup guard to rolling pool operations started over REST

The JSON-RPC methods pool.rollingUpdate and pool.rollingReboot refused to run
while a backup job targets the pool, but the REST actions
POST /pools/:id/actions/rolling_update and rolling_reboot called the
orchestrators directly and skipped that check.

The guard now lives in XenServers.rollingPoolUpdate and Pools.rollingPoolReboot,
so every entry point (JSON-RPC, REST, scheduled jobs) applies the same rule.
The bypass moves into backupGuard(poolId, { bypassBackupCheck, operation }),
which logs a single warning with the operation and the user, and the other
callers (host.restart, host.restartAgent, host.stop, sr.reclaimSpace and their
REST counterparts) use it instead of their own if/else and log.

The REST rolling actions accept bypassBackupCheck in their body and record the
body in the task params, like the host actions do.

* fix(xo-server,rest-api): pass the targeted object ID to the backup guard

@vates/types/src/xo-app.mts

@@ -201,7 +201,7 @@ export type XoApp = {
     userData?: { ip?: string },
     opts?: { bypassOtp?: boolean; bypassTaskCreation?: boolean }
   ) => Promise<{ bypassOtp: boolean; expiration: number; user: XoUser }>
-  backupGuard(poolId: XoPool['id']): Promise<void>
+  backupGuard(objectId: XapiXoRecord['id'], opts?: { bypassBackupCheck?: boolean; operation: string }): Promise<void>
   /* Throw if no authorization */
   checkFeatureAuthorization(featureCode: FeatureCode): Promise<void>
   /* validate, apply and persist the configuration of a plugin */
@@ -345,10 +345,13 @@ export type XoApp = {
       readOnly?: XoServer['readOnly']
     }
   ): Promise<XoServer>
-  rollingPoolReboot(pool: XoPool, opts?: { parentTask?: VatesTask; shutdownPinnedVms?: boolean }): Promise<void>
+  rollingPoolReboot(
+    pool: XoPool,
+    opts?: { bypassBackupCheck?: boolean; parentTask?: VatesTask; shutdownPinnedVms?: boolean }
+  ): Promise<void>
   rollingPoolUpdate(
     pool: XoPool,
-    opts?: { rebootVm?: boolean; parentTask?: VatesTask; shutdownPinnedVms?: boolean }
+    opts?: { bypassBackupCheck?: boolean; rebootVm?: boolean; parentTask?: VatesTask; shutdownPinnedVms?: boolean }
   ): Promise<void>
   setVmResourceSet(vmId: XoVm['id'], resourceSetId: string | null, force?: boolean): Promise<void>
   shareVmResourceSet(vmId: XoVm['id']): Promise<void>

@xen-orchestra/rest-api/src/abstract-classes/base-controller.mts

@@ -8,7 +8,6 @@ import type { VatesTask } from '@vates/types/lib/vates/task'
 import type { XapiXoRecord, XoRecord, XoTask } from '@vates/types/xo'
 import type { Xapi } from '@vates/types/lib/xen-orchestra/xapi'
 
-import { BASE_URL } from '../index.mjs'
 import { makeMarkdownTable } from '../helpers/markdown.helper.mjs'
 import { makeNdJsonStream } from '../helpers/stream.helper.mjs'
 import { RestApi } from '../rest-api/rest-api.mjs'
@@ -16,7 +15,7 @@ import { makeObjectMapper } from '../helpers/object-wrapper.helper.mjs'
 import type { MaybePromise, SendObjects, WithHref } from '../helpers/helper.type.mjs'
 import type { Response as ExResponse } from 'express'
 import { invalidParameters } from 'xo-common/api-errors.js'
-import { NDJSON_CONTENT_TYPE, safeParseComplexMatcher } from '../helpers/utils.helper.mjs'
+import { BASE_URL, NDJSON_CONTENT_TYPE, safeParseComplexMatcher } from '../helpers/utils.helper.mjs'
 
 const noop = () => {}
 

@xen-orchestra/rest-api/src/acl-roles/acl-role.controller.mts

@@ -38,9 +38,8 @@ import {
   unauthorizedResp,
   Unbrand,
 } from '../open-api/common/response.common.mjs'
-import { BASE_URL } from '../index.mjs'
 import { CreateActionReturnType } from '../abstract-classes/base-controller.mjs'
-import { limitAndFilterArray } from '../helpers/utils.helper.mjs'
+import { BASE_URL, limitAndFilterArray } from '../helpers/utils.helper.mjs'
 import type { SendObjects } from '../helpers/helper.type.mjs'
 import { taskLocation } from '../open-api/oa-examples/task.oa-example.mjs'
 import { XoController } from '../abstract-classes/xo-controller.mjs'

@xen-orchestra/rest-api/src/alarms/alarm.service.mts

@@ -1,8 +1,7 @@
 import { noSuchObject } from 'xo-common/api-errors.js'
 import { XapiXoRecord, XoAlarm, XoMessage } from '@vates/types'
 import { RestApi } from '../rest-api/rest-api.mjs'
-import { BASE_URL } from '../index.mjs'
-import { safeParseComplexMatcher } from '../helpers/utils.helper.mjs'
+import { BASE_URL, safeParseComplexMatcher } from '../helpers/utils.helper.mjs'
 
 // E.g: 'value: 0.6\nconfig:\n<variable>\n<name value="cpu_usage"/>\n<alarm_trigger_level value="0.4"/>\n<alarm_trigger_period value ="60"/>\n</variable>';
 const ALARM_BODY_REGEX = /^value:\s*(Infinity|NaN|-Infinity|\d+(?:\.\d+)?)\s*config:\s*<variable>\s*<name value="(.*?)"/

@xen-orchestra/rest-api/src/backup-jobs/backup-job.controller.mts

@@ -38,7 +38,7 @@ import {
   Unbrand,
 } from '../open-api/common/response.common.mjs'
 import { RestApi } from '../rest-api/rest-api.mjs'
-import { limitAndFilterArray, safeParseComplexMatcher } from '../helpers/utils.helper.mjs'
+import { BASE_URL, limitAndFilterArray, safeParseComplexMatcher } from '../helpers/utils.helper.mjs'
 import type {
   UnbrandAnyXoBackupJob,
   UnbrandXoMetadataBackupJob,
@@ -58,7 +58,6 @@ import {
   vmBackupJob,
   vmBackupJobIds,
 } from '../open-api/oa-examples/backup-job.oa-example.mjs'
-import { BASE_URL } from '../index.mjs'
 import { BackupJobService } from './backup-job.service.mjs'
 
 const log = createLogger('xo:rest-api:backupJob-controller')

@xen-orchestra/rest-api/src/helpers/object-wrapper.helper.mts

@@ -1,7 +1,7 @@
 import pick from 'lodash/pick.js'
 import { Request } from 'express'
 
-import { BASE_URL } from '../index.mjs'
+import { BASE_URL } from './utils.helper.mjs'
 import type { XoRecord } from '@vates/types/xo'
 import type { WithHref } from './helper.type.mjs'
 

@xen-orchestra/rest-api/src/helpers/object-wrapper.helper.test.mts

@@ -3,10 +3,9 @@ import { describe, it } from 'node:test'
 import type { Request } from 'express'
 
 import { makeObjectMapper } from './object-wrapper.helper.mjs'
+import { BASE_URL } from './utils.helper.mjs'
 import type { XoRecord } from '@vates/types'
 
-const BASE_URL = '/rest/v0'
-
 function makeRequest(path: string, fields?: string | string[]): Request {
   return {
     path,

@xen-orchestra/rest-api/src/helpers/utils.helper.mts

@@ -6,6 +6,7 @@ import { isPromise } from 'node:util/types'
 import { MaybePromise, PromiseWriteInStreamError } from './helper.type.mjs'
 import { Writable } from 'node:stream'
 import { ApiError } from './error.helper.mjs'
+export const BASE_URL = '/rest/v0'
 export const NDJSON_CONTENT_TYPE = 'application/x-ndjson'
 
 const log = createLogger('xo:rest-api:utils-helper')

@xen-orchestra/rest-api/src/hosts/host.controller.mts

@@ -751,7 +751,7 @@ export class HostController extends XapiXoController<XoHost> {
     @Query() sync?: boolean
   ): CreateActionReturnType<void> {
     const opts = {
-      bypassBackupCheck: body?.bypassBackupCheck ?? false,
+      bypassBackupCheck: body?.bypassBackupCheck,
       bypassVersionCheck: body?.bypassVersionCheck ?? false,
       bypassBlockedSuspend: body?.bypassBlockedSuspend ?? false,
       bypassCurrentVmCheck: body?.bypassCurrentVmCheck ?? false,

@xen-orchestra/rest-api/src/hosts/host.service.mts

@@ -101,13 +101,10 @@ export class HostService {
       bypassEvacuate?: boolean
     } = {}
   ): Promise<void> {
-    const host = this.#restApi.getObject<XoHost>(hostId)
-
-    if (opts?.bypassBackupCheck) {
-      log.warn('host clean_shutdown called with argument "bypassBackupCheck" set to true', { hostId })
-    } else {
-      await this.#restApi.xoApp.backupGuard(host.$pool)
-    }
+    await this.#restApi.xoApp.backupGuard(hostId, {
+      bypassBackupCheck: opts.bypassBackupCheck,
+      operation: 'host.clean_shutdown',
+    })
 
     await this.#restApi.getXapiObject(hostId, 'host').$xapi.shutdownHost(hostId, opts)
   }
@@ -116,7 +113,7 @@ export class HostService {
     hostId: XoHost['id'],
     opts: {
       force: boolean
-      bypassBackupCheck: boolean
+      bypassBackupCheck?: boolean
       bypassVersionCheck: boolean
     }
   ): Promise<void> {
@@ -128,7 +125,7 @@ export class HostService {
   async smartRebootHost(
     hostId: XoHost['id'],
     opts: {
-      bypassBackupCheck: boolean
+      bypassBackupCheck?: boolean
       bypassVersionCheck: boolean
       bypassBlockedSuspend: boolean
       bypassCurrentVmCheck: boolean
@@ -147,13 +144,10 @@ export class HostService {
       bypassBackupCheck?: boolean
     } = {}
   ): Promise<void> {
-    const host = this.#restApi.getObject<XoHost>(hostId)
-
-    if (opts?.bypassBackupCheck) {
-      log.warn('host.restartAgent called with argument "bypassBackupCheck" set to true', { hostId })
-    } else {
-      await this.#restApi.xoApp.backupGuard(host.$pool)
-    }
+    await this.#restApi.xoApp.backupGuard(hostId, {
+      bypassBackupCheck: opts.bypassBackupCheck,
+      operation: 'host.restartAgent',
+    })
 
     await this.#restApi.getXapiObject<XoHost>(hostId, 'host').$restartAgent()
   }
@@ -161,7 +155,7 @@ export class HostService {
   async #rebootChecks(
     hostId: XoHost['id'],
     opts: {
-      bypassBackupCheck: boolean
+      bypassBackupCheck?: boolean
       bypassVersionCheck: boolean
     }
   ): Promise<{ xapi: Xapi; xapiHost: XenApiHostWrapped }> {
@@ -170,11 +164,10 @@ export class HostService {
     const poolId = host.$pool
     const xapi = xapiHost.$xapi
 
-    if (opts.bypassBackupCheck) {
-      log.warn('host.reboot called with "bypassBackupCheck" set to true', { hostId })
-    } else {
-      await this.#restApi.xoApp.backupGuard(poolId)
-    }
+    await this.#restApi.xoApp.backupGuard(hostId, {
+      bypassBackupCheck: opts.bypassBackupCheck,
+      operation: 'host.reboot',
+    })
 
     if (opts.bypassVersionCheck) {
       log.warn('host.reboot called with "bypassVersionCheck" set to true', { hostId })

@xen-orchestra/rest-api/src/index.mts

@@ -12,6 +12,7 @@ import { logMiddleware } from './middlewares/log.middleware.mjs'
 import { mcpGateMiddleware } from './middlewares/mcp-gate.middleware.mjs'
 import { type OpenAPIV3 } from 'openapi-types'
 import { createExternalRouter, sendObjects } from './router/external-router.mjs'
+import { BASE_URL } from './helpers/utils.helper.mjs'
 
 export { sendObjects }
 
@@ -20,8 +21,6 @@ export { sendObjects }
 const require = createRequire(import.meta.url)
 const swaggerOpenApiSpec = require('../open-api/spec/swagger.json') as OpenAPIV3.Document
 
-export const BASE_URL = '/rest/v0'
-
 const SWAGGER_UI_OPTIONS = {
   swaggerOptions: {
     url: `${BASE_URL}/docs/swagger.json`,

@xen-orchestra/rest-api/src/pools/pool.controller.mts

@@ -81,13 +81,13 @@ import type {
   CreateVmBody,
   CreateVmParams,
   PoolDashboard,
+  RollingPoolActionBody,
 } from './pool.type.mjs'
 import { partialTasks, taskIds, taskLocation } from '../open-api/oa-examples/task.oa-example.mjs'
 import { createNetwork } from '../open-api/oa-examples/schedule.oa-example.mjs'
-import { BASE_URL } from '../index.mjs'
 import { VmService } from '../vms/vm.service.mjs'
 import { PoolService } from './pool.service.mjs'
-import { escapeUnsafeComplexMatcher, NDJSON_CONTENT_TYPE } from '../helpers/utils.helper.mjs'
+import { BASE_URL, escapeUnsafeComplexMatcher, NDJSON_CONTENT_TYPE } from '../helpers/utils.helper.mjs'
 import { messageIds, partialMessages } from '../open-api/oa-examples/message.oa-example.mjs'
 import type { CreateActionReturnType } from '../abstract-classes/base-controller.mjs'
 import { NetworkService } from '../networks/network.service.mjs'
@@ -365,7 +365,7 @@ export class PoolController extends XapiXoController<XoPool> {
    * with an `incorrect state` error listing their UUIDs.
    *
    * @example id "355ee47d-ff4c-4924-3db2-fd86ae629677"
-   * @example body { "shutdownPinnedVms": true }
+   * @example body { "bypassBackupCheck": false, "shutdownPinnedVms": true }
    */
   @Example(taskLocation)
   @Extension('x-mcp-exposure', 'confirm')
@@ -379,14 +379,13 @@ export class PoolController extends XapiXoController<XoPool> {
   @Response(incorrectStateResp.status, incorrectStateResp.description)
   rollingReboot(
     @Path() id: string,
-    @Body() body?: { shutdownPinnedVms?: boolean },
+    @Body() body?: RollingPoolActionBody,
     @Query() sync?: boolean
   ): CreateActionReturnType<void> {
     const poolId = id as XoPool['id']
-    const shutdownPinnedVms = body?.shutdownPinnedVms ?? false
     const action = async (task: VatesTask) => {
       const pool = this.getObject(poolId)
-      await this.restApi.xoApp.rollingPoolReboot(pool, { parentTask: task, shutdownPinnedVms })
+      await this.restApi.xoApp.rollingPoolReboot(pool, { ...body, parentTask: task })
     }
 
     return this.createAction<void>(action, {
@@ -395,6 +394,7 @@ export class PoolController extends XapiXoController<XoPool> {
       taskProperties: {
         name: 'rolling pool reboot',
         objectId: poolId,
+        params: body,
         progress: 0,
       },
     })
@@ -409,7 +409,7 @@ export class PoolController extends XapiXoController<XoPool> {
    * with an `incorrect state` error listing their UUIDs.
    *
    * @example id "355ee47d-ff4c-4924-3db2-fd86ae629677"
-   * @example body { "shutdownPinnedVms": true }
+   * @example body { "bypassBackupCheck": false, "shutdownPinnedVms": true }
    */
   @Example(taskLocation)
   @Extension('x-mcp-exposure', 'confirm')
@@ -423,14 +423,13 @@ export class PoolController extends XapiXoController<XoPool> {
   @Response(incorrectStateResp.status, incorrectStateResp.description)
   rollingUpdate(
     @Path() id: string,
-    @Body() body?: { shutdownPinnedVms?: boolean },
+    @Body() body?: RollingPoolActionBody,
     @Query() sync?: boolean
   ): CreateActionReturnType<void> {
     const poolId = id as XoPool['id']
-    const shutdownPinnedVms = body?.shutdownPinnedVms ?? false
     const action = async (task: VatesTask) => {
       const pool = this.getObject(poolId)
-      await this.restApi.xoApp.rollingPoolUpdate(pool, { parentTask: task, shutdownPinnedVms })
+      await this.restApi.xoApp.rollingPoolUpdate(pool, { ...body, parentTask: task })
     }
 
     return this.createAction<void>(action, {
@@ -439,6 +438,7 @@ export class PoolController extends XapiXoController<XoPool> {
       taskProperties: {
         name: 'rolling pool update',
         objectId: poolId,
+        params: body,
         progress: 0,
       },
     })

@xen-orchestra/rest-api/src/pools/pool.controller.test.mts

@@ -0,0 +1,51 @@
+import assert from 'node:assert/strict'
+import { describe, it } from 'node:test'
+import type { XoApp } from '@vates/types/xo-app'
+import type { XoPool } from '@vates/types'
+
+import { PoolController } from './pool.controller.mjs'
+import type { RestApi } from '../rest-api/rest-api.mjs'
+
+const pool = { id: 'pool-1', name_label: 'pool 1' } as XoPool
+
+type RollingOpts = NonNullable<Parameters<XoApp['rollingPoolUpdate']>[1]>
+
+function createController() {
+  const calls: unknown[] = []
+  const record =
+    (orchestrator: string) =>
+    async (_pool: XoPool, { parentTask, ...opts }: RollingOpts) => {
+      calls.push({ orchestrator, parentTask: parentTask?.id, ...opts })
+    }
+  const restApi = {
+    getObject: () => pool,
+    tasks: { create: (properties: unknown) => ({ id: 'task-1', properties, run: (fn: () => unknown) => fn() }) },
+    xoApp: {
+      rollingPoolReboot: record('rollingPoolReboot'),
+      rollingPoolUpdate: record('rollingPoolUpdate'),
+    },
+  } as unknown as RestApi
+  // the injected services are not used by the rolling actions
+  const controller = new PoolController(restApi, {} as never, {} as never, {} as never, {} as never)
+  return { calls, controller }
+}
+
+for (const [method, orchestrator] of [
+  ['rollingReboot', 'rollingPoolReboot'],
+  ['rollingUpdate', 'rollingPoolUpdate'],
+] as const) {
+  describe(`PoolController.${method}`, () => {
+    it('forwards bypassBackupCheck from the body to the orchestrator', async () => {
+      const { calls, controller } = createController()
+
+      await controller[method]('pool-1', undefined, true)
+      await controller[method]('pool-1', { bypassBackupCheck: true, shutdownPinnedVms: true }, true)
+
+      // the REST task is the parent of the orchestrator's task tree
+      assert.deepEqual(calls, [
+        { orchestrator, parentTask: 'task-1' },
+        { orchestrator, parentTask: 'task-1', bypassBackupCheck: true, shutdownPinnedVms: true },
+      ])
+    })
+  })
+}

@xen-orchestra/rest-api/src/pools/pool.type.mts

@@ -138,3 +138,9 @@ export type PoolDashboard = {
     percent: number
   }
 }
+
+export type RollingPoolActionBody = {
+  /** Skip the backup safety check before rebooting the hosts. Defaults to false. */
+  bypassBackupCheck?: boolean
+  shutdownPinnedVms?: boolean
+}

@xen-orchestra/rest-api/src/srs/sr.controller.mts

@@ -24,8 +24,7 @@ import { SUPPORTED_VDI_FORMAT } from '@vates/types'
 
 import { acl } from '../middlewares/acl.middleware.mjs'
 import { AlarmService } from '../alarms/alarm.service.mjs'
-import { BASE_URL } from '../index.mjs'
-import { escapeUnsafeComplexMatcher } from '../helpers/utils.helper.mjs'
+import { BASE_URL, escapeUnsafeComplexMatcher } from '../helpers/utils.helper.mjs'
 import { genericAlarmsExample } from '../open-api/oa-examples/alarm.oa-example.mjs'
 import {
   asynchronousActionResp,

@xen-orchestra/rest-api/src/vbds/vbd.controller.mts

@@ -24,7 +24,7 @@ import type { Xapi, XoAlarm, XoMessage, XoTask, XoVbd, XoVdi, XoVm } from '@vate
 
 import { acl } from '../middlewares/acl.middleware.mjs'
 import { AlarmService } from '../alarms/alarm.service.mjs'
-import { escapeUnsafeComplexMatcher } from '../helpers/utils.helper.mjs'
+import { BASE_URL, escapeUnsafeComplexMatcher } from '../helpers/utils.helper.mjs'
 import { genericAlarmsExample } from '../open-api/oa-examples/alarm.oa-example.mjs'
 import {
   asynchronousActionResp,
@@ -38,7 +38,6 @@ import {
   unauthorizedResp,
   type Unbrand,
 } from '../open-api/common/response.common.mjs'
-import { BASE_URL } from '../index.mjs'
 import type { CreateActionReturnType } from '../abstract-classes/base-controller.mjs'
 import { partialVbds, vbd, vbdId, vbdIds } from '../open-api/oa-examples/vbd.oa-example.mjs'
 import { RestApi } from '../rest-api/rest-api.mjs'

@xen-orchestra/rest-api/src/vdis/vdi.controller.mts

@@ -28,7 +28,7 @@ import { invalidParameters } from 'xo-common/api-errors.js'
 
 import { acl, actionsFromBody } from '../middlewares/acl.middleware.mjs'
 import { AlarmService } from '../alarms/alarm.service.mjs'
-import { escapeUnsafeComplexMatcher } from '../helpers/utils.helper.mjs'
+import { BASE_URL, escapeUnsafeComplexMatcher } from '../helpers/utils.helper.mjs'
 import { genericAlarmsExample } from '../open-api/oa-examples/alarm.oa-example.mjs'
 import {
   asynchronousActionResp,
@@ -42,7 +42,6 @@ import {
   unauthorizedResp,
   type Unbrand,
 } from '../open-api/common/response.common.mjs'
-import { BASE_URL } from '../index.mjs'
 import { RestApi } from '../rest-api/rest-api.mjs'
 import type { SendObjects } from '../helpers/helper.type.mjs'
 import type { CreateActionReturnType } from '../abstract-classes/base-controller.mjs'

@xen-orchestra/rest-api/src/vms/vm.controller.mts

@@ -55,8 +55,7 @@ import {
   unauthorizedResp,
   type Unbrand,
 } from '../open-api/common/response.common.mjs'
-import { BASE_URL } from '../index.mjs'
-import { limitAndFilterArray, NDJSON_CONTENT_TYPE } from '../helpers/utils.helper.mjs'
+import { BASE_URL, limitAndFilterArray, NDJSON_CONTENT_TYPE } from '../helpers/utils.helper.mjs'
 import { genericAlarmsExample } from '../open-api/oa-examples/alarm.oa-example.mjs'
 import { partialVms, vm, vmDashboard, vmIds, vmStatsExample, vmVdis } from '../open-api/oa-examples/vm.oa-example.mjs'
 import { RestApi } from '../rest-api/rest-api.mjs'

CHANGELOG.unreleased.md

@@ -39,6 +39,7 @@
 - [Backups] Fix slow replication startup and fallback to full on qcow2 (PR [#10333](https://github.com/vatesfr/xen-orchestra/pull/10333))
 - [REST API/SDN Controller] Fix deleting a non-existent traffic rule wrongly returning success instead of a 404 (PR [#9895](https://github.com/vatesfr/xen-orchestra/pull/9895))
 - [xo-server] Fix a memory leak when a client stops reading a proxied response, e.g. a Prometheus scrape of `/openmetrics` reaching its timeout: the request to the proxied service was never closed and its whole response stayed in memory, which could end up in the appliance being OOM-killed (PR [#10388](https://github.com/vatesfr/xen-orchestra/pull/10388))
+- [REST API] Rolling pool update and rolling pool reboot are now refused while a backup job runs on the pool, like their JSON-RPC counterparts, unless `bypassBackupCheck` is set in the request body (PR [#10313](https://github.com/vatesfr/xen-orchestra/pull/10313))
 
 ### Packages to release
 

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

@@ -1,11 +1,37 @@
+import { createLogger } from '@xen-orchestra/log'
 import { createPredicate } from 'value-matcher'
 import { extractIdsFromSimplePattern } from '@xen-orchestra/backups/extractIdsFromSimplePattern.mjs'
 import { forbiddenOperation } from 'xo-common/api-errors.js'
 
-export default async function backupGuard(poolId) {
+const log = createLogger('xo:api:backup-guard')
+
+/**
+ * Refuses an operation on an object while a backup job runs, or may run, on its pool.
+ *
+ * Must be called with `this` set to the xo-server instance.
+ *
+ * @param {string} objectId - ID of the XO object targeted by the operation (host, SR, pool…)
+ * @param {object} [opts]
+ * @param {boolean} [opts.bypassBackupCheck=false] - Skip the check, the bypass is logged
+ * @param {string} opts.operation - Name of the calling operation, for the log
+ * @throws {Error} `forbiddenOperation` if a backup runs or may run on the pool of the object
+ */
+export default async function backupGuard(objectId, { bypassBackupCheck = false, operation } = {}) {
+  const { $pool: poolId } = this.getObject(objectId)
+
+  if (bypassBackupCheck) {
+    log.warn(`${operation} with "bypassBackupCheck" set to true, skipping the backup guard`, {
+      objectId,
+      poolId,
+      userId: this.apiContext?.user?.id,
+    })
+    return
+  }
+
   const jobs = await this.getAllJobs('backup')
   const guard = id => {
-    if (this.getObject(id).$poolId === poolId) {
+    // a VM deleted since the job was configured cannot be backed up on this pool
+    if (this.hasObject(id) && this.getObject(id).$pool === poolId) {
       throw forbiddenOperation('Backup is running', `A backup is running on the pool: ${poolId}`)
     }
   }

packages/xo-server/src/api/_backupGuard.test.mjs

@@ -0,0 +1,56 @@
+import assert from 'node:assert/strict'
+import { describe, it } from 'node:test'
+import createMemoryTransport from '@xen-orchestra/log/transports/memory'
+import { configure } from '@xen-orchestra/log/configure'
+import { forbiddenOperation } from 'xo-common/api-errors.js'
+
+import backupGuard from './_backupGuard.mjs'
+
+const transport = createMemoryTransport()
+configure({ level: 'WARN', transport })
+
+const POOL_ID = 'pool-1'
+const HOST_ID = 'host-1'
+
+// a job with a `runId` is currently running, `host-1` and `vm-1` belong to the pool, `vm-gone` was deleted
+const createApp = jobs => ({
+  apiContext: { user: { id: 'user-1' } },
+  getAllJobs: async () => jobs,
+  hasObject: id => id !== 'vm-gone',
+  getObject: id => ({ $pool: id === 'vm-2' ? 'pool-2' : POOL_ID }),
+})
+
+describe('backupGuard', function () {
+  it('refuses the operation while a backup job runs on the pool of the object', async function () {
+    const app = createApp([{ runId: 'run-1', vms: { id: 'vm-1' } }])
+    await assert.rejects(backupGuard.call(app, HOST_ID), forbiddenOperation.is)
+    // the object may be the pool itself
+    await assert.rejects(backupGuard.call(app, POOL_ID), forbiddenOperation.is)
+  })
+
+  it('lets the operation through when no backup job runs on the pool', async function () {
+    await backupGuard.call(createApp([{ vms: { id: 'vm-1' } }, { runId: 'run-2', vms: { id: 'vm-2' } }]), HOST_ID)
+  })
+
+  it('ignores the VMs of a running job that no longer exist', async function () {
+    const running = ids => createApp([{ runId: 'run-1', vms: { id: { __or: ids } } }])
+    await backupGuard.call(running(['vm-gone', 'vm-2']), POOL_ID)
+    await assert.rejects(backupGuard.call(running(['vm-gone', 'vm-1']), POOL_ID), forbiddenOperation.is)
+  })
+
+  it('decides from the pool pattern of a running smart mode job', async function () {
+    const running = vms => createApp([{ runId: 'run-1', vms }])
+    await backupGuard.call(running({ $pool: { __or: ['pool-2'] } }), POOL_ID)
+    await assert.rejects(backupGuard.call(running({ $pool: { __or: [POOL_ID] } }), POOL_ID), forbiddenOperation.is)
+    // no pool pattern: the job may target any pool
+    await assert.rejects(backupGuard.call(running({ power_state: 'Running' }), POOL_ID), forbiddenOperation.is)
+  })
+
+  it('skips the check and logs who bypassed it on which object when bypassBackupCheck is set', async function () {
+    const app = createApp([{ runId: 'run-1', vms: { id: 'vm-1' } }])
+    await backupGuard.call(app, HOST_ID, { bypassBackupCheck: true, operation: 'host.restart' })
+    const [log] = transport.logs.filter(({ message }) => message.includes('bypassBackupCheck'))
+    assert.match(log.message, /^host\.restart /)
+    assert.deepEqual(log.data, { objectId: HOST_ID, poolId: POOL_ID, userId: 'user-1' })
+  })
+})

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

@@ -9,7 +9,6 @@ import { incorrectState, invalidParameters } from 'xo-common/api-errors.js'
 
 import { asyncEach } from '@vates/async-each'
 import { fromCallback } from 'promise-toolbox'
-import backupGuard from './_backupGuard.mjs'
 
 import { debounceWithKey } from '../_pDebounceWithKey.mjs'
 
@@ -157,7 +156,7 @@ set.resolve = {
 // FIXME: set force to false per default when correctly implemented in
 // UI.
 export async function restart({
-  bypassBackupCheck = false,
+  bypassBackupCheck,
   host,
   force = false,
   suspendResidentVms,
@@ -207,11 +206,7 @@ export async function restart({
     }
   }
 
-  if (bypassBackupCheck) {
-    log.warn('host.restart with argument "bypassBackupCheck" set to true', { hostId: host.id })
-  } else {
-    await backupGuard.call(this, host.$poolId)
-  }
+  await this.backupGuard(host.id, { bypassBackupCheck, operation: 'host.restart' })
 
   const xapi = this.getXapi(host)
   return suspendResidentVms
@@ -255,12 +250,8 @@ restart.resolve = {
 
 // -------------------------------------------------------------------
 
-export async function restartAgent({ bypassBackupCheck = false, host }) {
-  if (bypassBackupCheck) {
-    log.warn('host.restartAgent with argument "bypassBackupCheck" set to true', { hostId: host.id })
-  } else {
-    await backupGuard.call(this, host.$poolId)
-  }
+export async function restartAgent({ bypassBackupCheck, host }) {
+  await this.backupGuard(host.id, { bypassBackupCheck, operation: 'host.restartAgent' })
   return this.getXapiObject(host).$restartAgent()
 }
 
@@ -314,12 +305,8 @@ start.resolve = {
 
 // -------------------------------------------------------------------
 
-export async function stop({ bypassBackupCheck = false, host, bypassEvacuate }) {
-  if (bypassBackupCheck) {
-    log.warn('host.stop with argument "bypassBackupCheck" set to true', { hostId: host.id })
-  } else {
-    await backupGuard.call(this, host.$poolId)
-  }
+export async function stop({ bypassBackupCheck, host, bypassEvacuate }) {
+  await this.backupGuard(host.id, { bypassBackupCheck, operation: 'host.stop' })
   return this.getXapi(host).shutdownHost(host._xapiId, { bypassEvacuate })
 }
 

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

@@ -7,8 +7,6 @@ import tarStream from 'tar-stream'
 import { Ref } from 'xen-api'
 import { incorrectState, invalidParameters } from 'xo-common/api-errors.js'
 
-import backupGuard from './_backupGuard.mjs'
-
 import { fromCallback } from 'promise-toolbox'
 import { moveFirst } from '../_moveFirst.mjs'
 
@@ -251,15 +249,8 @@ installPatches.description = 'Install patches on hosts'
 
 // -------------------------------------------------------------------
 
-export const rollingUpdate = async function ({ bypassBackupCheck = false, pool, rebootVm, shutdownPinnedVms }) {
-  const poolId = pool.id
-  if (bypassBackupCheck) {
-    log.warn('pool.rollingUpdate with argument "bypassBackupCheck" set to true', { poolId })
-  } else {
-    await backupGuard.call(this, poolId)
-  }
-
-  await this.rollingPoolUpdate(pool, { rebootVm, shutdownPinnedVms })
+export const rollingUpdate = async function ({ bypassBackupCheck, pool, rebootVm, shutdownPinnedVms }) {
+  await this.rollingPoolUpdate(pool, { bypassBackupCheck, rebootVm, shutdownPinnedVms })
 }
 
 rollingUpdate.params = {
@@ -287,19 +278,12 @@ rollingUpdate.resolve = {
 // -------------------------------------------------------------------
 
 export async function rollingReboot({ bypassBackupCheck, pool, shutdownPinnedVms }) {
-  const poolId = pool.id
-  if (bypassBackupCheck) {
-    log.warn('pool.rollingReboot with argument "bypassBackupCheck" set to true', { poolId })
-  } else {
-    await backupGuard.call(this, poolId)
-  }
-
-  await this.rollingPoolReboot(pool, { shutdownPinnedVms })
+  await this.rollingPoolReboot(pool, { bypassBackupCheck, shutdownPinnedVms })
 }
 
 rollingReboot.params = {
   bypassBackupCheck: {
-    default: false,
+    optional: true,
     type: 'boolean',
   },
   pool: { type: 'string' },

packages/xo-server/src/api/pool.test.mjs

@@ -0,0 +1,24 @@
+import assert from 'node:assert/strict'
+import { describe, it } from 'node:test'
+
+import * as handlers from './pool.mjs'
+
+const pool = { id: 'pool-1' }
+
+for (const [method, orchestrator, params] of [
+  ['rollingUpdate', 'rollingPoolUpdate', { rebootVm: true, shutdownPinnedVms: false }],
+  ['rollingReboot', 'rollingPoolReboot', { shutdownPinnedVms: true }],
+]) {
+  describe(`pool.${method}`, function () {
+    it('forwards bypassBackupCheck to the orchestrator', async function () {
+      const calls = []
+      const app = { [orchestrator]: async (pool, opts) => calls.push([pool, opts]) }
+      await handlers[method].call(app, { pool, ...params })
+      await handlers[method].call(app, { pool, bypassBackupCheck: true, ...params })
+      assert.deepEqual(calls, [
+        [pool, { bypassBackupCheck: undefined, ...params }],
+        [pool, { bypassBackupCheck: true, ...params }],
+      ])
+    })
+  })
+}

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

@@ -1,16 +1,12 @@
 import isEmpty from 'lodash/isEmpty.js'
 import throttle from 'lodash/throttle.js'
-import { createLogger } from '@xen-orchestra/log'
 
-import backupGuard from './_backupGuard.mjs'
 import ensureArray from '../_ensureArray.mjs'
 import { asInteger } from '../xapi/utils.mjs'
 import { destroy as destroyXostor } from './xostor.mjs'
 import { forEach, isSrWritable, parseXml } from '../utils.mjs'
 import { PREFERED_IMAGE_FORMAT_PROPERTY } from '@xen-orchestra/xapi/pbd.mjs'
 
-const log = createLogger('xo:api:sr')
-
 const AUTHORIZED_IMAGE_FORMAT_LISTS = ['vhd', 'qcow2', 'vhd, qcow2', 'qcow2, vhd']
 
 // ===================================================================
@@ -1051,12 +1047,8 @@ disableMaintenanceMode.resolve = {
 
 // -------------------------------------------------------------------
 
-export async function reclaimSpace({ sr, bypassBackupCheck = false }) {
-  if (bypassBackupCheck) {
-    log.warn('sr.reclaimSpace with argument "bypassBackupCheck" set to true', { srId: sr.id })
-  } else {
-    await backupGuard.call(this, sr.$pool)
-  }
+export async function reclaimSpace({ sr, bypassBackupCheck }) {
+  await this.backupGuard(sr.id, { bypassBackupCheck, operation: 'sr.reclaimSpace' })
   await this.getXapiObject(sr).$reclaimSpace()
 }
 

packages/xo-server/src/xo-mixins/jobs/index.mjs

@@ -376,7 +376,7 @@ export default class Jobs {
     }
   }
 
-  backupGuard(poolId) {
-    return backupGuard.call(this._app, poolId)
+  backupGuard(objectId, opts) {
+    return backupGuard.call(this._app, objectId, opts)
   }
 }

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

@@ -205,9 +205,20 @@ export default class Pools {
     )
   }
 
-  async rollingPoolReboot(pool, { parentTask, shutdownPinnedVms } = {}) {
+  /**
+   * Reboots the hosts of the pool one at a time.
+   *
+   * @param {object} pool - XO pool object
+   * @param {object} [opts]
+   * @param {boolean} [opts.bypassBackupCheck] - Skip the backup guard, the bypass is logged
+   * @param {Task} [opts.parentTask] - Run as a subtask of this task instead of as a new root task
+   * @param {boolean} [opts.shutdownPinnedVms] - Shut down the VMs that cannot be migrated before their host reboots
+   * @throws {Error} `forbiddenOperation` if a backup runs or may run on the pool
+   */
+  async rollingPoolReboot(pool, { bypassBackupCheck, parentTask, shutdownPinnedVms } = {}) {
     const { _app } = this
     await _app.checkFeatureAuthorization('ROLLING_POOL_REBOOT')
+    await _app.backupGuard(pool.id, { bypassBackupCheck, operation: 'rollingPoolReboot' })
     const releaseGuard = acquireRpuGuard(pool.id, 'rollingPoolReboot')
     const trace = openRpuTrace({ dir: getRpuTracesConfig(_app).dir, kind: 'rpr', poolId: pool.id })
     try {

packages/xo-server/src/xo-mixins/pool.test.mjs

@@ -0,0 +1,57 @@
+import assert from 'node:assert/strict'
+import { after, describe, it } from 'node:test'
+import { mkdtempSync, rmSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { forbiddenOperation } from 'xo-common/api-errors.js'
+import { Task } from '@vates/task'
+
+import Pools from './pool.mjs'
+
+const pool = { id: 'pool-1', name_label: 'pool 1' }
+const tracesDir = mkdtempSync(join(tmpdir(), 'xo-rpr-test-'))
+after(() => rmSync(tracesDir, { recursive: true, force: true }))
+
+function createApp({ backupRunning = false } = {}) {
+  const calls = []
+  return {
+    calls,
+    hooks: { on() {} },
+    config: {
+      getOptional: key => (key === 'rpu.tracesDir' ? tracesDir : undefined),
+      getOptionalDuration: () => undefined,
+    },
+    tasks: { create: properties => new Task({ properties }) },
+    async checkFeatureAuthorization() {},
+    async backupGuard(poolId, opts) {
+      calls.push(['backupGuard', poolId, opts])
+      if (backupRunning) {
+        throw forbiddenOperation('backup')
+      }
+    },
+    getXapi: () => ({
+      async rollingPoolReboot(task, opts) {
+        calls.push(['xapi.rollingPoolReboot', opts])
+      },
+    }),
+  }
+}
+
+describe('Pools.rollingPoolReboot', function () {
+  it('is refused by the backup guard before anything else happens', async function () {
+    const app = createApp({ backupRunning: true })
+    await assert.rejects(new Pools(app).rollingPoolReboot(pool), forbiddenOperation.is)
+    assert.deepEqual(app.calls, [
+      ['backupGuard', 'pool-1', { bypassBackupCheck: undefined, operation: 'rollingPoolReboot' }],
+    ])
+  })
+
+  it('forwards bypassBackupCheck to the backup guard, then runs', async function () {
+    const app = createApp()
+    await new Pools(app).rollingPoolReboot(pool, { bypassBackupCheck: true, shutdownPinnedVms: true })
+    assert.deepEqual(app.calls, [
+      ['backupGuard', 'pool-1', { bypassBackupCheck: true, operation: 'rollingPoolReboot' }],
+      ['xapi.rollingPoolReboot', { shutdownPinnedVms: true }],
+    ])
+  })
+})

packages/xo-server/src/xo-mixins/xen-servers.mjs

@@ -947,12 +947,25 @@ export default class XenServers {
     })
   }
 
-  async rollingPoolUpdate($defer, pool, { rebootVm, parentTask, shutdownPinnedVms } = {}) {
+  /**
+   * Updates the hosts of the pool one at a time, the backup schedules of the pool are disabled meanwhile.
+   *
+   * @param {Function} $defer - Injected by the `defer` decorator
+   * @param {object} pool - XO pool object
+   * @param {object} [opts]
+   * @param {boolean} [opts.bypassBackupCheck] - Skip the backup guard, the bypass is logged
+   * @param {boolean} [opts.rebootVm] - Accept the VM reboots required by the update guidances (XenServer 8.4+),
+   *   otherwise such an update is refused with an `incorrectState` error
+   * @param {Task} [opts.parentTask] - Run as a subtask of this task instead of as a new root task
+   * @param {boolean} [opts.shutdownPinnedVms] - Shut down the VMs that cannot be migrated before their host reboots
+   * @throws {Error} `forbiddenOperation` if a backup runs or may run on the pool
+   */
+  async rollingPoolUpdate($defer, pool, { bypassBackupCheck, rebootVm, parentTask, shutdownPinnedVms } = {}) {
     const app = this._app
-    await app.checkFeatureAuthorization('ROLLING_POOL_UPDATE')
-    const [schedules, jobs] = await Promise.all([app.getAllSchedules(), app.getAllJobs('backup')])
-
     const poolId = pool.id
+    await app.checkFeatureAuthorization('ROLLING_POOL_UPDATE')
+    await app.backupGuard(poolId, { bypassBackupCheck, operation: 'rollingPoolUpdate' })
+    const [schedules, jobs] = await Promise.all([app.getAllSchedules(), app.getAllJobs('backup')])
 
     $defer(acquireRpuGuard(poolId, 'rollingPoolUpdate'))
 

packages/xo-server/src/xo-mixins/xen-servers.test.mjs

@@ -0,0 +1,77 @@
+import assert from 'node:assert/strict'
+import { after, describe, it, mock } from 'node:test'
+import { mkdtempSync, rmSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { forbiddenOperation } from 'xo-common/api-errors.js'
+import { Task } from '@vates/task'
+
+import XenServers from './xen-servers.mjs'
+
+const pool = { id: 'pool-1', name_label: 'pool 1', _xapiRef: 'OpaqueRef:pool-1' }
+const tracesDir = mkdtempSync(join(tmpdir(), 'xo-rpu-test-'))
+after(() => rmSync(tracesDir, { recursive: true, force: true }))
+
+function createXenServers({ backupRunning = false } = {}) {
+  const calls = []
+  const app = {
+    apiContext: { user: { preferences: {} } },
+    hooks: { on() {} },
+    config: {
+      getOptional: key => (key === 'rpu.tracesDir' ? tracesDir : undefined),
+      getOptionalDuration: () => undefined,
+      watchDuration() {},
+    },
+    tasks: { create: properties => new Task({ properties }) },
+    async checkFeatureAuthorization() {},
+    async backupGuard(poolId, opts) {
+      calls.push(['backupGuard', poolId, opts])
+      if (backupRunning) {
+        throw forbiddenOperation('backup')
+      }
+    },
+    async getAllJobs() {
+      calls.push(['getAllJobs'])
+      return []
+    },
+    async getAllSchedules() {
+      return []
+    },
+    async getOptionalPlugin() {},
+  }
+  // the constructor arms a timeout that rejects if the `core started` hook,
+  // never fired here, does not set up the server collection in time
+  mock.timers.enable({ apis: ['setTimeout'] })
+  const xenServers = new XenServers(app, { safeMode: true })
+  mock.timers.reset()
+  // no server is registered in the test: stub the XAPI lookup
+  xenServers.getXapi = () => ({
+    async getField() {
+      return false
+    },
+    async rollingPoolUpdate(task, { rebootVm, shutdownPinnedVms }) {
+      calls.push(['xapi.rollingPoolUpdate', { rebootVm, shutdownPinnedVms }])
+    },
+  })
+  return { calls, xenServers }
+}
+
+describe('XenServers.rollingPoolUpdate', function () {
+  it('is refused by the backup guard before anything else happens', async function () {
+    const { calls, xenServers } = createXenServers({ backupRunning: true })
+    await assert.rejects(xenServers.rollingPoolUpdate(pool), forbiddenOperation.is)
+    assert.deepEqual(calls, [
+      ['backupGuard', 'pool-1', { bypassBackupCheck: undefined, operation: 'rollingPoolUpdate' }],
+    ])
+  })
+
+  it('forwards bypassBackupCheck to the backup guard, then runs', async function () {
+    const { calls, xenServers } = createXenServers()
+    await xenServers.rollingPoolUpdate(pool, { bypassBackupCheck: true, rebootVm: true, shutdownPinnedVms: false })
+    assert.deepEqual(calls, [
+      ['backupGuard', 'pool-1', { bypassBackupCheck: true, operation: 'rollingPoolUpdate' }],
+      ['getAllJobs'],
+      ['xapi.rollingPoolUpdate', { rebootVm: true, shutdownPinnedVms: false }],
+    ])
+  })
+})