Re: [Patch] udev: new dasd_id tool

[Kay Sievers <kay.sievers@vrfy.org>]

On Mon, 2006-11-27 at 18:05 +0100, Volker Sameske wrote:
> > > etc/udev/debian/persistent.rules             |    2
> > > etc/udev/redhat/50-udev.rules                |    2
> > > etc/udev/slackware/udev.rules                |    2
> > > etc/udev/suse/60-persistent-storage.rules    |    2
> > 
> > You can't patch any of these rules, they are usually copies _from_ the
> > distro as examples, not _to_ the distro.
> 
> Good point, I'll remove these 4 chunks.
>  
> > > - *     This program is free software; you can redistribute it 
> > and/or modify it
> > > - *     under the terms of the GNU General Public License as 
> > published by the
> > > - *     Free Software Foundation version 2 of the License.
> > 
> > Why do you want to remove the license?
> 
> Right, but I did not really remove it. I wrote a new tool 
> with a new header. And I have to follow IBM directives, how such
> a header has to look like. But it is still GPL.
> 
> > > -               value = getenv("UDEV_LOG");
> > > -               if (value)
> > > -                       udev_log = log_priority(value);
> > > -               else
> > > -                       udev_log = LOG_ERR;
> > 
> > Why don't we want that anymore?
> 
> Well, initially I wrote a new tool for the multipath-tools
> package. It was called dasdinfo. To a later point in time the 
> idea came to replace the existing udev dasd_id with it. So 
> even if the patch looks like I've changed dasd_id, I wrote a 
> new tool.
> But good point. I'll think about adding these lines.
>  
> > > +static int dinfo_create_devnode(dev_t dev, char** devno)
> > > +{
> > > +        char* result;
> > > +        char* pathname[] = { "/dev", getenv("TMPDIR"), "/tmp",
> > > +                             getenv("HOME"), "." , "/"};
> > 
> > What's are you trying to fight here? That's absolutely not needed,
> > just let udev provide you a temporary node, or use /dev, which is
> > obviously always writable in a udev setup.
> 
> Agree, if this would be only a udev-tool. But this tool will be
> also used within multipath-tools and there I have to create my own
> temporary device nodes. So I can't remove that.
>  
> > > +       sprintf(tmp,"/sys/bus/ccw/devices/%s/", busid);
> > > +       directory = opendir(tmp);
> > 
> > You can't reference /sys/bus/ directly, look at udev_sysfs.c, how to do 
> it.
> 
> Initially I had a sysfs-mountpoint detection. But it has been
> removed during a long discussion about that topic and after some
> review cycles. There are many many other tools with hard-coded
> sysfs mount points.
> And this sysfs-mount-point-detection takes some time since it will be
> done for every device. And there could be several hundred DASD devices
> on a single IBM System z.

No that's not what I mean, /sys is even hard-coded in udev, only
overwritable for the test-suite. But /sys/block may live
in /sys/subsystem/block and all that stuff that is queued at the moment
for sysfs changes. There will be backwards compat-links in sysfs for
existing apps to work, but in the udev tree, nothing new is accepted,
that relies on the compat-links.

> > > +       while ((dir_entry = readdir(directory)) != NULL) {
> > > +               if (strncmp(dir_entry->d_name, "block:", 6) = 0) {
> > 
> > You can't use these links, they will go away some day.
> > 
> > > +   sscanf(uname_buf.release, "%d.%d", &version,&release);
> > > +   if (strcmp(uname_buf.sysname,"Linux") ||
> > > +       version < 2 || release < 6) {
> > 
> > No need for that, there is no udev for kernels < 2.6.
> 
> Yes, but since this tools is not only used for udev I'll keep this check.
>  
> > Instead of all that, can't you just read the values from sysfs with
> > udev using $attr{} in a rule, and possibly fall back to the old tool
> > only if you don't find the values there?
> 
> Yes, if udev would be the only exploiter of this tool. Udev is one of
> the main exploiter, but not the single one.

Oh, then your tool probably doesn't want to hide in /lib/udev/ too,
right? The udev tarball does not want to carry around generic or
generally useful tools, only things which are private to udev and can be
changed to our needs without really caring about possible other users.

It may make sense, to remove the current tool from the udev tree, so
this udev version can just rely on the sysfs-value to be present.

Your new tool would go into its own package, from a distribution point
of view. Packagers can still use and integrate the new tool into
udev-rules, if they really feel the need for that kind of compatibility,
which I don't expect for udev's need.

Does that make sense?

Thanks,
Kay



-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CIDÞVDEV
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel