fix(vm.create): correctly compute limits usage (#389)

Fixes vatesfr/xo-web#1365

src/api/vm.coffee

@@ -61,6 +61,21 @@ extract = (obj, prop) ->
 
 # TODO: Implement ACLs
 create = $coroutine (params) ->
+  console.log(params)
+
+  checkLimits = limits = null
+
+  { user } = this
+  resourceSet = extract(params, 'resourceSet')
+  if resourceSet
+    yield this.checkResourceSetConstraints(resourceSet, user.id, objectIds)
+    checkLimits = $coroutine (limits2) =>
+      console.log('limits', limits, limits2)
+      yield this.allocateLimitsInResourceSet(limits, resourceSet)
+      yield this.allocateLimitsInResourceSet(limits2, resourceSet)
+  else unless user.permission is 'admin'
+    throw new Unauthorized()
+
   template = extract(params, 'template')
   params.template = template._xapiId
 
@@ -81,7 +96,7 @@ create = $coroutine (params) ->
       vbd.type is 'Disk' and
       (vdi = vbd.$VDI)
     )
-      vdiSizesByDevice[vbd.device] = +vdi.virtual_size
+      vdiSizesByDevice[vbd.userdevice] = +vdi.virtual_size
 
     return
   )
@@ -95,7 +110,7 @@ create = $coroutine (params) ->
     limits.disk += size
 
     return $assign({}, vdi, {
-      device: vdi.device ? vdi.position,
+      device: vdi.userdevice ? vdi.device ? vdi.position,
       size,
       SR: sr._xapiId,
       type: vdi.type
@@ -103,10 +118,10 @@ create = $coroutine (params) ->
   )
 
   existingVdis = extract(params, 'existingDisks')
-  params.existingVdis = existingVdis and map(existingVdis, (vdi, device) =>
+  params.existingVdis = existingVdis and map(existingVdis, (vdi, userdevice) =>
     if vdi.size?
       size = parseSize(vdi.size)
-      vdiSizesByDevice[device] = size
+      vdiSizesByDevice[userdevice] = size
 
     if vdi.$SR
       sr = @getObject(vdi.$SR)
@@ -118,6 +133,7 @@ create = $coroutine (params) ->
     })
   )
 
+  console.log(vdiSizesByDevice)
   forEach(vdiSizesByDevice, (size) => limits.disk += size)
 
   vifs = extract(params, 'VIFs')
@@ -135,18 +151,9 @@ create = $coroutine (params) ->
   installation = extract(params, 'installation')
   params.installRepository = installation && installation.repository
 
-  resourceSet = extract(params, 'resourceSet')
-
-  xapiVm = yield xapi.createVm(template._xapiId, params)
+  xapiVm = yield xapi.createVm(template._xapiId, params, checkLimits)
   vm = xapi.xo.addObject(xapiVm)
 
-  { user } = this
-  if resourceSet
-    yield this.checkResourceSetConstraints(resourceSet, user.id, objectIds)
-    yield this.allocateLimitsInResourceSet(limits, resourceSet)
-  else unless user.permission is 'admin'
-    throw new Unauthorized()
-
   if resourceSet
     yield Promise.all([
       @addAcl(user.id, vm.id, 'admin')

src/xapi/mixins/vm.js

@@ -2,6 +2,9 @@ import find from 'lodash/find'
 import gte from 'lodash/gte'
 import lte from 'lodash/lte'
 
+import {
+  deferrable
+} from '../../decorators'
 import {
   forEach,
   mapToArray,
@@ -18,7 +21,8 @@ import {
 
 export default {
   // TODO: clean up on error.
-  async createVm (templateId, {
+  @deferrable.onFailure
+  async createVm ($onFailure, templateId, {
     name_label, // deprecated
     nameLabel = name_label, // eslint-disable-line camelcase
 
@@ -34,7 +38,7 @@ export default {
     cloudConfig = undefined,
 
     ...props
-  } = {}) {
+  } = {}, checkLimits) {
     const installMethod = (() => {
       if (installRepository == null) {
         return 'none'
@@ -50,23 +54,23 @@ export default {
     const template = this.getObject(templateId)
 
     // Clones the template.
-    let vm = await this._getOrWaitObject(
-      await this[clone ? '_cloneVm' : '_copyVm'](template, nameLabel)
-    )
+    const vmRef = await this[clone ? '_cloneVm' : '_copyVm'](template, nameLabel)
+    $onFailure(() => this.deleteVm(vmRef, true)::pCatch(noop))
 
     // TODO: copy BIOS strings?
 
     // Removes disks from the provision XML, we will create them by
     // ourselves.
-    await this.call('VM.remove_from_other_config', vm.$ref, 'disks')::pCatch(noop)
+    await this.call('VM.remove_from_other_config', vmRef, 'disks')::pCatch(noop)
 
     // Creates the VDIs and executes the initial steps of the
     // installation.
-    await this.call('VM.provision', vm.$ref)
+    await this.call('VM.provision', vmRef)
+
+    let vm = await this._getOrWaitObject(vmRef)
 
     // Set VMs params.
-    // TODO: checkLimits
-    this._editVm(vm, props)
+    await this._editVm(vm, props, checkLimits)
 
     // Sets boot parameters.
     {
@@ -243,7 +247,6 @@ export default {
     },
 
     cpuCap: {
-      addToLimits: true,
       get: vm => vm.VCPUs_params.cap && +vm.VCPUs_params.cap,
       set (cap, vm) {
         return this._updateObjectMapProperty(vm, 'VCPUs_params', { cap })
@@ -260,7 +263,6 @@ export default {
     },
 
     cpuWeight: {
-      addToLimits: true,
       get: vm => vm.VCPUs_params.weight && +vm.VCPUs_params.weight,
       set (weight, vm) {
         return this._updateObjectMapProperty(vm, 'VCPUs_params', { weight })
@@ -285,6 +287,7 @@ export default {
     memory: 'memoryMax',
     memoryMax: {
       addToLimits: true,
+      limitName: 'memory',
       constraints: {
         memoryMin: lte,
         memoryStaticMax: gte

src/xapi/utils.js

@@ -224,6 +224,9 @@ export const makeEditObject = specs => {
     if (spec.addToLimits === true) {
       spec.addToLimits = _DEFAULT_ADD_TO_LIMITS
     }
+    if (!spec.limitName) {
+      spec.limitName = name
+    }
 
     forEach(spec.constraints, (constraint, constraintName) => {
       if (!isFunction(constraint)) {
@@ -302,7 +305,8 @@ export const makeEditObject = specs => {
 
         let addToLimits
         if (limits && (addToLimits = spec.addToLimits)) {
-          limits[name] = addToLimits(value, current)
+          console.log(spec.limitName, value, current, addToLimits(value, current))
+          limits[spec.limitName] = addToLimits(value, current)
         }
       }