Signed-off-by: Jeff Garzik <[email protected]>
---
drivers/scsi/qla2xxx/qla_attr.c | 52 +++++++++++++++++++++++++++++++---------
drivers/scsi/qla2xxx/qla_gbl.h | 4 ---
drivers/scsi/qla2xxx/qla_os.c | 7 ++++-
diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index ee75a71..54e4a60 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -379,22 +379,52 @@ static struct bin_attribute sysfs_sfp_at
.read = qla2x00_sysfs_read_sfp,
};
-void
+int
qla2x00_alloc_sysfs_attr(scsi_qla_host_t *ha)
{
struct Scsi_Host *host = ha->host;
-
- sysfs_create_bin_file(&host->shost_gendev.kobj, &sysfs_fw_dump_attr);
- sysfs_create_bin_file(&host->shost_gendev.kobj, &sysfs_nvram_attr);
- sysfs_create_bin_file(&host->shost_gendev.kobj, &sysfs_optrom_attr);
- sysfs_create_bin_file(&host->shost_gendev.kobj,
- &sysfs_optrom_ctl_attr);
+ int rc;
+
+ rc = sysfs_create_bin_file(&host->shost_gendev.kobj,
+ &sysfs_fw_dump_attr);
+ if (rc) goto err;
+ rc = sysfs_create_bin_file(&host->shost_gendev.kobj,
+ &sysfs_nvram_attr);
+ if (rc) goto err_fwd;
+ rc = sysfs_create_bin_file(&host->shost_gendev.kobj,
+ &sysfs_optrom_attr);
+ if (rc) goto err_nvr;
+ rc = sysfs_create_bin_file(&host->shost_gendev.kobj,
+ &sysfs_optrom_ctl_attr);
+ if (rc) goto err_optrom;
if (IS_QLA24XX(ha) || IS_QLA54XX(ha)) {
- sysfs_create_bin_file(&host->shost_gendev.kobj,
- &sysfs_vpd_attr);
- sysfs_create_bin_file(&host->shost_gendev.kobj,
- &sysfs_sfp_attr);
+ rc = sysfs_create_bin_file(&host->shost_gendev.kobj,
+ &sysfs_vpd_attr);
+ if (rc) goto err_opt_ctl;
+ rc = sysfs_create_bin_file(&host->shost_gendev.kobj,
+ &sysfs_sfp_attr);
+ if (rc) goto err_vpd;
}
+
+ return 0;
+
+err_vpd:
+ sysfs_remove_bin_file(&host->shost_gendev.kobj,
+ &sysfs_vpd_attr);
+err_opt_ctl:
+ sysfs_remove_bin_file(&host->shost_gendev.kobj,
+ &sysfs_optrom_ctl_attr);
+err_optrom:
+ sysfs_remove_bin_file(&host->shost_gendev.kobj,
+ &sysfs_optrom_attr);
+err_nvr:
+ sysfs_remove_bin_file(&host->shost_gendev.kobj,
+ &sysfs_nvram_attr);
+err_fwd:
+ sysfs_remove_bin_file(&host->shost_gendev.kobj,
+ &sysfs_fw_dump_attr);
+err:
+ return rc;
}
void
diff --git a/drivers/scsi/qla2xxx/qla_gbl.h b/drivers/scsi/qla2xxx/qla_gbl.h
index 7da6983..60ade19 100644
--- a/drivers/scsi/qla2xxx/qla_gbl.h
+++ b/drivers/scsi/qla2xxx/qla_gbl.h
@@ -296,9 +296,7 @@ struct class_device_attribute;
extern struct class_device_attribute *qla2x00_host_attrs[];
struct fc_function_template;
extern struct fc_function_template qla2xxx_transport_functions;
-extern void qla2x00_alloc_sysfs_attr(scsi_qla_host_t *);
+extern int qla2x00_alloc_sysfs_attr(scsi_qla_host_t *);
extern void qla2x00_free_sysfs_attr(scsi_qla_host_t *);
extern void qla2x00_init_host_attr(scsi_qla_host_t *);
-extern void qla2x00_alloc_sysfs_attr(scsi_qla_host_t *);
-extern void qla2x00_free_sysfs_attr(scsi_qla_host_t *);
#endif /* _QLA_GBL_H */
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 3f20d76..d70162b 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -1638,7 +1638,9 @@ qla2x00_probe_one(struct pci_dev *pdev,
if (ret)
goto probe_failed;
- qla2x00_alloc_sysfs_attr(ha);
+ ret = qla2x00_alloc_sysfs_attr(ha);
+ if (ret)
+ goto probe_failed_rmhost;
qla2x00_init_host_attr(ha);
@@ -1658,6 +1660,9 @@ qla2x00_probe_one(struct pci_dev *pdev,
return 0;
+probe_failed_rmhost:
+ scsi_remove_host(host);
+
probe_failed:
qla2x00_free_device(ha);
On Wed, 11 Oct 2006, Jeff Garzik wrote:
> Signed-off-by: Jeff Garzik <[email protected]>
>
> ---
>
> drivers/scsi/qla2xxx/qla_attr.c | 52 +++++++++++++++++++++++++++++++---------
> drivers/scsi/qla2xxx/qla_gbl.h | 4 ---
> drivers/scsi/qla2xxx/qla_os.c | 7 ++++-
>
> diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
> index ee75a71..54e4a60 100644
> --- a/drivers/scsi/qla2xxx/qla_attr.c
> +++ b/drivers/scsi/qla2xxx/qla_attr.c
> @@ -379,22 +379,52 @@ static struct bin_attribute sysfs_sfp_at
> .read = qla2x00_sysfs_read_sfp,
> };
>
> -void
> +int
> qla2x00_alloc_sysfs_attr(scsi_qla_host_t *ha)
> {
> struct Scsi_Host *host = ha->host;
> -
> - sysfs_create_bin_file(&host->shost_gendev.kobj, &sysfs_fw_dump_attr);
> - sysfs_create_bin_file(&host->shost_gendev.kobj, &sysfs_nvram_attr);
> - sysfs_create_bin_file(&host->shost_gendev.kobj, &sysfs_optrom_attr);
> - sysfs_create_bin_file(&host->shost_gendev.kobj,
> - &sysfs_optrom_ctl_attr);
> + int rc;
> +
> + rc = sysfs_create_bin_file(&host->shost_gendev.kobj,
> + &sysfs_fw_dump_attr);
> + if (rc) goto err;
NACK, please don't do this. SYSFS entries, albiet important, aren't
necessarilly critical to a functioning driver. I'd rather the driver
not error out.
Here's what I had stewing to address the must_check directives and
qla2xxx:
---
qla2xxx: Check return value of sysfs_create_bin_file() usage.
Signed-off-by: Andrew Vasquez <[email protected]>
---
drivers/scsi/qla2xxx/qla_attr.c | 51 ++++++++++++++++++++++++--------------
1 files changed, 32 insertions(+), 19 deletions(-)
diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index ee75a71..285c8e8 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -379,21 +379,37 @@ static struct bin_attribute sysfs_sfp_at
.read = qla2x00_sysfs_read_sfp,
};
+static struct sysfs_entry {
+ char *name;
+ struct bin_attribute *attr;
+ int is4GBp_only;
+} bin_file_entries[] = {
+ { "fw_dump", &sysfs_fw_dump_attr, },
+ { "nvram", &sysfs_nvram_attr, },
+ { "optrom", &sysfs_optrom_attr, },
+ { "optrom_ctl", &sysfs_optrom_ctl_attr, },
+ { "vpd", &sysfs_vpd_attr, 1 },
+ { "sfp", &sysfs_sfp_attr, 1 },
+ { 0 },
+};
+
void
qla2x00_alloc_sysfs_attr(scsi_qla_host_t *ha)
{
struct Scsi_Host *host = ha->host;
+ struct sysfs_entry *iter;
+ int ret;
- sysfs_create_bin_file(&host->shost_gendev.kobj, &sysfs_fw_dump_attr);
- sysfs_create_bin_file(&host->shost_gendev.kobj, &sysfs_nvram_attr);
- sysfs_create_bin_file(&host->shost_gendev.kobj, &sysfs_optrom_attr);
- sysfs_create_bin_file(&host->shost_gendev.kobj,
- &sysfs_optrom_ctl_attr);
- if (IS_QLA24XX(ha) || IS_QLA54XX(ha)) {
- sysfs_create_bin_file(&host->shost_gendev.kobj,
- &sysfs_vpd_attr);
- sysfs_create_bin_file(&host->shost_gendev.kobj,
- &sysfs_sfp_attr);
+ for (iter = bin_file_entries; iter->name; iter++) {
+ if (iter->is4GBp_only && (!IS_QLA24XX(ha) && !IS_QLA54XX(ha)))
+ continue;
+
+ ret = sysfs_create_bin_file(&host->shost_gendev.kobj,
+ iter->attr);
+ if (ret)
+ qla_printk(KERN_INFO, ha,
+ "Unable to create sysfs %s binary attribute "
+ "(%d).\n", iter->name, ret);
}
}
@@ -401,17 +417,14 @@ void
qla2x00_free_sysfs_attr(scsi_qla_host_t *ha)
{
struct Scsi_Host *host = ha->host;
+ struct sysfs_entry *iter;
+
+ for (iter = bin_file_entries; iter->name; iter++) {
+ if (iter->is4GBp_only && (!IS_QLA24XX(ha) && !IS_QLA54XX(ha)))
+ continue;
- sysfs_remove_bin_file(&host->shost_gendev.kobj, &sysfs_fw_dump_attr);
- sysfs_remove_bin_file(&host->shost_gendev.kobj, &sysfs_nvram_attr);
- sysfs_remove_bin_file(&host->shost_gendev.kobj, &sysfs_optrom_attr);
- sysfs_remove_bin_file(&host->shost_gendev.kobj,
- &sysfs_optrom_ctl_attr);
- if (IS_QLA24XX(ha) || IS_QLA54XX(ha)) {
- sysfs_remove_bin_file(&host->shost_gendev.kobj,
- &sysfs_vpd_attr);
sysfs_remove_bin_file(&host->shost_gendev.kobj,
- &sysfs_sfp_attr);
+ iter->attr);
}
if (ha->beacon_blink_led == 1)
--
1.4.3.rc2.g0503
Andrew Vasquez wrote:
> NACK, please don't do this. SYSFS entries, albiet important, aren't
> necessarilly critical to a functioning driver. I'd rather the driver
> not error out.
As discussed before, the only errors thrown are either ENOMEM or EFAULT,
both of which are quite serious.
> Here's what I had stewing to address the must_check directives and
> qla2xxx:
If you're gonna change it that much, might as well use attribute groups.
Jeff
On Thu, 12 Oct 2006, Jeff Garzik wrote:
> Andrew Vasquez wrote:
> >NACK, please don't do this. SYSFS entries, albiet important, aren't
> >necessarilly critical to a functioning driver. I'd rather the driver
> >not error out.
>
> As discussed before, the only errors thrown are either ENOMEM or EFAULT,
> both of which are quite serious.
Absolutely. But, given the relatively late-stage initialization of
these attributes (it's the last thing that gets done before probe()
completes)-- the driver has already allocated (successfully) memory,
intialized hardware and (possibly) presented storage, the complete
unwinding, seems over-kill.
> >Here's what I had stewing to address the must_check directives and
> >qla2xxx:
>
> If you're gonna change it that much, might as well use attribute groups.
sysfs_create_group() and friends don't appear to support
binary attributes.