Refactor some vginfo manipulation code.

WHATS_NEW

@@ -6,6 +6,7 @@ Version 2.02.38 -
 
 Version 2.02.37 - 
 =================================
+  Refactor some vginfo manipulation code.
   Add assertions to trap deprecated P_ and V_ lock usage.
   Add missing mutex around clvmd lvmcache_drop_metadata library call.
   Fix uninitialised mutex in clvmd if all daemons are not running at startup.

configure.in

@@ -539,9 +539,10 @@ AC_ARG_WITH(dmdir,
 	    [ DMDIR= ])
 
 # Convert a relative dir name to absolute.
-case $DMDIR in
+case x$DMDIR in
-  /*) ;;
+  x/*) ;;
-  *) DMDIR="`pwd`/$DMDIR" ;;
+  x) ;;
+  x*) DMDIR="`pwd`/$DMDIR" ;;
 esac
 
 ################################################################################
@@ -601,6 +602,7 @@ AC_SUBST(MIRRORS)
 AC_SUBST(OWNER)
 AC_SUBST(GROUP)
 AC_SUBST(CFLAGS)
+AC_SUBST(CPPFLAGS)
 AC_SUBST(COPTIMISE_FLAG)
 AC_SUBST(CLDFLAGS)
 AC_SUBST(CLDWHOLEARCHIVE)

daemons/clvmd/Makefile.in

@@ -83,7 +83,7 @@ INSTALL_TARGETS = \
 	install_clvmd
 
 clvmd: $(OBJECTS) $(top_srcdir)/lib/liblvm.a
-	$(CC) -o clvmd $(OBJECTS) $(CFLAGS) $(LDFLAGS) \
+	$(CC) -o clvmd $(OBJECTS) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) \
 		$(LVMLIBS) $(LMLIBS) $(LIBS)
 
 .PHONY: install_clvmd

lib/cache/lvmcache.c

@@ -217,6 +217,23 @@ int vgs_locked(void)
 	return _vgs_locked;
 }
 
+static void _vginfo_attach_info(struct lvmcache_vginfo *vginfo,
+				struct lvmcache_info *info)
+{
+	info->vginfo = vginfo;
+	list_add(&vginfo->infos, &info->list);
+}
+
+static void _vginfo_detach_info(struct lvmcache_info *info)
+{
+	if (!list_empty(&info->list)) {
+		list_del(&info->list);
+		list_init(&info->list);
+	}
+
+	info->vginfo = NULL;
+}
+
 /* If vgid supplied, require a match. */
 struct lvmcache_vginfo *vginfo_from_vgname(const char *vgname, const char *vgid)
 {
@@ -336,6 +353,7 @@ static int _vginfo_is_valid(struct lvmcache_vginfo *vginfo)
 {
 	struct lvmcache_info *info;
 
+	/* Invalid if any info is invalid */
 	list_iterate_items(info, &vginfo->infos)
 		if (!_info_is_valid(info))
 			return 0;
@@ -580,38 +598,51 @@ struct device *device_from_pvid(struct cmd_context *cmd, struct id *pvid)
 	return NULL;
 }
 
+static int _free_vginfo(struct lvmcache_vginfo *vginfo)
+{
+	int r = 1;
+
+	_free_cached_vgmetadata(vginfo);
+
+	dm_hash_remove(_vgname_hash, vginfo->vgname);
+	if (vginfo->next && !dm_hash_insert(_vgname_hash, vginfo->vgname,
+					    vginfo->next)) {
+		log_error("_vgname_hash re-insertion for %s failed",
+			  vginfo->vgname);
+		r = 0;
+	}
+
+	if (vginfo->vgname)
+		dm_free(vginfo->vgname);
+
+	if (vginfo->creation_host)
+		dm_free(vginfo->creation_host);
+
+	if (*vginfo->vgid && _vgid_hash)
+		dm_hash_remove(_vgid_hash, vginfo->vgid);
+
+	list_del(&vginfo->list);
+
+	dm_free(vginfo);
+
+	return r;
+}
+
 /*
  * vginfo must be info->vginfo unless info is NULL
  */
 static int _drop_vginfo(struct lvmcache_info *info, struct lvmcache_vginfo *vginfo)
 {
-	if (info && !list_empty(&info->list)) {
-		list_del(&info->list);
-		list_init(&info->list);
-	}
-
-	if (vginfo && !is_orphan_vg(vginfo->vgname) && list_empty(&vginfo->infos)) {
-		dm_hash_remove(_vgname_hash, vginfo->vgname);
-		if (vginfo->next) {
-			if (!dm_hash_insert(_vgname_hash, vginfo->vgname, vginfo->next)) {
-				log_error("vg hash re-insertion failed: %s",
-					  vginfo->vgname);
-				return 0;
-			}
-		}
-
-		if (vginfo->vgname)
-			dm_free(vginfo->vgname);
-		if (vginfo->creation_host)
-			dm_free(vginfo->creation_host);
-		if (*vginfo->vgid)
-			dm_hash_remove(_vgid_hash, vginfo->vgid);
-		list_del(&vginfo->list);
-		dm_free(info->vginfo);
-	}
-
 	if (info)
-		info->vginfo = NULL;
+		_vginfo_detach_info(info);
+
+	/* vginfo still referenced? */
+	if (!vginfo || is_orphan_vg(vginfo->vgname) ||
+	    !list_empty(&vginfo->infos))
+		return 1;
+
+	if (!_free_vginfo(vginfo))
+		return_0;
 
 	return 1;
 }
@@ -829,7 +860,7 @@ static int _lvmcache_update_vgname(struct lvmcache_info *info,
 			return 0;
 		}
 		list_init(&vginfo->infos);
-			primary_vginfo = vginfo_from_vgname(vgname, NULL);
+		primary_vginfo = vginfo_from_vgname(vgname, NULL);
 		if (!_insert_vginfo(vginfo, vgid, vgstatus, creation_host,
 				    primary_vginfo)) {
 			dm_free(vginfo->vgname);
@@ -846,10 +877,9 @@ static int _lvmcache_update_vgname(struct lvmcache_info *info,
 ***/
 	}
 
-	if (info) {
+	if (info)
-		info->vginfo = vginfo;
+		_vginfo_attach_info(vginfo, info);
-		list_add(&vginfo->infos, &info->list);
+	else if (!_lvmcache_update_vgid(NULL, vginfo, vgid)) /* Orphans */
-	} else if (!_lvmcache_update_vgid(info, vginfo, vgid)) /* Orphans */
 		return_0;
 
 	_update_cache_vginfo_lock_state(vginfo, vgname_is_locked(vgname));
@@ -1071,8 +1101,7 @@ struct lvmcache_info *lvmcache_add(struct labeller *labeller, const char *pvid,
 
 static void _lvmcache_destroy_entry(struct lvmcache_info *info)
 {
-	if (!list_empty(&info->list))
+	_vginfo_detach_info(info);
-		list_del(&info->list);
 	strcpy(info->dev->pvid, "");
 	label_destroy(info->label);
 	dm_free(info);
@@ -1084,12 +1113,8 @@ static void _lvmcache_destroy_vgnamelist(struct lvmcache_vginfo *vginfo)
 
 	do {
 		next = vginfo->next;
-		if (vginfo->vgname)
+		if (!_free_vginfo(vginfo))
-			dm_free(vginfo->vgname);
+			stack;
-		if (vginfo->creation_host)
-			dm_free(vginfo->creation_host);
-		_free_cached_vgmetadata(vginfo);
-		dm_free(vginfo);
 	} while ((vginfo = next));
 }
 
@@ -1141,6 +1166,8 @@ void lvmcache_destroy(struct cmd_context *cmd, int retain_orphans)
 		_lock_hash = NULL;
 	}
 
+	if (!list_empty(&_vginfos))
+		log_error("Internal error: _vginfos list should be empty");
 	list_init(&_vginfos);
 
 	if (retain_orphans)

make.tmpl.in

@@ -26,6 +26,7 @@ LN_S = @LN_S@
 LIBS = @LIBS@
 DEFS += @DEFS@
 CFLAGS += @CFLAGS@
+CPPFLAGS += @CPPFLAGS@
 CLDFLAGS += @CLDFLAGS@
 LDDEPS += @LDDEPS@
 LDFLAGS += @LDFLAGS@
@@ -94,14 +95,14 @@ STRIP=
 LIB_VERSION := $(shell cat $(top_srcdir)/VERSION | \
 		 awk -F '.' '{printf "%s.%s",$$1,$$2}')
 
-INCLUDES += -I$(top_srcdir)/include
+CPPFLAGS := -I$(top_srcdir)/include $(CPPFLAGS)
 
 ifneq ("@DMDIR@", "")
-  INCLUDES += -I@DMDIR@/include
+  CPPFLAGS += -I@DMDIR@/include
 endif
 
 ifdef DESTDIR
-  INCLUDES += -I$(DESTDIR)/usr/include
+  CPPFLAGS += -I$(DESTDIR)/usr/include
 endif
 
 INC_LNS = $(top_srcdir)/include/.symlinks_created
@@ -161,14 +162,14 @@ endif
 $(TARGETS): $(OBJECTS)
 
 %.o: %.c
-	$(CC) -c $(INCLUDES) $(DEFS) $(CFLAGS) $< -o $@
+	$(CC) -c $(CPPFLAGS) $(DEFS) $(CFLAGS) $< -o $@
 
 %.pot: %.c Makefile
-	$(CC) -E $(INCLUDES) -include $(top_srcdir)/include/pogen.h \
+	$(CC) -E $(CPPFLAGS) -include $(top_srcdir)/include/pogen.h \
 		$(DEFS) $(CFLAGS) $< > $@
 
 %.so: %.o
-	$(CC) -c $(INCLUDES) $(DEFS) $(CFLAGS) $< -o $@
+	$(CC) -c $(CPPFLAGS) $(DEFS) $(CFLAGS) $< -o $@
 
 ifeq ("@LIB_SUFFIX@","so")
 $(LIB_SHARED): $(OBJECTS) $(LDDEPS)
@@ -195,7 +196,7 @@ $(LIB_STATIC): $(OBJECTS)
 	set -e; \
 	FILE=`echo $@ | sed 's/\\//\\\\\\//g;s/\\.d//g'`; \
 	DEPS=`echo $(DEPS) | sed -e 's/\\//\\\\\\//g'`; \
-	$(CC) -MM $(INCLUDES) $(DEFS) $(CFLAGS) $< | \
+	$(CC) -MM $(CPPFLAGS) $(DEFS) $(CFLAGS) $< | \
 	sed "s/\(.*\)\.o[ :]*/$$FILE.o $$FILE.d $$FILE.pot: $$DEPS /g" > $@; \
 	[ -s $@ ] || $(RM) $@
 

tools/Makefile.in

@@ -98,12 +98,12 @@ DEFS += -DLVM_SHARED_PATH=\"$(exec_prefix)/sbin/lvm\"
 include $(top_srcdir)/make.tmpl
 
 lvm: $(OBJECTS) lvm.o $(top_srcdir)/lib/liblvm.a
-	$(CC) -o $@ $(CFLAGS) $(OBJECTS) lvm.o \
+	$(CC) -o $@ $(CPPFLAGS) $(CFLAGS) $(OBJECTS) lvm.o \
 		$(LDFLAGS) $(LVMLIBS) $(LIBS) -rdynamic
 
 LIB_PTHREAD = @LIB_PTHREAD@
 lvm.static: $(OBJECTS) lvm-static.o $(top_srcdir)/lib/liblvm.a
-	$(CC) -o $@ $(CFLAGS) $(OBJECTS) lvm-static.o -static \
+	$(CC) -o $@ $(CPPFLAGS) $(CFLAGS) $(OBJECTS) lvm-static.o -static \
 		$(LDFLAGS) $(LVMLIBS) $(LIBS) $(LIB_PTHREAD) -rdynamic
 
 liblvm2cmd.a: $(top_srcdir)/lib/liblvm.a $(OBJECTS) lvmcmdlib.o lvm2cmd.o
@@ -128,15 +128,15 @@ CFLOW_FLAGS += --cpp="$(CC) -E"
 .PHONY: lvm.cflow
 -include $(top_srcdir)/lib/liblvm.cflow
 lvm.cflow: $(SOURCES)
-	@CFLOW_CMD@ -o$@ $(CFLOW_FLAGS) $(INCLUDES) $(DEFS) $(SOURCES)
+	@CFLOW_CMD@ -o$@ $(CFLOW_FLAGS) $(CPPFLAGS) $(DEFS) $(SOURCES)
 lvm.rxref: $(SOURCES)
-	@CFLOW_CMD@ -o$@ --cpp="$(CC) -E" -r --omit-arguments -x $(INCLUDES) $(DEFS) $(SOURCES)
+	@CFLOW_CMD@ -o$@ --cpp="$(CC) -E" -r --omit-arguments -x $(CPPFLAGS) $(DEFS) $(SOURCES)
 lvm.xref: $(SOURCES)
-	@CFLOW_CMD@ -o$@ --cpp="$(CC) -E" --omit-arguments -x $(INCLUDES) $(DEFS) $(SOURCES)
+	@CFLOW_CMD@ -o$@ --cpp="$(CC) -E" --omit-arguments -x $(CPPFLAGS) $(DEFS) $(SOURCES)
 lvm.rtree: $(SOURCES)
-	@CFLOW_CMD@ -o$@ --cpp="$(CC) -E" -r --omit-arguments -T -b $(INCLUDES) $(DEFS) $(SOURCES)
+	@CFLOW_CMD@ -o$@ --cpp="$(CC) -E" -r --omit-arguments -T -b $(CPPFLAGS) $(DEFS) $(SOURCES)
 lvm.tree: $(SOURCES)
-	@CFLOW_CMD@ -o$@ --cpp="$(CC) -E" --omit-arguments -T -b $(INCLUDES) $(DEFS) $(SOURCES)
+	@CFLOW_CMD@ -o$@ --cpp="$(CC) -E" --omit-arguments -T -b $(CPPFLAGS) $(DEFS) $(SOURCES)
 cflow: lvm.cflow lvm.xref lvm.tree lvm.rtree lvm.rxref
 endif