Re: [Xen-devel] [PATCH 20 of 26] xl: free all data on exit from the domain monitor daemon

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Tue, 2010-08-17 at 13:27 +0100, Stefano Stabellini wrote:
> On Mon, 16 Aug 2010, Ian Campbell wrote:
> > # HG changeset patch
> > # User Ian Campbell <ian.campbell@xxxxxxxxxx>
> > # Date 1281969065 -3600
> > # Node ID dd25ac29c0f74a2d909df3e3d072ed5d59320312
> > # Parent  a33d1901115bfeceab9cad25954472acdd475744
> > xl: free all data on exit from the domain monitor daemon.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > 
> > diff -r a33d1901115b -r dd25ac29c0f7 tools/libxl/xl_cmdimpl.c
> > --- a/tools/libxl/xl_cmdimpl.c      Mon Aug 16 15:31:05 2010 +0100
> > +++ b/tools/libxl/xl_cmdimpl.c      Mon Aug 16 15:31:05 2010 +0100
> > @@ -1581,9 +1581,11 @@ start:
> >                  if (ret) {
> >                      switch (handle_domain_death(&ctx, domid, &event, 
> > &d_config, &info)) {
> >                      case 2:
> > -                        if (!preserve_domain(&ctx, domid, &event, 
> > &d_config, &info))
> > +                        if (!preserve_domain(&ctx, domid, &event, 
> > &d_config, &info)) {
> >                              /* If we fail then exit leaving the old domain 
> > in place. */
> > -                            exit(-1);
> > +                            ret = -1;
> > +                            goto out;
> > +                        }
> >  
> >                          /* Otherwise fall through and restart. */
> >                      case 1:
> > @@ -1601,6 +1603,8 @@ start:
> >                          goto start;
> >                      case 0:
> >                          LOG("Done. Exiting now");
> > +                        ret = 0;
> > +                        goto out;
> >                          exit(0);
> >                      }
> >                  }
> 
> you can remove the exit(0) from there

oops, yes indeed.

> 
> 
> > @@ -1612,9 +1616,6 @@ start:
> >          }
> >          libxl_free_event(&event);
> >      }
> > -
> > -    close(logfile);
> > -    exit(0);
> >  
>  
> shouldn't the close(logfile) be moved to the out path?

I guess it should be done somewhere but as far as I can tell the code in
the current location was never reachable.

I think it needs an out_closelog label which does the closing since the
regular out label is used from places which do not open the logfile.
I'll make that change.

Ian.




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel