-
Notifications
You must be signed in to change notification settings - Fork 25
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
base: master
Are you sure you want to change the base?
Conversation
@@ -73,7 +73,6 @@ void MerVirtualMachineSettingsWidget::setVmFeatures(VirtualMachine::Features fea | |||
else | |||
setToolTip(ui->cpuInfoLabel, tr("Virtual Machine does not allow changing cpu count")); | |||
|
|||
setStorageSizeLimits(); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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!
No description provided.