scsi_id - EBUSY and serialization

scsi_id - EBUSY and serialization

[Harald Hoyer]

[-- Attachment #1.1: Type: text/plain, Size: 402 bytes --]

We have a customer, who misses some symlinks for some of his scsi devices. Because of limited access and 
debugging facilities, I could only guess a fix.

The first patch retries opening the scsi device, if it is EBUSY.
The second patch serializes access with flock.

With these patches it seems to work. I don't know if the flock patch is really necessary. Any help and 
comments appreciated.

Harald

[-- Attachment #1.2: 0001-retry-open-on-EBUSY.patch --]
[-- Type: text/plain, Size: 1122 bytes --]

From b7d94db71475a92efc11351cfd621e081b6c8bce Mon Sep 17 00:00:00 2001
From: Harald Hoyer <harald@redhat.com>
Date: Wed, 11 Jun 2008 11:32:34 +0200
Subject: [PATCH] retry open on EBUSY

---
 extras/scsi_id/scsi_serial.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/extras/scsi_id/scsi_serial.c b/extras/scsi_id/scsi_serial.c
index c5cacfa..ec72c7d 100644
--- a/extras/scsi_id/scsi_serial.c
+++ b/extras/scsi_id/scsi_serial.c
@@ -822,10 +822,17 @@ int scsi_get_serial (struct scsi_id_device *dev_scsi, const char *devname,
 	int fd;
 	int ind;
 	int retval;
+	int cnt = 10;
 
 	memset(dev_scsi->serial, 0, len);
 	dbg("opening %s\n", devname);
-	fd = open(devname, O_RDONLY | O_NONBLOCK);
+	while (((fd = open(devname, O_RDONLY | O_NONBLOCK)) != 0) && (cnt > 0) && errno == EBUSY) {
+		info("%s: cannot open %s: %s",
+		    dev_scsi->kernel_name, devname, strerror(errno));	  	  
+		info("retrying in 0.5 seconds");
+		usleep(500000 + (rand() % 100000) );
+		cnt--;
+	}
 	if (fd < 0) {
 		info("%s: cannot open %s: %s\n",
 		    dev_scsi->kernel, devname, strerror(errno));
-- 
1.5.5.1


[-- Attachment #1.3: 0002-serialize-scsi_id-access-with-flock.patch --]
[-- Type: text/plain, Size: 1341 bytes --]

From ec3abdfd0b2e4f596c4cc988d5b7e6714cdb264e Mon Sep 17 00:00:00 2001
From: Harald Hoyer <harald@redhat.com>
Date: Wed, 11 Jun 2008 11:42:00 +0200
Subject: [PATCH] serialize scsi_id access with flock

---
 extras/scsi_id/scsi_id.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/extras/scsi_id/scsi_id.c b/extras/scsi_id/scsi_id.c
index 5eb95e8..04ae676 100644
--- a/extras/scsi_id/scsi_id.c
+++ b/extras/scsi_id/scsi_id.c
@@ -26,6 +26,7 @@
 #include <ctype.h>
 #include <getopt.h>
 #include <sys/stat.h>
+#include <sys/file.h>
 
 #include "../../udev.h"
 #include "scsi_id.h"
@@ -601,6 +602,17 @@ static int scsi_id(char *maj_min_dev)
 	int page_code;
 	char serial_short[MAX_SERIAL_LEN] = "";
 
+ 	char flock_path[MAX_PATH_LEN];
+ 	int flock_fd = -1;
+ 
+ 	strncpy(flock_path, "/sys/", MAX_PATH_LEN-1);
+ 	strncat(flock_path, devpath, MAX_PATH_LEN-1);
+ 	flock_path[MAX_PATH_LEN-1] = 0;
+ 
+ 	flock_fd = open(flock_path, O_RDONLY);
+ 	if (flock_fd >= 0)
+ 	  flock(flock_fd, LOCK_EX);
+ 
 	set_inq_values(&dev_scsi, maj_min_dev);
 
 	/* get per device (vendor + model) options from the config file */
@@ -636,6 +648,11 @@ static int scsi_id(char *maj_min_dev)
 		retval = 0;
 	}
 
+	if (flock_fd >= 0) {
+	  flock(flock_fd, LOCK_UN);
+	  close(flock_fd);
+	}
+
 	return retval;
 }
 
-- 
1.5.5.1


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/x-pkcs7-signature, Size: 3636 bytes --]

Re: scsi_id - EBUSY and serialization

[Kay Sievers]

On Wed, Jun 11, 2008 at 11:55 AM, Harald Hoyer <harald@redhat.com> wrote:
> We have a customer, who misses some symlinks for some of his scsi devices.
> Because of limited access and debugging facilities, I could only guess a
> fix.
>
> The first patch retries opening the scsi device, if it is EBUSY.
> The second patch serializes access with flock.
>
> With these patches it seems to work. I don't know if the flock patch is
> really necessary. Any help and comments appreciated.

> -       fd = open(devname, O_RDONLY | O_NONBLOCK);
> +       while (((fd = open(devname, O_RDONLY | O_NONBLOCK)) != 0) && (cnt >

What is open() != 0 doing? I guess you want < 0?

> +       strncpy(flock_path, "/sys/", MAX_PATH_LEN-1);
> +       strncat(flock_path, devpath, MAX_PATH_LEN-1);

Current scsi_id has no idea about sysfs anymore. I guess, we should
try if the retry patch is enough.

Kay

Re: scsi_id - EBUSY and serialization

[Harald Hoyer]

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

Kay Sievers wrote:
> On Wed, Jun 11, 2008 at 11:55 AM, Harald Hoyer <harald@redhat.com> wrote:
>> We have a customer, who misses some symlinks for some of his scsi devices.
>> Because of limited access and debugging facilities, I could only guess a
>> fix.
>>
>> The first patch retries opening the scsi device, if it is EBUSY.
>> The second patch serializes access with flock.
>>
>> With these patches it seems to work. I don't know if the flock patch is
>> really necessary. Any help and comments appreciated.
> 
>> -       fd = open(devname, O_RDONLY | O_NONBLOCK);
>> +       while (((fd = open(devname, O_RDONLY | O_NONBLOCK)) != 0) && (cnt >
> 
> What is open() != 0 doing? I guess you want < 0?

errr... yes :)

> 
>> +       strncpy(flock_path, "/sys/", MAX_PATH_LEN-1);
>> +       strncat(flock_path, devpath, MAX_PATH_LEN-1);
> 
> Current scsi_id has no idea about sysfs anymore. I guess, we should
> try if the retry patch is enough.
> 
> Kay

if not, we could flock s.th. like  "/dev/.udev/tmp_" + major + minor

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/x-pkcs7-signature, Size: 3636 bytes --]

Re: scsi_id - EBUSY and serialization

[Kay Sievers]

On Wed, Jun 11, 2008 at 9:35 PM, Harald Hoyer <harald@redhat.com> wrote:
> Kay Sievers wrote:
>>
>> On Wed, Jun 11, 2008 at 11:55 AM, Harald Hoyer <harald@redhat.com> wrote:
>>>
>>> We have a customer, who misses some symlinks for some of his scsi
>>> devices.
>>> Because of limited access and debugging facilities, I could only guess a
>>> fix.
>>>
>>> The first patch retries opening the scsi device, if it is EBUSY.
>>> The second patch serializes access with flock.
>>>
>>> With these patches it seems to work. I don't know if the flock patch is
>>> really necessary. Any help and comments appreciated.
>>
>>> -       fd = open(devname, O_RDONLY | O_NONBLOCK);
>>> +       while (((fd = open(devname, O_RDONLY | O_NONBLOCK)) != 0) && (cnt
>>> >
>>
>> What is open() != 0 doing? I guess you want < 0?
>
> errr... yes :)

Applied.

Thanks,
Kay