STRBUF_ON_STACK() uses the cleanup __attribute__, which doesn't get run if a thread is cancelled. condlog() will call fprintf() when run under systemd, which is a cancellation point. The snprint function for the generic mutipath and generic path operations both call cancellation points. Also, the keyword print functions can call cancellation points. Because of all this, I did not see any safe uses of STRBUF_ON_STACK() outside of the unit tests. Replace them all with pthread cleanup handlers.
Signed-off-by: Benjamin Marzinski <[email protected]> --- libmpathutil/parser.c | 5 +- libmpathutil/strbuf.h | 4 +- libmultipath/alias.c | 59 +++++++++++------- libmultipath/blacklist.c | 4 +- libmultipath/discovery.c | 34 +++++++---- libmultipath/dmparser.c | 21 +++---- libmultipath/foreign.c | 4 +- libmultipath/generic.c | 10 +++- libmultipath/print.c | 50 ++++++++++------ libmultipath/prioritizers/weightedpath.c | 16 ++--- libmultipath/propsel.c | 76 ++++++++++++++++++------ libmultipath/sysfs.h | 11 +--- libmultipath/uevent.c | 6 +- 13 files changed, 195 insertions(+), 105 deletions(-) diff --git a/libmpathutil/parser.c b/libmpathutil/parser.c index 8d3ac53a..04c65b51 100644 --- a/libmpathutil/parser.c +++ b/libmpathutil/parser.c @@ -25,6 +25,7 @@ #include "parser.h" #include "debug.h" #include "strbuf.h" +#include "util.h" /* local vars */ static int sublevel = 0; @@ -155,11 +156,12 @@ snprint_keyword(struct strbuf *buff, const char *fmt, struct keyword *kw, int r = 0; char *f; struct config *conf; - STRBUF_ON_STACK(sbuf); + struct strbuf sbuf = STRBUF_INIT; if (!kw || !kw->print) return 0; + pthread_cleanup_push_cast(reset_strbuf, &sbuf); do { f = strchr(fmt, '%'); if (f == NULL) { @@ -193,6 +195,7 @@ out: if (r >= 0) r = __append_strbuf_str(buff, get_strbuf_str(&sbuf), get_strbuf_len(&sbuf)); + pthread_cleanup_pop(1); return r; } diff --git a/libmpathutil/strbuf.h b/libmpathutil/strbuf.h index ae863417..5aa54677 100644 --- a/libmpathutil/strbuf.h +++ b/libmpathutil/strbuf.h @@ -42,7 +42,9 @@ void free_strbuf(struct strbuf *buf); * macro: STRBUF_ON_STACK * * Define and initialize a local struct @strbuf to be cleaned up when - * the current scope is left + * the current scope is left. This can only be used in non-threaded + * programs, or if there are no pthread cancellation points in the + * current scope, after the buffer could first be used. */ #define STRBUF_ON_STACK(__x) \ struct strbuf __attribute__((cleanup(reset_strbuf))) (__x) = STRBUF_INIT; diff --git a/libmultipath/alias.c b/libmultipath/alias.c index 05201224..3a81a7ad 100644 --- a/libmultipath/alias.c +++ b/libmultipath/alias.c @@ -112,12 +112,14 @@ scan_devname(const char *alias, const char *prefix) static int id_already_taken(int id, const char *prefix, const char *map_wwid) { - STRBUF_ON_STACK(buf); + struct strbuf buf = STRBUF_INIT; const char *alias; + int r = 0; + pthread_cleanup_push_cast(reset_strbuf, &buf); if (append_strbuf_str(&buf, prefix) < 0 || format_devname(&buf, id) < 0) - return 0; + goto out; alias = get_strbuf_str(&buf); if (dm_map_present(alias)) { @@ -126,11 +128,13 @@ id_already_taken(int id, const char *prefix, const char *map_wwid) /* If both the name and the wwid match, then it's fine.*/ if (dm_get_uuid(alias, wwid, sizeof(wwid)) == 0 && strncmp(map_wwid, wwid, sizeof(wwid)) == 0) - return 0; + goto out; condlog(3, "%s: alias '%s' already taken, but not in bindings file. reselecting alias", map_wwid, alias); - return 1; + r = 1; } - return 0; +out: + pthread_cleanup_pop(1); + return r; } @@ -277,10 +281,12 @@ rlookup_binding(FILE *f, char *buff, const char *map_alias) static char * allocate_binding(int fd, const char *wwid, int id, const char *prefix) { - STRBUF_ON_STACK(buf); + struct strbuf buf = STRBUF_INIT; off_t offset; ssize_t len; - char *alias, *c; + char *alias = NULL; + const char *str; + int end; if (id <= 0) { condlog(0, "%s: cannot allocate new binding for id %d", @@ -288,38 +294,41 @@ allocate_binding(int fd, const char *wwid, int id, const char *prefix) return NULL; } + pthread_cleanup_push_cast(reset_strbuf, &buf); if (append_strbuf_str(&buf, prefix) < 0 || format_devname(&buf, id) == -1) - return NULL; + goto out; if (print_strbuf(&buf, " %s\n", wwid) < 0) - return NULL; + goto out; offset = lseek(fd, 0, SEEK_END); if (offset < 0){ condlog(0, "Cannot seek to end of bindings file : %s", strerror(errno)); - return NULL; + goto out; } len = get_strbuf_len(&buf); - alias = steal_strbuf_str(&buf); + str = get_strbuf_str(&buf); - if (write(fd, alias, len) != len) { + if (write(fd, str, len) != len) { condlog(0, "Cannot write binding to bindings file : %s", strerror(errno)); /* clear partial write */ if (ftruncate(fd, offset)) condlog(0, "Cannot truncate the header : %s", strerror(errno)); - free(alias); - return NULL; + goto out; } - c = strchr(alias, ' '); - if (c) - *c = '\0'; + end = strcspn(str, " "); - condlog(3, "Created new binding [%s] for WWID [%s]", alias, wwid); + condlog(3, "Created new binding [%.*s] for WWID [%s]", end, str, wwid); + alias = steal_strbuf_str(&buf); + + alias[len] = '\0'; +out: + pthread_cleanup_pop(1); return alias; } @@ -549,24 +558,28 @@ static int add_binding(Bindings *bindings, const char *alias, const char *wwid) static int write_bindings_file(const Bindings *bindings, int fd) { struct binding *bnd; - STRBUF_ON_STACK(line); - int i; + struct strbuf line = STRBUF_INIT; + int i, r = -1; if (write(fd, BINDINGS_FILE_HEADER, sizeof(BINDINGS_FILE_HEADER) - 1) != sizeof(BINDINGS_FILE_HEADER) - 1) return -1; + pthread_cleanup_push_cast(reset_strbuf, &line); vector_foreach_slot(bindings, bnd, i) { int len; if ((len = print_strbuf(&line, "%s %s\n", bnd->alias, bnd->wwid)) < 0) - return -1; + goto out; if (write(fd, get_strbuf_str(&line), len) != len) - return -1; + goto out; truncate_strbuf(&line, 0); } - return 0; + r = 0; +out: + pthread_cleanup_pop(1); + return r; } static int fix_bindings_file(const struct config *conf, diff --git a/libmultipath/blacklist.c b/libmultipath/blacklist.c index 8d15d2ea..7e0b036a 100644 --- a/libmultipath/blacklist.c +++ b/libmultipath/blacklist.c @@ -341,11 +341,12 @@ int filter_protocol(const struct _vector *blist, const struct _vector *elist, const struct path *pp) { - STRBUF_ON_STACK(buf); + struct strbuf buf = STRBUF_INIT; const char *prot; int r = MATCH_NOTHING; if (pp) { + pthread_cleanup_push_cast(reset_strbuf, &buf); snprint_path_protocol(&buf, pp); prot = get_strbuf_str(&buf); @@ -354,6 +355,7 @@ filter_protocol(const struct _vector *blist, const struct _vector *elist, else if (match_reglist(blist, prot)) r = MATCH_PROTOCOL_BLIST; log_filter(pp->dev, NULL, NULL, NULL, NULL, prot, r, 3); + pthread_cleanup_pop(1); } return r; diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c index f3fccedd..32181259 100644 --- a/libmultipath/discovery.c +++ b/libmultipath/discovery.c @@ -838,13 +838,15 @@ static void scsi_tmo_error_msg(struct path *pp) { STATIC_BITFIELD(bf, LAST_BUS_PROTOCOL_ID + 1); - STRBUF_ON_STACK(proto_buf); + struct strbuf proto_buf = STRBUF_INIT; unsigned int proto_id = bus_protocol_id(pp); + pthread_cleanup_push_cast(reset_strbuf, &proto_buf); snprint_path_protocol(&proto_buf, pp); condlog(2, "%s: setting scsi timeouts is unsupported for protocol %s", pp->dev, get_strbuf_str(&proto_buf)); set_bit_in_bitfield(proto_id, bf); + pthread_cleanup_pop(1); } int @@ -1142,12 +1144,13 @@ parse_vpd_pg83(const unsigned char *in, size_t in_len, size_t len, vpd_len, i; int vpd_type, prio = -1; int err = -ENODATA; - STRBUF_ON_STACK(buf); + struct strbuf buf = STRBUF_INIT; /* Need space at least for one digit */ if (out_len <= 1) return 0; + pthread_cleanup_push_cast(reset_strbuf, &buf); d = in + 4; while (d <= in + in_len - 4) { bool invalid = false; @@ -1241,8 +1244,9 @@ parse_vpd_pg83(const unsigned char *in, size_t in_len, } if (prio <= 0) - return err; + goto out; + err = 0; if (d != in + in_len) /* Should this be fatal? (overflow covered above) */ condlog(2, "%s: warning: last descriptor end %zd != VPD length %zu", @@ -1262,10 +1266,10 @@ parse_vpd_pg83(const unsigned char *in, size_t in_len, size_t i; if ((err = print_strbuf(&buf, "%d", vpd_type)) < 0) - return err; + goto out; for (i = 0; i < vpd_len; i++) if ((err = print_strbuf(&buf, "%02x", vpd[i])) < 0) - return err; + goto out; } else if (vpd_type == 0x8) { char type; @@ -1276,12 +1280,12 @@ parse_vpd_pg83(const unsigned char *in, size_t in_len, else type = '8'; if ((err = fill_strbuf(&buf, type, 1)) < 0) - return err; + goto out; vpd += 4; len = vpd_len - 4; if ((err = __append_strbuf_str(&buf, (const char *)vpd, len)) < 0) - return err; + goto out; /* The input is 0-padded, make sure the length is correct */ truncate_strbuf(&buf, strlen(get_strbuf_str(&buf))); @@ -1298,12 +1302,12 @@ parse_vpd_pg83(const unsigned char *in, size_t in_len, size_t p_len; if ((err = fill_strbuf(&buf, '1', 1)) < 0) - return err; + goto out; while (vpd && (p = memchr(vpd, ' ', vpd_len))) { p_len = p - vpd; if ((err = __append_strbuf_str(&buf, (const char *)vpd, p_len)) < 0) - return err; + goto out; vpd = p; vpd_len -= p_len; while (vpd && vpd_len > 0 && *vpd == ' ') { @@ -1311,12 +1315,12 @@ parse_vpd_pg83(const unsigned char *in, size_t in_len, vpd_len --; } if (vpd_len > 0 && (err = fill_strbuf(&buf, '_', 1)) < 0) - return err; + goto out; } if (vpd_len > 0) { if ((err = __append_strbuf_str(&buf, (const char *)vpd, vpd_len)) < 0) - return err; + goto out; } } @@ -1331,6 +1335,10 @@ parse_vpd_pg83(const unsigned char *in, size_t in_len, len = out_len - 1; } strlcpy(out, get_strbuf_str(&buf), len + 1); +out: + pthread_cleanup_pop(1); + if (err) + return err; return len; } @@ -2435,11 +2443,13 @@ int pathinfo(struct path *pp, struct config *conf, int mask) * It's likely that this path is not fit for * multipath use. */ - STRBUF_ON_STACK(buf); + struct strbuf buf = STRBUF_INIT; + pthread_cleanup_push_cast(reset_strbuf, &buf); snprint_path(&buf, "%T", pp, 0); condlog(1, "%s: no WWID in state \"%s\", giving up", pp->dev, get_strbuf_str(&buf)); + pthread_cleanup_pop(1); return PATHINFO_SKIPPED; } return PATHINFO_OK; diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c index 3b37a926..44650aaa 100644 --- a/libmultipath/dmparser.c +++ b/libmultipath/dmparser.c @@ -48,10 +48,10 @@ int assemble_map(struct multipath *mp, char **params) { static const char no_path_retry[] = "queue_if_no_path"; static const char retain_hwhandler[] = "retain_attached_hw_handler"; - int i, j; + int i, j, r = 1; int minio; int nr_priority_groups, initial_pg_nr; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; struct pathgroup * pgp; struct path * pp; @@ -68,15 +68,16 @@ int assemble_map(struct multipath *mp, char **params) get_linux_version_code() < KERNEL_VERSION(4, 3, 0)) add_feature(&mp->features, retain_hwhandler); + pthread_cleanup_push_cast(reset_strbuf, &buff); if (print_strbuf(&buff, "%s %s %i %i", mp->features, mp->hwhandler, nr_priority_groups, initial_pg_nr) < 0) - goto err; + goto out; vector_foreach_slot (mp->pg, pgp, i) { pgp = VECTOR_SLOT(mp->pg, i); if (print_strbuf(&buff, " %s %i 1", mp->selector, VECTOR_SIZE(pgp->paths)) < 0) - goto err; + goto out; vector_foreach_slot (pgp->paths, pp, j) { int tmp_minio = minio; @@ -86,19 +87,19 @@ int assemble_map(struct multipath *mp, char **params) tmp_minio = minio * pp->priority; if (!strlen(pp->dev_t) ) { condlog(0, "dev_t not set for '%s'", pp->dev); - goto err; + goto out; } if (print_strbuf(&buff, " %s %d", pp->dev_t, tmp_minio) < 0) - goto err; + goto out; } } *params = steal_strbuf_str(&buff); condlog(4, "%s: assembled map [%s]", mp->alias, *params); - return 0; - -err: - return 1; + r = 0; +out: + pthread_cleanup_pop(1); + return r; } /* diff --git a/libmultipath/foreign.c b/libmultipath/foreign.c index d01a5ef0..8981ff58 100644 --- a/libmultipath/foreign.c +++ b/libmultipath/foreign.c @@ -547,7 +547,7 @@ int snprint_foreign_topology(struct strbuf *buf, int verbosity, void print_foreign_topology(int verbosity) { - STRBUF_ON_STACK(buf); + struct strbuf buf = STRBUF_INIT; struct foreign *fgn; int i; fieldwidth_t *width __attribute__((cleanup(cleanup_ucharp))) = NULL; @@ -559,6 +559,7 @@ void print_foreign_topology(int verbosity) unlock_foreigns(NULL); return; } + pthread_cleanup_push_cast(reset_strbuf, &buf); pthread_cleanup_push(unlock_foreigns, NULL); vector_foreach_slot(foreigns, fgn, i) { const struct _vector *vec; @@ -573,6 +574,7 @@ void print_foreign_topology(int verbosity) __snprint_foreign_topology(&buf, verbosity, width); pthread_cleanup_pop(1); printf("%s", get_strbuf_str(&buf)); + pthread_cleanup_pop(1); } int snprint_foreign_paths(struct strbuf *buf, const char *style, diff --git a/libmultipath/generic.c b/libmultipath/generic.c index e7cf2975..3e2268e6 100644 --- a/libmultipath/generic.c +++ b/libmultipath/generic.c @@ -23,15 +23,19 @@ int generic_style(const struct gen_multipath* gm, struct strbuf *buf, __attribute__((unused)) int verbosity) { - STRBUF_ON_STACK(tmp); + struct strbuf tmp = STRBUF_INIT; char *alias_buf __attribute__((cleanup(cleanup_charp))); const char *wwid_buf; + int ret; + pthread_cleanup_push_cast(reset_strbuf, &tmp); gm->ops->snprint(gm, &tmp, 'n'); alias_buf = steal_strbuf_str(&tmp); gm->ops->snprint(gm, &tmp, 'w'); wwid_buf = get_strbuf_str(&tmp); - return print_strbuf(buf, "%%n %s[%%G]:%%d %%s", - strcmp(alias_buf, wwid_buf) ? "(%w) " : ""); + ret = print_strbuf(buf, "%%n %s[%%G]:%%d %%s", + strcmp(alias_buf, wwid_buf) ? "(%w) " : ""); + pthread_cleanup_pop(1); + return ret; } diff --git a/libmultipath/print.c b/libmultipath/print.c index d7d522c8..97f9a177 100644 --- a/libmultipath/print.c +++ b/libmultipath/print.c @@ -911,19 +911,21 @@ void _get_path_layout (const struct _vector *gpvec, enum layout_reset reset, return; for (j = 0; j < ARRAY_SIZE(pd); j++) { - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; reset_width(&width[j], reset, pd[j].header); if (gpvec == NULL) continue; + pthread_cleanup_push_cast(reset_strbuf, &buff); vector_foreach_slot (gpvec, gp, i) { gp->ops->snprint(gp, &buff, pd[j].wildcard); width[j] = MAX(width[j], MIN(get_strbuf_len(&buff), MAX_FIELD_WIDTH)); truncate_strbuf(&buff, 0); } + pthread_cleanup_pop(1); } } @@ -951,19 +953,21 @@ _get_multipath_layout (const struct _vector *gmvec, enum layout_reset reset, if (width == NULL) return; for (j = 0; j < ARRAY_SIZE(mpd); j++) { - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; reset_width(&width[j], reset, mpd[j].header); if (gmvec == NULL) continue; + pthread_cleanup_push_cast(reset_strbuf, &buff); vector_foreach_slot (gmvec, gm, i) { gm->ops->snprint(gm, &buff, mpd[j].wildcard); width[j] = MAX(width[j], MIN(get_strbuf_len(&buff), MAX_FIELD_WIDTH)); truncate_strbuf(&buff, 0); } + pthread_cleanup_pop(1); condlog(4, "%s: width %d", mpd[j].header, width[j]); } } @@ -1182,7 +1186,7 @@ int _snprint_pathgroup(const struct gen_pathgroup *ggp, struct strbuf *line, void _print_multipath_topology(const struct gen_multipath *gmp, int verbosity) { - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; fieldwidth_t *p_width __attribute__((cleanup(cleanup_ucharp))) = NULL; const struct gen_pathgroup *gpg; const struct _vector *pgvec, *pathvec; @@ -1202,8 +1206,10 @@ void _print_multipath_topology(const struct gen_multipath *gmp, int verbosity) gmp->ops->rel_pathgroups(gmp, pgvec); } + pthread_cleanup_push_cast(reset_strbuf, &buff); _snprint_multipath_topology(gmp, &buff, verbosity, p_width); printf("%s", get_strbuf_str(&buff)); + pthread_cleanup_pop(1); } int snprint_multipath_style(const struct gen_multipath *gmp, @@ -1225,10 +1231,10 @@ int _snprint_multipath_topology(const struct gen_multipath *gmp, struct strbuf *buff, int verbosity, const fieldwidth_t *p_width) { - int j, i, rc; + int j, i, rc = 0; const struct _vector *pgvec; const struct gen_pathgroup *gpg; - STRBUF_ON_STACK(style); + struct strbuf style = STRBUF_INIT; size_t initial_len = get_strbuf_len(buff); fieldwidth_t *width __attribute__((cleanup(cleanup_ucharp))) = NULL; @@ -1241,17 +1247,21 @@ int _snprint_multipath_topology(const struct gen_multipath *gmp, if (verbosity == 1) return _snprint_multipath(gmp, buff, "%n", width); + pthread_cleanup_push_cast(reset_strbuf, &style); if(isatty(1) && (rc = print_strbuf(&style, "%c[%dm", 0x1B, 1)) < 0) /* bold on */ - return rc; + goto err; if ((rc = gmp->ops->style(gmp, &style, verbosity)) < 0) - return rc; + goto err; if(isatty(1) && (rc = print_strbuf(&style, "%c[%dm", 0x1B, 0)) < 0) /* bold off */ - return rc; + goto err; - if ((rc = _snprint_multipath(gmp, buff, get_strbuf_str(&style), width)) < 0 - || (rc = _snprint_multipath(gmp, buff, PRINT_MAP_PROPS, width)) < 0) + if ((rc = _snprint_multipath(gmp, buff, get_strbuf_str(&style), width)) >= 0) + rc = _snprint_multipath(gmp, buff, PRINT_MAP_PROPS, width); +err: + pthread_cleanup_pop(1); + if (rc < 0) return rc; pgvec = gmp->ops->get_pathgroups(gmp); @@ -1868,7 +1878,7 @@ int __snprint_config(const struct config *conf, struct strbuf *buff, { int rc; - if ((rc = snprint_defaults(conf, buff)) < 0 || + if ((rc = snprint_defaults(conf, buff)) < 1 || (rc = snprint_blacklist(conf, buff)) < 0 || (rc = snprint_blacklist_except(conf, buff)) < 0 || (rc = snprint_hwtable(conf, buff, @@ -1887,17 +1897,21 @@ int __snprint_config(const struct config *conf, struct strbuf *buff, char *snprint_config(const struct config *conf, int *len, const struct _vector *hwtable, const struct _vector *mpvec) { - STRBUF_ON_STACK(buff); - char *reply; - int rc = __snprint_config(conf, &buff, hwtable, mpvec); + struct strbuf buff = STRBUF_INIT; + char *reply = NULL; + int rc; + + pthread_cleanup_push_cast(reset_strbuf, &buff); + rc = __snprint_config(conf, &buff, hwtable, mpvec); if (rc < 0) - return NULL; + goto out; if (len) *len = get_strbuf_len(&buff); reply = steal_strbuf_str(&buff); - +out: + pthread_cleanup_pop(1); return reply; } @@ -2012,7 +2026,7 @@ static void print_all_paths_custo(vector pathvec, int banner, const char *fmt) { int i; struct path * pp; - STRBUF_ON_STACK(line); + struct strbuf line = STRBUF_INIT; fieldwidth_t *width __attribute__((cleanup(cleanup_ucharp))) = NULL; if (!VECTOR_SIZE(pathvec)) { @@ -2025,6 +2039,7 @@ static void print_all_paths_custo(vector pathvec, int banner, const char *fmt) return; get_path_layout(pathvec, 1, width); + pthread_cleanup_push_cast(reset_strbuf, &line); if (banner) append_strbuf_str(&line, "===== paths list =====\n"); @@ -2034,6 +2049,7 @@ static void print_all_paths_custo(vector pathvec, int banner, const char *fmt) snprint_path(&line, fmt, pp, width); printf("%s", get_strbuf_str(&line)); + pthread_cleanup_pop(1); } void print_all_paths(vector pathvec, int banner) diff --git a/libmultipath/prioritizers/weightedpath.c b/libmultipath/prioritizers/weightedpath.c index 561ebb48..df700bf3 100644 --- a/libmultipath/prioritizers/weightedpath.c +++ b/libmultipath/prioritizers/weightedpath.c @@ -63,7 +63,7 @@ build_wwn_path(struct path *pp, struct strbuf *buf) /* main priority routine */ int prio_path_weight(struct path *pp, char *prio_args) { - STRBUF_ON_STACK(path); + struct strbuf path = STRBUF_INIT; char *arg __attribute__((cleanup(cleanup_charp))) = NULL; char *temp, *regex, *prio; char split_char[] = " \t"; @@ -84,24 +84,25 @@ int prio_path_weight(struct path *pp, char *prio_args) return priority; } + pthread_cleanup_push_cast(reset_strbuf, &path); if (!strcmp(regex, HBTL)) { if (print_strbuf(&path, "%d:%d:%d:%" PRIu64, pp->sg_id.host_no, pp->sg_id.channel, pp->sg_id.scsi_id, pp->sg_id.lun) < 0) - return priority; + goto out; } else if (!strcmp(regex, DEV_NAME)) { if (append_strbuf_str(&path, pp->dev) < 0) - return priority; + goto out; } else if (!strcmp(regex, SERIAL)) { if (build_serial_path(pp, &path) != 0) - return priority; + goto out; } else if (!strcmp(regex, WWN)) { if (build_wwn_path(pp, &path) != 0) - return priority; + goto out; } else { condlog(0, "%s: %s - Invalid arguments", pp->dev, pp->prio.name); - return priority; + goto out; } while (!path_found) { @@ -121,7 +122,8 @@ int prio_path_weight(struct path *pp, char *prio_args) regfree(&pathe); } } - +out: + pthread_cleanup_pop(1); return priority; } diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c index d4f19897..f35acdaa 100644 --- a/libmultipath/propsel.c +++ b/libmultipath/propsel.c @@ -217,7 +217,7 @@ out: int select_rr_weight(struct config *conf, struct multipath * mp) { const char *origin; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; mp_set_mpe(rr_weight); mp_set_ovr(rr_weight); @@ -225,16 +225,18 @@ int select_rr_weight(struct config *conf, struct multipath * mp) mp_set_conf(rr_weight); mp_set_default(rr_weight, DEFAULT_RR_WEIGHT); out: + pthread_cleanup_push_cast(reset_strbuf, &buff); print_rr_weight(&buff, mp->rr_weight); condlog(3, "%s: rr_weight = %s %s", mp->alias, get_strbuf_str(&buff), origin); + pthread_cleanup_pop(1); return 0; } int select_pgfailback(struct config *conf, struct multipath * mp) { const char *origin; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; mp_set_mpe(pgfailback); mp_set_ovr(pgfailback); @@ -242,9 +244,11 @@ int select_pgfailback(struct config *conf, struct multipath * mp) mp_set_conf(pgfailback); mp_set_default(pgfailback, DEFAULT_FAILBACK); out: + pthread_cleanup_push_cast(reset_strbuf, &buff); print_pgfailback(&buff, mp->pgfailback); condlog(3, "%s: failback = %s %s", mp->alias, get_strbuf_str(&buff), origin); + pthread_cleanup_pop(1); return 0; } @@ -367,7 +371,7 @@ void reconcile_features_with_options(const char *id, char **features, int* no_pa { static const char q_i_n_p[] = "queue_if_no_path"; static const char r_a_h_h[] = "retain_attached_hw_handler"; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; if (*features == NULL) return; @@ -382,6 +386,7 @@ void reconcile_features_with_options(const char *id, char **features, int* no_pa * For backward compatibility we allow 'features "1 queue_if_no_path"'; * it's translated into "no_path_retry queue" here. */ + pthread_cleanup_push_cast(reset_strbuf, &buff); if (strstr(*features, q_i_n_p)) { condlog(0, "%s: option 'features \"1 %s\"' is deprecated, " "please use 'no_path_retry queue' instead", @@ -400,6 +405,7 @@ void reconcile_features_with_options(const char *id, char **features, int* no_pa } remove_feature(features, q_i_n_p); } + pthread_cleanup_pop(1); if (strstr(*features, r_a_h_h)) { condlog(0, "%s: option 'features \"1 %s\"' is deprecated", id, r_a_h_h); @@ -779,7 +785,7 @@ out: int select_no_path_retry(struct config *conf, struct multipath *mp) { const char *origin = NULL; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; if (mp->disable_queueing) { condlog(0, "%s: queueing disabled", mp->alias); @@ -791,6 +797,7 @@ int select_no_path_retry(struct config *conf, struct multipath *mp) mp_set_hwe(no_path_retry); mp_set_conf(no_path_retry); out: + pthread_cleanup_push_cast(reset_strbuf, &buff); print_no_path_retry(&buff, mp->no_path_retry); if (origin) condlog(3, "%s: no_path_retry = %s %s", mp->alias, @@ -798,6 +805,7 @@ out: else condlog(3, "%s: no_path_retry = undef %s", mp->alias, default_origin); + pthread_cleanup_pop(1); return 0; } @@ -845,23 +853,25 @@ int select_minio(struct config *conf, struct multipath *mp) int select_fast_io_fail(struct config *conf, struct path *pp) { const char *origin; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; pp_set_ovr_pce(fast_io_fail); pp_set_hwe(fast_io_fail); pp_set_conf(fast_io_fail); pp_set_default(fast_io_fail, DEFAULT_FAST_IO_FAIL); out: + pthread_cleanup_push_cast(reset_strbuf, &buff); print_undef_off_zero(&buff, pp->fast_io_fail); condlog(3, "%s: fast_io_fail_tmo = %s %s", pp->dev, get_strbuf_str(&buff), origin); + pthread_cleanup_pop(1); return 0; } int select_dev_loss(struct config *conf, struct path *pp) { const char *origin; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; pp_set_ovr_pce(dev_loss); pp_set_hwe(dev_loss); @@ -869,16 +879,18 @@ int select_dev_loss(struct config *conf, struct path *pp) pp->dev_loss = DEV_LOSS_TMO_UNSET; return 0; out: + pthread_cleanup_push_cast(reset_strbuf, &buff); print_dev_loss(&buff, pp->dev_loss); condlog(3, "%s: dev_loss_tmo = %s %s", pp->dev, get_strbuf_str(&buff), origin); + pthread_cleanup_pop(1); return 0; } int select_eh_deadline(struct config *conf, struct path *pp) { const char *origin; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; pp_set_ovr_pce(eh_deadline); pp_set_hwe(eh_deadline); @@ -887,9 +899,11 @@ int select_eh_deadline(struct config *conf, struct path *pp) /* not changing sysfs in default cause, so don't print anything */ return 0; out: + pthread_cleanup_push_cast(reset_strbuf, &buff); print_undef_off_zero(&buff, pp->eh_deadline); condlog(3, "%s: eh_deadline = %s %s", pp->dev, get_strbuf_str(&buff), origin); + pthread_cleanup_pop(1); return 0; } @@ -911,7 +925,7 @@ out: int select_reservation_key(struct config *conf, struct multipath *mp) { const char *origin; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; char *from_file = ""; uint64_t prkey = 0; @@ -929,10 +943,12 @@ out: else put_be64(mp->reservation_key, prkey); } + pthread_cleanup_push_cast(reset_strbuf, &buff); print_reservation_key(&buff, mp->reservation_key, mp->sa_flags, mp->prkey_source); condlog(3, "%s: reservation_key = %s %s%s", mp->alias, get_strbuf_str(&buff), origin, from_file); + pthread_cleanup_pop(1); return 0; } @@ -1029,16 +1045,18 @@ use_delay_watch_checks(struct config *conf, struct multipath *mp) { int value = NU_UNDEF; const char *origin = default_origin; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; do_set(delay_watch_checks, mp->mpe, value, multipaths_origin); do_set(delay_watch_checks, conf->overrides, value, overrides_origin); do_set_from_hwe(delay_watch_checks, mp, value, hwe_origin); do_set(delay_watch_checks, conf, value, conf_origin); out: + pthread_cleanup_push_cast(reset_strbuf, &buff); if (print_off_int_undef(&buff, value) > 0) condlog(3, "%s: delay_watch_checks = %s %s", mp->alias, get_strbuf_str(&buff), origin); + pthread_cleanup_pop(1); return value; } @@ -1047,23 +1065,25 @@ use_delay_wait_checks(struct config *conf, struct multipath *mp) { int value = NU_UNDEF; const char *origin = default_origin; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; do_set(delay_wait_checks, mp->mpe, value, multipaths_origin); do_set(delay_wait_checks, conf->overrides, value, overrides_origin); do_set_from_hwe(delay_wait_checks, mp, value, hwe_origin); do_set(delay_wait_checks, conf, value, conf_origin); out: + pthread_cleanup_push_cast(reset_strbuf, &buff); if (print_off_int_undef(&buff, value) > 0) condlog(3, "%s: delay_wait_checks = %s %s", mp->alias, get_strbuf_str(&buff), origin); + pthread_cleanup_pop(1); return value; } int select_delay_checks(struct config *conf, struct multipath *mp) { int watch_checks, wait_checks; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; watch_checks = use_delay_watch_checks(conf, mp); wait_checks = use_delay_wait_checks(conf, mp); @@ -1077,6 +1097,7 @@ int select_delay_checks(struct config *conf, struct multipath *mp) mp->san_path_err_threshold = 1; condlog(3, "%s: san_path_err_threshold = 1 %s", mp->alias, (watch_checks > 0)? delay_watch_origin : delay_wait_origin); + pthread_cleanup_push_cast(reset_strbuf, &buff); if (watch_checks > 0) { mp->san_path_err_forget_rate = watch_checks; print_off_int_undef(&buff, mp->san_path_err_forget_rate); @@ -1091,6 +1112,7 @@ int select_delay_checks(struct config *conf, struct multipath *mp) condlog(3, "%s: san_path_err_recovery_time = %s %s", mp->alias, get_strbuf_str(&buff), delay_wait_origin); } + pthread_cleanup_pop(1); return 0; } @@ -1108,7 +1130,7 @@ static int san_path_deprecated_warned; int select_san_path_err_threshold(struct config *conf, struct multipath *mp) { const char *origin; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; if (marginal_path_check_enabled(mp) || (conf->marginal_pathgroups == MARGINAL_PATHGROUP_FPIN)) { mp->san_path_err_threshold = NU_NO; @@ -1124,17 +1146,19 @@ int select_san_path_err_threshold(struct config *conf, struct multipath *mp) mp_set_conf(san_path_err_threshold); mp_set_default(san_path_err_threshold, DEFAULT_ERR_CHECKS); out: + pthread_cleanup_push_cast(reset_strbuf, &buff); if (print_off_int_undef(&buff, mp->san_path_err_threshold) > 0) condlog(3, "%s: san_path_err_threshold = %s %s", mp->alias, get_strbuf_str(&buff), origin); warn_san_path_deprecated(mp, san_path_err_threshold); + pthread_cleanup_pop(1); return 0; } int select_san_path_err_forget_rate(struct config *conf, struct multipath *mp) { const char *origin; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; if (marginal_path_check_enabled(mp) || (conf->marginal_pathgroups == MARGINAL_PATHGROUP_FPIN)) { mp->san_path_err_forget_rate = NU_NO; @@ -1150,9 +1174,11 @@ int select_san_path_err_forget_rate(struct config *conf, struct multipath *mp) mp_set_conf(san_path_err_forget_rate); mp_set_default(san_path_err_forget_rate, DEFAULT_ERR_CHECKS); out: + pthread_cleanup_push_cast(reset_strbuf, &buff); if (print_off_int_undef(&buff, mp->san_path_err_forget_rate) > 0) condlog(3, "%s: san_path_err_forget_rate = %s %s", mp->alias, get_strbuf_str(&buff), origin); + pthread_cleanup_pop(1); warn_san_path_deprecated(mp, san_path_err_forget_rate); return 0; @@ -1161,7 +1187,7 @@ out: int select_san_path_err_recovery_time(struct config *conf, struct multipath *mp) { const char *origin; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; if (marginal_path_check_enabled(mp) || (conf->marginal_pathgroups == MARGINAL_PATHGROUP_FPIN)) { mp->san_path_err_recovery_time = NU_NO; @@ -1177,9 +1203,11 @@ int select_san_path_err_recovery_time(struct config *conf, struct multipath *mp) mp_set_conf(san_path_err_recovery_time); mp_set_default(san_path_err_recovery_time, DEFAULT_ERR_CHECKS); out: + pthread_cleanup_push_cast(reset_strbuf, &buff); if (print_off_int_undef(&buff, mp->san_path_err_recovery_time) != 0) condlog(3, "%s: san_path_err_recovery_time = %s %s", mp->alias, get_strbuf_str(&buff), origin); + pthread_cleanup_pop(1); warn_san_path_deprecated(mp, san_path_err_recovery_time); return 0; @@ -1188,7 +1216,7 @@ out: int select_marginal_path_err_sample_time(struct config *conf, struct multipath *mp) { const char *origin; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; if (conf->marginal_pathgroups == MARGINAL_PATHGROUP_FPIN) { mp->marginal_path_err_sample_time = NU_NO; @@ -1208,16 +1236,18 @@ out: mp->alias, 2 * IOTIMEOUT_SEC); mp->marginal_path_err_sample_time = 2 * IOTIMEOUT_SEC; } + pthread_cleanup_push_cast(reset_strbuf, &buff); if (print_off_int_undef(&buff, mp->marginal_path_err_sample_time) > 0) condlog(3, "%s: marginal_path_err_sample_time = %s %s", mp->alias, get_strbuf_str(&buff), origin); + pthread_cleanup_pop(1); return 0; } int select_marginal_path_err_rate_threshold(struct config *conf, struct multipath *mp) { const char *origin; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; if (conf->marginal_pathgroups == MARGINAL_PATHGROUP_FPIN) { mp->marginal_path_err_rate_threshold = NU_NO; @@ -1231,16 +1261,18 @@ int select_marginal_path_err_rate_threshold(struct config *conf, struct multipat mp_set_conf(marginal_path_err_rate_threshold); mp_set_default(marginal_path_err_rate_threshold, DEFAULT_ERR_CHECKS); out: + pthread_cleanup_push_cast(reset_strbuf, &buff); if (print_off_int_undef(&buff, mp->marginal_path_err_rate_threshold) > 0) condlog(3, "%s: marginal_path_err_rate_threshold = %s %s", mp->alias, get_strbuf_str(&buff), origin); + pthread_cleanup_pop(1); return 0; } int select_marginal_path_err_recheck_gap_time(struct config *conf, struct multipath *mp) { const char *origin; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; if (conf->marginal_pathgroups == MARGINAL_PATHGROUP_FPIN) { mp->marginal_path_err_recheck_gap_time = NU_NO; @@ -1254,17 +1286,19 @@ int select_marginal_path_err_recheck_gap_time(struct config *conf, struct multip mp_set_conf(marginal_path_err_recheck_gap_time); mp_set_default(marginal_path_err_recheck_gap_time, DEFAULT_ERR_CHECKS); out: + pthread_cleanup_push_cast(reset_strbuf, &buff); if (print_off_int_undef(&buff, mp->marginal_path_err_recheck_gap_time) > 0) condlog(3, "%s: marginal_path_err_recheck_gap_time = %s %s", mp->alias, get_strbuf_str(&buff), origin); + pthread_cleanup_pop(1); return 0; } int select_marginal_path_double_failed_time(struct config *conf, struct multipath *mp) { const char *origin; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; if (conf->marginal_pathgroups == MARGINAL_PATHGROUP_FPIN) { mp->marginal_path_double_failed_time = NU_NO; @@ -1278,9 +1312,11 @@ int select_marginal_path_double_failed_time(struct config *conf, struct multipat mp_set_conf(marginal_path_double_failed_time); mp_set_default(marginal_path_double_failed_time, DEFAULT_ERR_CHECKS); out: + pthread_cleanup_push_cast(reset_strbuf, &buff); if (print_off_int_undef(&buff, mp->marginal_path_double_failed_time) > 0) condlog(3, "%s: marginal_path_double_failed_time = %s %s", mp->alias, get_strbuf_str(&buff), origin); + pthread_cleanup_pop(1); return 0; } @@ -1324,7 +1360,7 @@ out: int select_ghost_delay (struct config *conf, struct multipath * mp) { const char *origin; - STRBUF_ON_STACK(buff); + struct strbuf buff = STRBUF_INIT; mp_set_mpe(ghost_delay); mp_set_ovr(ghost_delay); @@ -1332,9 +1368,11 @@ int select_ghost_delay (struct config *conf, struct multipath * mp) mp_set_conf(ghost_delay); mp_set_default(ghost_delay, DEFAULT_GHOST_DELAY); out: + pthread_cleanup_push_cast(reset_strbuf, &buff); if (print_off_int_undef(&buff, mp->ghost_delay) != 0) condlog(3, "%s: ghost_delay = %s %s", mp->alias, get_strbuf_str(&buff), origin); + pthread_cleanup_pop(1); return 0; } diff --git a/libmultipath/sysfs.h b/libmultipath/sysfs.h index 799f68e9..7e80b411 100644 --- a/libmultipath/sysfs.h +++ b/libmultipath/sysfs.h @@ -26,14 +26,9 @@ ssize_t sysfs_bin_attr_get_value(struct udev_device *dev, const char *attr_name, sysfs_attr_value_ok(__rc, __l); \ }) -#define log_sysfs_attr_set_value(prio, rc, fmt, __args...) \ -do { \ - STRBUF_ON_STACK(__buf); \ - if (print_strbuf(&__buf, fmt, ##__args) >= 0 && \ - print_strbuf(&__buf, ": %s", rc < 0 ? strerror(-rc) : \ - "write underflow") >= 0) \ - condlog(prio, "%s", get_strbuf_str(&__buf)); \ -} while(0) +#define log_sysfs_attr_set_value(prio, rc, fmt, __args...) \ + condlog(prio, fmt ": %s", ##__args , rc < 0 ? strerror(-rc) : \ + "write underflow") int sysfs_get_size (struct path *pp, unsigned long long * size); int sysfs_check_holders(char * check_devt, char * new_devt); diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c index 57447ca0..db6da984 100644 --- a/libmultipath/uevent.c +++ b/libmultipath/uevent.c @@ -460,11 +460,12 @@ static void print_uevq(const char *msg, struct list_head *uevq) { struct uevent *uev; int i = 0; - STRBUF_ON_STACK(buf); + struct strbuf buf = STRBUF_INIT; if (4 > MAX_VERBOSITY || 4 > libmp_verbosity) return; + pthread_cleanup_push_cast(reset_strbuf, &buf); if (list_empty(uevq)) append_strbuf_str(&buf, "*empty*"); else @@ -473,7 +474,8 @@ static void print_uevq(const char *msg, struct list_head *uevq) print_uev(&buf, uev); } - condlog(4, "uevent queue (%s): %s", msg, steal_strbuf_str(&buf)); + condlog(4, "uevent queue (%s): %s", msg, get_strbuf_str(&buf)); + pthread_cleanup_pop(1); } static void -- 2.17.2 -- dm-devel mailing list [email protected] https://listman.redhat.com/mailman/listinfo/dm-devel
