On Mon 28 Oct 2013 05:11:19 PM EDT, J. Bruce Fields wrote: > On Mon, Oct 28, 2013 at 08:59:59PM +0000, Myklebust, Trond wrote: >>> -----Original Message----- >>> From: linux-nfs-owner@xxxxxxxxxxxxxxx [mailto:linux-nfs- >>> owner@xxxxxxxxxxxxxxx] On Behalf Of J. Bruce Fields >>> Sent: Monday, October 28, 2013 4:54 PM >>> To: Schumaker, Bryan >>> Cc: linux-nfs@xxxxxxxxxxxxxxx >>> Subject: Re: [PATCH 2/4] NFSD: Create nfs v4.2 decode ops >>> >>> On Mon, Oct 28, 2013 at 10:57:24AM -0400, Anna Schumaker wrote: >>>> I'm doing this in a separate patch to keep from putting in a lot of >>>> extra code when I go to add operations to the server for real. >>> >>> Makes sense. >>> >>> But: now we're duplicating the list of 4.0 op decoders 3 times and the >>> 4.1 ops twice. We'll never need different decoders for different >>> minorversions (worst case we can test for the minorversion in the decoder if >>> necessary). >>> >>> I wonder if there's a better way to organize this.... Maybe something more >>> like a single array with >>> >>> [OP_SETCLIENTID] = { >>> .op_decode = (nfsd4_dec)nfsd4_decode_access, >>> .op_unsupported_since_version = 1, >>> } >>> ... >>> [OP_EXCHANGE_ID] = { >>> .op_decode = (nfsd4_dec)nfsd4_decode_exchange_id, >>> .op_first_supported_in_version = 1, >>> } >>> >>> ? >> >> Is that really necessary? Why not just have a single array and have nfsd4_decode_clientid itself check the minor version? > > That'd work too. Every operation that's been introduced more recently > than 4.0 or that's since been deprecated would need a version check at > the top of its decoder. That'd be a dozen or so. > > But the information has to go somewhere and perhaps that's more > straightforward than sticking this in data.... OK, I'd be fine with > that. Makes sense. Do you want me to put this in with this patch series or do it as something separate either before or after? > > --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
