Bug 1009469

Summary: 3.3 RHEV-toolsSetup iso is not attached to Windows VM
Product: Red Hat Enterprise Virtualization Manager Reporter: Jiri Belka <jbelka>
Component: vdsmAssignee: Martin Polednik <mpoledni>
Status: CLOSED ERRATA QA Contact: Ilanit Stein <istein>
Severity: urgent Docs Contact:
Priority: unspecified    
Version: 3.3.0CC: acathrow, bazulay, danken, dkajan, ecohen, emesika, iheim, lpeer, mavital, michal.skrivanek, ofrenkel, shavivi, yeylon
Target Milestone: ---Keywords: Regression
Target Release: 3.3.0   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard: virt
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 16:15:51 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1049022    
Attachments:
Description Flags
vdsm.log none

Description Jiri Belka 2013-09-18 14:04:34 UTC
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 12:29:34 UTC
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 10:22:03 UTC
(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 10:51:54 UTC
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 11:09:01 UTC
(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 13:13:48 UTC
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 09:48:00 UTC
(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 10:54:27 UTC
(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 11:13:14 UTC
(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 16:15:51 UTC
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