Skip to contentSkip to conversation

Pvmin, udev, testing

PATCH New Zdenek Kabelac 2 participants 17 messages Jan 24, 2011 – Jan 30, 2011

lore
Share & tools
lore
#0 lore raw

cover Pvmin, udev, testing

Patchset combince few 'smaller' changes which are trying to fix couple problem with current git tree.

0001-0002 - add configurable pv_min_size

0003-0005 - fix udev problem with stacking multiple dev ops.

0006 - code optimization

0007-0011 - testing speedup

0012 - coverage improvement

0013 - man page fix proposal - we should decide which way to go

0014-0015 - gcc compilation warning fixes

Zdenek Kabelac (15):
Add find_config_tree_int64 function Replace PV_MIN_SIZE with function pv_min_size() Test pv_min_size functionality
Move add_dev_node to DM_DEVICE_RESUME
Fix multiple operations on the same node Micro optimization of _eat_space and _get_token
Testing: Display duration of tests
Testing: Accelerate mirror tests
Testing: Add udev_wait for sfdisk
Testing: Lower mem usage of inconsistent mda test
Testing: Lower device size of pvmove test Skip compilation of debug code
Man page consistency fixes
Void* arithmetic replaced with char*
const fixing

 daemons/clvmd/clvmd.c            |    4 +-
 daemons/clvmd/clvmd.h            |    2 +-
 doc/example.conf.in              |    5 +++
 lib/activate/activate.c          |    4 +-
 lib/activate/activate.h          |    2 +-
 lib/activate/fs.c                |   49 +++++++++++++++++++++++++++--
 lib/commands/toolcontext.c       |    8 ++++-
 lib/config/config.c              |   42 ++++++++++++++-----------
 lib/config/config.h              |    2 +
 lib/filters/filter-regex.c       |    2 +-
 lib/filters/filter.c             |    2 +-
 lib/format_text/text_label.c     |    4 +-
 lib/metadata/lv.h                |    2 +-
 lib/metadata/lv_manip.c          |    3 +-
 lib/metadata/metadata-exported.h |    9 ++---
 lib/metadata/metadata.c          |   33 +++++++++++--------
 lib/metadata/metadata.h          |    1 -
 lib/metadata/mirror.c            |   18 +++++-----
 lib/misc/lvm-globals.c           |   12 +++++++
 lib/misc/lvm-globals.h           |    5 ++-
 lib/report/properties.c          |    3 +-
 lib/report/report.c              |    6 ++--
 libdm/ioctl/libdm-iface.c        |    8 ++--
 libdm/libdm-common.c             |   57 +++++++++++++++++++++++++++-------
 libdm/mm/dbg_malloc.c            |    4 ++
 liblvm/lvm_vg.c                  |    4 +-
 man/pvcreate.8.in                |   63 +++++++++++++++++--------------------
 man/pvremove.8.in                |   15 +++++---
 man/pvresize.8.in                |   13 +++----
 man/pvscan.8.in                  |   14 +++-----
 test/api/percent.sh              |    2 +-
 test/lib/aux.sh                  |   10 ++++++
 test/lib/check.sh                |    4 ++-
 test/lib/harness.c               |   32 +++++++++++++------
 test/t-inconsistent-metadata.sh  |    2 +-
 test/t-lvconvert-mirror-basic.sh |    6 ++--
 test/t-pv-min-size.sh            |   25 +++++++++++++++
 test/t-pvcreate-operation-md.sh  |    8 ++++-
 test/t-pvmove-basic.sh           |   24 ++++++++------
 tools/lvcreate.c                 |    2 +-
 tools/pvresize.c                 |    6 ++--
 tools/reporter.c                 |    4 +-
 tools/toollib.c                  |   10 +++---
 tools/toollib.h                  |    4 +-
 tools/vgcfgrestore.c             |    2 +-
 tools/vgcreate.c                 |    2 +-
 tools/vgextend.c                 |    4 +-
 tools/vgmerge.c                  |    2 +-
 tools/vgreduce.c                 |    2 +-
 tools/vgrename.c                 |    2 +-
 tools/vgsplit.c                  |    2 +-
 51 files changed, 360 insertions(+), 191 deletions(-)
 create mode 100755 test/t-pv-min-size.sh
Signature · 1 line
1.7.3.5
#1 lore raw

PATCH 1/15 Add find_config_tree_int64 function

Add function for reading 64bit ints.

---
 lib/config/config.c |    6 ++++++
 lib/config/config.h |    2 ++
 2 files changed, 8 insertions(+), 0 deletions(-)
lib/config/config.c +6 -0
index 72908f2..15bf401 100644
@@ -1047,6 +1047,12 @@ int find_config_tree_int(struct cmd_context *cmd, const char *path,
 	return (int) _find_config_int64(cmd->cft_override ? cmd->cft_override->root : NULL, cmd->cft->root, path, (int64_t) fail);
 }
 
+int64_t find_config_tree_int64(struct cmd_context *cmd, const char *path, int64_t fail)
+{
+	return _find_config_int64(cmd->cft_override ? cmd->cft_override->root : NULL,
+				  cmd->cft->root, path, fail);
+}
+
 float find_config_tree_float(struct cmd_context *cmd, const char *path,
 			     float fail)
 {
lib/config/config.h +2 -0
index f70deb0..ae0c6e6 100644
@@ -95,6 +95,8 @@ const char *find_config_tree_str(struct cmd_context *cmd,
 				 const char *path, const char *fail);
 int find_config_tree_int(struct cmd_context *cmd, const char *path,
 			 int fail);
+int64_t find_config_tree_int64(struct cmd_context *cmd, const char *path,
+			     int64_t fail);
 float find_config_tree_float(struct cmd_context *cmd, const char *path,
 			     float fail);
 
Signature · 1 line
1.7.3.5
#2 lore raw

PATCH 2/15 Replace PV_MIN_SIZE with function pv_min_size()

In reply to Zdenek Kabelac

Add configurable option for defining minimal size of PV. pv_min_size() is added to lvm-globals and it's being initialized through _init_globals.

Macro PV_MIN_SIZE is unusued and removed.
Add new define DEFAULT_PV_MIN_SIZE_KB used in lvm-global
- unlike PV_MIN_SIZE it uses Kilobytes units.

Should help user with various slow devices attached to the system, which cannot be easily filtered out:
https://bugzilla.redhat.com/show_bug.cgi?id=644578

---
 doc/example.conf.in              |    5 +++++
 lib/commands/toolcontext.c       |    8 +++++++-
 lib/filters/filter.c             |    2 +-
 lib/metadata/metadata-exported.h |    1 -
 lib/metadata/metadata.c          |    6 +++---
 lib/metadata/metadata.h          |    1 -
 lib/misc/lvm-globals.c           |   12 ++++++++++++
 lib/misc/lvm-globals.h           |    5 ++++-
 tools/pvresize.c                 |    6 +++---
 9 files changed, 35 insertions(+), 11 deletions(-)
doc/example.conf.in +5 -0
index 8504f5d..4853862 100644
@@ -144,6 +144,11 @@ devices {
 
     # Allow use of pvcreate --uuid without requiring --restorefile.
     require_restorefile_with_uuid = 1
+
+    # Minimal size (in KB) of PV partion which will be consider to be able
+    # to hold PV. It allows to automatically skip too small partitions
+    # like FDD devices from scan. Any value smaller then 512 is ignored.
+    pv_min_size = 512
 }
 
 # This section allows you to configure the way in which LVM selects
lib/commands/toolcontext.c +7 -1
index 4799016..5d99137 100644
@@ -1111,9 +1111,15 @@ static void _init_rand(struct cmd_context *cmd)
 
 static void _init_globals(struct cmd_context *cmd)
 {
+	int64_t pvmin = find_config_tree_int64(cmd, "devices/pv_min_size",
+					       DEFAULT_PV_MIN_SIZE_KB);
+	if (pvmin < DEFAULT_PV_MIN_SIZE_KB)
+		log_warn("Ignoring too small pv_min_size = %" PRId64, pvmin);
+	else
+		/* lvm internally works with device size in sectors */
+		init_pv_min_size((uint64_t)pvmin * 1024 >> SECTOR_SHIFT);
 	init_full_scan_done(0);
 	init_mirror_in_sync(0);
-
 }
 
 /* Entry point */
lib/filters/filter.c +1 -1
index 58d7c79..314a1f0 100644
@@ -158,7 +158,7 @@ static int _passes_lvm_type_device_filter(struct dev_filter *f __attribute__((un
 		goto out;
 	}
 
-	if (size < PV_MIN_SIZE) {
+	if (size < pv_min_size()) {
 		log_debug("%s: Skipping: Too small to hold a PV", name);
 		goto out;
 	}
lib/metadata/metadata-exported.h +0 -1
index a9709d6..4e2463a 100644
@@ -33,7 +33,6 @@
 #define STRIPE_SIZE_MIN ( (unsigned) lvm_getpagesize() >> SECTOR_SHIFT)	/* PAGESIZE in sectors */
 #define STRIPE_SIZE_MAX ( 512L * 1024L >> SECTOR_SHIFT)	/* 512 KB in sectors */
 #define STRIPE_SIZE_LIMIT ((UINT_MAX >> 2) + 1)
-#define PV_MIN_SIZE ( 512L * 1024L >> SECTOR_SHIFT)	/* 512 KB in sectors */
 #define MAX_RESTRICTED_LVS 255	/* Used by FMT_RESTRICTED_LVIDS */
 
 /* Layer suffix */
lib/metadata/metadata.c +3 -3
index 771cdb0..a9c7830 100644
@@ -1626,9 +1626,9 @@ struct physical_volume *pv_create(const struct cmd_context *cmd,
 		pv->size = size;
 	}
 
-	if (pv->size < PV_MIN_SIZE) {
-		log_error("%s: Size must exceed minimum of %ld sectors.",
-			  pv_dev_name(pv), PV_MIN_SIZE);
+	if (pv->size < pv_min_size()) {
+		log_error("%s: Size must exceed minimum of %" PRIu64 " sectors.",
+			  pv_dev_name(pv), pv_min_size());
 		goto bad;
 	}
 
lib/metadata/metadata.h +0 -1
index c0f9148..3d51c5b 100644
@@ -32,7 +32,6 @@
 //#define STRIPE_SIZE_MIN ( (unsigned) lvm_getpagesize() >> SECTOR_SHIFT)	/* PAGESIZE in sectors */
 //#define STRIPE_SIZE_MAX ( 512L * 1024L >> SECTOR_SHIFT)	/* 512 KB in sectors */
 //#define STRIPE_SIZE_LIMIT ((UINT_MAX >> 2) + 1)
-//#define PV_MIN_SIZE ( 512L * 1024L >> SECTOR_SHIFT)	/* 512 KB in sectors */
 //#define MAX_RESTRICTED_LVS 255	/* Used by FMT_RESTRICTED_LVIDS */
 #define MIRROR_LOG_OFFSET	2	/* sectors */
 #define VG_MEMPOOL_CHUNK	10240	/* in bytes, hint only */
lib/misc/lvm-globals.c +12 -0
index 9da61fe..44c8d7e 100644
@@ -19,6 +19,7 @@
 #include "lvm-string.h"
 #include "lvm-file.h"
 #include "defaults.h"
+#include "metadata-exported.h"
 
 #include <stdarg.h>
 
@@ -42,6 +43,7 @@ static unsigned _is_static = 0;
 static int _udev_checking = 1;
 static char _sysfs_dir_path[PATH_MAX] = "";
 static int _dev_disable_after_error_count = DEFAULT_DISABLE_AFTER_ERROR_COUNT;
+static uint64_t _pv_min_size = (DEFAULT_PV_MIN_SIZE_KB * 1024L >> SECTOR_SHIFT);
 
 void init_verbose(int level)
 {
@@ -128,6 +130,11 @@ void init_dev_disable_after_error_count(int value)
 	_dev_disable_after_error_count = value;
 }
 
+void init_pv_min_size(uint64_t value)
+{
+	_pv_min_size = value;
+}
+
 void set_cmd_name(const char *cmd)
 {
 	strncpy(_cmd_name, cmd, sizeof(_cmd_name));
@@ -247,3 +254,8 @@ int dev_disable_after_error_count(void)
 {
 	return _dev_disable_after_error_count;
 }
+
+uint64_t pv_min_size(void)
+{
+	return _pv_min_size;
+}
lib/misc/lvm-globals.h +4 -1
index 2fabbc7..bb383f4 100644
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.  
- * Copyright (C) 2004-2007 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2004-2011 Red Hat, Inc. All rights reserved.
  *
  * This file is part of LVM2.
  *
@@ -18,6 +18,7 @@
 
 #define VERBOSE_BASE_LEVEL _LOG_WARN
 #define SECURITY_LEVEL 0
+#define DEFAULT_PV_MIN_SIZE_KB		512		/* 512 KB */
 
 void init_verbose(int level);
 void init_test(int level);
@@ -38,6 +39,7 @@ void init_error_message_produced(int produced);
 void init_is_static(unsigned value);
 void init_udev_checking(int checking);
 void init_dev_disable_after_error_count(int value);
+void init_pv_min_size(uint64_t sectors);
 
 void set_cmd_name(const char *cmd_name);
 void set_sysfs_dir_path(const char *path);
@@ -59,6 +61,7 @@ const char *log_command_name(void);
 unsigned is_static(void);
 int udev_checking(void);
 const char *sysfs_dir_path(void);
+uint64_t pv_min_size(void);
 
 #define DMEVENTD_MONITOR_IGNORE -1
 int dmeventd_monitor_mode(void);
tools/pvresize.c +3 -3
index 8582ef4..a471f36 100644
@@ -111,9 +111,9 @@ static int _pv_resize_single(struct cmd_context *cmd,
 		size = new_size;
 	}
 
-	if (size < PV_MIN_SIZE) {
-		log_error("%s: Size must exceed minimum of %ld sectors.",
-			  pv_name, PV_MIN_SIZE);
+	if (size < pv_min_size()) {
+		log_error("%s: Size must exceed minimum of %" PRIu64 " sectors.",
+			  pv_name, pv_min_size());
 		goto out;
 	}
 
Signature · 1 line
1.7.3.5
#3 lore raw

PATCH 3/15 Test pv_min_size functionality

In reply to Zdenek Kabelac
---
 test/t-pv-min-size.sh |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)
 create mode 100755 test/t-pv-min-size.sh
test/t-pv-min-size.sh new file +25 -0
new file mode 100755
index 0000000..c1f941c
@@ -0,0 +1,25 @@
+#!/bin/sh
+# Copyright (C) 2011 Red Hat, Inc. All rights reserved.
+#
+# This copyrighted material is made available to anyone wishing to use,
+# modify, copy, or redistribute it subject to the terms and conditions
+# of the GNU General Public License v.2.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software Foundation,
+# Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+. lib/test
+
+# use small default size  - 512KB
+aux lvmconf 'devices/pv_min_size = 512'
+
+aux prepare_pvs 1 8
+
+check pv_field $dev1 pv_name $dev1
+
+# increase min size beyond created PV size 10MB
+aux lvmconf 'devices/pv_min_size = 10240'
+
+# and test device is not visible
+not check pv_field $dev1 pv_name $dev1
Signature · 1 line
1.7.3.5
#4 lore raw

PATCH 4/15 Move add_dev_node to DM_DEVICE_RESUME

In reply to Zdenek Kabelac

This patch came out from discussion with Petr. We need valid 'check_udev' for stacked ops processing.

---
 libdm/ioctl/libdm-iface.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
libdm/ioctl/libdm-iface.c +4 -4
index 92c8e9e..c3df610 100644
@@ -2070,10 +2070,6 @@ repeat_ioctl:
 
 	switch (dmt->type) {
 	case DM_DEVICE_CREATE:
-		if (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);
 		break;
 	case DM_DEVICE_REMOVE:
 		/* FIXME Kernel needs to fill in dmi->name */
@@ -2089,6 +2085,10 @@ repeat_ioctl:
 		break;
 
 	case DM_DEVICE_RESUME:
+		if (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);
 		/* FIXME Kernel needs to fill in dmi->name */
 		set_dev_node_read_ahead(dmt->dev_name, dmt->read_ahead,
 					dmt->read_ahead_flags);
Signature · 1 line
1.7.3.5
#5 lore raw

PATCH 5/15 Fix multiple operations on the same node

In reply to Zdenek Kabelac

As we are now able to stack many operations in one udev transaction - in same cases we are adding and removing device at the same time.

This leads to a problem of checking stacked operations:
i.e. remove /dev/node1 followed by create /dev/node1

If the node creation is handled with udev - there is a problem as stacked operation gives warning about existing node1 and will try to remove it - while next operation needs to recreate it.

Current code removes all previous stacked operation if the fs op is FS_DEL - patch adds similar behavior for FS_ADD - it will try to remove any 'delete' operation if udev is in use.

As a very simple optimisation counters were added for each stacked ops type to avoid unneeded list scans.

For the complexity probably the best would be to sort list before processing stacked ops and removing of unwanted ops - but it seem to complex for the speed gains we could achieve.

CHECKME: handling of rename operation needs to be better checked for liblvm2api usage.

(Patch currently checks variable 'check_udev' flag - previous patch is needed for that)

---
 lib/activate/fs.c    |   49 ++++++++++++++++++++++++++++++++++++++++--
 libdm/libdm-common.c |   57 +++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 91 insertions(+), 15 deletions(-)
lib/activate/fs.c +46 -3
index ae94763..8b8b585 100644
@@ -257,7 +257,8 @@ static int _rm_link(const char *dev_dir, const char *vg_name,
 typedef enum {
 	FS_ADD,
 	FS_DEL,
-	FS_RENAME
+	FS_RENAME,
+	NUM_FS_OPS
 } fs_op_t;
 
 static int _do_fs_op(fs_op_t type, const char *dev_dir, const char *vg_name,
@@ -283,12 +284,18 @@ static int _do_fs_op(fs_op_t type, const char *dev_dir, const char *vg_name,
 
 		if (!_mk_link(dev_dir, vg_name, lv_name, dev, check_udev))
 			stack;
+	default:;
 	}
 
 	return 1;
 }
 
 static DM_LIST_INIT(_fs_ops);
+/*
+ * Count number of stacked fs_op_t operations to allow to skip dm_list search.
+ * FIXME: handling of FS_RENAME
+ */
+static int _c_fs_ops[NUM_FS_OPS];
 
 struct fs_op_parms {
 	struct dm_list list;
@@ -309,15 +316,51 @@ static void _store_str(char **pos, char **ptr, const char *str)
 	*pos += strlen(*ptr) + 1;
 }
 
+static int _other_fs_ops(fs_op_t type)
+{
+	int i;
+
+	for (i = 0; i < NUM_FS_OPS; i++)
+		if (type != i && _c_fs_ops[i])
+			return 1;
+	return 0;
+}
+
+static void _del_fs_op(struct fs_op_parms *fsp)
+{
+	_c_fs_ops[fsp->type]--;
+	dm_list_del(&fsp->list);
+	dm_free(fsp);
+}
+
 static int _stack_fs_op(fs_op_t type, const char *dev_dir, const char *vg_name,
 			const char *lv_name, const char *dev,
 			const char *old_lv_name, int check_udev)
 {
+	struct dm_list *fsph, *fspht;
 	struct fs_op_parms *fsp;
 	size_t len = strlen(dev_dir) + strlen(vg_name) + strlen(lv_name) +
 	    strlen(dev) + strlen(old_lv_name) + 5;
 	char *pos;
 
+	if (type == FS_DEL && _other_fs_ops(type))
+		dm_list_iterate_safe(fsph, fspht, &_fs_ops) {
+			fsp = dm_list_item(fsph, struct fs_op_parms);
+			if (!strcmp(fsp->lv_name, lv_name) &&
+			    !strcmp(fsp->vg_name, vg_name))
+				_del_fs_op(fsp);
+		}
+	else if (type == FS_ADD && _c_fs_ops[FS_DEL] &&
+		 check_udev && dm_udev_get_sync_support() &&
+		 dm_udev_get_checking())
+		dm_list_iterate_safe(fsph, fspht, &_fs_ops) {
+			fsp = dm_list_item(fsph, struct fs_op_parms);
+			if ((fsp->type == FS_DEL || fsp->type == FS_ADD) &&
+			    !strcmp(fsp->lv_name, lv_name) &&
+			    !strcmp(fsp->vg_name, vg_name))
+				_del_fs_op(fsp);
+		}
+
 	if (!(fsp = dm_malloc(sizeof(*fsp) + len))) {
 		log_error("No space to stack fs operation");
 		return 0;
@@ -333,6 +376,7 @@ static int _stack_fs_op(fs_op_t type, const char *dev_dir, const char *vg_name,
 	_store_str(&pos, &fsp->dev, dev);
 	_store_str(&pos, &fsp->old_lv_name, old_lv_name);
 
+	_c_fs_ops[type]++;
 	dm_list_add(&_fs_ops, &fsp->list);
 
 	return 1;
@@ -347,8 +391,7 @@ static void _pop_fs_ops(void)
 		fsp = dm_list_item(fsph, struct fs_op_parms);
 		_do_fs_op(fsp->type, fsp->dev_dir, fsp->vg_name, fsp->lv_name,
 			  fsp->dev, fsp->old_lv_name, fsp->check_udev);
-		dm_list_del(&fsp->list);
-		dm_free(fsp);
+		_del_fs_op(fsp);
 	}
 }
 
libdm/libdm-common.c +45 -12

102 lines not on this page · raw message

Signature · 1 line
1.7.3.5
#6 lore raw

PATCH 6/15 Micro optimization of _eat_space and _get_token

In reply to Zdenek Kabelac

Makes the code more readable and has a smaller number of memory accesses.

For _get_token() optimize number parsing. Check for '.' char only if it's not a digit. Move pointer incrementation into one place.

For _eat_space() check only p->te for '\0' in skipping of comment line. Avoid check for '\0' when we know it is space. Also master while loop doesn't need checking p->tb for '\0'. We just need to check p->tb isn't already at the end of buffer. This could give 'extra' loop cycle if we are already there - but safes memory access in every other case.

---
 lib/config/config.c |   36 ++++++++++++++++++------------------
 1 files changed, 18 insertions(+), 18 deletions(-)
lib/config/config.c +18 -18
index 15bf401..4efe990 100644
@@ -821,18 +821,20 @@ static void _get_token(struct parser *p, int tok_prev)
 	case '+':
 	case '-':
 		if (values_allowed) {
-			te++;
-			while ((te != p->fe) && (*te)) {
-				if (*te == '.') {
-					if (p->t == TOK_FLOAT)
-						break;
-					p->t = TOK_FLOAT;
-				} else if (!isdigit((int) *te))
+			while (++te != p->fe) {
+				if (!isdigit((int) *te)) {
+					if (*te == '.') {
+						if (p->t != TOK_FLOAT) {
+							p->t = TOK_FLOAT;
+							continue;
+						}
+					}
 					break;
-				te++;
+				}
 			}
 			break;
 		}
+		/* fall through */
 
 	default:
 		p->t = TOK_IDENTIFIER;
@@ -849,21 +851,19 @@ static void _get_token(struct parser *p, int tok_prev)
 
 static void _eat_space(struct parser *p)
 {
-	while ((p->tb != p->fe) && (*p->tb)) {
+	while (p->tb != p->fe) {
 		if (*p->te == '#')
 			while ((p->te != p->fe) && (*p->te) && (*p->te != '\n'))
 				p->te++;
 
-		else if (isspace(*p->te)) {
-			while ((p->te != p->fe) && (*p->te) && isspace(*p->te)) {
-				if (*p->te == '\n')
-					p->line++;
-				p->te++;
-			}
-		}
+		else if (!isspace(*p->te))
+			break;
 
-		else
-			return;
+		while ((p->te != p->fe) && isspace(*p->te)) {
+			if (*p->te == '\n')
+				p->line++;
+			p->te++;
+		}
 
 		p->tb = p->te;
 	}
Signature · 1 line
1.7.3.5
#7 lore raw

PATCH 7/15 Testing: Display duration of tests

In reply to Zdenek Kabelac

Enhance output with info about test duration.
Cleanup few declarations in the code.

---
 test/lib/harness.c |   32 ++++++++++++++++++++++----------
 1 files changed, 22 insertions(+), 10 deletions(-)
test/lib/harness.c +22 -10
index f07d2ed..ae6c344 100644
@@ -20,6 +20,7 @@
 #include <sys/wait.h>
 #include <unistd.h>
 #include <stdlib.h>
+#include <time.h>
 
 static pid_t pid;
 static int fds[2];
@@ -82,9 +83,9 @@ static int outline(char *buf, int start, int force) {
                 char *line = strndup(from, next - from);
                 char *a = line, *b;
                 do {
-                        b = line + strlen(line);
                         int idx = -1;
                         int i;
+                        b = line + strlen(line);
                         for ( i = 0; i < 2; ++i ) {
                                 if (subst[i].key) {
                                         // printf("trying: %s -> %s\n", subst[i].value, subst[i].key);
@@ -145,15 +146,24 @@ static void drain(void) {
 	}
 }
 
-static void passed(int i, char *f) {
+static const char *duration(time_t start)
+{
+	static char buf[16];
+	int t = (int)(time(NULL) - start);
+
+	sprintf(buf, "%2d:%02d", t / 60, t % 60);
+	return buf;
+}
+
+static void passed(int i, char *f, time_t t) {
 	if (strstr(readbuf, "TEST WARNING")) {
 		++s.nwarned;
 		s.status[i] = WARNED;
-		printf("warnings\n");
+		printf("warnings  %s\n", duration(t));
 	} else {
 		++ s.npassed;
 		s.status[i] = PASSED;
-		printf("passed.\n");
+		printf("passed.   %s\n", duration(t));
 	}
 }
 
@@ -192,12 +202,13 @@ static void run(int i, char *f) {
 		fflush(stderr);
 		_exit(202);
 	} else {
+		int st, w;
+		time_t start = time(NULL);
 		char buf[128];
 		snprintf(buf, 128, "%s ...", f);
 		buf[127] = 0;
 		printf("Running %-40s ", buf);
 		fflush(stdout);
-		int st, w;
 		while ((w = waitpid(pid, &st, WNOHANG)) == 0) {
 			drain();
 			usleep(20000);
@@ -209,7 +220,7 @@ static void run(int i, char *f) {
 		drain();
 		if (WIFEXITED(st)) {
 			if (WEXITSTATUS(st) == 0) {
-				passed(i, f);
+				passed(i, f, start);
 			} else if (WEXITSTATUS(st) == 200) {
 				skipped(i, f);
 			} else {
@@ -223,6 +234,8 @@ static void run(int i, char *f) {
 }
 
 int main(int argc, char **argv) {
+	const char *be_verbose = getenv("VERBOSE");
+	time_t start = time(NULL);
 	int i;
 
 	if (argc >= MAX) {
@@ -230,9 +243,6 @@ int main(int argc, char **argv) {
 		exit(1);
 	}
 
-	s.nwarned = s.nfailed = s.npassed = s.nskipped = 0;
-
-	char *be_verbose = getenv("VERBOSE");
 	if (be_verbose && atoi(be_verbose))
 		verbose = 1; // XXX
 
@@ -260,8 +270,9 @@ int main(int argc, char **argv) {
 			break;
 	}
 
-	printf("\n## %d tests: %d OK, %d warnings, %d failures; %d skipped\n",
+	printf("\n## %d tests %s : %d OK, %d warnings, %d failures; %d skipped\n",
 	       s.nwarned + s.npassed + s.nfailed + s.nskipped,
+	       duration(start),
 	       s.npassed, s.nwarned, s.nfailed, s.nskipped);
 
 	/* print out a summary */
@@ -279,5 +290,6 @@ int main(int argc, char **argv) {
 		printf("\n");
 		return s.nfailed > 0 || die;
 	}
+
 	return die;
 }
Signature · 1 line
1.7.3.5
#8 lore raw

PATCH 8/15 Testing: Accelerate mirror tests

In reply to Zdenek Kabelac

Instead of recreation whole device set for each test round, just cleanup created LVs to empty VG.

---
 test/t-lvconvert-mirror-basic.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
test/t-lvconvert-mirror-basic.sh +3 -3
index d2d43e5..413e6f2 100644
@@ -77,8 +77,6 @@ test_lvconvert()
 		max_log_count=$start_log_count
 	fi
 
-	aux prepare_vg 5
-
 	if [ $start_count -gt 0 ]; then
 		# Are there extra devices for the log or do we overlap
 		if [ $(($start_count_p1 + $start_log_count)) -gt ${#dev_array[@]} ]; then
@@ -123,7 +121,7 @@ test_lvconvert()
 	fi
 }
 
-aux prepare_vg 5
+aux prepare_vg 5 16
 
 test_many() {
 	i=$1
@@ -135,7 +133,9 @@ test_many() {
 				: "Testing mirror conversion -m$i/$k -> -m$j/$l"
 				: ----------------------------------------------------
 				test_lvconvert $i $k $j $l 0
+				lvremove -ff $vg
 				test_lvconvert $i $k $j $l 1
+				lvremove -ff $vg
 			done
 		done
 	done
Signature · 1 line
1.7.3.5
#9 lore raw

PATCH 9/15 Testing: Add udev_wait for sfdisk

In reply to Zdenek Kabelac

sfdisk needs to wait for device creation by udev
(when /dev is mounted on tmpfs)

mdadm --stop seems to exit before actual node is
deleted on /dev mounted on devtmpfs.

-o pipefile produces exit code 141 (128 + SIGPIPE) when pipe from dmsetup table | egrep -q is closed after egrep exit.

---
 test/api/percent.sh             |    2 +-
 test/lib/aux.sh                 |   10 ++++++++++
 test/lib/check.sh               |    4 +++-
 test/t-pvcreate-operation-md.sh |    8 +++++++-
 4 files changed, 21 insertions(+), 3 deletions(-)
test/api/percent.sh +1 -1
index 52cc799..e93e4cc 100644
@@ -23,5 +23,5 @@ lvcreate -s -n snap $vg/foo -l 2 -c 4k
 lvcreate -s -n snap2 $vg/foo -l 6 -c 4k
 dd if=/dev/urandom of=$DM_DEV_DIR/$vg/snap2 count=1 bs=1024
 lvcreate -m 1 -n mirr $vg -l 1 --mirrorlog core
-lvs
+lvs $vg
 aux apitest percent $vg
test/lib/aux.sh +10 -0
index 04367ea..89f3bca 100644
@@ -71,6 +71,7 @@ teardown_devs() {
 
 	}
 
+	udev_wait
 	# NOTE: SCSI_DEBUG_DEV test must come before the LOOP test because
 	# prepare_scsi_debug_dev() also sets LOOP to short-circuit prepare_loop()
 	if test -f SCSI_DEBUG_DEV; then
@@ -367,6 +368,15 @@ api() {
 	$abs_top_builddir/test/api/wrapper "$@"
 }
 
+udev_wait() {
+	pgrep udev >/dev/null || return
+        if test -n "$1" ; then
+		udevadm settle --exit-if-exists=$1
+        else
+		udevadm settle --timeout=5 
+        fi
+}
+
 test -f DEVICES && devs=$(cat DEVICES)
 test -f LOOP && LOOP=$(cat LOOP)
 
test/lib/check.sh +3 -1
index 23b6970..ab8ba9e 100644
@@ -23,7 +23,9 @@
 
 # ...
 
-set -e -o pipefail
+set -e 
+# pipefail doesn't work with breaking pipe output from the larger dmsetup table
+#-o pipefail
 
 trim()
 {
test/t-pvcreate-operation-md.sh +7 -1
index a844e32..5085ce1 100644
@@ -34,8 +34,9 @@ mdadm_maj=$(mdadm --version 2>&1 | perl -pi -e 's|.* v(\d+).*|\1|')
 cleanup_md() {
     # sleeps offer hack to defeat: 'md: md127 still in use'
     # see: https://bugzilla.redhat.com/show_bug.cgi?id=509908#c25
-    sleep 2
+    aux udev_wait
     mdadm --stop $mddev || true
+    aux udev_wait
     if [ -b "$mddev" ]; then
         # mdadm doesn't always cleanup the device node
 	sleep 2
@@ -102,6 +103,11 @@ EOF
     base_mddev_p=`basename $mddev_p_sysfs_name`
     mddev_p=/dev/${base_mddev_p}
 
+    # in case the system is running without devtmpfs /dev
+    # wait here for created device node on tmpfs
+    aux udev_wait $mddev_p
+    test -b $mddev_p || exit 200
+
     # Checking for 'alignment_offset' in sysfs implies Linux >= 2.6.31
     # but reliable alignment_offset support requires kernel.org Linux >= 2.6.33
     sysfs_alignment_offset=/sys/dev/block/${mddev_maj_min}/${base_mddev_p}/alignment_offset
Signature · 1 line
1.7.3.5
#10 lore raw

PATCH 10/15 Testing: Lower mem usage of inconsistent mda test

In reply to Zdenek Kabelac

Smaller size of devices speedups this test.

---
 test/t-inconsistent-metadata.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
test/t-inconsistent-metadata.sh +1 -1
index 4dcc766..ce920be 100644
@@ -11,7 +11,7 @@
 
 . lib/test
 
-aux prepare_vg 3
+aux prepare_vg 3 12
 
 lvcreate -m 1 -l 1 -n mirror $vg
 lvcreate -l 1 -n resized $vg
Signature · 1 line
1.7.3.5
#11 lore raw

PATCH 11/15 Testing: Lower device size of pvmove test

In reply to Zdenek Kabelac

Lower size of devices in test.
And check only once that LVs are prepared properly. Make the test faster a bit.

---
 test/t-pvmove-basic.sh |   24 ++++++++++++++----------
 1 files changed, 14 insertions(+), 10 deletions(-)
test/t-pvmove-basic.sh +14 -10
index 56e1f99..89d24e2 100755
@@ -43,30 +43,34 @@ check_dev_sum_() {
 # ---------------------------------------------------------------------
 # Initialize PVs and VGs
 
-aux prepare_vg 5 80
+aux prepare_vg 5 40
 
 # ---------------------------------------------------------------------
 # Common environment setup/cleanup for each sub testcases
+FIRST=""
 
 prepare_lvs_() {
   lvcreate -l2 -n $lv1 $vg $dev1 
-    lv_is_on_ $vg/$lv1 $dev1 
+    test -z "$FIRST" && lv_is_on_ $vg/$lv1 $dev1
   lvcreate -l9 -i3 -n $lv2 $vg $dev2 $dev3 $dev4 
-    lv_is_on_ $vg/$lv2 $dev2 $dev3 $dev4 
+    test -z "$FIRST" && lv_is_on_ $vg/$lv2 $dev2 $dev3 $dev4
   lvextend -l+2 $vg/$lv1 $dev2 
-    lv_is_on_ $vg/$lv1 $dev1 $dev2 
+    test -z "$FIRST" && lv_is_on_ $vg/$lv1 $dev1 $dev2
   lvextend -l+2 $vg/$lv1 $dev3 
-    lv_is_on_ $vg/$lv1 $dev1 $dev2 $dev3 
+    test -z "$FIRST" && lv_is_on_ $vg/$lv1 $dev1 $dev2 $dev3
   lvextend -l+2 $vg/$lv1 $dev1 
-    lv_is_on_ $vg/$lv1 $dev1 $dev2 $dev3 $dev1 
+    test -z "$FIRST" && lv_is_on_ $vg/$lv1 $dev1 $dev2 $dev3 $dev1
   lvcreate -l1 -n $lv3 $vg $dev2 
-    lv_is_on_ $vg/$lv3 $dev2 
+    test -z "$FIRST" && lv_is_on_ $vg/$lv3 $dev2
   save_dev_sum_ $(lvdev_ $vg $lv1) 
   save_dev_sum_ $(lvdev_ $vg $lv2) 
   save_dev_sum_ $(lvdev_ $vg $lv3) 
-  lvs -a -o devices --noheadings $vg/$lv1 > ${lv1}_devs 
-  lvs -a -o devices --noheadings $vg/$lv2 > ${lv2}_devs 
-  lvs -a -o devices --noheadings $vg/$lv3 > ${lv3}_devs
+  if test -z "$FIRST" ; then
+    lvs -a -o devices --noheadings $vg/$lv1 > ${lv1}_devs
+    lvs -a -o devices --noheadings $vg/$lv2 > ${lv2}_devs
+    lvs -a -o devices --noheadings $vg/$lv3 > ${lv3}_devs
+  fi
+  FIRST=done
 }
 
 lv_not_changed_() {
Signature · 1 line
1.7.3.5
#12 lore raw

PATCH 12/15 Skip compilation of debug code

In reply to Zdenek Kabelac

When it's not in use - do not compile this code.
Improves code coverage results.

---
 libdm/mm/dbg_malloc.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
libdm/mm/dbg_malloc.c +4 -0
index db7f5f3..5f06d1e 100644
@@ -15,6 +15,8 @@
 
 #include "dmlib.h"
 
+#ifdef DEBUG_MEM
+
 #include <assert.h>
 #include <stdarg.h>
 
@@ -249,6 +251,8 @@ void dm_bounds_check_debug(void)
 	}
 }
 
+#endif
+
 void *dm_malloc_aux(size_t s, const char *file __attribute__((unused)),
 		    int line __attribute__((unused)))
 {
Signature · 1 line
1.7.3.5
#13 lore raw

PATCH 13/15 Man page consistency fixes

In reply to Zdenek Kabelac

Make a better looking colorized manpages for few commands. It's rather a my proposal how do I'd like to see man pages to be looking. For view I'm using 'midnight commander' and its colorized man page reader output looked quite unreabable with original man pages.

Also it removes some common options - they are better described in lvm(8) page.

This patch makes them IMHO better - but I see it rather for discussion how the options and options' parameters should be visible.

Also it extends a bit pvremove man page with -ff and -y option description.

---
 man/pvcreate.8.in |   63 ++++++++++++++++++++++++----------------------------
 man/pvremove.8.in |   15 +++++++-----
 man/pvresize.8.in |   13 +++++------
 man/pvscan.8.in   |   14 ++++-------
 4 files changed, 49 insertions(+), 56 deletions(-)
man/pvcreate.8.in +29 -34
index cb10049..14f0a38 100644
@@ -3,25 +3,20 @@
 pvcreate \- initialize a disk or partition for use by LVM
 .SH SYNOPSIS
 .B pvcreate
-.RB [ \-d | \-\-debug ]
 .RB [ \-f [ f ]| \-\-force " [" \-\-force ]]
 .RB [ \-y | \-\-yes ]
-.RB [ \-h | \-\-help ]
-.RB [ \-t | \-\-test ]
-.RB [ \-v | \-\-verbose ]
 .RB [ \-\-labelsector ]
-.RB [ \-M | \-\-metadatatype type ]
-.RB [ \-\-[pv]metadatacopies #copies ]
-.RB [ \-\-metadatasize size ]
-.RB [ \-\-metadataignore y|n ]
-.RB [ \-\-dataalignment alignment ]
-.RB [ \-\-dataalignmentoffset alignment_offset ]
-.RB [ \-\-restorefile file ]
+.RB [ \-M | \-\-metadatatype " " \fItype ]
+.RB [ \-\-[pv]metadatacopies " " \fI#copies ]
+.RB [ \-\-metadatasize " " \fIsize ]
+.RB [ \-\-metadataignore " {" \fIy | \fIn }]
+.RB [ \-\-dataalignment " " \fIalignment ]
+.RB [ \-\-dataalignmentoffset " " \fIalignment_offset ]
+.RB [ \-\-restorefile " " \fIfile ]
 .RB [ \-\-norestorefile ]
-.RB [ \-\-setphysicalvolumesize size ]
-.RB [ \-u | \-\-uuid uuid ]
-.RB [ \-\-version ]
-.RB [ \-Z | \-\-zero y|n ]
+.RB [ \-\-setphysicalvolumesize " " \fIsize ]
+.RB [ \-u | \-\-uuid " " \fIuuid ]
+.RB [ \-Z | \-\-zero " {" \fIy | \fIn }]
 .IR PhysicalVolume " [" PhysicalVolume ...]
 .SH DESCRIPTION
 .B pvcreate
@@ -54,9 +49,9 @@ See \fBlvm\fP(8) for common options.
 .BR \-f ", " \-\-force
 Force the creation without any confirmation.  You can not recreate
 (reinitialize) a physical volume belonging to an existing volume group.
-In an emergency you can override this behaviour with -ff.
+In an emergency you can override this behaviour with \fB-ff\fP.
 .TP
-.BR \-u ", " \-\-uuid " uuid"
+.BR \-u ", " \-\-uuid " " \fIuuid
 Specify the uuid for the device.  
 Without this option, \fBpvcreate\fP generates a random uuid.
 All of your physical volumes must have unique uuids.
@@ -68,12 +63,12 @@ used.
 .BR \-y ", " \-\-yes
 Answer yes to all questions.
 .TP
-.BR \-Z ", " \-\-zero " y|n"
+.BR \-Z ", " \-\-zero " " \fIy | \fIn
 Whether or not the first 4 sectors (2048 bytes) of the device should be 
 wiped.
 If this option is not given, the 
-default is to wipe these sectors unless either or both of the --restorefile 
-or --uuid options were specified.
+default is to wipe these sectors unless either or both of the \fB--restorefile\fP
+or \fB--uuid\fP options were specified.
 .SH NEW METADATA OPTIONS
 LVM2 introduces a new format for storing metadata on disk.
 This new format is more efficient and resilient than the format the 
@@ -91,25 +86,25 @@ Additional tools for manipulating the locations and sizes of metadata areas
 will be written in due course.  Use the verbose/debug options on the tools
 to see where the metadata areas are placed.
 .TP
-.BR \-\-metadatasize " size"
+.BI \-\-metadatasize " size"
 The approximate amount of space to be set aside for each metadata area.
 (The size you specify may get rounded.)
 .TP
-.BR \-\-dataalignment " alignment"
+.BI \-\-dataalignment " alignment"
 Align the start of the data to a multiple of this number.
-You should also specify an appropriate \fBPhysicalExtentSize\fP when creating
+You should also specify an appropriate \fIPhysicalExtentSize\fP when creating
 the Volume Group with \fBvgcreate\fP.
 .sp
 To see the location of the first Physical Extent of an existing Physical Volume
 use \fBpvs -o +pe_start\fP .  It will be a multiple of the requested
-\fBalignment\fP.  In addition it may be shifted by \fBalignment_offset\fP from
-\fBdata_alignment_offset_detection\fP (if enabled in \fBlvm.conf\fP) or
+alignment.  In addition it may be shifted by \fIalignment_offset\fP from
+\fIdata_alignment_offset_detection\fP (if enabled in \fBlvm.conf\fP) or
 \fB--dataalignmentoffset\fP.
 .TP
-.BR \-\-dataalignmentoffset " alignment_offset"
-Shift the start of the data area by this additional \fBalignment_offset\fP.
+.BI \-\-dataalignmentoffset " alignment_offset"
+Shift the start of the data area by this additional \fIalignment_offset\fP.
 .TP
-.BR \-\-[pv]metadatacopies " copies"
+.BI \-\-[pv]metadatacopies " copies"
 The number of metadata areas to set aside on each PV.  Currently
 this can be 0, 1 or 2.  
 If set to 2, two copies of the volume group metadata 
@@ -121,7 +116,7 @@ with VGs containing large numbers of PVs.  But if you do this and
 then later use \fBvgsplit\fP you must ensure that each VG is still going 
 to have a suitable number of copies of the metadata after the split!
 .TP
-.BR \-\-metadataignore " y|n"
+.BI \-\-metadataignore " y" \fR| n
 Ignore or un-ignore metadata areas on this physical volume.
 The default is "n".  This setting can be changed with \fBpvchange\fP.
 If metadata areas on a physical volume are ignored, LVM will
@@ -132,7 +127,7 @@ metadata on this device, it is still wise always to allocate a metadata
 area in case you need it in the future and to use this option to instruct
 LVM2 to ignore it.
 .TP
-.BR \-\-restorefile " file"
+.BI \-\-restorefile " file"
 In conjunction with \fB--uuid\fP, this extracts the location and size
 of the data on the PV from the file (produced by \fBvgcfgbackup\fP)
 and ensures that the metadata that the program produces is consistent 
@@ -141,17 +136,17 @@ the same place and not get overwritten by new metadata.  This provides
 a mechanism to upgrade the metadata format or to add/remove metadata
 areas. Use with care. See also \fBvgconvert\fP(8).
 .TP
-.BR \-\-norestorefile
-In conjunction with \fB--uuid\fP, this allows a uuid to be specified
+.B \-\-norestorefile
+In conjunction with \fB--uuid\fP, this allows a \fIuuid\fP to be specified
 without also requiring that a backup of the metadata be provided.
 .TP
-.BR \-\-labelsector " sector"
+.BI \-\-labelsector " sector"
 By default the PV is labelled with an LVM2 identifier in its second 
 sector (sector 1).  This lets you use a different sector near the
 start of the disk (between 0 and 3 inclusive - see LABEL_SCAN_SECTORS
 in the source).  Use with care.
 .TP
-.BR \-\-setphysicalvolumesize " size"
+.BI \-\-setphysicalvolumesize " size"
 Overrides the automatically-detected size of the PV.  Use with care.
 .SH EXAMPLES
 Initialize partition #4 on the third SCSI disk and the entire fifth
man/pvremove.8.in +9 -6
index b435fc7..3522e80 100644
@@ -3,11 +3,7 @@
 pvremove \- remove a physical volume
 .SH SYNOPSIS
 .B pvremove
-.RB [ \-d | \-\-debug]
-.RB [ \-f [ f ]| \-\-force " [" \-\-force ]]
-.RB [\-h | \-\-help]
-.RB [ \-t | \-\-test ]
-.RB [ \-v [ v ]| \-\-verbose " [" \-\-verbose ]]
+.RB [ \-ff ]| \-\-force " " \-\-force ]
 .RB [ \-y | \-\-yes ]
 .IR PhysicalVolume " [" PhysicalVolume ...]
 .SH DESCRIPTION
@@ -15,7 +11,14 @@ pvremove \- remove a physical volume
 wipes the label on a device so that LVM will no longer recognise it
 as a physical volume.
 .SH OPTIONS
-See \fBlvm\fP for common options.
+See \fBlvm\fP(8) for common options.
+.TP
+.BR \-ff ", " \-\-force " " \-\-force
+Force the removal of a physical volume belonging to an existing volume group.
+You cannot remove a physical volume which in use by some active logical volume.
+.TP
+.BR \-y ", " \-\-yes
+Answer yes to all questions.
 .SH SEE ALSO
 .BR lvm (8),
 .BR pvcreate (8),
man/pvresize.8.in +6 -7
index 845f6d3..10a0b50 100644
@@ -3,11 +3,7 @@
 pvresize \- resize a disk or partition in use by LVM2
 .SH SYNOPSIS
 .B pvresize
-.RB [ \-d | \-\-debug ]
-.RB [ \-h | \-\-help ]
-.RB [ \-t | \-\-test ]
-.RB [ \-v | \-\-verbose ]
-.RB [ \-\-setphysicalvolumesize size ]
+.RB [ \-\-setphysicalvolumesize " " \fIsize ]
 .IR PhysicalVolume " [" PhysicalVolume ...]
 .SH DESCRIPTION
 .B pvresize
@@ -18,7 +14,7 @@ allocated on it.
 .SH OPTIONS
 See \fBlvm\fP(8) for common options.
 .TP
-.BR \-\-setphysicalvolumesize " size"
+.BI \-\-setphysicalvolumesize " size"
 Overrides the automatically-detected size of the PV.  Use with care, or
 prior to reducing the physical size of the device.
 .SH EXAMPLES
@@ -46,4 +42,7 @@ does.
 won't currently work correctly on LVM1 volumes or PVs with extra
 metadata areas.
 .SH SEE ALSO
-.BR lvm "(8), " pvmove "(8), " lvresize "(8), " fdisk "(8)"
+.BR lvm (8),
+.BR pvmove (8),
+.BR lvresize (8),
+.BR fdisk (8)
man/pvscan.8.in +5 -9
index 4a6bcd7..a3147d7 100644
@@ -3,19 +3,15 @@
 pvscan \- scan all disks for physical volumes
 .SH SYNOPSIS
 .B pvscan
-.RB [ \-d | \-\-debug]
-.RB [\-e | \-\-exported]
-.RB [\-h | \-\-help]
-.RB [\-\-ignorelockingfailure]
-.RB [ \-n | \-\-novolumegroup]
-.RB [\-s | \-\-short]
-.RB [\-u | \-\-uuid]
-.RB [ \-v [ v ]| \-\-verbose " [" \-\-verbose ]]
+.RB [ \-e | \-\-exported ]
+.RB [ \-n | \-\-novolumegroup ]
+.RB [ \-s | \-\-short ]
+.RB [ \-u | \-\-uuid ]
 .SH DESCRIPTION
 .B pvscan
 scans all supported LVM block devices in the system for physical volumes.
 .SH OPTIONS
-See \fBlvm\fP for common options.
+See \fBlvm\fP(8) for common options.
 .TP
 .BR \-e ", " \-\-exported
 Only show physical volumes belonging to exported volume groups.
Signature · 1 line
1.7.3.5
#14 lore raw

PATCH 14/15 Void* arithmetic replaced with char*

In reply to Zdenek Kabelac
---
 daemons/clvmd/clvmd.c        |    4 ++--
 lib/format_text/text_label.c |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
daemons/clvmd/clvmd.c +2 -2
index 8e21732..1f3184e 100644
@@ -1867,7 +1867,7 @@ static int send_message(void *buf, int msglen, const char *csid, int fd,
 				break;
 			}
 
-			len = write(fd, buf + ptr, msglen - ptr);
+			len = write(fd, (char*)buf + ptr, msglen - ptr);
 
 			if (len <= 0) {
 				if (errno == EINTR)
@@ -2091,7 +2091,7 @@ error:
 	return -1;
 }
 
-void process_message(struct local_client *client, const char *buf, int len,
+void process_message(struct local_client *client, char *buf, int len,
 		     const char *csid)
 {
 	struct clvm_header *inheader;
lib/format_text/text_label.c +2 -2
index e459cde..675dbe8 100644
@@ -52,7 +52,7 @@ static int _text_write(struct label *label, void *buf)
 
 	strncpy((char *)lh->type, label->type, sizeof(label->type));
 
-	pvhdr = (struct pv_header *) ((void *) buf + xlate32(lh->offset_xl));
+	pvhdr = (struct pv_header *) ((char *) buf + xlate32(lh->offset_xl));
 	info = (struct lvmcache_info *) label->info;
 	pvhdr->device_size_xl = xlate64(info->device_size);
 	memcpy(pvhdr->pv_uuid, &info->dev->pvid, sizeof(struct id));
@@ -266,7 +266,7 @@ static int _text_read(struct labeller *l, struct device *dev, void *buf,
 	char *creation_host;
 	struct mda_header *mdah;
 
-	pvhdr = (struct pv_header *) ((void *) buf + xlate32(lh->offset_xl));
+	pvhdr = (struct pv_header *) ((char *) buf + xlate32(lh->offset_xl));
 
 	if (!(info = lvmcache_add(l, (char *)pvhdr->pv_uuid, dev,
 				  FMT_TEXT_ORPHAN_VG_NAME,
Signature · 1 line
1.7.3.5
#15 lore raw

PATCH 15/15 const fixing

In reply to Zdenek Kabelac

Fixing some of the remaining const warnings - major change:

int vg_extend(struct volume_group *vg, int pv_count, const char *const *pv_names,

Change is needed - as lvm2api seems to expect const behaviour here. So vg_extend() is doing local strdup for unescaping.

skip_dev_dir return const char* from const char* vg_name.

Rest of the patch is cleanup of related warnings.

---
 daemons/clvmd/clvmd.h            |    2 +-
 lib/activate/activate.c          |    4 ++--
 lib/activate/activate.h          |    2 +-
 lib/filters/filter-regex.c       |    2 +-
 lib/metadata/lv.h                |    2 +-
 lib/metadata/lv_manip.c          |    3 ++-
 lib/metadata/metadata-exported.h |    8 ++++----
 lib/metadata/metadata.c          |   27 ++++++++++++++++-----------
 lib/metadata/mirror.c            |   18 +++++++++---------
 lib/report/properties.c          |    3 +--
 lib/report/report.c              |    6 +++---
 liblvm/lvm_vg.c                  |    4 ++--
 tools/lvcreate.c                 |    2 +-
 tools/reporter.c                 |    4 ++--
 tools/toollib.c                  |   10 +++++-----
 tools/toollib.h                  |    4 ++--
 tools/vgcfgrestore.c             |    2 +-
 tools/vgcreate.c                 |    2 +-
 tools/vgextend.c                 |    4 ++--
 tools/vgmerge.c                  |    2 +-
 tools/vgreduce.c                 |    2 +-
 tools/vgrename.c                 |    2 +-
 tools/vgsplit.c                  |    2 +-
 23 files changed, 61 insertions(+), 56 deletions(-)
daemons/clvmd/clvmd.h +1 -1
index 95244e1..5bad43a 100644
@@ -112,7 +112,7 @@ extern void cmd_client_cleanup(struct local_client *client);
 extern int add_client(struct local_client *new_client);
 
 extern void clvmd_cluster_init_completed(void);
-extern void process_message(struct local_client *client, const char *buf,
+extern void process_message(struct local_client *client, char *buf,
 			    int len, const char *csid);
 extern void debuglog(const char *fmt, ... )
   __attribute__ ((format(printf, 1, 2)));
lib/activate/activate.c +2 -2
index 3670ea4..3331ed2 100644
@@ -162,7 +162,7 @@ int lv_snapshot_percent(const struct logical_volume *lv, percent_t *percent)
 {
 	return 0;
 }
-int lv_mirror_percent(struct cmd_context *cmd, struct logical_volume *lv,
+int lv_mirror_percent(struct cmd_context *cmd, const struct logical_volume *lv,
 		      int wait, percent_t *percent, uint32_t *event_nr)
 {
 	return 0;
@@ -541,7 +541,7 @@ int lv_snapshot_percent(const struct logical_volume *lv, percent_t *percent)
 }
 
 /* FIXME Merge with snapshot_percent */
-int lv_mirror_percent(struct cmd_context *cmd, struct logical_volume *lv,
+int lv_mirror_percent(struct cmd_context *cmd, const struct logical_volume *lv,
 		      int wait, percent_t *percent, uint32_t *event_nr)
 {
 	int r;
lib/activate/activate.h +1 -1
index 2bc73dd..24d8cb0 100644
@@ -84,7 +84,7 @@ int lv_check_transient(struct logical_volume *lv);
  * Returns 1 if percent has been set, else 0.
  */
 int lv_snapshot_percent(const struct logical_volume *lv, percent_t *percent);
-int lv_mirror_percent(struct cmd_context *cmd, struct logical_volume *lv,
+int lv_mirror_percent(struct cmd_context *cmd, const struct logical_volume *lv,
 		      int wait, percent_t *percent, uint32_t *event_nr);
 
 /*
lib/filters/filter-regex.c +1 -1
index e046c71..fc83469 100644
@@ -135,7 +135,7 @@ static int _build_matcher(struct rfilter *rf, const struct config_value *val)
 	/*
 	 * build the matcher.
 	 */
-	if (!(rf->engine = dm_regex_create(rf->mem, (const char **) regex,
+	if (!(rf->engine = dm_regex_create(rf->mem, (const char * const*) regex,
 					   count)))
 		goto_out;
 	r = 1;
lib/metadata/lv.h +1 -1
index bff36d2..50afa28 100644
@@ -23,7 +23,7 @@ struct replicator_device;
 
 struct logical_volume {
 	union lvid lvid;
-	char *name;
+	const char *name;
 
 	struct volume_group *vg;
 
lib/metadata/lv_manip.c +2 -1
index a2ae30e..fc0d97a 100644
@@ -1897,7 +1897,8 @@ static int _rename_sub_lv(struct cmd_context *cmd,
 			  struct logical_volume *lv,
 			  const char *lv_name_old, const char *lv_name_new)
 {
-	char *suffix, *new_name;
+	const char *suffix;
+	char *new_name;
 	size_t len;
 
 	/*
lib/metadata/metadata-exported.h +4 -4
index 4e2463a..32f0ac3 100644
@@ -410,9 +410,9 @@ void vg_remove_pvs(struct volume_group *vg);
 int vg_remove(struct volume_group *vg);
 int vg_rename(struct cmd_context *cmd, struct volume_group *vg,
 	      const char *new_name);
-int vg_extend(struct volume_group *vg, int pv_count, char **pv_names,
+int vg_extend(struct volume_group *vg, int pv_count, const char *const *pv_names,
 	      struct pvcreate_params *pp);
-int vg_reduce(struct volume_group *vg, char *pv_name);
+int vg_reduce(struct volume_group *vg, const char *pv_name);
 int vg_change_tag(struct volume_group *vg, const char *tag, int add_tag);
 int vg_split_mdas(struct cmd_context *cmd, struct volume_group *vg_from,
 		  struct volume_group *vg_to);
@@ -698,7 +698,7 @@ struct logical_volume *find_pvmove_lv_from_pvname(struct cmd_context *cmd,
 						  uint32_t lv_type);
 const char *get_pvmove_pvname_from_lv(struct logical_volume *lv);
 const char *get_pvmove_pvname_from_lv_mirr(struct logical_volume *lv_mirr);
-percent_t copy_percent(struct logical_volume *lv_mirr);
+percent_t copy_percent(const struct logical_volume *lv_mirr);
 struct dm_list *lvs_using_lv(struct cmd_context *cmd, struct volume_group *vg,
 			  struct logical_volume *lv);
 
@@ -721,7 +721,7 @@ int lv_has_unknown_segments(const struct logical_volume *lv);
 int vg_has_unknown_segments(const struct volume_group *vg);
 
 struct vgcreate_params {
-	char *vg_name;
+	const char *vg_name;
 	uint32_t extent_size;
 	size_t max_pv;
 	size_t max_lv;
lib/metadata/metadata.c +16 -11
index a9c7830..cfc209c 100644
@@ -662,33 +662,38 @@ static int vg_extend_single_pv(struct volume_group *vg, char *pv_name,
  * - pp: parameters to pass to implicit pvcreate; if NULL, do not pvcreate
  *
  */
-int vg_extend(struct volume_group *vg, int pv_count, char **pv_names,
+int vg_extend(struct volume_group *vg, int pv_count, const char *const *pv_names,
 	      struct pvcreate_params *pp)
 {
 	int i;
+	char *pv_name;
 
 	if (_vg_bad_status_bits(vg, RESIZEABLE_VG))
 		return 0;
 
 	/* attach each pv */
 	for (i = 0; i < pv_count; i++) {
-		unescape_colons_and_at_signs(pv_names[i], NULL, NULL);
-		if (!vg_extend_single_pv(vg, pv_names[i], pp))
-			goto bad;
+		if (!(pv_name = dm_strdup(pv_names[i]))) {
+			log_error("Failed to duplicate pv name %s.", pv_names[i]);
+			return 0;
+		}
+		unescape_colons_and_at_signs(pv_name, NULL, NULL);
+		if (!vg_extend_single_pv(vg, pv_name, pp)) {
+			log_error("Unable to add physical volume '%s' to "
+				  "volume group '%s'.", pv_name, vg->name);
+			dm_free(pv_name);
+			return 0;
+		}
+		dm_free(pv_name);
 	}
 
 /* FIXME Decide whether to initialise and add new mdahs to format instance */
 
 	return 1;
-
-      bad:
-	log_error("Unable to add physical volume '%s' to "
-		  "volume group '%s'.", pv_names[i], vg->name);
-	return 0;
 }
 
 /* FIXME: use this inside vgreduce_single? */
-int vg_reduce(struct volume_group *vg, char *pv_name)
+int vg_reduce(struct volume_group *vg, const char *pv_name)
 {
 	struct physical_volume *pv;
 	struct pv_list *pvl;
@@ -1875,7 +1880,7 @@ int vgs_are_compatible(struct cmd_context *cmd __attribute__((unused)),
 {
 	struct lv_list *lvl1, *lvl2;
 	struct pv_list *pvl;
-	char *name1, *name2;
+	const char *name1, *name2;
 
 	if (lvs_in_vg_activated(vg_from)) {
 		log_error("Logical volumes in \"%s\" must be inactive",
lib/metadata/mirror.c +9 -9
index f4d3a10..dc123ec 100644
@@ -1509,7 +1509,7 @@ struct dm_list *lvs_using_lv(struct cmd_context *cmd, struct volume_group *vg,
 	return lvs;
 }
 
-percent_t copy_percent(struct logical_volume *lv_mirr)
+percent_t copy_percent(const struct logical_volume *lv_mirr)
 {
 	uint32_t numerator = 0u, denominator = 0u;
 	struct lv_segment *seg;
@@ -1722,8 +1722,8 @@ static struct logical_volume *_set_up_mirror_log(struct cmd_context *cmd,
 						 int in_sync)
 {
 	struct logical_volume *log_lv;
-	const char *suffix, *c;
-	char *lv_name;
+	const char *suffix, *lv_name;
+	char *tmp_name;
 	size_t len;
 	struct lv_segment *seg;
 
@@ -1742,21 +1742,21 @@ static struct logical_volume *_set_up_mirror_log(struct cmd_context *cmd,
 	    strstr(seg_lv(seg, 0)->name, MIRROR_SYNC_LAYER)) {
 		lv_name = lv->name;
 		suffix = "_mlogtmp_%d";
-	} else if ((c = strstr(lv->name, MIRROR_SYNC_LAYER))) {
-		len = c - lv->name + 1;
-		if (!(lv_name = alloca(len)) ||
-		    !dm_snprintf(lv_name, len, "%s", lv->name)) {
+	} else if ((lv_name = strstr(lv->name, MIRROR_SYNC_LAYER))) {
+		len = lv_name - lv->name + 1;
+		if (!(tmp_name = alloca(len)) ||
+		    !dm_snprintf(tmp_name, len, "%s", lv->name)) {
 			log_error("mirror log name allocation failed");
 			return 0;
 		}
+		lv_name = tmp_name;
 		suffix = "_mlog";
 	} else {
 		lv_name = lv->name;
 		suffix = "_mlog";
 	}
 
-	if (!(log_lv = _create_mirror_log(lv, ah, alloc,
-					  (const char *) lv_name, suffix))) {
+	if (!(log_lv = _create_mirror_log(lv, ah, alloc, lv_name, suffix))) {
 		log_error("Failed to create mirror log.");
 		return NULL;
 	}
lib/report/properties.c +1 -2
index 06e11ba..a08470c 100644
@@ -88,8 +88,7 @@ static int _not_implemented_set(void *obj, struct lvm_property_type *prop)
 
 static percent_t _copy_percent(const struct logical_volume *lv) {
 	percent_t perc;
-	if (!lv_mirror_percent(lv->vg->cmd, (struct logical_volume *) lv,
-			   0, &perc, NULL))
+	if (!lv_mirror_percent(lv->vg->cmd, lv, 0, &perc, NULL))
 		perc = PERCENT_INVALID;
 	return perc;
 }
lib/report/report.c +3 -3
index 136ad4c..5079cd1 100644
@@ -615,7 +615,7 @@ static int _uuid_disp(struct dm_report *rh __attribute__((unused)), struct dm_po
 {
 	char *repstr = NULL;
 
-	if (!(repstr = id_format_and_copy(mem, (struct id *)data)))
+	if (!(repstr = id_format_and_copy(mem, data)))
 		return_0;
 
 	dm_report_field_set_value(field, repstr, NULL);
@@ -853,7 +853,7 @@ static int _copypercent_disp(struct dm_report *rh __attribute__((unused)),
 			     struct dm_report_field *field,
 			     const void *data, void *private __attribute__((unused)))
 {
-	struct logical_volume *lv = (struct logical_volume *) data;
+	const struct logical_volume *lv = (const struct logical_volume *) data;
 	percent_t percent;
 	uint64_t *sortval;
 	char *repstr;
@@ -899,7 +899,7 @@ static struct format_instance _dummy_fid = {
 
 static struct volume_group _dummy_vg = {
 	.fid = &_dummy_fid,
-	.name = (char *) "",
+	.name = "",
 	.system_id = (char *) "",
 	.pvs = { &(_dummy_vg.pvs), &(_dummy_vg.pvs) },
 	.lvs = { &(_dummy_vg.lvs), &(_dummy_vg.lvs) },
liblvm/lvm_vg.c +2 -2
index f087ba4..dfc0c5e 100644
@@ -79,7 +79,7 @@ int lvm_vg_extend(vg_t vg, const char *device)
 	}
 
 	pvcreate_params_set_defaults(&pp);
-	if (!vg_extend(vg, 1, (char **) &device, &pp)) {
+	if (!vg_extend(vg, 1, &device, &pp)) {
 		unlock_vg(vg->cmd, VG_ORPHANS);
 		return -1;
 	}
@@ -98,7 +98,7 @@ int lvm_vg_reduce(vg_t vg, const char *device)
 	if (!vg_check_write_mode(vg))
 		return -1;
 
-	if (!vg_reduce(vg, (char *)device))
+	if (!vg_reduce(vg, device))
 		return -1;
 	return 0;
 }
tools/lvcreate.c +1 -1
index 7016417..d1abdf7 100644
@@ -31,7 +31,7 @@ static int _lvcreate_name_params(struct lvcreate_params *lp,
 {
 	int argc = *pargc;
 	char **argv = *pargv, *ptr;
-	char *vg_name;
+	const char *vg_name;
 
 	lp->lv_name = arg_str_value(cmd, name_ARG, NULL);
 
tools/reporter.c +2 -2
index fbd0c27..ebe85e7 100644
@@ -61,7 +61,7 @@ static int _pvsegs_sub_single(struct cmd_context *cmd,
 
 	struct volume_group _free_vg = {
 		.cmd = cmd,
-		.name = (char *)"",
+		.name = "",
 	};
 
         if (!(_free_vg.vgmem = dm_pool_create("_free_vg", 10240)))
@@ -69,7 +69,7 @@ static int _pvsegs_sub_single(struct cmd_context *cmd,
 
 	struct logical_volume _free_logical_volume = {
 		.vg = vg ?: &_free_vg,
-		.name = (char *) "",
+		.name = "",
 		.snapshot = NULL,
 		.status = VISIBLE_LV,
 		.major = -1,
tools/toollib.c +5 -5
index f76aacd..09e3c37 100644
@@ -28,7 +28,7 @@ const char *command_name(struct cmd_context *cmd)
 /*
  * Strip dev_dir if present
  */
-char *skip_dev_dir(struct cmd_context *cmd, const char *vg_name,
+const char *skip_dev_dir(struct cmd_context *cmd, const char *vg_name,
 		   unsigned *dev_dir_found)
 {
 	const char *dmdir = dm_dir();
@@ -54,7 +54,7 @@ char *skip_dev_dir(struct cmd_context *cmd, const char *vg_name,
 		    *layer) {
 			log_error("skip_dev_dir: Couldn't split up device name %s",
 				  vg_name);
-			return (char *) vg_name;
+			return vg_name;
 		}
 		vglv_sz = strlen(vgname) + strlen(lvname) + 2;
 		if (!(vglv = dm_pool_alloc(cmd->mem, vglv_sz)) ||
@@ -62,7 +62,7 @@ char *skip_dev_dir(struct cmd_context *cmd, const char *vg_name,
 				 *lvname ? "/" : "",
 				 lvname) < 0) {
 			log_error("vg/lv string alloc failed");
-			return (char *) vg_name;
+			return vg_name;
 		}
 		return vglv;
 	}
@@ -76,7 +76,7 @@ char *skip_dev_dir(struct cmd_context *cmd, const char *vg_name,
 	} else if (dev_dir_found)
 		*dev_dir_found = 0;
 
-	return (char *) vg_name;
+	return vg_name;
 }
 
 /*
@@ -898,7 +898,7 @@ const char *extract_vgname(struct cmd_context *cmd, const char *lv_name)
  */
 char *default_vgname(struct cmd_context *cmd)
 {
-	char *vg_path;
+	const char *vg_path;
 
 	/* Take default VG from environment? */
 	vg_path = getenv("LVM_VG_NAME");
tools/toollib.h +2 -2
index 71e516f..62dd427 100644
@@ -84,8 +84,8 @@ int process_each_lv_in_vg(struct cmd_context *cmd,
 
 char *default_vgname(struct cmd_context *cmd);
 const char *extract_vgname(struct cmd_context *cmd, const char *lv_name);
-char *skip_dev_dir(struct cmd_context *cmd, const char *vg_name,
-		   unsigned *dev_dir_found);
+const char *skip_dev_dir(struct cmd_context *cmd, const char *vg_name,
+			 unsigned *dev_dir_found);
 
 /*
  * Builds a list of pv's from the names in argv.  Used in
tools/vgcfgrestore.c +1 -1
index dc0158f..1f1ab52 100644
@@ -17,7 +17,7 @@
 
 int vgcfgrestore(struct cmd_context *cmd, int argc, char **argv)
 {
-	char *vg_name = NULL;
+	const char *vg_name = NULL;
 
 	if (argc == 1) {
 		vg_name = skip_dev_dir(cmd, argv[0], NULL);
tools/vgcreate.c +1 -1
index 49574f3..62b5ac7 100644
@@ -74,7 +74,7 @@ int vgcreate(struct cmd_context *cmd, int argc, char **argv)
 	}
 
 	/* attach the pv's */
-	if (!vg_extend(vg, argc, argv, &pp))
+	if (!vg_extend(vg, argc, (const char* const*)argv, &pp))
 		goto_bad;
 
 	if (vp_new.max_lv != vg->max_lv)
tools/vgextend.c +2 -2
index a0c166b..5d2b54d 100644
@@ -40,7 +40,7 @@ static int _restore_pv(struct volume_group *vg, char *pv_name)
 
 int vgextend(struct cmd_context *cmd, int argc, char **argv)
 {
-	char *vg_name;
+	const char *vg_name;
 	struct volume_group *vg = NULL;
 	int r = ECMD_FAILED;
 	struct pvcreate_params pp;
@@ -107,7 +107,7 @@ int vgextend(struct cmd_context *cmd, int argc, char **argv)
 		}
 
 		/* extend vg */
-		if (!vg_extend(vg, argc, argv, &pp))
+		if (!vg_extend(vg, argc, (const char* const*)argv, &pp))
 			goto_bad;
 
 		if (arg_count(cmd, metadataignore_ARG) &&
tools/vgmerge.c +1 -1
index 391764f..adb00a0 100644
@@ -163,7 +163,7 @@ bad:
 
 int vgmerge(struct cmd_context *cmd, int argc, char **argv)
 {
-	char *vg_name_to, *vg_name_from;
+	const char *vg_name_to, *vg_name_from;
 	int opt = 0;
 	int ret = 0, ret_max = 0;
 
tools/vgreduce.c +1 -1
index 4aa8f01..5e61cf8 100644
@@ -457,7 +457,7 @@ bad:
 int vgreduce(struct cmd_context *cmd, int argc, char **argv)
 {
 	struct volume_group *vg;
-	char *vg_name;
+	const char *vg_name;
 	int ret = ECMD_FAILED;
 	int fixed = 1;
 	int repairing = arg_count(cmd, removemissing_ARG);
tools/vgrename.c +1 -1
index 7a4e730..2f83fc6 100644
@@ -63,7 +63,7 @@ static int vg_rename_path(struct cmd_context *cmd, const char *old_vg_path,
 	int found_id = 0;
 	struct dm_list *vgids;
 	struct str_list *sl;
-	char *vg_name_new;
+	const char *vg_name_new;
 	const char *vgid = NULL, *vg_name, *vg_name_old;
 	char old_path[NAME_LEN], new_path[NAME_LEN];
 	struct volume_group *vg = NULL;
tools/vgsplit.c +1 -1
index bd2f0ab..b97db97 100644
@@ -281,7 +281,7 @@ int vgsplit(struct cmd_context *cmd, int argc, char **argv)
 {
 	struct vgcreate_params vp_new;
 	struct vgcreate_params vp_def;
-	char *vg_name_from, *vg_name_to;
+	const char *vg_name_from, *vg_name_to;
 	struct volume_group *vg_to = NULL, *vg_from = NULL;
 	int opt;
 	int existing_vg = 0;
Signature · 1 line
1.7.3.5
#16 lore raw

cover Pvmin, udev, testing

In reply to Zdenek Kabelac
Quoted 2 lines · Zdenek Kabelac

Patchset combince few 'smaller' changes which are trying to fix couple problem with current git tree.

I have looked at the testing portion of this patchset -- ack (ex post).

Yours,
Petr