Skip to content

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

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.

Sign up for GitHub

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

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

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/plugins/mer/mervirtualmachinesettingswidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy Markdown
Member

@martyone martyone Aug 12, 2021

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
Copy Markdown
Contributor Author

@etaishev etaishev Aug 12, 2021 •
edited
Loading

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
Copy Markdown
Member

@martyone martyone Aug 12, 2021

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
Copy Markdown
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.

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
Copy Markdown
Member

@martyone martyone Aug 12, 2021

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!

if (features & (VirtualMachine::GrowStorageSize | VirtualMachine::ShrinkStorageSize))
setToolTip(ui->storageSizeInfoLabel, stopVmText);
else
Expand Down