[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] iscsi: Refuse to open as writable if the LUN is
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH] iscsi: Refuse to open as writable if the LUN is write protected |
Date: |
Wed, 29 Oct 2014 19:28:01 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 |
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.
Paolo