Re: [libvirt] PATCH: 1/7: Removing state from lxc_vm_t

"Daniel P. Berrange" <berrange(a)redhat.com&gt; 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.

Oh, and the new name, "monitor" (new struct member and local/param in several functions) would be more readable as "monitor_fd".