o Use lvm_snprintf wherever return value is used

o Add parameters to set retention limits for backups

9 files changed, 65 insertions, 39 deletions

diff --git a/lib/activate/activate.c b/lib/activate/activate.c
index 5e280ed1..42f957b9 100644
--- a/lib/activate/activate.c
+++ b/lib/activate/activate.c
@@ -9,6 +9,7 @@
 #include "display.h"
 #include "log.h"
 #include "fs.h"
+#include "lvm-string.h"
 
 static void _build_lv_name(char *buffer, size_t s, struct logical_volume *lv)
 {
@@ -99,7 +100,7 @@ static int _emit_target(struct dm_task *dmt, struct stripe_segment *seg)
 		"Insufficient space to write target parameters.";
 
 	if (stripes > 1) {
-		tw = snprintf(params, sizeof(params), "%u %u ",
+		tw = lvm_snprintf(params, sizeof(params), "%u %u ",
 			      stripes, seg->stripe_size);
 
 		if (tw < 0) {
@@ -112,7 +113,7 @@ static int _emit_target(struct dm_task *dmt, struct stripe_segment *seg)
 
 	
 	for (s = 0; s < stripes; s++, w += tw) {
-		tw = snprintf(params + w, sizeof(params) - w,
+		tw = lvm_snprintf(params + w, sizeof(params) - w,
 			      "%s %" PRIu64 "%s",
 			      dev_name(seg->area[s].pv->dev),
 			      (seg->area[s].pv->pe_start +
diff --git a/lib/format_text/export.c b/lib/format_text/export.c
index c89395db..278c55e2 100644
--- a/lib/format_text/export.c
+++ b/lib/format_text/export.c
@@ -131,7 +131,7 @@ static int _sectors_to_units(uint64_t sectors, char *buffer, size_t s)
 	/* FIXME: arrange so this doesn't print a
 	 * decimal point unless we have a
 	 * fractional part. */
-	return snprintf(buffer, s, "# %g %s", d, _units[i]) > 0;
+	return lvm_snprintf(buffer, s, "# %g %s", d, _units[i]) > 0;
 }
 
 /*
diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c
index e70092b8..2cf6b3aa 100644
--- a/lib/metadata/lv_manip.c
+++ b/lib/metadata/lv_manip.c
@@ -8,6 +8,7 @@
 #include "pv_map.h"
 #include "log.h"
 #include "dbg_malloc.h"
+#include "lvm-string.h"
 
 #include <assert.h>
 
@@ -353,7 +354,7 @@ static char *_generate_lv_name(struct volume_group *vg,
 			high = i;
 	}
 
-	if ((s = snprintf(buffer, len, "lvol%d", high + 1)) < 0 || s >= len)
+	if (lvm_snprintf(buffer, len, "lvol%d", high + 1) < 0)
 		return NULL;
 
 	return buffer;
diff --git a/man/lvm.conf.5 b/man/lvm.conf.5
index abc116da..daed3dba 100644
--- a/man/lvm.conf.5
+++ b/man/lvm.conf.5
@@ -46,8 +46,17 @@ is invoked.  By default tools append messages to the log file.
 .TP
 \fBbackups\fP \(em Configuration for metadata backups
 .IP
-\fBdir\fP \(em Directory used for automatic metadata backups
-(defaults to "/etc/lvm")
+\fBdir\fP \(em Directory used for automatic metadata backups.
+Defaults to "/etc/lvm/backup".
+.IP
+\fBkeep\fP \(em Minimum number of backups to keep.
+Defaults to 10.
+.IP
+\fBdays\fP \(em Minimum number of days to keep backup files.
+Defaults to 14.
+.IP
+\fBauto\fP \(em Whether or not tools make automatic backups after changing
+metadata.  Default is 1 (automatic backups enabled).  Set to 0 to disable.
 .TP
 \fBshell\fP \(em LVM2 built-in readline shell settings
 .IP
diff --git a/tools/lvcreate.c b/tools/lvcreate.c
index 049ff029..c2064704 100644
--- a/tools/lvcreate.c
+++ b/tools/lvcreate.c
@@ -233,13 +233,17 @@ int lvcreate(int argc, char **argv)
 		struct device *dev;
 		char *name;
 
-		if (!(name = dbg_malloc(NAME_LEN))) {
+		if (!(name = dbg_malloc(PATH_MAX))) {
 			log_error("Name allocation failed - device not zeroed");
 			return ECMD_FAILED;
 		}
 
-		snprintf(name, NAME_LEN, "%s%s/%s", fid->cmd->dev_dir,
-			 lv->vg->name, lv->name);
+		if (lvm_snprintf(name, PATH_MAX, "%s%s/%s", fid->cmd->dev_dir,
+				 lv->vg->name, lv->name) < 0) {
+			log_error("Name too long - device not zeroed (%s)", 
+				  lv->name);
+			return ECMD_FAILED;
+		}
 
 		log_verbose("Zeroing start of logical volume %s", name);
 
diff --git a/tools/lvm.c b/tools/lvm.c
index 02e0f8a8..61b06089 100644
--- a/tools/lvm.c
+++ b/tools/lvm.c
@@ -13,7 +13,6 @@
 #include <libgen.h>
 #include <sys/stat.h>
 #include <ctype.h>
-#include <limits.h>
 
 #include "stub.h"
 #include "vgcache.h"
@@ -68,21 +67,25 @@ static int _debug;
 static int _default_verbose;
 static int _verbose;
 
-
 /*
  * The lvm_sys_dir contains:
  *
  * o  The lvm configuration (lvm.conf)
  * o  The persistent filter cache (.cache)
- * o  Volume group backups (backups)
+ * o  Volume group backups (backup/)
  *
  */
 static char _sys_dir[PATH_MAX] = "/etc/lvm";
 static char _backup_dir[PATH_MAX];
 static char _dev_dir[PATH_MAX];
 
+static int _backup_days = 14; 		/* Keep at least 14 days */
+static int _backup_number = 10;		/* Keep at least 10 backups */
+static int _backup_auto = 1;		/* Autobackups enabled by default */
+
 #define DEFAULT_DEV_DIR "/dev"
 
+
 /* static functions */
 static void register_commands(void);
 static struct command *find_command(const char *name);
@@ -548,7 +551,8 @@ static int process_common_commands(struct command *com)
 	/* Set autobackup if command takes this option */
 	for (l = 0; l < com->num_args; l++)
 		if (com->valid_args[l] == autobackup_ARG)
-			if (!autobackup_init(_backup_dir))
+			if (!autobackup_init(_backup_dir, _backup_days,
+					     _backup_number, _backup_auto))
 				return EINVALID_CMD_LINE;
 
 	/* Zero indicates it's OK to continue processing this command */
@@ -845,13 +849,22 @@ static int init(void)
 
 	dm_log_init(print_log);
 
-	if (lvm_snprintf(_backup_dir, sizeof(_backup_dir), "%s",
+	if (lvm_snprintf(_backup_dir, sizeof(_backup_dir), "%s/backup",
         		 find_config_str(cmd->cf->root, "backup/dir", 
 					 '/', _sys_dir)) < 0) {
 		log_error("Backup directory given in config file too long");
 		return 0;
 	}
 
+	_backup_days = find_config_int(cmd->cf->root, "backup/days", '/', 
+				       _backup_days);
+
+	_backup_number = find_config_int(cmd->cf->root, "backup/keep", '/', 
+					 _backup_number);
+
+	_backup_auto = find_config_int(cmd->cf->root, "backup/auto", '/', 
+					 _backup_auto);
+
 	if (!dev_cache_setup(cmd->cf))
 		return 0;
 
diff --git a/tools/toollib.c b/tools/toollib.c
index 2581265c..ae6e220e 100644
--- a/tools/toollib.c
+++ b/tools/toollib.c
@@ -5,52 +5,45 @@
  */
 
 #include "tools.h"
-#include "format-text.h"
-#include "metadata.h"
-#include "lvm-string.h"
 
-#include <ctype.h>
-#include <limits.h>
-
-static int _autobackup = 1;
+static int _autobackup;
 static char _backup_dir[PATH_MAX];
-static int _period = 7;		/* backups will be kept for at least 7 days */
-static int _min_backups = 10;	/* always have at least ten backups, even
-				 * if they're older than the period */
+static int _keep_days;		/* keep for at least this number of days */
+static int _keep_number;	/* keep at least this number of backups */
 
 /*
- * Work out by looking at command line, config
- * file and environment variable whether we should
- * do an autobackup.
+ * Determine whether or not to do autobackup.  
+ * Cmd line overrides environment variable which in turn overrides config file
  */
-int autobackup_init(const char *system_dir)
+int autobackup_init(const char *backup_dir, int keep_days, int keep_number,
+		    int autobackup)
 {
 	char *lvm_autobackup;
 
 	if (lvm_snprintf(_backup_dir, sizeof(_backup_dir), 
-			 "%s/backup", system_dir) < 0) {
-		log_err("Backup directory (%s/backup) too long.", system_dir);
+			 "%s", backup_dir) < 0) {
+		log_error("Backup directory name too long.");
 		return 0;
 	}
 
+	_keep_days = keep_days;
+	_keep_number = keep_number;
+	_autobackup = autobackup;	/* Config file setting */
+
 	if (arg_count(autobackup_ARG)) {
 		_autobackup = !strcmp(arg_str_value(autobackup_ARG, "y"), "y");
 		return 1;
 	}
 
-	_autobackup = 1;	/* default */
-
 	lvm_autobackup = getenv("LVM_AUTOBACKUP");
 	if (!lvm_autobackup)
 		return 1;
 
-	log_print("using environment variable LVM_AUTOBACKUP "
-		  "to set option A");
+	log_verbose("Setting autobackup from environment (LVM_AUTOBACKUP)");
 	if (!strcasecmp(lvm_autobackup, "no"))
 		_autobackup = 0;
-
 	else if (strcasecmp(lvm_autobackup, "yes")) {
-		log_error("environment variable LVM_AUTOBACKUP has "
+		log_error("Environment variable LVM_AUTOBACKUP has "
 			  "invalid value \"%s\"!", lvm_autobackup);
 		return 0;
 	}
@@ -78,8 +71,8 @@ static int __autobackup(struct volume_group *vg)
 	}
 
 	if (!(backer = backup_format_create(vg->cmd, _backup_dir,
-					    _period, _min_backups))) {
-		log_err("Couldn't create backup object.");
+					    _keep_days, _keep_number))) {
+		log_error("Couldn't create backup object.");
 		return 0;
 	}
 
diff --git a/tools/toollib.h b/tools/toollib.h
index 559ee36a..5a222ca8 100644
--- a/tools/toollib.h
+++ b/tools/toollib.h
@@ -22,7 +22,8 @@
 #define _LVM_TOOLLIB_H
 
 int autobackup_set(void);
-int autobackup_init(const char *system_dir);
+int autobackup_init(const char *backup_dir, int keep_days, int keep_number,
+		    int autobackup);
 int autobackup(struct volume_group *vg);
 
 int process_each_vg(int argc, char **argv,
diff --git a/tools/tools.h b/tools/tools.h
index c9a1934f..90851fb9 100644
--- a/tools/tools.h
+++ b/tools/tools.h
@@ -25,12 +25,15 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include <sys/types.h>
+#include <ctype.h>
 #include <string.h>
+#include <limits.h>
 
 #include "pool.h"
 #include "dbg_malloc.h"
 #include "list.h"
 #include "log.h"
+#include "lvm-string.h"
 #include "metadata.h"
 #include "config.h"
 #include "dev-cache.h"
@@ -42,6 +45,7 @@
 #include "filter-composite.h"
 #include "filter-regex.h"
 #include "format1.h"
+#include "format-text.h"
 #include "toollib.h"
 #include "activate.h"