Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mer: Do not update vm storage limits before value updated. JB#55104 #447

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

etaishev
Copy link
Contributor

No description provided.

@@ -73,7 +73,6 @@ void MerVirtualMachineSettingsWidget::setVmFeatures(VirtualMachine::Features fea
else
setToolTip(ui->cpuInfoLabel, tr("Virtual Machine does not allow changing cpu count"));

setStorageSizeLimits();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am afraid this is definitely not a valid fix for the issue you observe. setStorageSizeLimits must be called here.

Copy link
Contributor Author

@etaishev etaishev Aug 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What for? Nowhere above is the current disk size set and only the tooltips are set

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sets the limits for the current disk size (as it is known to the SDK) at the time the widget is initialized and every time the disk size is changed using the widget. Check the code. It is not only done to restrict it to growing/shrinking depending on the available VM features, but also to restrict the maximum increment in case of growing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The setStorageSizeLimits(); calls twice: from setVmFeatures() and setStorageSizeMb(). When setStorageSizeLimits is called from setVmFeatures the disk value is not set and it is set as 0+MAX_STORAGE_SIZE_INCREMENT_GB. At the moment when setStorageSizeMb is called there is already a limit, which is based on wrong data

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because the order of calling setVmFeatures and setStorageSizeMb inside MerEmulatorDetailsWidget::setEmulator and MerBuildEngineDetailsWidget::setBuildEngine is wrong - this is what should be fixed instead. Be sure to test very well the effect of correcting the order as there may be conflicting requirements!

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.

2 participants