[LU-7004] fix "lctl set_param -P" to allow deprecation of "lctl conf_param"

• Status: Resolved
• Project: Lustre
• Affects Version/s: Lustre 2.9.0
• Fix Version/s: Lustre 2.13.0
• Type: Improvement
• Priority: Critical
• Reporter: Andreas Dilger
• Assignee: James A Simmons
• Resolution: Fixed
• Labels: llnl, patch

Description

The lctl set_param -P functionality was landed via LU-3155 for 2.5.0 and deprecates lctl conf_param usage. There is a deprecation message printed in jt_lcfg_mgsparam() since 2.7.53 that conf_param is obsolete and set_param should be used instead.

The test scripts need to be changed over to use "lctl set_param -P" exclusively, both to quiet the deprecation warning, as well as verify that the "lctl set_param -P" functionality is working correctly.

For now, I'm going to bump the deprecation warning to 2.8.53 to give us time to make this change in the 2.9 release cycle since there isn't enough time to do it for 2.8.0 anymore.

Issue Links

• is related to: LU-8414 DNE: Setting of remote_dir_gid parame...
• is related to: LU-3155 Permanent parameters with lctl set_pa...
• is related to: LU-10937 Use sysfs to fix up sptlrpc handling.
• is related to: LU-8482 auster run of sanity trying to use de...
• is related to: LU-9431 class_process_proc_param can't handle...
• is related to: LU-7031 lctl set_param -P behaves differently...
• is related to: LU-9399 lctl conf_param and lctl set_param -P...
• is related to: LU-10626 utils/tests: lctl set_param -P does n...
• is related to: LU-10756 Send Uevents for interesting Lustre c...

Comments

[Gerrit Updater]

Andreas Dilger ([e-mail]) uploaded a new patch: http://review.whamcloud.com/15979
Subject: LU-7004 utils: defer conf_param deprecation
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: e44795ec78b6d42fa8b6702cc6e4a57718cb5db1

[Gerrit Updater]

Andreas Dilger ([e-mail]) merged in patch http://review.whamcloud.com/15979/
Subject: LU-7004 utils: defer conf_param deprecation
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 1305c503ce158b3a8a65b8f03b54a3f4894c5108

[Christopher Morrone]

I would argue that deprecating lctl conf_param requires a lot more discussion and planning. lctl set_param -P is currently broken for some (many?) settings. There is also some question in my mind about whether lctl set_param -P is even a reasonable user interface.

[Andreas Dilger]

I bumped into the "conf_param is deprecated" message again after the build version was bumped to 2.8.53.

Artem, is there any plan to address the testing issues in this ticket and LU-7183, as well as update the User Manual to use set_param -P instead of conf_param?

For now I'm going to remove the deprecation warning completely instead of pushing it to a later release, since there has been no action on this issue in a year.

[Gerrit Updater]

Andreas Dilger ([e-mail]) uploaded a new patch: http://review.whamcloud.com/20573
Subject: LU-7004 utils: remove "lctl conf_param" deprecation
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 737413d7b7a0c5dd2e5a88c77ffe8d72df5847fd

[Artem Blagodarenko (Inactive)]

>Artem, is there any plan to address the testing issues in this ticket and LU-7183, as well as update the User Manual to use set_param -P instead of conf_param?

Andreas, yes sure. But I need to find time this.

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/20573/
Subject: LU-7004 utils: remove "lctl conf_param" deprecation
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: e3c6c6739d8edc48e325843e1075a1fe8c157ec9

[Peter Jones]

Look like everything landed for 2.9

[Andreas Dilger]

The landed patch only removed the deprecation of "conf_param", but didn't actually fix "set_param -P" at all.

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: https://review.whamcloud.com/28590
Subject: LU-7004 obd: make LCFG_SET_PARAM functional
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 2323a12774ab66d546a257240d6d931dcabc3af9

[James A Simmons]

After much sweat and tears I think I got lctl set_param -P working again for the most part. The issue with sptlrpc might be more involved but for the other stuff it appears to work now. Once we have this working I would suggest that we back port this to b2_10 and re-enable the lctl conf_param deprecate warning so user can move to lctl set_param -P before the next major LTS lustre version.

[James A Simmons]

Okay I rewrote the mgs llog to be more understandable. I do have one question which is the not clear in the docs. I see failover.nid being set with:

lctl conf_param $fsname-MDT0001.failover.node=$nid

Now with lctl set_param -P is it the same format:

lctl set_param -P $fsname-MDT0001.failover.node=$nid

or should it be:

lctl set_param -P mdt.$fsname-MDT0001.failover.node=$nid

[Andreas Dilger]

With lctl set_param -P it should be <device>.<instance>.param. That said, I can't find anything in /{proc,sys}/fs/lustre named either node or failover (excluding a few nodemap files). I suspec this is a "virtual" attribute, and not one that actually lives in the /proc tree somewhere?

[James A Simmons]

We have 4 classes of parameters.

Class one are the global values i.e PARAM_TIMEOUT, PARAM_JOBID_VAR. The patch for this ticket adds support for this class for set_param -P.

The second class are the ones settable at mount or initial format i.e PARAM_MGSNODE, PARAM_NETWORK. Those should never be changed with set_param -P so they are filtered out. The patch for this ticket adds that filtering. Currently you can stomp on those values with set_param -P

The 3rd class is your typical obd proc mapping. This somewhat worked for set_param -P.

Lastly we have your "virtual" devices. Some of these that were virtual can now be mapped to real proc/sysfs entries. The two for this case are PARAM_ACTIVATE and PARAM_ID_UPCALL. Its PARAM_FAILNODE and PARAM_SRPC that are currently virtual. I have managed to allow setting the failnode node using set_param -P but it doesn't transmit the change to the client for some reason. As for PARAM_SRPC I have no idea what the goal was there. In lustre_param.h it list srpc.flavor as a proc entry but it doesn't exist. Anyways I will have to cover that in LU-7183.

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: https://review.whamcloud.com/30087
Subject: LU-7004 tests: move from lctl conf_param to lctl set_param -P
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 57e99d4dd495dd0e69daee9dc52931c33fa45395

[James A Simmons]

I also got this working. Only one test fails with lctl set_param -P which I tracked down to the problem that the lcfg changes are only being transmitted to the clients. So trying to deactivate an OST/MDT only appears to happen on the client and the servers don't see the same change. It appears the choice of where to send the lcfg info is done in the mgc layer. Does anyone know the exact method of how this done?

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/28590/
Subject: LU-7004 obd: make LCFG_SET_PARAM functional
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: dfe60d0b98a1a888ca4ffce14788938c192b1520

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: https://review.whamcloud.com/31081
Subject: LU-7004 quota: make lctl set_param -P functional for quota
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: e68d0a91625c64dd66dc2f7395cf7dd102dba57b

[Gerrit Updater]

Minh Diep ([e-mail]) uploaded a new patch: https://review.whamcloud.com/31207
Subject: LU-7004 obd: make LCFG_SET_PARAM functional
Project: fs/lustre-release
Branch: b2_10
Current Patch Set: 1
Commit: 14d209c516d710585d4c492bfdfa6d2eb1add25d

[Gerrit Updater]

John L. Hammond ([e-mail]) merged in patch https://review.whamcloud.com/31207/
Subject: LU-7004 obd: make LCFG_SET_PARAM functional
Project: fs/lustre-release
Branch: b2_10
Current Patch Set:
Commit: d7f9f2130c5d8c74004f36aa68938df57778a6b1

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/31081/
Subject: LU-7004 quota: make lctl set_param -P functional for quota
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 6d9df3b2c0d54b16d00b6e419c7bb958e6b54844

[James A Simmons]

So the good news is that I found lctl conf_param lustre-OST*.failover.node=10.0.0.1@tcp can be done with set_param P with already existing code. The command to do it is lctl set_param -P osc.lustre-OST*.import=connection=10.0.0.1@tcp. So its just a matter of updating the test in this case. Only sptlrpc is not functional.

[James A Simmons]

looks like this work is coming to completion

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: https://review.whamcloud.com/32784
Subject: LU-7004 obd: handle fsname-MDT0000-**MDT0000 for obdname2fsname
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 29b45b5dd7c87dbf4b6168ca07d8bbd0acbafe84

[James A Simmons]

So I see one regression with lctl set_param -P.  Some test use the form get the parameter valu:

lctl set_param osc.lustre-OST0001-osc-[^M]*.active=1

The shell seems to expand this properly or the get_param routine parses this correctly.

Now if I run on the MGS lctl set_param "P" osc.lustre-OST0001-osc[^M]*.active=1 then the [^M] is actually cached in the config logs. The function obdname2fsname() chokes on this wildcard. So the question is how to handle this. I see the following:

1) Make the shell expand the [^M] to some value before passing the string to lctl itself. I don't know how to       do that

2) Use regex or something in the C code of lctl to expand this out before packing the data for the ioctl on           the MGS.

3) Change the test scripts to drop the [^M] for lctl set_param -P.

Which is the best option?

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/30087/
Subject: LU-7004 tests: move from lctl conf_param to lctl set_param -P
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: b9c359a70db9d5a6b69cc222c5f3b306a23df8b1

[James A Simmons]

Can patch https://review.whamcloud.com/#/c/32784 be reviewed so it can land for 2.12. It was discovered that obdname2fsname() doesn't cover all the cases so this patch removes the use of obdname2fsname() on the MSG server when dealing with config logs. Currently it can break in some corner cases. After this patch then obdname2fsname() will only be used with sptlrpc like before. That can be addressed in the follow on LU-10937 patch for 2.13.

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/32784/
Subject: LU-7004 mgs: remove using obdname2fsname() from mgs layer.
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 5da91cd4e44f8113b00035324738a9bb67f8a597

[Peter Jones]

James

Do I recall correctly - this is the only fix needed but it should be queued up for 2.12.1 in addition to being in 2.13?

Peter

[James A Simmons]

Yes this last patch should land for 2.12. Currently the params files on the MGS server can have malformed names. Rare but can happen. 

[Gerrit Updater]

"Andreas Dilger <[e-mail]>" uploaded a new patch: https://review.whamcloud.com/c/doc/manual/+/52553
Subject: LU-7004 utils: prefer 'set_param -P' over 'conf_param'
Project: doc/manual
Branch: master
Current Patch Set: 1
Commit: d8be3b93093855f1a1a27994fe37aa8daecf04ef

[Gerrit Updater]

"Andreas Dilger <[e-mail]>" merged in patch https://review.whamcloud.com/c/doc/manual/+/52553/
Subject: LU-7004 utils: prefer 'set_param -P' over 'conf_param'
Project: doc/manual
Branch: master
Current Patch Set:
Commit: 19c2cce1fa5a52aa2536cc75833bc8c0d4593d7d