[Piglit] [PATCH] Add dmesg option for reboot policy

yann.argotti at linux.intel.com yann.argotti at linux.intel.com
Wed Nov 25 06:32:25 PST 2015


> On Tue, Nov 24, 2015 at 04:14:23AM -0800, yann.argotti at linux.intel.com
> wrote:

>
> I don't like the aproach of passing the returncode through every level
> of piglit at all. I'd much rather use an exception for this, either by
> extending PiglitFatalError or by adding a new exception, and then
> catching it either in the exceptions.handler decorator, or if we have to
> in run/resume.

Understood, I will apply exception approach then.

>
> I've got a couple more comments inline.
>
> Also, please figure out where the line-wrapping is coming from and
> remove it (if you're pasting this into a window in your email client you
> will get line-wrap which breaks git am and git apply). Using git
> send-email is really the best way to do this.

I should have fix issue on my side now.

> [snip]
>
>> +
>> +    def reboot_is_needed(self):
>> +        """ Simply return the reboot_needed variable state """
>> +        return self._reboot_needed
>
> This is what a property is for:
> @property
> def reboot_needed(self):
> return self._reboot_needed
>
> Then you access the value as an attribute rather than as a function.
>
> If self.reboot_needed:
>     print('reboot now!')

Thanks for highlighting this :)

>
> There is a ton of duplication between LinuxDmesg and
> LinuxMonitoredDmesg. I haven't looked too closely, but it seems like
> LinuxMonitoredDmesg could inherit from LinuxDmesg?

LinuxMonitoredDmesg is quite close but couldn't inherit directly (or may
need to refactor LinuxDmesg code). In addition to look for certain
patterns in dmesg, this class doesn't replace pass with dmesg-warn, and
warn and fail with dmesg-fail, like LinuxDmesg class is doing.
But following various review & comments, I am going to modify the approach
to allow pattern definition and as well as defining where & how to look
for these patterns.

>
> [snip]
>
> Dylan
>
Thanks Dylan for your feedback.

Yann


More information about the Piglit mailing list