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

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

Hi Mauro,

>-----Original Message-----
>From: Shiju Jose [mailto:shiju.jose@huawei.com]
>Sent: 04 January 2021 09:36
>To: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
>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
>
>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.

The cause of exception would be vary on different errors.
Thus I think we cannot add a specific error message here.
" no such table: memory_failure_event " is the print of DBI:errstr  
when the  table is not found in the SQL database. There could be other
error cases as well.
>>
>>-
>>
>>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