qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: Kevin Wolf <kwolf@redhat.com>, qemu-devel <qemu-devel@nongnu.org>,
	Markus Armbruster <armbru@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v3] ide: Add resize callback to ide/core
Date: Thu, 04 Sep 2014 13:54:54 -0400	[thread overview]
Message-ID: <5408A76E.1080501@redhat.com> (raw)
In-Reply-To: <CAJSP0QUUz+3aoYnQWjP0rBLMRxhvNifobMtPMcU68Sadj578ig@mail.gmail.com>



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.

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 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.

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

--j

  reply	other threads:[~2014-09-04 17:55 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-14 21:03 [Qemu-devel] [PATCH v3] ide: Add resize callback to ide/core John Snow
2014-08-15  7:55 ` Markus Armbruster
2014-08-18 13:12 ` Stefan Hajnoczi
2014-08-19 19:20   ` John Snow
2014-08-20 14:27     ` Stefan Hajnoczi
2014-09-04 16:13       ` Stefan Hajnoczi
2014-09-04 17:54         ` John Snow [this message]
2014-09-05  9:02           ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5408A76E.1080501@redhat.com \
    --to=jsnow@redhat.com \
    --cc=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).