Skip to contentSkip to conversation

Alter the scsi_add_device() API to conform to what users expect

PATCH In review James Bottomley 2 participants 2 messages Sep 10, 2005 SCSI

lore
Share & tools
lore

James Bottomley

author
#0 lore raw

PATCH 1/1 Alter the scsi_add_device() API to conform to what users expect

The original API returned either an ERR_PTR() or a refcounted sdev. Unfortunately, if it's successful, you need to do a scsi_device_put() on the sdev otherwise the refcounting is wrong.

Everyone seems to expect that scsi_add_device() should be callable without doing the ref put, so alter the API so it is (we still have __scsi_add_device with the original behaviour).

The only actual caller that needs altering is the one in firewire ... not because it gets this right, but because it acts on the error if one is returned.

James

drivers/ieee1394/sbp2.c +4 -4
@@ -790,7 +790,7 @@ static void sbp2_host_reset(struct hpsb_
 static int sbp2_start_device(struct scsi_id_instance_data *scsi_id)
 {
 	struct sbp2scsi_host_info *hi = scsi_id->hi;
-	struct scsi_device *sdev;
+	int error;
 
 	SBP2_DEBUG("sbp2_start_device");
 

@@ -939,10 +939,10 @@ alloc_fail:
 	sbp2_max_speed_and_size(scsi_id);
 
 	/* Add this device to the scsi layer now */
-	sdev = scsi_add_device(scsi_id->scsi_host, 0, scsi_id->ud->id, 0);
-	if (IS_ERR(sdev)) {
+	error = scsi_add_device(scsi_id->scsi_host, 0, scsi_id->ud->id, 0);
+	if (error) {
 		SBP2_ERR("scsi_add_device failed");
-		return PTR_ERR(sdev);
+		return error;
 	}
 
 	return 0;
drivers/scsi/scsi_scan.c +13 -0
@@ -1264,6 +1264,19 @@ struct scsi_device *__scsi_add_device(st
 }
 EXPORT_SYMBOL(__scsi_add_device);
 
+int scsi_add_device(struct Scsi_Host *host, uint channel,
+		    uint target, uint lun)
+{
+	struct scsi_device *sdev = 
+		__scsi_add_device(host, channel, target, lun, NULL);
+	if (IS_ERR(sdev))
+		return PTR_ERR(sdev);
+
+	scsi_device_put(sdev);
+	return 0;
+}
+EXPORT_SYMBOL(scsi_add_device);
+
 void scsi_rescan_device(struct device *dev)
 {
 	struct scsi_driver *drv;
include/scsi/scsi_device.h +2 -2
@@ -178,8 +178,8 @@ static inline struct scsi_target *scsi_t
 
 extern struct scsi_device *__scsi_add_device(struct Scsi_Host *,
 		uint, uint, uint, void *hostdata);
-#define scsi_add_device(host, channel, target, lun) \
-	__scsi_add_device(host, channel, target, lun, NULL)
+extern int scsi_add_device(struct Scsi_Host *host, uint channel,
+			   uint target, uint lun);
 extern void scsi_remove_device(struct scsi_device *);
 extern int scsi_device_cancel(struct scsi_device *, int);
#1 lore raw

James Bottomley wrote:
...

drivers/ieee1394/sbp2.c static void sbp2_host_reset(struct hpsb_ · line 793

Quoted 2 lines
drivers/ieee1394/sbp2.c +1 -1
-	struct scsi_device *sdev;
+	int error;

...

Side note: The most recent sbp2 at svn.linux1394.org (which is not in Linus' tree yet and probably also not in -mm) needs a different approach because the sdev is required in sbp2's .remove hook later. I will submit an according update to linux1394-devel when the time is rife.

Signature · 3 lines
Stefan Richter
-=====-=-=-= =--= -=-=-
http://arcgraph.de/sr/