Re: [Qemu-devel] [PATCH] ATAPI: Add support for ASCQ in sense codes

[Kevin Wolf]

Am 31.07.2012 09:14, schrieb Paolo Bonzini:
> Il 31/07/2012 04:07, Ronnie Sahlberg ha scritto:
>> Add support for setting the ASCQ for SCSI sense codes in the ATAPI driver.
>> Use this to set ASCQ==2 for the medium removal prevention that is 
>> recommended in MMC for this condition.
>>
>> asc:0x53 ascq:0x02 is the recommended error for MEDIUM_REMOVAL_PREVENTED and 
>> is listed in Annex F in MMC
> 
> You also need to cover migration.
> 
> You could either add a subsection, or something like this:
> 
> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
> index f7f714c..89c0157 100644
> --- a/hw/ide/atapi.c
> +++ b/hw/ide/atapi.c
> @@ -1143,3 +1143,20 @@ void ide_atapi_cmd(IDEState *s)
>  
>      ide_atapi_cmd_error(s, ILLEGAL_REQUEST, ASC_ILLEGAL_OPCODE);
>  }
> +
> +void ide_atapi_post_load(IDEState *s, int version_id)
> +{
> +    if (version_id < 3) {
> +        if (s->sense_key == UNIT_ATTENTION &&
> +            s->asc == ASC_MEDIUM_MAY_HAVE_CHANGED) {
> +            s->cdrom_changed = 1;
> +        }
> +    }
> +
> +    /* This is simpler than adding a subsection just for the ascq.  */
> +    if (s->asc == ASC_MEDIA_REMOVAL_PREVENTED) {
> +        s->ascq = 2;
> +    } else {
> +        s->ascq = 0;
> +    }
> +}
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index cb5ca4b..959ac48 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -2154,12 +2154,7 @@ static int ide_drive_post_load(void *opaque, int 
> version_id)
>  {
>      IDEState *s = opaque;
>  
> -    if (version_id < 3) {
> -        if (s->sense_key == UNIT_ATTENTION &&
> -            s->asc == ASC_MEDIUM_MAY_HAVE_CHANGED) {
> -            s->cdrom_changed = 1;
> -        }
> -    }
> +    ide_atapi_post_load(s, version_id);
>      if (s->identify_set) {
>          bdrv_set_enable_write_cache(s->bs, !!(s->identify_data[85] & (1 << 
> 5)));
>      }
> diff --git a/hw/ide/internal.h b/hw/ide/internal.h
> index 7170bd9..2572461 100644
> --- a/hw/ide/internal.h
> +++ b/hw/ide/internal.h
> @@ -572,6 +572,7 @@ BlockDriverAIOCB *ide_issue_trim(BlockDriverState *bs,
>  /* hw/ide/atapi.c */
>  void ide_atapi_cmd(IDEState *s);
>  void ide_atapi_cmd_reply_end(IDEState *s);
> +void ide_atapi_post_load(IDEState *s, int version_id);
>  
>  /* hw/ide/qdev.c */
>  void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id);
> 
> 
> In fact, I wonder if it is simpler to "make up" the ascq directly in
> cmd_request_sense, instead of changing all invocations of
> ide_atapi_cmd_error.  Kevin, any preferences?

Oh, I missed this question and wondered why there was no v2... I'd
prefer this patch with migration support added.

Kevin