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 <f...@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
