On 06/10/2015 11:06 AM, Jiri Denemark wrote:
> On Wed, Jun 10, 2015 at 10:27:11 -0400, John Ferlan wrote:
>>
>>
>> On 06/10/2015 09:42 AM, Jiri Denemark wrote:
>>> QEMU will soon (patches are available on qemu-devel) get support for
>>> migration events which will finally allow us to get rid of polling
>>> query-migrate every 50ms. However, we first need to be able to wait for
>>> all events related to migration (migration status changes, block job
>>> events, async abort requests) at once. This series prepares the
>>> infrastructure and uses it to switch all polling loops in migration code
>>> to pthread_cond_wait.
>>>
>>>
https://bugzilla.redhat.com/show_bug.cgi?id=1212077
>>>
>>> Version 3 (see individual patches for details):
>>> - most of the series has been ACKed in v2
>>> - "qemu: Use domain condition for synchronous block jobs" was
split in 3
>>> patches for easier review
>>> - minor changes requested in v2 review
>>>
>>> Version 2 (see individual patches for details):
>>> - rewritten using per-domain condition variable
>>> - enahnced to fully support the migration events
>>
>> Just ran this through my Coverity checker - only one issue
>>
>> RESOURCE_LEAK in qemuMigrationRun:
>>
>> 4235 if (qemuMigrationCheckJobStatus(driver, vm,
>> 4236 QEMU_ASYNC_JOB_MIGRATION_OUT) <
0)
>>
>> (4) Event if_end: End of if statement
>>
>> 4237 goto cancel;
>> 4238
>>
>> (5) Event open_fn: Returning handle opened by "accept".
>> (6) Event var_assign: Assigning: "fd" = handle returned from
"accept(spec->dest.unix_socket.sock, __SOCKADDR_ARG({ .__sockaddr__ = NULL}),
NULL)".
>> (7) Event cond_false: Condition "(fd =
accept(spec->dest.unix_socket.sock, __SOCKADDR_ARG({ .__sockaddr__ = NULL}), NULL))
< 0", taking false branch
>> Also see events: [leaked_handle]
>>
>> 4239 while ((fd = accept(spec->dest.unix_socket.sock, NULL, NULL))
< 0) {
>> 4240 if (errno == EAGAIN || errno == EINTR)
>> 4241 continue;
>
> Hmm, what an old and unused (except for some ancient QEMU versions) code
> path :-) However, this code is only executed if spec->destType ==
> MIGRATION_DEST_UNIX, which only happens in tunnelled migration path,
> which also sets spec.fwdType = MIGRATION_FWD_STREAM.
>
Placing "sa_assert(spec->fwdType == MIGRATION_FWD_STREAM);" above the
while loop makes Coverity happy.
Feel free to push the sa_assert, it's completely unrelated to this
series and it has been there for ages.
Jirka