feat(xen-api/Ref): introduce new utils to manipulate refs (#5650)

Fixes xoa-support#3463

See xapi-project/xen-api#4338

@xen-orchestra/cr-seed-cli/index.js

@@ -1,7 +1,7 @@
 #!/usr/bin/env node
 
 const defer = require('golike-defer').default
-const { NULL_REF, Xapi } = require('xen-api')
+const { Ref, Xapi } = require('xen-api')
 
 const pkg = require('./package.json')
 
@@ -11,7 +11,7 @@ Xapi.prototype.getVmDisks = async function (vm) {
     ...vm.VBDs.map(async vbdRef => {
       const vbd = await this.getRecord('VBD', vbdRef)
       let vdiRef
-      if (vbd.type === 'Disk' && (vdiRef = vbd.VDI) !== NULL_REF) {
+      if (vbd.type === 'Disk' && Ref.isNotEmpty((vdiRef = vbd.VDI))) {
         disks[vbd.userdevice] = await this.getRecord('VDI', vdiRef)
       }
     }),

@xen-orchestra/xapi/src/_isValidRef.js

@@ -1,3 +0,0 @@
-const { NULL_REF, isOpaqueRef } = require('xen-api')
-
-module.exports = ref => ref !== NULL_REF && isOpaqueRef(ref)

@xen-orchestra/xapi/src/vbd.js

@@ -1,7 +1,7 @@
 const identity = require('lodash/identity')
 const ignoreErrors = require('promise-toolbox/ignoreErrors')
+const { Ref } = require('xen-api')
 
-const isValidRef = require('./_isValidRef')
 const isVmRunning = require('./_isVmRunning')
 
 const noop = Function.prototype
@@ -20,7 +20,7 @@ module.exports = class Vbd {
     VDI,
     VM,
 
-    empty = !isValidRef(VDI),
+    empty = !Ref.isNotEmpty(VDI),
     mode = type === 'Disk' ? 'RW' : 'RO',
   }) {
     if (userdevice == null) {

@xen-orchestra/xapi/src/vm.js

@@ -7,10 +7,9 @@ const pCatch = require('promise-toolbox/catch')
 const pRetry = require('promise-toolbox/retry')
 const { asyncMap } = require('@xen-orchestra/async-map')
 const { createLogger } = require('@xen-orchestra/log')
-const { NULL_REF } = require('xen-api')
+const { Ref } = require('xen-api')
 
 const extractOpaqueRef = require('./_extractOpaqueRef')
-const isValidRef = require('./_isValidRef')
 const isVmRunning = require('./_isVmRunning')
 
 const { warn } = createLogger('xo:xapi:vm')
@@ -113,7 +112,7 @@ module.exports = class Vm {
   async assertHealthyVdiChains(vmRef, tolerance = this._maxUncoalescedVdis) {
     const vdiRefs = {}
     ;(await this.getRecords('VBD', await this.getField('VM', vmRef, 'VBDs'))).forEach(({ VDI: ref }) => {
-      if (isValidRef(ref)) {
+      if (Ref.isNotEmpty(ref)) {
         vdiRefs[ref] = true
       }
     })
@@ -145,7 +144,7 @@ module.exports = class Vm {
       actions_after_crash = 'reboot',
       actions_after_reboot = 'reboot',
       actions_after_shutdown = 'destroy',
-      affinity = NULL_REF,
+      affinity = Ref.EMPTY,
       appliance,
       blocked_operations,
       domain_type,
@@ -363,7 +362,7 @@ module.exports = class Vm {
 
     const disks = { __proto__: null }
     ;(await this.getRecords('VBD', vbdRefs)).forEach(vbd => {
-      if (vbd.type === 'Disk' && isValidRef(vbd.VDI)) {
+      if (vbd.type === 'Disk' && Ref.isNotEmpty(vbd.VDI)) {
         disks[vbd.VDI] = true
       }
     })
@@ -410,7 +409,7 @@ module.exports = class Vm {
         const vbd = await this.getRecord('VBD', vbdRef)
         if (
           vbd.type === 'Disk' &&
-          isValidRef(vbd.VDI) &&
+          Ref.isNotEmpty(vbd.VDI) &&
           (await this.getField('VDI', vbd.VDI, 'name_label')).startsWith('[NOBAK]')
         ) {
           await this.VBD_destroy(vbdRef)

CHANGELOG.unreleased.md

@@ -36,6 +36,7 @@
 >
 > In case of conflict, the highest (lowest in previous list) `$version` wins.
 
+- xen-api minor
 - @xen-orchestra/xapi patch
 - @xen-orchestra/backups minor
 - xo-server minor

packages/xen-api/examples/utils.js

@@ -4,7 +4,7 @@ const { PassThrough, pipeline } = require('readable-stream')
 const humanFormat = require('human-format')
 const Throttle = require('throttle')
 
-const isOpaqueRef = require('../dist/_isOpaqueRef').default
+const Ref = require('../dist/_Ref').default
 
 exports.createInputStream = path => {
   if (path === undefined || path === '-') {
@@ -55,7 +55,7 @@ exports.pipeline = (...streams) => {
 }
 
 const resolveRef = (xapi, type, refOrUuidOrNameLabel) =>
-  isOpaqueRef(refOrUuidOrNameLabel)
+  Ref.is(refOrUuidOrNameLabel)
     ? refOrUuidOrNameLabel
     : xapi.call(`${type}.get_by_uuid`, refOrUuidOrNameLabel).catch(() =>
         xapi.call(`${type}.get_by_name_label`, refOrUuidOrNameLabel).then(refs => {

packages/xen-api/src/_Ref.js

@@ -0,0 +1,32 @@
+const EMPTY = 'OpaqueRef:NULL'
+const PREFIX = 'OpaqueRef:'
+
+export default {
+  // Reference to use to indicate it's not pointing to an object
+  EMPTY,
+
+  // Whether this value is a reference (probably) pointing to an object
+  isNotEmpty(val) {
+    return val !== EMPTY && typeof val === 'string' && val.startsWith(PREFIX)
+  },
+
+  // Whether this value looks like a reference
+  is(val) {
+    return (
+      typeof val === 'string' &&
+      (val.startsWith(PREFIX) ||
+        // 2019-02-07 - JFT: even if `value` should not be an empty string for
+        // a ref property, an user had the case on XenServer 7.0 on the CD VBD
+        // of a VM created by XenCenter
+        val === '' ||
+        // 2021-03-08 - JFT: there is an bug in XCP-ng/XenServer which leads to
+        // some refs to be `Ref:*` instead of being rewritten
+        //
+        // We'll consider them as empty refs in this lib to avoid issues with
+        // _wrapRecord.
+        //
+        // See https://github.com/xapi-project/xen-api/issues/4338
+        val.startsWith('Ref:'))
+    )
+  },
+}

packages/xen-api/src/_isOpaqueRef.js

@@ -1,3 +0,0 @@
-const PREFIX = 'OpaqueRef:'
-
-export default value => typeof value === 'string' && value.startsWith(PREFIX)

packages/xen-api/src/index.js

@@ -12,10 +12,10 @@ import coalesceCalls from './_coalesceCalls'
 import debug from './_debug'
 import getTaskResult from './_getTaskResult'
 import isGetAllRecordsMethod from './_isGetAllRecordsMethod'
-import isOpaqueRef from './_isOpaqueRef'
 import isReadOnlyCall from './_isReadOnlyCall'
 import makeCallSetting from './_makeCallSetting'
 import parseUrl from './_parseUrl'
+import Ref from './_Ref'
 import replaceSensitiveValues from './_replaceSensitiveValues'
 
 // ===================================================================
@@ -29,9 +29,7 @@ const { defineProperties, defineProperty, freeze, keys: getKeys } = Object
 
 // -------------------------------------------------------------------
 
-export const NULL_REF = 'OpaqueRef:NULL'
-
-export { isOpaqueRef }
+export { Ref }
 
 // -------------------------------------------------------------------
 
@@ -1094,7 +1092,7 @@ export class Xapi extends EventEmitter {
 
         const value = data[field]
         if (Array.isArray(value)) {
-          if (value.length === 0 || isOpaqueRef(value[0])) {
+          if (value.length === 0 || Ref.is(value[0])) {
             getters[$field] = function () {
               const value = this[field]
               return value.length === 0 ? value : value.map(getObjectByRef)
@@ -1121,10 +1119,7 @@ export class Xapi extends EventEmitter {
               ? xapi.setFieldEntry(this.$type, this.$ref, field, entries, value)
               : xapi.setFieldEntries(this.$type, this.$ref, field, entries)
           }
-        } else if (value === '' || isOpaqueRef(value)) {
-          // 2019-02-07 - JFT: even if `value` should not be an empty string for
-          // a ref property, an user had the case on XenServer 7.0 on the CD VBD
-          // of a VM created by XenCenter
+        } else if (Ref.is(value)) {
           getters[$field] = function () {
             return xapi._objectsByRef[this[field]]
           }

packages/xo-server/src/api/xosan.js

@@ -6,9 +6,9 @@ import execa from 'execa'
 import fs from 'fs-extra'
 import map from 'lodash/map'
 import { tap, delay } from 'promise-toolbox'
-import { NULL_REF } from 'xen-api'
 import { invalidParameters } from 'xo-common/api-errors'
 import { includes, remove, filter, find, range } from 'lodash'
+import { Ref } from 'xen-api'
 
 import ensureArray from '../_ensureArray'
 import { parseXml } from '../utils'
@@ -937,7 +937,7 @@ async function _prepareGlusterVm(
   log.debug(`waiting for boot of ${ip}`)
   // wait until we find the assigned IP in the networks, we are just checking the boot is complete
   // fix #3688
-  const vm = await xapi._waitObjectState(newVM.$id, _ => _.guest_metrics !== NULL_REF)
+  const vm = await xapi._waitObjectState(newVM.$id, _ => Ref.isNotEmpty(_.guest_metrics))
   await xapi._waitObjectState(vm.guest_metrics, _ => includes(_.networks, ip))
   log.debug(`booted ${ip}`)
   const localEndpoint = { xapi: xapi, hosts: [host], addresses: [ip] }

packages/xo-server/src/xapi/index.js

@@ -14,9 +14,9 @@ import { cancelable, defer, fromEvents, ignoreErrors, pCatch, pRetry } from 'pro
 import { parseDuration } from '@vates/parse-duration'
 import { PassThrough } from 'stream'
 import { forbiddenOperation } from 'xo-common/api-errors'
-import { NULL_REF } from 'xen-api'
 import { Xapi as XapiBase } from '@xen-orchestra/xapi'
 import { filter, find, flatMap, flatten, groupBy, identity, includes, isEmpty, noop, omit, once, uniq } from 'lodash'
+import { Ref } from 'xen-api'
 import { satisfies as versionSatisfies } from 'semver'
 
 import createSizeStream from '../size-stream'
@@ -523,7 +523,7 @@ export default class Xapi extends XapiBase {
         actions_after_crash,
         actions_after_reboot,
         actions_after_shutdown,
-        affinity: affinity == null ? NULL_REF : affinity,
+        affinity: affinity == null ? Ref.EMPTY : affinity,
         HVM_boot_params,
         HVM_boot_policy,
         is_a_template: asBoolean(is_a_template),
@@ -612,7 +612,9 @@ export default class Xapi extends XapiBase {
     return Promise.all([
       asyncMapSettled(vm.$snapshots, snapshot => this._deleteVm(snapshot))::ignoreErrors(),
 
-      vm.power_state === 'Suspended' && vm.suspend_VDI !== NULL_REF && this._deleteVdi(vm.suspend_VDI)::ignoreErrors(),
+      vm.power_state === 'Suspended' &&
+        Ref.isNotEmpty(vm.suspend_VDI) &&
+        this._deleteVdi(vm.suspend_VDI)::ignoreErrors(),
 
       deleteDisks &&
         asyncMapSettled(disks, async vdi => {
@@ -1612,7 +1614,7 @@ export default class Xapi extends XapiBase {
       xenstore_data,
 
       size,
-      sr = SR !== undefined && SR !== NULL_REF ? SR : this.pool.default_SR,
+      sr = Ref.isNotEmpty(SR) ? SR : this.pool.default_SR,
     },
     {
       // blindly copying `sm_config` from another VDI can create problems,
@@ -1972,7 +1974,7 @@ export default class Xapi extends XapiBase {
     })
 
     const vlans = uniq(mapToArray(pifs, pif => pif.VLAN_master_of))
-    await Promise.all(mapToArray(vlans, vlan => vlan !== NULL_REF && this.callAsync('VLAN.destroy', vlan)))
+    await Promise.all(mapToArray(vlans, vlan => Ref.isNotEmpty(vlan) && this.callAsync('VLAN.destroy', vlan)))
 
     const newPifs = await this.call('pool.create_VLAN_from_PIF', physPif.$ref, pif.network, asInteger(vlan))
     await Promise.all(
@@ -2008,7 +2010,7 @@ export default class Xapi extends XapiBase {
     const pifs = network.$PIFs
 
     const vlans = uniq(mapToArray(pifs, pif => pif.VLAN_master_of))
-    await Promise.all(mapToArray(vlans, vlan => vlan !== NULL_REF && this.callAsync('VLAN.destroy', vlan)))
+    await Promise.all(mapToArray(vlans, vlan => Ref.isNotEmpty(vlan) && this.callAsync('VLAN.destroy', vlan)))
 
     const bonds = uniq(flatten(mapToArray(pifs, pif => pif.bond_master_of)))
     await Promise.all(mapToArray(bonds, bond => this.call('Bond.destroy', bond)))

packages/xo-server/src/xapi/mixins/vm.js

@@ -1,7 +1,7 @@
 import deferrable from 'golike-defer'
 import { find, gte, includes, isEmpty, lte, mapValues, noop } from 'lodash'
 import { cancelable, ignoreErrors, pCatch } from 'promise-toolbox'
-import { NULL_REF } from 'xen-api'
+import { Ref } from 'xen-api'
 
 import { forEach, mapToArray, parseSize } from '../../utils'
 
@@ -250,7 +250,7 @@ export default {
   _editVm: makeEditObject({
     affinityHost: {
       get: 'affinity',
-      set: (value, vm) => vm.set_affinity(value ? vm.$xapi.getObject(value).$ref : NULL_REF),
+      set: (value, vm) => vm.set_affinity(value ? vm.$xapi.getObject(value).$ref : Ref.EMPTY),
     },
 
     autoPoweron: {

packages/xo-server/src/xapi/other-config-template.js

@@ -1,4 +1,4 @@
-import { NULL_REF } from 'xen-api'
+import { Ref } from 'xen-api'
 
 const OTHER_CONFIG_TEMPLATE = {
   actions_after_crash: 'restart',
@@ -34,7 +34,7 @@ const OTHER_CONFIG_TEMPLATE = {
     hpet: 'true',
     viridian: 'true',
   },
-  protection_policy: NULL_REF,
+  protection_policy: Ref.EMPTY,
   PV_args: '',
   PV_bootloader: '',
   PV_bootloader_args: '',

packages/xo-server/src/xo-mixins/proxies.js

@@ -12,8 +12,8 @@ import { createLogger } from '@xen-orchestra/log'
 import { format, parse } from 'json-rpc-peer'
 import { incorrectState, noSuchObject } from 'xo-common/api-errors'
 import { isEmpty, mapValues, some, omit } from 'lodash'
-import { NULL_REF } from 'xen-api'
 import { parseDuration } from '@vates/parse-duration'
+import { Ref } from 'xen-api'
 import { timeout } from 'promise-toolbox'
 
 import Collection from '../collection/redis'
@@ -308,7 +308,7 @@ export default class Proxy {
     // ensure appliance has an IP address
     const vmNetworksTimeout = parseDuration(xoProxyConf.vmNetworksTimeout)
     vm = await timeout.call(
-      xapi._waitObjectState(vm.$id, _ => _.guest_metrics !== NULL_REF),
+      xapi._waitObjectState(vm.$id, _ => Ref.isNotEmpty(_.guest_metrics)),
       vmNetworksTimeout
     )
     await timeout.call(