Add framework for validation of ioctls. Doesn't do any checks yet.

dmsetup --checks
libdevmapper: dm_task_enable_checks()
lvm.conf: activation/checks=1

diff --git a/WHATS_NEW b/WHATS_NEW
index fd56b3b1bf1158c1fea5a300c52955a6fa7900d2..b826817c44e40115f0391c26fe81ee93f87943b5 100644 (file)
--- 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 a28ed9b13a4e42914aac23b96d9c9e4ede2bb28e..951b6ac9294d8907be8eadc874aed49e7010b2f1 100644 (file)
--- 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 dc4ac0b6a636503e0949a337f32c2eefba02285d..a7219846aa769d2ecf190bceb0813297e96ea85b 100644 (file)
--- 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 89d7b23d30d0482346eeeba2bddc19d822d7cdb7..712f5784cc1965407eeb58ae39113d83acc69a6c 100644 (file)
--- 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 018447e9800820c471babea60d12db44c7f0276f..f3adcc2c5cc4c29d3691382f84d8ef1ce36ea6fa 100644 (file)
--- 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 51a6013443ed36ed8d3c49aba585651afa17b4f8..e50fe3d38157d1f584e77a020710f2f1bdc4fb64 100644 (file)
--- 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 01f1bf1911ae128562b0d86334695f65255c6dd9..afd8dc1aaaa2d90596e588735769042b189b85f3 100644 (file)
--- 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 812cc0096ee63a2f43d6567803d4f3693f3eb41c..b9ece7fe766f1f3ba4cae204ab88e2d29ae0df7d 100644 (file)
--- 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 7cfd972378fbab4fdc50c6c5f986b4f5ca2beb25..2cdfdd15649cbc289698f8788f14ce6ca01a35ad 100644 (file)
--- 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 da49201883ee055719fafe1529733bccfc18ed1b..cfc19b62cf06daa0c3a3d505ff755082243f0956 100644 (file)
--- 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 d8cca844ca4677b6190d19296ee967abf346be7f..d61ccfdab93f1c5c8030ed0d515f04b9d5f1017d 100644 (file)
--- 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 a3ae88070c0f685ee7db6562175ea2c9181d45e5..340e6e51b8e00ee7886a7edf17d5889a4c35dbf5 100644 (file)
--- 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 fa6746a04f9b3f86def10d39137dfe05dfade64b..31a44e72fb3c6c893b722851915cf1438f32b920 100644 (file)
--- 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 3b82b64e2c99ee19b78202855d9cef9f4149b202..175a429e658b5a8f98477c5712a7665e517fb648 100644 (file)
--- 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 10e9492ceb0df7cfe1c537a4db02839006b4e6cf..1a2cdf9456292ba962309acda79334c17da8e753 100644 (file)
--- 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 3cf18a02a3c0c1817804ddd6dff131985327ac79..2cc4daa9f6a554dd6110f03424ba7586e4ce5481 100644 (file)
--- 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 [+]<sectors>|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);