Skip to contentSkip to conversation

udev_volume_id -d option is broken

PATCH In review Christian Bornträger 2 participants 5 messages Jan 28, 2005 – Feb 8, 2005

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 udev_volume_id -d option is broken

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.

extras/volume_id/udev_volume_id.c +1 -1
diff -u udev-050/extras/volume_id/udev_volume_id.c new/extras/volume_id/udev_volume_id.c
@@ -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;
 
#1 lore raw
Quoted 7 lines · Christian Bornträger

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
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#2 lore raw
In reply to Christian Bornträger
Quoted 10 lines · Kay Sievers
Quoted 7 lines · Christian Bornträger

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
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#3 lore raw
In reply to Christian Bornträger

Kay Sievers <[email protected]> wrote:

[...]

Quoted 5 lines

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?

Signature · 20 lines
Mit freundlichen Grüßen / Best Regards / Un cordial saludo

Christian Bornträger
Linux Software Engineer
zSeries Linux & Virtualization
IBM Deutschland Entwicklung GmbH
email: [email protected]
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_id396&opÌk
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
#4 lore raw
In reply to Christian Bornträger
Quoted 19 lines · Christian Borntraeger

Kay Sievers <[email protected]> wrote:

[...]

Quoted 5 lines

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:

Quoted 3 lines

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.

...

Quoted 9 lines

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

extras/volume_id/udev_volume_id.c +5 -5
diff -Nru a/extras/volume_id/udev_volume_id.c b/extras/volume_id/udev_volume_id.c
@@ -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_id396&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel