diff --git a/WHATS_NEW b/WHATS_NEW index fd56b3b1b..b826817c4 100644 --- a/WHATS_NEW +++ b/WHATS_NEW @@ -1,5 +1,6 @@ Version 2.02.86 - ================================= + Add activation/checks to lvm.conf to perform additional ioctl validation. When suspending, automatically preload newly-visible existing LVs. Report internal error when parameters are missing on table load. Teardown any stray devices with $COMMON_PREFIX during test runs. diff --git a/WHATS_NEW_DM b/WHATS_NEW_DM index a28ed9b13..951b6ac92 100644 --- a/WHATS_NEW_DM +++ b/WHATS_NEW_DM @@ -1,5 +1,6 @@ Version 1.02.65 - ================================== + Add dmsetup --checks and dm_task_enable_checks framework to validate ioctls. Add age_in_minutes parameter to dmsetup udevcomplete_all. Return immediately from dm_lib_exit() if called more than once. Disable udev fallback by default and add --verifyudev option to dmsetup. diff --git a/doc/example.conf.in b/doc/example.conf.in index dc4ac0b6a..a7219846a 100644 --- a/doc/example.conf.in +++ b/doc/example.conf.in @@ -411,6 +411,12 @@ global { } activation { + # Set to 1 to perform internal checks on the operations issued to + # libdevmapper. Useful for debugging problems with activation. + # Some of the checks may be expensive, so it's best to use this + # only when there seems to be a problem. + checks = 0 + # Set to 0 to disable udev synchronisation (if compiled into the binaries). # Processes will not wait for notification from udev. # They will continue irrespective of any possible udev processing diff --git a/lib/activate/activate.c b/lib/activate/activate.c index 89d7b23d3..712f5784c 100644 --- a/lib/activate/activate.c +++ b/lib/activate/activate.c @@ -394,6 +394,9 @@ int target_version(const char *target_name, uint32_t *maj, if (!(dmt = dm_task_create(DM_DEVICE_LIST_VERSIONS))) return_0; + if (activation_checks() && !dm_task_enable_checks(dmt)) + goto_out; + if (!dm_task_run(dmt)) { log_debug("Failed to get %s target version", target_name); /* Assume this was because LIST_VERSIONS isn't supported */ diff --git a/lib/activate/dev_manager.c b/lib/activate/dev_manager.c index 018447e98..f3adcc2c5 100644 --- a/lib/activate/dev_manager.c +++ b/lib/activate/dev_manager.c @@ -88,6 +88,9 @@ static struct dm_task *_setup_task(const char *name, const char *uuid, if (major && !dm_task_set_major_minor(dmt, major, minor, 1)) goto_out; + if (activation_checks() && !dm_task_enable_checks(dmt)) + goto_out; + return dmt; out: dm_task_destroy(dmt); @@ -148,6 +151,9 @@ int device_is_usable(struct device *dev) if (!dm_task_set_major_minor(dmt, MAJOR(dev->dev), MINOR(dev->dev), 1)) goto_out; + if (activation_checks() && !dm_task_enable_checks(dmt)) + goto_out; + if (!dm_task_run(dmt)) { log_error("Failed to get state of mapped device"); goto out; diff --git a/lib/commands/toolcontext.c b/lib/commands/toolcontext.c index 51a601344..e50fe3d38 100644 --- a/lib/commands/toolcontext.c +++ b/lib/commands/toolcontext.c @@ -285,6 +285,9 @@ static int _process_config(struct cmd_context *cmd) "activation/udev_sync", DEFAULT_UDEV_SYNC); + init_activation_checks(find_config_tree_int(cmd, "activation/checks", + DEFAULT_ACTIVATION_CHECKS)); + #ifdef UDEV_SYNC_SUPPORT /* * We need udev rules to be applied, otherwise we would end up with no diff --git a/lib/config/defaults.h b/lib/config/defaults.h index 01f1bf191..afd8dc1aa 100644 --- a/lib/config/defaults.h +++ b/lib/config/defaults.h @@ -79,6 +79,7 @@ #define DEFAULT_UDEV_RULES 1 #define DEFAULT_UDEV_SYNC 0 #define DEFAULT_VERIFY_UDEV_OPERATIONS 0 +#define DEFAULT_ACTIVATION_CHECKS 0 #define DEFAULT_EXTENT_SIZE 4096 /* In KB */ #define DEFAULT_MAX_PV 0 #define DEFAULT_MAX_LV 0 diff --git a/lib/misc/lvm-globals.c b/lib/misc/lvm-globals.c index 812cc0096..b9ece7fe7 100644 --- a/lib/misc/lvm-globals.c +++ b/lib/misc/lvm-globals.c @@ -42,6 +42,7 @@ static int _ignore_suspended_devices = 0; static int _error_message_produced = 0; static unsigned _is_static = 0; static int _udev_checking = 1; +static int _activation_checks = 0; static char _sysfs_dir_path[PATH_MAX] = ""; static int _dev_disable_after_error_count = DEFAULT_DISABLE_AFTER_ERROR_COUNT; static uint64_t _pv_min_size = (DEFAULT_PV_MIN_SIZE_KB * 1024L >> SECTOR_SHIFT); @@ -131,6 +132,14 @@ void init_udev_checking(int checking) log_debug("LVM udev checking disabled"); } +void init_activation_checks(int checks) +{ + if ((_activation_checks = checks)) + log_debug("LVM activation checks enabled"); + else + log_debug("LVM activation checks disabled"); +} + void init_dev_disable_after_error_count(int value) { _dev_disable_after_error_count = value; @@ -256,6 +265,11 @@ int udev_checking(void) return _udev_checking; } +int activation_checks(void) +{ + return _activation_checks; +} + const char *sysfs_dir_path(void) { return _sysfs_dir_path; diff --git a/lib/misc/lvm-globals.h b/lib/misc/lvm-globals.h index 7cfd97237..2cdfdd156 100644 --- a/lib/misc/lvm-globals.h +++ b/lib/misc/lvm-globals.h @@ -40,6 +40,7 @@ void init_is_static(unsigned value); void init_udev_checking(int checking); void init_dev_disable_after_error_count(int value); void init_pv_min_size(uint64_t sectors); +void init_activation_checks(int checks); void set_cmd_name(const char *cmd_name); void set_sysfs_dir_path(const char *path); @@ -63,6 +64,7 @@ unsigned is_static(void); int udev_checking(void); const char *sysfs_dir_path(void); uint64_t pv_min_size(void); +int activation_checks(void); #define DMEVENTD_MONITOR_IGNORE -1 int dmeventd_monitor_mode(void); diff --git a/libdm/ioctl/libdm-iface.c b/libdm/ioctl/libdm-iface.c index da4920188..cfc19b62c 100644 --- a/libdm/ioctl/libdm-iface.c +++ b/libdm/ioctl/libdm-iface.c @@ -1884,6 +1884,16 @@ no_match: return r; } +static int _suspend_with_validation_v4(struct dm_task *dmt) +{ + /* + * FIXME Ensure we can't leave any I/O trapped between suspended devices. + */ + dmt->enable_checks = 0; + + return dm_task_run(dmt); +} + static const char *_sanitise_message(char *message) { const char *sanitised_message = message ?: ""; @@ -1963,7 +1973,7 @@ static struct dm_ioctl *_do_dm_ioctl(struct dm_task *dmt, unsigned command, } log_debug("dm %s %s%s %s%s%s %s%.0d%s%.0d%s" - "%s%c%c%s%s%s%s %.0" PRIu64 " %s [%u]", + "%s%c%c%s%s%s%s%s %.0" PRIu64 " %s [%u]", _cmd_data_v4[dmt->type].name, dmt->new_uuid ? "UUID " : "", dmi->name, dmi->uuid, dmt->newname ? " " : "", @@ -1980,6 +1990,7 @@ static struct dm_ioctl *_do_dm_ioctl(struct dm_task *dmt, unsigned command, dmt->skip_lockfs ? "S " : "", dmt->secure_data ? "W " : "", dmt->query_inactive_table ? "I " : "", + dmt->enable_checks ? "C" : "", dmt->sector, _sanitise_message(dmt->message), dmi->data_size); #ifdef DM_IOCTLS @@ -2054,6 +2065,9 @@ int dm_task_run(struct dm_task *dmt) if ((dmt->type == DM_DEVICE_RELOAD) && dmt->suppress_identical_reload) return _reload_with_suppression_v4(dmt); + if ((dmt->type == DM_DEVICE_SUSPEND) && dmt->enable_checks) + return _suspend_with_validation_v4(dmt); + if (!_open_control()) { _udev_complete(dmt); return 0; diff --git a/libdm/ioctl/libdm-targets.h b/libdm/ioctl/libdm-targets.h index d8cca844c..d61ccfdab 100644 --- a/libdm/ioctl/libdm-targets.h +++ b/libdm/ioctl/libdm-targets.h @@ -65,6 +65,7 @@ struct dm_task { int cookie_set; int new_uuid; int secure_data; + int enable_checks; char *uuid; }; diff --git a/libdm/libdevmapper.h b/libdm/libdevmapper.h index a3ae88070..340e6e51b 100644 --- a/libdm/libdevmapper.h +++ b/libdm/libdevmapper.h @@ -190,6 +190,12 @@ int dm_task_skip_lockfs(struct dm_task *dmt); int dm_task_query_inactive_table(struct dm_task *dmt); int dm_task_suppress_identical_reload(struct dm_task *dmt); int dm_task_secure_data(struct dm_task *dmt); + +/* + * Enable checks for common mistakes such as issuing ioctls in an unsafe order. + */ +int dm_task_enable_checks(struct dm_task *dmt); + typedef enum { DM_ADD_NODE_ON_RESUME, /* add /dev/mapper node with dmsetup resume */ DM_ADD_NODE_ON_CREATE /* add /dev/mapper node with dmsetup create */ diff --git a/libdm/libdm-common.c b/libdm/libdm-common.c index fa6746a04..31a44e72f 100644 --- a/libdm/libdm-common.c +++ b/libdm/libdm-common.c @@ -395,6 +395,13 @@ int dm_task_set_mode(struct dm_task *dmt, mode_t mode) return 1; } +int dm_task_enable_checks(struct dm_task *dmt) +{ + dmt->enable_checks = 1; + + return 1; +} + int dm_task_add_target(struct dm_task *dmt, uint64_t start, uint64_t size, const char *ttype, const char *params) { diff --git a/man/dmsetup.8.in b/man/dmsetup.8.in index 3b82b64e2..175a429e6 100644 --- a/man/dmsetup.8.in +++ b/man/dmsetup.8.in @@ -114,6 +114,10 @@ Invoking the command as \fBdevmap_name\fP is equivalent to .br \fBdmsetup info -c --noheadings -j \fImajor\fB -m \fIminor\fP. .SH OPTIONS +.IP \fB--checks +Perform additional checks on the operations requested and report +potential problems. Useful when debugging scripts. +In some cases these checks may slow down operations noticeably. .IP \fB-c|-C|--columns .br Display output in columns rather than as Field: Value lines. diff --git a/test/lib/aux.sh b/test/lib/aux.sh index 10e9492ce..1a2cdf945 100644 --- a/test/lib/aux.sh +++ b/test/lib/aux.sh @@ -381,6 +381,7 @@ global/locking_dir = "$TESTDIR/var/lock/lvm" global/locking_type=$LVM_TEST_LOCKING global/si_unit_consistency = 1 global/fallback_to_local_locking = 0 +activation/checks = 1 activation/udev_sync = 1 activation/udev_rules = 1 activation/verify_udev_operations = $VERIFY_UDEV diff --git a/tools/dmsetup.c b/tools/dmsetup.c index 3cf18a02a..2cc4daa9f 100644 --- a/tools/dmsetup.c +++ b/tools/dmsetup.c @@ -117,6 +117,9 @@ extern char *optarg; */ enum { READ_ONLY = 0, + ADD_NODE_ON_CREATE_ARG, + ADD_NODE_ON_RESUME_ARG, + CHECKS_ARG, COLS_ARG, EXEC_ARG, FORCE_ARG, @@ -153,8 +156,6 @@ enum { VERIFYUDEV_ARG, VERSION_ARG, YES_ARG, - ADD_NODE_ON_RESUME_ARG, - ADD_NODE_ON_CREATE_ARG, NUM_SWITCHES }; @@ -317,6 +318,9 @@ static struct dm_task *_get_deps_task(int major, int minor) if (_switches[INACTIVE_ARG] && !dm_task_query_inactive_table(dmt)) goto err; + if (_switches[CHECKS_ARG] && !dm_task_enable_checks(dmt)) + goto err; + if (!dm_task_run(dmt)) goto err; @@ -572,6 +576,9 @@ static int _load(CMD_ARGS) if (_switches[INACTIVE_ARG] && !dm_task_query_inactive_table(dmt)) goto out; + if (_switches[CHECKS_ARG] && !dm_task_enable_checks(dmt)) + goto out; + if (!dm_task_run(dmt)) goto out; @@ -645,6 +652,8 @@ static int _create(CMD_ARGS) udev_flags |= DM_UDEV_DISABLE_DM_RULES_FLAG | DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG; + if (_switches[CHECKS_ARG] && !dm_task_enable_checks(dmt)) + goto out; if (!_set_task_add_node(dmt)) goto out; @@ -699,6 +708,9 @@ static int _rename(CMD_ARGS) if (_switches[INACTIVE_ARG] && !dm_task_query_inactive_table(dmt)) goto out; + if (_switches[CHECKS_ARG] && !dm_task_enable_checks(dmt)) + goto out; + if (_switches[NOUDEVRULES_ARG]) udev_flags |= DM_UDEV_DISABLE_DM_RULES_FLAG | DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG; @@ -778,6 +790,9 @@ static int _message(CMD_ARGS) if (_switches[INACTIVE_ARG] && !dm_task_query_inactive_table(dmt)) goto out; + if (_switches[CHECKS_ARG] && !dm_task_enable_checks(dmt)) + goto out; + if (!dm_task_run(dmt)) goto out; @@ -816,6 +831,9 @@ static int _setgeometry(CMD_ARGS) if (_switches[INACTIVE_ARG] && !dm_task_query_inactive_table(dmt)) goto out; + if (_switches[CHECKS_ARG] && !dm_task_enable_checks(dmt)) + goto out; + /* run the task */ if (!dm_task_run(dmt)) goto out; @@ -1234,6 +1252,9 @@ static int _simple(int task, const char *name, uint32_t event_nr, int display) if (_switches[NOLOCKFS_ARG] && !dm_task_skip_lockfs(dmt)) goto out; + if (_switches[CHECKS_ARG] && !dm_task_enable_checks(dmt)) + goto out; + /* FIXME: needs to coperate with udev */ if (!_set_task_add_node(dmt)) goto out; @@ -1314,6 +1335,9 @@ static int _process_all(const struct command *cmd, int argc, char **argv, int si if (!(dmt = dm_task_create(DM_DEVICE_LIST))) return 0; + if (_switches[CHECKS_ARG] && !dm_task_enable_checks(dmt)) + goto out; + if (!dm_task_run(dmt)) { r = 0; goto out; @@ -1362,6 +1386,9 @@ static uint64_t _get_device_size(const char *name) if (_switches[INACTIVE_ARG] && !dm_task_query_inactive_table(dmt)) goto out; + if (_switches[CHECKS_ARG] && !dm_task_enable_checks(dmt)) + goto out; + if (!dm_task_run(dmt)) goto out; @@ -1411,6 +1438,9 @@ static int _error_device(CMD_ARGS) if (_switches[INACTIVE_ARG] && !dm_task_query_inactive_table(dmt)) goto error; + if (_switches[CHECKS_ARG] && !dm_task_enable_checks(dmt)) + goto error; + if (!dm_task_run(dmt)) goto error; @@ -1586,6 +1616,9 @@ static int _status(CMD_ARGS) if (_switches[INACTIVE_ARG] && !dm_task_query_inactive_table(dmt)) goto out; + if (_switches[CHECKS_ARG] && !dm_task_enable_checks(dmt)) + goto out; + if (!dm_task_run(dmt)) goto out; @@ -1659,6 +1692,9 @@ static int _targets(CMD_ARGS) if (!(dmt = dm_task_create(DM_DEVICE_LIST_VERSIONS))) return 0; + if (_switches[CHECKS_ARG] && !dm_task_enable_checks(dmt)) + goto out; + if (!dm_task_run(dmt)) goto out; @@ -1709,6 +1745,9 @@ static int _info(CMD_ARGS) if (_switches[INACTIVE_ARG] && !dm_task_query_inactive_table(dmt)) goto out; + if (_switches[CHECKS_ARG] && !dm_task_enable_checks(dmt)) + goto out; + if (!dm_task_run(dmt)) goto out; @@ -1749,6 +1788,9 @@ static int _deps(CMD_ARGS) if (_switches[INACTIVE_ARG] && !dm_task_query_inactive_table(dmt)) goto out; + if (_switches[CHECKS_ARG] && !dm_task_enable_checks(dmt)) + goto out; + if (!dm_task_run(dmt)) goto out; @@ -2764,7 +2806,7 @@ static void _usage(FILE *out) fprintf(out, "Usage:\n\n"); fprintf(out, "dmsetup [--version] [-h|--help [-c|-C|--columns]]\n" - " [-v|--verbose [-v|--verbose ...]]\n" + " [--checks] [-v|--verbose [-v|--verbose ...]]\n" " [-r|--readonly] [--noopencount] [--nolockfs] [--inactive]\n" " [--udevcookie [cookie]] [--noudevrules] [--noudevsync] [--verifyudev]\n" " [-y|--yes] [--readahead [+]|auto|none]\n" @@ -3119,6 +3161,7 @@ static int _process_switches(int *argc, char ***argv, const char *dev_dir) #ifdef HAVE_GETOPTLONG static struct option long_options[] = { {"readonly", 0, &ind, READ_ONLY}, + {"checks", 0, &ind, CHECKS_ARG}, {"columns", 0, &ind, COLS_ARG}, {"exec", 1, &ind, EXEC_ARG}, {"force", 0, &ind, FORCE_ARG}, @@ -3258,6 +3301,8 @@ static int _process_switches(int *argc, char ***argv, const char *dev_dir) _switches[ADD_NODE_ON_RESUME_ARG]++; if (ind == ADD_NODE_ON_CREATE_ARG) _switches[ADD_NODE_ON_CREATE_ARG]++; + if (ind == CHECKS_ARG) + _switches[CHECKS_ARG]++; if (ind == UDEVCOOKIE_ARG) { _switches[UDEVCOOKIE_ARG]++; _udev_cookie = _get_cookie_value(optarg);