LVM2/libdm/ioctl libdm-iface.c

LVM2/libdm/ioctl libdm-iface.c

[prajnoha]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	prajnoha at sourceware.org	2009-08-06 15:02:01

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	Fix failure situations in dm_task_run for udev sync.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/ioctl/libdm-iface.c.diff?cvsroot=lvm2&r1=1.60&r2=1.61

--- LVM2/libdm/ioctl/libdm-iface.c	2009/08/03 18:01:48	1.60
+++ LVM2/libdm/ioctl/libdm-iface.c	2009/08/06 15:02:01	1.61
@@ -1486,6 +1486,18 @@
 	return _process_all_v4(dmt);
 }
 
+/*
+ * If an operation that uses a cookie fails, decrement the
+ * semaphore instead of udev.
+ */
+static int _udev_complete(struct dm_task *dmt)
+{
+	if (dmt->cookie_set)
+		return dm_udev_complete(dmt->event_nr);
+
+	return 1;
+}
+
 static int _create_and_load_v4(struct dm_task *dmt)
 {
 	struct dm_task *task;
@@ -1494,17 +1506,20 @@
 	/* Use new task struct to create the device */
 	if (!(task = dm_task_create(DM_DEVICE_CREATE))) {
 		log_error("Failed to create device-mapper task struct");
+		_udev_complete(dmt);
 		return 0;
 	}
 
 	/* Copy across relevant fields */
 	if (dmt->dev_name && !dm_task_set_name(task, dmt->dev_name)) {
 		dm_task_destroy(task);
+		_udev_complete(dmt);
 		return 0;
 	}
 
 	if (dmt->uuid && !dm_task_set_uuid(task, dmt->uuid)) {
 		dm_task_destroy(task);
+		_udev_complete(dmt);
 		return 0;
 	}
 
@@ -1516,18 +1531,22 @@
 
 	r = dm_task_run(task);
 	dm_task_destroy(task);
-	if (!r)
-		return r;
+	if (!r) {
+		_udev_complete(dmt);
+		return 0;
+	}
 
 	/* Next load the table */
 	if (!(task = dm_task_create(DM_DEVICE_RELOAD))) {
 		log_error("Failed to create device-mapper task struct");
+		_udev_complete(dmt);
 		return 0;
 	}
 
 	/* Copy across relevant fields */
 	if (dmt->dev_name && !dm_task_set_name(task, dmt->dev_name)) {
 		dm_task_destroy(task);
+		_udev_complete(dmt);
 		return 0;
 	}
 
@@ -1540,8 +1559,10 @@
 	task->head = NULL;
 	task->tail = NULL;
 	dm_task_destroy(task);
-	if (!r)
+	if (!r) {
+		_udev_complete(dmt);
 		goto revert;
+	}
 
 	/* Use the original structure last so the info will be correct */
 	dmt->type = DM_DEVICE_RESUME;
@@ -1557,6 +1578,7 @@
  	dmt->type = DM_DEVICE_REMOVE;
 	dm_free(dmt->uuid);
 	dmt->uuid = NULL;
+	dmt->cookie_set = 0;
 
 	if (!dm_task_run(dmt))
 		log_error("Failed to revert device creation.");
@@ -1739,19 +1761,15 @@
 	if ((dmt->type == DM_DEVICE_RELOAD) && dmt->suppress_identical_reload)
 		return _reload_with_suppression_v4(dmt);
 
-	if (!_open_control())
+	if (!_open_control()) {
+		_udev_complete(dmt);
 		return 0;
+	}
 
 	/* FIXME Detect and warn if cookie set but should not be. */
 repeat_ioctl:
 	if (!(dmi = _do_dm_ioctl(dmt, command, _ioctl_buffer_double_factor))) {
-		/*
-		 * If an operation that uses a cookie fails, decrement the 
-		 * semaphore instead of udev.
-		 * FIXME Review error paths: found one where uevent fired too.
-		 */
-		if (dmt->cookie_set)
-			dm_udev_complete(dmt->event_nr);
+		_udev_complete(dmt);
 		return 0;
 	}
 

LVM2/libdm/ioctl libdm-iface.c

[prajnoha]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	prajnoha at sourceware.org	2010-05-03 22:08:39

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	Specify exactly which ioctl doesn't have a cookie set (for better debugging).

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/ioctl/libdm-iface.c.diff?cvsroot=lvm2&r1=1.72&r2=1.73

--- LVM2/libdm/ioctl/libdm-iface.c	2010/05/03 21:06:53	1.72
+++ LVM2/libdm/ioctl/libdm-iface.c	2010/05/03 22:08:38	1.73
@@ -1770,11 +1770,13 @@
 		 * libdevmapper's node and symlink creation code.
 		 */
 		if (!dmt->cookie_set && dm_udev_get_sync_support()) {
-			log_debug("Cookie value is not set while trying to call "
-				  "DM_DEVICE_RESUME, DM_DEVICE_REMOVE or DM_DEVICE_RENAME "
+			log_debug("Cookie value is not set while trying to call %s "
 				  "ioctl. Please, consider using libdevmapper's udev "
 				  "synchronisation interface or disable it explicitly "
-				  "by calling dm_udev_set_sync_support(0).");
+				  "by calling dm_udev_set_sync_support(0).",
+				  dmt->type == DM_DEVICE_RESUME ? "DM_DEVICE_RESUME" :
+				  dmt->type == DM_DEVICE_REMOVE ? "DM_DEVICE_REMOVE" :
+								  "DM_DEVICE_RENAME");
 			log_debug("Switching off device-mapper and all subsystem related "
 				  "udev rules. Falling back to libdevmapper node creation.");
 			/*

LVM2/libdm/ioctl libdm-iface.c

[prajnoha]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	prajnoha at sourceware.org	2010-07-28 10:30:29

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	Revert unsuccessful table load preparation in combined "create, load and resume" scenario.
	
	There was missing "revert" call in _create_and_load_v4 fn while the preparation
	for table load ends up with failure in create/load/resume sequence. Otherwise
	we could end up with a device being created, but not table-loaded nor resumed.
	
	Even though the table is not loaded and the device is not resumed at this
	stage, we still need to synchronize with udev when calling the revert
	"remove" ioctl - there's still a remove uevent generated! The "revert"
	code does exactly that.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/ioctl/libdm-iface.c.diff?cvsroot=lvm2&r1=1.76&r2=1.77

--- LVM2/libdm/ioctl/libdm-iface.c	2010/06/23 12:54:46	1.76
+++ LVM2/libdm/ioctl/libdm-iface.c	2010/07/28 10:30:28	1.77
@@ -1655,14 +1655,16 @@
 	if (!(task = dm_task_create(DM_DEVICE_RELOAD))) {
 		log_error("Failed to create device-mapper task struct");
 		_udev_complete(dmt);
-		return 0;
+		r = 0;
+		goto revert;
 	}
 
 	/* Copy across relevant fields */
 	if (dmt->dev_name && !dm_task_set_name(task, dmt->dev_name)) {
 		dm_task_destroy(task);
 		_udev_complete(dmt);
-		return 0;
+		r = 0;
+		goto revert;
 	}
 
 	task->read_only = dmt->read_only;

LVM2/libdm/ioctl libdm-iface.c

[prajnoha]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	prajnoha at sourceware.org	2010-08-16 11:13:19

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	dm-mod autoloading support is in kernel 2.6.36 actually.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/ioctl/libdm-iface.c.diff?cvsroot=lvm2&r1=1.79&r2=1.80

--- LVM2/libdm/ioctl/libdm-iface.c	2010/08/03 13:16:21	1.79
+++ LVM2/libdm/ioctl/libdm-iface.c	2010/08/16 11:13:18	1.80
@@ -392,7 +392,7 @@
 	 */
 	if (!_control_exists(control, 0, 0) ||
 	    !(KERNEL_VERSION(_kernel_major, _kernel_minor, _kernel_release) >=
-	      KERNEL_VERSION(2, 6, 35)) ||
+	      KERNEL_VERSION(2, 6, 36)) ||
 	    !_open_and_assign_control_fd(control)) {
 		if (!_control_device_number(&major, &minor))
 			log_error("Is device-mapper driver missing from kernel?");

LVM2/libdm/ioctl libdm-iface.c

[zkabelac]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac at sourceware.org	2010-11-30 22:32:45

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	Add error path stack traces
	
	Check for errors from dm_task_set_name() and dm_task_run().
	Add stack traces for error paths.
	Return 0 if some error is found.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/ioctl/libdm-iface.c.diff?cvsroot=lvm2&r1=1.83&r2=1.84

--- LVM2/libdm/ioctl/libdm-iface.c	2010/10/25 11:44:20	1.83
+++ LVM2/libdm/ioctl/libdm-iface.c	2010/11/30 22:32:44	1.84
@@ -1594,8 +1594,15 @@
 		    !strcmp(dirent->d_name, "..") ||
 		    !strcmp(dirent->d_name, "control"))
 			continue;
-		dm_task_set_name(dmt, dirent->d_name);
-		dm_task_run(dmt);
+		if (!dm_task_set_name(dmt, dirent->d_name)) {
+			r = 0;
+			stack;
+			continue; /* try next name */
+		}
+		if (!dm_task_run(dmt)) {
+			r = 0;
+			stack;  /* keep going */
+		}
 	}
 
 	if (closedir(d))

LVM2/libdm/ioctl libdm-iface.c

[prajnoha]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	prajnoha at sourceware.org	2011-01-31 11:54:55

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	Add a debug message when uevent is not generated and we call udev_complete internally.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/ioctl/libdm-iface.c.diff?cvsroot=lvm2&r1=1.88&r2=1.89

--- LVM2/libdm/ioctl/libdm-iface.c	2011/01/28 10:16:04	1.88
+++ LVM2/libdm/ioctl/libdm-iface.c	2011/01/31 11:54:55	1.89
@@ -1979,8 +1979,11 @@
 		}
 	}
 
-	if (ioctl_with_uevent && !_check_uevent_generated(dmi))
+	if (ioctl_with_uevent && !_check_uevent_generated(dmi)) {
+		log_debug("Uevent not generated! Calling udev_complete "
+			  "internally to avoid process lock-up.");
 		_udev_complete(dmt);
+	}
 
 #else /* Userspace alternative for testing */
 #endif

LVM2/libdm/ioctl libdm-iface.c

[agk]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	agk at sourceware.org	2011-02-04 21:26:33

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	Warn if secure data flag requested but not supported by kernel.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/ioctl/libdm-iface.c.diff?cvsroot=lvm2&r1=1.91&r2=1.92

--- LVM2/libdm/ioctl/libdm-iface.c	2011/02/04 19:33:53	1.91
+++ LVM2/libdm/ioctl/libdm-iface.c	2011/02/04 21:26:33	1.92
@@ -1541,8 +1541,12 @@
 		dmi->flags |= DM_READONLY_FLAG;
 	if (dmt->skip_lockfs)
 		dmi->flags |= DM_SKIP_LOCKFS_FLAG;
-	if (dmt->secure_data)
+	if (dmt->secure_data) {
+		if (_dm_version_minor < 20)
+			log_warn("WARNING: Secure data flag unsupported by "
+				 "kernel.  Buffers will not be wiped after use.");
 		dmi->flags |= DM_SECURE_DATA_FLAG;
+	}
 	if (dmt->query_inactive_table) {
 		if (_dm_version_minor < 16)
 			log_warn("WARNING: Inactive table query unsupported "

LVM2/libdm/ioctl libdm-iface.c

[zkabelac]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac at sourceware.org	2011-03-02 08:41:55

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	Indent case part properly

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/ioctl/libdm-iface.c.diff?cvsroot=lvm2&r1=1.96&r2=1.97

--- LVM2/libdm/ioctl/libdm-iface.c	2011/03/02 00:29:57	1.96
+++ LVM2/libdm/ioctl/libdm-iface.c	2011/03/02 08:41:55	1.97
@@ -2111,8 +2111,8 @@
 
 	switch (dmt->type) {
 	case DM_DEVICE_CREATE:
-	    if ((dmt->add_node == DM_ADD_NODE_ON_CREATE) &&
-		dmt->dev_name && *dmt->dev_name && !udev_only)
+		if ((dmt->add_node == DM_ADD_NODE_ON_CREATE) &&
+		    dmt->dev_name && *dmt->dev_name && !udev_only)
 			add_dev_node(dmt->dev_name, MAJOR(dmi->dev),
 				     MINOR(dmi->dev), dmt->uid, dmt->gid,
 				     dmt->mode, check_udev);

LVM2/libdm/ioctl libdm-iface.c

[mbroz]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mbroz at sourceware.org	2011-03-05 21:17:19

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	Move secure flag warning to verbose level.
	
	This feature is not yet upstream and becuase cryptsetup in next version
	uses this flag, it will cause a lot of noise on old kernels.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/ioctl/libdm-iface.c.diff?cvsroot=lvm2&r1=1.97&r2=1.98

--- LVM2/libdm/ioctl/libdm-iface.c	2011/03/02 08:41:55	1.97
+++ LVM2/libdm/ioctl/libdm-iface.c	2011/03/05 21:17:19	1.98
@@ -1560,8 +1560,8 @@
 		dmi->flags |= DM_SKIP_LOCKFS_FLAG;
 	if (dmt->secure_data) {
 		if (_dm_version_minor < 20)
-			log_warn("WARNING: Secure data flag unsupported by "
-				 "kernel.  Buffers will not be wiped after use.");
+			log_verbose("Secure data flag unsupported by kernel. "
+				    "Buffers will not be wiped after use.");
 		dmi->flags |= DM_SECURE_DATA_FLAG;
 	}
 	if (dmt->query_inactive_table) {

LVM2/libdm/ioctl libdm-iface.c

[Alasdair G Kergon]

On Sat, Mar 05, 2011 at 09:17:19PM -0000, mbroz at sourceware.org wrote:
> 	This feature is not yet upstream and becuase cryptsetup in next version
> 	uses this flag, it will cause a lot of noise on old kernels.
 
Let's take this as a temporary change then, and once the new code becomes
reasonably widely used, revert this patch, OK?

Alasdair

LVM2/libdm/ioctl libdm-iface.c

[Milan Broz]

On 03/06/2011 02:18 AM, Alasdair G Kergon wrote:
> On Sat, Mar 05, 2011 at 09:17:19PM -0000, mbroz at sourceware.org wrote:
>> 	This feature is not yet upstream and becuase cryptsetup in next version
>> 	uses this flag, it will cause a lot of noise on old kernels.
>  
> Let's take this as a temporary change then, and once the new code becomes
> reasonably widely used, revert this patch, OK?

IMHO that message should not be there at all. If application relies on
it, then it must fail completely, warning is not enough.
For other uses it is optional.

Anyway, we can revert it even now, I added version check into cryptsetup
to detect kernel DM version and do not use that call if it is old.
And the same change will be needed in RHEL or people will complain...

Milan

LVM2/libdm/ioctl libdm-iface.c

[Alasdair G Kergon]

On Sun, Mar 06, 2011 at 02:36:58AM +0100, Milan Broz wrote:
> IMHO that message should not be there at all. If application relies on
> it, then it must fail completely, warning is not enough.
 
Indeed - that should be the long-term change - to fail instead of warn,
and let the application decide whether to retry less-securely.

Alasdair

LVM2/libdm/ioctl libdm-iface.c

[agk]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	agk at sourceware.org	2011-03-20 02:00:52

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	Fix last checkin - added error message text wrong.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/ioctl/libdm-iface.c.diff?cvsroot=lvm2&r1=1.101&r2=1.102

--- LVM2/libdm/ioctl/libdm-iface.c	2011/03/18 13:21:02	1.101
+++ LVM2/libdm/ioctl/libdm-iface.c	2011/03/20 02:00:52	1.102
@@ -965,10 +965,8 @@
 	if (_check_version(dmversion, sizeof(dmversion), _dm_compat))
 		return 1;
 
-	if (!_dm_compat) {
-		log_error("Support for older device-mapped version is disabled.");
-		goto bad;
-	}
+	if (!_dm_compat)
+		goto_bad;
 
 	log_verbose("device-mapper ioctl protocol version %u failed. "
 		    "Trying protocol version 1.", _dm_version);

LVM2/libdm/ioctl libdm-iface.c

[Zdenek Kabelac]

Dne 20.3.2011 03:00, agk at sourceware.org napsal(a):
> CVSROOT:	/cvs/lvm2
> Module name:	LVM2
> Changes by:	agk at sourceware.org	2011-03-20 02:00:52
> 
> Modified files:
> 	libdm/ioctl    : libdm-iface.c 
> 
> Log message:
> 	Fix last checkin - added error message text wrong.
> 
> Patches:

>  
> -	if (!_dm_compat) {
> -		log_error("Support for older device-mapped version is disabled.");
> -		goto bad;
> -	}
> +	if (!_dm_compat)
> +		goto_bad;

My patch was only fixing error reporting logic.

Now it's reports 'stack' trace before the actual report.

AFAIK your own rule is - to always report error first and then report 'stack'
traces upward.

Did I get this rule wrong - are there some exceptions?

Zdenek

LVM2/libdm/ioctl libdm-iface.c

[Alasdair G Kergon]

On Sun, Mar 20, 2011 at 02:09:13PM +0100, Zdenek Kabelac wrote:
> My patch was only fixing error reporting logic.
 
One output case:
  Failed to get device-mapper version
  Support for older device-mapped version is disabled.

A quite misleading pair of messages, I think.

Another output case, when only one version is compiled into the binary:
  ...
  Support for older device-mapped version is disabled.

Support was not compiled in so we shouldn't mention anything about it to the
user.

> Did I get this rule wrong - are there some exceptions?

  We must always have at least one log_error on the error path, but as
few as possible.
  We try to place it as low down in the code as we can consistent with
minimising the number of log_errors: it is normally, but not always, at the
lowest level.  
  'stack' is used in other places on error paths.
 
Alasdair

LVM2/libdm/ioctl libdm-iface.c

[Zdenek Kabelac]

Dne 20.3.2011 22:41, Alasdair G Kergon napsal(a):
> On Sun, Mar 20, 2011 at 02:09:13PM +0100, Zdenek Kabelac wrote:
>> My patch was only fixing error reporting logic.
>  
> One output case:
>   Failed to get device-mapper version
>   Support for older device-mapped version is disabled.
> 
> A quite misleading pair of messages, I think.

The order would be opposite - and I believe that giving the user knowledge,
that lvm lacks some part of code to handle this situation and code needs to be
recompiled with such support is IMHO good hint for error like this.

(Hmm typo -  device-mapper)

In fact -  Failed to get... should be probably log_warn

And also it's worth to know - we have 2 other log_error() already there:

--
dm_task_create: malloc(
--
Incompatible libdevmapper %s%s and kernel driver %s
--

Which might be printed together with "Failed to get..."

So that's why I've tried to add a bit more consistency (which strikes to my
eye with latest Milan's commit to this code).


> Another output case, when only one version is compiled into the binary:
>   ...
>   Support for older device-mapped version is disabled.
> 
> Support was not compiled in so we shouldn't mention anything about it to the
> user.

Well that's where I think 'root' should know why the operation failed.


>   We try to place it as low down in the code as we can consistent with
> minimising the number of log_errors: it is normally, but not always, at the
> lowest level.  
>   'stack' is used in other places on error paths.

The other way around would be to remove then all log_error below
dm_task_create() - and use there only log_debug/log_warn
(i.e. similar to dm_malloc() case where the actual error is defered)

Zdenek

LVM2/libdm/ioctl libdm-iface.c

[Alasdair G Kergon]

> The order would be opposite - and I believe that giving the user knowledge,
> that lvm lacks some part of code to handle this situation and code needs to be
> recompiled with such support is IMHO good hint for error like this.
 
_dm_compat is deprecated functionality and I think the proposed message is
very misleading, and most likely nothing to do with the actual problem
the user needs to address on that code path.

> And also it's worth to know - we have 2 other log_error() already there:

which both look fine (on the deprecated code path).

> > Support was not compiled in so we shouldn't mention anything about it to the
> > user.
> Well that's where I think 'root' should know why the operation failed.
 
No - if the support was not compiled in it is because it is known not to be
required.

Alasdair

LVM2/libdm/ioctl libdm-iface.c

[agk]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	agk at sourceware.org	2011-06-13 03:53:02

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	Fix fields in warning message.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/ioctl/libdm-iface.c.diff?cvsroot=lvm2&r1=1.105&r2=1.106

--- LVM2/libdm/ioctl/libdm-iface.c	2011/06/13 03:32:46	1.105
+++ LVM2/libdm/ioctl/libdm-iface.c	2011/06/13 03:53:02	1.106
@@ -2062,11 +2062,11 @@
 	    dmt->type == DM_DEVICE_RELOAD)
 		log_error("Performing unsafe table load while %d device(s) "
 			  "are known to be suspended: "
-			  "%s%s %s %s%.0d%s%.0d%s%s",
+			  "%s%s%s %s%.0d%s%.0d%s%s",
 			  suspended_counter,
-	                  dmt->new_uuid ? "UUID " : "",
-	                  dmi->name,
-			  dmi->uuid,
+	                  dmt->dev_name ? : "",
+	                  dmt->uuid ? " UUID " : "",
+			  dmt->uuid ? : "",
 	                  dmt->major > 0 ? "(" : "",
 	                  dmt->major > 0 ? dmt->major : 0,
 	                  dmt->major > 0 ? ":" : "",

LVM2/libdm/ioctl libdm-iface.c

[agk]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	agk at sourceware.org	2011-06-29 08:54:14

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	Temporary conversion to internal error and failure, to see how many
	instances of this problem this flushes out.

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

--- LVM2/libdm/ioctl/libdm-iface.c	2011/06/27 21:43:59	1.108
+++ LVM2/libdm/ioctl/libdm-iface.c	2011/06/29 08:54:13	1.109
@@ -2059,8 +2059,8 @@
 	}
 
 	if ((suspended_counter = dm_get_suspended_counter()) &&
-	    dmt->type == DM_DEVICE_RELOAD)
-		log_error("Performing unsafe table load while %d device(s) "
+	    dmt->type == DM_DEVICE_RELOAD) {
+		log_error(INTERNAL_ERROR "Performing unsafe table load while %d device(s) "
 			  "are known to be suspended: "
 			  "%s%s%s %s%.0d%s%.0d%s%s",
 			  suspended_counter,
@@ -2073,6 +2073,8 @@
 	                  dmt->minor > 0 ? dmt->minor : 0,
 	                  dmt->major > 0 && dmt->minor == 0 ? "0" : "",
 	                  dmt->major > 0 ? ") " : "");
+		return 0;
+	}
 
 	/* FIXME Detect and warn if cookie set but should not be. */
 repeat_ioctl:

LVM2/libdm/ioctl libdm-iface.c

[agk]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	agk at sourceware.org	2011-06-29 11:36:37

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	Remove temporary failures now, but continue to give INTERNAL_ERROR.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/ioctl/libdm-iface.c.diff?cvsroot=lvm2&r1=1.109&r2=1.110

--- LVM2/libdm/ioctl/libdm-iface.c	2011/06/29 08:54:13	1.109
+++ LVM2/libdm/ioctl/libdm-iface.c	2011/06/29 11:36:37	1.110
@@ -2059,7 +2059,7 @@
 	}
 
 	if ((suspended_counter = dm_get_suspended_counter()) &&
-	    dmt->type == DM_DEVICE_RELOAD) {
+	    dmt->type == DM_DEVICE_RELOAD)
 		log_error(INTERNAL_ERROR "Performing unsafe table load while %d device(s) "
 			  "are known to be suspended: "
 			  "%s%s%s %s%.0d%s%.0d%s%s",
@@ -2073,8 +2073,6 @@
 	                  dmt->minor > 0 ? dmt->minor : 0,
 	                  dmt->major > 0 && dmt->minor == 0 ? "0" : "",
 	                  dmt->major > 0 ? ") " : "");
-		return 0;
-	}
 
 	/* FIXME Detect and warn if cookie set but should not be. */
 repeat_ioctl:

LVM2/libdm/ioctl libdm-iface.c

[agk]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	agk at sourceware.org	2011-06-29 16:08:33

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	debug log readonly flag with ioctls

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/ioctl/libdm-iface.c.diff?cvsroot=lvm2&r1=1.110&r2=1.111

--- LVM2/libdm/ioctl/libdm-iface.c	2011/06/29 11:36:37	1.110
+++ LVM2/libdm/ioctl/libdm-iface.c	2011/06/29 16:08:33	1.111
@@ -1963,7 +1963,7 @@
 	}
 
 	log_debug("dm %s %s%s %s%s%s %s%.0d%s%.0d%s"
-		  "%s%c%c%s%s%s %.0" PRIu64 " %s [%u]",
+		  "%s%c%c%s%s%s%s %.0" PRIu64 " %s [%u]",
 		  _cmd_data_v4[dmt->type].name,
 		  dmt->new_uuid ? "UUID " : "",
 		  dmi->name, dmi->uuid, dmt->newname ? " " : "",
@@ -1976,6 +1976,7 @@
 		  dmt->major > 0 ? ") " : "",
 		  dmt->no_open_count ? 'N' : 'O',
 		  dmt->no_flush ? 'N' : 'F',
+		  dmt->read_only ? "R" : "",
 		  dmt->skip_lockfs ? "S " : "",
 		  dmt->secure_data ? "W " : "",
 		  dmt->query_inactive_table ? "I " : "",

LVM2/libdm/ioctl libdm-iface.c

[agk]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	agk at sourceware.org	2011-08-19 16:49:00

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	revert incomplete inconsistent log message change for now

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/ioctl/libdm-iface.c.diff?cvsroot=lvm2&r1=1.117&r2=1.118

--- LVM2/libdm/ioctl/libdm-iface.c	2011/08/18 19:43:09	1.117
+++ LVM2/libdm/ioctl/libdm-iface.c	2011/08/19 16:49:00	1.118
@@ -1639,10 +1639,9 @@
 				    	    _cmd_data_v4[dmt->type].name,
 					    strerror(errno));
 			else
-				log_error("device-mapper: %s ioctl for %s "
+				log_error("device-mapper: %s ioctl "
 					  "failed: %s",
 					  _cmd_data_v4[dmt->type].name,
-					  dmi->name ? dmi->name : dmi->uuid,
 					  strerror(errno));
 			_dm_zfree_dmi(dmi);
 			return NULL;

LVM2/libdm/ioctl libdm-iface.c

[prajnoha]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	prajnoha at sourceware.org	2011-09-22 17:59:59

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	Initialize 'retryable' variable.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/ioctl/libdm-iface.c.diff?cvsroot=lvm2&r1=1.121&r2=1.122

--- LVM2/libdm/ioctl/libdm-iface.c	2011/09/22 17:09:48	1.121
+++ LVM2/libdm/ioctl/libdm-iface.c	2011/09/22 17:59:58	1.122
@@ -1689,7 +1689,7 @@
 	int rely_on_udev;
 	int suspended_counter;
 	unsigned ioctl_retry = 1;
-	int retryable;
+	int retryable = 0;
 
 	if ((unsigned) dmt->type >=
 	    (sizeof(_cmd_data_v4) / sizeof(*_cmd_data_v4))) {

LVM2/libdm/ioctl libdm-iface.c

[agk]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	agk at sourceware.org	2011-09-23 17:16:28

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	explain why we may now retry

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

--- LVM2/libdm/ioctl/libdm-iface.c	2011/09/22 17:59:58	1.122
+++ LVM2/libdm/ioctl/libdm-iface.c	2011/09/23 17:16:28	1.123
@@ -1654,6 +1654,11 @@
 					  _cmd_data_v4[dmt->type].name,
 					  strerror(errno));
 
+			/*
+			 * It's sometimes worth retrying after EBUSY in case
+			 * it's a transient failure caused by an asynchronous
+			 * process quickly scanning the device.
+			 */
 			*retryable = errno == EBUSY;
 
 			_dm_zfree_dmi(dmi);
@@ -1739,6 +1744,12 @@
 repeat_ioctl:
 	if (!(dmi = _do_dm_ioctl(dmt, command, _ioctl_buffer_double_factor,
 				 ioctl_retry, &retryable))) {
+		/*
+		 * Async udev rules that scan devices commonly cause transient
+		 * failures.  Normally you'd expect the user to have made sure
+		 * nothing was using the device before issuing REMOVE, so it's
+		 * worth retrying in case the failure is indeed transient.
+		 */
 		if (retryable && dmt->type == DM_DEVICE_REMOVE &&
 		    dmt->retry_remove && ++ioctl_retry <= DM_IOCTL_RETRIES) {
 			usleep(DM_RETRY_USLEEP_DELAY);

LVM2/libdm/ioctl libdm-iface.c

[zkabelac]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac at sourceware.org	2011-10-17 14:36:06

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	Use zalloc for malloc,memset

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

--- LVM2/libdm/ioctl/libdm-iface.c	2011/09/23 17:16:28	1.123
+++ LVM2/libdm/ioctl/libdm-iface.c	2011/10/17 14:36:06	1.124
@@ -852,7 +852,7 @@
 struct target *create_target(uint64_t start, uint64_t len, const char *type,
 			     const char *params)
 {
-	struct target *t = dm_malloc(sizeof(*t));
+	struct target *t = dm_zalloc(sizeof(*t));
 
 	if (!t) {
 		log_error("create_target: malloc(%" PRIsize_t ") failed",
@@ -860,8 +860,6 @@
 		return NULL;
 	}
 
-	memset(t, 0, sizeof(*t));
-
 	if (!(t->params = dm_strdup(params))) {
 		log_error("create_target: strdup(params) failed");
 		goto bad;

LVM2/libdm/ioctl libdm-iface.c

[snitzer]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	snitzer at sourceware.org	2011-11-08 19:02:21

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	Add missing free() for line that is malloc()'d by getline().

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/ioctl/libdm-iface.c.diff?cvsroot=lvm2&r1=1.127&r2=1.128

--- LVM2/libdm/ioctl/libdm-iface.c	2011/11/08 17:32:11	1.127
+++ LVM2/libdm/ioctl/libdm-iface.c	2011/11/08 19:02:21	1.128
@@ -172,7 +172,7 @@
 {
 	FILE *fl;
 	char nm[256];
-	char *line;
+	char *line = NULL;
 	size_t len;
 	uint32_t num;
 
@@ -188,6 +188,7 @@
 					*number = num;
 					if (fclose(fl))
 						log_sys_error("fclose", file);
+					free(line);
 					return 1;
 				}
 				dm_bit_set(_dm_bitset, num);
@@ -196,6 +197,7 @@
 	}
 	if (fclose(fl))
 		log_sys_error("fclose", file);
+	free(line);
 
 	if (number) {
 		log_error("%s: No entry for %s found", file, name);

LVM2/libdm/ioctl libdm-iface.c

[zkabelac]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac at sourceware.org	2012-03-01 10:46:39

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	Missed ;

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/ioctl/libdm-iface.c.diff?cvsroot=lvm2&r1=1.139&r2=1.140

--- LVM2/libdm/ioctl/libdm-iface.c	2012/03/01 10:07:38	1.139
+++ LVM2/libdm/ioctl/libdm-iface.c	2012/03/01 10:46:39	1.140
@@ -479,7 +479,7 @@
 	    (snprintf(version, size, "%u.%u.%u", v[0], v[1], v[2]) < 0)) {
 		log_error("Buffer for version is to short.");
 		if (size > 0)
-			version[0] = '\0'
+			version[0] = '\0';
 		return 0;
 	}
 

LVM2/libdm/ioctl libdm-iface.c

[prajnoha]

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	prajnoha at sourceware.org	2012-03-05 14:45:00

Modified files:
	libdm/ioctl    : libdm-iface.c 

Log message:
	Use already acquired variable...

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/libdm/ioctl/libdm-iface.c.diff?cvsroot=lvm2&r1=1.143&r2=1.144

--- LVM2/libdm/ioctl/libdm-iface.c	2012/03/05 12:48:13	1.143
+++ LVM2/libdm/ioctl/libdm-iface.c	2012/03/05 14:45:00	1.144
@@ -1562,8 +1562,7 @@
 	if (!check_multiple_mangled_name_allowed(mode, name))
 		return_0;
 
-	if ((r = unmangle_name(name, DM_NAME_LEN, buf, sizeof(buf),
-			       dm_get_name_mangling_mode())) < 0) {
+	if ((r = unmangle_name(name, DM_NAME_LEN, buf, sizeof(buf), mode)) < 0) {
 		log_debug("_do_dm_ioctl_unmangle_name: failed to "
 			  "unmangle \"%s\"", name);
 		return 0;