master - dev-type: print log_sys_debug

master - dev-type: print log_sys_debug

[Zdenek Kabelac]

Gitweb:        http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=a94abc0fdd33b31cd292d9b9a61ddceb7b72208f
Commit:        a94abc0fdd33b31cd292d9b9a61ddceb7b72208f
Parent:        ac60c876c43d0ebc7e642dcc92528b974bd7b9f5
Author:        Zdenek Kabelac <zkabelac@redhat.com>
AuthorDate:    Thu Jul 3 12:03:16 2014 +0200
Committer:     Zdenek Kabelac <zkabelac@redhat.com>
CommitterDate: Fri Jul 4 12:31:17 2014 +0200

dev-type: print log_sys_debug

For non-fatal error use log_sys_debug as the tool
is not stopping on these errors.
---
 lib/device/dev-type.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/lib/device/dev-type.c b/lib/device/dev-type.c
index e0cae45..0d91017 100644
--- a/lib/device/dev-type.c
+++ b/lib/device/dev-type.c
@@ -676,7 +676,7 @@ static unsigned long _dev_topology_attribute(struct dev_types *dt,
 	 */
 	if (stat(path, &info) == -1) {
 		if (errno != ENOENT) {
-			log_sys_error("stat", path);
+			log_sys_debug("stat", path);
 			return 0;
 		}
 		if (!dev_get_primary_dev(dt, dev, &primary))
@@ -688,24 +688,23 @@ static unsigned long _dev_topology_attribute(struct dev_types *dt,
 
 		if (stat(path, &info) == -1) {
 			if (errno != ENOENT)
-				log_sys_error("stat", path);
+				log_sys_debug("stat", path);
 			return 0;
 		}
 	}
 
 	if (!(fp = fopen(path, "r"))) {
-		log_sys_error("fopen", path);
+		log_sys_debug("fopen", path);
 		return 0;
 	}
 
 	if (!fgets(buffer, sizeof(buffer), fp)) {
-		log_sys_error("fgets", path);
+		log_sys_debug("fgets", path);
 		goto out;
 	}
 
 	if (sscanf(buffer, "%lu", &result) != 1) {
-		log_error("sysfs file %s not in expected format: %s", path,
-			  buffer);
+		log_warn("sysfs file %s not in expected format: %s", path, buffer);
 		goto out;
 	}
 
@@ -714,7 +713,7 @@ static unsigned long _dev_topology_attribute(struct dev_types *dt,
 
 out:
 	if (fclose(fp))
-		log_sys_error("fclose", path);
+		log_sys_debug("fclose", path);
 
 	return result >> SECTOR_SHIFT;
 }

master - dev-type: print log_sys_debug

[Alasdair G Kergon]

On Fri, Jul 04, 2014 at 10:32:04AM +0000, Zdenek Kabelac wrote:
> For non-fatal error use log_sys_debug as the tool
> is not stopping on these errors.

To be clear about the criteria for the different message levels:

If the code exits with an error status code there MUST have been a log_error.

But issuing a log_error does NOT mean the code must stop processing and exit.

Using log_error means "This is something the user needs to be told about".

That might include something that should never occur on a properly-functioning
system, but which if it does occur is not fatal to the continued processing
of the command.

Look at it the other way around:
  Given that the error *has* occurred, what can be deduced about the likely
  cause and does the user need to be aware of that cause in case they
  want to investigate it?

However, the cases where a log_error is non-fatal should be quite rare in the
code.

log_warn is for something that MIGHT lead to unexpected behaviour, incorrect
processing or irreversible change etc. but which MIGHT also be perfectly OK if
it was indeed the user's intention.  If there are NO circumstances where the
thing being reported is normal or expected, log_warn should NOT be used.

Alasdair

master - dev-type: print log_sys_debug

[Zdenek Kabelac]

Dne 4.7.2014 19:22, Alasdair G Kergon napsal(a):
> On Fri, Jul 04, 2014 at 10:32:04AM +0000, Zdenek Kabelac wrote:
>> For non-fatal error use log_sys_debug as the tool
>> is not stopping on these errors.
>
> To be clear about the criteria for the different message levels:
>
> If the code exits with an error status code there MUST have been a log_error.
>
> But issuing a log_error does NOT mean the code must stop processing and exit.
>
> Using log_error means "This is something the user needs to be told about".
>
> That might include something that should never occur on a properly-functioning
> system, but which if it does occur is not fatal to the continued processing
> of the command.
>
> Look at it the other way around:
>    Given that the error *has* occurred, what can be deduced about the likely
>    cause and does the user need to be aware of that cause in case they
>    want to investigate it?
>
> However, the cases where a log_error is non-fatal should be quite rare in the
> code.


Then we most likely need something like

log_non_fatal_error() -

since log_error is tracked and checked if there has been error for the
reason the command has exited with error code.

If we would left log_error() as is - then 'error' usage is already marked,
and the 'real' error might not be actually printed (forgotten to be added to 
the code) - thus user could get misleading idea what's been the real reason 
for error.

(Thought in this particular case - it really looked rather like debug,
since nothing bad happens - and user could be just more confused)

Zdenek

master - dev-type: print log_sys_debug

[Alasdair G Kergon]

On Fri, Jul 04, 2014 at 10:15:27PM +0200, Zdenek Kabelac wrote:
> If we would left log_error() as is - then 'error' usage is already marked,
> and the 'real' error might not be actually printed (forgotten to be added 
> to the code) - thus user could get misleading idea what's been the real 
> reason for error.

That doesn't concern me, as we consider the likelihood of the first non-fatal
error should be rare enough already - it's not important if the "less-important"
error code is the one returned should there be a further error later on: all
errors should be investigated in that case.

Alasdair