[PATCH] [SCSI] bnx2fc: move the dereference below the NULL test
From: Wei Yongjun
The dereference should be moved below the NULL test.
spatch with a semantic match is used to found this.
(http://coccinelle.lip6.fr/)
Signed-off-by: Wei Yongjun
---
drivers/scsi/bnx2fc/bnx2fc_io.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/bnx2fc/bnx2fc_io.c b/drivers/scsi/bnx2fc/bnx2fc_io.c
index 73f231c..298a0fe 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_io.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_io.c
@@ -686,7 +686,7 @@ static int bnx2fc_initiate_tmf(struct scsi_cmnd *sc_cmd, u8
tm_flags)
{
struct fc_lport *lport;
struct fc_rport *rport = starget_to_rport(scsi_target(sc_cmd->device));
- struct fc_rport_libfc_priv *rp = rport->dd_data;
+ struct fc_rport_libfc_priv *rp;
struct fcoe_port *port;
struct bnx2fc_interface *interface;
struct bnx2fc_rport *tgt;
@@ -722,6 +722,8 @@ static int bnx2fc_initiate_tmf(struct scsi_cmnd *sc_cmd, u8
tm_flags)
rc = FAILED;
goto tmf_err;
}
+
+ rp = rport->dd_data;
/* rport and tgt are allocated together, so tgt should be non-NULL */
tgt = (struct bnx2fc_rport *)&rp[1];
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix a use-after-free triggered by device removal
On 09/07/12 01:20, Tejun Heo wrote: > I think Mike is wondering whether your patch in isolation is enough or > we also need to have DEAD check there too. The proposed patch can't > handle the case where q->request_fn() is invoked after drain is > complete. I'm not really sure whether that can happen tho. Hello Tejun, I'm not sure it would be a good idea to add a blk_queue_dead() check in any of the __blk_run_queue() variants since blk_drain_queue() can invoke __blk_run_queue() to drain the queue. Also, as far as I can see the functions that can insert a request into the queue (blk_insert_cloned_request(), queue_unplugged(), blk_execute_rq_nowait()) all check whether the queue is dead before inserting a request. That should be sufficient to prevent that new requests are queued after QUEUE_FLAG_DEAD has been set. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] target: move the dereference below the NULL test
From: Wei Yongjun
The dereference should be moved below the NULL test.
spatch with a semantic match is used to found this.
(http://coccinelle.lip6.fr/)
Signed-off-by: Wei Yongjun
---
drivers/target/target_core_pr.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index 1e94650..7e2e9be 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -197,10 +197,10 @@ int target_scsi2_reservation_release(struct se_cmd *cmd)
{
struct se_device *dev = cmd->se_dev;
struct se_session *sess = cmd->se_sess;
- struct se_portal_group *tpg = sess->se_tpg;
+ struct se_portal_group *tpg;
int ret = 0, rc;
- if (!sess || !tpg)
+ if (!sess || !sess->se_tpg)
goto out;
rc = target_check_scsi2_reservation_conflict(cmd);
if (rc == 1)
@@ -228,6 +228,7 @@ int target_scsi2_reservation_release(struct se_cmd *cmd)
dev->dev_res_bin_isid = 0;
dev->dev_flags &= ~DF_SPC2_RESERVATIONS_WITH_ISID;
}
+ tpg = sess->se_tpg;
pr_debug("SCSI-2 Released reservation for %s LUN: %u ->"
" MAPPED LUN: %u for %s\n", tpg->se_tpg_tfo->get_fabric_name(),
cmd->se_lun->unpacked_lun, cmd->se_deve->mapped_lun,
@@ -245,7 +246,7 @@ int target_scsi2_reservation_reserve(struct se_cmd *cmd)
{
struct se_device *dev = cmd->se_dev;
struct se_session *sess = cmd->se_sess;
- struct se_portal_group *tpg = sess->se_tpg;
+ struct se_portal_group *tpg;
int ret = 0, rc;
if ((cmd->t_task_cdb[1] & 0x01) &&
@@ -260,7 +261,7 @@ int target_scsi2_reservation_reserve(struct se_cmd *cmd)
* This is currently the case for target_core_mod passthrough struct
se_cmd
* ops
*/
- if (!sess || !tpg)
+ if (!sess || !sess->se_tpg)
goto out;
rc = target_check_scsi2_reservation_conflict(cmd);
if (rc == 1)
@@ -272,6 +273,7 @@ int target_scsi2_reservation_reserve(struct se_cmd *cmd)
}
ret = 0;
+ tpg = sess->se_tpg;
spin_lock(&dev->dev_reservation_lock);
if (dev->dev_reserved_node_acl &&
(dev->dev_reserved_node_acl != sess->se_node_acl)) {
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] tcm_fc: move the dereference below the NULL test
From: Wei Yongjun
The dereference should be moved below the NULL test.
spatch with a semantic match is used to found this.
(http://coccinelle.lip6.fr/)
Signed-off-by: Wei Yongjun
---
drivers/target/tcm_fc/tfc_io.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/target/tcm_fc/tfc_io.c b/drivers/target/tcm_fc/tfc_io.c
index ad36ede1..1b367a9 100644
--- a/drivers/target/tcm_fc/tfc_io.c
+++ b/drivers/target/tcm_fc/tfc_io.c
@@ -328,11 +328,12 @@ drop:
*/
void ft_invl_hw_context(struct ft_cmd *cmd)
{
- struct fc_seq *seq = cmd->seq;
+ struct fc_seq *seq;
struct fc_exch *ep = NULL;
struct fc_lport *lport = NULL;
BUG_ON(!cmd);
+ seq = cmd->seq;
/* Cleanup the DDP context in HW if DDP was setup */
if (cmd->was_ddp_setup && seq) {
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 7/7] libata: acpi: respect may_power_off flag
On 09/04/2012 10:24 AM, Aaron Lu wrote:
From: Aaron Lu
If user does not want the device being powered off when runtime
suspended by setting may_power_off flag to 0, we will not choose D3 cold
ACPI D-State for it.
Signed-off-by: Aaron Lu
---
drivers/ata/libata-acpi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 6c8f89c..774180d 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -869,7 +869,9 @@ void ata_acpi_set_state(struct ata_port *ap, pm_message_t
state)
if (state.event != PM_EVENT_ON) {
acpi_state = acpi_pm_device_sleep_state(
- &dev->sdev->sdev_gendev, NULL, ACPI_STATE_D3);
+ &dev->sdev->sdev_gendev, NULL,
+ dev->sdev->may_power_off ?
+ ACPI_STATE_D3_COLD : ACPI_STATE_D3_HOT);
if (acpi_state > 0)
acpi_bus_set_power(handle, acpi_state);
Acked-by: Jeff Garzik
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 4/7] libata: acpi: set can_power_off for both ODD and HD
On 09/04/2012 10:24 AM, Aaron Lu wrote: From: Aaron Lu Hard disk may also be runtime powered off, so set can_power_off flag for it too if condition satisfies. Signed-off-by: Aaron Lu Acked-by: Jeff Garzik -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix a use-after-free triggered by device removal
Hello, Bart, Mike. On Thu, Sep 06, 2012 at 08:52:09PM +0200, Bart Van Assche wrote: > >> The purpose of this patch is indeed to make *blk_run_queue() calls from > >> the block layer safe. There are several direct or indirect > >> *blk_run_queue() calls in the block layer where a reference on the queue > >> is held but not on the sdev, e.g. in the md, dm and bsg drivers. Yeah, while writing blk_drain_queue(), I was thinking only about requests. I didn't consider control reaching into driver. > > Is there a race still? If some blk code is calling blk_run_queue > > (waiting on the queue lock) but no IO is queued, > > blk_drain_queue/blk_cleanup_queue could complete since the drain > > counters are zero. Then blk_run_queue could grab the queue lock and call > > the request_fn on a freed scsi_device (sdev pointed to by q->queuedata > > would be freed so scsi_reuqest_fn would be freed). > > > > Do we need a check in __blk_run_queue for QUEUE_FLAG_DEAD (if dead then > > fail IO?), or do we need a check in scsi_request_fn for this. A dead > > queue check or maybe null q->queuedata in > > scsi_device_dev_release_usercontext (do this with the queue lock held), > > then check for null q->queuedata in scsi_request_fn? > > Yet another scenario is that scsi_remove_host() gets invoked and > finishes after scsi_request_fn() has unlocked the queue and before it > locks the queue again. That's a scenario that can't be handled by adding > more checks at the start of __blk_run_queue() or scsi_request_fn(). I think Mike is wondering whether your patch in isolation is enough or we also need to have DEAD check there too. The proposed patch can't handle the case where q->request_fn() is invoked after drain is complete. I'm not really sure whether that can happen tho. Other than that, yeah, I think it's a real problem and we definitely want to remove get/put_device() from scsi_request_fn(). Refcnting oneself is often wrong (as shown here too) and generally just sad. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix a use-after-free triggered by device removal
On 09/06/12 20:14, Mike Christie wrote: > On 09/06/2012 12:58 PM, Bart Van Assche wrote: >> _suOn 09/06/12 18:27, Michael Christie wrote: >>> On Sep 3, 2012, at 9:12 AM, Bart Van Assche wrote: If the put_device() call in scsi_request_fn() drops the sdev refcount to zero then the spin_lock() call after the put_device() call triggers a use-after-free. Avoid that by making sure that blk_cleanup_queue() can only finish after all active scsi_request_fn() calls have returned. >>> >>> If we have this patch http://marc.info/?l=linux-scsi&m=134453905402413&w=2 >>> it seems we have all the scsi layer callers of the request_fn/ >>> *blk_run_queue holding a reference to the device when they make the call. >>> Right, or are there some other places missing? >>> >>> What are the other places we can call the request_fn without already >>> holding a reference to the device? Is it the block layer? Is that why we >>> need this patch? >> >> The purpose of this patch is indeed to make *blk_run_queue() calls from >> the block layer safe. There are several direct or indirect >> *blk_run_queue() calls in the block layer where a reference on the queue >> is held but not on the sdev, e.g. in the md, dm and bsg drivers. > > Is there a race still? If some blk code is calling blk_run_queue > (waiting on the queue lock) but no IO is queued, > blk_drain_queue/blk_cleanup_queue could complete since the drain > counters are zero. Then blk_run_queue could grab the queue lock and call > the request_fn on a freed scsi_device (sdev pointed to by q->queuedata > would be freed so scsi_reuqest_fn would be freed). > > Do we need a check in __blk_run_queue for QUEUE_FLAG_DEAD (if dead then > fail IO?), or do we need a check in scsi_request_fn for this. A dead > queue check or maybe null q->queuedata in > scsi_device_dev_release_usercontext (do this with the queue lock held), > then check for null q->queuedata in scsi_request_fn? Yet another scenario is that scsi_remove_host() gets invoked and finishes after scsi_request_fn() has unlocked the queue and before it locks the queue again. That's a scenario that can't be handled by adding more checks at the start of __blk_run_queue() or scsi_request_fn(). Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/7] scsi: sr: support runtime pm for ODD
On Thu, 6 Sep 2012, Oliver Neukum wrote: > > > > But in the long run that wouldn't be a good solution. What I'd really > > > > like is a way to do the status polling without having it reset the > > > > idle timer. > > > > > > > > Oliver, what do you think? Would that be a good solution? > > > > > > Well, we could introduce a flag into the requests for the polls. > > > But best would be to simply declare a device immediately idle > > > as soon as we learn that it has no medium. No special casing > > > would be needed. > > > > We could do that, but what about idle drives that do have media? > > Then we do have a problem. To handle this optimally we'd have to make > a difference between the first time a new medium is noticed and later > polls. That's right. It shouldn't be too difficult to accomplish. One case to watch out for is a ZPODD with no media and an open door. We should put the drive into runtime suspend immediately, but continue polling and leave the power on. The runtime suspend after each poll will to see if the door is shut; when it is then polling can be turned off and power removed. This leads to a question: How should the sr device tell its parent whether or not to turn off the power? Aaron's current patches simply rely on the device being runtime suspended, but that won't work with this scheme. Do we need a "ready_to_power_down" flag? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix a use-after-free triggered by device removal
On 09/06/2012 12:58 PM, Bart Van Assche wrote: > _suOn 09/06/12 18:27, Michael Christie wrote: >> On Sep 3, 2012, at 9:12 AM, Bart Van Assche wrote: >>> If the put_device() call in scsi_request_fn() drops the sdev refcount >>> to zero then the spin_lock() call after the put_device() call triggers >>> a use-after-free. Avoid that by making sure that blk_cleanup_queue() >>> can only finish after all active scsi_request_fn() calls have returned. >> >> If we have this patch http://marc.info/?l=linux-scsi&m=134453905402413&w=2 >> it seems we have all the scsi layer callers of the request_fn/ >> *blk_run_queue holding a reference to the device when they make the call. >> Right, or are there some other places missing? >> >> What are the other places we can call the request_fn without already >> holding a reference to the device? Is it the block layer? Is that why we >> need this patch? > > Hello Mike, > > The purpose of this patch is indeed to make *blk_run_queue() calls from > the block layer safe. There are several direct or indirect > *blk_run_queue() calls in the block layer where a reference on the queue > is held but not on the sdev, e.g. in the md, dm and bsg drivers. > Is there a race still? If some blk code is calling blk_run_queue (waiting on the queue lock) but no IO is queued, blk_drain_queue/blk_cleanup_queue could complete since the drain counters are zero. Then blk_run_queue could grab the queue lock and call the request_fn on a freed scsi_device (sdev pointed to by q->queuedata would be freed so scsi_reuqest_fn would be freed). Do we need a check in __blk_run_queue for QUEUE_FLAG_DEAD (if dead then fail IO?), or do we need a check in scsi_request_fn for this. A dead queue check or maybe null q->queuedata in scsi_device_dev_release_usercontext (do this with the queue lock held), then check for null q->queuedata in scsi_request_fn? -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/7] scsi: sr: support runtime pm for ODD
On Thursday 06 September 2012 13:08:18 Alan Stern wrote: > On Thu, 6 Sep 2012, Oliver Neukum wrote: > > > On Thursday 06 September 2012 11:06:49 Alan Stern wrote: > > > On Thu, 6 Sep 2012, Aaron Lu wrote: > > > > > > > > That's why we have an autosuspend delay. Although for some reason > > > > > the > > > > > SCSI subsystem doesn't use it currently... We need to add a call to > > > > > pm_runtime_use_autosuspend() in scsi_sysfs_add_sdev(). Likewise, the > > > > > pm_schedule_suspend() call in scsi_runtime_idle() should be changed > > > > > to > > > > > pm_runtime_autosuspend(). And there should be calls to > > > > > pm_runtime_set_autosuspend_delay() in the sd and sr drivers. > > > > > > > > I tried to use autosuspend when preparing the patch, but the fact that > > > > the devices will be polled every 2 seconds make it impossible to enter > > > > suspend state if the autosuspend delay is larger than that. > > > > > > You can always increase the polling interval. > > > > > > But in the long run that wouldn't be a good solution. What I'd really > > > like is a way to do the status polling without having it reset the > > > idle timer. > > > > > > Oliver, what do you think? Would that be a good solution? > > > > Well, we could introduce a flag into the requests for the polls. > > But best would be to simply declare a device immediately idle > > as soon as we learn that it has no medium. No special casing > > would be needed. > > We could do that, but what about idle drives that do have media? Then we do have a problem. To handle this optimally we'd have to make a difference between the first time a new medium is noticed and later polls. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix a use-after-free triggered by device removal
_suOn 09/06/12 18:27, Michael Christie wrote: > On Sep 3, 2012, at 9:12 AM, Bart Van Assche wrote: >> If the put_device() call in scsi_request_fn() drops the sdev refcount >> to zero then the spin_lock() call after the put_device() call triggers >> a use-after-free. Avoid that by making sure that blk_cleanup_queue() >> can only finish after all active scsi_request_fn() calls have returned. > > If we have this patch http://marc.info/?l=linux-scsi&m=134453905402413&w=2 > it seems we have all the scsi layer callers of the request_fn/ > *blk_run_queue holding a reference to the device when they make the call. > Right, or are there some other places missing? > > What are the other places we can call the request_fn without already > holding a reference to the device? Is it the block layer? Is that why we > need this patch? Hello Mike, The purpose of this patch is indeed to make *blk_run_queue() calls from the block layer safe. There are several direct or indirect *blk_run_queue() calls in the block layer where a reference on the queue is held but not on the sdev, e.g. in the md, dm and bsg drivers. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] isci: Allow SSP tasks into the task management path.
This commit fixes a driver bug for SSP tasks that require task management
in the target after they complete in the SCU hardware. The problem was
manifested in the function "isci_task_abort_task", which tests
to see if the sas_task.lldd_task is non-NULL before allowing task
management; this bug would always NULL lldd_task in the SCU I/O completion
path even if target management was required, which would prevent
task / target manangement from happening.
Note that in the case of SATA/STP targets, error recovery is provided by
the libata error handler which is why SATA/STP device recovery worked
correctly even though SSP handling did not.
Signed-off-by: Jeff Skirvin
---
Resubmitting to include James...
drivers/scsi/isci/host.c | 10 ++
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/isci/host.c b/drivers/scsi/isci/host.c
index 45385f5..446fdd8 100644
--- a/drivers/scsi/isci/host.c
+++ b/drivers/scsi/isci/host.c
@@ -1079,7 +1079,6 @@ static void sci_controller_completion_handler(struct
isci_host *ihost)
void ireq_done(struct isci_host *ihost, struct isci_request *ireq, struct
sas_task *task)
{
- task->lldd_task = NULL;
if (!test_bit(IREQ_ABORT_PATH_ACTIVE, &ireq->flags) &&
!(task->task_state_flags & SAS_TASK_STATE_ABORTED)) {
if (test_bit(IREQ_COMPLETE_IN_TARGET, &ireq->flags)) {
@@ -1087,16 +1086,19 @@ void ireq_done(struct isci_host *ihost, struct
isci_request *ireq, struct sas_ta
dev_dbg(&ihost->pdev->dev,
"%s: Normal - ireq/task = %p/%p\n",
__func__, ireq, task);
-
+ task->lldd_task = NULL;
task->task_done(task);
} else {
dev_dbg(&ihost->pdev->dev,
"%s: Error - ireq/task = %p/%p\n",
__func__, ireq, task);
-
+ if (sas_protocol_ata(task->task_proto))
+ task->lldd_task = NULL;
sas_task_abort(task);
}
- }
+ } else
+ task->lldd_task = NULL;
+
if (test_and_clear_bit(IREQ_ABORT_PATH_ACTIVE, &ireq->flags))
wake_up_all(&ihost->eventq);
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/7] scsi: sr: support runtime pm for ODD
On Thu, 6 Sep 2012, Oliver Neukum wrote: > On Thursday 06 September 2012 11:06:49 Alan Stern wrote: > > On Thu, 6 Sep 2012, Aaron Lu wrote: > > > > > > That's why we have an autosuspend delay. Although for some reason the > > > > SCSI subsystem doesn't use it currently... We need to add a call to > > > > pm_runtime_use_autosuspend() in scsi_sysfs_add_sdev(). Likewise, the > > > > pm_schedule_suspend() call in scsi_runtime_idle() should be changed to > > > > pm_runtime_autosuspend(). And there should be calls to > > > > pm_runtime_set_autosuspend_delay() in the sd and sr drivers. > > > > > > I tried to use autosuspend when preparing the patch, but the fact that > > > the devices will be polled every 2 seconds make it impossible to enter > > > suspend state if the autosuspend delay is larger than that. > > > > You can always increase the polling interval. > > > > But in the long run that wouldn't be a good solution. What I'd really > > like is a way to do the status polling without having it reset the > > idle timer. > > > > Oliver, what do you think? Would that be a good solution? > > Well, we could introduce a flag into the requests for the polls. > But best would be to simply declare a device immediately idle > as soon as we learn that it has no medium. No special casing > would be needed. We could do that, but what about idle drives that do have media? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix a use-after-free triggered by device removal
On Sep 3, 2012, at 9:12 AM, Bart Van Assche wrote: > If the put_device() call in scsi_request_fn() drops the sdev refcount > to zero then the spin_lock() call after the put_device() call triggers > a use-after-free. Avoid that by making sure that blk_cleanup_queue() > can only finish after all active scsi_request_fn() calls have returned. If we have this patch http://marc.info/?l=linux-scsi&m=134453905402413&w=2 it seems we have all the scsi layer callers of the request_fn/*blk_run_queue holding a reference to the device when they make the call. Right, or are there some other places missing? What are the other places we can call the request_fn without already holding a reference to the device? Is it the block layer? Is that why we need this patch?-- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/10] drivers/scsi/sym53c8xx_2/sym_fw.c: removes unnecessary semicolon
From: Peter Senna Tschudin removes unnecessary semicolon Found by Coccinelle: http://coccinelle.lip6.fr/ Signed-off-by: Peter Senna Tschudin --- drivers/scsi/sym53c8xx_2/sym_fw.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff -u -p a/drivers/scsi/sym53c8xx_2/sym_fw.c b/drivers/scsi/sym53c8xx_2/sym_fw.c --- a/drivers/scsi/sym53c8xx_2/sym_fw.c +++ b/drivers/scsi/sym53c8xx_2/sym_fw.c @@ -386,7 +386,7 @@ void sym_fw_bind_script(struct sym_hcb * sym_name(np), (int) (cur-start)); ++cur; continue; - }; + } /* * We use the bogus value 0xf00ff00f ;-) @@ -494,7 +494,7 @@ void sym_fw_bind_script(struct sym_hcb * default: relocs = 0; break; - }; + } /* * Scriptify:) the opcode. @@ -550,5 +550,5 @@ void sym_fw_bind_script(struct sym_hcb * *cur++ = cpu_to_scr(new); } - }; + } } -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/12] qla4xxx: Updates for scsi "misc" branch.
On Aug 22, 2012, at 6:54 AM, vikas.chaudh...@qlogic.com wrote: > Vikas Chaudhary (12): > qla4xxx: Update function name from 8xxx to 82xx > qla4xxx: Update structure and variable names > qla4xxx: Update func name from ql4_ to qla4_ > qla4xxx: Rename macros from 82XX to 8XXX > qla4xxx: Clean-up and optimize macros > qla4xxx: Added new functions in isp_ops > qla4xxx: Replace all !is_qla8022() with is_qla40XX() > qla4xxx: Set IDC version in correct way > qla4xxx: Added new function qla4_8xxx_get_minidump > qla4xxx: Added support for ISP83XX > qla4xxx: Update Copyright header > qla4xxx: Update driver version to 5.03.00-k0 Patches look ok overall. There are some minor coding style issues in patch 10. - do not need a "return" statement at the end of a function that does not return something - some people do not like a extra newlines like this: x = something; if (x == something else) but I think it would be ok to clean those up later since they are minor. Reviewed-by: Mike Christie -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [SCSI] libfcoe: use list_move instead of list_del/list_add
On 9/5/2012 9:36 PM, Wei Yongjun wrote: > From: Wei Yongjun > > Using list_move() instead of list_del() + list_add(). > > spatch with a semantic match is used to found this problem. > (http://coccinelle.lip6.fr/) > > Signed-off-by: Wei Yongjun Acked-by: Robert Love -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/7] scsi: sr: support runtime pm for ODD
On Thursday 06 September 2012 11:06:49 Alan Stern wrote: > On Thu, 6 Sep 2012, Aaron Lu wrote: > > > > That's why we have an autosuspend delay. Although for some reason the > > > SCSI subsystem doesn't use it currently... We need to add a call to > > > pm_runtime_use_autosuspend() in scsi_sysfs_add_sdev(). Likewise, the > > > pm_schedule_suspend() call in scsi_runtime_idle() should be changed to > > > pm_runtime_autosuspend(). And there should be calls to > > > pm_runtime_set_autosuspend_delay() in the sd and sr drivers. > > > > I tried to use autosuspend when preparing the patch, but the fact that > > the devices will be polled every 2 seconds make it impossible to enter > > suspend state if the autosuspend delay is larger than that. > > You can always increase the polling interval. > > But in the long run that wouldn't be a good solution. What I'd really > like is a way to do the status polling without having it reset the > idle timer. > > Oliver, what do you think? Would that be a good solution? Well, we could introduce a flag into the requests for the polls. But best would be to simply declare a device immediately idle as soon as we learn that it has no medium. No special casing would be needed. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/7] scsi: sr: support runtime pm for ODD
On Thu, 6 Sep 2012, Aaron Lu wrote: > > That's why we have an autosuspend delay. Although for some reason the > > SCSI subsystem doesn't use it currently... We need to add a call to > > pm_runtime_use_autosuspend() in scsi_sysfs_add_sdev(). Likewise, the > > pm_schedule_suspend() call in scsi_runtime_idle() should be changed to > > pm_runtime_autosuspend(). And there should be calls to > > pm_runtime_set_autosuspend_delay() in the sd and sr drivers. > > I tried to use autosuspend when preparing the patch, but the fact that > the devices will be polled every 2 seconds make it impossible to enter > suspend state if the autosuspend delay is larger than that. You can always increase the polling interval. But in the long run that wouldn't be a good solution. What I'd really like is a way to do the status polling without having it reset the idle timer. Oliver, what do you think? Would that be a good solution? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] [SCSI] scsi_debug: Add "removable" parameter
On 12-09-06 06:04 AM, Martin Pitt wrote: Add "removable" module parameter to set the "removable" attribute of any subsequently created debug block device. It is a writable driver option, so that you can switch between removable and "fixed" media block devices in between the add_host calls. This is useful for being able to test the different behaviour/required privileges in e. g. the udisks test suite. Signed-off-by: Martin Pitt Acked-By: David Zeuthen Acked-by: Douglas Gilbert -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Ping^3] Re: [PATCH] sg_io: allow UNMAP and WRITE SAME without CAP_SYS_RAWIO
On Thu, 6 Sep 2012, Paolo Bonzini wrote: > Date: Thu, 06 Sep 2012 14:36:53 +0200 > From: Paolo Bonzini > To: Ric Wheeler > Cc: ax...@kernel.dk, Mike Snitzer , > Alan Cox , > Martin K. Petersen , > linux-ker...@vger.kernel.org, linux-scsi@vger.kernel.org > Subject: Re: [Ping^3] Re: [PATCH] sg_io: allow UNMAP and WRITE SAME without > CAP_SYS_RAWIO > > Il 06/09/2012 14:08, Ric Wheeler ha scritto: > >> According to the standard, the translation layer can write a > >> user-provided pattern to every sector in the disk. It's an optional > >> feature and libata doesn't do that, but it is still possible. > > > > It is not possible today with our stack though, any patch that would > > change that would also need to be vetted. > > It is not possible with SATA disks, but native SCSI disks might well > interpret FORMAT UNIT destructively. > > >>> I don't see allowing anyone who can open the device to zero the data as > >>> better though :) > >> Note: anyone who can open it for writing! And they can just as well > >> issue WRITE, it just takes a little more effort than with WRITE SAME. :) > >> If you only have read access, you cannot issue WRITE or FORMAT UNIT, > >> and with this patch you will not be able to issue WRITE SAME. > > > > This just seems like an argument over whether or not capabilities make > > sense. In general, anything as destructive as a single CDB that can kill > > all of your data should be tightly controlled. > > In practice, a single write to the first MB of the disk is just as > destructive. For that you do not even need a SCSI command. > > > Pushing more code in the data path is not where we are going - we > > routinely need to disable IO scheduling for example when driving IO to > > high speed/low latency devices and are actively looking at how to tackle > > other performance bottlenecks in the stack. > > I am not talking about the regular data path, only of SG_IO. > > > I don't see a strong reason that our existing scheme (root or > > CAP_SYS_RAWIO access) prevents you from doing what you need to do. > > Here are three: > > - CAP_SYS_RAWIO partly bypasses DAC; you can issue destructive commands > even if you only opened the disk for reading. CAP_SYS_RAWIO also gives > access to _really_ destructive commands (WRITE BUFFER and PERSISTENT > RESERVE OUT for example). > > - CAP_SYS_RAWIO lets you send SCSI commands to partitions, and they will > gladly read/write the disk going outside the boundaries of the > partition. Changing this behavior was rejected upstream already. > > - CAP_SYS_RAWIO also gives access to I/O ports, mmap at address 0, and > too many other insecure things. > > All the above mean that: > > - any application using CAP_SYS_RAWIO would have to implement its own > whitelisting, even if just to duplicate what is done in the kernel; > > - exploiting a CAP_SYS_RAWIO process leads to root too easily, and it is > not possible to give the capability to anything that will run in a > hostile environment (in my case QEMU). So at fist I did not think this is such a good idea however there are several good points you've mentioned. CAP_SYS_RAWIO is indeed too big hammer for this and it is not secure to allow such application to possess such capability. Moreover WRITE_SAME indeed is almost the SAME as WRITE :), only easier and delegated to the storage itself. UNMAP or WRITE_WAME w/ unamp bit is a little bit trickier but thinking about it some more I do not see any real reason why the user with write permission should not be able to use this. Yes, it is not technically write and it has other consequences as well, but none of it seems to be exploitable more than simple write. Moreover looking at BLKDISCARD ioctl there is no such restrictions (obviously) but neither is with TRIM ata command. So with sata SSD you're allowed to use TRIM command if you have write permission to that device. So I guess having this consistent is good idea and considering the points above I think it is ok to allow WRITE_SAME and UNMAP without CAP_SYS_RAWIO. Thanks! -Lukas > > Paolo > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Ping^3] Re: [PATCH] sg_io: allow UNMAP and WRITE SAME without CAP_SYS_RAWIO
Il 06/09/2012 14:08, Ric Wheeler ha scritto: >> According to the standard, the translation layer can write a >> user-provided pattern to every sector in the disk. It's an optional >> feature and libata doesn't do that, but it is still possible. > > It is not possible today with our stack though, any patch that would > change that would also need to be vetted. It is not possible with SATA disks, but native SCSI disks might well interpret FORMAT UNIT destructively. >>> I don't see allowing anyone who can open the device to zero the data as >>> better though :) >> Note: anyone who can open it for writing! And they can just as well >> issue WRITE, it just takes a little more effort than with WRITE SAME. :) >> If you only have read access, you cannot issue WRITE or FORMAT UNIT, >> and with this patch you will not be able to issue WRITE SAME. > > This just seems like an argument over whether or not capabilities make > sense. In general, anything as destructive as a single CDB that can kill > all of your data should be tightly controlled. In practice, a single write to the first MB of the disk is just as destructive. For that you do not even need a SCSI command. > Pushing more code in the data path is not where we are going - we > routinely need to disable IO scheduling for example when driving IO to > high speed/low latency devices and are actively looking at how to tackle > other performance bottlenecks in the stack. I am not talking about the regular data path, only of SG_IO. > I don't see a strong reason that our existing scheme (root or > CAP_SYS_RAWIO access) prevents you from doing what you need to do. Here are three: - CAP_SYS_RAWIO partly bypasses DAC; you can issue destructive commands even if you only opened the disk for reading. CAP_SYS_RAWIO also gives access to _really_ destructive commands (WRITE BUFFER and PERSISTENT RESERVE OUT for example). - CAP_SYS_RAWIO lets you send SCSI commands to partitions, and they will gladly read/write the disk going outside the boundaries of the partition. Changing this behavior was rejected upstream already. - CAP_SYS_RAWIO also gives access to I/O ports, mmap at address 0, and too many other insecure things. All the above mean that: - any application using CAP_SYS_RAWIO would have to implement its own whitelisting, even if just to duplicate what is done in the kernel; - exploiting a CAP_SYS_RAWIO process leads to root too easily, and it is not possible to give the capability to anything that will run in a hostile environment (in my case QEMU). Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Ping^3] Re: [PATCH] sg_io: allow UNMAP and WRITE SAME without CAP_SYS_RAWIO
On 09/06/2012 07:49 AM, Paolo Bonzini wrote: Il 06/09/2012 13:31, Ric Wheeler ha scritto: Both of these commands are destructive. WRITE_SAME (if done without the discard bits set) can also take a very long time to be destructive and tie up the storage. FORMAT_UNIT has the same characteristics and yet it is allowed (btw, I don't think WRITE SAME slowness is limited to the case where a real write is requested; discarding can be just as slow). Also, the two new commands are anyway restricted to programs that have write access to the disk. If you have read-only access, you won't be able to issue any destructive command (there is one exception, START STOP UNIT is allowed even with read-only capability and is somewhat destructive). Honestly, the only reason why these two commands weren't included, is that the current whitelist is heavily tailored towards CD/DVD burning. I assume that FORMAT_UNIT is for CD/DVD needs - not sure what a S-ATA disk would do with that. According to the standard, the translation layer can write a user-provided pattern to every sector in the disk. It's an optional feature and libata doesn't do that, but it is still possible. It is not possible today with our stack though, any patch that would change that would also need to be vetted. If it is destructive, we should probably think about how to make it more secure and see how many applications we would break. We have filesystem permissions to make it secure. They already do. I think that restricting them to CAP_SYS_RAWIO seems reasonable - better to vet and give the appropriate apps the needed capability than to widely open up the safety check? CAP_SYS_RAWIO is so wide in its scope, that anything that requires it is insecure. I don't see allowing anyone who can open the device to zero the data as better though :) Note: anyone who can open it for writing! And they can just as well issue WRITE, it just takes a little more effort than with WRITE SAME. :) If you only have read access, you cannot issue WRITE or FORMAT UNIT, and with this patch you will not be able to issue WRITE SAME. I'm all for providing more versatile filters---which can be both stricter and looser depending on the configuration than the default. For example http://www.redhat.com/archives/libvir-list/2012-June/msg00505.html is a possible spec for BPF-based filtering of CDBs. However, the default whitelist (which is all we have for now) should provide a reasonable default for a user that already has been granted access to the device by the normal access control mechanisms. I believe WRITE SAME and UNMAP fit the definition of a reasonable default. Paolo This just seems like an argument over whether or not capabilities make sense. In general, anything as destructive as a single CDB that can kill all of your data should be tightly controlled. Pushing more code in the data path is not where we are going - we routinely need to disable IO scheduling for example when driving IO to high speed/low latency devices and are actively looking at how to tackle other performance bottlenecks in the stack. I don't see a strong reason that our existing scheme (root or CAP_SYS_RAWIO access) prevents you from doing what you need to do. Ric -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Ping^3] Re: [PATCH] sg_io: allow UNMAP and WRITE SAME without CAP_SYS_RAWIO
Il 06/09/2012 13:31, Ric Wheeler ha scritto: >>> Both of these commands are destructive. WRITE_SAME (if done without the >>> discard bits set) can also take a very long time to be destructive and >>> tie up the storage. >> >> FORMAT_UNIT has the same characteristics and yet it is allowed (btw, I >> don't think WRITE SAME slowness is limited to the case where a real >> write is requested; discarding can be just as slow). >> >> Also, the two new commands are anyway restricted to programs that have >> write access to the disk. If you have read-only access, you won't be >> able to issue any destructive command (there is one exception, START >> STOP UNIT is allowed even with read-only capability and is somewhat >> destructive). >> >> Honestly, the only reason why these two commands weren't included, is >> that the current whitelist is heavily tailored towards CD/DVD burning. > > I assume that FORMAT_UNIT is for CD/DVD needs - not sure what a S-ATA > disk would do with that. According to the standard, the translation layer can write a user-provided pattern to every sector in the disk. It's an optional feature and libata doesn't do that, but it is still possible. > If it is destructive, we should probably think > about how to make it more secure and see how many applications we would > break. We have filesystem permissions to make it secure. They already do. >>> I think that restricting them to CAP_SYS_RAWIO seems reasonable - better >>> to vet and give the appropriate apps the needed capability than to >>> widely open up the safety check? >> CAP_SYS_RAWIO is so wide in its scope, that anything that requires it is >> insecure. > > I don't see allowing anyone who can open the device to zero the data as > better though :) Note: anyone who can open it for writing! And they can just as well issue WRITE, it just takes a little more effort than with WRITE SAME. :) If you only have read access, you cannot issue WRITE or FORMAT UNIT, and with this patch you will not be able to issue WRITE SAME. I'm all for providing more versatile filters---which can be both stricter and looser depending on the configuration than the default. For example http://www.redhat.com/archives/libvir-list/2012-June/msg00505.html is a possible spec for BPF-based filtering of CDBs. However, the default whitelist (which is all we have for now) should provide a reasonable default for a user that already has been granted access to the device by the normal access control mechanisms. I believe WRITE SAME and UNMAP fit the definition of a reasonable default. Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 43234] kernel crashes whenever the thumbdrive or USB devices are remove
https://bugzilla.kernel.org/show_bug.cgi?id=43234 Alan changed: What|Removed |Added Status|NEW |RESOLVED CC||a...@lxorguk.ukuu.org.uk Resolution||CODE_FIX -- Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Ping^3] Re: [PATCH] sg_io: allow UNMAP and WRITE SAME without CAP_SYS_RAWIO
On 09/06/2012 02:31 AM, Paolo Bonzini wrote: Il 05/09/2012 22:18, Ric Wheeler ha scritto: Hi Paolo, Both of these commands are destructive. WRITE_SAME (if done without the discard bits set) can also take a very long time to be destructive and tie up the storage. FORMAT_UNIT has the same characteristics and yet it is allowed (btw, I don't think WRITE SAME slowness is limited to the case where a real write is requested; discarding can be just as slow). Also, the two new commands are anyway restricted to programs that have write access to the disk. If you have read-only access, you won't be able to issue any destructive command (there is one exception, START STOP UNIT is allowed even with read-only capability and is somewhat destructive). Honestly, the only reason why these two commands weren't included, is that the current whitelist is heavily tailored towards CD/DVD burning. Hi Paolo, I assume that FORMAT_UNIT is for CD/DVD needs - not sure what a S-ATA disk would do with that. If it is destructive, we should probably think about how to make it more secure and see how many applications we would break. I think that restricting them to CAP_SYS_RAWIO seems reasonable - better to vet and give the appropriate apps the needed capability than to widely open up the safety check? CAP_SYS_RAWIO is so wide in its scope, that anything that requires it is insecure. Paolo I don't see allowing anyone who can open the device to zero the data as better though :) Ric -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 43164] FC card not detected/mounted via external multicard reader
https://bugzilla.kernel.org/show_bug.cgi?id=43164 Alan changed: What|Removed |Added CC||a...@lxorguk.ukuu.org.uk Component|SCSI|USB AssignedTo|linux-scsi@vger.kernel.org |g...@kroah.com Product|IO/Storage |Drivers -- Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] [SCSI] scsi_debug: Add "removable" parameter
Add "removable" module parameter to set the "removable" attribute of any
subsequently created debug block device. It is a writable driver option, so
that you can switch between removable and "fixed" media block devices in
between the add_host calls.
This is useful for being able to test the different behaviour/required
privileges in e. g. the udisks test suite.
Signed-off-by: Martin Pitt
Acked-By: David Zeuthen
---
drivers/scsi/scsi_debug.c | 30 +++---
1 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 182d5a5..57fbd5a 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -109,6 +109,7 @@ static const char * scsi_debug_version_date = "20100324";
#define DEF_OPT_BLKS 64
#define DEF_PHYSBLK_EXP 0
#define DEF_PTYPE 0
+#define DEF_REMOVABLE false
#define DEF_SCSI_LEVEL 5/* INQUIRY, byte2 [5->SPC-3] */
#define DEF_SECTOR_SIZE 512
#define DEF_UNMAP_ALIGNMENT 0
@@ -193,11 +194,11 @@ static unsigned int scsi_debug_unmap_granularity =
DEF_UNMAP_GRANULARITY;
static unsigned int scsi_debug_unmap_max_blocks = DEF_UNMAP_MAX_BLOCKS;
static unsigned int scsi_debug_unmap_max_desc = DEF_UNMAP_MAX_DESC;
static unsigned int scsi_debug_write_same_length = DEF_WRITESAME_LENGTH;
+static bool scsi_debug_removable = DEF_REMOVABLE;
static int scsi_debug_cmnd_count = 0;
#define DEV_READONLY(TGT) (0)
-#define DEV_REMOVEABLE(TGT)(0)
static unsigned int sdebug_store_sectors;
static sector_t sdebug_capacity; /* in sectors */
@@ -919,7 +920,7 @@ static int resp_inquiry(struct scsi_cmnd * scp, int target,
return ret;
}
/* drops through here for a standard inquiry */
- arr[1] = DEV_REMOVEABLE(target) ? 0x80 : 0; /* Removable disk */
+ arr[1] = scsi_debug_removable ? 0x80 : 0; /* Removable disk */
arr[2] = scsi_debug_scsi_level;
arr[3] = 2;/* response_data_format==2 */
arr[4] = SDEBUG_LONG_INQ_SZ - 5;
@@ -1211,7 +1212,7 @@ static int resp_format_pg(unsigned char * p, int
pcontrol, int target)
p[11] = sdebug_sectors_per & 0xff;
p[12] = (scsi_debug_sector_size >> 8) & 0xff;
p[13] = scsi_debug_sector_size & 0xff;
- if (DEV_REMOVEABLE(target))
+ if (scsi_debug_removable)
p[20] |= 0x20; /* should agree with INQUIRY */
if (1 == pcontrol)
memset(p + 2, 0, sizeof(format_pg) - 2);
@@ -2754,6 +2755,7 @@ module_param_named(opt_blks, scsi_debug_opt_blks, int,
S_IRUGO);
module_param_named(opts, scsi_debug_opts, int, S_IRUGO | S_IWUSR);
module_param_named(physblk_exp, scsi_debug_physblk_exp, int, S_IRUGO);
module_param_named(ptype, scsi_debug_ptype, int, S_IRUGO | S_IWUSR);
+module_param_named(removable, scsi_debug_removable, bool, S_IRUGO | S_IWUSR);
module_param_named(scsi_level, scsi_debug_scsi_level, int, S_IRUGO);
module_param_named(sector_size, scsi_debug_sector_size, int, S_IRUGO);
module_param_named(unmap_alignment, scsi_debug_unmap_alignment, int, S_IRUGO);
@@ -2796,6 +2798,7 @@ MODULE_PARM_DESC(opt_blks, "optimal transfer length in
block (def=64)");
MODULE_PARM_DESC(opts, "1->noise, 2->medium_err, 4->timeout,
8->recovered_err... (def=0)");
MODULE_PARM_DESC(physblk_exp, "physical block exponent (def=0)");
MODULE_PARM_DESC(ptype, "SCSI peripheral type(def=0[disk])");
+MODULE_PARM_DESC(removable, "claim to have removable media (def=0)");
MODULE_PARM_DESC(scsi_level, "SCSI level to simulate(def=5[SPC-3])");
MODULE_PARM_DESC(sector_size, "logical block size in bytes (def=512)");
MODULE_PARM_DESC(unmap_alignment, "lowest aligned thin provisioning lba
(def=0)");
@@ -3205,6 +3208,25 @@ static ssize_t sdebug_map_show(struct device_driver
*ddp, char *buf)
}
DRIVER_ATTR(map, S_IRUGO, sdebug_map_show, NULL);
+static ssize_t sdebug_removable_show(struct device_driver *ddp,
+char *buf)
+{
+ return scnprintf(buf, PAGE_SIZE, "%d\n", scsi_debug_removable ? 1 : 0);
+}
+static ssize_t sdebug_removable_store(struct device_driver *ddp,
+ const char *buf, size_t count)
+{
+ int n;
+
+ if ((count > 0) && (1 == sscanf(buf, "%d", &n)) && (n >= 0)) {
+ scsi_debug_removable = (n > 0);
+ return count;
+ }
+ return -EINVAL;
+}
+DRIVER_ATTR(removable, S_IRUGO | S_IWUSR, sdebug_removable_show,
+ sdebug_removable_store);
+
/* Note: The following function creates attribute files in the
/sys/bus/pseudo/drivers/scsi_debug directory. The advantage of these
@@ -3230,6 +3252,7 @@ static int do_create_driverfs_files(void)
ret |= driver_create_file(&sdebug_driverfs_driver,
&driver_attr_num_tgts);
ret |= driver_create_file(&sdebug_driverfs_driver, &driver_attr_ptype);
ret |= driver_create_file(&sdebug_driverfs_driver, &driver_attr_opts);
+ ret |= driver_create_file(&s
[PATCH 0/1] Option for scsi_debug to fake removable devices
Hello all, I already re-sent this 1.5 months ago, but did not get any answer back then; I guess it got lost in the noise by now. So, patiently retrying again. For the purposes of automatically testing udisks and gvfs automounting I would like to add a parameter to scsi_debug to control the "removable" attribute of the created block device. With that, we can test system-internal and removable drives, as well as CD-ROMs (which scsi_debug can already emulate). udisks requires different privileges for mounting system-internal drives vs. removable/hotpluggable drives. This will also allow us to write system integration tests for gvfs, which will exercise the whole stack including the actual polkit configuration in a VM. I wrote a simple kernel patch for this (against linux-next), and tested this quite thoroughly. I ran the style checker, and it reports two problems: 8< -- WARNING: line over 80 characters #109: FILE: drivers/scsi/scsi_debug.c:3255: + ret |= driver_create_file(&sdebug_driverfs_driver, &driver_attr_removable); WARNING: Prefer pr_err(... to printk(KERN_ERR, ... #126: FILE: drivers/scsi/scsi_debug.c:3353: + printk(KERN_ERR "scsi_debug_init: removable must be 0 or 1\n"); 8< -- But as the existing code uses this style in the adjacent lines, I favored consistency over fixing those. If the latter is desired, I'd rather send a separate patch with just the style cleanup for the whole file. I got an ack from David Zeuthen (the primary udisks maintainer) already, noted so in the patch. Thank you in advance for considering, Martin -- Martin Pitt| http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org) signature.asc Description: Digital signature
