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

[George Dunlap <george.dunlap@citrix.com>]

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@eu.citrix.com>
> > > > 
> > > > 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?

 -George

> 
> Ian.
>