On Mon, Aug 11, 2008 at 12:25:52PM +0200, Jim Meyering wrote:
"Daniel P. Berrange" <berrange@redhat.com> wrote:
This patch does some simple re-factoring of the way the TTYs and control socket are handled to reduce the amount of state stored in the lxc_vm_t structure, in preparation for the switchover to the generic domain handling APIs. ... diff -r 63b8398c302e src/lxc_driver.c --- a/src/lxc_driver.c Mon Jul 14 12:18:23 2008 +0100 +++ b/src/lxc_driver.c Tue Jul 15 11:55:48 2008 +0100 ... @@ -989,15 +896,18 @@ lxc_vm_t * vm) { int rc = -1; - lxc_vm_def_t *vmDef = vm->def; + int sockpair[2]; ... + if (lxcOpenTty(conn, &parentTty, &vm->def->tty, 1) < 0) { goto cleanup; }
/* open container tty */ - if (lxcSetupContainerTty(conn, &(vm->containerTtyFd), &(vm->containerTty)) < 0) { + if (lxcOpenTty(conn, &containerTty, &containerTtyPath, 0) < 0) { goto cleanup; }
...
+ if (0 != socketpair(PF_UNIX, SOCK_STREAM, 0, sockpair)) { ... cleanup: - close(vm->sockpair[LXC_PARENT_SOCKET]); - vm->sockpair[LXC_PARENT_SOCKET] = -1; - close(vm->sockpair[LXC_CONTAINER_SOCKET]); - vm->sockpair[LXC_CONTAINER_SOCKET] = -1; + close(sockpair[0]); + close(sockpair[1]); + VIR_FREE(containerTtyPath);
return rc; }
All looks fine except for the possibility that the cleanup code can close undefined sockpair[0,1]. The obvious fix is to initialize them to -1 and not close in that case.
Yep, I've made that change & committed this.
Oh, and the new name, "monitor" (new struct member and local/param in several functions) would be more readable as "monitor_fd".
The struct member will be going away in later re-factoring so I've not changed this naming Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|