Change in vdsm[ovirt-3.6]: supervdsm: move udevadm methods to udevadm module

mpolednik at redhat.com mpolednik at redhat.com
Tue Sep 22 08:55:03 UTC 2015


Hello Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    https://gerrit.ovirt.org/46506

to review the following change.

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

supervdsm: move udevadm methods to udevadm module

udevadm module already exists, so there is no reason to keep the
method logic in supervdsm. udevadm also provides us with a means to
execute a command, it is therefore used instead of EXT_UDEVADM. To
keep the behavior same, Error raised by trigger is caught in
supervdsm and re-raised as IOError (same as before the patch).

Change-Id: Ie2b72b580e77872068ae6357410d940a83626b05
Signed-off-by: Martin Polednik <mpolednik at redhat.com>
Reviewed-on: https://gerrit.ovirt.org/44808
Continuous-Integration: Jenkins CI
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M configure.ac
M lib/vdsm/constants.py.in
M lib/vdsm/udevadm.py
M vdsm/supervdsmServer
4 files changed, 45 insertions(+), 53 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/06/46506/1

diff --git a/configure.ac b/configure.ac
index ae2b722..382370c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -317,7 +317,6 @@
 AC_PATH_PROG([TEE_PATH], [tee], [/usr/bin/tee])
 AC_PATH_PROG([TOUCH_PATH], [touch], [/bin/touch])
 AC_PATH_PROG([TUNE2FS_PATH], [tune2fs], [/sbin/tune2fs])
-AC_PATH_PROG([UDEVADM_PATH], [udevadm], [/sbin/udevadm])
 AC_PATH_PROG([UMOUNT_PATH], [umount], [/bin/umount])
 AC_PATH_PROG([WGET_PATH], [wget], [/usr/bin/wget])
 
diff --git a/lib/vdsm/constants.py.in b/lib/vdsm/constants.py.in
index 541786b..82fc6d1 100644
--- a/lib/vdsm/constants.py.in
+++ b/lib/vdsm/constants.py.in
@@ -140,8 +140,6 @@
 EXT_TAR = '@TAR_PATH@'
 EXT_TUNE2FS = '@TUNE2FS_PATH@'
 
-EXT_UDEVADM = '@UDEVADM_PATH@'
-
 EXT_UMOUNT = '@UMOUNT_PATH@'
 
 EXT_SAFELEASE = '@SAFELEASE_PATH@'
diff --git a/lib/vdsm/udevadm.py b/lib/vdsm/udevadm.py
index 2e9bdc2..24327d3 100644
--- a/lib/vdsm/udevadm.py
+++ b/lib/vdsm/udevadm.py
@@ -61,6 +61,43 @@
         logging.error("%s", e)
 
 
+def trigger(attr_matches=(), property_matches=()):
+    '''
+    Request device events from the kernel.
+
+    Arguments:
+
+    attr_matches        List of 2-tuples that contain attribute name and
+                        it's value. These are expanded like this:
+
+                        [('a', 'b'), ('c', 'd')] ~>
+                        --attr-match=a=b --attr-match=c=d
+
+                        and causes only events from devices that match
+                        given attributes to be triggered.
+
+    property_matches    Similar to attr_matches. Expects list of 2-tuples
+                        that expand in similar fashion, that is
+
+                        [('a', 'b'), ('c', 'd')] ~>
+                        --property-match=a=b --property-match=c=d
+
+                        and causes only events from devices that match
+                        given property to be triggered.
+    '''
+    _run_command(['control', '--reload'])
+
+    cmd = ['trigger', '--verbose', '--action', 'change']
+
+    for name, value in property_matches:
+        cmd.append('--property-match={}={}'.format(name, value))
+
+    for name, value in attr_matches:
+        cmd.append('--attr-match={}={}'.format(name, value))
+
+    _run_command(cmd)
+
+
 def _run_command(args):
     cmd = [_UDEVADM.cmd]
     cmd.extend(args)
diff --git a/vdsm/supervdsmServer b/vdsm/supervdsmServer
index 70fedb8..05b189d 100755
--- a/vdsm/supervdsmServer
+++ b/vdsm/supervdsmServer
@@ -55,6 +55,7 @@
 except ImportError:
     _glusterEnabled = False
 
+from vdsm import udevadm
 from vdsm import utils
 from vdsm import sysctl
 from vdsm.tool import restore_nets
@@ -73,7 +74,7 @@
 from storage import multipath
 from storage.fileUtils import chown, resolveGid, resolveUid
 from storage.fileUtils import validateAccess as _validateAccess
-from vdsm.constants import METADATA_GROUP, EXT_CHOWN, EXT_UDEVADM, \
+from vdsm.constants import METADATA_GROUP, EXT_CHOWN, \
     DISKIMAGE_USER, DISKIMAGE_GROUP, \
     P_LIBVIRT_VMCHANNELS, P_OVIRT_VMCONSOLES, \
     VDSM_USER, QEMU_PROCESS_USER, QEMU_PROCESS_GROUP, RHEV_ENABLED
@@ -302,46 +303,11 @@
 
     @logDecorator
     def udevTriggerMultipath(self, guid):
-        self._udevTrigger(property_matches=(('DM_NAME', guid),))
-
-    def _udevTrigger(self, attr_matches=(), property_matches=()):
-        '''
-        Request device events from the kernel.
-
-        Arguments:
-
-        attr_matches        An iterable of attribute-value iterables,
-                            expanded like this:
-
-                            (('a', 'b'), ('c', 'd')) ~>
-                            --attr-match=a=b --attr-match=c=d
-
-                            and cause only events from devices that match
-                            given attributes to be triggered.
-
-        property_matches    Similar to attr_matches, it is an iterable of
-                            attribute-value iterables that expand
-                            in a similar fashion:
-
-                            (('a', 'b'), ('c', 'd')) ~>
-                            --property-match=a=b --property-match=c=d
-
-                            and cause only events from devices that match
-                            given properties to be triggered.
-        '''
-        self.__udevReloadRules()
-        cmd = [EXT_UDEVADM, 'trigger', '--verbose', '--action', 'change']
-
-        for name, value in property_matches:
-            cmd.append('--property-match={}={}'.format(name, value))
-
-        for name, value in attr_matches:
-            cmd.append('--attr-match={}={}'.format(name, value))
-
-        rc, out, err = utils.execCmd(cmd)
-        if rc:
+        try:
+            udevadm.trigger(property_matches=(('DM_NAME', guid),))
+        except udevadm.Error as e:
             raise OSError(errno.EINVAL, 'Could not trigger change '
-                          'out %s\nerr %s' % (out, err))
+                          'out %s\nerr %s' % (e.out, e.err))
 
     @logDecorator
     def appropriateDevice(self, guid, thiefId):
@@ -396,7 +362,7 @@
                 self.log.debug("Creating rule %s: %r", rule_file, rule)
                 rf.write(rule)
 
-            self._udevTrigger()
+            udevadm.trigger()
 
     @logDecorator
     def rmAppropriateIommuGroup(self, iommu_group):
@@ -423,7 +389,7 @@
             self.log.debug("Removing rule %s", rule_file)
 
         if not error:
-            self._udevTrigger()
+            udevadm.trigger()
 
     @logDecorator
     def ksmTune(self, tuningParams):
@@ -498,14 +464,6 @@
     @logDecorator
     def set_rp_filter_strict(self, dev):
         sysctl.set_rp_filter_strict(dev)
-
-    def __udevReloadRules(self):
-        cmd = [EXT_UDEVADM, 'control', '--reload']
-        rc, out, err = utils.execCmd(cmd)
-        if rc:
-            self.log.error("Udevadm reload-rules command failed rc=%s, "
-                           "out=\"%s\", err=\"%s\"", rc, out, err)
-            raise OSError(errno.EINVAL, "Could not reload rules")
 
 
 def terminate(signo, frame):


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie2b72b580e77872068ae6357410d940a83626b05
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>


More information about the vdsm-patches mailing list