Make a cleaner split between config tree and config file functionality. Move

the latter out of libdm.

lib/cache/lvmcache.c

@@ -85,7 +85,7 @@ static void _free_cached_vgmetadata(struct lvmcache_vginfo *vginfo)
 
 	/* Release also cached config tree */
 	if (vginfo->cft) {
-		destroy_config_tree(vginfo->cft);
+		dm_config_destroy(vginfo->cft);
 		vginfo->cft = NULL;
 	}
 

lib/commands/toolcontext.c

@@ -500,7 +500,7 @@ static int _load_config_file(struct cmd_context *cmd, const char *tag)
 		return 0;
 	}
 
-	if (!(cfl->cft = dm_config_create(config_file, 0))) {
+	if (!(cfl->cft = config_file_open(config_file, 0))) {
 		log_error("config_tree allocation failed");
 		return 0;
 	}
@@ -512,14 +512,14 @@ static int _load_config_file(struct cmd_context *cmd, const char *tag)
 			goto out;
 		}
 		log_sys_error("stat", config_file);
-		destroy_config_tree(cfl->cft);
+		config_file_destroy(cfl->cft);
 		return 0;
 	}
 
 	log_very_verbose("Loading config file: %s", config_file);
-	if (!read_config_file(cfl->cft)) {
+	if (!config_file_read(cfl->cft)) {
 		log_error("Failed to load config file %s", config_file);
-		destroy_config_tree(cfl->cft);
+		config_file_destroy(cfl->cft);
 		return 0;
 	}
 
@@ -540,7 +540,7 @@ static int _init_lvm_conf(struct cmd_context *cmd)
 {
 	/* No config file if LVM_SYSTEM_DIR is empty */
 	if (!*cmd->system_dir) {
-		if (!(cmd->cft = dm_config_create(NULL, 0))) {
+		if (!(cmd->cft = config_file_open(NULL, 0))) {
 			log_error("Failed to create config tree");
 			return 0;
 		}
@@ -573,7 +573,7 @@ static struct dm_config_tree *_merge_config_files(struct cmd_context *cmd, struc
 
 	/* Replace temporary duplicate copy of lvm.conf */
 	if (cft->root) {
-		if (!(cft = dm_config_create(NULL, 0))) {
+		if (!(cft = config_file_open(NULL, 0))) {
 			log_error("Failed to create config tree");
 			return 0;
 		}
@@ -602,7 +602,7 @@ int config_files_changed(struct cmd_context *cmd)
 	struct config_tree_list *cfl;
 
 	dm_list_iterate_items(cfl, &cmd->config_files) {
-		if (dm_config_changed(cfl->cft))
+		if (config_file_changed(cfl->cft))
 			return 1;
 	}
 
@@ -627,11 +627,11 @@ static struct dm_config_tree *_destroy_tag_configs(struct cmd_context *cmd)
 	dm_list_iterate_items(cfl, &cmd->config_files) {
 		if (cfl->cft == cmd->cft)
 			cmd->cft = NULL;
-		destroy_config_tree(cfl->cft);
+		config_file_destroy(cfl->cft);
 	}
 
 	if (cmd->cft) {
-		destroy_config_tree(cmd->cft);
+		config_file_destroy(cmd->cft);
 		cmd->cft = NULL;
 	}
 
@@ -852,7 +852,7 @@ static int _init_filters(struct cmd_context *cmd, unsigned load_persistent_cache
 	 */
 	if (load_persistent_cache && !cmd->is_long_lived &&
 	    !stat(dev_cache, &st) &&
-	    (st.st_ctime > dm_config_timestamp(cmd->cft)) &&
+	    (st.st_ctime > config_file_timestamp(cmd->cft)) &&
 	    !persistent_filter_load(f4, NULL))
 		log_verbose("Failed to load existing device cache from %s",
 			    dev_cache);

lib/config/config.c

@@ -29,13 +29,128 @@
 #include <unistd.h>
 #include <fcntl.h>
 #include <ctype.h>
+#include <assert.h>
 
-void destroy_config_tree(struct dm_config_tree *cft)
+struct config_file {
+	time_t timestamp;
+	off_t st_size;
+	char *filename;
+	int exists;
+	int keep_open;
+	struct device *dev;
+};
+
+/*
+ * public interface
+ */
+struct dm_config_tree *config_file_open(const char *filename, int keep_open)
 {
-	struct device *dev = dm_config_get_custom(cft);
+	struct dm_config_tree *cft = dm_config_create();
+	struct config_file *cf;
+	if (!cft)
+		return NULL;
 
-	if (dev)
+	cf = dm_pool_zalloc(cft->mem, sizeof(struct config_file));
-		dev_close(dev);
+	if (!cf) goto fail;
+
+	cf->timestamp = 0;
+	cf->exists = 0;
+	cf->keep_open = keep_open;
+	dm_config_set_custom(cft, cf);
+
+	if (filename &&
+	    !(cf->filename = dm_pool_strdup(cft->mem, filename))) {
+		log_error("Failed to duplicate filename.");
+		goto fail;
+	}
+
+	return cft;
+fail:
+	dm_config_destroy(cft);
+	return NULL;
+}
+
+/*
+ * Doesn't populate filename if the file is empty.
+ */
+int config_file_check(struct dm_config_tree *cft, const char **filename, struct stat *info)
+{
+	struct config_file *cf = dm_config_get_custom(cft);
+	struct stat _info;
+
+	if (!info)
+		info = &_info;
+
+	if (stat(cf->filename, info)) {
+		log_sys_error("stat", cf->filename);
+		cf->exists = 0;
+		return 0;
+	}
+
+	if (!S_ISREG(info->st_mode)) {
+		log_error("%s is not a regular file", cf->filename);
+		cf->exists = 0;
+		return 0;
+	}
+
+	cf->exists = 1;
+	cf->timestamp = info->st_ctime;
+	cf->st_size = info->st_size;
+
+	if (info->st_size == 0)
+		log_verbose("%s is empty", cf->filename);
+	else if (filename)
+		*filename = cf->filename;
+
+	return 1;
+}
+
+/*
+ * Return 1 if config files ought to be reloaded
+ */
+int config_file_changed(struct dm_config_tree *cft)
+{
+	struct config_file *cf = dm_config_get_custom(cft);
+	struct stat info;
+
+	if (!cf->filename)
+		return 0;
+
+	if (stat(cf->filename, &info) == -1) {
+		/* Ignore a deleted config file: still use original data */
+		if (errno == ENOENT) {
+			if (!cf->exists)
+				return 0;
+			log_very_verbose("Config file %s has disappeared!",
+					 cf->filename);
+			goto reload;
+		}
+		log_sys_error("stat", cf->filename);
+		log_error("Failed to reload configuration files");
+		return 0;
+	}
+
+	if (!S_ISREG(info.st_mode)) {
+		log_error("Configuration file %s is not a regular file",
+			  cf->filename);
+		goto reload;
+	}
+
+	/* Unchanged? */
+	if (cf->timestamp == info.st_ctime && cf->st_size == info.st_size)
+		return 0;
+
+      reload:
+	log_verbose("Detected config file change to %s", cf->filename);
+	return 1;
+}
+
+void config_file_destroy(struct dm_config_tree *cft)
+{
+	struct config_file *cf = dm_config_get_custom(cft);
+
+	if (cf && cf->dev)
+		dev_close(cf->dev);
 
 	dm_config_destroy(cft);
 }
@@ -74,7 +189,7 @@ int override_config_tree_from_string(struct cmd_context *cmd,
 	return 0;
 }
 
-int read_config_fd(struct dm_config_tree *cft, struct device *dev,
+int config_file_read_fd(struct dm_config_tree *cft, struct device *dev,
 			off_t offset, size_t size, off_t offset2, size_t size2,
 			checksum_fn_t checksum_fn, uint32_t checksum)
 {
@@ -137,40 +252,46 @@ int read_config_fd(struct dm_config_tree *cft, struct device *dev,
 	return r;
 }
 
-int read_config_file(struct dm_config_tree *cft)
+int config_file_read(struct dm_config_tree *cft)
 {
 	const char *filename = NULL;
-	struct device *dev = dm_config_get_custom(cft);
+	struct config_file *cf = dm_config_get_custom(cft);
 	struct stat info;
 	int r;
 
-	if (!dm_config_check_file(cft, &filename, &info))
+	if (!config_file_check(cft, &filename, &info))
 		return_0;
 
 	/* Nothing to do.  E.g. empty file. */
 	if (!filename)
 		return 1;
 
-	if (!dev) {
+	if (!cf->dev) {
-		if (!(dev = dev_create_file(filename, NULL, NULL, 1)))
+		if (!(cf->dev = dev_create_file(filename, NULL, NULL, 1)))
 			return_0;
 
-		if (!dev_open_readonly_buffered(dev))
+		if (!dev_open_readonly_buffered(cf->dev))
 			return_0;
 	}
 
-	dm_config_set_custom(cft, dev);
+	r = config_file_read_fd(cft, cf->dev, 0, (size_t) info.st_size, 0, 0,
-	r = read_config_fd(cft, dev, 0, (size_t) info.st_size, 0, 0,
 				(checksum_fn_t) NULL, 0);
 
-	if (!dm_config_keep_open(cft)) {
+	if (!cf->keep_open) {
-		dev_close(dev);
+		dev_close(cf->dev);
-		dm_config_set_custom(cft, NULL);
+		cf->dev = NULL;
 	}
 
 	return r;
 }
 
+time_t config_file_timestamp(struct dm_config_tree *cft)
+{
+	struct config_file *cf = dm_config_get_custom(cft);
+	assert(cf);
+	return cf->timestamp;
+}
+
 const struct dm_config_node *find_config_tree_node(struct cmd_context *cmd,
 						   const char *path)
 {

lib/config/config.h

@@ -23,19 +23,23 @@ struct cmd_context;
 
 int override_config_tree_from_string(struct cmd_context *cmd,
 				     const char *config_settings);
-void destroy_config_tree(struct dm_config_tree *cft);
 struct dm_config_tree *remove_overridden_config_tree(struct cmd_context *cmd);
 
 typedef uint32_t (*checksum_fn_t) (uint32_t initial, const uint8_t *buf, uint32_t size);
 
-int read_config_fd(struct dm_config_tree *cft, struct device *dev,
+struct dm_config_tree *config_file_open(const char *filename, int keep_open);
+int config_file_read_fd(struct dm_config_tree *cft, struct device *dev,
 			off_t offset, size_t size, off_t offset2, size_t size2,
 			checksum_fn_t checksum_fn, uint32_t checksum);
-
+	int config_file_read(struct dm_config_tree *cft);
 int config_write(struct dm_config_tree *cft, const char *file,
 		 int argc, char **argv);
+void config_file_destroy(struct dm_config_tree *cft);
+
+time_t config_file_timestamp(struct dm_config_tree *cft);
+int config_file_changed(struct dm_config_tree *cft);
+int config_file_check(struct dm_config_tree *cft, const char **filename, struct stat *info);
 
-int read_config_file(struct dm_config_tree *cft);
 
 int merge_config_tree(struct cmd_context *cmd, struct dm_config_tree *cft,
 		      struct dm_config_tree *newdata);

lib/filters/filter-persistent.c

@@ -122,10 +122,10 @@ int persistent_filter_load(struct dev_filter *f, struct dm_config_tree **cft_out
 		return_0;
 	}
 
-	if (!(cft = dm_config_create(pf->file, 1)))
+	if (!(cft = config_file_open(pf->file, 1)))
 		return_0;
 
-	if (!read_config_file(cft))
+	if (!config_file_read(cft))
 		goto_out;
 
 	_read_array(pf, cft, "persistent_filter_cache/valid_devices",
@@ -147,7 +147,7 @@ int persistent_filter_load(struct dev_filter *f, struct dm_config_tree **cft_out
 	if (r && cft_out)
 		*cft_out = cft;
 	else
-		destroy_config_tree(cft);
+		config_file_destroy(cft);
 	return r;
 }
 
@@ -271,7 +271,7 @@ out:
 	fcntl_unlock_file(lockfd);
 
 	if (cft)
-		destroy_config_tree(cft);
+		config_file_destroy(cft);
 
 	return r;
 }

lib/format_text/import.c

@@ -49,11 +49,11 @@ const char *text_vgname_import(const struct format_type *fmt,
 
 	_init_text_import();
 
-	if (!(cft = dm_config_create(NULL, 0)))
+	if (!(cft = config_file_open(NULL, 0)))
 		return_NULL;
 
-	if ((!dev && !read_config_file(cft)) ||
+	if ((!dev && !config_file_read(cft)) ||
-	    (dev && !read_config_fd(cft, dev, offset, size,
+	    (dev && !config_file_read_fd(cft, dev, offset, size,
 					 offset2, size2, checksum_fn, checksum)))
 		goto_out;
 
@@ -72,7 +72,7 @@ const char *text_vgname_import(const struct format_type *fmt,
 	}
 
       out:
-	destroy_config_tree(cft);
+	config_file_destroy(cft);
 	return vgname;
 }
 
@@ -94,11 +94,11 @@ struct volume_group *text_vg_import_fd(struct format_instance *fid,
 	*desc = NULL;
 	*when = 0;
 
-	if (!(cft = dm_config_create(file, 0)))
+	if (!(cft = config_file_open(file, 0)))
 		return_NULL;
 
-	if ((!dev && !read_config_file(cft)) ||
+	if ((!dev && !config_file_read(cft)) ||
-	    (dev && !read_config_fd(cft, dev, offset, size,
+	    (dev && !config_file_read_fd(cft, dev, offset, size,
 					 offset2, size2, checksum_fn, checksum))) {
 		log_error("Couldn't read volume group metadata.");
 		goto out;
@@ -119,7 +119,7 @@ struct volume_group *text_vg_import_fd(struct format_instance *fid,
 	}
 
       out:
-	destroy_config_tree(cft);
+	config_file_destroy(cft);
 	return vg;
 }
 

libdm/libdevmapper.h

@@ -1321,16 +1321,15 @@ struct dm_config_node {
 struct dm_config_tree {
 	struct dm_config_node *root;
 	struct dm_config_tree *cascade;
+	struct dm_pool *mem;
+	void *custom;
 };
 
-struct dm_config_tree *dm_config_create(const char *filename, int keep_open);
+struct dm_config_tree *dm_config_create(void);
 struct dm_config_tree *dm_config_from_string(const char *config_settings);
 int dm_config_parse(struct dm_config_tree *cft, const char *start, const char *end);
 
 void *dm_config_get_custom(struct dm_config_tree *cft);
-int dm_config_check_file(struct dm_config_tree *cft, const char **filename, struct stat *info);
-int dm_config_keep_open(struct dm_config_tree *ctf);
-
 void dm_config_set_custom(struct dm_config_tree *cft, void *custom);
 
 /*
@@ -1349,9 +1348,6 @@ void dm_config_destroy(struct dm_config_tree *cft);
 typedef int (*dm_putline_fn)(const char *line, void *baton);
 int dm_config_write_node(const struct dm_config_node *cn, dm_putline_fn putline, void *baton);
 
-time_t dm_config_timestamp(struct dm_config_tree *cft);
-int dm_config_changed(struct dm_config_tree *cft);
-
 struct dm_config_node *dm_config_find_node(struct dm_config_node *cn, const char *path);
 int dm_config_has_node(const struct dm_config_node *cn, const char *path);
 const char *dm_config_find_str(const struct dm_config_node *cn, const char *path, const char *fail);

libdm/libdm-common.c

@@ -900,7 +900,7 @@ static int _stack_node_op(node_op_t type, const char *dev_name, uint32_t major,
 				break; /* no other DEL ops */
 			}
 		}
-	else if ((type == NODE_RENAME))
+	else if (type == NODE_RENAME)
 		/*
 		 * Ignore any outstanding operations if renaming it.
 		 *

libdm/libdm-config.c

@@ -50,17 +50,6 @@ struct parser {
 	struct dm_pool *mem;
 };
 
-struct cs {
-	struct dm_config_tree cft;
-	struct dm_pool *mem;
-	time_t timestamp;
-	off_t st_size;
-	char *filename;
-	int exists;
-	int keep_open; // FIXME AGK Remove this before release
-	void *custom; /* LVM uses this for a device pointer */
-};
-
 struct output_line {
 	struct dm_pool *mem;
 	dm_putline_fn putline;
@@ -100,12 +89,9 @@ static int _tok_match(const char *str, const char *b, const char *e)
 	return !(*str || (b != e));
 }
 
-/*
+struct dm_config_tree *dm_config_create()
- * public interface
- */
-struct dm_config_tree *dm_config_create(const char *filename, int keep_open)
 {
-	struct cs *c;
+	struct dm_config_tree *cft;
 	struct dm_pool *mem = dm_pool_create("config", 10 * 1024);
 
 	if (!mem) {
@@ -113,54 +99,31 @@ struct dm_config_tree *dm_config_create(const char *filename, int keep_open)
 		return 0;
 	}
 
-	if (!(c = dm_pool_zalloc(mem, sizeof(*c)))) {
+	if (!(cft = dm_pool_zalloc(mem, sizeof(*cft)))) {
 		log_error("Failed to allocate config tree.");
 		dm_pool_destroy(mem);
 		return 0;
 	}
-
+	cft->root = NULL;
-	c->mem = mem;
+	cft->cascade = NULL;
-	c->cft.root = (struct dm_config_node *) NULL;
+	cft->custom = NULL;
-	c->cft.cascade = NULL;
+	cft->mem = mem;
-	c->timestamp = 0;
+	return cft;
-	c->exists = 0;
-	c->keep_open = keep_open;
-	c->custom = NULL;
-	if (filename &&
-	    !(c->filename = dm_pool_strdup(c->mem, filename))) {
-		log_error("Failed to duplicate filename.");
-		dm_pool_destroy(mem);
-		return 0;
-	}
-
-	return &c->cft;
 }
 
 void dm_config_set_custom(struct dm_config_tree *cft, void *custom)
 {
-	struct cs *c = (struct cs *) cft;
+	cft->custom = custom;
-
-	c->custom = custom;
 }
 
 void *dm_config_get_custom(struct dm_config_tree *cft)
 {
-	struct cs *c = (struct cs *) cft;
+	return cft->custom;
-
-	return c->custom;
-}
-
-int dm_config_keep_open(struct dm_config_tree *cft)
-{
-	struct cs *c = (struct cs *) cft;
-
-	return c->keep_open;
 }
 
 void dm_config_destroy(struct dm_config_tree *cft)
 {
-	struct cs *c = (struct cs *) cft;
+	dm_pool_destroy(cft->mem);
-	dm_pool_destroy(c->mem);
 }
 
 /*
@@ -194,12 +157,11 @@ int dm_config_parse(struct dm_config_tree *cft, const char *start, const char *e
 {
 	/* TODO? if (start == end) return 1; */
 
-	struct cs *c = (struct cs *) cft;
 	struct parser *p;
-	if (!(p = dm_pool_alloc(c->mem, sizeof(*p))))
+	if (!(p = dm_pool_alloc(cft->mem, sizeof(*p))))
 		return_0;
 
-	p->mem = c->mem;
+	p->mem = cft->mem;
 	p->fb = start;
 	p->fe = end;
 	p->tb = p->te = p->fb;
@@ -216,7 +178,7 @@ struct dm_config_tree *dm_config_from_string(const char *config_settings)
 {
 	struct dm_config_tree *cft;
 
-	if (!(cft = dm_config_create(NULL, 0)))
+	if (!(cft = dm_config_create()))
 		return_NULL;
 
 	if (!dm_config_parse(cft, config_settings, config_settings + strlen(config_settings))) {
@@ -227,88 +189,6 @@ struct dm_config_tree *dm_config_from_string(const char *config_settings)
 	return cft;
 }
 
-/*
- * Doesn't populate filename if the file is empty.
- */
-int dm_config_check_file(struct dm_config_tree *cft, const char **filename, struct stat *info)
-{
-	struct cs *c = (struct cs *) cft;
-	struct stat _info;
-
-	if (!info)
-		info = &_info;
-
-	if (stat(c->filename, info)) {
-		log_sys_error("stat", c->filename);
-		c->exists = 0;
-		return 0;
-	}
-
-	if (!S_ISREG(info->st_mode)) {
-		log_error("%s is not a regular file", c->filename);
-		c->exists = 0;
-		return 0;
-	}
-
-	c->exists = 1;
-	c->timestamp = info->st_ctime;
-	c->st_size = info->st_size;
-
-	if (info->st_size == 0)
-		log_verbose("%s is empty", c->filename);
-	else if (filename)
-		*filename = c->filename;
-
-	return 1;
-}
-
-time_t dm_config_timestamp(struct dm_config_tree *cft)
-{
-	struct cs *c = (struct cs *) cft;
-
-	return c->timestamp;
-}
-
-/*
- * Return 1 if config files ought to be reloaded
- */
-int dm_config_changed(struct dm_config_tree *cft)
-{
-	struct cs *c = (struct cs *) cft;
-	struct stat info;
-
-	if (!c->filename)
-		return 0;
-
-	if (stat(c->filename, &info) == -1) {
-		/* Ignore a deleted config file: still use original data */
-		if (errno == ENOENT) {
-			if (!c->exists)
-				return 0;
-			log_very_verbose("Config file %s has disappeared!",
-					 c->filename);
-			goto reload;
-		}
-		log_sys_error("stat", c->filename);
-		log_error("Failed to reload configuration files");
-		return 0;
-	}
-
-	if (!S_ISREG(info.st_mode)) {
-		log_error("Configuration file %s is not a regular file",
-			  c->filename);
-		goto reload;
-	}
-
-	/* Unchanged? */
-	if (c->timestamp == info.st_ctime && c->st_size == info.st_size)
-		return 0;
-
-      reload:
-	log_verbose("Detected config file change to %s", c->filename);
-	return 1;
-}
-
 static int _line_start(struct output_line *outline)
 {
 	if (!dm_pool_begin_object(outline->mem, 128)) {
@@ -1242,24 +1122,22 @@ struct dm_config_node *dm_config_clone_node_with_mem(struct dm_pool *mem, const
 
 struct dm_config_node *dm_config_clone_node(struct dm_config_tree *cft, const struct dm_config_node *node, int sib)
 {
-	struct cs *c = (struct cs *) cft;
+	return dm_config_clone_node_with_mem(cft->mem, node, sib);
-	return dm_config_clone_node_with_mem(c->mem, node, sib);
 }
 
 struct dm_config_node *dm_config_create_node(struct dm_config_tree *cft, const char *key)
 {
-	struct cs *c = (struct cs *) cft;
 	struct dm_config_node *cn;
 
-	if (!(cn = _create_node(c->mem))) {
+	if (!(cn = _create_node(cft->mem))) {
 		log_error("Failed to create config node.");
 		return NULL;
 	}
-	if (!(cn->key = dm_pool_strdup(c->mem, key))) {
+	if (!(cn->key = dm_pool_strdup(cft->mem, key))) {
 		log_error("Failed to create config node's key.");
 		return NULL;
 	}
-	if (!(cn->v = _create_value(c->mem))) {
+	if (!(cn->v = _create_value(cft->mem))) {
 		log_error("Failed to create config node's value.");
 		return NULL;
 	}
@@ -1272,12 +1150,10 @@ struct dm_config_node *dm_config_create_node(struct dm_config_tree *cft, const c
 
 struct dm_config_value *dm_config_create_value(struct dm_config_tree *cft)
 {
-	struct cs *c = (struct cs *) cft;
+	return _create_value(cft->mem);
-	return _create_value(c->mem);
 }
 
 struct dm_pool *dm_config_memory(struct dm_config_tree *cft)
 {
-	struct cs *c = (struct cs *) cft;
+	return cft->mem;
-	return c->mem;
 }