public inbox for lvm2-cvs@sourceware.org
help / color / mirror / Atom feed
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2011-07-20 18:24 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2011-07-20 18:24 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2011-07-20 18:24:49

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	Free up allocated memory before exiting, in lvmetad.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.12&r2=1.13

--- LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 16:49:21	1.12
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 18:24:49	1.13
@@ -274,6 +274,13 @@
 static int fini(daemon_state *s)
 {
 	lvmetad_state *ls = s->private;
+	struct dm_hash_node *n = dm_hash_get_first(ls->vgs);
+	while (n) {
+		destroy_config_tree(dm_hash_get_data(ls->vgs, n));
+		n = dm_hash_get_next(ls->vgs, n);
+	}
+	dm_hash_destroy(ls->pvs);
+	dm_hash_destroy(ls->vgs);
 	return 1;
 }
 


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2012-03-23 10:34 zkabelac
  0 siblings, 0 replies; 31+ messages in thread
From: zkabelac @ 2012-03-23 10:34 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac@sourceware.org	2012-03-23 10:34:51

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	Add fixmes
	
	There is missing some proper reaction when update fails ?

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.48&r2=1.49

--- LVM2/daemons/lvmetad/lvmetad-core.c	2012/03/23 10:33:27	1.48
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2012/03/23 10:34:51	1.49
@@ -675,6 +675,7 @@
 	assert(oldname);
 
 	if (update_pvids)
+		/* FIXME: What should happen when update fails */
 		update_pvid_to_vgid(s, old, "#orphan", 0);
 	/* need to update what we have since we found a newer version */
 	dm_hash_remove(s->vgid_to_metadata, vgid);
@@ -804,7 +805,8 @@
 	unlock_vgid_to_metadata(s);
 
 	if (retval)
-		update_pvid_to_vgid(s, cft, vgid, 1);
+		/* FIXME: What should happen when update fails */
+		retval = update_pvid_to_vgid(s, cft, vgid, 1);
 
 	unlock_pvid_to_vgid(s);
 out:


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2012-02-27 10:19 zkabelac
  0 siblings, 0 replies; 31+ messages in thread
From: zkabelac @ 2012-02-27 10:19 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac@sourceware.org	2012-02-27 10:19:00

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	Add assert for oldname
	
	Code cannot proceed if oldname would be NULL.
	Since lvmetad currently doesn't use logging mechanism of lvm to report
	internal errors - stay with current code style of lvmetad which uses
	plain asserts for cases like this.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.43&r2=1.44

--- LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/27 10:10:43	1.43
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/27 10:19:00	1.44
@@ -595,6 +595,7 @@
 
 	if (!old)
 		return 0;
+	assert(oldname);
 
 	if (update_pvids)
 		update_pvid_to_vgid(s, old, "#orphan", 0);


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2012-02-27 10:10 zkabelac
  0 siblings, 0 replies; 31+ messages in thread
From: zkabelac @ 2012-02-27 10:10 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac@sourceware.org	2012-02-27 10:10:43

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	Move allocation after check for vgid
	
	so there is no mem leak on this error path.
	Also actually check if the hash exists.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.42&r2=1.43

--- LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/24 00:24:37	1.42
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/27 10:10:43	1.43
@@ -548,7 +548,7 @@
 			       const char *vgid, int nuke_empty)
 {
 	struct dm_config_node *pv = pvs(vg->root);
-	struct dm_hash_table *to_check = dm_hash_create(32);
+	struct dm_hash_table *to_check;
 	struct dm_hash_node *n;
 	const char *pvid;
 	const char *vgid_old;
@@ -556,6 +556,9 @@
 	if (!vgid)
 		return 0;
 
+	if (!(to_check = dm_hash_create(32)))
+		return 0;
+
 	while (pv) {
 		pvid = dm_config_find_str(pv->child, "id", NULL);
 		vgid_old = dm_hash_lookup(s->pvid_to_vgid, pvid);


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2012-02-24  0:24 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2012-02-24  0:24 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2012-02-24 00:24:37

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	Fix server-side leaks in lvmetad.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.41&r2=1.42

--- LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/24 00:11:59	1.41
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/24 00:24:37	1.42
@@ -365,12 +365,14 @@
 	if (!pvid) {
 		debug("pv_lookup: could not find device %" PRIu64 "\n", devt);
 		unlock_pvid_to_pvmeta(s);
+		dm_config_destroy(res.cft);
 		return daemon_reply_simple("failed", "reason = %s", "device not found", NULL);
 	}
 
 	pv = make_pv_node(s, pvid, res.cft, NULL, res.cft->root);
 	if (!pv) {
 		unlock_pvid_to_pvmeta(s);
+		dm_config_destroy(res.cft);
 		return daemon_reply_simple("failed", "reason = %s", "PV not found", NULL);
 	}
 
@@ -761,7 +763,7 @@
 	const char *vgid = daemon_request_str(r, "metadata/id", NULL);
 	struct dm_config_node *pvmeta = dm_config_find_node(r.cft->root, "pvmeta");
 	uint64_t device;
-	struct dm_config_tree *cft;
+	struct dm_config_tree *cft, *pvmeta_old = NULL;
 	const char *old;
 	const char *pvid_dup;
 	int complete = 0, orphan = 0;
@@ -778,14 +780,18 @@
 
 	lock_pvid_to_pvmeta(s);
 
-	if ((old = dm_hash_lookup_binary(s->device_to_pvid, &device, sizeof(device))))
+	if ((old = dm_hash_lookup_binary(s->device_to_pvid, &device, sizeof(device)))) {
+		pvmeta_old = dm_hash_lookup(s->pvid_to_pvmeta, old);
 		dm_hash_remove(s->pvid_to_pvmeta, old);
+	}
 
 	cft = dm_config_create();
 	cft->root = dm_config_clone_node(cft, pvmeta, 0);
 	pvid_dup = dm_config_find_str(cft->root, "pvmeta/id", NULL);
 	dm_hash_insert(s->pvid_to_pvmeta, pvid, cft);
 	dm_hash_insert_binary(s->device_to_pvid, &device, sizeof(device), (void*)pvid_dup);
+	if (pvmeta_old)
+		dm_config_destroy(pvmeta_old);
 
 	unlock_pvid_to_pvmeta(s);
 


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2012-02-24  0:12 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2012-02-24  0:12 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2012-02-24 00:11:59

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	Clean up the lvmetad state more thoroughly upon shutdown.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.40&r2=1.41

--- LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/23 23:52:11	1.40
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/24 00:11:59	1.41
@@ -941,6 +941,12 @@
 		n = dm_hash_get_next(ls->vgid_to_metadata, n);
 	}
 
+	n = dm_hash_get_first(ls->pvid_to_pvmeta);
+	while (n) {
+		dm_config_destroy(dm_hash_get_data(ls->pvid_to_pvmeta, n));
+		n = dm_hash_get_next(ls->pvid_to_pvmeta, n);
+	}
+
 	n = dm_hash_get_first(ls->lock.vg);
 	while (n) {
 		pthread_mutex_destroy(dm_hash_get_data(ls->lock.vg, n));
@@ -952,6 +958,8 @@
 	dm_hash_destroy(ls->pvid_to_pvmeta);
 	dm_hash_destroy(ls->device_to_pvid);
 	dm_hash_destroy(ls->vgid_to_metadata);
+	dm_hash_destroy(ls->vgid_to_vgname);
+	dm_hash_destroy(ls->vgname_to_vgid);
 	dm_hash_destroy(ls->pvid_to_vgid);
 	return 1;
 }


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2012-02-21  9:19 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2012-02-21  9:19 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2012-02-21 09:19:08

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	Tweak lvmetad a bit more:
	- allow at most one PV on any given device
	- allow PV lookup by device
	- merge the pvmeta info into VG metadata when responding to vg_lookup

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.37&r2=1.38

--- LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/15 17:37:09	1.37
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/21 09:19:08	1.38
@@ -224,6 +224,25 @@
 	vg->sib = NULL; /* Drop any trailing garbage. */
 }
 
+static void merge_pvmeta(struct dm_config_node *pv, struct dm_config_node *pvmeta)
+{
+	if (!pvmeta)
+		return;
+
+	struct dm_config_node *tmp = pvmeta;
+	while (tmp->sib) {
+		/* drop the redundant ID and dev_size nodes */
+		if (!strcmp(tmp->sib->key, "id") || !strcmp(tmp->sib->key, "dev_size"))
+			tmp->sib = tmp->sib->sib;
+		if (!tmp->sib) break;
+		tmp = tmp->sib;
+		tmp->parent = pv;
+	}
+	tmp->sib = pv->child;
+	pv->child = pvmeta;
+	pvmeta->parent = pv;
+}
+
 /* Either the "big" vgs lock, or a per-vg lock needs to be held before entering
  * this function. */
 static int update_pv_status(lvmetad_state *s,
@@ -241,10 +260,9 @@
 		if (act) {
 			set_flag(cft, pv, "status", "MISSING", !pvmeta);
 			if (pvmeta) {
-				// debug_cft("PV META", pvmeta->root);
-				make_int_node(cft, "device",
-					      dm_config_find_int(pvmeta->root, "pvmeta/device", 0),
-					      pv, NULL);
+				struct dm_config_node *pvmeta_cn =
+					dm_config_clone_node(cft, pvmeta->root->child, 1);
+				merge_pvmeta(pv, pvmeta_cn);
 			}
 		}
 		if (!pvmeta) {
@@ -285,6 +303,7 @@
 	if (parent && !parent->child)
 		parent->child = pv;
 	pv->parent = parent;
+	pv->key = pvid;
 
 	/* Add the "variable" bits to it. */
 	struct dm_config_node *cn = NULL;
@@ -318,16 +337,15 @@
 
 	unlock_pvid_to_pvmeta(s);
 
-	// debug_cft("PV LIST", res.cft->root);
-
 	return res;
 }
 
 static response pv_lookup(lvmetad_state *s, request r)
 {
 	const char *pvid = daemon_request_str(r, "uuid", NULL);
-	if (!pvid)
-		return daemon_reply_simple("failed", "reason = %s", "need PVID", NULL);
+	int64_t devt = daemon_request_int(r, "device", 0);
+	if (!pvid && !devt)
+		return daemon_reply_simple("failed", "reason = %s", "need PVID or device", NULL);
 
 	response res = { .buffer = NULL };
 	res.cft = dm_config_create();
@@ -336,6 +354,15 @@
 	struct dm_config_node *pv;
 
 	lock_pvid_to_pvmeta(s);
+	if (!pvid && devt)
+		pvid = dm_hash_lookup_binary(s->device_to_pvid, &devt, sizeof(devt));
+
+	if (!pvid) {
+		debug("pv_lookup: could not find device %lld\n", devt);
+		unlock_pvid_to_pvmeta(s);
+		return daemon_reply_simple("failed", "reason = %s", "device not found", NULL);
+	}
+
 	pv = make_pv_node(s, pvid, res.cft, NULL, res.cft->root);
 	if (!pv) {
 		unlock_pvid_to_pvmeta(s);
@@ -345,8 +372,6 @@
 	pv->key = "physical_volume";
 	unlock_pvid_to_pvmeta(s);
 
-	// debug_cft("PV LOOKUP", res.cft->root);
-
 	return res;
 }
 
@@ -455,8 +480,6 @@
 
 	update_pv_status(s, res.cft, n, 1); /* FIXME report errors */
 
-	// debug_cft("METADATA", n);
-
 	return res;
 }
 
@@ -673,7 +696,6 @@
 		remove_metadata(s, vgid, 1);
 	}
 
-	// debug_cft("METADATA", metadata);
 	lock_vgid_to_metadata(s);
 	dm_hash_insert(s->vgid_to_metadata, vgid, cft);
 	debug("Mapping %s to %s\n", vgid, name);
@@ -697,7 +719,6 @@
 	int64_t device = daemon_request_int(r, "device", 0);
 
 	debug("pv_gone: %s / %lld\n", pvid, device);
-	debug_cft("PV_GONE", r.cft->root);
 
 	lock_pvid_to_pvmeta(s);
 	if (!pvid && device > 0)
@@ -733,8 +754,6 @@
 
 	int complete = 0, orphan = 0;
 
-	// debug_cft("INCOMING PV", r.cft->root);
-
 	if (!pvid)
 		return daemon_reply_simple("failed", "reason = %s", "need PV UUID", NULL);
 	if (!pvmeta)
@@ -747,6 +766,10 @@
 
 	lock_pvid_to_pvmeta(s);
 	{
+		const char *old = dm_hash_lookup_binary(s->device_to_pvid, &device, sizeof(device));
+		if (old)
+			dm_hash_remove(s->pvid_to_pvmeta, old);
+
 		struct dm_config_tree *cft = dm_config_create();
 		cft->root = dm_config_clone_node(cft, pvmeta, 0);
 		const char *pvid_dup = dm_config_find_str(cft->root, "pvmeta/id", NULL);
@@ -758,7 +781,7 @@
 	if (metadata) {
 		if (!vgid)
 			return daemon_reply_simple("failed", "reason = %s", "need VG UUID", NULL);
-		debug("obtained vgid = %s, vgname = %s", vgid, vgname);
+		debug("obtained vgid = %s, vgname = %s\n", vgid, vgname);
 		if (!vgname)
 			return daemon_reply_simple("failed", "reason = %s", "need VG name", NULL);
 		if (daemon_request_int(r, "metadata/seqno", -1) < 0)


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2012-02-15 17:37 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2012-02-15 17:37 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2012-02-15 17:37:10

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	Drop the now-redundant pvid_to_status hash.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.36&r2=1.37

--- LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/15 17:30:07	1.36
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/15 17:37:09	1.37
@@ -12,9 +12,8 @@
 #include "daemon-server.h"
 
 typedef struct {
-	struct dm_hash_table *pvid_to_status;
-	struct dm_hash_table *pvid_to_pvmeta; /* shares locks with status */
-	struct dm_hash_table *device_to_pvid; /* shares locks with status */
+	struct dm_hash_table *pvid_to_pvmeta;
+	struct dm_hash_table *device_to_pvid; /* shares locks with above */
 
 	struct dm_hash_table *vgid_to_metadata;
 	struct dm_hash_table *vgid_to_vgname;
@@ -22,7 +21,7 @@
 	struct dm_hash_table *pvid_to_vgid;
 	struct {
 		struct dm_hash_table *vg;
-		pthread_mutex_t pvid_to_status;
+		pthread_mutex_t pvid_to_pvmeta;
 		pthread_mutex_t vgid_to_metadata;
 		pthread_mutex_t pvid_to_vgid;
 	} lock;
@@ -47,10 +46,10 @@
 	dm_config_write_node(n, &debug_cft_line, NULL);
 }
 
-static void lock_pvid_to_status(lvmetad_state *s) {
-	pthread_mutex_lock(&s->lock.pvid_to_status); }
-static void unlock_pvid_to_status(lvmetad_state *s) {
-	pthread_mutex_unlock(&s->lock.pvid_to_status); }
+static void lock_pvid_to_pvmeta(lvmetad_state *s) {
+	pthread_mutex_lock(&s->lock.pvid_to_pvmeta); }
+static void unlock_pvid_to_pvmeta(lvmetad_state *s) {
+	pthread_mutex_unlock(&s->lock.pvid_to_pvmeta); }
 
 static void lock_vgid_to_metadata(lvmetad_state *s) {
 	pthread_mutex_lock(&s->lock.vgid_to_metadata); }
@@ -234,14 +233,13 @@
 	struct dm_config_node *pv;
 	int complete = 1;
 
-	lock_pvid_to_status(s);
+	lock_pvid_to_pvmeta(s);
 	pv = pvs(vg);
 	while (pv) {
 		const char *uuid = dm_config_find_str(pv->child, "id", NULL);
-		int found = uuid ? (dm_hash_lookup(s->pvid_to_status, uuid) ? 1 : 0) : 0;
+		struct dm_config_tree *pvmeta = dm_hash_lookup(s->pvid_to_pvmeta, uuid);
 		if (act) {
-			set_flag(cft, pv, "status", "MISSING", !found);
-			struct dm_config_tree *pvmeta = dm_hash_lookup(s->pvid_to_pvmeta, uuid);
+			set_flag(cft, pv, "status", "MISSING", !pvmeta);
 			if (pvmeta) {
 				// debug_cft("PV META", pvmeta->root);
 				make_int_node(cft, "device",
@@ -249,16 +247,16 @@
 					      pv, NULL);
 			}
 		}
-		if (!found) {
+		if (!pvmeta) {
 			complete = 0;
 			if (!act) { /* optimisation */
-				unlock_pvid_to_status(s);
+				unlock_pvid_to_pvmeta(s);
 				return complete;
 			}
 		}
 		pv = pv->sib;
 	}
-	unlock_pvid_to_status(s);
+	unlock_pvid_to_pvmeta(s);
 
 	return complete;
 }
@@ -309,7 +307,7 @@
 	res.cft->root = make_text_node(res.cft, "response", "OK", NULL, NULL);
 	cn_pvs = make_config_node(res.cft, "physical_volumes", NULL, res.cft->root);
 
-	lock_pvid_to_status(s);
+	lock_pvid_to_pvmeta(s);
 
 	struct dm_hash_node *n = dm_hash_get_first(s->pvid_to_pvmeta);
 	while (n) {
@@ -318,7 +316,7 @@
 		n = dm_hash_get_next(s->pvid_to_pvmeta, n);
 	}
 
-	unlock_pvid_to_status(s);
+	unlock_pvid_to_pvmeta(s);
 
 	// debug_cft("PV LIST", res.cft->root);
 
@@ -337,15 +335,15 @@
 
 	struct dm_config_node *pv;
 
-	lock_pvid_to_status(s);
+	lock_pvid_to_pvmeta(s);
 	pv = make_pv_node(s, pvid, res.cft, NULL, res.cft->root);
 	if (!pv) {
-		unlock_pvid_to_status(s);
+		unlock_pvid_to_pvmeta(s);
 		return daemon_reply_simple("failed", "reason = %s", "PV not found", NULL);
 	}
 
 	pv->key = "physical_volume";
-	unlock_pvid_to_status(s);
+	unlock_pvid_to_pvmeta(s);
 
 	// debug_cft("PV LOOKUP", res.cft->root);
 
@@ -584,12 +582,12 @@
 
 	int missing = 1;
 
-	lock_pvid_to_status(s);
+	lock_pvid_to_pvmeta(s);
 
 	while (pv) {
 		const char *pvid = dm_config_find_str(pv->child, "id", NULL);
 		const char *vgid_check = dm_hash_lookup(s->pvid_to_vgid, pvid);
-		if (dm_hash_lookup(s->pvid_to_status, pvid) &&
+		if (dm_hash_lookup(s->pvid_to_pvmeta, pvid) &&
 		    vgid_check && !strcmp(vgid, vgid_check))
 			missing = 0; /* at least one PV is around */
 		pv = pv->sib;
@@ -600,7 +598,7 @@
 		remove_metadata(s, vgid, 0);
 	}
 
-	unlock_pvid_to_status(s);
+	unlock_pvid_to_pvmeta(s);
 
 	return 1;
 }
@@ -701,11 +699,11 @@
 	debug("pv_gone: %s / %lld\n", pvid, device);
 	debug_cft("PV_GONE", r.cft->root);
 
-	lock_pvid_to_status(s);
+	lock_pvid_to_pvmeta(s);
 	if (!pvid && device > 0)
 		pvid = dm_hash_lookup_binary(s->device_to_pvid, &device, sizeof(device));
 	if (!pvid) {
-		unlock_pvid_to_status(s);
+		unlock_pvid_to_pvmeta(s);
 		return daemon_reply_simple("failed", "reason = %s", "device not in cache", NULL);
 	}
 
@@ -713,10 +711,9 @@
 
 	struct dm_config_tree *pvmeta = dm_hash_lookup(s->pvid_to_pvmeta, pvid);
 	dm_hash_remove_binary(s->device_to_pvid, &device, sizeof(device));
-	dm_hash_remove(s->pvid_to_status, pvid);
 	dm_hash_remove(s->pvid_to_pvmeta, pvid);
 	vg_remove_if_missing(s, dm_hash_lookup(s->pvid_to_vgid, pvid));
-	unlock_pvid_to_status(s);
+	unlock_pvid_to_pvmeta(s);
 
 	if (pvmeta) {
 		dm_config_destroy(pvmeta);
@@ -748,9 +745,7 @@
 
 	debug("pv_found %s, vgid = %s, device = %lld\n", pvid, vgid, device);
 
-	lock_pvid_to_status(s);
-	dm_hash_insert(s->pvid_to_status, pvid, (void*)1);
-
+	lock_pvid_to_pvmeta(s);
 	{
 		struct dm_config_tree *cft = dm_config_create();
 		cft->root = dm_config_clone_node(cft, pvmeta, 0);
@@ -758,8 +753,7 @@
 		dm_hash_insert(s->pvid_to_pvmeta, pvid, cft);
 		dm_hash_insert_binary(s->device_to_pvid, &device, sizeof(device), (void*)pvid_dup);
 	}
-
-	unlock_pvid_to_status(s);
+	unlock_pvid_to_pvmeta(s);
 
 	if (metadata) {
 		if (!vgid)
@@ -880,7 +874,6 @@
 	pthread_mutexattr_t rec;
 	lvmetad_state *ls = s->private;
 
-	ls->pvid_to_status = dm_hash_create(32);
 	ls->pvid_to_pvmeta = dm_hash_create(32);
 	ls->device_to_pvid = dm_hash_create(32);
 	ls->vgid_to_metadata = dm_hash_create(32);
@@ -890,7 +883,7 @@
 	ls->lock.vg = dm_hash_create(32);
 	pthread_mutexattr_init(&rec);
 	pthread_mutexattr_settype(&rec, PTHREAD_MUTEX_RECURSIVE_NP);
-	pthread_mutex_init(&ls->lock.pvid_to_status, &rec);
+	pthread_mutex_init(&ls->lock.pvid_to_pvmeta, &rec);
 	pthread_mutex_init(&ls->lock.vgid_to_metadata, &rec);
 	pthread_mutex_init(&ls->lock.pvid_to_vgid, NULL);
 
@@ -923,7 +916,6 @@
 	}
 
 	dm_hash_destroy(ls->lock.vg);
-	dm_hash_destroy(ls->pvid_to_status);
 	dm_hash_destroy(ls->pvid_to_pvmeta);
 	dm_hash_destroy(ls->device_to_pvid);
 	dm_hash_destroy(ls->vgid_to_metadata);


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2012-02-15 17:30 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2012-02-15 17:30 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2012-02-15 17:30:07

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	Update lvmetad: use device major/minor pair to track devices. Keep a pvmeta
	config tree per PV which is mostly provided by the client, so it can be used to
	keep track of things like label_sector, PV format, mda count / offsets and so
	on.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.35&r2=1.36

--- LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/15 14:15:50	1.35
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/15 17:30:07	1.36
@@ -12,14 +12,10 @@
 #include "daemon-server.h"
 
 typedef struct {
-	const char *path;
-	dev_t number;
-} device; /* TODO */
-
-typedef struct {
 	struct dm_hash_table *pvid_to_status;
-	struct dm_hash_table *pvid_to_device; /* shares locks with status */
+	struct dm_hash_table *pvid_to_pvmeta; /* shares locks with status */
 	struct dm_hash_table *device_to_pvid; /* shares locks with status */
+
 	struct dm_hash_table *vgid_to_metadata;
 	struct dm_hash_table *vgid_to_vgname;
 	struct dm_hash_table *vgname_to_vgid;
@@ -164,6 +160,56 @@
 	return 1;
 }
 
+static struct dm_config_node *make_config_node(struct dm_config_tree *cft,
+					       const char *key,
+					       struct dm_config_node *parent,
+					       struct dm_config_node *pre_sib)
+{
+	struct dm_config_node *cn = dm_config_create_node(cft, key);
+	cn->parent = parent;
+	cn->sib = NULL;
+	cn->v = NULL;
+	cn->child = NULL;
+
+	if (parent && parent->child && !pre_sib) { /* find the last one */
+		pre_sib = parent->child;
+		while (pre_sib && pre_sib->sib) pre_sib = pre_sib->sib;
+	}
+
+	if (parent && !parent->child)
+		parent->child = cn;
+	if (pre_sib)
+		pre_sib->sib = cn;
+
+	return cn;
+}
+
+static struct dm_config_node *make_text_node(struct dm_config_tree *cft,
+					     const char *key,
+					     const char *value,
+					     struct dm_config_node *parent,
+					     struct dm_config_node *pre_sib)
+{
+	struct dm_config_node *cn = make_config_node(cft, key, parent, pre_sib);
+	cn->v = dm_config_create_value(cft);
+	cn->v->type = DM_CFG_STRING;
+	cn->v->v.str = value;
+	return cn;
+}
+
+static struct dm_config_node *make_int_node(struct dm_config_tree *cft,
+					    const char *key,
+					    int64_t value,
+					    struct dm_config_node *parent,
+					    struct dm_config_node *pre_sib)
+{
+	struct dm_config_node *cn = make_config_node(cft, key, parent, pre_sib);
+	cn->v = dm_config_create_value(cft);
+	cn->v->type = DM_CFG_INT;
+	cn->v->v.i = value;
+	return cn;
+}
+
 static void filter_metadata(struct dm_config_node *vg) {
 	struct dm_config_node *pv = pvs(vg);
 	while (pv) {
@@ -195,16 +241,12 @@
 		int found = uuid ? (dm_hash_lookup(s->pvid_to_status, uuid) ? 1 : 0) : 0;
 		if (act) {
 			set_flag(cft, pv, "status", "MISSING", !found);
-			const char *devpath = dm_hash_lookup(s->pvid_to_device, uuid);
-			// debug("setting device of %s to %s\n", uuid, devpath);
-			if (devpath) {
-				struct dm_config_node *dev = dm_config_create_node(cft, "device");
-				dev->sib = pv->child;
-				dev->v = dm_config_create_value(cft);
-				dev->v->type = DM_CFG_STRING;
-				dev->v->v.str = dm_hash_lookup(s->pvid_to_device, uuid);
-				dev->parent = pv;
-				pv->child = dev;
+			struct dm_config_tree *pvmeta = dm_hash_lookup(s->pvid_to_pvmeta, uuid);
+			if (pvmeta) {
+				// debug_cft("PV META", pvmeta->root);
+				make_int_node(cft, "device",
+					      dm_config_find_int(pvmeta->root, "pvmeta/device", 0),
+					      pv, NULL);
 			}
 		}
 		if (!found) {
@@ -221,46 +263,16 @@
 	return complete;
 }
 
-static struct dm_config_node *make_config_node(struct dm_config_tree *cft,
-					       const char *key,
-					       struct dm_config_node *parent,
-					       struct dm_config_node *pre_sib)
-{
-	struct dm_config_node *cn = dm_config_create_node(cft, key);
-	cn->parent = parent;
-	cn->sib = NULL;
-	cn->v = NULL;
-	cn->child = NULL;
-
-	if (parent && !parent->child)
-		parent->child = cn;
-	if (pre_sib)
-		pre_sib->sib = cn;
-
-	return cn;
-}
-
-static struct dm_config_node *make_text_node(struct dm_config_tree *cft,
-					     const char *key,
-					     const char *value,
-					     struct dm_config_node *parent,
-					     struct dm_config_node *pre_sib)
-{
-	struct dm_config_node *cn = make_config_node(cft, key, parent, pre_sib);
-	cn->v = dm_config_create_value(cft);
-	cn->v->type = DM_CFG_STRING;
-	cn->v->v.str = value;
-	return cn;
-}
-
 static struct dm_config_node *make_pv_node(lvmetad_state *s, const char *pvid,
 					   struct dm_config_tree *cft,
 					   struct dm_config_node *parent,
 					   struct dm_config_node *pre_sib)
 {
-	const char *path = dm_hash_lookup(s->pvid_to_device, pvid),
-		*vgid = dm_hash_lookup(s->pvid_to_vgid, pvid),
-		*vgname = NULL;
+	struct dm_config_tree *pvmeta = dm_hash_lookup(s->pvid_to_pvmeta, pvid);
+	const char *vgid = dm_hash_lookup(s->pvid_to_vgid, pvid), *vgname = NULL;
+
+	if (!pvmeta)
+		return NULL;
 
 	if (vgid) {
 		lock_vgid_to_metadata(s); // XXX
@@ -268,11 +280,17 @@
 		unlock_vgid_to_metadata(s);
 	}
 
-	struct dm_config_node *pv = make_config_node(cft, pvid, parent, pre_sib), *cn = NULL;
+	/* Nick the pvmeta config tree. */
+	struct dm_config_node *pv = dm_config_clone_node(cft, pvmeta->root, 0);
+	if (pre_sib)
+		pre_sib->sib = pv;
+	if (parent && !parent->child)
+		parent->child = pv;
+	pv->parent = parent;
+
+	/* Add the "variable" bits to it. */
+	struct dm_config_node *cn = NULL;
 
-	cn = make_text_node(cft, "id", pvid, pv, cn);
-	if (path)
-		cn = make_text_node(cft, "path", path, pv, cn);
 	if (vgid && strcmp(vgid, "#orphan"))
 		cn = make_text_node(cft, "vgid", vgid, pv, cn);
 	if (vgname)
@@ -293,16 +311,16 @@
 
 	lock_pvid_to_status(s);
 
-	struct dm_hash_node *n = dm_hash_get_first(s->pvid_to_device);
+	struct dm_hash_node *n = dm_hash_get_first(s->pvid_to_pvmeta);
 	while (n) {
-		const char *id = dm_hash_get_key(s->pvid_to_device, n);
+		const char *id = dm_hash_get_key(s->pvid_to_pvmeta, n);
 		cn = make_pv_node(s, id, res.cft, cn_pvs, cn);
-		n = dm_hash_get_next(s->pvid_to_device, n);
+		n = dm_hash_get_next(s->pvid_to_pvmeta, n);
 	}
 
 	unlock_pvid_to_status(s);
 
-	debug_cft("PV LIST", res.cft->root);
+	// debug_cft("PV LIST", res.cft->root);
 
 	return res;
 }
@@ -321,6 +339,11 @@
 
 	lock_pvid_to_status(s);
 	pv = make_pv_node(s, pvid, res.cft, NULL, res.cft->root);
+	if (!pv) {
+		unlock_pvid_to_status(s);
+		return daemon_reply_simple("failed", "reason = %s", "PV not found", NULL);
+	}
+
 	pv->key = "physical_volume";
 	unlock_pvid_to_status(s);
 
@@ -672,54 +695,70 @@
 static response pv_gone(lvmetad_state *s, request r)
 {
 	int found = 0;
-	const char *pvid = daemon_request_str(r, "uuid", NULL),
-		    *dev = daemon_request_str(r, "device", NULL);
+	const char *pvid = daemon_request_str(r, "uuid", NULL);
+	int64_t device = daemon_request_int(r, "device", 0);
 
-	debug("pv_gone: %s / %s\n", pvid, dev);
+	debug("pv_gone: %s / %lld\n", pvid, device);
+	debug_cft("PV_GONE", r.cft->root);
 
 	lock_pvid_to_status(s);
-	if (!pvid && dev)
-		pvid = dm_hash_lookup(s->device_to_pvid, dev);
+	if (!pvid && device > 0)
+		pvid = dm_hash_lookup_binary(s->device_to_pvid, &device, sizeof(device));
 	if (!pvid) {
 		unlock_pvid_to_status(s);
 		return daemon_reply_simple("failed", "reason = %s", "device not in cache", NULL);
 	}
 
-	debug("pv_gone (updated): %s / %s\n", pvid, dev);
+	debug("pv_gone (updated): %s / %lld\n", pvid, device);
 
-	found = dm_hash_lookup(s->pvid_to_status, pvid) ? 1 : 0;
+	struct dm_config_tree *pvmeta = dm_hash_lookup(s->pvid_to_pvmeta, pvid);
+	dm_hash_remove_binary(s->device_to_pvid, &device, sizeof(device));
 	dm_hash_remove(s->pvid_to_status, pvid);
-	dm_hash_remove(s->pvid_to_device, pvid);
-
+	dm_hash_remove(s->pvid_to_pvmeta, pvid);
 	vg_remove_if_missing(s, dm_hash_lookup(s->pvid_to_vgid, pvid));
 	unlock_pvid_to_status(s);
 
-	if (found)
+	if (pvmeta) {
+		dm_config_destroy(pvmeta);
 		return daemon_reply_simple("OK", NULL);
-	else
+	} else
 		return daemon_reply_simple("failed", "reason = %s", "PVID does not exist", NULL);
 }
 
 static response pv_found(lvmetad_state *s, request r)
 {
 	struct dm_config_node *metadata = dm_config_find_node(r.cft->root, "metadata");
-	const char *pvid = daemon_request_str(r, "uuid", NULL);
+	const char *pvid = daemon_request_str(r, "pvmeta/id", NULL);
 	const char *vgname = daemon_request_str(r, "vgname", NULL);
-	const char *devpath = dm_strdup(daemon_request_str(r, "device", NULL));
 	const char *vgid = daemon_request_str(r, "metadata/id", NULL);
+	struct dm_config_node *pvmeta = dm_config_find_node(r.cft->root, "pvmeta");
+	uint64_t device;
+
 	int complete = 0, orphan = 0;
 
+	// debug_cft("INCOMING PV", r.cft->root);
+
 	if (!pvid)
 		return daemon_reply_simple("failed", "reason = %s", "need PV UUID", NULL);
-	if (!devpath)
-		return daemon_reply_simple("failed", "reason = %s", "need PV device", NULL);
+	if (!pvmeta)
+		return daemon_reply_simple("failed", "reason = %s", "need PV metadata", NULL);
+
+	if (!dm_config_get_uint64(pvmeta, "pvmeta/device", &device))
+		return daemon_reply_simple("failed", "reason = %s", "need PV device number", NULL);
 
-	debug("pv_found %s, vgid = %s, device = %s\n", pvid, vgid, devpath);
+	debug("pv_found %s, vgid = %s, device = %lld\n", pvid, vgid, device);
 
 	lock_pvid_to_status(s);
 	dm_hash_insert(s->pvid_to_status, pvid, (void*)1);
-	dm_hash_insert(s->pvid_to_device, pvid, (void*)devpath);
-	dm_hash_insert(s->device_to_pvid, devpath, (void*)dm_strdup(pvid));
+
+	{
+		struct dm_config_tree *cft = dm_config_create();
+		cft->root = dm_config_clone_node(cft, pvmeta, 0);
+		const char *pvid_dup = dm_config_find_str(cft->root, "pvmeta/id", NULL);
+		dm_hash_insert(s->pvid_to_pvmeta, pvid, cft);
+		dm_hash_insert_binary(s->device_to_pvid, &device, sizeof(device), (void*)pvid_dup);
+	}
+
 	unlock_pvid_to_status(s);
 
 	if (metadata) {
@@ -842,7 +881,7 @@
 	lvmetad_state *ls = s->private;
 
 	ls->pvid_to_status = dm_hash_create(32);
-	ls->pvid_to_device = dm_hash_create(32);
+	ls->pvid_to_pvmeta = dm_hash_create(32);
 	ls->device_to_pvid = dm_hash_create(32);
 	ls->vgid_to_metadata = dm_hash_create(32);
 	ls->vgid_to_vgname = dm_hash_create(32);
@@ -885,7 +924,7 @@
 
 	dm_hash_destroy(ls->lock.vg);
 	dm_hash_destroy(ls->pvid_to_status);
-	dm_hash_destroy(ls->pvid_to_device);
+	dm_hash_destroy(ls->pvid_to_pvmeta);
 	dm_hash_destroy(ls->device_to_pvid);
 	dm_hash_destroy(ls->vgid_to_metadata);
 	dm_hash_destroy(ls->pvid_to_vgid);


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2012-02-15 14:15 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2012-02-15 14:15 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2012-02-15 14:15:51

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	(lvmetad) Remove unused variable.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.34&r2=1.35

--- LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/15 14:06:48	1.34
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/15 14:15:50	1.35
@@ -283,7 +283,7 @@
 
 static response pv_list(lvmetad_state *s, request r)
 {
-	struct dm_config_node *cn = NULL, *cn_pvs, *cn_last = NULL;
+	struct dm_config_node *cn = NULL, *cn_pvs;
 	response res = { .buffer = NULL };
 	res.cft = dm_config_create();
 


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2012-02-15 14:06 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2012-02-15 14:06 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2012-02-15 14:06:48

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	In lvmetad, also nuke VGs when all their PVs are stolen by another VG (vgmerge
	& vgsplit do this).

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.33&r2=1.34

--- LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/15 11:43:06	1.33
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/15 14:06:48	1.34
@@ -487,26 +487,45 @@
 	return result;
 }
 
+static int vg_remove_if_missing(lvmetad_state *s, const char *vgid);
+
 /* You need to be holding the pvid_to_vgid lock already to call this. */
-static int update_pvid_to_vgid(lvmetad_state *s, struct dm_config_tree *vg, const char *vgid)
+static int update_pvid_to_vgid(lvmetad_state *s, struct dm_config_tree *vg,
+			       const char *vgid, int nuke_empty)
 {
 	struct dm_config_node *pv = pvs(vg->root);
+	struct dm_hash_table *to_check = dm_hash_create(32);
 
 	if (!vgid)
 		return 0;
 
 	while (pv) {
 		const char *pvid = dm_config_find_str(pv->child, "id", NULL);
+		const char *vgid_old = dm_hash_lookup(s->pvid_to_vgid, pvid);
+		if (vgid_old && nuke_empty)
+			dm_hash_insert(to_check, vgid_old, (void*) 1);
 		dm_hash_insert(s->pvid_to_vgid, pvid, (void *) vgid);
 		debug("remap PV %s to VG %s\n", pvid, vgid);
 		pv = pv->sib;
 	}
 
+	struct dm_hash_node *n = dm_hash_get_first(to_check);
+
+	while (n) {
+		const char *check_vgid = dm_hash_get_key(to_check, n);
+		lock_vg(s, check_vgid);
+		vg_remove_if_missing(s, check_vgid);
+		unlock_vg(s, check_vgid);
+		n = dm_hash_get_next(to_check, n);
+	}
+
+	dm_hash_destroy(to_check);
+
 	return 1;
 }
 
-/* A pvid map lock needs to be held. */
-static int remove_metadata(lvmetad_state *s, const char *vgid)
+/* A pvid map lock needs to be held if update_pvids = 1. */
+static int remove_metadata(lvmetad_state *s, const char *vgid, int update_pvids)
 {
 	struct dm_config_tree *old;
 	const char *oldname;
@@ -518,7 +537,8 @@
 	if (!old)
 		return 0;
 
-	update_pvid_to_vgid(s, old, "#orphan");
+	if (update_pvids)
+		update_pvid_to_vgid(s, old, "#orphan", 0);
 	/* need to update what we have since we found a newer version */
 	dm_hash_remove(s->vgid_to_metadata, vgid);
 	dm_hash_remove(s->vgid_to_vgname, vgid);
@@ -554,7 +574,7 @@
 
 	if (missing) {
 		debug("nuking VG %s\n", vgid);
-		remove_metadata(s, vgid);
+		remove_metadata(s, vgid, 0);
 	}
 
 	unlock_pvid_to_status(s);
@@ -629,7 +649,7 @@
 	if (haveseq >= 0 && haveseq < seq) {
 		debug("Updating metadata for %s at %d to %d\n", _vgid, haveseq, seq);
 		/* temporarily orphan all of our PVs */
-		remove_metadata(s, vgid);
+		remove_metadata(s, vgid, 1);
 	}
 
 	// debug_cft("METADATA", metadata);
@@ -640,7 +660,7 @@
 	dm_hash_insert(s->vgname_to_vgid, name, (void *)vgid);
 	unlock_vgid_to_metadata(s);
 
-	update_pvid_to_vgid(s, cft, vgid);
+	update_pvid_to_vgid(s, cft, vgid, 1);
 
 	unlock_pvid_to_vgid(s);
 	retval = 1;
@@ -773,7 +793,7 @@
 	fprintf(stderr, "vg_remove: %s\n", vgid);
 
 	lock_pvid_to_vgid(s);
-	remove_metadata(s, vgid);
+	remove_metadata(s, vgid, 1);
 	unlock_pvid_to_vgid(s);
 
 	return daemon_reply_simple("OK", NULL);


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2012-02-15 11:43 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2012-02-15 11:43 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2012-02-15 11:43:07

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	lvmetad server-side update:
	- rename the hashes to be explicit about the mapping
	- add VG/PV listing calls to the protocol
	- cache slightly more of the per-PV state
	- filter cached metadata
	- compare the metadata upon metadata_update

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.32&r2=1.33

--- LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/13 14:25:14	1.32
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/15 11:43:06	1.33
@@ -7,20 +7,28 @@
 #include <stdint.h>
 #include <unistd.h>
 
+#include "configure.h"
 #include "libdevmapper.h"
 #include "daemon-server.h"
 
 typedef struct {
-	struct dm_hash_table *pvs;
-	struct dm_hash_table *vgs;
-	struct dm_hash_table *vg_names;
-	struct dm_hash_table *vgname_map;
-	struct dm_hash_table *pvid_map;
+	const char *path;
+	dev_t number;
+} device; /* TODO */
+
+typedef struct {
+	struct dm_hash_table *pvid_to_status;
+	struct dm_hash_table *pvid_to_device; /* shares locks with status */
+	struct dm_hash_table *device_to_pvid; /* shares locks with status */
+	struct dm_hash_table *vgid_to_metadata;
+	struct dm_hash_table *vgid_to_vgname;
+	struct dm_hash_table *vgname_to_vgid;
+	struct dm_hash_table *pvid_to_vgid;
 	struct {
 		struct dm_hash_table *vg;
-		pthread_mutex_t pvs;
-		pthread_mutex_t vgs;
-		pthread_mutex_t pvid_map;
+		pthread_mutex_t pvid_to_status;
+		pthread_mutex_t vgid_to_metadata;
+		pthread_mutex_t pvid_to_vgid;
 	} lock;
 } lvmetad_state;
 
@@ -31,16 +39,32 @@
 	fprintf(stderr, "[D %lu] ", pthread_self());
 	vfprintf(stderr, fmt, ap);
 	va_end(ap);
-};
-
-static void lock_pvs(lvmetad_state *s) { pthread_mutex_lock(&s->lock.pvs); }
-static void unlock_pvs(lvmetad_state *s) { pthread_mutex_unlock(&s->lock.pvs); }
+}
 
-static void lock_vgs(lvmetad_state *s) { pthread_mutex_lock(&s->lock.vgs); }
-static void unlock_vgs(lvmetad_state *s) { pthread_mutex_unlock(&s->lock.vgs); }
+static int debug_cft_line(const char *line, void *baton) {
+	fprintf(stderr, "| %s\n", line);
+	return 0;
+}
 
-static void lock_pvid_map(lvmetad_state *s) { pthread_mutex_lock(&s->lock.pvid_map); }
-static void unlock_pvid_map(lvmetad_state *s) { pthread_mutex_unlock(&s->lock.pvid_map); }
+static void debug_cft(const char *id, struct dm_config_node *n) {
+	debug("%s\n", id);
+	dm_config_write_node(n, &debug_cft_line, NULL);
+}
+
+static void lock_pvid_to_status(lvmetad_state *s) {
+	pthread_mutex_lock(&s->lock.pvid_to_status); }
+static void unlock_pvid_to_status(lvmetad_state *s) {
+	pthread_mutex_unlock(&s->lock.pvid_to_status); }
+
+static void lock_vgid_to_metadata(lvmetad_state *s) {
+	pthread_mutex_lock(&s->lock.vgid_to_metadata); }
+static void unlock_vgid_to_metadata(lvmetad_state *s) {
+	pthread_mutex_unlock(&s->lock.vgid_to_metadata); }
+
+static void lock_pvid_to_vgid(lvmetad_state *s) {
+	pthread_mutex_lock(&s->lock.pvid_to_vgid); }
+static void unlock_pvid_to_vgid(lvmetad_state *s) {
+	pthread_mutex_unlock(&s->lock.pvid_to_vgid); }
 
 /*
  * TODO: It may be beneficial to clean up the vg lock hash from time to time,
@@ -51,7 +75,7 @@
 	pthread_mutex_t *vg;
 	struct dm_config_tree *cft;
 
-	lock_vgs(s);
+	lock_vgid_to_metadata(s);
 	vg = dm_hash_lookup(s->lock.vg, id);
 	if (!vg) {
 		pthread_mutexattr_t rec;
@@ -61,19 +85,19 @@
 		pthread_mutex_init(vg, &rec);
 		dm_hash_insert(s->lock.vg, id, vg);
 	}
-	debug("lock VG %s\n", id);
+	// debug("lock VG %s\n", id);
 	pthread_mutex_lock(vg);
-	cft = dm_hash_lookup(s->vgs, id);
-	unlock_vgs(s);
+	cft = dm_hash_lookup(s->vgid_to_metadata, id);
+	unlock_vgid_to_metadata(s);
 	return cft;
 }
 
 static void unlock_vg(lvmetad_state *s, const char *id) {
-	debug("unlock VG %s\n", id);
-	lock_vgs(s); /* someone might be changing the s->lock.vg structure right
-		      * now, so avoid stepping on each other's toes */
+	// debug("unlock VG %s\n", id);
+	lock_vgid_to_metadata(s); /* someone might be changing the s->lock.vg structure right
+				   * now, so avoid stepping on each other's toes */
 	pthread_mutex_unlock(dm_hash_lookup(s->lock.vg, id));
-	unlock_vgs(s);
+	unlock_vgid_to_metadata(s);
 }
 
 static struct dm_config_node *pvs(struct dm_config_node *vg)
@@ -140,6 +164,21 @@
 	return 1;
 }
 
+static void filter_metadata(struct dm_config_node *vg) {
+	struct dm_config_node *pv = pvs(vg);
+	while (pv) {
+		struct dm_config_node *item = pv->child;
+		while (item) {
+			/* Remove the advisory device nodes. */
+			if (item->sib && !strcmp(item->sib->key, "device"))
+				item->sib = item->sib->sib;
+			item = item->sib;
+		}
+		pv = pv->sib;
+	}
+	vg->sib = NULL; /* Drop any trailing garbage. */
+}
+
 /* Either the "big" vgs lock, or a per-vg lock needs to be held before entering
  * this function. */
 static int update_pv_status(lvmetad_state *s,
@@ -149,30 +188,199 @@
 	struct dm_config_node *pv;
 	int complete = 1;
 
-	lock_pvs(s);
+	lock_pvid_to_status(s);
 	pv = pvs(vg);
 	while (pv) {
 		const char *uuid = dm_config_find_str(pv->child, "id", NULL);
-		int found = uuid ? (dm_hash_lookup(s->pvs, uuid) ? 1 : 0) : 0;
-		if (act &&
-		    !set_flag(cft, pv, "status", "MISSING", !found)) {
-			complete =  0;
-			break;
+		int found = uuid ? (dm_hash_lookup(s->pvid_to_status, uuid) ? 1 : 0) : 0;
+		if (act) {
+			set_flag(cft, pv, "status", "MISSING", !found);
+			const char *devpath = dm_hash_lookup(s->pvid_to_device, uuid);
+			// debug("setting device of %s to %s\n", uuid, devpath);
+			if (devpath) {
+				struct dm_config_node *dev = dm_config_create_node(cft, "device");
+				dev->sib = pv->child;
+				dev->v = dm_config_create_value(cft);
+				dev->v->type = DM_CFG_STRING;
+				dev->v->v.str = dm_hash_lookup(s->pvid_to_device, uuid);
+				dev->parent = pv;
+				pv->child = dev;
+			}
 		}
 		if (!found) {
 			complete = 0;
-			if (!act) { // optimisation
-				unlock_pvs(s);
+			if (!act) { /* optimisation */
+				unlock_pvid_to_status(s);
 				return complete;
 			}
 		}
 		pv = pv->sib;
 	}
-	unlock_pvs(s);
+	unlock_pvid_to_status(s);
 
 	return complete;
 }
 
+static struct dm_config_node *make_config_node(struct dm_config_tree *cft,
+					       const char *key,
+					       struct dm_config_node *parent,
+					       struct dm_config_node *pre_sib)
+{
+	struct dm_config_node *cn = dm_config_create_node(cft, key);
+	cn->parent = parent;
+	cn->sib = NULL;
+	cn->v = NULL;
+	cn->child = NULL;
+
+	if (parent && !parent->child)
+		parent->child = cn;
+	if (pre_sib)
+		pre_sib->sib = cn;
+
+	return cn;
+}
+
+static struct dm_config_node *make_text_node(struct dm_config_tree *cft,
+					     const char *key,
+					     const char *value,
+					     struct dm_config_node *parent,
+					     struct dm_config_node *pre_sib)
+{
+	struct dm_config_node *cn = make_config_node(cft, key, parent, pre_sib);
+	cn->v = dm_config_create_value(cft);
+	cn->v->type = DM_CFG_STRING;
+	cn->v->v.str = value;
+	return cn;
+}
+
+static struct dm_config_node *make_pv_node(lvmetad_state *s, const char *pvid,
+					   struct dm_config_tree *cft,
+					   struct dm_config_node *parent,
+					   struct dm_config_node *pre_sib)
+{
+	const char *path = dm_hash_lookup(s->pvid_to_device, pvid),
+		*vgid = dm_hash_lookup(s->pvid_to_vgid, pvid),
+		*vgname = NULL;
+
+	if (vgid) {
+		lock_vgid_to_metadata(s); // XXX
+		vgname = dm_hash_lookup(s->vgid_to_vgname, vgid);
+		unlock_vgid_to_metadata(s);
+	}
+
+	struct dm_config_node *pv = make_config_node(cft, pvid, parent, pre_sib), *cn = NULL;
+
+	cn = make_text_node(cft, "id", pvid, pv, cn);
+	if (path)
+		cn = make_text_node(cft, "path", path, pv, cn);
+	if (vgid && strcmp(vgid, "#orphan"))
+		cn = make_text_node(cft, "vgid", vgid, pv, cn);
+	if (vgname)
+		cn = make_text_node(cft, "vgname", vgname, pv, cn);
+
+	return pv;
+}
+
+static response pv_list(lvmetad_state *s, request r)
+{
+	struct dm_config_node *cn = NULL, *cn_pvs, *cn_last = NULL;
+	response res = { .buffer = NULL };
+	res.cft = dm_config_create();
+
+	/* The response field */
+	res.cft->root = make_text_node(res.cft, "response", "OK", NULL, NULL);
+	cn_pvs = make_config_node(res.cft, "physical_volumes", NULL, res.cft->root);
+
+	lock_pvid_to_status(s);
+
+	struct dm_hash_node *n = dm_hash_get_first(s->pvid_to_device);
+	while (n) {
+		const char *id = dm_hash_get_key(s->pvid_to_device, n);
+		cn = make_pv_node(s, id, res.cft, cn_pvs, cn);
+		n = dm_hash_get_next(s->pvid_to_device, n);
+	}
+
+	unlock_pvid_to_status(s);
+
+	debug_cft("PV LIST", res.cft->root);
+
+	return res;
+}
+
+static response pv_lookup(lvmetad_state *s, request r)
+{
+	const char *pvid = daemon_request_str(r, "uuid", NULL);
+	if (!pvid)
+		return daemon_reply_simple("failed", "reason = %s", "need PVID", NULL);
+
+	response res = { .buffer = NULL };
+	res.cft = dm_config_create();
+	res.cft->root = make_text_node(res.cft, "response", "OK", NULL, NULL);
+
+	struct dm_config_node *pv;
+
+	lock_pvid_to_status(s);
+	pv = make_pv_node(s, pvid, res.cft, NULL, res.cft->root);
+	pv->key = "physical_volume";
+	unlock_pvid_to_status(s);
+
+	// debug_cft("PV LOOKUP", res.cft->root);
+
+	return res;
+}
+
+static response vg_list(lvmetad_state *s, request r)
+{
+	struct dm_config_node *cn, *cn_vgs, *cn_last = NULL;
+	response res = { .buffer = NULL };
+	res.cft = dm_config_create();
+
+	/* The response field */
+	res.cft->root = cn = dm_config_create_node(res.cft, "response");
+	cn->parent = res.cft->root;
+	cn->v = dm_config_create_value(res.cft);
+	cn->v->type = DM_CFG_STRING;
+	cn->v->v.str = "OK";
+
+	cn_vgs = cn = cn->sib = dm_config_create_node(res.cft, "volume_groups");
+	cn->parent = res.cft->root;
+	cn->v = NULL;
+	cn->child = NULL;
+
+	lock_vgid_to_metadata(s);
+
+	struct dm_hash_node *n = dm_hash_get_first(s->vgid_to_vgname);
+	while (n) {
+		const char *id = dm_hash_get_key(s->vgid_to_vgname, n),
+			 *name = dm_hash_get_data(s->vgid_to_vgname, n);
+
+		cn = dm_config_create_node(res.cft, id);
+		if (cn_last)
+			cn_last->sib = cn;
+
+		cn->parent = cn_vgs;
+		cn->sib = NULL;
+		cn->v = NULL;
+
+		cn->child = dm_config_create_node(res.cft, "name");
+		cn->child->parent = cn;
+		cn->child->sib = 0;
+		cn->child->v = dm_config_create_value(res.cft);
+		cn->child->v->type = DM_CFG_STRING;
+		cn->child->v->v.str = name;
+
+		if (!cn_vgs->child)
+			cn_vgs->child = cn;
+		cn_last = cn;
+
+		n = dm_hash_get_next(s->vgid_to_vgname, n);
+	}
+
+	unlock_vgid_to_metadata(s);
+
+	return res;
+}
+
 static response vg_lookup(lvmetad_state *s, request r)
 {
 	struct dm_config_tree *cft;
@@ -181,15 +389,16 @@
 
 	const char *uuid = daemon_request_str(r, "uuid", NULL),
 		   *name = daemon_request_str(r, "name", NULL);
+
 	debug("vg_lookup: uuid = %s, name = %s\n", uuid, name);
 
 	if (!uuid || !name) {
-		lock_vgs(s);
+		lock_vgid_to_metadata(s);
 		if (name && !uuid)
-			uuid = dm_hash_lookup(s->vgname_map, (void *)name);
+			uuid = dm_hash_lookup(s->vgname_to_vgid, (void *)name);
 		if (uuid && !name)
-			name = dm_hash_lookup(s->vg_names, (void *)uuid);
-		unlock_vgs(s);
+			name = dm_hash_lookup(s->vgid_to_vgname, (void *)uuid);
+		unlock_vgid_to_metadata(s);
 	}
 
 	debug("vg_lookup: updated uuid = %s, name = %s\n", uuid, name);
@@ -223,7 +432,9 @@
 	res.error = 0;
 	unlock_vg(s, uuid);
 
-	update_pv_status(s, cft, n, 1); /* FIXME error reporting */
+	update_pv_status(s, res.cft, n, 1); /* FIXME report errors */
+
+	// debug_cft("METADATA", n);
 
 	return res;
 }
@@ -239,8 +450,8 @@
 
 	switch (a->type) {
 	case DM_CFG_STRING: r = strcmp(a->v.str, b->v.str); break;
-	case DM_CFG_FLOAT: r = (a->v.f == b->v.f); break;
-	case DM_CFG_INT: r = (a->v.i == b->v.i); break;
+	case DM_CFG_FLOAT: r = (a->v.f == b->v.f) ? 0 : (a->v.f > b->v.f) ? 1 : -1; break;
+	case DM_CFG_INT: r = (a->v.i == b->v.i) ? 0 : (a->v.i > b->v.i) ? 1 : -1; break;
 	case DM_CFG_EMPTY_ARRAY: return 0;
 	}
 
@@ -261,8 +472,10 @@
 	if (a->child && b->child)
 		result = compare_config(a->child, b->child);
 
-	if (result)
+	if (result) {
+		debug("config inequality at %s / %s\n", a->key, b->key);
 		return result;
+	}
 
 	if (a->sib && b->sib)
 		result = compare_config(a->sib, b->sib);
@@ -274,8 +487,8 @@
 	return result;
 }
 
-/* You need to be holding the pvid_map lock already to call this. */
-static int update_pvid_map(lvmetad_state *s, struct dm_config_tree *vg, const char *vgid)
+/* You need to be holding the pvid_to_vgid lock already to call this. */
+static int update_pvid_to_vgid(lvmetad_state *s, struct dm_config_tree *vg, const char *vgid)
 {
 	struct dm_config_node *pv = pvs(vg->root);
 
@@ -284,7 +497,8 @@
 
 	while (pv) {
 		const char *pvid = dm_config_find_str(pv->child, "id", NULL);
-		dm_hash_insert(s->pvid_map, pvid, (void *) vgid);
+		dm_hash_insert(s->pvid_to_vgid, pvid, (void *) vgid);
+		debug("remap PV %s to VG %s\n", pvid, vgid);
 		pv = pv->sib;
 	}
 
@@ -296,23 +510,58 @@
 {
 	struct dm_config_tree *old;
 	const char *oldname;
-	lock_vgs(s);
-	old = dm_hash_lookup(s->vgs, vgid);
-	oldname = dm_hash_lookup(s->vg_names, vgid);
-	unlock_vgs(s);
+	lock_vgid_to_metadata(s);
+	old = dm_hash_lookup(s->vgid_to_metadata, vgid);
+	oldname = dm_hash_lookup(s->vgid_to_vgname, vgid);
+	unlock_vgid_to_metadata(s);
 
 	if (!old)
 		return 0;
 
-	update_pvid_map(s, old, "#orphan");
+	update_pvid_to_vgid(s, old, "#orphan");
 	/* need to update what we have since we found a newer version */
-	dm_hash_remove(s->vgs, vgid);
-	dm_hash_remove(s->vg_names, vgid);
-	dm_hash_remove(s->vgname_map, oldname);
+	dm_hash_remove(s->vgid_to_metadata, vgid);
+	dm_hash_remove(s->vgid_to_vgname, vgid);
+	dm_hash_remove(s->vgname_to_vgid, oldname);
 	dm_config_destroy(old);
 	return 1;
 }
 
+/* The VG must be locked. */
+static int vg_remove_if_missing(lvmetad_state *s, const char *vgid)
+{
+	if (!vgid)
+		return 0;
+
+	struct dm_config_tree *vg = dm_hash_lookup(s->vgid_to_metadata, vgid);
+	if (!vg)
+		return 1;
+
+	struct dm_config_node *pv = pvs(vg->root);
+
+	int missing = 1;
+
+	lock_pvid_to_status(s);
+
+	while (pv) {
+		const char *pvid = dm_config_find_str(pv->child, "id", NULL);
+		const char *vgid_check = dm_hash_lookup(s->pvid_to_vgid, pvid);
+		if (dm_hash_lookup(s->pvid_to_status, pvid) &&
+		    vgid_check && !strcmp(vgid, vgid_check))
+			missing = 0; /* at least one PV is around */
+		pv = pv->sib;
+	}
+
+	if (missing) {
+		debug("nuking VG %s\n", vgid);
+		remove_metadata(s, vgid);
+	}
+
+	unlock_pvid_to_status(s);
+
+	return 1;
+}
+
 /* No locks need to be held. The pointers are never used outside of the scope of
  * this function, so they can be safely destroyed after update_metadata returns
  * (anything that might have been retained is copied). */
@@ -327,34 +576,40 @@
 	const char *oldname = NULL;
 	const char *vgid;
 
-	lock_vgs(s);
-	old = dm_hash_lookup(s->vgs, _vgid);
+	lock_vgid_to_metadata(s);
+	old = dm_hash_lookup(s->vgid_to_metadata, _vgid);
 	lock_vg(s, _vgid);
-	unlock_vgs(s);
+	unlock_vgid_to_metadata(s);
 
 	seq = dm_config_find_int(metadata, "metadata/seqno", -1);
 
 	if (old) {
 		haveseq = dm_config_find_int(old->root, "metadata/seqno", -1);
-		oldname = dm_hash_lookup(s->vg_names, _vgid);
+		oldname = dm_hash_lookup(s->vgid_to_vgname, _vgid);
 		assert(oldname);
 	}
 
 	if (seq < 0)
 		goto out;
 
+	filter_metadata(metadata); /* sanitize */
+
 	if (seq == haveseq) {
 		retval = 1;
 		if (compare_config(metadata, old->root))
 			retval = 0;
-		debug("Not updating metadata for %s at %d (equal = %d)\n", _vgid, haveseq, retval);
+		debug("Not updating metadata for %s at %d (%s)\n", _vgid, haveseq,
+		      retval ? "ok" : "MISMATCH");
+		if (!retval) {
+			debug_cft("OLD: ", old->root);
+			debug_cft("NEW: ", metadata);
+		}
 		goto out;
 	}
 
 	if (seq < haveseq) {
 		debug("Refusing to update metadata for %s at %d to %d\n", _vgid, haveseq, seq);
-		// TODO: we may want to notify the client that their metadata is
-		// out of date?
+		/* TODO: notify the client that their metadata is out of date? */
 		retval = 1;
 		goto out;
 	}
@@ -369,7 +624,7 @@
 		goto out;
 	}
 
-	lock_pvid_map(s);
+	lock_pvid_to_vgid(s);
 
 	if (haveseq >= 0 && haveseq < seq) {
 		debug("Updating metadata for %s at %d to %d\n", _vgid, haveseq, seq);
@@ -377,16 +632,17 @@
 		remove_metadata(s, vgid);
 	}
 
-	lock_vgs(s);
-	dm_hash_insert(s->vgs, vgid, cft);
+	// debug_cft("METADATA", metadata);
+	lock_vgid_to_metadata(s);
+	dm_hash_insert(s->vgid_to_metadata, vgid, cft);
 	debug("Mapping %s to %s\n", vgid, name);
-	dm_hash_insert(s->vg_names, vgid, dm_pool_strdup(dm_config_memory(cft), name));
-	dm_hash_insert(s->vgname_map, name, (void *)vgid);
-	unlock_vgs(s);
+	dm_hash_insert(s->vgid_to_vgname, vgid, dm_pool_strdup(dm_config_memory(cft), name));
+	dm_hash_insert(s->vgname_to_vgid, name, (void *)vgid);
+	unlock_vgid_to_metadata(s);
 
-	update_pvid_map(s, cft, vgid);
+	update_pvid_to_vgid(s, cft, vgid);
 
-	unlock_pvid_map(s);
+	unlock_pvid_to_vgid(s);
 	retval = 1;
 out:
 	unlock_vg(s, _vgid);
@@ -396,13 +652,27 @@
 static response pv_gone(lvmetad_state *s, request r)
 {
 	int found = 0;
-	const char *pvid = daemon_request_str(r, "uuid", NULL);
-	debug("pv_gone: %s\n", pvid);
+	const char *pvid = daemon_request_str(r, "uuid", NULL),
+		    *dev = daemon_request_str(r, "device", NULL);
+
+	debug("pv_gone: %s / %s\n", pvid, dev);
+
+	lock_pvid_to_status(s);
+	if (!pvid && dev)
+		pvid = dm_hash_lookup(s->device_to_pvid, dev);
+	if (!pvid) {
+		unlock_pvid_to_status(s);
+		return daemon_reply_simple("failed", "reason = %s", "device not in cache", NULL);
+	}
 
-	lock_pvs(s);
-	found = dm_hash_lookup(s->pvs, pvid) ? 1 : 0;
-	dm_hash_remove(s->pvs, pvid);
-	unlock_pvs(s);
+	debug("pv_gone (updated): %s / %s\n", pvid, dev);
+
+	found = dm_hash_lookup(s->pvid_to_status, pvid) ? 1 : 0;
+	dm_hash_remove(s->pvid_to_status, pvid);
+	dm_hash_remove(s->pvid_to_device, pvid);
+
+	vg_remove_if_missing(s, dm_hash_lookup(s->pvid_to_vgid, pvid));
+	unlock_pvid_to_status(s);
 
 	if (found)
 		return daemon_reply_simple("OK", NULL);
@@ -415,21 +685,27 @@
 	struct dm_config_node *metadata = dm_config_find_node(r.cft->root, "metadata");
 	const char *pvid = daemon_request_str(r, "uuid", NULL);
 	const char *vgname = daemon_request_str(r, "vgname", NULL);
+	const char *devpath = dm_strdup(daemon_request_str(r, "device", NULL));
 	const char *vgid = daemon_request_str(r, "metadata/id", NULL);
-	int complete = 0;
+	int complete = 0, orphan = 0;
 
 	if (!pvid)
 		return daemon_reply_simple("failed", "reason = %s", "need PV UUID", NULL);
+	if (!devpath)
+		return daemon_reply_simple("failed", "reason = %s", "need PV device", NULL);
 
-	debug("pv_found %s, vgid = %s\n", pvid, vgid);
+	debug("pv_found %s, vgid = %s, device = %s\n", pvid, vgid, devpath);
 
-	lock_pvs(s);
-	dm_hash_insert(s->pvs, pvid, (void*)1);
-	unlock_pvs(s);
+	lock_pvid_to_status(s);
+	dm_hash_insert(s->pvid_to_status, pvid, (void*)1);
+	dm_hash_insert(s->pvid_to_device, pvid, (void*)devpath);
+	dm_hash_insert(s->device_to_pvid, devpath, (void*)dm_strdup(pvid));
+	unlock_pvid_to_status(s);
 
 	if (metadata) {
 		if (!vgid)
 			return daemon_reply_simple("failed", "reason = %s", "need VG UUID", NULL);
+		debug("obtained vgid = %s, vgname = %s", vgid, vgname);
 		if (!vgname)
 			return daemon_reply_simple("failed", "reason = %s", "need VG name", NULL);
 		if (daemon_request_int(r, "metadata/seqno", -1) < 0)
@@ -439,23 +715,28 @@
 			return daemon_reply_simple("failed", "reason = %s",
 						   "metadata update failed", NULL);
 	} else {
-		lock_pvid_map(s);
-		vgid = dm_hash_lookup(s->pvid_map, pvid);
-		unlock_pvid_map(s);
+		lock_pvid_to_vgid(s);
+		vgid = dm_hash_lookup(s->pvid_to_vgid, pvid);
+		unlock_pvid_to_vgid(s);
 	}
 
 	if (vgid) {
 		struct dm_config_tree *cft = lock_vg(s, vgid);
-		if (!cft) {
+		if (cft) {
+			complete = update_pv_status(s, cft, cft->root, 0);
+		} else if (!strcmp(vgid, "#orphan")) {
+			orphan = 1;
+		} else {
 			unlock_vg(s, vgid);
-			return daemon_reply_simple("failed", "reason = %s", "vg unknown and no PV metadata", NULL);
+			return daemon_reply_simple("failed", "reason = %s",
+						   "internal treason!", NULL);
 		}
-		complete = update_pv_status(s, cft, cft->root, 0);
 		unlock_vg(s, vgid);
 	}
 
 	return daemon_reply_simple("OK",
-				   "status = %s", complete ? "complete" : "partial",
+				   "status = %s", orphan ? "orphan" :
+				                     (complete ? "complete" : "partial"),
 				   "vgid = %s", vgid ? vgid : "#orphan",
 				   NULL);
 }
@@ -473,6 +754,8 @@
 		if (daemon_request_int(r, "metadata/seqno", -1) < 0)
 			return daemon_reply_simple("failed", "reason = %s", "need VG seqno", NULL);
 
+		/* TODO defer metadata update here; add a separate vg_commit
+		 * call; if client does not commit, die */
 		if (!update_metadata(s, vgname, vgid, metadata))
 			return daemon_reply_simple("failed", "reason = %s",
 						   "metadata update failed", NULL);
@@ -489,9 +772,9 @@
 
 	fprintf(stderr, "vg_remove: %s\n", vgid);
 
-	lock_pvid_map(s);
+	lock_pvid_to_vgid(s);
 	remove_metadata(s, vgid);
-	unlock_pvid_map(s);
+	unlock_pvid_to_vgid(s);
 
 	return daemon_reply_simple("OK", NULL);
 }
@@ -501,13 +784,18 @@
 	lvmetad_state *state = s.private;
 	const char *rq = daemon_request_str(r, "request", "NONE");
 
-	debug("REQUEST: %s\n", rq);
-
+	/*
+	 * TODO Add a stats call, with transaction count/rate, time since last
+	 * update &c.
+	 */
 	if (!strcmp(rq, "pv_found"))
 		return pv_found(state, r);
 
 	if (!strcmp(rq, "pv_gone"))
-		pv_gone(state, r);
+		return pv_gone(state, r);
+
+	if (!strcmp(rq, "pv_lookup"))
+		return pv_lookup(state, r);
 
 	if (!strcmp(rq, "vg_update"))
 		return vg_update(state, r);
@@ -518,6 +806,13 @@
 	if (!strcmp(rq, "vg_lookup"))
 		return vg_lookup(state, r);
 
+	if (!strcmp(rq, "pv_list")) {
+		return pv_list(state, r);
+	}
+
+	if (!strcmp(rq, "vg_list"))
+		return vg_list(state, r);
+
 	return daemon_reply_simple("failed", "reason = %s", "no such request", NULL);
 }
 
@@ -526,20 +821,22 @@
 	pthread_mutexattr_t rec;
 	lvmetad_state *ls = s->private;
 
-	ls->pvs = dm_hash_create(32);
-	ls->vgs = dm_hash_create(32);
-	ls->vg_names = dm_hash_create(32);
-	ls->pvid_map = dm_hash_create(32);
-	ls->vgname_map = dm_hash_create(32);
+	ls->pvid_to_status = dm_hash_create(32);
+	ls->pvid_to_device = dm_hash_create(32);
+	ls->device_to_pvid = dm_hash_create(32);
+	ls->vgid_to_metadata = dm_hash_create(32);
+	ls->vgid_to_vgname = dm_hash_create(32);
+	ls->pvid_to_vgid = dm_hash_create(32);
+	ls->vgname_to_vgid = dm_hash_create(32);
 	ls->lock.vg = dm_hash_create(32);
 	pthread_mutexattr_init(&rec);
 	pthread_mutexattr_settype(&rec, PTHREAD_MUTEX_RECURSIVE_NP);
-	pthread_mutex_init(&ls->lock.pvs, NULL);
-	pthread_mutex_init(&ls->lock.vgs, &rec);
-	pthread_mutex_init(&ls->lock.pvid_map, NULL);
+	pthread_mutex_init(&ls->lock.pvid_to_status, &rec);
+	pthread_mutex_init(&ls->lock.vgid_to_metadata, &rec);
+	pthread_mutex_init(&ls->lock.pvid_to_vgid, NULL);
 
-	debug("initialised state: vgs = %p\n", ls->vgs);
-	if (!ls->pvs || !ls->vgs)
+	debug("initialised state: vgid_to_metadata = %p\n", ls->vgid_to_metadata);
+	if (!ls->pvid_to_vgid || !ls->vgid_to_metadata)
 		return 0;
 
 	/* if (ls->initial_registrations)
@@ -551,12 +848,12 @@
 static int fini(daemon_state *s)
 {
 	lvmetad_state *ls = s->private;
-	struct dm_hash_node *n = dm_hash_get_first(ls->vgs);
+	struct dm_hash_node *n = dm_hash_get_first(ls->vgid_to_metadata);
 
 	debug("fini\n");
 	while (n) {
-		dm_config_destroy(dm_hash_get_data(ls->vgs, n));
-		n = dm_hash_get_next(ls->vgs, n);
+		dm_config_destroy(dm_hash_get_data(ls->vgid_to_metadata, n));
+		n = dm_hash_get_next(ls->vgid_to_metadata, n);
 	}
 
 	n = dm_hash_get_first(ls->lock.vg);
@@ -567,9 +864,11 @@
 	}
 
 	dm_hash_destroy(ls->lock.vg);
-	dm_hash_destroy(ls->pvs);
-	dm_hash_destroy(ls->vgs);
-	dm_hash_destroy(ls->pvid_map);
+	dm_hash_destroy(ls->pvid_to_status);
+	dm_hash_destroy(ls->pvid_to_device);
+	dm_hash_destroy(ls->device_to_pvid);
+	dm_hash_destroy(ls->vgid_to_metadata);
+	dm_hash_destroy(ls->pvid_to_vgid);
 	return 1;
 }
 
@@ -596,10 +895,13 @@
 	s.daemon_init = init;
 	s.daemon_fini = fini;
 	s.handler = handler;
-	s.socket_path = "/var/run/lvm/lvmetad.socket";
-	s.pidfile = "/var/run/lvm/lvmetad.pid";
+	s.socket_path = getenv("LVM_LVMETAD_SOCKET");
+	if (!s.socket_path)
+		s.socket_path = DEFAULT_RUN_DIR "/lvmetad.socket";
+	s.pidfile = DEFAULT_RUN_DIR "/lvmetad.pid";
         s.log_level = 0;
 
+	// use getopt_long
 	while ((opt = getopt(argc, argv, "?fhVdRs:")) != EOF) {
 		switch (opt) {
 		case 'h':
@@ -617,7 +919,7 @@
 		case 'd':
 			s.log_level++;
 			break;
-		case 's':
+		case 's': // --socket
 			s.socket_path = optarg;
 			break;
 		case 'V':


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2012-02-13 14:25 zkabelac
  0 siblings, 0 replies; 31+ messages in thread
From: zkabelac @ 2012-02-13 14:25 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac@sourceware.org	2012-02-13 14:25:14

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	Add some FIXME around allocation code
	
	Remove also unreachable break..

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.31&r2=1.32

--- LVM2/daemons/lvmetad/lvmetad-core.c	2012/01/25 21:42:09	1.31
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2012/02/13 14:25:14	1.32
@@ -88,7 +88,7 @@
  * TODO: This set_flag function is pretty generic and might make sense in a
  * library here or there.
  */
-static void set_flag(struct dm_config_tree *cft, struct dm_config_node *parent,
+static int set_flag(struct dm_config_tree *cft, struct dm_config_node *parent,
 		     const char *field, const char *flag, int want) {
 	struct dm_config_value *value = NULL, *pred = NULL;
 	struct dm_config_node *node = dm_config_find_node(parent->child, field);
@@ -103,10 +103,10 @@
 	}
 
 	if (value && want)
-		return;
+		return 1;
 
 	if (!value && !want)
-		return;
+		return 1;
 
 	if (value && !want) {
 		if (pred) {
@@ -127,12 +127,17 @@
 			node->parent = parent;
 			parent->child = node;
 		}
-		new = dm_config_create_value(cft);
+		if (!(new = dm_config_create_value(cft))) {
+			/* FIXME error reporting */
+			return 0;
+		}
 		new->type = DM_CFG_STRING;
 		new->v.str = flag;
 		new->next = node->v;
 		node->v = new;
 	}
+
+	return 1;
 }
 
 /* Either the "big" vgs lock, or a per-vg lock needs to be held before entering
@@ -149,8 +154,11 @@
 	while (pv) {
 		const char *uuid = dm_config_find_str(pv->child, "id", NULL);
 		int found = uuid ? (dm_hash_lookup(s->pvs, uuid) ? 1 : 0) : 0;
-		if (act)
-			set_flag(cft, pv, "status", "MISSING", !found);
+		if (act &&
+		    !set_flag(cft, pv, "status", "MISSING", !found)) {
+			complete =  0;
+			break;
+		}
 		if (!found) {
 			complete = 0;
 			if (!act) { // optimisation
@@ -215,7 +223,7 @@
 	res.error = 0;
 	unlock_vg(s, uuid);
 
-	update_pv_status(s, cft, n, 1);
+	update_pv_status(s, cft, n, 1); /* FIXME error reporting */
 
 	return res;
 }
@@ -615,7 +623,6 @@
 		case 'V':
 			printf("lvmetad version 0\n");
 			exit(1);
-			break;
 		}
 	}
 


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2012-01-25 21:42 zkabelac
  0 siblings, 0 replies; 31+ messages in thread
From: zkabelac @ 2012-01-25 21:42 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac@sourceware.org	2012-01-25 21:42:10

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	Add breaks for cases

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.30&r2=1.31

--- LVM2/daemons/lvmetad/lvmetad-core.c	2012/01/25 13:06:57	1.30
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2012/01/25 21:42:09	1.31
@@ -230,9 +230,9 @@
 		return -1;
 
 	switch (a->type) {
-	case DM_CFG_STRING: r = strcmp(a->v.str, b->v.str);
-	case DM_CFG_FLOAT: r = (a->v.f == b->v.f);
-	case DM_CFG_INT: r = (a->v.i == b->v.i);
+	case DM_CFG_STRING: r = strcmp(a->v.str, b->v.str); break;
+	case DM_CFG_FLOAT: r = (a->v.f == b->v.f); break;
+	case DM_CFG_INT: r = (a->v.i == b->v.i); break;
 	case DM_CFG_EMPTY_ARRAY: return 0;
 	}
 


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2011-12-18 22:31 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2011-12-18 22:31 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2011-12-18 22:31:11

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	Fix up lvmetad for the minor API change in dm_config_create.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.27&r2=1.28

--- LVM2/daemons/lvmetad/lvmetad-core.c	2011/10/30 21:58:59	1.27
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2011/12/18 22:31:10	1.28
@@ -175,7 +175,7 @@
 
 	metadata = cft->root;
 
-	res.cft = dm_config_create(NULL, 0);
+	res.cft = dm_config_create();
 
 	/* The response field */
 	res.cft->root = n = dm_config_create_node(res.cft, "response");
@@ -295,7 +295,7 @@
 		goto out;
 	}
 
-	cft = dm_config_create(NULL, 0);
+	cft = dm_config_create();
 	cft->root = dm_config_clone_node(cft, metadata, 0);
 	vgid = dm_config_find_str(cft->root, "metadata/id", NULL);
 


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2011-09-17 13:33 zkabelac
  0 siblings, 0 replies; 31+ messages in thread
From: zkabelac @ 2011-09-17 13:33 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac@sourceware.org	2011-09-17 13:33:51

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	Fix for gcc compilation warnings
	
	and put _XOPEN_SOURCE so pthread_mutexattr is properly defined.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.25&r2=1.26

--- LVM2/daemons/lvmetad/lvmetad-core.c	2011/09/02 11:04:12	1.25
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2011/09/17 13:33:51	1.26
@@ -1,3 +1,6 @@
+#define _GNU_SOURCE
+#define _XOPEN_SOURCE 500  /* pthread */
+
 #include <assert.h>
 #include <pthread.h>
 #include <malloc.h>
@@ -19,6 +22,7 @@
 	} lock;
 } lvmetad_state;
 
+__attribute__ ((format(printf, 1, 2)))
 static void debug(const char *fmt, ...) {
 	va_list ap;
 	va_start(ap, fmt);
@@ -42,8 +46,11 @@
  * allocating memory that we never release. Not good.
  */
 static struct dm_config_tree *lock_vg(lvmetad_state *s, const char *id) {
+	pthread_mutex_t *vg;
+	struct dm_config_tree *cft;
+
 	lock_vgs(s);
-	pthread_mutex_t *vg = dm_hash_lookup(s->lock.vg, id);
+	vg = dm_hash_lookup(s->lock.vg, id);
 	if (!vg) {
 		pthread_mutexattr_t rec;
 		pthread_mutexattr_init(&rec);
@@ -53,7 +60,7 @@
 		dm_hash_insert(s->lock.vg, id, vg);
 	}
 	pthread_mutex_lock(vg);
-	struct dm_config_tree *cft = dm_hash_lookup(s->vgs, id);
+	cft = dm_hash_lookup(s->vgs, id);
 	unlock_vgs(s);
 	return cft;
 }
@@ -81,6 +88,7 @@
 		     const char *field, const char *flag, int want) {
 	struct dm_config_value *value = NULL, *pred = NULL;
 	struct dm_config_node *node = dm_config_find_node(parent->child, field);
+	struct dm_config_value *new;
 
 	if (node)
 		value = node->v;
@@ -112,7 +120,7 @@
 			node->parent = parent;
 			parent->child = node;
 		}
-		struct dm_config_value *new = dm_config_create_value(cft);
+		new = dm_config_create_value(cft);
 		new->type = DM_CFG_STRING;
 		new->v.str = flag;
 		new->next = node->v;
@@ -126,10 +134,11 @@
 			    struct dm_config_tree *cft,
 			    struct dm_config_node *vg, int act)
 {
+	struct dm_config_node *pv;
 	int complete = 1;
 
 	lock_pvs(s);
-	struct dm_config_node *pv = pvs(vg);
+	pv = pvs(vg);
 	while (pv) {
 		const char *uuid = dm_config_find_str(pv->child, "id", NULL);
 		int found = uuid ? (dm_hash_lookup(s->pvs, uuid) ? 1 : 0) : 0;
@@ -151,18 +160,21 @@
 
 static response vg_by_uuid(lvmetad_state *s, request r)
 {
+	struct dm_config_tree *cft;
+	struct dm_config_node *metadata;
+	struct dm_config_node *n;
+	response res = { .buffer = NULL };
 	const char *uuid = daemon_request_str(r, "uuid", "NONE");
+
 	debug("vg_by_uuid: %s (vgs = %p)\n", uuid, s->vgs);
-	struct dm_config_tree *cft = lock_vg(s, uuid);
+	cft = lock_vg(s, uuid);
 	if (!cft || !cft->root) {
 		unlock_vg(s, uuid);
 		return daemon_reply_simple("failed", "reason = %s", "uuid not found", NULL);
 	}
 
-	struct dm_config_node *metadata = cft->root;
+	metadata = cft->root;
 
-	response res = { .buffer = NULL };
-	struct dm_config_node *n;
 	res.cft = dm_config_create(NULL, 0);
 
 	/* The response field */
@@ -249,14 +261,19 @@
  * (anything that might have been retained is copied). */
 static int update_metadata(lvmetad_state *s, const char *_vgid, struct dm_config_node *metadata)
 {
+	struct dm_config_tree *cft;
+	struct dm_config_tree *old;
+	const char *vgid;
 	int retval = 0;
+	int haveseq = -1;
+	int seq;
+
 	lock_vgs(s);
-	struct dm_config_tree *old = dm_hash_lookup(s->vgs, _vgid);
+	old = dm_hash_lookup(s->vgs, _vgid);
 	lock_vg(s, _vgid);
 	unlock_vgs(s);
 
-	int seq = dm_config_find_int(metadata, "metadata/seqno", -1);
-	int haveseq = -1;
+	seq = dm_config_find_int(metadata, "metadata/seqno", -1);
 
 	if (old)
 		haveseq = dm_config_find_int(old->root, "metadata/seqno", -1);
@@ -278,9 +295,9 @@
 		goto out;
 	}
 
-	struct dm_config_tree *cft = dm_config_create(NULL, 0);
+	cft = dm_config_create(NULL, 0);
 	cft->root = dm_config_clone_node(cft, metadata, 0);
-	const char *vgid = dm_config_find_str(cft->root, "metadata/id", NULL);
+	vgid = dm_config_find_str(cft->root, "metadata/id", NULL);
 
 	if (!vgid)
 		goto out;
@@ -313,6 +330,7 @@
 	struct dm_config_node *metadata = dm_config_find_node(r.cft->root, "metadata");
 	const char *pvid = daemon_request_str(r, "uuid", NULL);
 	const char *vgid = daemon_request_str(r, "metadata/id", NULL);
+	int complete = 0;
 
 	if (!pvid)
 		return daemon_reply_simple("failed", "reason = %s", "need PV UUID", NULL);
@@ -338,7 +356,6 @@
 		unlock_pvid_map(s);
 	}
 
-	int complete = 0;
 	if (vgid) {
 		struct dm_config_tree *cft = lock_vg(s, vgid);
 		complete = update_pv_status(s, cft, cft->root, 0);
@@ -374,14 +391,13 @@
 
 static int init(daemon_state *s)
 {
+	pthread_mutexattr_t rec;
 	lvmetad_state *ls = s->private;
 
 	ls->pvs = dm_hash_create(32);
 	ls->vgs = dm_hash_create(32);
 	ls->pvid_map = dm_hash_create(32);
-
 	ls->lock.vg = dm_hash_create(32);
-	pthread_mutexattr_t rec;
 	pthread_mutexattr_init(&rec);
 	pthread_mutexattr_settype(&rec, PTHREAD_MUTEX_RECURSIVE_NP);
 	pthread_mutex_init(&ls->lock.pvs, NULL);
@@ -400,9 +416,10 @@
 
 static int fini(daemon_state *s)
 {
-	debug("fini\n");
 	lvmetad_state *ls = s->private;
 	struct dm_hash_node *n = dm_hash_get_first(ls->vgs);
+
+	debug("fini\n");
 	while (n) {
 		dm_config_destroy(dm_hash_get_data(ls->vgs, n));
 		n = dm_hash_get_next(ls->vgs, n);
@@ -447,6 +464,7 @@
 	s.handler = handler;
 	s.socket_path = "/var/run/lvm/lvmetad.socket";
 	s.pidfile = "/var/run/lvm/lvmetad.pid";
+        s.log_level = 0;
 
 	while ((opt = getopt(argc, argv, "?fhVdR")) != EOF) {
 		switch (opt) {


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2011-09-02 11:04 zkabelac
  0 siblings, 0 replies; 31+ messages in thread
From: zkabelac @ 2011-09-02 11:04 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	zkabelac@sourceware.org	2011-09-02 11:04:12

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	Compile fix
	
	Reflecting change in dm_config_value  float r -> float f.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.24&r2=1.25

--- LVM2/daemons/lvmetad/lvmetad-core.c	2011/08/31 12:39:58	1.24
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2011/09/02 11:04:12	1.25
@@ -192,7 +192,7 @@
 
 	switch (a->type) {
 	case DM_CFG_STRING: r = strcmp(a->v.str, b->v.str);
-	case DM_CFG_FLOAT: r = (a->v.r == b->v.r);
+	case DM_CFG_FLOAT: r = (a->v.f == b->v.f);
 	case DM_CFG_INT: r = (a->v.i == b->v.i);
 	case DM_CFG_EMPTY_ARRAY: return 0;
 	}


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2011-07-25 17:59 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2011-07-25 17:59 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2011-07-25 17:59:51

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	lvmetad: Edit the MISSING_PV flags only after making a "reply" copy of the
	metadata, which is then serialised and discarded. This fixes a couple of
	outstanding TODO items about handling the MISSING flags correctly.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.21&r2=1.22

--- LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/25 15:51:51	1.21
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/25 17:59:50	1.22
@@ -65,6 +65,89 @@
 	unlock_vgs(s);
 }
 
+static struct config_node *pvs(struct config_node *vg)
+{
+	struct config_node *pv = find_config_node(vg, "metadata/physical_volumes");
+	if (pv)
+		pv = pv->child;
+	return pv;
+}
+
+/*
+ * TODO: This set_flag function is pretty generic and might make sense in a
+ * library here or there.
+ */
+static void set_flag(struct config_tree *cft, struct config_node *parent,
+		     char *field, const char *flag, int want) {
+	struct config_value *value = NULL, *pred = NULL;
+	struct config_node *node = find_config_node(parent->child, field);
+	int found = 0;
+
+	if (node)
+		value = node->v;
+
+	while (value && value->type != CFG_EMPTY_ARRAY && strcmp(value->v.str, flag)) {
+		pred = value;
+		value = value->next;
+	}
+
+	if (value && want)
+		return;
+
+	if (!value && !want)
+		return;
+
+	if (value && !want) {
+		if (pred)
+			pred->next = value->next;
+		if (value == node->v)
+			node->v = value->next;
+	}
+
+	if (!value && want) {
+		if (!node) {
+			node = create_config_node(cft, field);
+			node->sib = parent->child;
+			node->v = create_config_value(cft);
+			node->v->type = CFG_EMPTY_ARRAY;
+			node->parent = parent;
+			parent->child = node;
+		}
+		struct config_value *new = create_config_value(cft);
+		new->type = CFG_STRING;
+		new->v.str = flag;
+		new->next = node->v;
+		node->v = new;
+	}
+}
+
+/* Either the "big" vgs lock, or a per-vg lock needs to be held before entering
+ * this function. */
+static int update_pv_status(lvmetad_state *s, struct config_tree *cft, struct config_node *vg, int act)
+{
+	int complete = 1;
+
+	lock_pvs(s);
+	struct config_node *pv = pvs(vg);
+	while (pv) {
+		const char *uuid = find_config_str(pv->child, "id", NULL);
+		int found = uuid ? (dm_hash_lookup(s->pvs, uuid) ? 1 : 0) : 0;
+		if (act)
+			set_flag(cft, pv, "status", "MISSING", !found);
+		if (!found) {
+			complete = 0;
+			if (!act) { // optimisation
+				unlock_pvs(s);
+				return complete;
+			}
+		}
+		pv = pv->sib;
+	}
+	unlock_pvs(s);
+
+	return complete;
+}
+
 static response vg_by_uuid(lvmetad_state *s, request r)
 {
 	const char *uuid = daemon_request_str(r, "uuid", "NONE");
@@ -92,14 +175,11 @@
 	res.error = 0;
 	unlock_vg(s, uuid);
 
+	update_pv_status(s, cft, n, 1);
+
 	return res;
 }
 
-/*
- * TODO: Accept different flag permutations? Or else, ensure fixed ordering of
- * flags in set_flag below (following the same order as we have in
- * lib/format_text/flags.c).
- */
 static int compare_value(struct config_value *a, struct config_value *b)
 {
 	if (a->type > b->type)
@@ -143,85 +223,6 @@
 	return result;
 }
 
-/*
- * TODO: This set_flag function is pretty generic and might make sense in a
- * library here or there.
- */
-static void set_flag(struct config_tree *cft, struct config_node *parent,
-		     char *field, const char *flag, int want) {
-	struct config_value *value = NULL, *pred = NULL;
-	struct config_node *node = find_config_node(parent->child, field);
-	int found = 0;
-
-	if (node)
-		value = node->v;
-
-	while (value && value->type != CFG_EMPTY_ARRAY && strcmp(value->v.str, flag)) {
-		pred = value;
-		value = value->next;
-	}
-
-	if (value && want)
-		return;
-
-	if (!value && !want)
-		return;
-
-	if (value && !want) {
-		if (pred)
-			pred->next = value->next;
-		if (value == node->v)
-			node->v = value->next;
-	}
-
-	if (!value && want) {
-		if (!node) {
-			node = create_config_node(cft, field);
-			node->sib = parent->child;
-			node->v = create_config_value(cft);
-			node->v->type = CFG_EMPTY_ARRAY;
-			node->parent = parent;
-			parent->child = node;
-		}
-		struct config_value *new = create_config_value(cft);
-		new->type = CFG_STRING;
-		new->v.str = flag;
-		new->next = node->v;
-		node->v = new;
-	}
-}
-
-struct config_node *pvs(struct config_tree *vg)
-{
-	struct config_node *pv = find_config_node(vg->root, "metadata/physical_volumes");
-	if (pv)
-		pv = pv->child;
-	return pv;
-}
-
-/* Either the "big" vgs lock, or a per-vg lock needs to be held before entering
- * this function. */
-static int update_pv_status(lvmetad_state *s, struct config_tree *vg)
-{
-	int complete = 1;
-
-	lock_pvs(s);
-	struct config_node *pv = pvs(vg);
-	while (pv) {
-		const char *uuid = find_config_str(pv->child, "id", NULL);
-		int found = uuid ? (dm_hash_lookup(s->pvs, uuid) ? 1 : 0) : 0;
-		// TODO: avoid the override here if MISSING came from the actual
-		// metadata, as opposed from our manipulation...
-		set_flag(vg, pv, "status", "MISSING", !found);
-		if (!found)
-			complete = 0;
-		pv = pv->sib;
-	}
-	unlock_pvs(s);
-
-	return complete;
-}
-
 /* You need to be holding the pvid_map lock already to call this. */
 int update_pvid_map(lvmetad_state *s, struct config_tree *vg, const char *vgid)
 {
@@ -261,7 +262,6 @@
 
 	if (seq == haveseq) {
 		retval = 1;
-		// TODO: disregard the MISSING_PV flag status in this comparison
 		if (compare_config(metadata, old->root))
 			retval = 0;
 		goto out;
@@ -337,7 +337,7 @@
 	int complete = 0;
 	if (vgid) {
 		struct config_tree *cft = lock_vg(s, vgid);
-		complete = update_pv_status(s, cft);
+		complete = update_pv_status(s, cft, cft->root, 0);
 		unlock_vg(s, vgid);
 	}
 


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2011-07-25 15:51 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2011-07-25 15:51 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2011-07-25 15:51:51

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	lvmetad: Check integrity of multiple metadata copies, i.e. ensure that seqno
	equality implies metadata equality. Signal error in response to any update
	requests that try to overwrite metadata without providing a higher seqno.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.20&r2=1.21

--- LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/25 15:33:04	1.20
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/25 15:51:51	1.21
@@ -96,6 +96,54 @@
 }
 
 /*
+ * TODO: Accept different flag permutations? Or else, ensure fixed ordering of
+ * flags in set_flag below (following the same order as we have in
+ * lib/format_text/flags.c).
+ */
+static int compare_value(struct config_value *a, struct config_value *b)
+{
+	if (a->type > b->type)
+		return 1;
+	if (a->type < b->type)
+		return -1;
+
+	switch (a->type) {
+	case CFG_STRING: return strcmp(a->v.str, b->v.str);
+	case CFG_FLOAT: return a->v.r == b->v.r;
+	case CFG_INT: return a->v.i == b->v.i;
+	case CFG_EMPTY_ARRAY: return 0;
+	}
+
+	if (a->next && b->next)
+		return compare_value(a->next, b->next);
+}
+
+static int compare_config(struct config_node *a, struct config_node *b)
+{
+	int result = 0;
+	if (a->v && b->v)
+		result = compare_value(a->v, b->v);
+	if (a->v && !b->v)
+		result = 1;
+	if (!a->v && b->v)
+		result = -1;
+	if (a->child && b->child)
+		result = compare_config(a->child, b->child);
+
+	if (result)
+		return result;
+
+	if (a->sib && b->sib)
+		result = compare_config(a->sib, b->sib);
+	if (a->sib && !b->sib)
+		result = 1;
+	if (!a->sib && b->sib)
+		result = -1;
+
+	return result;
+}
+
+/*
  * TODO: This set_flag function is pretty generic and might make sense in a
  * library here or there.
  */
@@ -212,8 +260,10 @@
 		goto out;
 
 	if (seq == haveseq) {
-		// TODO: compare old->root with metadata to ensure equality
 		retval = 1;
+		// TODO: disregard the MISSING_PV flag status in this comparison
+		if (compare_config(metadata, old->root))
+			retval = 0;
 		goto out;
 	}
 


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2011-07-25 15:33 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2011-07-25 15:33 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2011-07-25 15:33:04

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	lvmetad: A couple of TODOs, and fix a few trivial memory leaks.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.19&r2=1.20

--- LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 21:33:41	1.19
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/25 15:33:04	1.20
@@ -36,6 +36,11 @@
 void lock_pvid_map(lvmetad_state *s) { pthread_mutex_lock(&s->lock.pvid_map); }
 void unlock_pvid_map(lvmetad_state *s) { pthread_mutex_unlock(&s->lock.pvid_map); }
 
+/*
+ * TODO: It may be beneficial to clean up the vg lock hash from time to time,
+ * since if we have many "rogue" requests for nonexistent things, we will keep
+ * allocating memory that we never release. Not good.
+ */
 struct config_tree *lock_vg(lvmetad_state *s, const char *id) {
 	lock_vgs(s);
 	pthread_mutex_t *vg = dm_hash_lookup(s->lock.vg, id);
@@ -90,6 +95,10 @@
 	return res;
 }
 
+/*
+ * TODO: This set_flag function is pretty generic and might make sense in a
+ * library here or there.
+ */
 static void set_flag(struct config_tree *cft, struct config_node *parent,
 		     char *field, const char *flag, int want) {
 	struct config_value *value = NULL, *pred = NULL;
@@ -337,14 +346,25 @@
 
 static int fini(daemon_state *s)
 {
+	debug("fini\n");
 	lvmetad_state *ls = s->private;
 	struct dm_hash_node *n = dm_hash_get_first(ls->vgs);
 	while (n) {
 		destroy_config_tree(dm_hash_get_data(ls->vgs, n));
 		n = dm_hash_get_next(ls->vgs, n);
 	}
+
+	n = dm_hash_get_first(ls->lock.vg);
+	while (n) {
+		pthread_mutex_destroy(dm_hash_get_data(ls->lock.vg, n));
+		free(dm_hash_get_data(ls->lock.vg, n));
+		n = dm_hash_get_next(ls->lock.vg, n);
+	}
+
+	dm_hash_destroy(ls->lock.vg);
 	dm_hash_destroy(ls->pvs);
 	dm_hash_destroy(ls->vgs);
+	dm_hash_destroy(ls->pvid_map);
 	return 1;
 }
 


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2011-07-20 21:33 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2011-07-20 21:33 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2011-07-20 21:33:41

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	lvmetad: Obliterate vg_status by returning the same information from
	update_pv_status, saving a dozen lines of code and execution time of one
	walkthrough of the PV list.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.18&r2=1.19

--- LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 21:27:28	1.18
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 21:33:41	1.19
@@ -144,8 +144,10 @@
 
 /* Either the "big" vgs lock, or a per-vg lock needs to be held before entering
  * this function. */
-static void update_pv_status(lvmetad_state *s, struct config_tree *vg)
+static int update_pv_status(lvmetad_state *s, struct config_tree *vg)
 {
+	int complete = 1;
+
 	lock_pvs(s);
 	struct config_node *pv = pvs(vg);
 	while (pv) {
@@ -154,30 +156,13 @@
 		// TODO: avoid the override here if MISSING came from the actual
 		// metadata, as opposed from our manipulation...
 		set_flag(vg, pv, "status", "MISSING", !found);
+		if (!found)
+			complete = 0;
 		pv = pv->sib;
 	}
 	unlock_pvs(s);
-}
-
-static int vg_status(lvmetad_state *s, const char *vgid)
-{
-	struct config_tree *vg = lock_vg(s, vgid);
-	struct config_node *pv = pvs(vg);
 
-	while (pv) {
-		const char *uuid = find_config_str(pv->child, "id", NULL);
-		lock_pvs(s);
-		int found = uuid ? (dm_hash_lookup(s->pvs, uuid) ? 1 : 0) : 0;
-		unlock_pvs(s);
-		if (!found) {
-			unlock_vg(s, vgid);
-			return 0;
-		}
-		pv = pv->sib;
-	}
-
-	unlock_vg(s, vgid);
-	return 1;
+	return complete;
 }
 
 /* You need to be holding the pvid_map lock already to call this. */
@@ -269,6 +254,8 @@
 	if (!pvid)
 		return daemon_reply_simple("failed", "reason = %s", "need PV UUID", NULL);
 
+	debug("pv_add %s, vgid = %s\n", pvid, vgid);
+
 	lock_pvs(s);
 	dm_hash_insert(s->pvs, pvid, (void*)1);
 	unlock_pvs(s);
@@ -288,14 +275,13 @@
 		unlock_pvid_map(s);
 	}
 
+	int complete = 0;
 	if (vgid) {
 		struct config_tree *cft = lock_vg(s, vgid);
-		update_pv_status(s, cft);
+		complete = update_pv_status(s, cft);
 		unlock_vg(s, vgid);
 	}
 
-	int complete = vgid ? vg_status(s, vgid) : 0;
-
 	return daemon_reply_simple("OK",
 				   "status = %s", complete ? "complete" : "partial",
 				   "vgid = %s", vgid ? vgid : "#orphan",


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2011-07-20 21:27 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2011-07-20 21:27 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2011-07-20 21:27:29

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	lvmetad: Fix a possible infinite loop in vg_status.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.17&r2=1.18

--- LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 21:26:18	1.17
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 21:27:28	1.18
@@ -166,11 +166,8 @@
 
 	while (pv) {
 		const char *uuid = find_config_str(pv->child, "id", NULL);
-		if (!uuid)
-			continue; // FIXME?
-
 		lock_pvs(s);
-		int found = dm_hash_lookup(s->pvs, uuid) ? 1 : 0;
+		int found = uuid ? (dm_hash_lookup(s->pvs, uuid) ? 1 : 0) : 0;
 		unlock_pvs(s);
 		if (!found) {
 			unlock_vg(s, vgid);


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2011-07-20 21:26 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2011-07-20 21:26 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2011-07-20 21:26:18

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	lvmetad: Robustify update_pv_status and remove an useless lookup.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.16&r2=1.17

--- LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 21:23:43	1.16
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 21:26:18	1.17
@@ -149,9 +149,8 @@
 	lock_pvs(s);
 	struct config_node *pv = pvs(vg);
 	while (pv) {
-		const char *uuid = find_config_str(pv->child, "id", "N/A");
-		const char *vgid = find_config_str(vg->root, "metadata/id", "N/A");
-		int found = dm_hash_lookup(s->pvs, uuid) ? 1 : 0;
+		const char *uuid = find_config_str(pv->child, "id", NULL);
+		int found = uuid ? (dm_hash_lookup(s->pvs, uuid) ? 1 : 0) : 0;
 		// TODO: avoid the override here if MISSING came from the actual
 		// metadata, as opposed from our manipulation...
 		set_flag(vg, pv, "status", "MISSING", !found);


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2011-07-20 21:23 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2011-07-20 21:23 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2011-07-20 21:23:44

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	First stab at making lvmetad-core threadsafe. The current design should allow
	very reasonable amount of parallel access, although the hash tables may become
	a point of contention under heavy loads. Nevertheless, there should be orders
	of magnitude less contention on the hash table locks than we currently have on
	block device scanning.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.15&r2=1.16

--- LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 18:45:32	1.15
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 21:23:43	1.16
@@ -1,4 +1,5 @@
 #include <assert.h>
+#include <pthread.h>
 
 #include "libdevmapper.h"
 #include <malloc.h>
@@ -10,15 +11,64 @@
 	struct dm_hash_table *pvs;
 	struct dm_hash_table *vgs;
 	struct dm_hash_table *pvid_map;
+	struct {
+		struct dm_hash_table *vg;
+		pthread_mutex_t pvs;
+		pthread_mutex_t vgs;
+		pthread_mutex_t pvid_map;
+	} lock;
 } lvmetad_state;
 
+void debug(const char *fmt, ...) {
+	va_list ap;
+	va_start(ap, fmt);
+	fprintf(stderr, "[D %u] ", pthread_self());
+	vfprintf(stderr, fmt, ap);
+	va_end(ap);
+};
+
+void lock_pvs(lvmetad_state *s) { pthread_mutex_lock(&s->lock.pvs); }
+void unlock_pvs(lvmetad_state *s) { pthread_mutex_unlock(&s->lock.pvs); }
+
+void lock_vgs(lvmetad_state *s) { pthread_mutex_lock(&s->lock.vgs); }
+void unlock_vgs(lvmetad_state *s) { pthread_mutex_unlock(&s->lock.vgs); }
+
+void lock_pvid_map(lvmetad_state *s) { pthread_mutex_lock(&s->lock.pvid_map); }
+void unlock_pvid_map(lvmetad_state *s) { pthread_mutex_unlock(&s->lock.pvid_map); }
+
+struct config_tree *lock_vg(lvmetad_state *s, const char *id) {
+	lock_vgs(s);
+	pthread_mutex_t *vg = dm_hash_lookup(s->lock.vg, id);
+	if (!vg) {
+		pthread_mutexattr_t rec;
+		pthread_mutexattr_init(&rec);
+		pthread_mutexattr_settype(&rec, PTHREAD_MUTEX_RECURSIVE_NP);
+		vg = malloc(sizeof(pthread_mutex_t));
+		pthread_mutex_init(vg, &rec);
+		dm_hash_insert(s->lock.vg, id, vg);
+	}
+	pthread_mutex_lock(vg);
+	struct config_tree *cft = dm_hash_lookup(s->vgs, id);
+	unlock_vgs(s);
+	return cft;
+}
+
+void unlock_vg(lvmetad_state *s, const char *id) {
+	lock_vgs(s); /* someone might be changing the s->lock.vg structure right
+		      * now, so avoid stepping on each other's toes */
+	pthread_mutex_unlock(dm_hash_lookup(s->lock.vg, id));
+	unlock_vgs(s);
+}
+
 static response vg_by_uuid(lvmetad_state *s, request r)
 {
 	const char *uuid = daemon_request_str(r, "uuid", "NONE");
-	fprintf(stderr, "[D] vg_by_uuid: %s (vgs = %p)\n", uuid, s->vgs);
-	struct config_tree *cft = dm_hash_lookup(s->vgs, uuid);
-	if (!cft || !cft->root)
+	debug("vg_by_uuid: %s (vgs = %p)\n", uuid, s->vgs);
+	struct config_tree *cft = lock_vg(s, uuid);
+	if (!cft || !cft->root) {
+		unlock_vg(s, uuid);
 		return daemon_reply_simple("failed", "reason = %s", "uuid not found", NULL);
+	}
 
 	struct config_node *metadata = cft->root;
 
@@ -35,6 +85,8 @@
 	n = n->sib = clone_config_node(res.cft, metadata, 1);
 	n->parent = res.cft->root;
 	res.error = 0;
+	unlock_vg(s, uuid);
+
 	return res;
 }
 
@@ -47,7 +99,7 @@
 	if (node)
 		value = node->v;
 
-	while (value && strcmp(value->v.str, flag)) {
+	while (value && value->type != CFG_EMPTY_ARRAY && strcmp(value->v.str, flag)) {
 		pred = value;
 		value = value->next;
 	}
@@ -90,62 +142,49 @@
 	return pv;
 }
 
-static void update_pv_status_in_vg(lvmetad_state *s, struct config_tree *vg)
+/* Either the "big" vgs lock, or a per-vg lock needs to be held before entering
+ * this function. */
+static void update_pv_status(lvmetad_state *s, struct config_tree *vg)
 {
+	lock_pvs(s);
 	struct config_node *pv = pvs(vg);
 	while (pv) {
 		const char *uuid = find_config_str(pv->child, "id", "N/A");
 		const char *vgid = find_config_str(vg->root, "metadata/id", "N/A");
 		int found = dm_hash_lookup(s->pvs, uuid) ? 1 : 0;
+		// TODO: avoid the override here if MISSING came from the actual
+		// metadata, as opposed from our manipulation...
 		set_flag(vg, pv, "status", "MISSING", !found);
 		pv = pv->sib;
 	}
+	unlock_pvs(s);
 }
 
 static int vg_status(lvmetad_state *s, const char *vgid)
 {
-	struct config_tree *vg = dm_hash_lookup(s->vgs, vgid);
+	struct config_tree *vg = lock_vg(s, vgid);
 	struct config_node *pv = pvs(vg);
 
 	while (pv) {
-		const char *uuid = find_config_str(pv->child, "id", "N/A");
-		const char *vgid = find_config_str(vg->root, "metadata/id", "N/A");
+		const char *uuid = find_config_str(pv->child, "id", NULL);
+		if (!uuid)
+			continue; // FIXME?
+
+		lock_pvs(s);
 		int found = dm_hash_lookup(s->pvs, uuid) ? 1 : 0;
-		if (!found)
+		unlock_pvs(s);
+		if (!found) {
+			unlock_vg(s, vgid);
 			return 0;
+		}
 		pv = pv->sib;
 	}
 
+	unlock_vg(s, vgid);
 	return 1;
 }
 
-/*
- * Walk through metadata cache and update PV flags to reflect our current
- * picture of the PVs in the system. If pvid is non-NULL, this is used as a hint
- * as to which PV has changed state. Otherwise, all flags are recomputed from
- * authoritative data (the s->pvs hash).
- */
-static void update_pv_status(lvmetad_state *s, const char *pvid)
-{
-	if (pvid) {
-		const char *vgid = dm_hash_lookup(s->pvid_map, pvid);
-		if (!vgid)
-			return; /* nothing to update */
-
-		struct config_tree *vg = dm_hash_lookup(s->vgs, vgid);
-		assert(vg);
-
-		update_pv_status_in_vg(s, vg);
-	} else {
-		struct dm_hash_node *n = dm_hash_get_first(s->vgs);
-		while (n) {
-			struct config_tree *vg = dm_hash_get_data(s->vgs, n);
-			update_pv_status_in_vg(s, vg);
-			n = dm_hash_get_next(s->vgs, n);
-		}
-	}
-}
-
+/* You need to be holding the pvid_map lock already to call this. */
 int update_pvid_map(lvmetad_state *s, struct config_tree *vg, const char *vgid)
 {
 	struct config_node *pv = pvs(vg);
@@ -162,9 +201,17 @@
 	return 1;
 }
 
+/* No locks need to be held. The pointers are never used outside of the scope of
+ * this function, so they can be safely destroyed after update_metadata returns
+ * (anything that might have been retained is copied). */
 static int update_metadata(lvmetad_state *s, const char *_vgid, struct config_node *metadata)
 {
+	int retval = 0;
+	lock_vgs(s);
 	struct config_tree *old = dm_hash_lookup(s->vgs, _vgid);
+	lock_vg(s, _vgid);
+	unlock_vgs(s);
+
 	int seq = find_config_int(metadata, "metadata/seqno", -1);
 	int haveseq = -1;
 
@@ -172,17 +219,19 @@
 		haveseq = find_config_int(old->root, "metadata/seqno", -1);
 
 	if (seq < 0)
-		return 0; /* bad */
+		goto out;
 
 	if (seq == haveseq) {
 		// TODO: compare old->root with metadata to ensure equality
-		return 1;
+		retval = 1;
+		goto out;
 	}
 
 	if (seq < haveseq) {
 		// TODO: we may want to notify the client that their metadata is
 		// out of date?
-		return 1;
+		retval = 1;
+		goto out;
 	}
 
 	struct config_tree *cft = create_config_tree(NULL, 0);
@@ -190,7 +239,9 @@
 	const char *vgid = find_config_str(cft->root, "metadata/id", NULL);
 
 	if (!vgid)
-		return 0;
+		goto out;
+
+	lock_pvid_map(s);
 
 	if (haveseq >= 0 && haveseq < seq) {
 		/* temporarily orphan all of our PVs */
@@ -200,10 +251,17 @@
 		dm_hash_remove(s->vgs, vgid);
 	}
 
+	lock_vgs(s);
 	dm_hash_insert(s->vgs, vgid, cft);
+	unlock_vgs(s);
+
 	update_pvid_map(s, cft, vgid);
 
-	return 1;
+	unlock_pvid_map(s);
+	retval = 1;
+out:
+	unlock_vg(s, _vgid);
+	return retval;
 }
 
 static response pv_add(lvmetad_state *s, request r)
@@ -215,7 +273,9 @@
 	if (!pvid)
 		return daemon_reply_simple("failed", "reason = %s", "need PV UUID", NULL);
 
+	lock_pvs(s);
 	dm_hash_insert(s->pvs, pvid, (void*)1);
+	unlock_pvs(s);
 
 	if (metadata) {
 		if (!vgid)
@@ -226,10 +286,18 @@
 		if (!update_metadata(s, vgid, metadata))
 			return daemon_reply_simple("failed", "reason = %s",
 						   "metadata update failed", NULL);
-	} else
+	} else {
+		lock_pvid_map(s);
 		vgid = dm_hash_lookup(s->pvid_map, pvid);
+		unlock_pvid_map(s);
+	}
+
+	if (vgid) {
+		struct config_tree *cft = lock_vg(s, vgid);
+		update_pv_status(s, cft);
+		unlock_vg(s, vgid);
+	}
 
-	update_pv_status(s, pvid);
 	int complete = vgid ? vg_status(s, vgid) : 0;
 
 	return daemon_reply_simple("OK",
@@ -247,7 +315,7 @@
 	lvmetad_state *state = s.private;
 	const char *rq = daemon_request_str(r, "request", "NONE");
 
-	fprintf(stderr, "[D] REQUEST: %s\n", rq);
+	debug("REQUEST: %s\n", rq);
 
 	if (!strcmp(rq, "pv_add"))
 		return pv_add(state, r);
@@ -266,7 +334,16 @@
 	ls->pvs = dm_hash_create(32);
 	ls->vgs = dm_hash_create(32);
 	ls->pvid_map = dm_hash_create(32);
-	fprintf(stderr, "[D] initialised state: vgs = %p\n", ls->vgs);
+
+	ls->lock.vg = dm_hash_create(32);
+	pthread_mutexattr_t rec;
+	pthread_mutexattr_init(&rec);
+	pthread_mutexattr_settype(&rec, PTHREAD_MUTEX_RECURSIVE_NP);
+	pthread_mutex_init(&ls->lock.pvs, NULL);
+	pthread_mutex_init(&ls->lock.vgs, &rec);
+	pthread_mutex_init(&ls->lock.pvid_map, NULL);
+
+	debug("initialised state: vgs = %p\n", ls->vgs);
 	if (!ls->pvs || !ls->vgs)
 		return 0;
 


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2011-07-20 18:45 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2011-07-20 18:45 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2011-07-20 18:45:33

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	lvmetad: Avoid stale PV -> VG mappings on metadata update.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.14&r2=1.15

--- LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 18:34:57	1.14
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 18:45:32	1.15
@@ -146,10 +146,9 @@
 	}
 }
 
-int update_pvid_map(lvmetad_state *s, struct config_tree *vg)
+int update_pvid_map(lvmetad_state *s, struct config_tree *vg, const char *vgid)
 {
 	struct config_node *pv = pvs(vg);
-	char *vgid = find_config_str(vg->root, "metadata/id", NULL);
 
 	if (!vgid)
 		return 0;
@@ -163,9 +162,9 @@
 	return 1;
 }
 
-static int update_metadata(lvmetad_state *s, const char *vgid, struct config_node *metadata)
+static int update_metadata(lvmetad_state *s, const char *_vgid, struct config_node *metadata)
 {
-	struct config_tree *old = dm_hash_lookup(s->vgs, vgid);
+	struct config_tree *old = dm_hash_lookup(s->vgs, _vgid);
 	int seq = find_config_int(metadata, "metadata/seqno", -1);
 	int haveseq = -1;
 
@@ -186,16 +185,23 @@
 		return 1;
 	}
 
+	struct config_tree *cft = create_config_tree(NULL, 0);
+	cft->root = clone_config_node(cft, metadata, 0);
+	const char *vgid = find_config_str(cft->root, "metadata/id", NULL);
+
+	if (!vgid)
+		return 0;
+
 	if (haveseq >= 0 && haveseq < seq) {
+		/* temporarily orphan all of our PVs */
+		update_pvid_map(s, old, "#orphan");
 		/* need to update what we have since we found a newer version */
 		destroy_config_tree(old);
 		dm_hash_remove(s->vgs, vgid);
 	}
 
-	struct config_tree *cft = create_config_tree(NULL, 0);
-	cft->root = clone_config_node(cft, metadata, 0);
 	dm_hash_insert(s->vgs, vgid, cft);
-	update_pvid_map(s, cft);
+	update_pvid_map(s, cft, vgid);
 
 	return 1;
 }


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2011-07-20 18:34 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2011-07-20 18:34 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2011-07-20 18:34:57

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	Optimise PV -> VG lookups by using a UUID (hash) map.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.13&r2=1.14

--- LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 18:24:49	1.13
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 18:34:57	1.14
@@ -9,7 +9,7 @@
 typedef struct {
 	struct dm_hash_table *pvs;
 	struct dm_hash_table *vgs;
-	struct dm_hash_table *pvid_to_vgid;
+	struct dm_hash_table *pvid_map;
 } lvmetad_state;
 
 static response vg_by_uuid(lvmetad_state *s, request r)
@@ -39,7 +39,7 @@
 }
 
 static void set_flag(struct config_tree *cft, struct config_node *parent,
-		     const char *field, const char *flag, int want) {
+		     char *field, const char *flag, int want) {
 	struct config_value *value = NULL, *pred = NULL;
 	struct config_node *node = find_config_node(parent->child, field);
 	int found = 0;
@@ -128,11 +128,14 @@
 static void update_pv_status(lvmetad_state *s, const char *pvid)
 {
 	if (pvid) {
-		const char *vgid = dm_hash_lookup(s->pvid_to_vgid, pvid);
-		assert(vgid);
+		const char *vgid = dm_hash_lookup(s->pvid_map, pvid);
+		if (!vgid)
+			return; /* nothing to update */
+
 		struct config_tree *vg = dm_hash_lookup(s->vgs, vgid);
 		assert(vg);
-		update_pv_status_in_vg(s, vg->root);
+
+		update_pv_status_in_vg(s, vg);
 	} else {
 		struct dm_hash_node *n = dm_hash_get_first(s->vgs);
 		while (n) {
@@ -143,24 +146,21 @@
 	}
 }
 
-struct config_tree *vg_from_pvid(lvmetad_state *s, const char *pvid)
+int update_pvid_map(lvmetad_state *s, struct config_tree *vg)
 {
-	struct dm_hash_node *n = dm_hash_get_first(s->vgs);
-
-	while (n) {
-		struct config_tree *vg = dm_hash_get_data(s->vgs, n);
-		struct config_node *pv = pvs(vg);
+	struct config_node *pv = pvs(vg);
+	char *vgid = find_config_str(vg->root, "metadata/id", NULL);
 
-		while (pv) {
-			const char *uuid = find_config_str(pv->child, "id", "N/A");
-			if (!strcmp(uuid, pvid))
-				return vg;
-			pv = pv->sib;
-		}
+	if (!vgid)
+		return 0;
 
-		n = dm_hash_get_next(s->vgs, n);
+	while (pv) {
+		char *pvid = find_config_str(pv->child, "id", NULL);
+		dm_hash_insert(s->pvid_map, pvid, vgid);
+		pv = pv->sib;
 	}
-	return NULL;
+
+	return 1;
 }
 
 static int update_metadata(lvmetad_state *s, const char *vgid, struct config_node *metadata)
@@ -195,6 +195,7 @@
 	struct config_tree *cft = create_config_tree(NULL, 0);
 	cft->root = clone_config_node(cft, metadata, 0);
 	dm_hash_insert(s->vgs, vgid, cft);
+	update_pvid_map(s, cft);
 
 	return 1;
 }
@@ -208,7 +209,7 @@
 	if (!pvid)
 		return daemon_reply_simple("failed", "reason = %s", "need PV UUID", NULL);
 
-	dm_hash_insert(s->pvs, pvid, 1);
+	dm_hash_insert(s->pvs, pvid, (void*)1);
 
 	if (metadata) {
 		if (!vgid)
@@ -219,13 +220,10 @@
 		if (!update_metadata(s, vgid, metadata))
 			return daemon_reply_simple("failed", "reason = %s",
 						   "metadata update failed", NULL);
-	} else {
-		struct config_tree *vg = vg_from_pvid(s, pvid);
-		if (vg)
-			vgid = find_config_str(vg->root, "metadata/id", NULL);
-	}
+	} else
+		vgid = dm_hash_lookup(s->pvid_map, pvid);
 
-	update_pv_status(s, NULL);
+	update_pv_status(s, pvid);
 	int complete = vgid ? vg_status(s, vgid) : 0;
 
 	return daemon_reply_simple("OK",
@@ -261,6 +259,7 @@
 
 	ls->pvs = dm_hash_create(32);
 	ls->vgs = dm_hash_create(32);
+	ls->pvid_map = dm_hash_create(32);
 	fprintf(stderr, "[D] initialised state: vgs = %p\n", ls->vgs);
 	if (!ls->pvs || !ls->vgs)
 		return 0;


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2011-07-20 16:49 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2011-07-20 16:49 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2011-07-20 16:49:21

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	Can't have a global memory pool in lvmetad (that would constitute an ongoing
	memory leak) => remove it (it's been unused anyway).

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.11&r2=1.12

--- LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 16:46:40	1.11
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 16:49:21	1.12
@@ -7,7 +7,6 @@
 #include "../common/daemon-server.h"
 
 typedef struct {
-	struct dm_pool *mem;
 	struct dm_hash_table *pvs;
 	struct dm_hash_table *vgs;
 	struct dm_hash_table *pvid_to_vgid;
@@ -262,9 +261,8 @@
 
 	ls->pvs = dm_hash_create(32);
 	ls->vgs = dm_hash_create(32);
-	ls->mem = dm_pool_create("lvmetad", 1024); /* whatever */
 	fprintf(stderr, "[D] initialised state: vgs = %p\n", ls->vgs);
-	if (!ls->pvs || !ls->vgs || !ls->mem)
+	if (!ls->pvs || !ls->vgs)
 		return 0;
 
 	/* if (ls->initial_registrations)
@@ -276,7 +274,6 @@
 static int fini(daemon_state *s)
 {
 	lvmetad_state *ls = s->private;
-	dm_pool_destroy(ls->mem);
 	return 1;
 }
 


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2011-07-20 16:46 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2011-07-20 16:46 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2011-07-20 16:46:40

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	Make lvmetad also report VGID in reply when adding a PV without MDAs (this
	obviously only works for VGs that already had at least some MDA discovered).

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.10&r2=1.11

--- LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 15:14:17	1.10
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 16:46:40	1.11
@@ -83,12 +83,17 @@
 	}
 }
 
-static void update_pv_status_in_vg(lvmetad_state *s, struct config_tree *vg)
+struct config_node *pvs(struct config_tree *vg)
 {
 	struct config_node *pv = find_config_node(vg->root, "metadata/physical_volumes");
 	if (pv)
 		pv = pv->child;
+	return pv;
+}
 
+static void update_pv_status_in_vg(lvmetad_state *s, struct config_tree *vg)
+{
+	struct config_node *pv = pvs(vg);
 	while (pv) {
 		const char *uuid = find_config_str(pv->child, "id", "N/A");
 		const char *vgid = find_config_str(vg->root, "metadata/id", "N/A");
@@ -101,9 +106,7 @@
 static int vg_status(lvmetad_state *s, const char *vgid)
 {
 	struct config_tree *vg = dm_hash_lookup(s->vgs, vgid);
-	struct config_node *pv = find_config_node(vg->root, "metadata/physical_volumes");
-	if (pv)
-		pv = pv->child;
+	struct config_node *pv = pvs(vg);
 
 	while (pv) {
 		const char *uuid = find_config_str(pv->child, "id", "N/A");
@@ -141,6 +144,26 @@
 	}
 }
 
+struct config_tree *vg_from_pvid(lvmetad_state *s, const char *pvid)
+{
+	struct dm_hash_node *n = dm_hash_get_first(s->vgs);
+
+	while (n) {
+		struct config_tree *vg = dm_hash_get_data(s->vgs, n);
+		struct config_node *pv = pvs(vg);
+
+		while (pv) {
+			const char *uuid = find_config_str(pv->child, "id", "N/A");
+			if (!strcmp(uuid, pvid))
+				return vg;
+			pv = pv->sib;
+		}
+
+		n = dm_hash_get_next(s->vgs, n);
+	}
+	return NULL;
+}
+
 static int update_metadata(lvmetad_state *s, const char *vgid, struct config_node *metadata)
 {
 	struct config_tree *old = dm_hash_lookup(s->vgs, vgid);
@@ -198,8 +221,9 @@
 			return daemon_reply_simple("failed", "reason = %s",
 						   "metadata update failed", NULL);
 	} else {
-		// TODO: find the corresponding VGID when available to give to
-		// the caller, and to find out whether the VG is complete
+		struct config_tree *vg = vg_from_pvid(s, pvid);
+		if (vg)
+			vgid = find_config_str(vg->root, "metadata/id", NULL);
 	}
 
 	update_pv_status(s, NULL);
@@ -207,7 +231,7 @@
 
 	return daemon_reply_simple("OK",
 				   "status = %s", complete ? "complete" : "partial",
-				   vgid ? "vgid = %s" : "", vgid,
+				   "vgid = %s", vgid ? vgid : "#orphan",
 				   NULL);
 }
 


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2011-07-20 15:14 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2011-07-20 15:14 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2011-07-20 15:14:17

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	Make lvmetad report the VG ID and status (complete, partial) in reply to pv_add
	requests.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.9&r2=1.10

--- LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/19 19:15:22	1.9
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/20 15:14:17	1.10
@@ -4,7 +4,6 @@
 #include <malloc.h>
 #include <stdint.h>
 
-#include "metadata-exported.h"
 #include "../common/daemon-server.h"
 
 typedef struct {
@@ -99,6 +98,25 @@
 	}
 }
 
+static int vg_status(lvmetad_state *s, const char *vgid)
+{
+	struct config_tree *vg = dm_hash_lookup(s->vgs, vgid);
+	struct config_node *pv = find_config_node(vg->root, "metadata/physical_volumes");
+	if (pv)
+		pv = pv->child;
+
+	while (pv) {
+		const char *uuid = find_config_str(pv->child, "id", "N/A");
+		const char *vgid = find_config_str(vg->root, "metadata/id", "N/A");
+		int found = dm_hash_lookup(s->pvs, uuid) ? 1 : 0;
+		if (!found)
+			return 0;
+		pv = pv->sib;
+	}
+
+	return 1;
+}
+
 /*
  * Walk through metadata cache and update PV flags to reflect our current
  * picture of the PVs in the system. If pvid is non-NULL, this is used as a hint
@@ -163,7 +181,7 @@
 {
 	struct config_node *metadata = find_config_node(r.cft->root, "metadata");
 	const char *pvid = daemon_request_str(r, "uuid", NULL);
-	fprintf(stderr, "[D] pv_add buffer: %s\n", r.buffer);
+	const char *vgid = daemon_request_str(r, "metadata/id", NULL);
 
 	if (!pvid)
 		return daemon_reply_simple("failed", "reason = %s", "need PV UUID", NULL);
@@ -171,7 +189,6 @@
 	dm_hash_insert(s->pvs, pvid, 1);
 
 	if (metadata) {
-		const char *vgid = daemon_request_str(r, "metadata/id", NULL);
 		if (!vgid)
 			return daemon_reply_simple("failed", "reason = %s", "need VG UUID", NULL);
 		if (daemon_request_int(r, "metadata/seqno", -1) < 0)
@@ -180,11 +197,18 @@
 		if (!update_metadata(s, vgid, metadata))
 			return daemon_reply_simple("failed", "reason = %s",
 						   "metadata update failed", NULL);
+	} else {
+		// TODO: find the corresponding VGID when available to give to
+		// the caller, and to find out whether the VG is complete
 	}
 
 	update_pv_status(s, NULL);
+	int complete = vgid ? vg_status(s, vgid) : 0;
 
-	return daemon_reply_simple("OK", NULL);
+	return daemon_reply_simple("OK",
+				   "status = %s", complete ? "complete" : "partial",
+				   vgid ? "vgid = %s" : "", vgid,
+				   NULL);
 }
 
 static void pv_del(lvmetad_state *s, request r)


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2011-07-19 19:15 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2011-07-19 19:15 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2011-07-19 19:15:22

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	Towards MISSING (PV) flag management in lvmetad.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.8&r2=1.9

--- LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/19 16:48:13	1.8
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/19 19:15:22	1.9
@@ -40,19 +40,61 @@
 	return res;
 }
 
-static void update_pv_status_in_vg(lvmetad_state *s, struct config_node *vg)
+static void set_flag(struct config_tree *cft, struct config_node *parent,
+		     const char *field, const char *flag, int want) {
+	struct config_value *value = NULL, *pred = NULL;
+	struct config_node *node = find_config_node(parent->child, field);
+	int found = 0;
+
+	if (node)
+		value = node->v;
+
+	while (value && strcmp(value->v.str, flag)) {
+		pred = value;
+		value = value->next;
+	}
+
+	if (value && want)
+		return;
+
+	if (!value && !want)
+		return;
+
+	if (value && !want) {
+		if (pred)
+			pred->next = value->next;
+		if (value == node->v)
+			node->v = value->next;
+	}
+
+	if (!value && want) {
+		if (!node) {
+			node = create_config_node(cft, field);
+			node->sib = parent->child;
+			node->v = create_config_value(cft);
+			node->v->type = CFG_EMPTY_ARRAY;
+			node->parent = parent;
+			parent->child = node;
+		}
+		struct config_value *new = create_config_value(cft);
+		new->type = CFG_STRING;
+		new->v.str = flag;
+		new->next = node->v;
+		node->v = new;
+	}
+}
+
+static void update_pv_status_in_vg(lvmetad_state *s, struct config_tree *vg)
 {
-	struct config_node *pv = find_config_node(vg, "metadata/physical_volumes");
+	struct config_node *pv = find_config_node(vg->root, "metadata/physical_volumes");
 	if (pv)
 		pv = pv->child;
 
 	while (pv) {
 		const char *uuid = find_config_str(pv->child, "id", "N/A");
-		if (dm_hash_lookup(s->pvs, uuid)) {
-			fprintf(stderr, "[D] PV %s found\n", uuid);
-		} else {
-			fprintf(stderr, "[D] PV %s is MISSING\n", uuid);
-		}
+		const char *vgid = find_config_str(vg->root, "metadata/id", "N/A");
+		int found = dm_hash_lookup(s->pvs, uuid) ? 1 : 0;
+		set_flag(vg, pv, "status", "MISSING", !found);
 		pv = pv->sib;
 	}
 }
@@ -75,9 +117,7 @@
 		struct dm_hash_node *n = dm_hash_get_first(s->vgs);
 		while (n) {
 			struct config_tree *vg = dm_hash_get_data(s->vgs, n);
-			fprintf(stderr, "[D] checking VG: %s\n",
-				find_config_str(vg, "metadata/id", "?"));
-			update_pv_status_in_vg(s, vg->root);
+			update_pv_status_in_vg(s, vg);
 			n = dm_hash_get_next(s->vgs, n);
 		}
 	}


^ permalink raw reply	[flat|nested] 31+ messages in thread
* LVM2/daemons/lvmetad lvmetad-core.c
@ 2011-07-19 14:14 mornfall
  0 siblings, 0 replies; 31+ messages in thread
From: mornfall @ 2011-07-19 14:14 UTC (permalink / raw)
  To: lvm-devel, lvm2-cvs

CVSROOT:	/cvs/lvm2
Module name:	LVM2
Changes by:	mornfall@sourceware.org	2011-07-19 14:13:59

Modified files:
	daemons/lvmetad: lvmetad-core.c 

Log message:
	More work on cache maintenance code in lvmetad: keep track of PV status.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/LVM2/daemons/lvmetad/lvmetad-core.c.diff?cvsroot=lvm2&r1=1.6&r2=1.7

--- LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/18 14:48:30	1.6
+++ LVM2/daemons/lvmetad/lvmetad-core.c	2011/07/19 14:13:59	1.7
@@ -9,8 +9,9 @@
 
 typedef struct {
 	struct dm_pool *mem;
-	struct dm_hash_table *pvids;
+	struct dm_hash_table *pvs;
 	struct dm_hash_table *vgs;
+	struct dm_hash_table *pvid_to_vgid;
 } lvmetad_state;
 
 static response vg_by_uuid(lvmetad_state *s, request r)
@@ -32,35 +33,85 @@
 	n->v->v.str = "OK";
 
 	/* The metadata section */
-	n = n->sib = create_config_node(res.cft, "metadata");
-	n->v = NULL;
+	n = n->sib = clone_config_node(res.cft, metadata, 1);
 	n->parent = res.cft->root;
-	n->child = clone_config_node(res.cft, metadata, 1);
 	res.error = 0;
-	fprintf(stderr, "[D] vg_by_uuid signing off\n");
 	return res;
 }
 
+static void update_pv_status_in_vg(lvmetad_state *s, struct config_node *vg)
+{
+	struct config_node *pv = find_config_node(vg, "metadata/physical_volumes");
+	if (pv)
+		pv = pv->child;
+
+	while (pv) {
+		const char *uuid = find_config_str(pv->child, "id", "N/A");
+		if (dm_hash_lookup(s->pvs, uuid)) {
+			fprintf(stderr, "[D] PV %s found\n", uuid);
+		} else {
+			fprintf(stderr, "[D] PV %s is MISSING\n", uuid);
+		}
+		pv = pv->sib;
+	}
+}
+
+/*
+ * Walk through metadata cache and update PV flags to reflect our current
+ * picture of the PVs in the system. If pvid is non-NULL, this is used as a hint
+ * as to which PV has changed state. Otherwise, all flags are recomputed from
+ * authoritative data (the s->pvs hash).
+ */
+static void update_pv_status(lvmetad_state *s, const char *pvid)
+{
+	if (pvid) {
+		const char *vgid = dm_hash_lookup(s->pvid_to_vgid, pvid);
+		assert(vgid);
+		struct config_node *vg = dm_hash_lookup(s->vgs, vgid);
+		assert(vg);
+		update_pv_status_in_vg(s, vg);
+	} else {
+		struct dm_hash_node *n = dm_hash_get_first(s->vgs);
+		while (n) {
+			struct config_node *vg = dm_hash_get_data(s->vgs, n);
+			fprintf(stderr, "[D] checking VG: %s\n",
+				find_config_str(vg, "metadata/id", "?"));
+			update_pv_status_in_vg(s, vg);
+			n = dm_hash_get_next(s->vgs, n);
+		}
+	}
+}
+
+static int update_metadata(lvmetad_state *s, const char *vgid, struct config_node *metadata)
+{
+	struct config_node *metadata_clone =
+		clone_config_node_with_mem(s->mem, metadata, 0);
+	/* TODO: seqno-based comparison with existing metadata version */
+	dm_hash_insert(s->vgs, vgid, (void*) metadata_clone);
+	fprintf(stderr, "[D] metadata stored at %p\n", metadata_clone);
+}
+
 static response pv_add(lvmetad_state *s, request r)
 {
-	const struct config_node *metadata = find_config_node(r.cft->root, "metadata");
+	struct config_node *metadata = find_config_node(r.cft->root, "metadata");
 	const char *pvid = daemon_request_str(r, "uuid", NULL);
 	fprintf(stderr, "[D] pv_add buffer: %s\n", r.buffer);
 
 	if (!pvid)
 		return daemon_reply_simple("failed", "reason = %s", "need PV UUID", NULL);
 
+	dm_hash_insert(s->pvs, pvid, 1);
+
 	if (metadata) {
 		const char *vgid = daemon_request_str(r, "metadata/id", NULL);
 		if (!vgid)
 			return daemon_reply_simple("failed", "reason = %s", "need VG UUID", NULL);
-		// TODO
-		const struct config_node *metadata_clone =
-			clone_config_node_with_mem(s->mem, metadata, 0);
-		dm_hash_insert(s->vgs, vgid, (void*) metadata_clone);
-		fprintf(stderr, "[D] metadata stored at %p\n", metadata_clone);
+
+		update_metadata(s, vgid, metadata);
 	}
 
+	update_pv_status(s, NULL);
+
 	return daemon_reply_simple("OK", NULL);
 }
 
@@ -89,11 +140,11 @@
 {
 	lvmetad_state *ls = s->private;
 
-	ls->pvids = dm_hash_create(32);
+	ls->pvs = dm_hash_create(32);
 	ls->vgs = dm_hash_create(32);
 	ls->mem = dm_pool_create("lvmetad", 1024); /* whatever */
 	fprintf(stderr, "[D] initialised state: vgs = %p\n", ls->vgs);
-	if (!ls->pvids || !ls->vgs || !ls->mem)
+	if (!ls->pvs || !ls->vgs || !ls->mem)
 		return 0;
 
 	/* if (ls->initial_registrations)


^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2012-03-23 10:34 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-20 18:24 LVM2/daemons/lvmetad lvmetad-core.c mornfall
  -- strict thread matches above, loose matches on Subject: below --
2012-03-23 10:34 zkabelac
2012-02-27 10:19 zkabelac
2012-02-27 10:10 zkabelac
2012-02-24  0:24 mornfall
2012-02-24  0:12 mornfall
2012-02-21  9:19 mornfall
2012-02-15 17:37 mornfall
2012-02-15 17:30 mornfall
2012-02-15 14:15 mornfall
2012-02-15 14:06 mornfall
2012-02-15 11:43 mornfall
2012-02-13 14:25 zkabelac
2012-01-25 21:42 zkabelac
2011-12-18 22:31 mornfall
2011-09-17 13:33 zkabelac
2011-09-02 11:04 zkabelac
2011-07-25 17:59 mornfall
2011-07-25 15:51 mornfall
2011-07-25 15:33 mornfall
2011-07-20 21:33 mornfall
2011-07-20 21:27 mornfall
2011-07-20 21:26 mornfall
2011-07-20 21:23 mornfall
2011-07-20 18:45 mornfall
2011-07-20 18:34 mornfall
2011-07-20 16:49 mornfall
2011-07-20 16:46 mornfall
2011-07-20 15:14 mornfall
2011-07-19 19:15 mornfall
2011-07-19 14:14 mornfall

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).