Linux Hotplug development
 help / color / mirror / Atom feed
* Re: [PATCH] udevd: help to debug timouts of RUN+=""
From: Harald Hoyer @ 2011-04-18  8:28 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <1303113946-3692-1-git-send-email-harald@redhat.com>

Am 18.04.2011 10:05, schrieb harald@redhat.com:
> From: Harald Hoyer <harald@redhat.com>
> 
> Store pointer of command to RUN in the event, to aid debugging of timed
> out events.

forget it... will send correct patch later.

^ permalink raw reply

* [PATCH] udevd: help to debug timouts of RUN+=""
From: harald @ 2011-04-18  8:05 UTC (permalink / raw)
  To: linux-hotplug

From: Harald Hoyer <harald@redhat.com>

Store pointer of command to RUN in the event, to aid debugging of timed
out events.
---
 udev/udev-event.c |    2 ++
 udev/udevd.c      |    8 +++++++-
 2 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/udev/udev-event.c b/udev/udev-event.c
index 60d06aa..6ac6ff5 100644
--- a/udev/udev-event.c
+++ b/udev/udev-event.c
@@ -688,10 +688,12 @@ int udev_event_execute_run(struct udev_event *event, const sigset_t *sigmask)
 				info(event->udev, "delay execution of '%s'\n", program);
 				sleep(event->exec_delay);
 			}
+			event->program = program;
 			if (util_run_program(event->udev, program, envp, NULL, 0, NULL, sigmask) != 0) {
 				if (udev_list_entry_get_flags(list_entry))
 					err = -1;
 			}
+			event->program = NULL;
 		}
 	}
 	return err;
diff --git a/udev/udevd.c b/udev/udevd.c
index f730cab..47bc2f4 100644
--- a/udev/udevd.c
+++ b/udev/udevd.c
@@ -106,6 +106,7 @@ struct event {
 	dev_t devnum;
 	bool is_block;
 	int ifindex;
+	char *program;
 };
 
 static struct event *node_to_event(struct udev_list_node *node)
@@ -807,9 +808,14 @@ static void handle_signal(struct udev *udev, int signo)
 
 				info(udev, "worker [%u] exit\n", pid);
 				if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {
-					err(udev, "worker [%u] unexpectedly returned with status 0x%04x\n", pid, status);
+					if (WIFSIGNALED(status))
+						err(udev, "worker [%u] killed by signal %d\n", pid, WTERMSIG(status));
+					if (WEXITSTATUS(status))
+						err(udev, "worker [%u] unexpectedly returned with status %d\n", pid, WEXITSTATUS(status));
 					if (worker->event != NULL) {
 						err(udev, "worker [%u] failed while handling '%s'\n", pid, worker->event->devpath);
+						if (worker->event->program)
+							err(udev, "worker [%u] failed while running '%s'\n", pid, worker->event->program);
 						worker->event->exitcode = -32;
 						event_queue_delete(worker->event, true);
 						/* drop reference from running event */
-- 
1.7.3.4


^ permalink raw reply related

* Re: 75-persistent-net-generator.rules on Hyper-V virtual interfaces
From: Kay Sievers @ 2011-04-18  3:14 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <EB4A396CAC8FC54BB5AA357485423C8E03B4D5B6@mbx-e1-nj-2.intermedia-inc.net>

On Wed, Apr 6, 2011 at 17:12, Leonid Antonenkov <antonenk@intermedia.net> wrote:
> Hello!
>
> Problem: Hyper-V changes MAC address of virtual network adapter on every migration,
> So  '75-persistent-net-generator.rules' script changes network interface number.

Applied.

Thanks,
Kay

^ permalink raw reply

* Re: [PULL] Docs: README: Copyediting
From: Kay Sievers @ 2011-04-18  3:11 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <6169a635-4560-4e13-822f-a815cc6e7a4c-mfwitten@gmail.com>

On Fri, Apr 15, 2011 at 19:40, Michael Witten <mfwitten@gmail.com> wrote:
> On Mon, 11 Apr 2011 06:22:57 +0000, Michael Witten wrote:
>
>> I did some copyediting whilst looking through the README;
>> this email contains a pull request as well as the inlined
>> diff of all the changes squashed together.

Pulled, and pushed.

Thanks for doing that,
Kay

^ permalink raw reply

* Re: [PULL] Docs: udev.xml: Copyediting
From: Michael Witten @ 2011-04-15 17:41 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <5cb4f3ce-b6e7-4c2a-b175-2081daad3acb-mfwitten@gmail.com>

On Mon, 11 Apr 2011 06:31:30 +0000, Michael Witten wrote:

> I did some copyediting whilst looking through udev/udev.xml;
> this email contains a pull request as well as the inlined
> diff of all the changes squashed together.
>
> The changes apply cleanly to 960250952193db522b8ced336bf998bd7e8e4a31:
>
>   selinux: firmware - do not label files in runtime dir (2011-04-08 03:08:16 +0200)
>
> and are available here:
>
>   git://github.com/mfwitten/udev.git docs/udev.xml
>
> Squash them if desired.
>
>       Docs: udev.xml: Offset daemon name with commas
>       Docs: udev.xml: Remove commas (and unnecessary repetition)
>       Docs: udev.xml: `are' -> `is'; the subject is `Access'
>       Docs: udev.xml: Use present tense
>       Docs: udev.xml: Clarification through proper wording
>       Docs: udev.xml: `,' -> `;'
>       Docs: udev.xml: `key value' -> `key-value'
>       Docs: udev.xml: `,' -> `:'
>       Docs: udev.xml: Use `assignment' consistently
>       Docs: udev.xml: `comma-separated' is a better description
>       Docs: udev.xml: Remove unnecessary repitition
>       Docs: udev.xml: Add a few more words for context
>       Docs: udev.xml: Use `unless' for clarity
>       Docs: udev.xml: Clarify PROGRAM key
>       Docs: udev.xml: `a shell style' -> `shell-style'
>       Docs: udev.xml: Clean `*' description
>       Docs: udev.xml: Clean character range description
>       Docs: udev.xml: Clean up description of NAME assignment key
>       Docs: udev.xml: Clean up description of SYMLINK assignment key
>       Docs: udev.xml: Clean up description of ENV assignment key
>       Docs: udev.xml: Clean up description of RUN assignment key
>       Docs: udev.xml: Clean up description of LABEL assignment key
>       Docs: udev.xml: Add missing `.'
>       Docs: udev.xml: `which' -> `content of which'
>       Docs: udev.xml: `commandline' -> `command line'
>       Docs: udev.xml: Clean up WAIT_FOR description
>       Docs: udev.xml: `a' -> `the'
>       Docs: udev.xml: Clean up introduction to substitutions.
>       Docs: udev.xml: Use normal sentence structure
>       Docs: udev.xml: Actually make a separate paragraph
>       Docs: udev.xml: Add comma
>       Docs: udev.xml: `char' -> `character'
>       Docs: udev.xml: `comma-separated' is a better description
>       Docs: udev.xml: Clarify through a change in word ordering
>       Docs: udev.xml: Improved word order
>       Docs: udev.xml: Fix dangling modifier
>
>  udev/udev.xml |  169 +++++++++++++++++++++++++++++----------------------------
>  1 files changed, 86 insertions(+), 83 deletions(-)
>
> [diff]

Would someone like to commit this?

^ permalink raw reply

* Re: [PULL] Docs: README: Copyediting
From: Michael Witten @ 2011-04-15 17:40 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <6169a635-4560-4e13-822f-a815cc6e7a4c-mfwitten@gmail.com>

On Mon, 11 Apr 2011 06:22:57 +0000, Michael Witten wrote:

> I did some copyediting whilst looking through the README;
> this email contains a pull request as well as the inlined
> diff of all the changes squashed together.
>
> The changes apply cleanly to 960250952193db522b8ced336bf998bd7e8e4a31:
>
>   selinux: firmware - do not label files in runtime dir (2011-04-08 03:08:16 +0200)
>
> and are available here:
>
>   git://github.com/mfwitten/udev.git docs/README
>
> Squash them if desired.
>
>       Docs: README: `to replace' -> `replacing'
>       Docs: README: `,' -> `;'
>       Docs: README: Clean up a sentence
>       Docs: README: Use present tense
>       Docs: README: Add missing `and'
>       Docs: README: Remove commas and use subjective mood
>       Docs: README: Clean up `udev extras' requirements
>       Docs: README: Clarify configuration of existing devices
>       Docs: README: `does never apply' -> `never applies'
>       Docs: README: Flip sentence structure to improve wording
>       Docs: README: `set up' is the verb; `setup' is a noun
>       Docs: README: Add a comma to offset the modifier
>
>  README |   44 ++++++++++++++++++++++----------------------
>  1 files changed, 22 insertions(+), 22 deletions(-)
>
> [diff]

Would someone like to commit this?

^ permalink raw reply

* a library for udev rules?
From: Martin Vidner @ 2011-04-15 11:43 UTC (permalink / raw)
  To: linux-hotplug

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

Hi, 

as I am refactoring some messy code in yast2-network.rpm, I found a
part dealing with udev rules
(/etc/udev/rules.d/70-persistent-net.rules in particular).

Do you know of any library dealing with the rule configuration
files? AFAIK libudev only deals with the events and devices, not
with the config.  My search has not found an Augeas lens for the
rules, nothing for perl or python or ruby.

If I go about writing such a library, would you be interested in
having it upstream?
-- 
Martin Vidner, YaST developer
http://en.opensuse.org/User:Mvidner

Kuracke oddeleni v restauraci je jako fekalni oddeleni v bazenu

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: udev and systemd socket activation
From: Greg KH @ 2011-04-14 23:51 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <BANLkTik0WUkH5Fu7LmVmptT9j4cDnBitAw@mail.gmail.com>

On Fri, Apr 15, 2011 at 01:34:50AM +0200, Kay Sievers wrote:
> Just a heads up,
> udev -git has new systemd socket/service files that will work only
> with current systemd -git. Older systemd versions with this version of
> udev will not properly boot up!
> 
> The new udev.socket file will instruct systemd to bind the udev
> control socket and the netlink uevent socket, and let systemd
> auto-start udevd when any events arrive.
> 
> It looks like this:
>   # udevd runs
>   $ pidof udevd
>   4544 4543 4401
> 
>   # kill it
>   $ udevadm control --exit
>   $ pidof udevd
> 
>   # trigger any device event
>   $ echo change > /sys/class/mem/null/uevent
> 
>   # udevd is back and has handled this event
>   $ pidof udevd
>   4624 4623

Very cool stuff, thanks for doing this work!

greg k-h

^ permalink raw reply

* udev and systemd socket activation
From: Kay Sievers @ 2011-04-14 23:34 UTC (permalink / raw)
  To: linux-hotplug

Just a heads up,
udev -git has new systemd socket/service files that will work only
with current systemd -git. Older systemd versions with this version of
udev will not properly boot up!

The new udev.socket file will instruct systemd to bind the udev
control socket and the netlink uevent socket, and let systemd
auto-start udevd when any events arrive.

It looks like this:
  # udevd runs
  $ pidof udevd
  4544 4543 4401

  # kill it
  $ udevadm control --exit
  $ pidof udevd

  # trigger any device event
  $ echo change > /sys/class/mem/null/uevent

  # udevd is back and has handled this event
  $ pidof udevd
  4624 4623

This will enable us in the future to upgrade udev on the running
system without missing any event. Even without a running udevd, all
events will be queued up in the still open socket in pid 1.

Note: during package upgrade any incoming packet from the kernel would
start udevd. To prevent that, the udev socket needs to be stopped
during that window. Or in case a seamless upgrade is wanted, the
udev.service needs to be masked while the udev files are replaced on
disk. After unmasking the udev.service, the next incoming or already
queued events will start the daemon automatically.

Kay

^ permalink raw reply

* Re: why does udev-167 break /lib64/udev/firmware ?
From: Kay Sievers @ 2011-04-14 23:00 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <201104141753.22335.jason.vas.dias@gmail.com>

On Fri, Apr 15, 2011 at 00:56, Jason Vas Dias <jason.vas.dias@gmail.com> wrote:
> On Thursday 14 April 2011 22:26:58 Kay Sievers wrote:
>> > Maybe the libusb message I was seeing was an artefact, but really, why are you using an executable to do a shell script's job ?
>>
>> Because we need to encode filenames in that binary.
>>
> ever heard of cpio ? the kernel understands that.

Ever heard of reading source code, before asking needless questions?

Kay

^ permalink raw reply

* Re: why does udev-167 break /lib64/udev/firmware ?
From: Jason Vas Dias @ 2011-04-14 22:56 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <201104141753.22335.jason.vas.dias@gmail.com>

On Thursday 14 April 2011 22:26:58 Kay Sievers wrote:
> > Maybe the libusb message I was seeing was an artefact, but really, why are you using an executable to do a shell script's job ?
> 
> Because we need to encode filenames in that binary.
> 
ever heard of cpio ? the kernel understands that.
Why do you need to encode filenames ? surely /sys/devices/.../ etc is "trusted" ? 
In what way are filenames encoded ? or do you mean "encrypted" - why ? 
b43 / ssb / /usr/src/linux/drivers/base/firmware_class.c doesn't seem to mention "encoded" or "encrypted" filenames
and seem to work fine without any filenames being sent ?  Why isn't this "filename encoding" interface to 
drivers/base/class/firmware* documented anywhere ?



^ permalink raw reply

* Re: why does udev-167 break /lib64/udev/firmware ?
From: Kay Sievers @ 2011-04-14 21:26 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <201104141753.22335.jason.vas.dias@gmail.com>

On Thu, Apr 14, 2011 at 21:03, Jason Vas Dias <jason.vas.dias@gmail.com> wrote:
> On Thursday 14 April 2011 18:13:09 Kay Sievers wrote:
>> On Thu, Apr 14, 2011 at 18:53, Jason Vas Dias <jason.vas.dias@gmail.com> wrote:
>> >   I upgraded to the gentoo version of udev-167 today and found that my wireless interface ( a b43 )
>> >   was disabled,  that I only now ( a few hours later ) now found was because /lib64/udev/firmware
>> >   seems to be thoroughly broken (and somehow the gentoo's firmware loading rules usually in
>> >   /etc/udev/rules.d/50_firmware had disappeared - maybe because they couldn't get this program
>> >   to work either ? )
>> >
>> >   Having cobbled together a new /etc/udev/rules.d/50_firmware :
>> >
>> >   SUBSYSTEM="firmware", ACTION="add", RUN+="/lib64/udev/firmware"
>> >
>> >   "firmware" simply emits some error message the first line of which contains "libusb: main.c:"
>> >   for valid arguments and does nothing .  I double, triple-checked that the correct "FIRMWARE_PATH"
>> >   is generated during config .
>> >
>> >   But the whole idea of using some complicated libusb internal function,  or even any new executable program,
>> >   is incomprehensible to me - to implement these few lines of shell script , which I ended up having to write,
>> >   because udev's complicated libusb method failed - so I'm posting here in the hope that udev will see sense
>> >   and give us a working firmware script instead of some broken libusb using executable, and to help anyone
>> >   else caught by the same issue:
>> >
>> >   to fix, as root:
>> >   #  mv /lib64/udev/firmware /lib64/udev/firmware.broken
>> >   #  cat <<'EOF'
>> > #!/bin/bash
>> > logger "FIRMWARE: $1 $2"
>> > if  ! echo "$1" | egrep -q '^--firmware' ; then
>> >   logger "FIRMWARE: bad firmware arg: $1";
>> >   exit 1;
>> > elif ! echo "$2" | egrep -q '^--devpath' ; then
>> >   logger "FIRMWARE: bad  devpath arg: $2";
>> >   exit 1;
>> > fi
>> > firmware=$1
>> > firmware=${firmware#--firmware=}
>> > devpath=$2
>> > devpath=${devpath#--devpath=}
>> > if [ ! -f /sys/${devpath}/loading ] ; then
>> >   logger "FIRMWARE: no loading: /sys/${devpath}/loading";
>> >   exit 1;
>> > fi
>> > if [ ! -f /sys/${devpath}/data ] ; then
>> >   logger "FIRMWARE: no data: /sys/${devpath}/data";
>> >   exit 1;
>> > fi
>> > echo 1 > /sys/${devpath}/loading
>> > cp -fp /lib64/firmware/${firmware} /sys/${devpath}/data;
>> > status=$?;
>> > echo 0 > /sys/${devpath}/loading
>> > logger "FIRMWARE: copied /lib64/firmware/$firmware to /sys/${devpath}/loading OK: $status"
>> > exit $status;
>> > EOF
>> >     >> /lib64/udev/firmware; chmod a+rx /lib64/udev/firmware
>>
>> There is something seriously broken on your system:
>>
>> First, there is no /lib64/udev/ directory for udev ever. Gentoo should
>> finally stop doing that nonsense. /lib64 is for shared libs only, the
>> application private directory is /lib/udev/ regardless of the
>> architecture.
>>
>> Also all udev default rules like the firmware loader live in
>> /lib/udev/rules.d/, not in /etc/udev/rules.d/. /etc is only for
>> admins, not for packages.
>>
>> Udev's firmware binary does never link against libusb:
>>   $ # ldd /lib/udev/firmware
>>       linux-vdso.so.1 =>  (0x00007fff011ff000)
>>       librt.so.1 => /lib64/librt.so.1 (0x00007fd2e37ea000)
>>       libc.so.6 => /lib64/libc.so.6 (0x00007fd2e347d000)
>>       libdl.so.2 => /lib64/libdl.so.2 (0x00007fd2e3279000)
>>       /lib64/ld-linux-x86-64.so.2 (0x00007fd2e3c12000)
>>       libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fd2e305c000)
>>
>> The udev shipped rules file looks like this:
>>   $ cat /lib/udev/rules.d/50-firmware.rules
>>   # do not edit this file, it will be overwritten on update
>>
>>   # firmware-class requests, copies files into the kernel
>>   SUBSYSTEM="firmware", ACTION="add", \
>>     RUN+="firmware --firmware=$env{FIRMWARE} --devpath=$env{DEVPATH}"
>>
>> Seems there is a lot to do to clean up the mess you have on your box.
>> That's nothing we can fix here. :)
>>
>> > P.S. why doesn't udev use libusb-1.0 ? Rebuilding against libusb-0.1-compat didn't help though.
>>
>> That should work fine, and does work here.

> Hi - sorry, but I disagree with several points made above :
>
>> there is no /lib64/udev/ directory for udev ever
> then why does your configure script support the '--with-rootlibdir=/lib64' setting ?

_My_ script supports everything, but _your_ options are broken. Nobody
prevents --libdir=/tmp if you like to do that too. :)

> If you intend not to support a different rootlibdir,  then you should enforce that by refusing to install on one -
> my system has being running fine for years with no /usr/lib or /lib directories -
> I use both elf_x86_64 binaries, which link to /usr/lib64, and elf_i386 binaries,
> which link to /lib64 - this is simply done by editing /etc/ld.so.conf and /usr/lib{64,32}/lib{c,pthread}.so
> linker scripts (and by changing the default dynamic-linker for gcc to /lib%{-m32:32}%{!-m32:64}/ld-linux.so.2
> in gcc's linux.h / linux64.h header files before building (now on gcc-4.6.0, which passes all tests in test suite).
>
> Either say you do not support any other linux platform than Redhat / Fedora derived ones , or support them .

Nonsense. "Application directories" are not arch specific, and do not
belong into that dir. It's nothing about Red Hat or anybody else, it's
just some total nonsense Gentoo is doing. Maybe you should have a
/usr64/ and /etc64 too. :)

>> Udev's firmware binary does never link against libusb:
>
> but it refuses to build without libusb if '--disable-extras' is not given to configure, and without enabling extras,  you
> don't get any firmware loading functionality at all -
> I infer that in your opinion,   firmware loading functionality is an optional extra.
> Ever tried to do a network boot over a b43 wireless card without firmware loading enabled ?
> That might show you how "optional" it is.
>
>> /lib/udev/rules.d/, not in /etc/udev/rules.d/. /etc is only for
>
> If you do not support your configure script's  "libexecdir" setting, then why does it provide one ?

It does. Udev supports and needs both. Please read the man page why.

> I use this configure command line:
>
>  $ ./configure --prefix=/usr --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --mandir=/usr/share/man --infodir=/usr/share/info --datadir=/usr/share \
>     --sysconfdir=/etc --localstatedir=/var/lib --prefix=/usr --sysconfdir=/etc --sbindir=/sbin --libdir=/usr/lib64 --with-rootlibdir=/lib64 --libexecdir=/lib64/udev \
>     --enable-logging --enable-static --without-selinux --disable-introspection --with-firmware-path=\"/lib64/firmware\"
>
> which does NOT complete unless libusb is installed, and it MUST be libusb-0.1, not libusb-1.0 - why ?

Because it's a different API and I don't care to convert rather exotic
tools I personally don't use. Nothing in core udev uses it.

> Maybe the libusb message I was seeing was an artefact, but really, why are you using an executable to do a shell script's job ?

Because we need to encode filenames in that binary.

> Do you expect people to modify udevd to load /lib/udev/firmware with GDB to diagnose firmware loading problems ?

No, I expect them to configure and install udev correctly and it will
work out-of-the-box, like for everybody else for years. :)

> I only installed the gentoo portage version of udev because the problem arose with the vanilla build - and
> there were still a few old gentoo udev scripts lying around, such as /etc/udev/rules.d/50_firmware that got removed .

Such file was never part of udev. Not sure what you install on your system.

> RE:
>>   # firmware-class requests, copies files into the kernel
>>   SUBSYSTEM="firmware", ACTION="add", \
>>     RUN+="firmware --firmware=$env{FIRMWARE} --devpath=$env{DEVPATH}"
>
> Thanks for the correct 50_firmware file settings, but they make no difference  - if udevd runs /lib64/udev/firmware executable, firmware fails to load and the system hangs
> for @ 5 minutes .
>
> I guess we just must be from different planets of programming - I cannot understand why this /lib64/udev/firmware executable is required to do the job of a shell script
> of a few lines.

Good luck,
Kay

^ permalink raw reply

* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names
From: Greg KH @ 2011-04-14 20:07 UTC (permalink / raw)
  To: Nao Nishijima
  Cc: linux-kernel, linux-scsi, linux-hotplug, Kay Sievers,
	James Bottomley, Jon Masters, 2nddept-manager
In-Reply-To: <4DA6AD28.5020604@hitachi.com>

On Thu, Apr 14, 2011 at 05:15:36PM +0900, Nao Nishijima wrote:
> Hi,
> 
> (2011/04/09 1:12), Greg KH wrote:
> > On Fri, Apr 08, 2011 at 11:07:41PM +0900, Nao Nishijima wrote:
> >> Hi,
> >>
> >> (2011/04/06 1:14), Greg KH wrote:
> >>> On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote:
> >>>> This patch series provides a SCSI option for persistent device
> >>>> names in kernel. With this option, user can always assign a
> >>>> same device name (e.g. sda) to a specific device according to
> >>>> udev rules and device id.
> >>>>
> >>>> Issue:
> >>>> Recently, kernel registers block devices in parallel. As a result,
> >>>> different device names will be assigned at each boot time. This
> >>>> will confuse file-system mounter, thus we usually use persistent
> >>>> symbolic links provided by udev. However, dmesg and procfs outputs
> >>>> show device names instead of the symbolic link names. This causes
> >>>> a serious problem when managing multiple devices (e.g. on a
> >>>> large-scale storage), because usually, device errors are output
> >>>> with device names on dmesg. We also concern about some commands
> >>>> which output device names, as well as kernel messages.
> >>>>
> >>>> Solution:
> >>>> To assign a persistent device name, I create unnamed devices (not
> >>>> a block device, but an intermediate device. users cannot read/write
> >>>> this device). When scsi device driver finds a LU, the LU is registered
> >>>> as an unnamed device and inform to udev. After udev finds the unnamed
> >>>> device, udev assigns a persistent device name to a specific device
> >>>> according to udev rules and registers it as a block device. Hence,
> >>>> this is just a naming, not a renaming.
> >>>>
> >>>> Some users are satisfied with current udev solution. Therefore, my
> >>>> proposal is implemented as an option.
> >>>>
> >>>> If using this option, add the following kernel parameter.
> >>>>
> >>>> 	scsi_mod.persistent_name=1
> >>>>
> >>>> Also, if you want to assign a device name with sda, add the
> >>>> following udev rules.
> >>>>
> >>>> SUBSYSTEM="scsi_unnamed", ATTR{disk_id}="xxx", PROGRAM="/bin/sh
> >>>> -c 'echo -n sda > /sys/%p/disk_name'"
> >>>
> >>> Also, where is the "real" program you have created to properly name
> >>> devices from userspace?  You need that to properly test this patch,
> >>> right?
> >>>
> >>
> >> In the udev rule described above, notation “xxx” indicated by
> >> ATTR(disk_id) is scsi id given by disk. Then, when udev finds this rule,
> >> "/bin/sh -c 'echo -n sda>  /sys/%p/disk_name'" indicated by PROGRAM is
> >> operated using xxx (scsi id) if udev find the disk with scic id xxx.
> >> Thus, persistent device name is assigned.
> >>
> >> I have tested this patch using the udev rule. and It works well.
> > 
> > That's nice, but it's a "toy".  What have you used to test this with
> > 20000 scsi devices to properly work like it does today?  Where is the
> > program that will name them in a deterministic manner?
> > 
> > We have that functionality today, you can't break it.
> > 
> > thanks,
> 
> Indeed, I have not (can not, of course) tested 20000 scsi devices to
> properly work.

Why not?  You should be able to do this easily with a laptop these days
with the dummy scsi device code.

> This feature targets only scsi disk/cdrom/tape devices.
> Not all devices. We expect the target users are system administrators
> who need to control all devices for maintenance. They know all devices
> connected to the server and those devices will tested before connecting.

But that's not the large majority of the people using Linux.  You need a
sane default, which this does not provide.

> We expect that admin will enable this option after installation. This
> means that device names are assigned by current mechanism when
> installation. Admin enables the option and makes udev rules (we are
> planning to make shell script which generate udev rules use by-id and
> assigned device names). In other word, this feature will be used just
> for "fixing" the names.

So admins are going to write their own udev rules by hand?  I find that
very unlikely.

> In our scenario, admin appends a new rule manually when a new LU is
> added. In the future, we are planning to enhance udev to append a new
> rule automatically as like as NIC.

I recommend doing the other options that have been proposed please.

thanks,

greg k-h

^ permalink raw reply

* Re: why does udev-167 break /lib64/udev/firmware ?
From: Jason Vas Dias @ 2011-04-14 19:29 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <201104141753.22335.jason.vas.dias@gmail.com>

corrections - a couple of typos in previous mails:
> I use both elf_x86_64 binaries, which link to /usr/lib64, and elf_i386 binaries,
> which link to /lib64
                           ^- should be '32'

Final scriptlet:

#!/bin/bash
logger "FIRMWARE: $1 $2"
if  ! echo "$1" | egrep -q '^--firmware' ; then 
   logger "FIRMWARE: bad firmware arg: $1";
   exit 1;
elif ! echo "$2" | egrep -q '^--devpath' ; then 
   logger "FIRMWARE: bad  devpath arg: $2";
   exit 1;
fi
firmware=$1
firmware=${firmware#--firmware=}
devpath=$2
devpath=${devpath#--devpath=}
if [ ! -f /sys/${devpath}/loading ] ; then
   logger "FIRMWARE: no loading: /sys/${devpath}/loading";
   exit 1;
fi
if [ ! -f /sys/${devpath}/data ] ; then
   logger "FIRMWARE: no data: /sys/${devpath}/data";
   exit 1;
fi
echo 1 > /sys/${devpath}/loading
cat /lib64/firmware/${firmware} > /sys/${devpath}/data;
# ^-- not 'cp -fp', though this works too
status=$?;
echo 0 > /sys/${devpath}/loading
logger "FIRMWARE: copied /lib64/firmware/$firmware to /sys/${devpath}/loading OK: $status"
exit $status;
#

That libusb message arose when this script was just :
/lib64/udev/firmware.bin "$1" "$2" >/tmp/FW 2>&1;
status=$?;
logger "FIRMWARE STATUS: $status  -  $(cat /tmp/FW) - LOADING : $(ls -l /sys/devices/pci0000:00/0000:00:06.0/0000:30:00.0/ssb0:0/firmware/ssb0:0/loading)";
rm -f /tmp/FW || : ; 

The first time udev calls this with the correct udev arguments, the first line of a message containing the text "libusb main.c:"  is emitted. 
Then it is called once every 10 seconds or so with NO arguments for around 5 minutes because the binary executable fails .

That's OK, I've submitted a patch to remove the firmware binary and replace with the above script on all udev builds on my system -
I just thought I should let you know and potentially help others with the same issue .

Thanks & Regards,
Jason




^ permalink raw reply

* Re: why does udev-167 break /lib64/udev/firmware ?
From: Jason Vas Dias @ 2011-04-14 19:03 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <201104141753.22335.jason.vas.dias@gmail.com>

On Thursday 14 April 2011 18:13:09 Kay Sievers wrote:
> On Thu, Apr 14, 2011 at 18:53, Jason Vas Dias <jason.vas.dias@gmail.com> wrote:
> >   I upgraded to the gentoo version of udev-167 today and found that my wireless interface ( a b43 )
> >   was disabled,  that I only now ( a few hours later ) now found was because /lib64/udev/firmware
> >   seems to be thoroughly broken (and somehow the gentoo's firmware loading rules usually in
> >   /etc/udev/rules.d/50_firmware had disappeared - maybe because they couldn't get this program
> >   to work either ? )
> >
> >   Having cobbled together a new /etc/udev/rules.d/50_firmware :
> >
> >   SUBSYSTEM="firmware", ACTION="add", RUN+="/lib64/udev/firmware"
> >
> >   "firmware" simply emits some error message the first line of which contains "libusb: main.c:"
> >   for valid arguments and does nothing .  I double, triple-checked that the correct "FIRMWARE_PATH"
> >   is generated during config .
> >
> >   But the whole idea of using some complicated libusb internal function,  or even any new executable program,
> >   is incomprehensible to me - to implement these few lines of shell script , which I ended up having to write,
> >   because udev's complicated libusb method failed - so I'm posting here in the hope that udev will see sense
> >   and give us a working firmware script instead of some broken libusb using executable, and to help anyone
> >   else caught by the same issue:
> >
> >   to fix, as root:
> >   #  mv /lib64/udev/firmware /lib64/udev/firmware.broken
> >   #  cat <<'EOF'
> > #!/bin/bash
> > logger "FIRMWARE: $1 $2"
> > if  ! echo "$1" | egrep -q '^--firmware' ; then
> >   logger "FIRMWARE: bad firmware arg: $1";
> >   exit 1;
> > elif ! echo "$2" | egrep -q '^--devpath' ; then
> >   logger "FIRMWARE: bad  devpath arg: $2";
> >   exit 1;
> > fi
> > firmware=$1
> > firmware=${firmware#--firmware=}
> > devpath=$2
> > devpath=${devpath#--devpath=}
> > if [ ! -f /sys/${devpath}/loading ] ; then
> >   logger "FIRMWARE: no loading: /sys/${devpath}/loading";
> >   exit 1;
> > fi
> > if [ ! -f /sys/${devpath}/data ] ; then
> >   logger "FIRMWARE: no data: /sys/${devpath}/data";
> >   exit 1;
> > fi
> > echo 1 > /sys/${devpath}/loading
> > cp -fp /lib64/firmware/${firmware} /sys/${devpath}/data;
> > status=$?;
> > echo 0 > /sys/${devpath}/loading
> > logger "FIRMWARE: copied /lib64/firmware/$firmware to /sys/${devpath}/loading OK: $status"
> > exit $status;
> > EOF
> >     >> /lib64/udev/firmware; chmod a+rx /lib64/udev/firmware
> 
> There is something seriously broken on your system:
> 
> First, there is no /lib64/udev/ directory for udev ever. Gentoo should
> finally stop doing that nonsense. /lib64 is for shared libs only, the
> application private directory is /lib/udev/ regardless of the
> architecture.
> 
> Also all udev default rules like the firmware loader live in
> /lib/udev/rules.d/, not in /etc/udev/rules.d/. /etc is only for
> admins, not for packages.
> 
> Udev's firmware binary does never link against libusb:
>   $ # ldd /lib/udev/firmware
> 	linux-vdso.so.1 =>  (0x00007fff011ff000)
> 	librt.so.1 => /lib64/librt.so.1 (0x00007fd2e37ea000)
> 	libc.so.6 => /lib64/libc.so.6 (0x00007fd2e347d000)
> 	libdl.so.2 => /lib64/libdl.so.2 (0x00007fd2e3279000)
> 	/lib64/ld-linux-x86-64.so.2 (0x00007fd2e3c12000)
> 	libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fd2e305c000)
> 
> The udev shipped rules file looks like this:
>   $ cat /lib/udev/rules.d/50-firmware.rules
>   # do not edit this file, it will be overwritten on update
> 
>   # firmware-class requests, copies files into the kernel
>   SUBSYSTEM="firmware", ACTION="add", \
>     RUN+="firmware --firmware=$env{FIRMWARE} --devpath=$env{DEVPATH}"
> 
> Seems there is a lot to do to clean up the mess you have on your box.
> That's nothing we can fix here. :)
> 
> > P.S. why doesn't udev use libusb-1.0 ? Rebuilding against libusb-0.1-compat didn't help though.
> 
> That should work fine, and does work here.
> 
> Kay
> 
Hi - sorry, but I disagree with several points made above :

> there is no /lib64/udev/ directory for udev ever
then why does your configure script support the '--with-rootlibdir=/lib64' setting ?

If you intend not to support a different rootlibdir,  then you should enforce that by refusing to install on one -
my system has being running fine for years with no /usr/lib or /lib directories -
I use both elf_x86_64 binaries, which link to /usr/lib64, and elf_i386 binaries,
which link to /lib64 - this is simply done by editing /etc/ld.so.conf and /usr/lib{64,32}/lib{c,pthread}.so 
linker scripts (and by changing the default dynamic-linker for gcc to /lib%{-m32:32}%{!-m32:64}/ld-linux.so.2
in gcc's linux.h / linux64.h header files before building (now on gcc-4.6.0, which passes all tests in test suite).

Either say you do not support any other linux platform than Redhat / Fedora derived ones , or support them .

> Udev's firmware binary does never link against libusb:

but it refuses to build without libusb if '--disable-extras' is not given to configure, and without enabling extras,  you
don't get any firmware loading functionality at all -
I infer that in your opinion,   firmware loading functionality is an optional extra.   
Ever tried to do a network boot over a b43 wireless card without firmware loading enabled ? 
That might show you how "optional" it is.

> /lib/udev/rules.d/, not in /etc/udev/rules.d/. /etc is only for

If you do not support your configure script's  "libexecdir" setting, then why does it provide one ?

I use this configure command line:

  $ ./configure --prefix=/usr --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --mandir=/usr/share/man --infodir=/usr/share/info --datadir=/usr/share \
     --sysconfdir=/etc --localstatedir=/var/lib --prefix=/usr --sysconfdir=/etc --sbindir=/sbin --libdir=/usr/lib64 --with-rootlibdir=/lib64 --libexecdir=/lib64/udev \
     --enable-logging --enable-static --without-selinux --disable-introspection --with-firmware-path=\"/lib64/firmware\"

which does NOT complete unless libusb is installed, and it MUST be libusb-0.1, not libusb-1.0 - why ?

Maybe the libusb message I was seeing was an artefact, but really, why are you using an executable to do a shell script's job ?

Do you expect people to modify udevd to load /lib/udev/firmware with GDB to diagnose firmware loading problems ?

I only installed the gentoo portage version of udev because the problem arose with the vanilla build - and
there were still a few old gentoo udev scripts lying around, such as /etc/udev/rules.d/50_firmware that got removed .

RE:
>   # firmware-class requests, copies files into the kernel
>   SUBSYSTEM="firmware", ACTION="add", \
>     RUN+="firmware --firmware=$env{FIRMWARE} --devpath=$env{DEVPATH}"

Thanks for the correct 50_firmware file settings, but they make no difference  - if udevd runs /lib64/udev/firmware executable, firmware fails to load and the system hangs 
for @ 5 minutes .

I guess we just must be from different planets of programming - I cannot understand why this /lib64/udev/firmware executable is required to do the job of a shell script
of a few lines.

Thanks & all the best,
Jason

^ permalink raw reply

* Re: why does udev-167 break /lib64/udev/firmware ?
From: Kay Sievers @ 2011-04-14 17:13 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <201104141753.22335.jason.vas.dias@gmail.com>

On Thu, Apr 14, 2011 at 18:53, Jason Vas Dias <jason.vas.dias@gmail.com> wrote:
>   I upgraded to the gentoo version of udev-167 today and found that my wireless interface ( a b43 )
>   was disabled,  that I only now ( a few hours later ) now found was because /lib64/udev/firmware
>   seems to be thoroughly broken (and somehow the gentoo's firmware loading rules usually in
>   /etc/udev/rules.d/50_firmware had disappeared - maybe because they couldn't get this program
>   to work either ? )
>
>   Having cobbled together a new /etc/udev/rules.d/50_firmware :
>
>   SUBSYSTEM="firmware", ACTION="add", RUN+="/lib64/udev/firmware"
>
>   "firmware" simply emits some error message the first line of which contains "libusb: main.c:"
>   for valid arguments and does nothing .  I double, triple-checked that the correct "FIRMWARE_PATH"
>   is generated during config .
>
>   But the whole idea of using some complicated libusb internal function,  or even any new executable program,
>   is incomprehensible to me - to implement these few lines of shell script , which I ended up having to write,
>   because udev's complicated libusb method failed - so I'm posting here in the hope that udev will see sense
>   and give us a working firmware script instead of some broken libusb using executable, and to help anyone
>   else caught by the same issue:
>
>   to fix, as root:
>   #  mv /lib64/udev/firmware /lib64/udev/firmware.broken
>   #  cat <<'EOF'
> #!/bin/bash
> logger "FIRMWARE: $1 $2"
> if  ! echo "$1" | egrep -q '^--firmware' ; then
>   logger "FIRMWARE: bad firmware arg: $1";
>   exit 1;
> elif ! echo "$2" | egrep -q '^--devpath' ; then
>   logger "FIRMWARE: bad  devpath arg: $2";
>   exit 1;
> fi
> firmware=$1
> firmware=${firmware#--firmware=}
> devpath=$2
> devpath=${devpath#--devpath=}
> if [ ! -f /sys/${devpath}/loading ] ; then
>   logger "FIRMWARE: no loading: /sys/${devpath}/loading";
>   exit 1;
> fi
> if [ ! -f /sys/${devpath}/data ] ; then
>   logger "FIRMWARE: no data: /sys/${devpath}/data";
>   exit 1;
> fi
> echo 1 > /sys/${devpath}/loading
> cp -fp /lib64/firmware/${firmware} /sys/${devpath}/data;
> status=$?;
> echo 0 > /sys/${devpath}/loading
> logger "FIRMWARE: copied /lib64/firmware/$firmware to /sys/${devpath}/loading OK: $status"
> exit $status;
> EOF
>     >> /lib64/udev/firmware; chmod a+rx /lib64/udev/firmware

There is something seriously broken on your system:

First, there is no /lib64/udev/ directory for udev ever. Gentoo should
finally stop doing that nonsense. /lib64 is for shared libs only, the
application private directory is /lib/udev/ regardless of the
architecture.

Also all udev default rules like the firmware loader live in
/lib/udev/rules.d/, not in /etc/udev/rules.d/. /etc is only for
admins, not for packages.

Udev's firmware binary does never link against libusb:
  $ # ldd /lib/udev/firmware
	linux-vdso.so.1 =>  (0x00007fff011ff000)
	librt.so.1 => /lib64/librt.so.1 (0x00007fd2e37ea000)
	libc.so.6 => /lib64/libc.so.6 (0x00007fd2e347d000)
	libdl.so.2 => /lib64/libdl.so.2 (0x00007fd2e3279000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fd2e3c12000)
	libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fd2e305c000)

The udev shipped rules file looks like this:
  $ cat /lib/udev/rules.d/50-firmware.rules
  # do not edit this file, it will be overwritten on update

  # firmware-class requests, copies files into the kernel
  SUBSYSTEM="firmware", ACTION="add", \
    RUN+="firmware --firmware=$env{FIRMWARE} --devpath=$env{DEVPATH}"

Seems there is a lot to do to clean up the mess you have on your box.
That's nothing we can fix here. :)

> P.S. why doesn't udev use libusb-1.0 ? Rebuilding against libusb-0.1-compat didn't help though.

That should work fine, and does work here.

Kay

^ permalink raw reply

* why does udev-167 break /lib64/udev/firmware ?
From: Jason Vas Dias @ 2011-04-14 16:53 UTC (permalink / raw)
  To: linux-hotplug

Hi - 
 
   I upgraded to the gentoo version of udev-167 today and found that my wireless interface ( a b43 )
   was disabled,  that I only now ( a few hours later ) now found was because /lib64/udev/firmware
   seems to be thoroughly broken (and somehow the gentoo's firmware loading rules usually in
   /etc/udev/rules.d/50_firmware had disappeared - maybe because they couldn't get this program
   to work either ? ) 

   Having cobbled together a new /etc/udev/rules.d/50_firmware :
  
   SUBSYSTEM="firmware", ACTION="add", RUN+="/lib64/udev/firmware"

   "firmware" simply emits some error message the first line of which contains "libusb: main.c:" 
   for valid arguments and does nothing .  I double, triple-checked that the correct "FIRMWARE_PATH"
   is generated during config . 
   
   But the whole idea of using some complicated libusb internal function,  or even any new executable program,
   is incomprehensible to me - to implement these few lines of shell script , which I ended up having to write,
   because udev's complicated libusb method failed - so I'm posting here in the hope that udev will see sense
   and give us a working firmware script instead of some broken libusb using executable, and to help anyone
   else caught by the same issue:

   to fix, as root:
   #  mv /lib64/udev/firmware /lib64/udev/firmware.broken
   #  cat <<'EOF' 
#!/bin/bash
logger "FIRMWARE: $1 $2"
if  ! echo "$1" | egrep -q '^--firmware' ; then 
   logger "FIRMWARE: bad firmware arg: $1";
   exit 1;
elif ! echo "$2" | egrep -q '^--devpath' ; then 
   logger "FIRMWARE: bad  devpath arg: $2";
   exit 1;
fi
firmware=$1
firmware=${firmware#--firmware=}
devpath=$2
devpath=${devpath#--devpath=}
if [ ! -f /sys/${devpath}/loading ] ; then
   logger "FIRMWARE: no loading: /sys/${devpath}/loading";
   exit 1;
fi
if [ ! -f /sys/${devpath}/data ] ; then
   logger "FIRMWARE: no data: /sys/${devpath}/data";
   exit 1;
fi
echo 1 > /sys/${devpath}/loading
cp -fp /lib64/firmware/${firmware} /sys/${devpath}/data;
status=$?;
echo 0 > /sys/${devpath}/loading
logger "FIRMWARE: copied /lib64/firmware/$firmware to /sys/${devpath}/loading OK: $status"
exit $status;
EOF
     >> /lib64/udev/firmware; chmod a+rx /lib64/udev/firmware


P.S. why doesn't udev use libusb-1.0 ? Rebuilding against libusb-0.1-compat didn't help though.

Sorry, new to this list - please reply to : jason.vas.dias@gmail.com

All the best,
Jason.

^ permalink raw reply

* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names
From: Nao Nishijima @ 2011-04-14  8:15 UTC (permalink / raw)
  To: Greg KH
  Cc: linux-kernel, linux-scsi, linux-hotplug, Kay Sievers,
	James Bottomley, Jon Masters, 2nddept-manager
In-Reply-To: <20110408161212.GA12111@kroah.com>

Hi,

(2011/04/09 1:12), Greg KH wrote:
> On Fri, Apr 08, 2011 at 11:07:41PM +0900, Nao Nishijima wrote:
>> Hi,
>>
>> (2011/04/06 1:14), Greg KH wrote:
>>> On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote:
>>>> This patch series provides a SCSI option for persistent device
>>>> names in kernel. With this option, user can always assign a
>>>> same device name (e.g. sda) to a specific device according to
>>>> udev rules and device id.
>>>>
>>>> Issue:
>>>> Recently, kernel registers block devices in parallel. As a result,
>>>> different device names will be assigned at each boot time. This
>>>> will confuse file-system mounter, thus we usually use persistent
>>>> symbolic links provided by udev. However, dmesg and procfs outputs
>>>> show device names instead of the symbolic link names. This causes
>>>> a serious problem when managing multiple devices (e.g. on a
>>>> large-scale storage), because usually, device errors are output
>>>> with device names on dmesg. We also concern about some commands
>>>> which output device names, as well as kernel messages.
>>>>
>>>> Solution:
>>>> To assign a persistent device name, I create unnamed devices (not
>>>> a block device, but an intermediate device. users cannot read/write
>>>> this device). When scsi device driver finds a LU, the LU is registered
>>>> as an unnamed device and inform to udev. After udev finds the unnamed
>>>> device, udev assigns a persistent device name to a specific device
>>>> according to udev rules and registers it as a block device. Hence,
>>>> this is just a naming, not a renaming.
>>>>
>>>> Some users are satisfied with current udev solution. Therefore, my
>>>> proposal is implemented as an option.
>>>>
>>>> If using this option, add the following kernel parameter.
>>>>
>>>> 	scsi_mod.persistent_name=1
>>>>
>>>> Also, if you want to assign a device name with sda, add the
>>>> following udev rules.
>>>>
>>>> SUBSYSTEM="scsi_unnamed", ATTR{disk_id}="xxx", PROGRAM="/bin/sh
>>>> -c 'echo -n sda > /sys/%p/disk_name'"
>>>
>>> Also, where is the "real" program you have created to properly name
>>> devices from userspace?  You need that to properly test this patch,
>>> right?
>>>
>>
>> In the udev rule described above, notation “xxx” indicated by
>> ATTR(disk_id) is scsi id given by disk. Then, when udev finds this rule,
>> "/bin/sh -c 'echo -n sda>  /sys/%p/disk_name'" indicated by PROGRAM is
>> operated using xxx (scsi id) if udev find the disk with scic id xxx.
>> Thus, persistent device name is assigned.
>>
>> I have tested this patch using the udev rule. and It works well.
> 
> That's nice, but it's a "toy".  What have you used to test this with
> 20000 scsi devices to properly work like it does today?  Where is the
> program that will name them in a deterministic manner?
> 
> We have that functionality today, you can't break it.
> 
> thanks,

Indeed, I have not (can not, of course) tested 20000 scsi devices to
properly work. This feature targets only scsi disk/cdrom/tape devices.
Not all devices. We expect the target users are system administrators
who need to control all devices for maintenance. They know all devices
connected to the server and those devices will tested before connecting.

We expect that admin will enable this option after installation. This
means that device names are assigned by current mechanism when
installation. Admin enables the option and makes udev rules (we are
planning to make shell script which generate udev rules use by-id and
assigned device names). In other word, this feature will be used just
for "fixing" the names.

In our scenario, admin appends a new rule manually when a new LU is
added. In the future, we are planning to enhance udev to append a new
rule automatically as like as NIC.

Thanks,


-- 
Nao NISHIJIMA
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., YOKOHAMA Research  Laboratory
Email: nao.nishijima.xt@hitachi.com

^ permalink raw reply

* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names
From: Greg KH @ 2011-04-14  2:18 UTC (permalink / raw)
  To: Nao Nishijima
  Cc: Hannes Reinecke, linux-kernel, linux-scsi, linux-hotplug,
	Kay Sievers, James Bottomley, Jon Masters, 2nddept-manager
In-Reply-To: <4DA656A9.8040609@hitachi.com>

On Thu, Apr 14, 2011 at 11:06:33AM +0900, Nao Nishijima wrote:
> Hi Hannes
> 
> (2011/04/08 23:33), Hannes Reinecke wrote:
> > On 04/08/2011 07:12 AM, Nao Nishijima wrote:
> >> Hi,
> >>
> >> (2011/04/06 1:14), Greg KH wrote:
> >>> On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote:
> >>>> This patch series provides a SCSI option for persistent device
> >>>> names in kernel. With this option, user can always assign a
> >>>> same device name (e.g. sda) to a specific device according to
> >>>> udev rules and device id.
> >>>>
> >>>> Issue:
> >>>> Recently, kernel registers block devices in parallel. As a result,
> >>>> different device names will be assigned at each boot time. This
> >>>> will confuse file-system mounter, thus we usually use persistent
> >>>> symbolic links provided by udev.
> >>>
> >>> Yes, that is what you should use if you care about this.
> >>>
> >>>> However, dmesg and procfs outputs
> >>>> show device names instead of the symbolic link names. This causes
> >>>> a serious problem when managing multiple devices (e.g. on a
> >>>> large-scale storage), because usually, device errors are output
> >>>> with device names on dmesg.
> >>>
> >>> Then fix your tools that read the output of these files to point to the
> >>> proper persistent name instead of trying to get the kernel to solve the
> >>> problem.
> >>>
> >>
> >> Yes, the tools should be revised if users get a device name using them.
> >>
> >> The problem I would like to discuss here is that users can not identify
> >> a disk from kernel messages when they DIRECTLY refer to these messages.
> >> For example, a device name is used instead of a symbolic link names in
> >> bootup messages, I/O devices errors and /proc/partitions …etc.
> >>
> >> In particular, users can not identify an appropriate device from a
> >> device name in syslog since different device name may be assigned to it
> >> at each boot time.
> >>
> >> My idea is able to fix this issue with small changes in scsi subsystem.
> >> Also, it is implemented as an option. Therefore, it does not affect
> >> users who do not select this option.
> >>
> > We have been discussing this problem several times in the past, and
> > indeed on these very mailing list.
> > 
> > The conclusion we arrived at is that the kernel-provided device node
> > name is inherently unstable and impossible to fix within the existing
> > 'sdX' naming scheme.
> > So the choices have been to either move to a totally different naming
> > scheme or keep the naming scheme and provide the required information
> > by other means.
> > We have decided on the latter, and agreed on using udev to provide
> > persistent device names.
> 
> Could you tell me why you chose the latter?
> 
> 
> > We are fully aware that any kernel related messages are subject to
> > chance after reboot, but then most kernel related messages are
> > (PID number, timestamps, login tty etc).
> > And also we are aware that any kernel messages need to be matched
> > against the current system layout to figure out any hardware-related
> > issue.
> > 
> > But then basically all products requiring to filter out information
> > from kernel messages already do so I don't see a problem with that.
> > 
> 
> All users did not always use the products. Users can see directly
> kernel messages (dmesg, /proc/partitions). Therefore I think that
> kernel messages need to provide the required mapping.

No they don't.  Anyone who wants to look at those files "knows" what
they are doing and the kernel name is fine to use.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names
From: Nao Nishijima @ 2011-04-14  2:06 UTC (permalink / raw)
  To: Hannes Reinecke
  Cc: Greg KH, linux-kernel, linux-scsi, linux-hotplug, Kay Sievers,
	James Bottomley, Jon Masters, 2nddept-manager
In-Reply-To: <4D9F1CD1.2020600@suse.de>

Hi Hannes

(2011/04/08 23:33), Hannes Reinecke wrote:
> On 04/08/2011 07:12 AM, Nao Nishijima wrote:
>> Hi,
>>
>> (2011/04/06 1:14), Greg KH wrote:
>>> On Tue, Apr 05, 2011 at 09:49:46PM +0900, Nao Nishijima wrote:
>>>> This patch series provides a SCSI option for persistent device
>>>> names in kernel. With this option, user can always assign a
>>>> same device name (e.g. sda) to a specific device according to
>>>> udev rules and device id.
>>>>
>>>> Issue:
>>>> Recently, kernel registers block devices in parallel. As a result,
>>>> different device names will be assigned at each boot time. This
>>>> will confuse file-system mounter, thus we usually use persistent
>>>> symbolic links provided by udev.
>>>
>>> Yes, that is what you should use if you care about this.
>>>
>>>> However, dmesg and procfs outputs
>>>> show device names instead of the symbolic link names. This causes
>>>> a serious problem when managing multiple devices (e.g. on a
>>>> large-scale storage), because usually, device errors are output
>>>> with device names on dmesg.
>>>
>>> Then fix your tools that read the output of these files to point to the
>>> proper persistent name instead of trying to get the kernel to solve the
>>> problem.
>>>
>>
>> Yes, the tools should be revised if users get a device name using them.
>>
>> The problem I would like to discuss here is that users can not identify
>> a disk from kernel messages when they DIRECTLY refer to these messages.
>> For example, a device name is used instead of a symbolic link names in
>> bootup messages, I/O devices errors and /proc/partitions …etc.
>>
>> In particular, users can not identify an appropriate device from a
>> device name in syslog since different device name may be assigned to it
>> at each boot time.
>>
>> My idea is able to fix this issue with small changes in scsi subsystem.
>> Also, it is implemented as an option. Therefore, it does not affect
>> users who do not select this option.
>>
> We have been discussing this problem several times in the past, and
> indeed on these very mailing list.
> 
> The conclusion we arrived at is that the kernel-provided device node
> name is inherently unstable and impossible to fix within the existing
> 'sdX' naming scheme.
> So the choices have been to either move to a totally different naming
> scheme or keep the naming scheme and provide the required information
> by other means.
> We have decided on the latter, and agreed on using udev to provide
> persistent device names.

Could you tell me why you chose the latter?


> We are fully aware that any kernel related messages are subject to
> chance after reboot, but then most kernel related messages are
> (PID number, timestamps, login tty etc).
> And also we are aware that any kernel messages need to be matched
> against the current system layout to figure out any hardware-related
> issue.
> 
> But then basically all products requiring to filter out information
> from kernel messages already do so I don't see a problem with that.
> 

All users did not always use the products. Users can see directly
kernel messages (dmesg, /proc/partitions). Therefore I think that
kernel messages need to provide the required mapping.


> Just adding an in-kernel identifier to the LUN will only be an
> incomplete solution, as other identifiers will still be volatile.
> 
> So I would prefer by keeping the in-kernel information as small
> as possible to reduce memory consumption and rely on out-of-band
> programs to provide the required mapping.
> 
> Cheers,
> 

Thanks,

-- 
Nao NISHIJIMA
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., YOKOHAMA Research  Laboratory
Email: nao.nishijima.xt@hitachi.com

^ permalink raw reply

* Re: [PATCH] Set PCIE maxpayload for card during hotplug insertion
From: Jesse Barnes @ 2011-04-12 16:08 UTC (permalink / raw)
  To: linux-hotplug
In-Reply-To: <1301005267-31006-1-git-send-email-jordan_hargrave@dell.com>

Kenji-san, does the most recent version look ok?  An added tested-by to
confirm the payload size is set correctly would be a bonus.

Thanks,
Jesse

On Tue, 29 Mar 2011 16:42:25 -0500
<Jordan_Hargrave@Dell.com> wrote:

> Are there any further comments on this diff?
> 
> Would it be better to have a separate pcie_get_maxpayload/pcie_set_maxpayload function? 
> 
> What about cards that fail the 'if (psz > dmax)' test?  Technically the code should walk up to the root complex and set the speed there to the least common denominator speed of all children.  However if that happens, the performance of other cards/PCI devices in the system maybe adversely affected.  Any ideas on how to implement this or if this is desired?
> 
> --jordan hargrave
> Dell Enterprise Linux Engineering
> 
> > -----Original Message-----
> > From: Hargrave, Jordan
> > Sent: Monday, March 28, 2011 2:53 AM
> > Cc: 'linux-hotplug@vger.kernel.org'; 'linux-pci@vger.kernel.org'
> > Subject: RE: [PATCH] Set PCIE maxpayload for card during hotplug
> > insertion
> > 
> > > -----Original Message-----
> > > From: Kenji Kaneshige [mailto:kaneshige.kenji@jp.fujitsu.com]
> > > Sent: Thursday, March 24, 2011 8:07 PM
> > > To: Hargrave, Jordan
> > > Cc: linux-hotplug@vger.kernel.org; linux-pci@vger.kernel.org;
> > > jbarnes@virtuousgeek.org
> > > Subject: Re: [PATCH] Set PCIE maxpayload for card during hotplug
> > > insertion
> > >
> > > (2011/03/25 7:21), Jordan Hargrave wrote:
> > > > Reference: http://marc.info/?l=linux-hotplug&m\x128932324625063&w=2
> > > >
> > > > The following patch sets the MaxPayload setting to match the parent
> > > reading when inserting
> > > > a PCIE card into a hotplug slot.  On our system, the upstream
> > bridge
> > > is set to 256, but when
> > > > inserting a card, the card setting defaults to 128.  As soon as I/O
> > > is performed to the card
> > > > it starts receiving errors since the payload size is too small.
> > > >
> > > > Signed-off-by: Jordan_Hargrave@dell.com
> > > >
> > > > ---
> > > >   drivers/pci/hotplug/pcihp_slot.c |   45
> > > ++++++++++++++++++++++++++++++++++++++
> > > >   1 files changed, 45 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/hotplug/pcihp_slot.c
> > > b/drivers/pci/hotplug/pcihp_slot.c
> > > > index 80b461c..912d55b 100644
> > > > --- a/drivers/pci/hotplug/pcihp_slot.c
> > > > +++ b/drivers/pci/hotplug/pcihp_slot.c
> > > > @@ -158,6 +158,47 @@ static void program_hpp_type2(struct pci_dev
> > > *dev, struct hpp_type2 *hpp)
> > > >   	 */
> > > >   }
> > > >
> > > > +/* Program PCIE MaxPayload setting on device: ensure parent
> > > maxpayload<= device */
> > > > +static int pci_set_payload(struct pci_dev *dev)
> > > > +{
> > > > +	int pos, ppos;
> > > > +	u16 pctl, psz;
> > > > +	u16 dctl, dsz, dcap, dmax;
> > > > +	struct pci_dev *parent;
> > > > +
> > > > +	parent = dev->bus->self;
> > > > +	pos = pci_find_capability(dev, PCI_CAP_ID_EXP);
> > > > +	if (!pos)
> > > > +		return 0;
> > >
> > > You can use pci_pcie_cap(), which just returns dev->pcie_cap, instead
> > > of pci_find_capability().
> > >
> > > > +
> > > > +	/* Read Device MaxPayload capability and setting */
> > > > +	pci_read_config_word(dev, pos + PCI_EXP_DEVCTL,&dctl);
> > > > +	pci_read_config_word(dev, pos + PCI_EXP_DEVCAP,&dcap);
> > > > +	dsz = (dctl&  PCI_EXP_DEVCTL_PAYLOAD)>>  5;
> > > > +	dmax = (dcap&  PCI_EXP_DEVCAP_PAYLOAD);
> > > > +
> > > > +	/* Read Parent MaxPayload setting */
> > > > +	ppos = pci_find_capability(parent, PCI_CAP_ID_EXP);
> > > > +	if (!ppos)
> > > > +		return 0;
> > >
> > > Ditto.
> > >
> > > > +	pci_read_config_word(parent, ppos + PCI_EXP_DEVCTL,&pctl);
> > > > +	psz = (pctl&   PCI_EXP_DEVCTL_PAYLOAD)>>  5;
> > > > +
> > > > +	/* If parent payload>  device max payload ->  error
> > > > +	 * If parent payload>  device payload ->  set speed
> > > > +	 * If parent payload<= device payload ->  do nothing
> > > > +	 */
> > > > +	if (psz>  dmax)
> > > > +		return -1;
> > > > +	else if (psz>  dsz) {
> > >
> > > Maybe just "if" (not "else if") here is easier to read.
> > >
> > > > +		dev_info(&dev->dev, "Setting MaxPayload to %d\n", 128<<
> > > psz);
> > > > +		pci_write_config_word(dev, pos + PCI_EXP_DEVCTL,
> > > > +				      (dctl&  ~PCI_EXP_DEVCTL_PAYLOAD) +
> > > > +				      (psz<<  5));
> > >
> > > What about
> > >
> > > 	dctrl = (dctrl & ~PCI_EXP_DEVCTL_PAYLOAD) | (psz << 5);
> > > 	pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, dctrl);
> > >
> > > ?
> > >
> > > Regards,
> > > Kenji Kaneshige
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Jesse Barnes, Intel Open Source Technology Center

^ permalink raw reply

* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device
From: James Bottomley @ 2011-04-12 13:29 UTC (permalink / raw)
  To: Nao Nishijima
  Cc: Hannes Reinecke, Greg KH, linux-kernel, linux-scsi, linux-hotplug,
	Kay Sievers, Jon Masters, 2nddept-manager
In-Reply-To: <4DA4526B.2030609@hitachi.com>

On Tue, 2011-04-12 at 22:23 +0900, Nao Nishijima wrote:
> (2011/04/09 0:14), James Bottomley wrote:
> > On Fri, 2011-04-08 at 07:33 -0700, Hannes Reinecke wrote:
> >>> The problem I would like to discuss here is that users can not identify
> >>> a disk from kernel messages when they DIRECTLY refer to these messages.
> >>> For example, a device name is used instead of a symbolic link names in
> >>> bootup messages, I/O devices errors and /proc/partitions …etc.
> >>>
> >>> In particular, users can not identify an appropriate device from a
> >>> device name in syslog since different device name may be assigned to it
> >>> at each boot time.
> >>>
> >>> My idea is able to fix this issue with small changes in scsi subsystem.
> >>> Also, it is implemented as an option. Therefore, it does not affect
> >>> users who do not select this option.
> >>>
> >> We have been discussing this problem several times in the past, and
> >> indeed on these very mailing list.
> >>
> >> The conclusion we arrived at is that the kernel-provided device node
> >> name is inherently unstable and impossible to fix within the existing
> >> 'sdX' naming scheme.
> >> So the choices have been to either move to a totally different naming
> >> scheme or keep the naming scheme and provide the required information
> >> by other means.
> >> We have decided on the latter, and agreed on using udev to provide
> >> persistent device names.
> >> We are fully aware that any kernel related messages are subject to
> >> chance after reboot, but then most kernel related messages are
> >> (PID number, timestamps, login tty etc).
> >> And also we are aware that any kernel messages need to be matched
> >> against the current system layout to figure out any hardware-related
> >> issue.
> >>
> >> But then basically all products requiring to filter out information
> >> from kernel messages already do so I don't see a problem with that.
> >>
> >> Just adding an in-kernel identifier to the LUN will only be an
> >> incomplete solution, as other identifiers will still be volatile.
> >>
> >> So I would prefer by keeping the in-kernel information as small
> >> as possible to reduce memory consumption and rely on out-of-band
> >> programs to provide the required mapping.
> > 
> > So, while I agree totally with the above: udev and userspace is the way
> > to go, I'm not totally opposed to having a non-invasive mechanism for
> > indicating a user's preferred name for a device.  I think there are a
> > couple of ways to do this:
> > 
> >      1. Entirely in userspace: just have udev consult a preferred name
> >         file and create say /dev/disk/by-preferred.  Then have all the
> >         tools that normally output device information do the same (i.e.
> >         since real name to preferred name is 1:1, they could all do a
> >         reverse lookup).
> >      2. have a writeable sysfs preferred_name field, either in the
> >         generic device or just in SCSI.  The preferred name would be
> >         used by outbound only (i.e. kernel dev_printk messages and
> >         possibly /proc/partitions).  All inbound uses of the device
> >         would come via the standard udev mechanisms
> >         (i.e. /dev/disk/by-preferred would be the usual symlink).  This
> >         means from the kernel point of view, no renaming has happened.
> >         We'd just try to print out the preferred name in certain
> >         circumstances, which should solve most of the described problem.
> > 
> > James
> > 
> > 
> > 
> 
> I have a question. Why is in-kernel device name necessary? The kernel
> can identify a device by major/miner number and udev can create a
> device node of a prefer name.

Well, that's the inbound vs outbound name I referred to above.  If all
you care about is inbound, this is true.

> Currently, device names are only used to show to users. Therefore I
> think that in-kernel device name is unnecessary if we introduce your
> /dev/disk/by-prefferd idea.

So if you have no problem with the kernel still printing out sdX in logs
and it appearing in /proc/partitions, I'm happy with this.

James



^ permalink raw reply

* Re: [PATCH 1/2] SCSI: Add a SCSI option for persistent device names
From: Nao Nishijima @ 2011-04-12 13:23 UTC (permalink / raw)
  To: James Bottomley
  Cc: Hannes Reinecke, Greg KH, linux-kernel, linux-scsi, linux-hotplug,
	Kay Sievers, Jon Masters, 2nddept-manager
In-Reply-To: <1302275652.4090.10.camel@mulgrave.site>

Hi, James

(2011/04/09 0:14), James Bottomley wrote:
> On Fri, 2011-04-08 at 07:33 -0700, Hannes Reinecke wrote:
>>> The problem I would like to discuss here is that users can not identify
>>> a disk from kernel messages when they DIRECTLY refer to these messages.
>>> For example, a device name is used instead of a symbolic link names in
>>> bootup messages, I/O devices errors and /proc/partitions …etc.
>>>
>>> In particular, users can not identify an appropriate device from a
>>> device name in syslog since different device name may be assigned to it
>>> at each boot time.
>>>
>>> My idea is able to fix this issue with small changes in scsi subsystem.
>>> Also, it is implemented as an option. Therefore, it does not affect
>>> users who do not select this option.
>>>
>> We have been discussing this problem several times in the past, and
>> indeed on these very mailing list.
>>
>> The conclusion we arrived at is that the kernel-provided device node
>> name is inherently unstable and impossible to fix within the existing
>> 'sdX' naming scheme.
>> So the choices have been to either move to a totally different naming
>> scheme or keep the naming scheme and provide the required information
>> by other means.
>> We have decided on the latter, and agreed on using udev to provide
>> persistent device names.
>> We are fully aware that any kernel related messages are subject to
>> chance after reboot, but then most kernel related messages are
>> (PID number, timestamps, login tty etc).
>> And also we are aware that any kernel messages need to be matched
>> against the current system layout to figure out any hardware-related
>> issue.
>>
>> But then basically all products requiring to filter out information
>> from kernel messages already do so I don't see a problem with that.
>>
>> Just adding an in-kernel identifier to the LUN will only be an
>> incomplete solution, as other identifiers will still be volatile.
>>
>> So I would prefer by keeping the in-kernel information as small
>> as possible to reduce memory consumption and rely on out-of-band
>> programs to provide the required mapping.
> 
> So, while I agree totally with the above: udev and userspace is the way
> to go, I'm not totally opposed to having a non-invasive mechanism for
> indicating a user's preferred name for a device.  I think there are a
> couple of ways to do this:
> 
>      1. Entirely in userspace: just have udev consult a preferred name
>         file and create say /dev/disk/by-preferred.  Then have all the
>         tools that normally output device information do the same (i.e.
>         since real name to preferred name is 1:1, they could all do a
>         reverse lookup).
>      2. have a writeable sysfs preferred_name field, either in the
>         generic device or just in SCSI.  The preferred name would be
>         used by outbound only (i.e. kernel dev_printk messages and
>         possibly /proc/partitions).  All inbound uses of the device
>         would come via the standard udev mechanisms
>         (i.e. /dev/disk/by-preferred would be the usual symlink).  This
>         means from the kernel point of view, no renaming has happened.
>         We'd just try to print out the preferred name in certain
>         circumstances, which should solve most of the described problem.
> 
> James
> 
> 
> 

I have a question. Why is in-kernel device name necessary? The kernel
can identify a device by major/miner number and udev can create a
device node of a prefer name.

Currently, device names are only used to show to users. Therefore I
think that in-kernel device name is unnecessary if we introduce your
/dev/disk/by-prefferd idea.


thanks,

-- 
Nao NISHIJIMA
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., YOKOHAMA Research  Laboratory
Email: nao.nishijima.xt@hitachi.com

^ permalink raw reply

* [PULL] Docs: udev.xml: Copyediting
From: Michael Witten @ 2011-04-11  6:31 UTC (permalink / raw)
  To: linux-hotplug

I did some copyediting whilst looking through udev/udev.xml;
this email contains a pull request as well as the inlined
diff of all the changes squashed together.

The changes apply cleanly to 960250952193db522b8ced336bf998bd7e8e4a31:

  selinux: firmware - do not label files in runtime dir (2011-04-08 03:08:16 +0200)

and are available here:

  git://github.com/mfwitten/udev.git docs/udev.xml

Squash them if desired.

      Docs: udev.xml: Offset daemon name with commas
      Docs: udev.xml: Remove commas (and unnecessary repetition)
      Docs: udev.xml: `are' -> `is'; the subject is `Access'
      Docs: udev.xml: Use present tense
      Docs: udev.xml: Clarification through proper wording
      Docs: udev.xml: `,' -> `;'
      Docs: udev.xml: `key value' -> `key-value'
      Docs: udev.xml: `,' -> `:'
      Docs: udev.xml: Use `assignment' consistently
      Docs: udev.xml: `comma-separated' is a better description
      Docs: udev.xml: Remove unnecessary repitition
      Docs: udev.xml: Add a few more words for context
      Docs: udev.xml: Use `unless' for clarity
      Docs: udev.xml: Clarify PROGRAM key
      Docs: udev.xml: `a shell style' -> `shell-style'
      Docs: udev.xml: Clean `*' description
      Docs: udev.xml: Clean character range description
      Docs: udev.xml: Clean up description of NAME assignment key
      Docs: udev.xml: Clean up description of SYMLINK assignment key
      Docs: udev.xml: Clean up description of ENV assignment key
      Docs: udev.xml: Clean up description of RUN assignment key
      Docs: udev.xml: Clean up description of LABEL assignment key
      Docs: udev.xml: Add missing `.'
      Docs: udev.xml: `which' -> `content of which'
      Docs: udev.xml: `commandline' -> `command line'
      Docs: udev.xml: Clean up WAIT_FOR description
      Docs: udev.xml: `a' -> `the'
      Docs: udev.xml: Clean up introduction to substitutions.
      Docs: udev.xml: Use normal sentence structure
      Docs: udev.xml: Actually make a separate paragraph
      Docs: udev.xml: Add comma
      Docs: udev.xml: `char' -> `character'
      Docs: udev.xml: `comma-separated' is a better description
      Docs: udev.xml: Clarify through a change in word ordering
      Docs: udev.xml: Improved word order
      Docs: udev.xml: Fix dangling modifier

 udev/udev.xml |  169 +++++++++++++++++++++++++++++----------------------------
 1 files changed, 86 insertions(+), 83 deletions(-)

diff --git a/udev/udev.xml b/udev/udev.xml
index ae91798..8a4212f 100644
--- a/udev/udev.xml
+++ b/udev/udev.xml
@@ -27,23 +27,23 @@
     names provide a way to reliably identify devices based on their properties or
     current configuration.</para>
 
-    <para>The udev daemon <citerefentry><refentrytitle>udevd</refentrytitle>
+    <para>The udev daemon, <citerefentry><refentrytitle>udevd</refentrytitle>
-    <manvolnum>8</manvolnum></citerefentry> receives device uevents directly from
+    <manvolnum>8</manvolnum></citerefentry>, receives device uevents directly from
     the kernel whenever a device is added or removed from the system, or it changes its
     state. When udev receives a device event, it matches its configured set of rules
-    against various device attributes to identify the device. Rules that match, may
+    against various device attributes to identify the device. Rules that match may
-    provide additional device information to be stored in the udev database, or information
+    provide additional device information to be stored in the udev database or
     to be used to create meaningful symlink names.</para>
 
-    <para>All device information udev processes, is stored in the udev database and
+    <para>All device information udev processes is stored in the udev database and
     sent out to possible event subscribers. Access to all stored data and the event
-    sources are provided by the library libudev.</para>
+    sources is provided by the library libudev.</para>
   </refsect1>
 
   <refsect1><title>Configuration</title>
     <para>udev configuration files are placed in <filename>/etc/udev/</filename>
-    and <filename>/lib/udev/</filename>. All empty lines, or lines beginning with
+    and <filename>/lib/udev/</filename>. All empty lines or lines beginning with
-    '#' will be ignored.</para>
+    '#' are ignored.</para>
 
     <refsect2><title>Configuration file</title>
       <para>udev expects its main configuration file at <filename>/etc/udev/udev.conf</filename>.
@@ -74,26 +74,26 @@
       default rules directory <filename>/lib/udev/rules.d/</filename>,
       the custom rules directory <filename>/etc/udev/rules.d/</filename>
       and the temporary rules directory <filename>/run/udev/rules.d/</filename>.
-      All rule files are sorted and processed in lexical order, regardless
-      in which of these directories they live. Files in
-      <filename>/etc/udev/rules.d/</filename> have precedence over files with
-      the same name in <filename>/lib/udev/rules.d/</filename>. This can be
+      All rule files are collectively sorted and processed in lexical order,
+      regardless of the directories in which they live. However, files in
+      <filename>/etc/udev/rules.d/</filename> take precedence over files with
+      the same name in <filename>/lib/udev/rules.d/</filename>; this can be
       used to ignore a default rules file if needed.</para>
 
-      <para>Rule files must end in <filename>.rules</filename>, other extensions
+      <para>Rule files must have the extension <filename>.rules</filename>; other
-      are ignored.</para>
+      extensions are ignored.</para>
 
-      <para>Every line in the rules file contains at least one key value pair.
+      <para>Every line in the rules file contains at least one key-value pair.
-      There are two kind of keys, match and assignment keys.
+      There are two kind of keys: match and assignment.
       If all match keys are matching against its value, the rule gets applied and the
-      assign keys get the specified value assigned.</para>
+      assignment keys get the specified value assigned.</para>
 
       <para>A matching rule may rename a network interface, add symlinks
       pointing to the device node, or run a specified program as part of
       the event handling.</para>
 
-      <para>A rule consists of a list of one or more key value pairs separated by
+      <para>A rule consists of a comma-separated list of one or more key-value pairs.
-      a comma. Each key has a distinct operation, depending on the used operator. Valid
+      Each key has a distinct operation, depending on the used operator. Valid
       operators are:</para>
       <variablelist>
         <varlistentry>
@@ -113,7 +113,7 @@
         <varlistentry>
           <term><option>=</option></term>
           <listitem>
-            <para>Assign a value to a key. Keys that represent a list, are reset
+            <para>Assign a value to a key. Keys that represent a list are reset
             and only this single value is assigned.</para>
           </listitem>
         </varlistentry>
@@ -128,8 +128,7 @@
         <varlistentry>
           <term><option>:=</option></term>
           <listitem>
-            <para>Assign  a  value  to  a key finally; disallow any later changes,
-            which may be used to prevent changes by any later rules.</para>
+            <para>Assign  a  value  to  a key finally; disallow any later changes.</para>
           </listitem>
         </varlistentry>
       </variablelist>
@@ -189,7 +188,7 @@
         <varlistentry>
           <term><option>DRIVER</option></term>
           <listitem>
-            <para>Match the driver name of the event device. Only set for devices
+            <para>Match the driver name of the event device. Only set this key for devices
             which are bound to a driver at the time the event is generated.</para>
           </listitem>
         </varlistentry>
@@ -197,8 +196,8 @@
           <term><option>ATTR{<replaceable>filename</replaceable>}</option></term>
           <listitem>
             <para>Match sysfs attribute values of the event device. Trailing
-            whitespace in the attribute values is ignored, if the specified match
+            whitespace in the attribute values is ignored unless the specified match
-            value does not contain trailing whitespace itself.
+            value itself contains trailing whitespace.
             </para>
           </listitem>
         </varlistentry>
@@ -229,8 +228,8 @@
           <listitem>
             <para>Search the devpath upwards for a device with matching sysfs attribute values.
             If multiple <option>ATTRS</option> matches are specified, all of them
-            must match on the same device. Trailing whitespace in the attribute values is ignored,
+            must match on the same device. Trailing whitespace in the attribute values is ignored
-            if the specified match value does not contain trailing whitespace itself.</para>
+            unless the specified match value itself contains trailing whitespace.</para>
           </listitem>
         </varlistentry>
 
@@ -259,10 +258,11 @@
         <varlistentry>
           <term><option>PROGRAM</option></term>
           <listitem>
-            <para>Execute a program. The key is true, if the program returns
+            <para>Execute a program to determine whether there
+            is a match; the key is true if the program returns
             successfully. The device properties are made available to the
-            executed program in the environment. The program's output printed to
+            executed program in the environment. The program's stdout
-            stdout, is available in the RESULT key.</para>
+            is available in the RESULT key.</para>
           </listitem>
         </varlistentry>
 
@@ -275,13 +275,13 @@
         </varlistentry>
       </variablelist>
 
-      <para>Most of the fields support a shell style pattern matching. The following
+      <para>Most of the fields support shell-style pattern matching. The following
       pattern characters are supported:</para>
       <variablelist>
         <varlistentry>
           <term><option>*</option></term>
           <listitem>
-            <para>Matches zero, or any number of characters.</para>
+            <para>Matches zero or more characters.</para>
           </listitem>
         </varlistentry>
         <varlistentry>
@@ -295,8 +295,8 @@
           <listitem>
             <para>Matches any single character specified within the brackets. For
             example, the pattern string 'tty[SR]' would match either 'ttyS' or 'ttyR'.
-            Ranges are also supported within this match with the '-' character.
+            Ranges are also supported via the '-' character.
-            For example, to match on the range of all digits, the pattern [0-9] would
+            For example, to match on the range of all digits, the pattern [0-9] could
             be used. If the first character following the '[' is a '!', any characters
             not enclosed are matched.</para>
           </listitem>
@@ -308,33 +308,33 @@
         <varlistentry>
           <term><option>NAME</option></term>
           <listitem>
-            <para>The name, a network interface should be renamed to. Or as
-            a temporary workaround, the name a device node should be named.
-            Usually the kernel provides the defined node name, or even creates
+            <para>What a network interface should be named.</para>
+            <para>Also, as a temporary workaround, this is what a device node
+            should be named; usually the kernel provides the defined node name or creates
             and removes the node before udev even receives any event. Changing
             the node name from the kernel's default creates inconsistencies
             and is not supported. If the kernel and NAME specify different names,
-            an error will be logged. Udev is only expected to handle device node
+            an error is logged. udev is only expected to handle device node
             permissions and to create additional symlinks, not to change
             kernel-provided device node names. Instead of renaming a device node,
-            SYMLINK should be used. Symlink names must never conflict with
+            SYMLINK should be used. However, symlink names must never conflict with
-            device node names, it will result in unpredictable behavior.</para>
+            device node names, as that would result in unpredictable behavior.</para>
           </listitem>
         </varlistentry>
 
         <varlistentry>
           <term><option>SYMLINK</option></term>
           <listitem>
-            <para>The name of a symlink targeting the node. Every matching rule will add
+            <para>The name of a symlink targeting the node. Every matching rule adds
             this value to the list of symlinks to be created. Multiple symlinks may be
             specified by separating the names by the space character. In case multiple
-            devices claim the same name, the link will always point to the device with
-            the highest link_priority. If the current device goes away, the links will
-            be re-evaluated and the device with the next highest link_priority will own
-            the link. If no link_priority is specified, the order of the devices, and
-            which one of them will own the link, is undefined. Claiming the same name for
-            a symlink, which is or might be used for a device node, may result in
-            unexpected behavior and is not supported.
+            devices claim the same name, the link always points to the device with
+            the highest link_priority. If the current device goes away, the links are
+            re-evaluated and the device with the next highest link_priority becomes the owner of
+            the link. If no link_priority is specified, the order of the devices (and
+            which one of them owns the link) is undefined. Also, symlink names must
+            never conflict with the kernel's default device node names, as that would
+            result in unpredictable behavior.
             </para>
           </listitem>
         </varlistentry>
@@ -359,7 +359,8 @@
           <term><option>ENV{<replaceable>key</replaceable>}</option></term>
           <listitem>
             <para>Set a device property value. Property names with a leading '.'
-            are not stored in the database or exported to external tool or events.</para>
+            are neither stored in the database nor exported to events or
+            external tools (run by, say, the PROGRAM match key).</para>
           </listitem>
         </varlistentry>
 
@@ -385,23 +386,25 @@
             this or a dependent device. Long running tasks need to be immediately
             detached from the event process itself.</para>
             <para>If no absolute path is given, the program is expected to live in
-            <filename>/lib/udev</filename>, otherwise the absolute path must be
-            specified. Program name and arguments are separated by spaces. Single quotes
-            can be used to specify arguments with spaces.</para>
+            the directory provided at compile-time to configure via --libexecdir
+            (this is usually <filename>/lib/udev</filename>), otherwise the absolute
+            path must be specified. The program name and following arguments are
+            separated by spaces. Single quotes can be used to specify arguments with
+            spaces.</para>
           </listitem>
         </varlistentry>
 
         <varlistentry>
           <term><option>LABEL</option></term>
           <listitem>
-            <para>Named label where a GOTO can jump to.</para>
+            <para>A named label to which a GOTO may jump.</para>
           </listitem>
         </varlistentry>
 
         <varlistentry>
           <term><option>GOTO</option></term>
           <listitem>
-            <para>Jumps to the next LABEL with a matching name</para>
+            <para>Jumps to the next LABEL with a matching name.</para>
           </listitem>
         </varlistentry>
 
@@ -423,8 +426,8 @@
               <varlistentry>
                 <term><option>file</option></term>
                 <listitem>
-                  <para>Import a text file specified as the assigned value, which must be in
-                  environment key format.</para>
+                  <para>Import a text file specified as the assigned value, the content
+                  of which must be in environment key format.</para>
                 </listitem>
               </varlistentry>
               <varlistentry>
@@ -438,8 +441,8 @@
               <varlistentry>
                 <term><option>cmdline</option></term>
                 <listitem>
-                  <para>Import a single property from the kernel commandline. For simple flags
+                  <para>Import a single property from the kernel command line. For simple flags
-                  the value of the property will be set to '1'.</para>
+                  the value of the property is set to '1'.</para>
                 </listitem>
               </varlistentry>
               <varlistentry>
@@ -453,7 +456,7 @@
                 </listitem>
               </varlistentry>
             </variablelist>
-            <para>If no option is given, udev will choose between <option>program</option>
+            <para>If no option is given, udev chooses between <option>program</option>
             and <option>file</option> based on the executable bit of the file
             permissions.</para>
           </listitem>
@@ -462,9 +465,9 @@
         <varlistentry>
           <term><option>WAIT_FOR</option></term>
           <listitem>
-            <para>Wait for a file to become available or until a 10
+            <para>Wait for a file to become available or until a timeout of
-            seconds timeout expires. The path is relative to the sysfs device,
+            10 seconds expires. The path is relative to the sysfs device;
-            i. e. if no path is specified this waits for an attribute to appear.</para>
+            if no path is specified, this waits for an attribute to appear.</para>
           </listitem>
         </varlistentry>
 
@@ -483,8 +486,8 @@
               <varlistentry>
                 <term><option>event_timeout=</option></term>
                 <listitem>
-                  <para>Number of seconds an event will wait for operations to finish, before it
+                  <para>Number of seconds an event waits for operations to finish before
-                  will terminate itself.</para>
+                  giving up and terminating itself.</para>
                 </listitem>
               </varlistentry>
               <varlistentry>
@@ -498,18 +501,18 @@
               <varlistentry>
                 <term><option>static_node=</option></term>
                 <listitem>
-                  <para>Apply the permissions specified in this rule to a static device node with
+                  <para>Apply the permissions specified in this rule to the static device node with
-                  the specified name. Static device nodes might be provided by kernel modules,
+                  the specified name. Static device nodes might be provided by kernel modules
                   or copied from <filename>/lib/udev/devices</filename>. These nodes might not have
-                  a corresponding kernel device at the time udevd is started, and allow to trigger
+                  a corresponding kernel device at the time udevd is started; they can trigger
-                  automatic kernel module on-demand loading.</para>
+                  automatic kernel module loading.</para>
                 </listitem>
               </varlistentry>
               <varlistentry>
                 <term><option>watch</option></term>
                 <listitem>
-                  <para>Watch the device node with inotify, when closed after being opened for
+                  <para>Watch the device node with inotify; when the node is closed after being opened for
-                  writing, a change uevent will be synthesised.</para>
+                  writing, a change uevent is synthesized.</para>
                 </listitem>
               </varlistentry>
               <varlistentry>
@@ -525,10 +528,10 @@
 
       <para>The <option>NAME</option>, <option>SYMLINK</option>, <option>PROGRAM</option>,
       <option>OWNER</option>, <option>GROUP</option>, <option>MODE</option>  and  <option>RUN</option>
-      fields support simple printf-like string substitutions. The <option>RUN</option>
+      fields support simple string substitutions. The <option>RUN</option>
-      format chars gets applied after all rules have been processed, right before the program
+      substitutions are performed after all rules have been processed, right before the program
-      is executed. It allows the use of device properties set by earlier matching
+      is executed, allowing for the use of device properties set by earlier matching
-      rules. For all other fields, substitutions are applied while the individual rule is
+      rules. For all other fields, substitutions are performed while the individual rule is
       being processed. The available substitutions are:</para>
       <variablelist>
         <varlistentry>
@@ -574,12 +577,12 @@
         <varlistentry>
           <term><option>$attr{<replaceable>file</replaceable>}</option>, <option>%s{<replaceable>file</replaceable>}</option></term>
           <listitem>
-            <para>The value of a sysfs attribute found at the device, where
+            <para>The value of a sysfs attribute found at the device where
             all keys of the rule have matched. If the matching device does not have
             such an attribute, and a previous KERNELS, SUBSYSTEMS, DRIVERS, or
-            ATTRS test selected a parent device, use the attribute from that
+            ATTRS test selected a parent device, then the attribute from that
-            parent device.
+            parent device is used.</para>
-            If the attribute is a symlink, the last element of the symlink target is
+            <para>If the attribute is a symlink, the last element of the symlink target is
             returned as the value.</para>
           </listitem>
         </varlistentry>
@@ -609,9 +612,9 @@
           <term><option>$result</option>, <option>%c</option></term>
           <listitem>
             <para>The string returned by the external program requested with PROGRAM.
-            A single part of the string, separated by a space character may be selected
+            A single part of the string, separated by a space character, may be selected
             by specifying the part number as an attribute: <option>%c{N}</option>.
-            If the number is followed by the '+' char this part plus all remaining parts
+            If the number is followed by the '+' character, this part plus all remaining parts
             of the result string are substituted: <option>%c{N+}</option></para>
           </listitem>
         </varlistentry>
@@ -634,8 +637,8 @@
         <varlistentry>
           <term><option>$links</option></term>
           <listitem>
-            <para>The current list of symlinks, separated by a space character. The value is
+            <para>A space-separated list of the current symlinks. The value is
-            only set if an earlier rule assigned a value, or during a remove events.</para>
+            only set during a remove event or if an earlier rule assigned a value.</para>
           </listitem>
         </varlistentry>
 
@@ -656,7 +659,7 @@
         <varlistentry>
           <term><option>$tempnode</option>, <option>%N</option></term>
           <listitem>
-            <para>The name of a created temporary device node to provide access to
+            <para>The name of a temporary device node created to provide access to
             the device from a external program before the real node is created.</para>
           </listitem>
         </varlistentry>

^ permalink raw reply related

* [PULL] Docs: README: Copyediting
From: Michael Witten @ 2011-04-11  6:22 UTC (permalink / raw)
  To: linux-hotplug

I did some copyediting whilst looking through the README;
this email contains a pull request as well as the inlined
diff of all the changes squashed together.

The changes apply cleanly to 960250952193db522b8ced336bf998bd7e8e4a31:

  selinux: firmware - do not label files in runtime dir (2011-04-08 03:08:16 +0200)

and are available here:

  git://github.com/mfwitten/udev.git docs/README

Squash them if desired.

      Docs: README: `to replace' -> `replacing'
      Docs: README: `,' -> `;'
      Docs: README: Clean up a sentence
      Docs: README: Use present tense
      Docs: README: Add missing `and'
      Docs: README: Remove commas and use subjective mood
      Docs: README: Clean up `udev extras' requirements
      Docs: README: Clarify configuration of existing devices
      Docs: README: `does never apply' -> `never applies'
      Docs: README: Flip sentence structure to improve wording
      Docs: README: `set up' is the verb; `setup' is a noun
      Docs: README: Add a comma to offset the modifier

 README |   44 ++++++++++++++++++++++----------------------
 1 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/README b/README
index 7878746..4520f3d 100644
--- a/README
+++ b/README
@@ -3,17 +3,17 @@ udev - Linux userspace device management
 Integrating udev in the system has complex dependencies and may differ from
 distribution to distribution. A system may not be able to boot up or work
 reliably without a properly installed udev version. The upstream udev project
-does not recommend to replace a distro's udev installation with the upstream
+does not recommend replacing a distro's udev installation with the upstream
 version.
 
 The upstream udev project's set of default rules may require a most recent
 kernel release to work properly. This is currently version 2.6.31.
 
 Tools and rules shipped by udev are not public API and may change at any time.
-Never call any private tool in /lib/udev from any external application, it might
+Never call any private tool in /lib/udev from any external application; it might
 just go away in the next release. Access to udev information is only offered
-by udevadm and libudev. Tools and rules in /lib/udev, and the entire content of
+by udevadm and libudev. Tools and rules in /lib/udev and the entire contents of
-the /dev/.udev directory is private to udev and does change whenever needed.
+the /dev/.udev directory are private to udev and do change whenever needed.
 
 Requirements:
   - Version 2.6.27 of the Linux kernel with sysfs, procfs, signalfd, inotify,
@@ -31,7 +31,7 @@ Requirements:
       CONFIG_TMPFS_POSIX_ACL=y (user ACLs for device nodes)
       CONFIG_BLK_DEV_BSG=y (SCSI devices)
 
-  - Udev will not work with the CONFIG_SYSFS_DEPRECATED* option.
+  - Udev does not work with the CONFIG_SYSFS_DEPRECATED* option.
 
   - Unix domain sockets (CONFIG_UNIX) as a loadable kernel module may work,
     but it is not supported.
@@ -41,46 +41,46 @@ Requirements:
     unusable because the kernel may create too many processes in parallel
     so that the system runs out-of-memory.
 
-  - The proc filesystem must be mounted on /proc, the sysfs filesystem must
+  - The proc filesystem must be mounted on /proc, and the sysfs filesystem must
     be mounted at /sys. No other locations are supported by a standard
     udev installation.
 
   - The system must have the following group names resolvable at udev startup:
-      disk, cdrom, floppy, tape, audio, video, lp, tty, dialout, kmem.
+      disk, cdrom, floppy, tape, audio, video, lp, tty, dialout, and kmem.
-    Especially in LDAP setups, it is required, that getgrnam() is able to resolve
+    Especially in LDAP setups, it is required that getgrnam() be able to resolve
-    these group names with only the rootfs mounted, and while no network is
+    these group names with only the rootfs mounted and while no network is
     available.
 
-  - To build all 'udev extras', libacl, libglib2, libusb, usbutils, pciutils,
-    gperf are needed. These dependencies can be disabled with the
-    --disable-extras configure option.
+  - The 'udev extras' has the following dependencies:
+      libacl, libglib2, libusb, usbutils, pciutils, and gperf.
+    These dependencies can be disabled with the --disable-extras configure option.
 
 Setup:
   - At bootup, the /dev directory should get the 'devtmpfs' filesystem
-    mounted. Udev will manage permissions and ownership of the kernel-created
+    mounted. Udev manages the permissions and ownership of the kernel-created
-    device nodes, and possibly create additional symlinks. If needed, udev also
+    device nodes, and udev possibly creates additional symlinks. If needed, udev also
     works on an empty 'tmpfs' filesystem, but some static device nodes like
     /dev/null, /dev/console, /dev/kmsg are needed to be able to start udev itself.
 
   - The udev daemon should be started to handle device events sent by the kernel.
     During bootup, the kernel can be asked to send events for all already existing
-    devices, to apply the configuration to these devices. This is usually done by:
+    devices so that they too can be configured by udev. This is usually done by:
       /sbin/udevadm trigger --type=subsystems
       /sbin/udevadm trigger --typefivices
 
-  - Restarting the daemon does never apply any rules to existing devices.
+  - Restarting the daemon never applies any rules to existing devices.
 
-  - New/changed rule files are picked up automatically, there is no daemon
+  - New/changed rule files are picked up automatically; there is no daemon
     restart or signal needed.
 
 Operation:
-  - Udev creates/removes device nodes in /dev, based on events the kernel
-    sends out on device creation/removal.
+  - Based on events the kernel sends out on device creation/removal, udev
+    creates/removes device nodes in the /dev directory.
 
   - All kernel events are matched against a set of specified rules, which
     possibly hook into the event processing and load required kernel
-    modules to setup devices. For all devices the kernel exports a major/minor
+    modules to set up devices. For all devices, the kernel exports a major/minor
-    number, if needed, udev will create a device node with the default kernel
+    number; if needed, udev creates a device node with the default kernel
     name. If specified, udev applies permissions/ownership to the device
     node, creates additional symlinks pointing to the node, and executes
     programs to handle the device.
@@ -90,7 +90,7 @@ Operation:
       http://www.kernel.org/pub/linux/utils/kernel/hotplug/libudev/
       http://www.kernel.org/pub/linux/utils/kernel/hotplug/gudev/
 
-For more details about udev and udev rules see the udev(7) man page.
+For more details about udev and udev rules, see the udev(7) man page.
 
 Please direct any comment/question to the linux-hotplug mailing list at:
   linux-hotplug@vger.kernel.org
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox