On 30.10.2014 13:09, Paolo Bonzini wrote:
On 10/30/2014 12:43 PM, Peter Lieven wrote:
On 30.10.2014 12:23, 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>
---
v2: Improve error message.
Fall back to a warning if mode sense failed.
Check unmarshal return value.
---
block/iscsi.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/block/iscsi.c b/block/iscsi.c
index 233f462..dcacbca 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1219,6 +1219,44 @@ static void
iscsi_attach_aio_context(BlockDriverState *bs,
qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + NOP_INTERVAL);
}
+static bool iscsi_is_write_protected(IscsiLun *iscsilun)
+{
+ struct scsi_task *task;
+ struct scsi_mode_sense *ms = NULL;
+
+ task = iscsi_modesense6_sync(iscsilun->iscsi, iscsilun->lun,
+ 1, SCSI_MODESENSE_PC_CURRENT,
+ 0x3F,
+ 0, 255);
+
+ if (task == NULL) {
+ error_report("Failed to send MODE_SENSE6 command: %s",
+ iscsi_get_error(iscsilun->iscsi));
+ goto out;
+ }
+
+ if (task->status != SCSI_STATUS_GOOD) {
+ error_report("MODE_SENSE6 failed: %s",
+ iscsi_get_error(iscsilun->iscsi));
+ goto out;
+ }
+ ms = scsi_datain_unmarshall(task);
+ if (!ms) {
+ error_report("MODE_SENSE6 failed: %s",
+ iscsi_get_error(iscsilun->iscsi));
+ goto out;
+ }
+out:
+ if (task) {
+ scsi_free_scsi_task(task);
+ }
+ if (!ms) {
ms points to freed memory after scsi_free_scsi_task.
furthermore the requests likely fails with task->status != SCSI_STATUS_GOOD
if the modesense implementation is broken etc.
This is a mix of your and Fam's code. Looks good?
static bool iscsi_is_write_protected(IscsiLun *iscsilun)
{
struct scsi_task *task;
struct scsi_mode_sense *ms = NULL;
bool wrprotected = false;
task = iscsi_modesense6_sync(iscsilun->iscsi, iscsilun->lun,
1, SCSI_MODESENSE_PC_CURRENT,
0x3F, 0, 255);
if (task == NULL) {
error_report("Failed to send MODE_SENSE(6) command: %s",
iscsi_get_error(iscsilun->iscsi));
goto out;
}
if (task->status != SCSI_STATUS_GOOD) {
error_report("MODE_SENSE(6) failed: %s",
iscsi_get_error(iscsilun->iscsi));
goto out;
}
ms = scsi_datain_unmarshall(task);
if (!ms) {
error_report("Failed to unmarshall MODE_SENSE(6) data: %s",
iscsi_get_error(iscsilun->iscsi));
goto out;
}
wrprotected = ms->device_specific_parameter & 0x80;
out:
if (task) {
scsi_free_scsi_task(task);
}
return wrprotected;
}
i would add the prefix "iSCSI: " to the error_reports as we
have it for other outputs. (noticed this after writing my mail).
Otherwise looks good.
Reviewed-by: Peter Lieven <p...@kamp.de>
and actually also
Tested-by: Peter Lieven <p...@kamp.de>
Peter
