* [PATCH 0/6] Misc code updates @ 2010-07-07 12:29 Zdenek Kabelac 2010-07-07 12:29 ` [PATCH 1/6] Fix compiler warning about strict-aliasing break Zdenek Kabelac ` (5 more replies) 0 siblings, 6 replies; 14+ messages in thread From: Zdenek Kabelac @ 2010-07-07 12:29 UTC (permalink / raw) To: lvm-devel 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(-) ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/6] Fix compiler warning about strict-aliasing break 2010-07-07 12:29 [PATCH 0/6] Misc code updates Zdenek Kabelac @ 2010-07-07 12:29 ` Zdenek Kabelac 2010-10-06 15:04 ` Petr Rockai 2010-07-07 12:29 ` [PATCH 2/6] configure changes - AC_HELP_STRING, AH_TEMPLATE Zdenek Kabelac ` (4 subsequent siblings) 5 siblings, 1 reply; 14+ messages in thread From: Zdenek Kabelac @ 2010-07-07 12:29 UTC (permalink / raw) To: lvm-devel 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 1/6] Fix compiler warning about strict-aliasing break 2010-07-07 12:29 ` [PATCH 1/6] Fix compiler warning about strict-aliasing break Zdenek Kabelac @ 2010-10-06 15:04 ` Petr Rockai 0 siblings, 0 replies; 14+ messages in thread From: Petr Rockai @ 2010-10-06 15:04 UTC (permalink / raw) To: lvm-devel 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/6] configure changes - AC_HELP_STRING, AH_TEMPLATE 2010-07-07 12:29 [PATCH 0/6] Misc code updates Zdenek Kabelac 2010-07-07 12:29 ` [PATCH 1/6] Fix compiler warning about strict-aliasing break Zdenek Kabelac @ 2010-07-07 12:29 ` Zdenek Kabelac 2010-07-07 18:10 ` Alasdair G Kergon 2010-07-07 12:29 ` [PATCH 3/6] nextbit Zdenek Kabelac ` (3 subsequent siblings) 5 siblings, 1 reply; 14+ messages in thread From: Zdenek Kabelac @ 2010-07-07 12:29 UTC (permalink / raw) To: lvm-devel 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/6] configure changes - AC_HELP_STRING, AH_TEMPLATE 2010-07-07 12:29 ` [PATCH 2/6] configure changes - AC_HELP_STRING, AH_TEMPLATE Zdenek Kabelac @ 2010-07-07 18:10 ` Alasdair G Kergon 0 siblings, 0 replies; 14+ messages in thread From: Alasdair G Kergon @ 2010-07-07 18:10 UTC (permalink / raw) To: lvm-devel Whatever. Ack. Alasdair ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/6] nextbit 2010-07-07 12:29 [PATCH 0/6] Misc code updates Zdenek Kabelac 2010-07-07 12:29 ` [PATCH 1/6] Fix compiler warning about strict-aliasing break Zdenek Kabelac 2010-07-07 12:29 ` [PATCH 2/6] configure changes - AC_HELP_STRING, AH_TEMPLATE Zdenek Kabelac @ 2010-07-07 12:29 ` Zdenek Kabelac 2010-07-07 18:49 ` Alasdair G Kergon 2010-07-07 12:29 ` [PATCH 4/6] Do not show backtrace Zdenek Kabelac ` (2 subsequent siblings) 5 siblings, 1 reply; 14+ messages in thread From: Zdenek Kabelac @ 2010-07-07 12:29 UTC (permalink / raw) To: lvm-devel 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/6] nextbit 2010-07-07 12:29 ` [PATCH 3/6] nextbit Zdenek Kabelac @ 2010-07-07 18:49 ` Alasdair G Kergon 0 siblings, 0 replies; 14+ messages in thread From: Alasdair G Kergon @ 2010-07-07 18:49 UTC (permalink / raw) To: lvm-devel Ack Alasdair ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/6] Do not show backtrace 2010-07-07 12:29 [PATCH 0/6] Misc code updates Zdenek Kabelac ` (2 preceding siblings ...) 2010-07-07 12:29 ` [PATCH 3/6] nextbit Zdenek Kabelac @ 2010-07-07 12:29 ` Zdenek Kabelac 2010-07-07 18:54 ` Alasdair G Kergon 2010-07-07 12:29 ` [PATCH 5/6] Better alligned debug message - memlock Zdenek Kabelac 2010-07-07 12:29 ` [PATCH 6/6] Exit status 0 for 'dmsetup -c -o help' Zdenek Kabelac 5 siblings, 1 reply; 14+ messages in thread From: Zdenek Kabelac @ 2010-07-07 12:29 UTC (permalink / raw) To: lvm-devel 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/6] Do not show backtrace 2010-07-07 12:29 ` [PATCH 4/6] Do not show backtrace Zdenek Kabelac @ 2010-07-07 18:54 ` Alasdair G Kergon 0 siblings, 0 replies; 14+ messages in thread From: Alasdair G Kergon @ 2010-07-07 18:54 UTC (permalink / raw) To: lvm-devel Ack. Alasdair ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 5/6] Better alligned debug message - memlock 2010-07-07 12:29 [PATCH 0/6] Misc code updates Zdenek Kabelac ` (3 preceding siblings ...) 2010-07-07 12:29 ` [PATCH 4/6] Do not show backtrace Zdenek Kabelac @ 2010-07-07 12:29 ` Zdenek Kabelac 2010-07-07 18:19 ` Alasdair G Kergon 2010-07-07 12:29 ` [PATCH 6/6] Exit status 0 for 'dmsetup -c -o help' Zdenek Kabelac 5 siblings, 1 reply; 14+ messages in thread From: Zdenek Kabelac @ 2010-07-07 12:29 UTC (permalink / raw) To: lvm-devel 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 5/6] Better alligned debug message - memlock 2010-07-07 12:29 ` [PATCH 5/6] Better alligned debug message - memlock Zdenek Kabelac @ 2010-07-07 18:19 ` Alasdair G Kergon 0 siblings, 0 replies; 14+ messages in thread From: Alasdair G Kergon @ 2010-07-07 18:19 UTC (permalink / raw) To: lvm-devel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 6/6] Exit status 0 for 'dmsetup -c -o help' 2010-07-07 12:29 [PATCH 0/6] Misc code updates Zdenek Kabelac ` (4 preceding siblings ...) 2010-07-07 12:29 ` [PATCH 5/6] Better alligned debug message - memlock Zdenek Kabelac @ 2010-07-07 12:29 ` Zdenek Kabelac 2010-07-07 18:16 ` Alasdair G Kergon 5 siblings, 1 reply; 14+ messages in thread From: Zdenek Kabelac @ 2010-07-07 12:29 UTC (permalink / raw) To: lvm-devel 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 6/6] Exit status 0 for 'dmsetup -c -o help' 2010-07-07 12:29 ` [PATCH 6/6] Exit status 0 for 'dmsetup -c -o help' Zdenek Kabelac @ 2010-07-07 18:16 ` Alasdair G Kergon 2010-07-08 15:46 ` Zdenek Kabelac 0 siblings, 1 reply; 14+ messages in thread From: Alasdair G Kergon @ 2010-07-07 18:16 UTC (permalink / raw) To: lvm-devel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 6/6] Exit status 0 for 'dmsetup -c -o help' 2010-07-07 18:16 ` Alasdair G Kergon @ 2010-07-08 15:46 ` Zdenek Kabelac 0 siblings, 0 replies; 14+ messages in thread From: Zdenek Kabelac @ 2010-07-08 15:46 UTC (permalink / raw) To: lvm-devel 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)) ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2010-10-06 15:04 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-07-07 12:29 [PATCH 0/6] Misc code updates Zdenek Kabelac 2010-07-07 12:29 ` [PATCH 1/6] Fix compiler warning about strict-aliasing break Zdenek Kabelac 2010-10-06 15:04 ` Petr Rockai 2010-07-07 12:29 ` [PATCH 2/6] configure changes - AC_HELP_STRING, AH_TEMPLATE Zdenek Kabelac 2010-07-07 18:10 ` Alasdair G Kergon 2010-07-07 12:29 ` [PATCH 3/6] nextbit Zdenek Kabelac 2010-07-07 18:49 ` Alasdair G Kergon 2010-07-07 12:29 ` [PATCH 4/6] Do not show backtrace Zdenek Kabelac 2010-07-07 18:54 ` Alasdair G Kergon 2010-07-07 12:29 ` [PATCH 5/6] Better alligned debug message - memlock Zdenek Kabelac 2010-07-07 18:19 ` Alasdair G Kergon 2010-07-07 12:29 ` [PATCH 6/6] Exit status 0 for 'dmsetup -c -o help' Zdenek Kabelac 2010-07-07 18:16 ` Alasdair G Kergon 2010-07-08 15:46 ` Zdenek Kabelac
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.