[Piglit] [Patch v3 1/4] framework/dmesg: Adds a new module for dmesg
Dylan Baker
baker.dylan.c at gmail.com
Mon Feb 3 16:39:06 PST 2014
On Monday, February 03, 2014 06:53:53 PM Ilia Mirkin wrote:
> On Mon, Feb 3, 2014 at 6:40 PM, Dylan Baker <baker.dylan.c at 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
> >
> > Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
> > ---
> >
> > framework/dmesg.py | 167
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed,
> > 167 insertions(+)
> > create mode 100644 framework/dmesg.py
> >
> > diff --git a/framework/dmesg.py b/framework/dmesg.py
> > new file mode 100644
> > index 0000000..d270aaa
> > --- /dev/null
> > +++ b/framework/dmesg.py
> > @@ -0,0 +1,167 @@
> > +# 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()
> > +
> > + # 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]
>
> > + if not re.match('\[\s*\d+\.\d+\]', self._last_message):
> So if I do dmesg -c; ./piglit-run bla this will fail, right? (I'm
> surprised that the string works without the r prefix for raw string,
> but I double-checked and it does...)
I'll add the r
And I don't know how I managed to drop the check to ensure that dmesg wasn't
Falsy, I wrote that code originally.
>
> > + raise EnvironmentError("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 + try:
> > + self._new_messages = dmesg[dmesg.index(self._last_message) +
> > 1:]
> IMO it'd still be an improvement to search from the end -- the vast
> majority of the time, when a piglit test does not emit a new message,
> the _last_message will match up to the last element in dmesg. Instead
> of checking n items (n = # of lines in dmesg), you'd be only checking
> 1...
It's the obvious solution, but I can't find a way to do it that doesn't create
a lot of code bloat with minimal performance gains, or that doesn't produce
performance regressions. I'm opened to suggestions here, I tried to implement
your idea in the other thread, but it ended up being over 20 lines before I
got it working, and it wasn't any faster.
>
> > + except ValueError:
> > + self._new_messages = dmesg
> > +
> > + # Attempt to store the contents of the last message in the dmesg
> > + # snapshot for lookup later. If something happens inbetween (say
> > a user + # calls dmesg -C), then set it to None
>
> ... or if dmesg is clean at the start -- it doesn't have to be someone
> clearing it in the middle of the run.
I'll reword this
>
> > + try:
> > + self._last_message = dmesg[-1]
> > + except IndexError:
> > + self._last_message = None
>
> Completely optional suggestion:
>
> self._last_message = dmesg[-1] if dmesg else None
>
> (In general I prefer to avoid exceptions since you can end up catching
> things you didn't mean to. But that's not everyone's style, so feel
> free to do it whichever way you prefer.)
Yeah, I don't really care either, I'll do it your way
>
> > +
> > +
> > +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
> > +
> > +
> > +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 at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/piglit
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20140203/898699f3/attachment.pgp>
More information about the Piglit
mailing list