Re: [Qemu-devel] [PATCH v3] ide: Add resize callback to ide/core

[Markus Armbruster]

John Snow <address@hidden> writes:

> On 09/04/2014 12:13 PM, Stefan Hajnoczi wrote:
>> This patch seems to break tests/bios-tables-test.c:
>> ERROR:tests/bios-tables-test.c:744:test_acpi_one: assertion failed
>> (signature == SIGNATURE): (0x00000000 == 0x0000dead)
>> GTester: last random seed: R02S3d881198f35228a485b4c3d116dff3b1
>>
>> I have run it many times to make sure the failure is deterministic and
>> I used git-bisect(1) to find this commit as the cause.
>>
>> Not sure why but it seems to break the test.  Please take a look.
>>
>> Dropped from the block branch.
>>
>> Stefan
>>
>
> I've fixed it in a v4, but before I submit and while I'm at it, you
> didn't like the comments I left in the identify functions because they
> were "prone to bitrot." would you prefer I excised them entirely?
>
> I found them helpful because it makes sense to read these functions
> alongside the identify data specs, and excising any references to
> those word indices in their natural order obfuscates the code
> needlessly.
>
> My inclination is to leave them in.

I'd replace them by a pointer to the factored-out code.

> Meanwhile, the bios-tables-test problem is such:
> We serve the identify request out of the io_buffer, not the
> identify_data cache, thus for 2nd and subsequent requests for
> identify_data, we get correct size information, but for the 1st
> request to ide_identify, we get zeroes.

I found that part confusing and stared at it until I believed it works.
Looks like I believed too quickly.

>                                         I corrected this by making
> ide_identify and ide_atapi_identify mimic the flow of
> ide_cfata_identify, which is more clear about the nature of the two
> buffers.

Yup, that one's much better.

> Why this causes a failure here and for only the Q35 machine type I am
> not certain, but this is the causative bug.

My best guess is different firmware behavior.