[libvirt] [PATCH v3 00/24] Add support for migration events

[Jiri Denemark]

On Wed, Jun 10, 2015 at 11:16:29 -0400, John Ferlan wrote:
> 
> 
> 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