[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]