John Snow <js...@redhat.com> 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.