On Wed, Aug 21, 2024 at 12:51:51PM -0400, Jeff Layton wrote: > On Wed, 2024-08-21 at 10:38 -0400, Chuck Lever wrote: > > On Wed, Aug 21, 2024 at 10:22:15AM -0400, Jeff Layton wrote: > > > On Tue, 2024-08-20 at 10:46 -0400, cel@xxxxxxxxxx wrote: > > > > From: Chuck Lever <chuck.lever@xxxxxxxxxx> > > > > > > > > Build an NFSv4 protocol snippet to support the delstid > > > > extensions. > > > > The new fs/nfsd/nfs4_1.x file can be added to over time as other > > > > parts of NFSD's XDR functions are converted to machine-generated > > > > code. > > > > > > > > Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx> > > > > --- > > > > fs/nfsd/nfs4_1.x | 164 +++++++++++++++++++++++++++++ > > > > fs/nfsd/nfs4xdr_gen.c | 236 > > > > ++++++++++++++++++++++++++++++++++++++++++ > > > > fs/nfsd/nfs4xdr_gen.h | 113 ++++++++++++++++++++ > > > > 3 files changed, 513 insertions(+) > > > > create mode 100644 fs/nfsd/nfs4_1.x > > > > create mode 100644 fs/nfsd/nfs4xdr_gen.c > > > > create mode 100644 fs/nfsd/nfs4xdr_gen.h > > > > > > > > > > I see the patches in your lkxdrgen branch. I gave this a try and > > > started rebasing my delstid work on top of it, but I hit the same > > > symbol conflicts I hit before once I started trying to include the > > > full-blown nfs4xdr_gen.h header: > > > > > > ------------------------8<--------------------------- > > > In file included from fs/nfsd/nfs4xdr.c:58: > > > fs/nfsd/nfs4xdr_gen.h:86:9: error: redeclaration of enumerator > > > ‘FATTR4_OPEN_ARGUMENTS’ > > > 86 | FATTR4_OPEN_ARGUMENTS = 86 > > > | ^~~~~~~~~~~~~~~~~~~~~ > > > In file included from fs/nfsd/nfsfh.h:15, > > > from fs/nfsd/state.h:41, > > > from fs/nfsd/xdr4.h:40, > > > from fs/nfsd/nfs4xdr.c:51: > > > ./include/linux/nfs4.h:518:9: note: previous definition of > > > ‘FATTR4_OPEN_ARGUMENTS’ with type ‘enum <anonymous>’ > > > 518 | FATTR4_OPEN_ARGUMENTS = 86, > > > | ^~~~~~~~~~~~~~~~~~~~~ > > > fs/nfsd/nfs4xdr_gen.h:102:9: error: redeclaration of enumerator > > > ‘FATTR4_TIME_DELEG_ACCESS’ > > > 102 | FATTR4_TIME_DELEG_ACCESS = 84 > > > | ^~~~~~~~~~~~~~~~~~~~~~~~ > > > ./include/linux/nfs4.h:516:9: note: previous definition of > > > ‘FATTR4_TIME_DELEG_ACCESS’ with type ‘enum <anonymous>’ > > > 516 | FATTR4_TIME_DELEG_ACCESS = 84, > > > | ^~~~~~~~~~~~~~~~~~~~~~~~ > > > fs/nfsd/nfs4xdr_gen.h:106:9: error: redeclaration of enumerator > > > ‘FATTR4_TIME_DELEG_MODIFY’ > > > 106 | FATTR4_TIME_DELEG_MODIFY = 85 > > > | ^~~~~~~~~~~~~~~~~~~~~~~~ > > > ./include/linux/nfs4.h:517:9: note: previous definition of > > > ‘FATTR4_TIME_DELEG_MODIFY’ with type ‘enum <anonymous>’ > > > 517 | FATTR4_TIME_DELEG_MODIFY = 85, > > > | ^~~~~~~~~~~~~~~~~~~~~~~~ > > > ------------------------8<--------------------------- > > > > > > I'm not sure of the best way to work around this, unless we want to > > > try > > > to split up nfs4.h. > > > > That header is shared with the client, so I consider it immutable > > for our purposes here. > > > > > > One option would be to namespace the generated data items. Eg, name > > them: > > > > XG_FATTR4_TIME_DELEG_ACCESS > > XG_FATTR4_TIME_DELEG_MODIFY > > > > That way they don't conflict with existing definitions. > > > > I'd rather avoid that if we can. Having things named exactly the same > as in the spec makes the code easier to read and understand. Agreed, matching names is one of the reasons I started this work. My thought was that the prefixed names could be fixed eventually after the hand-rolled code has been replaced. > What might be best is to autogenerate a header from a (combined) > nfs4.x, and then have the old nfs4.h file include it. Then we'd just > have to eliminate anything from nfs4.h that conflicts with the > autogenerated symbols. > > That's definitely not treating include/linux/nfs4.h as immutable, but > we might still be able to avoid a lot of changes to the client code > that way. Include the current nfs4xdr_gen.h in nfs4.h, and then remove from nfs4.h anything that has a naming conflict? The downside of that is that approach mixes generated and hand-rolled code. Not an objection from me, but it was something that made Neil uncomfortable. > We'd need Trond and Anna to buy in on that though. Or the client can continue to use include/linux/nfs4.h and the server can start using its own copy of that header. -- Chuck Lever
