From 29d773ed30bb9af3d8d33c605d57db11f764501b Mon Sep 17 00:00:00 2001
From: Florent <florent.gluck@hesge.ch>
Date: Sat, 28 Dec 2024 23:53:04 +0100
Subject: [PATCH] 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 | 2 +-
src/client/cmdVM/vmEdit.go | 10 ++++++++--
src/client/cmdVM/vmList.go | 5 ++++-
src/client/cmdVM/vmListSingle.go | 8 +++++---
src/server/vms/vms.go | 28 +++++++++++++++++++++-------
tests/run_tests | 18 ++++++++++++++----
6 files changed, 53 insertions(+), 18 deletions(-)
diff --git a/src/client/cmdVM/vmCreate.go b/src/client/cmdVM/vmCreate.go
index 57f8994..4a4763c 100644
--- a/src/client/cmdVM/vmCreate.go
+++ b/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())
diff --git a/src/client/cmdVM/vmEdit.go b/src/client/cmdVM/vmEdit.go
index eced17e..4650648 100644
--- a/src/client/cmdVM/vmEdit.go
+++ b/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)
+ }
}
}
diff --git a/src/client/cmdVM/vmList.go b/src/client/cmdVM/vmList.go
index a2fa9de..c1f1d79 100644
--- a/src/client/cmdVM/vmList.go
+++ b/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
}
diff --git a/src/client/cmdVM/vmListSingle.go b/src/client/cmdVM/vmListSingle.go
index 8d06c5d..983769e 100644
--- a/src/client/cmdVM/vmListSingle.go
+++ b/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
}
diff --git a/src/server/vms/vms.go b/src/server/vms/vms.go
index ffa120d..657a22e 100644
--- a/src/server/vms/vms.go
+++ b/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
}
diff --git a/tests/run_tests b/tests/run_tests
index 35ec464..da41f16 100755
--- a/tests/run_tests
+++ b/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
--
GitLab