Re: [Qemu-devel] [PULL v3 00/51] Ivshmem patches

[John Snow <jsnow@redhat.com>]

On 10/13/2015 12:10 PM, Marc-André Lureau wrote:
> Hi
> 
> ----- Original Message -----
>> On 13 October 2015 at 15:25,  <marcandre.lureau@redhat.com> wrote:
>>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>
>>> The following changes since commit
>>> c49d3411faae8ffaab8f7e5db47405a008411c10:
>>>
>>>   Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2015-10-12'
>>>   into staging (2015-10-13 10:42:06 +0100)
>>>
>>> are available in the git repository at:
>>>
>>>   git@github.com:elmarco/qemu.git tags/ivshmem-pull-request
>>>
>>> for you to fetch changes up to feb3f96c4ff1613dd4d0bebda09fe349f8c3e2dd:
>>>
>>>   doc: document ivshmem & hugepages (2015-10-13 15:29:53 +0200)
>>>
>>> ----------------------------------------------------------------
>>> v3 with build fixes on osx & x86
>>> ----------------------------------------------------------------
>>
>> This asserts in the tests on OSX:
>>
>> GTESTER check-qtest-i386
>> blkdebug: Suspended request 'A'
>> blkdebug: Resuming request 'A'
>> Using POSIX shared memory: /qtest-68262-3644687833
>> ftruncate(/qtest-68262-3644687833) failed: Invalid argument
>> **
> 
> I'll try to reproduce on freebsd. It's weird that this ftruncate() would fail on osx but not on linux, perhaps a osx security?
> 
>> ERROR:/Users/pm215/src/qemu-for-merges/tests/ivshmem-test.c:299:void
>> test_ivshmem_server(): assertion failed (ret == 0): (-1 == 0)
>> GTester: last random seed: R02S141a4c6774f852248b61ebcd666b7ad5
>>
>> (I'm afraid I didn't notice this in earlier testing because
>> for some reason I'm not clear on an assertion failure doesn't
>> always cause the test harness to fail.)
>>
>> Some asides, which you should look into but which don't need
>> to be fixed for this pull request:
>>  * having the test use 'is QTEST_LOG set' as its "should we be verbose
>> in the server failure path" is not terribly helpful because QTEST_LOG
>> enables vast volumes of libqtest tracing of communications between
>> qemu and the test harness, and anything else is lost in the noise
> 
> What do you suggest instead?
> 

https://developer.gnome.org/glib/stable/glib-Testing.html#g-test-verbose

>>  * ivshmem_server_init() has uses of IVSHMEM_SERVER_DEBUG before the
>> verbose flag is copied into server->verbose, which means they won't
>> print things out when they should
> 
> True, I moved it up.
> 
>>  * ivshmem_server_start() is inconsistent about whether it wants
>> to report "something failed messages to stderr or via the debug macro
> 
> stderr is for fatal errors. Only gethugepagesize() that I added uses stderr too because it's a generic function. I'll make it take IvshmemServer argument and use IVSHMEM_SERVER_DEBUG.
> 
>>  * ivshmem_server_ftruncate() is using some bizarre code to
>> align up to a power of 2. We have pow2ceil() for this
> 
> ok
> 
>>  * Printing "Using POSIX shared memory" in the test output for a
>> normal non-verbose test run isn't great: generally our tests should
>> be silent except regarding failures
> 
> This was added by earlier reviewer request. I guess during make check, there could be a way to make it silent instead.
> 
> thanks
>