Re: [Qemu-devel] [PATCH] ide: fix ATAPI read drive structure command (v3)

[Anthony Liguori <aliguori@us.ibm.com>]

Alex Williamson wrote:
> On Tue, 2008-06-03 at 11:59 -0500, Anthony Liguori wrote:
>   
>> Can you send this as an attachment or inlined as plain text?
>>     
>
> Hmm, shows up as plain text in the archives, although there is a strange
> character in front of the patch title.  Here it is again.  Thanks,
>
> 	Alex
>
> Fix ATAPI read drive structure command
>
> Previous version ignored the allocation length parameter and read the
> format byte from the wrong location.  Re-implement to support the full
> requirements for DVD-ROM and allow for easy extension later.
>
> Signed-off-by: Alex Williamson <alex.williamson@hp.com>
> --
>
> --- a/trunk/hw/ide.c	2008-06-02 16:37:12.000000000 -0600
> +++ b/trunk/hw/ide.c	2008-06-02 16:40:11.000000000 -0600
> @@ -351,6 +351,7 @@
>  #define ASC_ILLEGAL_OPCODE                   0x20
>  #define ASC_LOGICAL_BLOCK_OOR                0x21
>  #define ASC_INV_FIELD_IN_CMD_PACKET          0x24
> +#define ASC_INCOMPATIBLE_FORMAT              0x30
>  #define ASC_MEDIUM_NOT_PRESENT               0x3a
>  #define ASC_SAVING_PARAMETERS_NOT_SUPPORTED  0x39
>  
> @@ -434,6 +435,22 @@
>      int media_changed;
>  } IDEState;
>  
> +/* XXX: DVDs that could fit on a CD will be reported as a CD */
> +static inline int media_present(IDEState *s)
> +{
> +    return (s->nb_sectors > 0);
> +}
> +
> +static inline int media_is_dvd(IDEState *s)
> +{
> +    return (media_present(s) && s->nb_sectors > CD_MAX_SECTORS);
> +}
> +
> +static inline int media_is_cd(IDEState *s)
> +{
> +    return (media_present(s) && s->nb_sectors <= CD_MAX_SECTORS);
> +}
>
>   

I think the automatic probing is good, but we should have a -drive 
parameter that allows the cd type to explicitly be set to either CDROM 
or DVD.  This can be done in a follow-up patch though.  I did some 
testing with Windows and this patch seems to do the right thing.

Tested-by: Anthony Liguori <aliguori@us.ibm.com>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori