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)
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); } }
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)
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); }
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; }
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; }
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;
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; }
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;
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);
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);
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",
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; }
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; }
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); }
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; }
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) {
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);
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; }
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; } }
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':
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);
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();
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);
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);
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)
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; }
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);
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);
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);
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: