On Thu, Jul 13, 2017 at 06:34:33AM -0700, Ben Warren wrote: > Hi, > > On Jul 13, 2017, at 4:51 AM, Marc-André Lureau > <marcandre.lur...@gmail.com> > wrote: > > Hi > > On Thu, Jul 13, 2017 at 1:32 PM Laszlo Ersek <ler...@redhat.com> wrote: > > On 07/13/17 12:47, Peter Maydell wrote: > > On 12 July 2017 at 00:43, Ben Warren <b...@skyportsystems.com> > wrote: > >> Yes, it’s definitely a setup time problem. With the values that > are > checked > >> in, I can’t get it to fail on my setup, but if I wind the numbers > down I see > >> the same failure as Peter. So now we have the ages-old problem of > “what new > >> arbitrary value should I use that will not cause false failures but > will > >> eventually time out”. > > > > Empirically, we already have an answer to this, in the form > > of the existing code in tests/boot-sector.c, which is used > > by both that test and the bios-tables-test.c code to wait > > for the BIOS initialization to complete, and which doesn't > > cause false test timeouts in practice. > > > > Can we make this test just use that existing function to > > wait for the BIOS to be done, rather than having its own > > timeout loop? > > This is incredibly cool. Now that I've looked at "tests/boot-sector.c" > (again), I recall having seen it earlier, but I couldn't have > remembered > it off-hand. > > Perhaps this boot sector code should be factored out and moved to > "tests/acpi-utils". > > Marc-André, do you think it would be feasible for the vmcoreinfo unit > test as well? (Which is derived from the vmgenid unit test.) > > > It seems likely. > > Ben, are you going to work on the fix? > > > Yes, I will take care of this. I remember seeing the boot-sector > synchronization code before, but didn’t really grok it. This time I’ll dig > deeper.
It's already a library, I don't think you need changes or refactoring. You can just link boot_sector.c and call boot_sector_test like the pxe test does. If this passes you know all acpi tables are in memory. I'll post a patch. There's no clever logic around timeout there though - right now your timeout is 10 seconds while boot_sector_test has a timeout of 90 seconds. So it is still worth improving to check the per-thread clock imho, but I agree it's best to rework vm gen id test. While at it, I think it's best to drop the assumption that vmgenid is the 1st table in the ssdt. > Thanks > -- > Marc-André Lureau > > > —Ben >