Skip to content

Conversation

@zabdulre
Copy link

@zabdulre zabdulre commented Jul 2, 2025

Issues

  1. VmListModel.onNewTemplate() doesn't validate the VM model prior to creating template
  2. VmModelBehaviorBase has concurrency issues updating memory or CPU

This results in incorrect vCPU values and potentially incorrect memory values when creating a template from a VM or VM snapshot. This does not occur if the VM has its operating system set to "Other OS".

Changes introduced with this PR

  1. Make VmListModel.onNewTemplate() run model.validate(), like what VmSnapshotListModel.onCloneTemplate() does. So that "Create Template" could fail fast if VM model is invalid, instead of creating a template with wrong memory/CPU settings.
  2. Fix updateMemory() and updateTotalCpuCores() in VmModelBehaviorBase. The memory/CPU is updated in an async fashion, and the values are read before the async call. By the time comparison is done in the callback, the values become outdated, resulting in wrong results. Changed to read values inside the callback.

Manual tests performed

  1. Created a template from the compute/virtual-machines page and ensured custom memory size, maximum memory, physical memory and vCPU values were all correct
  2. Created a template from a snapshot and ensured custom memory size, maximum memory, physical memory and vCPU values were all correct

Are you the owner of the code you are sending in, or do you have permission of the owner?

yes

@zabdulre zabdulre requested a review from sgratch as a code owner July 2, 2025 17:56
@zabdulre zabdulre force-pushed the template_settings_fix branch 2 times, most recently from 7891a13 to ad61bf1 Compare July 2, 2025 18:38
@zabdulre
Copy link
Author

zabdulre commented Jul 2, 2025

Updated commit message to be more detailed and include author signature "signed-off-by"

@JasperB-TeamBlue
Copy link
Contributor

Please structure your commit message following the guidelines provided in CONTRIBUTING.adoc

@zabdulre zabdulre changed the title Fix template creation resource issues webadmin: fix template creation resource issues Jul 8, 2025
@zabdulre zabdulre force-pushed the template_settings_fix branch from ad61bf1 to ee01f4d Compare July 8, 2025 17:24
@zabdulre
Copy link
Author

zabdulre commented Jul 8, 2025

Updated commit message to follow the guidelines provided in CONTRIBUTING.adoc:

  1. added correct title prefix "webadmin" with short prefix
  2. kept long description but shortened lines to be under 72 chars

Fixes an issue where creating a template from a VM or VM snapshot
can result in incorrect vCPU values and
potentially incorrect memory values in the template.

With this change, VmListModel.onNewTemplate() now runs model.validate()
so that "Create Template" could fail fast if the VM model is invalid,
instead of creating a template with wrong memory/CPU settings.

Furthermore, getTotalCpuCores() is now called inside a callback function
in order to avoid a race condition.

Signed-off-by: Guoyong Zhang <[email protected]>
@zabdulre zabdulre force-pushed the template_settings_fix branch from ee01f4d to 86be6e5 Compare July 8, 2025 17:31
@zabdulre
Copy link
Author

Can this please be reviewed and considered for merge? Thank you

@JasperB-TeamBlue
Copy link
Contributor

You suggest that this issue only surfaces when the VM is set to something else then "Other OS". Why is this?
Also, what do you mean exactly with incorrect vCPU and memory, could you please clarify this?

@zabdulre
Copy link
Author

When the VM is set to "Other OS", we found that there are less calls to the faulty VmModelBehaviorBase.updateMemory() and updateTotalCpuCores() functions.

There are less UnitVmModel.oSType_SelectedItemChanged invocations if the VM is set to "Other OS". oSType_SelectedItemChanged calls VmModelBehaviorBase.updateMemory() and updateTotalCpuCores(). Thus there are less calls to VmModelBehaviorBase.updateMemory() and updateTotalCpuCores(). As a result, the bug doesn't or almost never occurs when the VM is set to "Other OS".

By incorrect vCPU and memory, we mean that the user specified settings for "Total Virtual CPUs" and "Memory Size" are set to some value other than what the user specifies when creating the template. In some test runs, for example, I set the vCPU value to 3 but after creating the template, the vCPU value was 1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants