master - cleanup: unify NULL custom check
by Zdenek Kabelac
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=121341e52c8cb3...
Commit: 121341e52c8cb3bf14571cad0c3d90c1ec9d717d
Parent: cad3568def7f82ae58c3585a94ceb377bb155167
Author: Zdenek Kabelac <zkabelac(a)redhat.com>
AuthorDate: Tue Nov 17 13:19:39 2015 +0100
Committer: Zdenek Kabelac <zkabelac(a)redhat.com>
CommitterDate: Tue Nov 17 19:01:25 2015 +0100
cleanup: unify NULL custom check
Unify testing of NULL custom pointer.
Resolve 'factor' only in required if() branch.
---
libdm/libdm-report.c | 72 ++++++++++++++++++++++++++++---------------------
1 files changed, 41 insertions(+), 31 deletions(-)
diff --git a/libdm/libdm-report.c b/libdm/libdm-report.c
index 2457335..140a8d7 100644
--- a/libdm/libdm-report.c
+++ b/libdm/libdm-report.c
@@ -3211,7 +3211,9 @@ static const char *_tok_value(struct dm_report *rh,
break;
case DM_REPORT_FIELD_TYPE_STRING_LIST:
- str_list = (struct selection_str_list **) custom;
+ if (!(str_list = (struct selection_str_list **) custom))
+ goto_bad;
+
s = _tok_value_string_list(ft, mem, s, begin, end, str_list);
if (!(*str_list)) {
log_error("Failed to parse string list value "
@@ -3232,8 +3234,6 @@ static const char *_tok_value(struct dm_report *rh,
return NULL;
}
- factor = (uint64_t *) custom;
-
if (*s == DM_PERCENT_CHAR) {
s++;
c = DM_PERCENT_CHAR;
@@ -3244,24 +3244,29 @@ static const char *_tok_value(struct dm_report *rh,
"numeric" : "size", ft->id);
return NULL;
}
- } else if ((*factor = dm_units_to_factor(s, &c, 0, &tmp))) {
- s = tmp;
- if (expected_type != DM_REPORT_FIELD_TYPE_SIZE) {
- log_error("Found size unit specifier "
- "but %s value expected for "
- "selection field %s.",
- expected_type == DM_REPORT_FIELD_TYPE_NUMBER ?
- "numeric" : "percent", ft->id);
- return NULL;
+ } else {
+ if (!(factor = (uint64_t *) custom))
+ goto_bad;
+
+ if ((*factor = dm_units_to_factor(s, &c, 0, &tmp))) {
+ s = tmp;
+ if (expected_type != DM_REPORT_FIELD_TYPE_SIZE) {
+ log_error("Found size unit specifier "
+ "but %s value expected for "
+ "selection field %s.",
+ expected_type == DM_REPORT_FIELD_TYPE_NUMBER ?
+ "numeric" : "percent", ft->id);
+ return NULL;
+ }
+ } else if (expected_type == DM_REPORT_FIELD_TYPE_SIZE) {
+ /*
+ * If size unit is not defined in the selection
+ * and the type expected is size, use use 'm'
+ * (1 MiB) for the unit by default. This is the
+ * same behaviour as seen in lvcreate -L <size>.
+ */
+ *factor = 1024*1024;
}
- } else if (expected_type == DM_REPORT_FIELD_TYPE_SIZE) {
- /*
- * If size unit is not defined in the selection
- * and the type expected is size, use use 'm'
- * (1 MiB) for the unit by default. This is the
- * same behaviour as seen in lvcreate -L <size>.
- */
- *factor = 1024*1024;
}
*flags |= expected_type;
@@ -3273,7 +3278,9 @@ static const char *_tok_value(struct dm_report *rh,
break;
case DM_REPORT_FIELD_TYPE_TIME:
- tval = (struct time_value *) custom;
+ if (!(tval = (struct time_value *) custom))
+ goto_bad;
+
if (!(s = _tok_value_time(ft, mem, s, begin, end, tval))) {
log_error("Failed to parse time value "
"for selection field %s.", ft->id);
@@ -3290,6 +3297,10 @@ static const char *_tok_value(struct dm_report *rh,
}
return s;
+bad:
+ log_error(INTERNAL_ERROR "Forbidden NULL custom detected.");
+
+ return NULL;
}
/*
@@ -3412,7 +3423,8 @@ static struct field_selection *_create_field_selection(struct dm_report *rh,
}
if (((rvw->reserved && (rvw->reserved->type & DM_REPORT_FIELD_RESERVED_VALUE_RANGE)) ||
- (((flags & DM_REPORT_FIELD_TYPE_MASK) == DM_REPORT_FIELD_TYPE_TIME) && ((struct time_value *) custom)->range))
+ (((flags & DM_REPORT_FIELD_TYPE_MASK) == DM_REPORT_FIELD_TYPE_TIME) &&
+ custom && ((struct time_value *) custom)->range))
&&
!(fs->value->next = dm_pool_zalloc(rh->selection->mem, sizeof(struct field_selection_value)))) {
log_error(_field_selection_value_alloc_failed_msg, field_id);
@@ -3529,10 +3541,8 @@ static struct field_selection *_create_field_selection(struct dm_report *rh,
}
break;
case DM_REPORT_FIELD_TYPE_STRING_LIST:
- if (!custom) {
- log_error(INTERNAL_ERROR "Custom selection list is undefined.");
- goto error;
- }
+ if (!custom)
+ goto_bad;
fs->value->v.l = *(struct selection_str_list **)custom;
if (_check_value_is_strictly_reserved(rh, field_num, DM_REPORT_FIELD_TYPE_STRING_LIST, fs->value->v.l, NULL)) {
log_error("String list value found in selection is reserved.");
@@ -3545,11 +3555,8 @@ static struct field_selection *_create_field_selection(struct dm_report *rh,
if (rvw->reserved->type & DM_REPORT_FIELD_RESERVED_VALUE_RANGE)
fs->value->next->v.t = (((const time_t *) rvw->value)[1]);
} else {
- if (!custom) {
- log_error(INTERNAL_ERROR "Custom time value is undefined.");
- goto error;
- }
- tval = (struct time_value *) custom;
+ if (!(tval = (struct time_value *) custom))
+ goto_bad;
fs->value->v.t = tval->t1;
if (tval->range)
fs->value->next->v.t = tval->t2;
@@ -3567,8 +3574,11 @@ static struct field_selection *_create_field_selection(struct dm_report *rh,
}
return fs;
+bad:
+ log_error(INTERNAL_ERROR "Forbiden NULL custom detected.");
error:
dm_pool_free(rh->selection->mem, fs);
+
return NULL;
}
8 years, 5 months
master - raid: drop unneeded NULL test
by Zdenek Kabelac
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=cad3568def7f82...
Commit: cad3568def7f82ae58c3585a94ceb377bb155167
Parent: 51dfba002b05cc65346b539b4b030f0c2dfffeaf
Author: Zdenek Kabelac <zkabelac(a)redhat.com>
AuthorDate: Tue Nov 17 13:17:59 2015 +0100
Committer: Zdenek Kabelac <zkabelac(a)redhat.com>
CommitterDate: Tue Nov 17 19:01:25 2015 +0100
raid: drop unneeded NULL test
Skip testing target_pvs for NULL, we already
dereference it in many other places.
If check would ever be needed - it needs to be
in front of _raid_extract_images().
---
lib/metadata/raid_manip.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/lib/metadata/raid_manip.c b/lib/metadata/raid_manip.c
index 4b2cb61..b99b671 100644
--- a/lib/metadata/raid_manip.c
+++ b/lib/metadata/raid_manip.c
@@ -913,7 +913,7 @@ static int _raid_extract_images(struct logical_volume *lv, uint32_t new_count,
display_lvname(seg_lv(seg, s)));
} else {
/* Conditions for second pass */
- if (!target_pvs || !lv_is_on_pvs(seg_lv(seg, s), target_pvs) ||
+ if (!lv_is_on_pvs(seg_lv(seg, s), target_pvs) ||
!lv_is_on_pvs(seg_metalv(seg, s), target_pvs))
continue;
8 years, 5 months
master - libdm: update error message
by Zdenek Kabelac
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=51dfba002b05cc...
Commit: 51dfba002b05cc65346b539b4b030f0c2dfffeaf
Parent: 931fede81b5fdc3aecd730567fefc3e31c5275ce
Author: Zdenek Kabelac <zkabelac(a)redhat.com>
AuthorDate: Mon Nov 16 14:39:09 2015 +0100
Committer: Zdenek Kabelac <zkabelac(a)redhat.com>
CommitterDate: Tue Nov 17 19:01:25 2015 +0100
libdm: update error message
Correcting error message.
---
libdm/libdm-string.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/libdm/libdm-string.c b/libdm/libdm-string.c
index 79a969c..ef93a6b 100644
--- a/libdm/libdm-string.c
+++ b/libdm/libdm-string.c
@@ -96,7 +96,7 @@ int dm_split_lvm_name(struct dm_pool *mem, const char *dmname,
char **vgname, char **lvname, char **layer)
{
if (!vgname || !lvname || !layer) {
- log_error(INTERNAL_ERROR "dm_split_lvm_name: vgname, lvname nor layer could be NULL.");
+ log_error(INTERNAL_ERROR "dm_split_lvm_name: Forbidden NULL parameter detected.");
return 0;
}
8 years, 5 months
master - hash: change name of new lookup function
by David Teigland
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=931fede81b5fdc...
Commit: 931fede81b5fdc3aecd730567fefc3e31c5275ce
Parent: 4d37db123d4ea2735c538bf9ad8a57be051ac9c3
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Tue Nov 17 11:59:44 2015 -0600
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Tue Nov 17 11:59:44 2015 -0600
hash: change name of new lookup function
---
daemons/lvmetad/lvmetad-core.c | 6 +++---
libdm/.exported_symbols.Base | 2 +-
libdm/datastruct/hash.c | 4 ++--
libdm/libdevmapper.h | 6 +++---
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/daemons/lvmetad/lvmetad-core.c b/daemons/lvmetad/lvmetad-core.c
index 5bed562..7af9bde 100644
--- a/daemons/lvmetad/lvmetad-core.c
+++ b/daemons/lvmetad/lvmetad-core.c
@@ -1101,7 +1101,7 @@ static int _update_metadata_new_vgid(lvmetad_state *s,
goto out;
}
- if (!dm_hash_insert_multival(s->vgname_to_vgid, arg_name, new_vgid_dup, strlen(new_vgid_dup) + 1)) {
+ if (!dm_hash_insert_allow_multiple(s->vgname_to_vgid, arg_name, new_vgid_dup, strlen(new_vgid_dup) + 1)) {
ERROR(s, "update_metadata_new_vgid out of memory for vgid hash insert for %s %s", arg_name, new_vgid);
abort_daemon = 1;
goto out;
@@ -1215,7 +1215,7 @@ static int _update_metadata_new_name(lvmetad_state *s,
goto out;
}
- if (!dm_hash_insert_multival(s->vgname_to_vgid, new_name, arg_vgid_dup, strlen(arg_vgid_dup) + 1)) {
+ if (!dm_hash_insert_allow_multiple(s->vgname_to_vgid, new_name, arg_vgid_dup, strlen(arg_vgid_dup) + 1)) {
ERROR(s, "update_metadata_new_name out of memory for vgid hash insert for %s %s", new_name, arg_vgid);
abort_daemon = 1;
goto out;
@@ -1284,7 +1284,7 @@ static int _update_metadata_add_new(lvmetad_state *s, const char *new_name, cons
goto out;
}
- if (!dm_hash_insert_multival(s->vgname_to_vgid, new_name, new_vgid_dup, strlen(new_vgid_dup) + 1)) {
+ if (!dm_hash_insert_allow_multiple(s->vgname_to_vgid, new_name, new_vgid_dup, strlen(new_vgid_dup) + 1)) {
ERROR(s, "update_metadata_add_new out of memory for vgid hash insert for %s %s", new_name, new_vgid);
abort_daemon = 1;
goto out;
diff --git a/libdm/.exported_symbols.Base b/libdm/.exported_symbols.Base
index 47097fc..ee7c4f2 100644
--- a/libdm/.exported_symbols.Base
+++ b/libdm/.exported_symbols.Base
@@ -95,7 +95,7 @@ dm_hash_wipe
dm_hash_lookup_with_val
dm_hash_lookup_with_count
dm_hash_remove_with_val
-dm_hash_insert_multival
+dm_hash_insert_allow_multiple
dm_is_dm_major
dm_is_empty_dir
dm_lib_exit
diff --git a/libdm/datastruct/hash.c b/libdm/datastruct/hash.c
index 2c5108d..f9b8b9c 100644
--- a/libdm/datastruct/hash.c
+++ b/libdm/datastruct/hash.c
@@ -232,8 +232,8 @@ static struct dm_hash_node **_find_str_with_val(struct dm_hash_table *t,
return NULL;
}
-int dm_hash_insert_multival(struct dm_hash_table *t, const char *key,
- const void *val, uint32_t val_len)
+int dm_hash_insert_allow_multiple(struct dm_hash_table *t, const char *key,
+ const void *val, uint32_t val_len)
{
struct dm_hash_node *n;
struct dm_hash_node *first;
diff --git a/libdm/libdevmapper.h b/libdm/libdevmapper.h
index e02a3bc..6619aa7 100644
--- a/libdm/libdevmapper.h
+++ b/libdm/libdevmapper.h
@@ -1859,7 +1859,7 @@ struct dm_hash_node *dm_hash_get_next(struct dm_hash_table *t, struct dm_hash_no
* val_len is the size of the data being inserted.
*
* If two entries with the same key exist,
- * (added using dm_hash_insert_multival), then:
+ * (added using dm_hash_insert_allow_multiple), then:
* . dm_hash_lookup() returns the first one it finds, and
* dm_hash_lookup_with_val() returns the one with a matching
* val_len/val.
@@ -1897,8 +1897,8 @@ void *dm_hash_lookup_with_val(struct dm_hash_table *t, const char *key,
const void *val, uint32_t val_len);
void dm_hash_remove_with_val(struct dm_hash_table *t, const char *key,
const void *val, uint32_t val_len);
-int dm_hash_insert_multival(struct dm_hash_table *t, const char *key,
- const void *val, uint32_t val_len);
+int dm_hash_insert_allow_multiple(struct dm_hash_table *t, const char *key,
+ const void *val, uint32_t val_len);
void *dm_hash_lookup_with_count(struct dm_hash_table *t, const char *key, int *count);
8 years, 5 months
master - lvmetad: improve error message for VGs with same name
by David Teigland
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=4d37db123d4ea2...
Commit: 4d37db123d4ea2735c538bf9ad8a57be051ac9c3
Parent: 485d2ca9454ab4725605ab9a33928164aef1bfc8
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Tue Nov 17 09:33:50 2015 -0600
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Tue Nov 17 10:35:42 2015 -0600
lvmetad: improve error message for VGs with same name
---
lib/cache/lvmetad.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/lib/cache/lvmetad.c b/lib/cache/lvmetad.c
index f8393df..8b5bcfb 100644
--- a/lib/cache/lvmetad.c
+++ b/lib/cache/lvmetad.c
@@ -316,6 +316,15 @@ static int _lvmetad_handle_reply(daemon_reply reply, const char *action, const c
return 1;
}
+ /* Multiple VGs with the same name were found. */
+ if (found && !strcmp(daemon_reply_str(reply, "response", ""), "multiple")) {
+ log_very_verbose("Request to %s %s%sin lvmetad found multiple matching objects.",
+ action, object, *object ? " " : "");
+ if (found)
+ *found = 2;
+ return 1;
+ }
+
log_error("Request to %s %s%sin lvmetad gave response %s. Reason: %s",
action, object, *object ? " " : "",
daemon_reply_str(reply, "response", "<missing>"),
@@ -527,6 +536,12 @@ struct volume_group *lvmetad_vg_lookup(struct cmd_context *cmd, const char *vgna
if (_lvmetad_handle_reply(reply, "lookup VG", diag_name, &found) && found) {
+ if ((found == 2) && vgname) {
+ log_error("Multiple VGs found with the same name: %s.", vgname);
+ log_error("See the --select option with VG UUID (vg_uuid).");
+ goto out;
+ }
+
if (!(top = dm_config_find_node(reply.cft->root, "metadata"))) {
log_error(INTERNAL_ERROR "metadata config node not found.");
goto out;
8 years, 5 months
master - lvmetad: different style for hash functions
by David Teigland
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=485d2ca9454ab4...
Commit: 485d2ca9454ab4725605ab9a33928164aef1bfc8
Parent: d42cedae5818a50edde3ddb1db4bec6a2591c920
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Tue Nov 17 10:19:04 2015 -0600
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Tue Nov 17 10:27:16 2015 -0600
lvmetad: different style for hash functions
In lookup, return a count of entries with the
same key rather than the value from a second
entry with the same key.
Using some slightly different names.
---
daemons/lvmetad/lvmetad-core.c | 36 ++++++++++++++--------------
libdm/.exported_symbols.Base | 6 ++--
libdm/datastruct/hash.c | 50 ++++++++++++++++------------------------
libdm/libdevmapper.h | 32 ++++++++++++-------------
4 files changed, 56 insertions(+), 68 deletions(-)
diff --git a/daemons/lvmetad/lvmetad-core.c b/daemons/lvmetad/lvmetad-core.c
index 8e5bc37..5bed562 100644
--- a/daemons/lvmetad/lvmetad-core.c
+++ b/daemons/lvmetad/lvmetad-core.c
@@ -722,7 +722,7 @@ static response vg_lookup(lvmetad_state *s, request r)
response res = { 0 };
const char *uuid = daemon_request_str(r, "uuid", NULL);
const char *name = daemon_request_str(r, "name", NULL);
- const char *uuid2 = NULL;
+ int count = 0;
buffer_init( &res.buffer );
@@ -736,14 +736,14 @@ static response vg_lookup(lvmetad_state *s, request r)
lock_vgid_to_metadata(s);
if (name && !uuid)
- uuid = dm_hash_lookup_multival(s->vgname_to_vgid, name, (void *)&uuid2);
+ uuid = dm_hash_lookup_with_count(s->vgname_to_vgid, name, &count);
else if (uuid && !name)
name = dm_hash_lookup(s->vgid_to_vgname, uuid);
unlock_vgid_to_metadata(s);
- if (name && uuid && uuid2) {
- DEBUGLOG(s, "vg_lookup name %s found multiple vgids %s %s",
- name, uuid, uuid2);
+ if (name && uuid && (count > 1)) {
+ DEBUGLOG(s, "vg_lookup name %s vgid %s found %d vgids",
+ name, uuid, count);
return daemon_reply_simple("multiple", "reason = %s", "Multiple VGs found with same name", NULL);
}
@@ -752,7 +752,7 @@ static response vg_lookup(lvmetad_state *s, request r)
} else {
char *name_lookup = dm_hash_lookup(s->vgid_to_vgname, uuid);
- char *uuid_lookup = dm_hash_lookup_withval(s->vgname_to_vgid, name, uuid, strlen(uuid) + 1);
+ char *uuid_lookup = dm_hash_lookup_with_val(s->vgname_to_vgid, name, uuid, strlen(uuid) + 1);
/* FIXME: comment out these sanity checks when not testing */
@@ -923,7 +923,7 @@ static int remove_metadata(lvmetad_state *s, const char *vgid, int update_pvids)
name_lookup = dm_hash_lookup(s->vgid_to_vgname, vgid);
outdated_pvs_lookup = dm_hash_lookup(s->vgid_to_outdated_pvs, vgid);
if (name_lookup)
- vgid_lookup = dm_hash_lookup_withval(s->vgname_to_vgid, name_lookup, vgid, strlen(vgid) + 1);
+ vgid_lookup = dm_hash_lookup_with_val(s->vgname_to_vgid, name_lookup, vgid, strlen(vgid) + 1);
/* remove hash table mappings */
@@ -932,7 +932,7 @@ static int remove_metadata(lvmetad_state *s, const char *vgid, int update_pvids)
dm_hash_remove(s->vgid_to_vgname, vgid);
dm_hash_remove(s->vgid_to_outdated_pvs, vgid);
if (name_lookup)
- dm_hash_remove_withval(s->vgname_to_vgid, name_lookup, vgid, strlen(vgid) + 1);
+ dm_hash_remove_with_val(s->vgname_to_vgid, name_lookup, vgid, strlen(vgid) + 1);
unlock_vgid_to_metadata(s);
@@ -1012,8 +1012,8 @@ static void _purge_metadata(lvmetad_state *s, const char *arg_name, const char *
lock_pvid_to_vgid(s);
remove_metadata(s, arg_vgid, 1);
- if ((rem_vgid = dm_hash_lookup_withval(s->vgname_to_vgid, arg_name, arg_vgid, strlen(arg_vgid) + 1))) {
- dm_hash_remove_withval(s->vgname_to_vgid, arg_name, arg_vgid, strlen(arg_vgid) + 1);
+ if ((rem_vgid = dm_hash_lookup_with_val(s->vgname_to_vgid, arg_name, arg_vgid, strlen(arg_vgid) + 1))) {
+ dm_hash_remove_with_val(s->vgname_to_vgid, arg_name, arg_vgid, strlen(arg_vgid) + 1);
dm_free(rem_vgid);
}
unlock_pvid_to_vgid(s);
@@ -1080,7 +1080,7 @@ static int _update_metadata_new_vgid(lvmetad_state *s,
dm_config_destroy(old_meta);
old_meta = NULL;
- dm_hash_remove_withval(s->vgname_to_vgid, arg_name, old_vgid, strlen(old_vgid) + 1);
+ dm_hash_remove_with_val(s->vgname_to_vgid, arg_name, old_vgid, strlen(old_vgid) + 1);
dm_hash_remove(s->vgid_to_vgname, old_vgid);
dm_free((char *)old_vgid);
old_vgid = NULL;
@@ -1195,7 +1195,7 @@ static int _update_metadata_new_name(lvmetad_state *s,
old_meta = NULL;
dm_hash_remove(s->vgid_to_vgname, arg_vgid);
- dm_hash_remove_withval(s->vgname_to_vgid, old_name, arg_vgid, strlen(arg_vgid) + 1);
+ dm_hash_remove_with_val(s->vgname_to_vgid, old_name, arg_vgid, strlen(arg_vgid) + 1);
dm_free((char *)old_name);
old_name = NULL;
@@ -1345,13 +1345,13 @@ static int _update_metadata(lvmetad_state *s, const char *arg_name, const char *
const char *new_name = NULL;
const char *old_vgid = NULL;
const char *new_vgid = NULL;
- const char *old_vgid2 = NULL;
const char *new_metadata_vgid;
int old_seq = -1;
int new_seq = -1;
int needs_repair = 0;
int abort_daemon = 0;
int retval = 0;
+ int count = 0;
if (!arg_vgid || !arg_name) {
ERROR(s, "update_metadata missing args arg_vgid %s arg_name %s pvid %s",
@@ -1372,7 +1372,7 @@ static int _update_metadata(lvmetad_state *s, const char *arg_name, const char *
lock_vgid_to_metadata(s);
arg_name_lookup = dm_hash_lookup(s->vgid_to_vgname, arg_vgid);
- arg_vgid_lookup = dm_hash_lookup_withval(s->vgname_to_vgid, arg_name, arg_vgid, strlen(arg_vgid) + 1);
+ arg_vgid_lookup = dm_hash_lookup_with_val(s->vgname_to_vgid, arg_name, arg_vgid, strlen(arg_vgid) + 1);
/*
* A new VG when there is no existing record of the name or vgid args.
@@ -1404,7 +1404,7 @@ static int _update_metadata(lvmetad_state *s, const char *arg_name, const char *
}
new_vgid = arg_vgid;
- old_vgid = dm_hash_lookup_multival(s->vgname_to_vgid, arg_name, (void *)&old_vgid2);
+ old_vgid = dm_hash_lookup_with_count(s->vgname_to_vgid, arg_name, &count);
/*
* FIXME: this ensures that arg_name maps to only one existing
@@ -1415,9 +1415,9 @@ static int _update_metadata(lvmetad_state *s, const char *arg_name, const char *
* But as it is now, the vgid arg to this function is the new
* vgid and the existing VG is specified only by name.
*/
- if (old_vgid && old_vgid2) {
- ERROR(s, "update_metadata arg_vgid %s arg_name %s found two vgids for name %s %s",
- arg_vgid, arg_name, old_vgid, old_vgid2);
+ if (old_vgid && (count > 1)) {
+ ERROR(s, "update_metadata arg_vgid %s arg_name %s found %d vgids for name",
+ arg_vgid, arg_name, count);
old_vgid = NULL;
}
diff --git a/libdm/.exported_symbols.Base b/libdm/.exported_symbols.Base
index 61df67c..47097fc 100644
--- a/libdm/.exported_symbols.Base
+++ b/libdm/.exported_symbols.Base
@@ -92,9 +92,9 @@ dm_hash_lookup_binary
dm_hash_remove
dm_hash_remove_binary
dm_hash_wipe
-dm_hash_lookup_withval
-dm_hash_lookup_multival
-dm_hash_remove_withval
+dm_hash_lookup_with_val
+dm_hash_lookup_with_count
+dm_hash_remove_with_val
dm_hash_insert_multival
dm_is_dm_major
dm_is_empty_dir
diff --git a/libdm/datastruct/hash.c b/libdm/datastruct/hash.c
index 406f5fd..2c5108d 100644
--- a/libdm/datastruct/hash.c
+++ b/libdm/datastruct/hash.c
@@ -209,9 +209,9 @@ void dm_hash_remove(struct dm_hash_table *t, const char *key)
dm_hash_remove_binary(t, key, strlen(key) + 1);
}
-static struct dm_hash_node **_find_str_withval(struct dm_hash_table *t,
- const void *key, const void *val,
- uint32_t len, uint32_t val_len)
+static struct dm_hash_node **_find_str_with_val(struct dm_hash_table *t,
+ const void *key, const void *val,
+ uint32_t len, uint32_t val_len)
{
struct dm_hash_node **c;
unsigned h;
@@ -265,12 +265,12 @@ int dm_hash_insert_multival(struct dm_hash_table *t, const char *key,
* Look through multiple entries with the same key for one that has a
* matching val and return that. If none have maching val, return NULL.
*/
-void *dm_hash_lookup_withval(struct dm_hash_table *t, const char *key,
- const void *val, uint32_t val_len)
+void *dm_hash_lookup_with_val(struct dm_hash_table *t, const char *key,
+ const void *val, uint32_t val_len)
{
struct dm_hash_node **c;
- c = _find_str_withval(t, key, val, strlen(key) + 1, val_len);
+ c = _find_str_with_val(t, key, val, strlen(key) + 1, val_len);
return (c && *c) ? (*c)->data : 0;
}
@@ -279,12 +279,12 @@ void *dm_hash_lookup_withval(struct dm_hash_table *t, const char *key,
* Look through multiple entries with the same key for one that has a
* matching val and remove that.
*/
-void dm_hash_remove_withval(struct dm_hash_table *t, const char *key,
- const void *val, uint32_t val_len)
+void dm_hash_remove_with_val(struct dm_hash_table *t, const char *key,
+ const void *val, uint32_t val_len)
{
struct dm_hash_node **c;
- c = _find_str_withval(t, key, val, strlen(key) + 1, val_len);
+ c = _find_str_with_val(t, key, val, strlen(key) + 1, val_len);
if (c && *c) {
struct dm_hash_node *old = *c;
@@ -295,28 +295,26 @@ void dm_hash_remove_withval(struct dm_hash_table *t, const char *key,
}
/*
- * Look for multiple entries with the same key.
+ * Look up the value for a key and count how many
+ * entries have the same key.
*
- * If no entries have key, return NULL.
+ * If no entries have key, return NULL and set count to 0.
*
* If one entry has the key, the function returns the val,
- * and sets val2 to NULL.
- *
- * If two entries have the key, the function returns the val
- * from the first entry, and the val2 arg is set to the val
- * from the second entry.
+ * and sets count to 1.
*
- * If more than two entries have the key, the function looks
- * at only the first two.
+ * If N entries have the key, the function returns the val
+ * from the first entry, and sets count to N.
*/
-void *dm_hash_lookup_multival(struct dm_hash_table *t, const char *key, const void **val2)
+void *dm_hash_lookup_with_count(struct dm_hash_table *t, const char *key, int *count)
{
struct dm_hash_node **c;
struct dm_hash_node **c1 = NULL;
- struct dm_hash_node **c2 = NULL;
uint32_t len = strlen(key) + 1;
unsigned h;
+ *count = 0;
+
h = _hash(key, len) & (t->num_slots - 1);
for (c = &t->slots[h]; *c; c = &((*c)->next)) {
@@ -324,20 +322,12 @@ void *dm_hash_lookup_multival(struct dm_hash_table *t, const char *key, const vo
continue;
if (!memcmp(key, (*c)->key, len)) {
- if (!c1) {
+ (*count)++;
+ if (!c1)
c1 = c;
- } else if (!c2) {
- c2 = c;
- break;
- }
}
}
- if (!c2)
- *val2 = NULL;
- else
- *val2 = (*c2)->data;
-
if (!c1)
return NULL;
else
diff --git a/libdm/libdevmapper.h b/libdm/libdevmapper.h
index 2c3900b..e02a3bc 100644
--- a/libdm/libdevmapper.h
+++ b/libdm/libdevmapper.h
@@ -1854,27 +1854,27 @@ struct dm_hash_node *dm_hash_get_next(struct dm_hash_table *t, struct dm_hash_no
* entry with a matching key if one exists. Otherwise
* it adds a new entry.
*
- * dm_hash_insert_withval() inserts a new entry if
+ * dm_hash_insert_with_val() inserts a new entry if
* another entry with the same key already exists.
* val_len is the size of the data being inserted.
*
* If two entries with the same key exist,
* (added using dm_hash_insert_multival), then:
* . dm_hash_lookup() returns the first one it finds, and
- * dm_hash_lookup_withval() returns the one with a matching
+ * dm_hash_lookup_with_val() returns the one with a matching
* val_len/val.
* . dm_hash_remove() removes the first one it finds, and
- * dm_hash_remove_withval() removes the one with a matching
+ * dm_hash_remove_with_val() removes the one with a matching
* val_len/val.
*
* If a single entry with a given key exists, and it has
* zero val_len, then:
* . dm_hash_lookup() returns it
- * . dm_hash_lookup_withval(val_len=0) returns it
+ * . dm_hash_lookup_with_val(val_len=0) returns it
* . dm_hash_remove() removes it
- * . dm_hash_remove_withval(val_len=0) removes it
+ * . dm_hash_remove_with_val(val_len=0) removes it
*
- * dm_hash_lookup_multival() is a single call that will
+ * dm_hash_lookup_with_count() is a single call that will
* both lookup a key's value and check if there is more
* than one entry with the given key.
*
@@ -1884,24 +1884,22 @@ struct dm_hash_node *dm_hash_get_next(struct dm_hash_table *t, struct dm_hash_no
* both look up the value and indicate if multiple values
* exist for the key.)
*
- * dm_hash_lookup_multival:
+ * dm_hash_lookup_with_count:
* . If no entries exist, the function returns NULL, and
- * the val2 arg is set to NULL.
+ * the count is set to 0.
* . If only one entry exists, the value of that entry is
- * returned and the val2 arg is set to NULL.
- * . If more than one entry exists, the value of one entry
- * is returned and the val2 arg is set to the value of a
- * second entry. Testing that the val2 arg is not NULL
- * indicates that more than one entry exists.
+ * returned and count is set to 1.
+ * . If N entries exists, the value of the first entry is
+ * returned and count is set to N.
*/
-void *dm_hash_lookup_withval(struct dm_hash_table *t, const char *key,
+void *dm_hash_lookup_with_val(struct dm_hash_table *t, const char *key,
+ const void *val, uint32_t val_len);
+void dm_hash_remove_with_val(struct dm_hash_table *t, const char *key,
const void *val, uint32_t val_len);
-void dm_hash_remove_withval(struct dm_hash_table *t, const char *key,
- const void *val, uint32_t val_len);
int dm_hash_insert_multival(struct dm_hash_table *t, const char *key,
const void *val, uint32_t val_len);
-void *dm_hash_lookup_multival(struct dm_hash_table *t, const char *key, const void **val2);
+void *dm_hash_lookup_with_count(struct dm_hash_table *t, const char *key, int *count);
#define dm_hash_iterate(v, h) \
8 years, 5 months
master - lvmetad: add error explaining duplicate VG names
by David Teigland
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=d42cedae5818a5...
Commit: d42cedae5818a50edde3ddb1db4bec6a2591c920
Parent: 7d1c9e1d5a11e7e06c52036c380dd51a2c761f66
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Mon Nov 16 16:23:28 2015 -0600
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Mon Nov 16 16:25:49 2015 -0600
lvmetad: add error explaining duplicate VG names
When a VG name lookup fails because the name matches
multiple VGs, return an explanatory error.
---
daemons/lvmetad/lvmetad-core.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/daemons/lvmetad/lvmetad-core.c b/daemons/lvmetad/lvmetad-core.c
index 2ceaa02..8e5bc37 100644
--- a/daemons/lvmetad/lvmetad-core.c
+++ b/daemons/lvmetad/lvmetad-core.c
@@ -741,8 +741,11 @@ static response vg_lookup(lvmetad_state *s, request r)
name = dm_hash_lookup(s->vgid_to_vgname, uuid);
unlock_vgid_to_metadata(s);
- if (name && uuid && uuid2)
- return reply_unknown("Multiple VGs found with same name");
+ if (name && uuid && uuid2) {
+ DEBUGLOG(s, "vg_lookup name %s found multiple vgids %s %s",
+ name, uuid, uuid2);
+ return daemon_reply_simple("multiple", "reason = %s", "Multiple VGs found with same name", NULL);
+ }
if (!uuid || !name)
return reply_unknown("VG not found");
8 years, 5 months
master - lvmetad: fix some error logging
by David Teigland
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=7d1c9e1d5a11e7...
Commit: 7d1c9e1d5a11e7e06c52036c380dd51a2c761f66
Parent: 68c386cce71f5bf876cc9ed754d901f1ca9daeed
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Mon Nov 16 15:08:52 2015 -0600
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Mon Nov 16 15:13:26 2015 -0600
lvmetad: fix some error logging
For some errors old_meta may not be set, so
check before logging it.
---
daemons/lvmetad/lvmetad-core.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/daemons/lvmetad/lvmetad-core.c b/daemons/lvmetad/lvmetad-core.c
index 623e1df..2ceaa02 100644
--- a/daemons/lvmetad/lvmetad-core.c
+++ b/daemons/lvmetad/lvmetad-core.c
@@ -1603,7 +1603,8 @@ static int _update_metadata(lvmetad_state *s, const char *arg_name, const char *
if (pvid && needs_repair) {
ERROR(s, "update_metadata ignore inconsistent metadata on PV %s seqno %d for %s %s seqno %d",
pvid, new_seq, arg_vgid, arg_name, old_seq);
- DEBUGLOG_cft(s, "OLD: ", old_meta->root);
+ if (old_meta)
+ DEBUGLOG_cft(s, "OLD: ", old_meta->root);
DEBUGLOG_cft(s, "NEW: ", new_metadata);
retval = 0;
goto out;
@@ -1615,7 +1616,8 @@ static int _update_metadata(lvmetad_state *s, const char *arg_name, const char *
if (!pvid && needs_repair) {
ERROR(s, "update_metadata inconsistent with cache for vgid %s and name %s",
arg_vgid, arg_name);
- DEBUGLOG_cft(s, "OLD: ", old_meta->root);
+ if (old_meta)
+ DEBUGLOG_cft(s, "OLD: ", old_meta->root);
DEBUGLOG_cft(s, "NEW: ", new_metadata);
abort_daemon = 1;
retval = 0;
8 years, 5 months
master - lvmetad: use lookup_withval in another location
by David Teigland
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=68c386cce71f5b...
Commit: 68c386cce71f5bf876cc9ed754d901f1ca9daeed
Parent: 4a984cabc475160cf54274f0c822259a67bd8d1f
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Mon Nov 16 14:44:32 2015 -0600
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Mon Nov 16 15:13:22 2015 -0600
lvmetad: use lookup_withval in another location
Simply use lookup_withval right away rather than doing a
standard lookup, checking for the wrong mapping, then
repeating with lookup_withval to get the right mapping.
---
daemons/lvmetad/lvmetad-core.c | 43 +---------------------------------------
1 files changed, 1 insertions(+), 42 deletions(-)
diff --git a/daemons/lvmetad/lvmetad-core.c b/daemons/lvmetad/lvmetad-core.c
index 08c4632..623e1df 100644
--- a/daemons/lvmetad/lvmetad-core.c
+++ b/daemons/lvmetad/lvmetad-core.c
@@ -1342,7 +1342,6 @@ static int _update_metadata(lvmetad_state *s, const char *arg_name, const char *
const char *new_name = NULL;
const char *old_vgid = NULL;
const char *new_vgid = NULL;
- const char *arg_vgid2 = NULL;
const char *old_vgid2 = NULL;
const char *new_metadata_vgid;
int old_seq = -1;
@@ -1370,47 +1369,7 @@ static int _update_metadata(lvmetad_state *s, const char *arg_name, const char *
lock_vgid_to_metadata(s);
arg_name_lookup = dm_hash_lookup(s->vgid_to_vgname, arg_vgid);
- arg_vgid_lookup = dm_hash_lookup(s->vgname_to_vgid, arg_name);
-
- /*
- * A new PV has been found with a VG that:
- * has a vgid we don't know about (null arg_name_lookup),
- * has a name we do know about (non-null arg_vgid_lookup).
- * This happens when there are two different VGs with the
- * same name.
- */
- if (pvid && !arg_name_lookup && arg_vgid_lookup &&
- strcmp(arg_vgid_lookup, arg_vgid)) {
- if ((arg_vgid2 = dm_hash_lookup_withval(s->vgname_to_vgid, arg_name, arg_vgid, strlen(arg_vgid) + 1))) {
- /* This VG already exists in the cache. */
- DEBUGLOG(s, "update_metadata arg_vgid %s arg_name %s found VG with same name as %s",
- arg_vgid, arg_name, arg_vgid_lookup);
- arg_vgid_lookup = arg_vgid2;
- } else {
- /* This VG doesn't exist in cache yet. */
- DEBUGLOG(s, "update_metadata arg_vgid %s arg_name %s found VG with same name as %s",
- arg_vgid, arg_name, arg_vgid_lookup);
- arg_vgid_lookup = NULL;
- }
- }
-
- /*
- * Updated VG metadata has been sent from a command
- * for a VG but we have two VGs with this same name,
- * so we need to figure out which of the VGs it is.
- */
- if (!pvid && arg_name_lookup && arg_vgid_lookup &&
- !strcmp(arg_name_lookup, arg_name) &&
- strcmp(arg_vgid_lookup, arg_vgid)) {
- if ((arg_vgid2 = dm_hash_lookup_withval(s->vgname_to_vgid, arg_name, arg_vgid, strlen(arg_vgid) + 1))) {
- /* The first lookup found the another VG with the same name. */
- DEBUGLOG(s, "update_metadata arg_vgid %s arg_name %s update VG with same name as %s",
- arg_vgid, arg_name, arg_vgid_lookup);
- arg_vgid_lookup = arg_vgid2;
- } else {
- /* This case is detected as an error below. */
- }
- }
+ arg_vgid_lookup = dm_hash_lookup_withval(s->vgname_to_vgid, arg_name, arg_vgid, strlen(arg_vgid) + 1);
/*
* A new VG when there is no existing record of the name or vgid args.
8 years, 5 months
master - lvmetad: remove unneeded FIXME
by David Teigland
Gitweb: http://git.fedorahosted.org/git/?p=lvm2.git;a=commitdiff;h=4a984cabc47516...
Commit: 4a984cabc475160cf54274f0c822259a67bd8d1f
Parent: 920a28199411445f6489912efd5b8bfca00dc588
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Mon Nov 16 13:44:57 2015 -0600
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Mon Nov 16 13:44:57 2015 -0600
lvmetad: remove unneeded FIXME
This case is now handled correctly.
---
daemons/lvmetad/lvmetad-core.c | 22 +++++-----------------
1 files changed, 5 insertions(+), 17 deletions(-)
diff --git a/daemons/lvmetad/lvmetad-core.c b/daemons/lvmetad/lvmetad-core.c
index 0fddd7c..08c4632 100644
--- a/daemons/lvmetad/lvmetad-core.c
+++ b/daemons/lvmetad/lvmetad-core.c
@@ -1542,23 +1542,11 @@ static int _update_metadata(lvmetad_state *s, const char *arg_name, const char *
}
if (strcmp(arg_vgid_lookup, arg_vgid)) {
- /*
- * This shouldn't usually happen, but could when
- * disks are moved (or filters are changed?)
- *
- * FIXME: two different VGs with different uuids, on
- * different PVs could have the same name, e.g. disks
- * are moved, filters are changed. Possible solution:
- * purge all info for these VGs and return an error
- * that causes the command to go to disk. Would this
- * only happen from pv_found?
- * Until there's a proper solution, ignore this VG.
- */
- ERROR(s, "update_metadata ignore VG with duplicate name %s vgid %s pvid %s existing vgid %s",
- arg_name, arg_vgid, pvid ?: "none", arg_vgid_lookup);
- unlock_vgid_to_metadata(s);
- retval = 0;
- goto out;
+ /* This shouldn't happen. Two VGs with the same name is handled above. */
+ ERROR(s, "update_metadata arg_vgid %s arg_name %s mismatch arg_vgid_lookup %s",
+ arg_vgid, arg_name, arg_vgid_lookup);
+ needs_repair = 1;
+ goto update;
}
/* old_vgid == arg_vgid, and old_name == arg_name */
8 years, 5 months