Set appropriate udev flags for reserved LVs.

There's no need for foreign udev rules to touch LVM reserved devices
(snapshot, pvmove, _mlog, _mimage, _vorigin) even if they happen to
be visible. The same applies for /dev/disk content - no need to create
any content for these devices (and so no need to run any "blkid" etc.).
This also prevents setting any inotify "watch" from udev rules on such
devices that is a source of race conditions (the rules need to honor
DM_UDEV_DISABLE_OTHER_RULES_FLAG for this to work though).

diff --git a/WHATS_NEW b/WHATS_NEW
index f692deff8531cffc95d4289e1c022334265206ce..a99a5956b89d03cbbca7687ceb23e0efed4ec390 100644 (file)
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
 Version 2.02.64 -
 =================================
+  Set appropriate udev flags for reserved LVs.
   Don't preload the origin when removing a snapshot whose merge is pending.
   Disallow the addition of mirror images while a conversion is happening.
   Disallow primary mirror image removal when mirror is not in-sync.

diff --git a/lib/activate/dev_manager.c b/lib/activate/dev_manager.c
index cf3aeaed3175304d6cd9d894c8bd7f5b3db925f8..d52d46f601dcb2e0a6fed3b8c671d1ff5e5dfbad 100644 (file)
--- a/lib/activate/dev_manager.c
+++ b/lib/activate/dev_manager.c
@@ -751,12 +751,55 @@ int dev_manager_mknodes(const struct logical_volume *lv)
        return r;
 }
 
+static uint16_t _get_udev_flags(struct dev_manager *dm, struct logical_volume *lv,
+                               const char *layer)
+{
+       uint16_t udev_flags = 0;
+
+       /*
+        * Is this top-level and visible device?
+        * If not, create just the /dev/mapper content.
+        */
+       if (layer || !lv_is_visible(lv))
+               udev_flags |= DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG |
+                             DM_UDEV_DISABLE_DISK_RULES_FLAG |
+                             DM_UDEV_DISABLE_OTHER_RULES_FLAG;
+       /*
+        * There's no need for other udev rules to touch special LVs with
+        * reserved names. We don't need to populate /dev/disk here either.
+        * Even if they happen to be visible and top-level.
+        */
+       else if (is_reserved_lvname(lv->name))
+               udev_flags |= DM_UDEV_DISABLE_DISK_RULES_FLAG |
+                             DM_UDEV_DISABLE_OTHER_RULES_FLAG;
+
+       /*
+        * Snapshots and origins could have the same rule applied that will
+        * give symlinks exactly the same name (e.g. a name based on
+        * filesystem UUID). We give preference to origins to make such
+        * naming deterministic (e.g. symlinks in /dev/disk/by-uuid).
+        */
+       if (lv_is_cow(lv))
+               udev_flags |= DM_UDEV_LOW_PRIORITY_FLAG;
+
+       /*
+        * Finally, add flags to disable /dev/mapper and /dev/<vgname> content
+        * to be created by udev if it is requested by user's configuration.
+        * This is basically an explicit fallback to old node/symlink creation
+        * without udev.
+        */
+       if (!dm->cmd->current_settings.udev_rules)
+               udev_flags |= DM_UDEV_DISABLE_DM_RULES_FLAG |
+                             DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG;
+
+       return udev_flags;
+}
+
 static int _add_dev_to_dtree(struct dev_manager *dm, struct dm_tree *dtree,
                               struct logical_volume *lv, const char *layer)
 {
        char *dlid, *name;
        struct dm_info info, info2;
-       uint16_t udev_flags = 0;
 
        if (!(name = build_dm_name(dm->mem, lv->vg->name, lv->name, layer)))
                return_0;
@@ -794,20 +837,8 @@ static int _add_dev_to_dtree(struct dev_manager *dm, struct dm_tree *dtree,
                }
        }
 
-       if (layer || !lv_is_visible(lv))
-               udev_flags |= DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG |
-                             DM_UDEV_DISABLE_DISK_RULES_FLAG |
-                             DM_UDEV_DISABLE_OTHER_RULES_FLAG;
-
-       if (lv_is_cow(lv))
-               udev_flags |= DM_UDEV_LOW_PRIORITY_FLAG;
-
-       if (!dm->cmd->current_settings.udev_rules)
-               udev_flags |= DM_UDEV_DISABLE_DM_RULES_FLAG |
-                             DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG;
-
-       if (info.exists && !dm_tree_add_dev_with_udev_flags(dtree, info.major,
-                                                           info.minor, udev_flags)) {
+       if (info.exists && !dm_tree_add_dev_with_udev_flags(dtree, info.major, info.minor,
+                                                       _get_udev_flags(dm, lv, layer))) {
                log_error("Failed to add device (%" PRIu32 ":%" PRIu32") to dtree",
                          info.major, info.minor);
                return 0;
@@ -1164,7 +1195,6 @@ static int _add_new_lv_to_dtree(struct dev_manager *dm, struct dm_tree *dtree,
        uint32_t max_stripe_size = UINT32_C(0);
        uint32_t read_ahead = lv->read_ahead;
        uint32_t read_ahead_flags = UINT32_C(0);
-       uint16_t udev_flags = 0;
 
        /* FIXME Seek a simpler way to lay out the snapshot-merge tree. */
 
@@ -1205,18 +1235,6 @@ static int _add_new_lv_to_dtree(struct dev_manager *dm, struct dm_tree *dtree,
 
        lvlayer->lv = lv;
 
-       if (layer || !lv_is_visible(lv))
-               udev_flags |= DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG |
-                             DM_UDEV_DISABLE_DISK_RULES_FLAG |
-                             DM_UDEV_DISABLE_OTHER_RULES_FLAG;
-
-       if (lv_is_cow(lv))
-               udev_flags |= DM_UDEV_LOW_PRIORITY_FLAG;
-
-       if (!dm->cmd->current_settings.udev_rules)
-               udev_flags |= DM_UDEV_DISABLE_DM_RULES_FLAG |
-                             DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG;
-
        /*
         * Add LV to dtree.
         * If we're working with precommitted metadata, clear any
@@ -1229,7 +1247,7 @@ static int _add_new_lv_to_dtree(struct dev_manager *dm, struct dm_tree *dtree,
                                             _read_only_lv(lv),
                                             (lv->vg->status & PRECOMMITTED) ? 1 : 0,
                                             lvlayer,
-                                            udev_flags)))
+                                            _get_udev_flags(dm, lv, layer))))
                return_0;
 
        /* Store existing name so we can do rename later */

diff --git a/lib/misc/lvm-string.c b/lib/misc/lvm-string.c
index f79c8b03905bee931b7b4ba8ccd1fd196319acfa..7eed79951dad75ba167f6719af3a4ab2c6a83134 100644 (file)
--- a/lib/misc/lvm-string.c
+++ b/lib/misc/lvm-string.c
@@ -242,3 +242,49 @@ int validate_name(const char *n)
 
        return 1;
 }
+
+int apply_lvname_restrictions(const char *name)
+{
+       if (!strncmp(name, "snapshot", 8)) {
+               log_error("Names starting \"snapshot\" are reserved. "
+                         "Please choose a different LV name.");
+               return 0;
+       }
+
+       if (!strncmp(name, "pvmove", 6)) {
+               log_error("Names starting \"pvmove\" are reserved. "
+                         "Please choose a different LV name.");
+               return 0;
+       }
+
+       if (strstr(name, "_mlog")) {
+               log_error("Names including \"_mlog\" are reserved. "
+                         "Please choose a different LV name.");
+               return 0;
+       }
+
+       if (strstr(name, "_mimage")) {
+               log_error("Names including \"_mimage\" are reserved. "
+                         "Please choose a different LV name.");
+               return 0;
+       }
+
+       if (strstr(name, "_vorigin")) {
+               log_error("Names including \"_vorigin\" are reserved. "
+                         "Please choose a different LV name.");
+               return 0;
+       }
+
+       return 1;
+}
+
+int is_reserved_lvname(const char *name)
+{
+       int rc, old_suppress;
+
+       old_suppress = log_suppress(2);
+       rc = !apply_lvname_restrictions(name);
+       log_suppress(old_suppress);
+
+       return rc;
+}

diff --git a/lib/misc/lvm-string.h b/lib/misc/lvm-string.h
index d5e544949b1dd85f97fb556b5efecb84bf69e33d..35d9245888c6ee8ec1daf5ece7aef74f7af8e9fd 100644 (file)
--- a/lib/misc/lvm-string.h
+++ b/lib/misc/lvm-string.h
@@ -34,6 +34,9 @@ char *build_dm_uuid(struct dm_pool *mem, const char *lvid,
 
 int validate_name(const char *n);
 
+int apply_lvname_restrictions(const char *name);
+int is_reserved_lvname(const char *name);
+
 /*
  * Returns number of occurrences of c in first len characters of str.
  */

diff --git a/tools/toollib.c b/tools/toollib.c
index 81e81fa485f19eaae9c28b68bd473ba0e66d0be4..475e90e71fd18d182d17ddb8f8c32ba604af250c 100644 (file)
--- a/tools/toollib.c
+++ b/tools/toollib.c
@@ -1125,52 +1125,6 @@ struct dm_list *clone_pv_list(struct dm_pool *mem, struct dm_list *pvsl)
        return r;
 }
 
-int apply_lvname_restrictions(const char *name)
-{
-       if (!strncmp(name, "snapshot", 8)) {
-               log_error("Names starting \"snapshot\" are reserved. "
-                         "Please choose a different LV name.");
-               return 0;
-       }
-
-       if (!strncmp(name, "pvmove", 6)) {
-               log_error("Names starting \"pvmove\" are reserved. "
-                         "Please choose a different LV name.");
-               return 0;
-       }
-
-       if (strstr(name, "_mlog")) {
-               log_error("Names including \"_mlog\" are reserved. "
-                         "Please choose a different LV name.");
-               return 0;
-       }
-
-       if (strstr(name, "_mimage")) {
-               log_error("Names including \"_mimage\" are reserved. "
-                         "Please choose a different LV name.");
-               return 0;
-       }
-
-       if (strstr(name, "_vorigin")) {
-               log_error("Names including \"_vorigin\" are reserved. "
-                         "Please choose a different LV name.");
-               return 0;
-       }
-
-       return 1;
-}
-
-int is_reserved_lvname(const char *name)
-{
-       int rc, old_suppress;
-
-       old_suppress = log_suppress(2);
-       rc = !apply_lvname_restrictions(name);
-       log_suppress(old_suppress);
-
-       return rc;
-}
-
 void vgcreate_params_set_defaults(struct vgcreate_params *vp_def,
                                  struct volume_group *vg)
 {