harikrishna-patnala commented on a change in pull request #5008:
URL: https://github.com/apache/cloudstack/pull/5008#discussion_r699004049
##########
File path:
server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java
##########
@@ -1655,19 +1660,6 @@ protected boolean hostCanAccessSPool(Host host,
StoragePool pool) {
}
} else {
useLocalStorage = diskOffering.isUseLocalStorage();
-
- // TODO: this is a hacking fix for the problem of deploy
- // ISO-based VM on local storage
- // when deploying VM based on ISO, we have a service offering
- // and an additional disk offering, use-local storage flag is
- // actually
- // saved in service offering, override the flag from service
- // offering when it is a ROOT disk
- if (!useLocalStorage &&
vmProfile.getServiceOffering().isUseLocalStorage()) {
Review comment:
Since Compute and disk offerings are decoupled, the uselocalstorage flag
is persisted/accessed in disk offering only.
Previously uselocalstorage flag is considered in the following way.
1. If useLocalStorage Flag of the disk offering is true, then use local
storage only.
2. If useLocalStorage Flag of the disk offering is false, then check
useLocalStorage Flag in service offering.
To my understanding this is not correct, if useLocalStorage Flag of the disk
offering is true, then useLocalStorage Flag in service offering is ignored
completely. The preference is neither given to disk offering nor compute
offering.
The useLocalStorage flag is supposed to be a property of disk offering, so
we have to consider only the value defined in the volume's disk offering.
Now, also we are introducing a way to override the disk offering for root
volume, so volume has the proper source of truth.
This will not effect the existing VMs since volume is already created.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
