Bug 1069269
| Summary: | Allocation Policy changed by engine to its default from user defined after changing to another storage domain | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [oVirt] ovirt-engine | Reporter: | Nikolai Sednev <nsednev> | ||||||||||
| Component: | Frontend.WebAdmin | Assignee: | Eyal Shenitzky <eshenitz> | ||||||||||
| Status: | CLOSED CURRENTRELEASE | QA Contact: | Shir Fishbain <sfishbai> | ||||||||||
| Severity: | medium | Docs Contact: | |||||||||||
| Priority: | unspecified | ||||||||||||
| Version: | --- | CC: | acanan, bugs, derez, ebenahar, eshenitz, istein, jhunsaker, mchappel, mgoldboi, nsednev, ratamir, scohen, s.kieske, srevivo, tnisan | ||||||||||
| Target Milestone: | ovirt-4.3.0 | Flags: | scohen:
needinfo+
rule-engine: ovirt-4.3+ |
||||||||||
| Target Release: | 4.3.0 | ||||||||||||
| Hardware: | x86_64 | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Whiteboard: | |||||||||||||
| Fixed In Version: | ovirt-engine-4.3.0_alpha | Doc Type: | Bug Fix | ||||||||||
| Doc Text: | Story Points: | --- | |||||||||||
| Clone Of: | Environment: | ||||||||||||
| Last Closed: | 2019-02-13 07:46:41 UTC | Type: | Bug | ||||||||||
| Regression: | --- | Mount Type: | --- | ||||||||||
| Documentation: | --- | CRM: | |||||||||||
| Verified Versions: | Category: | --- | |||||||||||
| oVirt Team: | Storage | RHEL 7.3 requirements from Atomic Host: | |||||||||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||||||||
| Embargoed: | |||||||||||||
| Bug Depends On: | 1093025 | ||||||||||||
| Bug Blocks: | 1161662 | ||||||||||||
| Attachments: |
|
||||||||||||
How can this be tagged as an RFE as it is clearly breaking expected behaviour of an existing feature? Does this just happen for iSCSI or also for other Storage Types? Allon - can you review this one? This is an automated message. Re-targeting all non-blocker bugs still open on 3.4.0 to 3.4.1. Sven - agreed. This is a bug, not an RFE. This problem seems to have existed for several versions now, but it should indeed be handled. Daniel, can you please take a look? According to the current design, thin allocation is selected by default for file domains. Whereas, for block domains, preallocated allocation is selected by default. The desired behavior could, indeed, be something along the lines of: changing allocation policy, according to above described default, only if the user hasn't already selected a policy. afaik, we don't use any similar mechanism in the UI (i.e. there's no differentiation between user selection and programmatic selection). Einav - do you think we should introduce such a solution to the infrastructure? (In reply to Daniel Erez from comment #5) > ... > afaik, we don't use any similar mechanism in the UI (i.e. there's no > differentiation between user selection and programmatic selection). AFAIK, you are right - we are not differentiating between the two. > Einav - do you think we should introduce such a solution to the > infrastructure? we probably should; I am wondering if instead and/or as an interim solution for this situation (and perhaps similar situations in other places in the GUI), we should add an "Auto" option to the "Allocation Policy" drop-down. So by default, the selected "Allocation Policy" will be "Auto". So in case the selected storage domain is a file domain, the value will be "Auto [thin allocation]" and if the selected storage domain is changed to a block domain, the Allocation Policy will change to "Auto [preallocated]". I.e. if "Auto" is selected, the value will be determined by the programmatic selection. And if the user explicitly selected "preallocated" or "thin allocation", the programmatic selection will be "disabled" [*] That way, programmatic selection is not colliding with user selection. thoughts? [*] The user will always be able to return to "Auto", if he wants to "re-enable" the programmatic selection; we can maybe put a note somewhere in the dialog mentioning that the "Auto" option will choose the optimal allocation policy based on the storage domain storage type - not sure if necessary (In reply to Einav Cohen from comment #6) > (In reply to Daniel Erez from comment #5) > > ... > > afaik, we don't use any similar mechanism in the UI (i.e. there's no > > differentiation between user selection and programmatic selection). > > AFAIK, you are right - we are not differentiating between the two. > > > Einav - do you think we should introduce such a solution to the > > infrastructure? > > we probably should; > > I am wondering if instead and/or as an interim solution for this situation > (and perhaps similar situations in other places in the GUI), we should add > an "Auto" option to the "Allocation Policy" drop-down. > > So by default, the selected "Allocation Policy" will be "Auto". > So in case the selected storage domain is a file domain, the value will be > "Auto [thin allocation]" and if the selected storage domain is changed to a > block domain, the Allocation Policy will change to "Auto [preallocated]". > > I.e. if "Auto" is selected, the value will be determined by the programmatic > selection. How will the user know if "Auto" means thin or preallocated? > > And if the user explicitly selected "preallocated" or "thin allocation", the > programmatic selection will be "disabled" [*] So, iiuc, we'll still need an infrastructure to determine whether the selection was initiated by the user? > > That way, programmatic selection is not colliding with user selection. > > thoughts? Do we have any similar behavior in the UI (sounds like it's conflicting our current conventions..) ? > > [*] The user will always be able to return to "Auto", if he wants to > "re-enable" the programmatic selection; we can maybe put a note somewhere in > the dialog mentioning that the "Auto" option will choose the optimal > allocation policy based on the storage domain storage type - not sure if > necessary Created attachment 874084 [details]
screen-shot: Console Options dialog
Created attachment 874085 [details]
mock-up: Allocation Policy - Auto
(In reply to Daniel Erez from comment #7) > How will the user know if "Auto" means thin or preallocated? we can write it within the drop-down item, i.e. "Auto [Preallocated]" or "Auto [Thin Provision]". see attachment 874085 [details] for mock-ups - I hope it will clarify. > So, iiuc, we'll still need an infrastructure to determine whether the > selection > was initiated by the user? what you really need to determine is whether the 'Allocation Policy' value can be changed programmatically (e.g. based on storage domain selection change) or not. The answer is: If "Auto [something]" is selected: yes - you should change the value from "Auto [Preallocated]" to "Auto [Thin Provision]" (and vice-versa) programmatically, based on the changes of the data-center/storage-domain selected values. So as long as we are in the Auto "context", the 'Allocation Policy' value should change programmatically within that "context". If 'Auto' is not selected [i.e. if the selected value is "Preallocated" (and not "Auto [Preallocated]") or "Thin Provision" (and not "Auto [Thin Provision]")], it means that the user explicitly chose the selected value, so you should NOT change this value programmatically (i.e. the user took us out of the Auto "Context"; from that point on, we are in a User-Selection "context"). > Do we have any similar behavior in the UI (sounds like it's conflicting our > current conventions..) ? we have something somewhat similar in Console Options (see attachment 874084 [details]) - the main difference is that AFAIK, the 'Auto' is a real option (i.e. this is what is saved in the DB) - the real value is determined only upon connecting to the VM's console, and it depends on multiple things, such as browser/OS from which we are connecting to the console, etc. (i.e. things that cannot be determined while we are in the context of the 'Console Options' dialog). [In our case, we can display (and save to the DB) the actual value of 'Auto', since it is determined only by parameters within the 'Add Virtual Disk' dialog context] but from a user experience point of view, the general idea is the same: by default, the 'Auto' option is selected, and the user can override it with a specific option. (In reply to Einav Cohen from comment #10) > (In reply to Daniel Erez from comment #7) > > How will the user know if "Auto" means thin or preallocated? > > we can write it within the drop-down item, i.e. "Auto [Preallocated]" or > "Auto [Thin Provision]". > see attachment 874085 [details] for mock-ups - I hope it will clarify. > > > So, iiuc, we'll still need an infrastructure to determine whether the > > selection > > was initiated by the user? > > what you really need to determine is whether the 'Allocation Policy' value > can be changed programmatically (e.g. based on storage domain selection > change) or not. > > The answer is: > > If "Auto [something]" is selected: yes - you should change the value from > "Auto [Preallocated]" to "Auto [Thin Provision]" (and vice-versa) > programmatically, based on the changes of the data-center/storage-domain > selected values. So as long as we are in the Auto "context", the 'Allocation > Policy' value should change programmatically within that "context". > > If 'Auto' is not selected [i.e. if the selected value is "Preallocated" (and > not "Auto [Preallocated]") or "Thin Provision" (and not "Auto [Thin > Provision]")], it means that the user explicitly chose the selected value, > so you should NOT change this value programmatically (i.e. the user took us > out of the Auto "Context"; from that point on, we are in a User-Selection > "context"). > > > Do we have any similar behavior in the UI (sounds like it's conflicting our > > current conventions..) ? > > we have something somewhat similar in Console Options (see attachment 874084 [details] > [details]) - the main difference is that AFAIK, the 'Auto' is a real option > (i.e. this is what is saved in the DB) - the real value is determined only > upon connecting to the VM's console, and it depends on multiple things, such > as browser/OS from which we are connecting to the console, etc. (i.e. things > that cannot be determined while we are in the context of the 'Console > Options' dialog). > > [In our case, we can display (and save to the DB) the actual value of > 'Auto', since it is determined only by parameters within the 'Add Virtual > Disk' dialog context] > > but from a user experience point of view, the general idea is the same: by > default, the 'Auto' option is selected, and the user can override it with a > specific option. Thanks for thorough and detailed feedback Einav! I clearly understand the solution now. However, I still don't sure it won't be a confusing behavior to the user. As you've mentioned, the only place that we have something similar is when "Auto" is a real value. So, from the user point of view, it seems like a third option should be somehow different from the other two. Personally, I think I would still prefer a generic solution here rather than introducing such a unique behavior for a specific scenario (as we most probably have similar issues in other dialogs). @Sean - any preference? do you find the suggested solution clear enough? perhaps disabling default allocation policy altogether is the way to go here? @Einav - in any case, should I open an RFE for introducing a generic solution into the UI infrastructure? (In reply to Daniel Erez from comment #11) > ... > @Einav - in any case, should I open an RFE for introducing a generic > solution into the UI infrastructure? sure, sounds good. thanks. (In reply to Daniel Erez from comment #11) > (In reply to Einav Cohen from comment #10) > > (In reply to Daniel Erez from comment #7) > > > How will the user know if "Auto" means thin or preallocated? > > > > we can write it within the drop-down item, i.e. "Auto [Preallocated]" or > > "Auto [Thin Provision]". > > see attachment 874085 [details] for mock-ups - I hope it will clarify. > > > > > So, iiuc, we'll still need an infrastructure to determine whether the > > > selection > > > was initiated by the user? > > > > what you really need to determine is whether the 'Allocation Policy' value > > can be changed programmatically (e.g. based on storage domain selection > > change) or not. > > > > The answer is: > > > > If "Auto [something]" is selected: yes - you should change the value from > > "Auto [Preallocated]" to "Auto [Thin Provision]" (and vice-versa) > > programmatically, based on the changes of the data-center/storage-domain > > selected values. So as long as we are in the Auto "context", the 'Allocation > > Policy' value should change programmatically within that "context". > > > > If 'Auto' is not selected [i.e. if the selected value is "Preallocated" (and > > not "Auto [Preallocated]") or "Thin Provision" (and not "Auto [Thin > > Provision]")], it means that the user explicitly chose the selected value, > > so you should NOT change this value programmatically (i.e. the user took us > > out of the Auto "Context"; from that point on, we are in a User-Selection > > "context"). > > > > > Do we have any similar behavior in the UI (sounds like it's conflicting our > > > current conventions..) ? > > > > we have something somewhat similar in Console Options (see attachment 874084 [details] > > [details]) - the main difference is that AFAIK, the 'Auto' is a real option > > (i.e. this is what is saved in the DB) - the real value is determined only > > upon connecting to the VM's console, and it depends on multiple things, such > > as browser/OS from which we are connecting to the console, etc. (i.e. things > > that cannot be determined while we are in the context of the 'Console > > Options' dialog). > > > > [In our case, we can display (and save to the DB) the actual value of > > 'Auto', since it is determined only by parameters within the 'Add Virtual > > Disk' dialog context] > > > > but from a user experience point of view, the general idea is the same: by > > default, the 'Auto' option is selected, and the user can override it with a > > specific option. > > Thanks for thorough and detailed feedback Einav! I clearly understand the > solution now. However, I still don't sure it won't be a confusing behavior > to the user. As you've mentioned, the only place that we have something > similar is when "Auto" is a real value. So, from the user point of view, it > seems like a third option should be somehow different from the other two. > Personally, I think I would still prefer a generic solution here rather than > introducing such a unique behavior for a specific scenario (as we most > probably have similar issues in other dialogs). > > @Sean - any preference? do you find the suggested solution clear enough? > perhaps disabling default allocation policy altogether is the way to go here? I totally agree with Daniel, that the 'Auto' concept will be a confusing behavior to the user. For 3.5 let's keep it strait forward (change allocation policy according to the described default, only if the user hasn't already selected a policy) and leave the more Generic solution for 4.0 Sean Pending missing infra described in bug 1093025 - pushing out to 3.6.0. This can be closed once we remove allocation policy, right? (In reply to Yaniv Dary from comment #15) > This can be closed once we remove allocation policy, right? Not sure. We're removing the notion of preallocated vs. sparse, but a user should still have a choice about the format (raw vs qcow). Let's use this RFE to track the GUI change of not resetting the choice when switching a DC. *** Bug 1161662 has been marked as a duplicate of this bug. *** This is an automated message. This Bugzilla report has been opened on a version which is not maintained anymore. Please check if this bug is still relevant in oVirt 3.5.4. If it's not relevant anymore, please close it (you may use EOL or CURRENT RELEASE resolution) If it's an RFE please update the version to 4.0 if still relevant. Moving need info to acanan, as nsednev no longer on Storage QE. Happens on 3.6.0-11 as well We need to target it. (In reply to Aharon Canan from comment #20) > Happens on 3.6.0-11 as well > We need to target it. It happens not only on 3.6, it will also happen on 3.1 - we don't have the infra for it in 3.6.0, unfortunately - we're pending on bug 1093025. Target release should be placed once a package build is known to fix a issue. Since this bug is not modified, the target version has been reset. Please use target milestone to plan a fix for a oVirt release. Still relevant when we remove DC? (In reply to Yaniv Kaul from comment #23) > Still relevant when we remove DC? sure. this happens whenever you change the **domain** in the dialog. This is a webadmin issue which depends on UI-infra work that I assume will never happen since we're deprecating the current GUI. What should I do with this bug? Close? Postpone to 4.something to revisit on the new GUI? (In reply to Allon Mureinik from comment #28) > This is a webadmin issue which depends on UI-infra work that I assume will > never happen since we're deprecating the current GUI. > > What should I do with this bug? > Close? Postpone to 4.something to revisit on the new GUI? We will review this in the next scrub. (In reply to Yaniv Dary from comment #29) > (In reply to Allon Mureinik from comment #28) > > This is a webadmin issue which depends on UI-infra work that I assume will > > never happen since we're deprecating the current GUI. > > > > What should I do with this bug? > > Close? Postpone to 4.something to revisit on the new GUI? > > We will review this in the next scrub. Please do. Is this still needed? I'm probably still missing something, but can't this be achieved in the specific place with a simple flag/latch, something like userChangedField = false ... <user changes it> ... userChangedField = true and if (!userChangedField) setField(whatever) (In reply to Greg Sheremeta from comment #31) > Is this still needed? > > I'm probably still missing something, but can't this be achieved in the > specific place with a simple flag/latch, something like > > userChangedField = false > ... <user changes it> ... > userChangedField = true > > and > > if (!userChangedField) > setField(whatever) IIUC, for that we'll still need some means to differentiate between user selection and programmatic. We were waiting for it to be provided in the infra level (bug 1093025). (In reply to Daniel Erez from comment #32) > (In reply to Greg Sheremeta from comment #31) > > Is this still needed? > > > > I'm probably still missing something, but can't this be achieved in the > > specific place with a simple flag/latch, something like > > > > userChangedField = false > > ... <user changes it> ... > > userChangedField = true > > > > and > > > > if (!userChangedField) > > setField(whatever) > > IIUC, for that we'll still need some means to differentiate between user > selection and programmatic. We were waiting for it to be provided in the > infra level (bug 1093025). That's what I'm getting at -- I don't understand why bug 1093025 is needed. Wouldn't onclick / onmousedown / onkeypress etc tell you if a user changed something, and then on those events you can set a flag to guard against other code setting a var? (In reply to Greg Sheremeta from comment #33) > (In reply to Daniel Erez from comment #32) > > (In reply to Greg Sheremeta from comment #31) > > > Is this still needed? > > > > > > I'm probably still missing something, but can't this be achieved in the > > > specific place with a simple flag/latch, something like > > > > > > userChangedField = false > > > ... <user changes it> ... > > > userChangedField = true > > > > > > and > > > > > > if (!userChangedField) > > > setField(whatever) > > > > IIUC, for that we'll still need some means to differentiate between user > > selection and programmatic. We were waiting for it to be provided in the > > infra level (bug 1093025). > > That's what I'm getting at -- I don't understand why bug 1093025 is needed. > Wouldn't onclick / onmousedown / onkeypress etc tell you if a user changed > something, and then on those events you can set a flag to guard against > other code setting a var? Sure, it could be done in a specific location, but it would be nice to have it in the infra for re-usability. (In reply to Daniel Erez from comment #34) > Sure, it could be done in a specific location, but it would be nice to have > it in the infra for re-usability. Do you know how many places need this? That would help us determine whether it's worth special infra work. (In reply to Greg Sheremeta from comment #35) > (In reply to Daniel Erez from comment #34) > > Sure, it could be done in a specific location, but it would be nice to have > > it in the infra for re-usability. > > Do you know how many places need this? That would help us determine whether > it's worth special infra work. Not really sure actually. But I think it would be nice to see it in the infra (in some level) anyway as it might be helpful in the future. I'm afraid it could look quite cumbersome and unnatural if done only in a specific location. Worth rethinking now that we support preallocation for file based storage. (In reply to Allon Mureinik from comment #37) > Worth rethinking now that we support preallocation for file based storage. This is a UI bug which is not related to the support preallocation for file-based storage. This bug is open for more than 4 years now and the infrastructure doesn't implement in the new UI, I think that in this case, a simple solution as mentioned in comment #31 can solve it easily. (In reply to Eyal Shenitzky from comment #39) > This bug is open for more than 4 years now and the infrastructure doesn't > implement in the new UI, > > I think that in this case, a simple solution as mentioned in comment #31 can > solve it easily. I'm not sure it worth introducing such mechanism just for a single use-case, I think it should be integrated in the infrastructure level (see bug 1093025). I'm not really in favor of adding an ad hoc solution for that, which couldn't be reusable and would be inconsistent with other flows. Tal, Do we want to block this bug until bug 1093025 will be implemented? We are attempting to move away from GWT in webadmin and switching to micro-frontend architecture with react plugins implementing pieces of the UI. Therefore, we'll avoid writing GWT code as much as possible, and certainly can't prioritize any GWT infrastructure. Now that bug 1093025 closed there is no question whether to implement it now or not. The allocation policy tab doesn't change when a user decided to do so. When changing the allocation policy tab from Preallocated (by default) to Thin Provision and change the SD, the allocation policy tab doesn't change. ovirt-engine-4.3.0-0.0.master.20180815091554.gitd5455ea.el7.noarch vdsm-4.30.0-527.gitcec1054.el7.x86_64 This bugzilla is included in oVirt 4.2.7 release, published on November 2nd 2018. Since the problem described in this bug report should be resolved in oVirt 4.2.7 release, it has been closed with a resolution of CURRENT RELEASE. If the solution does not work for you, please open a new bug report. Closed by mistake, moving back to qa -> verified Reopening forth to the fact that the issue still being reproduced. Please see the screencast as appears in attachment. Components: ovirt-engine-setup-4.2.7.4-0.1.el7ev.noarch rhvm-appliance-4.2-20181026.1.el7.noarch ovirt-hosted-engine-setup-2.2.31-1.el7ev.noarch ovirt-hosted-engine-ha-2.2.18-1.el7ev.noarch vdsm-4.20.43-1.el7ev.x86_64 libvirt-4.5.0-10.el7_6.2.x86_64 sanlock-3.6.0-1.el7.x86_64 qemu-kvm-rhev-2.12.0-18.el7_6.1.x86_64 Linux 3.10.0-957.el7.x86_64 #1 SMP Thu Oct 4 20:48:51 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux Red Hat Enterprise Linux Server release 7.6 (Maipo) Created attachment 1502382 [details]
screencast
This patch was merged only to 4.3 version and didn't backported to 4.2. Please verify it on the relevant version. QE verification bot: the bug was verified upstream (In reply to Eyal Shenitzky from comment #49) > This patch was merged only to 4.3 version and didn't backported to 4.2. > > Please verify it on the relevant version. If its not backported to 4.2, will it be backported to it later? Do we need separate bug on this? (In reply to Nikolai Sednev from comment #51) > (In reply to Eyal Shenitzky from comment #49) > > This patch was merged only to 4.3 version and didn't backported to 4.2. > > > > Please verify it on the relevant version. > > If its not backported to 4.2, will it be backported to it later? > Do we need separate bug on this? No since this bug was targeted to 4.3 (In reply to Eyal Shenitzky from comment #52) > (In reply to Nikolai Sednev from comment #51) > > (In reply to Eyal Shenitzky from comment #49) > > > This patch was merged only to 4.3 version and didn't backported to 4.2. > > > > > > Please verify it on the relevant version. > > > > If its not backported to 4.2, will it be backported to it later? > > Do we need separate bug on this? > > No since this bug was targeted to 4.3 But what about 4.2? will not contain the fix. This bugzilla is included in oVirt 4.3.0 release, published on February 4th 2019. Since the problem described in this bug report should be resolved in oVirt 4.3.0 release, it has been closed with a resolution of CURRENT RELEASE. If the solution does not work for you, please open a new bug report. |
Created attachment 867027 [details] Screen shots for reconstruction Description of problem: Allocation Policy changed by engine to its default value "Preallocated" from user defined "Thin provision" after changing to another storage domain. Version-Release number of selected component (if applicable): ovirt-engine-webadmin-portal-3.4.0-0.7.beta2.el6.noarch How reproducible: Always Steps to Reproduce: 1.Have a setup with at least two different iSCSI storage domains under the same DC. 2.Disks->Add 3.Fill in any values like 1Gig, name the disk "aaa" and choose "thin provision". 4.Go to "Storage Domain" tab and change through the drop down menu to different SD. 5.Check that system changes "Allocation Policy" to "Preallocated", regardless your previously choice, already taken in section 3. Actual results: WEBUI changes "Allocation Policy" by itself from "Thin provision" to "Preallocated". Expected results: Once user chosen "Allocation Policy", it should not be changed, unless user decides to do so. Additional info: screen shots 1,2,3.