Re: [Qemu-devel] [PATCH 04/35] atapi: fill in AUDIO_CTL page correctly

[Kevin Wolf]

Am 17.10.2011 16:04, schrieb Paolo Bonzini:
> On 10/17/2011 04:05 PM, Kevin Wolf wrote:
>>>>  diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
>>>>  index 347c38d..10f161f 100644
>>>>  --- a/hw/ide/atapi.c
>>>>  +++ b/hw/ide/atapi.c
>>>>  @@ -731,6 +731,8 @@ static void cmd_mode_sense(IDEState *s, uint8_t *buf)
>>>>                buf[7] = 0;
>>>>
>>>>                /* Fill with CDROM audio volume */
>>>>  +            buf[8] = 0x0e;
>>
>> 0x0e would be more readable if it was written as MODE_PAGE_AUDIO_CTL.
>>
>>>>  +            buf[9] = 0x0e;
>>
>> And this is really the length. But I'm not sure if writing it as 24 - 10
>> would help a lot.
> 
> Yes, the coincidence is ugly.

Maybe we should define structs for the mode pages so that you have real
names instead of just offsets into the output buffer?

>> Of course the same is true for the other pages, so it could be done for
>> all of them on top. Or maybe you actually have included it later in the
>> series?
> 
> Only for SCSI.  I can queue it as a cleanup for ATAPI.

That would be good.

Kevin