[PATCH] Domain backup and restore samba-tool commands

Douglas Bagnall douglas.bagnall at catalyst.net.nz
Wed Jun 27 05:59:15 UTC 2018


Hi Tim,

On 27/06/18 16:52, Tim Beale via samba-technical wrote:
> Hi,
> 
> Attached are the first few 'pre-work' patches for the backup tool (the
> backup tool is dependent on these changes, but this patch-set doesn't
> include the backup tool itself). These are ready for review and delivery
> (pending a clean CI run).
> https://gitlab.com/catalyst-samba/samba/pipelines/24648347
> 
> They're basically just a sub-set of the patches I sent out this morning,
> with the following changes:
> - new patch: tests: Add basic test for non-global LoadParm behaviour
> - new patch: tests: Add test that Samba cannot be started with a  backup DB
> - updated patch: samba: read backup date field on init and fail if
> present (the new test case highlighted that the code didn't actually work).
> 
> Review appreciated.
> 
> Thanks,
> Tim

#1 looks good!

#2:

> +	ldb_ctx = samdb_connect(db_context,
> +				event_ctx,
> +				cmdline_lp_ctx,
> +				system_session(cmdline_lp_ctx),
> +				NULL,
> +				0);

There should be a NULL check here...

> +	privilege_connect(db_context, cmdline_lp_ctx);

And I suppose this one too. Obviously you're not dereferencing it,
but presumably it does something useful and it would be better to hear
about the error sooner rather than later.

Otherwise it looks good.

I will look at the rest in the morning, though I did notice `git am`
had this complaint:

  Applying: param: Add non-global smb.cfg option (support 2 different smb.confs)
  .git/rebase-apply/patch:79: trailing whitespace.
  		
  warning: 1 line adds whitespace errors.
  Applying: tests: Add basic test for non-global LoadParm behaviour

cheers,
Douglas



More information about the samba-technical mailing list