server: removed potentially long mutex lock when import/export files into VM

updated tests as they were not running anymore
client: fixed VM display in vmedit

src/client/cmdVM/vmCreate.go

@@ -124,7 +124,7 @@ func (cmd *Create) Run(args []string) int {
 
 		vm, err := libclient.VMCreate(vmArgs)
 		if err != nil {
-			u.PrintlnErr(err)
+			u.PrintlnErr("Error: failed creating VM: " + err.Error())
 			statusCode = 1
 		} else {
 			u.Println("Created VM \"" + vm.Name + "\" | " + vm.ID.String())

src/client/cmdVM/vmEdit.go

@@ -82,8 +82,14 @@ func (cmd *Edit) Run(args []string) int {
 			u.PrintlnErr(err)
 			statusCode = 1
 		} else {
-			u.Println("Successfully edited VM \"" + vmID + "\":")
-			u.Println(vmEdited.String())
+			str, err := vmEdited.String()
+			if err != nil {
+				u.Println("Successfully edited VM \"" + vmID + "\" but failed decoding VM to string.")
+				statusCode = 1
+			} else {
+				u.Println("Successfully edited VM \"" + vmID + "\":")
+				u.Println(str)
+			}
 		}
 	}
 

src/client/cmdVM/vmList.go

@@ -61,11 +61,14 @@ func (cmd *List) printFilteredVMs(args []string, route string) int {
 		return 1
 	}
 
+	statusCode := 0
+
 	if foundLongOutputFlag >= 0 {
 		for _, vm := range vms {
 			str, err := vm.String()
 			if err != nil {
 				u.PrintlnErr("Failed decoding VM " + vm.ID.String() + " to string. Skipped.")
+				statusCode = 1
 			} else {
 				u.Println(str)
 			}
@@ -94,5 +97,5 @@ func (cmd *List) printFilteredVMs(args []string, route string) int {
 		}
 	}
 
-	return 0
+	return statusCode
 }

src/client/cmdVM/vmListSingle.go

@@ -42,11 +42,13 @@ func (cmd *ListSingle) Run(args []string) int {
 		return 1
 	}
 
-	vmStr, err := vm.String()
+	str, err := vm.String()
 	if err != nil {
-		u.PrintlnErr(err)
+		u.PrintlnErr("Failed decoding VM " + vm.ID.String() + " to string. Skipped.")
 		return 1
+	} else {
+		u.Println(str)
 	}
-	u.Println(vmStr)
+
 	return 0
 }

src/server/vms/vms.go

@@ -375,15 +375,13 @@ func (vms *VMs) prepareStartVM(vmID uuid.UUID, attachPwd string) (*osexec.Cmd, *
 // in argument is used.
 // Concurrency: safe
 func (vms *VMs) StartVM(vmID uuid.UUID, attachPwd string) error {
-	prefix := "Failed starting VM[2]: "
-
 	cmd, vm, attachPwd, spicePort, estimatedVmRAM, err := vms.prepareStartVM(vmID, attachPwd)
 	if err != nil {
 		return err
 	} else {
 		go func() {
 			if err := cmd.Wait(); err != nil {
-				log.Error(prefix + vm.v.ID.String() + ": exec.Wait error: " + err.Error())
+				log.Error("Failed starting VM: " + vm.v.ID.String() + ": exec.Wait error: " + err.Error())
 				log.Error("Failed cmd: " + cmd.String())
 			}
 
@@ -783,27 +781,35 @@ func (vms *VMs) ExportVMFiles(vm *VM, vmDir, tarGzFile string) error {
 	prefix := "Failed exporting files from VM: "
 
 	vm.mutex.Lock()
-	defer vm.mutex.Unlock()
 
 	if vm.IsRunning() {
+		vm.mutex.Unlock()
 		return errors.New(prefix + "VM must be stopped")
 	}
 
 	if vm.IsDiskBusy() {
+		vm.mutex.Unlock()
 		return errors.New(prefix + "disk in use (busy)")
 	}
 
 	vm.DiskBusy = true
-	defer func(vm *VM) { vm.DiskBusy = false }(vm)
+	vm.mutex.Unlock()
 
 	vmDisk := vm.getDiskPath()
 
 	if err := exec.CopyFromVM(vmDisk, vmDir, tarGzFile); err != nil {
+		vm.mutex.Lock()
+		vm.DiskBusy = false
+		vm.mutex.Unlock()
 		msg := prefix + err.Error()
 		log.Error(msg)
 		return errors.New(msg)
 	}
 
+	vm.mutex.Lock()
+	vm.DiskBusy = false
+	vm.mutex.Unlock()
+
 	return nil
 }
 
@@ -813,26 +819,34 @@ func (vms *VMs) ImportFilesToVM(vm *VM, tarGzFile, vmDir string) error {
 	prefix := "Failed importing files into VM: "
 
 	vm.mutex.Lock()
-	defer vm.mutex.Unlock()
 
 	if vm.IsRunning() {
+		vm.mutex.Unlock()
 		return errors.New(prefix + "VM must be stopped")
 	}
 
 	if vm.IsDiskBusy() {
+		vm.mutex.Unlock()
 		return errors.New(prefix + "disk in use (busy)")
 	}
 
 	vm.DiskBusy = true
-	defer func(vm *VM) { vm.DiskBusy = false }(vm)
+	vm.mutex.Unlock()
 
 	vmDisk := vm.getDiskPath()
 
 	if err := exec.CopyToVM(vmDisk, tarGzFile, vmDir); err != nil {
+		vm.mutex.Lock()
+		vm.DiskBusy = false
+		vm.mutex.Unlock()
 		msg := prefix + err.Error()
 		log.Error(msg)
 		return errors.New(msg)
 	}
 
+	vm.mutex.Lock()
+	vm.DiskBusy = false
+	vm.mutex.Unlock()
+
 	return nil
 }

tests/run_tests

@@ -26,7 +26,7 @@ cleanup () {
     $nexus_cli vmkill "$vm_base_name"
     sleep 3
     $nexus_cli vmdel "$vm_tpl_name"
-    $nexus_cli tpldel "$vm_tpl_name"
+    $nexus_cli tpldel "$template_id"
     echo "calling del_gen_files..."
     del_gen_files
 }
@@ -76,6 +76,8 @@ OK
 
 echo -e "Starting validation tests...\n"
 
+echo "[1]==========================================="
+
 # Look for the first template containing "Xubuntu 22.04" in its name
 tpl="Xubuntu 22.04"
 echo "List templates..."
@@ -91,12 +93,12 @@ check_last_cmd_failed "vmcreate"
 OK
 
 echo "Attempt to create an invalid VM (invalid CPU)..."
-vm_id=`$nexus_cli vmcreate "$vm_base_name" 17 1024 none none $base_template_id`
+vm_id=`$nexus_cli vmcreate "$vm_base_name" 33 1024 none none $base_template_id`
 check_last_cmd_failed "vmcreate"
 OK
 
 echo "Attempt to create an invalid VM (invalid RAM)..."
-vm_id=`$nexus_cli vmcreate "$vm_base_name" 1 250 user 067b:2303 $base_template_id`
+vm_id=`$nexus_cli vmcreate "$vm_base_name" 1 127 user 067b:2303 $base_template_id`
 check_last_cmd_failed "vmcreate"
 OK
 
@@ -120,6 +122,8 @@ vm_id=`$nexus_cli vmcreate "$vm_base_name" 1 1024 none 067b:23x3 $base_template_
 check_last_cmd_failed "vmcreate"
 OK
 
+echo "[2]==========================================="
+
 passes=3
 echo -e "Start stress tests: $passes passes...\n"
 
@@ -158,6 +162,8 @@ done
 
 echo -e "\nStress tests completed.\n"
 
+echo "[3]==========================================="
+
 echo "Create base VM..."
 vm_id=`$nexus_cli vmcreate "$vm_base_name" 2 1024 user 067b:2303 $base_template_id`
 check_last_cmd_succeeded "vmcreate"
@@ -182,6 +188,8 @@ $nexus_cli tpledit "$template_id" name="$vm_tpl_name extra"
 check_last_cmd_succeeded "tpledit"
 OK
 
+echo "[4]==========================================="
+
 echo "Create students VMs..."
 $nexus_cli vmcreate "$vm_base_name" 2 1024 none 1fc9:0132,03eb:6124 $template_id data/students.csv
 check_last_cmd_succeeded "vmcreate VMs"
@@ -208,6 +216,8 @@ $nexus_cli vmcreds2csv "$vm_base_name" $creds_csv_file
 check_last_cmd_succeeded "vmcreds2csv"
 OK
 
+echo "[5]==========================================="
+
 echo "Kill students VMs..."
 $nexus_cli vmkill "$vm_base_name"
 sleep 3
@@ -215,7 +225,7 @@ check_last_cmd_succeeded "vmkill VMs"
 OK
 
 echo "Export files from VMs..."
-$nexus_cli vmexportdir "$vm_base_name" /home
+$nexus_cli vmexportdir "$vm_base_name" /usr/share/pixmaps
 check_last_cmd_succeeded "vmexportdir"
 OK