-
Notifications
You must be signed in to change notification settings - Fork 296
CP-310956: Remove legacy winbind configuration #6822
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
232236 (Dev Run) Authentication sanity test looks good. |
7ef47bb to
7d443a5
Compare
7d443a5 to
2f7d1fe
Compare
|
Note: xcp-ng still hasn't updated to samba 4.20, this means that samba needs to be updated before this change is adapted in xcp-ng |
| ; "" (* Empty line at the end *) | ||
| ] | ||
| [ | ||
| "# autogenerated by xapi" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to change it to:
# autogenerated by xapi
# do not change because changes will be overwritten"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does't autogenerated by xapi already indicate changes will be overwritten? and we do expect customer to change it and restart winbind temporary for debug purpose.
do we have an estimation about when it be done? |
I don't think we need to wait to merge this, just to be aware of it, are you planning to add a dependency in the .spec file? |
|
I'm not fond of a XAPI update enforcing a samba upgrade on a maintenance release of XCP-ng. I need to better understand why this change absolutely needs to be merged prior to branching off the maintenance branch of XS 8.4+XCP-ng 8.3. |
|
@stormi , I agree in principle, but this particular update was needed to address a serious issue for Window VMs. See the linked commit 9a468bf and https://bugzilla.samba.org/show_bug.cgi?id=15876. |
|
|
This PR is not large or complex. So we could hold it back or only merge it into the next major release and keeping the old code around on this release. But we do would like to not carry along old code. |
In this case, I will hold this PR until xapi branch out. |
309c1e5 to
2f7d1fe
Compare
9a468bf updated samba to 4.2x and keep legacy configuration in smb.conf conditionally for backward compatibility. Now samba binary is updated in all releases, this commit just drop the legacy configurations Signed-off-by: Lin Liu <[email protected]>
2f7d1fe to
273f961
Compare
|
@psafont Can you help to tick the PR to indicate vates are happy with this merge? |
psafont
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have been told that samba 4.21.3 is being used in the xcp-ng 9 prototype, so the removal of support for older versions should be fine
9a468bf updated samba to 4.2x and keep legacy configuration in smb.conf conditionally for backward compatibility.
Now samba binary is updated in all releases, this commit just drop the legacy configurations