* PATCH 0/4] multipath-tools: Ceph rbd support v2
@ 2016-08-08 12:01 Mike Christie
2016-08-08 12:01 ` [PATCH 1/4] libmultipath: add rbd discovery Mike Christie
` (4 more replies)
0 siblings, 5 replies; 18+ messages in thread
From: Mike Christie @ 2016-08-08 12:01 UTC (permalink / raw)
To: dm-devel, christophe.varoqui
The following patches made over Christophe's tree today,
add Ceph rbd support for handling blacklisted devices.
This is not general support for rbd and multipath. There is
no boot/root support and it does not support features like
multibus. My use is for HA configurations, specifically for exporting
rbd images through multiple LIO instances. In this case, we have one
rbd instance that has the ceph rbd exclusive lock and it can send
WRITE requests. If that host becomes unreachable, then another host
will grab the lock, and blacklist the original host to prevent it from
sending stale IO (when blacklisted IO will be failed by the OSD).
To recover from that type of scenario, this patchset adds a repair()
callout to the checker. If the path is in the PATH_DOWN state this
callout can be used to fix it up. For my case, I am remapping
the device to flush stale IO and cleanup the old lock,
and then unblacklisting the path, so it can be used again.
Changes since v1:
- Drop ID_UID use and implemented sysfs getuid support.
- Drop settings that were defaults and follow template.
- Fix ceph auth/user.
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/4] libmultipath: add rbd discovery 2016-08-08 12:01 PATCH 0/4] multipath-tools: Ceph rbd support v2 Mike Christie @ 2016-08-08 12:01 ` Mike Christie 2016-08-08 12:01 ` [PATCH 2/4] multipath-tools: add checker callout to repair path Mike Christie ` (3 subsequent siblings) 4 siblings, 0 replies; 18+ messages in thread From: Mike Christie @ 2016-08-08 12:01 UTC (permalink / raw) To: dm-devel, christophe.varoqui; +Cc: Mike Christie rbd is a block device interface for Ceph. It does not support any SCSI commands, so this patch adds bus detection and virtual vendor/product pathinfo. Changes since v1: 1. Drop ID_UID use and implemented sysfs getuid support. Signed-off-by: Mike Christie <mchristi@redhat.com> --- libmultipath/checkers.h | 1 + libmultipath/discovery.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++ libmultipath/structs.h | 1 + 3 files changed, 72 insertions(+) diff --git a/libmultipath/checkers.h b/libmultipath/checkers.h index ac382d7..8fc8616 100644 --- a/libmultipath/checkers.h +++ b/libmultipath/checkers.h @@ -84,6 +84,7 @@ enum path_check_state { #define EMC_CLARIION "emc_clariion" #define READSECTOR0 "readsector0" #define CCISS_TUR "cciss_tur" +#define RBD "rbd" #define DEFAULT_CHECKER TUR diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c index 1fb4db4..61b389f 100644 --- a/libmultipath/discovery.c +++ b/libmultipath/discovery.c @@ -1170,6 +1170,21 @@ scsi_sysfs_pathinfo (struct path * pp, vector hwtable) } static int +rbd_sysfs_pathinfo (struct path * pp, vector hwtable) +{ + sprintf(pp->vendor_id, "Ceph"); + sprintf(pp->product_id, "RBD"); + + condlog(3, "%s: vendor = %s product = %s", pp->dev, pp->vendor_id, + pp->product_id); + /* + * set the hwe configlet pointer + */ + pp->hwe = find_hwe(hwtable, pp->vendor_id, pp->product_id, NULL); + return 0; +} + +static int ccw_sysfs_pathinfo (struct path * pp, vector hwtable) { struct udev_device *parent; @@ -1371,6 +1386,8 @@ sysfs_pathinfo(struct path * pp, vector hwtable) pp->bus = SYSFS_BUS_CCW; if (!strncmp(pp->dev,"sd", 2)) pp->bus = SYSFS_BUS_SCSI; + if (!strncmp(pp->dev,"rbd", 3)) + pp->bus = SYSFS_BUS_RBD; if (pp->bus == SYSFS_BUS_UNDEF) return 0; @@ -1383,6 +1400,9 @@ sysfs_pathinfo(struct path * pp, vector hwtable) } else if (pp->bus == SYSFS_BUS_CCISS) { if (cciss_sysfs_pathinfo(pp, hwtable)) return 1; + } else if (pp->bus == SYSFS_BUS_RBD) { + if (rbd_sysfs_pathinfo(pp, hwtable)) + return 1; } return 0; } @@ -1540,6 +1560,53 @@ get_udev_uid(struct path * pp, char *uid_attribute) } static int +get_rbd_uid(struct path * pp) +{ + struct udev_device *rbd_bus_dev; + int ret, rbd_bus_id; + const char *pool, *image, *snap; + char sysfs_path[PATH_SIZE]; + uint64_t snap_id, max_snap_id = -3; + + ret = sscanf(pp->dev, "rbd%d", &rbd_bus_id); + if (ret != 1) + return -EINVAL; + + snprintf(sysfs_path, sizeof(sysfs_path), "/sys/bus/rbd/devices/%d", + rbd_bus_id); + rbd_bus_dev = udev_device_new_from_syspath(udev, sysfs_path); + if (!rbd_bus_dev) + return -ENODEV; + + ret = -EINVAL; + pool = udev_device_get_sysattr_value(rbd_bus_dev, "pool_id"); + if (!pool) + goto free_dev; + + image = udev_device_get_sysattr_value(rbd_bus_dev, "image_id"); + if (!image) + goto free_dev; + + snap = udev_device_get_sysattr_value(rbd_bus_dev, "snap_id"); + if (!snap) + goto free_dev; + snap_id = strtoull(snap, NULL, 19); + if (snap_id >= max_snap_id) + ret = snprintf(pp->wwid, WWID_SIZE, "%s-%s", pool, image); + else + ret = snprintf(pp->wwid, WWID_SIZE, "%s-%s-%s", pool, + image, snap); + if (ret >= WWID_SIZE) { + condlog(0, "%s: wwid overflow", pp->dev); + ret = -EOVERFLOW; + } + +free_dev: + udev_device_unref(rbd_bus_dev); + return ret; +} + +static int get_vpd_uid(struct path * pp) { struct udev_device *parent = pp->udev; @@ -1591,6 +1658,9 @@ get_uid (struct path * pp, int path_state) } else len = strlen(pp->wwid); origin = "callout"; + } else if (pp->bus == SYSFS_BUS_RBD) { + len = get_rbd_uid(pp); + origin = "sysfs"; } else { int retrigger; diff --git a/libmultipath/structs.h b/libmultipath/structs.h index a87cda6..cb5d532 100644 --- a/libmultipath/structs.h +++ b/libmultipath/structs.h @@ -52,6 +52,7 @@ enum sysfs_buses { SYSFS_BUS_IDE, SYSFS_BUS_CCW, SYSFS_BUS_CCISS, + SYSFS_BUS_RBD, }; enum pathstates { -- 2.7.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/4] multipath-tools: add checker callout to repair path 2016-08-08 12:01 PATCH 0/4] multipath-tools: Ceph rbd support v2 Mike Christie 2016-08-08 12:01 ` [PATCH 1/4] libmultipath: add rbd discovery Mike Christie @ 2016-08-08 12:01 ` Mike Christie 2016-08-11 15:50 ` Bart Van Assche 2016-08-08 12:01 ` [PATCH 3/4] multipath-tools: Add rbd checker Mike Christie ` (2 subsequent siblings) 4 siblings, 1 reply; 18+ messages in thread From: Mike Christie @ 2016-08-08 12:01 UTC (permalink / raw) To: dm-devel, christophe.varoqui; +Cc: Mike Christie This patch adds a callback which can be used to repair a path if check() has determined it is in the PATH_DOWN state. The next patch that adds rbd checker support which will use this to handle the case where a rbd device is blacklisted. Signed-off-by: Mike Christie <mchristi@redhat.com> --- libmultipath/checkers.c | 23 +++++++++++++++++++++++ libmultipath/checkers.h | 4 ++++ libmultipath/checkers/cciss_tur.c | 5 +++++ libmultipath/checkers/directio.c | 5 +++++ libmultipath/checkers/emc_clariion.c | 5 +++++ libmultipath/checkers/hp_sw.c | 5 +++++ libmultipath/checkers/rdac.c | 5 +++++ libmultipath/checkers/readsector0.c | 5 +++++ libmultipath/checkers/tur.c | 5 +++++ multipathd/main.c | 9 +++++++++ 10 files changed, 71 insertions(+) diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c index 42baa86..8976c89 100644 --- a/libmultipath/checkers.c +++ b/libmultipath/checkers.c @@ -138,6 +138,14 @@ struct checker * add_checker (char *multipath_dir, char * name) if (!c->free) goto out; + c->repair = (void (*)(struct checker *)) dlsym(c->handle, + "libcheck_repair"); + errstr = dlerror(); + if (errstr != NULL) + condlog(0, "A dynamic linking error occurred: (%s)", errstr); + if (!c->repair) + goto out; + c->fd = 0; c->sync = 1; list_add(&c->node, &checkers); @@ -203,6 +211,20 @@ void checker_put (struct checker * dst) free_checker(src); } +void checker_repair (struct checker * c) +{ + if (!c) + return; + + c->message[0] = '\0'; + if (c->disable) { + MSG(c, "checker disabled"); + return; + } + + c->repair(c); +} + int checker_check (struct checker * c) { int r; @@ -272,6 +294,7 @@ void checker_get (char *multipath_dir, struct checker * dst, char * name) dst->sync = src->sync; strncpy(dst->name, src->name, CHECKER_NAME_LEN); strncpy(dst->message, src->message, CHECKER_MSG_LEN); + dst->repair = src->repair; dst->check = src->check; dst->init = src->init; dst->free = src->free; diff --git a/libmultipath/checkers.h b/libmultipath/checkers.h index 8fc8616..502d2d9 100644 --- a/libmultipath/checkers.h +++ b/libmultipath/checkers.h @@ -113,6 +113,9 @@ struct checker { multipath-wide. Use MALLOC if you want to stuff data in. */ int (*check)(struct checker *); + void (*repair)(struct checker *); /* called if check returns + PATH_DOWN to bring path into + usable state */ int (*init)(struct checker *); /* to allocate the context */ void (*free)(struct checker *); /* to free the context */ }; @@ -132,6 +135,7 @@ void checker_set_async (struct checker *); void checker_set_fd (struct checker *, int); void checker_enable (struct checker *); void checker_disable (struct checker *); +void checker_repair (struct checker *); int checker_check (struct checker *); int checker_selected (struct checker *); char * checker_name (struct checker *); diff --git a/libmultipath/checkers/cciss_tur.c b/libmultipath/checkers/cciss_tur.c index 81500dc..a0ffffe 100644 --- a/libmultipath/checkers/cciss_tur.c +++ b/libmultipath/checkers/cciss_tur.c @@ -59,6 +59,11 @@ void libcheck_free (struct checker * c) return; } +void libcheck_repair (struct checker * c) +{ + return; +} + extern int libcheck_check (struct checker * c) { diff --git a/libmultipath/checkers/directio.c b/libmultipath/checkers/directio.c index 94bf8f7..eec12d5 100644 --- a/libmultipath/checkers/directio.c +++ b/libmultipath/checkers/directio.c @@ -118,6 +118,11 @@ void libcheck_free (struct checker * c) free(ct); } +void libcheck_repair (struct checker * c) +{ + return; +} + static int check_state(int fd, struct directio_context *ct, int sync, int timeout_secs) { diff --git a/libmultipath/checkers/emc_clariion.c b/libmultipath/checkers/emc_clariion.c index 5e416a0..a7b9f86 100644 --- a/libmultipath/checkers/emc_clariion.c +++ b/libmultipath/checkers/emc_clariion.c @@ -91,6 +91,11 @@ void libcheck_free (struct checker * c) free(c->context); } +void libcheck_repair (struct checker * c) +{ + return; +} + int libcheck_check (struct checker * c) { unsigned char sense_buffer[128] = { 0, }; diff --git a/libmultipath/checkers/hp_sw.c b/libmultipath/checkers/hp_sw.c index fe5e0f9..0cc1111 100644 --- a/libmultipath/checkers/hp_sw.c +++ b/libmultipath/checkers/hp_sw.c @@ -44,6 +44,11 @@ void libcheck_free (struct checker * c) return; } +void libcheck_repair (struct checker * c) +{ + return; +} + static int do_inq(int sg_fd, int cmddt, int evpd, unsigned int pg_op, void *resp, int mx_resp_len, int noisy, unsigned int timeout) diff --git a/libmultipath/checkers/rdac.c b/libmultipath/checkers/rdac.c index 00e3c44..68682c8 100644 --- a/libmultipath/checkers/rdac.c +++ b/libmultipath/checkers/rdac.c @@ -139,6 +139,11 @@ void libcheck_free (struct checker * c) return; } +void libcheck_repair (struct checker * c) +{ + return; +} + static int do_inq(int sg_fd, unsigned int pg_op, void *resp, int mx_resp_len, unsigned int timeout) diff --git a/libmultipath/checkers/readsector0.c b/libmultipath/checkers/readsector0.c index 1c2a868..8fccb46 100644 --- a/libmultipath/checkers/readsector0.c +++ b/libmultipath/checkers/readsector0.c @@ -23,6 +23,11 @@ void libcheck_free (struct checker * c) return; } +void libcheck_repair (struct checker * c) +{ + return; +} + int libcheck_check (struct checker * c) { unsigned char buf[4096]; diff --git a/libmultipath/checkers/tur.c b/libmultipath/checkers/tur.c index c4ef8a8..219afc3 100644 --- a/libmultipath/checkers/tur.c +++ b/libmultipath/checkers/tur.c @@ -97,6 +97,11 @@ void libcheck_free (struct checker * c) return; } +void libcheck_repair (struct checker * c) +{ + return; +} + #define TUR_MSG(msg, fmt, args...) snprintf(msg, CHECKER_MSG_LEN, fmt, ##args); int diff --git a/multipathd/main.c b/multipathd/main.c index 20589a7..f34500c 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -1725,6 +1725,14 @@ check_path (struct vectors * vecs, struct path * pp, int ticks) return 1; } +void repair_path(struct vectors * vecs, struct path * pp) +{ + if (pp->state != PATH_DOWN) + return; + + checker_repair(&pp->checker); +} + static void * checkerloop (void *ap) { @@ -1796,6 +1804,7 @@ checkerloop (void *ap) i--; } else num_paths += rc; + repair_path(vecs, pp); } lock_cleanup_pop(vecs->lock); } -- 2.7.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] multipath-tools: add checker callout to repair path 2016-08-08 12:01 ` [PATCH 2/4] multipath-tools: add checker callout to repair path Mike Christie @ 2016-08-11 15:50 ` Bart Van Assche 2016-08-11 20:33 ` Mike Christie 0 siblings, 1 reply; 18+ messages in thread From: Bart Van Assche @ 2016-08-11 15:50 UTC (permalink / raw) To: Mike Christie, dm-devel, christophe.varoqui On 08/08/2016 05:01 AM, Mike Christie wrote: > This patch adds a callback which can be used to repair a path > if check() has determined it is in the PATH_DOWN state. > > The next patch that adds rbd checker support which will use this to > handle the case where a rbd device is blacklisted. Hello Mike, With this patch applied, with the TUR checker enabled in multipath.conf I see the following crash if I trigger SRP failover and failback: ion-dev-ib-ini:~ # gdb ~bart/software/multipath-tools/multipathd/multipathd (gdb) handle SIGPIPE noprint nostop Signal Stop Print Pass to program Description SIGPIPE No No Yes Broken pipe (gdb) run -d Aug 11 08:46:27 | sde: remove path (uevent) Aug 11 08:46:27 | mpathbe: adding map Aug 11 08:46:27 | 8:64: cannot find block device Aug 11 08:46:27 | Invalid device number 1 Aug 11 08:46:27 | 1: cannot find block device Aug 11 08:46:27 | 8:96: cannot find block device Aug 11 08:46:27 | mpathbe: failed to setup multipath Aug 11 08:46:27 | dm-0: uev_add_map failed Aug 11 08:46:27 | uevent trigger error Thread 4 "multipathd" received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7ffff7f8b700 (LWP 8446)] 0x0000000000000000 in ?? () (gdb) bt #0 0x0000000000000000 in ?? () #1 0x00007ffff6c41905 in checker_repair (c=0x7fffdc001ef0) at checkers.c:225 #2 0x000000000040a760 in repair_path (vecs=0x66d7e0, pp=0x7fffdc001a40) at main.c:1733 #3 0x000000000040ab27 in checkerloop (ap=0x66d7e0) at main.c:1807 #4 0x00007ffff79bb474 in start_thread (arg=0x7ffff7f8b700) at pthread_create.c:333 #5 0x00007ffff63243ed in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109 (gdb) up #1 0x00007ffff6c41905 in checker_repair (c=0x7fffdc001ef0) at checkers.c:225 225 c->repair(c); (gdb) print *c $1 = {node = {next = 0x0, prev = 0x0}, handle = 0x0, refcount = 0, fd = 0, sync = 0, timeout = 0, disable = 0, name = '\000' <repeats 15 times>, message = '\000' <repeats 255 times>, context = 0x0, mpcontext = 0x0, check = 0x0, repair = 0x0, init = 0x0, free = 0x0} Can you have a look at this? Thanks, Bart. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] multipath-tools: add checker callout to repair path 2016-08-11 15:50 ` Bart Van Assche @ 2016-08-11 20:33 ` Mike Christie 2016-08-11 21:41 ` Bart Van Assche 0 siblings, 1 reply; 18+ messages in thread From: Mike Christie @ 2016-08-11 20:33 UTC (permalink / raw) To: Bart Van Assche, dm-devel, christophe.varoqui [-- Attachment #1: Type: text/plain, Size: 2571 bytes --] On 08/11/2016 10:50 AM, Bart Van Assche wrote: > On 08/08/2016 05:01 AM, Mike Christie wrote: >> This patch adds a callback which can be used to repair a path >> if check() has determined it is in the PATH_DOWN state. >> >> The next patch that adds rbd checker support which will use this to >> handle the case where a rbd device is blacklisted. > > Hello Mike, > > With this patch applied, with the TUR checker enabled in multipath.conf > I see the following crash if I trigger SRP failover and failback: > > ion-dev-ib-ini:~ # gdb ~bart/software/multipath-tools/multipathd/multipathd > (gdb) handle SIGPIPE noprint nostop > Signal Stop Print Pass to program Description > SIGPIPE No No Yes Broken pipe > (gdb) run -d > Aug 11 08:46:27 | sde: remove path (uevent) > Aug 11 08:46:27 | mpathbe: adding map > Aug 11 08:46:27 | 8:64: cannot find block device > Aug 11 08:46:27 | Invalid device number 1 > Aug 11 08:46:27 | 1: cannot find block device > Aug 11 08:46:27 | 8:96: cannot find block device > Aug 11 08:46:27 | mpathbe: failed to setup multipath > Aug 11 08:46:27 | dm-0: uev_add_map failed > Aug 11 08:46:27 | uevent trigger error > > Thread 4 "multipathd" received signal SIGSEGV, Segmentation fault. > [Switching to Thread 0x7ffff7f8b700 (LWP 8446)] > 0x0000000000000000 in ?? () > (gdb) bt > #0 0x0000000000000000 in ?? () > #1 0x00007ffff6c41905 in checker_repair (c=0x7fffdc001ef0) at checkers.c:225 > #2 0x000000000040a760 in repair_path (vecs=0x66d7e0, pp=0x7fffdc001a40) > at main.c:1733 > #3 0x000000000040ab27 in checkerloop (ap=0x66d7e0) at main.c:1807 > #4 0x00007ffff79bb474 in start_thread (arg=0x7ffff7f8b700) > at pthread_create.c:333 > #5 0x00007ffff63243ed in clone () > at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109 > (gdb) up > #1 0x00007ffff6c41905 in checker_repair (c=0x7fffdc001ef0) at checkers.c:225 > 225 c->repair(c); > (gdb) print *c > $1 = {node = {next = 0x0, prev = 0x0}, handle = 0x0, refcount = 0, fd = 0, > sync = 0, timeout = 0, disable = 0, name = '\000' <repeats 15 times>, > message = '\000' <repeats 255 times>, context = 0x0, mpcontext = 0x0, > check = 0x0, repair = 0x0, init = 0x0, free = 0x0} > Sorry about the stupid bug. Could you try the attached patch. I found two segfaults. If check_path returns less than 0 then we free the path and so we cannot call repair on it. If libcheck_init fails it memsets the checker, so we cannot call repair on it too. I moved the repair call to the specific paths that the path is down. [-- Attachment #2: multipathd-only-call-repair-when-failed.patch --] [-- Type: text/x-patch, Size: 1390 bytes --] diff --git a/multipathd/main.c b/multipathd/main.c index f34500c..9f213cc 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -1442,6 +1442,16 @@ int update_path_groups(struct multipath *mpp, struct vectors *vecs, int refresh) return 0; } +void repair_path(struct path * pp) +{ + if (pp->state != PATH_DOWN) + return; + + checker_repair(&pp->checker); + if (strlen(checker_message(&pp->checker))) + LOG_MSG(1, checker_message(&pp->checker)); +} + /* * Returns '1' if the path has been checked, '-1' if it was blacklisted * and '0' otherwise @@ -1606,6 +1616,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks) pp->mpp->failback_tick = 0; pp->mpp->stat_path_failures++; + repair_path(pp); return 1; } @@ -1700,7 +1711,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks) } pp->state = newstate; - + repair_path(pp); if (pp->mpp->wait_for_udev) return 1; @@ -1725,14 +1736,6 @@ check_path (struct vectors * vecs, struct path * pp, int ticks) return 1; } -void repair_path(struct vectors * vecs, struct path * pp) -{ - if (pp->state != PATH_DOWN) - return; - - checker_repair(&pp->checker); -} - static void * checkerloop (void *ap) { @@ -1804,7 +1807,6 @@ checkerloop (void *ap) i--; } else num_paths += rc; - repair_path(vecs, pp); } lock_cleanup_pop(vecs->lock); } [-- Attachment #3: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] multipath-tools: add checker callout to repair path 2016-08-11 20:33 ` Mike Christie @ 2016-08-11 21:41 ` Bart Van Assche 2016-08-12 16:54 ` Mike Christie 2016-08-14 8:41 ` Mike Christie 0 siblings, 2 replies; 18+ messages in thread From: Bart Van Assche @ 2016-08-11 21:41 UTC (permalink / raw) To: Mike Christie, dm-devel, christophe.varoqui On 08/11/2016 01:33 PM, Mike Christie wrote: > Could you try the attached patch. I found two segfaults. If check_path > returns less than 0 then we free the path and so we cannot call repair > on it. If libcheck_init fails it memsets the checker, so we cannot call > repair on it too. > > I moved the repair call to the specific paths that the path is down. Hello Mike, Thanks for the patch. Unfortunately even with this patch applied I can still trigger a segfault sporadically: # valgrind --read-var-info=yes multipathd -d Aug 11 14:02:21 | mpathbf: load table [0 2097152 multipath 3 queue_if_no_path pg_init_retries 50 0 2 1 queue-length 0 1 1 8:160 1000 queue-length 0 1 1 8:64 1000] Aug 11 14:02:21 | mpathbf: event checker started Aug 11 14:02:21 | sdk [8:160]: path added to devmap mpathbf Aug 11 14:02:21 | sdd: add path (uevent) ==2452== Thread 4: ==2452== Jump to the invalid address stated on the next line ==2452== at 0x0: ??? ==2452== by 0x409BBE: repair_path (main.c:1451) ==2452== by 0x40A905: check_path (main.c:1715) ==2452== by 0x40AE72: checkerloop (main.c:1808) ==2452== by 0x5047473: start_thread (pthread_create.c:333) ==2452== by 0x671B3EC: clone (clone.S:109) ==2452== Address 0x0 is not stack'd, malloc'd or (recently) free'd ==2452== ==2452== ==2452== Process terminating with default action of signal 11 (SIGSEGV) ==2452== Bad permissions for mapped region at address 0x0 ==2452== at 0x0: ??? ==2452== by 0x409BBE: repair_path (main.c:1451) ==2452== by 0x40A905: check_path (main.c:1715) ==2452== by 0x40AE72: checkerloop (main.c:1808) ==2452== by 0x5047473: start_thread (pthread_create.c:333) ==2452== by 0x671B3EC: clone (clone.S:109) ==2452== (gdb) list main.c:1451 1446 void repair_path(struct path * pp) 1447 { 1448 if (pp->state != PATH_DOWN) 1449 return; 1450 1451 checker_repair(&pp->checker); 1452 if (strlen(checker_message(&pp->checker))) 1453 LOG_MSG(1, checker_message(&pp->checker)); 1454 } 1455 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] multipath-tools: add checker callout to repair path 2016-08-11 21:41 ` Bart Van Assche @ 2016-08-12 16:54 ` Mike Christie 2016-08-12 17:10 ` Bart Van Assche 2016-08-14 8:41 ` Mike Christie 1 sibling, 1 reply; 18+ messages in thread From: Mike Christie @ 2016-08-12 16:54 UTC (permalink / raw) To: Bart Van Assche, dm-devel, christophe.varoqui On 08/11/2016 04:41 PM, Bart Van Assche wrote: > On 08/11/2016 01:33 PM, Mike Christie wrote: >> Could you try the attached patch. I found two segfaults. If check_path >> returns less than 0 then we free the path and so we cannot call repair >> on it. If libcheck_init fails it memsets the checker, so we cannot call >> repair on it too. >> >> I moved the repair call to the specific paths that the path is down. > > Hello Mike, > > Thanks for the patch. Unfortunately even with this patch applied I can > still trigger a segfault sporadically: > I can't seem to replicate the problem with my patch and I do not see anything. Could you send me your multipath.conf/hwtable settings? For the fo/fb test, dev_loss_tmo is firing causing paths to be added/deleted right? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] multipath-tools: add checker callout to repair path 2016-08-12 16:54 ` Mike Christie @ 2016-08-12 17:10 ` Bart Van Assche 0 siblings, 0 replies; 18+ messages in thread From: Bart Van Assche @ 2016-08-12 17:10 UTC (permalink / raw) To: Mike Christie, dm-devel, christophe.varoqui On 08/12/2016 09:54 AM, Mike Christie wrote: > On 08/11/2016 04:41 PM, Bart Van Assche wrote: >> On 08/11/2016 01:33 PM, Mike Christie wrote: >>> Could you try the attached patch. I found two segfaults. If check_path >>> returns less than 0 then we free the path and so we cannot call repair >>> on it. If libcheck_init fails it memsets the checker, so we cannot call >>> repair on it too. >>> >>> I moved the repair call to the specific paths that the path is down. >> >> Thanks for the patch. Unfortunately even with this patch applied I can >> still trigger a segfault sporadically: > > I can't seem to replicate the problem with my patch and I do not see > anything. Could you send me your multipath.conf/hwtable settings? Please find that file at the end of this e-mail. > For the fo/fb test, dev_loss_tmo is firing causing paths to be > added/deleted right? The script that I'm using to simulate path loss writes into /sys/class /srp_remote_ports/*/delete. That causes the ib_srp driver to call scsi_remove_host(). That script is available at https://github.com/bvanassche/srp-test. However, an InfiniBand HCA is needed to run this script. Bart. /etc/multipath.conf: defaults { user_friendly_names yes queue_without_daemon no } blacklist { device { vendor "ATA" product ".*" } } devices { device { vendor "SCST_BIO|LIO-ORG" product ".*" features "3 queue_if_no_path pg_init_retries 50" path_grouping_policy group_by_prio path_selector "queue-length 0" path_checker tur } } blacklist_exceptions { property ".*" } ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] multipath-tools: add checker callout to repair path 2016-08-11 21:41 ` Bart Van Assche 2016-08-12 16:54 ` Mike Christie @ 2016-08-14 8:41 ` Mike Christie 2016-08-15 16:24 ` Bart Van Assche 1 sibling, 1 reply; 18+ messages in thread From: Mike Christie @ 2016-08-14 8:41 UTC (permalink / raw) To: Bart Van Assche, dm-devel, christophe.varoqui [-- Attachment #1: Type: text/plain, Size: 1247 bytes --] On 08/11/2016 04:41 PM, Bart Van Assche wrote: > On 08/11/2016 01:33 PM, Mike Christie wrote: >> Could you try the attached patch. I found two segfaults. If check_path >> returns less than 0 then we free the path and so we cannot call repair >> on it. If libcheck_init fails it memsets the checker, so we cannot call >> repair on it too. >> >> I moved the repair call to the specific paths that the path is down. > > Hello Mike, > > Thanks for the patch. Unfortunately even with this patch applied I can > still trigger a segfault sporadically: > Ok. This should fix all of them. Attached patch fixes: 1. If check_path returns less than 0 then we free the path and so we cannot call repair on it of course. 2. If libcheck_init fails it memsets the checker, so we cannot call repair on it too. 3. We can hit a race where when pathinfo is setting up a path, the path could have gone down. In the DI_CHECKER chunk we then do not run get_state and attach a checker. Later when check_path is run path_offline we could still return PATH_DOWN or PATH_REMOVED and get_state is again not run so we do not get to attach a checker again. I was then running repair_path since the state was PATH_DOWN, and kaboom. Attached patch should fix these issues. [-- Attachment #2: multipathd-fix-segfault.patch --] [-- Type: text/x-patch, Size: 1731 bytes --] diff --git a/libmultipath/checkers.c b/libmultipath/checkers.c index 8976c89..fd999b0 100644 --- a/libmultipath/checkers.c +++ b/libmultipath/checkers.c @@ -213,7 +213,7 @@ void checker_put (struct checker * dst) void checker_repair (struct checker * c) { - if (!c) + if (!c || !checker_selected(c)) return; c->message[0] = '\0'; diff --git a/multipathd/main.c b/multipathd/main.c index f5e9a01..c4ffe6f 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -1442,6 +1442,16 @@ int update_path_groups(struct multipath *mpp, struct vectors *vecs, int refresh) return 0; } +void repair_path(struct path * pp) +{ + if (pp->state != PATH_DOWN) + return; + + checker_repair(&pp->checker); + if (strlen(checker_message(&pp->checker))) + LOG_MSG(1, checker_message(&pp->checker)); +} + /* * Returns '1' if the path has been checked, '-1' if it was blacklisted * and '0' otherwise @@ -1606,6 +1616,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks) pp->mpp->failback_tick = 0; pp->mpp->stat_path_failures++; + repair_path(pp); return 1; } @@ -1700,7 +1711,7 @@ check_path (struct vectors * vecs, struct path * pp, int ticks) } pp->state = newstate; - + repair_path(pp); if (pp->mpp->wait_for_udev) return 1; @@ -1725,14 +1736,6 @@ check_path (struct vectors * vecs, struct path * pp, int ticks) return 1; } -void repair_path(struct vectors * vecs, struct path * pp) -{ - if (pp->state != PATH_DOWN) - return; - - checker_repair(&pp->checker); -} - static void * checkerloop (void *ap) { @@ -1804,7 +1807,6 @@ checkerloop (void *ap) i--; } else num_paths += rc; - repair_path(vecs, pp); } lock_cleanup_pop(vecs->lock); } [-- Attachment #3: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] multipath-tools: add checker callout to repair path 2016-08-14 8:41 ` Mike Christie @ 2016-08-15 16:24 ` Bart Van Assche 0 siblings, 0 replies; 18+ messages in thread From: Bart Van Assche @ 2016-08-15 16:24 UTC (permalink / raw) To: Mike Christie, dm-devel, christophe.varoqui On 08/14/2016 01:41 AM, Mike Christie wrote: > Attached patch should fix these issues. Hello Mike, With that patch applied multipathd works again reliable on my test setup. Thanks! Bart. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 3/4] multipath-tools: Add rbd checker. 2016-08-08 12:01 PATCH 0/4] multipath-tools: Ceph rbd support v2 Mike Christie 2016-08-08 12:01 ` [PATCH 1/4] libmultipath: add rbd discovery Mike Christie 2016-08-08 12:01 ` [PATCH 2/4] multipath-tools: add checker callout to repair path Mike Christie @ 2016-08-08 12:01 ` Mike Christie 2016-08-08 12:01 ` [PATCH 4/4] multipath-tools: Add rbd to the hwtable Mike Christie 2016-08-09 15:36 ` PATCH 0/4] multipath-tools: Ceph rbd support v2 Christophe Varoqui 4 siblings, 0 replies; 18+ messages in thread From: Mike Christie @ 2016-08-08 12:01 UTC (permalink / raw) To: dm-devel, christophe.varoqui; +Cc: Mike Christie This checker currently only handles the case where a path is failed due to it being blacklisted by the ceph cluster. The specific use case for me is when LIO exports rbd images through multiple LIO instances. The problem it handles is when rbd instance1 has the exclusive lock, but becomes unreachable another host in the cluster will take over and blacklist the instance1. This prevents it from sending stale IO and corrupting data. Later, when the host is reachable, we will want to failback to it. To this, the checker will detect we were blacklisted, unmap the old image which will make sure old IO is failed, and then remap the image and unblacklist the host. multipathd will then handle this like a path being removed and re-added. Changes since v1: 1. Call the rbd command to remap images instead of trying to interact with sysfs directly. This fixes ceph auth issues hit when using the sysfs interface directly. We currently do not support rbd root/boot. When that is supported we will have a lib to interact with the kernel and we can then convert to it. Signed-off-by: Mike Christie <mchristi@redhat.com> --- libmultipath/checkers/Makefile | 6 +- libmultipath/checkers/rbd.c | 639 +++++++++++++++++++++++++++++++++++++++++ multipath/multipath.conf.5 | 3 + 3 files changed, 647 insertions(+), 1 deletion(-) create mode 100644 libmultipath/checkers/rbd.c diff --git a/libmultipath/checkers/Makefile b/libmultipath/checkers/Makefile index fb8fff0..5fd2099 100644 --- a/libmultipath/checkers/Makefile +++ b/libmultipath/checkers/Makefile @@ -11,12 +11,16 @@ LIBS= \ libcheckdirectio.so \ libcheckemc_clariion.so \ libcheckhp_sw.so \ - libcheckrdac.so + libcheckrdac.so \ + libcheckrbd.so CFLAGS += -I.. all: $(LIBS) +libcheckrbd.so: rbd.o + $(CC) $(LDFLAGS) $(SHARED_FLAGS) -o $@ $^ -lrados -ludev + libcheckdirectio.so: libsg.o directio.o $(CC) $(LDFLAGS) $(SHARED_FLAGS) -o $@ $^ -laio diff --git a/libmultipath/checkers/rbd.c b/libmultipath/checkers/rbd.c new file mode 100644 index 0000000..6f1b53a --- /dev/null +++ b/libmultipath/checkers/rbd.c @@ -0,0 +1,639 @@ +/* + * Copyright (c) 2016 Red Hat + * Copyright (c) 2004 Christophe Varoqui + * + * Code based off of tur.c and ceph's krbd.cc + */ +#define _GNU_SOURCE +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> +#include <fcntl.h> +#include <errno.h> +#include <pthread.h> +#include <libudev.h> +#include <ifaddrs.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <sys/ioctl.h> +#include <sys/time.h> +#include <sys/wait.h> + +#include "rados/librados.h" + +#include "structs.h" +#include "checkers.h" + +#include "../libmultipath/debug.h" +#include "../libmultipath/uevent.h" + +struct rbd_checker_context; +typedef int (thread_fn)(struct rbd_checker_context *ct, char *msg); + +#define RBD_MSG(msg, fmt, args...) snprintf(msg, CHECKER_MSG_LEN, fmt, ##args); + +struct rbd_checker_context { + int rbd_bus_id; + char *client_addr; + char *config_info; + char *snap; + char *pool; + char *image; + char *username; + int remapped; + int blacklisted; + + rados_t cluster; + + int state; + int running; + time_t time; + thread_fn *fn; + pthread_t thread; + pthread_mutex_t lock; + pthread_cond_t active; + pthread_spinlock_t hldr_lock; + int holders; + char message[CHECKER_MSG_LEN]; +}; + +int libcheck_init(struct checker * c) +{ + struct rbd_checker_context *ct; + struct udev_device *block_dev; + struct udev_device *bus_dev; + struct udev *udev; + struct stat sb; + const char *block_name, *addr, *config_info; + const char *image, *pool, *snap, *username; + char sysfs_path[PATH_SIZE]; + int ret; + + ct = malloc(sizeof(struct rbd_checker_context)); + if (!ct) + return 1; + memset(ct, 0, sizeof(struct rbd_checker_context)); + ct->holders = 1; + pthread_cond_init(&ct->active, NULL); + pthread_mutex_init(&ct->lock, NULL); + pthread_spin_init(&ct->hldr_lock, PTHREAD_PROCESS_PRIVATE); + c->context = ct; + + /* + * The rbd block layer sysfs device is not linked to the rbd bus + * device that we interact with, so figure that out now. + */ + if (fstat(c->fd, &sb) != 0) + goto free_ct; + + udev = udev_new(); + if (!udev) + goto free_ct; + + block_dev = udev_device_new_from_devnum(udev, 'b', sb.st_rdev); + if (!block_dev) + goto free_udev; + + block_name = udev_device_get_sysname(block_dev); + ret = sscanf(block_name, "rbd%d", &ct->rbd_bus_id); + + udev_device_unref(block_dev); + if (ret != 1) + goto free_udev; + + snprintf(sysfs_path, sizeof(sysfs_path), "/sys/bus/rbd/devices/%d", + ct->rbd_bus_id); + bus_dev = udev_device_new_from_syspath(udev, sysfs_path); + if (!bus_dev) + goto free_udev; + + 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"); + goto free_dev; + } + + ct->client_addr = strdup(addr); + if (!ct->client_addr) + goto free_dev; + + config_info = udev_device_get_sysattr_value(bus_dev, "config_info"); + if (!config_info) + goto free_addr; + + ct->config_info = strdup(config_info); + if (!ct->config_info) + goto free_addr; + + username = strstr(config_info, "name="); + if (username) { + char *end; + int len; + + username += 5; + end = strchr(username, ','); + if (!end) + goto free_info; + len = end - username; + + ct->username = malloc(len + 1); + if (!ct->username) + goto free_info; + strncpy(ct->username, username, len); + ct->username[len] = '\0'; + } + + image = udev_device_get_sysattr_value(bus_dev, "name"); + if (!image) + goto free_username; + + ct->image = strdup(image); + if (!ct->image) + goto free_info; + + pool = udev_device_get_sysattr_value(bus_dev, "pool"); + if (!pool) + goto free_image; + + ct->pool = strdup(pool); + if (!ct->pool) + goto free_image; + + snap = udev_device_get_sysattr_value(bus_dev, "current_snap"); + if (!snap) + goto free_pool; + + if (strcmp("-", snap)) { + ct->snap = strdup(snap); + if (!ct->snap) + goto free_pool; + } + + if (rados_create(&ct->cluster, NULL) < 0) { + condlog(0, "Could not create rados cluster"); + goto free_snap; + } + + if (rados_conf_read_file(ct->cluster, NULL) < 0) { + condlog(0, "Could not read rados conf"); + goto shutdown_rados; + } + + ret = rados_connect(ct->cluster); + if (ret < 0) { + condlog(0, "Could not connect to rados cluster"); + goto shutdown_rados; + } + + udev_device_unref(bus_dev); + udev_unref(udev); + + condlog(3, "rbd%d checker init %s %s/%s@%s %s", ct->rbd_bus_id, + ct->client_addr, ct->pool, ct->image, ct->snap ? ct->snap : "-", + ct->username ? ct->username : "none"); + return 0; + +shutdown_rados: + rados_shutdown(ct->cluster); +free_snap: + if (ct->snap) + free(ct->snap); +free_pool: + free(ct->pool); +free_image: + free(ct->image); +free_username: + if (ct->username) + free(ct->username); +free_info: + free(ct->config_info); +free_addr: + free(ct->client_addr); +free_dev: + udev_device_unref(bus_dev); +free_udev: + udev_unref(udev); +free_ct: + free(ct); + return 1; +} + +void cleanup_context(struct rbd_checker_context *ct) +{ + pthread_mutex_destroy(&ct->lock); + pthread_cond_destroy(&ct->active); + pthread_spin_destroy(&ct->hldr_lock); + + rados_shutdown(ct->cluster); + + if (ct->username) + free(ct->username); + if (ct->snap) + free(ct->snap); + free(ct->pool); + free(ct->image); + free(ct->config_info); + free(ct->client_addr); + free(ct); +} + +void libcheck_free(struct checker * c) +{ + if (c->context) { + struct rbd_checker_context *ct = c->context; + int holders; + pthread_t thread; + + pthread_spin_lock(&ct->hldr_lock); + ct->holders--; + holders = ct->holders; + thread = ct->thread; + pthread_spin_unlock(&ct->hldr_lock); + if (holders) + pthread_cancel(thread); + else + cleanup_context(ct); + c->context = NULL; + } +} + +static int rbd_is_blacklisted(struct rbd_checker_context *ct, char *msg) +{ + char *addr_tok, *start, *save; + char *cmd[2]; + char *blklist, *stat; + size_t blklist_len, stat_len; + int ret; + char *end; + + cmd[0] = "{\"prefix\": \"osd blacklist ls\"}"; + cmd[1] = NULL; + + 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); + return ret; + } + + if (!blklist || !blklist_len) + goto free_bufs; + + /* + * parse list of addrs with the format + * ipv4:port/nonce date time\n + * or + * [ipv6]:port/nonce date time\n + */ + ret = 0; + for (start = blklist; ; start = NULL) { + addr_tok = strtok_r(start, "\n", &save); + if (!addr_tok || !strlen(addr_tok)) + break; + + end = strchr(addr_tok, ' '); + if (!end) { + RBD_MSG(msg, "rbd%d checker failed: invalid blacklist %s", + ct->rbd_bus_id, 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); + ret = 1; + break; + } + } + +free_bufs: + rados_buffer_free(blklist); + rados_buffer_free(stat); + return ret; +} + +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"); + /* + * Path may have issues, but the ceph cluster is at least + * accepting IO, so we can attempt to do IO. + * + * TODO: in future versions, we can run other tests to + * verify OSDs and networks. + */ + return PATH_UP; +} + +int safe_write(int fd, const void *buf, size_t count) +{ + while (count > 0) { + ssize_t r = write(fd, buf, count); + if (r < 0) { + if (errno == EINTR) + continue; + return -errno; + } + count -= r; + buf = (char *)buf + r; + } + return 0; +} + +static int sysfs_write_rbd_bus(const char *which, const char *buf, + size_t buf_len) +{ + char sysfs_path[PATH_SIZE]; + int fd; + int r; + + /* we require newer kernels so single_major should alwayws be there */ + snprintf(sysfs_path, sizeof(sysfs_path), + "/sys/bus/rbd/%s_single_major", which); + fd = open(sysfs_path, O_WRONLY); + if (fd < 0) + return -errno; + + r = safe_write(fd, buf, buf_len); + close(fd); + return r; +} + +static int rbd_remap(struct rbd_checker_context *ct) +{ + char *argv[11]; + pid_t pid; + int ret = 0, i = 0; + int status; + + pid = fork(); + switch (pid) { + case 0: + argv[i++] = "rbd"; + argv[i++] = "map"; + argv[i++] = "-o noshare"; + if (ct->username) { + argv[i++] = "--id"; + argv[i++] = ct->username; + } + argv[i++] = "--pool"; + argv[i++] = ct->pool; + if (ct->snap) { + argv[i++] = "--snap"; + argv[i++] = ct->snap; + } + argv[i++] = ct->image; + argv[i] = NULL; + + ret = execvp(argv[0], argv); + condlog(0, "Error executing rbd: %s", strerror(errno)); + exit(-1); + case -1: + condlog(0, "fork failed: %s", strerror(errno)); + return -1; + default: + ret = -1; + wait(&status); + if (WIFEXITED(status)) { + status = WEXITSTATUS(status); + if (status == 0) + ret = 0; + else + condlog(0, "rbd failed with %d", status); + } + } + + return ret; +} + +static int sysfs_write_rbd_remove(const char *buf, int buf_len) +{ + return sysfs_write_rbd_bus("remove", buf, buf_len); +} + +static int rbd_rm_blacklist(struct rbd_checker_context *ct) +{ + char *cmd[2]; + char *stat, *cmd_str; + size_t stat_len; + int ret; + + ret = asprintf(&cmd_str, "{\"prefix\": \"osd blacklist\", \"blacklistop\": \"rm\", \"addr\": \"%s\"}", + ct->client_addr); + if (ret == -1) + return -ENOMEM; + + cmd[0] = cmd_str; + cmd[1] = NULL; + + 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", + ct->rbd_bus_id, ct->client_addr, ret); + goto free_cmd; + } + + condlog(1, "rbd%d repair rm blacklist for %s", + ct->rbd_bus_id, ct->client_addr); + free(stat); +free_cmd: + free(cmd_str); + return ret; +} + +static int rbd_repair(struct rbd_checker_context *ct, char *msg) +{ + char del[17]; + int ret; + + if (!ct->blacklisted) + return PATH_UP; + + 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); + return PATH_DOWN; + } + } + ct->remapped = 1; + + 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); + 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); + return PATH_DOWN; + } + + ct->remapped = 0; + ct->blacklisted = 0; + + RBD_MSG(msg, "rbd%d has been repaired", ct->rbd_bus_id); + return PATH_UP; +} + +#define rbd_thread_cleanup_push(ct) pthread_cleanup_push(cleanup_func, ct) +#define rbd_thread_cleanup_pop(ct) pthread_cleanup_pop(1) + +void cleanup_func(void *data) +{ + int holders; + struct rbd_checker_context *ct = data; + pthread_spin_lock(&ct->hldr_lock); + ct->holders--; + holders = ct->holders; + ct->thread = 0; + pthread_spin_unlock(&ct->hldr_lock); + if (!holders) + cleanup_context(ct); +} + +void *rbd_thread(void *ctx) +{ + struct rbd_checker_context *ct = ctx; + int state; + + condlog(3, "rbd%d thread starting up", ct->rbd_bus_id); + + ct->message[0] = '\0'; + /* This thread can be canceled, so setup clean up */ + rbd_thread_cleanup_push(ct) + + /* checker start up */ + pthread_mutex_lock(&ct->lock); + ct->state = PATH_PENDING; + pthread_mutex_unlock(&ct->lock); + + state = ct->fn(ct, ct->message); + + /* checker done */ + pthread_mutex_lock(&ct->lock); + ct->state = state; + pthread_mutex_unlock(&ct->lock); + pthread_cond_signal(&ct->active); + + condlog(3, "rbd%d thead finished, state %s", ct->rbd_bus_id, + checker_state_name(state)); + rbd_thread_cleanup_pop(ct); + return ((void *)0); +} + +static void rbd_timeout(struct timespec *tsp) +{ + struct timeval now; + + gettimeofday(&now, NULL); + tsp->tv_sec = now.tv_sec; + tsp->tv_nsec = now.tv_usec * 1000; + tsp->tv_nsec += 1000000; /* 1 millisecond */ +} + +static int rbd_exec_fn(struct checker *c, thread_fn *fn) +{ + struct rbd_checker_context *ct = c->context; + struct timespec tsp; + pthread_attr_t attr; + int rbd_status, r; + + if (c->sync) + return rbd_check(ct, c->message); + /* + * Async mode + */ + r = pthread_mutex_lock(&ct->lock); + if (r != 0) { + 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); + 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); + rbd_status = PATH_PENDING; + } else { + /* checker done */ + ct->running = 0; + rbd_status = ct->state; + strncpy(c->message, ct->message, CHECKER_MSG_LEN); + c->message[CHECKER_MSG_LEN - 1] = '\0'; + } + pthread_mutex_unlock(&ct->lock); + } else { + /* Start new checker */ + ct->state = PATH_UNCHECKED; + ct->fn = fn; + pthread_spin_lock(&ct->hldr_lock); + ct->holders++; + pthread_spin_unlock(&ct->hldr_lock); + setup_thread_attr(&attr, 32 * 1024, 1); + r = pthread_create(&ct->thread, &attr, rbd_thread, ct); + if (r) { + pthread_mutex_unlock(&ct->lock); + ct->thread = 0; + ct->holders--; + condlog(3, "rbd%d failed to start rbd thread, using sync mode", + ct->rbd_bus_id); + return fn(ct, c->message); + } + pthread_attr_destroy(&attr); + rbd_timeout(&tsp); + r = pthread_cond_timedwait(&ct->active, &ct->lock, &tsp); + rbd_status = ct->state; + strncpy(c->message, ct->message,CHECKER_MSG_LEN); + c->message[CHECKER_MSG_LEN -1] = '\0'; + pthread_mutex_unlock(&ct->lock); + + if (ct->thread && + (rbd_status == PATH_PENDING || rbd_status == PATH_UNCHECKED)) { + condlog(3, "rbd%d thread still running", + ct->rbd_bus_id); + ct->running = 1; + rbd_status = PATH_PENDING; + } + } + + return rbd_status; +} + +void libcheck_repair(struct checker * c) +{ + struct rbd_checker_context *ct = c->context; + + if (!ct || !ct->blacklisted) + return; + rbd_exec_fn(c, rbd_repair); +} + +int libcheck_check(struct checker * c) +{ + struct rbd_checker_context *ct = c->context; + + if (!ct) + return PATH_UNCHECKED; + + if (ct->blacklisted) + return PATH_DOWN; + + return rbd_exec_fn(c, rbd_check); +} diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5 index aaf2705..8fd61f9 100644 --- a/multipath/multipath.conf.5 +++ b/multipath/multipath.conf.5 @@ -344,6 +344,9 @@ Please use \fItur\fR instead. (Hardware-dependent) Check the path state for HP/COMPAQ Smart Array(CCISS) controllers. .TP +.I rbd +Check if the path is in the Ceph blacklist and remap the path if it is. +.TP Default value is: \fBtur\fR .RE .TP -- 2.7.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/4] multipath-tools: Add rbd to the hwtable 2016-08-08 12:01 PATCH 0/4] multipath-tools: Ceph rbd support v2 Mike Christie ` (2 preceding siblings ...) 2016-08-08 12:01 ` [PATCH 3/4] multipath-tools: Add rbd checker Mike Christie @ 2016-08-08 12:01 ` Mike Christie 2016-08-09 15:36 ` PATCH 0/4] multipath-tools: Ceph rbd support v2 Christophe Varoqui 4 siblings, 0 replies; 18+ messages in thread From: Mike Christie @ 2016-08-08 12:01 UTC (permalink / raw) To: dm-devel, christophe.varoqui; +Cc: Mike Christie Add rbd to hwtable. These defaults are for the HA type of setup supported by the checker. We do no support features like multibus at the dm-multipath level yet. Changes since v1: 1. Drop settings that were defaults and follow template. 2. Drop ID_UID use. Signed-off-by: Mike Christie <mchristi@redhat.com> --- libmultipath/hwtable.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c index 8c074f9..c307477 100644 --- a/libmultipath/hwtable.c +++ b/libmultipath/hwtable.c @@ -839,6 +839,21 @@ static struct hwentry default_hw[] = { .flush_on_last_del = FLUSH_ENABLED, .dev_loss = 30, }, + { + /* + * Red Hat + * + * Maintainer: Mike Christie + * Mail: mchristi@redhat.com + */ + .vendor = "Ceph", + .product = "RBD", + .pgpolicy = FAILOVER, + .no_path_retry = NO_PATH_RETRY_FAIL, + .checker_name = RBD, + .deferred_remove = DEFERRED_REMOVE_ON, + }, + /* * Tegile Systems */ -- 2.7.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: PATCH 0/4] multipath-tools: Ceph rbd support v2 2016-08-08 12:01 PATCH 0/4] multipath-tools: Ceph rbd support v2 Mike Christie ` (3 preceding siblings ...) 2016-08-08 12:01 ` [PATCH 4/4] multipath-tools: Add rbd to the hwtable Mike Christie @ 2016-08-09 15:36 ` Christophe Varoqui 2016-08-09 18:26 ` Mike Christie 4 siblings, 1 reply; 18+ messages in thread From: Christophe Varoqui @ 2016-08-09 15:36 UTC (permalink / raw) To: Mike Christie; +Cc: device-mapper development [-- Attachment #1.1: Type: text/plain, Size: 1324 bytes --] Merged. I think it is worth advertizing here that the build now requires rados.h. Thanks. On Mon, Aug 8, 2016 at 2:01 PM, Mike Christie <mchristi@redhat.com> wrote: > The following patches made over Christophe's tree today, > add Ceph rbd support for handling blacklisted devices. > > > This is not general support for rbd and multipath. There is > no boot/root support and it does not support features like > multibus. My use is for HA configurations, specifically for exporting > rbd images through multiple LIO instances. In this case, we have one > rbd instance that has the ceph rbd exclusive lock and it can send > WRITE requests. If that host becomes unreachable, then another host > will grab the lock, and blacklist the original host to prevent it from > sending stale IO (when blacklisted IO will be failed by the OSD). > > To recover from that type of scenario, this patchset adds a repair() > callout to the checker. If the path is in the PATH_DOWN state this > callout can be used to fix it up. For my case, I am remapping > the device to flush stale IO and cleanup the old lock, > and then unblacklisting the path, so it can be used again. > > Changes since v1: > - Drop ID_UID use and implemented sysfs getuid support. > - Drop settings that were defaults and follow template. > - Fix ceph auth/user. > > [-- Attachment #1.2: Type: text/html, Size: 1745 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: PATCH 0/4] multipath-tools: Ceph rbd support v2 2016-08-09 15:36 ` PATCH 0/4] multipath-tools: Ceph rbd support v2 Christophe Varoqui @ 2016-08-09 18:26 ` Mike Christie 2016-08-10 7:55 ` Christophe Varoqui 0 siblings, 1 reply; 18+ messages in thread From: Mike Christie @ 2016-08-09 18:26 UTC (permalink / raw) To: Christophe Varoqui; +Cc: device-mapper development [-- Attachment #1: Type: text/plain, Size: 1791 bytes --] On 08/09/2016 10:36 AM, Christophe Varoqui wrote: > Merged. > Thanks. > I think it is worth advertizing here that the build now requires rados.h. > Sorry about adding the silly dependency. How about the attached patch to make the rbd checker only be built if the user requests it. In the documentation for my use, I will describe how to build it. This way majority of users will not be bothered. > Thanks. > > On Mon, Aug 8, 2016 at 2:01 PM, Mike Christie <mchristi@redhat.com> wrote: > >> The following patches made over Christophe's tree today, >> add Ceph rbd support for handling blacklisted devices. >> >> >> This is not general support for rbd and multipath. There is >> no boot/root support and it does not support features like >> multibus. My use is for HA configurations, specifically for exporting >> rbd images through multiple LIO instances. In this case, we have one >> rbd instance that has the ceph rbd exclusive lock and it can send >> WRITE requests. If that host becomes unreachable, then another host >> will grab the lock, and blacklist the original host to prevent it from >> sending stale IO (when blacklisted IO will be failed by the OSD). >> >> To recover from that type of scenario, this patchset adds a repair() >> callout to the checker. If the path is in the PATH_DOWN state this >> callout can be used to fix it up. For my case, I am remapping >> the device to flush stale IO and cleanup the old lock, >> and then unblacklisting the path, so it can be used again. >> >> Changes since v1: >> - Drop ID_UID use and implemented sysfs getuid support. >> - Drop settings that were defaults and follow template. >> - Fix ceph auth/user. >> >> > > > > -- > dm-devel mailing list > dm-devel@redhat.com > https://www.redhat.com/mailman/listinfo/dm-devel > [-- Attachment #2: 0001-PATCH-checker-do-not-build-rbd-by-default.patch --] [-- Type: text/x-patch, Size: 913 bytes --] From 4d0bfdfc032d4fabf0ac08ba105125ded168bd95 Mon Sep 17 00:00:00 2001 From: Mike Christie <mchristi@redhat.com> Date: Tue, 9 Aug 2016 13:19:41 -0500 Subject: [PATCH 1/1] checker: do not build rbd by default rbd is not a common driver and users will probably not have the tools/libs to use it. Make the rbd checker not built by default. Signed-off-by: Mike Christie <mchristi@redhat.com> --- libmultipath/checkers/Makefile | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libmultipath/checkers/Makefile b/libmultipath/checkers/Makefile index 5fd2099..aeb425f 100644 --- a/libmultipath/checkers/Makefile +++ b/libmultipath/checkers/Makefile @@ -11,8 +11,11 @@ LIBS= \ libcheckdirectio.so \ libcheckemc_clariion.so \ libcheckhp_sw.so \ - libcheckrdac.so \ - libcheckrbd.so + libcheckrdac.so + +ifneq ($(CHECKER_RBD),) +LIBS += libcheckrbd.so +endif CFLAGS += -I.. -- 2.7.2 [-- Attachment #3: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: PATCH 0/4] multipath-tools: Ceph rbd support v2 2016-08-09 18:26 ` Mike Christie @ 2016-08-10 7:55 ` Christophe Varoqui 2016-08-10 15:42 ` Bart Van Assche 0 siblings, 1 reply; 18+ messages in thread From: Christophe Varoqui @ 2016-08-10 7:55 UTC (permalink / raw) To: Mike Christie; +Cc: device-mapper development [-- Attachment #1.1: Type: text/plain, Size: 2297 bytes --] Distributors will surely want to build all checkers to have the docs and hwtable in sync with the shipped checkers, and cover the widest range of setups. I'd rather drop this patch or, if it proves to be a problem, merge a variant with the rbd checker build by default and not-buildable by explicit define. But thanks, Christophe On Tue, Aug 9, 2016 at 8:26 PM, Mike Christie <mchristi@redhat.com> wrote: > On 08/09/2016 10:36 AM, Christophe Varoqui wrote: > > Merged. > > > > Thanks. > > > I think it is worth advertizing here that the build now requires rados.h. > > > > Sorry about adding the silly dependency. How about the attached patch to > make the rbd checker only be built if the user requests it. In the > documentation for my use, I will describe how to build it. This way > majority of users will not be bothered. > > > > Thanks. > > > > On Mon, Aug 8, 2016 at 2:01 PM, Mike Christie <mchristi@redhat.com> > wrote: > > > >> The following patches made over Christophe's tree today, > >> add Ceph rbd support for handling blacklisted devices. > >> > >> > >> This is not general support for rbd and multipath. There is > >> no boot/root support and it does not support features like > >> multibus. My use is for HA configurations, specifically for exporting > >> rbd images through multiple LIO instances. In this case, we have one > >> rbd instance that has the ceph rbd exclusive lock and it can send > >> WRITE requests. If that host becomes unreachable, then another host > >> will grab the lock, and blacklist the original host to prevent it from > >> sending stale IO (when blacklisted IO will be failed by the OSD). > >> > >> To recover from that type of scenario, this patchset adds a repair() > >> callout to the checker. If the path is in the PATH_DOWN state this > >> callout can be used to fix it up. For my case, I am remapping > >> the device to flush stale IO and cleanup the old lock, > >> and then unblacklisting the path, so it can be used again. > >> > >> Changes since v1: > >> - Drop ID_UID use and implemented sysfs getuid support. > >> - Drop settings that were defaults and follow template. > >> - Fix ceph auth/user. > >> > >> > > > > > > > > -- > > dm-devel mailing list > > dm-devel@redhat.com > > https://www.redhat.com/mailman/listinfo/dm-devel > > > > [-- Attachment #1.2: Type: text/html, Size: 3347 bytes --] [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: PATCH 0/4] multipath-tools: Ceph rbd support v2 2016-08-10 7:55 ` Christophe Varoqui @ 2016-08-10 15:42 ` Bart Van Assche 0 siblings, 0 replies; 18+ messages in thread From: Bart Van Assche @ 2016-08-10 15:42 UTC (permalink / raw) To: Christophe Varoqui, Mike Christie; +Cc: device-mapper development On 08/10/2016 12:55 AM, Christophe Varoqui wrote: > Distributors will surely want to build all checkers to have the docs and > hwtable in sync with the shipped checkers, and cover the widest range of > setups. > > I'd rather drop this patch or, if it proves to be a problem, merge a > variant with the rbd checker build by default and not-buildable by > explicit define. Hello Mike and Christophe, How about detecting at compile time whether the rbd header files are available? That approach avoids that we have to introduce a configure script and also avoids that the user has to enable rbd support explicitly. In e.g. the fio makefile there are several tests to check which libraries are available and can be used during the fio build. Bart. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/4] multipath-tools: Add rbd to the hwtable @ 2016-07-19 12:28 Xose Vazquez Perez 0 siblings, 0 replies; 18+ messages in thread From: Xose Vazquez Perez @ 2016-07-19 12:28 UTC (permalink / raw) To: Mike Christie, device-mapper development, Christophe Varoqui Mike Christie wrote: > Add rbd to hwtable. These defaults are for the HA type of setup > supported by the checker. We do no support features like multibus > at the dm-multipath level yet. Please use this patch as guide: https://patchwork.kernel.org/patch/9225521/ > Signed-off-by: Mike Christie <mchristi@redhat.com> > --- > libmultipath/hwtable.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c > index 6116124..27a6ff1 100644 > --- a/libmultipath/hwtable.c > +++ b/libmultipath/hwtable.c > @@ -1206,6 +1206,21 @@ static struct hwentry default_hw[] = { > .pgfailback = 30, > .minio = 128, > }, > + { > + .vendor = "Ceph", > + .product = "RBD", > + .features = DEFAULT_FEATURES, > + .hwhandler = DEFAULT_HWHANDLER, > + .pgpolicy = FAILOVER, > + .pgfailback = -FAILBACK_IMMEDIATE, > + .no_path_retry = NO_PATH_RETRY_FAIL, > + .checker_name = RBD, > + .user_friendly_names = USER_FRIENDLY_NAMES_ON, USER_FRIENDLY_NAMES_ON is highly inadvisable. > + .uid_attribute = "ID_UID", > + .prio_name = PRIO_CONST, > + .deferred_remove = DEFERRED_REMOVE_ON, ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 0/4] multipath-tools: Ceph rbd support @ 2016-07-05 8:12 Mike Christie 2016-07-05 8:12 ` [PATCH 4/4] multipath-tools: Add rbd to the hwtable Mike Christie 0 siblings, 1 reply; 18+ messages in thread From: Mike Christie @ 2016-07-05 8:12 UTC (permalink / raw) To: dm-devel, christophe.varoqui The following patches add Ceph rbd support for handling blacklisted devices. This does not support features like multibus. My specific use is for exporting rbd images through multiple LIO instances. In this case, we have one rbd instance that has the exclusive lock and send WRITE requests. If that host becomes unreachable, then another host will grab the lock, and blacklist the original host to prevent it from sending stale IO (when blacklisted IO will be failed by the OSD). To recover from this, this patchset adds a repair() callout to the checker. If the path is in the PATH_DOWN state this callout can be used to fix it up. For my case, I am remapping the device to flush stale IO and cleanup the old lock, and then unblacklisting myself. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 4/4] multipath-tools: Add rbd to the hwtable 2016-07-05 8:12 [PATCH 0/4] multipath-tools: Ceph rbd support Mike Christie @ 2016-07-05 8:12 ` Mike Christie 0 siblings, 0 replies; 18+ messages in thread From: Mike Christie @ 2016-07-05 8:12 UTC (permalink / raw) To: dm-devel, christophe.varoqui; +Cc: Mike Christie Add rbd to hwtable. These defaults are for the HA type of setup supported by the checker. We do no support features like multibus at the dm-multipath level yet. Signed-off-by: Mike Christie <mchristi@redhat.com> --- libmultipath/hwtable.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c index 6116124..27a6ff1 100644 --- a/libmultipath/hwtable.c +++ b/libmultipath/hwtable.c @@ -1206,6 +1206,21 @@ static struct hwentry default_hw[] = { .pgfailback = 30, .minio = 128, }, + { + .vendor = "Ceph", + .product = "RBD", + .features = DEFAULT_FEATURES, + .hwhandler = DEFAULT_HWHANDLER, + .pgpolicy = FAILOVER, + .pgfailback = -FAILBACK_IMMEDIATE, + .no_path_retry = NO_PATH_RETRY_FAIL, + .checker_name = RBD, + .user_friendly_names = USER_FRIENDLY_NAMES_ON, + .uid_attribute = "ID_UID", + .prio_name = PRIO_CONST, + .deferred_remove = DEFERRED_REMOVE_ON, + }, + /* * EOL */ -- 2.5.5 ^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2016-08-15 16:24 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-08-08 12:01 PATCH 0/4] multipath-tools: Ceph rbd support v2 Mike Christie 2016-08-08 12:01 ` [PATCH 1/4] libmultipath: add rbd discovery Mike Christie 2016-08-08 12:01 ` [PATCH 2/4] multipath-tools: add checker callout to repair path Mike Christie 2016-08-11 15:50 ` Bart Van Assche 2016-08-11 20:33 ` Mike Christie 2016-08-11 21:41 ` Bart Van Assche 2016-08-12 16:54 ` Mike Christie 2016-08-12 17:10 ` Bart Van Assche 2016-08-14 8:41 ` Mike Christie 2016-08-15 16:24 ` Bart Van Assche 2016-08-08 12:01 ` [PATCH 3/4] multipath-tools: Add rbd checker Mike Christie 2016-08-08 12:01 ` [PATCH 4/4] multipath-tools: Add rbd to the hwtable Mike Christie 2016-08-09 15:36 ` PATCH 0/4] multipath-tools: Ceph rbd support v2 Christophe Varoqui 2016-08-09 18:26 ` Mike Christie 2016-08-10 7:55 ` Christophe Varoqui 2016-08-10 15:42 ` Bart Van Assche -- strict thread matches above, loose matches on Subject: below -- 2016-07-19 12:28 [PATCH 4/4] multipath-tools: Add rbd to the hwtable Xose Vazquez Perez 2016-07-05 8:12 [PATCH 0/4] multipath-tools: Ceph rbd support Mike Christie 2016-07-05 8:12 ` [PATCH 4/4] multipath-tools: Add rbd to the hwtable Mike Christie
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.