From db6d48f8ca91e7a2e2da4a9dcba8e23db030ef2c Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Thu, 11 Jun 2015 16:30:59 +0200 Subject: [PATCH 01/20] vm.create() refactoring. --- src/api-errors.js | 1 + src/api.js | 16 ++-- src/api/vm.coffee | 191 +++++------------------------------------- src/xapi.js | 208 +++++++++++++++++++++++++++++++++++++++++----- 4 files changed, 217 insertions(+), 199 deletions(-) diff --git a/src/api-errors.js b/src/api-errors.js index cf1263ccc..aadc48b25 100644 --- a/src/api-errors.js +++ b/src/api-errors.js @@ -7,6 +7,7 @@ export { InvalidJson, InvalidParameters, InvalidRequest, + JsonRpcError, MethodNotFound } from 'json-rpc-protocol' diff --git a/src/api.js b/src/api.js index 64292a0c2..81f681e53 100644 --- a/src/api.js +++ b/src/api.js @@ -12,6 +12,7 @@ import schemaInspector from 'schema-inspector' import { InvalidParameters, + JsonRpcError, MethodNotFound, NoSuchObject, Unauthorized @@ -289,11 +290,12 @@ export default class Api { context.user = await context._getUser(userId) } - await checkPermission.call(context, method) - checkParams(method, params) - - await resolveParams.call(context, method, params) try { + await checkPermission.call(context, method) + checkParams(method, params) + + await resolveParams.call(context, method, params) + let result = await method.call(context, params) // If nothing was returned, consider this operation a success @@ -306,7 +308,11 @@ export default class Api { return result } catch (error) { - debug('Error: %s(...) → %s', name, error) + if (error instanceof JsonRpcError) { + debug('Error: %s(...) → %s', name, error) + } else { + console.error(error && error.stack || error) + } throw error } diff --git a/src/api/vm.coffee b/src/api/vm.coffee index 3d83cd7bb..0e349238e 100644 --- a/src/api/vm.coffee +++ b/src/api/vm.coffee @@ -28,7 +28,6 @@ $isVMRunning = do -> #===================================================================== # TODO: Implement ACLs -# FIXME: Make the method as atomic as possible. create = $coroutine ({ installation name_label @@ -36,135 +35,13 @@ create = $coroutine ({ VDIs VIFs }) -> - # Gets the corresponding connection. - xapi = @getXAPI template + vm = yield @getXAPI(template).createVm(template.id, name_label, { + installRepository: installation.repository, + vdis: VDIs, + vifs: VIFs + }) - # Clones the VM from the template. - vm = yield xapi.cloneVm(template.ref, name_label) - - # TODO: if there is an error from now, removes this VM. - - # TODO: remove existing VIFs. - # Creates associated virtual interfaces. - # - # FIXME: device n may already exists, we have to find the first - # free device number. - deviceId = 0 - yield Bluebird.all(map(VIFs, (VIF) => - return xapi.createVirtualInterface(vm.$id, VIF.network, { - position: deviceId++ - }) - )) - - # TODO: ? yield xapi.call 'VM.set_PV_args', vm.$ref, 'noninteractive' - - # Updates the number of existing vCPUs. - if CPUs? - yield xapi.call 'VM.set_VCPUs_at_startup', vm.$ref, CPUs - - # TODO: remove existing VDIs (o make sure we have only those we - # asked. - # - # Problem: how to know which VMs to clones for instance. - if VDIs? - # Transform the VDIs specs to conform to XAPI. - $forEach VDIs, (VDI, key) -> - VDI.bootable = if VDI.bootable then 'true' else 'false' - VDI.size = "#{VDI.size}" - VDI.sr = VDI.SR - delete VDI.SR - - # Preparation for the XML generation. - VDIs[key] = { $: VDI } - - return - - # Converts the provision disks spec to XML. - VDIs = $js2xml { - provision: { - disk: VDIs - } - } - - # Replace the existing entry in the VM object. - try yield xapi.call 'VM.remove_from_other_config', vm.$ref, 'disks' - yield xapi.call 'VM.add_to_other_config', vm.$ref, 'disks', VDIs - - try yield xapi.call( - 'VM.remove_from_other_config' - vm.$ref - 'install-repository' - ) - if installation - switch installation.method - when 'cdrom' - yield xapi.call( - 'VM.add_to_other_config', vm.$ref - 'install-repository', 'cdrom' - ) - when 'ftp', 'http', 'nfs' - yield xapi.call( - 'VM.add_to_other_config', vm.$ref - 'install-repository', installation.repository - ) - else - @throw( - 'INVALID_PARAMS' - "Unsupported installation method #{installation.method}" - ) - - # Creates the VDIs and executes the initial steps of the - # installation. - yield xapi.call 'VM.provision', vm.$ref - - # Gets the VM record. - VM = yield xapi.call 'VM.get_record', vm.$ref - - if installation.method is 'cdrom' - # Gets the VDI containing the ISO to mount. - try - VDIref = (@getObject installation.repository, 'VDI').ref - catch - @throw 'NO_SUCH_OBJECT', 'installation.repository' - - # Finds the VBD associated to the newly created VM which is a - # CD. - CD_drive = null - for ref in VM.VBDs - VBD = yield xapi.call 'VBD.get_record', vm.$ref - # TODO: Checks it has been correctly retrieved. - if VBD.type is 'CD' - CD_drive = VBD.ref - break - - # No CD drives have been found, creates one. - unless CD_drive - # See: https://github.com/xenserver/xenadmin/blob/da00b13bb94603b369b873b0a555d44f15fa0ca5/XenModel/Actions/VM/CreateVMAction.cs#L370 - CD_drive = yield xapi.call 'VBD.create', { - bootable: true - device: '' - empty: true - mode: 'RO' - other_config: {} - qos_algorithm_params: {} - qos_algorithm_type: '' - type: 'CD' - unpluggable: true - userdevice: (yield xapi.call 'VM.get_allowed_VBD_devices', vm.$ref)[0] - VDI: 'OpaqueRef:NULL' - VM: vm.$ref - } - - # If the CD drive as not been found, throws. - @throw 'NO_SUCH_OBJECT' unless CD_drive - - # Mounts the VDI into the VBD. - yield xapi.call 'VBD.insert', CD_drive, VDIref - else - yield xapi.call 'VM.provision', vm.$ref - - # The VM should be properly created. - return vm.uuid + return vm.id create.permission = 'admin' @@ -280,41 +157,8 @@ exports.ejectCd = ejectCd #--------------------------------------------------------------------- insertCd = $coroutine ({vm, vdi, force}) -> - xapi = @getXAPI vm - - # Finds the CD drive. - cdDrive = null - $forEach (@getObjects vm.$VBDs), (VBD) -> - if VBD.is_cd_drive - cdDrive = VBD - return false - return - - if cdDrive - cdDriveRef = cdDrive.ref - - if cdDrive.VDI - @throw 'INVALID_PARAMS' unless force - yield xapi.call 'VBD.eject', cdDriveRef - else - cdDriveRef = yield xapi.call 'VBD.create', { - bootable: true - device: '' - empty: true - mode: 'RO' - other_config: {} - qos_algorithm_params: {} - qos_algorithm_type: '' - type: 'CD' - unpluggable: true - userdevice: (yield xapi.call 'VM.get_allowed_VBD_devices', vm.ref)[0] - VDI: 'OpaqueRef:NULL' - VM: vm.ref - } - - yield xapi.call 'VBD.insert', cdDriveRef, vdi.ref - - return true + yield @getXAPI(vm).insertCdIntoVm(vdi.id, vm.id, force) + return insertCd.params = { id: { type: 'string' } @@ -582,13 +426,14 @@ exports.restart = restart #--------------------------------------------------------------------- clone = $coroutine ({vm, name, full_copy}) -> - xapi = @getXAPI vm - if full_copy - yield xapi.call 'VM.copy', vm.ref, name, '' - else - yield xapi.call 'VM.clone', vm.ref, name + xapi = @getXAPI(vm) - return true + newVm = yield if full_copy + xapi.copyVm(vm.ref, null, name) + else + xapi.cloneVm(vm.ref, name) + + return newVm.$id clone.params = { id: { type: 'string' } @@ -818,7 +663,11 @@ exports.import = import_ # FIXME: if position is used, all other disks after this position # should be shifted. attachDisk = $coroutine ({vm, vdi, position, mode, bootable}) -> - yield @getXAPI(vm).attachVdiToVm(vdi.id, vm.id, {bootable, mode, position}) + yield @getXAPI(vm).attachVdiToVm(vdi.id, vm.id, { + bootable, + position, + readOnly: mode is 'RO' + }) return attachDisk.params = { diff --git a/src/xapi.js b/src/xapi.js index 01203c471..61d1a947f 100644 --- a/src/xapi.js +++ b/src/xapi.js @@ -13,7 +13,12 @@ import { } from 'xen-api' import {debounce} from './decorators' -import {ensureArray, noop, parseXml, pFinally} from './utils' +import { + ensureArray, + formatXml, + noop, parseXml, + pFinally +} from './utils' import {JsonRpcError} from './api-errors' const debug = createDebug('xo:xapi') @@ -404,10 +409,9 @@ export default class Xapi extends XapiBase { // ================================================================= - async _deleteVdi (vdiId) { - const vdi = this.getObject(vdiId) + async _cloneVm (vm, nameLabel = vm.name_label) { + return await this.call('VM.clone', vm.$ref, nameLabel) - await this.call('VDI.destroy', vdi.$ref) } async _snapshotVm (vm, nameLabel = vm.name_label) { @@ -419,15 +423,98 @@ export default class Xapi extends XapiBase { return ref } - async cloneVm (vmId, name_label = undefined) { + async cloneVm (vmId, nameLabel = undefined) { + return this._getOrWaitObject( + await this._cloneVm(this.getObject(vmId), nameLabel) + ) + } + + async copyVm (vmId, srId = null, nameLabel = undefined) { const vm = this.getObject(vmId) - if (name_label == null) { - ({name_label} = vm) + const srRef = (srId == null) ? + '' : + this.getObject(srId).$ref + + return await this._getOrWaitObject( + await this.call('VM.copy', vm.$ref, nameLabel || vm.nameLabel, srRef) + ) + } + + // TODO: clean up on error. + async createVm (templateId, nameLabel, { + cpus = undefined, + installRepository = undefined, + vdis = [], + vifs = [] + } = {}) { + const template = this.getObject(templateId) + + // Clones the template. + const vm = await this._getOrWaitObject( + await this._cloneVm(template, nameLabel) + ) + + // Creates the VIFs. + // + // TODO: removes existing VIFs. + { + let position = 0 + await Promise.all(map(vifs, vif => this._createVif( + vm, + this.getObject(vif.network), + { position: position++ } + ))) } - const ref = this.call('VM.clone', vm.$ref, name_label) + // TODO: ? await this.call('VM.set_PV_args', vm.$ref, 'noninteractive') - return await this._getOrWaitObject(ref) + // Sets the number of CPUs. + if (cpus != null) { + await this.call('VM.set_VCPUs_at_startup') + } + + // TODO: remove existing VDIs (to make sure there are only those + // wanted). + // + // Registers the VDIs description for the provisioner. + if (vdis.length) { + const vdisXml = formatXml({ + provisition: { + disks: map(vdis, vdi => { + const bootable = String(Boolean(vdi.bootable)) + const size = String(vdi.size) + const sr = vdi.sr || vdi.SR + return {$: {bootable, size, sr}} + }) + } + }) + + // Removes any preexisting entry. + await this.call('VM.remove_from_other_config', vm.$ref, 'disks').catch(noop) + + await this.call('VM.add_to_other_config', vm.$ref, 'disks', vdisXml) + } + + // Removes any preexisting entry. + await this.call('VM.remove_from_other_config', vm.$ref, 'install-repository').catch(noop) + if (installRepository != null) { + await this.call('VM.add_to_other_config', vm.$ref, 'install-repository', installRepository) + } + + // Creates the VDIs and executes the initial steps of the + // installation. + await this.call('VM.provision', vm.$ref) + + if (installRepository != null) { + try { + const cd = this.getObject(installRepository) + + await this._insertCdIntoVm(cd, vm) + } catch (_) {} + } + + // Returns the last state of the new VM. + return await this._waitObject(vm.$id) } async deleteVm (vmId, deleteDisks = false) { @@ -503,14 +590,15 @@ export default class Xapi extends XapiBase { ) } - async attachVdiToVm (vdiId, vmId, { - bootable = false, - mode = 'RW', - position - } = {}) { - const vdi = this.getObject(vdiId) - const vm = this.getObject(vmId) + // ================================================================= + async _createVbd (vm, vdi, { + bootable = false, + position = undefined, + type = 'Disk', + readOnly = (type !== 'Disk') + }) { + // TODO: use VM.get_allowed_VBD_devices()? if (position == null) { forEach(vm.$VBDs, vbd => { const curPos = +vbd.userdevice @@ -525,11 +613,12 @@ export default class Xapi extends XapiBase { const vbdRef = await this.call('VBD.create', { bootable, empty: false, - mode, + mode: readOnly ? 'RO' : 'RW', other_config: {}, qos_algorithm_params: {}, qos_algorithm_type: '', - type: 'Disk', + type, + unpluggable: (type !== 'Disk'), userdevice: String(position), VDI: vdi.$ref, VM: vm.$ref @@ -538,17 +627,80 @@ export default class Xapi extends XapiBase { if (isVmRunning(vm)) { await this.call('VBD.plug', vbdRef) } + + return vbdRef + } + + async _deleteVdi (vdiId) { + const vdi = this.getObject(vdiId) + + await this.call('VDI.destroy', vdi.$ref) + } + + _getVmCdDrive (vm) { + for (let vbd of vm.$VBDs) { + if (vbd.type === 'CD') { + return vbd + } + } + } + + async _insertCdIntoVm (cd, vm, force) { + const cdDrive = await this._getVmCdDrive(vm) + if (cdDrive) { + try { + await this.call('VBD.insert', cdDrive.$ref, cd.$ref).catch() + } catch (error) { + if (force && error.code === 'VBD_NOT_EMPTY') { + await this.call('VBD.eject', cdDrive.$ref).catch(noop) + + return this._insertCdIntoVm(cd, vm, force) + } + + throw error + } + } else { + await this._createVbd(vm, cd, { + bootable: true, + type: 'CD' + }) + } + } + + async attachVdiToVm (vdiId, vmId, opts = undefined) { + await this._createVbd( + this.getObject(vdiId), + this.getObject(vmId), + opts + ) + } + + async insertCdIntoVm (cdId, vmId, force = undefined) { + await this._insertCdIntoVm( + this.getObject(cdId), + this.getObject(vmId), + force + ) } // ================================================================= - async createVirtualInterface (vmId, networkId, { + async _createVif (vm, network, { mac = '', mtu = 1500, - position = 0 + position = undefined } = {}) { - const vm = this.getObject(vmId) - const network = this.getObject(networkId) + // TODO: use VM.get_allowed_VIF_devices()? + if (position == null) { + forEach(vm.$VIFs, vif => { + const curPos = +vif.device + if (!(position > curPos)) { + position = curPos + } + }) + + position = position == null ? 0 : position + 1 + } const vifRef = await this.call('VIF.create', { device: String(position), @@ -565,7 +717,17 @@ export default class Xapi extends XapiBase { await this.call('VIF.plug', vifRef) } - return await this._getOrWaitObject(vifRef) + return vifRef + } + + async createVirtualInterface (vmId, networkId, opts = undefined) { + return await this._getOrWaitObject( + await this._createVif( + this.getObject(vmId), + this.getObject(networkId), + opts + ) + ) } // ================================================================= From 7148fec6e169aba505e7b5cff94efe457784639e Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Thu, 11 Jun 2015 16:33:49 +0200 Subject: [PATCH 02/20] Object watchers are never rejected. --- src/xapi.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/xapi.js b/src/xapi.js index 61d1a947f..a9858b65b 100644 --- a/src/xapi.js +++ b/src/xapi.js @@ -129,17 +129,15 @@ export default class Xapi extends XapiBase { _waitObject (idOrUuidOrRef) { let watcher = this._objectWatchers[idOrUuidOrRef] if (!watcher) { - let resolve, reject - const promise = new Promise((resolve_, reject_) => { + let resolve + const promise = new Promise(resolve_ => { resolve = resolve_ - reject = reject_ }) // Register the watcher. watcher = this._objectWatchers[idOrUuidOrRef] = { promise, - resolve, - reject + resolve } } From db97787b15d4bb95c223aecf028f295411424887 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Thu, 11 Jun 2015 16:44:14 +0200 Subject: [PATCH 03/20] Fix other_config.disks XML generation. --- src/xapi.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xapi.js b/src/xapi.js index a9858b65b..61a0b76fb 100644 --- a/src/xapi.js +++ b/src/xapi.js @@ -478,7 +478,7 @@ export default class Xapi extends XapiBase { if (vdis.length) { const vdisXml = formatXml({ provisition: { - disks: map(vdis, vdi => { + disk: map(vdis, vdi => { const bootable = String(Boolean(vdi.bootable)) const size = String(vdi.size) const sr = vdi.sr || vdi.SR From 153dca81379772f548fbd43806a34e0a5019453f Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Thu, 11 Jun 2015 16:44:36 +0200 Subject: [PATCH 04/20] More tolerant parsing of VM-template.disks. --- src/xapi-objects-to-xo.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/xapi-objects-to-xo.js b/src/xapi-objects-to-xo.js index ca88bac03..1fab40d9a 100644 --- a/src/xapi-objects-to-xo.js +++ b/src/xapi-objects-to-xo.js @@ -239,11 +239,12 @@ export function vm (obj) { arch: otherConfig['install-arch'], disks: (function () { const {disks: xml} = otherConfig - if (!xml) { + let data + if (!xml || !(data = parseXml(xml)).provision) { return [] } - const disks = ensureArray(parseXml(xml).provision.disk) + const disks = ensureArray(data.provision.disk) forEach(disks, function normalize (disk) { disk.bootable = disk.bootable === 'true' disk.size = +disk.size From dcf63e3da2121b4c3f38479bd8589e58d3e4e781 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Thu, 11 Jun 2015 16:49:25 +0200 Subject: [PATCH 05/20] Typo. --- src/xapi.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xapi.js b/src/xapi.js index 61a0b76fb..8aaf4ed32 100644 --- a/src/xapi.js +++ b/src/xapi.js @@ -477,7 +477,7 @@ export default class Xapi extends XapiBase { // Registers the VDIs description for the provisioner. if (vdis.length) { const vdisXml = formatXml({ - provisition: { + provision: { disk: map(vdis, vdi => { const bootable = String(Boolean(vdi.bootable)) const size = String(vdi.size) From 8f1a31ad13a5137d5ec5ce996b7e07fd51c8c3c4 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Thu, 11 Jun 2015 17:20:14 +0200 Subject: [PATCH 06/20] Fix XML generation. --- src/utils.js | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/utils.js b/src/utils.js index 031e54ee0..dd22907d9 100644 --- a/src/utils.js +++ b/src/utils.js @@ -39,15 +39,7 @@ export const generateToken = (function (randomBytes) { export const formatXml = (function () { const builder = new xml2js.Builder({ - xmldec: { - // Do not include an XML header. - // - // This is not how this setting should be set but due to the - // implementation of both xml2js and xmlbuilder-js it works. - // - // TODO: Find a better alternative. - headless: true - } + headless: true }) return (...args) => builder.buildObject(...args) From f572cb5f3eeada950ee610a35c51dd517a5a8e0e Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Thu, 11 Jun 2015 17:25:58 +0200 Subject: [PATCH 07/20] Test utils.formatXml(). --- src/utils.spec.js | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/utils.spec.js b/src/utils.spec.js index 38142afff..f8463e150 100644 --- a/src/utils.spec.js +++ b/src/utils.spec.js @@ -6,12 +6,13 @@ import expect from 'must' import { ensureArray, - extractProperty + extractProperty, + formatXml } from './utils' // =================================================================== -describe('ensureArray', function () { +describe('ensureArray()', function () { it('wrap the value in an array', function () { const value = 'foo' @@ -31,7 +32,7 @@ describe('ensureArray', function () { // ------------------------------------------------------------------- -describe('extractProperty', function () { +describe('extractProperty()', function () { it('returns the value of the property', function () { const value = {} const obj = { prop: value } @@ -47,3 +48,21 @@ describe('extractProperty', function () { expect(obj).to.not.have.property('prop') }) }) + +// ------------------------------------------------------------------- + +describe('formatXml()', function () { + it('formats a JS object to an XML string', function () { + expect(formatXml({ + foo: { + bar: [ + {$: {baz: 'plop'}}, + {$: {baz: 'plip'}} + ] + } + })).to.equal(` + + +`) + }) +}) From a1a7cf59b3c753b31da2c6d29ee53bd10dbb7a34 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Thu, 11 Jun 2015 17:46:19 +0200 Subject: [PATCH 08/20] vm.create() returns the new VM id. --- src/api/vm.coffee | 2 +- src/xapi.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/api/vm.coffee b/src/api/vm.coffee index 0e349238e..19e5f1c04 100644 --- a/src/api/vm.coffee +++ b/src/api/vm.coffee @@ -41,7 +41,7 @@ create = $coroutine ({ vifs: VIFs }) - return vm.id + return vm.$id create.permission = 'admin' diff --git a/src/xapi.js b/src/xapi.js index 8aaf4ed32..a06218e55 100644 --- a/src/xapi.js +++ b/src/xapi.js @@ -471,6 +471,9 @@ export default class Xapi extends XapiBase { await this.call('VM.set_VCPUs_at_startup') } + // Removes any preexisting entry. + await this.call('VM.remove_from_other_config', vm.$ref, 'disks').catch(noop) + // TODO: remove existing VDIs (to make sure there are only those // wanted). // @@ -487,9 +490,6 @@ export default class Xapi extends XapiBase { } }) - // Removes any preexisting entry. - await this.call('VM.remove_from_other_config', vm.$ref, 'disks').catch(noop) - await this.call('VM.add_to_other_config', vm.$ref, 'disks', vdisXml) } From 7aa75539c99e7332dae0800a153a6937b34f0d40 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Thu, 11 Jun 2015 17:46:43 +0200 Subject: [PATCH 09/20] Fix VDIs creation. --- src/xapi.js | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/xapi.js b/src/xapi.js index a06218e55..88a0c6f71 100644 --- a/src/xapi.js +++ b/src/xapi.js @@ -479,22 +479,30 @@ export default class Xapi extends XapiBase { // // Registers the VDIs description for the provisioner. if (vdis.length) { + const {$default_SR: defaultSr} = vm.$pool + const vdisXml = formatXml({ provision: { - disk: map(vdis, vdi => { - const bootable = String(Boolean(vdi.bootable)) - const size = String(vdi.size) - const sr = vdi.sr || vdi.SR - return {$: {bootable, size, sr}} + disk: map(vdis, (vdi, i) => { + // Default values: + // - VDI type: system. + return {$: { + bootable: String(Boolean(vdi.bootable)), + device: String(i), + size: String(vdi.size), + sr: this.getObject(vdi.sr || vdi.SR, defaultSr).uuid + }} }) } }) + // TODO: set VDI name_label & name_description. await this.call('VM.add_to_other_config', vm.$ref, 'disks', vdisXml) } // Removes any preexisting entry. await this.call('VM.remove_from_other_config', vm.$ref, 'install-repository').catch(noop) + if (installRepository != null) { await this.call('VM.add_to_other_config', vm.$ref, 'install-repository', installRepository) } From cb65ddedb6b0fcebccd6329a54da5c2f1f26ec07 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Thu, 11 Jun 2015 17:47:46 +0200 Subject: [PATCH 10/20] Remove problematic _waitObject(). --- src/xapi.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/xapi.js b/src/xapi.js index 88a0c6f71..d4ef5ddb6 100644 --- a/src/xapi.js +++ b/src/xapi.js @@ -519,8 +519,7 @@ export default class Xapi extends XapiBase { } catch (_) {} } - // Returns the last state of the new VM. - return await this._waitObject(vm.$id) + return vm } async deleteVm (vmId, deleteDisks = false) { From 923091ca621b6f1660ebb633fde8c64f726d757c Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Fri, 12 Jun 2015 19:42:16 +0200 Subject: [PATCH 11/20] Various updates. --- package.json | 1 + src/api/disk.js | 15 +--- src/api/vdi.coffee | 7 +- src/api/vif.js | 3 +- src/api/vm.coffee | 5 +- src/xapi.js | 205 +++++++++++++++++++++++++++++++-------------- 6 files changed, 150 insertions(+), 86 deletions(-) diff --git a/package.json b/package.json index 89ae17e83..74c6fcca2 100644 --- a/package.json +++ b/package.json @@ -67,6 +67,7 @@ "lodash.map": "^3.0.0", "lodash.pick": "^3.0.0", "lodash.result": "^3.0.0", + "lodash.snakecase": "^3.0.1", "lodash.startswith": "^3.0.1", "make-error": "^1", "multikey-hash": "^1.0.1", diff --git a/src/api/disk.js b/src/api/disk.js index 26cea6cd0..c54f624d4 100644 --- a/src/api/disk.js +++ b/src/api/disk.js @@ -3,19 +3,10 @@ import {parseSize} from '../utils' // =================================================================== export async function create ({name, size, sr}) { - const xapi = this.getXAPI(sr) - - const ref = await xapi.call('VDI.create', { - name_label: name, - other_config: {}, - read_only: false, - sharable: false, - SR: sr.ref, - type: 'user', - virtual_size: String(parseSize(size)) + const vdi = await this.getXAPI(sr).createVdi(sr.id, parseSize(size), { + name_label: name }) - - return (await xapi.call('VDI.get_record', ref)).uuid + return vdi.$id } create.description = 'create a new disk on a SR' diff --git a/src/api/vdi.coffee b/src/api/vdi.coffee index fded37742..7ed7afc62 100644 --- a/src/api/vdi.coffee +++ b/src/api/vdi.coffee @@ -9,12 +9,9 @@ $isArray = require 'lodash.isarray' #===================================================================== delete_ = $coroutine ({vdi}) -> - xapi = @getXAPI vdi + yield @getXAPI(vdi).deleteVdi(vdi.id) - # TODO: check if VDI is attached before - yield xapi.call 'VDI.destroy', vdi.ref - - return true + return delete_.params = { id: { type: 'string' }, diff --git a/src/api/vif.js b/src/api/vif.js index c7d61a650..266bb251d 100644 --- a/src/api/vif.js +++ b/src/api/vif.js @@ -1,7 +1,6 @@ // TODO: move into vm and rename to removeInterface async function delete_ ({vif}) { - // TODO: check if VIF is attached before - await this.getXAPI(vif).call('VIF.destroy', vif.ref) + await this.getXAPI(vif).deleteVif(vif.id) } export {delete_ as delete} diff --git a/src/api/vm.coffee b/src/api/vm.coffee index 19e5f1c04..8b35012f8 100644 --- a/src/api/vm.coffee +++ b/src/api/vm.coffee @@ -35,8 +35,9 @@ create = $coroutine ({ VDIs VIFs }) -> - vm = yield @getXAPI(template).createVm(template.id, name_label, { + vm = yield @getXAPI(template).createVm(template.id, { installRepository: installation.repository, + nameLabel: name_label, vdis: VDIs, vifs: VIFs }) @@ -692,7 +693,7 @@ exports.attachDisk = attachDisk # FIXME: position should be optional and default to last. createInterface = $coroutine ({vm, network, position, mtu, mac}) -> - vif = yield @getXAPI(vm).createVirtualInterface(vm.id, network.id, { + vif = yield @getXAPI(vm).createVif(vm.id, network.id, { mac, mtu, position diff --git a/src/xapi.js b/src/xapi.js index d4ef5ddb6..68d7180ec 100644 --- a/src/xapi.js +++ b/src/xapi.js @@ -4,6 +4,7 @@ import find from 'lodash.find' import forEach from 'lodash.foreach' import got from 'got' import map from 'lodash.map' +import snakeCase from 'lodash.snakecase' import unzip from 'julien-f-unzip' import {PassThrough} from 'stream' import {promisify} from 'bluebird' @@ -75,6 +76,8 @@ const VM_RUNNING_POWER_STATES = { } export const isVmRunning = (vm) => VM_RUNNING_POWER_STATES[vm.power_state] +export const isVmHvm = (vm) => Boolean(vm.HVM_boot_params) + // =================================================================== export default class Xapi extends XapiBase { @@ -196,11 +199,11 @@ export default class Xapi extends XapiBase { // ================================================================= - async _setObjectProperties (id, props) { + async _setObjectProperties (object, props) { const { $ref: ref, $type: type - } = this.getObject(id) + } = object const namespace = getNamespaceForType(type) @@ -208,7 +211,7 @@ export default class Xapi extends XapiBase { // properties that failed to be set. await Promise.all(map(props, (value, name) => { if (value != null) { - return this.call(`${namespace}.set_${name}`, ref, value) + return this.call(`${namespace}.set_${snakeCase(name)}`, ref, value) } })) } @@ -217,7 +220,7 @@ export default class Xapi extends XapiBase { name_label, name_description }) { - await this._setObjectProperties(this.pool.$id, { + await this._setObjectProperties(this.pool, { name_label, name_description }) @@ -227,7 +230,7 @@ export default class Xapi extends XapiBase { name_label, name_description }) { - await this._setObjectProperties(id, { + await this._setObjectProperties(this.getObject(id), { name_label, name_description }) @@ -439,12 +442,27 @@ export default class Xapi extends XapiBase { } // TODO: clean up on error. - async createVm (templateId, nameLabel, { + async createVm (templateId, { + nameDescription = undefined, + nameLabel = undefined, cpus = undefined, installRepository = undefined, vdis = [], vifs = [] } = {}) { + const installMethod = (() => { + if (installRepository == null) { + return 'none' + } + + try { + installRepository = this.getObject(installRepository) + return 'cd' + } catch (_) { + return 'network' + } + })() + const template = this.getObject(templateId) // Clones the template. @@ -452,73 +470,94 @@ export default class Xapi extends XapiBase { await this._cloneVm(template, nameLabel) ) - // Creates the VIFs. - // - // TODO: removes existing VIFs. - { - let position = 0 - await Promise.all(map(vifs, vif => this._createVif( - vm, - this.getObject(vif.network), - { position: position++ } - ))) - } + // TODO: copy BIOS strings? - // TODO: ? await this.call('VM.set_PV_args', vm.$ref, 'noninteractive') - - // Sets the number of CPUs. - if (cpus != null) { - await this.call('VM.set_VCPUs_at_startup') - } - - // Removes any preexisting entry. - await this.call('VM.remove_from_other_config', vm.$ref, 'disks').catch(noop) - - // TODO: remove existing VDIs (to make sure there are only those - // wanted). - // - // Registers the VDIs description for the provisioner. - if (vdis.length) { - const {$default_SR: defaultSr} = vm.$pool - - const vdisXml = formatXml({ - provision: { - disk: map(vdis, (vdi, i) => { - // Default values: - // - VDI type: system. - return {$: { - bootable: String(Boolean(vdi.bootable)), - device: String(i), - size: String(vdi.size), - sr: this.getObject(vdi.sr || vdi.SR, defaultSr).uuid - }} - }) - } - }) - - // TODO: set VDI name_label & name_description. - await this.call('VM.add_to_other_config', vm.$ref, 'disks', vdisXml) - } - - // Removes any preexisting entry. - await this.call('VM.remove_from_other_config', vm.$ref, 'install-repository').catch(noop) - - if (installRepository != null) { - await this.call('VM.add_to_other_config', vm.$ref, 'install-repository', installRepository) - } + // TODO: Rewrite provision XML. // Creates the VDIs and executes the initial steps of the // installation. await this.call('VM.provision', vm.$ref) - if (installRepository != null) { - try { - const cd = this.getObject(installRepository) + // Set VMs params. + this._setObjectProperties(vm, { + nameDescription, + VCPUs_at_startup: cpus + }) - await this._insertCdIntoVm(cd, vm) - } catch (_) {} + // Sets boot parameters. + { + const isHvm = isVmHvm(vm) + + if (isHvm) { + if (!vdis.length || installMethod === 'network') { + // TODO: set boot order + } + } else { // PV + if (vm.PV_bootloader === 'eliloader') { + // Removes any preexisting entry. + await this.call('VM.remove_from_other_config', vm.$ref, 'install-repository').catch(noop) + + if (installMethod === 'network') { + // TODO: normalize RHEL URL? + + await this.call('VM.add_to_other_config', vm.$ref, 'install-repository', installRepository) + } else if (installMethod === 'cd') { + await this.call('VM.add_to_other_config', vm.$ref, 'install-repository', 'cdrom') + await this._insertCdIntoVm(installRepository, vm) + } + } + + // TODO: set PV args. + } } + // Creates the VDIs. + // + // TODO: set vm.suspend_SR + { + const {$default_SR: defaultSr} = this.pool + let position = 0 + await Promise.all(map(vdis, (vdiDescription, i) => { + return this._createVdi( + this.getObject(vdiDescription.sr || vdiDescription.SR, defaultSr), + vdiDescription.size, + { + name_label: vdiDescription.name_label, + name_description: vdiDescription.name_description + } + ) + .then(ref => this._getOrWaitObject(ref)) + .then(vdi => this._createVbd(vm, vdi, { + // TODO: should bootable be in the description or be + // deduced by the position in the array (i === 0)? + bootable: vdiDescription.bootable, + + position: position++ + })) + })) + } + + // Destroys the VIFs cloned from the template. + await Promise.all(map(vm.$vifs, vif => this._deleteVif(vif))) + + // Creates the VIFs specified by the user. + { + let position = 0 + await Promise.all(map(vifs, vif => this._createVif( + vm, + this.getObject(vif.network), + { + position: position++, + mac: vif.mac, + mtu: vif.mtu + } + ))) + } + + // TODO: Create Cloud config drives. + + // TODO: Assign VGPUs. + return vm } @@ -636,6 +675,23 @@ export default class Xapi extends XapiBase { return vbdRef } + async _createVdi (sr, size, { + name_label = '', + name_description = undefined + } = {}) { + return await this.call('VDI.create', { + name_label: name_label, + name_description: name_description, + other_config: {}, + read_only: false, + sharable: false, + SR: sr.$ref, + type: 'user', + virtual_size: String(size) + }) + } + + // TODO: check whether the VDI is attached. async _deleteVdi (vdiId) { const vdi = this.getObject(vdiId) @@ -680,6 +736,16 @@ export default class Xapi extends XapiBase { ) } + async createVdi (srId, size, opts) { + return await this._getOrWaitObject( + await this._createVdi(this.getObject(srId), size, opts) + ) + } + + async deleteVdi (vdiId) { + await this._deleteVdi(this.getObject(vdiId)) + } + async insertCdIntoVm (cdId, vmId, force = undefined) { await this._insertCdIntoVm( this.getObject(cdId), @@ -725,7 +791,12 @@ export default class Xapi extends XapiBase { return vifRef } - async createVirtualInterface (vmId, networkId, opts = undefined) { + // TODO: check whether the VIF was unplugged before. + async _deleteVif (vif) { + await this.call('VIF.destroy', vif.$ref) + } + + async createVif (vmId, networkId, opts = undefined) { return await this._getOrWaitObject( await this._createVif( this.getObject(vmId), @@ -735,6 +806,10 @@ export default class Xapi extends XapiBase { ) } + async deleteVif (vifId) { + await this._deleteVif(this.getObject(vifId)) + } + // ================================================================= async _doDockerAction (vmId, action, containerId) { From de693a08ad78091a115f692d7b332fecbd5ae124 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Thu, 18 Jun 2015 15:06:30 +0200 Subject: [PATCH 12/20] vm.create() seems to work ok. --- src/xapi.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/xapi.js b/src/xapi.js index 68d7180ec..b6aed9be1 100644 --- a/src/xapi.js +++ b/src/xapi.js @@ -472,7 +472,9 @@ export default class Xapi extends XapiBase { // TODO: copy BIOS strings? - // TODO: Rewrite provision XML. + // Removes disks from the provision XML, we will create them by + // ourselves. + await this.call('VM.remove_from_other_config', vm.$ref, 'disks').catch(noop) // Creates the VDIs and executes the initial steps of the // installation. From 7d82e199b7de7a7eabeec8539c339e6f27bbe025 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Thu, 18 Jun 2015 15:08:16 +0200 Subject: [PATCH 13/20] Fix Xapi#_deleteVdi(). --- src/xapi.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/xapi.js b/src/xapi.js index b6aed9be1..a7455a7a2 100644 --- a/src/xapi.js +++ b/src/xapi.js @@ -694,9 +694,7 @@ export default class Xapi extends XapiBase { } // TODO: check whether the VDI is attached. - async _deleteVdi (vdiId) { - const vdi = this.getObject(vdiId) - + async _deleteVdi (vdi) { await this.call('VDI.destroy', vdi.$ref) } From 2f4a76b3fa9b70b298c34377c66586b989daeb69 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Thu, 18 Jun 2015 15:18:42 +0200 Subject: [PATCH 14/20] Fix handling when vm.create(installation) is undefined. --- src/api/vm.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/api/vm.coffee b/src/api/vm.coffee index 8b35012f8..047b6b5e3 100644 --- a/src/api/vm.coffee +++ b/src/api/vm.coffee @@ -36,7 +36,7 @@ create = $coroutine ({ VIFs }) -> vm = yield @getXAPI(template).createVm(template.id, { - installRepository: installation.repository, + installRepository: installation && installation.repository, nameLabel: name_label, vdis: VDIs, vifs: VIFs From 379771a5c1de91187dca0601d8ce55f6eb0f0d00 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Thu, 18 Jun 2015 15:26:31 +0200 Subject: [PATCH 15/20] Remove unused import. --- src/xapi.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/xapi.js b/src/xapi.js index a7455a7a2..9d4eb1972 100644 --- a/src/xapi.js +++ b/src/xapi.js @@ -16,7 +16,6 @@ import { import {debounce} from './decorators' import { ensureArray, - formatXml, noop, parseXml, pFinally } from './utils' From 0abfb9f1e4b7a5682d9d8ee9304b2943ed3d7a44 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Thu, 18 Jun 2015 16:09:59 +0200 Subject: [PATCH 16/20] vm.create() can set the description. --- src/api/vm.coffee | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/api/vm.coffee b/src/api/vm.coffee index 047b6b5e3..8f623cfae 100644 --- a/src/api/vm.coffee +++ b/src/api/vm.coffee @@ -30,6 +30,7 @@ $isVMRunning = do -> # TODO: Implement ACLs create = $coroutine ({ installation + name_description name_label template VDIs @@ -37,6 +38,7 @@ create = $coroutine ({ }) -> vm = yield @getXAPI(template).createVm(template.id, { installRepository: installation && installation.repository, + nameDescription: name_description, nameLabel: name_label, vdis: VDIs, vifs: VIFs @@ -56,8 +58,9 @@ create.params = { } } - # Name of the new VM. + # Name/description of the new VM. name_label: { type: 'string' } + name_description: { type: 'string', optional: true } # TODO: add the install repository! # VBD.insert/eject From f393c7173c5544ebb6da0c16edecb2cddd2c3f90 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Thu, 18 Jun 2015 16:10:32 +0200 Subject: [PATCH 17/20] vm.create() only one VBD is bootable. --- src/api/vm.coffee | 1 - src/xapi.js | 8 ++------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/api/vm.coffee b/src/api/vm.coffee index 8f623cfae..d0c93f6b0 100644 --- a/src/api/vm.coffee +++ b/src/api/vm.coffee @@ -93,7 +93,6 @@ create.params = { items: { type: 'object' properties: { - bootable: { type: 'boolean' } device: { type: 'string' } size: { type: 'integer' } SR: { type: 'string' } diff --git a/src/xapi.js b/src/xapi.js index 9d4eb1972..1da5960e3 100644 --- a/src/xapi.js +++ b/src/xapi.js @@ -517,7 +517,6 @@ export default class Xapi extends XapiBase { // TODO: set vm.suspend_SR { const {$default_SR: defaultSr} = this.pool - let position = 0 await Promise.all(map(vdis, (vdiDescription, i) => { return this._createVdi( this.getObject(vdiDescription.sr || vdiDescription.SR, defaultSr), @@ -529,11 +528,8 @@ export default class Xapi extends XapiBase { ) .then(ref => this._getOrWaitObject(ref)) .then(vdi => this._createVbd(vm, vdi, { - // TODO: should bootable be in the description or be - // deduced by the position in the array (i === 0)? - bootable: vdiDescription.bootable, - - position: position++ + // Only the first VBD if installMethod is not cd is bootable. + bootable: installMethod !== 'cd' && !i })) })) } From 123677b6c63a1a87f68593caea0aaa9d9ec0ccfe Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Thu, 18 Jun 2015 16:10:45 +0200 Subject: [PATCH 18/20] Fix isVmHvm(). --- src/xapi.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xapi.js b/src/xapi.js index 1da5960e3..af707f72d 100644 --- a/src/xapi.js +++ b/src/xapi.js @@ -75,7 +75,7 @@ const VM_RUNNING_POWER_STATES = { } export const isVmRunning = (vm) => VM_RUNNING_POWER_STATES[vm.power_state] -export const isVmHvm = (vm) => Boolean(vm.HVM_boot_params) +export const isVmHvm = (vm) => Boolean(vm.HVM_boot_policy) // =================================================================== From e7b3e28f762e9d863e012c7e94040ea8295ab988 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Thu, 18 Jun 2015 16:11:08 +0200 Subject: [PATCH 19/20] vm.delete() must not remove unpluggable VDIs. --- src/xapi.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/xapi.js b/src/xapi.js index af707f72d..5c5d60b80 100644 --- a/src/xapi.js +++ b/src/xapi.js @@ -567,6 +567,11 @@ export default class Xapi extends XapiBase { if (deleteDisks) { await Promise.all(map(vm.$VBDs, vbd => { + // Do not delete unpluggable VDIs. + if (vbd.unpluggable) { + return + } + try { return this._deleteVdi(vbd.$VDI).catch(noop) } catch (_) {} From 8f79699e849d11ee440e60afcc218567beebe895 Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Thu, 18 Jun 2015 16:11:37 +0200 Subject: [PATCH 20/20] Xapi#_createVbd() correctly finds a default position. --- src/xapi.js | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/xapi.js b/src/xapi.js index 5c5d60b80..55fa6a3ed 100644 --- a/src/xapi.js +++ b/src/xapi.js @@ -644,16 +644,8 @@ export default class Xapi extends XapiBase { type = 'Disk', readOnly = (type !== 'Disk') }) { - // TODO: use VM.get_allowed_VBD_devices()? if (position == null) { - forEach(vm.$VBDs, vbd => { - const curPos = +vbd.userdevice - if (!(position > curPos)) { - position = curPos - } - }) - - position = position == null ? 0 : position + 1 + position = (await this.call('VM.get_allowed_VBD_devices', vm.$ref))[0] } const vbdRef = await this.call('VBD.create', {