[systemd-devel] [PATCH] rules: Fix by-path of mmc RPMB partitions and don't blkid them

2015-02-12 Thread Martin Pitt
Linux 3.10+ exposes RPMB (Replay Protected Memory Block) partitions of MMC
devices [1] ; trying to read them with blkid or other unspecific means will
cause kernel buffer I/O errors and timeouts. So don't run blkid on these.

Also ensure that /dev/disk/by-path creates proper symlinks and exposes the
-rpmb partition separately, instead of letting the "normal" partition symlink
point to the rpbm device (this is a race condition).

[1] 
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=090d25fe224c0

https://launchpad.net/bugs/1333140
---
 rules/60-persistent-storage.rules | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/rules/60-persistent-storage.rules 
b/rules/60-persistent-storage.rules
index 475b151..19fc4dc 100644
--- a/rules/60-persistent-storage.rules
+++ b/rules/60-persistent-storage.rules
@@ -53,7 +53,8 @@ KERNEL=="mspblk[0-9]p[0-9]", ENV{ID_NAME}=="?*", 
ENV{ID_SERIAL}=="?*", SYMLINK+=
 
 # by-path (parent device path)
 ENV{DEVTYPE}=="disk", DEVPATH!="*/virtual/*", IMPORT{builtin}="path_id"
-ENV{DEVTYPE}=="disk", ENV{ID_PATH}=="?*", SYMLINK+="disk/by-path/$env{ID_PATH}"
+ENV{DEVTYPE}=="disk", ENV{ID_PATH}=="?*", KERNEL=="mmcblk[0-9]rpmb", 
SYMLINK+="disk/by-path/$env{ID_PATH}-rpmb"
+ENV{DEVTYPE}=="disk", ENV{ID_PATH}=="?*", KERNEL!="mmcblk[0-9]rpmb", 
SYMLINK+="disk/by-path/$env{ID_PATH}"
 ENV{DEVTYPE}=="partition", ENV{ID_PATH}=="?*", 
SYMLINK+="disk/by-path/$env{ID_PATH}-part%n"
 
 # skip unpartitioned removable media devices from drivers which do not send 
"change" events
@@ -66,6 +67,9 @@ KERNEL=="sr*", ENV{DISK_EJECT_REQUEST}!="?*", 
ENV{ID_CDROM_MEDIA_TRACK_COUNT_DAT
 KERNEL=="sr*", ENV{DISK_EJECT_REQUEST}!="?*", 
ENV{ID_CDROM_MEDIA_TRACK_COUNT_DATA}=="?*", 
ENV{ID_CDROM_MEDIA_SESSION_LAST_OFFSET}=="", \
   IMPORT{builtin}="blkid --noraid"
 
+# don't try to read Replay Protected Memory Block partitions
+KERNEL=="mmcblk[0-9]rpmb", GOTO="persistent_storage_end"
+
 # probe filesystem metadata of disks
 KERNEL!="sr*", IMPORT{builtin}="blkid"
 
-- 
2.1.4

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] rules: Fix by-path of mmc RPMB partitions and don't blkid them

2015-02-12 Thread Lennart Poettering
On Thu, 12.02.15 18:11, Martin Pitt (martin.p...@ubuntu.com) wrote:

> Linux 3.10+ exposes RPMB (Replay Protected Memory Block) partitions of MMC
> devices [1] ; trying to read them with blkid or other unspecific means will
> cause kernel buffer I/O errors and timeouts. So don't run blkid on these.
> 
> Also ensure that /dev/disk/by-path creates proper symlinks and exposes the
> -rpmb partition separately, instead of letting the "normal" partition symlink
> point to the rpbm device (this is a race condition).
> 
> [1]
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=090d25fe224c0

Looks simple enough. Though I wonder if [0-9] is the right match,
shouldn't it also cover multiple numbers there?

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] rules: Fix by-path of mmc RPMB partitions and don't blkid them

2015-02-12 Thread Martin Pitt
Lennart Poettering [2015-02-12 18:50 +0100]:
> > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=090d25fe224c0
> 
> Looks simple enough. Though I wonder if [0-9] is the right match,
> shouldn't it also cover multiple numbers there?

Hm, yes. fnmatch() doesn't support a regexp-like "one or more digits";
it's less precise, but as the kernel doesn't generate arbitrary device
names but at most things like "mmcblk0p1", extending the glob to
mmcblk[0-9]*rpmb is actually safe.

Thanks,

Martin

-- 
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
From 23ab7d9cf443653c1a8c54a3ab154fc808a5a0e8 Mon Sep 17 00:00:00 2001
From: Martin Pitt 
Date: Wed, 11 Feb 2015 15:26:52 +0100
Subject: [PATCH] rules: Fix by-path of mmc RPMB partitions and don't blkid
 them

Linux 3.10+ exposes RPMB (Replay Protected Memory Block) partitions of MMC
devices [1] ; trying to read them with blkid or other unspecific means will
cause kernel buffer I/O errors and timeouts. So don't run blkid on these.

Also ensure that /dev/disk/by-path creates proper symlinks and exposes the
-rpmb partition separately, instead of letting the "normal" partition symlink
point to the rpbm device (this is a race condition).

[1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=090d25fe224c0

https://launchpad.net/bugs/1333140
---
 rules/60-persistent-storage.rules | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/rules/60-persistent-storage.rules b/rules/60-persistent-storage.rules
index 475b151..08ed1cc 100644
--- a/rules/60-persistent-storage.rules
+++ b/rules/60-persistent-storage.rules
@@ -53,7 +53,8 @@ KERNEL=="mspblk[0-9]p[0-9]", ENV{ID_NAME}=="?*", ENV{ID_SERIAL}=="?*", SYMLINK+=
 
 # by-path (parent device path)
 ENV{DEVTYPE}=="disk", DEVPATH!="*/virtual/*", IMPORT{builtin}="path_id"
-ENV{DEVTYPE}=="disk", ENV{ID_PATH}=="?*", SYMLINK+="disk/by-path/$env{ID_PATH}"
+ENV{DEVTYPE}=="disk", ENV{ID_PATH}=="?*", KERNEL=="mmcblk[0-9]*rpmb", SYMLINK+="disk/by-path/$env{ID_PATH}-rpmb"
+ENV{DEVTYPE}=="disk", ENV{ID_PATH}=="?*", KERNEL!="mmcblk[0-9]*rpmb", SYMLINK+="disk/by-path/$env{ID_PATH}"
 ENV{DEVTYPE}=="partition", ENV{ID_PATH}=="?*", SYMLINK+="disk/by-path/$env{ID_PATH}-part%n"
 
 # skip unpartitioned removable media devices from drivers which do not send "change" events
@@ -66,6 +67,9 @@ KERNEL=="sr*", ENV{DISK_EJECT_REQUEST}!="?*", ENV{ID_CDROM_MEDIA_TRACK_COUNT_DAT
 KERNEL=="sr*", ENV{DISK_EJECT_REQUEST}!="?*", ENV{ID_CDROM_MEDIA_TRACK_COUNT_DATA}=="?*", ENV{ID_CDROM_MEDIA_SESSION_LAST_OFFSET}=="", \
   IMPORT{builtin}="blkid --noraid"
 
+# don't try to read Replay Protected Memory Block partitions
+KERNEL=="mmcblk[0-9]*rpmb", GOTO="persistent_storage_end"
+
 # probe filesystem metadata of disks
 KERNEL!="sr*", IMPORT{builtin}="blkid"
 
-- 
2.1.4



signature.asc
Description: Digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] rules: Fix by-path of mmc RPMB partitions and don't blkid them

2015-02-12 Thread Martin Pitt
Martin Pitt [2015-02-12 19:52 +0100]:
> Lennart Poettering [2015-02-12 18:50 +0100]:
> > > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=090d25fe224c0
> > 
> > Looks simple enough. Though I wonder if [0-9] is the right match,
> > shouldn't it also cover multiple numbers there?
> 
> Hm, yes. fnmatch() doesn't support a regexp-like "one or more digits";
> it's less precise, but as the kernel doesn't generate arbitrary device
> names but at most things like "mmcblk0p1", extending the glob to
> mmcblk[0-9]*rpmb is actually safe.

Pushed now with this change:

  http://cgit.freedesktop.org/systemd/systemd/commit/?id=b87b01c

Martin
-- 
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)


signature.asc
Description: Digital signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] rules: Fix by-path of mmc RPMB partitions and don't blkid them

2015-02-13 Thread Kay Sievers
On Fri, Feb 13, 2015 at 8:51 AM, Martin Pitt  wrote:
> Martin Pitt [2015-02-12 19:52 +0100]:
>> Lennart Poettering [2015-02-12 18:50 +0100]:
>> > > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=090d25fe224c0
>> >
>> > Looks simple enough.

This looks awful. We should not litter generic rules with exotic niche
use cases like this. It will end up in a mess.

>> >Though I wonder if [0-9] is the right match,
>> > shouldn't it also cover multiple numbers there?
>>
>> Hm, yes. fnmatch() doesn't support a regexp-like "one or more digits";
>> it's less precise, but as the kernel doesn't generate arbitrary device
>> names but at most things like "mmcblk0p1", extending the glob to
>> mmcblk[0-9]*rpmb is actually safe.
>
> Pushed now with this change:
>
>   http://cgit.freedesktop.org/systemd/systemd/commit/?id=b87b01c

First, what is the use-case for by-path for mmc devices? If there is
no strong one, which I suspect, please just remove these rules again.

And we should not add more blacklists, especially not "randomly"
between multiple rules in the middle of a file.

This should be sorted out and be be fixed before the next release.

Thanks,
Kay
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] rules: Fix by-path of mmc RPMB partitions and don't blkid them

2015-02-13 Thread Martin Pitt
Kay Sievers [2015-02-13 10:12 +0100]:
> This looks awful. We should not litter generic rules with exotic niche
> use cases like this. It will end up in a mess.

Fully agreed :/

> First, what is the use-case for by-path for mmc devices? If there is
> no strong one, which I suspect, please just remove these rules again.

At least with the current blacklist approach, this is the important
bit:

| -ENV{DEVTYPE}=="disk", ENV{ID_PATH}=="?*", 
SYMLINK+="disk/by-path/$env{ID_PATH}"
| +ENV{DEVTYPE}=="disk", ENV{ID_PATH}=="?*", KERNEL!="mmcblk[0-9]*rpmb", 
SYMLINK+="disk/by-path/$env{ID_PATH}"

i. e. this fixes /dev/disks/by-path/mumble sometimes pointing to
/dev/mmcblk0 (correct) and sometimes pointing to /dev/mmcblk0rpmb
(awfully wrong)

| +ENV{DEVTYPE}=="disk", ENV{ID_PATH}=="?*", KERNEL=="mmcblk[0-9]*rpmb", 
SYMLINK+="disk/by-path/$env{ID_PATH}-rpmb"

This was more or less for completeness. I honestly don't know what to
do with these devices, I'm happy if we drop that rule again.

Martin
-- 
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] rules: Fix by-path of mmc RPMB partitions and don't blkid them

2015-02-13 Thread Kay Sievers
On Fri, Feb 13, 2015 at 10:35 AM, Martin Pitt  wrote:
> Kay Sievers [2015-02-13 10:12 +0100]:
>> This looks awful. We should not litter generic rules with exotic niche
>> use cases like this. It will end up in a mess.
>
> Fully agreed :/
>
>> First, what is the use-case for by-path for mmc devices? If there is
>> no strong one, which I suspect, please just remove these rules again.
>
> At least with the current blacklist approach, this is the important
> bit:
>
> | -ENV{DEVTYPE}=="disk", ENV{ID_PATH}=="?*", 
> SYMLINK+="disk/by-path/$env{ID_PATH}"
> | +ENV{DEVTYPE}=="disk", ENV{ID_PATH}=="?*", KERNEL!="mmcblk[0-9]*rpmb", 
> SYMLINK+="disk/by-path/$env{ID_PATH}"
>
> i. e. this fixes /dev/disks/by-path/mumble sometimes pointing to
> /dev/mmcblk0 (correct) and sometimes pointing to /dev/mmcblk0rpmb
> (awfully wrong)

Sure, as long as we have the blacklist and not a whitelist, we can add
the rpmb devices to the current list at the top of the file.

> | +ENV{DEVTYPE}=="disk", ENV{ID_PATH}=="?*", KERNEL=="mmcblk[0-9]*rpmb", 
> SYMLINK+="disk/by-path/$env{ID_PATH}-rpmb"
>
> This was more or less for completeness. I honestly don't know what to
> do with these devices, I'm happy if we drop that rule again.

Let's remove it. There was never a reason for by-path/ to cover all
exotic device types. disk/by-path/ is for slots, chassis, ports where
swappable disks can be connected and disconnected without the
identifier to change. I doubt that applies to mmc devices.

Kay
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] rules: Fix by-path of mmc RPMB partitions and don't blkid them

2015-02-13 Thread Martin Pitt
Hey Kay,

Kay Sievers [2015-02-13 10:47 +0100]:
> Let's remove it. There was never a reason for by-path/ to cover all
> exotic device types. disk/by-path/ is for slots, chassis, ports where
> swappable disks can be connected and disconnected without the
> identifier to change. I doubt that applies to mmc devices.

As discussed on IRC:

  http://cgit.freedesktop.org/systemd/systemd/commit/?id=0c13be389

Martin
-- 
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel


Re: [systemd-devel] [PATCH] rules: Fix by-path of mmc RPMB partitions and don't blkid them

2015-03-12 Thread Kay Sievers
On Fri, Feb 13, 2015 at 10:35 AM, Martin Pitt  wrote:
> Kay Sievers [2015-02-13 10:12 +0100]:
>> This looks awful. We should not litter generic rules with exotic niche
>> use cases like this. It will end up in a mess.
>
> Fully agreed :/

It is a rather small whitelist for now, which should not catch any
newly added device driver added in the future.

Kay
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel