[PATCH 0/6] Misc code updates

[PATCH 0/6] Misc code updates

[Zdenek Kabelac]

Small patchset contains various bits of code I keep in front of my tree.

First strict-aliasing is a bit contraversional and needs bigger review.

Configure patch basicaly cleanups help strings with the use of
AC_HELP_STRING.

Nextbit patch is tiny code optimization.

Backtrace patch skips printing of backtrace in the log if the situation
is not an error case.

Memlock slightly modifies memory map printing.

Last patch for dmsetup returns '0' exist status when showing option help.


Zdenek Kabelac (6):
  Fix compiler warning about strict-aliasing break
  configure changes - AC_HELP_STRING, AH_TEMPLATE
  nextbit
  Do not show backtrace
  Better alligned debug message - memlock
  Exit status 0 for 'dmsetup -c -o help'

 configure                 |  100 +++++++++++++++++++++------------------------
 configure.in              |   76 +++++++++++++++++++---------------
 lib/activate/activate.c   |    6 ++-
 lib/device/device.c       |   18 ++++----
 lib/mm/memlock.c          |    7 ++-
 libdm/datastruct/bitset.c |    4 +-
 tools/dmsetup.c           |    1 +
 7 files changed, 109 insertions(+), 103 deletions(-)

[PATCH 1/6] Fix compiler warning about strict-aliasing break

[Zdenek Kabelac]

This patch not really tested - and just show a possible solution
for strict-alliasing error.

So patch is just a proposal of one of many solution.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 lib/device/device.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/lib/device/device.c b/lib/device/device.c
index 7f9f40c..d24e8b3 100644
--- a/lib/device/device.c
+++ b/lib/device/device.c
@@ -58,9 +58,11 @@ static int _has_partition_table(struct device *dev)
 {
 	int ret = 0;
 	unsigned p;
-	uint16_t buf[SECTOR_SIZE/sizeof(uint16_t)];
-	uint16_t *part_magic;
-	struct partition *part;
+	struct {
+		uint8_t skip[PART_OFFSET];
+		struct partition part[4];
+		uint16_t magic;
+	} __attribute__((packed)) buf; /* sizeof() == SECTOR_SIZE */
 
 	if (!dev_read(dev, UINT64_C(0), sizeof(buf), &buf))
 		return_0;
@@ -68,17 +70,15 @@ static int _has_partition_table(struct device *dev)
 	/* FIXME Check for other types of partition table too */
 
 	/* Check for msdos partition table */
-	part_magic = buf + PART_MAGIC_OFFSET/sizeof(buf[0]);
-	if ((*part_magic == xlate16(PART_MAGIC))) {
-		part = (struct partition *) (buf + PART_OFFSET/sizeof(buf[0]));
-		for (p = 0; p < 4; p++, part++) {
+	if (buf.magic == xlate16(PART_MAGIC)) {
+		for (p = 0; p < 4; ++p) {
 			/* Table is invalid if boot indicator not 0 or 0x80 */
-			if ((part->boot_ind & 0x7f)) {
+			if (buf.part[p].boot_ind & 0x7f) {
 				ret = 0;
 				break;
 			}
 			/* Must have at least one non-empty partition */
-			if (part->nr_sects)
+			if (buf.part[p].nr_sects)
 				ret = 1;
 		}
 	}
-- 
1.7.1.1

[PATCH 2/6] configure changes - AC_HELP_STRING, AH_TEMPLATE

[Zdenek Kabelac]

change don't -> do not

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 configure    |  100 +++++++++++++++++++++++++++------------------------------
 configure.in |   76 ++++++++++++++++++++++++--------------------
 2 files changed, 89 insertions(+), 87 deletions(-)

diff --git a/configure b/configure
index d11af6a..62e4588 100755
--- a/configure
+++ b/configure
@@ -1535,7 +1535,7 @@ Optional Features:
   --enable-udev_rules     Install rule files needed for udev synchronisation
   --enable-compat         Enable support for old device-mapper versions
   --enable-units-compat   Enable output compatibility with old versions that
-                          that don't use KiB-style unit suffixes
+                          that do not use KiB-style unit suffixes
   --disable-driver        Disable calls to device-mapper in the kernel
   --disable-o_direct      Disable O_DIRECT
   --enable-applib         Build application library
@@ -1575,7 +1575,8 @@ Optional Packages:
                            * all                   (autodetect)
                            * none                  (disable build)
                           TYPE=none
-  --with-cmirrord-pidfile=PATH    cmirrord pidfile [/var/run/cmirrord.pid]
+  --with-cmirrord-pidfile=PATH
+                          cmirrord pidfile [/var/run/cmirrord.pid]
   --with-optimisation=OPT C optimisation flag [OPT=-O2]
   --with-localedir=DIR    Translation files in DIR [PREFIX/share/locale]
   --with-confdir=DIR      Configuration files in DIR [/etc]
@@ -1584,13 +1585,20 @@ Optional Packages:
   --with-usrsbindir=DIR
   --with-udev-prefix=UPREFIX      Install udev rule files in UPREFIX [EPREFIX]
   --with-udevdir=DIR      udev rules in DIR [UPREFIX/lib/udev/rules.d]
-  --with-dmeventd-pidfile=PATH    dmeventd pidfile [/var/run/dmeventd.pid]
-  --with-dmeventd-path=PATH       dmeventd path [EPREFIX/sbin/dmeventd]
-  --with-default-system-dir=DIR        Default LVM system directory [/etc/lvm]
-  --with-default-archive-subdir=SUBDIR Default metadata archive subdir [archive]
-  --with-default-backup-subdir=SUBDIR  Default metadata backup subdir [backup]
-  --with-default-cache-subdir=SUBDIR   Default metadata cache subdir [cache]
-  --with-default-locking-dir=DIR       Default locking directory [/var/lock/lvm]
+  --with-dmeventd-pidfile=PATH
+                          dmeventd pidfile [/var/run/dmeventd.pid]
+  --with-dmeventd-path=PATH
+                          dmeventd path [EPREFIX/sbin/dmeventd]
+  --with-default-system-dir=DIR
+                          Default LVM system directory [/etc/lvm]
+  --with-default-archive-subdir=SUBDIR
+                          Default metadata archive subdir [archive]
+  --with-default-backup-subdir=SUBDIR
+                          Default metadata backup subdir [backup]
+  --with-default-cache-subdir=SUBDIR
+                          Default metadata cache subdir [cache]
+  --with-default-locking-dir=DIR
+                          Default locking directory [/var/lock/lvm]
   --with-interface=IFACE  Choose kernel interface (ioctl) [ioctl]
 
 Some influential environment variables:
@@ -13836,23 +13844,20 @@ $as_echo "$CMIRRORD" >&6; }
 BUILD_CMIRRORD=$CMIRRORD
 
 ################################################################################
-
-
 if test "x$BUILD_CMIRRORD" = xyes; then
 
 # Check whether --with-cmirrord-pidfile was given.
 if test "${with_cmirrord_pidfile+set}" = set; then
-  withval=$with_cmirrord_pidfile;  cat >>confdefs.h <<_ACEOF
-#define CMIRRORD_PIDFILE "$withval"
-_ACEOF
-
+  withval=$with_cmirrord_pidfile;  CMIRRORD_PIDFILE=$withval
 else
-   cat >>confdefs.h <<_ACEOF
-#define CMIRRORD_PIDFILE "/var/run/cmirrord.pid"
-_ACEOF
-
+   CMIRRORD_PIDFILE="/var/run/cmirrord.pid"
 fi
 
+
+cat >>confdefs.h <<_ACEOF
+#define CMIRRORD_PIDFILE "$CMIRRORD_PIDFILE"
+_ACEOF
+
 fi
 
 ################################################################################
@@ -17651,115 +17656,104 @@ _ACEOF
 fi
 
 ################################################################################
-
-
 if test "$BUILD_DMEVENTD" = yes; then
 
 # Check whether --with-dmeventd-pidfile was given.
 if test "${with_dmeventd_pidfile+set}" = set; then
-  withval=$with_dmeventd_pidfile;  cat >>confdefs.h <<_ACEOF
-#define DMEVENTD_PIDFILE "$withval"
-_ACEOF
-
+  withval=$with_dmeventd_pidfile;  DMEVENTD_PIDFILE=$withval
 else
-   cat >>confdefs.h <<_ACEOF
-#define DMEVENTD_PIDFILE "/var/run/dmeventd.pid"
-_ACEOF
-
+   DMEVENTD_PIDFILE="/var/run/dmeventd.pid"
 fi
 
-fi
 
+cat >>confdefs.h <<_ACEOF
+#define DMEVENTD_PIDFILE "$DMEVENTD_PIDFILE"
+_ACEOF
 
+fi
 
 if test "$BUILD_DMEVENTD" = yes; then
 
 # Check whether --with-dmeventd-path was given.
 if test "${with_dmeventd_path+set}" = set; then
-  withval=$with_dmeventd_path;  cat >>confdefs.h <<_ACEOF
-#define DMEVENTD_PATH "$withval"
-_ACEOF
-
+  withval=$with_dmeventd_path;  DMEVENTD_PATH=$withval
 else
-   cat >>confdefs.h <<_ACEOF
-#define DMEVENTD_PATH "$lvm_exec_prefix/sbin/dmeventd"
-_ACEOF
-
+   DMEVENTD_PATH="$lvm_exec_prefix/sbin/dmeventd"
 fi
 
-fi
 
-################################################################################
+cat >>confdefs.h <<_ACEOF
+#define DMEVENTD_PATH "$DMEVENTD_PATH"
+_ACEOF
 
+fi
 
+################################################################################
 
 # Check whether --with-default-system-dir was given.
 if test "${with_default_system_dir+set}" = set; then
-  withval=$with_default_system_dir;  DEFAULT_SYS_DIR="$withval"
+  withval=$with_default_system_dir;  DEFAULT_SYS_DIR=$withval
 else
    DEFAULT_SYS_DIR="/etc/lvm"
 fi
 
+
 cat >>confdefs.h <<_ACEOF
 #define DEFAULT_SYS_DIR "$DEFAULT_SYS_DIR"
 _ACEOF
 
 
 
-
-
 # Check whether --with-default-archive-subdir was given.
 if test "${with_default_archive_subdir+set}" = set; then
-  withval=$with_default_archive_subdir;  DEFAULT_ARCHIVE_SUBDIR="$withval"
+  withval=$with_default_archive_subdir;  DEFAULT_ARCHIVE_SUBDIR=$withval
 else
    DEFAULT_ARCHIVE_SUBDIR="archive"
 fi
 
+
 cat >>confdefs.h <<_ACEOF
 #define DEFAULT_ARCHIVE_SUBDIR "$DEFAULT_ARCHIVE_SUBDIR"
 _ACEOF
 
 
 
-
-
 # Check whether --with-default-backup-subdir was given.
 if test "${with_default_backup_subdir+set}" = set; then
-  withval=$with_default_backup_subdir;  DEFAULT_BACKUP_SUBDIR="$withval"
+  withval=$with_default_backup_subdir;  DEFAULT_BACKUP_SUBDIR=$withval
 else
    DEFAULT_BACKUP_SUBDIR="backup"
 fi
 
+
 cat >>confdefs.h <<_ACEOF
 #define DEFAULT_BACKUP_SUBDIR "$DEFAULT_BACKUP_SUBDIR"
 _ACEOF
 
 
 
-
-
 # Check whether --with-default-cache-subdir was given.
 if test "${with_default_cache_subdir+set}" = set; then
-  withval=$with_default_cache_subdir;  DEFAULT_CACHE_SUBDIR="$withval"
+  withval=$with_default_cache_subdir;  DEFAULT_CACHE_SUBDIR=$withval
 else
    DEFAULT_CACHE_SUBDIR="cache"
 fi
 
+
 cat >>confdefs.h <<_ACEOF
 #define DEFAULT_CACHE_SUBDIR "$DEFAULT_CACHE_SUBDIR"
 _ACEOF
 
 
 
-
-
 # Check whether --with-default-locking-dir was given.
 if test "${with_default_locking_dir+set}" = set; then
-  withval=$with_default_locking_dir;  DEFAULT_LOCK_DIR="$withval"
+  withval=$with_default_locking_dir;  DEFAULT_LOCK_DIR=$withval
 else
    DEFAULT_LOCK_DIR="/var/lock/lvm"
 fi
 
+
 cat >>confdefs.h <<_ACEOF
 #define DEFAULT_LOCK_DIR "$DEFAULT_LOCK_DIR"
 _ACEOF
diff --git a/configure.in b/configure.in
index a8a5b92..e35b027 100644
--- a/configure.in
+++ b/configure.in
@@ -599,12 +599,11 @@ BUILD_CMIRRORD=$CMIRRORD
 
 ################################################################################
 dnl -- cmirrord pidfile
-AH_TEMPLATE(CMIRRORD_PIDFILE, [Path to cmirrord pidfile.])
 if test "x$BUILD_CMIRRORD" = xyes; then
 	AC_ARG_WITH(cmirrord-pidfile,
-		    [  --with-cmirrord-pidfile=PATH    cmirrord pidfile [[/var/run/cmirrord.pid]] ],
-		    [ AC_DEFINE_UNQUOTED(CMIRRORD_PIDFILE,"$withval") ],
-		    [ AC_DEFINE_UNQUOTED(CMIRRORD_PIDFILE,"/var/run/cmirrord.pid") ])
+		    AC_HELP_STRING([--with-cmirrord-pidfile=PATH], [cmirrord pidfile [[/var/run/cmirrord.pid]] ]),
+		    [ CMIRRORD_PIDFILE=$withval ], [ CMIRRORD_PIDFILE="/var/run/cmirrord.pid" ])
+	AC_DEFINE_UNQUOTED(CMIRRORD_PIDFILE, ["$CMIRRORD_PIDFILE"], [Path to cmirrord pidfile.])
 fi
 
 ################################################################################
@@ -710,8 +709,8 @@ AC_ARG_ENABLE(compat,   [  --enable-compat         Enable support for old device
 ################################################################################
 dnl -- Compatible units suffix mode
 AC_ARG_ENABLE(units-compat,
-  [  --enable-units-compat   Enable output compatibility with old versions that
-                          that don't use KiB-style unit suffixes],
+  AC_HELP_STRING([--enable-units-compat], 
+		 [Enable output compatibility with old versions that that do not use KiB-style unit suffixes]),
   UNITS_COMPAT=$enableval, UNITS_COMPAT=no)
 
 if test x$UNITS_COMPAT = xyes; then
@@ -1042,58 +1041,67 @@ fi
 
 ################################################################################
 dnl -- dmeventd pidfile and executable path
-AH_TEMPLATE(DMEVENTD_PIDFILE, [Path to dmeventd pidfile.])
 if test "$BUILD_DMEVENTD" = yes; then
 	AC_ARG_WITH(dmeventd-pidfile,
-		    [  --with-dmeventd-pidfile=PATH    dmeventd pidfile [[/var/run/dmeventd.pid]] ],
-		    [ AC_DEFINE_UNQUOTED(DMEVENTD_PIDFILE,"$withval") ],
-		    [ AC_DEFINE_UNQUOTED(DMEVENTD_PIDFILE,"/var/run/dmeventd.pid") ])
+		    AC_HELP_STRING([--with-dmeventd-pidfile=PATH],
+				   [dmeventd pidfile [[/var/run/dmeventd.pid]]]),
+		    [ DMEVENTD_PIDFILE=$withval ],
+		    [ DMEVENTD_PIDFILE="/var/run/dmeventd.pid" ])
+	AC_DEFINE_UNQUOTED(DMEVENTD_PIDFILE, ["$DMEVENTD_PIDFILE"],
+			   [Path to dmeventd pidfile.])
 fi
 
-AH_TEMPLATE(DMEVENTD_PATH, [Path to dmeventd binary.])
 if test "$BUILD_DMEVENTD" = yes; then
 	AC_ARG_WITH(dmeventd-path,
-		    [  --with-dmeventd-path=PATH       dmeventd path [[EPREFIX/sbin/dmeventd]] ],
-		    [ AC_DEFINE_UNQUOTED(DMEVENTD_PATH,"$withval") ],
-		    [ AC_DEFINE_UNQUOTED(DMEVENTD_PATH,"$lvm_exec_prefix/sbin/dmeventd") ])
+		    AC_HELP_STRING([--with-dmeventd-path=PATH],
+				   [dmeventd path [[EPREFIX/sbin/dmeventd]]]),
+		    [ DMEVENTD_PATH=$withval ], 
+		    [ DMEVENTD_PATH="$lvm_exec_prefix/sbin/dmeventd" ])
+	AC_DEFINE_UNQUOTED(DMEVENTD_PATH, ["$DMEVENTD_PATH"],
+			   [Path to dmeventd binary.])
 fi
 
 ################################################################################
 dnl -- various defaults
-AH_TEMPLATE(DEFAULT_SYS_DIR, [Path to LVM system directory.])
 AC_ARG_WITH(default-system-dir,
-	    [  --with-default-system-dir=DIR        Default LVM system directory [[/etc/lvm]] ],
-	    [ DEFAULT_SYS_DIR="$withval" ],
+	    AC_HELP_STRING([--with-default-system-dir=DIR],
+			   [Default LVM system directory [[/etc/lvm]]]),
+	    [ DEFAULT_SYS_DIR=$withval ],
 	    [ DEFAULT_SYS_DIR="/etc/lvm" ])
-AC_DEFINE_UNQUOTED(DEFAULT_SYS_DIR,["$DEFAULT_SYS_DIR"] )
+AC_DEFINE_UNQUOTED(DEFAULT_SYS_DIR, ["$DEFAULT_SYS_DIR"],
+		   [Path to LVM system directory.])
 
-AH_TEMPLATE(DEFAULT_ARCHIVE_SUBDIR, [Name of default metadata archive subdirectory.])
 AC_ARG_WITH(default-archive-subdir,
-	    [  --with-default-archive-subdir=SUBDIR Default metadata archive subdir [[archive]] ],
-	    [ DEFAULT_ARCHIVE_SUBDIR="$withval" ],
+	    AC_HELP_STRING([--with-default-archive-subdir=SUBDIR],
+			   [Default metadata archive subdir [[archive]]]),
+	    [ DEFAULT_ARCHIVE_SUBDIR=$withval ],
 	    [ DEFAULT_ARCHIVE_SUBDIR="archive" ])
-AC_DEFINE_UNQUOTED(DEFAULT_ARCHIVE_SUBDIR,["$DEFAULT_ARCHIVE_SUBDIR"])
+AC_DEFINE_UNQUOTED(DEFAULT_ARCHIVE_SUBDIR, ["$DEFAULT_ARCHIVE_SUBDIR"],
+		   [Name of default metadata archive subdirectory.])
 
-AH_TEMPLATE(DEFAULT_BACKUP_SUBDIR, [Name of default metadata backup subdirectory.])
 AC_ARG_WITH(default-backup-subdir,
-	    [  --with-default-backup-subdir=SUBDIR  Default metadata backup subdir [[backup]] ],
-	    [ DEFAULT_BACKUP_SUBDIR="$withval" ],
+	    AC_HELP_STRING([--with-default-backup-subdir=SUBDIR],
+			   [Default metadata backup subdir [[backup]]]),
+	    [ DEFAULT_BACKUP_SUBDIR=$withval ],
 	    [ DEFAULT_BACKUP_SUBDIR="backup" ])
-AC_DEFINE_UNQUOTED(DEFAULT_BACKUP_SUBDIR,["$DEFAULT_BACKUP_SUBDIR"] )
+AC_DEFINE_UNQUOTED(DEFAULT_BACKUP_SUBDIR, ["$DEFAULT_BACKUP_SUBDIR"],
+                   [Name of default metadata backup subdirectory.])
 
-AH_TEMPLATE(DEFAULT_CACHE_SUBDIR, [Name of default metadata cache subdirectory.])
 AC_ARG_WITH(default-cache-subdir,
-	    [  --with-default-cache-subdir=SUBDIR   Default metadata cache subdir [[cache]] ],
-	    [ DEFAULT_CACHE_SUBDIR="$withval" ],
+	    AC_HELP_STRING([--with-default-cache-subdir=SUBDIR],
+			   [Default metadata cache subdir [[cache]]]),
+	    [ DEFAULT_CACHE_SUBDIR=$withval ],
 	    [ DEFAULT_CACHE_SUBDIR="cache" ])
-AC_DEFINE_UNQUOTED(DEFAULT_CACHE_SUBDIR,["$DEFAULT_CACHE_SUBDIR"] )
+AC_DEFINE_UNQUOTED(DEFAULT_CACHE_SUBDIR, ["$DEFAULT_CACHE_SUBDIR"],
+		   [Name of default metadata cache subdirectory.])
 
-AH_TEMPLATE(DEFAULT_LOCK_DIR, [Name of default locking directory.])
 AC_ARG_WITH(default-locking-dir,
-	    [  --with-default-locking-dir=DIR       Default locking directory [[/var/lock/lvm]] ],
-	    [ DEFAULT_LOCK_DIR="$withval" ],
+	    AC_HELP_STRING([--with-default-locking-dir=DIR],
+			   [Default locking directory [[/var/lock/lvm]]]),
+	    [ DEFAULT_LOCK_DIR=$withval ],
 	    [ DEFAULT_LOCK_DIR="/var/lock/lvm" ])
-AC_DEFINE_UNQUOTED(DEFAULT_LOCK_DIR,["$DEFAULT_LOCK_DIR"] )
+AC_DEFINE_UNQUOTED(DEFAULT_LOCK_DIR, ["$DEFAULT_LOCK_DIR"],
+		   [Name of default locking directory.])
 
 ################################################################################
 dnl -- which kernel interface to use (ioctl only)
-- 
1.7.1.1

[PATCH 3/6] nextbit

[Zdenek Kabelac]

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 libdm/datastruct/bitset.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libdm/datastruct/bitset.c b/libdm/datastruct/bitset.c
index 9b8aff9..563f684 100644
--- a/libdm/datastruct/bitset.c
+++ b/libdm/datastruct/bitset.c
@@ -69,9 +69,9 @@ void dm_bit_union(dm_bitset_t out, dm_bitset_t in1, dm_bitset_t in2)
 
 static int _test_word(uint32_t test, int bit)
 {
-	int next_set_bit;
+	uint32_t tb = test >> bit;
 
-	return ((next_set_bit = ffs(test >> bit)) ? next_set_bit + bit - 1 : -1);
+	return (tb ? ffs(tb) + bit - 1 : -1);
 }
 
 int dm_bit_get_next(dm_bitset_t bs, int last_bit)
-- 
1.7.1.1

[PATCH 4/6] Do not show backtrace

[Zdenek Kabelac]

When we expect this state and behavior is correct.

FIXME: Add resonable error message instead of 'goto_out'

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 lib/activate/activate.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/activate/activate.c b/lib/activate/activate.c
index 2128c41..7edd728 100644
--- a/lib/activate/activate.c
+++ b/lib/activate/activate.c
@@ -961,8 +961,10 @@ static int _lv_resume(struct cmd_context *cmd, const char *lvid_s,
 		goto_out;
 
 	if (!info.exists || !info.suspended) {
-		r = error_if_not_active ? 0 : 1;
-		goto_out;
+		if (error_if_not_active)
+			goto_out;
+		r = 1;
+		goto out;
 	}
 
 	if (!_lv_activate_lv(lv))
-- 
1.7.1.1

[PATCH 5/6] Better alligned debug message - memlock

[Zdenek Kabelac]

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 lib/mm/memlock.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/mm/memlock.c b/lib/mm/memlock.c
index bc18c0d..de1f777 100644
--- a/lib/mm/memlock.c
+++ b/lib/mm/memlock.c
@@ -137,7 +137,8 @@ static int _maps_line(struct cmd_context *cmd, lvmlock_t lock,
 
 	/* Select readable maps */
 	if (fr != 'r') {
-		log_debug("mlock area unreadable '%s': Skipping.", line);
+		log_debug("m%slock area unreadable %s : Skipping.",
+			  (lock == LVM_MLOCK) ? "" : "un", line);
 		return 1;
 	}
 
@@ -171,8 +172,8 @@ static int _maps_line(struct cmd_context *cmd, lvmlock_t lock,
 	}
 
 	*mstats += sz;
-	log_debug("%s %10ldKiB %12lx - %12lx %c%c%c%c %s",
-		  (lock == LVM_MLOCK) ? "mlock" : "munlock",
+	log_debug("m%slock %10ldKiB %12lx - %12lx %c%c%c%c%s",
+		  (lock == LVM_MLOCK) ? "" : "un",
 		  ((long)sz + 1023) / 1024, from, to, fr, fw, fx, fp, line + pos);
 
 	if (lock == LVM_MLOCK) {
-- 
1.7.1.1

[PATCH 6/6] Exit status 0 for 'dmsetup -c -o help'

[Zdenek Kabelac]

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 tools/dmsetup.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tools/dmsetup.c b/tools/dmsetup.c
index 5e71542..479cbb0 100644
--- a/tools/dmsetup.c
+++ b/tools/dmsetup.c
@@ -3340,6 +3340,7 @@ int main(int argc, char **argv)
 	}
 
 	if (argc == 0) {
+		r = 0;
 		_usage(stderr);
 		goto out;
 	}
-- 
1.7.1.1

[PATCH 2/6] configure changes - AC_HELP_STRING, AH_TEMPLATE

[Alasdair G Kergon]

Whatever.

Ack.

Alasdair

[PATCH 6/6] Exit status 0 for 'dmsetup -c -o help'

[Alasdair G Kergon]

On Wed, Jul 07, 2010 at 02:29:36PM +0200, Zdenek Kabelac wrote:
> diff --git a/tools/dmsetup.c b/tools/dmsetup.c
> index 5e71542..479cbb0 100644
> --- a/tools/dmsetup.c
> +++ b/tools/dmsetup.c
> @@ -3340,6 +3340,7 @@ int main(int argc, char **argv)
>  	}
>  
>  	if (argc == 0) {
> +		r = 0;
>  		_usage(stderr);
>  		goto out;

How does that work?

# dmsetup

Should give failure status.
(It already does.)

# dmsetup help

Should give success.
(It already does.)

# dmsetup -c -o help

Should give success.
(It doesn't.)

Alasdair

[PATCH 5/6] Better alligned debug message - memlock

[Alasdair G Kergon]

Ack.

On Wed, Jul 07, 2010 at 02:29:35PM +0200, Zdenek Kabelac wrote:
> -	log_debug("%s %10ldKiB %12lx - %12lx %c%c%c%c %s",
> -		  (lock == LVM_MLOCK) ? "mlock" : "munlock",
> +	log_debug("m%slock %10ldKiB %12lx - %12lx %c%c%c%c%s",
> +		  (lock == LVM_MLOCK) ? "" : "un",

?!

Alasdair

[PATCH 3/6] nextbit

[Alasdair G Kergon]

Ack

Alasdair

[PATCH 4/6] Do not show backtrace

[Alasdair G Kergon]

Ack.

Alasdair

[PATCH 6/6] Exit status 0 for 'dmsetup -c -o help'

[Zdenek Kabelac]

Dne 7.7.2010 20:16, Alasdair G Kergon napsal(a):
> On Wed, Jul 07, 2010 at 02:29:36PM +0200, Zdenek Kabelac wrote:
>> diff --git a/tools/dmsetup.c b/tools/dmsetup.c
>> index 5e71542..479cbb0 100644
>> --- a/tools/dmsetup.c
>> +++ b/tools/dmsetup.c
>> @@ -3340,6 +3340,7 @@ int main(int argc, char **argv)
>>   	}
>>
>>   	if (argc == 0) {
>> +		r = 0;
>>   		_usage(stderr);
>>   		goto out;
>
> How does that work?
>
> # dmsetup
>
> Should give failure status.
> (It already does.)
>
> # dmsetup help
>
> Should give success.
> (It already does.)
>
> # dmsetup -c -o help
>
> Should give success.
> (It doesn't.)
>


Here is second try to address only this explicit problem:

_report_init() is able to return 1 for 'help'/'?'
(same check is in libdm-report.c)
Following checks detects that 'info' invoked it.

What still looks weird is 'dmsetup ls -c -o help' - which returns 1 - but 
displays meaningless help message for this command, (but this is not 
influenced by this patch).

Zdenek


Index: tools/dmsetup.c
===================================================================
RCS file: /cvs/lvm2/LVM2/tools/dmsetup.c,v
retrieving revision 1.141
diff -u -p -r1.141 dmsetup.c
--- tools/dmsetup.c	8 Jul 2010 14:29:28 -0000	1.141
+++ tools/dmsetup.c	8 Jul 2010 15:40:35 -0000
@@ -2652,6 +2652,9 @@ static int _report_init(struct command *
  	r = 1;

  out:
+	if (!strcasecmp(options, "help") || !strcmp(options, "?"))
+		r = 1;
+
  	if (len)
  		dm_free(options);

@@ -3360,8 +3363,15 @@ int main(int argc, char **argv)
  	if (!_switches[COLS_ARG] && !strcmp(c->name, "splitname"))
  		_switches[COLS_ARG]++;

-	if (_switches[COLS_ARG] && !_report_init(c))
-		goto out;
+	if (_switches[COLS_ARG]) {
+		if (!_report_init(c))
+			goto out;
+		if (!_report) {
+			if (!strcmp(c->name, "info"))
+				r = 0;  /* info -c -o help */
+			goto out;
+		}
+	}

  	#ifdef UDEV_SYNC_SUPPORT
  	if (!_set_up_udev_support(dev_dir))

[PATCH 1/6] Fix compiler warning about strict-aliasing break

[Petr Rockai]

Zdenek Kabelac <zkabelac@redhat.com> writes:

> This patch not really tested - and just show a possible solution
> for strict-alliasing error.
>
> So patch is just a proposal of one of many solution.

The patch looks OK, the code is actually cleaner that way. You can also
drop PART_MAGIC_OFFSET which is no longer used (I have checked that it
is indeed PART_OFFSET + 4*sizeof(struct partition)).

I believe the code is equivalent to what was there before. Nevertheless,
a regression test that would cover this area would be more than welcome.

Reviewed-By: Petr Rockai <prockai@redhat.com>

Yours,
  Petr.