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

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.

... > (28) Event cond_false: Condition "spec->fwdType != MIGRATION_FWD_DIRECT", taking false branch > > 4289 if (spec->fwdType != MIGRATION_FWD_DIRECT) { > 4290 if (iothread && qemuMigrationStopTunnel(iothread, ret < 0) < 0) > 4291 ret = -1; > 4292 VIR_FORCE_CLOSE(fd); Which means "spec->fwdType != MIGRATION_FWD_DIRECT" will be true and fd will be correctly closed. Jirka