Fixed minor mistake in README.md

Reworked VMs code a bit to have more conistent behavior among methods

README.md

@@ -375,8 +375,8 @@ Legend for the "Done" column:
 
 | Done | Route             | Description       | Method | Parameters             | Req. user cap.                |
 |---   |---                |---                |---     |---                     |---                            |
-|  x   | `/templates/vm`   | create a template | POST   | vmID,name,descr,access | `TPL_CREATE`                  |
-|  x   | `/templates/qcow` | create a template | POST   | qcow,name,descr,access | `TPL_CREATE`                  |
+|  x   | `/templates/vm`   | create a template | POST   | vmID,name,access       | `TPL_CREATE`                  |
+|  x   | `/templates/qcow` | create a template | POST   | qcow,name,access       | `TPL_CREATE`                  |
 |  x   | `/templates/{id}` | delete a template | DELETE |                        | `TPL_DESTROY_ANY|TPL_DESTROY` |
 |  x   | `/templates`      | list templates    | GET    |                        | `TPL_LIST_ANY|TPL_LIST`       |
 

src/router/routerTemplates.go

@@ -185,7 +185,7 @@ func (r *RouterTemplates)CreateTemplateFromQCOW(c echo.Context) error {
 // CAP_TPL_DESTROY_ANY: any template can be deleted.
 // CAP_TPL_DESTROY:     only a template owned by the user can be deleted.
 // Remark: a template can only be deleted if no VM references it!
-// curl --cacert ca.pem -X DELETE https://localhost:1077/templates/Xubuntu_22.04_+_gcc_+_vscodium -H "Authorization: Bearer <AccessToken>"
+// curl --cacert ca.pem -X DELETE https://localhost:1077/templates/4913a2bb-edfe-4dfe-af53-38197a44523b -H "Authorization: Bearer <AccessToken>"
 func (r *RouterTemplates)DeleteTemplateByID(c echo.Context) error {
 	// Retrieves logged user from context.
 	user, err := getLoggedUser(r.users, c)

src/router/routerVMs.go

@@ -359,75 +359,6 @@ func (r *RouterVMs)DeleteVMAccessForUser(c echo.Context) error {
 	return c.JSONPretty(http.StatusOK, jsonMsg("OK"), "    ")
 }
 
-// Helper function that returns a list of VMs that match either:
-// - the logged user has the userCapabilityAny capability.
-// - the VM access for the logged user matches the vmAccessCapability capability.
-// Also, VMs for which cond is false are filtered out.
-func (r *RouterVMs)performVMsList(c echo.Context, userCapabilityAny, vmAccessCapability string, cond vms.VMKeeperFn) error {
-	// Retrieves logged user from context.
-	user, err := getLoggedUser(r.users, c)
-	if err != nil {
-		return echo.NewHTTPError(http.StatusUnauthorized, err.Error())
-	}
-
-	// If the logged user has the XX_ANY capability (userCapabilityAny), returns all VMs.
-	if user.HasCapability(userCapabilityAny) {
-		// Returns all VMs that pass the condition.
-		return c.JSONPretty(http.StatusOK, r.vms.GetVMs(cond), "    ")
-	} else {
-		// Returns all VMs for which VM access for the logged user matches the specified the VM Access capability (vmAccessCapability).
-		return c.JSONPretty(http.StatusOK, r.vms.GetVMs(
-			func(vm vms.VM) bool {
-				capabilities, exists := vm.Access[user.Email]
-				if exists {
-					_, visible := capabilities[vmAccessCapability]
-					return visible && cond(vm)
-				} else {
-					return false
-				}
-			  }), "    ")
-	}
-}
-
-// Helper function that performs an action on a VM based either on:
-// - the logged user has the userCapabilityAny capability.
-// - the VM access for the logged user matches the vmAccessCapability capability.
-func (r *RouterVMs)performVMAction(c echo.Context, userCapabilityAny, vmAccessCapability string, action vmActionFn) error {
-	// Retrieves the VM on which to perform the action.
-	id, err := uuid.Parse(c.Param("id"))
-	if err != nil {
-		return echo.NewHTTPError(http.StatusBadRequest, err.Error())
-	}
-	vm, err := r.vms.GetVM(id)
-	if err != nil {
-		return echo.NewHTTPError(http.StatusNotFound, err.Error())
-	}
-
-	// Retrieves the logged user from context.
-	user, err := getLoggedUser(r.users, c)
-	if err != nil {
-		return echo.NewHTTPError(http.StatusUnauthorized, err.Error())
-	}
-
-	// First, checks if the user has the XX_ANY capability
-	if user.HasCapability(userCapabilityAny) {
-		return action(c, &vm)
-	} else {
-		// Check if the VM access for the logged user matches the required capability
-		userCaps, exists := vm.Access[user.Email]
-		if !exists {
-			return echo.NewHTTPError(http.StatusUnauthorized, msgInsufficientCaps)
-		}
-
-		_, hasAccess := userCaps[vmAccessCapability]
-		if !hasAccess {
-			return echo.NewHTTPError(http.StatusUnauthorized, msgInsufficientCaps)
-		}
-
-		return action(c, &vm)
-	}
-}
-
 // Exports a VM's directory into a .tar archive.
 // Requires either capability:
 // User cap: VM_READFS_ANY: any VM can have their filesystem read.
@@ -512,3 +443,72 @@ func (r *RouterVMs)ImportFilesToVM(c echo.Context) error {
 		return c.JSONPretty(http.StatusCreated, jsonMsg("OK"), "    ")
 	})
 }
+
+// Helper function that returns a list of VMs that match either:
+// - the logged user has the userCapabilityAny capability.
+// - the VM access for the logged user matches the vmAccessCapability capability.
+// Also, VMs for which cond is false are filtered out.
+func (r *RouterVMs)performVMsList(c echo.Context, userCapabilityAny, vmAccessCapability string, cond vms.VMKeeperFn) error {
+	// Retrieves logged user from context.
+	user, err := getLoggedUser(r.users, c)
+	if err != nil {
+		return echo.NewHTTPError(http.StatusUnauthorized, err.Error())
+	}
+
+	// If the logged user has the XX_ANY capability (userCapabilityAny), returns all VMs.
+	if user.HasCapability(userCapabilityAny) {
+		// Returns all VMs that pass the condition.
+		return c.JSONPretty(http.StatusOK, r.vms.GetVMs(cond), "    ")
+	} else {
+		// Returns all VMs for which VM access for the logged user matches the specified the VM Access capability (vmAccessCapability).
+		return c.JSONPretty(http.StatusOK, r.vms.GetVMs(
+			func(vm vms.VM) bool {
+				capabilities, exists := vm.Access[user.Email]
+				if exists {
+					_, visible := capabilities[vmAccessCapability]
+					return visible && cond(vm)
+				} else {
+					return false
+				}
+			  }), "    ")
+	}
+}
+
+// Helper function that performs an action on a VM based either on:
+// - the logged user has the userCapabilityAny capability.
+// - the VM access for the logged user matches the vmAccessCapability capability.
+func (r *RouterVMs)performVMAction(c echo.Context, userCapabilityAny, vmAccessCapability string, action vmActionFn) error {
+	// Retrieves the VM on which to perform the action.
+	id, err := uuid.Parse(c.Param("id"))
+	if err != nil {
+		return echo.NewHTTPError(http.StatusBadRequest, err.Error())
+	}
+	vm, err := r.vms.GetVM(id)
+	if err != nil {
+		return echo.NewHTTPError(http.StatusNotFound, err.Error())
+	}
+
+	// Retrieves the logged user from context.
+	user, err := getLoggedUser(r.users, c)
+	if err != nil {
+		return echo.NewHTTPError(http.StatusUnauthorized, err.Error())
+	}
+
+	// First, checks if the user has the XX_ANY capability
+	if user.HasCapability(userCapabilityAny) {
+		return action(c, &vm)
+	} else {
+		// Check if the VM access for the logged user matches the required capability
+		userCaps, exists := vm.Access[user.Email]
+		if !exists {
+			return echo.NewHTTPError(http.StatusUnauthorized, msgInsufficientCaps)
+		}
+
+		_, hasAccess := userCaps[vmAccessCapability]
+		if !hasAccess {
+			return echo.NewHTTPError(http.StatusUnauthorized, msgInsufficientCaps)
+		}
+
+		return action(c, &vm)
+	}
+}

src/vms/vms.go

@@ -94,11 +94,11 @@ func InitVMs() error {
 }
 
 // Returns the list of VMs for which VMKeeperFn returns true.
-func (vms *VMs)GetVMs(keep VMKeeperFn) []VM {
+func (vms *VMs)GetVMs(keepFn VMKeeperFn) []VM {
 	vms.rwlock.RLock()
 	list := []VM{}
 	for _, vm := range vms.m {
-		if keep(vm) {
+		if keepFn(vm) {
 			list = append(list, vm)
 		}
 	}
@@ -115,6 +115,10 @@ func (vms *VMs)GetVMs(keep VMKeeperFn) []VM {
 func (vms *VMs)GetVM(vmID uuid.UUID) (VM, error) {
 	vms.rwlock.RLock()
 	defer vms.rwlock.RUnlock()
+	return vms.getVMUnsafe(vmID)
+}
+
+func (vms *VMs)getVMUnsafe(vmID uuid.UUID) (VM, error) {
 	vm, exists := vms.m[vmID.String()]
 	if !exists {
 		return dummyVM, errors.New("VM not found")
@@ -123,12 +127,15 @@ func (vms *VMs)GetVM(vmID uuid.UUID) (VM, error) {
 }
 
 // Deletes a VM by its ID and deletes its files.
-func (vms *VMs)DeleteVM(id uuid.UUID) error {
-	key := id.String()
+func (vms *VMs)DeleteVM(vmID uuid.UUID) error {
 	vms.rwlock.Lock()
 	defer vms.rwlock.Unlock()
-	vm, exists := vms.m[key]
-	if exists {
+
+	vm, err := vms.getVMUnsafe(vmID)
+	if err != nil {
+		return err
+	}
+
 	// Deletes the VM's files (and directories).
 	vm.mutex.Lock()
 	if err := vm.delete(); err != nil {
@@ -137,11 +144,9 @@ func (vms *VMs)DeleteVM(id uuid.UUID) error {
 	}
 	vm.mutex.Unlock()
 	// Removes the VM from the map.
-		delete(vms.m, key)
+	delete(vms.m, vm.ID.String())
 	return nil
 }
-	return errors.New("VM not found")
-}
 
 // Adds a VM and writes its files.
 func (vms *VMs)AddVM(vm *VM) error {
@@ -165,13 +170,13 @@ func (vms *VMs)AddVM(vm *VM) error {
 
 // Starts a VM by its ID.
 // Returns the port on which the VM is running and the access password.
-func (vms *VMs)StartVM(id uuid.UUID) (int, string, error) {
+func (vms *VMs)StartVM(vmID uuid.UUID) (int, string, error) {
 	vms.rwlock.Lock()
 	defer vms.rwlock.Unlock()
 
-	vm, exists := vms.m[id.String()]
-	if !exists {
-		return -1, "", errors.New("VM not found")
+	vm, err := vms.getVMUnsafe(vmID)
+	if err != nil {
+		return 0, "", err
 	}
 
 	// Check there will be at least 1GB of RAM left after the VM has started,
@@ -215,33 +220,33 @@ func (vms *VMs)StartVM(id uuid.UUID) (int, string, error) {
 }
 
 // Stops by force a VM by its ID.
-func (vms *VMs)StopVM(id uuid.UUID) error {
+func (vms *VMs)StopVM(vmID uuid.UUID) error {
 	vms.rwlock.RLock()
 	defer vms.rwlock.RUnlock()
 
-	vm, exists := vms.m[id.String()]
-	if !exists {
-		return errors.New("VM not found")
+	vm, err := vms.getVMUnsafe(vmID)
+	if err != nil {
+		return err
 	}
 
 	vm.mutex.Lock()
-	err := vm.stop()
+	err = vm.stop()
 	vm.mutex.Unlock()
 	return err
 }
 
 // Gracefully stops a VM by its ID.
-func (vms *VMs)ShutdownVM(id uuid.UUID) error {
+func (vms *VMs)ShutdownVM(vmID uuid.UUID) error {
 	vms.rwlock.Lock()
 	defer vms.rwlock.Unlock()
 
-	vm, exists := vms.m[id.String()]
-	if !exists {
-		return errors.New("VM not found")
+	vm, err := vms.getVMUnsafe(vmID)
+	if err != nil {
+		return err
 	}
 
 	vm.mutex.Lock()
-	err := vm.shutdown()
+	err = vm.shutdown()
 	vm.mutex.Unlock()
 	return err
 }
@@ -261,10 +266,9 @@ func (vms *VMs)IsTemplateUsed(templateID string) bool {
 
 // Edit a VM' specs: name, cpus, ram, nic
 func (vms *VMs)EditVM(vmID uuid.UUID, name string, cpus, ram int, nic NicType) error {
-	// Retrieves the VM to be edited.
-	vm, err := vms.GetVM(vmID)
+	vm, err := vms.getVMUnsafe(vmID)
 	if err != nil {
-		return errors.New("VM not found")
+		return err
 	}
 
 	// Only updates fields that have changed.
@@ -305,12 +309,18 @@ func (vms *VMs)SetVMAccess(vmID uuid.UUID, loggedUserEmail, userEmail string, ne
 		return errors.New("Invalid capability")
 	}
 
+	vms.rwlock.Lock()
+	defer vms.rwlock.Unlock()
+
 	// Retrieves the VM for which the access caps must be changed.
-	vm, err := vms.GetVM(vmID)
+	vm, err := vms.getVMUnsafe(vmID)
 	if err != nil {
-		return errors.New("VM not found")
+		return err
 	}
 
+	vm.mutex.Lock()
+	defer vm.mutex.Unlock()
+
 	// Checks the logged user has VM_SET_ACCESS set in her/his VM access.
 	userCaps := vm.Access[loggedUserEmail]
 	_, exists := userCaps[caps.CAP_VM_SET_ACCESS]
@@ -320,11 +330,6 @@ func (vms *VMs)SetVMAccess(vmID uuid.UUID, loggedUserEmail, userEmail string, ne
 
 	vm.Access[userEmail] = newAccess
 
-	vm.mutex.Lock()
-	defer vm.mutex.Unlock()
-	vms.rwlock.Lock()
-	defer vms.rwlock.Unlock()
-
 	if err = vms.updateVM(&vm); err != nil {
 		return errors.New("Failed updating VM")
 	}
@@ -336,12 +341,18 @@ func (vms *VMs)SetVMAccess(vmID uuid.UUID, loggedUserEmail, userEmail string, ne
 // loggedUserEmail is the email of the currently logged user
 // userMail is the email of the user for which to remove the access
 func (vms *VMs)DeleteVMAccess(vmID uuid.UUID, loggedUserEmail, userEmail string) error {
+	vms.rwlock.Lock()
+	defer vms.rwlock.Unlock()
+
 	// Retrieves the VM for which the access caps must be changed.
-	vm, err := vms.GetVM(vmID)
+	vm, err := vms.getVMUnsafe(vmID)
 	if err != nil {
-		return errors.New("VM not found")
+		return err
 	}
 
+	vm.mutex.Lock()
+	defer vm.mutex.Unlock()
+
 	// Checks the user has VM_SET_ACCESS set in her/his VM access.
 	userCaps := vm.Access[loggedUserEmail]
 	_, exists := userCaps[caps.CAP_VM_SET_ACCESS]
@@ -352,11 +363,6 @@ func (vms *VMs)DeleteVMAccess(vmID uuid.UUID, loggedUserEmail, userEmail string)
 	// Removes the user from the Access map
 	delete(vm.Access, userEmail)
 
-	vm.mutex.Lock()
-	defer vm.mutex.Unlock()
-	vms.rwlock.Lock()
-	defer vms.rwlock.Unlock()
-
 	if err = vms.updateVM(&vm); err != nil {
 		return errors.New("Failed updating VM")
 	}