Francesco Romani has posted comments on this change.

Change subject: vdsm: add support for pci host device passthrough
......................................................................


Patch Set 20:

(3 comments)

initial comments

http://gerrit.ovirt.org/#/c/22462/20/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 1286:         m.setAttrs(model=self.specParams['model'])
Line 1287:         return m
Line 1288: 
Line 1289: 
Line 1290: class AcquiredHostDevice(VmDevice):
Why not just HostDevice? name clash with hostdev?
Line 1291:     __slots__ = ('name', 'startupPolicy', '_hostdevice', '_vm')
Line 1292: 
Line 1293:     def __init__(self, *args, **kwargs):
Line 1294:         super(AcquiredHostDevice, self).__init__(*args, **kwargs)


Line 1309:         # Althought not called instantly, we can reattach the device 
when
Line 1310:         # GC is up for it OR it will happen by map's populate() 
method.
Line 1311:         # If the device was already reattached, thanks to __del__'s 
behaviour
Line 1312:         # the exception will simply be ignored
Line 1313:         self._vm.cif.hostDeviceMapper.release(self.name)
Not sure about this, especially about exploiting side effects to ignore 
exceptions.
Line 1314: 
Line 1315:     def getXML(self):
Line 1316:         """
Line 1317:         Create domxml for a hostdev device.


Line 1324:         </hostdev>
Line 1325:         """
Line 1326: 
Line 1327:         if not self._hostdevice:
Line 1328:             return
When could this happen?
Line 1329: 
Line 1330:         hostdev = self.createXmlElem(self.device, self.type)
Line 1331:         hostdev.setAttrs(managed='no', mode='subsystem')
Line 1332:         source = hostdev.appendChildWithArgs('source')


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I363d2622d72ca2db75f60032fe0892c348bab121
Gerrit-PatchSet: 20
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Martin Polednik <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to