RE: [PATCH 3/3] rasdaemon: ras-mc-ctl: Add exception handling

[Shiju Jose <shiju.jose@huawei.com>]

Hi Mauro,

>-----Original Message-----
>From: Mauro Carvalho Chehab [mailto:mchehab+huawei@kernel.org]
>Sent: 23 December 2020 10:04
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: linux-edac@vger.kernel.org; Linuxarm <linuxarm@huawei.com>;
>tanxiaofei <tanxiaofei@huawei.com>
>Subject: Re: [PATCH 3/3] rasdaemon: ras-mc-ctl: Add exception handling
>
>Em Tue, 3 Nov 2020 14:22:58 +0000
>Shiju Jose <shiju.jose@huawei.com> escreveu:
>
>> Add exception handling in the ras-mc-ctl.
>>
>> For example, when an event's table is not present in the SQLite DB,
>> then the DBI would detect exception and ras-mc-ctl exit without read
>> and log remaining event's information. This would happen when an event
>> is not enabled in the rasdaemon. Following is the error log when the
>> devlink_event table is not present in the DB, "DBD::SQLite::db prepare
>> failed: no such table: devlink_event at ./ras-mc-ctl line 1198.
>> Can't call method "execute" on an undefined value at ./ras-mc-ctl line
>1199"
>>
>> Also disabled the DBI's automatic error logging by setting the
>> $dbh->{PrintError} = 0 to avoid duplicate exception logs.
>
>Hmm...
>
>
>	$ ./util/ras-mc-ctl --summary
>	No Memory errors.
>
>	No PCIe AER errors.
>
>	No ARM processor errors.
>
>	No Extlog errors.
>
>	No devlink errors.
>	No disk errors.
>	Exception: no such table: memory_failure_event
>
>	No MCE errors.
>
>While it sounds a good idea to catch such events, printing it as an exception
>doesn't seem the right thing to me, at least for things like "no such table".
>
>IMO, it should print something more intuitive, like:
>
>	"Warning: Memory failure detection not enabled"

Sure. I will change.
>
>-
>
>Yet, on a separate note, there's no memory_failure_event upstream.
>
>Maybe I missed some prior patch to be applied before this one?

This patch was posted previously.
https://patchwork.kernel.org/project/linux-edac/patch/20201002180144.1365-1-shiju.jose@huawei.com/

>
>
>Thanks,
>Mauro

Thanks,
Shiju