Re: [Xen-devel] [PATCH] libxl: fix transaction leak in logdirty error path

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2012-07-24 at 17:04 +0100, Ian Campbell wrote:
> On Tue, 2012-07-24 at 17:01 +0100, Ian Jackson wrote:
> > libxl__domain_suspend_common_switch_qemu_logdirty would leak t if
> > there was an error.  Fix this.
> > 
> > Also, document the intended usage for libxl__xs_transaction_* in the
> > doc comment in libxl_internal.h.
> > 
> > Reported-by: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
> > Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> 
> Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Applied, squashing some trailing whitespace as I went.

> 
> > 
> > ---
> >  tools/libxl/libxl_dom.c      |    1 +
> >  tools/libxl/libxl_internal.h |   36 ++++++++++++++++++++++++++++++++++++
> >  2 files changed, 37 insertions(+), 0 deletions(-)
> > 
> > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> > index b6111ad..b8a4aae 100644
> > --- a/tools/libxl/libxl_dom.c
> > +++ b/tools/libxl/libxl_dom.c
> > @@ -627,6 +627,7 @@ void libxl__domain_suspend_common_switch_qemu_logdirty
> >  
> >   out:
> >      LOG(ERROR,"logdirty switch failed (rc=%d), aborting suspend",rc);
> > +    libxl__xs_transaction_abort(gc, &t);
> >      switch_logdirty_done(egc,dss,-1);
> >  }
> >  
> > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> > index e938660..43bb69b 100644
> > --- a/tools/libxl/libxl_internal.h
> > +++ b/tools/libxl/libxl_internal.h
> > @@ -543,6 +543,42 @@ int libxl__xs_rm_checked(libxl__gc *gc, 
> > xs_transaction_t t, const char *path);
> >   *   +1  commit conflict; transaction has been destroyed and caller
> >   *        must go round again (call _start again and retry)
> >   *    0  committed successfully
> > + *
> > + * The intended usage pattern looks like this:
> > + *    int some_function()
> > + *    {
> > + *        int rc;
> > + *        xs_transaction_t t = 0;
> > + *        // other initialisations
> > + *    
> > + *        // do whatever you need to do before the xenstore stuff
> > + *        // errors?  set rc and goto out.
> > + *    
> > + *        for (;;) {
> > + *            rc = libxl__xs_transaction_start(gc, &t);
> > + *            if (rc) goto out;
> > + *    
> > + *            // do your work here, including all xenstore reads and writes
> > + *            // libxl__xs_*_checked are useful; pass them t.
> > + *            // errors?  set rc and goto out.
> > + *    
> > + *            rc = libxl__xs_transaction_commit(gc, &t);
> > + *            if (!rc) break;
> > + *            if (rc<0) goto out;
> > + *        }
> > + *    
> > + *        // now the xenstore transaction succeeded
> > + *        // do whatever else you need to do
> > + *        // errors?  set rc and goto out.
> > + *    
> > + *        return something;
> > + *    
> > + *     out:
> > + *        // other cleanups
> > + *        libxl__xs_transaction_abort(gc, &t);
> > + *        // other cleanups
> > + *        return rc;
> > + *    }
> >   */
> >  int libxl__xs_transaction_start(libxl__gc *gc, xs_transaction_t *t);
> >  int libxl__xs_transaction_commit(libxl__gc *gc, xs_transaction_t *t);
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel