Re: [Xen-devel] [PATCH 2 of 2] libxc: Print domain ID in save restore messages

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2012-03-12 at 10:37 +0000, George Dunlap wrote:
> On Fri, 2012-03-09 at 18:33 +0000, Ian Campbell wrote:
> > On Fri, 2012-03-09 at 13:25 -0500, George Dunlap wrote:
> > > On Fri, 2012-03-09 at 18:19 +0000, Ian Campbell wrote:
> > > > On Fri, 2012-03-09 at 12:29 -0500, George Dunlap wrote:
> > > > > XenServer redirects all libxc output to /var/log/messages, so when a
> > > > > large stress test is running, it's hard to tell which message belongs
> > > > > to which domain.
> > > > > 
> > > > > This patch adds the domain ID to output made by the save/restore
> > > > > functions.
> > > > > 
> > > > > To do this, we introduce a layer of indirection in the libxc print
> > > > > macros, so that they can be "interposed on" by individual functions.
> > > > > 
> > > > > We then add the domain ID to the context structs in the save and 
> > > > > restore
> > > > > paths, and replace the toplevel macros with ones which add the domain 
> > > > > to the
> > > > > output.
> > > > > 
> > > > > Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> > > > > 
> > > > > diff --git a/tools/libxc/xc_domain_restore.c 
> > > > > b/tools/libxc/xc_domain_restore.c
> > > > > --- a/tools/libxc/xc_domain_restore.c
> > > > > +++ b/tools/libxc/xc_domain_restore.c
> > > > > @@ -33,7 +33,20 @@
> > > > >  #include <xen/hvm/ioreq.h>
> > > > >  #include <xen/hvm/params.h>
> > > > >  
> > > > > +/* Override default output f'ns with functions that include the 
> > > > > domain id */
> > > > > +#undef IPRINTF
> > > > > +#define IPRINTF(_F, _A...) _IPRINTF("d%d:" _F, ctx->dom, ## _A)
> > > > 
> > > > I think
> > > > 
> > > > #define DIPRINTF(_F, _A...) IPRINTF("d%s:" _F, ctx->dom, ## _A)
> > > > 
> > > > (or DOMIPRINTF etc) would end up looking nicer than these undef and the
> > > > _IPRINTFs you've had to scatter around the place.
> > > 
> > > Perhaps; but that also requires that every person changing this file
> > > forever more must remember to write "DIPRINTF" instead of "IPRINTF".
> > 
> > They already have to remember to write IPRINTF instead of printf(), most
> > people will just copy whatever is used nearby, whether that is IPRINTF,
> > _IPRINTF or DIPRINTF...
> > 
> > > (And it requires me to change 10x as many LoC.)
> > 
> > That's irritating but not a show stopper IMHO.
> > 
> > Coming from the other angle can you omit all uses of _IPRINTF by passing
> > the context around a few more places? I'd have expected that everything
> > in xc_domain_save.c was ultimately called from xc_domain_save and
> > therefore the is a dom which could be printed?
> 
> There's a non-static function xc_map_m2p() which is defined in
> xc_domain_save.c, but called from xc_offline_page.c and
> tools/tests/mce-test/tools/xce-mceinj.c.  That should probably then be
> moved to another file in any case.
> 
> If I move that function to a different file, so that there are no
> _IPRINTF's, would that suffice?

I don't much like the undef'ery stuff but I guess it'll do.

Ian.



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