fix(vm.deltaCopy): correctly destroy VDIs (2) (#591)

src/xapi/index.js

@@ -54,6 +54,7 @@ import {
   extractOpaqueRef,
   filterUndefineds,
   getNamespaceForType,
+  getVmDisks,
   canSrHaveNewVdiOfSize,
   isVmHvm,
   isVmRunning,
@@ -647,47 +648,52 @@ export default class Xapi extends XapiBase {
   async _deleteVm (vm, deleteDisks = true) {
     debug(`Deleting VM ${vm.name_label}`)
 
+    const { $ref } = vm
+
     // It is necessary for suspended VMs to be shut down
     // to be able to delete their VDIs.
     if (vm.power_state !== 'Halted') {
-      await this.call('VM.hard_shutdown', vm.$ref)
+      await this.call('VM.hard_shutdown', $ref)
     }
 
-    await this.barrier('VM', vm.$ref)
+    await this.barrier('VM', $ref)
 
-    if (deleteDisks) {
-      // Compute the VDIs list without duplicates.
-      const vdis = {}
-      forEach(vm.$VBDs, vbd => {
-        let vdi
-        if (
-          // Do not remove CDs and Floppies.
-          vbd.type === 'Disk' &&
+    return Promise.all([
+      this.call('VM.destroy', $ref),
 
-          // Ignore VBD without VDI.
-          (vdi = vbd.$VDI)
-        ) {
-          vdis[vdi.$id] = vdi
+      asyncMap(vm.$snapshots, snapshot =>
+        this._deleteVm(snapshot)
+      )::ignoreErrors(),
+
+      deleteDisks && asyncMap(getVmDisks(vm), ({ $ref: vdiRef }) => {
+        let onFailure = () => {
+          onFailure = vdi => {
+            console.error(`cannot delete VDI ${vdi.name_label} (from VM ${vm.name_label})`)
+            forEach(vdi.$VBDs, vbd => {
+              if (vbd.VM !== $ref) {
+                const vm = vbd.$VM
+                console.error('- %s (%s)', vm.name_label, vm.uuid)
+              }
+            })
+          }
+
+          // maybe the control domain has not yet unmounted the VDI,
+          // check and retry after 5 seconds
+          return pDelay(5e3).then(test)
         }
-      })
-
-      await Promise.all(mapToArray(vdis, vdi => {
-        if (
-          // Do not remove VBDs attached to other VMs.
-          vdi.VBDs.length < 2 ||
-          every(vdi.$VBDs, vbd => vbd.VM === vm.$ref)
-        ) {
-          return this._deleteVdi(vdi)::ignoreErrors()
+        const test = () => {
+          const vdi = this.getObjectByRef(vdiRef)
+          return (
+            // Only remove VBDs not attached to other VMs.
+            vdi.VBDs.length < 2 ||
+            every(vdi.$VBDs, vbd => vbd.VM === $ref)
+          )
+            ? this._deleteVdi(vdi)
+            : onFailure(vdi)
         }
-        console.error(`cannot delete VDI ${vdi.name_label} (from VM ${vm.name_label})`)
-      }))
-    }
-
-    await Promise.all(mapToArray(vm.$snapshots, snapshot =>
-      this.deleteVm(snapshot.$id)::ignoreErrors()
-    ))
-
-    await this.call('VM.destroy', vm.$ref)
+        return test()
+      })::ignoreErrors()
+    ])
   }
 
   async deleteVm (vmId, deleteDisks) {

src/xapi/utils.js

@@ -113,6 +113,25 @@ export const getNamespaceForType = type => TYPE_TO_NAMESPACE[type] || type
 
 // -------------------------------------------------------------------
 
+export const getVmDisks = vm => {
+  const disks = createRawObject(null)
+  forEach(vm.$VBDs, vbd => {
+    let vdi
+    if (
+      // Do not remove CDs and Floppies.
+      vbd.type === 'Disk' &&
+
+      // Ignore VBD without VDI.
+      (vdi = vbd.$VDI)
+    ) {
+      disks[vdi.$id] = vdi
+    }
+  })
+  return disks
+}
+
+// -------------------------------------------------------------------
+
 // Format a date (pseudo ISO 8601) from one XenServer get by
 // xapi.call('host.get_servertime', host.$ref) for example
 export const formatDateTime = utcFormat('%Y%m%dT%H:%M:%SZ')