LVM2 ./WHATS_NEW_DM libdm/libdm-deptree.c

LVM2 ./WHATS_NEW_DM libdm/libdm-deptree.c

[zkabelac]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac at sourceware.org	2008-12-11 16:25:51

Modified files:
	.              : WHATS_NEW_DM 
	libdm          : libdm-deptree.c 

Log message:
	Replace _dm_snprintf with EMIT_PARAMS macro for creating target lines

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW_DM.diff?cvsroot=lvm2&r1=1.257&r2=1.258
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/libdm-deptree.c.diff?cvsroot=lvm2&r1=1.47&r2=1.48

--- LVM2/WHATS_NEW_DM	2008/11/19 14:14:52	1.257
+++ LVM2/WHATS_NEW_DM	2008/12/11 16:25:51	1.258
@@ -1,5 +1,6 @@
 Version 1.02.30 -
 ====================================
+  Replace _dm_snprintf with EMIT_PARAMS macro for creating target lines
 
 Version 1.02.29 - 10th November 2008
 ====================================
--- LVM2/libdm/libdm-deptree.c	2008/11/04 15:07:45	1.47
+++ LVM2/libdm/libdm-deptree.c	2008/12/11 16:25:51	1.48
@@ -135,22 +135,6 @@
 	int no_flush;		/* 1 sets noflush (mirrors/multipath) */
 };
 
-/* FIXME Consider exporting this */
-static int _dm_snprintf(char *buf, size_t bufsize, const char *format, ...)
-{
-        int n;
-        va_list ap;
-
-        va_start(ap, format);
-        n = vsnprintf(buf, bufsize, format, ap);
-        va_end(ap);
-
-        if (n < 0 || (n > (int) bufsize - 1))
-                return -1;
-
-        return n;
-}
-
 struct dm_tree *dm_tree_create(void)
 {
 	struct dm_tree *dtree;
@@ -1238,27 +1222,32 @@
 	return 1;
 }
 
+/* simplify string emiting code */
+#define EMIT_PARAMS(p, str...)\
+	do {\
+		const size_t bufsize = paramsize - (size_t)p;\
+		int w;\
+		\
+		if ((w = snprintf(params + p, bufsize, str)) < 0\
+		    || ((size_t)w >= bufsize)) {\
+			stack; /* Out of space */\
+			return -1;\
+		}\
+		p += w;\
+	} while (0)
+
 static int _emit_areas_line(struct dm_task *dmt __attribute((unused)),
 			    struct load_segment *seg, char *params,
 			    size_t paramsize, int *pos)
 {
 	struct seg_area *area;
 	char devbuf[DM_FORMAT_DEV_BUFSIZE];
-	int tw;
-	const char *prefix = "";
 
 	dm_list_iterate_items(area, &seg->areas) {
 		if (!_build_dev_string(devbuf, sizeof(devbuf), area->dev_node))
 			return_0;
 
-		if ((tw = _dm_snprintf(params + *pos, paramsize - *pos, "%s%s %" PRIu64,
-					prefix, devbuf, area->offset)) < 0) {
-                        stack;	/* Out of space */
-                        return -1;
-                }
-
-		prefix = " ";
-		*pos += tw;
+		EMIT_PARAMS(*pos, " %s %" PRIu64, devbuf, area->offset);
 	}
 
 	return 1;
@@ -1267,9 +1256,8 @@
 static int _emit_segment_line(struct dm_task *dmt, struct load_segment *seg, uint64_t *seg_start, char *params, size_t paramsize)
 {
 	unsigned log_parm_count;
-        int pos = 0;
-	int tw;
-        int r;
+	int pos = 0;
+	int r;
 	char originbuf[DM_FORMAT_DEV_BUFSIZE], cowbuf[DM_FORMAT_DEV_BUFSIZE];
 	char logbuf[DM_FORMAT_DEV_BUFSIZE];
 	const char *logtype;
@@ -1289,11 +1277,7 @@
 		if (seg->clustered) {
 			if (seg->uuid)
 				log_parm_count++;
-			if ((tw = _dm_snprintf(params + pos, paramsize - pos, "clustered-")) < 0) {
-                        	stack;	/* Out of space */
-                        	return -1;
-                	}
-			pos += tw;
+			EMIT_PARAMS(pos, "clustered-");
 		}
 
 		if (!seg->log)
@@ -1305,61 +1289,25 @@
 				return_0;
 		}
 
-		if ((tw = _dm_snprintf(params + pos, paramsize - pos, "%s %u ", logtype, log_parm_count)) < 0) {
-                        stack;	/* Out of space */
-                        return -1;
-                }
-		pos += tw;
-
-		if (seg->log) {
-			if ((tw = _dm_snprintf(params + pos, paramsize - pos, "%s ", logbuf)) < 0) {
-                        	stack;	/* Out of space */
-                        	return -1;
-                	}
-			pos += tw;
-		}
-
-		if ((tw = _dm_snprintf(params + pos, paramsize - pos, "%u ", seg->region_size)) < 0) {
-                       	stack; /* Out of space */
-                       	return -1;
-               	}
-		pos += tw;
-
-		if (seg->clustered && seg->uuid) {
-			if ((tw = _dm_snprintf(params + pos, paramsize - pos, "%s ", seg->uuid)) < 0) {
-				stack;  /* Out of space */
-				return -1;
-			}
-			pos += tw;
-		}
+		EMIT_PARAMS(pos, "%s %u", logtype, log_parm_count);
+
+		if (seg->log)
+			EMIT_PARAMS(pos, " %s", logbuf);
+
+		EMIT_PARAMS(pos, " %u", seg->region_size);
 
-		if ((seg->flags & DM_NOSYNC)) {
-			if ((tw = _dm_snprintf(params + pos, paramsize - pos, "nosync ")) < 0) {
-                       		stack; /* Out of space */
-                       		return -1;
-               		}
-			pos += tw;
-		} else if ((seg->flags & DM_FORCESYNC)) {
-			if ((tw = _dm_snprintf(params + pos, paramsize - pos, "sync ")) < 0) {
-                       		stack; /* Out of space */
-                       		return -1;
-               		}
-			pos += tw;
-		}
-
-		if ((seg->flags & DM_BLOCK_ON_ERROR)) {
-			if ((tw = _dm_snprintf(params + pos, paramsize - pos, "block_on_error ")) < 0) {
-                       		stack; /* Out of space */
-                       		return -1;
-               		}
-			pos += tw;
-		}
-
-		if ((tw = _dm_snprintf(params + pos, paramsize - pos, "%u ", seg->mirror_area_count)) < 0) {
-                       	stack; /* Out of space */
-                       	return -1;
-               	}
-		pos += tw;
+		if (seg->clustered && seg->uuid)
+			EMIT_PARAMS(pos, " %s", seg->uuid);
+
+		if ((seg->flags & DM_NOSYNC))
+			EMIT_PARAMS(pos, " nosync");
+		else if ((seg->flags & DM_FORCESYNC))
+			EMIT_PARAMS(pos, " sync");
+
+		if ((seg->flags & DM_BLOCK_ON_ERROR))
+			EMIT_PARAMS(pos, " block_on_error");
+
+		EMIT_PARAMS(pos, " %u", seg->mirror_area_count);
 
 		break;
 	case SEG_SNAPSHOT:
@@ -1367,30 +1315,16 @@
 			return_0;
 		if (!_build_dev_string(cowbuf, sizeof(cowbuf), seg->cow))
 			return_0;
-		if ((pos = _dm_snprintf(params, paramsize, "%s %s %c %d",
-                                        originbuf, cowbuf,
-					seg->persistent ? 'P' : 'N',
-                                        seg->chunk_size)) < 0) {
-                        stack;	/* Out of space */
-                        return -1;
-                }
+		EMIT_PARAMS(pos, "%s %s %c %d", originbuf, cowbuf,
+			    seg->persistent ? 'P' : 'N', seg->chunk_size);
 		break;
 	case SEG_SNAPSHOT_ORIGIN:
 		if (!_build_dev_string(originbuf, sizeof(originbuf), seg->origin))
 			return_0;
-		if ((pos = _dm_snprintf(params, paramsize, "%s",
-                                        originbuf)) < 0) {
-                        stack;	/* Out of space */
-                        return -1;
-                }
+		EMIT_PARAMS(pos, "%s", originbuf);
 		break;
 	case SEG_STRIPED:
-		if ((pos = _dm_snprintf(params, paramsize, "%u %u ",
-                                         seg->area_count,
-                                         seg->stripe_size)) < 0) {
-                        stack;	/* Out of space */
-                        return -1;
-                }
+		EMIT_PARAMS(pos, "%u %u", seg->area_count, seg->stripe_size);
 		break;
 	}
 
@@ -1421,6 +1355,8 @@
 	return 1;
 }
 
+#undef EMIT_PARAMS
+
 static int _emit_segment(struct dm_task *dmt, struct load_segment *seg,
 			 uint64_t *seg_start)
 {

Re: LVM2 ./WHATS_NEW_DM libdm/libdm-deptree.c

[Dave Wysochanski]

> +/* simplify string emiting code */
> +#define EMIT_PARAMS(p, str...)\
> +	do {\
> +		const size_t bufsize = paramsize - (size_t)p;\
> +		int w;\
> +		\
> +		if ((w = snprintf(params + p, bufsize, str)) < 0\
> +		    || ((size_t)w >= bufsize)) {\
> +			stack; /* Out of space */\
> +			return -1;\
> +		}\
> +		p += w;\
> +	} while (0)
> +

Do we have to do a macro here?  Macros like this are harder to debug...

LVM2 ./WHATS_NEW_DM libdm/libdm-deptree.c

[Alasdair G Kergon]

On Thu, Dec 11, 2008 at 04:25:52PM -0000, zkabelac at sourceware.org wrote:
> Changes by:	zkabelac at sourceware.org	2008-12-11 16:25:51
> Modified files:
> 	.              : WHATS_NEW_DM 
> 	libdm          : libdm-deptree.c 
> 
> Log message:
> 	Replace _dm_snprintf with EMIT_PARAMS macro for creating target lines
 

> +++ LVM2/libdm/libdm-deptree.c	2008/12/11 16:25:51	1.48
> -/* FIXME Consider exporting this */
> -static int _dm_snprintf(char *buf, size_t bufsize, const char *format, ...)

We already did export it in libdm-string.c!

> +/* simplify string emiting code */
> +#define EMIT_PARAMS(p, str...)\
> +	do {\
> +		const size_t bufsize = paramsize - (size_t)p;\
> +		int w;\
> +		\
> +		if ((w = snprintf(params + p, bufsize, str)) < 0\
> +		    || ((size_t)w >= bufsize)) {\

Can we use dm_snprintf() here now?
Also, a style point, we put || on the end of the previous line.

>  			    struct load_segment *seg, char *params,
>  			    size_t paramsize, int *pos)
>  {
>  	struct seg_area *area;
>  	char devbuf[DM_FORMAT_DEV_BUFSIZE];
> -	int tw;
> -	const char *prefix = "";
>  
>  	dm_list_iterate_items(area, &seg->areas) {
>  		if (!_build_dev_string(devbuf, sizeof(devbuf), area->dev_node))
>  			return_0;
>  
> -		if ((tw = _dm_snprintf(params + *pos, paramsize - *pos, "%s%s %" PRIu64,
> -					prefix, devbuf, area->offset)) < 0) {
> -                        stack;	/* Out of space */
> -                        return -1;
> -                }
> -
> -		prefix = " ";
> -		*pos += tw;
> +		EMIT_PARAMS(*pos, " %s %" PRIu64, devbuf, area->offset);

Has this changed the prefix behaviour by adding a space to the first one or
does that cancel out somewhere else?
(Any changes to whitespace require thorough testing - there is code that
relies on comparing these lines as strings against equivalent lines generated
in-kernel.)

Alasdair
-- 
agk at redhat.com

Re: LVM2 ./WHATS_NEW_DM libdm/libdm-deptree.c

[Alasdair G Kergon]

On Thu, Dec 11, 2008 at 11:35:23AM -0500, Dave Wysochanski wrote:
> 
> > +/* simplify string emiting code */
> > +#define EMIT_PARAMS(p, str...)\
> > +	do {\

> Do we have to do a macro here?  Macros like this are harder to debug...

Indeed - but sometimes there's stuff that's just too awkward to do
as a function.  I don't mind this one - we have a similar one in the kernel
called DMEMIT - so long as it is correct:-)
 
Alasdair
-- 
agk at redhat.com

Re: LVM2 ./WHATS_NEW_DM libdm/libdm-deptree.c

[Zdenek Kabelac]

Dave Wysochanski napsal(a):
>> +/* simplify string emiting code */
>> +#define EMIT_PARAMS(p, str...)\
>> +	do {\
>> +		const size_t bufsize = paramsize - (size_t)p;\
>> +		int w;\
>> +		\
>> +		if ((w = snprintf(params + p, bufsize, str)) < 0\
>> +		    || ((size_t)w >= bufsize)) {\
>> +			stack; /* Out of space */\
>> +			return -1;\
>> +		}\
>> +		p += w;\
>> +	} while (0)
>> +
> 
> Do we have to do a macro here?  Macros like this are harder to debug...
> 


I think it's actually minimizing the chance you will add a buggy code by some 
cut&paste operation when you add new string emitting line.

Also it makes the code more readable.

Do you think there is some potential debug problem in this code, that makes
worth to keep the original emitting style?

(i.e. replicator emits string 11 times - without this macro it makes the code 
even less readable,  and also using inline function and 'stack' macro is not 
going to work together, so it's hard to use gdb-friendly inline here :( )

Zdenek

Re: LVM2 ./WHATS_NEW_DM libdm/libdm-deptree.c

[Dave Wysochanski]

On Fri, 2008-12-12 at 10:26 +0100, Zdenek Kabelac wrote:
> Dave Wysochanski napsal(a):
> >> +/* simplify string emiting code */
> >> +#define EMIT_PARAMS(p, str...)\
> >> +	do {\
> >> +		const size_t bufsize = paramsize - (size_t)p;\
> >> +		int w;\
> >> +		\
> >> +		if ((w = snprintf(params + p, bufsize, str)) < 0\
> >> +		    || ((size_t)w >= bufsize)) {\
> >> +			stack; /* Out of space */\
> >> +			return -1;\
> >> +		}\
> >> +		p += w;\
> >> +	} while (0)
> >> +
> > 
> > Do we have to do a macro here?  Macros like this are harder to debug...
> > 
> 
> 
> I think it's actually minimizing the chance you will add a buggy code by some 
> cut&paste operation when you add new string emitting line.
> 
> Also it makes the code more readable.
> 
> Do you think there is some potential debug problem in this code, that makes
> worth to keep the original emitting style?
> 
> (i.e. replicator emits string 11 times - without this macro it makes the code 
> even less readable,  and also using inline function and 'stack' macro is not 
> going to work together, so it's hard to use gdb-friendly inline here :( )
> 

I agree something like this makes the code more readable - I was not
suggesting keeping the original code.  Was just wondering why you chose
the macro vs a regular function.  Is performance very critical here?
Something to do with using va_list?  Is there another reason for a macro
I'm missing?

Re: LVM2 ./WHATS_NEW_DM libdm/libdm-deptree.c

[Zdenek Kabelac]

Dave Wysochanski napsal(a):
> On Fri, 2008-12-12 at 10:26 +0100, Zdenek Kabelac wrote:
>> Dave Wysochanski napsal(a):
>>>> +/* simplify string emiting code */
>>>> +#define EMIT_PARAMS(p, str...)\
>>>> +	do {\
>>>> +		const size_t bufsize = paramsize - (size_t)p;\
>>>> +		int w;\
>>>> +		\
>>>> +		if ((w = snprintf(params + p, bufsize, str)) < 0\
>>>> +		    || ((size_t)w >= bufsize)) {\
>>>> +			stack; /* Out of space */\
>>>> +			return -1;\
>>>> +		}\
>>>> +		p += w;\
>>>> +	} while (0)
>>>> +
>>> Do we have to do a macro here?  Macros like this are harder to debug...
>>>
>>
>> I think it's actually minimizing the chance you will add a buggy code by some 
>> cut&paste operation when you add new string emitting line.
>>
>> Also it makes the code more readable.
>>
>> Do you think there is some potential debug problem in this code, that makes
>> worth to keep the original emitting style?
>>
>> (i.e. replicator emits string 11 times - without this macro it makes the code 
>> even less readable,  and also using inline function and 'stack' macro is not 
>> going to work together, so it's hard to use gdb-friendly inline here :( )
>>
> 
> I agree something like this makes the code more readable - I was not
> suggesting keeping the original code.  Was just wondering why you chose
> the macro vs a regular function.  Is performance very critical here?
> Something to do with using va_list?  Is there another reason for a macro
> I'm missing?


Inline function would not resolve the 'if (emit() < 0) { stack; return -1;}' 
repeating part of the code. So with inline we would have actually saved only 
'p += w;' line

Eventually we could use macro 'IF_MEM_OK(emit())' to only hide 'stack; return' 
and keep emit() as a static incline function, but then there is no big victory 
against the current EMIT_PARAMS macro.

Zdenek

LVM2 ./WHATS_NEW_DM libdm/libdm-deptree.c

[agk]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	agk at sourceware.org	2009-07-03 12:45:56

Modified files:
	.              : WHATS_NEW_DM 
	libdm          : libdm-deptree.c 

Log message:
	Add device number to more log messages during activation.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW_DM.diff?cvsroot=lvm2&r1=1.282&r2=1.283
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/libdm-deptree.c.diff?cvsroot=lvm2&r1=1.51&r2=1.52

--- LVM2/WHATS_NEW_DM	2009/07/01 09:31:46	1.282
+++ LVM2/WHATS_NEW_DM	2009/07/03 12:45:55	1.283
@@ -1,5 +1,6 @@
 Version 1.02.34 - 
 ================================
+  Add device number to more log messages during activation.
 
 Version 1.02.33 - 30th June 2009
 ================================
--- LVM2/libdm/libdm-deptree.c	2009/06/09 16:10:25	1.51
+++ LVM2/libdm/libdm-deptree.c	2009/07/03 12:45:56	1.52
@@ -1263,7 +1263,10 @@
 	return 1;
 }
 
-static int _emit_segment_line(struct dm_task *dmt, struct load_segment *seg, uint64_t *seg_start, char *params, size_t paramsize)
+static int _emit_segment_line(struct dm_task *dmt, uint32_t major,
+			      uint32_t minor, struct load_segment *seg,
+			      uint64_t *seg_start, char *params,
+			      size_t paramsize)
 {
 	unsigned log_parm_count;
 	int pos = 0;
@@ -1362,7 +1365,8 @@
 		break;
 	}
 
-	log_debug("Adding target: %" PRIu64 " %" PRIu64 " %s %s",
+	log_debug("Adding target to (%" PRIu32 ":%" PRIu32 "): %" PRIu64
+		  " %" PRIu64 " %s %s", major, minor,
 		  *seg_start, seg->size, dm_segtypes[seg->type].target, params);
 
 	if (!dm_task_add_target(dmt, *seg_start, seg->size, dm_segtypes[seg->type].target, params))
@@ -1375,8 +1379,8 @@
 
 #undef EMIT_PARAMS
 
-static int _emit_segment(struct dm_task *dmt, struct load_segment *seg,
-			 uint64_t *seg_start)
+static int _emit_segment(struct dm_task *dmt, uint32_t major, uint32_t minor,
+			 struct load_segment *seg, uint64_t *seg_start)
 {
 	char *params;
 	size_t paramsize = 4096;
@@ -1389,7 +1393,8 @@
 		}
 
 		params[0] = '\0';
-		ret = _emit_segment_line(dmt, seg, seg_start, params, paramsize);
+		ret = _emit_segment_line(dmt, major, minor, seg, seg_start,
+					 params, paramsize);
 		dm_free(params);
 
 		if (!ret)
@@ -1415,7 +1420,8 @@
 	struct load_segment *seg;
 	uint64_t seg_start = 0;
 
-	log_verbose("Loading %s table", dnode->name);
+	log_verbose("Loading %s table (%" PRIu32 ":%" PRIu32 ")", dnode->name,
+		    dnode->info.major, dnode->info.minor);
 
 	if (!(dmt = dm_task_create(DM_DEVICE_RELOAD))) {
 		log_error("Reload dm_task creation failed for %s", dnode->name);
@@ -1437,7 +1443,8 @@
 		log_error("Failed to disable open_count");
 
 	dm_list_iterate_items(seg, &dnode->props.segs)
-		if (!_emit_segment(dmt, seg, &seg_start))
+		if (!_emit_segment(dmt, dnode->info.major, dnode->info.minor,
+				   seg, &seg_start))
 			goto_out;
 
 	if (!dm_task_suppress_identical_reload(dmt))

LVM2 ./WHATS_NEW_DM libdm/libdm-deptree.c

[zkabelac]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac at sourceware.org	2011-02-18 16:13:57

Modified files:
	.              : WHATS_NEW_DM 
	libdm          : libdm-deptree.c 

Log message:
	Add debug message for open_count failure
	
	Report  open_count problem as debug.
	
	Function using _node_has_closed_parents decides whether
	it's error or could be ignored.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW_DM.diff?cvsroot=lvm2&r1=1.447&r2=1.448
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/libdm-deptree.c.diff?cvsroot=lvm2&r1=1.89&r2=1.90

--- LVM2/WHATS_NEW_DM	2011/02/18 14:38:47	1.447
+++ LVM2/WHATS_NEW_DM	2011/02/18 16:13:56	1.448
@@ -1,5 +1,6 @@
 Version 1.02.64 - 
 ===================================
+  Log debug open_count in _node_has_closed_parents().
   Change dm_report_field_string() API to accept const char *const *data.
 
 Version 1.02.63 - 9th February 2011
--- LVM2/libdm/libdm-deptree.c	2010/11/29 12:42:10	1.89
+++ LVM2/libdm/libdm-deptree.c	2011/02/18 16:13:56	1.90
@@ -938,8 +938,11 @@
 		    !info.exists)
 			continue;
 
-		if (info.open_count)
+		if (info.open_count) {
+			log_debug("Node %s %d:%d has open_count %d", uuid_prefix,
+				  dinfo->major, dinfo->minor, info.open_count);
 			return 0;
+		}
 	}
 
 	return 1;

LVM2 ./WHATS_NEW_DM libdm/libdm-deptree.c

[zkabelac]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac at sourceware.org	2011-09-07 08:37:48

Modified files:
	.              : WHATS_NEW_DM 
	libdm          : libdm-deptree.c 

Log message:
	Remove unused passed parameters

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW_DM.diff?cvsroot=lvm2&r1=1.497&r2=1.498
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/libdm-deptree.c.diff?cvsroot=lvm2&r1=1.108&r2=1.109

--- LVM2/WHATS_NEW_DM	2011/09/01 21:04:14	1.497
+++ LVM2/WHATS_NEW_DM	2011/09/07 08:37:48	1.498
@@ -1,5 +1,6 @@
 Version 1.02.68 -
 ==================================
+  Remove unused passed parameters for _mirror_emit_segment_line().
   Add dm_config and string character escaping functions to libdevmapper.
   Mark unreleased memory pools as internal error.
 
--- LVM2/libdm/libdm-deptree.c	2011/08/19 17:02:48	1.108
+++ LVM2/libdm/libdm-deptree.c	2011/09/07 08:37:48	1.109
@@ -1594,10 +1594,8 @@
 /*
  * Returns: 1 on success, 0 on failure
  */
-static int _mirror_emit_segment_line(struct dm_task *dmt, uint32_t major,
-				     uint32_t minor, struct load_segment *seg,
-				     uint64_t *seg_start, char *params,
-				     size_t paramsize)
+static int _mirror_emit_segment_line(struct dm_task *dmt, struct load_segment *seg,
+				     char *params, size_t paramsize)
 {
 	int block_on_error = 0;
 	int handle_errors = 0;
@@ -1784,8 +1782,7 @@
 		break;
 	case SEG_MIRRORED:
 		/* Mirrors are pretty complicated - now in separate function */
-		r = _mirror_emit_segment_line(dmt, major, minor, seg, seg_start,
-					      params, paramsize);
+		r = _mirror_emit_segment_line(dmt, seg, params, paramsize);
 		if (!r)
 			return_0;
 		break;

LVM2 ./WHATS_NEW_DM libdm/libdm-deptree.c

[zkabelac]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac at sourceware.org	2011-10-03 18:28:26

Modified files:
	.              : WHATS_NEW_DM 
	libdm          : libdm-deptree.c 

Log message:
	Update error path tracing for _resume_node
	
	dm_task_create & dm_task_set_name produces it's own log_error
	Add missing stacks for dm_task_set_cookie, dm_task_run,
	dm_task_get_info.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW_DM.diff?cvsroot=lvm2&r1=1.508&r2=1.509
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/libdm-deptree.c.diff?cvsroot=lvm2&r1=1.116&r2=1.117

--- LVM2/WHATS_NEW_DM	2011/09/29 08:53:48	1.508
+++ LVM2/WHATS_NEW_DM	2011/10/03 18:28:25	1.509
@@ -1,5 +1,6 @@
 Version 1.02.68 -
 ==================================
+  Update debug logging for _resume_node.
   Add functions to support thin provisioning target (API unstable).
   Improve libdm-config error path reporting.
   Update dmsetup resume man with --addnodeonresume/create options.
--- LVM2/libdm/libdm-deptree.c	2011/10/03 18:26:07	1.116
+++ LVM2/libdm/libdm-deptree.c	2011/10/03 18:28:25	1.117
@@ -1127,13 +1127,13 @@
 	log_verbose("Resuming %s (%" PRIu32 ":%" PRIu32 ")", name, major, minor);
 
 	if (!(dmt = dm_task_create(DM_DEVICE_RESUME))) {
-		log_error("Suspend dm_task creation failed for %s", name);
+		log_debug("Suspend dm_task creation failed for %s.", name);
 		return 0;
 	}
 
 	/* FIXME Kernel should fill in name on return instead */
 	if (!dm_task_set_name(dmt, name)) {
-		log_error("Failed to set readahead device name for %s", name);
+		log_debug("Failed to set device name for %s resumption.", name);
 		goto out;
 	}
 
@@ -1149,13 +1149,16 @@
 		log_error("Failed to set read ahead");
 
 	if (!dm_task_set_cookie(dmt, cookie, udev_flags))
-		goto out;
+		goto_out;
 
-	if ((r = dm_task_run(dmt))) {
-		if (already_suspended)
-			dec_suspended();
-		r = dm_task_get_info(dmt, newinfo);
-	}
+	if (!(r = dm_task_run(dmt)))
+		goto_out;
+
+	if (already_suspended)
+		dec_suspended();
+
+	if (!(r = dm_task_get_info(dmt, newinfo)))
+		stack;
 
 out:
 	dm_task_destroy(dmt);

LVM2 ./WHATS_NEW_DM libdm/libdm-deptree.c

[zkabelac]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac at sourceware.org	2011-10-14 13:34:19

Modified files:
	.              : WHATS_NEW_DM 
	libdm          : libdm-deptree.c 

Log message:
	Use pool for dm_tree allocation
	
	Using the same pool allocation strategy as we use for vg,
	so dm_tree structure is part of the pool itself.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW_DM.diff?cvsroot=lvm2&r1=1.509&r2=1.510
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/libdm-deptree.c.diff?cvsroot=lvm2&r1=1.122&r2=1.123

--- LVM2/WHATS_NEW_DM	2011/10/03 18:28:25	1.509
+++ LVM2/WHATS_NEW_DM	2011/10/14 13:34:19	1.510
@@ -1,5 +1,6 @@
 Version 1.02.68 -
 ==================================
+  Allocate dm_tree structure from dm_tree pool.
   Update debug logging for _resume_node.
   Add functions to support thin provisioning target (API unstable).
   Improve libdm-config error path reporting.
--- LVM2/libdm/libdm-deptree.c	2011/10/06 14:45:42	1.122
+++ LVM2/libdm/libdm-deptree.c	2011/10/14 13:34:19	1.123
@@ -248,10 +248,14 @@
 
 struct dm_tree *dm_tree_create(void)
 {
+	struct dm_pool *dmem;
 	struct dm_tree *dtree;
 
-	if (!(dtree = dm_zalloc(sizeof(*dtree)))) {
-		log_error("dm_tree_create malloc failed");
+	if (!(dmem = dm_pool_create("dtree", 1024)) ||
+	    !(dtree = dm_pool_zalloc(dmem, sizeof(*dtree)))) {
+		log_error("Failed to allocate dtree.");
+		if (dmem)
+			dm_pool_destroy(dmem);
 		return NULL;
 	}
 
@@ -260,17 +264,11 @@
 	dm_list_init(&dtree->root.used_by);
 	dtree->skip_lockfs = 0;
 	dtree->no_flush = 0;
-
-	if (!(dtree->mem = dm_pool_create("dtree", 1024))) {
-		log_error("dtree pool creation failed");
-		dm_free(dtree);
-		return NULL;
-	}
+	dtree->mem = dmem;
 
 	if (!(dtree->devs = dm_hash_create(8))) {
 		log_error("dtree hash creation failed");
 		dm_pool_destroy(dtree->mem);
-		dm_free(dtree);
 		return NULL;
 	}
 
@@ -278,7 +276,6 @@
 		log_error("dtree uuid hash creation failed");
 		dm_hash_destroy(dtree->devs);
 		dm_pool_destroy(dtree->mem);
-		dm_free(dtree);
 		return NULL;
 	}
 
@@ -293,7 +290,6 @@
 	dm_hash_destroy(dtree->uuids);
 	dm_hash_destroy(dtree->devs);
 	dm_pool_destroy(dtree->mem);
-	dm_free(dtree);
 }
 
 static int _nodes_are_linked(const struct dm_tree_node *parent,

LVM2 ./WHATS_NEW_DM libdm/libdm-deptree.c

[mbroz]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mbroz at sourceware.org	2011-10-17 13:15:35

Modified files:
	.              : WHATS_NEW_DM 
	libdm          : libdm-deptree.c 

Log message:
	Fix alignment warning in bitcount calculation for raid segment.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW_DM.diff?cvsroot=lvm2&r1=1.510&r2=1.511
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/libdm-deptree.c.diff?cvsroot=lvm2&r1=1.123&r2=1.124

--- LVM2/WHATS_NEW_DM	2011/10/14 13:34:19	1.510
+++ LVM2/WHATS_NEW_DM	2011/10/17 13:15:35	1.511
@@ -1,5 +1,6 @@
 Version 1.02.68 -
 ==================================
+  Fix alignment warning in bitcount calculation for raid segment.
   Allocate dm_tree structure from dm_tree pool.
   Update debug logging for _resume_node.
   Add functions to support thin provisioning target (API unstable).
--- LVM2/libdm/libdm-deptree.c	2011/10/14 13:34:19	1.123
+++ LVM2/libdm/libdm-deptree.c	2011/10/17 13:15:35	1.124
@@ -1856,7 +1856,7 @@
 				   uint64_t *seg_start, char *params,
 				   size_t paramsize)
 {
-	uint32_t i, *tmp;
+	uint32_t i;
 	int param_count = 1; /* mandatory 'chunk size'/'stripe size' arg */
 	int pos = 0;
 
@@ -1866,9 +1866,9 @@
 	if (seg->region_size)
 		param_count += 2;
 
-	tmp = (uint32_t *)(&seg->rebuilds); /* rebuilds is 64-bit */
-	param_count += 2 * hweight32(tmp[0]);
-	param_count += 2 * hweight32(tmp[1]);
+	/* rebuilds is 64-bit */
+	param_count += 2 * hweight32(seg->rebuilds & 0xFFFFFFFF);
+	param_count += 2 * hweight32(seg->rebuilds >> 32);
 
 	if ((seg->type == SEG_RAID1) && seg->stripe_size)
 		log_error("WARNING: Ignoring RAID1 stripe size");

LVM2 ./WHATS_NEW_DM libdm/libdm-deptree.c

[zkabelac]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac at sourceware.org	2012-01-25 21:50:51

Modified files:
	.              : WHATS_NEW_DM 
	libdm          : libdm-deptree.c 

Log message:
	Ensure whole info is initialised
	
	Since _create_dm_tree_node is copying whole structure,
	make sure all members are initialized.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW_DM.diff?cvsroot=lvm2&r1=1.532&r2=1.533
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/libdm-deptree.c.diff?cvsroot=lvm2&r1=1.152&r2=1.153

--- LVM2/WHATS_NEW_DM	2012/01/18 18:52:02	1.532
+++ LVM2/WHATS_NEW_DM	2012/01/25 21:50:50	1.533
@@ -1,5 +1,6 @@
 Version 1.02.68 -
 ==================================
+  Reset all members of info struct in dm_tree_add_new_dev_with_udev_flags.
   Add dmsetup wipe_table to replace table with one that uses error target.
   Add 'blkdevname' and 'blkdevs_used' field to dmsetup info -c -o.
   Add 'blkdevname' option to dmsetup ls --tree to see block device names.
--- LVM2/libdm/libdm-deptree.c	2012/01/25 08:46:21	1.152
+++ LVM2/libdm/libdm-deptree.c	2012/01/25 21:50:51	1.153
@@ -1101,12 +1101,7 @@
 			return NULL;
 		}
 
-		info.major = 0;
-		info.minor = 0;
-		info.exists = 0;
-		info.live_table = 0;
-		info.inactive_table = 0;
-		info.read_only = 0;
+		memset(&info, 0, sizeof(info));
 
 		if (!(dnode = _create_dm_tree_node(dtree, name2, uuid2, &info,
 						   context, 0)))

LVM2 ./WHATS_NEW_DM libdm/libdm-deptree.c

[zkabelac]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac at sourceware.org	2012-02-10 14:42:29

Modified files:
	.              : WHATS_NEW_DM 
	libdm          : libdm-deptree.c 

Log message:
	Add validation of name and uuid
	
	Do not accept NULL pointers.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW_DM.diff?cvsroot=lvm2&r1=1.546&r2=1.547
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/libdm-deptree.c.diff?cvsroot=lvm2&r1=1.154&r2=1.155

--- LVM2/WHATS_NEW_DM	2012/02/10 14:00:07	1.546
+++ LVM2/WHATS_NEW_DM	2012/02/10 14:42:28	1.547
@@ -1,5 +1,6 @@
 Version 1.02.70 - 
 ===================================
+  Validate name and uuid params of dm_tree_add_new_dev_with_udev_flags().  
   Do not crash for dm_report_init() sort_key == NULL and behave like "".
   Return error for failing allocation in dm_asprintf().
   Add missing test for failing allocation in dm_realloc() code.
--- LVM2/libdm/libdm-deptree.c	2012/02/08 11:36:18	1.154
+++ LVM2/libdm/libdm-deptree.c	2012/02/10 14:42:28	1.155
@@ -1086,6 +1086,11 @@
 	const char *name2;
 	const char *uuid2;
 
+	if (!name || !uuid) {
+		log_error("Cannot add device without name and uuid.");
+		return NULL;
+	}
+
 	/* Do we need to add node to tree? */
 	if (!(dnode = dm_tree_find_node_by_uuid(dtree, uuid))) {
 		if (!(name2 = dm_pool_strdup(dtree->mem, name))) {

LVM2 ./WHATS_NEW_DM libdm/libdm-deptree.c

[zkabelac]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac at sourceware.org	2012-02-10 14:48:30

Modified files:
	.              : WHATS_NEW_DM 
	libdm          : libdm-deptree.c 

Log message:
	Check for deps pointer before dererence
	
	As _deps() call may return NULL - check for it.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW_DM.diff?cvsroot=lvm2&r1=1.547&r2=1.548
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/libdm-deptree.c.diff?cvsroot=lvm2&r1=1.155&r2=1.156

--- LVM2/WHATS_NEW_DM	2012/02/10 14:42:28	1.547
+++ LVM2/WHATS_NEW_DM	2012/02/10 14:48:28	1.548
@@ -1,6 +1,7 @@
 Version 1.02.70 - 
 ===================================
-  Validate name and uuid params of dm_tree_add_new_dev_with_udev_flags().  
+  Add pointer test for dependency check in _add_dev().
+  Validate name and uuid params of dm_tree_add_new_dev_with_udev_flags().
   Do not crash for dm_report_init() sort_key == NULL and behave like "".
   Return error for failing allocation in dm_asprintf().
   Add missing test for failing allocation in dm_realloc() code.
--- LVM2/libdm/libdm-deptree.c	2012/02/10 14:42:28	1.155
+++ LVM2/libdm/libdm-deptree.c	2012/02/10 14:48:28	1.156
@@ -1180,7 +1180,7 @@
 		goto out;
 
 	/* Can't recurse if not a mapped device or there are no dependencies */
-	if (!node->info.exists || !deps->count) {
+	if (!node->info.exists || !deps || !deps->count) {
 		if (!_add_to_bottomlevel(node)) {
 			stack;
 			node = NULL;

LVM2 ./WHATS_NEW_DM libdm/libdm-deptree.c

[agk]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	agk at sourceware.org	2012-05-15 21:27:25

Modified files:
	.              : WHATS_NEW_DM 
	libdm          : libdm-deptree.c 

Log message:
	Set delay_resume_if_new on deptree snapshot origin.
	(Must avoid activating snapshot origin more than once concurrently.)

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/WHATS_NEW_DM.diff?cvsroot=lvm2&r1=1.601&r2=1.602
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/libdm-deptree.c.diff?cvsroot=lvm2&r1=1.165&r2=1.166

--- LVM2/WHATS_NEW_DM	2012/05/08 14:31:44	1.601
+++ LVM2/WHATS_NEW_DM	2012/05/15 21:27:24	1.602
@@ -1,5 +1,6 @@
 Version 1.02.75 - 
 ================================
+  Set delay_resume_if_new on deptree snapshot origin.
   Log value chosen in _find_config_bool like other variable types do.
   Synchronize with dead of dmeventd.
   Rename (Blk)DevNames/DevNos dmsetup header to (Blk)DevNamesUsed/DevNosUsed.
--- LVM2/libdm/libdm-deptree.c	2012/05/15 20:03:12	1.165
+++ LVM2/libdm/libdm-deptree.c	2012/05/15 21:27:24	1.166
@@ -2358,8 +2358,10 @@
 	if ((r = dm_task_run(dmt))) {
 		r = dm_task_get_info(dmt, &dnode->info);
 		if (r && !dnode->info.inactive_table)
-			log_verbose("Suppressed %s identical table reload.",
-				    dnode->name);
+			log_verbose("Suppressed %s (%" PRIu32 ":%" PRIu32
+				    ") identical table reload.",
+				    dnode->name,
+				    dnode->info.major, dnode->info.minor);
 
 		existing_table_size = dm_task_get_existing_table_size(dmt);
 		if ((dnode->props.size_changed =
@@ -2548,6 +2550,12 @@
 	/* Resume snapshot origins after new snapshots */
 	dnode->activation_priority = 1;
 
+	/*
+	 * Don't resume the origin immediately in case it is a non-trivial 
+	 * target that must not be active more than once concurrently!
+	 */
+	origin_node->props.delay_resume_if_new = 1;
+
 	return 1;
 }