Re: Problems using scsi_id with udevstart

[Andrew Patterson <andrew.patterson@hp.com>]

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

On Wed, 2004-10-06 at 23:39 +0200, Kay Sievers wrote:
> On Wed, Oct 06, 2004 at 03:22:03PM -0600, Andrew Patterson wrote:
> > On Wed, 2004-10-06 at 21:13 +0200, Kay Sievers wrote:
> > > On Wed, 2004-10-06 at 12:42 -0600, Andrew Patterson wrote:
> > > > On Wed, 2004-10-06 at 09:59 -0700, Patrick Mansfield wrote:
> > > > > On Wed, Oct 06, 2004 at 10:26:13AM -0600, Andrew Patterson wrote:
> > > > > 
> > > > > > Note: here is the error you get when scsi_id is run without any
> > > > > > parameters:
> > > > > > 
> > > > > > # /sbin/scsi_id
> > > > > > -s must be specified
> > > > > 
> > > > > For the udev PROGRAM rules with no arguments, it is effectively invoked
> > > > > like this (well there are other environment values but scsi_id only cares
> > > > > about DEVPATH):
> > > > > 
> > > > > DEVPATH=/block/sda /sbin/scsi_id block
> > > > > 
> > > > 
> > > > This is what I thought was supposed to happen.  I printed out the
> > > > arguments and the environment when scsi_id was invoked through
> > > > udevstart.  No arguments were passed and UDEVPATH was always set
> > > > to /class/net/lo for every path in /sys.  Note that DEVNAME does seem to
> > > > be set correctly.
> > > 
> > > Yes, this is a bug. We need to export the DEVPATH for the callout if we
> > > run as udevstart. I will fix that now.
> > 
> > So did you confirm that the subsystem is indeed being passed as an
> > argument when running udevstart as well?  My testing shows no.  I think
> > you need both to make this work.
> 
> Yes, when udevstart was running we didn't set the environment and the
> subsystem argument for the callouts the dev.d/ scripts.
> 
> Here is a fix, that sets that with every udevstart iteration, corrects
> argv[0] to be the basename() only not the whole path  and adds a test
> for invoking callouts without arguments.
> 
> Thanks,
> Kay

This works fine.  Thanks.

Andrew

> Plain text document attachment (udevstart-env-fix-01.patch)
> ===== namedev.c 1.147 vs edited =====
> --- 1.147/namedev.c	2004-09-20 16:01:58 +02:00
> +++ edited/namedev.c	2004-10-06 23:27:35 +02:00
> @@ -412,7 +412,7 @@ static void fix_kernel_name(struct udevi
>  	}
>  }
>  
> -static int execute_program(char *path, char *value, int len)
> +static int execute_program(const char *path, char *value, int len)
>  {
>  	int retval;
>  	int count;
> @@ -421,12 +421,12 @@ static int execute_program(char *path, c
>  	pid_t pid;
>  	char *pos;
>  	char arg[PROGRAM_SIZE];
> -	char *argv[sizeof(arg) / 2];
> +	char *argv[(PROGRAM_SIZE / 2) + 1];
>  	int i;
>  
> +	strfieldcpy(arg, path);
>  	i = 0;
>  	if (strchr(path, ' ')) {
> -		strfieldcpy(arg, path);
>  		pos = arg;
>  		while (pos != NULL) {
>  			if (pos[0] == '\'') {
> @@ -441,8 +441,19 @@ static int execute_program(char *path, c
>  			dbg("arg[%i] '%s'", i, argv[i]);
>  			i++;
>  		}
> +		argv[i] =  NULL;
> +		dbg("execute '%s' with parsed arguments", arg);
> +	} else {
> +		argv[0] = arg;
> +		argv[1] = main_argv[1];
> +		argv[2] = NULL;
> +		dbg("execute '%s' with subsystem '%s' argument", arg, argv[1]);
>  	}
> -	argv[i] =  NULL;
> +
> +	/* set basename() only */
> +	pos = strrchr(argv[0], '/');
> +	if (pos != NULL)
> +		argv[0] = &pos[1];
>  
>  	retval = pipe(fds);
>  	if (retval != 0) {
> @@ -456,13 +467,7 @@ static int execute_program(char *path, c
>  		/* child */
>  		/* dup2 write side of pipe to STDOUT */
>  		dup2(fds[1], STDOUT_FILENO);
> -		if (argv[0] !=  NULL) {
> -			dbg("execute '%s' with given arguments", argv[0]);
> -			retval = execv(argv[0], argv);
> -		} else {
> -			dbg("execute '%s' with main argument", path);
> -			retval = execv(path, main_argv);
> -		}
> +		retval = execv(arg, argv);
>  
>  		info(FIELD_PROGRAM " execution of '%s' failed", path);
>  		exit(1);
> ===== udevstart.c 1.17 vs edited =====
> --- 1.17/udevstart.c	2004-09-14 15:13:59 +02:00
> +++ edited/udevstart.c	2004-10-06 22:43:28 +02:00
> @@ -86,6 +86,21 @@ static char *first_list[] = {
>  	NULL,
>  };
>  
> +static void add_device(char *path, char *subsys, int fake)
> +{
> +	char *argv[3];
> +
> +	/* fake argument vector and environment for callouts and dev.d/ */
> +	argv[0] = "udev";
> +	argv[1] = subsys;
> +	argv[2] = NULL;
> +
> +	main_argv = argv;
> +	setenv("DEVPATH", path, 1);
> +	setenv("ACTION", "add", 1);
> +	udev_add_device(path, subsys, fake);
> +}
> +
>  static void exec_list(struct list_head *device_list)
>  {
>  	struct device *loop_device;
> @@ -96,7 +111,7 @@ static void exec_list(struct list_head *
>  	list_for_each_entry_safe(loop_device, tmp_device, device_list, list) {
>  		for (i=0; first_list[i] != NULL; i++) {
>  			if (strncmp(loop_device->path, first_list[i], strlen(first_list[i])) == 0) {
> -				udev_add_device(loop_device->path, loop_device->subsys, NOFAKE);
> +				add_device(loop_device->path, loop_device->subsys, NOFAKE);
>  				list_del(&loop_device->list);
>  				free(loop_device);
>  				break;
> @@ -116,14 +131,14 @@ static void exec_list(struct list_head *
>  		if (found)
>  			continue;
>  
> -		udev_add_device(loop_device->path, loop_device->subsys, NOFAKE);
> +		add_device(loop_device->path, loop_device->subsys, NOFAKE);
>  		list_del(&loop_device->list);
>  		free(loop_device);
>  	}
>  
>  	/* handle the rest of the devices left over, if any */
>  	list_for_each_entry_safe(loop_device, tmp_device, device_list, list) {
> -		udev_add_device(loop_device->path, loop_device->subsys, NOFAKE);
> +		add_device(loop_device->path, loop_device->subsys, NOFAKE);
>  		list_del(&loop_device->list);
>  		free(loop_device);
>  	}
> ===== test/udev-test.pl 1.58 vs edited =====
> --- 1.58/test/udev-test.pl	2004-09-14 19:42:53 +02:00
> +++ edited/test/udev-test.pl	2004-10-06 23:11:09 +02:00
> @@ -256,6 +256,15 @@ BUS="scsi", PROGRAM="/bin/echo -n specia
>  EOF
>  	},
>  	{
> +		desc		=> "program result substitution (no argument should be subsystem)",
> +		subsys		=> "block",
> +		devpath		=> "/block/sda/sda3",
> +		exp_name	=> "subsys_block" ,
> +		conf		=> <<EOF
> +BUS="scsi", PROGRAM="/bin/echo", RESULT="block", NAME="subsys_block"
> +EOF
> +	},
> +	{
>  		desc		=> "program result substitution (newline removal)",
>  		subsys		=> "block",
>  		devpath		=> "/block/sda/sda3",

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]