Adding '%s' format specifier to NAME and SYMLINK

PATCH In review Hannes Reinecke 3 participants 7 messages Feb 10, 2004 – Feb 16, 2004

lore

Share & tools

lore

Hannes Reinecke

author

2004-02-10 08:14 UTC #0 lore raw

PATCH 1/1 Adding '%s' format specifier to NAME and SYMLINK

Hi all,

this patch makes the format for NAME and SYMLINK a bit more flexible: I've added a new format specifier '%s{<SYSFS_var>}', which allows for the value of any sysfs entry found for this device to be inserted. Example (for our S/390 fcp adapter):

BUS="ccw", SYSFS_devtype="1732/03", NAME="%k" \
SYMLINK="zfcp-%s{hba_id}-%s{wwpn}:%s{fcp_lun}"

I know this could also be done with an external program, but having this incorporated into udev makes life easier, especially if run from initramfs. Plus it makes the rules easier to follow, as the result is directly visible and need not to be looked up in some external program.

Comments etc. welcome.

Cheers,

Hannes

Signature · 4 lines

Dr. Hannes Reinecke			[email protected]
SuSE Linux AG				S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de

===== namedev.c 1.98 vs edited =====

namedev.c +90 ~~-15~~

@@ -168,7 +168,7 @@
 	return default_group_str;
 }
 
-static void apply_format(struct udevice *udev, unsigned char *string)
+static void apply_format(struct udevice *udev, unsigned char *string, struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device)
 {
 	char temp[NAME_SIZE];
 	char temp1[NAME_SIZE];

@@ -176,50 +176,75 @@
 	char *pos;
 	char *pos2;
 	char *pos3;
-	int num;
+	int num, len;
+	struct sysfs_attribute *tmpattr;
 
 	pos = string;
+	strcpy(temp, string);
+	tail = temp;
 	while (1) {
 		num = 0;
-		pos = strchr(pos, '%');
+		dbg("string is '%s' (index '%c')\n",string, *pos);
+#if 1
+		if (tail) {
+			pos2 = strchr(tail, '%');
+			if (!pos2) {
+				strcat(pos,tail);
+			} else {
+				if ((len = pos2 - tail) > 0) {
+					strncat(pos,tail,len);
+					pos += len;
+				}
+				*pos = '\0';
+			}
+			tail = pos2;
+		}
+		dbg("string '%s', tail '%s'\n", string, tail);
+#else
+		pos = strchr(pos,'%');
+#endif
 
-		if (pos) {
-			pos[0] = '\0';
-			tail = pos+1;
+		if (tail) {
+			tail ++;
 			if (isdigit(tail[0])) {
-				num = (int) strtoul(&pos[1], &tail, 10);
-				if (tail == NULL) {
-					dbg("format parsing error '%s'", pos+1);
+				num = (int) strtoul(tail, &pos2, 10);
+				if (pos2 == NULL) {
+					dbg("format parsing error '%s'", tail);
 					break;
 				}
+				tail = pos2;
 			}
-			strfieldcpy(temp, tail+1);
 
 			switch (tail[0]) {
 			case 'b':
 				if (strlen(udev->bus_id) == 0)
 					break;
 				strcat(pos, udev->bus_id);
+				tail++;
 				dbg("substitute bus_id '%s'", udev->bus_id);
 				break;
 			case 'k':
 				if (strlen(udev->kernel_name) == 0)
 					break;
 				strcat(pos, udev->kernel_name);
+				tail++;
 				dbg("substitute kernel name '%s'", udev->kernel_name);
 				break;
 			case 'n':
 				if (strlen(udev->kernel_number) == 0)
 					break;
 				strcat(pos, udev->kernel_number);
+				tail++;
 				dbg("substitute kernel number '%s'", udev->kernel_number);
 				break;
 			case 'm':
 				sprintf(pos, "%u", udev->minor);
+				tail++;
 				dbg("substitute minor number '%u'", udev->minor);
 				break;
 			case 'M':
 				sprintf(pos, "%u", udev->major);
+				tail++;
 				dbg("substitute major number '%u'", udev->major);
 				break;
 			case 'c':

@@ -245,16 +270,66 @@
 					strcat(pos, udev->program_result);
 					dbg("substitute result string '%s'", udev->program_result);
 				}
+				tail++;
 				break;
+			case 's':
+				if (tail[1] != '{') {
+					dbg("missing open braces for format");
+					break;
+				}
+				pos2 = tail + 2;
+				if (!(pos3 = strchr(pos2,'}'))) {
+					dbg("missing close brace for format");
+					break;
+				}
+				dbg("Tail is '%s'\n", temp);
+				*pos3 = '\0';
+				tail = pos3 + 1;
+				dbg("look for class attribute '%s'", pos2);
+				/* try to find the attribute in the class device directory */
+				tmpattr = sysfs_get_classdev_attr(class_dev, 
+								  pos2);
+				if (tmpattr) {
+				    pos3 = tmpattr->value + 
+					strlen(tmpattr->value)-1;
+				    if (*pos3 == '\n')
+					*pos3 = 0x00;
+				    strcpy(pos, tmpattr->value);
+				    pos += strlen(tmpattr->value);
+				    dbg("substitute sysfs value '%s'", 
+					tmpattr->value);
+				    break;
+				}
+				
+				/* look in the class device directory 
+				   if present */
+				if (!sysfs_device) {
+				    break;
+				}
+				dbg("look for device attribute '%s'", pos2);
+				tmpattr = sysfs_get_device_attr(sysfs_device, 
+								pos2);
+				if (tmpattr) {
+				    pos3 = tmpattr->value +
+					strlen(tmpattr->value)-1;
+				    if (*pos3 == '\n')
+					*pos3 = 0x00;
+				    strcpy(pos, tmpattr->value);
+				    pos += strlen(tmpattr->value);
+				    dbg("substitute sysfs value '%s'", 
+					tmpattr->value);
+				}				
+			        break;
+				
 			case '%':
 				strcat(pos, "%");
 				pos++;
 				break;
 			default:
-				dbg("unknown substitution type '%%%c'", pos[1]);
+				dbg("unknown substitution type '%%%c'", tail[1]);
 				break;
 			}
-			strcat(string, temp);
+/*			strcat(string, temp); */
 		} else
 			break;
 	}

@@ -647,7 +722,7 @@
 		/* execute external program */
 		if (dev->program[0] != '\0') {
 			dbg("check " FIELD_PROGRAM);
-			apply_format(udev, dev->program);
+			apply_format(udev, dev->program, class_dev, sysfs_device);
 			if (execute_program(dev->program, udev->program_result, NAME_SIZE) != 0) {
 				dbg(FIELD_PROGRAM " returned nozero");
 				goto no_good;

@@ -739,8 +814,8 @@
 
 found:
 	/* substitute placeholder */
-	apply_format(udev, udev->name);
-	apply_format(udev, udev->symlink);
+	apply_format(udev, udev->name, class_dev, sysfs_device);
+	apply_format(udev, udev->symlink, class_dev, sysfs_device);
 
 done:
 	perm = find_perm(udev->name);

Greg KH

2004-02-13 01:34 UTC #1 lore raw

Quoted 10 lines · Hannes Reinecke

Hi all,
this patch makes the format for NAME and SYMLINK a bit more flexible:
I've added a new format specifier '%s{<SYSFS_var>}', which allows for the value of any sysfs entry found for this device to be inserted. Example (for our S/390 fcp adapter):
BUS="ccw", SYSFS_devtype="1732/03", NAME="%k" \
SYMLINK="zfcp-%s{hba_id}-%s{wwpn}:%s{fcp_lun}"

Quoted 6 lines

I know this could also be done with an external program, but having this incorporated into udev makes life easier, especially if run from initramfs. Plus it makes the rules easier to follow, as the result is directly visible and need not to be looked up in some external program.
Comments etc. welcome.

Oops, sorry I missed this for the 017 release. I'll look at it tomorrow and get back to you. At first glance it looks like a good thing.

Oh, you forgot to update the documentation, that's important to do if you want this change to make it in :)

thanks,

greg k-h

-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now. Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id56&alloc_id438&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Kay Sievers

2004-02-14 18:32 UTC #2 lore raw

In reply to Hannes Reinecke

Quoted 16 lines · Greg KH

Quoted 15 lines · Hannes Reinecke
Hi all,
this patch makes the format for NAME and SYMLINK a bit more flexible:
I've added a new format specifier '%s{<SYSFS_var>}', which allows for the value of any sysfs entry found for this device to be inserted. Example (for our S/390 fcp adapter):
BUS="ccw", SYSFS_devtype="1732/03", NAME="%k" \
SYMLINK="zfcp-%s{hba_id}-%s{wwpn}:%s{fcp_lun}"
I know this could also be done with an external program, but having this incorporated into udev makes life easier, especially if run from initramfs. Plus it makes the rules easier to follow, as the result is directly visible and need not to be looked up in some external program.

Quoted 7 lines

Quoted 1 line
Comments etc. welcome.
Oops, sorry I missed this for the 017 release. I'll look at it tomorrow and get back to you. At first glance it looks like a good thing.
Oh, you forgot to update the documentation, that's important to do if you want this change to make it in :)

I took a part of the code and made a version that uses already implemented attribute finding logic.

The parsing of the format length '%3x' and the '%x{attribute}' is a fuction now, maybe there are more possible users in the future.

I've also added the test to udev-test.pl.

thanks,
Kay

===== namedev.c 1.104 vs edited =====

namedev.c +148 ~~-80~~

@@ -41,6 +41,8 @@
 #include "libsysfs/libsysfs.h"
 #include "klibc_fixups.h"
 
+static struct sysfs_attribute *find_sysfs_attribute(struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device, char *attr);
+
 LIST_HEAD(config_device_list);
 LIST_HEAD(perm_device_list);

@@ -168,7 +170,46 @@
 	return default_group_str;
 }
 
-static void apply_format(struct udevice *udev, unsigned char *string)
+/* extract possible {attr} and move str behind it */
+static char *get_format_attribute(char **str)
+{
+	char *pos;
+	char *attr = NULL;
+
+	if (*str[0] == '{') {
+		pos = strchr(*str, '}');
+		if (pos == NULL) {
+			dbg("missing closing brace for format");
+			return NULL;
+		}
+		pos[0] = '\0';
+		attr = *str+1;
+		*str = pos+1;
+		dbg("attribute='%s', str='%s'", attr, *str);
+	}
+	return attr;
+}
+
+/* extract possible format length and move str behind it*/
+static int get_format_len(char **str)
+{
+	int num;
+	char *tail;
+
+	if (isdigit(*str[0])) {
+		num = (int) strtoul(*str, &tail, 10);
+		if (tail != NULL) {
+			*str = tail;
+			dbg("format length=%i", num);
+			return num;
+		} else {
+			dbg("format parsing error '%s'", *str);
+		}
+	}
+	return -1;
+}
+
+static void apply_format(struct udevice *udev, unsigned char *string, struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device)
 {
 	char temp[NAME_SIZE];
 	char temp1[NAME_SIZE];

@@ -176,87 +217,100 @@
 	char *pos;
 	char *pos2;
 	char *pos3;
+	char *attr;
 	int num;
+	char c;
+	struct sysfs_attribute *tmpattr;
 
 	pos = string;
+
 	while (1) {
-		num = 0;
 		pos = strchr(pos, '%');
-
-		if (pos) {
+		if (pos != NULL) {
 			pos[0] = '\0';
 			tail = pos+1;
-			if (isdigit(tail[0])) {
-				num = (int) strtoul(&pos[1], &tail, 10);
-				if (tail == NULL) {
-					dbg("format parsing error '%s'", pos+1);
-					break;
-				}
-			}
+			num = get_format_len(&tail);
+			c = tail[0];
 			strfieldcpy(temp, tail+1);
+			tail = temp;
+		} else {
+			break;
+		}
+		dbg("format=%c, string='%s', tail='%s'",c , string, tail);
 
-			switch (tail[0]) {
-			case 'b':
-				if (strlen(udev->bus_id) == 0)
-					break;
-				strcat(pos, udev->bus_id);
-				dbg("substitute bus_id '%s'", udev->bus_id);
+		attr = get_format_attribute(&tail);
+
+		switch (c) {
+		case 'b':
+			if (strlen(udev->bus_id) == 0)
 				break;
-			case 'k':
-				if (strlen(udev->kernel_name) == 0)
-					break;
-				strcat(pos, udev->kernel_name);
-				dbg("substitute kernel name '%s'", udev->kernel_name);
+			strcat(pos, udev->bus_id);
+			dbg("substitute bus_id '%s'", udev->bus_id);
+			break;
+		case 'k':
+			if (strlen(udev->kernel_name) == 0)
 				break;
-			case 'n':
-				if (strlen(udev->kernel_number) == 0)
-					break;
-				strcat(pos, udev->kernel_number);
-				dbg("substitute kernel number '%s'", udev->kernel_number);
+			strcat(pos, udev->kernel_name);
+			dbg("substitute kernel name '%s'", udev->kernel_name);
+			break;
+		case 'n':
+			if (strlen(udev->kernel_number) == 0)
 				break;
-			case 'm':
-				sprintf(pos, "%u", udev->minor);
-				dbg("substitute minor number '%u'", udev->minor);
+			strcat(pos, udev->kernel_number);
+			dbg("substitute kernel number '%s'", udev->kernel_number);
 				break;
+		case 'm':
+			sprintf(pos, "%u", udev->minor);
+			dbg("substitute minor number '%u'", udev->minor);
+			break;
 			case 'M':
-				sprintf(pos, "%u", udev->major);
-				dbg("substitute major number '%u'", udev->major);
+			sprintf(pos, "%u", udev->major);
+			dbg("substitute major number '%u'", udev->major);
+			break;
+		case 'c':
+			if (strlen(udev->program_result) == 0)
 				break;
-			case 'c':
-				if (strlen(udev->program_result) == 0)
-					break;
-				if (num) {
-					/* get part of return string */
-					strncpy(temp1, udev->program_result, sizeof(temp1));
-					pos2 = temp1;
-					while (num) {
-						num--;
-						pos3 = strsep(&pos2, " ");
-						if (pos3 == NULL) {
-							dbg("requested part of result string not found");
-							break;
-						}
-					}
-					if (pos3) {
-						strcat(pos, pos3);
-						dbg("substitute part of result string '%s'", pos3);
+			if (num > 0) {
+				strncpy(temp1, udev->program_result, sizeof(temp1));
+				pos2 = temp1;
+				while (num) {
+					num--;
+					pos3 = strsep(&pos2, " ");
+					if (pos3 == NULL) {
+						dbg("requested part of result string not found");
+						break;
 					}
-				} else {
-					strcat(pos, udev->program_result);
-					dbg("substitute result string '%s'", udev->program_result);
 				}
-				break;
-			case '%':
-				strcat(pos, "%");
-				pos++;
-				break;
-			default:
-				dbg("unknown substitution type '%%%c'", pos[1]);
-				break;
+				if (pos3) {
+					strcat(pos, pos3);
+					dbg("substitute part of result string '%s'", pos3);
+				}
+			} else {
+				strcat(pos, udev->program_result);
+				dbg("substitute result string '%s'", udev->program_result);
 			}
-			strcat(string, temp);
-		} else
 			break;
+		case 's':
+			if (attr != NULL) {
+				tmpattr = find_sysfs_attribute(class_dev, sysfs_device, attr);
+				if (tmpattr == NULL) {
+					dbg("sysfa attribute '%s' not found", attr);
+					break;
+				}
+				strcpy(pos, tmpattr->value);
+				dbg("substitute sysfs value '%s'", tmpattr->value);
+			} else {
+				dbg("missing attribute");
+			}
+			break;
+		case '%':
+			strcat(pos, "%");
+			break;
+		default:
+			dbg("unknown substitution type '%%%c'", c);
+			break;
+		}
+		strcat(pos, tail);
 	}
 }

@@ -422,32 +476,46 @@
 	return retval;
 }
 
-static int compare_sysfs_attribute(struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device, struct sysfs_pair *pair)
+static struct sysfs_attribute *find_sysfs_attribute(struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device, char *attr)
 {
 	struct sysfs_attribute *tmpattr = NULL;
 	char *c;
 
-	if ((pair == NULL) || (pair->file[0] == '\0') || (pair->value == '\0'))
-		return -ENODEV;
-
-	dbg("look for device attribute '%s'", pair->file);
+	dbg("look for device attribute '%s'", attr);
 	/* try to find the attribute in the class device directory */
-	tmpattr = sysfs_get_classdev_attr(class_dev, pair->file);
+	tmpattr = sysfs_get_classdev_attr(class_dev, attr);
 	if (tmpattr)
-		goto label_found;
+		goto attr_found;
 
 	/* look in the class device directory if present */
 	if (sysfs_device) {
-		tmpattr = sysfs_get_device_attr(sysfs_device, pair->file);
+		tmpattr = sysfs_get_device_attr(sysfs_device, attr);
 		if (tmpattr)
-			goto label_found;
+			goto attr_found;
 	}
-	return -ENODEV;
 
-label_found:
-	c = tmpattr->value + strlen(tmpattr->value)-1;
-	if (*c == '\n')
-		*c = 0x00;
+	return NULL;
+
+attr_found:
+	c = strchr(tmpattr->value, '\n');
+	if (c != NULL)
+		c[0] = '\0';
+
+	dbg("found attribute '%s'", tmpattr->path);
+	return tmpattr;
+}
+
+static int compare_sysfs_attribute(struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device, struct sysfs_pair *pair)
+{
+	struct sysfs_attribute *tmpattr;
+
+	if ((pair == NULL) || (pair->file[0] == '\0') || (pair->value == '\0'))
+		return -ENODEV;
+
+	tmpattr = find_sysfs_attribute(class_dev, sysfs_device, pair->file);
+	if (tmpattr == NULL)
+		return -ENODEV;
+
 	dbg("compare attribute '%s' value '%s' with '%s'",
 		  pair->file, tmpattr->value, pair->value);
 	if (strcmp_pattern(pair->value, tmpattr->value) != 0)

@@ -658,7 +726,7 @@
 		/* execute external program */
 		if (dev->program[0] != '\0') {
 			dbg("check " FIELD_PROGRAM);
-			apply_format(udev, dev->program);
+			apply_format(udev, dev->program, class_dev, sysfs_device);
 			if (execute_program(dev->program, udev->program_result, NAME_SIZE) != 0) {
 				dbg(FIELD_PROGRAM " returned nozero");
 				goto try_parent;

@@ -750,8 +818,8 @@
 
 found:
 	/* substitute placeholder */
-	apply_format(udev, udev->name);
-	apply_format(udev, udev->symlink);
+	apply_format(udev, udev->name, class_dev, sysfs_device);
+	apply_format(udev, udev->symlink, class_dev, sysfs_device);
 
 done:
 	perm = find_perm(udev->name);

===== test/udev-test.pl 1.30 vs edited =====

test/udev-test.pl +11 -1

@@ -194,6 +194,16 @@
 EOF
 	},
 	{
+		desc     => "sustitution of sysfs value (%s{file})",
+		subsys   => "block",
+		devpath  => "block/sda",
+		expected => "disk-IBM-ESXS-sda" ,
+		conf     => <<EOF
+BUS="scsi", SYSFS_vendor="IBM-ESXS", NAME="disk-%s{vendor}-%k"
+KERNEL="ttyUSB0", NAME="visor"
+EOF
+	},
+	{
 		desc     => "program result substitution",
 		subsys   => "block",
 		devpath  => "block/sda/sda3",

@@ -418,7 +428,7 @@
 	$test_num = $ARGV[0];
 	print "udev-test will run test number $test_num only\n";
 
-	run_test($tests[$test_num], $test_num);
+	run_test($tests[$test_num-1], $test_num-1);
 } else {
 	# test all
 	print "\nudev-test will run ".($#tests + 1)." tests:\n\n";

Kay Sievers

2004-02-15 02:36 UTC #3 lore raw

In reply to Hannes Reinecke

Quoted 25 lines · Kay Sievers

Quoted 16 lines · Greg KH
Quoted 15 lines · Hannes Reinecke
Hi all,
this patch makes the format for NAME and SYMLINK a bit more flexible:
I've added a new format specifier '%s{<SYSFS_var>}', which allows for the value of any sysfs entry found for this device to be inserted. Example (for our S/390 fcp adapter):
BUS="ccw", SYSFS_devtype="1732/03", NAME="%k" \
SYMLINK="zfcp-%s{hba_id}-%s{wwpn}:%s{fcp_lun}"
I know this could also be done with an external program, but having this incorporated into udev makes life easier, especially if run from initramfs. Plus it makes the rules easier to follow, as the result is directly visible and need not to be looked up in some external program.
Quoted 7 lines
Quoted 1 line
Comments etc. welcome.
Oops, sorry I missed this for the 017 release. I'll look at it tomorrow and get back to you. At first glance it looks like a good thing.
Oh, you forgot to update the documentation, that's important to do if you want this change to make it in :)

Quoted 7 lines

I took a part of the code and made a version that uses already implemented attribute finding logic.
The parsing of the format length '%3x' and the '%x{attribute}' is a fuction now, maybe there are more possible users in the future.
I've also added the test to udev-test.pl.

Since we have %s{file} it may be nice to allow SYSFS{file}. This patch allows:

BUS="usb", SYSFS{idProduct}="a511", NAME="video%n"

compared to the current:

BUS="usb", SYSFS_idProduct="a511", NAME="video%n"

The curent syntax is still supported.
Looks a bit nicer and less hackish, I think.

thanks,
Kay

namedev_parse.c +6 -2

diff -Nru a/namedev_parse.c b/namedev_parse.c
@@ -92,6 +92,7 @@
 	char *temp;
 	char *temp2;
 	char *temp3;
+	char *pos;
 	FILE *fd;
 	int program_given = 0;
 	int retval = 0;

@@ -150,7 +151,7 @@
 				continue;
 			}
 
-			if (strncasecmp(temp2, FIELD_SYSFS, sizeof(FIELD_SYSFS)-1) == 0) {
+			if (strncasecmp(temp2, FIELD_SYSFS, sizeof(FIELD_SYSFS)-2) == 0) {
 				struct sysfs_pair *pair = &dev.sysfs_pair[0];
 				int sysfs_pair_num = 0;

@@ -164,8 +165,11 @@
 					++pair;
 				}
 				if (pair) {
-					/* remove prepended 'SYSFS_' */
+					/* extract file from 'SYSFS{file}' or 'SYSFS_file' */
 					strfieldcpy(pair->file, temp2 + sizeof(FIELD_SYSFS)-1);
+					pos = strchr(pair->file, '}');
+					if (pos != NULL)
+						pos[0] = '\0';
 					strfieldcpy(pair->value, temp3);
 				}
 				continue;

test/udev-test.pl +10 -0

diff -Nru a/test/udev-test.pl b/test/udev-test.pl
@@ -194,6 +194,16 @@
 EOF
 	},
 	{
+		desc     => "select sysfs attribute by SYFS{vendor}",
+		subsys   => "block",
+		devpath  => "block/sda",
+		expected => "disk-IBM-ESXS-sda" ,
+		conf     => <<EOF
+BUS="scsi", SYSFS{vendor}="IBM-ESXS", NAME="disk-%s{vendor}-%k"
+KERNEL="ttyUSB0", NAME="visor"
+EOF
+	},
+	{
 		desc     => "sustitution of sysfs value (%s{file})",
 		subsys   => "block",
 		devpath  => "block/sda",

Kay Sievers

2004-02-16 00:54 UTC #4 lore raw

In reply to Hannes Reinecke

Quoted 37 lines · Kay Sievers

Quoted 25 lines · Kay Sievers
Quoted 16 lines · Greg KH
Quoted 15 lines · Hannes Reinecke
Hi all,
this patch makes the format for NAME and SYMLINK a bit more flexible:
I've added a new format specifier '%s{<SYSFS_var>}', which allows for the value of any sysfs entry found for this device to be inserted. Example (for our S/390 fcp adapter):
BUS="ccw", SYSFS_devtype="1732/03", NAME="%k" \
SYMLINK="zfcp-%s{hba_id}-%s{wwpn}:%s{fcp_lun}"
I know this could also be done with an external program, but having this incorporated into udev makes life easier, especially if run from initramfs. Plus it makes the rules easier to follow, as the result is directly visible and need not to be looked up in some external program.
Quoted 7 lines
Quoted 1 line
Comments etc. welcome.
Oops, sorry I missed this for the 017 release. I'll look at it tomorrow and get back to you. At first glance it looks like a good thing.
Oh, you forgot to update the documentation, that's important to do if you want this change to make it in :)
Quoted 7 lines
I took a part of the code and made a version that uses already implemented attribute finding logic.
The parsing of the format length '%3x' and the '%x{attribute}' is a fuction now, maybe there are more possible users in the future.
I've also added the test to udev-test.pl.
Since we have %s{file} it may be nice to allow SYSFS{file}. This patch allows:

Quoted 8 lines

BUS="usb", SYSFS{idProduct}="a511", NAME="video%n"
compared to the current:
BUS="usb", SYSFS_idProduct="a511", NAME="video%n"
The curent syntax is still supported.
Looks a bit nicer and less hackish, I think.

Better patch with infrastructure to easily implement KEY{attribute} for every other key. The first user is the SYSFS{file} key. Both versions, brackets or underscore is supported for the attribute.

thanks,
Kay

namedev.h +1 -1

diff -Nru a/namedev.h b/namedev.h
@@ -36,7 +36,7 @@
 #define PROGRAM_SIZE	100
 
 #define FIELD_BUS	"BUS"
-#define FIELD_SYSFS	"SYSFS_"
+#define FIELD_SYSFS	"SYSFS"
 #define FIELD_ID	"ID"
 #define FIELD_PLACE	"PLACE"
 #define FIELD_PROGRAM	"PROGRAM"

namedev_parse.c +39 -4

71 lines not on this page · raw message

test/udev-test.pl +10 -0

diff -Nru a/test/udev-test.pl b/test/udev-test.pl
@@ -194,6 +194,16 @@
 EOF
 	},
 	{
+		desc     => "select sysfs attribute by SYSFS{vendor}",
+		subsys   => "block",
+		devpath  => "block/sda",
+		expected => "disk-IBM-ESXS-sda" ,
+		conf     => <<EOF
+BUS="scsi", SYSFS{vendor}="IBM-ESXS", NAME="disk-%s{vendor}-%k"
+KERNEL="ttyUSB0", NAME="visor"
+EOF
+	},
+	{
 		desc     => "sustitution of sysfs value (%s{file})",
 		subsys   => "block",
 		devpath  => "block/sda",

Greg KH

2004-02-16 21:40 UTC #5 lore raw

In reply to Hannes Reinecke

Quoted 25 lines · Kay Sievers

Quoted 16 lines · Greg KH
Quoted 15 lines · Hannes Reinecke
Hi all,
this patch makes the format for NAME and SYMLINK a bit more flexible:
I've added a new format specifier '%s{<SYSFS_var>}', which allows for the value of any sysfs entry found for this device to be inserted. Example (for our S/390 fcp adapter):
BUS="ccw", SYSFS_devtype="1732/03", NAME="%k" \
SYMLINK="zfcp-%s{hba_id}-%s{wwpn}:%s{fcp_lun}"
I know this could also be done with an external program, but having this incorporated into udev makes life easier, especially if run from initramfs. Plus it makes the rules easier to follow, as the result is directly visible and need not to be looked up in some external program.
Quoted 7 lines
Quoted 1 line
Comments etc. welcome.
Oops, sorry I missed this for the 017 release. I'll look at it tomorrow and get back to you. At first glance it looks like a good thing.
Oh, you forgot to update the documentation, that's important to do if you want this change to make it in :)

Quoted 7 lines

I took a part of the code and made a version that uses already implemented attribute finding logic.
The parsing of the format length '%3x' and the '%x{attribute}' is a fuction now, maybe there are more possible users in the future.
I've also added the test to udev-test.pl.

Nice cleanup. I've applied this.

thanks,

greg k-h

Greg KH

2004-02-16 21:40 UTC #6 lore raw

In reply to Hannes Reinecke

Quoted 8 lines · Kay Sievers

Quoted 7 lines · Kay Sievers
Since we have %s{file} it may be nice to allow SYSFS{file}. This patch allows:
BUS="usb", SYSFS{idProduct}="a511", NAME="video%n"
compared to the current:

Quoted 8 lines

Quoted 4 lines
BUS="usb", SYSFS_idProduct="a511", NAME="video%n"
The curent syntax is still supported.
Looks a bit nicer and less hackish, I think.
Better patch with infrastructure to easily implement KEY{attribute} for every other key. The first user is the SYSFS{file} key. Both versions, brackets or underscore is supported for the attribute.

Aplied, thanks.

greg k-h