Igor Lvovsky has posted comments on this change.
Change subject: Integrate Smartcard support
......................................................................
Patch Set 17: I would prefer that you didn't submit this
(4 inline comments)
Maybe I am missing something but it looks to me as very partial.
In additional I would prefer to get smartcard device as any other devices
within 'devices' dictionary in API but not as odd bool parameter
....................................................
File vdsm/libvirtvm.py
Line 1188: """
Line 1189: Add smartcard section to domain xml
Line 1190:
Line 1191: <smartcard mode='passthrough' type='spicevmc'>
Line 1192: <address ... />
there is no such thing 'address' for type='spicevms'
Line 1193: </smartcard>
Line 1194: """
Line 1195: card = self.createXmlElem(self.device, None, ['address'])
Line 1196: card.setAttribute('mode', self.specParams['mode'])
Line 1193: </smartcard>
Line 1194: """
Line 1195: card = self.createXmlElem(self.device, None, ['address'])
Line 1196: card.setAttribute('mode', self.specParams['mode'])
Line 1197: card.setAttribute('type', self.specParams['type'])
It looks to me as very partially according to
http://libvirt.org/formatdomain.html#elementsSmartcard
Line 1198: return card
Line 1199:
Line 1200:
Line 1201: class RedirDevice(LibvirtVmDevice):
Line 2654: if dev['device'] == vm.SMARTCARD_DEVICES and \
Line 2655: not dev.get('address'):
Line 2656: dev['address'] = address
Line 2657: dev['alias'] = alias
Line 2658:
Maybe I am missing something but according to
http://libvirt.org/formatdomain.html#elementsSmartcard there is no 'alias' in
smartcard device
and address exists only for type='tcp'
Line 2659: def _getUnderlyingWatchdogDeviceInfo(self):
Line 2660: """
Line 2661: Obtain watchdog device info from libvirt.
Line 2662: """
....................................................
File vdsm/vm.py
Line 553: cards = []
Line 554: if self.conf.get('smartcard'):
Line 555: cards.append({'device': SMARTCARD_DEVICES,
Line 556: 'specParams': {'mode': 'passthrough',
Line 557: 'type': 'spicevmc'}})
why it's hardcoded?
why not to get it from engine as normal device instead of bizarre bool
parameter?
According to http://libvirt.org/formatdomain.html#elementsSmartcard
there are another type='tcp' exist.
Line 558: return cards
Line 559:
Line 560: def getConfSound(self):
Line 561: """
--
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: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Igor Lvovsky <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Peter V. Saveliev <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
