Skip to content

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

Open
etaishev wants to merge 1 commit intosailfishos:masterfrom
etaishev:jb55104
Open

Mer: Do not update vm storage limits before value updated. JB#55104#447
etaishev wants to merge 1 commit intosailfishos:masterfrom
etaishev:jb55104

Conversation

@etaishev
Copy link
Copy Markdown
Contributor

No description provided.

else
setToolTip(ui->cpuInfoLabel, tr("Virtual Machine does not allow changing cpu count"));

setStorageSizeLimits();
Copy link
Copy Markdown
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
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.

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

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