From 74756182d2da0f8889b3a0a1fded274a2baa14f5 Mon Sep 17 00:00:00 2001 From: Matthew Macy Date: Thu, 12 Sep 2019 13:28:26 -0700 Subject: [PATCH] Enable compiler to typecheck logging Annotate spa logging declarations with printflike Workaround gcc bug (non disable-able warning) by replacing "" with " " Reviewed-by: Matt Ahrens Reviewed-by: Brian Behlendorf Signed-off-by: Matt Macy Closes #9316 --- include/os/linux/spl/sys/debug.h | 2 ++ include/sys/spa.h | 6 +++--- lib/libspl/include/sys/debug.h | 4 ++++ module/zfs/dmu_objset.c | 4 ++-- module/zfs/dmu_recv.c | 4 ++-- module/zfs/dsl_dataset.c | 4 ++-- module/zfs/dsl_destroy.c | 8 ++++---- module/zfs/dsl_scan.c | 12 +++++++----- module/zfs/dsl_userhold.c | 2 +- module/zfs/spa.c | 11 +++++++---- module/zfs/spa_checkpoint.c | 2 +- module/zfs/vdev_removal.c | 7 ++++--- 12 files changed, 39 insertions(+), 27 deletions(-) diff --git a/include/os/linux/spl/sys/debug.h b/include/os/linux/spl/sys/debug.h index ecda6bcb8..8fad3bef6 100644 --- a/include/os/linux/spl/sys/debug.h +++ b/include/os/linux/spl/sys/debug.h @@ -50,6 +50,8 @@ /* * Common DEBUG functionality. */ +#define __printflike(a, b) __printf(a, b) + int spl_panic(const char *file, const char *func, int line, const char *fmt, ...); void spl_dumpstack(void); diff --git a/include/sys/spa.h b/include/sys/spa.h index 8323662f6..51e4c0f77 100644 --- a/include/sys/spa.h +++ b/include/sys/spa.h @@ -1153,11 +1153,11 @@ extern int spa_history_log_nvl(spa_t *spa, nvlist_t *nvl); extern void spa_history_log_version(spa_t *spa, const char *operation, dmu_tx_t *tx); extern void spa_history_log_internal(spa_t *spa, const char *operation, - dmu_tx_t *tx, const char *fmt, ...); + dmu_tx_t *tx, const char *fmt, ...) __printflike(4, 5); extern void spa_history_log_internal_ds(struct dsl_dataset *ds, const char *op, - dmu_tx_t *tx, const char *fmt, ...); + dmu_tx_t *tx, const char *fmt, ...) __printflike(4, 5); extern void spa_history_log_internal_dd(dsl_dir_t *dd, const char *operation, - dmu_tx_t *tx, const char *fmt, ...); + dmu_tx_t *tx, const char *fmt, ...) __printflike(4, 5); extern const char *spa_state_to_name(spa_t *spa); diff --git a/lib/libspl/include/sys/debug.h b/lib/libspl/include/sys/debug.h index fde4a0120..c6a8c6784 100644 --- a/lib/libspl/include/sys/debug.h +++ b/lib/libspl/include/sys/debug.h @@ -29,4 +29,8 @@ #include +#ifndef __printflike +#define __printflike(x, y) __attribute__((__format__(__printf__, x, y))) +#endif + #endif diff --git a/module/zfs/dmu_objset.c b/module/zfs/dmu_objset.c index 9350322ff..469137ecf 100644 --- a/module/zfs/dmu_objset.c +++ b/module/zfs/dmu_objset.c @@ -1262,7 +1262,7 @@ dmu_objset_create_sync(void *arg, dmu_tx_t *tx) mutex_exit(&ds->ds_lock); } - spa_history_log_internal_ds(ds, "create", tx, ""); + spa_history_log_internal_ds(ds, "create", tx, " "); zvol_create_minors(spa, doca->doca_name, B_TRUE); dsl_dataset_rele_flags(ds, DS_HOLD_FLAG_DECRYPT, FTAG); @@ -1375,7 +1375,7 @@ dmu_objset_clone_sync(void *arg, dmu_tx_t *tx) VERIFY0(dsl_dataset_hold_obj(pdd->dd_pool, obj, FTAG, &ds)); dsl_dataset_name(origin, namebuf); spa_history_log_internal_ds(ds, "clone", tx, - "origin=%s (%llu)", namebuf, origin->ds_object); + "origin=%s (%llu)", namebuf, (u_longlong_t)origin->ds_object); zvol_create_minors(dp->dp_spa, doca->doca_clone, B_TRUE); dsl_dataset_rele(ds, FTAG); dsl_dataset_rele(origin, FTAG); diff --git a/module/zfs/dmu_recv.c b/module/zfs/dmu_recv.c index ac27c98be..6249e165f 100644 --- a/module/zfs/dmu_recv.c +++ b/module/zfs/dmu_recv.c @@ -915,7 +915,7 @@ dmu_recv_begin_sync(void *arg, dmu_tx_t *tx) drba->drba_cookie->drc_ds = newds; - spa_history_log_internal_ds(newds, "receive", tx, ""); + spa_history_log_internal_ds(newds, "receive", tx, " "); } static int @@ -1093,7 +1093,7 @@ dmu_recv_resume_begin_sync(void *arg, dmu_tx_t *tx) drba->drba_cookie->drc_ds = ds; - spa_history_log_internal_ds(ds, "resume receive", tx, ""); + spa_history_log_internal_ds(ds, "resume receive", tx, " "); } /* diff --git a/module/zfs/dsl_dataset.c b/module/zfs/dsl_dataset.c index ba24e499b..8673c6d31 100644 --- a/module/zfs/dsl_dataset.c +++ b/module/zfs/dsl_dataset.c @@ -1843,7 +1843,7 @@ dsl_dataset_snapshot_sync_impl(dsl_dataset_t *ds, const char *snapname, dsl_dir_snap_cmtime_update(ds->ds_dir); - spa_history_log_internal_ds(ds->ds_prev, "snapshot", tx, ""); + spa_history_log_internal_ds(ds->ds_prev, "snapshot", tx, " "); } void @@ -3722,7 +3722,7 @@ dsl_dataset_promote_sync(void *arg, dmu_tx_t *tx) dsl_dir_remove_livelist(origin_ds->ds_dir, tx, B_TRUE); /* log history record */ - spa_history_log_internal_ds(hds, "promote", tx, ""); + spa_history_log_internal_ds(hds, "promote", tx, " "); dsl_dir_rele(odd, FTAG); promote_rele(ddpa, FTAG); diff --git a/module/zfs/dsl_destroy.c b/module/zfs/dsl_destroy.c index a30018341..ee237a85b 100644 --- a/module/zfs/dsl_destroy.c +++ b/module/zfs/dsl_destroy.c @@ -321,14 +321,14 @@ dsl_destroy_snapshot_sync_impl(dsl_dataset_t *ds, boolean_t defer, dmu_tx_t *tx) ASSERT(spa_version(dp->dp_spa) >= SPA_VERSION_USERREFS); dmu_buf_will_dirty(ds->ds_dbuf, tx); dsl_dataset_phys(ds)->ds_flags |= DS_FLAG_DEFER_DESTROY; - spa_history_log_internal_ds(ds, "defer_destroy", tx, ""); + spa_history_log_internal_ds(ds, "defer_destroy", tx, " "); return; } ASSERT3U(dsl_dataset_phys(ds)->ds_num_children, <=, 1); /* We need to log before removing it from the namespace. */ - spa_history_log_internal_ds(ds, "destroy", tx, ""); + spa_history_log_internal_ds(ds, "destroy", tx, " "); dsl_scan_ds_destroyed(ds, tx); @@ -1000,7 +1000,7 @@ dsl_destroy_head_sync_impl(dsl_dataset_t *ds, dmu_tx_t *tx) ASSERT(RRW_WRITE_HELD(&dp->dp_config_rwlock)); /* We need to log before removing it from the namespace. */ - spa_history_log_internal_ds(ds, "destroy", tx, ""); + spa_history_log_internal_ds(ds, "destroy", tx, " "); rmorigin = (dsl_dir_is_clone(ds->ds_dir) && DS_IS_DEFER_DESTROY(ds->ds_prev) && @@ -1167,7 +1167,7 @@ dsl_destroy_head_begin_sync(void *arg, dmu_tx_t *tx) dmu_buf_will_dirty(ds->ds_dbuf, tx); dsl_dataset_phys(ds)->ds_flags |= DS_FLAG_INCONSISTENT; - spa_history_log_internal_ds(ds, "destroy begin", tx, ""); + spa_history_log_internal_ds(ds, "destroy begin", tx, " "); dsl_dataset_rele(ds, FTAG); } diff --git a/module/zfs/dsl_scan.c b/module/zfs/dsl_scan.c index ec71a6d91..7845f1de2 100644 --- a/module/zfs/dsl_scan.c +++ b/module/zfs/dsl_scan.c @@ -759,7 +759,8 @@ dsl_scan_setup_sync(void *arg, dmu_tx_t *tx) spa_history_log_internal(spa, "scan setup", tx, "func=%u mintxg=%llu maxtxg=%llu", - *funcp, scn->scn_phys.scn_min_txg, scn->scn_phys.scn_max_txg); + *funcp, (u_longlong_t)scn->scn_phys.scn_min_txg, + (u_longlong_t)scn->scn_phys.scn_max_txg); } /* @@ -900,13 +901,13 @@ dsl_scan_done(dsl_scan_t *scn, boolean_t complete, dmu_tx_t *tx) if (dsl_scan_restarting(scn, tx)) spa_history_log_internal(spa, "scan aborted, restarting", tx, - "errors=%llu", spa_get_errlog_size(spa)); + "errors=%llu", (u_longlong_t)spa_get_errlog_size(spa)); else if (!complete) spa_history_log_internal(spa, "scan cancelled", tx, - "errors=%llu", spa_get_errlog_size(spa)); + "errors=%llu", (u_longlong_t)spa_get_errlog_size(spa)); else spa_history_log_internal(spa, "scan done", tx, - "errors=%llu", spa_get_errlog_size(spa)); + "errors=%llu", (u_longlong_t)spa_get_errlog_size(spa)); if (DSL_SCAN_IS_SCRUB_RESILVER(scn)) { spa->spa_scrub_started = B_FALSE; @@ -957,7 +958,8 @@ dsl_scan_done(dsl_scan_t *scn, boolean_t complete, dmu_tx_t *tx) if (resilver_needed) { spa_history_log_internal(spa, "starting deferred resilver", tx, - "errors=%llu", spa_get_errlog_size(spa)); + "errors=%llu", + (u_longlong_t)spa_get_errlog_size(spa)); spa_async_request(spa, SPA_ASYNC_RESILVER); } } diff --git a/module/zfs/dsl_userhold.c b/module/zfs/dsl_userhold.c index 2b2182fad..ba850c2f7 100644 --- a/module/zfs/dsl_userhold.c +++ b/module/zfs/dsl_userhold.c @@ -197,7 +197,7 @@ dsl_dataset_user_hold_sync_one_impl(nvlist_t *tmpholds, dsl_dataset_t *ds, spa_history_log_internal_ds(ds, "hold", tx, "tag=%s temp=%d refs=%llu", - htag, minor != 0, ds->ds_userrefs); + htag, minor != 0, (u_longlong_t)ds->ds_userrefs); } typedef struct zfs_hold_cleanup_arg { diff --git a/module/zfs/spa.c b/module/zfs/spa.c index 56c6dd8cd..8330ab1ce 100644 --- a/module/zfs/spa.c +++ b/module/zfs/spa.c @@ -887,7 +887,7 @@ spa_change_guid_sync(void *arg, dmu_tx_t *tx) spa_config_exit(spa, SCL_STATE, FTAG); spa_history_log_internal(spa, "guid change", tx, "old=%llu new=%llu", - oldguid, *newguid); + (u_longlong_t)oldguid, (u_longlong_t)*newguid); } /* @@ -7898,7 +7898,8 @@ spa_async_thread(void *arg) if (new_space != old_space) { spa_history_log_internal(spa, "vdev online", NULL, "pool '%s' size: %llu(+%llu)", - spa_name(spa), new_space, new_space - old_space); + spa_name(spa), (u_longlong_t)new_space, + (u_longlong_t)(new_space - old_space)); } } @@ -8388,7 +8389,8 @@ spa_sync_version(void *arg, dmu_tx_t *tx) spa->spa_uberblock.ub_version = version; vdev_config_dirty(spa->spa_root_vdev); - spa_history_log_internal(spa, "set", tx, "version=%lld", version); + spa_history_log_internal(spa, "set", tx, "version=%lld", + (longlong_t)version); } /* @@ -8502,7 +8504,8 @@ spa_sync_props(void *arg, dmu_tx_t *tx) spa->spa_pool_props_object, propname, 8, 1, &intval, tx)); spa_history_log_internal(spa, "set", tx, - "%s=%lld", nvpair_name(elem), intval); + "%s=%lld", nvpair_name(elem), + (longlong_t)intval); } else { ASSERT(0); /* not allowed */ } diff --git a/module/zfs/spa_checkpoint.c b/module/zfs/spa_checkpoint.c index b167a8321..f6dfdab9d 100644 --- a/module/zfs/spa_checkpoint.c +++ b/module/zfs/spa_checkpoint.c @@ -524,7 +524,7 @@ spa_checkpoint_sync(void *arg, dmu_tx_t *tx) spa_feature_incr(spa, SPA_FEATURE_POOL_CHECKPOINT, tx); spa_history_log_internal(spa, "spa checkpoint", tx, - "checkpointed uberblock txg=%llu", checkpoint.ub_txg); + "checkpointed uberblock txg=%llu", (u_longlong_t)checkpoint.ub_txg); } /* diff --git a/module/zfs/vdev_removal.c b/module/zfs/vdev_removal.c index c9156054c..abec4d50f 100644 --- a/module/zfs/vdev_removal.c +++ b/module/zfs/vdev_removal.c @@ -347,7 +347,7 @@ vdev_remove_initiate_sync(void *arg, dmu_tx_t *tx) vic->vic_mapping_object); spa_history_log_internal(spa, "vdev remove started", tx, - "%s vdev %llu %s", spa_name(spa), vd->vdev_id, + "%s vdev %llu %s", spa_name(spa), (u_longlong_t)vd->vdev_id, (vd->vdev_path != NULL) ? vd->vdev_path : "-"); /* * Setting spa_vdev_removal causes subsequent frees to call @@ -1111,7 +1111,7 @@ vdev_remove_complete_sync(void *arg, dmu_tx_t *tx) spa_finish_removal(dmu_tx_pool(tx)->dp_spa, DSS_FINISHED, tx); /* vd->vdev_path is not available here */ spa_history_log_internal(spa, "vdev remove completed", tx, - "%s vdev %llu", spa_name(spa), vd->vdev_id); + "%s vdev %llu", spa_name(spa), (u_longlong_t)vd->vdev_id); } static void @@ -1757,7 +1757,8 @@ spa_vdev_remove_cancel_sync(void *arg, dmu_tx_t *tx) vd->vdev_id, dmu_tx_get_txg(tx)); spa_history_log_internal(spa, "vdev remove canceled", tx, "%s vdev %llu %s", spa_name(spa), - vd->vdev_id, (vd->vdev_path != NULL) ? vd->vdev_path : "-"); + (u_longlong_t)vd->vdev_id, + (vd->vdev_path != NULL) ? vd->vdev_path : "-"); } static int