* [Qemu-devel] [PATCH] iscsi: Refuse to open as writable if the LUN is write protected
@ 2014-10-29 13:13 Fam Zheng
2014-10-29 13:31 ` Paolo Bonzini
0 siblings, 1 reply; 8+ messages in thread
From: Fam Zheng @ 2014-10-29 13:13 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Paolo Bonzini, Peter Lieven, Stefan Hajnoczi,
Ronnie Sahlberg
Before, when a write protected iSCSI target is attached as scsi-disk
with BDRV_O_RDWR, we report it as writable, while in fact all writes
will fail.
One way to improve this is to report write protect flag as true to
guest, but a even better way is to refuse using a write protected LUN to
guest.
Target write protect flag is checked with a mode sense query.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block/iscsi.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/block/iscsi.c b/block/iscsi.c
index 233f462..c154928 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1339,6 +1339,36 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
scsi_free_scsi_task(task);
task = NULL;
+ /* Check the write protect flag of the LUN if we want to write */
+ if (flags & BDRV_O_RDWR) {
+ struct scsi_mode_sense *ms;
+
+ task = iscsi_modesense6_sync(iscsilun->iscsi, iscsilun->lun,
+ 1, SCSI_MODESENSE_PC_CURRENT,
+ 0x3F,
+ 0, 255);
+
+ if (task == NULL) {
+ error_setg(errp, "Failed to send MODE_SENSE10 command: %s\n",
+ iscsi_get_error(iscsilun->iscsi));
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (task->status != SCSI_STATUS_GOOD) {
+ error_setg(errp, "MODE_SENSE10 failed: %s\n",
+ iscsi_get_error(iscsi));
+ ret = -EINVAL;
+ goto out;
+ }
+ ms = scsi_datain_unmarshall(task);
+ if (ms->device_specific_parameter & 0x80) {
+ error_setg(errp, "Cannot open a write protected LUN as read-write");
+ ret = -EPERM;
+ goto out;
+ }
+ }
+
iscsi_readcapacity_sync(iscsilun, &local_err);
if (local_err != NULL) {
error_propagate(errp, local_err);
--
1.9.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH] iscsi: Refuse to open as writable if the LUN is write protected
2014-10-29 13:13 [Qemu-devel] [PATCH] iscsi: Refuse to open as writable if the LUN is write protected Fam Zheng
@ 2014-10-29 13:31 ` Paolo Bonzini
2014-10-29 16:18 ` Peter Lieven
0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2014-10-29 13:31 UTC (permalink / raw)
To: Fam Zheng, qemu-devel
Cc: Kevin Wolf, Peter Lieven, Stefan Hajnoczi, Ronnie Sahlberg
On 10/29/2014 02:13 PM, Fam Zheng wrote:
> Before, when a write protected iSCSI target is attached as scsi-disk
> with BDRV_O_RDWR, we report it as writable, while in fact all writes
> will fail.
>
> One way to improve this is to report write protect flag as true to
> guest, but a even better way is to refuse using a write protected LUN to
> guest.
>
> Target write protect flag is checked with a mode sense query.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> block/iscsi.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 233f462..c154928 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1339,6 +1339,36 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
> scsi_free_scsi_task(task);
> task = NULL;
>
> + /* Check the write protect flag of the LUN if we want to write */
> + if (flags & BDRV_O_RDWR) {
> + struct scsi_mode_sense *ms;
> +
> + task = iscsi_modesense6_sync(iscsilun->iscsi, iscsilun->lun,
> + 1, SCSI_MODESENSE_PC_CURRENT,
> + 0x3F,
> + 0, 255);
> +
> + if (task == NULL) {
> + error_setg(errp, "Failed to send MODE_SENSE10 command: %s\n",
This is MODE SENSE(6). Fixed and applied.
Paolo
> + iscsi_get_error(iscsilun->iscsi));
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + if (task->status != SCSI_STATUS_GOOD) {
> + error_setg(errp, "MODE_SENSE10 failed: %s\n",
> + iscsi_get_error(iscsi));
> + ret = -EINVAL;
> + goto out;
> + }
> + ms = scsi_datain_unmarshall(task);
> + if (ms->device_specific_parameter & 0x80) {
> + error_setg(errp, "Cannot open a write protected LUN as read-write");
> + ret = -EPERM;
> + goto out;
> + }
> + }
> +
> iscsi_readcapacity_sync(iscsilun, &local_err);
> if (local_err != NULL) {
> error_propagate(errp, local_err);
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH] iscsi: Refuse to open as writable if the LUN is write protected
2014-10-29 13:31 ` Paolo Bonzini
@ 2014-10-29 16:18 ` Peter Lieven
2014-10-29 18:28 ` Paolo Bonzini
0 siblings, 1 reply; 8+ messages in thread
From: Peter Lieven @ 2014-10-29 16:18 UTC (permalink / raw)
To: Paolo Bonzini, Fam Zheng, qemu-devel
Cc: Kevin Wolf, Stefan Hajnoczi, Ronnie Sahlberg
Am 29.10.2014 um 14:31 schrieb Paolo Bonzini:
>
> On 10/29/2014 02:13 PM, Fam Zheng wrote:
>> Before, when a write protected iSCSI target is attached as scsi-disk
>> with BDRV_O_RDWR, we report it as writable, while in fact all writes
>> will fail.
>>
>> One way to improve this is to report write protect flag as true to
>> guest, but a even better way is to refuse using a write protected LUN to
>> guest.
>>
>> Target write protect flag is checked with a mode sense query.
>>
>> Signed-off-by: Fam Zheng <famz@redhat.com>
>> ---
>> block/iscsi.c | 30 ++++++++++++++++++++++++++++++
>> 1 file changed, 30 insertions(+)
>>
>> diff --git a/block/iscsi.c b/block/iscsi.c
>> index 233f462..c154928 100644
>> --- a/block/iscsi.c
>> +++ b/block/iscsi.c
>> @@ -1339,6 +1339,36 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
>> scsi_free_scsi_task(task);
>> task = NULL;
>>
>> + /* Check the write protect flag of the LUN if we want to write */
>> + if (flags & BDRV_O_RDWR) {
>> + struct scsi_mode_sense *ms;
>> +
>> + task = iscsi_modesense6_sync(iscsilun->iscsi, iscsilun->lun,
>> + 1, SCSI_MODESENSE_PC_CURRENT,
>> + 0x3F,
>> + 0, 255);
>> +
>> + if (task == NULL) {
>> + error_setg(errp, "Failed to send MODE_SENSE10 command: %s\n",
> This is MODE SENSE(6). Fixed and applied.
>
> Paolo
>
>> + iscsi_get_error(iscsilun->iscsi));
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> + if (task->status != SCSI_STATUS_GOOD) {
>> + error_setg(errp, "MODE_SENSE10 failed: %s\n",
>> + iscsi_get_error(iscsi));
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> + ms = scsi_datain_unmarshall(task);
scsi_datain_unmarshall may fail. You need to check for NULL here.
Peter
>> + if (ms->device_specific_parameter & 0x80) {
>> + error_setg(errp, "Cannot open a write protected LUN as read-write");
>> + ret = -EPERM;
>> + goto out;
>> + }
>> + }
>> +
>> iscsi_readcapacity_sync(iscsilun, &local_err);
>> if (local_err != NULL) {
>> error_propagate(errp, local_err);
>>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH] iscsi: Refuse to open as writable if the LUN is write protected
2014-10-29 16:18 ` Peter Lieven
@ 2014-10-29 18:28 ` Paolo Bonzini
2014-10-29 21:12 ` Peter Lieven
2014-10-30 10:27 ` Peter Lieven
0 siblings, 2 replies; 8+ messages in thread
From: Paolo Bonzini @ 2014-10-29 18:28 UTC (permalink / raw)
To: Peter Lieven, Fam Zheng, qemu-devel
Cc: Kevin Wolf, Stefan Hajnoczi, Ronnie Sahlberg
On 10/29/2014 05:18 PM, Peter Lieven wrote:
> Am 29.10.2014 um 14:31 schrieb Paolo Bonzini:
>>
>> On 10/29/2014 02:13 PM, Fam Zheng wrote:
>>> Before, when a write protected iSCSI target is attached as scsi-disk
>>> with BDRV_O_RDWR, we report it as writable, while in fact all writes
>>> will fail.
>>>
>>> One way to improve this is to report write protect flag as true to
>>> guest, but a even better way is to refuse using a write protected LUN to
>>> guest.
>>>
>>> Target write protect flag is checked with a mode sense query.
>>>
>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>> ---
>>> block/iscsi.c | 30 ++++++++++++++++++++++++++++++
>>> 1 file changed, 30 insertions(+)
>>>
>>> diff --git a/block/iscsi.c b/block/iscsi.c
>>> index 233f462..c154928 100644
>>> --- a/block/iscsi.c
>>> +++ b/block/iscsi.c
>>> @@ -1339,6 +1339,36 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
>>> scsi_free_scsi_task(task);
>>> task = NULL;
>>>
>>> + /* Check the write protect flag of the LUN if we want to write */
>>> + if (flags & BDRV_O_RDWR) {
>>> + struct scsi_mode_sense *ms;
>>> +
>>> + task = iscsi_modesense6_sync(iscsilun->iscsi, iscsilun->lun,
>>> + 1, SCSI_MODESENSE_PC_CURRENT,
>>> + 0x3F,
>>> + 0, 255);
>>> +
>>> + if (task == NULL) {
>>> + error_setg(errp, "Failed to send MODE_SENSE10 command: %s\n",
>> This is MODE SENSE(6). Fixed and applied.
>>
>> Paolo
>>
>>> + iscsi_get_error(iscsilun->iscsi));
>>> + ret = -EINVAL;
>>> + goto out;
>>> + }
>>> +
>>> + if (task->status != SCSI_STATUS_GOOD) {
>>> + error_setg(errp, "MODE_SENSE10 failed: %s\n",
>>> + iscsi_get_error(iscsi));
>>> + ret = -EINVAL;
>>> + goto out;
>>> + }
>>> + ms = scsi_datain_unmarshall(task);
>
> scsi_datain_unmarshall may fail. You need to check for NULL here.
Thanks for the remark, I fixed this too.
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH] iscsi: Refuse to open as writable if the LUN is write protected
2014-10-29 18:28 ` Paolo Bonzini
@ 2014-10-29 21:12 ` Peter Lieven
2014-10-30 10:27 ` Peter Lieven
1 sibling, 0 replies; 8+ messages in thread
From: Peter Lieven @ 2014-10-29 21:12 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Kevin Wolf, Stefan Hajnoczi, Fam Zheng, qemu-devel,
Ronnie Sahlberg
Am 29.10.2014 um 19:28 schrieb Paolo Bonzini <pbonzini@redhat.com>:
> On 10/29/2014 05:18 PM, Peter Lieven wrote:
>> Am 29.10.2014 um 14:31 schrieb Paolo Bonzini:
>>>
>>> On 10/29/2014 02:13 PM, Fam Zheng wrote:
>>>> Before, when a write protected iSCSI target is attached as scsi-disk
>>>> with BDRV_O_RDWR, we report it as writable, while in fact all writes
>>>> will fail.
>>>>
>>>> One way to improve this is to report write protect flag as true to
>>>> guest, but a even better way is to refuse using a write protected LUN to
>>>> guest.
>>>>
>>>> Target write protect flag is checked with a mode sense query.
>>>>
>>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>>> ---
>>>> block/iscsi.c | 30 ++++++++++++++++++++++++++++++
>>>> 1 file changed, 30 insertions(+)
>>>>
>>>> diff --git a/block/iscsi.c b/block/iscsi.c
>>>> index 233f462..c154928 100644
>>>> --- a/block/iscsi.c
>>>> +++ b/block/iscsi.c
>>>> @@ -1339,6 +1339,36 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
>>>> scsi_free_scsi_task(task);
>>>> task = NULL;
>>>>
>>>> + /* Check the write protect flag of the LUN if we want to write */
>>>> + if (flags & BDRV_O_RDWR) {
>>>> + struct scsi_mode_sense *ms;
>>>> +
>>>> + task = iscsi_modesense6_sync(iscsilun->iscsi, iscsilun->lun,
>>>> + 1, SCSI_MODESENSE_PC_CURRENT,
>>>> + 0x3F,
>>>> + 0, 255);
>>>> +
>>>> + if (task == NULL) {
>>>> + error_setg(errp, "Failed to send MODE_SENSE10 command: %s\n",
>>> This is MODE SENSE(6). Fixed and applied.
>>>
>>> Paolo
>>>
>>>> + iscsi_get_error(iscsilun->iscsi));
>>>> + ret = -EINVAL;
>>>> + goto out;
>>>> + }
>>>> +
>>>> + if (task->status != SCSI_STATUS_GOOD) {
>>>> + error_setg(errp, "MODE_SENSE10 failed: %s\n",
>>>> + iscsi_get_error(iscsi));
>>>> + ret = -EINVAL;
>>>> + goto out;
>>>> + }
>>>> + ms = scsi_datain_unmarshall(task);
>>
>> scsi_datain_unmarshall may fail. You need to check for NULL here.
>
> Thanks for the remark, I fixed this too.
Is it in your repo? I can do some testing tomorrow ;-)
Peter
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH] iscsi: Refuse to open as writable if the LUN is write protected
2014-10-29 18:28 ` Paolo Bonzini
2014-10-29 21:12 ` Peter Lieven
@ 2014-10-30 10:27 ` Peter Lieven
2014-10-30 10:50 ` Fam Zheng
1 sibling, 1 reply; 8+ messages in thread
From: Peter Lieven @ 2014-10-30 10:27 UTC (permalink / raw)
To: Paolo Bonzini, Fam Zheng, qemu-devel
Cc: Kevin Wolf, Stefan Hajnoczi, Ronnie Sahlberg
[-- Attachment #1: Type: text/plain, Size: 2749 bytes --]
On 29.10.2014 19:28, Paolo Bonzini wrote:
> On 10/29/2014 05:18 PM, Peter Lieven wrote:
>> Am 29.10.2014 um 14:31 schrieb Paolo Bonzini:
>>> On 10/29/2014 02:13 PM, Fam Zheng wrote:
>>>> Before, when a write protected iSCSI target is attached as scsi-disk
>>>> with BDRV_O_RDWR, we report it as writable, while in fact all writes
>>>> will fail.
>>>>
>>>> One way to improve this is to report write protect flag as true to
>>>> guest, but a even better way is to refuse using a write protected LUN to
>>>> guest.
>>>>
>>>> Target write protect flag is checked with a mode sense query.
>>>>
>>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>>> ---
>>>> block/iscsi.c | 30 ++++++++++++++++++++++++++++++
>>>> 1 file changed, 30 insertions(+)
>>>>
>>>> diff --git a/block/iscsi.c b/block/iscsi.c
>>>> index 233f462..c154928 100644
>>>> --- a/block/iscsi.c
>>>> +++ b/block/iscsi.c
>>>> @@ -1339,6 +1339,36 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
>>>> scsi_free_scsi_task(task);
>>>> task = NULL;
>>>>
>>>> + /* Check the write protect flag of the LUN if we want to write */
>>>> + if (flags & BDRV_O_RDWR) {
>>>> + struct scsi_mode_sense *ms;
>>>> +
>>>> + task = iscsi_modesense6_sync(iscsilun->iscsi, iscsilun->lun,
>>>> + 1, SCSI_MODESENSE_PC_CURRENT,
>>>> + 0x3F,
>>>> + 0, 255);
>>>> +
>>>> + if (task == NULL) {
>>>> + error_setg(errp, "Failed to send MODE_SENSE10 command: %s\n",
>>> This is MODE SENSE(6). Fixed and applied.
>>>
>>> Paolo
>>>
>>>> + iscsi_get_error(iscsilun->iscsi));
>>>> + ret = -EINVAL;
>>>> + goto out;
>>>> + }
>>>> +
>>>> + if (task->status != SCSI_STATUS_GOOD) {
>>>> + error_setg(errp, "MODE_SENSE10 failed: %s\n",
>>>> + iscsi_get_error(iscsi));
>>>> + ret = -EINVAL;
>>>> + goto out;
>>>> + }
>>>> + ms = scsi_datain_unmarshall(task);
>> scsi_datain_unmarshall may fail. You need to check for NULL here.
> Thanks for the remark, I fixed this too.
>
> Paolo
I am not 100% happy with this patch as it may break support for some targets.
It seems that MODESENSE is somewhat tricky with some targets. See function sd_read_write_protect_flag
in <http://lxr.free-electrons.com/ident?i=sd_read_write_protect_flag>drivers/scsi/sd.c of the Linux kernel.
I would not fail if this modesense fails, but just assume write enabled. Maybe drop a warning.
In the command itself:
0x3F => SCSI_MODEPAGE_RETURN_ALL_PAGES
Can you please post the resulting patch somewhere for review.
Thanks,
Peter
[-- Attachment #2: Type: text/html, Size: 3499 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH] iscsi: Refuse to open as writable if the LUN is write protected
2014-10-30 10:27 ` Peter Lieven
@ 2014-10-30 10:50 ` Fam Zheng
2014-10-30 10:52 ` Peter Lieven
0 siblings, 1 reply; 8+ messages in thread
From: Fam Zheng @ 2014-10-30 10:50 UTC (permalink / raw)
To: Peter Lieven
Cc: Kevin Wolf, Paolo Bonzini, Ronnie Sahlberg, qemu-devel,
Stefan Hajnoczi
On Thu, 10/30 11:27, Peter Lieven wrote:
> On 29.10.2014 19:28, Paolo Bonzini wrote:
> >On 10/29/2014 05:18 PM, Peter Lieven wrote:
> >>Am 29.10.2014 um 14:31 schrieb Paolo Bonzini:
> >>>On 10/29/2014 02:13 PM, Fam Zheng wrote:
> >>>>Before, when a write protected iSCSI target is attached as scsi-disk
> >>>>with BDRV_O_RDWR, we report it as writable, while in fact all writes
> >>>>will fail.
> >>>>
> >>>>One way to improve this is to report write protect flag as true to
> >>>>guest, but a even better way is to refuse using a write protected LUN to
> >>>>guest.
> >>>>
> >>>>Target write protect flag is checked with a mode sense query.
> >>>>
> >>>>Signed-off-by: Fam Zheng <famz@redhat.com>
> >>>>---
> >>>> block/iscsi.c | 30 ++++++++++++++++++++++++++++++
> >>>> 1 file changed, 30 insertions(+)
> >>>>
> >>>>diff --git a/block/iscsi.c b/block/iscsi.c
> >>>>index 233f462..c154928 100644
> >>>>--- a/block/iscsi.c
> >>>>+++ b/block/iscsi.c
> >>>>@@ -1339,6 +1339,36 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
> >>>> scsi_free_scsi_task(task);
> >>>> task = NULL;
> >>>>+ /* Check the write protect flag of the LUN if we want to write */
> >>>>+ if (flags & BDRV_O_RDWR) {
> >>>>+ struct scsi_mode_sense *ms;
> >>>>+
> >>>>+ task = iscsi_modesense6_sync(iscsilun->iscsi, iscsilun->lun,
> >>>>+ 1, SCSI_MODESENSE_PC_CURRENT,
> >>>>+ 0x3F,
> >>>>+ 0, 255);
> >>>>+
> >>>>+ if (task == NULL) {
> >>>>+ error_setg(errp, "Failed to send MODE_SENSE10 command: %s\n",
> >>>This is MODE SENSE(6). Fixed and applied.
> >>>
> >>>Paolo
> >>>
> >>>>+ iscsi_get_error(iscsilun->iscsi));
> >>>>+ ret = -EINVAL;
> >>>>+ goto out;
> >>>>+ }
> >>>>+
> >>>>+ if (task->status != SCSI_STATUS_GOOD) {
> >>>>+ error_setg(errp, "MODE_SENSE10 failed: %s\n",
> >>>>+ iscsi_get_error(iscsi));
> >>>>+ ret = -EINVAL;
> >>>>+ goto out;
> >>>>+ }
> >>>>+ ms = scsi_datain_unmarshall(task);
> >>scsi_datain_unmarshall may fail. You need to check for NULL here.
> >Thanks for the remark, I fixed this too.
> >
> >Paolo
>
> I am not 100% happy with this patch as it may break support for some targets.
>
> It seems that MODESENSE is somewhat tricky with some targets. See function sd_read_write_protect_flag
> in <http://lxr.free-electrons.com/ident?i=sd_read_write_protect_flag>drivers/scsi/sd.c of the Linux kernel.
>
> I would not fail if this modesense fails, but just assume write enabled. Maybe drop a warning.
OK.
>
> In the command itself:
> 0x3F => SCSI_MODEPAGE_RETURN_ALL_PAGES
That requires a newer version of libiscsi than we currently check for.
Seeing drivers/scsi/sd.c used 0x3F, I left this.
>
> Can you please post the resulting patch somewhere for review.
I'll fix this with the error messsage and unmarshal failure and post another
version.
Fam
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH] iscsi: Refuse to open as writable if the LUN is write protected
2014-10-30 10:50 ` Fam Zheng
@ 2014-10-30 10:52 ` Peter Lieven
0 siblings, 0 replies; 8+ messages in thread
From: Peter Lieven @ 2014-10-30 10:52 UTC (permalink / raw)
To: Fam Zheng
Cc: Kevin Wolf, Paolo Bonzini, Ronnie Sahlberg, qemu-devel,
Stefan Hajnoczi
On 30.10.2014 11:50, Fam Zheng wrote:
> On Thu, 10/30 11:27, Peter Lieven wrote:
>> On 29.10.2014 19:28, Paolo Bonzini wrote:
>>> On 10/29/2014 05:18 PM, Peter Lieven wrote:
>>>> Am 29.10.2014 um 14:31 schrieb Paolo Bonzini:
>>>>> On 10/29/2014 02:13 PM, Fam Zheng wrote:
>>>>>> Before, when a write protected iSCSI target is attached as scsi-disk
>>>>>> with BDRV_O_RDWR, we report it as writable, while in fact all writes
>>>>>> will fail.
>>>>>>
>>>>>> One way to improve this is to report write protect flag as true to
>>>>>> guest, but a even better way is to refuse using a write protected LUN to
>>>>>> guest.
>>>>>>
>>>>>> Target write protect flag is checked with a mode sense query.
>>>>>>
>>>>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>>>>> ---
>>>>>> block/iscsi.c | 30 ++++++++++++++++++++++++++++++
>>>>>> 1 file changed, 30 insertions(+)
>>>>>>
>>>>>> diff --git a/block/iscsi.c b/block/iscsi.c
>>>>>> index 233f462..c154928 100644
>>>>>> --- a/block/iscsi.c
>>>>>> +++ b/block/iscsi.c
>>>>>> @@ -1339,6 +1339,36 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
>>>>>> scsi_free_scsi_task(task);
>>>>>> task = NULL;
>>>>>> + /* Check the write protect flag of the LUN if we want to write */
>>>>>> + if (flags & BDRV_O_RDWR) {
>>>>>> + struct scsi_mode_sense *ms;
>>>>>> +
>>>>>> + task = iscsi_modesense6_sync(iscsilun->iscsi, iscsilun->lun,
>>>>>> + 1, SCSI_MODESENSE_PC_CURRENT,
>>>>>> + 0x3F,
>>>>>> + 0, 255);
>>>>>> +
>>>>>> + if (task == NULL) {
>>>>>> + error_setg(errp, "Failed to send MODE_SENSE10 command: %s\n",
>>>>> This is MODE SENSE(6). Fixed and applied.
>>>>>
>>>>> Paolo
>>>>>
>>>>>> + iscsi_get_error(iscsilun->iscsi));
>>>>>> + ret = -EINVAL;
>>>>>> + goto out;
>>>>>> + }
>>>>>> +
>>>>>> + if (task->status != SCSI_STATUS_GOOD) {
>>>>>> + error_setg(errp, "MODE_SENSE10 failed: %s\n",
>>>>>> + iscsi_get_error(iscsi));
>>>>>> + ret = -EINVAL;
>>>>>> + goto out;
>>>>>> + }
>>>>>> + ms = scsi_datain_unmarshall(task);
>>>> scsi_datain_unmarshall may fail. You need to check for NULL here.
>>> Thanks for the remark, I fixed this too.
>>>
>>> Paolo
>> I am not 100% happy with this patch as it may break support for some targets.
>>
>> It seems that MODESENSE is somewhat tricky with some targets. See function sd_read_write_protect_flag
>> in <http://lxr.free-electrons.com/ident?i=sd_read_write_protect_flag>drivers/scsi/sd.c of the Linux kernel.
>>
>> I would not fail if this modesense fails, but just assume write enabled. Maybe drop a warning.
> OK.
>
>> In the command itself:
>> 0x3F => SCSI_MODEPAGE_RETURN_ALL_PAGES
> That requires a newer version of libiscsi than we currently check for.
> Seeing drivers/scsi/sd.c used 0x3F, I left this.
Okay, I wasn't ware. It seems it went in for 1.10.0 and we check for 1.9.0, right?
>
>> Can you please post the resulting patch somewhere for review.
> I'll fix this with the error messsage and unmarshal failure and post another
> version.
Thanks,
Peter
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-10-30 10:52 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-29 13:13 [Qemu-devel] [PATCH] iscsi: Refuse to open as writable if the LUN is write protected Fam Zheng
2014-10-29 13:31 ` Paolo Bonzini
2014-10-29 16:18 ` Peter Lieven
2014-10-29 18:28 ` Paolo Bonzini
2014-10-29 21:12 ` Peter Lieven
2014-10-30 10:27 ` Peter Lieven
2014-10-30 10:50 ` Fam Zheng
2014-10-30 10:52 ` Peter Lieven
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).