[PATCH 1/2] Modernize udev rules

[PATCH 1/2] Modernize udev rules

[Thomas Bächler]

* $tempnode is deprecated, use $devnode
* blkid -o udev output is deprecated, use IMPORT{builtin}="blkid" instead
---
 udev-md-raid.rules | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/udev-md-raid.rules b/udev-md-raid.rules
index cc7f5ef..ea97261 100644
--- a/udev-md-raid.rules
+++ b/udev-md-raid.rules
@@ -10,7 +10,7 @@ LABEL="md_inc"
 
 # remember you can limit what gets auto/incrementally assembled by
 # mdadm.conf(5)'s 'AUTO' and selectively whitelist using 'ARRAY'
-ACTION=="add", RUN+="/sbin/mdadm --incremental $tempnode --offroot"
+ACTION=="add", RUN+="/sbin/mdadm --incremental $devnode --offroot"
 ACTION=="remove", ENV{ID_PATH}=="?*", RUN+="/sbin/mdadm -If $name --path $env{ID_PATH}"
 ACTION=="remove", ENV{ID_PATH}!="?*", RUN+="/sbin/mdadm -If $name"
 
@@ -31,7 +31,7 @@ TEST!="md/array_state", ENV{SYSTEMD_READY}="0", GOTO="md_end"
 ATTR{md/array_state}=="|clear|inactive", ENV{SYSTEMD_READY}="0", GOTO="md_end"
 LABEL="md_ignore_state"
 
-IMPORT{program}="/sbin/mdadm --detail --export $tempnode"
+IMPORT{program}="/sbin/mdadm --detail --export $devnode"
 ENV{DEVTYPE}=="disk", ENV{MD_NAME}=="?*", SYMLINK+="disk/by-id/md-name-$env{MD_NAME}", OPTIONS+="string_escape=replace"
 ENV{DEVTYPE}=="disk", ENV{MD_UUID}=="?*", SYMLINK+="disk/by-id/md-uuid-$env{MD_UUID}"
 ENV{DEVTYPE}=="disk", ENV{MD_DEVNAME}=="?*", SYMLINK+="md/$env{MD_DEVNAME}"
@@ -40,7 +40,7 @@ ENV{DEVTYPE}=="partition", ENV{MD_UUID}=="?*", SYMLINK+="disk/by-id/md-uuid-$env
 ENV{DEVTYPE}=="partition", ENV{MD_DEVNAME}=="*[^0-9]", SYMLINK+="md/$env{MD_DEVNAME}%n"
 ENV{DEVTYPE}=="partition", ENV{MD_DEVNAME}=="*[0-9]", SYMLINK+="md/$env{MD_DEVNAME}p%n"
 
-IMPORT{program}="/sbin/blkid -o udev -p $tempnode"
+IMPORT{builtin}="blkid"
 OPTIONS+="link_priority=100"
 OPTIONS+="watch"
 ENV{ID_FS_USAGE}=="filesystem|other|crypto", ENV{ID_FS_UUID_ENC}=="?*", SYMLINK+="disk/by-uuid/$env{ID_FS_UUID_ENC}"
-- 
1.8.1.3

[PATCH 2/2] udev: Fix order of execution of the md rules

[Thomas Bächler]

Right now, the rules that run blkid on raid arrays are executed after
the assembly rules. This means incremental assembly will always fail
when raid arrays are again physical components of raid arrays.

Instead of simply reversing the order, split the rules up into two files,
one dealing with array properties and one dealing with assembly.
---
 Makefile                    |  5 +++--
 udev-md-raid-arrays.rules   | 33 ++++++++++++++++++++++++++++++
 udev-md-raid-assembly.rules | 19 ++++++++++++++++++
 udev-md-raid.rules          | 49 ---------------------------------------------
 4 files changed, 55 insertions(+), 51 deletions(-)
 create mode 100644 udev-md-raid-arrays.rules
 create mode 100644 udev-md-raid-assembly.rules
 delete mode 100644 udev-md-raid.rules

diff --git a/Makefile b/Makefile
index b6edb23..ad0819d 100644
--- a/Makefile
+++ b/Makefile
@@ -262,8 +262,9 @@ install-man: mdadm.8 md.4 mdadm.conf.5 mdmon.8
 	$(INSTALL) -D -m 644 md.4 $(DESTDIR)$(MAN4DIR)/md.4
 	$(INSTALL) -D -m 644 mdadm.conf.5 $(DESTDIR)$(MAN5DIR)/mdadm.conf.5
 
-install-udev: udev-md-raid.rules
-	$(INSTALL) -D -m 644 udev-md-raid.rules $(DESTDIR)$(UDEVDIR)/rules.d/64-md-raid.rules
+install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules
+	$(INSTALL) -D -m 644 udev-md-raid-arrays.rules $(DESTDIR)$(UDEVDIR)/rules.d/63-md-raid-arrays.rules
+	$(INSTALL) -D -m 644 udev-md-raid-assembly.rules $(DESTDIR)$(UDEVDIR)/rules.d/64-md-raid-assembly.rules
 
 install-systemd: systemd/mdmon@.service
 	$(INSTALL) -D -m 644 systemd/mdmon@.service $(DESTDIR)$(SYSTEMD_DIR)/mdmon@.service
diff --git a/udev-md-raid-arrays.rules b/udev-md-raid-arrays.rules
new file mode 100644
index 0000000..883ee4d
--- /dev/null
+++ b/udev-md-raid-arrays.rules
@@ -0,0 +1,33 @@
+# do not edit this file, it will be overwritten on update
+
+# handle md arrays
+ACTION!="add|change", GOTO="md_end"
+KERNEL!="md*", GOTO="md_end"
+
+# partitions have no md/{array_state,metadata_version}, but should not
+# for that reason be ignored.
+ENV{DEVTYPE}=="partition", GOTO="md_ignore_state"
+
+# container devices have a metadata version of e.g. 'external:ddf' and
+# never leave state 'inactive'
+ATTR{md/metadata_version}=="external:[A-Za-z]*", ATTR{md/array_state}=="inactive", GOTO="md_ignore_state"
+TEST!="md/array_state", ENV{SYSTEMD_READY}="0", GOTO="md_end"
+ATTR{md/array_state}=="|clear|inactive", ENV{SYSTEMD_READY}="0", GOTO="md_end"
+LABEL="md_ignore_state"
+
+IMPORT{program}="/sbin/mdadm --detail --export $devnode"
+ENV{DEVTYPE}=="disk", ENV{MD_NAME}=="?*", SYMLINK+="disk/by-id/md-name-$env{MD_NAME}", OPTIONS+="string_escape=replace"
+ENV{DEVTYPE}=="disk", ENV{MD_UUID}=="?*", SYMLINK+="disk/by-id/md-uuid-$env{MD_UUID}"
+ENV{DEVTYPE}=="disk", ENV{MD_DEVNAME}=="?*", SYMLINK+="md/$env{MD_DEVNAME}"
+ENV{DEVTYPE}=="partition", ENV{MD_NAME}=="?*", SYMLINK+="disk/by-id/md-name-$env{MD_NAME}-part%n", OPTIONS+="string_escape=replace"
+ENV{DEVTYPE}=="partition", ENV{MD_UUID}=="?*", SYMLINK+="disk/by-id/md-uuid-$env{MD_UUID}-part%n"
+ENV{DEVTYPE}=="partition", ENV{MD_DEVNAME}=="*[^0-9]", SYMLINK+="md/$env{MD_DEVNAME}%n"
+ENV{DEVTYPE}=="partition", ENV{MD_DEVNAME}=="*[0-9]", SYMLINK+="md/$env{MD_DEVNAME}p%n"
+
+IMPORT{builtin}="blkid"
+OPTIONS+="link_priority=100"
+OPTIONS+="watch"
+ENV{ID_FS_USAGE}=="filesystem|other|crypto", ENV{ID_FS_UUID_ENC}=="?*", SYMLINK+="disk/by-uuid/$env{ID_FS_UUID_ENC}"
+ENV{ID_FS_USAGE}=="filesystem|other", ENV{ID_FS_LABEL_ENC}=="?*", SYMLINK+="disk/by-label/$env{ID_FS_LABEL_ENC}"
+
+LABEL="md_end"
diff --git a/udev-md-raid-assembly.rules b/udev-md-raid-assembly.rules
new file mode 100644
index 0000000..b89775e
--- /dev/null
+++ b/udev-md-raid-assembly.rules
@@ -0,0 +1,19 @@
+# do not edit this file, it will be overwritten on update
+
+# assemble md arrays
+
+SUBSYSTEM!="block", GOTO="md_end"
+
+# handle potential components of arrays (the ones supported by md)
+ENV{ID_FS_TYPE}=="ddf_raid_member|isw_raid_member|linux_raid_member", GOTO="md_inc"
+GOTO="md_inc_end"
+
+LABEL="md_inc"
+
+# remember you can limit what gets auto/incrementally assembled by
+# mdadm.conf(5)'s 'AUTO' and selectively whitelist using 'ARRAY'
+ACTION=="add", RUN+="/sbin/mdadm --incremental $devnode --offroot"
+ACTION=="remove", ENV{ID_PATH}=="?*", RUN+="/sbin/mdadm -If $name --path $env{ID_PATH}"
+ACTION=="remove", ENV{ID_PATH}!="?*", RUN+="/sbin/mdadm -If $name"
+
+LABEL="md_inc_end"
diff --git a/udev-md-raid.rules b/udev-md-raid.rules
deleted file mode 100644
index ea97261..0000000
--- a/udev-md-raid.rules
+++ /dev/null
@@ -1,49 +0,0 @@
-# do not edit this file, it will be overwritten on update
-
-SUBSYSTEM!="block", GOTO="md_end"
-
-# handle potential components of arrays (the ones supported by md)
-ENV{ID_FS_TYPE}=="ddf_raid_member|isw_raid_member|linux_raid_member", GOTO="md_inc"
-GOTO="md_inc_skip"
-
-LABEL="md_inc"
-
-# remember you can limit what gets auto/incrementally assembled by
-# mdadm.conf(5)'s 'AUTO' and selectively whitelist using 'ARRAY'
-ACTION=="add", RUN+="/sbin/mdadm --incremental $devnode --offroot"
-ACTION=="remove", ENV{ID_PATH}=="?*", RUN+="/sbin/mdadm -If $name --path $env{ID_PATH}"
-ACTION=="remove", ENV{ID_PATH}!="?*", RUN+="/sbin/mdadm -If $name"
-
-LABEL="md_inc_skip"
-
-# handle md arrays
-ACTION!="add|change", GOTO="md_end"
-KERNEL!="md*", GOTO="md_end"
-
-# partitions have no md/{array_state,metadata_version}, but should not
-# for that reason be ignored.
-ENV{DEVTYPE}=="partition", GOTO="md_ignore_state"
-
-# container devices have a metadata version of e.g. 'external:ddf' and
-# never leave state 'inactive'
-ATTR{md/metadata_version}=="external:[A-Za-z]*", ATTR{md/array_state}=="inactive", GOTO="md_ignore_state"
-TEST!="md/array_state", ENV{SYSTEMD_READY}="0", GOTO="md_end"
-ATTR{md/array_state}=="|clear|inactive", ENV{SYSTEMD_READY}="0", GOTO="md_end"
-LABEL="md_ignore_state"
-
-IMPORT{program}="/sbin/mdadm --detail --export $devnode"
-ENV{DEVTYPE}=="disk", ENV{MD_NAME}=="?*", SYMLINK+="disk/by-id/md-name-$env{MD_NAME}", OPTIONS+="string_escape=replace"
-ENV{DEVTYPE}=="disk", ENV{MD_UUID}=="?*", SYMLINK+="disk/by-id/md-uuid-$env{MD_UUID}"
-ENV{DEVTYPE}=="disk", ENV{MD_DEVNAME}=="?*", SYMLINK+="md/$env{MD_DEVNAME}"
-ENV{DEVTYPE}=="partition", ENV{MD_NAME}=="?*", SYMLINK+="disk/by-id/md-name-$env{MD_NAME}-part%n", OPTIONS+="string_escape=replace"
-ENV{DEVTYPE}=="partition", ENV{MD_UUID}=="?*", SYMLINK+="disk/by-id/md-uuid-$env{MD_UUID}-part%n"
-ENV{DEVTYPE}=="partition", ENV{MD_DEVNAME}=="*[^0-9]", SYMLINK+="md/$env{MD_DEVNAME}%n"
-ENV{DEVTYPE}=="partition", ENV{MD_DEVNAME}=="*[0-9]", SYMLINK+="md/$env{MD_DEVNAME}p%n"
-
-IMPORT{builtin}="blkid"
-OPTIONS+="link_priority=100"
-OPTIONS+="watch"
-ENV{ID_FS_USAGE}=="filesystem|other|crypto", ENV{ID_FS_UUID_ENC}=="?*", SYMLINK+="disk/by-uuid/$env{ID_FS_UUID_ENC}"
-ENV{ID_FS_USAGE}=="filesystem|other", ENV{ID_FS_LABEL_ENC}=="?*", SYMLINK+="disk/by-label/$env{ID_FS_LABEL_ENC}"
-
-LABEL="md_end"
-- 
1.8.1.3

[PATCHv2 2/2] udev: Fix order of execution of the md rules

[Thomas Bächler]

Right now, the rules that run blkid on raid arrays are executed after
the assembly rules. This means incremental assembly will always fail
when raid arrays are again physical components of raid arrays.

Instead of simply reversing the order, split the rules up into two files,
one dealing with array properties and one dealing with assembly.
---
 Makefile                    |  5 +++--
 udev-md-raid-arrays.rules   | 35 ++++++++++++++++++++++++++++++++
 udev-md-raid-assembly.rules | 19 ++++++++++++++++++
 udev-md-raid.rules          | 49 ---------------------------------------------
 4 files changed, 57 insertions(+), 51 deletions(-)
 create mode 100644 udev-md-raid-arrays.rules
 create mode 100644 udev-md-raid-assembly.rules
 delete mode 100644 udev-md-raid.rules

diff --git a/Makefile b/Makefile
index b6edb23..ad0819d 100644
--- a/Makefile
+++ b/Makefile
@@ -262,8 +262,9 @@ install-man: mdadm.8 md.4 mdadm.conf.5 mdmon.8
 	$(INSTALL) -D -m 644 md.4 $(DESTDIR)$(MAN4DIR)/md.4
 	$(INSTALL) -D -m 644 mdadm.conf.5 $(DESTDIR)$(MAN5DIR)/mdadm.conf.5
 
-install-udev: udev-md-raid.rules
-	$(INSTALL) -D -m 644 udev-md-raid.rules $(DESTDIR)$(UDEVDIR)/rules.d/64-md-raid.rules
+install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules
+	$(INSTALL) -D -m 644 udev-md-raid-arrays.rules $(DESTDIR)$(UDEVDIR)/rules.d/63-md-raid-arrays.rules
+	$(INSTALL) -D -m 644 udev-md-raid-assembly.rules $(DESTDIR)$(UDEVDIR)/rules.d/64-md-raid-assembly.rules
 
 install-systemd: systemd/mdmon@.service
 	$(INSTALL) -D -m 644 systemd/mdmon@.service $(DESTDIR)$(SYSTEMD_DIR)/mdmon@.service
diff --git a/udev-md-raid-arrays.rules b/udev-md-raid-arrays.rules
new file mode 100644
index 0000000..0540ed8
--- /dev/null
+++ b/udev-md-raid-arrays.rules
@@ -0,0 +1,35 @@
+# do not edit this file, it will be overwritten on update
+
+SUBSYSTEM!="block", GOTO="md_end"
+
+# handle md arrays
+ACTION!="add|change", GOTO="md_end"
+KERNEL!="md*", GOTO="md_end"
+
+# partitions have no md/{array_state,metadata_version}, but should not
+# for that reason be ignored.
+ENV{DEVTYPE}=="partition", GOTO="md_ignore_state"
+
+# container devices have a metadata version of e.g. 'external:ddf' and
+# never leave state 'inactive'
+ATTR{md/metadata_version}=="external:[A-Za-z]*", ATTR{md/array_state}=="inactive", GOTO="md_ignore_state"
+TEST!="md/array_state", ENV{SYSTEMD_READY}="0", GOTO="md_end"
+ATTR{md/array_state}=="|clear|inactive", ENV{SYSTEMD_READY}="0", GOTO="md_end"
+LABEL="md_ignore_state"
+
+IMPORT{program}="/sbin/mdadm --detail --export $devnode"
+ENV{DEVTYPE}=="disk", ENV{MD_NAME}=="?*", SYMLINK+="disk/by-id/md-name-$env{MD_NAME}", OPTIONS+="string_escape=replace"
+ENV{DEVTYPE}=="disk", ENV{MD_UUID}=="?*", SYMLINK+="disk/by-id/md-uuid-$env{MD_UUID}"
+ENV{DEVTYPE}=="disk", ENV{MD_DEVNAME}=="?*", SYMLINK+="md/$env{MD_DEVNAME}"
+ENV{DEVTYPE}=="partition", ENV{MD_NAME}=="?*", SYMLINK+="disk/by-id/md-name-$env{MD_NAME}-part%n", OPTIONS+="string_escape=replace"
+ENV{DEVTYPE}=="partition", ENV{MD_UUID}=="?*", SYMLINK+="disk/by-id/md-uuid-$env{MD_UUID}-part%n"
+ENV{DEVTYPE}=="partition", ENV{MD_DEVNAME}=="*[^0-9]", SYMLINK+="md/$env{MD_DEVNAME}%n"
+ENV{DEVTYPE}=="partition", ENV{MD_DEVNAME}=="*[0-9]", SYMLINK+="md/$env{MD_DEVNAME}p%n"
+
+IMPORT{builtin}="blkid"
+OPTIONS+="link_priority=100"
+OPTIONS+="watch"
+ENV{ID_FS_USAGE}=="filesystem|other|crypto", ENV{ID_FS_UUID_ENC}=="?*", SYMLINK+="disk/by-uuid/$env{ID_FS_UUID_ENC}"
+ENV{ID_FS_USAGE}=="filesystem|other", ENV{ID_FS_LABEL_ENC}=="?*", SYMLINK+="disk/by-label/$env{ID_FS_LABEL_ENC}"
+
+LABEL="md_end"
diff --git a/udev-md-raid-assembly.rules b/udev-md-raid-assembly.rules
new file mode 100644
index 0000000..b653265
--- /dev/null
+++ b/udev-md-raid-assembly.rules
@@ -0,0 +1,19 @@
+# do not edit this file, it will be overwritten on update
+
+# assemble md arrays
+
+SUBSYSTEM!="block", GOTO="md_inc_end"
+
+# handle potential components of arrays (the ones supported by md)
+ENV{ID_FS_TYPE}=="ddf_raid_member|isw_raid_member|linux_raid_member", GOTO="md_inc"
+GOTO="md_inc_end"
+
+LABEL="md_inc"
+
+# remember you can limit what gets auto/incrementally assembled by
+# mdadm.conf(5)'s 'AUTO' and selectively whitelist using 'ARRAY'
+ACTION=="add", RUN+="/sbin/mdadm --incremental $devnode --offroot"
+ACTION=="remove", ENV{ID_PATH}=="?*", RUN+="/sbin/mdadm -If $name --path $env{ID_PATH}"
+ACTION=="remove", ENV{ID_PATH}!="?*", RUN+="/sbin/mdadm -If $name"
+
+LABEL="md_inc_end"
diff --git a/udev-md-raid.rules b/udev-md-raid.rules
deleted file mode 100644
index ea97261..0000000
--- a/udev-md-raid.rules
+++ /dev/null
@@ -1,49 +0,0 @@
-# do not edit this file, it will be overwritten on update
-
-SUBSYSTEM!="block", GOTO="md_end"
-
-# handle potential components of arrays (the ones supported by md)
-ENV{ID_FS_TYPE}=="ddf_raid_member|isw_raid_member|linux_raid_member", GOTO="md_inc"
-GOTO="md_inc_skip"
-
-LABEL="md_inc"
-
-# remember you can limit what gets auto/incrementally assembled by
-# mdadm.conf(5)'s 'AUTO' and selectively whitelist using 'ARRAY'
-ACTION=="add", RUN+="/sbin/mdadm --incremental $devnode --offroot"
-ACTION=="remove", ENV{ID_PATH}=="?*", RUN+="/sbin/mdadm -If $name --path $env{ID_PATH}"
-ACTION=="remove", ENV{ID_PATH}!="?*", RUN+="/sbin/mdadm -If $name"
-
-LABEL="md_inc_skip"
-
-# handle md arrays
-ACTION!="add|change", GOTO="md_end"
-KERNEL!="md*", GOTO="md_end"
-
-# partitions have no md/{array_state,metadata_version}, but should not
-# for that reason be ignored.
-ENV{DEVTYPE}=="partition", GOTO="md_ignore_state"
-
-# container devices have a metadata version of e.g. 'external:ddf' and
-# never leave state 'inactive'
-ATTR{md/metadata_version}=="external:[A-Za-z]*", ATTR{md/array_state}=="inactive", GOTO="md_ignore_state"
-TEST!="md/array_state", ENV{SYSTEMD_READY}="0", GOTO="md_end"
-ATTR{md/array_state}=="|clear|inactive", ENV{SYSTEMD_READY}="0", GOTO="md_end"
-LABEL="md_ignore_state"
-
-IMPORT{program}="/sbin/mdadm --detail --export $devnode"
-ENV{DEVTYPE}=="disk", ENV{MD_NAME}=="?*", SYMLINK+="disk/by-id/md-name-$env{MD_NAME}", OPTIONS+="string_escape=replace"
-ENV{DEVTYPE}=="disk", ENV{MD_UUID}=="?*", SYMLINK+="disk/by-id/md-uuid-$env{MD_UUID}"
-ENV{DEVTYPE}=="disk", ENV{MD_DEVNAME}=="?*", SYMLINK+="md/$env{MD_DEVNAME}"
-ENV{DEVTYPE}=="partition", ENV{MD_NAME}=="?*", SYMLINK+="disk/by-id/md-name-$env{MD_NAME}-part%n", OPTIONS+="string_escape=replace"
-ENV{DEVTYPE}=="partition", ENV{MD_UUID}=="?*", SYMLINK+="disk/by-id/md-uuid-$env{MD_UUID}-part%n"
-ENV{DEVTYPE}=="partition", ENV{MD_DEVNAME}=="*[^0-9]", SYMLINK+="md/$env{MD_DEVNAME}%n"
-ENV{DEVTYPE}=="partition", ENV{MD_DEVNAME}=="*[0-9]", SYMLINK+="md/$env{MD_DEVNAME}p%n"
-
-IMPORT{builtin}="blkid"
-OPTIONS+="link_priority=100"
-OPTIONS+="watch"
-ENV{ID_FS_USAGE}=="filesystem|other|crypto", ENV{ID_FS_UUID_ENC}=="?*", SYMLINK+="disk/by-uuid/$env{ID_FS_UUID_ENC}"
-ENV{ID_FS_USAGE}=="filesystem|other", ENV{ID_FS_LABEL_ENC}=="?*", SYMLINK+="disk/by-label/$env{ID_FS_LABEL_ENC}"
-
-LABEL="md_end"
-- 
1.8.1.3

Re: [PATCH 1/2] Modernize udev rules

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 2137 bytes --]

On Sat,  9 Feb 2013 18:48:38 +0100 Thomas Bächler <thomas@archlinux.org>
wrote:

> * $tempnode is deprecated, use $devnode
> * blkid -o udev output is deprecated, use IMPORT{builtin}="blkid" instead
> ---
>  udev-md-raid.rules | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/udev-md-raid.rules b/udev-md-raid.rules
> index cc7f5ef..ea97261 100644
> --- a/udev-md-raid.rules
> +++ b/udev-md-raid.rules
> @@ -10,7 +10,7 @@ LABEL="md_inc"
>  
>  # remember you can limit what gets auto/incrementally assembled by
>  # mdadm.conf(5)'s 'AUTO' and selectively whitelist using 'ARRAY'
> -ACTION=="add", RUN+="/sbin/mdadm --incremental $tempnode --offroot"
> +ACTION=="add", RUN+="/sbin/mdadm --incremental $devnode --offroot"
>  ACTION=="remove", ENV{ID_PATH}=="?*", RUN+="/sbin/mdadm -If $name --path $env{ID_PATH}"
>  ACTION=="remove", ENV{ID_PATH}!="?*", RUN+="/sbin/mdadm -If $name"
>  
> @@ -31,7 +31,7 @@ TEST!="md/array_state", ENV{SYSTEMD_READY}="0", GOTO="md_end"
>  ATTR{md/array_state}=="|clear|inactive", ENV{SYSTEMD_READY}="0", GOTO="md_end"
>  LABEL="md_ignore_state"
>  
> -IMPORT{program}="/sbin/mdadm --detail --export $tempnode"
> +IMPORT{program}="/sbin/mdadm --detail --export $devnode"
>  ENV{DEVTYPE}=="disk", ENV{MD_NAME}=="?*", SYMLINK+="disk/by-id/md-name-$env{MD_NAME}", OPTIONS+="string_escape=replace"
>  ENV{DEVTYPE}=="disk", ENV{MD_UUID}=="?*", SYMLINK+="disk/by-id/md-uuid-$env{MD_UUID}"
>  ENV{DEVTYPE}=="disk", ENV{MD_DEVNAME}=="?*", SYMLINK+="md/$env{MD_DEVNAME}"
> @@ -40,7 +40,7 @@ ENV{DEVTYPE}=="partition", ENV{MD_UUID}=="?*", SYMLINK+="disk/by-id/md-uuid-$env
>  ENV{DEVTYPE}=="partition", ENV{MD_DEVNAME}=="*[^0-9]", SYMLINK+="md/$env{MD_DEVNAME}%n"
>  ENV{DEVTYPE}=="partition", ENV{MD_DEVNAME}=="*[0-9]", SYMLINK+="md/$env{MD_DEVNAME}p%n"
>  
> -IMPORT{program}="/sbin/blkid -o udev -p $tempnode"
> +IMPORT{builtin}="blkid"
>  OPTIONS+="link_priority=100"
>  OPTIONS+="watch"
>  ENV{ID_FS_USAGE}=="filesystem|other|crypto", ENV{ID_FS_UUID_ENC}=="?*", SYMLINK+="disk/by-uuid/$env{ID_FS_UUID_ENC}"

Applied, thanks.

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCHv2 2/2] udev: Fix order of execution of the md rules

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 8640 bytes --]

On Sat,  9 Feb 2013 21:49:47 +0100 Thomas Bächler <thomas@archlinux.org>
wrote:

> Right now, the rules that run blkid on raid arrays are executed after
> the assembly rules. This means incremental assembly will always fail
> when raid arrays are again physical components of raid arrays.

I'm not at all against splitting the udev rules into two files, but your
reasoning seems a bit odd and I want to be sure that I understand.

We run "mdadm -I" based on the contents of ID_FS_TYPE, and ID_FS_TYPE is set
by "blkid", which is called after the "mdadm -I" rules.  So why do they work
at all?

Presumably something else is calling "blkid" ?
That seems to be "persistent-storage.rules"  which calls "blkid" on some
devices, but not on others.  That seems sub-optimal.   It is probably
reasonable that it skips "sr*" (which is explicit) but for us it is
unfortunate that it skips "md*".

I cannot see a good reason for persistent-storage-rules to skip 'md' devices,
so I would suggest that the best way to fix the problem is the fix that
rules file.  Then we could remove the "blkid" call from the md rules which is
nice as it removes duplication.

We could still split the md rules files, but I would rather do that because
it was a good and clean thing to do, not do that because it helps work around
a short-coming in some other rules files.

Thoughts?

Thanks,
NeilBrown



> 
> Instead of simply reversing the order, split the rules up into two files,
> one dealing with array properties and one dealing with assembly.
> ---
>  Makefile                    |  5 +++--
>  udev-md-raid-arrays.rules   | 35 ++++++++++++++++++++++++++++++++
>  udev-md-raid-assembly.rules | 19 ++++++++++++++++++
>  udev-md-raid.rules          | 49 ---------------------------------------------
>  4 files changed, 57 insertions(+), 51 deletions(-)
>  create mode 100644 udev-md-raid-arrays.rules
>  create mode 100644 udev-md-raid-assembly.rules
>  delete mode 100644 udev-md-raid.rules
> 
> diff --git a/Makefile b/Makefile
> index b6edb23..ad0819d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -262,8 +262,9 @@ install-man: mdadm.8 md.4 mdadm.conf.5 mdmon.8
>  	$(INSTALL) -D -m 644 md.4 $(DESTDIR)$(MAN4DIR)/md.4
>  	$(INSTALL) -D -m 644 mdadm.conf.5 $(DESTDIR)$(MAN5DIR)/mdadm.conf.5
>  
> -install-udev: udev-md-raid.rules
> -	$(INSTALL) -D -m 644 udev-md-raid.rules $(DESTDIR)$(UDEVDIR)/rules.d/64-md-raid.rules
> +install-udev: udev-md-raid-arrays.rules udev-md-raid-assembly.rules
> +	$(INSTALL) -D -m 644 udev-md-raid-arrays.rules $(DESTDIR)$(UDEVDIR)/rules.d/63-md-raid-arrays.rules
> +	$(INSTALL) -D -m 644 udev-md-raid-assembly.rules $(DESTDIR)$(UDEVDIR)/rules.d/64-md-raid-assembly.rules
>  
>  install-systemd: systemd/mdmon@.service
>  	$(INSTALL) -D -m 644 systemd/mdmon@.service $(DESTDIR)$(SYSTEMD_DIR)/mdmon@.service
> diff --git a/udev-md-raid-arrays.rules b/udev-md-raid-arrays.rules
> new file mode 100644
> index 0000000..0540ed8
> --- /dev/null
> +++ b/udev-md-raid-arrays.rules
> @@ -0,0 +1,35 @@
> +# do not edit this file, it will be overwritten on update
> +
> +SUBSYSTEM!="block", GOTO="md_end"
> +
> +# handle md arrays
> +ACTION!="add|change", GOTO="md_end"
> +KERNEL!="md*", GOTO="md_end"
> +
> +# partitions have no md/{array_state,metadata_version}, but should not
> +# for that reason be ignored.
> +ENV{DEVTYPE}=="partition", GOTO="md_ignore_state"
> +
> +# container devices have a metadata version of e.g. 'external:ddf' and
> +# never leave state 'inactive'
> +ATTR{md/metadata_version}=="external:[A-Za-z]*", ATTR{md/array_state}=="inactive", GOTO="md_ignore_state"
> +TEST!="md/array_state", ENV{SYSTEMD_READY}="0", GOTO="md_end"
> +ATTR{md/array_state}=="|clear|inactive", ENV{SYSTEMD_READY}="0", GOTO="md_end"
> +LABEL="md_ignore_state"
> +
> +IMPORT{program}="/sbin/mdadm --detail --export $devnode"
> +ENV{DEVTYPE}=="disk", ENV{MD_NAME}=="?*", SYMLINK+="disk/by-id/md-name-$env{MD_NAME}", OPTIONS+="string_escape=replace"
> +ENV{DEVTYPE}=="disk", ENV{MD_UUID}=="?*", SYMLINK+="disk/by-id/md-uuid-$env{MD_UUID}"
> +ENV{DEVTYPE}=="disk", ENV{MD_DEVNAME}=="?*", SYMLINK+="md/$env{MD_DEVNAME}"
> +ENV{DEVTYPE}=="partition", ENV{MD_NAME}=="?*", SYMLINK+="disk/by-id/md-name-$env{MD_NAME}-part%n", OPTIONS+="string_escape=replace"
> +ENV{DEVTYPE}=="partition", ENV{MD_UUID}=="?*", SYMLINK+="disk/by-id/md-uuid-$env{MD_UUID}-part%n"
> +ENV{DEVTYPE}=="partition", ENV{MD_DEVNAME}=="*[^0-9]", SYMLINK+="md/$env{MD_DEVNAME}%n"
> +ENV{DEVTYPE}=="partition", ENV{MD_DEVNAME}=="*[0-9]", SYMLINK+="md/$env{MD_DEVNAME}p%n"
> +
> +IMPORT{builtin}="blkid"
> +OPTIONS+="link_priority=100"
> +OPTIONS+="watch"
> +ENV{ID_FS_USAGE}=="filesystem|other|crypto", ENV{ID_FS_UUID_ENC}=="?*", SYMLINK+="disk/by-uuid/$env{ID_FS_UUID_ENC}"
> +ENV{ID_FS_USAGE}=="filesystem|other", ENV{ID_FS_LABEL_ENC}=="?*", SYMLINK+="disk/by-label/$env{ID_FS_LABEL_ENC}"
> +
> +LABEL="md_end"
> diff --git a/udev-md-raid-assembly.rules b/udev-md-raid-assembly.rules
> new file mode 100644
> index 0000000..b653265
> --- /dev/null
> +++ b/udev-md-raid-assembly.rules
> @@ -0,0 +1,19 @@
> +# do not edit this file, it will be overwritten on update
> +
> +# assemble md arrays
> +
> +SUBSYSTEM!="block", GOTO="md_inc_end"
> +
> +# handle potential components of arrays (the ones supported by md)
> +ENV{ID_FS_TYPE}=="ddf_raid_member|isw_raid_member|linux_raid_member", GOTO="md_inc"
> +GOTO="md_inc_end"
> +
> +LABEL="md_inc"
> +
> +# remember you can limit what gets auto/incrementally assembled by
> +# mdadm.conf(5)'s 'AUTO' and selectively whitelist using 'ARRAY'
> +ACTION=="add", RUN+="/sbin/mdadm --incremental $devnode --offroot"
> +ACTION=="remove", ENV{ID_PATH}=="?*", RUN+="/sbin/mdadm -If $name --path $env{ID_PATH}"
> +ACTION=="remove", ENV{ID_PATH}!="?*", RUN+="/sbin/mdadm -If $name"
> +
> +LABEL="md_inc_end"
> diff --git a/udev-md-raid.rules b/udev-md-raid.rules
> deleted file mode 100644
> index ea97261..0000000
> --- a/udev-md-raid.rules
> +++ /dev/null
> @@ -1,49 +0,0 @@
> -# do not edit this file, it will be overwritten on update
> -
> -SUBSYSTEM!="block", GOTO="md_end"
> -
> -# handle potential components of arrays (the ones supported by md)
> -ENV{ID_FS_TYPE}=="ddf_raid_member|isw_raid_member|linux_raid_member", GOTO="md_inc"
> -GOTO="md_inc_skip"
> -
> -LABEL="md_inc"
> -
> -# remember you can limit what gets auto/incrementally assembled by
> -# mdadm.conf(5)'s 'AUTO' and selectively whitelist using 'ARRAY'
> -ACTION=="add", RUN+="/sbin/mdadm --incremental $devnode --offroot"
> -ACTION=="remove", ENV{ID_PATH}=="?*", RUN+="/sbin/mdadm -If $name --path $env{ID_PATH}"
> -ACTION=="remove", ENV{ID_PATH}!="?*", RUN+="/sbin/mdadm -If $name"
> -
> -LABEL="md_inc_skip"
> -
> -# handle md arrays
> -ACTION!="add|change", GOTO="md_end"
> -KERNEL!="md*", GOTO="md_end"
> -
> -# partitions have no md/{array_state,metadata_version}, but should not
> -# for that reason be ignored.
> -ENV{DEVTYPE}=="partition", GOTO="md_ignore_state"
> -
> -# container devices have a metadata version of e.g. 'external:ddf' and
> -# never leave state 'inactive'
> -ATTR{md/metadata_version}=="external:[A-Za-z]*", ATTR{md/array_state}=="inactive", GOTO="md_ignore_state"
> -TEST!="md/array_state", ENV{SYSTEMD_READY}="0", GOTO="md_end"
> -ATTR{md/array_state}=="|clear|inactive", ENV{SYSTEMD_READY}="0", GOTO="md_end"
> -LABEL="md_ignore_state"
> -
> -IMPORT{program}="/sbin/mdadm --detail --export $devnode"
> -ENV{DEVTYPE}=="disk", ENV{MD_NAME}=="?*", SYMLINK+="disk/by-id/md-name-$env{MD_NAME}", OPTIONS+="string_escape=replace"
> -ENV{DEVTYPE}=="disk", ENV{MD_UUID}=="?*", SYMLINK+="disk/by-id/md-uuid-$env{MD_UUID}"
> -ENV{DEVTYPE}=="disk", ENV{MD_DEVNAME}=="?*", SYMLINK+="md/$env{MD_DEVNAME}"
> -ENV{DEVTYPE}=="partition", ENV{MD_NAME}=="?*", SYMLINK+="disk/by-id/md-name-$env{MD_NAME}-part%n", OPTIONS+="string_escape=replace"
> -ENV{DEVTYPE}=="partition", ENV{MD_UUID}=="?*", SYMLINK+="disk/by-id/md-uuid-$env{MD_UUID}-part%n"
> -ENV{DEVTYPE}=="partition", ENV{MD_DEVNAME}=="*[^0-9]", SYMLINK+="md/$env{MD_DEVNAME}%n"
> -ENV{DEVTYPE}=="partition", ENV{MD_DEVNAME}=="*[0-9]", SYMLINK+="md/$env{MD_DEVNAME}p%n"
> -
> -IMPORT{builtin}="blkid"
> -OPTIONS+="link_priority=100"
> -OPTIONS+="watch"
> -ENV{ID_FS_USAGE}=="filesystem|other|crypto", ENV{ID_FS_UUID_ENC}=="?*", SYMLINK+="disk/by-uuid/$env{ID_FS_UUID_ENC}"
> -ENV{ID_FS_USAGE}=="filesystem|other", ENV{ID_FS_LABEL_ENC}=="?*", SYMLINK+="disk/by-label/$env{ID_FS_LABEL_ENC}"
> -
> -LABEL="md_end"


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 1/2] Modernize udev rules

[Michael Tokarev]

11.02.2013 04:20, NeilBrown wrote:
> On Sat,  9 Feb 2013 18:48:38 +0100 Thomas Bächler <thomas@archlinux.org> wrote:

>> -IMPORT{program}="/sbin/blkid -o udev -p $tempnode"
>> +IMPORT{builtin}="blkid"

Does this has additional udev version constrain?
I mean, which is the minimum udev version to support this properly,
do we care?

Thanks,

/mjt
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 2/2] udev: Fix order of execution of the md rules

[Thomas Bächler]

[-- Attachment #1: Type: text/plain, Size: 2545 bytes --]

Am 11.02.2013 01:31, schrieb NeilBrown:
> On Sat,  9 Feb 2013 21:49:47 +0100 Thomas Bächler <thomas@archlinux.org>
> wrote:
> 
>> Right now, the rules that run blkid on raid arrays are executed after
>> the assembly rules. This means incremental assembly will always fail
>> when raid arrays are again physical components of raid arrays.
> 
> I'm not at all against splitting the udev rules into two files, but your
> reasoning seems a bit odd and I want to be sure that I understand.
> 
> We run "mdadm -I" based on the contents of ID_FS_TYPE, and ID_FS_TYPE is set
> by "blkid", which is called after the "mdadm -I" rules.  So why do they work
> at all?

If a new device, say /dev/md0 is added by udev, before the patch, the
rules would do the following:

1) go to md_inc_skip, because ID_FS_TYPE is unset, thus skip incremental
assembly.
2) call blkid and set ID_FS_TYPE

If ID_FS_TYPE is again linux_raid_member, then it would have been
necessary to call mdadm -I for /dev/md0 here.

This is probably what happens here: https://bugs.archlinux.org/task/32558

The short version is: The reporter joins three disks as a RAID0 (md2),
then combines md2 with another hard drive as RAID1.

With my changes, incremental assembly should work in this case, because
ID_FS_TYPE is set before incremental assembly tests for it.

> Presumably something else is calling "blkid" ?
> That seems to be "persistent-storage.rules"  which calls "blkid" on some
> devices, but not on others.  That seems sub-optimal.   It is probably
> reasonable that it skips "sr*" (which is explicit) but for us it is
> unfortunate that it skips "md*".

Yes, udev's standard rules skip md*, and I do not know why.

> I cannot see a good reason for persistent-storage-rules to skip 'md' devices,
> so I would suggest that the best way to fix the problem is the fix that
> rules file.  Then we could remove the "blkid" call from the md rules which is
> nice as it removes duplication.

CC'ing Kay here. Kay, is there a good reason to skip md* in
60-persistent-storage.rules?

> We could still split the md rules files, but I would rather do that because
> it was a good and clean thing to do, not do that because it helps work around
> a short-coming in some other rules files.

The "good reason" is that the rules fulfill two different tasks:
1) Apply incremental assembly to RAID members.
2) Set properties on RAID arrays.
Two tasks means two separate files, so you can easily disable or
override only one of them.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

Re: [PATCH 1/2] Modernize udev rules

[Thomas Bächler]

[-- Attachment #1: Type: text/plain, Size: 1342 bytes --]

Am 11.02.2013 09:33, schrieb Michael Tokarev:
> 11.02.2013 04:20, NeilBrown wrote:
>> On Sat,  9 Feb 2013 18:48:38 +0100 Thomas Bächler
>> <thomas@archlinux.org> wrote:
> 
>>> -IMPORT{program}="/sbin/blkid -o udev -p $tempnode"
>>> +IMPORT{builtin}="blkid"
> 
> Does this has additional udev version constrain?
> I mean, which is the minimum udev version to support this properly,
> do we care?

Udev relies on devtmpfs and thus does not create devices since version
176, therefore $tempnode is deprecated and only $devnode should be used
($tempnode seems to be around for compatibility, no idea if/when it will
be dropped).

The blkid builtin is also available since udev 176. In latest
util-linux, the 'blkid -o udev' output is marked as deprecated, although
Karel says it's not entirely going away for now.

Why do we care? We want correct udev rules ($tempnode vs. $devnode), and
the using builtin is faster and less error-prone than calling an
external tool (plus, it is used everywhere else in udev).

As for compatibility: I see no reason why anyone would use a version of
udev that is more than one year old (175) together with a mdadm version
that is to be released in the future (3.2.7). If someone really wants
that, they should be competent enough to adjust the udev rules for their
local build.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

Re: [PATCH 1/2] Modernize udev rules

[Michael Tokarev]

11.02.2013 14:11, Thomas Bächler wrote:
> Am 11.02.2013 09:33, schrieb Michael Tokarev:
>> 11.02.2013 04:20, NeilBrown wrote:
>>> On Sat,  9 Feb 2013 18:48:38 +0100 Thomas Bächler
>>> <thomas@archlinux.org> wrote:
>>
>>>> -IMPORT{program}="/sbin/blkid -o udev -p $tempnode"
>>>> +IMPORT{builtin}="blkid"
>>
>> Does this has additional udev version constrain?
>> I mean, which is the minimum udev version to support this properly,
>> do we care?
>
> Udev relies on devtmpfs and thus does not create devices since version
> 176, therefore $tempnode is deprecated and only $devnode should be used
> ($tempnode seems to be around for compatibility, no idea if/when it will
> be dropped).
>
> The blkid builtin is also available since udev 176. In latest
> util-linux, the 'blkid -o udev' output is marked as deprecated, although
> Karel says it's not entirely going away for now.

Thank you for the explanation.

> Why do we care? We want correct udev rules ($tempnode vs. $devnode), and
> the using builtin is faster and less error-prone than calling an
> external tool (plus, it is used everywhere else in udev).
>
> As for compatibility: I see no reason why anyone would use a version of
> udev that is more than one year old (175) together with a mdadm version
> that is to be released in the future (3.2.7). If someone really wants
> that, they should be competent enough to adjust the udev rules for their
> local build.

Well, it was just a question.  Think about a distribution which is being
upgraded from one release to another, -- inter-package versioned dependencies
should be set correctly, so that the system will not be broken by upgrading
mdadm which relies on udev, but not upgrading udev.  It's a good idea to
note somewhere that the next release of mdadm will require at least udev
175.  That's about it.

Thank you!

/mjt
>

--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Modernize udev rules

[Thomas Bächler]

[-- Attachment #1: Type: text/plain, Size: 383 bytes --]

Am 11.02.2013 11:16, schrieb Michael Tokarev:
> Well, it was just a question.  Think about a distribution which is being
> upgraded from one release to another, -- inter-package versioned
> dependencies
> should be set correctly, so that the system will not be broken by upgrading
> mdadm which relies on udev, but not upgrading udev.

That is true - and very important!



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

Re: [PATCH 1/2] Modernize udev rules

[Dmitrijs Ledkovs]

On 11 February 2013 10:11, Thomas Bächler <thomas@archlinux.org> wrote:
> Am 11.02.2013 09:33, schrieb Michael Tokarev:
>> 11.02.2013 04:20, NeilBrown wrote:
>>> On Sat,  9 Feb 2013 18:48:38 +0100 Thomas Bächler
>>> <thomas@archlinux.org> wrote:
>>
>>>> -IMPORT{program}="/sbin/blkid -o udev -p $tempnode"
>>>> +IMPORT{builtin}="blkid"
>>
>> Does this has additional udev version constrain?
>> I mean, which is the minimum udev version to support this properly,
>> do we care?
>
> Udev relies on devtmpfs and thus does not create devices since version
> 176, therefore $tempnode is deprecated and only $devnode should be used
> ($tempnode seems to be around for compatibility, no idea if/when it will
> be dropped).
>
> The blkid builtin is also available since udev 176. In latest

So 175 or 176? Neither debian nor ubuntu have 176 yet, only 175.
And upgrading udev in debian/ubuntu is proving to be a hard task.

Regards,

Dmitrijs.


> util-linux, the 'blkid -o udev' output is marked as deprecated, although
> Karel says it's not entirely going away for now.
>
> Why do we care? We want correct udev rules ($tempnode vs. $devnode), and
> the using builtin is faster and less error-prone than calling an
> external tool (plus, it is used everywhere else in udev).
>
> As for compatibility: I see no reason why anyone would use a version of
> udev that is more than one year old (175) together with a mdadm version
> that is to be released in the future (3.2.7). If someone really wants
> that, they should be competent enough to adjust the udev rules for their
> local build.
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Modernize udev rules

[Thomas Bächler]

[-- Attachment #1: Type: text/plain, Size: 1097 bytes --]

Am 11.02.2013 11:44, schrieb Dmitrijs Ledkovs:
> On 11 February 2013 10:11, Thomas Bächler <thomas@archlinux.org> wrote:
>> Am 11.02.2013 09:33, schrieb Michael Tokarev:
>>> 11.02.2013 04:20, NeilBrown wrote:
>>>> On Sat,  9 Feb 2013 18:48:38 +0100 Thomas Bächler
>>>> <thomas@archlinux.org> wrote:
>>>
>>>>> -IMPORT{program}="/sbin/blkid -o udev -p $tempnode"
>>>>> +IMPORT{builtin}="blkid"
>>>
>>> Does this has additional udev version constrain?
>>> I mean, which is the minimum udev version to support this properly,
>>> do we care?
>>
>> Udev relies on devtmpfs and thus does not create devices since version
>> 176, therefore $tempnode is deprecated and only $devnode should be used
>> ($tempnode seems to be around for compatibility, no idea if/when it will
>> be dropped).
>>
>> The blkid builtin is also available since udev 176. In latest
> 
> So 175 or 176? Neither debian nor ubuntu have 176 yet, only 175.
> And upgrading udev in debian/ubuntu is proving to be a hard task.

If I see this correctly, this was added after 175, so it first appeard
in 176.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]