From 0bcabb9f0da509d0f84aec0200d48edf9d71bf4d Mon Sep 17 00:00:00 2001 From: Julien Fontanet Date: Fri, 29 Dec 2017 23:26:56 +0100 Subject: [PATCH] chore(Xapi#createVdi): better implementation (#637) - accept a VDI record as sole parameter - accept a high level size parameter which will be parsed if necessary - remove unused `Xapi#_createVdi()` --- src/api/disk.js | 3 +- src/xapi/index.js | 68 +++++++++++++++++++------------------------ src/xapi/mixins/vm.js | 15 ++++------ 3 files changed, 38 insertions(+), 48 deletions(-) diff --git a/src/api/disk.js b/src/api/disk.js index 9a222bfd6..69f0a8b5a 100644 --- a/src/api/disk.js +++ b/src/api/disk.js @@ -15,8 +15,9 @@ export async function create ({ name, size, sr, vm, bootable, position, mode }) } const xapi = this.getXapi(sr) - const vdi = await xapi.createVdi(parseSize(size), { + const vdi = await xapi.createVdi({ name_label: name, + size, sr: sr._xapiId, }) diff --git a/src/xapi/index.js b/src/xapi/index.js index b84e8a60b..21554e026 100644 --- a/src/xapi/index.js +++ b/src/xapi/index.js @@ -39,6 +39,7 @@ import { map, mapToArray, pAll, + parseSize, pDelay, pFinally, promisifyAll, @@ -971,7 +972,7 @@ export default class Xapi extends XapiBase { const newVdis = await map(delta.vdis, async vdi => { const remoteBaseVdiUuid = vdi.other_config[TAG_BASE_DELTA] if (!remoteBaseVdiUuid) { - const newVdi = await this.createVdi(vdi.virtual_size, { + const newVdi = await this.createVdi({ ...vdi, other_config: { ...vdi.other_config, @@ -1263,9 +1264,10 @@ export default class Xapi extends XapiBase { const vifDevices = await this.call('VM.get_allowed_VIF_devices', vm.$ref) await Promise.all( map(disks, async disk => { - const vdi = vdis[disk.path] = await this.createVdi(disk.capacity, { + const vdi = vdis[disk.path] = await this.createVdi({ name_description: disk.descriptionLabel, name_label: disk.nameLabel, + size: disk.capacity, sr: sr.$ref, }) $defer.onFailure(() => this._deleteVdi(vdi)) @@ -1608,47 +1610,38 @@ export default class Xapi extends XapiBase { return this.call('VDI.clone', vdi.$ref) } - async _createVdi (size, { - name_description = undefined, - name_label = '', + async createVdi ({ + name_description, + name_label, other_config = {}, read_only = false, sharable = false, - - // FIXME: should be named srId or an object. - sr = this.pool.default_SR, - - tags = [], + sm_config, + SR, + tags, type = 'user', - xenstore_data = undefined, - } = {}) { - if (sr === NULL_REF) { - throw new Error('SR required to create VDI') - } + virtual_size, + xenstore_data, + size, + sr = SR !== undefined && SR !== NULL_REF ? SR : this.pool.default_SR, + } = {}) { sr = this.getObject(sr) debug(`Creating VDI ${name_label} on ${sr.name_label}`) - sharable = Boolean(sharable) - read_only = Boolean(read_only) - - const data = { + return this._getOrWaitObject(await this.call('VDI.create', { name_description, name_label, other_config, read_only, sharable, + sm_config, + SR: sr.$ref, tags, type, - virtual_size: String(size), - SR: sr.$ref, - } - - if (xenstore_data) { - data.xenstore_data = xenstore_data - } - - return /* await */ this.call('VDI.create', data) + virtual_size: size !== undefined ? parseSize(size) : virtual_size, + xenstore_data, + })) } async moveVdi (vdiId, srId) { @@ -1780,12 +1773,6 @@ export default class Xapi extends XapiBase { ) } - async createVdi (size, opts) { - return /* await */ this._getOrWaitObject( - await this._createVdi(size, opts) - ) - } - async deleteVdi (vdiId) { await this._deleteVdi(this.getObject(vdiId)) } @@ -2106,7 +2093,11 @@ export default class Xapi extends XapiBase { // First, create a small VDI (10MB) which will become the ConfigDrive const buffer = fatfsBufferInit() - const vdi = await this.createVdi(buffer.length, { name_label: 'XO CloudConfigDrive', sr: sr.$ref }) + const vdi = await this.createVdi({ + name_label: 'XO CloudConfigDrive', + size: buffer.length, + sr: sr.$ref, + }) $defer.onFailure(() => this._deleteVdi(vdi)) // Then, generate a FAT fs @@ -2131,10 +2122,11 @@ export default class Xapi extends XapiBase { @deferrable async createTemporaryVdiOnSr ($defer, stream, sr, name_label, name_description) { - const vdi = await this.createVdi(stream.length, { - sr: sr.$ref, - name_label, + const vdi = await this.createVdi({ name_description, + name_label, + size: stream.length, + sr: sr.$ref, }) $defer.onFailure(() => this._deleteVdi(vdi)) diff --git a/src/xapi/mixins/vm.js b/src/xapi/mixins/vm.js index 1c16ca80a..bd628e4aa 100644 --- a/src/xapi/mixins/vm.js +++ b/src/xapi/mixins/vm.js @@ -153,15 +153,12 @@ export default { // TODO: set vm.suspend_SR if (!isEmpty(vdis)) { const devices = await this.call('VM.get_allowed_VBD_devices', vm.$ref) - await Promise.all(mapToArray(vdis, (vdiDescription, i) => this._createVdi( - vdiDescription.size, // FIXME: Should not be done in Xapi. - { - name_label: vdiDescription.name_label, - name_description: vdiDescription.name_description, - sr: vdiDescription.sr || vdiDescription.SR, - } - ) - .then(ref => this._getOrWaitObject(ref)) + await Promise.all(mapToArray(vdis, (vdiDescription, i) => this.createVdi({ + name_description: vdiDescription.name_description, + name_label: vdiDescription.name_label, + size: vdiDescription.size, + sr: vdiDescription.sr || vdiDescription.SR, + }) .then(vdi => this.createVbd({ // Either the CD or the 1st disk is bootable (only useful for PV VMs) bootable: !(hasBootableDisk || i),