Skip to contentSkip to conversation

Miscellaneous fixes

PATCH New Zdenek Kabelac 2 participants 16 messages Jul 28, 2010

lore
Share & tools
lore
#0 lore raw

cover Miscellaneous fixes

Miscellaneous fixes:

One fix for non-srcdir builds.
Fxes for testsuit.
Cleanups for compiler warnings.
Updates to dmsetup code.

Last patch is proposal for 3 solution how to fix udev patch checking - I think the check should be a separate function in lib dm used by dmsetup and liblvm. This patch is not supposed to be applied in this form.

Zdenek Kabelac (14):
Skip VPATH usage
Testsuit: Shell &&||
Testsuit: create empty files
Testsuit: test variables with quotes
Testsuit: speedup
Testsuit: add missing prepare_dmeventd
Fix constness warning
Fix C function definition
Fix constness warning
Fix void* arithmetic - use char*
Fix void* arithmetic and more readable char[]
log_debug event_nr
Reorder read_ahead check
Proposal of different check for udev dir

 daemons/clvmd/clvmd-singlenode.c        |    6 ++--
 daemons/clvmd/lvm-functions.c           |    4 +-
 include/Makefile.in                     |    1 -
 lib/activate/dev_manager.c              |    8 +++---
 lib/format_text/archive.c               |    4 +-
 lib/format_text/import.c                |    2 +-
 lib/locking/file_locking.c              |    2 +-
 libdm/ioctl/libdm-iface.c               |   13 ++++++-----
 libdm/mm/dbg_malloc.c                   |    6 ++--
 test/lvm-utils.sh                       |   30 +++++++++++++++++----------
 test/t-000-basic.sh                     |    4 +-
 test/t-activate-missing.sh              |    1 +
 test/t-activate-partial.sh              |    1 +
 test/t-inconsistent-metadata.sh         |    1 +
 test/t-listings.sh                      |    1 +
 test/t-lock-blocking.sh                 |    2 +-
 test/t-lvchange-mirror.sh               |    1 +
 test/t-lvconvert-mirror-basic.sh        |    1 +
 test/t-lvconvert-mirror.sh              |    1 +
 test/t-lvconvert-repair-dmeventd.sh     |    2 +-
 test/t-lvconvert-repair-policy.sh       |    1 +
 test/t-lvconvert-repair-replace.sh      |    1 +
 test/t-lvconvert-repair-transient.sh    |    1 +
 test/t-lvconvert-repair.sh              |    2 +-
 test/t-lvcreate-mirror.sh               |    1 +
 test/t-lvcreate-operation.sh            |    1 +
 test/t-lvcreate-pvtags.sh               |    1 +
 test/t-lvcreate-usage.sh                |    1 +
 test/t-lvresize-mirror.sh               |    1 +
 test/t-metadata.sh                      |    1 +
 test/t-mirror-names.sh                  |    1 +
 test/t-mirror-vgreduce-removemissing.sh |    1 +
 test/t-nomda-missing.sh                 |    1 +
 test/t-pvchange-usage.sh                |    2 +-
 test/t-pvmove-basic.sh                  |    7 ++---
 test/t-snapshots-of-mirrors.sh          |    1 +
 test/t-unknown-segment.sh               |    1 +
 test/t-unlost-pv.sh                     |    1 +
 test/test-utils.sh                      |    2 +-
 tools/dmsetup.c                         |   33 ++++++++++++++++++++++++++++--
 tools/lvmcmdline.c                      |    4 ++-
 41 files changed, 107 insertions(+), 49 deletions(-)
Signature · 1 line
1.7.2
#1 lore raw

PATCH 1/14 Skip VPATH usage

Do not use VPATH as we may get wrong builds when $srcdir has complete build and different $builddir is used later. Files from $builddir need to be used.

---
 include/Makefile.in |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
include/Makefile.in +0 -1
index 3023d4a..0354d2e 100644
@@ -17,7 +17,6 @@ SHELL = /bin/sh
 srcdir = @srcdir@
 top_srcdir = @top_srcdir@
 top_builddir = @top_builddir@
-VPATH = @srcdir@
 
 LN_S = @LN_S@
 
Signature · 1 line
1.7.2
#2 lore raw

PATCH 2/14 Testsuit: Shell &&||

In reply to Zdenek Kabelac

Visually better align lines which are executed as a result of true or false result of previous command.

---
 test/lvm-utils.sh      |    6 +++---
 test/t-000-basic.sh    |    4 ++--
 test/t-pvmove-basic.sh |    7 +++----
 3 files changed, 8 insertions(+), 9 deletions(-)
test/lvm-utils.sh +3 -3
index 39d52fd..6c00084 100644
@@ -129,9 +129,9 @@ vg_validate_pvlv_counts_()
 
 	lvs -a -o+devices $local_vg
 
-	check_vg_field_ $local_vg pv_count $num_pvs &&
-	check_vg_field_ $local_vg lv_count $num_lvs &&
-	check_vg_field_ $local_vg snap_count $num_snaps
+	check_vg_field_ $local_vg pv_count $num_pvs && \
+	  check_vg_field_ $local_vg lv_count $num_lvs && \
+	  check_vg_field_ $local_vg snap_count $num_snaps
 }
 
 dmsetup_has_dm_devdir_support_()
test/t-000-basic.sh +2 -2
index c70a7f8..ed76a6f 100755
@@ -20,9 +20,9 @@ lvm pvmove --version|sed -n "1s/.*: *\([0-9][^ ]*\) .*/\1/p" > actual
 # ensure they are the same
 diff -u actual expected
 
-mknod $DM_DEV_DIR/null c 1 3 ||
+mknod $DM_DEV_DIR/null c 1 3 || \
   error "Can't create nodes on filesystem"
-echo >$DM_DEV_DIR/null || 
+echo >$DM_DEV_DIR/null || \
   error "Filesystem for tests does not allow using device nodes (check nodev)"
 
 # ensure we can create devices (uses dmsetup, etc)
test/t-pvmove-basic.sh +3 -4
index cc2145b..44b533c 100755
@@ -33,8 +33,7 @@ lv_is_on_() {
 }
 
 save_dev_sum_() {
-  mkfs.ext3 $1 > /dev/null &&
-  md5sum $1 > md5.$(basename $1)
+  mkfs.ext3 $1 > /dev/null && md5sum $1 > md5.$(basename $1)
 }
 
 check_dev_sum_() {
@@ -83,8 +82,8 @@ check_and_cleanup_lvs_() {
   lvs -a -o name $vg > out && ! grep ^pvmove out
   lvremove -ff $vg
 	if ! dmsetup table|not grep $vg; then
-		echo "ERROR: lvremove did leave some some mappings in DM behind!" &&
-		return 1
+		echo "ERROR: lvremove did leave some some mappings in DM behind!" && \
+			return 1
 	fi
 	:
 }
Signature · 1 line
1.7.2
#3 lore raw

PATCH 3/14 Testsuit: create empty files

In reply to Zdenek Kabelac

Using count=0 is sufficient for creation of zeroed files.

---
 test/test-utils.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
test/test-utils.sh +1 -1
index b3ee6f6..c044de0 100644
@@ -187,7 +187,7 @@ prepare_loop() {
 	done
 
 	LOOPFILE="$PWD/test.img"
-	dd if=/dev/zero of="$LOOPFILE" bs=$((1024*1024)) count=1 seek=$(($size-1))
+	dd if=/dev/zero of="$LOOPFILE" bs=$((1024*1024)) count=0 seek=$(($size-1))
 	if LOOP=`losetup -s -f "$LOOPFILE" 2>/dev/null`; then
 		return 0
 	elif LOOP=`losetup -f` && losetup $LOOP "$LOOPFILE"; then
Signature · 1 line
1.7.2
#4 lore raw

PATCH 4/14 Testsuit: test variables with quotes

In reply to Zdenek Kabelac

Add shell function to trim spaces.
Test values as "$val" to avoid weird results when spaces are in output.

---
 test/lvm-utils.sh        |   24 ++++++++++++++++--------
 test/t-pvchange-usage.sh |    2 +-
 2 files changed, 17 insertions(+), 9 deletions(-)
test/lvm-utils.sh +16 -8
index 6c00084..486f2d8 100644
@@ -16,6 +16,14 @@ export LVM_SUPPRESS_FD_WARNINGS=1
 ME=$(basename "$0")
 warn() { echo >&2 "$ME: $@"; }
 
+trim()
+{
+    trimmed=${1%% }
+    trimmed=${trimmed## }
+
+    echo "$trimmed"
+}
+
 compare_two_fields_()
 {
     local cmd1=$1;
@@ -33,7 +41,7 @@ if test "$verbose" = "t"
 then
   echo "compare_two_fields_ $obj1($field1): $val1 $obj2($field2): $val2"
 fi
-  test $val1 = $val2
+  test "$val1" = "$val2"
 }
 
 compare_vg_field_()
@@ -50,7 +58,7 @@ if test "$verbose" = "t"
 then
   echo "compare_vg_field_ VG1: $val1 VG2: $val2"
 fi
-  test $val1 = $val2
+  test "$val1" = "$val2"
 }
 
 
@@ -82,12 +90,12 @@ check_vg_field_()
     local expected=$3;
     local actual;
 
-    actual=$(vgs --noheadings -o $field $vg)
+    actual=$(trim $(vgs --noheadings -o $field $vg))
 if test "$verbose" = "t"
 then
   echo "check_vg_field_ VG=$vg, field=$field, actual=$actual, expected=$expected"
 fi
-  test $actual = $expected
+  test "$actual" = "$expected"
 }
 
 check_pv_field_()
@@ -97,12 +105,12 @@ check_pv_field_()
     local expected=$3;
     local actual;
 
-    actual=$(pvs --noheadings -o $field $pv)
+    actual=$(trim $(pvs --noheadings -o $field $pv))
 if test "$verbose" = "t"
 then
   echo "check_pv_field_ PV=$pv, field=$field, actual=$actual, expected=$expected"
 fi
-    test $actual = $expected
+    test "$actual" = "$expected"
 }
 
 check_lv_field_()
@@ -112,12 +120,12 @@ check_lv_field_()
     local expected=$3;
     local actual;
 
-    actual=$(lvs --noheadings -o $field $lv)
+    actual=$(trim $(lvs --noheadings -o $field $lv))
 if test "$verbose" = "t"
 then
   echo "check_lv_field_ LV=$lv, field=$field, actual=$actual, expected=$expected"
 fi
-  test $actual = $expected
+  test "$actual" = "$expected"
 }
 
 vg_validate_pvlv_counts_()
test/t-pvchange-usage.sh +1 -1
index 4c28c62..0f69249 100755
@@ -26,7 +26,7 @@ do
 	pvchange $dev1 --addtag test$mda 
 	check_pv_field_ $dev1 pv_tags test$mda 
 	pvchange $dev1 --deltag test$mda 
-	check_pv_field_ $dev1 pv_tags " "
+	check_pv_field_ $dev1 pv_tags ""
 
 # "vgchange disable/enable allocation for pvs with metadatacopies = $mda (bz452982)"
 	pvchange $dev1 -x n 
Signature · 1 line
1.7.2
#5 lore raw

PATCH 5/14 Testsuit: speedup

In reply to Zdenek Kabelac

Instead of waiting for flock end kill lock.

---
 test/t-lock-blocking.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
test/t-lock-blocking.sh +1 -1
index 7e5321e..166340c 100644
@@ -33,4 +33,4 @@ flock -w 5 $TESTDIR/var/lock/lvm/P_orphans -c "sleep 10" &
 flock_pid=`jobs -p`
 not vgremove --config 'global { wait_for_locks = 0 }' $vg
 ps $flock_pid # still running
-wait $flock_pid
+kill $flock_pid
Signature · 1 line
1.7.2
#6 lore raw

PATCH 6/14 Testsuit: add missing prepare_dmeventd

In reply to Zdenek Kabelac

All these tests may start dmeventd, so use prepare_dmeventd function to keep dmeventd local for the test.

---
 test/t-activate-missing.sh              |    1 +
 test/t-activate-partial.sh              |    1 +
 test/t-inconsistent-metadata.sh         |    1 +
 test/t-listings.sh                      |    1 +
 test/t-lvchange-mirror.sh               |    1 +
 test/t-lvconvert-mirror-basic.sh        |    1 +
 test/t-lvconvert-mirror.sh              |    1 +
 test/t-lvconvert-repair-dmeventd.sh     |    2 +-
 test/t-lvconvert-repair-policy.sh       |    1 +
 test/t-lvconvert-repair-replace.sh      |    1 +
 test/t-lvconvert-repair-transient.sh    |    1 +
 test/t-lvconvert-repair.sh              |    2 +-
 test/t-lvcreate-mirror.sh               |    1 +
 test/t-lvcreate-operation.sh            |    1 +
 test/t-lvcreate-pvtags.sh               |    1 +
 test/t-lvcreate-usage.sh                |    1 +
 test/t-lvresize-mirror.sh               |    1 +
 test/t-metadata.sh                      |    1 +
 test/t-mirror-names.sh                  |    1 +
 test/t-mirror-vgreduce-removemissing.sh |    1 +
 test/t-nomda-missing.sh                 |    1 +
 test/t-snapshots-of-mirrors.sh          |    1 +
 test/t-unknown-segment.sh               |    1 +
 test/t-unlost-pv.sh                     |    1 +
 24 files changed, 24 insertions(+), 2 deletions(-)
test/t-activate-missing.sh +1 -0
index 86fb09c..761a88f 100644
@@ -8,6 +8,7 @@
 # - linear LVs with bits missing are not activated
 
 . ./test-utils.sh
+prepare_dmeventd
 
 prepare_vg 4
 
test/t-activate-partial.sh +1 -0
index 15f5965..1a18661 100644
@@ -1,4 +1,5 @@
 . ./test-utils.sh
+prepare_dmeventd
 
 aux prepare_vg 3
 
test/t-inconsistent-metadata.sh +1 -0
index 310dde5..75664e6 100644
@@ -10,6 +10,7 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 . ./test-utils.sh
+prepare_dmeventd
 
 aux prepare_vg 3
 
test/t-listings.sh +1 -0
index 9ea10b6..f069d15 100644
@@ -13,6 +13,7 @@
 #
 
 . ./test-utils.sh
+prepare_dmeventd
 
 get_lvs_()
 {
test/t-lvchange-mirror.sh +1 -0
index 7c915be..bc5841f 100644
@@ -10,6 +10,7 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 . ./test-utils.sh
+prepare_dmeventd
 aux prepare_vg 3
 
 # force resync 2-way active mirror
test/t-lvconvert-mirror-basic.sh +1 -0
index 665f34f..77ff1de 100644
@@ -1,4 +1,5 @@
 . ./test-utils.sh
+prepare_dmeventd
 
 log_name_to_count()
 {
test/t-lvconvert-mirror.sh +1 -0
index 3a58f35..20ec4be 100644
@@ -10,6 +10,7 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 . ./test-utils.sh
+prepare_dmeventd
 
 # convert from linear to 2-way mirror
 aux prepare_vg 5
test/t-lvconvert-repair-dmeventd.sh +1 -1
index 27744bf..ea6cef3 100644
@@ -10,9 +10,9 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 . ./test-utils.sh
+prepare_dmeventd
 
 prepare_vg 5
-prepare_dmeventd
 
 which mkfs.ext2 || exit 200
 
test/t-lvconvert-repair-policy.sh +1 -0
index 1f31215..89c096f 100644
@@ -10,6 +10,7 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 . ./test-utils.sh
+prepare_dmeventd
 
 prepare_vg 4
 
test/t-lvconvert-repair-replace.sh +1 -0
index 7315a0e..ec380ba 100644
@@ -10,6 +10,7 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 . ./test-utils.sh
+prepare_dmeventd
 
 prepare_vg 6
 
test/t-lvconvert-repair-transient.sh +1 -0
index b4e1a06..2b21314 100644
@@ -10,6 +10,7 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 . ./test-utils.sh
+prepare_dmeventd
 
 prepare_vg 5
 
test/t-lvconvert-repair.sh +1 -1
index bce8b53..80fd053 100644
@@ -10,7 +10,7 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 . ./test-utils.sh
-
+prepare_dmeventd
 
 # fail multiple devices
 
test/t-lvcreate-mirror.sh +1 -0
index ae3fceb..706674c 100644
@@ -10,6 +10,7 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 . ./test-utils.sh
+prepare_dmeventd
 aux prepare_vg 5 80
 
 # 2-way mirror with corelog, 2 PVs
test/t-lvcreate-operation.sh +1 -0
index 8a31759..67f245b 100644
@@ -12,6 +12,7 @@
 # 'Exercise some lvcreate diagnostics'
 
 . ./test-utils.sh
+prepare_dmeventd
 
 cleanup_lvs() {
 	lvremove -ff $vg
test/t-lvcreate-pvtags.sh +1 -0
index b9a4380..a066069 100755
@@ -9,6 +9,7 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 . ./test-utils.sh
+prepare_dmeventd
 
 aux prepare_pvs 3
 # not required, just testing
test/t-lvcreate-usage.sh +1 -0
index 0bb06ab..7fe89dc 100755
@@ -12,6 +12,7 @@
 # 'Exercise some lvcreate diagnostics'
 
 . ./test-utils.sh
+prepare_dmeventd
 
 aux prepare_pvs 4
 aux pvcreate --metadatacopies 0 $dev1
test/t-lvresize-mirror.sh +1 -0
index 0d63102..758b9b2 100644
@@ -10,6 +10,7 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 . ./test-utils.sh
+prepare_dmeventd
 aux prepare_vg 5 80
 
 # extend 2-way mirror
test/t-metadata.sh +1 -0
index 6f0268e..5a8648e 100755
@@ -9,6 +9,7 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 . ./test-utils.sh
+prepare_dmeventd
 
 aux prepare_devs 5
 
test/t-mirror-names.sh +1 -0
index 1363d84..930d5e2 100644
@@ -13,6 +13,7 @@
 test_description="check namings of mirrored LV"
 
 . ./test-utils.sh
+prepare_dmeventd
 
 # ---------------------------------------------------------------------
 # Utilities
test/t-mirror-vgreduce-removemissing.sh +1 -0
index 0f6a8b0..9e7f3de 100755
@@ -12,6 +12,7 @@
 test_description="ensure that 'vgreduce --removemissing' works on mirrored LV"
 
 . ./test-utils.sh
+prepare_dmeventd
 
 lv_is_on_ ()
 {
test/t-nomda-missing.sh +1 -0
index fa4ca49..9aaed13 100644
@@ -1,6 +1,7 @@
 #!/bin/bash
 
 . ./test-utils.sh
+prepare_dmeventd
 
 prepare_devs 4
 pvcreate $dev1 $dev2
test/t-snapshots-of-mirrors.sh +1 -0
index 7ee8249..4e01bf2 100644
@@ -1,4 +1,5 @@
 . ./test-utils.sh
+prepare_dmeventd
 
 prepare_vg 4
 
test/t-unknown-segment.sh +1 -0
index 74a2710..4707ade 100644
@@ -10,6 +10,7 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 . ./test-utils.sh
+prepare_dmeventd
 
 aux prepare_vg 4
 
test/t-unlost-pv.sh +1 -0
index 7a120fb..6a894ca 100644
@@ -10,6 +10,7 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 . ./test-utils.sh
+prepare_dmeventd
 
 aux prepare_vg 3
 
Signature · 1 line
1.7.2
#7 lore raw

PATCH 7/14 Fix constness warning

In reply to Zdenek Kabelac
---
 lib/format_text/archive.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
lib/format_text/archive.c +2 -2
index 3bc14bf..95ac49d 100644
@@ -50,7 +50,7 @@
 struct archive_file {
 	struct dm_list list;
 
-	char *path;
+	const char *path;
 	uint32_t index;
 };
 
@@ -353,7 +353,7 @@ int archive_list_file(struct cmd_context *cmd, const char *file)
 {
 	struct archive_file af;
 
-	af.path = (char *)file;
+	af.path = file;
 
 	if (!path_exists(af.path)) {
 		log_error("Archive file %s not found.", af.path);
Signature · 1 line
1.7.2
#8 lore raw

PATCH 8/14 Fix C function definition

In reply to Zdenek Kabelac

Fix gcc prototype warnings

---
 daemons/clvmd/clvmd-singlenode.c |    6 +++---
 daemons/clvmd/lvm-functions.c    |    4 ++--
 lib/format_text/import.c         |    2 +-
 lib/locking/file_locking.c       |    2 +-
 libdm/ioctl/libdm-iface.c        |    2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)
daemons/clvmd/clvmd-singlenode.c +3 -3
index ec98f2c..8624077 100644
@@ -29,7 +29,7 @@
 static const char SINGLENODE_CLVMD_SOCKNAME[] = "\0singlenode_clvmd";
 static int listen_fd = -1;
 
-static int init_comms()
+static int init_comms(void)
 {
 	struct sockaddr_un addr;
 
@@ -97,7 +97,7 @@ static int _name_from_csid(const char *csid, char *name)
 	return 0;
 }
 
-static int _get_num_nodes()
+static int _get_num_nodes(void)
 {
 	return 1;
 }
@@ -207,7 +207,7 @@ static int _unlock_resource(const char *resource, int lockid)
 	return 0;
 }
 
-static int _is_quorate()
+static int _is_quorate(void)
 {
 	return 1;
 }
daemons/clvmd/lvm-functions.c +2 -2
index e7db685..aedc923 100644
@@ -624,7 +624,7 @@ int do_refresh_cache()
 /* Only called at gulm startup. Drop any leftover VG or P_orphan locks
    that might be hanging around if we died for any reason
 */
-static void drop_vg_locks()
+static void drop_vg_locks(void)
 {
 	char vg[128];
 	char line[255];
@@ -810,7 +810,7 @@ static void lvm2_log_fn(int level, const char *file, int line, int dm_errno,
 }
 
 /* This checks some basic cluster-LVM configuration stuff */
-static void check_config()
+static void check_config(void)
 {
 	int locking_type;
 
lib/format_text/import.c +1 -1
index f686418..3e0df94 100644
@@ -25,7 +25,7 @@ static struct text_vg_version_ops *(_text_vsn_list[2]);
 
 static int _text_import_initialised = 0;
 
-static void _init_text_import()
+static void _init_text_import(void)
 {
 	if (_text_import_initialised)
 		return;
lib/locking/file_locking.c +1 -1
index 627cb68..1bf8f1b 100644
@@ -118,7 +118,7 @@ static void _trap_ctrl_c(int sig __attribute__((unused)))
 	log_error("CTRL-c detected: giving up waiting for lock");
 }
 
-static void _install_ctrl_c_handler()
+static void _install_ctrl_c_handler(void)
 {
 	_handler_installed = 1;
 
libdm/ioctl/libdm-iface.c +1 -1
index 5f71098..d2579ad 100644
@@ -139,7 +139,7 @@ static void *_align(void *ptr, unsigned int a)
 	return (void *) (((unsigned long) ptr + agn) & ~agn);
 }
 
-static int _uname()
+static int _uname(void)
 {
 	static int _uts_set = 0;
 	struct utsname _uts;
Signature · 1 line
1.7.2
#9 lore raw

PATCH 9/14 Fix constness warning

In reply to Zdenek Kabelac

Don't use const char* when it doesn't bring anything useful.

---
 lib/activate/dev_manager.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
lib/activate/dev_manager.c +4 -4
index 7de341e..9b27616 100644
@@ -220,7 +220,7 @@ int dev_manager_info(struct dm_pool *mem, const struct logical_volume *lv,
 		     int with_open_count, int with_read_ahead,
 		     struct dm_info *info, uint32_t *read_ahead)
 {
-	const char *dlid, *name;
+	char *dlid, *name;
 	int r;
 
 	if (!(name = build_dm_name(mem, lv->vg->name, lv->name, NULL))) {
@@ -236,7 +236,7 @@ int dev_manager_info(struct dm_pool *mem, const struct logical_volume *lv,
 	log_debug("Getting device info for %s [%s]", name, dlid);
 	r = _info(dlid, with_open_count, with_read_ahead, info, read_ahead);
 
-	dm_pool_free(mem, (char*)name);
+	dm_pool_free(mem, name);
 	return r;
 }
 
@@ -812,7 +812,7 @@ static int _dev_manager_lv_rmnodes(const struct logical_volume *lv)
 int dev_manager_mknodes(const struct logical_volume *lv)
 {
 	struct dm_info dminfo;
-	const char *name;
+	char *name;
 	int r = 0;
 
 	if (!(name = build_dm_name(lv->vg->cmd->mem, lv->vg->name, lv->name, NULL)))
@@ -826,7 +826,7 @@ int dev_manager_mknodes(const struct logical_volume *lv)
 			r = _dev_manager_lv_rmnodes(lv);
 	}
 
-	dm_pool_free(lv->vg->cmd->mem, (char*)name);
+	dm_pool_free(lv->vg->cmd->mem, name);
 	return r;
 }
 
Signature · 1 line
1.7.2
#10 lore raw

PATCH 10/14 Fix void* arithmetic - use char*

In reply to Zdenek Kabelac
---
 libdm/ioctl/libdm-iface.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
libdm/ioctl/libdm-iface.c +3 -3
index d2579ad..bbe1de4 100644
@@ -1104,7 +1104,7 @@ struct dm_deps *dm_task_get_deps(struct dm_task *dmt)
 		return _dm_task_get_deps_v1(dmt);
 #endif
 
-	return (struct dm_deps *) (((void *) dmt->dmi.v4) +
+	return (struct dm_deps *) (((char *) dmt->dmi.v4) +
 				   dmt->dmi.v4->data_start);
 }
 
@@ -1115,13 +1115,13 @@ struct dm_names *dm_task_get_names(struct dm_task *dmt)
 		return _dm_task_get_names_v1(dmt);
 #endif
 
-	return (struct dm_names *) (((void *) dmt->dmi.v4) +
+	return (struct dm_names *) (((char *) dmt->dmi.v4) +
 				    dmt->dmi.v4->data_start);
 }
 
 struct dm_versions *dm_task_get_versions(struct dm_task *dmt)
 {
-	return (struct dm_versions *) (((void *) dmt->dmi.v4) +
+	return (struct dm_versions *) (((char *) dmt->dmi.v4) +
 				       dmt->dmi.v4->data_start);
 }
 
Signature · 1 line
1.7.2
#11 lore raw

PATCH 11/14 Fix void* arithmetic and more readable char[]

In reply to Zdenek Kabelac
---
 libdm/mm/dbg_malloc.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
libdm/mm/dbg_malloc.c +3 -3
index 0add7b6..d86326d 100644
@@ -196,12 +196,12 @@ int dm_dump_memory_debug(void)
 		for (c = 0; c < sizeof(str) - 1; c++) {
 			if (c >= mb->length)
 				str[c] = ' ';
-			else if (*(char *)(mb->magic + c) == '\0')
+			else if (((char *)mb->magic)[c] == '\0')
 				str[c] = '\0';
-			else if (*(char *)(mb->magic + c) < ' ')
+			else if (((char *)mb->magic)[c] < ' ')
 				str[c] = '?';
 			else
-				str[c] = *(char *)(mb->magic + c);
+				str[c] = ((char *)mb->magic)[c];
 		}
 		str[sizeof(str) - 1] = '\0';
 
Signature · 1 line
1.7.2
#12 lore raw

PATCH 12/14 log_debug event_nr

In reply to Zdenek Kabelac
---
 libdm/ioctl/libdm-iface.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
libdm/ioctl/libdm-iface.c +3 -2
index bbe1de4..3f8ffe9 100644
@@ -1870,7 +1870,7 @@ static struct dm_ioctl *_do_dm_ioctl(struct dm_task *dmt, unsigned command,
 	}
 
 	log_debug("dm %s %s %s%s%s %s%.0d%s%.0d%s"
-		  "%s%c%c%s%s %.0" PRIu64 " %s [%u]",
+		  "%s%c%c%s%s %.0" PRIu64 " %s [%u] en=0x%x",
 		  _cmd_data_v4[dmt->type].name,
 		  dmi->name, dmi->uuid, dmt->newname ? " " : "",
 		  dmt->newname ? dmt->newname : "",
@@ -1885,7 +1885,8 @@ static struct dm_ioctl *_do_dm_ioctl(struct dm_task *dmt, unsigned command,
 		  dmt->skip_lockfs ? "S " : "",
 		  dmt->query_inactive_table ? "I " : "",
 		  dmt->sector, _sanitise_message(dmt->message),
-		  dmi->data_size);
+		  dmi->data_size,
+		  dmi->event_nr);
 #ifdef DM_IOCTLS
 	if (ioctl(_control_fd, command, dmi) < 0) {
 		if (errno == ENXIO && ((dmt->type == DM_DEVICE_INFO) ||
Signature · 1 line
1.7.2
#13 lore raw

PATCH 13/14 Reorder read_ahead check

In reply to Zdenek Kabelac

Wait for udev for debug print - otherwise read_ahead check tries to access device not yet available through udev.

---
 tools/dmsetup.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
tools/dmsetup.c +7 -0
index 10b9e0b..2af9c28 100644
@@ -633,9 +633,16 @@ static int _create(int argc, char **argv, void *data __attribute__((unused)))
 
 	r = 1;
 
+	if (!_udev_cookie)
+		(void) dm_udev_wait(cookie);
+
 	if (_switches[VERBOSE_ARG])
 		r = _display_info(dmt);
 
+	dm_task_destroy(dmt);
+
+	return r;
+
       out:
 	if (!_udev_cookie)
 		(void) dm_udev_wait(cookie);
Signature · 1 line
1.7.2
#14 lore raw

PATCH 14/14 Proposal of different check for udev dir

In reply to Zdenek Kabelac

This is rather test code to check differently for 'udev' dir usage. Upstream implementation doesn't allow to use directories like /dev/shm as normal /tmp dirs as it thinks it starts with /dev so it is udev 'dev' dir.

Note - few different approaches are here - my prefered version is the one in dmsetup #if 1

New code allows only 'devdir' or 'devdir/' and simplifies code around.

Also as this code seems to be needed for dmsetup & lvm - it should be probably moved as a dm library function usable by both.

---
 tools/dmsetup.c    |   26 +++++++++++++++++++++++---
 tools/lvmcmdline.c |    4 +++-
 2 files changed, 26 insertions(+), 4 deletions(-)
tools/dmsetup.c +23 -3
index 2af9c28..3be96b4 100644
@@ -978,11 +978,30 @@ static int _set_up_udev_support(const char *dev_dir)
 	 * races between udev and libdevmapper but only in case udev "dev path"
 	 * is the same as "dev path" used by libdevmapper.
 	 */
-
 	/* There's always a slash at the end of dev_dir. But check udev_dev_dir! */
-	if (udev_dev_dir[udev_dev_dir_len - 1] != '/')
+#if 1
+	if ((strncmp(dev_dir, udev_dev_dir, udev_dev_dir_len) == 0) &&
+	    (dev_dir[udev_dev_dir_len] == '\0' ||
+	     (dev_dir[udev_dev_dir_len] == '/' &&
+	      dev_dir[udev_dev_dir_len + 1] == '\0'))) {
+		_udev_only = _udev_cookie;
+	} else {
+		log_debug("The path %s used for creating device nodes that is "
+			  "set via DM_DEV_DIR environment variable differs from "
+			  "the path %s that is used by udev. All warnings "
+			  "about udev not working correctly while processing "
+			  "particular nodes will be suppressed. These nodes "
+			  "and symlinks will be managed in each directory "
+			  "separately.", dev_dir, udev_dev_dir);
+		dm_udev_set_checking(0);
+		_udev_only = 0;
+	}
+#else
+	if (udev_dev_dir_len > 0 &&
+	    udev_dev_dir[udev_dev_dir_len - 1] != '/' &&
+	    strlen(dev_dir) == (udev_dev_dir_len + 1)) {
 		dirs_diff = strncmp(dev_dir, udev_dev_dir, udev_dev_dir_len);
-	else
+	} else
 		dirs_diff = strcmp(dev_dir, udev_dev_dir);
 
 	_udev_only = _udev_cookie && !dirs_diff;
@@ -997,6 +1016,7 @@ static int _set_up_udev_support(const char *dev_dir)
 			  "separately.", dev_dir, udev_dev_dir);
 		dm_udev_set_checking(0);
 	}
+#endif
 
 	udev_unref(udev);
 	return 1;
tools/lvmcmdline.c +3 -1
index 6c66fab..7a29e02 100644
@@ -945,7 +945,9 @@ static int _set_udev_checking(struct cmd_context *cmd)
 	udev_dev_dir_len = strlen(udev_dev_dir);
 
 	/* There's always a slash at the end of dev_dir. But check udev_dev_dir! */
-	if (udev_dev_dir[udev_dev_dir_len - 1] != '/')
+	if (strlen(cmd->dev_dir) > udev_dev_dir_len)
+		dirs_diff = 1;
+	else if (udev_dev_dir[udev_dev_dir_len - 1] != '/')
 		dirs_diff = strncmp(cmd->dev_dir, udev_dev_dir,
 				    udev_dev_dir_len);
 	else
Signature · 1 line
1.7.2
#15 lore raw

cover Miscellaneous fixes

In reply to Zdenek Kabelac

Looks OK, with the exception of patch 06 "Testsuit: add missing prepare_dmeventd" and 14 "Proposal of different check for udev dir". The former is just wrong and needs a different approach, the latter needs to be decided about (I am OK with Zden?k's preferred version, I suppose).

Yours,
Petr.