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

@@ -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,
   })
 

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, {
+  async createVdi ({
-    name_description = undefined,
+    name_description,
-    name_label = '',
+    name_label,
     other_config = {},
     read_only = false,
     sharable = false,
-
+    sm_config,
-    // FIXME: should be named srId or an object.
+    SR,
-    sr = this.pool.default_SR,
+    tags,
-
-    tags = [],
     type = 'user',
-    xenstore_data = undefined,
+    virtual_size,
-  } = {}) {
+    xenstore_data,
-    if (sr === NULL_REF) {
-      throw new Error('SR required to create VDI')
-    }
 
+    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)
+    return this._getOrWaitObject(await this.call('VDI.create', {
-    read_only = Boolean(read_only)
-
-    const data = {
       name_description,
       name_label,
       other_config,
       read_only,
       sharable,
+      sm_config,
+      SR: sr.$ref,
       tags,
       type,
-      virtual_size: String(size),
+      virtual_size: size !== undefined ? parseSize(size) : virtual_size,
-      SR: sr.$ref,
+      xenstore_data,
-    }
+    }))
-
-    if (xenstore_data) {
-      data.xenstore_data = xenstore_data
-    }
-
-    return /* await */ this.call('VDI.create', 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, {
+    const vdi = await this.createVdi({
-      sr: sr.$ref,
-      name_label,
       name_description,
+      name_label,
+      size: stream.length,
+      sr: sr.$ref,
     })
     $defer.onFailure(() => this._deleteVdi(vdi))
 

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(
+      await Promise.all(mapToArray(vdis, (vdiDescription, i) => this.createVdi({
-        vdiDescription.size, // FIXME: Should not be done in Xapi.
+        name_description: vdiDescription.name_description,
-        {
+        name_label: vdiDescription.name_label,
-          name_label: vdiDescription.name_label,
+        size: vdiDescription.size,
-          name_description: vdiDescription.name_description,
+        sr: vdiDescription.sr || vdiDescription.SR,
-          sr: vdiDescription.sr || vdiDescription.SR,
+      })
-        }
-      )
-        .then(ref => this._getOrWaitObject(ref))
         .then(vdi => this.createVbd({
           // Either the CD or the 1st disk is bootable (only useful for PV VMs)
           bootable: !(hasBootableDisk || i),