Bug 875881 - monitors/usb configuration interaction (stable devices addresses collision?)
monitors/usb configuration interaction (stable devices addresses collision?)
Product: Red Hat Enterprise Virtualization Manager
Classification: Red Hat
Component: ovirt-engine (Show other bugs)
Unspecified Unspecified
urgent Severity unspecified
: ---
: 3.2.0
Assigned To: Arik
Jiri Belka
: 891567 (view as bug list)
Depends On:
Blocks: 915537
  Show dependency treegraph
Reported: 2012-11-12 13:18 EST by David Jaša
Modified: 2013-06-11 05:28 EDT (History)
13 users (show)

See Also:
Fixed In Version: sf3
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed:
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)
engine log (7.86 KB, text/plain)
2012-11-14 03:37 EST, David Jaša
no flags Details
mapping usb configuration from rest api to backend values on update request (8.37 KB, application/octet-stream)
2012-11-29 03:44 EST, Arik
no flags Details
spreadsheet with actions to take on all scenarios (8.52 KB, application/vnd.oasis.opendocument.spreadsheet)
2012-12-05 07:51 EST, Omer Frenkel
no flags Details

  None (edit)
Description David Jaša 2012-11-12 13:18:41 EST
Created attachment 643658 [details]
rest api log

Description of problem:

Version-Release number of selected component (if applicable):
si24.1 / 3.1.0-28

How reproducible:

Steps to Reproduce:
1. set up VM with no USB and single-monitor console
2. set USB mode to Native, confirm
3. set number of monitors to 2
Actual results:
API: monitors are set to 2 but USB is switched to disabled
webadmin/UP: the dialog keeps "spinning" indefinitely

Expected results:
changes are applied exactly as requested

Additional info:
attaching a sample REST API communication
Comment 3 Barak 2012-11-13 14:44:51 EST
David - what happens When you do the same flows in the UI ?
Comment 4 Barak 2012-11-13 14:47:38 EST
Michael - can you please check the API side ?
Comment 5 Michael Pasternak 2012-11-14 03:26:07 EST
(In reply to comment #4)
> Michael - can you please check the API side ?

what to check? we only map to BE entity, and afaiu this is backend issue so it should be the same in all clients.
Comment 6 David Jaša 2012-11-14 03:37:31 EST
Created attachment 644650 [details]
engine log
Comment 7 David Jaša 2012-11-14 03:42:16 EST
engine log is attached, vdsm log is not relevant as you can reproduce the bug on newly-created VM without actually running it.

(In reply to comment #3)
> David - what happens When you do the same flows in the UI ?
the dialog freezes as I already stated in Description.

I suspect stable device addresses because both monitor addition and native usb addition to the VM means that a PCI device has to be added to the VM.
Comment 8 Arik 2012-11-25 03:57:24 EST
Comment 9 Arik 2012-11-26 08:27:15 EST
David - I'll need some help with reproducing that. I tried to reproduce it from the UI with the latest build and si24.1 - with no success (it worked)
the dialog doesn't freeze and the usb remains native with 2 monitors ..
Comment 10 David Jaša 2012-11-26 08:49:24 EST
* API path is 100 % reproducible
* portals do sometimes work for me, too, but more often, they do not:
  * even though General tab says "USB Policy: Native", "Edit VM" 
    dialog says "Disabled"
  * first time I tried now, the page has frozen after I just switched
    USB without even touching of the monitors settings...
Comment 11 Arik 2012-11-29 03:44:53 EST
Created attachment 654054 [details]
mapping usb configuration from rest api to backend values on update request
Comment 12 Arik 2012-11-29 03:48:37 EST
My findings are as follow:

usb configuration is represented in the backend by an enum of 3 values: DISABLED\LEGACY\NATIVE, and in the rest api it is represented in the following structure in the xml file:

When a creation request is received via rest api the mapping between the above representations includes the following logic: if the usb tag is null, map it to DISABLED value in the backend enum.

In the scenario David described above, we get from the rest api an XML in which the display tag is the only tag that is not null (because it contains the monitors), and specifically the usb tag is null. the problem is that in this case we use the same mapping that is being used on creation request (although it's an update request), thus the usb is mapped to DISABLED in the backend enum.

I suggest to define a different mapping for usb configuration on update VM request via rest api, in which null usb tag will be mapped to the existing usb configuration in the backend (no change will be made in the backend). that should fix the bug.

When I started to implement the different mapping for usb configuration on update requests via rest api, I found that there are values which I'm not sure how to map. attachment 654054 [details] contains a table of the proposed mapping - cells that contain '?' are those that I'm not sure about.

Simon - could you please define the desired mapping in those cases?
Comment 13 Andrew Cathrow 2012-12-05 07:27:23 EST
Targeting 3.2
We will reconsider if we get customer issues reports.

Omer has completed spreadsheet.
Comment 14 Omer Frenkel 2012-12-05 07:51:16 EST
Created attachment 658167 [details]
spreadsheet with actions to take on all scenarios
Comment 15 Arik 2012-12-09 07:16:44 EST
Comment 16 Arik 2012-12-10 04:43:59 EST
Andrew - the new mapping for the update vm call says to raise an error if usb configuration with enable=true and no explicit type (legacy/native) is received. I saw that for the create vm call there's a different approach for that case: the usb type is set according to the cluster version (if the version>=3.1 then it is set to native, and to legacy otherwise).
can we use the same logic for the update vm in that case instead of raising an error?
Comment 18 Michal Skrivanek 2013-01-25 05:20:09 EST
*** Bug 891567 has been marked as a duplicate of this bug. ***
Comment 19 Jiri Belka 2013-01-25 12:32:36 EST
OK, sf4, native is kept while changing number of monitors (AP,UP,api).
Comment 20 Itamar Heim 2013-06-11 04:55:54 EDT
3.2 has been released
Comment 21 Itamar Heim 2013-06-11 04:55:58 EDT
3.2 has been released
Comment 22 Itamar Heim 2013-06-11 04:56:08 EDT
3.2 has been released
Comment 23 Itamar Heim 2013-06-11 04:58:40 EDT
3.2 has been released
Comment 24 Itamar Heim 2013-06-11 05:28:09 EDT
3.2 has been released

Note You need to log in before you can comment on or make changes to this bug.