On Wed, 2024-08-21 at 13:03 -0400, Chuck Lever wrote: > 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. > Not exactly. The hand-rolled code would become dependent on the generated headers, but we shouldn't need make changes to the generated headers themselves. > > > 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. > Sure, that would work too. -- Jeff Layton <jlayton@xxxxxxxxxx>
