On Tue, 2021-04-20 at 14:09 +0000, Chuck Lever III wrote: > > > > On Apr 20, 2021, at 9:11 AM, Steve Dickson <SteveD@xxxxxxxxxx> > > wrote: > > > > > > > > On 4/18/21 12:51 PM, Chuck Lever III wrote: > > > > > > > > > > On Apr 17, 2021, at 1:50 PM, Steve Dickson <SteveD@xxxxxxxxxx> > > > > wrote: > > > > > > > > > > > > > > > > On 4/17/21 12:36 PM, Chuck Lever III wrote: > > > > > > > > > > > > > > > > On Apr 17, 2021, at 12:18 PM, Steve Dickson < > > > > > > SteveD@xxxxxxxxxx> wrote: > > > > > > > > > > > > > > > > > > > > > > > > On 4/15/21 12:37 PM, Chuck Lever III wrote: > > > > > > > > > > > > > > > > > > > > > > On Apr 15, 2021, at 11:33 AM, Steve Dickson < > > > > > > > > SteveD@xxxxxxxxxx> wrote: > > > > > > > > > > > > > > > > Hey Chuck! > > > > > > > > > > > > > > > > On 4/14/21 7:26 PM, Chuck Lever III wrote: > > > > > > > > > Hi Steve- > > > > > > > > > > > > > > > > > > > On Apr 14, 2021, at 2:10 PM, Steve Dickson < > > > > > > > > > > SteveD@xxxxxxxxxx> wrote: > > > > > > > > > > > > > > > > > > > > This is a tweak of the patch set Alice Mitchell > > > > > > > > > > posted last July [1]. > > > > > > > > > > > > > > > > > > That approach was dropped last July because it is not > > > > > > > > > container-aware. > > > > > > > > > It should be simple for someone to write a udev > > > > > > > > > script that uses the > > > > > > > > > existing sysfs API that can update nfs4_client_id in > > > > > > > > > a namespace. I > > > > > > > > > would prefer the sysfs/udev approach for setting > > > > > > > > > nfs4_client_id, > > > > > > > > > since it is container-aware and makes this setting > > > > > > > > > completely > > > > > > > > > automatic (zero touch). > > > > > > > > As I said in in my cover letter, I see this more as > > > > > > > > introduction of > > > > > > > > a mechanism more than a way to set the unique id. > > > > > > > > > > > > > > Yep, I got that. > > > > > > > > > > > > > > I'm not addressing the question of whether adding a > > > > > > > mechanism to set a module parameter in nfs.conf is good > > > > > > > or not. I'm saying nfs4_client_id is not an appropriate > > > > > > > parameter to add to nfs.conf. Can you pick another > > > > > > > module parameter as an example for your mechanism? > > > > > > The request was to set that parameter... > > > > > > > > > > The cover letter and the quoted e-mail above state that > > > > > you are just demonstrating a mechanism to set module > > > > > parameters via nfs.conf. I guess that statement was not > > > > > entirely accurate, then. Thanks for clarifying. > > > > I was trying to keep the conversation off of what > > > > was being set to how it was being set... > > > > > > > > I had no idea the entire "options nfs" API is compromised > > > > when it comes to containers... Not sure why... but I will > > > > believe you... > > > > > > Module parameters take effect for all namespaces. It's > > > not just nfs.ko that has this issue. > > Right... > > > > > > > > > > > If there's a bug report somewhere that requests a > > > > > feature, it's normal practice to include a URL pointing > > > > > to that report in the patch description. > > > > I just assumed, since it had a customer case, the bz was > > > > private... it turns out not to be the case > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1801326 > > > > > > > > > > > Actual I have customers asking for this type > > > > > > > > of functionality > > > > > > Hrm. The reporter of 1801326 is dwyso, a Red Hat employee, > > > not a customer. > > > > > > Also, I see nothing that requires it to be set in nfs.conf. > > > So what actual functionality is being requested, and why > > > can't it be addressed with a udev script, which has been > > > the design already in the works for many months? > > The bz was open by a request of a customer... There is a > > lot of info in bzs that are not public... > > > > Having a one, centralized place to configure NFS > > I thought was a good idea... > > > > > > > > > > > > > > > Ask yourself why they might want it. It's probably > > > > > > > because > > > > > > > we don't set it correctly currently. If we have a way to > > > > > > > automatically get it right every time, there's really no > > > > > > > need for this setting to be exposed. > > > > > > If we shouldn't expose it... Lets get rid of it... > > > > > > You added the param in the fall 2012... If it hasn't > > > > > > been used correctly or can't be used correctly for > > > > > > all theses years... why does it exist? > > > > > > > > > > Because back then we didn't care about container awareness > > > > > enough to make it an initial part of the feature. We were > > > > > trying to address the problem of how to ensure that the > > > > > nfs4_client_id is unique. But clearly it was only half a > > > > > solution. > > > > Right... I was just trying to build a mechanism to > > > > set the value, and not worrying (yet) about what the > > > > value is set to... > > > > > > > > So at this point, all of the nfs kernel module parameter > > > > can be set through the sysfs/udev interface? > > > > > > The only module parameter that has been explicitly replaced > > > is the one that sets nfs4_client_id, as far as I am aware. > > > There might be some other settings available in /sys: > > > > > > # ls /sys/module/nfsv4/parameters > > > delegation_watermark layoutstats_timer > > > # > > > > > > [cel@manet linux]$ git grep MODULE_PARM -- fs/nfs/ > > > fs/nfs/cache_lib.c:MODULE_PARM_DESC(cache_getent, "Path to the > > > client cache upcall program"); > > > fs/nfs/cache_lib.c:MODULE_PARM_DESC(cache_getent_timeout, > > > "Timeout (in seconds) after which " > > > fs/nfs/dir.c:MODULE_PARM_DESC(nfs_access_max_cachesize, "NFS > > > access maximum total cache length"); > > > fs/nfs/filelayout/filelayoutdev.c:MODULE_PARM_DESC(dataserver_ret > > > rans, "The number of times the NFSv4.1 client " > > > fs/nfs/filelayout/filelayoutdev.c:MODULE_PARM_DESC(dataserver_tim > > > eo, "The time (in tenths of a second) the " > > > fs/nfs/flexfilelayout/flexfilelayout.c:MODULE_PARM_DESC(io_maxret > > > rans, "The number of times the NFSv4.1 client " > > > fs/nfs/flexfilelayout/flexfilelayoutdev.c:MODULE_PARM_DESC(datase > > > rver_retrans, "The number of times the NFSv4.1 client " > > > fs/nfs/flexfilelayout/flexfilelayoutdev.c:MODULE_PARM_DESC(datase > > > rver_timeo, "The time (in tenths of a second) the " > > > fs/nfs/namespace.c:MODULE_PARM_DESC(nfs_mountpoint_expiry_timeout > > > , > > > fs/nfs/super.c:MODULE_PARM_DESC(callback_nr_threads, "Number of > > > threads that will be " > > > fs/nfs/super.c:MODULE_PARM_DESC(nfs4_disable_idmapping, > > > fs/nfs/super.c:MODULE_PARM_DESC(max_session_slots, "Maximum > > > number of outstanding NFSv4.1 " > > > fs/nfs/super.c:MODULE_PARM_DESC(max_session_cb_slots, "Maximum > > > number of parallel NFSv4.1 " > > > fs/nfs/super.c:MODULE_PARM_DESC(send_implementation_id, > > > fs/nfs/super.c:MODULE_PARM_DESC(nfs4_unique_id, "nfs_client_id4 > > > uniquifier string"); > > > fs/nfs/super.c:MODULE_PARM_DESC(recover_lost_locks, > > > [cel@manet linux]$ git grep MODULE_PARM -- fs/nfsd/ > > > fs/nfsd/nfs4idmap.c:MODULE_PARM_DESC(nfs4_disable_idmapping, > > > fs/nfsd/nfs4proc.c:MODULE_PARM_DESC(inter_copy_offload_enable, > > > fs/nfsd/nfs4recover.c:MODULE_PARM_DESC(cltrack_prog, "Path to the > > > nfsdcltrack upcall program"); > > > fs/nfsd/nfs4recover.c:MODULE_PARM_DESC(cltrack_legacy_disable, > > > [cel@manet linux]$ > > > > > > Each of the above has to be considered on a case-by-case > > > basis, IMO. > > > > > > > > > > If that is the cause... have the variables in > > > > nfs.conf create sysfs/udev file would work better? > > > > > > Seems to me we have the same argument for a separate file > > > to store the nfs4_unique_id that we have for breaking > > > /etc/exports into a directory of individual files: no > > > parsing is necessary. Scripts and applications can simply > > > read the string right out of the file, or update it just > > > by writing the string into that file. > > I'm sure I'm following this analogy with the export... > > but being able to set things up from one configuration > > file and command is key. > > I find that to be a red herring. We're not ever going to be > at a place where everything is configured in one file. Are > you going to replace /etc/exports.d and automounter.conf > and krb5.conf and all the other things with just /etc/nfs.conf? > Probably not. So let's stop using this strange logic to > insist that /etc/nfs.conf is the only place for the clientid > uniqifier. > > Please, let's just focus on what's right for this one setting. > > > > nfsconf does an excellent job parsing nfs.conf and I would > > like to build on that... > > Just because it is technically possible to save the uniqifier > in /etc/nfs.conf doesn't mean that's what's best for our users. > We're in better shape if we can guarantee that setting is > correct and administrators can't break it. > > > > I understand we have to work well in containers which > > is one of the reason I was trying to come up with a > > nfsv4 only nfs-utils... That went over like a lead balloon ;-) > > I didn't have a problem with an nfsv4-only configuration. > What I didn't like about that is that you were making the > administrative interface more complex, and it didn't need to > be. > > > > What I don't understand is why we can't come up with a > > solution that uniquely set a param that is set by > > nfsconf using nfs.conf. > > Once we have an automated mechanism to set the uniqifier, > it does not need to be set by humans. Let's keep it out of > nfs.conf. > > I'm in favor of making this as automatic as possible. No > setting is better than an exposed setting that is never > touched. > > I prefer no change to nfs.conf, and put the uniqifier in a > separate file. > I think the important thing is, as Chuck said, that the setting of the uniquifier has to be automated. There are too many instances out there of people who get confused because they are using a default hostname, such as 'localhost.localdomain' and are setting no uniquifier. So the point is that it needs to be persisted by an automated script if unset. While that script could use nfsconf to get/set the persisted uniquifier, the worry is that such an automated change might be made while the user is performing some other edit of nfs.conf. What happens then? -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@xxxxxxxxxxxxxxx
