From 12153a414d4346e0179f7fa94d94eb91da40a7ce Mon Sep 17 00:00:00 2001
From: Pierre Donias <pierre.donias@gmail.com>
Date: Tue, 26 Oct 2021 16:53:09 +0200
Subject: [PATCH] fix(xo-server/{clone,copy}Vm): force `is_a_template` to
 `false` on the new VM (#5955)

See xoa-support#4137
---
 CHANGELOG.unreleased.md           |  1 +
 packages/xo-server/src/api/vm.mjs | 41 ++++++++++++++++++++-----------
 2 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md
index 5513ddc49..9863df346 100644
--- a/CHANGELOG.unreleased.md
+++ b/CHANGELOG.unreleased.md
@@ -18,6 +18,7 @@
 - [Backups] Delete unused snapshots related to other schedules (even no longer existing) (PR [#5949](https://github.com/vatesfr/xen-orchestra/pull/5949))
 - [Jobs] Fix `job.runSequence` method (PR [#5944](https://github.com/vatesfr/xen-orchestra/pull/5944))
 - [Netbox] Fix error when testing plugin on versions older than 2.10 (PR [#5963](https://github.com/vatesfr/xen-orchestra/pull/5963))
+- [Snapshot] Fix "Create VM from snapshot" creating a template instead of a VM (PR [#5955](https://github.com/vatesfr/xen-orchestra/pull/5955))
 
 ### Packages to release
 
diff --git a/packages/xo-server/src/api/vm.mjs b/packages/xo-server/src/api/vm.mjs
index 0949b86af..897fb398f 100644
--- a/packages/xo-server/src/api/vm.mjs
+++ b/packages/xo-server/src/api/vm.mjs
@@ -659,22 +659,28 @@ export const clone = defer(async function ($defer, { vm, name, full_copy: fullCo
   await checkPermissionOnSrs.call(this, vm)
   const xapi = this.getXapi(vm)
 
-  const { $id: cloneId, $ref: cloneRef } = await xapi.cloneVm(vm._xapiRef, {
+  const newVm = await xapi.cloneVm(vm._xapiRef, {
     nameLabel: name,
     fast: !fullCopy,
   })
-  $defer.onFailure(() => xapi.VM_destroy(cloneRef))
+  $defer.onFailure(() => xapi.VM_destroy(newVm.$ref))
+
+  // A snapshot may have its `is_a_template` flag set to true, which isn't
+  // automatically set to false when cloning it
+  if (vm.type !== 'VM-template') {
+    await newVm.set_is_a_template(false)
+  }
 
   const isAdmin = this.user.permission === 'admin'
   if (!isAdmin) {
-    await this.addAcl(this.user.id, cloneId, 'admin')
+    await this.addAcl(this.user.id, newVm.$id, 'admin')
   }
 
   if (vm.resourceSet !== undefined) {
     await this.allocateLimitsInResourceSet(await this.computeVmResourcesUsage(vm), vm.resourceSet, isAdmin)
   }
 
-  return cloneId
+  return newVm.$id
 })
 
 clone.params = {
@@ -691,25 +697,32 @@ clone.resolve = {
 
 // TODO: implement resource sets
 export async function copy({ compress, name: nameLabel, sr, vm }) {
+  let newVm
   if (vm.$pool === sr.$pool) {
     if (vm.power_state === 'Running') {
       await checkPermissionOnSrs.call(this, vm)
     }
 
-    return this.getXapi(vm)
-      .copyVm(vm._xapiId, {
+    newVm = await this.getXapi(vm).copyVm(vm._xapiId, {
+      nameLabel,
+      srOrSrId: sr._xapiId,
+    })
+  } else {
+    newVm = (
+      await this.getXapi(vm).remoteCopyVm(vm._xapiId, this.getXapi(sr), sr._xapiId, {
+        compress,
         nameLabel,
-        srOrSrId: sr._xapiId,
       })
-      .then(vm => vm.$id)
+    ).vm
   }
 
-  return this.getXapi(vm)
-    .remoteCopyVm(vm._xapiId, this.getXapi(sr), sr._xapiId, {
-      compress,
-      nameLabel,
-    })
-    .then(({ vm }) => vm.$id)
+  // A snapshot may have its `is_a_template` flag set to true, which isn't
+  // automatically set to false when copying it
+  if (vm.type !== 'VM-template') {
+    await newVm.set_is_a_template(false)
+  }
+
+  return newVm.$id
 }
 
 copy.params = {