Change in vdsm[master]: supervdsm: move udevadm methods to udevadm module

nsoffer at redhat.com nsoffer at redhat.com
Fri Aug 14 13:37:02 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: supervdsm: move udevadm methods to udevadm module
......................................................................


Patch Set 3: Code-Review-1

(2 comments)

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

Line 84: 
Line 85:                         and causes only events from devices that match
Line 86:                         given property to be triggered.
Line 87:     '''
Line 88:     self.__udevReloadRules()
> this should be renamed or dropped, propably
__udevReloadRules should move here.
Line 89:     cmd = ['trigger', '--verbose', '--action', 'change']
Line 90: 
Line 91:     for name, value in property_matches:
Line 92:         cmd.append('--property-match={}={}'.format(name, value))


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

Line 281:     def setSafeNetworkConfig(self):
Line 282:         return setSafeNetworkConfig()
Line 283: 
Line 284:     @logDecorator
Line 285:     def udevTrigger(self, *args, **kwargs):
Using *args, **kwargs make it hard to understand the code. Now you have to drill down to udevadm.trigger to understand what are the arguments.

Should be:

    def udevTrigger(self, attr_matches=(), property_matches=()):
Line 286:         try:
Line 287:             udevadm.trigger(*args, **kwargs)
Line 288:         except udevadm.Error as e:
Line 289:             raise OSError(errno.EINVAL, 'Could not trigger change '


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2b72b580e77872068ae6357410d940a83626b05
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
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