Hi Amaury, Sorry for the delay, refactoring the commits took a bit longer than expected. I'm back after a discussion about this feature with Willy. We are both interested to see this feature merged. However there is still final remarks before this can be done. Happy to hear, thanks for the follow-up.
First, Willy has pointed to me that the backend option to allow renaming is probably unnecessary. It can be frustrating for users to discover at runtime that a server can't be renamed as the config option is missing. Also, add and delete server operation are not conditioned by an extra option, so it's better to keep a similar approach here. Agreed. The parity argument with add server / delete server makes sense. The idea was just to guard it since I expected it to be potentially unsafe. I removed the option entirely. Regarding the discussion about how to determine if a server can or cannot be renamed, here are our proposition. First, you should provide a new patch which introduces a new server flag, used for servers who are referenced in the configuration by their name. This SRV_F_NAME_REFD (I let you propose a better naming if you want) should replace SRV_F_NON_PURGEABLE in most places, except for the resolvers case. In the same patch, delete server handler must be adjusted to now test the flag combination SRV_F_NAME_REFD + SRV_F_NON_PURGEABLE. Done, kept your proposed name. It reads well enough that I didn't try to rename it. Reclassification of existing call sites: - proxy.c use-server resolver → SRV_F_NAME_REFD - server.c track chain setup → SRV_F_NAME_REFD - sample.c ARGT_SRV resolver → SRV_F_NAME_REFD - resolvers.c DNS link → unchanged (object pinning, not name) After the latest changes, del server now checks both flags so the set of servers it rejects is unchanged. The rename handler gates on just SRV_F_NAME_REFD. Hum I had a new look at the current code and I see that the server ID pointer can be directly referenced when using srv_name sample fetch. I'm not sure thus that the rename operation is totally safe. Maybe Willy or others whill have some ideas about this ? […] In fact, ACL results are usable immediately and not preserved in memory, so it seems that server rename should not have any issue with srv_name() sample fetch. Thanks, that matches what I see in the consumer paths. I understand that sample buffers are acted on within the current task (ACL evaluation, log format expansion, header insertion) and not retained across wakeups, so freeing the old name immediately under thread isolation is sufficient. The rename handler now does exactly that. However, looking at srv_name fetch I noticed that smp_fetch_srv_name() does not set SMP_F_CONST, while every sibling id-pointer fetch (smp_fetch_be_name on px->id, the SSL helpers using OBJ_nid2sn() / SSL_get_cipher_name(), etc.) correctly does. Without it, srv_name,lower writes into srv->id in place via smp_make_rw(). I guess it has gone unnoticed because srv->id is never read back by name. I split it out as patch 1 of the series and tagged BUG/MINOR so it can be backported on its own. If I am wrong here, feel free to skip this patch. Let me know what you think! Cheers, Alexander From: Amaury Denoyelle <[email protected]> Date: Wednesday, 24. June 2026 at 15:32 To: Stephan, Alexander <[email protected]> Cc: [email protected] <[email protected]> Subject: Re: [PATCH 1/3 2/3 3/3] MINOR: server: add 'option server-rename' and 'set server name' CLI command On Fri, Jun 19, 2026 at 11:12:54AM +0000, Stephan, Alexander wrote: > Hi, > Thanks for the second pass! > ▎ The PR_O3_SRV_RENAME approach seems good. > Good to hear. > ▎ Regarding the stick-table checks: these are not present in del server, > ▎ so either del server has a bug, or these checks are unnecessary here. > You are right. I removed the stick-table/peer-sync checks to stay consistent > with del server. > ▎ srv_validate_name(): add server already goes through _srv_parse_init() > ▎ which calls invalid_char(), so the call in cli_parse_add_server() is > ▎ redundant. And for rename, invalid_char() should be sufficient. > Agreed. I removed srv_validate_name() entirely. The rename path now uses > invalid_char() for the character-set check (which covers > [A-Za-z0-9_:.-], including : for IP:port names) plus an explicit length > check against the event data name field and an empty-string check for > a cleaner error message. The redundant call in cli_parse_add_server() is also > gone. > ▎ Formatting: checkpatch will complain but that is not an issue. > Alright, good to know, thanks. > Then, I am looking forward to your feedback after the internal discussion on > the resolver/SRV_F_NON_PURGEABLE question. Hi Alexander, I'm back after a discussion about this feature with Willy. We are both interested to see this feature merged. However there is still final remarks before this can be done. First, Willy has pointed to me that the backend option to allow renaming is probably unnecessary. It can be frustrating for users to discover at runtime that a server can't be renamed as the config option is missing. Also, add and delete server operation are not conditionned by an extra option, so it's better to keep a similar approach here. Regarding the discussion about how to determine if a server can or cannot be renamed, here are our proposition. First, you should provide a new patch which introduces a new server flag, used for servers who are referenced in the configuration by their name. This SRV_F_NAME_REFD (I let you propose a better naming if you want) should replace SRV_F_NON_PURGEABLE in most places, except for the resolvers case. In the same patch, delete server handler must be adjusted to now test the flag combination SRV_F_NAME_REFD + SRV_F_NON_PURGEABLE. Then, in the patch dedicated to the rename server handler, you just have to check for SRV_F_NAME_REFD. This guarantees that rename operation will only be blocked due to a name reference. This is the better approach for future evolutions. Hum I had a new look at the current code and I see that the server ID pointer can be directly referenced when using srv_name sample fetch. I'm not sure thus that the rename operation is totally safe. Maybe Willy or others whill have some ideas about this ? Regards, -- Amaury Denoyelle
0001-BUG-MINOR-sample-set-SMP_F_CONST-on-srv_name-fetch.patch
Description: 0001-BUG-MINOR-sample-set-SMP_F_CONST-on-srv_name-fetch.patch
0002-MINOR-server-distinguish-name-references-with-new-SR.patch
Description: 0002-MINOR-server-distinguish-name-references-with-new-SR.patch
0003-MINOR-server-add-set-server-name-CLI-command-for-run.patch
Description: 0003-MINOR-server-add-set-server-name-CLI-command-for-run.patch
0004-REGTESTS-server-add-test-for-set-server-name-CLI-com.patch
Description: 0004-REGTESTS-server-add-test-for-set-server-name-CLI-com.patch
0005-DOC-server-document-set-server-name-CLI-command.patch
Description: 0005-DOC-server-document-set-server-name-CLI-command.patch
