Success story with patch: Samba4 on FreeBSD - one year in production

[Andrew Bartlett]

On Mon, 2012-05-21 at 14:39 +0300, Andriy Syrovenko wrote:
> Hi,
> 
> We were running Samba4a14 on FreeBSD 8.2 in production for more then a
> year, and have recently upgraded to a20 on FreeBSD 9.0. It serves a
> small network of about 50 PCs running XP, Vista and 7. Despite being
> named 'alpha', Samba4 works really stable- no major issues so far.
> Great work, folks!
> 
> Please find attached a small patch I prepared against alpha20. This
> patch has to be applied to the Samba4's source tree in order to make
> Samba4 successfully build and run on FreeBSD. The patch addresses
> three issues:

To get the changes into master, we need to make a few changes.  If you
want to take them on, then great, otherwise I'll use it as a TODO list.

> 1. Quite a few of Samba's external dependencies are not in the base on
> FreeBSD, but rather installed from ports/packages, and thus reside in
> /usr/local (/usr/local/include for headers and /usr/local/lib for
> shared objects). These are, for instance, gettext, iconv, popt just to
> name a few. So I modified 'buildtools/wafsamba/samba_optimisation.py'
> to append the required paths to _CCINCFLAGS and LINKFLAGS.

I've started work on this.  See how in master --with-libiconv now takes
an argument and defaults to /usr/local?  This we need to do for all the
other packages you list. 

> 2. There are 'sys/uuid.h' header in /usr/include on FreeBSD, which
> defines struct uuid but does not define any functions to work with it.
> The e2fsprogs-libuuid package, on the other hand, installs
> 'uuid/uuid.h' header in /usr/local/include, which defines both struct
> uuid as well as related functions. The problem is the definitions of
> struct uuid in these files are different and are not compatible with
> each other, so I modified #ifdef in 'lib/addns/dnsutils.c' file so
> that only 'uuid/uuid.h' header is included in case both header are
> present in the system.

I've looked at the uuid use, and I can't see any good excuse for this
external dependency in this code.  We have our own GUID handlers,
including GUID_random() and GUID_string().  The adddns code should just
use that. 

> 3. The third part of the patch adds extattr support on FreeBSD to the
> Samba4 binaries. Extended attributes handling on FreeBSD differs from
> Linux in that the attribute name does not include the namespace name.
> Namespace is specified as an additional (numeric) parameter to the
> extattr related function instead. Additionally, there only two
> universally defined namespaces- system and user, and there no security
> namespace. So, I decided to put everything into the user namespace
> using the original attribute names as they are defined in Samba
> (including '.' character which seems to be perfectly legal on
> FreeBSD).

The correct way to handle this is to move the code in
source3/lib/system.c to lib/replace, changing it from sys_fgetxattr to
rep_fgetxattr(), and then adding the replacement macros.

> Please consider the possibility to include this patch into the main
> source tree. I can work on some changes if required.

I would very much appreciate it if you could, but understand if it's too
much. 

> There were also some issues while upgrading a14 to a20 which I'd like to report.
> 
> 1. There were some locking problems that resulted in the following
> error messages during upgradeprovision:
> 
> ltdb: tdb(/usr/local/samba/private/sam.ldb): Locking error:
> tdb_allrecord_upgrade failed: count 0 too high
> ltdb: tdb(/usr/local/samba/private/sam.ldb): Locking error:
> tdb1_transaction_prepare_commit: failed to upgrade hash locks
> ltdb: tdb(/usr/local/samba/private/sam.ldb.d/CN=SCHEMA,CN=CONFIGURATION,DC=SYTOSS,DC=INTRA.ldb):
> Invalid parameter: tdb1_transaction_cancel: no transaction
> ltdb: tdb(/usr/local/samba/private/sam.ldb.d/CN=CONFIGURATION,DC=SYTOSS,DC=INTRA.ldb):
> Invalid parameter: tdb1_transaction_cancel: no transaction
> ltdb: tdb(/usr/local/samba/private/sam.ldb.d/DC=SYTOSS,DC=INTRA.ldb):
> Invalid parameter: tdb1_transaction_cancel: no transaction
> partition del transaction mismatch
> 
> To deal with them I just recreated all LDB files using tdbdump and
> tdbrestore prior to upgradeprovision. Hope I did not break anything.
> :)

That's odd.  I'll leave this for a tdb maintainer to comment on.

> 2. Alpha17's readme file suggests running 'samba-tool dbcheck
> --cross-ncs --fix'. As it turned out this tool tries to fix some
> errors in schema partitions as well, which is not allowed by default,
> so the right way to run this tool seems to be:
> 
> samba-tool dbcheck --cross-ncs --fix --option="dsdb:schema update allowed"=true

Thanks, we will need to ensure that the schema update dis-allowance code
considers the dbwrap control 

> 3. I had to add the following line to 'samba_upgradedns', so it could
> locate the required modules.
> 
> sys.path.insert(0, "/usr/local/samba/lib/python2.7/site-packages")

I think this has been fixed - because in the original source used "
rather than ' quotes, it wasn't replaced during make install. 

> 4. Bind DLZ module requires some libraries that are placed in
> /usr/local/samba/lib/private, which does not seem to be in RPATH for
> these modules. The libraries in question are 'libsamdb-common.so',
> 'libldb.so.1', 'libauthkrb5.so', 'libndr-samba.so' and 'libevents.so'.
> To fix the problem I created symlinks to these libraries in
> /usr/local/samba/lib, though I am not sure it was the right way to go.

I understand this has since been fixed.

Thank you very much for the feedback, and do let me know if you want to
take on any of the changes.

Thanks!

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org