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

[Ian Campbell <Ian.Campbell@citrix.com>]

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?

Ian.