Change in vdsm[master]: vm: graphdev: fix parameter passing

danken at redhat.com danken at redhat.com
Mon Aug 11 15:11:32 UTC 2014


Dan Kenigsberg has submitted this change and it was merged.

Change subject: vm: graphdev: fix parameter passing
......................................................................


vm: graphdev: fix parameter passing

This patch fixes the parameter passing to updateDevice.
The problem is I originally overlooked how hook params are
supposed to be passed to the hook.

The tests with this patch highlights the problem:

File "vdsm/vdsm/virt/vm.py", line 4408,
in _setTicketForGraphicDev
hooks.before_vm_set_ticket(self._lastXMLDesc, self.conf, params)
File "/home/fromani/Projects/upstream/vdsm/tests/vmTests.py", line 1403,
in _check_ticket_params
self.assertEqual(params, TICKET_PARAMS)
AssertionError: {'params': {'userName': 'admin', 'userId':
'fdfc627c-d875-11e0-90f0-83df133b58cc [truncated]... != {'userName':
'admin', 'userId': 'fdfc627c-d875-11e0-90f0-83df133b58cc'}
- {'params': {'userId': 'fdfc627c-d875-11e0-90f0-83df133b58cc',
? -----------

+ {'userId': 'fdfc627c-d875-11e0-90f0-83df133b58cc', 'userName':
'admin'} ? +++++++++++++++++++++

    'userName': 'admin'}}

Here the meaning of 'params', per API spec, is the same as setTicket.
'params' should be a StringMap.

Unfortunately, updateDevice accepts a single argument:

'params': 'vmUpdateDeviceParams' (quoting spec again)

Confusion arises by the nesting of 'params', hence the bug.
The hook receives the nested params instead of the plain StringMap
it should receive as per spec.

Moreover, no hooks are using graphics devices, so this bug slipped
unnoticed. The new tests will help this to not happen again.

As nice side effect, this allows a cleaner and shorter
implementation.

Change-Id: I3716b48a5fbe5beebb819754d0f500b097f04677
Signed-off-by: Francesco Romani <fromani at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/28036
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M tests/vmTests.py
M vdsm/virt/vm.py
2 files changed, 22 insertions(+), 15 deletions(-)

Approvals:
  Dan Kenigsberg: Looks good to me, approved
  Francesco Romani: Verified



-- 
To view, visit http://gerrit.ovirt.org/28036
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I3716b48a5fbe5beebb819754d0f500b097f04677
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Roy Golan <rgolan at redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list