[BUG, Patch] udev_volume_id -d option is broken

[BUG, Patch] udev_volume_id -d option is broken

[Christian Bornträger]

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

Hello all,

The -d option in udev_allows to go from a partition to the underlying disk 
for s390 dasd labels. If the device is already the disk itself, finding the 
parent will fail, therefore -d on /sys/block/dasda/ for example gives no 
result at all. 
This patch seems to fix this issue.


[-- Attachment #2: dasd1.patch --]
[-- Type: text/x-diff, Size: 470 bytes --]

diff -u udev-050/extras/volume_id/udev_volume_id.c new/extras/volume_id/udev_volume_id.c
--- udev-050/extras/volume_id/udev_volume_id.c	2004-12-18 06:53:07.000000000 +0100
+++ new/extras/volume_id/udev_volume_id.c	2005-01-27 19:15:39.598923207 +0100
@@ -187,7 +186,7 @@
 		if (class_dev_parent != NULL)
 			vid = open_classdev(class_dev_parent);
 		else
-			vid = open_classdev(class_dev_parent);
+			vid = open_classdev(class_dev);
 		if (vid == NULL)
 			goto exit;
 

Re: [BUG, Patch] udev_volume_id -d option is broken

[Kay Sievers]

On Fri, 2005-01-28 at 14:41 +0100, Christian Bornträger wrote:
> The -d option in udev_allows to go from a partition to the underlying
> disk 
> for s390 dasd labels. If the device is already the disk itself,
> finding the 
> parent will fail, therefore -d on /sys/block/dasda/ for example gives
> no 
> result at all.

Yes, seems like a stupid bug. I've applied it to my tree.

Thanks for fixing it, 
Kay



-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
_______________________________________________
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

Re: [BUG, Patch] udev_volume_id -d option is broken

[Kay Sievers]

On Sun, 2005-01-30 at 05:36 +0100, Kay Sievers wrote:
> On Fri, 2005-01-28 at 14:41 +0100, Christian Bornträger wrote:
> > The -d option in udev_allows to go from a partition to the underlying
> > disk 
> > for s390 dasd labels. If the device is already the disk itself,
> > finding the 
> > parent will fail, therefore -d on /sys/block/dasda/ for example gives
> > no 
> > result at all.
> 
> Yes, seems like a stupid bug. I've applied it to my tree.

If you have a chance to try the version in udev-051 on one of these
boxes, it would be nice to know if it is still working as expected. I
have moved the dasd label reading into a restructured version of the
volume_id library now.

Thanks,
Kay



-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
_______________________________________________
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

Re: [BUG, Patch] udev_volume_id -d option is broken

[Christian Borntraeger]

Kay Sievers <kay.sievers@vrfy.org> wrote:

[...]
> If you have a chance to try the version in udev-051 on one of these
> boxes, it would be nice to know if it is still working as expected. I
> have moved the dasd label reading into a restructured version of the
> volume_id library now.
> 

Yes - sort of. Using -d alone works fine, but prints a lot of information:

[root@tel22fe dev]# DEVPATH=/block/dasdb/dasdb1  /sbin/udev_volume_id -d
F:disklabel
T:dasd
V:
L:0XCE3E
N:0XCE3E
U:

Usually, you are only interested in the volume lavel for your udev rule, 
so, you might combine -d with -l. Unfortunately that does not work. It 
also failed with udev 50....but it worked with udev 24 IIRC.
Reason is the following code:

-------udev_volume_id.c-------------------------------
        case 'l':
                if (name[0] = '\0' || vid->usage_id != 
VOLUME_ID_FILESYSTEM) {
                        rc = 2;
                        goto exit;
                }
                printf("%s\n", name);
                break;
-------------------------------------------------------

Of course, vid->usage_id is != VOLUME_ID_FILESYSTEM if you read a volume 
label.
Therefore, the "-l" option does not work on a dasd.

I see 2 posibilities:

1: Make -d -l working, by changing this if statement

2: change the meaning of -d : only print the volume label

Ideas?



-- 
Mit freundlichen Grüßen / Best Regards / Un cordial saludo

Christian Bornträger
Linux Software Engineer
zSeries Linux & Virtualization
IBM Deutschland Entwicklung GmbH
email: CBORNTRA@de.ibm.com
Tel +49  7031 16 1975



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&opÌk
_______________________________________________
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

Re: [BUG, Patch] udev_volume_id -d option is broken

[Kay Sievers]

On Mon, 2005-02-07 at 19:39 +0100, Christian Borntraeger wrote:
> Kay Sievers <kay.sievers@vrfy.org> wrote:
> 
> [...]
> > If you have a chance to try the version in udev-051 on one of these
> > boxes, it would be nice to know if it is still working as expected. I
> > have moved the dasd label reading into a restructured version of the
> > volume_id library now.
> > 
> 
> Yes - sort of. Using -d alone works fine, but prints a lot of information:
> 
> [root@tel22fe dev]# DEVPATH=/block/dasdb/dasdb1  /sbin/udev_volume_id -d
> F:disklabel
> T:dasd
> V:
> L:0XCE3E
> N:0XCE3E
> U:
> 
> Usually, you are only interested in the volume lavel for your udev rule, 
> so, you might combine -d with -l. Unfortunately that does not work. It 
> also failed with udev 50....but it worked with udev 24 IIRC.

Hmm, bad, sorry. Thanks for testing it. 

...
> Of course, vid->usage_id is != VOLUME_ID_FILESYSTEM if you read a volume 
> label.
> Therefore, the "-l" option does not work on a dasd.
> 
> I see 2 posibilities:
> 
> 1: Make -d -l working, by changing this if statement
> 
> 2: change the meaning of -d : only print the volume label

This should work to print only the label. I've applied this to my tree
and it will appear in the next release.

Thanks again,
Kay

diff -Nru a/extras/volume_id/udev_volume_id.c b/extras/volume_id/udev_volume_id.c
--- a/extras/volume_id/udev_volume_id.c	2005-02-08 20:41:16 +01:00
+++ b/extras/volume_id/udev_volume_id.c	2005-02-08 20:41:16 +01:00
@@ -89,7 +89,7 @@
 	struct sysfs_class_device *class_dev_parent = NULL;
 	struct volume_id *vid = NULL;
 	char *devpath;
-	char probe_main_device = 0;
+	char probe_disk_label = 0;
 	char print = 'a';
 	static char name[VOLUME_ID_LABEL_SIZE];
 	int len, i, j;
@@ -116,7 +116,7 @@
 			print = 'u';
 			continue;
 		case 'd':
-			probe_main_device = 1;
+			probe_disk_label = 1;
 			continue;
 		case 'h':
 		case '?':
@@ -146,8 +146,7 @@
 		goto exit;
 	}
 
-	if (probe_main_device = 0) {
-		/* open block device */
+	if (probe_disk_label = 0) {
 		vid = open_classdev(class_dev);
 		if (vid = NULL)
 			goto exit;
@@ -205,7 +204,8 @@
 		printf("%s\n", vid->type);
 		break;
 	case 'l':
-		if (name[0] = '\0' || vid->usage_id != VOLUME_ID_FILESYSTEM) {
+		if (name[0] = '\0' ||
+		    (vid->usage_id != VOLUME_ID_FILESYSTEM && vid->usage_id != VOLUME_ID_DISKLABEL)) {
 			rc = 2;
 			goto exit;
 		}




-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
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