From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:55744)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1XjXyo-00057v-D8
	for qemu-devel@nongnu.org; Wed, 29 Oct 2014 14:28:28 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1XjXyi-0001gf-6l
	for qemu-devel@nongnu.org; Wed, 29 Oct 2014 14:28:22 -0400
Received: from mx1.redhat.com ([209.132.183.28]:49589)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1XjXyh-0001gZ-V4
	for qemu-devel@nongnu.org; Wed, 29 Oct 2014 14:28:16 -0400
Message-ID: <545131B1.6020401@redhat.com>
Date: Wed, 29 Oct 2014 19:28:01 +0100
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1414588414-633-1-git-send-email-famz@redhat.com>
	<5450EC1A.9020502@redhat.com> <54511367.8010703@kamp.de>
In-Reply-To: <54511367.8010703@kamp.de>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] iscsi: Refuse to open as writable if the
 LUN is write protected
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Peter Lieven <pl@kamp.de>, Fam Zheng <famz@redhat.com>, qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Ronnie Sahlberg <ronniesahlberg@gmail.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.

Paolo