CP-311169: samba: include /etc/samba/smb.extra.conf

[liulinC]

Samba has lots of feature and configurations,
https://www.samba.org/samba/docs/current/man-html/smb.conf.5.html

Some of them are passthrough from xapi configuration to samba However, this requires xapi rebuild and make xapi uncessary complicated.

On the other side, sometimes we do need customer to tune some args, especially for debug purpose, and we do want such configuration can persist cross xapi restart.

A new configuration /etc/samba/cust.conf is introduced. The xapi generated/overrided /etc/samba/smb.conf include it. Customer can update this file with guide of support team for debug purpose

[liulinC]

XenRT regression test looks good:
233474 (Dev Run)

[lindig]

• What happens if this file does not exist?
• Are we shipping an empty file and how?
• I am not sure cust is the best name; was thinking of local.conf or extra.conf. It sounds like this file would not be affected by updates. However, this may create problems if the content becomes incompatible. There is no easy solution here.

[lindig]

According to Copilot, a missing include file is logged but it does not lead to failure. But it would be good to verify this.

[stormi]

For multipath configuration there's a /etc/multipath/conf.d/custom.conf. Naming it the same would be consistent (better than cust.conf). However I agree that this name I initially proposed and that was adopted for multipath config is not necessarily the best we could have chosen, so I'm also open to other options, if we can find name that is without ambiguity.

[stormi]

I also advise to create the file in advance, and pre-fill it with comments explaining to users how to use the file, whether changes will be retained upon maintenance update, upon product version upgrades, and what the risks are with modifying it (introducing incompatible changes, or changes that are good now but may bitrot).

Here's how it looks currently on the multipath side:

[17:02 xcpng-ci-83-b1 ~]# head /etc/multipath.conf
# --- WARNING: DO NOT EDIT THIS FILE ---
# The contents of this file may be overwritten at any future time through a
# system update, causing any custom configuration to be lost.
#
# For custom multipath configuration, create a separate .conf file in the
# /etc/multipath/conf.d/ directory.
# --- END OF WARNING ---

# This configuration file is used to overwrite the built-in configuration of
# multipathd.

and

[17:03 xcpng-ci-83-b1 ~]# head /etc/multipath/conf.d/custom.conf 
# Custom configuration for multipathd

# Changes made to this file will not be overwritten by future system updates.
# They will also be retained through system upgrades to newer releases.

# Refer to "man multipath.conf"

Notice also how visible the warning is. This matters, in my opinion.

[liulinC]

• If the file does not exist, there is a warning in winbind/samba log and ignored
• I am going to ship an empty file(With proper desc as comments) by samba build, together with smb.conf
• It is a good question, I am also considering some other names. rpm update will not override this file and I am not going to preserve this file through RPU. This file is appended at the end to override xapi auto-generated values by intention. This should be only used carefully with TS guide and I am going to comment this in the file header.

[liulinC]

I also advise to create the file in advance, and pre-fill it with comments explaining to users how to use the file, whether changes will be retained upon maintenance update, upon product version upgrades, and what the risks are with modifying it (introducing incompatible changes, or changes that are good now but may bitrot).

Here's how it looks currently on the multipath side:

[17:02 xcpng-ci-83-b1 ~]# head /etc/multipath.conf
# --- WARNING: DO NOT EDIT THIS FILE ---
# The contents of this file may be overwritten at any future time through a
# system update, causing any custom configuration to be lost.
#
# For custom multipath configuration, create a separate .conf file in the
# /etc/multipath/conf.d/ directory.
# --- END OF WARNING ---

# This configuration file is used to overwrite the built-in configuration of
# multipathd.

and

[17:03 xcpng-ci-83-b1 ~]# head /etc/multipath/conf.d/custom.conf 
# Custom configuration for multipathd

# Changes made to this file will not be overwritten by future system updates.
# They will also be retained through system upgrades to newer releases.

# Refer to "man multipath.conf"

Notice also how visible the warning is. This matters, in my opinion.

Thanks, I am going to add a kind of notes as well. This file will goes to samba package, as smb.conf

[psafont]

Had a chat with the OS team at vates, there's no obvious support for loading conf files in a directory (/etc/smb.conf.d/), so this is the only approach that works.

Thanks for following through with this :)

[stormi]

I'm surprised and disappointed that this was merged without acknowledging my comments, the suggestions I made for more consistent configuration file handling. And also that there was a push immediately followed by a merge without a re-review from the maintainers.

Oh, sorry for the misunderstanding. Github was showing me the merged status, the new push, but not the new comments.

[liulinC]

I'm surprised and disappointed that this was merged without acknowledging my comments, the suggestions I made for more consistent configuration file handling. And also that there was a push immediately followed by a merge without a re-review from the maintainers.

Oh, sorry for the misunderstanding. Github was showing me the merged status, the new push, but not the new comments.

Do not worry.
JFI, /etc/samba/smb.extra.conf would contains some comments reminding the overrides from /etc/samba/smb.conf.
That file goes to samba package (same as /etc/samba/samb.conf) and it residents in our internal repo. (xcp-ng can get it once released from source rpm).
This is a simple PR and I do not want to bother and waste other maintainer time on every single piece of minor update.
Otherwise, the repo should set to un-approve on every push.