[pnfs] [PATCH 6/8] nfsd: dprint operation names

[J. Bruce Fields]

On Mon, Jun 30, 2008 at 04:22:41PM -0400, bfields wrote:
> On Mon, Jun 30, 2008 at 11:16:16PM +0300, Benny Halevy wrote:
> > J. Bruce Fields wrote:
> > > On Mon, Jun 30, 2008 at 09:08:15PM +0300, Benny Halevy wrote:
> > >> Signed-off-by: Benny Halevy <bhalevy at panasas.com>
> > >> ---
> > >>  fs/nfsd/nfs4proc.c |   57 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> > >>  1 files changed, 56 insertions(+), 1 deletions(-)
> > >>
> > >> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > >> index f3226c0..28ae1dc 100644
> > >> --- a/fs/nfsd/nfs4proc.c
> > >> +++ b/fs/nfsd/nfs4proc.c
> > >> @@ -51,6 +51,59 @@
> > >>  
> > >>  #define NFSDDBG_FACILITY		NFSDDBG_PROC
> > >>  
> > >> +#ifdef OP
> > >> +#error OP is defined
> > >> +#endif
> > >> +#define OP(op) [OP_ ## op] = #op
> > >> +char *nfsd4_op_names[] = {
> > >> +	OP(ACCESS),
> > > 
> > > I'm not sure how the macro helps--wouldn't
> > > 
> > > 	"OP_ACCESS"
> > > 
> > > be just as nice here?
> > 
> > Do you mean, use no macro and code this as
> > 	[OP_ACCESS] = "OP_ACCESS",
> > 
> > or define OP as follows?
> > #define OP(op) [OP_ ## op] = "OP_" #op
> > 
> > I'm OK either way...
> 
> Cripes--I missed the need for the index on the left side of the
> assignment.  OK, makes sense!
> 
> The #ifdef/#error seems like mild paranoia--we use similarly generic
> macro names ("PROC") locally elsewhere and it hasn't caused problem--but
> I don't really have a problem with it if you prefer it that way.

Hm. If we're sharing nfsd4_ops array with v4 anyway, I wonder if it'd
make sense just to add an op_name field to the nfsd4_operation struct
instead of defining a separate array?

(But if we do that I'd rather avoid the temptation to create another
macro like PROC()--that confuses everyone who greps for the caller of
one of these functions.)

--b.

> 
> --b.
> 
> > 
> > Benny
> > 
> > > 
> > > --b.
> > > 
> > >> +	OP(CLOSE),
> > >> +	OP(COMMIT),
> > >> +	OP(CREATE),
> > >> +	OP(DELEGPURGE),
> > >> +	OP(DELEGRETURN),
> > >> +	OP(GETATTR),
> > >> +	OP(GETFH),
> > >> +	OP(LINK),
> > >> +	OP(LOCK),
> > >> +	OP(LOCKT),
> > >> +	OP(LOCKU),
> > >> +	OP(LOOKUP),
> > >> +	OP(LOOKUPP),
> > >> +	OP(NVERIFY),
> > >> +	OP(OPEN),
> > >> +	OP(OPENATTR),
> > >> +	OP(OPEN_CONFIRM),
> > >> +	OP(OPEN_DOWNGRADE),
> > >> +	OP(PUTFH),
> > >> +	OP(PUTPUBFH),
> > >> +	OP(PUTROOTFH),
> > >> +	OP(READ),
> > >> +	OP(READDIR),
> > >> +	OP(READLINK),
> > >> +	OP(REMOVE),
> > >> +	OP(RENAME),
> > >> +	OP(RENEW),
> > >> +	OP(RESTOREFH),
> > >> +	OP(SAVEFH),
> > >> +	OP(SECINFO),
> > >> +	OP(SETATTR),
> > >> +	OP(SETCLIENTID),
> > >> +	OP(SETCLIENTID_CONFIRM),
> > >> +	OP(VERIFY),
> > >> +	OP(WRITE),
> > >> +	OP(RELEASE_LOCKOWNER),
> > >> +};
> > >> +#undef OP
> > >> +
> > >> +static inline char *
> > >> +nfsd4_op_name(unsigned opnum)
> > >> +{
> > >> +	if (opnum < ARRAY_SIZE(nfsd4_op_names))
> > >> +		return nfsd4_op_names[opnum];
> > >> +	return "unknown_operation";
> > >> +}
> > >> +
> > >>  static inline void
> > >>  fh_dup2(struct svc_fh *dst, struct svc_fh *src)
> > >>  {
> > >> @@ -888,7 +941,9 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
> > >>  	while (!status && resp->opcnt < args->opcnt) {
> > >>  		op = &args->ops[resp->opcnt++];
> > >>  
> > >> -		dprintk("nfsv4 compound op #%d: %d\n", resp->opcnt, op->opnum);
> > >> +		dprintk("nfsv4 compound op #%d/%d: %d (%s)\n",
> > >> +			resp->opcnt, args->opcnt, op->opnum,
> > >> +			nfsd4_op_name(op->opnum));
> > >>  
> > >>  		/*
> > >>  		 * The XDR decode routines may have pre-set op->status;
> > >> -- 
> > >> 1.5.6.GIT
> > >>
> >