Latest Time Machine Patch
Jeremy Allison
jra at samba.org
Tue Oct 3 20:27:58 UTC 2017
On Mon, Oct 02, 2017 at 08:34:53PM -0400, Kevin Anderson wrote:
> Hi Jeremy/Ralph,
> I have attached a new patch
> (0001-s3-smbd-register-Time-Machine-shares-with-Avahi.patch) from Omri
> which adds the NULL checks and a TALLOC allocator. The remaining
> patches have not been modified. I have tested the Avahi change and
> things appear to function as expected. Can I have a review on these
> changes?
Yes, these look good to me. Just one request - did you check this
under valgrind to ensure it's clean ? If so then RB+ from me.
Cheers,
Jeremy.
> Kevin Anderson
>
> On Fri, Sep 29, 2017 at 8:27 PM, Jeremy Allison <jra at samba.org> wrote:
> > On Fri, Sep 29, 2017 at 07:21:46PM -0500, Omri Mor wrote:
> >> This was discussed previously. Did you read the warning I put in the top of the file?
> >>
> >> /*
> >> * Avahi aborts on allocation failure (OOM),
> >> * unless a custom allocator which doesn't do so has been set.
> >> *
> >> * This is particularly important for the avahi_string_list_*() functions,
> >> * which return NULL on allocation failure.
> >> * Since it should abort on allocation failure (before returning NULL),
> >> * we don't check the result.
> >> */
> >>
> >> Adding in a check for NULL doesn’t matter, as it aborts long before we would see that NULL.
> >> In fact, the documentation doesn’t even state that it /can/ return NULL—I only found that out by reading the Avahi source.
> >
> > Yes, I read the source to determine it can return NULL,
> > that's why I wanted the checks here.
> >
> > I must have missed the previous discussion on this, 'cos
> > I'd never have agreed to ignore NULL checks on allocation
> > return. Just because the current allocator aborts doesn't
> > mean the newest-shiny-updated one also will.
> >
> > Please add the explicit NULL checks, otherwise
> > we're going to get coverity false positives.
> >
> > Jeremy.
> From b1d3e9ce1bc7b0c48d83c429acf9410691365afc Mon Sep 17 00:00:00 2001
> From: Omri Mor <omri50 at gmail.com>
> Date: Sun, 1 Oct 2017 21:39:47 -0500
> Subject: [PATCH] s3/smbd: register Time Machine shares with Avahi
>
> Adds support for automatically registering the required _adisk._tcp
> mDNS record based on the setting of "fruit:time machine".
>
> Signed-off-by: Omri Mor <omri50 at gmail.com>
> ---
> source3/smbd/avahi_register.c | 97 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 96 insertions(+), 1 deletion(-)
>
> diff --git a/source3/smbd/avahi_register.c b/source3/smbd/avahi_register.c
> index c118e61ccc..91e8a439b8 100644
> --- a/source3/smbd/avahi_register.c
> +++ b/source3/smbd/avahi_register.c
> @@ -24,6 +24,8 @@
> #include <avahi-client/client.h>
> #include <avahi-client/publish.h>
> #include <avahi-common/error.h>
> +#include <avahi-common/malloc.h>
> +#include <avahi-common/strlst.h>
>
> struct avahi_state_struct {
> struct AvahiPoll *poll;
> @@ -32,6 +34,39 @@ struct avahi_state_struct {
> uint16_t port;
> };
>
> +static void *avahi_allocator_ctx = NULL;
> +
> +static void * avahi_allocator_malloc(size_t size)
> +{
> + return talloc_size(avahi_allocator_ctx, size);
> +}
> +
> +static void avahi_allocator_free(void *p)
> +{
> + TALLOC_FREE(p);
> +}
> +
> +static void * avahi_allocator_realloc(void *p, size_t size)
> +{
> + return talloc_realloc_size(avahi_allocator_ctx, p, size);
> +}
> +
> +static void * avahi_allocator_calloc(size_t count, size_t size)
> +{
> + void *p = talloc_array_size(avahi_allocator_ctx, size, count);
> + if (p) {
> + memset(p, 0, size * count);
> + }
> + return p;
> +}
> +
> +static const struct AvahiAllocator avahi_talloc_allocator = {
> + &avahi_allocator_malloc,
> + &avahi_allocator_free,
> + &avahi_allocator_realloc,
> + &avahi_allocator_calloc
> +};
> +
> static void avahi_entry_group_callback(AvahiEntryGroup *g,
> AvahiEntryGroupState status,
> void *userdata)
> @@ -70,7 +105,13 @@ static void avahi_client_callback(AvahiClient *c, AvahiClientState status,
> int error;
>
> switch (status) {
> - case AVAHI_CLIENT_S_RUNNING:
> + case AVAHI_CLIENT_S_RUNNING: {
> + int snum;
> + int num_services = lp_numservices();
> + int dk = 0;
> + AvahiStringList *adisk = NULL;
> + AvahiStringList *adisk2 = NULL;
> +
> DBG_DEBUG("AVAHI_CLIENT_S_RUNNING\n");
>
> state->entry_group = avahi_entry_group_new(
> @@ -94,6 +135,53 @@ static void avahi_client_callback(AvahiClient *c, AvahiClientState status,
> break;
> }
>
> + for (snum = 0; snum < num_services; snum++) {
> + if (lp_snum_ok(snum) &&
> + lp_parm_bool(snum, "fruit", "time machine", false))
> + {
> + adisk2 = avahi_string_list_add_printf(
> + adisk, "dk%d=adVN=%s,adVF=0x82",
> + dk++, lp_const_servicename(snum));
> + if (adisk2 == NULL) {
> + DBG_DEBUG("avahi_string_list_add_printf"
> + "failed: returned NULL\n");
> + avahi_string_list_free(adisk);
> + avahi_entry_group_free(state->entry_group);
> + state->entry_group = NULL;
> + break;
> + }
> + adisk = adisk2;
> + adisk2 = NULL;
> + }
> + }
> + if (dk > 0) {
> + adisk2 = avahi_string_list_add(adisk, "sys=adVF=0x100");
> + if (adisk2 == NULL) {
> + DBG_DEBUG("avahi_string_list_add failed: "
> + "returned NULL\n");
> + avahi_string_list_free(adisk);
> + avahi_entry_group_free(state->entry_group);
> + state->entry_group = NULL;
> + break;
> + }
> + adisk = adisk2;
> + adisk2 = NULL;
> +
> + error = avahi_entry_group_add_service_strlst(
> + state->entry_group, AVAHI_IF_UNSPEC,
> + AVAHI_PROTO_UNSPEC, 0, lp_netbios_name(),
> + "_adisk._tcp", NULL, NULL, 0, adisk);
> + avahi_string_list_free(adisk);
> + adisk = NULL;
> + if (error != AVAHI_OK) {
> + DBG_DEBUG("avahi_entry_group_add_service_strlst "
> + "failed: %s\n", avahi_strerror(error));
> + avahi_entry_group_free(state->entry_group);
> + state->entry_group = NULL;
> + break;
> + }
> + }
> +
> error = avahi_entry_group_commit(state->entry_group);
> if (error != AVAHI_OK) {
> DBG_DEBUG("avahi_entry_group_commit failed: %s\n",
> @@ -103,6 +191,7 @@ static void avahi_client_callback(AvahiClient *c, AvahiClientState status,
> break;
> }
> break;
> + }
> case AVAHI_CLIENT_FAILURE:
> error = avahi_client_errno(c);
>
> @@ -139,6 +228,12 @@ void *avahi_start_register(TALLOC_CTX *mem_ctx, struct tevent_context *ev,
> struct avahi_state_struct *state;
> int error;
>
> + avahi_allocator_ctx = talloc_new(mem_ctx);
> + if (avahi_allocator_ctx == NULL) {
> + return NULL;
> + }
> + avahi_set_allocator(&avahi_talloc_allocator);
> +
> state = talloc(mem_ctx, struct avahi_state_struct);
> if (state == NULL) {
> return state;
> --
> 2.14.2
>
> From 4f377e1321484a6fa767258b22d53b3287db0222 Mon Sep 17 00:00:00 2001
> From: Kevin Anderson <andersonkw2 at gmail.com>
> Date: Sun, 23 Oct 2016 20:32:27 -0400
> Subject: [PATCH 2/3] docs/vfs_fruit: Add Time Machine support
>
> Add the capability to advertise FULLSYNC volume capabilities
> to clients that request them. This is mainly used for supporting
> Mac OS Time Machine backups from clients. The capability does
> not perform any additional action.
>
> Signed-off-by: Kevin Anderson <andersonkw2 at gmail.com>
> ---
> docs-xml/manpages/vfs_fruit.8.xml | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/docs-xml/manpages/vfs_fruit.8.xml b/docs-xml/manpages/vfs_fruit.8.xml
> index c5ffc828fc3..b0cb9f0cb25 100644
> --- a/docs-xml/manpages/vfs_fruit.8.xml
> +++ b/docs-xml/manpages/vfs_fruit.8.xml
> @@ -214,6 +214,34 @@
> </varlistentry>
>
> <varlistentry>
> + <term>fruit:time machine = [ yes | no ]</term>
> + <listitem>
> + <para>Controls if Time Machine support via the FULLSYNC volume
> + capability is advertised to clients.</para>
> +
> + <itemizedlist>
> + <listitem><para><command>yes</command> - Enables Time Machine
> + support for this share. Also registers the share with mDNS in
> + case Samba is built with mDNS support.</para></listitem>
> +
> + <listitem><para><command>no (default)</command> Disables
> + advertising Time Machine support.</para></listitem>
> +
> + </itemizedlist>
> +
> + <para>This option enforces the following settings per share (or
> + for all shares if enabled globally):</para>
> + <itemizedlist>
> + <listitem><para><command>durable handles = yes</command></para></listitem>
> + <listitem><para><command>kernel oplocks = no</command></para></listitem>
> + <listitem><para><command>kernel share modes = no</command></para></listitem>
> + <listitem><para><command>posix locking = no</command></para></listitem>
> + </itemizedlist>
> +
> + </listitem>
> + </varlistentry>
> +
> + <varlistentry>
> <term>fruit:metadata = [ stream | netatalk ]</term>
> <listitem>
> <para>Controls where the OS X metadata stream is stored:</para>
> --
> 2.13.5
>
> From 4024d20819a1e3e0f7158730f8ef4b327f927bc2 Mon Sep 17 00:00:00 2001
> From: Kevin Anderson <andersonkw2 at gmail.com>
> Date: Mon, 14 Nov 2016 19:14:44 +0100
> Subject: [PATCH 1/3] vfs_fruit: Add Time Machine support
>
> Add a configuration option to disable/enable Time Machine support via
> the FULLSYNC AAPL flag.
>
> Signed-off-by: Kevin Anderson <andersonkw2 at gmail.com>
> ---
> libcli/smb/smb2_create_ctx.h | 1 +
> source3/modules/vfs_fruit.c | 21 +++++++++++++++++++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/libcli/smb/smb2_create_ctx.h b/libcli/smb/smb2_create_ctx.h
> index cb194f5c536..0e0b713e55c 100644
> --- a/libcli/smb/smb2_create_ctx.h
> +++ b/libcli/smb/smb2_create_ctx.h
> @@ -42,5 +42,6 @@
> /* "AAPL" Volume Capabilities bitmap */
> #define SMB2_CRTCTX_AAPL_SUPPORT_RESOLVE_ID 1
> #define SMB2_CRTCTX_AAPL_CASE_SENSITIVE 2
> +#define SMB2_CRTCTX_AAPL_FULL_SYNC 4
>
> #endif
> diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
> index 3ba59967482..5c9e680d299 100644
> --- a/source3/modules/vfs_fruit.c
> +++ b/source3/modules/vfs_fruit.c
> @@ -139,6 +139,7 @@ struct fruit_config_data {
> bool posix_rename;
> bool aapl_zero_file_id;
> const char *model;
> + bool time_machine;
>
> /*
> * Additional options, all enabled by default,
> @@ -1549,6 +1550,9 @@ static int init_fruit_config(vfs_handle_struct *handle)
> config->use_aapl = lp_parm_bool(
> -1, FRUIT_PARAM_TYPE_NAME, "aapl", true);
>
> + config->time_machine = lp_parm_bool(
> + SNUM(handle->conn), FRUIT_PARAM_TYPE_NAME, "time machine", false);
> +
> config->unix_info_enabled = lp_parm_bool(
> -1, FRUIT_PARAM_TYPE_NAME, "nfs_aces", true);
>
> @@ -2206,6 +2210,10 @@ static NTSTATUS check_aapl(vfs_handle_struct *handle,
> break;
> }
>
> + if (config->time_machine) {
> + caps |= SMB2_CRTCTX_AAPL_FULL_SYNC;
> + }
> +
> SBVAL(p, 0, caps);
>
> ok = data_blob_append(req, &blob, p, 8);
> @@ -2637,6 +2645,19 @@ static int fruit_connect(vfs_handle_struct *handle,
> "0x0d:0xf00d");
> }
>
> + if (config->time_machine) {
> + DBG_NOTICE("Enabling durable handles for Time Machine "
> + "support on [%s]\n", service);
> + lp_do_parameter(SNUM(handle->conn), "durable handles", "yes");
> + lp_do_parameter(SNUM(handle->conn), "kernel oplocks", "no");
> + lp_do_parameter(SNUM(handle->conn), "kernel share modes", "no");
> + if (!lp_strict_sync(SNUM(handle->conn))) {
> + DBG_WARNING("Time Machine without strict sync is not "
> + "recommended!\n");
> + }
> + lp_do_parameter(SNUM(handle->conn), "posix locking", "no");
> + }
> +
> return rc;
> }
>
> --
> 2.13.5
>
More information about the samba-technical
mailing list