[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[lvm-devel] [RFC PATCH] Reduce noise about missing devices.



Hi,

this is not intended to be directly applied, but more to start a
discussion. One problem with our handling of missing devices is the huge
amount of log noise each missing device produces. In a single run of
lvconvert --repair, I get multiple screens of "read error" and "Couldn't
find device with uuid"...

The attached patch reduces that noise by only reporting these things
once per run. I have used two different approaches for read errors and
for uuid's. Neither is perfect...

Another possible approach (one that I would probably prefer) is to add a
global log_error_once to log.c that would maintain a dm_hash_table of
error messages and downgrade already-reported errors to log_debug or
log_very_verbose automatically. A few error paths could use this.

The downside of all these approaches is that this is sort of global
knowledge, but I guess that's hard to avoid. Presumably, adding a call
to erase the hashtable to _init_logging would solve most of the problems
with this. I suspect that threading a toolcontext pointer through
everything so it could be passed to logging functions is not a viable
option.

About the device read errors, I am ambivalent what exactly to report as
log_error. It makes sense to only report first error per device, but it
also makes sense to report all unique errors per device. The global
log_error_once approach would give the latter, which I think may be
actually preferable.

Opinions?

Yours,
   Petr.

Tue Apr 27 16:52:50 CEST 2010  Petr Rockai <me mornfall net>
  * Reduce log noise about missing devices.
diff -rN -u -p old-upstream/lib/device/device.h new-upstream/lib/device/device.h
--- old-upstream/lib/device/device.h	2010-04-27 23:20:04.000000000 +0200
+++ new-upstream/lib/device/device.h	2010-04-27 23:20:04.000000000 +0200
@@ -46,7 +46,8 @@ struct device {
 	struct dm_list open_list;
 
 	char pvid[ID_LEN + 1];
-	char _padding[7];
+	int errors_reported;
+	char _padding[3];
 };
 
 struct device_list {
diff -rN -u -p old-upstream/lib/device/dev-io.c new-upstream/lib/device/dev-io.c
--- old-upstream/lib/device/dev-io.c	2010-04-27 23:20:04.000000000 +0200
+++ new-upstream/lib/device/dev-io.c	2010-04-27 23:20:04.000000000 +0200
@@ -94,13 +94,16 @@ static int _io(struct device_area *where
 			    read(fd, buffer, (size_t) where->size - total);
 		while ((n < 0) && ((errno == EINTR) || (errno == EAGAIN)));
 
-		if (n < 0)
-			log_error("%s: %s failed after %" PRIu64 " of %" PRIu64
-				  " at %" PRIu64 ": %s", dev_name(where->dev),
-				  should_write ? "write" : "read",
-				  (uint64_t) total,
-				  (uint64_t) where->size,
-				  (uint64_t) where->start, strerror(errno));
+		if (n < 0) {
+			log_(where->dev->errors_reported ? _LOG_NOTICE : _LOG_ERR,
+			     "%s: %s failed after %" PRIu64 " of %" PRIu64
+			     " at %" PRIu64 ": %s", dev_name(where->dev),
+			     should_write ? "write" : "read",
+			     (uint64_t) total,
+			     (uint64_t) where->size,
+			     (uint64_t) where->start, strerror(errno));
+			++ where->dev->errors_reported;
+		}
 
 		if (n <= 0)
 			break;
diff -rN -u -p old-upstream/lib/format_text/import_vsn1.c new-upstream/lib/format_text/import_vsn1.c
--- old-upstream/lib/format_text/import_vsn1.c	2010-04-27 23:20:04.000000000 +0200
+++ new-upstream/lib/format_text/import_vsn1.c	2010-04-27 23:20:04.000000000 +0200
@@ -153,6 +153,19 @@ static int _read_flag_config(struct conf
 	return 1;
 }
 
+
+static void report_uuid_missing(int sev, char *uuid) {
+	static struct dm_hash_table *reported = NULL;
+	if (!reported) {
+		reported = dm_hash_create(128);
+	}
+	if (dm_hash_lookup(reported, uuid))
+		return;
+
+	dm_hash_insert(reported, uuid, (void*)1);
+	log_(sev, "Coludn't find device with uuid %s.", uuid);
+}
+
 static int _read_pv(struct format_instance *fid, struct dm_pool *mem,
 		    struct volume_group *vg, struct config_node *pvn,
 		    struct config_node *vgn __attribute((unused)),
@@ -197,10 +210,7 @@ static int _read_pv(struct format_instan
 
 		if (!id_write_format(&pv->id, buffer, sizeof(buffer)))
 			buffer[0] = '\0';
-		if (report_missing_devices)
-			log_error("Couldn't find device with uuid %s.", buffer);
-		else
-			log_very_verbose("Couldn't find device with uuid %s.", buffer);
+		report_uuid_missing(report_missing_devices ? _LOG_ERR : _LOG_NOTICE, buffer);
 	}
 
 	if (!(pv->vg_name = dm_pool_strdup(mem, vg->name)))
diff -rN -u -p old-upstream/lib/log/log.h new-upstream/lib/log/log.h
--- old-upstream/lib/log/log.h	2010-04-27 23:20:04.000000000 +0200
+++ new-upstream/lib/log/log.h	2010-04-27 23:20:04.000000000 +0200
@@ -63,6 +63,7 @@
 #define log_err(x...) LOG_LINE_WITH_ERRNO(_LOG_ERR, EUNCLASSIFIED, x)
 #define log_err_suppress(s, x...) LOG_LINE_WITH_ERRNO(s ? _LOG_NOTICE : _LOG_ERR, EUNCLASSIFIED, x)
 #define log_fatal(x...) LOG_LINE_WITH_ERRNO(_LOG_FATAL, EUNCLASSIFIED, x)
+#define log_(sev, x...) LOG_LINE_WITH_ERRNO(sev, EUNCLASSIFIED, x)
 
 #define stack log_debug("<backtrace>")	/* Backtrace on error */
 #define log_very_verbose(args...) log_info(args)

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]