Antoni Segura Puimedon has posted comments on this change.
Change subject: Integrate Smartcard support
......................................................................
Patch Set 15: (4 inline comments)
Just a couple of pep8 things. Now I will review the rest.
....................................................
File tests/libvirtvmTests.py
Line 93:
Line 94: def testSmartcardXML(self):
Line 95: smartcardXML = '<smartcard mode="passthrough"
type="spicevmc"/>'
Line 96: dev = {'device': 'smartcard',
Line 97: 'specParams': {'mode': 'passthrough',
'type': 'spicevmc'}}
The indentation is wrong. It should be:
dev = {'device': 'smartcard',
'specParams': {'mode': 'passthrough', 'type':
'spicevmc'}}
Line 98: smartcard = libvirtvm.SmartCardDevice(self.conf, self.log, **dev)
Line 99: self.assertXML(smartcard.getXML(), smartcardXML)
Line 100:
Line 101: def testFeaturesXML(self):
....................................................
File vdsm/libvirtvm.py
Line 2636: Obtain smartcard device info from libvirt.
Line 2637: """
Line 2638: smartcardxml = _domParseStr(self._lastXMLDesc).childNodes[0]. \
Line 2639: getElementsByTagName('devices')[0]. \
Line 2640: getElementsByTagName('smartcard')
The indentation of the previous two lines is wrong. It should be aligned to the
's' in self.
Line 2641: for x in smartcardxml:
Line 2642: if not x.getElementsByTagName('address'):
Line 2643: continue
Line 2644:
Line 2651: dev.alias = alias
Line 2652:
Line 2653: for dev in self.conf['devices']:
Line 2654: if ((dev['device'] == vm.SMARTCARD_DEVICES) and
Line 2655: not dev.get('address')):
This is not pep8 compliant. I recommend putting:
if dev['device'] == vm.SMARCARD_DEVICES and \
not dev.get('address'):
Line 2656: dev['address'] = address
Line 2657: dev['alias'] = alias
Line 2658:
Line 2659: def _getUnderlyingWatchdogDeviceInfo(self):
....................................................
File vdsm/vm.py
Line 554: if self.conf.get('smartcard'):
Line 555: cards.append({'device': SMARTCARD_DEVICES,
Line 556: 'specParams': {
Line 557: 'mode': 'passthrough',
Line 558: 'type': 'spicevmc'}})
lines 555-558 are not pep8 compliant. I recommend:
cards.append({'device': SMARTCARD_DEVICES,
'specParams': {'mode': 'passthrough',
'type': 'spicevmc'}})
Line 559: return cards
Line 560:
Line 561: def getConfSound(self):
Line 562: """
--
To view, visit
http://gerrit.ovirt.org/8450
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7cdaef420c8381d588f6215e66e6a80dd9d2e44b
Gerrit-PatchSet: 15
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <tjelinek(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Barak Azulay <bazulay(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Gal Hammer <ghammer(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Peter V. Saveliev <peet(a)redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjelinek(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server