On Thu, 10/30 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;
> }
Looks good. Thanks!
Fam
