Change in vdsm[master]: supervdsm: only trigger 'vfio' subsystem for VFIO devices

nsoffer at redhat.com nsoffer at redhat.com
Fri Sep 4 23:51:25 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: supervdsm: only trigger 'vfio' subsystem for VFIO devices
......................................................................


Patch Set 3: Code-Review-1

(4 comments)

https://gerrit.ovirt.org/#/c/45345/3/lib/vdsm/udevadm.py
File lib/vdsm/udevadm.py:

Line 86:                         given property to be triggered.
Line 87: 
Line 88:     subsystem_matches   Expects tuple of subsystems.
Line 89: 
Line 90:                         ('a', 'b') ~> --subsystem-match=a --subsystem-match=b
You need a rebase and update the text using same terms used in master, something like "iteratble of subsystems names"
Line 91: 
Line 92:                         Causes only events related to specified subsystem to
Line 93:                         be triggered.
Line 94:     '''


https://gerrit.ovirt.org/#/c/45345/3/vdsm/supervdsmServer
File vdsm/supervdsmServer:

Line 289:             raise OSError(errno.EINVAL, 'Could not trigger change '
Line 290:                           'out %s\nerr %s' % (e.out, e.err))
Line 291: 
Line 292:     @logDecorator
Line 293:     def udevTriggerVFIO(self):
Lets keep this private, as we don't have code calling this yet - right?

And more important - you should not call a public function decorated by @logDecorator from another public function decorated with @logDecorator.

If we do need a public version, it should call a private one doing the error handling and preparing the subsystem matches.
Line 294:         try:
Line 295:             udevadm.trigger(subsystem_matches=('vfio',))
Line 296:         except udevadm.Error as e:
Line 297:             raise OSError(errno.EINVAL, 'Could not trigger change '


Line 349:             with open(rule_file, "w") as rf:
Line 350:                 self.log.debug("Creating rule %s: %r", rule_file, rule)
Line 351:                 rf.write(rule)
Line 352: 
Line 353:             self.udevTriggerVFIO()
Should not call public function.
Line 354: 
Line 355:     @logDecorator
Line 356:     def rmAppropriateIommuGroup(self, iommu_group):
Line 357:         """


Line 376:         else:
Line 377:             self.log.debug("Removing rule %s", rule_file)
Line 378: 
Line 379:         if not error:
Line 380:             self.udevTriggerVFIO()
Same
Line 381: 
Line 382:     @logDecorator
Line 383:     def ksmTune(self, tuningParams):
Line 384:         '''


-- 
To view, visit https://gerrit.ovirt.org/45345
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb4fecbe5108a6ff7d695887e6e60df36e4874d7
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list