On Tuesday, February 04, 2014 10:59:20 AM Ilia Mirkin wrote: > On Tue, Feb 4, 2014 at 10:50 AM, Dylan Baker <baker.dyla...@gmail.com> wrote: > > This modules implements two classes and a helper function. The two > > classes are both dmesg wrapper classes, one, PosixDmesg is used on posix > > systems when requested to actually do checks on dmesg. The second class, > > DummyDmesg, is used to reduce code complexity by providing dummy > > versions of the methods in PosixDmesg. This means that we don't need > > separate code paths for Posix systems wanting to check dmesg, Posix > > systems not wanting to check dmesg, and non-posix systems which lack > > dmesg. > > > > Beyond reducing complexity this module also gives better isolation, > > dmesg is only used in Test.execute(), no where else. Additional classes > > don't need to worry about dmesg that way, it's just available. > > > > v2: - Remove unnecessary assert from PosixDmesg.update_result() > > > > - simplify replace helper in PoixDmesg.update_result() > > - replace try/except with if check > > - Change PosixDmesg.update_dmesg() to work even if dmesg 'wraps' > > > > v3: - Try/Except assignment of PosixDmesg._last_message in update_dmesg() > > > > - Set the dmesg levels the same as the previous implementation > > - Change PosixDmesg into LinuxDmesg and enforce that dmesg has > > > > timestamps > > > > v4: - Add check in LinuxDmesg.__init__ to ensure there is something in > > > > dmesg before doing the timestamp check. This should make problems > > clearer > > > > - Refactor some code in LinuxDmesg.update_result() > > > > Signed-off-by: Dylan Baker <baker.dyla...@gmail.com> > > --- > > > > framework/dmesg.py | 174 > > +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, > > 174 insertions(+) > > create mode 100644 framework/dmesg.py > > > > diff --git a/framework/dmesg.py b/framework/dmesg.py > > new file mode 100644 > > index 0000000..eac9606 > > --- /dev/null > > +++ b/framework/dmesg.py > > @@ -0,0 +1,174 @@ > > +# Copyright (c) 2013-2014 Intel Corporation > > +# > > +# Permission is hereby granted, free of charge, to any person obtaining a > > +# copy of this software and associated documentation files (the > > "Software"), +# to deal in the Software without restriction, including > > without limitation +# the rights to use, copy, modify, merge, publish, > > distribute, sublicense, +# and/or sell copies of the Software, and to > > permit persons to whom the +# Software is furnished to do so, subject to > > the following conditions: +# > > +# The above copyright notice and this permission notice (including the > > next +# paragraph) shall be included in all copies or substantial > > portions of the +# Software. > > +# > > +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS > > OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > > MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. > > IN NO EVENT SHALL +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY > > CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, > > TORT OR OTHERWISE, ARISING +# FROM, OUT OF OR IN CONNECTION WITH THE > > SOFTWARE OR THE USE OR OTHER DEALINGS +# IN THE SOFTWARE. > > + > > +""" Module implementing classes for reading posix dmesg > > + > > +Currently this module only has the default DummyDmesg, and a LinuxDmesg. > > +The method used on Linux requires that timetamps are enabled, and no > > other > > +posix system has timestamps. > > + > > +On OSX and *BSD one would likely want to implement a system that reads > > the > > +sysloger, since timestamps can be added by the sysloger, and are not > > inserted +by dmesg. > > + > > +""" > > + > > +import re > > +import sys > > +import subprocess > > + > > +__all__ = ['LinuxDmesg', 'DummyDmesg', 'get_dmesg'] > > + > > + > > +class LinuxDmesg(object): > > + """ Read dmesg on posix systems > > + > > + This reads the dmesg ring buffer, stores the contents of the buffer, > > and + then compares it whenever called, returning the difference > > between the + calls. It requires timestamps to be enabled. > > + > > + """ > > + DMESG_COMMAND = ['dmesg', '--level', > > 'emerg,alert,crit,err,warn,notice'] + > > + def __init__(self): > > + """ Create a dmesg instance """ > > + self._last_message = None > > + self._new_messages = [] > > + > > + # Populate self.dmesg initially, otherwise the first test will > > always + # be full of dmesg crud. > > + self.update_dmesg() > > + > > + if not self._last_message: > > + # We need to ensure that there is something in dmesg to check > > + # timestamps. > > + raise DmesgError( > > + "Your dmesg ringbuffer is empty, so it's not possible to > > check " + "for timestamp support. Try adding something to > > dmesg by " + "writing /dev/kmsg. ex `echo 'foo' >> > > /dev/kmsg") > TBH, I'm not a fan of that -- doing "dmesg -c" isn't such an uncommon > thing. Printing out a warning is fine (and then erroring out when > things don't pan out), but I dunno about making it a hard requirement > to have something in dmesg. >
Yeah, I'm not super happy with it either. I just don't see anywhere in the actually called methods we can bail out without having unexpected results first, without doing something as awful as a check for timestamps in dmesg_update(), which would lead to a bad performance hit. Any suggestions? > > + if not re.match(r'\[\s*\d+\.\d+\]', self._last_message): > > + # Do an initial check to ensure that dmesg has timestamps, we > > need + # timestamps to work correctly. A proper linux dmesg > > timestamp + # looks like this: [ 0.00000] > > + raise DmesgError( > > + "Your kernel does not seem to support timestamps, which " > > + "are required by the --dmesg option") > > + > > + def update_result(self, result): > > + """ Takes a TestResult object and updates it with dmesg statuses > > + > > + If dmesg is enabled, and if dmesg has been updated, then replace > > pass + with dmesg-warn and warn and fail with dmesg-fail. If dmesg > > has not + been updated, it will return the original result passed > > in. + > > + Arguments: > > + result -- A TestResult instance > > + > > + """ > > + > > + def replace(res): > > + """ helper to replace statuses with the new dmesg status > > + > > + Replaces pass with dmesg-warn, and warn and fail with > > dmesg-fail, + otherwise returns the input > > + > > + """ > > + return {"pass": "dmesg-warn", > > + "warn": "dmesg-fail", > > + "fail": "dmesg-fail"}.get(res, res) > > + > > + # Get a new snapshot of dmesg > > + self.update_dmesg() > > + > > + # if update_dmesg() found new entries replace the results of the > > test + # and subtests > > + if self._new_messages: > > + result['result'] = replace(result['result']) > > + > > + # Replace any subtests > > + if 'subtest' in result: > > + for key, value in result['subtest'].iteritems(): > > + result['subtest'][key] = replace(value) > > + > > + # Add the dmesg values to the result > > + result['dmesg'] = self._new_messages > > + > > + return result > > + > > + def update_dmesg(self): > > + """ Call dmesg using subprocess.check_output > > + > > + Get the contents of dmesg, then calculate new messages, finally > > set + self.dmesg equal to the just read contents of dmesg. > > + > > + """ > > + dmesg = > > subprocess.check_output(self.DMESG_COMMAND).strip().splitlines() + > > + # Find all new entries, do this by slicing the list of dmesg to > > only + # returns elements after the last element stored. If there > > are not + # matches a value error is raised, that means all of > > dmesg is new + l = 0 > > + for index, item in enumerate(reversed(dmesg)): > > + if item == self._last_message: > > + l = len(dmesg) - index # don't include the matched > > element > Forgot a break here? That's the part that makes this whole thing worthwhile > :) yup. > > + self._new_messages = dmesg[l:] > > + > > + # Attempt to store the last element of dmesg, unless there was no > > dmesg + self._last_message = dmesg[-1] if dmesg else None > > + > > + > > +class DummyDmesg(object): > > + """ An dummy class for dmesg on non unix-like systems > > + > > + This implements a dummy version of the LinuxDmesg, and can be used > > anytime + it is not desirable to actually read dmesg, such as > > non-posix systems, or + when the contents of dmesg don't matter. > > + > > + """ > > + DMESG_COMMAND = [] > > + > > + def __init__(self): > > + pass > > + > > + def update_dmesg(self): > > + """ Dummy version of update_dmesg """ > > + pass > > + > > + def update_result(self, result): > > + """ Dummy version of update_result """ > > + return result > > + > > + > > +class DmesgError(EnvironmentError): > > + pass > > + > > + > > +def get_dmesg(not_dummy=True): > > + """ Return a Dmesg type instance > > + > > + Normally this does a system check, and returns the type that proper > > for + your system. However, if Dummy is True then it will always > > return a + DummyDmesg instance. > > + > > + """ > > + if sys.platform.startswith('linux') and not_dummy: > > + return LinuxDmesg() > > + return DummyDmesg() > > -- > > 1.8.5.3 > > > > _______________________________________________ > > Piglit mailing list > > Piglit@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/piglit
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit