Bug 1009469 - 3.3 RHEV-toolsSetup iso is not attached to Windows VM
3.3 RHEV-toolsSetup iso is not attached to Windows VM
Status: CLOSED ERRATA
Product: Red Hat Enterprise Virtualization Manager
Classification: Red Hat
Component: vdsm (Show other bugs)
3.3.0
Unspecified Unspecified
unspecified Severity urgent
: ---
: 3.3.0
Assigned To: Martin Polednik
Ilanit Stein
virt
: Regression
Depends On:
Blocks: 3.3rc1
  Show dependency treegraph
 
Reported: 2013-09-18 10:04 EDT by Jiri Belka
Modified: 2015-09-22 09 EDT (History)
13 users (show)

See Also:
Fixed In Version: is28
Doc Type: Bug Fix
Doc Text:
prepareVolumePath looked for the 'path' key in specParams without looking at the path in the device itself, causing empty paths in specParams to override the device path. Consequently, the RHEV-toolsSetup ISO which provides virtio-drivers was not automatically attached to Windows virtual machines. Now, device.path is added to the checking mechanism to avoid overriding a valid path. The RHEV-toolsSetup ISO is now automatically attached to Windows virtual machines.
Story Points: ---
Clone Of:
Environment:
Last Closed: 2014-01-21 11:15:51 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
vdsm.log (202.80 KB, text/plain)
2013-09-18 10:04 EDT, Jiri Belka
no flags Details


External Trackers
Tracker ID Priority Status Summary Last Updated
oVirt gerrit 20074 None None None Never
oVirt gerrit 22324 None None None Never

  None (edit)
Description Jiri Belka 2013-09-18 10:04:34 EDT
Created attachment 799395 [details]
vdsm.log

Description of problem:
Windows VM always had attached latest RHEV-toolsSetup iso from ISO domain by default. This feature has got broken recently, no iso is attached at all by default. The feature was handy because usual Windows user wants to have virtio drivers and at least spice agent for his/her comfortability.

3.2:

drive file=/rhev/data-center/mnt/10.34.63.204:_home_iso_shared/0c78b4d6-ba00-4d3e-9f9f-65c7d5899d71/images/11111111-1111-1111-1111-111111111111/RHEV-toolsSetup_3.2_14.iso,if=none,media=cdrom,id=drive-ide0-1-0,readonly=on,format=raw,serial= -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0

3.3:
-drive if=none,media=cdrom,id=drive-ide0-1-0,readonly=on,format=raw,serial= -device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0

although engine passes valid iso to vdsm:

Thread-1648::DEBUG::2013-09-18 15:41:18,664::BindingXMLRPC::981::vds::(wrapper) return vmCreate with {'status': {'message': 'Done', 'code': 0}, 'vmList': {'status': 'WaitForLaunch', 'acpiEnable': 'true', 'emulatedMachine': 'rhel6.5.0', 'vmId': '5d93ea43-c794-4ee1-b788-7239b4faa885', 'pid': '0', 'memGuaranteedSize': 2048, 'timeOffset': '3600', 'keyboardLayout': 'en-us', 'displayPort': '-1', 'displaySecurePort': '-1', 'spiceSslCipherSuite': 'DEFAULT', 'cpuType': 'SandyBridge', 'custom': {}, 'clientIp': '', 'nicModel': 'rtl8139,pv', 'smartcardEnable': 'false', 'kvmEnable': 'true', 'transparentHugePages': 'true', 'devices': [{'device': 'qxl', 'specParams': {'vram': '32768', 'heads': '1'}, 'type': 'video', 'deviceId': '7700ed7e-9051-4721-8683-35393374c045', 'address': {'bus': '0x00', ' slot': '0x02', ' domain': '0x0000', ' type': 'pci', ' function': '0x0'}}, {'index': '2', 'iface': 'ide', 'address': {' controller': '0', ' target': '0', 'unit': '0', ' bus': '1', ' type': 'drive'}, 'specParams': {'path': ''}, 'readonly': 'true', 'deviceId': 'b78e69f8-3bfb-4682-8f9f-522e8a8c2666', 'path': '/rhev/data-center/mnt/10.34.63.204:_home_iso_shared/0c78b4d6-ba00-4d3e-9f9f-65c7d5899d71/images/11111111-1111-1111-1111-111111111111/RHEV-toolsSetup_3.3_5.iso', 'device': 'cdrom', 'shared': 'false', 'type': 'disk'}, {'index': 0, 'iface': 'virtio', 'format': 'cow', 'bootOrder': '1', 'poolID': '5849b030-626e-47cb-ad90-3ce782d831b3', 'volumeID': 'e513a76e-98ed-4858-9a0c-253d9997e4e6', 'imageID': '8ac5910a-9b90-47c5-bfbe-f6d268c32045', 'specParams': {}, 'readonly': 'false', 'domainID': '8adc79ee-8269-48b1-993b-407fb829f8ff', 'optional': 'false', 'deviceId': '8ac5910a-9b90-47c5-bfbe-f6d268c32045', 'address': {'bus': '0x00', ' slot': '0x07', ' domain': '0x0000', ' type': 'pci', ' function': '0x0'}, 'device': 'disk', 'shared': 'false', 'propagateErrors': 'off', 'type': 'disk'}, {'device': 'scsi', 'model': 'virtio-scsi', 'type': 'controller'}, {'nicModel': 'pv', 'macAddr': '00:1a:4a:2d:4d:24', 'linkActive': 'true', 'network': 'rhevm', 'custom': {}, 'filter': 'vdsm-no-mac-spoofing', 'specParams': {}, 'deviceId': '83e7018d-65c7-41c2-8460-b27d0a9d09fa', 'address': {'bus': '0x00', ' slot': '0x03', ' domain': '0x0000', ' type': 'pci', ' function': '0x0'}, 'device': 'bridge', 'type': 'interface'}, {'device': 'ac97', 'specParams': {}, 'type': 'sound', 'deviceId': '848aea41-ff77-497a-84db-e25c9fa3dd06', 'address': {'bus': '0x00', ' slot': '0x04', ' domain': '0x0000', ' type': 'pci', ' function': '0x0'}}, {'device': 'memballoon', 'specParams': {'model': 'virtio'}, 'type': 'balloon', 'deviceId': '22ede7f1-aafd-4101-84cd-162572b2102d'}], 'smp': '2', 'vmType': 'kvm', 'memSize': 2048, 'displayIp': '0', 'spiceSecureChannels': 'smain,sinputs,scursor,splayback,srecord,sdisplay,susbredir,ssmartcard', 'smpCoresPerSocket': '1', 'vmName': 'jb-w8-x86', 'display': 'qxl', 'nice': '0'}}

but it gets lost somewhere later:

# sed -n '/<domain/,/domain>/p' /tmp/out.log  | sed -n '/device.*cdrom/,/disk>/p'
                <disk device="cdrom" snapshot="no" type="file">
                        <address bus="1" controller="0" target="0" type="drive" unit="0"/>
                        <source file="" startupPolicy="optional"/>
                        <target bus="ide" dev="hdc"/>
                        <readonly/>
                        <serial/>
                </disk>

iso of course exists:

# ls -l /rhev/data-center/mnt/10.34.63.204:_home_iso_shared/0c78b4d6-ba00-4d3e-9f9f-65c7d5899d71/images/11111111-1111-1111-1111-111111111111/RHEV-toolsSetup_3.3_5.iso
-rw-r-----. 1 vdsm kvm 295915520 Sep 10 14:30 /rhev/data-center/mnt/10.34.63.204:_home_iso_shared/0c78b4d6-ba00-4d3e-9f9f-65c7d5899d71/images/11111111-1111-1111-1111-111111111111/RHEV-toolsSetup_3.3_5.iso

Version-Release number of selected component (if applicable):
is15

How reproducible:
100%

Steps to Reproduce:
1. iso domain with tools iso (major.minor version must match engine)
2. start a windows vm
3.

Actual results:
tools iso not attached

Expected results:
tools iso must be attached

Additional info:
Comment 1 Martin Polednik 2013-10-02 08:29:34 EDT
this bug is caused by duplicate cdrom paths being sent to VDSM by vmCreate call:

vmCreate with 'run once' attached CD:
{..., 'specParams': {}, ..., 'path': 'REAL_PATH', 'device': 'cdrom', 'type': 'disk'}

vmCreate with Windows as target machine type:
{..., 'specParams': {'path': ''}, ..., 'path': 'REAL_PATH', 'device': 'cdrom', 'type': 'disk'}
- in this case, the VDSM assumes that when there is empty path in specParams the cdrom should be empty

fix should be trivial, question is whether we want to change the way VDSM checks for path in received query or fix engine side to avoid sending both paths
Comment 2 Shahar Havivi 2013-10-10 06:22:03 EDT
(In reply to Martin Polednik from comment #1)
AFAIK there is a default CDROM which is the first CDROM that Engine send to VDSM.
When sending empty path VDSM should attach empty CDROM (for the guest tools).

Martin,
What is the patch that broke it?
Comment 3 Martin Polednik 2013-10-10 06:51:54 EDT
My guess would be http://gerrit.ovirt.org/#/c/6803/ (quite old, the change was probably some time after this patch and happened in engine)
where prepareVolumePath started handling empty cdrom, the problem is data comes in cdrom.path instead of cdrom.specParams['path'] where the second one is actually being checked for being empty 
(if 'specParams' in drive and 'path' in specParams and drive['specParams']['path'] = '')
Comment 4 Shahar Havivi 2013-10-10 07:09:01 EDT
(In reply to Martin Polednik from comment #3)
> My guess would be http://gerrit.ovirt.org/#/c/6803/ (quite old, the change
> was probably some time after this patch and happened in engine)
> where prepareVolumePath started handling empty cdrom, the problem is data
> comes in cdrom.path instead of cdrom.specParams['path'] where the second one
> is actually being checked for being empty 
> (if 'specParams' in drive and 'path' in specParams and
> drive['specParams']['path'] = '')
Yes,
This is more reasonable that the path will have its own property and not pass via the specParams.
In this case the fix should be in VDSM
Comment 5 Dan Kenigsberg 2013-10-11 09:13:48 EDT
Shahar, do you have any idea why drive['specParams']['path'] is sent from Engine to begin with? I think neither Vdsm nor Engine use it.
Comment 6 Eli Mesika 2013-10-13 05:48:00 EDT
(In reply to Dan Kenigsberg from comment #5)
> Shahar, do you have any idea why drive['specParams']['path'] is sent from
> Engine to begin with? I think neither Vdsm nor Engine use it.

Possibly uncleaned code from the first Stable PCI Devices implementation, we has switched to pass the path directly later on ....
Comment 8 Dan Kenigsberg 2013-10-13 06:54:27 EDT
(In reply to Eli Mesika from comment #6)
> Possibly uncleaned code from the first Stable PCI Devices implementation, we
> has switched to pass the path directly later on ....

Could this code be dropped now?
Comment 9 Eli Mesika 2013-10-13 07:13:14 EDT
(In reply to Dan Kenigsberg from comment #8)
> (In reply to Eli Mesika from comment #6)
> > Possibly uncleaned code from the first Stable PCI Devices implementation, we
> > has switched to pass the path directly later on ....
> 
> Could this code be dropped now?

AFAIK can be removed from engine since we are using path directly , please check and verify for VDSM
Comment 12 errata-xmlrpc 2014-01-21 11:15:51 EST
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2014-0040.html

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