* [PATCH 0/5] rbd checker fixes
@ 2016-10-11 21:36 Mike Christie
2016-10-11 21:36 ` [PATCH 1/5] rbd: fix sync repair support Mike Christie
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: Mike Christie @ 2016-10-11 21:36 UTC (permalink / raw)
To: dm-devel, christophe.varoqui
The following pathces made over the multipath-tools tree today
fix some bugs in the rbd checker and sync up log messages.
The first 4 patches are a resend from here:
https://www.redhat.com/archives/dm-devel/2016-August/msg00429.html
I did not see any review comments (of course let me know if I missed
them), so the only changes are from being rebased on the current tree.
The last patch is new and adds a fix for when rbd's lock_on_read
feature is used.
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/5] rbd: fix sync repair support 2016-10-11 21:36 [PATCH 0/5] rbd checker fixes Mike Christie @ 2016-10-11 21:36 ` Mike Christie 2016-10-11 21:36 ` [PATCH 2/5] rbd: check for nonshared clients Mike Christie ` (4 subsequent siblings) 5 siblings, 0 replies; 7+ messages in thread From: Mike Christie @ 2016-10-11 21:36 UTC (permalink / raw) To: dm-devel, christophe.varoqui; +Cc: Mike Christie If sync was set we were calling check instead of function passed in. Signed-off-by: Mike Christie <mchristi@redhat.com> --- libmultipath/checkers/rbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libmultipath/checkers/rbd.c b/libmultipath/checkers/rbd.c index bfec2c9..8f88154 100644 --- a/libmultipath/checkers/rbd.c +++ b/libmultipath/checkers/rbd.c @@ -552,7 +552,7 @@ static int rbd_exec_fn(struct checker *c, thread_fn *fn) int rbd_status, r; if (c->sync) - return rbd_check(ct, c->message); + return fn(ct, c->message); /* * Async mode */ -- 2.7.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/5] rbd: check for nonshared clients 2016-10-11 21:36 [PATCH 0/5] rbd checker fixes Mike Christie 2016-10-11 21:36 ` [PATCH 1/5] rbd: fix sync repair support Mike Christie @ 2016-10-11 21:36 ` Mike Christie 2016-10-11 21:36 ` [PATCH 3/5] rbd: check for exclusive lock enabled Mike Christie ` (3 subsequent siblings) 5 siblings, 0 replies; 7+ messages in thread From: Mike Christie @ 2016-10-11 21:36 UTC (permalink / raw) To: dm-devel, christophe.varoqui; +Cc: Mike Christie The rbd checker only supports nonshared clients so add a check during init time. Signed-off-by: Mike Christie <mchristi@redhat.com> --- libmultipath/checkers/rbd.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libmultipath/checkers/rbd.c b/libmultipath/checkers/rbd.c index 8f88154..f0497db 100644 --- a/libmultipath/checkers/rbd.c +++ b/libmultipath/checkers/rbd.c @@ -124,6 +124,11 @@ int libcheck_init(struct checker * c) if (!config_info) goto free_addr; + if (!strstr(config_info, "noshare")) { + condlog(3, "Only nonshared clients supported."); + goto free_addr; + } + ct->config_info = strdup(config_info); if (!ct->config_info) goto free_addr; -- 2.7.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/5] rbd: check for exclusive lock enabled 2016-10-11 21:36 [PATCH 0/5] rbd checker fixes Mike Christie 2016-10-11 21:36 ` [PATCH 1/5] rbd: fix sync repair support Mike Christie 2016-10-11 21:36 ` [PATCH 2/5] rbd: check for nonshared clients Mike Christie @ 2016-10-11 21:36 ` Mike Christie 2016-10-11 21:36 ` [PATCH 4/5] rbd: fixup log messages Mike Christie ` (2 subsequent siblings) 5 siblings, 0 replies; 7+ messages in thread From: Mike Christie @ 2016-10-11 21:36 UTC (permalink / raw) To: dm-devel, christophe.varoqui; +Cc: Mike Christie Only attach the checker if the rbd image has the exclusive lock enabled. Signed-off-by: Mike Christie <mchristi@redhat.com> --- libmultipath/checkers/rbd.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/libmultipath/checkers/rbd.c b/libmultipath/checkers/rbd.c index f0497db..89d7525 100644 --- a/libmultipath/checkers/rbd.c +++ b/libmultipath/checkers/rbd.c @@ -34,6 +34,8 @@ typedef int (thread_fn)(struct rbd_checker_context *ct, char *msg); #define RBD_MSG(msg, fmt, args...) snprintf(msg, CHECKER_MSG_LEN, fmt, ##args); +#define RBD_FEATURE_EXCLUSIVE_LOCK (1 << 2) + struct rbd_checker_context { int rbd_bus_id; char *client_addr; @@ -66,8 +68,9 @@ int libcheck_init(struct checker * c) struct udev_device *bus_dev; struct udev *udev; struct stat sb; - const char *block_name, *addr, *config_info; + const char *block_name, *addr, *config_info, *features_str; const char *image, *pool, *snap, *username; + uint64_t features = 0; char sysfs_path[PATH_SIZE]; int ret; @@ -120,6 +123,15 @@ int libcheck_init(struct checker * c) if (!ct->client_addr) goto free_dev; + features_str = udev_device_get_sysattr_value(bus_dev, "features"); + if (!features_str) + goto free_addr; + features = strtoll(features_str, NULL, 16); + if (!(features & RBD_FEATURE_EXCLUSIVE_LOCK)) { + condlog(3, "Exclusive lock not set."); + goto free_addr; + } + config_info = udev_device_get_sysattr_value(bus_dev, "config_info"); if (!config_info) goto free_addr; -- 2.7.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/5] rbd: fixup log messages 2016-10-11 21:36 [PATCH 0/5] rbd checker fixes Mike Christie ` (2 preceding siblings ...) 2016-10-11 21:36 ` [PATCH 3/5] rbd: check for exclusive lock enabled Mike Christie @ 2016-10-11 21:36 ` Mike Christie 2016-10-11 21:36 ` [PATCH 5/5] rbd: use lock_on_read if set Mike Christie 2016-10-16 7:56 ` [PATCH 0/5] rbd checker fixes Christophe Varoqui 5 siblings, 0 replies; 7+ messages in thread From: Mike Christie @ 2016-10-11 21:36 UTC (permalink / raw) To: dm-devel, christophe.varoqui; +Cc: Mike Christie Add rbd device prefix to condlog messages that was missing it, and drop it in RBD_MSG because it is already added by caller. Signed-off-by: Mike Christie <mchristi@redhat.com> --- libmultipath/checkers/rbd.c | 67 +++++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/libmultipath/checkers/rbd.c b/libmultipath/checkers/rbd.c index 89d7525..e516166 100644 --- a/libmultipath/checkers/rbd.c +++ b/libmultipath/checkers/rbd.c @@ -114,8 +114,8 @@ int libcheck_init(struct checker * c) addr = udev_device_get_sysattr_value(bus_dev, "client_addr"); if (!addr) { - condlog(0, "Could not find client_addr in rbd sysfs. Try " - "updating kernel"); + condlog(0, "rbd%d: Could not find client_addr in rbd sysfs. " + "Try updating kernel", ct->rbd_bus_id); goto free_dev; } @@ -128,7 +128,7 @@ int libcheck_init(struct checker * c) goto free_addr; features = strtoll(features_str, NULL, 16); if (!(features & RBD_FEATURE_EXCLUSIVE_LOCK)) { - condlog(3, "Exclusive lock not set."); + condlog(3, "rbd%d: Exclusive lock not set.", ct->rbd_bus_id); goto free_addr; } @@ -137,7 +137,8 @@ int libcheck_init(struct checker * c) goto free_addr; if (!strstr(config_info, "noshare")) { - condlog(3, "Only nonshared clients supported."); + condlog(3, "rbd%d: Only nonshared clients supported.", + ct->rbd_bus_id); goto free_addr; } @@ -190,18 +191,20 @@ int libcheck_init(struct checker * c) } if (rados_create(&ct->cluster, NULL) < 0) { - condlog(0, "Could not create rados cluster"); + condlog(0, "rbd%d: Could not create rados cluster", + ct->rbd_bus_id); goto free_snap; } if (rados_conf_read_file(ct->cluster, NULL) < 0) { - condlog(0, "Could not read rados conf"); + condlog(0, "rbd%d: Could not read rados conf", ct->rbd_bus_id); goto shutdown_rados; } ret = rados_connect(ct->cluster); if (ret < 0) { - condlog(0, "Could not connect to rados cluster"); + condlog(0, "rbd%d: Could not connect to rados cluster", + ct->rbd_bus_id); goto shutdown_rados; } @@ -292,8 +295,7 @@ static int rbd_is_blacklisted(struct rbd_checker_context *ct, char *msg) ret = rados_mon_command(ct->cluster, (const char **)cmd, 1, "", 0, &blklist, &blklist_len, &stat, &stat_len); if (ret < 0) { - RBD_MSG(msg, "rbd checker failed: mon command failed %d", - ret); + RBD_MSG(msg, "checker failed: mon command failed %d", ret); return ret; } @@ -314,16 +316,15 @@ static int rbd_is_blacklisted(struct rbd_checker_context *ct, char *msg) end = strchr(addr_tok, ' '); if (!end) { - RBD_MSG(msg, "rbd%d checker failed: invalid blacklist %s", - ct->rbd_bus_id, addr_tok); + RBD_MSG(msg, "checker failed: invalid blacklist %s", + addr_tok); break; } *end = '\0'; if (!strcmp(addr_tok, ct->client_addr)) { ct->blacklisted = 1; - RBD_MSG(msg, "rbd%d checker: %s is blacklisted", - ct->rbd_bus_id, ct->client_addr); + RBD_MSG(msg, "%s is blacklisted", ct->client_addr); ret = 1; break; } @@ -340,7 +341,7 @@ static int rbd_check(struct rbd_checker_context *ct, char *msg) if (ct->blacklisted || rbd_is_blacklisted(ct, msg) == 1) return PATH_DOWN; - RBD_MSG(msg, "rbd checker reports path is up"); + RBD_MSG(msg, "checker reports path is up"); /* * Path may have issues, but the ceph cluster is at least * accepting IO, so we can attempt to do IO. @@ -412,10 +413,12 @@ static int rbd_remap(struct rbd_checker_context *ct) argv[i] = NULL; ret = execvp(argv[0], argv); - condlog(0, "Error executing rbd: %s", strerror(errno)); + condlog(0, "rbd%d: Error executing rbd: %s", ct->rbd_bus_id, + strerror(errno)); exit(-1); case -1: - condlog(0, "fork failed: %s", strerror(errno)); + condlog(0, "rbd%d: fork failed: %s", ct->rbd_bus_id, + strerror(errno)); return -1; default: ret = -1; @@ -425,7 +428,8 @@ static int rbd_remap(struct rbd_checker_context *ct) if (status == 0) ret = 0; else - condlog(0, "rbd failed with %d", status); + condlog(0, "rbd%d: failed with %d", + ct->rbd_bus_id, status); } } @@ -455,12 +459,12 @@ static int rbd_rm_blacklist(struct rbd_checker_context *ct) ret = rados_mon_command(ct->cluster, (const char **)cmd, 1, "", 0, NULL, 0, &stat, &stat_len); if (ret < 0) { - condlog(1, "rbd%d repair failed to remove blacklist for %s %d", + condlog(1, "rbd%d: repair failed to remove blacklist for %s %d", ct->rbd_bus_id, ct->client_addr, ret); goto free_cmd; } - condlog(1, "rbd%d repair rm blacklist for %s", + condlog(1, "rbd%d: repair rm blacklist for %s", ct->rbd_bus_id, ct->client_addr); free(stat); free_cmd: @@ -479,8 +483,7 @@ static int rbd_repair(struct rbd_checker_context *ct, char *msg) if (!ct->remapped) { ret = rbd_remap(ct); if (ret) { - RBD_MSG(msg, "rbd%d repair failed to remap. Err %d", - ct->rbd_bus_id, ret); + RBD_MSG(msg, "repair failed to remap. Err %d", ret); return PATH_DOWN; } } @@ -489,22 +492,21 @@ static int rbd_repair(struct rbd_checker_context *ct, char *msg) snprintf(del, sizeof(del), "%d force", ct->rbd_bus_id); ret = sysfs_write_rbd_remove(del, strlen(del) + 1); if (ret) { - RBD_MSG(msg, "rbd%d repair failed to clean up. Err %d", - ct->rbd_bus_id, ret); + RBD_MSG(msg, "repair failed to clean up. Err %d", ret); return PATH_DOWN; } ret = rbd_rm_blacklist(ct); if (ret) { - RBD_MSG(msg, "rbd%d repair could not remove blacklist entry. Err %d", - ct->rbd_bus_id, ret); + RBD_MSG(msg, "repair could not remove blacklist entry. Err %d", + ret); return PATH_DOWN; } ct->remapped = 0; ct->blacklisted = 0; - RBD_MSG(msg, "rbd%d has been repaired", ct->rbd_bus_id); + RBD_MSG(msg, "has been repaired"); return PATH_UP; } @@ -529,7 +531,7 @@ static void *rbd_thread(void *ctx) struct rbd_checker_context *ct = ctx; int state; - condlog(3, "rbd%d thread starting up", ct->rbd_bus_id); + condlog(3, "rbd%d: thread starting up", ct->rbd_bus_id); ct->message[0] = '\0'; /* This thread can be canceled, so setup clean up */ @@ -548,7 +550,7 @@ static void *rbd_thread(void *ctx) pthread_cond_signal(&ct->active); pthread_mutex_unlock(&ct->lock); - condlog(3, "rbd%d thead finished, state %s", ct->rbd_bus_id, + condlog(3, "rbd%d: thead finished, state %s", ct->rbd_bus_id, checker_state_name(state)); rbd_thread_cleanup_pop(ct); return ((void *)0); @@ -575,16 +577,17 @@ static int rbd_exec_fn(struct checker *c, thread_fn *fn) */ r = pthread_mutex_lock(&ct->lock); if (r != 0) { - condlog(2, "rbd%d mutex lock failed with %d", ct->rbd_bus_id, + condlog(2, "rbd%d: mutex lock failed with %d", ct->rbd_bus_id, r); - MSG(c, "rbd%d thread failed to initialize", ct->rbd_bus_id); + MSG(c, "rbd%d: thread failed to initialize", ct->rbd_bus_id); return PATH_WILD; } if (ct->running) { /* Check if checker is still running */ if (ct->thread) { - condlog(3, "rbd%d thread not finished", ct->rbd_bus_id); + condlog(3, "rbd%d: thread not finished", + ct->rbd_bus_id); rbd_status = PATH_PENDING; } else { /* checker done */ @@ -621,7 +624,7 @@ static int rbd_exec_fn(struct checker *c, thread_fn *fn) if (ct->thread && (rbd_status == PATH_PENDING || rbd_status == PATH_UNCHECKED)) { - condlog(3, "rbd%d thread still running", + condlog(3, "rbd%d: thread still running", ct->rbd_bus_id); ct->running = 1; rbd_status = PATH_PENDING; -- 2.7.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 5/5] rbd: use lock_on_read if set 2016-10-11 21:36 [PATCH 0/5] rbd checker fixes Mike Christie ` (3 preceding siblings ...) 2016-10-11 21:36 ` [PATCH 4/5] rbd: fixup log messages Mike Christie @ 2016-10-11 21:36 ` Mike Christie 2016-10-16 7:56 ` [PATCH 0/5] rbd checker fixes Christophe Varoqui 5 siblings, 0 replies; 7+ messages in thread From: Mike Christie @ 2016-10-11 21:36 UTC (permalink / raw) To: dm-devel, christophe.varoqui; +Cc: Mike Christie If lock_on_read is set initially, pass it as a map option when remapping the image. Signed-off-by: Mike Christie <mchristi@redhat.com> --- libmultipath/checkers/rbd.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/libmultipath/checkers/rbd.c b/libmultipath/checkers/rbd.c index e516166..c920cbb 100644 --- a/libmultipath/checkers/rbd.c +++ b/libmultipath/checkers/rbd.c @@ -46,6 +46,7 @@ struct rbd_checker_context { char *username; int remapped; int blacklisted; + int lock_on_read:1; rados_t cluster; @@ -142,6 +143,9 @@ int libcheck_init(struct checker * c) goto free_addr; } + if (strstr(config_info, "lock_on_read")) + ct->lock_on_read = 1; + ct->config_info = strdup(config_info); if (!ct->config_info) goto free_addr; @@ -398,7 +402,10 @@ static int rbd_remap(struct rbd_checker_context *ct) case 0: argv[i++] = "rbd"; argv[i++] = "map"; - argv[i++] = "-o noshare"; + if (ct->lock_on_read) + argv[i++] = "-o noshare,lock_on_read"; + else + argv[i++] = "-o noshare"; if (ct->username) { argv[i++] = "--id"; argv[i++] = ct->username; -- 2.7.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 0/5] rbd checker fixes 2016-10-11 21:36 [PATCH 0/5] rbd checker fixes Mike Christie ` (4 preceding siblings ...) 2016-10-11 21:36 ` [PATCH 5/5] rbd: use lock_on_read if set Mike Christie @ 2016-10-16 7:56 ` Christophe Varoqui 5 siblings, 0 replies; 7+ messages in thread From: Christophe Varoqui @ 2016-10-16 7:56 UTC (permalink / raw) To: Mike Christie; +Cc: device-mapper development [-- Attachment #1.1: Type: text/plain, Size: 574 bytes --] Merged. Thanks. On Tue, Oct 11, 2016 at 11:36 PM, Mike Christie <mchristi@redhat.com> wrote: > The following pathces made over the multipath-tools tree today > fix some bugs in the rbd checker and sync up log messages. > > The first 4 patches are a resend from here: > https://www.redhat.com/archives/dm-devel/2016-August/msg00429.html > I did not see any review comments (of course let me know if I missed > them), so the only changes are from being rebased on the current tree. > > The last patch is new and adds a fix for when rbd's lock_on_read > feature is used. > > [-- Attachment #1.2: Type: text/html, Size: 1040 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-10-16 7:56 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-11 21:36 [PATCH 0/5] rbd checker fixes Mike Christie 2016-10-11 21:36 ` [PATCH 1/5] rbd: fix sync repair support Mike Christie 2016-10-11 21:36 ` [PATCH 2/5] rbd: check for nonshared clients Mike Christie 2016-10-11 21:36 ` [PATCH 3/5] rbd: check for exclusive lock enabled Mike Christie 2016-10-11 21:36 ` [PATCH 4/5] rbd: fixup log messages Mike Christie 2016-10-11 21:36 ` [PATCH 5/5] rbd: use lock_on_read if set Mike Christie 2016-10-16 7:56 ` [PATCH 0/5] rbd checker fixes Christophe Varoqui
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.