WHATS_NEW | 7 ++++
configure.in | 6 ++--
daemons/lvmetad/lvmetad-core.c | 25 +++++++++----
lib/cache/lvmetad.c | 2 --
lib/metadata/metadata.c | 76 ++++++++++++++++++++++++++--------------
lib/metadata/metadata.h | 3 --
libdaemon/client/daemon-io.c | 2 +-
libdaemon/server/daemon-server.c | 6 ++--
libdm/Makefile.in | 2 +-
libdm/mm/pool-fast.c | 4 +++
libdm/mm/pool.c | 9 +++--
test/Makefile.in | 3 +-
test/shell/lvmcache-exercise.sh | 17 +++++++--
test/shell/vgrename-usage.sh | 15 ++++++++
tools/vgrename.c | 2 ++
15 files changed, 129 insertions(+), 50 deletions(-)
diff --git a/WHATS_NEW b/WHATS_NEW
index cd55d54..e49a98d 100644
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,12 @@
Version 2.02.104 -
===================================
+ Fix possible race during daemon worker thread creation (lvmetad).
+ Fix possible deadlock while clearing lvmetad cache for full rescan.
+ Fix possible race while creating/destroying memory pools.
+ Fix failing metadata repair when lvmetad is used.
+ Fix incorrect memory handling when reading messages from lvmetad.
+ Fix locking in lvmetad when handling the PV which is gone.
+ Run full scan before vgrename operation to avoid any cache name collision.
Recognize new flag to skip udev scanning in udev rules and act appropriately.
Add support for flagging an LV to skip udev scanning during activation.
Improve message when unable to change discards setting on active thin pool.
diff --git a/configure.in b/configure.in
index 611ab37..3bd2439 100644
--- a/configure.in
+++ b/configure.in
@@ -1181,10 +1181,8 @@ Features cannot be 'shared' when building statically
fi
################################################################################
-if [[ "$DMEVENTD" = yes -o "$CLVMD" != none ]] ; then
- AC_CHECK_LIB([pthread], [pthread_mutex_lock],
- [PTHREAD_LIBS="-lpthread"], hard_bailout)
-fi
+AC_CHECK_LIB([pthread], [pthread_mutex_lock],
+ [PTHREAD_LIBS="-lpthread"], hard_bailout)
################################################################################
dnl -- Disable selinux
diff --git a/daemons/lvmetad/lvmetad-core.c b/daemons/lvmetad/lvmetad-core.c
index e1ec5a8..285c8cc 100644
--- a/daemons/lvmetad/lvmetad-core.c
+++ b/daemons/lvmetad/lvmetad-core.c
@@ -551,7 +551,7 @@ static int compare_config(struct dm_config_node *a, struct dm_config_node *b)
return result;
}
-static int vg_remove_if_missing(lvmetad_state *s, const char *vgid);
+static int vg_remove_if_missing(lvmetad_state *s, const char *vgid, int update_pvids);
/* 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,
@@ -590,7 +590,7 @@ static int update_pvid_to_vgid(lvmetad_state *s, struct dm_config_tree *vg,
n = dm_hash_get_next(to_check, n)) {
check_vgid = dm_hash_get_key(to_check, n);
lock_vg(s, check_vgid);
- vg_remove_if_missing(s, check_vgid);
+ vg_remove_if_missing(s, check_vgid, 0);
unlock_vg(s, check_vgid);
}
@@ -631,7 +631,7 @@ static int remove_metadata(lvmetad_state *s, const char *vgid, int update_pvids)
}
/* The VG must be locked. */
-static int vg_remove_if_missing(lvmetad_state *s, const char *vgid)
+static int vg_remove_if_missing(lvmetad_state *s, const char *vgid, int update_pvids)
{
struct dm_config_tree *vg;
struct dm_config_node *pv;
@@ -658,7 +658,7 @@ static int vg_remove_if_missing(lvmetad_state *s, const char *vgid)
if (missing) {
DEBUGLOG(s, "removing empty VG %s", vgid);
- remove_metadata(s, vgid, 0);
+ remove_metadata(s, vgid, update_pvids);
}
unlock_pvid_to_pvmeta(s);
@@ -796,11 +796,24 @@ static response pv_gone(lvmetad_state *s, request r)
pvmeta = dm_hash_lookup(s->pvid_to_pvmeta, pvid);
pvid_old = dm_hash_lookup_binary(s->device_to_pvid, &device, sizeof(device));
+ char *vgid = dm_hash_lookup(s->pvid_to_vgid, pvid);
+
+ if (vgid && !(vgid = dm_strdup(vgid))) {
+ unlock_pvid_to_pvmeta(s);
+ return reply_fail("out of memory");
+ }
+
dm_hash_remove_binary(s->device_to_pvid, &device, sizeof(device));
dm_hash_remove(s->pvid_to_pvmeta, pvid);
- vg_remove_if_missing(s, dm_hash_lookup(s->pvid_to_vgid, pvid));
unlock_pvid_to_pvmeta(s);
+ if (vgid) {
+ lock_vg(s, vgid);
+ vg_remove_if_missing(s, vgid, 1);
+ unlock_vg(s, vgid);
+ dm_free(vgid);
+ }
+
if (pvid_old)
dm_free(pvid_old);
@@ -816,8 +829,8 @@ static response pv_clear_all(lvmetad_state *s, request r)
DEBUGLOG(s, "pv_clear_all");
lock_pvid_to_pvmeta(s);
- lock_vgid_to_metadata(s);
lock_pvid_to_vgid(s);
+ lock_vgid_to_metadata(s);
destroy_metadata_hashes(s);
create_metadata_hashes(s);
diff --git a/lib/cache/lvmetad.c b/lib/cache/lvmetad.c
index f43283f..8940704 100644
--- a/lib/cache/lvmetad.c
+++ b/lib/cache/lvmetad.c
@@ -392,8 +392,6 @@ struct volume_group *lvmetad_vg_lookup(struct cmd_context *cmd, const char *vgna
pvl->pv->dev = lvmcache_device(info);
if (!pvl->pv->dev)
pvl->pv->status |= MISSING_PV;
- else
- check_reappeared_pv(vg, pvl->pv);
if (!lvmcache_fid_add_mdas_pv(info, fid)) {
vg = NULL;
goto_out; /* FIXME error path */
diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 4ffd502..8571e0a 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -2883,10 +2883,11 @@ int vg_missing_pv_count(const struct volume_group *vg)
return ret;
}
-void check_reappeared_pv(struct volume_group *correct_vg,
- struct physical_volume *pv)
+static int _check_reappeared_pv(struct volume_group *correct_vg,
+ struct physical_volume *pv, int act)
{
struct pv_list *pvl;
+ int rv = 0;
/*
* Skip these checks in case the tool is going to deal with missing
@@ -2894,21 +2895,47 @@ void check_reappeared_pv(struct volume_group *correct_vg,
* confusing.
*/
if (correct_vg->cmd->handles_missing_pvs)
- return;
+ return rv;
dm_list_iterate_items(pvl, &correct_vg->pvs)
if (pv->dev == pvl->pv->dev && is_missing_pv(pvl->pv)) {
- log_warn("Missing device %s reappeared, updating "
- "metadata for VG %s to version %u.",
- pv_dev_name(pvl->pv), pv_vg_name(pvl->pv),
- correct_vg->seqno);
+ if (act)
+ log_warn("Missing device %s reappeared, updating "
+ "metadata for VG %s to version %u.",
+ pv_dev_name(pvl->pv), pv_vg_name(pvl->pv),
+ correct_vg->seqno);
if (pvl->pv->pe_alloc_count == 0) {
- pv->status &= ~MISSING_PV;
- pvl->pv->status &= ~MISSING_PV;
- } else
+ if (act) {
+ pv->status &= ~MISSING_PV;
+ pvl->pv->status &= ~MISSING_PV;
+ }
+ ++ rv;
+ } else if (act)
log_warn("Device still marked missing because of allocated data "
"on it, remove volumes and consider vgreduce --removemissing.");
}
+ return rv;
+}
+
+static int _repair_inconsistent_vg(struct volume_group *vg)
+{
+ unsigned saved_handles_missing_pvs = vg->cmd->handles_missing_pvs;
+
+ vg->cmd->handles_missing_pvs = 1;
+ if (!vg_write(vg)) {
+ log_error("Automatic metadata correction failed");
+ vg->cmd->handles_missing_pvs = saved_handles_missing_pvs;
+ return 0;
+ }
+
+ vg->cmd->handles_missing_pvs = saved_handles_missing_pvs;
+
+ if (!vg_commit(vg)) {
+ log_error("Automatic metadata correction commit failed");
+ return 0;
+ }
+
+ return 1;
}
static int _check_mda_in_use(struct metadata_area *mda, void *_in_use)
@@ -2951,12 +2978,12 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
int inconsistent_mdas = 0;
int inconsistent_mda_count = 0;
unsigned use_precommitted = precommitted;
- unsigned saved_handles_missing_pvs = cmd->handles_missing_pvs;
struct dm_list *pvids;
struct pv_list *pvl, *pvl2;
struct dm_list all_pvs;
char uuid[64] __attribute__((aligned(8)));
unsigned seqno = 0;
+ int reappeared = 0;
if (is_orphan_vg(vgname)) {
if (use_precommitted) {
@@ -2969,8 +2996,16 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
}
if (lvmetad_active() && !use_precommitted) {
- *consistent = 1;
- return lvmcache_get_vg(cmd, vgname, vgid, precommitted);
+ if ((correct_vg = lvmcache_get_vg(cmd, vgname, vgid, precommitted))) {
+ dm_list_iterate_items(pvl, &correct_vg->pvs)
+ if (pvl->pv->dev)
+ reappeared += _check_reappeared_pv(correct_vg, pvl->pv, *consistent);
+ if (reappeared && *consistent)
+ *consistent = _repair_inconsistent_vg(correct_vg);
+ else
+ *consistent = !reappeared;
+ }
+ return correct_vg;
}
/*
@@ -3339,22 +3374,11 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
* update metadata and remove MISSING flag
*/
dm_list_iterate_items(pvl, &all_pvs)
- check_reappeared_pv(correct_vg, pvl->pv);
+ _check_reappeared_pv(correct_vg, pvl->pv, 1);
- cmd->handles_missing_pvs = 1;
- if (!vg_write(correct_vg)) {
- log_error("Automatic metadata correction failed");
+ if (!_repair_inconsistent_vg(correct_vg)) {
_free_pv_list(&all_pvs);
release_vg(correct_vg);
- cmd->handles_missing_pvs = saved_handles_missing_pvs;
- return NULL;
- }
- cmd->handles_missing_pvs = saved_handles_missing_pvs;
-
- if (!vg_commit(correct_vg)) {
- log_error("Automatic metadata correction commit "
- "failed");
- release_vg(correct_vg);
return NULL;
}
diff --git a/lib/metadata/metadata.h b/lib/metadata/metadata.h
index 2408c23..21ac204 100644
--- a/lib/metadata/metadata.h
+++ b/lib/metadata/metadata.h
@@ -486,7 +486,4 @@ int add_pv_to_vg(struct volume_group *vg, const char *pv_name,
uint64_t find_min_mda_size(struct dm_list *mdas);
char *tags_format_and_copy(struct dm_pool *mem, const struct dm_list *tags);
-void check_reappeared_pv(struct volume_group *correct_vg,
- struct physical_volume *pv);
-
#endif
diff --git a/libdaemon/client/daemon-io.c b/libdaemon/client/daemon-io.c
index 906f375..e2c5180 100644
--- a/libdaemon/client/daemon-io.c
+++ b/libdaemon/client/daemon-io.c
@@ -38,7 +38,7 @@ int buffer_read(int fd, struct buffer *buffer) {
result = read(fd, buffer->mem + buffer->used, buffer->allocated - buffer->used);
if (result > 0) {
buffer->used += result;
- if (!strncmp((buffer->mem) + buffer->used - 4, "\n##\n", 4)) {
+ if (buffer->used >= 4 && !strncmp((buffer->mem) + buffer->used - 4, "\n##\n", 4)) {
buffer->used -= 4;
buffer->mem[buffer->used] = 0;
break; /* success, we have the full message now */
diff --git a/libdaemon/server/daemon-server.c b/libdaemon/server/daemon-server.c
index df2c852..b114b9f 100644
--- a/libdaemon/server/daemon-server.c
+++ b/libdaemon/server/daemon-server.c
@@ -381,6 +381,7 @@ static void *client_thread(void *baton)
request req;
response res;
+ b->client.thread_id = pthread_self();
buffer_init(&req.buffer);
while (1) {
@@ -431,6 +432,7 @@ static int handle_connect(daemon_state s)
struct sockaddr_un sockaddr;
client_handle client = { .thread_id = 0 };
socklen_t sl = sizeof(sockaddr);
+ pthread_t tid;
client.socket_fd = accept(s.socket_fd, (struct sockaddr *) &sockaddr, &sl);
if (client.socket_fd < 0)
@@ -446,10 +448,10 @@ static int handle_connect(daemon_state s)
baton->s = s;
baton->client = client;
- if (pthread_create(&baton->client.thread_id, NULL, client_thread, baton))
+ if (pthread_create(&tid, NULL, client_thread, baton))
return 0;
- pthread_detach(baton->client.thread_id);
+ pthread_detach(tid);
return 1;
}
diff --git a/libdm/Makefile.in b/libdm/Makefile.in
index bddb0a0..2aa70d4 100644
--- a/libdm/Makefile.in
+++ b/libdm/Makefile.in
@@ -57,7 +57,7 @@ include $(top_builddir)/make.tmpl
DEFS += -DDM_DEVICE_UID=@DM_DEVICE_UID@ -DDM_DEVICE_GID=@DM_DEVICE_GID@ \
-DDM_DEVICE_MODE=@DM_DEVICE_MODE@
-LIBS += $(SELINUX_LIBS) $(UDEV_LIBS)
+LIBS += $(SELINUX_LIBS) $(UDEV_LIBS) $(PTHREAD_LIBS)
device-mapper: all
diff --git a/libdm/mm/pool-fast.c b/libdm/mm/pool-fast.c
index 2b494d6..edb31a0 100644
--- a/libdm/mm/pool-fast.c
+++ b/libdm/mm/pool-fast.c
@@ -62,7 +62,9 @@ struct dm_pool *dm_pool_create(const char *name, size_t chunk_hint)
while (new_size < p->chunk_size)
new_size <<= 1;
p->chunk_size = new_size;
+ pthread_mutex_lock(&_dm_pools_mutex);
dm_list_add(&_dm_pools, &p->list);
+ pthread_mutex_unlock(&_dm_pools_mutex);
return p;
}
@@ -77,7 +79,9 @@ void dm_pool_destroy(struct dm_pool *p)
c = pr;
}
+ pthread_mutex_lock(&_dm_pools_mutex);
dm_list_del(&p->list);
+ pthread_mutex_unlock(&_dm_pools_mutex);
dm_free(p);
}
diff --git a/libdm/mm/pool.c b/libdm/mm/pool.c
index fd08307..ef006a4 100644
--- a/libdm/mm/pool.c
+++ b/libdm/mm/pool.c
@@ -15,9 +15,10 @@
#include "dmlib.h"
#include <sys/mman.h>
+#include <pthread.h>
-/* FIXME: thread unsafe */
static DM_LIST_INIT(_dm_pools);
+static pthread_mutex_t _dm_pools_mutex = PTHREAD_MUTEX_INITIALIZER;
void dm_pools_check_leaks(void);
#ifdef DEBUG_ENFORCE_POOL_LOCKING
@@ -81,8 +82,11 @@ void dm_pools_check_leaks(void)
{
struct dm_pool *p;
- if (dm_list_empty(&_dm_pools))
+ pthread_mutex_lock(&_dm_pools_mutex);
+ if (dm_list_empty(&_dm_pools)) {
+ pthread_mutex_unlock(&_dm_pools_mutex);
return;
+ }
log_error("You have a memory leak (not released memory pool):");
dm_list_iterate_items(p, &_dm_pools) {
@@ -94,6 +98,7 @@ void dm_pools_check_leaks(void)
log_error(" [%p] %s", p, p->name);
#endif
}
+ pthread_mutex_unlock(&_dm_pools_mutex);
log_error(INTERNAL_ERROR "Unreleased memory pool(s) found.");
}
diff --git a/test/Makefile.in b/test/Makefile.in
index 1b9789f..5685544 100644
--- a/test/Makefile.in
+++ b/test/Makefile.in
@@ -34,7 +34,8 @@ T ?= .
S ?= @ # never match anything by default
VERBOSE ?= 0
ALL = $(shell find $(srcdir) \( -path \*/shell/\*.sh -or -path \*/api/\*.sh \) | sort)
-RUN = $(shell find $(srcdir) -regextype posix-egrep \( -path \*/shell/\*.sh -or -path \*/api/\*.sh \) -and -regex "$(srcdir)/.*($(T)).*" -and -not -regex "$(srcdir)/.*($(S)).*" | sort)
+comma = ,
+RUN = $(shell find $(srcdir) -regextype posix-egrep \( -path \*/shell/\*.sh -or -path \*/api/\*.sh \) -and -regex "$(srcdir)/.*($(subst $(comma),|,$(T))).*" -and -not -regex "$(srcdir)/.*($(subst $(comma),|,$(S))).*" | sort)
RUN_BASE = $(subst $(srcdir)/,,$(RUN))
# Shell quote;
diff --git a/test/shell/lvmcache-exercise.sh b/test/shell/lvmcache-exercise.sh
index b1e2b92..6e8efda 100644
--- a/test/shell/lvmcache-exercise.sh
+++ b/test/shell/lvmcache-exercise.sh
@@ -1,5 +1,5 @@
#!/bin/sh
-# Copyright (C) 2008 Red Hat, Inc. All rights reserved.
+# Copyright (C) 2008-2013 Red Hat, Inc. All rights reserved.
#
# This copyrighted material is made available to anyone wishing to use,
# modify, copy, or redistribute it subject to the terms and conditions
@@ -14,10 +14,23 @@
aux prepare_pvs 5
vgcreate $vg1 "$dev1"
-vgcreate $vg2 "$dev3"
+vgcreate $vg2 "$dev3" "$dev4" "$dev5"
aux disable_dev "$dev1"
pvscan
vgcreate $vg1 "$dev2"
aux enable_dev "$dev1"
pvs
+
+# reappearing device (rhbz 995440)
+lvcreate -aey -m2 --type mirror -l4 --alloc anywhere --corelog -n $lv1 $vg2
+
+aux disable_dev "$dev3"
+lvconvert --yes --repair $vg2/$lv1
+aux enable_dev "$dev3"
+
+# here it should fix any reappeared devices
+lvs
+
+lvs -a $vg2 -o+devices 2>&1 | tee out
+not grep reappeared out
diff --git a/test/shell/vgrename-usage.sh b/test/shell/vgrename-usage.sh
index 2b8ac5a..59576ac 100644
--- a/test/shell/vgrename-usage.sh
+++ b/test/shell/vgrename-usage.sh
@@ -38,3 +38,18 @@ vgcreate $vg1 "$dev1"
vgcreate $vg2 "$dev2"
not vgrename $vg1 $vg2
vgremove $vg1 $vg2
+
+# vgrename duplicate name
+vgcreate $vg1 "$dev1"
+aux disable_dev "$dev1"
+vgcreate $vg1 "$dev2"
+UUID=$(vgs --noheading -o vg_uuid $vg1)
+aux enable_dev "$dev1"
+
+not vgrename $vg1 $vg2
+vgrename $UUID $vg2
+not vgrename $UUID $vg1
+
+vgs
+
+vgremove $vg1 $vg2
diff --git a/tools/vgrename.c b/tools/vgrename.c
index 154a6f3..b5e778f 100644
--- a/tools/vgrename.c
+++ b/tools/vgrename.c
@@ -83,6 +83,8 @@ static int vg_rename_path(struct cmd_context *cmd, const char *old_vg_path,
if (!lvmetad_vg_list_to_lvmcache(cmd))
stack;
+ lvmcache_label_scan(cmd, 2);
+
/* Avoid duplicates */
if (!(vgids = get_vgids(cmd, 0)) || dm_list_empty(vgids)) {
log_error("No complete volume groups found");