Re: [Qemu-devel] [PATCH] iscsi: Refuse to open as writable if the LUN is write protected

[Peter Lieven]

Am 29.10.2014 um 19:28 schrieb Paolo Bonzini <address@hidden>:

> 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 <address@hidden>
>>>> ---
>>>> 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